Merge branch 'use-cross-pipeline-dependencies' into 'master'

Use cross pipeline dependencies

See merge request gitlab-org/gitlab!48342

app/models/ci/build_dependencies.rb

@@ -2,6 +2,8 @@
 
 module Ci
   class BuildDependencies
+    include ::Gitlab::Utils::StrongMemoize
+
     attr_reader :processable
 
     def initialize(processable)
@@ -9,7 +11,7 @@ module Ci
     end
 
     def all
-      (local + cross_project).uniq
+      (local + cross_pipeline + cross_project).uniq
     end
 
     # Dependencies local to the given pipeline
@@ -23,6 +25,14 @@ module Ci
       deps
     end
 
+    # Dependencies from the same parent-pipeline hierarchy excluding
+    # the current job's pipeline
+    def cross_pipeline
+      strong_memoize(:cross_pipeline) do
+        fetch_dependencies_in_hierarchy
+      end
+    end
+
     # Dependencies that are defined by project and ref
     def cross_project
       []
@@ -33,7 +43,7 @@ module Ci
     end
 
     def valid?
-      valid_local? && valid_cross_project?
+      valid_local? && valid_cross_pipeline? && valid_cross_project?
     end
 
     private
@@ -44,6 +54,54 @@ module Ci
       ::Ci::Build
     end
 
+    def fetch_dependencies_in_hierarchy
+      deps_specifications = specified_cross_pipeline_dependencies
+      return [] if deps_specifications.empty?
+
+      deps_specifications = expand_variables_and_validate(deps_specifications)
+      jobs_in_pipeline_hierarchy(deps_specifications)
+    end
+
+    def jobs_in_pipeline_hierarchy(deps_specifications)
+      all_pipeline_ids = []
+      all_job_names = []
+
+      deps_specifications.each do |spec|
+        all_pipeline_ids << spec[:pipeline]
+        all_job_names << spec[:job]
+      end
+
+      model_class.latest.success
+        .in_pipelines(processable.pipeline.same_family_pipeline_ids)
+        .in_pipelines(all_pipeline_ids.uniq)
+        .by_name(all_job_names.uniq)
+        .select do |dependency|
+          # the query may not return exact matches pipeline-job, so we filter
+          # them separately.
+          deps_specifications.find do |spec|
+            spec[:pipeline] == dependency.pipeline_id &&
+              spec[:job] == dependency.name
+          end
+        end
+    end
+
+    def expand_variables_and_validate(specifications)
+      specifications.map do |spec|
+        pipeline = ExpandVariables.expand(spec[:pipeline].to_s, processable_variables).to_i
+        # current pipeline is not allowed because local dependencies
+        # should be used instead.
+        next if pipeline == processable.pipeline_id
+
+        job = ExpandVariables.expand(spec[:job], processable_variables)
+
+        { job: job, pipeline: pipeline }
+      end.compact
+    end
+
+    def valid_cross_pipeline?
+      cross_pipeline.size == specified_cross_pipeline_dependencies.size
+    end
+
     def valid_local?
       return true if Feature.enabled?(:ci_disable_validates_dependencies)
 
@@ -78,6 +136,22 @@ module Ci
 
       scope.where(name: processable.options[:dependencies])
     end
+
+    def processable_variables
+      -> { processable.simple_variables_without_dependencies }
+    end
+
+    def specified_cross_pipeline_dependencies
+      strong_memoize(:specified_cross_pipeline_dependencies) do
+        next [] unless Feature.enabled?(:ci_cross_pipeline_artifacts_download, processable.project, default_enabled: false)
+
+        specified_cross_dependencies.select { |dep| dep[:pipeline] && dep[:artifacts] }
+      end
+    end
+
+    def specified_cross_dependencies
+      Array(processable.options[:cross_dependencies])
+    end
   end
 end
 

config/feature_flags/development/ci_cross_pipeline_artifacts_download.yml

+---
+name: ci_cross_pipeline_artifacts_download
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48342 
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/287622 
+milestone: '13.7'
+type: development
+group: group::continuous integration
+default_enabled: false

ee/app/models/ee/ci/build_dependencies.rb

