Commit 4a25029f authored by Stan Hu's avatar Stan Hu

Log audit and Geo events within a project destroy transaction

This prevents odd issues where audit events are not logged if
the project is abruptly destroyed due to Sidekiq shutting down
or some other reason.

Relates to https://gitlab.com/gitlab-com/migration/issues/295
parent 85a62776
...@@ -136,6 +136,7 @@ module Projects ...@@ -136,6 +136,7 @@ module Projects
raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.') raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.')
end end
log_destroy_event
trash_repositories! trash_repositories!
# Rails attempts to load all related records into memory before # Rails attempts to load all related records into memory before
...@@ -149,6 +150,10 @@ module Projects ...@@ -149,6 +150,10 @@ module Projects
end end
end end
def log_destroy_event
log_info("Attempting to destroy #{project.full_path} (#{project.id})")
end
## ##
# This method makes sure that we correctly remove registry tags # This method makes sure that we correctly remove registry tags
# for legacy image repository (when repository path equals project path). # for legacy image repository (when repository path equals project path).
......
...@@ -12,13 +12,19 @@ module EE ...@@ -12,13 +12,19 @@ module EE
# and clean up where we can. # and clean up where we can.
if project&.destroyed? if project&.destroyed?
mirror_cleanup(project) mirror_cleanup(project)
log_geo_event(project)
log_audit_event(project)
end end
succeeded succeeded
end end
override :log_destroy_event
def log_destroy_event
super
log_geo_event(project)
log_audit_event(project)
end
def mirror_cleanup(project) def mirror_cleanup(project)
return unless project.mirror? return unless project.mirror?
......
---
title: Log audit and Geo events within a project destroy transaction
merge_request: 6059
author:
type: fixed
...@@ -36,11 +36,13 @@ describe Projects::DestroyService do ...@@ -36,11 +36,13 @@ describe Projects::DestroyService do
it 'logs an event to the Geo event log' do it 'logs an event to the Geo event log' do
# Run Sidekiq immediately to check that renamed repository will be removed # Run Sidekiq immediately to check that renamed repository will be removed
Sidekiq::Testing.inline! do Sidekiq::Testing.inline! do
expect(subject).to receive(:log_destroy_event).and_call_original
expect { subject.execute }.to change(Geo::RepositoryDeletedEvent, :count).by(1) expect { subject.execute }.to change(Geo::RepositoryDeletedEvent, :count).by(1)
end end
end end
it 'does not log event to the Geo log if project deletion fails' do it 'does not log event to the Geo log if project deletion fails' do
expect(subject).to receive(:log_destroy_event).and_call_original
expect_any_instance_of(Project) expect_any_instance_of(Project)
.to receive(:destroy!).and_raise(StandardError.new('Other error message')) .to receive(:destroy!).and_raise(StandardError.new('Other error message'))
...@@ -80,6 +82,7 @@ describe Projects::DestroyService do ...@@ -80,6 +82,7 @@ describe Projects::DestroyService do
end end
it 'logs an audit event' do it 'logs an audit event' do
expect(subject).to receive(:log_destroy_event).and_call_original
expect { subject.execute }.to change(AuditEvent, :count) expect { subject.execute }.to change(AuditEvent, :count)
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment