Merge branch 'job-rules-e2e' into 'master'

Passing job:rules downstream and E2E specs for job:rules configuration See merge request gitlab-org/gitlab-ce!32609

Merge branch 'job-rules-e2e' into 'master'
Passing job:rules downstream and E2E specs for job:rules configuration See merge request gitlab-org/gitlab-ce!32609
a71789b7 · Kamil Trzciński · ee5bd244 · 9458192f · a71789b7 · a71789b7
--- a/changelogs/unreleased/job-rules-e2e.yml
+++ b/changelogs/unreleased/job-rules-e2e.yml
+---
+title: Passing job rules downstream and E2E specs for job:rules configuration
+merge_request: 32609
+author:
+type: fixed
--- a/lib/gitlab/ci/build/rules.rb
+++ b/lib/gitlab/ci/build/rules.rb
@@ -6,7 +6,14 @@ module Build
      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)

--- a/lib/gitlab/ci/build/rules/rule/clause.rb
+++ b/lib/gitlab/ci/build/rules/rule/clause.rb
@@ -13,9 +13,7 @@ class Rules::Rule::Clause
        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)

--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -122,7 +122,7 @@ class Job < ::Gitlab::Config::Entry::Node

          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 @@ def compose!(deps = nil)
              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 @@ def to_hash
              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,

--- a/lib/gitlab/ci/config/entry/rules.rb
+++ b/lib/gitlab/ci/config/entry/rules.rb
@@ -26,6 +26,10 @@ def compose!(deps = nil)
              end
            end
          end
+
+          def value
+            @config
+          end
        end
      end
    end

--- a/lib/gitlab/ci/pipeline/seed/build.rb
+++ b/lib/gitlab/ci/pipeline/seed/build.rb
@@ -145,7 +145,7 @@ def included_by_rules?

          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

--- a/lib/gitlab/ci/yaml_processor.rb
+++ b/lib/gitlab/ci/yaml_processor.rb
@@ -42,6 +42,7 @@ def build_attributes(name)
          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],

--- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
@@ -46,7 +46,7 @@
      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 @@
            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

--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -16,7 +16,10 @@ module Ci
          let(:config) do
            YAML.dump(
              before_script: ['pwd'],
-              rspec: { script: 'rspec' }
+              rspec: {
+                script: 'rspec',
+                interruptible: true
+              }
            )
          end

@@ -29,6 +32,7 @@ module Ci
                before_script: ["pwd"],
                script: ["rspec"]
              },
+              interruptible: true,
              allow_failure: false,
              when: "on_success",
              yaml_variables: []
@@ -36,6 +40,36 @@ module Ci
          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 Ci
        end
      end

-      describe 'rules' do
+      context 'with when/rules conflict' do
        subject { Gitlab::Ci::YamlProcessor.new(YAML.dump(config)) }

        let(:config) do

--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -760,33 +760,32 @@ def previous_commit_sha_from_ref(ref)
    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 @@ def previous_commit_sha_from_ref(ref)
              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 @@ def previous_commit_sha_from_ref(ref)
              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 @@ def previous_commit_sha_from_ref(ref)

        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 @@ def previous_commit_sha_from_ref(ref)

        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