BUGFIX: IDN support is broken for domain names (#3845)

# Issue

Fixes #3842

CC @das7pad

# Resolution

Convert domain.Name to IDN earlier in the pipeline. Hack the --domains
processing to convert everything to IDN.

* Domain names are now stored 3 ways: The original input from
dnsconfig.js, canonical IDN format (`xn--...`), and Unicode format. All
are downcased. Providers that haven't been updated will receive the IDN
format instead of the original input format. This might break some
providers but only for users with unicode in their D("domain.tld").
PLEASE TEST YOUR PROVIDER.
* BIND filename formatting options have been added to access the new
formats.

# Breaking changes

* BIND zonefiles may change. The default used the name input in the D()
statement. It now defaults to the IDN name + "!tag" if there is a tag.
* Providers that are not IDN-aware may break (hopefully only if they
weren't processing IDN already)

---------

Co-authored-by: Jakob Ackermann <[email protected]>

Author: tlimoncelli

commands/commands.go

@@ -303,39 +303,3 @@ func (args *FilterArgs) flags() []cli.Flag {
 		},
 	}
 }
-
-// domainInList takes a domain and a list of domains and returns true if the
-// domain is in the list, accounting for wildcards and tags.
-func domainInList(domain string, list []string) bool {
-	for _, item := range list {
-		if item == domain {
-			return true
-		}
-		if strings.HasPrefix(item, "*") && strings.HasSuffix(domain, item[1:]) {
-			return true
-		}
-		filterDom, filterTag, isFilterTagged := strings.Cut(item, "!")
-		splitDom, domainTag, isDomainTagged := strings.Cut(domain, "!")
-		if splitDom == filterDom {
-			if isDomainTagged {
-				if filterTag == "*" {
-					return true
-				}
-				if domainTag == "" && !isFilterTagged {
-					// domain example.com! == filter example.com
-					return true
-				}
-				if isFilterTagged && domainTag == filterTag {
-					return true
-				}
-			}
-			if isFilterTagged {
-				if filterTag == "" && !isDomainTagged {
-					// filter example.com! == domain example.com
-					return true
-				}
-			}
-		}
-	}
-	return false
-}

commands/commands_test.go

