Skip to content

G
gitlab-ce
Commit 750b16ff authored by Grzegorz Bizon's avatar Grzegorz Bizon
Browse files
Options

Merge branch 'add-config-processable' into 'master' 

Introduce Processable concern for config

See merge request gitlab-org/gitlab!25683
parents 36a899b8 dcae09d1
Hide whitespace changes
Inline Side-by-side
Showing with 413 additions and 166 deletions
lib/gitlab/ci/config/entry/bridge.rb
View file @ 750b16ff
......@@ -9,34 +9,21 @@ module Gitlab
# defining a downstream project trigger.
#
class Bridge < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Configurable
include ::Gitlab::Config::Entry::Attributable
include ::Gitlab::Config::Entry::Inheritable
include ::Gitlab::Ci::Config::Entry::Processable
ALLOWED_KEYS = %i[trigger stage allow_failure only except
when extends variables needs rules].freeze
ALLOWED_KEYS = %i[trigger allow_failure when variables needs].freeze
validations do
validates :config, allowed_keys: ALLOWED_KEYS
validates :config, presence: true
validates :name, presence: true
validates :name, type: Symbol
validates :config, disallowed_keys: {
in: %i[only except when start_in],
message: 'key may not be used with `rules`'
},
if: :has_rules?
validates :config, allowed_keys: ALLOWED_KEYS + PROCESSABLE_ALLOWED_KEYS
with_options allow_nil: true do
validates :when,
inclusion: { in: %w[on_success on_failure always],
message: 'should be on_success, on_failure or always' }
validates :extends, type: String
validates :rules, array_of_hashes: true
end
validate on: :composed do
unless trigger.present? || bridge_needs.present?
unless trigger_defined? || bridge_needs.present?
errors.add(:config, 'should contain either a trigger or a needs:pipeline')
end
end
......@@ -58,32 +45,13 @@ module Gitlab
inherit: false,
metadata: { allowed_needs: %i[job bridge] }
entry :stage, ::Gitlab::Ci::Config::Entry::Stage,
description: 'Pipeline stage this job will be executed into.',
inherit: false
entry :only, ::Gitlab::Ci::Config::Entry::Policy,
description: 'Refs policy this job will be executed for.',
default: ::Gitlab::Ci::Config::Entry::Policy::DEFAULT_ONLY,
inherit: false
entry :except, ::Gitlab::Ci::Config::Entry::Policy,
description: 'Refs policy this job will be executed for.',
inherit: false
entry :rules, ::Gitlab::Ci::Config::Entry::Rules,
description: 'List of evaluable Rules to determine job inclusion.',
inherit: false,
metadata: {
allowed_when: %w[on_success on_failure always never manual delayed].freeze
}
entry :variables, ::Gitlab::Ci::Config::Entry::Variables,
description: 'Environment variables available for this job.',
inherit: false
helpers(*ALLOWED_KEYS)
attributes(*ALLOWED_KEYS)
helpers :trigger, :needs, :variables
attributes :when, :allow_failure
def self.matching?(name, config)
!name.to_s.start_with?('.') &&
......@@ -95,56 +63,20 @@ module Gitlab
true
end
def compose!(deps = nil)
super do
has_workflow_rules = deps&.workflow&.has_rules?
# If workflow:rules: or rules: are used
# they are considered not compatible
# with `only/except` defaults
#
# Context: https://gitlab.com/gitlab-org/gitlab/merge_requests/21742
if has_rules? || has_workflow_rules
# Remove only/except defaults
# defaults are not considered as defined
@entries.delete(:only) unless only_defined?
@entries.delete(:except) unless except_defined?
end
end
end
def has_rules?
@config&.key?(:rules)
end
def name
@metadata[:name]
end
def value
{ name: name,
super.merge(
trigger: (trigger_value if trigger_defined?),
needs: (needs_value if needs_defined?),
ignore: !!allow_failure,
stage: stage_value,
when: when_value,
extends: extends_value,
when: self.when,
variables: (variables_value if variables_defined?),
rules: (rules_value if has_rules?),
only: only_value,
except: except_value,
scheduling_type: needs_defined? && !bridge_needs ? :dag : :stage }.compact
scheduling_type: needs_defined? && !bridge_needs ? :dag : :stage
).compact
end
def bridge_needs
needs_value[:bridge] if needs_value
end
private
def overwrite_entry(deps, key, current_entry)
deps.default[key] unless current_entry.specified?
end
end
end
end
......
lib/gitlab/ci/config/entry/job.rb
View file @ 750b16ff
......@@ -8,33 +8,21 @@ module Gitlab
# Entry that represents a concrete CI/CD job.
#
class Job < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Configurable
include ::Gitlab::Config::Entry::Attributable
include ::Gitlab::Config::Entry::Inheritable
include ::Gitlab::Ci::Config::Entry::Processable
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
ALLOWED_KEYS = %i[tags script type image services
allow_failure type when start_in artifacts cache
dependencies before_script needs after_script variables
environment coverage retry parallel extends interruptible timeout
environment coverage retry parallel interruptible timeout
resource_group release].freeze
REQUIRED_BY_NEEDS = %i[stage].freeze
validations do
validates :config, type: Hash
validates :config, allowed_keys: ALLOWED_KEYS
validates :config, allowed_keys: ALLOWED_KEYS + PROCESSABLE_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?
validates :config,
disallowed_keys: {
in: %i[release],
......@@ -53,8 +41,6 @@ module Gitlab
}
validates :dependencies, array_of_strings: true
validates :extends, array_of_strings_or_string: true
validates :rules, array_of_hashes: true
validates :resource_group, type: String
end
......@@ -81,10 +67,6 @@ module Gitlab
description: 'Commands that will be executed in this job.',
inherit: false
entry :stage, Entry::Stage,
description: 'Pipeline stage this job will be executed into.',
inherit: false
entry :type, Entry::Stage,
description: 'Deprecated: stage this job will be executed into.',
inherit: false
......@@ -125,22 +107,6 @@ module Gitlab
description: 'Artifacts configuration for this job.',
inherit: true
entry :only, Entry::Policy,
description: 'Refs policy this job will be executed for.',
default: ::Gitlab::Ci::Config::Entry::Policy::DEFAULT_ONLY,
inherit: false
entry :except, Entry::Policy,
description: 'Refs policy this job will be executed for.',
inherit: false
entry :rules, Entry::Rules,
description: 'List of evaluable Rules to determine job inclusion.',
inherit: false,
metadata: {
allowed_when: %w[on_success on_failure always never manual delayed].freeze
}
entry :needs, Entry::Needs,
description: 'Needs configuration for this job.',
metadata: { allowed_needs: %i[job cross_dependency] },
......@@ -162,13 +128,13 @@ module Gitlab
description: 'This job will produce a release.',
inherit: false
helpers :before_script, :script, :stage, :type, :after_script,
:cache, :image, :services, :only, :except, :variables,
:artifacts, :environment, :coverage, :retry, :rules,
:parallel, :needs, :interruptible, :release, :tags
helpers :before_script, :script, :type, :after_script,
:cache, :image, :services, :variables,
:artifacts, :environment, :coverage, :retry,
:needs, :interruptible, :release, :tags
attributes :script, :tags, :allow_failure, :when, :dependencies,
:needs, :retry, :parallel, :extends, :start_in, :rules,
:needs, :retry, :parallel, :start_in,
:interruptible, :timeout, :resource_group, :release
def self.matching?(name, config)
......@@ -187,31 +153,9 @@ module Gitlab
end
@entries.delete(:type)
has_workflow_rules = deps&.workflow&.has_rules?
# If workflow:rules: or rules: are used
# they are considered not compatible
# with `only/except` defaults
#
# Context: https://gitlab.com/gitlab-org/gitlab/merge_requests/21742
if has_rules? || has_workflow_rules
# Remove only/except defaults
# defaults are not considered as defined
@entries.delete(:only) unless only_defined?
@entries.delete(:except) unless except_defined?
end
end
end
def name
@metadata[:name]
end
def value
@config.merge(to_hash.compact)
end
def manual_action?
self.when == 'manual'
end
......@@ -220,38 +164,27 @@ module Gitlab
self.when == 'delayed'
end
def has_rules?
@config.try(:key?, :rules)
end
def ignored?
allow_failure.nil? ? manual_action? : allow_failure
end
private
def overwrite_entry(deps, key, current_entry)
deps.default[key] unless current_entry.specified?
end
def to_hash
{ name: name,
def value
super.merge(
before_script: before_script_value,
script: script_value,
image: image_value,
services: services_value,
stage: stage_value,
cache: cache_value,
tags: tags_value,
only: only_value,
except: except_value,
rules: has_rules? ? rules_value : nil,
when: self.when,
start_in: self.start_in,
dependencies: dependencies,
variables: variables_defined? ? variables_value : {},
environment: environment_defined? ? environment_value : nil,
environment_name: environment_defined? ? environment_value[:name] : nil,
coverage: coverage_defined? ? coverage_value : nil,
retry: retry_defined? ? retry_value : nil,
parallel: parallel_defined? ? parallel_value.to_i : nil,
parallel: has_parallel? ? parallel.to_i : nil,
interruptible: interruptible_defined? ? interruptible_value : nil,
timeout: has_timeout? ? ChronicDuration.parse(timeout.to_s) : nil,
artifacts: artifacts_value,
......@@ -260,7 +193,8 @@ module Gitlab
ignore: ignored?,
needs: needs_defined? ? needs_value : nil,
resource_group: resource_group,
scheduling_type: needs_defined? ? :dag : :stage }
scheduling_type: needs_defined? ? :dag : :stage
).compact
end
end
end
......
lib/gitlab/ci/config/entry/processable.rb 0 → 100644
View file @ 750b16ff
# frozen_string_literal: true
module Gitlab
module Ci
class Config
module Entry
##
# Entry that represents a CI/CD Processable (a job)
#
module Processable
extend ActiveSupport::Concern
include ::Gitlab::Config::Entry::Configurable
include ::Gitlab::Config::Entry::Attributable
include ::Gitlab::Config::Entry::Inheritable
PROCESSABLE_ALLOWED_KEYS = %i[extends stage only except rules].freeze
included do
validations do
validates :config, presence: true
validates :name, presence: true
validates :name, type: Symbol
validates :config, disallowed_keys: {
in: %i[only except when start_in],
message: 'key may not be used with `rules`'
},
if: :has_rules?
with_options allow_nil: true do
validates :extends, array_of_strings_or_string: true
validates :rules, array_of_hashes: true
end
end
entry :stage, Entry::Stage,
description: 'Pipeline stage this job will be executed into.',
inherit: false
entry :only, ::Gitlab::Ci::Config::Entry::Policy,
description: 'Refs policy this job will be executed for.',
default: ::Gitlab::Ci::Config::Entry::Policy::DEFAULT_ONLY,
inherit: false
entry :except, ::Gitlab::Ci::Config::Entry::Policy,
description: 'Refs policy this job will be executed for.',
inherit: false
entry :rules, ::Gitlab::Ci::Config::Entry::Rules,
description: 'List of evaluable Rules to determine job inclusion.',
inherit: false,
metadata: {
allowed_when: %w[on_success on_failure always never manual delayed].freeze
}
helpers :stage, :only, :except, :rules
attributes :extends, :rules
end
def compose!(deps = nil)
super do
has_workflow_rules = deps&.workflow&.has_rules?
# If workflow:rules: or rules: are used
# they are considered not compatible
# with `only/except` defaults
#
# Context: https://gitlab.com/gitlab-org/gitlab/merge_requests/21742
if has_rules? || has_workflow_rules
# Remove only/except defaults
# defaults are not considered as defined
@entries.delete(:only) unless only_defined? # rubocop:disable Gitlab/ModuleWithInstanceVariables
@entries.delete(:except) unless except_defined? # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
yield if block_given?
end
end
def name
metadata[:name]
end
def overwrite_entry(deps, key, current_entry)
deps.default[key] unless current_entry.specified?
end
def value
{ name: name,
stage: stage_value,
extends: extends,
rules: rules_value,
only: only_value,
except: except_value }.compact
end
end
end
end
end
end
lib/gitlab/ci/config/entry/root.rb
View file @ 750b16ff
......@@ -67,7 +67,9 @@ module Gitlab
entry :workflow, Entry::Workflow,
description: 'List of evaluable rules to determine Pipeline status'
helpers :default, :jobs, :stages, :types, :variables, :workflow
helpers :default, :stages, :types, :variables, :workflow
helpers :jobs, dynamic: true
delegate :before_script_value,
:image_value,
......
lib/gitlab/config/entry/configurable.rb
View file @ 750b16ff
......@@ -75,6 +75,8 @@ module Gitlab
# rubocop: disable CodeReuse/ActiveRecord
def entry(key, entry, description: nil, default: nil, inherit: nil, reserved: nil, metadata: {})
raise ArgumentError, "Entry #{key} already defined" if @nodes.to_h[key.to_sym]
factory = ::Gitlab::Config::Entry::Factory.new(entry)
.with(description: description)
.with(default: default)
......@@ -86,8 +88,16 @@ module Gitlab
end
# rubocop: enable CodeReuse/ActiveRecord
def helpers(*nodes)
def helpers(*nodes, dynamic: false)
nodes.each do |symbol|
if method_defined?("#{symbol}_defined?") || method_defined?("#{symbol}_value")
raise ArgumentError, "Method #{symbol}_defined? or #{symbol}_value already defined"
end
unless @nodes.to_h[symbol]
raise ArgumentError, "Entry for #{symbol} is undefined" unless dynamic
end
define_method("#{symbol}_defined?") do
entries[symbol]&.specified?
end
......
spec/lib/gitlab/ci/config/entry/processable_spec.rb 0 → 100644
View file @ 750b16ff
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Ci::Config::Entry::Processable do
let(:node_class) do
Class.new(::Gitlab::Config::Entry::Node) do
include Gitlab::Ci::Config::Entry::Processable
def self.name
'job'
end
end
end
let(:entry) { node_class.new(config, name: :rspec) }
describe 'validations' do
before do
entry.compose!
end
context 'when entry config value is correct' do
let(:config) { { stage: 'test' } }
describe '#valid?' do
it 'is valid' do
expect(entry).to be_valid
end
end
context 'when job name is empty' do
let(:entry) { node_class.new(config, name: ''.to_sym) }
it 'reports error' do
expect(entry.errors).to include "job name can't be blank"
end
end
end
context 'when entry value is not correct' do
context 'incorrect config value type' do
let(:config) { ['incorrect'] }
describe '#errors' do
it 'reports error about a config type' do
expect(entry.errors)
.to include 'job config should be a hash'
end
end
end
context 'when config is empty' do
let(:config) { {} }
describe '#valid' do
it 'is invalid' do
expect(entry).not_to be_valid
end
end
end
context 'when extends key is not a string' do
let(:config) { { extends: 123 } }
it 'returns error about wrong value type' do
expect(entry).not_to be_valid
expect(entry.errors).to include "job extends should be an array of strings or a string"
end
end
context 'when it uses both "when:" and "rules:"' do
let(:config) do
{
script: 'echo',
when: 'on_failure',
rules: [{ if: '$VARIABLE', when: 'on_success' }]
}
end
it 'returns an error about when: being combined with rules' do
expect(entry).not_to be_valid
expect(entry.errors).to include 'job config key may not be used with `rules`: when'
end
end
context 'when only: is used with rules:' do
let(:config) { { only: ['merge_requests'], rules: [{ if: '$THIS' }] } }
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
end
end
describe '#relevant?' do
it 'is a relevant entry' do
entry = node_class.new({ stage: 'test' }, name: :rspec)
expect(entry).to be_relevant
end
end
describe '#compose!' do
let(:specified) do
double('specified', 'specified?' => true, value: 'specified')
end
let(:unspecified) { double('unspecified', 'specified?' => false) }
let(:default) { double('default', '[]' => unspecified) }
let(:workflow) { double('workflow', 'has_rules?' => false) }
let(:deps) { double('deps', 'default' => default, '[]' => unspecified, 'workflow' => workflow) }
context 'with workflow rules' do
using RSpec::Parameterized::TableSyntax
where(:name, :has_workflow_rules?, :only, :rules, :result) do
"uses default only" | false | nil | nil | { refs: %w[branches tags] }
"uses user only" | false | %w[branches] | nil | { refs: %w[branches] }
"does not define only" | false | nil | [] | nil
"does not define only" | true | nil | nil | nil
"uses user only" | true | %w[branches] | nil | { refs: %w[branches] }
"does not define only" | true | nil | [] | nil
end
with_them do
let(:config) { { script: 'ls', rules: rules, only: only }.compact }
it "#{name}" do
expect(workflow).to receive(:has_rules?) { has_workflow_rules? }
entry.compose!(deps)
expect(entry.only_value).to eq(result)
end
end
end
context 'when workflow rules is used' do
context 'when rules are used' do
let(:config) { { script: 'ls', cache: { key: 'test' }, rules: [] } }
it 'does not define only' do
expect(entry).not_to be_only_defined
end
end
context 'when rules are not used' do
let(:config) { { script: 'ls', cache: { key: 'test' }, only: [] } }
it 'does not define only' do
expect(entry).not_to be_only_defined
end
end
end
end
context 'when composed' do
before do
entry.compose!
end
describe '#value' do
context 'when entry is correct' do
let(:config) do
{ stage: 'test' }
end
it 'returns correct value' do
expect(entry.value)
.to eq(name: :rspec,
stage: 'test',
only: { refs: %w[branches tags] })
end
end
end
end
end
spec/lib/gitlab/ci/yaml_processor_spec.rb
View file @ 750b16ff
......@@ -2419,7 +2419,9 @@ module Gitlab
it 'returns errors and empty configuration' do
expect(subject.valid?).to eq(false)
expect(subject.errors).to eq(['jobs:rspec config contains unknown keys: bad_tags', 'jobs:rspec rules should be an array of hashes'])
expect(subject.errors).to contain_exactly(
'jobs:rspec config contains unknown keys: bad_tags',
'jobs:rspec rules should be an array of hashes')
expect(subject.content).to be_blank
end
end
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7