Handle conflicting stage mergesmc/feature/merge-stages-includes

Throws an error when attempting to merge conflicting stage
configurations: e.g. ['build', 'test'] and ['test', build']

3 files changed, 26 insertions, 5 deletions

diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index cde042c5e0a..565c557a777 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -11,7 +11,8 @@ module Gitlab
       RESCUE_ERRORS = [
         Gitlab::Config::Loader::FormatError,
         Extendable::ExtensionError,
-        External::Processor::IncludeError
+        External::Processor::IncludeError,
+        Normalize::ConfigMerger::StageMergeError
       ].freeze
 
       attr_reader :root
diff --git a/lib/gitlab/ci/config/normalize/config_merger.rb b/lib/gitlab/ci/config/normalize/config_merger.rb
index 91dcea9d683..afc2e10b086 100644
--- a/lib/gitlab/ci/config/normalize/config_merger.rb
+++ b/lib/gitlab/ci/config/normalize/config_merger.rb
@@ -5,6 +5,8 @@ module Gitlab
     class Config
       module Normalize
         class ConfigMerger
+          StageMergeError = Class.new(StandardError)
+
           def initialize(base_config, additional_config)
             @base_config = base_config
             @additional_config = additional_config
@@ -12,7 +14,7 @@ module Gitlab
 
           def merge
             @base_config.deep_merge(@additional_config).tap do |config|
-              config[:stages] = normalize_stages if both_configs_have_stages? && Feature.enabled?(:merge_stages_accross_includes)
+              config[:stages] = normalize_stages if both_configs_have_stages? && Feature.enabled?(:merge_stages_across_includes)
             end
           end
 
@@ -40,6 +42,8 @@ module Gitlab
             Gitlab::Utils::TopologicalSort.new.tap do |thash|
               all_stages.each { |stages| graph_stages(stages, thash) }
             end.tsort
+          rescue TSort::Cyclic => e
+            raise StageMergeError, e.message
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/normalize/config_merger_spec.rb b/spec/lib/gitlab/ci/config/normalize/config_merger_spec.rb
index a9ec702d21f..6953a3bf5e4 100644
--- a/spec/lib/gitlab/ci/config/normalize/config_merger_spec.rb
+++ b/spec/lib/gitlab/ci/config/normalize/config_merger_spec.rb
@@ -1,12 +1,19 @@
 # frozen_string_literal: true
 
-require 'fast_spec_helper'
+require 'spec_helper'
 
 describe Gitlab::Ci::Config::Normalize::ConfigMerger do
-  let(:base_config) { { stages: %w[build test deploy production], job_name: { script: 'echo hello' } } }
-  let(:additional_config) { { stages: %w[deploy staging production], other_job_name: { script: 'echo other_hello' } } }
+  let(:base_config) { { stages: base_stages, job_name: { script: 'echo hello' } } }
+  let(:additional_config) { { stages: additional_stages, other_job_name: { script: 'echo other_hello' } } }
+
+  before do
+    stub_feature_flags(merge_stages_across_includes: true)
+  end
 
   describe '#merge' do
+    let(:base_stages) { %w[build test deploy production] }
+    let(:additional_stages) { %w[deploy staging production] }
+
     subject { described_class.new(base_config, additional_config).merge }
 
     it 'deep merges everything except stages' do
@@ -18,5 +25,14 @@ describe Gitlab::Ci::Config::Normalize::ConfigMerger do
     it 'tsort merges stages' do
       expect(subject[:stages]).to eq(%w[build test deploy staging production])
     end
+
+    context 'when the stages configuration is conflicting' do
+      let(:base_stages) { %w[build test] }
+      let(:additional_stages) { %w[test build] }
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(described_class::StageMergeError)
+      end
+    end
   end
 end