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

Handle image names with an image digest instead of a tag #885

Open
mithrandi opened this issue Dec 22, 2017 · 9 comments · May be fixed by #2162

Comments

@mithrandi
Copy link
Contributor

@mithrandi mithrandi commented Dec 22, 2017

An image name like this: gcr.io/decisive-cinema-167507/kube-cert-manager@sha256:4e79e70added5a2e95fb10bf9a63e4234395a3e047c9bf6a6c1131c383c9f04a

Will cause an error like this:

ts=2017-12-22T15:43:30.33587349Z caller=warming.go:154 component=warmer err="requesting tags: mux: variable \"decisive-cinema-167507/kube-cert-manager@sha256\" doesn't match, expected \"^(?:(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])(?:(?:\\\\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(?::[0-9]+)?/)?[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?$\""

This can probably be fixed by just adding more craziness to the regex, although it'll take me a little while to comprehend what is already there.

@squaremo

This comment has been minimized.

Copy link
Member

@squaremo squaremo commented Dec 22, 2017

Yeah we should bite the bullet and support image refs with digests. I don't know what we'll do about automation -- maybe ignore them? -- but we could certainly understand them at least.

@squaremo squaremo changed the title Handle image names with an image ID instead of a tag Handle image names with an image digest instead of a tag May 29, 2018
@Nogbit

This comment has been minimized.

Copy link

@Nogbit Nogbit commented Dec 19, 2018

Any update on this? It still clutters the logs every 2-4 times per minute.

@munnerz

This comment has been minimized.

Copy link

@munnerz munnerz commented Mar 6, 2019

I'd be happy to take a look at this, as I much prefer pinning my images with sha256 tags.

Is there anything in particular that needs doing here aside from relaxing the regex? I see this issue has "blocked-design" and wonder what extra complexities in flux I may not be aware of that this could affect 😄

@squaremo

This comment has been minimized.

Copy link
Member

@squaremo squaremo commented Mar 6, 2019

Is there anything in particular that needs doing here aside from relaxing the regex? I see this issue has "blocked-design"

Lots of the code, especially automated updates, assumes it's dealing with image tags. That will have to be adapted, and how it's adapted needs some thinking through (-> "blocked-design").

@2opremio

This comment has been minimized.

Copy link
Collaborator

@2opremio 2opremio commented Mar 6, 2019

@munnerz we need to think about what to do with digest references with respect to updates.

  1. Regarding automatic updates:

    I don't think we should ignore them, I would say we do business as usual (i.e. if the workload is annotated as flux.weave.works/automated: "true" we just update it as we used to, regardless of its tag being a sha256 reference).

    However (at least for now) I don't think we should do any smart stuff such us:

    • using the tag of the image the sha256 refers to for semver comparison
    • using a sha256 of the promoted image if an update happens (i.e. if an update happens, automatic or not, we use the tag).
  2. Regarding manual updates (e.g. fluxctl release) we should understand them an update workload containers to the image provided as usual.

To summarize, if @squaremo agrees, I think this would entail:

  1. Making sure image digest references are understood by flux when:
    • read from a workload manifest in Git and a workload in K8s
    • provided as an argument to fluxctl and, more generally, the HTTP & net/rpc APIs
  2. Making sure that we can distinguish the digest references from the usual tag references , updating the image.Ref if necessary and making sure backwards compatibility isn't broken in the API.
@squaremo

This comment has been minimized.

Copy link
Member

@squaremo squaremo commented Mar 6, 2019

@2opremio Yes I'd go along with that. It's not the absolute minimum that could be done (which is something like, parse images refs with digests but skip them when updating things), but perhaps the minimal satisfactory step to take.

@carlosjgp

This comment has been minimized.

Copy link

@carlosjgp carlosjgp commented Jun 14, 2019

@2opremio , @squaremo
I think the point of using sha256 hash as an image tag is avoiding upgrades and stick to that particular version always. This is particularly useful for images that only publish latest tags and you can't choose between receiving patches, minor or major upgrades

eg: https://hub.docker.com/r/solsson/kafka-prometheus-jmx-exporter/tags

So in the case of a workload annotated to receive upgrades you might want to upgrade only 1 of the containers of the pod.

Would you accept a PR to remove the log error about not understanding the image tag? It's filling our logs with a lot of noise :(

@carlosjgp

This comment has been minimized.

Copy link

@carlosjgp carlosjgp commented Jun 14, 2019

It might not be clear on my previous comment but on my comment "Remove the log error"... I meant by extending the regex, adding a special condition or any other approach without affecting the current Flux functionality

@2opremio

This comment has been minimized.

Copy link
Collaborator

@2opremio 2opremio commented Jun 14, 2019

I would much rather get a contribution implementing digest support (you have tips for how to this in my comment above)

carlosjgp added a commit to carlosjgp/flux that referenced this issue Jun 15, 2019
In some occasions the same tag can be assign to different images.
Like for example the official Python images tagged with `3.7` or other
projects which may not have a labeling strategy which only use label
like `latest` or `master`. Using digests instead of labels ensures that
you don't recieve unexpected updagrades

More info about tags and digests can be found
[here](https://success.docker.com/article/images-tagging-vs-digests)
Fixes fluxcd#885
carlosjgp added a commit to carlosjgp/flux that referenced this issue Jun 15, 2019
In some occasions the same tag can be assigned to different images.
Like for example the official Python images tagged with `3.7` or other
projects which may not have a labeling strategy which only uses a label
like `latest` or `master`. Using digests instead of labels ensures that
you don't receive unexpected upgrades

More info about tags and digests can be found
[here](https://success.docker.com/article/images-tagging-vs-digests)

Fixes fluxcd#885
@carlosjgp carlosjgp referenced a pull request that will close this issue Jun 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.