From d64ca9e5dd641c7ae773f40061fa8874e1eb7164 Mon Sep 17 00:00:00 2001 From: "j.seto" Date: Wed, 15 Feb 2023 14:33:13 -0500 Subject: [PATCH 1/2] Log audit events for manually triggered housekeeping * Add a new audit event type for manually triggered housekeeping * Log audit events housekeeping is triggered by a user Contributes to: https://gitlab.com/gitlab-org/gitlab/-/issues/390761 Changelog: other --- app/controllers/projects_controller.rb | 11 +++++++- .../types/manually_trigger_housekeeping.yml | 9 +++++++ lib/api/projects.rb | 11 +++++++- spec/controllers/projects_controller_spec.rb | 14 ++++++++++ spec/requests/api/projects_spec.rb | 26 ++++++++++++++++--- 5 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 config/audit_events/types/manually_trigger_housekeeping.yml diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 0338c912b53330..71ad747b6b1da3 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -228,7 +228,16 @@ def housekeeping :eager end - ::Repositories::HousekeepingService.new(@project, task).execute + ::Repositories::HousekeepingService.new(@project, task).execute do + ::Gitlab::Audit::Auditor.audit( + name: 'manually_trigger_housekeeping', + author: current_user, + scope: @project, + target: @project, + message: "Housekeeping task: #{task}", + created_at: DateTime.current + ) + end redirect_to( project_path(@project), diff --git a/config/audit_events/types/manually_trigger_housekeeping.yml b/config/audit_events/types/manually_trigger_housekeeping.yml new file mode 100644 index 00000000000000..70c9818d8e80f8 --- /dev/null +++ b/config/audit_events/types/manually_trigger_housekeeping.yml @@ -0,0 +1,9 @@ +--- +name: manually_trigger_housekeeping +description: Triggered when manually triggering housekeeping via api or admin UI +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/390761 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112095 +feature_category: source_code_management +milestone: '15.9' +saved_to_database: true +streamed: true diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 36c132fdc10059..6eea56ea117f72 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -875,7 +875,16 @@ def add_import_params(params) authorize_admin_project begin - ::Repositories::HousekeepingService.new(user_project, params[:task]).execute + ::Repositories::HousekeepingService.new(user_project, params[:task]).execute do + ::Gitlab::Audit::Auditor.audit( + name: 'manually_trigger_housekeeping', + author: current_user, + scope: user_project, + target: user_project, + message: "Housekeeping task: #{params[:task]}", + created_at: DateTime.current + ) + end rescue ::Repositories::HousekeepingService::LeaseTaken => error conflict!(error.message) end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index c0c5dcfe21d783..cf87b2c443738d 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -664,6 +664,20 @@ def get_show expect(response).to have_gitlab_http_status(:found) end + it 'logs an audit event' do + expect(housekeeping).to receive(:execute).once.and_yield + + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(a_hash_including( + name: 'manually_trigger_housekeeping', + author: user, + scope: project, + target: project, + message: "Housekeeping task: eager" + )) + + subject + end + context 'and requesting prune' do let(:prune) { true } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 9a481c524e2ec7..903ed1564905c6 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -4719,6 +4719,9 @@ def failure_message(diff) describe 'POST /projects/:id/housekeeping' do let(:housekeeping) { Repositories::HousekeepingService.new(project) } + let(:params) { {} } + + subject { post api("/projects/#{project.id}/housekeeping", user), params: params } before do allow(Repositories::HousekeepingService).to receive(:new).with(project, :eager).and_return(housekeeping) @@ -4728,26 +4731,43 @@ def failure_message(diff) it 'starts the housekeeping process' do expect(housekeeping).to receive(:execute).once - post api("/projects/#{project.id}/housekeeping", user) + subject expect(response).to have_gitlab_http_status(:created) end + it 'logs an audit event' do + expect(housekeeping).to receive(:execute).once.and_yield + expect(::Gitlab::Audit::Auditor).to receive(:audit).with(a_hash_including( + name: 'manually_trigger_housekeeping', + author: user, + scope: project, + target: project, + message: "Housekeeping task: eager" + )) + + subject + end + context 'when requesting prune' do + let(:params) { { task: :prune } } + it 'triggers a prune' do expect(Repositories::HousekeepingService).to receive(:new).with(project, :prune).and_return(housekeeping) expect(housekeeping).to receive(:execute).once - post api("/projects/#{project.id}/housekeeping", user), params: { task: :prune } + subject expect(response).to have_gitlab_http_status(:created) end end context 'when requesting an unsupported task' do + let(:params) { { task: :unsupported_task } } + it 'responds with bad_request' do expect(Repositories::HousekeepingService).not_to receive(:new) - post api("/projects/#{project.id}/housekeeping", user), params: { task: :unsupported_task } + subject expect(response).to have_gitlab_http_status(:bad_request) end end -- GitLab From b779d3aa9a5cccf13b9f6a902b24cb8e8355699a Mon Sep 17 00:00:00 2001 From: "j.seto" Date: Thu, 16 Feb 2023 09:43:33 -0500 Subject: [PATCH 2/2] Refactor a bit --- spec/requests/api/projects_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 903ed1564905c6..e78ef2f763028d 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -4767,7 +4767,9 @@ def failure_message(diff) it 'responds with bad_request' do expect(Repositories::HousekeepingService).not_to receive(:new) + subject + expect(response).to have_gitlab_http_status(:bad_request) end end @@ -4776,7 +4778,7 @@ def failure_message(diff) it 'returns conflict' do expect(housekeeping).to receive(:execute).once.and_raise(Repositories::HousekeepingService::LeaseTaken) - post api("/projects/#{project.id}/housekeeping", user) + subject expect(response).to have_gitlab_http_status(:conflict) expect(json_response['message']).to match(/Somebody already triggered housekeeping for this resource/) -- GitLab