Skip to content

caddytls: Expand ACME credentials#7554

Merged
steadytao merged 2 commits intocaddyserver:masterfrom
tribut:tls-placeholders
May 2, 2026
Merged

caddytls: Expand ACME credentials#7554
steadytao merged 2 commits intocaddyserver:masterfrom
tribut:tls-placeholders

Conversation

@tribut
Copy link
Copy Markdown
Contributor

@tribut tribut commented Mar 7, 2026

This allows using global placeholders such as {file./run/secrets/key_id} when setting up the tls configuration.

The feature was also requested / discussed at https://caddy.community/t/illegal-base64-data-error-with-file-placeholder-for-eab-secrets/33343

Assistance Disclosure

Copilot was used to locate the file and function that this PR modifies.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 7, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enables use of global placeholders (e.g. {file./run/secrets/...}) in ACME issuer configuration so values like CA directory endpoints and EAB credentials can be sourced dynamically (e.g. from mounted secrets).

Changes:

  • Expand placeholders in ACMEIssuer.CA during provisioning.
  • Expand placeholders in ACMEIssuer.ExternalAccount (KeyID, MACKey) during provisioning.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread modules/caddytls/acmeissuer.go Outdated
Comment thread modules/caddytls/acmeissuer.go
@steadytao
Copy link
Copy Markdown
Member

Copilot's comments are good. And;

Locally, the PR appears to be based on v2.11.2 rather than current master. It is missing #7609 which added placeholder expansion for dns_challenge override_domain in the same ACMEIssuer.Provision() area. Even if GH considers the branch mergeable I would probably double check so the existing override-domain expansion is preserved alongside the new CA/EAB expansion. I would also like to see a targeted provisioning test that verifies placeholders are expanded for CA, ExternalAccount.KeyID and ExternalAccount.MACKey. Can be added by myself if needed.

Also, a quick question. Should TestCA be expanded too? Since this PR expands CA, leaving the fallback/test CA endpoint unexpanded feels a little inconsistent.

This allows using global placeholders such as {file./run/secrets/key_id}
when setting up the tls configuration.
@tribut tribut force-pushed the tls-placeholders branch from 23aa9c3 to d706a6c Compare May 1, 2026 10:01
@tribut
Copy link
Copy Markdown
Contributor Author

tribut commented May 1, 2026

Thank you for the feedback @steadytao, I agree that TestCA should be expanded as well.

I've rebased onto current master, redacted the mac key, added TestCA expansion and added a simple test for the fields added in this PR.

Let me know if you have further comments.

@steadytao
Copy link
Copy Markdown
Member

Thank you for the rebase. Looks much better, just did a quick gofmt, after that passes, happy to merge.

Copy link
Copy Markdown
Member

@steadytao steadytao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint failure is not caused by this PR;

  • caused by an action update to v2.12.1 of golangci-lint.

Will be handled separately.

@steadytao steadytao merged commit ef496e5 into caddyserver:master May 2, 2026
24 of 27 checks passed
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.

4 participants