From 2cae1f94e96f676e76a4cece009b7824a1447c4a Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Thu, 9 Oct 2025 07:40:08 +0900 Subject: [PATCH 1/3] fix(txt-register): reset existingTXTs even when ApplyChanges is skipped to avoid stale TXT records --- registry/txt.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/registry/txt.go b/registry/txt.go index fef387e26a..7802367ce2 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -173,6 +173,12 @@ func (im *TXTRegistry) OwnerID() string { // If TXT records was created previously to indicate ownership its corresponding value // will be added to the endpoints Labels map func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error) { + // existingTXTs must always hold the latest TXT records, so it needs to be reset every time. + // Previously, it was reset with a defer after ApplyChanges, but ApplyChanges is not called + // when plan.HasChanges() is false (i.e., when there are no changes to apply). + // In that case, stale TXT record information could remain, so we reset it here instead. + im.existingTXTs.reset() + // If we have the zones cached AND we have refreshed the cache since the // last given interval, then just use the cached results. if im.recordsCache != nil && time.Since(im.recordsCacheRefreshTime) < im.cacheInterval { @@ -321,8 +327,6 @@ func (im *TXTRegistry) generateTXTRecordWithFilter(r *endpoint.Endpoint, filter // 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 - filteredChanges := &plan.Changes{ Create: changes.Create, UpdateNew: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateNew), From c446014baff5c2b48ef7a2d2ceabd8bd1a32ccb4 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Thu, 9 Oct 2025 08:24:59 +0900 Subject: [PATCH 2/3] test(txt-registry): add regression test to ensure TXT records are recreated after deletion --- registry/txt_test.go | 80 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/registry/txt_test.go b/registry/txt_test.go index ed3876479a..5f6c1209c8 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -10,6 +10,7 @@ You may obtain a copy of the License at Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and limitations under the License. */ @@ -2109,3 +2110,82 @@ func TestTXTRecordMigration(t *testing.T) { assert.Equal(t, updatedTXTRecord[0].Targets, expectedFinalTXT[0].Targets) } + +// TestRecreateRecordAfterDeletion ensures that when A and TXT records are deleted, +// both are correctly recreated in subsequent reconciliation loops. +// This prevents regression of the issue where stale TXT record state +// caused ExternalDNS to skip recreating TXT records after deletion. +func TestRecreateRecordAfterDeletion(t *testing.T) { + ownerID := "foo" + ctx := context.Background() + p := inmemory.NewInMemoryProvider() + p.CreateZone(testZone) + + r, _ := NewTXTRegistry(p, "%{record_type}-", "", "foo", 0, "", []string{endpoint.RecordTypeA}, []string{}, false, nil, "") + + createdRecords := newEndpointWithOwnerAndLabels("bar.test-zone.example.org", "1.2.3.4", endpoint.RecordTypeA, ownerID, nil) + txtRecord := r.generateTXTRecord(createdRecords) + + // 1. Create initial A and TXT records. + creates := append([]*endpoint.Endpoint{createdRecords}, txtRecord...) + err := p.ApplyChanges(ctx, &plan.Changes{ + Create: creates, + }) + assert.NoError(t, err) + + // 2. Simulate a "no change" reconciliation (ApplyChanges won't be called). + desired := []*endpoint.Endpoint{ + { + DNSName: "bar.test-zone.example.org", + Targets: endpoint.Targets{ + "1.2.3.4", + }, + RecordType: endpoint.RecordTypeA, + }, + } + + records, err := r.Records(ctx) + assert.NoError(t, err) + + calculated := &plan.Plan{ + Policies: []plan.Policy{&plan.SyncPolicy{}}, + ManagedRecords: []string{endpoint.RecordTypeA}, + Current: records, + Desired: desired, + OwnerID: ownerID, + } + calculated = calculated.Calculate() + // ApplyChanges is not called to simulate no changes. + assert.False(t, calculated.Changes.HasChanges(), "There should be no changes") + + // 3. Delete both A and TXT records (simulate manual deletion) + deletes := append([]*endpoint.Endpoint{createdRecords}, txtRecord...) + err = p.ApplyChanges(ctx, &plan.Changes{ + Delete: deletes, + }) + assert.NoError(t, err) + + // 4. Run reconciliation again — both A and TXT should be recreated. + records, err = r.Records(ctx) + assert.NoError(t, err) + + calculated = &plan.Plan{ + Policies: []plan.Policy{&plan.SyncPolicy{}}, + ManagedRecords: []string{endpoint.RecordTypeA}, + Current: records, + Desired: desired, + OwnerID: ownerID, + } + calculated = calculated.Calculate() + if !calculated.Changes.HasChanges() { + assert.Fail(t, "There should be changes") + } + + err = r.ApplyChanges(ctx, calculated.Changes) + assert.NoError(t, err) + + // 5. Verify that both A and TXT records are recreated successfully. + records, err = p.Records(ctx) + assert.NoError(t, err) + assert.True(t, testutils.SameEndpoints(records, append(desired, txtRecord...)), "Expected records after reconciliation: %v, but got: %v", append(desired, txtRecord...), records) +} From b70bde40119d89583fb4112e3be3908b233e9789 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Thu, 9 Oct 2025 08:29:59 +0900 Subject: [PATCH 3/3] chore: remove unintended blank line --- registry/txt_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/registry/txt_test.go b/registry/txt_test.go index 5f6c1209c8..b7bb36732b 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -10,7 +10,6 @@ You may obtain a copy of the License at Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and limitations under the License. */