diff --git a/TESTING.md b/TESTING.md index 659db13..70e3899 100644 --- a/TESTING.md +++ b/TESTING.md @@ -2,8 +2,12 @@ ## DNS Resolution Tests -All resolver tests **MUST** use live queries against real DNS servers. -No mocking of the DNS client layer is permitted. +DNS is never mocked in this project, not in tests and not anywhere +else; see the README section "No DNS mocking. Ever." Every test that +looks something up in DNS **MUST** query live DNS servers, never a +stand-in. Logic that works on record data, such as comparing or +formatting records, may be tested on that data directly with no +lookup. ### Rationale @@ -21,14 +25,18 @@ real servers ensures the resolver works correctly in production. - Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) plus processing margin - Root server fan-out is limited to reduce parallel query load +- Live lookups that expect an answer go through `internal/livedns`, + which limits how many run at once in a test binary and retries a + lookup that got none - Flaky failures from transient network issues are acceptable and should be investigated as potential resolver bugs, not papered over with mocks or skip flags ### 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, fake or stub DNS** anywhere: no stand-in `DNSClient`, + no stand-in for the watcher's `DNSResolver`, no fake DNS server, no + canned responses - **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 diff --git a/TODO.md b/TODO.md index 5ce0129..c03615b 100644 --- a/TODO.md +++ b/TODO.md @@ -10,11 +10,7 @@ # Status -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. +pre-1.0. No git tags. # Next Step @@ -23,6 +19,13 @@ Rationale, Design, TODO, License, Author) if any are still missing. # Completed Steps +- 2026-09-29: nothing stands in for DNS any more. Watcher tests that look + something up in DNS use the real resolver against live DNS servers and test + record and nameserver changes by preparing the saved state a check starts + from; the resolver timeout test queries an address that never answers, and + `NewFromLoggerWithClient`, used only by its stand-in client, is gone. The + live-DNS retry and concurrency limit moved to `internal/livedns`, which both + test packages use. `TESTING.md` states the README's rule (closes #159). - 2026-09-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 @@ -261,6 +264,5 @@ 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 +- DNS is never mocked; tests that look something up in DNS query live DNS + servers (README, "No DNS mocking. Ever.") diff --git a/internal/livedns/livedns.go b/internal/livedns/livedns.go new file mode 100644 index 0000000..b86794b --- /dev/null +++ b/internal/livedns/livedns.go @@ -0,0 +1,121 @@ +// Package livedns runs the live DNS operations of tests. Tests that +// look something up in DNS query live DNS servers, never a stand-in — +// see TESTING.md. Nothing here mocks, fakes, stubs, records or replays +// DNS, and nothing here skips a test: it only changes *how* the live +// queries are issued, so that a single dropped UDP packet or one slow +// authoritative server does not turn correct code into a red build. +// +// Two mechanisms: +// +// 1. Bounded concurrency. Tests run in parallel and the build hosts +// have many cores, so without a limit every test starts its own +// iterative resolution at the same instant and they all hit the +// first root server within a few milliseconds of each other. Root +// servers rate-limit that, which shows up as a different arbitrary +// subset of tests failing on each run. Run caps how many live +// operations are in flight at once in one test binary. +// +// 2. Retry with exponential backoff. Each live operation gets several +// attempts with its own timeout. An attempt is retried when it +// obtained nothing to check, never because of what the test +// asserts about the result, so a wrong result still fails on the +// first attempt. A fault in the code under test that leaves +// nothing to check looks the same as live DNS not answering, and +// fails only after the last attempt. +package livedns + +import ( + "context" + "errors" + "testing" + "time" +) + +const ( + // attempts is how many times a live DNS operation is attempted + // before the test fails. + attempts = 3 + + // AttemptTimeout bounds one attempt. Worst case for an operation + // is attempts * AttemptTimeout plus the backoff — about 26 + // seconds, well inside the 90-second `go test -timeout` backstop + // even when several operations exhaust their attempts. + AttemptTimeout = 8 * time.Second + + // backoffBase is the delay after the first failed attempt; it is + // multiplied by backoffFactor each time. + backoffBase = 500 * time.Millisecond + + // backoffFactor is the exponential backoff multiplier. + backoffFactor = 2 + + // Concurrency caps how many live operations may be in flight + // across one test binary at once. + Concurrency = 6 +) + +// gate bounds concurrent live operations. It has to be package scoped: +// the whole point is that it is shared by every parallel test in the +// test binary. +// +//nolint:gochecknoglobals // package-wide live query rate limit +var gate = make(chan struct{}, Concurrency) + +// ErrNoAnswer reports that a live operation produced no usable answer, +// which is retried rather than asserted on. +var ErrNoAnswer = errors.New("no answer from live DNS") + +// Run executes one attempt of a live operation, holding a slot in gate +// for its duration and bounding it with its own timeout. +func Run(op func(ctx context.Context) error) error { + gate <- struct{}{} + defer func() { <-gate }() + + ctx, cancel := context.WithTimeout( + context.Background(), AttemptTimeout, + ) + defer cancel() + + return op(ctx) +} + +// Retry runs op until it reports success, retrying failures with +// exponential backoff, and fails the test if every attempt fails. op +// returns an error only for a failure to obtain an answer — never for +// an answer the test disagrees with, which belongs in an assertion so +// that it fails immediately. op stores whatever it obtained where its +// caller can find it. +func Retry( + t *testing.T, + what string, + op func(ctx context.Context) error, +) { + t.Helper() + + var last error + + backoff := backoffBase + + for attempt := range attempts { + if attempt > 0 { + t.Logf( + "%s: attempt %d of %d failed (%v), "+ + "retrying in %s", + what, attempt, attempts, last, backoff, + ) + time.Sleep(backoff) + + backoff *= backoffFactor + } + + last = Run(op) + if last == nil { + return + } + } + + t.Fatalf( + "%s: all %d live attempts failed: %v", + what, attempts, last, + ) +} diff --git a/internal/livedns/livedns_test.go b/internal/livedns/livedns_test.go new file mode 100644 index 0000000..1c0f5fd --- /dev/null +++ b/internal/livedns/livedns_test.go @@ -0,0 +1,103 @@ +package livedns_test + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "sneak.berlin/go/dnswatcher/internal/livedns" +) + +// Tests for the retry and the concurrency limit themselves. They +// perform no DNS resolution of any kind. + +func TestRetryRecoversFromTransientFailure(t *testing.T) { + t.Parallel() + + const wantAttempts = 2 + + attempts := 0 + + livedns.Retry(t, "transient", func(_ context.Context) error { + attempts++ + + if attempts < wantAttempts { + return livedns.ErrNoAnswer + } + + return nil + }) + + assert.Equal(t, wantAttempts, attempts) +} + +func TestRetryGivesEachAttemptADeadline(t *testing.T) { + t.Parallel() + + livedns.Retry(t, "deadline", func(ctx context.Context) error { + deadline, ok := ctx.Deadline() + assert.True(t, ok, "attempt should carry a deadline") + + remaining := time.Until(deadline) + + assert.LessOrEqual(t, remaining, livedns.AttemptTimeout) + + // Lower bound too: without one this passes for a + // deadline far shorter than intended, which would + // silently turn every live attempt into an instant + // timeout. + assert.Greater(t, remaining, livedns.AttemptTimeout/2) + + return nil + }) +} + +func TestRunBoundsConcurrency(t *testing.T) { + t.Parallel() + + const workers = 24 + + var ( + mu sync.Mutex + wg sync.WaitGroup + inFlight int + maxSeen int + ) + + wg.Add(workers) + + for range workers { + go func() { + defer wg.Done() + + _ = livedns.Run(func(_ context.Context) error { + mu.Lock() + inFlight++ + + if inFlight > maxSeen { + maxSeen = inFlight + } + mu.Unlock() + + time.Sleep(time.Millisecond) + + mu.Lock() + inFlight-- + mu.Unlock() + + return nil + }) + }() + } + + wg.Wait() + + assert.Positive(t, maxSeen) + assert.LessOrEqual( + t, maxSeen, livedns.Concurrency, + "live queries must stay under the package-wide gate", + ) +} diff --git a/internal/resolver/dns_client.go b/internal/resolver/dns_client.go index 589c657..13135e5 100644 --- a/internal/resolver/dns_client.go +++ b/internal/resolver/dns_client.go @@ -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 sends one DNS message to a nameserver and returns the +// reply. The resolver holds one for UDP and one for TCP. type DNSClient interface { ExchangeContext( ctx context.Context, diff --git a/internal/resolver/livedns_harness_test.go b/internal/resolver/livedns_harness_test.go index 87ce704..2efdb39 100644 --- a/internal/resolver/livedns_harness_test.go +++ b/internal/resolver/livedns_harness_test.go @@ -1,10 +1,7 @@ package resolver_test import ( - "context" - "sync" "testing" - "time" "github.com/stretchr/testify/assert" @@ -12,9 +9,8 @@ import ( ) // Tests for the live-DNS harness in livedns_test.go itself. These -// exercise pure logic and the retry/concurrency plumbing; they -// perform no DNS resolution of any kind, so they neither mock DNS -// nor depend on it. +// exercise pure logic; they perform no DNS resolution of any kind, so +// they neither mock DNS nor depend on it. // Names for the synthetic status maps below. Nothing is ever queried // at them: they are map keys handed to the package's pure counting @@ -90,47 +86,6 @@ func TestStatusCountingIgnoresSilentNameservers(t *testing.T) { ) } -func TestRetryLiveRecoversFromTransientFailure(t *testing.T) { - t.Parallel() - - const wantAttempts = 2 - - attempts := 0 - - retryLive(t, "transient", func(_ context.Context) error { - attempts++ - - if attempts < wantAttempts { - return errLiveNoAnswer - } - - return nil - }) - - assert.Equal(t, wantAttempts, attempts) -} - -func TestRetryLiveGivesEachAttemptADeadline(t *testing.T) { - t.Parallel() - - retryLive(t, "deadline", func(ctx context.Context) error { - deadline, ok := ctx.Deadline() - assert.True(t, ok, "attempt should carry a deadline") - - remaining := time.Until(deadline) - - assert.LessOrEqual(t, remaining, liveAttemptTimeout) - - // Lower bound too: without one this passes for a - // deadline far shorter than intended, which would - // silently turn every live attempt into an instant - // timeout. - assert.Greater(t, remaining, liveAttemptTimeout/2) - - return nil - }) -} - // TestUnsanctionedStatusesRejectsWrongAnswers is the regression test // for the defect this allowlist exists to prevent: a minority of // nameservers answering WRONGLY while quorum keeps the suite green. @@ -235,50 +190,3 @@ func TestUnsanctionedStatusesToleratesSilenceOnly(t *testing.T) { unsanctionedStatuses(results, allowed...), ) } - -func TestRunLiveBoundsConcurrency(t *testing.T) { - t.Parallel() - - const workers = 24 - - var ( - mu sync.Mutex - wg sync.WaitGroup - inFlight int - maxSeen int - ) - - wg.Add(workers) - - for range workers { - go func() { - defer wg.Done() - - _ = runLive(func(_ context.Context) error { - mu.Lock() - inFlight++ - - if inFlight > maxSeen { - maxSeen = inFlight - } - mu.Unlock() - - time.Sleep(time.Millisecond) - - mu.Lock() - inFlight-- - mu.Unlock() - - return nil - }) - }() - } - - wg.Wait() - - assert.Positive(t, maxSeen) - assert.LessOrEqual( - t, maxSeen, liveConcurrency, - "live queries must stay under the package-wide gate", - ) -} diff --git a/internal/resolver/livedns_test.go b/internal/resolver/livedns_test.go index b864460..b7d1be2 100644 --- a/internal/resolver/livedns_test.go +++ b/internal/resolver/livedns_test.go @@ -8,8 +8,8 @@ import ( "sort" "strings" "testing" - "time" + "sneak.berlin/go/dnswatcher/internal/livedns" "sneak.berlin/go/dnswatcher/internal/resolver" ) @@ -17,144 +17,34 @@ import ( // Live DNS test support // ---------------------------------------------------------------- // -// Every test in this package resolves against the real, live DNS — -// see TESTING.md. Nothing here mocks, fakes, stubs, records or -// replays DNS, and nothing here skips or gates a test: the helpers -// below only change *how* the live queries are issued, so that a -// single dropped UDP packet or one slow authoritative server does -// not turn a correct resolver into a red build. +// Tests that look something up in DNS query live DNS servers, never a +// stand-in; logic that works on record data may be tested on that +// data with no lookup (see TESTING.md). Each live operation below goes +// through livedns.Retry, which bounds how many resolutions are in +// flight at once and retries an operation that got no answer (see +// package livedns). // -// Three mechanisms, all test-side: +// Where an assertion spans several independent nameservers, a quorum +// is enough: a strict majority answering as expected. A server that +// fails to answer is tolerated, while a server that answers *wrongly* +// still fails the test. // -// 1. Bounded concurrency. The package's tests are parallel and the -// build hosts have many cores, so without a limit every test -// starts its own iterative resolution at the same instant and -// they all hit the first root server in rootServerList() within -// a few milliseconds of each other. Root servers rate-limit -// that, which shows up as a different arbitrary subset of tests -// failing on each run. liveGate caps how many resolutions are -// in flight at once. -// -// 2. Retry with exponential backoff. Each live operation gets -// several attempts with its own timeout. The retry predicate is -// strictly transport-level — "did a nameserver answer at all" — -// never the assertion the test is making. A resolver that -// answers incorrectly still fails on the first attempt. -// -// 3. Quorum. Where an assertion spans several independent -// nameservers, a strict majority answering as expected is -// enough; a server that fails to answer is tolerated, while a -// server that answers *wrongly* still fails the test. -// -// The tolerance in (3) is expressed as an ALLOWLIST of sanctioned -// statuses, never as a blocklist of known-bad ones. A blocklist bans -// the one wrong answer its author thought of and silently admits -// every other status, including any added to the resolver later; an -// allowlist fails on anything nobody explicitly sanctioned. Silence -// (timeout, error) is the only thing quorum exists to tolerate. A -// *wrong answer* — nxdomain for a name that exists, ok for one that -// does not, nodata for either — is never tolerated at any count. +// That tolerance is expressed as an ALLOWLIST of sanctioned statuses, +// never as a blocklist of known-bad ones. A blocklist bans the one +// wrong answer its author thought of and silently admits every other +// status, including any added to the resolver later; an allowlist +// fails on anything nobody explicitly sanctioned. Silence (timeout, +// error) is the only thing quorum exists to tolerate. A *wrong +// answer* — nxdomain for a name that exists, ok for one that does +// not, nodata for either — is never tolerated at any count. -const ( - // liveAttempts is how many times a live DNS operation is - // attempted before the test fails. - liveAttempts = 3 +// minNameservers is the smallest nameserver count a well-run zone is +// expected to publish. +const minNameservers = 2 - // liveAttemptTimeout bounds one attempt. Worst case for an - // operation is liveAttempts * liveAttemptTimeout plus the - // backoff — about 26 seconds, well inside the 90-second - // `go test -timeout` backstop even when several operations - // exhaust their attempts. - liveAttemptTimeout = 8 * time.Second - - // liveBackoffBase is the delay after the first failed - // attempt; it is multiplied by liveBackoffFactor each time. - liveBackoffBase = 500 * time.Millisecond - - // liveBackoffFactor is the exponential backoff multiplier. - liveBackoffFactor = 2 - - // liveConcurrency caps how many live resolutions may be in - // flight across the whole package at once. - liveConcurrency = 6 - - // minNameservers is the smallest nameserver count a - // well-run zone is expected to publish. - minNameservers = 2 -) - -// liveGate bounds concurrent live resolutions package-wide. It has -// to be package scoped: the whole point is that it is shared by -// every parallel test in the package. -// -//nolint:gochecknoglobals // package-wide live query rate limit -var liveGate = make(chan struct{}, liveConcurrency) - -var ( - // errLiveNoAnswer reports that a live operation produced no - // usable answer, which is retried rather than asserted on. - errLiveNoAnswer = errors.New("no answer from live DNS") - - // errLiveNoQuorum reports that too few of a domain's - // nameservers answered for a quorum assertion to be made. - errLiveNoQuorum = errors.New("no nameserver quorum") -) - -// runLive executes one attempt of a live operation, holding a slot -// in liveGate for its duration and bounding it with its own -// timeout. -func runLive(op func(ctx context.Context) error) error { - liveGate <- struct{}{} - defer func() { <-liveGate }() - - ctx, cancel := context.WithTimeout( - context.Background(), liveAttemptTimeout, - ) - defer cancel() - - return op(ctx) -} - -// retryLive runs op until it reports success, retrying transport -// failures with exponential backoff, and fails the test if every -// attempt fails. op returns an error only for a failure to obtain -// an answer — never for an answer the test disagrees with, which -// belongs in an assertion so that it fails immediately. op stores -// whatever it obtained where its caller can find it. -func retryLive( - t *testing.T, - what string, - op func(ctx context.Context) error, -) { - t.Helper() - - var last error - - backoff := liveBackoffBase - - for attempt := range liveAttempts { - if attempt > 0 { - t.Logf( - "%s: attempt %d of %d failed (%v), "+ - "retrying in %s", - what, attempt, liveAttempts, last, backoff, - ) - time.Sleep(backoff) - - backoff *= liveBackoffFactor - } - - last = runLive(op) - if last == nil { - return - } - } - - t.Fatalf( - "%s: no answer after %d live attempts: %v", - what, liveAttempts, last, - ) -} +// errLiveNoQuorum reports that too few of a domain's nameservers +// answered for a quorum assertion to be made. +var errLiveNoQuorum = errors.New("no nameserver quorum") // liveQuorum is how many of total nameservers must agree for a // multi-nameserver assertion to hold: a strict majority. @@ -272,7 +162,7 @@ func liveFindAuthoritative( var out []string - retryLive( + livedns.Retry( t, "FindAuthoritativeNameservers("+domain+")", func(ctx context.Context) error { @@ -284,7 +174,7 @@ func liveFindAuthoritative( if len(ns) == 0 { return fmt.Errorf( "%w: %s has no nameservers", - errLiveNoAnswer, domain, + livedns.ErrNoAnswer, domain, ) } @@ -308,7 +198,7 @@ func liveLookupNS( var out []string - retryLive( + livedns.Retry( t, "LookupNS("+domain+")", func(ctx context.Context) error { @@ -320,7 +210,7 @@ func liveLookupNS( if len(ns) == 0 { return fmt.Errorf( "%w: %s has no nameservers", - errLiveNoAnswer, domain, + livedns.ErrNoAnswer, domain, ) } @@ -350,7 +240,7 @@ func liveQueryNameserver( var out *resolver.NameserverResponse - retryLive( + livedns.Retry( t, what, func(ctx context.Context) error { @@ -365,7 +255,7 @@ func liveQueryNameserver( resp.Status == resolver.StatusError { return fmt.Errorf( "%w: %s returned %s: %s", - errLiveNoAnswer, nameserver, + livedns.ErrNoAnswer, nameserver, resp.Status, resp.Error, ) } @@ -392,7 +282,7 @@ func liveQueryAllNameservers( var out map[string]*resolver.NameserverResponse - retryLive( + livedns.Retry( t, "QueryAllNameservers("+hostname+")", func(ctx context.Context) error { @@ -404,7 +294,7 @@ func liveQueryAllNameservers( if len(results) == 0 { return fmt.Errorf( "%w: no nameservers queried for %s", - errLiveNoAnswer, hostname, + livedns.ErrNoAnswer, hostname, ) } @@ -437,7 +327,7 @@ func liveResolveIPs( var out []string - retryLive( + livedns.Retry( t, "ResolveIPAddresses("+hostname+")", func(ctx context.Context) error { @@ -449,7 +339,7 @@ func liveResolveIPs( if len(ips) == 0 { return fmt.Errorf( "%w: no addresses for %s", - errLiveNoAnswer, hostname, + livedns.ErrNoAnswer, hostname, ) } @@ -476,7 +366,7 @@ func liveResolveIPsAllowingEmpty( var out []string - retryLive( + livedns.Retry( t, "ResolveIPAddresses("+hostname+")", func(ctx context.Context) error { diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index aec9b89..83b3f47 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -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. diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 13bdcde..2dd0369 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -10,7 +10,6 @@ import ( "testing" "time" - "github.com/miekg/dns" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -33,8 +32,8 @@ func newTestResolver(t *testing.T) *resolver.Resolver { } // findOneNSForDomain picks one authoritative nameserver to aim a -// test at. Live-DNS retry, concurrency and quorum handling live in -// livedns_test.go. +// test at. Quorum handling lives in livedns_test.go, and the live-DNS +// retry and concurrency limit in package livedns. func findOneNSForDomain( t *testing.T, r *resolver.Resolver, @@ -526,21 +525,18 @@ func TestQueryAllNameservers_ContextCanceled(t *testing.T) { 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{}, - ) + r := newTestResolver(t) + // Nothing answers at 192.0.2.1, a documentation address. The + // resolver tries each query twice; if the deadline has passed + // before the second try starts, the query is reported as nodata, + // not timeout. So the deadline must outlast the first try's + // two-second timeout. ctx, cancel := context.WithTimeout( - context.Background(), 10*time.Second, + context.Background(), 3*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", @@ -551,27 +547,6 @@ func TestQueryNameserverIP_Timeout(t *testing.T) { 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 } - func TestResolveIPAddresses_ContextCanceled(t *testing.T) { t.Parallel() diff --git a/internal/watcher/watcher_test.go b/internal/watcher/watcher_test.go index 0069af2..5ad6030 100644 --- a/internal/watcher/watcher_test.go +++ b/internal/watcher/watcher_test.go @@ -2,151 +2,77 @@ package watcher_test import ( "context" - "errors" "fmt" + "log/slog" + "slices" "sync" "testing" "time" "sneak.berlin/go/dnswatcher/internal/config" + "sneak.berlin/go/dnswatcher/internal/livedns" "sneak.berlin/go/dnswatcher/internal/portcheck" + "sneak.berlin/go/dnswatcher/internal/resolver" "sneak.berlin/go/dnswatcher/internal/state" "sneak.berlin/go/dnswatcher/internal/tlscheck" "sneak.berlin/go/dnswatcher/internal/watcher" ) -// errNotFound is returned when mock data is missing. -var errNotFound = errors.New("not found") - -// Fixture values shared across tests. +// The watcher looks these names up in live DNS with the real resolver, +// so tests assert on what the watcher does with the answers, never on +// the records these zones publish. testHost's nameservers and addresses +// stay the same from one check to the next, which the tests that check +// it twice rely on. const ( - testDomain = "example.com" - testHost = "www.example.com" - testNS1 = "ns1.example.com." - testNS2 = "ns2.example.com." - testIPv4 = "93.184.216.34" - testIP = "1.2.3.4" + testDomain = "google.com" + testHost = "cloudflare.com" testIssuer = "DigiCert" ) -// --- Mock implementations --- +// Saved-state values that live DNS never returns: nameserver names +// under .invalid and a documentation address. +const ( + oldNS1 = "ns1.example.invalid." + oldNS2 = "ns2.example.invalid." + oldIP = "192.0.2.1" +) -type mockResolver struct { - mu sync.Mutex - nsRecords map[string][]string - allRecords map[string]map[string]map[string][]string - ipAddresses map[string][]string - lookupNSErr error - allRecordsErr error - resolveIPErr error - lookupNSCalls int - allRecordCalls int -} - -func (m *mockResolver) LookupNS( - _ context.Context, - domain string, -) ([]string, error) { - m.mu.Lock() - defer m.mu.Unlock() - - m.lookupNSCalls++ - - if m.lookupNSErr != nil { - return nil, m.lookupNSErr - } - - ns, ok := m.nsRecords[domain] - if !ok { - return nil, fmt.Errorf( - "%w: NS for %s", errNotFound, domain, - ) - } - - return ns, nil -} - -func (m *mockResolver) LookupAllRecords( - _ context.Context, - hostname string, -) (map[string]map[string][]string, error) { - m.mu.Lock() - defer m.mu.Unlock() - - m.allRecordCalls++ - - if m.allRecordsErr != nil { - return nil, m.allRecordsErr - } - - recs, ok := m.allRecords[hostname] - if !ok { - return nil, fmt.Errorf( - "%w: records for %s", errNotFound, hostname, - ) - } - - return recs, nil -} - -func (m *mockResolver) ResolveIPAddresses( - _ context.Context, - hostname string, -) ([]string, error) { - m.mu.Lock() - defer m.mu.Unlock() - - if m.resolveIPErr != nil { - return nil, m.resolveIPErr - } - - ips, ok := m.ipAddresses[hostname] - if !ok { - return nil, fmt.Errorf( - "%w: IPs for %s", errNotFound, hostname, - ) - } - - return ips, nil -} +// --- Stand-ins for the port checker, TLS checker and notifier --- +// +// DNS has none: the watchers built here use the real resolver (see +// TESTING.md). +// mockPortChecker reports every port open until closed is set. type mockPortChecker struct { - mu sync.Mutex - results map[string]bool - err error - calls int + mu sync.Mutex + closed bool + calls int } func (m *mockPortChecker) CheckPort( _ context.Context, - address string, - port int, + _ string, + _ int, ) (*portcheck.PortResult, error) { m.mu.Lock() defer m.mu.Unlock() m.calls++ - if m.err != nil { - return nil, m.err - } - - key := fmt.Sprintf("%s:%d", address, port) - open := m.results[key] - - return &portcheck.PortResult{Open: open}, nil + return &portcheck.PortResult{Open: !m.closed}, nil } +// mockTLSChecker returns a certificate for the requested hostname that +// expires at notAfter. type mockTLSChecker struct { - mu sync.Mutex - certs map[string]*tlscheck.CertificateInfo - err error - calls int + mu sync.Mutex + notAfter time.Time + calls int } func (m *mockTLSChecker) CheckCertificate( _ context.Context, - ip string, + _ string, hostname string, ) (*tlscheck.CertificateInfo, error) { m.mu.Lock() @@ -154,20 +80,12 @@ func (m *mockTLSChecker) CheckCertificate( m.calls++ - if m.err != nil { - return nil, m.err - } - - key := fmt.Sprintf("%s:%s", ip, hostname) - cert, ok := m.certs[key] - - if !ok { - return nil, fmt.Errorf( - "%w: cert for %s", errNotFound, key, - ) - } - - return cert, nil + return &tlscheck.CertificateInfo{ + CommonName: hostname, + Issuer: testIssuer, + NotAfter: m.notAfter, + SubjectAlternativeNames: []string{hostname}, + }, nil } type notification struct { @@ -205,10 +123,9 @@ func (m *mockNotifier) getNotifications() []notification { return result } -// --- Helper to build a Watcher for testing --- +// --- Helpers to build a Watcher and run its checks against live DNS --- type testDeps struct { - resolver *mockResolver portChecker *mockPortChecker tlsChecker *mockTLSChecker notifier *mockNotifier @@ -223,16 +140,9 @@ func newTestWatcher( t.Helper() deps := &testDeps{ - resolver: &mockResolver{ - nsRecords: make(map[string][]string), - allRecords: make(map[string]map[string]map[string][]string), - ipAddresses: make(map[string][]string), - }, - portChecker: &mockPortChecker{ - results: make(map[string]bool), - }, + portChecker: &mockPortChecker{}, tlsChecker: &mockTLSChecker{ - certs: make(map[string]*tlscheck.CertificateInfo), + notAfter: time.Now().Add(90 * 24 * time.Hour), }, notifier: &mockNotifier{}, config: cfg, @@ -243,7 +153,7 @@ func newTestWatcher( w := watcher.NewForTest( deps.config, deps.state, - deps.resolver, + resolver.NewFromLogger(slog.Default()), deps.portChecker, deps.tlsChecker, deps.notifier, @@ -263,6 +173,122 @@ func defaultTestConfig(t *testing.T) *config.Config { } } +// checkOnce runs the watcher's checks once and returns an error when a +// configured name has no hostname state saved by this check, or that +// state holds no address. Either live DNS gave no answer for the name, +// or the watcher saved no fresh result for it. +func checkOnce( + ctx context.Context, + w *watcher.Watcher, + deps *testDeps, +) error { + started := time.Now() + + w.RunOnce(ctx) + + names := slices.Concat(deps.config.Domains, deps.config.Hostnames) + + for _, name := range names { + hs, ok := deps.state.GetHostnameState(name) + if !ok || hs.LastChecked.Before(started) || + len(addresses(hs)) == 0 { + return fmt.Errorf( + "%s: %w, or the watcher saved no fresh "+ + "result for it", + name, livedns.ErrNoAnswer, + ) + } + } + + return nil +} + +// runChecks builds a watcher, lets prepare set up the saved state and +// stand-ins it starts from, and runs its checks once against live DNS. +// If change is not nil, change then alters the saved state or stand-ins +// and the checks run a second time. When either check finds no fresh +// address for a name (see checkOnce), the watcher is thrown away and +// all of this runs again on a new one, so a failed attempt leaves +// nothing behind in the saved state, the stand-ins or the notifications. +func runChecks( + t *testing.T, + cfg *config.Config, + prepare, change func(deps *testDeps), +) *testDeps { + t.Helper() + + var deps *testDeps + + livedns.Retry(t, "watcher checks", func(ctx context.Context) error { + var w *watcher.Watcher + + w, deps = newTestWatcher(t, cfg) + + if prepare != nil { + prepare(deps) + } + + err := checkOnce(ctx, w, deps) + if err != nil || change == nil { + return err + } + + change(deps) + + return checkOnce(ctx, w, deps) + }) + + return deps +} + +// addresses returns the A and AAAA values saved for a hostname. +func addresses(hs *state.HostnameState) []string { + var ips []string + + for _, nsState := range hs.RecordsByNameserver { + ips = append(ips, nsState.Records["A"]...) + ips = append(ips, nsState.Records["AAAA"]...) + } + + return ips +} + +// assertNotified checks that a notification with this title and +// priority was sent. +func assertNotified( + t *testing.T, + deps *testDeps, + title, priority string, +) { + t.Helper() + + notifications := deps.notifier.getNotifications() + + for _, n := range notifications { + if n.Title == title && n.Priority == priority { + return + } + } + + t.Errorf( + "expected %s notification %q, got: %v", + priority, title, notifications, + ) +} + +// countNotifications counts the notifications sent with this title. +func countNotifications(deps *testDeps, title string) int { + count := 0 + + for _, n := range deps.notifier.getNotifications() { + if n.Title == title { + count++ + } + } + + return count +} + func TestFirstRunBaseline(t *testing.T) { t.Parallel() @@ -270,51 +296,12 @@ func TestFirstRunBaseline(t *testing.T) { cfg.Domains = []string{testDomain} cfg.Hostnames = []string{testHost} - w, deps := newTestWatcher(t, cfg) - setupBaselineMocks(deps) - - w.RunOnce(t.Context()) + deps := runChecks(t, cfg, nil, nil) assertNoNotifications(t, deps) assertStatePopulated(t, deps) } -func setupBaselineMocks(deps *testDeps) { - deps.resolver.nsRecords[testDomain] = []string{ - testNS1, - testNS2, - } - deps.resolver.allRecords[testDomain] = map[string]map[string][]string{ - testNS1: {"A": {testIPv4}}, - testNS2: {"A": {testIPv4}}, - } - deps.resolver.allRecords[testHost] = map[string]map[string][]string{ - testNS1: {"A": {testIPv4}}, - testNS2: {"A": {testIPv4}}, - } - deps.resolver.ipAddresses[testHost] = []string{ - testIPv4, - } - deps.portChecker.results["93.184.216.34:80"] = true - deps.portChecker.results["93.184.216.34:443"] = true - deps.tlsChecker.certs["93.184.216.34:www.example.com"] = &tlscheck.CertificateInfo{ - CommonName: testHost, - Issuer: testIssuer, - NotAfter: time.Now().Add(90 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - testHost, - }, - } - deps.tlsChecker.certs["93.184.216.34:example.com"] = &tlscheck.CertificateInfo{ - CommonName: testDomain, - Issuer: testIssuer, - NotAfter: time.Now().Add(90 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - testDomain, - }, - } -} - func assertNoNotifications( t *testing.T, deps *testDeps, @@ -361,26 +348,7 @@ func TestDomainPortAndTLSChecks(t *testing.T) { cfg := defaultTestConfig(t) cfg.Domains = []string{testDomain} - w, deps := newTestWatcher(t, cfg) - - deps.resolver.nsRecords[testDomain] = []string{ - testNS1, - } - deps.resolver.allRecords[testDomain] = map[string]map[string][]string{ - testNS1: {"A": {testIPv4}}, - } - deps.portChecker.results["93.184.216.34:80"] = true - deps.portChecker.results["93.184.216.34:443"] = true - deps.tlsChecker.certs["93.184.216.34:example.com"] = &tlscheck.CertificateInfo{ - CommonName: testDomain, - Issuer: testIssuer, - NotAfter: time.Now().Add(90 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - testDomain, - }, - } - - w.RunOnce(t.Context()) + deps := runChecks(t, cfg, nil, nil) snap := deps.state.GetSnapshot() @@ -419,50 +387,18 @@ func TestNSChangeDetection(t *testing.T) { cfg := defaultTestConfig(t) cfg.Domains = []string{testDomain} - w, deps := newTestWatcher(t, cfg) + // The saved state lists nameservers that live DNS does not. + deps := runChecks(t, cfg, func(deps *testDeps) { + deps.state.SetDomainState(testDomain, &state.DomainState{ + Nameservers: []string{oldNS1, oldNS2}, + }) + }, nil) - deps.resolver.nsRecords[testDomain] = []string{ - testNS1, - testNS2, - } - deps.resolver.allRecords[testDomain] = map[string]map[string][]string{ - testNS1: {"A": {testIP}}, - testNS2: {"A": {testIP}}, - } - deps.portChecker.results["1.2.3.4:80"] = false - deps.portChecker.results["1.2.3.4:443"] = false + assertNotified(t, deps, "NS Change: "+testDomain, "warning") - ctx := t.Context() - w.RunOnce(ctx) - - deps.resolver.mu.Lock() - deps.resolver.nsRecords[testDomain] = []string{ - testNS1, - "ns3.example.com.", - } - deps.resolver.allRecords[testDomain] = map[string]map[string][]string{ - testNS1: {"A": {testIP}}, - "ns3.example.com.": {"A": {testIP}}, - } - deps.resolver.mu.Unlock() - - w.RunOnce(ctx) - - notifications := deps.notifier.getNotifications() - if len(notifications) == 0 { - t.Error("expected notification for NS change") - } - - found := false - - for _, n := range notifications { - if n.Priority == "warning" { - found = true - } - } - - if !found { - t.Error("expected warning-priority NS change notification") + ds, _ := deps.state.GetDomainState(testDomain) + if slices.Contains(ds.Nameservers, oldNS1) { + t.Errorf("saved nameservers not updated: %v", ds.Nameservers) } } @@ -472,40 +408,18 @@ func TestRecordChangeDetection(t *testing.T) { cfg := defaultTestConfig(t) cfg.Hostnames = []string{testHost} - w, deps := newTestWatcher(t, cfg) + // Between the checks, save for every nameserver an address live DNS + // never returns. + deps := runChecks(t, cfg, nil, func(deps *testDeps) { + hs, _ := deps.state.GetHostnameState(testHost) + for _, nsState := range hs.RecordsByNameserver { + nsState.Records = map[string][]string{"A": {oldIP}} + } - deps.resolver.allRecords[testHost] = map[string]map[string][]string{ - testNS1: {"A": {testIPv4}}, - } - deps.resolver.ipAddresses[testHost] = []string{ - testIPv4, - } - deps.portChecker.results["93.184.216.34:80"] = false - deps.portChecker.results["93.184.216.34:443"] = false + deps.state.SetHostnameState(testHost, hs) + }) - ctx := t.Context() - w.RunOnce(ctx) - - deps.resolver.mu.Lock() - deps.resolver.allRecords[testHost] = map[string]map[string][]string{ - testNS1: {"A": {"93.184.216.35"}}, - } - deps.resolver.ipAddresses[testHost] = []string{ - "93.184.216.35", - } - deps.resolver.mu.Unlock() - - deps.portChecker.mu.Lock() - deps.portChecker.results["93.184.216.35:80"] = false - deps.portChecker.results["93.184.216.35:443"] = false - deps.portChecker.mu.Unlock() - - w.RunOnce(ctx) - - notifications := deps.notifier.getNotifications() - if len(notifications) == 0 { - t.Error("expected notification for record change") - } + assertNotified(t, deps, "Record Change: "+testHost, "warning") } func TestPortStateChange(t *testing.T) { @@ -514,38 +428,23 @@ func TestPortStateChange(t *testing.T) { cfg := defaultTestConfig(t) cfg.Hostnames = []string{testHost} - w, deps := newTestWatcher(t, cfg) + // Between the checks, every port closes. + deps := runChecks(t, cfg, nil, func(deps *testDeps) { + deps.portChecker.mu.Lock() + deps.portChecker.closed = true + deps.portChecker.mu.Unlock() + }) - deps.resolver.allRecords[testHost] = map[string]map[string][]string{ - testNS1: {"A": {testIP}}, - } - deps.resolver.ipAddresses[testHost] = []string{ - testIP, - } - deps.portChecker.results["1.2.3.4:80"] = true - deps.portChecker.results["1.2.3.4:443"] = true - deps.tlsChecker.certs["1.2.3.4:www.example.com"] = &tlscheck.CertificateInfo{ - CommonName: testHost, - Issuer: testIssuer, - NotAfter: time.Now().Add(90 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - testHost, - }, - } + hs, _ := deps.state.GetHostnameState(testHost) + assertNotified( + t, deps, "Port Change: "+addresses(hs)[0]+":443", "warning", + ) +} - ctx := t.Context() - w.RunOnce(ctx) - - deps.portChecker.mu.Lock() - deps.portChecker.results["1.2.3.4:443"] = false - deps.portChecker.mu.Unlock() - - w.RunOnce(ctx) - - notifications := deps.notifier.getNotifications() - if len(notifications) == 0 { - t.Error("expected notification for port state change") - } +// expiresInThreeDays makes the TLS checker return certificates that +// expire within the seven-day warning period. +func expiresInThreeDays(deps *testDeps) { + deps.tlsChecker.notAfter = time.Now().Add(3 * 24 * time.Hour) } func TestTLSExpiryWarning(t *testing.T) { @@ -554,49 +453,9 @@ func TestTLSExpiryWarning(t *testing.T) { cfg := defaultTestConfig(t) cfg.Hostnames = []string{testHost} - w, deps := newTestWatcher(t, cfg) + deps := runChecks(t, cfg, expiresInThreeDays, nil) - deps.resolver.allRecords[testHost] = map[string]map[string][]string{ - testNS1: {"A": {testIP}}, - } - deps.resolver.ipAddresses[testHost] = []string{ - testIP, - } - deps.portChecker.results["1.2.3.4:80"] = true - deps.portChecker.results["1.2.3.4:443"] = true - deps.tlsChecker.certs["1.2.3.4:www.example.com"] = &tlscheck.CertificateInfo{ - CommonName: testHost, - Issuer: testIssuer, - NotAfter: time.Now().Add(3 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - testHost, - }, - } - - ctx := t.Context() - - // First run = baseline - w.RunOnce(ctx) - - // Second run should warn about expiry - w.RunOnce(ctx) - - notifications := deps.notifier.getNotifications() - - found := false - - for _, n := range notifications { - if n.Priority == "warning" { - found = true - } - } - - if !found { - t.Errorf( - "expected expiry warning, got: %v", - notifications, - ) - } + assertNotified(t, deps, "TLS Expiry Warning: "+testHost, "warning") } func TestTLSExpiryWarningDedup(t *testing.T) { @@ -606,50 +465,25 @@ func TestTLSExpiryWarningDedup(t *testing.T) { cfg.Hostnames = []string{testHost} cfg.TLSInterval = 24 * time.Hour - w, deps := newTestWatcher(t, cfg) + title := "TLS Expiry Warning: " + testHost - deps.resolver.allRecords[testHost] = map[string]map[string][]string{ - testNS1: {"A": {testIP}}, - } - deps.resolver.ipAddresses[testHost] = []string{ - testIP, - } - deps.portChecker.results["1.2.3.4:80"] = true - deps.portChecker.results["1.2.3.4:443"] = true - deps.tlsChecker.certs["1.2.3.4:www.example.com"] = &tlscheck.CertificateInfo{ - CommonName: testHost, - Issuer: testIssuer, - NotAfter: time.Now().Add(3 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - testHost, - }, + // The second check comes within the TLS interval of the first, + // so it must not warn again. + var warnings int + + deps := runChecks(t, cfg, expiresInThreeDays, func(deps *testDeps) { + warnings = countNotifications(deps, title) + }) + + if warnings == 0 { + t.Fatal("expected expiry warnings from the first check") } - ctx := t.Context() - - // First run = baseline, no notifications - w.RunOnce(ctx) - - // Second run should fire one expiry warning - w.RunOnce(ctx) - - // Third run should NOT fire another warning (dedup) - w.RunOnce(ctx) - - notifications := deps.notifier.getNotifications() - - expiryCount := 0 - - for _, n := range notifications { - if n.Title == "TLS Expiry Warning: www.example.com" { - expiryCount++ - } - } - - if expiryCount != 1 { + got := countNotifications(deps, title) + if got != warnings { t.Errorf( - "expected exactly 1 expiry warning (dedup), got %d", - expiryCount, + "expected %d expiry warnings (dedup), got %d", + warnings, got, ) } } @@ -657,21 +491,12 @@ func TestTLSExpiryWarningDedup(t *testing.T) { func TestGracefulShutdown(t *testing.T) { t.Parallel() + // No domains or hostnames: stopping does not involve DNS. cfg := defaultTestConfig(t) - cfg.Domains = []string{testDomain} cfg.DNSInterval = 100 * time.Millisecond cfg.TLSInterval = 100 * time.Millisecond - w, deps := newTestWatcher(t, cfg) - - deps.resolver.nsRecords[testDomain] = []string{ - testNS1, - } - deps.resolver.allRecords[testDomain] = map[string]map[string][]string{ - testNS1: {"A": {testIP}}, - } - deps.portChecker.results["1.2.3.4:80"] = false - deps.portChecker.results["1.2.3.4:443"] = false + w, _ := newTestWatcher(t, cfg) ctx, cancel := context.WithCancel(t.Context()) @@ -693,90 +518,52 @@ func TestGracefulShutdown(t *testing.T) { } } -func setupHostnameIP( - deps *testDeps, - hostname, ip string, -) { - deps.resolver.allRecords[hostname] = map[string]map[string][]string{ - testNS1: {"A": {ip}}, - } - deps.portChecker.results[ip+":80"] = true - deps.portChecker.results[ip+":443"] = true - deps.tlsChecker.certs[ip+":"+hostname] = &tlscheck.CertificateInfo{ - CommonName: hostname, - Issuer: testIssuer, - NotAfter: time.Now().Add(90 * 24 * time.Hour), - SubjectAlternativeNames: []string{hostname}, - } -} - -func updateHostnameIP(deps *testDeps, hostname, ip string) { - deps.resolver.mu.Lock() - deps.resolver.allRecords[hostname] = map[string]map[string][]string{ - testNS1: {"A": {ip}}, - } - deps.resolver.mu.Unlock() - - deps.portChecker.mu.Lock() - deps.portChecker.results[ip+":80"] = true - deps.portChecker.results[ip+":443"] = true - deps.portChecker.mu.Unlock() - - deps.tlsChecker.mu.Lock() - deps.tlsChecker.certs[ip+":"+hostname] = &tlscheck.CertificateInfo{ - CommonName: hostname, - Issuer: testIssuer, - NotAfter: time.Now().Add(90 * 24 * time.Hour), - SubjectAlternativeNames: []string{hostname}, - } - deps.tlsChecker.mu.Unlock() -} - func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) cfg.Hostnames = []string{testHost} - w, deps := newTestWatcher(t, cfg) - - setupHostnameIP(deps, testHost, "10.0.0.1") - - ctx := t.Context() - w.RunOnce(ctx) + // The saved state says the last check found testHost at oldIP. + deps := runChecks(t, cfg, func(deps *testDeps) { + deps.state.SetHostnameState(testHost, &state.HostnameState{ + RecordsByNameserver: map[string]*state.NameserverRecordState{ + oldNS1: { + Records: map[string][]string{"A": {oldIP}}, + Status: "ok", + }, + }, + }) + }, nil) snap := deps.state.GetSnapshot() - if _, ok := snap.Ports["10.0.0.1:80"]; !ok { - t.Fatal("expected port state for 10.0.0.1:80") + + if _, ok := snap.Ports[oldIP+":80"]; ok { + t.Error("port check used stale DNS: found " + oldIP + ":80") } - // DNS changes to a new IP; port and TLS must pick it up. - updateHostnameIP(deps, testHost, "10.0.0.2") + // Port and TLS checks must use the addresses this check found. + for _, ip := range addresses(snap.Hostnames[testHost]) { + if _, ok := snap.Ports[ip+":80"]; !ok { + t.Error("port check used stale DNS: missing " + ip + ":80") + } - w.RunOnce(ctx) - - snap = deps.state.GetSnapshot() - - if _, ok := snap.Ports["10.0.0.2:80"]; !ok { - t.Error("port check used stale DNS: missing 10.0.0.2:80") - } - - certKey := "10.0.0.2:443:www.example.com" - if _, ok := snap.Certificates[certKey]; !ok { - t.Error("TLS check used stale DNS: missing " + certKey) + certKey := ip + ":443:" + testHost + if _, ok := snap.Certificates[certKey]; !ok { + t.Error("TLS check used stale DNS: missing " + certKey) + } } } func TestSendTestNotification_Enabled(t *testing.T) { t.Parallel() + // No domains or hostnames: the startup notification does not + // involve DNS. cfg := defaultTestConfig(t) - cfg.Domains = []string{testDomain} - cfg.Hostnames = []string{testHost} cfg.SendTestNotification = true w, deps := newTestWatcher(t, cfg) - setupBaselineMocks(deps) w.RunOnce(t.Context()) @@ -797,14 +584,11 @@ func TestSendTestNotification_ViaRun(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{testDomain} - cfg.Hostnames = []string{testHost} cfg.SendTestNotification = true cfg.DNSInterval = 24 * time.Hour cfg.TLSInterval = 24 * time.Hour w, deps := newTestWatcher(t, cfg) - setupBaselineMocks(deps) ctx, cancel := context.WithCancel(t.Context()) @@ -844,14 +628,11 @@ func TestSendTestNotification_Disabled(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{testDomain} - cfg.Hostnames = []string{testHost} cfg.SendTestNotification = false cfg.DNSInterval = 24 * time.Hour cfg.TLSInterval = 24 * time.Hour w, deps := newTestWatcher(t, cfg) - setupBaselineMocks(deps) ctx, cancel := context.WithCancel(t.Context()) @@ -884,32 +665,22 @@ func TestNSFailureAndRecovery(t *testing.T) { cfg := defaultTestConfig(t) cfg.Hostnames = []string{testHost} - w, deps := newTestWatcher(t, cfg) + // Between the checks, save every nameserver the first check found + // as failed, and add, as answering, one that live DNS does not list. + deps := runChecks(t, cfg, nil, func(deps *testDeps) { + hs, _ := deps.state.GetHostnameState(testHost) + for _, nsState := range hs.RecordsByNameserver { + nsState.Status = "error" + } - deps.resolver.allRecords[testHost] = map[string]map[string][]string{ - testNS1: {"A": {testIP}}, - testNS2: {"A": {testIP}}, - } - deps.resolver.ipAddresses[testHost] = []string{ - testIP, - } - deps.portChecker.results["1.2.3.4:80"] = false - deps.portChecker.results["1.2.3.4:443"] = false + hs.RecordsByNameserver[oldNS1] = &state.NameserverRecordState{ + Records: map[string][]string{"A": {oldIP}}, + Status: "ok", + } - ctx := t.Context() + deps.state.SetHostnameState(testHost, hs) + }) - w.RunOnce(ctx) - - deps.resolver.mu.Lock() - deps.resolver.allRecords[testHost] = map[string]map[string][]string{ - testNS1: {"A": {testIP}}, - } - deps.resolver.mu.Unlock() - - w.RunOnce(ctx) - - notifications := deps.notifier.getNotifications() - if len(notifications) == 0 { - t.Error("expected notification for NS disappearance") - } + assertNotified(t, deps, "NS Failure: "+testHost, "error") + assertNotified(t, deps, "NS Recovery: "+testHost, "success") }