Skip to content

Commit

Permalink
[plugin/route53]: Do not return NXDOMAIN where it should be NODATA. (c…
Browse files Browse the repository at this point in the history 
…oredns#2734)

* [plugin/route53]: Do not return NXDOMAIN where it should be NODATA.

Signed-off-by: Dmitry Ilyevskiy <dmitry.ilyevskiy@getcruise.com>

* Fix bad merge.

Signed-off-by: Dmitry Ilyevskiy <dmitry.ilyevskiy@getcruise.com>
  • Loading branch information
@dilyevsky @yongtang
dilyevsky authored and yongtang committed Mar 31, 2019
1 parent db34c10 commit 1e15067
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 45 deletions.
7 changes: 5 additions & 2 deletions plugin/route53/route53.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,15 @@ func (h *Route53) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg
h.zMu.RLock()
m.Answer, m.Ns, m.Extra, result = hostedZone.z.Lookup(ctx, state, qname)
h.zMu.RUnlock()
if len(m.Answer) != 0 {

// Take the answer if it's non-empty OR if there is another
// record type exists for this name (NODATA).
if len(m.Answer) != 0 || result == file.NoData {
break
}
}

if len(m.Answer) == 0 && h.Fall.Through(qname) {
if len(m.Answer) == 0 && result != file.NoData && h.Fall.Through(qname) {
return plugin.NextOrFailure(h.Name(), h.Next, ctx, w, r)
}

Expand Down
100 changes: 57 additions & 43 deletions plugin/route53/route53_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,14 @@ func (fakeRoute53) ListResourceRecordSetsPagesWithContext(_ aws.Context, in *rou
{"PTR", "example.org.", "ptr.example.org.", "1234567890"},
{"SOA", "org.", "ns-1536.awsdns-00.co.uk. awsdns-hostmaster.amazon.com. 1 7200 900 1209600 86400", "1234567890"},
{"NS", "com.", "ns-1536.awsdns-00.co.uk.", "1234567890"},
{"A", "split-example.gov.", "1.2.3.4", "1234567890"},
// Unsupported type should be ignored.
{"YOLO", "swag.", "foobar", "1234567890"},
// hosted zone with the same name, but a different id
// Hosted zone with the same name, but a different id.
{"A", "other-example.org.", "3.5.7.9", "1357986420"},
{"A", "split-example.org.", "1.2.3.4", "1357986420"},
{"SOA", "org.", "ns-15.awsdns-00.co.uk. awsdns-hostmaster.amazon.com. 1 7200 900 1209600 86400", "1357986420"},
// Hosted zone without SOA.
} {
rrs, ok := rrsResponse[r.hostedZoneID]
if !ok {
Expand Down Expand Up @@ -84,7 +87,7 @@ func TestRoute53(t *testing.T) {
t.Fatalf("Expected errors for zone bad.")
}

r, err = New(ctx, fakeRoute53{}, map[string][]string{"org.": []string{"1357986420", "1234567890"}, "gov": []string{"Z098765432"}}, &upstream.Upstream{})
r, err = New(ctx, fakeRoute53{}, map[string][]string{"org.": []string{"1357986420", "1234567890"}, "gov.": []string{"Z098765432", "1234567890"}}, &upstream.Upstream{})
if err != nil {
t.Fatalf("Failed to create Route53: %v", err)
}
Expand All @@ -105,6 +108,7 @@ func TestRoute53(t *testing.T) {

m.Authoritative = true
rcode = dns.RcodeSuccess

}

m.SetRcode(r, rcode)
Expand All @@ -119,38 +123,35 @@ func TestRoute53(t *testing.T) {
tests := []struct {
qname string
qtype uint16
expectedCode int
wantRetCode int
wantAnswer []string // ownernames for the records in the additional section.
wantMsgRCode int
wantNS []string
expectedErr error
}{
// 0. example.org A found - success.
{
qname: "example.org",
qtype: dns.TypeA,
expectedCode: dns.RcodeSuccess,
qname: "example.org",
qtype: dns.TypeA,
wantAnswer: []string{"example.org. 300 IN A 1.2.3.4"},
},
// 1. example.org AAAA found - success.
{
qname: "example.org",
qtype: dns.TypeAAAA,
expectedCode: dns.RcodeSuccess,
qname: "example.org",
qtype: dns.TypeAAAA,
wantAnswer: []string{"example.org. 300 IN AAAA 2001:db8:85a3::8a2e:370:7334"},
},
// 2. exampled.org PTR found - success.
{
qname: "example.org",
qtype: dns.TypePTR,
expectedCode: dns.RcodeSuccess,
qname: "example.org",
qtype: dns.TypePTR,
wantAnswer: []string{"example.org. 300 IN PTR ptr.example.org."},
},
// 3. sample.example.org points to example.org CNAME.
// Query must return both CNAME and A recs.
{
qname: "sample.example.org",
qtype: dns.TypeA,
expectedCode: dns.RcodeSuccess,
qname: "sample.example.org",
qtype: dns.TypeA,
wantAnswer: []string{
"sample.example.org. 300 IN CNAME example.org.",
"example.org. 300 IN A 1.2.3.4",
Expand All @@ -159,57 +160,66 @@ func TestRoute53(t *testing.T) {
// 4. Explicit CNAME query for sample.example.org.
// Query must return just CNAME.
{
qname: "sample.example.org",
qtype: dns.TypeCNAME,
expectedCode: dns.RcodeSuccess,
qname: "sample.example.org",
qtype: dns.TypeCNAME,
wantAnswer: []string{"sample.example.org. 300 IN CNAME example.org."},
},
// 5. Explicit SOA query for example.org.
{
qname: "example.org",
qtype: dns.TypeSOA,
expectedCode: dns.RcodeSuccess,
qname: "example.org",
qtype: dns.TypeSOA,
wantAnswer: []string{"org. 300 IN SOA ns-15.awsdns-00.co.uk. awsdns-hostmaster.amazon.com. 1 7200 900 1209600 86400"},
},
// 6. Explicit SOA query for example.org.
{
qname: "example.org",
qtype: dns.TypeNS,
expectedCode: dns.RcodeSuccess,
qname: "example.org",
qtype: dns.TypeNS,
wantNS: []string{"org. 300 IN SOA ns-1536.awsdns-00.co.uk. awsdns-hostmaster.amazon.com. 1 7200 900 1209600 86400"},
},
// 7. Zone not configured.
// 7. AAAA query for split-example.org must return NODATA.
{
qname: "split-example.gov",
qtype: dns.TypeAAAA,
wantRetCode: dns.RcodeSuccess,
wantNS: []string{"org. 300 IN SOA ns-1536.awsdns-00.co.uk. awsdns-hostmaster.amazon.com. 1 7200 900 1209600 86400"},
},
// 8. Zone not configured.
{
qname: "badexample.com",
qtype: dns.TypeA,
expectedCode: dns.RcodeServerFailure,
wantRetCode: dns.RcodeServerFailure,
wantMsgRCode: dns.RcodeServerFailure,
},
// 8. No record found. Return SOA record.
// 9. No record found. Return SOA record.
{
qname: "bad.org",
qtype: dns.TypeA,
expectedCode: dns.RcodeSuccess,
wantRetCode: dns.RcodeSuccess,
wantMsgRCode: dns.RcodeNameError,
wantNS: []string{"org. 300 IN SOA ns-1536.awsdns-00.co.uk. awsdns-hostmaster.amazon.com. 1 7200 900 1209600 86400"},
},
// 9. No record found. Fallthrough.
// 10. No record found. Fallthrough.
{
qname: "example.gov",
qtype: dns.TypeA,
expectedCode: dns.RcodeSuccess,
qname: "example.gov",
qtype: dns.TypeA,
wantAnswer: []string{"example.gov. 300 IN A 2.4.6.8"},
},
// 10. other-zone.example.org is stored in a different hosted zone. success
// 11. other-zone.example.org is stored in a different hosted zone. success
{
qname: "other-example.org",
qtype: dns.TypeA,
expectedCode: dns.RcodeSuccess,
qname: "other-example.org",
qtype: dns.TypeA,
wantAnswer: []string{"other-example.org. 300 IN A 3.5.7.9"},
},
// 11. *.www.example.org is a wildcard CNAME to www.example.org.
// 12. split-example.org only has A record. Expect NODATA.
{
qname: "a.www.example.org",
qtype: dns.TypeA,
expectedCode: dns.RcodeSuccess,
qname: "split-example.org",
qtype: dns.TypeAAAA,
wantNS: []string{"org. 300 IN SOA ns-15.awsdns-00.co.uk. awsdns-hostmaster.amazon.com. 1 7200 900 1209600 86400"},
},
// 13. *.www.example.org is a wildcard CNAME to www.example.org.
{
qname: "a.www.example.org",
qtype: dns.TypeA,
wantAnswer: []string{
"a.www.example.org. 300 IN CNAME www.example.org.",
"www.example.org. 300 IN A 1.2.3.4",
Expand All @@ -227,8 +237,12 @@ func TestRoute53(t *testing.T) {
if err != tc.expectedErr {
t.Fatalf("Test %d: Expected error %v, but got %v", ti, tc.expectedErr, err)
}
if code != int(tc.expectedCode) {
t.Fatalf("Test %d: Expected status code %s, but got %s", ti, dns.RcodeToString[tc.expectedCode], dns.RcodeToString[code])
if code != int(tc.wantRetCode) {
t.Fatalf("Test %d: Expected returned status code %s, but got %s", ti, dns.RcodeToString[tc.wantRetCode], dns.RcodeToString[code])
}

if tc.wantMsgRCode != rec.Msg.Rcode {
t.Errorf("Test %d: Unexpected msg status code. Want: %s, got: %s", ti, dns.RcodeToString[tc.wantMsgRCode], dns.RcodeToString[rec.Msg.Rcode])
}

if len(tc.wantAnswer) != len(rec.Msg.Answer) {
Expand All @@ -250,7 +264,7 @@ func TestRoute53(t *testing.T) {
t.Errorf("Test %d: Unexpected NS type. Want: SOA, got: %v", ti, reflect.TypeOf(got))
}
if got.String() != tc.wantNS[i] {
t.Errorf("Test %d: Unexpected NS. Want: %v, got: %v", ti, tc.wantNS[i], got)
t.Errorf("Test %d: Unexpected NS.\nWant: %v\nGot: %v", ti, tc.wantNS[i], got)
}
}
}
Expand Down

0 comments on commit 1e15067

Please sign in to comment.