Run the linter at the pinned version locally too (closes #78) #79
Reference in New Issue
Block a user
Delete Branch "fix-pinned-linter"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #78.
script/lintnow runs the digest-pinnedgolangci-lintimage instead ofwhatever binary happens to be on
PATH, somake lintand CI cannotdisagree about findings.
What changed
script/lintextracts the image reference (tag and digest)from the
Dockerfilelint stageFROMline withawkand runs thatexact image via
docker run. Extra arguments are passed through togolangci-lint run.Dockerfileline 8 (the lint stageFROM).Docker requires a literal in
FROM, so that line is the only placethat can hold the reference; everything else derives from it. Bumping
the linter is now a one-line edit there.
Makefiledepstarget no longer doesgo install ...@v2.12.2, andscript/bootstrapno longer installs an unpinnedgolangci-lintfromthe distro package manager (issue DoD item 3). With linting
containerized, a second copy on
PATHcan only drift.bootstrapnowprints a notice if
dockeris missing.Dockerfilerunsmake lintinside thelint image, where there is no Docker daemon.
script/lintruns thePATHbinary only whengolangci-lint versionreports a versionexactly equal to the pin parsed from the
FROMline — the same linterby definition. Any other version, or none, goes through Docker.
no path from a mismatched
PATHbinary to a passing lint.script/lint-fixdelegates toscript/lint --fix, so autofixescome from the pinned linter too.
script/check,script/precommit,make lintand theDockerfilelint stage were the callers; all keepworking unchanged (
script/lintwith no arguments behaves as before).make checkis authoritativebecause
script/lintuses the pinned linter, and calls outscript/cibuildas the full CI-equivalent gate.TODO.mdupdated inthe same commit.
Not touched:
.golangci.yml(sha256021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb,verified before push) and the
gomodguarddeprecation warning.Requirement 4: proof it catches the real regression
Reproduced the exact failure from PR #77 / commit
b960ca3: the four//nolint:gosecdirectives that 2.10.1 requires and 2.12.2 reports asunused were temporarily reinstated (
internal/log/log.go:73,internal/ui/ui.go:116,internal/vaultik/verify.go:317and:329),then removed again.
Old
script/lint(the pre-change version fromorigin/main, ambient2.10.1 on
PATH) on that tree:New
make lint(pinned 2.12.2 image) on the same tree, same host, same2.10.1 still installed:
Green locally, red in CI → red locally. The temporary directives were
reverted (
git checkout --) and the tree is clean; they are not in thecommit.
Worth noting for the record: with the ambient 2.10.1, the old
script/linton unmodifiedmainreports4 issues: gosec: 4— i.e.today
make lintis also red on a tree CI lints clean. The drift cutsboth ways, and both directions are gone now.
Requirement 5: no-Docker behavior
Daemon unreachable (
DOCKER_HOST=unix:///nonexistent/docker.sock) →exit 1:
dockernot onPATHat all → exit 1 with the analogous "docker isrequired to run the pinned linter" message. Neither path falls back to
the
PATHbinary.Requirement 6: timings
Measured on this host, whole-repo lint:
PATH2.10.1)The steady-state cost is ~0.9s of container startup. The container mounts
persistent
GOCACHE,GOMODCACHEandGOLANGCI_LINT_CACHEdirectoriesunder
${XDG_CACHE_HOME:-~/.cache}/vaultik-lintand runs as the invokinguid/gid (
--user "$(id -u):$(id -g)"), so caches persist across runs andnothing in the worktree or the cache ends up root-owned.
make checkend to end: 22.8s.Verification
script/cibuild→EXIT=0. Not a cache no-op: the lint stage printed0 issues.and the test stage printed all 14okpackage lines withnone
(cached).make check→ exit 0 (tests, pinned lint0 issues., fmt-check).make fmtrun;script/lint-fixexercised (exit 0, no files changed)..golangci.ymlsha256 unchanged.`script/lint` ran whatever `golangci-lint` was on `PATH` while CI ran the digest-pinned image from the `Dockerfile` lint stage. The two versions disagree about real findings, so `make check` could be green on a tree CI fails - and, on this host's 2.10.1, red on a tree CI passes. A gate that can differ from CI is not a gate. `script/lint` now runs the pinned image itself. The single source of truth for the linter version is the `Dockerfile` lint stage `FROM` line: `script/lint` parses the image reference (tag AND digest) out of it with awk and runs exactly that image, so bumping the linter is a one-line edit there and nowhere else. The duplicate pin in the `Makefile` `deps` target (`go install ...@v2.12.2`) and the unpinned `golangci-lint` install in `script/bootstrap` are removed rather than kept in sync: with linting containerized, a second copy on `PATH` is only a way to drift. A `golangci-lint` on `PATH` is used only when its version is exactly equal to the pin - the same binary by definition, and the case that matters is the lint stage itself, which runs `make lint` inside the pinned container where no Docker daemon exists. Every other version goes through Docker, and a missing or unreachable daemon is a hard error naming the required image, never a silent fallback. The container run mounts persistent `GOCACHE`, `GOMODCACHE` and `GOLANGCI_LINT_CACHE` directories under `${XDG_CACHE_HOME:-~/.cache}` and runs as the invoking uid/gid, so repeat runs stay fast (2.7s warm vs 1.8s for the ambient binary) and nothing lands root-owned. `script/lint-fix` delegates to `script/lint --fix` so autofixes come from the same pinned linter. README documents that `make check` is authoritative because of this, and points at `script/cibuild` as the full CI-equivalent gate.Gate results for this branch (commit
1808773):script/cibuild→EXIT=0. Real run, not a cache no-op: thelint stage printed
0 issues.and the test stage printed all 14okpackage lines, none
(cached). 2m32s wall.make check→ exit 0 (14 packagesok, pinned lint0 issues.,fmt-check clean), 22.8s.
.golangci.ymlsha256 before push:021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb(unchanged, not in the diff).
One correction to the body: the single source of truth is
Dockerfileline 9 — the
FROM golangci/golangci-lint:v2.12.2-alpine@sha256:91b2... AS lintline — not line 8; the comment block above it shifted thenumbering.
script/lintlocates it structurally(
$1 == "FROM" && $3 == "AS" && $4 == "lint"), so it does not depend onthe line number.
Reviewer's shortcut for requirement 4: add
//nolint:gosecto the end ofinternal/log/log.go:73and runmake lint— it must fail with anolintlint"directive ... is unused" finding even though this host hasgolangci-lint 2.10.1 on
PATH, which does need that directive.Review of PR #79 (head
1808773)Verdict: PASS. No blocking findings. Every claim in the PR body was
re-verified independently; all of it holds. Six non-blocking findings
are listed at the end.
Gate verification (independent, on a detached worktree of
origin/fix-pinned-linter)docker build --no-cache .→ exit 0.Not a cache no-op. Lint stage
#17 [lint 8/8] RUN make lintprinted0 issues.in 47.2s; test stage#24 [builder 8/9] RUN make testprinted 14
okpackage lines, none(cached), plus the 4[no test files]packages, in 52.8s.script/cibuild→EXIT=0(captured immediately).make check→ exit 0 (14ok,0 issues., fmt-check clean).make fmt→ no files changed;git status --porcelainempty.1808773:check / check (pull_request)→ success,2m18s.
origin/main(e496aa3) is an ancestor of1808773;Gitea reports
mergeable: true. No rebase needed.Definition of done (issue #78)
make lintuses the pinned linter (option (a))script/lintruns the digest-pinned image; the run emits thegomodguarddeprecation warning that only exists from v2.12.0, proving 2.12.2 executed while 2.10.1 is on this host'sPATH.grep -rn '2\.12\.2'over the tree finds the version only inDockerfile:8(comment) andDockerfile:9(theFROM), plus prose inTODO.mdhistory andscript/lintdoc comments. No second executable pin survives inMakefile,script/bootstrap,.gitea/workflows/check.yml, orREADME.md.script/bootstrapno longer installs an unpinned linterscript/cibuildexits 0Requirement 4 reproduced (not taken on faith)
I reinstated the four historical directives from
b960ca3(
internal/log/log.go:73,internal/ui/ui.go:116,internal/vaultik/verify.go:317and:329) and ran both linters on theidentical tree:
Directives reverted with
git checkout --; tree confirmed cleanafterwards, and the review worktree was removed. The PR does what it
claims.
The escape hatch — attacked, holds
main()runs thePATHbinary when[ "$(installed_version)" = "$(pinned_version)" ]. I tried to turn thisback into the bug it fixes and could not produce a silent false green:
pinned_versionon the realFROMline →2.12.2(verifieddirectly).
installed_versionagainst this host's binary →2.10.1(verified). The awk picks the first bare
N.N.Nfield, which isthe version; the trailing
go1.26.5and thebuilt ... ontimestampcannot match ahead of it.
Dockerfilewhose lintFROMisgolangci/golangci-lint:v-alpine@sha256:..., which does drivepinned_versionto the empty string, and ran it with nogolangci-lintonPATH. Result:exec: golangci-lint: not found,exit 127 — a loud failure, not a pass. The only way to reach a
silent wrong-linter run is for the
FROMline and the versionbanner to be simultaneously unparseable, which I could not construct
from any realistic input.
FROM(golangci-lint@sha256:..., no tag) yields thenon-version string
golangci/golangci, which can never equal aninstalled version → always Docker. Fail-safe.
registry:5000/img:v2.12.2-alpine) parsescorrectly because
s/.*://is greedy and the digest is strippedfirst.
installed_versionever fails to parse, the script goes to Docker,and inside the lint image there is no daemon, so the build fails
visibly. Confirmed the in-container path is load-bearing and works:
the uncached lint stage ran
make lintnatively and printed0 issues.(golangci-lint versioninside the pinned image prints abare
2.12.2).The hatch does reduce the guarantee from digest identity to version-
string identity for a developer who happens to have exactly 2.12.2 on
PATH(e.g. anyone who ran the oldmake deps). That is preciselyoption (b) from the issue's own definition of done, applied only as
a narrow secondary path, so it is within scope. See finding 1 for the
hardening I would want.
No-Docker behavior
Both branches verified as hard errors:
DOCKER_HOST=unix:///nonexistent/docker.sock ./script/lint→exit 1, "the docker daemon is not reachable", image reference
printed.
PATHstripped ofdocker→ exit 1, "docker is required torun the pinned linter".
Neither falls back to the 2.10.1 binary sitting on this host.
script/lint-fixand container mechanicswhitespace/wsl_v5defect ininternal/globals/globals.go, ran./script/lint-fix: the file wasactually rewritten in place (1840 → 1839 bytes) and ownership
stayed
user:user, not root. Argument pass-through(
script/lint --fix "$@"→golangci-lint run --fix ./...) works.exec docker runis correct(
NEW_EXIT=2above).GOFLAGS=-buildvcs=falsemasks nothing lint-relevant; it onlysuppresses VCS stamping, which no linter consumes.
${XDG_CACHE_HOME:-~/.cache}/vaultik-lintarecreated by the invoking user before the mount; warm run measured
2.18s.
.dockerignoredoes not excludeDockerfile, solint_image()resolves inside the lint stage. Confirmed by the successful uncached
build.
Policy and hygiene
.golangci.ymlsha256 =021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb— matches, and the file is not in the diff. The
gomodguarddeprecation was correctly left alone (still emitted on every run).
(closes #78). Body wrapped at≤72 columns. No attribution trailers of any kind. No vendor
references anywhere in the commit, diff, or PR body.
set -eu, root located via$(cd "$(dirname "$0")/.." && pwd -P), perREPO_POLICIES.md.lint_image/pinned_version/installed_version/require_docker/run_in_dockerreadconsistently with the surrounding scripts.
Non-blocking findings
script/lint:119— the hatch is version-gated, notcontext-gated. The reason the native path exists is "we are inside
the lint container", but the condition tested is "the version string
matches". Those are not the same predicate, and the second one admits
a developer's locally built 2.12.2 (different Go toolchain, different
build) that the digest pin exists to exclude. Acceptable looks
like: additionally require that we are in the lint container —
[ -f /.dockerenv ], or an env var theDockerfilesets beforeRUN make lint— so the ambient path is unreachable on adeveloper workstation and version equality is only the secondary
check. This preserves today's behavior in CI and closes the last
ambient path.
script/bootstrap:61-66— bootstrap no longer produces amachine on which
make checkcan run. Docker is now a harddependency of
script/lint, therefore ofscript/checkandscript/precommit(you cannot even commit without it), yet bootstraponly warns and still exits 0 printing
bootstrap complete. Thatcontradicts the script's own header ("install all dependencies needed
to build and develop this repo") and
REPO_POLICIES.md("installsall dependencies idempotently and assumes nothing is present"). The
message is also not actionable and reads as two fragments:
docker not found; script/lint needs it to run/the pinned linter (see the Dockerfile lint stage). Acceptablelooks like: say what breaks and what to do — e.g. "docker not
found:
make lint,make checkand the pre-commit hook will failuntil Docker is installed and its daemon is running" — and
either install it via
pkg_installwhere that is meaningful, orexit nonzero so
make bootstrapdoes not claim success.script/lint:39-50— hand-rolled scraping of a human-readablebanner where a machine-readable flag exists.
golangci-lint version --shortprints exactly2.10.1on this host and exactly2.12.2inside the pinned image (both verified). The current awk depends on
the version being printed without a leading
vand as the first bareN.N.Ntoken; golangci-lint has historically printedv1.55.2inthat position. If a future bump restores the
v,installed_versionreturns empty, the in-container path is lost, and the
Dockerfilelint stage starts failing with "docker is required" — a
confusing failure for a version bump. Acceptable looks like:
golangci-lint version --shortwith the awk kept only as a fallback.TODO.md— "make checkis therefore now as trustworthy asscript/cibuild" overstates the result, and contradicts theREADME.mdtext added by the same commit, which correctly saysscript/cibuild"also catches anything that depends on host state".make checkstill runs tests andgofmton the host, not in thepinned
golang:1.26.1-alpineimage. Only the lint leg is nowequivalent. Acceptable looks like: "
make checkand CI can nolonger disagree about lint findings;
script/cibuildremains thefull gate."
README.md## requirementsstill lists only Go 1.26+ and objectstorage. Docker is now required to lint, check, or commit. The
Entrypoints section says so under
script/lint, which satisfies DoDitem 5, but a developer reading the requirements list will not learn
it. Acceptable looks like: add "Docker (for
make lint/make check; the linter runs from a digest-pinned image)".PR body says "single source of truth:
Dockerfileline 8"; theFROMis on line 9 (line 8 is the dated comment). Alreadycorrected in a comment, and
script/lintlocates the linestructurally rather than by number, so nothing functional depends on
it — the body text is simply still wrong.
None of the six blocks the merge. Findings 1 and 2 are worth a
follow-up issue.