Skip to content

Match exact record data when rediscovering IDs for deletion - #5

Closed
bjornmp wants to merge 1 commit into
libdns:masterfrom
bjornmp:robust-delete-records
Closed

Match exact record data when rediscovering IDs for deletion#5
bjornmp wants to merge 1 commit into
libdns:masterfrom
bjornmp:robust-delete-records

Conversation

@bjornmp

@bjornmp bjornmp commented May 9, 2026

Copy link
Copy Markdown

Summary

This change makes DeleteRecords match existing records by name, type, and data when it needs to rediscover a Njalla record ID. Previously the fallback lookup matched only name and type.

Why

When DeleteRecords is called without provider-specific ID data, the provider has to list records and find the matching Njalla ID before calling remove-record.

Matching only name and type is not precise enough. Multiple TXT records with the same name and type can exist at the same time, with different values. The fallback lookup should therefore include the record data/value to avoid deleting the wrong record.

Related: caddyserver/certmagic#382 would preserve provider metadata during CertMagic DNS-01 cleanup, making Njalla's provider ID available more often and reducing how often this fallback path is needed there. This fallback should still be correct for callers that pass only portable DNS record data without provider-specific ID metadata.

Validation

Added regression unit tests for:

  • no-ID deletion lookup selecting the TXT record with matching content
  • no-ID deletion after AppendRecords(...).RR() drops provider-specific metadata

Tested with:

go test ./...

Fixes #4

@engels74

Copy link
Copy Markdown
Collaborator

Thanks for this. The diagnosis in #4 holds, and is understated: the current code does not merely pick imprecisely, it deletes the wrong record. With T1=tok1 and T2=tok2 at the same name, asking to delete tok2 removes T1 on master.

Verifying this branch against the live Njalla API turned up two regressions, though:

  • Njalla stores hostnames verbatim and matches them byte-for-byte. libdns recommends trailing dots, so MX{Target: "mail.example.com."} has RR().Data of 10 mail.example.com. while the zone returns 10 mail.example.com. Exact comparison of rr.Data stops matching, and deleting MX, CNAME, NS, or SRV becomes a silent no-op that still returns a nil error.
  • libdns defines an empty value as "match any value". libdns.RR{Name, Type} with no Data deletes one record before this change and none after.

Superseded by #6. Njalla's own remove-record accepts name, type, and content directly, so the matching is delegated to the API: that fixes #4, gives the empty-value semantics for free, and drops the list-records call entirely, which also fixes caddy-dns/njalla#6 where an ACME-scoped token's prefix-filtered listing made cleanup fail.

Both of your regression tests are carried over in #6. Thanks for finding this.

@engels74 engels74 closed this Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DeleteRecords fallback should match exact record data

2 participants