diff --git a/README.md b/README.md index 7fd92fa..b449313 100644 --- a/README.md +++ b/README.md @@ -355,6 +355,17 @@ 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 no lookup in that round got a reply from a root +server, because none of them answered or because 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 in which the +root servers answered gives `ok` even when a name's own nameservers do not +answer or a domain does not exist; those are reported as notifications. + 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 diff --git a/TODO.md b/TODO.md index f2674ea..fe97974 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,9 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7 # Completed Steps +- 2026-10-18: the health check answers 503 with `status` `error` when no lookup + in the most recent round of DNS checks got a reply from a root server, as on a + network that intercepts DNS queries (closes #302). - 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 diff --git a/internal/handlers/healthcheck.go b/internal/handlers/healthcheck.go index 7185944..6c4fa18 100644 --- a/internal/handlers/healthcheck.go +++ b/internal/handlers/healthcheck.go @@ -2,16 +2,25 @@ package handlers import ( "net/http" + + "sneak.berlin/go/dnswatcher/internal/healthcheck" ) -// HandleHealthCheck returns the health check handler. +// HandleHealthCheck returns the health check handler. It answers 503 +// 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, ) { - h.respondJSON( - writer, request, h.hc.Check(), http.StatusOK, - ) + resp := h.hc.Check() + + code := http.StatusOK + if resp.Status == healthcheck.StatusError { + code = http.StatusServiceUnavailable + } + + h.respondJSON(writer, request, resp, code) } } diff --git a/internal/handlers/healthcheck_test.go b/internal/handlers/healthcheck_test.go new file mode 100644 index 0000000..68090b9 --- /dev/null +++ b/internal/handlers/healthcheck_test.go @@ -0,0 +1,158 @@ +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) + } +} diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index 34c035a..5c0e000 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -11,6 +11,13 @@ 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. @@ -20,6 +27,7 @@ type Params struct { Globals *globals.Globals Config *config.Config Logger *logger.Logger + Watcher *watcher.Watcher } // Healthcheck provides health status information. @@ -61,10 +69,17 @@ func New( return healthcheck, nil } -// Check returns the current health status. +// Check returns the current health status. Status is StatusError when +// 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: "ok", + Status: status, Now: time.Now().UTC().Format(time.RFC3339Nano), UptimeSeconds: int64(h.uptime().Seconds()), UptimeHuman: h.uptime().String(), diff --git a/internal/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go index b87c3d1..fe9c9f1 100644 --- a/internal/healthcheck/healthcheck_test.go +++ b/internal/healthcheck/healthcheck_test.go @@ -3,10 +3,12 @@ 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" @@ -15,6 +17,12 @@ 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 @@ -30,12 +38,14 @@ 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. +// the fx lifecycle would set it. It follows the watcher it returns, +// 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 { +) (*healthcheck.Healthcheck, *watcher.Watcher) { t.Helper() g := &globals.Globals{Appname: "dnswatcher", Version: version} @@ -43,12 +53,21 @@ 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: &config.Config{MaintenanceMode: maintenance}, + Config: cfg, Logger: log, + Watcher: w, }) require.NoError(t, err) @@ -57,16 +76,53 @@ func newHealthcheck( require.NotNil(t, lifecycle.hooks[0].OnStart) require.NoError(t, lifecycle.hooks[0].OnStart(context.Background())) - return hc + return hc, w +} + +// 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(t, false, "v9.9.9") + hc, _ := newHealthcheck( + t, resolver.NewFromLogger(slog.Default()), false, "v9.9.9", + ) resp := hc.Check() - assert.Equal(t, "ok", resp.Status) + assert.Equal(t, healthcheck.StatusOK, 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 @@ -114,7 +170,10 @@ func TestCheckMaintenanceModeReflectsConfig(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - hc := newHealthcheck(t, tt.maintenance, "test") + hc, _ := newHealthcheck( + t, resolver.NewFromLogger(slog.Default()), tt.maintenance, + "test", + ) resp := hc.Check() assert.Equal(t, tt.maintenance, resp.Maintenance, "maintenanceMode must mirror Config.MaintenanceMode") @@ -125,10 +184,38 @@ func TestCheckMaintenanceModeReflectsConfig(t *testing.T) { func TestCheckSurfacesVersionAndAppname(t *testing.T) { t.Parallel() - hc := newHealthcheck(t, false, "surfaced-version-123") + hc, _ := newHealthcheck( + 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") +} diff --git a/internal/resolver/errors.go b/internal/resolver/errors.go index 5cf5b46..becc078 100644 --- a/internal/resolver/errors.go +++ b/internal/resolver/errors.go @@ -36,6 +36,12 @@ 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") diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index cfca76f..63ce592 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -293,7 +293,8 @@ func shuffled( // 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. +// ErrIntercepted. When the root servers gave no usable reply otherwise, +// it is ErrNoRootServerAnswered. func (r *Resolver) queryServers( ctx context.Context, servers []string, @@ -352,6 +353,10 @@ func (r *Resolver) queryServers( ) } + if zone == "." { + return nil, fmt.Errorf("%w: %w", ErrNoRootServerAnswered, lastErr) + } + return nil, fmt.Errorf("all servers failed: %w", lastErr) } @@ -528,7 +533,7 @@ func (r *Resolver) resolveNSIterative( } domain = dns.Fqdn(domain) - servers := rootServerList() + servers := r.rootServers zone := "." for range maxDelegation { @@ -592,7 +597,7 @@ func (r *Resolver) resolveARecord( } hostname = dns.Fqdn(hostname) - servers := rootServerList() + servers := r.rootServers zone := "." var withoutAddresses []string @@ -652,7 +657,7 @@ func (r *Resolver) FindAuthoritativeNameservers( ctx context.Context, domain string, ) ([]string, error) { - return r.findAuthoritativeNameservers(ctx, domain, rootServerList()) + return r.findAuthoritativeNameservers(ctx, domain, r.rootServers) } // findAuthoritativeNameservers is FindAuthoritativeNameservers with each @@ -1127,7 +1132,7 @@ func (r *Resolver) LookupNS( } return r.followDelegation( - ctx, dns.Fqdn(strings.ToLower(domain)), ".", nil, rootServerList(), + ctx, dns.Fqdn(strings.ToLower(domain)), ".", nil, r.rootServers, ) } diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 1bdfdb9..4f7b2dd 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -43,10 +43,12 @@ 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 + log *slog.Logger + client DNSClient + tcp DNSClient + rootServers []string } // New creates a new Resolver instance for use with uber/fx. @@ -55,9 +57,10 @@ func New( params Params, ) (*Resolver, error) { return &Resolver{ - log: params.Logger.Get(), - client: &udpClient{timeout: queryTimeoutDuration}, - tcp: &tcpClient{timeout: queryTimeoutDuration}, + log: params.Logger.Get(), + client: &udpClient{timeout: queryTimeoutDuration}, + tcp: &tcpClient{timeout: queryTimeoutDuration}, + rootServers: rootServerList(), }, nil } @@ -65,10 +68,22 @@ func New( // useful for testing without the fx lifecycle. func NewFromLogger(log *slog.Logger) *Resolver { return &Resolver{ - log: log, - client: &udpClient{timeout: queryTimeoutDuration}, - tcp: &tcpClient{timeout: queryTimeoutDuration}, + 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. diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index f67fb06..0e47010 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -815,6 +815,7 @@ 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() @@ -848,6 +849,34 @@ 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 diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 297d505..ff1107e 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -15,14 +15,21 @@ 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, minus the watcher/resolver subtree that -// would touch live DNS, plus the given option. config.New reads viper, -// so the caller must first configure it, which is also why the caller +// as cmd/dnswatcher wires it, plus the given option. The health check +// follows the watcher, so the watcher is in the app too. Starting the +// app starts the watcher's first round of checks, against live DNS, +// 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( @@ -33,11 +40,29 @@ 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, ) } diff --git a/internal/watcher/dnsunreachable_test.go b/internal/watcher/dnsunreachable_test.go new file mode 100644 index 0000000..76d1c08 --- /dev/null +++ b/internal/watcher/dnsunreachable_test.go @@ -0,0 +1,65 @@ +package watcher_test + +import ( + "log/slog" + "testing" + + "sneak.berlin/go/dnswatcher/internal/resolver" + "sneak.berlin/go/dnswatcher/internal/watcher" +) + +// TestDNSUnreachableWhenNoRootServerReplies 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 no lookup got a reply from a root server, so the round could not +// reach the DNS. TestFirstRunBaseline and TestDomainThatDoesNotExist +// check rounds in which the root servers answered. +func TestDNSUnreachableWhenNoRootServerReplies(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 lookup got a reply from a root server") + } + }) + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index f0f26ff..8d7c67b 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -11,6 +11,7 @@ import ( "sort" "strconv" "strings" + "sync/atomic" "time" "go.uber.org/fx" @@ -62,6 +63,11 @@ 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. @@ -197,6 +203,18 @@ 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: no lookup in it got a reply from a +// root server, because none of them answered or because the network +// intercepts DNS queries. A round in which the root servers answered +// reached the DNS, even when a name's own nameservers did not answer or +// a domain does not exist. 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 || @@ -265,15 +283,40 @@ 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. +// checks operate on current IP addresses. Unless ctx was cut short, it +// 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 { - w.checkDomain(ctx, domain) + errs = append(errs, w.checkDomain(ctx, domain)) } for _, hostname := range w.config.Hostnames { - w.checkHostname(ctx, hostname) + _, err := w.checkHostname(ctx, hostname) + errs = append(errs, err) } + + if ctx.Err() == nil { + w.dnsUnreachable.Store(noRootServerReplied(errs)) + } +} + +// noRootServerReplied reports whether errs, the errors of the lookups in +// a round of DNS checks, nil for a lookup that succeeded, show that no +// lookup got a reply from a root server: each failed because none of +// them answered, or because the network intercepts DNS queries. A round +// with no lookups shows nothing. +func noRootServerReplied(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 @@ -292,14 +335,16 @@ func (w *Watcher) logFailedLookup( } // checkDomain looks up domain's delegation and checks domain with it -// (see checkDomainResult). +// (see checkDomainResult). It returns the lookup's error. 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, @@ -358,7 +403,7 @@ func (w *Watcher) checkDomainResult( // check, as detectNSDisappearances needs. if nxdomain { w.state.DeleteHostnameState(domain) - } else if !w.checkHostname(ctx, domain) { + } else if saved, _ := w.checkHostname(ctx, domain); !saved { return } @@ -484,11 +529,12 @@ func (w *Watcher) detectNSAddressChanges( } // checkHostname looks up hostname's records and checks them (see -// checkHostnameResult). It reports whether it saved them. +// checkHostnameResult). It reports whether it saved them, and returns +// the lookup's error. func (w *Watcher) checkHostname( ctx context.Context, hostname string, -) bool { +) (bool, error) { results, err := w.resolver.LookupAllRecords(ctx, hostname) if err != nil { w.logFailedLookup( @@ -499,7 +545,7 @@ func (w *Watcher) checkHostname( ) } - return w.checkHostnameResult(ctx, hostname, results, err) + return w.checkHostnameResult(ctx, hostname, results, err), err } // checkHostnameResult finishes a check of hostname from results and err, diff --git a/internal/watcher/watcher_test.go b/internal/watcher/watcher_test.go index 79feb23..5f5306f 100644 --- a/internal/watcher/watcher_test.go +++ b/internal/watcher/watcher_test.go @@ -385,11 +385,16 @@ func TestFirstRunBaseline(t *testing.T) { cfg.Domains = []string{testSmallDomain} cfg.Hostnames = []string{testHost} - _, deps := runChecks(t, cfg, nil) + w, 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. @@ -551,7 +556,9 @@ 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. +// records are removed rather than asked for at the .com servers. The +// 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() @@ -560,11 +567,12 @@ func TestDomainThatDoesNotExist(t *testing.T) { cfg := defaultTestConfig(t) cfg.Domains = []string{domain} - 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) deps.state.SetDomainState(domain, &state.DomainState{ @@ -602,6 +610,11 @@ 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,