fix(txt/endpoint/crd/coredns) Arbitrary TXT Record Bug Fixes and Support for CoreDNS

controller/execute.go

@@ -241,7 +241,7 @@ func buildProvider(
 	case "dnsimple":
 		p, err = dnsimple.NewDnsimpleProvider(domainFilter, zoneIDFilter, cfg.DryRun)
 	case "coredns", "skydns":
-		p, err = coredns.NewCoreDNSProvider(domainFilter, cfg.CoreDNSPrefix, cfg.DryRun)
+		p, err = coredns.NewCoreDNSProvider(domainFilter, cfg.CoreDNSPrefix, cfg.TXTOwnerID, cfg.DryRun)
 	case "exoscale":
 		p, err = exoscale.NewExoscaleProvider(
 			cfg.ExoscaleAPIEnvironment,

docs/sources/txt-record.md

@@ -1,14 +1,17 @@
 # Creating TXT record with CRD source
 
 You can create and manage TXT records with the help of [CRD source](../sources/crd.md)
-and `DNSEndpoint` CRD. Currently, this feature is only supported by `digitalocean` providers.
+and `DNSEndpoint` CRD. Currently, this feature is only supported by `digitalocean` and `coredns` providers.
 
 In order to start managing TXT records you need to set the `--managed-record-types=TXT` flag.
 
 ```console
 external-dns --source crd --provider {digitalocean} --managed-record-types=A --managed-record-types=CNAME --managed-record-types=TXT
 ```
 
+> **NOTE**: for the `coredns` provider, it is also recommended to set the `--txt-prefix` to avoid it confusing its registry TXT records within SkyDNS,
+> and `--policy=sync` in order for updates to `DNSEndpoint` to be applied.
+
 Targets within the CRD need to be specified according to the RFC 1035 (section 3.3.14). Below is an example of
 `example.com` DNS TXT two records creation.
 
@@ -27,4 +30,5 @@ spec:
       targets:
         - SOMETXT
         - ANOTHERTXT
+        - ANDEVENMORETXT.
 ```

endpoint/endpoint.go

@@ -116,11 +116,16 @@ func (t Targets) Same(o Targets) bool {
 	if len(t) != len(o) {
 		return false
 	}
-	sort.Stable(t)
-	sort.Stable(o)
-
-	for i, e := range t {
-		if !strings.EqualFold(e, o[i]) {
+	// Create copies to avoid mutating the original slices during comparison
+	tCopy := make(Targets, len(t))
+	oCopy := make(Targets, len(o))
+	copy(tCopy, t)
+	copy(oCopy, o)
+	sort.Stable(tCopy)
+	sort.Stable(oCopy)
+
+	for i, e := range tCopy {
+		if !strings.EqualFold(e, oCopy[i]) {
 			// IPv6 can be shortened, so it should be parsed for equality checking
 			ipA, err := netip.ParseAddr(e)
 			if err != nil {
@@ -130,7 +135,7 @@ func (t Targets) Same(o Targets) bool {
 				}).Debugf("Couldn't parse %s as an IP address: %v", e, err)
 			}
 
-			ipB, err := netip.ParseAddr(o[i])
+			ipB, err := netip.ParseAddr(oCopy[i])
 			if err != nil {
 				log.WithFields(log.Fields{
 					"targets":           t,
@@ -252,7 +257,13 @@ func NewEndpoint(dnsName, recordType string, targets ...string) *Endpoint {
 func NewEndpointWithTTL(dnsName, recordType string, ttl TTL, targets ...string) *Endpoint {
 	cleanTargets := make([]string, len(targets))
 	for idx, target := range targets {
-		cleanTargets[idx] = strings.TrimSuffix(target, ".")
+		// Only trim trailing dots for domain name record types, not for TXT records
+		// TXT records can contain arbitrary text including multiple dots
+		if recordType == RecordTypeTXT || recordType == RecordTypeNAPTR {
+			cleanTargets[idx] = target
+		} else {
+			cleanTargets[idx] = strings.TrimSuffix(target, ".")
+		}
 	}
 
 	for label := range strings.SplitSeq(dnsName, ".") {

endpoint/endpoint_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestNewEndpoint(t *testing.T) {
@@ -968,3 +969,58 @@ func TestEndpoint_UniqueOrderedTargets(t *testing.T) {
 		})
 	}
 }
+
+// TestNewEndpointWithTTLPreservesDotsInTXTRecords tests that trailing dots are preserved in TXT records
+func TestNewEndpointWithTTLPreservesDotsInTXTRecords(t *testing.T) {
+	// TXT records should preserve trailing dots (and any arbitrary text)
+	txtEndpoint := NewEndpointWithTTL("example.com", RecordTypeTXT, TTL(300),
+		"v=1;some_signature=aBx3d5..",
+		"text.with.dots...",
+		"simple-text")
+
+	require.NotNil(t, txtEndpoint, "TXT endpoint should be created")
+	require.Len(t, txtEndpoint.Targets, 3, "should have 3 targets")
+
+	// All dots should be preserved in TXT targets
+	assert.Equal(t, "v=1;some_signature=aBx3d5..", txtEndpoint.Targets[0])
+	assert.Equal(t, "text.with.dots...", txtEndpoint.Targets[1])
+	assert.Equal(t, "simple-text", txtEndpoint.Targets[2])
+
+	// Domain name record types should still have trailing dots trimmed
+	aEndpoint := NewEndpointWithTTL("example.com", RecordTypeA, TTL(300), "1.2.3.4.")
+	require.NotNil(t, aEndpoint, "A endpoint should be created")
+	assert.Equal(t, "1.2.3.4", aEndpoint.Targets[0], "A record should have trailing dot trimmed")
+
+	cnameEndpoint := NewEndpointWithTTL("example.com", RecordTypeCNAME, TTL(300), "target.example.com.")
+	require.NotNil(t, cnameEndpoint, "CNAME endpoint should be created")
+	assert.Equal(t, "target.example.com", cnameEndpoint.Targets[0], "CNAME record should have trailing dot trimmed")
+}
+
+// TestTargetsOrderPreservation verifies that Targets.Same() doesn't mutate target ordering
+func TestTargetsOrderPreservation(t *testing.T) {
+	// Create targets in specific YAML order (not alphabetical)
+	originalTargets := NewTargets("other.text.woo=!", "please-delete all this", "other text")
+	comparisonTargets := NewTargets("other.text.woo=!", "please-delete all this", "other text")
+
+	// Verify they are considered the same
+	assert.True(t, originalTargets.Same(comparisonTargets), "Identical targets should be considered the same")
+
+	// Verify original ordering is preserved after comparison
+	assert.Equal(t, "other.text.woo=!", originalTargets[0], "First target should remain unchanged")
+	assert.Equal(t, "please-delete all this", originalTargets[1], "Second target should remain unchanged")
+	assert.Equal(t, "other text", originalTargets[2], "Third target should remain unchanged")
+
+	// Verify comparison targets are also preserved
+	assert.Equal(t, "other.text.woo=!", comparisonTargets[0], "Comparison first target should remain unchanged")
+	assert.Equal(t, "please-delete all this", comparisonTargets[1], "Comparison second target should remain unchanged")
+	assert.Equal(t, "other text", comparisonTargets[2], "Comparison third target should remain unchanged")
+
+	// Test with alphabetically different order that should still be equal
+	reorderedTargets := NewTargets("other text", "other.text.woo=!", "please-delete all this")
+	assert.True(t, originalTargets.Same(reorderedTargets), "Targets with same content but different order should be considered equal")
+
+	// Verify original targets still preserved
+	assert.Equal(t, "other.text.woo=!", originalTargets[0], "Original first target should remain unchanged after reordered comparison")
+	assert.Equal(t, "please-delete all this", originalTargets[1], "Original second target should remain unchanged after reordered comparison")
+	assert.Equal(t, "other text", originalTargets[2], "Original third target should remain unchanged after reordered comparison")
+}

endpoint/labels.go

@@ -63,11 +63,14 @@ func NewLabelsFromStringPlain(labelText string) (Labels, error) {
 	tokens := strings.Split(labelText, ",")
 	foundExternalDNSHeritage := false
 	for _, token := range tokens {
-		if len(strings.Split(token, "=")) != 2 {
+		// Split on the last occurrence of '=' since target content can contain '=' characters
+		// but the hash value (after the last '=') should not contain '='
+		lastEquals := strings.LastIndex(token, "=")
+		if lastEquals == -1 {
 			continue
 		}
-		key := strings.Split(token, "=")[0]
-		val := strings.Split(token, "=")[1]
+		key := token[:lastEquals]
+		val := token[lastEquals+1:]
 		if key == "heritage" && val != heritage {
 			return nil, ErrInvalidHeritage
 		}

endpoint/labels_test.go

@@ -23,6 +23,7 @@ import (
 	"testing"
 
 	log "github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -181,3 +182,21 @@ func (suite *LabelsSuite) TestDeserialize() {
 func TestLabels(t *testing.T) {
 	suite.Run(t, new(LabelsSuite))
 }
+
+// TestLabelsWithEqualsInValue tests label parsing when values contain '=' characters
+func TestLabelsWithEqualsInValue(t *testing.T) {
+	// This simulates the bug where label values containing '=' characters were truncated
+	labelText := `"heritage=external-dns,external-dns/owner=default,external-dns/prefix=default,external-dns/v=1;some_signature=aBx3d5..====1b6eef32"`
+
+	labels, err := NewLabelsFromStringPlain(labelText)
+	require.NoError(t, err, "should succeed for valid label text with '=' in values")
+
+	// Verify the full enode string is preserved
+	expectedValue := "1b6eef32"
+	actualValue := labels["v=1;some_signature=aBx3d5..==="]
+	require.Equal(t, expectedValue, actualValue, "should preserve full value including '=' characters")
+
+	// Verify other labels are also parsed correctly
+	require.Equal(t, "default", labels["owner"])
+	require.Equal(t, "default", labels["prefix"])
+}