From 5700d32247076b3f0d3e766c2db656b7b3ce5553 Mon Sep 17 00:00:00 2001 From: huzaifaiftikhar1 Date: Mon, 3 Feb 2025 16:00:26 +0530 Subject: [PATCH 1/3] Add service class to trigger external control Changelog: added EE: true --- .../trigger_external_control_service.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ee/app/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service.rb b/ee/app/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service.rb index 49946bd59c73e1..f304cbe872feb7 100644 --- a/ee/app/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service.rb +++ b/ee/app/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service.rb @@ -70,6 +70,9 @@ def handle_response(response) audit_success(response.code) ServiceResponse.success(payload: { control: control }) + + ComplianceManagement::TimeoutPendingExternalControlsWorker.perform_in(31.minutes, + { 'control_id' => control.id, 'project_id' => project.id }) else audit_error(Rack::Utils::HTTP_STATUS_CODES[response.code], response.code) -- GitLab From 0cfd321f2ab4de205cc332958e592f17da7c6bc1 Mon Sep 17 00:00:00 2001 From: huzaifaiftikhar1 Date: Mon, 3 Feb 2025 16:49:13 +0530 Subject: [PATCH 2/3] Add worker to timeout pending compliance external controls Changelog: added EE: true --- config/sidekiq_queues.yml | 2 + doc/user/compliance/audit_event_types.md | 1 + .../trigger_external_control_service.rb | 4 +- ee/app/workers/all_queues.yml | 10 ++ ...imeout_pending_external_controls_worker.rb | 41 +++++++ ...ing_compliance_external_control_failed.yml | 10 ++ .../trigger_external_control_service_spec.rb | 7 ++ ...t_pending_external_controls_worker_spec.rb | 111 ++++++++++++++++++ 8 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 ee/app/workers/compliance_management/timeout_pending_external_controls_worker.rb create mode 100644 ee/config/audit_events/types/pending_compliance_external_control_failed.yml create mode 100644 ee/spec/workers/compliance_management/timeout_pending_external_controls_worker_spec.rb diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 587ebc37ccbae1..b250ce3bbb7d68 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -271,6 +271,8 @@ - 1 - - compliance_management_standards_soc2_at_least_one_non_author_approval_group - 1 +- - compliance_management_timeout_pending_external_controls + - 1 - - compliance_management_update_default_framework - 1 - - compliance_management_violation_export_mailer diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 02f9287bb1062d..58f453f35ec336 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -170,6 +170,7 @@ Audit event types belong to the following product categories. | [`merge_request_merged`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/164846) | A merge request is merged | **{check-circle}** Yes | GitLab [17.5](https://gitlab.com/gitlab-org/gitlab/-/issues/442279) | Project | | [`omniauth_login_failed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123080) | An OmniAuth login fails | **{check-circle}** Yes | GitLab [16.3](https://gitlab.com/gitlab-org/gitlab/-/issues/374107) | User | | [`password_reset_requested`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/114548) | A user requests a password reset using a registered email address | **{check-circle}** Yes | GitLab [15.11](https://gitlab.com/gitlab-org/gitlab/-/issues/374107) | User | +| [`pending_compliance_external_control_failed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180016) | A project's compliance external control status is updated to fail because of timeout. | **{check-circle}** Yes | GitLab [17.9](https://gitlab.com/gitlab-org/gitlab/-/issues/513421) | Project | | [`personal_access_token_created`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108952) | A user creates a personal access token | **{check-circle}** Yes | GitLab [15.9](https://gitlab.com/gitlab-org/gitlab/-/issues/374113) | User | | [`personal_access_token_revoked`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108952) | A personal access token is revoked | **{check-circle}** Yes | GitLab [15.9](https://gitlab.com/gitlab-org/gitlab/-/issues/374113) | User | | [`project_archived`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/117528) | A project is archived | **{check-circle}** Yes | GitLab [15.11](https://gitlab.com/gitlab-org/gitlab/-/issues/374105) | Project | diff --git a/ee/app/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service.rb b/ee/app/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service.rb index f304cbe872feb7..23cbca83fdd97d 100644 --- a/ee/app/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service.rb +++ b/ee/app/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service.rb @@ -69,10 +69,10 @@ def handle_response(response) if response.success? audit_success(response.code) - ServiceResponse.success(payload: { control: control }) - ComplianceManagement::TimeoutPendingExternalControlsWorker.perform_in(31.minutes, { 'control_id' => control.id, 'project_id' => project.id }) + + ServiceResponse.success(payload: { control: control }) else audit_error(Rack::Utils::HTTP_STATUS_CODES[response.code], response.code) diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index 30c8e74eaaa492..965aaf875dfd03 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -1693,6 +1693,16 @@ :idempotent: true :tags: [] :queue_namespace: +- :name: compliance_management_timeout_pending_external_controls + :worker_name: ComplianceManagement::TimeoutPendingExternalControlsWorker + :feature_category: :compliance_management + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: - :name: compliance_management_update_default_framework :worker_name: ComplianceManagement::UpdateDefaultFrameworkWorker :feature_category: :compliance_management diff --git a/ee/app/workers/compliance_management/timeout_pending_external_controls_worker.rb b/ee/app/workers/compliance_management/timeout_pending_external_controls_worker.rb new file mode 100644 index 00000000000000..97e06dbdb57480 --- /dev/null +++ b/ee/app/workers/compliance_management/timeout_pending_external_controls_worker.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module ComplianceManagement + class TimeoutPendingExternalControlsWorker + include ApplicationWorker + + idempotent! + feature_category :compliance_management + data_consistency :sticky + urgency :low + + def perform(args = {}) + args = args.with_indifferent_access + control_id, project_id = args.values_at(:control_id, :project_id) + + control = ComplianceManagement::ComplianceFramework::ComplianceRequirementsControl.find_by_id(control_id) + + return unless control + + project_control_compliance_status = ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus + .for_project_and_control(project_id, control_id).last + + unless project_control_compliance_status&.pending? && + project_control_compliance_status.updated_at < 30.minutes.ago + return + end + + project_control_compliance_status.fail! + + audit_context = { + name: 'pending_compliance_external_control_failed', + author: ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'), + scope: project_control_compliance_status.project, + target: project_control_compliance_status.project, + message: "Project control compliance status with URL #{control.external_url} marked as fail." + } + + ::Gitlab::Audit::Auditor.audit(audit_context) + end + end +end diff --git a/ee/config/audit_events/types/pending_compliance_external_control_failed.yml b/ee/config/audit_events/types/pending_compliance_external_control_failed.yml new file mode 100644 index 00000000000000..5ef423c472102a --- /dev/null +++ b/ee/config/audit_events/types/pending_compliance_external_control_failed.yml @@ -0,0 +1,10 @@ +--- +name: pending_compliance_external_control_failed +description: A project's compliance external control status is updated to fail because of timeout. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/513421 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180016 +feature_category: compliance_management +milestone: '17.9' +saved_to_database: true +streamed: true +scope: [Project] diff --git a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service_spec.rb b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service_spec.rb index a31096e0f5c973..4cf4fc42295d67 100644 --- a/ee/spec/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service_spec.rb +++ b/ee/spec/services/compliance_management/compliance_framework/compliance_requirements/trigger_external_control_service_spec.rb @@ -111,6 +111,13 @@ end end + it 'schedules a timeout worker' do + expect(ComplianceManagement::TimeoutPendingExternalControlsWorker).to receive(:perform_in) + .with(31.minutes, { 'control_id' => control.id, 'project_id' => project.id }) + + service.execute + end + it 'creates an audit event for successful request' do expect(Gitlab::Audit::Auditor).to receive(:audit).with( hash_including( diff --git a/ee/spec/workers/compliance_management/timeout_pending_external_controls_worker_spec.rb b/ee/spec/workers/compliance_management/timeout_pending_external_controls_worker_spec.rb new file mode 100644 index 00000000000000..9f05980beea2b8 --- /dev/null +++ b/ee/spec/workers/compliance_management/timeout_pending_external_controls_worker_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ComplianceManagement::TimeoutPendingExternalControlsWorker, feature_category: :compliance_management do + let_it_be(:project) { create(:project) } + let_it_be(:control) { create(:compliance_requirements_control, :external) } + + let(:worker) { described_class.new } + let(:args) { { control_id: control.id, project_id: project.id } } + + describe '#perform' do + context 'when control does not exist' do + let(:args) { { control_id: non_existing_record_id, project_id: project.id } } + + it 'does nothing' do + expect(ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus) + .not_to receive(:for_project_and_control) + + worker.perform(args) + end + end + + context 'when compliance status does not exist' do + it 'does nothing' do + expect(::Gitlab::Audit::Auditor).not_to receive(:audit) + + worker.perform(args) + end + end + + context 'when compliance status exists' do + let_it_be(:compliance_status) do + create(:project_control_compliance_status, + project: project, + compliance_requirements_control: control, + status: :pending + ) + end + + context 'when status is not pending' do + before do + compliance_status.update!(status: :fail) + end + + it 'does nothing' do + expect(compliance_status).not_to receive(:fail!) + + worker.perform(args) + end + end + + context 'when status was updated less than 30 minutes ago' do + before do + compliance_status.touch + end + + it 'does nothing' do + expect(compliance_status).not_to receive(:fail!) + + worker.perform(args) + end + end + + context 'when status is pending and was updated more than 30 minutes ago' do + before do + compliance_status.update!(updated_at: 31.minutes.ago) + end + + it 'marks status as failed' do + expect(compliance_status.reload.status).to eq("pending") + + worker.perform(args) + + expect(compliance_status.reload.status).to eq("fail") + end + + it 'creates an audit event' do + expected_message = "Project control compliance status with URL #{control.external_url} marked as fail." + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with( + hash_including( + name: 'pending_compliance_external_control_failed', + author: instance_of(::Gitlab::Audit::UnauthenticatedAuthor), + scope: project, + target: project, + message: expected_message + ) + ) + + worker.perform(args) + end + end + end + + context 'when args are strings' do + let(:string_args) { { 'control_id' => control.id.to_s, 'project_id' => project.id.to_s } } + + it 'handles string keys' do + expect(ComplianceManagement::ComplianceFramework::ComplianceRequirementsControl) + .to receive(:find_by_id).with(control.id.to_s).and_return(control) + + worker.perform(string_args) + end + end + end + + it_behaves_like 'an idempotent worker' do + let(:job_args) { { control_id: control.id, project_id: project.id } } + end +end -- GitLab From fffea9652038f6d93642b6d2232e9e910c27aea7 Mon Sep 17 00:00:00 2001 From: huzaifaiftikhar1 Date: Wed, 12 Feb 2025 22:28:34 +0530 Subject: [PATCH 3/3] Simplify perform method --- ...imeout_pending_external_controls_worker.rb | 46 +++++++++++++------ 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/ee/app/workers/compliance_management/timeout_pending_external_controls_worker.rb b/ee/app/workers/compliance_management/timeout_pending_external_controls_worker.rb index 97e06dbdb57480..cab206845c7414 100644 --- a/ee/app/workers/compliance_management/timeout_pending_external_controls_worker.rb +++ b/ee/app/workers/compliance_management/timeout_pending_external_controls_worker.rb @@ -9,30 +9,48 @@ class TimeoutPendingExternalControlsWorker data_consistency :sticky urgency :low + PENDING_STATUS_TIMEOUT = 30.minutes + def perform(args = {}) - args = args.with_indifferent_access - control_id, project_id = args.values_at(:control_id, :project_id) + @args = args.with_indifferent_access + + return unless valid_control? + return unless valid_project_control_compliance_status? + return unless timeout_project_control_compliance_status? + + @project_control_compliance_status.fail! + + create_audit_log + end - control = ComplianceManagement::ComplianceFramework::ComplianceRequirementsControl.find_by_id(control_id) + private - return unless control + def valid_control? + control_id = @args[:control_id] + @control = ComplianceManagement::ComplianceFramework::ComplianceRequirementsControl.find_by_id(control_id) + @control.present? + end - project_control_compliance_status = ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus - .for_project_and_control(project_id, control_id).last + def valid_project_control_compliance_status? + control_id, project_id = @args.values_at(:control_id, :project_id) + @project_control_compliance_status = ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus + .for_project_and_control(project_id, control_id).last - unless project_control_compliance_status&.pending? && - project_control_compliance_status.updated_at < 30.minutes.ago - return - end + @project_control_compliance_status.present? + end - project_control_compliance_status.fail! + def timeout_project_control_compliance_status? + @project_control_compliance_status.pending? && + @project_control_compliance_status.updated_at < PENDING_STATUS_TIMEOUT.ago + end + def create_audit_log audit_context = { name: 'pending_compliance_external_control_failed', author: ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'), - scope: project_control_compliance_status.project, - target: project_control_compliance_status.project, - message: "Project control compliance status with URL #{control.external_url} marked as fail." + scope: @project_control_compliance_status.project, + target: @project_control_compliance_status.project, + message: "Project control compliance status with URL #{@control.external_url} marked as fail." } ::Gitlab::Audit::Auditor.audit(audit_context) -- GitLab