Allow use of `when` and `rules`

* Remove validation on Entry::Processable

Changelog: added

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

@@ -26,7 +26,7 @@ module Gitlab
               validates :name, length: { maximum: 255 }, if: -> { ::Feature.enabled?(:ci_validate_job_length, default_enabled: :yaml) }
 
               validates :config, disallowed_keys: {
-                  in: %i[only except when start_in],
+                  in: %i[only except start_in],
                   message: 'key may not be used with `rules`'
                 },
                 if: :has_rules?

lib/gitlab/ci/pipeline/seed/build.rb

@@ -28,7 +28,7 @@ module Gitlab
             @except = Gitlab::Ci::Build::Policy
               .fabricate(attributes.delete(:except))
             @rules = Gitlab::Ci::Build::Rules
-              .new(attributes.delete(:rules), default_when: 'on_success')
+              .new(attributes.delete(:rules), default_when: attributes[:when])
             @cache = Gitlab::Ci::Build::Cache
               .new(attributes.delete(:cache), @pipeline)
 

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

@@ -163,7 +163,7 @@ RSpec.describe Gitlab::Ci::Config::Entry::Bridge do
         })
       end
 
-      it { is_expected.not_to be_valid }
+      it { is_expected.to be_valid }
     end
 
     context 'when bridge configuration uses rules with only' do

spec/lib/gitlab/ci/config/entry/job_spec.rb

@@ -118,6 +118,20 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do
         end
       end
 
+      context 'when config uses both "when:" and "rules:"' do
+        let(:config) do
+          {
+            script: 'echo',
+            when: 'on_failure',
+            rules: [{ if: '$VARIABLE', when: 'on_success' }]
+          }
+        end
+
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+
       context 'when delayed job' do
         context 'when start_in is specified' do
           let(:config) { { script: 'echo', when: 'delayed', start_in: '1 week' } }
@@ -268,21 +282,6 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do
         end
       end
 
-      context 'when it uses both "when:" and "rules:"' do
-        let(:config) do
-          {
-            script: 'echo',
-            when: 'on_failure',
-            rules: [{ if: '$VARIABLE', when: 'on_success' }]
-          }
-        end
-
-        it 'returns an error about when: being combined with rules' do
-          expect(entry).not_to be_valid
-          expect(entry.errors).to include 'job config key may not be used with `rules`: when'
-        end
-      end
-
       context 'when delayed job' do
         context 'when start_in is specified' do
           let(:config) { { script: 'echo', when: 'delayed', start_in: '1 week' } }

spec/lib/gitlab/ci/config/entry/processable_spec.rb

@@ -33,6 +33,20 @@ RSpec.describe Gitlab::Ci::Config::Entry::Processable do
         end
       end
 
+      context 'when config uses both "when:" and "rules:"' do
+        let(:config) do
+          {
+            script: 'echo',
+            when: 'on_failure',
+            rules: [{ if: '$VARIABLE', when: 'on_success' }]
+          }
+        end
+
+        it 'is valid' do
+          expect(entry).to be_valid
+        end
+      end
+
       context 'when job name is more than 255' do
         let(:entry) { node_class.new(config, name: ('a' * 256).to_sym) }
 
@@ -90,21 +104,6 @@ RSpec.describe Gitlab::Ci::Config::Entry::Processable do
         end
       end
 
-      context 'when it uses both "when:" and "rules:"' do
-        let(:config) do
-          {
-            script: 'echo',
-            when: 'on_failure',
-            rules: [{ if: '$VARIABLE', when: 'on_success' }]
-          }
-        end
-
-        it 'returns an error about when: being combined with rules' do
-          expect(entry).not_to be_valid
-          expect(entry.errors).to include 'job config key may not be used with `rules`: when'
-        end
-      end
-
       context 'when only: is used with rules:' do
         let(:config) { { only: ['merge_requests'], rules: [{ if: '$THIS' }] } }
 

spec/lib/gitlab/ci/pipeline/seed/build_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
   let(:pipeline) { build(:ci_empty_pipeline, project: project, sha: head_sha) }
   let(:root_variables) { [] }
   let(:seed_context) { double(pipeline: pipeline, root_variables: root_variables) }
-  let(:attributes) { { name: 'rspec', ref: 'master', scheduling_type: :stage } }
+  let(:attributes) { { name: 'rspec', ref: 'master', scheduling_type: :stage, when: 'on_success' } }
   let(:previous_stages) { [] }
   let(:current_stage) { double(seeds_names: [attributes[:name]]) }
 
@@ -61,17 +61,35 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
       end
     end
 
-    context 'with job:rules but no explicit when:' do
-      context 'is matched' do
-        let(:attributes) { { name: 'rspec', ref: 'master', rules: [{ if: '$VAR == null' }] } }
+    context 'with job: rules but no explicit when:' do
+      let(:base_attributes) { { name: 'rspec', ref: 'master' } }
+
+      context 'with a manual job' do
+        context 'with a matched rule' do
+          let(:attributes) { base_attributes.merge(when: 'manual', rules: [{ if: '$VAR == null' }]) }
+
+          it { is_expected.to include(when: 'manual') }
+        end
+
+        context 'is not matched' do
+          let(:attributes) { base_attributes.merge(when: 'manual', rules: [{ if: '$VAR != null' }]) }
 
