Upload custom SSL certificates for proxied accessories - #1951
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
A missing private-key secret can leave a newly uploaded certificate paired with the previous key.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds custom SSL certificate uploads for proxied accessories, addressing #1769.
Changes:
- Namespaces accessory certificates under
tls/accessories/<name>/. - Uploads certificates before accessory proxy registration.
- Adds configuration and CLI coverage.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
lib/kamal/cli/accessory.rb |
Uploads accessory certificates during boot. |
lib/kamal/commands/accessory/proxy.rb |
Creates accessory TLS directories. |
lib/kamal/configuration/accessory.rb |
Assigns namespaced TLS paths. |
test/cli/accessory_test.rb |
Tests boot uploads and proxy arguments. |
test/configuration/accessory_test.rb |
Tests accessory certificate paths. |
test/fixtures/deploy_with_accessories_with_different_registries.yml |
Adds a proxied accessory fixture. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
An accessory can run behind kamal-proxy on its own hostname with a
certificate you provide, instead of one from Let's Encrypt:
accessories:
imgproxy:
proxy:
host: img.example.com
ssl:
certificate_pem: IMGPROXY_CERT
private_key_pem: IMGPROXY_KEY
The typical case is a subdomain fronted by a CDN: for example imgproxy
behind Cloudflare, which caches the processed images and reaches the
origin over HTTPS using a Cloudflare Origin Certificate. The config has
been accepted since custom certificates were added, but booting such an
accessory has always failed:
Error: unable to load certificate
`kamal-proxy deploy` is told where the certificate lives inside the
proxy's apps-config directory, but nothing ever writes it there. Only
the app boot path (Kamal::Cli::App::SslCertificates) uploads
certificates; `kamal accessory boot` never did.
Upload the certificate and key in the accessory boot path, right after
the secrets and before the container is registered with kamal-proxy,
using the same Proxy accessors and upload! calls as the app. There is
no extra validation to add: the config validator already requires both
keys, and Kamal::Secrets raises when a secret is missing. Both secrets
are resolved before the directory is created, so a missing one fails
without touching the host.
Store the files under tls/accessories/<name>/ by giving the accessory's
Proxy config "accessories/<name>" as its role_name. App roles use
tls/<role>/, so an accessory named like a role would otherwise
overwrite that role's certificate. This mirrors how the env directory
already keeps env/roles/<name>.env and env/accessories/<name>.env
apart.
Fixes basecamp#1769
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2f522bc to
cac0352
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The implementation consistently namespaces and uploads both TLS files before proxy registration, with focused coverage and no unresolved issues.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Balanced
An accessory can run behind kamal-proxy on its own hostname with a certificate you provide, for example imgproxy on a subdomain fronted by Cloudflare, with a Cloudflare Origin Certificate:
The config is accepted, but
kamal accessory bootfails withError: unable to load certificatefromkamal-proxy deploy. The deploy command points at the certificate inside the proxy's apps-config directory, but nothing writes it there: only the app boot path (Kamal::Cli::App::SslCertificates) uploads certificates.This does for accessories what the app boot already does:
Proxyaccessors andupload!calls as the apptls/accessories/<name>/, since app roles usetls/<role>/and an accessory named like a role would otherwise overwrite that role's certificate. This mirrors how the env directory already keepsenv/roles/<name>.envandenv/accessories/<name>.envapartNo extra validation: the proxy validator already requires both keys, and
Kamal::Secretsraises when a secret is missing.Unit tests cover the config paths and the boot output. I also verified it end to end in the integration environment: a proxied accessory with a custom certificate boots, the files land in
.kamal/proxy/apps-config/<service>/tls/accessories/<name>/, and kamal-proxy serves it over HTTPS with that certificate.CI (RuboCop, unit and integration tests on Ruby 3.2 to 4.0) is green on my fork: https://github.com/pigoz/kamal/actions/runs/33794121433
Fixes #1769
Notes for review
Kamal::Secrets#[]raises aConfigurationErrornaming the missing one, so a bad config fails without touching the host. That is also why the uploads have no nil guards.0644. kamal-proxy runs as its own user inside the container and reads the files through the bind-mounted apps-config directory, so they must be readable by that user. The app path uploads with the same mode, into the same directory tree under the deploy user's.kamal.tls/<role>/for roles andenv/accessories/<name>.envfor accessories today. Neither role nor accessory names are validated anywhere, and an accessory name containing/already fails at the env upload and atdocker runbefore this code runs. Validating names would be a separate change that could break existing configs.deploy_with_accessories_with_different_registries.ymlis only used bytest/cli/accessory_test.rb. Givingbusyboxa proxy, astest/commands/accessory_test.rbalready does, only adds akamal-proxy deployline to theboot alloutput, and lets the new test reuse the fixture with the same stubs as the app'sboot with custom ssl certificatetest.kamal accessory startdoes not re-upload the files, same askamal app start.rebootgoes throughboot, and the files persist on the host across proxy reboots.