Validate need types in config

Validates need types in Job and Bridge config.

- Only Pipeline type needs are allowed in regular jobs
- Only Bridge and Pipeline type needs are allowed in bridge jobs
- Only one Bridge type need is allowed in a bridge job

ee/lib/ee/gitlab/ci/config/entry/bridge.rb

@@ -29,6 +29,22 @@ module EE
                 end
               end
 
+              validate do
+                next unless needs.present?
+
+                if [needs].flatten.any? { |need| !(Entry::Need::Bridge.matching?(need) || ::Gitlab::Ci::Config::Entry::Need::Pipeline.matching?(need)) }
+                  errors.add(:needs, 'can only have bridge or pipeline type needs')
+                end
+              end
+
+              validate do
+                next unless needs.present?
+
+                if [needs].flatten.count { |need| Entry::Need::Bridge.matching?(need) } > 1
+                  errors.add(:needs, 'can only have one bridge type needs')
+                end
+              end
+
               with_options allow_nil: true do
                 validates :when,
                   inclusion: { in: %w[on_success on_failure always],

ee/lib/ee/gitlab/ci/config/entry/need.rb

@@ -25,17 +25,12 @@ module EE
                 validates :pipeline, type: String, presence: true
               end
 
-              def type
-                :bridge
+              def self.matching?(config)
+                config.is_a?(Hash)
               end
-            end
 
-            module UnknownStrategy
-              extend ::Gitlab::Utils::Override
-
-              override :errors
-              def errors
-                ["#{location} has to be a string, symbol or hash"]
+              def type
+                :bridge
               end
             end
           end

ee/lib/ee/gitlab/ci/config/entry/needs.rb

+# frozen_string_literal: true
+
+module EE
+  module Gitlab
+    module Ci
+      module Config
+        module Entry
+          module Needs
+            extend ActiveSupport::Concern
+
+            class_methods do
+              extend ::Gitlab::Utils::Override
+
+              override :all_types
+              def all_types
+                super + [Entry::Need::Bridge]
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb

@@ -160,6 +160,38 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
       end
     end
 
+    context 'when bridge has bridge and pipeline needs' do
+      let(:config) do
+        {
+          trigger: 'other-project',
+          needs: ['some_job', { pipeline: 'some/other_project' }]
+        }
+      end
+
+      describe '#valid?' do
+        it { is_expected.to be_valid }
+      end
+    end
+
+    context 'when bridge has more than one valid bridge needs' do
+      let(:config) do
+        {
+          trigger: 'other-project',
+          needs: [{ pipeline: 'some/project' }, { pipeline: 'some/other_project' }]
+        }
+      end
+
+      describe '#valid?' do
+        it { is_expected.not_to be_valid }
+      end
+
+      describe '#errors' do
+        it 'returns an error about too many bridge needs' do
+          expect(subject.errors).to contain_exactly('bridge needs can only have one bridge type needs')
+        end
+      end
+    end
+
     context 'when bridge config contains unknown keys' do
       let(:config) { { unknown: 123 } }
 

ee/spec/lib/ee/gitlab/ci/config/entry/need_spec.rb

@@ -28,23 +28,8 @@ describe ::Gitlab::Ci::Config::Entry::Need do
 
     describe '#errors' do
       it 'is returns an error about an empty config' do
-        expect(subject.errors.first)
-          .to end_with("bridge config can't be blank")
-      end
-    end
-  end
-
-  context 'when need is the wrong type' do
-    let(:config) { 123 }
-
-    describe '#valid?' do
-      it { is_expected.not_to be_valid }
-    end
-
-    describe '#errors' do
-      it 'is returns an error about an empty config' do
-        expect(subject.errors.first)
-          .to end_with('has to be a string, symbol or hash')
+        expect(subject.errors)
+          .to include("bridge config can't be blank")
       end
     end
   end

ee/spec/lib/ee/gitlab/ci/config/entry/needs_spec.rb

@@ -27,10 +27,8 @@ describe ::Gitlab::Ci::Config::Entry::Needs do
 
       describe '#errors' do
         it 'returns error about incorrect type' do
-          error_message = Gitlab.ee? ? 'need has to be a string, symbol or hash' : 'need has to be a string or symbol'
-
           expect(needs.errors)
-            .to include error_message
+            .to contain_exactly('needs need has an unsupported type')
         end
       end
     end
@@ -54,7 +52,7 @@ describe ::Gitlab::Ci::Config::Entry::Needs do
 
       describe '#value' do
         it 'returns key value' do
-          expect(needs.value).to eq(pipeline: ['first_job_name'], bridge: { pipeline: 'some/project' })
+          expect(needs.value).to eq(pipeline: [{ name: 'first_job_name' }], bridge: { pipeline: 'some/project' })
         end
       end
 

lib/gitlab/ci/config/entry/job.rb

@@ -57,6 +57,14 @@ module Gitlab
             validates :start_in, duration: { limit: '1 day' }, if: :delayed?
             validates :start_in, absence: true, if: -> { has_rules? || !delayed? }
 
+            validate do
+              next unless needs.present?
+
+              if [needs].flatten.any? { |need| !Entry::Need::Pipeline.matching?(need) }
+                errors.add(:needs, 'can only have pipeline type needs')
+              end
+            end
+
             validate do
               next unless dependencies.present?
               next unless needs.present?

lib/gitlab/ci/config/entry/need.rb

@@ -14,6 +14,10 @@ module Gitlab
               validates :config, presence: true
             end
 
+            def self.matching?(config)
+              config.is_a?(String) || config.is_a?(Symbol)
+            end
+
             def type
               :pipeline
             end
@@ -24,9 +28,6 @@ module Gitlab
           end
 
           class UnknownStrategy < ::Gitlab::Config::Entry::Node
-            def errors
-              ["#{location} has to be a string or symbol"]
-            end
           end
         end
       end
@@ -34,5 +35,4 @@ module Gitlab
   end
 end
 
-::Gitlab::Ci::Config::Entry::Need.prepend_if_ee('::EE::Gitlab::Ci::Config::Entry::Need') # rubocop: disable Cop/InjectEnterpriseEditionModule
-::Gitlab::Ci::Config::Entry::Need::UnknownStrategy.prepend_if_ee('::EE::Gitlab::Ci::Config::Entry::Need::UnknownStrategy')
+::Gitlab::Ci::Config::Entry::Need.prepend_if_ee('::EE::Gitlab::Ci::Config::Entry::Need')

lib/gitlab/ci/config/entry/needs.rb

@@ -12,6 +12,28 @@ module Gitlab
 
           validations do
             validates :config, presence: true
+
+            validate do
+              [config].flatten.each do |need|
+                if Needs.find_type(need).nil?
+                  errors.add(:need, 'has an unsupported type')
+                end
+              end
+            end
+          end
+
+          TYPES = [Entry::Need::Pipeline].freeze
+
+          private_constant :TYPES
+
+          def self.all_types
+            TYPES
+          end
+
+          def self.find_type(config)
+            self.all_types.find do |type|
+              type.matching?(config)
+            end
           end
 
           def compose!(deps = nil)
@@ -30,12 +52,18 @@ module Gitlab
           end
 
           def value
-            @entries.values.group_by(&:type).transform_values do |values|
+            values = @entries.values.group_by(&:type).transform_values do |values|
               values.map(&:value)
             end
+
+            values.tap do |values_hash|
+              values_hash[:bridge] = values_hash[:bridge].first if values_hash[:bridge]
+            end
           end
         end
       end
     end
   end
 end
+
+::Gitlab::Ci::Config::Entry::Needs.prepend_if_ee('::EE::Gitlab::Ci::Config::Entry::Needs')

lib/gitlab/ci/yaml_processor.rb

@@ -164,12 +164,14 @@ module Gitlab
         stage_index = @stages.index(job[:stage])
 
         job[:needs][:pipeline].each do |need|
-          raise ValidationError, "#{name} job: undefined need: #{need}" unless @jobs[need.to_sym]
+          need_job_name = need[:name]
 
-          needs_stage_index = @stages.index(@jobs[need.to_sym][:stage])
+          raise ValidationError, "#{name} job: undefined need: #{need_job_name}" unless @jobs[need_job_name.to_sym]
+
+          needs_stage_index = @stages.index(@jobs[need_job_name.to_sym][:stage])
 
           unless needs_stage_index.present? && needs_stage_index < stage_index
-            raise ValidationError, "#{name} job: need #{need} is not defined in prior stages"
+            raise ValidationError, "#{name} job: need #{need_job_name} is not defined in prior stages"
           end
         end
       end

spec/lib/gitlab/ci/config/entry/job_spec.rb

@@ -413,6 +413,21 @@ describe Gitlab::Ci::Config::Entry::Job do
             expect(entry.errors).to include 'job config missing required keys: stage'
           end
         end
+
+        context 'when needs is bridge type' do
+          let(:config) do
+            {
+              script: 'echo',
+              stage: 'test',
+              needs: { pipeline: 'some/project' }
+            }
+          end
+
+          it 'returns error about invalid needs type' do
+            expect(entry).not_to be_valid
+            expect(entry.errors).to contain_exactly('job needs can only have pipeline type needs')
+          end
+        end
       end
 
       context 'when timeout value is not correct' do

spec/lib/gitlab/ci/config/entry/need_spec.rb

@@ -14,7 +14,7 @@ describe ::Gitlab::Ci::Config::Entry::Need do
 
     describe '#value' do
       it 'returns job needs configuration' do
-        expect(need.value).to eq('job_name')
+        expect(need.value).to eq(name: 'job_name')
       end
     end
   end
@@ -28,7 +28,7 @@ describe ::Gitlab::Ci::Config::Entry::Need do
 
     describe '#value' do
       it 'returns job needs configuration' do
-        expect(need.value).to eq('job_name')
+        expect(need.value).to eq(name: :job_name)
       end
     end
   end
@@ -42,25 +42,8 @@ describe ::Gitlab::Ci::Config::Entry::Need do
 
     describe '#errors' do
       it 'is returns an error about an empty config' do
-        expect(need.errors.first)
-          .to end_with("pipeline config can't be blank")
-      end
-    end
-  end
-
-  context 'when need is not a string' do
-    let(:config) { 123 }
-
-    describe '#valid?' do
-      it { is_expected.not_to be_valid }
-    end
-
-    describe '#errors' do
-      it 'is returns an error about an empty config' do
-        error_message = Gitlab.ee? ? 'has to be a string, symbol or hash' : 'has to be a string or symbol'
-
-        expect(need.errors.first)
-          .to end_with(error_message)
+        expect(need.errors)
+          .to contain_exactly("pipeline config can't be blank")
       end
     end
   end

spec/lib/gitlab/ci/config/entry/needs_spec.rb

@@ -27,10 +27,8 @@ describe ::Gitlab::Ci::Config::Entry::Needs do
 
       describe '#errors' do
         it 'returns error about incorrect type' do
-          error_message = Gitlab.ee? ? 'need has to be a string, symbol or hash' : 'need has to be a string or symbol'
-
           expect(needs.errors)
-            .to include error_message
+            .to contain_exactly('needs need has an unsupported type')
         end
       end
     end
@@ -46,7 +44,7 @@ describe ::Gitlab::Ci::Config::Entry::Needs do
 
       describe '#value' do
         it 'returns key value' do
-          expect(needs.value).to eq(pipeline: %w[first_job_name second_job_name])
+          expect(needs.value).to eq(pipeline: [{ name: 'first_job_name' }, { name: 'second_job_name' }])
         end
       end