Make it easier to add new Rtypes. (#169)

* NEW: docs/adding-new-rtypes.md
* Mark all "if" and "switch" statements with `#rtype_variations`
* Make consistent use of `default: panic()`

cmd/convertzone/main.go

@@ -109,7 +109,7 @@ func rrFormat(zonename string, filename string, r io.Reader, defaultTTL uint32,
 		if !dsl { // TSV format:
 			fmt.Printf("%s\t%s\t%s\t%s\t%s\n", name, ttl, classStr, typeStr, target)
 		} else { // DSL format:
-			switch hdr.Rrtype {
+			switch hdr.Rrtype { // #rtype_variations
 			case dns.TypeMX:
 				m := strings.SplitN(target, "\t", 2)
 				target = m[0] + ", '" + m[1] + "'"

docs/adding-new-rtypes.md

@@ -0,0 +1,150 @@
+---
+layout: default
+---
+
+# Creating new DNS Resource Types (Rtypes)
+
+Everyone is familiar with A, AAAA, CNAME, NS and other Rtypes.
+However there are new record types being added all the time (possibly
+too many).  Each new record type requires special handling by
+DNSControl.
+
+If a record simply has a single "target", then there is little to
+do because it is handled similarly to A, CNAME, and so on.  However
+if there are multiple fields within the record you have more work
+to do.
+
+Our general philosophy is:
+
+* Internally the individual fields of a record are kept separate. If a particular provider combines them into one big string, that kind of thing is done in the provider code at the end of the food chain.  For example, an MX record has a Target (`aspmx.l.google.com.`) and a preference (`10`).  Some systems combine this into one string (`10 aspmx.l.google.com.`).  We keep the two values separate in `RecordConfig` and leave it up to the individual providers to merge them when required. An earlier implementation kept everything combined and we found ourselves constantly parsing and re-parsing the target. It was inefficient and lead to many bugs.
+* Anywhere we have a special case for a particular Rtype, we use a `switch` statement and have a `case` for every single record type, usually with a `default:` case that calls `panic()`. This way developers adding a new record type will quickly find where they need to add code (the panic will tell them where).  Before we did this, missing implementation code would go unnoticed for months.
+* Keep things alphabetical. If you are adding your record type to a case statement, function library, or whatever, please list it alphabetically along with the others when possible.
+
+## Step 1: Update `RecordConfig` in `models/dns.go`
+
+If the record has any unique fields, add them to `RecordConfig`.
+The field name should be the record type, then the field name as
+used in `github.com/miekg/dns/types.go`. For example, the `CAA`
+record has a field called `Flag`, therefore the field name in
+`RecordConfig` is CaaFlag (not `CaaFlags` or `CAAFlags`).
+
+Here are some examples:
+
+```
+type RecordConfig struct {
+  ...
+  MxPreference uint16            `json:"mxpreference,omitempty"` // FIXME(tlim): Rename to MxPreference
+  SrvPriority  uint16            `json:"srvpriority,omitempty"`
+  SrvWeight    uint16            `json:"srvweight,omitempty"`
+  SrvPort      uint16            `json:"srvport,omitempty"`
+  CaaTag       string            `json:"caatag,omitempty"`
+  CaaFlag      uint8             `json:"caaflag,omitempty"`
+  ...
+}
+```
+
+## Step 2: Add a capability for the record
+
+You'll need to mark which providers support this record type.  The
+initial PR should implement this record for the `bind` provider at
+a minimum.
+
+* Add the capability to the file `dnscontrol/providers/providers.go` (look for `CanUseAlias` and add
+it to the end of the list.)
+* Mark the `bind` provider as supporting this record type by updating `dnscontrol/providers/bind/bindProvider.go` (look for `providers.CanUs` and you'll see what to do).
+
+## Step 2: Add a helper function
+
+Add a function to `pkg/js/helpers.js` for the new record type.  This
+is the Javascript file that defines `dnsconfig.js`'s functions like
+`A()` and `MX()`.  Look at the definition of A, MX and CAA for good
+examples to use as a base.
+
+Please add the function alphabetically with the others.
+
+## Step 3: Search for `#rtype_variations`
+
+Anywhere a rtype requires special handling has been marked with a
+comment that includes the string `#rtype_variations`.  Search for
+this string and add your new type to this code.
+
+## Step 4: Add a `parse_tests` test case.
+
+Add at least one test case to the `pkg/js/parse_tests` directory.
+Test `013-mx.js` is a very simple one and is good for cloning.
+
+Run these tests via:
+
+    cd dnscontrol/pkg/js
+    go test ./...
+
+If this works, then you know the `dnsconfig.js` and `helpers.js`
+code is working correctly.
+
+As you debug, if there are places that haven't been marked
+`#rtype_variations` that should be, add such a comment.
+Every time you do this, an angel gets its wings.
+
+## Step 5: Add an `integrationTest` test case.
+
+Add at least one test case to the `integrationTest/integration_test.go`
+file. Look for `var tests =` and add the test to the end of this
+list.
+
+Each entry in the list is a new state.  For example:
+
+```
+  //MX
+  tc("Empty"),                                    <<< 1
+  tc("MX record", mx("@", 5, "foo.com.")),        <<< 2
+  tc("Change MX pref", mx("@", 10, "foo.com.")),  <<< 3
+```
+
+Line 1: An `tc()` entry with no records (just a comment). The test
+system will delete all records from the domain to make the domain
+match this empty configuration. This creates a "clean slate"
+situation.
+
+Line 2: A `tc()` entry with 1 record.  To get to this state, the
+provider will have to add the record. If this works, basic functionality
+for the MX record type has been achieved.
+
+Line 3: A `tc()` entry with 1 record, with a different priority.
+To get to this state, the provider will have to either change the
+priority on an existing record, or delete the old record and insert
+a new one. Either way, this test case assures us that the diff'ing
+functionality is working properly.
+
+If you look at the tests for `CAA`, it inserts a few records then
+attempts to modify each field of a record one at a time.  This test
+was useful because it turns out we hadn't written the code to
+properly see a change in priority. We fixed this bug before the
+code made it into production.
+
+Also notice that some tests include `.IfHasCapability()`. This
+limits the test to providers with certain capabilities.  You'll
+want to use this feature so that the tests only run on providers
+that support your new record type.
+
+To run the integration test with the BIND provider:
+
+    cd dnscontrol/integrationTest
+    go test -v -verbose -provider BIND
+
+Once the code works for BIND, consider submitting a PR at this point.
+
+As you debug, if there are places that haven't been marked
+`#rtype_variations` that should be, add such a comment.
+If you fail to do this, God kills a cute little kitten.
+
+## Step 6: Support more providers
+
+Now add support other providers.  Add the `providers.CanUse...`
+flag to the provider and re-run the integration tests:
+
+For example, this will run the tests on Amazon AWS Route53:
+
+    export R53_DOMAIN=dnscontroltest-r53.com  # Use a test domain.
+    export R53_KEY_ID=CHANGE_TO_THE_ID
+    export R53_KEY='CHANGE_TO_THE_KEY'
+    go test -v -verbose -provider ROUTE53

docs/index.md

@@ -5,24 +5,35 @@ layout: default
 
 Dnscontrol is a platform for seamlessly managing your dns configuration across any number of DNS hosts, both in the cloud or in your own infrastructure. It manages all of the domains for the Stack Exchange network.
 
-## [Getting Started]({{site.github.url}}/getting-started): A walk-through of the basics.
+## Getting Started
 
-## [Providers]({{site.github.url}}/provider-list): Which DNS providers are supported.
+### [Getting Started]({{site.github.url}}/getting-started): A walk-through of the basics.
 
-## [Language Reference]({{site.github.url}}/js): Description of the DNSControl language (DSL).
+### [Providers]({{site.github.url}}/provider-list): Which DNS providers are supported.
 
-## [Examples]({{site.github.url}}/examples): The DNSControl language by example.
+### [Examples]({{site.github.url}}/examples): The DNSControl language by example.
 
-## [Migrating]({{site.github.url}}/migrating): Migrating zones to DNSControl.
+### [Migrating]({{site.github.url}}/migrating): Migrating zones to DNSControl.
-
-## [Testing]({{site.github.url}}/unittests): Unit Testing DNS Data.
-
-## [github](https://github.com/StackExchange/dnscontrol): Get the source!
 
 
+## Reference
 
-## FAQs and design notes
+### [Language Reference]({{site.github.url}}/js): Description of the DNSControl language (DSL).
 
-- [Why CNAME/MX/NS targets require a trailing "dot"]({{site.github.url}}/why-the-dot)
+### [ALIAS / ANAME records in dnscontrol]({{site.github.url}}/alias)
-- [Writing Providers]({{site.github.url}}/writing-providers)
+
-- [ALIAS records in dnscontrol]({{site.github.url}}/alias)
+### [Why CNAME/MX/NS targets require a trailing "dot"]({{site.github.url}}/why-the-dot)
+
+
+## Advanced Usage
+
+### [Testing]({{site.github.url}}/unittests): Unit Testing for you DNS Data.
+
+
+## Developer info
+
+### [github](https://github.com/StackExchange/dnscontrol): Get the source!
+
+### [Writing Providers]({{site.github.url}}/writing-providers)
+
+### [Adding new DNS record types]({{site.github.url}}/adding-new-rtypes)

models/dns.go

@@ -85,7 +85,9 @@ func (r *RecordConfig) String() (content string) {
 	}
 
 	content = fmt.Sprintf("%s %s %s %d", r.Type, r.NameFQDN, r.Target, r.TTL)
-	switch r.Type {
+	switch r.Type { // #rtype_variations
+	case "A", "AAAA", "CNAME", "PTR", "TXT":
+		// Nothing special.
 	case "MX":
 		content += fmt.Sprintf(" priority=%d", r.MxPreference)
 	case "SOA":
@@ -93,7 +95,10 @@ func (r *RecordConfig) String() (content string) {
 	case "CAA":
 		content += fmt.Sprintf(" caatag=%s caaflag=%d", r.CaaTag, r.CaaFlag)
 	default:
-		// assume nothing special for A,CNAME,AAAA, and other simple types.
+		msg := fmt.Sprintf("rc.String rtype %v unimplemented", r.Type)
+		panic(msg)
+		// We panic so that we quickly find any switch statements
+		// that have not been updated for a new RR type.
 	}
 	for k, v := range r.Metadata {
 		content += fmt.Sprintf(" %s=%s", k, v)
@@ -168,7 +173,7 @@ func (rc *RecordConfig) ToRR() dns.RR {
 	}
 
 	// Fill in the data.
-	switch rdtype {
+	switch rdtype { // #rtype_variations
 	case dns.TypeA:
 		rr.(*dns.A).A = net.ParseIP(rc.Target)
 	case dns.TypeAAAA:
@@ -205,6 +210,8 @@ func (rc *RecordConfig) ToRR() dns.RR {
 		rr.(*dns.TXT).Txt = []string{rc.Target}
 	default:
 		panic(fmt.Sprintf("ToRR: Unimplemented rtype %v", rc.Type))
+		// We panic so that we quickly find any switch statements
+		// that have not been updated for a new RR type.
 	}
 
 	return rr
@@ -269,11 +276,19 @@ func (dc *DomainConfig) Punycode() error {
 		if err != nil {
 			return err
 		}
-		if rec.Type == "MX" || rec.Type == "CNAME" {
+		switch rec.Type { // #rtype_variations
+		case "ALIAS", "MX", "NS", "CNAME", "SRV":
 			rec.Target, err = idna.ToASCII(rec.Target)
 			if err != nil {
 				return err
 			}
+		case "A", "CAA":
+			// Nothing to do.
+		default:
+			msg := fmt.Sprintf("Punycode rtype %v unimplemented", rec.Type)
+			panic(msg)
+			// We panic so that we quickly find any switch statements
+			// that have not been updated for a new RR type.
 		}
 	}
 	return nil

pkg/normalize/validate.go

@@ -127,7 +127,7 @@ func checkTargets(rec *models.RecordConfig, domain string) (errs []error) {
 			errs = append(errs, err)
 		}
 	}
-	switch rec.Type {
+	switch rec.Type { // #rtype_variations
 	case "A":
 		check(checkIPv4(target))
 	case "AAAA":
@@ -192,7 +192,7 @@ func importTransform(srcDomain, dstDomain *models.DomainConfig, transforms []tra
 			}
 			return rec2
 		}
-		switch rec.Type {
+		switch rec.Type { // #rtype_variations
 		case "A":
 			trs, err := transform.TransformIPToList(net.ParseIP(rec.Target), transforms)
 			if err != nil {

providers/activedir/domains.go

@@ -165,7 +165,7 @@ func (r *RecordConfigJson) unpackRecord(origin string) *models.RecordConfig {
 	rc.Type = r.Type
 	rc.TTL = r.TTL
 
-	switch rc.Type {
+	switch rc.Type { // #rtype_variations
 	case "A":
 		rc.Target = r.Data
 	case "CNAME":
@@ -200,7 +200,7 @@ func (c *adProvider) generatePowerShellCreate(domainname string, rec *models.Rec
 	text += fmt.Sprintf(` -ZoneName "%s"`, domainname)
 	text += fmt.Sprintf(` -Name "%s"`, rec.Name)
 	text += fmt.Sprintf(` -TimeToLive $(New-TimeSpan -Seconds %d)`, rec.TTL)
-	switch rec.Type {
+	switch rec.Type { // #rtype_variations
 	case "CNAME":
 		text += fmt.Sprintf(` -HostNameAlias "%s"`, content)
 	case "A":
@@ -209,6 +209,8 @@ func (c *adProvider) generatePowerShellCreate(domainname string, rec *models.Rec
 		text = fmt.Sprintf("\r\n"+`echo "Skipping NS update (%v %v)"`+"\r\n", rec.Name, rec.Target)
 	default:
 		panic(fmt.Errorf("ERROR: generatePowerShellCreate() does not yet handle recType=%s recName=%#v content=%#v)\n", rec.Type, rec.Name, content))
+		// We panic so that we quickly find any switch statements
+		// that have not been updated for a new RR type.
 	}
 	text += "\r\n"
 
@@ -220,7 +222,7 @@ func (c *adProvider) generatePowerShellModify(domainname, recName, recType, oldC
 
 	var queryField, queryContent string
 
-	switch recType {
+	switch recType { // #rtype_variations
 	case "A":
 		queryField = "IPv4address"
 		queryContent = `"` + oldContent + `"`
@@ -229,6 +231,8 @@ func (c *adProvider) generatePowerShellModify(domainname, recName, recType, oldC
 		queryContent = `"` + oldContent + `"`
 	default:
 		panic(fmt.Errorf("ERROR: generatePowerShellModify() does not yet handle recType=%s recName=%#v content=(%#v, %#v)\n", recType, recName, oldContent, newContent))
+		// We panic so that we quickly find any switch statements
+		// that have not been updated for a new RR type.
 	}
 
 	text := "\r\n" // Skip a line.

providers/bind/bindProvider.go

@@ -88,7 +88,7 @@ func rrToRecord(rr dns.RR, origin string, replaceSerial uint32) (models.RecordCo
 	rc.NameFQDN = strings.ToLower(strings.TrimSuffix(header.Name, "."))
 	rc.Name = strings.ToLower(dnsutil.TrimDomainName(header.Name, origin))
 	rc.TTL = header.Ttl
-	switch v := rr.(type) {
+	switch v := rr.(type) { // #rtype_variations
 	case *dns.A:
 		rc.Target = v.A.String()
 	case *dns.AAAA:

providers/bind/prettyzone.go

@@ -39,7 +39,7 @@ func (z *zoneGenData) Less(i, j int) bool {
 	if rrtypeA != rrtypeB {
 		return zoneRrtypeLess(rrtypeA, rrtypeB)
 	}
-	switch rrtypeA {
+	switch rrtypeA { // #rtype_variations
 	case dns.TypeNS, dns.TypeTXT:
 		// pass through.
 	case dns.TypeA:
@@ -82,6 +82,8 @@ func (z *zoneGenData) Less(i, j int) bool {
 		}
 	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.
 	}
 	return a.String() < b.String()
 }

providers/namedotcom/records.go

@@ -105,7 +105,7 @@ func (r *nameComRecord) toRecord() *models.RecordConfig {
 		TTL:      uint32(ttl),
 		Original: r,
 	}
-	switch r.Type {
+	switch r.Type { // #rtype_variations
 	case "A", "AAAA", "ANAME", "CNAME", "NS", "TXT":
 		// nothing additional.
 	case "MX":
@@ -121,6 +121,8 @@ func (r *nameComRecord) toRecord() *models.RecordConfig {
 		rc.Target = parts[2] + "."
 	default:
 		panic(fmt.Sprintf("toRecord unimplemented rtype %v", r.Type))
+		// We panic so that we quickly find any switch statements
+		// that have not been updated for a new RR type.
 	}
 	return rc
 }
@@ -173,7 +175,7 @@ func (n *nameDotCom) createRecord(rc *models.RecordConfig, domain string) error
 	if dat.Hostname == "@" {
 		dat.Hostname = ""
 	}
-	switch rc.Type {
+	switch rc.Type { // #rtype_variations
 	case "A", "AAAA", "ANAME", "CNAME", "MX", "NS", "TXT":
 		// nothing
 	case "SRV":
@@ -181,6 +183,8 @@ func (n *nameDotCom) createRecord(rc *models.RecordConfig, domain string) error
 		dat.Priority = rc.SrvPriority
 	default:
 		panic(fmt.Sprintf("createRecord rtype %v unimplemented", rc.Type))
+		// We panic so that we quickly find any switch statements
+		// that have not been updated for a new RR type.
 	}
 	resp, err := n.post(n.apiCreateRecord(domain), dat)
 	if err != nil {