feat(dashmate): add Let's Encrypt SSL provider support #3000
+711
−11
Conversation
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
Add Let's Encrypt as a new SSL certificate provider option using the goacme/lego Docker client. This enables IP-based certificate issuance with automatic renewal support for shortlived certificates. New files: - LegoCertificate.js - Certificate model for parsing PEM files - validateLetsEncryptCertificateFactory.js - Validation logic - obtainLetsEncryptCertificateTaskFactory.js - Main obtain task - scheduleRenewLetsEncryptCertificateFactory.js - Renewal scheduler Changes: - Add LETSENCRYPT to SSL_PROVIDERS constant - Add letsencrypt config schema and defaults - Add Let's Encrypt to setup wizard choices - Update ssl obtain command with --provider flag - Update helper.js with Let's Encrypt renewal scheduling Co-Authored-By: Claude Opus 4.5 <[email protected]>
Contributor
📝 WalkthroughWalkthroughAdds Let’s Encrypt as a new SSL provider: schema, constants, DI wiring, certificate model and validators, obtain/renew Listr tasks (via Docker Lego), scheduler for automatic renewals, CLI/provider selection updates, defaults and migration to populate letsencrypt config. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI Command
participant DI as DI Container
participant Task as Obtain Task
participant Docker as Docker/Lego
participant Validator as Validator
participant Config as Config Repository
User->>CLI: run obtain (--provider letsencrypt)
CLI->>DI: resolve obtainLetsEncryptCertificateTask
DI->>Task: provide task with dependencies
Task->>Validator: validate existing certificate (config, expirationDays)
Validator->>Task: return validation result
alt Certificate missing/invalid
Task->>Docker: pull lego image & run lego container
Docker->>Task: produce cert and key files
end
Task->>Validator: parse/verify obtained cert
Validator->>Task: confirm validity
Task->>Config: enable SSL, set provider, save certs
Config->>Task: persisted
Task->>CLI: complete
sequenceDiagram
participant Scheduler as Renewal Scheduler
participant CronJob as CronJob
participant Task as Obtain Task
participant DockerCompose as Docker Compose
participant Gateway as Gateway Service
Scheduler->>Scheduler: read existing lego cert files
Scheduler->>Scheduler: compute renewal time (expiry - EXPIRATION_LIMIT_DAYS)
Scheduler->>CronJob: schedule renewal
CronJob->>Task: trigger obtainLetsEncryptCertificateTask
Task->>Task: obtain/renew certificate
Task->>Scheduler: return success/failure
alt success
Scheduler->>DockerCompose: execCommand SIGHUP to PID 1
DockerCompose->>Gateway: reload SSL configuration
Scheduler->>CronJob: stop job
else failure
Scheduler->>CronJob: schedule retry (1 hour)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/dashmate/src/helper/scheduleRenewLetsEncryptCertificateFactory.js`:
- Around line 67-94: The current scheduleRenewLetsEncryptCertificate CronJob
immediately reschedules on any failure (see CronJob callback, job.stop(), and
process.nextTick(() => scheduleRenewLetsEncryptCertificate(config))) which can
create a tight retry loop; modify the error path in the catch block so that
instead of immediately calling process.nextTick to reschedule, it waits a fixed
backoff (e.g., setTimeout with a sensible delay like 1 hour) before invoking
scheduleRenewLetsEncryptCertificate(config), while leaving the success path
unchanged; update references around obtainLetsEncryptCertificateTask, job.stop,
and process.nextTick so only the failure branch uses the delayed reschedule.
In
`@packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js`:
- Around line 43-44: The code uses the logical OR operator when setting
expirationDays which treats 0 as falsy; change the assignment in the
obtainLetsEncryptCertificateTaskFactory to use the nullish coalescing operator
so an explicit ctx.expirationDays = 0 is honored (i.e. replace "const
expirationDays = ctx.expirationDays || LegoCertificate.EXPIRATION_LIMIT_DAYS;"
with a nullish-coalescing version), then pass that expirationDays into
validateLetsEncryptCertificate unchanged.
In
`@packages/dashmate/src/ssl/letsencrypt/validateLetsEncryptCertificateFactory.js`:
- Around line 95-101: The current check in
validateLetsEncryptCertificateFactory.js only inspects
data.certificate.commonName and short-circuits when CN is null, so update
validate logic to inspect SubjectAltName (SAN) IP entries instead: extract
IP-type SANs from data.certificate.subjectAltName (or equivalent SAN list on
data.certificate), ensure at least one SAN IP equals data.externalIp, and return
ERRORS.CERTIFICATE_IP_MISMATCH if none match; also update the error message used
in obtainLetsEncryptCertificateTaskFactory.js (the message that currently prints
the null commonName) to reference the external IP and/or the SANs (not
commonName) so it never prints "null" as the certificate IP.
🧹 Nitpick comments (1)
packages/dashmate/src/config/configJsonSchema.js (1)
703-712: Consider adding email format validation to the schema.The
letsencrypt.emailfield accepts strings but has no format constraint. While the UI prompt validates email format, invalid emails could be loaded directly from config files bypassing the UI. Adding JSON Schema'sformat: 'email'would provide defense-in-depth validation.♻️ Suggested improvement
letsencrypt: { type: ['object'], properties: { email: { type: ['string', 'null'], + format: 'email', }, }, required: ['email'], additionalProperties: false, },Note: JSON Schema's
format: 'email'requires the validator to have format validation enabled.
packages/dashmate/src/helper/scheduleRenewLetsEncryptCertificateFactory.js
Show resolved
Hide resolved
packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js
Outdated
Show resolved
Hide resolved
packages/dashmate/src/ssl/letsencrypt/validateLetsEncryptCertificateFactory.js
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js`:
- Around line 186-193: After verifying ctx.legoCertPath exists, also check that
ctx.legoKeyPath exists and throw an Error if the key file was not created
(similar message to the cert check); update the success path only after both
files exist (the block inside the obtainLetsEncryptCertificateTaskFactory task
where task.output is set) so downstream code that reads ctx.legoKeyPath will not
fail.
- Around line 80-82: The error thrown for ERRORS.CERTIFICATE_IP_MISMATCH
currently interpolates ctx.certificate.commonName which can be null when
--disable-cn is used; update the message in the throw inside the
ERRORS.CERTIFICATE_IP_MISMATCH branch to avoid printing "null" by preferring a
SAN value when available (e.g., use ctx.certificate.subjectAlternativeNames or
the first SAN) or by omitting the CN and phrasing the message generically (e.g.,
"Certificate IP does not match external IP <ctx.externalIp>"). Make the change
in the ERRORS.CERTIFICATE_IP_MISMATCH handler where ctx.certificate.commonName
and ctx.externalIp are referenced so the message never shows "null."
- Around line 145-156: The docker container creation sets User: `${uid}:${gid}`
when calling docker.createContainer (containerName, LEGO_IMAGE, legoArgs) which
will prevent binding to port 80 as a non-root user; change the container options
to either remove the User field or add the NET_BIND_SERVICE capability in
HostConfig (CapAdd: ['NET_BIND_SERVICE']) so the goacme/lego image can bind to
privileged ports, or alternatively switch the task to use a DNS challenge
instead of exposing 80/tcp; update the block that builds the container options
(the object passed to docker.createContainer) accordingly and ensure
ExposedPorts/PortBindings remain consistent with the chosen approach.
♻️ Duplicate comments (2)
packages/dashmate/src/ssl/letsencrypt/validateLetsEncryptCertificateFactory.js (1)
98-104: IP mismatch check is ineffective when CN is disabled.The obtain flow passes
--disable-cn, socommonNameis always null. The AND condition short-circuits, skipping validation entirely. This was flagged in the previous review—please address by validating SubjectAltName (SAN) IP entries instead.packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js (1)
43-44: Honor explicitexpiration-days = 0.Using
||treats 0 as falsy. This was flagged in the previous review—use nullish coalescing (??) instead.
packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js
Show resolved
Hide resolved
packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js
Show resolved
Hide resolved
packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js
Outdated
Show resolved
Hide resolved
Contributor
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
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.
Add Let's Encrypt as a new SSL certificate provider option using the goacme/lego Docker client. This enables IP-based certificate issuance with automatic renewal support for shortlived certificates.
New files:
Changes:
Issue being fixed or feature implemented
What was done?
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Chores / Migrations
✏️ Tip: You can customize this high-level summary in your review settings.