Return CI config exception data as JSON string

- Removed custom Exception classes from Extendable:
  - InvalidExtensionError
  - CircularDependencyError
  - NestingTooDeepError
- Spec refactors for single Exception type

3 files changed, 45 insertions, 53 deletions

diff --git a/lib/gitlab/ci/config/extendable/entry.rb b/lib/gitlab/ci/config/extendable/entry.rb
index 0001a259281..8d39a37c0c9 100644
--- a/lib/gitlab/ci/config/extendable/entry.rb
+++ b/lib/gitlab/ci/config/extendable/entry.rb
@@ -7,10 +7,6 @@ module Gitlab
         class Entry
           include Gitlab::Utils::StrongMemoize
 
-          InvalidExtensionError = Class.new(Extendable::ExtensionError)
-          CircularDependencyError = Class.new(Extendable::ExtensionError)
-          NestingTooDeepError = Class.new(Extendable::ExtensionError)
-
           MAX_NESTING_LEVELS = 10
 
           attr_reader :key
@@ -19,6 +15,7 @@ module Gitlab
             @key = key
             @context = context
             @parent = parent
+            @errors = []
 
             unless @context.key?(@key)
               raise StandardError, 'Invalid entry key!'
@@ -62,34 +59,36 @@ module Gitlab
           def extend!
             return value unless extensible?
 
+            validate!
+
+            merged = {}
+            base_hashes!.each { |h| merged.deep_merge!(h) }
+
+            @context[key] = merged.deep_merge!(value)
+          end
+
+          private
+
+          def validate!
             if unknown_extensions.any?
-              raise Entry::InvalidExtensionError,
-                    "#{key}: unknown keys in `extends` (#{show_keys(unknown_extensions)})"
+              @errors << { message: 'unknown keys found in `extends`', unknown_keys: show_keys(unknown_extensions) }
             end
 
             if invalid_bases.any?
-              raise Entry::InvalidExtensionError,
-                    "#{key}: invalid base hashes in `extends` (#{show_keys(invalid_bases)})"
+              @errors << { message: 'invalid base hashes in `extends`', unknown_keys: show_keys(invalid_bases) }
             end
 
             if nesting_too_deep?
-              raise Entry::NestingTooDeepError,
-                    "#{key}: nesting too deep in `extends`"
+              @errors << { message: 'nesting too deep in `extends`' }
             end
 
             if circular_dependency?
-              raise Entry::CircularDependencyError,
-                    "#{key}: circular dependency detected in `extends`"
+              @errors << { message: 'circular dependency detected in `extends`' }
             end
 
-            merged = {}
-            base_hashes!.each { |h| merged.deep_merge!(h) }
-
-            @context[key] = merged.deep_merge!(value)
+            raise Extendable::ExtensionError, json_error_message if @errors.any?
           end
 
-          private
-
           def show_keys(keys)
             keys.join(', ')
           end
@@ -113,6 +112,10 @@ module Gitlab
               extends_keys.reject { |key| @context[key].is_a?(Hash) }
             end
           end
+
+          def json_error_message
+            { key: key, context: @context, errors: @errors }.to_json
+          end
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/extendable_spec.rb b/spec/lib/gitlab/ci/config/extendable_spec.rb
index 90213f6603d..739a6a28f8d 100644
--- a/spec/lib/gitlab/ci/config/extendable_spec.rb
+++ b/spec/lib/gitlab/ci/config/extendable_spec.rb
@@ -1,7 +1,7 @@
-require 'fast_spec_helper'
+require 'spec_helper'
 
 describe Gitlab::Ci::Config::Extendable do
-  subject { described_class.new(hash) }
+  let(:extendable) { described_class.new(hash) }
 
   describe '#each' do
     context 'when there is extendable entry in the hash' do
@@ -14,12 +14,14 @@ describe Gitlab::Ci::Config::Extendable do
       end
 
       it 'yields control' do
-        expect { |b| subject.each(&b) }.to yield_control
+        expect { |b| extendable.each(&b) }.to yield_control
       end
     end
   end
 
   describe '#to_hash' do
