Introducing new Syntax for Ci::Build inclusion rules

- Added Gitlab::Ci::Config::Entry::Rules and Gitlab::Ci::Config::Entry::Rules:Rule
  to handle lists of Rule objects to be evalauted for job inclusion
- Added `if:` and `changes:` as available Rules::Rule::Clause classes
- Added Rules handling logic to Seed::Build#included? with extra specs
- Use DisallowedKeysValidator to mutually exclude rules: from only:/except: on job config

changelogs/unreleased/ci-config-on-policy.yml

+---
+title: Introduced Build::Rules configuration for Ci::Build
+merge_request: 29011
+author:
+type: added

lib/gitlab/ci/build/policy/variables.rb

@@ -10,7 +10,7 @@ module Gitlab
           end
 
           def satisfied_by?(pipeline, seed)
-            variables = seed.to_resource.scoped_variables_hash
+            variables = seed.scoped_variables_hash
 
             statements = @expressions.map do |statement|
               ::Gitlab::Ci::Pipeline::Expression::Statement

lib/gitlab/ci/build/rules.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Build
+      class Rules
+        include ::Gitlab::Utils::StrongMemoize
+
+        Result = Struct.new(:when, :start_in)
+
+        def initialize(rule_hashes, default_when = 'on_success')
+          @rule_list    = Rule.fabricate_list(rule_hashes)
+          @default_when = default_when
+        end
+
+        def evaluate(pipeline, build)
+          if @rule_list.nil?
+            Result.new(@default_when)
+          elsif matched_rule = match_rule(pipeline, build)
+            Result.new(
+              matched_rule.attributes[:when] || @default_when,
+              matched_rule.attributes[:start_in]
+            )
+          else
+            Result.new('never')
+          end
+        end
+
+        private
+
+        def match_rule(pipeline, build)
+          @rule_list.find { |rule| rule.matches?(pipeline, build) }
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/build/rules/rule.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Build
+      class Rules::Rule
+        attr_accessor :attributes
+
+        def self.fabricate_list(list)
+          list.map(&method(:new)) if list
+        end
+
+        def initialize(spec)
+          @clauses    = []
+          @attributes = {}
+
+          spec.each do |type, value|
+            if clause = Clause.fabricate(type, value)
+              @clauses << clause
+            else
+              @attributes.merge!(type => value)
+            end
+          end
+        end
+
+        def matches?(pipeline, build)
+          @clauses.all? { |clause| clause.satisfied_by?(pipeline, build) }
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/build/rules/rule/clause.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Build
+      class Rules::Rule::Clause
+        ##
+        # Abstract class that defines an interface of a single
+        # job rule specification.
+        #
+        # Used for job's inclusion rules configuration.
+        #
+        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)
+        end
+
+        def initialize(spec)
+          @spec = spec
+        end
+
+        def satisfied_by?(pipeline, seed = nil)
+          raise NotImplementedError
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/build/rules/rule/clause/changes.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Build
+      class Rules::Rule::Clause::Changes < Rules::Rule::Clause
+        def initialize(globs)
+          @globs = Array(globs)
+        end
+
+        def satisfied_by?(pipeline, seed)
+          return true if pipeline.modified_paths.nil?
+
+          pipeline.modified_paths.any? do |path|
+            @globs.any? do |glob|
+              File.fnmatch?(glob, path, File::FNM_PATHNAME | File::FNM_DOTMATCH | File::FNM_EXTGLOB)
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/build/rules/rule/clause/if.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Build
+      class Rules::Rule::Clause::If < Rules::Rule::Clause
+        def initialize(expression)
+          @expression = expression
+        end
+
+        def satisfied_by?(pipeline, seed)
+          variables = seed.scoped_variables_hash
+
+          ::Gitlab::Ci::Pipeline::Expression::Statement.new(@expression, variables).truthful?
+        end
+      end
+    end
+  end
+end

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

@@ -11,7 +11,8 @@ module Gitlab
           include ::Gitlab::Config::Entry::Configurable
           include ::Gitlab::Config::Entry::Attributable
 
