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) }