Skip to content

G
gitlab-ce
Commit c16e1c8b authored by Kamil Trzciński | OoO on 2019-11-10's avatar Kamil Trzciński | OoO on 2019-11-10
Browse files
Options

Merge branch 'revert-96251702' into 'master' 

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

See merge request gitlab-org/gitlab!19479
parents e99f178f 04291872
Show whitespace changes
Inline Side-by-side
Showing with 126 additions and 581 deletions
ee/lib/ee/gitlab/ci/config/entry/bridge.rb
View file @ c16e1c8b
......@@ -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 @ e99f178f
# 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 @ c16e1c8b
# 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 @ c16e1c8b
......@@ -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 @ e99f178f
# 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 @ c16e1c8b
# 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
describe 'validations' do
before do
needs.compose!
end
context 'when entry config value is correct' do
let(:config) { ['job_name', pipeline: 'some/project'] }
context 'when upstream config is a non-empty string' do
let(:config) { { pipeline: 'some/project' } }
describe '#valid?' do
it { is_expected.to be_valid }
end
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')
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
context 'when bridge needs has wrong attributes' do
let(:config) { ['job_name', project: 'some/project'] }
context 'when upstream configuration is not valid' do
context 'when branch is not provided' do
let(:config) { { pipeline: 123 } }
describe '#valid?' do
it { is_expected.not_to be_valid }
end
end
end
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))
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 @ e99f178f
# 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 @ e99f178f
# 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 @ c16e1c8b
......@@ -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 @ e99f178f
# 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 @ e99f178f
# 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 @ c16e1c8b
......@@ -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 @ c16e1c8b
......@@ -29,7 +29,6 @@ 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
......@@ -48,7 +47,6 @@ module Gitlab
entry.compose!(deps)
end
end
end
# rubocop: enable CodeReuse/ActiveRecord
def skip_config_hash_validation?
......@@ -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 @ c16e1c8b
......@@ -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 @ c16e1c8b
......@@ -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 @ c16e1c8b
......@@ -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 @ c16e1c8b
......@@ -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 @ e99f178f
# 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 @ e99f178f
# 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 @ c16e1c8b
......@@ -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