Validate regex before enqueuing a CleanupContainerRepositoryWorker job
What does this MR do?
As described in the description of #216088 (closed), the CleanupContainerRepositoryWorker
is running way too many times with invalid regex (that are received as parameters).
This MR aims to reduce and even eliminate the situations where a CleanupContainerRepositoryWorker
job is enqueued with invalid regex.
The CleanupContainerRepositoryWorker
can be called by two components:
- By
ContainerExpirationPolicyService
, when executing automatically container expiration policies. - By
API::ProjectContainerRepositories
when the api receives a request to delete tags in bulk.
There is also a 3rd situation where a CleanupContainerRepositoryWorker
job with invalid regex raise a validation Error -> the job is re enqueued.
To prevent these scenarios, 3 guards have been implemented:
- The
ContainerExpirationPolicyService
will now enqueue aCleanupContainerRepositoryWorker
job only if the given container expiration policy is valid. In addition, if the container expiration is not valid, it will be disabled as we will don't need to schedule the next run on it. - A Grape API validator based on
Gitlab::UntrustedRegexp
has been added. This validator has been added toAPI::ProjectContainerRepositories
so that Grape API will automatically discard any request with invalid regex. - The service used by the
CleanupContainerRepositoryWorker
will now return an error response upon a regex validation error instead of raising an error which would bubble up and make the worker job fail (and re enqueue).
Screenshots
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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