Merge branch 'bridge-rules' into 'master'

rules: for bridge jobs

See merge request gitlab-org/gitlab!19605

changelogs/unreleased/bridge-rules.yml

+---
+title: Added rules configuration for Ci::Bridge
+merge_request: 19605
+author:
+type: added

ee/lib/ee/gitlab/ci/config/entry/bridge.rb

@@ -15,19 +15,25 @@ module EE
             include ::Gitlab::Config::Entry::Inheritable
 
             ALLOWED_KEYS = %i[trigger stage allow_failure only except
-                              when extends variables needs].freeze
+                              when extends variables needs rules].freeze
 
             validations do
               validates :config, allowed_keys: ALLOWED_KEYS
               validates :config, presence: true
               validates :name, presence: true
               validates :name, type: Symbol
+              validates :config, disallowed_keys: {
+                  in: %i[only except when start_in],
+                  message: 'key may not be used with `rules`'
+                },
+                if: :has_rules?
 
               with_options allow_nil: true do
                 validates :when,
                   inclusion: { in: %w[on_success on_failure always],
                                message: 'should be on_success, on_failure or always' }
                 validates :extends, type: String
+                validates :rules, array_of_hashes: true
               end
 
               validate on: :composed do
@@ -66,6 +72,13 @@ module EE
               description: 'Refs policy this job will be executed for.',
               inherit: false
 
+            entry :rules, ::Gitlab::Ci::Config::Entry::Rules,
+              description: 'List of evaluable Rules to determine job inclusion.',
+              inherit: false,
+              metadata: {
+                allowed_when: %w[on_success on_failure always never manual delayed].freeze
+              }
+
             entry :variables, ::Gitlab::Ci::Config::Entry::Variables,
               description: 'Environment variables available for this job.',
               inherit: false
@@ -84,6 +97,21 @@ module EE
               true
             end
 
+            def compose!(deps = nil)
+              super do
+                # This is something of a hack, see issue for details:
+                # https://gitlab.com/gitlab-org/gitlab/issues/31685
+                if !only_defined? && has_rules?
+                  @entries.delete(:only)
+                  @entries.delete(:except)
+                end
+              end
+            end
+
+            def has_rules?
+              @config&.key?(:rules)
+            end
+
             def name
               @metadata[:name]
             end
@@ -97,6 +125,7 @@ module EE
                 when: when_value,
                 extends: extends_value,
                 variables: (variables_value if variables_defined?),
+                rules: (rules_value if has_rules?),
                 only: only_value,
                 except: except_value }.compact
             end

ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb

@@ -34,6 +34,17 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
       end
 
       it { is_expected.to be_falsey }
+
+      context 'with rules' do
+        let(:config) do
+          {
+            script: 'ls -al',
+            rules: [{ if: '$VAR == "value"', when: 'always' }]
+          }
+        end
+
+        it { is_expected.to be_falsey }
+      end
     end
 
     context 'when config is a bridge job' do
@@ -43,6 +54,17 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
       end
 
       it { is_expected.to be_truthy }
+
+      context 'with rules' do
+        let(:config) do
+          {
+            trigger: 'other-project',
+            rules: [{ if: '$VAR == "value"', when: 'always' }]
+          }
+        end
+
+        it { is_expected.to be_truthy }
+      end
     end
 
     context 'when config is a hidden job' do
@@ -60,6 +82,16 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
       subject.compose!
     end
 
+    let(:base_config) do
+      {
+        trigger: { project: 'some/project', branch: 'feature' },
+        needs: { pipeline: 'other/project' },
+        extends: '.some-key',
+        stage: 'deploy',
+        variables: { VARIABLE: '123' }
+      }
+    end
+
     context 'when trigger config is a non-empty string' do
       let(:config) { { trigger: 'some/project' } }
 
@@ -117,21 +149,57 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
       end
     end
 
-    context 'when bridge configuration contains all supported keys' do
+    context 'when bridge configuration contains trigger, needs, when, extends, stage, only, except, and variables' do
       let(:config) do
-        { trigger: { project: 'some/project', branch: 'feature' },
-          needs: { pipeline: 'other/project' },
+        base_config.merge({
           when: 'always',
-          extends: '.some-key',
-          stage: 'deploy',
           only: { variables: %w[$SOMEVARIABLE] },
-          except: { refs: %w[feature] },
-          variables: { VARIABLE: '123' } }
+          except: { refs: %w[feature] }
+        })
       end
 
       it { is_expected.to be_valid }
     end
 
