vaultik/cmd/vaultik/lintdocker_test.go

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
	}
}