Commit 0718305c authored by Stan Hu's avatar Stan Hu

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

This reverts merge request !21817
parent 4372a6c9
...@@ -14,8 +14,7 @@ module EE ...@@ -14,8 +14,7 @@ module EE
downstream_bridge_project_not_found: 1_002, downstream_bridge_project_not_found: 1_002,
invalid_bridge_trigger: 1_003, invalid_bridge_trigger: 1_003,
upstream_bridge_project_not_found: 1_004, 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 end
end end
......
...@@ -10,8 +10,7 @@ module EE ...@@ -10,8 +10,7 @@ module EE
insufficient_upstream_permissions: 'This job could not be executed because of insufficient permissions to track the upstream project.', 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.', 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.', 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 ).freeze
EE::CommitStatusPresenter.private_constant :EE_CALLOUT_FAILURE_MESSAGES EE::CommitStatusPresenter.private_constant :EE_CALLOUT_FAILURE_MESSAGES
......
...@@ -45,13 +45,6 @@ module Ci ...@@ -45,13 +45,6 @@ module Ci
return false return false
end 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) unless can_create_downstream_pipeline?(target_ref)
@bridge.drop!(:insufficient_bridge_permissions) @bridge.drop!(:insufficient_bridge_permissions)
return false return false
......
---
title: Prevent a child pipeline to create further child pipelines
merge_request: 21817
author:
type: added
...@@ -51,7 +51,6 @@ module EE ...@@ -51,7 +51,6 @@ module EE
include ::Gitlab::Config::Entry::Attributable include ::Gitlab::Config::Entry::Attributable
include ::Gitlab::Config::Entry::Configurable include ::Gitlab::Config::Entry::Configurable
INCLUDE_MAX_SIZE = 3
ALLOWED_KEYS = %i[strategy include].freeze ALLOWED_KEYS = %i[strategy include].freeze
attributes :strategy attributes :strategy
...@@ -63,8 +62,7 @@ module EE ...@@ -63,8 +62,7 @@ module EE
entry :include, ::Gitlab::Ci::Config::Entry::Includes, entry :include, ::Gitlab::Ci::Config::Entry::Includes,
description: 'List of external YAML files to include.', description: 'List of external YAML files to include.',
reserved: true, reserved: true
metadata: { max_size: INCLUDE_MAX_SIZE }
def value def value
@config @config
......
...@@ -15,8 +15,7 @@ module EE ...@@ -15,8 +15,7 @@ module EE
downstream_bridge_project_not_found: 'downstream project could not be found', downstream_bridge_project_not_found: 'downstream project could not be found',
upstream_bridge_project_not_found: 'upstream 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_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 ).freeze
EE::Gitlab::Ci::Status::Build::Failed.private_constant :EE_REASONS EE::Gitlab::Ci::Status::Build::Failed.private_constant :EE_REASONS
end end
......
...@@ -13,7 +13,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do ...@@ -13,7 +13,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
end end
describe '#value' do describe '#value' do
it 'returns a trigger configuration hash' do it 'is returns a trigger configuration hash' do
expect(subject.value).to eq(project: 'some/project') expect(subject.value).to eq(project: 'some/project')
end end
end end
...@@ -27,7 +27,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do ...@@ -27,7 +27,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
end end
describe '#errors' do describe '#errors' do
it 'returns an error about an empty config' do it 'is returns an error about an empty config' do
expect(subject.errors.first) expect(subject.errors.first)
.to match /config can't be blank/ .to match /config can't be blank/
end end
...@@ -43,7 +43,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do ...@@ -43,7 +43,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
end end
describe '#value' do describe '#value' do
it 'returns a trigger configuration hash' do it 'is returns a trigger configuration hash' do
expect(subject.value) expect(subject.value)
.to eq(project: 'some/project', branch: 'feature') .to eq(project: 'some/project', branch: 'feature')
end end
...@@ -59,7 +59,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do ...@@ -59,7 +59,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
end end
describe '#value' do describe '#value' do
it 'returns a trigger configuration hash' do it 'is returns a trigger configuration hash' do
expect(subject.value) expect(subject.value)
.to eq(project: 'some/project', strategy: 'depend') .to eq(project: 'some/project', strategy: 'depend')
end end
...@@ -74,7 +74,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do ...@@ -74,7 +74,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
end end
describe '#errors' do describe '#errors' do
it 'returns an error about unknown config key' do it 'is returns an error about unknown config key' do
expect(subject.errors.first) expect(subject.errors.first)
.to match /trigger strategy should be depend/ .to match /trigger strategy should be depend/
end end
...@@ -98,7 +98,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do ...@@ -98,7 +98,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
it { is_expected.not_to be_valid } it { is_expected.not_to be_valid }
it 'returns an error' do it 'is returns an error' do
expect(subject.errors.first) expect(subject.errors.first)
.to match /config contains unknown keys: project/ .to match /config contains unknown keys: project/
end end
...@@ -109,7 +109,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do ...@@ -109,7 +109,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
it { is_expected.not_to be_valid } it { is_expected.not_to be_valid }
it 'returns an error' do it 'is returns an error' do
expect(subject.errors.first) expect(subject.errors.first)
.to match /config contains unknown keys: branch/ .to match /config contains unknown keys: branch/
end end
...@@ -137,7 +137,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do ...@@ -137,7 +137,7 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
end end
describe '#errors' do describe '#errors' do
it 'returns an error about unknown config key' do it 'is returns an error about unknown config key' do
expect(subject.errors.first) expect(subject.errors.first)
.to match /config contains unknown keys: unknown/ .to match /config contains unknown keys: unknown/
end end
......
...@@ -217,27 +217,6 @@ describe Ci::CreateCrossProjectPipelineService, '#execute' do ...@@ -217,27 +217,6 @@ describe Ci::CreateCrossProjectPipelineService, '#execute' do
# it does not auto-cancel pipelines from the same family # it does not auto-cancel pipelines from the same family
it_behaves_like 'creates a child pipeline' it_behaves_like 'creates a child pipeline'
end 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
end end
......
...@@ -177,77 +177,6 @@ describe Ci::CreatePipelineService, '#execute' do ...@@ -177,77 +177,6 @@ describe Ci::CreatePipelineService, '#execute' do
end end
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! def create_pipeline!
service.execute(:push) service.execute(:push)
end end
......
...@@ -12,15 +12,6 @@ module Gitlab ...@@ -12,15 +12,6 @@ module Gitlab
validations do validations do
validates :config, array_or_string: true 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 end
def self.aspects 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