Require `needs:` to be presentrequire-needs-to-be-present

This changes the `needs:` logic to require
that all jobs to be present. Instead of skipping
do fail the pipeline creation if `needs:` dependency
is not found.

7 files changed, 74 insertions, 23 deletions

diff --git a/lib/gitlab/ci/pipeline/chain/populate.rb b/lib/gitlab/ci/pipeline/chain/populate.rb
index 0405292a25b..65029f5ce7f 100644
--- a/lib/gitlab/ci/pipeline/chain/populate.rb
+++ b/lib/gitlab/ci/pipeline/chain/populate.rb
@@ -23,12 +23,17 @@ module Gitlab
             @command.seeds_block&.call(pipeline)
 
             ##
-            # Populate pipeline with all stages, and stages with builds.
+            # Gather all runtime build/stage errors
             #
-            pipeline.stage_seeds.each do |stage|
-              pipeline.stages << stage.to_resource
+            if seeds_errors = pipeline.stage_seeds.flat_map(&:errors).compact.presence
+              return error(seeds_errors.join("\n"))
             end
 
+            ##
+            # Populate pipeline with all stages, and stages with builds.
+            #
+            pipeline.stages = pipeline.stage_seeds.map(&:to_resource)
+
             if pipeline.stages.none?
               return error('No stages / jobs for this pipeline.')
             end
diff --git a/lib/gitlab/ci/pipeline/seed/base.rb b/lib/gitlab/ci/pipeline/seed/base.rb
index 1fd3a61017f..e9e22569ae0 100644
--- a/lib/gitlab/ci/pipeline/seed/base.rb
+++ b/lib/gitlab/ci/pipeline/seed/base.rb
@@ -13,6 +13,10 @@ module Gitlab
             raise NotImplementedError
           end
 
+          def errors
+            raise NotImplementedError
+          end
+
           def to_resource
             raise NotImplementedError
           end
diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb
index ab0d4c38ab6..32086735556 100644
--- a/lib/gitlab/ci/pipeline/seed/build.rb
+++ b/lib/gitlab/ci/pipeline/seed/build.rb
@@ -13,6 +13,7 @@ module Gitlab
             @pipeline = pipeline
             @attributes = attributes
             @previous_stages = previous_stages
+            @needs_attributes = dig(:needs_attributes)
 
             @only = Gitlab::Ci::Build::Policy
               .fabricate(attributes.delete(:only))
@@ -27,8 +28,15 @@ module Gitlab
           def included?
             strong_memoize(:inclusion) do
               all_of_only? &&
-                none_of_except? &&
-                all_of_needs?
+                none_of_except?
+            end
+          end
+
+          def errors
+            return unless included?
+
+            strong_memoize(:errors) do
+              needs_errors
             end
           end
 
@@ -48,6 +56,18 @@ module Gitlab
             @attributes.to_h.dig(:options, :trigger).present?
           end
 
+          def to_resource
+            strong_memoize(:resource) do
+              if bridge?
+                ::Ci::Bridge.new(attributes)
+              else
+                ::Ci::Build.new(attributes)
+              end
+            end
+          end
+
+          private
+
           def all_of_only?
             @only.all? { |spec| spec.satisfied_by?(@pipeline, self) }
           end
@@ -56,25 +76,17 @@ module Gitlab
             @except.none? { |spec| spec.satisfied_by?(@pipeline, self) }
           end
 
-          def all_of_needs?
-            return true unless Feature.enabled?(:ci_dag_support, @pipeline.project)
-            return true if dig(:needs_attributes).nil?
+          def needs_errors
+            return unless Feature.enabled?(:ci_dag_support, @pipeline.project)
+            return if @needs_attributes.nil?
 
-            dig(:needs_attributes).all? do |need|
-              @previous_stages.any? do |stage|
+            @needs_attributes.flat_map do |need|
+              result = @previous_stages.any? do |stage|
                 stage.seeds_names.include?(need[:name])
               end
-            end
-          end
 
-          def to_resource
-            strong_memoize(:resource) do
-              if bridge?
-                ::Ci::Bridge.new(attributes)
-              else
-                ::Ci::Build.new(attributes)
-              end
-            end
+              "#{name}: needs '#{need[:name]}'" unless result
+            end.compact
           end
         end
       end
diff --git a/lib/gitlab/ci/pipeline/seed/stage.rb b/lib/gitlab/ci/pipeline/seed/stage.rb
index 7c737027445..b600df2f656 100644
--- a/lib/gitlab/ci/pipeline/seed/stage.rb
+++ b/lib/gitlab/ci/pipeline/seed/stage.rb
@@ -33,6 +33,12 @@ module Gitlab
             end
           end
 
+          def errors
+            strong_memoize(:errors) do
+              seeds.flat_map(&:errors).compact
+            end
+          end
+
           def seeds_names
             strong_memoize(:seeds_names) do
               seeds.map(&:name).to_set
diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
index 762025f9bd9..e0fbd2e7369 100644
--- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
@@ -396,7 +396,14 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
     end
 
     context 'when build job is not present in prior stages' do
-      it { is_expected.not_to be_included }
+      it "is included" do
+        is_expected.to be_included
+      end
+
+      it "returns an error" do
+        expect(subject.errors).to contain_exactly(
+          "rspec: needs 'build'")
+      end
     end
 
     context 'when build job is part of prior stages' do
@@ -414,7 +421,13 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
       let(:previous_stages) { [stage_seed] }
 
-      it { is_expected.to be_included }
+      it "is included" do
+        is_expected.to be_included
+      end
+
+      it "does not have errors" do
+        expect(subject.errors).to be_empty
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb
index 6fba9f37d91..a13335f63d5 100644
--- a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb
@@ -121,6 +121,16 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do
     end
   end
 
+  describe '#seeds_errors' do
+    it 'returns all errors from seeds' do
+      expect(subject.seeds.first)
+        .to receive(:errors) { ["build error"] }
+
+      expect(subject.errors).to contain_exactly(
+        "build error")
+    end
+  end
+
   describe '#to_resource' do
     it 'builds a valid stage object with all builds' do
       subject.to_resource.save!
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 7e2f311a065..deb68899309 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -1113,7 +1113,7 @@ describe Ci::CreatePipelineService do
           test_a: {
             stage: "test",
             script: "ls",
-            only: %w[master feature tags],
+            only: %w[master feature],
             needs: %w[build_a]
           },
           deploy: {
@@ -1143,6 +1143,7 @@ describe Ci::CreatePipelineService do
         it 'does not create a pipeline as test_a depends on build_a' do
           expect(pipeline).not_to be_persisted
           expect(pipeline.builds).to be_empty
+          expect(pipeline.errors[:base]).to contain_exactly("test_a: needs 'build_a'")
         end
       end