diff --git a/app/workers/concerns/security/orchestration_configuration_bot_management_for_namespaces.rb b/app/workers/concerns/security/orchestration_configuration_bot_management_for_namespaces.rb
new file mode 100644
index 0000000000000000000000000000000000000000..72e527f478c10c52f282773b4e9df86cea5ca6c9
--- /dev/null
+++ b/app/workers/concerns/security/orchestration_configuration_bot_management_for_namespaces.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+module Security
+ module OrchestrationConfigurationBotManagementForNamespaces
+ extend ActiveSupport::Concern
+
+ PROJECTS_BATCH_SYNC_DELAY = 1.second
+
+ included do
+ feature_category :security_policy_management
+ urgency :low
+ concurrency_limit -> { 200 }
+ end
+
+ def perform(namespace_id, current_user_id)
+ namespace = Namespace.find_by_id(namespace_id)
+ return unless namespace
+
+ return unless User.id_exists?(current_user_id)
+
+ project_ids = namespace.security_orchestration_policy_configuration.all_project_ids
+
+ worker.bulk_perform_in_with_contexts(
+ PROJECTS_BATCH_SYNC_DELAY,
+ project_ids,
+ arguments_proc: ->(project_id) { [project_id, current_user_id] },
+ context_proc: ->(namespace) { { namespace: namespace } }
+ )
+
+ after_perform(namespace, current_user_id)
+ end
+
+ def after_perform(namespace, current_user_id); end
+ end
+end
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index 582d299ffc2ab7b600f753ccd190583d639e38e1..cc1d6edf9d8f2198f71d63844b8cb414e08c26d3 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -859,6 +859,8 @@
- 1
- - security_orchestration_configuration_create_bot
- 1
+- - security_orchestration_configuration_create_bot_for_namespace
+ - 1
- - security_orchestration_configuration_remove_bot
- 1
- - security_orchestration_configuration_remove_bot_for_namespace
diff --git a/ee/app/services/security/orchestration/assign_service.rb b/ee/app/services/security/orchestration/assign_service.rb
index a40b6dd10ec58aafa35cd8e9f1022aeb069f96fa..3e6d30d314b02fc6348fe0f79b7674a29f643e23 100644
--- a/ee/app/services/security/orchestration/assign_service.rb
+++ b/ee/app/services/security/orchestration/assign_service.rb
@@ -114,6 +114,14 @@ def policy_project_id
def create_security_policy_project_bot
if container.is_a?(Project)
Security::OrchestrationConfigurationCreateBotWorker.perform_async(container.id, current_user.id)
+ else
+ create_bot_for_namespace
+ end
+ end
+
+ def create_bot_for_namespace
+ if Feature.enabled?(:security_policy_bot_worker, container)
+ Security::OrchestrationConfigurationCreateBotForNamespaceWorker.perform_async(container.id, current_user.id)
else
container.security_orchestration_policy_configuration.all_project_ids.each do |project_id|
Security::OrchestrationConfigurationCreateBotWorker.perform_async(project_id, current_user.id)
diff --git a/ee/app/services/security/orchestration/unassign_service.rb b/ee/app/services/security/orchestration/unassign_service.rb
index 6dfd081621c3041e25be5a8de874b1586fb541f1..3e9d322f06a1e938ef7ab72e34e7d34e79b8c943 100644
--- a/ee/app/services/security/orchestration/unassign_service.rb
+++ b/ee/app/services/security/orchestration/unassign_service.rb
@@ -3,6 +3,8 @@
module Security
module Orchestration
class UnassignService < ::BaseContainerService
+ include Gitlab::Utils::StrongMemoize
+
def execute(delete_bot: true)
return error(_('Policy project doesn\'t exist')) unless security_orchestration_policy_configuration
@@ -10,7 +12,9 @@ def execute(delete_bot: true)
remove_bot(security_orchestration_policy_configuration) if delete_bot
- delete_configuration(security_orchestration_policy_configuration, old_policy_project)
+ delete_configuration(security_orchestration_policy_configuration, old_policy_project) if delete_configuration?
+
+ success
end
private
@@ -20,8 +24,6 @@ def execute(delete_bot: true)
def delete_configuration(configuration, old_policy_project)
Security::DeleteOrchestrationConfigurationWorker.perform_async(
configuration.id, current_user.id, old_policy_project.id)
-
- success
end
def success
@@ -33,7 +35,7 @@ def error(message)
end
def remove_bot(security_orchestration_policy_configuration)
- if container.is_a?(Project)
+ if project?
Security::OrchestrationConfigurationRemoveBotWorker.perform_async(container.id, current_user.id)
else
remove_bot_for_namespace(security_orchestration_policy_configuration)
@@ -41,7 +43,7 @@ def remove_bot(security_orchestration_policy_configuration)
end
def remove_bot_for_namespace(security_orchestration_policy_configuration)
- if Feature.enabled?(:security_policy_bot_worker, container)
+ if namespace_worker_enabled?
Security::OrchestrationConfigurationRemoveBotForNamespaceWorker.perform_async(container.id, current_user.id)
else
security_orchestration_policy_configuration.all_project_ids.each do |project_id|
@@ -49,6 +51,20 @@ def remove_bot_for_namespace(security_orchestration_policy_configuration)
end
end
end
+
+ def namespace_worker_enabled?
+ Feature.enabled?(:security_policy_bot_worker, container)
+ end
+ strong_memoize_attr :namespace_worker_enabled?
+
+ def project?
+ container.is_a?(Project)
+ end
+ strong_memoize_attr :project?
+
+ def delete_configuration?
+ project? || !namespace_worker_enabled?
+ end
end
end
end
diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml
index 815e36b6d88e8f7b985c675a4a574684228665e9..5e69d0b3a399313d4fb2f2bc84deb1fd29c325c1 100644
--- a/ee/app/workers/all_queues.yml
+++ b/ee/app/workers/all_queues.yml
@@ -3043,6 +3043,16 @@
:idempotent: true
:tags: []
:queue_namespace:
+- :name: security_orchestration_configuration_create_bot_for_namespace
+ :worker_name: Security::OrchestrationConfigurationCreateBotForNamespaceWorker
+ :feature_category: :security_policy_management
+ :has_external_dependencies: false
+ :urgency: :low
+ :resource_boundary: :unknown
+ :weight: 1
+ :idempotent: true
+ :tags: []
+ :queue_namespace:
- :name: security_orchestration_configuration_remove_bot
:worker_name: Security::OrchestrationConfigurationRemoveBotWorker
:feature_category: :security_policy_management
diff --git a/ee/app/workers/security/orchestration_configuration_create_bot_for_namespace_worker.rb b/ee/app/workers/security/orchestration_configuration_create_bot_for_namespace_worker.rb
new file mode 100644
index 0000000000000000000000000000000000000000..dd4672ae67ccd6930a7853d87085ee403ca0a677
--- /dev/null
+++ b/ee/app/workers/security/orchestration_configuration_create_bot_for_namespace_worker.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module Security
+ class OrchestrationConfigurationCreateBotForNamespaceWorker
+ include ApplicationWorker
+ include Security::OrchestrationConfigurationBotManagementForNamespaces
+
+ data_consistency :sticky
+ idempotent!
+
+ def worker
+ Security::OrchestrationConfigurationCreateBotWorker
+ end
+ end
+end
diff --git a/ee/app/workers/security/orchestration_configuration_remove_bot_for_namespace_worker.rb b/ee/app/workers/security/orchestration_configuration_remove_bot_for_namespace_worker.rb
index 3da5ca9a36457a6b4363e0ffc43a74fe820d0dd4..3af4ed813f8f3e8cbc2c88b73cce87ccb0014086 100644
--- a/ee/app/workers/security/orchestration_configuration_remove_bot_for_namespace_worker.rb
+++ b/ee/app/workers/security/orchestration_configuration_remove_bot_for_namespace_worker.rb
@@ -3,30 +3,35 @@
module Security
class OrchestrationConfigurationRemoveBotForNamespaceWorker
include ApplicationWorker
+ include Security::OrchestrationConfigurationBotManagementForNamespaces
- PROJECTS_BATCH_SYNC_DELAY = 1.second
-
- feature_category :security_policy_management
data_consistency :sticky
- urgency :low
idempotent!
- concurrency_limit -> { 200 }
+ def worker
+ Security::OrchestrationConfigurationRemoveBotWorker
+ end
+
+ def after_perform(namespace, current_user_id)
+ delete_configuration(namespace, current_user_id)
+ end
- def perform(namespace_id, current_user_id)
- namespace = Namespace.find_by_id(namespace_id)
- return unless namespace
+ private
- return unless User.id_exists?(current_user_id)
+ def delete_configuration(namespace, current_user_id)
+ old_policy_project = old_policy_project(namespace)
+ configuration = configuration(namespace)
- project_ids = namespace.security_orchestration_policy_configuration.all_project_ids
+ Security::DeleteOrchestrationConfigurationWorker.perform_async(
+ configuration.id, current_user_id, old_policy_project.id)
+ end
+
+ def configuration(namespace)
+ namespace.security_orchestration_policy_configuration
+ end
- Security::OrchestrationConfigurationRemoveBotWorker.bulk_perform_in_with_contexts(
- PROJECTS_BATCH_SYNC_DELAY,
- project_ids,
- arguments_proc: ->(project_id) { [project_id, current_user_id] },
- context_proc: ->(namespace) { { namespace: namespace } }
- )
+ def old_policy_project(namespace)
+ namespace.security_orchestration_policy_configuration.security_policy_management_project
end
end
end
diff --git a/ee/spec/services/security/orchestration/assign_service_spec.rb b/ee/spec/services/security/orchestration/assign_service_spec.rb
index 610a5252a04ac4041d5a5cabb05d4ceb29f4d1c7..df764d9314f20a42b1ba667bc2d0185cbf65291d 100644
--- a/ee/spec/services/security/orchestration/assign_service_spec.rb
+++ b/ee/spec/services/security/orchestration/assign_service_spec.rb
@@ -281,8 +281,29 @@
let(:another_container) { another_namespace }
it_behaves_like 'executes assign service'
- it_behaves_like 'triggers bot user create worker' do
- let!(:expected_projects) { create_list(:project, 2, group: container) }
+
+ context 'when the `security_policy_bot_worker` feature flag is disabled' do
+ before do
+ stub_feature_flags(security_policy_bot_worker: false)
+ end
+
+ it_behaves_like 'triggers bot user create worker' do
+ let!(:expected_projects) { create_list(:project, 2, group: container) }
+ end
+ end
+
+ context 'when the `security_policy_bot_worker` feature flag is enabled' do
+ context 'with owner access' do
+ before do
+ container.add_owner(current_user)
+ end
+
+ it 'triggers the project bot user create for namespace worker' do
+ expect(Security::OrchestrationConfigurationCreateBotForNamespaceWorker).to receive(:perform_async).with(container.id, current_user.id)
+
+ expect(service).to be_success
+ end
+ end
end
describe 'redundant policy configurations within namespace' do
diff --git a/ee/spec/services/security/orchestration/unassign_service_spec.rb b/ee/spec/services/security/orchestration/unassign_service_spec.rb
index b85a6086ec2796cd67f7c93d4d93b04518c45df9..87cf035e5385585be0d7b7428781125a135274c6 100644
--- a/ee/spec/services/security/orchestration/unassign_service_spec.rb
+++ b/ee/spec/services/security/orchestration/unassign_service_spec.rb
@@ -173,19 +173,18 @@
end
end
- it 'unassigns policy project and enqueues OrchestrationConfigurationRemoveBotForNamespaceWorker',
+ it 'does not unassign the policy project and enqueues OrchestrationConfigurationRemoveBotForNamespaceWorker',
:aggregate_failures do
expect(Security::OrchestrationConfigurationRemoveBotForNamespaceWorker).to receive(:perform_async).with(
container.id, current_user.id)
expect(result).to be_success
+
exists = Security::OrchestrationPolicyConfiguration.exists?(
container.security_orchestration_policy_configuration.id)
- expect(exists).to be(false)
+ expect(exists).to be(true)
end
end
-
- it_behaves_like 'unassigns policy project'
end
end
end
diff --git a/ee/spec/support/shared_examples/workers/security/orchestration_configuration_bot_management_for_namespace_shared_examples.rb b/ee/spec/support/shared_examples/workers/security/orchestration_configuration_bot_management_for_namespace_shared_examples.rb
new file mode 100644
index 0000000000000000000000000000000000000000..3faab12962a0eb9c7dde71e3a71034db3e6dff79
--- /dev/null
+++ b/ee/spec/support/shared_examples/workers/security/orchestration_configuration_bot_management_for_namespace_shared_examples.rb
@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'bot management worker examples' do
+ describe '#perform' do
+ let_it_be(:namespace, reload: true) { create(:group, :with_security_orchestration_policy_configuration) }
+ let_it_be(:namespace_projects) { create_list(:project, 2, group: namespace) }
+ let_it_be(:user) { create(:user) }
+ let(:current_user_id) { nil }
+ let(:namespace_project_ids) { namespace_projects.map(&:id) }
+
+ subject(:run_worker) { described_class.new.perform(namespace_id, current_user_id) }
+
+ before_all do
+ namespace_projects.each do |project|
+ project.add_owner(user)
+ end
+ end
+
+ shared_examples_for 'worker exits without error' do
+ it 'does not enqueues additional workers' do
+ expect(management_worker).not_to receive(:bulk_perform_in_with_contexts)
+
+ run_worker
+ end
+
+ it 'exits without error' do
+ expect { run_worker }.not_to raise_error
+ end
+ end
+
+ context 'with invalid namespace_id' do
+ let(:namespace_id) { non_existing_record_id }
+
+ it_behaves_like 'worker exits without error'
+ end
+
+ context 'with valid project_id' do
+ let(:namespace_id) { namespace.id }
+
+ context 'when user with given current_user_id does not exist' do
+ let(:current_user_id) { non_existing_record_id }
+
+ it_behaves_like 'worker exits without error'
+ end
+
+ context 'when current user is provided' do
+ let(:current_user_id) { user.id }
+
+ it 'enqueues a worker for each projects', :aggregate_failures do
+ expect(management_worker)
+ .to receive(:bulk_perform_in_with_contexts)
+ .with(kind_of(Integer), namespace_project_ids,
+ { arguments_proc: kind_of(Proc), context_proc: kind_of(Proc) })
+
+ run_worker
+ end
+
+ it_behaves_like 'an idempotent worker' do
+ let(:job_args) { [namespace_id, current_user_id] }
+ end
+ end
+ end
+ end
+end
diff --git a/ee/spec/workers/security/orchestration_configuration_create_bot_for_namespace_worker_spec.rb b/ee/spec/workers/security/orchestration_configuration_create_bot_for_namespace_worker_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..acfad466524a475f68ce232d5c1ebd0f295672fa
--- /dev/null
+++ b/ee/spec/workers/security/orchestration_configuration_create_bot_for_namespace_worker_spec.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::OrchestrationConfigurationCreateBotForNamespaceWorker, feature_category: :security_policy_management do
+ let(:management_worker) { Security::OrchestrationConfigurationCreateBotWorker }
+
+ it_behaves_like 'bot management worker examples'
+end
diff --git a/ee/spec/workers/security/orchestration_configuration_remove_bot_for_namespace_worker_spec.rb b/ee/spec/workers/security/orchestration_configuration_remove_bot_for_namespace_worker_spec.rb
index fafc30c70861ad318db0b84404359538b4e74af7..1c2775af73910861148e4036f32b20fdcda317cd 100644
--- a/ee/spec/workers/security/orchestration_configuration_remove_bot_for_namespace_worker_spec.rb
+++ b/ee/spec/workers/security/orchestration_configuration_remove_bot_for_namespace_worker_spec.rb
@@ -3,62 +3,26 @@
require 'spec_helper'
RSpec.describe Security::OrchestrationConfigurationRemoveBotForNamespaceWorker, feature_category: :security_policy_management do
- describe '#perform' do
- let_it_be(:namespace, reload: true) { create(:group, :with_security_orchestration_policy_configuration) }
- let_it_be(:namespace_projects) { create_list(:project, 2, group: namespace) }
- let_it_be(:user) { create(:user) }
- let(:current_user_id) { nil }
- let(:namespace_project_ids) { namespace_projects.map(&:id) }
+ let(:management_worker) { Security::OrchestrationConfigurationRemoveBotWorker }
- subject(:run_worker) { described_class.new.perform(namespace_id, current_user_id) }
-
- before_all do
- namespace_projects.each do |project|
- project.add_owner(user)
- end
- end
-
- shared_examples_for 'worker exits without error' do
- it 'does not enqueues the Security::OrchestrationConfigurationRemoveBotWorker' do
- expect(Security::OrchestrationConfigurationRemoveBotWorker).not_to receive(:bulk_perform_in_with_contexts)
-
- run_worker
- end
-
- it 'exits without error' do
- expect { run_worker }.not_to raise_error
- end
- end
-
- context 'with invalid namespace_id' do
- let(:namespace_id) { non_existing_record_id }
-
- it_behaves_like 'worker exits without error'
- end
+ it_behaves_like 'bot management worker examples'
+ describe 'delete_configuration' do
context 'with valid project_id' do
+ let_it_be(:namespace, reload: true) { create(:group, :with_security_orchestration_policy_configuration) }
let(:namespace_id) { namespace.id }
- context 'when user with given current_user_id does not exist' do
- let(:current_user_id) { non_existing_record_id }
-
- it_behaves_like 'worker exits without error'
- end
-
context 'when current user is provided' do
- let(:current_user_id) { user.id }
+ let_it_be(:current_user) { create(:user) }
+ let(:current_user_id) { current_user.id }
- it 'enqueues the OrchestrationConfigurationRemoveBotWorker for all projects', :aggregate_failures do
- expect(Security::OrchestrationConfigurationRemoveBotWorker)
- .to receive(:bulk_perform_in_with_contexts)
- .with(kind_of(Integer), namespace_project_ids,
- { arguments_proc: kind_of(Proc), context_proc: kind_of(Proc) })
+ let(:policy_configuration) { namespace.security_orchestration_policy_configuration }
- run_worker
- end
+ it 'enqueues for deletion' do
+ expect(Security::DeleteOrchestrationConfigurationWorker).to receive(:perform_async).with(
+ policy_configuration.id, current_user_id, policy_configuration.security_policy_management_project.id)
- it_behaves_like 'an idempotent worker' do
- let(:job_args) { [namespace_id, current_user_id] }
+ described_class.new.perform(namespace_id, current_user_id)
end
end
end
diff --git a/ee/spec/workers/security/unassign_redundant_policy_configurations_worker_spec.rb b/ee/spec/workers/security/unassign_redundant_policy_configurations_worker_spec.rb
index c5d936b9f5f5f4ad058cae4bb12d1d5c20c80195..08c675cd9bdf73b1a8967908f7d40e05e6da9d06 100644
--- a/ee/spec/workers/security/unassign_redundant_policy_configurations_worker_spec.rb
+++ b/ee/spec/workers/security/unassign_redundant_policy_configurations_worker_spec.rb
@@ -50,28 +50,46 @@
create(:project_member, user: security_policy_bot, project: project)
end
- it 'does not unassign top-level group' do
- expect { execute }.not_to change {
- Security::OrchestrationPolicyConfiguration.exists?(policy_configuration_a.id)
- }.from(true)
- end
+ context 'when the `security_policy_bot_worker` feature flag is disabled' do
+ before do
+ stub_feature_flags(security_policy_bot_worker: false)
+ end
- it 'unassigns projects and groups with redundant policy projects' do
- execute
+ it 'does not unassign top-level group' do
+ expect { execute }.not_to change {
+ Security::OrchestrationPolicyConfiguration.exists?(policy_configuration_a.id)
+ }.from(true)
+ end
+
+ it 'unassigns projects and groups with redundant policy projects' do
+ execute
- [policy_configuration_b, policy_configuration_c].each do |config|
- expect(Security::OrchestrationPolicyConfiguration.exists?(config.id)).to be(false)
+ [policy_configuration_b, policy_configuration_c].each do |config|
+ expect(Security::OrchestrationPolicyConfiguration.exists?(config.id)).to be(false)
+ end
end
- end
- it 'does not unassign unrelated projects' do
- expect { execute }.not_to change {
- Security::OrchestrationPolicyConfiguration.exists?(other_policy_configuration.id)
- }.from(true)
+ it 'does not unassign unrelated projects' do
+ expect { execute }.not_to change {
+ Security::OrchestrationPolicyConfiguration.exists?(other_policy_configuration.id)
+ }.from(true)
+ end
+
+ it 'keeps bot users' do
+ expect { execute }.not_to change { project.security_policy_bot }.from(security_policy_bot)
+ end
end
- it 'keeps bot users' do
- expect { execute }.not_to change { project.security_policy_bot }.from(security_policy_bot)
+ it 'delegates the unassign to Security::Orchestration::UnassignService' do
+ [subgroup_b, project].each do |container|
+ expect_next_instance_of(::Security::Orchestration::UnassignService,
+ container: container,
+ current_user: user) do |service|
+ expect(service).to receive(:execute).with(delete_bot: false).once.and_call_original
+ end
+ end
+
+ execute
end
end