Skip to content

Conversation

@lsm5
Copy link
Member

@lsm5 lsm5 commented Nov 26, 2025

Add a new manifest.DigestWithAlgorithm function that
allows computing the digest of a manifest using a specified algorithm
(e.g., SHA256, SHA512) while properly handling v2s1 signed manifest
signature stripping.

This addresses the need for skopeo's --manifest-digest flag to support
different digest algorithms while correctly handling all manifest types,
particularly Docker v2s1 signed manifests that require signature
stripping before digest computation.

Note: Currently rebased on #475 .

@github-actions github-actions bot added the image Related to "image" package label Nov 26, 2025
@lsm5 lsm5 force-pushed the digest-redux-manifest branch from e1149c9 to bdbac34 Compare November 26, 2025 19:40
@lsm5 lsm5 changed the title manifest: Add DigestWithAlgorithm function image/manifest: Add DigestWithAlgorithm function Nov 26, 2025
@packit-as-a-service
Copy link

Packit jobs failed. @containers/packit-build please check.

podmanbot pushed a commit to podmanbot/buildah that referenced this pull request Nov 26, 2025
@podmanbot
Copy link

✅ A new PR has been created in buildah to vendor these changes: containers/buildah#6541

Add a new `manifest.DigestWithAlgorithm` function that
allows computing the digest of a manifest using a specified algorithm
(e.g., SHA256, SHA512) while properly handling v2s1 signed manifest
signature stripping.

This addresses the need for skopeo's `--manifest-digest` flag to support
different digest algorithms while correctly handling all manifest types,
particularly Docker v2s1 signed manifests that require signature
stripping before digest computation.

Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
@lsm5 lsm5 marked this pull request as draft December 1, 2025 14:57
@lsm5 lsm5 force-pushed the digest-redux-manifest branch from bdbac34 to d346c06 Compare December 1, 2025 15:00
podmanbot pushed a commit to podmanbot/buildah that referenced this pull request Dec 1, 2025
@lsm5
Copy link
Member Author

lsm5 commented Dec 1, 2025

@containers/container-libs-maintainers could someone rerun the failing test please?

@lsm5
Copy link
Member Author

lsm5 commented Dec 1, 2025

also, is it possible to allow PR authors to rerun failing tests? I'm guessing they could do this by force-pushing anyway, so re-run access to authors shouldn't be a problem imho.

@lsm5
Copy link
Member Author

lsm5 commented Dec 1, 2025

also, is it possible to allow PR authors to rerun failing tests? I'm guessing they could do this by force-pushing anyway, so re-run access to authors shouldn't be a problem imho.

Hmm, looks like if you have sufficient privileges in Cirrus, you can hit the rerun button from there even if you don't see it here. Do we need any consistency between github and cirrus access?

@lsm5 lsm5 marked this pull request as ready for review December 1, 2025 16:11
@Luap99
Copy link
Member

Luap99 commented Dec 1, 2025

also, is it possible to allow PR authors to rerun failing tests? I'm guessing they could do this by force-pushing anyway, so re-run access to authors shouldn't be a problem imho.

Hmm, looks like if you have sufficient privileges in Cirrus, you can hit the rerun button from there even if you don't see it here. Do we need any consistency between github and cirrus access?

That is just how it works. The github UI won't allow re-runs unless you have write permissions in github. While cirrus UI allows re-runs if you have write perms or are the author. So yeah as author you have to log in the cirrus UI to rerun. Personally unless people know what they are doing I would not want contributors to re-run tests by default.
They could just as well add a flake in their tests and hide it by re-running before a maintainer has a chance to see it and then we might merge bad tests. I much rather have maintainers to actually look at the failure to confirm this first before re-running.

@lsm5
Copy link
Member Author

lsm5 commented Dec 1, 2025

@mtrmac good for another look. Thanks!

@lsm5 lsm5 changed the title image/manifest: Add DigestWithAlgorithm function [sha512] image/manifest: Add DigestWithAlgorithm function Dec 2, 2025
Copy link
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM, just a nit to save future readers a bit of time.

func Digest(manifest []byte) (digest.Digest, error) {
// stripManifestSignature strips v1s1 signatures from a manifest if present.
// Returns the manifest bytes (either the original or the unsigned payload).
func stripManifestSignature(manifest []byte) ([]byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please inline this back into the (only) caller again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

image Related to "image" package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants