Add additional txt record with record type marker

In order to track multiple record types with the same name, lets migrate
to new format, were record name contains record type in it.

Signed-off-by: Dinar Valeev <dinar.valeev@absa.africa>

docs/proposal/registry.md

@@ -32,8 +32,7 @@ The following presents two ways to implement the registry, and we are planning t
 
 This implementation idea is borrowed from [Mate](https://github.com/linki/mate/)
 
-Each record created by external-dns is accompanied by the TXT record, which internally stores the external-dns identifier. For example, if external dns with `owner-id="external-dns-1"` record to be created with dns name `foo.zone.org`, external-dns will create a TXT record with the same dns name `foo.zone.org` and injected value of `"external-dns-1"`. The transfer of ownership can be done by modifying the value of the TXT record.  If no TXT record exists for the record or the value does not match its own `owner-id`, then external-dns will simply ignore it.
-
+Each record created by external-dns is accompanied by the TXT record, which internally stores the external-dns identifier. For example, if external dns with `owner-id="external-dns-1"` record to be created with dns name `foo.zone.org`, external-dns will create a TXT record with the same dns name `<record_type>-foo.zone.org` and injected value of `"external-dns-1"`. The transfer of ownership can be done by modifying the value of the TXT record.  If no TXT record exists for the record or the value does not match its own `owner-id`, then external-dns will simply ignore it.
 
 #### Goods
 1. Easy to guarantee cross-cluster ownership safety

docs/registry.md

@@ -0,0 +1,14 @@
+### TXT Registry migration to a new format ###
+
+In order to support more record types and be able to track ownership without TXT record name clash, a new TXT record is introduced.
+It contains record type it manages, e.g.:
+* A record foo.example.com will be tracked with classic foo.example.com TXT record
+* At the same time a new TXT record will be created a-foo.example.com
+
+Prefix and suffix are extended with %{record_type} template where the user can control how prefixed/suffixed records should look like.
+
+In order to maintain compatibility, both records will be maintained for some time, in order to have downgrade possibility.
+
+Later on, the old format will be dropped and only the new format will be kept (<record_type>-<endpoint_name>).
+
+Cleanup will be done by controller itself.

pkg/apis/externaldns/types.go

@@ -512,8 +512,8 @@ func (cfg *Config) ParseFlags(args []string) error {
 	// Flags related to the registry
 	app.Flag("registry", "The registry implementation to use to keep track of DNS record ownership (default: txt, options: txt, noop, aws-sd)").Default(defaultConfig.Registry).EnumVar(&cfg.Registry, "txt", "noop", "aws-sd")
 	app.Flag("txt-owner-id", "When using the TXT registry, a name that identifies this instance of ExternalDNS (default: default)").Default(defaultConfig.TXTOwnerID).StringVar(&cfg.TXTOwnerID)
-	app.Flag("txt-prefix", "When using the TXT registry, a custom string that's prefixed to each ownership DNS record (optional). Mutual exclusive with txt-suffix!").Default(defaultConfig.TXTPrefix).StringVar(&cfg.TXTPrefix)
-	app.Flag("txt-suffix", "When using the TXT registry, a custom string that's suffixed to the host portion of each ownership DNS record (optional). Mutual exclusive with txt-prefix!").Default(defaultConfig.TXTSuffix).StringVar(&cfg.TXTSuffix)
+	app.Flag("txt-prefix", "When using the TXT registry, a custom string that's prefixed to each ownership DNS record (optional). Could contain record type template like '%{record_type}-prefix-'. Mutual exclusive with txt-suffix!").Default(defaultConfig.TXTPrefix).StringVar(&cfg.TXTPrefix)
+	app.Flag("txt-suffix", "When using the TXT registry, a custom string that's suffixed to the host portion of each ownership DNS record (optional). Could contain record type template like '-%{record_type}-suffix'. Mutual exclusive with txt-prefix!").Default(defaultConfig.TXTSuffix).StringVar(&cfg.TXTSuffix)
 	app.Flag("txt-wildcard-replacement", "When using the TXT registry, a custom string that's used instead of an asterisk for TXT records corresponding to wildcard DNS records (optional)").Default(defaultConfig.TXTWildcardReplacement).StringVar(&cfg.TXTWildcardReplacement)
 
 	// Flags related to the main control loop

registry/txt.go

@@ -30,6 +30,8 @@ import (
 	"sigs.k8s.io/external-dns/provider"
 )
 
+const recordTemplate = "%{record_type}"
+
 // TXTRegistry implements registry interface with ownership implemented via associated TXT records
 type TXTRegistry struct {
 	provider provider.Provider
@@ -68,6 +70,10 @@ func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID st
 	}, nil
 }
 
+func getSupportedTypes() []string {
+	return []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS}
+}
+
 func (im *TXTRegistry) GetDomainFilter() endpoint.DomainFilterInterface {
 	return im.provider.GetDomainFilter()
 }
@@ -140,6 +146,19 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error
 	return endpoints, nil
 }
 
+// generateTXTRecord generates both "old" and "new" TXT records.
+// Once we decide to drop old format we need to drop toTXTName() and rename toNewTXTName
+func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) []*endpoint.Endpoint {
+	// old TXT record format
+	txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)).WithSetIdentifier(r.SetIdentifier)
+	txt.ProviderSpecific = r.ProviderSpecific
+	// new TXT record format (containing record type)
+	txtNew := endpoint.NewEndpoint(im.mapper.toNewTXTName(r.DNSName, r.RecordType), endpoint.RecordTypeTXT, r.Labels.Serialize(true)).WithSetIdentifier(r.SetIdentifier)
+	txtNew.ProviderSpecific = r.ProviderSpecific
+
+	return []*endpoint.Endpoint{txt, txtNew}
+}
+
 // ApplyChanges updates dns provider with the changes
 // for each created/deleted record it will also take into account TXT records for creation/deletion
 func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) error {
@@ -154,22 +173,19 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes)
 			r.Labels = make(map[string]string)
 		}
 		r.Labels[endpoint.OwnerLabelKey] = im.ownerID
-		txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)).WithSetIdentifier(r.SetIdentifier)
-		txt.ProviderSpecific = r.ProviderSpecific
-		filteredChanges.Create = append(filteredChanges.Create, txt)
+
+		filteredChanges.Create = append(filteredChanges.Create, im.generateTXTRecord(r)...)
 
 		if im.cacheInterval > 0 {
 			im.addToCache(r)
 		}
 	}
 
 	for _, r := range filteredChanges.Delete {
-		txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)).WithSetIdentifier(r.SetIdentifier)
-		txt.ProviderSpecific = r.ProviderSpecific
-
 		// when we delete TXT records for which value has changed (due to new label) this would still work because
 		// !!! TXT record value is uniquely generated from the Labels of the endpoint. Hence old TXT record can be uniquely reconstructed
-		filteredChanges.Delete = append(filteredChanges.Delete, txt)
+		// !!! After migration to the new TXT registry format we can drop records in old format here!!!
+		filteredChanges.Delete = append(filteredChanges.Delete, im.generateTXTRecord(r)...)
 
 		if im.cacheInterval > 0 {
 			im.removeFromCache(r)
@@ -178,11 +194,9 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes)
 
 	// make sure TXT records are consistently updated as well
 	for _, r := range filteredChanges.UpdateOld {
-		txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)).WithSetIdentifier(r.SetIdentifier)
-		txt.ProviderSpecific = r.ProviderSpecific
 		// when we updateOld TXT records for which value has changed (due to new label) this would still work because
 		// !!! TXT record value is uniquely generated from the Labels of the endpoint. Hence old TXT record can be uniquely reconstructed
