diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 913253e4e9225f647ea437f4108b7164b6a52cce..12926bc2379f01d66782f4077cd8627562507b4a 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -607,8 +607,14 @@ def config_processor
rescue Gitlab::Ci::YamlProcessor::ValidationError => e
self.yaml_errors = e.message
nil
- rescue
- self.yaml_errors = 'Undefined error'
+ rescue => ex
+ self.yaml_errors = "Undefined error (#{Labkit::Correlation::CorrelationId.current_id})"
+
+ Gitlab::Sentry.track_acceptable_exception(ex, extra: {
+ project_id: project.id,
+ sha: sha,
+ ci_yaml_file: ci_yaml_file_path
+ })
nil
end
end
diff --git a/ee/lib/ee/gitlab/ci/config/entry/bridge.rb b/ee/lib/ee/gitlab/ci/config/entry/bridge.rb
index 8943f67257c6feeb8a2a38f9a14b8ca1f6a133bc..a505bc9391d6edf0b3d44e74703210ec670a80a1 100644
--- a/ee/lib/ee/gitlab/ci/config/entry/bridge.rb
+++ b/ee/lib/ee/gitlab/ci/config/entry/bridge.rb
@@ -23,27 +23,35 @@ class Bridge < ::Gitlab::Config::Entry::Node
validates :name, presence: true
validates :name, type: Symbol
- validate do
- unless trigger.present? || needs.present?
- errors.add(:config, 'should contain either a trigger or a needs:pipeline')
- end
- end
-
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
end
+
+ validate on: :composed do
+ unless trigger.present? || bridge_needs.present?
+ errors.add(:config, 'should contain either a trigger or a needs:pipeline')
+ end
+ end
+
+ validate on: :composed do
+ next unless bridge_needs.present?
+ next if bridge_needs.one?
+
+ errors.add(:config, 'should contain at most one bridge need')
+ end
end
entry :trigger, ::EE::Gitlab::Ci::Config::Entry::Trigger,
description: 'CI/CD Bridge downstream trigger definition.',
inherit: false
- entry :needs, ::EE::Gitlab::Ci::Config::Entry::Needs,
+ entry :needs, ::Gitlab::Ci::Config::Entry::Needs,
description: 'CI/CD Bridge needs dependency definition.',
- inherit: false
+ inherit: false,
+ metadata: { allowed_needs: %i[bridge job] }
entry :stage, ::Gitlab::Ci::Config::Entry::Stage,
description: 'Pipeline stage this job will be executed into.',
@@ -93,6 +101,10 @@ def value
except: except_value }.compact
end
+ def bridge_needs
+ needs_value[:bridge] if needs_value
+ end
+
private
def overwrite_entry(deps, key, current_entry)
diff --git a/ee/lib/ee/gitlab/ci/config/entry/need.rb b/ee/lib/ee/gitlab/ci/config/entry/need.rb
new file mode 100644
index 0000000000000000000000000000000000000000..1f34412364ff5b3a50e569de498262e8fc915133
--- /dev/null
+++ b/ee/lib/ee/gitlab/ci/config/entry/need.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+module EE
+ module Gitlab
+ module Ci
+ module Config
+ module Entry
+ module Need
+ extend ActiveSupport::Concern
+
+ prepended do
+ strategy :Bridge, class: EE::Gitlab::Ci::Config::Entry::Need::Bridge, if: -> (config) { config.is_a?(Hash) }
+ end
+
+ class Bridge < ::Gitlab::Config::Entry::Node
+ include ::Gitlab::Config::Entry::Validatable
+ include ::Gitlab::Config::Entry::Attributable
+
+ ALLOWED_KEYS = %i[pipeline].freeze
+ attributes :pipeline
+
+ validations do
+ validates :config, presence: true
+ validates :config, allowed_keys: ALLOWED_KEYS
+ validates :pipeline, type: String, presence: true
+ end
+
+ def type
+ :bridge
+ end
+ end
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/ee/lib/ee/gitlab/ci/config/entry/needs.rb b/ee/lib/ee/gitlab/ci/config/entry/needs.rb
deleted file mode 100644
index f27492465cf90014b26b6fac63a7d6cc77c2db8c..0000000000000000000000000000000000000000
--- a/ee/lib/ee/gitlab/ci/config/entry/needs.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-# frozen_string_literal: true
-
-module EE
- module Gitlab
- module Ci
- module Config
- module Entry
- ##
- # Entry that represents a cross-project needs dependency.
- #
- class Needs < ::Gitlab::Config::Entry::Node
- include ::Gitlab::Config::Entry::Validatable
- include ::Gitlab::Config::Entry::Attributable
-
- ALLOWED_KEYS = %i[pipeline].freeze
- attributes :pipeline
-
- validations do
- validates :config, presence: true
- validates :config, allowed_keys: ALLOWED_KEYS
- validates :pipeline, type: String, presence: true
- end
- end
- end
- end
- end
- end
-end
diff --git a/ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb b/ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb
index 848fec54b86b05abc3a565c925d220cea47b64d6..a4404a6b1c992dfe5bd35b0bc174e973a1670447 100644
--- a/ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb
+++ b/ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb
@@ -88,7 +88,7 @@
describe '#value' do
it 'is returns a bridge job configuration' do
expect(subject.value).to eq(name: :my_bridge,
- needs: { pipeline: 'some/project' },
+ needs: { bridge: [{ pipeline: 'some/project' }] },
ignore: false,
stage: 'test',
only: { refs: %w[branches tags] })
@@ -160,6 +160,50 @@
end
end
+ context 'when bridge has only job needs' do
+ let(:config) do
+ {
+ needs: ['some_job']
+ }
+ end
+
+ describe '#valid?' do
+ it { is_expected.not_to be_valid }
+ end
+ end
+
+ context 'when bridge has bridge and job needs' do
+ let(:config) do
+ {
+ trigger: 'other-project',
+ needs: ['some_job', { pipeline: 'some/other_project' }]
+ }
+ end
+
+ describe '#valid?' do
+ it { is_expected.to be_valid }
+ end
+ end
+
+ context 'when bridge has more than one valid bridge needs' do
+ let(:config) do
+ {
+ trigger: 'other-project',
+ needs: [{ pipeline: 'some/project' }, { pipeline: 'some/other_project' }]
+ }
+ end
+
+ describe '#valid?' do
+ it { is_expected.not_to be_valid }
+ end
+
+ describe '#errors' do
+ it 'returns an error about too many bridge needs' do
+ expect(subject.errors).to contain_exactly('bridge config should contain at most one bridge need')
+ end
+ end
+ end
+
context 'when bridge config contains unknown keys' do
let(:config) { { unknown: 123 } }
diff --git a/ee/spec/lib/ee/gitlab/ci/config/entry/need_spec.rb b/ee/spec/lib/ee/gitlab/ci/config/entry/need_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..2883851ce33e6e0cf1ff84589c50d958c78cd26b
--- /dev/null
+++ b/ee/spec/lib/ee/gitlab/ci/config/entry/need_spec.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::Gitlab::Ci::Config::Entry::Need do
+ subject { described_class.new(config) }
+
+ context 'when upstream is specified' do
+ let(:config) { { pipeline: 'some/project' } }
+
+ describe '#valid?' do
+ it { is_expected.to be_valid }
+ end
+
+ describe '#value' do
+ it 'returns job needs configuration' do
+ expect(subject.value).to eq(pipeline: 'some/project')
+ end
+ end
+ end
+
+ context 'when need is empty' do
+ let(:config) { {} }
+
+ describe '#valid?' do
+ it { is_expected.not_to be_valid }
+ end
+
+ describe '#errors' do
+ it 'is returns an error about an empty config' do
+ expect(subject.errors)
+ .to include("bridge config can't be blank")
+ end
+ end
+ end
+end
diff --git a/ee/spec/lib/ee/gitlab/ci/config/entry/needs_spec.rb b/ee/spec/lib/ee/gitlab/ci/config/entry/needs_spec.rb
index f0c5cddb811d11b0efbea2b5e5c82703f2101517..2df14ae0b4cb12e545255c5ef3a8df313968d104 100644
--- a/ee/spec/lib/ee/gitlab/ci/config/entry/needs_spec.rb
+++ b/ee/spec/lib/ee/gitlab/ci/config/entry/needs_spec.rb
@@ -1,52 +1,77 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
-require_dependency 'active_model'
+require 'spec_helper'
-describe EE::Gitlab::Ci::Config::Entry::Needs do
- subject { described_class.new(config) }
+describe ::Gitlab::Ci::Config::Entry::Needs do
+ subject(:needs) { described_class.new(config) }
- context 'when upstream config is a non-empty string' do
- let(:config) { { pipeline: 'some/project' } }
+ before do
+ needs.metadata[:allowed_needs] = %i[job bridge]
+ end
- describe '#valid?' do
- it { is_expected.to be_valid }
+ describe 'validations' do
+ before do
+ needs.compose!
end
- describe '#value' do
- it 'is returns a upstream configuration hash' do
- expect(subject.value).to eq(pipeline: 'some/project')
+ context 'when entry config value is correct' do
+ let(:config) { ['job_name', pipeline: 'some/project'] }
+
+ describe '#valid?' do
+ it { is_expected.to be_valid }
end
end
- end
- context 'when upstream config an empty string' do
- let(:config) { '' }
+ context 'when wrong needs type is used' do
+ let(:config) { ['job_name', { pipeline: 'some/project' }, 123] }
- describe '#valid?' do
- it { is_expected.not_to be_valid }
- end
+ describe '#valid?' do
+ it { is_expected.not_to be_valid }
+ end
- describe '#errors' do
- it 'is returns an error about an empty config' do
- expect(subject.errors.first)
- .to eq("needs config can't be blank")
+ describe '#errors' do
+ it 'returns error about incorrect type' do
+ expect(needs.errors).to contain_exactly(
+ 'need has an unsupported type')
+ end
end
end
- end
- context 'when upstream configuration is not valid' do
- context 'when branch is not provided' do
- let(:config) { { pipeline: 123 } }
+ context 'when bridge needs has wrong attributes' do
+ let(:config) { ['job_name', project: 'some/project'] }
describe '#valid?' do
it { is_expected.not_to be_valid }
end
+ end
+ end
- describe '#errors' do
- it 'returns an error message' do
- expect(subject.errors.first)
- .to eq('needs pipeline should be a string')
+ describe '.compose!' do
+ context 'when valid job entries composed' do
+ let(:config) { ['first_job_name', pipeline: 'some/project'] }
+
+ before do
+ needs.compose!
+ end
+
+ it 'is valid' do
+ expect(needs).to be_valid
+ end
+
+ describe '#value' do
+ it 'returns key value' do
+ expect(needs.value).to eq(
+ job: [{ name: 'first_job_name' }],
+ bridge: [{ pipeline: 'some/project' }]
+ )
+ end
+ end
+
+ describe '#descendants' do
+ it 'creates valid descendant nodes' do
+ expect(needs.descendants.count).to eq 2
+ expect(needs.descendants)
+ .to all(be_an_instance_of(::Gitlab::Ci::Config::Entry::Need))
end
end
end
diff --git a/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb b/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..9502fb9b5214b3e4229cc69cf0972e90c97fd499
--- /dev/null
+++ b/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Entry::Job do
+ let(:entry) { described_class.new(config, name: :rspec) }
+
+ describe 'validations' do
+ before do
+ entry.compose!
+ end
+
+ context 'when entry value is not correct' do
+ context 'when has needs' do
+ context 'when needs is bridge type' do
+ let(:config) do
+ {
+ script: 'echo',
+ stage: 'test',
+ needs: { pipeline: 'some/project' }
+ }
+ end
+
+ it 'returns error about invalid needs type' do
+ expect(entry).not_to be_valid
+ expect(entry.errors).to contain_exactly('needs config uses invalid types: bridge')
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb b/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..e782c762daacd735fef3b6ffa39296e6e4a7e537
--- /dev/null
+++ b/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Ci::YamlProcessor do
+ describe 'Bridge Needs' do
+ let(:config) do
+ {
+ build: { stage: 'build', script: 'test' },
+ bridge: { stage: 'test', needs: needs }
+ }
+ end
+
+ subject { described_class.new(YAML.dump(config)) }
+
+ context 'needs upstream pipeline' do
+ let(:needs) { { pipeline: 'some/project' } }
+
+ it 'creates jobs with valid specification' do
+ expect(subject.builds.size).to eq(2)
+ expect(subject.builds[0]).to eq(
+ stage: "build",
+ stage_idx: 1,
+ name: "build",
+ options: {
+ script: ["test"]
+ },
+ when: "on_success",
+ allow_failure: false,
+ yaml_variables: []
+ )
+ expect(subject.builds[1]).to eq(
+ stage: "test",
+ stage_idx: 2,
+ name: "bridge",
+ options: {
+ bridge_needs: { pipeline: 'some/project' }
+ },
+ when: "on_success",
+ allow_failure: false,
+ yaml_variables: []
+ )
+ end
+ end
+
+ context 'needs both job and pipeline' do
+ let(:needs) { ['build', { pipeline: 'some/project' }] }
+
+ it 'creates jobs with valid specification' do
+ expect(subject.builds.size).to eq(2)
+ expect(subject.builds[0]).to eq(
+ stage: "build",
+ stage_idx: 1,
+ name: "build",
+ options: {
+ script: ["test"]
+ },
+ when: "on_success",
+ allow_failure: false,
+ yaml_variables: []
+ )
+ expect(subject.builds[1]).to eq(
+ stage: "test",
+ stage_idx: 2,
+ name: "bridge",
+ options: {
+ bridge_needs: { pipeline: 'some/project' }
+ },
+ needs_attributes: [
+ { name: "build" }
+ ],
+ when: "on_success",
+ allow_failure: false,
+ yaml_variables: []
+ )
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index 1298e2d34628de9b2ac633eac4636661d1763efd..2d5981a42559b8cefc08562663b0b11108fde4b5 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -50,7 +50,6 @@ class Job < ::Gitlab::Config::Entry::Node
validates :timeout, duration: { limit: ChronicDuration.output(Project::MAX_BUILD_TIMEOUT) }
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
@@ -114,6 +113,11 @@ class Job < ::Gitlab::Config::Entry::Node
description: 'List of evaluable Rules to determine job inclusion.',
inherit: false
+ entry :needs, Entry::Needs,
+ description: 'Needs configuration for this job.',
+ metadata: { allowed_needs: %i[job] },
+ inherit: false
+
entry :variables, Entry::Variables,
description: 'Environment variables available for this job.',
inherit: false
diff --git a/lib/gitlab/ci/config/entry/need.rb b/lib/gitlab/ci/config/entry/need.rb
new file mode 100644
index 0000000000000000000000000000000000000000..b6db546d8ff42ec45b728b2b908e212dd9d3979f
--- /dev/null
+++ b/lib/gitlab/ci/config/entry/need.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Ci
+ class Config
+ module Entry
+ class Need < ::Gitlab::Config::Entry::Simplifiable
+ strategy :Job, if: -> (config) { config.is_a?(String) }
+
+ class Job < ::Gitlab::Config::Entry::Node
+ include ::Gitlab::Config::Entry::Validatable
+
+ validations do
+ validates :config, presence: true
+ validates :config, type: String
+ end
+
+ def type
+ :job
+ end
+
+ def value
+ { name: @config }
+ end
+ end
+
+ class UnknownStrategy < ::Gitlab::Config::Entry::Node
+ def type
+ end
+
+ def value
+ end
+
+ def errors
+ ["#{location} has an unsupported type"]
+ end
+ end
+ end
+ end
+ end
+ end
+end
+
+::Gitlab::Ci::Config::Entry::Need.prepend_if_ee('::EE::Gitlab::Ci::Config::Entry::Need')
diff --git a/lib/gitlab/ci/config/entry/needs.rb b/lib/gitlab/ci/config/entry/needs.rb
new file mode 100644
index 0000000000000000000000000000000000000000..28452aaaa16b48e0d9d75a773dba7a713b0681df
--- /dev/null
+++ b/lib/gitlab/ci/config/entry/needs.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Ci
+ class Config
+ module Entry
+ ##
+ # Entry that represents a set of needs dependencies.
+ #
+ class Needs < ::Gitlab::Config::Entry::Node
+ include ::Gitlab::Config::Entry::Validatable
+
+ validations do
+ validates :config, presence: true
+
+ validate do
+ unless config.is_a?(Hash) || config.is_a?(Array)
+ errors.add(:config, 'can only be a Hash or an Array')
+ end
+ end
+
+ validate on: :composed do
+ extra_keys = value.keys - opt(:allowed_needs)
+ if extra_keys.any?
+ errors.add(:config, "uses invalid types: #{extra_keys.join(', ')}")
+ end
+ end
+ end
+
+ def compose!(deps = nil)
+ super(deps) do
+ [@config].flatten.each_with_index do |need, index|
+ @entries[index] = ::Gitlab::Config::Entry::Factory.new(Entry::Need)
+ .value(need)
+ .with(key: "need", parent: self, description: "need definition.") # rubocop:disable CodeReuse/ActiveRecord
+ .create!
+ end
+
+ @entries.each_value do |entry|
+ entry.compose!(deps)
+ end
+ end
+ end
+
+ def value
+ values = @entries.values.select(&:type)
+ values.group_by(&:type).transform_values do |values|
+ values.map(&:value)
+ end
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/config/normalizer.rb b/lib/gitlab/ci/config/normalizer.rb
index 09f9bf5f69f7bc8b41d74e1e242b59994de3f585..e714ef225f5938c36c839e98dd63dc196cdba077 100644
--- a/lib/gitlab/ci/config/normalizer.rb
+++ b/lib/gitlab/ci/config/normalizer.rb
@@ -18,8 +18,8 @@ def normalize_jobs
config[:dependencies] = expand_names(config[:dependencies])
end
- if config[:needs]
- config[:needs] = expand_names(config[:needs])
+ if job_needs = config.dig(:needs, :job)
+ config[:needs][:job] = expand_needs(job_needs)
end
config
@@ -36,6 +36,22 @@ def expand_names(job_names)
end
end
+ def expand_needs(job_needs)
+ return unless job_needs
+
+ job_needs.flat_map do |job_need|
+ job_need_name = job_need[:name].to_sym
+
+ if all_job_names = parallelized_jobs[job_need_name]
+ all_job_names.map do |job_name|
+ { name: job_name }
+ end
+ else
+ job_need
+ end
+ end
+ end
+
def parallelized_jobs
strong_memoize(:parallelized_jobs) do
@jobs_config.each_with_object({}) do |(job_name, config), hash|
diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb
index f6a3abefcfb350d9ff69560a579b3c9473612a8b..c2a55fa8b1b444c494728b74d52f1e0fc0c2424c 100644
--- a/lib/gitlab/ci/yaml_processor.rb
+++ b/lib/gitlab/ci/yaml_processor.rb
@@ -40,7 +40,7 @@ def build_attributes(name)
environment: job[:environment_name],
coverage_regex: job[:coverage],
yaml_variables: yaml_variables(name),
- needs_attributes: job[:needs]&.map { |need| { name: need } },
+ needs_attributes: job.dig(:needs, :job),
interruptible: job[:interruptible],
rules: job[:rules],
options: {
@@ -59,7 +59,7 @@ def build_attributes(name)
instance: job[:instance],
start_in: job[:start_in],
trigger: job[:trigger],
- bridge_needs: job[:needs]
+ bridge_needs: job.dig(:needs, :bridge)&.first
}.compact }.compact
end
@@ -159,17 +159,19 @@ def validate_job_dependencies!(name, job)
end
def validate_job_needs!(name, job)
- return unless job[:needs]
+ return unless job.dig(:needs, :job)
stage_index = @stages.index(job[:stage])
- job[:needs].each do |need|
- raise ValidationError, "#{name} job: undefined need: #{need}" unless @jobs[need.to_sym]
+ job.dig(:needs, :job).each do |need|
+ need_job_name = need[:name]
- needs_stage_index = @stages.index(@jobs[need.to_sym][:stage])
+ raise ValidationError, "#{name} job: undefined need: #{need_job_name}" unless @jobs[need_job_name.to_sym]
+
+ needs_stage_index = @stages.index(@jobs[need_job_name.to_sym][:stage])
unless needs_stage_index.present? && needs_stage_index < stage_index
- raise ValidationError, "#{name} job: need #{need} is not defined in prior stages"
+ raise ValidationError, "#{name} job: need #{need_job_name} is not defined in prior stages"
end
end
end
diff --git a/lib/gitlab/config/entry/configurable.rb b/lib/gitlab/config/entry/configurable.rb
index b7ec4b7c4f8203f0fdde0213c7f1057d880de458..bda84dc2cff1f8226e85b7f871f8729610bb6334 100644
--- a/lib/gitlab/config/entry/configurable.rb
+++ b/lib/gitlab/config/entry/configurable.rb
@@ -29,22 +29,24 @@ module Configurable
def compose!(deps = nil)
return unless valid?
- self.class.nodes.each do |key, factory|
- # If we override the config type validation
- # we can end with different config types like String
- next unless config.is_a?(Hash)
+ super do
+ self.class.nodes.each do |key, factory|
+ # If we override the config type validation
+ # we can end with different config types like String
+ next unless config.is_a?(Hash)
- factory
- .value(config[key])
- .with(key: key, parent: self)
+ factory
+ .value(config[key])
+ .with(key: key, parent: self)
- entries[key] = factory.create!
- end
+ entries[key] = factory.create!
+ end
- yield if block_given?
+ yield if block_given?
- entries.each_value do |entry|
- entry.compose!(deps)
+ entries.each_value do |entry|
+ entry.compose!(deps)
+ end
end
end
# rubocop: enable CodeReuse/ActiveRecord
@@ -67,12 +69,13 @@ def reserved_node_names
private
# rubocop: disable CodeReuse/ActiveRecord
- def entry(key, entry, description: nil, default: nil, inherit: nil, reserved: nil)
+ def entry(key, entry, description: nil, default: nil, inherit: nil, reserved: nil, metadata: {})
factory = ::Gitlab::Config::Entry::Factory.new(entry)
.with(description: description)
.with(default: default)
.with(inherit: inherit)
.with(reserved: reserved)
+ .metadata(metadata)
(@nodes ||= {}).merge!(key.to_sym => factory)
end
diff --git a/lib/gitlab/config/entry/node.rb b/lib/gitlab/config/entry/node.rb
index e014f15fbd8c305aca0017d21906dab20600fc57..84d3409ed91dcb0678a578599d351753d145b66e 100644
--- a/lib/gitlab/config/entry/node.rb
+++ b/lib/gitlab/config/entry/node.rb
@@ -112,6 +112,10 @@ def self.aspects
@aspects ||= []
end
+ def self.with_aspect(blk)
+ self.aspects.append(blk)
+ end
+
private
attr_reader :entries
diff --git a/lib/gitlab/config/entry/simplifiable.rb b/lib/gitlab/config/entry/simplifiable.rb
index d58aba07d15f82a020b39b7ff34944c0259ea03b..315f1947e2c9eef1069928e2fe5cdade8634bb73 100644
--- a/lib/gitlab/config/entry/simplifiable.rb
+++ b/lib/gitlab/config/entry/simplifiable.rb
@@ -4,11 +4,11 @@ module Gitlab
module Config
module Entry
class Simplifiable < SimpleDelegator
- EntryStrategy = Struct.new(:name, :condition)
+ EntryStrategy = Struct.new(:name, :klass, :condition)
attr_reader :subject
- def initialize(config, **metadata)
+ def initialize(config, **metadata, &blk)
unless self.class.const_defined?(:UnknownStrategy)
raise ArgumentError, 'UndefinedStrategy not available!'
end
@@ -19,14 +19,13 @@ def initialize(config, **metadata)
entry = self.class.entry_class(strategy)
- @subject = entry.new(config, metadata)
+ @subject = entry.new(config, metadata, &blk)
- yield(@subject) if block_given?
super(@subject)
end
def self.strategy(name, **opts)
- EntryStrategy.new(name, opts.fetch(:if)).tap do |strategy|
+ EntryStrategy.new(name, opts.dig(:class), opts.fetch(:if)).tap do |strategy|
strategies.append(strategy)
end
end
@@ -37,7 +36,7 @@ def self.strategies
def self.entry_class(strategy)
if strategy.present?
- self.const_get(strategy.name, false)
+ strategy.klass || self.const_get(strategy.name, false)
else
self::UnknownStrategy
end
diff --git a/lib/gitlab/config/entry/validatable.rb b/lib/gitlab/config/entry/validatable.rb
index 1c88c68c11cd71904eaec929e0d4f4b546d9b457..45b852dc2e0f29a55f0c09230e8154a69f916e23 100644
--- a/lib/gitlab/config/entry/validatable.rb
+++ b/lib/gitlab/config/entry/validatable.rb
@@ -7,14 +7,27 @@ module Validatable
extend ActiveSupport::Concern
def self.included(node)
- node.aspects.append -> do
- @validator = self.class.validator.new(self)
- @validator.validate(:new)
+ node.with_aspect -> do
+ validate(:new)
end
end
+ def validator
+ @validator ||= self.class.validator.new(self)
+ end
+
+ def validate(context = nil)
+ validator.validate(context)
+ end
+
+ def compose!(deps = nil, &blk)
+ super(deps, &blk)
+
+ validate(:composed)
+ end
+
def errors
- @validator.messages + descendants.flat_map(&:errors) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+ validator.messages + descendants.flat_map(&:errors)
end
class_methods do
diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb
index d3eb5a9663fd90e7a4fe53779e287f03eb7c5229..9fe18caf6898b03d7ef74da1cd05858e0c5dfe09 100644
--- a/spec/lib/gitlab/ci/config/entry/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb
@@ -23,7 +23,7 @@
let(:result) do
%i[before_script script stage type after_script cache
- image services only except rules variables artifacts
+ image services only except rules needs variables artifacts
environment coverage retry]
end
@@ -384,21 +384,6 @@
end
context 'when has needs' do
- context 'that are not a array of strings' do
- let(:config) do
- {
- stage: 'test',
- script: 'echo',
- needs: 'build-job'
- }
- end
-
- it 'returns error about invalid type' do
- expect(entry).not_to be_valid
- expect(entry.errors).to include 'job needs should be an array of strings'
- end
- end
-
context 'when have dependencies that are not subset of needs' do
let(:config) do
{
diff --git a/spec/lib/gitlab/ci/config/entry/need_spec.rb b/spec/lib/gitlab/ci/config/entry/need_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..d119e60490085b92115ab94736e56779b7ab0669
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/entry/need_spec.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::Gitlab::Ci::Config::Entry::Need do
+ subject(:need) { described_class.new(config) }
+
+ context 'when job is specified' do
+ let(:config) { 'job_name' }
+
+ describe '#valid?' do
+ it { is_expected.to be_valid }
+ end
+
+ describe '#value' do
+ it 'returns job needs configuration' do
+ expect(need.value).to eq(name: 'job_name')
+ end
+ end
+ end
+
+ context 'when need is empty' do
+ let(:config) { '' }
+
+ describe '#valid?' do
+ it { is_expected.not_to be_valid }
+ end
+
+ describe '#errors' do
+ it 'is returns an error about an empty config' do
+ expect(need.errors)
+ .to contain_exactly("job config can't be blank")
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/ci/config/entry/needs_spec.rb b/spec/lib/gitlab/ci/config/entry/needs_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..f4a76b52d30c0e2c2f37dc3f569b95a94ea582b2
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/entry/needs_spec.rb
@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::Gitlab::Ci::Config::Entry::Needs do
+ subject(:needs) { described_class.new(config) }
+
+ before do
+ needs.metadata[:allowed_needs] = %i[job]
+ end
+
+ describe 'validations' do
+ before do
+ needs.compose!
+ end
+
+ context 'when entry config value is correct' do
+ let(:config) { ['job_name'] }
+
+ describe '#valid?' do
+ it { is_expected.to be_valid }
+ end
+ end
+
+ context 'when config value has wrong type' do
+ let(:config) { 123 }
+
+ describe '#valid?' do
+ it { is_expected.not_to be_valid }
+ end
+
+ describe '#errors' do
+ it 'returns error about incorrect type' do
+ expect(needs.errors)
+ .to include('needs config can only be a hash or an array')
+ end
+ end
+ end
+
+ context 'when wrong needs type is used' do
+ let(:config) { [123] }
+
+ describe '#valid?' do
+ it { is_expected.not_to be_valid }
+ end
+
+ describe '#errors' do
+ it 'returns error about incorrect type' do
+ expect(needs.errors).to contain_exactly(
+ 'need has an unsupported type')
+ end
+ end
+ end
+ end
+
+ describe '.compose!' do
+ context 'when valid job entries composed' do
+ let(:config) { %w[first_job_name second_job_name] }
+
+ before do
+ needs.compose!
+ end
+
+ describe '#value' do
+ it 'returns key value' do
+ expect(needs.value).to eq(
+ job: [
+ { name: 'first_job_name' },
+ { name: 'second_job_name' }
+ ]
+ )
+ end
+ end
+
+ describe '#descendants' do
+ it 'creates valid descendant nodes' do
+ expect(needs.descendants.count).to eq 2
+ expect(needs.descendants)
+ .to all(be_an_instance_of(::Gitlab::Ci::Config::Entry::Need))
+ end
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/ci/config/normalizer_spec.rb b/spec/lib/gitlab/ci/config/normalizer_spec.rb
index 6b766cc37bfcb5a50bee4621371637aab654c590..bf88047838755a37ecd16f84d1b4c577a0979893 100644
--- a/spec/lib/gitlab/ci/config/normalizer_spec.rb
+++ b/spec/lib/gitlab/ci/config/normalizer_spec.rb
@@ -7,6 +7,16 @@
let(:job_config) { { script: 'rspec', parallel: 5, name: 'rspec' } }
let(:config) { { job_name => job_config } }
+ let(:expanded_job_names) do
+ [
+ "rspec 1/5",
+ "rspec 2/5",
+ "rspec 3/5",
+ "rspec 4/5",
+ "rspec 5/5"
+ ]
+ end
+
describe '.normalize_jobs' do
subject { described_class.new(config).normalize_jobs }
@@ -15,9 +25,7 @@
end
it 'has parallelized jobs' do
- job_names = [:"rspec 1/5", :"rspec 2/5", :"rspec 3/5", :"rspec 4/5", :"rspec 5/5"]
-
- is_expected.to include(*job_names)
+ is_expected.to include(*expanded_job_names.map(&:to_sym))
end
it 'sets job instance in options' do
@@ -43,49 +51,109 @@
let(:job_name) { :"rspec 35/2" }
it 'properly parallelizes job names' do
- job_names = [:"rspec 35/2 1/5", :"rspec 35/2 2/5", :"rspec 35/2 3/5", :"rspec 35/2 4/5", :"rspec 35/2 5/5"]
+ job_names = [
+ :"rspec 35/2 1/5",
+ :"rspec 35/2 2/5",
+ :"rspec 35/2 3/5",
+ :"rspec 35/2 4/5",
+ :"rspec 35/2 5/5"
+ ]
is_expected.to include(*job_names)
end
end
- %i[dependencies needs].each do |context|
- context "when job has #{context} on parallelized jobs" do
+ context 'for dependencies' do
+ context "when job has dependencies on parallelized jobs" do
let(:config) do
{
job_name => job_config,
- other_job: { script: 'echo 1', context => [job_name.to_s] }
+ other_job: { script: 'echo 1', dependencies: [job_name.to_s] }
}
end
- it "parallelizes #{context}" do
- job_names = ["rspec 1/5", "rspec 2/5", "rspec 3/5", "rspec 4/5", "rspec 5/5"]
-
- expect(subject[:other_job][context]).to include(*job_names)
+ it "parallelizes dependencies" do
+ expect(subject[:other_job][:dependencies]).to eq(expanded_job_names)
end
it "does not include original job name in #{context}" do
- expect(subject[:other_job][context]).not_to include(job_name)
+ expect(subject[:other_job][:dependencies]).not_to include(job_name)
end
end
- context "when there are #{context} which are both parallelized and not" do
+ context "when there are dependencies which are both parallelized and not" do
let(:config) do
{
job_name => job_config,
other_job: { script: 'echo 1' },
- final_job: { script: 'echo 1', context => [job_name.to_s, "other_job"] }
+ final_job: { script: 'echo 1', dependencies: [job_name.to_s, "other_job"] }
}
end
- it "parallelizes #{context}" do
+ it "parallelizes dependencies" do
job_names = ["rspec 1/5", "rspec 2/5", "rspec 3/5", "rspec 4/5", "rspec 5/5"]
- expect(subject[:final_job][context]).to include(*job_names)
+ expect(subject[:final_job][:dependencies]).to include(*job_names)
+ end
+
+ it "includes the regular job in dependencies" do
+ expect(subject[:final_job][:dependencies]).to include('other_job')
+ end
+ end
+ end
+
+ context 'for needs' do
+ let(:expanded_job_attributes) do
+ expanded_job_names.map do |job_name|
+ { name: job_name }
+ end
+ end
+
+ context "when job has needs on parallelized jobs" do
+ let(:config) do
+ {
+ job_name => job_config,
+ other_job: {
+ script: 'echo 1',
+ needs: {
+ job: [
+ { name: job_name.to_s }
+ ]
+ }
+ }
+ }
+ end
+
+ it "parallelizes needs" do
+ expect(subject.dig(:other_job, :needs, :job)).to eq(expanded_job_attributes)
+ end
+ end
+
+ context "when there are dependencies which are both parallelized and not" do
+ let(:config) do
+ {
+ job_name => job_config,
+ other_job: {
+ script: 'echo 1'
+ },
+ final_job: {
+ script: 'echo 1',
+ needs: {
+ job: [
+ { name: job_name.to_s },
+ { name: "other_job" }
+ ]
+ }
+ }
+ }
+ end
+
+ it "parallelizes dependencies" do
+ expect(subject.dig(:final_job, :needs, :job)).to include(*expanded_job_attributes)
end
- it "includes the regular job in #{context}" do
- expect(subject[:final_job][context]).to include('other_job')
+ it "includes the regular job in dependencies" do
+ expect(subject.dig(:final_job, :needs, :job)).to include(name: 'other_job')
end
end
end
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index c747ea670bbf23715b9f2dad5255ef4b5f91ad3b..dc7bbc519ee6cd0323969398c1076de491915c66 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -1293,7 +1293,7 @@ module Ci
end
end
- describe "Needs" do
+ describe "Job Needs" do
let(:needs) { }
let(:dependencies) { }
@@ -1301,6 +1301,7 @@ module Ci
{
build1: { stage: 'build', script: 'test' },
build2: { stage: 'build', script: 'test' },
+ parallel: { stage: 'build', script: 'test', parallel: 2 },
test1: { stage: 'test', script: 'test', needs: needs, dependencies: dependencies },
test2: { stage: 'test', script: 'test' },
deploy: { stage: 'test', script: 'test' }
@@ -1317,7 +1318,7 @@ module Ci
let(:needs) { %w(build1 build2) }
it "does create jobs with valid specification" do
- expect(subject.builds.size).to eq(5)
+ expect(subject.builds.size).to eq(7)
expect(subject.builds[0]).to eq(
stage: "build",
stage_idx: 1,
@@ -1329,16 +1330,11 @@ module Ci
allow_failure: false,
yaml_variables: []
)
- expect(subject.builds[2]).to eq(
+ expect(subject.builds[4]).to eq(
stage: "test",
stage_idx: 2,
name: "test1",
- options: {
- script: ["test"],
- # This does not make sense, there is a follow-up:
- # https://gitlab.com/gitlab-org/gitlab-foss/issues/65569
- bridge_needs: %w[build1 build2]
- },
+ options: { script: ["test"] },
needs_attributes: [
{ name: "build1" },
{ name: "build2" }
@@ -1350,10 +1346,25 @@ module Ci
end
end
- context 'needs two builds defined as symbols' do
- let(:needs) { [:build1, :build2] }
+ context 'needs parallel job' do
+ let(:needs) { %w(parallel) }
- it { expect { subject }.not_to raise_error }
+ it "does create jobs with valid specification" do
+ expect(subject.builds.size).to eq(7)
+ expect(subject.builds[4]).to eq(
+ stage: "test",
+ stage_idx: 2,
+ name: "test1",
+ options: { script: ["test"] },
+ needs_attributes: [
+ { name: "parallel 1/2" },
+ { name: "parallel 2/2" }
+ ],
+ when: "on_success",
+ allow_failure: false,
+ yaml_variables: []
+ )
+ end
end
context 'undefined need' do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 5e5a94f8cda710c3f857a6e1f65f232b78054462..9295bb993ceb7df19a48447a01e787b99f165926 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -2893,6 +2893,25 @@ def create_build(name, stage_idx)
it 'contains yaml errors' do
expect(pipeline).to have_yaml_errors
+ expect(pipeline.yaml_errors).to include('contains unknown keys')
+ end
+ end
+
+ context 'when pipeline has undefined error' do
+ let(:pipeline) do
+ create(:ci_pipeline, config: {})
+ end
+
+ it 'contains yaml errors' do
+ expect(::Gitlab::Ci::YamlProcessor).to receive(:new)
+ .and_raise(RuntimeError, 'undefined failure')
+
+ expect(Gitlab::Sentry).to receive(:track_acceptable_exception)
+ .with(be_a(RuntimeError), anything)
+ .and_call_original
+
+ expect(pipeline).to have_yaml_errors
+ expect(pipeline.yaml_errors).to include('Undefined error')
end
end