Tests: ensure provider capabilities are checked (#650)

* Tests: ensure provider capabilities are checked

Adds test: `TestCapabilitiesAreFiltered`

We have a number of records and pseudo-records which in theory can only
be used with a given provider if that provider indicates support.  In
practice, we've been missing the checks for that support and have been
passing the records down anyway.  The advice comment in the
providers/capabilities.go file to edit `checkProviderCapabilities()` has
not been reliably followed.

We need an internal self-consistency test.  The constants are not
directly exported or enumerable based solely on the package interfaces
at run-time, but with source access for a test suite, we can use the
`go/ast` and related interfaces to examine the code, extract all the
constants from a given package, figure out which ones we want to be
handled, and then insist that they're handled.

Before my recent work, we only checked:

    ALIAS PTR SRV CAA TLSA

After this commit, we check:

    ALIAS AUTODNSSEC CAA NAPTR PTR R53_ALIAS SSHFP SRV TLSA

I've added `AUTODNSSEC` as a new feature; `SSHFP` and `PTR` were caught
in other recent commits from me; implementing this test caused me to
have to add `NAPTR` and `R53_ALIAS`.  I whitelist `CanUseTXTMulti` as a
special-case.

This should prevent regressions.  We will probably want to post publicly
to warn people that if they're using SSHFP/PTR/NAPTR/R53_ALIAS then they
should check the feature matrix and if they don't see their provider
listed, to report is as "hey that actually works" so we can update the
provider flags.  Bonus: our feature matrix will suddenly be more
accurate.

* Add comments/docs for capabilities authors

* fixup!

* fixup!

docs/adding-new-rtypes.md

@@ -48,7 +48,8 @@ type RecordConfig struct {
 
 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.
+a minimum, unless this is a fake or pseudo-type that only a particular
+provider supports.
 
 * Add the capability to the file `dnscontrol/providers/capabilities.go` (look for `CanUseAlias` and add
 it to the end of the list.)
@@ -60,10 +61,32 @@ it to the end of the list.)
   `pkg/normalize/validate.go`.
 * Mark the `bind` provider as supporting this record type by updating `dnscontrol/providers/bind/bindProvider.go` (look for `providers.CanUse` and you'll see what to do).
 
+DNSControl will warn/error if this new record is used with a
+provider that does not support the capability.
+
+* Add the capability to the validations in `pkg/normalize/validate.go`
+  by adding it to `providerCapabilityChecks`
+* Some capabilities can't be tested for, such as `CanUseTXTMulti`.  If
+  such testing can't be done, add it to the whitelist in function
+  `TestCapabilitiesAreFiltered` in
+  `pkg/normalize/capabilities_test.go`
+
+If the capabilities testing is not configured correctly, `go test ./...`
+will report something like the `MISSING` message below. In this
+example we removed `providers.CanUseCAA` from the
+`providerCapabilityChecks` list.
+
+```
+--- FAIL: TestCapabilitiesAreFiltered (0.00s)
+    capabilities_test.go:66: ok: providers.CanUseAlias (0) is checked for with "ALIAS"
+    capabilities_test.go:68: MISSING: providers.CanUseCAA (1) is not checked by checkProviderCapabilities
+    capabilities_test.go:66: ok: providers.CanUseNAPTR (3) is checked for with "NAPTR"
+```
+
 ## Step 3: 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
+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.
 
@@ -99,6 +122,10 @@ 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.
 
+The tests also verify that for every "capability" there is a
+validation. This is explained in Step 2 (search for
+`TestCapabilitiesAreFiltered` or `MISSING`)
+
 ## Step 6: Add an `integrationTest` test case.
 
 Add at least one test case to the `integrationTest/integration_test.go`

pkg/normalize/capabilities_test.go

@@ -0,0 +1,72 @@
+package normalize
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"sort"
+	"strings"
+	"testing"
+)
+
+const providersImportDir = "../../providers"
+const providersPackageName = "providers"
+
+func TestCapabilitiesAreFiltered(t *testing.T) {
+	// Any capabilities which we wish to whitelist because it's not directly
+	// something we can test against.
+	skipCheckCapabilities := make(map[string]struct{})
+	skipCheckCapabilities["CanUseTXTMulti"] = struct{}{}
+
+	fset := token.NewFileSet()
+	pkgs, err := parser.ParseDir(fset, providersImportDir, nil, 0)
+	if err != nil {
+		t.Fatalf("unable to load Go code from providers: %s", err)
+	}
+	providers, ok := pkgs[providersPackageName]
+	if !ok {
+		t.Fatalf("did not find package %q in %q", providersPackageName, providersImportDir)
+	}
+
+	constantNames := make([]string, 0, 50)
+	capabilityInts := make(map[string]int, 50)
+
+	// providers.Scope was nil in my testing
+	for fileName := range providers.Files {
+		scope := providers.Files[fileName].Scope
+		for itemName, obj := range scope.Objects {
+			if obj.Kind != ast.Con {
+				continue
+			}
+			// In practice, the object.Type is nil here so we can't filter for
+			// capabilities so easily.
+			if !strings.HasPrefix(itemName, "CanUse") {
+				continue
+			}
+			constantNames = append(constantNames, itemName)
+			capabilityInts[itemName] = obj.Data.(int)
+		}
+	}
+	sort.Strings(constantNames)
+
+	if len(providerCapabilityChecks) == 0 {
+		t.Fatal("missing entries in providerCapabilityChecks")
+	}
+
+	capIntsToNames := make(map[int]string, len(providerCapabilityChecks))
+	for _, pair := range providerCapabilityChecks {
+		capIntsToNames[int(pair.cap)] = pair.rType
+	}
+
+	for _, capName := range constantNames {
+		capInt := capabilityInts[capName]
+		if _, ok := skipCheckCapabilities[capName]; ok {
+			t.Logf("ok: providers.%s (%d) is exempt from checkProviderCapabilities", capName, capInt)
+		} else if rType, ok := capIntsToNames[capInt]; ok {
+			t.Logf("ok: providers.%s (%d) is checked for with %q", capName, capInt, rType)
+		} else {
+			t.Errorf("MISSING: providers.%s (%d) is not checked by checkProviderCapabilities", capName, capInt)
+		}
+	}
+
+}

pkg/normalize/validate.go

@@ -432,20 +432,36 @@ func checkDuplicates(records []*models.RecordConfig) (errs []error) {
 	return errs
 }
 
-func checkProviderCapabilities(dc *models.DomainConfig) error {
-	types := []struct {
-		rType string
-		cap   providers.Capability
-	}{
+// We pull this out of checkProviderCapabilities() so that it's visible within
+// the package elsewhere, so that our test suite can look at the list of
+// capabilities we're checking and make sure that it's up-to-date.
+var providerCapabilityChecks []pairTypeCapability
+
+type pairTypeCapability struct {
+	rType string
+	cap   providers.Capability
+}
+
+func init() {
+	providerCapabilityChecks = []pairTypeCapability{
+		// If a zone uses rType X, the provider must support capability Y.
+		//{"X", providers.Y},
 		{"ALIAS", providers.CanUseAlias},
 		{"AUTODNSSEC", providers.CanAutoDNSSEC},
 		{"CAA", providers.CanUseCAA},
+		{"NAPTR", providers.CanUseNAPTR},
 		{"PTR", providers.CanUsePTR},
+		{"R53_ALIAS", providers.CanUseRoute53Alias},
 		{"SSHFP", providers.CanUseSSHFP},
 		{"SRV", providers.CanUseSRV},
 		{"TLSA", providers.CanUseTLSA},
 	}
-	for _, ty := range types {
+}
+
+func checkProviderCapabilities(dc *models.DomainConfig) error {
+	// Check if the zone uses a capability that the provider doesn't
+	// support.
+	for _, ty := range providerCapabilityChecks {
 		hasAny := false
 		switch ty.rType {
 		case "AUTODNSSEC":