resolver: error from ResolveIPAddresses when no nameserver answered (closes #190)
check / check (push) Failing after 2m9s
check / check (push) Failing after 2m9s
ResolveIPAddresses now returns an error, not no addresses, when no nameserver of the name's zone answered. A nameserver with status timeout or error is not an answer; one answer, even NXDOMAIN, is enough for an empty result without an error. When every server of a zone fails, FindAuthoritativeNameservers moves on to the parent name, whose servers only refer the query onward. Such a referral now has status error, so it is no answer either, and a hostname's saved records show it as error. The only caller, the nameserver address lookup, already keeps the previous addresses on an error; its comment no longer says the resolver hides this case. Model: opus-5-5
This commit is contained in:
@@ -488,8 +488,8 @@ reachability:
|
|||||||
| `error` | Query failed (timeout, SERVFAIL, REFUSED, network error) |
|
| `error` | Query failed (timeout, SERVFAIL, REFUSED, network error) |
|
||||||
|
|
||||||
A nameserver that answers NXDOMAIN or with no records has status `ok` and empty
|
A nameserver that answers NXDOMAIN or with no records has status `ok` and empty
|
||||||
`records`. A nameserver whose query failed has status `error`, empty `records`,
|
`records`. A nameserver whose query failed, or that only referred it to other
|
||||||
and the reason in `error`.
|
nameservers, has status `error`, empty `records`, and the reason in `error`.
|
||||||
|
|
||||||
`nameserverAddresses` lists, by nameserver, the sorted addresses its name
|
`nameserverAddresses` lists, by nameserver, the sorted addresses its name
|
||||||
resolves to. A state file without it loads, and the next check fills it in
|
resolves to. A state file without it loads, and the next check fills it in
|
||||||
|
|||||||
@@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-01: `ResolveIPAddresses` returns an error, not no addresses, when no
|
||||||
|
nameserver of the name's zone answered (closes #190).
|
||||||
- 2026-10-01: `make fmt` and `make fmt-check` cover Markdown with prettier, run
|
- 2026-10-01: `make fmt` and `make fmt-check` cover Markdown with prettier, run
|
||||||
in Docker at the version pinned by `yarn.lock` (closes #119).
|
in Docker at the version pinned by `yarn.lock` (closes #119).
|
||||||
- 2026-10-01: `make fmt-check` fails on a file `goimports` would change; both
|
- 2026-10-01: `make fmt-check` fails on a file `goimports` would change; both
|
||||||
|
|||||||
@@ -10,6 +10,11 @@ var (
|
|||||||
"no authoritative nameservers found",
|
"no authoritative nameservers found",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// ErrNoNameserverAnswered is returned when every nameserver
|
||||||
|
// asked about a name timed out, failed or returned a referral,
|
||||||
|
// so whether the name has addresses is unknown.
|
||||||
|
ErrNoNameserverAnswered = errors.New("no nameserver answered")
|
||||||
|
|
||||||
// ErrCNAMEDepthExceeded is returned when a CNAME chain
|
// ErrCNAMEDepthExceeded is returned when a CNAME chain
|
||||||
// exceeds MaxCNAMEDepth.
|
// exceeds MaxCNAMEDepth.
|
||||||
ErrCNAMEDepthExceeded = errors.New(
|
ErrCNAMEDepthExceeded = errors.New(
|
||||||
|
|||||||
@@ -11,6 +11,13 @@ func ExtractRecordValue(rr dns.RR) string {
|
|||||||
return extractRecordValue(rr)
|
return extractRecordValue(rr)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// CollectIPs exports collectIPs for testing.
|
||||||
|
func CollectIPs(
|
||||||
|
results map[string]*NameserverResponse,
|
||||||
|
) ([]string, string, error) {
|
||||||
|
return collectIPs(results)
|
||||||
|
}
|
||||||
|
|
||||||
// QueryEachNS exports queryEachNS for testing.
|
// QueryEachNS exports queryEachNS for testing.
|
||||||
func (r *Resolver) QueryEachNS(
|
func (r *Resolver) QueryEachNS(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
|
|||||||
@@ -516,6 +516,7 @@ type queryState struct {
|
|||||||
gotSERVFAIL bool
|
gotSERVFAIL bool
|
||||||
gotRefused bool
|
gotRefused bool
|
||||||
gotTimeout bool
|
gotTimeout bool
|
||||||
|
gotReferral bool
|
||||||
netErr error
|
netErr error
|
||||||
hasRecords bool
|
hasRecords bool
|
||||||
}
|
}
|
||||||
@@ -578,6 +579,18 @@ func (r *Resolver) querySingleType(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A reply with no answer that lists other nameservers, from a server
|
||||||
|
// that does not hold the name's zone, is a referral and says nothing
|
||||||
|
// about the name's records. A parent zone's servers send one when
|
||||||
|
// every server of the name's own zone failed and
|
||||||
|
// FindAuthoritativeNameservers moved on to the parent name.
|
||||||
|
if !msg.Authoritative && len(msg.Answer) == 0 &&
|
||||||
|
len(extractNSSet(msg.Ns)) > 0 {
|
||||||
|
state.gotReferral = true
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
collectAnswerRecords(msg, resp, state)
|
collectAnswerRecords(msg, resp, state)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -626,6 +639,9 @@ func classifyResponse(resp *NameserverResponse, state queryState) {
|
|||||||
case state.netErr != nil && !state.hasRecords:
|
case state.netErr != nil && !state.hasRecords:
|
||||||
resp.Status = StatusError
|
resp.Status = StatusError
|
||||||
resp.Error = "network error: " + state.netErr.Error()
|
resp.Error = "network error: " + state.netErr.Error()
|
||||||
|
case state.gotReferral && !state.hasRecords:
|
||||||
|
resp.Status = StatusError
|
||||||
|
resp.Error = "server returned a referral"
|
||||||
case !state.hasRecords && !state.gotNXDomain:
|
case !state.hasRecords && !state.gotNXDomain:
|
||||||
resp.Status = StatusNoData
|
resp.Status = StatusNoData
|
||||||
}
|
}
|
||||||
@@ -734,7 +750,9 @@ func (r *Resolver) LookupAllRecords(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ResolveIPAddresses resolves a hostname to all IPv4 and IPv6
|
// ResolveIPAddresses resolves a hostname to all IPv4 and IPv6
|
||||||
// addresses, following CNAME chains up to MaxCNAMEDepth.
|
// addresses, following CNAME chains up to MaxCNAMEDepth. When no
|
||||||
|
// nameserver of the name's zone answered, it returns an error rather
|
||||||
|
// than no addresses.
|
||||||
func (r *Resolver) ResolveIPAddresses(
|
func (r *Resolver) ResolveIPAddresses(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
hostname string,
|
hostname string,
|
||||||
@@ -760,7 +778,10 @@ func (r *Resolver) resolveIPWithCNAME(
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
ips, cnameTarget := collectIPs(results)
|
ips, cnameTarget, err := collectIPs(results)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("resolving %s: %w", hostname, err)
|
||||||
|
}
|
||||||
|
|
||||||
if len(ips) == 0 && cnameTarget != "" {
|
if len(ips) == 0 && cnameTarget != "" {
|
||||||
return r.resolveIPWithCNAME(ctx, cnameTarget, depth+1)
|
return r.resolveIPWithCNAME(ctx, cnameTarget, depth+1)
|
||||||
@@ -771,16 +792,28 @@ func (r *Resolver) resolveIPWithCNAME(
|
|||||||
return ips, nil
|
return ips, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// collectIPs returns the addresses in the nameservers' answers and the
|
||||||
|
// first CNAME target among them. It returns ErrNoNameserverAnswered when
|
||||||
|
// every nameserver timed out, failed or returned a referral: that is not
|
||||||
|
// a name with no addresses.
|
||||||
func collectIPs(
|
func collectIPs(
|
||||||
results map[string]*NameserverResponse,
|
results map[string]*NameserverResponse,
|
||||||
) ([]string, string) {
|
) ([]string, string, error) {
|
||||||
seen := make(map[string]bool)
|
seen := make(map[string]bool)
|
||||||
|
|
||||||
var ips []string
|
var ips []string
|
||||||
|
|
||||||
var cnameTarget string
|
var cnameTarget string
|
||||||
|
|
||||||
|
answered := false
|
||||||
|
|
||||||
for _, resp := range results {
|
for _, resp := range results {
|
||||||
|
if resp.Status == StatusTimeout || resp.Status == StatusError {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
answered = true
|
||||||
|
|
||||||
if resp.Status == StatusNXDomain {
|
if resp.Status == StatusNXDomain {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -804,5 +837,9 @@ func collectIPs(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return ips, cnameTarget
|
if !answered {
|
||||||
|
return nil, "", ErrNoNameserverAnswered
|
||||||
|
}
|
||||||
|
|
||||||
|
return ips, cnameTarget, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -5,10 +5,42 @@ import (
|
|||||||
|
|
||||||
"github.com/miekg/dns"
|
"github.com/miekg/dns"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
"sneak.berlin/go/dnswatcher/internal/resolver"
|
"sneak.berlin/go/dnswatcher/internal/resolver"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// TestCollectIPs_OneAnswerIsEnough checks that one nameserver answering
|
||||||
|
// NXDOMAIN says the name has no addresses, though the other timed out.
|
||||||
|
func TestCollectIPs_OneAnswerIsEnough(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
ips, _, err := resolver.CollectIPs(
|
||||||
|
map[string]*resolver.NameserverResponse{
|
||||||
|
"ns1.example.": {Status: resolver.StatusTimeout},
|
||||||
|
"ns2.example.": {Status: resolver.StatusNXDomain},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Empty(t, ips)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCollectIPs_FailedIsNoAnswer checks that nameservers that all have
|
||||||
|
// status error, from a refusal, a server failure, a network error or a
|
||||||
|
// referral, are no answer rather than a name with no addresses.
|
||||||
|
func TestCollectIPs_FailedIsNoAnswer(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
ips, _, err := resolver.CollectIPs(
|
||||||
|
map[string]*resolver.NameserverResponse{
|
||||||
|
"ns1.example.": {Status: resolver.StatusError},
|
||||||
|
"ns2.example.": {Status: resolver.StatusError},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
require.ErrorIs(t, err, resolver.ErrNoNameserverAnswered)
|
||||||
|
assert.Empty(t, ips)
|
||||||
|
}
|
||||||
|
|
||||||
func TestExtractRecordValue_LetterCase(t *testing.T) {
|
func TestExtractRecordValue_LetterCase(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -633,6 +633,82 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
|
|||||||
assert.NotEmpty(t, resp.Error)
|
assert.NotEmpty(t, resp.Error)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestCollectIPs_NoNameserverAnswered takes the response of a
|
||||||
|
// nameserver at 192.0.2.1, where nothing answers, as
|
||||||
|
// TestQueryNameserverIP_Timeout does. Addresses collected from
|
||||||
|
// nameservers that all failed to answer are an error, not none.
|
||||||
|
func TestCollectIPs_NoNameserverAnswered(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
r := newTestResolver(t)
|
||||||
|
|
||||||
|
// The deadline outlasts the first try, as in
|
||||||
|
// TestQueryNameserverIP_Timeout.
|
||||||
|
ctx, cancel := context.WithTimeout(
|
||||||
|
context.Background(), 3*time.Second,
|
||||||
|
)
|
||||||
|
t.Cleanup(cancel)
|
||||||
|
|
||||||
|
resp, err := r.QueryNameserverIP(
|
||||||
|
ctx, "unreachable.test.", "192.0.2.1",
|
||||||
|
"example.com",
|
||||||
|
)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
ips, _, err := resolver.CollectIPs(
|
||||||
|
map[string]*resolver.NameserverResponse{resp.Nameserver: resp},
|
||||||
|
)
|
||||||
|
require.ErrorIs(t, err, resolver.ErrNoNameserverAnswered)
|
||||||
|
assert.Empty(t, ips)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCollectIPs_ReferralIsNoAnswer asks a root server about
|
||||||
|
// example.com, which the root zone does not hold, so it only refers the
|
||||||
|
// query to the com servers. That reply is no answer, as is a parent
|
||||||
|
// zone's when every server of the name's own zone failed.
|
||||||
|
func TestCollectIPs_ReferralIsNoAnswer(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
r := newTestResolver(t)
|
||||||
|
|
||||||
|
var resp *resolver.NameserverResponse
|
||||||
|
|
||||||
|
livednstest.Retry(
|
||||||
|
t,
|
||||||
|
"QueryNameserverIP(a.root-servers.net, example.com)",
|
||||||
|
func(ctx context.Context) error {
|
||||||
|
var err error
|
||||||
|
|
||||||
|
resp, err = r.QueryNameserverIP(
|
||||||
|
ctx, "a.root-servers.net.", "198.41.0.4",
|
||||||
|
"example.com",
|
||||||
|
)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// A timeout or a network error is no reply at all.
|
||||||
|
if resp.Status == resolver.StatusTimeout ||
|
||||||
|
strings.HasPrefix(resp.Error, "network error") {
|
||||||
|
return fmt.Errorf(
|
||||||
|
"%w: %s", livednstest.ErrNoAnswer, resp.Error,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert.Equal(t, resolver.StatusError, resp.Status)
|
||||||
|
assert.Equal(t, "server returned a referral", resp.Error)
|
||||||
|
|
||||||
|
ips, _, err := resolver.CollectIPs(
|
||||||
|
map[string]*resolver.NameserverResponse{resp.Nameserver: resp},
|
||||||
|
)
|
||||||
|
require.ErrorIs(t, err, resolver.ErrNoNameserverAnswered)
|
||||||
|
assert.Empty(t, ips)
|
||||||
|
}
|
||||||
|
|
||||||
func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
|
func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -321,10 +321,9 @@ func (w *Watcher) detectNSChanges(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// resolveNameserverAddresses returns the sorted addresses each
|
// resolveNameserverAddresses returns the sorted addresses each
|
||||||
// nameserver's name resolves to. A nameserver whose lookup fails or
|
// nameserver's name resolves to. A nameserver whose lookup fails, as it
|
||||||
// finds no address keeps its addresses from prev: the resolver finds no
|
// does when no nameserver of the name's zone answers, or finds no
|
||||||
// address, without an error, when every server it asks times out, and
|
// address keeps its addresses from prev and is not an address change.
|
||||||
// that is not an address change.
|
|
||||||
func (w *Watcher) resolveNameserverAddresses(
|
func (w *Watcher) resolveNameserverAddresses(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
nameservers []string,
|
nameservers []string,
|
||||||
|
|||||||
Reference in New Issue
Block a user