Compare commits

..
1 Commits
Author SHA1 Message Date
clawbot d7eeacbd35 tests: remove the DNS stand-ins from the watcher and resolver tests (closes #159)
check / check (push) Successful in 1m18s
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. The
watcher tests now run the real resolver against live DNS. A 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 and
the DNSClient comment now state the README's rule.

Model: opus-5-5
2026-09-29 02:28:48 +00:00
7 changed files with 115 additions and 185 deletions
+2 -66
View File
@@ -10,20 +10,14 @@ 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
@@ -34,64 +28,6 @@ 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.
# 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,8 +278,6 @@ 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
livedns/livedns.go Retry and concurrency limit for tests
against live DNS (imported only by tests)
``` ```
### Design Principles ### Design Principles
+13 -16
View File
@@ -10,7 +10,11 @@
# Status # Status
pre-1.0. No git tags. pre-1.0. No git tags. Core resolver work in flight on feature/resolver
(dirty: internal/resolver/resolver_test.go). Local checkout has diverged
from origin: origin/main is 8 commits ahead (watcher orchestrator,
unified TARGETS) and origin/feature/resolver already contains the full
iterative resolver implementation with hermetic mocked tests.
# Next Step # Next Step
@@ -19,19 +23,13 @@ Rationale, Design, TODO, License, Author) if any are still missing.
# Completed Steps # Completed Steps
- 2026-09-29: `.golangci.yml` re-fetched unchanged from `sneak/prompts`. It - 2026-09-29: nothing stands in for DNS any more. The watcher tests use the
replaces the deprecated `gomodguard` with `gomodguard_v2`, so `make lint` no real resolver against live DNS and test changes by preparing the saved
longer warns about it, and turns on `depguard` with the org `test-support` state a check starts from; the resolver timeout test queries an address
rule, which rejects `net/http/httptest` except in test files and in files that never answers, and `NewFromLoggerWithClient`, used only by its
under a directory whose name ends in `test`. This repo had no `deny` entries stand-in client, is gone. The live-DNS retry and concurrency limit moved
of its own to carry forward (closes #123). to `internal/livedns`, which both test packages use. `TESTING.md` states
- 2026-09-29: nothing stands in for DNS any more. Watcher tests that look the README's rule (closes #159).
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
from; the resolver timeout test queries an address that never answers, and
`NewFromLoggerWithClient`, used only by its stand-in client, is gone. The
live-DNS retry and concurrency limit moved to `internal/livedns`, which 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
@@ -270,5 +268,4 @@ 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
- DNS is never mocked; tests that look something up in DNS query live DNS - Tests use live DNS and never mock it (README, "No DNS mocking. Ever.")
servers (README, "No DNS mocking. Ever.")
+7 -9
View File
@@ -1,5 +1,5 @@
// Package livedns runs the live DNS operations of tests. Tests that // Package livedns runs the live DNS operations of tests. Every test in
// look something up in DNS query live DNS servers, never a stand-in — // this project that needs DNS resolves against the real, live DNS —
// 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
// queries are issued, so that a single dropped UDP packet or one slow // queries are issued, so that a single dropped UDP packet or one slow
@@ -16,12 +16,10 @@
// operations are in flight at once in one test binary. // operations are in flight at once in one test binary.
// //
// 2. Retry with exponential backoff. Each live operation gets several // 2. Retry with exponential backoff. Each live operation gets several
// attempts with its own timeout. An attempt is retried when it // attempts with its own timeout. The retry condition is strictly
// obtained nothing to check, never because of what the test // transport-level — "did a nameserver answer at all" — never the
// asserts about the result, so a wrong result still fails on the // assertion the test is making. Code that answers incorrectly
// first attempt. A fault in the code under test that leaves // still fails on the first attempt.
// nothing to check looks the same as live DNS not answering, and
// fails only after the last attempt.
package livedns package livedns
import ( import (
@@ -115,7 +113,7 @@ func Retry(
} }
t.Fatalf( t.Fatalf(
"%s: all %d live attempts failed: %v", "%s: no answer after %d live attempts: %v",
what, attempts, last, what, attempts, last,
) )
} }
+5 -6
View File
@@ -17,12 +17,11 @@ import (
// Live DNS test support // Live DNS test support
// ---------------------------------------------------------------- // ----------------------------------------------------------------
// //
// Tests that look something up in DNS query live DNS servers, never a // Every test in this package resolves against the real, live DNS —
// stand-in; logic that works on record data may be tested on that // see TESTING.md. Each live operation below goes through
// data with no lookup (see TESTING.md). Each live operation below goes // livedns.Retry, which bounds how many resolutions are in flight at
// through livedns.Retry, which bounds how many resolutions are in // once and retries an operation that got no answer (see package
// flight at once and retries an operation that got no answer (see // livedns).
// package livedns).
// //
// 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
+6 -6
View File
@@ -32,8 +32,8 @@ 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. Live-DNS retry, concurrency and quorum handling live in
// retry and concurrency limit in package livedns. // livedns_test.go.
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, and the first try gives up // resolver tries each query twice; if the deadline has passed
// after two seconds. A deadline that ends during the first try // before the second try starts, the query is reported as nodata,
// makes the status vary from run to run between nodata and // not timeout. So the deadline must outlast the first try's
// timeout, so the deadline must outlast the first try. // two-second timeout.
ctx, cancel := context.WithTimeout( ctx, cancel := context.WithTimeout(
context.Background(), 3*time.Second, context.Background(), 3*time.Second,
) )
+65 -63
View File
@@ -19,10 +19,9 @@ import (
) )
// The watcher looks these names up in live DNS with the real resolver, // The watcher looks these names up in live DNS with the real resolver,
// so tests assert on what the watcher does with the answers, never on // so tests assert on notifications and saved state, never on the
// the records these zones publish. testHost's nameservers and addresses // records these zones publish. testHost's addresses stay the same from
// stay the same from one check to the next, which the tests that check // one check to the next, which the tests that check it twice rely on.
// it twice rely on.
const ( const (
testDomain = "google.com" testDomain = "google.com"
testHost = "cloudflare.com" testHost = "cloudflare.com"
@@ -39,8 +38,7 @@ const (
// --- Stand-ins for the port checker, TLS checker and notifier --- // --- Stand-ins for the port checker, TLS checker and notifier ---
// //
// DNS has none: the watchers built here use the real resolver (see // DNS has none: the watcher uses the real resolver (see TESTING.md).
// TESTING.md).
// mockPortChecker reports every port open until closed is set. // mockPortChecker reports every port open until closed is set.
type mockPortChecker struct { type mockPortChecker struct {
@@ -173,10 +171,11 @@ func defaultTestConfig(t *testing.T) *config.Config {
} }
} }
// checkOnce runs the watcher's checks once and returns an error when a // checkOnce runs the watcher's checks once and returns
// configured name has no hostname state saved by this check, or that // livedns.ErrNoAnswer when live DNS did not answer for a configured
// state holds no address. Either live DNS gave no answer for the name, // name. The watcher saves a name's hostname state only when all of the
// or the watcher saved no fresh result for it. // name's lookups succeed, so live DNS answered for a name when this
// check saved its hostname state and that state holds an address.
func checkOnce( func checkOnce(
ctx context.Context, ctx context.Context,
w *watcher.Watcher, w *watcher.Watcher,
@@ -192,53 +191,53 @@ func checkOnce(
hs, ok := deps.state.GetHostnameState(name) hs, ok := deps.state.GetHostnameState(name)
if !ok || hs.LastChecked.Before(started) || if !ok || hs.LastChecked.Before(started) ||
len(addresses(hs)) == 0 { len(addresses(hs)) == 0 {
return fmt.Errorf( return fmt.Errorf("%w: %s", livedns.ErrNoAnswer, name)
"%s: %w, or the watcher saved no fresh "+
"result for it",
name, livedns.ErrNoAnswer,
)
} }
} }
return nil return nil
} }
// runChecks builds a watcher, lets prepare set up the saved state and // runFirstCheck builds a watcher, lets prepare set up the saved state
// stand-ins it starts from, and runs its checks once against live DNS. // and stand-ins it starts from, and runs its checks once against live
// If change is not nil, change then alters the saved state or stand-ins // DNS. When live DNS does not answer, the watcher is thrown away and
// and the checks run a second time. When either check finds no fresh // built again, so a failed attempt leaves nothing behind in the state
// address for a name (see checkOnce), the watcher is thrown away and // or the notifications.
// all of this runs again on a new one, so a failed attempt leaves func runFirstCheck(
// nothing behind in the saved state, the stand-ins or the notifications.
func runChecks(
t *testing.T, t *testing.T,
cfg *config.Config, cfg *config.Config,
prepare, change func(deps *testDeps), prepare func(deps *testDeps),
) *testDeps { ) (*watcher.Watcher, *testDeps) {
t.Helper() t.Helper()
var deps *testDeps var (
w *watcher.Watcher
livedns.Retry(t, "watcher checks", func(ctx context.Context) error { deps *testDeps
var w *watcher.Watcher )
livedns.Retry(t, "first check", func(ctx context.Context) error {
w, deps = newTestWatcher(t, cfg) w, deps = newTestWatcher(t, cfg)
if prepare != nil { if prepare != nil {
prepare(deps) prepare(deps)
} }
err := checkOnce(ctx, w, deps)
if err != nil || change == nil {
return err
}
change(deps)
return checkOnce(ctx, w, deps) return checkOnce(ctx, w, deps)
}) })
return deps return w, deps
}
// runCheck runs the watcher's checks once more against live DNS,
// repeating them while live DNS does not answer. A failed lookup keeps
// the name's saved records, so a repeat compares against the same
// saved state.
func runCheck(t *testing.T, w *watcher.Watcher, deps *testDeps) {
t.Helper()
livedns.Retry(t, "check", func(ctx context.Context) error {
return checkOnce(ctx, w, deps)
})
} }
// addresses returns the A and AAAA values saved for a hostname. // addresses returns the A and AAAA values saved for a hostname.
@@ -296,7 +295,7 @@ func TestFirstRunBaseline(t *testing.T) {
cfg.Domains = []string{testDomain} cfg.Domains = []string{testDomain}
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
deps := runChecks(t, cfg, nil, nil) _, deps := runFirstCheck(t, cfg, nil)
assertNoNotifications(t, deps) assertNoNotifications(t, deps)
assertStatePopulated(t, deps) assertStatePopulated(t, deps)
@@ -348,7 +347,7 @@ func TestDomainPortAndTLSChecks(t *testing.T) {
cfg := defaultTestConfig(t) cfg := defaultTestConfig(t)
cfg.Domains = []string{testDomain} cfg.Domains = []string{testDomain}
deps := runChecks(t, cfg, nil, nil) _, deps := runFirstCheck(t, cfg, nil)
snap := deps.state.GetSnapshot() snap := deps.state.GetSnapshot()
@@ -388,11 +387,11 @@ func TestNSChangeDetection(t *testing.T) {
cfg.Domains = []string{testDomain} cfg.Domains = []string{testDomain}
// The saved state lists nameservers that live DNS does not. // The saved state lists nameservers that live DNS does not.
deps := runChecks(t, cfg, func(deps *testDeps) { _, deps := runFirstCheck(t, cfg, func(deps *testDeps) {
deps.state.SetDomainState(testDomain, &state.DomainState{ deps.state.SetDomainState(testDomain, &state.DomainState{
Nameservers: []string{oldNS1, oldNS2}, Nameservers: []string{oldNS1, oldNS2},
}) })
}, nil) })
assertNotified(t, deps, "NS Change: "+testDomain, "warning") assertNotified(t, deps, "NS Change: "+testDomain, "warning")
@@ -408,16 +407,17 @@ func TestRecordChangeDetection(t *testing.T) {
cfg := defaultTestConfig(t) cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
// Between the checks, save for every nameserver an address live DNS w, deps := runFirstCheck(t, cfg, nil)
// never returns.
deps := runChecks(t, cfg, nil, func(deps *testDeps) { // Save, for every nameserver, an address live DNS never returns.
hs, _ := deps.state.GetHostnameState(testHost) hs, _ := deps.state.GetHostnameState(testHost)
for _, nsState := range hs.RecordsByNameserver { for _, nsState := range hs.RecordsByNameserver {
nsState.Records = map[string][]string{"A": {oldIP}} nsState.Records = map[string][]string{"A": {oldIP}}
} }
deps.state.SetHostnameState(testHost, hs) deps.state.SetHostnameState(testHost, hs)
})
runCheck(t, w, deps)
assertNotified(t, deps, "Record Change: "+testHost, "warning") assertNotified(t, deps, "Record Change: "+testHost, "warning")
} }
@@ -428,12 +428,13 @@ func TestPortStateChange(t *testing.T) {
cfg := defaultTestConfig(t) cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
// Between the checks, every port closes. w, deps := runFirstCheck(t, cfg, nil)
deps := runChecks(t, cfg, nil, func(deps *testDeps) {
deps.portChecker.mu.Lock() deps.portChecker.mu.Lock()
deps.portChecker.closed = true deps.portChecker.closed = true
deps.portChecker.mu.Unlock() deps.portChecker.mu.Unlock()
})
runCheck(t, w, deps)
hs, _ := deps.state.GetHostnameState(testHost) hs, _ := deps.state.GetHostnameState(testHost)
assertNotified( assertNotified(
@@ -453,7 +454,7 @@ func TestTLSExpiryWarning(t *testing.T) {
cfg := defaultTestConfig(t) cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
deps := runChecks(t, cfg, expiresInThreeDays, nil) _, deps := runFirstCheck(t, cfg, expiresInThreeDays)
assertNotified(t, deps, "TLS Expiry Warning: "+testHost, "warning") assertNotified(t, deps, "TLS Expiry Warning: "+testHost, "warning")
} }
@@ -465,20 +466,19 @@ func TestTLSExpiryWarningDedup(t *testing.T) {
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
cfg.TLSInterval = 24 * time.Hour cfg.TLSInterval = 24 * time.Hour
w, deps := runFirstCheck(t, cfg, expiresInThreeDays)
title := "TLS Expiry Warning: " + testHost title := "TLS Expiry Warning: " + testHost
// The second check comes within the TLS interval of the first, warnings := countNotifications(deps, title)
// so it must not warn again.
var warnings int
deps := runChecks(t, cfg, expiresInThreeDays, func(deps *testDeps) {
warnings = countNotifications(deps, title)
})
if warnings == 0 { if warnings == 0 {
t.Fatal("expected expiry warnings from the first check") t.Fatal("expected expiry warnings from the first check")
} }
// The second check comes within the TLS interval of the first,
// so it must not warn again.
runCheck(t, w, deps)
got := countNotifications(deps, title) got := countNotifications(deps, title)
if got != warnings { if got != warnings {
t.Errorf( t.Errorf(
@@ -525,7 +525,7 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
// The saved state says the last check found testHost at oldIP. // The saved state says the last check found testHost at oldIP.
deps := runChecks(t, cfg, func(deps *testDeps) { _, deps := runFirstCheck(t, cfg, func(deps *testDeps) {
deps.state.SetHostnameState(testHost, &state.HostnameState{ deps.state.SetHostnameState(testHost, &state.HostnameState{
RecordsByNameserver: map[string]*state.NameserverRecordState{ RecordsByNameserver: map[string]*state.NameserverRecordState{
oldNS1: { oldNS1: {
@@ -534,7 +534,7 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
}, },
}, },
}) })
}, nil) })
snap := deps.state.GetSnapshot() snap := deps.state.GetSnapshot()
@@ -665,9 +665,10 @@ func TestNSFailureAndRecovery(t *testing.T) {
cfg := defaultTestConfig(t) cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
// Between the checks, save every nameserver the first check found w, deps := runFirstCheck(t, cfg, nil)
// as failed, and add, as answering, one that live DNS does not list.
deps := runChecks(t, cfg, nil, func(deps *testDeps) { // Save every nameserver the first check found as failed, and add
// one that live DNS does not list as having answered.
hs, _ := deps.state.GetHostnameState(testHost) hs, _ := deps.state.GetHostnameState(testHost)
for _, nsState := range hs.RecordsByNameserver { for _, nsState := range hs.RecordsByNameserver {
nsState.Status = "error" nsState.Status = "error"
@@ -679,7 +680,8 @@ func TestNSFailureAndRecovery(t *testing.T) {
} }
deps.state.SetHostnameState(testHost, hs) deps.state.SetHostnameState(testHost, hs)
})
runCheck(t, w, deps)
assertNotified(t, deps, "NS Failure: "+testHost, "error") assertNotified(t, deps, "NS Failure: "+testHost, "error")
assertNotified(t, deps, "NS Recovery: "+testHost, "success") assertNotified(t, deps, "NS Recovery: "+testHost, "success")