IMPORT_TRANSFORM_SUFFIX: Fix for CNAMEs (#3192)

Author: tlimoncelli

documentation/language-reference/domain-modifiers/IMPORT_TRANSFORM_STRIP.md

@@ -16,10 +16,13 @@ Don't use this feature. It was added for a very specific situation at Stack Over
 `IMPORT_TRANSFORM_STRIP` is the same as `IMPORT_TRANSFORM` with an additional parameter: `suffixstrip`.
 
 When `IMPORT_TRANSFORM_STRIP` is generating the label for new records, it
-checks the label.  If the label ends with `suffixstrip`, that suffix is removed.
+checks the label.  If the label ends with `.` + `suffixstrip`, that suffix is removed.
 If the label does not end with `suffixstrip`, an error is returned.
 
+For CNAMEs, the `suffixstrip` is stripped from the beginning (prefix) of the target domain.
+
 For example, if the domain is `com.extra` and the label is `foo.com`,
 `IMPORT_TRANSFORM` would generate a label `foo.com.com.extra`.
 `IMPORT_TRANSFORM_STRIP(... , '.com')` would generate
 the label `foo.com.extra` instead.
+A CNAME's target would be `foo.com.extra`.

pkg/normalize/validate.go

@@ -9,7 +9,6 @@ import (
 	"github.com/StackExchange/dnscontrol/v4/models"
 	"github.com/StackExchange/dnscontrol/v4/pkg/transform"
 	"github.com/StackExchange/dnscontrol/v4/providers"
-	"github.com/miekg/dns"
 	"github.com/miekg/dns/dnsutil"
 )
 
@@ -239,13 +238,14 @@ func checkTargets(rec *models.RecordConfig, domain string) (errs []error) {
 }
 
 func transformCNAME(target, oldDomain, newDomain, suffixstrip string) string {
-	// Canonicalize. If it isn't a FQDN, add the newDomain.
-	result := dnsutil.AddOrigin(target, oldDomain)
-	if dns.IsFqdn(result) {
-		result = result[:len(result)-1]
-	}
-	result = strings.TrimSuffix(result, suffixstrip)
-	return dnsutil.AddOrigin(result, newDomain) + "."
+	// Canonicalize the target.  Add the newDomain minus the suffixstrip.
+	//  foo -> foo.oldDomain.newDomain
+	//  foo. -> foo.newDomain
+	nd := strings.TrimPrefix(newDomain, suffixstrip+".")
+	if strings.HasSuffix(target, ".") {
+		return target + nd + "."
+	}
+	return dnsutil.AddOrigin(target, oldDomain) + "." + nd + "."
 }
 
 func newRec(rec *models.RecordConfig, ttl uint32) *models.RecordConfig {
@@ -260,6 +260,7 @@ func transformLabel(label, suffixstrip string) (string, error) {
 	if suffixstrip == "" {
 		return label, nil
 	}
+	suffixstrip = "." + suffixstrip
 	if !strings.HasSuffix(label, suffixstrip) {
 		return "", fmt.Errorf("label %q does not end with %q", label, suffixstrip)
 	}

pkg/normalize/validate_test.go

@@ -186,37 +186,37 @@ func Test_transform_cname_strip(t *testing.T) {
 		p        []string
 		expected string
 	}{
-		{[]string{"ai.meta.stackexchange.com.", "stackexchange.com", "com.internal", ".com"},
+		{[]string{"ai.meta.stackexchange.com.", "stackexchange.com", "com.internal", "com"},
 			"ai.meta.stackexchange.com.internal."},
-		{[]string{"askubuntu.com.", "askubuntu.com", "com.internal", ".com"},
+		{[]string{"askubuntu.com.", "askubuntu.com", "com.internal", "com"},
 			"askubuntu.com.internal."},
-		{[]string{"blogoverflow.com.", "stackoverflow.com", "com.internal", ".com"},
+		{[]string{"blogoverflow.com.", "stackoverflow.com", "com.internal", "com"},
 			"blogoverflow.com.internal."},
-		{[]string{"careers.stackoverflow.com.", "stackoverflow.com", "com.internal", ".com"},
+		{[]string{"careers.stackoverflow.com.", "stackoverflow.com", "com.internal", "com"},
 			"careers.stackoverflow.com.internal."},
-		{[]string{"chat.stackexchange.com.", "askubuntu.com", "com.internal", ".com"},
+		{[]string{"chat.stackexchange.com.", "askubuntu.com", "com.internal", "com"},
 			"chat.stackexchange.com.internal."},
-		{[]string{"chat.stackexchange.com.", "stackoverflow.com", "com.internal", ".com"},
+		{[]string{"chat.stackexchange.com.", "stackoverflow.com", "com.internal", "com"},
 			"chat.stackexchange.com.internal."},
-		{[]string{"chat.stackexchange.com.", "superuser.com", "com.internal", ".com"},
+		{[]string{"chat.stackexchange.com.", "superuser.com", "com.internal", "com"},
 			"chat.stackexchange.com.internal."},
-		{[]string{"sstatic.net.", "sstatic.net", "net.internal", ".net"},
+		{[]string{"sstatic.net.", "sstatic.net", "net.internal", "net"},
 			"sstatic.net.internal."},
-		{[]string{"stackapps.com.", "stackapps.com", "com.internal", ".com"},
+		{[]string{"stackapps.com.", "stackapps.com", "com.internal", "com"},
 			"stackapps.com.internal."},
-		{[]string{"stackexchange.com.", "stackexchange.com", "com.internal", ".com"},
+		{[]string{"stackexchange.com.", "stackexchange.com", "com.internal", "com"},
 			"stackexchange.com.internal."},
-		{[]string{"stackoverflow.com.", "stackoverflow.com", "com.internal", ".com"},
+		{[]string{"stackoverflow.com.", "stackoverflow.com", "com.internal", "com"},
 			"stackoverflow.com.internal."},
-		{[]string{"superuser.com.", "superuser.com", "com.internal", ".com"},
+		{[]string{"superuser.com.", "superuser.com", "com.internal", "com"},
 			"superuser.com.internal."},
-		{[]string{"teststackoverflow.com.", "teststackoverflow.com", "com.internal", ".com"},
+		{[]string{"teststackoverflow.com.", "teststackoverflow.com", "com.internal", "com"},
 			"teststackoverflow.com.internal."},
-		{[]string{"webapps.stackexchange.com.", "stackexchange.com", "com.internal", ".com"},
+		{[]string{"webapps.stackexchange.com.", "stackexchange.com", "com.internal", "com"},
 			"webapps.stackexchange.com.internal."},
 		//
-		{[]string{"sstatic.net.", "sstatic.net", "com.internal", ".com"},
-			"sstatic.net.com.internal."},
+		{[]string{"sstatic.net.", "sstatic.net", "com.internal", "com"},
+			"sstatic.net.internal."},
 	}
 
 	for _, test := range tests {