Skip to content

G
gitlab-ce
Commit 04291872 authored by Marin Jankovski's avatar Marin Jankovski
Browse files
Options

Revert "Merge branch 'mc/backstage/needs-canonical-form' into 'master'" 

This reverts merge request !17539
parent 91e1e5cb
Hide whitespace changes
Inline Side-by-side
Showing with 126 additions and 581 deletions
ee/lib/ee/gitlab/ci/config/entry/bridge.rb
View file @ 04291872
......@@ -23,35 +23,27 @@ module EE
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, ::Gitlab::Ci::Config::Entry::Needs,
entry :needs, ::EE::Gitlab::Ci::Config::Entry::Needs,
description: 'CI/CD Bridge needs dependency definition.',
inherit: false,
metadata: { allowed_needs: %i[bridge job] }
inherit: false
entry :stage, ::Gitlab::Ci::Config::Entry::Stage,
description: 'Pipeline stage this job will be executed into.',
......@@ -101,10 +93,6 @@ module EE
except: except_value }.compact
end
def bridge_needs
needs_value[:bridge] if needs_value
end
private
def overwrite_entry(deps, key, current_entry)
......
ee/lib/ee/gitlab/ci/config/entry/need.rb deleted 100644 → 0
View file @ 91e1e5cb
# 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
ee/lib/ee/gitlab/ci/config/entry/needs.rb 0 → 100644
View file @ 04291872
# 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
ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb
View file @ 04291872
......@@ -88,7 +88,7 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
describe '#value' do
it 'is returns a bridge job configuration' do
expect(subject.value).to eq(name: :my_bridge,
needs: { bridge: [{ pipeline: 'some/project' }] },
needs: { pipeline: 'some/project' },
ignore: false,
stage: 'test',
only: { refs: %w[branches tags] })
......@@ -160,50 +160,6 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
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 } }
......
ee/spec/lib/ee/gitlab/ci/config/entry/need_spec.rb deleted 100644 → 0
View file @ 91e1e5cb
# 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
ee/spec/lib/ee/gitlab/ci/config/entry/needs_spec.rb
View file @ 04291872
# frozen_string_literal: true
require 'spec_helper'
require 'fast_spec_helper'
require_dependency 'active_model'
describe ::Gitlab::Ci::Config::Entry::Needs do
subject(:needs) { described_class.new(config) }
describe EE::Gitlab::Ci::Config::Entry::Needs do
subject { described_class.new(config) }
before do
needs.metadata[:allowed_needs] = %i[job bridge]
end
context 'when upstream config is a non-empty string' do
let(:config) { { pipeline: 'some/project' } }
describe 'validations' do
before do
needs.compose!
describe '#valid?' do
it { is_expected.to be_valid }
end
context 'when entry config value is correct' do
let(:config) { ['job_name', pipeline: 'some/project'] }
describe '#valid?' do
it { is_expected.to be_valid }
describe '#value' do
it 'is returns a upstream configuration hash' do
expect(subject.value).to eq(pipeline: 'some/project')
end
end
end
context 'when wrong needs type is used' do
let(:config) { ['job_name', { pipeline: 'some/project' }, 123] }
context 'when upstream config an empty string' do
let(:config) { '' }
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
describe '#valid?' do
it { is_expected.not_to be_valid }
end
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 }
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")
end
end
end
describe '.compose!' do
context 'when valid job entries composed' do
let(:config) { ['first_job_name', pipeline: 'some/project'] }
context 'when upstream configuration is not valid' do
context 'when branch is not provided' do
let(:config) { { pipeline: 123 } }
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
describe '#valid?' do
it { is_expected.not_to be_valid }
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))
describe '#errors' do
it 'returns an error message' do
expect(subject.errors.first)
.to eq('needs pipeline should be a string')
end
end
end
......
ee/spec/lib/gitlab/ci/config/entry/job_spec.rb deleted 100644 → 0
View file @ 91e1e5cb
# 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
ee/spec/lib/gitlab/ci/yaml_processor_spec.rb deleted 100644 → 0
View file @ 91e1e5cb
# 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
lib/gitlab/ci/config/entry/job.rb
View file @ 04291872
......@@ -50,6 +50,7 @@ module Gitlab
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
......@@ -113,11 +114,6 @@ module Gitlab
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
......
lib/gitlab/ci/config/entry/need.rb deleted 100644 → 0
View file @ 91e1e5cb
# 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')
lib/gitlab/ci/config/entry/needs.rb deleted 100644 → 0
View file @ 91e1e5cb
# 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
lib/gitlab/ci/yaml_processor.rb
View file @ 04291872
......@@ -40,7 +40,7 @@ module Gitlab
environment: job[:environment_name],
coverage_regex: job[:coverage],
yaml_variables: yaml_variables(name),
needs_attributes: job.dig(:needs, :job),
needs_attributes: job[:needs]&.map { |need| { name: need } },
interruptible: job[:interruptible],
rules: job[:rules],
options: {
......@@ -59,7 +59,7 @@ module Gitlab
instance: job[:instance],
start_in: job[:start_in],
trigger: job[:trigger],
bridge_needs: job.dig(:needs, :bridge)&.first
bridge_needs: job[:needs]
}.compact }.compact
end
......@@ -159,19 +159,17 @@ module Gitlab
end
def validate_job_needs!(name, job)
return unless job.dig(:needs, :job)
return unless job[:needs]
stage_index = @stages.index(job[:stage])
job.dig(:needs, :job).each do |need|
need_job_name = need[:name]
job[:needs].each do |need|
raise ValidationError, "#{name} job: undefined need: #{need}" unless @jobs[need.to_sym]
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])
needs_stage_index = @stages.index(@jobs[need.to_sym][:stage])
unless needs_stage_index.present? && needs_stage_index < stage_index
raise ValidationError, "#{name} job: need #{need_job_name} is not defined in prior stages"
raise ValidationError, "#{name} job: need #{need} is not defined in prior stages"
end
end
end
......
lib/gitlab/config/entry/configurable.rb
View file @ 04291872
......@@ -29,24 +29,22 @@ module Gitlab
def compose!(deps = nil)
return unless valid?
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)
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)
end
entries.each_value do |entry|
entry.compose!(deps)
end
end
# rubocop: enable CodeReuse/ActiveRecord
......@@ -69,13 +67,12 @@ module Gitlab
private
# 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)
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
......
lib/gitlab/config/entry/node.rb
View file @ 04291872
......@@ -112,10 +112,6 @@ module Gitlab
@aspects ||= []
end
def self.with_aspect(blk)
self.aspects.append(blk)
end
private
attr_reader :entries
......
lib/gitlab/config/entry/simplifiable.rb
View file @ 04291872
......@@ -4,11 +4,11 @@ module Gitlab
module Config
module Entry
class Simplifiable < SimpleDelegator
EntryStrategy = Struct.new(:name, :klass, :condition)
EntryStrategy = Struct.new(:name, :condition)
attr_reader :subject
def initialize(config, **metadata, &blk)
def initialize(config, **metadata)
unless self.class.const_defined?(:UnknownStrategy)
raise ArgumentError, 'UndefinedStrategy not available!'
end
......@@ -19,13 +19,14 @@ module Gitlab
entry = self.class.entry_class(strategy)
@subject = entry.new(config, metadata, &blk)
@subject = entry.new(config, metadata)
yield(@subject) if block_given?
super(@subject)
end
def self.strategy(name, **opts)
EntryStrategy.new(name, opts.dig(:class), opts.fetch(:if)).tap do |strategy|
EntryStrategy.new(name, opts.fetch(:if)).tap do |strategy|
strategies.append(strategy)
end
end
......@@ -36,7 +37,7 @@ module Gitlab
def self.entry_class(strategy)
if strategy.present?
strategy.klass || self.const_get(strategy.name, false)
self.const_get(strategy.name, false)
else
self::UnknownStrategy
end
......
lib/gitlab/config/entry/validatable.rb
View file @ 04291872
......@@ -7,27 +7,14 @@ module Gitlab
extend ActiveSupport::Concern
def self.included(node)
node.with_aspect -> do
validate(:new)
node.aspects.append -> do
@validator = self.class.validator.new(self)
@validator.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)
@validator.messages + descendants.flat_map(&:errors) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
class_methods do
......
spec/lib/gitlab/ci/config/entry/job_spec.rb
View file @ 04291872
......@@ -23,7 +23,7 @@ describe Gitlab::Ci::Config::Entry::Job do
let(:result) do
%i[before_script script stage type after_script cache
image services only except rules needs variables artifacts
image services only except rules variables artifacts
environment coverage retry]
end
......@@ -384,6 +384,21 @@ describe Gitlab::Ci::Config::Entry::Job do
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
{
......
spec/lib/gitlab/ci/config/entry/need_spec.rb deleted 100644 → 0
View file @ 91e1e5cb
# 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
spec/lib/gitlab/ci/config/entry/needs_spec.rb deleted 100644 → 0
View file @ 91e1e5cb
# 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
spec/lib/gitlab/ci/yaml_processor_spec.rb
View file @ 04291872
......@@ -1293,7 +1293,7 @@ module Gitlab
end
end
describe "Job Needs" do
describe "Needs" do
let(:needs) { }
let(:dependencies) { }
......@@ -1333,7 +1333,12 @@ module Gitlab
stage: "test",
stage_idx: 2,
name: "test1",
options: { script: ["test"] },
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]
},
needs_attributes: [
{ name: "build1" },
{ name: "build2" }
......@@ -1345,6 +1350,12 @@ module Gitlab
end
end
context 'needs two builds defined as symbols' do
let(:needs) { [:build1, :build2] }
it { expect { subject }.not_to raise_error }
end
context 'undefined need' do
let(:needs) { ['undefined'] }
......
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