@@ -200,7 +200,10 @@ func GetZone(args GetZoneArgs) error {
 	// fetch all of the records
 	zoneRecs := make([]models.Records, len(zones))
 	for i, zone := range zones {
-		recs, err := provider.GetZoneRecords(zone, nil)
+		recs, err := provider.GetZoneRecords(zone,
+			map[string]string{
+				models.DomainUniqueName: zone,
+			})
 		if err != nil {
 			return fmt.Errorf("failed GetZone gzr: %w", err)
 		}

commands/getZones.go

@@ -63,7 +63,7 @@ func testFormat(t *testing.T, domain, format string) {
 	// Read the expected result
 	want, err := os.ReadFile(expectedFilename)
 	if err != nil {
-		log.Fatal(fmt.Errorf("can't read expected %q: %w", outfile.Name(), err))
+		log.Fatal(fmt.Errorf("can't read expected %q: %w", expectedFilename, err))
 	}
 
 	if w, g := string(want), string(got); w != g {

commands/gz_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/StackExchange/dnscontrol/v4/models"
 	"github.com/StackExchange/dnscontrol/v4/pkg/bindserial"
 	"github.com/StackExchange/dnscontrol/v4/pkg/credsfile"
+	"github.com/StackExchange/dnscontrol/v4/pkg/domaintags"
 	"github.com/StackExchange/dnscontrol/v4/pkg/nameservers"
 	"github.com/StackExchange/dnscontrol/v4/pkg/normalize"
 	"github.com/StackExchange/dnscontrol/v4/pkg/notifications"
@@ -288,7 +289,7 @@ func prun(args PPreviewArgs, push bool, interactive bool, out printer.CLI, repor
 					continue // Do not emit noise when zone exists
 				}
 				if !started {
-					out.StartDomain(zone.GetUniqueName())
+					out.StartDomain(zone)
 					started = true
 				}
 				skip := skipProvider(provider.Name, providersToProcess)
@@ -351,7 +352,7 @@ func prun(args PPreviewArgs, push bool, interactive bool, out printer.CLI, repor
 	// Now we know what to do, print or do the tasks.
 	out.PrintfIf(fullMode, "PHASE 3: CORRECTIONS\n")
 	for _, zone := range zonesToProcess {
-		out.StartDomain(zone.GetUniqueName())
+		out.StartDomain(zone)
 
 		// Process DNS provider changes:
 		providersToProcess := whichProvidersToProcess(zone.DNSProviderInstances, args.Providers)
@@ -400,29 +401,16 @@ func prun(args PPreviewArgs, push bool, interactive bool, out printer.CLI, repor
 	return nil
 }
 
-// func countActions(corrections []*models.Correction) int {
-//	r := 0
-//	for _, c := range corrections {
-//		if c.F != nil {
-//			r++
-//		}
-//	}
-//	return r
-//}
-
 // whichZonesToProcess takes a list of DomainConfigs and a filter string and
-// returns a list of DomainConfigs whose metadata[DomainUniqueName] matched the
+// returns a list of DomainConfigs whose Domain.UniqueName matched the
 // filter. The filter string is a comma-separated list of domain names. If the
 // filter string is empty or "all", all domains are returned.
 func whichZonesToProcess(domains []*models.DomainConfig, filter string) []*models.DomainConfig {
-	if filter == "" || filter == "all" {
-		return domains
-	}
+	fh := domaintags.CompilePermitList(filter)
 
-	permitList := strings.Split(filter, ",")
 	var picked []*models.DomainConfig
 	for _, domain := range domains {
-		if domainInList(domain.GetUniqueName(), permitList) {
+		if fh.Permitted(domain.GetUniqueName()) {
 			picked = append(picked, domain)
 		}
 	}

commands/ppreviewPush.go

@@ -23,7 +23,7 @@ func Test_whichZonesToProcess(t *testing.T) {
 	}
 
 	for _, dc := range allDC {
-		dc.UpdateSplitHorizonNames()
+		dc.PostProcess()
 	}
 
 	type args struct {

commands/ppreviewPush_test.go

@@ -22,7 +22,8 @@ Example:
 {
   "bind": {
     "TYPE": "BIND",
-    "directory": "myzones"
+    "directory": "myzones",
+    "filenameformat": "%U.zone"
   }
 }
 ```
@@ -89,10 +90,13 @@ file name is the name as specified in the `D()` function plus ".zone".
 
 The filenameformat is a string with a few printf-like `%` verbs:
 
-  * `%U`  the domain name as specified in `D()`
-  * `%D`  the domain name without any split horizon tag (the "example.com" part of "example.com!tag")
-  * `%T`  the split horizon tag, or "" (the "tag" part of "example.com!tag")
-  * `%?x` this returns `x` if the split horizon tag is non-null, otherwise nothing. `x` can be any printable.
+  * The domain name without tag (the `example.com` part of `example.com!tag`):
+    * `%D`  as specified in `D()` (no IDN conversion, but downcased)
+    * `%I`  converted to IDN/Punycode (`xn--...`) and downcased.
+    * `%N`  converted to Unicode (downcased first)
+  * `%T`  the split horizon tag, or "" (the `tag` part of `example.com!tag`)
+  * `%?x` this returns `x` if the split horizon tag is non-null, otherwise nothing. `x` can be any printable but is usually `!`.
+  * `%U`  short for "%I%?!%T". This is the universal, canonical, name for the domain used for comparisons within DNSControl. This is best for filenames which is why it is used in the default.
   * `%%`  `%`
   * ordinary characters (not `%`) are copied unchanged to the output stream
   * FYI: format strings must not end with an incomplete `%` or `%?`
@@ -101,19 +105,17 @@ Typical values:
 
   * `%U.zone` (The default)
     * `example.com.zone` or `example.com!tag.zone`
-  * `%T%*U%D.zone`  (optional tag and `_` + domain + `.zone`)
+  * `%T%?_%I.zone`  (optional tag and `_` + domain + `.zone`)
     * `tag_example.com.zone` or `example.com.zone`
   * `db_%T%?_%D`
     * `db_inside_example.com` or `db_example.com`
-  * `db_%D`
-    * `db_example.com`
-
-The last example will generate the same name for both
-`D("example.com!inside")` and `D("example.com!outside")`.  This
-assumes two BIND providers are configured in `creds.json`, each with
-a different `directory` setting. Otherwise `dnscontrol` will write
-both domains to the same file, flapping between the two back and
-forth.
+
+{% hint style="warning" %}
+**Warning** DNSControl will not warn you if two zones generate the same
+filename. Instead, each will write to the same place.  The content would end up
+flapping back and forth between the two.  The best way to prevent this is to
+always include the tag (`%T`) or use `%U` which includes the tag.
+{% endhint %}
 
 (new in v4.2.0) `dnscontrol push` will create subdirectories along the path to
 the filename. This includes both the portion of the path created by the

documentation/provider/bind.md

@@ -40,7 +40,7 @@ func getDomainConfigWithNameservers(t *testing.T, prv providers.DNSServiceProvid
 	dc := &models.DomainConfig{
 		Name: domainName,
 	}
-	dc.UpdateSplitHorizonNames()
+	dc.PostProcess()
 
 	// fix up nameservers
 	ns, err := prv.GetNameservers(domainName)
@@ -148,6 +148,8 @@ func makeChanges(t *testing.T, prv providers.DNSServiceProvider, dc *models.Doma
 			return
 		}
 
+		//fmt.Printf("DEBUG: Running test %q: Names %q %q %q\n", desc, dom.Name, dom.NameRaw, dom.NameUnicode)
+
 		// get and run corrections for first time
 		_, corrections, actualChangeCount, err := zonerecs.CorrectZoneRecords(prv, dom)
 		if err != nil {

integrationTest/helpers_integration_test.go

@@ -105,17 +105,10 @@ type Correction struct {
 	Msg string
 }
 
-// DomainContainingFQDN finds the best domain from the dns config for the given record fqdn.
-// It will chose the domain whose name is the longest suffix match for the fqdn.
-func (config *DNSConfig) DomainContainingFQDN(fqdn string) *DomainConfig {
-	fqdn = strings.TrimSuffix(fqdn, ".")
-	longestLength := 0
-	var d *DomainConfig
-	for _, dom := range config.Domains {
-		if (dom.Name == fqdn || strings.HasSuffix(fqdn, "."+dom.Name)) && len(dom.Name) > longestLength {
-			longestLength = len(dom.Name)
-			d = dom
-		}
+// PostProcess performs and post-processing required after running dnsconfig.js and loading the result.
+func (config *DNSConfig) PostProcess() error {
+	for _, domain := range config.Domains {
+		domain.PostProcess()
 	}
-	return d
+	return nil
 }