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

Support Docker image references with tag and digest #1736

Open
zregvart opened this issue Nov 30, 2022 · 8 comments
Open

Support Docker image references with tag and digest #1736

zregvart opened this issue Nov 30, 2022 · 8 comments

Comments

@zregvart
Copy link

Currently Docker image references with tag and digest are not supported.
This leads to failure when trying to invoke tools that depend on containers/image, like skopeo:

$ skopeo inspect docker://repository.io/repository/image:14d68911e6d941f997e8693bc6a5d92c@sha256:902ed9a2410e9b58dfe7b0259af41e13193310b2bd82b848a21f128fa701db52 
FATA[0000] Error parsing image name "docker://repository.io/repository/image:14d68911e6d941f997e8693bc6a5d92c@sha256:902ed9a2410e9b58dfe7b0259af41e13193310b2bd82b848a21f128fa701db52": Docker references with both a tag and digest are currently not supported 
zregvart added a commit to zregvart/build-definitions that referenced this issue Nov 30, 2022
The `sanity-inspect-image` Task is using `skopeo` that currently doesn't
support image references containing both tag and digest
(see containers/image#1736). The task has been
modified to check if the image reference contains both and in that case
the tag is removed and digest is used solely to reference the image.
@mtrmac
Copy link
Collaborator

mtrmac commented Nov 30, 2022

Thanks for your report.

Yes, and that’s mostly intentional.

The historical reasoning is containers/buildah#1407 (comment) .

Since then, https://github.com/moby/moby/releases/tag/v20.10.20 has changed again to the older and less problematic “require both the tag and digest to match” behavior; so that at least removes the strongest reason to completely and strictly reject such input.

Still, now we have to worry about c/image users assuming that valid references only have one of (tag, digest), being broken if c/image starts accepting both. That’s just a worry, not a complete show-stopper.

But then the benefit of accepting the :tag@digest syntax, in the first place, is unclear — in which situations does the consumer, which has a specific digest pinned, really care that the tag still exists and points to that tag? Would the consumer not be better off removing the tag from the input, and keeping the deployment/build process working even if the tag is moved?

So… I think it’s definitely worth discussing, but it’s not obvious to me that it’s worth the risk and review effort.

zregvart added a commit to konflux-ci/build-definitions that referenced this issue Nov 30, 2022
The `sanity-inspect-image` Task is using `skopeo` that currently doesn't
support image references containing both tag and digest
(see containers/image#1736). The task has been
modified to check if the image reference contains both and in that case
the tag is removed and digest is used solely to reference the image.
@zregvart
Copy link
Author

I see tags carrying classifying information, tagging an image with production vs staging makes a difference. For floating tags I still wish to distinguish between two different versions of an image tagged with the same tag. Seeing the tag alone doesn't give me tracking, and not seeing the tag doesn't give me the classifier. Having both I can reason about an image that is classified for production being of certain version via the hash. Seeing that in an image reference I can be sure that the right image gets deployed to the right environment with the expected version.

I don't really care how this is handled internally as outlined in the two different ways Docker handles it on the comment you linked. I'd like to use that :tag@digest syntax and I'd like to propagate it throughout all the tools I use, skopeo being one of them and the issue of skopeo not supporting this syntax originates from containers/images.

Seems to me like the second option of, disregarding the tag if the digest is specified would not be difficult to implement, if that's something the project is willing to accept I'd be happy to contribute a pull request.

@mtrmac
Copy link
Collaborator

mtrmac commented Nov 30, 2022

Are you saying that you want to intentionally, simultaneously, use image:production@digest1 and image:production@digest2?

That’s seems so disconnected from how the registries actually operate that I think that should be strictly rejected. The tag field is not a general-purpose comment storage space.

@zregvart
Copy link
Author

zregvart commented Nov 30, 2022

I don't care how registries operate, I care how the tools are used. I want to be certain that an image tagged with production of a certain digest is the one I'm operating on. Not given that opportunity I'll need to resort to using digests or tags alone. Given tags or digests alone I can't tell looking at the image reference if the image in question is for production and the correct version.
I outlined the second option as easy to implement from the comment, I also offer to provide a pull request for the first option. Again, I'm happy to contribute this if there is a consensus in which direction the implementation should be heading.
Seems to me the trade-off is between correctness and compatibility, and one of those needs to give.

@mtrmac
Copy link
Collaborator

mtrmac commented Dec 1, 2022

I don't care how registries operate, I care how the tools are used. I want to be certain that an image tagged with production of a certain digest is the one I'm operating on.

AFAICS that makes the rolling production tag contradictory: it can’t be updated it to a newer digest because it would break production deployments. It just doesn’t work at the scale where deployments are automated and automatically re-started. Hence my “not a comment storage space” description. The deployment configuration must choose between deploying the tag, whichever digest it points to, and deploying the digest, whether or not the tag points to — or the practical certainty of downtime that could be avoided either of the two other choices.

Seems to me the trade-off is between correctness and compatibility, and one of those needs to give.

There are actually more layers to this: containers/common#1248 at a caller of c/image. c/common values compatibility more; but as the lower infrastructure, c/image is primarily in the business of reliably and correctly doing what the caller asks it to do, or refusing to do it if it can’t be done.

That c/common issue might end up requiring the c/image feature to be implemented, purely for compatibility. But it will remain a bad idea to use in production, IMHO.

@zregvart
Copy link
Author

zregvart commented Dec 1, 2022

Thank you for raising containers/common#1248, I'm looking forward to the conclusion there.

@eero-t
Copy link

eero-t commented May 20, 2024

AFAICS that makes the rolling production tag contradictory: it can’t be updated it to a newer digest because it would break production deployments. It just doesn’t work at the scale where deployments are automated and automatically re-started.

Using digests to verify floating tags does have a use-case. You run test for a floating tag with digest, and if image passes tests, add non-floating tag for that digest. After which the floating tag can be updated. Digest is extra safety to make sure test results match the final tag.

Alternatives, and why they would be worse:

  • Adding non-floating tag beforehand: a version/date tag is typically an indication of some extra testing being passed, so adding it beforehand and removing it later on if tests do not pass, is violation of that expectation
  • Using just digest for testing: with a tag, it's much easier to see from e.g. logs which of the image variant is being tested and potentially causing trouble

(I assume it's obvious why digest for non-floating tags is desirable.)

@jhmartin
Copy link

Image tags nicely map to build-job ids (main-25) or git tags (1.20.5) and are human-readable. The digest is good for avoiding surprises in what actually gets run. Yes the VCS / build tag could be put in annotations, but the image name is much more readily available.

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

4 participants