Commit 750953be authored by Fabio Pitino's avatar Fabio Pitino

Limit creation of child pipelines

* Allow at most 3 entries in trigger:include
* Don't allow a child pipeline to create child pipelies
parent 15a86e9d
......@@ -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
......
......@@ -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
......
......@@ -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
......
---
title: Prevent a child pipeline to create further child pipelines
merge_request: 21817
author:
type: added
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment