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 sha256 digest pinning #165

Open
Tracked by #4245
plockaby opened this issue May 11, 2021 · 21 comments · May be fixed by #508
Open
Tracked by #4245

Support sha256 digest pinning #165

plockaby opened this issue May 11, 2021 · 21 comments · May be fixed by #508
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@plockaby
Copy link

I figured it'd be more polite to open a new issue about this rather than spawn two conversations from #164 so I'm quoting from that.

For example, if I'm deploying haproxy:2.2.14 the maintainers will periodically rebuild their image when their base image changes (e.g. they rebuild 2.2.14 because of a CVE in a dependent library like SSL) and they'll release a new "version" of 2.2.14 with the same exact tag but with a different hash. Can/does flux2 pick that up and deploy the new image

This is not possible with any version of Flux today, since Flux works from git, there is no change to apply, and since the image metadata is the only place where Flux could look to know that something has changed, and Flux no longer does anything with image metadata as it is commonly rate limited and expensive to pull the metadata from each image tag one by one, there would not currently be any way for Flux to know if something has changed in the manner you are describing.

I've been thinking about this a bit more. As a user of flux v1 I very much understand the rate limiting issues that were arising. However, tags are mutable and (from my understanding) it is considered a good security practice to pin to digests. Indeed, if I have two nodes that run haproxy it is possible that I could end up deploying two different versions of haproxy:2.2.14 if I do not use digest pinning. (I'm going to keep using haproxy as an example. Sorry.) While I don't want to discourage anyone from just putting into their yaml files a tag and being happy, I would like request support for digest pinning.

Here's how I figure it can work:

  1. I put Image: haproxy:2.2.14@sha256:792ac7e9a42dcb028039bd6ec02cfdde75e94980652121e1fe557e7cf8b847bb in my configuration.
  2. Flux says "hey you want 2.2.14, let's go check the digest for 2.2.14 and see if it matches what is pinned".
  3. It matches! Yay, leave it alone.
  4. It does not match. Push a commit with the new/correct digest and update the cluster.

Getting the correct digest fortunately only requires one call to the registry for the one tag and not repeated calls for all tags and also does not need to be made if the flux user isn't using digest pinning. It's as simple as:

REGISTRY=https://index.docker.io/v2
REPO=library
IMAGE=haproxy
TAG=2.2.14
TOKEN=$(curl -sSL "https://auth.docker.io/token?service=registry.docker.io&scope=repository:$REPO/$IMAGE:pull" | jq --raw-output .token)

curl --head -L -H "Accept: application/vnd.docker.distribution.manifest.list.v2+json" -H "Authorization: Bearer ${TOKEN}" "$REGISTRY/$REPO/$IMAGE/manifests/$TAG" | grep "Docker-Content-Digest"

> Docker-Content-Digest: sha256:792ac7e9a42dcb028039bd6ec02cfdde75e94980652121e1fe557e7cf8b847bb

I don't feel like this is outlandish since this is basically the same support that dependabot has:

However, I do know that this has been requested a few times on the v1 repository and rejected. It's ok with me if you don't want to adopt this for v2 but if I don't ask then I can't receive. :)

@yebyen
Copy link

yebyen commented May 12, 2021

That's an interesting perspective. One would think that pinning a digest implies you want to fix the image at that digest, and refuse any others. But as you explained, that's not the only way to read this scrawl - you might just want the digest included for accounting purposes, even just so you can force a reload when it has changed upstream.

This sounds like a potential novel application of ImagePolicy that wouldn't have been possible without overcomplicating in Flux v1. I think we will go on recommending immutable tags, but some folks are not in control of the repos upstream, like I assume you don't in your example, haproxy.

We usually say that image tags must be immutable, but like git tags they are not immutable unless policy is configured preventing them from being overwritten. In the vast majority of image repos in the wild (and git repos, for that matter) this isn't the default behavior and it probably hasn't been configured that way, either.

Thanks for opening a new thread.

@plockaby
Copy link
Author

