Remove backward compatibility for manifest with artifact

This commit removes support for manifests storing their data inside an artifact instead of using the recently introduced Manifest.data text field. closes #1621
pulp · Jun 18, 2024 · ed8bdf6 · ed8bdf6
1 parent 2c7a4da
commit ed8bdf6
Show file tree

Hide file tree

Showing 7 changed files with 14 additions and 154 deletions.
diff --git a/CHANGES/1621.bugfix b/CHANGES/1621.bugfix
@@ -0,0 +1 @@
+Removed backward compatibility support for manifests which store their data inside an artifact.
diff --git a/pulp_container/app/downloaders.py b/pulp_container/app/downloaders.py
@@ -10,8 +10,6 @@
 
 from pulpcore.plugin.download import DownloaderFactory, HttpDownloader
 
-from pulp_container.constants import V2_ACCEPT_HEADERS
-
 log = getLogger(__name__)
 
 HeadResult = namedtuple(
@@ -53,11 +51,7 @@ async def _run(self, handle_401=True, extra_data=None):
             handle_401(bool): If true, catch 401, request a new token and retry.
 
         """
-        # manifests are header sensitive, blobs do not care
-        # these accept headers are going to be sent with every request to ensure downloader
-        # can download manifests, namely in the repair core task
-        # FIXME this can be rolledback after https://github.com/pulp/pulp_container/issues/1288
-        headers = V2_ACCEPT_HEADERS
+        headers = {}
         repo_name = None
         if extra_data is not None:
             headers = extra_data.get("headers", headers)

diff --git a/pulp_container/app/redirects.py b/pulp_container/app/redirects.py
@@ -1,7 +1,6 @@
 from django.conf import settings
 from django.core.exceptions import ObjectDoesNotExist
 from django.shortcuts import redirect
-from django.http import Http404
 
 from pulp_container.app.exceptions import ManifestNotFound
 from pulp_container.app.utils import get_accepted_media_types
@@ -91,47 +90,6 @@ def redirect_to_object_storage(self, artifact, return_media_type):
         )
         return redirect(content_url)
 
-    # TODO: BACKWARD COMPATIBILITY - remove after fully migrating to artifactless manifests
-    def redirect_to_artifact(self, content_name, manifest, manifest_media_type):
-        """
-        Search for the passed manifest's artifact and issue a redirect.
-        """
-        try:
-            artifact = manifest._artifacts.get()
-        except ObjectDoesNotExist:
-            raise Http404(f"An artifact for '{content_name}' was not found")
-
-        return self.redirect_to_object_storage(artifact, manifest_media_type)
-
-    def issue_tag_redirect(self, tag):
-        """
-        Issue a redirect if an accepted media type requires it or return not found if manifest
-        version is not supported.
-        """
-        if tag.tagged_manifest.data:
-            return super().issue_tag_redirect(tag)
-
-        manifest_media_type = tag.tagged_manifest.media_type
-        if manifest_media_type == MEDIA_TYPE.MANIFEST_V1:
-            return self.redirect_to_artifact(
-                tag.name, tag.tagged_manifest, MEDIA_TYPE.MANIFEST_V1_SIGNED
-            )
-        elif manifest_media_type in get_accepted_media_types(self.request.headers):
-            return self.redirect_to_artifact(tag.name, tag.tagged_manifest, manifest_media_type)
-        else:
-            raise ManifestNotFound(reference=tag.name)
-
-    def issue_manifest_redirect(self, manifest):
-        """
-        Directly redirect to an associated manifest's artifact.
-        """
-        if manifest.data:
-            return super().issue_manifest_redirect(manifest)
-
-        return self.redirect_to_artifact(manifest.digest, manifest, manifest.media_type)
-
-    # END OF BACKWARD COMPATIBILITY
-
 
 class AzureStorageRedirects(S3StorageRedirects):
     """

diff --git a/pulp_container/app/registry.py b/pulp_container/app/registry.py
@@ -201,10 +201,6 @@ async def get_tag(self, request):
                 "Content-Type": return_media_type,
                 "Docker-Content-Digest": tag.tagged_manifest.digest,
             }
-            # TODO: BACKWARD COMPATIBILITY - remove after fully migrating to artifactless manifest
-            if not tag.tagged_manifest.data:
-                return await self.dispatch_tag(request, tag, response_headers)
-            # END OF BACKWARD COMPATIBILITY
             return web.Response(text=tag.tagged_manifest.data, headers=response_headers)
 
         # return what was found in case media_type is accepted header (docker, oci)
