Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions .github/renovate.json5
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@
{
customType: 'regex',
managerFilePatterns: [
'modules/cert-manager/**/*.mk',
'modules/tools/**/*.mk',
'modules/**/*.mk',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the manager widened to modules/**/*.mk, the first packageRules entry (matchManagers: ['custom.regex'] with postUpgradeTasks: make vendor-go learn-image-shas) now also fires for the base image digest PRs.

scripts/learn_image_shas.sh reads only modules/cert-manager/00_mod.mk and runs eight crane digest calls against the cert-manager release images. For a base image PR that is a Go toolchain download, a crane build and network round trips for a no-op. If quay.io is flaky the unrelated task fails the base image PR; if a release tag were ever re-pushed, cert-manager digest edits would land inside a base image PR. #748 shows the task does run and does rewrite that file.

Scope the rule the same way the Tools rule below it already is:

      matchFileNames: ['modules/cert-manager/**'],

[Claude Fable 5.1]

],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new base image PRs will not auto-merge, and there will be two of them per rebuild.

The deleted workflow added the skip-review label (see #735: dco-signoff: yes, skip-review, merged by tide 30 seconds after creation). Tide only merges renovate[bot] PRs that carry skip-review plus dco-signoff: yes (testing config.yaml#L306-L318). The shared preset adds skip-review only for gcr.io/distroless/**, gcr.io/cloud-builders/**, docker.io/library/golang and github.com/cert-manager/** (default.json5), so quay.io/jetstack/base-static* digest PRs will wait for a human. The repo's own groupName: null rule for digest updates also splits the two images into separate PRs; 4 of the last 5 bumps changed both digests in one commit.

cert-manager has the rule to copy (renovate.json5#L47-L58). Place it after the ungroup rule so it wins:

    {
      groupName: 'Base images',
      matchManagers: ['custom.regex'],
      matchFileNames: ['modules/oci-build/**'],
      addLabels: ['skip-review'], // Adding label to allow PRs to automerge
    },

If the intent is that a human now reviews base image bumps, please say so in the PR description.

[Claude Fable 5.1]

matchStrings: [
'(?:^|\\r\\n|\\r|\\n)#\\s*renovate:\\s*datasource=(?<datasource>\\S+)\\s+packageName=(?<packageName>\\S+)(?:^|\\r\\n|\\r|\\n)\\S+ \\+= \\S+=(?<currentValue>\\S+)',
'(?:^|\\r\\n|\\r|\\n)#\\s*renovate:\\s*datasource=(?<datasource>\\S+)\\s+packageName=(?<packageName>\\S+)(?:^|\\r\\n|\\r|\\n)\\S+ (:|\\?)= (?<currentValue>\\S+)'
'(?:^|\\r\\n|\\r|\\n)#\\s*renovate:\\s*datasource=(?<datasource>\\S+)\\s+packageName=(?<packageName>\\S+)(?:^|\\r\\n|\\r|\\n)\\S+\\s+(?:\\+=\\s+[^=]*=|(:|\\?)\\s*=\\s*)(?<currentValue>[^\\s:@]+)\\s*(?:\\r\\n|\\r|\\n|$)',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new end-of-line terminator on this pattern makes Renovate skip any # renovate: entry that directly follows another one.

\s*(?:\r\n|\r|\n|$) consumes the newline after the value (and any blank lines after it, via the greedy \s*). Renovate applies matchStrings with a global regex, so matches do not overlap. The next entry's leading (?:^|\r\n|\r|\n) anchor then has no newline left to match and that entry is silently dropped. It only works today because every tools += entry happens to have a # https://... comment line above it.

Reproduced with Python re (same non-overlapping semantics as RE2 + g):

# renovate: datasource=github-releases packageName=a/a
tools += a=v1
# renovate: datasource=github-releases packageName=b/b
tools += b=v2

new pattern: ['a/a']; the two patterns on main: ['a/a', 'b/b']. Same result with a blank line between the entries. A value with a trailing comment (x := v1.2.3 # pinned) also stops matching.

The rewrite is not needed for the exclusivity Copilot asked for. The old patterns already cannot match the two docker lines because they require \s+packageName= in the comment, and the docker comments have none. I checked: old patterns and new pattern give identical matches (47/47) on modules/tools and modules/cert-manager, and both give 0 matches on the two base_image_* lines.

Suggested change
'(?:^|\\r\\n|\\r|\\n)#\\s*renovate:\\s*datasource=(?<datasource>\\S+)\\s+packageName=(?<packageName>\\S+)(?:^|\\r\\n|\\r|\\n)\\S+\\s+(?:\\+=\\s+[^=]*=|(:|\\?)\\s*=\\s*)(?<currentValue>[^\\s:@]+)\\s*(?:\\r\\n|\\r|\\n|$)',
'(?:^|\\r\\n|\\r|\\n)#\\s*renovate:\\s*datasource=(?<datasource>\\S+)\\s+packageName=(?<packageName>\\S+)(?:^|\\r\\n|\\r|\\n)\\S+ \\+= \\S+=(?<currentValue>\\S+)',
'(?:^|\\r\\n|\\r|\\n)#\\s*renovate:\\s*datasource=(?<datasource>\\S+)\\s+packageName=(?<packageName>\\S+)(?:^|\\r\\n|\\r|\\n)\\S+ (:|\\?)= (?<currentValue>\\S+)',

[Claude Fable 5.1]

'(?:^|\\r\\n|\\r|\\n)#\\s*renovate:\\s*datasource=(?<datasource>\\S+)(?:\\s+packageName=(?<packageName>\\S+))?(?:^|\\r\\n|\\r|\\n)\\S+\\s+:=\\s+(?<depName>[^@:\\s]+):(?<currentValue>[^@\\s]+)(?:@(?<currentDigest>sha256:[a-f0-9]+))?',
],
},
{
Expand Down
97 changes: 0 additions & 97 deletions .github/workflows/base-images-upgrade.yaml

This file was deleted.

6 changes: 0 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,6 @@ include modules/generate-verify/02_mod.mk

## Upgrade targets

.PHONY: upgrade-base-images
upgrade-base-images: | $(NEEDS_CRANE)
@CRANE=$(CRANE) \
./scripts/upgrade_base_images.sh

# Upgrade the kind images to the latest available version from
# the kind release description. This script is useful when kind publishes
# a new kubernetes image and updates the kind release description.
Expand Down Expand Up @@ -135,7 +130,6 @@ test-e2e:
help: ## Show this help
@echo "Usage: make [target] ..."
@echo
@echo "make upgrade-base-images"
@echo "make upgrade-kind-images"
@echo
@echo "make learn-golang-shas"
Expand Down
8 changes: 4 additions & 4 deletions modules/oci-build/00_mod.mk
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
# limitations under the License.

# Use distroless as minimal base image to package the manager binary
# To get latest SHA run "crane digest quay.io/jetstack/base-static:latest"
base_image_static := quay.io/jetstack/base-static@sha256:dc719ea428e9b88843862bd8de896bbe974088bc838d000a33f091a835744c33
# renovate: datasource=docker
base_image_static := quay.io/jetstack/base-static:latest@sha256:dc719ea428e9b88843862bd8de896bbe974088bc838d000a33f091a835744c33
Comment thread
erikgb marked this conversation as resolved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding :latest to these values exists only to give the second regex a currentValue. Renovate's currentValueTemplate does that without changing the data, and cert-manager already uses it for the same job (renovate.json5#L14-L27, currentValueTemplate: 'nonroot'; it produced #9350 and #9329 this month).

A separate manager scoped to this file would replace the second matchStrings entry, the optional packageName group, the widening to modules/**/*.mk, and both # renovate: comments:

    {
      customType: 'regex',
      managerFilePatterns: ['modules/oci-build/00_mod.mk'],
      matchStrings: [
        '(?<depName>quay\\.io/jetstack/[^@\\s]+)@(?<currentDigest>sha256:[a-f0-9]{64})',
      ],
      datasourceTemplate: 'docker',
      currentValueTemplate: 'latest',
    },

Why it matters:

  • The value shape flows to the eight repos that klone oci-build and feed it to ko as .defaultBaseImage (01_mod.mk:32). ko handles repo:tag@sha256:... correctly (go-containerregistry NewDigest strips the tag), so nothing breaks, but every consumer gets a churn diff and a :latest tag that reads as floating when the digest pins it.
  • The two matchStrings are only disjoint because the docker values contain : and the tools values do not. The second regex also accepts only := (not ?=), stops depName at the first : (registry ports), and does not match the pre-PR repo@sha256:... form; all fail silently.
  • The deleted crane digest quay.io/jetstack/base-static:latest hint was the only documented manual bump path. With a scoped manager the lines and their comments can stay exactly as on main.

[Claude Fable 5.1]


# Use custom apko-built image as minimal base image to package the manager binary
# To get latest SHA run "crane digest quay.io/jetstack/base-static-csi:latest"
base_image_csi-static := quay.io/jetstack/base-static-csi@sha256:83a2ffc4266da49036b08fef76e8f424a76b12ab870a702231bb4132ae3dbb88
# renovate: datasource=docker
base_image_csi-static := quay.io/jetstack/base-static-csi:latest@sha256:83a2ffc4266da49036b08fef76e8f424a76b12ab870a702231bb4132ae3dbb88

# Utility functions
fatal_if_undefined = $(if $(findstring undefined,$(origin $1)),$(error $1 is not set))
Expand Down
63 changes: 0 additions & 63 deletions scripts/upgrade_base_images.sh

This file was deleted.

Loading