From 087b84abbbc5348f47021a24ff0db7574068ecac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20C=CC=8Cavoj?= Date: Mon, 21 Jul 2025 19:06:00 +0200 Subject: [PATCH] Fix PEP cyclic dependencies error with non-standard project stages order When project pipeline uses the default stages in a non-standard order, inject_policy pipeline execution policy has to specify the stages in the same relative order, otherwise pipelines fail to start with cyclic dependencies detected error. This change fixes it by not collecting the policy stages unless the stages are explicitly declared. Changelog: fixed EE: true --- .../pipeline_context.rb | 6 ++- .../pipeline_context_spec.rb | 16 ++++++++ .../pipeline_execution_policy_spec.rb | 37 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/ee/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context.rb b/ee/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context.rb index 17cce3ce5a3215..eb886e6becb60b 100644 --- a/ee/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context.rb +++ b/ee/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context.rb @@ -89,7 +89,9 @@ def collect_declared_stages!(new_stages) if current_policy.strategy_override_project_ci? collect_declared_override_stages!(new_stages) elsif current_policy.strategy_inject_policy? - @injected_policy_stages << new_stages + # If current policy uses default stages, we don't need to use them in StagesMerger. + # It would interfere with project stages if they are in a non-standard order. + @injected_policy_stages << new_stages if @current_policy_has_declared_stages end end @@ -123,6 +125,7 @@ def valid_stage?(stage) def enforce_stages!(config:) return config unless inject_policy_stages? + @current_policy_has_declared_stages = config[:stages].present? config = ::Gitlab::Ci::Config::Stages.new(config).inject_reserved_stages! return config if creating_policy_pipeline? @@ -192,6 +195,7 @@ def create_pipeline(policy, partition_id) # For example, it allows us to collect declared stages if @current_policy is `override_project_ci`. def with_policy_context(policy) @current_policy = policy + @current_policy_has_declared_stages = false yield.tap do @current_policy = nil end diff --git a/ee/spec/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context_spec.rb b/ee/spec/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context_spec.rb index 48c2bd2c0db2dc..6831ae6cdb4530 100644 --- a/ee/spec/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context_spec.rb +++ b/ee/spec/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context_spec.rb @@ -508,6 +508,7 @@ context 'with inject_policy' do let(:current_policy) { build(:pipeline_execution_policy_config, :inject_policy) } + let(:current_policy_has_declared_stages) { true } let(:stages1) do %w[.pipeline-policy-pre .pre build test policy-test .post .pipeline-policy-post] end @@ -516,6 +517,10 @@ %w[.pipeline-policy-pre .pre policy-build .post .pipeline-policy-post] end + before do + context.instance_variable_set(:@current_policy_has_declared_stages, current_policy_has_declared_stages) + end + it 'includes stages from all policies' do context.collect_declared_stages!(stages1) context.collect_declared_stages!(stages2) @@ -524,6 +529,17 @@ expect(context.injected_policy_stages).to contain_exactly(stages1, stages2) end + context 'when current policy does not declare stages' do + let(:current_policy_has_declared_stages) { false } + + it 'does not collect the declared stages' do + context.collect_declared_stages!(stages1) + + expect(context.override_policy_stages).to be_empty + expect(context.injected_policy_stages).to be_empty + end + end + context 'when creating a project pipeline' do let(:current_policy) { nil } diff --git a/ee/spec/services/ci/create_pipeline_service/pipeline_execution_policy_spec.rb b/ee/spec/services/ci/create_pipeline_service/pipeline_execution_policy_spec.rb index c660af9321b130..8e379413c264eb 100644 --- a/ee/spec/services/ci/create_pipeline_service/pipeline_execution_policy_spec.rb +++ b/ee/spec/services/ci/create_pipeline_service/pipeline_execution_policy_spec.rb @@ -719,6 +719,43 @@ /^Pipeline execution policy error: Cyclic dependencies detected when enforcing policies./) end end + + context 'when policy does not use custom stages and project contains default stages in different order' do + let(:namespace_policy_content) do + { namespace_pre_job: { stage: '.pipeline-policy-pre', script: 'policy build script' } } + end + + let(:project_policy_content) do + { project_post_job: { stage: '.pipeline-policy-post', script: 'policy test script' } } + end + + let(:project_ci_yaml) do + <<~YAML + stages: [test, build] + build: + stage: build + script: exit 0 + rspec: + stage: test + script: + - echo 'rspec' + YAML + end + + it 'responds with success and does not result in cyclic dependency error', :aggregate_failures do + expect { execute }.to change { Ci::Build.count }.from(0).to(4) + expect(execute).to be_success + expect(execute.payload).to be_persisted + + stages = execute.payload.stages + expect(stages.sort_by(&:position).map(&:name)).to eq(%w[.pipeline-policy-pre test build .pipeline-policy-post]) + + expect(stages.find_by(name: '.pipeline-policy-pre').builds.map(&:name)).to contain_exactly('namespace_pre_job') + expect(stages.find_by(name: 'build').builds.map(&:name)).to contain_exactly('build') + expect(stages.find_by(name: 'test').builds.map(&:name)).to contain_exactly('rspec') + expect(stages.find_by(name: '.pipeline-policy-post').builds.map(&:name)).to contain_exactly('project_post_job') + end + end end context 'when policy content does not match the valid schema' do -- GitLab