Move Needs entry to core

Moves the Needs entry to core, because it's required for use in DAG
needs.

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

@@ -41,7 +41,7 @@ module EE
               description: 'CI/CD Bridge downstream trigger definition.',
               inherit: false
 
-            entry :needs, ::EE::Gitlab::Ci::Config::Entry::Needs,
+            entry :needs, ::Gitlab::Ci::Config::Entry::Needs,
               description: 'CI/CD Bridge needs dependency definition.',
               inherit: false
 

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

@@ -5,14 +5,14 @@ module EE
     module Ci
       module Config
         module Entry
-          ##
-          # Entry that represents a cross-project needs dependency.
-          #
-          class Needs < ::Gitlab::Config::Entry::Simplifiable
-            strategy :BridgeNeeds, if: -> (config) { config.is_a?(Hash) }
-            strategy :ComplexNeeds, if: -> (config) { config.is_a?(Array) }
+          module Need
+            extend ActiveSupport::Concern
 
-            class BridgeNeeds < ::Gitlab::Config::Entry::Node
+            prepended do
+              strategy :Bridge, if: -> (config) { config.is_a?(Hash) }
+            end
+
+            class Bridge < ::Gitlab::Config::Entry::Node
               include ::Gitlab::Config::Entry::Validatable
               include ::Gitlab::Config::Entry::Attributable
 
@@ -24,43 +24,22 @@ module EE
                 validates :config, allowed_keys: ALLOWED_KEYS
                 validates :pipeline, type: String, presence: true
               end
-            end
-
-            class ComplexNeeds < ::Gitlab::Config::Entry::Node
-              include ::Gitlab::Config::Entry::Validatable
-
-              validations do
-                validates :config, presence: true
-                validates :config, type: Array
-                validate :one_needs_pipeline
-                validate :needs_array_elements
-              end
 
-              def one_needs_pipeline
-                if config.count { |element| element.is_a?(Hash) } > 1
-                  errors.add(:needs, 'needs hash element needs to have a pipeline key')
+              def bridge?
+                true
               end
-              end
-
-              def needs_array_elements
-                config.each do |element|
-                  next if element.is_a?(String)
 
-                  unless element.is_a?(Hash) && element[:pipeline]
-                    errors.add(:needs, 'needs hash element needs to have a pipeline key')
-                  end
+              def pipeline?
+                false
               end
             end
 
-              def value
-                bridge, pipeline = config.partition { |element| element.is_a?(Hash) }
-                { bridge: bridge.first, pipeline: pipeline }
-              end
-            end
+            module UnknownStrategy
+              extend ::Gitlab::Utils::Override
 
-            class UnknownStrategy < ::Gitlab::Config::Entry::Node
+              override :errors
               def errors
-                ["#{location} has to be either an array of conditions or a hash"]
+                ["#{location} has to be a string, symbol or hash"]
               end
             end
           end

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

