Do not muck with ordering of XFRs (#2329)

The loadbalancer plugin reorders records. It was doing this for zone
transfers - if you had a CNAME in the zone then your transfer would
be broken because it would get put before the SOA record.

2 files changed, 40 insertions, 0 deletions

diff --git a/plugin/loadbalance/loadbalance.go b/plugin/loadbalance/loadbalance.go
index 39e70368d..3f3049202 100644
--- a/plugin/loadbalance/loadbalance.go
+++ b/plugin/loadbalance/loadbalance.go
@@ -14,6 +14,10 @@ func (r *RoundRobinResponseWriter) WriteMsg(res *dns.Msg) error {
 		return r.ResponseWriter.WriteMsg(res)
 	}
 
+	if res.Question[0].Qtype == dns.TypeAXFR || res.Question[0].Qtype == dns.TypeIXFR {
+		return r.ResponseWriter.WriteMsg(res)
+	}
+
 	res.Answer = roundRobin(res.Answer)
 	res.Ns = roundRobin(res.Ns)
 	res.Extra = roundRobin(res.Extra)
diff --git a/plugin/loadbalance/loadbalance_test.go b/plugin/loadbalance/loadbalance_test.go
index d5f62af0c..44a5e4b05 100644
--- a/plugin/loadbalance/loadbalance_test.go
+++ b/plugin/loadbalance/loadbalance_test.go
@@ -124,6 +124,42 @@ func TestLoadBalance(t *testing.T) {
 	}
 }
 
+func TestLoadBalanceXFR(t *testing.T) {
+	rm := RoundRobin{Next: handler()}
+
+	answer := []dns.RR{
+		test.SOA("skydns.test.	30	IN	SOA	ns.dns.skydns.test. hostmaster.skydns.test. 1542756695 7200 1800 86400 30"),
+		test.MX("mx.region2.skydns.test.			300	IN	MX		1	mx1.region2.skydns.test."),
+		test.A("endpoint.region2.skydns.test.		300	IN	A			10.240.0.1"),
+		test.A("endpoint.region2.skydns.test.		300	IN	A			10.240.0.2"),
+		test.MX("mx.region2.skydns.test.			300	IN	MX		1	mx2.region2.skydns.test."),
+		test.CNAME("cname2.region2.skydns.test.	300	IN	CNAME		cname3.region2.skydns.test."),
+		test.A("endpoint.region2.skydns.test.		300	IN	A			10.240.0.3"),
+		test.MX("mx.region2.skydns.test.			300	IN	MX		1	mx3.region2.skydns.test."),
+		test.SOA("skydns.test.	30	IN	SOA	ns.dns.skydns.test. hostmaster.skydns.test. 1542756695 7200 1800 86400 30"),
+	}
+
+	for _, xfrtype := range []uint16{dns.TypeIXFR, dns.TypeAXFR} {
+		rec := dnstest.NewRecorder(&test.ResponseWriter{})
+		req := new(dns.Msg)
+		req.SetQuestion("skydns.test.", xfrtype)
+		req.Answer = answer
+		_, err := rm.ServeDNS(context.TODO(), rec, req)
+		if err != nil {
+			t.Errorf("Expected no error, but got %s for %s", err, dns.TypeToString[xfrtype])
+			continue
+		}
+
+		if rec.Msg.Answer[0].Header().Rrtype != dns.TypeSOA {
+			t.Errorf("Expected SOA record for first answer for %s", dns.TypeToString[xfrtype])
+		}
+
+		if rec.Msg.Answer[len(rec.Msg.Answer)-1].Header().Rrtype != dns.TypeSOA {
+			t.Errorf("Expected SOA record for last answer for %s", dns.TypeToString[xfrtype])
+		}
+	}
+}
+
 func countRecords(result []dns.RR) (cname int, address int, mx int, sorted bool) {
 	const (
 		Start = iota