Skip to content

Conversation

@ktechmidas
Copy link
Collaborator

@ktechmidas ktechmidas commented Jan 25, 2026

Issue being fixed or feature implemented

Fixes several issues discovered during testing of the Let's Encrypt SSL provider (#3000):

  1. The --force flag didn't work because config values (email, externalIp) weren't loaded when the validation task was skipped
  2. Certificates couldn't be parsed by node-forge because lego generates ECDSA keys by default and node-forge only supports RSA
  3. dashmate doctor didn't recognize the letsencrypt SSL provider, causing a config error

What was done?

  • Added "Initialize configuration" task that always runs (even with --force) to load email, IP, and directory paths
  • Added --key-type rsa2048 flag to lego command for node-forge compatibility
  • Added letsencrypt case to collectSamplesTaskFactory.js for doctor SSL sample collection
  • Added Let's Encrypt error descriptions to analyseConfigFactory.js for doctor diagnostics
  • Renamed ERRORS import to ZEROSSL_ERRORS and added LETSENCRYPT_ERRORS import to avoid collision

How Has This Been Tested?

Breaking Changes

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • New Features
    • Added Let's Encrypt as an alternative SSL certificate provider alongside ZeroSSL
    • Implemented enhanced validation for required certificate configuration fields (email, external IP)
    • Support for RSA 2048-bit key generation to improve compatibility
    • Improved error handling with clearer guidance for certificate-related issues

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions github-actions bot added this to the v3.1.0 milestone Jan 25, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 25, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

Adds Let's Encrypt support across certificate analysis, sample collection, and obtain/renew tasks: provider-specific error mappings, a LetsEncrypt validation path in sample collection, and initialization plus rsa2048 key handling in the obtain/renew task.

Changes

Cohort / File(s) Summary
Doctor analysis
packages/dashmate/src/doctor/analyse/analyseConfigFactory.js
Replace single ERRORS import with LETSENCRYPT_ERRORS and ZEROSSL_ERRORS; update SSL error lookups to provider-specific mappings; add new Let's Encrypt error cases and messages.
Sample collection
packages/dashmate/src/listr/tasks/doctor/collectSamplesTaskFactory.js
Add validateLetsEncryptCertificate parameter and import LegoCertificate; add 'letsencrypt' gateway SSL branch that validates via LetsEncrypt validator, obfuscates results, and records via ctx.samples.setServiceInfo.
Let's Encrypt obtain/renew task
packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js
Add explicit initialization step to load/validate config, create lego directories, and derive paths; merge ctx values without overwriting existing ones; add --key-type rsa2048 to lego command arguments; adjust control flow to preserve pre-set ctx values.
Manifests
manifest_file, package.json
Updated manifest/package entries (lines changed).

Sequence Diagram(s)

sequenceDiagram
  participant Collector as CollectSamplesTask
  participant Validator as validateLetsEncryptCertificate
  participant Lego as LegoCertificate
  participant Store as ctx.samples.setServiceInfo

  Collector->>Validator: validateLetsEncryptCertificate(config, EXPIRATION_LIMIT_DAYS)
  Validator-->>Collector: { error, data }
  Collector->>Lego: obfuscate string fields in data (using Lego rules)
  Collector->>Store: setServiceInfo('gateway','ssl',{ error, data })
Loading
sequenceDiagram
  participant Task as obtainLetsEncryptCertificateTask
  participant FS as Filesystem (dirs)
  participant LegoCmd as lego CLI
  participant Cert as certificate files

  Task->>FS: load config (email, external IP, dirs), validate required fields
  Task->>FS: create lego directories if missing
  Task->>LegoCmd: run lego with args (include --key-type rsa2048)
  LegoCmd-->>Cert: write certificate and key files
  Task->>Task: check certificate validity / set flags (certificateValid,isRenewal)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Hooray — I hopped through lines of code today,
I found new paths where certs can safely play.
Let's Encrypt and ZeroSSL hand-in-paw,
Secure and snug beneath the meadow's awn. 🌿🔐

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately summarizes the primary changes: fixes to Let's Encrypt renewal and dashmate doctor functionality, which aligns with the substantial changes across three files addressing certificate renewal initialization, command configuration, and diagnostic improvements.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@packages/dashmate/src/doctor/analyse/analyseConfigFactory.js`:
- Around line 131-159: The two description strings for the LETSENCRYPT_ERRORS
entries use double quotes instead of single quotes; update the description
values for LETSENCRYPT_ERRORS.EMAIL_IS_NOT_SET and
LETSENCRYPT_ERRORS.CERTIFICATE_NOT_FOUND to use single-quoted plain strings
(keep all chalk`` template usages unchanged), so the LETSENCRYPT_ERRORS mapping
matches the project's single-quote style and ESLint rules.

In
`@packages/dashmate/src/listr/tasks/ssl/letsencrypt/obtainLetsEncryptCertificateTaskFactory.js`:
- Around line 79-80: The current merge using Object.assign(ctx, data) in
obtainLetsEncryptCertificateTaskFactory will overwrite pre-set ctx fields (e.g.,
ctx.email, ctx.externalIp, path fields); change the merge logic to only set
fields that are missing on ctx (e.g., for each key in data, if ctx[key] ===
undefined or null then set ctx[key] = data[key]) so existing values are
preserved, or update the comment to reflect that overwrites are intended; target
the Object.assign usage and the ctx/data merge in
obtainLetsEncryptCertificateTaskFactory.

@ktechmidas ktechmidas changed the title Letsencrypt renewal and dashmate doctor fixes fix(dashmate): Letsencrypt renewal and dashmate doctor fixes Jan 26, 2026
@ktechmidas ktechmidas requested a review from shumkov January 26, 2026 04:53
@ktechmidas ktechmidas changed the title fix(dashmate): Letsencrypt renewal and dashmate doctor fixes fix(dashmate): letsencrypt renewal and dashmate doctor fixes Jan 26, 2026
shumkov
shumkov previously approved these changes Jan 26, 2026
@shumkov shumkov changed the base branch from v3.1-dev to master January 26, 2026 05:11
@shumkov shumkov dismissed their stale review January 26, 2026 05:11

The base branch was changed.

@ktechmidas ktechmidas force-pushed the fix/letsencrypt_renewal branch from b29ad46 to 8009d72 Compare January 26, 2026 05:39
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.

3 participants