test: restore transport-failure coverage with loopback nameservers
check / check (push) Failing after 13s
check / check (push) Failing after 13s
The DNS-mock removal deleted TestQueryNameserverIP_Timeout and left a comment in its place, so the resolver's StatusTimeout / StatusError classification branch went untested. The stated obstacle was that a query to a black-holed RFC 5737 address comes back StatusOK, because the build environment transparently intercepts UDP/53 and answers it locally. That is a property of that environment, not of the resolver, and it only rules out choosing a remote address. internal/resolver/transport_test.go binds real nameservers on 127.0.0.1 instead and aims the query at them: one silent on A queries and answering every other type (StatusTimeout), one answering SERVFAIL (StatusError), and one address with nothing listening, which is refused rather than dropped and so classifies as NoData. This is not a mock — no DNSClient is substituted. The resolver dials a real socket, writes a real query with the real miekg/dns client, and applies its real deadline and real classification logic to what comes back. Substituting the client is what TESTING.md bans; choosing the server is not, and the resolver is aimed at a caller-chosen nameserver in production too. queryDNS now dials a nameserver address that already carries a port as written, defaulting to 53 only for a bare address. That is what makes a nameserver on any other port reachable, on loopback or otherwise. Silence on one record type rather than all eight keeps the timeout test to two query timeouts (4s) instead of sixteen (32s), and it is asserted: the test fails if it ever costs more than 8s. The watcher's assertStatePopulated and TestDomainPortAndTLSChecks asserted only that hostname, port and certificate state were non-empty, plus non-zero checker call counts. Neither was vacuous, but neither would have caught the watcher resolving the wrong addresses. The port and TLS test doubles now record their arguments, and both tests assert that the state keys and the arguments the checkers were actually called with match the addresses live DNS returned — exactly those, no more and no fewer. Verified by mutation: making the watcher drop all but one resolved address fails both tests, and it passed both of them before. TESTING.md records why a loopback nameserver is not a mock, so the new tests are not mistaken for a violation of the rule they respect.
This commit is contained in:
@@ -20,6 +20,10 @@ const (
|
||||
minDomainLabels = 2
|
||||
)
|
||||
|
||||
// defaultDNSPort is the port a nameserver is assumed to listen on
|
||||
// when its address does not carry one.
|
||||
const defaultDNSPort = "53"
|
||||
|
||||
// ErrRefused is returned when a DNS server refuses a query.
|
||||
var ErrRefused = errors.New("dns query refused")
|
||||
|
||||
@@ -106,6 +110,19 @@ func (r *Resolver) retryTCP(
|
||||
return resp
|
||||
}
|
||||
|
||||
// nameserverAddr renders a nameserver address for dialling. A bare
|
||||
// address — the normal case, and what a delegation's glue records
|
||||
// carry — is given the default DNS port. An address that already
|
||||
// specifies a port is dialled as written, which is what makes a
|
||||
// nameserver listening somewhere other than 53 reachable.
|
||||
func nameserverAddr(nsIP string) string {
|
||||
if _, _, err := net.SplitHostPort(nsIP); err == nil {
|
||||
return nsIP
|
||||
}
|
||||
|
||||
return net.JoinHostPort(nsIP, defaultDNSPort)
|
||||
}
|
||||
|
||||
// queryDNS sends a DNS query to a specific server IP.
|
||||
// Tries non-recursive first, falls back to recursive on
|
||||
// REFUSED (handles DNS interception environments).
|
||||
@@ -120,7 +137,7 @@ func (r *Resolver) queryDNS(
|
||||
}
|
||||
|
||||
name = dns.Fqdn(name)
|
||||
addr := net.JoinHostPort(serverIP, "53")
|
||||
addr := nameserverAddr(serverIP)
|
||||
|
||||
msg := new(dns.Msg)
|
||||
msg.SetQuestion(name, qtype)
|
||||
|
||||
Reference in New Issue
Block a user