diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2019-01-24 11:20:43 +0100 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2019-06-14 14:30:41 +0200 |
commit | 58404960ce9587c9d44f351450797642a7c6f63e (patch) | |
tree | 2d65ce34341352bc566e49f11c9c3acd6fefb9bc | |
parent | 577832598f1b35187efafc426068ef7ac36ae09f (diff) | |
download | gitlab-ce-58404960ce9587c9d44f351450797642a7c6f63e.tar.gz |
Introduce default: for gitlab-ci.ymlmove-all-configs-to-global
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.
21 files changed, 825 insertions, 316 deletions
diff --git a/changelogs/unreleased/move-all-configs-to-global.yml b/changelogs/unreleased/move-all-configs-to-global.yml new file mode 100644 index 00000000000..ff311d57f8d --- /dev/null +++ b/changelogs/unreleased/move-all-configs-to-global.yml @@ -0,0 +1,5 @@ +--- +title: 'Introduce default: for gitlab-ci.yml' +merge_request: +author: +type: added diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index f187e456993..98c752fcf8e 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -8,13 +8,15 @@ module Gitlab class Config ConfigError = Class.new(StandardError) + 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 Gitlab::Config::Loader::FormatError, Extendable::ExtensionError, External::Processor::IncludeError => e @@ -22,11 +24,11 @@ module Gitlab end def valid? - @global.valid? + @root.valid? end def errors - @global.errors + @root.errors end def to_hash @@ -36,36 +38,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 diff --git a/lib/gitlab/ci/config/entry/default.rb b/lib/gitlab/ci/config/entry/default.rb new file mode 100644 index 00000000000..6200d7c7f87 --- /dev/null +++ b/lib/gitlab/ci/config/entry/default.rb @@ -0,0 +1,74 @@ +# 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 diff --git a/lib/gitlab/ci/config/entry/global.rb b/lib/gitlab/ci/config/entry/global.rb deleted file mode 100644 index 2b5a59c078e..00000000000 --- a/lib/gitlab/ci/config/entry/global.rb +++ /dev/null @@ -1,79 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - class Config - module Entry - ## - # This class represents a global entry - root Entry for entire - # GitLab CI Configuration file. - # - class Global < ::Gitlab::Config::Entry::Node - include ::Gitlab::Config::Entry::Configurable - - entry :before_script, Entry::Script, - description: 'Script that will be executed before each job.' - - entry :image, Entry::Image, - description: 'Docker image that will be used to execute jobs.' - - entry :include, Entry::Includes, - description: 'List of external YAML files to include.' - - entry :services, Entry::Services, - description: 'Docker images that will be linked to the container.' - - entry :after_script, Entry::Script, - description: 'Script that will be executed after each job.' - - entry :variables, Entry::Variables, - description: 'Environment variables that will be used.' - - entry :stages, Entry::Stages, - description: 'Configuration of stages for this pipeline.' - - entry :types, Entry::Stages, - description: 'Deprecated: stages for this pipeline.' - - entry :cache, Entry::Cache, - description: 'Configure caching between build jobs.' - - helpers :before_script, :image, :services, :after_script, - :variables, :stages, :types, :cache, :jobs - - def compose!(_deps = nil) - super(self) do - compose_jobs! - compose_deprecated_entries! - end - end - - private - - # rubocop: disable CodeReuse/ActiveRecord - def compose_jobs! - factory = ::Gitlab::Config::Entry::Factory.new(Entry::Jobs) - .value(@config.except(*self.class.nodes.keys)) - .with(key: :jobs, parent: self, - description: 'Jobs definition for this pipeline') - - @entries[:jobs] = factory.create! - end - # rubocop: enable CodeReuse/ActiveRecord - - def compose_deprecated_entries! - ## - # Deprecated `:types` key workaround - if types are defined and - # stages are not defined we use types definition as stages. - # - if types_defined? && !stages_defined? - @entries[:stages] = @entries[:types] - end - - @entries.delete(:types) - end - end - end - end - end -end diff --git a/lib/gitlab/ci/config/entry/hidden.rb b/lib/gitlab/ci/config/entry/hidden.rb index 76e5d05639f..f9a9f036527 100644 --- a/lib/gitlab/ci/config/entry/hidden.rb +++ b/lib/gitlab/ci/config/entry/hidden.rb @@ -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 diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 762532f7007..5ab795359b8 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -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, diff --git a/lib/gitlab/ci/config/entry/jobs.rb b/lib/gitlab/ci/config/entry/jobs.rb index 9845c4af655..9d1a1ee8c4b 100644 --- a/lib/gitlab/ci/config/entry/jobs.rb +++ b/lib/gitlab/ci/config/entry/jobs.rb @@ -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 - def hidden?(name) - name.to_s.start_with?('.') + TYPES = [Entry::Hidden, Entry::Job].freeze + + 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 || {}) diff --git a/lib/gitlab/ci/config/entry/root.rb b/lib/gitlab/ci/config/entry/root.rb new file mode 100644 index 00000000000..0589ad3edf9 --- /dev/null +++ b/lib/gitlab/ci/config/entry/root.rb @@ -0,0 +1,146 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module Entry + ## + # This class represents a global entry - root Entry for entire + # GitLab CI Configuration file. + # + class Root < ::Gitlab::Config::Entry::Node + include ::Gitlab::Config::Entry::Configurable + + ALLOWED_KEYS = %i[default include before_script image services + after_script variables stages types cache].freeze + + 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.', + 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.', + reserved: true + + entry :after_script, Entry::Script, + description: 'Script that will be executed after each job.', + reserved: true + + entry :variables, Entry::Variables, + description: 'Environment variables that will be used.', + reserved: true + + entry :stages, Entry::Stages, + description: 'Configuration of stages for this pipeline.', + reserved: true + + entry :types, Entry::Stages, + description: 'Deprecated: stages for this pipeline.', + reserved: true + + entry :cache, Entry::Cache, + 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 + + def initialize(config, **metadata) + super do + filter_jobs! + end + end + + def compose!(_deps = nil) + super(self) do + 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(jobs_config) + .with(key: :jobs, parent: self, + description: 'Jobs definition for this pipeline') + + @entries[:jobs] = factory.create! + end + # rubocop: enable CodeReuse/ActiveRecord + + def compose_deprecated_entries! + ## + # Deprecated `:types` key workaround - if types are defined and + # stages are not defined we use types definition as stages. + # + if types_defined? && !stages_defined? + @entries[:stages] = @entries[:types] + end + + @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 + end +end diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 07ba6f83d47..a5693dc4f81 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -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 diff --git a/lib/gitlab/config/entry/configurable.rb b/lib/gitlab/config/entry/configurable.rb index 6667a5d3d33..b7ec4b7c4f8 100644 --- a/lib/gitlab/config/entry/configurable.rb +++ b/lib/gitlab/config/entry/configurable.rb @@ -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 diff --git a/lib/gitlab/config/entry/factory.rb b/lib/gitlab/config/entry/factory.rb index 3c06b1e0d24..8f1f4a81bb5 100644 --- a/lib/gitlab/config/entry/factory.rb +++ b/lib/gitlab/config/entry/factory.rb @@ -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) diff --git a/spec/controllers/projects/ci/lints_controller_spec.rb b/spec/controllers/projects/ci/lints_controller_spec.rb index cc6ac83ca38..96e82b7086c 100644 --- a/spec/controllers/projects/ci/lints_controller_spec.rb +++ b/spec/controllers/projects/ci/lints_controller_spec.rb @@ -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 diff --git a/spec/lib/gitlab/ci/config/entry/default_spec.rb b/spec/lib/gitlab/ci/config/entry/default_spec.rb new file mode 100644 index 00000000000..a901dd80c2c --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/default_spec.rb @@ -0,0 +1,109 @@ +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 diff --git a/spec/lib/gitlab/ci/config/entry/hidden_spec.rb b/spec/lib/gitlab/ci/config/entry/hidden_spec.rb index 459362761e6..c88ee10550c 100644 --- a/spec/lib/gitlab/ci/config/entry/hidden_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/hidden_spec.rb @@ -1,47 +1,65 @@ require 'spec_helper' describe Gitlab::Ci::Config::Entry::Hidden do - let(:entry) { described_class.new(config) } + describe '.matching?' do + subject { described_class.matching?(name, {}) } - describe 'validations' do - context 'when entry config value is correct' do - let(:config) { [:some, :array] } + context 'when name starts with dot' do + let(:name) { '.hidden_job' } - describe '#value' do - it 'returns key value' do - expect(entry.value).to eq [:some, :array] + 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 + context 'when entry config value is correct' do + let(:config) { [:some, :array] } + + describe '#value' do + it 'returns key value' do + expect(entry.value).to eq [:some, :array] + end end - end - describe '#valid?' do - it 'is valid' do - expect(entry).to be_valid + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end end end - end - context 'when entry value is not correct' do - context 'when config is empty' do - let(:config) { {} } + context 'when entry value is not correct' do + context 'when config is empty' do + let(:config) { {} } - describe '#valid' do - it 'is invalid' do - expect(entry).not_to be_valid + describe '#valid' do + it 'is invalid' do + expect(entry).not_to be_valid + end end end end end - end - describe '#leaf?' do - it 'is a leaf' do - expect(entry).to be_leaf + describe '#leaf?' do + it 'is a leaf' do + expect(entry).to be_leaf + end end - end - describe '#relevant?' do - it 'is not a relevant entry' do - expect(entry).not_to be_relevant + describe '#relevant?' do + it 'is not a relevant entry' do + expect(entry).not_to be_relevant + end end end end diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index e0552ae8c57..25766d34c65 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -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 diff --git a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb index 271ee30df3c..3e4137930dc 100644 --- a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb @@ -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,46 +80,50 @@ describe Gitlab::Ci::Config::Entry::Jobs do end end - context 'when valid job entries composed' do - before do - entry.compose! - end + describe '.compose!' do + context 'when valid job entries composed' do + before do + entry.compose! + end - let(:config) do - { rspec: { script: 'rspec' }, - spinach: { script: 'spinach' }, - '.hidden'.to_sym => {} } - end + let(:config) do + { rspec: { script: 'rspec' }, + spinach: { script: 'spinach' }, + '.hidden'.to_sym => {} } + end - describe '#value' do - it 'returns key value' do - expect(entry.value).to eq( - rspec: { name: :rspec, - script: %w[rspec], - ignore: false, - stage: 'test', - only: { refs: %w[branches tags] } }, - spinach: { name: :spinach, - script: %w[spinach], - ignore: false, - stage: 'test', - only: { refs: %w[branches tags] } }) + describe '#value' do + it 'returns key value' do + expect(entry.value).to eq( + rspec: { name: :rspec, + script: %w[rspec], + ignore: false, + stage: 'test', + only: { refs: %w[branches tags] }, + variables: {} }, + spinach: { name: :spinach, + script: %w[spinach], + ignore: false, + stage: 'test', + only: { refs: %w[branches tags] }, + variables: {} }) + end end - end - describe '#descendants' do - it 'creates valid descendant nodes' do - expect(entry.descendants.count).to eq 3 - expect(entry.descendants.first(2)) - .to all(be_an_instance_of(Gitlab::Ci::Config::Entry::Job)) - expect(entry.descendants.last) - .to be_an_instance_of(Gitlab::Ci::Config::Entry::Hidden) + describe '#descendants' do + it 'creates valid descendant nodes' do + expect(entry.descendants.count).to eq 3 + expect(entry.descendants.first(2)) + .to all(be_an_instance_of(Gitlab::Ci::Config::Entry::Job)) + expect(entry.descendants.last) + .to be_an_instance_of(Gitlab::Ci::Config::Entry::Hidden) + end end - end - describe '#value' do - it 'returns value of visible jobs only' do - expect(entry.value.keys).to eq [:rspec, :spinach] + describe '#value' do + it 'returns value of visible jobs only' do + expect(entry.value.keys).to eq [:rspec, :spinach] + end end end end diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/root_spec.rb index e23efff18d5..7a252ed675a 100644 --- a/spec/lib/gitlab/ci/config/entry/global_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/root_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe Gitlab::Ci::Config::Entry::Global do - let(:global) { described_class.new(hash) } +describe Gitlab::Ci::Config::Entry::Root do + let(:root) { described_class.new(hash) } describe '.nodes' do it 'returns a hash' do @@ -12,17 +12,18 @@ describe Gitlab::Ci::Config::Entry::Global do it 'contains the expected node names' do expect(described_class.nodes.keys) .to match_array(%i[before_script image services - after_script variables stages - types cache include]) + after_script variables cache + stages types include default]) end end end context 'when configuration is valid' do - context 'when some entries defined' do + context 'when top-level entries are defined' do let(:hash) do { before_script: %w(ls pwd), image: 'ruby:2.2', + default: {}, services: ['postgres:9.1', 'mysql:5.5'], variables: { VAR: 'value' }, after_script: ['make clean'], @@ -34,97 +35,53 @@ describe Gitlab::Ci::Config::Entry::Global do describe '#compose!' do before do - global.compose! + root.compose! end it 'creates nodes hash' do - expect(global.descendants).to be_an Array + expect(root.descendants).to be_an Array end it 'creates node object for each entry' do - expect(global.descendants.count).to eq 9 + expect(root.descendants.count).to eq 10 end it 'creates node object using valid class' do - expect(global.descendants.first) - .to be_an_instance_of Gitlab::Ci::Config::Entry::Script - expect(global.descendants.second) - .to be_an_instance_of Gitlab::Ci::Config::Entry::Image + expect(root.descendants.first) + .to be_an_instance_of Gitlab::Ci::Config::Entry::Default + expect(root.descendants.second) + .to be_an_instance_of Gitlab::Config::Entry::Unspecified end it 'sets correct description for nodes' do - expect(global.descendants.first.description) - .to eq 'Script that will be executed before each job.' - expect(global.descendants.second.description) - .to eq 'Docker image that will be used to execute jobs.' + expect(root.descendants.first.description) + .to eq 'Default configuration for all jobs.' + expect(root.descendants.second.description) + .to eq 'List of external YAML files to include.' end describe '#leaf?' do it 'is not leaf' do - expect(global).not_to be_leaf - end - end - end - - context 'when not composed' do - describe '#before_script_value' do - it 'returns nil' do - expect(global.before_script_value).to be nil - end - end - - describe '#leaf?' do - it 'is leaf' do - expect(global).to be_leaf + expect(root).not_to be_leaf end end end context 'when composed' do before do - global.compose! + root.compose! end describe '#errors' do it 'has no errors' do - expect(global.errors).to be_empty - end - end - - describe '#before_script_value' do - it 'returns correct script' do - expect(global.before_script_value).to eq %w(ls pwd) - end - end - - describe '#image_value' do - it 'returns valid image' do - expect(global.image_value).to eq(name: 'ruby:2.2') - end - end - - describe '#services_value' do - it 'returns array of services' do - expect(global.services_value).to eq [{ name: 'postgres:9.1' }, { name: 'mysql:5.5' }] - end - end - - describe '#after_script_value' do - it 'returns after script' do - expect(global.after_script_value).to eq ['make clean'] - end - end - - describe '#variables_value' do - it 'returns variables' do - expect(global.variables_value).to eq('VAR' => 'value') + expect(root.errors).to be_empty end end describe '#stages_value' do context 'when stages key defined' do it 'returns array of stages' do - expect(global.stages_value).to eq %w[build pages] + expect(root.stages_value).to eq %w[build pages] end end @@ -135,21 +92,14 @@ describe Gitlab::Ci::Config::Entry::Global do end it 'returns array of types as stages' do - expect(global.stages_value).to eq %w[test deploy] + expect(root.stages_value).to eq %w[test deploy] end end end - describe '#cache_value' do - it 'returns cache configuration' do - expect(global.cache_value) - .to eq(key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push') - end - end - describe '#jobs_value' do it 'returns jobs configuration' do - expect(global.jobs_value).to eq( + expect(root.jobs_value).to eq( rspec: { name: :rspec, script: %w[rspec ls], before_script: %w(ls pwd), @@ -157,7 +107,7 @@ describe Gitlab::Ci::Config::Entry::Global do services: [{ name: 'postgres:9.1' }, { name: 'mysql:5.5' }], stage: 'test', cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push' }, - variables: { 'VAR' => 'value' }, + variables: {}, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] } }, @@ -178,9 +128,66 @@ describe Gitlab::Ci::Config::Entry::Global do end end + context 'when a mix of top-level and default entries is used' do + let(:hash) do + { before_script: %w(ls pwd), + after_script: ['make clean'], + default: { + image: 'ruby:2.1', + services: ['postgres:9.1', 'mysql:5.5'] + }, + variables: { VAR: 'value' }, + stages: %w(build pages), + cache: { key: 'k', untracked: true, paths: ['public/'] }, + rspec: { script: %w[rspec ls] }, + spinach: { before_script: [], variables: { VAR: 'AA' }, script: 'spinach' } } + end + + context 'when composed' do + before do + root.compose! + end + + describe '#errors' do + it 'has no errors' do + expect(root.errors).to be_empty + end + end + + describe '#jobs_value' do + it 'returns jobs configuration' do + expect(root.jobs_value).to eq( + rspec: { name: :rspec, + script: %w[rspec ls], + before_script: %w(ls pwd), + image: { name: 'ruby:2.1' }, + services: [{ name: 'postgres:9.1' }, { name: 'mysql:5.5' }], + stage: 'test', + cache: { key: 'k', untracked: true, paths: ['public/'], policy: "pull-push" }, + variables: {}, + ignore: false, + after_script: ['make clean'], + only: { refs: %w[branches tags] } }, + spinach: { name: :spinach, + before_script: [], + script: %w[spinach], + image: { name: 'ruby:2.1' }, + services: [{ name: 'postgres:9.1' }, { name: 'mysql:5.5' }], + stage: 'test', + cache: { key: 'k', untracked: true, paths: ['public/'], policy: "pull-push" }, + variables: { 'VAR' => 'AA' }, + ignore: false, + after_script: ['make clean'], + only: { refs: %w[branches tags] } } + ) + end + end + end + end + context 'when most of entires not defined' do before do - global.compose! + root.compose! end let(:hash) do @@ -189,30 +196,55 @@ describe Gitlab::Ci::Config::Entry::Global do describe '#nodes' do it 'instantizes all nodes' do - expect(global.descendants.count).to eq 9 + expect(root.descendants.count).to eq 10 end it 'contains unspecified nodes' do - expect(global.descendants.first) + expect(root.descendants.first) .not_to be_specified end end describe '#variables_value' do - it 'returns default value for variables' do - expect(global.variables_value).to eq({}) + it 'returns root value for variables' do + expect(root.variables_value).to eq({}) end end describe '#stages_value' do - it 'returns an array of default stages' do - expect(global.stages_value).to eq %w[build test deploy] + it 'returns an array of root stages' do + expect(root.stages_value).to eq %w[build test deploy] end end describe '#cache_value' do it 'returns correct cache definition' do - expect(global.cache_value).to eq(key: 'a', policy: 'pull-push') + expect(root.cache_value).to eq(key: 'a', policy: 'pull-push') + end + end + end + + context 'when variables resembles script-type job' do + before do + root.compose! + end + + let(:hash) do + { + variables: { script: "ENV_VALUE" }, + rspec: { script: "echo Hello World" } + } + end + + describe '#variables_value' do + it 'returns root value for variables' do + expect(root.variables_value).to eq("script" => "ENV_VALUE") + end + end + + describe '#jobs_value' do + it 'returns one job' do + expect(root.jobs_value.keys).to contain_exactly(:rspec) end end end @@ -225,7 +257,7 @@ describe Gitlab::Ci::Config::Entry::Global do # context 'when entires specified but not defined' do before do - global.compose! + root.compose! end let(:hash) do @@ -233,8 +265,8 @@ describe Gitlab::Ci::Config::Entry::Global do end describe '#variables_value' do - it 'undefined entry returns a default value' do - expect(global.variables_value).to eq({}) + it 'undefined entry returns a root value' do + expect(root.variables_value).to eq({}) end end end @@ -242,7 +274,7 @@ describe Gitlab::Ci::Config::Entry::Global do context 'when configuration is not valid' do before do - global.compose! + root.compose! end context 'when before script is not an array' do @@ -252,22 +284,16 @@ describe Gitlab::Ci::Config::Entry::Global do describe '#valid?' do it 'is not valid' do - expect(global).not_to be_valid + expect(root).not_to be_valid end end describe '#errors' do it 'reports errors from child nodes' do - expect(global.errors) + expect(root.errors) .to include 'before_script config should be an array of strings' end end - - describe '#before_script_value' do - it 'returns nil' do - expect(global.before_script_value).to be_nil - end - end end context 'when job does not have commands' do @@ -277,8 +303,8 @@ describe Gitlab::Ci::Config::Entry::Global do describe '#errors' do it 'reports errors about missing script' do - expect(global.errors) - .to include "jobs:rspec script can't be blank" + expect(root.errors) + .to include "root config contains unknown keys: rspec" end end end @@ -289,26 +315,26 @@ describe Gitlab::Ci::Config::Entry::Global do describe '#valid?' do it 'is not valid' do - expect(global).not_to be_valid + expect(root).not_to be_valid end end describe '#errors' do it 'returns error about invalid type' do - expect(global.errors.first).to match /should be a hash/ + expect(root.errors.first).to match /should be a hash/ end end end describe '#specified?' do it 'is concrete entry that is defined' do - expect(global.specified?).to be true + expect(root.specified?).to be true end end describe '#[]' do before do - global.compose! + root.compose! end let(:hash) do @@ -317,15 +343,15 @@ describe Gitlab::Ci::Config::Entry::Global do context 'when entry exists' do it 'returns correct entry' do - expect(global[:cache]) + expect(root[:cache]) .to be_an_instance_of Gitlab::Ci::Config::Entry::Cache - expect(global[:jobs][:rspec][:script].value).to eq ['ls'] + expect(root[:jobs][:rspec][:script].value).to eq ['ls'] end end context 'when entry does not exist' do it 'always return unspecified node' do - expect(global[:some][:unknown][:node]) + expect(root[:some][:unknown][:node]) .not_to be_specified end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb index e6c6a82b463..00bbc4c817a 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb @@ -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 diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 635b4e556e8..789d6813d5b 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -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 diff --git a/spec/lib/gitlab/config/entry/configurable_spec.rb b/spec/lib/gitlab/config/entry/configurable_spec.rb index 37e38e49c0d..62dbf20ddf8 100644 --- a/spec/lib/gitlab/config/entry/configurable_spec.rb +++ b/spec/lib/gitlab/config/entry/configurable_spec.rb @@ -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 diff --git a/spec/lib/gitlab/config/entry/factory_spec.rb b/spec/lib/gitlab/config/entry/factory_spec.rb index c29d17eaee3..42f8be8e141 100644 --- a/spec/lib/gitlab/config/entry/factory_spec.rb +++ b/spec/lib/gitlab/config/entry/factory_spec.rb @@ -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 |