resolver: error from ResolveIPAddresses when no nameserver answered (closes #190)
check / check (push) Failing after 2m25s
check / check (push) Failing after 2m25s
ResolveIPAddresses now returns an error wrapping ErrNoNameserverAnswered when every nameserver of the name's zone timed out or failed, instead of no addresses and no error. It reads each nameserver's status as the lookup already sets it; one answer, even NXDOMAIN, is enough for an empty result without an 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:
@@ -20,6 +20,8 @@ https://git.eeqj.de/sneak/dnswatcher/issues/149
|
||||
|
||||
# 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: a hostname is queried at the servers of the zone it is in, found
|
||||
by following delegations for the name, not its last two labels (closes #189).
|
||||
- 2026-10-01: each nameserver's addresses are saved with its domain, and a
|
||||
|
||||
@@ -10,6 +10,11 @@ var (
|
||||
"no authoritative nameservers found",
|
||||
)
|
||||
|
||||
// ErrNoNameserverAnswered is returned when every nameserver
|
||||
// asked about a name timed out or failed, so whether the name
|
||||
// has addresses is unknown.
|
||||
ErrNoNameserverAnswered = errors.New("no nameserver answered")
|
||||
|
||||
// ErrCNAMEDepthExceeded is returned when a CNAME chain
|
||||
// exceeds MaxCNAMEDepth.
|
||||
ErrCNAMEDepthExceeded = errors.New(
|
||||
|
||||
@@ -11,6 +11,13 @@ func ExtractRecordValue(rr dns.RR) string {
|
||||
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.
|
||||
func (r *Resolver) QueryEachNS(
|
||||
ctx context.Context,
|
||||
|
||||
@@ -734,7 +734,9 @@ func (r *Resolver) LookupAllRecords(
|
||||
}
|
||||
|
||||
// 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 wrapping
|
||||
// ErrNoNameserverAnswered rather than no addresses.
|
||||
func (r *Resolver) ResolveIPAddresses(
|
||||
ctx context.Context,
|
||||
hostname string,
|
||||
@@ -760,7 +762,10 @@ func (r *Resolver) resolveIPWithCNAME(
|
||||
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 != "" {
|
||||
return r.resolveIPWithCNAME(ctx, cnameTarget, depth+1)
|
||||
@@ -771,16 +776,28 @@ func (r *Resolver) resolveIPWithCNAME(
|
||||
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 or failed: that is not a name with no
|
||||
// addresses.
|
||||
func collectIPs(
|
||||
results map[string]*NameserverResponse,
|
||||
) ([]string, string) {
|
||||
) ([]string, string, error) {
|
||||
seen := make(map[string]bool)
|
||||
|
||||
var ips []string
|
||||
|
||||
var cnameTarget string
|
||||
|
||||
answered := false
|
||||
|
||||
for _, resp := range results {
|
||||
if resp.Status == StatusTimeout || resp.Status == StatusError {
|
||||
continue
|
||||
}
|
||||
|
||||
answered = true
|
||||
|
||||
if resp.Status == StatusNXDomain {
|
||||
continue
|
||||
}
|
||||
@@ -804,5 +821,9 @@ func collectIPs(
|
||||
}
|
||||
}
|
||||
|
||||
return ips, cnameTarget
|
||||
if !answered {
|
||||
return nil, "", ErrNoNameserverAnswered
|
||||
}
|
||||
|
||||
return ips, cnameTarget, nil
|
||||
}
|
||||
|
||||
@@ -5,10 +5,26 @@ import (
|
||||
|
||||
"github.com/miekg/dns"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"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)
|
||||
}
|
||||
|
||||
func TestExtractRecordValue_LetterCase(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -633,6 +633,35 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
|
||||
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)
|
||||
}
|
||||
|
||||
func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -321,10 +321,9 @@ func (w *Watcher) detectNSChanges(
|
||||
}
|
||||
|
||||
// resolveNameserverAddresses returns the sorted addresses each
|
||||
// nameserver's name resolves to. A nameserver whose lookup fails or
|
||||
// finds no address keeps its addresses from prev: the resolver finds no
|
||||
// address, without an error, when every server it asks times out, and
|
||||
// that is not an address change.
|
||||
// nameserver's name resolves to. A nameserver whose lookup fails, as it
|
||||
// does when no nameserver of the name's zone answers, or finds no
|
||||
// address keeps its addresses from prev and is not an address change.
|
||||
func (w *Watcher) resolveNameserverAddresses(
|
||||
ctx context.Context,
|
||||
nameservers []string,
|
||||
|
||||
Reference in New Issue
Block a user