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,147 @@
+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
+// under .invalid, which live DNS never resolves. The 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,
+	)
+
+	prev := map[string][]string{oldNS1: {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}, prev,
+		)
+
+		return nil
+	})
+
+	if !reflect.DeepEqual(got, prev) {
+		t.Errorf("saved %v, want %v", got, prev)
+	}
+}

internal/watcher/watcher.go

@@ -216,13 +216,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
@@ -290,6 +302,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

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"log/slog"
 	"slices"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -26,11 +27,16 @@ 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 so do the addresses of testSmallDomain's
+// nameservers. 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. 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
@@ -196,8 +202,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,
@@ -221,6 +229,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
 }
 
@@ -314,7 +336,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)
@@ -367,7 +389,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)
 
@@ -406,23 +428,101 @@ 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}
+
+	// Between the checks, save for every nameserver an address live DNS
+	// never returns.
+	deps := runChecks(t, cfg, nil, func(deps *testDeps) {
+		ds, _ := deps.state.GetDomainState(testSmallDomain)
+		for _, ns := range ds.Nameservers {
+			ds.NameserverAddresses[ns] = []string{oldIP}
+		}
+
+		deps.state.SetDomainState(testSmallDomain, ds)
+	})
+
+	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}
+
+	// Between the checks, replace the first nameserver in the saved
+	// state with one live DNS does not list, so that the next check
+	// finds the first nameserver added and that one removed.
+	deps := runChecks(t, cfg, nil, func(deps *testDeps) {
+		ds, _ := deps.state.GetDomainState(testSmallDomain)
+		delete(ds.NameserverAddresses, ds.Nameservers[0])
+		ds.NameserverAddresses[oldNS1] = []string{oldIP}
+		ds.Nameservers = append([]string{oldNS1}, ds.Nameservers[1:]...)
+
+		deps.state.SetDomainState(testSmallDomain, ds)
+	})
+
+	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()