diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index a7182e334180c67941d677461929b10acacf33cf..ddb3830d37bbb924e0dbc1c238385a07cf63a086 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -753,6 +753,8 @@ - 1 - - vulnerabilities_mark_dropped_as_resolved - 1 +- - vulnerabilities_process_transfer_events + - 1 - - vulnerabilities_remove_all_vulnerabilities - 1 - - vulnerabilities_statistics_adjustment diff --git a/ee/app/services/ee/projects/transfer_service.rb b/ee/app/services/ee/projects/transfer_service.rb index f36677dbf399760129061394c9d9cba55b27c334..539712e34f289e695820f844d60c082c0cf7822a 100644 --- a/ee/app/services/ee/projects/transfer_service.rb +++ b/ee/app/services/ee/projects/transfer_service.rb @@ -67,6 +67,7 @@ def delete_compliance_framework_setting end def schedule_vulnerability_reads_update + return if ::Feature.enabled?(:update_vuln_reads_traversal_ids_via_event, project, type: :gitlab_com_derisk) return unless project.project_setting&.has_vulnerabilities? Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker.perform_async(project.id) diff --git a/ee/app/services/vulnerabilities/update_namespace_ids_of_vulnerability_reads_service.rb b/ee/app/services/vulnerabilities/update_namespace_ids_of_vulnerability_reads_service.rb index 99baee1afbf6c07dc99c4f1807abcfeda20d7a98..75c9c8f521738f3584aaed7bbccdd846bf055359 100644 --- a/ee/app/services/vulnerabilities/update_namespace_ids_of_vulnerability_reads_service.rb +++ b/ee/app/services/vulnerabilities/update_namespace_ids_of_vulnerability_reads_service.rb @@ -5,7 +5,6 @@ class UpdateNamespaceIdsOfVulnerabilityReadsService include Gitlab::ExclusiveLeaseHelpers BATCH_SIZE = 100 - LEASE_KEY = 'update_vulnerability_reads_namespace_id' LEASE_TTL = 5.minutes LEASE_TRY_AFTER = 3.seconds @@ -20,20 +19,24 @@ def initialize(project_id) def execute return unless project - in_lock(LEASE_KEY, ttl: LEASE_TTL, sleep_sec: LEASE_TRY_AFTER) { update_vulnerability_reads } + in_lock(lease_key, ttl: LEASE_TTL, sleep_sec: LEASE_TRY_AFTER) { update_vulnerability_reads } end private attr_reader :project_id + def lease_key + "update_vulnerability_reads_namespace_id_and_traversal_ids:projects:#{project_id}" + end + def project @project ||= Project.find_by_id(project_id) end def update_vulnerability_reads project.vulnerability_reads.each_batch(of: BATCH_SIZE) do |batch| - batch.update_all(namespace_id: project.namespace_id) + batch.update_all(namespace_id: project.namespace_id, traversal_ids: project.namespace.traversal_ids) end end end diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index 18f6fff4d67a5db13dc056a130d580f04444e0c6..074872f5a32582138e30ff39a188b956fc56f4f5 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -2100,6 +2100,15 @@ :weight: 1 :idempotent: true :tags: [] +- :name: vulnerabilities_process_transfer_events + :worker_name: Vulnerabilities::ProcessTransferEventsWorker + :feature_category: :vulnerability_management + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: vulnerabilities_remove_all_vulnerabilities :worker_name: Vulnerabilities::RemoveAllVulnerabilitiesWorker :feature_category: :vulnerability_management diff --git a/ee/app/workers/vulnerabilities/process_transfer_events_worker.rb b/ee/app/workers/vulnerabilities/process_transfer_events_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..c8d6bb80924ac0d3537bacdc819b0602274cb7b6 --- /dev/null +++ b/ee/app/workers/vulnerabilities/process_transfer_events_worker.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Vulnerabilities + class ProcessTransferEventsWorker + include Gitlab::EventStore::Subscriber + + idempotent! + deduplicate :until_executing, including_scheduled: true + data_consistency :always + + feature_category :vulnerability_management + + def handle_event(event) + bulk_arguments = ProjectSetting + .for_projects(project_ids(event)) + .has_vulnerabilities + .pluck_primary_key + .zip + + Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker.perform_bulk(bulk_arguments) + end + + private + + def project_ids(event) + case event + when ::Projects::ProjectTransferedEvent + [event.data[:project_id]] + when ::Groups::GroupTransferedEvent + group = Group.find_by_id(event.data[:group_id]) + + return [] unless group + + group.all_project_ids + end + end + end +end diff --git a/ee/config/feature_flags/gitlab_com_derisk/update_vuln_reads_traversal_ids_via_event.yml b/ee/config/feature_flags/gitlab_com_derisk/update_vuln_reads_traversal_ids_via_event.yml new file mode 100644 index 0000000000000000000000000000000000000000..7269964d56a58fa6d1b1dded0621a40670e5e428 --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/update_vuln_reads_traversal_ids_via_event.yml @@ -0,0 +1,9 @@ +--- +name: update_vuln_reads_traversal_ids_via_event +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/437669 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143537 +rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/17555 +milestone: '16.9' +group: group::threat insights +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/lib/ee/gitlab/event_store.rb b/ee/lib/ee/gitlab/event_store.rb index 3324ac09c32d213eb3d3192adfb1011b77c152ab..114f18c65424169052badcd6ba0ede5e79a0c1a1 100644 --- a/ee/lib/ee/gitlab/event_store.rb +++ b/ee/lib/ee/gitlab/event_store.rb @@ -49,6 +49,18 @@ def configure!(store) to: ::ProjectAuthorizations::AuthorizationsAddedEvent store.subscribe ::Security::RefreshComplianceFrameworkSecurityPoliciesWorker, to: ::Projects::ComplianceFrameworkChangedEvent + store.subscribe ::Vulnerabilities::ProcessTransferEventsWorker, + to: ::Projects::ProjectTransferedEvent, + if: ->(event) { + ::Feature.enabled?(:update_vuln_reads_traversal_ids_via_event, + ::Project.find_by_id(event.data['project_id']), type: :gitlab_com_derisk) + } + store.subscribe ::Vulnerabilities::ProcessTransferEventsWorker, + to: ::Groups::GroupTransferedEvent, + if: ->(event) { + ::Feature.enabled?(:update_vuln_reads_traversal_ids_via_event, + ::Group.find_by_id(event.data['group_id']), type: :gitlab_com_derisk) + } end end end diff --git a/ee/spec/factories/projects.rb b/ee/spec/factories/projects.rb index f85412aa038c8e0f0f343279ef45a6356e616d41..5a3310b537fc972d145d51052e8c2bc3f0dba42e 100644 --- a/ee/spec/factories/projects.rb +++ b/ee/spec/factories/projects.rb @@ -30,12 +30,14 @@ trait :with_vulnerability do after(:create) do |project| create(:vulnerability, :detected, project: project) + project.project_setting.update!(has_vulnerabilities: true) end end trait :with_vulnerabilities do after(:create) do |project| create_list(:vulnerability, 2, :with_finding, :detected, project: project) + project.project_setting.update!(has_vulnerabilities: true) end end diff --git a/ee/spec/services/projects/transfer_service_spec.rb b/ee/spec/services/projects/transfer_service_spec.rb index d85a3dfa90e0f8ee44f22ac4a6ca67deb7c8df69..9cbc63f01ff2a2f7291c6b102d357bb7b9c5ef99 100644 --- a/ee/spec/services/projects/transfer_service_spec.rb +++ b/ee/spec/services/projects/transfer_service_spec.rb @@ -70,23 +70,51 @@ def operation allow(Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker).to receive(:perform_async) end - context 'when the project does not have vulnerabilities' do - it 'does not schedule the update job' do - subject.execute(group) + context 'when update_vuln_reads_on_project_transfer_via_event is disabled' do + before do + stub_feature_flags(update_vuln_reads_traversal_ids_via_event: false) + end + + context 'when the project does not have vulnerabilities' do + it 'does not schedule the update job' do + subject.execute(group) + + expect(Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker).not_to have_received(:perform_async) + end + end + + context 'when the project has vulnerabilities' do + before do + create(:project_setting, project: project, has_vulnerabilities: true) + end + + it 'schedules the update job' do + subject.execute(group) - expect(Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker).not_to have_received(:perform_async) + expect(Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker).to have_received(:perform_async).with(project.id) + end end end - context 'when the project has vulnerabilities' do - before do - create(:project_setting, project: project, has_vulnerabilities: true) + context 'when update_vuln_reads_on_project_transfer_via_event is enabled' do + context 'when the project does not have vulnerabilities' do + it 'does not schedule the update job' do + subject.execute(group) + + expect(Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker).not_to have_received(:perform_async) + end end - it 'schedules the update job' do - subject.execute(group) + context 'when the project has vulnerabilities' do + before do + create(:project_setting, project: project, has_vulnerabilities: true) + end - expect(Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker).to have_received(:perform_async).with(project.id) + it 'does not schedule the update job' do + subject.execute(group) + + expect(Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker).not_to have_received(:perform_async) + end end end end diff --git a/ee/spec/services/vulnerabilities/update_namespace_ids_of_vulnerability_reads_service_spec.rb b/ee/spec/services/vulnerabilities/update_namespace_ids_of_vulnerability_reads_service_spec.rb index f2eeb1c5d18df8567b24914bf93d825948b57207..3b8ca99664ecb74f9fae3012cf7d21094e26131a 100644 --- a/ee/spec/services/vulnerabilities/update_namespace_ids_of_vulnerability_reads_service_spec.rb +++ b/ee/spec/services/vulnerabilities/update_namespace_ids_of_vulnerability_reads_service_spec.rb @@ -35,17 +35,21 @@ before do vulnerability_read.update_column(:namespace_id, old_namespace.id) + vulnerability_read.update_column(:traversal_ids, old_namespace.traversal_ids) end - it 'changes the `namespace_id` of vulnerability read record' do + it 'changes the `namespace_id` and `traversal_ids` of vulnerability read record' do expect { update_namespace_ids } .to change { vulnerability_read.reload.namespace_id }.from(old_namespace.id).to(project.namespace_id) + .and change { + vulnerability_read.reload.traversal_ids + }.from(old_namespace.traversal_ids).to(project.namespace.traversal_ids) end describe 'parallel execution' do include ExclusiveLeaseHelpers - let(:lease_key) { 'update_vulnerability_reads_namespace_id' } + let(:lease_key) { "update_vulnerability_reads_namespace_id_and_traversal_ids:projects:#{project_id}" } let(:lease_ttl) { 5.minutes } before do diff --git a/ee/spec/workers/vulnerabilities/process_transfer_events_worker_spec.rb b/ee/spec/workers/vulnerabilities/process_transfer_events_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ac73f0573549014f9d91592f573ca87a0d6b24e8 --- /dev/null +++ b/ee/spec/workers/vulnerabilities/process_transfer_events_worker_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Vulnerabilities::ProcessTransferEventsWorker, feature_category: :vulnerability_management, type: :job do + let_it_be(:old_group) { create(:group) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :with_vulnerability, group: group) } + let_it_be(:other_project) { create(:project, :with_vulnerability, group: group) } + let_it_be(:project_without_vulnerabilities) { create(:project, group: group) } + + let(:project_event) do + ::Projects::ProjectTransferedEvent.new(data: { + project_id: project.id, + old_namespace_id: old_group.id, + old_root_namespace_id: old_group.id, + new_namespace_id: group.id, + new_root_namespace_id: group.id + }) + end + + let(:group_event) do + ::Groups::GroupTransferedEvent.new(data: { + group_id: group.id, + old_root_namespace_id: old_group.id, + new_root_namespace_id: group.id + }) + end + + it_behaves_like 'worker with data consistency', described_class, data_consistency: :always + + subject(:use_event) { consume_event(subscriber: described_class, event: event) } + + context 'when the associated project has vulnerabilities' do + before do + project.project_setting.update!(has_vulnerabilities: true) + end + + context 'when a project transfered event is published', :sidekiq_inline do + let(:event) { project_event } + + context 'when update_vuln_reads_on_project_transfer_via_event is disabled' do + before do + stub_feature_flags(update_vuln_reads_traversal_ids_via_event: false) + end + + it_behaves_like 'ignores the published event' + end + + it_behaves_like 'subscribes to event' + + it 'enqueues a vulnerability reads namespace id update job for the project id' do + expect(Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker).to receive(:perform_bulk).with( + [[project.id]] + ) + + use_event + end + end + + context 'when a group transfered event is published', :sidekiq_inline do + let(:event) { group_event } + + context 'when update_vuln_reads_on_project_transfer_via_event is disabled' do + before do + stub_feature_flags(update_vuln_reads_traversal_ids_via_event: false) + end + + it_behaves_like 'ignores the published event' + end + + it_behaves_like 'subscribes to event' + + it 'enqueues a vulnerability reads namespace id update job for each project id belonging to the namespace id' do + expect(Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker).to receive(:perform_bulk).with( + match_array([[project.id], [other_project.id]]) + ) + + use_event + end + end + end + + context 'when the associated project does not have vulnerabilities' do + let(:project) { project_without_vulnerabilities } + + context 'when a project transfered event is published', :sidekiq_inline do + let(:event) { project_event } + + it 'enqueues a vulnerability reads namespace id update job for the project id' do + expect(Vulnerabilities::UpdateNamespaceIdsOfVulnerabilityReadsWorker).to receive(:perform_bulk).with([]) + + use_event + end + end + end +end