Fix error when third level trigger pipeline

When implementing child of child pipelines, we add the triggered
pipelines, which have different projects, into the hierarchy depth.

In this commit, we are starting to ignore other project pipelines.

app/models/ci/pipeline.rb

@@ -823,7 +823,9 @@ module Ci
 
     def same_family_pipeline_ids
       if ::Gitlab::Ci::Features.child_of_child_pipeline_enabled?(project)
-        ::Gitlab::Ci::PipelineObjectHierarchy.new(base_and_ancestors).base_and_descendants.select(:id)
+        ::Gitlab::Ci::PipelineObjectHierarchy.new(
+          base_and_ancestors(same_project: true), options: { same_project: true }
+        ).base_and_descendants.select(:id)
       else
         # If pipeline is a child of another pipeline, include the parent
         # and the siblings, otherwise return only itself and children.
@@ -1062,11 +1064,11 @@ module Ci
       self.ci_ref = Ci::Ref.ensure_for(self)
     end
 
-    def base_and_ancestors
+    def base_and_ancestors(same_project: false)
       # Without using `unscoped`, caller scope is also included into the query.
       # Using `unscoped` here will be redundant after Rails 6.1
       ::Gitlab::Ci::PipelineObjectHierarchy
-        .new(self.class.unscoped.where(id: id))
+        .new(self.class.unscoped.where(id: id), options: { same_project: same_project })
         .base_and_ancestors
     end
 

app/services/ci/create_downstream_pipeline_service.rb

@@ -121,7 +121,7 @@ module Ci
     def has_max_descendants_depth?
       return false unless @bridge.triggers_child_pipeline?
 
-      ancestors_of_new_child = @bridge.pipeline.base_and_ancestors
+      ancestors_of_new_child = @bridge.pipeline.base_and_ancestors(same_project: true)
       ancestors_of_new_child.count > MAX_DESCENDANTS_DEPTH
     end
   end

changelogs/unreleased/29651-fix-triggered-pipelines-depth.yml

+---
+title: Fix error when third level trigger pipeline
+merge_request: 42192
+author:
+type: fixed

lib/gitlab/ci/pipeline_object_hierarchy.rb

@@ -20,14 +20,26 @@ module Gitlab
       def ancestor_conditions(cte)
         middle_table[:source_pipeline_id].eq(objects_table[:id]).and(
           middle_table[:pipeline_id].eq(cte.table[:id])
+        ).and(
+          same_project_condition
         )
       end
 
       def descendant_conditions(cte)
         middle_table[:pipeline_id].eq(objects_table[:id]).and(
           middle_table[:source_pipeline_id].eq(cte.table[:id])
+        ).and(
+          same_project_condition
         )
       end
+
+      def same_project_condition
+        if options[:same_project]
+          middle_table[:source_project_id].eq(middle_table[:project_id])
+        else
+          Arel.sql('TRUE')
+        end
+      end
     end
   end
 end

lib/gitlab/object_hierarchy.rb

@@ -7,18 +7,19 @@ module Gitlab
   class ObjectHierarchy
     DEPTH_COLUMN = :depth
 
-    attr_reader :ancestors_base, :descendants_base, :model
+    attr_reader :ancestors_base, :descendants_base, :model, :options
 
     # ancestors_base - An instance of ActiveRecord::Relation for which to
     #                  get parent objects.
     # descendants_base - An instance of ActiveRecord::Relation for which to
     #                    get child objects. If omitted, ancestors_base is used.
-    def initialize(ancestors_base, descendants_base = ancestors_base)
+    def initialize(ancestors_base, descendants_base = ancestors_base, options: {})
       raise ArgumentError.new("Model of ancestors_base does not match model of descendants_base") if ancestors_base.model != descendants_base.model
 
       @ancestors_base = ancestors_base
       @descendants_base = descendants_base
       @model = ancestors_base.model
+      @options = options
     end
 
     # Returns the set of descendants of a given relation, but excluding the given

spec/lib/gitlab/ci/pipeline_object_hierarchy_spec.rb

