From ed996a1b4d953838c23a497065461f019dc58517 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Tue, 19 May 2026 14:07:30 -0400 Subject: [PATCH] Switch push created repo to ContainerRepository https://redhat.atlassian.net/browse/PULP-1748 Assisted by: cursor composer 2 --- CHANGES/+default-push-repo.feature | 1 + pulp_container/app/registry_api.py | 49 +++++++++----- pulp_container/app/serializers.py | 2 +- .../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 | 5 +- .../functional/api/test_rbac_repo_content.py | 12 ++-- .../functional/api/test_rbac_repo_versions.py | 7 +- .../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 ++++++++ 14 files changed, 142 insertions(+), 57 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/pulp_container/app/registry_api.py b/pulp_container/app/registry_api.py index b3db3c1ed..fa69182f6 100644 --- a/pulp_container/app/registry_api.py +++ b/pulp_container/app/registry_api.py @@ -45,9 +45,15 @@ 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 +from pulpcore.plugin.util import get_domain, get_objects_for_user from pulp_container.app import models, serializers from pulp_container.app.authorization import AuthorizationService, PermissionChecker @@ -423,6 +429,7 @@ def get_dr_push(self, request, path, create=False): Optionally create them if not found. """ domain = get_domain() + print("user:", request.user) try: distribution = models.ContainerDistribution.objects.get( base_path=path, pulp_domain=domain @@ -436,12 +443,10 @@ 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( - {"name": path, "pulp_domain": domain} + repository, _ = models.ContainerRepository.objects.get_or_create( + name=path, pulp_domain=domain ) distribution.repository = repository distribution.save() @@ -451,22 +456,36 @@ 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} - ) - distribution = serializers.ContainerDistributionSerializer.get_or_create( - {"base_path": path, "name": path, "pulp_domain": domain}, - {"repository": get_url(repository)}, + # 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, _ = models.ContainerRepository.objects.get_or_create( + name=path, pulp_domain=domain + ) + distribution, _ = models.ContainerDistribution.objects.get_or_create( + base_path=path, + name=path, + pulp_domain=domain, + defaults={"repository": repository}, ) except ObjectDoesNotExist: 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 0281e5693..910451a4a 100644 --- a/pulp_container/app/serializers.py +++ b/pulp_container/app/serializers.py @@ -248,7 +248,7 @@ class Meta: model = models.ContainerNamespace -class ContainerRepositorySerializer(RepositorySerializer): +class ContainerRepositorySerializer(RepositorySerializer, GetOrCreateSerializerMixin): """ Serializer for Container Repositories. """ 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 e1f4936f6..c424874a7 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..0639f858a 100644 --- a/pulp_container/tests/functional/api/test_rbac_push_repositories.py +++ b/pulp_container/tests/functional/api/test_rbac_push_repositories.py @@ -9,6 +9,7 @@ def test_rbac_push_repository( add_to_cleanup, + container_push_repository_factory, gen_user, registry_client, local_registry, @@ -17,7 +18,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 +35,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..4d01bb6f1 100644 --- a/pulp_container/tests/functional/api/test_rbac_repo_versions.py +++ b/pulp_container/tests/functional/api/test_rbac_repo_versions.py @@ -132,6 +132,7 @@ def create_new_repo_version(): def test_rbac_push_repository_version( add_to_cleanup, + container_push_repository_factory, gen_user, registry_client, local_registry, @@ -139,7 +140,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 +161,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 +232,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 66dcc86ec..dc7f65a49 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( _, _, 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):