Audit status check response updates
The following discussion from !113926 (merged) should be addressed:
-
@huzaifaiftikhar1 started a discussion: (+4 comments) Question: Can you please help me answer the following questions?
- Do we want to create an audit event every time a status check response is updated? Won't this be a high volume event? If we want to capture this we should probably keep it as a stream only event.
- I believe this will create an audit event with a generic 'audit_operation' event type and we probably want to avoid doing that. @harsimarsandhu Should we continue using the
Auditableconcern or use::Gitlab::Audit::Auditor.audithere? I am not too sure therefore wanted your opinion on this one.
Based on the discussions in the above thread and some investigations on how we're currently using ::Gitlab::Audit::Auditor, the audit event was removed from !113926 (merged)
Removed code
ee/app/models/merge_requests/status_check_response.rb (not pushed but working locally)
after_update :log_audit_event
def log_audit_event
project = merge_request.project
audit_context = {
name: 'status_check_response_update',
author: ::Gitlab::Audit::UnauthenticatedAuthor.new(name: '(System)'),
scope: project,
target: project,
message: "Updated status check response #{id} for status check #{external_status_check.name}"
}
::Gitlab::Audit::Auditor.audit(audit_context)
end
ee/spec/services/external_status_checks/retry_service_spec.rb
describe 'audit events' do
let(:status) { 'failed' }
context 'when licensed', :request_store do
before do
stub_licensed_features(audit_events: true)
stub_licensed_features(external_status_checks: true)
end
context 'when rule retry operation succeeds' do
let(:expected_message) do
"Updated status check response #{status_check_response.id} for status check #{rule.name}"
end
it 'logs an audit event' do
expect { subject }.to change { AuditEvent.count }.by(1)
expect(AuditEvent.last.details).to match(
{
author_class: 'Gitlab::Audit::UnauthenticatedAuthor',
author_name: '(System)',
custom_message: expected_message,
target_details: project.name,
target_id: project.id,
target_type: 'Project',
}
)
end
end
context 'when rule destroy operation fails' do
before do
allow_any_instance_of(::MergeRequests::StatusCheckResponse).to receive(:update).and_return(false) # rubocop:disable RSpec/AnyInstanceOf - It's not the next instance
end
it 'does not log any audit event' do
expect { subject }.not_to change { AuditEvent.count }
expect(subject).to be_error
end
end
end
it_behaves_like 'does not create audit event when not licensed'
end
Edited by Imam Hossain