fix(registry/txt): create ownership TXT records before primary records

[Arkelenia]

Problem

TXTRegistry.ApplyChanges builds the Create batch it hands to the provider by iterating over filteredChanges.Create and appending the generated TXT ownership record(s) for each endpoint to that same slice:

for _, r := range filteredChanges.Create {
    ...
    filteredChanges.Create = append(filteredChanges.Create, im.generateTXTRecordWithFilter(r, im.existingTXTs.isAbsent)...)
    ...
}

The resulting batch order is always: all primary records first, then all their TXT ownership records.

Why this matters

registry.ApplyChanges hands this ordered slice straight to provider.ApplyChanges. Providers that apply changes as a single atomic batch (e.g. AWS Route53's ChangeResourceRecordSets) are unaffected — either the whole batch lands or none of it does.

Providers that don't have atomic batch semantics are a different story. Azure in particular inserts records serially, one API call per record, in the order they appear in the slice. If such a batch partially fails partway through, the current ordering means the failure mode is:

• Primary record (e.g. an A record) gets created.
• Its corresponding TXT ownership record — being later in the slice — never gets created because the batch aborted before reaching it.

Once that happens, the registry has a primary record with no TXT owner record. On every subsequent reconciliation, TXTRegistry sees a record it doesn't recognize as owned and treats it as unowned — it will never touch, update, or clean up that record again. The record is permanently orphaned from ExternalDNS's perspective, and the only way out is a manual fix in the DNS zone.

Fix

Collect the generated TXT ownership records into a separate slice and prepend them to filteredChanges.Create, instead of appending them in-place:

createTXTRecords := make([]*endpoint.Endpoint, 0, len(filteredChanges.Create))
for _, r := range filteredChanges.Create {
    ...
    createTXTRecords = append(createTXTRecords, im.generateTXTRecordWithFilter(r, im.existingTXTs.isAbsent)...)
    ...
}
filteredChanges.Create = append(createTXTRecords, filteredChanges.Create...)

Now the batch order is: all TXT ownership records first, then their primary records. If a non-atomic provider like Azure only partially applies the batch, the worst case is now an orphan TXT record with no primary record — which is harmless (it doesn't affect DNS resolution) and gets picked up and either reused or cleaned up on the very next reconciliation.

This mirrors the ordering the Delete path already has: primary records are deleted before their TXT records (they appear first in that slice already, by the same append-while-ranging quirk), so a partial delete failure on a non-atomic provider similarly degrades to "an orphan, harmless TXT record left behind" rather than "a primary record silently losing its owner."

The general invariant this restores: a TXT ownership record must always be created before, and deleted after, its corresponding primary record, so that no window exists where a primary record can exist without a TXT owner.

Scope / impact

This is a correctness fix for any provider without atomic multi-record apply semantics. It's a no-op for providers that apply the whole batch atomically (AWS, Google Cloud DNS, etc.), since order within the batch doesn't matter when it's all-or-nothing.

Test plan

• go test ./registry/...

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: Arkelenia / name: Frederic Hemery (27c379d)

[kubernetes-prow]

Hi @Arkelenia. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign mloiseleur for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[u-kai]

Out of curiosity — how did you verify that this change doesn't break any provider? I checked for code that references Changes.Create positionally (didn't find any), but wondering if you also ran anything more specific, e.g. against certain
providers.

Also wanted to raise a question: this fix helps when the whole batch application gets cut short partway through (crash, ctx timeout/cancellation, pod eviction, etc.), but I don't think it does much for implementations that keep processing after a single record's creation fails.

Since TXT and primary creation are effectively attempted independently, reordering them doesn't stop "TXT fails, A still succeeds right after" from happening (Azure's current updateRecords/deleteRecords are a
concrete example of this — they just log.Errorf on a per-record failure and keep going, no return/break).

If that's a fair read, could you scope the PR description to "batch execution gets interrupted partway through" rather than
implying it covers partial failures in general? As written it reads like it also protects against providers that swallow
per-record errors and continue.

One more suggestion: could you add a regression test that pins down the ordering — i.e. asserts that TXT record(s) come
before their primary record in filteredChanges.Create after ApplyChanges? Right now nothing in registry_test.go checks this
specifically (existing assertions tend to use order-independent matching), so a future refactor of that loop could silently
reintroduce the original bug without any test catching it. Something like:

func TestApplyChangesCreatesTXTBeforePrimary(t *testing.T) {
    // ... registry setup ...
    err = r.ApplyChanges(ctx, &plan.Changes{
        Create: []*endpoint.Endpoint{
            endpoint.NewEndpoint("foo.example.com", endpoint.RecordTypeA, "1.2.3.4"),
        },
    })
    require.NoError(t, err)

    changes := mockProvider.RecordedChanges
    require.Len(t, changes.Create, 2)
    assert.Equal(t, endpoint.RecordTypeTXT, changes.Create[0].RecordType)
    assert.Equal(t, endpoint.RecordTypeA, changes.Create[1].RecordType)
}