watcher: notify NS query failure and recovery (closes #104)

LookupAllRecords now returns each nameserver's response, so the
watcher saves its status: ok when it answered, NXDOMAIN and no records
included, and error with the reason when it timed out, answered
SERVFAIL or REFUSED, or could not be reached. A nameserver that starts
failing sends NS Failure and one that answers again sends NS Recovery.
A failing nameserver is left out of the record change and
inconsistency comparisons. The resolver used to report REFUSED and
network errors as an answer with no records; they are now errors. A
lookup cut short by its context now returns an error instead of a
failure of the nameserver it was querying.

Model: opus-5-5

Dockerfile

@@ -41,18 +41,15 @@ RUN make build
 # alpine 3.21, 2026-02-28
 FROM alpine@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709
 
-RUN apk add --no-cache ca-certificates tzdata
+RUN apk add --no-cache ca-certificates tzdata su-exec
 
 COPY --from=builder /src/bin/dnswatcher /usr/local/bin/dnswatcher
+COPY deploy/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
 
-# 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.
+# dnswatcher runs as this unprivileged user. The entrypoint creates the
+# data directory and gives it to this user on every start.
 RUN addgroup -S -g 10001 dnswatcher \
-    && adduser -S -G dnswatcher -u 10001 dnswatcher \
-    && mkdir -p /var/lib/dnswatcher \
-    && chown dnswatcher:dnswatcher /var/lib/dnswatcher
+    && adduser -S -G dnswatcher -u 10001 dnswatcher
 
 ENV DNSWATCHER_DATA_DIR=/var/lib/dnswatcher
 
@@ -62,7 +59,8 @@ ENV DNSWATCHER_DATA_DIR=/var/lib/dnswatcher
 # data directory, or the binary's directory, the working directory.
 WORKDIR /
 
-USER dnswatcher
+# No USER: the entrypoint must start as root to set up the data
+# directory; it then runs dnswatcher as the dnswatcher user.
 
 EXPOSE 8080
 
@@ -72,4 +70,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/dnswatcher"]
+ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]

README.md

@@ -71,18 +71,25 @@ rejected.
     did on the previous check (additions, removals, value changes).
   - **NS query failure**: A nameserver that previously responded
     becomes unreachable (timeout, SERVFAIL, REFUSED, network error).
-    This is distinct from "responded with no records."
+    This is distinct from "responded with no records": a nameserver
+    that answers NXDOMAIN or with no records has responded. The alert
+    is sent once, on the check where it starts failing. A failing
+    nameserver gives no records, so it is not reported as a record
+    change or compared for inconsistency. A nameserver that is already
+    failing on the first check that sees it is recorded silently.
   - **NS recovery**: A previously-unreachable nameserver starts
-    responding again.
+    responding again. Its records are not compared with those from
+    before it failed, so a change made while it was failing is not
+    reported as a record change.
   - **Inconsistency detected**: Two nameservers return different record
     sets for the same hostname and did not already differ on the previous
     check. Every pair of nameservers is compared. The alert is sent once
     for each such pair, on the check where they start to disagree, and not
     again while they keep disagreeing, including after a restart. A
     nameserver that was not in the previous check (newly added, or back
-    after dropping out) and answers differently is reported on the check
-    where it appears. If a pair agrees again and later disagrees, the
-    alert is sent again.
+    after dropping out), or failed on it, and answers differently is
+    reported on the check where it answers. If a pair agrees again and
+    later disagrees, the alert is sent again.
 
 ### TCP Port Monitoring
 
@@ -442,9 +449,13 @@ The `status` field for each per-nameserver entry and certificate entry
 tracks reachability:
 
 | Status      | Meaning                                                    |
-|-------------|-------------------------------------------------|
+|-------------|------------------------------------------------------------|
 | `ok`        | Query succeeded, records are current                       |
-| `error`     | Query failed (timeout, SERVFAIL, network error)  |
+| `error`     | Query failed (timeout, SERVFAIL, REFUSED, network error)   |
+
+A nameserver that answers NXDOMAIN or with no records has status `ok` and
+empty `records`. A nameserver whose query failed has status `error`, empty
+`records`, and the reason in `error`.
 
 ---
 