+    context 'when bridge configuration uses rules' do
+      let(:config) { base_config.merge({ rules: [{ if: '$VAR == null', when: 'never' }] }) }
+
+      it { is_expected.to be_valid }
+    end
+
+    context 'when bridge configuration uses rules with job:when' do
+      let(:config) do
+        base_config.merge({
+          when: 'always',
+          rules: [{ if: '$VAR == null', when: 'never' }]
+        })
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'when bridge configuration uses rules with only' do
+      let(:config) do
+        base_config.merge({
+          only: { variables: %w[$SOMEVARIABLE] },
+          rules: [{ if: '$VAR == null', when: 'never' }]
+        })
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'when bridge configuration uses rules with except' do
+      let(:config) do
+        base_config.merge({
+          except: { refs: %w[feature] },
+          rules: [{ if: '$VAR == null', when: 'never' }]
+        })
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+
     context 'when trigger config is nil' do
       let(:config) { { trigger: nil } }
 

ee/spec/services/ci/create_cross_project_pipeline_service_spec.rb

@@ -184,5 +184,44 @@ describe Ci::CreateCrossProjectPipelineService, '#execute' do
         end
       end
     end
+
+    # TODO: Move this context into a feature spec that uses
+    # multiple pipeline processing services. Location TBD in:
+    # https://gitlab.com/gitlab-org/gitlab/issues/36216
+    context 'when configured with bridge job rules' do
+      before do
+        stub_ci_pipeline_yaml_file(config)
+        downstream_project.add_maintainer(upstream_project.owner)
+      end
+
+      let(:config) do
+        <<-EOY
+          hello:
+            script: echo world
+
+          bridge-job:
+            rules:
+              - if: $CI_COMMIT_REF_NAME == "master"
+            trigger:
+              project: #{downstream_project.full_path}
+              branch: master
+        EOY
+      end
+
+      let(:primary_pipeline) do
+        Ci::CreatePipelineService.new(upstream_project, upstream_project.owner, { ref: 'master' })
+          .execute(:push, save_on_errors: false)
+      end
+
+      let(:bridge)  { primary_pipeline.processables.find_by(name: 'bridge-job') }
+      let(:service) { described_class.new(upstream_project, upstream_project.owner) }
+
+      context 'that include the bridge job' do
+        it 'creates the downstream pipeline' do
+          expect { service.execute(bridge) }
+            .to change(downstream_project.ci_pipelines, :count).by(1)
+        end
+      end
+    end
   end
 end

ee/spec/services/ci/create_pipeline_service_spec.rb

@@ -8,9 +8,10 @@ describe Ci::CreatePipelineService, '#execute' do
   set(:plan_limits) { create(:plan_limits, plan: gold_plan) }
   set(:project) { create(:project, :repository, namespace: namespace) }
   set(:user) { create(:user) }
+  let(:ref_name) { 'master' }
 
   let(:service) do
-    params = { ref: 'master',
+    params = { ref: ref_name,
                before: '00000000',
                after: project.commit.id,
                commits: [{ message: 'some commit' }] }
@@ -101,6 +102,46 @@ describe Ci::CreatePipelineService, '#execute' do
       expect(bridge.yaml_variables)
         .to include(key: 'CROSS', value: 'downstream', public: true)
     end
+
+    context 'when configured with rules' do
+      before do
+        stub_ci_pipeline_yaml_file(config)
+      end
+
+      let(:downstream_project) { create(:project, :repository) }
+
+      let(:config) do
+        <<-EOY
+          hello:
+            script: echo world
+
+          bridge-job:
+            rules:
+              - if: $CI_COMMIT_REF_NAME == "master"
+            trigger:
+              project: #{downstream_project.full_path}
+              branch: master
+        EOY
+      end
+
+      context 'that include the bridge job' do
+        it 'persists the bridge job' do
+          pipeline = create_pipeline!
+
+          expect(pipeline.processables.pluck(:name)).to contain_exactly('hello', 'bridge-job')
+        end
+      end
+
+      context 'that exclude the bridge job' do
+        let(:ref_name) { 'refs/heads/wip' }
+
+        it 'does not include the bridge job' do
+          pipeline = create_pipeline!
+
+          expect(pipeline.processables.pluck(:name)).to eq(%w[hello])
+        end
+      end
+    end
   end
 
   def create_pipeline!

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

@@ -173,7 +173,7 @@ module Gitlab
               @entries.delete(:type)
 
               # This is something of a hack, see issue for details:
-              # https://gitlab.com/gitlab-org/gitlab-foss/issues/67150
+              # https://gitlab.com/gitlab-org/gitlab/issues/31685
               if !only_defined? && has_rules?
                 @entries.delete(:only)
                 @entries.delete(:except)