Skip to content

Commit

Permalink
Remove backward compatibility for manifest with artifact
Browse files Browse the repository at this point in the history
This commit removes support for manifests storing their data inside an
artifact instead of using the recently introduced  Manifest.data text
field.

closes #1621
  • Loading branch information
MichalPysik committed Jun 3, 2024
1 parent 2c7a4da commit 0657cbb
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 141 deletions.
1 change: 1 addition & 0 deletions CHANGES/1621.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Removed backward compatibility support for manifests which store their data inside an artifact.
42 changes: 0 additions & 42 deletions pulp_container/app/redirects.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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):
"""
Expand Down
44 changes: 0 additions & 44 deletions pulp_container/app/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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),
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion pulp_container/app/registry_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
22 changes: 4 additions & 18 deletions pulp_container/app/tasks/sign.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
41 changes: 5 additions & 36 deletions pulp_container/app/tasks/sync_stages.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
determine_media_type,
validate_manifest,
calculate_digest,
get_content_data,
)

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -470,22 +453,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
Expand Down

0 comments on commit 0657cbb

Please sign in to comment.