diff --git a/TODO.md b/TODO.md index 927a598..f096568 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-02: a name's CNAME is stored once per nameserver, not once per record + type asked for; a state file with repeats loads each value once (closes #220). - 2026-10-02: a resolver test that reads one record type from a nameserver's answer asks again when that type is missing from it (closes #218). - 2026-10-02: a plain `docker build .` of a clone stamps its tag or short diff --git a/internal/resolver/export_test.go b/internal/resolver/export_test.go index 17c57d7..58b88b1 100644 --- a/internal/resolver/export_test.go +++ b/internal/resolver/export_test.go @@ -11,6 +11,13 @@ func ExtractRecordValue(rr dns.RR) string { return extractRecordValue(rr) } +// CollectAnswerRecords exports collectAnswerRecords for testing. +func CollectAnswerRecords(msg *dns.Msg, resp *NameserverResponse) { + var state queryState + + collectAnswerRecords(msg, resp, &state) +} + // UsableReply exports usableReply for testing. func UsableReply(resp *dns.Msg, zone string, name string) bool { return usableReply(resp, zone, name) diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index 1f06a24..6ffedc5 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -714,6 +714,10 @@ func (r *Resolver) querySingleType( collectAnswerRecords(msg, resp, state) } +// collectAnswerRecords adds the records in msg's answer to resp, each +// value once per record type. For a name with a CNAME, a nameserver +// answers a query of any type with that CNAME, so the same value comes +// in the answer to every type asked for. func collectAnswerRecords( msg *dns.Msg, resp *NameserverResponse, @@ -726,9 +730,12 @@ func collectAnswerRecords( } typeName := dns.TypeToString[rr.Header().Rrtype] - resp.Records[typeName] = append( - resp.Records[typeName], val, - ) + if !slices.Contains(resp.Records[typeName], val) { + resp.Records[typeName] = append( + resp.Records[typeName], val, + ) + } + state.hasRecords = true } } diff --git a/internal/resolver/iterative_test.go b/internal/resolver/iterative_test.go index 4f539ae..ca422cd 100644 --- a/internal/resolver/iterative_test.go +++ b/internal/resolver/iterative_test.go @@ -238,6 +238,38 @@ func TestExtractRecordValue_LetterCase(t *testing.T) { } } +// TestCollectAnswerRecords_CNAMEOnce collects the answers a nameserver +// gives for a name with a CNAME, one for each record type a check asks +// for. Each answer holds the CNAME, which must be stored once. +func TestCollectAnswerRecords_CNAMEOnce(t *testing.T) { + t.Parallel() + + cname := &dns.CNAME{ + Hdr: dns.RR_Header{ + Name: "git.eeqj.de.", Rrtype: dns.TypeCNAME, Class: dns.ClassINET, + }, + Target: "fsn1app1.datavi.be.", + } + + resp := &resolver.NameserverResponse{Records: map[string][]string{}} + + for _, qtype := range []uint16{ + dns.TypeA, dns.TypeAAAA, dns.TypeCNAME, dns.TypeMX, + dns.TypeTXT, dns.TypeSRV, dns.TypeCAA, dns.TypeNS, + } { + msg := new(dns.Msg) + msg.SetQuestion("git.eeqj.de.", qtype) + msg.Answer = []dns.RR{cname} + + resolver.CollectAnswerRecords(msg, resp) + } + + assert.Equal(t, + map[string][]string{"CNAME": {"fsn1app1.datavi.be."}}, + resp.Records, + ) +} + // TestShuffled shuffles the root servers with many seeds. Every order // must hold each root server once, so each is tried before a // resolution fails; each root server must come first for some seed, so diff --git a/internal/state/state.go b/internal/state/state.go index a8af3f9..3f5bb7c 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -8,6 +8,7 @@ import ( "log/slog" "os" "path/filepath" + "slices" "sync" "time" @@ -196,6 +197,19 @@ func (s *State) Load() error { return fmt.Errorf("parsing state file: %w", err) } + // A state file saved before each record value was stored once can + // hold a hostname's CNAME once for every record type asked for. + // Each value is kept once, so the first check does not see a + // record change. + for _, hs := range snapshot.Hostnames { + for _, ns := range hs.RecordsByNameserver { + for recordType, values := range ns.Records { + slices.Sort(values) + ns.Records[recordType] = slices.Compact(values) + } + } + } + s.snapshot = &snapshot s.log.Info("loaded state from disk", "path", path) diff --git a/internal/state/state_test.go b/internal/state/state_test.go index 16f709d..ea8f180 100644 --- a/internal/state/state_test.go +++ b/internal/state/state_test.go @@ -188,6 +188,53 @@ func TestLoadStateFromBeforeNameserverAddresses(t *testing.T) { } } +// TestLoadStateWithRepeatedValues loads a state file saved when a +// hostname's CNAME was stored once for every record type asked for. +// Each value must load once. +func TestLoadStateWithRepeatedValues(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + data := []byte(`{ + "version": 1, + "hostnames": { + "www.example.com": { + "recordsByNameserver": { + "ns1.example.com.": { + "records": { + "CNAME": ["a.example.net.", "a.example.net.", "a.example.net."] + }, + "status": "ok" + } + } + } + } + }`) + + err := os.WriteFile(filepath.Join(dir, "state.json"), data, 0o600) + if err != nil { + t.Fatalf("writing state file: %v", err) + } + + s := state.NewForTestWithDataDir(dir) + + err = s.Load() + if err != nil { + t.Fatalf("Load() error: %v", err) + } + + hs, ok := s.GetHostnameState(testHostname) + if !ok { + t.Fatal("missing hostname " + testHostname) + } + + got := hs.RecordsByNameserver[testNS1].Records["CNAME"] + if !reflect.DeepEqual(got, []string{"a.example.net."}) { + t.Errorf("CNAME: got %v, want it once", got) + } +} + // TestSaveLoadRoundTrip_Hostnames verifies hostname data survives a save/load cycle. func TestSaveLoadRoundTrip_Hostnames(t *testing.T) { t.Parallel() diff --git a/internal/watcher/inconsistency_test.go b/internal/watcher/inconsistency_test.go index 494ec2b..a2f34db 100644 --- a/internal/watcher/inconsistency_test.go +++ b/internal/watcher/inconsistency_test.go @@ -183,3 +183,53 @@ func TestInconsistencyAlert(t *testing.T) { }) } } + +// TestFirstCheckAfterRepeatedValuesLoaded saves a state file holding a +// hostname's CNAME once for every record type asked for, as checks did +// before each value was stored once, and loads it. A check that then +// finds the CNAME once at each nameserver must notify nothing. +func TestFirstCheckAfterRepeatedValuesLoaded(t *testing.T) { + t.Parallel() + + const cname = "c.example.net." + + cfg := defaultTestConfig(t) + repeated := map[string][]string{ + "CNAME": {cname, cname, cname, cname, cname, cname, cname, cname}, + } + once := map[string][]string{"CNAME": {cname}} + + saved := newTestDeps(t, cfg).state + saved.SetHostnameState(host, hostnameState(map[string]map[string][]string{ + nsA: repeated, nsB: repeated, + })) + + err := saved.Save() + if err != nil { + t.Fatalf("saving the state file: %v", err) + } + + deps := newTestDeps(t, cfg) + + err = deps.state.Load() + if err != nil { + t.Fatalf("loading the state file: %v", err) + } + + prev, ok := deps.state.GetHostnameState(host) + if !ok { + t.Fatal("the state file has no state for " + host) + } + + current := hostnameState(map[string]map[string][]string{ + nsA: once, nsB: once, + }) + + // The hostname change detection uses only the notifier. + w := watcher.NewForTest(nil, nil, nil, nil, nil, deps.notifier) + w.DetectHostnameChanges(t.Context(), host, prev, current) + + if got := deps.notifier.getNotifications(); len(got) != 0 { + t.Errorf("sent %v, want no notification", got) + } +}