@@ -7,7 +7,7 @@ module EE
       extend ::Gitlab::Utils::Override
       include ::Gitlab::Utils::StrongMemoize
 
-      LIMIT = ::Gitlab::Ci::Config::Entry::Needs::NEEDS_CROSS_PROJECT_DEPENDENCIES_LIMIT
+      CROSS_PROJECT_LIMIT = ::Gitlab::Ci::Config::Entry::Needs::NEEDS_CROSS_PROJECT_DEPENDENCIES_LIMIT
 
       override :cross_project
       def cross_project
@@ -36,21 +36,21 @@ module EE
         deps = specified_cross_project_dependencies
         return model_class.none unless deps.any?
 
-        relationship_fragments = build_cross_dependencies_fragments(deps, model_class.latest.success)
+        relationship_fragments = build_cross_project_dependencies_fragments(deps, model_class.latest.success)
         return model_class.none unless relationship_fragments.any?
 
-        model_class.from_union(relationship_fragments).limit(LIMIT)
+        model_class.from_union(relationship_fragments).limit(CROSS_PROJECT_LIMIT)
       end
 
-      def build_cross_dependencies_fragments(deps, search_scope)
+      def build_cross_project_dependencies_fragments(deps, search_scope)
         deps.inject([]) do |fragments, dep|
           next fragments unless dep[:artifacts]
 
-          fragments << build_cross_dependency_relationship_fragment(dep, search_scope)
+          fragments << build_cross_project_dependency_relationship_fragment(dep, search_scope)
         end
       end
 
-      def build_cross_dependency_relationship_fragment(dependency, search_scope)
+      def build_cross_project_dependency_relationship_fragment(dependency, search_scope)
         args = dependency.values_at(:job, :ref, :project)
         args = args.map { |value| ExpandVariables.expand(value, processable_variables) }
 
@@ -62,17 +62,9 @@ module EE
         processable.user
       end
 
-      def processable_variables
-        -> { processable.simple_variables_without_dependencies }
-      end
-
       def specified_cross_project_dependencies
         specified_cross_dependencies.select { |dep| dep[:project] }
       end
-
-      def specified_cross_dependencies
-        Array(processable.options[:cross_dependencies])
-      end
     end
   end
 end

ee/spec/models/ee/ci/build_dependencies_spec.rb

@@ -3,36 +3,36 @@
 require 'spec_helper'
 
 RSpec.describe Ci::BuildDependencies do
-  describe '#cross_project' do
-    let_it_be(:user) { create(:user) }
-    let_it_be(:project, refind: true) { create(:project, :repository) }
-    let(:dependencies) { }
-
-    let(:pipeline) do
-      create(:ci_pipeline,
-        project: project,
-        sha: project.commit.id,
-        ref: project.default_branch,
-        status: 'success')
-    end
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project, refind: true) { create(:project, :repository) }
+  let(:dependencies) { }
+
+  let(:pipeline) do
+    create(:ci_pipeline,
+      project: project,
+      sha: project.commit.id,
+      ref: project.default_branch,
+      status: 'success')
+  end
 
-    let!(:job) do
-      create(:ci_build,
-        pipeline: pipeline,
-        name: 'final',
-        stage_idx: 3,
-        stage: 'deploy',
-        user: user,
-        options: { cross_dependencies: dependencies })
-    end
+  let!(:job) do
+    create(:ci_build,
+      pipeline: pipeline,
+      name: 'final',
+      stage_idx: 3,
+      stage: 'deploy',
+      user: user,
+      options: { cross_dependencies: dependencies })
+  end
 
-    subject { described_class.new(job).cross_project }
+  before do
+    project.add_developer(user)
+    pipeline.update!(user: user)
+    stub_licensed_features(cross_project_pipelines: true)
+  end
 
-    before do
-      project.add_developer(user)
-      pipeline.update!(user: user)
-      stub_licensed_features(cross_project_pipelines: true)
-    end
+  describe '#cross_project' do
+    subject { described_class.new(job).cross_project }
 
     context 'when cross_dependencies are not defined' do
       it { is_expected.to be_empty }
@@ -222,6 +222,42 @@ RSpec.describe Ci::BuildDependencies do
       end
     end
 
