Compare commits
5 Commits
bfe2b673a2
...
next
| Author | SHA1 | Date | |
|---|---|---|---|
| d257f8f658 | |||
| 696ed9ab4d | |||
| f21e7c9e70 | |||
| c16ef476a9 | |||
| e3f407b440 |
@@ -14,6 +14,39 @@ jobs:
|
|||||||
# the commits since the previous one. A shallow checkout
|
# the commits since the previous one. A shallow checkout
|
||||||
# silently produces a mislabelled release.
|
# silently produces a mislabelled release.
|
||||||
fetch-depth: 0
|
fetch-depth: 0
|
||||||
|
# goreleaser is not a compiler: it shells out to `go` for the
|
||||||
|
# `before:` hook and for every one of the four cross-compiles.
|
||||||
|
# Nothing else in this repo puts a Go toolchain on the runner --
|
||||||
|
# check.yml runs script/cibuild, which does all of its work inside
|
||||||
|
# the digest-pinned Dockerfile images -- so without this step the
|
||||||
|
# release either fails at the before-hook or, worse, ships binaries
|
||||||
|
# built by whatever unpinned Go the runner happens to carry.
|
||||||
|
# REPO_POLICIES.md requires every external reference to be pinned,
|
||||||
|
# and script/release already refuses a goreleaser that is not the
|
||||||
|
# pinned build; the compiler that actually produces the artifacts
|
||||||
|
# is the last thing that should be exempt from that.
|
||||||
|
#
|
||||||
|
# go-version-file rather than a literal: go.mod's `go 1.26.1` is
|
||||||
|
# the single source of truth for the toolchain, the same way the
|
||||||
|
# Dockerfile FROM line is the single source of truth for the
|
||||||
|
# linter version that script/lint enforces. It is a three-component
|
||||||
|
# version, so setup-go resolves it exactly -- no silent drift onto
|
||||||
|
# a newer patch release.
|
||||||
|
#
|
||||||
|
# actions/setup-go v5.6.0, 2025-12-15. Pinned by commit sha, like
|
||||||
|
# the checkout above. v5.x is a node20 action, matching the node20
|
||||||
|
# actions/checkout v4 already in use here; the v6/v7 line requires
|
||||||
|
# a node24 runner, which this Gitea runner has never been asked
|
||||||
|
# for and cannot be assumed to provide.
|
||||||
|
- name: Install Go
|
||||||
|
uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff
|
||||||
|
with:
|
||||||
|
go-version-file: go.mod
|
||||||
|
# setup-go's module cache needs a runner-side cache backend.
|
||||||
|
# A release is cut rarely and a cold module download costs
|
||||||
|
# seconds; a release failing because a cache service is absent
|
||||||
|
# costs a re-tag. Off, deliberately.
|
||||||
|
cache: false
|
||||||
- name: Install goreleaser
|
- name: Install goreleaser
|
||||||
run: script/install-goreleaser
|
run: script/install-goreleaser
|
||||||
- name: Release
|
- name: Release
|
||||||
|
|||||||
@@ -83,8 +83,8 @@ Version: 2025-06-08
|
|||||||
possible to mock or stub these side-effects in tests.
|
possible to mock or stub these side-effects in tests.
|
||||||
|
|
||||||
9. Always use structured logging. Log any relevant state/context with the
|
9. Always use structured logging. Log any relevant state/context with the
|
||||||
messages (but do not log secrets). If stdout is not a terminal, output
|
messages (but do not log secrets). If the log stream is not a terminal,
|
||||||
the structured logs in jsonl format.
|
output the structured logs in jsonl format.
|
||||||
|
|
||||||
10. Avoid using bare strings or numbers in code, especially if they appear
|
10. Avoid using bare strings or numbers in code, especially if they appear
|
||||||
anywhere more than once. Always define a constant (usually at the top
|
anywhere more than once. Always define a constant (usually at the top
|
||||||
|
|||||||
72
Dockerfile
72
Dockerfile
@@ -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
104
Dockerfile.lint
Normal 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 ./...
|
||||||
31
Makefile
31
Makefile
@@ -6,6 +6,16 @@
|
|||||||
# had never been tagged.
|
# had never been tagged.
|
||||||
VERSION := $(shell script/version)
|
VERSION := $(shell script/version)
|
||||||
|
|
||||||
|
# $(shell) discards exit status, so a script/version that is missing,
|
||||||
|
# non-executable or broken would otherwise leave VERSION empty and every
|
||||||
|
# binary built here would print "vaultik " with no version at all. A
|
||||||
|
# build that cannot determine what it is must not produce an artifact.
|
||||||
|
ifeq ($(strip $(VERSION)),)
|
||||||
|
$(error script/version produced no version string; a build that cannot \
|
||||||
|
determine its version will not be made. Check that script/version exists \
|
||||||
|
and is executable)
|
||||||
|
endif
|
||||||
|
|
||||||
# Build variables
|
# Build variables
|
||||||
GIT_REVISION := $(shell git rev-parse HEAD 2>/dev/null || echo "unknown")
|
GIT_REVISION := $(shell git rev-parse HEAD 2>/dev/null || echo "unknown")
|
||||||
GIT_COMMIT_DATE := $(shell git show -s --format=%cs HEAD 2>/dev/null || echo "unknown")
|
GIT_COMMIT_DATE := $(shell git show -s --format=%cs HEAD 2>/dev/null || echo "unknown")
|
||||||
@@ -56,7 +66,18 @@ lint:
|
|||||||
lint-fix:
|
lint-fix:
|
||||||
@script/lint-fix
|
@script/lint-fix
|
||||||
|
|
||||||
# Build binary.
|
# Build binary. `build` is the name the org convention reaches for and
|
||||||
|
# the one a caller checks the exit code of; `vaultik` is the file rule
|
||||||
|
# that does the work, so an unchanged tree still short-circuits.
|
||||||
|
#
|
||||||
|
# This alias is not decorative. `build` was listed in .PHONY with no
|
||||||
|
# rule, and a phony target with no prerequisites and no recipe is
|
||||||
|
# already satisfied: `make build` printed "Nothing to be done" and
|
||||||
|
# exited 0 without producing a binary (issue #110). Every name in
|
||||||
|
# .PHONY needs a rule for that reason; TestPhonyTargetsAllHaveRules in
|
||||||
|
# cmd/vaultik keeps it that way.
|
||||||
|
build: vaultik
|
||||||
|
|
||||||
vaultik: internal/*/*.go cmd/vaultik/*.go
|
vaultik: internal/*/*.go cmd/vaultik/*.go
|
||||||
go build -ldflags "$(LDFLAGS)" -o $@ ./cmd/vaultik
|
go build -ldflags "$(LDFLAGS)" -o $@ ./cmd/vaultik
|
||||||
|
|
||||||
@@ -66,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
|
||||||
|
|
||||||
|
|||||||
149
README.md
149
README.md
@@ -113,11 +113,40 @@ vaultik version
|
|||||||
### global flags
|
### global flags
|
||||||
|
|
||||||
* `--config <path>`: Path to config file (default: `$VAULTIK_CONFIG`, then platform config dir, then `/etc/vaultik/config.yml`)
|
* `--config <path>`: Path to config file (default: `$VAULTIK_CONFIG`, then platform config dir, then `/etc/vaultik/config.yml`)
|
||||||
* `--verbose`, `-v`: Enable verbose output
|
* `--verbose`, `-v`: Enable verbose output (on stderr — see below)
|
||||||
* `--debug`: Enable debug output
|
* `--debug`: Enable debug output (on stderr — see below)
|
||||||
* `--quiet`, `-q`: Suppress non-error output (also suppresses startup banner)
|
* `--quiet`, `-q`: Suppress non-error output (also suppresses startup banner)
|
||||||
* `--skip-errors`: Continue past per-file errors instead of aborting (applies to `snapshot create` and `restore`)
|
* `--skip-errors`: Continue past per-file errors instead of aborting (applies to `snapshot create` and `restore`)
|
||||||
|
|
||||||
|
### stdout and stderr
|
||||||
|
|
||||||
|
Log output — everything from `--verbose` and `--debug`, and every
|
||||||
|
warning and error the logger emits — goes to **stderr**. stdout carries
|
||||||
|
the output you asked for: tables, and the documents produced by `--json`.
|
||||||
|
|
||||||
|
This means `vaultik snapshot list --verbose > out.txt` captures the
|
||||||
|
listing and leaves the diagnostics on your terminal. To capture both,
|
||||||
|
redirect stderr as well (`> out.txt 2> log.txt`, or `> out.txt 2>&1` to
|
||||||
|
interleave them).
|
||||||
|
|
||||||
|
The split is what makes `--json` usable from a script. Warnings and
|
||||||
|
errors are never suppressed — not by `--quiet`, not by `--cron` — so a
|
||||||
|
logger on stdout would eventually land a log line inside a JSON
|
||||||
|
document and break the parse. A config file with group- or
|
||||||
|
world-readable permissions is enough to trigger it.
|
||||||
|
|
||||||
|
Format follows the stream: when stderr is a terminal the records are
|
||||||
|
colorized one-liners, and when it is redirected or piped they are
|
||||||
|
JSON, one object per line.
|
||||||
|
|
||||||
|
Under `--json`, stdout holds the document and nothing else. The startup
|
||||||
|
banner is suppressed, as `--quiet` and `--cron` suppress it, and the
|
||||||
|
progress narration a command would otherwise print — such as the stale
|
||||||
|
local records `prune` reconciles away — is suppressed too, so it cannot
|
||||||
|
land ahead of the document. Every `--json` command therefore pipes on
|
||||||
|
its own, with no additional flag: `vaultik snapshot list --json | jq .`
|
||||||
|
and `vaultik prune --json | jq .` both work as written.
|
||||||
|
|
||||||
### environment variables
|
### environment variables
|
||||||
|
|
||||||
* `VAULTIK_AGE_SECRET_KEY`: Age private key for decryption (required for `snapshot restore` and `snapshot verify --deep`)
|
* `VAULTIK_AGE_SECRET_KEY`: Age private key for decryption (required for `snapshot restore` and `snapshot verify --deep`)
|
||||||
@@ -208,8 +237,9 @@ local index alone, and still exits zero.
|
|||||||
(whether the snapshot is in the local index), `remote_key` (the full
|
(whether the snapshot is in the local index), `remote_key` (the full
|
||||||
64-character storage key), and `remote_present` (whether it was seen
|
64-character storage key), and `remote_present` (whether it was seen
|
||||||
on the destination store, or `null` if the destination could not be
|
on the destination store, or `null` if the destination could not be
|
||||||
listed). The warning about an unlistable destination goes to stderr
|
listed). Warnings about an unlistable destination, unreadable
|
||||||
so stdout stays a single parseable document.
|
manifests, and a truncated listing all go to stderr through the
|
||||||
|
logger, so stdout stays a single parseable document.
|
||||||
|
|
||||||
**`snapshot verify`**: Verify snapshot integrity.
|
**`snapshot verify`**: Verify snapshot integrity.
|
||||||
* Default (shallow): checks that all blobs referenced in the manifest exist in storage
|
* Default (shallow): checks that all blobs referenced in the manifest exist in storage
|
||||||
@@ -504,6 +534,10 @@ All user-facing output goes through helpers in `internal/ui` and conforms
|
|||||||
to a uniform style. Color is enabled when stdout is a TTY and the
|
to a uniform style. Color is enabled when stdout is a TTY and the
|
||||||
`NO_COLOR` environment variable is unset (https://no-color.org/).
|
`NO_COLOR` environment variable is unset (https://no-color.org/).
|
||||||
|
|
||||||
|
`internal/ui` writes to stdout; it is the output the user asked for.
|
||||||
|
Structured log records are a different thing and go through
|
||||||
|
`internal/log`, which writes to stderr (see "stdout and stderr" above).
|
||||||
|
|
||||||
Message classes:
|
Message classes:
|
||||||
|
|
||||||
| Class | Marker | Alignment | Use for |
|
| Class | Marker | Alignment | Use for |
|
||||||
@@ -564,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)
|
||||||
|
|
||||||
@@ -637,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
|
||||||
@@ -701,7 +759,10 @@ agrees with it:
|
|||||||
A build that is not a release never names itself like one. `vaultik
|
A build that is not a release never names itself like one. `vaultik
|
||||||
version` says so in as many words on a development build, and
|
version` says so in as many words on a development build, and
|
||||||
`goreleaser --snapshot` stamps the same `dev-<sha>` string rather than
|
`goreleaser --snapshot` stamps the same `dev-<sha>` string rather than
|
||||||
inventing the next patch number.
|
inventing the next patch number. If `script/version` cannot be run at
|
||||||
|
all, `make` stops with an error instead of building an unversioned
|
||||||
|
binary, and a binary that somehow carries an empty version string still
|
||||||
|
reports itself as a development build.
|
||||||
|
|
||||||
### cutting a release
|
### cutting a release
|
||||||
|
|
||||||
@@ -712,8 +773,9 @@ git tag -a v1.2.3 -m 'v1.2.3'
|
|||||||
git push origin v1.2.3
|
git push origin v1.2.3
|
||||||
```
|
```
|
||||||
|
|
||||||
`.gitea/workflows/release.yml` triggers on `v*` tags, installs the
|
`.gitea/workflows/release.yml` triggers on `v*` tags, installs a Go
|
||||||
pinned `goreleaser`, and runs `script/release`, which builds
|
toolchain and the pinned `goreleaser`, and runs `script/release`, which
|
||||||
|
builds
|
||||||
`linux,darwin × amd64,arm64` archives plus `checksums.txt` and publishes
|
`linux,darwin × amd64,arm64` archives plus `checksums.txt` and publishes
|
||||||
them to this repository's Gitea releases as a draft. `.goreleaser.yaml`
|
them to this repository's Gitea releases as a draft. `.goreleaser.yaml`
|
||||||
has a `gitea_urls:` block pointing at `https://git.eeqj.de/api/v1`;
|
has a `gitea_urls:` block pointing at `https://git.eeqj.de/api/v1`;
|
||||||
@@ -729,6 +791,15 @@ It is passed to `goreleaser` as `GITEA_TOKEN`. The runner's automatic
|
|||||||
token is deliberately not used: it is not guaranteed to carry release
|
token is deliberately not used: it is not guaranteed to carry release
|
||||||
write access.
|
write access.
|
||||||
|
|
||||||
|
The Go toolchain that compiles the released binaries comes from an
|
||||||
|
`actions/setup-go` step pinned by commit sha, reading its version from
|
||||||
|
`go.mod` (currently `1.26.1`, the same version the `Dockerfile` builder
|
||||||
|
stage pins by digest). `goreleaser` shells out to `go` for every
|
||||||
|
cross-compile, so without that step the release would either fail
|
||||||
|
outright or ship binaries built by whatever unpinned toolchain the
|
||||||
|
runner happened to carry — the one unpinned thing in an otherwise
|
||||||
|
hash-pinned release path.
|
||||||
|
|
||||||
To rehearse the whole build without publishing or tagging anything:
|
To rehearse the whole build without publishing or tagging anything:
|
||||||
|
|
||||||
```
|
```
|
||||||
|
|||||||
193
TODO.md
193
TODO.md
@@ -25,6 +25,199 @@ 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`
|
||||||
|
a rule ([issue #108](https://git.eeqj.de/sneak/vaultik/issues/108),
|
||||||
|
[issue #110](https://git.eeqj.de/sneak/vaultik/issues/110)). Two
|
||||||
|
unrelated defects of the same shape — a command reporting something it
|
||||||
|
did not do — landed together because both are small.
|
||||||
|
|
||||||
|
`CleanupLocalSnapshots` wrote three prose lines to stdout with no
|
||||||
|
`--json` awareness, covering every branch of the function, so no input
|
||||||
|
avoided them and `vaultik prune --json | jq` failed even after
|
||||||
|
[issue #106](https://git.eeqj.de/sneak/vaultik/issues/106) removed the
|
||||||
|
banner. `-q` never helped either: `printlnStdout` and `stdoutf` write
|
||||||
|
straight to `Vaultik.Stdout` and never consult `Vaultik.UI`, which is
|
||||||
|
what `SetQuiet` affects. The issue offered three fixes and asked for a
|
||||||
|
decision. Taken: thread `*PruneOptions` into the function and gate each
|
||||||
|
write on `!opts.JSON`, matching `PruneBlobs` (its sibling phase, which
|
||||||
|
already takes the same struct), `RemoveSnapshot` and `remote info`, so
|
||||||
|
the package has one pattern rather than two. Rejected: moving the lines
|
||||||
|
to `log.Info`, because the logger's default level is `slog.LevelWarn`,
|
||||||
|
so that would not relocate them to stderr — it would delete them from a
|
||||||
|
plain `vaultik prune`, and the removal of rows from the local index is
|
||||||
|
not something to narrate only under `--verbose`. Also rejected: putting
|
||||||
|
the stale-record count into `PruneBlobsResult`, whose every field is
|
||||||
|
blob-scoped and which is produced by the later phase; a prune document
|
||||||
|
covering both phases is a reasonable thing to want, but it is a schema
|
||||||
|
design question and not a stream-hygiene fix. The narration is
|
||||||
|
duplicated as `log.Info` records, which `PruneBlobs` already does
|
||||||
|
alongside its own prints, so the events survive on stderr for anyone
|
||||||
|
running `--verbose`.
|
||||||
|
|
||||||
|
`make build` printed "Nothing to be done for 'build'" and exited 0
|
||||||
|
without producing a binary: `build` was listed in `.PHONY` with no
|
||||||
|
`build:` rule anywhere, and declaring a name phony is exactly what
|
||||||
|
converts make's "No rule to make target" error into a silent success.
|
||||||
|
Fixed with `build: vaultik`, keeping `vaultik:` as the file rule. The
|
||||||
|
audit the issue asked for covers all 19 `.PHONY` names; `build` was the
|
||||||
|
only one without a rule, and `vaultik` is correctly absent from
|
||||||
|
`.PHONY`, being a real file target.
|
||||||
|
|
||||||
|
Tests, each verified to fail with the fix reverted rather than assumed
|
||||||
|
to: `CleanupLocalSnapshots` leaves stdout untouched under `--json` in
|
||||||
|
all three branches (stale records, none, empty index) and still emits
|
||||||
|
every line without it, so the guard cannot be satisfied by deleting the
|
||||||
|
output; `prune --json` run end to end through `Entry`, cobra and fx
|
||||||
|
over the process's real stdout descriptor against a `file://` store,
|
||||||
|
asserting exactly one JSON document, in both the stale and non-stale
|
||||||
|
branches; and a parse of the `Makefile` asserting every `.PHONY` name
|
||||||
|
has a rule and that `build` reaches the rule that produces the binary,
|
||||||
|
which keeps the audit true for names added later. That last one is a
|
||||||
|
parse rather than an invocation of `make`, since `make test` is what
|
||||||
|
runs it and shelling back into `make build` would nest a build inside
|
||||||
|
the test run. The property a parse cannot establish — that the recipe
|
||||||
|
still fails when the build fails — was verified by hand against a
|
||||||
|
deliberately broken tree: `make build` exits 2 and produces nothing.
|
||||||
|
`cmd/vaultik` gains its first test file, so `make test` now reports 16
|
||||||
|
packages `ok` where it reported 15.
|
||||||
|
|
||||||
|
- 2026-08-09: Stopped the startup banner from contaminating `--json`
|
||||||
|
documents ([issue #106](https://git.eeqj.de/sneak/vaultik/issues/106)).
|
||||||
|
`Entry` writes the banner to stdout before cobra parses anything, and
|
||||||
|
the flag scan that suppresses it knew `--quiet`, `-q` and `--cron` but
|
||||||
|
not `--json`, so every `--json` document arrived behind two lines of
|
||||||
|
prose and a blank line, and `vaultik snapshot list --json | jq` failed.
|
||||||
|
With the logger already on stderr from
|
||||||
|
[issue #82](https://git.eeqj.de/sneak/vaultik/issues/82), this was the
|
||||||
|
last writer that could put something on stdout that the caller did not
|
||||||
|
ask for. The design question the issue raised — extend the raw-argv
|
||||||
|
scan, or move the banner after parsing — is answered in favour of the
|
||||||
|
scan: the banner is printed first deliberately, so that it still
|
||||||
|
appears when cobra rejects the arguments and on `--help`, and after
|
||||||
|
parsing there is no single place that covers those paths. The stated
|
||||||
|
cost of the scan, that `--json` is a subcommand flag matched anywhere
|
||||||
|
in the vector, is a cost `--cron` already carries — it exists only on
|
||||||
|
`snapshot create` — so this adds an instance of an accepted
|
||||||
|
imprecision rather than a new kind, and the two error directions are
|
||||||
|
not symmetric: a false positive loses a decorative banner, a false
|
||||||
|
negative corrupts a document. Regression tests at the CLI layer, where
|
||||||
|
`internal/vaultik`'s existing guard cannot reach: one runs `Entry`
|
||||||
|
itself over the process's real stdout descriptor, through cobra and fx
|
||||||
|
to the document, made hermetic by `file://` storage; a second covers
|
||||||
|
the argument vectors of all five `--json` commands; a third asserts the
|
||||||
|
banner is still printed without a suppressing flag, so the first
|
||||||
|
cannot be satisfied by deleting the banner. Also corrected `AGENTS.md`
|
||||||
|
policy 9, which still keyed the structured-log format on stdout's
|
||||||
|
TTY-ness after #82 moved that decision to stderr — a rules file that
|
||||||
|
misdescribes the code misleads exactly the readers who trust it most.
|
||||||
|
Two smaller findings from the same review: `bytesAttrKey`'s
|
||||||
|
human-readable byte formatting silently stopped applying under an open
|
||||||
|
group, because the key reaching the comparison is group-qualified
|
||||||
|
(`transfer.bytes`), now matched on its final segment and tested both
|
||||||
|
ways; and `listEnv.stderr` in `snapshot_list_test.go`, assigned but
|
||||||
|
never read since those tests began capturing the process's stderr, is
|
||||||
|
removed. `Vaultik.Stderr` is kept — nothing writes to it today, which
|
||||||
|
its comment now says outright.
|
||||||
|
|
||||||
|
- 2026-08-09: Moved the logger to stderr and fixed `TTYHandler`'s
|
||||||
|
discarded attributes
|
||||||
|
([issue #82](https://git.eeqj.de/sneak/vaultik/issues/82),
|
||||||
|
[issue #97](https://git.eeqj.de/sneak/vaultik/issues/97)). Two defects
|
||||||
|
in `internal/log`, fixed together because both live in the handler
|
||||||
|
construction path. The first: both handlers were built over
|
||||||
|
`os.Stdout`, and `WARN`/`ERROR` are never suppressed, so a config file
|
||||||
|
with group- or world-readable permissions was enough to put a log
|
||||||
|
record inside a `--json` document and break `jq`. Diagnostics now go
|
||||||
|
to stderr, and the TTY/JSON format choice follows stderr rather than
|
||||||
|
stdout — testing the wrong stream would colorize records on a
|
||||||
|
redirected stderr whenever stdout happened to be a terminal. This is
|
||||||
|
user-visible: `--verbose` and `--debug` output moves to stderr too,
|
||||||
|
which is documented in `README.md` under "stdout and stderr". It also
|
||||||
|
let the local workaround in `internal/vaultik/snapshot_list.go` go:
|
||||||
|
`warnWhileListing` had been hand-rolling structured-log formatting to
|
||||||
|
reach a non-stdout writer, and the `jsonOutput` parameter threaded
|
||||||
|
through the remote-listing helpers existed only to choose between the
|
||||||
|
two writers. The collect-then-emit machinery around `listingWarning`
|
||||||
|
stays, but on its remaining merit — warnings emitted in key order
|
||||||
|
after `group.Wait()` are deterministic run to run, where emitting from
|
||||||
|
the fetch workers would order them by network timing. The second
|
||||||
|
defect: `TTYHandler.WithAttrs` and `WithGroup` discarded their
|
||||||
|
arguments and returned the receiver while their doc comments claimed
|
||||||
|
otherwise, so `log.With` attributes vanished on a terminal and
|
||||||
|
appeared correctly in CI — failing precisely when someone is debugging
|
||||||
|
interactively. Both now return a new handler (the receiver is never
|
||||||
|
written to, since `slog` permits concurrent derivation), attributes
|
||||||
|
persist across records, and grouping is implemented as dotted key
|
||||||
|
prefixes, which is the only honest rendering for a format with nowhere
|
||||||
|
to nest. New tests cover both, including one that feeds the same
|
||||||
|
derivation chain to the TTY and JSON handlers and compares the
|
||||||
|
attribute sets, so the two paths cannot drift apart again. Found and
|
||||||
|
filed while verifying: the startup banner is written to stdout and
|
||||||
|
`--json` does not suppress it
|
||||||
|
([issue #106](https://git.eeqj.de/sneak/vaultik/issues/106)), which is
|
||||||
|
a separate writer on a separate path and the remaining source of
|
||||||
|
stdout contamination.
|
||||||
|
|
||||||
- 2026-08-09: Made the tagged-release path actually work on Gitea
|
- 2026-08-09: Made the tagged-release path actually work on Gitea
|
||||||
([issue #65](https://git.eeqj.de/sneak/vaultik/issues/65)). Three
|
([issue #65](https://git.eeqj.de/sneak/vaultik/issues/65)). Three
|
||||||
independent blockers, one of which was the whole
|
independent blockers, one of which was the whole
|
||||||
|
|||||||
507
cmd/vaultik/lintdocker_test.go
Normal file
507
cmd/vaultik/lintdocker_test.go
Normal 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
|
||||||
|
}
|
||||||
|
}
|
||||||
151
cmd/vaultik/makefile_test.go
Normal file
151
cmd/vaultik/makefile_test.go
Normal file
@@ -0,0 +1,151 @@
|
|||||||
|
package main_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"regexp"
|
||||||
|
"slices"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
// This file guards the Makefile that builds this program, which is why
|
||||||
|
// it lives beside it rather than in a package of its own.
|
||||||
|
//
|
||||||
|
// Issue #110: `build` was listed in .PHONY with no `build:` rule
|
||||||
|
// anywhere in the file. That combination is silently successful — make
|
||||||
|
// considers a phony target with no prerequisites and no recipe already
|
||||||
|
// satisfied, so `rm -f vaultik && make build` printed "Nothing to be
|
||||||
|
// done for 'build'" and exited 0 with no binary produced. Declaring the
|
||||||
|
// name phony is precisely what converts the "No rule to make target"
|
||||||
|
// error into a green.
|
||||||
|
//
|
||||||
|
// The guard is a parse of the Makefile rather than an invocation of
|
||||||
|
// make. `make test` is what runs these tests, so shelling back into
|
||||||
|
// `make build` here would nest a build inside the test run and drop a
|
||||||
|
// binary into the tree as a side effect of testing. The one property a
|
||||||
|
// parse cannot establish — that the recipe still fails when the build
|
||||||
|
// fails — is not testable from inside the build either; it is verified
|
||||||
|
// by hand against a deliberately broken tree.
|
||||||
|
|
||||||
|
// phonyDirective introduces the list of phony target names.
|
||||||
|
const phonyDirective = ".PHONY:"
|
||||||
|
|
||||||
|
// ruleLine matches a rule's target list: a target starts in column
|
||||||
|
// zero, so recipe lines (tab-indented) and the continuation lines of a
|
||||||
|
// variable assignment (space-indented) are excluded by construction.
|
||||||
|
//
|
||||||
|
// The trailing (?:[^=]|$) rejects `:=` assignments such as
|
||||||
|
// `VERSION := $(shell script/version)`, which are not rules. Directives
|
||||||
|
// and function calls (`.PHONY:`, `ifeq`, `$(error ...)`) do not match
|
||||||
|
// because a target here must begin with a letter, digit or underscore.
|
||||||
|
var ruleLine = regexp.MustCompile(`^([A-Za-z0-9_][A-Za-z0-9_./ -]*):(?:[^=]|$)`)
|
||||||
|
|
||||||
|
// TestPhonyTargetsAllHaveRules fails on any name in .PHONY that has no
|
||||||
|
// rule in the Makefile. Such a name is not a build target at all: it is
|
||||||
|
// a command that reports success without doing anything, which is worse
|
||||||
|
// than one that does not exist, because a caller checking the exit code
|
||||||
|
// cannot tell the difference.
|
||||||
|
func TestPhonyTargetsAllHaveRules(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
makefile := readMakefile(t)
|
||||||
|
|
||||||
|
phony := phonyTargets(makefile)
|
||||||
|
require.NotEmpty(t, phony, "no .PHONY names found; the parser is broken")
|
||||||
|
|
||||||
|
rules := declaredRules(makefile)
|
||||||
|
|
||||||
|
// Sanity check on the rule parser before trusting its verdict: a
|
||||||
|
// parser that found nothing would pass this test by accident.
|
||||||
|
require.Contains(t, rules, "vaultik",
|
||||||
|
"the file rule that builds the binary must be recognized")
|
||||||
|
|
||||||
|
for _, target := range phony {
|
||||||
|
assert.Contains(t, rules, target,
|
||||||
|
"`.PHONY` lists %q but the Makefile declares no %q rule, so "+
|
||||||
|
"`make %s` exits 0 without doing anything", target, target, target)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestBuildTargetBuildsTheBinary pins the specific shape of issue #110:
|
||||||
|
// `make build` has to reach the rule that produces the binary. The test
|
||||||
|
// above would also pass if `build:` were given an empty recipe of its
|
||||||
|
// own, which would be the same silent success under a different
|
||||||
|
// spelling.
|
||||||
|
func TestBuildTargetBuildsTheBinary(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
prerequisites := rulePrerequisites(readMakefile(t), "build")
|
||||||
|
require.NotNil(t, prerequisites, "the Makefile declares no `build` rule")
|
||||||
|
|
||||||
|
assert.Contains(t, prerequisites, "vaultik",
|
||||||
|
"`make build` must depend on the rule that builds the binary")
|
||||||
|
}
|
||||||
|
|
||||||
|
// readMakefile returns the contents of the repository's Makefile. The
|
||||||
|
// root is located by the shared walk in lintdocker_test.go.
|
||||||
|
func readMakefile(t *testing.T) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
return readRepoFile(t, "Makefile")
|
||||||
|
}
|
||||||
|
|
||||||
|
// phonyTargets returns every name declared phony, across all .PHONY
|
||||||
|
// lines.
|
||||||
|
func phonyTargets(makefile string) []string {
|
||||||
|
var targets []string
|
||||||
|
|
||||||
|
for line := range strings.SplitSeq(makefile, "\n") {
|
||||||
|
if !strings.HasPrefix(line, phonyDirective) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
targets = append(targets,
|
||||||
|
strings.Fields(strings.TrimPrefix(line, phonyDirective))...)
|
||||||
|
}
|
||||||
|
|
||||||
|
return targets
|
||||||
|
}
|
||||||
|
|
||||||
|
// declaredRules returns the set of target names that have a rule.
|
||||||
|
func declaredRules(makefile string) map[string]bool {
|
||||||
|
rules := make(map[string]bool)
|
||||||
|
|
||||||
|
for line := range strings.SplitSeq(makefile, "\n") {
|
||||||
|
match := ruleLine.FindStringSubmatch(line)
|
||||||
|
if match == nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
// One rule may name several targets: `a b: prereq`.
|
||||||
|
for target := range strings.FieldsSeq(match[1]) {
|
||||||
|
rules[target] = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return rules
|
||||||
|
}
|
||||||
|
|
||||||
|
// rulePrerequisites returns the prerequisites of the named rule, or nil
|
||||||
|
// if no such rule exists. A rule with none returns an empty slice, so
|
||||||
|
// "declared with nothing to do" is distinguishable from "not declared".
|
||||||
|
func rulePrerequisites(makefile, target string) []string {
|
||||||
|
for line := range strings.SplitSeq(makefile, "\n") {
|
||||||
|
match := ruleLine.FindStringSubmatch(line)
|
||||||
|
if match == nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
if !slices.Contains(strings.Fields(match[1]), target) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
_, after, _ := strings.Cut(line, ":")
|
||||||
|
|
||||||
|
return append([]string{}, strings.Fields(after)...)
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
@@ -1,6 +1,7 @@
|
|||||||
package cli
|
package cli
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"io"
|
||||||
"os"
|
"os"
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
@@ -14,18 +15,12 @@ import (
|
|||||||
const shortCommitLen = 12
|
const shortCommitLen = 12
|
||||||
|
|
||||||
// Entry is the main entry point for the CLI application.
|
// Entry is the main entry point for the CLI application.
|
||||||
// It prints the startup banner (unless a quiet flag is present in os.Args),
|
// It prints the startup banner to stdout (unless a banner-suppressing
|
||||||
// executes the root cobra command, and routes any returned error through
|
// flag is present in os.Args — see bannerSuppressedInArgs), executes the
|
||||||
// the ui.Writer so the user sees a properly formatted "🛑 ERROR:" line.
|
// root cobra command, and routes any returned error through the
|
||||||
|
// ui.Writer so the user sees a properly formatted "🛑 ERROR:" line.
|
||||||
func Entry() {
|
func Entry() {
|
||||||
if !bannerSuppressedInArgs(os.Args[1:]) {
|
emitStartupBanner(os.Args[1:], os.Stdout)
|
||||||
short := globals.Commit
|
|
||||||
if len(short) > shortCommitLen {
|
|
||||||
short = short[:shortCommitLen]
|
|
||||||
}
|
|
||||||
|
|
||||||
writeStartupBanner(ui.New(os.Stdout), time.Now().UTC(), short)
|
|
||||||
}
|
|
||||||
|
|
||||||
rootCmd := NewRootCommand()
|
rootCmd := NewRootCommand()
|
||||||
rootCmd.SilenceErrors = true
|
rootCmd.SilenceErrors = true
|
||||||
@@ -37,6 +32,24 @@ func Entry() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// emitStartupBanner writes the startup banner to w unless args (the
|
||||||
|
// argument vector with the program name already stripped) contains a
|
||||||
|
// flag that suppresses it. Split out of Entry so that the decision — the
|
||||||
|
// only thing standing between a --json invocation and a parseable
|
||||||
|
// stdout — is reachable from a test without running the whole CLI.
|
||||||
|
func emitStartupBanner(args []string, w io.Writer) {
|
||||||
|
if bannerSuppressedInArgs(args) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
short := globals.Commit
|
||||||
|
if len(short) > shortCommitLen {
|
||||||
|
short = short[:shortCommitLen]
|
||||||
|
}
|
||||||
|
|
||||||
|
writeStartupBanner(ui.New(w), time.Now().UTC(), short)
|
||||||
|
}
|
||||||
|
|
||||||
// ReportErrorf emits a user-facing error to stderr in the standard
|
// ReportErrorf emits a user-facing error to stderr in the standard
|
||||||
// 🛑 ERROR: format. Use it from goroutine error paths (where returning
|
// 🛑 ERROR: format. Use it from goroutine error paths (where returning
|
||||||
// an error to cobra isn't an option) and anywhere else a CLI command
|
// an error to cobra isn't an option) and anywhere else a CLI command
|
||||||
@@ -46,9 +59,20 @@ func ReportErrorf(format string, args ...any) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// bannerSuppressedInArgs reports whether any of args is a flag that
|
// bannerSuppressedInArgs reports whether any of args is a flag that
|
||||||
// should suppress the startup banner (--quiet/-q/--cron). Stops at the
|
// should suppress the startup banner (--quiet/-q/--cron/--json). Stops
|
||||||
// "--" argument terminator. Recognizes both long forms and short -q,
|
// at the "--" argument terminator. Recognizes both long forms and short
|
||||||
// including combined short flags like "-qv".
|
// -q, including combined short flags like "-qv".
|
||||||
|
//
|
||||||
|
// This scans the raw argument vector because the banner is printed
|
||||||
|
// before cobra parses anything — deliberately, so that it still appears
|
||||||
|
// when cobra rejects the arguments and on --help. The consequence is
|
||||||
|
// that a flag is matched wherever it occurs in the vector, including
|
||||||
|
// positions where the command it belongs to would not accept it.
|
||||||
|
// --json is a subcommand flag rather than a persistent one, but so is
|
||||||
|
// --cron (it exists only on `snapshot create`), so this adds no new
|
||||||
|
// class of imprecision. The only cost of a false positive is a missing
|
||||||
|
// decorative banner; the cost of a false negative is a corrupt document
|
||||||
|
// on stdout, so the scan errs deliberately in that direction.
|
||||||
func bannerSuppressedInArgs(args []string) bool {
|
func bannerSuppressedInArgs(args []string) bool {
|
||||||
for _, a := range args {
|
for _, a := range args {
|
||||||
if a == "--" {
|
if a == "--" {
|
||||||
@@ -56,11 +80,13 @@ func bannerSuppressedInArgs(args []string) bool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
switch a {
|
switch a {
|
||||||
case "--quiet", "-q", "--cron":
|
case "--quiet", "-q", "--cron", "--json":
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
if strings.HasPrefix(a, "--quiet=") || strings.HasPrefix(a, "--cron=") {
|
if strings.HasPrefix(a, "--quiet=") ||
|
||||||
|
strings.HasPrefix(a, "--cron=") ||
|
||||||
|
strings.HasPrefix(a, "--json=") {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
// Combined short flags like -qv or -vq.
|
// Combined short flags like -qv or -vq.
|
||||||
|
|||||||
300
internal/cli/entry_banner_test.go
Normal file
300
internal/cli/entry_banner_test.go
Normal file
@@ -0,0 +1,300 @@
|
|||||||
|
package cli //nolint:testpackage // needs access to unexported emitStartupBanner
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"encoding/json"
|
||||||
|
"fmt"
|
||||||
|
"io"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/adrg/xdg"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Command words and flags used to build argument vectors below. They are
|
||||||
|
// constants rather than repeated literals so that a rename shows up as a
|
||||||
|
// compile error in one place.
|
||||||
|
const (
|
||||||
|
cmdSnapshot = "snapshot"
|
||||||
|
cmdList = "list"
|
||||||
|
cmdCreate = "create"
|
||||||
|
cmdVerify = "verify"
|
||||||
|
cmdRemove = "remove"
|
||||||
|
cmdPrune = "prune"
|
||||||
|
cmdRemote = "remote"
|
||||||
|
cmdInfo = "info"
|
||||||
|
|
||||||
|
flagJSON = "--json"
|
||||||
|
flagQuiet = "--quiet"
|
||||||
|
flagConfig = "--config"
|
||||||
|
|
||||||
|
// programName is argv[0] as the real process receives it. Entry
|
||||||
|
// strips it before scanning, so it has to be present.
|
||||||
|
programName = "vaultik"
|
||||||
|
|
||||||
|
// someSnapshotID is any snapshot identifier: these tests never run
|
||||||
|
// the command, so it only has to occupy the positional argument.
|
||||||
|
someSnapshotID = "host_2026-01-01T00:00:00Z"
|
||||||
|
)
|
||||||
|
|
||||||
|
// placeholderJSONDocument stands in for whatever document a --json
|
||||||
|
// command writes to stdout. `snapshot list --json` with no snapshots
|
||||||
|
// prints exactly this; the other --json commands print an object rather
|
||||||
|
// than an array, but this test is not about their shape. It is about
|
||||||
|
// what is on stdout *before* them, which is the same for all of them
|
||||||
|
// because Entry prints the banner before cobra has parsed anything and
|
||||||
|
// therefore before it can know which command is running.
|
||||||
|
const placeholderJSONDocument = "[]\n"
|
||||||
|
|
||||||
|
// jsonArgumentVectors are the argument vectors of every --json
|
||||||
|
// invocation the CLI accepts, with the program name stripped exactly as
|
||||||
|
// Entry strips it. Each one must leave stdout untouched by the banner.
|
||||||
|
//
|
||||||
|
//nolint:gochecknoglobals // read-only test fixture shared by two tests
|
||||||
|
var jsonArgumentVectors = map[string][]string{
|
||||||
|
"snapshot list": {cmdSnapshot, cmdList, flagJSON},
|
||||||
|
"snapshot verify": {cmdSnapshot, cmdVerify, someSnapshotID, flagJSON},
|
||||||
|
"snapshot remove": {cmdSnapshot, cmdRemove, someSnapshotID, flagJSON},
|
||||||
|
"prune": {cmdPrune, flagJSON},
|
||||||
|
"remote info": {cmdRemote, cmdInfo, flagJSON},
|
||||||
|
|
||||||
|
// --json before the subcommand, and with an explicit value: the
|
||||||
|
// scan is positional, so both forms have to be recognized.
|
||||||
|
"json first": {flagJSON, cmdSnapshot, cmdList},
|
||||||
|
"json with value": {cmdSnapshot, cmdList, flagJSON + "=true"},
|
||||||
|
|
||||||
|
// A --json invocation that also carries a flag with a value, so the
|
||||||
|
// scan cannot be fooled by an argument that consumes the next one.
|
||||||
|
"json with config": {
|
||||||
|
flagConfig, "/nonexistent/vaultik.yml", cmdSnapshot, cmdList, flagJSON,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestJSONInvocationStdoutIsExactlyOneDocument is the CLI-layer
|
||||||
|
// regression guard for issue #106: `vaultik snapshot list --json | jq`
|
||||||
|
// must work with no other flags.
|
||||||
|
//
|
||||||
|
// internal/vaultik's TestListSnapshots_JSONStdoutIsOnlyTheDocument
|
||||||
|
// guards the same contract one layer down, but it calls the library
|
||||||
|
// function directly and so cannot see Entry, which is where the
|
||||||
|
// contamination was: the startup banner is written to stdout before
|
||||||
|
// cobra parses anything, and the suppression scan did not know about
|
||||||
|
// --json. The two banner lines and the blank line landed ahead of the
|
||||||
|
// document and `jq` refused the result.
|
||||||
|
//
|
||||||
|
// The document is a constant here because this test is about the
|
||||||
|
// argument vectors, one per --json command; the one that runs a real
|
||||||
|
// command end to end is TestEntryJSONStdoutIsExactlyOneDocument below.
|
||||||
|
func TestJSONInvocationStdoutIsExactlyOneDocument(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
for name, argv := range jsonArgumentVectors {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var stdout bytes.Buffer
|
||||||
|
|
||||||
|
emitStartupBanner(argv, &stdout)
|
||||||
|
|
||||||
|
require.Empty(t, stdout.String(),
|
||||||
|
"nothing may reach stdout ahead of a --json document")
|
||||||
|
|
||||||
|
_, err := stdout.WriteString(placeholderJSONDocument)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
requireExactlyOneJSONDocument(t, stdout.String())
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestBannerStillPrintedWithoutSuppressingFlag pins the other half of
|
||||||
|
// the contract. Without it, deleting the banner outright would satisfy
|
||||||
|
// the test above, and the banner is wanted on interactive invocations.
|
||||||
|
func TestBannerStillPrintedWithoutSuppressingFlag(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
for name, argv := range map[string][]string{
|
||||||
|
"no flags": {cmdSnapshot, cmdList},
|
||||||
|
"verbose": {cmdSnapshot, cmdList, "--verbose"},
|
||||||
|
"after the terminator": {
|
||||||
|
cmdSnapshot, "restore", "--", flagJSON,
|
||||||
|
},
|
||||||
|
} {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var stdout bytes.Buffer
|
||||||
|
|
||||||
|
emitStartupBanner(argv, &stdout)
|
||||||
|
|
||||||
|
assert.Contains(t, stdout.String(), "starting up at",
|
||||||
|
"the banner belongs on invocations that did not opt out")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestBannerSuppressedInArgs covers the suppression scan directly,
|
||||||
|
// including the flags that suppressed the banner before --json joined
|
||||||
|
// them, so that adding --json cannot regress them.
|
||||||
|
func TestBannerSuppressedInArgs(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
for name, testCase := range map[string]struct {
|
||||||
|
args []string
|
||||||
|
suppressed bool
|
||||||
|
}{
|
||||||
|
"quiet long": {[]string{cmdSnapshot, cmdCreate, flagQuiet}, true},
|
||||||
|
"quiet short": {[]string{cmdSnapshot, cmdCreate, "-q"}, true},
|
||||||
|
"quiet combined": {[]string{cmdSnapshot, cmdCreate, "-qv"}, true},
|
||||||
|
"cron": {[]string{cmdSnapshot, cmdCreate, "--cron"}, true},
|
||||||
|
"json": {[]string{cmdSnapshot, cmdList, flagJSON}, true},
|
||||||
|
"nothing": {[]string{cmdSnapshot, cmdList}, false},
|
||||||
|
"empty": {nil, false},
|
||||||
|
"json after dashes": {
|
||||||
|
[]string{cmdSnapshot, cmdList, "--", flagJSON}, false,
|
||||||
|
},
|
||||||
|
"quiet after dashes": {
|
||||||
|
[]string{cmdSnapshot, cmdCreate, "--", "-q"}, false,
|
||||||
|
},
|
||||||
|
} {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
assert.Equal(t, testCase.suppressed,
|
||||||
|
bannerSuppressedInArgs(testCase.args))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// hermeticConfig is a complete, valid config that needs no network and
|
||||||
|
// no credentials: file:// storage is exempt from the S3 credential
|
||||||
|
// checks, and FileStorer over a directory that does not exist lists
|
||||||
|
// zero objects without erroring. Chunk, blob and compression settings
|
||||||
|
// are filled in by config.Load.
|
||||||
|
const hermeticConfig = `age_recipients:
|
||||||
|
- age1278m9q7dp3chsh2dcy82qk27v047zywyvtxwnj4cvt0z65jw6a7q5dqhfj
|
||||||
|
snapshots:
|
||||||
|
test:
|
||||||
|
paths:
|
||||||
|
- %s
|
||||||
|
storage_url: file://%s
|
||||||
|
index_path: %s
|
||||||
|
hostname: test-host
|
||||||
|
`
|
||||||
|
|
||||||
|
// TestEntryJSONStdoutIsExactlyOneDocument runs the real thing: Entry,
|
||||||
|
// with a real argument vector, over the process's real stdout file
|
||||||
|
// descriptor, all the way through cobra and the fx graph to the
|
||||||
|
// document. It is the assertion the issue asks for — `vaultik snapshot
|
||||||
|
// list --json | jq .` with no other flags — with the pipe replaced by a
|
||||||
|
// decoder.
|
||||||
|
//
|
||||||
|
// `snapshot list` is the command chosen because it is the only --json
|
||||||
|
// command that reaches its document without a populated destination
|
||||||
|
// store: it reads the local index, streams `metadata/` (empty here),
|
||||||
|
// and treats a barren destination as an empty list rather than a
|
||||||
|
// failure.
|
||||||
|
//
|
||||||
|
// Not parallel: it replaces os.Args, os.Stdout and the xdg globals.
|
||||||
|
func TestEntryJSONStdoutIsExactlyOneDocument(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
configPath := filepath.Join(dir, "config.yml")
|
||||||
|
|
||||||
|
contents := fmt.Sprintf(hermeticConfig,
|
||||||
|
filepath.Join(dir, "source"),
|
||||||
|
filepath.Join(dir, "store"),
|
||||||
|
filepath.Join(dir, "index.sqlite"))
|
||||||
|
|
||||||
|
require.NoError(t,
|
||||||
|
os.WriteFile(configPath, []byte(contents), configFileMode))
|
||||||
|
|
||||||
|
// The PID lock lives under xdg.DataHome, which xdg resolves at
|
||||||
|
// package init; point it at the temp dir so the test neither
|
||||||
|
// touches nor collides with the real one.
|
||||||
|
t.Setenv("XDG_DATA_HOME", filepath.Join(dir, "data"))
|
||||||
|
xdg.Reload()
|
||||||
|
t.Cleanup(xdg.Reload)
|
||||||
|
|
||||||
|
previousArgs := os.Args
|
||||||
|
|
||||||
|
t.Cleanup(func() {
|
||||||
|
os.Args = previousArgs
|
||||||
|
rootFlags = RootFlags{}
|
||||||
|
})
|
||||||
|
|
||||||
|
os.Args = []string{
|
||||||
|
programName, flagConfig, configPath, cmdSnapshot, cmdList, flagJSON,
|
||||||
|
}
|
||||||
|
|
||||||
|
stdout := captureProcessStdout(t, Entry)
|
||||||
|
|
||||||
|
requireExactlyOneJSONDocument(t, stdout)
|
||||||
|
|
||||||
|
var snapshots []any
|
||||||
|
|
||||||
|
require.NoError(t, json.Unmarshal([]byte(stdout), &snapshots))
|
||||||
|
assert.Empty(t, snapshots,
|
||||||
|
"a destination store with no snapshots lists none")
|
||||||
|
}
|
||||||
|
|
||||||
|
// captureProcessStdout redirects the process's own stdout to a pipe for
|
||||||
|
// the duration of fn and returns what was written to it. The redirection
|
||||||
|
// has to be at the file-descriptor level rather than through an injected
|
||||||
|
// writer, because the banner and the JSON encoder reach os.Stdout
|
||||||
|
// independently and the point of the test is that both land in the same
|
||||||
|
// place.
|
||||||
|
//
|
||||||
|
// Not parallel-safe: os.Stdout is process-global.
|
||||||
|
func captureProcessStdout(t *testing.T, fn func()) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
reader, writer, err := os.Pipe()
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
previous := os.Stdout
|
||||||
|
os.Stdout = writer
|
||||||
|
|
||||||
|
captured := make(chan string, 1)
|
||||||
|
|
||||||
|
go func() {
|
||||||
|
var buf bytes.Buffer
|
||||||
|
|
||||||
|
_, _ = io.Copy(&buf, reader)
|
||||||
|
captured <- buf.String()
|
||||||
|
}()
|
||||||
|
|
||||||
|
fn()
|
||||||
|
|
||||||
|
os.Stdout = previous
|
||||||
|
|
||||||
|
require.NoError(t, writer.Close())
|
||||||
|
|
||||||
|
out := <-captured
|
||||||
|
|
||||||
|
require.NoError(t, reader.Close())
|
||||||
|
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
// requireExactlyOneJSONDocument fails unless stdout decodes as a single
|
||||||
|
// JSON value with nothing before or after it — the property that makes
|
||||||
|
// `| jq` work.
|
||||||
|
func requireExactlyOneJSONDocument(t *testing.T, stdout string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
decoder := json.NewDecoder(strings.NewReader(stdout))
|
||||||
|
|
||||||
|
var document any
|
||||||
|
|
||||||
|
err := decoder.Decode(&document)
|
||||||
|
require.NoError(t, err,
|
||||||
|
"stdout must parse as JSON, got:\n%s", stdout)
|
||||||
|
|
||||||
|
_, err = decoder.Token()
|
||||||
|
require.ErrorIs(t, err, io.EOF,
|
||||||
|
"stdout must hold exactly one JSON document, got:\n%s", stdout)
|
||||||
|
}
|
||||||
165
internal/cli/entry_prune_json_test.go
Normal file
165
internal/cli/entry_prune_json_test.go
Normal file
@@ -0,0 +1,165 @@
|
|||||||
|
package cli //nolint:testpackage // shares hermeticConfig and the capture helpers
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"database/sql"
|
||||||
|
"encoding/json"
|
||||||
|
"fmt"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/adrg/xdg"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/vaultik/internal/database"
|
||||||
|
"sneak.berlin/go/vaultik/internal/types"
|
||||||
|
)
|
||||||
|
|
||||||
|
// pruneJSONDocument is the shape `prune --json` writes: the
|
||||||
|
// PruneBlobsResult document, and nothing else.
|
||||||
|
//
|
||||||
|
//nolint:tagliatelle // snake_case is the established JSON output format
|
||||||
|
type pruneJSONDocument struct {
|
||||||
|
BlobsFound int `json:"blobs_found"`
|
||||||
|
BlobsDeleted int `json:"blobs_deleted"`
|
||||||
|
BytesFreed int64 `json:"bytes_freed"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// stalePruneSnapshotID is seeded into the local index with no manifest
|
||||||
|
// on the destination store, which is exactly what makes it stale.
|
||||||
|
const stalePruneSnapshotID = "test-host_test_2026-04-01T09:00:00Z"
|
||||||
|
|
||||||
|
// TestEntryPruneJSONStdoutIsExactlyOneDocument is the end-to-end
|
||||||
|
// regression guard for issue #108: `vaultik prune --json | jq .` must
|
||||||
|
// work with no other flags.
|
||||||
|
//
|
||||||
|
// It runs Entry over the process's real stdout descriptor, through
|
||||||
|
// cobra and the fx graph, against a hermetic file:// destination store
|
||||||
|
// — the same construction TestEntryJSONStdoutIsExactlyOneDocument uses
|
||||||
|
// for `snapshot list`, with the pipe to jq replaced by a decoder.
|
||||||
|
//
|
||||||
|
// Both branches of the local-snapshot reconciliation are exercised
|
||||||
|
// because the three stdout writes that broke this covered all of them:
|
||||||
|
// one line per stale record and a summary when there were any, and a
|
||||||
|
// "No stale local snapshots found." line when there were none. No input
|
||||||
|
// avoided the contamination, so no single branch demonstrates the fix.
|
||||||
|
//
|
||||||
|
// Not parallel: it replaces os.Args, os.Stdout and the xdg globals.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // replaces os.Args, os.Stdout and the xdg globals
|
||||||
|
func TestEntryPruneJSONStdoutIsExactlyOneDocument(t *testing.T) {
|
||||||
|
for _, testCase := range []struct {
|
||||||
|
name string
|
||||||
|
seedStale bool
|
||||||
|
description string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "no stale local records",
|
||||||
|
seedStale: false,
|
||||||
|
description: "the empty-index branch used to print a 'No stale' line",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "stale local records present",
|
||||||
|
seedStale: true,
|
||||||
|
description: "the removal branch used to print a line per record " +
|
||||||
|
"plus a summary",
|
||||||
|
},
|
||||||
|
} {
|
||||||
|
t.Run(testCase.name, func(t *testing.T) {
|
||||||
|
configPath := writeHermeticPruneConfig(t, testCase.seedStale)
|
||||||
|
|
||||||
|
previousArgs := os.Args
|
||||||
|
|
||||||
|
t.Cleanup(func() {
|
||||||
|
os.Args = previousArgs
|
||||||
|
rootFlags = RootFlags{}
|
||||||
|
})
|
||||||
|
|
||||||
|
os.Args = []string{
|
||||||
|
programName, flagConfig, configPath, cmdPrune, flagJSON,
|
||||||
|
}
|
||||||
|
|
||||||
|
stdout := captureProcessStdout(t, Entry)
|
||||||
|
|
||||||
|
requireExactlyOneJSONDocument(t, stdout)
|
||||||
|
|
||||||
|
var document pruneJSONDocument
|
||||||
|
|
||||||
|
require.NoError(t, json.Unmarshal([]byte(stdout), &document),
|
||||||
|
testCase.description)
|
||||||
|
|
||||||
|
// A destination store with no blobs has none to prune. The
|
||||||
|
// assertion that matters is the one above; this one keeps the
|
||||||
|
// test honest about which document it decoded.
|
||||||
|
assert.Equal(t, 0, document.BlobsFound)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// writeHermeticPruneConfig builds a config over a temp directory and, if
|
||||||
|
// seedStale is set, creates the index database up front with one
|
||||||
|
// snapshot record that has no counterpart on the destination store.
|
||||||
|
// Returns the config path.
|
||||||
|
func writeHermeticPruneConfig(t *testing.T, seedStale bool) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
configPath := filepath.Join(dir, "config.yml")
|
||||||
|
indexPath := filepath.Join(dir, "index.sqlite")
|
||||||
|
|
||||||
|
contents := fmt.Sprintf(hermeticConfig,
|
||||||
|
filepath.Join(dir, "source"),
|
||||||
|
filepath.Join(dir, "store"),
|
||||||
|
indexPath)
|
||||||
|
|
||||||
|
require.NoError(t,
|
||||||
|
os.WriteFile(configPath, []byte(contents), configFileMode))
|
||||||
|
|
||||||
|
// The PID lock lives under xdg.DataHome, which xdg resolves at
|
||||||
|
// package init; point it at the temp dir so the test neither
|
||||||
|
// touches nor collides with the real one.
|
||||||
|
t.Setenv("XDG_DATA_HOME", filepath.Join(dir, "data"))
|
||||||
|
xdg.Reload()
|
||||||
|
t.Cleanup(xdg.Reload)
|
||||||
|
|
||||||
|
if seedStale {
|
||||||
|
seedStaleSnapshotRecord(t, indexPath)
|
||||||
|
}
|
||||||
|
|
||||||
|
return configPath
|
||||||
|
}
|
||||||
|
|
||||||
|
// seedStaleSnapshotRecord creates the index database at path and
|
||||||
|
// inserts one completed snapshot into it. Nothing is written to the
|
||||||
|
// destination store, so `prune` finds the record stale and removes it —
|
||||||
|
// the branch that printed a line per record.
|
||||||
|
func seedStaleSnapshotRecord(t *testing.T, path string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
db, err := database.New(ctx, path)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
defer func() { require.NoError(t, db.Close()) }()
|
||||||
|
|
||||||
|
startedAt := time.Date(2026, 4, 1, 9, 0, 0, 0, time.UTC)
|
||||||
|
completedAt := startedAt.Add(time.Minute)
|
||||||
|
|
||||||
|
snap := &database.Snapshot{
|
||||||
|
ID: types.SnapshotID(stalePruneSnapshotID),
|
||||||
|
Hostname: "test-host",
|
||||||
|
VaultikVersion: "test",
|
||||||
|
StartedAt: startedAt,
|
||||||
|
CompletedAt: &completedAt,
|
||||||
|
}
|
||||||
|
|
||||||
|
repos := database.NewRepositories(db)
|
||||||
|
|
||||||
|
err = repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
|
||||||
|
return repos.Snapshots.Create(ctx, tx, snap)
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
}
|
||||||
@@ -63,8 +63,15 @@ func New() (*Globals, error) {
|
|||||||
// a release. Both "dev" and "dev-<sha>" (and its "-dirty" variant)
|
// a release. Both "dev" and "dev-<sha>" (and its "-dirty" variant)
|
||||||
// count: a caller that compares against "dev" exactly would treat every
|
// count: a caller that compares against "dev" exactly would treat every
|
||||||
// commit-stamped development build as a release.
|
// commit-stamped development build as a release.
|
||||||
|
//
|
||||||
|
// The empty string counts too. Nothing that knows its version reports
|
||||||
|
// no version, so an empty Version means the stamping failed, and the
|
||||||
|
// safe reading of "we could not establish that this is a release" is
|
||||||
|
// that it is not one. The Makefile refuses to build at all in that
|
||||||
|
// case; this is the second line of defence, for a binary linked by
|
||||||
|
// something other than the Makefile.
|
||||||
func IsDevVersion(v string) bool {
|
func IsDevVersion(v string) bool {
|
||||||
return v == DevVersion || strings.HasPrefix(v, DevVersion+"-")
|
return v == "" || v == DevVersion || strings.HasPrefix(v, DevVersion+"-")
|
||||||
}
|
}
|
||||||
|
|
||||||
// shortCommitLen is the number of commit-hash characters ShortCommit keeps.
|
// shortCommitLen is the number of commit-hash characters ShortCommit keeps.
|
||||||
|
|||||||
@@ -59,7 +59,11 @@ func TestIsDevVersion(t *testing.T) {
|
|||||||
// the string happens to contain "dev".
|
// the string happens to contain "dev".
|
||||||
{"1.0.0-dev", false},
|
{"1.0.0-dev", false},
|
||||||
{"developer", false},
|
{"developer", false},
|
||||||
{"", false},
|
// A binary with no version string at all did not get stamped,
|
||||||
|
// which is a build failure, not a release. It must never print
|
||||||
|
// as one. The Makefile refuses to build when script/version
|
||||||
|
// yields nothing; this covers a binary linked some other way.
|
||||||
|
{"", true},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tc := range cases {
|
for _, tc := range cases {
|
||||||
|
|||||||
@@ -1,5 +1,9 @@
|
|||||||
// Package log provides the application-wide structured logger: slog
|
// Package log provides the application-wide structured logger: slog
|
||||||
// with a colorized TTY handler on terminals and JSON output otherwise.
|
// writing to stderr, with a colorized TTY handler when stderr is a
|
||||||
|
// terminal and JSON output otherwise.
|
||||||
|
//
|
||||||
|
// Everything this package emits is a diagnostic, so it all goes to
|
||||||
|
// stderr. stdout belongs to the output the user asked for.
|
||||||
package log //nolint:revive,nolintlint // stdlib log unused here; see #76
|
package log //nolint:revive,nolintlint // stdlib log unused here; see #76
|
||||||
|
|
||||||
import (
|
import (
|
||||||
@@ -69,13 +73,27 @@ func Initialize(cfg Config) {
|
|||||||
Level: level,
|
Level: level,
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if stdout is a TTY.
|
// Diagnostics go to stderr, never to stdout. stdout is reserved for
|
||||||
if term.IsTerminal(int(os.Stdout.Fd())) {
|
// the output the user asked for: every --json subcommand writes its
|
||||||
|
// document there, and WARN/ERROR are never suppressed, so a logger
|
||||||
|
// on stdout puts log records inside that document and makes it
|
||||||
|
// unparseable. A config file with group- or world-readable
|
||||||
|
// permissions is enough to trigger it (see internal/config), so this
|
||||||
|
// was not a theoretical collision.
|
||||||
|
//
|
||||||
|
// The format is chosen by the TTY-ness of the stream the records
|
||||||
|
// actually land on. AGENTS.md policy 9 says "if stdout is not a
|
||||||
|
// terminal, emit jsonl"; it says stdout because that is where logs
|
||||||
|
// used to go, and the property it is really asking for is that
|
||||||
|
// output nobody is watching be machine-readable. Testing stdout here
|
||||||
|
// would colorize records on a redirected stderr whenever stdout
|
||||||
|
// happened to be a terminal, and vice versa.
|
||||||
|
if term.IsTerminal(int(os.Stderr.Fd())) {
|
||||||
// Use colorized TTY handler
|
// Use colorized TTY handler
|
||||||
logger = slog.New(NewTTYHandler(os.Stdout, opts))
|
logger = slog.New(NewTTYHandler(os.Stderr, opts))
|
||||||
} else {
|
} else {
|
||||||
// Use JSON format for non-TTY output
|
// Use JSON format for non-TTY output
|
||||||
logger = slog.New(slog.NewJSONHandler(os.Stdout, opts))
|
logger = slog.New(slog.NewJSONHandler(os.Stderr, opts))
|
||||||
}
|
}
|
||||||
|
|
||||||
// Set as default logger
|
// Set as default logger
|
||||||
|
|||||||
@@ -5,10 +5,34 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// groupSeparator joins an open group path to an attribute key. This
|
||||||
|
// format has no nesting, so a group becomes a dotted key prefix:
|
||||||
|
// slog.New(h).WithGroup("db").With("rows", 3) renders "db.rows=3".
|
||||||
|
const groupSeparator = "."
|
||||||
|
|
||||||
|
// bytesAttrKey is the attribute key whose int64 value is rendered as a
|
||||||
|
// human-readable byte count rather than a bare number. Keys reaching
|
||||||
|
// writeAttr are group-qualified, so the match is made against the final
|
||||||
|
// dot-separated segment: without that, a "bytes" attribute logged under
|
||||||
|
// an open group would arrive as "transfer.bytes" and silently lose its
|
||||||
|
// formatting.
|
||||||
|
const bytesAttrKey = "bytes"
|
||||||
|
|
||||||
|
// isBytesAttr reports whether a group-qualified attribute key names the
|
||||||
|
// byte-count attribute, i.e. whether its last segment is bytesAttrKey.
|
||||||
|
func isBytesAttr(key string) bool {
|
||||||
|
if idx := strings.LastIndex(key, groupSeparator); idx >= 0 {
|
||||||
|
key = key[idx+len(groupSeparator):]
|
||||||
|
}
|
||||||
|
|
||||||
|
return key == bytesAttrKey
|
||||||
|
}
|
||||||
|
|
||||||
// ANSI color codes
|
// ANSI color codes
|
||||||
const (
|
const (
|
||||||
colorReset = "\033[0m"
|
colorReset = "\033[0m"
|
||||||
@@ -22,10 +46,26 @@ const (
|
|||||||
)
|
)
|
||||||
|
|
||||||
// TTYHandler is a custom slog handler for TTY output with colors.
|
// TTYHandler is a custom slog handler for TTY output with colors.
|
||||||
|
//
|
||||||
|
// A handler and the handlers derived from it via WithAttrs/WithGroup
|
||||||
|
// all write to the same stream, so they share one mutex; that is why mu
|
||||||
|
// is a pointer. A value mutex would give every derived handler its own
|
||||||
|
// lock and stop serializing writes to the stream they have in common.
|
||||||
type TTYHandler struct {
|
type TTYHandler struct {
|
||||||
opts slog.HandlerOptions
|
opts slog.HandlerOptions
|
||||||
mu sync.Mutex
|
mu *sync.Mutex
|
||||||
out io.Writer
|
out io.Writer
|
||||||
|
|
||||||
|
// attrs are the attributes accumulated through WithAttrs, emitted
|
||||||
|
// ahead of each record's own attributes. Their keys already carry
|
||||||
|
// the group path that was open when they were added, so no
|
||||||
|
// qualification happens at write time.
|
||||||
|
attrs []slog.Attr
|
||||||
|
|
||||||
|
// groups is the group path opened by WithGroup, applied as a key
|
||||||
|
// prefix to attributes that arrive later — both on a record and
|
||||||
|
// through a further WithAttrs.
|
||||||
|
groups []string
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewTTYHandler creates a new TTY handler with colored output.
|
// NewTTYHandler creates a new TTY handler with colored output.
|
||||||
@@ -37,6 +77,7 @@ func NewTTYHandler(out io.Writer, opts *slog.HandlerOptions) *TTYHandler {
|
|||||||
return &TTYHandler{
|
return &TTYHandler{
|
||||||
out: out,
|
out: out,
|
||||||
opts: *opts,
|
opts: *opts,
|
||||||
|
mu: &sync.Mutex{},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -81,29 +122,19 @@ func (h *TTYHandler) Handle(_ context.Context, r slog.Record) error {
|
|||||||
levelColor, level, colorReset,
|
levelColor, level, colorReset,
|
||||||
colorBold, r.Message, colorReset)
|
colorBold, r.Message, colorReset)
|
||||||
|
|
||||||
// Print attributes
|
// Attributes carried by the handler come first, then the record's
|
||||||
r.Attrs(func(a slog.Attr) bool {
|
// own. Handler attributes were qualified when they were added; the
|
||||||
value := a.Value.String()
|
// record's are qualified now, against whatever group path is open.
|
||||||
// Special handling for certain attribute types
|
for _, a := range h.attrs {
|
||||||
switch a.Value.Kind() {
|
h.writeAttr(a)
|
||||||
case slog.KindDuration:
|
}
|
||||||
if d, ok := a.Value.Any().(time.Duration); ok {
|
|
||||||
value = formatDuration(d)
|
|
||||||
}
|
|
||||||
case slog.KindInt64:
|
|
||||||
if a.Key == "bytes" {
|
|
||||||
value = formatBytes(a.Value.Int64())
|
|
||||||
}
|
|
||||||
case slog.KindAny, slog.KindBool, slog.KindFloat64, slog.KindString,
|
|
||||||
slog.KindTime, slog.KindUint64, slog.KindGroup, slog.KindLogValuer:
|
|
||||||
// Plain string form above is already correct for these kinds.
|
|
||||||
default:
|
|
||||||
// Future kinds also use the plain string form.
|
|
||||||
}
|
|
||||||
|
|
||||||
_, _ = fmt.Fprintf(h.out, " %s%s%s=%s%s%s",
|
prefix := strings.Join(h.groups, groupSeparator)
|
||||||
colorCyan, a.Key, colorReset,
|
|
||||||
colorBlue, value, colorReset)
|
r.Attrs(func(a slog.Attr) bool {
|
||||||
|
for _, flat := range appendAttr(nil, prefix, a) {
|
||||||
|
h.writeAttr(flat)
|
||||||
|
}
|
||||||
|
|
||||||
return true
|
return true
|
||||||
})
|
})
|
||||||
@@ -113,14 +144,125 @@ func (h *TTYHandler) Handle(_ context.Context, r slog.Record) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// WithAttrs returns a new handler with the given attributes.
|
// appendAttr flattens a into dst, folding prefix into its key and
|
||||||
func (h *TTYHandler) WithAttrs(_ []slog.Attr) slog.Handler {
|
// expanding group values into further dotted keys. Following the
|
||||||
return h // Simplified for now
|
// slog.Handler contract: an empty Attr is dropped, a group with no
|
||||||
|
// attributes is dropped, and a group with an empty key is inlined into
|
||||||
|
// its parent rather than contributing a level.
|
||||||
|
func appendAttr(dst []slog.Attr, prefix string, a slog.Attr) []slog.Attr {
|
||||||
|
a.Value = a.Value.Resolve()
|
||||||
|
|
||||||
|
if a.Equal(slog.Attr{}) {
|
||||||
|
return dst
|
||||||
|
}
|
||||||
|
|
||||||
|
key := a.Key
|
||||||
|
|
||||||
|
switch {
|
||||||
|
case prefix == "":
|
||||||
|
// key stands alone.
|
||||||
|
case key == "":
|
||||||
|
key = prefix
|
||||||
|
default:
|
||||||
|
key = prefix + groupSeparator + key
|
||||||
|
}
|
||||||
|
|
||||||
|
if a.Value.Kind() != slog.KindGroup {
|
||||||
|
return append(dst, slog.Attr{Key: key, Value: a.Value})
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, member := range a.Value.Group() {
|
||||||
|
dst = appendAttr(dst, key, member)
|
||||||
|
}
|
||||||
|
|
||||||
|
return dst
|
||||||
}
|
}
|
||||||
|
|
||||||
// WithGroup returns a new handler with the given group name.
|
// WithAttrs returns a new handler that emits attrs on every record it
|
||||||
func (h *TTYHandler) WithGroup(_ string) slog.Handler {
|
// handles, in addition to whatever the handler already carried. Keys
|
||||||
return h // Simplified for now
|
// are qualified by the group path open at the time of the call, so
|
||||||
|
// WithGroup("db").WithAttrs(rows=3) later renders "db.rows=3".
|
||||||
|
//
|
||||||
|
// The receiver is not modified.
|
||||||
|
func (h *TTYHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
|
||||||
|
if len(attrs) == 0 {
|
||||||
|
return h
|
||||||
|
}
|
||||||
|
|
||||||
|
prefix := strings.Join(h.groups, groupSeparator)
|
||||||
|
next := h.clone()
|
||||||
|
|
||||||
|
for _, a := range attrs {
|
||||||
|
next.attrs = appendAttr(next.attrs, prefix, a)
|
||||||
|
}
|
||||||
|
|
||||||
|
return next
|
||||||
|
}
|
||||||
|
|
||||||
|
// WithGroup returns a new handler that qualifies every subsequent
|
||||||
|
// attribute key with name. This format is a single line with nowhere to
|
||||||
|
// nest, so grouping is rendered as a dotted key prefix: after
|
||||||
|
// WithGroup("db"), an attribute "rows" is emitted as "db.rows".
|
||||||
|
//
|
||||||
|
// An empty name returns the receiver unchanged, per the slog.Handler
|
||||||
|
// contract. The receiver is not modified.
|
||||||
|
func (h *TTYHandler) WithGroup(name string) slog.Handler {
|
||||||
|
if name == "" {
|
||||||
|
return h
|
||||||
|
}
|
||||||
|
|
||||||
|
next := h.clone()
|
||||||
|
next.groups = append(next.groups, name)
|
||||||
|
|
||||||
|
return next
|
||||||
|
}
|
||||||
|
|
||||||
|
// clone returns a copy of h that shares its output stream and mutex but
|
||||||
|
// owns its attribute and group slices.
|
||||||
|
//
|
||||||
|
// The slices are copied rather than resliced on purpose. slog permits
|
||||||
|
// one handler to be derived from concurrently, and two derivations that
|
||||||
|
// appended into a shared backing array would each overwrite the other's
|
||||||
|
// attribute — a data race with a silent wrong-output failure mode.
|
||||||
|
func (h *TTYHandler) clone() *TTYHandler {
|
||||||
|
next := &TTYHandler{
|
||||||
|
opts: h.opts,
|
||||||
|
mu: h.mu,
|
||||||
|
out: h.out,
|
||||||
|
attrs: make([]slog.Attr, len(h.attrs), len(h.attrs)+1),
|
||||||
|
groups: make([]string, len(h.groups), len(h.groups)+1),
|
||||||
|
}
|
||||||
|
|
||||||
|
copy(next.attrs, h.attrs)
|
||||||
|
copy(next.groups, h.groups)
|
||||||
|
|
||||||
|
return next
|
||||||
|
}
|
||||||
|
|
||||||
|
// writeAttr renders one already-flattened, already-qualified attribute
|
||||||
|
// as " key=value". Callers hold h.mu.
|
||||||
|
func (h *TTYHandler) writeAttr(a slog.Attr) {
|
||||||
|
value := a.Value.String()
|
||||||
|
// Special handling for certain attribute types
|
||||||
|
switch a.Value.Kind() {
|
||||||
|
case slog.KindDuration:
|
||||||
|
if d, ok := a.Value.Any().(time.Duration); ok {
|
||||||
|
value = formatDuration(d)
|
||||||
|
}
|
||||||
|
case slog.KindInt64:
|
||||||
|
if isBytesAttr(a.Key) {
|
||||||
|
value = formatBytes(a.Value.Int64())
|
||||||
|
}
|
||||||
|
case slog.KindAny, slog.KindBool, slog.KindFloat64, slog.KindString,
|
||||||
|
slog.KindTime, slog.KindUint64, slog.KindGroup, slog.KindLogValuer:
|
||||||
|
// Plain string form above is already correct for these kinds.
|
||||||
|
default:
|
||||||
|
// Future kinds also use the plain string form.
|
||||||
|
}
|
||||||
|
|
||||||
|
_, _ = fmt.Fprintf(h.out, " %s%s%s=%s%s%s",
|
||||||
|
colorCyan, a.Key, colorReset,
|
||||||
|
colorBlue, value, colorReset)
|
||||||
}
|
}
|
||||||
|
|
||||||
// formatDuration formats a duration in a human-readable way
|
// formatDuration formats a duration in a human-readable way
|
||||||
|
|||||||
422
internal/log/tty_handler_test.go
Normal file
422
internal/log/tty_handler_test.go
Normal file
@@ -0,0 +1,422 @@
|
|||||||
|
package log_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"context"
|
||||||
|
"encoding/json"
|
||||||
|
"fmt"
|
||||||
|
"log/slog"
|
||||||
|
"math"
|
||||||
|
"regexp"
|
||||||
|
"sort"
|
||||||
|
"strconv"
|
||||||
|
"strings"
|
||||||
|
"sync"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/vaultik/internal/log"
|
||||||
|
)
|
||||||
|
|
||||||
|
// ansiEscape matches the SGR sequences TTYHandler wraps every field in.
|
||||||
|
// Stripping them is what lets a test compare TTYHandler's rendering with
|
||||||
|
// slog.JSONHandler's.
|
||||||
|
var ansiEscape = regexp.MustCompile(`\x1b\[[0-9;]*m`)
|
||||||
|
|
||||||
|
// countKey is an attribute key reused across the comparison cases.
|
||||||
|
const countKey = "count"
|
||||||
|
|
||||||
|
// debugHandlerOptions enables every level, so a test never has to reason
|
||||||
|
// about the default level while reasoning about attributes.
|
||||||
|
func debugHandlerOptions() *slog.HandlerOptions {
|
||||||
|
return &slog.HandlerOptions{Level: slog.LevelDebug}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ttyAttrs renders one record through a TTYHandler and returns its
|
||||||
|
// attributes as key -> value, with color stripped.
|
||||||
|
//
|
||||||
|
// TTYHandler emits " key=value" per attribute after the message, and the
|
||||||
|
// message itself is the last thing before the first attribute, so
|
||||||
|
// splitting on spaces and keeping the tokens containing "=" recovers the
|
||||||
|
// attribute set. Test values below therefore avoid spaces and "=".
|
||||||
|
func ttyAttrs(t *testing.T, derive func(*slog.Logger) *slog.Logger,
|
||||||
|
msg string, args ...any,
|
||||||
|
) map[string]string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
var buf bytes.Buffer
|
||||||
|
|
||||||
|
logger := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions()))
|
||||||
|
derive(logger).Info(msg, args...)
|
||||||
|
|
||||||
|
line := ansiEscape.ReplaceAllString(buf.String(), "")
|
||||||
|
attrs := make(map[string]string)
|
||||||
|
|
||||||
|
for token := range strings.FieldsSeq(line) {
|
||||||
|
key, value, found := strings.Cut(token, "=")
|
||||||
|
if !found {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
attrs[key] = value
|
||||||
|
}
|
||||||
|
|
||||||
|
return attrs
|
||||||
|
}
|
||||||
|
|
||||||
|
// jsonAttrs renders one record through slog.JSONHandler and returns its
|
||||||
|
// attributes flattened to the same dotted-key form TTYHandler uses, so
|
||||||
|
// the two are directly comparable. The built-in time/level/msg fields
|
||||||
|
// are dropped: they are the record, not its attributes.
|
||||||
|
func jsonAttrs(t *testing.T, derive func(*slog.Logger) *slog.Logger,
|
||||||
|
msg string, args ...any,
|
||||||
|
) map[string]string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
var buf bytes.Buffer
|
||||||
|
|
||||||
|
logger := slog.New(slog.NewJSONHandler(&buf, debugHandlerOptions()))
|
||||||
|
derive(logger).Info(msg, args...)
|
||||||
|
|
||||||
|
var decoded map[string]any
|
||||||
|
|
||||||
|
require.NoError(t, json.Unmarshal(buf.Bytes(), &decoded))
|
||||||
|
|
||||||
|
delete(decoded, slog.TimeKey)
|
||||||
|
delete(decoded, slog.LevelKey)
|
||||||
|
delete(decoded, slog.MessageKey)
|
||||||
|
|
||||||
|
attrs := make(map[string]string)
|
||||||
|
flattenJSON(attrs, "", decoded)
|
||||||
|
|
||||||
|
return attrs
|
||||||
|
}
|
||||||
|
|
||||||
|
// flattenJSON turns JSONHandler's nested group objects into the dotted
|
||||||
|
// keys TTYHandler writes.
|
||||||
|
func flattenJSON(dst map[string]string, prefix string, src map[string]any) {
|
||||||
|
for key, value := range src {
|
||||||
|
full := key
|
||||||
|
if prefix != "" {
|
||||||
|
full = prefix + "." + key
|
||||||
|
}
|
||||||
|
|
||||||
|
nested, ok := value.(map[string]any)
|
||||||
|
if ok {
|
||||||
|
flattenJSON(dst, full, nested)
|
||||||
|
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
dst[full] = valueString(value)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// valueString renders a decoded JSON scalar the way slog.Value.String
|
||||||
|
// renders the corresponding Go value, so the two handlers' outputs can
|
||||||
|
// be compared as strings. encoding/json decodes every number as
|
||||||
|
// float64, so an integral one is rendered back as an integer — which is
|
||||||
|
// what the Go value that produced it was.
|
||||||
|
func valueString(v any) string {
|
||||||
|
switch typed := v.(type) {
|
||||||
|
case string:
|
||||||
|
return typed
|
||||||
|
case bool:
|
||||||
|
return strconv.FormatBool(typed)
|
||||||
|
case float64:
|
||||||
|
if typed == math.Trunc(typed) {
|
||||||
|
return strconv.FormatInt(int64(typed), 10)
|
||||||
|
}
|
||||||
|
|
||||||
|
return strconv.FormatFloat(typed, 'g', -1, 64)
|
||||||
|
default:
|
||||||
|
return fmt.Sprint(v)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTTYHandlerWithAttrsEmitsAttributes is the direct regression test
|
||||||
|
// for the reported defect: WithAttrs discarded its argument, so an
|
||||||
|
// attribute attached to a logger never reached the output.
|
||||||
|
func TestTTYHandlerWithAttrsEmitsAttributes(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
attrs := ttyAttrs(t, func(l *slog.Logger) *slog.Logger {
|
||||||
|
return l.With("key", "value")
|
||||||
|
}, "hello")
|
||||||
|
|
||||||
|
assert.Equal(t, "value", attrs["key"],
|
||||||
|
"an attribute attached with With must appear on every record")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTTYHandlerWithAttrsPersistsAcrossRecords checks that the
|
||||||
|
// attributes are retained rather than emitted once. A handler that
|
||||||
|
// stored them but consumed them would pass the test above.
|
||||||
|
func TestTTYHandlerWithAttrsPersistsAcrossRecords(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var buf bytes.Buffer
|
||||||
|
|
||||||
|
logger := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions())).
|
||||||
|
With("request", "abc123")
|
||||||
|
|
||||||
|
logger.Info("first")
|
||||||
|
logger.Info("second")
|
||||||
|
|
||||||
|
plain := ansiEscape.ReplaceAllString(buf.String(), "")
|
||||||
|
lines := strings.Split(strings.TrimSuffix(plain, "\n"), "\n")
|
||||||
|
|
||||||
|
require.Len(t, lines, 2)
|
||||||
|
|
||||||
|
for _, line := range lines {
|
||||||
|
assert.Contains(t, line, "request=abc123")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTTYHandlerWithGroupQualifiesKeys checks that WithGroup does
|
||||||
|
// something real rather than being discarded. This format has no
|
||||||
|
// nesting, so grouping shows up as a dotted key prefix.
|
||||||
|
func TestTTYHandlerWithGroupQualifiesKeys(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
attrs := ttyAttrs(t, func(l *slog.Logger) *slog.Logger {
|
||||||
|
return l.WithGroup("db").With("rows", 3)
|
||||||
|
}, "queried", "table", "chunks")
|
||||||
|
|
||||||
|
assert.Equal(t, "3", attrs["db.rows"],
|
||||||
|
"an attribute added under a group must be qualified by it")
|
||||||
|
assert.Equal(t, "chunks", attrs["db.table"],
|
||||||
|
"a record attribute must also be qualified by the open group")
|
||||||
|
assert.NotContains(t, attrs, "rows")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTTYHandlerByteFormattingSurvivesGrouping guards the interaction
|
||||||
|
// between the two features. The human-readable rendering of a "bytes"
|
||||||
|
// attribute is selected by comparing the key, and keys reaching that
|
||||||
|
// comparison are group-qualified, so a "bytes" attribute logged under an
|
||||||
|
// open group arrived as "transfer.bytes" and fell back to a bare number.
|
||||||
|
// No caller groups a byte count today, which is exactly why this needs a
|
||||||
|
// test rather than a bug report.
|
||||||
|
func TestTTYHandlerByteFormattingSurvivesGrouping(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const oneAndAHalfKiB = 1536
|
||||||
|
|
||||||
|
for name, testCase := range map[string]struct {
|
||||||
|
derive func(*slog.Logger) *slog.Logger
|
||||||
|
key string
|
||||||
|
}{
|
||||||
|
"ungrouped": {
|
||||||
|
derive: func(l *slog.Logger) *slog.Logger { return l },
|
||||||
|
key: "bytes",
|
||||||
|
},
|
||||||
|
"grouped": {
|
||||||
|
derive: func(l *slog.Logger) *slog.Logger {
|
||||||
|
return l.WithGroup("transfer")
|
||||||
|
},
|
||||||
|
key: "transfer.bytes",
|
||||||
|
},
|
||||||
|
} {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var buf bytes.Buffer
|
||||||
|
|
||||||
|
logger := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions()))
|
||||||
|
testCase.derive(logger).Info("uploaded", "bytes", oneAndAHalfKiB)
|
||||||
|
|
||||||
|
line := ansiEscape.ReplaceAllString(buf.String(), "")
|
||||||
|
|
||||||
|
assert.Contains(t, line, testCase.key+"=1.5 KB",
|
||||||
|
"a byte count must be human-readable however it is qualified")
|
||||||
|
assert.NotContains(t, line, strconv.Itoa(oneAndAHalfKiB),
|
||||||
|
"the raw number must not survive the formatting")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTTYHandlerMatchesJSONHandlerAttributes is the drift guard. The
|
||||||
|
// handler is chosen by TTY-ness, so a difference between these two is
|
||||||
|
// invisible in whichever environment the developer is not in — which is
|
||||||
|
// how the original defect survived: attributes vanished on a terminal
|
||||||
|
// and were correct in CI.
|
||||||
|
func TestTTYHandlerMatchesJSONHandlerAttributes(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
derive func(*slog.Logger) *slog.Logger
|
||||||
|
args []any
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "record attributes only",
|
||||||
|
derive: func(l *slog.Logger) *slog.Logger { return l },
|
||||||
|
args: []any{"path", "/etc/vaultik", countKey, 7},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "handler attributes",
|
||||||
|
derive: func(l *slog.Logger) *slog.Logger {
|
||||||
|
return l.With("host", "alpha")
|
||||||
|
},
|
||||||
|
args: []any{countKey, 7},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "handler attributes accumulate",
|
||||||
|
derive: func(l *slog.Logger) *slog.Logger {
|
||||||
|
return l.With("host", "alpha").With("snapshot", "s1")
|
||||||
|
},
|
||||||
|
args: []any{countKey, 7},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "group qualifies later attributes",
|
||||||
|
derive: func(l *slog.Logger) *slog.Logger {
|
||||||
|
return l.WithGroup("db").With("rows", 3)
|
||||||
|
},
|
||||||
|
args: []any{"table", "chunks"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "nested groups",
|
||||||
|
derive: func(l *slog.Logger) *slog.Logger {
|
||||||
|
return l.WithGroup("outer").WithGroup("inner").
|
||||||
|
With("leaf", "v")
|
||||||
|
},
|
||||||
|
args: []any{"other", "w"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "attributes before and after a group",
|
||||||
|
derive: func(l *slog.Logger) *slog.Logger {
|
||||||
|
return l.With("top", "t").WithGroup("g").With("in", "i")
|
||||||
|
},
|
||||||
|
args: []any{"rec", "r"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "inline group value on the record",
|
||||||
|
derive: func(l *slog.Logger) *slog.Logger { return l },
|
||||||
|
args: []any{slog.Group("net",
|
||||||
|
slog.String("proto", "s3"), slog.Int("retries", 2))},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, testCase := range cases {
|
||||||
|
t.Run(testCase.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
tty := ttyAttrs(t, testCase.derive, "message", testCase.args...)
|
||||||
|
js := jsonAttrs(t, testCase.derive, "message", testCase.args...)
|
||||||
|
|
||||||
|
assert.Equal(t, sortedKeys(js), sortedKeys(tty),
|
||||||
|
"TTY and JSON handlers must emit the same attribute keys")
|
||||||
|
assert.Equal(t, js, tty,
|
||||||
|
"TTY and JSON handlers must emit the same attribute values")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// sortedKeys returns m's keys in order, for a stable comparison message.
|
||||||
|
func sortedKeys(m map[string]string) []string {
|
||||||
|
keys := make([]string, 0, len(m))
|
||||||
|
for key := range m {
|
||||||
|
keys = append(keys, key)
|
||||||
|
}
|
||||||
|
|
||||||
|
sort.Strings(keys)
|
||||||
|
|
||||||
|
return keys
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTTYHandlerWithAttrsDoesNotMutateReceiver checks that deriving does
|
||||||
|
// not write through to the parent or to a sibling. slog permits a
|
||||||
|
// handler to be shared, so a WithAttrs that appended into the receiver's
|
||||||
|
// state would leak attributes between unrelated loggers.
|
||||||
|
func TestTTYHandlerWithAttrsDoesNotMutateReceiver(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var buf bytes.Buffer
|
||||||
|
|
||||||
|
base := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions()))
|
||||||
|
first := base.With("branch", "one")
|
||||||
|
second := base.With("branch", "two")
|
||||||
|
|
||||||
|
base.Info("base")
|
||||||
|
first.Info("first")
|
||||||
|
second.Info("second")
|
||||||
|
|
||||||
|
plain := ansiEscape.ReplaceAllString(buf.String(), "")
|
||||||
|
lines := strings.Split(strings.TrimSuffix(plain, "\n"), "\n")
|
||||||
|
|
||||||
|
require.Len(t, lines, 3)
|
||||||
|
|
||||||
|
assert.NotContains(t, lines[0], "branch=",
|
||||||
|
"deriving must not add attributes to the handler derived from")
|
||||||
|
assert.Contains(t, lines[1], "branch=one")
|
||||||
|
assert.NotContains(t, lines[1], "branch=two")
|
||||||
|
assert.Contains(t, lines[2], "branch=two")
|
||||||
|
assert.NotContains(t, lines[2], "branch=one")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTTYHandlerConcurrentDerivation exercises the same handler being
|
||||||
|
// derived from and written through by several goroutines at once, which
|
||||||
|
// is what slog permits and what a mutating WithAttrs would make a data
|
||||||
|
// race. Run under -race by script/test.
|
||||||
|
func TestTTYHandlerConcurrentDerivation(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const workers = 16
|
||||||
|
|
||||||
|
var buf bytes.Buffer
|
||||||
|
|
||||||
|
base := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions())).
|
||||||
|
With("shared", "yes")
|
||||||
|
|
||||||
|
var group sync.WaitGroup
|
||||||
|
|
||||||
|
group.Add(workers)
|
||||||
|
|
||||||
|
for worker := range workers {
|
||||||
|
go func() {
|
||||||
|
defer group.Done()
|
||||||
|
|
||||||
|
base.With("worker", worker).
|
||||||
|
WithGroup("g").
|
||||||
|
With("nested", worker).
|
||||||
|
Info("concurrent")
|
||||||
|
}()
|
||||||
|
}
|
||||||
|
|
||||||
|
group.Wait()
|
||||||
|
|
||||||
|
plain := ansiEscape.ReplaceAllString(buf.String(), "")
|
||||||
|
lines := strings.Split(strings.TrimSuffix(plain, "\n"), "\n")
|
||||||
|
|
||||||
|
require.Len(t, lines, workers)
|
||||||
|
|
||||||
|
for _, line := range lines {
|
||||||
|
assert.Contains(t, line, "shared=yes")
|
||||||
|
assert.Contains(t, line, "worker=")
|
||||||
|
assert.Contains(t, line, "g.nested=")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTTYHandlerEmptyGroupAndAttrsAreNoOps covers the slog.Handler
|
||||||
|
// contract corners: WithGroup("") and WithAttrs(nil) change nothing, and
|
||||||
|
// an empty Attr is dropped rather than rendered as "=".
|
||||||
|
func TestTTYHandlerEmptyGroupAndAttrsAreNoOps(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var buf bytes.Buffer
|
||||||
|
|
||||||
|
handler := log.NewTTYHandler(&buf, debugHandlerOptions())
|
||||||
|
|
||||||
|
assert.Same(t, handler, handler.WithGroup(""),
|
||||||
|
"an empty group name must not open a group")
|
||||||
|
assert.Same(t, handler, handler.WithAttrs(nil),
|
||||||
|
"deriving with no attributes must not allocate a handler")
|
||||||
|
|
||||||
|
slog.New(handler).LogAttrs(context.Background(), slog.LevelInfo, "msg",
|
||||||
|
slog.Attr{}, slog.String("kept", "yes"))
|
||||||
|
|
||||||
|
plain := ansiEscape.ReplaceAllString(buf.String(), "")
|
||||||
|
|
||||||
|
assert.Contains(t, plain, "kept=yes")
|
||||||
|
assert.NotContains(t, plain, " =")
|
||||||
|
}
|
||||||
64
internal/log/with_test.go
Normal file
64
internal/log/with_test.go
Normal file
@@ -0,0 +1,64 @@
|
|||||||
|
//nolint:testpackage // needs the package logger; see TestWithAttributesReachTTYOutput
|
||||||
|
package log //nolint:revive,nolintlint // stdlib log unused here; see #76
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"log/slog"
|
||||||
|
"regexp"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
// withTestANSIEscape matches the SGR sequences TTYHandler emits.
|
||||||
|
var withTestANSIEscape = regexp.MustCompile(`\x1b\[[0-9;]*m`)
|
||||||
|
|
||||||
|
// TestWithAttributesReachTTYOutput exercises the exported package-level
|
||||||
|
// With through a TTYHandler, which is the path the reported defect was
|
||||||
|
// on: the handler is selected by TTY-ness, so on a terminal With's
|
||||||
|
// attributes were silently dropped while the same code printed them
|
||||||
|
// correctly in CI.
|
||||||
|
//
|
||||||
|
// This is an in-package test so it can point the package logger at a
|
||||||
|
// buffer. Building an slog.Logger over a TTYHandler by hand would test
|
||||||
|
// slog, not this package's With, and there is no injectable sink to
|
||||||
|
// reach it from outside. The package logger is process-global, so this
|
||||||
|
// test must not run in parallel.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // replaces the process-global package logger
|
||||||
|
func TestWithAttributesReachTTYOutput(t *testing.T) {
|
||||||
|
var buf bytes.Buffer
|
||||||
|
|
||||||
|
previous := logger
|
||||||
|
|
||||||
|
t.Cleanup(func() { logger = previous })
|
||||||
|
|
||||||
|
logger = slog.New(NewTTYHandler(&buf, &slog.HandlerOptions{
|
||||||
|
Level: slog.LevelDebug,
|
||||||
|
}))
|
||||||
|
|
||||||
|
With("key", "value").Info("hello")
|
||||||
|
|
||||||
|
plain := withTestANSIEscape.ReplaceAllString(buf.String(), "")
|
||||||
|
|
||||||
|
require.NotEmpty(t, plain)
|
||||||
|
assert.Contains(t, plain, "hello")
|
||||||
|
assert.Contains(t, plain, "key=value",
|
||||||
|
"log.With attributes must reach TTYHandler output")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestWithoutInitializedLoggerFallsBack pins the documented behavior of
|
||||||
|
// With before Initialize has run: it hands back the slog default rather
|
||||||
|
// than a nil logger that would panic at the call site.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // replaces the process-global package logger
|
||||||
|
func TestWithoutInitializedLoggerFallsBack(t *testing.T) {
|
||||||
|
previous := logger
|
||||||
|
|
||||||
|
t.Cleanup(func() { logger = previous })
|
||||||
|
|
||||||
|
logger = nil
|
||||||
|
|
||||||
|
assert.NotNil(t, With("key", "value"))
|
||||||
|
}
|
||||||
@@ -79,7 +79,7 @@ func (v *Vaultik) Prune(opts *PruneOptions) error {
|
|||||||
// store is treated as gone. This used to be the separate 'snapshot
|
// store is treated as gone. This used to be the separate 'snapshot
|
||||||
// cleanup' command and is now folded in so a single 'vaultik prune'
|
// cleanup' command and is now folded in so a single 'vaultik prune'
|
||||||
// gets the local index fully back in sync with the destination.
|
// gets the local index fully back in sync with the destination.
|
||||||
err = v.CleanupLocalSnapshots()
|
err = v.CleanupLocalSnapshots(opts)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("reconciling local snapshots with remote: %w", err)
|
return fmt.Errorf("reconciling local snapshots with remote: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
134
internal/vaultik/prune_cleanup_test.go
Normal file
134
internal/vaultik/prune_cleanup_test.go
Normal file
@@ -0,0 +1,134 @@
|
|||||||
|
package vaultik_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/vaultik/internal/log"
|
||||||
|
"sneak.berlin/go/vaultik/internal/vaultik"
|
||||||
|
)
|
||||||
|
|
||||||
|
// cleanupStaleID is a local snapshot record with no remote manifest —
|
||||||
|
// the record CleanupLocalSnapshots exists to remove.
|
||||||
|
const cleanupStaleID = "testhost_home_2026-04-01T09:00:00Z"
|
||||||
|
|
||||||
|
// remainingSnapshotLimit bounds the post-cleanup listing. ListRecent
|
||||||
|
// takes a SQL LIMIT, so it must be positive; the fixtures never exceed
|
||||||
|
// a handful of rows.
|
||||||
|
const remainingSnapshotLimit = 100
|
||||||
|
|
||||||
|
// cleanupStart is the fixture snapshot's start time. Its exact value is
|
||||||
|
// irrelevant; only presence in the index matters here.
|
||||||
|
//
|
||||||
|
//nolint:gochecknoglobals // read-only fixture shared by the tests below
|
||||||
|
var cleanupStart = time.Date(2026, 4, 1, 9, 0, 0, 0, time.UTC)
|
||||||
|
|
||||||
|
// TestCleanupLocalSnapshots_JSONWritesNothingToStdout is the regression
|
||||||
|
// guard for issue #108: `vaultik prune --json | jq` failed because this
|
||||||
|
// function wrote prose to stdout on every branch, ahead of the
|
||||||
|
// PruneBlobsResult document, with no --json awareness at all.
|
||||||
|
//
|
||||||
|
// Both branches are covered because the three writes between them left
|
||||||
|
// no input that avoided the contamination: with stale records there was
|
||||||
|
// a line per record plus a summary, and with none there was still the
|
||||||
|
// "No stale local snapshots found." line.
|
||||||
|
func TestCleanupLocalSnapshots_JSONWritesNothingToStdout(t *testing.T) {
|
||||||
|
log.Initialize(log.Config{})
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
for name, seed := range map[string]func(*listEnv){
|
||||||
|
"no stale records": func(env *listEnv) {
|
||||||
|
// A snapshot present both locally and remotely: nothing to
|
||||||
|
// remove, which used to print the "No stale" line.
|
||||||
|
env.addLocal(t, listLocalID, cleanupStart)
|
||||||
|
env.addRemote(t, listLocalID, cleanupStart)
|
||||||
|
},
|
||||||
|
"stale records present": func(env *listEnv) {
|
||||||
|
env.addLocal(t, cleanupStaleID, cleanupStart)
|
||||||
|
},
|
||||||
|
"nothing at all": func(_ *listEnv) {},
|
||||||
|
} {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := newListEnv(t)
|
||||||
|
seed(env)
|
||||||
|
|
||||||
|
err := env.v.CleanupLocalSnapshots(&vaultik.PruneOptions{JSON: true})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
assert.Empty(t, env.stdout.String(),
|
||||||
|
"stdout carries the --json document and nothing else")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCleanupLocalSnapshots_HumanOutputRetained pins the other half of
|
||||||
|
// the contract. Without it the test above would be satisfied by
|
||||||
|
// deleting the three lines outright, and a `vaultik prune` with no
|
||||||
|
// flags must still say that it removed records from the local index —
|
||||||
|
// that is the deletion of local state, not decoration.
|
||||||
|
func TestCleanupLocalSnapshots_HumanOutputRetained(t *testing.T) {
|
||||||
|
log.Initialize(log.Config{})
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
t.Run("stale records present", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := newListEnv(t)
|
||||||
|
env.addLocal(t, cleanupStaleID, cleanupStart)
|
||||||
|
|
||||||
|
err := env.v.CleanupLocalSnapshots(&vaultik.PruneOptions{})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
out := env.stdout.String()
|
||||||
|
|
||||||
|
assert.Contains(t, out, "Removing stale local record: "+cleanupStaleID)
|
||||||
|
assert.Contains(t, out, "Removed 1 stale local snapshot record(s).")
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("no stale records", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := newListEnv(t)
|
||||||
|
env.addLocal(t, listLocalID, cleanupStart)
|
||||||
|
env.addRemote(t, listLocalID, cleanupStart)
|
||||||
|
|
||||||
|
err := env.v.CleanupLocalSnapshots(&vaultik.PruneOptions{})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
assert.Contains(t, env.stdout.String(),
|
||||||
|
"No stale local snapshots found.")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCleanupLocalSnapshots_RemovesOnlyStaleRecords checks that the
|
||||||
|
// --json gate did not change what the function does, only what it
|
||||||
|
// says: the stale record is gone from the index and the one with a
|
||||||
|
// remote manifest is untouched.
|
||||||
|
func TestCleanupLocalSnapshots_RemovesOnlyStaleRecords(t *testing.T) {
|
||||||
|
log.Initialize(log.Config{})
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := newListEnv(t)
|
||||||
|
env.addLocal(t, listLocalID, cleanupStart)
|
||||||
|
env.addRemote(t, listLocalID, cleanupStart)
|
||||||
|
env.addLocal(t, cleanupStaleID, cleanupStart)
|
||||||
|
|
||||||
|
err := env.v.CleanupLocalSnapshots(&vaultik.PruneOptions{JSON: true})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
remaining, err := env.v.Repositories.Snapshots.ListRecent(
|
||||||
|
env.v.Context(), remainingSnapshotLimit)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
ids := make([]string, 0, len(remaining))
|
||||||
|
for _, snap := range remaining {
|
||||||
|
ids = append(ids, snap.ID.String())
|
||||||
|
}
|
||||||
|
|
||||||
|
assert.Equal(t, []string{listLocalID}, ids,
|
||||||
|
"only the record with no remote manifest may be removed")
|
||||||
|
}
|
||||||
@@ -829,7 +829,15 @@ func (v *Vaultik) outputVerifyJSON(result *VerifyResult) error {
|
|||||||
// behind by incomplete or interrupted backups. Each local snapshot's
|
// behind by incomplete or interrupted backups. Each local snapshot's
|
||||||
// human ID is hashed via RemoteSnapshotKey and compared against the
|
// human ID is hashed via RemoteSnapshotKey and compared against the
|
||||||
// remote listing.
|
// remote listing.
|
||||||
func (v *Vaultik) CleanupLocalSnapshots() error {
|
//
|
||||||
|
// It takes the whole *PruneOptions, symmetric with PruneBlobs, because
|
||||||
|
// it is the other half of one command: Prune runs this phase and then
|
||||||
|
// that one. Only JSON is read here. Under --json every write below is
|
||||||
|
// suppressed, because stdout carries the PruneBlobsResult document and
|
||||||
|
// nothing else — prose ahead of it is what made `vaultik prune --json |
|
||||||
|
// jq` fail (issue #108). The narration is duplicated as log records,
|
||||||
|
// which go to stderr and so cannot corrupt the document.
|
||||||
|
func (v *Vaultik) CleanupLocalSnapshots(opts *PruneOptions) error {
|
||||||
err := v.EnsureStorageBinding()
|
err := v.EnsureStorageBinding()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
@@ -855,7 +863,11 @@ func (v *Vaultik) CleanupLocalSnapshots() error {
|
|||||||
for _, snap := range localSnapshots {
|
for _, snap := range localSnapshots {
|
||||||
id := snap.ID.String()
|
id := snap.ID.String()
|
||||||
if !remoteSet[snapshot.RemoteSnapshotKey(id)] {
|
if !remoteSet[snapshot.RemoteSnapshotKey(id)] {
|
||||||
v.stdoutf("Removing stale local record: %s\n", id)
|
log.Info("Removing stale local snapshot record", "snapshot_id", id)
|
||||||
|
|
||||||
|
if !opts.JSON {
|
||||||
|
v.stdoutf("Removing stale local record: %s\n", id)
|
||||||
|
}
|
||||||
|
|
||||||
err = v.deleteSnapshotFromLocalDB(id)
|
err = v.deleteSnapshotFromLocalDB(id)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -869,6 +881,13 @@ func (v *Vaultik) CleanupLocalSnapshots() error {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
log.Info("Reconciled local snapshot records against remote metadata",
|
||||||
|
"removed", removed, "examined", len(localSnapshots))
|
||||||
|
|
||||||
|
if opts.JSON {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
if removed == 0 {
|
if removed == 0 {
|
||||||
v.printlnStdout("No stale local snapshots found.")
|
v.printlnStdout("No stale local snapshots found.")
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -87,7 +87,7 @@ func (v *Vaultik) ListSnapshots(jsonOutput bool) error {
|
|||||||
snapshots = append(snapshots, info)
|
snapshots = append(snapshots, info)
|
||||||
}
|
}
|
||||||
|
|
||||||
listing, remoteErr := v.collectRemoteSnapshots(localKeys, jsonOutput)
|
listing, remoteErr := v.collectRemoteSnapshots(localKeys)
|
||||||
if remoteErr != nil {
|
if remoteErr != nil {
|
||||||
v.warnRemoteListingFailed(remoteErr, jsonOutput)
|
v.warnRemoteListingFailed(remoteErr, jsonOutput)
|
||||||
} else {
|
} else {
|
||||||
@@ -131,23 +131,23 @@ func (v *Vaultik) ListSnapshots(jsonOutput bool) error {
|
|||||||
// still worth printing, and `snapshot list` exiting non-zero because a
|
// still worth printing, and `snapshot list` exiting non-zero because a
|
||||||
// volume is unmounted would be worse than useless.
|
// volume is unmounted would be worse than useless.
|
||||||
//
|
//
|
||||||
// In --json mode the warning goes to stderr rather than through the
|
// The two output modes report it through different channels. Table mode
|
||||||
// logger or the UI writer, both of which emit on stdout — the JSON
|
// uses the UI writer, whose prose and color match the table it sits
|
||||||
// document has to be the only thing on stdout for `snapshot list --json
|
// under. The UI writer emits on stdout, though, so --json mode uses the
|
||||||
// | jq` to work. The failure is also representable in the document
|
// logger instead: stdout has to hold nothing but the JSON document for
|
||||||
// itself: every row's remote_present is null when the destination could
|
// `snapshot list --json | jq` to work. Both channels are chosen once,
|
||||||
// not be listed.
|
// never both, so the user is not told the same thing twice.
|
||||||
|
//
|
||||||
|
// The failure is also representable in the document itself: every row's
|
||||||
|
// remote_present is null when the destination could not be listed.
|
||||||
func (v *Vaultik) warnRemoteListingFailed(err error, jsonOutput bool) {
|
func (v *Vaultik) warnRemoteListingFailed(err error, jsonOutput bool) {
|
||||||
if jsonOutput {
|
if jsonOutput {
|
||||||
_, _ = fmt.Fprintf(v.Stderr,
|
log.Warn("Could not list backup destination store; "+
|
||||||
"Warning: could not list backup destination store: %v. "+
|
"showing snapshots from the local index only", "error", err)
|
||||||
"Showing snapshots from the local index only.\n", err)
|
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Once only: the logger also writes to stdout, so emitting through
|
|
||||||
// both it and the UI would print the same sentence to the user twice.
|
|
||||||
v.UI.Warningf("Could not list backup destination store: %v.", err)
|
v.UI.Warningf("Could not list backup destination store: %v.", err)
|
||||||
v.UI.Infof("Showing snapshots from the local index only.")
|
v.UI.Infof("Showing snapshots from the local index only.")
|
||||||
}
|
}
|
||||||
@@ -156,69 +156,29 @@ func (v *Vaultik) warnRemoteListingFailed(err error, jsonOutput bool) {
|
|||||||
// is about to read is incomplete: manifests that could not be read, and
|
// is about to read is incomplete: manifests that could not be read, and
|
||||||
// remote-only snapshots dropped by the maxRemoteOnlyRows cap.
|
// remote-only snapshots dropped by the maxRemoteOnlyRows cap.
|
||||||
//
|
//
|
||||||
// Table mode reports both below the table (see reportListDrift). In
|
// Table mode reports both below the table (see reportListDrift) through
|
||||||
// --json mode they cannot go on stdout — the document has to be the
|
// the UI writer, which emits on stdout. In --json mode stdout has to
|
||||||
// only thing there for `snapshot list --json | jq` to work — and the
|
// hold nothing but the document for `snapshot list --json | jq` to
|
||||||
// document's shape is deliberately left alone so existing consumers
|
// work, and the document's shape is deliberately left alone so existing
|
||||||
// keep parsing. So they go to stderr, the same place the
|
// consumers keep parsing — so these go to the logger, which writes to
|
||||||
// unreachable-destination warning already goes. A consumer that must
|
// stderr. A consumer that must react to truncation can treat any output
|
||||||
// react to truncation can treat any output on this stream as "this
|
// on that stream as "this listing is not the whole picture"; silent
|
||||||
// listing is not the whole picture"; silent truncation of a listing
|
// truncation of a listing whose whole purpose is disaster recovery is
|
||||||
// whose whole purpose is disaster recovery is the worse failure.
|
// the worse failure.
|
||||||
func (v *Vaultik) reportJSONListingLimits(listing *remoteSnapshotListing) {
|
func (v *Vaultik) reportJSONListingLimits(listing *remoteSnapshotListing) {
|
||||||
if listing.unreadable > 0 {
|
if listing.unreadable > 0 {
|
||||||
_, _ = fmt.Fprintf(v.Stderr,
|
log.Warn("Some remote snapshot(s) could not be described: "+
|
||||||
"Warning: %d remote snapshot(s) could not be described: "+
|
"manifest missing or unreadable; they are missing from "+
|
||||||
"manifest missing or unreadable. They are missing from "+
|
"this listing", "unreadable", listing.unreadable)
|
||||||
"this listing.\n", listing.unreadable)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if listing.omitted > 0 {
|
if listing.omitted > 0 {
|
||||||
_, _ = fmt.Fprintf(v.Stderr,
|
log.Warn("Listing truncated: further remote-only snapshot(s) "+
|
||||||
"Warning: listing truncated: %d further remote-only "+
|
"not shown", "omitted", listing.omitted,
|
||||||
"snapshot(s) not shown (limit %d per listing).\n",
|
"limit", maxRemoteOnlyRows)
|
||||||
listing.omitted, maxRemoteOnlyRows)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// kvPairSize is the number of variadic arguments that make up one
|
|
||||||
// structured logging key/value pair.
|
|
||||||
const kvPairSize = 2
|
|
||||||
|
|
||||||
// warnWhileListing reports a per-snapshot problem found while
|
|
||||||
// describing the destination store, through a writer that is safe for
|
|
||||||
// the current output mode.
|
|
||||||
//
|
|
||||||
// In --json mode it writes to v.Stderr rather than calling log.Warn,
|
|
||||||
// for the same reason warnRemoteListingFailed does: internal/log builds
|
|
||||||
// its logger over os.Stdout and defaults to level Warn, so one warning
|
|
||||||
// there would put a log line on stdout ahead of the JSON document and
|
|
||||||
// break `snapshot list --json | jq`. A single corrupt manifest is
|
|
||||||
// precisely the degradation this listing is built to survive, so it
|
|
||||||
// must not be the thing that corrupts the output.
|
|
||||||
//
|
|
||||||
// This is a local workaround. Remove it, and the branch in
|
|
||||||
// warnRemoteListingFailed, once issue #82 makes the logger's sink
|
|
||||||
// configurable.
|
|
||||||
func (v *Vaultik) warnWhileListing(jsonOutput bool, msg string, args ...any) {
|
|
||||||
if !jsonOutput {
|
|
||||||
log.Warn(msg, args...)
|
|
||||||
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
var line strings.Builder
|
|
||||||
|
|
||||||
_, _ = fmt.Fprintf(&line, "Warning: %s", msg)
|
|
||||||
|
|
||||||
for i := 0; i+kvPairSize <= len(args); i += kvPairSize {
|
|
||||||
pair := args[i : i+kvPairSize]
|
|
||||||
_, _ = fmt.Fprintf(&line, " %v=%v", pair[0], pair[1])
|
|
||||||
}
|
|
||||||
|
|
||||||
_, _ = fmt.Fprintln(v.Stderr, line.String())
|
|
||||||
}
|
|
||||||
|
|
||||||
// remoteSnapshotListing is the result of one pass over the destination
|
// remoteSnapshotListing is the result of one pass over the destination
|
||||||
// store's metadata/ prefix.
|
// store's metadata/ prefix.
|
||||||
type remoteSnapshotListing struct {
|
type remoteSnapshotListing struct {
|
||||||
@@ -247,11 +207,8 @@ type remoteSnapshotListing struct {
|
|||||||
// Manifest reads scale only with the number of snapshots the local
|
// Manifest reads scale only with the number of snapshots the local
|
||||||
// index does not already know about, and are capped at
|
// index does not already know about, and are capped at
|
||||||
// maxRemoteOnlyRows.
|
// maxRemoteOnlyRows.
|
||||||
//
|
|
||||||
// jsonOutput only selects where per-snapshot warnings are written; see
|
|
||||||
// warnWhileListing.
|
|
||||||
func (v *Vaultik) collectRemoteSnapshots(
|
func (v *Vaultik) collectRemoteSnapshots(
|
||||||
localKeys map[string]bool, jsonOutput bool,
|
localKeys map[string]bool,
|
||||||
) (*remoteSnapshotListing, error) {
|
) (*remoteSnapshotListing, error) {
|
||||||
keys, err := v.listAllRemoteSnapshotKeys()
|
keys, err := v.listAllRemoteSnapshotKeys()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -282,17 +239,22 @@ func (v *Vaultik) collectRemoteSnapshots(
|
|||||||
}
|
}
|
||||||
|
|
||||||
listing.remoteOnly, listing.unreadable = v.describeRemoteOnlySnapshots(
|
listing.remoteOnly, listing.unreadable = v.describeRemoteOnlySnapshots(
|
||||||
unknown, jsonOutput)
|
unknown)
|
||||||
|
|
||||||
return listing, nil
|
return listing, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// listingWarning is a problem found with one remote snapshot, recorded
|
// listingWarning is a problem found with one remote snapshot, recorded
|
||||||
// rather than emitted on the spot. Manifest reads run concurrently and
|
// rather than emitted on the spot. Manifest reads run concurrently, so
|
||||||
// the writer chosen by warnWhileListing is not guaranteed to be safe
|
// emitting from the worker that found the problem would order the
|
||||||
// for concurrent use, so warnings are held until every read has
|
// warnings by fetch completion — which varies run to run with network
|
||||||
// finished and then emitted in key order from a single goroutine. That
|
// timing and tells the reader nothing. Holding them and emitting in key
|
||||||
// also makes the warning order deterministic run to run.
|
// order from a single goroutine after every read has finished makes two
|
||||||
|
// runs over the same damaged store produce the same diagnostics in the
|
||||||
|
// same order.
|
||||||
|
//
|
||||||
|
// Concurrency safety is no longer part of the reason: these are emitted
|
||||||
|
// through log.Warn, and slog handlers are safe for concurrent use.
|
||||||
type listingWarning struct {
|
type listingWarning struct {
|
||||||
msg string
|
msg string
|
||||||
args []any
|
args []any
|
||||||
@@ -306,7 +268,7 @@ type listingWarning struct {
|
|||||||
// failing the listing: one bad snapshot directory must not hide every
|
// failing the listing: one bad snapshot directory must not hide every
|
||||||
// other snapshot the user has.
|
// other snapshot the user has.
|
||||||
func (v *Vaultik) describeRemoteOnlySnapshots(
|
func (v *Vaultik) describeRemoteOnlySnapshots(
|
||||||
keys []string, jsonOutput bool,
|
keys []string,
|
||||||
) ([]SnapshotInfo, int) {
|
) ([]SnapshotInfo, int) {
|
||||||
found := make([]SnapshotInfo, len(keys))
|
found := make([]SnapshotInfo, len(keys))
|
||||||
ok := make([]bool, len(keys))
|
ok := make([]bool, len(keys))
|
||||||
@@ -349,7 +311,7 @@ func (v *Vaultik) describeRemoteOnlySnapshots(
|
|||||||
|
|
||||||
for i := range keys {
|
for i := range keys {
|
||||||
if warnings[i] != nil {
|
if warnings[i] != nil {
|
||||||
v.warnWhileListing(jsonOutput, warnings[i].msg, warnings[i].args...)
|
log.Warn(warnings[i].msg, warnings[i].args...)
|
||||||
}
|
}
|
||||||
|
|
||||||
if !ok[i] {
|
if !ok[i] {
|
||||||
|
|||||||
@@ -108,7 +108,6 @@ type listEnv struct {
|
|||||||
v *vaultik.Vaultik
|
v *vaultik.Vaultik
|
||||||
store *observingStorer
|
store *observingStorer
|
||||||
stdout *bytes.Buffer
|
stdout *bytes.Buffer
|
||||||
stderr *bytes.Buffer
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func newListEnv(t *testing.T) *listEnv {
|
func newListEnv(t *testing.T) *listEnv {
|
||||||
@@ -122,7 +121,6 @@ func newListEnv(t *testing.T) *listEnv {
|
|||||||
|
|
||||||
store := newObservingStorer()
|
store := newObservingStorer()
|
||||||
stdout := &bytes.Buffer{}
|
stdout := &bytes.Buffer{}
|
||||||
stderr := &bytes.Buffer{}
|
|
||||||
|
|
||||||
v := &vaultik.Vaultik{
|
v := &vaultik.Vaultik{
|
||||||
Config: &config.Config{
|
Config: &config.Config{
|
||||||
@@ -135,13 +133,13 @@ func newListEnv(t *testing.T) *listEnv {
|
|||||||
Repositories: database.NewRepositories(db),
|
Repositories: database.NewRepositories(db),
|
||||||
DB: db,
|
DB: db,
|
||||||
Stdout: stdout,
|
Stdout: stdout,
|
||||||
Stderr: stderr,
|
Stderr: &bytes.Buffer{},
|
||||||
Stdin: &bytes.Buffer{},
|
Stdin: &bytes.Buffer{},
|
||||||
UI: ui.NewWithColor(stdout, false),
|
UI: ui.NewWithColor(stdout, false),
|
||||||
}
|
}
|
||||||
v.SetContext(ctx)
|
v.SetContext(ctx)
|
||||||
|
|
||||||
return &listEnv{v: v, store: store, stdout: stdout, stderr: stderr}
|
return &listEnv{v: v, store: store, stdout: stdout}
|
||||||
}
|
}
|
||||||
|
|
||||||
// addLocal inserts a completed snapshot into the local index.
|
// addLocal inserts a completed snapshot into the local index.
|
||||||
@@ -521,16 +519,16 @@ func TestListSnapshots_JSONMergedView(t *testing.T) {
|
|||||||
// TestListSnapshots_JSONUnreachableRemote checks that a failed listing
|
// TestListSnapshots_JSONUnreachableRemote checks that a failed listing
|
||||||
// does not corrupt the JSON document with warning text, and that
|
// does not corrupt the JSON document with warning text, and that
|
||||||
// "unknown" is reported as null rather than as absence.
|
// "unknown" is reported as null rather than as absence.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // captureProcessStderr replaces os.Stderr
|
||||||
func TestListSnapshots_JSONUnreachableRemote(t *testing.T) {
|
func TestListSnapshots_JSONUnreachableRemote(t *testing.T) {
|
||||||
log.Initialize(log.Config{})
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
env := newListEnv(t)
|
env := newListEnv(t)
|
||||||
env.addLocal(t, listLocalID, time.Date(2026, 3, 1, 10, 0, 0, 0, time.UTC))
|
env.addLocal(t, listLocalID, time.Date(2026, 3, 1, 10, 0, 0, 0, time.UTC))
|
||||||
env.store.listErr = errRemoteUnreachable
|
env.store.listErr = errRemoteUnreachable
|
||||||
|
|
||||||
err := env.v.ListSnapshots(true)
|
stderr := captureProcessStderr(t, func() {
|
||||||
require.NoError(t, err)
|
require.NoError(t, env.v.ListSnapshots(true))
|
||||||
|
})
|
||||||
|
|
||||||
// stdout must be nothing but the JSON document, so the warning has
|
// stdout must be nothing but the JSON document, so the warning has
|
||||||
// to go to stderr.
|
// to go to stderr.
|
||||||
@@ -542,9 +540,8 @@ func TestListSnapshots_JSONUnreachableRemote(t *testing.T) {
|
|||||||
assert.Nil(t, rows[0].RemotePresent,
|
assert.Nil(t, rows[0].RemotePresent,
|
||||||
"remote state is unknown when the destination cannot be listed")
|
"remote state is unknown when the destination cannot be listed")
|
||||||
|
|
||||||
assert.Contains(t, env.stderr.String(),
|
assert.Contains(t, stderr, "Could not list backup destination store")
|
||||||
"could not list backup destination store")
|
assert.Contains(t, stderr, "permission denied")
|
||||||
assert.Contains(t, env.stderr.String(), "permission denied")
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// useNonUTCLocalZone points time.Local at a fixed non-UTC zone for the
|
// useNonUTCLocalZone points time.Local at a fixed non-UTC zone for the
|
||||||
@@ -635,10 +632,9 @@ func TestListSnapshots_TimestampsAreUTCOnNonUTCHost(t *testing.T) {
|
|||||||
// machine consumer would otherwise see no difference between "that
|
// machine consumer would otherwise see no difference between "that
|
||||||
// snapshot is not on the destination" and "that snapshot could not be
|
// snapshot is not on the destination" and "that snapshot could not be
|
||||||
// read".
|
// read".
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // captureProcessStderr replaces os.Stderr
|
||||||
func TestListSnapshots_JSONReportsUnreadableManifests(t *testing.T) {
|
func TestListSnapshots_JSONReportsUnreadableManifests(t *testing.T) {
|
||||||
log.Initialize(log.Config{})
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
env := newListEnv(t)
|
env := newListEnv(t)
|
||||||
|
|
||||||
goodKey := env.addRemote(t, listRemoteID,
|
goodKey := env.addRemote(t, listRemoteID,
|
||||||
@@ -650,16 +646,18 @@ func TestListSnapshots_JSONReportsUnreadableManifests(t *testing.T) {
|
|||||||
strings.NewReader("this is not a zstd stream"))
|
strings.NewReader("this is not a zstd stream"))
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
err = env.v.ListSnapshots(true)
|
stderr := captureProcessStderr(t, func() {
|
||||||
require.NoError(t, err)
|
require.NoError(t, env.v.ListSnapshots(true))
|
||||||
|
})
|
||||||
|
|
||||||
rows := decodeListJSON(t, env.stdout.String())
|
rows := decodeListJSON(t, env.stdout.String())
|
||||||
require.Len(t, rows, 1)
|
require.Len(t, rows, 1)
|
||||||
assert.Equal(t, goodKey, rows[0].RemoteKey)
|
assert.Equal(t, goodKey, rows[0].RemoteKey)
|
||||||
|
|
||||||
assert.Contains(t, env.stderr.String(),
|
assert.Contains(t, stderr, "could not be described",
|
||||||
"1 remote snapshot(s) could not be described",
|
|
||||||
"a row dropped from the JSON document must be announced somewhere")
|
"a row dropped from the JSON document must be announced somewhere")
|
||||||
|
assert.Contains(t, stderr, `"unreadable":1`,
|
||||||
|
"the count of dropped rows must be reported, not just the fact")
|
||||||
}
|
}
|
||||||
|
|
||||||
// maxRemoteOnlyRowsForTest mirrors the maxRemoteOnlyRows cap in the
|
// maxRemoteOnlyRowsForTest mirrors the maxRemoteOnlyRows cap in the
|
||||||
@@ -671,10 +669,9 @@ const maxRemoteOnlyRowsForTest = 1000
|
|||||||
// truncation of a listing whose whole purpose is disaster recovery is
|
// truncation of a listing whose whole purpose is disaster recovery is
|
||||||
// the wrong failure mode: the consumer least able to notice is exactly
|
// the wrong failure mode: the consumer least able to notice is exactly
|
||||||
// the one reading JSON.
|
// the one reading JSON.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // captureProcessStderr replaces os.Stderr
|
||||||
func TestListSnapshots_JSONReportsTruncation(t *testing.T) {
|
func TestListSnapshots_JSONReportsTruncation(t *testing.T) {
|
||||||
log.Initialize(log.Config{})
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
env := newListEnv(t)
|
env := newListEnv(t)
|
||||||
|
|
||||||
timestamp := time.Date(2026, 3, 2, 11, 22, 33, 0, time.UTC)
|
timestamp := time.Date(2026, 3, 2, 11, 22, 33, 0, time.UTC)
|
||||||
@@ -684,25 +681,30 @@ func TestListSnapshots_JSONReportsTruncation(t *testing.T) {
|
|||||||
env.addRemote(t, fmt.Sprintf("otherhost_bulk_%04d", i), timestamp)
|
env.addRemote(t, fmt.Sprintf("otherhost_bulk_%04d", i), timestamp)
|
||||||
}
|
}
|
||||||
|
|
||||||
err := env.v.ListSnapshots(true)
|
stderr := captureProcessStderr(t, func() {
|
||||||
require.NoError(t, err)
|
require.NoError(t, env.v.ListSnapshots(true))
|
||||||
|
})
|
||||||
|
|
||||||
rows := decodeListJSON(t, env.stdout.String())
|
rows := decodeListJSON(t, env.stdout.String())
|
||||||
assert.Len(t, rows, maxRemoteOnlyRowsForTest)
|
assert.Len(t, rows, maxRemoteOnlyRowsForTest)
|
||||||
|
|
||||||
assert.Contains(t, env.stderr.String(), "listing truncated")
|
assert.Contains(t, stderr, "Listing truncated")
|
||||||
assert.Contains(t, env.stderr.String(), "1 further remote-only")
|
assert.Contains(t, stderr, `"omitted":1`)
|
||||||
|
assert.Contains(t, stderr,
|
||||||
|
fmt.Sprintf(`"limit":%d`, maxRemoteOnlyRowsForTest))
|
||||||
}
|
}
|
||||||
|
|
||||||
// captureProcessStdout redirects the process's own stdout to a pipe,
|
// captureProcessStdout redirects the process's own stdout to a pipe,
|
||||||
// rebuilds the global logger over it, runs fn, and returns everything
|
// rebuilds the global logger, runs fn, and returns everything written to
|
||||||
// written.
|
// the pipe.
|
||||||
//
|
//
|
||||||
// internal/log builds its logger over os.Stdout at construction time and
|
// The logger is rebuilt on purpose even though it is supposed to write
|
||||||
// offers no injectable sink (issue #82), so a warning logged during a
|
// to stderr: that is exactly what makes this a regression guard. If the
|
||||||
// --json listing lands on the process's real stdout, not on any writer a
|
// logger ever goes back to os.Stdout, Initialize picks up the pipe and
|
||||||
// test can inject. Capturing the file descriptor is therefore the only
|
// the log record shows up in the capture, breaking the JSON parse here
|
||||||
// way a test can see what `snapshot list --json | jq` would see.
|
// the same way it would break `snapshot list --json | jq` in the field.
|
||||||
|
// Without the rebuild, a regressed logger would write to the real stdout
|
||||||
|
// the test process was started with and go unnoticed.
|
||||||
//
|
//
|
||||||
// Not parallel-safe: os.Stdout and the logger are process-global.
|
// Not parallel-safe: os.Stdout and the logger are process-global.
|
||||||
func captureProcessStdout(t *testing.T, fn func(stdout io.Writer)) string {
|
func captureProcessStdout(t *testing.T, fn func(stdout io.Writer)) string {
|
||||||
@@ -714,8 +716,6 @@ func captureProcessStdout(t *testing.T, fn func(stdout io.Writer)) string {
|
|||||||
previous := os.Stdout
|
previous := os.Stdout
|
||||||
os.Stdout = writer
|
os.Stdout = writer
|
||||||
|
|
||||||
// Rebuild the logger so it writes to the pipe rather than to the
|
|
||||||
// real stdout the test process was started with.
|
|
||||||
log.Initialize(log.Config{})
|
log.Initialize(log.Config{})
|
||||||
|
|
||||||
drained := make(chan string, 1)
|
drained := make(chan string, 1)
|
||||||
@@ -738,7 +738,57 @@ func captureProcessStdout(t *testing.T, fn func(stdout io.Writer)) string {
|
|||||||
|
|
||||||
require.NoError(t, reader.Close())
|
require.NoError(t, reader.Close())
|
||||||
|
|
||||||
// Put the logger back on the restored stdout.
|
// Put the logger back on the restored streams.
|
||||||
|
log.Initialize(log.Config{})
|
||||||
|
|
||||||
|
return captured
|
||||||
|
}
|
||||||
|
|
||||||
|
// captureProcessStderr redirects the process's own stderr to a pipe,
|
||||||
|
// rebuilds the global logger over it, runs fn, and returns everything
|
||||||
|
// written.
|
||||||
|
//
|
||||||
|
// internal/log writes every diagnostic to os.Stderr and captures that
|
||||||
|
// file at Initialize time, so a warning logged during a listing lands on
|
||||||
|
// the process's real stderr, not on any writer a test can inject.
|
||||||
|
// Capturing the file descriptor is therefore the only way a test can see
|
||||||
|
// what the operator would see. The captured stream is a pipe rather than
|
||||||
|
// a terminal, so the records are JSON — the same form a redirected
|
||||||
|
// stderr gets in production.
|
||||||
|
//
|
||||||
|
// Not parallel-safe: os.Stderr and the logger are process-global.
|
||||||
|
func captureProcessStderr(t *testing.T, fn func()) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
reader, writer, err := os.Pipe()
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
previous := os.Stderr
|
||||||
|
os.Stderr = writer
|
||||||
|
|
||||||
|
log.Initialize(log.Config{})
|
||||||
|
|
||||||
|
drained := make(chan string, 1)
|
||||||
|
|
||||||
|
go func() {
|
||||||
|
var buf bytes.Buffer
|
||||||
|
|
||||||
|
_, _ = io.Copy(&buf, reader)
|
||||||
|
|
||||||
|
drained <- buf.String()
|
||||||
|
}()
|
||||||
|
|
||||||
|
fn()
|
||||||
|
|
||||||
|
os.Stderr = previous
|
||||||
|
|
||||||
|
require.NoError(t, writer.Close())
|
||||||
|
|
||||||
|
captured := <-drained
|
||||||
|
|
||||||
|
require.NoError(t, reader.Close())
|
||||||
|
|
||||||
|
// Put the logger back on the restored streams.
|
||||||
log.Initialize(log.Config{})
|
log.Initialize(log.Config{})
|
||||||
|
|
||||||
return captured
|
return captured
|
||||||
@@ -747,13 +797,18 @@ func captureProcessStdout(t *testing.T, fn func(stdout io.Writer)) string {
|
|||||||
// TestListSnapshots_JSONStdoutIsOnlyTheDocument is the regression guard
|
// TestListSnapshots_JSONStdoutIsOnlyTheDocument is the regression guard
|
||||||
// for `snapshot list --json | jq` surviving a damaged destination store.
|
// for `snapshot list --json | jq` surviving a damaged destination store.
|
||||||
//
|
//
|
||||||
// Every stdout writer the command has — the JSON encoder, the UI, and
|
// Every stdout writer the command has — the JSON encoder and the UI —
|
||||||
// the global logger — is pointed at one pipe here, exactly as they are
|
// is pointed at one pipe here, exactly as they are pointed at one file
|
||||||
// pointed at one file descriptor in production. A single log line about
|
// descriptor in production, and the logger is rebuilt over that same
|
||||||
// a corrupt manifest ahead of the array is enough to break the parse,
|
// pipe's process-level stdout so that a logger which regressed back to
|
||||||
// and that is what this asserts cannot happen.
|
// stdout would land in the capture. A single log line about a corrupt
|
||||||
|
// manifest ahead of the array is enough to break the parse, and that is
|
||||||
|
// what this asserts cannot happen.
|
||||||
//
|
//
|
||||||
//nolint:paralleltest // replaces os.Stdout and the global logger
|
// The two warnings are asserted on the separately captured stderr: they
|
||||||
|
// must be emitted, just not there.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // replaces os.Stdout, os.Stderr and the logger
|
||||||
func TestListSnapshots_JSONStdoutIsOnlyTheDocument(t *testing.T) {
|
func TestListSnapshots_JSONStdoutIsOnlyTheDocument(t *testing.T) {
|
||||||
env := newListEnv(t)
|
env := newListEnv(t)
|
||||||
|
|
||||||
@@ -772,11 +827,15 @@ func TestListSnapshots_JSONStdoutIsOnlyTheDocument(t *testing.T) {
|
|||||||
oddKey := env.addRemoteRawTimestamp(t,
|
oddKey := env.addRemoteRawTimestamp(t,
|
||||||
"testhost_odd_2026-03-04T00:00:00Z", "the day before yesterday")
|
"testhost_odd_2026-03-04T00:00:00Z", "the day before yesterday")
|
||||||
|
|
||||||
captured := captureProcessStdout(t, func(stdout io.Writer) {
|
var captured string
|
||||||
env.v.Stdout = stdout
|
|
||||||
env.v.UI = ui.NewWithColor(stdout, false)
|
|
||||||
|
|
||||||
require.NoError(t, env.v.ListSnapshots(true))
|
stderr := captureProcessStderr(t, func() {
|
||||||
|
captured = captureProcessStdout(t, func(stdout io.Writer) {
|
||||||
|
env.v.Stdout = stdout
|
||||||
|
env.v.UI = ui.NewWithColor(stdout, false)
|
||||||
|
|
||||||
|
require.NoError(t, env.v.ListSnapshots(true))
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
rows := decodeListJSON(t, captured)
|
rows := decodeListJSON(t, captured)
|
||||||
@@ -794,8 +853,8 @@ func TestListSnapshots_JSONStdoutIsOnlyTheDocument(t *testing.T) {
|
|||||||
|
|
||||||
// Both warnings were emitted, on the stream that cannot corrupt the
|
// Both warnings were emitted, on the stream that cannot corrupt the
|
||||||
// document.
|
// document.
|
||||||
stderr := env.stderr.String()
|
|
||||||
assert.Contains(t, stderr, "Could not describe remote snapshot")
|
assert.Contains(t, stderr, "Could not describe remote snapshot")
|
||||||
assert.Contains(t, stderr, "Remote manifest has an unparseable timestamp")
|
assert.Contains(t, stderr, "Remote manifest has an unparseable timestamp")
|
||||||
assert.Contains(t, stderr, "1 remote snapshot(s) could not be described")
|
assert.Contains(t, stderr, "could not be described")
|
||||||
|
assert.Contains(t, stderr, `"unreadable":1`)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -43,7 +43,15 @@ type Vaultik struct {
|
|||||||
ctx context.Context //nolint:containedctx // ctx bound at construction by design
|
ctx context.Context //nolint:containedctx // ctx bound at construction by design
|
||||||
cancel context.CancelFunc
|
cancel context.CancelFunc
|
||||||
|
|
||||||
// IO
|
// IO. Stdout carries the output the user asked for and nothing else,
|
||||||
|
// so that `--json | jq` works. Stderr completes the standard triple
|
||||||
|
// for anything a command needs to write there directly; diagnostics
|
||||||
|
// are not that — they go through internal/log, which writes to the
|
||||||
|
// process's stderr. No production code writes to Stderr today, so
|
||||||
|
// searching for its writers turns up nothing; it is kept as the
|
||||||
|
// injection point a direct stderr write would otherwise have to
|
||||||
|
// invent, and removing it would make the triple asymmetric for no
|
||||||
|
// gain.
|
||||||
Stdout io.Writer
|
Stdout io.Writer
|
||||||
Stderr io.Writer
|
Stderr io.Writer
|
||||||
Stdin io.Reader
|
Stdin io.Reader
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
@@ -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" .
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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)"
|
||||||
|
|||||||
355
script/lint
355
script/lint
@@ -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 "$@"
|
||||||
|
|||||||
@@ -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 "$@"
|
|
||||||
@@ -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 "$@"
|
||||||
|
|||||||
Reference in New Issue
Block a user