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: watcher tests send far fewer queries and a live attempt may take
+  18s; nameserver addresses are asked only for A, AAAA, CNAME (closes #214).
 - 2026-10-02: the resolver tries root servers, and every other server list it
   walks, in a random order each time, not always from the top (closes #138).
 - 2026-10-02: a name listed more than once in `DNSWATCHER_TARGETS`, in any

internal/livednstest/livednstest.go

@@ -36,11 +36,18 @@ const (
 	// before the test fails.
 	attempts = 3
 
-	// AttemptTimeout bounds one attempt. Worst case for an operation
-	// is attempts * AttemptTimeout plus the backoff — about 26
-	// seconds, well inside the 90-second `go test -timeout` backstop
-	// even when several operations exhaust their attempts.
-	AttemptTimeout = 8 * time.Second
+	// AttemptTimeout bounds one attempt. It must fit the longest
+	// operation, a watcher check, which sends over a hundred queries one
+	// after another and on a slow build host takes several times as long
+	// as the few seconds it takes on a fast one. An operation whose
+	// every attempt fails takes attempts * AttemptTimeout plus the
+	// backoff, about 56 seconds, after it waits for one of the
+	// Concurrency slots that every live operation in the test binary
+	// shares. So when live DNS does not answer at all, a test binary
+	// with more live operations than slots runs into the 90-second
+	// `go test -timeout` backstop instead of each test failing on its
+	// own.
+	AttemptTimeout = 18 * time.Second
 
 	// backoffBase is the delay after the first failed attempt; it is
 	// multiplied by backoffFactor each time.

internal/resolver/export_test.go

@@ -34,7 +34,7 @@ func (r *Resolver) QueryEachNS(
 	nameservers []string,
 	hostname string,
 ) (map[string]*NameserverResponse, error) {
-	return r.queryEachNS(ctx, nameservers, hostname)
+	return r.queryEachNS(ctx, nameservers, hostname, recordTypes())
 }
 
 // ResolveNSIPs exports resolveNSIPs for testing.

internal/resolver/iterative.go

@@ -533,12 +533,39 @@ func (r *Resolver) FindAuthoritativeNameservers(
 	return nil, ErrNoNameservers
 }
 
+// recordTypes returns the record types a nameserver is asked for when a
+// name is checked.
+func recordTypes() []uint16 {
+	return []uint16{
+		dns.TypeA, dns.TypeAAAA, dns.TypeCNAME,
+		dns.TypeMX, dns.TypeTXT, dns.TypeSRV,
+		dns.TypeCAA, dns.TypeNS,
+	}
+}
+
+// addressTypes returns the record types ResolveIPAddresses asks for,
+// the only ones it reads.
+func addressTypes() []uint16 {
+	return []uint16{dns.TypeA, dns.TypeAAAA, dns.TypeCNAME}
+}
+
 // QueryNameserver queries a specific nameserver for all record
 // types and builds a NameserverResponse.
 func (r *Resolver) QueryNameserver(
 	ctx context.Context,
 	nsHostname string,
 	hostname string,
+) (*NameserverResponse, error) {
+	return r.queryNameserver(ctx, nsHostname, hostname, recordTypes())
+}
+
+// queryNameserver queries a specific nameserver for the record types
+// in qtypes and builds a NameserverResponse.
+func (r *Resolver) queryNameserver(
+	ctx context.Context,
+	nsHostname string,
+	hostname string,
+	qtypes []uint16,
 ) (*NameserverResponse, error) {
 	if checkCtx(ctx) != nil {
 		return nil, ErrContextCanceled
@@ -551,7 +578,7 @@ func (r *Resolver) QueryNameserver(
 
 	hostname = dns.Fqdn(hostname)
 
-	return r.queryAllTypes(ctx, nsHostname, nsIPs[0], hostname)
+	return r.queryTypes(ctx, nsHostname, nsIPs[0], hostname, qtypes)
 }
 
 // QueryNameserverIP queries a nameserver by its IP address directly,
@@ -568,14 +595,15 @@ func (r *Resolver) QueryNameserverIP(
 
 	hostname = dns.Fqdn(hostname)
 
-	return r.queryAllTypes(ctx, nsHostname, nsIP, hostname)
+	return r.queryTypes(ctx, nsHostname, nsIP, hostname, recordTypes())
 }
 
-func (r *Resolver) queryAllTypes(
+func (r *Resolver) queryTypes(
 	ctx context.Context,
 	nsHostname string,
 	nsIP string,
 	hostname string,
+	qtypes []uint16,
 ) (*NameserverResponse, error) {
 	resp := &NameserverResponse{
 		Nameserver: nsHostname,
@@ -583,12 +611,6 @@ func (r *Resolver) queryAllTypes(
 		Status:     StatusOK,
 	}
 
-	qtypes := []uint16{
-		dns.TypeA, dns.TypeAAAA, dns.TypeCNAME,
-		dns.TypeMX, dns.TypeTXT, dns.TypeSRV,
-		dns.TypeCAA, dns.TypeNS,
-	}
-
 	state := r.queryEachType(ctx, nsIP, hostname, qtypes, resp)
 	classifyResponse(resp, state)
 
@@ -779,18 +801,19 @@ func (r *Resolver) QueryAllNameservers(
 		return nil, err
 	}
 
-	return r.queryEachNS(ctx, nameservers, hostname)
+	return r.queryEachNS(ctx, nameservers, hostname, recordTypes())
 }
 
 func (r *Resolver) queryEachNS(
 	ctx context.Context,
 	nameservers []string,
 	hostname string,
+	qtypes []uint16,
 ) (map[string]*NameserverResponse, error) {
 	results := make(map[string]*NameserverResponse)
 
 	for _, ns := range nameservers {
-		resp, err := r.QueryNameserver(ctx, ns, hostname)
+		resp, err := r.queryNameserver(ctx, ns, hostname, qtypes)
 
 		// A query the context cut short says nothing about the
 		// nameserver, so it must not be returned as its failure.
@@ -835,9 +858,10 @@ func (r *Resolver) LookupAllRecords(
 }
 
 // ResolveIPAddresses resolves a hostname to all IPv4 and IPv6
-// addresses, following CNAME chains up to MaxCNAMEDepth. When no
-// nameserver of the name's zone answered, it returns an error rather
-// than no addresses.
+// addresses, following CNAME chains up to MaxCNAMEDepth. It asks each
+// nameserver of the name's zone for its A, AAAA and CNAME records only.
+// When no nameserver of the name's zone answered, it returns an error
+// rather than no addresses.
 func (r *Resolver) ResolveIPAddresses(
 	ctx context.Context,
 	hostname string,
@@ -858,7 +882,12 @@ func (r *Resolver) resolveIPWithCNAME(
 		return nil, ErrCNAMEDepthExceeded
 	}
 
-	results, err := r.QueryAllNameservers(ctx, hostname)
+	nameservers, err := r.FindAuthoritativeNameservers(ctx, hostname)
+	if err != nil {
+		return nil, err
+	}
+
+	results, err := r.queryEachNS(ctx, nameservers, hostname, addressTypes())
 	if err != nil {
 		return nil, err
 	}

internal/resolver/resolver_test.go

@@ -562,6 +562,34 @@ func TestResolveIPAddresses_CloudflareDomain(t *testing.T) {
 	assert.NotEmpty(t, ips)
 }
 
+// TestResolveIPAddresses_NameserverIPv4AndIPv6 looks up the addresses of
+// one of cloudflare.com's nameservers, as a domain check does for each
+// nameserver. That name has A and AAAA records, so both kinds of address
+// come back.
+func TestResolveIPAddresses_NameserverIPv4AndIPv6(t *testing.T) {
+	t.Parallel()
+
+	r := newTestResolver(t)
+	ns := findOneNSForDomain(t, r, "cloudflare.com")
+	ips := liveResolveIPs(t, r, ns)
+
+	var ipv4, ipv6 int
+
+	for _, ip := range ips {
+		parsed := net.ParseIP(ip)
+		require.NotNil(t, parsed, "should be valid IP: %s", ip)
+
+		if parsed.To4() != nil {
+			ipv4++
+		} else {
+			ipv6++
+		}
+	}
+
+	assert.Positive(t, ipv4, "no IPv4 address for %s: %v", ns, ips)
+	assert.Positive(t, ipv6, "no IPv6 address for %s: %v", ns, ips)
+}
+
 // ----------------------------------------------------------------
 // Context cancellation tests
 // ----------------------------------------------------------------

internal/watcher/watcher_test.go

@@ -26,18 +26,22 @@ import (
 
 // The watcher looks these names up in live DNS with the real resolver,
 // so tests assert on what the watcher does with the answers, never on
-// the records these zones publish. testHost's nameservers and addresses
-// stay the same from one check to the next, which the tests that check
-// it twice rely on, and testSmallDomain's nameservers stay the same
-// between a test looking them up and its check. A domain check looks up
-// each nameserver's addresses, about a second per nameserver, so the
-// tests that check a domain use testSmallDomain, which has two
-// nameservers, and check it once. The tests that query testDomain's
-// nameservers directly do no domain check.
+// the records these zones publish. The nameservers of testHost and
+// testSmallDomain stay the same between a test looking them up and its
+// check. Every query a check sends is one more that can be lost, so the
+// tests keep them few. A check asks each of a name's nameservers about
+// every record type, and both names have two. A domain check also looks
+// up each nameserver's addresses at every nameserver of the zone that
+// nameserver is in: testSmallDomain's nameservers are in zones with two
+// nameservers, while a domain whose nameservers are in, say,
+// cloudflare.com, which has five, makes each domain check much longer.
+// A test checks a domain only when it is about domains, and checks once,
+// from saved state it builds, rather than twice. The tests that query
+// testDomain's nameservers directly do no domain check.
 const (
 	testDomain      = "google.com"
-	testSmallDomain = "example.com"
-	testHost        = "cloudflare.com"
+	testSmallDomain = "desec.io"
+	testHost        = "example.org"
 	testIssuer      = "DigiCert"
 )
 
@@ -259,55 +263,48 @@ func checkOnce(
 
 // runChecks builds a watcher, lets prepare set up the saved state and
 // stand-ins it starts from, and runs its checks once against live DNS.
-// If change is not nil, change then alters the saved state or stand-ins
-// and the checks run a second time. When either check finds no fresh
-// address for a name (see checkOnce), the watcher is thrown away and
-// all of this runs again on a new one, so a failed attempt leaves
-// nothing behind in the saved state, the stand-ins or the notifications.
+// When the check finds no fresh address for a name (see checkOnce), the
+// watcher is thrown away and all of this runs again on a new one, so a
+// failed attempt leaves nothing behind in the saved state, the
+// stand-ins or the notifications.
 func runChecks(
 	t *testing.T,
 	cfg *config.Config,
-	prepare, change func(deps *testDeps),
-) *testDeps {
+	prepare func(deps *testDeps),
+) (*watcher.Watcher, *testDeps) {
 	t.Helper()
 
-	var deps *testDeps
+	var (
+		w    *watcher.Watcher
+		deps *testDeps
+	)
 
 	livednstest.Retry(t, "watcher checks", func(ctx context.Context) error {
-		var w *watcher.Watcher
-
 		w, deps = newTestWatcher(t, cfg)
 
 		if prepare != nil {
 			prepare(deps)
 		}
 
-		err := checkOnce(ctx, w, deps)
-		if err != nil || change == nil {
-			return err
-		}
-
-		change(deps)
-
 		return checkOnce(ctx, w, deps)
 	})
 
-	return deps
+	return w, deps
 }
 
-// lookupNameservers returns the nameservers live DNS lists for domain,
+// lookupNameservers returns the nameservers live DNS lists for name,
 // for a test to save in the state its check starts from.
-func lookupNameservers(t *testing.T, domain string) []string {
+func lookupNameservers(t *testing.T, name string) []string {
 	t.Helper()
 
 	res := resolver.NewFromLogger(slog.Default())
 
 	var nameservers []string
 
-	livednstest.Retry(t, "LookupNS("+domain+")", func(ctx context.Context) error {
+	livednstest.Retry(t, "LookupNS("+name+")", func(ctx context.Context) error {
 		var err error
 
-		nameservers, err = res.LookupNS(ctx, domain)
+		nameservers, err = res.LookupNS(ctx, name)
 
 		return err
 	})
@@ -370,7 +367,7 @@ func TestFirstRunBaseline(t *testing.T) {
 	cfg.Domains = []string{testSmallDomain}
 	cfg.Hostnames = []string{testHost}
 
-	deps := runChecks(t, cfg, nil, nil)
+	_, deps := runChecks(t, cfg, nil)
 
 	assertNoNotifications(t, deps)
 	assertStatePopulated(t, deps)
@@ -422,7 +419,7 @@ func TestDomainPortAndTLSChecks(t *testing.T) {
 	cfg := defaultTestConfig(t)
 	cfg.Domains = []string{testSmallDomain}
 
-	deps := runChecks(t, cfg, nil, nil)
+	_, deps := runChecks(t, cfg, nil)
 
 	snap := deps.state.GetSnapshot()
 
@@ -462,11 +459,11 @@ func TestNSChangeDetection(t *testing.T) {
 	cfg.Domains = []string{testSmallDomain}
 
 	// The saved state lists nameservers that live DNS does not.
-	deps := runChecks(t, cfg, func(deps *testDeps) {
+	_, deps := runChecks(t, cfg, func(deps *testDeps) {
 		deps.state.SetDomainState(testSmallDomain, &state.DomainState{
 			Nameservers: []string{oldNS1, oldNS2},
 		})
-	}, nil)
+	})
 
 	assertNotified(t, deps, "NS Change: "+testSmallDomain, "warning")
 
@@ -486,7 +483,7 @@ func TestNSAddressChangeDetection(t *testing.T) {
 
 	// The saved state lists the nameservers live DNS lists, each at an
 	// address live DNS never returns.
-	deps := runChecks(t, cfg, func(deps *testDeps) {
+	_, deps := runChecks(t, cfg, func(deps *testDeps) {
 		nsAddresses := make(map[string][]string, len(nameservers))
 		for _, ns := range nameservers {
 			nsAddresses[ns] = []string{oldIP}
@@ -496,7 +493,7 @@ func TestNSAddressChangeDetection(t *testing.T) {
 			Nameservers:         nameservers,
 			NameserverAddresses: nsAddresses,
 		})
-	}, nil)
+	})
 
 	title := "NS Address Change: " + testSmallDomain
 	ds, _ := deps.state.GetDomainState(testSmallDomain)
@@ -542,12 +539,12 @@ func TestNSAddedAndRemovedIsNoAddressChange(t *testing.T) {
 	// The saved state lists oldNS1, which live DNS does not, in place of
 	// the first nameserver live DNS lists, so that the check finds that
 	// one added and oldNS1 removed. Only oldNS1 has addresses saved.
-	deps := runChecks(t, cfg, func(deps *testDeps) {
+	_, deps := runChecks(t, cfg, func(deps *testDeps) {
 		deps.state.SetDomainState(testSmallDomain, &state.DomainState{
 			Nameservers:         append([]string{oldNS1}, nameservers[1:]...),
 			NameserverAddresses: map[string][]string{oldNS1: {oldIP}},
 		})
-	}, nil)
+	})
 
 	if n := countNotifications(deps, "NS Change: "+testSmallDomain); n != 1 {
 		t.Errorf("sent %d NS changes, want 1", n)
@@ -565,15 +562,17 @@ func TestRecordChangeDetection(t *testing.T) {
 	cfg := defaultTestConfig(t)
 	cfg.Hostnames = []string{testHost}
 
-	// Between the checks, save for every nameserver an address live DNS
-	// never returns.
-	deps := runChecks(t, cfg, nil, func(deps *testDeps) {
-		hs, _ := deps.state.GetHostnameState(testHost)
-		for _, nsState := range hs.RecordsByNameserver {
-			nsState.Records = map[string][]string{"A": {oldIP}}
+	nameservers := lookupNameservers(t, testHost)
+
+	// The saved state has every nameserver live DNS lists answering
+	// with an address live DNS never returns.
+	_, deps := runChecks(t, cfg, func(deps *testDeps) {
+		byNameserver := make(map[string]*state.NameserverRecordState)
+		for _, ns := range nameservers {
+			byNameserver[ns] = answered(map[string][]string{"A": {oldIP}})
 		}
 
-		deps.state.SetHostnameState(testHost, hs)
+		deps.state.SetHostnameState(testHost, saved(byNameserver))
 	})
 
 	assertNotified(t, deps, "Record Change: "+testHost, "warning")
@@ -585,12 +584,15 @@ func TestPortStateChange(t *testing.T) {
 	cfg := defaultTestConfig(t)
 	cfg.Hostnames = []string{testHost}
 
-	// Between the checks, every port closes.
-	deps := runChecks(t, cfg, nil, func(deps *testDeps) {
-		deps.portChecker.mu.Lock()
-		deps.portChecker.closed = true
-		deps.portChecker.mu.Unlock()
-	})
+	w, deps := runChecks(t, cfg, nil)
+
+	// Every port closes, and the port checks run again. They look
+	// nothing up.
+	deps.portChecker.mu.Lock()
+	deps.portChecker.closed = true
+	deps.portChecker.mu.Unlock()
+
+	w.CheckAllPorts(t.Context())
 
 	hs, _ := deps.state.GetHostnameState(testHost)
 	assertNotified(
@@ -610,7 +612,7 @@ func TestTLSExpiryWarning(t *testing.T) {
 	cfg := defaultTestConfig(t)
 	cfg.Hostnames = []string{testHost}
 
-	deps := runChecks(t, cfg, expiresInThreeDays, nil)
+	_, deps := runChecks(t, cfg, expiresInThreeDays)
 
 	assertNotified(t, deps, "TLS Expiry Warning: "+testHost, "warning")
 }
@@ -782,7 +784,7 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
 	cfg.Hostnames = []string{testHost}
 
 	// The saved state says the last check found testHost at oldIP.
-	deps := runChecks(t, cfg, func(deps *testDeps) {
+	_, deps := runChecks(t, cfg, func(deps *testDeps) {
 		deps.state.SetHostnameState(testHost, &state.HostnameState{
 			RecordsByNameserver: map[string]*state.NameserverRecordState{
 				oldNS1: {
@@ -791,7 +793,7 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
 				},
 			},
 		})
-	}, nil)
+	})
 
 	snap := deps.state.GetSnapshot()
 
@@ -922,21 +924,20 @@ func TestNSFailureAndRecovery(t *testing.T) {
 	cfg := defaultTestConfig(t)
 	cfg.Hostnames = []string{testHost}
 
-	// Between the checks, save every nameserver the first check found
-	// as one that did not answer, and add, as answering, one that live
-	// DNS does not list, which then disappears.
-	deps := runChecks(t, cfg, nil, func(deps *testDeps) {
-		hs, _ := deps.state.GetHostnameState(testHost)
-		for ns := range hs.RecordsByNameserver {
-			hs.RecordsByNameserver[ns] = failed()
+	nameservers := lookupNameservers(t, testHost)
+
+	// The saved state has every nameserver live DNS lists as one that
+	// did not answer, and, as answering, one that live DNS does not
+	// list, which then disappears.
+	_, deps := runChecks(t, cfg, func(deps *testDeps) {
+		byNameserver := map[string]*state.NameserverRecordState{
+			oldNS1: answered(map[string][]string{"A": {oldIP}}),
+		}
+		for _, ns := range nameservers {
+			byNameserver[ns] = failed()
 		}
 
-		hs.RecordsByNameserver[oldNS1] = &state.NameserverRecordState{
-			Records: map[string][]string{"A": {oldIP}},
-			Status:  "ok",
-		}
-
-		deps.state.SetHostnameState(testHost, hs)
+		deps.state.SetHostnameState(testHost, saved(byNameserver))
 	})
 
 	assertNotified(t, deps, "NS Failure: "+testHost, "error")