From 817df7fd34f77a4457168dcd6db3344a8e8870a0 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 13 Oct 2026 21:16:31 +0000 Subject: [PATCH] watcher: a hostname whose zone's nameservers all stop answering gets NS Failure (closes #284) When none of the nameservers of a hostname's zone answered, the walk to them failed and the check was only logged. The walk now returns a NoAnswerError naming the nameservers of the zone it reached when none of them answers. When those are the nameservers the previous check saved for the hostname, each is saved as failed with the reason, as a domain's are, so it gets an NS Failure and later an NS Recovery. A walk that fails before that zone, as when the host loses its network, is still only logged. Not tested live: the resolver naming a zone's nameservers that never answer, as no live zone has such nameservers; the watcher test builds that error. Model: opus-5-5 --- README.md | 8 +- TODO.md | 2 + internal/resolver/errors.go | 17 +++++ internal/resolver/iterative.go | 19 ++++- internal/resolver/resolver_test.go | 7 +- internal/watcher/export_test.go | 8 ++ internal/watcher/interfaces.go | 5 +- internal/watcher/nsfailure_test.go | 115 +++++++++++++++++++++++++++++ internal/watcher/watcher.go | 46 +++++++++++- 9 files changed, 216 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 1bcbf1c..bd9ba9e 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,13 @@ notification endpoint set, changes show only on the dashboard; see trace moves from a name to its parent only when the servers asked answer that the name has no delegation of its own, or does not exist. When they do not answer, the check fails and the hostname's records from the previous check are - kept. + kept, unless they are the nameservers the previous check saved for the + hostname. Each of those is then saved as failed, with the reason, as a + domain's nameservers are when none of them answers, so it gets an NS query + failure notification, and an NS recovery notification when it answers again + (see below). A trace that gets no answer before it reaches them, as when the + host running dnswatcher loses its network, fails the check without a + notification. - Queries **each** authoritative nameserver independently for **all** record types: A, AAAA, CNAME, MX, TXT, SRV, CAA, NS. - Each record type is a query of its own. When a nameserver answers some types diff --git a/TODO.md b/TODO.md index 0ebda04..a774565 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7 # Completed Steps +- 2026-10-13: a hostname whose zone's nameservers all stop answering gets an NS + Failure for each, and an NS Recovery when they answer again (closes #284). - 2026-10-13: an IPv6 address is written in brackets before its port, as in `[2001:db8::1]:443`, in keys and Port Change notifications (closes #270). - 2026-10-13: `TODO.md` names only open issues as next and future steps, and its diff --git a/internal/resolver/errors.go b/internal/resolver/errors.go index b628dd9..8c53adb 100644 --- a/internal/resolver/errors.go +++ b/internal/resolver/errors.go @@ -2,6 +2,23 @@ package resolver import "errors" +// NoAnswerError is returned when the walk to a name's nameservers +// reached a zone through a referral and none of that zone's +// nameservers, Nameservers as the referral names them, answered. Err +// says why they did not. +type NoAnswerError struct { + Nameservers []string + Err error +} + +func (e *NoAnswerError) Error() string { + return e.Err.Error() +} + +func (e *NoAnswerError) Unwrap() error { + return e.Err +} + // Sentinel errors returned by the resolver. var ( // ErrNoNameservers is returned when no authoritative NS diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index 35df296..da961ac 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -209,16 +209,19 @@ func glueIPs(nsNames []string, glue map[string][]net.IP) []string { // of the zone domain is in answer that domain does not exist, the error // is ErrNXDomain. When they answer that it has no delegation of its own, // because it is not the zone's apex, the set is empty and there is no -// error. Any other error means that no such answer came. +// error. Any other error means that no such answer came; it is a +// *NoAnswerError when the servers that did not answer are those of a +// zone a referral named. func (r *Resolver) followDelegation( ctx context.Context, domain string, servers []string, ) ([]string, error) { - // servers are the root servers, the servers of zone ".". + // servers are the root servers, the servers of zone ".". After a + // referral, nameservers are the names of the servers of zone. zone := "." - var withoutAddresses []string + var nameservers, withoutAddresses []string for range maxDelegation { if checkCtx(ctx) != nil { @@ -228,6 +231,12 @@ func (r *Resolver) followDelegation( resp, err := r.queryZone( ctx, servers, withoutAddresses, zone, domain, dns.TypeNS, 0, ) + + // A query that ctx cut short says nothing about the servers. + if err != nil && len(nameservers) > 0 && checkCtx(ctx) == nil { + return nil, &NoAnswerError{Nameservers: nameservers, Err: err} + } + if err != nil { return nil, err } @@ -256,6 +265,7 @@ func (r *Resolver) followDelegation( return r.resolveNSIterative(ctx, domain) } + nameservers = authNS servers, withoutAddresses = referralNameservers(resp) zone = referralZone(resp) } @@ -626,7 +636,8 @@ func (r *Resolver) resolveARecord( // them. When the servers asked answer that the name has no delegation // of its own, or does not exist, it tries each parent name in turn, so // it returns the nameservers of the zone the name is in. When they do -// not answer, it returns the error and tries no parent name. +// not answer, it returns the error and tries no parent name; when a +// referral named them, the error is a *NoAnswerError naming them. func (r *Resolver) FindAuthoritativeNameservers( ctx context.Context, domain string, diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 58033b7..d7e11e0 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -121,7 +121,9 @@ func TestFindAuthoritativeNameservers_DelegatedSubdomain( // delegation of its own, so the lookup returns that walk's error, about // www.google.com, and tries no parent name: trying google.com and com // would end in ErrNoNameservers, or in the error of a walk for one of -// them. +// them. The servers that did not answer are those the walk started at, +// which no referral named, so the error is no NoAnswerError, as when +// the host running dnswatcher has lost its network. func TestFindAuthoritativeNameservers_NoAnswer(t *testing.T) { t.Parallel() @@ -134,6 +136,9 @@ func TestFindAuthoritativeNameservers_NoAnswer(t *testing.T) { require.NotErrorIs(t, err, resolver.ErrNoNameservers) assert.Contains(t, err.Error(), "query www.google.com. @"+noAnswerAddress) assert.Empty(t, nameservers) + + var noAnswer *resolver.NoAnswerError + assert.NotErrorAs(t, err, &noAnswer) } func TestFindAuthoritativeNameservers_ReturnsSorted( diff --git a/internal/watcher/export_test.go b/internal/watcher/export_test.go index a3448e9..5049f15 100644 --- a/internal/watcher/export_test.go +++ b/internal/watcher/export_test.go @@ -122,6 +122,14 @@ func (w *Watcher) RunTLSChecks(ctx context.Context) { w.runTLSChecks(ctx) } +// FailedNameservers exports failedNameservers for testing. +func FailedNameservers( + err error, + prev *state.HostnameState, +) map[string]*resolver.NameserverResponse { + return failedNameservers(err, prev) +} + // BuildHostnameState exports buildHostnameState for testing. func BuildHostnameState( results map[string]*resolver.NameserverResponse, diff --git a/internal/watcher/interfaces.go b/internal/watcher/interfaces.go index 166ada4..c92d8d6 100644 --- a/internal/watcher/interfaces.go +++ b/internal/watcher/interfaces.go @@ -20,7 +20,10 @@ type DNSResolver interface { ) ([]string, error) // LookupAllRecords queries all record types for a hostname, - // returning each nameserver's response keyed by nameserver. + // returning each nameserver's response keyed by nameserver. When + // the walk to the nameservers reached a zone through a referral and + // none of that zone's nameservers answered, the error is a + // *resolver.NoAnswerError naming them. LookupAllRecords( ctx context.Context, hostname string, diff --git a/internal/watcher/nsfailure_test.go b/internal/watcher/nsfailure_test.go index e9147c1..d6c78e3 100644 --- a/internal/watcher/nsfailure_test.go +++ b/internal/watcher/nsfailure_test.go @@ -2,6 +2,7 @@ package watcher_test import ( "context" + "errors" "fmt" "log/slog" "strings" @@ -14,6 +15,10 @@ import ( "sneak.berlin/go/dnswatcher/internal/watcher" ) +// errNoReply is why a walk that got no reply from a zone's servers +// failed. +var errNoReply = errors.New("all servers failed: i/o timeout") + // answered is what a check saves for a nameserver that answered with // these records. func answered(records map[string][]string) *state.NameserverRecordState { @@ -183,6 +188,116 @@ func TestNSFailureAlertNamesHostnameNameserverAndReason(t *testing.T) { } } +// TestHostnameWhoseNameserversAllStopAnswering starts from a check of +// host in which nsA and nsB answered, then gives the watcher the error +// the walk to host's nameservers returns when it reached their zone and +// neither answered. No live zone has nameservers that never answer, so +// the error is built here. Both are saved as failed, with the reason, +// and each gets an NS Failure. On the next check both answer, and each +// gets an NS Recovery. +func TestHostnameWhoseNameserversAllStopAnswering(t *testing.T) { + t.Parallel() + + cfg := defaultTestConfig(t) + cfg.Hostnames = []string{host} + + // The watcher is given each check's lookup result, so it has no + // resolver. + deps := newTestDeps(t, cfg) + w := watcher.NewForTest( + cfg, deps.state, nil, + deps.portChecker, deps.tlsChecker, deps.notifier, + ) + w.SetFirstRun(false) + + records := map[string][]string{"A": {ip1}} + bothAnswer := map[string]*resolver.NameserverResponse{ + nsA: response(records), nsB: response(records), + } + + deps.state.SetHostnameState( + host, watcher.BuildHostnameState(bothAnswer, nil, time.Now()), + ) + + err := &resolver.NoAnswerError{ + Nameservers: []string{nsA, nsB}, Err: errNoReply, + } + + prev, _ := deps.state.GetHostnameState(host) + w.UpdateHostnameState(t.Context(), host, watcher.BuildHostnameState( + watcher.FailedNameservers(err, prev), prev, time.Now(), + )) + + current, _ := deps.state.GetHostnameState(host) + + for _, ns := range []string{nsA, nsB} { + got := current.RecordsByNameserver[ns] + if got.Status != failed().Status || got.Error != err.Error() { + t.Errorf( + "%s saved with status %q, error %q; want %q, %q", + ns, got.Status, got.Error, failed().Status, err.Error(), + ) + } + } + + if n := countNotifications(deps, "NS Failure: "+host); n != 2 { + t.Errorf("sent %d NS Failure notifications, want 2", n) + } + + w.UpdateHostnameState(t.Context(), host, watcher.BuildHostnameState( + bothAnswer, current, time.Now(), + )) + + if n := countNotifications(deps, "NS Recovery: "+host); n != 2 { + t.Errorf("sent %d NS Recovery notifications, want 2", n) + } +} + +// TestFailedNameserversOnlyForThoseOfThePreviousCheck gives +// FailedNameservers lookup errors after a check in which nsA and nsB +// answered. Only an error that names both makes them failed. +func TestFailedNameserversOnlyForThoseOfThePreviousCheck(t *testing.T) { + t.Parallel() + + records := map[string][]string{"A": {ip1}} + bothAnswered := saved(map[string]*state.NameserverRecordState{ + nsA: answered(records), nsB: answered(records), + }) + noAnswer := func(nameservers ...string) error { + return &resolver.NoAnswerError{Nameservers: nameservers, Err: errNoReply} + } + + tests := []struct { + name string + err error + prev *state.HostnameState + wantFailed bool + }{ + {"both did not answer", noAnswer(nsA, nsB), bothAnswered, true}, + { + "the servers of a parent zone did not answer", + noAnswer(nsC), bothAnswered, false, + }, + { + "the zone's delegation lists only nsA now", + noAnswer(nsA), bothAnswered, false, + }, + {"no root server answered", errNoReply, bothAnswered, false}, + {"no previous check", noAnswer(nsA, nsB), nil, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := watcher.FailedNameservers(tt.err, tt.prev) + if (got != nil) != tt.wantFailed { + t.Errorf("returned %v, want failed: %v", got, tt.wantFailed) + } + }) + } +} + // TestNameserverThatNeverAnswers asks a nameserver address where // nothing answers, 192.0.2.1, and checks what the watcher saves for it. // The deadline outlasts the resolver's first two-second try, as in the diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 45962b9..1632e87 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -465,12 +465,15 @@ func (w *Watcher) detectNSAddressChanges( } // checkHostname checks hostname's records and reports whether it saved -// them; it does not when their lookup fails. +// them; it does not when their lookup fails, unless the nameservers that +// did not answer are those of the previous check (see failedNameservers). func (w *Watcher) checkHostname( ctx context.Context, hostname string, ) bool { results, err := w.resolver.LookupAllRecords(ctx, hostname) + prev, _ := w.state.GetHostnameState(hostname) + if err != nil { w.logFailedLookup( ctx, @@ -479,11 +482,12 @@ func (w *Watcher) checkHostname( "error", err, ) - return false + results = failedNameservers(err, prev) + if results == nil { + return false + } } - prev, _ := w.state.GetHostnameState(hostname) - w.updateHostnameState( ctx, hostname, buildHostnameState(results, prev, time.Now().UTC()), ) @@ -491,6 +495,40 @@ func (w *Watcher) checkHostname( return true } +// failedNameservers returns a failed response for each nameserver in +// prev, a name's state from the previous check, with err as the reason, +// when err says that the walk to the name's nameservers reached their +// zone and none of them answered. For any other error, as when the walk +// got no answer before it reached their zone, or when prev is nil, it +// returns nil. +func failedNameservers( + err error, + prev *state.HostnameState, +) map[string]*resolver.NameserverResponse { + var noAnswer *resolver.NoAnswerError + if !errors.As(err, &noAnswer) || prev == nil { + return nil + } + + saved := slices.Sorted(maps.Keys(prev.RecordsByNameserver)) + if !slices.Equal(noAnswer.Nameservers, saved) { + return nil + } + + results := make(map[string]*resolver.NameserverResponse) + + for _, ns := range saved { + results[ns] = &resolver.NameserverResponse{ + Nameserver: ns, + Records: make(map[string][]string), + Status: resolver.StatusError, + Error: err.Error(), + } + } + + return results +} + // updateHostnameState finishes a check of hostname from newState, built // from its nameservers' answers: it follows the CNAME in them, notifies // what changed since the previous check, and saves newState.