Raise TaskResourceNotFound instead of bare DoesNotExist in tasks

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.

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.

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,39 @@ 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 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."""
 

pulp_container/app/tasks/builder.py

@@ -13,14 +13,15 @@
 )
 from pulpcore.plugin.util import get_domain
 
+from pulp_container.app.exceptions import ContainerBuildError
 from pulp_container.app.models import (
     Blob,
     BlobManifest,
     ContainerRepository,
     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 +139,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)
@@ -179,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(
@@ -188,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()
@@ -229,15 +232,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:
@@ -265,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(
@@ -274,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

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)

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)
 

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:

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 = []

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)

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(

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())

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.

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)