Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove backward compatibility for manifest with artifact #1644

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
8 changes: 1 addition & 7 deletions pulp_container/app/downloaders.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@

from pulpcore.plugin.download import DownloaderFactory, HttpDownloader

from pulp_container.constants import V2_ACCEPT_HEADERS

log = getLogger(__name__)

HeadResult = namedtuple(
Expand Down Expand Up @@ -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)
Expand Down
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bear in mind that we would like to return headers as well, similarly to return web.Response(text=tag.tagged_manifest.data, headers=response_headers).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lubosmj What headers would you like to return in this case?


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
49 changes: 7 additions & 42 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 @@ -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"])
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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", []):
Expand Down
Loading