@@ -214,41 +210,11 @@ async def get_tag(self, request):
                 "Content-Type": return_media_type,
                 "Docker-Content-Digest": tag.tagged_manifest.digest,
             }
-            # TODO: BACKWARD COMPATIBILITY - remove after fully migrating to artifactless manifest
-            if not tag.tagged_manifest.data:
-                return await self.dispatch_tag(request, tag, response_headers)
-            # END OF BACKWARD COMPATIBILITY
             return web.Response(text=tag.tagged_manifest.data, headers=response_headers)
 
         # return 404 in case the client is requesting docker manifest v2 schema 1
         raise PathNotResolved(tag_name)
 
-    # TODO: BACKWARD COMPATIBILITY - remove after fully migrating to artifactless manifest
-    async def dispatch_tag(self, request, tag, response_headers):
-        """
-        Finds an artifact associated with a Tag and sends it to the client, otherwise tries
-        to stream it.
-
-        Args:
-            request(:class:`~aiohttp.web.Request`): The request to prepare a response for.
-            tag: Tag
-            response_headers (dict): dictionary that contains the 'Content-Type' header to send
-                with the response
-
-        Returns:
-            :class:`aiohttp.web.StreamResponse` or :class:`aiohttp.web.FileResponse`: The response
-                streamed back to the client.
-
-        """
-        try:
-            artifact = await tag.tagged_manifest._artifacts.aget()
-        except ObjectDoesNotExist:
-            ca = await sync_to_async(lambda x: x[0])(tag.tagged_manifest.contentartifact_set.all())
-            return await self._stream_content_artifact(request, web.StreamResponse(), ca)
-        else:
-            return await Registry._dispatch(artifact, response_headers)
-        # END OF BACKWARD COMPATIBILITY
-
     @RegistryContentCache(
         base_key=lambda req, cac: Registry.find_base_path_cached(req, cac),
         auth=lambda req, cac, bk: Registry.auth_cached(req, cac, bk),
@@ -284,16 +250,6 @@ async def get_by_digest(self, request):
                     "Content-Type": manifest.media_type,
                     "Docker-Content-Digest": manifest.digest,
                 }
