Merge branch 'feature/gb/allow-to-extend-keys-in-gitlab-ci-yml' into 'master'

Add support for advanced CI/CD config extension with `extends:`

Closes gitlab-ee#6136

See merge request gitlab-org/gitlab-ce!21243

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

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

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

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

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

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

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
 

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

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

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

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

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