diff --git a/Dockerfile b/Dockerfile index 554c32d..62efdec 100644 --- a/Dockerfile +++ b/Dockerfile @@ -32,10 +32,12 @@ FROM golang@sha256:56961d79ea8129efddcc0b8643fd8a5416b4e6228cfd477e3fd61deb2672c # We never build or run as root. Create an unprivileged user and point # HOME and the Go caches at its home so go build/test and golangci-lint # can write their caches when we drop to it below. $GOPATH/bin is on -# PATH because that is where script/bootstrap's `go install` lands: if -# the linter copied in below ever stops matching bootstrap's pin, -# bootstrap reinstalls it and then verifies the pin against what PATH -# resolves, which can only succeed if that directory is searched. +# PATH because that is where script/bootstrap's `go install` lands: a +# tool bootstrap installs must be runnable afterwards, and bootstrap +# verifies its own installs against what PATH resolves, so leaving that +# directory unsearched would make any install it performs both unusable +# and self-reported as shadowed. Nothing in this image is shadowed by +# it: the directory does not exist until bootstrap runs. RUN adduser -D -u 1000 builder ENV HOME=/home/builder ENV GOPATH=/home/builder/go @@ -52,22 +54,40 @@ WORKDIR /src # so it is what forces BuildKit to finish fmt-check and lint before # compilation and tests start. Remove it and the fail-fast design # dies silently: the build stops gating on lint and still exits 0. -# - It is what keeps the two stages on one toolchain. script/bootstrap -# version-checks whatever PATH resolves against its pin, so copying -# the lint stage's binary in first means every build now compares -# the lint stage's linter to that pin and fails loudly if they ever -# drift apart. Bootstrap installing its own linter here instead -# would restore exactly the two-independent-toolchains problem the -# copy prevents (and cost a from-source build of the linter). +# - Together with the check below it is what keeps the two stages on +# one toolchain: `make check` here runs the very binary the lint +# stage ran, not a second one that happens to agree. Bootstrap +# installing its own linter here instead would restore exactly the +# two-independent-toolchains problem the copy prevents (and cost a +# from-source build of the linter). COPY --from=lint /usr/bin/golangci-lint /usr/local/bin/golangci-lint +# Fail the build, naming both versions, unless the binary that just +# arrived from the lint stage is the version script/bootstrap pins. +# +# Nothing else enforces that. The linter version is pinned in two +# independent places — the lint stage's image digest above and +# GOLANGCI_LINT_VERSION in script/bootstrap — and bumping one alone is +# an easy mistake. Without this check that mistake is invisible: +# bootstrap below would see a version that is not its pin, quietly +# rebuild the pinned one from source into a directory that is on PATH, +# verify that, and exit 0. The build would go green with the lint stage +# having linted at one version and `make check` at another, which is +# precisely the divergence the copy above exists to prevent. +# +# It runs here, before bootstrap, so that a reinstall cannot satisfy it, +# and it needs no CHECK_EPOCH: its only inputs are the copied binary and +# script/, so Docker invalidates this layer exactly when a cached result +# would stop being true. +COPY script/ script/ +RUN script/verify-linter-pin /usr/local/bin/golangci-lint + # Install development prerequisites the same way a developer does, -# rather than duplicating the installs inline. script/ and the -# dependency manifests are copied first, and nothing else is, so this -# layer stays cached until the scripts or the dependencies change — +# rather than duplicating the installs inline. Only script/ (copied +# above) and the dependency manifests are copied first, nothing else, so +# this layer stays cached until the scripts or the dependencies change — # bootstrap ends in `go mod download`, which is why there is no separate # invocation of it here. -COPY script/ script/ COPY go.mod go.sum ./ RUN script/bootstrap diff --git a/README.md b/README.md index 9f17e30..6975dff 100644 --- a/README.md +++ b/README.md @@ -493,6 +493,16 @@ and may be invoked directly. The provided entrypoints are: - `script/install-precommit` — install the git pre-commit hook that runs `script/precommit`. The hook is written to the common git directory, so the main checkout and every worktree share it. +- `script/verify-linter-pin` — fail unless a `golangci-lint` binary + (given as its argument, default whatever `PATH` resolves) is + exactly the version `script/bootstrap` pins, naming both versions + if not. The `Dockerfile` build stage runs it on the linter it + copies out of the lint stage: the version is pinned independently + in the lint stage's image digest and in `script/bootstrap`, and + bumping one alone would otherwise be absorbed silently by + bootstrap rebuilding its pin from source, leaving the two stages + on different linters under a green build. The pin is read from + `script/bootstrap`, which stays its single source of truth. `script/docker` and `script/cibuild` both pass a freshly computed `CHECK_EPOCH` build argument, and the `Dockerfile`'s gate steps diff --git a/TODO.md b/TODO.md index 497a71f..5b8a809 100644 --- a/TODO.md +++ b/TODO.md @@ -40,18 +40,36 @@ is gone. `COPY --from=lint /usr/bin/golangci-lint` stays, and moves above the bootstrap layer. It is the only edge making this stage depend on the lint stage, so deleting it as redundant would end - fail-fast linting silently; putting it first also means bootstrap's - version check now compares the lint stage's linter against the pin - on every build, which is what makes the two stages provably one - toolchain instead of two that happen to agree. Letting bootstrap - install its own linter here would have reintroduced the second - toolchain and paid for a from-source build of it. `$GOPATH/bin` - joins `PATH` so that if the copied binary ever stops matching the - pin, bootstrap's reinstall lands somewhere `PATH` resolves rather - than failing its own verification. Everything added sits above - `ARG CHECK_EPOCH`, and the `chown` and `USER builder` still precede - `make check`. Verified: bootstrap runs clean under Alpine's `sh` and - its `apk` branch, installing `git` and `make` and finding the copied + fail-fast linting silently. Letting bootstrap install its own linter + here would have reintroduced the second toolchain and paid for a + from-source build of it. What makes the two stages provably one + toolchain rather than two that happen to agree is a new + `script/verify-linter-pin`, run in the build stage on the binary + that arrives from the lint stage, before bootstrap: it fails the + build naming both versions unless that binary is the version + `script/bootstrap` pins. Bootstrap's own check could not serve that + purpose — it reinstalls its pin from source and then verifies + whatever `PATH` resolves, so drift self-heals silently and a lint + stage image bumped on its own would lint at the new version while + `make check` ran at the old one, green. The linter version is pinned + in two independent places (the lint stage image digest and + `GOLANGCI_LINT_VERSION`) and nothing else keeps them in sync, so a + half-applied bump is now a build failure. The pin is read out of + `script/bootstrap`, which stays the single source of truth; a pin + that cannot be read is a hard failure, not a skip. The check needs + no `CHECK_EPOCH`: its only inputs are the copied binary and + `script/`, so Docker invalidates the layer exactly when a cached + result would stop being true, and it is documented with the other + entrypoints in the README. `$GOPATH/bin` joins `PATH` because + that is where bootstrap's `go install` lands and bootstrap verifies + its installs against what `PATH` resolves — nothing in the image is + shadowed by it, the directory does not exist until bootstrap runs. + Everything added sits above `ARG CHECK_EPOCH`, and the `chown` and + `USER builder` still precede `make check`. Verified: the guard fails + the build with both versions named when the lint stage's linter is + faked to a different version, and an unmodified build still passes + it; bootstrap runs clean under Alpine's `sh` and its `apk` branch, + installing `git` and `make` and finding the copied linter already at the pin; a second build served the bootstrap and dependency layers `CACHED` while both gates ran with a fresh epoch; a planted `unused` finding failed the build at the lint gate in diff --git a/script/verify-linter-pin b/script/verify-linter-pin new file mode 100755 index 0000000..774b92b --- /dev/null +++ b/script/verify-linter-pin @@ -0,0 +1,102 @@ +#!/bin/sh +# script/verify-linter-pin: fail unless a golangci-lint binary is exactly +# the version script/bootstrap pins. Takes the binary to check as its +# argument, defaulting to whatever PATH resolves. Our own extension to +# scripts-to-rule-them-all, not one of its entrypoints. +# +# The Dockerfile build stage runs this on the linter it copies out of the +# lint stage, before anything else runs there. Without it, drift between +# the two stages is silently absorbed: script/bootstrap reinstalls its +# pinned version from source, verifies that, and the build goes green +# with the lint stage having linted at one version and `make check` +# having run at another. Bumping the lint stage image alone is enough to +# produce that, and this is the check that turns it into a build failure +# naming both versions. +# +# The pin is read out of script/bootstrap rather than restated here. +# script/bootstrap is the single source of truth for the linter version, +# and a second hardcoded copy of it is exactly the drift this script +# exists to catch. A pin that cannot be read is therefore a hard failure +# and not a skip: silently comparing against an empty string would turn +# this check into the kind of unearned green it was written to stop. +set -eu + +ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" + +# Seconds to allow `golangci-lint --version` to run, so a wedged binary +# stops the build instead of hanging it. Bounded by timeout(1) where that +# exists; stock macOS has none, and there the call runs unbounded. +VERSION_TIMEOUT="30" + +version_output() { + if command -v timeout >/dev/null 2>&1; then + timeout "$VERSION_TIMEOUT" "$1" --version + else + "$1" --version + fi +} + +main() { + # Resolve the argument before changing directory, so a relative path + # means what the caller meant by it. + bin="${1:-golangci-lint}" + resolved="$(command -v "$bin" 2>/dev/null || true)" + + cd "$ROOT" + + pin="$( + sed -n 's/^GOLANGCI_LINT_VERSION="\([^"]*\)".*/\1/p' script/bootstrap + )" + if [ -z "$pin" ]; then + echo "verify-linter-pin: no GOLANGCI_LINT_VERSION assignment found" \ + "in script/bootstrap; that file is the single source of truth" \ + "for the linter version and this check cannot run without it" >&2 + exit 1 + fi + + if [ -z "$resolved" ]; then + echo "verify-linter-pin: $bin: not found (pin is $pin)" >&2 + exit 1 + fi + + # Same output shape script/bootstrap parses: + # golangci-lint has version X.Y.Z built with go1.26.5 from abc1234 + # so the version is the field after the literal word "version", with + # any leading "v" stripped. stderr is left connected so a binary that + # cannot execute (wrong architecture, missing shared library) says why + # rather than being reported as merely unparseable. + if ! out="$(version_output "$resolved")"; then + echo "verify-linter-pin: $resolved --version failed; the binary" \ + "cannot be executed or timed out (pin is $pin)" >&2 + exit 1 + fi + found="$( + echo "$out" | awk ' + { + for (i = 1; i < NF; i++) { + if ($i == "version") { + v = $(i + 1) + sub(/^v/, "", v) + print v + exit + } + } + } + ' + )" + + if [ "$found" != "$pin" ]; then + echo "verify-linter-pin: $resolved reports" \ + "${found:-no parseable version}, but script/bootstrap pins" \ + "$pin" >&2 + echo "verify-linter-pin: these must be the same version — bump the" \ + "Dockerfile lint stage image and GOLANGCI_LINT_VERSION in" \ + "script/bootstrap together" >&2 + exit 1 + fi + + echo "verify-linter-pin: $resolved is $found, matching the" \ + "script/bootstrap pin" +} + +main "$@"