Fix various with image pushes. #22
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tofay
requested review from
bacongobbler,
flavio and
thomastaylor312
as code owners
Pushing images with multiple layers now works, because the client has an upload session per layer, rather than an upload session per blob. The push response simlarly doesn't return an image_url, as that doesn't make sense for images with multiple layers. Manifests are canonicalized per https://github.com/opencontainers/image-spec/blob/main/considerations.md?plain=1#L18 The ImageData struct no longer produces a digest from the contained layers as digests are applicable to content blobs, or manifests. Signed-off-by: Tom Fay <tomfay@microsoft.com>
tofay
commented
Feb 3, 2022
shanesveller
reviewed
Feb 3, 2022
Signed-off-by: Tom Fay <tomfay@microsoft.com>
flavio
reviewed
Feb 7, 2022
Signed-off-by: Tom Fay <tomfay@microsoft.com>
flavio
approved these changes
Feb 11, 2022
|
Looking forward for release with this patch. I'm hitting similar issues. |
|
@cardil 👍 I personally would like to go through a bunch of open PR, so that we break the API fewer times :) |
thomastaylor312
pushed a commit
that referenced
this pull request
Mar 2, 2023
Fix various with image pushes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've been trying to use this crate to pull images from one registry and push them to another - I've fixed a few issues I encountered, and added a test for roundtrip pull/pushing an image with multiple layers.
The existing push doesn't work for images with multiple layers because the code conflates chunks and layers. e.g it starts a push session for ImageData, then proceeds to push all the layers as chunks in a single blob.
I've updated this to have a push session per layer, per the spec.
The push response for
Client::pushsimilarly doesn't return an image_url, as thatdoesn't make sense for images with multiple layers.
The ImageData struct no longer produces a digest from the contained
layers as digests are applicable to content blobs, or manifests.
If the manifest being pushed has a media type, then use that instead of the OCI V1 media type. (If the manifest is not the OCI v1 media type then the registry will reject the manifest push.)
Manifests are canonicalized to canonical json per
https://github.com/opencontainers/image-spec/blob/main/considerations.md?plain=1#L18.
I've also updated the manifest structs to not serialize unspecified optional fields as null. That complies with the json schemas in the image-spec repo. The spec wasn't clear on the behaviour here, so I raised opencontainers/image-spec#894. I probably should have done that in a separate PR - happy to drop it from this and submit in a new one if that's contravertial.