Commit 550466dd authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'store-variables-and-when-in-builds-table' into 'master'

Store when and yaml variables in builds table

## What does this MR do?
Stores `when` and `yaml_variables` in `ci_builds` table.

## Why was this MR needed?
This is done in order to simplify the code responsible for creating a pipeline builds.

## What are the relevant issue numbers?
This made as a pre-step for https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5295.

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !5296
parents 2556d6d3 ffea9c46
...@@ -16,6 +16,7 @@ v 8.10.0 (unreleased) ...@@ -16,6 +16,7 @@ v 8.10.0 (unreleased)
- Align flash messages with left side of page content !4959 (winniehell) - Align flash messages with left side of page content !4959 (winniehell)
- Display tooltip for "Copy to Clipboard" button !5164 (winniehell) - Display tooltip for "Copy to Clipboard" button !5164 (winniehell)
- Use default cursor for table header of project files !5165 (winniehell) - Use default cursor for table header of project files !5165 (winniehell)
- Store when and yaml variables in builds table
- Display last commit of deleted branch in push events !4699 (winniehell) - Display last commit of deleted branch in push events !4699 (winniehell)
- Escape file extension when parsing search results !5141 (winniehell) - Escape file extension when parsing search results !5141 (winniehell)
- Apply the trusted_proxies config to the rack request object for use with rack_attack - Apply the trusted_proxies config to the rack request object for use with rack_attack
......
...@@ -5,6 +5,7 @@ module Ci ...@@ -5,6 +5,7 @@ module Ci
belongs_to :erased_by, class_name: 'User' belongs_to :erased_by, class_name: 'User'
serialize :options serialize :options
serialize :yaml_variables
validates :coverage, numericality: true, allow_blank: true validates :coverage, numericality: true, allow_blank: true
validates_presence_of :ref validates_presence_of :ref
...@@ -52,6 +53,8 @@ module Ci ...@@ -52,6 +53,8 @@ module Ci
new_build.stage = build.stage new_build.stage = build.stage
new_build.stage_idx = build.stage_idx new_build.stage_idx = build.stage_idx
new_build.trigger_request = build.trigger_request new_build.trigger_request = build.trigger_request
new_build.yaml_variables = build.yaml_variables
new_build.when = build.when
new_build.user = user new_build.user = user
new_build.environment = build.environment new_build.environment = build.environment
new_build.save new_build.save
...@@ -118,7 +121,12 @@ module Ci ...@@ -118,7 +121,12 @@ module Ci
end end
def variables def variables
predefined_variables + yaml_variables + project_variables + trigger_variables variables = []
variables += predefined_variables
variables += yaml_variables if yaml_variables
variables += project_variables
variables += trigger_variables
variables
end end
def merge_request def merge_request
...@@ -395,30 +403,6 @@ module Ci ...@@ -395,30 +403,6 @@ module Ci
self.update(erased_by: user, erased_at: Time.now, artifacts_expire_at: nil) self.update(erased_by: user, erased_at: Time.now, artifacts_expire_at: nil)
end end
def yaml_variables
global_yaml_variables + job_yaml_variables
end
def global_yaml_variables
if pipeline.config_processor
pipeline.config_processor.global_variables.map do |key, value|
{ key: key, value: value, public: true }
end
else
[]
end
end
def job_yaml_variables
if pipeline.config_processor
pipeline.config_processor.job_variables(name).map do |key, value|
{ key: key, value: value, public: true }
end
else
[]
end
end
def project_variables def project_variables
project.variables.map do |variable| project.variables.map do |variable|
{ key: variable.key, value: variable.value, public: false } { key: variable.key, value: variable.value, public: false }
......
...@@ -36,7 +36,9 @@ module Ci ...@@ -36,7 +36,9 @@ module Ci
:allow_failure, :allow_failure,
:stage, :stage,
:stage_idx, :stage_idx,
:environment) :environment,
:when,
:yaml_variables)
build_attrs.merge!(pipeline: @pipeline, build_attrs.merge!(pipeline: @pipeline,
ref: @pipeline.ref, ref: @pipeline.ref,
......
class AddWhenAndYamlVariablesToCiBuilds < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
def change
add_column :ci_builds, :when, :string
add_column :ci_builds, :yaml_variables, :text
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20160715134306) do ActiveRecord::Schema.define(version: 20160716115710) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -168,6 +168,8 @@ ActiveRecord::Schema.define(version: 20160715134306) do ...@@ -168,6 +168,8 @@ ActiveRecord::Schema.define(version: 20160715134306) do
t.string "environment" t.string "environment"
t.datetime "artifacts_expire_at" t.datetime "artifacts_expire_at"
t.integer "artifacts_size" t.integer "artifacts_size"
t.string "when"
t.text "yaml_variables"
end end
add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree
......
...@@ -31,28 +31,34 @@ module Ci ...@@ -31,28 +31,34 @@ module Ci
raise ValidationError, e.message raise ValidationError, e.message
end end
def builds_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil) def jobs_for_ref(ref, tag = false, trigger_request = nil)
builds.select do |build| @jobs.select do |_, job|
build[:stage] == stage && process?(job[:only], job[:except], ref, tag, trigger_request)
process?(build[:only], build[:except], ref, tag, trigger_request)
end end
end end
def builds def jobs_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil)
@jobs.map do |name, job| jobs_for_ref(ref, tag, trigger_request).select do |_, job|
build_job(name, job) job[:stage] == stage
end end
end end
def global_variables def builds_for_ref(ref, tag = false, trigger_request = nil)
@variables jobs_for_ref(ref, tag, trigger_request).map do |name, job|
build_job(name, job)
end
end end
def job_variables(name) def builds_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil)
job = @jobs[name.to_sym] jobs_for_stage_and_ref(stage, ref, tag, trigger_request).map do |name, job|
return [] unless job build_job(name, job)
end
end
job[:variables] || [] def builds
@jobs.map do |name, job|
build_job(name, job)
end
end end
private private
...@@ -95,11 +101,10 @@ module Ci ...@@ -95,11 +101,10 @@ module Ci
commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"), commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"),
tag_list: job[:tags] || [], tag_list: job[:tags] || [],
name: name, name: name,
only: job[:only],
except: job[:except],
allow_failure: job[:allow_failure] || false, allow_failure: job[:allow_failure] || false,
when: job[:when] || 'on_success', when: job[:when] || 'on_success',
environment: job[:environment], environment: job[:environment],
yaml_variables: yaml_variables(name),
options: { options: {
image: job[:image] || @image, image: job[:image] || @image,
services: job[:services] || @services, services: job[:services] || @services,
...@@ -111,6 +116,24 @@ module Ci ...@@ -111,6 +116,24 @@ module Ci
} }
end end
def yaml_variables(name)
variables = global_variables.merge(job_variables(name))
variables.map do |key, value|
{ key: key, value: value, public: true }
end
end
def global_variables
@variables || {}
end
def job_variables(name)
job = @jobs[name.to_sym]
return {} unless job
job[:variables] || {}
end
def validate! def validate!
@jobs.each do |name, job| @jobs.each do |name, job|
validate_job!(name, job) validate_job!(name, job)
......
...@@ -15,6 +15,11 @@ FactoryGirl.define do ...@@ -15,6 +15,11 @@ FactoryGirl.define do
services: ["postgres"] services: ["postgres"]
} }
end end
yaml_variables do
[
{ key: :DB_NAME, value: 'postgres', public: true }
]
end
pipeline factory: :ci_pipeline pipeline factory: :ci_pipeline
......
...@@ -19,15 +19,14 @@ module Ci ...@@ -19,15 +19,14 @@ module Ci
expect(config_processor.builds_for_stage_and_ref(type, "master").first).to eq({ expect(config_processor.builds_for_stage_and_ref(type, "master").first).to eq({
stage: "test", stage: "test",
stage_idx: 1, stage_idx: 1,
except: nil,
name: :rspec, name: :rspec,
only: nil,
commands: "pwd\nrspec", commands: "pwd\nrspec",
tag_list: [], tag_list: [],
options: {}, options: {},
allow_failure: false, allow_failure: false,
when: "on_success", when: "on_success",
environment: nil, environment: nil,
yaml_variables: []
}) })
end end
...@@ -432,11 +431,9 @@ module Ci ...@@ -432,11 +431,9 @@ module Ci
expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1)
expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
except: nil,
stage: "test", stage: "test",
stage_idx: 1, stage_idx: 1,
name: :rspec, name: :rspec,
only: nil,
commands: "pwd\nrspec", commands: "pwd\nrspec",
tag_list: [], tag_list: [],
options: { options: {
...@@ -446,6 +443,7 @@ module Ci ...@@ -446,6 +443,7 @@ module Ci
allow_failure: false, allow_failure: false,
when: "on_success", when: "on_success",
environment: nil, environment: nil,
yaml_variables: []
}) })
end end
...@@ -461,11 +459,9 @@ module Ci ...@@ -461,11 +459,9 @@ module Ci
expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1)
expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
except: nil,
stage: "test", stage: "test",
stage_idx: 1, stage_idx: 1,
name: :rspec, name: :rspec,
only: nil,
commands: "pwd\nrspec", commands: "pwd\nrspec",
tag_list: [], tag_list: [],
options: { options: {
...@@ -475,101 +471,126 @@ module Ci ...@@ -475,101 +471,126 @@ module Ci
allow_failure: false, allow_failure: false,
when: "on_success", when: "on_success",
environment: nil, environment: nil,
yaml_variables: []
}) })
end end
end end
describe 'Variables' do describe 'Variables' do
context 'when global variables are defined' do let(:config_processor) { GitlabCiYamlProcessor.new(YAML.dump(config), path) }
it 'returns global variables' do
variables = {
VAR1: 'value1',
VAR2: 'value2',
}
config = YAML.dump({ subject { config_processor.builds.first[:yaml_variables] }
context 'when global variables are defined' do
let(:variables) do
{ VAR1: 'value1', VAR2: 'value2' }
end
let(:config) do
{
variables: variables, variables: variables,
before_script: ['pwd'], before_script: ['pwd'],
rspec: { script: 'rspec' } rspec: { script: 'rspec' }
}) }
end
config_processor = GitlabCiYamlProcessor.new(config, path) it 'returns global variables' do
expect(subject).to contain_exactly(
{ key: :VAR1, value: 'value1', public: true },
{ key: :VAR2, value: 'value2', public: true }
)
end
end
expect(config_processor.global_variables).to eq(variables) context 'when job and global variables are defined' do
let(:global_variables) do
{ VAR1: 'global1', VAR3: 'global3' }
end
let(:job_variables) do
{ VAR1: 'value1', VAR2: 'value2' }
end
let(:config) do
{
before_script: ['pwd'],
variables: global_variables,
rspec: { script: 'rspec', variables: job_variables }
}
end
it 'returns all unique variables' do
expect(subject).to contain_exactly(
{ key: :VAR3, value: 'global3', public: true },
{ key: :VAR1, value: 'value1', public: true },
{ key: :VAR2, value: 'value2', public: true }
)
end end
end end
context 'when job variables are defined' do context 'when job variables are defined' do
context 'when syntax is correct' do let(:config) do
it 'returns job variables' do {
variables = { before_script: ['pwd'],
KEY1: 'value1', rspec: { script: 'rspec', variables: variables }
SOME_KEY_2: 'value2'
} }
end
config = YAML.dump( context 'when also global variables are defined' do
{ before_script: ['pwd'],
rspec: {
variables: variables,
script: 'rspec' }
})
config_processor = GitlabCiYamlProcessor.new(config, path) end
expect(config_processor.job_variables(:rspec)).to eq variables context 'when syntax is correct' do
let(:variables) do
{ VAR1: 'value1', VAR2: 'value2' }
end
it 'returns job variables' do
expect(subject).to contain_exactly(
{ key: :VAR1, value: 'value1', public: true },
{ key: :VAR2, value: 'value2', public: true }
)
end end
end end
context 'when syntax is incorrect' do context 'when syntax is incorrect' do
context 'when variables defined but invalid' do context 'when variables defined but invalid' do
it 'raises error' do let(:variables) do
variables = [:KEY1, 'value1', :KEY2, 'value2'] [ :VAR1, 'value1', :VAR2, 'value2' ]
end
config = YAML.dump(
{ before_script: ['pwd'],
rspec: {
variables: variables,
script: 'rspec' }
})
expect { GitlabCiYamlProcessor.new(config, path) } it 'raises error' do
expect { subject }
.to raise_error(GitlabCiYamlProcessor::ValidationError, .to raise_error(GitlabCiYamlProcessor::ValidationError,
/job: variables should be a map/) /job: variables should be a map/)
end end
end end
context 'when variables key defined but value not specified' do context 'when variables key defined but value not specified' do
it 'returns empty array' do let(:variables) do
config = YAML.dump( nil
{ before_script: ['pwd'], end
rspec: {
variables: nil,
script: 'rspec' }
})
config_processor = GitlabCiYamlProcessor.new(config, path)
it 'returns empty array' do
## ##
# When variables config is empty, we assume this is a valid # When variables config is empty, we assume this is a valid
# configuration, see issue #18775 # configuration, see issue #18775
# #
expect(config_processor.job_variables(:rspec)) expect(subject).to be_an_instance_of(Array)
.to be_an_instance_of(Array).and be_empty expect(subject).to be_empty
end end
end end
end end
end end
context 'when job variables are not defined' do context 'when job variables are not defined' do
it 'returns empty array' do let(:config) do
config = YAML.dump({ {
before_script: ['pwd'], before_script: ['pwd'],
rspec: { script: 'rspec' } rspec: { script: 'rspec' }
}) }
end
config_processor = GitlabCiYamlProcessor.new(config, path)
expect(config_processor.job_variables(:rspec)).to eq [] it 'returns empty array' do
expect(subject).to be_an_instance_of(Array)
expect(subject).to be_empty
end end
end end
end end
...@@ -681,11 +702,9 @@ module Ci ...@@ -681,11 +702,9 @@ module Ci
expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1) expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1)
expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({ expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
except: nil,
stage: "test", stage: "test",
stage_idx: 1, stage_idx: 1,
name: :rspec, name: :rspec,
only: nil,
commands: "pwd\nrspec", commands: "pwd\nrspec",
tag_list: [], tag_list: [],
options: { options: {
...@@ -701,6 +720,7 @@ module Ci ...@@ -701,6 +720,7 @@ module Ci
when: "on_success", when: "on_success",
allow_failure: false, allow_failure: false,
environment: nil, environment: nil,
yaml_variables: []
}) })
end end
...@@ -819,17 +839,16 @@ module Ci ...@@ -819,17 +839,16 @@ module Ci
it "doesn't create jobs that start with dot" do it "doesn't create jobs that start with dot" do
expect(subject.size).to eq(1) expect(subject.size).to eq(1)
expect(subject.first).to eq({ expect(subject.first).to eq({
except: nil,
stage: "test", stage: "test",
stage_idx: 1, stage_idx: 1,
name: :normal_job, name: :normal_job,
only: nil,
commands: "test", commands: "test",
tag_list: [], tag_list: [],
options: {}, options: {},
when: "on_success", when: "on_success",
allow_failure: false, allow_failure: false,
environment: nil, environment: nil,
yaml_variables: []
}) })
end end
end end
...@@ -865,30 +884,28 @@ module Ci ...@@ -865,30 +884,28 @@ module Ci
it "is correctly supported for jobs" do it "is correctly supported for jobs" do
expect(subject.size).to eq(2) expect(subject.size).to eq(2)
expect(subject.first).to eq({ expect(subject.first).to eq({
except: nil,
stage: "build", stage: "build",
stage_idx: 0, stage_idx: 0,
name: :job1, name: :job1,
only: nil,
commands: "execute-script-for-job", commands: "execute-script-for-job",
tag_list: [], tag_list: [],
options: {}, options: {},
when: "on_success", when: "on_success",
allow_failure: false, allow_failure: false,
environment: nil, environment: nil,
yaml_variables: []
}) })
expect(subject.second).to eq({ expect(subject.second).to eq({
except: nil,
stage: "build", stage: "build",
stage_idx: 0, stage_idx: 0,
name: :job2, name: :job2,
only: nil,
commands: "execute-script-for-job", commands: "execute-script-for-job",
tag_list: [], tag_list: [],
options: {}, options: {},
when: "on_success", when: "on_success",
allow_failure: false, allow_failure: false,
environment: nil, environment: nil,
yaml_variables: []
}) })
end end
end end
......
...@@ -208,7 +208,7 @@ describe Ci::Build, models: true do ...@@ -208,7 +208,7 @@ describe Ci::Build, models: true do
end end
before do before do
build.update_attributes(stage: 'stage') build.update_attributes(stage: 'stage', yaml_variables: yaml_variables)
end end
it { is_expected.to eq(predefined_variables + yaml_variables) } it { is_expected.to eq(predefined_variables + yaml_variables) }
...@@ -260,22 +260,6 @@ describe Ci::Build, models: true do ...@@ -260,22 +260,6 @@ describe Ci::Build, models: true do
it { is_expected.to eq(predefined_variables + predefined_trigger_variable + yaml_variables + secure_variables + trigger_variables) } it { is_expected.to eq(predefined_variables + predefined_trigger_variable + yaml_variables + secure_variables + trigger_variables) }
end end
context 'when job variables are defined' do
##
# Job-level variables are defined in gitlab_ci.yml fixture
#
context 'when job variables are unique' do
let(:build) { create(:ci_build, name: 'staging') }
it 'includes job variables' do
expect(subject).to include(
{ key: :KEY1, value: 'value1', public: true },
{ key: :KEY2, value: 'value2', public: true }
)
end
end
end
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment