diff --git a/internal/idna/idna.go b/internal/idna/idna.go index 9290e8a51e..784d2039ec 100644 --- a/internal/idna/idna.go +++ b/internal/idna/idna.go @@ -17,6 +17,9 @@ limitations under the License. package idna import ( + "strings" + + log "github.com/sirupsen/logrus" "golang.org/x/net/idna" ) @@ -27,3 +30,16 @@ var ( idna.StrictDomainName(false), ) ) + +// normalizeDNSName converts a DNS name to a canonical form, so that we can use string equality +// it: removes space, get ASCII version of dnsName complient with Section 5 of RFC 5891, ensures there is a trailing dot +func NormalizeDNSName(dnsName string) string { + s, err := Profile.ToASCII(strings.TrimSpace(dnsName)) + if err != nil { + log.Warnf(`Got error while parsing DNSName %s: %v`, dnsName, err) + } + if !strings.HasSuffix(s, ".") { + s += "." + } + return s +} diff --git a/internal/idna/idna_test.go b/internal/idna/idna_test.go index f3ae93c446..5db2f498c8 100644 --- a/internal/idna/idna_test.go +++ b/internal/idna/idna_test.go @@ -57,3 +57,101 @@ func TestProfileWithDefault(t *testing.T) { }) } } + +func TestNormalizeDNSName(tt *testing.T) { + records := []struct { + dnsName string + expect string + }{ + { + "3AAAA.FOO.BAR.COM ", + "3aaaa.foo.bar.com.", + }, + { + " example.foo.com.", + "example.foo.com.", + }, + { + "example123.foo.com ", + "example123.foo.com.", + }, + { + "foo", + "foo.", + }, + { + "123foo.bar", + "123foo.bar.", + }, + { + "foo.com", + "foo.com.", + }, + { + "foo.com.", + "foo.com.", + }, + { + "_foo.com.", + "_foo.com.", + }, + { + "\u005Ffoo.com.", + "_foo.com.", + }, + { + ".foo.com.", + ".foo.com.", + }, + { + "foo123.COM", + "foo123.com.", + }, + { + "my-exaMple3.FOO.BAR.COM", + "my-example3.foo.bar.com.", + }, + { + " my-example1214.FOO-1235.BAR-foo.COM ", + "my-example1214.foo-1235.bar-foo.com.", + }, + { + "my-example-my-example-1214.FOO-1235.BAR-foo.COM", + "my-example-my-example-1214.foo-1235.bar-foo.com.", + }, + { + "點看.org.", + "xn--c1yn36f.org.", + }, + { + "nordic-ø.xn--kitty-點看pd34d.com", + "xn--nordic--w1a.xn--xn--kitty-pd34d-hn01b3542b.com.", + }, + { + "nordic-ø.kitty😸.com.", + "xn--nordic--w1a.xn--kitty-pd34d.com.", + }, + { + " nordic-ø.kitty😸.COM", + "xn--nordic--w1a.xn--kitty-pd34d.com.", + }, + { + "xn--nordic--w1a.kitty😸.com.", + "xn--nordic--w1a.xn--kitty-pd34d.com.", + }, + { + "*.example.com.", + "*.example.com.", + }, + { + "*.example.com", + "*.example.com.", + }, + } + for _, r := range records { + tt.Run(r.dnsName, func(t *testing.T) { + gotName := NormalizeDNSName(r.dnsName) + assert.Equal(t, r.expect, gotName) + }) + } +} diff --git a/plan/plan.go b/plan/plan.go index 325a3d80fa..52f0432ed2 100644 --- a/plan/plan.go +++ b/plan/plan.go @@ -19,7 +19,6 @@ package plan import ( "fmt" "slices" - "strings" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -142,7 +141,7 @@ func (t *planTable) addCandidate(e *endpoint.Endpoint) { func (t *planTable) newPlanKey(e *endpoint.Endpoint) planKey { key := planKey{ - dnsName: normalizeDNSName(e.DNSName), + dnsName: idna.NormalizeDNSName(e.DNSName), setIdentifier: e.SetIdentifier, } @@ -348,19 +347,6 @@ func filterRecordsForPlan(records []*endpoint.Endpoint, domainFilter endpoint.Ma return filtered } -// normalizeDNSName converts a DNS name to a canonical form, so that we can use string equality -// it: removes space, get ASCII version of dnsName complient with Section 5 of RFC 5891, ensures there is a trailing dot -func normalizeDNSName(dnsName string) string { - s, err := idna.Profile.ToASCII(strings.TrimSpace(dnsName)) - if err != nil { - log.Warnf(`Got error while parsing DNSName %s: %v`, dnsName, err) - } - if !strings.HasSuffix(s, ".") { - s += "." - } - return s -} - func IsManagedRecord(record string, managedRecords, excludeRecords []string) bool { if slices.Contains(excludeRecords, record) { return false diff --git a/plan/plan_test.go b/plan/plan_test.go index acce96b27d..a127f694a0 100644 --- a/plan/plan_test.go +++ b/plan/plan_test.go @@ -1054,104 +1054,6 @@ func validateEntries(t *testing.T, entries, expected []*endpoint.Endpoint) { } } -func TestNormalizeDNSName(tt *testing.T) { - records := []struct { - dnsName string - expect string - }{ - { - "3AAAA.FOO.BAR.COM ", - "3aaaa.foo.bar.com.", - }, - { - " example.foo.com.", - "example.foo.com.", - }, - { - "example123.foo.com ", - "example123.foo.com.", - }, - { - "foo", - "foo.", - }, - { - "123foo.bar", - "123foo.bar.", - }, - { - "foo.com", - "foo.com.", - }, - { - "foo.com.", - "foo.com.", - }, - { - "_foo.com.", - "_foo.com.", - }, - { - "\u005Ffoo.com.", - "_foo.com.", - }, - { - ".foo.com.", - ".foo.com.", - }, - { - "foo123.COM", - "foo123.com.", - }, - { - "my-exaMple3.FOO.BAR.COM", - "my-example3.foo.bar.com.", - }, - { - " my-example1214.FOO-1235.BAR-foo.COM ", - "my-example1214.foo-1235.bar-foo.com.", - }, - { - "my-example-my-example-1214.FOO-1235.BAR-foo.COM", - "my-example-my-example-1214.foo-1235.bar-foo.com.", - }, - { - "點看.org.", - "xn--c1yn36f.org.", - }, - { - "nordic-ø.xn--kitty-點看pd34d.com", - "xn--nordic--w1a.xn--xn--kitty-pd34d-hn01b3542b.com.", - }, - { - "nordic-ø.kitty😸.com.", - "xn--nordic--w1a.xn--kitty-pd34d.com.", - }, - { - " nordic-ø.kitty😸.COM", - "xn--nordic--w1a.xn--kitty-pd34d.com.", - }, - { - "xn--nordic--w1a.kitty😸.com.", - "xn--nordic--w1a.xn--kitty-pd34d.com.", - }, - { - "*.example.com.", - "*.example.com.", - }, - { - "*.example.com", - "*.example.com.", - }, - } - for _, r := range records { - tt.Run(r.dnsName, func(t *testing.T) { - gotName := normalizeDNSName(r.dnsName) - assert.Equal(t, r.expect, gotName) - }) - } -} - func TestShouldUpdateProviderSpecific(tt *testing.T) { for _, test := range []struct { name string diff --git a/registry/txt.go b/registry/txt.go index d814192183..2b533e0f99 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -26,8 +26,10 @@ import ( b64 "encoding/base64" log "github.com/sirupsen/logrus" + "k8s.io/utils/set" "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/internal/idna" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" ) @@ -66,6 +68,9 @@ type TXTRegistry struct { // existingTXTs is the TXT records that already exist in the zone so that // ApplyChanges() can skip re-creating them. See the struct below for details. existingTXTs *existingTXTs + + // rootApexDetector detects which domains are root zone apex domains + rootApexDetector *rootApexDetector } // existingTXTs stores pre‑existing TXT records to avoid duplicate creation. @@ -111,6 +116,95 @@ func (im *existingTXTs) reset() { im.entries = make(map[recordKey]struct{}) } +// rootApexDetector detects and tracks root zone apex domains by analyzing NS records. +// It maintains a set containing only the topmost apex domains within the observed NS records, +// not the actual DNS root domain ("."). Each "root apex" is the highest zone in its hierarchy +// among the zones that external-dns can observe. +// +// For example, if NS records exist for both "example.com" and "sub.example.com", +// only "example.com" will be tracked as a root apex. However, if only "deep.sub.example.com" +// has an NS record, then "deep.sub.example.com" becomes the root apex for that hierarchy. +type rootApexDetector struct { + apexes set.Set[string] +} + +func newRootApexDetector() *rootApexDetector { + return &rootApexDetector{ + apexes: set.New[string](), + } +} + +// addCandidate processes an NS record as a potential root apex candidate. +// It maintains the invariant that only root apex domains (no parent-child relationships) are stored. +func (d *rootApexDetector) addCandidate(ep *endpoint.Endpoint) { + if ep.RecordType != endpoint.RecordTypeNS { + return + } + + domain := idna.NormalizeDNSName(ep.DNSName) + + // Check if a parent apex already exists + if d.hasParentApex(domain) { + return // Don't add if parent apex exists + } + + // Remove any child apexes that would be superseded + d.removeChildApexes(domain) + + // Add this domain as a root apex + d.apexes.Insert(domain) +} + +// hasParentApex checks if any parent domain of the given domain exists in the apex set. +// For example, if "example.com." is in the set, "child.example.com." has a parent apex. +func (d *rootApexDetector) hasParentApex(domain string) bool { + // Remove trailing dot before split to avoid empty element at the end + // e.g., "child.example.com." -> ["child", "example", "com"] not ["child", "example", "com", ""] + parts := strings.Split(strings.TrimSuffix(domain, "."), ".") + // Start from i=1 to skip the domain itself + for i := 1; i < len(parts); i++ { + parent := strings.Join(parts[i:], ".") + "." + if d.apexes.Has(parent) { + return true + } + } + return false +} + +// removeChildApexes removes all child domains from the apex set. +// For example, if adding "example.com.", this would remove both "api.example.com." and "www.example.com." if they exist. +func (d *rootApexDetector) removeChildApexes(parentDomain string) { + suffix := "." + parentDomain + toRemove := make([]string, 0) + + // Collect all child domains to remove + for domain := range d.apexes { + if strings.HasSuffix(domain, suffix) { + toRemove = append(toRemove, domain) + } + } + + // Remove all collected child domains + for _, domain := range toRemove { + d.apexes.Delete(domain) + } +} + +// isObservedRootApex determines whether the endpoint represents an observed root apex domain. +// A root apex is a zone apex that has no parent zone among the observed NS records. +// Note: This is not the DNS root domain (".") but rather the topmost zone in the observed hierarchy. +func (d *rootApexDetector) isObservedRootApex(ep *endpoint.Endpoint) bool { + return d.apexes.Has(idna.NormalizeDNSName(ep.DNSName)) +} + +// reset clears the stored apex domains for the next reconciliation cycle. +func (d *rootApexDetector) reset() { + if d == nil { + return + } + d.apexes = d.apexes.Clear() +} + // NewTXTRegistry returns a new TXTRegistry object. When newFormatOnly is true, it will only // generate new format TXT records, otherwise it generates both old and new formats for // backwards compatibility. @@ -142,6 +236,8 @@ func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID st mapper := newaffixNameMapper(txtPrefix, txtSuffix, txtWildcardReplacement) + rootApexDetector := newRootApexDetector() + return &TXTRegistry{ provider: provider, ownerID: ownerID, @@ -154,6 +250,7 @@ func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID st txtEncryptAESKey: txtEncryptAESKey, oldOwnerID: oldOwnerID, existingTXTs: newExistingTXTs(), + rootApexDetector: rootApexDetector, }, nil } @@ -191,6 +288,7 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error txtRecordsMap := map[string]struct{}{} for _, record := range records { + im.rootApexDetector.addCandidate(record) if record.RecordType != endpoint.RecordTypeTXT { endpoints = append(endpoints, record) continue @@ -318,31 +416,51 @@ func (im *TXTRegistry) generateTXTRecordWithFilter(r *endpoint.Endpoint, filter return endpoints } +// shouldBlockApex determines whether an apex record should be blocked from creation. +// Returns true when the TXT format is unsafe AND the endpoint represents an apex domain. +func (im *TXTRegistry) shouldBlockApex(ep *endpoint.Endpoint) bool { + // If mapper supports apex records, always allow + if im.mapper.supportsApex() { + return false + } + + // Only block if it's actually a root apex domain + return im.rootApexDetector.isObservedRootApex(ep) +} + // 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 { - defer im.existingTXTs.reset() // reset existing TXTs for the next reconciliation loop + defer im.existingTXTs.reset() // reset existing TXTs for the next reconciliation loop + defer im.rootApexDetector.reset() // reset root apex detector for the next reconciliation loop - filteredChanges := &plan.Changes{ - Create: changes.Create, - UpdateNew: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateNew), - UpdateOld: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateOld), - Delete: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.Delete), - } + filteredCreates := make([]*endpoint.Endpoint, 0, len(changes.Create)) - for _, r := range filteredChanges.Create { + for _, r := range changes.Create { if r.Labels == nil { r.Labels = make(map[string]string) } + if im.shouldBlockApex(r) { + log.Warnf(`Cannot create %s record for %s (apex): TXT ownership record would be outside managed zone. Use txtPrefix="%%{record_type}."`, + r.RecordType, r.DNSName) + continue + } r.Labels[endpoint.OwnerLabelKey] = im.ownerID - - filteredChanges.Create = append(filteredChanges.Create, im.generateTXTRecordWithFilter(r, im.existingTXTs.isAbsent)...) + filteredCreates = append(filteredCreates, r) + filteredCreates = append(filteredCreates, im.generateTXTRecordWithFilter(r, im.existingTXTs.isAbsent)...) if im.cacheInterval > 0 { im.addToCache(r) } } + filteredChanges := &plan.Changes{ + Create: filteredCreates, + UpdateNew: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateNew), + UpdateOld: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateOld), + Delete: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.Delete), + } + for _, r := range filteredChanges.Delete { // 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 @@ -395,6 +513,7 @@ type nameMapper interface { toEndpointName(string) (endpointName string, recordType string) toTXTName(string, string) string recordTypeInAffix() bool + supportsApex() bool } type affixNameMapper struct { @@ -503,6 +622,15 @@ func (pr affixNameMapper) recordTypeInAffix() bool { return false } +func (pr affixNameMapper) supportsApex() bool { + // Apex records are supported only when: + // 1. prefix contains the record type template (%{record_type}) + // 2. prefix ends with a dot (.) + // This ensures TXT records are created as subdomains within the same zone + // Example: prefix="%{record_type}." creates "a.example.com" TXT for "example.com" A record + return strings.Contains(pr.prefix, recordTemplate) && strings.HasSuffix(pr.prefix, ".") +} + func (pr affixNameMapper) normalizeAffixTemplate(afix, recordType string) string { if strings.Contains(afix, recordTemplate) { return strings.ReplaceAll(afix, recordTemplate, recordType) diff --git a/registry/txt_test.go b/registry/txt_test.go index b5146aa49b..aa7533a544 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -586,6 +586,198 @@ func testTXTRegistryApplyChanges(t *testing.T) { t.Run("With Templated Suffix", testTXTRegistryApplyChangesWithTemplatedSuffix) t.Run("With Suffix", testTXTRegistryApplyChangesWithSuffix) t.Run("No prefix", testTXTRegistryApplyChangesNoPrefix) + t.Run("With Apex", testTXTRegistryApplyChangesWithApex) +} + +func testTXTRegistryApplyChangesWithApex(t *testing.T) { + for _, apexRecordType := range []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME} { + for _, tt := range []struct { + name string + prefix string + suffix string + creatableApexRecord bool + }{ + { + name: "With templated prefix with dot should create apex record", + prefix: "txt-%{record_type}.", + suffix: "", + creatableApexRecord: true, + }, + { + name: "With templated prefix without dot should not create apex record", + prefix: "txt-%{record_type}", + suffix: "", + creatableApexRecord: false, + }, + { + name: "With templated suffix with dot should not create apex record", + prefix: "", + suffix: ".txt-%{record_type}", + creatableApexRecord: false, + }, + { + name: "With templated suffix without dot should not create apex record", + prefix: "", + suffix: "txt-%{record_type}", + creatableApexRecord: false, + }, + { + name: "With prefix with dot should not create apex record", + prefix: "txt.", + suffix: "", + creatableApexRecord: false, + }, + { + name: "With prefix without dot should not create apex record", + prefix: "txt", + suffix: "", + }, + { + name: "With suffix with dot should not create apex record", + prefix: "", + suffix: ".txt", + creatableApexRecord: false, + }, + { + name: "With suffix without dot should not create apex record", + prefix: "", + suffix: "txt", + creatableApexRecord: false, + }, + { + name: "No prefix should not create apex record", + prefix: "", + suffix: "", + creatableApexRecord: false, + }, + } { + t.Run(apexRecordType+"-"+tt.name, func(t *testing.T) { + p := inmemory.NewInMemoryProvider() + p.CreateZone("test-zone.example.org") + p.CreateZone("test-zone2.example.org") + + ctxEndpoints := []*endpoint.Endpoint{} + ctx := context.WithValue(context.Background(), provider.RecordsContextKey, ctxEndpoints) + p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { + assert.Equal(t, ctxEndpoints, ctx.Value(provider.RecordsContextKey)) + } + r, _ := NewTXTRegistry(p, tt.prefix, tt.suffix, "owner", time.Hour, "", []string{}, []string{}, false, nil, "") + p.ApplyChanges(ctx, &plan.Changes{ + Create: []*endpoint.Endpoint{ + // NS record for the apex of the zone + newEndpointWithOwner("test-zone.example.org", "ns1.example.org", endpoint.RecordTypeNS, ""), + newEndpointWithOwner("test-zone2.example.org", "ns1.example.org", endpoint.RecordTypeNS, ""), + // NS record for the apex of the child zone + newEndpointWithOwner("child.test-zone.example.org", "ns1.child.example.org", endpoint.RecordTypeNS, ""), + + // other existing records + newEndpointWithOwner("foo.test-zone.example.org", "foo.loadbalancer.com", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("bar.test-zone.example.org", "my-domain.com", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("txt.bar.test-zone.example.org", "baz.test-zone.example.org", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "").WithSetIdentifier("test-set-1"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "").WithSetIdentifier("test-set-2"), + newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwner("qux.test-zone.example.org", "random", endpoint.RecordTypeTXT, ""), + + // existing records owned by external-dns(legacy) + newEndpointWithOwner("txt.bar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("txt.tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("txt.multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), + newEndpointWithOwner("txt.multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-1"), + newEndpointWithOwner("txt.foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + + // existing records owned by external-dns + newEndpointWithOwner(r.mapper.toTXTName("tar.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner(r.mapper.toTXTName("foobar.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner(r.mapper.toTXTName("multiple.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-1"), + newEndpointWithOwner(r.mapper.toTXTName("multiple.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), + + // Existing apex record created before apex blocking was implemented. + // Before apex blocking, TXT record creation could fail while the main record was created successfully, + // resulting in records without TXT ownership records. This tests backward compatibility - + // existing apex records should remain unaffected by the new blocking functionality. + newEndpointWithOwner("test-zone2.example.org", "1.1.1.1", endpoint.RecordTypeA, ""), + }, + }) + changes := &plan.Changes{ + Create: []*endpoint.Endpoint{ + // apex record + newEndpointWithOwnerResource("test-zone.example.org", "apex.loadbalancer.com", apexRecordType, "", "ingress/default/my-ingress"), + // child apex record + newEndpointWithOwnerResource("child.test-zone.example.org", "child-apex.loadbalancer.com", apexRecordType, "", "ingress/default/my-ingress"), + + newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "", "ingress/default/my-ingress"), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "lb3.loadbalancer.com", endpoint.RecordTypeCNAME, "", "ingress/default/my-ingress").WithSetIdentifier("test-set-3"), + }, + Delete: []*endpoint.Endpoint{ + newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-1"), + }, + UpdateNew: []*endpoint.Endpoint{ + newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), + }, + UpdateOld: []*endpoint.Endpoint{ + newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), + }, + } + expected := &plan.Changes{ + Create: []*endpoint.Endpoint{ + newEndpointWithOwnerResource("child.test-zone.example.org", "child-apex.loadbalancer.com", apexRecordType, "owner", "ingress/default/my-ingress"), + newEndpointWithOwnerAndOwnedRecord(r.mapper.toTXTName("child.test-zone.example.org", apexRecordType), "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "child.test-zone.example.org"), + newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress"), + + newEndpointWithOwnerAndOwnedRecord(r.mapper.toTXTName("new-record-1.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "lb3.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress").WithSetIdentifier("test-set-3"), + newEndpointWithOwnerAndOwnedRecord(r.mapper.toTXTName("multiple.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-3"), + }, + Delete: []*endpoint.Endpoint{ + newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + newEndpointWithOwnerAndOwnedRecord(r.mapper.toTXTName("foobar.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "foobar.test-zone.example.org"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-1"), + newEndpointWithOwnerAndOwnedRecord(r.mapper.toTXTName("multiple.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-1"), + }, + UpdateNew: []*endpoint.Endpoint{ + newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), + newEndpointWithOwnerAndOwnedRecord(r.mapper.toTXTName("tar.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), + newEndpointWithOwnerAndOwnedRecord(r.mapper.toTXTName("multiple.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-2"), + }, + UpdateOld: []*endpoint.Endpoint{ + newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + newEndpointWithOwnerAndOwnedRecord(r.mapper.toTXTName("tar.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), + newEndpointWithOwnerAndOwnedRecord(r.mapper.toTXTName("multiple.test-zone.example.org", endpoint.RecordTypeCNAME), "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-2"), + }, + } + if tt.creatableApexRecord { + expected.Create = append(expected.Create, newEndpointWithOwnerResource("test-zone.example.org", "apex.loadbalancer.com", apexRecordType, "owner", "ingress/default/my-ingress")) + expected.Create = append(expected.Create, newEndpointWithOwnerAndOwnedRecord(r.mapper.toTXTName("test-zone.example.org", apexRecordType), "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "test-zone.example.org")) + } + p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { + mExpected := map[string][]*endpoint.Endpoint{ + "Create": expected.Create, + "UpdateNew": expected.UpdateNew, + "UpdateOld": expected.UpdateOld, + "Delete": expected.Delete, + } + mGot := map[string][]*endpoint.Endpoint{ + "Create": got.Create, + "UpdateNew": got.UpdateNew, + "UpdateOld": got.UpdateOld, + "Delete": got.Delete, + } + assert.True(t, testutils.SamePlanChanges(mGot, mExpected)) + assert.Nil(t, ctx.Value(provider.RecordsContextKey)) + } + _, err := r.Records(ctx) + err = r.ApplyChanges(ctx, changes) + require.NoError(t, err) + }) + } + } } func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { @@ -2064,6 +2256,322 @@ func TestTXTRegistryRecreatesMissingRecords(t *testing.T) { } } +func TestRootApexDetector(t *testing.T) { + tests := []struct { + name string + txtPrefix string + existingRecords []*endpoint.Endpoint + recordsToCreate []*endpoint.Endpoint + expectedAllowed []string + }{ + { + name: "A record at root apex with no prefix should be blocked", + txtPrefix: "", + existingRecords: []*endpoint.Endpoint{ + { + DNSName: "test-zone.example.org", + RecordType: endpoint.RecordTypeNS, + }, + }, + recordsToCreate: []*endpoint.Endpoint{ + { + DNSName: "test-zone.example.org", + RecordType: endpoint.RecordTypeA, + }, + }, + expectedAllowed: nil, + }, + { + name: "A record at root apex and normal domain with no prefix should block only root apex", + txtPrefix: "", + existingRecords: []*endpoint.Endpoint{ + { + DNSName: "test-zone.example.org", + RecordType: endpoint.RecordTypeNS, + }, + }, + recordsToCreate: []*endpoint.Endpoint{ + { + DNSName: "test-zone.example.org", + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "api.test-zone.example.org", + RecordType: endpoint.RecordTypeA, + }, + }, + expectedAllowed: []string{ + "api.test-zone.example.org", + }, + }, + { + name: "A record at root apex with templated dot prefix should be allowed", + txtPrefix: "%{record_type}.", + existingRecords: []*endpoint.Endpoint{ + { + DNSName: "test-zone.example.org", + RecordType: endpoint.RecordTypeNS, + }, + { + DNSName: "api.example.org", + RecordType: endpoint.RecordTypeNS, + }, + }, + recordsToCreate: []*endpoint.Endpoint{ + { + DNSName: "test-zone.example.org", + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "api.example.org", + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "hoge.api.example.org", + RecordType: endpoint.RecordTypeA, + }, + }, + expectedAllowed: []string{ + "test-zone.example.org", + "api.example.org", + "hoge.api.example.org", + }, + }, + { + name: "Parent and child zones should block only parent zone apex", + txtPrefix: "", + existingRecords: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeNS, + }, + { + DNSName: "child.example.org", + RecordType: endpoint.RecordTypeNS, + }, + }, + recordsToCreate: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "child.example.org", + RecordType: endpoint.RecordTypeA, + }, + }, + expectedAllowed: []string{ + "child.example.org", + }, + }, + { + name: "Siblings should be removed when parent apex is observed", + txtPrefix: "", + existingRecords: []*endpoint.Endpoint{ + { + DNSName: "a.example.org", + RecordType: endpoint.RecordTypeNS, + }, + { + DNSName: "b.example.org", + RecordType: endpoint.RecordTypeNS, + }, + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeNS, + }, + }, + recordsToCreate: []*endpoint.Endpoint{ + { + DNSName: "a.example.org", + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "b.example.org", + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeA, + }, + }, + expectedAllowed: []string{ + "a.example.org", + "b.example.org", + }, + }, + { + name: "Multiple nested zones should block only top-level apex", + txtPrefix: "", + existingRecords: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeNS, + }, + { + DNSName: "sub.example.org", + RecordType: endpoint.RecordTypeNS, + }, + { + DNSName: "deep.sub.example.org", + RecordType: endpoint.RecordTypeNS, + }, + }, + recordsToCreate: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "sub.example.org", + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "deep.sub.example.org", + RecordType: endpoint.RecordTypeA, + }, + }, + expectedAllowed: []string{ + "sub.example.org", + "deep.sub.example.org", + }, + }, + { + name: "Different record types at root apex should all be blocked with non-templated prefix", + txtPrefix: "txt-", + existingRecords: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeNS, + }, + }, + recordsToCreate: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeAAAA, + }, + }, + expectedAllowed: nil, + }, + { + name: "Different record types at root apex should all be allowed with templated dot prefix", + txtPrefix: "%{record_type}.", + existingRecords: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeNS, + }, + }, + recordsToCreate: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeAAAA, + }, + }, + expectedAllowed: []string{ + "example.org", + "example.org", + }, + }, + { + name: "Non-NS records in existing records should be ignored", + txtPrefix: "", + existingRecords: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeNS, + }, + { + DNSName: "fake.example.org", + RecordType: endpoint.RecordTypeA, // Not NS, should be ignored + }, + { + DNSName: "another.example.org", + RecordType: endpoint.RecordTypeCNAME, // Not NS, should be ignored + }, + }, + recordsToCreate: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeA, + }, + { + DNSName: "fake.example.org", + RecordType: endpoint.RecordTypeA, + }, + }, + expectedAllowed: []string{ + "fake.example.org", + }, + }, + { + name: "Dot prefix with no template should block root apex records", + txtPrefix: "txt.", + existingRecords: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeNS, + }, + }, + recordsToCreate: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeA, + }, + }, + expectedAllowed: nil, + }, + { + name: "Template prefix without dot should block root apex records", + txtPrefix: "%{record_type}", + existingRecords: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeNS, + }, + }, + recordsToCreate: []*endpoint.Endpoint{ + { + DNSName: "example.org", + RecordType: endpoint.RecordTypeA, + }, + }, + expectedAllowed: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Create registry with the test prefix + p := inmemory.NewInMemoryProvider() + registry, _ := NewTXTRegistry(p, tt.txtPrefix, "", "owner", time.Hour, "", []string{}, []string{}, false, nil, "foo") + + // Add existing records to root apex detector + for _, record := range tt.existingRecords { + registry.rootApexDetector.addCandidate(record) + } + + // Test which records would be allowed + var created []string + for _, record := range tt.recordsToCreate { + if !registry.shouldBlockApex(record) { + created = append(created, record.DNSName) + } + } + assert.Equal(t, tt.expectedAllowed, created) + }) + } +} + func TestTXTRecordMigration(t *testing.T) { ctx := context.Background() p := inmemory.NewInMemoryProvider()