@@ -88,7 +88,7 @@ describe EE::Gitlab::Ci::Config::Entry::Bridge do
       describe '#value' do
         it 'is returns a bridge job configuration' do
           expect(subject.value).to eq(name: :my_bridge,
-                                      needs: { pipeline: 'some/project' },
+                                      needs: { bridge: { pipeline: 'some/project' } },
                                       ignore: false,
                                       stage: 'test',
                                       only: { refs: %w[branches tags] })

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::Gitlab::Ci::Config::Entry::Need do
+  subject { described_class.new(config) }
+
+  context 'when upstream is specified' do
+    let(:config) { { pipeline: 'some/project' } }
+
+    describe '#valid?' do
+      it { is_expected.to be_valid }
+    end
+
+    describe '#value' do
+      it 'returns job needs configuration' do
+        expect(subject.value).to eq(pipeline: 'some/project')
+      end
+    end
+  end
+
+  context 'when need is empty' do
+    let(:config) { {} }
+
+    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("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')
+      end
+    end
+  end
+end

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

 # frozen_string_literal: true
 
-require 'fast_spec_helper'
-require_dependency 'active_model'
+require 'spec_helper'
 
-describe EE::Gitlab::Ci::Config::Entry::Needs do
-  subject { described_class.new(config) }
+describe ::Gitlab::Ci::Config::Entry::Needs do
+  subject(:needs) { described_class.new(config) }
 
-  context 'when needs is a bridge needs' do
-    context 'when upstream config is a non-empty string' do
-      let(:config) { { pipeline: 'some/project' } }
+  describe 'validations' do
+    before do
+      needs.compose!
+    end
+
+    context 'when entry config value is correct' do
+      let(:config) { ['job_name', pipeline: 'some/project'] }
 
       describe '#valid?' do
         it { is_expected.to be_valid }
       end
-
-      describe '#value' do
-        it 'is returns a upstream configuration hash' do
-          expect(subject.value).to eq(pipeline: 'some/project')
-        end
-      end
     end
 
-    context 'when upstream config is not a string' do
-      let(:config) { { pipeline: 123 } }
+    context 'when wrong needs type is used' do
+      let(:config) { ['job_name', { pipeline: 'some/project' }, 123] }
 
       describe '#valid?' do
         it { is_expected.not_to be_valid }
       end
 
       describe '#errors' do
-        it 'returns an error message' do
-          expect(subject.errors.first)
-            .to eq('bridge needs pipeline should be a string')
-        end
+        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
         end
       end
     end
 
-  context 'when needs is a complex needs' do
-    let(:config) { ['test', { pipeline: 'test' }] }
+    context 'when bridge needs has wrong attributes' do
+      let(:config) { ['job_name', project: 'some/project'] }
 
-    it 'test' do
-      subject
+      describe '#valid?' do
+        it { is_expected.not_to be_valid }
+      end
     end
   end
 
-  context 'when needs is empty' do
-    let(:config) { '' }
+  describe '.compose!' do
+    context 'when valid job entries composed' do
+      let(:config) { ['first_job_name', pipeline: 'some/project'] }
 
-    describe '#valid?' do
-      it { is_expected.not_to be_valid }
+      before do
+        needs.compose!
       end
 
-    describe '#errors' do
-      it 'is returns an error about an empty config' do
-        expect(subject.errors.first)
-          .to end_with('has to be either an array of conditions or a hash')
+      describe '#value' do
+        it 'returns key value' do
+          expect(needs.value).to eq(pipeline: ['first_job_name'], bridge: { pipeline: 'some/project' })
+        end
+      end
+
+      describe '#descendants' do
+        it 'creates valid descendant nodes' do
+          expect(needs.descendants.count).to eq 2
+          expect(needs.descendants)
+            .to all(be_an_instance_of(::Gitlab::Ci::Config::Entry::Need))
+        end
       end
     end
   end

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

@@ -50,7 +50,6 @@ module Gitlab
               validates :timeout, duration: { limit: ChronicDuration.output(Project::MAX_BUILD_TIMEOUT) }
 
               validates :dependencies, array_of_strings: true
-              validates :needs, array_of_strings: true
               validates :extends, array_of_strings_or_string: true
               validates :rules, array_of_hashes: true
             end
@@ -114,6 +113,9 @@ module Gitlab
             description: 'List of evaluable Rules to determine job inclusion.',
             inherit: false
 
+          entry :needs, Entry::Needs,
+            description: 'Needs configuration for this job.'
+
           entry :variables, Entry::Variables,
             description: 'Environment variables available for this job.',
             inherit: false

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

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        class Need < ::Gitlab::Config::Entry::Simplifiable
+          strategy :Pipeline, if: -> (config) { config.is_a?(String) || config.is_a?(Symbol) }
+
+          class Pipeline < ::Gitlab::Config::Entry::Node
+            include ::Gitlab::Config::Entry::Validatable
+
+            validations do
+              validates :config, presence: true
+            end
+
+            def bridge?
+              false
+            end
+
+            def pipeline?
+              true
+            end
+
+            def value
+              @config.to_s
+            end
+          end
+
+          class UnknownStrategy < ::Gitlab::Config::Entry::Node
+            def errors
+              ["#{location} has to be a string or symbol"]
+            end
+          end
+        end
+      end
+    end
+  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')

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

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    class Config
+      module Entry
+        ##
+        # Entry that represents a set of needs dependencies.
+        #
+        class Needs < ::Gitlab::Config::Entry::Node
+          include ::Gitlab::Config::Entry::Validatable
+
+          validations do
+            validates :config, presence: true
+          end
+
+          def compose!(deps = nil)
+            super(deps) do
+              [].tap { |array| array.push(@config) }.flatten.each_with_index do |need, index|
+                @entries[index] = ::Gitlab::Config::Entry::Factory.new(Entry::Need)
+                  .value(need)
+                  .with(key: "need", parent: self, description: "need definition.") # rubocop:disable CodeReuse/ActiveRecord
+                  .create!
+              end
+
+              @entries.each_value do |entry|
+                entry.compose!(deps)
+              end
+            end
+          end
+
+          def value
+            {}.tap do |result_hash|
+              result_hash[:bridge] = bridge_values.first if bridge_values.any?
+              result_hash[:pipeline] = pipeline_values if pipeline_values.any?
+            end
+          end
+
+          private
+
+          def bridge_values
+            @entries.values.select(&:bridge?).map(&:value)
+          end
+
+          def pipeline_values
+            @entries.values.select(&:pipeline?).map(&:value)
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/yaml_processor.rb

@@ -40,7 +40,7 @@ module Gitlab
           environment: job[:environment_name],
           coverage_regex: job[:coverage],
           yaml_variables: yaml_variables(name),
-          needs_attributes: job[:needs][:pipeline]&.map { |need| { name: need } },
+          needs_attributes: job.dig(:needs, :pipeline)&.map { |need| { name: need } },
           interruptible: job[:interruptible],
           rules: job[:rules],
           options: {
@@ -59,7 +59,7 @@ module Gitlab
             instance: job[:instance],
             start_in: job[:start_in],
             trigger: job[:trigger],
-            bridge_needs: job[:needs][:bridge]
+            bridge_needs: job.dig(:needs, :bridge)
           }.compact }.compact
       end
 
