watcher: send the inconsistency alert once per disagreement #161

Merged
clawbot merged 1 commits from issue-158-inconsistency-alert-once into next 2026-09-29 02:29:54 +02:00
5 changed files with 262 additions and 17 deletions
Showing only changes of commit df68bbc615 - Show all commits
+9 -2
View File
@@ -74,8 +74,15 @@ 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 return different record
sets for the same hostname and did not already differ on the previous
check. 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. A
nameserver that was not in the previous check (newly added, or back
after dropping out) and answers differently is reported on the check
where it appears. If a pair agrees again and later disagrees, the
alert is sent again.
### TCP Port Monitoring
+5
View File
@@ -23,6 +23,11 @@ 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 start to disagree or where a nameserver that disagrees first
appears, 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).
+25
View File
@@ -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)
}
+182
View File
@@ -0,0 +1,182 @@
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 the first check and disagreeing alerts once",
loaded: onlyA,
checks: []map[string]map[string][]string{disagree, disagree},
want: [][][2]string{alert, 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()
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}}}
// 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,
},
{
name: "nameserver new on the first check and disagreeing alerts once",
loaded: onlyA,
want: 1,
},
}
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)
}
})
}
}
+41 -15
View File
@@ -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,43 @@ func (w *Watcher) detectInconsistencies(
}
}
// newlyDisagreeingPairs returns every pair of nameservers whose records
// differ in current, in sorted order of name, except pairs where both
// nameservers were in prev and already differed there. A nameserver
// missing from prev is paired with every nameserver it differs from.
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.