feat: kustomize image digests

[thepwagner]

Changes:

Modifies the kustomize manager to:

• Support updating digests when specified in the digest, newTag or newName field.
• Skip the pathological case where newTag and digest are both specified - as it's not clear what the user's expectation is here.
• Drop support for newTag: sha256:... which does not seem to be valid kustomize.

This doesn't support injecting digests where they don't already exist, so it is not fully #8089 .

Context:

I'm using this test repository: https://github.com/thepwagner/renovate-kustomize-image-digests

Kustomize images are complicated, the updated README contains several example possibilities. Renovate's implementation was limited to updating the newTag field, but version and digest information might also be included in newName or digest fields. This PR expands the kustomize manager to integrate those fields.

Edge cases exist when the user has provided additional information that kustomize won't use. Those are skipped, with a warning logged:

- name: quay.io/argoproj/argocd
  newTag: v2.0.5-moot # does not matter because digest is provided
  digest: sha256:b53a2f76f949849cbe7e0b1e343a2076c7f205cfcfe8116e461e6861369fa81f

There's a bias towards formats that provide the version+digest within a single YAML value:

- name: quay.io/argoproj/argocd
  newTag: v2.0.5@sha256:b53a2f76f949849cbe7e0b1e343a2076c7f205cfcfe8116e461e6861369fa81f
- name: quay.io/argoproj/argocd
  newName: custom/argo@v2.0.5@sha256:b53a2f76f949849cbe7e0b1e343a2076c7f205cfcfe8116e461e6861369fa81f
- name: quay.io/argoproj/argocd
  newName: custom/argo
  newTag: v2.0.5@sha256:b53a2f76f949849cbe7e0b1e343a2076c7f205cfcfe8116e461e6861369fa81f

So this is not #8089 , but it is enough for my use case and should be backwards compatible.

Documentation (please check one with an [x])

• I have updated the documentation, or
• No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

• Code inspection only, or
• Newly added/modified unit tests, or
• No unit tests but ran on a real repository, or
• Both unit tests + ran on a real repository

[rarkins]

If I understand correctly,

• before, Renovate would increase the tag, which not only wouldn't match the digest but also would be ignored too?
• with this PR, Renovate will increase the digest and never update the tag?

What I'm not sure is: is it the current digest for the existing tag (in which case the result is fully correct) or is it selecting the digest of the latest tag?

I'd really like to update both, but ok with a mid step if it improves the situation. I'm a big fan of having both the informative tag plus synced normative digest.

Also, would it be reasonable to support the combined tag@digest approach only and document that, if users want both updated in lockstep?

[thepwagner]

@rarkins Yep, your assumptions are correct. When digest: is provided, newTag can be treated like a comment.

What I'm not sure is: is it the current digest for the existing tag (in which case the result is fully correct) or is it selecting the digest of the latest tag?

