From ed8bdf641b93e774141b34cf016f452116816432 Mon Sep 17 00:00:00 2001 From: MichalPysik Date: Thu, 23 May 2024 18:10:26 +0200 Subject: [PATCH] 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 --- CHANGES/1621.bugfix | 1 + pulp_container/app/downloaders.py | 8 +--- pulp_container/app/redirects.py | 42 --------------------- pulp_container/app/registry.py | 44 ---------------------- pulp_container/app/registry_api.py | 2 +- pulp_container/app/tasks/sign.py | 22 ++--------- pulp_container/app/tasks/sync_stages.py | 49 ++++--------------------- 7 files changed, 14 insertions(+), 154 deletions(-) create mode 100644 CHANGES/1621.bugfix diff --git a/CHANGES/1621.bugfix b/CHANGES/1621.bugfix new file mode 100644 index 000000000..5d4b6531a --- /dev/null +++ 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 index d9c6fff1c..e8db2776b 100644 --- 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 index b938a562a..26f8e9e69 100644 --- 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 index e585cf41b..1b213c910 100644 --- 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 index 8ea7d04ba..27fca3c63 100644 --- 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 index e37dfb49b..e8911fa14 100644 --- 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 index 05530bc87..25d701d71 100644 --- 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", []):