-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NET-7813 - DNS : SERVFAIL when resolving PTR records #20679
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small non-blocking comment, but otherwise LGTM - thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
func TestDNS_ReverseLookup_NotFound(t *testing.T) { | ||
if testing.Short() { | ||
t.Skip("too slow for testing.Short") | ||
} | ||
|
||
for name, experimentsHCL := range getVersionHCL(true) { | ||
t.Run(name, func(t *testing.T) { | ||
// do not configure recursors | ||
a := NewTestAgent(t, experimentsHCL) | ||
defer a.Shutdown() | ||
testrpc.WaitForLeader(t, a.RPC, "dc1") | ||
|
||
// Do not register any nodes | ||
m := new(dns.Msg) | ||
qName := "2.0.0.127.in-addr.arpa." | ||
m.SetQuestion(qName, dns.TypeANY) | ||
|
||
c := new(dns.Client) | ||
in, _, err := c.Exchange(m, a.DNSAddr()) | ||
require.NoError(t, err) | ||
require.Nil(t, in.Answer) | ||
require.Nil(t, in.Extra) | ||
|
||
require.Equal(t, dns.RcodeNameError, in.Rcode) | ||
|
||
question := in.Question[0] | ||
require.Equal(t, qName, question.Name) | ||
require.Equal(t, dns.TypeANY, question.Qtype) | ||
require.Equal(t, uint16(dns.ClassINET), question.Qclass) | ||
|
||
soa, ok := in.Ns[0].(*dns.SOA) | ||
require.True(t, ok) | ||
require.Equal(t, "ns.consul.", soa.Ns) | ||
require.Equal(t, "hostmaster.consul.", soa.Mbox) | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Co-authored-by: Michael Zalimeni <michael.zalimeni@hashicorp.com>
* NET-7813 - DNS : SERVFAIL when resolving PTR records * Update agent/dns.go Co-authored-by: Michael Zalimeni <michael.zalimeni@hashicorp.com> * PR feedback --------- Co-authored-by: Michael Zalimeni <michael.zalimeni@hashicorp.com>
Description
For PTR records, the recursors enabled flag is not obeyed. It will return a SERVFAIL record in this case.
The acceptance criteria is:
NOTE: Some reverse lookup tests were moved from
dns_test.go
anddns_service_lookup_test.go
into the newdns_reverse_lookup_test.go
file so that we did not have to add the newTestDNS_ReverseLookup_NotFound()
test intodns_test.go
as it it already huge and has issues displaying in Github's UITesting & Reproduction steps
without recursors:
consul agent -dev
dig -p 8600 -x 127.0.0.3 @localhost
Expected Result:
with recursors array defined but without recursors enabled:
consul agent -dev -log-level=debug -hcl 'recursors = ["1.1.1.1"]'
Expected Result:
Links
PR Checklist