9 Commits
Author SHA1 Message Date
clawbot 6b0fdc477a test: restore transport-failure coverage with loopback nameservers
check / check (push) Failing after 13s
The DNS-mock removal deleted TestQueryNameserverIP_Timeout and left a
comment in its place, so the resolver's StatusTimeout / StatusError
classification branch went untested. The stated obstacle was that a
query to a black-holed RFC 5737 address comes back StatusOK, because
the build environment transparently intercepts UDP/53 and answers it
locally. That is a property of that environment, not of the resolver,
and it only rules out choosing a remote address.

internal/resolver/transport_test.go binds real nameservers on
127.0.0.1 instead and aims the query at them: one silent on A queries
and answering every other type (StatusTimeout), one answering SERVFAIL
(StatusError), and one address with nothing listening, which is
refused rather than dropped and so classifies as NoData. This is not a
mock — no DNSClient is substituted. The resolver dials a real socket,
writes a real query with the real miekg/dns client, and applies its
real deadline and real classification logic to what comes back.
Substituting the client is what TESTING.md bans; choosing the server
is not, and the resolver is aimed at a caller-chosen nameserver in
production too.

queryDNS now dials a nameserver address that already carries a port as
written, defaulting to 53 only for a bare address. That is what makes
a nameserver on any other port reachable, on loopback or otherwise.

Silence on one record type rather than all eight keeps the timeout
test to two query timeouts (4s) instead of sixteen (32s), and it is
asserted: the test fails if it ever costs more than 8s.

The watcher's assertStatePopulated and TestDomainPortAndTLSChecks
asserted only that hostname, port and certificate state were
non-empty, plus non-zero checker call counts. Neither was vacuous, but
neither would have caught the watcher resolving the wrong addresses.
The port and TLS test doubles now record their arguments, and both
tests assert that the state keys and the arguments the checkers were
actually called with match the addresses live DNS returned — exactly
those, no more and no fewer. Verified by mutation: making the watcher
drop all but one resolved address fails both tests, and it passed both
of them before.

TESTING.md records why a loopback nameserver is not a mock, so the new
tests are not mistaken for a violation of the rule they respect.
2026-09-03 22:58:01 +00:00
clawbot 62dec447e3 test: bound watcher live DNS and serialise packages after rebase
check / check (push) Successful in 1m37s
Rebasing this branch onto next surfaced two failures that the branch
did not have in isolation. Both come from the change this PR makes:
the watcher package now queries live DNS, and it is the second package
to do so.

Burst fan-out in the watcher package. All 13 watcher tests are
parallel and each runs a full iterative resolution, so they hit the
same root servers in the same instant and get rate-limited. This is
exactly the pathology internal/resolver/livedns_test.go was written to
prevent (9cb2c2b); that gate is package-scoped and cannot reach into
this package's test binary, so liveWatcherGate is its counterpart
here, acquired in newTestWatcher and released when the test ends.

Cross-package oversubscription. Go runs package binaries in parallel,
so with both live-DNS packages in flight their gates sum rather than
hold. The excess is rate-limited and the resolver's 8s per-attempt
deadlines expire, failing ResolveIPAddresses tests this branch never
touched. script/test now passes -p 1 so each gate is authoritative
while its package runs. Serialising is also net faster here, because
the retries it removes cost more than the lost parallelism: resolver
16-22s (was 30-36s under contention), watcher 12-13s (was 27-37s),
whole suite 39-46s against the 60s cap and the 90s -timeout backstop.

TestQueryNameserverIP_UnreachableServer is dropped rather than fixed.
It asserted that a query to an RFC 5737 documentation address comes
back non-OK with no records, which does not hold in the build
environment: that network transparently intercepts all UDP/53 traffic
regardless of destination and answers it locally, so the query returns
StatusOK with 9 real records for example.com. Verified directly with
dig @192.0.2.1 inside the build network. The mock DNSClient that used
to force this classification is what this PR removes, and a live
substitute would only be testing the sandbox's network behaviour, so
the coverage gap is recorded in a comment where the test was.

Verified: three consecutive `docker build .` runs green, after three
consecutive failures without these changes.
2026-09-03 17:01:08 +00:00
sneak 11b9b5527d Remove DNS mocking from tests; use live DNS everywhere
DNS is never mocked in this repository: tests exercise live DNS,
and robustness comes from handling real-world DNS behavior with
tolerant assertions and sensible timeouts, not from mocks.

watcher: drop mockResolver and wire the real iterative resolver
into the tests, querying stable public names (example.com,
www.example.com). Change detection is exercised by seeding the
state store with a synthetic previous observation that live DNS
cannot match (reserved .invalid nameserver names and RFC 5737
documentation addresses); DNS stays live in every run. The port
checker, TLS checker, and notifier remain test doubles since they
are not DNS, keeping notification and state assertions
deterministic against whatever addresses live DNS returns.

resolver: drop the timeoutClient fake DNSClient and the
NewFromLoggerWithClient mock constructor. The timeout test is
replaced by a live query against an RFC 5737 documentation
address where no nameserver can exist, asserting a classified
non-OK response with no records.

TESTING.md: extend the live-DNS policy to every package and
remove the carve-out that permitted DNS mocks in packages that
consume the resolver.

TODO.md: update stale references to hermetic mocked-DNS work to
reflect the no-mocking policy and the current state of
feature/resolver.

