diff --git a/docs/adding-new-rtypes.md b/docs/adding-new-rtypes.md index b6c89ce23..f6c654bc1 100644 --- a/docs/adding-new-rtypes.md +++ b/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` diff --git a/pkg/normalize/capabilities_test.go b/pkg/normalize/capabilities_test.go new file mode 100644 index 000000000..53279472a --- /dev/null +++ b/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) + } + } + +} diff --git a/pkg/normalize/validate.go b/pkg/normalize/validate.go index 0e389fe1f..f0f941e8e 100644 --- a/pkg/normalize/validate.go +++ b/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":