diff options
| -rw-r--r-- | changelogs/unreleased/job-rules-e2e.yml | 5 | ||||
| -rw-r--r-- | lib/gitlab/ci/build/rules.rb | 9 | ||||
| -rw-r--r-- | lib/gitlab/ci/build/rules/rule/clause.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/ci/config/entry/job.rb | 10 | ||||
| -rw-r--r-- | lib/gitlab/ci/config/entry/rules.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/ci/pipeline/seed/build.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/ci/yaml_processor.rb | 1 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/pipeline/seed/build_spec.rb | 4 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/yaml_processor_spec.rb | 38 | ||||
| -rw-r--r-- | spec/services/ci/create_pipeline_service_spec.rb | 335 |
10 files changed, 382 insertions, 30 deletions
diff --git a/changelogs/unreleased/job-rules-e2e.yml b/changelogs/unreleased/job-rules-e2e.yml new file mode 100644 index 00000000000..2c55dfcec49 --- /dev/null +++ b/changelogs/unreleased/job-rules-e2e.yml @@ -0,0 +1,5 @@ +--- +title: Passing job rules downstream and E2E specs for job:rules configuration +merge_request: 32609 +author: +type: fixed diff --git a/lib/gitlab/ci/build/rules.rb b/lib/gitlab/ci/build/rules.rb index 89623a809c9..43399c74457 100644 --- a/lib/gitlab/ci/build/rules.rb +++ b/lib/gitlab/ci/build/rules.rb @@ -6,7 +6,14 @@ module Gitlab class Rules include ::Gitlab::Utils::StrongMemoize - Result = Struct.new(:when, :start_in) + Result = Struct.new(:when, :start_in) do + def build_attributes + { + when: self.when, + options: { start_in: start_in }.compact + }.compact + end + end def initialize(rule_hashes, default_when = 'on_success') @rule_list = Rule.fabricate_list(rule_hashes) diff --git a/lib/gitlab/ci/build/rules/rule/clause.rb b/lib/gitlab/ci/build/rules/rule/clause.rb index ff0baf3348c..bf787fe95a6 100644 --- a/lib/gitlab/ci/build/rules/rule/clause.rb +++ b/lib/gitlab/ci/build/rules/rule/clause.rb @@ -13,9 +13,7 @@ module Gitlab UnknownClauseError = Class.new(StandardError) def self.fabricate(type, value) - type = type.to_s.camelize - - self.const_get(type).new(value) if self.const_defined?(type) + "#{self}::#{type.to_s.camelize}".safe_constantize&.new(value) end def initialize(spec) diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 3009c7e8329..f750886a8c5 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -122,7 +122,7 @@ module Gitlab helpers :before_script, :script, :stage, :type, :after_script, :cache, :image, :services, :only, :except, :variables, - :artifacts, :environment, :coverage, :retry, + :artifacts, :environment, :coverage, :retry, :rules, :parallel, :needs, :interruptible attributes :script, :tags, :allow_failure, :when, :dependencies, @@ -145,6 +145,13 @@ module Gitlab end @entries.delete(:type) + + # This is something of a hack, see issue for details: + # https://gitlab.com/gitlab-org/gitlab-ce/issues/67150 + if !only_defined? && has_rules? + @entries.delete(:only) + @entries.delete(:except) + end end inherit!(deps) @@ -203,6 +210,7 @@ module Gitlab cache: cache_value, only: only_value, except: except_value, + rules: has_rules? ? rules_value : nil, variables: variables_defined? ? variables_value : {}, environment: environment_defined? ? environment_value : nil, environment_name: environment_defined? ? environment_value[:name] : nil, diff --git a/lib/gitlab/ci/config/entry/rules.rb b/lib/gitlab/ci/config/entry/rules.rb index 65cad0880f5..2fbc3d9e367 100644 --- a/lib/gitlab/ci/config/entry/rules.rb +++ b/lib/gitlab/ci/config/entry/rules.rb @@ -26,6 +26,10 @@ module Gitlab end end end + + def value + @config + end end end end diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index 1066331062b..1f6b3853069 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/lib/gitlab/ci/pipeline/seed/build.rb @@ -145,7 +145,7 @@ module Gitlab def rules_attributes strong_memoize(:rules_attributes) do - @using_rules ? @rules.evaluate(@pipeline, self).to_h.compact : {} + @using_rules ? @rules.evaluate(@pipeline, self).build_attributes : {} end end end diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 501d91fa9ad..986605efdc3 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -42,6 +42,7 @@ module Gitlab yaml_variables: yaml_variables(name), needs_attributes: job[:needs]&.map { |need| { name: need } }, interruptible: job[:interruptible], + rules: job[:rules], options: { image: job[:image], services: job[:services], diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index 89431b80be3..023d7530b4b 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -46,7 +46,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do context 'is matched' do let(:attributes) { { name: 'rspec', ref: 'master', rules: [{ if: '$VAR == null', when: 'delayed', start_in: '3 hours' }] } } - it { is_expected.to include(when: 'delayed', start_in: '3 hours') } + it { is_expected.to include(when: 'delayed', options: { start_in: '3 hours' }) } end context 'is not matched' do @@ -541,7 +541,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do it { is_expected.to be_included } it 'correctly populates when:' do - expect(seed_build.attributes).to include(when: 'delayed', start_in: '1 day') + expect(seed_build.attributes).to include(when: 'delayed', options: { start_in: '1 day' }) end end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index cf496b79a62..9d9a9ecda33 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -16,7 +16,10 @@ module Gitlab let(:config) do YAML.dump( before_script: ['pwd'], - rspec: { script: 'rspec' } + rspec: { + script: 'rspec', + interruptible: true + } ) end @@ -29,6 +32,7 @@ module Gitlab before_script: ["pwd"], script: ["rspec"] }, + interruptible: true, allow_failure: false, when: "on_success", yaml_variables: [] @@ -36,6 +40,36 @@ module Gitlab end end + context 'with job rules' do + let(:config) do + YAML.dump( + rspec: { + script: 'rspec', + rules: [ + { if: '$CI_COMMIT_REF_NAME == "master"' }, + { changes: %w[README.md] } + ] + } + ) + end + + it 'returns valid build attributes' do + expect(subject).to eq({ + stage: 'test', + stage_idx: 1, + name: 'rspec', + options: { script: ['rspec'] }, + rules: [ + { if: '$CI_COMMIT_REF_NAME == "master"' }, + { changes: %w[README.md] } + ], + allow_failure: false, + when: 'on_success', + yaml_variables: [] + }) + end + end + describe 'coverage entry' do describe 'code coverage regexp' do let(:config) do @@ -1252,7 +1286,7 @@ module Gitlab end end - describe 'rules' do + context 'with when/rules conflict' do subject { Gitlab::Ci::YamlProcessor.new(YAML.dump(config)) } let(:config) do diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index d8880819d9f..fe86982af91 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -760,33 +760,32 @@ describe Ci::CreatePipelineService do end context 'when builds with auto-retries are configured' do + let(:pipeline) { execute_service } + let(:rspec_job) { pipeline.builds.find_by(name: 'rspec') } + + before do + stub_ci_pipeline_yaml_file(YAML.dump({ + rspec: { script: 'rspec', retry: retry_value } + })) + end + context 'as an integer' do - before do - config = YAML.dump(rspec: { script: 'rspec', retry: 2 }) - stub_ci_pipeline_yaml_file(config) - end + let(:retry_value) { 2 } it 'correctly creates builds with auto-retry value configured' do - pipeline = execute_service - expect(pipeline).to be_persisted - expect(pipeline.builds.find_by(name: 'rspec').retries_max).to eq 2 - expect(pipeline.builds.find_by(name: 'rspec').retry_when).to eq ['always'] + expect(rspec_job.retries_max).to eq 2 + expect(rspec_job.retry_when).to eq ['always'] end end context 'as hash' do - before do - config = YAML.dump(rspec: { script: 'rspec', retry: { max: 2, when: 'runner_system_failure' } }) - stub_ci_pipeline_yaml_file(config) - end + let(:retry_value) { { max: 2, when: 'runner_system_failure' } } it 'correctly creates builds with auto-retry value configured' do - pipeline = execute_service - expect(pipeline).to be_persisted - expect(pipeline.builds.find_by(name: 'rspec').retries_max).to eq 2 - expect(pipeline.builds.find_by(name: 'rspec').retry_when).to eq ['runner_system_failure'] + expect(rspec_job.retries_max).to eq 2 + expect(rspec_job.retry_when).to eq ['runner_system_failure'] end end end @@ -1174,7 +1173,7 @@ describe Ci::CreatePipelineService do expect(pipeline).to be_persisted expect(pipeline).to be_merge_request_event expect(pipeline.merge_request).to eq(merge_request) - expect(pipeline.builds.order(:stage_id).map(&:name)).to eq(%w[test]) + expect(pipeline.builds.order(:stage_id).pluck(:name)).to eq(%w[test]) end it 'persists the specified source sha' do @@ -1439,7 +1438,7 @@ describe Ci::CreatePipelineService do expect(pipeline).to be_persisted expect(pipeline).to be_web expect(pipeline.merge_request).to be_nil - expect(pipeline.builds.order(:stage_id).map(&:name)).to eq(%w[build pages]) + expect(pipeline.builds.order(:stage_id).pluck(:name)).to eq(%w[build pages]) end end end @@ -1479,7 +1478,7 @@ describe Ci::CreatePipelineService do it 'creates a pipeline with build_a and test_a' do expect(pipeline).to be_persisted - expect(pipeline.builds.map(&:name)).to contain_exactly("build_a", "test_a") + expect(pipeline.builds.pluck(:name)).to contain_exactly("build_a", "test_a") end end @@ -1514,7 +1513,303 @@ describe Ci::CreatePipelineService do it 'does create a pipeline only with deploy' do expect(pipeline).to be_persisted - expect(pipeline.builds.map(&:name)).to contain_exactly("deploy") + expect(pipeline.builds.pluck(:name)).to contain_exactly("deploy") + end + end + end + + context 'when rules are used' do + let(:ref_name) { 'refs/heads/master' } + let(:pipeline) { execute_service } + let(:build_names) { pipeline.builds.pluck(:name) } + let(:regular_job) { pipeline.builds.find_by(name: 'regular-job') } + let(:rules_job) { pipeline.builds.find_by(name: 'rules-job') } + let(:delayed_job) { pipeline.builds.find_by(name: 'delayed-job') } + + shared_examples 'rules jobs are excluded' do + it 'only persists the job without rules' do + expect(pipeline).to be_persisted + expect(regular_job).to be_persisted + expect(rules_job).to be_nil + expect(delayed_job).to be_nil + end + end + + before do + stub_ci_pipeline_yaml_file(config) + allow_any_instance_of(Ci::BuildScheduleWorker).to receive(:perform).and_return(true) + end + + context 'with simple if: clauses' do + let(:config) do + <<-EOY + regular-job: + script: 'echo Hello, World!' + + master-job: + script: "echo hello world, $CI_COMMIT_REF_NAME" + rules: + - if: $CI_COMMIT_REF_NAME == "nonexistant-branch" + when: never + - if: $CI_COMMIT_REF_NAME =~ /master/ + when: manual + + delayed-job: + script: "echo See you later, World!" + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + when: delayed + start_in: 1 hour + + never-job: + script: "echo Goodbye, World!" + rules: + - if: $CI_COMMIT_REF_NAME + when: never + EOY + end + + context 'with matches' do + it 'creates a pipeline with the vanilla and manual jobs' do + expect(pipeline).to be_persisted + expect(build_names).to contain_exactly('regular-job', 'delayed-job', 'master-job') + end + + it 'assigns job:when values to the builds' do + expect(pipeline.builds.pluck(:when)).to contain_exactly('on_success', 'delayed', 'manual') + end + + it 'assigns start_in for delayed jobs' do + expect(delayed_job.options[:start_in]).to eq('1 hour') + end + end + + context 'with no matches' do + let(:ref_name) { 'refs/heads/feature' } + + it_behaves_like 'rules jobs are excluded' + end + end + + context 'with complex if: clauses' do + let(:config) do + <<-EOY + regular-job: + script: 'echo Hello, World!' + rules: + - if: $VAR == 'present' && $OTHER || $CI_COMMIT_REF_NAME + when: manual + EOY + end + + it 'matches the first rule' do + expect(pipeline).to be_persisted + expect(build_names).to contain_exactly('regular-job') + expect(regular_job.when).to eq('manual') + end + end + + context 'with changes:' do + let(:config) do + <<-EOY + regular-job: + script: 'echo Hello, World!' + + rules-job: + script: "echo hello world, $CI_COMMIT_REF_NAME" + rules: + - changes: + - README.md + when: manual + - changes: + - app.rb + when: on_success + + delayed-job: + script: "echo See you later, World!" + rules: + - changes: + - README.md + when: delayed + start_in: 4 hours + EOY + end + + context 'and matches' do + before do + allow_any_instance_of(Ci::Pipeline) + .to receive(:modified_paths).and_return(%w[README.md]) + end + + it 'creates two jobs' do + expect(pipeline).to be_persisted + expect(build_names) + .to contain_exactly('regular-job', 'rules-job', 'delayed-job') + end + + it 'sets when: for all jobs' do + expect(regular_job.when).to eq('on_success') + expect(rules_job.when).to eq('manual') + expect(delayed_job.when).to eq('delayed') + expect(delayed_job.options[:start_in]).to eq('4 hours') + end + end + + context 'and matches the second rule' do + before do + allow_any_instance_of(Ci::Pipeline) + .to receive(:modified_paths).and_return(%w[app.rb]) + end + + it 'includes both jobs' do + expect(pipeline).to be_persisted + expect(build_names).to contain_exactly('regular-job', 'rules-job') + end + + it 'sets when: for the created rules job based on the second clause' do + expect(regular_job.when).to eq('on_success') + expect(rules_job.when).to eq('on_success') + end + end + + context 'and does not match' do + before do + allow_any_instance_of(Ci::Pipeline) + .to receive(:modified_paths).and_return(%w[useless_script.rb]) + end + + it_behaves_like 'rules jobs are excluded' + + it 'sets when: for the created job' do + expect(regular_job.when).to eq('on_success') + end + end + end + + context 'with mixed if: and changes: rules' do + let(:config) do + <<-EOY + regular-job: + script: 'echo Hello, World!' + + rules-job: + script: "echo hello world, $CI_COMMIT_REF_NAME" + rules: + - changes: + - README.md + when: manual + - if: $CI_COMMIT_REF_NAME == "master" + when: on_success + + delayed-job: + script: "echo See you later, World!" + rules: + - changes: + - README.md + when: delayed + start_in: 4 hours + - if: $CI_COMMIT_REF_NAME == "master" + when: delayed + start_in: 1 hour + EOY + end + + context 'and changes: matches before if' do + before do + allow_any_instance_of(Ci::Pipeline) + .to receive(:modified_paths).and_return(%w[README.md]) + end + + it 'creates two jobs' do + expect(pipeline).to be_persisted + expect(build_names) + .to contain_exactly('regular-job', 'rules-job', 'delayed-job') + end + + it 'sets when: for all jobs' do + expect(regular_job.when).to eq('on_success') + expect(rules_job.when).to eq('manual') + expect(delayed_job.when).to eq('delayed') + expect(delayed_job.options[:start_in]).to eq('4 hours') + end + end + + context 'and if: matches after changes' do + it 'includes both jobs' do + expect(pipeline).to be_persisted + expect(build_names).to contain_exactly('regular-job', 'rules-job', 'delayed-job') + end + + it 'sets when: for the created rules job based on the second clause' do + expect(regular_job.when).to eq('on_success') + expect(rules_job.when).to eq('on_success') + expect(delayed_job.when).to eq('delayed') + expect(delayed_job.options[:start_in]).to eq('1 hour') + end + end + + context 'and does not match' do + let(:ref_name) { 'refs/heads/wip' } + + it_behaves_like 'rules jobs are excluded' + + it 'sets when: for the created job' do + expect(regular_job.when).to eq('on_success') + end + end + end + + context 'with mixed if: and changes: clauses' do + let(:config) do + <<-EOY + regular-job: + script: 'echo Hello, World!' + + rules-job: + script: "echo hello world, $CI_COMMIT_REF_NAME" + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + changes: [README.md] + when: on_success + - if: $CI_COMMIT_REF_NAME =~ /master/ + changes: [app.rb] + when: manual + EOY + end + + context 'with if matches and changes matches' do + before do + allow_any_instance_of(Ci::Pipeline) + .to receive(:modified_paths).and_return(%w[app.rb]) + end + + it 'persists all jobs' do + expect(pipeline).to be_persisted + expect(regular_job).to be_persisted + expect(rules_job).to be_persisted + expect(rules_job.when).to eq('manual') + end + end + + context 'with if matches and no change matches' do + it_behaves_like 'rules jobs are excluded' + end + + context 'with change matches and no if matches' do + let(:ref_name) { 'refs/heads/feature' } + + before do + allow_any_instance_of(Ci::Pipeline) + .to receive(:modified_paths).and_return(%w[README.md]) + end + + it_behaves_like 'rules jobs are excluded' + end + + context 'and no matches' do + let(:ref_name) { 'refs/heads/feature' } + + it_behaves_like 'rules jobs are excluded' end end end |