@@ -3,37 +3,43 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Ci::PipelineObjectHierarchy do
-  let_it_be(:project) { create_default(:project, :repository) }
-  let_it_be(:pipeline) { create(:ci_empty_pipeline, status: :created, project: project) }
-  let_it_be(:ancestor) { create(:ci_pipeline, project: pipeline.project) }
-  let_it_be(:parent) { create(:ci_pipeline, project: pipeline.project) }
-  let_it_be(:child) { create(:ci_pipeline, project: pipeline.project) }
-  let_it_be(:cousin_parent) { create(:ci_pipeline, project: pipeline.project) }
-  let_it_be(:cousin) { create(:ci_pipeline, project: pipeline.project) }
+  include Ci::SourcePipelineHelpers
+
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:ancestor) { create(:ci_pipeline, project: project) }
+  let_it_be(:parent) { create(:ci_pipeline, project: project) }
+  let_it_be(:child) { create(:ci_pipeline, project: project) }
+  let_it_be(:cousin_parent) { create(:ci_pipeline, project: project) }
+  let_it_be(:cousin) { create(:ci_pipeline, project: project) }
+  let_it_be(:triggered_pipeline) { create(:ci_pipeline) }
 
   before_all do
-    create_source_relation(ancestor, parent)
-    create_source_relation(ancestor, cousin_parent)
-    create_source_relation(parent, child)
-    create_source_relation(cousin_parent, cousin)
+    create_source_pipeline(ancestor, parent)
+    create_source_pipeline(ancestor, cousin_parent)
+    create_source_pipeline(parent, child)
+    create_source_pipeline(cousin_parent, cousin)
+    create_source_pipeline(child, triggered_pipeline)
   end
 
   describe '#base_and_ancestors' do
     it 'includes the base and its ancestors' do
-      relation = described_class.new(::Ci::Pipeline.where(id: parent.id)).base_and_ancestors
+      relation = described_class.new(::Ci::Pipeline.where(id: parent.id),
+                                     options: { same_project: true }).base_and_ancestors
 
       expect(relation).to contain_exactly(ancestor, parent)
     end
 
     it 'can find ancestors upto a certain level' do
-      relation = described_class.new(::Ci::Pipeline.where(id: child.id)).base_and_ancestors(upto: ancestor.id)
+      relation = described_class.new(::Ci::Pipeline.where(id: child.id),
+                                     options: { same_project: true }).base_and_ancestors(upto: ancestor.id)
 
       expect(relation).to contain_exactly(parent, child)
     end
 
     describe 'hierarchy_order option' do
       let(:relation) do
-        described_class.new(::Ci::Pipeline.where(id: child.id)).base_and_ancestors(hierarchy_order: hierarchy_order)
+        described_class.new(::Ci::Pipeline.where(id: child.id),
+                            options: { same_project: true }).base_and_ancestors(hierarchy_order: hierarchy_order)
       end
 
       context ':asc' do
@@ -56,14 +62,16 @@ RSpec.describe Gitlab::Ci::PipelineObjectHierarchy do
 
   describe '#base_and_descendants' do
     it 'includes the base and its descendants' do
-      relation = described_class.new(::Ci::Pipeline.where(id: parent.id)).base_and_descendants
+      relation = described_class.new(::Ci::Pipeline.where(id: parent.id),
+                                     options: { same_project: true }).base_and_descendants
 
       expect(relation).to contain_exactly(parent, child)
     end
 
     context 'when with_depth is true' do
       let(:relation) do
-        described_class.new(::Ci::Pipeline.where(id: ancestor.id)).base_and_descendants(with_depth: true)
+        described_class.new(::Ci::Pipeline.where(id: ancestor.id),
+                            options: { same_project: true }).base_and_descendants(with_depth: true)
       end
 
       it 'includes depth in the results' do
@@ -84,7 +92,8 @@ RSpec.describe Gitlab::Ci::PipelineObjectHierarchy do
 
   describe '#all_objects' do
     it 'includes its ancestors and descendants' do
-      relation = described_class.new(::Ci::Pipeline.where(id: parent.id)).all_objects
+      relation = described_class.new(::Ci::Pipeline.where(id: parent.id),
+                                     options: { same_project: true }).all_objects
 
       expect(relation).to contain_exactly(ancestor, parent, child)
     end
@@ -92,20 +101,11 @@ RSpec.describe Gitlab::Ci::PipelineObjectHierarchy do
     it 'returns all family tree' do
       relation = described_class.new(
         ::Ci::Pipeline.where(id: child.id),
-        described_class.new(::Ci::Pipeline.where(id: child.id)).base_and_ancestors
+        described_class.new(::Ci::Pipeline.where(id: child.id), options: { same_project: true }).base_and_ancestors,
+        options: { same_project: true }
       ).all_objects
 
       expect(relation).to contain_exactly(ancestor, parent, cousin_parent, child, cousin)
     end
   end
