feat: per-container private key renewal#1268
Merged
buchdag merged 2 commits intoJun 19, 2026
Merged
Conversation
564ddb4 to
82e127a
Compare
There was a problem hiding this comment.
Pull request overview
Adds a per-proxied-container override for private key rotation on renewal, enabling stable keys for specific certificates (e.g., DANE/TLSA mail certs) while keeping the global default behavior for everything else.
Changes:
- Add
LETSENCRYPT_RENEW_PRIVATE_KEYSsupport via docker-gen templating into per-container variables. - Update certificate issuance logic to honor
LETSENCRYPT_${cid}_RENEW_PRIVATE_KEYS(with fallback to globalRENEW_PRIVATE_KEYS). - Add an integration test and register it in CI and the test suite.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
app/letsencrypt_service |
Reads per-container RENEW_PRIVATE_KEYS override and conditionally passes --always-force-new-domain-key. |
app/letsencrypt_service_data.tmpl |
Exposes LETSENCRYPT_RENEW_PRIVATE_KEYS from proxied containers into generated per-container env. |
docs/Container-configuration.md |
Documents the per-container override behavior. |
test/tests/renew_private_keys/run.sh |
New integration test to verify key reuse across a forced renewal when override is false. |
test/config.sh |
Registers the new test in the global test list. |
.github/workflows/test.yml |
Adds the new test to the CI matrix (2containers). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
buchdag
reviewed
Jun 18, 2026
RENEW_PRIVATE_KEYS could only be set globally, so users could not keep a stable private key for one certificate (e.g. a DANE/TLSA mail cert) while still rotating keys elsewhere (nginx-proxy#1191). Add a per-container override using the existing per-container nameref pattern. - app/letsencrypt_service: read LETSENCRYPT_${cid}_RENEW_PRIVATE_KEYS, falling back to the global RENEW_PRIVATE_KEYS, when deciding whether to pass acme.sh --always-force-new-domain-key. - app/letsencrypt_service_data.tmpl: surface LETSENCRYPT_RENEW_PRIVATE_KEYS from the proxied container into the per-container data. - docs/Container-configuration.md: document the per-container override. - test/tests/renew_private_keys: integration test asserting a container with LETSENCRYPT_RENEW_PRIVATE_KEYS=false keeps its key across a renewal. Closes nginx-proxy#1191 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Rename the per-container override to ACME_RENEW_PRIVATE_KEYS (was LETSENCRYPT_RENEW_PRIVATE_KEYS) per maintainer feedback: the ACME_ prefix is preferred now that Let's Encrypt is no longer the only ACME CA. - renew_private_keys test: capture the openssl output and fail explicitly when the key can't be read, so an empty fingerprint cannot cause a false pass (Copilot). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
0c929d5 to
2dc1e7a
Compare
buchdag
approved these changes
Jun 19, 2026
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.
What
Allow
RENEW_PRIVATE_KEYSto be overridden per proxied container via a newLETSENCRYPT_RENEW_PRIVATE_KEYSvariable, instead of only globally on the companion.Why
RENEW_PRIVATE_KEYScould only be set globally, so it was all-or-nothing. The reporter (#1191) wants to keep a stable private key for one certificate — a DANE/TLSA mail cert (rspamd), where rotating the key would break published TLSA records — while still rotating keys on every renewal for their other web certificates. There was no way to do that short of running a second companion instance.How
Follows the existing per-container nameref pattern (same as
ACME_${cid}_RENEW_AFTER,LETSENCRYPT_${cid}_KEYSIZE, etc.):app/letsencrypt_service—update_certnow readsLETSENCRYPT_${cid}_RENEW_PRIVATE_KEYSand falls back to the globalRENEW_PRIVATE_KEYSwhen unset, before deciding whether to pass acme.sh--always-force-new-domain-key.app/letsencrypt_service_data.tmpl— surfacesLETSENCRYPT_RENEW_PRIVATE_KEYSfrom the proxied container into the per-container data (both the SAN and single-domain-cert branches).docs/Container-configuration.md— documents the per-container override.test/tests/renew_private_keys/— integration test: a container withLETSENCRYPT_RENEW_PRIVATE_KEYS=falsemust keep the same private key across a forced renewal, even though the companion's global default (true) would otherwise rotate it. Registered intest/config.shand the CI matrix.The global
RENEW_PRIVATE_KEYS(and the legacyREUSE_PRIVATE_KEYS) behaviour is unchanged when the per-container variable is not set.Testing
renew_private_keysintegration test added (CI, 2containers).docker-genemitsLETSENCRYPT_<cid>_RENEW_PRIVATE_KEYS="false"for a container settingLETSENCRYPT_RENEW_PRIVATE_KEYS=false).shellcheckclean on the modified script and the new test.Closes #1191
🤖 Generated with Claude Code