diff options
| -rw-r--r-- | changelogs/unreleased/feature-gb-allow-to-extend-keys-in-gitlab-ci-yml.yml | 5 | ||||
| -rw-r--r-- | doc/ci/yaml/README.md | 74 | ||||
| -rw-r--r-- | lib/gitlab/ci/config.rb | 14 | ||||
| -rw-r--r-- | lib/gitlab/ci/config/entry/job.rb | 11 | ||||
| -rw-r--r-- | lib/gitlab/ci/config/extendable.rb | 29 | ||||
| -rw-r--r-- | lib/gitlab/ci/config/extendable/entry.rb | 95 | ||||
| -rw-r--r-- | lib/gitlab/ci/yaml_processor.rb | 2 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/config/entry/job_spec.rb | 14 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/config/extendable/entry_spec.rb | 227 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/config/extendable_spec.rb | 228 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/config_spec.rb | 52 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/yaml_processor_spec.rb | 60 |
12 files changed, 801 insertions, 10 deletions
diff --git a/changelogs/unreleased/feature-gb-allow-to-extend-keys-in-gitlab-ci-yml.yml b/changelogs/unreleased/feature-gb-allow-to-extend-keys-in-gitlab-ci-yml.yml new file mode 100644 index 00000000000..b46dfd47e7a --- /dev/null +++ b/changelogs/unreleased/feature-gb-allow-to-extend-keys-in-gitlab-ci-yml.yml @@ -0,0 +1,5 @@ +--- +title: Add support for extendable CI/CD config with +merge_request: 21243 +author: +type: added diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index e93060fec85..c1ebe39e076 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -56,6 +56,7 @@ A job is defined by a list of parameters that define the job behavior. | Keyword | Required | Description | |---------------|----------|-------------| | script | yes | Defines a shell script which is executed by Runner | +| extends | no | Defines a configuration entry that this job is going to inherit from | | image | no | Use docker image, covered in [Using Docker Images](../docker/using_docker_images.md#define-image-and-services-from-gitlab-ciyml) | | services | no | Use docker services, covered in [Using Docker Images](../docker/using_docker_images.md#define-image-and-services-from-gitlab-ciyml) | | stage | no | Defines a job stage (default: `test`) | @@ -75,6 +76,79 @@ A job is defined by a list of parameters that define the job behavior. | coverage | no | Define code coverage settings for a given job | | retry | no | Define how many times a job can be auto-retried in case of a failure | +### `extends` + +> Introduced in GitLab 11.3 + +`extends` defines an entry name that a job, that uses `extends` is going to +inherit from. + +`extends` in an alternative to using [YAML anchors](#anchors) that is a little +more flexible and readable. + +```yaml +.tests: + only: + refs: + - branches + +rspec: + extends: .tests + script: rake rspec + stage: test + only: + variables: + - $RSPEC +``` + +In the example above the `rspec` job is going to inherit from `.tests` +template. GitLab will perform a reverse deep merge, what means that it will +merge `rspec` contents into `.tests` recursively, and it is going to result in +following configuration of the `rspec` job: + +```yaml +rspec: + script: rake rspec + stage: test + only: + refs: + - branches + variables: + - $RSPEC +``` + +`.tests` in this example is a [hidden key](#hidden-keys-jobs), but it is +possible to inherit from regular jobs as well. + +`extends` supports multi-level inheritance, however it is not recommended to +use more than three levels of inheritance. Maximum nesting level supported is +10 levels. + + +```yaml +.tests: + only: + - pushes + +.rspec: + extends: .tests + script: rake rspec + +rspec 1: + variables: + RSPEC_SUITE: '1' + extends: .rspec + +rspec 2: + variables: + RSPEC_SUITE: '2' + extends: .rspec + +spinach: + extends: .tests + script: rake spinach +``` + ### `pages` `pages` is a special job that is used to upload static content to GitLab that diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index 66ac4a40616..46dad59eb8c 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -4,12 +4,17 @@ module Gitlab # Base GitLab CI Configuration facade # class Config - # EE would override this and utilize opts argument + ConfigError = Class.new(StandardError) + def initialize(config, opts = {}) - @config = Loader.new(config).load! + @config = Config::Extendable + .new(build_config(config, opts)) + .to_hash @global = Entry::Global.new(@config) @global.compose! + rescue Loader::FormatError, Extendable::ExtensionError => e + raise Config::ConfigError, e.message end def valid? @@ -58,6 +63,11 @@ module Gitlab def jobs @global.jobs_value end + + # 'opts' argument is used in EE see /ee/lib/ee/gitlab/ci/config.rb + def build_config(config, opts = {}) + Loader.new(config).load! + end end end end diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 91aac6df4b1..016a896bde5 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -9,9 +9,10 @@ module Gitlab include Configurable include Attributable - ALLOWED_KEYS = %i[tags script only except type image services allow_failure - type stage when artifacts cache dependencies before_script - after_script variables environment coverage retry].freeze + ALLOWED_KEYS = %i[tags script only except type image services + allow_failure type stage when artifacts cache + dependencies before_script after_script variables + environment coverage retry extends].freeze validations do validates :config, allowed_keys: ALLOWED_KEYS @@ -32,6 +33,7 @@ module Gitlab 'always or manual' } validates :dependencies, array_of_strings: true + validates :extends, type: String end end @@ -81,7 +83,8 @@ module Gitlab :cache, :image, :services, :only, :except, :variables, :artifacts, :commands, :environment, :coverage, :retry - attributes :script, :tags, :allow_failure, :when, :dependencies, :retry + attributes :script, :tags, :allow_failure, :when, :dependencies, + :retry, :extends def compose!(deps = nil) super do diff --git a/lib/gitlab/ci/config/extendable.rb b/lib/gitlab/ci/config/extendable.rb new file mode 100644 index 00000000000..a43901c69fe --- /dev/null +++ b/lib/gitlab/ci/config/extendable.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + class Extendable + include Enumerable + + ExtensionError = Class.new(StandardError) + + def initialize(hash) + @hash = hash.to_h.deep_dup + + each { |entry| entry.extend! if entry.extensible? } + end + + def each + @hash.each_key do |key| + yield Extendable::Entry.new(key, @hash) + end + end + + def to_hash + @hash.to_h + end + end + end + end +end diff --git a/lib/gitlab/ci/config/extendable/entry.rb b/lib/gitlab/ci/config/extendable/entry.rb new file mode 100644 index 00000000000..7793db09d33 --- /dev/null +++ b/lib/gitlab/ci/config/extendable/entry.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + class Extendable + class Entry + InvalidExtensionError = Class.new(Extendable::ExtensionError) + CircularDependencyError = Class.new(Extendable::ExtensionError) + NestingTooDeepError = Class.new(Extendable::ExtensionError) + + MAX_NESTING_LEVELS = 10 + + attr_reader :key + + def initialize(key, context, parent = nil) + @key = key + @context = context + @parent = parent + + unless @context.key?(@key) + raise StandardError, 'Invalid entry key!' + end + end + + def extensible? + value.is_a?(Hash) && value.key?(:extends) + end + + def value + @value ||= @context.fetch(@key) + end + + def base_hash! + @base ||= Extendable::Entry + .new(extends_key, @context, self) + .extend! + end + + def extends_key + value.fetch(:extends).to_s.to_sym if extensible? + end + + def ancestors + @ancestors ||= Array(@parent&.ancestors) + Array(@parent&.key) + end + + def extend! + return value unless extensible? + + if unknown_extension? + raise Entry::InvalidExtensionError, + "#{key}: unknown key in `extends`" + end + + if invalid_base? + raise Entry::InvalidExtensionError, + "#{key}: invalid base hash in `extends`" + end + + if nesting_too_deep? + raise Entry::NestingTooDeepError, + "#{key}: nesting too deep in `extends`" + end + + if circular_dependency? + raise Entry::CircularDependencyError, + "#{key}: circular dependency detected in `extends`" + end + + @context[key] = base_hash!.deep_merge(value) + end + + private + + def nesting_too_deep? + ancestors.count > MAX_NESTING_LEVELS + end + + def circular_dependency? + ancestors.include?(key) + end + + def unknown_extension? + !@context.key?(extends_key) + end + + def invalid_base? + !@context[extends_key].is_a?(Hash) + end + end + end + end + end +end diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index e829f2a95f8..5d1864ae9e2 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -16,7 +16,7 @@ module Gitlab end initial_parsing - rescue Gitlab::Ci::Config::Loader::FormatError => e + rescue Gitlab::Ci::Config::ConfigError => e raise ValidationError, e.message end diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 6769f64f950..2c9758401b7 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -1,4 +1,5 @@ -require 'spec_helper' +require 'fast_spec_helper' +require_dependency 'active_model' describe Gitlab::Ci::Config::Entry::Job do let(:entry) { described_class.new(config, name: :rspec) } @@ -81,6 +82,15 @@ describe Gitlab::Ci::Config::Entry::Job do end end + context 'when extends key is not a string' do + let(:config) { { extends: 123 } } + + it 'returns error about wrong value type' do + expect(entry).not_to be_valid + expect(entry.errors).to include "job extends should be a string" + end + end + context 'when retry value is not correct' do context 'when it is not a numeric value' do let(:config) { { retry: true } } @@ -124,6 +134,8 @@ describe Gitlab::Ci::Config::Entry::Job do describe '#relevant?' do it 'is a relevant entry' do + entry = described_class.new({ script: 'rspec' }, name: :rspec) + expect(entry).to be_relevant end end diff --git a/spec/lib/gitlab/ci/config/extendable/entry_spec.rb b/spec/lib/gitlab/ci/config/extendable/entry_spec.rb new file mode 100644 index 00000000000..0a148375d11 --- /dev/null +++ b/spec/lib/gitlab/ci/config/extendable/entry_spec.rb @@ -0,0 +1,227 @@ +require 'fast_spec_helper' + +describe Gitlab::Ci::Config::Extendable::Entry do + describe '.new' do + context 'when entry key is not included in the context hash' do + it 'raises error' do + expect { described_class.new(:test, something: 'something') } + .to raise_error StandardError, 'Invalid entry key!' + end + end + end + + describe '#value' do + it 'reads a hash value from the context' do + entry = described_class.new(:test, test: 'something') + + expect(entry.value).to eq 'something' + end + end + + describe '#extensible?' do + context 'when entry has inheritance defined' do + it 'is extensible' do + entry = described_class.new(:test, test: { extends: 'something' }) + + expect(entry).to be_extensible + end + end + + context 'when entry does not have inheritance specified' do + it 'is not extensible' do + entry = described_class.new(:test, test: { script: 'something' }) + + expect(entry).not_to be_extensible + end + end + + context 'when entry value is not a hash' do + it 'is not extensible' do + entry = described_class.new(:test, test: 'something') + + expect(entry).not_to be_extensible + end + end + end + + describe '#extends_key' do + context 'when entry is extensible' do + it 'returns symbolized extends key value' do + entry = described_class.new(:test, test: { extends: 'something' }) + + expect(entry.extends_key).to eq :something + end + end + + context 'when entry is not extensible' do + it 'returns nil' do + entry = described_class.new(:test, test: 'something') + + expect(entry.extends_key).to be_nil + end + end + end + + describe '#ancestors' do + let(:parent) do + described_class.new(:test, test: { extends: 'something' }) + end + + let(:child) do + described_class.new(:job, { job: { script: 'something' } }, parent) + end + + it 'returns ancestors keys' do + expect(child.ancestors).to eq [:test] + end + end + + describe '#base_hash!' do + subject { described_class.new(:test, hash) } + + context 'when base hash is not extensible' do + let(:hash) do + { + template: { script: 'rspec' }, + test: { extends: 'template' } + } + end + + it 'returns unchanged base hash' do + expect(subject.base_hash!).to eq(script: 'rspec') + end + end + + context 'when base hash is extensible too' do + let(:hash) do + { + first: { script: 'rspec' }, + second: { extends: 'first' }, + test: { extends: 'second' } + } + end + + it 'extends the base hash first' do + expect(subject.base_hash!).to eq(extends: 'first', script: 'rspec') + end + + it 'mutates original context' do + subject.base_hash! + + expect(hash.fetch(:second)).to eq(extends: 'first', script: 'rspec') + end + end + end + + describe '#extend!' do + subject { described_class.new(:test, hash) } + + context 'when extending a non-hash value' do + let(:hash) do + { + first: 'my value', + test: { extends: 'first' } + } + end + + it 'raises an error' do + expect { subject.extend! } + .to raise_error(described_class::InvalidExtensionError, + /invalid base hash/) + end + end + + context 'when extending unknown key' do + let(:hash) do + { test: { extends: 'something' } } + end + + it 'raises an error' do + expect { subject.extend! } + .to raise_error(described_class::InvalidExtensionError, + /unknown key/) + end + end + + context 'when extending a hash correctly' do + let(:hash) do + { + first: { script: 'my value' }, + second: { extends: 'first' }, + test: { extends: 'second' } + } + end + + let(:result) do + { + first: { script: 'my value' }, + second: { extends: 'first', script: 'my value' }, + test: { extends: 'second', script: 'my value' } + } + end + + it 'returns extended part of the hash' do + expect(subject.extend!).to eq result[:test] + end + + it 'mutates original context' do + subject.extend! + + expect(hash).to eq result + end + end + + context 'when hash is not extensible' do + let(:hash) do + { + first: { script: 'my value' }, + second: { extends: 'first' }, + test: { value: 'something' } + } + end + + it 'returns original key value' do + expect(subject.extend!).to eq(value: 'something') + end + + it 'does not mutate orignal context' do + original = hash.deep_dup + + subject.extend! + + expect(hash).to eq original + end + end + + context 'when circular depenency gets detected' do + let(:hash) do + { test: { extends: 'test' } } + end + + it 'raises an error' do + expect { subject.extend! } + .to raise_error(described_class::CircularDependencyError, + /circular dependency detected/) + end + end + + context 'when nesting level is too deep' do + before do + stub_const("#{described_class}::MAX_NESTING_LEVELS", 0) + end + + let(:hash) do + { + first: { script: 'my value' }, + second: { extends: 'first' }, + test: { extends: 'second' } + } + end + + it 'raises an error' do + expect { subject.extend! } + .to raise_error(described_class::NestingTooDeepError) + end + end + end +end diff --git a/spec/lib/gitlab/ci/config/extendable_spec.rb b/spec/lib/gitlab/ci/config/extendable_spec.rb new file mode 100644 index 00000000000..90213f6603d --- /dev/null +++ b/spec/lib/gitlab/ci/config/extendable_spec.rb @@ -0,0 +1,228 @@ +require 'fast_spec_helper' + +describe Gitlab::Ci::Config::Extendable do + subject { described_class.new(hash) } + + describe '#each' do + context 'when there is extendable entry in the hash' do + let(:test) do + { extends: 'something', only: %w[master] } + end + + let(:hash) do + { something: { script: 'ls' }, test: test } + end + + it 'yields control' do + expect { |b| subject.each(&b) }.to yield_control + end + end + end + + describe '#to_hash' do + context 'when hash does not contain extensions' do + let(:hash) do + { + test: { script: 'test' }, + production: { + script: 'deploy', + only: { variables: %w[$SOMETHING] } + } + } + end + + it 'does not modify the hash' do + expect(subject.to_hash).to eq hash + end + end + + context 'when hash has a single simple extension' do + let(:hash) do + { + something: { + script: 'deploy', + only: { variables: %w[$SOMETHING] } + }, + + test: { + extends: 'something', + script: 'ls', + only: { refs: %w[master] } + } + } + end + + it 'extends a hash with a deep reverse merge' do + expect(subject.to_hash).to eq( + something: { + script: 'deploy', + only: { variables: %w[$SOMETHING] } + }, + + test: { + extends: 'something', + script: 'ls', + only: { + refs: %w[master], + variables: %w[$SOMETHING] + } + } + ) + end + end + + context 'when a hash uses recursive extensions' do + let(:hash) do + { + test: { + extends: 'something', + script: 'ls', + only: { refs: %w[master] } + }, + + build: { + extends: 'something', + stage: 'build' + }, + + deploy: { + stage: 'deploy', + extends: '.first' + }, + + something: { + extends: '.first', + script: 'exec', + only: { variables: %w[$SOMETHING] } + }, + + '.first': { + script: 'run', + only: { kubernetes: 'active' } + } + } + end + + it 'extends a hash with a deep reverse merge' do + expect(subject.to_hash).to eq( + '.first': { + script: 'run', + only: { kubernetes: 'active' } + }, + + something: { + extends: '.first', + script: 'exec', + only: { + kubernetes: 'active', + variables: %w[$SOMETHING] + } + }, + + deploy: { + script: 'run', + stage: 'deploy', + only: { kubernetes: 'active' }, + extends: '.first' + }, + + build: { + extends: 'something', + script: 'exec', + stage: 'build', + only: { + kubernetes: 'active', + variables: %w[$SOMETHING] + } + }, + + test: { + extends: 'something', + script: 'ls', + only: { + refs: %w[master], + variables: %w[$SOMETHING], + kubernetes: 'active' + } + } + ) + end + end + + context 'when nested circular dependecy has been detected' do + let(:hash) do + { + test: { + extends: 'something', + script: 'ls', + only: { refs: %w[master] } + }, + + something: { + extends: '.first', + script: 'deploy', + only: { variables: %w[$SOMETHING] } + }, + + '.first': { + extends: 'something', + script: 'run', + only: { kubernetes: 'active' } + } + } + end + + it 'raises an error about circular dependency' do + expect { subject.to_hash } + .to raise_error(described_class::Entry::CircularDependencyError) + end + end + + context 'when circular dependecy to self has been detected' do + let(:hash) do + { + test: { + extends: 'test', + script: 'ls', + only: { refs: %w[master] } + } + } + end + + it 'raises an error about circular dependency' do + expect { subject.to_hash } + .to raise_error(described_class::Entry::CircularDependencyError) + end + end + + context 'when invalid extends value is specified' do + let(:hash) do + { something: { extends: 1, script: 'ls' } } + end + + it 'raises an error about invalid extension' do + expect { subject.to_hash } + .to raise_error(described_class::Entry::InvalidExtensionError) + end + end + + context 'when extensible entry has non-hash inheritance defined' do + let(:hash) do + { + test: { + extends: 'something', + script: 'ls', + only: { refs: %w[master] } + }, + + something: 'some text' + } + end + + it 'raises an error about invalid base' do + expect { subject.to_hash } + .to raise_error(described_class::Entry::InvalidExtensionError) + end + end + end +end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 2e204da307d..5a78ce783dd 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -1,4 +1,6 @@ -require 'spec_helper' +require 'fast_spec_helper' + +require_dependency 'active_model' describe Gitlab::Ci::Config do let(:config) do @@ -42,6 +44,36 @@ describe Gitlab::Ci::Config do end end + context 'when using extendable hash' do + let(:yml) do + <<-EOS + image: ruby:2.2 + + rspec: + script: rspec + + test: + extends: rspec + image: ruby:alpine + EOS + end + + it 'correctly extends the hash' do + hash = { + image: 'ruby:2.2', + rspec: { script: 'rspec' }, + test: { + extends: 'rspec', + image: 'ruby:alpine', + script: 'rspec' + } + } + + expect(config).to be_valid + expect(config.to_hash).to eq hash + end + end + context 'when config is invalid' do context 'when yml is incorrect' do let(:yml) { '// invalid' } @@ -49,7 +81,7 @@ describe Gitlab::Ci::Config do describe '.new' do it 'raises error' do expect { config }.to raise_error( - ::Gitlab::Ci::Config::Loader::FormatError, + described_class::ConfigError, /Invalid configuration format/ ) end @@ -75,5 +107,21 @@ describe Gitlab::Ci::Config do end end end + + context 'when invalid extended hash has been provided' do + let(:yml) do + <<-EOS + test: + extends: test + script: rspec + EOS + end + + it 'raises an error' do + expect { config }.to raise_error( + described_class::ConfigError, /circular dependency detected/ + ) + end + end end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index e73cdc54a15..fcbdf71a4e9 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -562,6 +562,58 @@ module Gitlab end end + context 'when using `extends`' do + let(:config_processor) { Gitlab::Ci::YamlProcessor.new(config) } + + subject { config_processor.builds.first } + + context 'when using simple `extends`' do + let(:config) do + <<~YAML + .template: + script: test + + rspec: + extends: .template + image: ruby:alpine + YAML + end + + it 'correctly extends rspec job' do + expect(config_processor.builds).to be_one + expect(subject.dig(:commands)).to eq 'test' + expect(subject.dig(:options, :image, :name)).to eq 'ruby:alpine' + end + end + + context 'when using recursive `extends`' do + let(:config) do + <<~YAML + rspec: + extends: .test + script: rspec + when: always + + .template: + before_script: + - bundle install + + .test: + extends: .template + script: test + image: image:test + YAML + end + + it 'correctly extends rspec job' do + expect(config_processor.builds).to be_one + expect(subject.dig(:commands)).to eq "bundle install\nrspec" + expect(subject.dig(:options, :image, :name)).to eq 'image:test' + expect(subject.dig(:when)).to eq 'always' + end + end + end + describe "When" do %w(on_success on_failure always).each do |when_state| it "returns #{when_state} when defined" do @@ -1309,6 +1361,14 @@ module Gitlab .to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:rspec:only variables invalid expression syntax') end + + it 'returns errors if extended hash configuration is invalid' do + config = YAML.dump({ rspec: { extends: 'something', script: 'test' } }) + + expect { Gitlab::Ci::YamlProcessor.new(config) } + .to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, + 'rspec: unknown key in `extends`') + end end describe "Validate configuration templates" do |