-
-  private
-
-  def create_source_relation(parent, child)
-    create(:ci_sources_pipeline,
-           source_job: create(:ci_build, pipeline: parent),
-           source_project: parent.project,
-           pipeline: child,
-           project: child.project)
-  end
 end

spec/models/ci/pipeline_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
   include ProjectForksHelper
   include StubRequests
+  include Ci::SourcePipelineHelpers
 
   let_it_be(:user) { create(:user) }
   let_it_be(:namespace) { create_default(:namespace) }
@@ -2629,17 +2630,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
       let(:sibling) { create(:ci_pipeline, project: pipeline.project) }
 
       before do
-        create(:ci_sources_pipeline,
-               source_job: create(:ci_build, pipeline: parent),
-               source_project: parent.project,
-               pipeline: pipeline,
-               project: pipeline.project)
-
-        create(:ci_sources_pipeline,
-               source_job: create(:ci_build, pipeline: parent),
-               source_project: parent.project,
-               pipeline: sibling,
-               project: sibling.project)
+        create_source_pipeline(parent, pipeline)
+        create_source_pipeline(parent, sibling)
       end
 
       it 'returns parent sibling and self ids' do
@@ -2651,11 +2643,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
       let(:child) { create(:ci_pipeline, project: pipeline.project) }
 
       before do
-        create(:ci_sources_pipeline,
-               source_job: create(:ci_build, pipeline: pipeline),
-               source_project: pipeline.project,
-               pipeline: child,
-               project: child.project)
+        create_source_pipeline(pipeline, child)
       end
 
       it 'returns self and child ids' do
@@ -2670,29 +2658,10 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
       let(:cousin) { create(:ci_pipeline, project: pipeline.project) }
 
       before do
-        create(:ci_sources_pipeline,
-               source_job: create(:ci_build, pipeline: ancestor),
-               source_project: ancestor.project,
-               pipeline: parent,
-               project: parent.project)
-
-        create(:ci_sources_pipeline,
-               source_job: create(:ci_build, pipeline: ancestor),
-               source_project: ancestor.project,
-               pipeline: cousin_parent,
-               project: cousin_parent.project)
-
-        create(:ci_sources_pipeline,
-               source_job: create(:ci_build, pipeline: parent),
-               source_project: parent.project,
-               pipeline: pipeline,
-               project: pipeline.project)
-
-        create(:ci_sources_pipeline,
-               source_job: create(:ci_build, pipeline: cousin_parent),
-               source_project: cousin_parent.project,
-               pipeline: cousin,
-               project: cousin.project)
+        create_source_pipeline(ancestor, parent)
+        create_source_pipeline(ancestor, cousin_parent)
+        create_source_pipeline(parent, pipeline)
+        create_source_pipeline(cousin_parent, cousin)
       end
 
       it 'returns all family ids' do
@@ -2701,6 +2670,18 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
         )
       end
     end
+
+    context 'when pipeline is a triggered pipeline' do
+      let(:upstream) { create(:ci_pipeline, project: create(:project)) }
+
+      before do
+        create_source_pipeline(upstream, pipeline)
+      end
+
+      it 'returns self id' do
+        expect(subject).to contain_exactly(pipeline.id)
+      end
+    end
   end
 
   describe '#stuck?' do
@@ -3556,4 +3537,78 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
       expect(builds).not_to include(karma)
     end
   end
