[BREAKING CHNAGES] feat: use txt record instead of comments to mark managed record

pkg/cloudflare-controller/dns.go

@@ -6,29 +6,57 @@ import (
 
 	"github.com/STRRL/cloudflare-tunnel-ingress-controller/pkg/exposure"
 	"github.com/cloudflare/cloudflare-go"
+	"github.com/go-logr/logr"
 )
 
-const ManagedCNAMERecordCommentMarkFormat = "managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [%s]"
+const ManagedRecordTXTContentFormat = "managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [%s]"
+
+// ctic, abbr for cloudflare tunnel ingress controller, does not include the dot
+const ManagedRecordTXTPrefix = "_ctic_managed"
 
 type DNSOperationCreate struct {
 	Hostname string
 	Type     string
 	Content  string
-	Comment  string
 }
 
 type DNSOperationUpdate struct {
 	OldRecord cloudflare.DNSRecord
 	Type      string
 	Content   string
-	Comment   string
 }
 
 type DNSOperationDelete struct {
 	OldRecord cloudflare.DNSRecord
 }
 
-func syncDNSRecord(exposures []exposure.Exposure, existedCNAMERecords []cloudflare.DNSRecord, tunnelId string, tunnelName string) ([]DNSOperationCreate, []DNSOperationUpdate, []DNSOperationDelete, error) {
+// syncDNSRecord syncs the DNS records for the exposures.
+// It creates, updates, and deletes the DNS records based on the exposures and the existing records.
+// for example, if we want to expose a service with hostname 'dash.strrl.cloud',
+// it will create a CNAME record and a TXT record
+//
+// - CNAME: dash.strrl.cloud -> <tunnel-id>.cfargotunnel.com
+//
+// - TXT: _ctic_managed.dash.strrl.cloud -> managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [<tunnel-name>]
+//
+// the CNAME record is **required** for the cloudflare tunnel to work,
+// the TXT record is used to identify the domain is managed by this controller.
+//
+// this controller is designed as "authoritative" for the DNS records,
+// it will ALWAYS create/update CNAME records and TXT records best effort,
+// so it will override the existing records **whatever they are managed by this controller or not**.
+//
+// but things are different for the deletion,
+// this controller will only delete the CNAME record,
+// and only when the TXT record is deleted, it will delete the CNAME record.
+func syncDNSRecord(
+	logger logr.Logger,
+	exposures []exposure.Exposure,
+	existedCNAMERecords []cloudflare.DNSRecord,
+	existedTXTRecords []cloudflare.DNSRecord,
+	tunnelId string,
+	tunnelName string) ([]DNSOperationCreate, []DNSOperationUpdate, []DNSOperationDelete, error) {
+	// effective exposures would be set online later
 	var effectiveExposures []exposure.Exposure
 	for _, item := range exposures {
 		if !item.IsDeleted {
@@ -38,34 +66,74 @@ func syncDNSRecord(exposures []exposure.Exposure, existedCNAMERecords []cloudfla
 
 	var toCreate []DNSOperationCreate
 	var toUpdate []DNSOperationUpdate
+	var toDelete []DNSOperationDelete
 
+	// create or update CNAME/TXT record for exposures should online
 	for _, item := range effectiveExposures {
-		contains, old := dnsRecordsContainsHostname(existedCNAMERecords, item.Hostname)
-
-		if contains {
+		containsCNAME, oldCNAME := dnsRecordsContainsHostname(existedCNAMERecords, item.Hostname)
+
+		if containsCNAME {
+			// Check if the record is managed by this controller
+			hasTXTRecord := false
+			for _, txtRecord := range existedTXTRecords {
+				if txtRecord.Name == fmt.Sprintf("%s.%s", ManagedRecordTXTPrefix, item.Hostname) {
+					hasTXTRecord = true
+					break
+				}
+			}
+			if !hasTXTRecord {
+				logger.Info("WARNING, the origin DNS record is not managed by this controller, it would be changed to managed record",
+					"origin-record", oldCNAME,
+				)
+			}
 			toUpdate = append(toUpdate, DNSOperationUpdate{
-				OldRecord: old,
+				OldRecord: oldCNAME,
 				Type:      "CNAME",
 				Content:   tunnelDomain(tunnelId),
-				Comment:   renderDNSRecordComment(tunnelName),
 			})
 		} else {
 			toCreate = append(toCreate, DNSOperationCreate{
 				Hostname: item.Hostname,
 				Type:     "CNAME",
 				Content:  tunnelDomain(tunnelId),
-				Comment:  renderDNSRecordComment(tunnelName),
+			})
+		}
+
+		containsTXT, oldTXT := dnsRecordsContainsHostname(existedTXTRecords, fmt.Sprintf("%s.%s", ManagedRecordTXTPrefix, item.Hostname))
+		if containsTXT {
+			toUpdate = append(toUpdate, DNSOperationUpdate{
+				OldRecord: oldTXT,
+				Type:      "TXT",
+				Content:   fmt.Sprintf(ManagedRecordTXTContentFormat, tunnelName),
+			})
+		} else {
+			toCreate = append(toCreate, DNSOperationCreate{
+				Hostname: fmt.Sprintf("%s.%s", ManagedRecordTXTPrefix, item.Hostname),
+				Type:     "TXT",
+				Content:  fmt.Sprintf(ManagedRecordTXTContentFormat, tunnelName),
 			})
 		}
 	}
 
-	var toDelete []DNSOperationDelete
-	for _, item := range existedCNAMERecords {
-		contains, _ := exposureContainsHostname(effectiveExposures, item.Name)
-		if !contains {
-			if item.Comment == renderDNSRecordComment(tunnelName) {
+	// delete CNAME/TXT record for exposures should offline
+	for _, cnameRecord := range existedCNAMERecords {
+		containsCNAME, _ := exposureContainsHostname(effectiveExposures, cnameRecord.Name)
+		if !containsCNAME {
+			// Check if there's a corresponding TXT record
+			var targetTXTRecord *cloudflare.DNSRecord
+			for _, txtRecord := range existedTXTRecords {
+				txtRecord := txtRecord
+				if txtRecord.Name == fmt.Sprintf("%s.%s", ManagedRecordTXTPrefix, cnameRecord.Name) {
+					targetTXTRecord = &txtRecord
+					break
+				}
+			}
+			if targetTXTRecord != nil {
 				toDelete = append(toDelete, DNSOperationDelete{
-					OldRecord: item,
+					OldRecord: cnameRecord,
+				})
+				toDelete = append(toDelete, DNSOperationDelete{
+					OldRecord: *targetTXTRecord,
 				})
 			}
 		}
@@ -97,8 +165,3 @@ const WellKnownTunnelDomainFormat = "%s.cfargotunnel.com"
 func tunnelDomain(tunnelId string) string {
 	return strings.ToLower(fmt.Sprintf(WellKnownTunnelDomainFormat, tunnelId))
 }
-
-func renderDNSRecordComment(tunnelName string) string {
-	// TODO: comment has a limitation with max 100 char, maybe use TXT record in the future?
-	return fmt.Sprintf(ManagedCNAMERecordCommentMarkFormat, tunnelName)
-}