-		filteredChanges.UpdateOld = append(filteredChanges.UpdateOld, txt)
+		filteredChanges.UpdateOld = append(filteredChanges.UpdateOld, im.generateTXTRecord(r)...)
 		// remove old version of record from cache
 		if im.cacheInterval > 0 {
 			im.removeFromCache(r)
@@ -191,9 +205,7 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes)
 
 	// make sure TXT records are consistently updated as well
 	for _, r := range filteredChanges.UpdateNew {
-		txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)).WithSetIdentifier(r.SetIdentifier)
-		txt.ProviderSpecific = r.ProviderSpecific
-		filteredChanges.UpdateNew = append(filteredChanges.UpdateNew, txt)
+		filteredChanges.UpdateNew = append(filteredChanges.UpdateNew, im.generateTXTRecord(r)...)
 		// add new version of record to cache
 		if im.cacheInterval > 0 {
 			im.addToCache(r)
@@ -229,6 +241,7 @@ func (im *TXTRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoi
 type nameMapper interface {
 	toEndpointName(string) string
 	toTXTName(string) string
+	toNewTXTName(string, string) string
 }
 
 type affixNameMapper struct {
@@ -239,37 +252,129 @@ type affixNameMapper struct {
 
 var _ nameMapper = affixNameMapper{}
 
-func newaffixNameMapper(prefix string, suffix string, wildcardReplacement string) affixNameMapper {
+func newaffixNameMapper(prefix, suffix, wildcardReplacement string) affixNameMapper {
 	return affixNameMapper{prefix: strings.ToLower(prefix), suffix: strings.ToLower(suffix), wildcardReplacement: strings.ToLower(wildcardReplacement)}
 }
 
+func dropRecordType(name string) string {
+	nameS := strings.Split(name, "-")
+	for _, t := range getSupportedTypes() {
+		if nameS[0] == strings.ToLower(t) {
+			return strings.TrimPrefix(name, nameS[0]+"-")
+		}
+	}
+	return name
+}
+
+// dropAffix strips TXT record to find an endpoint name it manages
+// It takes into consideration a fact that it could contain record type
+// So it gets stripped first
+func (pr affixNameMapper) dropAffix(name string) string {
+	if pr.recordTypeInAffix() {
+		for _, t := range getSupportedTypes() {
+			t = strings.ToLower(t)
+			iPrefix := strings.ReplaceAll(pr.prefix, recordTemplate, t)
+			iSuffix := strings.ReplaceAll(pr.suffix, recordTemplate, t)
+			if pr.isPrefix() && strings.HasPrefix(name, iPrefix) {
+				return strings.TrimPrefix(name, iPrefix)
+			}
+
+			if pr.isSuffix() && strings.HasSuffix(name, iSuffix) {
+				return strings.TrimSuffix(name, iSuffix)
+			}
+		}
+	}
+	if strings.HasPrefix(name, pr.prefix) && pr.isPrefix() {
+		return strings.TrimPrefix(name, pr.prefix)
+	}
+
+	if strings.HasSuffix(name, pr.suffix) && pr.isSuffix() {
+		return strings.TrimSuffix(name, pr.suffix)
+	}
+	return ""
+}
+
+func (pr affixNameMapper) dropAffixTemplate(name string) string {
+	return strings.ReplaceAll(name, recordTemplate, "")
+}
+
+func (pr affixNameMapper) isPrefix() bool {
+	return len(pr.suffix) == 0
+}
+func (pr affixNameMapper) isSuffix() bool {
+	return len(pr.prefix) == 0 && len(pr.suffix) > 0
+}
+
 func (pr affixNameMapper) toEndpointName(txtDNSName string) string {
-	lowerDNSName := strings.ToLower(txtDNSName)
-	if strings.HasPrefix(lowerDNSName, pr.prefix) && len(pr.suffix) == 0 {
-		return strings.TrimPrefix(lowerDNSName, pr.prefix)
+	lowerDNSName := dropRecordType(strings.ToLower(txtDNSName))
+
+	// drop prefix
+	if strings.HasPrefix(lowerDNSName, pr.prefix) && pr.isPrefix() {
+		return pr.dropAffix(lowerDNSName)
 	}
 
-	if len(pr.suffix) > 0 {
+	// drop suffix
+	if pr.isSuffix() {
 		DNSName := strings.SplitN(lowerDNSName, ".", 2)
-		if strings.HasSuffix(DNSName[0], pr.suffix) {
-			return strings.TrimSuffix(DNSName[0], pr.suffix) + "." + DNSName[1]
-		}
+		return pr.dropAffix(DNSName[0]) + "." + DNSName[1]
 	}
 	return ""
 }
 
 func (pr affixNameMapper) toTXTName(endpointDNSName string) string {
 	DNSName := strings.SplitN(endpointDNSName, ".", 2)
 
+	prefix := pr.dropAffixTemplate(pr.prefix)
+	suffix := pr.dropAffixTemplate(pr.suffix)
 	// If specified, replace a leading asterisk in the generated txt record name with some other string
 	if pr.wildcardReplacement != "" && DNSName[0] == "*" {
 		DNSName[0] = pr.wildcardReplacement
 	}
 
 	if len(DNSName) < 2 {
-		return pr.prefix + DNSName[0] + pr.suffix
+		return prefix + DNSName[0] + suffix
+	}
+	return prefix + DNSName[0] + suffix + "." + DNSName[1]
+}
+
+func (pr affixNameMapper) recordTypeInAffix() bool {
+	if strings.Contains(pr.prefix, recordTemplate) {
+		return true
 	}
-	return pr.prefix + DNSName[0] + pr.suffix + "." + DNSName[1]
+	if strings.Contains(pr.suffix, recordTemplate) {
+		return true
+	}
+	return false
+}
+
+func (pr affixNameMapper) normalizeAffixTemplate(afix, recordType string) string {
+	if strings.Contains(afix, recordTemplate) {
+		return strings.ReplaceAll(afix, recordTemplate, recordType)
+	}
+	return afix
+}
+func (pr affixNameMapper) toNewTXTName(endpointDNSName, recordType string) string {
+	DNSName := strings.SplitN(endpointDNSName, ".", 2)
+	recordType = strings.ToLower(recordType)
+	recordT := recordType + "-"
+
+	prefix := pr.normalizeAffixTemplate(pr.prefix, recordType)
+	suffix := pr.normalizeAffixTemplate(pr.suffix, recordType)
+
+	// If specified, replace a leading asterisk in the generated txt record name with some other string
+	if pr.wildcardReplacement != "" && DNSName[0] == "*" {
+		DNSName[0] = pr.wildcardReplacement
+	}
+
+	if !pr.recordTypeInAffix() {
+		DNSName[0] = recordT + DNSName[0]
+	}
+
+	if len(DNSName) < 2 {
+		return prefix + DNSName[0] + suffix
+	}
+
+	return prefix + DNSName[0] + suffix + "." + DNSName[1]
 }
 
 func (im *TXTRegistry) addToCache(ep *endpoint.Endpoint) {