Intentionally dropped coverage: the exact StatusTimeout
classification (previously forced by the fake client) is no
longer asserted, because a genuinely unreachable server may fail
fast instead of timing out depending on the network path; the
live test tolerantly accepts any failure classification.
2026-09-03 16:43:25 +00:00
sneak b8662b8a9c docs: correct stale script headers and record the config-verify cost (closes #137)
check / check (push) Successful in 51s
script/bootstrap credited the goimports pin to script/fmt-check, which
runs gofmt only; the header now credits script/fmt. script/cibuild
still described the Dockerfile as running make check, which stopped
being true once linting moved to its own stage. The docker-missing
warning in script/bootstrap is one sentence instead of three fragments
each carrying the bootstrap: prefix. Dockerfile.lint now states the
residual risk of skipping golangci-lint config verify: unknown
top-level keys in .golangci.yml are ignored silently, so a mistyped key
lints clean and applies nothing.

Comment and message text only; no behaviour changes.
2026-08-10 14:09:20 +00:00
sneak 168281ad60 docs: add MIT LICENSE and state the licence in the README (closes #102)
check / check (push) Has been cancelled
The repository had no licence file at all, which makes publicly readable
code all-rights-reserved by default: nobody may legally use it. That is a
1.0 blocker rather than a nicety, and `LICENSE` was the only file from
`REPO_POLICIES.md`'s required minimum still missing here.

The choice is standing org policy rather than a per-repo call: any public
repo lacking a licence gets MIT, while a private repo with no licence is
already all-rights-reserved and needs nothing. `sneak/dnswatcher` is
public, so MIT.

`LICENSE` carries the canonical MIT text byte-for-byte with only the
copyright line filled in; no clauses added, removed, reworded, or
reflowed. `README.md`'s first line now names the licence, which the
Description requirement in `REPO_POLICIES.md` calls for, and the License
section states MIT and points at the file instead of recording the
decision as pending.
2026-08-10 14:04:31 +00:00
sneak 6f6bf3a65b test: disable Go's test cache so every run queries live DNS (closes #139)
check / check (push) Successful in 1m34s
`script/test` did not pass `-count=1`, so on an unchanged tree Go
served the whole suite from its test cache: exit 0 in ~0.2s with every
package marked `(cached)` and not one DNS query made. This repo's suite
exists to exercise live resolution on every run (`TESTING.md`), so that
green asserted nothing — and it is exactly the green used as evidence
that a flakiness fix works, since "run it a few times" stops being
runs after the first.

`-count=1` now disables caching on every invocation.

The conditional verbose rerun that `REPO_POLICIES.md` mandates was
missing at the same spot and is added here rather than left broken: the
primary run had been unconditionally `-v`, which is the failure mode
the policy exists to prevent (unreadable CI and `docker build` logs on
success). Tests now run quiet, and only a failure triggers the `-v`
rerun. The rerun carries `-count=1` too, so it cannot replay a cached
copy of the failure it is meant to diagnose, and its exit status is
discarded in favour of a forced 1: the first failure already proved the
suite broken, so a flake that passes the second time must not turn the
build green.

`-timeout 90s` is untouched. It is a deliberate backstop that must
strictly exceed the 60s hard cap on suite duration.

No special-casing for the Docker build, which also reaches this script
via `RUN make test`: a fresh container's test cache is empty, so
`-count=1` changes nothing there and carving out an exception would
only create a second code path that could drift.

Verified: three back-to-back `make test` runs on an unchanged tree,
zero `(cached)` markers, ~4.0-4.5s wall each (was ~0.2s cached),
comfortably inside the 20s target with `-race` and `-cover` both still
working and coverage percentages unchanged. The rerun-and-still-fail
path was exercised against a purpose-built flaky test that fails once
then passes: quiet failure, verbose rerun that genuinely re-executed,
exit 1 regardless. `make check` green.
2026-08-10 13:48:15 +00:00
sneak 87bce43f8d test: rework live-DNS quorum unit — tolerate silence, never a wrong answer
check / check (push) Successful in 1m23s
Rework of the unit at #93
(commit 9cb2c2b), against the review at
#136 (comment).

Review of 9cb2c2b found the quorum assertions could not fail on a
class of wrong answer. Each test banned exactly one bad status —
_AllReturnOK banned only nxdomain, _NXDomainFromAllNS banned only ok
— so resolver.StatusNoData passed both. nodata is a wrong answer, not
silence, and answeredCount counted it as answered, so it did not even
trigger a retry; with a quorum of 3 of 4 a single wrong nameserver
slid through undetected. That is assertion-loosening beyond what the
quorum change requires.

Tolerance is now a closed allowlist rather than a blocklist of one
status. unsanctionedStatuses() reports every per-nameserver result
whose status the caller did not explicitly sanction: ok/timeout/error
for the all-OK test, nxdomain/timeout/error for the NXDOMAIN test.
Silence (timeout, error) is the only thing quorum exists to tolerate;
any other status, including one added to the resolver later, fails by
name. answeredCount is likewise an allowlist of ok/nxdomain/nodata, so
an unknown status counts as silence and can only cause a retry and
then a loud failure, never a quiet pass.

Two harness tests cover the regression directly: three OK plus one
nodata (quorum satisfied, no nxdomain present — the input that used
to pass) is now reported as unsanctioned, and an unknown status is
neither counted as answered nor tolerated.

Verified by re-running the reviewer's probe: queryEachNS patched to
force one of google.com's four nameservers to return StatusNoData
turns both tests red, naming the offending nameserver and status —

    --- FAIL: TestQueryAllNameservers_AllReturnOK (1.12s)
        Should be empty, but was [ns1.google.com.=nodata]
        every nameserver must answer OK or not answer at all:
        ns1.google.com.=nodata ns2.google.com.=ok ns3.google.com.=ok
        ns4.google.com.=ok
    --- FAIL: TestQueryAllNameservers_NXDomainFromAllNS (1.34s)
        Should be empty, but was [ns1.google.com.=nodata]
        every nameserver must report NXDOMAIN or not answer at all:
        ns1.google.com.=nodata ns2.google.com.=nxdomain
        ns3.google.com.=nxdomain ns4.google.com.=nxdomain

— and green with the probe reverted. Also fixes the review's nit: the
per-attempt deadline assertion had no lower bound, so it passed for a
deadline far shorter than intended.

No production code changed; DNS is still never mocked.
2026-08-10 13:33:53 +00:00
sneak 9cb2c2b7e0 test: make live DNS tests robust instead of gated (closes #93)
check / check (push) Successful in 1m18s
The resolver's live-DNS tests failed nondeterministically, a different
subset each run. Three structural causes, all test-side:

- Burst fan-out. Every test in the package is parallel and the build
  hosts have many cores, so all ~35 iterative resolutions started at
  the same instant and, because queryServers walks rootServerList() in
  fixed order, hit the same root server within milliseconds. Root
  servers rate-limit that.
- No retry anywhere. One dropped UDP packet in a delegation chain
  failed a test outright.
- Unanimity assertions. TestQueryAllNameservers_AllReturnOK and
  _NXDomainFromAllNS required every one of a domain's nameservers to
  answer, with no tolerance for one being slow.

New internal/resolver/livedns_test.go addresses each: a package-wide
gate bounds how many live resolutions are in flight at once, every
live operation gets three attempts with exponential backoff and its
own deadline, and multi-nameserver assertions now need a strict
majority rather than unanimity. The retry predicate is deliberately
transport-level -- "did a nameserver answer at all" -- never the
assertion under test, so a resolver that answers incorrectly still
fails on the first attempt. A nameserver that stays silent is
tolerated; one that answers wrongly is not.

livedns_harness_test.go tests that machinery directly: quorum
arithmetic, status counting, the gate's concurrency bound, per-attempt
deadlines, and recovery from a transient failure. It touches no DNS.

Nothing is mocked, faked, stubbed, recorded, skipped or build-tagged,
and production resolver behaviour is unchanged.

Test caps move to the new org-wide values ruled at prompts issue 41:
60s hard cap, 20s target, 90s -timeout backstop. REPO_POLICIES.md is
re-vendored byte-identical from sneak/prompts rather than hand-edited,
which also picks up the golangci-lint paragraph this copy had drifted
behind on. TESTING.md's stale 30-second target follows to 60.

#93
2026-08-10 13:09:17 +00:00
sneak cc86473410 build: run all linting in Docker via Dockerfile.lint (closes #134)
check / check (push) Successful in 1m17s
golangci-lint is no longer installed or run on the host. script/lint is
now a thin wrapper that builds the new root Dockerfile.lint, which COPYs
the repo into the digest-pinned golangci/golangci-lint:v2.12.2 image and
lints as a build step, so a successful build is a clean lint. This works
even where the docker daemon is remote and bind mounts are impossible.

Dockerfile.lint is split into a deps stage (base image, go mod download)
and a lint stage (source copy, linter run). script/lint passes
--no-cache-filter=lint so the lint stage executes on every invocation:
caching is explicitly waived for linting, and a cached build lints
nothing. The deps stage stays cached and no global cache invalidation is
performed. --progress=plain keeps the linter's own output visible.

golangci-lint config verify is deliberately omitted: it fetches its JSON
schema over a live, unpinned HTTPS call, which would make linting
network-dependent and defeat hash-pinning.

script/bootstrap no longer installs golangci-lint and warns instead when
docker is absent. The goimports install stays, since script/fmt and
script/fmt-check still run it on the host.

The root Dockerfile ran make check in its builder stage, which would now
recurse into script/lint and shell out to docker build with no daemon
available. It gains its own lint stage on the same pinned image, invoked
directly, with the builder depending on it via COPY --from=lint and
running make fmt-check, make test and make build.
2026-08-10 12:37:47 +00:00
15 changed files with 1102 additions and 1359 deletions
+3 -10
View File
@@ -218,8 +218,7 @@ internal/
- **Structured logging**: All logs use `log/slog` with JSON output in
production (TTY detection for development).
- **Graceful shutdown**: All background goroutines respect context
cancellation and the fx lifecycle. In-flight notification deliveries
are drained on shutdown, bounded by the shutdown timeout.
cancellation and the fx lifecycle.
---
@@ -464,14 +463,8 @@ docker run -d \
from a previous cycle.
4. **On change detection**: Send notifications to all configured
endpoints, update in-memory state, persist to disk.
5. **Shutdown**: Persist final state to disk, wait for in-flight
notification deliveries to complete, stop gracefully. The wait is
bounded by the fx shutdown timeout (15s by default): deliveries still
retrying against an unreachable endpoint when that expires are
abandoned, and the number abandoned is logged at warn level rather
than dropped silently. Notifications generated after shutdown has
begun are refused and logged, so a late burst cannot extend the
shutdown.
5. **Shutdown**: Persist final state to disk, complete in-flight
notifications, stop gracefully.
---
+58 -4
View File
@@ -2,8 +2,10 @@
## DNS Resolution Tests
All resolver tests **MUST** use live queries against real DNS servers.
No mocking of the DNS client layer is permitted.
All tests that involve DNS resolution — in every package, including
consumers of the resolver such as the watcher — **MUST** use live
queries against real DNS servers. No mocking, faking, or stubbing of
DNS at any layer is permitted.
### Rationale
@@ -12,6 +14,8 @@ the full delegation chain. Mocked responses cannot faithfully represent
the variety of real-world DNS behavior (truncation, referrals, glue
records, DNSSEC, varied response times, EDNS, etc.). Testing against
real servers ensures the resolver works correctly in production.
Robustness comes from handling real-world DNS behavior with tolerant
assertions and sensible timeouts, not from mocks.
### Constraints
@@ -24,14 +28,64 @@ real servers ensures the resolver works correctly in production.
- Flaky failures from transient network issues are acceptable and
should be investigated as potential resolver bugs, not papered over
with mocks or skip flags
- Watcher change-detection tests seed a synthetic *previous state*
and compare it against fresh live lookups; the DNS side is never
faked
- Live query concurrency is bounded per package (`liveGate` in
`internal/resolver`, `liveWatcherGate` in `internal/watcher`) so
parallel tests do not burst at the root servers
- Those gates are package-scoped and therefore per test binary, so
`script/test` also passes `-p 1`: with both live-DNS packages
running at once the gates sum instead of holding, and the resolver's
per-attempt deadlines start expiring
### Transport failures: loopback nameservers, not mocks
The resolver classifies a nameserver that stays silent as
`StatusTimeout` and one that answers SERVFAIL as `StatusError`. The
public network cannot be made to produce either on demand — a
black-holed address is only black-holed on some networks, and build
environments that transparently intercept UDP/53 answer it locally —
so a test built on a chosen remote address asserts on the network it
happens to run on rather than on the resolver.
`internal/resolver/transport_test.go` binds a real nameserver on
`127.0.0.1` instead and points the query at it.
`nameserverAddr` dials an address that already carries a port as
written, so no production behaviour is bypassed to arrange this.
**This is permitted, and it is not a mock.** The rule above bans
substituting `DNSClient` or any other DNS abstraction, which lets the
code under test skip DNS and hands it a manufactured verdict. A
loopback nameserver does the opposite: the resolver dials a real
socket, writes a real query with the real `miekg/dns` client, and
applies its real deadline and its real classification logic to what
comes back. Choosing which nameserver a live query is sent to is not
faking DNS — the resolver is aimed at a nameserver of the caller's
choosing in production too.
The distinction to hold on to: **substituting the client is banned;
choosing the server is not.** A test that reaches for a fake
`DNSClient` to force a classification is still forbidden, no matter
how awkward the alternative looks.
Such a test must stay cheap. The resolver asks for eight record
types and retries each once, so a nameserver silent on every type
costs sixteen query timeouts. `TestQueryNameserverIP_Timeout` is
silent on `A` alone and answers the rest, which is all the resolver
needs to classify the response and keeps the test to two.
### What NOT to do
- **Do not mock `DNSClient`** for resolver tests (the mock constructor
exists for unit-testing other packages that consume the resolver)
- **Do not mock `DNSClient`**, the watcher's `DNSResolver` interface,
or any other DNS abstraction — in any package, for any reason
- **Do not add `-short` flags** to skip slow tests
- **Do not increase `-timeout`** to hide hanging queries
- **Do not remove `-count=1` from `script/test`** — Go's test cache
replays a previous run's output without querying anything, so a
cached pass is not evidence that live resolution works
- **Do not remove `-p 1` from `script/test`** — running the live-DNS
packages in parallel oversubscribes live DNS past what their gates
bound, which surfaces as unrelated resolver tests failing on
expired deadlines
- **Do not modify linter configuration** to suppress findings
+33 -16
View File
@@ -14,7 +14,10 @@ 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.
iterative resolver implementation. DNS mocking is banned in this repo
(see `TESTING.md`): all tests use live DNS only. The hermetic mocked
tests previously noted on `feature/resolver` are gone from its current
tip, which carries a live-DNS suite against `*.dns.sneak.cloud`.
# Next Step
@@ -23,6 +26,22 @@ Rationale, Design, TODO, License, Author) if any are still missing.
# Completed Steps
- 2026-09-03: restored the transport-failure classification coverage
the DNS-mock removal had dropped, and tightened two over-tolerant
watcher assertions. `internal/resolver/transport_test.go` covers
`StatusTimeout`, `StatusError` and the connection-refused path by
binding real nameservers on loopback rather than by mocking
`DNSClient` or by aiming a query at a remote address and hoping the
network black-holes it; `queryDNS` now honours a port already
present in a nameserver address, which is what lets a query be
aimed at one. The watcher's `assertStatePopulated` and
`TestDomainPortAndTLSChecks` now assert that port and certificate
state, and the arguments the port and TLS checkers were called
with, match the addresses live DNS returned — previously they
asserted only that those sets were non-empty, which would not have
caught resolving the wrong addresses. `TESTING.md` records why a
loopback nameserver is not a mock.
- 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`
@@ -92,15 +111,10 @@ Rationale, Design, TODO, License, Author) if any are still missing.
`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-07: DNS mocking removed from the entire test suite; watcher
tests now drive the real iterative resolver against live DNS and
`TESTING.md` bans DNS mocks in every package (`remove-dns-mocking`
branch)
- 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
@@ -112,7 +126,8 @@ Rationale, Design, TODO, License, Author) if any are still missing.
- 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)
with mocked DNS (origin/feature/resolver, unmerged; superseded — DNS
mocking is banned, see `TESTING.md`)
- 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-02-20: watcher monitoring orchestrator merged to main (#8)
@@ -137,8 +152,9 @@ 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,
- Merge in-flight branches to main once green: feature/resolver
(confirm its tests remain live-DNS — DNS mocking is banned, see
`TESTING.md`), ci/make-check, feature/portcheck-implementation,
feature/tlscheck-implementation
Resolver (plan from untracked TODO.md; largely implemented on
@@ -222,6 +238,7 @@ 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
- Resolver tests originally used live DNS against *.dns.sneak.cloud
(required records documented in the test file header); origin now has
mocked hermetic tests, keep them hermetic
- Resolver tests originally used live DNS against `*.dns.sneak.cloud`
(required records documented in the test file header); `main` now
tests against live public DNS. DNS mocking is banned (see
`TESTING.md`); never reintroduce hermetic mocked DNS tests
+5 -21
View File
@@ -32,27 +32,11 @@ func NewRequestForTest(
// NewTestService creates a Service suitable for unit testing.
// It discards log output and uses the given transport.
func NewTestService(transport http.RoundTripper) *Service {
return newService(slog.New(slog.DiscardHandler), transport)
}
// NewTestServiceWithLogger creates a Service that writes to the
// given handler, so tests can assert on emitted log records.
func NewTestServiceWithLogger(
transport http.RoundTripper,
handler slog.Handler,
) *Service {
return newService(slog.New(handler), transport)
}
// Drain exports drain for testing.
func (svc *Service) Drain(ctx context.Context) {
svc.drain(ctx)
}
// OutstandingDeliveries reports how many delivery goroutines
// are currently tracked as in flight.
func (svc *Service) OutstandingDeliveries() int64 {
return svc.outstanding.Load()
return &Service{
log: slog.New(slog.DiscardHandler),
transport: transport,
history: NewAlertHistory(),
}
}
// SetNtfyURL sets the ntfy URL on a Service for testing.
+56 -73
View File
@@ -12,8 +12,6 @@ import (
"log/slog"
"net/http"
"net/url"
"sync"
"sync/atomic"
"time"
"go.uber.org/fx"
@@ -117,41 +115,19 @@ type Service struct {
history *AlertHistory
retryConfig RetryConfig
sleepFn func(time.Duration) <-chan time.Time
// Shutdown draining state. drainMu guards draining and
// serialises it against the counter increment in
// startDelivery; inFlight tracks the delivery goroutines
// themselves and outstanding mirrors its count so a timed
// out drain can report how many were abandoned.
drainMu sync.Mutex
draining bool
inFlight sync.WaitGroup
outstanding atomic.Int64
abandon chan struct{}
abandonOnce sync.Once
}
// newService builds a Service with the fields every Service
// needs regardless of how it was constructed.
func newService(
log *slog.Logger,
transport http.RoundTripper,
) *Service {
return &Service{
log: log,
transport: transport,
history: NewAlertHistory(),
abandon: make(chan struct{}),
}
}
// New creates a new notify Service.
func New(
lifecycle fx.Lifecycle,
_ fx.Lifecycle,
params Params,
) (*Service, error) {
svc := newService(params.Logger.Get(), http.DefaultTransport)
svc.config = params.Config
svc := &Service{
log: params.Logger.Get(),
transport: http.DefaultTransport,
config: params.Config,
history: NewAlertHistory(),
}
if params.Config.NtfyTopic != "" {
u, err := ValidateWebhookURL(
@@ -192,14 +168,6 @@ func New(
svc.mattermostWebhookURL = u
}
lifecycle.Append(fx.Hook{
OnStop: func(ctx context.Context) error {
svc.drain(ctx)
return nil
},
})
return svc, nil
}
@@ -226,32 +194,6 @@ func (svc *Service) SendNotification(
svc.dispatchMattermost(ctx, title, message, priority)
}
// dispatch delivers a notification to one endpoint on a
// tracked background goroutine.
//
// The delivery context is detached from ctx with
// context.WithoutCancel so that a cancelled caller does not
// kill a delivery already under way; the shutdown drain, not
// the caller, decides how long deliveries may keep running.
func (svc *Service) dispatch(
ctx context.Context,
endpoint string,
send func(context.Context) error,
) {
notifyCtx := context.WithoutCancel(ctx)
svc.startDelivery(endpoint, func() {
err := svc.deliverWithRetry(notifyCtx, endpoint, send)
if err != nil {
svc.log.Error(
"failed to send notification after retries",
"endpoint", endpoint,
"error", err,
)
}
})
}
func (svc *Service) dispatchNtfy(
ctx context.Context,
title, message, priority string,
@@ -260,11 +202,26 @@ func (svc *Service) dispatchNtfy(
return
}
svc.dispatch(ctx, "ntfy", func(c context.Context) error {
go func() {
notifyCtx := context.WithoutCancel(ctx)
err := svc.deliverWithRetry(
notifyCtx, "ntfy",
func(c context.Context) error {
return svc.sendNtfy(
c, svc.ntfyURL, title, message, priority,
c, svc.ntfyURL,
title, message, priority,
)
})
},
)
if err != nil {
svc.log.Error(
"failed to send ntfy notification "+
"after retries",
"error", err,
)
}
}()
}
func (svc *Service) dispatchSlack(
@@ -275,11 +232,26 @@ func (svc *Service) dispatchSlack(
return
}
svc.dispatch(ctx, "slack", func(c context.Context) error {
go func() {
notifyCtx := context.WithoutCancel(ctx)
err := svc.deliverWithRetry(
notifyCtx, "slack",
func(c context.Context) error {
return svc.sendSlack(
c, svc.slackWebhookURL, title, message, priority,
c, svc.slackWebhookURL,
title, message, priority,
)
})
},
)
if err != nil {
svc.log.Error(
"failed to send slack notification "+
"after retries",
"error", err,
)
}
}()
}
func (svc *Service) dispatchMattermost(
@@ -290,8 +262,11 @@ func (svc *Service) dispatchMattermost(
return
}
svc.dispatch(
ctx, "mattermost",
go func() {
notifyCtx := context.WithoutCancel(ctx)
err := svc.deliverWithRetry(
notifyCtx, "mattermost",
func(c context.Context) error {
return svc.sendSlack(
c, svc.mattermostWebhookURL,
@@ -299,6 +274,14 @@ func (svc *Service) dispatchMattermost(
)
},
)
if err != nil {
svc.log.Error(
"failed to send mattermost notification "+
"after retries",
"error", err,
)
}
}()
}
func (svc *Service) sendNtfy(
-9
View File
@@ -2,7 +2,6 @@ package notify
import (
"context"
"fmt"
"math"
"math/rand/v2"
"time"
@@ -122,14 +121,6 @@ func (svc *Service) deliverWithRetry(
select {
case <-ctx.Done():
return ctx.Err()
case <-svc.abandon:
// Shutdown drained past its deadline; stop
// sleeping rather than outlive the process.
// A nil channel (Service built without a
// constructor) simply never fires.
return fmt.Errorf(
"%w: %s", ErrDeliveryAbandoned, endpoint,
)
case <-svc.sleepFunc(delay):
}
}
-119
View File
@@ -1,119 +0,0 @@
package notify
import (
"context"
"errors"
)
// ErrDeliveryAbandoned is returned by a retry loop that was
// cut short because shutdown drained past its deadline.
var ErrDeliveryAbandoned = errors.New(
"notification delivery abandoned at shutdown",
)
// startDelivery runs fn on its own goroutine while tracking it,
// so that drain can wait for it during shutdown.
//
// The WaitGroup counter is incremented here, on the caller's
// goroutine, before the worker exists: incrementing it inside
// the worker would race with drain's Wait and could let
// shutdown sail past a delivery that had not started yet.
//
// Once draining has begun the delivery is refused outright
// rather than queued, so a steady stream of newly submitted
// notifications cannot keep extending the drain.
func (svc *Service) startDelivery(endpoint string, fn func()) {
svc.drainMu.Lock()
if svc.draining {
svc.drainMu.Unlock()
svc.log.Warn(
"notification not dispatched: shutdown in progress",
"endpoint", endpoint,
)
return
}
svc.outstanding.Add(1)
// WaitGroup.Go increments the counter synchronously, here,
// and only then starts the goroutine.
svc.inFlight.Go(func() {
// Runs before the WaitGroup counter is decremented, so
// a drain that times out reports an accurate count.
defer svc.outstanding.Add(-1)
fn()
})
svc.drainMu.Unlock()
}
// drain waits for in-flight notification deliveries to finish.
//
// It first stops accepting new deliveries, then waits until
// either every outstanding delivery has completed or ctx
// expires — whichever comes first. ctx is the context fx
// passes to the OnStop hook, so a permanently dead webhook
// cannot hang shutdown indefinitely.
//
// When the deadline arrives with deliveries still outstanding,
// the count is logged at warn level and the abandon channel is
// closed, which releases any retry loop sleeping in backoff.
// Deliveries already inside an HTTP round trip are bounded by
// the existing httpClientTimeout instead.
//
// A ctx that is already expired on entry is not by itself cause
// for alarm: if nothing is outstanding there is nothing to
// abandon, and the drain says so at debug level rather than
// warning about deliveries that do not exist.
func (svc *Service) drain(ctx context.Context) {
svc.drainMu.Lock()
svc.draining = true
svc.drainMu.Unlock()
done := make(chan struct{})
go func() {
svc.inFlight.Wait()
close(done)
}()
select {
case <-done:
svc.log.Debug(
"all in-flight notifications completed",
)
case <-ctx.Done():
// outstanding is decremented before the WaitGroup
// counter, and startDelivery can no longer add to it
// now that draining is set, so a zero here means every
// delivery really did finish. ctx expiring in that
// state (an OnStop context that was already cancelled
// on entry is the usual way) abandons nothing, so it
// must not close abandon or warn about it.
abandoned := svc.outstanding.Load()
if abandoned == 0 {
svc.log.Debug(
"all in-flight notifications completed",
)
return
}
svc.abandonOnce.Do(func() {
if svc.abandon != nil {
close(svc.abandon)
}
})
svc.log.Warn(
"shutdown deadline reached with notifications "+
"still in flight; abandoning them",
"abandoned", abandoned,
"error", ctx.Err(),
)
}
}
-531
View File
@@ -1,531 +0,0 @@
package notify_test
import (
"bytes"
"context"
"log/slog"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"sync"
"sync/atomic"
"testing"
"time"
"go.uber.org/fx"
"sneak.berlin/go/dnswatcher/internal/config"
"sneak.berlin/go/dnswatcher/internal/globals"
"sneak.berlin/go/dnswatcher/internal/logger"
"sneak.berlin/go/dnswatcher/internal/notify"
)
// Timings used by the drain tests. They stay in the same
// 10-100ms band as the retry tests so the suite never waits on
// a real backoff delay.
const (
// inFlightHold is how long a delivery is kept mid-request
// before the handler is released.
inFlightHold = 30 * time.Millisecond
// drainDeadline bounds a drain that is expected to time
// out.
drainDeadline = 50 * time.Millisecond
// drainSlack is the upper bound on how long a bounded
// drain may take; generous enough for a loaded CI box,
// still far below the 20s test ceiling.
drainSlack = 2 * time.Second
// settleDelay is how long to wait before asserting that
// something did *not* happen.
settleDelay = 50 * time.Millisecond
// idleDrainBound is the upper bound on a drain that has
// nothing in flight. It is deliberately far above the cost
// of the goroutine hop through inFlight.Wait() — which
// reached 57ms on a loaded box under -race with the package's
// parallel tests — and far below drainSlack, the deadline
// such a drain is given. A drain that blocked until its
// deadline instead of returning on the WaitGroup therefore
// still fails this bound, but scheduling delay alone cannot.
idleDrainBound = 500 * time.Millisecond
)
// syncBuffer is an io.Writer safe for concurrent use, so log
// output written from delivery goroutines can be inspected.
type syncBuffer struct {
mu sync.Mutex
buf bytes.Buffer
}
func (sb *syncBuffer) Write(p []byte) (int, error) {
sb.mu.Lock()
defer sb.mu.Unlock()
return sb.buf.Write(p) //nolint:wrapcheck // test helper
}
func (sb *syncBuffer) String() string {
sb.mu.Lock()
defer sb.mu.Unlock()
return sb.buf.String()
}
// newLoggingService returns a Service writing JSON logs into
// the returned buffer.
func newLoggingService(
transport http.RoundTripper,
) (*notify.Service, *syncBuffer) {
logs := &syncBuffer{}
handler := slog.NewJSONHandler(logs, nil)
return notify.NewTestServiceWithLogger(transport, handler),
logs
}
// blockingNtfyServer returns a server whose handler signals on
// entered, waits for release, and then responds 200.
func blockingNtfyServer(
entered chan<- struct{},
release <-chan struct{},
served *atomic.Bool,
) *httptest.Server {
var once sync.Once
return httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
once.Do(func() { close(entered) })
<-release
served.Store(true)
w.WriteHeader(http.StatusOK)
}),
)
}
// TestDrainWaitsForInFlightDelivery verifies that a delivery
// already under way when shutdown starts is allowed to finish.
func TestDrainWaitsForInFlightDelivery(t *testing.T) {
t.Parallel()
var served atomic.Bool
entered := make(chan struct{})
release := make(chan struct{})
srv := blockingNtfyServer(entered, release, &served)
defer srv.Close()
topicURL, _ := url.Parse(srv.URL)
svc := notify.NewTestService(http.DefaultTransport)
svc.SetNtfyURL(topicURL)
svc.SendNotification(
context.Background(), "t", "m", prioInfo,
)
// Make sure the delivery really is mid-request before the
// drain begins.
select {
case <-entered:
case <-time.After(drainSlack):
t.Fatal("delivery never reached the endpoint")
}
// As in TestDrainBoundedByContextDeadline: start is captured
// before the clock it is compared against, here the timer
// holding the delivery open, so elapsed covers the whole hold
// and the lower bound cannot come out short from scheduling
// delay alone.
start := time.Now()
timer := time.AfterFunc(inFlightHold, func() {
close(release)
})
defer timer.Stop()
ctx, cancel := context.WithTimeout(
context.Background(), drainSlack,
)
defer cancel()
svc.Drain(ctx)
elapsed := time.Since(start)
if !served.Load() {
t.Error(
"drain returned before the in-flight delivery " +
"completed",
)
}
if elapsed < inFlightHold {
t.Errorf(
"drain took %v, want at least %v",
elapsed, inFlightHold,
)
}
if got := svc.OutstandingDeliveries(); got != 0 {
t.Errorf("outstanding deliveries = %d, want 0", got)
}
}
// neverFires returns a channel that never delivers, standing in
// for a long backoff sleep without actually sleeping.
func neverFires(_ time.Duration) <-chan time.Time {
return make(chan time.Time)
}
// TestDrainBoundedByContextDeadline verifies that a delivery
// stuck retrying against a dead endpoint does not hold shutdown
// past the OnStop context deadline, and that the abandoned
// deliveries are logged at warn level rather than dropped
// silently.
func TestDrainBoundedByContextDeadline(t *testing.T) {
t.Parallel()
var requests atomic.Int64
srv := httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
requests.Add(1)
w.WriteHeader(http.StatusInternalServerError)
}),
)
defer srv.Close()
topicURL, _ := url.Parse(srv.URL)
svc, logs := newLoggingService(http.DefaultTransport)
svc.SetNtfyURL(topicURL)
// Never let the backoff sleep complete: the delivery is
// parked in its retry wait until shutdown releases it.
svc.SetSleepFunc(neverFires)
svc.SetRetryConfig(notify.RetryConfig{
MaxRetries: 5,
BaseDelay: time.Hour,
MaxDelay: time.Hour,
})
svc.SendNotification(
context.Background(), "t", "m", prioError,
)
waitForCondition(t, func() bool {
return requests.Load() >= 1 &&
svc.OutstandingDeliveries() == 1
})
// start must be captured *before* the deadline clock starts,
// so that the measured interval is a superset of the deadline
// interval. Capturing it after context.WithTimeout would
// make elapsed structurally smaller than drainDeadline and
// the lower bound below unfalsifiable-by-luck: it would fail
// whenever the two statements were separated by any
// scheduling delay, and pass otherwise, regardless of what
// the drain did.
start := time.Now()
ctx, cancel := context.WithTimeout(
context.Background(), drainDeadline,
)
defer cancel()
// The upper bound is enforced by a watchdog rather than by
// measuring after the fact: a drain that is not bounded at
// all never returns here (the delivery is parked in a backoff
// that never fires), so an unbounded drain must fail this
// test promptly instead of hanging the package until the test
// binary's 30s timeout.
returned := make(chan struct{})
go func() {
defer close(returned)
svc.Drain(ctx)
}()
select {
case <-returned:
case <-time.After(drainSlack):
t.Fatalf(
"drain did not return within %v; its %v deadline "+
"did not bound it",
drainSlack, drainDeadline,
)
}
// The lower bound is the real assertion: the drain must have
// waited for its whole deadline rather than giving up on the
// outstanding delivery early. With start captured above, an
// early return is the only thing that can make it fail.
if elapsed := time.Since(start); elapsed < drainDeadline {
t.Errorf(
"drain returned after %v, before its %v deadline",
elapsed, drainDeadline,
)
}
assertAbandonLogged(t, logs.String())
// The abandoned delivery must stop retrying rather than
// outlive the drain.
waitForCondition(t, func() bool {
return svc.OutstandingDeliveries() == 0
})
}
// assertAbandonLogged checks that the drain logged the
// abandoned deliveries at warn level with a count.
func assertAbandonLogged(t *testing.T, output string) {
t.Helper()
if !strings.Contains(output, `"level":"WARN"`) {
t.Errorf(
"abandoned deliveries not logged at warn level; "+
"log output: %s",
output,
)
}
if !strings.Contains(output, `"abandoned":1`) {
t.Errorf(
"abandoned delivery count not logged; "+
"log output: %s",
output,
)
}
}
// TestDrainRefusesNewDeliveries verifies that notifications
// submitted after the drain has begun are refused and logged,
// so a stream of new work cannot extend shutdown indefinitely.
func TestDrainRefusesNewDeliveries(t *testing.T) {
t.Parallel()
var requests atomic.Int64
srv := httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
requests.Add(1)
w.WriteHeader(http.StatusOK)
}),
)
defer srv.Close()
target, _ := url.Parse(srv.URL)
svc, logs := newLoggingService(http.DefaultTransport)
svc.SetNtfyURL(target)
svc.SetSlackWebhookURL(target)
svc.SetMattermostWebhookURL(target)
ctx, cancel := context.WithTimeout(
context.Background(), drainSlack,
)
defer cancel()
// Nothing is in flight, so this returns immediately and
// leaves the service refusing further deliveries.
svc.Drain(ctx)
for range 3 {
svc.SendNotification(
context.Background(), "t", "m", prioInfo,
)
}
time.Sleep(settleDelay)
if got := requests.Load(); got != 0 {
t.Errorf(
"%d requests reached the endpoint after drain, "+
"want 0",
got,
)
}
if got := svc.OutstandingDeliveries(); got != 0 {
t.Errorf("outstanding deliveries = %d, want 0", got)
}
output := logs.String()
if !strings.Contains(output, "shutdown in progress") {
t.Errorf(
"refused deliveries not logged; log output: %s",
output,
)
}
}
// recordingLifecycle is a minimal fx.Lifecycle that records the
// hooks appended to it, so the wiring done by notify.New can be
// inspected without standing up a whole fx application.
type recordingLifecycle struct {
hooks []fx.Hook
}
func (l *recordingLifecycle) Append(hook fx.Hook) {
l.hooks = append(l.hooks, hook)
}
// newNotifyService builds a Service through the real
// constructor, wired to the given lifecycle.
func newNotifyService(
t *testing.T,
lifecycle fx.Lifecycle,
ntfyTopic string,
) *notify.Service {
t.Helper()
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)
}
svc, err := notify.New(lifecycle, notify.Params{
Logger: log,
Config: &config.Config{NtfyTopic: ntfyTopic},
})
if err != nil {
t.Fatalf("notify.New: %v", err)
}
return svc
}
// TestNewRegistersDrainingStopHook verifies that notify.New
// wires an OnStop hook into the fx lifecycle and that the hook
// waits for in-flight deliveries.
func TestNewRegistersDrainingStopHook(t *testing.T) {
t.Parallel()
var served atomic.Bool
entered := make(chan struct{})
release := make(chan struct{})
srv := blockingNtfyServer(entered, release, &served)
defer srv.Close()
lifecycle := &recordingLifecycle{}
svc := newNotifyService(t, lifecycle, srv.URL)
if len(lifecycle.hooks) != 1 {
t.Fatalf(
"appended %d lifecycle hooks, want 1",
len(lifecycle.hooks),
)
}
stop := lifecycle.hooks[0].OnStop
if stop == nil {
t.Fatal("lifecycle hook has no OnStop function")
}
svc.SendNotification(
context.Background(), "t", "m", prioInfo,
)
select {
case <-entered:
case <-time.After(drainSlack):
t.Fatal("delivery never reached the endpoint")
}
timer := time.AfterFunc(inFlightHold, func() {
close(release)
})
defer timer.Stop()
ctx, cancel := context.WithTimeout(
context.Background(), drainSlack,
)
defer cancel()
err := stop(ctx)
if err != nil {
t.Fatalf("OnStop returned error: %v", err)
}
if !served.Load() {
t.Error(
"OnStop returned before the in-flight delivery " +
"completed",
)
}
}
// TestDrainWithoutDeliveriesReturnsImmediately verifies the
// common case: nothing in flight, shutdown is not delayed.
func TestDrainWithoutDeliveriesReturnsImmediately(t *testing.T) {
t.Parallel()
svc := notify.NewTestService(http.DefaultTransport)
// Captured before the deadline clock, as elsewhere in this
// file; for an upper bound that is the conservative
// direction, since the measured interval can then only be
// longer than the drain itself.
start := time.Now()
ctx, cancel := context.WithTimeout(
context.Background(), drainSlack,
)
defer cancel()
svc.Drain(ctx)
if elapsed := time.Since(start); elapsed > idleDrainBound {
t.Errorf(
"drain of an idle service took %v, want well "+
"under its %v deadline",
elapsed, drainSlack,
)
}
}
// TestDrainWithCancelledContextDoesNotWarn verifies that an
// OnStop context that is already dead on entry does not produce
// an "abandoning them" warning when there was nothing in flight
// to abandon. The expired context wins the select immediately,
// so only the outstanding count can tell the difference between
// a genuine timeout and a shutdown that had simply already run
// out of time with no work left.
func TestDrainWithCancelledContextDoesNotWarn(t *testing.T) {
t.Parallel()
svc, logs := newLoggingService(http.DefaultTransport)
ctx, cancel := context.WithCancel(context.Background())
cancel()
svc.Drain(ctx)
if output := logs.String(); strings.Contains(
output, `"level":"WARN"`,
) {
t.Errorf(
"drain with nothing in flight warned about "+
"abandoned deliveries; log output: %s",
output,
)
}
}
+2 -2
View File
@@ -7,8 +7,8 @@ import (
"github.com/miekg/dns"
)
// DNSClient abstracts DNS wire-protocol exchanges so the resolver
// can be tested without hitting real nameservers.
// DNSClient abstracts DNS wire-protocol exchanges over a single
// transport, letting the resolver switch between UDP and TCP.
type DNSClient interface {
ExchangeContext(
ctx context.Context,
+18 -1
View File
@@ -20,6 +20,10 @@ const (
minDomainLabels = 2
)
// defaultDNSPort is the port a nameserver is assumed to listen on
// when its address does not carry one.
const defaultDNSPort = "53"
// ErrRefused is returned when a DNS server refuses a query.
var ErrRefused = errors.New("dns query refused")
@@ -106,6 +110,19 @@ func (r *Resolver) retryTCP(
return resp
}
// nameserverAddr renders a nameserver address for dialling. A bare
// address — the normal case, and what a delegation's glue records
// carry — is given the default DNS port. An address that already
// specifies a port is dialled as written, which is what makes a
// nameserver listening somewhere other than 53 reachable.
func nameserverAddr(nsIP string) string {
if _, _, err := net.SplitHostPort(nsIP); err == nil {
return nsIP
}
return net.JoinHostPort(nsIP, defaultDNSPort)
}
// queryDNS sends a DNS query to a specific server IP.
// Tries non-recursive first, falls back to recursive on
// REFUSED (handles DNS interception environments).
@@ -120,7 +137,7 @@ func (r *Resolver) queryDNS(
}
name = dns.Fqdn(name)
addr := net.JoinHostPort(serverIP, "53")
addr := nameserverAddr(serverIP)
msg := new(dns.Msg)
msg.SetQuestion(name, qtype)
-13
View File
@@ -67,17 +67,4 @@ func NewFromLogger(log *slog.Logger) *Resolver {
}
}
// NewFromLoggerWithClient creates a Resolver with a custom DNS
// client, useful for testing with mock DNS responses.
func NewFromLoggerWithClient(
log *slog.Logger,
client DNSClient,
) *Resolver {
return &Resolver{
log: log,
client: client,
tcp: client,
}
}
// Method implementations are in iterative.go.
+3 -54
View File
@@ -8,9 +8,7 @@ import (
"sort"
"strings"
"testing"
"time"
"github.com/miekg/dns"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -519,58 +517,9 @@ func TestQueryAllNameservers_ContextCanceled(t *testing.T) {
assert.Error(t, err)
}
// ----------------------------------------------------------------
// Timeout tests
// ----------------------------------------------------------------
func TestQueryNameserverIP_Timeout(t *testing.T) {
t.Parallel()
log := slog.New(slog.NewTextHandler(
os.Stderr,
&slog.HandlerOptions{Level: slog.LevelDebug},
))
r := resolver.NewFromLoggerWithClient(
log, &timeoutClient{},
)
ctx, cancel := context.WithTimeout(
context.Background(), 10*time.Second,
)
t.Cleanup(cancel)
// Query any IP — the client always returns a timeout error.
resp, err := r.QueryNameserverIP(
ctx, "unreachable.test.", "192.0.2.1",
"example.com",
)
require.NoError(t, err)
assert.Equal(t, resolver.StatusTimeout, resp.Status)
assert.NotEmpty(t, resp.Error)
}
// timeoutClient simulates DNS timeout errors for testing.
type timeoutClient struct{}
func (c *timeoutClient) ExchangeContext(
_ context.Context,
_ *dns.Msg,
_ string,
) (*dns.Msg, time.Duration, error) {
return nil, 0, &net.OpError{
Op: "read",
Net: "udp",
Err: &timeoutError{},
}
}
type timeoutError struct{}
func (e *timeoutError) Error() string { return "i/o timeout" }
func (e *timeoutError) Timeout() bool { return true }
func (e *timeoutError) Temporary() bool { return true }
// Transport-failure classification (StatusTimeout / StatusError)
// is covered in transport_test.go, against real nameservers bound on
// loopback.
func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
t.Parallel()
+253
View File
@@ -0,0 +1,253 @@
package resolver_test
// Transport-failure classification tests.
//
// These are live tests, not mocks. Nothing here substitutes the
// resolver's DNSClient: the resolver dials a real UDP socket, writes
// a real DNS query with the real miekg/dns client, and applies its
// real deadline and its real classification logic to what comes
// back. The only thing under test control is which address the query
// is sent to, and what — if anything — is listening there.
//
// That distinction is what the no-DNS-mocks rule in TESTING.md is
// about. A fake DNSClient lets the code under test skip DNS entirely
// and hands it a manufactured verdict; a nameserver bound on
// loopback makes it speak DNS for real and earn one. Pointing a live
// query at a nameserver of the test's choosing is no more a mock
// than pointing it at a.root-servers.net.
//
// The public network cannot produce these outcomes on demand. A
// black-holed address is not black-holed everywhere — build
// environments that intercept UDP/53 answer it locally — so a test
// built on one asserts on the network it happens to run on rather
// than on the resolver. A loopback nameserver is deterministic
// everywhere, and it is fast, because the test picks the deadline.
import (
"context"
"net"
"testing"
"time"
"github.com/miekg/dns"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/dnswatcher/internal/resolver"
)
const (
// transportBudget is the wall time the timeout test must stay
// under. The resolver asks a nameserver for eight record types
// in turn and retries each one once, so a nameserver silent on
// every type would cost sixteen query timeouts. The test's
// nameserver is silent on exactly one type, which costs two,
// and this budget fails loudly if that ever stops being true.
transportBudget = 8 * time.Second
// transportDeadline is the caller deadline the tests run
// under. It is generous on purpose: these tests are about the
// resolver classifying a nameserver's behaviour, so the
// caller's deadline must never be the thing that expires.
transportDeadline = 30 * time.Second
// silentNS and failingNS are the nameserver names reported
// back in NameserverResponse.Nameserver. They are .test names
// (RFC 6761) and are never resolved: the tests address the
// nameserver by its socket address.
silentNS = "silent.ns.test."
failingNS = "servfail.ns.test."
// transportHostname is the name queried. Nothing resolves it;
// the point is entirely how the nameserver behaves.
transportHostname = "example.com"
)
// startNameserver binds a real UDP nameserver on loopback and serves
// every datagram it receives with handle, which returns the reply to
// send or nil to stay silent. It returns the "host:port" address to
// aim a query at, and stops the server when the test ends.
func startNameserver(
t *testing.T,
handle func(query *dns.Msg) *dns.Msg,
) string {
t.Helper()
conn, err := net.ListenPacket("udp", "127.0.0.1:0")
require.NoError(t, err, "binding loopback nameserver")
stopped := make(chan struct{})
t.Cleanup(func() {
_ = conn.Close()
<-stopped
})
go serveNameserver(conn, handle, stopped)
return conn.LocalAddr().String()
}
// serveNameserver reads queries until conn is closed, replying with
// whatever handle produces.
func serveNameserver(
conn net.PacketConn,
handle func(query *dns.Msg) *dns.Msg,
stopped chan<- struct{},
) {
defer close(stopped)
buf := make([]byte, dns.MaxMsgSize)
for {
n, from, err := conn.ReadFrom(buf)
if err != nil {
return
}
query := new(dns.Msg)
if query.Unpack(buf[:n]) != nil {
continue
}
reply := handle(query)
if reply == nil {
continue
}
wire, err := reply.Pack()
if err != nil {
continue
}
if _, err := conn.WriteTo(wire, from); err != nil {
return
}
}
}
// unservedAddr returns a loopback address with nothing listening on
// it, by binding a port and releasing it again.
func unservedAddr(t *testing.T) string {
t.Helper()
conn, err := net.ListenPacket("udp", "127.0.0.1:0")
require.NoError(t, err, "binding loopback port")
addr := conn.LocalAddr().String()
require.NoError(t, conn.Close(), "releasing loopback port")
return addr
}
// TestQueryNameserverIP_Timeout covers the StatusTimeout branch: a
// nameserver that takes the query and never answers.
func TestQueryNameserverIP_Timeout(t *testing.T) {
t.Parallel()
// A real nameserver that drops A queries and answers every
// other type. Silence on one type is all the resolver needs to
// classify the response as a timeout, and it keeps the test
// two query timeouts long instead of sixteen.
addr := startNameserver(t, func(query *dns.Msg) *dns.Msg {
if len(query.Question) > 0 &&
query.Question[0].Qtype == dns.TypeA {
return nil
}
reply := new(dns.Msg)
reply.SetReply(query)
return reply
})
r := newTestResolver(t)
ctx, cancel := context.WithTimeout(
context.Background(), transportDeadline,
)
defer cancel()
start := time.Now()
resp, err := r.QueryNameserverIP(
ctx, silentNS, addr, transportHostname,
)
elapsed := time.Since(start)
require.NoError(t, err)
require.NotNil(t, resp)
assert.Equal(t, resolver.StatusTimeout, resp.Status)
assert.Equal(t, "all queries timed out", resp.Error)
assert.Empty(t, resp.Records)
assert.Equal(t, silentNS, resp.Nameserver)
assert.Less(
t, elapsed, transportBudget,
"one silent record type must cost one query's retries, "+
"not every record type's",
)
}
// TestQueryNameserverIP_ServFail covers the StatusError branch: a
// nameserver that answers, and answers SERVFAIL.
func TestQueryNameserverIP_ServFail(t *testing.T) {
t.Parallel()
addr := startNameserver(t, func(query *dns.Msg) *dns.Msg {
reply := new(dns.Msg)
reply.SetRcode(query, dns.RcodeServerFailure)
return reply
})
r := newTestResolver(t)
ctx, cancel := context.WithTimeout(
context.Background(), transportDeadline,
)
defer cancel()
resp, err := r.QueryNameserverIP(
ctx, failingNS, addr, transportHostname,
)
require.NoError(t, err)
require.NotNil(t, resp)
assert.Equal(t, resolver.StatusError, resp.Status)
assert.Equal(t, "server returned SERVFAIL", resp.Error)
assert.Empty(t, resp.Records)
assert.Equal(t, failingNS, resp.Nameserver)
}
// TestQueryNameserverIP_NoListener pins the third transport outcome:
// a refused datagram is not a timeout. The socket fails immediately
// with ECONNREFUSED rather than going quiet, so isTimeout is false,
// no failure flag is set, and the response classifies as NoData.
// Asserting it here is what stops that path being mistaken for the
// timeout path, in either direction.
func TestQueryNameserverIP_NoListener(t *testing.T) {
t.Parallel()
addr := unservedAddr(t)
r := newTestResolver(t)
ctx, cancel := context.WithTimeout(
context.Background(), transportDeadline,
)
defer cancel()
resp, err := r.QueryNameserverIP(
ctx, silentNS, addr, transportHostname,
)
require.NoError(t, err)
require.NotNil(t, resp)
assert.Equal(t, resolver.StatusNoData, resp.Status)
assert.Empty(t, resp.Records)
}
File diff suppressed because it is too large Load Diff
+13 -2
View File
@@ -19,15 +19,26 @@
#
# -timeout 90s is a deliberate backstop above the 60s hard cap on
# suite duration. Do not lower it.
#
# -p 1 runs one test package at a time, and is load-bearing. Live DNS
# is a resource outside the process: the concurrency gates that keep
# this suite from bursting at the root servers
# (internal/resolver/livedns_test.go, internal/watcher/watcher_test.go)
# are package-scoped, so each one only bounds its own test binary. Go
# runs package binaries in parallel by default, so with both live-DNS
# packages in flight at once their gates sum instead of holding, the
# root and TLD servers rate-limit the excess, and the resolver
# package's per-attempt deadlines expire. Serialising packages is what
# makes each gate authoritative while its package runs.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
go test -count=1 -race -timeout 90s -cover ./... || {
go test -count=1 -p 1 -race -timeout 90s -cover ./... || {
echo "--- Rerunning with -v for details ---" >&2
go test -count=1 -race -timeout 90s -v ./... || true
go test -count=1 -p 1 -race -timeout 90s -v ./... || true
exit 1
}
}