From 2d3c516e13e7a2ffbf6ea60a7ed263800e1e8989 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Tue, 19 May 2026 14:07:30 -0400 Subject: [PATCH 01/11] Switch push created repo to ContainerRepository https://redhat.atlassian.net/browse/PULP-1748 Assisted by: cursor composer 2 --- CHANGES/+default-push-repo.feature | 1 + docs/admin/learn/rbac.md | 49 ++---- pulp_container/app/authorization.py | 1 - .../app/global_access_conditions.py | 5 +- pulp_container/app/registry_api.py | 36 ++-- pulp_container/app/serializers.py | 2 +- pulp_container/app/viewsets.py | 159 +++++++++++++++--- .../tests/functional/api/test_domains.py | 2 +- .../tests/functional/api/test_flatpak.py | 4 +- .../tests/functional/api/test_push_content.py | 65 +++++-- .../functional/api/test_push_signatures.py | 4 +- .../api/test_rbac_push_repositories.py | 88 +++++++++- .../functional/api/test_rbac_repo_content.py | 12 +- .../functional/api/test_rbac_repo_versions.py | 74 +++++++- .../functional/api/test_recursive_remove.py | 11 +- .../functional/api/test_sign_manifests.py | 6 +- .../functional/api/test_tagging_images.py | 4 +- pulp_container/tests/functional/conftest.py | 27 +++ 18 files changed, 439 insertions(+), 111 deletions(-) create mode 100644 CHANGES/+default-push-repo.feature diff --git a/CHANGES/+default-push-repo.feature b/CHANGES/+default-push-repo.feature new file mode 100644 index 000000000..50d7078b9 --- /dev/null +++ b/CHANGES/+default-push-repo.feature @@ -0,0 +1 @@ +Changed the default created repository on push to be a ContainerRepository instead of a ContainerPushRepository. ContainerPushRepository will eventually be phased out in future releases. diff --git a/docs/admin/learn/rbac.md b/docs/admin/learn/rbac.md index d8f4d4056..ae0756337 100644 --- a/docs/admin/learn/rbac.md +++ b/docs/admin/learn/rbac.md @@ -349,8 +349,15 @@ repository is public, then anyone can `pull` from the repository. Distributions are Pulp resources that represent URLs where repositories can be consumed. Permissions for accessing specific container repositories are described in terms of permissions to access Container Distributions. Each time a new repository is pushed using `podman` or `docker`, -a Container Distribution is created. There is also a Container Push Repository created. Both of -these resources can be accessed using Pulp's API. +a Container Distribution and a Container Repository are created. Both resources can be accessed +using Pulp's API. Registry-pushed repositories inherit distribution and namespace permissions for +read and content-modifying API actions. + +!!! note + + Older versions of pulp-container created Container Push Repositories on docker pushes. Legacy + pushes still remain available under `/pulp/api/v3/repositories/container/container-push/`, but + will be migrated to Container Repositories in a future release. The creation of a new distribution creates three user groups that can access the distribution: Owners, Collaborators, and Consumers. The user that creates the distribution is automatically added to @@ -369,17 +376,10 @@ object permissions for the Distribution: "container.change_containerdistribution" ``` -The Owners group also has the following permissions for the Container Push Repository associated -with the Distribution: - -``` -"container.view_containerpushrepository" -"container.modify_content_containerpushrepository" -``` - -The owners of a Container Distribution have the ability to update and delete the repository -associated with the Distribution. They can also add/remove users from the groups associated with -the distribution. +Distribution roles grant access to the linked Container Repository through the distribution and +namespace permission checks described above. Distribution owners can update and delete the +repository associated with the distribution. They can also add/remove users from the roles +associated with the distribution. #### Distribution Collaborators @@ -392,14 +392,6 @@ following object permissions for the Distribution: "container.push_containerdistribution" ``` -The Collaborators group also has the following permissions for the Container Push Repository associated -with the Distribution: - -``` -"container.view_containerpushrepository" -"container.modify_content_containerpushrepository" -``` - Users in the Collaborator group can do everything that the owners can, with the exception for deleting the Distribution. @@ -413,14 +405,7 @@ object permissions for the distribution: "container.pull_containerdistribution" ``` -The Consumers group also has the following permissions for the Container Push Repository associated -with the Distribution: - -``` -"container.view_containerpushrepository" -``` - -Users in the Consumers group can the `pull` the repository. Users should only need to be added to +Users in the Consumers group can `pull` the repository. Users should only need to be added to this group if the Distribution has been configured with `private=True`. If the Distribution is public, then anyone can `pull` from the repository associated with the Distribution. @@ -484,8 +469,10 @@ permission on the Distribution: ``` Users that wish to be able to access the repository associated with the distribution with Pulp's -API need the following object level permission on the Container Push Repository: +API need one of the following: ``` -"container.view_containerpushrepository" +"container.view_containerrepository" (object-level repository role), or +"container.view_containerdistribution" (distribution consumer role or above), or +"container.namespace_view_containerdistribution" (namespace consumer role or above) ``` diff --git a/pulp_container/app/authorization.py b/pulp_container/app/authorization.py index 2054b16c0..c1dbdd145 100644 --- a/pulp_container/app/authorization.py +++ b/pulp_container/app/authorization.py @@ -295,7 +295,6 @@ def has_push_permissions(self, path): if not domain: return False - print("Checking push permissions for path ", path, "and domain ", domain.name) try: distribution = ContainerDistribution.objects.get(base_path=path, pulp_domain=domain) except ContainerDistribution.DoesNotExist: diff --git a/pulp_container/app/global_access_conditions.py b/pulp_container/app/global_access_conditions.py index 132e2e296..e3cf35e6c 100644 --- a/pulp_container/app/global_access_conditions.py +++ b/pulp_container/app/global_access_conditions.py @@ -28,9 +28,10 @@ def has_namespace_obj_perms(request, view, action, permission): if type(obj) is models.ContainerDistribution: namespace = obj.namespace return request.user.has_perm(permission, namespace) - elif type(obj) is models.ContainerPushRepository: + elif type(obj) is models.ContainerPushRepository or type(obj) is models.ContainerRepository: for dist in obj.distributions.all(): - if request.user.has_perm(permission, dist.cast().namespace): + namespace = dist.cast().namespace + if namespace and request.user.has_perm(permission, namespace): return True elif type(obj) is models.ContainerPullThroughDistribution: namespace = obj.namespace diff --git a/pulp_container/app/registry_api.py b/pulp_container/app/registry_api.py index 92adfcbc9..8c47c872b 100644 --- a/pulp_container/app/registry_api.py +++ b/pulp_container/app/registry_api.py @@ -45,7 +45,13 @@ from pulpcore.plugin import pulp_hashlib from pulpcore.plugin.exceptions import TimeoutException from pulpcore.plugin.files import PulpTemporaryUploadedFile -from pulpcore.plugin.models import Artifact, ContentArtifact, RemoteArtifact, UploadChunk +from pulpcore.plugin.models import ( + Artifact, + ContentArtifact, + RemoteArtifact, + Repository, + UploadChunk, +) from pulpcore.plugin.tasking import dispatch from pulpcore.plugin.util import get_domain, get_objects_for_user, get_url @@ -436,11 +442,9 @@ def get_dr_push(self, request, path, create=False): repository = distribution.repository if repository: repository = repository.cast() - if not repository.PUSH_ENABLED: - raise RepositoryInvalid(name=path, message="Repository is read-only.") elif create: with transaction.atomic(): - repository = serializers.ContainerPushRepositorySerializer.get_or_create( + repository = serializers.ContainerRepositorySerializer.get_or_create( {"name": path, "pulp_domain": domain} ) distribution.repository = repository @@ -451,11 +455,22 @@ def get_dr_push(self, request, path, create=False): def create_dr(self, path, request): domain = get_domain() + repository_types = [ + models.ContainerRepository.get_pulp_type(), + models.ContainerPushRepository.get_pulp_type(), + ] with transaction.atomic(): try: - repository = serializers.ContainerPushRepositorySerializer.get_or_create( - {"name": path, "pulp_domain": domain} - ) + # Handle new default of ContainerRepository and fallback old ContainerPushRepository + repository = Repository.objects.filter(name=path, pulp_domain=domain).first() + if repository: + if repository.pulp_type not in repository_types: + raise RepositoryInvalid(name=path, message="Repository is read-only.") + repository = repository.cast() + else: + repository = serializers.ContainerRepositorySerializer.get_or_create( + {"name": path, "pulp_domain": domain} + ) distribution = serializers.ContainerDistributionSerializer.get_or_create( {"base_path": path, "name": path, "pulp_domain": domain}, {"repository": get_url(repository)}, @@ -464,9 +479,10 @@ def create_dr(self, path, request): raise RepositoryInvalid(name=path, message="Repository is read-only.") if distribution.repository: - dist_repository = distribution.repository.cast() - if not dist_repository.PUSH_ENABLED or repository != dist_repository: - raise RepositoryInvalid(name=path, message="Repository is read-only.") + if repository.pk != distribution.repository.pk: + raise RepositoryInvalid( + name=path, message="Repository is not available for push." + ) else: distribution.repository = repository distribution.save() diff --git a/pulp_container/app/serializers.py b/pulp_container/app/serializers.py index bb2fc948d..ce1977440 100644 --- a/pulp_container/app/serializers.py +++ b/pulp_container/app/serializers.py @@ -250,7 +250,7 @@ class Meta: model = models.ContainerNamespace -class ContainerRepositorySerializer(RepositorySerializer): +class ContainerRepositorySerializer(RepositorySerializer, GetOrCreateSerializerMixin): """ Serializer for Container Repositories. """ diff --git a/pulp_container/app/viewsets.py b/pulp_container/app/viewsets.py index 61be19e8c..b302f28c4 100644 --- a/pulp_container/app/viewsets.py +++ b/pulp_container/app/viewsets.py @@ -173,6 +173,18 @@ def _repo_query_params(self, request, view, push_perm, mirror_perm): mirror_perm, repo ): repo_pks.append(repo.pk) + elif any( + request.user.has_perm(push_perm, dist.cast()) + or ( + dist.cast().namespace + and request.user.has_perm( + "container.namespace_view_containerdistribution", + dist.cast().namespace, + ) + ) + for dist in repo.distributions.all() + ): + repo_pks.append(repo.pk) return repo_pks def get_content_qs(self, qs, push_perm, mirror_perm): @@ -203,18 +215,46 @@ def get_content_qs(self, qs, push_perm, mirror_perm): models.ContainerDistribution.objects.filter(pulp_domain=get_domain()), ) ).only("pk") + domain = get_domain() allowed_mirror_repos = get_objects_for_user( self.request.user, mirror_perm, - models.ContainerRepository.objects.filter(pulp_domain=get_domain()), + models.ContainerRepository.objects.filter(pulp_domain=domain), + ).only("pk") + allowed_distributed_container_repos = models.ContainerRepository.objects.filter( + distributions__in=get_objects_for_user( + self.request.user, + push_perm, + models.ContainerDistribution.objects.filter(pulp_domain=domain), + ) ).only("pk") content_qs = qs.model.objects.filter( - Q(repositories__in=allowed_push_repos) | Q(repositories__in=allowed_mirror_repos) + Q(repositories__in=allowed_push_repos) + | Q(repositories__in=allowed_mirror_repos) + | Q(repositories__in=allowed_distributed_container_repos) ) return content_qs +def repository_deleted_with_distribution(distribution): + """ + Return (repository, serializer_name) when a distribution delete should also delete its repo. + + Push repositories are always removed with their distribution. Container repositories created + during a registry push (no remote, single distribution) follow the same lifecycle. + """ + if not distribution.repository: + return None + repository = distribution.repository.cast() + if repository.PUSH_ENABLED: + return repository, "ContainerPushRepositorySerializer" + if isinstance(repository, models.ContainerRepository) and not repository.remote: + if repository.distributions.count() <= 1: + return repository, "ContainerRepositorySerializer" + return None + + class TagViewSet(ContainerContentQuerySetMixin, ReadOnlyContentViewSet): """ ViewSet for Tag. @@ -657,6 +697,9 @@ class ContainerRepositoryViewSet( ): """ ViewSet for container repo. + + Repositories linked to a registry distribution inherit the distribution's access policy + for read and content-modifying actions, matching legacy push repository behavior. """ endpoint_name = "container" @@ -680,7 +723,11 @@ class ContainerRepositoryViewSet( "action": ["retrieve"], "principal": "authenticated", "effect": "allow", - "condition": "has_model_or_domain_or_obj_perms:container.view_containerrepository", + "condition_expression": [ + "has_model_or_domain_or_obj_perms:container.view_containerrepository or " + "has_namespace_obj_perms:container.namespace_view_containerdistribution or " + "has_distribution_perms:container.view_containerdistribution", + ], }, { "action": ["destroy"], @@ -695,9 +742,16 @@ class ContainerRepositoryViewSet( "action": ["update", "partial_update", "set_label", "unset_label"], "principal": "authenticated", "effect": "allow", - "condition": [ - "has_model_or_domain_or_obj_perms:container.change_containerrepository", - "has_model_or_domain_or_obj_perms:container.view_containerrepository", + "condition_expression": [ + "(" + "has_model_or_domain_or_obj_perms:container.change_containerrepository or " + "has_namespace_obj_perms:container.namespace_change_containerdistribution or " + "has_distribution_perms:container.change_containerdistribution" + ") and (" + "has_model_or_domain_or_obj_perms:container.view_containerrepository or " + "has_namespace_obj_perms:container.namespace_view_containerdistribution or " + "has_distribution_perms:container.view_containerdistribution" + ")", ], }, { @@ -714,9 +768,15 @@ class ContainerRepositoryViewSet( "action": ["add", "remove", "tag", "untag", "copy_tags", "copy_manifests", "sign"], "principal": "authenticated", "effect": "allow", - "condition": [ - "has_model_or_domain_or_obj_perms:container.modify_content_containerrepository", - "has_model_or_domain_or_obj_perms:container.view_containerrepository", + "condition_expression": [ + "(" + "has_model_or_domain_or_obj_perms:container.modify_content_containerrepository or " + "has_namespace_obj_perms:container.namespace_modify_content_containerrepository" + ") and (" + "has_model_or_domain_or_obj_perms:container.view_containerrepository or " + "has_namespace_obj_perms:container.namespace_view_containerdistribution or " + "has_distribution_perms:container.view_containerdistribution" + ")", ], }, { @@ -744,7 +804,13 @@ class ContainerRepositoryViewSet( "parameters": {"roles": "container.containerrepository_owner"}, }, ], - "queryset_scoping": {"function": "scope_queryset"}, + "queryset_scoping": { + "function": "get_container_repos_qs", + "parameters": { + "ns_perm": "container.view_containernamespace", + "dist_perm": "container.view_containerdistribution", + }, + }, } LOCKED_ROLES = { "container.containerrepository_creator": ["container.add_containerrepository"], @@ -770,6 +836,45 @@ class ContainerRepositoryViewSet( ], } + def get_container_repos_qs(self, qs, ns_perm, dist_perm): + """ + Scope repositories by direct permissions and linked distribution permissions. + + Mirrors push repository scoping so registry-pushed repositories remain visible to + distribution and namespace role holders. + """ + domain = get_domain() + qs = models.ContainerRepository.objects.filter(pulp_domain=domain) + namespaces = get_objects_for_user( + self.request.user, + ns_perm, + models.ContainerNamespace.objects.filter(pulp_domain=domain), + ) + ns_repository_pks = models.ContainerDistribution.objects.filter( + namespace__in=namespaces, + pulp_domain=domain, + ).values_list("repository") + dist_repository_pks = get_objects_for_user( + self.request.user, + dist_perm, + models.ContainerDistribution.objects.filter(pulp_domain=domain), + ).values_list("repository") + public_repository_pks = models.ContainerDistribution.objects.filter( + private=False, + pulp_domain=domain, + ).values_list("repository") + direct_repository_pks = get_objects_for_user( + self.request.user, + "container.view_containerrepository", + qs, + ).values_list("pk") + return qs.filter( + Q(pk__in=ns_repository_pks) + | Q(pk__in=dist_repository_pks) + | Q(pk__in=public_repository_pks) + | Q(pk__in=direct_repository_pks) + ) + # This decorator is necessary since a sync operation is asyncrounous and returns # the id and href of the sync task. @extend_schema( @@ -1012,7 +1117,11 @@ class ContainerRepositoryVersionViewSet(RepositoryVersionViewSet): "action": ["list", "retrieve"], "principal": "authenticated", "effect": "allow", - "condition": "has_repository_model_or_domain_or_obj_perms:container.view_containerrepository", # noqa + "condition_expression": [ + "has_repository_model_or_domain_or_obj_perms:container.view_containerrepository or " # noqa + "has_namespace_obj_perms:container.namespace_view_containerdistribution or " + "has_distribution_perms:container.view_containerdistribution", + ], }, { "action": ["destroy"], @@ -1415,18 +1524,19 @@ def get_dist_qs(self, qs, ns_perm, dist_perm): ) def destroy(self, request, pk, **kwargs): """ - Delete a distribution. If a push repository is associated to it, delete it as well. + Delete a distribution. If a push repository or push-created container repository is + associated to it, delete it as well. """ distribution = self.get_object() reservations = [distribution] instance_ids = [ (str(distribution.pk), "container", "ContainerDistributionSerializer"), ] - if distribution.repository and distribution.repository.cast().PUSH_ENABLED: - reservations.append(distribution.repository) - instance_ids.append( - (str(distribution.repository.pk), "container", "ContainerPushRepositorySerializer"), - ) + repo_delete = repository_deleted_with_distribution(distribution) + if repo_delete: + repository, serializer_name = repo_delete + reservations.append(repository) + instance_ids.append((str(repository.pk), "container", serializer_name)) async_result = dispatch( general_multi_delete, exclusive_resources=reservations, args=(instance_ids,) @@ -1673,7 +1783,8 @@ class ContainerNamespaceViewSet( def destroy(self, request, pk, **kwargs): """ Delete a Namespace with all distributions. - If a push repository is associated to any of its distributions, delete it as well. + If a push repository or push-created container repository is associated to any of its + distributions, delete it as well. """ namespace = self.get_object() reservations = [] @@ -1684,14 +1795,12 @@ def destroy(self, request, pk, **kwargs): instance_ids.append( (str(distribution.pk), "container", "ContainerDistributionSerializer"), ) - if distribution.repository and distribution.repository.cast().PUSH_ENABLED: - reservations.append(distribution.repository) + repo_delete = repository_deleted_with_distribution(distribution) + if repo_delete: + repository, serializer_name = repo_delete + reservations.append(repository) instance_ids.append( - ( - str(distribution.repository.pk), - "container", - "ContainerPushRepositorySerializer", - ), + (str(repository.pk), "container", serializer_name), ) reservations.append(namespace) diff --git a/pulp_container/tests/functional/api/test_domains.py b/pulp_container/tests/functional/api/test_domains.py index 982d5dbd0..5a8a0df8b 100644 --- a/pulp_container/tests/functional/api/test_domains.py +++ b/pulp_container/tests/functional/api/test_domains.py @@ -179,7 +179,7 @@ def mount_blob(blob, source, dest): local_repo = f"{domain1.name}/{source_repo}" local_registry.tag_and_push(image_path, local_repo) - repository = container_bindings.RepositoriesContainerPushApi.list( + repository = container_bindings.RepositoriesContainerApi.list( name=source_repo, pulp_domain=domain1.name ).results[0] blobs = container_bindings.ContentBlobsApi.list( diff --git a/pulp_container/tests/functional/api/test_flatpak.py b/pulp_container/tests/functional/api/test_flatpak.py index 5f9d706b9..81fd0bf95 100644 --- a/pulp_container/tests/functional/api/test_flatpak.py +++ b/pulp_container/tests/functional/api/test_flatpak.py @@ -61,7 +61,7 @@ def test_flatpak_install( registry_client, local_registry, container_namespace_api, - container_push_repository_api, + container_repository_api, container_tag_api, container_manifest_api, pulp_settings, @@ -84,7 +84,7 @@ def test_flatpak_install( namespace = container_namespace_api.list(name="pulptest").results[0] add_to_cleanup(container_namespace_api, namespace.pulp_href) - repo = container_push_repository_api.list(name="pulptest/oci-net.fishsoup.hello").results[0] + repo = container_repository_api.list(name="pulptest/oci-net.fishsoup.hello").results[0] tag = container_tag_api.list(repository_version=repo.latest_version_href).results[0] manifest = container_manifest_api.read(tag.tagged_manifest) diff --git a/pulp_container/tests/functional/api/test_push_content.py b/pulp_container/tests/functional/api/test_push_content.py index 3c1ce607f..3a1f3a5d2 100644 --- a/pulp_container/tests/functional/api/test_push_content.py +++ b/pulp_container/tests/functional/api/test_push_content.py @@ -122,20 +122,20 @@ def test_push_with_dist_perms( }, ) - assert container_bindings.RepositoriesContainerPushApi.list(name=repo_name).count == 1 + assert container_bindings.RepositoriesContainerApi.list(name=repo_name).count == 1 with user_creator: - assert container_bindings.RepositoriesContainerPushApi.list(name=repo_name).count == 1 + assert container_bindings.RepositoriesContainerApi.list(name=repo_name).count == 1 with user_dist_collaborator: - assert container_bindings.RepositoriesContainerPushApi.list(name=repo_name).count == 1 + assert container_bindings.RepositoriesContainerApi.list(name=repo_name).count == 1 with user_dist_consumer: - assert container_bindings.RepositoriesContainerPushApi.list(name=repo_name).count == 1 + assert container_bindings.RepositoriesContainerApi.list(name=repo_name).count == 1 with user_namespace_collaborator: - assert container_bindings.RepositoriesContainerPushApi.list(name=repo_name).count == 1 + assert container_bindings.RepositoriesContainerApi.list(name=repo_name).count == 1 with user_reader: - assert container_bindings.RepositoriesContainerPushApi.list(name=repo_name).count == 1 + assert container_bindings.RepositoriesContainerApi.list(name=repo_name).count == 1 with user_helpless: # "{repo_name}" turns out to be a public repository - assert container_bindings.RepositoriesContainerPushApi.list(name=repo_name).count == 1 + assert container_bindings.RepositoriesContainerApi.list(name=repo_name).count == 1 def test_push_with_view_perms( @@ -407,20 +407,49 @@ def test_push_to_existing_regular_repository( container_repository_factory, local_registry, registry_client, + container_bindings, full_path, ): - """ - Test the push to an existing non-push repository. - - It should fail to create a new push repository. - """ - container_repository_factory(name="foo") + """Test that push succeeds when a container repository already exists.""" + repository = container_repository_factory(name="foo") image_path = f"{REGISTRY_V2_REPO_PULP}:manifest_a" local_url = full_path("foo:1.0") registry_client.pull(image_path) - with pytest.raises(CalledProcessError): - local_registry.tag_and_push(image_path, local_url) + local_registry.tag_and_push(image_path, local_url) + + repository = container_bindings.RepositoriesContainerApi.read(repository.pulp_href) + tags = container_bindings.ContentTagsApi.list(repository_version=repository.latest_version_href) + assert tags.count == 1 + + +def test_push_to_existing_push_repository( + add_to_cleanup, + container_push_repository_factory, + local_registry, + registry_client, + container_bindings, + full_path, +): + """Test that push still works when a legacy ContainerPushRepository already exists.""" + repo_name = "legacy/push" + container_push_repository_factory(name=repo_name) + image_path = f"{REGISTRY_V2_REPO_PULP}:manifest_a" + local_url = full_path(f"{repo_name}:1.0") + + registry_client.pull(image_path) + local_registry.tag_and_push(image_path, local_url) + + assert container_bindings.RepositoriesContainerPushApi.list(name=repo_name).count == 1 + assert container_bindings.RepositoriesContainerApi.list(name=repo_name).count == 0 + + repository = container_bindings.RepositoriesContainerPushApi.list(name=repo_name).results[0] + tags = container_bindings.ContentTagsApi.list(repository_version=repository.latest_version_href) + assert tags.count == 1 + + distribution = container_bindings.DistributionsContainerApi.list(name=repo_name).results[0] + namespace = container_bindings.PulpContainerNamespacesApi.read(distribution.namespace) + add_to_cleanup(container_bindings.PulpContainerNamespacesApi, namespace.pulp_href) class TestPushManifestList: @@ -492,7 +521,7 @@ def test_push_manifest_list_v2s2( distribution = container_bindings.DistributionsContainerApi.list(name="foo_v2s2").results[0] add_to_cleanup(container_bindings.DistributionsContainerApi, distribution.pulp_href) - repo_version = container_bindings.RepositoriesContainerPushApi.read( + repo_version = container_bindings.RepositoriesContainerApi.read( distribution.repository ).latest_version_href @@ -544,7 +573,7 @@ def test_push_manifest_list_oci( distribution = container_bindings.DistributionsContainerApi.list(name="foo_oci").results[0] add_to_cleanup(container_bindings.DistributionsContainerApi, distribution.pulp_href) - repo_version = container_bindings.RepositoriesContainerPushApi.read( + repo_version = container_bindings.RepositoriesContainerApi.read( distribution.repository ).latest_version_href @@ -592,7 +621,7 @@ def test_push_empty_manifest_list( ] add_to_cleanup(container_bindings.DistributionsContainerApi, distribution.pulp_href) - repo_version = container_bindings.RepositoriesContainerPushApi.read( + repo_version = container_bindings.RepositoriesContainerApi.read( distribution.repository ).latest_version_href latest_tag = container_bindings.ContentTagsApi.list( diff --git a/pulp_container/tests/functional/api/test_push_signatures.py b/pulp_container/tests/functional/api/test_push_signatures.py index b20e5f91b..2d9e3e7f9 100644 --- a/pulp_container/tests/functional/api/test_push_signatures.py +++ b/pulp_container/tests/functional/api/test_push_signatures.py @@ -41,7 +41,7 @@ def distribution( def test_assert_signed_image( local_registry, - container_push_repository_api, + container_repository_api, container_manifest_api, container_signature_api, signing_gpg_metadata, @@ -51,7 +51,7 @@ def test_assert_signed_image( """Test whether an admin user can fetch a signature from the Pulp Registry.""" gpg, fingerprint, keyid = signing_gpg_metadata - repository = container_push_repository_api.read(distribution.repository) + repository = container_repository_api.read(distribution.repository) manifest = container_manifest_api.list( repository_version=repository.latest_version_href ).results[0] diff --git a/pulp_container/tests/functional/api/test_rbac_push_repositories.py b/pulp_container/tests/functional/api/test_rbac_push_repositories.py index 86841515e..75da63dee 100644 --- a/pulp_container/tests/functional/api/test_rbac_push_repositories.py +++ b/pulp_container/tests/functional/api/test_rbac_push_repositories.py @@ -7,8 +7,92 @@ from pulp_container.tests.functional.constants import REGISTRY_V2_REPO_PULP +def test_rbac_push_created_repository( + add_to_cleanup, + gen_user, + registry_client, + local_registry, + container_bindings, + full_path, + pulp_settings, + monitor_task, +): + """Verify RBAC for a ContainerRepository created by a registry push.""" + if pulp_settings.TOKEN_AUTH_DISABLED: + pytest.skip("RBAC cannot be tested when token authentication is disabled") + + namespace_name = str(uuid.uuid4()) + repo_name = f"{namespace_name}/perms" + local_url = full_path(f"{repo_name}:1.0") + + user_creator = gen_user( + model_roles=[ + "container.containerdistribution_creator", + "container.containernamespace_creator", + ] + ) + user_reader = gen_user(model_roles=["container.containerdistribution_consumer"]) + user_helpless = gen_user() + + image_path = f"{REGISTRY_V2_REPO_PULP}:manifest_d" + registry_client.pull(image_path) + with user_creator: + local_registry.tag_and_push(image_path, local_url) + repository = container_bindings.RepositoriesContainerApi.list(name=repo_name).results[0] + + namespace = container_bindings.PulpContainerNamespacesApi.list(name=namespace_name).results[0] + add_to_cleanup(container_bindings.PulpContainerNamespacesApi, namespace.pulp_href) + + """Read a repository by its href.""" + with user_creator: + container_bindings.RepositoriesContainerApi.read(repository.pulp_href) + with user_reader: + container_bindings.RepositoriesContainerApi.read(repository.pulp_href) + with user_helpless, pytest.raises(container_bindings.ApiException): + container_bindings.RepositoriesContainerApi.read(repository.pulp_href) + + """Read a repository by its name.""" + with user_creator: + page = container_bindings.RepositoriesContainerApi.list(name=repository.name) + assert len(page.results) == 1 + with user_reader: + page = container_bindings.RepositoriesContainerApi.list(name=repository.name) + assert len(page.results) == 1 + with user_helpless: + page = container_bindings.RepositoriesContainerApi.list(name=repository.name) + assert len(page.results) == 1 + + """Update a repository using HTTP PATCH.""" + body = {"description": "new_hotness"} + with user_helpless, pytest.raises(container_bindings.ApiException): + container_bindings.RepositoriesContainerApi.partial_update(repository.pulp_href, body) + with user_reader, pytest.raises(container_bindings.ApiException): + container_bindings.RepositoriesContainerApi.partial_update(repository.pulp_href, body) + with user_creator: + response = container_bindings.RepositoriesContainerApi.partial_update( + repository.pulp_href, body + ) + monitor_task(response.task) + repository = container_bindings.RepositoriesContainerApi.read(repository.pulp_href) + assert repository.description == body["description"] + + """Update a repository using HTTP PUT.""" + body = {"name": repository.name, "description": "old_busted"} + with user_helpless, pytest.raises(container_bindings.ApiException): + container_bindings.RepositoriesContainerApi.update(repository.pulp_href, body) + with user_reader, pytest.raises(container_bindings.ApiException): + container_bindings.RepositoriesContainerApi.update(repository.pulp_href, body) + with user_creator: + response = container_bindings.RepositoriesContainerApi.update(repository.pulp_href, body) + monitor_task(response.task) + with user_creator: + repository = container_bindings.RepositoriesContainerApi.read(repository.pulp_href) + assert repository.description == body["description"] + + def test_rbac_push_repository( add_to_cleanup, + container_push_repository_factory, gen_user, registry_client, local_registry, @@ -17,7 +101,7 @@ def test_rbac_push_repository( pulp_settings, monitor_task, ): - """Verify RBAC for a ContainerPushRepository.""" + """Verify RBAC for a legacy ContainerPushRepository.""" if pulp_settings.TOKEN_AUTH_DISABLED: pytest.skip("RBAC cannot be tested when token authentication is disabled") @@ -34,7 +118,7 @@ def test_rbac_push_repository( user_reader = gen_user(model_roles=["container.containerdistribution_consumer"]) user_helpless = gen_user() - # create a push repo + container_push_repository_factory(name=repo_name) image_path = f"{REGISTRY_V2_REPO_PULP}:manifest_d" registry_client.pull(image_path) with user_creator: diff --git a/pulp_container/tests/functional/api/test_rbac_repo_content.py b/pulp_container/tests/functional/api/test_rbac_repo_content.py index 67a0dfba0..44f613182 100644 --- a/pulp_container/tests/functional/api/test_rbac_repo_content.py +++ b/pulp_container/tests/functional/api/test_rbac_repo_content.py @@ -48,27 +48,27 @@ def test_rbac_repository_content( user_reader3 = gen_user(model_roles=["container.containerdistribution_consumer"]) user_helpless = gen_user() - # create a first push repo with user_creator + # create a first pushed container repo with user_creator image_path1 = f"{REGISTRY_V2_REPO_PULP}:manifest_a" registry_client.pull(image_path1) repo_name1 = "testcontent1/perms" local_url1 = full_path(f"{repo_name1}:1.0") with user_creator: local_registry.tag_and_push(image_path1, local_url1) - push_repository1 = container_bindings.RepositoriesContainerPushApi.list( + push_repository1 = container_bindings.RepositoriesContainerApi.list( name=repo_name1 ).results[0] distribution1 = container_bindings.DistributionsContainerApi.list(name=repo_name1).results[0] add_to_cleanup(container_bindings.PulpContainerNamespacesApi, distribution1.namespace) - # create a second push repo with user_creator2 + # create a second pushed container repo with user_creator2 image_path2 = f"{REGISTRY_V2_REPO_PULP}:manifest_b" registry_client.pull(image_path2) repo_name2 = "testcontent2/perms" local_url2 = full_path(f"{repo_name2}:1.0") with user_creator2: local_registry.tag_and_push(image_path2, local_url2) - push_repository2 = container_bindings.RepositoriesContainerPushApi.list( + push_repository2 = container_bindings.RepositoriesContainerApi.list( name=repo_name2 ).results[0] distribution2 = container_bindings.DistributionsContainerApi.list(name=repo_name2).results[0] @@ -85,10 +85,10 @@ def test_rbac_repository_content( monitor_task(sync_response.task) # Test that users can list content if they have enough permissions. - push_repository1_rv = container_bindings.RepositoriesContainerPushApi.read( + push_repository1_rv = container_bindings.RepositoriesContainerApi.read( push_repository1.pulp_href ).latest_version_href - push_repository2_rv = container_bindings.RepositoriesContainerPushApi.read( + push_repository2_rv = container_bindings.RepositoriesContainerApi.read( push_repository2.pulp_href ).latest_version_href repository_rv = container_bindings.RepositoriesContainerApi.read( diff --git a/pulp_container/tests/functional/api/test_rbac_repo_versions.py b/pulp_container/tests/functional/api/test_rbac_repo_versions.py index ed6f43417..1ca5d1637 100644 --- a/pulp_container/tests/functional/api/test_rbac_repo_versions.py +++ b/pulp_container/tests/functional/api/test_rbac_repo_versions.py @@ -130,8 +130,76 @@ def create_new_repo_version(): monitor_task(response.task) +def test_rbac_push_created_repository_version( + add_to_cleanup, + gen_user, + registry_client, + local_registry, + container_bindings, + full_path, + pulp_settings, +): + """Verify RBAC for a ContainerRepositoryVersion created by a registry push.""" + if pulp_settings.TOKEN_AUTH_DISABLED: + pytest.skip("RBAC cannot be tested when token authentication is disabled") + + try: + namespace = container_bindings.PulpContainerNamespacesApi.list( + name="test_push_created_repo" + ).results[0] + container_bindings.PulpContainerNamespacesApi.delete(namespace.pulp_href) + except IndexError: + pass + + user_creator = gen_user( + model_roles=[ + "container.containernamespace_creator", + ] + ) + user_reader = gen_user(model_roles=["container.containerdistribution_consumer"]) + user_helpless = gen_user() + + image_path = f"{REGISTRY_V2_REPO_PULP}:manifest_d" + registry_client.pull(image_path) + repo_name = "test_push_created_repo/perms" + local_url = full_path(f"{repo_name}:1.0") + with user_creator: + local_registry.tag_and_push(image_path, local_url) + repository = container_bindings.RepositoriesContainerApi.list(name=repo_name).results[0] + + add_to_cleanup( + container_bindings.PulpContainerNamespacesApi, + container_bindings.PulpContainerNamespacesApi.list(name="test_push_created_repo") + .results[0] + .pulp_href, + ) + + assert container_bindings.RepositoriesContainerVersionsApi.list(repository.pulp_href).count == 2 + with user_creator: + assert ( + container_bindings.RepositoriesContainerVersionsApi.list(repository.pulp_href).count + == 2 + ) + with user_reader: + assert ( + container_bindings.RepositoriesContainerVersionsApi.list(repository.pulp_href).count + == 2 + ) + with user_helpless, pytest.raises(container_bindings.ApiException): + container_bindings.RepositoriesContainerVersionsApi.list(repository.pulp_href) + + container_bindings.RepositoriesContainerVersionsApi.read(repository.latest_version_href) + with user_creator: + container_bindings.RepositoriesContainerVersionsApi.read(repository.latest_version_href) + with user_reader: + container_bindings.RepositoriesContainerVersionsApi.read(repository.latest_version_href) + with user_helpless, pytest.raises(container_bindings.ApiException): + container_bindings.RepositoriesContainerVersionsApi.read(repository.latest_version_href) + + def test_rbac_push_repository_version( add_to_cleanup, + container_push_repository_factory, gen_user, registry_client, local_registry, @@ -139,7 +207,7 @@ def test_rbac_push_repository_version( full_path, pulp_settings, ): - """Verify RBAC for a ContainerPushRepositoryVersion.""" + """Verify RBAC for a legacy ContainerPushRepositoryVersion.""" if pulp_settings.TOKEN_AUTH_DISABLED: pytest.skip("RBAC cannot be tested when token authentication is disabled") @@ -160,10 +228,10 @@ def test_rbac_push_repository_version( user_reader = gen_user(model_roles=["container.containerdistribution_consumer"]) user_helpless = gen_user() - # create a push repo image_path = f"{REGISTRY_V2_REPO_PULP}:manifest_d" registry_client.pull(image_path) repo_name = "test_push_repo/perms" + container_push_repository_factory(name=repo_name) local_url = full_path(f"{repo_name}:1.0") with user_creator: local_registry.tag_and_push(image_path, local_url) @@ -231,7 +299,7 @@ def test_cross_repository_blob_mount( image_path = f"{REGISTRY_V2_REPO_PULP}:manifest_a" registry_client.pull(image_path) local_registry.tag_and_push(image_path, local_url) - repository = container_bindings.RepositoriesContainerPushApi.list(name=source_repo).results[0] + repository = container_bindings.RepositoriesContainerApi.list(name=source_repo).results[0] blobs = container_bindings.ContentBlobsApi.list( repository_version=repository.latest_version_href ).results diff --git a/pulp_container/tests/functional/api/test_recursive_remove.py b/pulp_container/tests/functional/api/test_recursive_remove.py index 2af3a3a83..f6800f0dd 100644 --- a/pulp_container/tests/functional/api/test_recursive_remove.py +++ b/pulp_container/tests/functional/api/test_recursive_remove.py @@ -429,10 +429,17 @@ def test_cannot_remove_tagged_manifest( def test_remove_image_push_repo( - container_bindings, local_registry, registry_client, full_path, add_to_cleanup, monitor_task + container_push_repository_factory, + container_bindings, + local_registry, + registry_client, + full_path, + add_to_cleanup, + monitor_task, ): - """Test the image removal within a push repository.""" + """Test the image removal within a legacy push repository.""" # the image tagged as 'manifest_a' consists of 3 blobs, 1 manifest, and 1 tag + container_push_repository_factory(name="foo/bar") manifest_a_path = f"{REGISTRY_V2_REPO_PULP}:manifest_a" registry_client.pull(manifest_a_path) local_registry.tag_and_push(manifest_a_path, full_path("foo/bar:tag")) diff --git a/pulp_container/tests/functional/api/test_sign_manifests.py b/pulp_container/tests/functional/api/test_sign_manifests.py index 1706ccfff..e0bdaea60 100644 --- a/pulp_container/tests/functional/api/test_sign_manifests.py +++ b/pulp_container/tests/functional/api/test_sign_manifests.py @@ -10,7 +10,7 @@ def distribution( registry_client, local_registry, container_distribution_api, full_path, add_to_cleanup ): - """The fixture for a distribution that references a repository of the push type.""" + """The fixture for a distribution created by pushing an image to the registry.""" image_path = f"{REGISTRY_V2_REPO_PULP}:{MANIFEST_TAG}" registry_client.pull(image_path) local_registry.tag_and_push(image_path, full_path(f"test-1:{MANIFEST_TAG}")) @@ -25,7 +25,7 @@ def test_sign_manifest( signing_gpg_metadata, distribution, container_signing_service, - container_push_repository_api, + container_repository_api, container_signature_api, container_tag_api, container_manifest_api, @@ -35,7 +35,7 @@ def test_sign_manifest( _, fingerprint, keyid = signing_gpg_metadata sign_data = {"manifest_signing_service": container_signing_service.pulp_href} - response = container_push_repository_api.sign(distribution.repository, sign_data) + response = container_repository_api.sign(distribution.repository, sign_data) created_resources = monitor_task(response.task).created_resources tags = container_tag_api.list(repository_version=created_resources[0]) diff --git a/pulp_container/tests/functional/api/test_tagging_images.py b/pulp_container/tests/functional/api/test_tagging_images.py index 4b229c1cb..5b65d429d 100644 --- a/pulp_container/tests/functional/api/test_tagging_images.py +++ b/pulp_container/tests/functional/api/test_tagging_images.py @@ -201,7 +201,7 @@ def test_05_untag_second_image_again(self, container_bindings, setup): class TestPushRepositoryTagging: - """A test case for a container push repository.""" + """A test case for a container repository created by pushing to the registry.""" repository_name = "namespace/tags" @@ -225,7 +225,7 @@ def setup(self, tagger_helper, container_bindings, registry_client, full_path, a registry_client.push(tagged_registry_manifest_b) registry_client.logout(registry_name) - repository = container_bindings.RepositoriesContainerPushApi.list( + repository = container_bindings.RepositoriesContainerApi.list( name=self.repository_name ).results[0] tagger = tagger_helper(repository) diff --git a/pulp_container/tests/functional/conftest.py b/pulp_container/tests/functional/conftest.py index 13fe437e8..33c7c0b79 100644 --- a/pulp_container/tests/functional/conftest.py +++ b/pulp_container/tests/functional/conftest.py @@ -418,6 +418,33 @@ def container_repo(container_repository_factory): return container_repository_factory() +@pytest.fixture +def container_push_repository_factory(container_bindings): + """Create a ContainerPushRepository directly in the database. + + Push repositories have no create API; this fixture exists to test legacy + repositories created before pushes defaulted to ContainerRepository. + """ + + def _container_push_repository_factory(**body): + name = body.get("name", str(uuid4())) + pulp_domain = body.get("pulp_domain", "default") + script = ( + "from pulp_container.app.models import ContainerPushRepository; " + "from pulpcore.plugin.models import Domain; " + f"domain = Domain.objects.get(name='{pulp_domain}'); " + f"repo, _ = ContainerPushRepository.objects.get_or_create(" + f"name='{name}', pulp_domain=domain); " + ) + subprocess.check_output(("pulpcore-manager", "shell", "-c", script)) + kwargs = {"name": name} + if "pulp_domain" in body: + kwargs["pulp_domain"] = pulp_domain + return container_bindings.RepositoriesContainerPushApi.list(**kwargs).results[0] + + return _container_push_repository_factory + + @pytest.fixture(scope="class") def container_remote_factory(container_bindings, gen_object_with_cleanup): def _container_remote_factory(url=REGISTRY_V2_FEED_URL, **body): From 7b7c9ccd27255815587dbcc9d9461e3cb25f2f5d Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Mon, 1 Jun 2026 15:14:46 -0400 Subject: [PATCH 02/11] Fix CI failures for push-to-ContainerRepository change. Treat registry-pushed container repositories like push repos when signing, allow legacy push repo fixtures without a distribution, clean up test distributions, and update RBAC content expectations for repository viewers. Co-authored-by: Cursor --- pulp_container/app/serializers.py | 7 ++++++- pulp_container/tests/functional/api/test_push_content.py | 6 ++++++ .../tests/functional/api/test_rbac_repo_content.py | 6 +++--- pulp_container/tests/functional/conftest.py | 6 +++++- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/pulp_container/app/serializers.py b/pulp_container/app/serializers.py index ce1977440..ff3723cd7 100644 --- a/pulp_container/app/serializers.py +++ b/pulp_container/app/serializers.py @@ -984,7 +984,12 @@ def validate(self, data): } ) - if repository.PUSH_ENABLED: + registry_push = repository.PUSH_ENABLED or ( + isinstance(repository, models.ContainerRepository) + and not repository.remote + and repository.distributions.exists() + ) + if registry_push: if "future_base_path" in data: raise serializers.ValidationError( { diff --git a/pulp_container/tests/functional/api/test_push_content.py b/pulp_container/tests/functional/api/test_push_content.py index 3a1f3a5d2..5041c58af 100644 --- a/pulp_container/tests/functional/api/test_push_content.py +++ b/pulp_container/tests/functional/api/test_push_content.py @@ -404,6 +404,7 @@ def test_push_matching_username( def test_push_to_existing_regular_repository( + add_to_cleanup, container_repository_factory, local_registry, registry_client, @@ -422,6 +423,11 @@ def test_push_to_existing_regular_repository( tags = container_bindings.ContentTagsApi.list(repository_version=repository.latest_version_href) assert tags.count == 1 + distribution = container_bindings.DistributionsContainerApi.list(name="foo").results[0] + add_to_cleanup(container_bindings.DistributionsContainerApi, distribution.pulp_href) + namespace = container_bindings.PulpContainerNamespacesApi.read(distribution.namespace) + add_to_cleanup(container_bindings.PulpContainerNamespacesApi, namespace.pulp_href) + def test_push_to_existing_push_repository( add_to_cleanup, diff --git a/pulp_container/tests/functional/api/test_rbac_repo_content.py b/pulp_container/tests/functional/api/test_rbac_repo_content.py index 44f613182..86eeca0d1 100644 --- a/pulp_container/tests/functional/api/test_rbac_repo_content.py +++ b/pulp_container/tests/functional/api/test_rbac_repo_content.py @@ -132,14 +132,14 @@ def test_rbac_repository_content( assert container_bindings.ContentTagsApi.list(repository_version=repository_rv).count == 9 with user_reader2: - assert container_bindings.ContentTagsApi.list().count == 9 + assert container_bindings.ContentTagsApi.list().count == 11 assert ( container_bindings.ContentTagsApi.list(repository_version=push_repository1_rv).count - == 0 + == 1 ) assert ( container_bindings.ContentTagsApi.list(repository_version=push_repository2_rv).count - == 0 + == 1 ) assert container_bindings.ContentTagsApi.list(repository_version=repository_rv).count == 9 diff --git a/pulp_container/tests/functional/conftest.py b/pulp_container/tests/functional/conftest.py index 33c7c0b79..990315f94 100644 --- a/pulp_container/tests/functional/conftest.py +++ b/pulp_container/tests/functional/conftest.py @@ -440,7 +440,11 @@ def _container_push_repository_factory(**body): kwargs = {"name": name} if "pulp_domain" in body: kwargs["pulp_domain"] = pulp_domain - return container_bindings.RepositoriesContainerPushApi.list(**kwargs).results[0] + # Orphan legacy push repos have no distribution until the first registry push. + listed = container_bindings.RepositoriesContainerPushApi.list(**kwargs) + if listed.results: + return listed.results[0] + return None return _container_push_repository_factory From 6dfb489fc1a0c5886ec64604ace71c9f1b4fee35 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Mon, 1 Jun 2026 15:37:55 -0400 Subject: [PATCH 03/11] Allow distribution viewers to filter pushed repo content by version. Match ContainerPushRepository permission checks when scoping content queries by repository_version on registry-pushed container repositories. Co-authored-by: Cursor --- pulp_container/app/viewsets.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pulp_container/app/viewsets.py b/pulp_container/app/viewsets.py index b302f28c4..3be962e24 100644 --- a/pulp_container/app/viewsets.py +++ b/pulp_container/app/viewsets.py @@ -173,7 +173,7 @@ def _repo_query_params(self, request, view, push_perm, mirror_perm): mirror_perm, repo ): repo_pks.append(repo.pk) - elif any( + elif request.user.has_perm(push_perm) or any( request.user.has_perm(push_perm, dist.cast()) or ( dist.cast().namespace From f7492eaa4fcb6e770bf8c246f20d46a26d5413dd Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Mon, 1 Jun 2026 16:04:29 -0400 Subject: [PATCH 04/11] Scope distribution content access to registry-pushed repos only. Model-level view_containerdistribution must not grant repository_version filtering on synced container repositories that have a remote. Co-authored-by: Cursor --- pulp_container/app/viewsets.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/pulp_container/app/viewsets.py b/pulp_container/app/viewsets.py index 3be962e24..3970110fe 100644 --- a/pulp_container/app/viewsets.py +++ b/pulp_container/app/viewsets.py @@ -173,16 +173,19 @@ def _repo_query_params(self, request, view, push_perm, mirror_perm): mirror_perm, repo ): repo_pks.append(repo.pk) - elif request.user.has_perm(push_perm) or any( - request.user.has_perm(push_perm, dist.cast()) - or ( - dist.cast().namespace - and request.user.has_perm( - "container.namespace_view_containerdistribution", - dist.cast().namespace, + elif not repo.remote and ( + request.user.has_perm(push_perm) + or any( + request.user.has_perm(push_perm, dist.cast()) + or ( + dist.cast().namespace + and request.user.has_perm( + "container.namespace_view_containerdistribution", + dist.cast().namespace, + ) ) + for dist in repo.distributions.all() ) - for dist in repo.distributions.all() ): repo_pks.append(repo.pk) return repo_pks From 84bfba4f6dafdaf2197091aeb06f6959d6c86f4e Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Mon, 1 Jun 2026 16:27:46 -0400 Subject: [PATCH 05/11] Require a distribution for registry-pushed content RBAC checks. Distribution consumers may only filter content by repository_version on container repositories that have no remote and are tied to a distribution. Co-authored-by: Cursor --- pulp_container/app/viewsets.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/pulp_container/app/viewsets.py b/pulp_container/app/viewsets.py index 3970110fe..3d97e7a38 100644 --- a/pulp_container/app/viewsets.py +++ b/pulp_container/app/viewsets.py @@ -173,18 +173,22 @@ def _repo_query_params(self, request, view, push_perm, mirror_perm): mirror_perm, repo ): repo_pks.append(repo.pk) - elif not repo.remote and ( - request.user.has_perm(push_perm) - or any( - request.user.has_perm(push_perm, dist.cast()) - or ( - dist.cast().namespace - and request.user.has_perm( - "container.namespace_view_containerdistribution", - dist.cast().namespace, + elif ( + repo.remote_id is None + and repo.distributions.exists() + and ( + request.user.has_perm(push_perm) + or any( + request.user.has_perm(push_perm, dist.cast()) + or ( + dist.cast().namespace + and request.user.has_perm( + "container.namespace_view_containerdistribution", + dist.cast().namespace, + ) ) + for dist in repo.distributions.all() ) - for dist in repo.distributions.all() ) ): repo_pks.append(repo.pk) From 95cd5bb0f5d4fb39a26e058eed2c2bcdccb14010 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Mon, 1 Jun 2026 17:09:32 -0400 Subject: [PATCH 06/11] Keep repository viewers off registry-pushed container content. Exclude push-created container repositories from mirror-scoped tag access so distribution permissions govern that content instead. Co-authored-by: Cursor --- pulp_container/app/viewsets.py | 39 +++++++++++-------- .../functional/api/test_rbac_repo_content.py | 6 +-- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/pulp_container/app/viewsets.py b/pulp_container/app/viewsets.py index 3d97e7a38..7526cc87c 100644 --- a/pulp_container/app/viewsets.py +++ b/pulp_container/app/viewsets.py @@ -169,26 +169,26 @@ def _repo_query_params(self, request, view, push_perm, mirror_perm): ): repo_pks.append(repo.pk) elif isinstance(repo, models.ContainerRepository): - if request.user.has_perm(mirror_perm) or request.user.has_perm( - mirror_perm, repo + registry_pushed = ( + repo.remote_id is None and repo.distributions.exists() + ) + if not registry_pushed and ( + request.user.has_perm(mirror_perm) + or request.user.has_perm(mirror_perm, repo) ): repo_pks.append(repo.pk) - elif ( - repo.remote_id is None - and repo.distributions.exists() - and ( - request.user.has_perm(push_perm) - or any( - request.user.has_perm(push_perm, dist.cast()) - or ( - dist.cast().namespace - and request.user.has_perm( - "container.namespace_view_containerdistribution", - dist.cast().namespace, - ) + elif registry_pushed and ( + request.user.has_perm(push_perm) + or any( + request.user.has_perm(push_perm, dist.cast()) + or ( + dist.cast().namespace + and request.user.has_perm( + "container.namespace_view_containerdistribution", + dist.cast().namespace, ) - for dist in repo.distributions.all() ) + for dist in repo.distributions.all() ) ): repo_pks.append(repo.pk) @@ -223,10 +223,15 @@ def get_content_qs(self, qs, push_perm, mirror_perm): ) ).only("pk") domain = get_domain() + registry_pushed_repos = models.ContainerRepository.objects.filter( + pulp_domain=domain, remote_id__isnull=True + ).filter(distributions__isnull=False) allowed_mirror_repos = get_objects_for_user( self.request.user, mirror_perm, - models.ContainerRepository.objects.filter(pulp_domain=domain), + models.ContainerRepository.objects.filter(pulp_domain=domain).exclude( + pk__in=registry_pushed_repos + ), ).only("pk") allowed_distributed_container_repos = models.ContainerRepository.objects.filter( distributions__in=get_objects_for_user( diff --git a/pulp_container/tests/functional/api/test_rbac_repo_content.py b/pulp_container/tests/functional/api/test_rbac_repo_content.py index 86eeca0d1..44f613182 100644 --- a/pulp_container/tests/functional/api/test_rbac_repo_content.py +++ b/pulp_container/tests/functional/api/test_rbac_repo_content.py @@ -132,14 +132,14 @@ def test_rbac_repository_content( assert container_bindings.ContentTagsApi.list(repository_version=repository_rv).count == 9 with user_reader2: - assert container_bindings.ContentTagsApi.list().count == 11 + assert container_bindings.ContentTagsApi.list().count == 9 assert ( container_bindings.ContentTagsApi.list(repository_version=push_repository1_rv).count - == 1 + == 0 ) assert ( container_bindings.ContentTagsApi.list(repository_version=push_repository2_rv).count - == 1 + == 0 ) assert container_bindings.ContentTagsApi.list(repository_version=repository_rv).count == 9 From 2e30ab9993f7a6d2fe23b05da18a368c77e7f01e Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Mon, 1 Jun 2026 17:12:57 -0400 Subject: [PATCH 07/11] Apply black formatting to viewsets content RBAC change. Co-authored-by: Cursor --- pulp_container/app/viewsets.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pulp_container/app/viewsets.py b/pulp_container/app/viewsets.py index 7526cc87c..29e276966 100644 --- a/pulp_container/app/viewsets.py +++ b/pulp_container/app/viewsets.py @@ -169,9 +169,7 @@ def _repo_query_params(self, request, view, push_perm, mirror_perm): ): repo_pks.append(repo.pk) elif isinstance(repo, models.ContainerRepository): - registry_pushed = ( - repo.remote_id is None and repo.distributions.exists() - ) + registry_pushed = repo.remote_id is None and repo.distributions.exists() if not registry_pushed and ( request.user.has_perm(mirror_perm) or request.user.has_perm(mirror_perm, repo) From 27a87576c11f78b215f2fb5d8be7b7a0d16b7c38 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Thu, 4 Jun 2026 00:36:22 -0400 Subject: [PATCH 08/11] Add container repository namespace permissions, refactor access policies to be consistent --- .../0050_alter_containernamespace_options.py | 17 ++ pulp_container/app/models.py | 2 + pulp_container/app/viewsets.py | 231 ++++++------------ .../functional/api/test_rbac_repo_content.py | 59 ++--- 4 files changed, 114 insertions(+), 195 deletions(-) create mode 100644 pulp_container/app/migrations/0050_alter_containernamespace_options.py diff --git a/pulp_container/app/migrations/0050_alter_containernamespace_options.py b/pulp_container/app/migrations/0050_alter_containernamespace_options.py new file mode 100644 index 000000000..015710d83 --- /dev/null +++ b/pulp_container/app/migrations/0050_alter_containernamespace_options.py @@ -0,0 +1,17 @@ +# Generated by Django 5.2.13 on 2026-06-03 19:16 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('container', '0049_manifestsignature_fingerprint'), + ] + + operations = [ + migrations.AlterModelOptions( + name='containernamespace', + options={'permissions': [('namespace_add_containerdistribution', 'Add any distribution to a namespace'), ('namespace_delete_containerdistribution', 'Delete any distribution from a namespace'), ('namespace_view_containerdistribution', 'View any distribution in a namespace'), ('namespace_pull_containerdistribution', 'Pull from any distribution in a namespace'), ('namespace_push_containerdistribution', 'Push to any distribution in a namespace'), ('namespace_change_containerdistribution', 'Change any distribution in a namespace'), ('namespace_view_containerpushrepository', 'View any push repository in a namespace'), ('namespace_view_containerrepository', 'View any repository in a namespace'), ('namespace_modify_content_containerpushrepository', 'Modify content in any push repository in a namespace'), ('namespace_modify_content_containerrepository', 'Modify content in any repository in a namespace'), ('namespace_change_containerpushrepository', 'Update any existing push repository in a namespace'), ('namespace_change_containerrepository', 'Change any repository in a namespace'), ('manage_roles_containernamespace', 'Can manage role assignments on container namespace')]}, + ), + ] diff --git a/pulp_container/app/models.py b/pulp_container/app/models.py index 6ffc29f80..f98c18c95 100644 --- a/pulp_container/app/models.py +++ b/pulp_container/app/models.py @@ -458,6 +458,7 @@ class Meta: ("namespace_push_containerdistribution", "Push to any distribution in a namespace"), ("namespace_change_containerdistribution", "Change any distribution in a namespace"), ("namespace_view_containerpushrepository", "View any push repository in a namespace"), + ("namespace_view_containerrepository", "View any repository in a namespace"), ( "namespace_modify_content_containerpushrepository", "Modify content in any push repository in a namespace", @@ -470,6 +471,7 @@ class Meta: "namespace_change_containerpushrepository", "Update any existing push repository in a namespace", ), + ("namespace_change_containerrepository", "Change any repository in a namespace"), ( "manage_roles_containernamespace", "Can manage role assignments on container namespace", diff --git a/pulp_container/app/viewsets.py b/pulp_container/app/viewsets.py index 29e276966..5ecd5d31a 100644 --- a/pulp_container/app/viewsets.py +++ b/pulp_container/app/viewsets.py @@ -137,62 +137,7 @@ class ContainerContentQuerySetMixin: A mixin that provides container content models with querying utilities. """ - def _repo_query_params(self, request, view, push_perm, mirror_perm): - """ - Checks if the requests' query_params contain repository_version. - - This is used in the quryset scoping for content. - - Args: - request (rest_framework.request.Request): The request being made. - view (subclass rest_framework.viewsets.GenericViewSet): The view being checked for - authorization. - action (str): The action being performed, e.g. "destroy". - be checked. - - Returns: - List of repositories pk that the current user can view - - """ - repo_pks = [] - for key, param in request.query_params.items(): - if "repository_version" in key: - rv = NamedModelViewSet.get_resource(param, RepositoryVersion) - repo = rv.repository.cast() - if isinstance(repo, models.ContainerPushRepository): - if request.user.has_perm(push_perm) or any( - request.user.has_perm(push_perm, dist.cast()) - or request.user.has_perm( - "container.namespace_view_containerdistribution", dist.cast().namespace - ) - for dist in repo.distributions.all() - ): - repo_pks.append(repo.pk) - elif isinstance(repo, models.ContainerRepository): - registry_pushed = repo.remote_id is None and repo.distributions.exists() - if not registry_pushed and ( - request.user.has_perm(mirror_perm) - or request.user.has_perm(mirror_perm, repo) - ): - repo_pks.append(repo.pk) - elif registry_pushed and ( - request.user.has_perm(push_perm) - or any( - request.user.has_perm(push_perm, dist.cast()) - or ( - dist.cast().namespace - and request.user.has_perm( - "container.namespace_view_containerdistribution", - dist.cast().namespace, - ) - ) - for dist in repo.distributions.all() - ) - ): - repo_pks.append(repo.pk) - return repo_pks - - def get_content_qs(self, qs, push_perm, mirror_perm): + def get_content_qs(self, qs, ns_perm, dist_perm, repo_perm): """ Gets a QuerySet based on the current request. @@ -203,48 +148,16 @@ def get_content_qs(self, qs, push_perm, mirror_perm): allowed to see based on the repo permissions. """ - has_model_push_repo = self.request.user.has_perm(push_perm) - has_model_repo = self.request.user.has_perm(mirror_perm) + has_model_ns_perm = self.request.user.has_perm(ns_perm) + has_model_dist_perm = self.request.user.has_perm(dist_perm) + has_model_repo_perm = self.request.user.has_perm(repo_perm) # this will show also orphaned content - if has_model_push_repo and has_model_repo: + if has_model_ns_perm or (has_model_dist_perm and has_model_repo_perm): return qs - query_params = self.request.query_params - if query_params and "repository_version" in query_params: - repo_pks = self._repo_query_params(self.request, self, push_perm, mirror_perm) - content_qs = qs.model.objects.filter(repositories__in=repo_pks) - else: - allowed_push_repos = models.ContainerPushRepository.objects.filter( - distributions__in=get_objects_for_user( - self.request.user, - push_perm, - models.ContainerDistribution.objects.filter(pulp_domain=get_domain()), - ) - ).only("pk") - domain = get_domain() - registry_pushed_repos = models.ContainerRepository.objects.filter( - pulp_domain=domain, remote_id__isnull=True - ).filter(distributions__isnull=False) - allowed_mirror_repos = get_objects_for_user( - self.request.user, - mirror_perm, - models.ContainerRepository.objects.filter(pulp_domain=domain).exclude( - pk__in=registry_pushed_repos - ), - ).only("pk") - allowed_distributed_container_repos = models.ContainerRepository.objects.filter( - distributions__in=get_objects_for_user( - self.request.user, - push_perm, - models.ContainerDistribution.objects.filter(pulp_domain=domain), - ) - ).only("pk") - content_qs = qs.model.objects.filter( - Q(repositories__in=allowed_push_repos) - | Q(repositories__in=allowed_mirror_repos) - | Q(repositories__in=allowed_distributed_container_repos) - ) - return content_qs + repository_pks = get_viewable_repositories(self.request.user, ns_perm, dist_perm, repo_perm) + + return qs.filter(repositories__in=repository_pks) def repository_deleted_with_distribution(distribution): @@ -259,12 +172,48 @@ def repository_deleted_with_distribution(distribution): repository = distribution.repository.cast() if repository.PUSH_ENABLED: return repository, "ContainerPushRepositorySerializer" - if isinstance(repository, models.ContainerRepository) and not repository.remote: + if isinstance(repository, models.ContainerRepository): if repository.distributions.count() <= 1: return repository, "ContainerRepositorySerializer" return None +def get_viewable_repositories(user, ns_perm, dist_perm, repo_perm=None, domain=None): + """ + For a given user and namespace, distribution and repository permissions, return a set of + repository pks that the user can view. + """ + domain = domain or get_domain() + namespaces = get_objects_for_user( + user, + ns_perm, + models.ContainerNamespace.objects.filter(pulp_domain=domain), + ) + ns_repository_pks = models.ContainerDistribution.objects.filter( + namespace__in=namespaces, + pulp_domain=domain, + ).values_list("repository") + dist_repository_pks = get_objects_for_user( + user, + dist_perm, + models.ContainerDistribution.objects.filter(pulp_domain=domain), + ).values_list("repository") + public_repository_pks = models.ContainerDistribution.objects.filter( + private=False, + pulp_domain=domain, + ).values_list("repository") + if repo_perm: + direct_repository_pks = get_objects_for_user( + user, + repo_perm, + models.ContainerRepository.objects.filter(pulp_domain=domain), + ).values_list("pk") + else: + # PushContainerRepository case + direct_repository_pks = set() + return set(ns_repository_pks) | set(dist_repository_pks) | set(public_repository_pks) | set(direct_repository_pks) + + class TagViewSet(ContainerContentQuerySetMixin, ReadOnlyContentViewSet): """ ViewSet for Tag. @@ -299,8 +248,9 @@ class TagViewSet(ContainerContentQuerySetMixin, ReadOnlyContentViewSet): "queryset_scoping": { "function": "get_content_qs", "parameters": { - "push_perm": "container.view_containerdistribution", - "mirror_perm": "container.view_containerrepository", + "ns_perm": "container.view_containernamespace", + "dist_perm": "container.view_containerdistribution", + "repo_perm": "container.view_containerrepository", }, }, } @@ -340,8 +290,9 @@ class ManifestViewSet(ContainerContentQuerySetMixin, ReadOnlyContentViewSet): "queryset_scoping": { "function": "get_content_qs", "parameters": { - "push_perm": "container.view_containerdistribution", - "mirror_perm": "container.view_containerrepository", + "ns_perm": "container.view_containernamespace", + "dist_perm": "container.view_containerdistribution", + "repo_perm": "container.view_containerrepository", }, }, } @@ -381,8 +332,9 @@ class BlobViewSet(ContainerContentQuerySetMixin, ReadOnlyContentViewSet): "queryset_scoping": { "function": "get_content_qs", "parameters": { - "push_perm": "container.view_containerdistribution", - "mirror_perm": "container.view_containerrepository", + "ns_perm": "container.view_containernamespace", + "dist_perm": "container.view_containerdistribution", + "repo_perm": "container.view_containerrepository", }, }, } @@ -422,8 +374,9 @@ class ManifestSignatureViewSet(ContainerContentQuerySetMixin, ReadOnlyContentVie "queryset_scoping": { "function": "get_content_qs", "parameters": { - "push_perm": "container.view_containerdistribution", - "mirror_perm": "container.view_containerrepository", + "ns_perm": "container.view_containernamespace", + "dist_perm": "container.view_containerdistribution", + "repo_perm": "container.view_containerrepository", }, }, } @@ -735,7 +688,7 @@ class ContainerRepositoryViewSet( "effect": "allow", "condition_expression": [ "has_model_or_domain_or_obj_perms:container.view_containerrepository or " - "has_namespace_obj_perms:container.namespace_view_containerdistribution or " + "has_namespace_obj_perms:container.namespace_view_containerrepository or " "has_distribution_perms:container.view_containerdistribution", ], }, @@ -755,11 +708,11 @@ class ContainerRepositoryViewSet( "condition_expression": [ "(" "has_model_or_domain_or_obj_perms:container.change_containerrepository or " - "has_namespace_obj_perms:container.namespace_change_containerdistribution or " + "has_namespace_obj_perms:container.namespace_change_containerrepository or " "has_distribution_perms:container.change_containerdistribution" ") and (" "has_model_or_domain_or_obj_perms:container.view_containerrepository or " - "has_namespace_obj_perms:container.namespace_view_containerdistribution or " + "has_namespace_obj_perms:container.namespace_view_containerrepository or " "has_distribution_perms:container.view_containerdistribution" ")", ], @@ -819,6 +772,7 @@ class ContainerRepositoryViewSet( "parameters": { "ns_perm": "container.view_containernamespace", "dist_perm": "container.view_containerdistribution", + "repo_perm": "container.view_containerrepository", }, }, } @@ -846,7 +800,7 @@ class ContainerRepositoryViewSet( ], } - def get_container_repos_qs(self, qs, ns_perm, dist_perm): + def get_container_repos_qs(self, qs, ns_perm, dist_perm, repo_perm): """ Scope repositories by direct permissions and linked distribution permissions. @@ -855,35 +809,7 @@ def get_container_repos_qs(self, qs, ns_perm, dist_perm): """ domain = get_domain() qs = models.ContainerRepository.objects.filter(pulp_domain=domain) - namespaces = get_objects_for_user( - self.request.user, - ns_perm, - models.ContainerNamespace.objects.filter(pulp_domain=domain), - ) - ns_repository_pks = models.ContainerDistribution.objects.filter( - namespace__in=namespaces, - pulp_domain=domain, - ).values_list("repository") - dist_repository_pks = get_objects_for_user( - self.request.user, - dist_perm, - models.ContainerDistribution.objects.filter(pulp_domain=domain), - ).values_list("repository") - public_repository_pks = models.ContainerDistribution.objects.filter( - private=False, - pulp_domain=domain, - ).values_list("repository") - direct_repository_pks = get_objects_for_user( - self.request.user, - "container.view_containerrepository", - qs, - ).values_list("pk") - return qs.filter( - Q(pk__in=ns_repository_pks) - | Q(pk__in=dist_repository_pks) - | Q(pk__in=public_repository_pks) - | Q(pk__in=direct_repository_pks) - ) + return qs.filter(pk__in=get_viewable_repositories(self.request.user, ns_perm, dist_perm, repo_perm)) # This decorator is necessary since a sync operation is asyncrounous and returns # the id and href of the sync task. @@ -1129,7 +1055,7 @@ class ContainerRepositoryVersionViewSet(RepositoryVersionViewSet): "effect": "allow", "condition_expression": [ "has_repository_model_or_domain_or_obj_perms:container.view_containerrepository or " # noqa - "has_namespace_obj_perms:container.namespace_view_containerdistribution or " + "has_namespace_obj_perms:container.namespace_view_containerrepository or " "has_distribution_perms:container.view_containerdistribution", ], }, @@ -1291,29 +1217,7 @@ def get_push_repos_qs(self, qs, ns_perm, dist_perm): """ domain = get_domain() qs = models.ContainerPushRepository.objects.filter(pulp_domain=domain) - namespaces = get_objects_for_user( - self.request.user, - ns_perm, - models.ContainerNamespace.objects.filter(pulp_domain=domain), - ) - ns_repository_pks = models.ContainerDistribution.objects.filter( - namespace__in=namespaces, - pulp_domain=domain, - ).values_list("repository") - dist_repository_pks = get_objects_for_user( - self.request.user, - dist_perm, - models.ContainerDistribution.objects.filter(pulp_domain=domain), - ).values_list("repository") - public_repository_pks = models.ContainerDistribution.objects.filter( - private=False, - pulp_domain=domain, - ).values_list("repository") - return qs.filter( - Q(pk__in=ns_repository_pks) - | Q(pk__in=dist_repository_pks) - | Q(pk__in=public_repository_pks) - ) + return qs.filter(pk__in=get_viewable_repositories(self.request.user, ns_perm, dist_perm)) class ContainerPushRepositoryVersionViewSet( @@ -1760,9 +1664,11 @@ class ContainerNamespaceViewSet( "container.namespace_push_containerdistribution", "container.namespace_change_containerdistribution", "container.namespace_view_containerpushrepository", + "container.namespace_view_containerrepository", "container.namespace_modify_content_containerpushrepository", "container.namespace_modify_content_containerrepository", "container.namespace_change_containerpushrepository", + "container.namespace_change_containerrepository", "container.manage_roles_containernamespace", ], "container.containernamespace_collaborator": [ @@ -1774,15 +1680,18 @@ class ContainerNamespaceViewSet( "container.namespace_push_containerdistribution", "container.namespace_change_containerdistribution", "container.namespace_view_containerpushrepository", + "container.namespace_view_containerrepository", "container.namespace_modify_content_containerpushrepository", "container.namespace_modify_content_containerrepository", "container.namespace_change_containerpushrepository", + "container.namespace_change_containerrepository", ], "container.containernamespace_consumer": [ "container.view_containernamespace", "container.namespace_view_containerdistribution", "container.namespace_pull_containerdistribution", "container.namespace_view_containerpushrepository", + "container.namespace_view_containerrepository", ], } diff --git a/pulp_container/tests/functional/api/test_rbac_repo_content.py b/pulp_container/tests/functional/api/test_rbac_repo_content.py index 44f613182..5d2be2902 100644 --- a/pulp_container/tests/functional/api/test_rbac_repo_content.py +++ b/pulp_container/tests/functional/api/test_rbac_repo_content.py @@ -1,7 +1,7 @@ """Tests that verify that RBAC for content works properly.""" import pytest - +import uuid from pulpcore.client.pulp_container import SetLabel, UnsetLabel from pulpcore.client.pulp_container.exceptions import ForbiddenException @@ -38,20 +38,14 @@ def test_rbac_repository_content( "container.containerremote_creator", ] ) - user_reader = gen_user( - model_roles=[ - "container.containerrepository_viewer", - "container.containerdistribution_consumer", - ] - ) - user_reader2 = gen_user(model_roles=["container.containerrepository_viewer"]) - user_reader3 = gen_user(model_roles=["container.containerdistribution_consumer"]) + user_reader = gen_user(model_roles=["container.containerrepository_viewer"]) + user_reader2 = gen_user(model_roles=["container.containerdistribution_consumer"]) user_helpless = gen_user() # create a first pushed container repo with user_creator image_path1 = f"{REGISTRY_V2_REPO_PULP}:manifest_a" registry_client.pull(image_path1) - repo_name1 = "testcontent1/perms" + repo_name1 = f"{uuid.uuid4()}/perms" local_url1 = full_path(f"{repo_name1}:1.0") with user_creator: local_registry.tag_and_push(image_path1, local_url1) @@ -64,7 +58,7 @@ def test_rbac_repository_content( # create a second pushed container repo with user_creator2 image_path2 = f"{REGISTRY_V2_REPO_PULP}:manifest_b" registry_client.pull(image_path2) - repo_name2 = "testcontent2/perms" + repo_name2 = f"{uuid.uuid4()}/perms" local_url2 = full_path(f"{repo_name2}:1.0") with user_creator2: local_registry.tag_and_push(image_path2, local_url2) @@ -73,7 +67,12 @@ def test_rbac_repository_content( ).results[0] distribution2 = container_bindings.DistributionsContainerApi.list(name=repo_name2).results[0] add_to_cleanup(container_bindings.PulpContainerNamespacesApi, distribution2.namespace) - + # update the distribution to make it private + with user_creator2: + task = container_bindings.DistributionsContainerApi.partial_update( + distribution2.pulp_href, {"private": True} + ) + monitor_task(task.task) # sync a repo with user_creator with user_creator: remote = container_remote_factory(upstream_name=PULP_FIXTURE_1) @@ -95,7 +94,9 @@ def test_rbac_repository_content( repository.pulp_href ).latest_version_href + # Any repo with a distribution that is not private is public and can be viewed by anyone with user_creator: + # Their push repo (public) and their synced repo (1 + 9) assert container_bindings.ContentTagsApi.list().count == 10 assert ( container_bindings.ContentTagsApi.list(repository_version=push_repository1_rv).count @@ -108,10 +109,11 @@ def test_rbac_repository_content( assert container_bindings.ContentTagsApi.list(repository_version=repository_rv).count == 9 with user_creator2: - assert container_bindings.ContentTagsApi.list().count == 1 + # Their push repo (private) and the public push repo of user 1 (1 + 1) + assert container_bindings.ContentTagsApi.list().count == 2 assert ( container_bindings.ContentTagsApi.list(repository_version=push_repository1_rv).count - == 0 + == 1 ) assert ( container_bindings.ContentTagsApi.list(repository_version=push_repository2_rv).count @@ -120,6 +122,7 @@ def test_rbac_repository_content( assert container_bindings.ContentTagsApi.list(repository_version=repository_rv).count == 0 with user_reader: + # Model level repo permission allows viewing any repo content (1 + 1 + 9) assert container_bindings.ContentTagsApi.list().count == 11 assert ( container_bindings.ContentTagsApi.list(repository_version=push_repository1_rv).count @@ -132,18 +135,7 @@ def test_rbac_repository_content( assert container_bindings.ContentTagsApi.list(repository_version=repository_rv).count == 9 with user_reader2: - assert container_bindings.ContentTagsApi.list().count == 9 - assert ( - container_bindings.ContentTagsApi.list(repository_version=push_repository1_rv).count - == 0 - ) - assert ( - container_bindings.ContentTagsApi.list(repository_version=push_repository2_rv).count - == 0 - ) - assert container_bindings.ContentTagsApi.list(repository_version=repository_rv).count == 9 - - with user_reader3: + # Model level distro permission allows viewing any distro content (1 + 1) assert container_bindings.ContentTagsApi.list().count == 2 assert ( container_bindings.ContentTagsApi.list(repository_version=push_repository1_rv).count @@ -156,10 +148,11 @@ def test_rbac_repository_content( assert container_bindings.ContentTagsApi.list(repository_version=repository_rv).count == 0 with user_helpless: - assert container_bindings.ContentTagsApi.list().count == 0 + # No permissions so only public repos are visible (1) + assert container_bindings.ContentTagsApi.list().count == 1 assert ( container_bindings.ContentTagsApi.list(repository_version=push_repository1_rv).count - == 0 + == 1 ) assert ( container_bindings.ContentTagsApi.list(repository_version=push_repository2_rv).count @@ -170,18 +163,16 @@ def test_rbac_repository_content( # Test that users can read specific content if they have enough permissions. pushed_tag = container_bindings.ContentTagsApi.list( - repository_version_added=push_repository1.latest_version_href + repository_version_added=push_repository2_rv ).results[0] container_bindings.ContentTagsApi.read(pushed_tag.pulp_href) - with user_creator: + with user_creator, pytest.raises(container_bindings.ApiException): container_bindings.ContentTagsApi.read(pushed_tag.pulp_href) - with user_creator2, pytest.raises(container_bindings.ApiException): + with user_creator2: container_bindings.ContentTagsApi.read(pushed_tag.pulp_href) with user_reader: container_bindings.ContentTagsApi.read(pushed_tag.pulp_href) - with user_reader2, pytest.raises(container_bindings.ApiException): - container_bindings.ContentTagsApi.read(pushed_tag.pulp_href) - with user_reader3: + with user_reader2: container_bindings.ContentTagsApi.read(pushed_tag.pulp_href) with user_helpless, pytest.raises(container_bindings.ApiException): container_bindings.ContentTagsApi.read(pushed_tag.pulp_href) From 4aa0d3f26652530d08a1ba0dea569b0e45d5380a Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Thu, 4 Jun 2026 00:38:20 -0400 Subject: [PATCH 09/11] Update rbac docs --- docs/admin/learn/rbac.md | 842 ++++++++++++++++++++------------------- 1 file changed, 431 insertions(+), 411 deletions(-) diff --git a/docs/admin/learn/rbac.md b/docs/admin/learn/rbac.md index ae0756337..4286bce9a 100644 --- a/docs/admin/learn/rbac.md +++ b/docs/admin/learn/rbac.md @@ -4,475 +4,495 @@ Role-based access control (RBAC) **restricts** access to entities based on a use organization. A role consists of one or more permissions. Users having a proper set of roles can view, modify, or delete resources hosted on different endpoints. -By default, container repositories' content is accessible via `podman` or `docker` pull -commands, unless the opposite is *explicitly* specified. A private repository can be created via the -REST API for container distributions. An existing distribution can be updated with the parameter -`private=True`. +By default, the content in container repositories that have been distributed is public and accessible with `podman` or `docker pull`. Unless the container distribution is marked `private=True`, anyone can view and download the images available in these repositories. Push and delete permission is restricted to those with the *Owner* or *Collaborator* roles on the image's namespace or distribution. !!! note - Users logged in as administrators (superusers) always bypass any authorization checks. -## Roles - -Role based access control (RBAC) is configured using access policies for the following endpoints: - -- `pulp_container/namespaces` -- `distributions/container/container` -- `repositories/container/container-push` -- `remotes/container/container` -- `repositories/container/container` -- `repositories/container/container-push/versions` -- `repositories/container/container/versions` -- `content/container/blobs` -- `content/container/manifests` -- `content/container/tags` - -### Default Roles - -For each endpoint, a different set of roles is defined. The roles can be assigned at the model -or object level for every user or group. In the following sections, the *Creator*, *Owner*, -*Consumer*, and *Collaborator* roles are introduced. The *Consumer* and *Collaborator* roles are -defined only for namespaces and distributions (i.e., container repositories served by the Pulp Registry). - -#### Creator Role - -The *Creator* role contains the `add` permission for objects present on a particular endpoint. -For the distributions endpoint, only users with the `container.add_containerdistribution` -permission can create objects: - -```bash -pulp role show --name "container.containerdistribution_creator" -``` - -``` -{ - "pulp_href": "/pulp/api/v3/roles/1a8555c8-3bfc-4688-81e3-5bf6fa38b9d7/", - "pulp_created": "2022-05-26T12:02:28.872667Z", - "name": "container.containerdistribution_creator", - "description": null, - "permissions": [ - "container.add_containerdistribution" - ], - "locked": true -} -``` - -To perform operations on an endpoint (aka ViewSet actions), a user may need to have additional -permissions. One of the following *conditions* need to be satisfied to create a new distribution: - -```bash -pulp access-policy show --viewset-name "distributions/container/container" | jq -r '.statements[] | select(.action[] | contains("create"))' -``` - -``` -{ - "action": [ - "create" - ], - "effect": "allow", - "condition": "has_namespace_model_perms", - "principal": "authenticated" -} -{ - "action": [ - "create" - ], - "effect": "allow", - "condition": "has_namespace_perms:container.add_containerdistribution", - "principal": "authenticated" -} -{ - "action": [ - "create" - ], - "effect": "allow", - "condition": "namespace_is_username", - "principal": "authenticated" -} -``` +## Namespaces + +Permissions in `pulp-container` start with Namespaces, a grouping primative used to tie docker repositories together under a shared organization or team. In `pulp-container` the namespace is always the first part of an image's name, e.g. image `foo/hello` is under the namespace `foo` and image `bar` is under the namespace `bar`. Each container distribution links to a namespace with the same name as the first part of the distribution's `base_path` (since `base_path` is what determines the image's name). + +The default access policy for `pulp_container/namespaces` requires a user to have the `container.containernamespace_creator` role to create a new namespace. If the user has permission, then namespaces are automatically created and linked when a new distribution is created, either through the Pulp API or through a `docker push`. Namespaces can be created manually through the Pulp API before any image pushes. Also, a user is allowed to create a namespace that matches their username if it did not exist before, which they will become the owner of upon creation. + +Push and pull (for private repositories) permissions are first checked through the user's namespace permissions. There are three main roles, `container.containernamespace_owner`, `container.containernamespace_collaborator` and `container.containernamespace_consumer`, each with a different set of permissions for common usecases. Each role can be assigned at the model (global), domain, or object level. Roles assigned at the model level grant that role's permissions across *all* namespaces in Pulp, domain level across all namespaces in that domain, and object level with only permissions for just that one namespace. + +### Owner Role + +The *Owner* role contains all of the permissions available for the namespace except the `add` permission. + +=== "Show Owner Role" + + ```bash + pulp role show --name "container.containernamespace_owner" + ``` + +=== "Output" + + ```json + { + "pulp_href": "/pulp/api/v3/roles/019e8eea-9bd5-7a59-9ac5-aeed87729931/", + "prn": "prn:core.role:019e8eea-9bd5-7a59-9ac5-aeed87729931", + "pulp_created": "2026-06-03T19:16:40.534341Z", + "pulp_last_updated": "2026-06-03T19:25:27.207036Z", + "name": "container.containernamespace_owner", + "description": null, + "permissions": [ + "container.delete_containernamespace", + "container.manage_roles_containernamespace", + "container.namespace_add_containerdistribution", + "container.namespace_change_containerdistribution", + "container.namespace_change_containerpushrepository", + "container.namespace_change_containerrepository", + "container.namespace_delete_containerdistribution", + "container.namespace_modify_content_containerpushrepository", + "container.namespace_modify_content_containerrepository", + "container.namespace_pull_containerdistribution", + "container.namespace_push_containerdistribution", + "container.namespace_view_containerdistribution", + "container.namespace_view_containerpushrepository", + "container.namespace_view_containerrepository", + "container.view_containernamespace" + ], + "locked": true + } + ``` + +User's with the owner role can manage everything about the namespace, perform any action on images within the namespace, and manage the roles of other users for this namespace. The owner role is the strongest role (outside of being an admin) and is automatically assigned to the user who created the namespace. !!! note - - A user with the *Creator* role for namespaces does not need to have any additional roles to - create distributions within the given namespaces. Similarly, the user is allowed to create - distributions within the owning username namespace (e.g., user `alice` can create container - repositories like `alice/repo1`). - - -#### Owner Role - -The *Owner* role contains all of the permissions available for the associated ViewSet apart from -the `add` permission. For the ViewSet hosting namespaces, the set of permissions reads: - -```bash -pulp role show --name "container.containernamespace_owner" -``` - -``` -{ - "pulp_href": "/pulp/api/v3/roles/1f5519f5-5b2d-47cc-b081-2f38f256740e/", - "pulp_created": "2022-05-26T12:02:28.999330Z", - "name": "container.containernamespace_owner", - "description": null, - "permissions": [ - "container.delete_containernamespace", - "container.manage_roles_containernamespace", - "container.namespace_add_containerdistribution", - "container.namespace_change_containerdistribution", - "container.namespace_change_containerpushrepository", - "container.namespace_delete_containerdistribution", - "container.namespace_modify_content_containerpushrepository", - "container.namespace_pull_containerdistribution", - "container.namespace_push_containerdistribution", - "container.namespace_view_containerdistribution", - "container.namespace_view_containerpushrepository", - "container.view_containernamespace" - ], - "locked": true -} -``` - -Besides the permissions for *Read*, *Update*, and *Delete* actions, the *Owner* role has the -`mange_roles` permission that allows the user to call the ViewSet's `add_role` and -`remove_role` endpoints for easy management of roles around that ViewSet's object. - -The *Owner* role for namespaces contains permissions for any additional action that can be performed -on the related endpoints. The endpoints serving content for container clients permit access to -container distributions/repositories based on the presence of `pull_containerdistribution` and -`push_containerdistributuion` permissions. - -!!! note - - Repositories of the push type created with container clients (e.g., by running `podman push`) - are considered public and anyone can `pull` the images from them. - - -#### Consumer Role - -The *Consumer* role contains only the `view` and `pull` permissions. Below, a list of associated -permissions for distributions is showcased: - -```bash -pulp role show --name "container.containerdistribution_consumer" -``` - -``` -{ - "pulp_href": "/pulp/api/v3/roles/7b97928a-5d33-454f-982e-41cfe102b273/", - "pulp_created": "2022-05-26T12:02:28.945828Z", - "name": "container.containerdistribution_consumer", - "description": null, - "permissions": [ - "container.pull_containerdistribution", - "container.view_containerdistribution" - ], - "locked": true -} -``` - -Having the `view` and `pull` permissions allows a user to see and pull content from the Pulp + Creating the namespace requires the `container.containernamespace_creator` role or to have the same username as the name of the namespace to be created. + +### Consumer Role + +The *Consumer* role contains only the `view` and `pull` permissions. + +=== "Show Consumer Role" + + ```bash + pulp role show --name "container.containernamespace_consumer" + ``` + +=== "Output" + + ```json + { + "pulp_href": "/pulp/api/v3/roles/019e8eea-9bf1-7952-a621-d9495b479a31/", + "prn": "prn:core.role:019e8eea-9bf1-7952-a621-d9495b479a31", + "pulp_created": "2026-06-03T19:16:40.562222Z", + "pulp_last_updated": "2026-06-03T19:25:27.230164Z", + "name": "container.containernamespace_consumer", + "description": null, + "permissions": [ + "container.namespace_pull_containerdistribution", + "container.namespace_view_containerdistribution", + "container.namespace_view_containerpushrepository", + "container.namespace_view_containerrepository", + "container.view_containernamespace" + ], + "locked": true + } + ``` + +Having the `view` and `pull` permissions allows a user to see and pull private content from the Pulp Registry. Assigning this role only at the object level allows administrators and owners to select what the user can see. -```bash -pulp container distribution create --name "foo" --base-path "bar" -pulp user create --username "consumer" -pulp container distribution role add --name "foo" --user "consumer" --role "container.containerdistribution_consumer" -pulp user role-assignment list --username "consumer" -``` - -``` -[ - { - "pulp_href": "/pulp/api/v3/users/44/roles/6e58251d-7656-4c0d-9630-ea51ed7c29b5/", - "pulp_created": "2022-05-27T15:27:00.623004Z", - "role": "container.containerdistribution_consumer", - "content_object": "/pulp/api/v3/distributions/container/container/5b8ec13c-d578-4b3a-9b99-80986e5e00b6/" - } -] -``` +=== "Assign object level role" + + ```bash + pulp container distribution create --name "foo" --base-path "foo/hello" --private # Creates namespace 'foo' + pulp user create --username "consumer" + pulp container namespace role add --name "foo" --user "consumer" --role "container.containernamespace_consumer" + pulp user role-assignment list --username "consumer" + ``` + +=== "Final call output" + + ```json + [ + { + "pulp_href": "/pulp/api/v3/users/128/roles/019e90b4-3933-736e-8f5f-5210b6b7d894/", + "prn": "prn:core.userrole:019e90b4-3933-736e-8f5f-5210b6b7d894", + "pulp_created": "2026-06-04T03:36:30.772782Z", + "pulp_last_updated": "2026-06-04T03:36:30.772805Z", + "role": "container.containernamespace_consumer", + "content_object": "/pulp/api/v3/pulp_container/namespaces/019e90b3-da5d-727e-a35f-478b50a4233a/", + "content_object_prn": "prn:container.containernamespace:019e90b3-da5d-727e-a35f-478b50a4233a", + "description": null, + "permissions": [ + "container.namespace_pull_containerdistribution", + "container.namespace_view_containerdistribution", + "container.namespace_view_containerpushrepository", + "container.namespace_view_containerrepository", + "container.view_containernamespace" + ], + "domain": null + } + ] + ``` Also, it is possible to assign the role in the following manner: ```bash -PULP_HREF=$(pulp container distribution show --name "foo" | jq -r ".pulp_href") -pulp user role-assignment add --object ${PULP_HREF} --username "consumer" --role "container.containerdistribution_consumer" +PULP_HREF=$(pulp container namespace show --name "foo" | jq -r ".pulp_href") +pulp user role-assignment add --object ${PULP_HREF} --username "consumer" --role "container.containernamespace_consumer" ``` -#### Collaborator Role +### Collaborator Role The *Collaborator* role represents a set of permissions that a co-worker working within a same user-space should have. In addition to the *Consumer* role, users with the *Collaborator* role are allowed -to add (push) and modify content. The following set of permissions is evaluated for the *Collaborator* -role for distributions: - -```bash -pulp role show --name "container.containerdistribution_collaborator" -``` - -``` -{ - "pulp_href": "/pulp/api/v3/roles/933e0376-8945-489a-93a6-cafb6753f4bb/", - "pulp_created": "2022-05-26T12:02:28.924330Z", - "name": "container.containerdistribution_collaborator", - "description": null, - "permissions": [ - "container.pull_containerdistribution", - "container.push_containerdistribution", - "container.view_containerdistribution" - ], - "locked": true -} -``` - -```bash -pulp role show --name "container.containernamespace_collaborator" -``` - -``` -{ - "pulp_href": "/pulp/api/v3/roles/1466e614-73a7-4a58-ab36-ced0ab1a1809/", - "pulp_created": "2022-05-26T12:02:29.058226Z", - "name": "container.containernamespace_collaborator", - "description": null, - "permissions": [ - "container.namespace_add_containerdistribution", - "container.namespace_change_containerdistribution", - "container.namespace_change_containerpushrepository", - "container.namespace_delete_containerdistribution", - "container.namespace_modify_content_containerpushrepository", - "container.namespace_pull_containerdistribution", - "container.namespace_push_containerdistribution", - "container.namespace_view_containerdistribution", - "container.namespace_view_containerpushrepository", - "container.view_containernamespace" - ], - "locked": true -} -``` - -## Permissions - -A role is defined by one or more permissions. In this section, details of permissions used within -the container plugin are discussed. - -!!! warning - - The concept of managing granular permissions is obsolete. As of release 2.11.0, the plugin uses - `roles` instead of separate permission classes. To migrate the customized permission - classes to roles, follow the instructions shown at `migrating-perms-to-roles`. +to add (push) and modify content. + +=== "Show Collaborator Role" + + ```bash + pulp role show --name "container.containernamespace_collaborator" + ``` + +=== "Output" + + ```json + { + "pulp_href": "/pulp/api/v3/roles/019e8eea-9be6-7d41-9441-a46dd5e22cde/", + "prn": "prn:core.role:019e8eea-9be6-7d41-9441-a46dd5e22cde", + "pulp_created": "2026-06-03T19:16:40.551289Z", + "pulp_last_updated": "2026-06-03T19:25:27.222189Z", + "name": "container.containernamespace_collaborator", + "description": null, + "permissions": [ + "container.namespace_add_containerdistribution", + "container.namespace_change_containerdistribution", + "container.namespace_change_containerpushrepository", + "container.namespace_change_containerrepository", + "container.namespace_delete_containerdistribution", + "container.namespace_modify_content_containerpushrepository", + "container.namespace_modify_content_containerrepository", + "container.namespace_pull_containerdistribution", + "container.namespace_push_containerdistribution", + "container.namespace_view_containerdistribution", + "container.namespace_view_containerpushrepository", + "container.namespace_view_containerrepository", + "container.view_containernamespace" + ], + "locked": true + } + ``` + +Collaborators, like Owners, have the `container.namespace_add_containerdistribution` permission allowing them to push new images to Pulp under the same namespace, auto-creating the backing container distribution and repository. Newly pushed images are public by default, allowing anyone to view and pull from them. The user will also be granted owner roles for the created distribution and repository. + +## Distributions + +The second level of permissions in `pulp-container` is on Distributions, with each distribution representing an individual image that can be assigned permissions on. After checking namespace permissions, Pulp will then check to see if the user has any permissions on the distribution being accessed. Like namespaces, distributions follow the *Owner*, *Collaborator* and *Consumer* role scheme with similar permissions, just scoped to distributions instead. + +=== "Show Distribution Roles" + + ```bash + pulp role list --name-startswith "container.containerdistribution" + ``` + +=== "Output" + + ```json + [ + { + "pulp_href": "/pulp/api/v3/roles/019e8eea-9bb8-7244-8b64-77a4c0af732c/", + "prn": "prn:core.role:019e8eea-9bb8-7244-8b64-77a4c0af732c", + "pulp_created": "2026-06-03T19:16:40.505460Z", + "pulp_last_updated": "2026-06-03T19:25:27.183450Z", + "name": "container.containerdistribution_consumer", + "description": null, + "permissions": [ + "container.pull_containerdistribution", + "container.view_containerdistribution" + ], + "locked": true + }, + { + "pulp_href": "/pulp/api/v3/roles/019e8eea-9bab-7411-9fc0-080b573b616f/", + "prn": "prn:core.role:019e8eea-9bab-7411-9fc0-080b573b616f", + "pulp_created": "2026-06-03T19:16:40.492144Z", + "pulp_last_updated": "2026-06-03T19:25:27.177034Z", + "name": "container.containerdistribution_collaborator", + "description": null, + "permissions": [ + "container.pull_containerdistribution", + "container.push_containerdistribution", + "container.view_containerdistribution" + ], + "locked": true + }, + { + "pulp_href": "/pulp/api/v3/roles/019e8eea-9ba0-7b15-8ff1-0bfb19a8c069/", + "prn": "prn:core.role:019e8eea-9ba0-7b15-8ff1-0bfb19a8c069", + "pulp_created": "2026-06-03T19:16:40.480936Z", + "pulp_last_updated": "2026-06-03T19:25:27.169125Z", + "name": "container.containerdistribution_owner", + "description": null, + "permissions": [ + "container.change_containerdistribution", + "container.delete_containerdistribution", + "container.manage_roles_containerdistribution", + "container.pull_containerdistribution", + "container.push_containerdistribution", + "container.view_containerdistribution" + ], + "locked": true + }, + { + "pulp_href": "/pulp/api/v3/roles/019e8eea-9b94-7b12-a7ee-e0acaf594bea/", + "prn": "prn:core.role:019e8eea-9b94-7b12-a7ee-e0acaf594bea", + "pulp_created": "2026-06-03T19:16:40.468877Z", + "pulp_last_updated": "2026-06-03T19:25:27.151698Z", + "name": "container.containerdistribution_creator", + "description": null, + "permissions": [ + "container.add_containerdistribution" + ], + "locked": true + } + ] + ``` +!!! note + It is recommended to assign the namespace creator role to a user instead of the distribution creator role since creating a distribution will sometimes involve creating a new namespace if it does not already exist. -### Namespaces +## Roles and Access Policies -Pulp Container namespaces allow users to reuse repository names under different context. The -namespace can represent an organization, a team, or any other kind of logical grouping of container -repositories. Namespaces provide a naming convention for container repositories. Repositories in -the `foo` namespace are named `foo/something` and `foo/something-else`. +The default roles and policies in `pulp-container` are configured using access policies for the following `viewset_names`: -The default access policy for `pulp_container/namespaces` requires a user to have the -`container.add_containernamespace` permission to create a new namespace. Alternatively a user is -allowed to create a namespace that matches his username if it did not exist before. The new -namespace can be created by pushing an image using `podman` or `docker` client. This same -permissions allow the user of Pulp's API to create a new namespace. +- `pulp_container/namespaces` +- `distributions/container/container` +- `repositories/container/container-push` +- `remotes/container/container` +- `repositories/container/container` +- `repositories/container/container-push/versions` +- `repositories/container/container/versions` +- `content/container/blobs` +- `content/container/manifests` +- `content/container/tags` -The creation of a new namespace creates three user groups that can access the namespace: -Owners, Collaborators, and Consumers. The user that creates the namespace is automatically added to -the Owners group. +### Creating Custom Roles -#### Namespace Owners +The default roles can not be edited as they are locked, but new unlocked roles can be created and edited. -The group name is `container.namespace.owners.`. This group has the following -object permissions for the namespace: +```bash +pulp role create --name "docker-read-write" \ + --permission "container.view_containernamespace" \ + --permission "container.namespace_add_containerdistribution" \ + --permission "container.namespace_view_containerdistribution" \ + --permission "container.namespace_pull_containerdistribution" \ + --permission "container.namespace_push_containerdistribution" \ + --permission "container.namespace_view_containerpushrepository" \ + --permission "container.namespace_view_containerrepository" \ + --description "Read/Write/no delete only docker api" -``` -"container.view_containernamespace" -"container.delete_containernamespace" -"container.namespace_add_containerdistribution", -"container.namespace_delete_containerdistribution -"container.namespace_view_containerdistribution" -"container.namespace_pull_containerdistribution" -"container.namespace_push_containerdistribution" -"container.namespace_change_containerdistribution" -"container.namespace_view_containerpushrepository" -"container.namespace_modify_content_containerpushrepository" -"container.namespace_modify_content_containerrepository" +# Assign new role to alice +pulp container namespace add --name "foo" --username "alice" --role "docker-read-write" ``` -The users in the owners group have the permissions to add/remove users from all three groups -associated with the namespace. They also have the ability to create, update, and delete -repositories in the namespace. +### Editing the Access Policies -In addition to being able to use the `podman` or `docker` client to manage repositories, owners -can use Pulp's API to add and remove tags in the repositories for the namespace. +The access policies on a viewset determine who can see the objects of the viewset, what permissions are required to perform actions on those objects, and what roles are given upon object creation. The `queryset_scoping`, `statements` and `creation_hooks` of the access policy determine these behaviors respectively. Each of these attributes can be viewed and edited individually. -#### Namespace Collaborators - -The group name is `container.namespace.collaborators.`. This group has the -following object permissions for the namespace: +```bash +# View Container Namespace's Access Policy +pulp access-policy show --viewset-name "pulp_container/namespaces" -``` -"container.view_containernamespace" -"container.namespace_add_containerdistribution" -"container.namespace_delete_containerdistribution" -"container.namespace_view_containerdistribution" -"container.namespace_pull_containerdistribution" -"container.namespace_push_containerdistribution" -"container.namespace_change_containerdistribution" -"container.namespace_view_containerpushrepository" -"container.namespace_modify_content_containerpushrepository" -"container.namespace_modify_content_containerrepository" +# Update Container Namespace's Creation Hooks +pulp access-policy update --href "$NAMESPACE_AP_HREF" \ + --creation-hooks '[{"function": "add_roles", "parameters": {"roles": "docker-read-write"}}]' ``` -Users in the Collaborator group can do everything that the owners can, with the exception of -deleting the namespace. - -#### Namespace Consumers - -The group name is `container.namespace.consumers.`. This group has the following -object permissions for the namespace: - -``` -"container.view_containernamespace" -"container.namespace_view_containerdistribution" -"container.namespace_pull_containerdistribution" -"container.namespace_view_containerpushrepository" -``` - -Users in the Consumers group can `pull` from any of the repositories in the namespace. Users -should only need to be added to this group if private repositories are being used. If the -repository is public, then anyone can `pull` from the repository. - -### Distributions - -Distributions are Pulp resources that represent URLs where repositories can be consumed. -Permissions for accessing specific container repositories are described in terms of permissions -to access Container Distributions. Each time a new repository is pushed using `podman` or `docker`, -a Container Distribution and a Container Repository are created. Both resources can be accessed -using Pulp's API. Registry-pushed repositories inherit distribution and namespace permissions for -read and content-modifying API actions. - !!! note + Access polices can be reset to their default using the reset endpoint, e.g: pulp access-policy reset --href "$AP_HREF" + +### Docker API Permission Policies + +The main roles and permission policies for the Docker API described before are found under the `distributions/container/container` and `pulp_container/namespace` viewsets. Specifically the Container Distribution access policy contains special action statements that are used when determining the permissions a user has to perform a Docker action. + +#### `docker/podman pull` + +When pulling an image Pulp will check the `pull` action on the Container Distribution access policy. + +=== "Show `pull` Action Statements" + + ```bash + pulp access-policy show --viewset-name "distributions/container/container" \ + | jq '.statements[] | select(.action[] == "pull")' + ``` + +=== "Output" + + ```json + { + "action": [ + "pull" + ], + "effect": "allow", + "principal": "*", + "condition_expression": [ + "not is_private" + ] + } + { + "action": [ + "pull" + ], + "effect": "allow", + "condition": [ + "has_namespace_or_obj_perms:container.pull_containerdistribution" + ], + "principal": "authenticated" + } + ``` + +There are two action statements for `pull`, only one of them needs to be true for the user to be granted permission to pull an image. The order of statements is the order the statements are checked. So first Pulp checks if the distribution is public, i.e. `not is_private`. If the distribution is private than we check if the user has permission on the namespace or distribution. See the section below describing the custom conditions available for `pulp-container`. - Older versions of pulp-container created Container Push Repositories on docker pushes. Legacy - pushes still remain available under `/pulp/api/v3/repositories/container/container-push/`, but - will be migrated to Container Repositories in a future release. - -The creation of a new distribution creates three user groups that can access the distribution: -Owners, Collaborators, and Consumers. The user that creates the distribution is automatically added to -the Owners group. - -#### Distribution Owners - -The group name is `container.distribution.owners.`. This group has the following -object permissions for the Distribution: - -``` -"container.view_containerdistribution" -"container.pull_containerdistribution" -"container.push_containerdistribution" -"container.delete_containerdistribution" -"container.change_containerdistribution" -``` - -Distribution roles grant access to the linked Container Repository through the distribution and -namespace permission checks described above. Distribution owners can update and delete the -repository associated with the distribution. They can also add/remove users from the roles -associated with the distribution. +!!! note + If no statement evaluates to true then the request is denied. Admins always bypass any checks. + +#### `docker/podman push` + +When pushing an image to Pulp there are three different scenarios a user can find themselves in which determines the permissions that are checked for during the push. + +- Scenario 1: Pushing a new image and a new namespace +- Scenario 2: Pushing a new image inside an existing namespace +- Scenario 3: Pushing a new tag/manifest for an existing image + +Let's start with Scenario 3 which requires the least amount of permissions and only checks the `push` action on the Container Distribution access policy. + +=== "Show `push` Action Statements" + + ```bash + pulp access-policy show --viewset-name "distributions/container/container" \ + | jq '.statements[] | select(.action[] == "push")' + ``` + +=== "Output" + + ```json + { + "action": [ + "push" + ], + "effect": "allow", + "condition": [ + "has_namespace_or_obj_perms:container.push_containerdistribution", + "obj_exists" + ], + "principal": "authenticated" + } + { + "action": [ + "push" + ], + "effect": "allow", + "condition": [ + "has_namespace_or_obj_perms:container.add_containerdistribution", + "has_namespace_or_obj_perms:container.push_containerdistribution" + ], + "principal": "authenticated" + } + ``` + +The first statement checks that the request isn't a first push, our Scenario 3. Both of the two action statements for `push` check to see if the user has the `container.push_containerdistribution` permission on the namespace or distribution. This permission can be found in both the Distribution's and Namespace's *Owner* and *Collaborator* roles. -#### Distribution Collaborators +!!! note + Each condition inside a statement's `condition` list is AND together. -The group name is `container.distribution.collaborators.`. This group has the -following object permissions for the Distribution: +For Scenario 2 Pulp will check the `create_distribution` action on the Namespace access policy. -``` -"container.view_containerdistribution" -"container.pull_containerdistribution" -"container.push_containerdistribution" -``` +=== "Show `create_distribution` Action Statement" -Users in the Collaborator group can do everything that the owners can, with the exception for deleting -the Distribution. + ```bash + pulp access-policy show --viewset-name "pulp_container/namespaces" | jq '.statements[] | select(.action[] == "create_distribution")' + ``` -#### Distribution Consumers +=== "Output" -The group name is `container.distribution.consumers.`. This group has the following -object permissions for the distribution: + ```json + { + "action": [ + "create_distribution" + ], + "effect": "allow", + "condition": "has_model_or_domain_or_obj_perms:container.namespace_add_containerdistribution", + "principal": "authenticated" + } + ``` -``` -"container.view_containerdistribution" -"container.pull_containerdistribution" -``` +In order to create a new distribution inside the existing namespace, the user needs the `container.namespace_add_containerdistribution` permission. This permission is a part of the Namespace *Owner* and *Collobarator* roles. -Users in the Consumers group can `pull` the repository. Users should only need to be added to -this group if the Distribution has been configured with `private=True`. If the Distribution is -public, then anyone can `pull` from the repository associated with the Distribution. +And for Scenario 1 Pulp checks the `create` action on the Namespace access policy. +=== "Show `create` Action Statement" -#### Pull-through Distribution Owners + ```bash + pulp access-policy show --viewset-name "pulp_container/namespaces" | jq '.statements[] | select(.action[] == "create")' + ``` -This role allows users to manage and pull content from the pull-through cache distribution. +=== "Output" -``` -"container.view_containerpullthroughdistribution" -"container.delete_containerpullthroughdistribution" -"container.change_containerpullthroughdistribution" -"container.manage_roles_containerpullthroughdistribution" -"container.pull_new_containerdistribution" -``` - -#### Pull-through Distribution Collaborators + ```json + { + "action": [ + "create" + ], + "effect": "allow", + "condition": "has_model_or_domain_perms:container.add_containernamespace", + "principal": "authenticated" + } + { + "action": [ + "create" + ], + "effect": "allow", + "condition": "namespace_is_username", + "principal": "authenticated" + } + ``` -Users who have this role assigned can preview and pull new content from the main pull-through cache -distribution. +If the user has permission to create the namespace, then they will be granted the Namespace Owner role for the new namespace, which in turn will grant them the permission to create the distribution for the new image. -``` -"container.view_containerpullthroughdistribution" -"container.pull_new_containerdistribution" -``` +#### `skopeo list-tags` -#### Pull-through Distribution Consumers +Pulp also supports the `v2/_catalog` endpoint for each repository allowing a user to see the available tags for that image. This checks the `catalog` action on the Container Distribution access policy. -Similarly to the collaborator role, the following set of permissions is set for the consumer role: - -``` -"container.view_containerpullthroughdistribution" -"container.pull_new_containerdistribution" -``` +```bash +pulp access-policy show --viewset-name "distributions/container/container" \ + | jq '.statements[] | select(.action[] == "catalog")' -It is recommended to assign at least one role with these permissions to allow users to pull new -content from a remote repository: -``` -"container.namespace_modify_content_containerrepository" (e.g., namespace collaborator) -"container.namespace_add_containerdistribution" (e.g., namespace collaborator) -"container.pull_new_containerdistribution" (e.g., pull-through cache consumer) +{ + "action": [ + "catalog" + ], + "effect": "allow", + "principal": "authenticated" +} ``` -Users without the permissions can still pull already cached content from Pulp. This behaviour is -further restricted by flagging a distribution as `private=True`. +Any logged in user can list out the tags for an image. -### Private Repositories +### Custom Queryset Scoping -Users wishing to `pull` from a Container Distribution with `private=True` -will require the following object level permission on the Distribution: +In `pulp-container` there are custom queryset scoping methods for the Content, Repository, and Distribution viewsets. These custom methods are used to enforce the Namespace permissions and public/private behavior of repositories, but only for the Pulp APIs, they have no effect on the Docker APIs. -``` -"container.pull_containerdistribution" -``` - -Users that wish to be able to access the distribution with Pulp's API need the following object level -permission on the Distribution: +```bash +pulp access-policy show --viewset-name "repositories/container/container" | jq '.queryset_scoping' -``` -"container.view_containerdistribution" +# Ensure we can see every repository that is public or we have permission on through namespaces/distributions +{ + "function": "get_container_repos_qs", + "parameters": { + "ns_perm": "container.view_containernamespace", + "dist_perm": "container.view_containerdistribution", + "repo_perm": "container.view_containerrepository" + } +} ``` -Users that wish to be able to access the repository associated with the distribution with Pulp's -API need one of the following: +### `pulp-container`'s Custom Access Conditions -``` -"container.view_containerrepository" (object-level repository role), or -"container.view_containerdistribution" (distribution consumer role or above), or -"container.namespace_view_containerdistribution" (namespace consumer role or above) -``` +::: pulp_container.app.global_access_conditions From d651dbef0551eceaf6be7dec409a7ca705039e66 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Fri, 5 Jun 2026 15:46:28 -0400 Subject: [PATCH 10/11] Lint --- pulp_container/app/viewsets.py | 16 +++++++++++----- .../functional/api/test_rbac_repo_content.py | 4 +++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/pulp_container/app/viewsets.py b/pulp_container/app/viewsets.py index 5ecd5d31a..d5ff345d4 100644 --- a/pulp_container/app/viewsets.py +++ b/pulp_container/app/viewsets.py @@ -7,13 +7,12 @@ import logging -from django.db.models import Q from django_filters import CharFilter, MultipleChoiceFilter from drf_spectacular.utils import extend_schema from rest_framework import mixins from rest_framework.decorators import action -from pulpcore.plugin.models import Content, PulpTemporaryFile, RepositoryVersion +from pulpcore.plugin.models import Content, PulpTemporaryFile from pulpcore.plugin.serializers import AsyncOperationResponseSerializer from pulpcore.plugin.tasking import dispatch, general_multi_delete from pulpcore.plugin.util import ( @@ -180,7 +179,7 @@ def repository_deleted_with_distribution(distribution): def get_viewable_repositories(user, ns_perm, dist_perm, repo_perm=None, domain=None): """ - For a given user and namespace, distribution and repository permissions, return a set of + For a given user and namespace, distribution and repository permissions, return a set of repository pks that the user can view. """ domain = domain or get_domain() @@ -211,7 +210,12 @@ def get_viewable_repositories(user, ns_perm, dist_perm, repo_perm=None, domain=N else: # PushContainerRepository case direct_repository_pks = set() - return set(ns_repository_pks) | set(dist_repository_pks) | set(public_repository_pks) | set(direct_repository_pks) + return ( + set(ns_repository_pks) + | set(dist_repository_pks) + | set(public_repository_pks) + | set(direct_repository_pks) + ) class TagViewSet(ContainerContentQuerySetMixin, ReadOnlyContentViewSet): @@ -809,7 +813,9 @@ def get_container_repos_qs(self, qs, ns_perm, dist_perm, repo_perm): """ domain = get_domain() qs = models.ContainerRepository.objects.filter(pulp_domain=domain) - return qs.filter(pk__in=get_viewable_repositories(self.request.user, ns_perm, dist_perm, repo_perm)) + return qs.filter( + pk__in=get_viewable_repositories(self.request.user, ns_perm, dist_perm, repo_perm) + ) # This decorator is necessary since a sync operation is asyncrounous and returns # the id and href of the sync task. diff --git a/pulp_container/tests/functional/api/test_rbac_repo_content.py b/pulp_container/tests/functional/api/test_rbac_repo_content.py index 5d2be2902..fb987f138 100644 --- a/pulp_container/tests/functional/api/test_rbac_repo_content.py +++ b/pulp_container/tests/functional/api/test_rbac_repo_content.py @@ -1,7 +1,9 @@ """Tests that verify that RBAC for content works properly.""" -import pytest import uuid + +import pytest + from pulpcore.client.pulp_container import SetLabel, UnsetLabel from pulpcore.client.pulp_container.exceptions import ForbiddenException From a76c5437b31410ad358594b918819cad4a98f1e9 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Fri, 5 Jun 2026 16:10:41 -0400 Subject: [PATCH 11/11] Small touchups + doc changelog --- CHANGES/+rbac-rewrite.doc | 1 + pulp_container/app/serializers.py | 12 ++++-------- pulp_container/app/viewsets.py | 18 ++++++------------ 3 files changed, 11 insertions(+), 20 deletions(-) create mode 100644 CHANGES/+rbac-rewrite.doc diff --git a/CHANGES/+rbac-rewrite.doc b/CHANGES/+rbac-rewrite.doc new file mode 100644 index 000000000..a1815432a --- /dev/null +++ b/CHANGES/+rbac-rewrite.doc @@ -0,0 +1 @@ +Rewrote the RBAC documentation to be more comprehensive and detailed. diff --git a/pulp_container/app/serializers.py b/pulp_container/app/serializers.py index ff3723cd7..657f1d650 100644 --- a/pulp_container/app/serializers.py +++ b/pulp_container/app/serializers.py @@ -984,17 +984,13 @@ def validate(self, data): } ) - registry_push = repository.PUSH_ENABLED or ( - isinstance(repository, models.ContainerRepository) - and not repository.remote - and repository.distributions.exists() - ) - if registry_push: + distro_count = repository.distributions.count() + if distro_count == 1: if "future_base_path" in data: raise serializers.ValidationError( { "future_base_path": _( - "This field cannot be set since this is a push repo type." + "This field cannot be set since this repo has a single distribution." ) } ) @@ -1004,7 +1000,7 @@ def validate(self, data): raise serializers.ValidationError( { "future_base_path": _( - "This field is required since this is a sync repo type." + "This field is required since this repo has zero or multiple distributions." ) } ) diff --git a/pulp_container/app/viewsets.py b/pulp_container/app/viewsets.py index d5ff345d4..c5df96646 100644 --- a/pulp_container/app/viewsets.py +++ b/pulp_container/app/viewsets.py @@ -147,11 +147,10 @@ def get_content_qs(self, qs, ns_perm, dist_perm, repo_perm): allowed to see based on the repo permissions. """ - has_model_ns_perm = self.request.user.has_perm(ns_perm) - has_model_dist_perm = self.request.user.has_perm(dist_perm) - has_model_repo_perm = self.request.user.has_perm(repo_perm) # this will show also orphaned content - if has_model_ns_perm or (has_model_dist_perm and has_model_repo_perm): + if self.request.user.has_perm(ns_perm) or ( + self.request.user.has_perm(dist_perm) and self.request.user.has_perm(repo_perm) + ): return qs repository_pks = get_viewable_repositories(self.request.user, ns_perm, dist_perm, repo_perm) @@ -164,7 +163,7 @@ def repository_deleted_with_distribution(distribution): Return (repository, serializer_name) when a distribution delete should also delete its repo. Push repositories are always removed with their distribution. Container repositories created - during a registry push (no remote, single distribution) follow the same lifecycle. + during a registry push (single distribution) follow the same lifecycle. """ if not distribution.repository: return None @@ -811,11 +810,8 @@ def get_container_repos_qs(self, qs, ns_perm, dist_perm, repo_perm): Mirrors push repository scoping so registry-pushed repositories remain visible to distribution and namespace role holders. """ - domain = get_domain() - qs = models.ContainerRepository.objects.filter(pulp_domain=domain) - return qs.filter( - pk__in=get_viewable_repositories(self.request.user, ns_perm, dist_perm, repo_perm) - ) + viewable_repos = get_viewable_repositories(self.request.user, ns_perm, dist_perm, repo_perm) + return qs.filter(pk__in=viewable_repos) # This decorator is necessary since a sync operation is asyncrounous and returns # the id and href of the sync task. @@ -1221,8 +1217,6 @@ def get_push_repos_qs(self, qs, ns_perm, dist_perm): Returns a queryset by filtering by namespace permission to view distributions and distribution level permissions. """ - domain = get_domain() - qs = models.ContainerPushRepository.objects.filter(pulp_domain=domain) return qs.filter(pk__in=get_viewable_repositories(self.request.user, ns_perm, dist_perm))