Passing job:rules downstream and E2E specs for job:rules configuration

changelogs/unreleased/job-rules-e2e.yml

+---
+title: Passing job rules downstream and E2E specs for job:rules configuration
+merge_request: 32609
+author:
+type: fixed

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)

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)

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,

lib/gitlab/ci/config/entry/rules.rb

@@ -26,6 +26,10 @@ module Gitlab
               end
             end
           end
+
+          def value
+            @config
+          end
         end
       end
     end

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

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],

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

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

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
-      context 'as an integer' do
+      let(:pipeline)  { execute_service }
+      let(:rspec_job) { pipeline.builds.find_by(name: 'rspec') }
+
       before do
-          config = YAML.dump(rspec: { script: 'rspec', retry: 2 })
-          stub_ci_pipeline_yaml_file(config)
+        stub_ci_pipeline_yaml_file(YAML.dump({
+          rspec: { script: 'rspec', retry: retry_value }
+        }))
       end
 
-        it 'correctly creates builds with auto-retry value configured' do
-          pipeline = execute_service
+      context 'as an integer' do
+        let(:retry_value) { 2 }
 
+        it 'correctly creates builds with auto-retry value configured' do
           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