diff --git a/README.md b/README.md index b25ef86..0636e9b 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,9 @@ Contributions that introduce mocked, faked, or stubbed DNS will be rejected. - Port transitioned from open to closed (or vice versa). - New IP appeared (from DNS change) and its port state was recorded. - IP disappeared (from DNS change) — noted in the DNS change notification; - port state for that IP is removed. + port state for that IP is removed. When none of a name's nameservers + answered, its addresses are not known, so the port state saved for them is + kept. ### TLS Certificate Monitoring diff --git a/TODO.md b/TODO.md index 10f64bb..dc9e0a3 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-01: when none of a configured name's nameservers answered, the port + state saved for its addresses is kept, not removed (closes #193). - 2026-10-01: `ResolveIPAddresses` returns an error, not no addresses, when no nameserver of the name's zone answered (closes #190). - 2026-10-01: `make fmt` and `make fmt-check` cover Markdown with prettier, run diff --git a/internal/watcher/export_test.go b/internal/watcher/export_test.go index 772253f..557e787 100644 --- a/internal/watcher/export_test.go +++ b/internal/watcher/export_test.go @@ -67,6 +67,11 @@ func (w *Watcher) DetectNSAddressChanges( w.detectNSAddressChanges(ctx, domain, prev, current) } +// CheckAllPorts exports checkAllPorts for testing. +func (w *Watcher) CheckAllPorts(ctx context.Context) { + w.checkAllPorts(ctx) +} + // BuildHostnameState exports buildHostnameState for testing. func BuildHostnameState( results map[string]*resolver.NameserverResponse, diff --git a/internal/watcher/nsfailure_test.go b/internal/watcher/nsfailure_test.go index 91a5580..afae908 100644 --- a/internal/watcher/nsfailure_test.go +++ b/internal/watcher/nsfailure_test.go @@ -331,3 +331,118 @@ func TestNameserverThatRefuses(t *testing.T) { ) } } + +// TestPortStateWhenNoNameserverAnswered runs the port checks on +// hostname state built here, which gives the name no address. The port +// state saved for its old address is kept only when the name is a +// configured hostname or domain and none of its nameservers answered. +func TestPortStateWhenNoNameserverAnswered(t *testing.T) { + t.Parallel() + + noneAnswered := saved(map[string]*state.NameserverRecordState{ + nsA: failed(), nsB: failed(), + }) + oneAnsweredNoAddress := saved(map[string]*state.NameserverRecordState{ + nsA: answered(map[string][]string{}), nsB: failed(), + }) + + configured := []string{host} + + tests := []struct { + name string + hostname *state.HostnameState + hostnames []string + domains []string + wantKept bool + }{ + {"no nameserver answered", noneAnswered, configured, nil, true}, + { + "no nameserver answered, configured as a domain", + noneAnswered, nil, configured, true, + }, + { + "one answered with no address", + oneAnsweredNoAddress, configured, nil, false, + }, + {"no nameserver answered, not configured", noneAnswered, nil, nil, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cfg := defaultTestConfig(t) + cfg.Hostnames = tt.hostnames + cfg.Domains = tt.domains + + // The port checks read the saved hostname state and look + // nothing up, so the watcher has no resolver. + deps := newTestDeps(t, cfg) + w := watcher.NewForTest( + cfg, deps.state, nil, + deps.portChecker, deps.tlsChecker, deps.notifier, + ) + + key := ip1 + ":443" + + deps.state.SetHostnameState(host, tt.hostname) + deps.state.SetPortState(key, &state.PortState{ + Open: true, Hostnames: []string{host}, + }) + + w.CheckAllPorts(t.Context()) + + _, kept := deps.state.GetPortState(key) + if kept != tt.wantKept { + t.Errorf("port state %s kept: %v, want %v", key, kept, tt.wantKept) + } + }) + } +} + +// TestPortStateWhenNoNameserverAnsweredAndOtherNameMovesAway saves the +// port state of an address two configured hostnames resolve to. While +// none of the first one's nameservers answer, the port checks run with +// the other one still at that address, then after it moved away; the +// port state is kept both times. +func TestPortStateWhenNoNameserverAnsweredAndOtherNameMovesAway( + t *testing.T, +) { + t.Parallel() + + const other = "mail.example.net" + + cfg := defaultTestConfig(t) + cfg.Hostnames = []string{host, other} + + // The port checks read the saved hostname state and look nothing + // up, so the watcher has no resolver. + deps := newTestDeps(t, cfg) + w := watcher.NewForTest( + cfg, deps.state, nil, + deps.portChecker, deps.tlsChecker, deps.notifier, + ) + + key := ip1 + ":443" + + deps.state.SetPortState(key, &state.PortState{ + Open: true, Hostnames: []string{host, other}, + }) + deps.state.SetHostnameState(host, saved( + map[string]*state.NameserverRecordState{nsA: failed(), nsB: failed()}, + )) + + for _, otherIP := range []string{ip1, ip2} { + deps.state.SetHostnameState(other, saved( + map[string]*state.NameserverRecordState{ + nsA: answered(map[string][]string{"A": {otherIP}}), + }, + )) + + w.CheckAllPorts(t.Context()) + + if _, kept := deps.state.GetPortState(key); !kept { + t.Fatalf("port state %s removed with %s at %s", key, other, otherIP) + } + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 98f0c68..2df33f4 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log/slog" + "slices" "sort" "strings" "sync" @@ -708,15 +709,46 @@ func parsePortKey(key string) (string, int) { } // cleanupStalePorts removes port state entries that are no -// longer referenced by any hostname in the current DNS data. +// longer referenced by any hostname in the current DNS data. An +// entry saved for a configured name none of whose nameservers +// answered is kept: that name's addresses are not known, not gone. func (w *Watcher) cleanupStalePorts( currentAssociations map[string][]string, ) { for _, key := range w.state.GetAllPortKeys() { - if _, exists := currentAssociations[key]; !exists { - w.state.DeletePortState(key) + if _, exists := currentAssociations[key]; exists { + continue + } + + ps, ok := w.state.GetPortState(key) + if ok && slices.ContainsFunc(ps.Hostnames, w.noNameserverAnswered) { + continue + } + + w.state.DeletePortState(key) + } +} + +// noNameserverAnswered reports whether name is a configured domain or +// hostname and none of its nameservers answered on its last check. +func (w *Watcher) noNameserverAnswered(name string) bool { + if !slices.Contains(w.config.Hostnames, name) && + !slices.Contains(w.config.Domains, name) { + return false + } + + hs, ok := w.state.GetHostnameState(name) + if !ok { + return false + } + + for _, nsState := range hs.RecordsByNameserver { + if nsState.Status == statusOK { + return false } } + + return true } func (w *Watcher) collectIPs(hostname string) []string { @@ -795,9 +827,24 @@ func (w *Watcher) checkSinglePort( ) } + // A configured name on the saved list none of whose nameservers + // answered stays on it, so the entry is kept when the other names + // stop resolving to this address. + savedHostnames := slices.Clone(hostnames) + + if hasPrev { + for _, name := range prev.Hostnames { + if !slices.Contains(hostnames, name) && w.noNameserverAnswered(name) { + savedHostnames = append(savedHostnames, name) + } + } + + sort.Strings(savedHostnames) + } + w.state.SetPortState(key, &state.PortState{ Open: result.Open, - Hostnames: hostnames, + Hostnames: savedHostnames, LastChecked: now, }) }