From 0eb7f546cca2d7071eb378f5bfb7e28e72c60909 Mon Sep 17 00:00:00 2001 From: wussh Date: Mon, 27 Jul 2026 15:56:58 +0700 Subject: [PATCH 1/4] Raise TaskResourceNotFound instead of bare DoesNotExist in tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pulpcore's task executor treats any task exception that isn't a PulpException subclass as deprecated: it logs a pulpcore.deprecation warning today and will sanitize the message away entirely in pulpcore 3.130. Every task that looks up its arguments' referenced objects by pk (repository, remote, manifest, signing service, artifact, ...) hit this whenever that object was deleted between dispatch and execution — non-deterministic, so a different one surfaces on each CI run (observed both ManifestSigningService and ContainerDistribution DoesNotExist warnings on unrelated PRs). Add TaskResourceNotFound(PulpException) and a get_task_resource_or_raise() helper in app/utils.py, and use it at the top-of-task object lookups in tag.py, untag.py, recursive_add.py, recursive_remove.py, synchronize.py, download_image_data.py, sign.py, and builder.py. Leaves the intentional try/except DoesNotExist in builder.py's get_or_create_blob alone, since that's expected control flow, not an unhandled lookup failure. Note: pulpcore's own generic tasks (e.g. general_multi_delete, which can raise ContainerDistribution.DoesNotExist) are outside pulp_container's control and are not addressed by this change. --- CHANGES/+task-resource-not-found.bugfix | 1 + pulp_container/app/exceptions.py | 22 +++++++++++++++++++ pulp_container/app/tasks/builder.py | 14 +++++++----- .../app/tasks/download_image_data.py | 6 ++--- pulp_container/app/tasks/recursive_add.py | 3 ++- pulp_container/app/tasks/recursive_remove.py | 3 ++- pulp_container/app/tasks/sign.py | 6 ++--- pulp_container/app/tasks/synchronize.py | 5 +++-- pulp_container/app/tasks/tag.py | 5 +++-- pulp_container/app/tasks/untag.py | 3 ++- pulp_container/app/utils.py | 20 ++++++++++++++++- 11 files changed, 68 insertions(+), 20 deletions(-) create mode 100644 CHANGES/+task-resource-not-found.bugfix diff --git a/CHANGES/+task-resource-not-found.bugfix b/CHANGES/+task-resource-not-found.bugfix new file mode 100644 index 000000000..26cc5156a --- /dev/null +++ b/CHANGES/+task-resource-not-found.bugfix @@ -0,0 +1 @@ +Raised a proper ``PulpException`` subclass instead of a bare Django ``DoesNotExist`` when a task's referenced object (repository, remote, manifest, signing service, etc.) no longer exists, so the error is not sanitized away by pulpcore in a future release. diff --git a/pulp_container/app/exceptions.py b/pulp_container/app/exceptions.py index d8946477d..2b127611f 100644 --- a/pulp_container/app/exceptions.py +++ b/pulp_container/app/exceptions.py @@ -1,5 +1,7 @@ from rest_framework.exceptions import APIException, NotFound, ParseError +from pulpcore.plugin.exceptions import PulpException + class BadGateway(APIException): status_code = 502 @@ -162,6 +164,26 @@ def __init__(self, digest): ) +class TaskResourceNotFound(PulpException): + """Exception to signal that a resource a task depends on no longer exists. + + Tasks look up their arguments' referenced objects by pk. If that object was + deleted between dispatch and execution (e.g. by a racing delete), a bare Django + DoesNotExist is not a PulpException, which pulpcore's task executor logs as + deprecated and will sanitize away in a future release. Raise this instead so the + real reason is preserved on the task result. + """ + + error_code = "PLP_CONTAINER_TASK_RESOURCE_MISSING" + + def __init__(self, message): + """Initialize the exception with a description of the missing resource.""" + self.message = message + + def __str__(self): + return self.message + + class InvalidRequest(ParseError): """An exception to render an HTTP 400 response.""" diff --git a/pulp_container/app/tasks/builder.py b/pulp_container/app/tasks/builder.py index 155abbc22..e8acf28a7 100644 --- a/pulp_container/app/tasks/builder.py +++ b/pulp_container/app/tasks/builder.py @@ -20,7 +20,7 @@ Manifest, Tag, ) -from pulp_container.app.utils import calculate_digest +from pulp_container.app.utils import calculate_digest, get_task_resource_or_raise from pulp_container.constants import MEDIA_TYPE @@ -138,9 +138,11 @@ def build_image( raise RuntimeError("Neither a name nor temporary file for the Containerfile was specified.") if containerfile_tempfile_pk: - containerfile_artifact = PulpTemporaryFile.objects.get(pk=containerfile_tempfile_pk) + containerfile_artifact = get_task_resource_or_raise( + PulpTemporaryFile, pk=containerfile_tempfile_pk + ) - repository = ContainerRepository.objects.get(pk=repository_pk) + repository = get_task_resource_or_raise(ContainerRepository, pk=repository_pk) name = str(uuid4()) with tempfile.TemporaryDirectory(dir=".") as working_directory: working_directory = os.path.abspath(working_directory) @@ -229,15 +231,15 @@ def build_image_from_containerfile( image and tag. """ - containerfile = Artifact.objects.get(pk=containerfile_pk) - repository = ContainerRepository.objects.get(pk=repository_pk) + containerfile = get_task_resource_or_raise(Artifact, pk=containerfile_pk) + repository = get_task_resource_or_raise(ContainerRepository, pk=repository_pk) name = str(uuid4()) with tempfile.TemporaryDirectory(dir=".") as working_directory: working_directory = os.path.abspath(working_directory) context_path = os.path.join(working_directory, "context") os.makedirs(context_path, exist_ok=True) for key, val in artifacts.items(): - artifact = Artifact.objects.get(pk=key) + artifact = get_task_resource_or_raise(Artifact, pk=key) dest_path = os.path.join(context_path, val) dirs = os.path.split(dest_path)[0] if dirs: diff --git a/pulp_container/app/tasks/download_image_data.py b/pulp_container/app/tasks/download_image_data.py index 3eb712cb8..f0d456286 100644 --- a/pulp_container/app/tasks/download_image_data.py +++ b/pulp_container/app/tasks/download_image_data.py @@ -7,7 +7,7 @@ from pulpcore.plugin.tasking import add_and_remove from pulp_container.app.models import ContainerRemote, ContainerRepository, Tag -from pulp_container.app.utils import determine_media_type_from_json +from pulp_container.app.utils import determine_media_type_from_json, get_task_resource_or_raise from pulp_container.constants import MEDIA_TYPE from .sync_stages import ContainerFirstStage @@ -21,8 +21,8 @@ async def aadd_and_remove(*args, **kwargs): def download_image_data(repository_pk, remote_pk, raw_text_manifest_data, tag_name=None): - repository = ContainerRepository.objects.get(pk=repository_pk) - remote = ContainerRemote.objects.get(pk=remote_pk) + repository = get_task_resource_or_raise(ContainerRepository, pk=repository_pk) + remote = get_task_resource_or_raise(ContainerRemote, pk=remote_pk) log.info("Pulling cache: repository={r} remote={p}".format(r=repository.name, p=remote.name)) first_stage = ContainerPullThroughFirstStage(remote, raw_text_manifest_data, tag_name) dv = ContainerDeclarativeVersion(first_stage, repository) diff --git a/pulp_container/app/tasks/recursive_add.py b/pulp_container/app/tasks/recursive_add.py index da0254bf7..f846ad0d3 100644 --- a/pulp_container/app/tasks/recursive_add.py +++ b/pulp_container/app/tasks/recursive_add.py @@ -5,6 +5,7 @@ Manifest, Tag, ) +from pulp_container.app.utils import get_task_resource_or_raise def recursive_add_content(repository_pk, content_units): @@ -22,7 +23,7 @@ def recursive_add_content(repository_pk, content_units): should be added to the previous Repository Version for this Repository. """ - repository = ContainerRepository.objects.get(pk=repository_pk) + repository = get_task_resource_or_raise(ContainerRepository, pk=repository_pk) tags_to_add = Tag.objects.filter(pk__in=content_units) diff --git a/pulp_container/app/tasks/recursive_remove.py b/pulp_container/app/tasks/recursive_remove.py index 72835e227..7315cf408 100644 --- a/pulp_container/app/tasks/recursive_remove.py +++ b/pulp_container/app/tasks/recursive_remove.py @@ -9,6 +9,7 @@ ManifestSignature, Tag, ) +from pulp_container.app.utils import get_task_resource_or_raise def recursive_remove_content(repository_pk, content_units): @@ -36,7 +37,7 @@ def recursive_remove_content(repository_pk, content_units): should be removed from the Repository. """ - repository = Repository.objects.get(pk=repository_pk).cast() + repository = get_task_resource_or_raise(Repository, pk=repository_pk).cast() latest_version = repository.latest_version() latest_content = latest_version.content.all() if latest_version else Content.objects.none() if "*" in content_units: diff --git a/pulp_container/app/tasks/sign.py b/pulp_container/app/tasks/sign.py index 74a66858d..b2c666b88 100644 --- a/pulp_container/app/tasks/sign.py +++ b/pulp_container/app/tasks/sign.py @@ -13,7 +13,7 @@ ManifestSigningService, Tag, ) -from pulp_container.app.utils import extract_data_from_signature +from pulp_container.app.utils import extract_data_from_signature, get_task_resource_or_raise from pulp_container.constants import ( MANIFEST_MEDIA_TYPES, SIGNATURE_TYPE, @@ -41,7 +41,7 @@ def sign(repository_pk, signing_service_pk, reference, tags_list=None): should be signed. """ - repository = Repository.objects.get(pk=repository_pk).cast() + repository = get_task_resource_or_raise(Repository, pk=repository_pk).cast() latest_version = repository.latest_version() if tags_list: latest_repo_content_tags = latest_version.content.filter( @@ -55,7 +55,7 @@ def sign(repository_pk, signing_service_pk, reference, tags_list=None): .select_related("tagged_manifest") .exclude(Q(name__endswith=".sig") | Q(name__endswith=".att") | Q(name__endswith=".sbom")) ) - signing_service = ManifestSigningService.objects.get(pk=signing_service_pk) + signing_service = get_task_resource_or_raise(ManifestSigningService, pk=signing_service_pk) async def sign_manifests(): added_signatures = [] diff --git a/pulp_container/app/tasks/synchronize.py b/pulp_container/app/tasks/synchronize.py index aa2ffa051..eab490485 100644 --- a/pulp_container/app/tasks/synchronize.py +++ b/pulp_container/app/tasks/synchronize.py @@ -12,6 +12,7 @@ ) from pulp_container.app.models import ContainerRemote, ContainerRepository +from pulp_container.app.utils import get_task_resource_or_raise from .sync_stages import ContainerContentSaver, ContainerFirstStage @@ -34,8 +35,8 @@ def synchronize(remote_pk, repository_pk, mirror, signed_only): ValueError: If the remote does not specify a URL to sync """ - remote = ContainerRemote.objects.get(pk=remote_pk) - repository = ContainerRepository.objects.get(pk=repository_pk) + remote = get_task_resource_or_raise(ContainerRemote, pk=remote_pk) + repository = get_task_resource_or_raise(ContainerRepository, pk=repository_pk) log.info("Synchronizing: repository={r} remote={p}".format(r=repository.name, p=remote.name)) first_stage = ContainerFirstStage(remote, signed_only) dv = ContainerDeclarativeVersion(first_stage, repository, mirror) diff --git a/pulp_container/app/tasks/tag.py b/pulp_container/app/tasks/tag.py index 0853e01b4..eee80d122 100644 --- a/pulp_container/app/tasks/tag.py +++ b/pulp_container/app/tasks/tag.py @@ -2,6 +2,7 @@ from pulpcore.plugin.util import get_domain from pulp_container.app.models import Manifest, Tag +from pulp_container.app.utils import get_task_resource_or_raise def tag_image(manifest_pk, tag, repository_pk): @@ -14,9 +15,9 @@ def tag_image(manifest_pk, tag, repository_pk): a new repository version when a manifest contains a digest which is not equal to the digest passed with POST request. """ - manifest = Manifest.objects.get(pk=manifest_pk) + manifest = get_task_resource_or_raise(Manifest, pk=manifest_pk) - repository = Repository.objects.get(pk=repository_pk).cast() + repository = get_task_resource_or_raise(Repository, pk=repository_pk).cast() latest_version = repository.latest_version() tags_to_remove = Tag.objects.filter(pk__in=latest_version.content.all(), name=tag).exclude( diff --git a/pulp_container/app/tasks/untag.py b/pulp_container/app/tasks/untag.py index 68cde6834..023d4ee96 100644 --- a/pulp_container/app/tasks/untag.py +++ b/pulp_container/app/tasks/untag.py @@ -1,13 +1,14 @@ from pulpcore.plugin.models import Repository from pulp_container.app.models import Tag +from pulp_container.app.utils import get_task_resource_or_raise def untag_image(tag, repository_pk): """ Create a new repository version without a specified manifest's tag name. """ - repository = Repository.objects.get(pk=repository_pk).cast() + repository = get_task_resource_or_raise(Repository, pk=repository_pk).cast() latest_version = repository.latest_version() tags_in_latest_repository = latest_version.content.filter(pulp_type=Tag.get_pulp_type()) diff --git a/pulp_container/app/utils.py b/pulp_container/app/utils.py index d578bd62e..8e1e808fa 100644 --- a/pulp_container/app/utils.py +++ b/pulp_container/app/utils.py @@ -16,7 +16,7 @@ from pulpcore.plugin.models import Artifact, Task from pulpcore.plugin.util import get_domain -from pulp_container.app.exceptions import ManifestInvalid +from pulp_container.app.exceptions import ManifestInvalid, TaskResourceNotFound from pulp_container.app.json_schemas import ( DOCKER_MANIFEST_LIST_V2_SCHEMA, DOCKER_MANIFEST_V1_SCHEMA, @@ -42,6 +42,24 @@ def get_full_path(base_path, pulp_domain=None): return base_path +def get_task_resource_or_raise(model, **kwargs): + """ + Fetch a single model instance by the given lookup, or raise TaskResourceNotFound. + + Intended for the object lookups at the top of task functions (repository, remote, + manifest, etc. by pk), where a bare DoesNotExist would otherwise propagate as a + non-PulpException and get flagged by pulpcore as deprecated/sanitized-away. + """ + try: + return model.objects.get(**kwargs) + except model.DoesNotExist: + lookup = ", ".join(f"{key}={value}" for key, value in kwargs.items()) + raise TaskResourceNotFound( + f"{model.__name__} matching {{{lookup}}} does not exist. It may have been " + "deleted after this task was dispatched." + ) from None + + def get_accepted_media_types(headers): """ Returns a list of media types from the Accept headers. From cf8d2dd128022429534f585d565a0048c29364af Mon Sep 17 00:00:00 2001 From: wussh Date: Mon, 27 Jul 2026 16:22:15 +0700 Subject: [PATCH 2/4] Re-run CI (flaky test_manifest_pull / test_rbac_repository_content, unrelated to this diff) From 8761fe6d019890cff546bf6d52ee4c6e6e722ff4 Mon Sep 17 00:00:00 2001 From: wussh Date: Mon, 27 Jul 2026 11:56:16 +0700 Subject: [PATCH 3/4] Raise ContainerBuildError instead of bare Exception in image builder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pulpcore treats any task exception that isn't a PulpException subclass as deprecated: it logs a pulpcore.deprecation warning today and will sanitize the error message away entirely in pulpcore 3.130, losing the actual podman/buildah stderr. build_image_from_scratch and the deprecated build_image_from_containerfile both raised bare Exception on a failed podman build/push, tripping this path — surfaced as CI noise in the "deprecations" check, but a real forward-compat bug. Add ContainerBuildError(PulpException) and raise it with the decoded stderr instead; pytest.raises(PulpTaskError) assertions in test_build_images.py are unaffected since the exception still propagates through task.error the same way. --- CHANGES/+container-build-error-sanitization.bugfix | 1 + pulp_container/app/exceptions.py | 13 +++++++++++++ pulp_container/app/tasks/builder.py | 9 +++++---- 3 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 CHANGES/+container-build-error-sanitization.bugfix diff --git a/CHANGES/+container-build-error-sanitization.bugfix b/CHANGES/+container-build-error-sanitization.bugfix new file mode 100644 index 000000000..baa3c23cf --- /dev/null +++ b/CHANGES/+container-build-error-sanitization.bugfix @@ -0,0 +1 @@ +Raised a proper ``PulpException`` subclass instead of a bare ``Exception`` when building or pushing an OCI image fails, so the error is not sanitized away by pulpcore in a future release. diff --git a/pulp_container/app/exceptions.py b/pulp_container/app/exceptions.py index 2b127611f..6522b3e34 100644 --- a/pulp_container/app/exceptions.py +++ b/pulp_container/app/exceptions.py @@ -184,6 +184,19 @@ def __str__(self): return self.message +class ContainerBuildError(PulpException): + """Exception to signal that building or pushing an OCI image failed.""" + + error_code = "PLP_CONTAINER_BUILD_0001" + + def __init__(self, message): + """Initialize the exception with the decoded stderr of the failed podman command.""" + self.message = message + + def __str__(self): + return self.message + + class InvalidRequest(ParseError): """An exception to render an HTTP 400 response.""" diff --git a/pulp_container/app/tasks/builder.py b/pulp_container/app/tasks/builder.py index e8acf28a7..1296d4b9d 100644 --- a/pulp_container/app/tasks/builder.py +++ b/pulp_container/app/tasks/builder.py @@ -13,6 +13,7 @@ ) from pulpcore.plugin.util import get_domain +from pulp_container.app.exceptions import ContainerBuildError from pulp_container.app.models import ( Blob, BlobManifest, @@ -181,7 +182,7 @@ def build_image( stderr=subprocess.PIPE, ) if bud_cp.returncode != 0: - raise Exception(bud_cp.stderr) + raise ContainerBuildError(bud_cp.stderr.decode()) image_dir = os.path.join(working_directory, "image") os.makedirs(image_dir, exist_ok=True) push_cp = subprocess.run( @@ -190,7 +191,7 @@ def build_image( stderr=subprocess.PIPE, ) if push_cp.returncode != 0: - raise Exception(push_cp.stderr) + raise ContainerBuildError(push_cp.stderr.decode()) repository_version = add_image_from_directory_to_repository(image_dir, repository, tag) if isinstance(containerfile_artifact, PulpTemporaryFile): containerfile_artifact.delete() @@ -267,7 +268,7 @@ def build_image_from_containerfile( stderr=subprocess.PIPE, ) if bud_cp.returncode != 0: - raise Exception(bud_cp.stderr) + raise ContainerBuildError(bud_cp.stderr.decode()) image_dir = os.path.join(working_directory, "image") os.makedirs(image_dir, exist_ok=True) push_cp = subprocess.run( @@ -276,7 +277,7 @@ def build_image_from_containerfile( stderr=subprocess.PIPE, ) if push_cp.returncode != 0: - raise Exception(push_cp.stderr) + raise ContainerBuildError(push_cp.stderr.decode()) repository_version = add_image_from_directory_to_repository(image_dir, repository, tag) return repository_version From 4a22336bbcb6035cd600f58d944956ff507ca657 Mon Sep 17 00:00:00 2001 From: wussh Date: Mon, 27 Jul 2026 11:56:43 +0700 Subject: [PATCH 4/4] Clean up ContainerNamespace before domain deletion in domain tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ContainerNamespace has a PROTECT foreign key to Domain. cdomain_factory's teardown cleared content-redirect guards but never ContainerNamespace objects, so any domain-scoped ContainerDistribution left one behind (auto-created by ContainerNamespaceSerializer.get_or_create). When pulpcore's own domain_factory teardown then deleted the domain, Django raised ProtectedError — sanitized into a pulpcore.deprecation warning that failed the "deprecations" CI check. test_pull_in_domain creates two distributions and cleaned up neither namespace, making it the concrete trigger. Query-and-delete namespaces by domain in cdomain_factory's teardown instead of relying on each test to remember; safe to run even for tests that already clean their own namespace via add_to_cleanup since it re-lists current state. --- pulp_container/tests/functional/api/test_domains.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pulp_container/tests/functional/api/test_domains.py b/pulp_container/tests/functional/api/test_domains.py index e90cb32db..d8bda092b 100644 --- a/pulp_container/tests/functional/api/test_domains.py +++ b/pulp_container/tests/functional/api/test_domains.py @@ -11,7 +11,7 @@ @pytest.fixture -def cdomain_factory(domain_factory, pulpcore_bindings): +def cdomain_factory(domain_factory, pulpcore_bindings, container_bindings): domains = [] def _domain_factory(*args, **kwargs): @@ -22,6 +22,14 @@ def _domain_factory(*args, **kwargs): yield _domain_factory for domain in domains: + # ContainerNamespace has a PROTECT FK to Domain, so it must be cleared before + # domain_factory's own teardown deletes the domain, or that delete raises + # ProtectedError. Some tests already clean their own namespace up via + # add_to_cleanup; re-querying here is a no-op for those and a safety net for + # any test (e.g. one that creates multiple distributions) that doesn't. + namespaces = container_bindings.PulpContainerNamespacesApi.list(pulp_domain=domain.name) + for namespace in namespaces.results: + container_bindings.PulpContainerNamespacesApi.delete(namespace.pulp_href) guards = pulpcore_bindings.ContentguardsContentRedirectApi.list(pulp_domain=domain.name) for guard in guards.results: pulpcore_bindings.ContentguardsContentRedirectApi.delete(guard.pulp_href)