From 3959aedb6a4302a7ef833d419c1dbe5ab71c5787 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 7 Aug 2026 20:43:09 +0000 Subject: [PATCH] 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. --- TESTING.md | 15 +- TODO.md | 24 +- internal/resolver/dns_client.go | 4 +- internal/resolver/resolver.go | 13 - internal/resolver/resolver_test.go | 48 +- internal/watcher/watcher_test.go | 895 ++++++++++++++--------------- 6 files changed, 461 insertions(+), 538 deletions(-) diff --git a/TESTING.md b/TESTING.md index 3dd34c4..babd747 100644 --- a/TESTING.md +++ b/TESTING.md @@ -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,11 +28,14 @@ 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 ### 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 modify linter configuration** to suppress findings diff --git a/TODO.md b/TODO.md index 3e8e557..515761b 100644 --- a/TODO.md +++ b/TODO.md @@ -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 @@ -25,10 +28,15 @@ confirm make check still passes. # Completed Steps +- 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-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) @@ -55,8 +63,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 @@ -140,6 +149,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 diff --git a/internal/resolver/dns_client.go b/internal/resolver/dns_client.go index 589c657..99bca79 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 abstracts DNS wire-protocol exchanges over a single +// transport, letting the resolver switch between UDP and TCP. type DNSClient interface { ExchangeContext( ctx context.Context, 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 bcebfb9..b0dde5d 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" @@ -624,58 +623,41 @@ func TestQueryAllNameservers_ContextCanceled(t *testing.T) { } // ---------------------------------------------------------------- -// Timeout tests +// Unreachable nameserver tests // ---------------------------------------------------------------- -func TestQueryNameserverIP_Timeout(t *testing.T) { +func TestQueryNameserverIP_UnreachableServer(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) ctx, cancel := context.WithTimeout( context.Background(), 10*time.Second, ) t.Cleanup(cancel) - // Query any IP — the client always returns a timeout error. + // 192.0.2.1 is an RFC 5737 documentation address: no + // nameserver can exist there. Depending on the network + // path the queries either time out (silent drop) or fail + // fast (ICMP unreachable), so accept any non-OK status; + // the resolver must return a classified response with no + // records rather than an error or a hang. 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) -} + assert.NotEqual(t, resolver.StatusOK, resp.Status) -// 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{}, + totalRecords := 0 + for _, values := range resp.Records { + totalRecords += len(values) } + + assert.Zero(t, totalRecords) } -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 ea6dbef..ff4d998 100644 --- a/internal/watcher/watcher_test.go +++ b/internal/watcher/watcher_test.go @@ -4,112 +4,77 @@ import ( "context" "errors" "fmt" + "log/slog" + "os" + "sort" + "strings" "sync" "testing" "time" "sneak.berlin/go/dnswatcher/internal/config" "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. +// These tests exercise the watcher against live DNS. DNS is never +// mocked in this repository (see TESTING.md): the watcher is wired +// to the real iterative resolver and queries stable, well-known +// names. Only the non-DNS collaborators (port checker, TLS +// checker, notifier) are test doubles so that notification and +// state behavior can be asserted deterministically. +// +// Change detection is exercised by seeding the state store with a +// synthetic previous observation that live DNS cannot match; DNS +// itself stays live in every run. + +const ( + // testDomain and testHostname are IANA-operated names + // reserved for documentation use; they are stable and + // resolve worldwide. + testDomain = "example.com" + testHostname = "www.example.com" + + // fakeNS uses the RFC 2606 reserved .invalid TLD, so it can + // never collide with a nameserver name in live DNS results. + fakeNS = "ns-baseline.invalid." + + // fakeIP is an RFC 5737 documentation address that live DNS + // can never return for a real hostname. + fakeIP = "203.0.113.1" + + startupTitle = "✅ dnswatcher startup complete" + + // longLife is a certificate lifetime far outside the expiry + // warning window; shortLife is inside it. + longLife = 90 * 24 * time.Hour + shortLife = 3 * 24 * time.Hour + + pollInterval = 100 * time.Millisecond + scanTimeout = 25 * time.Second + notifyGrace = 500 * time.Millisecond + shutdownWait = 5 * time.Second +) + +// errNotFound is returned when a test double has no data. var errNotFound = errors.New("not found") -// --- Mock implementations --- - -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 -} +// --- Test doubles for non-DNS collaborators --- type mockPortChecker struct { mu sync.Mutex - results map[string]bool + openAll bool err error calls int } func (m *mockPortChecker) CheckPort( _ context.Context, - address string, - port int, + _ string, + _ int, ) (*portcheck.PortResult, error) { m.mu.Lock() defer m.mu.Unlock() @@ -120,22 +85,33 @@ func (m *mockPortChecker) CheckPort( return nil, m.err } - key := fmt.Sprintf("%s:%d", address, port) - open := m.results[key] + return &portcheck.PortResult{Open: m.openAll}, nil +} - return &portcheck.PortResult{Open: open}, nil +func (m *mockPortChecker) setOpenAll(open bool) { + m.mu.Lock() + defer m.mu.Unlock() + + m.openAll = open +} + +func (m *mockPortChecker) callCount() int { + m.mu.Lock() + defer m.mu.Unlock() + + return m.calls } type mockTLSChecker struct { mu sync.Mutex - certs map[string]*tlscheck.CertificateInfo + cert *tlscheck.CertificateInfo err error calls int } func (m *mockTLSChecker) CheckCertificate( _ context.Context, - ip string, + _ string, hostname string, ) (*tlscheck.CertificateInfo, error) { m.mu.Lock() @@ -147,16 +123,27 @@ func (m *mockTLSChecker) CheckCertificate( return nil, m.err } - key := fmt.Sprintf("%s:%s", ip, hostname) - cert, ok := m.certs[key] - - if !ok { + if m.cert == nil { return nil, fmt.Errorf( - "%w: cert for %s", errNotFound, key, + "%w: cert for %s", errNotFound, hostname, ) } - return cert, nil + return m.cert, nil +} + +func (m *mockTLSChecker) setCert(cert *tlscheck.CertificateInfo) { + m.mu.Lock() + defer m.mu.Unlock() + + m.cert = cert +} + +func (m *mockTLSChecker) callCount() int { + m.mu.Lock() + defer m.mu.Unlock() + + return m.calls } type notification struct { @@ -194,10 +181,9 @@ func (m *mockNotifier) getNotifications() []notification { return result } -// --- Helper to build a Watcher for testing --- +// --- Helpers to build a Watcher for testing --- type testDeps struct { - resolver *mockResolver portChecker *mockPortChecker tlsChecker *mockTLSChecker notifier *mockNotifier @@ -212,27 +198,23 @@ 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), - }, - tlsChecker: &mockTLSChecker{ - certs: make(map[string]*tlscheck.CertificateInfo), - }, - notifier: &mockNotifier{}, - config: cfg, + portChecker: &mockPortChecker{}, + tlsChecker: &mockTLSChecker{}, + notifier: &mockNotifier{}, + config: cfg, } - deps.state = state.NewForTest() + deps.state = state.NewForTestWithDataDir(cfg.DataDir) + + log := slog.New(slog.NewTextHandler( + os.Stderr, + &slog.HandlerOptions{Level: slog.LevelDebug}, + )) w := watcher.NewForTest( deps.config, deps.state, - deps.resolver, + resolver.NewFromLogger(log), deps.portChecker, deps.tlsChecker, deps.notifier, @@ -252,15 +234,122 @@ func defaultTestConfig(t *testing.T) *config.Config { } } +func testCert( + hostname string, + lifetime time.Duration, +) *tlscheck.CertificateInfo { + return &tlscheck.CertificateInfo{ + CommonName: hostname, + Issuer: "Test CA", + NotAfter: time.Now().Add(lifetime), + SubjectAlternativeNames: []string{hostname}, + } +} + +// setupHealthyEndpoints makes every port report open and every TLS +// check return a long-lived certificate, regardless of which IPs +// live DNS resolves. +func setupHealthyEndpoints(deps *testDeps, hostname string) { + deps.portChecker.setOpenAll(true) + deps.tlsChecker.setCert(testCert(hostname, longLife)) +} + +// countTitled counts notifications whose title starts with prefix. +func countTitled(deps *testDeps, prefix string) int { + count := 0 + + for _, n := range deps.notifier.getNotifications() { + if strings.HasPrefix(n.Title, prefix) { + count++ + } + } + + return count +} + +// waitFor polls cond until it returns true or timeout elapses. +func waitFor(timeout time.Duration, cond func() bool) bool { + deadline := time.Now().Add(timeout) + + for time.Now().Before(deadline) { + if cond() { + return true + } + + time.Sleep(pollInterval) + } + + return cond() +} + +// startWatcher runs w.Run in a goroutine and returns a stop +// function that cancels it and waits for a clean shutdown. +func startWatcher( + t *testing.T, + w *watcher.Watcher, +) func() { + t.Helper() + + ctx, cancel := context.WithCancel(t.Context()) + done := make(chan struct{}) + + go func() { + w.Run(ctx) + close(done) + }() + + return func() { + cancel() + + select { + case <-done: + // Shut down cleanly. + case <-time.After(shutdownWait): + t.Error("watcher did not shut down within timeout") + } + } +} + +// liveIPs extracts all A/AAAA addresses recorded for hostname in +// the snapshot, i.e. what live DNS actually resolved. +func liveIPs(snap state.Snapshot, hostname string) []string { + hs, ok := snap.Hostnames[hostname] + if !ok { + return nil + } + + seen := make(map[string]bool) + + var ips []string + + for _, nsState := range hs.RecordsByNameserver { + for _, recType := range []string{"A", "AAAA"} { + for _, ip := range nsState.Records[recType] { + if !seen[ip] { + seen[ip] = true + + ips = append(ips, ip) + } + } + } + } + + sort.Strings(ips) + + return ips +} + +// --- Tests --- + func TestFirstRunBaseline(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{"example.com"} - cfg.Hostnames = []string{"www.example.com"} + cfg.Domains = []string{testDomain} + cfg.Hostnames = []string{testHostname} w, deps := newTestWatcher(t, cfg) - setupBaselineMocks(deps) + setupHealthyEndpoints(deps, testHostname) w.RunOnce(t.Context()) @@ -268,42 +357,6 @@ func TestFirstRunBaseline(t *testing.T) { assertStatePopulated(t, deps) } -func setupBaselineMocks(deps *testDeps) { - deps.resolver.nsRecords["example.com"] = []string{ - "ns1.example.com.", - "ns2.example.com.", - } - deps.resolver.allRecords["example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"93.184.216.34"}}, - "ns2.example.com.": {"A": {"93.184.216.34"}}, - } - deps.resolver.allRecords["www.example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"93.184.216.34"}}, - "ns2.example.com.": {"A": {"93.184.216.34"}}, - } - deps.resolver.ipAddresses["www.example.com"] = []string{ - "93.184.216.34", - } - 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: "www.example.com", - Issuer: "DigiCert", - NotAfter: time.Now().Add(90 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - "www.example.com", - }, - } - deps.tlsChecker.certs["93.184.216.34:example.com"] = &tlscheck.CertificateInfo{ - CommonName: "example.com", - Issuer: "DigiCert", - NotAfter: time.Now().Add(90 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - "example.com", - }, - } -} - func assertNoNotifications( t *testing.T, deps *testDeps, @@ -313,8 +366,8 @@ func assertNoNotifications( notifications := deps.notifier.getNotifications() if len(notifications) != 0 { t.Errorf( - "expected 0 notifications on first run, got %d", - len(notifications), + "expected 0 notifications on first run, got %d: %v", + len(notifications), notifications, ) } } @@ -348,56 +401,31 @@ func TestDomainPortAndTLSChecks(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{"example.com"} + cfg.Domains = []string{testDomain} w, deps := newTestWatcher(t, cfg) - - deps.resolver.nsRecords["example.com"] = []string{ - "ns1.example.com.", - } - deps.resolver.allRecords["example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"93.184.216.34"}}, - } - 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: "example.com", - Issuer: "DigiCert", - NotAfter: time.Now().Add(90 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - "example.com", - }, - } + setupHealthyEndpoints(deps, testDomain) w.RunOnce(t.Context()) snap := deps.state.GetSnapshot() - // Domain should have port state populated + // The domain resolved via live DNS should have port state + // populated for its real addresses. if len(snap.Ports) == 0 { t.Error("expected port state for domain, got none") } - // Domain should have certificate state populated + // Domain should have certificate state populated. if len(snap.Certificates) == 0 { t.Error("expected certificate state for domain, got none") } - // Verify port checker was actually called - deps.portChecker.mu.Lock() - calls := deps.portChecker.calls - deps.portChecker.mu.Unlock() - - if calls == 0 { + if deps.portChecker.callCount() == 0 { t.Error("expected port checker to be called for domain") } - // Verify TLS checker was actually called - deps.tlsChecker.mu.Lock() - tlsCalls := deps.tlsChecker.calls - deps.tlsChecker.mu.Unlock() - - if tlsCalls == 0 { + if deps.tlsChecker.callCount() == 0 { t.Error("expected TLS checker to be called for domain") } } @@ -406,45 +434,30 @@ func TestNSChangeDetection(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{"example.com"} + cfg.Domains = []string{testDomain} w, deps := newTestWatcher(t, cfg) - deps.resolver.nsRecords["example.com"] = []string{ - "ns1.example.com.", - "ns2.example.com.", - } - deps.resolver.allRecords["example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"1.2.3.4"}}, - "ns2.example.com.": {"A": {"1.2.3.4"}}, - } - deps.portChecker.results["1.2.3.4:80"] = false - deps.portChecker.results["1.2.3.4:443"] = false - ctx := t.Context() w.RunOnce(ctx) - deps.resolver.mu.Lock() - deps.resolver.nsRecords["example.com"] = []string{ - "ns1.example.com.", - "ns3.example.com.", - } - deps.resolver.allRecords["example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"1.2.3.4"}}, - "ns3.example.com.": {"A": {"1.2.3.4"}}, - } - deps.resolver.mu.Unlock() + // Replace the recorded NS baseline with a synthetic value + // that live DNS cannot return; the next live run must + // detect the difference. + deps.state.SetDomainState(testDomain, &state.DomainState{ + Nameservers: []string{fakeNS}, + LastChecked: time.Now().UTC(), + }) w.RunOnce(ctx) - notifications := deps.notifier.getNotifications() - if len(notifications) == 0 { + if countTitled(deps, "NS Change: ") == 0 { t.Error("expected notification for NS change") } found := false - for _, n := range notifications { + for _, n := range deps.notifier.getNotifications() { if n.Priority == "warning" { found = true } @@ -459,40 +472,41 @@ func TestRecordChangeDetection(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Hostnames = []string{"www.example.com"} + cfg.Hostnames = []string{testHostname} w, deps := newTestWatcher(t, cfg) - deps.resolver.allRecords["www.example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"93.184.216.34"}}, - } - deps.resolver.ipAddresses["www.example.com"] = []string{ - "93.184.216.34", - } - deps.portChecker.results["93.184.216.34:80"] = false - deps.portChecker.results["93.184.216.34:443"] = false - ctx := t.Context() w.RunOnce(ctx) - deps.resolver.mu.Lock() - deps.resolver.allRecords["www.example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"93.184.216.35"}}, + hs, ok := deps.state.GetHostnameState(testHostname) + if !ok || len(hs.RecordsByNameserver) == 0 { + t.Fatal("expected hostname state after first live run") } - deps.resolver.ipAddresses["www.example.com"] = []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() + // Rewrite every nameserver's recorded answer to a + // documentation address; the next live run must observe + // different records and notify. + tampered := &state.HostnameState{ + RecordsByNameserver: make( + map[string]*state.NameserverRecordState, + ), + LastChecked: hs.LastChecked, + } + + for ns := range hs.RecordsByNameserver { + tampered.RecordsByNameserver[ns] = &state.NameserverRecordState{ + Records: map[string][]string{"A": {fakeIP}}, + Status: "ok", + LastChecked: time.Now().UTC(), + } + } + + deps.state.SetHostnameState(testHostname, tampered) w.RunOnce(ctx) - notifications := deps.notifier.getNotifications() - if len(notifications) == 0 { + if countTitled(deps, "Record Change: ") == 0 { t.Error("expected notification for record change") } } @@ -501,38 +515,20 @@ func TestPortStateChange(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Hostnames = []string{"www.example.com"} + cfg.Hostnames = []string{testHostname} w, deps := newTestWatcher(t, cfg) - - deps.resolver.allRecords["www.example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"1.2.3.4"}}, - } - deps.resolver.ipAddresses["www.example.com"] = []string{ - "1.2.3.4", - } - 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: "www.example.com", - Issuer: "DigiCert", - NotAfter: time.Now().Add(90 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - "www.example.com", - }, - } + setupHealthyEndpoints(deps, testHostname) ctx := t.Context() w.RunOnce(ctx) - deps.portChecker.mu.Lock() - deps.portChecker.results["1.2.3.4:443"] = false - deps.portChecker.mu.Unlock() + // All live-resolved addresses flip from open to closed. + deps.portChecker.setOpenAll(false) w.RunOnce(ctx) - notifications := deps.notifier.getNotifications() - if len(notifications) == 0 { + if countTitled(deps, "Port Change: ") == 0 { t.Error("expected notification for port state change") } } @@ -541,41 +537,19 @@ func TestTLSExpiryWarning(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Hostnames = []string{"www.example.com"} + cfg.Hostnames = []string{testHostname} w, deps := newTestWatcher(t, cfg) + deps.portChecker.setOpenAll(true) + deps.tlsChecker.setCert(testCert(testHostname, shortLife)) - deps.resolver.allRecords["www.example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"1.2.3.4"}}, - } - deps.resolver.ipAddresses["www.example.com"] = []string{ - "1.2.3.4", - } - 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: "www.example.com", - Issuer: "DigiCert", - NotAfter: time.Now().Add(3 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - "www.example.com", - }, - } - - ctx := t.Context() - - // First run = baseline - w.RunOnce(ctx) - - // Second run should warn about expiry - w.RunOnce(ctx) - - notifications := deps.notifier.getNotifications() + w.RunOnce(t.Context()) found := false - for _, n := range notifications { - if n.Priority == "warning" { + for _, n := range deps.notifier.getNotifications() { + if strings.HasPrefix(n.Title, "TLS Expiry Warning: ") && + n.Priority == "warning" { found = true } } @@ -583,7 +557,7 @@ func TestTLSExpiryWarning(t *testing.T) { if !found { t.Errorf( "expected expiry warning, got: %v", - notifications, + deps.notifier.getNotifications(), ) } } @@ -592,53 +566,31 @@ func TestTLSExpiryWarningDedup(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Hostnames = []string{"www.example.com"} + cfg.Hostnames = []string{testHostname} cfg.TLSInterval = 24 * time.Hour w, deps := newTestWatcher(t, cfg) - - deps.resolver.allRecords["www.example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"1.2.3.4"}}, - } - deps.resolver.ipAddresses["www.example.com"] = []string{ - "1.2.3.4", - } - 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: "www.example.com", - Issuer: "DigiCert", - NotAfter: time.Now().Add(3 * 24 * time.Hour), - SubjectAlternativeNames: []string{ - "www.example.com", - }, - } + deps.portChecker.setOpenAll(true) + deps.tlsChecker.setCert(testCert(testHostname, shortLife)) ctx := t.Context() - // First run = baseline, no notifications + // First run fires one expiry warning per live-resolved IP. 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++ - } + afterFirst := countTitled(deps, "TLS Expiry Warning: ") + if afterFirst == 0 { + t.Fatal("expected at least one expiry warning") } - if expiryCount != 1 { + // A second run within the TLS interval must not re-notify. + w.RunOnce(ctx) + + afterSecond := countTitled(deps, "TLS Expiry Warning: ") + if afterSecond != afterFirst { t.Errorf( - "expected exactly 1 expiry warning (dedup), got %d", - expiryCount, + "expected expiry warnings deduplicated at %d, got %d", + afterFirst, afterSecond, ) } } @@ -647,110 +599,71 @@ func TestGracefulShutdown(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{"example.com"} + cfg.Domains = []string{testDomain} cfg.DNSInterval = 100 * time.Millisecond cfg.TLSInterval = 100 * time.Millisecond - w, deps := newTestWatcher(t, cfg) + w, _ := newTestWatcher(t, cfg) - deps.resolver.nsRecords["example.com"] = []string{ - "ns1.example.com.", - } - deps.resolver.allRecords["example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"1.2.3.4"}}, - } - deps.portChecker.results["1.2.3.4:80"] = false - deps.portChecker.results["1.2.3.4:443"] = false - - ctx, cancel := context.WithCancel(t.Context()) - - done := make(chan struct{}) - - go func() { - w.Run(ctx) - close(done) - }() + stop := startWatcher(t, w) + // Cancel while live DNS work is likely still in flight; the + // watcher must still shut down promptly. time.Sleep(250 * time.Millisecond) - cancel() - - select { - case <-done: - // Shut down cleanly - case <-time.After(5 * time.Second): - t.Error("watcher did not shut down within timeout") - } -} - -func setupHostnameIP( - deps *testDeps, - hostname, ip string, -) { - deps.resolver.allRecords[hostname] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {ip}}, - } - deps.portChecker.results[ip+":80"] = true - deps.portChecker.results[ip+":443"] = true - deps.tlsChecker.certs[ip+":"+hostname] = &tlscheck.CertificateInfo{ - CommonName: hostname, - Issuer: "DigiCert", - 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{ - "ns1.example.com.": {"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: "DigiCert", - NotAfter: time.Now().Add(90 * 24 * time.Hour), - SubjectAlternativeNames: []string{hostname}, - } - deps.tlsChecker.mu.Unlock() + stop() } func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Hostnames = []string{"www.example.com"} + cfg.Hostnames = []string{testHostname} w, deps := newTestWatcher(t, cfg) + setupHealthyEndpoints(deps, testHostname) - setupHostnameIP(deps, "www.example.com", "10.0.0.1") + // Seed stale state pointing at an address live DNS cannot + // return. A correct cycle refreshes DNS before the port and + // TLS phases, so the stale address must be dropped, not + // probed. + deps.state.SetHostnameState(testHostname, &state.HostnameState{ + RecordsByNameserver: map[string]*state.NameserverRecordState{ + fakeNS: { + Records: map[string][]string{"A": {fakeIP}}, + Status: "ok", + LastChecked: time.Now().UTC(), + }, + }, + LastChecked: time.Now().UTC(), + }) + deps.state.SetPortState(fakeIP+":80", &state.PortState{ + Open: true, + Hostnames: []string{testHostname}, + LastChecked: time.Now().UTC(), + }) - ctx := t.Context() - w.RunOnce(ctx) + w.RunOnce(t.Context()) 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") + + ips := liveIPs(snap, testHostname) + if len(ips) == 0 { + t.Fatal("expected live-resolved IPs in hostname state") } - // DNS changes to a new IP; port and TLS must pick it up. - updateHostnameIP(deps, "www.example.com", "10.0.0.2") - - 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") + for _, ip := range ips { + if _, ok := snap.Ports[ip+":80"]; !ok { + t.Errorf( + "port check missed fresh DNS address %s", ip, + ) + } } - certKey := "10.0.0.2:443:www.example.com" + if _, ok := snap.Ports[fakeIP+":80"]; ok { + t.Error("port check used stale DNS: " + fakeIP + ":80") + } + + certKey := ips[0] + ":443:" + testHostname if _, ok := snap.Certificates[certKey]; !ok { t.Error("TLS check used stale DNS: missing " + certKey) } @@ -760,19 +673,16 @@ func TestSendTestNotification_Enabled(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{"example.com"} - cfg.Hostnames = []string{"www.example.com"} + cfg.Hostnames = []string{testHostname} cfg.SendTestNotification = true w, deps := newTestWatcher(t, cfg) - setupBaselineMocks(deps) + setupHealthyEndpoints(deps, testHostname) w.RunOnce(t.Context()) // RunOnce does not send the test notification — it is - // sent by Run after RunOnce completes. Call the exported - // RunOnce then check that no test notification was sent - // (only Run triggers it). We test the full path via Run. + // sent by Run after RunOnce completes. notifications := deps.notifier.getNotifications() if len(notifications) != 0 { t.Errorf( @@ -786,84 +696,73 @@ func TestSendTestNotification_ViaRun(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{"example.com"} - cfg.Hostnames = []string{"www.example.com"} + cfg.Hostnames = []string{testHostname} cfg.SendTestNotification = true cfg.DNSInterval = 24 * time.Hour cfg.TLSInterval = 24 * time.Hour w, deps := newTestWatcher(t, cfg) - setupBaselineMocks(deps) + setupHealthyEndpoints(deps, testHostname) - ctx, cancel := context.WithCancel(t.Context()) + stop := startWatcher(t, w) - done := make(chan struct{}) + // The startup notification is sent after the initial live + // scan completes; wait for it rather than assuming a scan + // duration. + found := waitFor(scanTimeout, func() bool { + return countTitled(deps, startupTitle) > 0 + }) - go func() { - w.Run(ctx) - close(done) - }() - - // Wait for the initial scan and test notification. - time.Sleep(500 * time.Millisecond) - cancel() - - <-done - - notifications := deps.notifier.getNotifications() - - found := false - - for _, n := range notifications { - if n.Priority == "success" && - n.Title == "✅ dnswatcher startup complete" { - found = true - } - } + stop() if !found { t.Errorf( "expected startup test notification, got: %v", - notifications, + deps.notifier.getNotifications(), ) } + + for _, n := range deps.notifier.getNotifications() { + if n.Title == startupTitle && n.Priority != "success" { + t.Errorf( + "expected success priority, got %q", n.Priority, + ) + } + } } func TestSendTestNotification_Disabled(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{"example.com"} - cfg.Hostnames = []string{"www.example.com"} + cfg.Hostnames = []string{testHostname} cfg.SendTestNotification = false cfg.DNSInterval = 24 * time.Hour cfg.TLSInterval = 24 * time.Hour w, deps := newTestWatcher(t, cfg) - setupBaselineMocks(deps) + setupHealthyEndpoints(deps, testHostname) - ctx, cancel := context.WithCancel(t.Context()) + stop := startWatcher(t, w) - done := make(chan struct{}) + // Wait until the initial scan has completed (RunOnce saves + // state as its final step), then allow a grace period for + // any notification that would follow it. + scanned := waitFor(scanTimeout, func() bool { + return !deps.state.GetSnapshot().LastUpdated.IsZero() + }) - go func() { - w.Run(ctx) - close(done) - }() + time.Sleep(notifyGrace) + stop() - time.Sleep(500 * time.Millisecond) - cancel() + if !scanned { + t.Fatal("initial live scan did not complete in time") + } - <-done - - notifications := deps.notifier.getNotifications() - - for _, n := range notifications { - if n.Title == "✅ dnswatcher startup complete" { - t.Error( - "test notification should not be sent when disabled", - ) - } + if countTitled(deps, startupTitle) != 0 { + t.Error( + "test notification should not be sent when disabled", + ) } } @@ -871,34 +770,72 @@ func TestNSFailureAndRecovery(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Hostnames = []string{"www.example.com"} + cfg.Hostnames = []string{testHostname} w, deps := newTestWatcher(t, cfg) - deps.resolver.allRecords["www.example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"1.2.3.4"}}, - "ns2.example.com.": {"A": {"1.2.3.4"}}, - } - deps.resolver.ipAddresses["www.example.com"] = []string{ - "1.2.3.4", - } - deps.portChecker.results["1.2.3.4:80"] = false - deps.portChecker.results["1.2.3.4:443"] = false - ctx := t.Context() + w.RunOnce(ctx) + + tamperNSBaseline(t, deps) w.RunOnce(ctx) - deps.resolver.mu.Lock() - deps.resolver.allRecords["www.example.com"] = map[string]map[string][]string{ - "ns1.example.com.": {"A": {"1.2.3.4"}}, - } - deps.resolver.mu.Unlock() - - w.RunOnce(ctx) - - notifications := deps.notifier.getNotifications() - if len(notifications) == 0 { + // fakeNS was recorded as healthy but is absent from live + // results: a disappearance. + if countTitled(deps, "NS Failure: ") == 0 { t.Error("expected notification for NS disappearance") } + + // A real nameserver was recorded as failed but answers in + // live results: a recovery. + if countTitled(deps, "NS Recovery: ") == 0 { + t.Error("expected notification for NS recovery") + } +} + +// tamperNSBaseline rewrites the recorded hostname state so that +// one real (live-observed) nameserver is marked as failed and a +// synthetic nameserver is recorded as healthy. The next live run +// must report the synthetic one as disappeared and the real one +// as recovered. +func tamperNSBaseline(t *testing.T, deps *testDeps) { + t.Helper() + + hs, ok := deps.state.GetHostnameState(testHostname) + if !ok || len(hs.RecordsByNameserver) == 0 { + t.Fatal("expected hostname state after first live run") + } + + tampered := &state.HostnameState{ + RecordsByNameserver: make( + map[string]*state.NameserverRecordState, + ), + LastChecked: hs.LastChecked, + } + + realNS := make([]string, 0, len(hs.RecordsByNameserver)) + + for ns, rec := range hs.RecordsByNameserver { + tampered.RecordsByNameserver[ns] = rec + realNS = append(realNS, ns) + } + + sort.Strings(realNS) + + failed := realNS[0] + tampered.RecordsByNameserver[failed] = &state.NameserverRecordState{ + Records: hs.RecordsByNameserver[failed].Records, + Status: "error", + Error: "synthetic baseline failure", + LastChecked: time.Now().UTC(), + } + + tampered.RecordsByNameserver[fakeNS] = &state.NameserverRecordState{ + Records: map[string][]string{"A": {fakeIP}}, + Status: "ok", + LastChecked: time.Now().UTC(), + } + + deps.state.SetHostnameState(testHostname, tampered) }