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

Inconsistent and underdocumented behavior when choosing a digest #1823

Open
mtrmac opened this issue Jan 25, 2024 · 4 comments
Open

Inconsistent and underdocumented behavior when choosing a digest #1823

mtrmac opened this issue Jan 25, 2024 · 4 comments

Comments

@mtrmac
Copy link
Collaborator

mtrmac commented Jan 25, 2024

In various locations, notably user-visible “inspect” operations, we expose a single digest value for an image.

That’s fundamentally insufficient because images are deduplicated; hence libimage.Image.Digest has a TODO to deprecate, but we can’t completely remove it, the inspect CLI operations would still need to output something.

What exactly we output is not generally consistent, though. Three notable origins of data:

  • Raw c/storage access; e.g. storageTransport.GetStoreImage(…).Digest . The c/storage.Image.Digest value is typically, by c/image, set when the image is first created; and if it was pulled from a multi-arch image, it is the multi-arch digest.
  • c/image/storageReference.resolveReference, e.g. storageTransport.ResolveReference().Digest for a tagged reference or a storage ID. The value used is BigDataDigests[storage.ImageDigestBigDataKey], set on the last write of a duplicate image, and if that was a multi-arch image pull, it is the per-arch digest.
  • (User input: storageTransport.ResolveReference().Digest when the input reference is a name@digest reference.)

The practical outcome is that podman image list currently returns Image.Digest values, but things like podman image inspect or podman container inspect return BigDataDigests[storage.ImageDigestBigDataKey] — and the two don’t match. That’s, at the very least, counterintuitive.


Fixing this should probably ensure that libimage consistently returns one of the values — and given the need to respect user input, probably the one from c/image/storageReference.resolveReference (but it is an option to change the implementation of that function).

But, also, that work should probably holistically include how multi-arch/per-arch digests are chosen/treated, notably

// FIXME: due to `ImageDigestBigDataKey` we'll always check the
// _last-written_ manifest which is causing issues for multi-arch image
// pulls.
//
// See https://github.com/containers/common/pull/1505#discussion_r1242677279.
,

common/libimage/runtime.go

Lines 360 to 362 in 4b53eca

// NOTE: we must reparse the reference another time below since
// an ordinary image may have resolved into a per-platform image
// without any regard to options.{Architecture,OS,Variant}.
and the like.

@mtrmac
Copy link
Collaborator Author

mtrmac commented Jan 25, 2024

A design question: If the user pulls several per-arch variants of the same multi-arch image, should the “digest” field in podman image list be the same for all variants (the multi-arch manifest)? Or different for all variants (the per-arch manifest)?

Either can be argued to be correct.

@rhatdan
Copy link
Member

rhatdan commented Jan 25, 2024

@vrothberg PTAL

@vrothberg
Copy link
Member

@vrothberg PTAL

I've no time to work on this in case that's the expectation.

If the user pulls several per-arch variants of the same multi-arch image, should the “digest” field in podman image list be the same for all variants (the multi-arch manifest)? Or different for all variants (the per-arch manifest)?

Purely by a gut feeling: I'd vote for individual digest over common list digest. Maybe even differentiate between list digest and individual digest long term? podman image inspect could display the two in separate fields.

@mtrmac
Copy link
Collaborator Author

mtrmac commented Jan 26, 2024

This was motivated by a failure vendoring c/common due to a behavior change. Short-term we adjusted the test, and it is not urgent.

This issue is filed

  • to record the situation as (with some effort) determined
  • to track fixing it; doing that consistently throughout the codebase might be a bit of an effort, and the design is unclear ATM
  • possibly to have a place to point users to; the change in digest values will be user-visible (for users who care at this level of detail) in Podman 5.0, so if it turned out to be hugely disruptive, having a single issue to track should help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants