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

need an API for altering image digests #999

Open
vrothberg opened this issue Aug 17, 2021 · 6 comments
Open

need an API for altering image digests #999

vrothberg opened this issue Aug 17, 2021 · 6 comments

Comments

@vrothberg
Copy link
Member

Similar to SetNames(...) for altering the names of an image, I desire an API to alter the digests of an image.

Background: docker rmi foo@RepoDigest is actually removing RepoDigest from foo without removing the image. However, we currently have no way of altering the digests and misbehave.

WDYT @mtrmac @nalind @rhatdan ?

@mtrmac
Copy link
Collaborator

mtrmac commented Aug 17, 2021

I don’t think that’s quite what is going on here? storage.Image.Digests are computed from recorded digests of associated manifests; it’s not a stored property. (It might be possible to just remove one of the manifests, but… the same image can truly and legitimately be repo1@samedigest and repo2@samedigest, and in that case docker rmi repo1@samedigest should not remove repo2@samedigest if I’m not mistaken.)

Isn’t what we want actually a correctly managed RepoDigests? If you search for that, e.g. in Podman, there are several tickets (notably containers/podman#3761 ) discussing how we reliably record only repo:tag values, and separately all digests, whereas the RepoDigests model (and various useful features) would like a full repo@digest association (e.g. on every pull, whether by digest or by tag, record repo@digest associating that digest with that repo, instead of reporting all possible combinations like we usually do in RepoDigests.

@mtrmac
Copy link
Collaborator

mtrmac commented Aug 17, 2021

TBH I don’t immediately know what docker rmi foo@digest does for untagged images — does it remove the image even with multiple RepoDigest values? I assume docker rmi repo@tag does not remove any repoDigest value (it doesn’t know which one), and therefore having several repoDigest values, and a single RepoTag could be fairly frequent occurrence.

Or is it expected to work some other way?

@vrothberg
Copy link
Member Author

TBH I don’t immediately know what docker rmi foo@digest does for untagged images — does it remove the image even with multiple RepoDigest values?

It's only removing the specified RepoDigest from the image.

~ $ sudo docker inspect --format "{{.RepoDigests}}" alpine
[alpine@sha256:eb3e4e175ba6d212ba1d6e04fc0782916c08e1c9d7b45892e9796141b1d379ae]
~ $ sudo docker rmi alpine@sha256:eb3e4e175ba6d212ba1d6e04fc0782916c08e1c9d7b45892e9796141b1d379ae
Untagged: alpine@sha256:eb3e4e175ba6d212ba1d6e04fc0782916c08e1c9d7b45892e9796141b1d379ae
~ $ sudo docker inspect --format "{{.RepoDigests}}" alpine
[]

@flouthoc
Copy link
Collaborator

@mtrmac Do you think we have a consensus on this and should we have this in c/storage cause it seems we need this for containers/common#1053, containers/buildah#3866

@flouthoc
Copy link
Collaborator

One reason for having this API is that if we want to implement solution for containers/buildah#3866 an image could contain digest which is not directly computed by generated from a remote repo and we only keep it in localstorage for a better UX.

@vrothberg vrothberg changed the title need an API for alterting image digests need an API for altering image digests May 31, 2022
@mtrmac
Copy link
Collaborator

mtrmac commented May 31, 2022

  • The “tagging” code from build(deps): bump github.com/moby/sys/mountinfo from 0.4.1 to 0.5.0 #1053 doesn’t need an “add digest reference” API; AddNames is already that, see the pull code.
  • Adding a digested reference to an image via AddNames still wouldn’t work: if the user then used that digested reference to refer to the image, c/image would read the existing manifest from c/storage and then try to validate the manifest against the user-provided digest, which would fail.

So, on push:

  • Get the pushed manifest, and add it to the existing image (SetImageBigDataImageDigestManifestBigDataNamePrefix…)
  • Call AddNames with the digested reference.

On pull, we already add the pushed manifest, and we add a reference that was used for pulling (tagged or digested); we should always record a digested reference even if the image was pulled by tag.

So, it seems attractive to have an “add a manifest and an appropriate digested reference” API, which could be used both for pulls (directly in c/image) and pushes (in libimage).

OTOH #595 (comment) … pulls ultimately shouldn’t use a set of a dozen individual “edit image” calls.

I suppose we can add one more “edit image” call like that, and continue deferring CreateImageAtomically — the new “edit image” feature should be easy enough to incorporate into …Atomically later.

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