fix: bring the provider in line with the libdns contract and the Njalla API - #6
Merged
Conversation
The package now uses the built-in min function, and 1.23 matches the toolchain the libdns CI targets. libdns v1.1.0 itself still declares 1.18, so this only raises the floor for this module. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
Verified against the live Njalla API; each item below reproduces there. SetRecords now implements RRset semantics. It previously looked up a single record per name and type and edited it once per input record, so setting two addresses over an existing set of three edited the same record twice and left the surplus in place, while reporting both inputs as set. Records are now grouped by RRset and reconciled: existing IDs are reused, missing records created, and surplus ones deleted. DeleteRecords now matches on the record value. It previously matched on name and type alone, which for several records sharing those could delete the wrong one. Njalla's remove-record accepts the record's fields directly, so matching is delegated to the API: this removes the listing step, makes an empty value delete the whole RRset as the contract requires, and lets deletion work with tokens restricted to the ACME challenge prefix, whose listing is filtered. A 404 means nothing matched, which the contract requires be treated as success. Njalla matches hostnames byte-for-byte, so both trailing-dot spellings are tried before concluding a record is absent. HTTPS and SVCB records are no longer conflated. Reading an HTTPS record left the ServiceBinding scheme unset, so it reported itself as SVCB, and every write was hardcoded to HTTPS. Both now follow the RR type. Opaque libdns.RR input is parsed rather than passed through whole. The contract requires accepting any Record value; previously an RR carrying an MX or SRV lost its priority, weight, and port. Numeric fields are transmitted when zero. The omitempty tags dropped them, and Njalla requires prio for MX and HTTPS and accepts a priority, weight, or port of 0 for SRV, so valid records were rejected with "Missing required field". Unknown types are parsed into concrete libdns structs where possible and otherwise returned as libdns.RR, which keeps Njalla's proprietary Redirect and Dynamic types readable. A null ttl no longer breaks decoding. Records Njalla will not store are reported before the request is sent, instead of surfacing as a generic "Invalid DNS record": empty TXT values and those containing a double quote, MX preference 0, SVCB records, and SRV records below the zone apex. The last two are API limitations documented in the README. The API client is now injectable through Provider.HTTPClient and reuses one client rather than building one per retry. Errors carry the response body, API failures are returned as *APIError so callers can inspect the code, and 429 responses honour Retry-After. getClient no longer latches a nil client when the token is set after the first call, and SetRecords serialises its read-modify-write per zone. The tests are rewritten alongside. The previous suite asserted the implementation's behaviour rather than the contract, so it passed while SetRecords corrupted RRsets and HTTPS records were returned as SVCB. Fixtures mirror responses captured from the live API, and the mock client asserts on decoded request parameters so tests can check which fields were sent and which were omitted. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
Runs the shared libdnstest suite against a real zone, following the layout used by libdns/cloudflare. It is skipped unless NJALLA_API_TOKEN and NJALLA_TEST_ZONE are set. SRV and SVCB are excluded because Njalla cannot represent them: it validates the last two labels of a SRV name, so it accepts SRV only at the zone apex and rejects the RFC 2782 form, and it rejects the _scheme label libdns generates for SVCB. Working around either would publish a record at the wrong name. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
Checks formatting, vet, and the race-enabled unit tests on every push and pull request. The conformance suite runs only when a test zone is configured, and is skipped for pull requests from forks. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
Records the API token permissions each libdns method needs, including Njalla's ACME token option and the fact that it also restricts what list-records returns, which is what made DNS challenge cleanup appear to fail in caddy-dns/njalla#6. Also documents the behaviours verified against the live API: SRV apex only, SVCB unsupported, no MX preference 0, TXT restrictions, verbatim trailing dots, and zero-TTL handling. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
This was referenced Aug 21, 2026
Restructured into Install, Authentication, Usage, Record types, Limitations, Behaviour, and Testing, matching the layout used in caddy-dns/njalla so the two read as a pair. The API constraints move into a table with the two that need explaining written out below it, replacing a run of similar-looking paragraphs. Prose cut from 716 to 589 words while gaining that table. Also removed the AI writing tells the text had picked up: passive constructions, adverbs, and "not X, Y" contrasts. The Go example now compiles and vets clean against the package. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings the provider in line with the libdns v1.1.0 contract and with how the
Njalla API actually behaves. Every behavioural claim below was verified against
the live API, and the shared
libdnstestconformance suite now passes against areal zone.
Fixes #4.
This supersedes #5, which correctly diagnosed the
DeleteRecordsproblem in #4but fixed it in a way that regressed other record types. Its two regression
tests are preserved here in equivalent form.
Correctness fixes
SetRecordsdid not implement RRset semantics. libdns requires that, foreach
(name, type)pair in the input, the supplied records become the onlyrecords in the zone with that name and type. The old implementation looked up a
single record per name and type and edited it once per input record. Setting two
addresses over an existing set of three issued
edit-recordtwice against thesame ID, left the surplus record in place, and returned both inputs as
successfully set. Records are now grouped by RRset and reconciled: existing IDs
are reused, missing records are created, and surplus ones are deleted.
DeleteRecordscould delete the wrong record. It matched on name and typealone, so with several records sharing those it removed whichever the API listed
first. This is #4. Njalla's
remove-recordaccepts the record's fields directly,so the match is now delegated to the API. That has three further benefits:
that an empty value matches any value;
list-recordscall is needed, so deletion works with tokens restricted tothe
_acme-challengeprefix, whose listing is filtered. This is the failurereported in DNS challenge TXT record not being removed caddy-dns/njalla#6;
outcome rather than echoing the caller's input.
A
404fromremove-recordmeans nothing matched, which the contract requiresbe treated as success rather than an error.
HTTPS and SVCB were conflated. Reading an HTTPS record left
ServiceBinding.Schemeunset, so the record reported its type asSVCB, andevery write was hardcoded to
HTTPS. Both directions now follow the RR type.Opaque
libdns.RRinput was mishandled. The contract requires accepting anyRecordvalue. AnRRcarrying an MX lost its preference and sent10 mail.example.com.as the content; anRRcarrying a SRV lost priority,weight, and port. Input is now parsed into its concrete type first.
Zero-valued numeric fields were dropped. The
omitemptytags removed themfrom the request. Njalla requires
priofor MX and HTTPS, and accepts apriority, weight, or port of
0for SRV, so valid records were rejected withMissing required field.Other fixes. Unknown types are parsed into concrete libdns structs where
possible and otherwise returned as
libdns.RR, keeping Njalla's proprietaryRedirectandDynamictypes readable; a nullttlno longer breaks decoding;getClientno longer latches a nil client if the token is set after the firstcall; and
SetRecordsserialises its read-modify-write per zone.API limitations now reported clearly
These are Njalla constraints, confirmed against the live API. They previously
surfaced as the API's generic
Invalid DNS record, and are now reported beforethe request is sent.
protocol labels first, so a record for
voiceis named_sip._tcp.voice.Njalla validates the last two labels, so it accepts
_sip._tcpbut rejects_sip._tcp.voice, and also rejects the example given in Njalla's owndocumentation (
_xmpp-server._tcp.conference). Reordering the labels tovoice._sip._tcpis accepted but publishes the record at a name that is not avalid SRV location, so this package does not do it.
giving
_dns.example, and Njalla rejects underscore labels for this type.HTTPS carries no such prefix and works normally.
round trips verbatim, including spaces, semicolons, non-ASCII text, and values
longer than 255 bytes, so no escaping or chunking is applied.
Two behaviours are documented rather than worked around. Njalla stores hostname
values exactly as given and matches them byte-for-byte, so trailing dots are
passed through unchanged in both directions; when deleting, both spellings are
tried so a record entered through the web interface can still be removed. And a
TTL of 0 is left unset, which Njalla defaults to 10800 seconds, matching the
libdns.RRguidance that a sub-second duration is the way to ask for 0.Additions
ListZonesvialist-domains, so the provider now implementsZoneLister.Provider.HTTPClientfor injecting a custom client, as libdns/cloudflare has.*APIErrorso callers can inspect the code, withIsNotFoundfor the specific case; HTTP errors carry the response body; and429responses honourRetry-After.Validation
The previous unit tests asserted the implementation's behaviour rather than the
contract, so they passed at 87.8% statement coverage while
SetRecordswascorrupting RRsets. They have been rewritten against the contract, with fixtures
mirroring responses captured from the live API. Coverage is now 89.7%, but the
relevant point is that the new tests fail against the old code.
libdnstest/is a new module running the shared libdns conformance suite,following the layout used by libdns/cloudflare. It is skipped unless
NJALLA_API_TOKENandNJALLA_TEST_ZONEare set, and excludes SRV and SVCB forthe reasons above.
Run against a live Njalla zone:
The zone's record set was byte-identical before and after each run.
Also validated by building Caddy with this provider through xcaddy and
confirming
dns.providers.njallaregisters and the Caddyfile adapts.Other changes
a test zone is configured.
godirective moves to 1.23, matching the toolchain libdns CI targets.token option, and the provider-specific behaviour above.
Note on releases
This repository has never been tagged, which is why caddy-dns/njalla pins a
commit pseudo-version. Tagging a release after this merges would let the Caddy
module depend on a real version.