diff --git a/Dockerfile b/Dockerfile index ec33b34..94b5b57 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,13 +1,9 @@ -# Build stage -# golang 1.25-alpine, 2026-02-28 -FROM golang@sha256:f6751d823c26342f9506c03797d2527668d095b0a15f1862cddb4d927a7a4ced AS builder - -RUN apk add --no-cache git make gcc musl-dev binutils-gold - -# golangci-lint v2.12.2, 2026-08-07 -RUN go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5 -# goimports v0.42.0 -RUN go install golang.org/x/tools/cmd/goimports@009367f5c17a8d4c45a961a3a509277190a9a6f0 +# Lint stage - fast feedback on lint issues, before the build starts. +# The linter is invoked directly rather than through `make lint`: that +# target shells out to `docker build -f Dockerfile.lint`, and there is +# no docker daemon inside a docker build. +# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-10 +FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS lint WORKDIR /src COPY go.mod go.sum ./ @@ -15,8 +11,26 @@ RUN go mod download COPY . . -# Run all checks - build fails if any check fails -RUN make check +RUN make fmt-check +RUN golangci-lint run --config .golangci.yml ./... + +# Build stage +# golang 1.25-alpine, 2026-02-28 +FROM golang@sha256:f6751d823c26342f9506c03797d2527668d095b0a15f1862cddb4d927a7a4ced AS builder + +RUN apk add --no-cache git make gcc musl-dev binutils-gold + +# Force BuildKit to run the lint stage before proceeding +COPY --from=lint /src/go.sum /dev/null + +WORKDIR /src +COPY go.mod go.sum ./ +RUN go mod download + +COPY . . + +# Run the tests - build fails if any test fails +RUN make test # Build the binary RUN make build diff --git a/Dockerfile.lint b/Dockerfile.lint new file mode 100644 index 0000000..4dcb14e --- /dev/null +++ b/Dockerfile.lint @@ -0,0 +1,29 @@ +# Lint-only image: used by script/lint. golangci-lint is never run on +# the host — the repo is COPYed into the build context and the linter +# runs as a build step, so a successful build IS a clean lint. This +# also works where the docker daemon is remote and bind mounts are +# impossible. +# +# `golangci-lint config verify` is deliberately NOT run here: it +# fetches its JSON schema over a live, unpinned HTTPS call, which would +# make linting network-dependent and defeat hash-pinning. The cost of +# that: unknown top-level keys in .golangci.yml are silently ignored, +# so a mistyped or wrong-schema key lints clean while applying nothing. +# +# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-10 +FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS deps + +WORKDIR /src + +# Dependencies first, so this stage stays cached across lint runs. +COPY go.mod go.sum ./ +RUN go mod download + +# Everything below is invalidated on every run by the +# --no-cache-filter=lint that script/lint passes: caching is explicitly +# waived for linting, and a cached build lints nothing. +FROM deps AS lint + +COPY . . + +RUN golangci-lint run --config .golangci.yml ./... diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..34edefe --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 sneak + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index 83f9226..3f5d5bf 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # dnswatcher -dnswatcher is a pre-1.0 Go daemon by [@sneak](https://sneak.berlin) that monitors DNS records, TCP port availability, and TLS certificates, delivering real-time change notifications via Slack, Mattermost, and ntfy webhooks. +dnswatcher is an MIT-licensed, pre-1.0 Go daemon by [@sneak](https://sneak.berlin) that monitors DNS records, TCP port availability, and TLS certificates, delivering real-time change notifications via Slack, Mattermost, and ntfy webhooks. > ⚠️ Pre-1.0 software. APIs, configuration, and behavior may change without notice. @@ -380,14 +380,25 @@ standard: normalized scripts in `script/` are the entrypoints for the development workflow, and the Makefile targets are thin shims that call them. We provide: -- `script/bootstrap` — install all dependencies (go, pinned - golangci-lint and goimports, `go mod download`) +- `script/bootstrap` — install all dependencies (go, pinned goimports, + `go mod download`). It does not install golangci-lint: see + `script/lint` below. - `script/setup` — make a fresh clone ready for development: bootstrap plus the git pre-commit hook - `script/projectname` — print the project name (used for the Docker image tag) -- `script/test` — run the test suite (race detector, coverage) -- `script/lint` — run golangci-lint +- `script/test` — run the test suite (race detector, coverage). Caching + is waived for testing, exactly as it is for linting: `-count=1` + forces every invocation to execute, because the suite queries live + DNS and a cached pass queries nothing. Failures are rerun with `-v` + automatically, and the build fails even if that rerun passes. +- `script/lint` — run golangci-lint, always inside Docker: it builds + `Dockerfile.lint`, which COPYs the repo into the digest-pinned + `golangci-lint` image and lints as a build step, so a successful + build is a clean lint. The linter is never installed or run on the + host, and Docker is the only prerequisite. Caching is waived for + linting: the lint stage is forced to execute on every run with + `--no-cache-filter`, because a cached build lints nothing. - `script/fmt` — format all code (gofmt -s, goimports) - `script/fmt-check` — check formatting (read-only) - `script/check` — run test, lint, and fmt-check @@ -403,7 +414,7 @@ them. We provide: ```sh make build # Build binary to bin/dnswatcher make test # Run tests with race detector -make lint # Run golangci-lint +make lint # Run golangci-lint in Docker (requires docker) make fmt # Format code make check # Run all checks (test, lint, fmt-check) make clean # Remove build artifacts @@ -475,8 +486,9 @@ Viper for configuration. ## License -License has not yet been chosen for this project. Pending decision by the -author (MIT, GPL, or WTFPL). +dnswatcher is released under the MIT License, Copyright (c) 2026 +[@sneak](https://sneak.berlin). See the [`LICENSE`](./LICENSE) file in the +repository root for the full text. ## Author diff --git a/REPO_POLICIES.md b/REPO_POLICIES.md index bc2f161..9aba6b0 100644 --- a/REPO_POLICIES.md +++ b/REPO_POLICIES.md @@ -1,6 +1,6 @@ --- title: Repository Policies -last_modified: 2026-07-06 +last_modified: 2026-08-07 --- This document covers repository structure, tooling, and workflow standards. Code @@ -189,8 +189,13 @@ style conventions are in separate documents: module under test to verify it compiles/parses. There is no excuse for `make test` to be a no-op. -- `make test` must complete in under 20 seconds. Add a 30-second timeout in the - Makefile. +- `make test` must complete in under 60 seconds. That is the hard cap, and a + suite that exceeds it fails. Under 20 seconds is the target. A suite between + 20 and 60 seconds is still green, but the overage must be filed as an + improvement bug against that repo. Add a 90-second timeout to the test + invocation in the Makefile (`go test -timeout 90s`). The backstop deliberately + sits above the hard cap so that it catches a genuinely hung test rather than a + merely slow one. - **`make test` should use the conditional verbose rerun pattern.** Run tests without `-v` (verbose) first. If tests fail, automatically rerun with `-v` to @@ -209,9 +214,9 @@ style conventions are in separate documents: ```makefile test: - @go test -timeout 30s -race -cover ./... || \ + @go test -timeout 90s -race -cover ./... || \ { echo "--- Rerunning with -v for details ---"; \ - go test -timeout 30s -race -v ./...; exit 1; } + go test -timeout 90s -race -v ./...; exit 1; } ``` Python example: @@ -260,7 +265,10 @@ style conventions are in separate documents: - `.golangci.yml` is standardized and must _NEVER_ be modified by an agent, only manually by the user. Fetch from - `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`. + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`. The + canonical golangci-lint version is v2.12.2 (released 2026-05-06), installed + commit-pinned via + `go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5`. - When pinning images or packages by hash, add a comment above the reference with the version and date (YYYY-MM-DD). diff --git a/TESTING.md b/TESTING.md index 3dd34c4..659db13 100644 --- a/TESTING.md +++ b/TESTING.md @@ -17,7 +17,7 @@ real servers ensures the resolver works correctly in production. - Tests hit real DNS infrastructure and require network access - Test duration depends on network conditions; timeout tuning keeps - the suite within the 30-second target + the suite within the 60-second target - Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) plus processing margin - Root server fan-out is limited to reduce parallel query load @@ -31,4 +31,7 @@ real servers ensures the resolver works correctly in production. exists for unit-testing other packages that consume the resolver) - **Do not add `-short` flags** to skip slow tests - **Do not increase `-timeout`** to hide hanging queries +- **Do not remove `-count=1` from `script/test`** — Go's test cache + replays a previous run's output without querying anything, so a + cached pass is not evidence that live resolution works - **Do not modify linter configuration** to suppress findings diff --git a/TODO.md b/TODO.md index bc4c519..7bf50f3 100644 --- a/TODO.md +++ b/TODO.md @@ -18,13 +18,80 @@ iterative resolver implementation with hermetic mocked tests. # Next Step -Policy scaffold commit: add LICENSE, REPO_POLICIES.md, .editorconfig, -.dockerignore, and .gitea/workflows/check.yml, and add the missing -fmt-check, docker, and hooks targets to the Makefile. One commit, then -confirm make check still passes. +Add the README sections required by policy (Description, Getting Started, +Rationale, Design, TODO, License, Author) if any are still missing. # Completed Steps +- 2026-08-10: comment-only corrections to `script/bootstrap`, + `script/cibuild`, and `Dockerfile.lint`. The `goimports` pin in + `script/bootstrap` was justified by a claim that `script/fmt-check` + runs it on the host; it does not (it runs `gofmt -l .` only), so the + header now credits `script/fmt` alone. `script/cibuild` still claimed + the `Dockerfile` runs `make check`, which stopped being true when + linting moved to its own stage; it now describes the lint stage + (`make fmt-check` plus `golangci-lint`) and the builder stage + (`make test`, `make build`). The `docker`-missing warning in + `script/bootstrap` reads as one sentence instead of three fragments + each re-prefixed with `bootstrap:`. `Dockerfile.lint` now records the + residual risk of omitting `golangci-lint config verify`: unknown + top-level keys in `.golangci.yml` are silently ignored, so a mistyped + key lints clean while applying nothing. No behaviour changed +- 2026-08-10: MIT `LICENSE` added at the repository root, closing the + last gap in `REPO_POLICIES.md`'s required-minimum file list and + removing the all-rights-reserved default that would otherwise have + shipped with a 1.0 tag. The licence choice is the standing org policy + (any public repo lacking a licence gets MIT; a private repo with no + licence is already all-rights-reserved), and this repo is public. The + file holds the canonical MIT text byte-for-byte with only the + copyright line filled in (`Copyright (c) 2026 sneak`); no clauses were + added, removed, or reflowed. `README.md`'s first line now names the + licence, as the Description requirement demands, and the License + section states MIT and points at the file instead of saying the choice + is pending. `make fmt` covers only Go sources (`gofmt -s`, + `goimports`), so it cannot reflow `LICENSE` +- 2026-08-10: the policy scaffold (`REPO_POLICIES.md`, `.editorconfig`, + `.dockerignore`, `.gitea/workflows/check.yml`, and the `fmt-check`, + `docker`, and hooks Makefile targets) is present; it landed piecemeal + across the scripts-to-rule-them-all and policy commits rather than as + the single commit this file once planned +- 2026-08-10: Go's test cache disabled for `script/test` via `-count=1`, + so every invocation actually executes. A cached pass replays an + earlier run's output without querying DNS at all, which in this repo + means the suite's entire premise goes unexercised while the run + reports green in under a second. The conditional verbose rerun that + `REPO_POLICIES.md` mandates was added at the same time (the primary + run had been unconditionally `-v`): quiet first, `-v` only on + failure, `-count=1` on both, and exit 1 forced regardless of the + rerun's result so a flake passing the second time cannot turn the + build green. `-timeout 90s` left alone as the deliberate backstop + above the 60s hard cap. Uncached suite runs ~4s, well inside the 20s + target +- 2026-08-10: live-DNS test flakiness addressed by robustness rather + than gating, per the owner's ruling on #93: new + `internal/resolver/livedns_test.go` adds a package-wide concurrency + gate (so parallel tests stop bursting at the first root server), + retry with exponential backoff on transport failures only, and + quorum instead of unanimity for multi-nameserver assertions. Quorum + tolerates silence only: every per-nameserver status must be in a + closed allowlist (`ok`/`timeout`/`error`, or + `nxdomain`/`timeout`/`error`), so a wrong answer from a minority — + `nodata` today, any status added later — fails the test instead of + sliding through under the majority. The + `make test` cap moved to the new org-wide 60s hard cap / 20s target + with a 90s `-timeout` backstop; `REPO_POLICIES.md` re-vendored + byte-identical from `sneak/prompts`. No mocks, no `-short`, no build + tags, no skips, and no change to production resolver behaviour +- 2026-08-10: all linting moved into Docker: new root `Dockerfile.lint` + on the digest-pinned `golangci/golangci-lint:v2.12.2` image, + `script/lint` reduced to a thin wrapper that builds it with + `--no-cache-filter=lint` so the linter actually executes every run, + golangci-lint install dropped from `script/bootstrap` (goimports + stays, `script/fmt` needs it on the host), and the root `Dockerfile` + given its own lint stage so its build no longer recurses through + `make check` into `script/lint`. `golangci-lint config verify` is + deliberately omitted: it fetches its schema over an unpinned live + HTTPS call - 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the org-standard v2-schema config used across the org's repos @@ -53,8 +120,6 @@ confirm make check still passes. Compliance: -- Add README sections required by policy (Description, Getting Started, - Rationale, Design, TODO, License, Author) if any are missing - Pin Dockerfile base images by sha256 and ensure the Docker build runs make check diff --git a/internal/resolver/livedns_harness_test.go b/internal/resolver/livedns_harness_test.go new file mode 100644 index 0000000..87ce704 --- /dev/null +++ b/internal/resolver/livedns_harness_test.go @@ -0,0 +1,284 @@ +package resolver_test + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "sneak.berlin/go/dnswatcher/internal/resolver" +) + +// Tests for the live-DNS harness in livedns_test.go itself. These +// exercise pure logic and the retry/concurrency plumbing; they +// perform no DNS resolution of any kind, so they neither mock DNS +// nor depend on it. + +// Names for the synthetic status maps below. Nothing is ever queried +// at them: they are map keys handed to the package's pure counting +// helpers, not a stand-in for a nameserver. +const ( + nsExample1 = "ns1.example." + nsExample2 = "ns2.example." + nsExample3 = "ns3.example." + nsExample4 = "ns4.example." +) + +func TestLiveQuorumIsStrictMajority(t *testing.T) { + t.Parallel() + + cases := map[int]int{ + 0: 1, + 1: 1, + 2: 2, + 3: 2, + 4: 3, + 5: 3, + 13: 7, + } + + for total, want := range cases { + assert.Equal( + t, want, liveQuorum(total), + "liveQuorum(%d)", total, + ) + } +} + +func TestStatusCountingIgnoresSilentNameservers(t *testing.T) { + t.Parallel() + + results := map[string]*resolver.NameserverResponse{ + nsExample1: { + Nameserver: nsExample1, + Status: resolver.StatusOK, + }, + nsExample2: { + Nameserver: nsExample2, + Status: resolver.StatusOK, + }, + nsExample3: { + Nameserver: nsExample3, + Status: resolver.StatusTimeout, + }, + nsExample4: { + Nameserver: nsExample4, + Status: resolver.StatusError, + }, + } + + assert.Equal( + t, 2, countStatus(results, resolver.StatusOK), + ) + assert.Equal( + t, 0, countStatus(results, resolver.StatusNXDomain), + ) + + // Two of four answered, which is short of the quorum of + // three: this is the state that triggers a retry rather + // than an assertion failure. + assert.Equal(t, 2, answeredCount(results)) + assert.Less(t, answeredCount(results), liveQuorum(len(results))) + + assert.Equal( + t, + "ns1.example.=ok ns2.example.=ok "+ + "ns3.example.=timeout ns4.example.=error", + describeStatuses(results), + ) +} + +func TestRetryLiveRecoversFromTransientFailure(t *testing.T) { + t.Parallel() + + const wantAttempts = 2 + + attempts := 0 + + retryLive(t, "transient", func(_ context.Context) error { + attempts++ + + if attempts < wantAttempts { + return errLiveNoAnswer + } + + return nil + }) + + assert.Equal(t, wantAttempts, attempts) +} + +func TestRetryLiveGivesEachAttemptADeadline(t *testing.T) { + t.Parallel() + + retryLive(t, "deadline", func(ctx context.Context) error { + deadline, ok := ctx.Deadline() + assert.True(t, ok, "attempt should carry a deadline") + + remaining := time.Until(deadline) + + assert.LessOrEqual(t, remaining, liveAttemptTimeout) + + // Lower bound too: without one this passes for a + // deadline far shorter than intended, which would + // silently turn every live attempt into an instant + // timeout. + assert.Greater(t, remaining, liveAttemptTimeout/2) + + return nil + }) +} + +// TestUnsanctionedStatusesRejectsWrongAnswers is the regression test +// for the defect this allowlist exists to prevent: a minority of +// nameservers answering WRONGLY while quorum keeps the suite green. +// nodata is the case that motivated it — it is a wrong answer, not +// silence, and it was previously banned by neither test. +func TestUnsanctionedStatusesRejectsWrongAnswers(t *testing.T) { + t.Parallel() + + // Four nameservers, three OK and one answering nodata: a + // quorum of three is satisfied and no NXDOMAIN is present, so + // the old blocklist assertions both passed on this input. + results := map[string]*resolver.NameserverResponse{ + nsExample1: { + Nameserver: nsExample1, + Status: resolver.StatusOK, + }, + nsExample2: { + Nameserver: nsExample2, + Status: resolver.StatusOK, + }, + nsExample3: { + Nameserver: nsExample3, + Status: resolver.StatusOK, + }, + nsExample4: { + Nameserver: nsExample4, + Status: resolver.StatusNoData, + }, + } + + assert.GreaterOrEqual( + t, + countStatus(results, resolver.StatusOK), + liveQuorum(len(results)), + ) + assert.Zero(t, countStatus(results, resolver.StatusNXDomain)) + + // nodata is an ANSWER, so it never triggers a retry: nothing + // but the allowlist stands between it and a false green. + assert.Equal(t, len(results), answeredCount(results)) + + assert.Equal( + t, + []string{nsExample4 + "=nodata"}, + unsanctionedStatuses( + results, + resolver.StatusOK, + resolver.StatusTimeout, + resolver.StatusError, + ), + "nodata must be reported as an unsanctioned status", + ) +} + +func TestUnsanctionedStatusesToleratesSilenceOnly(t *testing.T) { + t.Parallel() + + results := map[string]*resolver.NameserverResponse{ + nsExample1: { + Nameserver: nsExample1, + Status: resolver.StatusNXDomain, + }, + nsExample2: { + Nameserver: nsExample2, + Status: resolver.StatusTimeout, + }, + nsExample3: { + Nameserver: nsExample3, + Status: resolver.StatusError, + }, + } + + allowed := []string{ + resolver.StatusNXDomain, + resolver.StatusTimeout, + resolver.StatusError, + } + + assert.Empty( + t, + unsanctionedStatuses(results, allowed...), + "timeout and error are non-answers and are tolerated", + ) + + // The same silent nameservers do not count towards a quorum. + assert.Equal(t, 1, answeredCount(results)) + + // An unknown status is treated as silence by answeredCount — + // so it retries and fails loudly — and is unsanctioned by the + // allowlist rather than quietly permitted. + const laterStatus = "some-status-added-later" + + results[nsExample4] = &resolver.NameserverResponse{ + Nameserver: nsExample4, + Status: laterStatus, + } + + assert.Equal(t, 1, answeredCount(results)) + assert.Equal( + t, + []string{nsExample4 + "=" + laterStatus}, + unsanctionedStatuses(results, allowed...), + ) +} + +func TestRunLiveBoundsConcurrency(t *testing.T) { + t.Parallel() + + const workers = 24 + + var ( + mu sync.Mutex + wg sync.WaitGroup + inFlight int + maxSeen int + ) + + wg.Add(workers) + + for range workers { + go func() { + defer wg.Done() + + _ = runLive(func(_ context.Context) error { + mu.Lock() + inFlight++ + + if inFlight > maxSeen { + maxSeen = inFlight + } + mu.Unlock() + + time.Sleep(time.Millisecond) + + mu.Lock() + inFlight-- + mu.Unlock() + + return nil + }) + }() + } + + wg.Wait() + + assert.Positive(t, maxSeen) + assert.LessOrEqual( + t, maxSeen, liveConcurrency, + "live queries must stay under the package-wide gate", + ) +} diff --git a/internal/resolver/livedns_test.go b/internal/resolver/livedns_test.go new file mode 100644 index 0000000..b864460 --- /dev/null +++ b/internal/resolver/livedns_test.go @@ -0,0 +1,495 @@ +package resolver_test + +import ( + "context" + "errors" + "fmt" + "slices" + "sort" + "strings" + "testing" + "time" + + "sneak.berlin/go/dnswatcher/internal/resolver" +) + +// ---------------------------------------------------------------- +// Live DNS test support +// ---------------------------------------------------------------- +// +// Every test in this package resolves against the real, live DNS — +// see TESTING.md. Nothing here mocks, fakes, stubs, records or +// replays DNS, and nothing here skips or gates a test: the helpers +// below only change *how* the live queries are issued, so that a +// single dropped UDP packet or one slow authoritative server does +// not turn a correct resolver into a red build. +// +// Three mechanisms, all test-side: +// +// 1. Bounded concurrency. The package's tests are parallel and the +// build hosts have many cores, so without a limit every test +// starts its own iterative resolution at the same instant and +// they all hit the first root server in rootServerList() within +// a few milliseconds of each other. Root servers rate-limit +// that, which shows up as a different arbitrary subset of tests +// failing on each run. liveGate caps how many resolutions are +// in flight at once. +// +// 2. Retry with exponential backoff. Each live operation gets +// several attempts with its own timeout. The retry predicate is +// strictly transport-level — "did a nameserver answer at all" — +// never the assertion the test is making. A resolver that +// answers incorrectly still fails on the first attempt. +// +// 3. Quorum. Where an assertion spans several independent +// nameservers, a strict majority answering as expected is +// enough; a server that fails to answer is tolerated, while a +// server that answers *wrongly* still fails the test. +// +// The tolerance in (3) is expressed as an ALLOWLIST of sanctioned +// statuses, never as a blocklist of known-bad ones. A blocklist bans +// the one wrong answer its author thought of and silently admits +// every other status, including any added to the resolver later; an +// allowlist fails on anything nobody explicitly sanctioned. Silence +// (timeout, error) is the only thing quorum exists to tolerate. A +// *wrong answer* — nxdomain for a name that exists, ok for one that +// does not, nodata for either — is never tolerated at any count. + +const ( + // liveAttempts is how many times a live DNS operation is + // attempted before the test fails. + liveAttempts = 3 + + // liveAttemptTimeout bounds one attempt. Worst case for an + // operation is liveAttempts * liveAttemptTimeout plus the + // backoff — about 26 seconds, well inside the 90-second + // `go test -timeout` backstop even when several operations + // exhaust their attempts. + liveAttemptTimeout = 8 * time.Second + + // liveBackoffBase is the delay after the first failed + // attempt; it is multiplied by liveBackoffFactor each time. + liveBackoffBase = 500 * time.Millisecond + + // liveBackoffFactor is the exponential backoff multiplier. + liveBackoffFactor = 2 + + // liveConcurrency caps how many live resolutions may be in + // flight across the whole package at once. + liveConcurrency = 6 + + // minNameservers is the smallest nameserver count a + // well-run zone is expected to publish. + minNameservers = 2 +) + +// liveGate bounds concurrent live resolutions package-wide. It has +// to be package scoped: the whole point is that it is shared by +// every parallel test in the package. +// +//nolint:gochecknoglobals // package-wide live query rate limit +var liveGate = make(chan struct{}, liveConcurrency) + +var ( + // errLiveNoAnswer reports that a live operation produced no + // usable answer, which is retried rather than asserted on. + errLiveNoAnswer = errors.New("no answer from live DNS") + + // errLiveNoQuorum reports that too few of a domain's + // nameservers answered for a quorum assertion to be made. + errLiveNoQuorum = errors.New("no nameserver quorum") +) + +// runLive executes one attempt of a live operation, holding a slot +// in liveGate for its duration and bounding it with its own +// timeout. +func runLive(op func(ctx context.Context) error) error { + liveGate <- struct{}{} + defer func() { <-liveGate }() + + ctx, cancel := context.WithTimeout( + context.Background(), liveAttemptTimeout, + ) + defer cancel() + + return op(ctx) +} + +// retryLive runs op until it reports success, retrying transport +// failures with exponential backoff, and fails the test if every +// attempt fails. op returns an error only for a failure to obtain +// an answer — never for an answer the test disagrees with, which +// belongs in an assertion so that it fails immediately. op stores +// whatever it obtained where its caller can find it. +func retryLive( + t *testing.T, + what string, + op func(ctx context.Context) error, +) { + t.Helper() + + var last error + + backoff := liveBackoffBase + + for attempt := range liveAttempts { + if attempt > 0 { + t.Logf( + "%s: attempt %d of %d failed (%v), "+ + "retrying in %s", + what, attempt, liveAttempts, last, backoff, + ) + time.Sleep(backoff) + + backoff *= liveBackoffFactor + } + + last = runLive(op) + if last == nil { + return + } + } + + t.Fatalf( + "%s: no answer after %d live attempts: %v", + what, liveAttempts, last, + ) +} + +// liveQuorum is how many of total nameservers must agree for a +// multi-nameserver assertion to hold: a strict majority. +func liveQuorum(total int) int { + if total < 1 { + return 1 + } + + return total/2 + 1 +} + +// countStatus counts the responses carrying the given status. +func countStatus( + results map[string]*resolver.NameserverResponse, + status string, +) int { + n := 0 + + for _, resp := range results { + if resp.Status == status { + n++ + } + } + + return n +} + +// liveAnswerStatuses is the closed set of statuses that count as a +// nameserver having ANSWERED at all, whether or not the test agrees +// with the answer. It is deliberately an allowlist: a status added +// to the resolver later is treated as silence, so it can only ever +// cause a retry and then a loud failure, never a quiet pass. +func liveAnswerStatuses() []string { + return []string{ + resolver.StatusOK, + resolver.StatusNXDomain, + resolver.StatusNoData, + } +} + +// answeredCount counts the nameservers that produced an answer of +// any kind, as opposed to failing or timing out. +func answeredCount( + results map[string]*resolver.NameserverResponse, +) int { + answers := liveAnswerStatuses() + + n := 0 + + for _, resp := range results { + if slices.Contains(answers, resp.Status) { + n++ + } + } + + return n +} + +// unsanctionedStatuses returns "nameserver=status" for every result +// whose status the caller did not explicitly sanction, sorted for a +// stable failure message. Callers pass the full closed set they will +// accept — the expected answer plus whichever non-answers (timeout, +// error) quorum is allowed to tolerate — so that any status outside +// it fails the test by name. +func unsanctionedStatuses( + results map[string]*resolver.NameserverResponse, + allowed ...string, +) []string { + offenders := make([]string, 0, len(results)) + + for ns, resp := range results { + if slices.Contains(allowed, resp.Status) { + continue + } + + offenders = append( + offenders, fmt.Sprintf("%s=%s", ns, resp.Status), + ) + } + + sort.Strings(offenders) + + return offenders +} + +// describeStatuses renders per-nameserver statuses for use in +// assertion failure messages. +func describeStatuses( + results map[string]*resolver.NameserverResponse, +) string { + parts := make([]string, 0, len(results)) + for ns, resp := range results { + parts = append( + parts, fmt.Sprintf("%s=%s", ns, resp.Status), + ) + } + + sort.Strings(parts) + + return strings.Join(parts, " ") +} + +// ---------------------------------------------------------------- +// Live operation wrappers +// ---------------------------------------------------------------- + +// liveFindAuthoritative resolves a domain's authoritative +// nameservers, retrying until the delegation chain can be walked. +func liveFindAuthoritative( + t *testing.T, + r *resolver.Resolver, + domain string, +) []string { + t.Helper() + + var out []string + + retryLive( + t, + "FindAuthoritativeNameservers("+domain+")", + func(ctx context.Context) error { + ns, err := r.FindAuthoritativeNameservers(ctx, domain) + if err != nil { + return err + } + + if len(ns) == 0 { + return fmt.Errorf( + "%w: %s has no nameservers", + errLiveNoAnswer, domain, + ) + } + + out = ns + + return nil + }, + ) + + return out +} + +// liveLookupNS is liveFindAuthoritative through the LookupNS entry +// point, so that both entry points stay independently exercised. +func liveLookupNS( + t *testing.T, + r *resolver.Resolver, + domain string, +) []string { + t.Helper() + + var out []string + + retryLive( + t, + "LookupNS("+domain+")", + func(ctx context.Context) error { + ns, err := r.LookupNS(ctx, domain) + if err != nil { + return err + } + + if len(ns) == 0 { + return fmt.Errorf( + "%w: %s has no nameservers", + errLiveNoAnswer, domain, + ) + } + + out = ns + + return nil + }, + ) + + return out +} + +// liveQueryNameserver queries one nameserver, retrying while that +// nameserver fails to answer. NXDOMAIN and NODATA are answers and +// are returned to the caller to assert on. +func liveQueryNameserver( + t *testing.T, + r *resolver.Resolver, + nameserver string, + hostname string, +) *resolver.NameserverResponse { + t.Helper() + + what := fmt.Sprintf( + "QueryNameserver(%s, %s)", nameserver, hostname, + ) + + var out *resolver.NameserverResponse + + retryLive( + t, + what, + func(ctx context.Context) error { + resp, err := r.QueryNameserver( + ctx, nameserver, hostname, + ) + if err != nil { + return err + } + + if resp.Status == resolver.StatusTimeout || + resp.Status == resolver.StatusError { + return fmt.Errorf( + "%w: %s returned %s: %s", + errLiveNoAnswer, nameserver, + resp.Status, resp.Error, + ) + } + + out = resp + + return nil + }, + ) + + return out +} + +// liveQueryAllNameservers queries every authoritative nameserver +// for a hostname, retrying until a quorum of them has answered. +// Individual nameservers that stay silent are left in the result +// for the caller to account for. +func liveQueryAllNameservers( + t *testing.T, + r *resolver.Resolver, + hostname string, +) map[string]*resolver.NameserverResponse { + t.Helper() + + var out map[string]*resolver.NameserverResponse + + retryLive( + t, + "QueryAllNameservers("+hostname+")", + func(ctx context.Context) error { + results, err := r.QueryAllNameservers(ctx, hostname) + if err != nil { + return err + } + + if len(results) == 0 { + return fmt.Errorf( + "%w: no nameservers queried for %s", + errLiveNoAnswer, hostname, + ) + } + + answered := answeredCount(results) + if answered < liveQuorum(len(results)) { + return fmt.Errorf( + "%w: %d of %d answered: %s", + errLiveNoQuorum, answered, + len(results), describeStatuses(results), + ) + } + + out = results + + return nil + }, + ) + + return out +} + +// liveResolveIPs resolves a hostname that is expected to have +// addresses, retrying until at least one is returned. +func liveResolveIPs( + t *testing.T, + r *resolver.Resolver, + hostname string, +) []string { + t.Helper() + + var out []string + + retryLive( + t, + "ResolveIPAddresses("+hostname+")", + func(ctx context.Context) error { + ips, err := r.ResolveIPAddresses(ctx, hostname) + if err != nil { + return err + } + + if len(ips) == 0 { + return fmt.Errorf( + "%w: no addresses for %s", + errLiveNoAnswer, hostname, + ) + } + + out = ips + + return nil + }, + ) + + return out +} + +// liveResolveIPsAllowingEmpty resolves a hostname that may legitimately +// have no addresses, so the empty result is returned rather than +// retried. Used for names that must not exist; the corresponding +// QueryAllNameservers test is what proves the nameservers actively +// said NXDOMAIN rather than merely staying silent. +func liveResolveIPsAllowingEmpty( + t *testing.T, + r *resolver.Resolver, + hostname string, +) []string { + t.Helper() + + var out []string + + retryLive( + t, + "ResolveIPAddresses("+hostname+")", + func(ctx context.Context) error { + ips, err := r.ResolveIPAddresses(ctx, hostname) + if err != nil { + return err + } + + out = ips + + return nil + }, + ) + + return out +} diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index bcebfb9..13bdcde 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -32,32 +32,17 @@ func newTestResolver(t *testing.T) *resolver.Resolver { return resolver.NewFromLogger(log) } -func testContext(t *testing.T) context.Context { - t.Helper() - - ctx, cancel := context.WithTimeout( - context.Background(), 60*time.Second, - ) - t.Cleanup(cancel) - - return ctx -} - +// findOneNSForDomain picks one authoritative nameserver to aim a +// test at. Live-DNS retry, concurrency and quorum handling live in +// livedns_test.go. func findOneNSForDomain( t *testing.T, r *resolver.Resolver, - ctx context.Context, //nolint:revive // test helper domain string, ) string { t.Helper() - nameservers, err := r.FindAuthoritativeNameservers( - ctx, domain, - ) - require.NoError(t, err) - require.NotEmpty(t, nameservers) - - return nameservers[0] + return liveFindAuthoritative(t, r, domain)[0] } // ---------------------------------------------------------------- @@ -70,13 +55,7 @@ func TestFindAuthoritativeNameservers_ValidDomain( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - nameservers, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) - require.NotEmpty(t, nameservers) + nameservers := liveFindAuthoritative(t, r, "google.com") hasGoogleNS := false @@ -99,13 +78,9 @@ func TestFindAuthoritativeNameservers_Subdomain( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) + nameservers := liveFindAuthoritative(t, r, "www.google.com") - nameservers, err := r.FindAuthoritativeNameservers( - ctx, "www.google.com", - ) - require.NoError(t, err) - require.NotEmpty(t, nameservers) + assert.NotEmpty(t, nameservers) } func TestFindAuthoritativeNameservers_ReturnsSorted( @@ -114,12 +89,7 @@ func TestFindAuthoritativeNameservers_ReturnsSorted( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - nameservers, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) + nameservers := liveFindAuthoritative(t, r, "google.com") assert.True( t, @@ -134,17 +104,8 @@ func TestFindAuthoritativeNameservers_Deterministic( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - first, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) - - second, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) + first := liveFindAuthoritative(t, r, "google.com") + second := liveFindAuthoritative(t, r, "google.com") assert.Equal(t, first, second) } @@ -155,17 +116,8 @@ func TestFindAuthoritativeNameservers_TrailingDot( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - ns1, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) - - ns2, err := r.FindAuthoritativeNameservers( - ctx, "google.com.", - ) - require.NoError(t, err) + ns1 := liveFindAuthoritative(t, r, "google.com") + ns2 := liveFindAuthoritative(t, r, "google.com.") assert.Equal(t, ns1, ns2) } @@ -176,13 +128,7 @@ func TestFindAuthoritativeNameservers_CloudflareDomain( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - nameservers, err := r.FindAuthoritativeNameservers( - ctx, "cloudflare.com", - ) - require.NoError(t, err) - require.NotEmpty(t, nameservers) + nameservers := liveFindAuthoritative(t, r, "cloudflare.com") for _, ns := range nameservers { assert.True(t, strings.HasSuffix(ns, "."), @@ -199,13 +145,9 @@ func TestQueryNameserver_BasicA(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver(t, r, ns, "www.google.com") - resp, err := r.QueryNameserver( - ctx, ns, "www.google.com", - ) - require.NoError(t, err) require.NotNil(t, resp) assert.Equal(t, resolver.StatusOK, resp.Status) @@ -222,13 +164,8 @@ func TestQueryNameserver_AAAA(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "cloudflare.com") - - resp, err := r.QueryNameserver( - ctx, ns, "cloudflare.com", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "cloudflare.com") + resp := liveQueryNameserver(t, r, ns, "cloudflare.com") aaaaRecords := resp.Records["AAAA"] require.NotEmpty(t, aaaaRecords, @@ -247,13 +184,8 @@ func TestQueryNameserver_MX(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp, err := r.QueryNameserver( - ctx, ns, "google.com", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver(t, r, ns, "google.com") mxRecords := resp.Records["MX"] require.NotEmpty(t, mxRecords, @@ -265,13 +197,8 @@ func TestQueryNameserver_TXT(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp, err := r.QueryNameserver( - ctx, ns, "google.com", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver(t, r, ns, "google.com") txtRecords := resp.Records["TXT"] require.NotEmpty(t, txtRecords, @@ -297,14 +224,10 @@ func TestQueryNameserver_NXDomain(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp, err := r.QueryNameserver( - ctx, ns, - "this-surely-does-not-exist-xyz.google.com", + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver( + t, r, ns, "this-surely-does-not-exist-xyz.google.com", ) - require.NoError(t, err) assert.Equal(t, resolver.StatusNXDomain, resp.Status) } @@ -313,13 +236,8 @@ func TestQueryNameserver_RecordsSorted(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp, err := r.QueryNameserver( - ctx, ns, "google.com", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver(t, r, ns, "google.com") for recordType, values := range resp.Records { assert.True( @@ -336,13 +254,8 @@ func TestQueryNameserver_ResponseIncludesNameserver( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "cloudflare.com") - - resp, err := r.QueryNameserver( - ctx, ns, "cloudflare.com", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "cloudflare.com") + resp := liveQueryNameserver(t, r, ns, "cloudflare.com") assert.Equal(t, ns, resp.Nameserver) } @@ -353,14 +266,10 @@ func TestQueryNameserver_EmptyRecordsOnNXDomain( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp, err := r.QueryNameserver( - ctx, ns, - "this-surely-does-not-exist-xyz.google.com", + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver( + t, r, ns, "this-surely-does-not-exist-xyz.google.com", ) - require.NoError(t, err) totalRecords := 0 for _, values := range resp.Records { @@ -374,18 +283,9 @@ func TestQueryNameserver_TrailingDotHandling(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp1, err := r.QueryNameserver( - ctx, ns, "google.com", - ) - require.NoError(t, err) - - resp2, err := r.QueryNameserver( - ctx, ns, "google.com.", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "google.com") + resp1 := liveQueryNameserver(t, r, ns, "google.com") + resp2 := liveQueryNameserver(t, r, ns, "google.com.") assert.Equal(t, resp1.Status, resp2.Status) } @@ -398,15 +298,9 @@ func TestQueryAllNameservers_ReturnsAllNS(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) + results := liveQueryAllNameservers(t, r, "google.com") - results, err := r.QueryAllNameservers( - ctx, "google.com", - ) - require.NoError(t, err) - require.NotEmpty(t, results) - - assert.GreaterOrEqual(t, len(results), 2) + assert.GreaterOrEqual(t, len(results), minNameservers) for ns, resp := range results { assert.Equal(t, ns, resp.Nameserver) @@ -417,19 +311,36 @@ func TestQueryAllNameservers_AllReturnOK(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) + results := liveQueryAllNameservers(t, r, "google.com") - results, err := r.QueryAllNameservers( - ctx, "google.com", + // A quorum, not unanimity: one authoritative server being + // slow or rate-limiting us is a property of the live + // internet, not a resolver defect. + assert.GreaterOrEqual( + t, + countStatus(results, resolver.StatusOK), + liveQuorum(len(results)), + "a quorum of nameservers should answer OK: %s", + describeStatuses(results), ) - require.NoError(t, err) - for ns, resp := range results { - assert.Equal( - t, resolver.StatusOK, resp.Status, - "NS %s should return OK", ns, - ) - } + // Quorum tolerates SILENCE only. Every individual result must + // be either the expected answer or a non-answer: ok, timeout + // or error, and nothing else. Stated as a closed allowlist so + // that a wrong answer no one thought to ban — nxdomain and + // nodata today, any status added later — fails here rather + // than sliding through under the quorum. + assert.Empty( + t, + unsanctionedStatuses( + results, + resolver.StatusOK, + resolver.StatusTimeout, + resolver.StatusError, + ), + "every nameserver must answer OK or not answer at all: %s", + describeStatuses(results), + ) } func TestQueryAllNameservers_NXDomainFromAllNS( @@ -438,20 +349,34 @@ func TestQueryAllNameservers_NXDomainFromAllNS( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - results, err := r.QueryAllNameservers( - ctx, - "this-surely-does-not-exist-xyz.google.com", + results := liveQueryAllNameservers( + t, r, "this-surely-does-not-exist-xyz.google.com", ) - require.NoError(t, err) - for ns, resp := range results { - assert.Equal( - t, resolver.StatusNXDomain, resp.Status, - "NS %s should return nxdomain", ns, - ) - } + assert.GreaterOrEqual( + t, + countStatus(results, resolver.StatusNXDomain), + liveQuorum(len(results)), + "a quorum of nameservers should report NXDOMAIN: %s", + describeStatuses(results), + ) + + // Silence is tolerated; any actual answer other than NXDOMAIN + // is not. Closed allowlist for the same reason as above: a + // server answering `ok` or `nodata` for a name that must not + // exist is a wrong answer, not a slow one. + assert.Empty( + t, + unsanctionedStatuses( + results, + resolver.StatusNXDomain, + resolver.StatusTimeout, + resolver.StatusError, + ), + "every nameserver must report NXDOMAIN or not answer "+ + "at all: %s", + describeStatuses(results), + ) } // ---------------------------------------------------------------- @@ -462,11 +387,7 @@ func TestLookupNS_ValidDomain(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - nameservers, err := r.LookupNS(ctx, "google.com") - require.NoError(t, err) - require.NotEmpty(t, nameservers) + nameservers := liveLookupNS(t, r, "google.com") for _, ns := range nameservers { assert.True(t, strings.HasSuffix(ns, "."), @@ -479,10 +400,7 @@ func TestLookupNS_Sorted(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - nameservers, err := r.LookupNS(ctx, "google.com") - require.NoError(t, err) + nameservers := liveLookupNS(t, r, "google.com") assert.True(t, sort.StringsAreSorted(nameservers)) } @@ -491,15 +409,8 @@ func TestLookupNS_MatchesFindAuthoritative(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - fromLookup, err := r.LookupNS(ctx, "google.com") - require.NoError(t, err) - - fromFind, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) + fromLookup := liveLookupNS(t, r, "google.com") + fromFind := liveFindAuthoritative(t, r, "google.com") assert.Equal(t, fromFind, fromLookup) } @@ -512,11 +423,7 @@ func TestResolveIPAddresses_ReturnsIPs(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - ips, err := r.ResolveIPAddresses(ctx, "google.com") - require.NoError(t, err) - require.NotEmpty(t, ips) + ips := liveResolveIPs(t, r, "google.com") for _, ip := range ips { parsed := net.ParseIP(ip) @@ -530,10 +437,7 @@ func TestResolveIPAddresses_Deduplicated(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - ips, err := r.ResolveIPAddresses(ctx, "google.com") - require.NoError(t, err) + ips := liveResolveIPs(t, r, "google.com") seen := make(map[string]bool) @@ -547,10 +451,7 @@ func TestResolveIPAddresses_Sorted(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - ips, err := r.ResolveIPAddresses(ctx, "google.com") - require.NoError(t, err) + ips := liveResolveIPs(t, r, "google.com") assert.True(t, sort.StringsAreSorted(ips)) } @@ -561,13 +462,10 @@ func TestResolveIPAddresses_NXDomainReturnsEmpty( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - ips, err := r.ResolveIPAddresses( - ctx, - "this-surely-does-not-exist-xyz.google.com", + ips := liveResolveIPsAllowingEmpty( + t, r, "this-surely-does-not-exist-xyz.google.com", ) - require.NoError(t, err) + assert.Empty(t, ips) } @@ -575,11 +473,9 @@ func TestResolveIPAddresses_CloudflareDomain(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) + ips := liveResolveIPs(t, r, "cloudflare.com") - ips, err := r.ResolveIPAddresses(ctx, "cloudflare.com") - require.NoError(t, err) - require.NotEmpty(t, ips) + assert.NotEmpty(t, ips) } // ---------------------------------------------------------------- diff --git a/script/bootstrap b/script/bootstrap index 129cc77..06da5d0 100755 --- a/script/bootstrap +++ b/script/bootstrap @@ -3,15 +3,17 @@ # this repo. Idempotent: every install is guarded by a check so already # installed tools are skipped. Base tooling comes from nix, apt, brew, # or apk (detected in that order); assumes nothing is present. -# golangci-lint and goimports are installed via `go install` at the same -# pinned commits the Dockerfile uses (never "latest"). +# goimports is installed via `go install` at a pinned commit (never +# "latest") because script/fmt runs it on the host; script/fmt-check +# does not (it runs gofmt only). +# The linter is NOT installed here: golangci-lint runs via docker only +# (script/lint), pinned by image digest, so its only prerequisite is a +# working docker. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" -# Pinned versions, 2026-08-07 (same pins as the Dockerfile) -# golangci-lint v2.12.2 -GOLANGCI_LINT_REF="github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5" +# Pinned version, 2026-08-07 (same pin as the Dockerfile) # goimports v0.42.0 GOIMPORTS_REF="golang.org/x/tools/cmd/goimports@009367f5c17a8d4c45a961a3a509277190a9a6f0" @@ -69,11 +71,17 @@ main() { if missing make; then pkg_install gnumake make make make; fi if missing go; then pkg_install go golang go go; fi - # Lint/format tools, pinned via go install (installs into + # Format tools, pinned via go install (installs into # "$(go env GOPATH)/bin"; ensure that is on your PATH). - if missing golangci-lint; then go install "$GOLANGCI_LINT_REF"; fi if missing goimports; then go install "$GOIMPORTS_REF"; fi + # Linting runs via docker only (script/lint). Warn, don't fail: + # everything except `make lint` works without it. + if missing docker; then + echo "bootstrap: WARNING: docker not found; install it to" \ + "run make lint and make docker." >&2 + fi + go mod download echo "bootstrap complete" diff --git a/script/cibuild b/script/cibuild index 966f51d..1b9e57d 100755 --- a/script/cibuild +++ b/script/cibuild @@ -1,6 +1,7 @@ #!/bin/sh -# script/cibuild: run the CI build. The Dockerfile runs make check, so -# a successful build implies all checks pass. +# script/cibuild: run the CI build. The Dockerfile's lint stage runs +# make fmt-check and golangci-lint; its builder stage runs make test +# and make build. A successful build implies all of those passed. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" diff --git a/script/lint b/script/lint index 8017180..84a1972 100755 --- a/script/lint +++ b/script/lint @@ -1,12 +1,28 @@ #!/bin/sh -# script/lint: run the linter. +# script/lint: run the linter. golangci-lint is never installed or run +# on the host: it runs via docker only, one way, everywhere. This +# builds Dockerfile.lint, which COPYs the repo into the digest-pinned +# golangci-lint image and lints as a build step, so a successful build +# means a clean lint. +# +# --no-cache-filter=lint forces the lint stage (source copy + linter +# run) to execute on every invocation. Without it an unchanged tree +# returns success in well under a second having linted nothing. The +# deps stage (base image + go mod download) stays cached, and no global +# cache invalidation is performed. --progress=plain keeps the linter's +# own output visible. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - golangci-lint run --config .golangci.yml ./... + docker build \ + --progress=plain \ + --no-cache-filter=lint \ + --target lint \ + -f Dockerfile.lint \ + . } main "$@" diff --git a/script/test b/script/test index 50b1730..b4ef492 100755 --- a/script/test +++ b/script/test @@ -1,12 +1,35 @@ #!/bin/sh # script/test: run the test suite. +# +# -count=1 disables Go's test cache, and is load-bearing here. This +# suite queries live DNS on every run by policy (TESTING.md); a cached +# result is a replay of an earlier run's output with no query made at +# all. On an unchanged tree the whole suite would return success in +# under a second having resolved nothing, which makes the repeated-run +# green that is used as evidence for flakiness fixes worthless. Do not +# remove it. +# +# Conditional verbose rerun per REPO_POLICIES.md: run quiet first so +# CI and docker build logs stay readable, and rerun with -v only on +# failure. The rerun also carries -count=1 (a cached replay of the +# failure would show nothing new), and the exit status is forced to 1 +# no matter how the rerun ends: the first failure already proved the +# suite broken, so a flaky test that passes the second time must not +# turn the build green. +# +# -timeout 90s is a deliberate backstop above the 60s hard cap on +# suite duration. Do not lower it. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - go test -v -race -timeout 30s -cover ./... + go test -count=1 -race -timeout 90s -cover ./... || { + echo "--- Rerunning with -v for details ---" >&2 + go test -count=1 -race -timeout 90s -v ./... || true + exit 1 + } } main "$@"