Pipelines using default stages in non-standard order result in cyclic dependency error with inject_policy PEP

Why are we doing this work

When project pipeline uses the default stages in a non-standard order, such as [test, build, deploy], 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 is making a rollout in larger organizations more difficult, if teams are free to use any stages and the aim of the pipeline execution policies is to only inject jobs into the reserved stages.

When the stages are not declared, pipeline execution policies assume the default stages: [.pipeline-policy-pre, .pre, build, test, deploy, .post, .pipeline-policy-post]. The error happens because this order is thrown into the stages injection using DAG with the project stages: https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/lib/gitlab/ci/pipeline/pipeline_execution_policies/custom_stages_injector.rb#L18.

The DAG ordering is useful only when policies declare custom stages. When policy CI doesn't declare stages, we don't need to consider its stages in the CustomStagesInjector.

If we ignore inject_policy policies with undefined stages when we collect the injected stages, we will avoid the cyclic dependencies error.

Relevant links

Implementation plan

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 17cce3ce5a32..eb886e6becb6 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/services/ci/create_pipeline_service/pipeline_execution_policy_spec.rb b/ee/spec/services/ci/create_pipeline_service/pipeline_execution_policy_spec.rb
index c660af9321b1..8e379413c264 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

Verification steps

  1. Create a project
  2. In the project, create PEP without declaring stages: 
    policy-build-job:
  stage: build
  script: exit 0

policy-test-job:
  stage: test
  script: exit 0
  3. Create .gitlab-ci.yml in the project which uses non-standard order of stages: 
    stages: [test, build, deploy]

project-build:
  stage: build
  script: exit 0

project-test:
  stage: test
  script: exit 0

project-deploy:
  stage: deploy
  script: exit 0
  4. Run the pipeline and verify it can be created
Edited by 🤖 GitLab Bot 🤖