Protected containers: Rename attributes to minimum_access_level_for_xxx
requested to merge gitlab-community/gitlab:427546-protected-containters-write-protection-for-container-repositories-follow-up-renmaing-access-level-related-attributes into master
What does this MR do and why?
- As discussed in a previous MR discussion, we see the need to rename the fields
push_protected_up_to_access_level
anddelete_protected_up_to_access_level
in order to apply a positive logic (instead of negative logic)- Field
push_protected_up_to_access_level
is renamed to fieldminimum_access_level_for_push
- Field
delete_protected_up_to_access_level
is renamed to fieldminimum_access_level_for_delete
- Field
- But, when renaming these fields, we also need to change the logic and values of the fields => hence, we need to also migrate the data; the following mapping describes the mapping, e.g. when the field
push_protected_up_to_access_level
is set to the access level:developer
(meaning that a certain container is push protected up to the access level:developer
) then we will need to set the access level to:maintainer
for the fieldminimum_access_level_for_push
(because the access level:maintainer
is the lowest access level that is allowed to push a certain package).
push_protected_up_to_access_level |
minimum_access_level_for_push |
---|---|
:developer |
:maintainer |
:maintainer |
:owner |
:owner |
:admin |
Open Questions
-
What should be the value of the field minimum_access_level_for_push
when the fieldpush_protected_up_to_access_level
was previously set to:owner
? => !139413 (comment 1792865139) -
Should we also allow the option :developer
? => !139413 (comment 1792865163)
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
MR Checklist (@gerardo-navarro)
-
Changelog entry added, if necessary -
Documentation created/updated via this MR -
Documentation reviewed by technical writer or follow-up review issue created -
Tests added for this feature/bug -
Tested in all supported browsers -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines -
Conforms to the style guides -
Conforms to the javascript style guides -
Conforms to the database guides
DB queries
As part of the migration, the following SQL query is used.
UPDATE "container_registry_protection_rules" SET "minimum_access_level_for_delete" = 60 WHERE "container_registry_protection_rules"."delete_protected_up_to_access_level" = 50
Screenshots or screen recordings
No frontend changes. This MR contains primarily changes in the backend .
How to set up and validate locally
- Enable the container registry, see https://gitlab.com/gitlab-org/gitlab-development-kit/blob/main/doc/howto/registry.md (<= setup as HTTP and not HTTPS)
- Migrate the database
rails db:migrate
- Enable feature flag via
rails c
Feature.enable(:container_registry_protected_containers)
- In
rails c
, create aContainerRegistry::Protection::Rule
to protect against pushing new container images
ContainerRegistry::Protection::Rule.create(project: Project.find(7), repository_path_pattern: "flightjs/flight", minimum_access_level_for_push: :admin, minimum_access_level_for_delete: :admin)
- Open another terminal session
- Prepare for pushing the docker cli and docker container image, see https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/main/doc/howto/registry.md#using-the-docker-client
docker login registry.test:5000 -u gitlab-token -p "ypCa3Dzb23o5nvsixwPA"
docker pull hello-world:latest
docker tag hello-world:latest registry.test:5000/flightjs/flight:v1.0
- Push the container image => will be blocked by
ContainerRepositoryProtectionRule
docker push registry.test:5000/flightjs/flight:v1.0
- This docker push should fail because the container registry protection rule protects the container repository from new images with the repository path
- Lets try pushing an image with another repository path
docker tag hello-world:latest registry.test:5000/flightjs/flight/sub-image:v1.0
docker push registry.test:5000/flightjs/flight/sub-image:v1.0
- This docker push should be successful because the container registry protection rule does not protect this new image.
- You can also change the value of the field
minimum_access_level_for_push
to:owner
and then it is possible to push all kind of packages
Related to #427546
Edited by Gerardo Navarro