Add support for manifest list signature removal and sparse manifest list stripping#2892
Conversation
|
Hi @mtrmac, I added the I did not add the Please let me know what you think about it. |
*shrug* I don’t see a strong reason to think users want that feature individually, but it’s easier / more consistent to implement that way, so, why not.
Yeah, we’d have to move all of the (ACK to the general structure from a very brief skim, I’ll try to do a more careful review soonish.) |
| require.NoError(t, err) | ||
| assert.Equal(t, 3, len(manifestFiles), "Expected manifest list + 2 platform manifests after stripping") | ||
| } | ||
|
|
There was a problem hiding this comment.
It would be good to have a test that calls both --remove-list-signatures and --remove-signatures to make sure all get wiped.
There was a problem hiding this comment.
It might also be good to have a test to make sure --strip-sparse-manifest-list fails without a --remove*signatures option
There was a problem hiding this comment.
There was a problem hiding this comment.
It would be good to have a test that calls both --remove-list-signatures and --remove-signatures to make sure all get wiped.
Skipped — went with mtrmac's suggestion to reject both flags together instead. Implemented on d5d3ab84.
There was a problem hiding this comment.
It might also be good to have a test to make sure --strip-sparse-manifest-list fails without a --remove*signatures option
Implemented on 4646b190.
|
@aguidirh looks like you need to update the branch. |
|
Changes LGTM overall. |
mtrmac
left a comment
There was a problem hiding this comment.
Thanks!
As usual, naming things tends to be the hardest part :)
|
@mtrmac are these tests flakies? Because |
The same, newly added, test is failing in all three runs, I think this very likely to be legitimate. I guess, but did not verify, that the image is not signed, so that error path does not engage. I’m not very sure what
I think most tests there are skipped without |
mtrmac
left a comment
There was a problem hiding this comment.
Thanks! LGTM apart from the failing TestCopyPlatformListStripRemovedPlatformsFailsWithoutSignatureRemoval
|
@aguidirh needs rebase |
Sorry for the delay, I will have a look on the tests failing at some point next week. |
|
@mtrmac I just fixed the tests that were failing and I also submitted one commit adding the mutually exclusive test tags Please let me know if there is something else to be done on this PR or if I should squash the commits. |
mtrmac
left a comment
There was a problem hiding this comment.
Thanks!
One last nit; please squash the commits.
…stripping This commit introduces two new flags for the skopeo copy command to provide more granular control over multi-architecture image copying: --remove-list-signatures: Removes only the manifest list signature while preserving per-instance signatures. This provides finer control than the existing --remove-signatures flag which removes all signatures. --strip-sparse-manifest-list: Strips missing instances from manifest lists when copying only a subset of platforms (using --multi-arch with a platform list). This is useful for registries that don't support sparse manifest lists. The --strip-sparse-manifest-list flag requires explicit signature removal using either --remove-signatures or --remove-list-signatures, as stripping instances invalidates the manifest list signature. These flags address scenarios where users need to copy multi-architecture images to registries with different signature requirements or those that don't support sparse manifest lists, while still maintaining control over which signatures are preserved. Documentation has been updated with usage examples, and integration tests have been added to verify the new functionality. Signed-off-by: Alex Guidi <aguidi@redhat.com>
|
@mtrmac I believe the test failing is a flaky test, because I won't renamed the Test for something else and squashed the commits. |
|
Thanks! |
Summary
This PR introduces two new flags for the
skopeo copycommand to provide more granular control over multi-architecture image copying:--remove-list-signatures: Removes only the manifest list signature while preserving per-instance signatures--strip-sparse-manifest-list: Strips missing instances from manifest lists when copying specific platformsRelates to #2882
Motivation
When copying multi-architecture images with platform-specific selections, users currently face two challenges:
--remove-signaturesflag removes ALL signatures (both manifest list and per-instance), but sometimes users only want to remove the manifest list signatureChanges
New Flags
--remove-list-signatures--remove-signatures--remove-signaturestakes precedence--strip-sparse-manifest-list--remove-signaturesor--remove-list-signaturesDocumentation
docs/skopeo-copy.1.mdwith flag descriptions and usage examplesTests
TestCopyPlatformListWithStripSparseintegration testTestSharedCopyOptionsCopyOptionsunit tests to cover new flagsTest Plan
make test-unit-localmake test-integration-localmake validate-localmake validate-docsExample Usage
Copy specific platforms and strip the sparse manifest list:
Remove only manifest list signatures: