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

c8d/pull: Support legacy schema1 prettyjws manifests #46513

Merged
merged 3 commits into from Sep 19, 2023

Conversation

vvoland
Copy link
Contributor

@vvoland vvoland commented Sep 19, 2023

Makes it possible to pull application/vnd.docker.distribution.manifest.v1+prettyjws legacy manifests.

They are not stored in their original form but are converted to the OCI manifests.

- How to verify it

$ docker pull ubuntu:10.04
-Error response from daemon: application/vnd.docker.distribution.manifest.v1+prettyjws not supported
+86b54f4b6a4e: Download complete
+Digest: sha256:fb69ce785f2a695210a62bc4531fc85a0a0f28664d24ee83fb8205bc3763a9df
+Status: Downloaded newer image for ubuntu:10.04
+docker.io/library/ubuntu:10.04

- Description for the changelog

- containerd-integration: Support pulling legacy schema1 images

- A picture of a cute animal (not mandatory but encouraged)

@vvoland vvoland added status/2-code-review kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. area/images containerd-integration Issues and PRs related to containerd integration labels Sep 19, 2023
@vvoland vvoland added this to the 25.0.0 milestone Sep 19, 2023
Copy link
Member

@rumpl rumpl left a comment

Choose a reason for hiding this comment

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

Well that was easy

Makes it possible to pull `application/vnd.docker.distribution.manifest.v1+prettyjws`
legacy manifests.

They are not stored in their original form but are converted to the OCI
manifests.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@@ -104,6 +104,10 @@ func (i *ImageService) PullImage(ctx context.Context, image, tagOrDigest string,
infoHandler := snapshotters.AppendInfoHandlerWrapper(ref.String())
opts = append(opts, containerd.WithImageHandlerWrapper(infoHandler))

// Allow pulling application/vnd.docker.distribution.manifest.v1+prettyjws images
// by converting them to OCI manifests.
opts = append(opts, containerd.WithSchema1Conversion)
Copy link
Member

Choose a reason for hiding this comment

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

do you know if we can detect these in this flow, and print a deprecation warning? (see #46137)

Copy link
Member

Choose a reason for hiding this comment

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

We could print that deprecation in one of the image handlers maybe? That's if the manifest is not already converted once it gets to the handler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can! I somehow missed the deprecation warning when looking at the non-c8d output 😄

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit 1c34831 into moby:master Sep 19, 2023
103 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/images containerd-integration Issues and PRs related to containerd integration kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants