next: accumulated work for the upcoming release #114

Open
clawbot wants to merge 1 commits from next into main
13 changed files with 921 additions and 541 deletions

View File

@@ -1,23 +1,29 @@
# Lint stage # This file has no lint stage, deliberately.
# #
# This FROM line is the single source of truth for the linter version: # Linting lives in Dockerfile.lint, built by script/lint, and
# script/lint parses the image reference out of it and runs that exact # script/cibuild builds both. A lint stage here would have to either
# image, so a local `make lint` and CI use the same linter. Bump the # shell out to `make lint` -- which is now `docker build`, so
# linter here (tag AND digest) and nowhere else. # docker-in-docker inside a BuildKit step with no daemon -- or call
# golangci-lint directly, which would mean a second, independently
# bumpable digest pin for the linter alongside the one in
# Dockerfile.lint. Two pins for one tool is the drift that
# https://git.eeqj.de/sneak/vaultik/issues/78 was filed over. See
# https://git.eeqj.de/sneak/vaultik/issues/113 for the ruling.
# #
# golangci/golangci-lint:v2.12.2-alpine, 2026-08-07 # Consequence, stated rather than left to be discovered: script/docker
FROM golangci/golangci-lint:v2.12.2-alpine@sha256:91b27804074a0bacea298707f016911e60cf0cdbc6c7bf5ccacb5f0606d18d60 AS lint # builds this file only and therefore does not lint. `make fmt-check`
# and `make test` still run here, so what a green build of this file
# means is "formatted, tested, and it compiles" -- the lint verdict
# comes from script/lint or script/cibuild.
RUN apk add --no-cache make build-base # Build stage
# golang:1.26.1-alpine, 2026-03-17
FROM golang:1.26.1-alpine@sha256:2389ebfa5b7f43eeafbd6be0c3700cc46690ef842ad962f6c5bd6be49ed82039 AS builder
# The context signal for script/lint's native path. This stage runs ARG VERSION=dev
# `make lint` with no docker daemon available, so it is the one place
# that must run the golangci-lint on PATH directly. script/lint takes # Install build dependencies for CGO (mattn/go-sqlite3) and sqlite3 CLI (tests)
# that path only when this is set AND the version matches the pin above; RUN apk add --no-cache make build-base sqlite
# version equality alone would also admit a developer's locally
# installed copy on a host, bypassing the digest pin (issue #80).
# Nothing outside this stage sets it.
ENV VAULTIK_LINT_IN_CONTAINER=1
WORKDIR /src WORKDIR /src
@@ -28,7 +34,7 @@ RUN go mod download
# Copy source code # Copy source code
COPY . . COPY . .
# Run formatting check and linter. # Run the format check and the tests.
# #
# CHECK_EPOCH must stay immediately above these RUNs. These layers are # CHECK_EPOCH must stay immediately above these RUNs. These layers are
# keyed on its value, so they are cache-eligible only for a value # keyed on its value, so they are cache-eligible only for a value
@@ -47,45 +53,15 @@ COPY . .
# runs the checks and every one after it on an unchanged tree replays # runs the checks and every one after it on an unchanged tree replays
# these layers from cache, executes nothing, and still exits 0. Failed # these layers from cache, executes nothing, and still exits 0. Failed
# steps are never cached, so the guard fails on EVERY invocation rather # steps are never cached, so the guard fails on EVERY invocation rather
# than once -- a bare `docker build .` is now a loud error, not a quiet # than once -- a bare `docker build .` is a loud error, not a quiet
# green. Do not give CHECK_EPOCH a default value; a default would # green. Do not give CHECK_EPOCH a default value; a default would
# satisfy the guard with a constant and restore the hole. # satisfy the guard with a constant and restore the hole.
# #
# ARG scope is per-stage, so the builder stage declares its own.
# Everything above this line (apk, go.mod, `go mod download`) is # Everything above this line (apk, go.mod, `go mod download`) is
# deliberately outside the busted range and keeps caching. # deliberately outside the busted range and keeps caching.
ARG CHECK_EPOCH ARG CHECK_EPOCH
RUN [ -n "$CHECK_EPOCH" ] || exit 1 RUN [ -n "$CHECK_EPOCH" ] || exit 1
RUN echo "check epoch: ${CHECK_EPOCH}" && make fmt-check RUN echo "check epoch: ${CHECK_EPOCH}" && make fmt-check
RUN echo "check epoch: ${CHECK_EPOCH}" && make lint
# Build stage
# golang:1.26.1-alpine, 2026-03-17
FROM golang:1.26.1-alpine@sha256:2389ebfa5b7f43eeafbd6be0c3700cc46690ef842ad962f6c5bd6be49ed82039 AS builder
# Depend on lint stage passing
COPY --from=lint /src/go.sum /dev/null
ARG VERSION=dev
# Install build dependencies for CGO (mattn/go-sqlite3) and sqlite3 CLI (tests)
RUN apk add --no-cache make build-base sqlite
WORKDIR /src
# Copy go mod files first for better layer caching
COPY go.mod go.sum ./
RUN go mod download
# Copy source code
COPY . .
# Run tests. See the CHECK_EPOCH comment in the lint stage for the
# mechanism; ARG scope is per-stage, so this stage needs its own
# declaration, its own guard, and its own expansion, and they must stay
# immediately above the check RUN.
ARG CHECK_EPOCH
RUN [ -n "$CHECK_EPOCH" ] || exit 1
RUN echo "check epoch: ${CHECK_EPOCH}" && make test RUN echo "check epoch: ${CHECK_EPOCH}" && make test
# Build (pure Go, no CGO required since we use modernc.org/sqlite) # Build (pure Go, no CGO required since we use modernc.org/sqlite)

104
Dockerfile.lint Normal file
View File

@@ -0,0 +1,104 @@
# Lint image.
#
# Every lint run in this repo happens inside this image, invoked through
# script/lint, and linting is a BUILD STEP rather than a container
# command: a successful build of this file IS a clean lint. That shape
# also works where the docker daemon is remote and bind mounts are
# impossible, which `docker run` against a mounted worktree does not.
#
# This FROM line is the single source of truth for the linter version in
# this repo. Nothing else pins golangci-lint: the product Dockerfile has
# no lint stage, deliberately, so there is no second digest to bump and
# no pair of pins that can drift apart. Bump the tag AND the digest here
# and nowhere else.
#
# Note for readers coming from REPO_POLICIES.md: that document still
# describes the older pattern, a lint stage inside the product
# Dockerfile wired up with `COPY --from=lint /src/go.sum /dev/null`.
# That pattern is superseded here by the owner's ruling recorded in
# https://git.eeqj.de/sneak/vaultik/issues/113 -- lint runs in its own
# image, per run, with its own cache and its own lock, which is what
# makes concurrent runs on one host safe. The policy text is org-wide
# and is being amended separately; this file is what this repo does.
#
# golangci/golangci-lint:v2.12.2, 2026-08-10
FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240
WORKDIR /src
# Copy the dependency manifests first so the module download layer stays
# cached until they change. Everything above the ARG below is cacheable
# on purpose; a cold module download on every lint would make the inner
# loop unusable and buys nothing, because it is not what the gate is
# asserting.
COPY go.mod go.sum ./
RUN go mod download
COPY . .
# Force the check layers to execute on every invocation.
#
# CHECK_EPOCH must stay immediately above the RUNs below. Those layers
# are keyed on its value, so they are cache-eligible only for a value
# already built against this same tree; script/lint and script/cibuild
# each pass a fresh value on every invocation, which is what makes their
# green mean the linter really ran. Without it, `docker build -f
# Dockerfile.lint .` on an unchanged tree exits 0 in well under a second
# having linted nothing.
#
# The value is expanded into each check command itself rather than left
# to a bare declaration, so the cache miss does not depend on BuildKit's
# unreferenced-ARG handling staying as it is. It also puts the epoch in
# the build log, where a reader can see the layer was keyed fresh.
#
# The guard is what makes a build that omits --build-arg fail instead of
# lie. An unset ARG is an empty string, and an empty string is a
# perfectly stable cache key: without the guard the first such build
# lints and every one after it on an unchanged tree replays this layer,
# executes nothing, and still exits 0. Failed steps are never cached, so
# the guard fails on EVERY invocation rather than once. Do not give
# CHECK_EPOCH a default value; a default would satisfy the guard with a
# constant and restore the hole.
ARG CHECK_EPOCH
RUN [ -n "$CHECK_EPOCH" ] || exit 1
# Validate .golangci.yml before linting with it.
#
# This is not belt-and-braces; it closes a hole that `golangci-lint run`
# leaves wide open. `run` rejects YAML it cannot PARSE, but it silently
# IGNORES an unknown top-level KEY. Renaming `linters:` to `linterz:` --
# one character -- discards `default: all`, the whole disable list and
# every threshold, leaves only golangci-lint's small default linter set
# running, and exits 0 reporting `0 issues.` on a tree the real config
# fails. Demonstrated on this repo at this pin, recorded on
# https://git.eeqj.de/sneak/vaultik/pulls/114: with a planted
# over-length line, `script/lint` exits 1 naming the `lll` finding with
# `linters:` and exits 0 with `linterz:`. A set-but-ineffective config
# quietly falling back to defaults is precisely the false-green class
# this gate exists to eliminate, so it must not sit in the gate's own
# configuration.
#
# `config verify` catches it, and it does so OFFLINE at this pinned
# version -- verified, not assumed. Under `docker run --network none`
# against the pinned digest it exits 0 on this repo's config and exits 3
# on the `linterz:` variant with `additional properties 'linterz' not
# allowed`. An earlier revision of this file asserted the opposite, that
# the schema is fetched over live HTTPS from an unpinned URL, and used
# that to justify omitting this line. That claim was false at v2.12.2;
# the schema is embedded. If a future bump reintroduces a network fetch
# the failure is loud and this comment is where to record it.
#
# It is keyed on CHECK_EPOCH, like the lint run below, so it executes on
# every invocation. Content-addressing alone would arguably be enough --
# .golangci.yml arrives through `COPY . .`, so a cache hit here implies
# a byte-identical config was validated when the layer really ran. That
# argument is exactly the one that would also excuse caching the lint
# layer, and this repo has ruled it insufficient: a cached check layer
# checks nothing, and the cost of being wrong is silent. Forcing it costs
# milliseconds and puts the epoch in the log, where a reader can see that
# this validation ran rather than being replayed.
RUN echo "check epoch: ${CHECK_EPOCH}" && \
golangci-lint config verify --config .golangci.yml
RUN echo "check epoch: ${CHECK_EPOCH}" && \
golangci-lint run --config .golangci.yml ./...

View File

@@ -87,10 +87,10 @@ clean:
go clean go clean
# Install dependencies. The linter is deliberately not installed here: # Install dependencies. The linter is deliberately not installed here:
# script/lint runs the digest-pinned golangci-lint image declared by the # script/lint lints by building Dockerfile.lint, whose FROM line is the
# Dockerfile's lint stage, which is the single source of truth for the # single source of truth for the linter version. A second, separately
# linter version. A second, separately pinned copy on PATH could drift # pinned copy on PATH could drift from it and make a local `make lint`
# from it and make a local `make lint` disagree with CI. # disagree with CI.
deps: deps:
go mod download go mod download

View File

@@ -598,10 +598,11 @@ regardless of color setting (emoji are not color).
* Go 1.26 or later * Go 1.26 or later
* Docker, with a reachable daemon, to lint, check, or commit: * Docker, with a reachable daemon, to lint, check, or commit:
`script/lint` runs the digest-pinned `golangci-lint` image declared by `script/lint` lints by building `Dockerfile.lint`, which runs the
the `Dockerfile` lint stage, and `make check` and the pre-commit hook digest-pinned `golangci-lint` image as a build step, and `make check`
both run it. A `golangci-lint` installed on `PATH` is not a substitute and the pre-commit hook both run it. A `golangci-lint` installed on
and is never used on a host, whatever its version. `PATH` is not a substitute and is never used on a host, whatever its
version.
* `sqlite3` CLI, which the test suite shells out to * `sqlite3` CLI, which the test suite shells out to
* S3-compatible object storage (or local filesystem, or rclone remote) * S3-compatible object storage (or local filesystem, or rclone remote)
@@ -671,46 +672,69 @@ them. We provide:
diverges from the 30s `REPO_POLICIES.md` mandates; the reasoning is in diverges from the 30s `REPO_POLICIES.md` mandates; the reasoning is in
the comment in the script, and issue #101 proposes amending the policy the comment in the script, and issue #101 proposes amending the policy
text. text.
* `script/lint`run `golangci-lint run ./...` at the exact version CI * `script/lint`lint by building `Dockerfile.lint`, which runs
uses, by running the digest-pinned `golangci-lint` image declared by `golangci-lint run --config .golangci.yml ./...` as a build step
the `Dockerfile` lint stage (requires Docker; it fails loudly rather inside the digest-pinned `golangci-lint` image, so a successful build
than falling back to a differently versioned `golangci-lint` on *is* a clean lint. Nothing lints on the host, at any version, ever;
`PATH`). That `FROM` line is the single source of truth for the linter the script requires Docker and fails loudly rather than falling back
version — bump it there and nowhere else. to a `golangci-lint` on `PATH`. That `FROM` line is the single source
of truth for the linter version — bump it there and nowhere else.
It takes no arguments, because a build step has no command line to
pass flags to, and it passes a fresh `--build-arg CHECK_EPOCH` on
every invocation so the lint layer cannot be replayed from cache (see
`script/cibuild` below for what that mechanism defends against). To
watch the linter execute, run it as
`BUILDKIT_PROGRESS=plain script/lint` and check that the lint layer
says `RUN … golangci-lint` rather than `CACHED`.
One container per run means one lint cache and one `golangci-lint`
lock per run, both private to it and discarded with it, so concurrent
runs on one host cannot contaminate or block each other.
* `script/lint-fix` — apply the linter's autofixes (rewrites files), * `script/lint-fix` — apply the linter's autofixes (rewrites files),
using the same pinned linter using the same pinned image, parsed out of `Dockerfile.lint`. It
cannot be a build step, because fixes have to land in the worktree, so
it bind-mounts the tree into a `docker run` and therefore needs a
*local* daemon. It is a developer convenience and never a gate: no
gate reads its exit status. Run `make lint` afterwards to find out
whether the tree is clean.
* `script/fmt` — format all code (writes) * `script/fmt` — format all code (writes)
* `script/fmt-check` — check formatting (read-only) * `script/fmt-check` — check formatting (read-only)
* `script/check` — run `script/test`, `script/lint`, and * `script/check` — run `script/test`, `script/lint`, and
`script/fmt-check`. This is authoritative *because* `script/lint` uses `script/fmt-check`. This is authoritative *because* `script/lint`
the pinned linter: a local `make check` and CI cannot disagree about builds `Dockerfile.lint`: a local `make check` and CI cannot disagree
lint findings. about lint findings.
* `script/docker` — build the Docker image tagged via * `script/docker` — build the Docker image tagged via
`script/projectname`. Passes a fresh `--build-arg CHECK_EPOCH` for the `script/projectname`. Passes a fresh `--build-arg CHECK_EPOCH` for the
same reason `script/cibuild` does, so a local image build cannot be same reason `script/cibuild` does, so a local image build cannot be
green on checks it replayed from cache. green on checks it replayed from cache. It builds the *product* image
* `script/cibuild` — CI entrypoint: `docker build` (the `Dockerfile` only, and the product `Dockerfile` has no lint stage, so it does not
runs `make fmt-check` and `make lint` in its lint stage and `make lint: a green here means formatted, tested, and it compiles.
test` in its builder stage). This is the full CI-equivalent gate — it * `script/cibuild` — CI entrypoint, and the full gate. Two builds, in
runs the checks in the same containers CI does, from a clean copy of order: `Dockerfile.lint` (the linter, as a build step) and then
the tree, so it also catches anything that depends on host state. It `Dockerfile` (`make fmt-check` and `make test` in its builder stage,
passes a fresh `--build-arg CHECK_EPOCH`, unique per invocation, which then the product image). Either failing fails the script. It runs the
the `Dockerfile` declares immediately above the check `RUN`s in both checks in the same containers CI does, from a clean copy of the tree,
stages and expands into each check command. Those layers are keyed on so it also catches anything that depends on host state.
It passes a fresh `--build-arg CHECK_EPOCH` to each build, unique per
invocation, which both files declare immediately above their check
`RUN`s and expand into each check command. Those layers are keyed on
that value, so a new value re-runs them even on a byte-identical tree, that value, so a new value re-runs them even on a byte-identical tree,
and a green from this script means the checks executed. Dependency and and a green from this script means the checks executed. Dependency and
module layers sit above the `ARG` and still cache, so a build is not module layers sit above the `ARG` and still cache, so a build is not
cold. cold.
A build that supplies no `CHECK_EPOCH` — a bare `docker build .` A build that supplies no `CHECK_EPOCH` — a bare `docker build .` or
fails rather than lying. An unset `ARG` is an empty string and an `docker build -f Dockerfile.lint .` — fails rather than lying. An
empty string is a stable cache key, so without a guard such a build unset `ARG` is an empty string and an empty string is a stable cache
would serve all three check layers from cache, execute nothing, and key, so without a guard such a build would serve every check layer
still exit 0. Each check stage therefore asserts the value is from cache, execute nothing, and still exit 0. Each file therefore
non-empty before running anything, and because failed steps are never asserts the value is non-empty before running anything, and because
cached that assertion fires on every invocation rather than once. Use failed steps are never cached that assertion fires on every
`script/cibuild` (or `script/docker`, which passes the same arg); a invocation rather than once. Use `script/lint`, `script/docker` or
bare `docker build .` is now a loud error. `script/cibuild`, which pass the arg; a bare `docker build` is a loud
error.
* `script/precommit` — pre-commit gate: `go mod tidy` + `go fmt` (must * `script/precommit` — pre-commit gate: `go mod tidy` + `go fmt` (must
not change files), then `script/check` not change files), then `script/check`
* `script/install-precommit` — install the git pre-commit hook that * `script/install-precommit` — install the git pre-commit hook that

60
TODO.md
View File

@@ -25,6 +25,66 @@ release" is exactly the contradiction
# Completed Steps # Completed Steps
- 2026-08-10: Moved every lint run into its own container, as a build
step ([issue #113](https://git.eeqj.de/sneak/vaultik/issues/113)).
New root `Dockerfile.lint`, built by `script/lint`, runs
`golangci-lint run --config .golangci.yml ./...` as a `RUN`
instruction in the digest-pinned `golangci/golangci-lint` image: a
successful build of that file *is* a clean lint, and it works even
where the daemon is remote and bind mounts are impossible. That
`FROM` line is now the only pin of the linter version in the repo.
This supersedes the per-worktree cache isolation landed for
[issue #99](https://git.eeqj.de/sneak/vaultik/issues/99). Isolation
fixed cross-worktree contamination but not lock contention — two
concurrent runs with entirely separate cache directories still
collided. A container per run has its own cache and its own lock, so
the whole class is gone, and with it the per-worktree cache
machinery, the lock-retry loop, and `script/lint-audit`, which
existed to catch replayed findings from a cache that no longer
exists. The host lint path went too: no escape hatch, no
`VAULTIK_LINT_IN_CONTAINER`, no version detection. Nothing lints on
the host at any version.
A cached build lints nothing, so the same `CHECK_EPOCH` mechanism the
product `Dockerfile` already used is what makes the green mean
something: `ARG CHECK_EPOCH` with no default below the module layers,
a `RUN [ -n "$CHECK_EPOCH" ] || exit 1` guard, the value expanded
into each check command, and a fresh `$(date +%s%N)$$` per invocation
computed as a bare assignment. `cmd/vaultik/lintdocker_test.go`
parses both Dockerfiles and both scripts and fails if any part of
that is dropped, because every way of losing it is silent. Its
host-lint assertion is structural — no script runs `golangci-lint`
except through `docker` — rather than a search for the one retired
variable name, which nothing could ever reintroduce.
The product `Dockerfile` lost its lint stage rather than gaining a
second linter pin: `make lint` is now `docker build`, so the stage
would have been docker-in-docker with no daemon, and calling
`golangci-lint` directly there would have restored the two-pins drift
of [issue #78](https://git.eeqj.de/sneak/vaultik/issues/78).
`make fmt-check` moved beside `make test` in the builder stage, and
`script/cibuild` now builds `Dockerfile.lint` and then `Dockerfile`,
each with its own fresh epoch. Consequence, stated rather than left
to be found: `script/docker` builds the product image only and no
longer lints; the gates are `script/check` and `script/cibuild`.
`golangci-lint config verify` runs as its own epoch-keyed layer,
above the lint. `golangci-lint run` rejects a config it cannot parse
but silently ignores an unknown top-level *key*: renaming `linters:`
to `linterz:` discarded `default: all` and every threshold and still
exited 0 on a tree the real config fails. `config verify` catches
that, and it does so with the network off at this pin — checked under
`docker run --network none`, not assumed. An earlier revision omitted
it on the claim that it fetches its schema over live HTTPS; that
claim was false at v2.12.2.
`script/lint-fix` is kept, reimplemented as a
bind-mounted `docker run` against the image parsed out of
`Dockerfile.lint` — it cannot be a build step, because fixes have to
land in the worktree — and marked in its header as a developer
convenience that no gate reads.
- 2026-08-09: Finished the `--json` stdout contract and gave `make build` - 2026-08-09: Finished the `--json` stdout contract and gave `make build`
a rule ([issue #108](https://git.eeqj.de/sneak/vaultik/issues/108), a rule ([issue #108](https://git.eeqj.de/sneak/vaultik/issues/108),
[issue #110](https://git.eeqj.de/sneak/vaultik/issues/110)). Two [issue #110](https://git.eeqj.de/sneak/vaultik/issues/110)). Two

View File

@@ -0,0 +1,507 @@
package main_test
import (
"os"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// This file guards the shape of the lint gate. Every property asserted
// here is one whose loss is SILENT: the build still exits 0, the gate
// still looks green, and nothing was linted or tested.
//
// The gate is a build step. script/lint builds Dockerfile.lint, which
// runs golangci-lint as a RUN instruction, so a successful build is a
// clean lint. BuildKit will happily replay that RUN from cache on an
// unchanged tree in well under a second, which is why the check layers
// are keyed on a CHECK_EPOCH build arg that the calling script
// regenerates per invocation, and why an empty value is a hard error
// rather than a stable cache key.
//
// These are parses rather than invocations. Shelling out to docker from
// the test suite would nest a build inside `make test`, which itself
// runs inside a build in CI. The one property a parse cannot establish
// -- that a real finding actually fails the build -- is verified by
// hand against a deliberately broken tree, recorded on the pull
// request.
// The files under guard, relative to the repository root.
const (
lintDockerfile = "Dockerfile.lint"
productDockerfile = "Dockerfile"
lintScript = "script/lint"
cibuildScript = "script/cibuild"
)
// linterBinary is the linter's command name. Every occurrence of it in
// executable shell in this repo must be inside a docker invocation; see
// TestNoHostLintPathRemains.
const linterBinary = "golangci-lint"
// checkEpochARG is the declaration, with no default value. A default
// would satisfy the non-empty guard with a constant, and a constant is
// a stable cache key: the checks would be replayed from cache forever
// after the first build.
const checkEpochARG = "ARG CHECK_EPOCH"
// checkEpochGuard is what turns a build that omits --build-arg into a
// loud failure instead of a quiet green. Failed steps are never cached,
// so it fires on every such invocation rather than once.
const checkEpochGuard = `RUN [ -n "$CHECK_EPOCH" ] || exit 1`
// freshEpoch is the epoch computation the calling scripts must use, as
// a bare assignment on its own line. Inline in an argument, a failing
// `date` would not abort under `set -eu`; CHECK_EPOCH would become the
// empty string, and the guard above would be the only thing standing
// between that and a permanently cached green. `$$` is required because
// `date +%s` is second-granular and busybox silently drops `%N`, so
// without the pid two concurrent runs in one second can collide.
const freshEpoch = `epoch="$(date +%s%N)$$"`
// TestLintDockerfilePinsTheLinterByDigest fails if the lint image stops
// being pinned. An unpinned tag makes the gate's verdict depend on
// whatever the registry currently serves under that name.
func TestLintDockerfilePinsTheLinterByDigest(t *testing.T) {
t.Parallel()
from := ""
for _, instruction := range instructions(t, lintDockerfile) {
if strings.HasPrefix(instruction, "FROM ") {
from = instruction
break
}
}
require.NotEmpty(t, from, "%s declares no FROM", lintDockerfile)
assert.Contains(t, from, "golangci/golangci-lint",
"the lint image must be the golangci-lint image")
assert.Contains(t, from, "@sha256:",
"the lint image must be pinned by digest, not by tag alone")
}
// TestLintDockerfileCannotBeCachedGreen pins the whole cache-busting
// mechanism in the file that lints: the declaration with no default,
// the non-empty guard, and the value expanded into the lint command
// itself rather than merely declared.
func TestLintDockerfileCannotBeCachedGreen(t *testing.T) {
t.Parallel()
found := instructions(t, lintDockerfile)
argAt := indexOf(found, checkEpochARG)
require.GreaterOrEqual(t, argAt, 0,
"%s must declare `%s` with no default value",
lintDockerfile, checkEpochARG)
assert.GreaterOrEqual(t, indexOf(found, checkEpochGuard), argAt,
"%s must guard against an empty CHECK_EPOCH with `%s`",
lintDockerfile, checkEpochGuard)
assertEpochExpandedInto(t, found[argAt:], "golangci-lint run")
// Dependency layers must stay above the ARG, or every lint run
// re-downloads the module cache and the inner loop becomes
// unusable.
download := indexOf(found, "RUN go mod download")
require.GreaterOrEqual(t, download, 0,
"%s must download modules in their own layer", lintDockerfile)
assert.Less(t, download, argAt,
"`%s` must come after `go mod download` so dependency layers"+
" still cache", checkEpochARG)
}
// TestLintDockerfileVerifiesTheLinterConfig guards the validation of
// .golangci.yml itself. `golangci-lint run` rejects a config it cannot
// parse but silently IGNORES an unknown top-level key, so renaming
// `linters:` to `linterz:` discards `default: all` and every threshold
// and still exits 0 reporting no issues. `config verify` is what turns
// that into a failure, and it has to run BEFORE the lint, or the lint
// spends a minute reporting a verdict from a config already known to be
// wrong.
func TestLintDockerfileVerifiesTheLinterConfig(t *testing.T) {
t.Parallel()
found := instructions(t, lintDockerfile)
verify := linterBinary + " config verify"
verifyAt := indexContaining(found, verify)
require.GreaterOrEqual(t, verifyAt, 0,
"%s must run `%s --config .golangci.yml`: without it a typo'd"+
" top-level key in .golangci.yml is silently ignored and the"+
" gate passes with only the default linter set", lintDockerfile,
verify)
runAt := indexContaining(found, linterBinary+" run")
require.GreaterOrEqual(t, runAt, 0, "%s must lint", lintDockerfile)
assert.Less(t, verifyAt, runAt,
"%s must verify the config before linting with it", lintDockerfile)
// Keyed on the epoch like every other check layer, so it executes
// per invocation rather than being replayed. A cached validation
// validates nothing.
assertEpochExpandedInto(t, found, verify)
}
// TestProductDockerfileCannotBeCachedGreen holds the same line for the
// checks that remain in the product image build.
func TestProductDockerfileCannotBeCachedGreen(t *testing.T) {
t.Parallel()
found := instructions(t, productDockerfile)
argAt := indexOf(found, checkEpochARG)
require.GreaterOrEqual(t, argAt, 0,
"%s must declare `%s` with no default value",
productDockerfile, checkEpochARG)
assert.GreaterOrEqual(t, indexOf(found, checkEpochGuard), argAt,
"%s must guard against an empty CHECK_EPOCH", productDockerfile)
assertEpochExpandedInto(t, found[argAt:], "make fmt-check")
assertEpochExpandedInto(t, found[argAt:], "make test")
}
// TestProductDockerfileDoesNotLint records the split deliberately: the
// linter lives in Dockerfile.lint and nowhere else, so there is exactly
// one digest pinning it. A lint stage reintroduced here would either be
// docker-in-docker (`make lint` is now `docker build`) or a second,
// independently bumpable pin.
func TestProductDockerfileDoesNotLint(t *testing.T) {
t.Parallel()
contents := readRepoFile(t, productDockerfile)
for _, forbidden := range []string{"golangci", "make lint"} {
assert.NotContains(t, instructionText(contents), forbidden,
"%s must not lint: the linter is pinned once, in %s",
productDockerfile, lintDockerfile)
}
}
// TestLintScriptBuildsTheLintDockerfileWithAFreshEpoch is the other
// half of the mechanism. The Dockerfile's guard only rejects an EMPTY
// epoch; a constant non-empty one would satisfy it and still be served
// from cache forever.
func TestLintScriptBuildsTheLintDockerfileWithAFreshEpoch(t *testing.T) {
t.Parallel()
script := readRepoFile(t, lintScript)
assertBareEpochAssignment(t, script, lintScript)
assert.Contains(t, script, `--build-arg CHECK_EPOCH="$epoch"`,
"%s must pass the fresh epoch to the build", lintScript)
assert.Contains(t, script, lintDockerfile,
"%s must build %s", lintScript, lintDockerfile)
}
// TestCibuildBuildsBothDockerfilesWithFreshEpochs guards the CI gate:
// dropping either build silently removes a whole class of check from
// CI while leaving it green.
func TestCibuildBuildsBothDockerfilesWithFreshEpochs(t *testing.T) {
t.Parallel()
script := readRepoFile(t, cibuildScript)
assertBareEpochAssignment(t, script, cibuildScript)
assert.Equal(t, 2, strings.Count(script, freshEpoch),
"%s must compute a fresh epoch for each of its two builds",
cibuildScript)
assert.Equal(t, 2,
strings.Count(script, `--build-arg CHECK_EPOCH="$epoch"`),
"%s must pass a fresh epoch to both builds", cibuildScript)
assert.Contains(t, script, "-f Dockerfile.lint",
"%s must build %s", cibuildScript, lintDockerfile)
}
// TestNoHostLintPathRemains fails if any escape hatch to a host linter
// comes back. The owner's ruling is that every lint run happens inside
// a container; a PATH binary that happens to match the pinned version
// is a different build reached by a different code path, and admitting
// it is what lets a local pass disagree with CI.
//
// This asserts the PROPERTY -- no script invokes the linter except
// through docker -- rather than the absence of any particular variable
// name. An earlier version of this test looked only for the literal
// VAULTIK_LINT_IN_CONTAINER, the name of the hatch that was removed
// alongside it, so nothing could ever trip it again: a hatch under any
// other name left it passing. A structural test that passes on a broken
// tree is worse than no test, because it is what a later reader trusts
// instead of re-deriving the invariant.
//
// script/lint-fix is not exempted. It is the one script that runs the
// linter as a container rather than as a build step, but it still runs
// it in one, so the same property holds of it.
func TestNoHostLintPathRemains(t *testing.T) {
t.Parallel()
root := repoRoot(t)
entries, err := os.ReadDir(filepath.Join(root, "script"))
require.NoError(t, err)
require.NotEmpty(t, entries, "no scripts found to scan")
for _, entry := range entries {
if entry.IsDir() {
continue
}
name := filepath.Join("script", entry.Name())
for _, line := range shellCode(readRepoFile(t, name)) {
assertLinterIsContainerised(t, name, line)
}
}
}
// assertLinterIsContainerised fails if the line runs the linter without
// handing it to docker first. Position matters: docker has to come
// before the binary, or the line is running the host linter and merely
// mentioning docker afterwards.
func assertLinterIsContainerised(t *testing.T, name, line string) {
t.Helper()
at := strings.Index(line, linterBinary)
if at < 0 {
return
}
docker := strings.Index(line, "docker")
assert.True(t, docker >= 0 && docker < at,
"%s runs %s on the host; every lint run happens in a container"+
" (line: %s)", name, linterBinary, line)
}
// TestShellCodeSeesCodeAndNotProse keeps the scanner above honest. It
// has to ignore comments and here-document bodies, because script/lint
// and script/bootstrap both NAME golangci-lint in prose -- in comments,
// and in the error text they print -- precisely to say that the host
// binary is never used. A scanner that went blind, by over-eager
// stripping or by failing to join continuation lines, would make
// TestNoHostLintPathRemains pass on everything.
func TestShellCodeSeesCodeAndNotProse(t *testing.T) {
t.Parallel()
script := strings.Join([]string{
"#!/bin/sh",
"# a comment naming golangci-lint",
"cat >&2 <<EOF",
"prose naming golangci-lint, printed not executed",
"EOF",
"docker run --rm \\",
" \"$image\" \\",
" golangci-lint run ./...",
}, "\n")
assert.Equal(t,
[]string{"cat >&2 <<EOF", `docker run --rm "$image" golangci-lint run ./...`},
shellCode(script))
}
// assertEpochExpandedInto fails unless some instruction runs the named
// command with the epoch expanded into it. Expansion, not mere
// declaration: an ARG that no instruction references is not guaranteed
// to key the layer, and the expansion also puts the value in the build
// log where a reader can see the layer was keyed fresh.
func assertEpochExpandedInto(t *testing.T, found []string, command string) {
t.Helper()
for _, instruction := range found {
if !strings.HasPrefix(instruction, "RUN ") {
continue
}
if strings.Contains(instruction, command) &&
strings.Contains(instruction, "${CHECK_EPOCH}") {
return
}
}
assert.Fail(t, "no epoch-keyed layer runs the command",
"`%s` must run in a layer that expands ${CHECK_EPOCH}, or it"+
" will be replayed from cache without executing", command)
}
// assertBareEpochAssignment fails unless the script computes the epoch
// as a bare assignment on its own line.
func assertBareEpochAssignment(t *testing.T, script, name string) {
t.Helper()
for line := range strings.SplitSeq(script, "\n") {
if strings.TrimSpace(line) == freshEpoch {
return
}
}
assert.Fail(t, "no bare epoch assignment",
"%s must compute `%s` as a bare assignment on its own line, so"+
" `set -e` catches a failing date instead of quietly"+
" building with an empty epoch", name, freshEpoch)
}
// instructions returns the Dockerfile's instructions, one per element,
// with comments and blank lines dropped and continuation lines joined,
// so a multi-line RUN is one string.
func instructions(t *testing.T, name string) []string {
t.Helper()
return strings.Split(instructionText(readRepoFile(t, name)), "\n")
}
// instructionText is instructions' parse, before splitting: it is also
// what a "must not contain" assertion should look at, so that a word
// appearing only in a comment is not mistaken for behaviour.
func instructionText(contents string) string {
var (
out []string
continued string
isContinued bool
)
for line := range strings.SplitSeq(contents, "\n") {
trimmed := strings.TrimSpace(line)
if !isContinued && (trimmed == "" || strings.HasPrefix(trimmed, "#")) {
continue
}
isContinued = strings.HasSuffix(trimmed, `\`)
continued += strings.TrimSuffix(trimmed, `\`)
if isContinued {
continue
}
out = append(out, strings.Join(strings.Fields(continued), " "))
continued = ""
}
return strings.Join(out, "\n")
}
// indexOf returns the position of the first instruction equal to, or
// beginning with, want; -1 if there is none.
func indexOf(found []string, want string) int {
for i, instruction := range found {
if instruction == want || strings.HasPrefix(instruction, want+" ") {
return i
}
}
return -1
}
// indexContaining returns the position of the first instruction
// containing want; -1 if there is none.
func indexContaining(found []string, want string) int {
for i, instruction := range found {
if strings.Contains(instruction, want) {
return i
}
}
return -1
}
// shellCode returns a POSIX shell script's executable lines: comments
// dropped, here-document bodies dropped, and backslash continuations
// joined so a multi-line command is a single string. Whitespace is
// collapsed, as it is for Dockerfile instructions.
//
// Both exclusions are load-bearing rather than tidiness. The scripts
// name golangci-lint in prose to state that the host binary is never
// used, and joining continuations is what lets the one legitimate
// container invocation -- script/lint-fix's `docker run`, whose linter
// command sits several lines below the word `docker` -- be recognised
// as containerised.
func shellCode(contents string) []string {
var (
out []string
joined string
terminate string
)
for line := range strings.SplitSeq(contents, "\n") {
trimmed := strings.TrimSpace(line)
if terminate != "" {
if trimmed == terminate {
terminate = ""
}
continue
}
if joined == "" && (trimmed == "" || strings.HasPrefix(trimmed, "#")) {
continue
}
joined += strings.TrimSuffix(trimmed, `\`) + " "
if strings.HasSuffix(trimmed, `\`) {
continue
}
joined = strings.Join(strings.Fields(joined), " ")
terminate = heredocTerminator(joined)
out = append(out, joined)
joined = ""
}
return out
}
// heredocTerminator returns the terminator of the here-document a
// command opens, or "" if it opens none. Only the first on a line is
// recognised; nothing in script/ opens two.
func heredocTerminator(line string) string {
_, after, opens := strings.Cut(line, "<<")
if !opens {
return ""
}
// `<<-` strips leading tabs from the body; the terminator word is
// the same either way, and callers compare against trimmed lines.
word, _, _ := strings.Cut(strings.TrimPrefix(after, "-"), " ")
return strings.Trim(word, `'"`)
}
// readRepoFile reads a file by its path relative to the repository
// root.
func readRepoFile(t *testing.T, name string) string {
t.Helper()
//nolint:gosec // G304: the path is a constant relative to this repo
contents, err := os.ReadFile(filepath.Join(repoRoot(t), name))
require.NoError(t, err)
return string(contents)
}
// repoRoot returns the repository root. The test binary runs with its
// package directory as the working directory, so the root is found by
// walking up until the module file appears.
func repoRoot(t *testing.T) string {
t.Helper()
dir, err := os.Getwd()
require.NoError(t, err)
for {
_, err = os.Stat(filepath.Join(dir, "go.mod"))
if err == nil {
return dir
}
parent := filepath.Dir(dir)
require.NotEqual(t, dir, parent,
"walked to the filesystem root without finding a go.mod")
dir = parent
}
}

View File

@@ -1,8 +1,6 @@
package main_test package main_test
import ( import (
"os"
"path/filepath"
"regexp" "regexp"
"slices" "slices"
"strings" "strings"
@@ -87,27 +85,11 @@ func TestBuildTargetBuildsTheBinary(t *testing.T) {
} }
// readMakefile returns the contents of the repository's Makefile. The // readMakefile returns the contents of the repository's Makefile. The
// test binary runs with its package directory as the working directory, // root is located by the shared walk in lintdocker_test.go.
// so the root is found by walking up until the Makefile appears.
func readMakefile(t *testing.T) string { func readMakefile(t *testing.T) string {
t.Helper() t.Helper()
dir, err := os.Getwd() return readRepoFile(t, "Makefile")
require.NoError(t, err)
for {
//nolint:gosec // G304: the path is this test's own directory walk
contents, err := os.ReadFile(filepath.Join(dir, "Makefile"))
if err == nil {
return string(contents)
}
parent := filepath.Dir(dir)
require.NotEqual(t, dir, parent,
"walked to the filesystem root without finding a Makefile")
dir = parent
}
} }
// phonyTargets returns every name declared phony, across all .PHONY // phonyTargets returns every name declared phony, across all .PHONY

View File

@@ -48,11 +48,12 @@ missing() {
! command -v "$1" >/dev/null 2>&1 ! command -v "$1" >/dev/null 2>&1
} }
# Docker is a hard requirement, not a nice-to-have: script/lint runs the # Docker is a hard requirement, not a nice-to-have: script/lint lints by
# digest-pinned golangci-lint image from the Dockerfile's lint stage, and # building Dockerfile.lint, whose digest-pinned golangci-lint image is
# script/check and script/precommit both run script/lint. A bootstrap # the only place the linter runs, and script/check and script/precommit
# that prints "bootstrap complete" on a machine where `make check` cannot # both run script/lint. A bootstrap that prints "bootstrap complete" on a
# run is a false success, so this fails instead. # machine where `make check` cannot run is a false success, so this fails
# instead.
# #
# Installing docker from here was considered and rejected: it needs root, # Installing docker from here was considered and rejected: it needs root,
# a running daemon, and on macOS a GUI cask, so an attempt would itself # a running daemon, and on macOS a GUI cask, so an attempt would itself
@@ -79,13 +80,15 @@ bootstrap: FAILED - $reason.
Docker is required to develop this repo. Without it these do not work: Docker is required to develop this repo. Without it these do not work:
script/lint runs the digest-pinned golangci-lint image declared script/lint builds Dockerfile.lint, which runs the linter as a
by the Dockerfile's lint stage, which is the single build step in a digest-pinned golangci-lint image.
source of truth for the linter version That FROM line is the single source of truth for the
linter version
script/check runs script/lint script/check runs script/lint
script/precommit runs script/check, so commits are blocked by the script/precommit runs script/check, so commits are blocked by the
pre-commit hook installed by script/setup pre-commit hook installed by script/setup
script/cibuild builds the Dockerfile, which is what CI runs script/cibuild builds Dockerfile.lint and Dockerfile, which is what
CI runs
Install docker (and start the daemon, checking DOCKER_HOST and your Install docker (and start the daemon, checking DOCKER_HOST and your
group membership), then re-run script/bootstrap. golangci-lint on PATH group membership), then re-run script/bootstrap. golangci-lint on PATH
@@ -104,12 +107,12 @@ main() {
# Go toolchain # Go toolchain
if missing go; then pkg_install go golang go go; fi if missing go; then pkg_install go golang go go; fi
# golangci-lint is deliberately NOT installed: script/lint runs the # golangci-lint is deliberately NOT installed: script/lint lints by
# digest-pinned golangci-lint image from the Dockerfile's lint stage, # building Dockerfile.lint, whose digest-pinned image is the only
# so whatever a package manager happens to ship would only be a # place the linter runs, so whatever a package manager happens to
# shadow of the pinned version that could drift from CI. script/lint # ship would only be a shadow of the pinned version that could drift
# will not use a PATH binary on a host at any version, so installing # from CI. Nothing on the host is ever used as a linter, at any
# one here would buy nothing. # version, so installing one here would buy nothing.
# sqlite3 CLI: the test suite shells out to it (VACUUM). # sqlite3 CLI: the test suite shells out to it (VACUUM).
if missing sqlite3; then pkg_install sqlite sqlite3 sqlite sqlite; fi if missing sqlite3; then pkg_install sqlite sqlite3 sqlite sqlite; fi

View File

@@ -1,22 +1,31 @@
#!/bin/sh #!/bin/sh
# script/cibuild: run the CI build. The Dockerfile does not run # script/cibuild: run the CI build. This is the full gate, and it is two
# script/check; it runs `make fmt-check` and `make lint` in its lint # builds, in this order:
# stage and `make test` in its builder stage. A successful build #
# implies those three passed, provided they actually ran -- which is # Dockerfile.lint the linter, as a build step (a clean build IS a
# what the CHECK_EPOCH below is for. # clean lint)
# Generic: needs no adaptation. The Gitea workflow runs this on push. # Dockerfile `make fmt-check` and `make test` in the builder
# stage, then the product image
#
# Either one failing fails this script. Note what follows from the
# split: script/docker builds only the product image and so no longer
# lints -- this script and script/check (which runs script/lint) are the
# things that decide whether the tree is clean.
#
# Generic apart from the two Dockerfiles: the Gitea workflow runs this
# on push.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
# The Dockerfile's check layers are keyed on CHECK_EPOCH, so a # Both Dockerfiles key their check layers on CHECK_EPOCH, so a fresh
# fresh value here is what forces them to re-run: without it an # value is what forces those layers to re-run: without it an
# unchanged tree replays them from cache, the checks never execute, # unchanged tree replays them from cache, the checks never execute,
# and the build still exits 0. The ARG sits immediately above the # and the build still exits 0. Each ARG sits immediately above the
# check RUNs, so dependency and module layers still cache. The # check RUNs, so dependency and module layers still cache. Both
# Dockerfile also refuses to build at all when CHECK_EPOCH is empty, # Dockerfiles also refuse to build at all when CHECK_EPOCH is empty,
# so a missing value fails loudly here rather than passing quietly. # so a missing value fails loudly here rather than passing quietly.
# #
# The value must be unique per invocation, not per second. `date +%s` # The value must be unique per invocation, not per second. `date +%s`
@@ -35,6 +44,18 @@ main() {
# script exists to prevent -- so the guard would disarm itself and # script exists to prevent -- so the guard would disarm itself and
# still exit 0. As a bare assignment, `set -e` catches a failing # still exit 0. As a bare assignment, `set -e` catches a failing
# `date` and no build starts. # `date` and no build starts.
#
# A separate value per build, because they are separate builds: one
# `date` shared between them would still be fresh, but reusing it
# invites the two to be collapsed into a single value that is
# computed somewhere else and passed in.
epoch="$(date +%s%N)$$"
# cacheonly for the lint build: its verdict is the exit status and
# the image is never run, so exporting it is pure cost. See
# script/lint.
docker build --output=type=cacheonly \
--build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint .
epoch="$(date +%s%N)$$" epoch="$(date +%s%N)$$"
docker build --build-arg CHECK_EPOCH="$epoch" . docker build --build-arg CHECK_EPOCH="$epoch" .
} }

View File

@@ -2,6 +2,13 @@
# script/docker: build the Docker image tagged with the project name. # script/docker: build the Docker image tagged with the project name.
# Identical in all repos; the tag comes from script/projectname. # Identical in all repos; the tag comes from script/projectname.
# Generic: needs no adaptation. # Generic: needs no adaptation.
#
# This builds the PRODUCT image only, and the product Dockerfile has no
# lint stage: linting lives in Dockerfile.lint and is run by
# script/lint. So a green here means `make fmt-check` and `make test`
# passed and the image built -- it says nothing about lint. The gates
# are script/check (which runs script/lint) and script/cibuild (which
# builds both files).
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"

View File

@@ -1,110 +1,42 @@
#!/bin/sh #!/bin/sh
# script/lint: run the linter. # script/lint: run the linter.
# #
# The linter always runs at the version pinned by the Dockerfile's lint # The linter runs inside the image built by Dockerfile.lint, and it runs
# stage, so a local run and a CI run of the same tree cannot disagree. # there as a BUILD STEP: a successful build of that file IS a clean
# That FROM line (image tag plus digest) is the single source of truth # lint. Nothing lints on the host, at any version, ever. That FROM line
# for the linter version in this repo: bump it there and nothing else # is the single source of truth for the linter version in this repo, so
# needs editing. # a local run and a CI run of the same tree cannot disagree.
# #
# Normally that means running the pinned image with docker. The one # One container per run means one lint cache and one golangci-lint lock
# exception is running INSIDE that image: the Dockerfile's lint stage # per run, both private to that run and thrown away with it. That is
# runs `make lint`, and there is no docker daemon in there. That stage # what makes concurrent runs on a shared host safe, and it is why this
# sets VAULTIK_LINT_IN_CONTAINER=1, and only when that variable is set # script no longer carries per-worktree cache directories, a lock-retry
# is a golangci-lint on PATH used directly - and then only if its # loop, or an output audit: there is no shared state left for them to
# version is exactly the pin. Version equality alone is deliberately NOT # defend (issue https://git.eeqj.de/sneak/vaultik/issues/113).
# enough: it also matches a developer's locally installed copy of the
# same version, which is a different build with a different Go
# toolchain, reached by a different code path, and it would bypass the
# digest pin this script exists to enforce. /.dockerenv was considered
# as the context signal and rejected: dockerd creates it for `docker
# run`, but it is not reliably present during a BuildKit `docker build`,
# which is exactly the case the exception exists for.
# #
# The linter's output is checked before it is believed: every run is # To watch the linter execute, set BUILDKIT_PROGRESS=plain, which docker
# audited by script/lint-audit for findings that cannot belong to this # honours directly:
# tree, and a run refused by golangci-lint's cross-process lock is
# retried rather than reported as a verdict. See the lock-retry loop in
# main and the header of script/lint-audit.
# #
# Extra arguments are passed through to `golangci-lint run`, before # BUILDKIT_PROGRESS=plain script/lint
# `./...` (see script/lint-fix). #
# The check layers -- `golangci-lint config verify` and then
# `golangci-lint run` -- must appear as executing rather than CACHED on
# every run; see the CHECK_EPOCH comment in Dockerfile.lint.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
DOCKERFILE="$ROOT/Dockerfile" DOCKERFILE="$ROOT/Dockerfile.lint"
# golangci-lint takes a cross-process lock and refuses to start while
# another instance holds it. That refusal is not a lint result, and
# exiting non-zero on it is indistinguishable to a caller from real
# findings - so it is retried rather than reported. Bounded, because a
# lock that is never released must fail rather than hang.
LOCK_MESSAGE="parallel golangci-lint is running"
LOCK_ATTEMPTS=6
LOCK_SLEEP=15
# The image reference of the Dockerfile's lint stage, tag and digest
# included, e.g.
# golangci/golangci-lint:v2.12.2-alpine@sha256:91b2...
lint_image() {
awk '$1 == "FROM" && $3 == "AS" && $4 == "lint" { print $2; exit }' \
"$DOCKERFILE"
}
# The bare version that image reference pins, e.g. 2.12.2
pinned_version() {
lint_image | sed -e 's/@.*//' -e 's/.*://' -e 's/^v//' -e 's/-.*//'
}
# The version of the golangci-lint on PATH, if any, e.g. 2.12.2
#
# `version --short` prints the bare version and is the interface meant
# for this (checked against 2.10.1 and 2.12.2). The banner scrape below
# it is a fallback for a release where --short is absent or silent; the
# banner's exact wording is not a stable interface, which is why it is
# no longer the primary parse.
installed_version() {
command -v golangci-lint >/dev/null 2>&1 || return 0
short="$(golangci-lint version --short 2>/dev/null |
tr -d '[:space:]' | sed -e 's/^v//')"
case "$short" in
*[0-9].[0-9]*.[0-9]*)
echo "$short"
return 0
;;
esac
golangci-lint version 2>/dev/null | awk '
{
for (i = 1; i <= NF; i++) {
if ($i ~ /^[0-9]+\.[0-9]+\.[0-9]+$/) {
print $i
exit
}
}
}'
}
# True inside the Dockerfile's lint stage, which sets this. Nothing else
# sets it: setting it by hand on a host is an explicit, visible decision
# to lint with an unpinned binary, not something reached by accident.
in_lint_container() {
[ "${VAULTIK_LINT_IN_CONTAINER:-}" = "1" ]
}
require_docker() { require_docker() {
image="$1"
if ! command -v docker >/dev/null 2>&1; then if ! command -v docker >/dev/null 2>&1; then
cat >&2 <<EOF cat >&2 <<EOF
lint: docker is required to run the pinned linter. lint: docker is required to run the pinned linter.
pinned image: $image lint image declared by: $DOCKERFILE
Install docker. Linting with any other golangci-lint is not supported: Install docker. Linting with any other golangci-lint is not supported:
it is what lets a local run pass while CI fails. An installed it is what lets a local run pass while CI fails. A golangci-lint on
golangci-lint on PATH is not used, whatever its version; only the lint PATH is never used, whatever its version.
stage of the Dockerfile itself runs the linter natively.
EOF EOF
exit 1 exit 1
fi fi
@@ -113,7 +45,7 @@ EOF
lint: the docker daemon is not reachable, so the pinned linter cannot lint: the docker daemon is not reachable, so the pinned linter cannot
run. run.
pinned image: $image lint image declared by: $DOCKERFILE
Start the daemon (and check DOCKER_HOST / your group membership). This Start the daemon (and check DOCKER_HOST / your group membership). This
script will not fall back to a different linter version or to an script will not fall back to a different linter version or to an
@@ -123,213 +55,54 @@ EOF
fi fi
} }
# Where the per-worktree caches live. usage() {
cache_home() { cat >&2 <<EOF
echo "${XDG_CACHE_HOME:-${HOME:-/tmp}/.cache}/vaultik-lint" usage: $(basename "$0")
}
# A short, stable digest of this worktree's path. script/lint takes no arguments. The linter runs as a build step, so
path_digest() { there is no command line to pass flags to; anything accepted here would
if command -v sha256sum >/dev/null 2>&1; then have to be silently dropped. To apply autofixes, use script/lint-fix,
printf '%s' "$ROOT" | sha256sum | cut -c1-12 which runs the same pinned image as a container for exactly this
elif command -v shasum >/dev/null 2>&1; then reason.
printf '%s' "$ROOT" | shasum -a 256 | cut -c1-12 EOF
else exit 2
printf '%s' "$ROOT" | cksum | tr -cd '0-9' | cut -c1-12
fi
}
# Caches for the containerized linter, private to THIS worktree.
#
# Keeping them out of the repo and persisting them between runs is what
# keeps the inner loop fast: a warm run costs about the same as a native
# one plus container startup. Keeping them keyed on the worktree path is
# what keeps them correct. One shared cache for the whole repo was the
# defect in issue #99: two worktrees of this repo have identical file
# contents, so their cache keys collide, and golangci-lint replays the
# stored results - including the file paths recorded when they were
# produced. That silently reports one worktree's findings, or one
# worktree's clean bill of health, for another.
cache_dir() {
slug="$(printf '%s' "$(basename "$ROOT")" | tr -c 'A-Za-z0-9._-' '-')"
echo "$(cache_home)/$slug-$(path_digest)"
}
# One cache per worktree means throwaway worktrees would otherwise leave
# caches behind forever. Each cache records the worktree it belongs to,
# and any cache whose worktree no longer exists is collected here, so
# growth is bounded by the number of worktrees that actually exist. The
# whole tree also sits under XDG_CACHE_HOME (~/.cache by default), so it
# is disposable by definition: `rm -rf "${XDG_CACHE_HOME:-~/.cache}/vaultik-lint"`
# costs nothing but the next run's cold cache.
prune_dead_caches() {
home="$(cache_home)"
if [ ! -d "$home" ]; then
return 0
fi
for dir in "$home"/*; do
if [ ! -f "$dir/worktree" ]; then
continue
fi
owner="$(cat "$dir/worktree")"
if [ -z "$owner" ]; then
continue
fi
if [ ! -d "$owner" ]; then
# The Go module cache inside is deliberately read-only, and
# rm(1) cannot unlink a file out of a directory it may not
# write, so the tree has to be made writable first. And
# failing to tidy up is a housekeeping problem, never a
# reason to fail a lint: without the fallback below, `set
# -e` turns a stale cache that will not delete into a lint
# error, which is a gate failing for a reason that has
# nothing to do with the code. (Observed, not theorised.)
chmod -R u+w "$dir" 2>/dev/null || true
if ! rm -rf "$dir" 2>/dev/null; then
# Restore the marker on a partial removal: an
# unmarked leftover would be skipped by every future
# run and never collected.
mkdir -p "$dir" 2>/dev/null || true
echo "$owner" >"$dir/worktree" 2>/dev/null || true
echo "lint: could not remove stale cache $dir" >&2
fi
fi
done
}
prepare_cache() {
cache="$1"
mkdir -p "$cache/go-build" "$cache/go-mod" "$cache/golangci-lint"
echo "$ROOT" >"$cache/worktree"
}
# Run the linter, wherever it is that this script is allowed to run it.
run_linter() {
if in_lint_container; then
golangci-lint run "$@" ./...
return $?
fi
docker run --rm \
--user "$(id -u):$(id -g)" \
--env HOME=/tmp \
--env GOFLAGS=-buildvcs=false \
--env GOCACHE=/cache/go-build \
--env GOMODCACHE=/cache/go-mod \
--env GOLANGCI_LINT_CACHE=/cache/golangci-lint \
--volume "$ROOT:/src" \
--volume "$CACHE:/cache" \
--workdir /src \
"$IMAGE" \
golangci-lint run "$@" ./...
}
# Run the linter, streaming its combined output while also capturing it,
# and hand back its exit status. The output has to be inspected before
# it is believed, which is why this script no longer just execs the
# linter. `tee` would swallow the status, so it is smuggled out through
# a file: there is no pipefail in POSIX sh.
run_capture() {
capture="$1"
shift
rm -f "$capture.status"
{
rc=0
# `set -e` is in force inside this subshell too, so the status
# has to be caught here: an unguarded non-zero exit (which is
# what "the linter found something" looks like) would abort the
# subshell before the status was ever written.
run_linter "$@" 2>&1 || rc=$?
echo "$rc" >"$capture.status"
} | tee "$capture"
if [ ! -s "$capture.status" ]; then
echo "lint: the linter did not report an exit status" >&2
exit 1
fi
read -r captured_status <"$capture.status"
rm -f "$capture.status"
return "$captured_status"
}
# Reject output that cannot describe this tree. See script/lint-audit
# for what that means and why: in short, a finding citing a file that is
# not here means the result being reported was produced somewhere else,
# and a PASS built out of another checkout's analysis is silent (issue
# #99). The audit therefore runs on clean output as well.
audit_output() {
capture="$1"
if ! "$ROOT/script/lint-audit" "$capture"; then
if [ -n "$CACHE" ]; then
echo " this tree's lint cache: $CACHE" >&2
fi
exit 1
fi
} }
main() { main() {
[ "$#" -eq 0 ] || usage
cd "$ROOT" cd "$ROOT"
require_docker
IMAGE="$(lint_image)" # A fresh epoch per invocation is what forces the check layers to
if [ -z "$IMAGE" ]; then # execute; the layers above the ARG in Dockerfile.lint still cache,
echo "lint: no lint stage found in $DOCKERFILE" >&2 # so a run is not cold. The value must be unique per invocation, not
exit 1 # per second: `date +%s` is second-granular, so two concurrent
fi # invocations in the same second would get identical epochs and the
# later one could be served from cache -- the false green in
# miniature. `%N` alone does not fix it either, because busybox
# silently drops %N, exits 0, and hands back second granularity with
# no warning. `$$` is what makes this correct regardless, since
# concurrent invocations have different pids.
#
# Assign it on its own line rather than inline in the argument.
# Under `set -eu` a command substitution that fails inside an
# argument does NOT abort the script: CHECK_EPOCH would become an
# empty string, an empty string is a constant, and a constant epoch
# is exactly the cached-lint false green this guards against. As a
# bare assignment, `set -e` catches a failing `date` and no build
# starts.
epoch="$(date +%s%N)$$"
CACHE="" # cacheonly: the lint verdict is the build's exit status, and the
if in_lint_container; then # image it would otherwise produce is never run. Exporting it costs
# No docker daemon in here, so there is no fallback: a mismatch # most of the wall time of a warm run and leaves a dangling image
# is a hard error rather than a quiet substitution. # behind on every invocation, on a host that may be running many.
installed="$(installed_version)" docker build \
pinned="$(pinned_version)" --output=type=cacheonly \
if [ -z "$installed" ] || [ "$installed" != "$pinned" ]; then --build-arg CHECK_EPOCH="$epoch" \
cat >&2 <<EOF -f "$DOCKERFILE" \
lint: VAULTIK_LINT_IN_CONTAINER is set, so this is expected to be "$ROOT"
running inside the Dockerfile's pinned lint image, but the golangci-lint
on PATH does not match the pin.
pinned: $pinned ($IMAGE)
installed: ${installed:-<none>}
EOF
exit 1
fi
else
require_docker "$IMAGE"
prune_dead_caches
CACHE="$(cache_dir)"
prepare_cache "$CACHE"
fi
capture="$(mktemp "${TMPDIR:-/tmp}/vaultik-lint.XXXXXX")"
trap 'rm -f "$capture" "$capture.status"' EXIT HUP INT TERM
attempt=1
while :; do
status=0
run_capture "$capture" "$@" || status=$?
if grep -Fq "$LOCK_MESSAGE" "$capture"; then
if [ "$attempt" -lt "$LOCK_ATTEMPTS" ]; then
echo "lint: another golangci-lint holds the lock;" \
"retrying in ${LOCK_SLEEP}s" \
"(attempt $attempt of $LOCK_ATTEMPTS)" >&2
sleep "$LOCK_SLEEP"
attempt=$((attempt + 1))
continue
fi
cat >&2 <<EOF
lint: gave up after $LOCK_ATTEMPTS attempts, each blocked by another
golangci-lint holding the cross-process lock. This is NOT a lint
verdict: the tree was never analysed. Re-run when the other run has
finished.
EOF
exit 1
fi
audit_output "$capture"
exit "$status"
done
} }
main "$@" main "$@"

View File

@@ -1,113 +0,0 @@
#!/bin/sh
# script/lint-audit: audit a captured golangci-lint run for output that
# cannot describe this tree. Called by script/lint on every run; usable
# on its own against any saved lint output.
#
# script/lint-audit <capture-file>
#
# Exits 0 when every finding cites a file in this tree, 1 when any does
# not. It NEVER certifies that a lint run passed - it has no idea
# whether the run found issues, and does not look. It only rejects
# output that is impossible for this tree, which is a different and much
# weaker claim. Do not use it as a gate; use script/lint.
#
# Why this exists (issue #99): golangci-lint caches analysis results,
# and a cache shared between two checkouts of this repo can serve one
# checkout's stored findings for another, file paths included. The
# failure is symmetric and only one direction is loud - a clean tree
# failed by a dirty sibling gets investigated, while a dirty tree passed
# by a clean sibling is silent. This turns the silent direction into a
# hard error, which is why it runs on clean output too.
#
# The primary fix is that script/lint now keys its cache on the worktree
# path so the collision cannot happen. This is the backstop, because a
# backstop that only runs when we already believe things are fine is
# worth more than one more assumption.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# Where script/lint bind-mounts the tree inside the pinned image. A
# containerized run that prints absolute paths (`--path-mode abs`)
# prints them under this, so they are this tree's files under another
# name. Note the consequence, and why the cache key rather than this
# check is the real fix: two containerized runs of different checkouts
# both call themselves /src, so contamination between two container
# runs is not distinguishable by path alone.
CONTAINER_ROOT="/src"
usage() {
echo "usage: $(basename "$0") <capture-file>" >&2
exit 2
}
# Every path cited by a finding that is not a file in this tree.
#
# The linter runs with the tree root as its working directory, so a
# legitimate finding cites either a relative path that resolves inside
# the tree or an absolute path under the root. A path that escapes
# (absolute and elsewhere, or with a `..` component) or that names a
# file which is not here describes something this run did not analyse.
foreign_paths() {
capture="$1"
awk -F: '$1 ~ /\.go$/ && $2 ~ /^[0-9]+$/ { print $1 }' "$capture" |
sort -u |
while IFS= read -r path; do
case "$path" in
"$ROOT"/*)
path="${path#"$ROOT"/}"
;;
"$CONTAINER_ROOT"/*)
path="${path#"$CONTAINER_ROOT"/}"
;;
/*)
printf '%s\n' "$path"
continue
;;
../* | */../*)
printf '%s\n' "$path"
continue
;;
esac
if [ ! -e "$ROOT/$path" ]; then
printf '%s\n' "$path"
fi
done
}
main() {
[ "$#" -eq 1 ] || usage
capture="$1"
if [ ! -f "$capture" ]; then
echo "lint-audit: no such capture file: $capture" >&2
exit 2
fi
foreign="$(foreign_paths "$capture")"
if [ -z "$foreign" ]; then
exit 0
fi
cat >&2 <<EOF
lint: REJECTED - the linter reported findings for files that are not in
this tree, so its output does not describe the tree that was linted.
This result is void, whichever way it went: a pass here would be a pass
earned by analysing someone else's code.
tree: $ROOT
Paths reported that are not in this tree:
EOF
printf '%s\n' "$foreign" | sed -e 's/^/ /' >&2
cat >&2 <<EOF
This is the signature of analysis replayed from a cache belonging to
another checkout (issue #99). Clear this tree's lint cache and re-run:
rm -rf "\${XDG_CACHE_HOME:-\$HOME/.cache}/vaultik-lint"
EOF
exit 1
}
main "$@"

View File

@@ -1,18 +1,54 @@
#!/bin/sh #!/bin/sh
# script/lint-fix: run the linter's autofixer. Rewrites files in place # script/lint-fix: run the linter's autofixer. Rewrites files in place
# for every finding the enabled linters know how to fix; findings # for every finding the enabled linters know how to fix; findings
# without an autofix are reported but left alone (exit status is # without an autofix are reported but left alone.
# nonzero while any remain).
# #
# Delegates to script/lint so the autofixer is the same pinned linter # THIS IS A DEVELOPER CONVENIENCE AND NEVER A GATE. Nothing in
# version that script/lint and CI use - fixes written by a different # script/check, script/precommit or script/cibuild calls it, and no gate
# version are not necessarily fixes for the version that gates. # reads its exit status. The gate is script/lint, which builds
# Dockerfile.lint; run that afterwards to find out whether the tree is
# actually clean.
#
# Unlike script/lint this cannot be a build step: a build step writes
# into an image, and fixes have to land in the worktree. So it runs the
# same pinned image as a container with the tree bind-mounted, which
# means it needs a LOCAL docker daemon -- a remote daemon has no access
# to these files, and this script will appear to do nothing there. The
# image reference is parsed out of Dockerfile.lint's FROM line, so the
# autofixer is always the same version as the linter that gates; fixes
# written by a different version are not necessarily fixes for the
# version that decides.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
DOCKERFILE="$ROOT/Dockerfile.lint"
# The image reference from Dockerfile.lint, tag and digest included.
lint_image() {
awk '$1 == "FROM" { print $2; exit }' "$DOCKERFILE"
}
main() { main() {
exec "$SCRIPT_DIR/lint" --fix "$@" cd "$ROOT"
image="$(lint_image)"
if [ -z "$image" ]; then
echo "lint-fix: no FROM line found in $DOCKERFILE" >&2
exit 1
fi
# Run as the invoking user so the rewritten files stay owned by
# them. HOME is set because the Go and golangci-lint caches default
# under it and that user has no home inside the container; those
# caches are per-container and discarded with it.
docker run --rm \
--user "$(id -u):$(id -g)" \
--env HOME=/tmp \
--env GOFLAGS=-buildvcs=false \
--volume "$ROOT:/src" \
--workdir /src \
"$image" \
golangci-lint run --config .golangci.yml --fix "$@" ./...
} }
main "$@" main "$@"