Skip to content

Allow destruction of records in archived pending_delete Projects

drew stachon requested to merge delete-records-from-deleting-projects into master

What does this MR do and why?

This change stops the prevent-create-update-destroy permission from applying to records within Projects that are pending_delete: true. the purpose of the prevent is to make "archived" projects reliably read-only, but this obviously shouldn't be the case if someone is deleting a project.

This hasn't been an issue before since we relied of FK constraints to remove the associated records after a Project is deleted. But with the FKs going away in the database decomposition effort, so too must our cascading destroys. Luckily we have very performant code to destroy records from ci_pipelines, ci_builds, and ci_job_artifacts. We just need to allow that action to be taken in application logic permissions, where it will have to live going forward.

This use case recently caused a production issue requiring escalation to development: https://gitlab.com/gitlab-com/support/internal-requests/-/issues/10553

We tracked the problem down to this, and the spec introduced here successfully reproduces the issue.

Screenshots or screen recordings

Live debugging output in the internal support ticket: https://gitlab.com/gitlab-com/support/internal-requests/-/issues/10553#note_721539319

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

  1. Remove the new condition ( & ~pending_delete) from the modified in rule in ProjectPolicy.
  2. Run the added 'for an archived project' context in projects/destroy_service_spec.rb and the updated 'pipeline feature' spec context in project_policy_spec.rb.
  3. Observe spec failures.
  4. Add the & ~pending_delete back in.
  5. Run the added 'for an archived project' context in projects/destroy_service_spec.rb and the updated 'pipeline feature' spec context in project_policy_spec.rb.
  6. Observe spec successes.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by drew stachon

Merge request reports