With this proposed implementation+example, I think the latter: since the currentValue parsed from the dependency is undefined any image with a digest: set will be treated as :latest.
That could be improved if newTag was parsed as currentValue (like it is when digest: isn't set), but until the fields are maintained lockstep all options produce ambiguous output.

I'd really like to update both, but ok with a mid step if it improves the situation

This is definitely a mid-step; I was working on a solution that maintained both, but it awkwardly parsed the images: block twice (to deserialize as YAML, and to break into big replaceString tokens) and used a autoReplaceStringTemplate so it might reorder keys - I figured this would be easier to land.

Also, would it be reasonable to support the combined tag@digest approach only and document that, if users want both updated in lockstep?

I think that's the most reasonable middle ground, it's the compromise I was willing to make! I can take this PR in that direction.

• Would it be appropriate to info/warning a message if digest: is provided? or should it continue to be ignored by Renovate?
• Are there docs beyond https://github.com/renovatebot/renovate/blob/e0488a3ebbd8ddc5bbe3d1a303ec4cf31ad6cf7f/lib/manager/kustomize/readme.md to update?

Thanks for the feedback!

[rarkins]

With this proposed implementation+example, I think the latter: since the currentValue parsed from the dependency is undefined any image with a digest: set will be treated as :latest.

I would prefer to avoid this behavior, because I think it's downside is far worse than what we have today. Today, we can mislead people that they updated from version X to version X+1 but actually nothing changed. In other words, they stay on a version which hopefully works.

With this PR's change, we could be updating them to a completely non-working version for them, because sometimes latest tags can be semi-canary builds. And to make it worse: it would look like a simple digest update (low risk) when it could be potentially be breaking.

Have I understood correctly and do you agree?

That could be improved if newTag was parsed as currentValue (like it is when digest: isn't set), but until the fields are maintained lockstep all options produce ambiguous output.

I think this is definitely best. I would only like to do a digest update if it matches newTag. It's a pity though we have no way to "only update digests if newTag is defined and skip any newTag updates", leaving only:

• Digest updates matching existing newTag
• newTag updates only when digest is missing

What if we intentionally skipped any extracted dependency which has both newTag and digest in separate fields and logged a warning that it's unsupported? i.e. support only one field at a time.

This is definitely a mid-step; I was working on a solution that maintained both, but it awkwardly parsed the images: block twice (to deserialize as YAML, and to break into big replaceString tokens) and used a autoReplaceStringTemplate so it might reorder keys - I figured this would be easier to land.

Considering how small these YAML files should be, we shouldn't be concerned with "optimization" of two parse-throughs. But if you think the logic itself is too convoluted then it's another matter.

I think that's the most reasonable middle ground, it's the compromise I was willing to make! I can take this PR in that direction.

Would that align with what I suggested earlier about intentionally skipping extracted deps with both fields defined?

(And let's open a new issue for the advanced case of both fields present, so that one day someone may work out how to elegantly achieve it)

• Would it be appropriate to info/warning a message if digest: is provided? or should it continue to be ignored by Renovate?

I think warning is best.

• Are there docs beyond e0488a3/lib/manager/kustomize/readme.md to update?

I think that's the only one we need.

Thanks!

[viceice]

#8089 (comment)

I've already written some weird samples where it's not clear what should happen.

[thepwagner]

This PR is still in progress - I'll edit the OP and mark RFR when it is.

But an update: I've ported most of _viceice's cases to https://github.com/thepwagner/renovate-kustomize-image-digests.
The most interesting result was that newTag: sha256:abcabcabc doesn't seem to actually work with kustomize-4.2.0. I didn't dig into earlier versions of kustomize that might have supported that syntax, I just updated to return SkipReason.InvalidValue.
I wanted to flag that diff ASAP as it contradicts #7987 - I'm missing some context that might make newTag: sha256:... valid.

[viceice]

@thepwagner did you saw my previous work here: #8110 ?

[thepwagner]

@thepwagner did you saw my previous work here: #8110 ?

Yep! Sorry, I should have mentioned that. I liked the pattern of a regex to match multiple YAML keys. I figured that pattern would need to include newName as well, and didn't like the prospect of abc|acb|bac|bca|cab|cba (since YAML will allow the values in any order).

That's what sent me look at pre-splitting the images: block as a string. Key order wouldn't matter, as long as there was a name: in there somewhere the replaceString contain the fully indented .images[] entry.

I abandoned the latter approach as autoReplaceStringTemplate felt necessary to support injecting newDigest if it didn't already exist (which would afford pinDigests: true and fully close #8089). Doing this while maintaining the user's original key order did not seem like fun. I may have been to focused on maintaining key order. While it reads as weird to me, the following should be supported, right?

  - newTag: 3.12.2
    name: alpine
    digest: sha256:25f5332d060da2c7ea2c8a85d2eac623bd0b5f97d508b165f846c7d172897438
  - digest: sha256:e1488cb900233d035575f0a7787448cb1fa93bed0ccc0d4efc1963d7d72a8f17
    newTag: 1.32.1
    name: busybox:1.30.0
  - newName: amd64/busybox:1.30.1
    name: busybox
    digest: sha256:e1488cb900233d035575f0a7787448cb1fa93bed0ccc0d4efc1963d7d72a8f17

So when I realized newTag: tag@digest worked for my use case, I gave up on multiple keys and started on this priority-driven approach: digest > newTag > newName. It seems correct, but admittedly punishes users that provide too much information that kustomize will ignore. Per the discussion with _rarkins, I think nudging those users towards newTag: tag@digest is a better course.

[HonkingGoose]

I don't understand enough about these changes to confirm/deny that the docs changes are good.

I added a review comment with some things I noticed.

[thepwagner]

@viceice I've addressed comments, please take a look.

[viceice]

ups, some small fix

[renovate-release]

🎉 This PR is included in version 27.4.0 🎉

The release is available on:

• GitHub release
• 27.4.0

Your semantic-release bot 📦🚀