Oh it just occurs to me to add that I don't want this to ignore the ImagePolicy. I want this to enhance it. That is, if I have an ImagePolicy that says policy.semver.range = 2.2.x and an image that says image = haproxy:2.2.14@sha256:792ac7e9a42dcb028039bd6ec02cfdde75e94980652121e1fe557e7cf8b847bb then when 2.2.15 comes out I would still expect Flux to update the image to be image = haproxy:2.2.15@sha256:abc123..... So my steps above should actually read:

  1. I put image = haproxy:2.2.14@sha256:792ac7e9a42dcb028039bd6ec02cfdde75e94980652121e1fe557e7cf8b847bb in my cluster yaml.
  2. Flux applies any applicable ImagePolicy to see if 2.2.14 should be bumped. If an ImagePolicy matches and there is a version bump then update the tag. If there is a digest then update the digest, too. If there is not a digest then do not add a digest. If Flux updated the tag then we are done because the digest will be to date, too.
  3. If we are here then the ImagePolicy did not find a new tag but there is a digest. Flux says "hey you want 2.2.14, let's go check the digest for 2.2.14 and see if it matches what is pinned". If the digest for 2.2.14 does not match what is pinned then push a commit with the new/correct digest and update the cluster. If the digest for 2.2.14 does match what is pinned then do nothing.

I still agree with you 100% that I would love tags to immutable and I do not want you to recommend it any other way. This is just the situation that I find myself in.

@stefanprodan
Copy link
Member

stefanprodan commented May 12, 2021

@plockaby how would Flux know which digest to chose? Most images are multi-arch and for the same tag there are many digests available.

@plockaby
Copy link
Author

Ha, in the few hours since I started this thread 2.2.14 was indeed replaced with a new digest.

For multiarch images there is always a digest that represents the "parent". So for haproxy:2.2.14, these are the digests of the individual architectures:

linux/s390x: sha256:88d4465a398b20357befa2125b348a2d4d4fd38846a5b0bc40ff403ce3411bb1
linux/arm64/v8: sha256:b4353de7134457cde9239d7f5990288e5a117e247e52fda0a070edb36faf0bc8
linux/arm/v5: sha256:9794c13ea712f7ed6881ea208e2fd383348c09f3c2742d41ba83d5aa49287433
linux/arm/v7: sha256:31f6f79a206ac9c1eede9bfdde9b1d22e24265d7d8e6c0cb5211c6d71312e7ca
linux/mips64le: sha256:e1c2dad45dff2975488f39cc6dc1abca01182d8e3c4413018bfcda7eba01d3ec
linux/386: sha256:481622045fb02fb6b31c3c1aac02351d13a0e075c2781dc0d8fe13ddf240a5b2
linux/amd64: sha256:6706d478e4e608c4d7aff4b100c029ed8ed5387f41f6d7d8b4b6ac9ecba3b745
linux:pp64le: sha256:5f6c4cd389f0eef07efa70a7b79e9034d39ad81308ee3d22b593b48c6e72038f

However, when I ask the registry for the hash then I get this:

$ curl --head -L -H "Accept: application/vnd.docker.distribution.manifest.list.v2+json" -H "Authorization: Bearer ${TOKEN}" "$REGISTRY/$REPO/$IMAGE/manifests/$TAG" 2>&1 | grep -i "docker-content-digest"
> Docker-Content-Digest: sha256:5e3f5e91b42b1d8673b0155fa134396bb746adbe22485e6236b8367c6527b828

And when I restart haproxy and force it to pull a new one on my cluster:

$ kubectl -n loadbalancer describe pod haproxy-tqkzm | grep "Image ID: "
>    Image ID:      docker.io/library/haproxy@sha256:5e3f5e91b42b1d8673b0155fa134396bb746adbe22485e6236b8367c6527b828

This is in fact the same thing that Dependabot does. If you put an architecture specific digest into your Dockerfile dependabot will replace it with the architecture general digest.

@stefanprodan stefanprodan added the enhancement New feature or request label May 12, 2021
@stefanprodan
Copy link
Member

To accommodate the digest patching we could introduce the following setters:

  • {"$imagepolicy": "<policy-namespace>:<policy-name>:digest"}
    replaces the full image URI e.g. docker.io/haproxy:2.2.14@sha256:792ac7e9a42dcb028039bd6ec02cfdde75e94980652121e1fe557e7cf8b847bb
  • {"$imagepolicy": "<policy-namespace>:<policy-name>:tag:digest"}
    replaces the image tag e.g.
    2.2.14@sha256:792ac7e9a42dcb028039bd6ec02cfdde75e94980652121e1fe557e7cf8b847bb

@squaremo
Copy link
Member

