Commit dcae09d1 authored by Kamil Trzciński's avatar Kamil Trzciński

Add additional entry validations

Fix support for Bridge job when

Fix support for dependencies
parent 3ec80328
...@@ -20,12 +20,10 @@ module Gitlab ...@@ -20,12 +20,10 @@ module Gitlab
validates :when, validates :when,
inclusion: { in: %w[on_success on_failure always], inclusion: { in: %w[on_success on_failure always],
message: 'should be on_success, on_failure or always' } message: 'should be on_success, on_failure or always' }
validates :extends, type: String
validates :rules, array_of_hashes: true
end end
validate on: :composed do 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') errors.add(:config, 'should contain either a trigger or a needs:pipeline')
end end
end end
...@@ -47,16 +45,13 @@ module Gitlab ...@@ -47,16 +45,13 @@ module Gitlab
inherit: false, inherit: false,
metadata: { allowed_needs: %i[job bridge] } 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 :variables, ::Gitlab::Ci::Config::Entry::Variables, entry :variables, ::Gitlab::Ci::Config::Entry::Variables,
description: 'Environment variables available for this job.', description: 'Environment variables available for this job.',
inherit: false inherit: false
helpers(*ALLOWED_KEYS) helpers :trigger, :needs, :variables
attributes(*ALLOWED_KEYS)
attributes :when, :allow_failure
def self.matching?(name, config) def self.matching?(name, config)
!name.to_s.start_with?('.') && !name.to_s.start_with?('.') &&
...@@ -73,7 +68,7 @@ module Gitlab ...@@ -73,7 +68,7 @@ module Gitlab
trigger: (trigger_value if trigger_defined?), trigger: (trigger_value if trigger_defined?),
needs: (needs_value if needs_defined?), needs: (needs_value if needs_defined?),
ignore: !!allow_failure, ignore: !!allow_failure,
when: when_value, when: self.when,
variables: (variables_value if variables_defined?), variables: (variables_value if variables_defined?),
scheduling_type: needs_defined? && !bridge_needs ? :dag : :stage scheduling_type: needs_defined? && !bridge_needs ? :dag : :stage
).compact ).compact
......
...@@ -131,7 +131,7 @@ module Gitlab ...@@ -131,7 +131,7 @@ module Gitlab
helpers :before_script, :script, :type, :after_script, helpers :before_script, :script, :type, :after_script,
:cache, :image, :services, :variables, :cache, :image, :services, :variables,
:artifacts, :environment, :coverage, :retry, :artifacts, :environment, :coverage, :retry,
:parallel, :needs, :interruptible, :release, :tags :needs, :interruptible, :release, :tags
attributes :script, :tags, :allow_failure, :when, :dependencies, attributes :script, :tags, :allow_failure, :when, :dependencies,
:needs, :retry, :parallel, :start_in, :needs, :retry, :parallel, :start_in,
...@@ -176,12 +176,15 @@ module Gitlab ...@@ -176,12 +176,15 @@ module Gitlab
services: services_value, services: services_value,
cache: cache_value, cache: cache_value,
tags: tags_value, tags: tags_value,
when: self.when,
start_in: self.start_in,
dependencies: dependencies,
variables: variables_defined? ? variables_value : {}, variables: variables_defined? ? variables_value : {},
environment: environment_defined? ? environment_value : nil, environment: environment_defined? ? environment_value : nil,
environment_name: environment_defined? ? environment_value[:name] : nil, environment_name: environment_defined? ? environment_value[:name] : nil,
coverage: coverage_defined? ? coverage_value : nil, coverage: coverage_defined? ? coverage_value : nil,
retry: retry_defined? ? retry_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, interruptible: interruptible_defined? ? interruptible_value : nil,
timeout: has_timeout? ? ChronicDuration.parse(timeout.to_s) : nil, timeout: has_timeout? ? ChronicDuration.parse(timeout.to_s) : nil,
artifacts: artifacts_value, artifacts: artifacts_value,
......
...@@ -54,7 +54,7 @@ module Gitlab ...@@ -54,7 +54,7 @@ module Gitlab
allowed_when: %w[on_success on_failure always never manual delayed].freeze allowed_when: %w[on_success on_failure always never manual delayed].freeze
} }
helpers :stage, :only, :except, :rules, :extends helpers :stage, :only, :except, :rules
attributes :extends, :rules attributes :extends, :rules
end end
...@@ -74,6 +74,8 @@ module Gitlab ...@@ -74,6 +74,8 @@ module Gitlab
@entries.delete(:only) unless only_defined? # rubocop:disable Gitlab/ModuleWithInstanceVariables @entries.delete(:only) unless only_defined? # rubocop:disable Gitlab/ModuleWithInstanceVariables
@entries.delete(:except) unless except_defined? # rubocop:disable Gitlab/ModuleWithInstanceVariables @entries.delete(:except) unless except_defined? # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
yield if block_given?
end end
end end
...@@ -88,8 +90,8 @@ module Gitlab ...@@ -88,8 +90,8 @@ module Gitlab
def value def value
{ name: name, { name: name,
stage: stage_value, stage: stage_value,
extends: extends_value, extends: extends,
rules: (rules_value if has_rules?), rules: rules_value,
only: only_value, only: only_value,
except: except_value }.compact except: except_value }.compact
end end
......
...@@ -67,7 +67,9 @@ module Gitlab ...@@ -67,7 +67,9 @@ module Gitlab
entry :workflow, Entry::Workflow, entry :workflow, Entry::Workflow,
description: 'List of evaluable rules to determine Pipeline status' 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, delegate :before_script_value,
:image_value, :image_value,
......
...@@ -75,6 +75,8 @@ module Gitlab ...@@ -75,6 +75,8 @@ module Gitlab
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def entry(key, entry, description: nil, default: nil, inherit: nil, reserved: nil, metadata: {}) 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) factory = ::Gitlab::Config::Entry::Factory.new(entry)
.with(description: description) .with(description: description)
.with(default: default) .with(default: default)
...@@ -86,10 +88,14 @@ module Gitlab ...@@ -86,10 +88,14 @@ module Gitlab
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def helpers(*nodes) def helpers(*nodes, dynamic: false)
nodes.each do |symbol| nodes.each do |symbol|
if method_defined?("#{symbol}_defined?") || method_defined?("#{symbol}_value") if method_defined?("#{symbol}_defined?") || method_defined?("#{symbol}_value")
raise ArgumentError, "Method #{attribute}_defined? or #{attribute}_value already defined" 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 end
define_method("#{symbol}_defined?") do define_method("#{symbol}_defined?") do
......
...@@ -248,10 +248,6 @@ describe Gitlab::Ci::Config::Entry::Processable do ...@@ -248,10 +248,6 @@ describe Gitlab::Ci::Config::Entry::Processable do
end end
describe '#value' do describe '#value' do
before do
entry.compose!
end
context 'when entry is correct' do context 'when entry is correct' do
let(:config) do let(:config) do
{ stage: 'test' } { stage: 'test' }
......
...@@ -2419,7 +2419,9 @@ module Gitlab ...@@ -2419,7 +2419,9 @@ module Gitlab
it 'returns errors and empty configuration' do it 'returns errors and empty configuration' do
expect(subject.valid?).to eq(false) 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 expect(subject.content).to be_blank
end end
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