feat: add tests and CI, and remove the module template scaffolding - #7
Merged
Conversation
The module still carried the caddy-dns template's placeholder comments,
including "THIS IS JUST AN EXAMPLE AND NEEDS TO BE CUSTOMIZED" on the
UnmarshalCaddyfile doc comment, which is published on pkg.go.dev.
The parser is restructured along the lines of caddy-dns/cloudflare: the
directive name is consumed once, and the inline and block forms are
mutually exclusive rather than partially overlapping.
Provision now reports an empty API token instead of letting it through.
A placeholder such as {env.NJALLA_API_TOKEN} resolves to an empty string
when the variable is unset, which otherwise surfaces much later as an
authentication failure.
Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
The module had no tests. These cover both supported syntaxes, the argument and subdirective errors, placeholder resolution, and the module registration itself. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
Mirrors the one in caddy-dns/cloudflare, building Caddy with this module via xcaddy. It builds an image locally; no registry publishing is configured. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
Checks formatting, vet, and race-enabled tests. A separate job builds Caddy with the module through xcaddy and asserts that dns.providers.njalla is registered, which a plain package build does not prove. A third job builds the Docker image so it cannot rot; it does not publish. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
Adds Caddyfile examples alongside the existing JSON one, including the global acme_dns form, and explains building with xcaddy or Docker. Documents Njalla's ACME token option, which grants exactly the permissions the DNS challenge needs, and adds troubleshooting notes for the failure modes seen in #5 and #6. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
Restructured into Install, Authentication, Usage, Troubleshooting, and Testing, matching the layout used in libdns/njalla so the two read as a pair. Troubleshooting becomes a symptom-to-cause table, and the Caddyfile examples are ordered per-site, global, then the block form. Every Caddyfile snippet in the file adapts cleanly against Caddy v2.11.4. Also removed the AI writing tells the text had picked up: three em dashes, adverbs, and passive constructions. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
"Issue for the Njalla domain" read as a noun. Reworded so the instruction is unambiguous. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
The restructured parser consumed the inline argument and then returned without reading the block, so a config specifying both silently used the argument and discarded the block. The previous parser rejected this. Both forms are still accepted on their own; supplying both now reports the ambiguity. Claude-Session: https://claude.ai/code/session_014sP5zuZiTmWEzSvL6JxaTZ
Replaces the 2025 commit pseudo-version with the first tagged release, which carries the corrected SetRecords and DeleteRecords behaviour. DeleteRecords no longer lists records to find the one to remove, so the ACME DNS-01 challenge cleans up correctly with a token scoped to the _acme-challenge prefix. That is the failure reported in #6. 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
Removes the caddy-dns template scaffolding this module still shipped, adds the
tests and CI it had none of, and expands the documentation. Structure follows
caddy-dns/cloudflare.
Template scaffolding
The module still carried the template's placeholder comments, including three
// TODO: This is just an examplemarkers and, on theUnmarshalCaddyfiledoccomment, the line:
That doc comment is published verbatim on pkg.go.dev, so it was the first thing
a prospective user read about configuring the module.
Caddyfile parsing
Restructured along the lines of caddy-dns/cloudflare: the directive name is
consumed once rather than in a loop, and the inline and block forms are now
mutually exclusive rather than partially overlapping. Both supported syntaxes
are unchanged from a user's point of view:
Provisionnow reports an empty API token instead of passing it through. Aplaceholder such as
{env.NJALLA_API_TOKEN}resolves to an empty string whenthe variable is not set in the process running Caddy, which otherwise surfaces
much later as an authentication failure against the Njalla API. This is a
common cause of confusion when Caddy runs under systemd, which does not inherit
the invoking shell's environment.
No token format validation is included. caddy-dns/cloudflare can do this because
Cloudflare's token formats are documented; Njalla publishes no such format, and
guessing a pattern risks rejecting valid tokens.
Tests
The module previously had none. The new tests cover both Caddyfile syntaxes,
the argument and subdirective error paths, placeholder resolution during
provisioning, the empty-token rejection, and module registration.
CI
Three jobs:
go vet, and race-enabled tests;dns.providers.njallaappears incaddy list-modules, which a plain package build does not prove;Docker
Adds a
Dockerfilemirroring the one in caddy-dns/cloudflare, building Caddywith this module via xcaddy. It builds an image locally and CI builds it on
every run so it cannot rot. Publishing to a registry is deliberately not
configured here, since that is an organisation-level decision; caddy-dns/
cloudflare does publish to GHCR on tags, and the same could be added later if
wanted.
Documentation
Adds Caddyfile examples alongside the existing JSON one, including the global
acme_dnsform, and covers building with xcaddy or Docker.Documents Njalla's ACME token option, which grants exactly the permissions the
DNS-01 challenge needs, and adds troubleshooting notes for the two failure modes
reported in this repository: the build error in #5 and the DNS challenge cleanup
failure in #6.
Validation
Built with xcaddy against Caddy v2.11.4;
dns.providers.njallaregisters, andcaddy adaptproduces the expected TLS automation policy for both Caddyfilesyntaxes while rejecting an unknown subdirective with a clear message.
Also built with libdns/njalla#6 applied, confirming the updated provider links
and registers correctly.
Follow-up
This PR does not change
go.mod. The module currently pins libdns/njalla at a2025 commit pseudo-version, because that repository has never been tagged. Once
libdns/njalla#6 merges and a release is tagged, a follow-up should bump this
dependency to that version, which is what delivers the DNS challenge cleanup fix
tracked in #6 here.