+    subject { extendable.to_hash }
+
     context 'when hash does not contain extensions' do
       let(:hash) do
         {
@@ -32,7 +34,7 @@ describe Gitlab::Ci::Config::Extendable do
       end
 
       it 'does not modify the hash' do
-        expect(subject.to_hash).to eq hash
+        expect(subject).to eq hash
       end
     end
 
@@ -43,7 +45,6 @@ describe Gitlab::Ci::Config::Extendable do
             script: 'deploy',
             only: { variables: %w[$SOMETHING] }
           },
-
           test: {
             extends: 'something',
             script: 'ls',
@@ -53,12 +54,11 @@ describe Gitlab::Ci::Config::Extendable do
       end
 
       it 'extends a hash with a deep reverse merge' do
-        expect(subject.to_hash).to eq(
+        expect(subject).to eq(
           something: {
             script: 'deploy',
             only: { variables: %w[$SOMETHING] }
           },
-
           test: {
             extends: 'something',
             script: 'ls',
@@ -79,23 +79,19 @@ describe Gitlab::Ci::Config::Extendable do
             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' }
@@ -104,12 +100,11 @@ describe Gitlab::Ci::Config::Extendable do
       end
 
       it 'extends a hash with a deep reverse merge' do
-        expect(subject.to_hash).to eq(
+        expect(subject).to eq(
           '.first': {
             script: 'run',
             only: { kubernetes: 'active' }
           },
-
           something: {
             extends: '.first',
             script: 'exec',
@@ -118,14 +113,12 @@ describe Gitlab::Ci::Config::Extendable do
               variables: %w[$SOMETHING]
             }
           },
-
           deploy: {
             script: 'run',
             stage: 'deploy',
             only: { kubernetes: 'active' },
             extends: '.first'
           },
-
           build: {
             extends: 'something',
             script: 'exec',
@@ -135,7 +128,6 @@ describe Gitlab::Ci::Config::Extendable do
               variables: %w[$SOMETHING]
             }
           },
-
           test: {
             extends: 'something',
             script: 'ls',
@@ -157,13 +149,11 @@ describe Gitlab::Ci::Config::Extendable do
             script: 'ls',
             only: { refs: %w[master] }
           },
-
           something: {
             extends: '.first',
             script: 'deploy',
             only: { variables: %w[$SOMETHING] }
           },
-
           '.first': {
             extends: 'something',
             script: 'run',
@@ -172,9 +162,8 @@ describe Gitlab::Ci::Config::Extendable do
         }
       end
 
-      it 'raises an error about circular dependency' do
-        expect { subject.to_hash }
-          .to raise_error(described_class::Entry::CircularDependencyError)
+      it 'raises an ExtensionError' do
+        expect { subject }.to raise_error(described_class::ExtensionError)
       end
     end
 
@@ -189,9 +178,8 @@ describe Gitlab::Ci::Config::Extendable do
         }
       end
 
-      it 'raises an error about circular dependency' do
-        expect { subject.to_hash }
-          .to raise_error(described_class::Entry::CircularDependencyError)
+      it 'raises an ExtensionError' do
+        expect { subject }.to raise_error(described_class::ExtensionError)
       end
     end
 
@@ -200,9 +188,8 @@ describe Gitlab::Ci::Config::Extendable 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)
+      it 'raises an ExtensionError' do
+        expect { subject }.to raise_error(described_class::ExtensionError)
       end
     end
 
@@ -214,14 +201,12 @@ describe Gitlab::Ci::Config::Extendable do
             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)
+      it 'raises an ExtensionError' do
+        expect { subject }.to raise_error(described_class::ExtensionError)
       end
     end
   end
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index 789d6813d5b..2e53b9a0592 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -1513,12 +1513,16 @@ module Gitlab
                             'jobs:rspec:only changes should be an array of strings')
         end
 
-        it 'returns errors if extended hash configuration is invalid' do
+        it 'raises an exception 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 keys in `extends` (something)')
+          expect { Gitlab::Ci::YamlProcessor.new(config) }.to raise_error do |error|
+            expect(error).to be_a(Gitlab::Ci::YamlProcessor::ValidationError)
+
+            %w(key context errors).each do |key|
+              expect(JSON.parse(error.message)[key]).to be_present
+            end
+          end
         end
       end