From acaa77dc22fb9bb5d9c493f5f51b144a4b170fb5 Mon Sep 17 00:00:00 2001
From: Tom Limoncelli <tlimoncelli@stackexchange.com>
Date: Thu, 22 Mar 2018 09:25:40 -0400
Subject: [PATCH] BIND: Fix panic on CNAME (#347)

* BIND: Fix panic on CNAME
* Improve tests
---
 providers/bind/prettyzone.go      | 12 ++++----
 providers/bind/prettyzone_test.go | 50 +++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/providers/bind/prettyzone.go b/providers/bind/prettyzone.go
index d71bc4f4b..032220208 100644
--- a/providers/bind/prettyzone.go
+++ b/providers/bind/prettyzone.go
@@ -41,8 +41,6 @@ func (z *zoneGenData) Less(i, j int) bool {
 		return zoneRrtypeLess(rrtypeA, rrtypeB)
 	}
 	switch rrtypeA { // #rtype_variations
-	case dns.TypeNS, dns.TypeTXT, dns.TypeTLSA:
-		// pass through.
 	case dns.TypeA:
 		ta2, tb2 := a.(*dns.A), b.(*dns.A)
 		ipa, ipb := ta2.A.To4(), tb2.A.To4()
@@ -57,7 +55,11 @@ func (z *zoneGenData) Less(i, j int) bool {
 	case dns.TypeMX:
 		ta2, tb2 := a.(*dns.MX), b.(*dns.MX)
 		pa, pb := ta2.Preference, tb2.Preference
-		return pa < pb
+		// sort by priority. If they are equal, sort by Mx.
+		if pa != pb {
+			return pa < pb
+		}
+		return ta2.Mx < tb2.Mx
 	case dns.TypeSRV:
 		ta2, tb2 := a.(*dns.SRV), b.(*dns.SRV)
 		pa, pb := ta2.Port, tb2.Port
@@ -92,9 +94,7 @@ func (z *zoneGenData) Less(i, j int) bool {
 			return fa > fb
 		}
 	default:
-		panic(fmt.Sprintf("zoneGenData Less: unimplemented rtype %v", dns.TypeToString[rrtypeA]))
-		// We panic so that we quickly find any switch statements
-		// that have not been updated for a new RR type.
+		// pass through. String comparison is sufficient.
 	}
 	return a.String() < b.String()
 }
diff --git a/providers/bind/prettyzone_test.go b/providers/bind/prettyzone_test.go
index 3bb86162f..fc7f1ec7c 100644
--- a/providers/bind/prettyzone_test.go
+++ b/providers/bind/prettyzone_test.go
@@ -229,6 +229,56 @@ var testdataZFCAA = `$TTL 300
                  IN CAA   0 issuewild ";"
 `
 
+// Test 1 of each record type
+
+func mustNewRR(s string) dns.RR {
+	r, err := dns.NewRR(s)
+	if err != nil {
+		panic(err)
+	}
+	return r
+}
+
+func TestWriteZoneFileEach(t *testing.T) {
+	// Each rtype should be listed in this test exactly once.
+	// If an rtype has more than one variations, add a test like TestWriteZoneFileCaa to test each.
+	var d []dns.RR
+	// #rtype_variations
+	d = append(d, mustNewRR(`4.5                  300 IN PTR   y.bosun.org.`)) // Wouldn't actually be in this domain.
+	d = append(d, mustNewRR(`bosun.org.           300 IN A     1.2.3.4`))
+	d = append(d, mustNewRR(`bosun.org.           300 IN MX    1 bosun.org.`))
+	d = append(d, mustNewRR(`bosun.org.           300 IN TXT   "my text"`))
+	d = append(d, mustNewRR(`bosun.org.           300 IN AAAA  4500:fe::1`))
+	d = append(d, mustNewRR(`bosun.org.           300 IN SRV   10 10 9999 foo.com.`))
+	d = append(d, mustNewRR(`bosun.org.           300 IN CAA   0 issue "letsencrypt.org"`))
+	d = append(d, mustNewRR(`_443._tcp.bosun.org. 300 IN TLSA  3 1 1 abcdef0`)) // Label must be _port._proto
+	d = append(d, mustNewRR(`sub.bosun.org.       300 IN NS    bosun.org.`))    // Must be a label with no other records.
+	d = append(d, mustNewRR(`x.bosun.org.         300 IN CNAME bosun.org.`))    // Must be a label with no other records.
+	buf := &bytes.Buffer{}
+	WriteZoneFile(buf, d, "bosun.org")
+	if buf.String() != testdataZFEach {
+		t.Log(buf.String())
+		t.Log(testdataZFEach)
+		t.Fatalf("Zone file does not match.")
+	}
+	parseAndRegen(t, buf, testdataZFEach)
+}
+
+var testdataZFEach = `$TTL 300
+4.5.             IN PTR   y.bosun.org.
+@                IN A     1.2.3.4
+                 IN MX    1 bosun.org.
+                 IN TXT   "my text"
+                 IN AAAA  4500:fe::1
+                 IN SRV   10 10 9999 foo.com.
+                 IN CAA   0 issue "letsencrypt.org"
+_443._tcp        IN TLSA  3 1 1 abcdef0
+sub              IN NS    bosun.org.
+x                IN CNAME bosun.org.
+`
+
+// Test sorting
+
 func TestWriteZoneFileOrder(t *testing.T) {
 	var records []dns.RR
 	for i, td := range []string{