diff --git a/CHANGES/+task-resource-not-found.bugfix b/CHANGES/+task-resource-not-found.bugfix new file mode 100644 index 000000000..cca010c3d --- /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..bd4573006 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 = "CON0001" + + 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..07a6e2551 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 TaskResourceNotFound from pulp_container.app.models import ( Blob, BlobManifest, @@ -138,9 +139,21 @@ 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) + try: + containerfile_artifact = PulpTemporaryFile.objects.get(pk=containerfile_tempfile_pk) + except PulpTemporaryFile.DoesNotExist: + raise TaskResourceNotFound( + f"PulpTemporaryFile matching pk={containerfile_tempfile_pk} does not exist. " + "It may have been deleted after this task was dispatched." + ) from None - repository = ContainerRepository.objects.get(pk=repository_pk) + try: + repository = ContainerRepository.objects.get(pk=repository_pk) + except ContainerRepository.DoesNotExist: + raise TaskResourceNotFound( + f"ContainerRepository matching pk={repository_pk} does not exist. It may " + "have been deleted after this task was dispatched." + ) from None name = str(uuid4()) with tempfile.TemporaryDirectory(dir=".") as working_directory: working_directory = os.path.abspath(working_directory) @@ -229,15 +242,33 @@ def build_image_from_containerfile( image and tag. """ - containerfile = Artifact.objects.get(pk=containerfile_pk) - repository = ContainerRepository.objects.get(pk=repository_pk) + try: + containerfile = Artifact.objects.get(pk=containerfile_pk) + except Artifact.DoesNotExist: + raise TaskResourceNotFound( + f"Artifact matching pk={containerfile_pk} does not exist. It may have been " + "deleted after this task was dispatched." + ) from None + try: + repository = ContainerRepository.objects.get(pk=repository_pk) + except ContainerRepository.DoesNotExist: + raise TaskResourceNotFound( + f"ContainerRepository matching pk={repository_pk} does not exist. It may " + "have been deleted after this task was dispatched." + ) from None 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) + try: + artifact = Artifact.objects.get(pk=key) + except Artifact.DoesNotExist: + raise TaskResourceNotFound( + f"Artifact matching pk={key} does not exist. It may have been " + "deleted after this task was dispatched." + ) from None 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..2a5152bd8 100644 --- a/pulp_container/app/tasks/download_image_data.py +++ b/pulp_container/app/tasks/download_image_data.py @@ -6,6 +6,7 @@ from pulpcore.plugin.stages import DeclarativeContent from pulpcore.plugin.tasking import add_and_remove +from pulp_container.app.exceptions import TaskResourceNotFound from pulp_container.app.models import ContainerRemote, ContainerRepository, Tag from pulp_container.app.utils import determine_media_type_from_json from pulp_container.constants import MEDIA_TYPE @@ -21,8 +22,20 @@ 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) + try: + repository = ContainerRepository.objects.get(pk=repository_pk) + except ContainerRepository.DoesNotExist: + raise TaskResourceNotFound( + f"ContainerRepository matching pk={repository_pk} does not exist. It may " + "have been deleted after this task was dispatched." + ) from None + try: + remote = ContainerRemote.objects.get(pk=remote_pk) + except ContainerRemote.DoesNotExist: + raise TaskResourceNotFound( + f"ContainerRemote matching pk={remote_pk} does not exist. It may have been " + "deleted after this task was dispatched." + ) from None 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..55ddcf1f4 100644 --- a/pulp_container/app/tasks/recursive_add.py +++ b/pulp_container/app/tasks/recursive_add.py @@ -1,3 +1,4 @@ +from pulp_container.app.exceptions import TaskResourceNotFound from pulp_container.app.models import ( MEDIA_TYPE, Blob, @@ -22,7 +23,13 @@ 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) + try: + repository = ContainerRepository.objects.get(pk=repository_pk) + except ContainerRepository.DoesNotExist: + raise TaskResourceNotFound( + f"ContainerRepository matching pk={repository_pk} does not exist. It may " + "have been deleted after this task was dispatched." + ) from None 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..d101e68b5 100644 --- a/pulp_container/app/tasks/recursive_remove.py +++ b/pulp_container/app/tasks/recursive_remove.py @@ -2,6 +2,7 @@ from pulpcore.plugin.models import Content, Repository +from pulp_container.app.exceptions import TaskResourceNotFound from pulp_container.app.models import ( MEDIA_TYPE, Blob, @@ -36,7 +37,13 @@ def recursive_remove_content(repository_pk, content_units): should be removed from the Repository. """ - repository = Repository.objects.get(pk=repository_pk).cast() + try: + repository = Repository.objects.get(pk=repository_pk).cast() + except Repository.DoesNotExist: + raise TaskResourceNotFound( + f"Repository matching pk={repository_pk} does not exist. It may have been " + "deleted after this task was dispatched." + ) from None 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..6d57e0c61 100644 --- a/pulp_container/app/tasks/sign.py +++ b/pulp_container/app/tasks/sign.py @@ -8,6 +8,7 @@ from pulpcore.plugin.models import Repository +from pulp_container.app.exceptions import TaskResourceNotFound from pulp_container.app.models import ( ManifestSignature, ManifestSigningService, @@ -41,7 +42,13 @@ def sign(repository_pk, signing_service_pk, reference, tags_list=None): should be signed. """ - repository = Repository.objects.get(pk=repository_pk).cast() + try: + repository = Repository.objects.get(pk=repository_pk).cast() + except Repository.DoesNotExist: + raise TaskResourceNotFound( + f"Repository matching pk={repository_pk} does not exist. It may have been " + "deleted after this task was dispatched." + ) from None latest_version = repository.latest_version() if tags_list: latest_repo_content_tags = latest_version.content.filter( @@ -55,7 +62,13 @@ 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) + try: + signing_service = ManifestSigningService.objects.get(pk=signing_service_pk) + except ManifestSigningService.DoesNotExist: + raise TaskResourceNotFound( + f"ManifestSigningService matching pk={signing_service_pk} does not exist. " + "It may have been deleted after this task was dispatched." + ) from None async def sign_manifests(): added_signatures = [] diff --git a/pulp_container/app/tasks/synchronize.py b/pulp_container/app/tasks/synchronize.py index aa2ffa051..523072b68 100644 --- a/pulp_container/app/tasks/synchronize.py +++ b/pulp_container/app/tasks/synchronize.py @@ -11,6 +11,7 @@ ResolveContentFutures, ) +from pulp_container.app.exceptions import TaskResourceNotFound from pulp_container.app.models import ContainerRemote, ContainerRepository from .sync_stages import ContainerContentSaver, ContainerFirstStage @@ -34,8 +35,20 @@ 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) + try: + remote = ContainerRemote.objects.get(pk=remote_pk) + except ContainerRemote.DoesNotExist: + raise TaskResourceNotFound( + f"ContainerRemote matching pk={remote_pk} does not exist. It may have been " + "deleted after this task was dispatched." + ) from None + try: + repository = ContainerRepository.objects.get(pk=repository_pk) + except ContainerRepository.DoesNotExist: + raise TaskResourceNotFound( + f"ContainerRepository matching pk={repository_pk} does not exist. It may " + "have been deleted after this task was dispatched." + ) from None 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..e61566471 100644 --- a/pulp_container/app/tasks/tag.py +++ b/pulp_container/app/tasks/tag.py @@ -1,6 +1,7 @@ from pulpcore.plugin.models import CreatedResource, Repository from pulpcore.plugin.util import get_domain +from pulp_container.app.exceptions import TaskResourceNotFound from pulp_container.app.models import Manifest, Tag @@ -14,9 +15,21 @@ 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) - - repository = Repository.objects.get(pk=repository_pk).cast() + try: + manifest = Manifest.objects.get(pk=manifest_pk) + except Manifest.DoesNotExist: + raise TaskResourceNotFound( + f"Manifest matching pk={manifest_pk} does not exist. It may have been " + "deleted after this task was dispatched." + ) from None + + try: + repository = Repository.objects.get(pk=repository_pk).cast() + except Repository.DoesNotExist: + raise TaskResourceNotFound( + f"Repository matching pk={repository_pk} does not exist. It may have been " + "deleted after this task was dispatched." + ) from None 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..26eb95215 100644 --- a/pulp_container/app/tasks/untag.py +++ b/pulp_container/app/tasks/untag.py @@ -1,5 +1,6 @@ from pulpcore.plugin.models import Repository +from pulp_container.app.exceptions import TaskResourceNotFound from pulp_container.app.models import Tag @@ -7,7 +8,13 @@ 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() + try: + repository = Repository.objects.get(pk=repository_pk).cast() + except Repository.DoesNotExist: + raise TaskResourceNotFound( + f"Repository matching pk={repository_pk} does not exist. It may have been " + "deleted after this task was dispatched." + ) from None latest_version = repository.latest_version() tags_in_latest_repository = latest_version.content.filter(pulp_type=Tag.get_pulp_type())