@@ -159,7 +159,7 @@ module Gitlab
       end
 
       def validate_job_needs!(name, job)
-        return unless job[:needs][:pipeline]
+        return unless job.dig(:needs, :pipeline)
 
         stage_index = @stages.index(job[:stage])
 

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

@@ -23,7 +23,7 @@ describe Gitlab::Ci::Config::Entry::Job do
 
       let(:result) do
         %i[before_script script stage type after_script cache
-           image services only except rules variables artifacts
+           image services only except rules needs variables artifacts
            environment coverage retry]
       end
 
@@ -384,21 +384,6 @@ describe Gitlab::Ci::Config::Entry::Job do
       end
 
       context 'when has needs' do
-        context 'that are not a array of strings' do
-          let(:config) do
-            {
-              stage: 'test',
-              script: 'echo',
-              needs: 'build-job'
-            }
-          end
-
-          it 'returns error about invalid type' do
-            expect(entry).not_to be_valid
-            expect(entry.errors).to include 'job needs should be an array of strings'
-          end
-        end
-
         context 'when have dependencies that are not subset of needs' do
           let(:config) do
             {

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::Gitlab::Ci::Config::Entry::Need do
+  subject(:need) { described_class.new(config) }
+
+  context 'when job is specified' do
+    let(:config) { 'job_name' }
+
+    describe '#valid?' do
+      it { is_expected.to be_valid }
+    end
+
+    describe '#value' do
+      it 'returns job needs configuration' do
+        expect(need.value).to eq('job_name')
+      end
+    end
+  end
+
+  context 'when job is specified as symbol' do
+    let(:config) { :job_name }
+
+    describe '#valid?' do
+      it { is_expected.to be_valid }
+    end
+
+    describe '#value' do
+      it 'returns job needs configuration' do
+        expect(need.value).to eq('job_name')
+      end
+    end
+  end
+
+  context 'when need is empty' do
+    let(:config) { '' }
+
+    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(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)
+      end
+    end
+  end
+end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::Gitlab::Ci::Config::Entry::Needs do
+  subject(:needs) { described_class.new(config) }
+
+  describe 'validations' do
+    before do
+      needs.compose!
+    end
+
+    context 'when entry config value is correct' do
+      let(:config) { ['job_name'] }
+
+      describe '#valid?' do
+        it { is_expected.to be_valid }
+      end
+    end
+
+    context 'when wrong needs type is used' do
+      let(:config) { [123] }
+
+      describe '#valid?' do
+        it { is_expected.not_to be_valid }
+      end
+
+      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
+        end
+      end
+    end
+  end
+
+  describe '.compose!' do
+    context 'when valid job entries composed' do
+      let(:config) { %w[first_job_name second_job_name] }
+
+      before do
+        needs.compose!
+      end
+
+      describe '#value' do
+        it 'returns key value' do
+          expect(needs.value).to eq(pipeline: %w[first_job_name second_job_name])
+        end
+      end
+
+      describe '#descendants' do
+        it 'creates valid descendant nodes' do
+          expect(needs.descendants.count).to eq 2
+          expect(needs.descendants)
+            .to all(be_an_instance_of(::Gitlab::Ci::Config::Entry::Need))
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -1293,12 +1293,7 @@ module Gitlab
               stage: "test",
               stage_idx: 2,
               name: "test1",
-              options: {
-                script: ["test"],
-                # This does not make sense, there is a follow-up:
-                # https://gitlab.com/gitlab-org/gitlab-foss/issues/65569
-                bridge_needs: %w[build1 build2]
-              },
+              options: { script: ["test"] },
               needs_attributes: [
                 { name: "build1" },
                 { name: "build2" }