From dc3f3ec32e0797e06cf09786414bef13c061ea44 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Tue, 25 Nov 2025 21:37:45 -0500 Subject: [PATCH 01/24] Add empty_pipeline_behaviour option to Pipeline Execution Policies Adds configurable empty_pipeline_behaviour to control policy injection into empty pipelines, addressing duplicate pipeline issues. Options: - always_inject (default): Always injects policies into empty pipelines - inject_if_no_config: Only injects when project has no CI config - never_inject: Never injects policies into empty pipelines When multiple policies have different settings, the most restrictive setting applies (always_inject > inject_if_no_config > never_inject). The option is configured via pipeline_config_strategy which now accepts either a string (for backward compatibility) or an object with type and empty_pipeline_behaviour fields. See https://gitlab.com/gitlab-org/gitlab/-/work_items/582196 Changelog: changed EE: true --- .../pipeline_execution_policy/config.rb | 19 +- .../pipeline_execution_policy_content.json | 44 +++- .../security_orchestration_policy.json | 44 +++- .../pipeline/chain/evaluate_workflow_rules.rb | 2 +- .../ee/gitlab/ci/pipeline/chain/populate.rb | 28 +++ .../pipeline_context.rb | 51 +++++ .../pipeline_execution_policy_configs.rb | 15 ++ .../pipeline_execution_policy_pipeline.rb | 33 +++ ee/spec/factories/security/policies.rb | 12 ++ ...ity_orchestration_policy_configurations.rb | 4 + .../chain/evaluate_workflow_rules_spec.rb | 55 ++++- .../apply_policies_spec.rb | 9 +- .../gitlab/ci/pipeline/chain/populate_spec.rb | 106 ++++++++++ .../gitlab/ci/pipeline/chain/populate_spec.rb | 58 ------ .../pipeline_context_spec.rb | 92 ++++++++ .../pipeline_execution_policy/config_spec.rb | 44 ++++ .../pipeline_execution_policy_content_spec.rb | 196 ++++++++++++++++++ lib/gitlab/ci/pipeline/chain/populate.rb | 2 + 18 files changed, 733 insertions(+), 81 deletions(-) create mode 100644 ee/lib/ee/gitlab/ci/pipeline/chain/populate.rb create mode 100644 ee/spec/lib/ee/gitlab/ci/pipeline/chain/populate_spec.rb delete mode 100644 ee/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb create mode 100644 ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb diff --git a/ee/app/models/security/pipeline_execution_policy/config.rb b/ee/app/models/security/pipeline_execution_policy/config.rb index c75b704c17b094..9c422966cb457a 100644 --- a/ee/app/models/security/pipeline_execution_policy/config.rb +++ b/ee/app/models/security/pipeline_execution_policy/config.rb @@ -10,10 +10,12 @@ class Config DEFAULT_SUFFIX_STRATEGY = 'on_conflict' SUFFIX_STRATEGIES = { on_conflict: 'on_conflict', never: 'never' }.freeze DEFAULT_SKIP_CI_STRATEGY = { allowed: false }.freeze + DEFAULT_EMPTY_PIPELINE_BEHAVIOUR = 'always_inject' + EMPTY_PIPELINE_BEHAVIOUR_OPTIONS = %w[always_inject inject_if_no_config never_inject].freeze POLICY_JOB_SUFFIX = ':policy' attr_reader :content, :config_strategy, :suffix_strategy, :policy_project_id, :policy_index, :name, - :skip_ci_strategy, :variables_override_strategy, :policy_config, :policy_sha + :skip_ci_strategy, :variables_override_strategy, :policy_config, :policy_sha, :empty_pipeline_behaviour delegate :experiment_enabled?, to: :policy_config @@ -22,7 +24,7 @@ def initialize(policy:, policy_config:, policy_index:) @policy_config = policy_config @policy_project_id = policy_config.security_policy_management_project_id @policy_index = policy_index - @config_strategy = policy.fetch(:pipeline_config_strategy).to_sym + parse_pipeline_config_strategy(policy.fetch(:pipeline_config_strategy)) @suffix_strategy = policy[:suffix] || DEFAULT_SUFFIX_STRATEGY @name = policy.fetch(:name) @skip_ci_strategy = policy[:skip_ci].presence || DEFAULT_SKIP_CI_STRATEGY @@ -55,6 +57,19 @@ def suffix_on_conflict? def skip_ci_allowed?(user_id) skip_ci_allowed_for_strategy?(skip_ci_strategy, user_id) end + + private + + def parse_pipeline_config_strategy(strategy_config) + if strategy_config.is_a?(Hash) + @config_strategy = strategy_config.fetch(:type).to_sym + @empty_pipeline_behaviour = + strategy_config.fetch(:empty_pipeline_behaviour, DEFAULT_EMPTY_PIPELINE_BEHAVIOUR).to_s + else + @config_strategy = strategy_config.to_sym + @empty_pipeline_behaviour = DEFAULT_EMPTY_PIPELINE_BEHAVIOUR + end + end end end end diff --git a/ee/app/validators/json_schemas/pipeline_execution_policy_content.json b/ee/app/validators/json_schemas/pipeline_execution_policy_content.json index 22b34c2c82a403..80dbe5c5f35d07 100644 --- a/ee/app/validators/json_schemas/pipeline_execution_policy_content.json +++ b/ee/app/validators/json_schemas/pipeline_execution_policy_content.json @@ -38,12 +38,44 @@ "additionalProperties": false }, "pipeline_config_strategy": { - "description": "Defines the method for merging the policy configuration with the project pipeline. `inject_ci` preserves the project CI configuration and injects additional jobs from the policy. Having multiple policies enabled injects all jobs additively. `inject_policy` behaves like `inject_ci`, but allows custom policy stages to be injected too. `override_project_ci` replaces the project CI configuration and keeps only the policy jobs in the pipeline.", - "type": "string", - "enum": [ - "inject_ci", - "inject_policy", - "override_project_ci" + "description": "Defines the method for merging the policy configuration with the project pipeline. Can be a string or an object with type and empty_pipeline_behaviour options.", + "oneOf": [ + { + "type": "string", + "enum": [ + "inject_ci", + "inject_policy", + "override_project_ci" + ], + "description": "Simple string format for backward compatibility. `inject_ci` preserves the project CI configuration and injects additional jobs from the policy. Having multiple policies enabled injects all jobs additively. `inject_policy` behaves like `inject_ci`, but allows custom policy stages to be injected too. `override_project_ci` replaces the project CI configuration and keeps only the policy jobs in the pipeline." + }, + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "inject_ci", + "inject_policy", + "override_project_ci" + ], + "description": "The strategy type for merging policy configuration." + }, + "empty_pipeline_behaviour": { + "type": "string", + "enum": [ + "always_inject", + "inject_if_no_config", + "never_inject" + ], + "description": "Controls when policies inject into empty pipelines. `always_inject`: Always injects (current default). `inject_if_no_config`: Only injects when project has no CI config. `never_inject`: Never injects a fallback pipeline." + } + }, + "required": [ + "type" + ], + "additionalProperties": false + } ] }, "suffix": { diff --git a/ee/app/validators/json_schemas/security_orchestration_policy.json b/ee/app/validators/json_schemas/security_orchestration_policy.json index 6aad52e58a8715..318e125347b184 100644 --- a/ee/app/validators/json_schemas/security_orchestration_policy.json +++ b/ee/app/validators/json_schemas/security_orchestration_policy.json @@ -358,12 +358,44 @@ "$ref": "#/$defs/pipeline_execution_content" }, "pipeline_config_strategy": { - "description": "Defines the method for merging the policy configuration with the project pipeline. `inject_ci` preserves the project CI configuration and injects additional jobs from the policy. Having multiple policies enabled injects all jobs additively. `inject_policy` behaves like `inject_ci`, but allows custom policy stages to be injected too. `override_project_ci` replaces the project CI configuration and keeps only the policy jobs in the pipeline.", - "type": "string", - "enum": [ - "inject_ci", - "inject_policy", - "override_project_ci" + "description": "Defines the method for merging the policy configuration with the project pipeline. Can be a string or an object with type and empty_pipeline_behaviour options.", + "oneOf": [ + { + "type": "string", + "enum": [ + "inject_ci", + "inject_policy", + "override_project_ci" + ], + "description": "Simple string format for backward compatibility. `inject_ci` preserves the project CI configuration and injects additional jobs from the policy. Having multiple policies enabled injects all jobs additively. `inject_policy` behaves like `inject_ci`, but allows custom policy stages to be injected too. `override_project_ci` replaces the project CI configuration and keeps only the policy jobs in the pipeline." + }, + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "inject_ci", + "inject_policy", + "override_project_ci" + ], + "description": "The strategy type for merging policy configuration." + }, + "empty_pipeline_behaviour": { + "type": "string", + "enum": [ + "always_inject", + "inject_if_no_config", + "never_inject" + ], + "description": "Controls when policies inject into empty pipelines. `always_inject`: Always injects (current default). `inject_if_no_config`: Only injects when project has no CI config. `never_inject`: Never injects a fallback pipeline." + } + }, + "required": [ + "type" + ], + "additionalProperties": false + } ] }, "suffix": { diff --git a/ee/lib/ee/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb b/ee/lib/ee/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb index 298c16ce84fbf5..6eccbc4c900074 100644 --- a/ee/lib/ee/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb +++ b/ee/lib/ee/gitlab/ci/pipeline/chain/evaluate_workflow_rules.rb @@ -12,7 +12,7 @@ module EvaluateWorkflowRules override :force_pipeline_creation_to_continue? def force_pipeline_creation_to_continue? - command.pipeline_policy_context.pipeline_execution_context.has_execution_policy_pipelines? + command.pipeline_policy_context.pipeline_execution_context.force_pipeline_creation?(pipeline) end end end diff --git a/ee/lib/ee/gitlab/ci/pipeline/chain/populate.rb b/ee/lib/ee/gitlab/ci/pipeline/chain/populate.rb new file mode 100644 index 00000000000000..8b315b991fea68 --- /dev/null +++ b/ee/lib/ee/gitlab/ci/pipeline/chain/populate.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module EE + module Gitlab + module Ci + module Pipeline + module Chain + module Populate + extend ::Gitlab::Utils::Override + + private + + override :has_execution_policy_pipelines? + def has_execution_policy_pipelines? + super && force_pipeline_creation_for_policies? + end + + def force_pipeline_creation_for_policies? + command.pipeline_policy_context + .pipeline_execution_context + .force_pipeline_creation?(pipeline) + end + end + end + end + end + end +end 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 6cdec159a07f14..a9448843088ec8 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 @@ -79,6 +79,33 @@ def scheduled_execution_policy_pipeline? source == ::Security::PipelineExecutionPolicies::RunScheduleWorker::PIPELINE_SOURCE end + def force_pipeline_creation?(pipeline) + return false unless has_execution_policy_pipelines? + + behaviour = effective_empty_pipeline_behaviour + + return true if behaviour.nil? || behaviour == 'always_inject' + return false if behaviour == 'never_inject' + + # Handle 'inject_if_no_config' behavior + if behaviour == 'inject_if_no_config' + # If the project has a CI config, we only inject if the project creates a pipeline. + # This is because we don't have a good way of knowing when both the MR and branch + # pipeline creation has been attempted. To ensure PEPs run in such projects, the + # "Pipelines must succeed" setting is needed as well. + return true unless pipeline.stages.empty? + + # Only continue if we're using the fallback source + return false unless pipeline.pipeline_execution_policy_forced? + + # For projects with no CI config, always inject + return true + end + + # Unknown behaviour defaults to always_inject + true + end + def skip_ci_allowed? return true unless has_execution_policy_pipelines? @@ -257,6 +284,30 @@ def stages_compatible?(stages, target_stages) stages == target_stages & stages end + def effective_empty_pipeline_behaviour + # Determine the effective empty_pipeline_behaviour setting + # If policies have different settings, use the most restrictive + # (always_inject > inject_if_no_config > never_inject) + # "Restrictive" means most limiting to project freedom + behaviour_values = policy_pipelines.filter_map do |policy_pipeline| + if policy_pipeline.policy_config.experiment_enabled?(:empty_pipeline_behaviour_option) + policy_pipeline.policy_config.empty_pipeline_behaviour + end + end + + behaviour_values.min_by { |value| empty_pipeline_behaviour_priority(value) } + end + + def empty_pipeline_behaviour_priority(behaviour) + # Lower number = more restrictive (most limiting to project freedom) + case behaviour + when 'always_inject' then 0 + when 'inject_if_no_config' then 1 + when 'never_inject' then 2 + else 3 # unknown + end + end + delegate :measure, to: ::Security::SecurityOrchestrationPolicies::ObserveHistogramsService end end diff --git a/ee/spec/factories/security/pipeline_execution_policy_configs.rb b/ee/spec/factories/security/pipeline_execution_policy_configs.rb index 812e678b1ed448..dc2c3808ecffe3 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_configs.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_configs.rb @@ -45,5 +45,20 @@ trait :variables_override_disallowed do policy factory: [:pipeline_execution_policy, :variables_override_disallowed] end + + trait :empty_pipeline_always_inject do + policy factory: [:pipeline_execution_policy, :empty_pipeline_always_inject] + policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment] + end + + trait :empty_pipeline_inject_if_no_config do + policy factory: [:pipeline_execution_policy, :empty_pipeline_inject_if_no_config] + policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment] + end + + trait :empty_pipeline_never_inject do + policy factory: [:pipeline_execution_policy, :empty_pipeline_never_inject] + policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment] + end end end diff --git a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb index 3fda9348ecf699..23e024bcb1b94c 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb @@ -31,6 +31,39 @@ policy_config factory: [:pipeline_execution_policy_config, :skip_ci_disallowed] end + trait :empty_pipeline_always_inject do + policy_config do + association( + :pipeline_execution_policy_config, :empty_pipeline_always_inject, + policy_config: association( + :security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment + ) + ) + end + end + + trait :empty_pipeline_inject_if_no_config do + policy_config do + association( + :pipeline_execution_policy_config, :empty_pipeline_inject_if_no_config, + policy_config: association( + :security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment + ) + ) + end + end + + trait :empty_pipeline_never_inject do + policy_config do + association( + :pipeline_execution_policy_config, :empty_pipeline_never_inject, + policy_config: association( + :security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment + ) + ) + end + end + transient do job_script { nil } end diff --git a/ee/spec/factories/security/policies.rb b/ee/spec/factories/security/policies.rb index de454b4d206bd9..17289b86b70803 100644 --- a/ee/spec/factories/security/policies.rb +++ b/ee/spec/factories/security/policies.rb @@ -372,6 +372,18 @@ pipeline_config_strategy { 'inject_policy' } end + trait :empty_pipeline_always_inject do + pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behaviour: 'always_inject' } } + end + + trait :empty_pipeline_inject_if_no_config do + pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behaviour: 'inject_if_no_config' } } + end + + trait :empty_pipeline_never_inject do + pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behaviour: 'never_inject' } } + end + trait :suffix_on_conflict do suffix { 'on_conflict' } end diff --git a/ee/spec/factories/security_orchestration_policy_configurations.rb b/ee/spec/factories/security_orchestration_policy_configurations.rb index 5fb4673fdbbfe3..19108d8c1dfa3d 100644 --- a/ee/spec/factories/security_orchestration_policy_configurations.rb +++ b/ee/spec/factories/security_orchestration_policy_configurations.rb @@ -10,5 +10,9 @@ project { nil } namespace end + + trait :with_empty_pipeline_behaviour_experiment do + experiments { { empty_pipeline_behaviour_option: { enabled: true } } } + end end end diff --git a/ee/spec/lib/ee/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb b/ee/spec/lib/ee/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb index e845017bf43ed3..293c46fb944a31 100644 --- a/ee/spec/lib/ee/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb +++ b/ee/spec/lib/ee/gitlab/ci/pipeline/chain/evaluate_workflow_rules_spec.rb @@ -103,19 +103,60 @@ end context 'with execution_policy_pipelines' do + let(:pipeline_execution_context) do + instance_double(Gitlab::Ci::Pipeline::PipelineExecutionPolicies::PipelineContext) + end + before do allow(command) - .to receive_message_chain(:pipeline_policy_context, :pipeline_execution_context, - :has_execution_policy_pipelines?).and_return(true) - step.perform! + .to receive_message_chain(:pipeline_policy_context, :pipeline_execution_context) + .and_return(pipeline_execution_context) end - it_behaves_like 'pipeline not skipped' + context 'when force_pipeline_creation? returns true' do + before do + allow(pipeline_execution_context).to receive(:force_pipeline_creation?).with(pipeline).and_return(true) + end - it 'clears the jobs from the main pipeline in the yaml_processor_result' do - expect(yaml_processor_result).to receive(:clear_jobs!) + it_behaves_like 'pipeline not skipped' do + before do + step.perform! + end + end - step.perform! + it 'clears the jobs from the main pipeline in the yaml_processor_result' do + expect(yaml_processor_result).to receive(:clear_jobs!) + + step.perform! + end + end + + context 'when force_pipeline_creation? returns false' do + before do + allow(pipeline_execution_context).to receive(:force_pipeline_creation?).with(pipeline).and_return(false) + step.perform! + end + + it 'does not save the pipeline' do + expect(pipeline).not_to be_persisted + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + + it 'attaches an error to the pipeline' do + expect(pipeline.errors[:base]).to include(sanitize_message(Ci::Pipeline.workflow_rules_failure_message)) + end + + it 'saves workflow_rules_result' do + expect(command.workflow_rules_result.variables).to eq(workflow_rules_variables) + end + + it 'sets the failure reason', :aggregate_failures do + expect(pipeline).to be_failed + expect(pipeline).to be_filtered_by_workflow_rules + end end end end diff --git a/ee/spec/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies_spec.rb b/ee/spec/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies_spec.rb index 265fd73fad2c9e..fc17304405563e 100644 --- a/ee/spec/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies_spec.rb +++ b/ee/spec/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies_spec.rb @@ -4,6 +4,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::PipelineExecutionPolicies::ApplyPolicies, feature_category: :security_policy_management do include Ci::PipelineExecutionPolicyHelpers + include Ci::PipelineMessageHelpers let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, group: group) } @@ -492,10 +493,16 @@ def run_previous_chain(pipeline, command) Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules.new(pipeline, command), Gitlab::Ci::Pipeline::Chain::Seed.new(pipeline, command), Gitlab::Ci::Pipeline::Chain::Populate.new(pipeline, command) - ].map(&:perform!) + ].each do |step| + step.perform! + break if step.break? + end end def perform_chain(pipeline, command) + # If the chain already broke in a previous step, ApplyPolicies should not run + raise "Chain already broken before ApplyPolicies step" if pipeline.errors.any? + described_class.new(pipeline, command).perform! end end diff --git a/ee/spec/lib/ee/gitlab/ci/pipeline/chain/populate_spec.rb b/ee/spec/lib/ee/gitlab/ci/pipeline/chain/populate_spec.rb new file mode 100644 index 00000000000000..b201b7d6798e33 --- /dev/null +++ b/ee/spec/lib/ee/gitlab/ci/pipeline/chain/populate_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Pipeline::Chain::Populate, feature_category: :pipeline_composition do + include Ci::PipelineMessageHelpers + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + + let(:pipeline) do + build(:ci_empty_pipeline, project: project, ref: 'master', user: user) + end + + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, + origin_ref: 'master', + pipeline_policy_context: instance_double( + Gitlab::Ci::Pipeline::ExecutionPolicies::PipelineContext, + pipeline_execution_context: instance_double( + Gitlab::Ci::Pipeline::PipelineExecutionPolicies::PipelineContext, + valid_stage?: true, + applying_config_override?: false, + has_execution_policy_pipelines?: true, + job_options: {}, + creating_policy_pipeline?: true, + force_pipeline_creation?: force_pipeline_creation + ) + ), + seeds_block: nil + ) + end + + let(:force_pipeline_creation) { true } + + let(:dependencies) do + [ + Gitlab::Ci::Pipeline::Chain::Config::Content.new(pipeline, command), + Gitlab::Ci::Pipeline::Chain::Config::Process.new(pipeline, command), + Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules.new(pipeline, command), + Gitlab::Ci::Pipeline::Chain::SeedBlock.new(pipeline, command), + Gitlab::Ci::Pipeline::Chain::Seed.new(pipeline, command) + ] + end + + let(:step) do + result = described_class.new(pipeline, command) + + dependencies.each do |dependency| + dependency.perform! + + # Sanity check. All the dependencies are required for the step object to be valid + raise "Dependency #{dependency.class} failed with #{pipeline.errors.full_messages}" if dependency.break? + end + + result + end + + let(:config) do + { rspec: { + script: 'ls', + only: ['something'] + } } + end + + before do + stub_ci_pipeline_yaml_file(config.to_yaml) + + allow(command.pipeline_policy_context.pipeline_execution_context).to( + receive(:enforce_stages!) { |config:| config } + ) + end + + context 'when pipeline is empty and there are policy pipelines' do + it 'does not break the chain' do + step.perform! + + expect(step.break?).to be false + end + + it 'does not append an error' do + step.perform! + + expect(pipeline.errors).to be_empty + end + + context 'when execution is not forced' do + let(:force_pipeline_creation) { false } + + it 'breaks the chain' do + step.perform! + + expect(step.break?).to be true + end + + it 'appends an error about no stages/jobs' do + step.perform! + + expect(pipeline.errors.to_a) + .to include sanitize_message(::Ci::Pipeline.rules_failure_message) + end + end + end +end diff --git a/ee/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/ee/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb deleted file mode 100644 index 8ce752bf29a108..00000000000000 --- a/ee/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ /dev/null @@ -1,58 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Ci::Pipeline::Chain::Populate, feature_category: :pipeline_composition do - let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { create(:user) } - - let(:pipeline) do - build(:ci_pipeline, project: project, ref: 'master', user: user) - end - - let(:command) do - Gitlab::Ci::Pipeline::Chain::Command.new( - project: project, - current_user: user, - origin_ref: 'master', - seeds_block: nil) - end - - let(:step) { described_class.new(pipeline, command) } - - before do - stub_ci_pipeline_yaml_file(YAML.dump(config)) - end - - context 'when pipeline is empty and there are policy_pipelines' do - let(:command) do - Gitlab::Ci::Pipeline::Chain::Command.new( - project: project, - current_user: user, - origin_ref: 'master', - pipeline_policy_context: instance_double( - Gitlab::Ci::Pipeline::ExecutionPolicies::PipelineContext, - pipeline_execution_context: instance_double( - Gitlab::Ci::Pipeline::PipelineExecutionPolicies::PipelineContext, - policy_pipelines: [build(:pipeline_execution_policy_pipeline)] - ) - ), - seeds_block: nil) - end - - let(:config) do - { rspec: { - script: 'ls', - only: ['something'] - } } - end - - it 'does not break the chain' do - expect(step.break?).to be false - end - - it 'does not append an error' do - expect(pipeline.errors).to be_empty - end - end -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 1843a63cee8766..de60e74e718fc9 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 @@ -671,6 +671,98 @@ end end + describe '#force_pipeline_creation?' do + subject(:force_creation) { context.force_pipeline_creation?(pipeline) } + + include_context 'with mocked policy_pipelines' + + it { is_expected.to eq(false) } + + context 'with policy_pipelines' do + context 'when empty_pipeline_behaviour is "always_inject"' do + let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_always_inject) } + + it { is_expected.to eq(true) } + end + + context 'when empty_pipeline_behaviour is "never_inject"' do + let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_never_inject) } + + it { is_expected.to eq(false) } + end + + context 'when empty_pipeline_behaviour is "inject_if_no_config"' do + let(:policy_pipelines) do + build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_inject_if_no_config) + end + + context 'when project has CI config (pipeline has stages)' do + before do + allow(pipeline).to receive(:stages).and_return([instance_double(Ci::Stage)]) + end + + it 'forces pipeline creation' do + expect(force_creation).to eq(true) + end + end + + context 'when project has no CI config (pipeline has no stages)' do + before do + allow(pipeline).to receive_messages(stages: [], pipeline_execution_policy_forced?: true) + end + + it 'forces pipeline creation' do + expect(force_creation).to eq(true) + end + end + end + + context 'when multiple policies have different empty_pipeline_behaviour settings' do + let(:policy_pipelines) do + [ + build(:pipeline_execution_policy_pipeline, :empty_pipeline_always_inject), + build(:pipeline_execution_policy_pipeline, :empty_pipeline_never_inject) + ] + end + + it 'uses the most restrictive setting (always_inject) and forces creation' do + expect(force_creation).to eq(true) + end + end + + context 'when policies have inject_if_no_config and always_inject' do + let(:policy_pipelines) do + [ + build(:pipeline_execution_policy_pipeline, :empty_pipeline_always_inject), + build(:pipeline_execution_policy_pipeline, :empty_pipeline_inject_if_no_config) + ] + end + + it 'uses the more restrictive setting (always_inject) and forces creation' do + expect(force_creation).to eq(true) + end + end + + context 'when experiment is disabled' do + let(:policy_config_without_experiment) do + build(:security_orchestration_policy_configuration) + end + + let(:policy_pipelines) do + [ + build(:pipeline_execution_policy_pipeline, + policy_config: build(:pipeline_execution_policy_config, :empty_pipeline_never_inject, + policy_config: policy_config_without_experiment)) + ] + end + + it 'ignores empty_pipeline_behaviour and forces creation (default behavior)' do + expect(force_creation).to eq(true) + end + end + end + end + describe '#skip_ci_allowed?' do subject { context.skip_ci_allowed? } diff --git a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb b/ee/spec/models/security/pipeline_execution_policy/config_spec.rb index 70d88eb57e6445..ca336f40304c70 100644 --- a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb +++ b/ee/spec/models/security/pipeline_execution_policy/config_spec.rb @@ -46,6 +46,50 @@ end end + describe '#empty_pipeline_behaviour' do + subject(:empty_pipeline_behaviour_value) { config.empty_pipeline_behaviour } + + context 'with string strategy' do + let(:policy) { build(:pipeline_execution_policy, pipeline_config_strategy: 'inject_ci') } + + it 'defaults to always_inject' do + expect(empty_pipeline_behaviour_value).to eq('always_inject') + end + end + + context 'with object strategy and empty_pipeline_behaviour specified' do + let(:policy) do + build(:pipeline_execution_policy, + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'inject_if_no_config' }) + end + + it 'returns the specified empty_pipeline_behaviour value' do + expect(empty_pipeline_behaviour_value).to eq('inject_if_no_config') + end + end + + context 'with object strategy and no empty_pipeline_behaviour specified' do + let(:policy) do + build(:pipeline_execution_policy, pipeline_config_strategy: { type: 'inject_policy' }) + end + + it 'defaults to always_inject' do + expect(empty_pipeline_behaviour_value).to eq('always_inject') + end + end + + context 'with empty_pipeline_behaviour set to never_inject' do + let(:policy) do + build(:pipeline_execution_policy, + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'never_inject' }) + end + + it 'returns never_inject' do + expect(empty_pipeline_behaviour_value).to eq('never_inject') + end + end + end + describe '#suffix' do subject { config.suffix } diff --git a/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb b/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb new file mode 100644 index 00000000000000..ada2588e0d04bb --- /dev/null +++ b/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb @@ -0,0 +1,196 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'pipeline_execution_policy_content.json', feature_category: :security_policy_management do + let(:schema_path) do + Rails.root.join("ee/app/validators/json_schemas/pipeline_execution_policy_content.json") + end + + let(:schema) { JSONSchemer.schema(schema_path) } + let(:base_policy) do + { + content: { include: [{ project: "compliance-project", file: "compliance-pipeline.yml" }] } + } + end + + describe 'pipeline_config_strategy' do + context 'with string format' do + context 'with valid strategy' do + %w[inject_ci inject_policy override_project_ci].each do |strategy| + it "accepts #{strategy}" do + policy = base_policy.merge(pipeline_config_strategy: strategy) + expect(schema.valid?(policy)).to be true + end + end + end + + context 'with invalid strategy' do + it 'rejects unknown strategy' do + policy = base_policy.merge(pipeline_config_strategy: 'invalid_strategy') + expect(schema.valid?(policy)).to be false + end + end + end + + context 'with object format' do + context 'with valid type and empty_pipeline_behaviour' do + it 'accepts inject_policy with empty_pipeline_behaviour always_inject' do + policy = base_policy.merge( + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'always_inject' } + ) + expect(schema.valid?(policy)).to be true + end + + it 'accepts inject_policy with empty_pipeline_behaviour inject_if_no_config' do + policy = base_policy.merge( + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'inject_if_no_config' } + ) + expect(schema.valid?(policy)).to be true + end + + it 'accepts inject_policy with empty_pipeline_behaviour never_inject' do + policy = base_policy.merge( + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'never_inject' } + ) + expect(schema.valid?(policy)).to be true + end + + it 'accepts inject_ci with empty_pipeline_behaviour' do + policy = base_policy.merge( + pipeline_config_strategy: { type: 'inject_ci', empty_pipeline_behaviour: 'inject_if_no_config' } + ) + expect(schema.valid?(policy)).to be true + end + + it 'accepts override_project_ci with empty_pipeline_behaviour' do + policy = base_policy.merge( + pipeline_config_strategy: { type: 'override_project_ci', empty_pipeline_behaviour: 'never_inject' } + ) + expect(schema.valid?(policy)).to be true + end + end + + context 'with type only (no empty_pipeline_behaviour)' do + it 'accepts type without empty_pipeline_behaviour' do + policy = base_policy.merge( + pipeline_config_strategy: { type: 'inject_policy' } + ) + expect(schema.valid?(policy)).to be true + end + end + + context 'with invalid values' do + it 'rejects invalid type' do + policy = base_policy.merge( + pipeline_config_strategy: { type: 'invalid_type', empty_pipeline_behaviour: 'always_inject' } + ) + expect(schema.valid?(policy)).to be false + end + + it 'rejects invalid empty_pipeline_behaviour' do + policy = base_policy.merge( + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'invalid_behaviour' } + ) + expect(schema.valid?(policy)).to be false + end + + it 'rejects object without type' do + policy = base_policy.merge( + pipeline_config_strategy: { empty_pipeline_behaviour: 'always_inject' } + ) + expect(schema.valid?(policy)).to be false + end + + it 'rejects object with additional properties' do + policy = base_policy.merge( + pipeline_config_strategy: { + type: 'inject_policy', empty_pipeline_behaviour: 'always_inject', extra: 'value' + } + ) + expect(schema.valid?(policy)).to be false + end + end + end + end + + describe 'suffix' do + context 'with valid values' do + it 'accepts on_conflict' do + policy = base_policy.merge(pipeline_config_strategy: 'inject_ci', suffix: 'on_conflict') + expect(schema.valid?(policy)).to be true + end + + it 'accepts never' do + policy = base_policy.merge(pipeline_config_strategy: 'inject_ci', suffix: 'never') + expect(schema.valid?(policy)).to be true + end + + it 'accepts null' do + policy = base_policy.merge(pipeline_config_strategy: 'inject_ci', suffix: nil) + expect(schema.valid?(policy)).to be true + end + end + + context 'with invalid values' do + it 'rejects invalid suffix' do + policy = base_policy.merge(pipeline_config_strategy: 'inject_ci', suffix: 'invalid') + expect(schema.valid?(policy)).to be false + end + end + end + + describe 'skip_ci' do + context 'with valid configuration' do + it 'accepts allowed: true' do + policy = base_policy.merge(pipeline_config_strategy: 'inject_ci', skip_ci: { allowed: true }) + expect(schema.valid?(policy)).to be true + end + + it 'accepts allowed: false with allowlist' do + policy = base_policy.merge( + pipeline_config_strategy: 'inject_ci', + skip_ci: { allowed: false, allowlist: { users: [{ id: 123 }] } } + ) + expect(schema.valid?(policy)).to be true + end + end + + context 'with invalid configuration' do + it 'rejects skip_ci without allowed' do + policy = base_policy.merge(pipeline_config_strategy: 'inject_ci', skip_ci: { allowlist: {} }) + expect(schema.valid?(policy)).to be false + end + end + end + + describe 'variables_override' do + context 'with valid configuration' do + it 'accepts allowed: true' do + policy = base_policy.merge( + pipeline_config_strategy: 'inject_ci', + variables_override: { allowed: true } + ) + expect(schema.valid?(policy)).to be true + end + + it 'accepts allowed: false with exceptions' do + policy = base_policy.merge( + pipeline_config_strategy: 'inject_ci', + variables_override: { allowed: false, exceptions: %w[VAR1 VAR2] } + ) + expect(schema.valid?(policy)).to be true + end + end + + context 'with invalid configuration' do + it 'rejects variables_override without allowed' do + policy = base_policy.merge( + pipeline_config_strategy: 'inject_ci', + variables_override: { exceptions: %w[VAR1] } + ) + expect(schema.valid?(policy)).to be false + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/populate.rb b/lib/gitlab/ci/pipeline/chain/populate.rb index c7425199dbde31..996b86da3aecef 100644 --- a/lib/gitlab/ci/pipeline/chain/populate.rb +++ b/lib/gitlab/ci/pipeline/chain/populate.rb @@ -56,3 +56,5 @@ def stage_names end end end + +Gitlab::Ci::Pipeline::Chain::Populate.prepend_mod -- GitLab From 104ff3bff745ab3b97be7b9afbad32258ffbced8 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Thu, 4 Dec 2025 12:57:46 -0500 Subject: [PATCH 02/24] Wrap in feature flag --- ...cution_policy_empty_pipeline_behaviour.yml | 10 ++ .../pipeline_context.rb | 1 + .../pipeline_context_spec.rb | 130 ++++++++++-------- 3 files changed, 85 insertions(+), 56 deletions(-) create mode 100644 config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behaviour.yml diff --git a/config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behaviour.yml b/config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behaviour.yml new file mode 100644 index 00000000000000..18d26cb4210dcd --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behaviour.yml @@ -0,0 +1,10 @@ +--- +name: pipeline_execution_policy_empty_pipeline_behaviour +description: Enable configurable empty_pipeline_behaviour option for Pipeline Execution Policies +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/work_items/582196 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/214258 +rollout_issue_url: +milestone: "18.7" +group: group::security policies +type: gitlab_com_derisk +default_enabled: false 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 a9448843088ec8..43cf8e4fb21dff 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 @@ -81,6 +81,7 @@ def scheduled_execution_policy_pipeline? def force_pipeline_creation?(pipeline) return false unless has_execution_policy_pipelines? + return true unless Feature.enabled?(:pipeline_execution_policy_empty_pipeline_behaviour, pipeline.project) behaviour = effective_empty_pipeline_behaviour 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 de60e74e718fc9..0474f23b71bda4 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 @@ -679,85 +679,103 @@ it { is_expected.to eq(false) } context 'with policy_pipelines' do - context 'when empty_pipeline_behaviour is "always_inject"' do - let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_always_inject) } + context 'when feature flag is disabled' do + let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_never_inject) } - it { is_expected.to eq(true) } + before do + stub_feature_flags(pipeline_execution_policy_empty_pipeline_behaviour: false) + end + + it 'always forces pipeline creation (default behavior)' do + expect(force_creation).to eq(true) + end end - context 'when empty_pipeline_behaviour is "never_inject"' do - let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_never_inject) } + context 'when feature flag is enabled' do + before do + stub_feature_flags(pipeline_execution_policy_empty_pipeline_behaviour: true) + end - it { is_expected.to eq(false) } - end + context 'when empty_pipeline_behaviour is "always_inject"' do + let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_always_inject) } - context 'when empty_pipeline_behaviour is "inject_if_no_config"' do - let(:policy_pipelines) do - build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_inject_if_no_config) + it { is_expected.to eq(true) } end - context 'when project has CI config (pipeline has stages)' do - before do - allow(pipeline).to receive(:stages).and_return([instance_double(Ci::Stage)]) - end + context 'when empty_pipeline_behaviour is "never_inject"' do + let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_never_inject) } - it 'forces pipeline creation' do - expect(force_creation).to eq(true) - end + it { is_expected.to eq(false) } end - context 'when project has no CI config (pipeline has no stages)' do - before do - allow(pipeline).to receive_messages(stages: [], pipeline_execution_policy_forced?: true) + context 'when empty_pipeline_behaviour is "inject_if_no_config"' do + let(:policy_pipelines) do + build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_inject_if_no_config) end - it 'forces pipeline creation' do - expect(force_creation).to eq(true) + context 'when project has CI config (pipeline has stages)' do + before do + allow(pipeline).to receive(:stages).and_return([instance_double(Ci::Stage)]) + end + + it 'forces pipeline creation' do + expect(force_creation).to eq(true) + end end - end - end - context 'when multiple policies have different empty_pipeline_behaviour settings' do - let(:policy_pipelines) do - [ - build(:pipeline_execution_policy_pipeline, :empty_pipeline_always_inject), - build(:pipeline_execution_policy_pipeline, :empty_pipeline_never_inject) - ] - end + context 'when project has no CI config (pipeline has no stages)' do + before do + allow(pipeline).to receive_messages(stages: [], pipeline_execution_policy_forced?: true) + end - it 'uses the most restrictive setting (always_inject) and forces creation' do - expect(force_creation).to eq(true) + it 'forces pipeline creation' do + expect(force_creation).to eq(true) + end + end end - end - context 'when policies have inject_if_no_config and always_inject' do - let(:policy_pipelines) do - [ - build(:pipeline_execution_policy_pipeline, :empty_pipeline_always_inject), - build(:pipeline_execution_policy_pipeline, :empty_pipeline_inject_if_no_config) - ] - end + context 'when multiple policies have different empty_pipeline_behaviour settings' do + let(:policy_pipelines) do + [ + build(:pipeline_execution_policy_pipeline, :empty_pipeline_always_inject), + build(:pipeline_execution_policy_pipeline, :empty_pipeline_never_inject) + ] + end - it 'uses the more restrictive setting (always_inject) and forces creation' do - expect(force_creation).to eq(true) + it 'uses the most restrictive setting (always_inject) and forces creation' do + expect(force_creation).to eq(true) + end end - end - context 'when experiment is disabled' do - let(:policy_config_without_experiment) do - build(:security_orchestration_policy_configuration) - end + context 'when policies have inject_if_no_config and always_inject' do + let(:policy_pipelines) do + [ + build(:pipeline_execution_policy_pipeline, :empty_pipeline_always_inject), + build(:pipeline_execution_policy_pipeline, :empty_pipeline_inject_if_no_config) + ] + end - let(:policy_pipelines) do - [ - build(:pipeline_execution_policy_pipeline, - policy_config: build(:pipeline_execution_policy_config, :empty_pipeline_never_inject, - policy_config: policy_config_without_experiment)) - ] + it 'uses the more restrictive setting (always_inject) and forces creation' do + expect(force_creation).to eq(true) + end end - it 'ignores empty_pipeline_behaviour and forces creation (default behavior)' do - expect(force_creation).to eq(true) + context 'when experiment is disabled' do + let(:policy_config_without_experiment) do + build(:security_orchestration_policy_configuration) + end + + let(:policy_pipelines) do + [ + build(:pipeline_execution_policy_pipeline, + policy_config: build(:pipeline_execution_policy_config, :empty_pipeline_never_inject, + policy_config: policy_config_without_experiment)) + ] + end + + it 'ignores empty_pipeline_behaviour and forces creation (default behavior)' do + expect(force_creation).to eq(true) + end end end end -- GitLab From 9767c4c11290bbcf04803f4a80fff4e5980597c1 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Thu, 4 Dec 2025 22:32:03 -0500 Subject: [PATCH 03/24] Add validation to config parser --- .../pipeline_execution_policy/config.rb | 9 +++-- .../pipeline_execution_policy/config_spec.rb | 33 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/ee/app/models/security/pipeline_execution_policy/config.rb b/ee/app/models/security/pipeline_execution_policy/config.rb index 9c422966cb457a..70ee60e6d9f4b1 100644 --- a/ee/app/models/security/pipeline_execution_policy/config.rb +++ b/ee/app/models/security/pipeline_execution_policy/config.rb @@ -63,8 +63,13 @@ def skip_ci_allowed?(user_id) def parse_pipeline_config_strategy(strategy_config) if strategy_config.is_a?(Hash) @config_strategy = strategy_config.fetch(:type).to_sym - @empty_pipeline_behaviour = - strategy_config.fetch(:empty_pipeline_behaviour, DEFAULT_EMPTY_PIPELINE_BEHAVIOUR).to_s + behaviour = strategy_config.fetch(:empty_pipeline_behaviour, DEFAULT_EMPTY_PIPELINE_BEHAVIOUR).to_s + + @empty_pipeline_behaviour = if EMPTY_PIPELINE_BEHAVIOUR_OPTIONS.include?(behaviour) + behaviour + else + DEFAULT_EMPTY_PIPELINE_BEHAVIOUR + end else @config_strategy = strategy_config.to_sym @empty_pipeline_behaviour = DEFAULT_EMPTY_PIPELINE_BEHAVIOUR diff --git a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb b/ee/spec/models/security/pipeline_execution_policy/config_spec.rb index ca336f40304c70..f6b9431379fe95 100644 --- a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb +++ b/ee/spec/models/security/pipeline_execution_policy/config_spec.rb @@ -88,6 +88,39 @@ expect(empty_pipeline_behaviour_value).to eq('never_inject') end end + + context 'with invalid empty_pipeline_behaviour value' do + let(:policy) do + build(:pipeline_execution_policy, + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'invalid_value' }) + end + + it 'defaults to always_inject' do + expect(empty_pipeline_behaviour_value).to eq('always_inject') + end + end + + context 'with empty string empty_pipeline_behaviour value' do + let(:policy) do + build(:pipeline_execution_policy, + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: '' }) + end + + it 'defaults to always_inject' do + expect(empty_pipeline_behaviour_value).to eq('always_inject') + end + end + + context 'with numeric empty_pipeline_behaviour value' do + let(:policy) do + build(:pipeline_execution_policy, + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 123 }) + end + + it 'defaults to always_inject after converting to string' do + expect(empty_pipeline_behaviour_value).to eq('always_inject') + end + end end describe '#suffix' do -- GitLab From 8864af92d8dae896ed416c10162786b31fc227c8 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Fri, 5 Dec 2025 17:14:15 -0500 Subject: [PATCH 04/24] Add integration test --- .../pipeline_execution_policy_spec.rb | 192 ++++++++++++++++++ 1 file changed, 192 insertions(+) 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 e9b2acd8c130c4..c177281007cdce 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 @@ -1479,6 +1479,198 @@ end end + describe 'empty_pipeline_behaviour configuration' do + before do + stub_feature_flags(pipeline_execution_policy_empty_pipeline_behaviour: true) + namespace_configuration.update!(experiments: { empty_pipeline_behaviour_option: { enabled: true } }) + project_configuration.update!(experiments: { empty_pipeline_behaviour_option: { enabled: true } }) + end + + def policy_include(file) + { + include: [{ + project: compliance_project.full_path, + file: file, + ref: compliance_project.default_branch_or_main + }] + } + end + + let(:namespace_policy) do + build(:pipeline_execution_policy, behaviour_trait, content: policy_include(namespace_policy_file)) + end + + let(:project_policy) do + build(:pipeline_execution_policy, behaviour_trait, content: policy_include(project_policy_file)) + end + + let(:no_ci_config) { nil } + + let(:ci_yml) do + <<~YAML + build: + stage: build + script: + - echo 'build' + YAML + end + + let(:ci_yml_workflow_rules) do + <<~YAML + workflow: + rules: + - if: '$CI_COMMIT_BRANCH == "non-existent-branch"' + build: + stage: build + script: + - echo 'build' + YAML + end + + let(:ci_yml_job_rules) do + <<~YAML + build: + stage: build + rules: + - if: '$CI_COMMIT_BRANCH == "non-existent-branch"' + script: + - echo 'build' + YAML + end + + where(:project_ci_yaml, :behaviour, :expected_outcome, :expected_builds, :expected_failure_reason, + :expected_config_source) do + [ + # No CI config + [ref(:no_ci_config), :empty_pipeline_always_inject, :success, 2, nil, 'pipeline_execution_policy_forced'], + [ref(:no_ci_config), :empty_pipeline_inject_if_no_config, :success, 2, nil, 'pipeline_execution_policy_forced'], + [ref(:no_ci_config), :empty_pipeline_never_inject, :error, 0, :filtered_by_rules, nil], + # Has CI config + [ref(:ci_yml), :empty_pipeline_always_inject, :success, 3, nil, 'repository_source'], + [ref(:ci_yml), :empty_pipeline_inject_if_no_config, :success, 3, nil, 'repository_source'], + [ref(:ci_yml), :empty_pipeline_never_inject, :success, 3, nil, 'repository_source'], + # Workflow rules filter out pipeline + [ref(:ci_yml_workflow_rules), :empty_pipeline_always_inject, :success, 2, nil, 'repository_source'], + [ref(:ci_yml_workflow_rules), :empty_pipeline_inject_if_no_config, :error, 0, :filtered_by_workflow_rules, nil], + [ref(:ci_yml_workflow_rules), :empty_pipeline_never_inject, :error, 0, :filtered_by_workflow_rules, nil], + # Job rules filter out all jobs + [ref(:ci_yml_job_rules), :empty_pipeline_always_inject, :success, 2, nil, 'repository_source'], + [ref(:ci_yml_job_rules), :empty_pipeline_inject_if_no_config, :error, 0, :filtered_by_rules, nil], + [ref(:ci_yml_job_rules), :empty_pipeline_never_inject, :error, 0, :filtered_by_rules, nil] + ] + end + + with_them do + let(:behaviour_trait) { behaviour } + + if params[:expected_outcome] == :success + it 'creates pipeline', :aggregate_failures do + expect { execute }.to change { Ci::Build.count }.from(0).to(expected_builds) + expect(execute).to be_success + expect(execute.payload).to be_persisted + expect(execute.payload.config_source).to eq(expected_config_source) + end + else + it 'does not create pipeline', :aggregate_failures do + expect { execute }.not_to change { Ci::Build.count } + expect(execute).to be_error + expect(execute.payload).not_to be_persisted + expect(execute.payload.failure_reason).to eq(expected_failure_reason.to_s) + end + end + end + + context 'with mixed behaviours across policies' do + let(:project_ci_yaml) { nil } + + let(:namespace_policy) do + build(:pipeline_execution_policy, :empty_pipeline_always_inject, + content: policy_include(namespace_policy_file)) + end + + let(:project_policy) do + build(:pipeline_execution_policy, :empty_pipeline_never_inject, + content: policy_include(project_policy_file)) + end + + it 'uses the most restrictive behaviour (always_inject) and creates pipeline', :aggregate_failures do + expect { execute }.to change { Ci::Build.count }.from(0).to(2) + + expect(execute).to be_success + expect(execute.payload).to be_persisted + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(pipeline_execution_policy_empty_pipeline_behaviour: false) + end + + let(:project_ci_yaml) { nil } + + let(:namespace_policy) do + build(:pipeline_execution_policy, :empty_pipeline_never_inject, + content: { include: [{ + project: compliance_project.full_path, + file: namespace_policy_file, + ref: compliance_project.default_branch_or_main + }] }) + end + + let(:project_policy) do + build(:pipeline_execution_policy, :empty_pipeline_never_inject, + content: { include: [{ + project: compliance_project.full_path, + file: project_policy_file, + ref: compliance_project.default_branch_or_main + }] }) + end + + it 'ignores empty_pipeline_behaviour and uses default always_inject behaviour', :aggregate_failures do + expect { execute }.to change { Ci::Build.count }.from(0).to(2) + + expect(execute).to be_success + expect(execute.payload).to be_persisted + expect(execute.payload.config_source).to eq('pipeline_execution_policy_forced') + end + end + + context 'when experiment is disabled' do + before do + namespace_configuration.update!(experiments: {}) + project_configuration.update!(experiments: {}) + end + + let(:project_ci_yaml) { nil } + + let(:namespace_policy) do + build(:pipeline_execution_policy, :empty_pipeline_never_inject, + content: { include: [{ + project: compliance_project.full_path, + file: namespace_policy_file, + ref: compliance_project.default_branch_or_main + }] }) + end + + let(:project_policy) do + build(:pipeline_execution_policy, :empty_pipeline_never_inject, + content: { include: [{ + project: compliance_project.full_path, + file: project_policy_file, + ref: compliance_project.default_branch_or_main + }] }) + end + + it 'ignores empty_pipeline_behaviour and uses default always_inject behaviour', :aggregate_failures do + expect { execute }.to change { Ci::Build.count }.from(0).to(2) + + expect(execute).to be_success + expect(execute.payload).to be_persisted + expect(execute.payload.config_source).to eq('pipeline_execution_policy_forced') + end + end + end + private def get_job_variable(job, key) -- GitLab From 00100f145c1f083ed313e2136eb20f12a6aec9e0 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Sat, 6 Dec 2025 00:02:25 -0500 Subject: [PATCH 05/24] Disable RSpec/MultipleMemoizedHelpers for test --- .../create_pipeline_service/pipeline_execution_policy_spec.rb | 2 ++ 1 file changed, 2 insertions(+) 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 c177281007cdce..89f4731acd0c5d 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 @@ -1479,6 +1479,7 @@ end end + # rubocop:disable RSpec/MultipleMemoizedHelpers -- table-driven tests require multiple helpers describe 'empty_pipeline_behaviour configuration' do before do stub_feature_flags(pipeline_execution_policy_empty_pipeline_behaviour: true) @@ -1670,6 +1671,7 @@ def policy_include(file) end end end + # rubocop:enable RSpec/MultipleMemoizedHelpers private -- GitLab From 4da778ce96386b4ad496347dbff39028ddf48862 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Sat, 6 Dec 2025 00:04:24 -0500 Subject: [PATCH 06/24] Switch to fast_spec_helper --- .../json_schemas/pipeline_execution_policy_content_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb b/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb index ada2588e0d04bb..05e7f21dd18b57 100644 --- a/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb +++ b/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'spec_helper' +require 'fast_spec_helper' RSpec.describe 'pipeline_execution_policy_content.json', feature_category: :security_policy_management do let(:schema_path) do -- GitLab From 19a0c4b330500eb83c541131dd454f3936b00a69 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Sat, 6 Dec 2025 01:26:25 -0500 Subject: [PATCH 07/24] Add rollout issue URL to FF --- .../pipeline_execution_policy_empty_pipeline_behaviour.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behaviour.yml b/config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behaviour.yml index 18d26cb4210dcd..ba3da9b55ee2cc 100644 --- a/config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behaviour.yml +++ b/config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behaviour.yml @@ -3,7 +3,7 @@ name: pipeline_execution_policy_empty_pipeline_behaviour description: Enable configurable empty_pipeline_behaviour option for Pipeline Execution Policies feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/work_items/582196 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/214258 -rollout_issue_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/work_items/583127 milestone: "18.7" group: group::security policies type: gitlab_com_derisk -- GitLab From ecfa8be1761d930118206fad7a200f985de1fe4b Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Tue, 9 Dec 2025 16:40:31 -0500 Subject: [PATCH 08/24] Undo apply_policies_spec.rb --- .../pipeline_execution_policies/apply_policies_spec.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/ee/spec/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies_spec.rb b/ee/spec/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies_spec.rb index fc17304405563e..265fd73fad2c9e 100644 --- a/ee/spec/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies_spec.rb +++ b/ee/spec/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies_spec.rb @@ -4,7 +4,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::PipelineExecutionPolicies::ApplyPolicies, feature_category: :security_policy_management do include Ci::PipelineExecutionPolicyHelpers - include Ci::PipelineMessageHelpers let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, group: group) } @@ -493,16 +492,10 @@ def run_previous_chain(pipeline, command) Gitlab::Ci::Pipeline::Chain::EvaluateWorkflowRules.new(pipeline, command), Gitlab::Ci::Pipeline::Chain::Seed.new(pipeline, command), Gitlab::Ci::Pipeline::Chain::Populate.new(pipeline, command) - ].each do |step| - step.perform! - break if step.break? - end + ].map(&:perform!) end def perform_chain(pipeline, command) - # If the chain already broke in a previous step, ApplyPolicies should not run - raise "Chain already broken before ApplyPolicies step" if pipeline.errors.any? - described_class.new(pipeline, command).perform! end end -- GitLab From 7bba7425ff55025e1009d950e3fe867f25969955 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Tue, 9 Dec 2025 17:01:18 -0500 Subject: [PATCH 09/24] Remove redundant stubs --- .../pipeline_execution_policies/pipeline_context_spec.rb | 4 ---- .../create_pipeline_service/pipeline_execution_policy_spec.rb | 1 - 2 files changed, 5 deletions(-) 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 0474f23b71bda4..338413a07b2e2b 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 @@ -692,10 +692,6 @@ end context 'when feature flag is enabled' do - before do - stub_feature_flags(pipeline_execution_policy_empty_pipeline_behaviour: true) - end - context 'when empty_pipeline_behaviour is "always_inject"' do let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_always_inject) } 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 89f4731acd0c5d..742368167eb254 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 @@ -1482,7 +1482,6 @@ # rubocop:disable RSpec/MultipleMemoizedHelpers -- table-driven tests require multiple helpers describe 'empty_pipeline_behaviour configuration' do before do - stub_feature_flags(pipeline_execution_policy_empty_pipeline_behaviour: true) namespace_configuration.update!(experiments: { empty_pipeline_behaviour_option: { enabled: true } }) project_configuration.update!(experiments: { empty_pipeline_behaviour_option: { enabled: true } }) end -- GitLab From 1caa46082cbf3379bae8f205bb85913d7afc9a96 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Tue, 9 Dec 2025 17:02:05 -0500 Subject: [PATCH 10/24] Change inject -> apply; remove redundant validation and comment --- .../pipeline_execution_policy/config.rb | 14 +++++--------- .../pipeline_context.rb | 18 +++++++++++------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/ee/app/models/security/pipeline_execution_policy/config.rb b/ee/app/models/security/pipeline_execution_policy/config.rb index 70ee60e6d9f4b1..d177a831ef97e7 100644 --- a/ee/app/models/security/pipeline_execution_policy/config.rb +++ b/ee/app/models/security/pipeline_execution_policy/config.rb @@ -10,8 +10,8 @@ class Config DEFAULT_SUFFIX_STRATEGY = 'on_conflict' SUFFIX_STRATEGIES = { on_conflict: 'on_conflict', never: 'never' }.freeze DEFAULT_SKIP_CI_STRATEGY = { allowed: false }.freeze - DEFAULT_EMPTY_PIPELINE_BEHAVIOUR = 'always_inject' - EMPTY_PIPELINE_BEHAVIOUR_OPTIONS = %w[always_inject inject_if_no_config never_inject].freeze + DEFAULT_EMPTY_PIPELINE_BEHAVIOUR = 'always_apply' + EMPTY_PIPELINE_BEHAVIOR_OPTIONS = %w[always_apply apply_if_no_config never_apply].freeze POLICY_JOB_SUFFIX = ':policy' attr_reader :content, :config_strategy, :suffix_strategy, :policy_project_id, :policy_index, :name, @@ -63,13 +63,9 @@ def skip_ci_allowed?(user_id) def parse_pipeline_config_strategy(strategy_config) if strategy_config.is_a?(Hash) @config_strategy = strategy_config.fetch(:type).to_sym - behaviour = strategy_config.fetch(:empty_pipeline_behaviour, DEFAULT_EMPTY_PIPELINE_BEHAVIOUR).to_s - - @empty_pipeline_behaviour = if EMPTY_PIPELINE_BEHAVIOUR_OPTIONS.include?(behaviour) - behaviour - else - DEFAULT_EMPTY_PIPELINE_BEHAVIOUR - end + @empty_pipeline_behaviour = strategy_config.fetch( + :empty_pipeline_behaviour, DEFAULT_EMPTY_PIPELINE_BEHAVIOUR + ).to_s else @config_strategy = strategy_config.to_sym @empty_pipeline_behaviour = DEFAULT_EMPTY_PIPELINE_BEHAVIOUR 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 43cf8e4fb21dff..cbdd4ab47a91c6 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 @@ -15,6 +15,10 @@ class PipelineContext all_pipelines: :gitlab_security_policies_pipeline_execution_policy_build_policy_pipelines }.freeze + EMPTY_PIPELINE_BEHAVIOUR_ALWAYS_APPLY = 'always_apply' + EMPTY_PIPELINE_BEHAVIOUR_APPLY_IF_NO_CONFIG = 'apply_if_no_config' + EMPTY_PIPELINE_BEHAVIOUR_NEVER_APPLY = 'never_apply' + attr_reader :policy_pipelines, :override_policy_stages, :injected_policy_stages # rubocop:disable Metrics/ParameterLists -- Explicit parameters needed to replace command object delegation @@ -85,11 +89,10 @@ def force_pipeline_creation?(pipeline) behaviour = effective_empty_pipeline_behaviour - return true if behaviour.nil? || behaviour == 'always_inject' - return false if behaviour == 'never_inject' + return true if behaviour.nil? || behaviour == EMPTY_PIPELINE_BEHAVIOUR_ALWAYS_APPLY + return false if behaviour == EMPTY_PIPELINE_BEHAVIOUR_NEVER_APPLY - # Handle 'inject_if_no_config' behavior - if behaviour == 'inject_if_no_config' + if behaviour == EMPTY_PIPELINE_BEHAVIOUR_APPLY_IF_NO_CONFIG # If the project has a CI config, we only inject if the project creates a pipeline. # This is because we don't have a good way of knowing when both the MR and branch # pipeline creation has been attempted. To ensure PEPs run in such projects, the @@ -298,13 +301,14 @@ def effective_empty_pipeline_behaviour behaviour_values.min_by { |value| empty_pipeline_behaviour_priority(value) } end + strong_memoize_attr :effective_empty_pipeline_behaviour def empty_pipeline_behaviour_priority(behaviour) # Lower number = more restrictive (most limiting to project freedom) case behaviour - when 'always_inject' then 0 - when 'inject_if_no_config' then 1 - when 'never_inject' then 2 + when EMPTY_PIPELINE_BEHAVIOUR_ALWAYS_APPLY then 0 + when EMPTY_PIPELINE_BEHAVIOUR_APPLY_IF_NO_CONFIG then 1 + when EMPTY_PIPELINE_BEHAVIOUR_NEVER_APPLY then 2 else 3 # unknown end end -- GitLab From b3be036953549613fd2a5aa42a88f31e7b3558aa Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Tue, 9 Dec 2025 22:02:42 -0500 Subject: [PATCH 11/24] Change all behaviour -> behavior --- ...cution_policy_empty_pipeline_behavior.yml} | 4 +- .../pipeline_execution_policy/config.rb | 10 ++--- .../pipeline_execution_policy_content.json | 4 +- .../security_orchestration_policy.json | 4 +- .../pipeline_context.rb | 36 ++++++++-------- .../pipeline_execution_policy_configs.rb | 6 +-- .../pipeline_execution_policy_pipeline.rb | 6 +-- ee/spec/factories/security/policies.rb | 6 +-- ...ity_orchestration_policy_configurations.rb | 4 +- .../pipeline_context_spec.rb | 12 +++--- .../pipeline_execution_policy/config_spec.rb | 42 +++++++++---------- .../pipeline_execution_policy_spec.rb | 12 +++--- .../pipeline_execution_policy_content_spec.rb | 36 ++++++++-------- 13 files changed, 91 insertions(+), 91 deletions(-) rename config/feature_flags/gitlab_com_derisk/{pipeline_execution_policy_empty_pipeline_behaviour.yml => pipeline_execution_policy_empty_pipeline_behavior.yml} (68%) diff --git a/config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behaviour.yml b/config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behavior.yml similarity index 68% rename from config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behaviour.yml rename to config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behavior.yml index ba3da9b55ee2cc..2d4c2cbb24b35b 100644 --- a/config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behaviour.yml +++ b/config/feature_flags/gitlab_com_derisk/pipeline_execution_policy_empty_pipeline_behavior.yml @@ -1,6 +1,6 @@ --- -name: pipeline_execution_policy_empty_pipeline_behaviour -description: Enable configurable empty_pipeline_behaviour option for Pipeline Execution Policies +name: pipeline_execution_policy_empty_pipeline_behavior +description: Enable configurable empty_pipeline_behavior option for Pipeline Execution Policies feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/work_items/582196 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/214258 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/work_items/583127 diff --git a/ee/app/models/security/pipeline_execution_policy/config.rb b/ee/app/models/security/pipeline_execution_policy/config.rb index d177a831ef97e7..54c6cfd93fca2f 100644 --- a/ee/app/models/security/pipeline_execution_policy/config.rb +++ b/ee/app/models/security/pipeline_execution_policy/config.rb @@ -10,12 +10,12 @@ class Config DEFAULT_SUFFIX_STRATEGY = 'on_conflict' SUFFIX_STRATEGIES = { on_conflict: 'on_conflict', never: 'never' }.freeze DEFAULT_SKIP_CI_STRATEGY = { allowed: false }.freeze - DEFAULT_EMPTY_PIPELINE_BEHAVIOUR = 'always_apply' + DEFAULT_EMPTY_PIPELINE_BEHAVIOR = 'always_apply' EMPTY_PIPELINE_BEHAVIOR_OPTIONS = %w[always_apply apply_if_no_config never_apply].freeze POLICY_JOB_SUFFIX = ':policy' attr_reader :content, :config_strategy, :suffix_strategy, :policy_project_id, :policy_index, :name, - :skip_ci_strategy, :variables_override_strategy, :policy_config, :policy_sha, :empty_pipeline_behaviour + :skip_ci_strategy, :variables_override_strategy, :policy_config, :policy_sha, :empty_pipeline_behavior delegate :experiment_enabled?, to: :policy_config @@ -63,12 +63,12 @@ def skip_ci_allowed?(user_id) def parse_pipeline_config_strategy(strategy_config) if strategy_config.is_a?(Hash) @config_strategy = strategy_config.fetch(:type).to_sym - @empty_pipeline_behaviour = strategy_config.fetch( - :empty_pipeline_behaviour, DEFAULT_EMPTY_PIPELINE_BEHAVIOUR + @empty_pipeline_behavior = strategy_config.fetch( + :empty_pipeline_behavior, DEFAULT_EMPTY_PIPELINE_BEHAVIOR ).to_s else @config_strategy = strategy_config.to_sym - @empty_pipeline_behaviour = DEFAULT_EMPTY_PIPELINE_BEHAVIOUR + @empty_pipeline_behavior = DEFAULT_EMPTY_PIPELINE_BEHAVIOR end end end diff --git a/ee/app/validators/json_schemas/pipeline_execution_policy_content.json b/ee/app/validators/json_schemas/pipeline_execution_policy_content.json index 80dbe5c5f35d07..a532cd571b456f 100644 --- a/ee/app/validators/json_schemas/pipeline_execution_policy_content.json +++ b/ee/app/validators/json_schemas/pipeline_execution_policy_content.json @@ -38,7 +38,7 @@ "additionalProperties": false }, "pipeline_config_strategy": { - "description": "Defines the method for merging the policy configuration with the project pipeline. Can be a string or an object with type and empty_pipeline_behaviour options.", + "description": "Defines the method for merging the policy configuration with the project pipeline. Can be a string or an object with type and empty_pipeline_behavior options.", "oneOf": [ { "type": "string", @@ -61,7 +61,7 @@ ], "description": "The strategy type for merging policy configuration." }, - "empty_pipeline_behaviour": { + "empty_pipeline_behavior": { "type": "string", "enum": [ "always_inject", diff --git a/ee/app/validators/json_schemas/security_orchestration_policy.json b/ee/app/validators/json_schemas/security_orchestration_policy.json index 318e125347b184..ab502694592c04 100644 --- a/ee/app/validators/json_schemas/security_orchestration_policy.json +++ b/ee/app/validators/json_schemas/security_orchestration_policy.json @@ -358,7 +358,7 @@ "$ref": "#/$defs/pipeline_execution_content" }, "pipeline_config_strategy": { - "description": "Defines the method for merging the policy configuration with the project pipeline. Can be a string or an object with type and empty_pipeline_behaviour options.", + "description": "Defines the method for merging the policy configuration with the project pipeline. Can be a string or an object with type and empty_pipeline_behavior options.", "oneOf": [ { "type": "string", @@ -381,7 +381,7 @@ ], "description": "The strategy type for merging policy configuration." }, - "empty_pipeline_behaviour": { + "empty_pipeline_behavior": { "type": "string", "enum": [ "always_inject", 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 cbdd4ab47a91c6..07d0e7502a606b 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 @@ -15,9 +15,9 @@ class PipelineContext all_pipelines: :gitlab_security_policies_pipeline_execution_policy_build_policy_pipelines }.freeze - EMPTY_PIPELINE_BEHAVIOUR_ALWAYS_APPLY = 'always_apply' - EMPTY_PIPELINE_BEHAVIOUR_APPLY_IF_NO_CONFIG = 'apply_if_no_config' - EMPTY_PIPELINE_BEHAVIOUR_NEVER_APPLY = 'never_apply' + EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY = 'always_apply' + EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG = 'apply_if_no_config' + EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY = 'never_apply' attr_reader :policy_pipelines, :override_policy_stages, :injected_policy_stages @@ -85,14 +85,14 @@ def scheduled_execution_policy_pipeline? def force_pipeline_creation?(pipeline) return false unless has_execution_policy_pipelines? - return true unless Feature.enabled?(:pipeline_execution_policy_empty_pipeline_behaviour, pipeline.project) + return true unless Feature.enabled?(:pipeline_execution_policy_empty_pipeline_behavior, pipeline.project) - behaviour = effective_empty_pipeline_behaviour + behaviour = effective_empty_pipeline_behavior - return true if behaviour.nil? || behaviour == EMPTY_PIPELINE_BEHAVIOUR_ALWAYS_APPLY - return false if behaviour == EMPTY_PIPELINE_BEHAVIOUR_NEVER_APPLY + return true if behaviour.nil? || behaviour == EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY + return false if behaviour == EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY - if behaviour == EMPTY_PIPELINE_BEHAVIOUR_APPLY_IF_NO_CONFIG + if behaviour == EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG # If the project has a CI config, we only inject if the project creates a pipeline. # This is because we don't have a good way of knowing when both the MR and branch # pipeline creation has been attempted. To ensure PEPs run in such projects, the @@ -288,27 +288,27 @@ def stages_compatible?(stages, target_stages) stages == target_stages & stages end - def effective_empty_pipeline_behaviour - # Determine the effective empty_pipeline_behaviour setting + def effective_empty_pipeline_behavior + # Determine the effective empty_pipeline_behavior setting # If policies have different settings, use the most restrictive # (always_inject > inject_if_no_config > never_inject) # "Restrictive" means most limiting to project freedom behaviour_values = policy_pipelines.filter_map do |policy_pipeline| - if policy_pipeline.policy_config.experiment_enabled?(:empty_pipeline_behaviour_option) - policy_pipeline.policy_config.empty_pipeline_behaviour + if policy_pipeline.policy_config.experiment_enabled?(:empty_pipeline_behavior_option) + policy_pipeline.policy_config.empty_pipeline_behavior end end - behaviour_values.min_by { |value| empty_pipeline_behaviour_priority(value) } + behaviour_values.min_by { |value| empty_pipeline_behavior_priority(value) } end - strong_memoize_attr :effective_empty_pipeline_behaviour + strong_memoize_attr :effective_empty_pipeline_behavior - def empty_pipeline_behaviour_priority(behaviour) + def empty_pipeline_behavior_priority(behaviour) # Lower number = more restrictive (most limiting to project freedom) case behaviour - when EMPTY_PIPELINE_BEHAVIOUR_ALWAYS_APPLY then 0 - when EMPTY_PIPELINE_BEHAVIOUR_APPLY_IF_NO_CONFIG then 1 - when EMPTY_PIPELINE_BEHAVIOUR_NEVER_APPLY then 2 + when EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY then 0 + when EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG then 1 + when EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY then 2 else 3 # unknown end end diff --git a/ee/spec/factories/security/pipeline_execution_policy_configs.rb b/ee/spec/factories/security/pipeline_execution_policy_configs.rb index dc2c3808ecffe3..4e4ff0e0ad82e9 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_configs.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_configs.rb @@ -48,17 +48,17 @@ trait :empty_pipeline_always_inject do policy factory: [:pipeline_execution_policy, :empty_pipeline_always_inject] - policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment] + policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] end trait :empty_pipeline_inject_if_no_config do policy factory: [:pipeline_execution_policy, :empty_pipeline_inject_if_no_config] - policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment] + policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] end trait :empty_pipeline_never_inject do policy factory: [:pipeline_execution_policy, :empty_pipeline_never_inject] - policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment] + policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] end end end diff --git a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb index 23e024bcb1b94c..41a4a4d699e34b 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb @@ -36,7 +36,7 @@ association( :pipeline_execution_policy_config, :empty_pipeline_always_inject, policy_config: association( - :security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment + :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment ) ) end @@ -47,7 +47,7 @@ association( :pipeline_execution_policy_config, :empty_pipeline_inject_if_no_config, policy_config: association( - :security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment + :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment ) ) end @@ -58,7 +58,7 @@ association( :pipeline_execution_policy_config, :empty_pipeline_never_inject, policy_config: association( - :security_orchestration_policy_configuration, :with_empty_pipeline_behaviour_experiment + :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment ) ) end diff --git a/ee/spec/factories/security/policies.rb b/ee/spec/factories/security/policies.rb index 17289b86b70803..5002b94928f019 100644 --- a/ee/spec/factories/security/policies.rb +++ b/ee/spec/factories/security/policies.rb @@ -373,15 +373,15 @@ end trait :empty_pipeline_always_inject do - pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behaviour: 'always_inject' } } + pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'always_inject' } } end trait :empty_pipeline_inject_if_no_config do - pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behaviour: 'inject_if_no_config' } } + pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'inject_if_no_config' } } end trait :empty_pipeline_never_inject do - pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behaviour: 'never_inject' } } + pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'never_inject' } } end trait :suffix_on_conflict do diff --git a/ee/spec/factories/security_orchestration_policy_configurations.rb b/ee/spec/factories/security_orchestration_policy_configurations.rb index 19108d8c1dfa3d..8f72de567b9a90 100644 --- a/ee/spec/factories/security_orchestration_policy_configurations.rb +++ b/ee/spec/factories/security_orchestration_policy_configurations.rb @@ -11,8 +11,8 @@ namespace end - trait :with_empty_pipeline_behaviour_experiment do - experiments { { empty_pipeline_behaviour_option: { enabled: true } } } + trait :with_empty_pipeline_behavior_experiment do + experiments { { empty_pipeline_behavior_option: { enabled: true } } } end end 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 338413a07b2e2b..cb669a9b19a0a7 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 @@ -683,7 +683,7 @@ let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_never_inject) } before do - stub_feature_flags(pipeline_execution_policy_empty_pipeline_behaviour: false) + stub_feature_flags(pipeline_execution_policy_empty_pipeline_behavior: false) end it 'always forces pipeline creation (default behavior)' do @@ -692,19 +692,19 @@ end context 'when feature flag is enabled' do - context 'when empty_pipeline_behaviour is "always_inject"' do + context 'when empty_pipeline_behavior is "always_inject"' do let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_always_inject) } it { is_expected.to eq(true) } end - context 'when empty_pipeline_behaviour is "never_inject"' do + context 'when empty_pipeline_behavior is "never_inject"' do let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_never_inject) } it { is_expected.to eq(false) } end - context 'when empty_pipeline_behaviour is "inject_if_no_config"' do + context 'when empty_pipeline_behavior is "inject_if_no_config"' do let(:policy_pipelines) do build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_inject_if_no_config) end @@ -730,7 +730,7 @@ end end - context 'when multiple policies have different empty_pipeline_behaviour settings' do + context 'when multiple policies have different empty_pipeline_behavior settings' do let(:policy_pipelines) do [ build(:pipeline_execution_policy_pipeline, :empty_pipeline_always_inject), @@ -769,7 +769,7 @@ ] end - it 'ignores empty_pipeline_behaviour and forces creation (default behavior)' do + it 'ignores empty_pipeline_behavior and forces creation (default behavior)' do expect(force_creation).to eq(true) end end diff --git a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb b/ee/spec/models/security/pipeline_execution_policy/config_spec.rb index f6b9431379fe95..4b607012ba394e 100644 --- a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb +++ b/ee/spec/models/security/pipeline_execution_policy/config_spec.rb @@ -46,79 +46,79 @@ end end - describe '#empty_pipeline_behaviour' do - subject(:empty_pipeline_behaviour_value) { config.empty_pipeline_behaviour } + describe '#empty_pipeline_behavior' do + subject(:empty_pipeline_behavior_value) { config.empty_pipeline_behavior } context 'with string strategy' do let(:policy) { build(:pipeline_execution_policy, pipeline_config_strategy: 'inject_ci') } it 'defaults to always_inject' do - expect(empty_pipeline_behaviour_value).to eq('always_inject') + expect(empty_pipeline_behavior_value).to eq('always_inject') end end - context 'with object strategy and empty_pipeline_behaviour specified' do + context 'with object strategy and empty_pipeline_behavior specified' do let(:policy) do build(:pipeline_execution_policy, - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'inject_if_no_config' }) + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'inject_if_no_config' }) end - it 'returns the specified empty_pipeline_behaviour value' do - expect(empty_pipeline_behaviour_value).to eq('inject_if_no_config') + it 'returns the specified empty_pipeline_behavior value' do + expect(empty_pipeline_behavior_value).to eq('inject_if_no_config') end end - context 'with object strategy and no empty_pipeline_behaviour specified' do + context 'with object strategy and no empty_pipeline_behavior specified' do let(:policy) do build(:pipeline_execution_policy, pipeline_config_strategy: { type: 'inject_policy' }) end it 'defaults to always_inject' do - expect(empty_pipeline_behaviour_value).to eq('always_inject') + expect(empty_pipeline_behavior_value).to eq('always_inject') end end - context 'with empty_pipeline_behaviour set to never_inject' do + context 'with empty_pipeline_behavior set to never_inject' do let(:policy) do build(:pipeline_execution_policy, - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'never_inject' }) + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'never_inject' }) end it 'returns never_inject' do - expect(empty_pipeline_behaviour_value).to eq('never_inject') + expect(empty_pipeline_behavior_value).to eq('never_inject') end end - context 'with invalid empty_pipeline_behaviour value' do + context 'with invalid empty_pipeline_behavior value' do let(:policy) do build(:pipeline_execution_policy, - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'invalid_value' }) + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'invalid_value' }) end it 'defaults to always_inject' do - expect(empty_pipeline_behaviour_value).to eq('always_inject') + expect(empty_pipeline_behavior_value).to eq('always_inject') end end - context 'with empty string empty_pipeline_behaviour value' do + context 'with empty string empty_pipeline_behavior value' do let(:policy) do build(:pipeline_execution_policy, - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: '' }) + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: '' }) end it 'defaults to always_inject' do - expect(empty_pipeline_behaviour_value).to eq('always_inject') + expect(empty_pipeline_behavior_value).to eq('always_inject') end end - context 'with numeric empty_pipeline_behaviour value' do + context 'with numeric empty_pipeline_behavior value' do let(:policy) do build(:pipeline_execution_policy, - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 123 }) + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 123 }) end it 'defaults to always_inject after converting to string' do - expect(empty_pipeline_behaviour_value).to eq('always_inject') + expect(empty_pipeline_behavior_value).to eq('always_inject') end end 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 742368167eb254..23ac0023ebcdbe 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 @@ -1480,10 +1480,10 @@ end # rubocop:disable RSpec/MultipleMemoizedHelpers -- table-driven tests require multiple helpers - describe 'empty_pipeline_behaviour configuration' do + describe 'empty_pipeline_behavior configuration' do before do - namespace_configuration.update!(experiments: { empty_pipeline_behaviour_option: { enabled: true } }) - project_configuration.update!(experiments: { empty_pipeline_behaviour_option: { enabled: true } }) + namespace_configuration.update!(experiments: { empty_pipeline_behavior_option: { enabled: true } }) + project_configuration.update!(experiments: { empty_pipeline_behavior_option: { enabled: true } }) end def policy_include(file) @@ -1603,7 +1603,7 @@ def policy_include(file) context 'when feature flag is disabled' do before do - stub_feature_flags(pipeline_execution_policy_empty_pipeline_behaviour: false) + stub_feature_flags(pipeline_execution_policy_empty_pipeline_behavior: false) end let(:project_ci_yaml) { nil } @@ -1626,7 +1626,7 @@ def policy_include(file) }] }) end - it 'ignores empty_pipeline_behaviour and uses default always_inject behaviour', :aggregate_failures do + it 'ignores empty_pipeline_behavior and uses default always_inject behaviour', :aggregate_failures do expect { execute }.to change { Ci::Build.count }.from(0).to(2) expect(execute).to be_success @@ -1661,7 +1661,7 @@ def policy_include(file) }] }) end - it 'ignores empty_pipeline_behaviour and uses default always_inject behaviour', :aggregate_failures do + it 'ignores empty_pipeline_behavior and uses default always_inject behaviour', :aggregate_failures do expect { execute }.to change { Ci::Build.count }.from(0).to(2) expect(execute).to be_success diff --git a/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb b/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb index 05e7f21dd18b57..34b72e5acf3cad 100644 --- a/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb +++ b/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb @@ -34,45 +34,45 @@ end context 'with object format' do - context 'with valid type and empty_pipeline_behaviour' do - it 'accepts inject_policy with empty_pipeline_behaviour always_inject' do + context 'with valid type and empty_pipeline_behavior' do + it 'accepts inject_policy with empty_pipeline_behavior always_inject' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'always_inject' } + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'always_inject' } ) expect(schema.valid?(policy)).to be true end - it 'accepts inject_policy with empty_pipeline_behaviour inject_if_no_config' do + it 'accepts inject_policy with empty_pipeline_behavior inject_if_no_config' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'inject_if_no_config' } + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'inject_if_no_config' } ) expect(schema.valid?(policy)).to be true end - it 'accepts inject_policy with empty_pipeline_behaviour never_inject' do + it 'accepts inject_policy with empty_pipeline_behavior never_inject' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'never_inject' } + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'never_inject' } ) expect(schema.valid?(policy)).to be true end - it 'accepts inject_ci with empty_pipeline_behaviour' do + it 'accepts inject_ci with empty_pipeline_behavior' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'inject_ci', empty_pipeline_behaviour: 'inject_if_no_config' } + pipeline_config_strategy: { type: 'inject_ci', empty_pipeline_behavior: 'inject_if_no_config' } ) expect(schema.valid?(policy)).to be true end - it 'accepts override_project_ci with empty_pipeline_behaviour' do + it 'accepts override_project_ci with empty_pipeline_behavior' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'override_project_ci', empty_pipeline_behaviour: 'never_inject' } + pipeline_config_strategy: { type: 'override_project_ci', empty_pipeline_behavior: 'never_inject' } ) expect(schema.valid?(policy)).to be true end end - context 'with type only (no empty_pipeline_behaviour)' do - it 'accepts type without empty_pipeline_behaviour' do + context 'with type only (no empty_pipeline_behavior)' do + it 'accepts type without empty_pipeline_behavior' do policy = base_policy.merge( pipeline_config_strategy: { type: 'inject_policy' } ) @@ -83,21 +83,21 @@ context 'with invalid values' do it 'rejects invalid type' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'invalid_type', empty_pipeline_behaviour: 'always_inject' } + pipeline_config_strategy: { type: 'invalid_type', empty_pipeline_behavior: 'always_inject' } ) expect(schema.valid?(policy)).to be false end - it 'rejects invalid empty_pipeline_behaviour' do + it 'rejects invalid empty_pipeline_behavior' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behaviour: 'invalid_behaviour' } + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'invalid_behaviour' } ) expect(schema.valid?(policy)).to be false end it 'rejects object without type' do policy = base_policy.merge( - pipeline_config_strategy: { empty_pipeline_behaviour: 'always_inject' } + pipeline_config_strategy: { empty_pipeline_behavior: 'always_inject' } ) expect(schema.valid?(policy)).to be false end @@ -105,7 +105,7 @@ it 'rejects object with additional properties' do policy = base_policy.merge( pipeline_config_strategy: { - type: 'inject_policy', empty_pipeline_behaviour: 'always_inject', extra: 'value' + type: 'inject_policy', empty_pipeline_behavior: 'always_inject', extra: 'value' } ) expect(schema.valid?(policy)).to be false -- GitLab From f5be9bbdb1dd80be341f5602782cb7e939d44e87 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Tue, 9 Dec 2025 23:53:20 -0500 Subject: [PATCH 12/24] Fix missing inject -> apply changes; add override strategy tests --- .../pipeline_execution_policy_content.json | 8 +- .../security_orchestration_policy.json | 8 +- .../pipeline_context.rb | 8 +- .../pipeline_execution_policy_configs.rb | 17 +++-- .../pipeline_execution_policy_pipeline.rb | 23 ++++-- ee/spec/factories/security/policies.rb | 22 ++++-- .../pipeline_context_spec.rb | 30 ++++---- .../pipeline_execution_policy/config_spec.rb | 32 ++++---- .../pipeline_execution_policy_spec.rb | 74 ++++++++++--------- .../pipeline_execution_policy_content_spec.rb | 22 +++--- 10 files changed, 139 insertions(+), 105 deletions(-) diff --git a/ee/app/validators/json_schemas/pipeline_execution_policy_content.json b/ee/app/validators/json_schemas/pipeline_execution_policy_content.json index a532cd571b456f..2c1d69b529b47f 100644 --- a/ee/app/validators/json_schemas/pipeline_execution_policy_content.json +++ b/ee/app/validators/json_schemas/pipeline_execution_policy_content.json @@ -64,11 +64,11 @@ "empty_pipeline_behavior": { "type": "string", "enum": [ - "always_inject", - "inject_if_no_config", - "never_inject" + "always_apply", + "apply_if_no_config", + "never_apply" ], - "description": "Controls when policies inject into empty pipelines. `always_inject`: Always injects (current default). `inject_if_no_config`: Only injects when project has no CI config. `never_inject`: Never injects a fallback pipeline." + "description": "Controls when policies apply to empty pipelines. `always_apply`: Always applies (current default). `apply_if_no_config`: Only applies when project has no CI config. `never_apply`: Never applies a fallback pipeline." } }, "required": [ diff --git a/ee/app/validators/json_schemas/security_orchestration_policy.json b/ee/app/validators/json_schemas/security_orchestration_policy.json index ab502694592c04..5235fa4cc03ccc 100644 --- a/ee/app/validators/json_schemas/security_orchestration_policy.json +++ b/ee/app/validators/json_schemas/security_orchestration_policy.json @@ -384,11 +384,11 @@ "empty_pipeline_behavior": { "type": "string", "enum": [ - "always_inject", - "inject_if_no_config", - "never_inject" + "always_apply", + "apply_if_no_config", + "never_apply" ], - "description": "Controls when policies inject into empty pipelines. `always_inject`: Always injects (current default). `inject_if_no_config`: Only injects when project has no CI config. `never_inject`: Never injects a fallback pipeline." + "description": "Controls when policies apply to empty pipelines. `always_apply`: Always applies (current default). `apply_if_no_config`: Only applies when project has no CI config. `never_apply`: Never applies a fallback pipeline." } }, "required": [ 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 07d0e7502a606b..7488e413006f74 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 @@ -93,7 +93,7 @@ def force_pipeline_creation?(pipeline) return false if behaviour == EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY if behaviour == EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG - # If the project has a CI config, we only inject if the project creates a pipeline. + # If the project has a CI config, we only apply if the project creates a pipeline. # This is because we don't have a good way of knowing when both the MR and branch # pipeline creation has been attempted. To ensure PEPs run in such projects, the # "Pipelines must succeed" setting is needed as well. @@ -102,11 +102,11 @@ def force_pipeline_creation?(pipeline) # Only continue if we're using the fallback source return false unless pipeline.pipeline_execution_policy_forced? - # For projects with no CI config, always inject + # For projects with no CI config, always apply return true end - # Unknown behaviour defaults to always_inject + # Unknown behaviour defaults to always apply true end @@ -291,7 +291,7 @@ def stages_compatible?(stages, target_stages) def effective_empty_pipeline_behavior # Determine the effective empty_pipeline_behavior setting # If policies have different settings, use the most restrictive - # (always_inject > inject_if_no_config > never_inject) + # (always_apply > apply_if_no_config > never_apply) # "Restrictive" means most limiting to project freedom behaviour_values = policy_pipelines.filter_map do |policy_pipeline| if policy_pipeline.policy_config.experiment_enabled?(:empty_pipeline_behavior_option) diff --git a/ee/spec/factories/security/pipeline_execution_policy_configs.rb b/ee/spec/factories/security/pipeline_execution_policy_configs.rb index 4e4ff0e0ad82e9..b233810dc2c7a1 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_configs.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_configs.rb @@ -46,18 +46,23 @@ policy factory: [:pipeline_execution_policy, :variables_override_disallowed] end - trait :empty_pipeline_always_inject do - policy factory: [:pipeline_execution_policy, :empty_pipeline_always_inject] + trait :empty_pipeline_always_apply do + policy factory: [:pipeline_execution_policy, :empty_pipeline_always_apply] policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] end - trait :empty_pipeline_inject_if_no_config do - policy factory: [:pipeline_execution_policy, :empty_pipeline_inject_if_no_config] + trait :empty_pipeline_apply_if_no_config do + policy factory: [:pipeline_execution_policy, :empty_pipeline_apply_if_no_config] policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] end - trait :empty_pipeline_never_inject do - policy factory: [:pipeline_execution_policy, :empty_pipeline_never_inject] + trait :empty_pipeline_never_apply do + policy factory: [:pipeline_execution_policy, :empty_pipeline_never_apply] + policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] + end + + trait :override_project_ci_strategy do + policy factory: [:pipeline_execution_policy, :override_project_ci_strategy] policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] end end diff --git a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb index 41a4a4d699e34b..1d3d5bbf738a59 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb @@ -31,10 +31,10 @@ policy_config factory: [:pipeline_execution_policy_config, :skip_ci_disallowed] end - trait :empty_pipeline_always_inject do + trait :empty_pipeline_always_apply do policy_config do association( - :pipeline_execution_policy_config, :empty_pipeline_always_inject, + :pipeline_execution_policy_config, :empty_pipeline_always_apply, policy_config: association( :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment ) @@ -42,10 +42,10 @@ end end - trait :empty_pipeline_inject_if_no_config do + trait :empty_pipeline_apply_if_no_config do policy_config do association( - :pipeline_execution_policy_config, :empty_pipeline_inject_if_no_config, + :pipeline_execution_policy_config, :empty_pipeline_apply_if_no_config, policy_config: association( :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment ) @@ -53,10 +53,21 @@ end end - trait :empty_pipeline_never_inject do + trait :empty_pipeline_never_apply do policy_config do association( - :pipeline_execution_policy_config, :empty_pipeline_never_inject, + :pipeline_execution_policy_config, :empty_pipeline_never_apply, + policy_config: association( + :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment + ) + ) + end + end + + trait :override_project_ci_strategy do + policy_config do + association( + :pipeline_execution_policy_config, :override_project_ci_strategy, policy_config: association( :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment ) diff --git a/ee/spec/factories/security/policies.rb b/ee/spec/factories/security/policies.rb index 5002b94928f019..db80358b517e59 100644 --- a/ee/spec/factories/security/policies.rb +++ b/ee/spec/factories/security/policies.rb @@ -372,16 +372,26 @@ pipeline_config_strategy { 'inject_policy' } end - trait :empty_pipeline_always_inject do - pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'always_inject' } } + trait :empty_pipeline_always_apply do + pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'always_apply' } } end - trait :empty_pipeline_inject_if_no_config do - pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'inject_if_no_config' } } + trait :empty_pipeline_apply_if_no_config do + pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'apply_if_no_config' } } end - trait :empty_pipeline_never_inject do - pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'never_inject' } } + trait :empty_pipeline_never_apply do + pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'never_apply' } } + end + + trait :override_project_ci_strategy do + # This trait modifies the pipeline_config_strategy to use override_project_ci + # It should be applied after a behavior trait + after(:build) do |policy_hash| + if policy_hash.is_a?(Hash) && policy_hash[:pipeline_config_strategy].is_a?(Hash) + policy_hash[:pipeline_config_strategy] = policy_hash[:pipeline_config_strategy].merge(type: 'override_project_ci') + end + end end trait :suffix_on_conflict do 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 cb669a9b19a0a7..19d90d60274c24 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 @@ -680,7 +680,7 @@ context 'with policy_pipelines' do context 'when feature flag is disabled' do - let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_never_inject) } + let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_never_apply) } before do stub_feature_flags(pipeline_execution_policy_empty_pipeline_behavior: false) @@ -692,21 +692,21 @@ end context 'when feature flag is enabled' do - context 'when empty_pipeline_behavior is "always_inject"' do - let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_always_inject) } + context 'when empty_pipeline_behavior is "always_apply"' do + let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_always_apply) } it { is_expected.to eq(true) } end - context 'when empty_pipeline_behavior is "never_inject"' do - let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_never_inject) } + context 'when empty_pipeline_behavior is "never_apply"' do + let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_never_apply) } it { is_expected.to eq(false) } end - context 'when empty_pipeline_behavior is "inject_if_no_config"' do + context 'when empty_pipeline_behavior is "apply_if_no_config"' do let(:policy_pipelines) do - build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_inject_if_no_config) + build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_apply_if_no_config) end context 'when project has CI config (pipeline has stages)' do @@ -733,25 +733,25 @@ context 'when multiple policies have different empty_pipeline_behavior settings' do let(:policy_pipelines) do [ - build(:pipeline_execution_policy_pipeline, :empty_pipeline_always_inject), - build(:pipeline_execution_policy_pipeline, :empty_pipeline_never_inject) + build(:pipeline_execution_policy_pipeline, :empty_pipeline_always_apply), + build(:pipeline_execution_policy_pipeline, :empty_pipeline_never_apply) ] end - it 'uses the most restrictive setting (always_inject) and forces creation' do + it 'uses the most restrictive setting (always_apply) and forces creation' do expect(force_creation).to eq(true) end end - context 'when policies have inject_if_no_config and always_inject' do + context 'when policies have apply_if_no_config and always_apply' do let(:policy_pipelines) do [ - build(:pipeline_execution_policy_pipeline, :empty_pipeline_always_inject), - build(:pipeline_execution_policy_pipeline, :empty_pipeline_inject_if_no_config) + build(:pipeline_execution_policy_pipeline, :empty_pipeline_always_apply), + build(:pipeline_execution_policy_pipeline, :empty_pipeline_apply_if_no_config) ] end - it 'uses the more restrictive setting (always_inject) and forces creation' do + it 'uses the more restrictive setting (always_apply) and forces creation' do expect(force_creation).to eq(true) end end @@ -764,7 +764,7 @@ let(:policy_pipelines) do [ build(:pipeline_execution_policy_pipeline, - policy_config: build(:pipeline_execution_policy_config, :empty_pipeline_never_inject, + policy_config: build(:pipeline_execution_policy_config, :empty_pipeline_never_apply, policy_config: policy_config_without_experiment)) ] end diff --git a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb b/ee/spec/models/security/pipeline_execution_policy/config_spec.rb index 4b607012ba394e..81ca7295c8ad08 100644 --- a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb +++ b/ee/spec/models/security/pipeline_execution_policy/config_spec.rb @@ -52,19 +52,19 @@ context 'with string strategy' do let(:policy) { build(:pipeline_execution_policy, pipeline_config_strategy: 'inject_ci') } - it 'defaults to always_inject' do - expect(empty_pipeline_behavior_value).to eq('always_inject') + it 'defaults to always_apply' do + expect(empty_pipeline_behavior_value).to eq('always_apply') end end context 'with object strategy and empty_pipeline_behavior specified' do let(:policy) do build(:pipeline_execution_policy, - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'inject_if_no_config' }) + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'apply_if_no_config' }) end it 'returns the specified empty_pipeline_behavior value' do - expect(empty_pipeline_behavior_value).to eq('inject_if_no_config') + expect(empty_pipeline_behavior_value).to eq('apply_if_no_config') end end @@ -73,19 +73,19 @@ build(:pipeline_execution_policy, pipeline_config_strategy: { type: 'inject_policy' }) end - it 'defaults to always_inject' do - expect(empty_pipeline_behavior_value).to eq('always_inject') + it 'defaults to always_apply' do + expect(empty_pipeline_behavior_value).to eq('always_apply') end end - context 'with empty_pipeline_behavior set to never_inject' do + context 'with empty_pipeline_behavior set to never_apply' do let(:policy) do build(:pipeline_execution_policy, - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'never_inject' }) + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'never_apply' }) end - it 'returns never_inject' do - expect(empty_pipeline_behavior_value).to eq('never_inject') + it 'returns never_apply' do + expect(empty_pipeline_behavior_value).to eq('never_apply') end end @@ -95,8 +95,8 @@ pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'invalid_value' }) end - it 'defaults to always_inject' do - expect(empty_pipeline_behavior_value).to eq('always_inject') + it 'defaults to always_apply' do + expect(empty_pipeline_behavior_value).to eq('always_apply') end end @@ -106,8 +106,8 @@ pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: '' }) end - it 'defaults to always_inject' do - expect(empty_pipeline_behavior_value).to eq('always_inject') + it 'defaults to always_apply' do + expect(empty_pipeline_behavior_value).to eq('always_apply') end end @@ -117,8 +117,8 @@ pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 123 }) end - it 'defaults to always_inject after converting to string' do - expect(empty_pipeline_behavior_value).to eq('always_inject') + it 'defaults to always_apply after converting to string' do + expect(empty_pipeline_behavior_value).to eq('always_apply') end end 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 23ac0023ebcdbe..7d49a47f6b464e 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 @@ -1496,14 +1496,6 @@ def policy_include(file) } end - let(:namespace_policy) do - build(:pipeline_execution_policy, behaviour_trait, content: policy_include(namespace_policy_file)) - end - - let(:project_policy) do - build(:pipeline_execution_policy, behaviour_trait, content: policy_include(project_policy_file)) - end - let(:no_ci_config) { nil } let(:ci_yml) do @@ -1541,28 +1533,44 @@ def policy_include(file) where(:project_ci_yaml, :behaviour, :expected_outcome, :expected_builds, :expected_failure_reason, :expected_config_source) do [ - # No CI config - [ref(:no_ci_config), :empty_pipeline_always_inject, :success, 2, nil, 'pipeline_execution_policy_forced'], - [ref(:no_ci_config), :empty_pipeline_inject_if_no_config, :success, 2, nil, 'pipeline_execution_policy_forced'], - [ref(:no_ci_config), :empty_pipeline_never_inject, :error, 0, :filtered_by_rules, nil], - # Has CI config - [ref(:ci_yml), :empty_pipeline_always_inject, :success, 3, nil, 'repository_source'], - [ref(:ci_yml), :empty_pipeline_inject_if_no_config, :success, 3, nil, 'repository_source'], - [ref(:ci_yml), :empty_pipeline_never_inject, :success, 3, nil, 'repository_source'], - # Workflow rules filter out pipeline - [ref(:ci_yml_workflow_rules), :empty_pipeline_always_inject, :success, 2, nil, 'repository_source'], - [ref(:ci_yml_workflow_rules), :empty_pipeline_inject_if_no_config, :error, 0, :filtered_by_workflow_rules, nil], - [ref(:ci_yml_workflow_rules), :empty_pipeline_never_inject, :error, 0, :filtered_by_workflow_rules, nil], - # Job rules filter out all jobs - [ref(:ci_yml_job_rules), :empty_pipeline_always_inject, :success, 2, nil, 'repository_source'], - [ref(:ci_yml_job_rules), :empty_pipeline_inject_if_no_config, :error, 0, :filtered_by_rules, nil], - [ref(:ci_yml_job_rules), :empty_pipeline_never_inject, :error, 0, :filtered_by_rules, nil] + # No CI config - inject_policy strategy (default) + [ref(:no_ci_config), :empty_pipeline_always_apply, :success, 2, nil, 'pipeline_execution_policy_forced'], + [ref(:no_ci_config), :empty_pipeline_apply_if_no_config, :success, 2, nil, 'pipeline_execution_policy_forced'], + [ref(:no_ci_config), :empty_pipeline_never_apply, :error, 0, :filtered_by_rules, nil], + # Has CI config - inject_policy strategy (default) + [ref(:ci_yml), :empty_pipeline_always_apply, :success, 3, nil, 'repository_source'], + [ref(:ci_yml), :empty_pipeline_apply_if_no_config, :success, 3, nil, 'repository_source'], + [ref(:ci_yml), :empty_pipeline_never_apply, :success, 3, nil, 'repository_source'], + # Workflow rules filter out pipeline - inject_policy strategy (default) + [ref(:ci_yml_workflow_rules), :empty_pipeline_always_apply, :success, 2, nil, 'repository_source'], + [ref(:ci_yml_workflow_rules), :empty_pipeline_apply_if_no_config, :error, 0, :filtered_by_workflow_rules, nil], + [ref(:ci_yml_workflow_rules), :empty_pipeline_never_apply, :error, 0, :filtered_by_workflow_rules, nil], + # Job rules filter out all jobs - inject_policy strategy (default) + [ref(:ci_yml_job_rules), :empty_pipeline_always_apply, :success, 2, nil, 'repository_source'], + [ref(:ci_yml_job_rules), :empty_pipeline_apply_if_no_config, :error, 0, :filtered_by_rules, nil], + [ref(:ci_yml_job_rules), :empty_pipeline_never_apply, :error, 0, :filtered_by_rules, nil], + # No CI config - override_project_ci strategy + [ref(:no_ci_config), [:empty_pipeline_always_apply, :override_project_ci_strategy], :success, 2, nil, 'pipeline_execution_policy_forced'], + [ref(:no_ci_config), [:empty_pipeline_apply_if_no_config, :override_project_ci_strategy], :success, 2, nil, 'pipeline_execution_policy_forced'], + [ref(:no_ci_config), [:empty_pipeline_never_apply, :override_project_ci_strategy], :error, 0, :filtered_by_rules, nil], + # Has CI config - override_project_ci strategy + [ref(:ci_yml), [:empty_pipeline_always_apply, :override_project_ci_strategy], :success, 2, nil, 'pipeline_execution_policy_forced'], + [ref(:ci_yml), [:empty_pipeline_apply_if_no_config, :override_project_ci_strategy], :success, 2, nil, 'pipeline_execution_policy_forced'], + [ref(:ci_yml), [:empty_pipeline_never_apply, :override_project_ci_strategy], :error, 0, :filtered_by_rules, nil] ] end with_them do let(:behaviour_trait) { behaviour } + let(:namespace_policy) do + build(:pipeline_execution_policy, *Array(behaviour_trait), content: policy_include(namespace_policy_file)) + end + + let(:project_policy) do + build(:pipeline_execution_policy, *Array(behaviour_trait), content: policy_include(project_policy_file)) + end + if params[:expected_outcome] == :success it 'creates pipeline', :aggregate_failures do expect { execute }.to change { Ci::Build.count }.from(0).to(expected_builds) @@ -1584,16 +1592,16 @@ def policy_include(file) let(:project_ci_yaml) { nil } let(:namespace_policy) do - build(:pipeline_execution_policy, :empty_pipeline_always_inject, + build(:pipeline_execution_policy, :empty_pipeline_always_apply, content: policy_include(namespace_policy_file)) end let(:project_policy) do - build(:pipeline_execution_policy, :empty_pipeline_never_inject, + build(:pipeline_execution_policy, :empty_pipeline_never_apply, content: policy_include(project_policy_file)) end - it 'uses the most restrictive behaviour (always_inject) and creates pipeline', :aggregate_failures do + it 'uses the most restrictive behaviour (always_apply) and creates pipeline', :aggregate_failures do expect { execute }.to change { Ci::Build.count }.from(0).to(2) expect(execute).to be_success @@ -1609,7 +1617,7 @@ def policy_include(file) let(:project_ci_yaml) { nil } let(:namespace_policy) do - build(:pipeline_execution_policy, :empty_pipeline_never_inject, + build(:pipeline_execution_policy, :empty_pipeline_never_apply, content: { include: [{ project: compliance_project.full_path, file: namespace_policy_file, @@ -1618,7 +1626,7 @@ def policy_include(file) end let(:project_policy) do - build(:pipeline_execution_policy, :empty_pipeline_never_inject, + build(:pipeline_execution_policy, :empty_pipeline_never_apply, content: { include: [{ project: compliance_project.full_path, file: project_policy_file, @@ -1626,7 +1634,7 @@ def policy_include(file) }] }) end - it 'ignores empty_pipeline_behavior and uses default always_inject behaviour', :aggregate_failures do + it 'ignores empty_pipeline_behavior and uses default always_apply behaviour', :aggregate_failures do expect { execute }.to change { Ci::Build.count }.from(0).to(2) expect(execute).to be_success @@ -1644,7 +1652,7 @@ def policy_include(file) let(:project_ci_yaml) { nil } let(:namespace_policy) do - build(:pipeline_execution_policy, :empty_pipeline_never_inject, + build(:pipeline_execution_policy, :empty_pipeline_never_apply, content: { include: [{ project: compliance_project.full_path, file: namespace_policy_file, @@ -1653,7 +1661,7 @@ def policy_include(file) end let(:project_policy) do - build(:pipeline_execution_policy, :empty_pipeline_never_inject, + build(:pipeline_execution_policy, :empty_pipeline_never_apply, content: { include: [{ project: compliance_project.full_path, file: project_policy_file, @@ -1661,7 +1669,7 @@ def policy_include(file) }] }) end - it 'ignores empty_pipeline_behavior and uses default always_inject behaviour', :aggregate_failures do + it 'ignores empty_pipeline_behavior and uses default always_apply behaviour', :aggregate_failures do expect { execute }.to change { Ci::Build.count }.from(0).to(2) expect(execute).to be_success diff --git a/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb b/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb index 34b72e5acf3cad..b9943e9cbc060c 100644 --- a/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb +++ b/ee/spec/validators/json_schemas/pipeline_execution_policy_content_spec.rb @@ -35,37 +35,37 @@ context 'with object format' do context 'with valid type and empty_pipeline_behavior' do - it 'accepts inject_policy with empty_pipeline_behavior always_inject' do + it 'accepts inject_policy with empty_pipeline_behavior always_apply' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'always_inject' } + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'always_apply' } ) expect(schema.valid?(policy)).to be true end - it 'accepts inject_policy with empty_pipeline_behavior inject_if_no_config' do + it 'accepts inject_policy with empty_pipeline_behavior apply_if_no_config' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'inject_if_no_config' } + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'apply_if_no_config' } ) expect(schema.valid?(policy)).to be true end - it 'accepts inject_policy with empty_pipeline_behavior never_inject' do + it 'accepts inject_policy with empty_pipeline_behavior never_apply' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'never_inject' } + pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'never_apply' } ) expect(schema.valid?(policy)).to be true end it 'accepts inject_ci with empty_pipeline_behavior' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'inject_ci', empty_pipeline_behavior: 'inject_if_no_config' } + pipeline_config_strategy: { type: 'inject_ci', empty_pipeline_behavior: 'apply_if_no_config' } ) expect(schema.valid?(policy)).to be true end it 'accepts override_project_ci with empty_pipeline_behavior' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'override_project_ci', empty_pipeline_behavior: 'never_inject' } + pipeline_config_strategy: { type: 'override_project_ci', empty_pipeline_behavior: 'never_apply' } ) expect(schema.valid?(policy)).to be true end @@ -83,7 +83,7 @@ context 'with invalid values' do it 'rejects invalid type' do policy = base_policy.merge( - pipeline_config_strategy: { type: 'invalid_type', empty_pipeline_behavior: 'always_inject' } + pipeline_config_strategy: { type: 'invalid_type', empty_pipeline_behavior: 'always_apply' } ) expect(schema.valid?(policy)).to be false end @@ -97,7 +97,7 @@ it 'rejects object without type' do policy = base_policy.merge( - pipeline_config_strategy: { empty_pipeline_behavior: 'always_inject' } + pipeline_config_strategy: { empty_pipeline_behavior: 'always_apply' } ) expect(schema.valid?(policy)).to be false end @@ -105,7 +105,7 @@ it 'rejects object with additional properties' do policy = base_policy.merge( pipeline_config_strategy: { - type: 'inject_policy', empty_pipeline_behavior: 'always_inject', extra: 'value' + type: 'inject_policy', empty_pipeline_behavior: 'always_apply', extra: 'value' } ) expect(schema.valid?(policy)).to be false -- GitLab From 1095dca170d7d9c7d3efc13d13723d0ba4bd8c2d Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Wed, 10 Dec 2025 00:31:23 -0500 Subject: [PATCH 13/24] Fix test post validation removal (JSON schema handles it) --- .../pipeline_execution_policy/config_spec.rb | 44 ------------------- 1 file changed, 44 deletions(-) diff --git a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb b/ee/spec/models/security/pipeline_execution_policy/config_spec.rb index 81ca7295c8ad08..5741f32019b284 100644 --- a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb +++ b/ee/spec/models/security/pipeline_execution_policy/config_spec.rb @@ -77,50 +77,6 @@ expect(empty_pipeline_behavior_value).to eq('always_apply') end end - - context 'with empty_pipeline_behavior set to never_apply' do - let(:policy) do - build(:pipeline_execution_policy, - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'never_apply' }) - end - - it 'returns never_apply' do - expect(empty_pipeline_behavior_value).to eq('never_apply') - end - end - - context 'with invalid empty_pipeline_behavior value' do - let(:policy) do - build(:pipeline_execution_policy, - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 'invalid_value' }) - end - - it 'defaults to always_apply' do - expect(empty_pipeline_behavior_value).to eq('always_apply') - end - end - - context 'with empty string empty_pipeline_behavior value' do - let(:policy) do - build(:pipeline_execution_policy, - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: '' }) - end - - it 'defaults to always_apply' do - expect(empty_pipeline_behavior_value).to eq('always_apply') - end - end - - context 'with numeric empty_pipeline_behavior value' do - let(:policy) do - build(:pipeline_execution_policy, - pipeline_config_strategy: { type: 'inject_policy', empty_pipeline_behavior: 123 }) - end - - it 'defaults to always_apply after converting to string' do - expect(empty_pipeline_behavior_value).to eq('always_apply') - end - end end describe '#suffix' do -- GitLab From 770b37d4d69e861c0a54575195c6dee3b9adc89f Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Wed, 10 Dec 2025 00:45:28 -0500 Subject: [PATCH 14/24] Fix factory trait and rubocop line length --- .../pipeline_execution_policy_configs.rb | 4 +-- .../pipeline_execution_policy_pipeline.rb | 4 +-- ee/spec/factories/security/policies.rb | 5 ++-- .../pipeline_execution_policy_spec.rb | 28 ++++++++++--------- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/ee/spec/factories/security/pipeline_execution_policy_configs.rb b/ee/spec/factories/security/pipeline_execution_policy_configs.rb index b233810dc2c7a1..e7629b75de6524 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_configs.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_configs.rb @@ -61,8 +61,8 @@ policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] end - trait :override_project_ci_strategy do - policy factory: [:pipeline_execution_policy, :override_project_ci_strategy] + trait :override_ci_strategy do + policy factory: [:pipeline_execution_policy, :override_ci_strategy] policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] end end diff --git a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb index 1d3d5bbf738a59..fad86daa00a24e 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb @@ -64,10 +64,10 @@ end end - trait :override_project_ci_strategy do + trait :override_ci_strategy do policy_config do association( - :pipeline_execution_policy_config, :override_project_ci_strategy, + :pipeline_execution_policy_config, :override_ci_strategy, policy_config: association( :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment ) diff --git a/ee/spec/factories/security/policies.rb b/ee/spec/factories/security/policies.rb index db80358b517e59..b1a027298a85c4 100644 --- a/ee/spec/factories/security/policies.rb +++ b/ee/spec/factories/security/policies.rb @@ -384,12 +384,13 @@ pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'never_apply' } } end - trait :override_project_ci_strategy do + trait :override_ci_strategy do # This trait modifies the pipeline_config_strategy to use override_project_ci # It should be applied after a behavior trait after(:build) do |policy_hash| if policy_hash.is_a?(Hash) && policy_hash[:pipeline_config_strategy].is_a?(Hash) - policy_hash[:pipeline_config_strategy] = policy_hash[:pipeline_config_strategy].merge(type: 'override_project_ci') + strategy = policy_hash[:pipeline_config_strategy] + strategy['type'] = 'override_project_ci' end end 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 7d49a47f6b464e..76701d079f75b1 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 @@ -1532,31 +1532,33 @@ def policy_include(file) where(:project_ci_yaml, :behaviour, :expected_outcome, :expected_builds, :expected_failure_reason, :expected_config_source) do + forced = 'pipeline_execution_policy_forced' + repo = 'repository_source' [ # No CI config - inject_policy strategy (default) - [ref(:no_ci_config), :empty_pipeline_always_apply, :success, 2, nil, 'pipeline_execution_policy_forced'], - [ref(:no_ci_config), :empty_pipeline_apply_if_no_config, :success, 2, nil, 'pipeline_execution_policy_forced'], + [ref(:no_ci_config), :empty_pipeline_always_apply, :success, 2, nil, forced], + [ref(:no_ci_config), :empty_pipeline_apply_if_no_config, :success, 2, nil, forced], [ref(:no_ci_config), :empty_pipeline_never_apply, :error, 0, :filtered_by_rules, nil], # Has CI config - inject_policy strategy (default) - [ref(:ci_yml), :empty_pipeline_always_apply, :success, 3, nil, 'repository_source'], - [ref(:ci_yml), :empty_pipeline_apply_if_no_config, :success, 3, nil, 'repository_source'], - [ref(:ci_yml), :empty_pipeline_never_apply, :success, 3, nil, 'repository_source'], + [ref(:ci_yml), :empty_pipeline_always_apply, :success, 3, nil, repo], + [ref(:ci_yml), :empty_pipeline_apply_if_no_config, :success, 3, nil, repo], + [ref(:ci_yml), :empty_pipeline_never_apply, :success, 3, nil, repo], # Workflow rules filter out pipeline - inject_policy strategy (default) - [ref(:ci_yml_workflow_rules), :empty_pipeline_always_apply, :success, 2, nil, 'repository_source'], + [ref(:ci_yml_workflow_rules), :empty_pipeline_always_apply, :success, 2, nil, repo], [ref(:ci_yml_workflow_rules), :empty_pipeline_apply_if_no_config, :error, 0, :filtered_by_workflow_rules, nil], [ref(:ci_yml_workflow_rules), :empty_pipeline_never_apply, :error, 0, :filtered_by_workflow_rules, nil], # Job rules filter out all jobs - inject_policy strategy (default) - [ref(:ci_yml_job_rules), :empty_pipeline_always_apply, :success, 2, nil, 'repository_source'], + [ref(:ci_yml_job_rules), :empty_pipeline_always_apply, :success, 2, nil, repo], [ref(:ci_yml_job_rules), :empty_pipeline_apply_if_no_config, :error, 0, :filtered_by_rules, nil], [ref(:ci_yml_job_rules), :empty_pipeline_never_apply, :error, 0, :filtered_by_rules, nil], # No CI config - override_project_ci strategy - [ref(:no_ci_config), [:empty_pipeline_always_apply, :override_project_ci_strategy], :success, 2, nil, 'pipeline_execution_policy_forced'], - [ref(:no_ci_config), [:empty_pipeline_apply_if_no_config, :override_project_ci_strategy], :success, 2, nil, 'pipeline_execution_policy_forced'], - [ref(:no_ci_config), [:empty_pipeline_never_apply, :override_project_ci_strategy], :error, 0, :filtered_by_rules, nil], + [ref(:no_ci_config), [:empty_pipeline_always_apply, :override_ci_strategy], :success, 2, nil, forced], + [ref(:no_ci_config), [:empty_pipeline_apply_if_no_config, :override_ci_strategy], :success, 2, nil, forced], + [ref(:no_ci_config), [:empty_pipeline_never_apply, :override_ci_strategy], :error, 0, :filtered_by_rules, nil], # Has CI config - override_project_ci strategy - [ref(:ci_yml), [:empty_pipeline_always_apply, :override_project_ci_strategy], :success, 2, nil, 'pipeline_execution_policy_forced'], - [ref(:ci_yml), [:empty_pipeline_apply_if_no_config, :override_project_ci_strategy], :success, 2, nil, 'pipeline_execution_policy_forced'], - [ref(:ci_yml), [:empty_pipeline_never_apply, :override_project_ci_strategy], :error, 0, :filtered_by_rules, nil] + [ref(:ci_yml), [:empty_pipeline_always_apply, :override_ci_strategy], :success, 2, nil, forced], + [ref(:ci_yml), [:empty_pipeline_apply_if_no_config, :override_ci_strategy], :success, 2, nil, forced], + [ref(:ci_yml), [:empty_pipeline_never_apply, :override_ci_strategy], :error, 0, :filtered_by_rules, nil] ] end -- GitLab From 2e774d51361c5e8623ecfcc39f9dcda48494c096 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Wed, 10 Dec 2025 13:31:11 -0500 Subject: [PATCH 15/24] Make trait transient and remove redundant override_ci_strategy trait --- .../pipeline_execution_policy_configs.rb | 5 ---- .../pipeline_execution_policy_pipeline.rb | 11 ------- ee/spec/factories/security/policies.rb | 29 ++++++++++++------- .../pipeline_execution_policy_spec.rb | 12 ++++---- 4 files changed, 24 insertions(+), 33 deletions(-) diff --git a/ee/spec/factories/security/pipeline_execution_policy_configs.rb b/ee/spec/factories/security/pipeline_execution_policy_configs.rb index e7629b75de6524..362330e1bcbd4e 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_configs.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_configs.rb @@ -60,10 +60,5 @@ policy factory: [:pipeline_execution_policy, :empty_pipeline_never_apply] policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] end - - trait :override_ci_strategy do - policy factory: [:pipeline_execution_policy, :override_ci_strategy] - policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] - end end end diff --git a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb index fad86daa00a24e..aa8c077c52740d 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb @@ -64,17 +64,6 @@ end end - trait :override_ci_strategy do - policy_config do - association( - :pipeline_execution_policy_config, :override_ci_strategy, - policy_config: association( - :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment - ) - ) - end - end - transient do job_script { nil } end diff --git a/ee/spec/factories/security/policies.rb b/ee/spec/factories/security/policies.rb index b1a027298a85c4..5bea85ea7a83ef 100644 --- a/ee/spec/factories/security/policies.rb +++ b/ee/spec/factories/security/policies.rb @@ -364,6 +364,10 @@ skip_ci { { allowed: false } } variables_override { nil } + transient do + empty_pipeline_behavior { nil } + end + trait :override_project_ci do pipeline_config_strategy { 'override_project_ci' } end @@ -372,26 +376,29 @@ pipeline_config_strategy { 'inject_policy' } end + trait :inject_ci do + pipeline_config_strategy { 'inject_ci' } + end + trait :empty_pipeline_always_apply do - pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'always_apply' } } + empty_pipeline_behavior { 'always_apply' } end trait :empty_pipeline_apply_if_no_config do - pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'apply_if_no_config' } } + empty_pipeline_behavior { 'apply_if_no_config' } end trait :empty_pipeline_never_apply do - pipeline_config_strategy { { type: 'inject_policy', empty_pipeline_behavior: 'never_apply' } } + empty_pipeline_behavior { 'never_apply' } end - trait :override_ci_strategy do - # This trait modifies the pipeline_config_strategy to use override_project_ci - # It should be applied after a behavior trait - after(:build) do |policy_hash| - if policy_hash.is_a?(Hash) && policy_hash[:pipeline_config_strategy].is_a?(Hash) - strategy = policy_hash[:pipeline_config_strategy] - strategy['type'] = 'override_project_ci' - end + after(:build) do |policy, evaluator| + if evaluator.empty_pipeline_behavior + strategy_type = policy[:pipeline_config_strategy] + policy[:pipeline_config_strategy] = { + type: strategy_type, + empty_pipeline_behavior: evaluator.empty_pipeline_behavior + } end 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 76701d079f75b1..7ba43e9cec1cca 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 @@ -1552,13 +1552,13 @@ def policy_include(file) [ref(:ci_yml_job_rules), :empty_pipeline_apply_if_no_config, :error, 0, :filtered_by_rules, nil], [ref(:ci_yml_job_rules), :empty_pipeline_never_apply, :error, 0, :filtered_by_rules, nil], # No CI config - override_project_ci strategy - [ref(:no_ci_config), [:empty_pipeline_always_apply, :override_ci_strategy], :success, 2, nil, forced], - [ref(:no_ci_config), [:empty_pipeline_apply_if_no_config, :override_ci_strategy], :success, 2, nil, forced], - [ref(:no_ci_config), [:empty_pipeline_never_apply, :override_ci_strategy], :error, 0, :filtered_by_rules, nil], + [ref(:no_ci_config), [:override_project_ci, :empty_pipeline_always_apply], :success, 2, nil, forced], + [ref(:no_ci_config), [:override_project_ci, :empty_pipeline_apply_if_no_config], :success, 2, nil, forced], + [ref(:no_ci_config), [:override_project_ci, :empty_pipeline_never_apply], :error, 0, :filtered_by_rules, nil], # Has CI config - override_project_ci strategy - [ref(:ci_yml), [:empty_pipeline_always_apply, :override_ci_strategy], :success, 2, nil, forced], - [ref(:ci_yml), [:empty_pipeline_apply_if_no_config, :override_ci_strategy], :success, 2, nil, forced], - [ref(:ci_yml), [:empty_pipeline_never_apply, :override_ci_strategy], :error, 0, :filtered_by_rules, nil] + [ref(:ci_yml), [:override_project_ci, :empty_pipeline_always_apply], :success, 2, nil, forced], + [ref(:ci_yml), [:override_project_ci, :empty_pipeline_apply_if_no_config], :success, 2, nil, forced], + [ref(:ci_yml), [:override_project_ci, :empty_pipeline_never_apply], :error, 0, :filtered_by_rules, nil] ] end -- GitLab From 8725ebdc941477bfac176c1672cfeac4f838ce09 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Wed, 10 Dec 2025 13:31:55 -0500 Subject: [PATCH 16/24] Remove unused constant --- ee/app/models/security/pipeline_execution_policy/config.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/ee/app/models/security/pipeline_execution_policy/config.rb b/ee/app/models/security/pipeline_execution_policy/config.rb index 54c6cfd93fca2f..9567550d66aec7 100644 --- a/ee/app/models/security/pipeline_execution_policy/config.rb +++ b/ee/app/models/security/pipeline_execution_policy/config.rb @@ -11,7 +11,6 @@ class Config SUFFIX_STRATEGIES = { on_conflict: 'on_conflict', never: 'never' }.freeze DEFAULT_SKIP_CI_STRATEGY = { allowed: false }.freeze DEFAULT_EMPTY_PIPELINE_BEHAVIOR = 'always_apply' - EMPTY_PIPELINE_BEHAVIOR_OPTIONS = %w[always_apply apply_if_no_config never_apply].freeze POLICY_JOB_SUFFIX = ':policy' attr_reader :content, :config_strategy, :suffix_strategy, :policy_project_id, :policy_index, :name, -- GitLab From 9d066c5f7d7286c98ae190241fcd9f807ddbf92c Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Wed, 10 Dec 2025 17:19:12 -0500 Subject: [PATCH 17/24] Fix bug that duo snuck into apply_if_no_config at some point --- .../pipeline_context.rb | 40 ++++--- .../pipeline_context_spec.rb | 107 +++++++++++++++++- .../pipeline_execution_policy_spec.rb | 87 ++++++++++++++ 3 files changed, 210 insertions(+), 24 deletions(-) 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 7488e413006f74..eb2cea3992e988 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 @@ -87,27 +87,26 @@ def force_pipeline_creation?(pipeline) return false unless has_execution_policy_pipelines? return true unless Feature.enabled?(:pipeline_execution_policy_empty_pipeline_behavior, pipeline.project) - behaviour = effective_empty_pipeline_behavior - - return true if behaviour.nil? || behaviour == EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY - return false if behaviour == EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY - - if behaviour == EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG - # If the project has a CI config, we only apply if the project creates a pipeline. - # This is because we don't have a good way of knowing when both the MR and branch - # pipeline creation has been attempted. To ensure PEPs run in such projects, the - # "Pipelines must succeed" setting is needed as well. - return true unless pipeline.stages.empty? - - # Only continue if we're using the fallback source + case effective_empty_pipeline_behavior + when nil, EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY + true + when EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY + false + when EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG + # We filter out `.pre/.post` stages, as they alone are not considered + # a complete pipeline: + # https://gitlab.com/gitlab-org/gitlab/issues/198518 + return true unless (pipeline.stages.map(&:name) - ::Gitlab::Ci::Config::Stages::EDGES).empty? + + # Only continue if we're using the fallback config source return false unless pipeline.pipeline_execution_policy_forced? - # For projects with no CI config, always apply - return true + # For projects with no CI config we prefer MR pipelines over branch to avoid duplicates + pipeline.merge_request? || (pipeline.branch? && pipeline.open_merge_requests_refs.empty?) + else + # Unknown behavior defaults to always apply + true end - - # Unknown behaviour defaults to always apply - true end def skip_ci_allowed? @@ -303,13 +302,12 @@ def effective_empty_pipeline_behavior end strong_memoize_attr :effective_empty_pipeline_behavior - def empty_pipeline_behavior_priority(behaviour) + def empty_pipeline_behavior_priority(behavior) # Lower number = more restrictive (most limiting to project freedom) - case behaviour + case behavior when EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY then 0 when EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG then 1 when EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY then 2 - else 3 # unknown end 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 19d90d60274c24..0c68734ea29cff 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 @@ -711,7 +711,7 @@ context 'when project has CI config (pipeline has stages)' do before do - allow(pipeline).to receive(:stages).and_return([instance_double(Ci::Stage)]) + allow(pipeline).to receive(:stages).and_return([instance_double(Ci::Stage, name: 'test')]) end it 'forces pipeline creation' do @@ -719,13 +719,76 @@ end end + context 'when pipeline only has .pre/.post stages' do + before do + pre_stage = instance_double(Ci::Stage, name: '.pre') + post_stage = instance_double(Ci::Stage, name: '.post') + allow(pipeline).to receive_messages( + stages: [pre_stage, post_stage], + pipeline_execution_policy_forced?: true, + merge_request?: false, + branch?: true, + open_merge_requests_refs: [] + ) + end + + it 'treats it as empty and forces pipeline creation' do + expect(force_creation).to eq(true) + end + end + context 'when project has no CI config (pipeline has no stages)' do before do allow(pipeline).to receive_messages(stages: [], pipeline_execution_policy_forced?: true) end - it 'forces pipeline creation' do - expect(force_creation).to eq(true) + context 'when pipeline is a merge request pipeline' do + before do + allow(pipeline).to receive_messages(merge_request?: true, branch?: false) + end + + it 'forces pipeline creation' do + expect(force_creation).to eq(true) + end + end + + context 'when pipeline is a branch pipeline' do + before do + allow(pipeline).to receive_messages(merge_request?: false, branch?: true) + end + + context 'when there are no open merge requests for the branch' do + before do + allow(pipeline).to receive(:open_merge_requests_refs).and_return([]) + end + + it 'forces pipeline creation' do + expect(force_creation).to eq(true) + end + end + + context 'when there are open merge requests for the branch' do + before do + allow(pipeline).to receive(:open_merge_requests_refs).and_return(['refs/merge-requests/1/head']) + end + + it 'does not force pipeline creation to avoid duplicates' do + expect(force_creation).to eq(false) + end + end + end + end + + context 'when pipeline is not using fallback config source' do + before do + allow(pipeline).to receive_messages( + stages: [], + pipeline_execution_policy_forced?: false + ) + end + + it 'does not force pipeline creation' do + expect(force_creation).to eq(false) end end end @@ -756,6 +819,32 @@ end end + context 'when policies have apply_if_no_config and never_apply' do + let(:policy_pipelines) do + [ + build(:pipeline_execution_policy_pipeline, :empty_pipeline_apply_if_no_config), + build(:pipeline_execution_policy_pipeline, :empty_pipeline_never_apply) + ] + end + + it 'uses the most restrictive setting (never_apply) and does not force creation' do + expect(force_creation).to eq(false) + end + end + + context 'when all policies have never_apply' do + let(:policy_pipelines) do + [ + build(:pipeline_execution_policy_pipeline, :empty_pipeline_never_apply), + build(:pipeline_execution_policy_pipeline, :empty_pipeline_never_apply) + ] + end + + it 'does not force pipeline creation' do + expect(force_creation).to eq(false) + end + end + context 'when experiment is disabled' do let(:policy_config_without_experiment) do build(:security_orchestration_policy_configuration) @@ -773,6 +862,18 @@ expect(force_creation).to eq(true) end end + + context 'when effective_empty_pipeline_behavior returns unknown value' do + let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1) } + + before do + allow(context).to receive(:effective_empty_pipeline_behavior).and_return('unknown_behaviour') + end + + it 'defaults to always apply and forces creation' do + expect(force_creation).to eq(true) + end + end end end 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 7ba43e9cec1cca..1b828652ec3108 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 @@ -1590,6 +1590,93 @@ def policy_include(file) end end + describe 'apply_if_no_ci_config by pipeline source' do + let(:project_ci_yaml) { nil } + + let(:namespace_policy) do + build(:pipeline_execution_policy, :empty_pipeline_apply_if_no_config, + content: policy_include(namespace_policy_file)) + end + + let(:project_policy) do + build(:pipeline_execution_policy, :empty_pipeline_apply_if_no_config, + content: policy_include(project_policy_file)) + end + + let(:namespace_policy_content) do + { + workflow: { rules: [{ when: 'always' }] }, + namespace_policy_job: { stage: 'build', script: 'namespace script' } + } + end + + let(:project_policy_content) do + { + workflow: { rules: [{ when: 'always' }] }, + project_policy_job: { stage: 'build', script: 'project script' } + } + end + + context 'when branch pipeline' do + context 'with an open MR' do + let(:params) { { ref: 'feature' } } + + before do + create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') + end + + it 'does not create branch pipeline to avoid duplicate with MR pipeline', :aggregate_failures do + expect { execute }.not_to change { Ci::Build.count } + + expect(execute).to be_error + expect(execute.payload).not_to be_persisted + end + end + + context 'without open MR' do + let(:params) { { ref: 'feature' } } + + it 'creates branch pipeline when no open MR exists', :aggregate_failures do + expect { execute }.to change { Ci::Build.count }.from(0).to(2) + + expect(execute).to be_success + expect(execute.payload).to be_persisted + expect(execute.payload.config_source).to eq('pipeline_execution_policy_forced') + end + end + end + + context 'when merge request pipeline' do + let(:source) { :merge_request_event } + + let(:merge_request) do + create(:merge_request, source_project: project, target_project: project, + source_branch: 'feature', target_branch: 'master') + end + + let(:opts) { { merge_request: merge_request } } + let(:params) do + { ref: merge_request.ref_path, + source_sha: merge_request.source_branch_sha, + target_sha: merge_request.target_branch_sha, + checkout_sha: merge_request.diff_head_sha } + end + + before do + stub_licensed_features(security_orchestration_policies: true) + end + + it 'creates MR pipeline for project without CI config', :aggregate_failures do + expect { execute }.to change { Ci::Build.count }.from(0).to(2) + + expect(execute).to be_success + expect(execute.payload).to be_persisted + expect(execute.payload.merge_request).to eq(merge_request) + expect(execute.payload.config_source).to eq('pipeline_execution_policy_forced') + end + end + end + context 'with mixed behaviours across policies' do let(:project_ci_yaml) { nil } -- GitLab From 5186bff8b01f0c17bc53697743ffc97cad1d98a6 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Thu, 11 Dec 2025 10:27:19 -0500 Subject: [PATCH 18/24] Attempt to fix undercoverage, add transient helper --- .../pipeline_context.rb | 8 +- .../pipeline_execution_policy_configs.rb | 15 ++++ .../pipeline_execution_policy_pipeline.rb | 76 ++++++++----------- .../pipeline_context_spec.rb | 8 ++ .../pipeline_execution_policy_spec.rb | 47 ++++++------ 5 files changed, 88 insertions(+), 66 deletions(-) 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 eb2cea3992e988..befbd63f2e210a 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 @@ -18,6 +18,11 @@ class PipelineContext EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY = 'always_apply' EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG = 'apply_if_no_config' EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY = 'never_apply' + EMPTY_PIPELINE_BEHAVIORS = [ + EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY, + EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG, + EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY + ].freeze attr_reader :policy_pipelines, :override_policy_stages, :injected_policy_stages @@ -294,7 +299,8 @@ def effective_empty_pipeline_behavior # "Restrictive" means most limiting to project freedom behaviour_values = policy_pipelines.filter_map do |policy_pipeline| if policy_pipeline.policy_config.experiment_enabled?(:empty_pipeline_behavior_option) - policy_pipeline.policy_config.empty_pipeline_behavior + b = policy_pipeline.policy_config.empty_pipeline_behavior + b if EMPTY_PIPELINE_BEHAVIORS.include?(b) end end diff --git a/ee/spec/factories/security/pipeline_execution_policy_configs.rb b/ee/spec/factories/security/pipeline_execution_policy_configs.rb index 362330e1bcbd4e..0c26664f92fefd 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_configs.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_configs.rb @@ -14,6 +14,17 @@ policy = attributes[:policy] policy[:content] = attributes[:content] if attributes[:content].present? policy_config = attributes[:policy_config] + + if attributes[:empty_pipeline_behavior] + strategy = policy[:pipeline_config_strategy] + strategy_type = strategy.is_a?(Hash) ? strategy[:type] : strategy + policy[:pipeline_config_strategy] = { + type: strategy_type, + empty_pipeline_behavior: attributes[:empty_pipeline_behavior].to_s + } + policy_config.experiments = { empty_pipeline_behavior_option: { enabled: true } } + end + allow(policy_config).to receive(:configuration_sha).and_return(attributes[:policy_sha] || 'policy_sha') new(policy: policy, policy_config: policy_config, policy_index: attributes[:policy_index]) end @@ -60,5 +71,9 @@ policy factory: [:pipeline_execution_policy, :empty_pipeline_never_apply] policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] end + + transient do + empty_pipeline_behavior { nil } + end end end diff --git a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb index aa8c077c52740d..09360ab2e58785 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb @@ -8,73 +8,63 @@ class: '::Security::PipelineExecutionPolicy::Pipeline' ) do pipeline factory: :ci_empty_pipeline - policy_config factory: :pipeline_execution_policy_config + + transient do + job_script { nil } + empty_pipeline_behavior { nil } + end + + policy_config do |evaluator| + if evaluator.empty_pipeline_behavior + association(:pipeline_execution_policy_config, empty_pipeline_behavior: evaluator.empty_pipeline_behavior) + else + association(:pipeline_execution_policy_config) + end + end skip_create initialize_with do - new(**attributes) + new(**attributes.except(:empty_pipeline_behavior)) end trait :override_project_ci do - policy_config factory: [:pipeline_execution_policy_config, :override_project_ci] + policy_config do |evaluator| + association(:pipeline_execution_policy_config, :override_project_ci, + empty_pipeline_behavior: evaluator.empty_pipeline_behavior) + end end trait :suffix_never do - policy_config factory: [:pipeline_execution_policy_config, :suffix_never] + policy_config do |evaluator| + association(:pipeline_execution_policy_config, :suffix_never, + empty_pipeline_behavior: evaluator.empty_pipeline_behavior) + end end trait :skip_ci_allowed do - policy_config factory: [:pipeline_execution_policy_config, :skip_ci_allowed] + policy_config do |evaluator| + association(:pipeline_execution_policy_config, :skip_ci_allowed, + empty_pipeline_behavior: evaluator.empty_pipeline_behavior) + end end trait :skip_ci_disallowed do - policy_config factory: [:pipeline_execution_policy_config, :skip_ci_disallowed] + policy_config do |evaluator| + association(:pipeline_execution_policy_config, :skip_ci_disallowed, + empty_pipeline_behavior: evaluator.empty_pipeline_behavior) + end end trait :empty_pipeline_always_apply do - policy_config do - association( - :pipeline_execution_policy_config, :empty_pipeline_always_apply, - policy_config: association( - :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment - ) - ) - end + empty_pipeline_behavior { 'always_apply' } end trait :empty_pipeline_apply_if_no_config do - policy_config do - association( - :pipeline_execution_policy_config, :empty_pipeline_apply_if_no_config, - policy_config: association( - :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment - ) - ) - end + empty_pipeline_behavior { 'apply_if_no_config' } end trait :empty_pipeline_never_apply do - policy_config do - association( - :pipeline_execution_policy_config, :empty_pipeline_never_apply, - policy_config: association( - :security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment - ) - ) - end - end - - transient do - job_script { nil } - end - - after(:build) do |instance, evaluator| - next unless evaluator.job_script - - job = instance.pipeline.stages[0].statuses[0] - updated_options = { script: evaluator.job_script } - - Ci::JobFactoryHelpers.mutate_temp_job_definition(job, options: updated_options) + empty_pipeline_behavior { 'never_apply' } end end 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 0c68734ea29cff..9256352f9adde1 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 @@ -692,6 +692,14 @@ end context 'when feature flag is enabled' do + context 'when empty_pipeline_behavior is an unexpected value' do + let(:policy_pipelines) do + build_list(:pipeline_execution_policy_pipeline, 1, empty_pipeline_behavior: :something_invalid) + end + + it { is_expected.to eq(true) } + end + context 'when empty_pipeline_behavior is "always_apply"' do let(:policy_pipelines) { build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_always_apply) } 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 1b828652ec3108..778e1530e84dfa 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 @@ -1530,35 +1530,36 @@ def policy_include(file) YAML end - where(:project_ci_yaml, :behaviour, :expected_outcome, :expected_builds, :expected_failure_reason, - :expected_config_source) do + where(:project_ci_yaml, :strategy, :empty_pipeline_behavior, :expected_outcome, :expected_builds, + :expected_failure_reason, :expected_config_source) do forced = 'pipeline_execution_policy_forced' repo = 'repository_source' [ # No CI config - inject_policy strategy (default) - [ref(:no_ci_config), :empty_pipeline_always_apply, :success, 2, nil, forced], - [ref(:no_ci_config), :empty_pipeline_apply_if_no_config, :success, 2, nil, forced], - [ref(:no_ci_config), :empty_pipeline_never_apply, :error, 0, :filtered_by_rules, nil], + [ref(:no_ci_config), :inject_policy, 'always_apply', :success, 2, nil, forced], + [ref(:no_ci_config), :inject_policy, 'apply_if_no_config', :success, 2, nil, forced], + [ref(:no_ci_config), :inject_policy, 'never_apply', :error, 0, :filtered_by_rules, nil], # Has CI config - inject_policy strategy (default) - [ref(:ci_yml), :empty_pipeline_always_apply, :success, 3, nil, repo], - [ref(:ci_yml), :empty_pipeline_apply_if_no_config, :success, 3, nil, repo], - [ref(:ci_yml), :empty_pipeline_never_apply, :success, 3, nil, repo], + [ref(:ci_yml), :inject_policy, 'always_apply', :success, 3, nil, repo], + [ref(:ci_yml), :inject_policy, 'apply_if_no_config', :success, 3, nil, repo], + [ref(:ci_yml), :inject_policy, 'never_apply', :success, 3, nil, repo], # Workflow rules filter out pipeline - inject_policy strategy (default) - [ref(:ci_yml_workflow_rules), :empty_pipeline_always_apply, :success, 2, nil, repo], - [ref(:ci_yml_workflow_rules), :empty_pipeline_apply_if_no_config, :error, 0, :filtered_by_workflow_rules, nil], - [ref(:ci_yml_workflow_rules), :empty_pipeline_never_apply, :error, 0, :filtered_by_workflow_rules, nil], + [ref(:ci_yml_workflow_rules), :inject_policy, 'always_apply', :success, 2, nil, repo], + [ref(:ci_yml_workflow_rules), :inject_policy, 'apply_if_no_config', :error, 0, :filtered_by_workflow_rules, + nil], + [ref(:ci_yml_workflow_rules), :inject_policy, 'never_apply', :error, 0, :filtered_by_workflow_rules, nil], # Job rules filter out all jobs - inject_policy strategy (default) - [ref(:ci_yml_job_rules), :empty_pipeline_always_apply, :success, 2, nil, repo], - [ref(:ci_yml_job_rules), :empty_pipeline_apply_if_no_config, :error, 0, :filtered_by_rules, nil], - [ref(:ci_yml_job_rules), :empty_pipeline_never_apply, :error, 0, :filtered_by_rules, nil], + [ref(:ci_yml_job_rules), :inject_policy, 'always_apply', :success, 2, nil, repo], + [ref(:ci_yml_job_rules), :inject_policy, 'apply_if_no_config', :error, 0, :filtered_by_rules, nil], + [ref(:ci_yml_job_rules), :inject_policy, 'never_apply', :error, 0, :filtered_by_rules, nil], # No CI config - override_project_ci strategy - [ref(:no_ci_config), [:override_project_ci, :empty_pipeline_always_apply], :success, 2, nil, forced], - [ref(:no_ci_config), [:override_project_ci, :empty_pipeline_apply_if_no_config], :success, 2, nil, forced], - [ref(:no_ci_config), [:override_project_ci, :empty_pipeline_never_apply], :error, 0, :filtered_by_rules, nil], + [ref(:no_ci_config), :override_project_ci, 'always_apply', :success, 2, nil, forced], + [ref(:no_ci_config), :override_project_ci, 'apply_if_no_config', :success, 2, nil, forced], + [ref(:no_ci_config), :override_project_ci, 'never_apply', :error, 0, :filtered_by_rules, nil], # Has CI config - override_project_ci strategy - [ref(:ci_yml), [:override_project_ci, :empty_pipeline_always_apply], :success, 2, nil, forced], - [ref(:ci_yml), [:override_project_ci, :empty_pipeline_apply_if_no_config], :success, 2, nil, forced], - [ref(:ci_yml), [:override_project_ci, :empty_pipeline_never_apply], :error, 0, :filtered_by_rules, nil] + [ref(:ci_yml), :override_project_ci, 'always_apply', :success, 2, nil, forced], + [ref(:ci_yml), :override_project_ci, 'apply_if_no_config', :success, 2, nil, forced], + [ref(:ci_yml), :override_project_ci, 'never_apply', :error, 0, :filtered_by_rules, nil] ] end @@ -1566,11 +1567,13 @@ def policy_include(file) let(:behaviour_trait) { behaviour } let(:namespace_policy) do - build(:pipeline_execution_policy, *Array(behaviour_trait), content: policy_include(namespace_policy_file)) + build(:pipeline_execution_policy, strategy, empty_pipeline_behavior: empty_pipeline_behavior, + content: policy_include(namespace_policy_file)) end let(:project_policy) do - build(:pipeline_execution_policy, *Array(behaviour_trait), content: policy_include(project_policy_file)) + build(:pipeline_execution_policy, strategy, empty_pipeline_behavior: empty_pipeline_behavior, + content: policy_include(project_policy_file)) end if params[:expected_outcome] == :success -- GitLab From ca82a5754ec9338bd2c57060b246db599ca77661 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Thu, 11 Dec 2025 12:30:30 -0500 Subject: [PATCH 19/24] Fix transient factory attributes --- .../pipeline_execution_policy_configs.rb | 41 +++++++++++-------- .../pipeline_execution_policy_pipeline.rb | 9 ++++ 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/ee/spec/factories/security/pipeline_execution_policy_configs.rb b/ee/spec/factories/security/pipeline_execution_policy_configs.rb index 0c26664f92fefd..b13044a50684ed 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_configs.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_configs.rb @@ -5,25 +5,38 @@ :pipeline_execution_policy_config, class: '::Security::PipelineExecutionPolicy::Config' ) do - policy factory: :pipeline_execution_policy - policy_config factory: :security_orchestration_policy_configuration, security_policy_management_project_id: 123456 policy_index { 0 } - skip_create - initialize_with do - policy = attributes[:policy] - policy[:content] = attributes[:content] if attributes[:content].present? - policy_config = attributes[:policy_config] + transient do + empty_pipeline_behavior { nil } + end - if attributes[:empty_pipeline_behavior] - strategy = policy[:pipeline_config_strategy] + policy do |evaluator| + base_policy = build(:pipeline_execution_policy) + if evaluator.empty_pipeline_behavior + strategy = base_policy[:pipeline_config_strategy] strategy_type = strategy.is_a?(Hash) ? strategy[:type] : strategy - policy[:pipeline_config_strategy] = { + base_policy[:pipeline_config_strategy] = { type: strategy_type, - empty_pipeline_behavior: attributes[:empty_pipeline_behavior].to_s + empty_pipeline_behavior: evaluator.empty_pipeline_behavior.to_s } - policy_config.experiments = { empty_pipeline_behavior_option: { enabled: true } } end + base_policy + end + + policy_config do |evaluator| + config = build(:security_orchestration_policy_configuration, security_policy_management_project_id: 123456) + if evaluator.empty_pipeline_behavior + config.experiments = { 'empty_pipeline_behavior_option' => { 'enabled' => true } } + end + config + end + + skip_create + initialize_with do + policy = attributes[:policy] + policy[:content] = attributes[:content] if attributes[:content].present? + policy_config = attributes[:policy_config] allow(policy_config).to receive(:configuration_sha).and_return(attributes[:policy_sha] || 'policy_sha') new(policy: policy, policy_config: policy_config, policy_index: attributes[:policy_index]) @@ -71,9 +84,5 @@ policy factory: [:pipeline_execution_policy, :empty_pipeline_never_apply] policy_config factory: [:security_orchestration_policy_configuration, :with_empty_pipeline_behavior_experiment] end - - transient do - empty_pipeline_behavior { nil } - end end end diff --git a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb index 09360ab2e58785..4765639465519b 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb @@ -66,5 +66,14 @@ trait :empty_pipeline_never_apply do empty_pipeline_behavior { 'never_apply' } end + + after(:build) do |instance, evaluator| + next unless evaluator.job_script + + job = instance.pipeline.stages[0].statuses[0] + updated_options = { script: evaluator.job_script } + + Ci::JobFactoryHelpers.mutate_temp_job_definition(job, options: updated_options) + end end end -- GitLab From 51420f2cefc474cccba98e1201f05408326ae52a Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Thu, 11 Dec 2025 14:56:29 -0500 Subject: [PATCH 20/24] Fix rubocop offenses --- .../factories/security/pipeline_execution_policy_configs.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ee/spec/factories/security/pipeline_execution_policy_configs.rb b/ee/spec/factories/security/pipeline_execution_policy_configs.rb index b13044a50684ed..897484215789ca 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_configs.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_configs.rb @@ -12,7 +12,7 @@ end policy do |evaluator| - base_policy = build(:pipeline_execution_policy) + base_policy = association(:pipeline_execution_policy) if evaluator.empty_pipeline_behavior strategy = base_policy[:pipeline_config_strategy] strategy_type = strategy.is_a?(Hash) ? strategy[:type] : strategy @@ -21,14 +21,16 @@ empty_pipeline_behavior: evaluator.empty_pipeline_behavior.to_s } end + base_policy end policy_config do |evaluator| - config = build(:security_orchestration_policy_configuration, security_policy_management_project_id: 123456) + config = association(:security_orchestration_policy_configuration, security_policy_management_project_id: 123456) if evaluator.empty_pipeline_behavior config.experiments = { 'empty_pipeline_behavior_option' => { 'enabled' => true } } end + config end -- GitLab From 11ae821417cc78dcd58bd3b71fb2ebdb02416725 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Fri, 12 Dec 2025 10:01:35 -0500 Subject: [PATCH 21/24] Simplify populate.rb override logic --- ee/lib/ee/gitlab/ci/pipeline/chain/populate.rb | 8 +++++--- lib/gitlab/ci/pipeline/chain/populate.rb | 8 +------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/ee/lib/ee/gitlab/ci/pipeline/chain/populate.rb b/ee/lib/ee/gitlab/ci/pipeline/chain/populate.rb index 8b315b991fea68..31b6402ec5e2d8 100644 --- a/ee/lib/ee/gitlab/ci/pipeline/chain/populate.rb +++ b/ee/lib/ee/gitlab/ci/pipeline/chain/populate.rb @@ -10,9 +10,11 @@ module Populate private - override :has_execution_policy_pipelines? - def has_execution_policy_pipelines? - super && force_pipeline_creation_for_policies? + override :no_pipeline_to_create? + def no_pipeline_to_create? + # If there are security policy pipelines that force pipeline creation, + # they will be merged onto the pipeline in PipelineExecutionPolicies::ApplyPolicies + super && !force_pipeline_creation_for_policies? end def force_pipeline_creation_for_policies? diff --git a/lib/gitlab/ci/pipeline/chain/populate.rb b/lib/gitlab/ci/pipeline/chain/populate.rb index 996b86da3aecef..1c4cf4c37593ac 100644 --- a/lib/gitlab/ci/pipeline/chain/populate.rb +++ b/lib/gitlab/ci/pipeline/chain/populate.rb @@ -36,13 +36,7 @@ def break? private def no_pipeline_to_create? - # If there are security policy pipelines, - # they will be merged onto the pipeline in PipelineExecutionPolicies::ApplyPolicies - stage_names.empty? && !has_execution_policy_pipelines? - end - - def has_execution_policy_pipelines? - @command.pipeline_policy_context&.pipeline_execution_context&.has_execution_policy_pipelines? + stage_names.empty? end def stage_names -- GitLab From 9a6cb3f47e18c1af060076d5bd494810635cb7fc Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Fri, 12 Dec 2025 10:23:55 -0500 Subject: [PATCH 22/24] Simplify empty_pipelien_behavior priority logic --- .../pipeline_context.rb | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) 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 befbd63f2e210a..a9725dd2d1620d 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 @@ -18,11 +18,17 @@ class PipelineContext EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY = 'always_apply' EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG = 'apply_if_no_config' EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY = 'never_apply' - EMPTY_PIPELINE_BEHAVIORS = [ - EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY, - EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG, - EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY - ].freeze + + # Priority ordering of the empty_pipeline_behavior between policies. + # If policies have different settings, use the most restrictive + # behavior (always_apply > apply_if_no_config > never_apply). + # Restrictive means most limiting to project freedom (applies more + # aggressively, creates more pipelines). + EMPTY_PIPELINE_BEHAVIOR_PRIORITIES = { + EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY => 2, + EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG => 1, + EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY => 0 + }.freeze attr_reader :policy_pipelines, :override_policy_stages, :injected_policy_stages @@ -293,30 +299,17 @@ def stages_compatible?(stages, target_stages) end def effective_empty_pipeline_behavior - # Determine the effective empty_pipeline_behavior setting - # If policies have different settings, use the most restrictive - # (always_apply > apply_if_no_config > never_apply) - # "Restrictive" means most limiting to project freedom - behaviour_values = policy_pipelines.filter_map do |policy_pipeline| + behavior_values = policy_pipelines.filter_map do |policy_pipeline| if policy_pipeline.policy_config.experiment_enabled?(:empty_pipeline_behavior_option) - b = policy_pipeline.policy_config.empty_pipeline_behavior - b if EMPTY_PIPELINE_BEHAVIORS.include?(b) + policy_pipeline.policy_config.empty_pipeline_behavior end end - behaviour_values.min_by { |value| empty_pipeline_behavior_priority(value) } + # defensive programming: unknown values get lowest priority + behavior_values.max_by { |value| EMPTY_PIPELINE_BEHAVIOR_PRIORITIES.fetch(value, -1) } end strong_memoize_attr :effective_empty_pipeline_behavior - def empty_pipeline_behavior_priority(behavior) - # Lower number = more restrictive (most limiting to project freedom) - case behavior - when EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY then 0 - when EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG then 1 - when EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY then 2 - end - end - delegate :measure, to: ::Security::SecurityOrchestrationPolicies::ObserveHistogramsService end end -- GitLab From e7c433972f51ab36ad5bd0838606bf839d4fa734 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Mon, 15 Dec 2025 07:59:30 -0500 Subject: [PATCH 23/24] Remove redundant empty check, memoize by pipeline --- .../pipeline_context.rb | 44 +++++++++---------- .../pipeline_context_spec.rb | 10 ----- 2 files changed, 21 insertions(+), 33 deletions(-) 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 a9725dd2d1620d..06a3ef8fe333df 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 @@ -95,28 +95,27 @@ def scheduled_execution_policy_pipeline? end def force_pipeline_creation?(pipeline) - return false unless has_execution_policy_pipelines? - return true unless Feature.enabled?(:pipeline_execution_policy_empty_pipeline_behavior, pipeline.project) - - case effective_empty_pipeline_behavior - when nil, EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY - true - when EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY - false - when EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG - # We filter out `.pre/.post` stages, as they alone are not considered - # a complete pipeline: - # https://gitlab.com/gitlab-org/gitlab/issues/198518 - return true unless (pipeline.stages.map(&:name) - ::Gitlab::Ci::Config::Stages::EDGES).empty? - - # Only continue if we're using the fallback config source - return false unless pipeline.pipeline_execution_policy_forced? - - # For projects with no CI config we prefer MR pipelines over branch to avoid duplicates - pipeline.merge_request? || (pipeline.branch? && pipeline.open_merge_requests_refs.empty?) - else - # Unknown behavior defaults to always apply - true + strong_memoize_with(:force_pipeline_creation, pipeline) do + return false unless has_execution_policy_pipelines? + + return true unless Feature.enabled?(:pipeline_execution_policy_empty_pipeline_behavior, + pipeline.project) + + case effective_empty_pipeline_behavior + when EMPTY_PIPELINE_BEHAVIOR_ALWAYS_APPLY + true + when EMPTY_PIPELINE_BEHAVIOR_NEVER_APPLY + false + when EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG + # Only continue if we're using the fallback config source + return false unless pipeline.pipeline_execution_policy_forced? + + # For projects with no CI config we prefer MR pipelines over branch to avoid duplicates + pipeline.merge_request? || (pipeline.branch? && pipeline.open_merge_requests_refs.empty?) + else + # Unknown behavior defaults to always apply + true + end end end @@ -308,7 +307,6 @@ def effective_empty_pipeline_behavior # defensive programming: unknown values get lowest priority behavior_values.max_by { |value| EMPTY_PIPELINE_BEHAVIOR_PRIORITIES.fetch(value, -1) } end - strong_memoize_attr :effective_empty_pipeline_behavior delegate :measure, to: ::Security::SecurityOrchestrationPolicies::ObserveHistogramsService 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 9256352f9adde1..b8252ff7f393fe 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 @@ -717,16 +717,6 @@ build_list(:pipeline_execution_policy_pipeline, 1, :empty_pipeline_apply_if_no_config) end - context 'when project has CI config (pipeline has stages)' do - before do - allow(pipeline).to receive(:stages).and_return([instance_double(Ci::Stage, name: 'test')]) - end - - it 'forces pipeline creation' do - expect(force_creation).to eq(true) - end - end - context 'when pipeline only has .pre/.post stages' do before do pre_stage = instance_double(Ci::Stage, name: '.pre') -- GitLab From ff97b7a990e67e4cc87f647212b36d2992a8fd63 Mon Sep 17 00:00:00 2001 From: Hordur Freyr Yngvason Date: Mon, 15 Dec 2025 08:41:55 -0500 Subject: [PATCH 24/24] Fix return from block --- .../pipeline_context.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) 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 06a3ef8fe333df..eae18db240acd2 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 @@ -95,10 +95,10 @@ def scheduled_execution_policy_pipeline? end def force_pipeline_creation?(pipeline) - strong_memoize_with(:force_pipeline_creation, pipeline) do - return false unless has_execution_policy_pipelines? + return false unless has_execution_policy_pipelines? - return true unless Feature.enabled?(:pipeline_execution_policy_empty_pipeline_behavior, + strong_memoize_with(:force_pipeline_creation, pipeline) do + break true unless Feature.enabled?(:pipeline_execution_policy_empty_pipeline_behavior, pipeline.project) case effective_empty_pipeline_behavior @@ -108,10 +108,10 @@ def force_pipeline_creation?(pipeline) false when EMPTY_PIPELINE_BEHAVIOR_APPLY_IF_NO_CONFIG # Only continue if we're using the fallback config source - return false unless pipeline.pipeline_execution_policy_forced? - - # For projects with no CI config we prefer MR pipelines over branch to avoid duplicates - pipeline.merge_request? || (pipeline.branch? && pipeline.open_merge_requests_refs.empty?) + pipeline.pipeline_execution_policy_forced? && ( + # For projects with no CI config we prefer MR pipelines over branch to avoid duplicates + pipeline.merge_request? || (pipeline.branch? && pipeline.open_merge_requests_refs.empty?) + ) else # Unknown behavior defaults to always apply true -- GitLab