Loading
Delete orphaned uploads when destroying vulnerability exports
What does this MR do and why?
Ensures that all Upload records associated with Vulnerability Exports are correctly, and fully, deleted when the overall export is expired.
This was an oversight of the original implementation where the Vulnerabilities::Export records would be deleted and that would ON DELETE CASCADE to the Vulnerabilities::Export::Part record, but there wasn't a further cascade to the Upload that correlated to the Part.
This MR:
- Makes
Vulnerabilities::Exports::BatchDestroyServicedelete each batch's export parts and their uploads before deleting the exports.Upload.destroy_for_associations!also removes the underlying files from object storage viabegin_fast_destroy/finalize_fast_destroy. - Guards that shared spec:
Gitlab::Database::TablesWithDestroyServices::EXTRA_TABLES_TO_SERVICESnamesVulnerabilities::Exports::BatchDestroyServiceunconditionally, but FOSS pipelines runrm -rf ee/, so the EE-only class can't be constantized there and the assertion would fail on FOSS once the todo entry was gone. Foreign keys whose owning services aren't loadable are now skipped outside EE.
Related work
vulnerability_exportshad a loose foreign key gap that orphaned uploads when deleting a project, group, user, or organization — fixed in !254398 (merged).Sbom::DeleteExpiredExportsWorkerhad the identical bug forDependencies::DependencyListExport::Part— fixed in !254397 (merged).- This MR doesn't remove orphans that already exist; use the documented
delete_orphaned_uploadsconsole step: https://docs.gitlab.com/administration/geo/replication/troubleshooting/synchronization_verification/#failed-verification-of-uploads-on-the-primary-geo-site
References
- Issue: #602878 (closed)
- The worker this service backs was added in !183197 (merged)
- Dedicated tracking: https://gitlab.com/groups/gitlab-com/gl-infra/gitlab-dedicated/-/work_items/991
How to set up and validate locally
- Start the rails console:
bundle exec rails c - Create an expired export with a part that has a file, then run the worker and confirm both the part and its upload are gone:
project = Project.last
author = User.last
export = FactoryBot.create(:vulnerability_export, :with_csv_file, expires_at: 1.hour.ago, project: project, author: author)
part = FactoryBot.create(:vulnerability_export_part, :with_csv_file, vulnerability_export: export)
Upload.for_model_type_and_id(Vulnerabilities::Export::Part, part.id).count # => 1
Vulnerabilities::DeleteExpiredExportsWorker.new.perform
Vulnerabilities::Export::Part.exists?(part.id) # => false
Upload.for_model_type_and_id(Vulnerabilities::Export::Part, part.id).count # => 0On master the last line returns 1.
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Edited by Ryan Wells