luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Don't add A records for NS requests, because the record being returned already resolves correctly. Also fixed all the unit tests, and ignored hostnames that don't meet valid dns hostname criteria

This commit is contained in:
Preetha Appan 2017-08-04 12:17:04 -05:00 committed by Frank Schroeder
parent 450d8a69b5
commit c7c4100503
No known key found for this signature in database
GPG Key ID: 4D65C6EAEC87DECD
2 changed files with 93 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
agent/dns.go
View File

@ -236,9 +236,9 @@ func (d *DNSServer) handleQuery(resp dns.ResponseWriter, req *dns.Msg) {
m.SetRcode(req, dns.RcodeSuccess)
case dns.TypeNS:
ns, glue := d.nameservers()
ns, _ := d.nameservers()
m.Answer = ns
m.Extra = glue
// no need to send A records with the IP address, since the ns record is a node name that resolves correctly
m.SetRcode(req, dns.RcodeSuccess)
default:
@ -295,7 +295,12 @@ func (d *DNSServer) nameservers() (ns []dns.RR, extra []dns.RR) {
// name is "name.dc" and domain is "consul."
// we want "name.node.dc.consul."
lastdot := strings.LastIndexByte(name, '.')
fqdn := name[:lastdot] + ".node" + name[lastdot:] + "." + d.domain
nodeName := name[:lastdot]
if InvalidDnsRe.MatchString(nodeName) {
d.logger.Printf("[WARN] dns: Node name %q is not a valid dns host name, will not be added to NS record", nodeName)
continue
}
fqdn := nodeName + ".node" + name[lastdot:] + "." + d.domain
// create a consistent, unique and sanitized name for the server
fqdn = dns.Fqdn(strings.ToLower(fqdn))

182
agent/dns_test.go
View File

@ -168,7 +168,7 @@ func TestDNS_NodeLookup(t *testing.T) {
t.Fatalf("err: %v", err)
}
if len(in.Ns) != 2 {
if len(in.Ns) != 1 {
t.Fatalf("Bad: %#v %#v", in, len(in.Answer))
}
@ -180,13 +180,6 @@ func TestDNS_NodeLookup(t *testing.T) {
t.Fatalf("Bad: %#v", in.Ns[0])
}
nsRec, ok := in.Ns[1].(*dns.NS)
if !ok {
t.Fatalf("Bad: %#v", in.Ns[1])
}
if nsRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Ns[1])
}
}
func TestDNS_CaseInsensitiveNodeLookup(t *testing.T) {
@ -627,7 +620,7 @@ func TestDNS_ServiceLookup(t *testing.T) {
t.Fatalf("err: %v", err)
}
if len(in.Ns) != 2 {
if len(in.Ns) != 1 {
t.Fatalf("Bad: %#v", in)
}
@ -639,13 +632,6 @@ func TestDNS_ServiceLookup(t *testing.T) {
t.Fatalf("Bad: %#v", in.Ns[0])
}
nsRec, ok := in.Ns[1].(*dns.NS)
if !ok {
t.Fatalf("Bad: %#v", in.Ns[1])
}
if nsRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Ns[1])
}
}
}
@ -706,10 +692,6 @@ func TestDNS_ServiceLookupWithInternalServiceAddress(t *testing.T) {
Hdr: dns.RR_Header{Name: "db.service.consul.", Rrtype: 0x1, Class: 0x1, Rdlength: 0x4},
A: []byte{0x7f, 0x0, 0x0, 0x1}, // 127.0.0.1
},
&dns.A{
Hdr: dns.RR_Header{Name: "server-my-test-node-dc1.consul.", Rrtype: 0x1, Class: 0x1, Rdlength: 0x4},
A: []byte{0x7f, 0x0, 0x0, 0x1}, // 127.0.0.1
},
}
verify.Values(t, "extra", in.Extra, wantExtra)
}
@ -864,7 +846,7 @@ func TestDNS_ExternalServiceToConsulCNAMELookup(t *testing.T) {
t.Fatalf("Bad: %#v", in.Answer[0])
}
if len(in.Extra) != 3 {
if len(in.Extra) != 2 {
t.Fatalf("Bad: %#v", in)
}
@ -896,22 +878,59 @@ func TestDNS_ExternalServiceToConsulCNAMELookup(t *testing.T) {
t.Fatalf("Bad: %#v", in.Extra[1])
}
aRec2, ok := in.Extra[2].(*dns.A)
if !ok {
t.Fatalf("Bad: %#v", in.Extra[2])
}
if aRec2.Hdr.Name != "server-test-node-dc1.consul." {
t.Fatalf("Bad: %#v", in.Extra[2])
}
if aRec2.A.String() != "127.0.0.1" {
t.Fatalf("Bad: %#v", in.Extra[2])
}
if aRec2.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Extra[2])
}
}
}
func TestDNS_NSRecords(t *testing.T) {
t.Parallel()
cfg := TestConfig()
cfg.Domain = "CONSUL."
cfg.NodeName = "foo"
a := NewTestAgent(t.Name(), cfg)
defer a.Shutdown()
// Register node
args := &structs.RegisterRequest{
Datacenter: "dc1",
Node: "foo",
Address: "127.0.0.1",
TaggedAddresses: map[string]string{
"wan": "127.0.0.2",
},
}
var out struct{}
if err := a.RPC("Catalog.Register", args, &out); err != nil {
t.Fatalf("err: %v", err)
}
m := new(dns.Msg)
m.SetQuestion("something.node.consul.", dns.TypeNS)
c := new(dns.Client)
addr, _ := a.Config.ClientListener("", a.Config.Ports.DNS)
in, _, err := c.Exchange(m, addr.String())
if err != nil {
t.Fatalf("err: %v", err)
}
if len(in.Answer) != 1 {
t.Fatalf("Bad: %#v", in)
}
nsRec, ok := in.Answer[0].(*dns.NS)
if !ok {
t.Fatalf("Bad: %#v", in.Answer[0])
}
if nsRec.Ns != "foo.node.dc1.consul." {
t.Fatalf("Bad: %#v", in.Answer[0])
}
if nsRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Answer[0])
}
}
func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) {
t.Parallel()
cfg := TestConfig()
@ -1006,7 +1025,7 @@ func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) {
t.Fatalf("Bad: %#v", in.Answer[0])
}
if len(in.Extra) != 4 {
if len(in.Extra) != 3 {
t.Fatalf("Bad: %#v", in)
}
@ -1051,20 +1070,6 @@ func TestDNS_ExternalServiceToConsulCNAMENestedLookup(t *testing.T) {
if aRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Extra[2])
}
aRec2, ok := in.Extra[3].(*dns.A)
if !ok {
t.Fatalf("Bad: %#v", in.Extra[3])
}
if aRec2.Hdr.Name != "server-test-node-dc1.consul." {
t.Fatalf("Bad: %#v", in.Extra[3])
}
if aRec2.A.String() != "127.0.0.1" {
t.Fatalf("Bad: %#v", in.Extra[3])
}
if aRec2.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Extra[3])
}
}
}
@ -3811,7 +3816,7 @@ func TestDNS_NonExistingLookup(t *testing.T) {
t.Fatalf("err: %v", err)
}
if len(in.Ns) != 2 {
if len(in.Ns) != 1 {
t.Fatalf("Bad: %#v %#v", in, len(in.Answer))
}
@ -3822,14 +3827,6 @@ func TestDNS_NonExistingLookup(t *testing.T) {
if soaRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Ns[0])
}
nsRec, ok := in.Ns[1].(*dns.NS)
if !ok {
t.Fatalf("Bad: %#v", in.Ns[1])
}
if nsRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Ns[1])
}
}
func TestDNS_NonExistingLookupEmptyAorAAAA(t *testing.T) {
@ -3920,28 +3917,21 @@ func TestDNS_NonExistingLookupEmptyAorAAAA(t *testing.T) {
t.Fatalf("err: %v", err)
}
if len(in.Ns) != 2 {
if len(in.Ns) != 1 {
t.Fatalf("Bad: %#v", in)
}
soaRec, ok := in.Ns[1].(*dns.SOA)
soaRec, ok := in.Ns[0].(*dns.SOA)
if !ok {
t.Fatalf("Bad: %#v", in.Ns[1])
t.Fatalf("Bad: %#v", in.Ns[0])
}
if soaRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Ns[1])
t.Fatalf("Bad: %#v", in.Ns[0])
}
if in.Rcode != dns.RcodeSuccess {
t.Fatalf("Bad: %#v", in)
}
nsRec, ok := in.Ns[0].(*dns.NS)
if !ok {
t.Fatalf("Bad: %#v", in.Ns[0])
}
if nsRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Ns[0])
}
}
// Check for ipv4 records on ipv6-only service directly and via the
@ -3961,24 +3951,16 @@ func TestDNS_NonExistingLookupEmptyAorAAAA(t *testing.T) {
t.Fatalf("err: %v", err)
}
if len(in.Ns) != 2 {
if len(in.Ns) != 1 {
t.Fatalf("Bad: %#v", in)
}
nsRec, ok := in.Ns[0].(*dns.NS)
soaRec, ok := in.Ns[0].(*dns.SOA)
if !ok {
t.Fatalf("Bad: %#v", in.Ns[0])
}
if nsRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Ns[0])
}
soaRec, ok := in.Ns[1].(*dns.SOA)
if !ok {
t.Fatalf("Bad: %#v", in.Ns[1])
}
if soaRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Ns[1])
t.Fatalf("Bad: %#v", in.Ns[0])
}
if in.Rcode != dns.RcodeSuccess {
@ -4020,7 +4002,7 @@ func TestDNS_PreparedQuery_AllowStale(t *testing.T) {
t.Fatalf("err: %v", err)
}
if len(in.Ns) != 2 {
if len(in.Ns) != 1 {
t.Fatalf("Bad: %#v", in)
}
@ -4032,14 +4014,6 @@ func TestDNS_PreparedQuery_AllowStale(t *testing.T) {
t.Fatalf("Bad: %#v", in.Ns[0])
}
nsRec, ok := in.Ns[1].(*dns.NS)
if !ok {
t.Fatalf("Bad: %#v", in.Ns[1])
}
if nsRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Ns[1])
}
}
}
@ -4067,7 +4041,7 @@ func TestDNS_InvalidQueries(t *testing.T) {
t.Fatalf("err: %v", err)
}
if len(in.Ns) != 2 {
if len(in.Ns) != 1 {
t.Fatalf("Bad: %#v", in)
}
@ -4079,13 +4053,6 @@ func TestDNS_InvalidQueries(t *testing.T) {
t.Fatalf("Bad: %#v", in.Ns[0])
}
nsRec, ok := in.Ns[1].(*dns.NS)
if !ok {
t.Fatalf("Bad: %#v", in.Ns[1])
}
if nsRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Ns[1])
}
}
}
@ -4781,3 +4748,24 @@ func TestDNS_Compression_Recurse(t *testing.T) {
t.Fatalf("doesn't look compressed: %d vs. %d", compressed, unc)
}
}
func TestDNSInvalidRegex(t *testing.T) {
tests := []struct {
desc string
in string
invalid bool
}{
{"Valid Hostname", "testnode", false},
{"Valid Hostname", "test-node", false},
{"Invalid Hostname with special chars", "test#$$!node", true},
{"Invalid Hostname with special chars in the end", "test-node%^", true},
}
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
if got, want := InvalidDnsRe.MatchString(test.in), test.invalid; got != want {
t.Fatalf("Expected %v to return %v", test.in, want)
}
})
}
}