Commit 505d71ec authored by Kamil Trzciński's avatar Kamil Trzciński Committed by Grzegorz Bizon

Introduce default: for gitlab-ci.yml

This moves all existing `image/services/before_script/variables`
into `default:`. This allows us to easily add a default and
top-level entries. `default`: is keep backward compatible: to
be considered to be job if `default:script:` is specified. This
behavior should be removed.

All existing `image/services/before_script/variables` are properly
handled in root context.
parent c167cc58
---
title: 'Introduce default: for gitlab-ci.yml'
merge_request:
author:
type: added
......@@ -14,23 +14,25 @@ module Gitlab
External::Processor::IncludeError
].freeze
attr_reader :root
def initialize(config, project: nil, sha: nil, user: nil)
@config = Config::Extendable
.new(build_config(config, project: project, sha: sha, user: user))
.to_hash
@global = Entry::Global.new(@config)
@global.compose!
@root = Entry::Root.new(@config)
@root.compose!
rescue *rescue_errors => e
raise Config::ConfigError, e.message
end
def valid?
@global.valid?
@root.valid?
end
def errors
@global.errors
@root.errors
end
def to_hash
......@@ -40,36 +42,16 @@ module Gitlab
##
# Temporary method that should be removed after refactoring
#
def before_script
@global.before_script_value
end
def image
@global.image_value
end
def services
@global.services_value
end
def after_script
@global.after_script_value
end
def variables
@global.variables_value
root.variables_value
end
def stages
@global.stages_value
end
def cache
@global.cache_value
root.stages_value
end
def jobs
@global.jobs_value
root.jobs_value
end
private
......
# frozen_string_literal: true
module Gitlab
module Ci
class Config
module Entry
##
# This class represents a default entry
# Entry containing default values for all jobs
# defined in configuration file.
#
class Default < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Configurable
DuplicateError = Class.new(Gitlab::Config::Loader::FormatError)
ALLOWED_KEYS = %i[before_script image services
after_script cache].freeze
validations do
validates :config, allowed_keys: ALLOWED_KEYS
end
entry :before_script, Entry::Script,
description: 'Script that will be executed before each job.',
inherit: true
entry :image, Entry::Image,
description: 'Docker image that will be used to execute jobs.',
inherit: true
entry :services, Entry::Services,
description: 'Docker images that will be linked to the container.',
inherit: true
entry :after_script, Entry::Script,
description: 'Script that will be executed after each job.',
inherit: true
entry :cache, Entry::Cache,
description: 'Configure caching between build jobs.',
inherit: true
helpers :before_script, :image, :services, :after_script, :cache
def compose!(deps = nil)
super(self)
inherit!(deps)
end
private
def inherit!(deps)
return unless deps
self.class.nodes.each do |key, factory|
next unless factory.inheritable?
root_entry = deps[key]
next unless root_entry.specified?
if self[key].specified?
raise DuplicateError, "#{key} is defined in top-level and `default:` entry"
end
@entries[key] = root_entry
end
end
end
end
end
end
end
......@@ -14,6 +14,14 @@ module Gitlab
validates :config, presence: true
end
def self.matching?(name, config)
name.to_s.start_with?('.')
end
def self.visible?
false
end
def relevant?
false
end
......
......@@ -42,7 +42,8 @@ module Gitlab
end
entry :before_script, Entry::Script,
description: 'Global before script overridden in this job.'
description: 'Global before script overridden in this job.',
inherit: true
entry :script, Entry::Commands,
description: 'Commands that will be executed in this job.'
......@@ -54,16 +55,20 @@ module Gitlab
description: 'Deprecated: stage this job will be executed into.'
entry :after_script, Entry::Script,
description: 'Commands that will be executed when finishing job.'
description: 'Commands that will be executed when finishing job.',
inherit: true
entry :cache, Entry::Cache,
description: 'Cache definition for this job.'
description: 'Cache definition for this job.',
inherit: true
entry :image, Entry::Image,
description: 'Image that will be used to execute this job.'
description: 'Image that will be used to execute this job.',
inherit: true
entry :services, Entry::Services,
description: 'Services that will be used to execute this job.'
description: 'Services that will be used to execute this job.',
inherit: true
entry :only, Entry::Policy,
description: 'Refs policy this job will be executed for.',
......@@ -95,6 +100,15 @@ module Gitlab
attributes :script, :tags, :allow_failure, :when, :dependencies,
:retry, :parallel, :extends, :start_in
def self.matching?(name, config)
!name.to_s.start_with?('.') &&
config.is_a?(Hash) && config.key?(:script)
end
def self.visible?
true
end
def compose!(deps = nil)
super do
if type_defined? && !stage_defined?
......@@ -129,15 +143,19 @@ module Gitlab
private
# We inherit config entries from `default:`
# if the entry has the `inherit: true` flag set
def inherit!(deps)
return unless deps
self.class.nodes.each_key do |key|
global_entry = deps[key]
self.class.nodes.each do |key, factory|
next unless factory.inheritable?
default_entry = deps.default[key]
job_entry = self[key]
if global_entry.specified? && !job_entry.specified?
@entries[key] = global_entry
if default_entry.specified? && !job_entry.specified?
@entries[key] = default_entry
end
end
end
......@@ -152,7 +170,7 @@ module Gitlab
cache: cache_value,
only: only_value,
except: except_value,
variables: variables_defined? ? variables_value : nil,
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,
......
......@@ -14,29 +14,48 @@ module Gitlab
validates :config, type: Hash
validate do
unless has_valid_jobs?
errors.add(:config, 'should contain valid jobs')
end
unless has_visible_job?
errors.add(:config, 'should contain at least one visible job')
end
end
def has_valid_jobs?
config.all? do |name, value|
Jobs.find_type(name, value)
end
end
def has_visible_job?
config.any? { |name, _| !hidden?(name) }
config.any? do |name, value|
Jobs.find_type(name, value)&.visible?
end
end
end
TYPES = [Entry::Hidden, Entry::Job].freeze
def hidden?(name)
name.to_s.start_with?('.')
private_constant :TYPES
def self.all_types
TYPES
end
def node_type(name)
hidden?(name) ? Entry::Hidden : Entry::Job
def self.find_type(name, config)
self.all_types.find do |type|
type.matching?(name, config)
end
end
# rubocop: disable CodeReuse/ActiveRecord
def compose!(deps = nil)
super do
@config.each do |name, config|
node = node_type(name)
node = self.class.find_type(name, config)
next unless node
factory = ::Gitlab::Config::Entry::Factory.new(node)
.value(config || {})
......
......@@ -8,52 +8,107 @@ module Gitlab
# This class represents a global entry - root Entry for entire
# GitLab CI Configuration file.
#
class Global < ::Gitlab::Config::Entry::Node
class Root < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Configurable
entry :before_script, Entry::Script,
description: 'Script that will be executed before each job.'
ALLOWED_KEYS = %i[default include before_script image services
after_script variables stages types cache].freeze
entry :image, Entry::Image,
description: 'Docker image that will be used to execute jobs.'
validations do
validates :config, allowed_keys: ALLOWED_KEYS
end
# reserved:
# defines whether the node name is reserved
# the reserved name cannot be used a job name
# reserved should not be used as it will make
# breaking change to `.gitlab-ci.yml`
entry :default, Entry::Default,
description: 'Default configuration for all jobs.',
default: {}
entry :include, Entry::Includes,
description: 'List of external YAML files to include.'
description: 'List of external YAML files to include.',
reserved: true
entry :before_script, Entry::Script,
description: 'Script that will be executed before each job.',
reserved: true
entry :image, Entry::Image,
description: 'Docker image that will be used to execute jobs.',
reserved: true
entry :services, Entry::Services,
description: 'Docker images that will be linked to the container.'
description: 'Docker images that will be linked to the container.',
reserved: true
entry :after_script, Entry::Script,
description: 'Script that will be executed after each job.'
description: 'Script that will be executed after each job.',
reserved: true
entry :variables, Entry::Variables,
description: 'Environment variables that will be used.'
description: 'Environment variables that will be used.',
reserved: true
entry :stages, Entry::Stages,
description: 'Configuration of stages for this pipeline.'
description: 'Configuration of stages for this pipeline.',
reserved: true
entry :types, Entry::Stages,
description: 'Deprecated: stages for this pipeline.'
description: 'Deprecated: stages for this pipeline.',
reserved: true
entry :cache, Entry::Cache,
description: 'Configure caching between build jobs.'
description: 'Configure caching between build jobs.',
reserved: true
helpers :default, :jobs, :stages, :types, :variables
delegate :before_script_value,
:image_value,
:services_value,
:after_script_value,
:cache_value, to: :default
attr_reader :jobs_config
class << self
include ::Gitlab::Utils::StrongMemoize
def reserved_nodes_names
strong_memoize(:reserved_nodes_names) do
self.nodes.select do |_, node|
node.reserved?
end.keys
end
end
end
helpers :before_script, :image, :services, :after_script,
:variables, :stages, :types, :cache, :jobs
def initialize(config, **metadata)
super do
filter_jobs!
end
end
def compose!(_deps = nil)
super(self) do
compose_jobs!
compose_deprecated_entries!
compose_jobs!
end
end
def default
self[:default]
end
private
# rubocop: disable CodeReuse/ActiveRecord
def compose_jobs!
factory = ::Gitlab::Config::Entry::Factory.new(Entry::Jobs)
.value(@config.except(*self.class.nodes.keys))
.value(jobs_config)
.with(key: :jobs, parent: self,
description: 'Jobs definition for this pipeline')
......@@ -72,6 +127,18 @@ module Gitlab
@entries.delete(:types)
end
def filter_jobs!
return unless @config.is_a?(Hash)
@jobs_config = @config
.except(*self.class.reserved_nodes_names) # rubocop: disable CodeReuse/ActiveRecord
.select do |name, config|
Entry::Jobs.find_type(name, config).present?
end
@config = @config.except(*@jobs_config.keys) # rubocop: disable CodeReuse/ActiveRecord
end
end
end
end
......
......@@ -7,7 +7,7 @@ module Gitlab
include Gitlab::Config::Entry::LegacyValidationHelpers
attr_reader :cache, :stages, :jobs
attr_reader :stages, :jobs
def initialize(config, opts = {})
@ci_config = Gitlab::Ci::Config.new(config, **opts)
......@@ -95,13 +95,8 @@ module Gitlab
##
# Global config
#
@before_script = @ci_config.before_script
@image = @ci_config.image
@after_script = @ci_config.after_script
@services = @ci_config.services
@variables = @ci_config.variables
@stages = @ci_config.stages
@cache = @ci_config.cache
##
# Jobs
......
......@@ -58,13 +58,21 @@ module Gitlab
Hash[(@nodes || {}).map { |key, factory| [key, factory.dup] }]
end
def reserved_node_names
self.nodes.select do |_, node|
node.reserved?
end.keys
end
private
# rubocop: disable CodeReuse/ActiveRecord
def entry(key, entry, metadata)
def entry(key, entry, description: nil, default: nil, inherit: nil, reserved: nil)
factory = ::Gitlab::Config::Entry::Factory.new(entry)
.with(description: metadata[:description])
.with(default: metadata[:default])
.with(description: description)
.with(default: default)
.with(inherit: inherit)
.with(reserved: reserved)
(@nodes ||= {}).merge!(key.to_sym => factory)
end
......
......@@ -30,6 +30,18 @@ module Gitlab
self
end
def description
@attributes[:description]
end
def inheritable?
@attributes[:inherit]
end
def reserved?
@attributes[:reserved]
end
def create!
raise InvalidFactory unless defined?(@value)
......
......@@ -107,7 +107,7 @@ describe Projects::Ci::LintsController do
end
it 'assigns errors' do
expect(assigns[:error]).to eq('jobs:rubocop config contains unknown keys: scriptt')
expect(assigns[:error]).to eq('root config contains unknown keys: rubocop')
end
end
......
require 'spec_helper'
describe Gitlab::Ci::Config::Entry::Default do
let(:entry) { described_class.new(config) }
describe '.nodes' do
it 'returns a hash' do
expect(described_class.nodes).to be_a(Hash)
end
context 'when filtering all the entry/node names' do
it 'contains the expected node names' do
expect(described_class.nodes.keys)
.to match_array(%i[before_script image services
after_script cache])
end
end
end
describe 'validations' do
before do
entry.compose!
end
context 'when default entry value is correct' do
let(:config) { { before_script: ['rspec'] } }
describe '#valid?' do
it 'is valid' do
expect(entry).to be_valid
end
end
end
context 'when default entry is empty' do
let(:config) { {} }
describe '#valid' do
it 'is valid' do
expect(entry).to be_valid
end
end
end
context 'when default entry 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 'default config should be a hash'
end
end
end
context 'when unknown keys detected' do
let(:config) { { unknown: true } }
describe '#valid' do
it 'is not valid' do
expect(entry).not_to be_valid
end
end
end
end
end
describe '#compose!' do
let(:specified) do
double('specified', 'specified?' => true, value: 'specified')
end
let(:unspecified) { double('unspecified', 'specified?' => false) }
let(:deps) { double('deps', '[]' => unspecified) }
context 'when default entry inherits configuration from root' do
let(:config) do
{ image: 'some_image' }
end
before do
allow(deps).to receive('[]').with(:image).and_return(specified)
end
it 'raises error' do
expect { entry.compose!(deps) }.to raise_error(
Gitlab::Ci::Config::Entry::Default::DuplicateError)
end
end
context 'when default entry inherits a non-defined configuration from root' do
let(:config) do
{ image: 'some_image' }
end
before do
allow(deps).to receive('[]').with(:after_script).and_return(specified)
entry.compose!(deps)
end
it 'inherits non-defined configuration entries' do
expect(entry[:image].value).to eq(name: 'some_image')
expect(entry[:after_script].value).to eq('specified')
end
end
end
end
require 'spec_helper'
describe Gitlab::Ci::Config::Entry::Hidden do
describe '.matching?' do
subject { described_class.matching?(name, {}) }
context 'when name starts with dot' do
let(:name) { '.hidden_job' }
it { is_expected.to be_truthy }
end
context 'when name does not start with dot' do
let(:name) { 'rspec' }
it { is_expected.to be_falsey }
end
end
describe '.new' do
let(:entry) { described_class.new(config) }
describe 'validations' do
......@@ -44,4 +61,5 @@ describe Gitlab::Ci::Config::Entry::Hidden do
expect(entry).not_to be_relevant
end
end
end
end
......@@ -17,6 +17,44 @@ describe Gitlab::Ci::Config::Entry::Job do
end
end
describe '.matching?' do
subject { described_class.matching?(name, config) }
context 'when config is not a hash' do
let(:name) { :rspec }
let(:config) { 'string' }
it { is_expected.to be_falsey }
end
context 'when config is a regular job' do
let(:name) { :rspec }
let(:config) do
{ script: 'ls -al' }
end
it { is_expected.to be_truthy }
end
context 'when config is a bridge job' do
let(:name) { :rspec }
let(:config) do
{ trigger: 'other-project' }
end
it { is_expected.to be_falsey }
end
context 'when config is a hidden job' do
let(:name) { '.rspec' }
let(:config) do
{ script: 'ls -al' }
end
it { is_expected.to be_falsey }
end
end
describe 'validations' do
before do
entry.compose!
......@@ -195,15 +233,15 @@ describe Gitlab::Ci::Config::Entry::Job do
end
describe '#compose!' do
let(:unspecified) { double('unspecified', 'specified?' => false) }
let(:specified) do
double('specified', 'specified?' => true, value: 'specified')
end
let(:deps) { double('deps', '[]' => unspecified) }
let(:unspecified) { double('unspecified', 'specified?' => false) }
let(:default) { double('default', '[]' => unspecified) }
let(:deps) { double('deps', 'default' => default, '[]' => unspecified) }
context 'when job config overrides global config' do
context 'when job config overrides default config' do
before do
entry.compose!(deps)
end
......@@ -212,21 +250,22 @@ describe Gitlab::Ci::Config::Entry::Job do
{ script: 'rspec', image: 'some_image', cache: { key: 'test' } }
end
it 'overrides global config' do
it 'overrides default config' do
expect(entry[:image].value).to eq(name: 'some_image')
expect(entry[:cache].value).to eq(key: 'test', policy: 'pull-push')
end
end
context 'when job config does not override global config' do
context 'when job config does not override default config' do
before do
allow(deps).to receive('[]').with(:image).and_return(specified)
allow(default).to receive('[]').with(:image).and_return(specified)
entry.compose!(deps)
end
let(:config) { { script: 'ls', cache: { key: 'test' } } }
it 'uses config from global entry' do
it 'uses config from default entry' do
expect(entry[:image].value).to eq 'specified'
expect(entry[:cache].value).to eq(key: 'test', policy: 'pull-push')
end
......@@ -258,7 +297,8 @@ describe Gitlab::Ci::Config::Entry::Job do
stage: 'test',
ignore: false,
after_script: %w[cleanup],
only: { refs: %w[branches tags] })
only: { refs: %w[branches tags] },
variables: {})
end
end
end
......
......@@ -3,6 +3,37 @@ require 'spec_helper'
describe Gitlab::Ci::Config::Entry::Jobs do
let(:entry) { described_class.new(config) }
describe '.all_types' do
subject { described_class.all_types }
it { is_expected.to include(::Gitlab::Ci::Config::Entry::Hidden) }
it { is_expected.to include(::Gitlab::Ci::Config::Entry::Job) }
end
describe '.find_type' do
using RSpec::Parameterized::TableSyntax
let(:config) do
{
'.hidden_job'.to_sym => { script: 'something' },
regular_job: { script: 'something' },
invalid_job: 'text'
}
end
where(:name, :type) do
:'.hidden_job' | ::Gitlab::Ci::Config::Entry::Hidden
:regular_job | ::Gitlab::Ci::Config::Entry::Job
:invalid_job | nil
end
subject { described_class.find_type(name, config[name]) }
with_them do
it { is_expected.to eq(type) }
end
end
describe 'validations' do
before do
entry.compose!
......@@ -29,11 +60,11 @@ describe Gitlab::Ci::Config::Entry::Jobs do
end
end
context 'when job is unspecified' do
context 'when job is invalid' do
let(:config) { { rspec: nil } }
it 'reports error' do
expect(entry.errors).to include "rspec config can't be blank"
expect(entry.errors).to include "jobs config should contain valid jobs"
end
end
......@@ -49,6 +80,7 @@ describe Gitlab::Ci::Config::Entry::Jobs do
end
end
describe '.compose!' do
context 'when valid job entries composed' do
before do
entry.compose!
......@@ -67,12 +99,14 @@ describe Gitlab::Ci::Config::Entry::Jobs do
script: %w[rspec],
ignore: false,
stage: 'test',
only: { refs: %w[branches tags] } },
only: { refs: %w[branches tags] },
variables: {} },
spinach: { name: :spinach,
script: %w[spinach],
ignore: false,
stage: 'test',
only: { refs: %w[branches tags] } })
only: { refs: %w[branches tags] },
variables: {} })
end
end
......@@ -92,4 +126,5 @@ describe Gitlab::Ci::Config::Entry::Jobs do
end
end
end
end
end
......@@ -77,7 +77,14 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Config do
end
context 'when pipeline contains configuration validation errors' do
let(:config) { { rspec: {} } }
let(:config) do
{
rspec: {
before_script: 10,
script: 'ls -al'
}
}
end
let(:pipeline) do
build(:ci_pipeline, project: project, config: config)
......@@ -85,7 +92,7 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Config do
it 'appends configuration validation errors to pipeline errors' do
expect(pipeline.errors.to_a)
.to include "jobs:rspec config can't be blank"
.to include "jobs:rspec:before_script config should be an array of strings"
end
it 'breaks the chain' do
......
......@@ -265,6 +265,19 @@ module Gitlab
end
end
context "in default context" do
let(:config) do
{
default: { before_script: ["global script"] },
test: { script: ["script"] }
}
end
it "return commands with scripts concencaced" do
expect(subject[:options][:before_script]).to eq(["global script"])
end
end
context "overwritten in local context" do
let(:config) do
{
......@@ -305,6 +318,19 @@ module Gitlab
end
end
context "in default context" do
let(:config) do
{
after_script: ["after_script"],
test: { script: ["script"] }
}
end
it "return after_script in options" do
expect(subject[:options][:after_script]).to eq(["after_script"])
end
end
context "overwritten in local context" do
let(:config) do
{
......@@ -774,6 +800,28 @@ module Gitlab
)
end
it "returns cache when defined in default context" do
config = YAML.dump(
{
default: {
cache: { paths: ["logs/", "binaries/"], untracked: true, key: 'key' }
},
rspec: {
script: "rspec"
}
})
config_processor = Gitlab::Ci::YamlProcessor.new(config)
expect(config_processor.stage_builds_attributes("test").size).to eq(1)
expect(config_processor.stage_builds_attributes("test").first[:options][:cache]).to eq(
paths: ["logs/", "binaries/"],
untracked: true,
key: 'key',
policy: 'pull-push'
)
end
it "returns cache when defined in a job" do
config = YAML.dump({
rspec: {
......@@ -1271,7 +1319,7 @@ module Gitlab
config = YAML.dump({ extra: "bundle update" })
expect do
Gitlab::Ci::YamlProcessor.new(config)
end.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, "jobs:extra config should be a hash")
end.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, "root config contains unknown keys: extra")
end
it "returns errors if services configuration is not correct" do
......
......@@ -34,18 +34,25 @@ describe Gitlab::Config::Entry::Configurable do
before do
entry.class_exec(entry_class) do |entry_class|
entry :object, entry_class, description: 'test object'
entry :object, entry_class,
description: 'test object',
inherit: true,
reserved: true
end
end
describe '.nodes' do
it 'has valid nodes' do
expect(entry.nodes).to include :object
expect(entry.nodes).to include(:object)
end
it 'creates a node factory' do
expect(entry.nodes[:object])
.to be_an_instance_of Gitlab::Config::Entry::Factory
factory = entry.nodes[:object]
expect(factory).to be_an_instance_of(Gitlab::Config::Entry::Factory)
expect(factory.description).to eq('test object')
expect(factory.inheritable?).to eq(true)
expect(factory.reserved?).to eq(true)
end
it 'returns a duplicated factory object' do
......@@ -55,5 +62,17 @@ describe Gitlab::Config::Entry::Configurable do
expect(first_factory).not_to be_equal(second_factory)
end
end
describe '.reserved_node_names' do
before do
entry.class_exec(entry_class) do |entry_class|
entry :not_reserved, entry_class
end
end
it 'returns all nodes with reserved: true' do
expect(entry.reserved_node_names).to contain_exactly(:object)
end
end
end
end
......@@ -23,17 +23,36 @@ describe Gitlab::Config::Entry::Factory do
end
context 'when setting description' do
it 'creates entry with description' do
entry = factory
before do
factory
.value(%w(ls pwd))
.with(description: 'test description')
.create!
end
it 'configures description' do
expect(factory.description).to eq 'test description'
end
it 'creates entry with description' do
entry = factory.create!
expect(entry.value).to eq %w(ls pwd)
expect(entry.description).to eq 'test description'
end
end
context 'when setting inherit' do
before do
factory
.value(%w(ls pwd))
.with(inherit: true)
end
it 'makes object inheritable' do
expect(factory.inheritable?).to eq true
end
end
context 'when setting key' do
it 'creates entry with custom key' do
entry = factory
......
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