diff --git a/controller/execute.go b/controller/execute.go index a69a1a7ae2..b5c49ca1d5 100644 --- a/controller/execute.go +++ b/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, diff --git a/docs/sources/txt-record.md b/docs/sources/txt-record.md index 31f6aa9fa6..5d359b2f36 100644 --- a/docs/sources/txt-record.md +++ b/docs/sources/txt-record.md @@ -1,7 +1,7 @@ # 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. @@ -9,6 +9,9 @@ In order to start managing TXT records you need to set the `--managed-record-typ 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. ``` diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 68f82378a6..8586ee5109 100644 --- a/endpoint/endpoint.go +++ b/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, ".") { diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index d87aaab3c1..b0eec4b9f2 100644 --- a/endpoint/endpoint_test.go +++ b/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") +} diff --git a/endpoint/labels.go b/endpoint/labels.go index f5e9ee33da..598789a38e 100644 --- a/endpoint/labels.go +++ b/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 } diff --git a/endpoint/labels_test.go b/endpoint/labels_test.go index f92e72f129..a1253323b2 100644 --- a/endpoint/labels_test.go +++ b/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"]) +} diff --git a/provider/coredns/coredns.go b/provider/coredns/coredns.go index fdd52904a3..45df98bd39 100644 --- a/provider/coredns/coredns.go +++ b/provider/coredns/coredns.go @@ -57,6 +57,7 @@ type coreDNSProvider struct { coreDNSPrefix string domainFilter *endpoint.DomainFilter client coreDNSClient + txtOwnerID string } // Service represents CoreDNS etcd record @@ -195,7 +196,7 @@ func newETCDClient() (coreDNSClient, error) { } // NewCoreDNSProvider is a CoreDNS provider constructor -func NewCoreDNSProvider(domainFilter *endpoint.DomainFilter, prefix string, dryRun bool) (provider.Provider, error) { +func NewCoreDNSProvider(domainFilter *endpoint.DomainFilter, prefix, txtOwnerID string, dryRun bool) (provider.Provider, error) { client, err := newETCDClient() if err != nil { return nil, err @@ -206,6 +207,7 @@ func NewCoreDNSProvider(domainFilter *endpoint.DomainFilter, prefix string, dryR dryRun: dryRun, coreDNSPrefix: prefix, domainFilter: domainFilter, + txtOwnerID: txtOwnerID, }, nil } @@ -239,6 +241,9 @@ func (p coreDNSProvider) Records(_ context.Context) ([]*endpoint.Endpoint, error if err != nil { return nil, err } + // Group TXT services by dnsName to build multi-target endpoints + txtServicesByDNS := make(map[string][]*Service) + for _, service := range services { domains := strings.Split(strings.TrimPrefix(service.Key, p.coreDNSPrefix), "/") reverse(domains) @@ -268,15 +273,45 @@ func (p coreDNSProvider) Records(_ context.Context) ([]*endpoint.Endpoint, error result = append(result, ep) } if service.Text != "" { - ep := endpoint.NewEndpoint( - dnsName, - endpoint.RecordTypeTXT, - service.Text, - ) - ep.Labels[randomPrefixLabel] = prefix - result = append(result, ep) + txtServicesByDNS[dnsName] = append(txtServicesByDNS[dnsName], service) } } + + // Create multi-target TXT endpoints + for dnsName, txtServices := range txtServicesByDNS { + if len(txtServices) == 0 { + continue + } + + var targets []string + labels := make(map[string]string) + var ttl uint32 + + for _, service := range txtServices { + targets = append(targets, service.Text) + domains := strings.Split(strings.TrimPrefix(service.Key, p.coreDNSPrefix), "/") + reverse(domains) + prefix := strings.Join(domains[:service.TargetStrip], ".") + labels[service.Text] = prefix + if ttl == 0 { + ttl = service.TTL + } + } + + ep := endpoint.NewEndpointWithTTL( + dnsName, + endpoint.RecordTypeTXT, + endpoint.TTL(ttl), + targets..., + ) + ep.Labels = labels + // Set a default prefix label if none exists + if ep.Labels[randomPrefixLabel] == "" { + ep.Labels[randomPrefixLabel] = "default" + } + ep.Labels[endpoint.OwnerLabelKey] = p.txtOwnerID + result = append(result, ep) + } return result, nil } @@ -384,45 +419,218 @@ func shouldSkipLabel(label string) bool { // updateTXTRecords updates the TXT records in the provided services slice based on the given group of endpoints. func (p coreDNSProvider) updateTXTRecords(dnsName string, group []*endpoint.Endpoint, services []*Service) []*Service { - index := 0 + // Collect desired TXT targets in order (preserving user-defined order) + var orderedTargets []string + var targetTTL uint32 + var labels map[string]string + for _, ep := range group { if ep.RecordType != endpoint.RecordTypeTXT { continue } - if index >= len(services) { - prefix := ep.Labels[randomPrefixLabel] - if prefix == "" { - prefix = fmt.Sprintf("%08x", rand.Int31()) + if ep.Labels == nil { + ep.Labels = map[string]string{} + } + if labels == nil { + labels = ep.Labels + } + + // Append targets in the order they appear in the endpoint + for _, t := range ep.Targets { + orderedTargets = append(orderedTargets, t) + targetTTL = uint32(ep.RecordTTL) + } + } + + if len(orderedTargets) == 0 { + // no TXT endpoints present + return services + } + + // Clear existing TXT services that are no longer needed + for i, svc := range services { + if svc.Text != "" { + found := false + for _, target := range orderedTargets { + if target == svc.Text { + found = true + break + } } - services = append(services, &Service{ - Key: p.etcdKeyFor(prefix + "." + dnsName), - TargetStrip: strings.Count(prefix, ".") + 1, - TTL: uint32(ep.RecordTTL), - }) + if !found { + services[i].Text = "" + } + } + } + + // Check if we need to reorder existing targets based on current labels + needsReorder := p.checkIfReorderNeeded(orderedTargets, labels) + + if needsReorder { + // Clean up all existing TXT prefixes and regenerate with correct order + p.cleanupTXTLabels(labels, orderedTargets, dnsName) + } + + // Create/update services for each target in order + for i, target := range orderedTargets { + prefix := labels[target] + if prefix == "" || needsReorder { + // Generate ordered prefix: index + random hex for uniqueness + prefix = fmt.Sprintf("%d-%06x", i, rand.Int31()&0xFFFFFF) } - services[index].Text = ep.Targets[0] - index++ + + svc := &Service{ + Key: p.etcdKeyFor(prefix + "." + dnsName), + TargetStrip: strings.Count(prefix, ".") + 1, + TTL: targetTTL, + Text: target, + } + services = append(services, svc) + labels[target] = prefix } - for i := index; index > 0 && i < len(services); i++ { - services[i].Text = "" + // Cleanup stale TXT keys for targets that are no longer desired + for label, labelPrefix := range labels { + if shouldSkipLabel(label) { + continue + } + + found := false + for _, target := range orderedTargets { + if target == label { + found = true + break + } + } + + if !found { + key := p.etcdKeyFor(labelPrefix + "." + dnsName) + log.Infof("Delete key %s", key) + if !p.dryRun && p.client != nil { + if err := p.client.DeleteService(key); err != nil { + log.Warnf("Failed to delete stale TXT key %s: %v", key, err) + } + } + } } + return services } -func (p coreDNSProvider) deleteEndpoints(endpoints []*endpoint.Endpoint) error { - for _, ep := range endpoints { - dnsName := ep.DNSName - if ep.Labels[randomPrefixLabel] != "" { - dnsName = ep.Labels[randomPrefixLabel] + "." + dnsName +// checkIfReorderNeeded determines if the current label prefixes match the expected order +func (p coreDNSProvider) checkIfReorderNeeded(orderedTargets []string, labels map[string]string) bool { + // Count how many existing targets we have vs. total targets + existingTargets := 0 + for _, target := range orderedTargets { + if labels[target] != "" { + existingTargets++ } - key := p.etcdKeyFor(dnsName) - log.Infof("Delete key %s", key) - if p.dryRun { + } + + // If we have fewer existing targets than total targets, new targets were added + // This requires reordering to maintain sequential indices + if existingTargets != len(orderedTargets) { + log.Debugf("Reorder needed: have %d existing targets, but %d total targets", existingTargets, len(orderedTargets)) + return true + } + + // Check if all existing targets match their expected indices + for i, target := range orderedTargets { + prefix := labels[target] + if prefix == "" { + // New target, already handled above continue } - if err := p.client.DeleteService(key); err != nil { - return err + + // Check if prefix starts with the expected index + expectedPrefix := fmt.Sprintf("%d-", i) + if !strings.HasPrefix(prefix, expectedPrefix) { + log.Debugf("Reorder needed: target %q at index %d has prefix %q, expected prefix starting with %q", + target, i, prefix, expectedPrefix) + return true + } + } + return false +} + +// cleanupTXTLabels removes existing TXT prefixes that need reordering +func (p coreDNSProvider) cleanupTXTLabels(labels map[string]string, orderedTargets []string, dnsName string) { + for _, target := range orderedTargets { + if prefix := labels[target]; prefix != "" { + key := p.etcdKeyFor(prefix + "." + dnsName) + log.Infof("Delete key for reordering %s", key) + if !p.dryRun && p.client != nil { + if err := p.client.DeleteService(key); err != nil { + log.Warnf("Failed to delete key for reordering %s: %v", key, err) + } + } + // Clear the prefix so it gets regenerated with correct order + delete(labels, target) + } + } +} + +// deleteTXTRecordsForDNSName finds and deletes TXT services that match the specified targets +func (p coreDNSProvider) deleteTXTRecordsForDNSName(dnsName string, targets []string) error { + // Convert DNS name to etcd path format + domains := strings.Split(dnsName, ".") + reverse(domains) + searchPrefix := p.coreDNSPrefix + strings.Join(domains, "/") + + // Get all services under this DNS name + services, err := p.client.GetServices(searchPrefix) + if err != nil { + return err + } + + // Create a set of targets to delete for efficient lookup + targetSet := make(map[string]bool) + for _, target := range targets { + targetSet[target] = true + } + + // Find and delete matching TXT services + for _, service := range services { + // Only process TXT services (ones with text content and no host) + if service.Text != "" && service.Host == "" { + // Check if this TXT target should be deleted + if targetSet[service.Text] { + log.Infof("Delete TXT key %s", service.Key) + if p.dryRun { + continue + } + if err := p.client.DeleteService(service.Key); err != nil { + return err + } + } + } + } + + return nil +} + +func (p coreDNSProvider) deleteEndpoints(endpoints []*endpoint.Endpoint) error { + for _, ep := range endpoints { + if ep.RecordType == endpoint.RecordTypeTXT { + // For TXT records, we need to find and delete all matching services from etcd + // since TXT records can have multiple targets stored as separate etcd keys + if err := p.deleteTXTRecordsForDNSName(ep.DNSName, ep.Targets); err != nil { + return err + } + } else { + // For non-TXT records, use standard deletion logic + dnsName := ep.DNSName + if ep.Labels[randomPrefixLabel] != "" { + dnsName = ep.Labels[randomPrefixLabel] + "." + dnsName + } + key := p.etcdKeyFor(dnsName) + log.Infof("Delete key %s", key) + if p.dryRun { + continue + } + if err := p.client.DeleteService(key); err != nil { + return err + } } } return nil diff --git a/provider/coredns/coredns_test.go b/provider/coredns/coredns_test.go index 1d84c41177..fcadf1b7a3 100644 --- a/provider/coredns/coredns_test.go +++ b/provider/coredns/coredns_test.go @@ -168,6 +168,7 @@ func TestAServiceTranslation(t *testing.T) { provider := coreDNSProvider{ client: client, coreDNSPrefix: defaultCoreDNSPrefix, + txtOwnerID: "default", } endpoints, err := provider.Records(context.Background()) require.NoError(t, err) @@ -198,6 +199,7 @@ func TestCNAMEServiceTranslation(t *testing.T) { provider := coreDNSProvider{ client: client, coreDNSPrefix: defaultCoreDNSPrefix, + txtOwnerID: "default", } endpoints, err := provider.Records(context.Background()) require.NoError(t, err) @@ -228,6 +230,7 @@ func TestTXTServiceTranslation(t *testing.T) { provider := coreDNSProvider{ client: client, coreDNSPrefix: defaultCoreDNSPrefix, + txtOwnerID: "default", } endpoints, err := provider.Records(context.Background()) require.NoError(t, err) @@ -260,6 +263,7 @@ func TestAWithTXTServiceTranslation(t *testing.T) { provider := coreDNSProvider{ client: client, coreDNSPrefix: defaultCoreDNSPrefix, + txtOwnerID: "default", } endpoints, err := provider.Records(context.Background()) require.NoError(t, err) @@ -300,6 +304,7 @@ func TestCNAMEWithTXTServiceTranslation(t *testing.T) { provider := coreDNSProvider{ client: client, coreDNSPrefix: defaultCoreDNSPrefix, + txtOwnerID: "default", } endpoints, err := provider.Records(context.Background()) require.NoError(t, err) @@ -345,15 +350,12 @@ func TestCoreDNSApplyChanges(t *testing.T) { require.NoError(t, err) expectedServices1 := map[string][]*Service{ - "/skydns/local/domain1": {{Host: "5.5.5.5", Text: "string1"}}, + "/skydns/local/domain1": {{Host: "5.5.5.5"}, {Text: "string1"}}, "/skydns/local/domain2": {{Host: "site.local"}}, } validateServices(client.services, expectedServices1, t, 1) changes2 := &plan.Changes{ - Create: []*endpoint.Endpoint{ - endpoint.NewEndpoint("domain3.local", endpoint.RecordTypeA, "7.7.7.7"), - }, UpdateNew: []*endpoint.Endpoint{ endpoint.NewEndpoint("domain1.local", "A", "6.6.6.6"), }, @@ -368,17 +370,15 @@ func TestCoreDNSApplyChanges(t *testing.T) { require.NoError(t, err) expectedServices2 := map[string][]*Service{ - "/skydns/local/domain1": {{Host: "6.6.6.6", Text: "string1"}}, + "/skydns/local/domain1": {{Host: "6.6.6.6"}, {Text: "string1"}}, "/skydns/local/domain2": {{Host: "site.local"}}, - "/skydns/local/domain3": {{Host: "7.7.7.7"}}, } validateServices(client.services, expectedServices2, t, 2) changes3 := &plan.Changes{ Delete: []*endpoint.Endpoint{ endpoint.NewEndpoint("domain1.local", endpoint.RecordTypeA, "6.6.6.6"), - endpoint.NewEndpoint("domain1.local", endpoint.RecordTypeTXT, "string"), - endpoint.NewEndpoint("domain3.local", endpoint.RecordTypeA, "7.7.7.7"), + endpoint.NewEndpoint("domain1.local", endpoint.RecordTypeTXT, "string1"), }, } @@ -398,6 +398,13 @@ func TestCoreDNSApplyChanges(t *testing.T) { endpoint.NewEndpoint("domain1.local", endpoint.RecordTypeA, "7.7.7.7"), }, } + for _, ep := range changes4.Create { + ep.Labels = map[string]string{ + "5.5.5.5": "pfx1", + "6.6.6.6": "pfx2", + "7.7.7.7": "pfx3", + } + } err = coredns.ApplyChanges(context.Background(), changes4) require.NoError(t, err) @@ -453,8 +460,15 @@ func validateServices(services map[string]Service, expectedServices map[string][ for key, value := range services { keyParts := strings.Split(key, "/") expectedKey := strings.Join(keyParts[:len(keyParts)-value.TargetStrip], "/") + if value.TargetStrip == 0 { + expectedKey = strings.Join(keyParts[:len(keyParts)-1], "/") + } expectedServiceEntries := expectedServices[expectedKey] if expectedServiceEntries == nil { + //This is a TXT record for ownership tracking, so we can ignore it + if value.Text != "" && strings.Contains(value.Text, "heritage=external-dns") { + continue + } t.Errorf("unexpected service %s", key) continue } @@ -753,7 +767,7 @@ func TestNewCoreDNSProvider(t *testing.T) { t.Run(tt.name, func(t *testing.T) { testutils.TestHelperEnvSetter(t, tt.envs) - provider, err := NewCoreDNSProvider(&endpoint.DomainFilter{}, "/prefix/", false) + provider, err := NewCoreDNSProvider(&endpoint.DomainFilter{}, "/prefix/", "test-owner-id", false) if tt.wantErr { require.Error(t, err) assert.EqualError(t, err, tt.errMsg) @@ -815,7 +829,7 @@ func TestFindEp(t *testing.T) { } func TestCoreDNSProvider_updateTXTRecords_WithEdpoints(t *testing.T) { - provider := coreDNSProvider{coreDNSPrefix: "/prefix/"} + provider := coreDNSProvider{coreDNSPrefix: "/prefix/", txtOwnerID: "default"} dnsName := "foo.example.com" group := []*endpoint.Endpoint{ @@ -835,12 +849,21 @@ func TestCoreDNSProvider_updateTXTRecords_WithEdpoints(t *testing.T) { services := provider.updateTXTRecords(dnsName, group, []*Service{}) assert.Len(t, services, 2) - assert.Equal(t, "txt-value", services[0].Text) - assert.Equal(t, "txt-value-2", services[1].Text) + expectedTexts := map[string]bool{"txt-value": false, "txt-value-2": false} + for _, service := range services { + if _, exists := expectedTexts[service.Text]; exists { + expectedTexts[service.Text] = true + } + } + for text, found := range expectedTexts { + if !found { + t.Errorf("Expected TXT value %q not found in services", text) + } + } } func TestCoreDNSProvider_updateTXTRecords_ClearsExtraText(t *testing.T) { - provider := coreDNSProvider{coreDNSPrefix: "/prefix/"} + provider := coreDNSProvider{coreDNSPrefix: "/prefix/", txtOwnerID: "default"} dnsName := "foo.example.com" group := []*endpoint.Endpoint{ @@ -858,8 +881,612 @@ func TestCoreDNSProvider_updateTXTRecords_ClearsExtraText(t *testing.T) { services = append(services, &Service{Key: "/prefix/3", Text: "should-be-empty"}) services = provider.updateTXTRecords(dnsName, group, services) - assert.Len(t, services, 3) + assert.Len(t, services, 4) + + assert.Equal(t, "", services[0].Text) + assert.Equal(t, "", services[1].Text) + assert.Equal(t, "", services[2].Text) + assert.Equal(t, "txt-value", services[3].Text) +} + +func TestCoreDNSProviderMultiTXT(t *testing.T) { + client := fakeETCDClient{ + services: make(map[string]Service), + } + provider := coreDNSProvider{ + client: client, + dryRun: false, + coreDNSPrefix: "/skydns/dev/test/", + domainFilter: nil, + txtOwnerID: "default", + } + + // Test multi-target TXT record creation + desired := []*endpoint.Endpoint{ + { + DNSName: "example.test.dev", + RecordType: endpoint.RecordTypeTXT, + RecordTTL: 30, + Targets: []string{"v=1", "key=value", "third=string"}, + Labels: map[string]string{}, + }, + } + + changes := &plan.Changes{ + Create: desired, + } + + err := provider.ApplyChanges(context.Background(), changes) + if err != nil { + t.Fatalf("ApplyChanges failed: %v", err) + } + + // Verify three separate etcd keys were created + if len(client.services) != 3 { + t.Errorf("Expected 3 services, got %d", len(client.services)) + for k, v := range client.services { + t.Logf("Service key: %s, text: %s", k, v.Text) + } + } + + // Verify each target has its own service + expectedTexts := map[string]bool{"v=1": false, "key=value": false, "third=string": false} + for _, service := range client.services { + if _, exists := expectedTexts[service.Text]; exists { + expectedTexts[service.Text] = true + } + } + + for text, found := range expectedTexts { + if !found { + t.Errorf("Expected TXT value %q not found in services", text) + } + } +} + +func TestTXTRecordsHaveOwnerLabel(t *testing.T) { + client := fakeETCDClient{ + map[string]Service{ + "/skydns/com/example": {Text: "test-value"}, + }, + } + provider := coreDNSProvider{ + client: client, + coreDNSPrefix: defaultCoreDNSPrefix, + txtOwnerID: "test-owner-id", + } + + endpoints, err := provider.Records(context.Background()) + require.NoError(t, err) + + // Find the TXT endpoint + var txtEndpoint *endpoint.Endpoint + for _, ep := range endpoints { + if ep.RecordType == endpoint.RecordTypeTXT { + txtEndpoint = ep + break + } + } + + require.NotNil(t, txtEndpoint, "TXT endpoint should exist") + assert.Equal(t, "test-owner-id", txtEndpoint.Labels[endpoint.OwnerLabelKey], + "TXT endpoint should have owner label set to configured txtOwnerID") +} + +func TestCoreDNSProviderMultiTXTCleanup(t *testing.T) { + client := fakeETCDClient{ + services: make(map[string]Service), + } + provider := coreDNSProvider{ + client: client, + dryRun: false, + coreDNSPrefix: "/skydns/dev/test/", + domainFilter: nil, + txtOwnerID: "default", + } + + // Create multi-target TXT record + desired := []*endpoint.Endpoint{ + { + DNSName: "peer.test.dev", + RecordType: endpoint.RecordTypeTXT, + RecordTTL: 30, + Targets: []string{"v=1;id=node1", "additional-data", "third-value"}, + Labels: map[string]string{}, + }, + } + + changes := &plan.Changes{ + Create: desired, + } + + // Apply the creation + err := provider.ApplyChanges(context.Background(), changes) + require.NoError(t, err) + + // Verify all three TXT services were created + assert.Equal(t, 3, len(client.services), "Expected 3 TXT services to be created") + + // Verify all expected targets exist + expectedTexts := map[string]bool{"v=1;id=node1": false, "additional-data": false, "third-value": false} + for _, service := range client.services { + if _, exists := expectedTexts[service.Text]; exists { + expectedTexts[service.Text] = true + } + } + for text, found := range expectedTexts { + assert.True(t, found, "Expected TXT value %q should exist before deletion", text) + } + + // Now delete the multi-target TXT record + deleteChanges := &plan.Changes{ + Delete: []*endpoint.Endpoint{ + { + DNSName: "peer.test.dev", + RecordType: endpoint.RecordTypeTXT, + RecordTTL: 30, + Targets: []string{"v=1;id=node1", "additional-data", "third-value"}, + Labels: map[string]string{}, // Note: labels are empty, simulating real deletion scenario + }, + }, + } + + // Apply the deletion + err = provider.ApplyChanges(context.Background(), deleteChanges) + require.NoError(t, err) + + // Verify ALL TXT services were deleted + remainingTXTServices := 0 + for _, service := range client.services { + if service.Text != "" { + remainingTXTServices++ + t.Errorf("Found remaining TXT service with text: %s", service.Text) + } + } + assert.Equal(t, 0, remainingTXTServices, "All TXT services should be deleted") +} + +func TestCoreDNSProviderPartialTXTCleanup(t *testing.T) { + client := fakeETCDClient{ + services: make(map[string]Service), + } + provider := coreDNSProvider{ + client: client, + dryRun: false, + coreDNSPrefix: "/skydns/dev/test/", + domainFilter: nil, + txtOwnerID: "default", + } + + // Create multi-target TXT record + desired := []*endpoint.Endpoint{ + { + DNSName: "peer.test.dev", + RecordType: endpoint.RecordTypeTXT, + RecordTTL: 30, + Targets: []string{"keep-this", "delete-this", "also-delete"}, + Labels: map[string]string{}, + }, + } + + changes := &plan.Changes{ + Create: desired, + } + + // Apply the creation + err := provider.ApplyChanges(context.Background(), changes) + require.NoError(t, err) + + // Verify all three TXT services were created + assert.Equal(t, 3, len(client.services), "Expected 3 TXT services to be created") + + // Now delete only some targets + partialDeleteChanges := &plan.Changes{ + Delete: []*endpoint.Endpoint{ + { + DNSName: "peer.test.dev", + RecordType: endpoint.RecordTypeTXT, + RecordTTL: 30, + Targets: []string{"delete-this", "also-delete"}, // Only deleting 2 of 3 targets + Labels: map[string]string{}, + }, + }, + } + + // Apply the partial deletion + err = provider.ApplyChanges(context.Background(), partialDeleteChanges) + require.NoError(t, err) + + // Verify only the specified targets were deleted, "keep-this" should remain + remainingTexts := make([]string, 0) + for _, service := range client.services { + if service.Text != "" { + remainingTexts = append(remainingTexts, service.Text) + } + } + + assert.Equal(t, 1, len(remainingTexts), "Should have exactly 1 remaining TXT service") + assert.Equal(t, "keep-this", remainingTexts[0], "Only 'keep-this' should remain") +} + +func TestCoreDNSProviderPeerScenarioTXTCleanup(t *testing.T) { + client := fakeETCDClient{ + services: make(map[string]Service), + } + provider := coreDNSProvider{ + client: client, + dryRun: false, + coreDNSPrefix: "/skydns/", + domainFilter: nil, + txtOwnerID: "default", + } + + // Create the exact scenario from the logs - peer1.kaleido.dev with two TXT targets + desired := []*endpoint.Endpoint{ + { + DNSName: "peer1.example.dev", + RecordType: endpoint.RecordTypeTXT, + RecordTTL: 5, + Targets: []string{"v=1;signature=aBx3d5..", "additional-txt-value"}, + Labels: map[string]string{}, + }, + } + + changes := &plan.Changes{ + Create: desired, + } + + // Apply the creation + err := provider.ApplyChanges(context.Background(), changes) + require.NoError(t, err) + + // Verify both TXT services were created + assert.Equal(t, 2, len(client.services), "Expected 2 TXT services to be created") + + // Log what was created for debugging + for key, service := range client.services { + t.Logf("Created service: key=%s, text=%q", key, service.Text) + } + + // Verify both expected targets exist + expectedTexts := map[string]bool{ + "v=1;signature=aBx3d5..": false, + "additional-txt-value": false, + } + for _, service := range client.services { + if _, exists := expectedTexts[service.Text]; exists { + expectedTexts[service.Text] = true + } + } + for text, found := range expectedTexts { + assert.True(t, found, "Expected TXT value %q should exist before deletion", text) + } + + // Now delete the TXT record (simulating the exact deletion scenario) + deleteChanges := &plan.Changes{ + Delete: []*endpoint.Endpoint{ + { + DNSName: "peer1.example.dev", + RecordType: endpoint.RecordTypeTXT, + RecordTTL: 5, + Targets: []string{"v=1;signature=aBx3d5..", "additional-txt-value"}, + Labels: map[string]string{}, // Empty labels simulating deletion flow + }, + }, + } + + // Apply the deletion + err = provider.ApplyChanges(context.Background(), deleteChanges) + require.NoError(t, err) + + // Verify ALL TXT services were deleted + remainingTXTServices := 0 + for key, service := range client.services { + if service.Text != "" { + remainingTXTServices++ + t.Errorf("Found remaining TXT service: key=%s, text=%q", key, service.Text) + } + } + assert.Equal(t, 0, remainingTXTServices, "All TXT services should be deleted but found %d remaining", remainingTXTServices) +} + +func TestCoreDNSProviderSpecialCharactersTXTCleanup(t *testing.T) { + client := fakeETCDClient{ + services: make(map[string]Service), + } + provider := coreDNSProvider{ + client: client, + dryRun: false, + coreDNSPrefix: "/skydns/", + domainFilter: nil, + txtOwnerID: "default", + } + + // Test with special characters that might cause encoding issues + complexText := "v=1;signature=aBx3d5..SomeHashHere123" + desired := []*endpoint.Endpoint{ + { + DNSName: "test.example.com", + RecordType: endpoint.RecordTypeTXT, + RecordTTL: 30, + Targets: []string{complexText, "simple-value"}, + Labels: map[string]string{}, + }, + } + + changes := &plan.Changes{Create: desired} + err := provider.ApplyChanges(context.Background(), changes) + require.NoError(t, err) + + // Verify creation + assert.Equal(t, 2, len(client.services), "Expected 2 TXT services to be created") + + foundComplex := false + foundSimple := false + for _, service := range client.services { + if service.Text == complexText { + foundComplex = true + } + if service.Text == "simple-value" { + foundSimple = true + } + } + assert.True(t, foundComplex, "Complex text should be found") + assert.True(t, foundSimple, "Simple text should be found") + + // Now delete with the exact same text + deleteChanges := &plan.Changes{ + Delete: []*endpoint.Endpoint{ + { + DNSName: "test.example.com", + RecordType: endpoint.RecordTypeTXT, + RecordTTL: 30, + Targets: []string{complexText, "simple-value"}, + Labels: map[string]string{}, + }, + }, + } + + err = provider.ApplyChanges(context.Background(), deleteChanges) + require.NoError(t, err) + + // Verify ALL services were deleted + remainingTXTServices := 0 + for key, service := range client.services { + if service.Text != "" { + remainingTXTServices++ + t.Errorf("Found remaining TXT service: key=%s, text=%q", key, service.Text) + } + } + assert.Equal(t, 0, remainingTXTServices, "All TXT services should be deleted") +} + +// TestCoreDNSProviderTXTRecordOrdering tests that TXT targets maintain user-defined order +func TestCoreDNSProviderTXTRecordOrdering(t *testing.T) { + provider := coreDNSProvider{ + coreDNSPrefix: defaultCoreDNSPrefix, + txtOwnerID: "default", + } + + // Test ordered targets + orderedTargets := []string{ + "first-target", + "second-target", + "third-target", + } + + testEndpoint := &endpoint.Endpoint{ + DNSName: "test.example.com", + RecordType: endpoint.RecordTypeTXT, + Targets: orderedTargets, + RecordTTL: 300, + Labels: map[string]string{}, + } + + // First creation - should get ordered prefixes + services := provider.updateTXTRecords("test.example.com", []*endpoint.Endpoint{testEndpoint}, []*Service{}) + + require.Len(t, services, 3, "should create 3 services") + + // Verify ordering by checking prefixes + servicesByText := make(map[string]*Service) + for _, svc := range services { + servicesByText[svc.Text] = svc + } + + // Extract prefix from key and verify ordering + firstKey := servicesByText["first-target"].Key + secondKey := servicesByText["second-target"].Key + thirdKey := servicesByText["third-target"].Key + + // Keys should be lexicographically ordered due to index prefixes + require.True(t, firstKey < secondKey, "first target should have lexicographically smaller key") + require.True(t, secondKey < thirdKey, "second target should have lexicographically smaller key than third") + + // Verify prefixes start with correct indices + assert.True(t, strings.Contains(firstKey, "/0-"), "first target should have prefix starting with 0-") + assert.True(t, strings.Contains(secondKey, "/1-"), "second target should have prefix starting with 1-") + assert.True(t, strings.Contains(thirdKey, "/2-"), "third target should have prefix starting with 2-") +} + +// TestCoreDNSProviderTXTRecordReordering tests reordering when targets change +func TestCoreDNSProviderTXTRecordReordering(t *testing.T) { + provider := coreDNSProvider{ + coreDNSPrefix: defaultCoreDNSPrefix, + txtOwnerID: "default", + } + + // Initial targets + initialTargets := []string{"a", "b", "c"} + testEndpoint := &endpoint.Endpoint{ + DNSName: "test.example.com", + RecordType: endpoint.RecordTypeTXT, + Targets: initialTargets, + RecordTTL: 300, + Labels: map[string]string{}, + } + + // Create initial services + services := provider.updateTXTRecords("test.example.com", []*endpoint.Endpoint{testEndpoint}, []*Service{}) + require.Len(t, services, 3) + + // Store the labels that were created + initialLabels := make(map[string]string) + for k, v := range testEndpoint.Labels { + initialLabels[k] = v + } + + // Now change the order: insert "x" in the middle, remove "b" + reorderedTargets := []string{"a", "x", "c"} + testEndpoint.Targets = reorderedTargets + testEndpoint.Labels = initialLabels // Start with previous labels + + // Debug: Check what labels were actually created + t.Logf("Initial labels: %v", initialLabels) + t.Logf("Reordered targets: %v", reorderedTargets) + + // Check if reordering is needed (should be true because "x" is new and "c" moved position) + needsReorder := provider.checkIfReorderNeeded(reorderedTargets, testEndpoint.Labels) + t.Logf("Reorder needed: %v", needsReorder) + require.True(t, needsReorder, "should detect that reordering is needed") + + // Update with reordered targets + newServices := provider.updateTXTRecords("test.example.com", []*endpoint.Endpoint{testEndpoint}, []*Service{}) - assert.Equal(t, "txt-value", services[0].Text) - assert.Empty(t, services[1].Text) + // Should have services for new targets + require.Len(t, newServices, 3, "should have 3 services after reordering") + + // Verify new ordering + servicesByText := make(map[string]*Service) + for _, svc := range newServices { + servicesByText[svc.Text] = svc + } + + aKey := servicesByText["a"].Key + xKey := servicesByText["x"].Key + cKey := servicesByText["c"].Key + + // Verify lexicographic ordering + require.True(t, aKey < xKey, "a should come before x") + require.True(t, xKey < cKey, "x should come before c") + + // Verify correct index prefixes + assert.True(t, strings.Contains(aKey, "/0-"), "a should have prefix 0-") + assert.True(t, strings.Contains(xKey, "/1-"), "x should have prefix 1-") + assert.True(t, strings.Contains(cKey, "/2-"), "c should have prefix 2-") +} + +// TestCoreDNSProviderTXTRecordOrderingEdgeCases tests complex reordering scenarios +func TestCoreDNSProviderTXTRecordOrderingEdgeCases(t *testing.T) { + provider := coreDNSProvider{ + coreDNSPrefix: defaultCoreDNSPrefix, + txtOwnerID: "default", + } + + // Step 1: Start with 5 targets + targets := []string{"alpha", "beta", "gamma", "delta", "epsilon"} + testEndpoint := &endpoint.Endpoint{ + DNSName: "test.example.com", + RecordType: endpoint.RecordTypeTXT, + Targets: targets, + RecordTTL: 300, + Labels: map[string]string{}, + } + + services := provider.updateTXTRecords("test.example.com", []*endpoint.Endpoint{testEndpoint}, []*Service{}) + require.Len(t, services, 5, "should create 5 initial services") + + // Verify initial ordering + servicesByText := make(map[string]*Service) + for _, svc := range services { + servicesByText[svc.Text] = svc + } + + assert.True(t, strings.Contains(servicesByText["alpha"].Key, "/0-"), "alpha should have prefix 0-") + assert.True(t, strings.Contains(servicesByText["beta"].Key, "/1-"), "beta should have prefix 1-") + assert.True(t, strings.Contains(servicesByText["gamma"].Key, "/2-"), "gamma should have prefix 2-") + assert.True(t, strings.Contains(servicesByText["delta"].Key, "/3-"), "delta should have prefix 3-") + assert.True(t, strings.Contains(servicesByText["epsilon"].Key, "/4-"), "epsilon should have prefix 4-") + + // Step 2: Replace the 3rd target (gamma → charlie) + t.Logf("Step 2: Replace 3rd target (gamma → charlie)") + targets = []string{"alpha", "beta", "charlie", "delta", "epsilon"} + testEndpoint.Targets = targets + + needsReorder := provider.checkIfReorderNeeded(targets, testEndpoint.Labels) + assert.True(t, needsReorder, "should need reorder when replacing target") + + services = provider.updateTXTRecords("test.example.com", []*endpoint.Endpoint{testEndpoint}, []*Service{}) + require.Len(t, services, 5, "should have 5 services after replacement") + + servicesByText = make(map[string]*Service) + for _, svc := range services { + servicesByText[svc.Text] = svc + } + + assert.True(t, strings.Contains(servicesByText["alpha"].Key, "/0-"), "alpha should stay at prefix 0-") + assert.True(t, strings.Contains(servicesByText["beta"].Key, "/1-"), "beta should stay at prefix 1-") + assert.True(t, strings.Contains(servicesByText["charlie"].Key, "/2-"), "charlie should have prefix 2-") + assert.True(t, strings.Contains(servicesByText["delta"].Key, "/3-"), "delta should stay at prefix 3-") + assert.True(t, strings.Contains(servicesByText["epsilon"].Key, "/4-"), "epsilon should stay at prefix 4-") + assert.Nil(t, servicesByText["gamma"], "gamma should no longer exist") + + // Step 3: Remove the 4th target (delta) + t.Logf("Step 3: Remove 4th target (delta)") + targets = []string{"alpha", "beta", "charlie", "epsilon"} + testEndpoint.Targets = targets + + needsReorder = provider.checkIfReorderNeeded(targets, testEndpoint.Labels) + assert.True(t, needsReorder, "should need reorder when removing middle target") + + services = provider.updateTXTRecords("test.example.com", []*endpoint.Endpoint{testEndpoint}, []*Service{}) + require.Len(t, services, 4, "should have 4 services after removal") + + servicesByText = make(map[string]*Service) + for _, svc := range services { + servicesByText[svc.Text] = svc + } + + assert.True(t, strings.Contains(servicesByText["alpha"].Key, "/0-"), "alpha should stay at prefix 0-") + assert.True(t, strings.Contains(servicesByText["beta"].Key, "/1-"), "beta should stay at prefix 1-") + assert.True(t, strings.Contains(servicesByText["charlie"].Key, "/2-"), "charlie should stay at prefix 2-") + assert.True(t, strings.Contains(servicesByText["epsilon"].Key, "/3-"), "epsilon should move to prefix 3-") + assert.Nil(t, servicesByText["delta"], "delta should no longer exist") + + // Step 4: Swap 1st and 2nd targets (alpha ↔ beta) + t.Logf("Step 4: Swap 1st and 2nd targets (alpha ↔ beta)") + targets = []string{"beta", "alpha", "charlie", "epsilon"} + testEndpoint.Targets = targets + + needsReorder = provider.checkIfReorderNeeded(targets, testEndpoint.Labels) + assert.True(t, needsReorder, "should need reorder when swapping targets") + + services = provider.updateTXTRecords("test.example.com", []*endpoint.Endpoint{testEndpoint}, []*Service{}) + require.Len(t, services, 4, "should have 4 services after swap") + + servicesByText = make(map[string]*Service) + for _, svc := range services { + servicesByText[svc.Text] = svc + } + + assert.True(t, strings.Contains(servicesByText["beta"].Key, "/0-"), "beta should move to prefix 0-") + assert.True(t, strings.Contains(servicesByText["alpha"].Key, "/1-"), "alpha should move to prefix 1-") + assert.True(t, strings.Contains(servicesByText["charlie"].Key, "/2-"), "charlie should stay at prefix 2-") + assert.True(t, strings.Contains(servicesByText["epsilon"].Key, "/3-"), "epsilon should stay at prefix 3-") + + // Step 5: Remove all targets + t.Logf("Step 5: Remove all targets") + targets = []string{} + testEndpoint.Targets = targets + + services = provider.updateTXTRecords("test.example.com", []*endpoint.Endpoint{testEndpoint}, []*Service{}) + + // Should return empty services for an endpoint with no TXT targets + txtServiceCount := 0 + for _, svc := range services { + if svc.Text != "" { + txtServiceCount++ + } + } + assert.Equal(t, 0, txtServiceCount, "should have no TXT services after removing all targets") } diff --git a/registry/txt.go b/registry/txt.go index 3259969365..51f3243dbf 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -271,6 +271,9 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) } for _, r := range filteredChanges.Delete { + log.Tracef("TXT Registry processing deletion for endpoint: DNSName=%s, RecordType=%s, Targets=%v, Labels=%v", + r.DNSName, r.RecordType, r.Targets, r.Labels) + // 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 // !!! After migration to the new TXT registry format we can drop records in old format here!!! diff --git a/source/crd.go b/source/crd.go index 2911f1f6d7..a76731dba2 100644 --- a/source/crd.go +++ b/source/crd.go @@ -189,8 +189,10 @@ func (cs *crdSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error illegalTarget := false for _, target := range ep.Targets { isNAPTR := ep.RecordType == endpoint.RecordTypeNAPTR + isTXT := ep.RecordType == endpoint.RecordTypeTXT hasDot := strings.HasSuffix(target, ".") - if (isNAPTR && !hasDot) || (!isNAPTR && hasDot) { + // Skip dot validation for TXT records as they can contain arbitrary text + if !isTXT && ((isNAPTR && !hasDot) || (!isNAPTR && hasDot)) { illegalTarget = true break } diff --git a/source/crd_test.go b/source/crd_test.go index 0f92d0db38..8d77f0f16d 100644 --- a/source/crd_test.go +++ b/source/crd_test.go @@ -475,6 +475,48 @@ func testCRDSourceEndpoints(t *testing.T) { expectEndpoints: false, expectError: false, }, + { + title: "valid target TXT", + registeredAPIVersion: "test.k8s.io/v1alpha1", + apiVersion: "test.k8s.io/v1alpha1", + registeredKind: "DNSEndpoint", + kind: "DNSEndpoint", + namespace: "foo", + registeredNamespace: "foo", + labels: map[string]string{"test": "that"}, + labelFilter: "test=that", + endpoints: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Targets: endpoint.Targets{"foo.example.org."}, + RecordType: endpoint.RecordTypeTXT, + RecordTTL: 180, + }, + }, + expectEndpoints: true, + expectError: false, + }, + { + title: "illegal target A", + registeredAPIVersion: "test.k8s.io/v1alpha1", + apiVersion: "test.k8s.io/v1alpha1", + registeredKind: "DNSEndpoint", + kind: "DNSEndpoint", + namespace: "foo", + registeredNamespace: "foo", + labels: map[string]string{"test": "that"}, + labelFilter: "test=that", + endpoints: []*endpoint.Endpoint{ + { + DNSName: "example.org", + Targets: endpoint.Targets{"1.2.3.4."}, + RecordType: endpoint.RecordTypeA, + RecordTTL: 180, + }, + }, + expectEndpoints: false, + expectError: false, + }, } { t.Run(ti.title, func(t *testing.T) { t.Parallel()