+    context 'with too many cross_dependencies' do
+      let(:cross_dependencies_limit) do
+        ::Gitlab::Ci::Config::Entry::Needs::NEEDS_CROSS_PROJECT_DEPENDENCIES_LIMIT
+      end
+
+      before do
+        cross_dependencies_limit.next.times do |index|
+          create(:ci_build, :success,
+            pipeline: pipeline, name: "dependency-#{index}",
+            stage_idx: 1, stage: 'build', user: user
+          )
+        end
+      end
+
+      let(:dependencies) do
+        Array.new(cross_dependencies_limit.next) do |index|
+          {
+            project: project.full_path,
+            job: "dependency-#{index}",
+            ref: pipeline.ref,
+            artifacts: true
+          }
+        end
+      end
+
+      it 'returns a limited number of dependencies' do
+        expect(subject.size).to eq(cross_dependencies_limit)
+      end
+    end
+  end
+
+  describe '#all' do
+    let(:build_dependencies) { described_class.new(job) }
+
+    subject { build_dependencies.all }
+
     context 'with both cross project and cross pipeline dependencies' do
       let(:other_project) { create(:project, :repository) }
 
@@ -263,54 +299,41 @@ RSpec.describe Ci::BuildDependencies do
           user: user)
       end
 
+      let(:pipeline) do
+        create(:ci_pipeline,
+          child_of: upstream_pipeline,
+          project: project,
+          sha: project.commit.id,
+          ref: project.default_branch,
+          status: 'success')
+      end
+
       let(:dependencies) do
         [
-          { pipeline: '$UPSTREAM_PIPELINE_ID', job: 'build', artifacts: true },
+          { pipeline: '$UPSTREAM_PIPELINE_ID', job: '$UPSTREAM_JOB', artifacts: true },
           { project: other_project.full_path, ref: other_project.default_branch, job: 'deploy', artifacts: true }
         ]
       end
 
       before do
         job.yaml_variables.push(key: 'UPSTREAM_PIPELINE_ID', value: upstream_pipeline.id.to_s, public: true)
+        job.yaml_variables.push(key: 'UPSTREAM_JOB', value: upstream_pipeline_dependency.name, public: true)
         job.save!
 
         other_project.add_developer(user)
       end
 
-      # TODO: In a follow-up MR we are adding support to querying pipelines in the same
-      # project.
-      it 'temporarily ignores cross pipeline dependencies' do
-        is_expected.to contain_exactly(cross_project_dependency)
-      end
-    end
-
-    context 'with too many cross_dependencies' do
-      let(:cross_dependencies_limit) do
-        ::Gitlab::Ci::Config::Entry::Needs::NEEDS_CROSS_PROJECT_DEPENDENCIES_LIMIT
-      end
-
-      before do
-        cross_dependencies_limit.next.times do |index|
-          create(:ci_build, :success,
-            pipeline: pipeline, name: "dependency-#{index}",
-            stage_idx: 1, stage: 'build', user: user
-          )
-        end
+      it 'returns both dependencies' do
+        is_expected.to contain_exactly(cross_project_dependency, upstream_pipeline_dependency)
       end
 
-      let(:dependencies) do
-        Array.new(cross_dependencies_limit.next) do |index|
-          {
-            project: project.full_path,
-            job: "dependency-#{index}",
-            ref: pipeline.ref,
-            artifacts: true
-          }
+      context 'when feature flag `ci_cross_pipeline_artifacts_download` is disabled' do
+        before do
+          stub_feature_flags(ci_cross_pipeline_artifacts_download: false)
         end
-      end
 
-      it 'returns a limited number of dependencies' do
-        expect(subject.size).to eq(cross_dependencies_limit)
+        it { is_expected.to contain_exactly(cross_project_dependency) }
+        it { expect(build_dependencies).to be_valid }
       end
     end
   end

spec/models/ci/build_dependencies_spec.rb

@@ -146,6 +146,204 @@ RSpec.describe Ci::BuildDependencies do
     end
   end
 
+  describe '#cross_pipeline' do
+    let!(:job) do
+      create(:ci_build,
+        pipeline: pipeline,
+        name: 'build_with_pipeline_dependency',
+        options: { cross_dependencies: dependencies })
+    end
+
+    subject { described_class.new(job) }
+
+    let(:cross_pipeline_deps) { subject.cross_pipeline }
+
+    context 'when dependency specifications are valid' do
+      context 'when pipeline exists in the hierarchy' do
+        let!(:pipeline) { create(:ci_pipeline, child_of: parent_pipeline) }
+        let!(:parent_pipeline) { create(:ci_pipeline, project: project) }
+
+        context 'when job exists' do
+          let(:dependencies) do
+            [{ pipeline: parent_pipeline.id.to_s, job: upstream_job.name, artifacts: true }]
+          end
+
+          let!(:upstream_job) { create(:ci_build, :success, pipeline: parent_pipeline) }
+
+          it { expect(cross_pipeline_deps).to contain_exactly(upstream_job) }
+          it { is_expected.to be_valid }
+
+          context 'when pipeline and job are specified via variables' do
+            let(:dependencies) do
+              [{ pipeline: '$parent_pipeline_ID', job: '$UPSTREAM_JOB', artifacts: true }]
+            end
+
+            before do
+              job.yaml_variables.push(key: 'parent_pipeline_ID', value: parent_pipeline.id.to_s, public: true)
+              job.yaml_variables.push(key: 'UPSTREAM_JOB', value: upstream_job.name, public: true)
+              job.save!
+            end
+
+            it { expect(cross_pipeline_deps).to contain_exactly(upstream_job) }
+            it { is_expected.to be_valid }
+          end
+
+          context 'when feature flag `ci_cross_pipeline_artifacts_download` is disabled' do
+            before do
+              stub_feature_flags(ci_cross_pipeline_artifacts_download: false)
+            end
+
+            it { expect(cross_pipeline_deps).to be_empty }
+            it { is_expected.to be_valid }
+          end
+        end
+
+        context 'when same job names exist in other pipelines in the hierarchy' do
+          let(:cross_pipeline_limit) do
+            ::Gitlab::Ci::Config::Entry::Needs::NEEDS_CROSS_PIPELINE_DEPENDENCIES_LIMIT
+          end
+
+          let(:sibling_pipeline) { create(:ci_pipeline, child_of: parent_pipeline) }
+
+          before do
+            cross_pipeline_limit.times do |index|
+              create(:ci_build, :success,
+                pipeline: parent_pipeline, name: "dependency-#{index}",
+                stage_idx: 1, stage: 'build', user: user
+              )
+
+              create(:ci_build, :success,
+                pipeline: sibling_pipeline, name: "dependency-#{index}",
+                stage_idx: 1, stage: 'build', user: user
+              )
+            end
+          end
+
+          let(:dependencies) do
+            [
+              { pipeline: parent_pipeline.id.to_s,  job: 'dependency-0', artifacts: true },
+              { pipeline: parent_pipeline.id.to_s,  job: 'dependency-1', artifacts: true },
+              { pipeline: parent_pipeline.id.to_s,  job: 'dependency-2', artifacts: true },
+              { pipeline: sibling_pipeline.id.to_s, job: 'dependency-3', artifacts: true },
+              { pipeline: sibling_pipeline.id.to_s, job: 'dependency-4', artifacts: true },
+              { pipeline: sibling_pipeline.id.to_s, job: 'dependency-5', artifacts: true }
+            ]
+          end
+
+          it 'returns a limited number of dependencies with the right match' do
+            expect(job.options[:cross_dependencies].size).to eq(cross_pipeline_limit.next)
+            expect(cross_pipeline_deps.size).to eq(cross_pipeline_limit)
+            expect(cross_pipeline_deps.map { |dep| [dep.pipeline_id, dep.name] }).to contain_exactly(
+              [parent_pipeline.id, 'dependency-0'],
+              [parent_pipeline.id, 'dependency-1'],
+              [parent_pipeline.id, 'dependency-2'],
+              [sibling_pipeline.id, 'dependency-3'],
+              [sibling_pipeline.id, 'dependency-4'])
+          end
+        end
+
+        context 'when job does not exist' do
+          let(:dependencies) do
+            [{ pipeline: parent_pipeline.id.to_s, job: 'non-existent', artifacts: true }]
+          end
+
+          it { expect(cross_pipeline_deps).to be_empty }
+          it { is_expected.not_to be_valid }
+        end
+      end
+
+      context 'when pipeline does not exist' do
+        let(:dependencies) do
+          [{ pipeline: '123', job: 'non-existent', artifacts: true }]
+        end
+
+        it { expect(cross_pipeline_deps).to be_empty }
+        it { is_expected.not_to be_valid }
+      end
+
+      context 'when jobs exist in different pipelines in the hierarchy' do
+        let!(:pipeline) { create(:ci_pipeline, child_of: parent_pipeline) }
+        let!(:parent_pipeline) { create(:ci_pipeline, project: project) }
+        let!(:parent_job) { create(:ci_build, :success, name: 'parent_job', pipeline: parent_pipeline) }
+
+        let!(:sibling_pipeline) { create(:ci_pipeline, child_of: parent_pipeline) }
+        let!(:sibling_job) { create(:ci_build, :success, name: 'sibling_job', pipeline: sibling_pipeline) }
+
+        context 'when pipeline and jobs dependencies are mismatched' do
+          let(:dependencies) do
+            [
+              { pipeline: parent_pipeline.id.to_s, job: sibling_job.name, artifacts: true },
+              { pipeline: sibling_pipeline.id.to_s, job: parent_job.name, artifacts: true }
+            ]
+          end
+
+          it { expect(cross_pipeline_deps).to be_empty }
+          it { is_expected.not_to be_valid }
+
+          context 'when dependencies contain a valid pair' do
+            let(:dependencies) do
+              [
+                { pipeline: parent_pipeline.id.to_s, job: sibling_job.name, artifacts: true },
+                { pipeline: sibling_pipeline.id.to_s, job: parent_job.name, artifacts: true },
+                { pipeline: sibling_pipeline.id.to_s, job: sibling_job.name, artifacts: true }
+              ]
+            end
+
+            it 'filters out the invalid ones' do
+              expect(cross_pipeline_deps).to contain_exactly(sibling_job)
+            end
+
+            it { is_expected.not_to be_valid }
+          end
+        end
+      end
+
+      context 'when job and pipeline exist outside the hierarchy' do
+        let!(:pipeline) { create(:ci_pipeline, project: project) }
+        let!(:another_pipeline) { create(:ci_pipeline, project: project) }
+        let!(:dependency) { create(:ci_build, :success, pipeline: another_pipeline) }
+
+        let(:dependencies) do
+          [{ pipeline: another_pipeline.id.to_s, job: dependency.name, artifacts: true }]
+        end
+
+        it 'ignores jobs outside the pipeline hierarchy' do
+          expect(cross_pipeline_deps).to be_empty
+        end
+
+        it { is_expected.not_to be_valid }
+      end
+
+      context 'when current pipeline is specified' do
+        let!(:pipeline) { create(:ci_pipeline, project: project) }
+        let!(:dependency) { create(:ci_build, :success, pipeline: pipeline) }
+
+        let(:dependencies) do
+          [{ pipeline: pipeline.id.to_s, job: dependency.name, artifacts: true }]
+        end
+
+        it 'ignores jobs from the current pipeline as simple needs should be used instead' do
+          expect(cross_pipeline_deps).to be_empty
+        end
+
+        it { is_expected.not_to be_valid }
+      end
+    end
+
+    context 'when artifacts:false' do
+      let!(:pipeline) { create(:ci_pipeline, child_of: parent_pipeline) }
+      let!(:parent_pipeline) { create(:ci_pipeline, project: project) }
+      let!(:parent_job) { create(:ci_build, :success, name: 'parent_job', pipeline: parent_pipeline) }
+
+      let(:dependencies) do
+        [{ pipeline: parent_pipeline.id.to_s, job: parent_job.name, artifacts: false }]
+      end
+
+      it { expect(cross_pipeline_deps).to be_empty }
+      it { is_expected.to be_valid } # we simply ignore it
+    end
+  end
+
   describe '#all' do
     let!(:job) do
       create(:ci_build, pipeline: pipeline, name: 'deploy', stage_idx: 3, stage: 'deploy')