Merge branch 'add-parent-child-pipeline-limitations' into 'master'

Add parent/child pipeline limitations (2nd attempt)

See merge request gitlab-org/gitlab!22675

ee/app/models/ee/commit_status_enums.rb

@@ -14,7 +14,8 @@ module EE
                     downstream_bridge_project_not_found: 1_002,
                     invalid_bridge_trigger: 1_003,
                     upstream_bridge_project_not_found: 1_004,
-                    insufficient_upstream_permissions: 1_005)
+                    insufficient_upstream_permissions: 1_005,
+                    bridge_pipeline_is_child_pipeline: 1_006)
       end
     end
   end

ee/app/presenters/ee/commit_status_presenter.rb

@@ -10,7 +10,8 @@ module EE
         insufficient_upstream_permissions: 'This job could not be executed because of insufficient permissions to track the upstream project.',
         downstream_bridge_project_not_found: 'This job could not be executed because downstream bridge project could not be found.',
         upstream_bridge_project_not_found: 'This job could not be executed because upstream bridge project could not be found.',
-        invalid_bridge_trigger: 'This job could not be executed because downstream pipeline trigger definition is invalid.'
+        invalid_bridge_trigger: 'This job could not be executed because downstream pipeline trigger definition is invalid.',
+        bridge_pipeline_is_child_pipeline: 'This job belongs to a child pipeline and cannot create further child pipelines.'
       ).freeze
 
       EE::CommitStatusPresenter.private_constant :EE_CALLOUT_FAILURE_MESSAGES

ee/app/services/ci/create_cross_project_pipeline_service.rb

@@ -45,6 +45,13 @@ module Ci
         return false
       end
 
+      # TODO: Remove this condition if favour of model validation
+      # https://gitlab.com/gitlab-org/gitlab/issues/38338
+      if @bridge.triggers_child_pipeline? && @bridge.pipeline.parent_pipeline.present?
+        @bridge.drop!(:bridge_pipeline_is_child_pipeline)
+        return false
+      end
+
       unless can_create_downstream_pipeline?(target_ref)
         @bridge.drop!(:insufficient_bridge_permissions)
         return false

ee/changelogs/unreleased/add-parent-child-pipeline-limitations.yml

+---
+title: Prevent a child pipeline to create further child pipelines
+merge_request: 22675
+author:
+type: added

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

@@ -51,6 +51,7 @@ module EE
                 include ::Gitlab::Config::Entry::Attributable
                 include ::Gitlab::Config::Entry::Configurable
 
+                INCLUDE_MAX_SIZE = 3
                 ALLOWED_KEYS = %i[strategy include].freeze
                 attributes :strategy
 
@@ -62,7 +63,8 @@ module EE
 
                 entry :include, ::Gitlab::Ci::Config::Entry::Includes,
                   description: 'List of external YAML files to include.',
-                  reserved: true
+                  reserved: true,
+                  metadata: { max_size: INCLUDE_MAX_SIZE }
 
                 def value
                   @config

ee/lib/ee/gitlab/ci/status/build/failed.rb

@@ -15,7 +15,8 @@ module EE
                 downstream_bridge_project_not_found: 'downstream project could not be found',
                 upstream_bridge_project_not_found: 'upstream project could not be found',
                 insufficient_bridge_permissions: 'no permissions to trigger downstream pipeline',
-                insufficient_upstream_permissions: 'no permissions to read upstream project'
+                insufficient_upstream_permissions: 'no permissions to read upstream project',
+                bridge_pipeline_is_child_pipeline: 'creation of child pipeline not allowed from another child pipeline'
               ).freeze
               EE::Gitlab::Ci::Status::Build::Failed.private_constant :EE_REASONS
             end

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

@@ -13,7 +13,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
     end
 
     describe '#value' do
-      it 'is returns a trigger configuration hash' do
+      it 'returns a trigger configuration hash' do
         expect(subject.value).to eq(project: 'some/project')
       end
     end
@@ -27,7 +27,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
     end
 
     describe '#errors' do
-      it 'is returns an error about an empty config' do
+      it 'returns an error about an empty config' do
         expect(subject.errors.first)
           .to match /config can't be blank/
       end
@@ -43,7 +43,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
       end
 
       describe '#value' do
-        it 'is returns a trigger configuration hash' do
+        it 'returns a trigger configuration hash' do
           expect(subject.value)
             .to eq(project: 'some/project', branch: 'feature')
         end
@@ -59,7 +59,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
         end
 
         describe '#value' do
