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), diff --git a/registry/txt_test.go b/registry/txt_test.go index ed3876479a..b7bb36732b 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -2109,3 +2109,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) +}