From c9b1efb95fe12940d958db061dbe7dfe338b0173 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Mon, 11 May 2026 08:18:40 -0400 Subject: [PATCH] fix: Prevent creating orphaned records --- registry/txt/registry.go | 19 ++++++- registry/txt/registry_test.go | 101 ++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/registry/txt/registry.go b/registry/txt/registry.go index a52ec2940e..ef25cc6520 100644 --- a/registry/txt/registry.go +++ b/registry/txt/registry.go @@ -339,13 +339,28 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) Delete: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.Delete), } - for _, r := range filteredChanges.Create { + pendingCreate := filteredChanges.Create + filteredChanges.Create = make([]*endpoint.Endpoint, 0, len(pendingCreate)) + for _, r := range pendingCreate { if r.Labels == nil { r.Labels = make(map[string]string) } r.Labels[endpoint.OwnerLabelKey] = im.ownerID - filteredChanges.Create = append(filteredChanges.Create, im.generateTXTRecordWithFilter(r, im.existingTXTs.isAbsent)...) + // Skip records whose ownership TXT cannot be established; creating + // them would leak an unreclaimable record into the zone. + txts := im.generateTXTRecord(r) + if len(txts) == 0 { + log.Warnf("Skipping create of %s %s: cannot establish ownership TXT (label exceeds RFC 1035 63-char limit)", r.RecordType, r.DNSName) + continue + } + + filteredChanges.Create = append(filteredChanges.Create, r) + for _, txt := range txts { + if im.existingTXTs.isAbsent(txt) { + filteredChanges.Create = append(filteredChanges.Create, txt) + } + } if im.cacheInterval > 0 { im.addToCache(r) diff --git a/registry/txt/registry_test.go b/registry/txt/registry_test.go index 9a81f9ade8..baa7219542 100644 --- a/registry/txt/registry_test.go +++ b/registry/txt/registry_test.go @@ -1138,6 +1138,107 @@ func testTXTRegistryApplyChangesNoPrefix(t *testing.T) { require.NoError(t, err) } +// TestTXTRegistryApplyChangesSkipsRecordWithoutOwnership verifies that the +// registry refuses to create a parent record whenever its ownership TXT cannot +// be produced. Such a parent would be unreclaimable by future reconciles and +// would leak in the zone. +// +// The subtests below exercise the only failure mode that exists today — the +// projected TXT name exceeding the RFC 1035 63-char label limit — across the +// record types and registry configurations whose differing TXT prefixes shift +// the overflow threshold. Any future failure path in TXT generation should +// hit the same guard. +func TestTXTRegistryApplyChangesSkipsRecordWithoutOwnership(t *testing.T) { + tests := []struct { + name string + txtPrefix string + newParent func(dnsName string) *endpoint.Endpoint + labelLen int + txtPrefx string // the prefix the mapper prepends, used to assert the TXT name is not in the change set + }{ + { + name: "CNAME", + newParent: func(dn string) *endpoint.Endpoint { return endpoint.NewEndpoint(dn, endpoint.RecordTypeCNAME, "lb.example.com") }, + labelLen: 60, // cname- (6) + 60 = 66 -> overflow + txtPrefx: "cname-", + }, + { + name: "AAAA", + newParent: func(dn string) *endpoint.Endpoint { return endpoint.NewEndpoint(dn, endpoint.RecordTypeAAAA, "fe80::1") }, + labelLen: 60, // aaaa- (5) + 60 = 65 -> overflow + txtPrefx: "aaaa-", + }, + { + name: "A", + newParent: func(dn string) *endpoint.Endpoint { return endpoint.NewEndpoint(dn, endpoint.RecordTypeA, "1.2.3.4") }, + labelLen: 62, // a- (2) + 62 = 64 -> overflow + txtPrefx: "a-", + }, + { + name: "AliasA", + newParent: func(dn string) *endpoint.Endpoint { + return endpoint.NewEndpoint(dn, endpoint.RecordTypeA, "lb.example.com").WithProviderSpecific(endpoint.ProviderSpecificAlias, "true") + }, + labelLen: 60, // alias-A is encoded as cname- (6) + 60 = 66 -> overflow + txtPrefx: "cname-", + }, + { + name: "WithTxtPrefix", + txtPrefix: "ext-", + newParent: func(dn string) *endpoint.Endpoint { return endpoint.NewEndpoint(dn, endpoint.RecordTypeCNAME, "lb.example.com") }, + labelLen: 56, // ext- (4) + cname- (6) + 56 = 66 -> overflow + txtPrefx: "ext-cname-", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := t.Context() + p := inmemory.NewInMemoryProvider() + require.NoError(t, p.CreateZone(testZone)) + + r, err := newRegistry(p, tc.txtPrefix, "", "owner", time.Hour, "", []string{}, []string{}, false, nil, "") + require.NoError(t, err) + + overflowLabel := strings.Repeat("a", tc.labelLen) + overflow := tc.newParent(overflowLabel + "." + testZone) + require.NotNil(t, overflow, "test setup: parent record at %d-char label must itself pass RFC 1035", tc.labelLen) + fits := tc.newParent("ok." + testZone) + + hook := logtest.LogsUnderTestWithLogLevel(log.WarnLevel, t) + + var got *plan.Changes + p.OnApplyChanges = func(_ context.Context, ch *plan.Changes) { + got = ch + } + require.NoError(t, r.ApplyChanges(ctx, &plan.Changes{ + Create: []*endpoint.Endpoint{overflow, fits}, + })) + + require.NotNil(t, got) + overflowTXT := tc.txtPrefx + overflow.DNSName + for _, ep := range got.Create { + assert.NotEqual(t, overflow.DNSName, ep.DNSName, "parent without ownership TXT must be dropped") + assert.NotEqual(t, overflowTXT, ep.DNSName, "TXT companion for dropped parent must not be created") + } + + fitsParent := false + fitsTXT := false + for _, ep := range got.Create { + if ep.DNSName == fits.DNSName && ep.RecordType == fits.RecordType { + fitsParent = true + } + if ep.DNSName == tc.txtPrefx+fits.DNSName && ep.RecordType == endpoint.RecordTypeTXT { + fitsTXT = true + } + } + assert.True(t, fitsParent, "fits-fine parent must still be created") + assert.True(t, fitsTXT, "fits-fine TXT companion must still be created") + + logtest.TestHelperLogContains("cannot establish ownership TXT", hook, t) + }) + } +} + func testTXTRegistryMissingRecords(t *testing.T) { t.Run("No prefix", testTXTRegistryMissingRecordsNoPrefix) t.Run("With Prefix", testTXTRegistryMissingRecordsWithPrefix)