1 Commits

Author SHA1 Message Date
520ce79709 build: isolate golangci-lint cache and lock per checkout (closes #121)
All checks were successful
check / check (push) Successful in 54s
script/lint used golangci-lint's per-user global state, which breaks
when several checkouts on one host lint concurrently. Two independent
failure modes, two causes:

- Cross-contamination. The analysis cache (GOLANGCI_LINT_CACHE,
  default ~/.cache/golangci-lint) is keyed by content, not by
  checkout, so a hit written by another checkout is replayed with
  that checkout's file paths. A run reports findings for files it
  never linted.

- Lock collision. golangci-lint locks os.TempDir()/golangci-lint.lock
  (pkg/commands/run.go, acquireFileLock), which is NOT in the cache
  directory, with a 5s timeout. Peers that hold it longer make the
  run abort with "parallel golangci-lint is running" - a non-result
  that reads as a lint failure. Isolating the cache does not move it.

Point GOLANGCI_LINT_CACHE and TMPDIR at .lint-cache/ under the
checkout root, using the existing $ROOT idiom. TMPDIR is what makes
the lock per-checkout, so the lock keeps serialising the runs that
actually share a cache instead of being disabled. .lint-cache/ is
git-ignored and Docker-ignored, and the cache persists across runs in
a checkout, so caching is not lost.

Reproduced both modes on the unfixed script across 12 copies of this
tree: 10 of 12 concurrent runs void with the lock error, and 11 of 12
sequential runs reported findings at ../w1/... after an identical
lint-failing file was added to every copy. After the fix, 20-way
concurrency gives 0 void and 0 foreign paths, and each copy reports
only its own relative path.
2026-08-09 14:35:42 +00:00
10 changed files with 590 additions and 459 deletions

View File

@@ -1,5 +1,6 @@
.git/ .git/
bin/ bin/
.lint-cache/
*.md *.md
LICENSE LICENSE
.editorconfig .editorconfig

1
.gitignore vendored
View File

@@ -1,6 +1,7 @@
bin/ bin/
vendor/ vendor/
data/ data/
.lint-cache/
.env .env
*.exe *.exe
/dnswatcher /dnswatcher

View File

@@ -387,7 +387,10 @@ them. We provide:
- `script/projectname` — print the project name (used for the Docker - `script/projectname` — print the project name (used for the Docker
image tag) image tag)
- `script/test` — run the test suite (race detector, coverage) - `script/test` — run the test suite (race detector, coverage)
- `script/lint` — run golangci-lint - `script/lint` — run golangci-lint, with its cache and its lock file
isolated to this checkout (under the git-ignored `.lint-cache/`) so
concurrent checkouts on one host cannot share cache entries or
contend on a single lock
- `script/fmt` — format all code (gofmt -s, goimports) - `script/fmt` — format all code (gofmt -s, goimports)
- `script/fmt-check` — check formatting (read-only) - `script/fmt-check` — check formatting (read-only)
- `script/check` — run test, lint, and fmt-check - `script/check` — run test, lint, and fmt-check

View File

@@ -2,10 +2,8 @@
## DNS Resolution Tests ## DNS Resolution Tests
All tests that involve DNS resolution — in every package, including All resolver tests **MUST** use live queries against real DNS servers.
consumers of the resolver such as the watcher — **MUST** use live No mocking of the DNS client layer is permitted.
queries against real DNS servers. No mocking, faking, or stubbing of
DNS at any layer is permitted.
### Rationale ### Rationale
@@ -14,8 +12,6 @@ the full delegation chain. Mocked responses cannot faithfully represent
the variety of real-world DNS behavior (truncation, referrals, glue the variety of real-world DNS behavior (truncation, referrals, glue
records, DNSSEC, varied response times, EDNS, etc.). Testing against records, DNSSEC, varied response times, EDNS, etc.). Testing against
real servers ensures the resolver works correctly in production. real servers ensures the resolver works correctly in production.
Robustness comes from handling real-world DNS behavior with tolerant
assertions and sensible timeouts, not from mocks.
### Constraints ### Constraints
@@ -28,14 +24,11 @@ assertions and sensible timeouts, not from mocks.
- Flaky failures from transient network issues are acceptable and - Flaky failures from transient network issues are acceptable and
should be investigated as potential resolver bugs, not papered over should be investigated as potential resolver bugs, not papered over
with mocks or skip flags with mocks or skip flags
- Watcher change-detection tests seed a synthetic *previous state*
and compare it against fresh live lookups; the DNS side is never
faked
### What NOT to do ### What NOT to do
- **Do not mock `DNSClient`**, the watcher's `DNSResolver` interface, - **Do not mock `DNSClient`** for resolver tests (the mock constructor
or any other DNS abstraction — in any package, for any reason exists for unit-testing other packages that consume the resolver)
- **Do not add `-short` flags** to skip slow tests - **Do not add `-short` flags** to skip slow tests
- **Do not increase `-timeout`** to hide hanging queries - **Do not increase `-timeout`** to hide hanging queries
- **Do not modify linter configuration** to suppress findings - **Do not modify linter configuration** to suppress findings

