diff --git a/README.md b/README.md index 8e51883..a621f12 100644 --- a/README.md +++ b/README.md @@ -74,8 +74,12 @@ rejected. This is distinct from "responded with no records." - **NS recovery**: A previously-unreachable nameserver starts responding again. - - **Inconsistency detected**: Two nameservers that previously agreed - now return different record sets for the same hostname. + - **Inconsistency detected**: Two nameservers that agreed on the + previous check now return different record sets for the same + hostname. Every pair of nameservers is compared. The alert is sent + once for each such pair, on the check where they start to disagree, + and not again while they keep disagreeing, including after a restart. + If they agree again and later disagree, it is sent again. ### TCP Port Monitoring diff --git a/TODO.md b/TODO.md index 36a1c9a..39eb918 100644 --- a/TODO.md +++ b/TODO.md @@ -23,6 +23,10 @@ Rationale, Design, TODO, License, Author) if any are still missing. # Completed Steps +- 2026-09-28: the inconsistency alert is sent once, on the check where two + nameservers that agreed start to disagree, instead of on every check while + they disagree, and not again after a restart. Every pair of nameservers is + compared, not only neighbours in sorted order of name (closes #158). - 2026-09-28: DNS names in record values (CNAME, MX, SRV and NS targets) are lower-cased, so nameservers that answer in different letter case no longer count as inconsistent or as a record change (closes #157). diff --git a/internal/watcher/export_test.go b/internal/watcher/export_test.go new file mode 100644 index 0000000..04a36d5 --- /dev/null +++ b/internal/watcher/export_test.go @@ -0,0 +1,25 @@ +package watcher + +import ( + "context" + + "sneak.berlin/go/dnswatcher/internal/state" +) + +// NewlyDisagreeingPairs exports newlyDisagreeingPairs for testing. +func NewlyDisagreeingPairs( + prev *state.HostnameState, + current map[string]map[string][]string, +) [][2]string { + return newlyDisagreeingPairs(prev, current) +} + +// DetectHostnameChanges exports detectHostnameChanges for testing. +func (w *Watcher) DetectHostnameChanges( + ctx context.Context, + hostname string, + prev *state.HostnameState, + current map[string]map[string][]string, +) { + w.detectHostnameChanges(ctx, hostname, prev, current) +} diff --git a/internal/watcher/inconsistency_test.go b/internal/watcher/inconsistency_test.go new file mode 100644 index 0000000..e8be173 --- /dev/null +++ b/internal/watcher/inconsistency_test.go @@ -0,0 +1,176 @@ +package watcher_test + +import ( + "slices" + "testing" + + "sneak.berlin/go/dnswatcher/internal/state" + "sneak.berlin/go/dnswatcher/internal/watcher" +) + +const ( + host = "www.example.net" + nsA = "a.ns.example.net." + nsB = "b.ns.example.net." + nsC = "c.ns.example.net." + ip1 = "192.0.2.1" + ip2 = "192.0.2.2" + ip3 = "192.0.2.3" +) + +// hostnameState builds the state a check with these records leaves behind. +func hostnameState( + records map[string]map[string][]string, +) *state.HostnameState { + hs := &state.HostnameState{ + RecordsByNameserver: make(map[string]*state.NameserverRecordState), + } + + for ns, recs := range records { + hs.RecordsByNameserver[ns] = &state.NameserverRecordState{ + Records: recs, + Status: "ok", + } + } + + return hs +} + +func TestNewlyDisagreeingPairs(t *testing.T) { + t.Parallel() + + onlyA := map[string]map[string][]string{nsA: {"A": {ip1}}} + agree := map[string]map[string][]string{nsA: {"A": {ip1}}, nsB: {"A": {ip1}}} + disagree := map[string]map[string][]string{nsA: {"A": {ip1}}, nsB: {"A": {ip2}}} + alert := [][2]string{{nsA, nsB}} + + // b already disagrees with a and c; then c changes, so a and c, + // which agreed, now differ. + bDiffers := map[string]map[string][]string{ + nsA: {"A": {ip1}}, nsB: {"A": {ip2}}, nsC: {"A": {ip1}}, + } + cChanges := map[string]map[string][]string{ + nsA: {"A": {ip1}}, nsB: {"A": {ip2}}, nsC: {"A": {ip3}}, + } + + // Each case starts from the state loaded at startup and runs the + // checks in order; want[i] is what check i alerts for. + tests := []struct { + name string + loaded map[string]map[string][]string + checks []map[string]map[string][]string + want [][][2]string + }{ + { + name: "disagreement persisting across checks alerts once", + loaded: agree, + checks: []map[string]map[string][]string{disagree, disagree, disagree}, + want: [][][2]string{alert, nil, nil}, + }, + { + name: "disagreement starting on a later check alerts on it", + loaded: agree, + checks: []map[string]map[string][]string{agree, agree, disagree}, + want: [][][2]string{nil, nil, alert}, + }, + { + name: "disagreement in the loaded state does not alert", + loaded: disagree, + checks: []map[string]map[string][]string{disagree, disagree}, + want: [][][2]string{nil, nil}, + }, + { + name: "nameserver new on this check does not alert", + loaded: onlyA, + checks: []map[string]map[string][]string{disagree}, + want: [][][2]string{nil}, + }, + { + name: "disagreement after agreeing again alerts again", + loaded: agree, + checks: []map[string]map[string][]string{disagree, agree, disagree}, + want: [][][2]string{alert, nil, alert}, + }, + { + name: "new disagreement while another nameserver differs alerts", + loaded: bDiffers, + checks: []map[string]map[string][]string{cChanges, cChanges}, + want: [][][2]string{{{nsA, nsC}}, nil}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + prev := hostnameState(tt.loaded) + + for i, current := range tt.checks { + got := watcher.NewlyDisagreeingPairs(prev, current) + if !slices.Equal(got, tt.want[i]) { + t.Errorf( + "check %d: alerted for %v, want %v", + i, got, tt.want[i], + ) + } + + prev = hostnameState(current) + } + }) + } +} + +func TestInconsistencyAlert(t *testing.T) { + t.Parallel() + + agree := map[string]map[string][]string{nsA: {"A": {ip1}}, nsB: {"A": {ip1}}} + disagree := map[string]map[string][]string{nsA: {"A": {ip1}}, nsB: {"A": {ip2}}} + + // Each case starts from the state loaded at startup and then sees + // the nameservers disagree on three checks in a row. + tests := []struct { + name string + loaded map[string]map[string][]string + want int + }{ + { + name: "disagreement lasting several checks alerts once", + loaded: agree, + want: 1, + }, + { + name: "disagreement in the loaded state does not alert", + loaded: disagree, + want: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // The hostname change detection uses only the notifier. + notifier := &mockNotifier{} + w := watcher.NewForTest(nil, nil, nil, nil, nil, notifier) + + prev := hostnameState(tt.loaded) + + for range 3 { + w.DetectHostnameChanges(t.Context(), host, prev, disagree) + prev = hostnameState(disagree) + } + + got := 0 + + for _, n := range notifier.getNotifications() { + if n.Title == "Inconsistency: "+host { + got++ + } + } + + if got != tt.want { + t.Errorf("sent %d inconsistency alerts, want %d", got, tt.want) + } + }) + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index fe2c4fb..3b3ba35 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -366,7 +366,7 @@ func (w *Watcher) detectHostnameChanges( ) { w.detectRecordChanges(ctx, hostname, prev, current) w.detectNSDisappearances(ctx, hostname, prev, current) - w.detectInconsistencies(ctx, hostname, current) + w.detectInconsistencies(ctx, hostname, prev, current) } func (w *Watcher) detectRecordChanges( @@ -448,22 +448,11 @@ func (w *Watcher) detectNSDisappearances( func (w *Watcher) detectInconsistencies( ctx context.Context, hostname string, + prev *state.HostnameState, current map[string]map[string][]string, ) { - nameservers := make([]string, 0, len(current)) - for ns := range current { - nameservers = append(nameservers, ns) - } - - sort.Strings(nameservers) - - for i := range len(nameservers) - 1 { - ns1 := nameservers[i] - ns2 := nameservers[i+1] - - if recordsEqual(current[ns1], current[ns2]) { - continue - } + for _, pair := range newlyDisagreeingPairs(prev, current) { + ns1, ns2 := pair[0], pair[1] msg := fmt.Sprintf( "Hostname: %s\n%s: %v\n%s: %v", @@ -481,6 +470,42 @@ func (w *Watcher) detectInconsistencies( } } +// newlyDisagreeingPairs returns every pair of nameservers whose records +// differ in current but were equal in prev, in sorted order of name. +// A pair with a nameserver missing from prev is not returned. +func newlyDisagreeingPairs( + prev *state.HostnameState, + current map[string]map[string][]string, +) [][2]string { + nameservers := make([]string, 0, len(current)) + for ns := range current { + nameservers = append(nameservers, ns) + } + + sort.Strings(nameservers) + + var pairs [][2]string + + for i, ns1 := range nameservers { + for _, ns2 := range nameservers[i+1:] { + if recordsEqual(current[ns1], current[ns2]) { + continue + } + + prev1, ok1 := prev.RecordsByNameserver[ns1] + prev2, ok2 := prev.RecordsByNameserver[ns2] + + if !ok1 || !ok2 || !recordsEqual(prev1.Records, prev2.Records) { + continue + } + + pairs = append(pairs, [2]string{ns1, ns2}) + } + } + + return pairs +} + func (w *Watcher) checkAllPorts(ctx context.Context) { // Phase 1: Build current IP:port → hostname associations // from fresh DNS data.