Follow-up from "Resolve "API endpoint to list the Docker images/tags of a group""
Problem to solve
The Gitlab Container Registry allows users to build, publish and share Docker images right alongside their source code and pipelines. Users can view images by navigating to the Container Registry UI, which is currently available at the project level, with group level support coming soon. They can also use the Container Registry API to view images and tags associated with a given project or group.
gitlab-foss#26866 (closed) created the endpoint for viewing images at the group level. However, there are several follow-up items that need to be resolved in order to ensure scalability and remove technical debt.
This issue captures those enhancements.
Intended users
- Delaney (Development Team Lead)
- Sasha (Software Developer)
- Devon (DevOps Engineer)
- Sidney (Systems Administrator)
Further details
-
@ayufan started a discussion: Is there a reason why we pass
id:
andcontainer_type:
instead of passingsubject
and actual object?I think it should not be advised to perform manually a
.find(
.Especially, this is not really
container_type:
. It is ratherid_type:
, because it implies what is the meaning ofid:
.I would also say that this is advised from gitlab-ce~2779335 point of view to also pass
current_user
and perform additional policy check that given user does have permission to access this operation. -
@ayufan started a discussion: This is also quite bad approach, as we do not sanity check the
container_type
, this code will fail if we pass some random ID, and container_type: :unknown.If we want to follow, but I see not reason to, we should prefer to use
case-when
.case container_type when :project project.container_repositories when :group group.container_repositories else raise ArgumentError, "invalid container_type" end
-
@ayufan started a discussion: It does not take repositories from nested groups.
-
@ayufan started a discussion: We should also validate if container registry is enabled for the project.
We seem to skip that when looking at given project, but also when getting a container repositories for groups.
-
@ayufan started a discussion: Is there a reason why we don't use
set(:reporter)
andset(:guest)
?I think also for a readability purposes it would be advised to add empty line in the future between blocks that use
set
andlet
.