Key go-installed tool binaries on the Go toolchain that builds them

[wallrj-cyberark]

Fixes: #481

Motivation

The Go 1.27 upgrade (#704) is breaking downstream repositories in a way which does not reproduce on a clean checkout — and which comes and goes between CI runs.

In cert-manager/cert-manager#9174 (the Renovate PR which vendors this module at 455529c), make-verify failed because a cached openapi-gen binary, built by Go 1.26, can no longer parse the Go 1.27 standard library (build log):

+++ Generating ACME openapi...
OpenAPI code generation error: failed making a parser: error(s) in "math/rand/v2":
.../goroot/src/math/rand/v2/rand.go:213:17: method must have no type parameters

A tool built from source is cached at:

$(DOWNLOAD_DIR)/tools/<tool>@<version>_<os>_<arch>

That path says nothing about the Go toolchain which produced the binary, so bumping VENDORED_GO_VERSION does not invalidate it. The download directory is deliberately persisted between CI runs — for cert-manager, preset-local-cache copies _bin/downloaded forward from a hostPath cache shared per Prow node; for other repositories it is a GitLab CI or GitHub Actions cache.

The failure is intermittent because it depends on which node a job lands on. Failed jobs do not write back their cache ("Local cache [update]: Job failed, not updating cache"), and cert-manager's master branch — still on Go 1.26.6, with openapi-gen at the same tool version — keeps re-priming node caches with a Go 1.26-built binary at the same path. A run scheduled on a master-primed node fails; a run on a node with a cold cache rebuilds the tool with Go 1.27 and passes (as the later runs of cert-manager/cert-manager#9174 did, where the job now fails only on the golangci-lint findings addressed by cert-manager/cert-manager#9175). The stale binaries cannot be cleared without purging the node caches by hand.

Changes

Include the Go toolchain version in the download path of tools built with go install:

$(DOWNLOAD_DIR)/tools/<tool>@<version>_<goversion>_<os>_<arch>

where <goversion> is the version of the toolchain which actually builds the tool: go$(VENDORED_GO_VERSION) when Go is vendored, otherwise the system Go's GOTOOLCHAIN=local go env GOVERSION. GOTOOLCHAIN=local stops the query itself triggering a toolchain download at makefile parse time (under the default GOTOOLCHAIN=auto, go env GOVERSION runs after toolchain selection, so a go.mod newer than the installed Go makes it download — or, with the network off, fail silently to an empty key). The value is sanitised because a devel toolchain reports a multi-word GOVERSION which would word-split the generated rules, and it falls back to unknown when no Go is installed so that targets like make help keep working. Keying on the actual toolchain means a system Go upgrade also invalidates the cache, a repository which does not vendor Go cannot mislabel binaries in the shared $HOME/.cache/makefile-modules cache, and such a repository does not rebuild every tool when a module bump changes a VENDORED_GO_VERSION it never uses.

To achieve this:

• tool_defs now exports a $(XXX_DOWNLOAD_PATH) variable instead of declaring the symlink rule directly; go_dependency overrides that variable for the tools it builds; and the symlink rule is generated afterwards, for every tool, from $(XXX_DOWNLOAD_PATH).
• The versioned binary is a normal (not order-only) prerequisite of the unversioned $(bin_dir)/tools/xxx symlink, so rebuilding the binary always re-points the symlink. In the steady state the symlink resolves to that same file, so nothing is remade. Go-built tools additionally depend on a $(bin_dir)/scratch/GO_TOOLCHAIN_VERSION stamp file, which catches reverting to an older, already-cached toolchain, where file modification times cannot be trusted.

Downloaded (non-Go) tools are unaffected: their paths and their learned checksums are unchanged, so make learn-tools-shas needs no rerun.

Known limitations:

• The first build after this lands will rebuild every Go tool once, and the previous, unsuffixed binaries will linger in the caches until they are evicted. No eviction is added here: pruning superseded _go*_ binaries would thrash the $HOME cache shared between repositories on different toolchains. tools: no eviction of superseded Go-toolchain-keyed binaries in the download cache #711 tracks an opt-in prune target as a follow-up.
• If a tool's own go.mod carries a toolchain directive which upgrades beyond the invoking Go, the label understates the truth: the label query is pinned with GOTOOLCHAIN=local, but the go install which builds the tool still runs under the default GOTOOLCHAIN=auto. That does not affect the problem being fixed here, which is that a Go upgrade must invalidate previously built binaries. This is an instance of the GOTOOLCHAIN=auto ambiguity described in Makefile Modules, Go Versions and Vendoring #202; adopting GOTOOLCHAIN=local module-wide there would make the label exact.

Testing

Verified locally against the two repositories where the breakage was observed, by copying the patched module over their vendored copy:

• cert-manager/cert-manager at #9174 — removed _bin/tools/openapi-gen and rebuilt: the binary was rebuilt at openapi-gen@v0.0.0-20260721132016-d427ff9ee9ad_go1.27.0_linux_amd64 and make generate-codegen then produced no diff, i.e. the make-verify failure on that pull request is entirely the stale-binary problem.
• venctl (vendors this module) — a fresh build produced klone@v0.3.0_go1.27.0_linux_amd64; a repeat run was a no-op; setting VENDORED_GO_VERSION := 1.26.5 rebuilt and relinked to klone@v0.3.0_go1.26.5_linux_amd64, and switching back relinked again without rebuilding.
• Simulated a non-vendored downstream repository with a stub go on PATH: a fresh build cached klone@v0.3.0_go1.24.5_linux_amd64 — the system toolchain, not VENDORED_GO_VERSION; a repeat run was a no-op; a toolchain bump rebuilt and re-pointed the symlink; a downgrade re-pointed to the already-cached binary without rebuilding; and a pre-existing symlink whose modification time was newer than every stamp file was still re-pointed after the rebuild.
• Downloaded tools were checked to be unchanged: helm@v4.2.4_linux_amd64, no suffix, no re-download.
• make test-e2e passes in this repository, and now asserts the contract of this PR directly: building _bin/tools/gojq, overriding GO_TOOLCHAIN_VERSION=go0.0.0-test rebuilds and re-points the symlink, and reverting re-points it back to the cached binary without rebuilding.
• scripts/learn_tools_shas.sh was exercised in dry-run mode for one Go tool and one downloaded tool, to confirm the helper makefile still evaluates cleanly under --warn-undefined-variables.

with claude fable-5

[Copilot]

Pull request overview

Keys Go-installed tool binaries by vendored Go version, preventing stale cached binaries after toolchain upgrades.

Changes:

• Adds per-tool download path variables.
• Adds Go versions to Go-built binary paths.
• Relinks tools when the vendored Go version changes.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[SgtCoDFish]

The design is right and the implementation is correct in the happy path. The refactor — tool_defs publishes $(XXX_DOWNLOAD_PATH), go_dependency overrides it, symlink rules generated afterwards from the variable — is a genuine simplification, and the write-up is exemplary.

I checked out the branch and verified the three behaviours the description claims, against a real gojq build:

steady state:      "Prerequisite '.../gojq@v0.12.19_go1.26.7_darwin_arm64' is older than
                    target '_bin/tools/gojq'" -> No need to remake     OK
toolchain bump:    rebuilds at gojq@v0.12.19_go9.9.9_darwin_arm64,
                    symlink re-pointed                                 OK
revert to cached:  relinks to gojq@v0.12.19_go1.26.7_darwin_arm64,
                    no rebuild                                         OK
non-go tools:      HELM_DOWNLOAD_PATH unchanged
                    (helm@v4.2.4_darwin_arm64)                         OK

My substantive comments are inline, and both land on the one new line that computes the key. Two further points that have no good anchor:

Cache growth — the "no eviction" limitation deserves a follow-up issue

You call this out, and I agree pruning the shared $HOME cache is the wrong move here. But the numbers are worth stating: cert-manager builds ~35 Go tools, several of them large (golangci-lint, cosign, syft, goreleaser; openapi-gen is 25MB on its own). A full generation is on the order of 1GB+, and GitHub Actions caps caches at 10GB per repo, so a few Go bumps will start evicting. Prow's hostPath node caches have no eviction at all. Could you file a follow-up for an opt-in prune-tools-cache target, or at least a note in the module README, so this doesn't quietly become someone's disk-full incident?

No test asserts the new behaviour

tests/e2e-projects/test-project/test-config.sh builds only kind, kubectl and etcd explicitly; Go tools are exercised incidentally via generate/verify, and nothing checks invalidation. A cheap addition that would have caught the whitespace bug below:

targets_to_run+=(
    "_bin/tools/gojq"
)

plus an assertion in test_e2e.sh that re-running with GO_TOOLCHAIN_VERSION=go0.0.0-test re-points _bin/tools/gojq, and that reverting re-points it back without rebuilding. That is the whole contract of this PR, and it is currently covered only by the manual testing in the description.

Nothing here blocks the fix in principle, but I'd want the two inline findings addressed before merge, since both silently defeat the invalidation this PR exists to provide.

[SgtCoDFish]

Two problems with this line, both silent, and both fixable together.

1. go env GOVERSION can download a toolchain at parse time, and its failure is swallowed

This is a := at the top level of the non-vendored branch, so it runs in $(CURDIR) — a Go module directory — on every make invocation in every downstream repo that doesn't vendor Go, including make help. With the default GOTOOLCHAIN=auto, if the repo's go.mod go/toolchain directive is newer than the installed Go, this triggers a toolchain download during makefile parsing:

$ cat go.mod          # module x / go 1.27.0, installed go is 1.26.7
$ GOPROXY=off go env GOVERSION
go: downloading go1.27.0 (darwin/arm64)
go: download go1.27.0 for darwin/arm64: toolchain not available
exit=1

When that fails, 2>/dev/null hides the message and $(shell) discards the exit status, so GO_TOOLCHAIN_VERSION is silently empty and every toolchain collapses onto one key:

$ make -n GO_TOOLCHAIN_VERSION= _bin/tools/gojq
... ln -fsn /tmp/mmtest708/dl/tools/gojq@v0.12.19__darwin_arm64 gojq

That is exactly the stale-binary bug this PR fixes, reintroduced — but now invisible, because the path looks keyed. Air-gapped CI, GOPROXY=off, a restricted-egress Prow job, or a bad GOFLAGS all land here.

GOTOOLCHAIN=local fixes both halves, and is the exact analogue of the vendored branch's go$(VENDORED_GO_VERSION) (also a local-toolchain label, not a switched one):

$ GOTOOLCHAIN=local go env GOVERSION     # in the same too-new module dir
go1.26.7                                 # never downloads, never fails

You already note under "Known limitations" that GOTOOLCHAIN=local would make the label exact (#202) — worth pulling that in here, since it buys robustness too, not just accuracy.

2. A whitespace-bearing GOVERSION silently corrupts the generated rules

runtime.Version() for a gotip/devel toolchain is multi-word. Because the value lands unquoted in target names, make word-splits it and the file quietly produces nonsense:

$ make -n 'GO_TOOLCHAIN_VERSION=devel go1.28-abc123 Wed Aug 20' _bin/tools/gojq
source tools//util/lock.sh 20_darwin_arm64; ... go install ...pinact/v4/cmd/pinact@v4.1.1 ...
cd _bin/tools/ && ln -fsn /tmp/.../gojq@v0.12.19_devel go1.28-abc123 Wed Aug 20_darwin_arm64 gojq

Note it built pinact while asked for gojq, and ln got four arguments. No error, no warning. GOTOOLCHAIN=local doesn't help here — a devel toolchain is the local one.

Suggested fix

# GOTOOLCHAIN=local: never trigger a toolchain download while parsing this file,
# and match the go$(VENDORED_GO_VERSION) form used when Go is vendored.
# The awk pass keeps the value safe to embed in a target name: a devel toolchain
# reports a multi-word GOVERSION, which would word-split the generated rules.
GO_TOOLCHAIN_VERSION := $(shell GOTOOLCHAIN=local go env GOVERSION 2>/dev/null | awk '{gsub(/[^A-Za-z0-9._-]/,"-"); print}')
ifeq ($(GO_TOOLCHAIN_VERSION),)
GO_TOOLCHAIN_VERSION := unknown
endif

I'd keep the empty case non-fatal rather than $(error ...): make help and make non-go-tools currently work with no Go installed and shouldn't regress. unknown is at least self-describing in a filename, and nothing can be built in that state anyway.

[wallrj]

Good catch on both counts. Reproduced the download here: go1.25.5 plus a go 1.99.0 module dir, then GOPROXY=off GOTOOLCHAIN=auto go env GOVERSION → go: downloading go1.99.0, exit 1 — and with 2>/dev/null it is completely silent. I had not realised go env GOVERSION goes through toolchain selection like any other subcommand: only GOTOOLCHAIN, GOMOD and GOWORK are exempt (https://github.com/golang/go/blob/go1.25.5/src/cmd/go/internal/toolchain/select.go#L105-L125).

Taken your suggestion as-is: GOTOOLCHAIN=local, the awk sanitiser, and a non-fatal unknown fallback so make help still works with no Go installed. Verified all three cases locally (too-new module dir with GOPROXY=off now yields the local version; the devel string sanitises to devel-go1.28-abc123-Wed-Aug-20; no Go on PATH yields unknown). PR description updated to match.

with claude fable-5

[SgtCoDFish]

Nit: the two _DOWNLOAD_PATH assignments escape differently for the same intent. This one uses $(HOST_OS)/$($(call uc,$1)_VERSION) but $$(GO_TOOLCHAIN_VERSION), while the one in tool_defs uses $$(HOST_OS)/$$($(call uc,$1)_VERSION). Both are := so the results are identical, but a reader has to work that out — and these two lines are precisely the pair you want read side by side. Worth making them consistent.

[wallrj]

Done — the go_dependency assignment now uses the same $$(...) escaping as the tool_defs one, so the two lines read identically.

with claude fable-5

[SgtCoDFish]

Two comment nits in this block:

• $(bin_dir)/scratch/GO_TOOLCHAIN_VERSION only works because the %_VERSION pattern rule ~250 lines up sets $* to GO_TOOLCHAIN and there happens to be a GO_TOOLCHAIN_VERSION variable. That's correct and rather neat, but non-obvious enough at this distance to earn half a sentence in the block comment above.

• The two @# comment lines moved verbatim, but they're misleading and this PR is the natural place to fix them. DOWNLOAD_DIR defaults to $(HOME)/.cache/makefile-modules/downloaded (or $(CURDIR)/$(bin_dir)/downloaded under CI), neither of which matches $(bin_dir)/%, so the patsubst is a no-op and the link is absolute in practice:

  $ ls -l _bin/tools/gojq
  _bin/tools/gojq -> /Users/…/.cache/makefile-modules/downloaded/tools/gojq@v0.12.19_go1.26.7_darwin_arm64

  The example ../downloaded/tools/helm@v4.0.1_darwin_arm64 never happens.

[wallrj]

Fixed both: the block comment now notes that the GO_TOOLCHAIN_VERSION stamp comes from the generic %_VERSION pattern rule stamping the make variable of the same name, and the @# recipe comments now say the link is absolute in practice, with the patsubst only applying when DOWNLOAD_DIR is overridden to live under $(bin_dir).

with claude fable-5

[SgtCoDFish]

(This is the first time I've ever actually let claude post its own review comments - normally I paraphrase what it says... but I know your workflow will be AI heavy and I think the comments are technical enough that paraphrasing won't help, so I'll give you the lossless source)

[wallrj]

Thanks for the thorough review — both inline findings were real, and are fixed (replies inline).

On the two further points:

• Cache growth: filed tools: no eviction of superseded Go-toolchain-keyed binaries in the download cache #711 for an opt-in prune-tools-cache target and a README note on cache sizing, quoting your numbers.
• Test: added _bin/tools/gojq to the test project's targets and an assertion in test_e2e.sh covering the contract directly: overriding GO_TOOLCHAIN_VERSION rebuilds and re-points the symlink, and reverting re-points to the cached binary without rebuilding. One wrinkle: "without rebuilding" cannot be asserted with mtimes, because the relink recipe deliberately touches the symlink — which follows it to the binary — so the assertion compares inodes instead (go install produces a new file; touch keeps the old one). The assertion uses an isolated DOWNLOAD_DIR, keeping the fake toolchain key out of the shared $HOME cache.

with claude fable-5

[SgtCoDFish]

/lgtm
/approve
/hold

Looks good now I think! Added a hold because Richard doesn't work Mondays - can unhold and merge tomorrow.

[cert-manager-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SgtCoDFish

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [SgtCoDFish]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[wallrj]

/unhold