diff options
22 files changed, 1406 insertions, 43 deletions
diff --git a/changelogs/unreleased/ci-config-on-policy.yml b/changelogs/unreleased/ci-config-on-policy.yml new file mode 100644 index 00000000000..d9804f0323a --- /dev/null +++ b/changelogs/unreleased/ci-config-on-policy.yml @@ -0,0 +1,5 @@ +--- +title: Introduced Build::Rules configuration for Ci::Build +merge_request: 29011 +author: +type: added diff --git a/lib/gitlab/ci/build/policy/variables.rb b/lib/gitlab/ci/build/policy/variables.rb index 0698136166a..e9c8864123f 100644 --- a/lib/gitlab/ci/build/policy/variables.rb +++ b/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 diff --git a/lib/gitlab/ci/build/rules.rb b/lib/gitlab/ci/build/rules.rb new file mode 100644 index 00000000000..89623a809c9 --- /dev/null +++ b/lib/gitlab/ci/build/rules.rb @@ -0,0 +1,37 @@ +# 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 diff --git a/lib/gitlab/ci/build/rules/rule.rb b/lib/gitlab/ci/build/rules/rule.rb new file mode 100644 index 00000000000..8d52158c8d2 --- /dev/null +++ b/lib/gitlab/ci/build/rules/rule.rb @@ -0,0 +1,32 @@ +# 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 diff --git a/lib/gitlab/ci/build/rules/rule/clause.rb b/lib/gitlab/ci/build/rules/rule/clause.rb new file mode 100644 index 00000000000..ff0baf3348c --- /dev/null +++ b/lib/gitlab/ci/build/rules/rule/clause.rb @@ -0,0 +1,31 @@ +# 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 diff --git a/lib/gitlab/ci/build/rules/rule/clause/changes.rb b/lib/gitlab/ci/build/rules/rule/clause/changes.rb new file mode 100644 index 00000000000..81d2ee6c24c --- /dev/null +++ b/lib/gitlab/ci/build/rules/rule/clause/changes.rb @@ -0,0 +1,23 @@ +# 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 diff --git a/lib/gitlab/ci/build/rules/rule/clause/if.rb b/lib/gitlab/ci/build/rules/rule/clause/if.rb new file mode 100644 index 00000000000..18c3b450f95 --- /dev/null +++ b/lib/gitlab/ci/build/rules/rule/clause/if.rb @@ -0,0 +1,19 @@ +# 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 diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 29a52b9da17..6e11c582750 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/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 diff --git a/lib/gitlab/ci/config/entry/rules.rb b/lib/gitlab/ci/config/entry/rules.rb new file mode 100644 index 00000000000..65cad0880f5 --- /dev/null +++ b/lib/gitlab/ci/config/entry/rules.rb @@ -0,0 +1,33 @@ +# 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 diff --git a/lib/gitlab/ci/config/entry/rules/rule.rb b/lib/gitlab/ci/config/entry/rules/rule.rb new file mode 100644 index 00000000000..1f2a34ec90e --- /dev/null +++ b/lib/gitlab/ci/config/entry/rules/rule.rb @@ -0,0 +1,42 @@ +# 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 diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index 7ec03d132c0..1066331062b 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/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 diff --git a/lib/gitlab/config/entry/validators.rb b/lib/gitlab/config/entry/validators.rb index 0289e675c6b..374f929878e 100644 --- a/lib/gitlab/config/entry/validators.rb +++ b/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) diff --git a/spec/lib/gitlab/ci/build/policy/variables_spec.rb b/spec/lib/gitlab/ci/build/policy/variables_spec.rb index f712f47a558..7140c14facb 100644 --- a/spec/lib/gitlab/ci/build/policy/variables_spec.rb +++ b/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']) diff --git a/spec/lib/gitlab/ci/build/rules/rule_spec.rb b/spec/lib/gitlab/ci/build/rules/rule_spec.rb new file mode 100644 index 00000000000..99852bd4228 --- /dev/null +++ b/spec/lib/gitlab/ci/build/rules/rule_spec.rb @@ -0,0 +1,50 @@ +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 diff --git a/spec/lib/gitlab/ci/build/rules_spec.rb b/spec/lib/gitlab/ci/build/rules_spec.rb new file mode 100644 index 00000000000..d7793ebc806 --- /dev/null +++ b/spec/lib/gitlab/ci/build/rules_spec.rb @@ -0,0 +1,168 @@ +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 diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 415ade7a096..1853efde350 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/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' } } diff --git a/spec/lib/gitlab/ci/config/entry/policy_spec.rb b/spec/lib/gitlab/ci/config/entry/policy_spec.rb index 266a27c1e47..a606eb303e7 100644 --- a/spec/lib/gitlab/ci/config/entry/policy_spec.rb +++ b/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) @@ -204,6 +200,14 @@ describe Gitlab::Ci::Config::Entry::Policy do 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] } } it 'returns errors' do diff --git a/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb b/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb new file mode 100644 index 00000000000..c25344ec1a4 --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb @@ -0,0 +1,208 @@ +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 diff --git a/spec/lib/gitlab/ci/config/entry/rules_spec.rb b/spec/lib/gitlab/ci/config/entry/rules_spec.rb new file mode 100644 index 00000000000..291e7373daf --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/rules_spec.rb @@ -0,0 +1,135 @@ +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 diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index 1a9350d68bd..89431b80be3 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -15,6 +15,60 @@ describe Gitlab::Ci::Pipeline::Seed::Build do it { is_expected.to be_a(Hash) } it { is_expected.to include(:name, :project, :ref) } + + context 'with job:when' do + let(:attributes) { { name: 'rspec', ref: 'master', when: 'on_failure' } } + + it { is_expected.to include(when: 'on_failure') } + end + + context 'with job:when:delayed' do + let(:attributes) { { name: 'rspec', ref: 'master', when: 'delayed', start_in: '3 hours' } } + + it { is_expected.to include(when: 'delayed', start_in: '3 hours') } + end + + context 'with job:rules:[when:]' do + context 'is matched' do + let(:attributes) { { name: 'rspec', ref: 'master', rules: [{ if: '$VAR == null', when: 'always' }] } } + + it { is_expected.to include(when: 'always') } + end + + context 'is not matched' do + let(:attributes) { { name: 'rspec', ref: 'master', rules: [{ if: '$VAR != null', when: 'always' }] } } + + it { is_expected.to include(when: 'never') } + end + end + + context 'with job:rules:[when:delayed]' 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') } + end + + context 'is not matched' do + let(:attributes) { { name: 'rspec', ref: 'master', rules: [{ if: '$VAR != null', when: 'delayed', start_in: '3 hours' }] } } + + it { is_expected.to include(when: 'never') } + 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' }] } } + + it { is_expected.to include(when: 'on_success') } + end + + context 'is not matched' do + let(:attributes) { { name: 'rspec', ref: 'master', rules: [{ if: '$VAR != null' }] } } + + it { is_expected.to include(when: 'never') } + end + end end describe '#bridge?' do @@ -366,9 +420,25 @@ describe Gitlab::Ci::Pipeline::Seed::Build do it { is_expected.not_to be_included } end + + context 'when using both only and except policies' do + let(:attributes) do + { + name: 'rspec', + only: { + refs: ["branches@#{pipeline.project_full_path}"] + }, + except: { + refs: ["branches@#{pipeline.project_full_path}"] + } + } + end + + it { is_expected.not_to be_included } + end end - context 'when repository path does not matches' do + context 'when repository path does not match' do context 'when using only' do let(:attributes) do { name: 'rspec', only: { refs: %w[branches@fork] } } @@ -397,6 +467,215 @@ describe Gitlab::Ci::Pipeline::Seed::Build do it { is_expected.not_to be_included } end end + + context 'using rules:' do + using RSpec::Parameterized + + let(:attributes) { { name: 'rspec', rules: rule_set } } + + context 'with a matching if: rule' do + context 'with an explicit `when: never`' do + where(:rule_set) do + [ + [[{ if: '$VARIABLE == null', when: 'never' }]], + [[{ if: '$VARIABLE == null', when: 'never' }, { if: '$VARIABLE == null', when: 'always' }]], + [[{ if: '$VARIABLE != "the wrong value"', when: 'never' }, { if: '$VARIABLE == null', when: 'always' }]] + ] + end + + with_them do + it { is_expected.not_to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'never') + end + end + end + + context 'with an explicit `when: always`' do + where(:rule_set) do + [ + [[{ if: '$VARIABLE == null', when: 'always' }]], + [[{ if: '$VARIABLE == null', when: 'always' }, { if: '$VARIABLE == null', when: 'never' }]], + [[{ if: '$VARIABLE != "the wrong value"', when: 'always' }, { if: '$VARIABLE == null', when: 'never' }]] + ] + end + + with_them do + it { is_expected.to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'always') + end + end + end + + context 'with an explicit `when: on_failure`' do + where(:rule_set) do + [ + [[{ if: '$CI_JOB_NAME == "rspec" && $VAR == null', when: 'on_failure' }]], + [[{ if: '$VARIABLE != null', when: 'delayed', start_in: '1 day' }, { if: '$CI_JOB_NAME == "rspec"', when: 'on_failure' }]], + [[{ if: '$VARIABLE == "the wrong value"', when: 'delayed', start_in: '1 day' }, { if: '$CI_BUILD_NAME == "rspec"', when: 'on_failure' }]] + ] + end + + with_them do + it { is_expected.to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'on_failure') + end + end + end + + context 'with an explicit `when: delayed`' do + where(:rule_set) do + [ + [[{ if: '$VARIABLE == null', when: 'delayed', start_in: '1 day' }]], + [[{ if: '$VARIABLE == null', when: 'delayed', start_in: '1 day' }, { if: '$VARIABLE == null', when: 'never' }]], + [[{ if: '$VARIABLE != "the wrong value"', when: 'delayed', start_in: '1 day' }, { if: '$VARIABLE == null', when: 'never' }]] + ] + end + + with_them do + it { is_expected.to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'delayed', start_in: '1 day') + end + end + end + + context 'without an explicit when: value' do + where(:rule_set) do + [ + [[{ if: '$VARIABLE == null' }]], + [[{ if: '$VARIABLE == null' }, { if: '$VARIABLE == null' }]], + [[{ if: '$VARIABLE != "the wrong value"' }, { if: '$VARIABLE == null' }]] + ] + end + + with_them do + it { is_expected.to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'on_success') + end + end + end + end + + context 'with a matching changes: rule' do + let(:pipeline) do + create(:ci_pipeline, project: project).tap do |pipeline| + stub_pipeline_modified_paths(pipeline, %w[app/models/ci/pipeline.rb spec/models/ci/pipeline_spec.rb .gitlab-ci.yml]) + end + end + + context 'with an explicit `when: never`' do + where(:rule_set) do + [ + [[{ changes: %w[*/**/*.rb], when: 'never' }, { changes: %w[*/**/*.rb], when: 'always' }]], + [[{ changes: %w[app/models/ci/pipeline.rb], when: 'never' }, { changes: %w[app/models/ci/pipeline.rb], when: 'always' }]], + [[{ changes: %w[spec/**/*.rb], when: 'never' }, { changes: %w[spec/**/*.rb], when: 'always' }]], + [[{ changes: %w[*.yml], when: 'never' }, { changes: %w[*.yml], when: 'always' }]], + [[{ changes: %w[.*.yml], when: 'never' }, { changes: %w[.*.yml], when: 'always' }]], + [[{ changes: %w[**/*], when: 'never' }, { changes: %w[**/*], when: 'always' }]], + [[{ changes: %w[*/**/*.rb *.yml], when: 'never' }, { changes: %w[*/**/*.rb *.yml], when: 'always' }]], + [[{ changes: %w[.*.yml **/*], when: 'never' }, { changes: %w[.*.yml **/*], when: 'always' }]] + ] + end + + with_them do + it { is_expected.not_to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'never') + end + end + end + + context 'with an explicit `when: always`' do + where(:rule_set) do + [ + [[{ changes: %w[*/**/*.rb], when: 'always' }, { changes: %w[*/**/*.rb], when: 'never' }]], + [[{ changes: %w[app/models/ci/pipeline.rb], when: 'always' }, { changes: %w[app/models/ci/pipeline.rb], when: 'never' }]], + [[{ changes: %w[spec/**/*.rb], when: 'always' }, { changes: %w[spec/**/*.rb], when: 'never' }]], + [[{ changes: %w[*.yml], when: 'always' }, { changes: %w[*.yml], when: 'never' }]], + [[{ changes: %w[.*.yml], when: 'always' }, { changes: %w[.*.yml], when: 'never' }]], + [[{ changes: %w[**/*], when: 'always' }, { changes: %w[**/*], when: 'never' }]], + [[{ changes: %w[*/**/*.rb *.yml], when: 'always' }, { changes: %w[*/**/*.rb *.yml], when: 'never' }]], + [[{ changes: %w[.*.yml **/*], when: 'always' }, { changes: %w[.*.yml **/*], when: 'never' }]] + ] + end + + with_them do + it { is_expected.to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'always') + end + end + end + + context 'without an explicit when: value' do + where(:rule_set) do + [ + [[{ changes: %w[*/**/*.rb] }]], + [[{ changes: %w[app/models/ci/pipeline.rb] }]], + [[{ changes: %w[spec/**/*.rb] }]], + [[{ changes: %w[*.yml] }]], + [[{ changes: %w[.*.yml] }]], + [[{ changes: %w[**/*] }]], + [[{ changes: %w[*/**/*.rb *.yml] }]], + [[{ changes: %w[.*.yml **/*] }]] + ] + end + + with_them do + it { is_expected.to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'on_success') + end + end + end + end + + context 'with no matching rule' do + where(:rule_set) do + [ + [[{ if: '$VARIABLE != null', when: 'never' }]], + [[{ if: '$VARIABLE != null', when: 'never' }, { if: '$VARIABLE != null', when: 'always' }]], + [[{ if: '$VARIABLE == "the wrong value"', when: 'never' }, { if: '$VARIABLE != null', when: 'always' }]], + [[{ if: '$VARIABLE != null', when: 'always' }]], + [[{ if: '$VARIABLE != null', when: 'always' }, { if: '$VARIABLE != null', when: 'never' }]], + [[{ if: '$VARIABLE == "the wrong value"', when: 'always' }, { if: '$VARIABLE != null', when: 'never' }]], + [[{ if: '$VARIABLE != null' }]], + [[{ if: '$VARIABLE != null' }, { if: '$VARIABLE != null' }]], + [[{ if: '$VARIABLE == "the wrong value"' }, { if: '$VARIABLE != null' }]] + ] + end + + with_them do + it { is_expected.not_to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'never') + end + end + end + + context 'with no rules' do + let(:rule_set) { [] } + + it { is_expected.not_to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'never') + end + end + end end describe 'applying needs: dependency' do @@ -476,4 +755,10 @@ describe Gitlab::Ci::Pipeline::Seed::Build do end end end + + describe '#scoped_variables_hash' do + subject { seed_build.scoped_variables_hash } + + it { is_expected.to eq(seed_build.to_resource.scoped_variables_hash) } + end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index d5567b4f166..91c559dcd9b 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/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 diff --git a/spec/support/helpers/stub_gitlab_calls.rb b/spec/support/helpers/stub_gitlab_calls.rb index badea94352a..7d10cffe920 100644 --- a/spec/support/helpers/stub_gitlab_calls.rb +++ b/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) |