-        it { is_expected.to include(when: 'on_success') }
+          it { is_expected.to include(when: 'never') }
+        end
       end
 
-      context 'is not matched' do
-        let(:attributes) { { name: 'rspec', ref: 'master', rules: [{ if: '$VAR != null' }] } }
+      context 'with an automatic job' do
+        context 'is matched' do
+          let(:attributes) { base_attributes.merge(when: 'on_success', rules: [{ if: '$VAR == null' }]) }
 
-        it { is_expected.to include(when: 'never') }
+          it { is_expected.to include(when: 'on_success') }
+        end
+
+        context 'is not matched' do
+          let(:attributes) { base_attributes.merge(when: 'on_success', rules: [{ if: '$VAR != null' }]) }
+
+          it { is_expected.to include(when: 'never') }
+        end
       end
     end
 
@@ -901,7 +919,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
     context 'using rules:' do
       using RSpec::Parameterized
 
-      let(:attributes) { { name: 'rspec', rules: rule_set } }
+      let(:attributes) { { name: 'rspec', rules: rule_set, when: 'on_success' } }
 
       context 'with a matching if: rule' do
         context 'with an explicit `when: never`' do

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -2139,7 +2139,7 @@ module Gitlab
         end
       end
 
-      context 'with when/rules conflict' do
+      context 'with when/rules' do
         subject { Gitlab::Ci::YamlProcessor.new(YAML.dump(config)).execute }
 
         let(:config) do
@@ -2174,7 +2174,7 @@ module Gitlab
             }
           end
 
-          it_behaves_like 'returns errors', /may not be used with `rules`: when/
+          it { is_expected.to be_valid }
         end
 
         context 'used with job-level when:delayed' do
@@ -2190,7 +2190,7 @@ module Gitlab
             }
           end
 
-          it_behaves_like 'returns errors', /may not be used with `rules`: when, start_in/
+          it_behaves_like 'returns errors', /may not be used with `rules`: start_in/
         end
       end
 

spec/services/ci/create_pipeline_service_spec.rb

@@ -1992,6 +1992,75 @@ RSpec.describe Ci::CreatePipelineService do
       let(:rules_job)   { find_job('rules-job') }
       let(:delayed_job) { find_job('delayed-job') }
 
+      context 'with when:manual' do
+        let(:config) do
+          <<-EOY
+          job-with-rules:
+            script: 'echo hey'
+            rules:
+              - if: $CI_COMMIT_REF_NAME =~ /master/
+
+          job-when-with-rules:
+            script: 'echo hey'
+            when: manual
+            rules:
+              - if: $CI_COMMIT_REF_NAME =~ /master/
+
+          job-when-with-rules-when:
+            script: 'echo hey'
+            when: manual
+            rules:
+              - if: $CI_COMMIT_REF_NAME =~ /master/
+                when: on_success
+
+          job-with-rules-when:
+            script: 'echo hey'
+            rules:
+              - if: $CI_COMMIT_REF_NAME =~ /master/
+                when: manual
+
+          job-without-rules:
+            script: 'echo this is a job with NO rules'
+          EOY
+        end
+
+        let(:job_with_rules) { find_job('job-with-rules') }
+        let(:job_when_with_rules) { find_job('job-when-with-rules') }
+        let(:job_when_with_rules_when) { find_job('job-when-with-rules-when') }
+        let(:job_with_rules_when) { find_job('job-with-rules-when') }
+        let(:job_without_rules) { find_job('job-without-rules') }
+
+        context 'when matching the rules' do
+          let(:ref_name) { 'refs/heads/master' }
+
+          it 'adds the job-with-rules with a when:manual' do
+            expect(job_with_rules).to be_persisted
+            expect(job_when_with_rules).to be_persisted
+            expect(job_when_with_rules_when).to be_persisted
+            expect(job_with_rules_when).to be_persisted
+            expect(job_without_rules).to be_persisted
+
+            expect(job_with_rules.when).to eq('on_success')
+            expect(job_when_with_rules.when).to eq('manual')
+            expect(job_when_with_rules_when.when).to eq('on_success')
+            expect(job_with_rules_when.when).to eq('manual')
+            expect(job_without_rules.when).to eq('on_success')
+          end
+        end
+
+        context 'when there is no match to the rule' do
+          let(:ref_name) { 'refs/heads/wip' }
+
+          it 'does not add job_with_rules' do
+            expect(job_with_rules).to be_nil
+            expect(job_when_with_rules).to be_nil
+            expect(job_when_with_rules_when).to be_nil
+            expect(job_with_rules_when).to be_nil
+            expect(job_without_rules).to be_persisted
+          end
+        end
+      end
+
       shared_examples 'rules jobs are excluded' do
         it 'only persists the job without rules' do
           expect(pipeline).to be_persisted