@@ -533,17 +544,7 @@ 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. 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
-  ```
-
+  the state file lives.
 - **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

TODO.md

@@ -1,279 +1,112 @@
 # Workflow
 
-* branch (from `main`)
+* branch (from `next`)
 * do the work in Next Step
 * move Next Step to the top of Completed Steps
 * move the top item of Future Steps into Next Step
 * commit (`TODO.md` changes in the same commit as the work)
-* merge to `main` if the branch is not protected, otherwise open a PR
 * push
+* open a PR against `next`
 
 # Status
 
-pre-1.0. No git tags.
+pre-1.0. No git tags. Work lands on `next` by PR. Open work for 1.0 is tracked
+on the 1.0 milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
 
 # Next Step
 
-Add the README sections required by policy (Description, Getting Started,
-Rationale, Design, TODO, License, Author) if any are still missing.
+nameserver IP address changes: https://git.eeqj.de/sneak/dnswatcher/issues/105
 
 # Completed Steps
 
+- 2026-10-01: a nameserver that does not answer is saved as `error` with the
+  reason, and NS failure and NS recovery are notified (closes #104).
+- 2026-10-01: `TODO.md` brought up to date: open issues listed by URL, every
+  Completed Steps entry cut to at most two lines (closes #146).
+- 2026-10-01: wildcard CORS now applies only to the public routes, not to
+  `/metrics`, and allows only the methods they serve (closes #100).
+- 2026-10-01: `internal/state` and `internal/watcher` no longer export test-only
+  constructors: two moved to `export_test.go`, one is deleted (closes #111).
 - 2026-10-01: notify shutdown tests use one timing constant per meaning, name
   the bound they check, and require the drain's debug line (closes #116).
-- 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
-  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/livednstest`, 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
-  a restart. Every pair of nameservers is compared, not only neighbours in
-  sorted order of name (closes #158).
+- 2026-09-29: the entrypoint chowns the data directory to `dnswatcher` and runs
+  dnswatcher as that user, so a host bind mount needs no chown (closes #166).
+- 2026-09-29: the live-DNS test package is renamed `internal/livednstest`;
+  `make lint` fails when program code imports it (closes #164).
+- 2026-09-29: `.golangci.yml` re-fetched from `sneak/prompts`, with
+  `gomodguard_v2` and the org `depguard` `test-support` rule (closes #123).
+- 2026-09-29: watcher and resolver tests that look something up in DNS use the
+  real resolver against live DNS servers (closes #159).
+- 2026-09-28: the inconsistency alert is sent once, when two nameservers start
+  to disagree; every pair of nameservers is compared (closes #158).
 - 2026-09-28: DNS names in record values (CNAME, MX, SRV and NS targets) are
-  lower-cased, so nameservers that answer in different letter case no longer
-  count as inconsistent or as a record change (closes #157).
-- 2026-09-28: `script/cibuild` and `script/docker` now pass
-  `--no-cache-filter=lint,builder` so lint and tests run every build (closes
-  #115).
-- 2026-09-28: the server timeout test now drives `Run` and checks the
-  `http.Server` it serves carries the timeouts; corrected the `ReadTimeout`
-  note in that test (closes #120).
-- 2026-09-28: upaas deploy readiness — runtime image runs as unprivileged
-  `dnswatcher`, Docker `HEALTHCHECK`, startup fails when the data directory is
-  not writable, README "Running under upaas" (closes #147).
+  lower-cased, so letter case alone is not a change (closes #157).
+- 2026-09-28: lint and tests run on every build: `script/cibuild` and
+  `script/docker` pass `--no-cache-filter=lint,builder` (closes #115).
+- 2026-09-28: the server timeout test drives `Run` and checks the timeouts on
+  the `http.Server` it serves (closes #120).
+- 2026-09-28: upaas deploy readiness: the image runs as user `dnswatcher` with a
+  `HEALTHCHECK`; README "Running under upaas" (closes #147).
 - 2026-09-21: added behavioural tests for `internal/globals`,
   `internal/healthcheck`, and `internal/logger` (closes #110).
 - 2026-09-21: `go mod tidy` dropped the redundant `golang.org/x/sync`
   `// indirect` line so `script/bootstrap` leaves a clean tree (#132)
-- 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-09: in-flight notification deliveries are now drained at
-  shutdown (#106): `notify.New` registers an fx `OnStop` hook that waits
-  on a `sync.WaitGroup` of tracked delivery goroutines, bounded by the
-  `OnStop` context; on expiry the outstanding count is logged at warn
-  level and parked retry backoffs are released instead of being dropped
-  silently, and deliveries submitted after the drain begins are refused
-  so shutdown cannot be extended indefinitely; an `OnStop` context that
-  is already expired on entry with nothing outstanding drains quietly
-  rather than warning about deliveries that were never abandoned
-- 2026-08-09: `http.Server` now sets all four socket-level timeouts
-  (`ReadTimeout` 15s, `ReadHeaderTimeout` 10s, `WriteTimeout` 75s,
-  `IdleTimeout` 120s) as named constants in `internal/server/server.go`,
-  closing the slowloris / unreaped-keep-alive exposure required by
-  `REPO_POLICIES.md` before 1.0; `WriteTimeout` is deliberately greater
-  than the 60s `chimw.Timeout` handler budget so that budget stays
-  reachable, and tests in `internal/server` pin both the non-zero
-  values and that relationship (#99)
-- 2026-08-09: security response headers middleware
-  (`SecurityHeaders()` in `internal/middleware/middleware.go`)
-  registered globally in `internal/server/routes.go`, so HSTS, CSP,
-  `X-Frame-Options`, `X-Content-Type-Options`, `Referrer-Policy`, and
-  `Permissions-Policy` are set on every response including `/s/...` and
-  `/metrics`; the CSP needs no `unsafe-inline`/`unsafe-eval` because the
-  dashboard ships no JavaScript and no inline styles; HSTS is emitted
-  unconditionally per policy (TLS-terminating proxy in front). Remaining
-  1.0 hardening items — `http.Server` timeouts, request body limits,
-  rate limiting, CORS scoping — are tracked separately
-- 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
-  (owner-authorized; same file is being landed as canonical via prompts
-  PR #24), with settings under `linters.settings` so the
-  lll/funlen/cyclop/dupl thresholds apply; fixed the resulting
-  `goconst`, `dupl`, and `lll` findings; the informational `gomodguard`
-  deprecation warning under this config is accepted
-- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
-  Makefile shims, README Entrypoints section
-- 2026-02-20: iterative DNS resolver implemented; tests made hermetic
-  with mocked DNS (origin/feature/resolver, unmerged)
-- 2026-02-20: CI actions and go install refs pinned to commit SHAs;
-  Gitea Actions workflow for make check (origin/ci/make-check, unmerged)
+- 2026-08-10: comment-only corrections to `script/bootstrap`, `script/cibuild`
+  and `Dockerfile.lint`; no behaviour changed.
+- 2026-08-10: MIT `LICENSE` added at the repository root; the README's first
+  line and License section name the licence.
+- 2026-08-10: policy scaffold present: `REPO_POLICIES.md`, `.editorconfig`,
+  `.dockerignore`, CI workflow, `make fmt-check`, `make docker`, `make hooks`.
+- 2026-08-10: Go's test cache disabled in `script/test` (`-count=1`), so every
+  run queries live DNS; a failed run is rerun with `-v`.
+- 2026-08-10: live-DNS tests made robust rather than gated (#93): a limit on
+  concurrent lookups, retries, and a quorum across nameservers.
+- 2026-08-10: all linting moved into Docker: `script/lint` builds
+  `Dockerfile.lint`, and the root `Dockerfile` has its own lint stage.
+- 2026-08-09: in-flight notification deliveries are drained at shutdown, bounded
+  by the shutdown deadline (#106).
+- 2026-08-09: `http.Server` sets all four socket timeouts; `WriteTimeout` stays
+  above the 60s handler timeout (#99).
+- 2026-08-09: `SecurityHeaders()` middleware sets HSTS, CSP and the other
+  security headers `REPO_POLICIES.md` requires on every response.
+- 2026-08-07: golangci-lint bumped to v2.12.2 and `.golangci.yml` set to the org
+  config; fixed the resulting `goconst`, `dupl` and `lll` findings.
+- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, Makefile
+  shims, README Entrypoints section
+- 2026-02-20: iterative DNS resolver implemented
+- 2026-02-20: CI actions and go install refs pinned to commit SHAs; Gitea
+  Actions workflow added
 - 2026-02-20: watcher monitoring orchestrator merged to main (#8)
 - 2026-02-20: DOMAINS/HOSTNAMES unified into single TARGETS config (#11)
 - 2026-02-19: TCP port connectivity checker, made concurrent with port
   validation; gosec G704 SSRF findings fixed without suppression
-  (feature branches, unmerged)
-- 2026-02-19: TLS certificate inspector with no-peer-certificates error
-  path and IP SANs (feature branch, unmerged)
+- 2026-02-19: TLS certificate inspector with no-peer-certificates error path and
+  IP SANs
 - 2026-02-19: gosec SSRF and formatting fixes on main
 - 2026-02-19: initial scaffold with per-nameserver DNS monitoring model
 
 # Future Steps
 
-Compliance:
-
-- Pin Dockerfile base images by sha256 and ensure the Docker build runs
-  make check
-
-Branch reconciliation:
-
-- Sync local checkout with origin: local main is 8 commits behind
-  origin/main; local feature/resolver has diverged from
-  origin/feature/resolver, which already implements the resolver
-- Merge in-flight branches to main once green: feature/resolver,
-  ci/make-check, feature/portcheck-implementation,
-  feature/tlscheck-implementation
-
-Resolver (plan from untracked TODO.md; largely implemented on
-origin/feature/resolver, verify each item before closing):
-
-- Add github.com/miekg/dns dependency
-- roots.go: hardcoded IANA root server list (a through m, IPv4/IPv6),
-  rootServers() returning ip:53 strings
-- query.go: low-level query(ctx, server, name, qtype): UDP with TCP
-  fallback on truncation, RD=0, context respected, 5s per-query timeout,
-  returns raw *dns.Msg
-- trace.go: iterative delegation chasing from roots: referral detection
-  (NOERROR, empty answer, NS in authority), glue extraction with
-  bailiwick check, out-of-bailiwick NS resolved with recursion guard,
-  delegation depth limit (20), retry across nameservers on failure, do
-  not chase CNAMEs inside trace
-- FindAuthoritativeNameservers: NS set via trace, sorted, FQDN
-  normalized, trailing dot handled; must pass its 9 tests
-- QueryNameserver: resolve NS host to IPs, query A/AAAA/CNAME/MX/TXT/
-  SRV/CAA/NS, build NameserverResponse with status mapping (OK,
-  NXDomain, NoData, Error), documented record formatting, sorted values,
-  lame delegation detection; must pass its 16 tests
-- QueryAllNameservers: find NS set for parent domain (public suffix
-  list), query all NS in parallel with bounded concurrency, return map
-  even when all fail, context cancellation; must pass its 4 tests
-- LookupNS: thin wrapper over FindAuthoritativeNameservers, sorted,
-  identical results; must pass its 3 tests
-- ResolveIPAddresses: collect A/AAAA from all NS, follow CNAME chains
-  with MaxCNAMEDepth, dedupe, sort, NXDOMAIN returns empty slice with
-  nil error; must pass its 9 tests
-- All 39 resolver tests pass, make check green, merge to main
-
-Watcher (internal/watcher/watcher.go):
-
-- Scheduling loop in Run(ctx): initial check on startup, separate
-  tickers for DNS/port and TLS intervals, persist state via state.Save()
-  after each cycle, clean shutdown on context cancel
-- Domain check: LookupNS, compare to stored state, store silently on
-  first run, notify with old/new NS lists on change
-- Hostname check: QueryAllNameservers, compare per-NS records; notify on
-  record changes, NS failure, NS recovery, inconsistency detected,
-  inconsistency resolved, empty response; store silently on first run
-- Port check: ResolveIPAddresses, check ports 80 and 443 per IP, notify
-  on open/closed transitions, handle new and disappeared IPs
-- TLS check: for each open IP:443, CheckCertificate; notify on expiry
-  warning, certificate change (CN/issuer/SANs), TLS failure/recovery
-
-Port checker (internal/portcheck/portcheck.go):
-
-- Tests against known-open ports and RFC documentation IPs
-- CheckPort: net.DialTimeout (5s), context respected; (true, nil) open,
-  (false, nil) closed/timeout/refused, error only for unexpected
-  failures
-
-TLS checker (internal/tlscheck/tlscheck.go):
-
-- Tests against known public HTTPS servers, verify fields populated
-- CheckCertificate: tls.Dial to specific IP:443 with hostname as SNI;
-  extract subject CN, issuer CN and org, NotAfter, SANs; error on
-  handshake failure
-
-Notification service (internal/notify/notify.go, Slack/Mattermost/ntfy
-backends exist):
-
-- Structured notification types: DNS change, port change, TLS expiry,
-  TLS change, NS failure, NS recovery, NS inconsistency
-- Per-backend formatting: Slack/Mattermost attachment colors (red
-  failures/expiry, yellow warnings, green recoveries, blue info); ntfy
-  priorities (urgent failures, high warnings, default changes, low
-  recoveries); include hostname, nameserver, old/new values, timestamps
-
-HTTP API handlers:
-
-- Wire *state.State and *watcher.Watcher into handler params
-- GET /api/v1/status: full state snapshot as JSON
-- GET /api/v1/domains: domain states with NS records and last-checked
-- GET /api/v1/hostnames: hostname states with per-NS record data
-
-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.")
+- `DNSWATCHER_SENTRY_DSN` does nothing:
+  https://git.eeqj.de/sneak/dnswatcher/issues/107
+- invalid DNS or TLS interval silently replaced by the default:
+  https://git.eeqj.de/sneak/dnswatcher/issues/177
+- rate limit on `/metrics` Basic Auth:
+  https://git.eeqj.de/sneak/dnswatcher/issues/101
+- images report version `dev`: https://git.eeqj.de/sneak/dnswatcher/issues/109
+- trial run of the finished image:
+  https://git.eeqj.de/sneak/dnswatcher/issues/149
+- 1.0 readiness: run it with a real config and read the logs:
+  https://git.eeqj.de/sneak/dnswatcher/issues/66
+- `goimports` in `make fmt-check`, Markdown formatting:
+  https://git.eeqj.de/sneak/dnswatcher/issues/119
+- final state save at shutdown: https://git.eeqj.de/sneak/dnswatcher/issues/114
+- `internal/notify` shutdown tests hang when a drain returns early:
+  https://git.eeqj.de/sneak/dnswatcher/issues/176
+- README accuracy sweep: https://git.eeqj.de/sneak/dnswatcher/issues/108
+- README sections required by policy:
+  https://git.eeqj.de/sneak/dnswatcher/issues/173
+- `script/install-precommit` in a linked worktree:
+  https://git.eeqj.de/sneak/dnswatcher/issues/129
+- fixed root server order: https://git.eeqj.de/sneak/dnswatcher/issues/138
+- review toward 1.0: https://git.eeqj.de/sneak/dnswatcher/issues/144

deploy/docker-entrypoint.sh

@@ -0,0 +1,17 @@
+#!/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 "$@"

internal/middleware/middleware.go

@@ -223,17 +223,14 @@ func realIP(r *http.Request) string {
 	return addr
 }
 
-// CORS returns CORS middleware.
+// CORS returns middleware that lets any origin read a response. It is
+// for the public, read-only routes only, so it allows only the
+// methods those routes serve and no Authorization header.
 func (m *Middleware) CORS() func(http.Handler) http.Handler {
 	return cors.Handler(cors.Options{
 		AllowedOrigins:   []string{"*"},
-		AllowedMethods: []string{
-			"GET", "POST", "PUT", "DELETE", "OPTIONS",
-		},
-		AllowedHeaders: []string{
-			"Accept", "Authorization",
-			"Content-Type", "X-CSRF-Token",
-		},
+		AllowedMethods:   []string{"GET", "OPTIONS"},
+		AllowedHeaders:   []string{"Accept", "Content-Type"},
 		ExposedHeaders:   []string{"Link"},
 		AllowCredentials: false,
 		MaxAge:           corsMaxAge,

internal/middleware/middleware_test.go

@@ -276,10 +276,18 @@ func newTestHandlers(t *testing.T) *handlers.Handlers {
 		t.Fatalf("notify.New: %v", err)
 	}
 
+	st, err := state.New(fxtest.NewLifecycle(t), state.Params{
+		Logger: log,
+		Config: &config.Config{DataDir: t.TempDir()},
+	})
+	if err != nil {
+		t.Fatalf("state.New: %v", err)
+	}
+
 	hnd, err := handlers.New(nil, handlers.Params{
 		Logger:  log,
 		Globals: glob,
-		State:   state.NewForTest(),
+		State:   st,
 		Notify:  notifier,
 	})
 	if err != nil {

internal/resolver/iterative.go

@@ -504,7 +504,9 @@ func (r *Resolver) queryAllTypes(
 type queryState struct {
 	gotNXDomain bool
 	gotSERVFAIL bool
+	gotRefused  bool
 	gotTimeout  bool
+	netErr      error
 	hasRecords  bool
 }
 
@@ -542,8 +544,13 @@ func (r *Resolver) querySingleType(
 ) {
 	msg, err := r.queryDNS(ctx, nsIP, hostname, qtype)
 	if err != nil {
-		if isTimeout(err) {
+		switch {
+		case isTimeout(err):
 			state.gotTimeout = true
+		case errors.Is(err, ErrRefused):
+			state.gotRefused = true
+		default:
+			state.netErr = err
 		}
 
 		return
@@ -603,6 +610,12 @@ func classifyResponse(resp *NameserverResponse, state queryState) {
 	case state.gotSERVFAIL && !state.hasRecords:
 		resp.Status = StatusError
 		resp.Error = "server returned SERVFAIL"
+	case state.gotRefused && !state.hasRecords:
+		resp.Status = StatusError
+		resp.Error = "server returned REFUSED"
+	case state.netErr != nil && !state.hasRecords:
+		resp.Status = StatusError
+		resp.Error = "network error: " + state.netErr.Error()
 	case !state.hasRecords && !state.gotNXDomain:
 		resp.Status = StatusNoData
 	}
@@ -682,11 +695,14 @@ func (r *Resolver) queryEachNS(
 	results := make(map[string]*NameserverResponse)
 
 	for _, ns := range nameservers {
+		resp, err := r.QueryNameserver(ctx, ns, hostname)
+
+		// A query the context cut short says nothing about the
+		// nameserver, so it must not be returned as its failure.
 		if checkCtx(ctx) != nil {
 			return nil, ErrContextCanceled
 		}
 
-		resp, err := r.QueryNameserver(ctx, ns, hostname)
 		if err != nil {
 			results[ns] = &NameserverResponse{
 				Nameserver: ns,
@@ -714,21 +730,13 @@ func (r *Resolver) LookupNS(
 
 // LookupAllRecords performs iterative resolution to find all DNS
 // records for the given hostname, keyed by authoritative nameserver.
+// Each nameserver's response carries its status and error with its
+// records.
 func (r *Resolver) LookupAllRecords(
 	ctx context.Context,
 	hostname string,
-) (map[string]map[string][]string, error) {
-	results, err := r.QueryAllNameservers(ctx, hostname)
-	if err != nil {
-		return nil, err
-	}
-
-	out := make(map[string]map[string][]string, len(results))
-	for ns, resp := range results {
-		out[ns] = resp.Records
-	}
-
-	return out, nil
+) (map[string]*NameserverResponse, error) {
+	return r.QueryAllNameservers(ctx, hostname)
 }
 
 // ResolveIPAddresses resolves a hostname to all IPv4 and IPv6

internal/resolver/resolver_test.go

@@ -2,6 +2,7 @@ package resolver_test
 
 import (
 	"context"
+	"fmt"
 	"log/slog"
 	"net"
 	"os"
@@ -13,6 +14,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"sneak.berlin/go/dnswatcher/internal/livednstest"
 	"sneak.berlin/go/dnswatcher/internal/resolver"
 )
 
@@ -231,6 +233,45 @@ func TestQueryNameserver_NXDomain(t *testing.T) {
 	assert.Equal(t, resolver.StatusNXDomain, resp.Status)
 }
 
+// TestQueryNameserver_Refused asks a google.com nameserver about
+// cloudflare.com, a zone it does not serve, which it refuses. Refusing
+// is a failure to answer, not an answer with no records.
+func TestQueryNameserver_Refused(t *testing.T) {
+	t.Parallel()
+
+	r := newTestResolver(t)
+	ns := findOneNSForDomain(t, r, "google.com")
+
+	var resp *resolver.NameserverResponse
+
+	livednstest.Retry(
+		t,
+		"QueryNameserver("+ns+", cloudflare.com)",
+		func(ctx context.Context) error {
+			var err error
+
+			resp, err = r.QueryNameserver(ctx, ns, "cloudflare.com")
+			if err != nil {
+				return err
+			}
+
+			// A timeout or a network error is no reply at all.
+			if resp.Status == resolver.StatusTimeout ||
+				strings.HasPrefix(resp.Error, "network error") {
+				return fmt.Errorf(
+					"%w: %s: %s",
+					livednstest.ErrNoAnswer, ns, resp.Error,
+				)
+			}
+
+			return nil
+		},
+	)
+
+	assert.Equal(t, resolver.StatusError, resp.Status)
+	assert.Equal(t, "server returned REFUSED", resp.Error)
+}
+
 func TestQueryNameserver_RecordsSorted(t *testing.T) {
 	t.Parallel()
 
@@ -530,7 +571,7 @@ func TestQueryNameserverIP_Timeout(t *testing.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
+	// makes the status vary from run to run between error and
 	// timeout, so the deadline must outlast the first try.
 	ctx, cancel := context.WithTimeout(
 		context.Background(), 3*time.Second,

internal/server/routes.go

@@ -23,14 +23,20 @@ func (s *Server) SetupRoutes() {
 	s.router.Use(chimw.RequestID)
 	s.router.Use(s.mw.SecurityHeaders())
 	s.router.Use(s.mw.Logging())
-	s.router.Use(s.mw.CORS())
 	s.router.Use(chimw.Timeout(requestTimeout))
 
+	// Public, unauthenticated, read-only routes, the only ones
+	// REPO_POLICIES.md allows wildcard CORS on. CORS is middleware of
+	// this whole router, not of a Group, so that it also answers
+	// OPTIONS preflight requests, which no route here registers.
+	public := chi.NewRouter()
+	public.Use(s.mw.CORS())
+
 	// Dashboard (read-only web UI)
-	s.router.Get("/", s.handlers.HandleDashboard())
+	public.Get("/", s.handlers.HandleDashboard())
 
 	// Static assets (embedded CSS/JS)
-	s.router.Mount(
+	public.Mount(
 		"/s",
 		http.StripPrefix(
 			"/s",
@@ -39,27 +45,30 @@ func (s *Server) SetupRoutes() {
 	)
 
 	// Health check (standard well-known path)
-	s.router.Get(
+	public.Get(
 		"/.well-known/healthcheck",
 		s.handlers.HandleHealthCheck(),
 	)
 
 	// Legacy health check (keep for backward compatibility)
-	s.router.Get("/health", s.handlers.HandleHealthCheck())
+	public.Get("/health", s.handlers.HandleHealthCheck())
 
 	// API v1 routes
-	s.router.Route("/api/v1", func(r chi.Router) {
+	public.Route("/api/v1", func(r chi.Router) {
 		r.Get("/status", s.handlers.HandleStatus())
 	})
 
-	// Metrics endpoint (optional, with basic auth)
+	s.router.Mount("/", public)
+
+	// Metrics endpoint (optional, with basic auth) and no CORS: a
+	// Prometheus scraper is not a browser. It is mounted rather than
+	// added with Get so that every method on /metrics, OPTIONS
+	// included, ends here instead of falling through to the public
+	// router and its CORS.
 	if s.params.Config.MetricsUsername != "" {
-		s.router.Group(func(r chi.Router) {
-			r.Use(s.mw.MetricsAuth())
-			r.Get(
-				"/metrics",
-				promhttp.Handler().ServeHTTP,
-			)
-		})
+		metrics := chi.NewRouter()
+		metrics.Use(s.mw.MetricsAuth())
+		metrics.Get("/", promhttp.Handler().ServeHTTP)
+		s.router.Mount("/metrics", metrics)
 	}
 }

internal/server/routes_test.go

@@ -0,0 +1,221 @@
+package server_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/spf13/viper"
+
+	"sneak.berlin/go/dnswatcher/internal/server"
+)
+
+// Credentials for /metrics, which is only routed when a username is set.
+const (
+	metricsUsername = "scraper"
+	metricsPassword = "scrape-secret"
+)
+
+// The tests below set env vars and touch viper global state, so like
+// the config tests they cannot use t.Parallel.
+
+// routedServer builds the server with its routes set up, ready to serve
+// test requests. The caller must first configure viper.
+func routedServer(t *testing.T) *server.Server {
+	t.Helper()
+
+	srv := buildServer(t)
+	srv.SetupRoutes()
+
+	return srv
+}
+
+// crossOriginRequest builds a request as a browser sends it from a page
+// on another site.
+func crossOriginRequest(
+	t *testing.T,
+	method string,
+	target string,
+) *http.Request {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), method, target, nil)
+	req.Header.Set("Origin", "https://example.net")
+
+	return req
+}
+
+// preflightRequest builds the OPTIONS request a browser sends before a
+// cross-origin request with the given method and request headers.
+func preflightRequest(
+	t *testing.T,
+	target string,
+	method string,
+	headers string,
+) *http.Request {
+	t.Helper()
+
+	req := crossOriginRequest(t, http.MethodOptions, target)
+	req.Header.Set("Access-Control-Request-Method", method)
+
+	if headers != "" {
+		req.Header.Set("Access-Control-Request-Headers", headers)
+	}
+
+	return req
+}
+
+func serve(
+	srv *server.Server,
+	req *http.Request,
+) *httptest.ResponseRecorder {
+	rec := httptest.NewRecorder()
+	srv.ServeHTTP(rec, req)
+
+	return rec
+}
+
+// publicPaths returns one path on each public route.
+func publicPaths() []string {
+	return []string{
+		"/",
+		"/s/css/tailwind.min.css",
+		"/api/v1/status",
+		"/health",
+		"/.well-known/healthcheck",
+	}
+}
+
+// TestPublicRoutesAllowAnyOrigin checks that every public route answers
+// a cross-origin GET with the CORS wildcard.
+func TestPublicRoutesAllowAnyOrigin(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_METRICS_USERNAME", metricsUsername)
+	t.Setenv("DNSWATCHER_METRICS_PASSWORD", metricsPassword)
+
+	srv := routedServer(t)
+
+	for _, path := range publicPaths() {
+		rec := serve(srv, crossOriginRequest(t, http.MethodGet, path))
+
+		if rec.Code != http.StatusOK {
+			t.Errorf("GET %s: status = %d, want 200", path, rec.Code)
+		}
+
+		got := rec.Header().Get("Access-Control-Allow-Origin")
+		if got != "*" {
+			t.Errorf(
+				"GET %s: Access-Control-Allow-Origin = %q, want %q",
+				path, got, "*",
+			)
+		}
+	}
+}
+
+// TestMetricsHasNoCORS checks that no request to the Basic-Auth
+// protected /metrics, preflight included, gets a CORS header.
+func TestMetricsHasNoCORS(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_METRICS_USERNAME", metricsUsername)
+	t.Setenv("DNSWATCHER_METRICS_PASSWORD", metricsPassword)
+
+	srv := routedServer(t)
+
+	authenticated := crossOriginRequest(t, http.MethodGet, "/metrics")
+	authenticated.SetBasicAuth(metricsUsername, metricsPassword)
+
+	tests := []struct {
+		name       string
+		req        *http.Request
+		wantStatus int
+	}{
+		{
+			"authenticated GET",
+			authenticated,
+			http.StatusOK,
+		},
+		{
+			"unauthenticated GET",
+			crossOriginRequest(t, http.MethodGet, "/metrics"),
+			http.StatusUnauthorized,
+		},
+		{
+			"preflight",
+			preflightRequest(t, "/metrics", http.MethodGet, ""),
+			http.StatusUnauthorized,
+		},
+	}
+
+	for _, tt := range tests {
+		rec := serve(srv, tt.req)
+
+		if rec.Code != tt.wantStatus {
+			t.Errorf(
+				"%s: status = %d, want %d",
+				tt.name, rec.Code, tt.wantStatus,
+			)
+		}
+
+		got := rec.Header().Get("Access-Control-Allow-Origin")
+		if got != "" {
+			t.Errorf(
+				"%s: Access-Control-Allow-Origin = %q, want none",
+				tt.name, got,
+			)
+		}
+	}
+}
+
+// TestPreflightAllowsOnlyWhatPublicRoutesServe checks what each public
+// route agrees to in a CORS preflight: GET, but not POST, PUT or
+// DELETE, which no route serves, and not the Authorization or
+// X-CSRF-Token headers, which no public route reads. It checks every
+// public route because one added with Get, such as /health, answers a
+// preflight only while CORS is middleware of a whole router; in a
+// Group, chi would answer it with 405 and no CORS headers.
+func TestPreflightAllowsOnlyWhatPublicRoutesServe(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_METRICS_USERNAME", metricsUsername)
+	t.Setenv("DNSWATCHER_METRICS_PASSWORD", metricsPassword)
+
+	srv := routedServer(t)
+
+	tests := []struct {
+		method  string
+		headers string
+		allowed bool
+	}{
+		{http.MethodGet, "", true},
+		{http.MethodGet, "Content-Type", true},
+		{http.MethodPost, "", false},
+		{http.MethodPut, "", false},
+		{http.MethodDelete, "", false},
+		{http.MethodGet, "Authorization", false},
+		{http.MethodGet, "X-CSRF-Token", false},
+	}
+
+	for _, path := range publicPaths() {
+		for _, tt := range tests {
+			rec := serve(srv, preflightRequest(
+				t, path, tt.method, tt.headers,
+			))
+
+			want := ""
+			if tt.allowed {
+				want = tt.method
+			}
+
+			got := rec.Header().Get("Access-Control-Allow-Methods")
+			if got != want {
+				t.Errorf(
+					"preflight to %s for %s with headers %q: "+
+						"Access-Control-Allow-Methods = %q, want %q",
+					path, tt.method, tt.headers, got, want,
+				)
+			}
+		}
+	}
+}

internal/state/export_test.go

@@ -0,0 +1,23 @@
+package state
+
+import (
+	"log/slog"
+
+	"sneak.berlin/go/dnswatcher/internal/config"
+)
+
+// NewForTestWithDataDir creates an empty State that saves to dataDir,
+// without the fx lifecycle.
+func NewForTestWithDataDir(dataDir string) *State {
+	return &State{
+		log: slog.Default(),
+		snapshot: &Snapshot{
+			Version:      stateVersion,
+			Domains:      make(map[string]*DomainState),
+			Hostnames:    make(map[string]*HostnameState),
+			Ports:        make(map[string]*PortState),
+			Certificates: make(map[string]*CertificateState),
+		},
+		config: &config.Config{DataDir: dataDir},
+	}
+}

internal/state/state_test.go

@@ -739,7 +739,7 @@ func TestPortStateUnmarshalJSON_BothFormats(t *testing.T) {
 func TestGetSnapshot_ReturnsCopy(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTest()
+	s := state.NewForTestWithDataDir(t.TempDir())
 
 	populateState(t, s)
 
@@ -761,7 +761,7 @@ func TestGetSnapshot_ReturnsCopy(t *testing.T) {
 func TestDomainState_GetSet(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTest()
+	s := state.NewForTestWithDataDir(t.TempDir())
 
 	// Get on missing key returns false.
 	_, ok := s.GetDomainState("nonexistent.com")
@@ -812,7 +812,7 @@ func TestDomainState_GetSet(t *testing.T) {
 func TestHostnameState_GetSet(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTest()
+	s := state.NewForTestWithDataDir(t.TempDir())
 
 	_, ok := s.GetHostnameState("missing.example.com")
 	if ok {
@@ -857,7 +857,7 @@ func TestHostnameState_GetSet(t *testing.T) {
 func TestPortState_GetSetDelete(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTest()
+	s := state.NewForTestWithDataDir(t.TempDir())
 
 	_, ok := s.GetPortState("1.2.3.4:80")
 	if ok {
@@ -895,7 +895,7 @@ func TestPortState_GetSetDelete(t *testing.T) {
 func TestGetAllPortKeys(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTest()
+	s := state.NewForTestWithDataDir(t.TempDir())
 
 	keys := s.GetAllPortKeys()
 	if len(keys) != 0 {
@@ -937,7 +937,7 @@ func TestGetAllPortKeys(t *testing.T) {
 func TestCertificateState_GetSet(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTest()
+	s := state.NewForTestWithDataDir(t.TempDir())
 
 	_, ok := s.GetCertificateState("1.2.3.4:443:www.example.com")
 	if ok {
@@ -1158,7 +1158,7 @@ func TestLoadPreservesExistingStateOnMissingFile(t *testing.T) {
 func TestConcurrentGetSet(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTest()
+	s := state.NewForTestWithDataDir(t.TempDir())
 
 	const goroutines = 20
 
@@ -1368,35 +1368,6 @@ func TestMultipleSavesOverwrite(t *testing.T) {
 	}
 }
 
-// TestNewForTest verifies the test helper creates a valid empty state.
-func TestNewForTest(t *testing.T) {
-	t.Parallel()
-
-	s := state.NewForTest()
-
-	snap := s.GetSnapshot()
-
-	if snap.Version != 1 {
-		t.Errorf("version: got %d, want 1", snap.Version)
-	}
-
-	if snap.Domains == nil {
-		t.Error("Domains map should be initialized")
-	}
-
-	if snap.Hostnames == nil {
-		t.Error("Hostnames map should be initialized")
-	}
-
-	if snap.Ports == nil {
-		t.Error("Ports map should be initialized")
-	}
-
-	if snap.Certificates == nil {
-		t.Error("Certificates map should be initialized")
-	}
-}
-
 // TestSaveFilePermissions verifies the saved file has restricted permissions.
 func TestSaveFilePermissions(t *testing.T) {
 	t.Parallel()

internal/state/state_test_helper.go

@@ -1,38 +0,0 @@
-package state
-
-import (
-	"log/slog"
-
-	"sneak.berlin/go/dnswatcher/internal/config"
-)
-
-// NewForTest creates a State for unit testing with no persistence.
-func NewForTest() *State {
-	return &State{
-		log: slog.Default(),
-		snapshot: &Snapshot{
-			Version:      stateVersion,
-			Domains:      make(map[string]*DomainState),
-			Hostnames:    make(map[string]*HostnameState),
-			Ports:        make(map[string]*PortState),
-			Certificates: make(map[string]*CertificateState),
-		},
-		config: &config.Config{DataDir: ""},
-	}
-}
-
-// NewForTestWithDataDir creates a State backed by the given directory
-// for tests that need file persistence.
-func NewForTestWithDataDir(dataDir string) *State {
-	return &State{
-		log: slog.Default(),
-		snapshot: &Snapshot{
-			Version:      stateVersion,
-			Domains:      make(map[string]*DomainState),
-			Hostnames:    make(map[string]*HostnameState),
-			Ports:        make(map[string]*PortState),
-			Certificates: make(map[string]*CertificateState),
-		},
-		config: &config.Config{DataDir: dataDir},
-	}
-}

internal/watcher/export_test.go

@@ -2,14 +2,39 @@ package watcher
 
 import (
 	"context"
+	"log/slog"
+	"time"
 
+	"sneak.berlin/go/dnswatcher/internal/config"
+	"sneak.berlin/go/dnswatcher/internal/resolver"
 	"sneak.berlin/go/dnswatcher/internal/state"
 )
 
+// NewForTest creates a Watcher without fx for unit testing.
+func NewForTest(
+	cfg *config.Config,
+	st *state.State,
+	res DNSResolver,
+	pc PortChecker,
+	tc TLSChecker,
+	n Notifier,
+) *Watcher {
+	return &Watcher{
+		log:            slog.Default(),
+		config:         cfg,
+		state:          st,
+		resolver:       res,
+		portCheck:      pc,
+		tlsCheck:       tc,
+		notify:         n,
+		firstRun:       true,
+		expiryNotified: make(map[string]time.Time),
+	}
+}
+
 // NewlyDisagreeingPairs exports newlyDisagreeingPairs for testing.
 func NewlyDisagreeingPairs(
-	prev *state.HostnameState,
-	current map[string]map[string][]string,
+	prev, current *state.HostnameState,
 ) [][2]string {
 	return newlyDisagreeingPairs(prev, current)
 }
@@ -18,8 +43,15 @@ func NewlyDisagreeingPairs(
 func (w *Watcher) DetectHostnameChanges(
 	ctx context.Context,
 	hostname string,
-	prev *state.HostnameState,
-	current map[string]map[string][]string,
+	prev, current *state.HostnameState,
 ) {
 	w.detectHostnameChanges(ctx, hostname, prev, current)
 }
+
+// BuildHostnameState exports buildHostnameState for testing.
+func BuildHostnameState(
+	results map[string]*resolver.NameserverResponse,
+	now time.Time,
+) *state.HostnameState {
+	return buildHostnameState(results, now)
+}

internal/watcher/inconsistency_test.go

@@ -105,7 +105,9 @@ func TestNewlyDisagreeingPairs(t *testing.T) {
 
 			prev := hostnameState(tt.loaded)
 
-			for i, current := range tt.checks {
+			for i, records := range tt.checks {
+				current := hostnameState(records)
+
 				got := watcher.NewlyDisagreeingPairs(prev, current)
 				if !slices.Equal(got, tt.want[i]) {
 					t.Errorf(
@@ -114,7 +116,7 @@ func TestNewlyDisagreeingPairs(t *testing.T) {
 					)
 				}
 
-				prev = hostnameState(current)
+				prev = current
 			}
 		})
 	}
@@ -162,8 +164,9 @@ func TestInconsistencyAlert(t *testing.T) {
 			prev := hostnameState(tt.loaded)
 
 			for range 3 {
-				w.DetectHostnameChanges(t.Context(), host, prev, disagree)
-				prev = hostnameState(disagree)
+				current := hostnameState(disagree)
+				w.DetectHostnameChanges(t.Context(), host, prev, current)
+				prev = current
 			}
 
 			got := 0

internal/watcher/interfaces.go

@@ -5,6 +5,7 @@ import (
 	"context"
 
 	"sneak.berlin/go/dnswatcher/internal/portcheck"
+	"sneak.berlin/go/dnswatcher/internal/resolver"
 	"sneak.berlin/go/dnswatcher/internal/tlscheck"
 )
 
@@ -17,11 +18,11 @@ type DNSResolver interface {
 	) ([]string, error)
 
 	// LookupAllRecords queries all record types for a hostname,
-	// returning results keyed by nameserver then record type.
+	// returning each nameserver's response keyed by nameserver.
 	LookupAllRecords(
 		ctx context.Context,
 		hostname string,
-	) (map[string]map[string][]string, error)
+	) (map[string]*resolver.NameserverResponse, error)
 
 	// ResolveIPAddresses resolves a hostname to all IP addresses.
 	ResolveIPAddresses(

internal/watcher/nsfailure_test.go

@@ -0,0 +1,333 @@
+package watcher_test
+
+import (
+	"context"
+	"fmt"
+	"log/slog"
+	"strings"
+	"testing"
+	"time"
+
+	"sneak.berlin/go/dnswatcher/internal/livednstest"
+	"sneak.berlin/go/dnswatcher/internal/resolver"
+	"sneak.berlin/go/dnswatcher/internal/state"
+	"sneak.berlin/go/dnswatcher/internal/watcher"
+)
+
+// answered is what a check saves for a nameserver that answered with
+// these records.
+func answered(records map[string][]string) *state.NameserverRecordState {
+	return &state.NameserverRecordState{Records: records, Status: "ok"}
+}
+
+// failed is what a check saves for a nameserver that did not answer.
+func failed() *state.NameserverRecordState {
+	return &state.NameserverRecordState{
+		Records: map[string][]string{},
+		Status:  "error",
+		Error:   "all queries timed out",
+	}
+}
+
+// saved builds the hostname state a check saves.
+func saved(
+	byNameserver map[string]*state.NameserverRecordState,
+) *state.HostnameState {
+	return &state.HostnameState{RecordsByNameserver: byNameserver}
+}
+
+// alertCounts counts the hostname alerts sent, by kind.
+type alertCounts struct {
+	failures, recoveries, recordChanges, inconsistencies int
+}
+
+// countAlerts runs the hostname change detection from the state loaded
+// at startup through each check in turn, and counts the alerts sent.
+func countAlerts(
+	t *testing.T,
+	loaded *state.HostnameState,
+	checks []*state.HostnameState,
+) alertCounts {
+	t.Helper()
+
+	// The hostname change detection uses only the notifier.
+	notifier := &mockNotifier{}
+	w := watcher.NewForTest(nil, nil, nil, nil, nil, notifier)
+
+	prev := loaded
+
+	for _, current := range checks {
+		w.DetectHostnameChanges(t.Context(), host, prev, current)
+		prev = current
+	}
+
+	var got alertCounts
+
+	for _, n := range notifier.getNotifications() {
+		kind, _, _ := strings.Cut(n.Title, ":")
+
+		switch kind {
+		case "NS Failure":
+			got.failures++
+		case "NS Recovery":
+			got.recoveries++
+		case "Record Change":
+			got.recordChanges++
+		case "Inconsistency":
+			got.inconsistencies++
+		}
+	}
+
+	return got
+}
+
+func TestNSFailureAndRecoveryAlerts(t *testing.T) {
+	t.Parallel()
+
+	records := map[string][]string{"A": {ip1}}
+
+	bothAnswer := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(records), nsB: answered(records),
+	})
+	bFails := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(records), nsB: failed(),
+	})
+	onlyA := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(records),
+	})
+	bAnswersNoRecords := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(records), nsB: answered(map[string][]string{}),
+	})
+	bAnswersDifferently := saved(map[string]*state.NameserverRecordState{
+		nsA: answered(records), nsB: answered(map[string][]string{"A": {ip2}}),
+	})
+
+	// Each case starts from the state loaded at startup and runs the
+	// checks in order.
+	tests := []struct {
+		name   string
+		loaded *state.HostnameState
+		checks []*state.HostnameState
+		want   alertCounts
+	}{
+		{
+			"failure lasting several checks alerts once",
+			bothAnswer, []*state.HostnameState{bFails, bFails, bFails},
+			alertCounts{failures: 1},
+		},
+		{
+			"recovery alerts once",
+			bFails, []*state.HostnameState{bothAnswer, bothAnswer},
+			alertCounts{recoveries: 1},
+		},
+		{
+			"failing again after recovering alerts again",
+			bothAnswer, []*state.HostnameState{bFails, bothAnswer, bFails},
+			alertCounts{failures: 2, recoveries: 1},
+		},
+		{
+			"nameserver failing when first seen does not alert",
+			onlyA, []*state.HostnameState{bFails, bFails},
+			alertCounts{},
+		},
+		{
+			"answer with no records is a record change, not a failure",
+			bothAnswer, []*state.HostnameState{bAnswersNoRecords},
+			alertCounts{recordChanges: 1, inconsistencies: 1},
+		},
+		{
+			"recovered nameserver that answers differently disagrees",
+			bFails, []*state.HostnameState{bAnswersDifferently},
+			alertCounts{recoveries: 1, inconsistencies: 1},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := countAlerts(t, tt.loaded, tt.checks)
+			if got != tt.want {
+				t.Errorf("sent %+v, want %+v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestNSFailureAlertNamesHostnameNameserverAndReason(t *testing.T) {
+	t.Parallel()
+
+	records := map[string][]string{"A": {ip1}}
+
+	notifier := &mockNotifier{}
+	w := watcher.NewForTest(nil, nil, nil, nil, nil, notifier)
+
+	w.DetectHostnameChanges(
+		t.Context(), host,
+		saved(map[string]*state.NameserverRecordState{nsA: answered(records)}),
+		saved(map[string]*state.NameserverRecordState{nsA: failed()}),
+	)
+
+	notifications := notifier.getNotifications()
+	if len(notifications) != 1 {
+		t.Fatalf("sent %v, want one NS Failure", notifications)
+	}
+
+	msg := notifications[0].Message
+	if !strings.Contains(msg, host) || !strings.Contains(msg, nsA) ||
+		!strings.Contains(msg, failed().Error) {
+		t.Errorf(
+			"message %q does not name %s, %s and the reason",
+			msg, host, nsA,
+		)
+	}
+}
+
+// TestNameserverThatNeverAnswers asks a nameserver address where
+// nothing answers, 192.0.2.1, and checks what the watcher saves for it.
+// The deadline outlasts the resolver's first two-second try, as in the
+// resolver's timeout test.
+func TestNameserverThatNeverAnswers(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second)
+	t.Cleanup(cancel)
+
+	res := resolver.NewFromLogger(slog.Default())
+
+	resp, err := res.QueryNameserverIP(ctx, nsA, "192.0.2.1", host)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	hs := watcher.BuildHostnameState(
+		map[string]*resolver.NameserverResponse{nsA: resp}, time.Now(),
+	)
+
+	got := hs.RecordsByNameserver[nsA]
+	if got.Status != "error" || got.Error == "" {
+		t.Errorf(
+			"saved status %q, error %q; want status error with a reason",
+			got.Status, got.Error,
+		)
+	}
+}
+
+// TestNameserverThatAnswersNXDOMAIN asks a real nameserver about a name
+// that does not exist and checks what the watcher saves for it: NXDOMAIN
+// is an answer, so the nameserver is saved as ok with no error.
+func TestNameserverThatAnswersNXDOMAIN(t *testing.T) {
+	t.Parallel()
+
+	res := resolver.NewFromLogger(slog.Default())
+	name := "this-surely-does-not-exist-xyz." + testDomain
+
+	var (
+		ns   string
+		resp *resolver.NameserverResponse
+	)
+
+	livednstest.Retry(t, "QueryNameserver("+name+")", func(ctx context.Context) error {
+		nameservers, err := res.LookupNS(ctx, testDomain)
+		if err != nil {
+			return err
+		}
+
+		ns = nameservers[0]
+
+		resp, err = res.QueryNameserver(ctx, ns, name)
+		if err != nil {
+			return err
+		}
+
+		// A timeout or a failure is no answer to check.
+		if resp.Status == resolver.StatusTimeout ||
+			resp.Status == resolver.StatusError {
+			return fmt.Errorf(
+				"%w: %s: %s", livednstest.ErrNoAnswer, ns, resp.Error,
+			)
+		}
+
+		return nil
+	})
+
+	if resp.Status != resolver.StatusNXDomain {
+		t.Fatalf("%s answered %q for %s, want NXDOMAIN", ns, resp.Status, name)
+	}
+
+	hs := watcher.BuildHostnameState(
+		map[string]*resolver.NameserverResponse{ns: resp}, time.Now(),
+	)
+
+	got := hs.RecordsByNameserver[ns]
+	if got.Status != "ok" || got.Error != "" {
+		t.Errorf(
+			"saved status %q, error %q; want status ok with no error",
+			got.Status, got.Error,
+		)
+	}
+}
+
+// TestNameserverThatRefuses asks a google.com nameserver about
+// cloudflare.com, a zone it does not serve, which it refuses, and checks
+// what the watcher saves for it: REFUSED is no answer, so the nameserver
+// is saved as error with the reason.
+func TestNameserverThatRefuses(t *testing.T) {
+	t.Parallel()
+
+	const reason = "server returned REFUSED"
+
+	res := resolver.NewFromLogger(slog.Default())
+
+	var (
+		ns   string
+		resp *resolver.NameserverResponse
+	)
+
+	livednstest.Retry(
+		t,
+		"QueryNameserver(cloudflare.com)",
+		func(ctx context.Context) error {
+			nameservers, err := res.LookupNS(ctx, testDomain)
+			if err != nil {
+				return err
+			}
+
+			ns = nameservers[0]
+
+			resp, err = res.QueryNameserver(ctx, ns, "cloudflare.com")
+			if err != nil {
+				return err
+			}
+
+			// A timeout or a network error is no reply at all.
+			if resp.Status == resolver.StatusTimeout ||
+				strings.HasPrefix(resp.Error, "network error") {
+				return fmt.Errorf(
+					"%w: %s: %s", livednstest.ErrNoAnswer, ns, resp.Error,
+				)
+			}
+
+			return nil
+		},
+	)
+
+	if resp.Error != reason {
+		t.Fatalf(
+			"%s answered %q (%s) for cloudflare.com, want REFUSED",
+			ns, resp.Status, resp.Error,
+		)
+	}
+
+	hs := watcher.BuildHostnameState(
+		map[string]*resolver.NameserverResponse{ns: resp}, time.Now(),
+	)
+
+	got := hs.RecordsByNameserver[ns]
+	if got.Status != failed().Status || got.Error != reason {
+		t.Errorf(
+			"saved status %q, error %q; want status %q, error %q",
+			got.Status, got.Error, failed().Status, reason,
+		)
+	}
+}

internal/watcher/watcher.go

@@ -13,6 +13,7 @@ import (
 
 	"sneak.berlin/go/dnswatcher/internal/config"
 	"sneak.berlin/go/dnswatcher/internal/logger"
+	"sneak.berlin/go/dnswatcher/internal/resolver"
 	"sneak.berlin/go/dnswatcher/internal/state"
 	"sneak.berlin/go/dnswatcher/internal/tlscheck"
 )
@@ -102,28 +103,6 @@ func New(
 	return w, nil
 }
 
-// NewForTest creates a Watcher without fx for unit testing.
-func NewForTest(
-	cfg *config.Config,
-	st *state.State,
-	res DNSResolver,
-	pc PortChecker,
-	tc TLSChecker,
-	n Notifier,
-) *Watcher {
-	return &Watcher{
-		log:            slog.Default(),
-		config:         cfg,
-		state:          st,
-		resolver:       res,
-		portCheck:      pc,
-		tlsCheck:       tc,
-		notify:         n,
-		firstRun:       true,
-		expiryNotified: make(map[string]time.Time),
-	}
-}
-
 // Run starts the monitoring loop with periodic scheduling.
 func (w *Watcher) Run(ctx context.Context) {
 	w.log.Info(
@@ -249,7 +228,7 @@ func (w *Watcher) checkDomain(
 	// Also look up A/AAAA records for the apex domain so that
 	// port and TLS checks (which read HostnameState) can find
 	// the domain's IP addresses.
-	records, err := w.resolver.LookupAllRecords(ctx, domain)
+	results, err := w.resolver.LookupAllRecords(ctx, domain)
 	if err != nil {
 		w.log.Error(
 			"failed to lookup records for domain",
@@ -260,12 +239,13 @@ func (w *Watcher) checkDomain(
 		return
 	}
 
+	newState := buildHostnameState(results, now)
+
 	prevHS, hasPrevHS := w.state.GetHostnameState(domain)
 	if hasPrevHS && !w.firstRun {
-		w.detectHostnameChanges(ctx, domain, prevHS, records)
+		w.detectHostnameChanges(ctx, domain, prevHS, newState)
 	}
 
-	newState := buildHostnameState(records, now)
 	w.state.SetHostnameState(domain, newState)
 }
 
@@ -314,7 +294,7 @@ func (w *Watcher) checkHostname(
 	ctx context.Context,
 	hostname string,
 ) {
-	records, err := w.resolver.LookupAllRecords(ctx, hostname)
+	results, err := w.resolver.LookupAllRecords(ctx, hostname)
 	if err != nil {
 		w.log.Error(
 			"failed to lookup records",
@@ -325,19 +305,22 @@ func (w *Watcher) checkHostname(
 		return
 	}
 
-	now := time.Now().UTC()
-	prev, hasPrev := w.state.GetHostnameState(hostname)
+	newState := buildHostnameState(results, time.Now().UTC())
 
+	prev, hasPrev := w.state.GetHostnameState(hostname)
 	if hasPrev && !w.firstRun {
-		w.detectHostnameChanges(ctx, hostname, prev, records)
+		w.detectHostnameChanges(ctx, hostname, prev, newState)
 	}
 
-	newState := buildHostnameState(records, now)
 	w.state.SetHostnameState(hostname, newState)
 }
 
+// buildHostnameState saves each nameserver's response. A nameserver
+// that answered, even with NXDOMAIN or no records, is saved as ok; one
+// that timed out or failed is saved as error with the reason, and its
+// empty record set is not an answer.
 func buildHostnameState(
-	records map[string]map[string][]string,
+	results map[string]*resolver.NameserverResponse,
 	now time.Time,
 ) *state.HostnameState {
 	hs := &state.HostnameState{
@@ -347,12 +330,20 @@ func buildHostnameState(
 		LastChecked: now,
 	}
 
-	for ns, recs := range records {
-		hs.RecordsByNameserver[ns] = &state.NameserverRecordState{
-			Records:     recs,
+	for ns, resp := range results {
+		nsState := &state.NameserverRecordState{
+			Records:     resp.Records,
 			Status:      statusOK,
 			LastChecked: now,
 		}
+
+		if resp.Status == resolver.StatusTimeout ||
+			resp.Status == resolver.StatusError {
+			nsState.Status = statusError
+			nsState.Error = resp.Error
+		}
+
+		hs.RecordsByNameserver[ns] = nsState
 	}
 
 	return hs
@@ -361,27 +352,29 @@ func buildHostnameState(
 func (w *Watcher) detectHostnameChanges(
 	ctx context.Context,
 	hostname string,
-	prev *state.HostnameState,
-	current map[string]map[string][]string,
+	prev, current *state.HostnameState,
 ) {
 	w.detectRecordChanges(ctx, hostname, prev, current)
 	w.detectNSDisappearances(ctx, hostname, prev, current)
+	w.detectNSFailures(ctx, hostname, prev, current)
 	w.detectInconsistencies(ctx, hostname, prev, current)
 }
 
+// detectRecordChanges compares each nameserver's records with those of
+// the previous check. Only answers are compared: a nameserver that
+// failed on either check has no records to compare.
 func (w *Watcher) detectRecordChanges(
 	ctx context.Context,
 	hostname string,
-	prev *state.HostnameState,
-	current map[string]map[string][]string,
+	prev, current *state.HostnameState,
 ) {
-	for ns, recs := range current {
+	for ns, cur := range current.RecordsByNameserver {
 		prevNS, ok := prev.RecordsByNameserver[ns]
-		if !ok {
+		if !ok || prevNS.Status != statusOK || cur.Status != statusOK {
 			continue
 		}
 
-		if recordsEqual(prevNS.Records, recs) {
+		if recordsEqual(prevNS.Records, cur.Records) {
 			continue
 		}
 
@@ -389,7 +382,7 @@ func (w *Watcher) detectRecordChanges(
 			"Hostname: %s\nNameserver: %s\n"+
 				"Old: %v\nNew: %v",
 			hostname, ns,
-			prevNS.Records, recs,
+			prevNS.Records, cur.Records,
 		)
 
 		w.notify.SendNotification(
@@ -404,11 +397,10 @@ func (w *Watcher) detectRecordChanges(
 func (w *Watcher) detectNSDisappearances(
 	ctx context.Context,
 	hostname string,
-	prev *state.HostnameState,
-	current map[string]map[string][]string,
+	prev, current *state.HostnameState,
 ) {
 	for ns, prevNS := range prev.RecordsByNameserver {
-		if _, ok := current[ns]; ok || prevNS.Status != statusOK {
+		if _, ok := current.RecordsByNameserver[ns]; ok || prevNS.Status != statusOK {
 			continue
 		}
 
@@ -424,13 +416,36 @@ func (w *Watcher) detectNSDisappearances(
 			"error",
 		)
 	}
+}
 
-	for ns := range current {
+// detectNSFailures notifies when a nameserver that answered on the
+// previous check fails, and when one that failed answers again. A
+// nameserver missing from the previous check is not compared.
+func (w *Watcher) detectNSFailures(
+	ctx context.Context,
+	hostname string,
+	prev, current *state.HostnameState,
+) {
+	for ns, cur := range current.RecordsByNameserver {
 		prevNS, ok := prev.RecordsByNameserver[ns]
-		if !ok || prevNS.Status != statusError {
+		if !ok {
 			continue
 		}
 
+		switch {
+		case prevNS.Status == statusOK && cur.Status == statusError:
+			msg := fmt.Sprintf(
+				"Hostname: %s\nNameserver: %s\nError: %s",
+				hostname, ns, cur.Error,
+			)
+
+			w.notify.SendNotification(
+				ctx,
+				"NS Failure: "+hostname,
+				msg,
+				"error",
+			)
+		case prevNS.Status == statusError && cur.Status == statusOK:
 			msg := fmt.Sprintf(
 				"Hostname: %s\nNameserver: %s recovered",
 				hostname, ns,
@@ -444,12 +459,12 @@ func (w *Watcher) detectNSDisappearances(
 			)
 		}
 	}
+}
 
 func (w *Watcher) detectInconsistencies(
 	ctx context.Context,
 	hostname string,
-	prev *state.HostnameState,
-	current map[string]map[string][]string,
+	prev, current *state.HostnameState,
 ) {
 	for _, pair := range newlyDisagreeingPairs(prev, current) {
 		ns1, ns2 := pair[0], pair[1]
@@ -457,8 +472,8 @@ func (w *Watcher) detectInconsistencies(
 		msg := fmt.Sprintf(
 			"Hostname: %s\n%s: %v\n%s: %v",
 			hostname,
-			ns1, current[ns1],
-			ns2, current[ns2],
+			ns1, current.RecordsByNameserver[ns1].Records,
+			ns2, current.RecordsByNameserver[ns2].Records,
 		)
 
 		w.notify.SendNotification(
@@ -470,18 +485,21 @@ func (w *Watcher) detectInconsistencies(
 	}
 }
 
-// newlyDisagreeingPairs returns every pair of nameservers whose records
-// differ in current, in sorted order of name, except pairs where both
-// nameservers were in prev and already differed there. A nameserver
-// missing from prev is paired with every nameserver it differs from.
+// newlyDisagreeingPairs returns every pair of nameservers that answered
+// in current and whose records differ there, in sorted order of name,
+// except pairs where both nameservers answered in prev and already
+// differed there. A nameserver missing from prev, or that failed there,
+// is paired with every nameserver it differs from. A nameserver that
+// failed in current has no records to compare and is in no pair.
 func newlyDisagreeingPairs(
-	prev *state.HostnameState,
-	current map[string]map[string][]string,
+	prev, current *state.HostnameState,
 ) [][2]string {
-	nameservers := make([]string, 0, len(current))
-	for ns := range current {
+	nameservers := make([]string, 0, len(current.RecordsByNameserver))
+	for ns, cur := range current.RecordsByNameserver {
+		if cur.Status == statusOK {
 			nameservers = append(nameservers, ns)
 		}
+	}
 
 	sort.Strings(nameservers)
 
@@ -489,14 +507,19 @@ func newlyDisagreeingPairs(
 
 	for i, ns1 := range nameservers {
 		for _, ns2 := range nameservers[i+1:] {
-			if recordsEqual(current[ns1], current[ns2]) {
+			if recordsEqual(
+				current.RecordsByNameserver[ns1].Records,
+				current.RecordsByNameserver[ns2].Records,
+			) {
 				continue
 			}
 
 			prev1, ok1 := prev.RecordsByNameserver[ns1]
 			prev2, ok2 := prev.RecordsByNameserver[ns2]
 
-			if ok1 && ok2 && !recordsEqual(prev1.Records, prev2.Records) {
+			if ok1 && ok2 &&
+				prev1.Status == statusOK && prev2.Status == statusOK &&
+				!recordsEqual(prev1.Records, prev2.Records) {
 				continue
 			}
 

internal/watcher/watcher_test.go

@@ -9,8 +9,12 @@ import (
 	"testing"
 	"time"
 
+	"go.uber.org/fx/fxtest"
+
 	"sneak.berlin/go/dnswatcher/internal/config"
+	"sneak.berlin/go/dnswatcher/internal/globals"
 	"sneak.berlin/go/dnswatcher/internal/livednstest"
+	"sneak.berlin/go/dnswatcher/internal/logger"
 	"sneak.berlin/go/dnswatcher/internal/portcheck"
 	"sneak.berlin/go/dnswatcher/internal/resolver"
 	"sneak.berlin/go/dnswatcher/internal/state"
@@ -148,7 +152,24 @@ func newTestWatcher(
 		config:   cfg,
 	}
 
-	deps.state = state.NewForTest()
+	g, err := globals.New(nil)
+	if err != nil {
+		t.Fatalf("globals.New: %v", err)
+	}
+
+	log, err := logger.New(nil, logger.Params{Globals: g})
+	if err != nil {
+		t.Fatalf("logger.New: %v", err)
+	}
+
+	// The watcher saves state after every check, into cfg.DataDir.
+	deps.state, err = state.New(fxtest.NewLifecycle(t), state.Params{
+		Logger: log,
+		Config: cfg,
+	})
+	if err != nil {
+		t.Fatalf("state.New: %v", err)
+	}
 
 	w := watcher.NewForTest(
 		deps.config,
@@ -666,11 +687,12 @@ func TestNSFailureAndRecovery(t *testing.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.
+	// as one that did not answer, and add, as answering, one that live
+	// DNS does not list, which then disappears.
 	deps := runChecks(t, cfg, nil, func(deps *testDeps) {
 		hs, _ := deps.state.GetHostnameState(testHost)
-		for _, nsState := range hs.RecordsByNameserver {
-			nsState.Status = "error"
+		for ns := range hs.RecordsByNameserver {
+			hs.RecordsByNameserver[ns] = failed()
 		}
 
 		hs.RecordsByNameserver[oldNS1] = &state.NameserverRecordState{
@@ -683,4 +705,10 @@ func TestNSFailureAndRecovery(t *testing.T) {
 
 	assertNotified(t, deps, "NS Failure: "+testHost, "error")
 	assertNotified(t, deps, "NS Recovery: "+testHost, "success")
+
+	// A nameserver that did not answer has no records to compare, so
+	// its recovery is not also a record change.
+	if n := countNotifications(deps, "Record Change: "+testHost); n != 0 {
+		t.Errorf("sent %d record changes on recovery, want 0", n)
+	}
 }