-          it 'is returns a trigger configuration hash' do
+          it 'returns a trigger configuration hash' do
             expect(subject.value)
               .to eq(project: 'some/project', strategy: 'depend')
           end
@@ -74,7 +74,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
         end
 
         describe '#errors' do
-          it 'is returns an error about unknown config key' do
+          it 'returns an error about unknown config key' do
             expect(subject.errors.first)
               .to match /trigger strategy should be depend/
           end
@@ -98,7 +98,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
 
         it { is_expected.not_to be_valid }
 
-        it 'is returns an error' do
+        it 'returns an error' do
           expect(subject.errors.first)
             .to match /config contains unknown keys: project/
         end
@@ -109,7 +109,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
 
         it { is_expected.not_to be_valid }
 
-        it 'is returns an error' do
+        it 'returns an error' do
           expect(subject.errors.first)
             .to match /config contains unknown keys: branch/
         end
@@ -137,7 +137,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
       end
 
       describe '#errors' do
-        it 'is returns an error about unknown config key' do
+        it 'returns an error about unknown config key' do
           expect(subject.errors.first)
             .to match /config contains unknown keys: unknown/
         end

ee/spec/services/ci/create_cross_project_pipeline_service_spec.rb

@@ -217,6 +217,27 @@ describe Ci::CreateCrossProjectPipelineService, '#execute' do
           # it does not auto-cancel pipelines from the same family
           it_behaves_like 'creates a child pipeline'
         end
+
+        context 'when upstream pipeline is a child pipeline' do
+          let!(:pipeline_source) 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 'does not create a further child pipeline' do
+            expect { service.execute(bridge) }
+              .not_to change { Ci::Pipeline.count }
+
+            expect(bridge.reload).to be_failed
+            expect(bridge.failure_reason).to eq 'bridge_pipeline_is_child_pipeline'
+          end
+        end
       end
     end
 

ee/spec/services/ci/create_pipeline_service_spec.rb

@@ -177,6 +177,77 @@ describe Ci::CreatePipelineService, '#execute' do
     end
   end
 
+  describe 'child pipeline triggers' do
+    context 'when YAML is valid' do
+      before do
+        stub_ci_pipeline_yaml_file <<~YAML
+          test:
+            script: rspec
+
+          deploy:
+            variables:
+              CROSS: downstream
+            stage: deploy
+            trigger:
+              include:
+                - local: path/to/child.yml
+        YAML
+      end
+
+      it 'creates bridge jobs correctly' do
+        pipeline = create_pipeline!
+
+        test = pipeline.statuses.find_by(name: 'test')
+        bridge = pipeline.statuses.find_by(name: 'deploy')
+
+        expect(pipeline).to be_persisted
+        expect(test).to be_a Ci::Build
+        expect(bridge).to be_a Ci::Bridge
+        expect(bridge.stage).to eq 'deploy'
+        expect(pipeline.statuses).to match_array [test, bridge]
+        expect(bridge.options).to eq(
+          'trigger' => { 'include' => [{ 'local' => 'path/to/child.yml' }] }
+        )
+        expect(bridge.yaml_variables)
+          .to include(key: 'CROSS', value: 'downstream', public: true)
+      end
+    end
+
+    context 'when YAML is invalid' do
+      let(:config) do
+        {
+          test: { script: 'rspec' },
+          deploy: {
+            trigger: { include: included_files }
+          }
+        }
+      end
+
+      let(:included_files) do
+        Array.new(include_max_size + 1) do |index|
+          { local: "file#{index}.yml" }
+        end
+      end
+
+      let(:include_max_size) do
+        EE::Gitlab::Ci::Config::Entry::Trigger::ComplexTrigger::SameProjectTrigger::INCLUDE_MAX_SIZE
+      end
+
+      before do
+        stub_ci_pipeline_yaml_file(YAML.dump(config))
+      end
+
+      it 'returns errors' do
+        pipeline = create_pipeline!
+
+        expect(pipeline.errors.full_messages.first).to match(/trigger:include config is too long/)
+        expect(pipeline.failure_reason).to eq 'config_error'
+        expect(pipeline).to be_persisted
+        expect(pipeline.status).to eq 'failed'
+      end
+    end
+  end
+
   def create_pipeline!
     service.execute(:push)
   end

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

@@ -12,6 +12,15 @@ module Gitlab
 
           validations do
             validates :config, array_or_string: true
+
+            validate do
+              next unless opt(:max_size)
+              next unless config.is_a?(Array)
+
+              if config.size > opt(:max_size)
+                errors.add(:config, "is too long (maximum is #{opt(:max_size)})")
+              end
+            end
           end
 
           def self.aspects