-                # TODO: BACKWARD COMPATIBILITY - remove after migrating to artifactless manifest
-                if not manifest.data:
-                    if saved_artifact := await manifest._artifacts.afirst():
-                        return await Registry._dispatch(saved_artifact, headers)
-                    else:
-                        ca = await sync_to_async(lambda x: x[0])(manifest.contentartifact_set.all())
-                        return await self._stream_content_artifact(
-                            request, web.StreamResponse(), ca
-                        )
-                # END OF BACKWARD COMPATIBILITY
                 return web.Response(text=manifest.data, headers=headers)
             elif content_type == "blobs":
                 ca = await ContentArtifact.objects.select_related("artifact", "content").aget(

diff --git a/pulp_container/app/registry_api.py b/pulp_container/app/registry_api.py
@@ -1084,7 +1084,7 @@ def handle_safe_method(self, request, path, pk):
                 else:
                     raise ManifestNotFound(reference=pk)
 
-            return redirects.issue_manifest_redirect(manifest)
+            return Response(manifest.data)
 
     def get_content_units_to_add(self, manifest, tag):
         add_content_units = [str(tag.pk), str(manifest.pk)]

diff --git a/pulp_container/app/tasks/sign.py b/pulp_container/app/tasks/sign.py
@@ -3,7 +3,6 @@
 import hashlib
 
 from aiofiles import tempfile
-from asgiref.sync import sync_to_async
 from django.conf import settings
 
 from pulpcore.plugin.models import Repository
@@ -99,23 +98,10 @@ async def create_signature(manifest, reference, signing_service):
     """
     async with semaphore:
         # download and write file for object storage
-        if not manifest.data:
-            # TODO: BACKWARD COMPATIBILITY - remove after fully migrating to artifactless manifest
-            artifact = await manifest._artifacts.aget()
-            if settings.DEFAULT_FILE_STORAGE != "pulpcore.app.models.storage.FileSystem":
-                async with tempfile.NamedTemporaryFile(dir=".", mode="wb", delete=False) as tf:
-                    await tf.write(await sync_to_async(artifact.file.read)())
-                    await tf.flush()
-                artifact.file.close()
-                manifest_path = tf.name
-            else:
-                manifest_path = artifact.file.path
-            # END OF BACKWARD COMPATIBILITY
-        else:
-            async with tempfile.NamedTemporaryFile(dir=".", mode="wb", delete=False) as tf:
-                await tf.write(manifest.data.encode("utf-8"))
-                await tf.flush()
-            manifest_path = tf.name
+        async with tempfile.NamedTemporaryFile(dir=".", mode="wb", delete=False) as tf:
+            await tf.write(manifest.data.encode("utf-8"))
+            await tf.flush()
+        manifest_path = tf.name
 
         async with tempfile.NamedTemporaryFile(dir=".", prefix="signature") as tf:
             sig_path = tf.name

diff --git a/pulp_container/app/tasks/sync_stages.py b/pulp_container/app/tasks/sync_stages.py
@@ -34,7 +34,6 @@
     determine_media_type,
     validate_manifest,
     calculate_digest,
-    get_content_data,
 )
 
 log = logging.getLogger(__name__)
@@ -77,25 +76,9 @@ async def _check_for_existing_manifest(self, download_tag):
 
         digest = response.headers.get("docker-content-digest")
 
-        if (
-            manifest := await Manifest.objects.prefetch_related("contentartifact_set")
-            .filter(digest=digest)
-            .afirst()
-        ):
-            if raw_text_data := manifest.data:
-                content_data = json.loads(raw_text_data)
-
-            # TODO: BACKWARD COMPATIBILITY - remove after fully migrating to artifactless manifest
-            elif saved_artifact := await manifest._artifacts.afirst():
-                content_data, raw_bytes_data = await sync_to_async(get_content_data)(saved_artifact)
-                raw_text_data = raw_bytes_data.decode("utf-8")
-            # if artifact is not available (due to reclaim space) we will download it again
-            else:
-                content_data, raw_text_data, response = await self._download_manifest_data(
-                    response.url
-                )
-            # END OF BACKWARD COMPATIBILITY
-
+        if manifest := await Manifest.objects.filter(digest=digest).afirst():
+            raw_text_data = manifest.data
+            content_data = json.loads(raw_text_data)
         else:
             content_data, raw_text_data, response = await self._download_manifest_data(response.url)
 
@@ -333,9 +316,7 @@ async def get_paginated_tag_list(self, rel_link, repo_name):
         while True:
             link = urljoin(self.remote.url, rel_link)
             list_downloader = self.remote.get_downloader(url=link)
-            # FIXME this can be rolledback after https://github.com/pulp/pulp_container/issues/1288
-            # tags/list endpoint does not like any unnecessary headers to be sent
-            await list_downloader.run(extra_data={"repo_name": repo_name, "headers": {}})
+            await list_downloader.run(extra_data={"repo_name": repo_name})
             with open(list_downloader.path) as tags_raw:
                 tags_dict = json.loads(tags_raw.read())
                 tag_list.extend(tags_dict["tags"])
@@ -470,22 +451,8 @@ async def create_listed_manifest(self, manifest_data):
         )
         manifest_url = urljoin(self.remote.url, relative_url)
 
-        if (
-            manifest := await Manifest.objects.prefetch_related("contentartifact_set")
-            .filter(digest=digest)
-            .afirst()
-        ):
-            if manifest.data:
-                content_data = json.loads(manifest.data)
-            # TODO: BACKWARD COMPATIBILITY - remove after fully migrating to artifactless manifest
-            elif saved_artifact := await manifest._artifacts.afirst():
-                content_data, _ = await sync_to_async(get_content_data)(saved_artifact)
-            # if artifact is not available (due to reclaim space) we will download it again
-            else:
-                content_data, manifest = await self._download_and_instantiate_manifest(
-                    manifest_url, digest
-                )
-            # END OF BACKWARD COMPATIBILITY
+        if manifest := await Manifest.objects.filter(digest=digest).afirst():
+            content_data = json.loads(manifest.data)
         else:
             content_data, manifest = await self._download_and_instantiate_manifest(
                 manifest_url, digest
@@ -582,9 +549,7 @@ async def create_signatures(self, man_dc, signature_source):
                 man_dc.content.digest,
             )
             signatures_downloader = self.remote.get_downloader(url=signatures_url)
-            # FIXME this can be rolledback after https://github.com/pulp/pulp_container/issues/1288
-            # signature extensions endpoint does not like any unnecessary headers to be sent
-            await signatures_downloader.run(extra_data={"headers": {}})
+            await signatures_downloader.run()
             with open(signatures_downloader.path) as signatures_fd:
                 api_extension_signatures = json.loads(signatures_fd.read())
             for signature in api_extension_signatures.get("signatures", []):