Compare commits

..
Author SHA1 Message Date
clawbot ab02a8663a 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 01:00:44 +00:00
10 changed files with 137 additions and 220 deletions
+2 -66
View File
@@ -10,20 +10,14 @@ run:
linters:
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:
# Genuinely incompatible with project patterns
- exhaustruct # Requires all struct fields
- depguard # Dependency allow/block lists
- godot # Requires comments to end with periods
- wsl # Deprecated, replaced by wsl_v5
- wrapcheck # Too verbose for internal packages
- 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:
lll:
line-length: 88
@@ -34,64 +28,6 @@ linters:
max-complexity: 15
dupl:
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:
max-issues-per-linter: 0
+10 -8
View File
@@ -41,15 +41,18 @@ RUN make build
# alpine 3.21, 2026-02-28
FROM alpine@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709
RUN apk add --no-cache ca-certificates tzdata su-exec
RUN apk add --no-cache ca-certificates tzdata
COPY --from=builder /src/bin/dnswatcher /usr/local/bin/dnswatcher
COPY deploy/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
# dnswatcher runs as this unprivileged user. The entrypoint creates the
# data directory and gives it to this user on every start.
# Run as an unprivileged user that owns the data directory. A fresh named
# volume inherits this ownership; a bind-mounted host directory must be
# owned by uid 10001 (see "Running under upaas" in README.md), or startup
# fails.
RUN addgroup -S -g 10001 dnswatcher \
&& adduser -S -G dnswatcher -u 10001 dnswatcher
&& adduser -S -G dnswatcher -u 10001 dnswatcher \
&& mkdir -p /var/lib/dnswatcher \
&& chown dnswatcher:dnswatcher /var/lib/dnswatcher
ENV DNSWATCHER_DATA_DIR=/var/lib/dnswatcher
@@ -59,8 +62,7 @@ ENV DNSWATCHER_DATA_DIR=/var/lib/dnswatcher
# data directory, or the binary's directory, the working directory.
WORKDIR /
# No USER: the entrypoint must start as root to set up the data
# directory; it then runs dnswatcher as the dnswatcher user.
USER dnswatcher
EXPOSE 8080
@@ -70,4 +72,4 @@ EXPOSE 8080
HEALTHCHECK --interval=10s --timeout=5s --start-period=10s --retries=3 \
CMD wget -q -O /dev/null "http://127.0.0.1:${PORT:-8080}/.well-known/healthcheck" || exit 1
ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
ENTRYPOINT ["/usr/local/bin/dnswatcher"]
+11 -3
View File
@@ -278,8 +278,6 @@ internal/
tlscheck/tlscheck.go TLS certificate inspector
notify/notify.go Notification service (Slack, Mattermost, ntfy)
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
@@ -533,7 +531,17 @@ repository's `Dockerfile` and runs it. The app needs:
- **Branch:** `prod`. `prod` is cut from `main`, and merging a `main` to
`prod` pull request is a deploy.
- **Volume:** one host directory mounted at `/var/lib/dnswatcher`, where
the state file lives.
the state file lives. upaas bind-mounts the host path it is given and
does not create it. The container runs as uid 10001 and does not start
unless it can write there. Create the directory before the first
deploy:
```sh
mkdir -p /path/to/data
chown 10001:10001 /path/to/data
chmod 700 /path/to/data
```
- **Network and port:** the dashboard is unauthenticated and shows every
watched name and recent alert, and upaas publishes every mapped port on
all interfaces of the host
+3 -4
View File
@@ -4,10 +4,9 @@
DNS is never mocked in this project, not in tests and not anywhere
else; see the README section "No DNS mocking. Ever." Every test that
looks something up in DNS **MUST** query live DNS servers, never a
stand-in. Logic that works on record data, such as comparing or
formatting records, may be tested on that data directly with no
lookup.
involves DNS **MUST** use live queries against real DNS servers: the
resolver's tests, and the tests of code that uses the resolver, such
as the watcher.
### Rationale
+13 -21
View File
@@ -10,7 +10,11 @@
# 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
@@ -19,24 +23,13 @@ Rationale, Design, TODO, License, Author) if any are still missing.
# Completed Steps
- 2026-09-29: the image sets up its own data directory. Its entrypoint,
`deploy/docker-entrypoint.sh`, starts as root, creates the data directory if
needed, gives it and everything in it to the `dnswatcher` user with mode 700
on the directory, then runs dnswatcher as that user with `su-exec`. A
bind-mounted host directory no longer has to be chowned first (closes #166).
- 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
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-29: nothing stands in for DNS any more. The watcher tests use the
real resolver against live DNS and test 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
nameservers start to disagree or where a nameserver that disagrees first
appears, instead of on every check while they disagree, and not again after
@@ -275,5 +268,4 @@ Infrastructure notes (from untracked TODO.md):
- Module path sneak.berlin/go/dnswatcher differs from the git.eeqj.de
remote intentionally; do not "fix" it
- Dependencies: github.com/miekg/dns, golang.org/x/net/publicsuffix
- DNS is never mocked; tests that look something up in DNS query live DNS
servers (README, "No DNS mocking. Ever.")
- Tests use live DNS and never mock it (README, "No DNS mocking. Ever.")
-17
View File
@@ -1,17 +0,0 @@
#!/bin/sh
# deploy/docker-entrypoint.sh: the Docker image's ENTRYPOINT. It runs as
# root only to give the data directory to the dnswatcher user: a host
# directory bind-mounted there keeps its host owner, often root, and may
# hold a state file left by another uid, which dnswatcher could neither
# read nor replace. dnswatcher itself always runs as the dnswatcher user.
set -eu
main() {
dir="${DNSWATCHER_DATA_DIR:-/var/lib/dnswatcher}"
mkdir -p "$dir"
chown -R dnswatcher:dnswatcher "$dir"
chmod 700 "$dir"
exec su-exec dnswatcher /usr/local/bin/dnswatcher "$@"
}
main "$@"
+7 -9
View File
@@ -1,5 +1,5 @@
// Package livedns runs the live DNS operations of tests. Tests that
// look something up in DNS query live DNS servers, never a stand-in —
// Package livedns runs the live DNS operations of tests. Every test in
// this project that needs DNS resolves against the real, live DNS —
// see TESTING.md. Nothing here mocks, fakes, stubs, records or replays
// 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
@@ -16,12 +16,10 @@
// operations are in flight at once in one test binary.
//
// 2. Retry with exponential backoff. Each live operation gets several
// attempts with its own timeout. An attempt is retried when it
// obtained nothing to check, never because of what the test
// asserts about the result, so a wrong result still fails on the
// first attempt. A fault in the code under test that leaves
// nothing to check looks the same as live DNS not answering, and
// fails only after the last attempt.
// attempts with its own timeout. The retry condition is strictly
// transport-level — "did a nameserver answer at all" — never the
// assertion the test is making. Code that answers incorrectly
// still fails on the first attempt.
package livedns
import (
@@ -115,7 +113,7 @@ func Retry(
}
t.Fatalf(
"%s: all %d live attempts failed: %v",
"%s: no answer after %d live attempts: %v",
what, attempts, last,
)
}
+5 -6
View File
@@ -17,12 +17,11 @@ import (
// Live DNS test support
// ----------------------------------------------------------------
//
// 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
// data with no lookup (see TESTING.md). Each live operation below goes
// through livedns.Retry, which bounds how many resolutions are in
// flight at once and retries an operation that got no answer (see
// package livedns).
// Every test in this package resolves against the real, live DNS —
// see TESTING.md. Each live operation below goes through
// livedns.Retry, which bounds how many resolutions are in flight at
// once and retries an operation that got no answer (see package
// livedns).
//
// Where an assertion spans several independent nameservers, a quorum
// is enough: a strict majority answering as expected. A server that
+4 -6
View File
@@ -32,8 +32,8 @@ func newTestResolver(t *testing.T) *resolver.Resolver {
}
// findOneNSForDomain picks one authoritative nameserver to aim a
// test at. Quorum handling lives in livedns_test.go, and the live-DNS
// retry and concurrency limit in package livedns.
// test at. Live-DNS retry, concurrency and quorum handling live in
// livedns_test.go.
func findOneNSForDomain(
t *testing.T,
r *resolver.Resolver,
@@ -528,10 +528,8 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
r := newTestResolver(t)
// Nothing answers at 192.0.2.1, a documentation address. The
// resolver tries each query twice, and the first try gives up
// after two seconds. A deadline that ends during the first try
// makes the status vary from run to run between nodata and
// timeout, so the deadline must outlast the first try.
// deadline must outlast one query timeout (two seconds): a query
// cut short by the deadline itself is not reported as a timeout.
ctx, cancel := context.WithTimeout(
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,
// so tests assert on what the watcher does with the answers, never on
// the records these zones publish. testHost's nameservers and addresses
// stay the same from one check to the next, which the tests that check
// it twice rely on.
// so tests assert on notifications and saved state, never on the
// records these zones publish. testHost's addresses stay the same from
// one check to the next, which the tests that check it twice rely on.
const (
testDomain = "google.com"
testHost = "cloudflare.com"
@@ -39,8 +38,7 @@ const (
// --- Stand-ins for the port checker, TLS checker and notifier ---
//
// DNS has none: the watchers built here use the real resolver (see
// TESTING.md).
// DNS has none: the watcher uses the real resolver (see TESTING.md).
// mockPortChecker reports every port open until closed is set.
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
// configured name has no hostname state saved by this check, or that
// state holds no address. Either live DNS gave no answer for the name,
// or the watcher saved no fresh result for it.
// checkOnce runs the watcher's checks once and returns
// livedns.ErrNoAnswer when live DNS did not answer for a configured
// name. The watcher saves a name's hostname state only when all of the
// 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(
ctx context.Context,
w *watcher.Watcher,
@@ -192,53 +191,53 @@ func checkOnce(
hs, ok := deps.state.GetHostnameState(name)
if !ok || hs.LastChecked.Before(started) ||
len(addresses(hs)) == 0 {
return fmt.Errorf(
"%s: %w, or the watcher saved no fresh "+
"result for it",
name, livedns.ErrNoAnswer,
)
return fmt.Errorf("%w: %s", livedns.ErrNoAnswer, name)
}
}
return nil
}
// runChecks builds a watcher, lets prepare set up the saved state and
// stand-ins it starts from, and runs its checks once against live DNS.
// If change is not nil, change then alters the saved state or stand-ins
// and the checks run a second time. When either check finds no fresh
// address for a name (see checkOnce), the watcher is thrown away and
// all of this runs again on a new one, so a failed attempt leaves
// nothing behind in the saved state, the stand-ins or the notifications.
func runChecks(
// runFirstCheck builds a watcher, lets prepare set up the saved state
// and stand-ins it starts from, and runs its checks once against live
// DNS. When live DNS does not answer, the watcher is thrown away and
// built again, so a failed attempt leaves nothing behind in the state
// or the notifications.
func runFirstCheck(
t *testing.T,
cfg *config.Config,
prepare, change func(deps *testDeps),
) *testDeps {
prepare func(deps *testDeps),
) (*watcher.Watcher, *testDeps) {
t.Helper()
var deps *testDeps
livedns.Retry(t, "watcher checks", func(ctx context.Context) error {
var w *watcher.Watcher
var (
w *watcher.Watcher
deps *testDeps
)
livedns.Retry(t, "first check", func(ctx context.Context) error {
w, deps = newTestWatcher(t, cfg)
if prepare != nil {
prepare(deps)
}
err := checkOnce(ctx, w, deps)
if err != nil || change == nil {
return err
}
change(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.
@@ -296,7 +295,7 @@ func TestFirstRunBaseline(t *testing.T) {
cfg.Domains = []string{testDomain}
cfg.Hostnames = []string{testHost}
deps := runChecks(t, cfg, nil, nil)
_, deps := runFirstCheck(t, cfg, nil)
assertNoNotifications(t, deps)
assertStatePopulated(t, deps)
@@ -348,7 +347,7 @@ func TestDomainPortAndTLSChecks(t *testing.T) {
cfg := defaultTestConfig(t)
cfg.Domains = []string{testDomain}
deps := runChecks(t, cfg, nil, nil)
_, deps := runFirstCheck(t, cfg, nil)
snap := deps.state.GetSnapshot()
@@ -388,11 +387,11 @@ func TestNSChangeDetection(t *testing.T) {
cfg.Domains = []string{testDomain}
// 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{
Nameservers: []string{oldNS1, oldNS2},
})
}, nil)
})
assertNotified(t, deps, "NS Change: "+testDomain, "warning")
@@ -408,16 +407,17 @@ func TestRecordChangeDetection(t *testing.T) {
cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost}
// Between the checks, save for every nameserver an address live DNS
// never returns.
deps := runChecks(t, cfg, nil, func(deps *testDeps) {
w, deps := runFirstCheck(t, cfg, nil)
// Save, for every nameserver, an address live DNS never returns.
hs, _ := deps.state.GetHostnameState(testHost)
for _, nsState := range hs.RecordsByNameserver {
nsState.Records = map[string][]string{"A": {oldIP}}
}
deps.state.SetHostnameState(testHost, hs)
})
runCheck(t, w, deps)
assertNotified(t, deps, "Record Change: "+testHost, "warning")
}
@@ -428,12 +428,13 @@ func TestPortStateChange(t *testing.T) {
cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost}
// Between the checks, every port closes.
deps := runChecks(t, cfg, nil, func(deps *testDeps) {
w, deps := runFirstCheck(t, cfg, nil)
deps.portChecker.mu.Lock()
deps.portChecker.closed = true
deps.portChecker.mu.Unlock()
})
runCheck(t, w, deps)
hs, _ := deps.state.GetHostnameState(testHost)
assertNotified(
@@ -453,7 +454,7 @@ func TestTLSExpiryWarning(t *testing.T) {
cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost}
deps := runChecks(t, cfg, expiresInThreeDays, nil)
_, deps := runFirstCheck(t, cfg, expiresInThreeDays)
assertNotified(t, deps, "TLS Expiry Warning: "+testHost, "warning")
}
@@ -465,20 +466,19 @@ func TestTLSExpiryWarningDedup(t *testing.T) {
cfg.Hostnames = []string{testHost}
cfg.TLSInterval = 24 * time.Hour
w, deps := runFirstCheck(t, cfg, expiresInThreeDays)
title := "TLS Expiry Warning: " + testHost
// The second check comes within the TLS interval of the first,
// so it must not warn again.
var warnings int
deps := runChecks(t, cfg, expiresInThreeDays, func(deps *testDeps) {
warnings = countNotifications(deps, title)
})
warnings := countNotifications(deps, title)
if warnings == 0 {
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)
if got != warnings {
t.Errorf(
@@ -525,7 +525,7 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
cfg.Hostnames = []string{testHost}
// 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{
RecordsByNameserver: map[string]*state.NameserverRecordState{
oldNS1: {
@@ -534,7 +534,7 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
},
},
})
}, nil)
})
snap := deps.state.GetSnapshot()
@@ -665,9 +665,10 @@ func TestNSFailureAndRecovery(t *testing.T) {
cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost}
// Between the checks, save every nameserver the first check found
// as failed, and add, as answering, one that live DNS does not list.
deps := runChecks(t, cfg, nil, func(deps *testDeps) {
w, deps := runFirstCheck(t, cfg, nil)
// 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)
for _, nsState := range hs.RecordsByNameserver {
nsState.Status = "error"
@@ -679,7 +680,8 @@ func TestNSFailureAndRecovery(t *testing.T) {
}
deps.state.SetHostnameState(testHost, hs)
})
runCheck(t, w, deps)
assertNotified(t, deps, "NS Failure: "+testHost, "error")
assertNotified(t, deps, "NS Recovery: "+testHost, "success")