However, I do know that this has been requested a few times on the v1 repository and rejected. It's ok with me if you don't want to adopt this for v2

With respect to flux v1, there's a bunch of cruft around dealing with images and their names, and it was hard to see how to fit digests into it. Hence, resistance. But I absolutely see the need for using digests.

For Flux v2 there's a better opportunity to cover that ground. #90 suggests including setters for digests (a suggestion repeated above). fluxcd/image-reflector-controller#87 talks about how to include the digest in the ImagePolicy.

@TrAnn3l
Copy link

TrAnn3l commented Jun 19, 2021

FYI: Here is a detailed writeup about the problem: https://blog.atomist.com/keeping-up-with-docker-official-images/

@kingdonb kingdonb added the good first issue Good for newcomers label Aug 19, 2021
@kingdonb
Copy link
Member

I think we agreed this would be a nice feature, and now it is just waiting for someone to implement it.

(Back up, it will make sense for there to be a more formal proposal first, perhaps this can be posted to Discussions?)

I do not think it makes sense to start work on this before the refactoring is over, but it can still get design cycles and be discussed as a proposal.

@mbrancato
Copy link

I need this as well, and I think it makes sense and would also close #90 by creating a digest setter.

@kingdonb can you elaborate or link to the refactoring going one?

As far as design, what I would propose is that there be another option for ImagePolicyChoice to represent a "pinned tag". For this to work, it makes sense that the setter should be using the digest setter. But there would be benefit to those using semver or other strategies as well. If electing to use the digest value in the tag, not only would they see a change in git when the version updates, but they can also audit when the image itself updated (same semver).

@kingdonb
Copy link
Member

@mbrancato This is the meta-issue to connect all of the meta-issues that are supposed to represent the big refactor:

Digest pinning opens up a lot of new possibilities that wouldn't have made sense with straight tags alone. When an image tag is reused non-immutably (in a way that wouldn't be possible with GitOps without digests) there can be a digest update mentioned in the gitrepo source, such that you can change the image to a later version without changing the tag itself.

This suggests to me the possibility of a "permissive" and "immutable" mode for digest automation policy. You can pin a tag at a particular digest, or you can pin it to the latest observed digest. This would be useful for containers like the memcached tags (that were used in the Flux v1 helm chart), where the software package itself is less frequently released than the base image from upstream, and tags are not considered as immutable (see: docker.io/library/memcached:1.6.10-alpine3.14. This tag gets pushed over by memcached CI infra each time a new alpine base image from upstream is published. It has the behavior of a latest tag in spite of being the most granular image tag offered from this library repo.

Flux should have an automation mode which is fully expressive and compatible with this, it is a common mode of operating.

@grglzrv
Copy link

grglzrv commented Dec 20, 2022

@stefanprodan any update on this matter? is it possible to use this marker {"$imagepolicy": "::tag:digest"}

@kingdonb
Copy link
Member

kingdonb commented Jan 5, 2023

@grglzrv This is not available for now

The $imagepolicy marker must always point at a named image policy in order to match. There is no digest tag policy.

I don't know what might block the digest tag policy type markers from being implemented, as described in #165 (comment)

I think that it gets a lot easier now that we're dropping libgit2 support everywhere across the board. This is one of the major refactors that we've been waiting for, before there can be more structural changes or expansion of features scope.

There is a compelling use case to write image digests directly into YAML manifests, especially considering how you could use them with cosign to make ImageUpdateAutomations actually secure and verified with a CI process that checks for things like policy violations or linter/syntax errors ahead of sending the artifact to the cluster to be applied,

I'm excited about the possibilities in this issue and I think this is something we should prioritize. (I'm flagging this one to be included in the discussion at Bug Scrub today as well, so it gets some more attention!)

@ehudyonasi
Copy link

@kingdonb, Can I try and take this issue? or do you think it is more suitable for a flux member?

@kingdonb
Copy link
Member

@ehudyonasi I'm happy to assign this to you, if you want to work on it!

Please bear in mind that without an RFC, we haven't communicated clearly and exhaustively what this feature should look like, in as few words as possible what I'm trying to say is that it may be easier to submit a PR than to get it merged.

But I'm strongly in support of this feature, and I know we haven't made room for it on any roadmap yet, if you want to work on it, then I'm happy to provide feedback!

@kingdonb
Copy link
Member

kingdonb commented Feb 8, 2023

