REFACTOR: Add Joined and Segmented accessors to TXT, and other cleanups (#2628)

models/dns.go

@@ -118,19 +118,3 @@ func (config *DNSConfig) DomainContainingFQDN(fqdn string) *DomainConfig {
 	}
 	return d
 }
-
-//// IgnoreName describes an IGNORE_NAME rule.
-//type IgnoreName struct {
-//	Pattern string `json:"pattern"` // Glob pattern.
-//	Types   string `json:"types"`   // All caps rtype names, comma separated.
-//}
-//
-//// IgnoreTarget describes an IGNORE_TARGET rule.
-//type IgnoreTarget struct {
-//	Pattern string `json:"pattern"` // Glob pattern.
-//	Type    string `json:"type"`    // All caps rtype name.
-//}
-//
-//func (i *IgnoreTarget) String() string {
-//	return i.Pattern
-//}

models/dnsrr.go

@@ -9,35 +9,11 @@ import (
 	"github.com/miekg/dns"
 )
 
-//// Header Header returns the header of an resource record.
-//func (rc *RecordConfig) Header() *dns.RR_Header {
-//	log.Fatal("Header not implemented")
-//	return nil
-//}
-
 // String returns the text representation of the resource record.
 func (rc *RecordConfig) String() string {
 	return rc.GetTargetCombined()
 }
 
-//// copy returns a copy of the RR
-//func (rc *RecordConfig) copy() dns.RR {
-//	log.Fatal("Copy not implemented")
-//	return dns.TypeToRR[dns.TypeA]()
-//}
-//
-//// len returns the length (in octets) of the uncompressed RR in wire format.
-//func (rc *RecordConfig) len() int {
-//	log.Fatal("len not implemented")
-//	return 0
-//}
-//
-//// pack packs an RR into wire format.
-//func (rc *RecordConfig) pack([]byte, int, map[string]int, bool) (int, error) {
-//	log.Fatal("pack not implemented")
-//	return 0, nil
-//}
-
 // Conversions
 
 // RRstoRCs converts []dns.RR to []RecordConfigs.

models/quotes_test.go

@@ -63,11 +63,11 @@ func TestParseQuotedTxt(t *testing.T) {
 	for i, test := range tests {
 		ls := ParseQuotedTxt(test.d1)
 		if len(ls) != len(test.e2) {
-			t.Errorf("%v: expected TxtStrings=(%v) got (%v)", i, test.e2, ls)
+			t.Errorf("%v: expected LEN TxtStrings=q(%q) got q(%q)", i, test.e2, ls)
 		}
 		for i := range ls {
 			if ls[i] != test.e2[i] {
-				t.Errorf("%v: expected TxtStrings=(%v) got (%v)", i, test.e2, ls)
+				t.Errorf("%v: expected TxtStrings=q(%q) got q(%q)", i, test.e2, ls)
 			}
 		}
 	}

models/record.go

@@ -269,14 +269,6 @@ func (rc *RecordConfig) SetLabel(short, origin string) {
 	}
 }
 
-// UnsafeSetLabelNull sets the label to "". Normally the FQDN is denoted by .Name being
-// "@" however this can be used to violate that assertion. It should only be used
-// on copies of a RecordConfig that is being used for non-standard things like
-// Marshalling yaml.
-func (rc *RecordConfig) UnsafeSetLabelNull() {
-	rc.Name = ""
-}
-
 // SetLabelFromFQDN sets the .Name/.NameFQDN fields given a FQDN and origin.
 // fqdn may have a trailing "." but it is not required.
 // origin may not have a trailing dot.
@@ -512,16 +504,6 @@ func (recs Records) HasRecordTypeName(rtype, name string) bool {
 	return false
 }
 
-// FQDNMap returns a map of all LabelFQDNs. Useful for making a
-// truthtable of labels that exist in Records.
-func (recs Records) FQDNMap() (m map[string]bool) {
-	m = map[string]bool{}
-	for _, rec := range recs {
-		m[rec.GetLabelFQDN()] = true
-	}
-	return m
-}
-
 // GetByType returns the records that match rtype typeName.
 func (recs Records) GetByType(typeName string) Records {
 	results := Records{}
@@ -542,19 +524,6 @@ func (recs Records) GroupedByKey() map[RecordKey]Records {
 	return groups
 }
 
-// GroupedByLabel returns a map of keys to records, and their original key order.
-func (recs Records) GroupedByLabel() ([]string, map[string]Records) {
-	order := []string{}
-	groups := map[string]Records{}
-	for _, rec := range recs {
-		if _, found := groups[rec.Name]; !found {
-			order = append(order, rec.Name)
-		}
-		groups[rec.Name] = append(groups[rec.Name], rec)
-	}
-	return order, groups
-}
-
 // GroupedByFQDN returns a map of keys to records, grouped by FQDN.
 func (recs Records) GroupedByFQDN() ([]string, map[string]Records) {
 	order := []string{}

models/t_txt.go

@@ -147,11 +147,29 @@ func (rc *RecordConfig) SetTargetTXTs(s []string) error {
 	return nil
 }
 
-// GetTargetTXTJoined returns the TXT target as one string. If it was stored as multiple strings, concatenate them.
+// GetTargetTXTJoined returns the TXT target as one string.
 func (rc *RecordConfig) GetTargetTXTJoined() string {
 	return strings.Join(rc.TxtStrings, "")
 }
 
+// GetTargetTXTSegmented returns the TXT target as 255-octet segments, with the remainder in the last segment.
+func (rc *RecordConfig) GetTargetTXTSegmented() []string {
+	return splitChunks(strings.Join(rc.TxtStrings, ""), 255)
+}
+
+func splitChunks(buf string, lim int) []string {
+	var chunk string
+	chunks := make([]string, 0, len(buf)/lim+1)
+	for len(buf) >= lim {
+		chunk, buf = buf[:lim], buf[lim:]
+		chunks = append(chunks, chunk)
+	}
+	if len(buf) > 0 {
+		chunks = append(chunks, buf[:])
+	}
+	return chunks
+}
+
 // SetTargetTXTfromRFC1035Quoted parses a series of quoted strings
 // and sets .TxtStrings based on the result.
 // Note: Most APIs do notThis is rarely used. Try using SetTargetTXT() first.

models/target.go

@@ -3,6 +3,7 @@ package models
 import (
 	"fmt"
 	"net"
+	"runtime/debug"
 	"strings"
 
 	"github.com/miekg/dns"
@@ -26,7 +27,7 @@ func (rc *RecordConfig) GetTargetField() string {
 	if debugWarnTxtField {
 		if rc.Type == "TXT" {
 			fmt.Printf("DEBUG: WARNING: GetTargetField called on TXT record is frequently wrong: %q\n", rc.target)
-			//debug.PrintStack()
+			debug.PrintStack()
 		}
 	}
 	return rc.target

pkg/diff/diff.go

@@ -32,37 +32,6 @@ func (d *differ) content(r *models.RecordConfig) string {
 	return r.ToDiffable()
 }
 
-// ChangesetLess returns true if c[i] < c[j].
-func ChangesetLess(c Changeset, i, j int) bool {
-	var a, b string
-	// Which fields are we comparing?
-	// Usually only Desired OR Existing content exists (we're either
-	// adding or deleting records).  In those cases, just use whichever
-	// isn't nil.
-	// In the case where both Desired AND Existing exist, it doesn't
-	// matter which we use as long as we are consistent.  I flipped a
-	// coin and picked to use Desired in that case.
-
-	if c[i].Desired != nil {
-		a = c[i].Desired.NameFQDN
-	} else {
-		a = c[i].Existing.NameFQDN
-	}
-
-	if c[j].Desired != nil {
-		b = c[j].Desired.NameFQDN
-	} else {
-		b = c[j].Existing.NameFQDN
-	}
-
-	return a < b
-
-	// TODO(tlim): This won't correctly sort:
-	// []string{"example.com", "foo.example.com", "bar.example.com"}
-	// A simple way to do that correctly is to split on ".", reverse the
-	// elements, and sort on the result.
-}
-
 // CorrectionLess returns true when comparing corrections.
 func CorrectionLess(c []*models.Correction, i, j int) bool {
 	return c[i].Msg < c[j].Msg

pkg/diff/diff2compat.go

@@ -10,15 +10,10 @@ import (
 // NewCompat is a constructor that uses the new pkg/diff2 system
 // instead of pkg/diff.
 //
-// It is for backwards compatibility only. New providers should use
-// pkg/diff2.  Older providers should use this to reduce their
-// dependency on pkg/diff2 until they can move to the pkg/diff2/By*()
-// functions.
+// It is for backwards compatibility only. New providers should use pkg/diff2.
 //
 // To use this simply change New() to NewCompat(). If that doesn't
-// work please report a bug. The only exception is if you depend on
-// the extraValues feature, which will not be supported. That
-// parameter must be set to nil.
+// work please report a bug. The extraValues feature is not supported.
 func NewCompat(dc *models.DomainConfig, extraValues ...func(*models.RecordConfig) map[string]string) Differ {
 	if len(extraValues) != 0 {
 		panic("extraValues not supported")

pkg/txtutil/txtutil.go

@@ -17,6 +17,11 @@ func SplitSingleLongTxt(records []*models.RecordConfig) {
 	}
 }
 
+// Segment returns the string as 255-octet segments, the last being the remainder.
+func Segment(s string) []string {
+	return splitChunks(s, 255)
+}
+
 func splitChunks(buf string, lim int) []string {
 	var chunk string
 	chunks := make([]string, 0, len(buf)/lim+1)

providers/powerdns/convert.go

@@ -22,7 +22,7 @@ func toRecordConfig(domain string, r zones.Record, ttl int, name string, rtype s
 
 	switch rtype {
 	case "TXT":
-		// PowerDNS API accepts long TXTs without requiring to split them
+		// PowerDNS API accepts long TXTs without requiring to split them.
 		// The API then returns them as they initially came in, e.g. "averylooooooo[...]oooooongstring" or "string" "string"
 		// So we need to strip away " and split into multiple string
 		// We can't use SetTargetRFC1035Quoted, it would split the long strings into multiple parts