resolver: store a name's CNAME once per nameserver (closes #220) #236

Merged
clawbot merged 1 commits from issue-220-cname-stored-once into next 2026-10-02 09:09:52 +02:00
7 changed files with 173 additions and 3 deletions
Showing only changes of commit d60720816b - Show all commits
+2
View File
@@ -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 DNS lookup that shutdown cuts short logs no error; one that
fails otherwise, or runs out of time, still does (closes #229).
- 2026-10-02: Record Change and Inconsistency notifications list only the record
+7
View File
@@ -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)
+10 -3
View File
@@ -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
}
}
+32
View File
@@ -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
+14
View File
@@ -8,6 +8,7 @@ import (
"log/slog"
"os"
"path/filepath"
"slices"
"sync"
"time"
@@ -201,6 +202,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)
+53
View File
@@ -277,6 +277,59 @@ func TestLoadStateFromBeforeCNAMEAddresses(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, and every different value must load.
func TestLoadStateWithRepeatedValues(t *testing.T) {
t.Parallel()
dir := t.TempDir()
data := []byte(`{
"version": 1,
"hostnames": {
"www.example.com": {
"recordsByNameserver": {
"ns1.example.com.": {
"records": {
"A": ["192.0.2.2", "192.0.2.1", "192.0.2.2", "192.0.2.1"],
"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)
}
want := map[string][]string{
"A": {"192.0.2.1", "192.0.2.2"},
"CNAME": {"a.example.net."},
}
got := hs.RecordsByNameserver[testNS1].Records
if !reflect.DeepEqual(got, want) {
t.Errorf("records: got %v, want %v", got, want)
}
}
// TestSaveLoadRoundTrip_Hostnames verifies hostname data survives a save/load cycle.
func TestSaveLoadRoundTrip_Hostnames(t *testing.T) {
t.Parallel()
+55
View File
@@ -183,3 +183,58 @@ 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 two addresses each repeated,
// and loads it. A check that then finds each value once at each
// nameserver must notify nothing.
func TestFirstCheckAfterRepeatedValuesLoaded(t *testing.T) {
t.Parallel()
const (
cnameType = "CNAME"
cname = "c.example.net."
)
cfg := defaultTestConfig(t)
repeated := map[string][]string{
"A": {ip2, ip1, ip2, ip1},
cnameType: {cname, cname, cname, cname, cname, cname, cname, cname},
}
once := map[string][]string{"A": {ip1, ip2}, cnameType: {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)
}
}