Fix multi-platform soci standalone convert#1947
Conversation
|
Hey @sondavidb @Shubhranshu153 @coderbirju can you take a look at this fix 😄 . |
prafgup
left a comment
There was a problem hiding this comment.
@KlwntSingh replied to your comments 😄
Also the the new integration test uses nerdctl to pull and verify all-platforms, previously this integration test would have failed, while other integration tests are untouched in the standalone path which I added previously as a part of #1881
3a5dec7 to
39a613b
Compare
|
Pinging @sondavidb regrading - #1943 (comment) |
|
Seems like one of the checks failed 🤔 "Pre-build / check (pull_request)Pre-build / check (pull_request)" Might be due to me not having |
sondavidb
left a comment
There was a problem hiding this comment.
Will look at the test a little later
Signed-off-by: Praful Gupta <prafulgupta6@gmail.com>
39a613b to
fccc6c4
Compare
prafgup
left a comment
There was a problem hiding this comment.
Addressed comments :) and added the signed-off in the commit.
|
Hey maintainers 🙂! Can someone else review/merge this (#1947) and #1943 (already approved by @\sondavidb) , and potentially release a minor version this with the critical vulnerability fix (already merged) - #1953 , would really help in consuming this package from releases on our end. (cc.ing a couple devs to take a look as well) @KlwntSingh @Swapnanil-Gupta @coderbirju |
|
Thanks. |
|
Resolved comments :) for merge compatibility ! Thanks for the review ! |
Issue #, if available:
Fixes - #1948
Description of changes:
The standalone layout loader parsed only index.json.manifests[0], so any input whose top-level index listed multiple per-platform manifests directly (e.g. from go-containerregistry's layout.Write) lost every platform after the first silently when using --all-platforms, or with a misleading "manifest not found" when using --platform.
Replace the single-element parse with resolveLayoutRoot, which handles all three shapes index.json (single image, nested manifest list, flat per-platform list), filters out children whose blobs are missing, and promotes the top-level index to a content-addressable blob when it is itself the manifest list.
Running the reproduction steps here now gives correct results - #1948
Testing performed:
Added Integration test for multiple platform image.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.