resolver: ask again over TCP when a reply offers recursion (closes #300)

dnswatcher never asks for recursion, so a reply marked recursion
available comes from a resolver on the network answering in the
server's place, as on a network that sends all DNS to one resolver.
Such a resolver answers only from its cache, or with SERVFAIL or
REFUSED, which made make check fail there.

A reply over UDP marked so is now asked again over TCP, which many such
networks leave alone, and the TCP reply is used. A reply from a root
server's address still marked so is not used, and when every root
server refuses or gives one, the error is the existing "this network
intercepts DNS queries".

Model: opus-5-5

README.md

@@ -355,17 +355,6 @@ dnswatcher exposes a lightweight HTTP API for operational visibility:
 | `GET /api/v1/status`           | Current monitoring state      |
 | `GET /metrics`                 | Prometheus metrics, see below |
 
-The health check, at `/.well-known/healthcheck` and `/health`, follows the most
-recent round of DNS checks. When every lookup in that round failed because no
-root server gave a usable reply, as when none of them answers or the network
-intercepts DNS queries (see DNS Resolution Strategy), dnswatcher could not reach
-the DNS at all. The health check then gives `status` `error` and answers
-`503 Service Unavailable`, so the image's health check fails and Docker and
-upaas see the container as unhealthy. Otherwise it gives `ok` and answers
-`200 OK`, as it also does before the first round finishes. A round gives `ok`
-even when a name's own nameservers do not answer or a domain does not exist,
-since the root servers gave a usable reply to those lookups.
-
 In `/api/v1/status`, each nameserver entry and certificate entry whose `status`
 is `error` also has `error`, the reason, as in the state file (see State File
 Format). A domain's own records are in its entry in `domains`, under
@@ -572,17 +561,16 @@ This approach ensures:
 - Ability to detect split-horizon or inconsistent responses across authoritative
   servers.
 
-A root server never offers recursion, so a reply marked "recursion available"
+dnswatcher never asks a server for recursion, so a reply marked "recursion
-from a root server's address came from a recursive resolver on the network
+available" did not come from the server it was sent to. It came from a recursive
-answering in its place, as on a network where a router rule, a VPN or a DNS
+resolver on the network answering in that server's place, as on a network where
-filter sends all DNS to one resolver. The first such reply from a root server's
+a router rule, a VPN or a DNS filter sends all DNS to one resolver. Such a query
-address, a refusal included, stops the lookup at once with the error "this
+is sent again over TCP, which many of those networks leave alone, and the reply
-network intercepts DNS queries", naming that address, and no other root server
+over TCP is used; when that retry fails, the reply over UDP is kept. A root
-is asked. A lookup that every root server refuses without that mark fails with
+server never offers recursion, so a reply from a root server's address that is
-the same error. Every lookup starts at the root servers, so on such a network
+still marked so after that is passed over. When every root server refuses a
-every lookup fails with that error, whether or not that resolver has the name
+query or gives such a reply, the lookup fails with the error "this network
-cached, and dnswatcher's tests fail there too. That is by design: nothing asks
+intercepts DNS queries".
-again another way or otherwise works around the interception.
 
 A watched name's records are stored as its nameservers return them, CNAME
 included. When they return a CNAME and no address, the chain of every CNAME

TODO.md

@@ -19,14 +19,10 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
 
 # Completed Steps
 
-- 2026-10-18: the health check answers 503 with `status` `error` when every
+- 2026-10-16: a reply over UDP marked "recursion available" is asked again over
-  lookup in the most recent round of DNS checks failed because no root server
+  TCP; such a reply from a root server's address is not used, and when every
-  gave a usable reply, as on a network that intercepts DNS queries (closes
+  root server refuses or gives one, the error is "this network intercepts DNS
-  #302).
+  queries" (closes #300).
-- 2026-10-16: the first reply marked "recursion available" from a root server's
-  address, a refusal included, stops the lookup with "this network intercepts
-  DNS queries", so on a network that sends DNS to a resolver dnswatcher and its
-  tests fail, by design (closes #300).
 - 2026-10-15: a query its caller's context ends before it is sent records no
   reason about the nameserver, and a try the context's deadline cuts short is a
   timeout, so a nameserver whose only failures were timeouts is `timeout`, not a

internal/handlers/healthcheck.go

@@ -2,25 +2,16 @@ package handlers
 
 import (
 	"net/http"
-
-	"sneak.berlin/go/dnswatcher/internal/healthcheck"
 )
 
-// HandleHealthCheck returns the health check handler. It answers 503
+// HandleHealthCheck returns the health check handler.
-// Service Unavailable when the health check's status is error, so that
-// a container health check fails, and 200 OK otherwise.
 func (h *Handlers) HandleHealthCheck() http.HandlerFunc {
 	return func(
 		writer http.ResponseWriter,
 		request *http.Request,
 	) {
-		resp := h.hc.Check()
+		h.respondJSON(
-
+			writer, request, h.hc.Check(), http.StatusOK,
-		code := http.StatusOK
+		)
-		if resp.Status == healthcheck.StatusError {
-			code = http.StatusServiceUnavailable
-		}
-
-		h.respondJSON(writer, request, resp, code)
 	}
 }

internal/handlers/healthcheck_test.go

@@ -1,158 +0,0 @@
-package handlers_test
-
-import (
-	"encoding/json"
-	"log/slog"
-	"net/http"
-	"net/http/httptest"
-	"testing"
-
-	"go.uber.org/fx/fxtest"
-
-	"sneak.berlin/go/dnswatcher/internal/config"
-	"sneak.berlin/go/dnswatcher/internal/globals"
-	"sneak.berlin/go/dnswatcher/internal/handlers"
-	"sneak.berlin/go/dnswatcher/internal/healthcheck"
-	"sneak.berlin/go/dnswatcher/internal/logger"
-	"sneak.berlin/go/dnswatcher/internal/notify"
-	"sneak.berlin/go/dnswatcher/internal/portcheck"
-	"sneak.berlin/go/dnswatcher/internal/resolver"
-	"sneak.berlin/go/dnswatcher/internal/state"
-	"sneak.berlin/go/dnswatcher/internal/tlscheck"
-	"sneak.berlin/go/dnswatcher/internal/watcher"
-)
-
-// TestHealthCheckAnswers503WhenDNSUnreachable serves the health check
-// before the watcher's first round of DNS checks, when it answers 200
-// with status ok, and after a round that could not reach the DNS, when
-// it answers 503 with status error. The watcher's resolver starts every
-// walk at public recursive resolvers of four operators, whose replies
-// are marked recursion available, as on a network that intercepts DNS
-// queries. If none of them replies, no root server answered, and the
-// round could not reach the DNS either.
-func TestHealthCheckAnswers503WhenDNSUnreachable(t *testing.T) {
-	t.Parallel()
-
-	hnd, w := newHandlersWithWatcher(t, resolver.NewWithRootServers(
-		slog.Default(),
-		[]string{"1.1.1.1", "8.8.8.8", "9.9.9.9", "208.67.222.222"},
-	))
-
-	assertHealthCheck(t, hnd, http.StatusOK, healthcheck.StatusOK)
-
-	w.RunOnce(t.Context())
-
-	assertHealthCheck(
-		t, hnd, http.StatusServiceUnavailable, healthcheck.StatusError,
-	)
-}
-
-// newHandlersWithWatcher builds real Handlers whose health check
-// follows the watcher it returns, which watches example.org with res.
-// The lifecycle is never started, so the watcher runs a round of checks
-// only when the test calls RunOnce.
-func newHandlersWithWatcher(
-	t *testing.T,
-	res *resolver.Resolver,
-) (*handlers.Handlers, *watcher.Watcher) {
-	t.Helper()
-
-	glob, err := globals.New(nil)
-	if err != nil {
-		t.Fatalf("globals.New: %v", err)
-	}
-
-	log, err := logger.New(nil, logger.Params{Globals: glob})
-	if err != nil {
-		t.Fatalf("logger.New: %v", err)
-	}
-
-	cfg := &config.Config{
-		Hostnames: []string{"example.org"},
-		DataDir:   t.TempDir(),
-	}
-	lifecycle := fxtest.NewLifecycle(t)
-
-	st, err := state.New(lifecycle, state.Params{Logger: log, Config: cfg})
-	if err != nil {
-		t.Fatalf("state.New: %v", err)
-	}
-
-	notifier, err := notify.New(
-		lifecycle, notify.Params{Logger: log, Config: cfg},
-	)
-	if err != nil {
-		t.Fatalf("notify.New: %v", err)
-	}
-
-	w, err := watcher.New(lifecycle, watcher.Params{
-		Logger:    log,
-		Config:    cfg,
-		State:     st,
-		Resolver:  res,
-		PortCheck: portcheck.NewStandalone(),
-		TLSCheck:  tlscheck.NewStandalone(),
-		Notify:    notifier,
-	})
-	if err != nil {
-		t.Fatalf("watcher.New: %v", err)
-	}
-
-	hc, err := healthcheck.New(lifecycle, healthcheck.Params{
-		Globals: glob,
-		Config:  cfg,
-		Logger:  log,
-		Watcher: w,
-	})
-	if err != nil {
-		t.Fatalf("healthcheck.New: %v", err)
-	}
-
-	hnd, err := handlers.New(nil, handlers.Params{
-		Logger:      log,
-		Globals:     glob,
-		Healthcheck: hc,
-		State:       st,
-		Notify:      notifier,
-	})
-	if err != nil {
-		t.Fatalf("handlers.New: %v", err)
-	}
-
-	return hnd, w
-}
-
-// assertHealthCheck serves one request to the health check and checks
-// that it answers with code and a JSON body whose status is status.
-func assertHealthCheck(
-	t *testing.T,
-	hnd *handlers.Handlers,
-	code int,
-	status string,
-) {
-	t.Helper()
-
-	rec := httptest.NewRecorder()
-	req := httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, "/.well-known/healthcheck", nil,
-	)
-
-	hnd.HandleHealthCheck()(rec, req)
-
-	if rec.Code != code {
-		t.Errorf("code = %d, want %d", rec.Code, code)
-	}
-
-	var body struct {
-		Status string `json:"status"`
-	}
-
-	err := json.Unmarshal(rec.Body.Bytes(), &body)
-	if err != nil {
-		t.Fatalf("decoding response: %v", err)
-	}
-
-	if body.Status != status {
-		t.Errorf("status = %q, want %q", body.Status, status)
-	}
-}

internal/healthcheck/healthcheck.go

@@ -11,13 +11,6 @@ import (
 	"sneak.berlin/go/dnswatcher/internal/config"
 	"sneak.berlin/go/dnswatcher/internal/globals"
 	"sneak.berlin/go/dnswatcher/internal/logger"
-	"sneak.berlin/go/dnswatcher/internal/watcher"
-)
-
-// Values of Response.Status.
-const (
-	StatusOK    = "ok"
-	StatusError = "error"
 )
 
 // Params contains dependencies for Healthcheck.
@@ -27,7 +20,6 @@ type Params struct {
 	Globals *globals.Globals
 	Config  *config.Config
 	Logger  *logger.Logger
-	Watcher *watcher.Watcher
 }
 
 // Healthcheck provides health status information.
@@ -69,17 +61,10 @@ func New(
 	return healthcheck, nil
 }
 
-// Check returns the current health status. Status is StatusError when
+// Check returns the current health status.
-// the watcher's most recent round of DNS checks could not reach the DNS
-// (see watcher.Watcher.DNSUnreachable), and StatusOK otherwise.
 func (h *Healthcheck) Check() *Response {
-	status := StatusOK
-	if h.params.Watcher.DNSUnreachable() {
-		status = StatusError
-	}
-
 	return &Response{
-		Status:        status,
+		Status:        "ok",
 		Now:           time.Now().UTC().Format(time.RFC3339Nano),
 		UptimeSeconds: int64(h.uptime().Seconds()),
 		UptimeHuman:   h.uptime().String(),

internal/healthcheck/healthcheck_test.go

@@ -3,12 +3,10 @@ package healthcheck_test
 import (
 	"context"
 	"encoding/json"
-	"log/slog"
 	"testing"
 	"time"
 
 	"go.uber.org/fx"
-	"go.uber.org/fx/fxtest"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -17,12 +15,6 @@ import (
 	"sneak.berlin/go/dnswatcher/internal/globals"
 	"sneak.berlin/go/dnswatcher/internal/healthcheck"
 	"sneak.berlin/go/dnswatcher/internal/logger"
-	"sneak.berlin/go/dnswatcher/internal/notify"
-	"sneak.berlin/go/dnswatcher/internal/portcheck"
-	"sneak.berlin/go/dnswatcher/internal/resolver"
-	"sneak.berlin/go/dnswatcher/internal/state"
-	"sneak.berlin/go/dnswatcher/internal/tlscheck"
-	"sneak.berlin/go/dnswatcher/internal/watcher"
 )
 
 // recordingLifecycle is a minimal fx.Lifecycle that records the hooks
@@ -38,14 +30,12 @@ func (l *recordingLifecycle) Append(hook fx.Hook) {
 
 // newHealthcheck builds a Healthcheck through the real constructor and
 // runs the registered OnStart hook so StartupTime is set the same way
-// the fx lifecycle would set it. It follows the watcher it returns,
+// the fx lifecycle would set it.
-// which watches example.org with res and has run no round of checks.
 func newHealthcheck(
 	t *testing.T,
-	res *resolver.Resolver,
 	maintenance bool,
 	version string,
-) (*healthcheck.Healthcheck, *watcher.Watcher) {
+) *healthcheck.Healthcheck {
 	t.Helper()
 
 	g := &globals.Globals{Appname: "dnswatcher", Version: version}
@@ -53,21 +43,12 @@ func newHealthcheck(
 	log, err := logger.New(nil, logger.Params{Globals: g})
 	require.NoError(t, err)
 
-	cfg := &config.Config{
-		MaintenanceMode: maintenance,
-		Hostnames:       []string{"example.org"},
-		DataDir:         t.TempDir(),
-	}
-
-	w := newWatcher(t, log, cfg, res)
-
 	lifecycle := &recordingLifecycle{}
 
 	hc, err := healthcheck.New(lifecycle, healthcheck.Params{
 		Globals: g,
-		Config:  cfg,
+		Config:  &config.Config{MaintenanceMode: maintenance},
 		Logger:  log,
-		Watcher: w,
 	})
 	require.NoError(t, err)
 
@@ -76,53 +57,16 @@ func newHealthcheck(
 	require.NotNil(t, lifecycle.hooks[0].OnStart)
 	require.NoError(t, lifecycle.hooks[0].OnStart(context.Background()))
 
-	return hc, w
+	return hc
-}
-
-// newWatcher builds a watcher through the real constructor, with res as
-// its resolver. Its lifecycle is never started, so it runs a round of
-// checks only when the test calls RunOnce.
-func newWatcher(
-	t *testing.T,
-	log *logger.Logger,
-	cfg *config.Config,
-	res *resolver.Resolver,
-) *watcher.Watcher {
-	t.Helper()
-
-	lifecycle := fxtest.NewLifecycle(t)
-
-	st, err := state.New(lifecycle, state.Params{Logger: log, Config: cfg})
-	require.NoError(t, err)
-
-	notifier, err := notify.New(
-		lifecycle, notify.Params{Logger: log, Config: cfg},
-	)
-	require.NoError(t, err)
-
-	w, err := watcher.New(lifecycle, watcher.Params{
-		Logger:    log,
-		Config:    cfg,
-		State:     st,
-		Resolver:  res,
-		PortCheck: portcheck.NewStandalone(),
-		TLSCheck:  tlscheck.NewStandalone(),
-		Notify:    notifier,
-	})
-	require.NoError(t, err)
-
-	return w
 }
 
 func TestCheckStatusAndPayloadShape(t *testing.T) {
 	t.Parallel()
 
-	hc, _ := newHealthcheck(
+	hc := newHealthcheck(t, false, "v9.9.9")
-		t, resolver.NewFromLogger(slog.Default()), false, "v9.9.9",
-	)
 	resp := hc.Check()
 
-	assert.Equal(t, healthcheck.StatusOK, resp.Status)
+	assert.Equal(t, "ok", resp.Status)
 
 	// The JSON shape and field names are part of the contract for the
 	// /health and /.well-known/healthcheck routes, so assert on the
@@ -170,10 +114,7 @@ func TestCheckMaintenanceModeReflectsConfig(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			hc, _ := newHealthcheck(
+			hc := newHealthcheck(t, tt.maintenance, "test")
-				t, resolver.NewFromLogger(slog.Default()), tt.maintenance,
-				"test",
-			)
 			resp := hc.Check()
 			assert.Equal(t, tt.maintenance, resp.Maintenance,
 				"maintenanceMode must mirror Config.MaintenanceMode")
@@ -184,38 +125,10 @@ func TestCheckMaintenanceModeReflectsConfig(t *testing.T) {
 func TestCheckSurfacesVersionAndAppname(t *testing.T) {
 	t.Parallel()
 
-	hc, _ := newHealthcheck(
+	hc := newHealthcheck(t, false, "surfaced-version-123")
-		t, resolver.NewFromLogger(slog.Default()), false,
-		"surfaced-version-123",
-	)
 	resp := hc.Check()
 
 	assert.Equal(t, "surfaced-version-123", resp.Version,
 		"version from globals must appear in the payload")
 	assert.Equal(t, "dnswatcher", resp.Appname)
 }
-
-// TestCheckStatusFollowsTheDNSChecks checks that the status is ok
-// before the watcher's first round of DNS checks, and error after a
-// round that could not reach the DNS. The watcher's resolver starts
-// every walk at public recursive resolvers of four operators, whose
-// replies are marked recursion available, as on a network that
-// intercepts DNS queries. If none of them replies, no root server
-// answered, and the round could not reach the DNS either.
-func TestCheckStatusFollowsTheDNSChecks(t *testing.T) {
-	t.Parallel()
-
-	res := resolver.NewWithRootServers(
-		slog.Default(),
-		[]string{"1.1.1.1", "8.8.8.8", "9.9.9.9", "208.67.222.222"},
-	)
-	hc, w := newHealthcheck(t, res, false, "test")
-
-	assert.Equal(t, healthcheck.StatusOK, hc.Check().Status,
-		"status before the first round")
-
-	w.RunOnce(t.Context())
-
-	assert.Equal(t, healthcheck.StatusError, hc.Check().Status,
-		"status after a round that could not reach the DNS")
-}

internal/resolver/errors.go

@@ -36,12 +36,6 @@ var (
 	// so whether the name has addresses is unknown.
 	ErrNoNameserverAnswered = errors.New("no nameserver answered")
 
-	// ErrNoRootServerAnswered is returned when the root servers were
-	// asked and none of them gave a usable reply, as when the host
-	// running dnswatcher has lost its network. When they all refused,
-	// the error is ErrIntercepted instead.
-	ErrNoRootServerAnswered = errors.New("no root server answered")
-
 	// ErrNoNameserverAddress is returned when a nameserver is to be
 	// asked and no address for it was given.
 	ErrNoNameserverAddress = errors.New("nameserver has no address")
@@ -59,13 +53,18 @@ var (
 		"reply truncated and its retry over TCP failed",
 	)
 
-	// ErrIntercepted is returned when a root server's address gave a
+	// ErrRecursionAvailable is the reason a reply from a root server
-	// reply marked recursion available, a refusal included, which ends
+	// marked recursion available is not used. Root servers offer no
-	// the lookup at once, or when every root server refused a query.
+	// recursion, so the reply came from something on the network
-	// Root servers do neither, so those replies came from something on
+	// answering in their place.
-	// the network answering in their place. On such a network
+	ErrRecursionAvailable = errors.New(
-	// dnswatcher fails with it, by design: nothing works around the
+		"reply marked recursion available, so not from a root server",
-	// interception.
+	)
+
+	// ErrIntercepted is returned when every root server refused a
+	// query or gave a reply marked recursion available. Root servers do
+	// neither, so those replies came from something on the network
+	// answering in their place.
 	ErrIntercepted = errors.New("this network intercepts DNS queries")
 
 	// ErrCNAMEDepthExceeded is returned when a CNAME chain

internal/resolver/iterative.go

@@ -94,15 +94,18 @@ func (r *Resolver) tryExchange(
 }
 
 // retryTCP returns the reply to msg over TCP when resp, its reply over
-// UDP, is truncated. When that fails it returns resp, still truncated,
+// UDP, is truncated or marked recursion available. msg does not ask for
-// which holds only the records that fit.
+// recursion, so a reply marked so came from a recursive resolver on the
+// network answering in the server's place, and many networks that send
+// DNS to one do so only for UDP. When the retry fails it returns resp;
+// a truncated one holds only the records that fit.
 func (r *Resolver) retryTCP(
 	ctx context.Context,
 	msg *dns.Msg,
 	addr string,
 	resp *dns.Msg,
 ) *dns.Msg {
-	if !resp.Truncated {
+	if !resp.Truncated && !resp.RecursionAvailable {
 		return resp
 	}
 
@@ -115,8 +118,9 @@ func (r *Resolver) retryTCP(
 }
 
 // queryDNS sends a DNS query to a specific server IP, never asking it
-// for recursion. A reply of REFUSED is returned together with
+// for recursion. When the reply over UDP is truncated or marked
-// ErrRefused, so that the caller can still read the reply's flags.
+// recursion available, the query is sent again over TCP (see retryTCP).
+// A reply of REFUSED is returned as ErrRefused.
 func (r *Resolver) queryDNS(
 	ctx context.Context,
 	serverIP string,
@@ -139,13 +143,15 @@ func (r *Resolver) queryDNS(
 		return nil, fmt.Errorf("query %s @%s: %w", name, serverIP, err)
 	}
 
+	resp = r.retryTCP(ctx, msg, addr, resp)
+
 	if resp.Rcode == dns.RcodeRefused {
-		return resp, fmt.Errorf(
+		return nil, fmt.Errorf(
 			"query %s @%s: %w", name, serverIP, ErrRefused,
 		)
 	}
 
-	return r.retryTCP(ctx, msg, addr, resp), nil
+	return resp, nil
 }
 
 func extractNSSet(rrs []dns.RR) []string {
@@ -285,16 +291,11 @@ func shuffled(
 }
 
 // queryServers asks servers, the servers of zone, about name in a random
-// order until one gives a usable reply. When they are the root servers,
+// order until one gives a reply it uses (see replyError). A server that
-// the first reply marked recursion available, a refusal included, ends
+// times out, refuses or gives a reply it does not use is passed over for
-// the lookup with ErrIntercepted, naming the address it came from, and
+// the next. When every server refused, the error says so. When they are
-// no other root server is asked: root servers offer no recursion, so
+// the root servers and each one refused or gave a reply marked recursion
-// that reply came from something on the network answering in their
+// available, the error is ErrIntercepted.
-// place. Otherwise a server that times out, refuses or gives a reply
-// that is not usable is passed over for the next. When every server
-// refused, the error says so, and when they are the root servers it is
-// ErrIntercepted. When the root servers gave no usable reply otherwise,
-// it is ErrNoRootServerAnswered.
 func (r *Resolver) queryServers(
 	ctx context.Context,
 	servers []string,
@@ -305,27 +306,16 @@ func (r *Resolver) queryServers(
 	var lastErr error
 
 	refused := 0
+	recursionAvailable := 0
 
 	for _, ip := range shuffled(servers, rand.Shuffle) {
 		if checkCtx(ctx) != nil {
 			return nil, ErrContextCanceled
 		}
 
-		// queryDNS returns a refusal together with its reply, so a
-		// refusal's mark is checked here too.
 		resp, err := r.queryDNS(ctx, ip, name, qtype)
-		if resp != nil && zone == "." && resp.RecursionAvailable {
+		if err == nil {
-			return nil, fmt.Errorf(
+			err = replyError(resp, ip, zone, name)
-				"query %s @%s: reply marked recursion available, "+
-					"so not from a root server: %w",
-				name, ip, ErrIntercepted,
-			)
-		}
-
-		if err == nil && !usableReply(resp, zone, name) {
-			err = fmt.Errorf(
-				"query %s @%s: %w", name, ip, ErrUnusableReply,
-			)
 		}
 
 		if err == nil {
@@ -336,6 +326,10 @@ func (r *Resolver) queryServers(
 			refused++
 		}
 
+		if errors.Is(err, ErrRecursionAvailable) {
+			recursionAvailable++
+		}
+
 		lastErr = err
 	}
 
@@ -346,6 +340,14 @@ func (r *Resolver) queryServers(
 		)
 	}
 
+	if refused+recursionAvailable == len(servers) && zone == "." {
+		return nil, fmt.Errorf(
+			"every root server refused a query for %s or gave a reply "+
+				"marked recursion available: %w",
+			name, ErrIntercepted,
+		)
+	}
+
 	if refused == len(servers) {
 		return nil, fmt.Errorf(
 			"every server of %s refused a query for %s: %w",
@@ -353,11 +355,28 @@ func (r *Resolver) queryServers(
 		)
 	}
 
-	if zone == "." {
+	return nil, fmt.Errorf("all servers failed: %w", lastErr)
-		return nil, fmt.Errorf("%w: %w", ErrNoRootServerAnswered, lastErr)
 }
 
-	return nil, fmt.Errorf("all servers failed: %w", lastErr)
+// replyError returns why resp, the reply of the server at ip, one of the
+// servers of zone, to a query about name, is not used, or nil when it
+// is. A reply that is not usable is not used. Nor is a reply from a root
+// server marked recursion available: root servers offer no recursion,
+// so it came from something on the network answering in their place.
+func replyError(resp *dns.Msg, ip string, zone string, name string) error {
+	if zone == "." && resp.RecursionAvailable {
+		return fmt.Errorf(
+			"query %s @%s: %w", name, ip, ErrRecursionAvailable,
+		)
+	}
+
+	if !usableReply(resp, zone, name) {
+		return fmt.Errorf(
+			"query %s @%s: %w", name, ip, ErrUnusableReply,
+		)
+	}
+
+	return nil
 }
 
 // isErrorReply reports whether msg is an error reply: one with any code
@@ -533,7 +552,7 @@ func (r *Resolver) resolveNSIterative(
 	}
 
 	domain = dns.Fqdn(domain)
-	servers := r.rootServers
+	servers := rootServerList()
 	zone := "."
 
 	for range maxDelegation {
@@ -597,7 +616,7 @@ func (r *Resolver) resolveARecord(
 	}
 
 	hostname = dns.Fqdn(hostname)
-	servers := r.rootServers
+	servers := rootServerList()
 	zone := "."
 
 	var withoutAddresses []string
@@ -657,7 +676,7 @@ func (r *Resolver) FindAuthoritativeNameservers(
 	ctx context.Context,
 	domain string,
 ) ([]string, error) {
-	return r.findAuthoritativeNameservers(ctx, domain, r.rootServers)
+	return r.findAuthoritativeNameservers(ctx, domain, rootServerList())
 }
 
 // findAuthoritativeNameservers is FindAuthoritativeNameservers with each
@@ -1132,7 +1151,7 @@ func (r *Resolver) LookupNS(
 	}
 
 	return r.followDelegation(
-		ctx, dns.Fqdn(strings.ToLower(domain)), ".", nil, r.rootServers,
+		ctx, dns.Fqdn(strings.ToLower(domain)), ".", nil, rootServerList(),
 	)
 }
 

internal/resolver/resolver.go

@@ -43,12 +43,10 @@ type NameserverResponse struct {
 }
 
 // Resolver performs iterative DNS resolution from root servers.
-// rootServers are the addresses every walk from the root starts at.
 type Resolver struct {
 	log    *slog.Logger
 	client DNSClient
 	tcp    DNSClient
-	rootServers []string
 }
 
 // New creates a new Resolver instance for use with uber/fx.
@@ -60,7 +58,6 @@ func New(
 		log:    params.Logger.Get(),
 		client: &udpClient{timeout: queryTimeoutDuration},
 		tcp:    &tcpClient{timeout: queryTimeoutDuration},
-		rootServers: rootServerList(),
 	}, nil
 }
 
@@ -71,19 +68,7 @@ func NewFromLogger(log *slog.Logger) *Resolver {
 		log:    log,
 		client: &udpClient{timeout: queryTimeoutDuration},
 		tcp:    &tcpClient{timeout: queryTimeoutDuration},
-		rootServers: rootServerList(),
 	}
 }
 
-// NewWithRootServers creates a Resolver as NewFromLogger does, except
-// that every walk from the root starts at servers instead of at the root
-// servers. Tests use it to show what dnswatcher does when no root server
-// answers, or when something on the network answers in their place.
-func NewWithRootServers(log *slog.Logger, servers []string) *Resolver {
-	r := NewFromLogger(log)
-	r.rootServers = servers
-
-	return r
-}
-
 // Method implementations are in iterative.go.

internal/resolver/resolver_test.go

@@ -815,7 +815,6 @@ func TestQueryServers_EveryRootServerRefused(t *testing.T) {
 // but nothing answers at 192.0.2.1, a documentation address, so not
 // every server refused, wherever 192.0.2.1 falls in the random order:
 // the error is not ErrIntercepted and does not say every server refused.
-// No root server gave a usable reply, so it is ErrNoRootServerAnswered.
 func TestQueryServers_NotEveryRootServerRefused(t *testing.T) {
 	t.Parallel()
 
@@ -849,77 +848,38 @@ func TestQueryServers_NotEveryRootServerRefused(t *testing.T) {
 	require.NotErrorIs(t, err, resolver.ErrIntercepted)
 	// Both errors for every server refusing say "refused a query for".
 	require.NotContains(t, err.Error(), "refused a query for")
-	require.ErrorIs(t, err, resolver.ErrNoRootServerAnswered)
-}
-
-// TestLookupsWhenNoRootServerAnswers starts every walk at 192.0.2.1, a
-// documentation address where nothing answers, as if no root server
-// answered. The lookups the watcher makes, LookupNS for a domain and
-// LookupAllRecords for a hostname, both fail with
-// ErrNoRootServerAnswered, which the watcher reads.
-func TestLookupsWhenNoRootServerAnswers(t *testing.T) {
-	t.Parallel()
-
-	r := resolver.NewWithRootServers(slog.Default(), []string{noAnswerAddress})
-
-	t.Run("LookupNS", func(t *testing.T) {
-		t.Parallel()
-
-		_, err := r.LookupNS(t.Context(), "google.com")
-		require.ErrorIs(t, err, resolver.ErrNoRootServerAnswered)
-		require.NotErrorIs(t, err, resolver.ErrIntercepted)
-	})
-
-	t.Run("LookupAllRecords", func(t *testing.T) {
-		t.Parallel()
-
-		_, err := r.LookupAllRecords(t.Context(), "www.google.com")
-		require.ErrorIs(t, err, resolver.ErrNoRootServerAnswered)
-		require.NotErrorIs(t, err, resolver.ErrIntercepted)
-	})
 }
 
 // TestQueryServers_RecursiveResolversAsRootServers passes public
-// recursive resolvers to QueryServers as the servers of the root zone,
+// recursive resolvers of four operators to QueryServers as the servers
-// as a network that sends every DNS query to a resolver would. Their
+// of the root zone, as a network that sends every DNS query to a
-// replies are marked recursion available, which no root server's is:
+// resolver would. Over UDP and over TCP alike, they mark every reply
-// SERVFAIL, REFUSED, or an answer from a resolver's cache, as 1.1.1.1
+// recursion available, which no root server does. Each refuses the
-// gives for com., which would be used if a root server gave it. The
+// query, replies SERVFAIL, or answers from its cache, as 1.1.1.1 does
-// first such reply ends the lookup with ErrIntercepted, naming the
+// for com.: a reply that would be used if a root server gave it. Either
-// resolver that gave it. In the first group the replies differ; every
+// way the error is ErrIntercepted, and since 1.1.1.1 and 8.8.8.8 refuse
-// resolver in the second refuses a query that does not ask for
+// no such query, it says a reply was marked recursion available.
-// recursion, as 9.9.9.9 does, so there a refusal ends the lookup
-// instead of being passed over. Each group has resolvers of four
-// operators, and a resolver that gives no reply is passed over for the
-// next, so one of them answering is enough.
 func TestQueryServers_RecursiveResolversAsRootServers(t *testing.T) {
 	t.Parallel()
 
 	r := newTestResolver(t)
-	groups := [][]string{
+	resolvers := []string{
-		{"1.1.1.1", "8.8.8.8", "9.9.9.9", "208.67.222.222"},
+		"1.1.1.1", "8.8.8.8", "9.9.9.9", "208.67.222.222",
-		{"64.6.64.6", "185.222.222.222", "4.2.2.1", "94.140.14.14"},
 	}
 
-	for _, resolvers := range groups {
 	for _, name := range []string{"com.", "desec.io."} {
 		var err error
 
 		livednstest.Retry(
 			t,
-				"QueryServers("+strings.Join(resolvers, ", ")+
+			"QueryServers(public recursive resolvers as root servers, "+
-					" as root servers, "+name+")",
+				name+")",
 			func(ctx context.Context) error {
-					_, err = r.QueryServers(
+				_, err = r.QueryServers(ctx, resolvers, ".", name, dns.TypeNS)
-						ctx, resolvers, ".", name, dns.TypeNS,
-					)
 
-					// When no resolver gave a reply marked recursion
+				// When the error is another, one resolver may have given
-					// available, none may have replied at all, so the
+				// no reply at all, so the attempt is tried again.
-					// attempt is tried again.
+				if err != nil && !errors.Is(err, resolver.ErrIntercepted) {
-					if err != nil && !strings.Contains(
-						err.Error(), "marked recursion available",
-					) {
 					return fmt.Errorf(
 						"%w: %w", livednstest.ErrNoAnswer, err,
 					)
@@ -930,21 +890,13 @@ func TestQueryServers_RecursiveResolversAsRootServers(t *testing.T) {
 		)
 
 		require.ErrorIs(t, err, resolver.ErrIntercepted)
-
+		require.EqualError(
-			// The error names the resolver whose reply ended the
+			t, err,
-			// lookup.
+			"every root server refused a query for "+name+" or gave a "+
-			messages := make([]string, 0, len(resolvers))
+				"reply marked recursion available: "+
-			for _, ip := range resolvers {
-				messages = append(messages,
-					"query "+name+" @"+ip+": reply marked recursion "+
-						"available, so not from a root server: "+
 				"this network intercepts DNS queries",
 		)
 	}
-
-			require.Contains(t, messages, err.Error())
-		}
-	}
 }
 
 func TestQueryNameserver_RecordsSorted(t *testing.T) {

internal/server/server_test.go

@@ -15,21 +15,14 @@ import (
 	"sneak.berlin/go/dnswatcher/internal/logger"
 	"sneak.berlin/go/dnswatcher/internal/middleware"
 	"sneak.berlin/go/dnswatcher/internal/notify"
-	"sneak.berlin/go/dnswatcher/internal/portcheck"
-	"sneak.berlin/go/dnswatcher/internal/resolver"
 	"sneak.berlin/go/dnswatcher/internal/server"
 	"sneak.berlin/go/dnswatcher/internal/state"
-	"sneak.berlin/go/dnswatcher/internal/tlscheck"
-	"sneak.berlin/go/dnswatcher/internal/watcher"
 )
 
 // newServerApp builds an fx app holding a *server.Server wired exactly
-// as cmd/dnswatcher wires it, plus the given option. The health check
+// as cmd/dnswatcher wires it, minus the watcher/resolver subtree that
-// follows the watcher, so the watcher is in the app too. Starting the
+// would touch live DNS, plus the given option. config.New reads viper,
-// app starts the watcher's first round of checks, against live DNS,
+// so the caller must first configure it, which is also why the caller
-// before the server; the tests that start it expect the server to fail
-// to start, which stops the watcher again. config.New reads viper, so
-// the caller must first configure it, which is also why the caller
 // cannot run in parallel.
 func newServerApp(option fx.Option) *fx.App {
 	return fx.New(
@@ -40,29 +33,11 @@ func newServerApp(option fx.Option) *fx.App {
 			config.New,
 			state.New,
 			healthcheck.New,
-			resolver.New,
-			portcheck.New,
-			tlscheck.New,
 			notify.New,
-			watcher.New,
 			middleware.New,
 			handlers.New,
 			server.New,
 		),
-		fx.Provide(
-			func(r *resolver.Resolver) watcher.DNSResolver {
-				return r
-			},
-			func(p *portcheck.Checker) watcher.PortChecker {
-				return p
-			},
-			func(t *tlscheck.Checker) watcher.TLSChecker {
-				return t
-			},
-			func(n *notify.Service) watcher.Notifier {
-				return n
-			},
-		),
 		option,
 	)
 }

internal/watcher/dnsunreachable_test.go

@@ -1,66 +0,0 @@
-package watcher_test
-
-import (
-	"log/slog"
-	"testing"
-
-	"sneak.berlin/go/dnswatcher/internal/resolver"
-	"sneak.berlin/go/dnswatcher/internal/watcher"
-)
-
-// TestDNSUnreachableWhenNoRootServerGivesUsableReply runs one round of
-// checks of a domain and a hostname with every walk starting at real
-// servers that are not root servers. At 192.0.2.1, a documentation
-// address, nothing answers, as when no root server answers. Public
-// recursive resolvers of four operators mark their replies recursion
-// available, as on a network that intercepts DNS queries; one that gives
-// no reply is passed over for the next, and if none replies, no root
-// server answered. Either way every lookup failed because no root server
-// gave a usable reply, so the round could not reach the DNS.
-// TestFirstRunBaseline and TestDomainThatDoesNotExist check rounds in
-// which the root servers gave a usable reply.
-func TestDNSUnreachableWhenNoRootServerGivesUsableReply(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name    string
-		servers []string
-	}{
-		{"no root server answers", []string{"192.0.2.1"}},
-		{
-			"the network intercepts DNS queries",
-			[]string{"1.1.1.1", "8.8.8.8", "9.9.9.9", "208.67.222.222"},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			cfg := defaultTestConfig(t)
-			cfg.Domains = []string{testSmallDomain}
-			cfg.Hostnames = []string{testHost}
-
-			deps := newTestDeps(t, cfg)
-			w := watcher.NewForTest(
-				cfg,
-				deps.state,
-				resolver.NewWithRootServers(slog.Default(), tt.servers),
-				deps.portChecker,
-				deps.tlsChecker,
-				deps.notifier,
-			)
-
-			if w.DNSUnreachable() {
-				t.Error("DNSUnreachable is true before the first round")
-			}
-
-			w.RunOnce(t.Context())
-
-			if !w.DNSUnreachable() {
-				t.Error("DNSUnreachable is false after a round in " +
-					"which no root server gave a usable reply")
-			}
-		})
-	}
-}

internal/watcher/watcher.go

@@ -11,7 +11,6 @@ import (
 	"sort"
 	"strconv"
 	"strings"
-	"sync/atomic"
 	"time"
 
 	"go.uber.org/fx"
@@ -63,11 +62,6 @@ type Watcher struct {
 	cancel    context.CancelFunc
 	done      chan struct{} // closed when Run returns
 	firstRun  bool
-
-	// dnsUnreachable is what DNSUnreachable reports. Each round of DNS
-	// checks sets it, and the HTTP server reads it from its own
-	// goroutines.
-	dnsUnreachable atomic.Bool
 }
 
 // New creates a new Watcher instance wired into the fx lifecycle.
@@ -203,18 +197,6 @@ func (w *Watcher) RunOnce(ctx context.Context) {
 	w.firstRun = false
 }
 
-// DNSUnreachable reports whether the most recent round of DNS checks
-// could not reach the DNS at all: every lookup in it failed because no
-// root server gave a usable reply, as when none of them answered or the
-// network intercepts DNS queries. A round reached the DNS even when a
-// name's own nameservers did not answer or a domain does not exist,
-// since the root servers gave a usable reply to those lookups. It is
-// false until the first round finishes, and a round that shutdown cut
-// short changes nothing. It may be called while a round runs.
-func (w *Watcher) DNSUnreachable() bool {
-	return w.dnsUnreachable.Load()
-}
-
 func (w *Watcher) detectFirstRun() {
 	snap := w.state.GetSnapshot()
 	hasState := len(snap.Domains) > 0 ||
@@ -283,40 +265,15 @@ func (w *Watcher) cleanupRemovedTargets() {
 // runDNSChecks performs DNS resolution for all configured domains
 // and hostnames, updating state with freshly resolved records.
 // This must complete before port or TLS checks run so those
-// checks operate on current IP addresses. Unless ctx was cut short, it
+// checks operate on current IP addresses.
-// then records whether this round could reach the DNS (see
-// DNSUnreachable).
 func (w *Watcher) runDNSChecks(ctx context.Context) {
-	errs := make([]error, 0, len(w.config.Domains)+len(w.config.Hostnames))
-
 	for _, domain := range w.config.Domains {
-		errs = append(errs, w.checkDomain(ctx, domain))
+		w.checkDomain(ctx, domain)
 	}
 
 	for _, hostname := range w.config.Hostnames {
-		_, err := w.checkHostname(ctx, hostname)
+		w.checkHostname(ctx, hostname)
-		errs = append(errs, err)
 	}
-
-	if ctx.Err() == nil {
-		w.dnsUnreachable.Store(noRootServerGaveUsableReply(errs))
-	}
-}
-
-// noRootServerGaveUsableReply reports whether errs, the errors of the
-// lookups in a round of DNS checks, nil for a lookup that succeeded,
-// show that every lookup failed because no root server gave a usable
-// reply: each is ErrNoRootServerAnswered or ErrIntercepted. A round with
-// no lookups shows nothing.
-func noRootServerGaveUsableReply(errs []error) bool {
-	for _, err := range errs {
-		if !errors.Is(err, resolver.ErrNoRootServerAnswered) &&
-			!errors.Is(err, resolver.ErrIntercepted) {
-			return false
-		}
-	}
-
-	return len(errs) > 0
 }
 
 // logFailedLookup logs a failed DNS lookup at error level, unless ctx
@@ -335,16 +292,14 @@ func (w *Watcher) logFailedLookup(
 }
 
 // checkDomain looks up domain's delegation and checks domain with it
-// (see checkDomainResult). It returns the lookup's error.
+// (see checkDomainResult).
 func (w *Watcher) checkDomain(
 	ctx context.Context,
 	domain string,
-) error {
+) {
 	nameservers, err := w.resolver.LookupNS(ctx, domain)
 
 	w.checkDomainResult(ctx, domain, nameservers, err)
-
-	return err
 }
 
 // checkDomainResult finishes a check of domain from nameservers and err,
@@ -403,7 +358,7 @@ func (w *Watcher) checkDomainResult(
 	// check, as detectNSDisappearances needs.
 	if nxdomain {
 		w.state.DeleteHostnameState(domain)
-	} else if saved, _ := w.checkHostname(ctx, domain); !saved {
+	} else if !w.checkHostname(ctx, domain) {
 		return
 	}
 
@@ -529,12 +484,11 @@ func (w *Watcher) detectNSAddressChanges(
 }
 
 // checkHostname looks up hostname's records and checks them (see
-// checkHostnameResult). It reports whether it saved them, and returns
+// checkHostnameResult). It reports whether it saved them.
-// the lookup's error.
 func (w *Watcher) checkHostname(
 	ctx context.Context,
 	hostname string,
-) (bool, error) {
+) bool {
 	results, err := w.resolver.LookupAllRecords(ctx, hostname)
 	if err != nil {
 		w.logFailedLookup(
@@ -545,7 +499,7 @@ func (w *Watcher) checkHostname(
 		)
 	}
 
-	return w.checkHostnameResult(ctx, hostname, results, err), err
+	return w.checkHostnameResult(ctx, hostname, results, err)
 }
 
 // checkHostnameResult finishes a check of hostname from results and err,

internal/watcher/watcher_test.go

@@ -385,16 +385,11 @@ func TestFirstRunBaseline(t *testing.T) {
 	cfg.Domains = []string{testSmallDomain}
 	cfg.Hostnames = []string{testHost}
 
-	w, deps := runChecks(t, cfg, nil)
+	_, deps := runChecks(t, cfg, nil)
 
 	assertNoNotifications(t, deps)
 	assertStatePopulated(t, deps)
 
-	if w.DNSUnreachable() {
-		t.Error("DNSUnreachable is true after a round whose lookups " +
-			"succeeded")
-	}
-
 	// testHost answers with an address, so the check saves an empty list
 	// of CNAME addresses for it; nil would mean the check did not look
 	// at whether to follow a CNAME.
@@ -556,9 +551,7 @@ func TestNSChangeDetection(t *testing.T) {
 // with nameservers and records saved by an earlier check. The .com
 // servers answer that it does not exist, so it is saved with nxdomain
 // set and no nameservers, an NS Change removes them all, and its saved
-// records are removed rather than asked for at the .com servers. The
+// records are removed rather than asked for at the .com servers.
-// round's only lookup failed, but the root servers and the .com servers
-// answered, so the round reached the DNS.
 func TestDomainThatDoesNotExist(t *testing.T) {
 	t.Parallel()
 
@@ -567,12 +560,11 @@ func TestDomainThatDoesNotExist(t *testing.T) {
 	cfg := defaultTestConfig(t)
 	cfg.Domains = []string{domain}
 
-	var (
+	var deps *testDeps
-		w    *watcher.Watcher
-		deps *testDeps
-	)
 
 	livednstest.Retry(t, "watcher checks", func(ctx context.Context) error {
+		var w *watcher.Watcher
+
 		w, deps = newTestWatcher(t, cfg)
 
 		deps.state.SetDomainState(domain, &state.DomainState{
@@ -610,11 +602,6 @@ func TestDomainThatDoesNotExist(t *testing.T) {
 		t.Errorf("records saved for %s: %v", domain, hs.RecordsByNameserver)
 	}
 
-	if w.DNSUnreachable() {
-		t.Error("DNSUnreachable is true after a round that reached " +
-			"the .com servers")
-	}
-
 	assertNotified(t, deps, "NS Change: "+domain, "warning")
 
 	// That is the only notification, and it removes both nameservers,