From 65e0a4ec3940efd0a9b05ef08964bc3e9a10cd8c Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 28 Sep 2026 23:04:03 +0000 Subject: [PATCH] watcher: send the inconsistency alert once per disagreement (closes #158) detectInconsistencies alerted for every neighbouring pair of nameservers whose records differed, on every DNS check, for as long as they differed. It now also takes the previous hostname state and alerts for a pair only when both nameservers were in that state with equal records. The state loaded at startup is the previous state for the first check, so a disagreement saved before a restart is not reported again. The choice of pairs is in newlyDisagreeingPairs, tested on record data built in the test. The README describes the new behaviour. Model: opus-5-5 --- README.md | 8 +- TODO.md | 3 + internal/watcher/export_test.go | 11 +++ internal/watcher/inconsistency_test.go | 116 +++++++++++++++++++++++++ internal/watcher/watcher.go | 57 ++++++++---- 5 files changed, 178 insertions(+), 17 deletions(-) create mode 100644 internal/watcher/export_test.go create mode 100644 internal/watcher/inconsistency_test.go diff --git a/README.md b/README.md index 8e51883..2e65a07 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. This is sent once, 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..c61166c 100644 --- a/TODO.md +++ b/TODO.md @@ -23,6 +23,9 @@ 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 (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..0bdc9cc --- /dev/null +++ b/internal/watcher/export_test.go @@ -0,0 +1,11 @@ +package watcher + +import "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) +} diff --git a/internal/watcher/inconsistency_test.go b/internal/watcher/inconsistency_test.go new file mode 100644 index 0000000..27f1afb --- /dev/null +++ b/internal/watcher/inconsistency_test.go @@ -0,0 +1,116 @@ +package watcher_test + +import ( + "slices" + "testing" + + "sneak.berlin/go/dnswatcher/internal/state" + "sneak.berlin/go/dnswatcher/internal/watcher" +) + +// 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() + + const ( + nsA = "a.ns.example.net." + nsB = "b.ns.example.net." + ) + + agree := map[string]map[string][]string{ + nsA: {"A": {"192.0.2.1"}}, + nsB: {"A": {"192.0.2.1"}}, + } + disagree := map[string]map[string][]string{ + nsA: {"A": {"192.0.2.1"}}, + nsB: {"A": {"192.0.2.2"}}, + } + alert := [][2]string{{nsA, nsB}} + + // 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: map[string]map[string][]string{ + nsA: {"A": {"192.0.2.1"}}, + }, + 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}, + }, + } + + 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) + } + }) + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index fe2c4fb..67aaa71 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,44 @@ func (w *Watcher) detectInconsistencies( } } +// newlyDisagreeingPairs returns the pairs of nameservers, neighbours in +// sorted order, whose records differ in current but were equal in prev. +// A pair is returned only when both nameservers are in prev, so a +// disagreement that was already in prev is not returned again. +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 := range len(nameservers) - 1 { + ns1 := nameservers[i] + ns2 := 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.