33
TODO.md
View File

@@ -14,10 +14,7 @@ pre-1.0. No git tags. Core resolver work in flight on feature/resolver
(dirty: internal/resolver/resolver_test.go). Local checkout has diverged (dirty: internal/resolver/resolver_test.go). Local checkout has diverged
from origin: origin/main is 8 commits ahead (watcher orchestrator, from origin: origin/main is 8 commits ahead (watcher orchestrator,
unified TARGETS) and origin/feature/resolver already contains the full unified TARGETS) and origin/feature/resolver already contains the full
iterative resolver implementation. DNS mocking is banned in this repo iterative resolver implementation with hermetic mocked tests.
(see `TESTING.md`): all tests use live DNS only. The hermetic mocked
tests previously noted on `feature/resolver` are gone from its current
tip, which carries a live-DNS suite against `*.dns.sneak.cloud`.
# Next Step # Next Step
@@ -28,10 +25,15 @@ confirm make check still passes.
# Completed Steps # Completed Steps
- 2026-08-07: DNS mocking removed from the entire test suite; watcher - 2026-08-09: `script/lint` now isolates golangci-lint's per-user global
tests now drive the real iterative resolver against live DNS and state to the checkout (#121): `GOLANGCI_LINT_CACHE` and `TMPDIR` are
`TESTING.md` bans DNS mocks in every package (`remove-dns-mocking` both pointed at the git-ignored, Docker-ignored `.lint-cache/`. The
branch) cache fixes cross-contamination; `TMPDIR` is what moves the lock,
which lives at `$TMPDIR/golangci-lint.lock` and not in the cache
directory. Reproduced both failure modes on the unfixed script (10 of
12 concurrent runs void with `parallel golangci-lint is running`; 11
of 12 reporting another checkout's paths) and both are gone at 20-way
concurrency after the fix
- 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs - 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs
in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the
org-standard v2-schema config used across the org's repos org-standard v2-schema config used across the org's repos
@@ -43,8 +45,7 @@ confirm make check still passes.
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
Makefile shims, README Entrypoints section Makefile shims, README Entrypoints section
- 2026-02-20: iterative DNS resolver implemented; tests made hermetic - 2026-02-20: iterative DNS resolver implemented; tests made hermetic
with mocked DNS (origin/feature/resolver, unmerged; superseded — DNS with mocked DNS (origin/feature/resolver, unmerged)
mocking is banned, see `TESTING.md`)
- 2026-02-20: CI actions and go install refs pinned to commit SHAs; - 2026-02-20: CI actions and go install refs pinned to commit SHAs;
Gitea Actions workflow for make check (origin/ci/make-check, unmerged) Gitea Actions workflow for make check (origin/ci/make-check, unmerged)
- 2026-02-20: watcher monitoring orchestrator merged to main (#8) - 2026-02-20: watcher monitoring orchestrator merged to main (#8)
@@ -71,9 +72,8 @@ Branch reconciliation:
- Sync local checkout with origin: local main is 8 commits behind - Sync local checkout with origin: local main is 8 commits behind
origin/main; local feature/resolver has diverged from origin/main; local feature/resolver has diverged from
origin/feature/resolver, which already implements the resolver origin/feature/resolver, which already implements the resolver
- Merge in-flight branches to main once green: feature/resolver - Merge in-flight branches to main once green: feature/resolver,
(confirm its tests remain live-DNS — DNS mocking is banned, see ci/make-check, feature/portcheck-implementation,
`TESTING.md`), ci/make-check, feature/portcheck-implementation,
feature/tlscheck-implementation feature/tlscheck-implementation
Resolver (plan from untracked TODO.md; largely implemented on Resolver (plan from untracked TODO.md; largely implemented on
@@ -157,7 +157,6 @@ Infrastructure notes (from untracked TODO.md):
- Module path sneak.berlin/go/dnswatcher differs from the git.eeqj.de - Module path sneak.berlin/go/dnswatcher differs from the git.eeqj.de
remote intentionally; do not "fix" it remote intentionally; do not "fix" it
- Dependencies: github.com/miekg/dns, golang.org/x/net/publicsuffix - Dependencies: github.com/miekg/dns, golang.org/x/net/publicsuffix
- Resolver tests originally used live DNS against `*.dns.sneak.cloud` - Resolver tests originally used live DNS against *.dns.sneak.cloud
(required records documented in the test file header); `main` now (required records documented in the test file header); origin now has
tests against live public DNS. DNS mocking is banned (see mocked hermetic tests, keep them hermetic
`TESTING.md`); never reintroduce hermetic mocked DNS tests

View File

@@ -7,8 +7,8 @@ import (
"github.com/miekg/dns" "github.com/miekg/dns"
) )
// DNSClient abstracts DNS wire-protocol exchanges over a single // DNSClient abstracts DNS wire-protocol exchanges so the resolver
// transport, letting the resolver switch between UDP and TCP. // can be tested without hitting real nameservers.
type DNSClient interface { type DNSClient interface {
ExchangeContext( ExchangeContext(
ctx context.Context, ctx context.Context,

View File

@@ -67,4 +67,17 @@ func NewFromLogger(log *slog.Logger) *Resolver {
} }
} }
// NewFromLoggerWithClient creates a Resolver with a custom DNS
// client, useful for testing with mock DNS responses.
func NewFromLoggerWithClient(
log *slog.Logger,
client DNSClient,
) *Resolver {
return &Resolver{
log: log,
client: client,
tcp: client,
}
}
// Method implementations are in iterative.go. // Method implementations are in iterative.go.

View File

@@ -10,6 +10,7 @@ import (
"testing" "testing"
"time" "time"
"github.com/miekg/dns"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@@ -623,40 +624,57 @@ func TestQueryAllNameservers_ContextCanceled(t *testing.T) {
} }
// ---------------------------------------------------------------- // ----------------------------------------------------------------
// Unreachable nameserver tests // Timeout tests
// ---------------------------------------------------------------- // ----------------------------------------------------------------
func TestQueryNameserverIP_UnreachableServer(t *testing.T) { func TestQueryNameserverIP_Timeout(t *testing.T) {
t.Parallel() t.Parallel()
r := newTestResolver(t) log := slog.New(slog.NewTextHandler(
os.Stderr,
&slog.HandlerOptions{Level: slog.LevelDebug},
))
r := resolver.NewFromLoggerWithClient(
log, &timeoutClient{},
)
ctx, cancel := context.WithTimeout( ctx, cancel := context.WithTimeout(
context.Background(), 10*time.Second, context.Background(), 10*time.Second,
) )
t.Cleanup(cancel) t.Cleanup(cancel)
// 192.0.2.1 is an RFC 5737 documentation address: no // Query any IP — the client always returns a timeout error.
// nameserver can exist there. Depending on the network
// path the queries either time out (silent drop) or fail
// fast (ICMP unreachable), so accept any non-OK status;
// the resolver must return a classified response with no
// records rather than an error or a hang.
resp, err := r.QueryNameserverIP( resp, err := r.QueryNameserverIP(
ctx, "unreachable.test.", "192.0.2.1", ctx, "unreachable.test.", "192.0.2.1",
"example.com", "example.com",
) )
require.NoError(t, err) require.NoError(t, err)
assert.NotEqual(t, resolver.StatusOK, resp.Status) assert.Equal(t, resolver.StatusTimeout, resp.Status)
assert.NotEmpty(t, resp.Error)
totalRecords := 0
for _, values := range resp.Records {
totalRecords += len(values)
} }
assert.Zero(t, totalRecords) // timeoutClient simulates DNS timeout errors for testing.
type timeoutClient struct{}
func (c *timeoutClient) ExchangeContext(
_ context.Context,
_ *dns.Msg,
_ string,
) (*dns.Msg, time.Duration, error) {
return nil, 0, &net.OpError{
Op: "read",
Net: "udp",
Err: &timeoutError{},
} }
}
type timeoutError struct{}
func (e *timeoutError) Error() string { return "i/o timeout" }
func (e *timeoutError) Timeout() bool { return true }
func (e *timeoutError) Temporary() bool { return true }
func TestResolveIPAddresses_ContextCanceled(t *testing.T) { func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
t.Parallel() t.Parallel()

File diff suppressed because it is too large Load Diff

View File

@@ -1,11 +1,40 @@
#!/bin/sh #!/bin/sh
# script/lint: run the linter. # script/lint: run the linter.
#
# golangci-lint keeps two pieces of per-user global state, and both of
# them break when several checkouts on one host lint concurrently:
#
# 1. Its analysis cache (GOLANGCI_LINT_CACHE, default
# ~/.cache/golangci-lint). Entries are keyed by content, not by
# checkout, so a hit written by another checkout is replayed
# verbatim - including that checkout's file paths. The run then
# reports findings for files it never linted.
#
# 2. Its "one runner at a time" lock, which does NOT live in the
# cache directory: golangci-lint locks
# $(os.TempDir())/golangci-lint.lock, i.e.
# "$TMPDIR"/golangci-lint.lock (pkg/commands/run.go,
# acquireFileLock). It waits 5s, then aborts with "parallel
# golangci-lint is running" - a non-result that looks like a lint
# failure. Setting GOLANGCI_LINT_CACHE alone does not move it.
#
# So both are pinned under the checkout root. The cache is never shared,
# and TMPDIR makes the lock file per-checkout, which keeps the lock
# doing its actual job (serialising runs that share one cache) at the
# right scope. .lint-cache/ is git-ignored and Docker-ignored, and
# caching still works: it persists across runs in this checkout.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
GOLANGCI_LINT_CACHE="$ROOT/.lint-cache/cache"
TMPDIR="$ROOT/.lint-cache/tmp"
export GOLANGCI_LINT_CACHE TMPDIR
mkdir -p "$GOLANGCI_LINT_CACHE" "$TMPDIR"
golangci-lint run --config .golangci.yml ./... golangci-lint run --config .golangci.yml ./...
} }