-          ALLOWED_KEYS = %i[tags script only except type image services
+          ALLOWED_WHEN = %w[on_success on_failure always manual delayed].freeze
+          ALLOWED_KEYS = %i[tags script only except rules type image services
                             allow_failure type stage when start_in artifacts cache
                             dependencies needs before_script after_script variables
                             environment coverage retry parallel extends].freeze
@@ -19,12 +20,19 @@ module Gitlab
           REQUIRED_BY_NEEDS = %i[stage].freeze
 
           validations do
+            validates :config, type: Hash
             validates :config, allowed_keys: ALLOWED_KEYS
             validates :config, required_keys: REQUIRED_BY_NEEDS, if: :has_needs?
             validates :config, presence: true
             validates :script, 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 :tags, array_of_strings: true
@@ -32,17 +40,19 @@ module Gitlab
               validates :parallel, numericality: { only_integer: true,
                                                    greater_than_or_equal_to: 2,
                                                    less_than_or_equal_to: 50 }
-              validates :when,
-                inclusion: { in: %w[on_success on_failure always manual delayed],
-                             message: 'should be on_success, on_failure, ' \
-                                      'always, manual or delayed' }
+              validates :when, inclusion: {
+                in: ALLOWED_WHEN,
+                message: "should be one of: #{ALLOWED_WHEN.join(', ')}"
+              }
+
               validates :dependencies, array_of_strings: true
               validates :needs, array_of_strings: true
               validates :extends, array_of_strings_or_string: true
+              validates :rules, array_of_hashes: true
             end
 
             validates :start_in, duration: { limit: '1 day' }, if: :delayed?
-            validates :start_in, absence: true, unless: :delayed?
+            validates :start_in, absence: true, if: -> { has_rules? || !delayed? }
 
             validate do
               next unless dependencies.present?
@@ -91,6 +101,9 @@ module Gitlab
           entry :except, Entry::Policy,
             description: 'Refs policy this job will be executed for.'
 
+          entry :rules, Entry::Rules,
+            description: 'List of evaluable Rules to determine job inclusion.'
+
           entry :variables, Entry::Variables,
             description: 'Environment variables available for this job.'
 
@@ -112,7 +125,7 @@ module Gitlab
                   :parallel, :needs
 
           attributes :script, :tags, :allow_failure, :when, :dependencies,
-                     :needs, :retry, :parallel, :extends, :start_in
+                     :needs, :retry, :parallel, :extends, :start_in, :rules
 
           def self.matching?(name, config)
             !name.to_s.start_with?('.') &&
@@ -151,6 +164,10 @@ module Gitlab
             self.when == 'delayed'
           end
 
+          def has_rules?
+            @config.try(:key?, :rules)
+          end
+
           def ignored?
             allow_failure.nil? ? manual_action? : allow_failure
           end

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

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        class Rules < ::Gitlab::Config::Entry::Node
+          include ::Gitlab::Config::Entry::Validatable
+
+          validations do
+            validates :config, presence: true
+            validates :config, type: Array
+          end
+
+          def compose!(deps = nil)
+            super(deps) do
+              @config.each_with_index do |rule, index|
+                @entries[index] = ::Gitlab::Config::Entry::Factory.new(Entry::Rules::Rule)
+                  .value(rule)
+                  .with(key: "rule", parent: self, description: "rule definition.") # rubocop:disable CodeReuse/ActiveRecord
+                  .create!
+              end
+
+              @entries.each_value do |entry|
+                entry.compose!(deps)
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

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

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        class Rules::Rule < ::Gitlab::Config::Entry::Node
+          include ::Gitlab::Config::Entry::Validatable
+          include ::Gitlab::Config::Entry::Attributable
+
+          CLAUSES      = %i[if changes].freeze
+          ALLOWED_KEYS = %i[if changes when start_in].freeze
+          ALLOWED_WHEN = %w[on_success on_failure always never manual delayed].freeze
+
+          attributes :if, :changes, :when, :start_in
+
+          validations do
+            validates :config, presence: true
+            validates :config, type: { with: Hash }
+            validates :config, allowed_keys: ALLOWED_KEYS
+            validates :config, disallowed_keys: %i[start_in], unless: :specifies_delay?
+            validates :start_in, presence: true, if: :specifies_delay?
+            validates :start_in, duration: { limit: '1 day' }, if: :specifies_delay?
+
+            with_options allow_nil: true do
+              validates :if, expression: true
+              validates :changes, array_of_strings: true
+              validates :when, allowed_values: { in: ALLOWED_WHEN }
+            end
+          end
+
+          def specifies_delay?
+            self.when == 'delayed'
+          end
+
+          def default
+          end
+        end
+      end
+    end
+  end
+end

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

@@ -7,7 +7,7 @@ module Gitlab
         class Build < Seed::Base
           include Gitlab::Utils::StrongMemoize
 
-          delegate :dig, to: :@attributes
+          delegate :dig, to: :@seed_attributes
 
           # When the `ci_dag_limit_needs` is enabled it uses the lower limit
           LOW_NEEDS_LIMIT = 5
@@ -15,14 +15,20 @@ module Gitlab
 
           def initialize(pipeline, attributes, previous_stages)
             @pipeline = pipeline
-            @attributes = attributes
+            @seed_attributes = attributes
             @previous_stages = previous_stages
             @needs_attributes = dig(:needs_attributes)
 
+            @using_rules  = attributes.key?(:rules)
+            @using_only   = attributes.key?(:only)
+            @using_except = attributes.key?(:except)
+
             @only = Gitlab::Ci::Build::Policy
               .fabricate(attributes.delete(:only))
             @except = Gitlab::Ci::Build::Policy
               .fabricate(attributes.delete(:except))
+            @rules = Gitlab::Ci::Build::Rules
+              .new(attributes.delete(:rules))
           end
 
           def name
@@ -31,8 +37,13 @@ module Gitlab
 
           def included?
             strong_memoize(:inclusion) do
-              all_of_only? &&
-                none_of_except?
+              if @using_rules
+                included_by_rules?
+              elsif @using_only || @using_except
+                all_of_only? && none_of_except?
+              else
+                true
+              end
             end
           end
 
@@ -45,19 +56,13 @@ module Gitlab
           end
 
           def attributes
-            @attributes.merge(
-              pipeline: @pipeline,
-              project: @pipeline.project,
-              user: @pipeline.user,
-              ref: @pipeline.ref,
-              tag: @pipeline.tag,
-              trigger_request: @pipeline.legacy_trigger,
-              protected: @pipeline.protected_ref?
-            )
+            @seed_attributes
+              .deep_merge(pipeline_attributes)
+              .deep_merge(rules_attributes)
           end
 
           def bridge?
-            attributes_hash = @attributes.to_h
+            attributes_hash = @seed_attributes.to_h
             attributes_hash.dig(:options, :trigger).present? ||
               (attributes_hash.dig(:options, :bridge_needs).instance_of?(Hash) &&
                attributes_hash.dig(:options, :bridge_needs, :pipeline).present?)
@@ -73,6 +78,18 @@ module Gitlab
             end
           end
 
+          def scoped_variables_hash
+            strong_memoize(:scoped_variables_hash) do
+              # This is a temporary piece of technical debt to allow us access
+              # to the CI variables to evaluate rules before we persist a Build
+              # with the result. We should refactor away the extra Build.new,
+              # but be able to get CI Variables directly from the Seed::Build.
+              ::Ci::Build.new(
+                @seed_attributes.merge(pipeline_attributes)
+              ).scoped_variables_hash
+            end
+          end
+
           private
 
           def all_of_only?
@@ -109,6 +126,28 @@ module Gitlab
               HARD_NEEDS_LIMIT
             end
           end
+
+          def pipeline_attributes
+            {
+              pipeline: @pipeline,
+              project: @pipeline.project,
+              user: @pipeline.user,
+              ref: @pipeline.ref,
+              tag: @pipeline.tag,
+              trigger_request: @pipeline.legacy_trigger,
+              protected: @pipeline.protected_ref?
+            }
+          end
+
+          def included_by_rules?
+            rules_attributes[:when] != 'never'
+          end
+
+          def rules_attributes
+            strong_memoize(:rules_attributes) do
+              @using_rules ? @rules.evaluate(@pipeline, self).to_h.compact : {}
+            end
+          end
         end
       end
     end

lib/gitlab/config/entry/validators.rb

@@ -20,8 +20,10 @@ module Gitlab
             present_keys = value.try(:keys).to_a & options[:in]
 
             if present_keys.any?
-              record.errors.add(attribute, "contains disallowed keys: " +
-                present_keys.join(', '))
+              message = options[:message] || "contains disallowed keys"
+              message += ": #{present_keys.join(', ')}"
+
+              record.errors.add(attribute, message)
             end
           end
         end
@@ -65,6 +67,16 @@ module Gitlab
           end
         end
 
+        class ArrayOfHashesValidator < ActiveModel::EachValidator
+          include LegacyValidationHelpers
+
+          def validate_each(record, attribute, value)
+            unless value.is_a?(Array) && value.map { |hsh| hsh.is_a?(Hash) }.all?
+              record.errors.add(attribute, 'should be an array of hashes')
+            end
+          end
+        end
+
         class ArrayOrStringValidator < ActiveModel::EachValidator
           def validate_each(record, attribute, value)
             unless value.is_a?(Array) || value.is_a?(String)
@@ -231,6 +243,14 @@ module Gitlab
           end
         end
 
+        class ExpressionValidator < ActiveModel::EachValidator
+          def validate_each(record, attribute, value)
+            unless value.is_a?(String) && ::Gitlab::Ci::Pipeline::Expression::Statement.new(value).valid?
+              record.errors.add(attribute, 'Invalid expression syntax')
+            end
+          end
+        end
+
         class PortNamePresentAndUniqueValidator < ActiveModel::EachValidator
           def validate_each(record, attribute, value)
             return unless value.is_a?(Array)

spec/lib/gitlab/ci/build/policy/variables_spec.rb

@@ -13,7 +13,12 @@ describe Gitlab::Ci::Build::Policy::Variables do
     build(:ci_build, pipeline: pipeline, project: project, ref: 'master')
   end
 
-  let(:seed) { double('build seed', to_resource: ci_build) }
+  let(:seed) do
+    double('build seed',
+      to_resource: ci_build,
+      scoped_variables_hash: ci_build.scoped_variables_hash
+    )
+  end
 
   before do
     pipeline.variables.build(key: 'CI_PROJECT_NAME', value: '')
@@ -83,7 +88,12 @@ describe Gitlab::Ci::Build::Policy::Variables do
         build(:ci_bridge, pipeline: pipeline, project: project, ref: 'master')
       end
 
-      let(:seed) { double('bridge seed', to_resource: bridge) }
+      let(:seed) do
+        double('bridge seed',
+          to_resource: bridge,
+          scoped_variables_hash: ci_build.scoped_variables_hash
+        )
+      end
 
       it 'is satisfied by a matching expression for a bridge job' do
         policy = described_class.new(['$MY_VARIABLE'])

spec/lib/gitlab/ci/build/rules/rule_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Build::Rules::Rule do
+  let(:seed) do
+    double('build seed',
+      to_resource: ci_build,
+      scoped_variables_hash: ci_build.scoped_variables_hash
+    )
+  end
+
+  let(:pipeline) { create(:ci_pipeline) }
+  let(:ci_build) { build(:ci_build, pipeline: pipeline) }
+  let(:rule)     { described_class.new(rule_hash) }
+
+  describe '#matches?' do
+    subject { rule.matches?(pipeline, seed) }
+
+    context 'with one matching clause' do
+      let(:rule_hash) do
+        { if: '$VAR == null', when: 'always' }
+      end
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'with two matching clauses' do
+      let(:rule_hash) do
+        { if: '$VAR == null', changes: '**/*', when: 'always' }
+      end
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'with a matching and non-matching clause' do
+      let(:rule_hash) do
+        { if: '$VAR != null', changes: '$VAR == null', when: 'always' }
+      end
+
+      it { is_expected.to eq(false) }
+    end
+
+    context 'with two non-matching clauses' do
+      let(:rule_hash) do
+        { if: '$VAR != null', changes: 'README', when: 'always' }
+      end
+
+      it { is_expected.to eq(false) }
+    end
+  end
+end

spec/lib/gitlab/ci/build/rules_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Build::Rules do
+  let(:pipeline) { create(:ci_pipeline) }
+  let(:ci_build) { build(:ci_build, pipeline: pipeline) }
+
+  let(:seed) do
+    double('build seed',
+      to_resource: ci_build,
+      scoped_variables_hash: ci_build.scoped_variables_hash
+    )
+  end
+
+  let(:rules) { described_class.new(rule_list) }
+
+  describe '.new' do
+    let(:rules_ivar)   { rules.instance_variable_get :@rule_list }
+    let(:default_when) { rules.instance_variable_get :@default_when }
+
+    context 'with no rules' do
+      let(:rule_list) { [] }
+
+      it 'sets @rule_list to an empty array' do
+        expect(rules_ivar).to eq([])
+      end
+
+      it 'sets @default_when to "on_success"' do
+        expect(default_when).to eq('on_success')
+      end
+    end
+
+    context 'with one rule' do
+      let(:rule_list) { [{ if: '$VAR == null', when: 'always' }] }
+
+      it 'sets @rule_list to an array of a single rule' do
+        expect(rules_ivar).to be_an(Array)
+      end
+
+      it 'sets @default_when to "on_success"' do
+        expect(default_when).to eq('on_success')
+      end
+    end
+
+    context 'with multiple rules' do
+      let(:rule_list) do
+        [
+          { if: '$VAR == null', when: 'always' },
+          { if: '$VAR == null', when: 'always' }
+        ]
+      end
+
+      it 'sets @rule_list to an array of a single rule' do
+        expect(rules_ivar).to be_an(Array)
+      end
+
+      it 'sets @default_when to "on_success"' do
+        expect(default_when).to eq('on_success')
+      end
+    end
+
+    context 'with a specified default when:' do
+      let(:rule_list) { [{ if: '$VAR == null', when: 'always' }] }
+      let(:rules)     { described_class.new(rule_list, 'manual') }
+
+      it 'sets @rule_list to an array of a single rule' do
+        expect(rules_ivar).to be_an(Array)
+      end
+
+      it 'sets @default_when to "manual"' do
+        expect(default_when).to eq('manual')
+      end
+    end
+  end
+
+  describe '#evaluate' do
+    subject { rules.evaluate(pipeline, seed) }
+
+    context 'with nil rules' do
+      let(:rule_list) { nil }
+
+      it { is_expected.to eq(described_class::Result.new('on_success')) }
+
+      context 'and when:manual set as the default' do
+        let(:rules) { described_class.new(rule_list, 'manual') }
+
+        it { is_expected.to eq(described_class::Result.new('manual')) }
+      end
+    end
+
+    context 'with no rules' do
+      let(:rule_list) { [] }
+
+      it { is_expected.to eq(described_class::Result.new('never')) }
+
+      context 'and when:manual set as the default' do
+        let(:rules) { described_class.new(rule_list, 'manual') }
+
+        it { is_expected.to eq(described_class::Result.new('never')) }
+      end
+    end
+
+    context 'with one rule without any clauses' do
+      let(:rule_list) { [{ when: 'manual' }] }
+
+      it { is_expected.to eq(described_class::Result.new('manual')) }
+    end
+
+    context 'with one matching rule' do
+      let(:rule_list) { [{ if: '$VAR == null', when: 'always' }] }
+
+      it { is_expected.to eq(described_class::Result.new('always')) }
+    end
+
+    context 'with two matching rules' do
+      let(:rule_list) do
+        [
+          { if: '$VAR == null', when: 'delayed', start_in: '1 day' },
+          { if: '$VAR == null', when: 'always' }
+        ]
+      end
+
+      it 'returns the value of the first matched rule in the list' do
+        expect(subject).to eq(described_class::Result.new('delayed', '1 day'))
+      end
+    end
+
+    context 'with a non-matching and matching rule' do
+      let(:rule_list) do
+        [
+          { if: '$VAR =! null', when: 'delayed', start_in: '1 day' },
+          { if: '$VAR == null', when: 'always' }
+        ]
+      end
+
+      it { is_expected.to eq(described_class::Result.new('always')) }
+    end
+
+    context 'with a matching and non-matching rule' do
+      let(:rule_list) do
+        [
+          { if: '$VAR == null', when: 'delayed', start_in: '1 day' },
+          { if: '$VAR != null', when: 'always' }
+        ]
+      end
+
+      it { is_expected.to eq(described_class::Result.new('delayed', '1 day')) }
+    end
+
+    context 'with non-matching rules' do
+      let(:rule_list) do
+        [
+          { if: '$VAR != null', when: 'delayed', start_in: '1 day' },
+          { if: '$VAR != null', when: 'always' }
+        ]
+      end
+
+      it { is_expected.to eq(described_class::Result.new('never')) }
+
+      context 'and when:manual set as the default' do
+        let(:rules) { described_class.new(rule_list, 'manual') }
+
+        it 'does not return the default when:' do
+          expect(subject).to eq(described_class::Result.new('never'))
+        end
+      end
+    end
+  end
+end

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

@@ -11,7 +11,7 @@ describe Gitlab::Ci::Config::Entry::Job do
 
       let(:result) do
         %i[before_script script stage type after_script cache
-           image services only except variables artifacts
+           image services only except rules variables artifacts
            environment coverage retry]
       end
 
@@ -201,6 +201,21 @@ describe Gitlab::Ci::Config::Entry::Job do
             expect(entry.errors).to include 'job parallel must be an integer'
           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
       end
 
       context 'when delayed job' do
@@ -240,6 +255,100 @@ describe Gitlab::Ci::Config::Entry::Job do
         end
       end
 
+      context 'when only: is used with rules:' do
+        let(:config) { { only: ['merge_requests'], rules: [{ if: '$THIS' }] } }
+
+        it 'returns error about mixing only: with rules:' do
+          expect(entry).not_to be_valid
+          expect(entry.errors).to include /may not be used with `rules`/
+        end
+
+        context 'and only: is blank' do
+          let(:config) { { only: nil, rules: [{ if: '$THIS' }] } }
+
+          it 'returns error about mixing only: with rules:' do
+            expect(entry).not_to be_valid
+            expect(entry.errors).to include /may not be used with `rules`/
+          end
+        end
+
+        context 'and rules: is blank' do
+          let(:config) { { only: ['merge_requests'], rules: nil } }
+
+          it 'returns error about mixing only: with rules:' do
+            expect(entry).not_to be_valid
+            expect(entry.errors).to include /may not be used with `rules`/
+          end
+        end
+      end
+
+      context 'when except: is used with rules:' do
+        let(:config) { { except: { refs: %w[master] }, rules: [{ if: '$THIS' }] } }
+
+        it 'returns error about mixing except: with rules:' do
+          expect(entry).not_to be_valid
+          expect(entry.errors).to include /may not be used with `rules`/
+        end
+
+        context 'and except: is blank' do
+          let(:config) { { except: nil, rules: [{ if: '$THIS' }] } }
+
+          it 'returns error about mixing except: with rules:' do
+            expect(entry).not_to be_valid
+            expect(entry.errors).to include /may not be used with `rules`/
+          end
+        end
+
+        context 'and rules: is blank' do
+          let(:config) { { except: { refs: %w[master] }, rules: nil } }
+
+          it 'returns error about mixing except: with rules:' do
+            expect(entry).not_to be_valid
+            expect(entry.errors).to include /may not be used with `rules`/
+          end
+        end
+      end
+
+      context 'when only: and except: are both used with rules:' do
+        let(:config) do
+          {
+            only: %w[merge_requests],
+            except: { refs: %w[master] },
+            rules: [{ if: '$THIS' }]
+          }
+        end
+
+        it 'returns errors about mixing both only: and except: with rules:' do
+          expect(entry).not_to be_valid
+          expect(entry.errors).to include /may not be used with `rules`/
+          expect(entry.errors).to include /may not be used with `rules`/
+        end
+
+        context 'when only: and except: as both blank' do
+          let(:config) do
+            { only: nil, except: nil, rules: [{ if: '$THIS' }] }
+          end
+
+          it 'returns errors about mixing both only: and except: with rules:' do
+            expect(entry).not_to be_valid
+            expect(entry.errors).to include /may not be used with `rules`/
+            expect(entry.errors).to include /may not be used with `rules`/
+          end
+        end
+
+        context 'when rules: is blank' do
+          let(:config) do
+            { only: %w[merge_requests], except: { refs: %w[master] }, rules: nil }
+          end
+
+          it 'returns errors about mixing both only: and except: with rules:' do
+            expect(entry).not_to be_valid
+            expect(entry.errors).to include /may not be used with `rules`/
+            expect(entry.errors).to include /may not be used with `rules`/
+          end
+        end
+      end
+
       context 'when start_in specified without delayed specification' do
         let(:config) { { start_in: '1 day' } }
 

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

@@ -51,8 +51,6 @@ describe Gitlab::Ci::Config::Entry::Policy do
 
           let(:config) { ['/^(?!master).+/'] }
 
-          subject { described_class.new([regexp]) }
-
           context 'when allow_unsafe_ruby_regexp is disabled' do
             before do
               stub_feature_flags(allow_unsafe_ruby_regexp: false)
@@ -113,8 +111,6 @@ describe Gitlab::Ci::Config::Entry::Policy do
 
       let(:config) { { refs: ['/^(?!master).+/'] } }
 
-      subject { described_class.new([regexp]) }
-
       context 'when allow_unsafe_ruby_regexp is disabled' do
         before do
           stub_feature_flags(allow_unsafe_ruby_regexp: false)
@@ -203,6 +199,14 @@ describe Gitlab::Ci::Config::Entry::Policy do
       end
     end
 
+    context 'when changes policy is invalid' do
+      let(:config) { { changes: 'some/*' } }
+
+      it 'returns errors' do
+        expect(entry.errors).to include /changes should be an array of strings/
+      end
+    end
+
     context 'when changes policy is invalid' do
       let(:config) { { changes: [1, 2] } }
 

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

+require 'fast_spec_helper'
+require 'chronic_duration'
+require 'support/helpers/stub_feature_flags'
+require_dependency 'active_model'
+
+describe Gitlab::Ci::Config::Entry::Rules::Rule do
+  let(:entry) { described_class.new(config) }
+
+  describe '.new' do
+    subject { entry }
+
+    context 'with a when: value but no clauses' do
+      let(:config) { { when: 'manual' } }
+
+      it { is_expected.to be_valid }
+    end
+
+    context 'when specifying an if: clause' do
+      let(:config) { { if: '$THIS || $THAT', when: 'manual' } }
+
+      it { is_expected.to be_valid }
+
+      describe '#when' do
+        subject { entry.when }
+
+        it { is_expected.to eq('manual') }
+      end
+    end
+
+    context 'using a list of multiple expressions' do
+      let(:config) { { if: ['$MY_VAR == "this"', '$YOUR_VAR == "that"'] } }
+
+      it { is_expected.not_to be_valid }
+
+      it 'reports an error about invalid format' do
+        expect(subject.errors).to include(/invalid expression syntax/)
+      end
+    end
+
+    context 'when specifying an invalid if: clause expression' do
+      let(:config) { { if: ['$MY_VAR =='] } }
+
+      it { is_expected.not_to be_valid }
+
+      it 'reports an error about invalid statement' do
+        expect(subject.errors).to include(/invalid expression syntax/)
+      end
+    end
+
+    context 'when specifying an if: clause expression with an invalid token' do
+      let(:config) { { if: ['$MY_VAR == 123'] } }
+
+      it { is_expected.not_to be_valid }
+
+      it 'reports an error about invalid statement' do
+        expect(subject.errors).to include(/invalid expression syntax/)
+      end
+    end
+
+    context 'when using invalid regex in an if: clause' do
+      let(:config) { { if: ['$MY_VAR =~ /some ( thing/'] } }
+
+      it 'reports an error about invalid expression' do
+        expect(subject.errors).to include(/invalid expression syntax/)
+      end
+    end
+
+    context 'when using an if: clause with lookahead regex character "?"' do
+      let(:config) { { if: '$CI_COMMIT_REF =~ /^(?!master).+/' } }
+
+      context 'when allow_unsafe_ruby_regexp is disabled' do
+        it { is_expected.not_to be_valid }
+
+        it 'reports an error about invalid expression syntax' do
+          expect(subject.errors).to include(/invalid expression syntax/)
+        end
+      end
+    end
+
+    context 'when using a changes: clause' do
+      let(:config) { { changes: %w[app/ lib/ spec/ other/* paths/**/*.rb] } }
+
+      it { is_expected.to be_valid }
+    end
+
+    context 'when using a string as an invalid changes: clause' do
+      let(:config) { { changes: 'a regular string' } }
+
+      it { is_expected.not_to be_valid }
+
+      it 'reports an error about invalid policy' do
+        expect(subject.errors).to include(/should be an array of strings/)
+      end
+    end
+
+    context 'when using a list as an invalid changes: clause' do
+      let(:config) { { changes: [1, 2] } }
+
+      it { is_expected.not_to be_valid }
+
+      it 'returns errors' do
+        expect(subject.errors).to include(/changes should be an array of strings/)
+      end
+    end
+
+    context 'specifying a delayed job' do
+      let(:config) { { if: '$THIS || $THAT', when: 'delayed', start_in: '15 minutes' } }
+
+      it { is_expected.to be_valid }
+
+      it 'sets attributes for the job delay' do
+        expect(entry.when).to eq('delayed')
+        expect(entry.start_in).to eq('15 minutes')
+      end
+
+      context 'without a when: key' do
+        let(:config) { { if: '$THIS || $THAT', start_in: '15 minutes' } }
+
+        it { is_expected.not_to be_valid }
+
+        it 'returns an error about the disallowed key' do
+          expect(entry.errors).to include(/disallowed keys: start_in/)
+        end
+      end
+
+      context 'without a start_in: key' do
+        let(:config) { { if: '$THIS || $THAT', when: 'delayed' } }
+
+        it { is_expected.not_to be_valid }
+
+        it 'returns an error about tstart_in being blank' do
+          expect(entry.errors).to include(/start in can't be blank/)
+        end
+      end
+    end
+
+    context 'when specifying unknown policy' do
+      let(:config) { { invalid: :something } }
+
+      it { is_expected.not_to be_valid }
+
+      it 'returns error about invalid key' do
+        expect(entry.errors).to include(/unknown keys: invalid/)
+      end
+    end
+
+    context 'when clause is empty' do
+      let(:config) { {} }
+
+      it { is_expected.not_to be_valid }
+
+      it 'is not a valid configuration' do
+        expect(entry.errors).to include(/can't be blank/)
+      end
+    end
+
+    context 'when policy strategy does not match' do
+      let(:config) { 'string strategy' }
+
+      it { is_expected.not_to be_valid }
+
+      it 'returns information about errors' do
+        expect(entry.errors)
+          .to include(/should be a hash/)
+      end
+    end
+  end
+
+  describe '#value' do
+    subject { entry.value }
+
+    context 'when specifying an if: clause' do
+      let(:config) { { if: '$THIS || $THAT', when: 'manual' } }
+
+      it 'stores the expression as "if"' do
+        expect(subject).to eq(if: '$THIS || $THAT', when: 'manual')
+      end
+    end
+
+    context 'when using a changes: clause' do
+      let(:config) { { changes: %w[app/ lib/ spec/ other/* paths/**/*.rb] } }
+
+      it { is_expected.to eq(config) }
+    end
+
+    context 'when default value has been provided' do
+      let(:config) { { changes: %w[app/**/*.rb] } }
+
+      before do
+        entry.default = { changes: %w[**/*] }
+      end
+
+      it 'does not set a default value' do
+        expect(entry.default).to eq(nil)
+      end
+
+      it 'does not add to provided configuration' do
+        expect(entry.value).to eq(config)
+      end
+    end
+  end
+
+  describe '.default' do
+    it 'does not have default value' do
+      expect(described_class.default).to be_nil
+    end
+  end
+end

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

+require 'fast_spec_helper'
+require 'support/helpers/stub_feature_flags'
+require_dependency 'active_model'
+
+describe Gitlab::Ci::Config::Entry::Rules do
+  let(:entry) { described_class.new(config) }
+
+  describe '.new' do
+    subject { entry }
+
+    context 'with a list of rule rule' do
+      let(:config) do
+        [{ if: '$THIS == "that"', when: 'never' }]
+      end
+
+      it { is_expected.to be_a(described_class) }
+      it { is_expected.to be_valid }
+
+      context 'after #compose!' do
+        before do
+          subject.compose!
+        end
+
+        it { is_expected.to be_valid }
+      end
+    end
+
+    context 'with a list of two rules' do
+      let(:config) do
+        [
+          { if: '$THIS == "that"', when: 'always' },
+          { if: '$SKIP',           when: 'never' }
+        ]
+      end
+
+      it { is_expected.to be_a(described_class) }
+      it { is_expected.to be_valid }
+
+      context 'after #compose!' do
+        before do
+          subject.compose!
+        end
+
+        it { is_expected.to be_valid }
+      end
+    end
+
+    context 'with a single rule object' do
+      let(:config) do
+        { if: '$SKIP', when: 'never' }
+      end
+
+      it { is_expected.not_to be_valid }
+    end
+
+    context 'with an invalid boolean when:' do
+      let(:config) do
+        [{ if: '$THIS == "that"', when: false }]
+      end
+
+      it { is_expected.to be_a(described_class) }
+      it { is_expected.to be_valid }
+
+      context 'after #compose!' do
+        before do
+          subject.compose!
+        end
+
+        it { is_expected.not_to be_valid }
+
+        it 'returns an error about invalid when:' do
+          expect(subject.errors).to include(/when unknown value: false/)
+        end
+      end
+    end
+
+    context 'with an invalid string when:' do
+      let(:config) do
+        [{ if: '$THIS == "that"', when: 'explode' }]
+      end
+
+      it { is_expected.to be_a(described_class) }
+      it { is_expected.to be_valid }
+
+      context 'after #compose!' do
+        before do
+          subject.compose!
+        end
+
+        it { is_expected.not_to be_valid }
+
+        it 'returns an error about invalid when:' do
+          expect(subject.errors).to include(/when unknown value: explode/)
+        end
+      end
+    end
+  end
+
+  describe '#value' do
+    subject { entry.value }
+
+    context 'with a list of rule rule' do
+      let(:config) do
+        [{ if: '$THIS == "that"', when: 'never' }]
+      end
+
+      it { is_expected.to eq(config) }
+    end
+
+    context 'with a list of two rules' do
+      let(:config) do
+        [
+          { if: '$THIS == "that"', when: 'always' },
+          { if: '$SKIP',           when: 'never' }
+        ]
+      end
+
+      it { is_expected.to eq(config) }
+    end
+
+    context 'with a single rule object' do
+      let(:config) do
+        { if: '$SKIP', when: 'never' }
+      end
+
+      it { is_expected.to eq(config) }
+    end
+  end
+
+  describe '.default' do
+    it 'does not have default policy' do
+      expect(described_class.default).to be_nil
+    end
+  end
+end

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -125,9 +125,11 @@ module Gitlab
         describe 'delayed job entry' do
           context 'when delayed is defined' do
             let(:config) do
-              YAML.dump(rspec: { script: 'rollout 10%',
-                                 when: 'delayed',
-                                 start_in: '1 day' })
+              YAML.dump(rspec: {
+                script:   'rollout 10%',
+                when:     'delayed',
+                start_in: '1 day'
+              })
             end
 
             it 'has the attributes' do
@@ -726,12 +728,12 @@ module Gitlab
         end
       end
 
-      describe "When" do
-        %w(on_success on_failure always).each do |when_state|
-          it "returns #{when_state} when defined" do
+      describe 'when:' do
+        (Gitlab::Ci::Config::Entry::Job::ALLOWED_WHEN - %w[delayed]).each do |when_state|
+          it "#{when_state} creates one build and sets when:" do
             config = YAML.dump({
-                                 rspec: { script: "rspec", when: when_state }
-                               })
+              rspec: { script: 'rspec', when: when_state }
+            })
 
             config_processor = Gitlab::Ci::YamlProcessor.new(config)
             builds = config_processor.stage_builds_attributes("test")
@@ -740,6 +742,35 @@ module Gitlab
             expect(builds.first[:when]).to eq(when_state)
           end
         end
+
+        context 'delayed' do
+          context 'with start_in' do
+            it 'creates one build and sets when:' do
+              config = YAML.dump({
+                rspec: { script: 'rspec', when: 'delayed', start_in: '1 hour' }
+              })
+
+              config_processor = Gitlab::Ci::YamlProcessor.new(config)
+              builds = config_processor.stage_builds_attributes("test")
+
+              expect(builds.size).to eq(1)
+              expect(builds.first[:when]).to eq('delayed')
+              expect(builds.first[:options][:start_in]).to eq('1 hour')
+            end
+          end
+
+          context 'without start_in' do
+            it 'raises an error' do
+              config = YAML.dump({
+                rspec: { script: 'rspec', when: 'delayed' }
+              })
+
+              expect do
+                Gitlab::Ci::YamlProcessor.new(config)
+              end.to raise_error(YamlProcessor::ValidationError, /start in should be a duration/)
+            end
+          end
+        end
       end
 
       describe 'Parallel' do
@@ -1132,7 +1163,7 @@ module Gitlab
           it { expect { subject }.not_to raise_error }
         end
 
-        context 'needs to builds' do
+        context 'needs two builds' do
           let(:needs) { %w(build1 build2) }
 
           it "does create jobs with valid specification" do
@@ -1169,7 +1200,7 @@ module Gitlab
           end
         end
 
-        context 'needs to builds defined as symbols' do
+        context 'needs two builds defined as symbols' do
           let(:needs) { [:build1, :build2] }
 
           it { expect { subject }.not_to raise_error }
@@ -1195,6 +1226,67 @@ module Gitlab
         end
       end
 
+      describe 'rules' do
+        subject { Gitlab::Ci::YamlProcessor.new(YAML.dump(config)) }
+
+        let(:config) do
+          {
+            var_default:     { stage: 'build',  script: 'test', rules: [{ if: '$VAR == null' }] },
+            var_when:        { stage: 'build',  script: 'test', rules: [{ if: '$VAR == null', when: 'always' }] },
+            var_and_changes: { stage: 'build',  script: 'test', rules: [{ if: '$VAR == null', changes: %w[README], when: 'always' }] },
+            changes_not_var: { stage: 'test',   script: 'test', rules: [{ if: '$VAR != null', changes: %w[README] }] },
+            var_not_changes: { stage: 'test',   script: 'test', rules: [{ if: '$VAR == null', changes: %w[other/file.rb], when: 'always' }] },
+            nothing:         { stage: 'test',   script: 'test', rules: [{ when: 'manual' }] },
+            var_never:       { stage: 'deploy', script: 'test', rules: [{ if: '$VAR == null', when: 'never' }] },
+            var_delayed:     { stage: 'deploy', script: 'test', rules: [{ if: '$VAR == null', when: 'delayed', start_in: '3 hours' }] },
+            two_rules:       { stage: 'deploy', script: 'test', rules: [{ if: '$VAR == null', when: 'on_success' }, { changes: %w[README], when: 'manual' }] }
+          }
+        end
+
+        it 'raises no exceptions' do
+          expect { subject }.not_to raise_error
+        end
+
+        it 'returns all jobs regardless of their inclusion' do
+          expect(subject.builds.count).to eq(config.keys.count)
+        end
+
+        context 'used with job-level when' do
+          let(:config) do
+            {
+              var_default: {
+                stage: 'build',
+                script: 'test',
+                when: 'always',
+                rules: [{ if: '$VAR == null' }]
+              }
+            }
+          end
+
+          it 'raises a ValidationError' do
+            expect { subject }.to raise_error(YamlProcessor::ValidationError, /may not be used with `rules`: when/)
+          end
+        end
+
+        context 'used with job-level when:delayed' do
+          let(:config) do
+            {
+              var_default: {
+                stage: 'build',
+                script: 'test',
+                when: 'delayed',
+                start_in: '10 minutes',
+                rules: [{ if: '$VAR == null' }]
+              }
+            }
+          end
+
+          it 'raises a ValidationError' do
+            expect { subject }.to raise_error(YamlProcessor::ValidationError, /may not be used with `rules`: when, start_in/)
+          end
+        end
+      end
+
       describe "Hidden jobs" do
         let(:config_processor) { Gitlab::Ci::YamlProcessor.new(config) }
         subject { config_processor.stage_builds_attributes("test") }
@@ -1513,7 +1605,7 @@ module Gitlab
           config = YAML.dump({ rspec: { script: "test", when: 1 } })
           expect do
             Gitlab::Ci::YamlProcessor.new(config)
-          end.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, "jobs:rspec when should be on_success, on_failure, always, manual or delayed")
+          end.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, "jobs:rspec when should be one of: #{Gitlab::Ci::Config::Entry::Job::ALLOWED_WHEN.join(', ')}")
         end
 
         it "returns errors if job artifacts:name is not an a string" do

spec/support/helpers/stub_gitlab_calls.rb

@@ -22,6 +22,10 @@ module StubGitlabCalls
     allow_any_instance_of(Ci::Pipeline).to receive(:ci_yaml_file) { ci_yaml }
   end
 
+  def stub_pipeline_modified_paths(pipeline, modified_paths)
+    allow(pipeline).to receive(:modified_paths).and_return(modified_paths)
+  end
+
   def stub_repository_ci_yaml_file(sha:, path: '.gitlab-ci.yml')
     allow_any_instance_of(Repository)
       .to receive(:gitlab_ci_yml_for).with(sha, path)