Skip to content

Update DeleteTagsService to allow policy to work

Steve Abrams requested to merge 219915-cleanup-policy-not-working into master

What does this MR do?

Projects::ContainerRepository::CleanupTagsService gets run in one of two ways:

  1. Via the tags bulk delete api
  2. Via an Cleanup Policy (which runs via a cron job)

When the cleanup policy runs, there is no user associated with it being run, so we have a param: container_expiration_policy that is attached to the worker that will call the CleanupTagsService. This param is passed to the CleanupTagsService, and is used to along the way allow the service to run when the user is nil, generally by bypassing permission checks.

The problem is, the CleanupTagsService calls Projects::ContainerRepository::DeleteTagsService, and does not pass along the param, but user permissions are checked once again in the DeleteTagsService. This means that right now, the cleanup policies are completely broken and will not do any actual cleaning up when they run.

This MR passes along that param into DeleteTagsService and bypasses the permission check there to allow the policies to run as intended.

Passing this param is not optimal and it would be much better to have a valid user and not bypass the permission checks. I've created #255969 as the follow up to this fix to refactor this feature to include a user rather than this param.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team

Related #219915 (closed)

Edited by Steve Abrams

Merge request reports