fix(certificates): write traefik registration yml into the watched dynamic directory - #4970
fix(certificates): write traefik registration yml into the watched dynamic directory#4970Siumauricio wants to merge 1 commit into
Conversation
| for (const certificate of allCertificates) { | ||
| try { | ||
| await createCertificateFiles(certificate); | ||
| } catch (error) { |
There was a problem hiding this comment.
Sequential SSH migration blocks startup
If a self-hosted installation has certificates assigned to slow or unreachable remote servers, this loop serially awaits each SSH operation before cron jobs, schedules, cancellation recovery, volume backups, and restart notifications are initialized, causing those background services to remain inactive while the timeouts accumulate.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| await execAsyncRemote( | ||
| certificate.serverId, | ||
| `rm -rf ${quote([certDir])}; rm -f ${quote([configFile])}`, | ||
| ); |
There was a problem hiding this comment.
PEM deletion failure is masked
When the remote PEM directory cannot be removed but the registration file can, the semicolon makes the compound command return success and the database row is deleted, leaving the private key and certificate files orphaned while the UI reports successful deletion.
How this was verified: The remote executor uses the compound shell command's final exit status, so a successful second rm masks a failed first rm.
| await execAsyncRemote( | |
| certificate.serverId, | |
| `rm -rf ${quote([certDir])}; rm -f ${quote([configFile])}`, | |
| ); | |
| await execAsyncRemote( | |
| certificate.serverId, | |
| `rm -rf ${quote([certDir])} && rm -f ${quote([configFile])}`, | |
| ); |
Closes #4503
Related: #3762, #4707
Uploaded certificates were written as
dynamic/certificates/<cert-path>/certificate.yml, but Traefik's file provider does not read subdirectories of the configureddirectory— so the TLS store registration was never loaded and SNI matching never served uploaded certs (Cloudflare Origin CA being the most common casualty).Changes:
dynamic/<cert-path>.yml(top level, watched). The PEM files stay indynamic/certificates/<cert-path>/since they are referenced by absolute path.certificate.yml.migrateCertificatesToWatchedConfig()rewrites all stored certificates into the new layout, fixing existing installs without user action. Cloud-managed remote servers get migrated the next time the certificate is saved/edited.Greptile Summary
The PR relocates uploaded-certificate registration YAML files into Traefik's watched dynamic directory, removes legacy nested registrations, and adds a startup migration for existing certificates.
Confidence Score: 4/5
The PR should not merge until remote certificate migration can no longer delay background-service startup and remote deletion reliably fails when PEM cleanup fails.
Startup now serially awaits potentially slow SSH operations before initializing scheduled and recovery work, while remote deletion can mask a failed PEM-directory removal and discard the corresponding database record.
Files Needing Attention: packages/server/src/services/certificate.ts and apps/dokploy/server/server.ts
Security Review
Remote certificate deletion can report success while retaining private-key material if PEM-directory removal fails but the subsequent registration-file removal succeeds.
How this was verified: The compound remote command uses
;, and its caller deletes the database row based on the final command's successful exit status even when the first removal failed.Reviews (1): Last reviewed commit: "fix(certificates): write traefik registr..." | Re-trigger Greptile