watcher: notify nameserver address changes (closes #105)

Each domain check now looks up the addresses every nameserver's name
resolves to, with the resolver's ResolveIPAddresses, and saves them
sorted in the domain's state. A nameserver that stays in the
delegation and resolves to different addresses sends one NS Address
Change notification naming the domain, the nameserver and the old and
new addresses. Added or removed nameservers get only the NS change
notification. A failed or empty lookup keeps the previous addresses,
because the resolver returns no address without an error when every
server it asks times out. State files without the field load, and the
next check fills it in silently. Watcher tests that run domain checks
use example.com, which has two nameservers, to stay within the
per-attempt limit.

Model: opus-5-5

internal/watcher/export_test.go

@@ -48,6 +48,25 @@ func (w *Watcher) DetectHostnameChanges(
 	w.detectHostnameChanges(ctx, hostname, prev, current)
 }
 
+// ResolveNameserverAddresses exports resolveNameserverAddresses for
+// testing.
+func (w *Watcher) ResolveNameserverAddresses(
+	ctx context.Context,
+	nameservers []string,
+	prev map[string][]string,
+) map[string][]string {
+	return w.resolveNameserverAddresses(ctx, nameservers, prev)
+}
+
+// DetectNSAddressChanges exports detectNSAddressChanges for testing.
+func (w *Watcher) DetectNSAddressChanges(
+	ctx context.Context,
+	domain string,
+	prev, current map[string][]string,
+) {
+	w.detectNSAddressChanges(ctx, domain, prev, current)
+}
+
 // BuildHostnameState exports buildHostnameState for testing.
 func BuildHostnameState(
 	results map[string]*resolver.NameserverResponse,

internal/watcher/nsaddress_test.go

@@ -0,0 +1,151 @@
+package watcher_test
+
+import (
+	"context"
+	"log/slog"
+	"reflect"
+	"testing"
+
+	"sneak.berlin/go/dnswatcher/internal/livednstest"
+	"sneak.berlin/go/dnswatcher/internal/resolver"
+	"sneak.berlin/go/dnswatcher/internal/watcher"
+)
+
+const domain = "example.net"
+
+func TestNSAddressChangeAlerts(t *testing.T) {
+	t.Parallel()
+
+	// Each case is the nameserver addresses saved by the previous check
+	// and by the current one.
+	tests := []struct {
+		name          string
+		prev, current map[string][]string
+		want          int
+	}{
+		{
+			"same addresses",
+			map[string][]string{nsA: {ip1, ip2}},
+			map[string][]string{nsA: {ip1, ip2}},
+			0,
+		},
+		{
+			"same addresses in another order",
+			map[string][]string{nsA: {ip2, ip1}},
+			map[string][]string{nsA: {ip1, ip2}},
+			0,
+		},
+		{
+			"address replaced",
+			map[string][]string{nsA: {ip1}},
+			map[string][]string{nsA: {ip2}},
+			1,
+		},
+		{
+			"address added",
+			map[string][]string{nsA: {ip1}},
+			map[string][]string{nsA: {ip1, ip2}},
+			1,
+		},
+		{
+			"two nameservers changed",
+			map[string][]string{nsA: {ip1}, nsB: {ip2}},
+			map[string][]string{nsA: {ip3}, nsB: {ip3}},
+			2,
+		},
+		{
+			"nameserver added",
+			map[string][]string{nsA: {ip1}},
+			map[string][]string{nsA: {ip1}, nsB: {ip2}},
+			0,
+		},
+		{
+			"nameserver removed",
+			map[string][]string{nsA: {ip1}, nsB: {ip2}},
+			map[string][]string{nsA: {ip1}},
+			0,
+		},
+		{
+			"state file from before addresses were saved",
+			nil,
+			map[string][]string{nsA: {ip1}, nsB: {ip2}},
+			0,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			notifier := &mockNotifier{}
+			w := watcher.NewForTest(nil, nil, nil, nil, nil, notifier)
+
+			w.DetectNSAddressChanges(t.Context(), domain, tt.prev, tt.current)
+
+			got := len(notifier.getNotifications())
+			if got != tt.want {
+				t.Errorf("sent %d address changes, want %d", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestNSAddressChangeAlertNamesDomainNameserverAndAddresses(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	notifier := &mockNotifier{}
+	w := watcher.NewForTest(nil, nil, nil, nil, nil, notifier)
+
+	w.DetectNSAddressChanges(
+		t.Context(), domain,
+		map[string][]string{nsA: {ip1}},
+		map[string][]string{nsA: {ip2, ip3}},
+	)
+
+	want := notification{
+		Title: "NS Address Change: " + domain,
+		Message: "Domain: " + domain + "\nNameserver: " + nsA +
+			"\nOld: " + ip1 + "\nNew: " + ip2 + ", " + ip3,
+		Priority: "warning",
+	}
+
+	got := notifier.getNotifications()
+	if len(got) != 1 || got[0] != want {
+		t.Errorf("sent %v, want %v", got, want)
+	}
+}
+
+// TestNameserverWithNoAddressKeepsPrevious looks up nameserver names
+// with no address: two under .invalid, whose lookup fails with an
+// error, and one that does not exist under a real zone, which live DNS
+// answers with no address and no error. Each one with addresses saved
+// by the previous check keeps them; the one without gets none.
+func TestNameserverWithNoAddressKeepsPrevious(t *testing.T) {
+	t.Parallel()
+
+	w := watcher.NewForTest(
+		nil, nil, resolver.NewFromLogger(slog.Default()), nil, nil, nil,
+	)
+
+	nonexistentNS := "this-surely-does-not-exist-xyz." + testSmallDomain + "."
+
+	prev := map[string][]string{oldNS1: {oldIP}, nonexistentNS: {oldIP}}
+
+	var got map[string][]string
+
+	// The result is the same whether or not live DNS answers, so the
+	// lookup is not retried.
+	_ = livednstest.Run(func(ctx context.Context) error {
+		got = w.ResolveNameserverAddresses(
+			ctx, []string{oldNS1, oldNS2, nonexistentNS}, prev,
+		)
+
+		return nil
+	})
+
+	if !reflect.DeepEqual(got, prev) {
+		t.Errorf("saved %v, want %v", got, prev)
+	}
+}

internal/watcher/watcher.go

@@ -234,13 +234,25 @@ func (w *Watcher) checkDomain(
 	now := time.Now().UTC()
 
 	prev, hasPrev := w.state.GetDomainState(domain)
+
+	var prevAddresses map[string][]string
+	if hasPrev {
+		prevAddresses = prev.NameserverAddresses
+	}
+
+	addresses := w.resolveNameserverAddresses(
+		ctx, nameservers, prevAddresses,
+	)
+
 	if hasPrev && !w.firstRun {
 		w.detectNSChanges(ctx, domain, prev.Nameservers, nameservers)
+		w.detectNSAddressChanges(ctx, domain, prevAddresses, addresses)
 	}
 
 	w.state.SetDomainState(domain, &state.DomainState{
-		Nameservers: nameservers,
-		LastChecked: now,
+		Nameservers:         nameservers,
+		NameserverAddresses: addresses,
+		LastChecked:         now,
 	})
 
 	// Also look up A/AAAA records for the apex domain so that
@@ -308,6 +320,73 @@ func (w *Watcher) detectNSChanges(
 	)
 }
 
+// resolveNameserverAddresses returns the sorted addresses each
+// nameserver's name resolves to. A nameserver whose lookup fails or
+// finds no address keeps its addresses from prev: the resolver finds no
+// address, without an error, when every server it asks times out, and
+// that is not an address change.
+func (w *Watcher) resolveNameserverAddresses(
+	ctx context.Context,
+	nameservers []string,
+	prev map[string][]string,
+) map[string][]string {
+	addresses := make(map[string][]string, len(nameservers))
+
+	for _, ns := range nameservers {
+		ips, err := w.resolver.ResolveIPAddresses(ctx, ns)
+		if err == nil && len(ips) > 0 {
+			sort.Strings(ips)
+			addresses[ns] = ips
+
+			continue
+		}
+
+		w.log.Error(
+			"no addresses found for nameserver",
+			"nameserver", ns,
+			"error", err,
+		)
+
+		if prevIPs, ok := prev[ns]; ok {
+			addresses[ns] = prevIPs
+		}
+	}
+
+	return addresses
+}
+
+// detectNSAddressChanges notifies when a nameserver in both checks
+// resolves to different addresses. A nameserver added or removed is
+// reported by detectNSChanges alone, and one with no addresses saved by
+// the previous check, as in a state file from before they were saved,
+// is not compared.
+func (w *Watcher) detectNSAddressChanges(
+	ctx context.Context,
+	domain string,
+	prev, current map[string][]string,
+) {
+	for ns, cur := range current {
+		old, ok := prev[ns]
+		if !ok || sliceEqual(old, cur) {
+			continue
+		}
+
+		msg := fmt.Sprintf(
+			"Domain: %s\nNameserver: %s\nOld: %s\nNew: %s",
+			domain, ns,
+			strings.Join(old, ", "),
+			strings.Join(cur, ", "),
+		)
+
+		w.notify.SendNotification(
+			ctx,
+			"NS Address Change: "+domain,
+			msg,
+			"warning",
+		)
+	}
+}
+
 func (w *Watcher) checkHostname(
 	ctx context.Context,
 	hostname string,

internal/watcher/watcher_test.go

@@ -6,6 +6,7 @@ import (
 	"log/slog"
 	"os"
 	"slices"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -27,11 +28,17 @@ import (
 // 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.
+// it twice rely on, and testSmallDomain's nameservers stay the same
+// between a test looking them up and its check. A domain check looks up
+// each nameserver's addresses, about a second per nameserver, so the
+// tests that check a domain use testSmallDomain, which has two
+// nameservers, and check it once. The tests that query testDomain's
+// nameservers directly do no domain check.
 const (
-	testDomain = "google.com"
-	testHost   = "cloudflare.com"
-	testIssuer = "DigiCert"
+	testDomain      = "google.com"
+	testSmallDomain = "example.com"
+	testHost        = "cloudflare.com"
+	testIssuer      = "DigiCert"
 )
 
 // Saved-state values that live DNS never returns: nameserver names
@@ -206,8 +213,10 @@ 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.
+// state holds no address, or a configured domain's nameserver has no
+// address saved or still has oldIP, which the tests save and live DNS
+// never returns. 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,
@@ -231,6 +240,20 @@ func checkOnce(
 		}
 	}
 
+	for _, name := range deps.config.Domains {
+		ds, _ := deps.state.GetDomainState(name)
+		for _, ns := range ds.Nameservers {
+			ips := ds.NameserverAddresses[ns]
+			if len(ips) == 0 || slices.Contains(ips, oldIP) {
+				return fmt.Errorf(
+					"%s: nameserver %s: %w, or the watcher saved "+
+						"no fresh addresses for it",
+					name, ns, livednstest.ErrNoAnswer,
+				)
+			}
+		}
+	}
+
 	return nil
 }
 
@@ -272,6 +295,26 @@ func runChecks(
 	return deps
 }
 
+// lookupNameservers returns the nameservers live DNS lists for domain,
+// for a test to save in the state its check starts from.
+func lookupNameservers(t *testing.T, domain string) []string {
+	t.Helper()
+
+	res := resolver.NewFromLogger(slog.Default())
+
+	var nameservers []string
+
+	livednstest.Retry(t, "LookupNS("+domain+")", func(ctx context.Context) error {
+		var err error
+
+		nameservers, err = res.LookupNS(ctx, domain)
+
+		return err
+	})
+
+	return nameservers
+}
+
 // addresses returns the A and AAAA values saved for a hostname.
 func addresses(hs *state.HostnameState) []string {
 	var ips []string
@@ -324,7 +367,7 @@ func TestFirstRunBaseline(t *testing.T) {
 	t.Parallel()
 
 	cfg := defaultTestConfig(t)
-	cfg.Domains = []string{testDomain}
+	cfg.Domains = []string{testSmallDomain}
 	cfg.Hostnames = []string{testHost}
 
 	deps := runChecks(t, cfg, nil, nil)
@@ -377,7 +420,7 @@ func TestDomainPortAndTLSChecks(t *testing.T) {
 	t.Parallel()
 
 	cfg := defaultTestConfig(t)
-	cfg.Domains = []string{testDomain}
+	cfg.Domains = []string{testSmallDomain}
 
 	deps := runChecks(t, cfg, nil, nil)
 
@@ -416,23 +459,106 @@ func TestNSChangeDetection(t *testing.T) {
 	t.Parallel()
 
 	cfg := defaultTestConfig(t)
-	cfg.Domains = []string{testDomain}
+	cfg.Domains = []string{testSmallDomain}
 
 	// The saved state lists nameservers that live DNS does not.
 	deps := runChecks(t, cfg, func(deps *testDeps) {
-		deps.state.SetDomainState(testDomain, &state.DomainState{
+		deps.state.SetDomainState(testSmallDomain, &state.DomainState{
 			Nameservers: []string{oldNS1, oldNS2},
 		})
 	}, nil)
 
-	assertNotified(t, deps, "NS Change: "+testDomain, "warning")
+	assertNotified(t, deps, "NS Change: "+testSmallDomain, "warning")
 
-	ds, _ := deps.state.GetDomainState(testDomain)
+	ds, _ := deps.state.GetDomainState(testSmallDomain)
 	if slices.Contains(ds.Nameservers, oldNS1) {
 		t.Errorf("saved nameservers not updated: %v", ds.Nameservers)
 	}
 }
 
+func TestNSAddressChangeDetection(t *testing.T) {
+	t.Parallel()
+
+	cfg := defaultTestConfig(t)
+	cfg.Domains = []string{testSmallDomain}
+
+	nameservers := lookupNameservers(t, testSmallDomain)
+
+	// The saved state lists the nameservers live DNS lists, each at an
+	// address live DNS never returns.
+	deps := runChecks(t, cfg, func(deps *testDeps) {
+		nsAddresses := make(map[string][]string, len(nameservers))
+		for _, ns := range nameservers {
+			nsAddresses[ns] = []string{oldIP}
+		}
+
+		deps.state.SetDomainState(testSmallDomain, &state.DomainState{
+			Nameservers:         nameservers,
+			NameserverAddresses: nsAddresses,
+		})
+	}, nil)
+
+	title := "NS Address Change: " + testSmallDomain
+	ds, _ := deps.state.GetDomainState(testSmallDomain)
+
+	// One alert per nameserver, naming it and the address it had.
+	for _, ns := range ds.Nameservers {
+		prefix := "Domain: " + testSmallDomain + "\nNameserver: " + ns +
+			"\nOld: " + oldIP + "\nNew: "
+
+		sent := 0
+
+		for _, n := range deps.notifier.getNotifications() {
+			if n.Title == title && strings.HasPrefix(n.Message, prefix) {
+				sent++
+			}
+		}
+
+		if sent != 1 {
+			t.Errorf("sent %d address changes for %s, want 1", sent, ns)
+		}
+	}
+
+	if n := countNotifications(deps, title); n != len(ds.Nameservers) {
+		t.Errorf(
+			"sent %d address changes for %d nameservers",
+			n, len(ds.Nameservers),
+		)
+	}
+
+	if n := countNotifications(deps, "NS Change: "+testSmallDomain); n != 0 {
+		t.Errorf("sent %d NS changes, want 0", n)
+	}
+}
+
+func TestNSAddedAndRemovedIsNoAddressChange(t *testing.T) {
+	t.Parallel()
+
+	cfg := defaultTestConfig(t)
+	cfg.Domains = []string{testSmallDomain}
+
+	nameservers := lookupNameservers(t, testSmallDomain)
+
+	// The saved state lists oldNS1, which live DNS does not, in place of
+	// the first nameserver live DNS lists, so that the check finds that
+	// one added and oldNS1 removed. Only oldNS1 has addresses saved.
+	deps := runChecks(t, cfg, func(deps *testDeps) {
+		deps.state.SetDomainState(testSmallDomain, &state.DomainState{
+			Nameservers:         append([]string{oldNS1}, nameservers[1:]...),
+			NameserverAddresses: map[string][]string{oldNS1: {oldIP}},
+		})
+	}, nil)
+
+	if n := countNotifications(deps, "NS Change: "+testSmallDomain); n != 1 {
+		t.Errorf("sent %d NS changes, want 1", n)
+	}
+
+	title := "NS Address Change: " + testSmallDomain
+	if n := countNotifications(deps, title); n != 0 {
+		t.Errorf("sent %d address changes, want 0", n)
+	}
+}
+
 func TestRecordChangeDetection(t *testing.T) {
 	t.Parallel()