[plugin/route53]: Do not return NXDOMAIN where it should be NODATA. (#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>

plugin/route53/route53.go

@@ -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)
 	}
 

plugin/route53/route53_test.go

@@ -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 {
@@ -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)
 	}
@@ -105,6 +108,7 @@ func TestRoute53(t *testing.T) {
 
 			m.Authoritative = true
 			rcode = dns.RcodeSuccess
+
 		}
 
 		m.SetRcode(r, rcode)
@@ -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",
@@ -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",
@@ -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) {
@@ -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)
 				}
 			}
 		}