@ehudyonasi I think since you volunteered to have this issue assigned to you, I've fielded this question no less than 4 times, or reached for this capability myself at least as many times. I'm really interested to hear if you've made progress!

@srgvg
Copy link

srgvg commented Feb 20, 2023

To allow binary authorization to work (GCP context), one can still tag an image, but the digest needs to be there as well.

This means this form:

image: europe-west1-docker.pkg.dev/orgtest/test-container:1.0.0@sha256:01e1f202e226be5034329224d036258e3d43c0eb9300974682876d64b164cdcf

So having a setter like @stefanprodan suggested would work for us.

To accommodate the digest patching we could introduce the following setters:

* `{"$imagepolicy": "<policy-namespace>:<policy-name>:digest"}`
  replaces the full image URI e.g. `docker.io/haproxy:2.2.14@sha256:792ac7e9a42dcb028039bd6ec02cfdde75e94980652121e1fe557e7cf8b847bb`

* `{"$imagepolicy": "<policy-namespace>:<policy-name>:tag:digest"}`
  replaces the image tag e.g.
  `2.2.14@sha256:792ac7e9a42dcb028039bd6ec02cfdde75e94980652121e1fe557e7cf8b847bb`

It seems there is currently no-one working on this however? I'll ask around in our org if we have someone capable of working on this. Is there perhaps somebody able to put us on the right path to start looking at this?

@ehudyonasi
Copy link

@kingdonb, I hope to send a PR until next week, and then from there we can discuss the issues if there will be until we can get it merged :)

makkes pushed a commit that referenced this issue Apr 5, 2023
Using the new `:digest` suffix in policy markers one can now instruct
IAC to store the digest of an image instead of the tag in the given
manifest.

This makes use of the the `.status.imageDigest` field introduced in
fluxcd/image-reflector-controller#368.

closes #165

Signed-off-by: Max Jonas Werner <mail@makk.es>
@makkes makkes linked a pull request Apr 5, 2023 that will close this issue
makkes pushed a commit that referenced this issue May 8, 2023
Using the new `:digest` suffix in policy markers one can now instruct
IAC to store the digest of an image instead of the tag in the given
manifest.

This makes use of the the `.status.imageDigest` field introduced in
fluxcd/image-reflector-controller#368.

closes #165

Signed-off-by: Max Jonas Werner <mail@makk.es>
makkes pushed a commit that referenced this issue May 8, 2023
Using the new `:digest` suffix in policy markers one can now instruct
IAC to store the digest of an image instead of the tag in the given
manifest.

This makes use of the the `.status.imageDigest` field introduced in
fluxcd/image-reflector-controller#368.

closes #165

Signed-off-by: Max Jonas Werner <mail@makk.es>
makkes pushed a commit that referenced this issue May 8, 2023
Using the new `:digest` suffix in policy markers one can now instruct
IAC to store the digest of an image instead of the tag in the given
manifest.

This makes use of the the `.status.imageDigest` field introduced in
fluxcd/image-reflector-controller#368.

closes #165

Signed-off-by: Max Jonas Werner <mail@makk.es>
makkes pushed a commit that referenced this issue May 9, 2023
Using the new `:digest` suffix in policy markers one can now instruct
IAC to store the digest of an image instead of the tag in the given
manifest.

This makes use of the the `.status.imageDigest` field introduced in
fluxcd/image-reflector-controller#368.

closes #165

Signed-off-by: Max Jonas Werner <mail@makk.es>
@ThisGuyCodes
Copy link

ThisGuyCodes commented Jul 22, 2023

I'm so happy to see @makkes PR #508 that makes this available, in particular so I can automate updates of upstream images that don't use any sensible tagging (e.g. just latest).

@makkes PR is untouched for months, despite the fact that he appears to be a flux core maintainer. This is just a long winded way of saying "bump" to try and get some activity I guess?

@makkes
Copy link
Member

makkes commented Jul 24, 2023

@ThisGuyCodes thanks for pushing this up. Flux team was mostly occupied with the GA release and we are slowly digging up the issues that had to be shelved in favor of GA. I took action moving this work further. Expect an update this week.

@BobyMCbobs
Copy link

Hi folks, this is a feature which interests me too. Happy to help however I can

@makkes
Copy link
Member

makkes commented Dec 26, 2023

It's on my list of things to pick up in Q1 2024.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.