From f33e48bf09692ab479eabd1e83674be76113515a Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Fri, 12 Dec 2025 15:10:37 +0100 Subject: [PATCH 1/5] Batch SynyPolicyWorker with delays to prevent worker saturation This change adds delay to batches of args for SyncPolicyWorker to reduce worker saturation when a large namespace does policy sync. EE: true Changelog: fixed --- ee/app/workers/security/sync_policy_worker.rb | 26 ++-- .../security/sync_policy_worker_spec.rb | 115 +++++++++++++++--- 2 files changed, 120 insertions(+), 21 deletions(-) diff --git a/ee/app/workers/security/sync_policy_worker.rb b/ee/app/workers/security/sync_policy_worker.rb index 9d58913612d52b..500aea1425f08a 100644 --- a/ee/app/workers/security/sync_policy_worker.rb +++ b/ee/app/workers/security/sync_policy_worker.rb @@ -12,6 +12,9 @@ class SyncPolicyWorker feature_category :security_policy_management + BATCH_DELAY_INTERVAL = 1.second + BATCH_SIZE = 100 + def handle_event(event) security_policy_id = event.data[:security_policy_id] policy = Security::Policy.find_by_id(security_policy_id) || return @@ -72,16 +75,25 @@ def sync_policy_for_projects(policy, event_data = {}, event = nil) clear_policy_sync_state(policy.security_orchestration_policy_configuration_id) + batch_index = 0 policy.security_orchestration_policy_configuration.all_project_ids do |project_ids| append_projects_to_sync(policy.security_orchestration_policy_configuration_id, project_ids) - ::Security::SyncProjectPolicyWorker.bulk_perform_async_with_contexts( - project_ids, - arguments_proc: ->(project_id) do - [project_id, policy.id, event_data, event_payload.deep_stringify_keys] - end, - context_proc: ->(_) { config_context } - ) + project_ids.each_slice(BATCH_SIZE).with_index do |batch, slice_index| + # Use [1.second, delay].max to ensure delay is always at least 1 second in the future + delay = [1.second, (batch_index + slice_index) * BATCH_DELAY_INTERVAL].max + + ::Security::SyncProjectPolicyWorker.bulk_perform_in_with_contexts( + delay, + batch, + arguments_proc: ->(project_id) do + [project_id, policy.id, event_data, event_payload.deep_stringify_keys] + end, + context_proc: ->(_) { config_context } + ) + end + + batch_index += (project_ids.size.to_f / BATCH_SIZE).ceil end end diff --git a/ee/spec/workers/security/sync_policy_worker_spec.rb b/ee/spec/workers/security/sync_policy_worker_spec.rb index 64ce497d64d3f5..c0fdf4c64582a3 100644 --- a/ee/spec/workers/security/sync_policy_worker_spec.rb +++ b/ee/spec/workers/security/sync_policy_worker_spec.rb @@ -110,10 +110,11 @@ let(:event) { policy_created_event } end - it 'calls Security::SyncProjectPolicyWorker' do + it 'calls Security::SyncProjectPolicyWorker with batching and delays' do expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) + .to receive(:bulk_perform_in_with_contexts) .with( + 1.second, [project.id], arguments_proc: kind_of(Proc), context_proc: kind_of(Proc) @@ -158,10 +159,11 @@ stub_csp_group(nil) end - it 'calls Security::SyncProjectPolicyWorker' do + it 'calls Security::SyncProjectPolicyWorker with batching and delays' do expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) + .to receive(:bulk_perform_in_with_contexts) .with( + 1.second, match_array([project1.id, project2.id]), arguments_proc: kind_of(Proc), context_proc: kind_of(Proc) @@ -193,10 +195,11 @@ let(:event) { policy_updated_event } end - it 'calls Security::SyncProjectPolicyWorker' do + it 'calls Security::SyncProjectPolicyWorker with batching and delays' do expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) + .to receive(:bulk_perform_in_with_contexts) .with( + 1.second, [project.id], arguments_proc: kind_of(Proc), context_proc: kind_of(Proc) @@ -219,10 +222,11 @@ stub_csp_group(nil) end - it 'calls Security::SyncProjectPolicyWorker' do + it 'calls Security::SyncProjectPolicyWorker with batching and delays' do expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) + .to receive(:bulk_perform_in_with_contexts) .with( + 1.second, match_array([project1.id, project2.id]), arguments_proc: kind_of(Proc), context_proc: kind_of(Proc) @@ -244,10 +248,11 @@ } end - it 'calls Security::SyncProjectPolicyWorker' do + it 'calls Security::SyncProjectPolicyWorker with batching and delays' do expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) + .to receive(:bulk_perform_in_with_contexts) .with( + 1.second, [project.id], arguments_proc: kind_of(Proc), context_proc: kind_of(Proc) @@ -266,10 +271,11 @@ } end - it 'calls Security::SyncProjectPolicyWorker' do + it 'calls Security::SyncProjectPolicyWorker with batching and delays' do expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) + .to receive(:bulk_perform_in_with_contexts) .with( + 1.second, [project.id], arguments_proc: kind_of(Proc), context_proc: kind_of(Proc) @@ -355,10 +361,11 @@ it_behaves_like 'clears policy sync state' - it 'calls Security::SyncProjectPolicyWorker with event payload' do + it 'calls Security::SyncProjectPolicyWorker with batching and delays' do expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) + .to receive(:bulk_perform_in_with_contexts) .with( + 1.second, [project.id], arguments_proc: kind_of(Proc), context_proc: kind_of(Proc) @@ -405,4 +412,84 @@ end end end + + describe 'batching and delays for large number of projects' do + let(:policy_created_event) { Security::PolicyCreatedEvent.new(data: { security_policy_id: policy.id }) } + + subject(:handle_event) { described_class.new.handle_event(policy_created_event) } + + context 'when policy_configuration affects more than BATCH_SIZE projects' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:projects) { create_list(:project, 10, namespace: namespace) } + let_it_be(:policy_configuration) do + create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) + end + + let_it_be(:policy) { create(:security_policy, security_orchestration_policy_configuration: policy_configuration) } + + before do + stub_csp_group(nil) + stub_const("#{described_class}::BATCH_SIZE", 3) + end + + it 'batches projects and applies delays' do + # With 10 projects and BATCH_SIZE of 3, we expect 4 batches + expect(::Security::SyncProjectPolicyWorker).to receive(:bulk_perform_in_with_contexts) + .exactly(4).times.and_call_original + + handle_event + end + + it 'applies correct delays to each batch' do + call_count = 0 + expected_delays = [1.second, 2.seconds, 3.seconds, 4.seconds] + + allow(::Security::SyncProjectPolicyWorker).to receive(:bulk_perform_in_with_contexts) do |delay, _batch, **_| + expect(delay).to eq(expected_delays[call_count]) + call_count += 1 + end + + handle_event + end + + it 'distributes projects across batches correctly' do + batches = [] + + allow(::Security::SyncProjectPolicyWorker).to receive(:bulk_perform_in_with_contexts) do |_delay, batch, **_| + batches << batch + end + + handle_event + + expect(batches[0].size).to eq(3) + expect(batches[1].size).to eq(3) + expect(batches[2].size).to eq(3) + expect(batches[3].size).to eq(1) + end + end + + context 'when policy_configuration affects fewer projects than BATCH_SIZE' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:projects) { create_list(:project, 2, namespace: namespace) } + let_it_be(:policy_configuration) do + create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) + end + + let_it_be(:policy) { create(:security_policy, security_orchestration_policy_configuration: policy_configuration) } + + before do + stub_csp_group(nil) + stub_const("#{described_class}::BATCH_SIZE", 3) + end + + it 'creates a single batch with 1 second delay' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_in_with_contexts) + .once + .with(1.second, match_array(projects.map(&:id)), arguments_proc: kind_of(Proc), context_proc: kind_of(Proc)) + + handle_event + end + end + end end -- GitLab From d59066450ea1b506a7aa469d4a6047897bbcf523 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Wed, 17 Dec 2025 12:21:51 +0100 Subject: [PATCH 2/5] Add feature flag and update spec --- ee/app/workers/security/sync_policy_worker.rb | 24 ++- .../security_policies_batched_sync_delay.yml | 10 + .../security/sync_policy_worker_spec.rb | 181 ++++++++++++++++-- 3 files changed, 194 insertions(+), 21 deletions(-) create mode 100644 ee/config/feature_flags/gitlab_com_derisk/security_policies_batched_sync_delay.yml diff --git a/ee/app/workers/security/sync_policy_worker.rb b/ee/app/workers/security/sync_policy_worker.rb index 500aea1425f08a..0fd36b3b351c19 100644 --- a/ee/app/workers/security/sync_policy_worker.rb +++ b/ee/app/workers/security/sync_policy_worker.rb @@ -76,16 +76,26 @@ def sync_policy_for_projects(policy, event_data = {}, event = nil) clear_policy_sync_state(policy.security_orchestration_policy_configuration_id) batch_index = 0 + feature_enabled = Feature.enabled?(:security_policies_batched_sync_delay, + policy.security_policy_management_project) + policy.security_orchestration_policy_configuration.all_project_ids do |project_ids| append_projects_to_sync(policy.security_orchestration_policy_configuration_id, project_ids) - project_ids.each_slice(BATCH_SIZE).with_index do |batch, slice_index| - # Use [1.second, delay].max to ensure delay is always at least 1 second in the future - delay = [1.second, (batch_index + slice_index) * BATCH_DELAY_INTERVAL].max - - ::Security::SyncProjectPolicyWorker.bulk_perform_in_with_contexts( - delay, - batch, + if feature_enabled + project_ids.each_slice(BATCH_SIZE).with_index do |batch, slice_index| + delay = [BATCH_DELAY_INTERVAL, (batch_index + slice_index) * BATCH_DELAY_INTERVAL].max + + ::Security::SyncProjectPolicyWorker.bulk_perform_in_with_contexts(delay, batch, + arguments_proc: ->(project_id) do + [project_id, policy.id, event_data, event_payload.deep_stringify_keys] + end, + context_proc: ->(_) { config_context } + ) + end + else + ::Security::SyncProjectPolicyWorker.bulk_perform_async_with_contexts( + project_ids, arguments_proc: ->(project_id) do [project_id, policy.id, event_data, event_payload.deep_stringify_keys] end, diff --git a/ee/config/feature_flags/gitlab_com_derisk/security_policies_batched_sync_delay.yml b/ee/config/feature_flags/gitlab_com_derisk/security_policies_batched_sync_delay.yml new file mode 100644 index 00000000000000..6db84e93d420db --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/security_policies_batched_sync_delay.yml @@ -0,0 +1,10 @@ +--- +name: security_policies_batched_sync_delay +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/580036 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/216298 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/584386 +milestone: '18.7' +group: group::security policies +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/spec/workers/security/sync_policy_worker_spec.rb b/ee/spec/workers/security/sync_policy_worker_spec.rb index c0fdf4c64582a3..1e3400d0dff18d 100644 --- a/ee/spec/workers/security/sync_policy_worker_spec.rb +++ b/ee/spec/workers/security/sync_policy_worker_spec.rb @@ -123,6 +123,24 @@ handle_event end + context 'when security_policies_batched_sync_delay is disabled' do + before do + stub_feature_flags(security_policies_batched_sync_delay: false) + end + + it 'calls Security::SyncProjectPolicyWorker without delays' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_async_with_contexts) + .with( + [project.id], + arguments_proc: kind_of(Proc), + context_proc: kind_of(Proc) + ) + + handle_event + end + end + it 'calls ComplianceFrameworks::SyncService' do expect(Security::SecurityOrchestrationPolicies::ComplianceFrameworks::SyncService) .to receive(:new) @@ -171,6 +189,24 @@ handle_event end + + context 'when security_policies_batched_sync_delay is disabled' do + before do + stub_feature_flags(security_policies_batched_sync_delay: false) + end + + it 'calls Security::SyncProjectPolicyWorker without delays' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_async_with_contexts) + .with( + match_array([project1.id, project2.id]), + arguments_proc: kind_of(Proc), + context_proc: kind_of(Proc) + ) + + handle_event + end + end end it_behaves_like 'triggers SyncPipelineExecutionPolicyMetadataWorker' @@ -208,6 +244,24 @@ handle_event end + context 'when security_policies_batched_sync_delay is disabled' do + before do + stub_feature_flags(security_policies_batched_sync_delay: false) + end + + it 'calls Security::SyncProjectPolicyWorker without delays' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_async_with_contexts) + .with( + [project.id], + arguments_proc: kind_of(Proc), + context_proc: kind_of(Proc) + ) + + handle_event + end + end + context 'when policy_configuration is scoped to a namespace with multiple projects' do let_it_be(:namespace) { create(:namespace) } let_it_be(:project1) { create(:project, namespace: namespace) } @@ -234,6 +288,24 @@ handle_event end + + context 'when security_policies_batched_sync_delay is disabled' do + before do + stub_feature_flags(security_policies_batched_sync_delay: false) + end + + it 'calls Security::SyncProjectPolicyWorker without delays' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_async_with_contexts) + .with( + match_array([project1.id, project2.id]), + arguments_proc: kind_of(Proc), + context_proc: kind_of(Proc) + ) + + handle_event + end + end end context 'when policy actions is changed' do @@ -260,6 +332,24 @@ handle_event end + + context 'when security_policies_batched_sync_delay is disabled' do + before do + stub_feature_flags(security_policies_batched_sync_delay: false) + end + + it 'calls Security::SyncProjectPolicyWorker without delays' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_async_with_contexts) + .with( + [project.id], + arguments_proc: kind_of(Proc), + context_proc: kind_of(Proc) + ) + + handle_event + end + end end context 'when policy scope changed' do @@ -284,6 +374,24 @@ handle_event end + context 'when security_policies_batched_sync_delay is disabled' do + before do + stub_feature_flags(security_policies_batched_sync_delay: false) + end + + it 'calls Security::SyncProjectPolicyWorker without delays' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_async_with_contexts) + .with( + [project.id], + arguments_proc: kind_of(Proc), + context_proc: kind_of(Proc) + ) + + handle_event + end + end + it 'calls ComplianceFrameworks::SyncService with policy_diff' do expect(Security::SecurityOrchestrationPolicies::ComplianceFrameworks::SyncService) .to receive(:new) @@ -374,6 +482,24 @@ handle_event end + context 'when security_policies_batched_sync_delay is disabled' do + before do + stub_feature_flags(security_policies_batched_sync_delay: false) + end + + it 'calls Security::SyncProjectPolicyWorker without delays' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_async_with_contexts) + .with( + [project.id], + arguments_proc: kind_of(Proc), + context_proc: kind_of(Proc) + ) + + handle_event + end + end + it 'calls ComplianceFrameworks::SyncService' do expect(Security::SecurityOrchestrationPolicies::ComplianceFrameworks::SyncService) .to receive(:new) @@ -425,7 +551,9 @@ create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) end - let_it_be(:policy) { create(:security_policy, security_orchestration_policy_configuration: policy_configuration) } + let_it_be(:policy) do + create(:security_policy, security_orchestration_policy_configuration: policy_configuration) + end before do stub_csp_group(nil) @@ -440,18 +568,6 @@ handle_event end - it 'applies correct delays to each batch' do - call_count = 0 - expected_delays = [1.second, 2.seconds, 3.seconds, 4.seconds] - - allow(::Security::SyncProjectPolicyWorker).to receive(:bulk_perform_in_with_contexts) do |delay, _batch, **_| - expect(delay).to eq(expected_delays[call_count]) - call_count += 1 - end - - handle_event - end - it 'distributes projects across batches correctly' do batches = [] @@ -475,7 +591,9 @@ create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) end - let_it_be(:policy) { create(:security_policy, security_orchestration_policy_configuration: policy_configuration) } + let_it_be(:policy) do + create(:security_policy, security_orchestration_policy_configuration: policy_configuration) + end before do stub_csp_group(nil) @@ -491,5 +609,40 @@ handle_event end end + + context 'when security_policies_batched_sync_delay is disabled' do + before do + stub_feature_flags(security_policies_batched_sync_delay: false) + end + + context 'when policy_configuration affects multiple projects' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:projects) { create_list(:project, 10, namespace: namespace) } + let_it_be(:policy_configuration) do + create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) + end + + let_it_be(:policy) do + create(:security_policy, security_orchestration_policy_configuration: policy_configuration) + end + + before do + stub_csp_group(nil) + end + + it 'calls bulk_perform_async_with_contexts without delays' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_async_with_contexts) + .once + .with( + match_array(projects.map(&:id)), + arguments_proc: kind_of(Proc), + context_proc: kind_of(Proc) + ) + + handle_event + end + end + end end end -- GitLab From 95692952698e363db816ac8accde9d856182299c Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Wed, 17 Dec 2025 13:29:46 +0100 Subject: [PATCH 3/5] Reduce duplicaiton in spec --- .../security/sync_policy_worker_spec.rb | 362 ++++++------------ 1 file changed, 109 insertions(+), 253 deletions(-) diff --git a/ee/spec/workers/security/sync_policy_worker_spec.rb b/ee/spec/workers/security/sync_policy_worker_spec.rb index 1e3400d0dff18d..c9739423ce07d4 100644 --- a/ee/spec/workers/security/sync_policy_worker_spec.rb +++ b/ee/spec/workers/security/sync_policy_worker_spec.rb @@ -83,6 +83,62 @@ end end + shared_examples 'syncs projects with feature flag behavior' do + it 'calls Security::SyncProjectPolicyWorker with batching and delays' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_in_with_contexts) + .with( + 1.second, + project_ids, + arguments_proc: kind_of(Proc), + context_proc: kind_of(Proc) + ) + + handle_event + end + + context 'when security_policies_batched_sync_delay is disabled' do + before do + stub_feature_flags(security_policies_batched_sync_delay: false) + end + + it 'calls Security::SyncProjectPolicyWorker without delays' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_async_with_contexts) + .with( + project_ids, + arguments_proc: kind_of(Proc), + context_proc: kind_of(Proc) + ) + + handle_event + end + end + end + + shared_examples 'batches projects correctly' do |batch_count, batch_sizes| + it 'batches projects and applies delays' do + expect(::Security::SyncProjectPolicyWorker).to receive(:bulk_perform_in_with_contexts) + .exactly(batch_count).times.and_call_original + + handle_event + end + + it 'distributes projects across batches correctly' do + batches = [] + + allow(::Security::SyncProjectPolicyWorker).to receive(:bulk_perform_in_with_contexts) do |_delay, batch, **_| + batches << batch + end + + handle_event + + batch_sizes.each_with_index do |size, index| + expect(batches[index].size).to eq(size) + end + end + end + context 'when event is Security::PolicyDeletedEvent' do let(:policy_deleted_event) do Security::PolicyDeletedEvent.new(data: { security_policy_id: policy.id }) @@ -110,35 +166,10 @@ let(:event) { policy_created_event } end - it 'calls Security::SyncProjectPolicyWorker with batching and delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_in_with_contexts) - .with( - 1.second, - [project.id], - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) + context 'with single project' do + let(:project_ids) { [project.id] } - handle_event - end - - context 'when security_policies_batched_sync_delay is disabled' do - before do - stub_feature_flags(security_policies_batched_sync_delay: false) - end - - it 'calls Security::SyncProjectPolicyWorker without delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) - .with( - [project.id], - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) - - handle_event - end + it_behaves_like 'syncs projects with feature flag behavior' end it 'calls ComplianceFrameworks::SyncService' do @@ -165,6 +196,7 @@ context 'when policy_configuration is scoped to a namespace with multiple projects' do let_it_be(:namespace) { create(:namespace) } + let(:project_ids) { [project1.id, project2.id] } let_it_be(:project1) { create(:project, namespace: namespace) } let_it_be(:project2) { create(:project, namespace: namespace) } let_it_be(:policy_configuration) do @@ -177,36 +209,7 @@ stub_csp_group(nil) end - it 'calls Security::SyncProjectPolicyWorker with batching and delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_in_with_contexts) - .with( - 1.second, - match_array([project1.id, project2.id]), - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) - - handle_event - end - - context 'when security_policies_batched_sync_delay is disabled' do - before do - stub_feature_flags(security_policies_batched_sync_delay: false) - end - - it 'calls Security::SyncProjectPolicyWorker without delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) - .with( - match_array([project1.id, project2.id]), - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) - - handle_event - end - end + it_behaves_like 'syncs projects with feature flag behavior' end it_behaves_like 'triggers SyncPipelineExecutionPolicyMetadataWorker' @@ -231,39 +234,15 @@ let(:event) { policy_updated_event } end - it 'calls Security::SyncProjectPolicyWorker with batching and delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_in_with_contexts) - .with( - 1.second, - [project.id], - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) - - handle_event - end - - context 'when security_policies_batched_sync_delay is disabled' do - before do - stub_feature_flags(security_policies_batched_sync_delay: false) - end - - it 'calls Security::SyncProjectPolicyWorker without delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) - .with( - [project.id], - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) + context 'with single project' do + let(:project_ids) { [project.id] } - handle_event - end + it_behaves_like 'syncs projects with feature flag behavior' end context 'when policy_configuration is scoped to a namespace with multiple projects' do let_it_be(:namespace) { create(:namespace) } + let(:project_ids) { [project1.id, project2.id] } let_it_be(:project1) { create(:project, namespace: namespace) } let_it_be(:project2) { create(:project, namespace: namespace) } let_it_be(:policy_configuration) do @@ -276,36 +255,7 @@ stub_csp_group(nil) end - it 'calls Security::SyncProjectPolicyWorker with batching and delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_in_with_contexts) - .with( - 1.second, - match_array([project1.id, project2.id]), - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) - - handle_event - end - - context 'when security_policies_batched_sync_delay is disabled' do - before do - stub_feature_flags(security_policies_batched_sync_delay: false) - end - - it 'calls Security::SyncProjectPolicyWorker without delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) - .with( - match_array([project1.id, project2.id]), - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) - - handle_event - end - end + it_behaves_like 'syncs projects with feature flag behavior' end context 'when policy actions is changed' do @@ -320,36 +270,9 @@ } end - it 'calls Security::SyncProjectPolicyWorker with batching and delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_in_with_contexts) - .with( - 1.second, - [project.id], - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) + let(:project_ids) { [project.id] } - handle_event - end - - context 'when security_policies_batched_sync_delay is disabled' do - before do - stub_feature_flags(security_policies_batched_sync_delay: false) - end - - it 'calls Security::SyncProjectPolicyWorker without delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) - .with( - [project.id], - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) - - handle_event - end - end + it_behaves_like 'syncs projects with feature flag behavior' end context 'when policy scope changed' do @@ -361,36 +284,9 @@ } end - it 'calls Security::SyncProjectPolicyWorker with batching and delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_in_with_contexts) - .with( - 1.second, - [project.id], - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) - - handle_event - end - - context 'when security_policies_batched_sync_delay is disabled' do - before do - stub_feature_flags(security_policies_batched_sync_delay: false) - end - - it 'calls Security::SyncProjectPolicyWorker without delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) - .with( - [project.id], - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) + let(:project_ids) { [project.id] } - handle_event - end - end + it_behaves_like 'syncs projects with feature flag behavior' it 'calls ComplianceFrameworks::SyncService with policy_diff' do expect(Security::SecurityOrchestrationPolicies::ComplianceFrameworks::SyncService) @@ -469,35 +365,10 @@ it_behaves_like 'clears policy sync state' - it 'calls Security::SyncProjectPolicyWorker with batching and delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_in_with_contexts) - .with( - 1.second, - [project.id], - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) + context 'with single project' do + let(:project_ids) { [project.id] } - handle_event - end - - context 'when security_policies_batched_sync_delay is disabled' do - before do - stub_feature_flags(security_policies_batched_sync_delay: false) - end - - it 'calls Security::SyncProjectPolicyWorker without delays' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_async_with_contexts) - .with( - [project.id], - arguments_proc: kind_of(Proc), - context_proc: kind_of(Proc) - ) - - handle_event - end + it_behaves_like 'syncs projects with feature flag behavior' end it 'calls ComplianceFrameworks::SyncService' do @@ -544,73 +415,58 @@ subject(:handle_event) { described_class.new.handle_event(policy_created_event) } - context 'when policy_configuration affects more than BATCH_SIZE projects' do - let_it_be(:namespace) { create(:namespace) } - let_it_be(:projects) { create_list(:project, 10, namespace: namespace) } - let_it_be(:policy_configuration) do - create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) - end - - let_it_be(:policy) do - create(:security_policy, security_orchestration_policy_configuration: policy_configuration) - end - + context 'when feature flag is enabled' do before do - stub_csp_group(nil) - stub_const("#{described_class}::BATCH_SIZE", 3) - end - - it 'batches projects and applies delays' do - # With 10 projects and BATCH_SIZE of 3, we expect 4 batches - expect(::Security::SyncProjectPolicyWorker).to receive(:bulk_perform_in_with_contexts) - .exactly(4).times.and_call_original - - handle_event + stub_feature_flags(security_policies_batched_sync_delay: true) end - it 'distributes projects across batches correctly' do - batches = [] + context 'when policy_configuration affects more than BATCH_SIZE projects' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:projects) { create_list(:project, 10, namespace: namespace) } + let_it_be(:policy_configuration) do + create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) + end - allow(::Security::SyncProjectPolicyWorker).to receive(:bulk_perform_in_with_contexts) do |_delay, batch, **_| - batches << batch + let_it_be(:policy) do + create(:security_policy, security_orchestration_policy_configuration: policy_configuration) end - handle_event + before do + stub_csp_group(nil) + stub_const("#{described_class}::BATCH_SIZE", 3) + end - expect(batches[0].size).to eq(3) - expect(batches[1].size).to eq(3) - expect(batches[2].size).to eq(3) - expect(batches[3].size).to eq(1) + it_behaves_like 'batches projects correctly', 4, [3, 3, 3, 1] end - end - context 'when policy_configuration affects fewer projects than BATCH_SIZE' do - let_it_be(:namespace) { create(:namespace) } - let_it_be(:projects) { create_list(:project, 2, namespace: namespace) } - let_it_be(:policy_configuration) do - create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) - end + context 'when policy_configuration affects fewer projects than BATCH_SIZE' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:projects) { create_list(:project, 2, namespace: namespace) } + let_it_be(:policy_configuration) do + create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) + end - let_it_be(:policy) do - create(:security_policy, security_orchestration_policy_configuration: policy_configuration) - end + let_it_be(:policy) do + create(:security_policy, security_orchestration_policy_configuration: policy_configuration) + end - before do - stub_csp_group(nil) - stub_const("#{described_class}::BATCH_SIZE", 3) - end + before do + stub_csp_group(nil) + stub_const("#{described_class}::BATCH_SIZE", 3) + end - it 'creates a single batch with 1 second delay' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_in_with_contexts) - .once - .with(1.second, match_array(projects.map(&:id)), arguments_proc: kind_of(Proc), context_proc: kind_of(Proc)) + it 'creates a single batch with 1 second delay' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_in_with_contexts) + .once + .with(1.second, match_array(projects.map(&:id)), arguments_proc: kind_of(Proc), context_proc: kind_of(Proc)) - handle_event + handle_event + end end end - context 'when security_policies_batched_sync_delay is disabled' do + context 'when feature flag is disabled' do before do stub_feature_flags(security_policies_batched_sync_delay: false) end -- GitLab From 50096bd26d397f2c9cdf724d3ac6ffa53dd1f7ce Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Thu, 18 Dec 2025 14:11:41 +0100 Subject: [PATCH 4/5] Update milestone and refactor spec --- .../security_policies_batched_sync_delay.yml | 2 +- .../security/sync_policy_worker_spec.rb | 70 +++++++++---------- 2 files changed, 33 insertions(+), 39 deletions(-) diff --git a/ee/config/feature_flags/gitlab_com_derisk/security_policies_batched_sync_delay.yml b/ee/config/feature_flags/gitlab_com_derisk/security_policies_batched_sync_delay.yml index 6db84e93d420db..0b325716f2aaeb 100644 --- a/ee/config/feature_flags/gitlab_com_derisk/security_policies_batched_sync_delay.yml +++ b/ee/config/feature_flags/gitlab_com_derisk/security_policies_batched_sync_delay.yml @@ -4,7 +4,7 @@ description: feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/580036 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/216298 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/584386 -milestone: '18.7' +milestone: '18.8' group: group::security policies type: gitlab_com_derisk default_enabled: false diff --git a/ee/spec/workers/security/sync_policy_worker_spec.rb b/ee/spec/workers/security/sync_policy_worker_spec.rb index c9739423ce07d4..513d3dbcb4c46b 100644 --- a/ee/spec/workers/security/sync_policy_worker_spec.rb +++ b/ee/spec/workers/security/sync_policy_worker_spec.rb @@ -415,54 +415,48 @@ subject(:handle_event) { described_class.new.handle_event(policy_created_event) } - context 'when feature flag is enabled' do - before do - stub_feature_flags(security_policies_batched_sync_delay: true) + context 'when policy_configuration affects more than BATCH_SIZE projects' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:projects) { create_list(:project, 10, namespace: namespace) } + let_it_be(:policy_configuration) do + create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) end - context 'when policy_configuration affects more than BATCH_SIZE projects' do - let_it_be(:namespace) { create(:namespace) } - let_it_be(:projects) { create_list(:project, 10, namespace: namespace) } - let_it_be(:policy_configuration) do - create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) - end + let_it_be(:policy) do + create(:security_policy, security_orchestration_policy_configuration: policy_configuration) + end - let_it_be(:policy) do - create(:security_policy, security_orchestration_policy_configuration: policy_configuration) - end + before do + stub_csp_group(nil) + stub_const("#{described_class}::BATCH_SIZE", 3) + end - before do - stub_csp_group(nil) - stub_const("#{described_class}::BATCH_SIZE", 3) - end + it_behaves_like 'batches projects correctly', 4, [3, 3, 3, 1] + end - it_behaves_like 'batches projects correctly', 4, [3, 3, 3, 1] + context 'when policy_configuration affects fewer projects than BATCH_SIZE' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:projects) { create_list(:project, 2, namespace: namespace) } + let_it_be(:policy_configuration) do + create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) end - context 'when policy_configuration affects fewer projects than BATCH_SIZE' do - let_it_be(:namespace) { create(:namespace) } - let_it_be(:projects) { create_list(:project, 2, namespace: namespace) } - let_it_be(:policy_configuration) do - create(:security_orchestration_policy_configuration, namespace: namespace, project: nil) - end - - let_it_be(:policy) do - create(:security_policy, security_orchestration_policy_configuration: policy_configuration) - end + let_it_be(:policy) do + create(:security_policy, security_orchestration_policy_configuration: policy_configuration) + end - before do - stub_csp_group(nil) - stub_const("#{described_class}::BATCH_SIZE", 3) - end + before do + stub_csp_group(nil) + stub_const("#{described_class}::BATCH_SIZE", 3) + end - it 'creates a single batch with 1 second delay' do - expect(::Security::SyncProjectPolicyWorker) - .to receive(:bulk_perform_in_with_contexts) - .once - .with(1.second, match_array(projects.map(&:id)), arguments_proc: kind_of(Proc), context_proc: kind_of(Proc)) + it 'creates a single batch with 1 second delay' do + expect(::Security::SyncProjectPolicyWorker) + .to receive(:bulk_perform_in_with_contexts) + .once + .with(1.second, match_array(projects.map(&:id)), arguments_proc: kind_of(Proc), context_proc: kind_of(Proc)) - handle_event - end + handle_event end end -- GitLab From 2cdaa3d759af2ef1ce968a0ec9430a53e8e476f4 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Thu, 18 Dec 2025 18:05:25 +0100 Subject: [PATCH 5/5] Fix flaky spec --- ee/spec/workers/security/sync_policy_worker_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/spec/workers/security/sync_policy_worker_spec.rb b/ee/spec/workers/security/sync_policy_worker_spec.rb index 513d3dbcb4c46b..83e682c6b9e791 100644 --- a/ee/spec/workers/security/sync_policy_worker_spec.rb +++ b/ee/spec/workers/security/sync_policy_worker_spec.rb @@ -89,7 +89,7 @@ .to receive(:bulk_perform_in_with_contexts) .with( 1.second, - project_ids, + match_array(project_ids), arguments_proc: kind_of(Proc), context_proc: kind_of(Proc) ) @@ -106,7 +106,7 @@ expect(::Security::SyncProjectPolicyWorker) .to receive(:bulk_perform_async_with_contexts) .with( - project_ids, + match_array(project_ids), arguments_proc: kind_of(Proc), context_proc: kind_of(Proc) ) -- GitLab