Break down image to repository, tag and registry

[rasheedamir]

According to https://docs.renovatebot.com/modules/manager/helm-values/#additional-information the values.yaml need to be structured like this for it to pick up the values:

image:
  repository: 'some-docker/dependency'
  tag: v1.0.0
  registry: registry.example.com # optional key, will default to "docker.io"

[rasheedamir]

@d3adb5 @aslafy-z Any one of you would like to pick it up? This will need major bump

[d3adb5]

At first I was a little confused about whether this change was to allow the Renovate bot to update Docker dependencies for this chart or to allow chart users to have Renovate update the dependencies they choose to wrap around with this chart. Turns out both outcomes are obtained.

However, careful inspection of the Renovate bot's source code reveals that right now we're already compliant (for the most part) with their specification: line 21 of helm-values/extract.ts shows the registry key truly is optional here, as it's simply prepended to ${repository}:${tag}. Additionally, running Renovate locally shows that where image: is found, a dependency is detected:

docker run -it --rm -e LOG_LEVEL=debug -v $(pwd):/usr/src/app:ro renovate/renovate:latest --platform=local
# ...
         "helm-values": [
           {
             "deps": [
               {
                 "depName": "repository/image-name",
                 "currentValue": "",
                 "datasource": "docker",
                 "replaceString": "",
                 "versioning": "docker",
                 "autoReplaceStringTemplate": "{{newValue}}{{#if newDigest}}@{{newDigest}}{{/if}}",
                 "updates": [],
                 "packageName": "repository/image-name",
                 "warnings": [],
                 "skipReason": "invalid-value"
               },
               {
                 "depName": "openshift/oauth-proxy",
                 "currentValue": "latest",
                 "replaceString": "openshift/oauth-proxy:latest",
                 "autoReplaceStringTemplate": "{{depName}}{{#if newValue}}:{{newValue}}{{/if}}{{#if newDigest}}@{{newDigest}}{{/if}}",
                 "datasource": "docker",
                 "updates": [],
                 "packageName": "openshift/oauth-proxy",
                 "versioning": "docker",
                 "warnings": [],
                 "skipReason": "invalid-value"
               }
             ],
             "packageFile": "application/values.yaml"
           }
# ...

We can see that Renovate is doing its job if we use the following values on top of the existing ones and run it locally again:

deployment:
  image:
    repository: eclipse-temurin
    tag: 17.0.7_7-jre # should recommend 17.0.8_7-jre
  openshiftOAuthProxy:
    image: openshift/oauth-proxy:v1.0.0 # should recommend v1.1.0

The "invalid_value" we got was due to our use of an empty string for the app image and latest for the OAuth Proxy, it appears, as this is what we get now:

         "helm-values": [
           {
             "deps": [
               {
                 "depName": "eclipse-temurin",
                 "currentValue": "17.0.7_7-jre",
                 "datasource": "docker",
                 "replaceString": "17.0.7_7-jre",
                 "versioning": "regex:^(?<major>\\d+)?(\\.(?<minor>\\d+))?(\\.(?<patch>\\d+))?([\\._+](?<build>\\d+))?(-(?<compatibility>.*))?$",
                 "autoReplaceStringTemplate": "{{newValue}}{{#if newDigest}}@{{newDigest}}{{/if}}",
                 "updates": [
                   {
                     "bucket": "non-major",
                     "newVersion": "17.0.8_7-jre",
                     "newValue": "17.0.8_7-jre",
                     "newMajor": 17,
                     "newMinor": 0,
                     "updateType": "patch",
                     "branchName": "renovate/eclipse-temurin-17.x"
                   }
                 ],
                 "packageName": "eclipse-temurin",
                 "warnings": [],
                 "registryUrl": "https://index.docker.io",
                 "currentVersion": "17.0.7_7-jre",
                 "isSingleVersion": true,
                 "fixedVersion": "17.0.7_7-jre"
               },
               {
                 "depName": "openshift/oauth-proxy",
                 "currentValue": "v1.0.0",
                 "replaceString": "openshift/oauth-proxy:v1.0.0",
                 "autoReplaceStringTemplate": "{{depName}}{{#if newValue}}:{{newValue}}{{/if}}{{#if newDigest}}@{{newDigest}}{{/if}}",
                 "datasource": "docker",
                 "updates": [
                   {
                     "bucket": "non-major",
                     "newVersion": "v1.1.0",
                     "newValue": "v1.1.0",
                     "newMajor": 1,
                     "newMinor": 1,
                     "updateType": "minor",
                     "branchName": "renovate/openshift-oauth-proxy-1.x"
                   }
                 ],
                 "packageName": "openshift/oauth-proxy",
                 "versioning": "docker",
                 "warnings": [],
                 "registryUrl": "https://index.docker.io",
                 "currentVersion": "v1.0.0",
                 "isSingleVersion": true,
                 "fixedVersion": "v1.0.0"
               }
             ],
             "packageFile": "application/values.yaml"
           }
         ]

I also tested other places where image: appeared, such as CronJob jobs, additional containers, and init containers, and they were all properly detected when using obsolete versions of busybox or NGINX.

What we need to do to fully support Renovate's helm-values manager is abandon the digest: key that was introduced in #186. Right now, Renovate simply ignores it.

Using an image digest is a good practice that ensures we do not fall victim to supply chain attacks that change tags we rely on. The digest dispenses the use of tags, and when present, most tools simply ignore the tag you're trying to use:

# This digest is actually for 17.0.7_7-jre, but Docker pulls the image based on the digest.
docker pull eclipse-temurin:17.0.8_7-jre@sha256:bfb9eb9a4a3f29575c900085fce271932692e9bc653e06fc6cfa3a7df1f849d3
# Afterwards, we don't even have eclipse-temurin:17.0.8_7-jre locally, Docker ignored the tag!

From what I witnessed, Renovate will create PRs for new image digests if they are detected, even if the same tag is used. In my opinion, this is dangerous, especially if we get accustomed to accepting Renovate's PRs and decide to merge them automatically or blindly accept new digests without carefully inspecting the new image beforehand.

Removing the digest: key and essentially reverting #186 doesn't take away the ability of our users to use an image digest, but Renovate's behavior forces them to adopt a tag --- which we established will be ignored --- regardless.

[d3adb5]

With all of this taken into consideration, I think we should keep digest: as an option for people who wish to force a specific image digest, with or without a tag, and bypass Renovate's "use the latest digest (for this tag | of all)" suggestions.

What do you think, @rasheedamir? If you'd rather remove the digest option and require tags, I can see sense in that too and don't mind submitting a PR reverting the additions from #186.

[d3adb5]

Did you have a chance to review my findings, @rasheedamir? I'm curious what your thoughts are.

[d3adb5]

@rasheedamir @aslafy-z Reminder to read over what I wrote above. I believe this issue can be closed if its premise was compliance with the Renovate bot.

[d3adb5]

Closing this as resolved/invalid for now for the reasons stated in my comments above. It can be reopened if deemed necessary.