Compare commits

..
Author SHA1 Message Date
sneak a75d4d51fd tests: rename internal/livedns to livednstest and deny it outside tests (closes #164)
check / check (push) Successful in 1m38s
The live-DNS retry and concurrency limit is only for tests, but nothing
stopped program code from importing it and compiling it into the
binary. Its directory name now ends in test, and its import path is on
the test-support deny list in .golangci.yml, so make lint fails when
program code imports it. Every import and mention is updated to the new
name.

Model: opus-5-5
2026-09-29 10:44:22 +00:00
clawbot 93c1fe15e3 golangci: re-vendor the org config with gomodguard_v2 (closes #123)
check / check (push) Successful in 1m10s
The org .golangci.yml now uses gomodguard_v2 in place of the
deprecated gomodguard, which made every lint run print a deprecation
warning. The file is copied unchanged from sneak/prompts. It also turns
on depguard with the org test-support rule, which rejects
net/http/httptest except in test files and in files under a directory
whose name ends in test. This repo's previous copy had no deny entries
of its own, so there were none to carry forward.

Model: opus-5-5
2026-09-29 10:27:07 +02:00
clawbot 6dd6043534 tests: remove the DNS stand-ins from the watcher and resolver tests (closes #159)
check / check (push) Successful in 1m21s
The watcher tests used a stand-in resolver and the resolver timeout test
a stand-in DNS client, against the rule that DNS is never mocked.
Watcher tests that look something up in DNS now run the real resolver
against live servers, each attempt on a new watcher. A DNS change is
tested by saving values live DNS never returns (names under .invalid,
192.0.2.1) in the state a check starts from, or by marking a real
nameserver failed. The timeout test queries 192.0.2.1, where nothing
answers. The live-DNS retry and concurrency limit moved from the
resolver tests to internal/livedns, so both packages share them.
NewFromLoggerWithClient had no other use and is gone. TESTING.md now
states the README's rule.

Model: opus-5-5
2026-09-29 08:44:07 +02:00
9 changed files with 117 additions and 38 deletions
+70 -2
View File
@@ -10,14 +10,20 @@ run:
linters: linters:
default: all default: all
enable:
# Successor to the deprecated gomodguard. Named explicitly, rather than
# left to `default: all`, because it carries the module policy below.
- gomodguard_v2
disable: disable:
# Genuinely incompatible with project patterns # Genuinely incompatible with project patterns
- exhaustruct # Requires all struct fields - exhaustruct # Requires all struct fields
- depguard # Dependency allow/block lists
- godot # Requires comments to end with periods - godot # Requires comments to end with periods
- wsl # Deprecated, replaced by wsl_v5
- wrapcheck # Too verbose for internal packages - wrapcheck # Too verbose for internal packages
- varnamelen # Short names like db, id are idiomatic Go - varnamelen # Short names like db, id are idiomatic Go
# Deprecated: the warning is attached to the old name, so it is
# silenced by disabling that name, not by enabling the successor.
- wsl # Deprecated, replaced by wsl_v5
- gomodguard # Deprecated, replaced by gomodguard_v2
settings: settings:
lll: lll:
line-length: 88 line-length: 88
@@ -28,6 +34,68 @@ linters:
max-complexity: 15 max-complexity: 15
dupl: dupl:
threshold: 100 threshold: 100
depguard:
# Test-support code must not be compiled into the shipped binary. A
# test-support package exists to hand a test privileges the program
# itself must never have, so a file that is not a test must not import
# one. Test files, and the files inside a package whose directory name
# ends in `test`, are where that code belongs, and are exempt.
#
# The deny list below is the one part of this file a repository is
# expected to extend, and the only part it may. depguard matches an
# import path against a list of prefixes, so it cannot be told "any path
# whose last segment ends in test"; a repository's own test-support
# packages have to be named here one at a time, by full import path,
# under a module path that differs from repository to repository. Add
# them; change nothing else.
rules:
test-support:
list-mode: lax
files:
- "$all"
- "!$test"
- "!**/*test/**"
deny:
- pkg: net/http/httptest
desc: >-
Test-support code belongs in test files and in packages whose
directory name ends in test, not in the shipped binary.
- pkg: sneak.berlin/go/dnswatcher/internal/livednstest
desc: >-
Live-DNS test support belongs in test files and in packages
whose directory name ends in test, not in the shipped binary.
# Only decisions already recorded in the Go package defaults are
# listed here. Every entry matches the module path exactly.
gomodguard_v2:
blocked:
- module: github.com/rs/zerolog
recommendations:
- log/slog
reason: "Structured logging is stdlib log/slog."
# One entry per pre-fork module path, because the later releases
# are separate paths. A prefix match would be shorter but would
# also reach github.com/go-redis/redismock, the test double for
# the successor these entries recommend.
- module: github.com/go-redis/redis
recommendations:
- github.com/redis/go-redis/v9
reason: "Pre-fork module; use the maintained go-redis v9."
- module: github.com/go-redis/redis/v7
recommendations:
- github.com/redis/go-redis/v9
reason: "Pre-fork module; use the maintained go-redis v9."
- module: github.com/go-redis/redis/v8
recommendations:
- github.com/redis/go-redis/v9
reason: "Pre-fork module; use the maintained go-redis v9."
- module: github.com/sergi/go-diff
recommendations:
- github.com/aymanbagabas/go-udiff
reason: "No unified diff output; use go-udiff."
- module: github.com/hexops/gotextdiff
recommendations:
- github.com/aymanbagabas/go-udiff
reason: "Unmaintained fork; use go-udiff."
issues: issues:
max-issues-per-linter: 0 max-issues-per-linter: 0
+2
View File
@@ -278,6 +278,8 @@ internal/
tlscheck/tlscheck.go TLS certificate inspector tlscheck/tlscheck.go TLS certificate inspector
notify/notify.go Notification service (Slack, Mattermost, ntfy) notify/notify.go Notification service (Slack, Mattermost, ntfy)
watcher/watcher.go Main monitoring orchestrator and scheduler watcher/watcher.go Main monitoring orchestrator and scheduler
livednstest/livednstest.go Retry and concurrency limit for tests
against live DNS (imported only by tests)
``` ```
### Design Principles ### Design Principles
+1 -1
View File
@@ -25,7 +25,7 @@ real servers ensures the resolver works correctly in production.
- Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) - Query timeout is calibrated to 3× maximum antipodal RTT (~300ms)
plus processing margin plus processing margin
- Root server fan-out is limited to reduce parallel query load - Root server fan-out is limited to reduce parallel query load
- Live lookups that expect an answer go through `internal/livedns`, - Live lookups that expect an answer go through `internal/livednstest`,
which limits how many run at once in a test binary and retries a which limits how many run at once in a test binary and retries a
lookup that got none lookup that got none
- Flaky failures from transient network issues are acceptable and - Flaky failures from transient network issues are acceptable and
+11 -2
View File
@@ -19,13 +19,22 @@ Rationale, Design, TODO, License, Author) if any are still missing.
# Completed Steps # Completed Steps
- 2026-09-29: the live-DNS test package is renamed `internal/livednstest` and
added to the `test-support` `deny` list in `.golangci.yml`, so `make lint`
fails when program code imports it (closes #164).
- 2026-09-29: `.golangci.yml` re-fetched unchanged from `sneak/prompts`. It
replaces the deprecated `gomodguard` with `gomodguard_v2`, so `make lint` no
longer warns about it, and turns on `depguard` with the org `test-support`
rule, which rejects `net/http/httptest` except in test files and in files
under a directory whose name ends in `test`. This repo had no `deny` entries
of its own to carry forward (closes #123).
- 2026-09-29: nothing stands in for DNS any more. Watcher tests that look - 2026-09-29: nothing stands in for DNS any more. Watcher tests that look
something up in DNS use the real resolver against live DNS servers and test something up in DNS use the real resolver against live DNS servers and test
record and nameserver changes by preparing the saved state a check starts record and nameserver changes by preparing the saved state a check starts
from; the resolver timeout test queries an address that never answers, and from; the resolver timeout test queries an address that never answers, and
`NewFromLoggerWithClient`, used only by its stand-in client, is gone. The `NewFromLoggerWithClient`, used only by its stand-in client, is gone. The
live-DNS retry and concurrency limit moved to `internal/livedns`, which both live-DNS retry and concurrency limit moved to `internal/livednstest`, which
test packages use. `TESTING.md` states the README's rule (closes #159). both test packages use. `TESTING.md` states the README's rule (closes #159).
- 2026-09-28: the inconsistency alert is sent once, on the check where two - 2026-09-28: the inconsistency alert is sent once, on the check where two
nameservers start to disagree or where a nameserver that disagrees first nameservers start to disagree or where a nameserver that disagrees first
appears, instead of on every check while they disagree, and not again after appears, instead of on every check while they disagree, and not again after
@@ -1,4 +1,4 @@
// Package livedns runs the live DNS operations of tests. Tests that // Package livednstest runs the live DNS operations of tests. Tests that
// look something up in DNS query live DNS servers, never a stand-in — // look something up in DNS query live DNS servers, never a stand-in —
// see TESTING.md. Nothing here mocks, fakes, stubs, records or replays // see TESTING.md. Nothing here mocks, fakes, stubs, records or replays
// DNS, and nothing here skips a test: it only changes *how* the live // DNS, and nothing here skips a test: it only changes *how* the live
@@ -22,7 +22,7 @@
// first attempt. A fault in the code under test that leaves // first attempt. A fault in the code under test that leaves
// nothing to check looks the same as live DNS not answering, and // nothing to check looks the same as live DNS not answering, and
// fails only after the last attempt. // fails only after the last attempt.
package livedns package livednstest
import ( import (
"context" "context"
@@ -1,4 +1,4 @@
package livedns_test package livednstest_test
import ( import (
"context" "context"
@@ -8,7 +8,7 @@ import (
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"sneak.berlin/go/dnswatcher/internal/livedns" "sneak.berlin/go/dnswatcher/internal/livednstest"
) )
// Tests for the retry and the concurrency limit themselves. They // Tests for the retry and the concurrency limit themselves. They
@@ -21,11 +21,11 @@ func TestRetryRecoversFromTransientFailure(t *testing.T) {
attempts := 0 attempts := 0
livedns.Retry(t, "transient", func(_ context.Context) error { livednstest.Retry(t, "transient", func(_ context.Context) error {
attempts++ attempts++
if attempts < wantAttempts { if attempts < wantAttempts {
return livedns.ErrNoAnswer return livednstest.ErrNoAnswer
} }
return nil return nil
@@ -37,19 +37,19 @@ func TestRetryRecoversFromTransientFailure(t *testing.T) {
func TestRetryGivesEachAttemptADeadline(t *testing.T) { func TestRetryGivesEachAttemptADeadline(t *testing.T) {
t.Parallel() t.Parallel()
livedns.Retry(t, "deadline", func(ctx context.Context) error { livednstest.Retry(t, "deadline", func(ctx context.Context) error {
deadline, ok := ctx.Deadline() deadline, ok := ctx.Deadline()
assert.True(t, ok, "attempt should carry a deadline") assert.True(t, ok, "attempt should carry a deadline")
remaining := time.Until(deadline) remaining := time.Until(deadline)
assert.LessOrEqual(t, remaining, livedns.AttemptTimeout) assert.LessOrEqual(t, remaining, livednstest.AttemptTimeout)
// Lower bound too: without one this passes for a // Lower bound too: without one this passes for a
// deadline far shorter than intended, which would // deadline far shorter than intended, which would
// silently turn every live attempt into an instant // silently turn every live attempt into an instant
// timeout. // timeout.
assert.Greater(t, remaining, livedns.AttemptTimeout/2) assert.Greater(t, remaining, livednstest.AttemptTimeout/2)
return nil return nil
}) })
@@ -73,7 +73,7 @@ func TestRunBoundsConcurrency(t *testing.T) {
go func() { go func() {
defer wg.Done() defer wg.Done()
_ = livedns.Run(func(_ context.Context) error { _ = livednstest.Run(func(_ context.Context) error {
mu.Lock() mu.Lock()
inFlight++ inFlight++
@@ -97,7 +97,7 @@ func TestRunBoundsConcurrency(t *testing.T) {
assert.Positive(t, maxSeen) assert.Positive(t, maxSeen)
assert.LessOrEqual( assert.LessOrEqual(
t, maxSeen, livedns.Concurrency, t, maxSeen, livednstest.Concurrency,
"live queries must stay under the package-wide gate", "live queries must stay under the package-wide gate",
) )
} }
+14 -14
View File
@@ -9,7 +9,7 @@ import (
"strings" "strings"
"testing" "testing"
"sneak.berlin/go/dnswatcher/internal/livedns" "sneak.berlin/go/dnswatcher/internal/livednstest"
"sneak.berlin/go/dnswatcher/internal/resolver" "sneak.berlin/go/dnswatcher/internal/resolver"
) )
@@ -20,9 +20,9 @@ import (
// Tests that look something up in DNS query live DNS servers, never a // Tests that look something up in DNS query live DNS servers, never a
// stand-in; logic that works on record data may be tested on that // stand-in; logic that works on record data may be tested on that
// data with no lookup (see TESTING.md). Each live operation below goes // data with no lookup (see TESTING.md). Each live operation below goes
// through livedns.Retry, which bounds how many resolutions are in // through livednstest.Retry, which bounds how many resolutions are in
// flight at once and retries an operation that got no answer (see // flight at once and retries an operation that got no answer (see
// package livedns). // package livednstest).
// //
// Where an assertion spans several independent nameservers, a quorum // Where an assertion spans several independent nameservers, a quorum
// is enough: a strict majority answering as expected. A server that // is enough: a strict majority answering as expected. A server that
@@ -162,7 +162,7 @@ func liveFindAuthoritative(
var out []string var out []string
livedns.Retry( livednstest.Retry(
t, t,
"FindAuthoritativeNameservers("+domain+")", "FindAuthoritativeNameservers("+domain+")",
func(ctx context.Context) error { func(ctx context.Context) error {
@@ -174,7 +174,7 @@ func liveFindAuthoritative(
if len(ns) == 0 { if len(ns) == 0 {
return fmt.Errorf( return fmt.Errorf(
"%w: %s has no nameservers", "%w: %s has no nameservers",
livedns.ErrNoAnswer, domain, livednstest.ErrNoAnswer, domain,
) )
} }
@@ -198,7 +198,7 @@ func liveLookupNS(
var out []string var out []string
livedns.Retry( livednstest.Retry(
t, t,
"LookupNS("+domain+")", "LookupNS("+domain+")",
func(ctx context.Context) error { func(ctx context.Context) error {
@@ -210,7 +210,7 @@ func liveLookupNS(
if len(ns) == 0 { if len(ns) == 0 {
return fmt.Errorf( return fmt.Errorf(
"%w: %s has no nameservers", "%w: %s has no nameservers",
livedns.ErrNoAnswer, domain, livednstest.ErrNoAnswer, domain,
) )
} }
@@ -240,7 +240,7 @@ func liveQueryNameserver(
var out *resolver.NameserverResponse var out *resolver.NameserverResponse
livedns.Retry( livednstest.Retry(
t, t,
what, what,
func(ctx context.Context) error { func(ctx context.Context) error {
@@ -255,7 +255,7 @@ func liveQueryNameserver(
resp.Status == resolver.StatusError { resp.Status == resolver.StatusError {
return fmt.Errorf( return fmt.Errorf(
"%w: %s returned %s: %s", "%w: %s returned %s: %s",
livedns.ErrNoAnswer, nameserver, livednstest.ErrNoAnswer, nameserver,
resp.Status, resp.Error, resp.Status, resp.Error,
) )
} }
@@ -282,7 +282,7 @@ func liveQueryAllNameservers(
var out map[string]*resolver.NameserverResponse var out map[string]*resolver.NameserverResponse
livedns.Retry( livednstest.Retry(
t, t,
"QueryAllNameservers("+hostname+")", "QueryAllNameservers("+hostname+")",
func(ctx context.Context) error { func(ctx context.Context) error {
@@ -294,7 +294,7 @@ func liveQueryAllNameservers(
if len(results) == 0 { if len(results) == 0 {
return fmt.Errorf( return fmt.Errorf(
"%w: no nameservers queried for %s", "%w: no nameservers queried for %s",
livedns.ErrNoAnswer, hostname, livednstest.ErrNoAnswer, hostname,
) )
} }
@@ -327,7 +327,7 @@ func liveResolveIPs(
var out []string var out []string
livedns.Retry( livednstest.Retry(
t, t,
"ResolveIPAddresses("+hostname+")", "ResolveIPAddresses("+hostname+")",
func(ctx context.Context) error { func(ctx context.Context) error {
@@ -339,7 +339,7 @@ func liveResolveIPs(
if len(ips) == 0 { if len(ips) == 0 {
return fmt.Errorf( return fmt.Errorf(
"%w: no addresses for %s", "%w: no addresses for %s",
livedns.ErrNoAnswer, hostname, livednstest.ErrNoAnswer, hostname,
) )
} }
@@ -366,7 +366,7 @@ func liveResolveIPsAllowingEmpty(
var out []string var out []string
livedns.Retry( livednstest.Retry(
t, t,
"ResolveIPAddresses("+hostname+")", "ResolveIPAddresses("+hostname+")",
func(ctx context.Context) error { func(ctx context.Context) error {
+5 -5
View File
@@ -33,7 +33,7 @@ func newTestResolver(t *testing.T) *resolver.Resolver {
// findOneNSForDomain picks one authoritative nameserver to aim a // findOneNSForDomain picks one authoritative nameserver to aim a
// test at. Quorum handling lives in livedns_test.go, and the live-DNS // test at. Quorum handling lives in livedns_test.go, and the live-DNS
// retry and concurrency limit in package livedns. // retry and concurrency limit in package livednstest.
func findOneNSForDomain( func findOneNSForDomain(
t *testing.T, t *testing.T,
r *resolver.Resolver, r *resolver.Resolver,
@@ -528,10 +528,10 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
r := newTestResolver(t) r := newTestResolver(t)
// Nothing answers at 192.0.2.1, a documentation address. The // Nothing answers at 192.0.2.1, a documentation address. The
// resolver tries each query twice; if the deadline has passed // resolver tries each query twice, and the first try gives up
// before the second try starts, the query is reported as nodata, // after two seconds. A deadline that ends during the first try
// not timeout. So the deadline must outlast the first try's // makes the status vary from run to run between nodata and
// two-second timeout. // timeout, so the deadline must outlast the first try.
ctx, cancel := context.WithTimeout( ctx, cancel := context.WithTimeout(
context.Background(), 3*time.Second, context.Background(), 3*time.Second,
) )
+3 -3
View File
@@ -10,7 +10,7 @@ import (
"time" "time"
"sneak.berlin/go/dnswatcher/internal/config" "sneak.berlin/go/dnswatcher/internal/config"
"sneak.berlin/go/dnswatcher/internal/livedns" "sneak.berlin/go/dnswatcher/internal/livednstest"
"sneak.berlin/go/dnswatcher/internal/portcheck" "sneak.berlin/go/dnswatcher/internal/portcheck"
"sneak.berlin/go/dnswatcher/internal/resolver" "sneak.berlin/go/dnswatcher/internal/resolver"
"sneak.berlin/go/dnswatcher/internal/state" "sneak.berlin/go/dnswatcher/internal/state"
@@ -195,7 +195,7 @@ func checkOnce(
return fmt.Errorf( return fmt.Errorf(
"%s: %w, or the watcher saved no fresh "+ "%s: %w, or the watcher saved no fresh "+
"result for it", "result for it",
name, livedns.ErrNoAnswer, name, livednstest.ErrNoAnswer,
) )
} }
} }
@@ -219,7 +219,7 @@ func runChecks(
var deps *testDeps var deps *testDeps
livedns.Retry(t, "watcher checks", func(ctx context.Context) error { livednstest.Retry(t, "watcher checks", func(ctx context.Context) error {
var w *watcher.Watcher var w *watcher.Watcher
w, deps = newTestWatcher(t, cfg) w, deps = newTestWatcher(t, cfg)