+
+  describe '#base_and_ancestors' do
+    let(:same_project) { false }
+
+    subject { pipeline.base_and_ancestors(same_project: same_project) }
+
+    context 'when pipeline is not child nor parent' do
+      it 'returns just the pipeline itself' do
+        expect(subject).to contain_exactly(pipeline)
+      end
+    end
+
+    context 'when pipeline is child' do
+      let(:parent) { create(:ci_pipeline, project: pipeline.project) }
+      let(:sibling) { create(:ci_pipeline, project: pipeline.project) }
+
+      before do
+        create_source_pipeline(parent, pipeline)
+        create_source_pipeline(parent, sibling)
+      end
+
+      it 'returns parent and self' do
+        expect(subject).to contain_exactly(parent, pipeline)
+      end
+    end
+
+    context 'when pipeline is parent' do
+      let(:child) { create(:ci_pipeline, project: pipeline.project) }
+
+      before do
+        create_source_pipeline(pipeline, child)
+      end
+
+      it 'returns self' do
+        expect(subject).to contain_exactly(pipeline)
+      end
+    end
+
+    context 'when pipeline is a child of a child pipeline' do
+      let(:ancestor) { create(:ci_pipeline, project: pipeline.project) }
+      let(:parent) { create(:ci_pipeline, project: pipeline.project) }
+
+      before do
+        create_source_pipeline(ancestor, parent)
+        create_source_pipeline(parent, pipeline)
+      end
+
+      it 'returns self, parent and ancestor' do
+        expect(subject).to contain_exactly(ancestor, parent, pipeline)
+      end
+    end
+
+    context 'when pipeline is a triggered pipeline' do
+      let(:upstream) { create(:ci_pipeline, project: create(:project)) }
+
+      before do
+        create_source_pipeline(upstream, pipeline)
+      end
+
+      context 'same_project: false' do
+        it 'returns upstream and self' do
+          expect(subject).to contain_exactly(pipeline, upstream)
+        end
+      end
+
+      context 'same_project: true' do
+        let(:same_project) { true }
+
+        it 'returns self' do
+          expect(subject).to contain_exactly(pipeline)
+        end
+      end
+    end
+  end
 end

spec/services/ci/create_downstream_pipeline_service_spec.rb

@@ -179,7 +179,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
       end
     end
 
-    context 'when downstream project is the same as the job project' do
+    context 'when downstream project is the same as the upstream project' do
       let(:trigger) do
         { trigger: { project: upstream_project.full_path } }
       end
@@ -311,18 +311,14 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
           end
         end
 
-        context 'when upstream pipeline is a first descendant pipeline' do
-          let!(:pipeline_source) do
+        context 'when upstream pipeline has a parent pipeline' do
+          before do
             create(:ci_sources_pipeline,
               source_pipeline: create(:ci_pipeline, project: upstream_pipeline.project),
               pipeline: upstream_pipeline
             )
           end
 
-          before do
-            upstream_pipeline.update!(source: :parent_pipeline)
-          end
-
           it 'creates the pipeline' do
             expect { service.execute(bridge) }
               .to change { Ci::Pipeline.count }.by(1)
@@ -345,8 +341,8 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
           end
         end
 
-        context 'when upstream pipeline is a second descendant pipeline' do
-          let!(:pipeline_source) do
+        context 'when upstream pipeline has a parent pipeline, which has a parent pipeline' do
+          before do
             parent_of_upstream_pipeline = create(:ci_pipeline, project: upstream_pipeline.project)
 
             create(:ci_sources_pipeline,
@@ -360,10 +356,6 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
             )
           end
 
-          before do
-            upstream_pipeline.update!(source: :parent_pipeline)
-          end
-
           it 'does not create a second descendant pipeline' do
             expect { service.execute(bridge) }
               .not_to change { Ci::Pipeline.count }
@@ -372,6 +364,27 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
             expect(bridge.failure_reason).to eq 'reached_max_descendant_pipelines_depth'
           end
         end
+
+        context 'when upstream pipeline has two level upstream pipelines from different projects' do
+          before do
+            upstream_of_upstream_of_upstream_pipeline = create(:ci_pipeline)
+            upstream_of_upstream_pipeline = create(:ci_pipeline)
+
+            create(:ci_sources_pipeline,
+              source_pipeline: upstream_of_upstream_of_upstream_pipeline,
+              pipeline: upstream_of_upstream_pipeline
+            )
+
+            create(:ci_sources_pipeline,
+              source_pipeline: upstream_of_upstream_pipeline,
+              pipeline: upstream_pipeline
+            )
+          end
+
+          it 'create the pipeline' do
+            expect { service.execute(bridge) }.to change { Ci::Pipeline.count }.by(1)
+          end
+        end
       end
     end
 

spec/support/helpers/ci/source_pipeline_helpers.rb

+# frozen_string_literal: true
+
+module Ci
+  module SourcePipelineHelpers
+    def create_source_pipeline(upstream, downstream)
+      create(:ci_sources_pipeline,
+             source_job: create(:ci_build, pipeline: upstream),
+             source_project: upstream.project,
+             pipeline: downstream,
+             project: downstream.project)
+    end
+  end
+end