Skip to content

fix(traefik): let users remove unused Let's Encrypt certificates - #5883

Closed
imrja8 wants to merge 7 commits into
Dokploy:canaryfrom
imrja8:fix/4894-unused-le-certificates
Closed

imrja8 wants to merge 7 commits into
Dokploy:canaryfrom
imrja8:fix/4894-unused-le-certificates

Conversation

@imrja8

@imrja8 imrja8 commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

What is this PR about?

Closes #4894.

On canary, deleting a domain, app, compose or preview leaves its Let's Encrypt certificate in acme.json. Traefik keeps trying to renew every certificate in that file, so certificates for removed domains fail renewal every day and fill Traefik's logs.

This PR adds Traefik → Clean Up Certificates (main server and remote servers) to remove the certificates nothing uses.

How it works

  • Listing: a Let's Encrypt certificate is listed only if none of its names (main + SANs) matches anything in use on that server:

    • Dokploy's domains for that server, and the panel domain on the main server
    • Traefik's live routes (/api/rawdata), including HostRegexp
    • router labels of all containers (stopped ones included) and Swarm services
    • hostnames in Traefik's config files

    If any of these can't be read, the dialog warns. Users untick any certificate they want to keep.

  • Removing: Dokploy re-checks the selection, backs up acme.json (the last 5 in /etc/dokploy/traefik/acme-backups/) and writes the new file. It then restarts Traefik with a single docker restart, because Traefik only reads acme.json on start. A Traefik that was stopped stays stopped, and nothing is restarted if nothing can be removed.

  • If something goes wrong: if Traefik doesn't stay up, the original file is put back and Traefik is restarted. If Traefik saved its certificates back during the restart, Dokploy tries once more.

  • Runs in the same per-server queue as Traefik recreates.

  • Delete-domain dialog: for Let's Encrypt domains, a note that the certificate is kept and how to remove it.

Tests

  • unused-certificates.test.ts: matching, removal, backups, undo and the retry; local via memfs, remote via mocked SSH, with real certificate fixtures.
  • restart-traefik.test.ts: the restart helper and the queue.
  • api/traefik-certificates.test.ts: the lock, progress, the removed count and the audit entry.
  • pnpm typecheck and Biome pass. pnpm test passes except application.real.test.ts, which needs nixpacks installed locally.

Limits: the standalone dokploy-traefik container (the default), the default acme.json path and the default API port.

Checklist

Before submitting this PR, please make sure that:

  • You created a dedicated branch based on the canary branch.
  • You have read the suggestions in the CONTRIBUTING.md file https://github.com/Dokploy/dokploy/blob/canary/CONTRIBUTING.md#pull-request
  • You have tested this PR in your local instance. If you have not tested it yet, please do so before submitting. This helps avoid wasting maintainers' time reviewing code that has not been verified by you.

Issues related (if applicable)

Closes #4894

Screenshots (if applicable)

Screenshot 2026-11-06 at 10 28 24

RetriggerView in GreptileConfidence Score: 3/5

The PR needs changes before merging because cleanup can overwrite certificate data written by Traefik or another Dokploy instance.

Summary

Adds certificate cleanup for local and remote Traefik servers.

  • Admins can review and remove unused certificates from a Traefik server.
  • The domain deletion prompt explains where its saved certificate can be removed.

Reviews (5) · Last reviewed commit: "fix(traefik): simplify certificate clean..." · Reviewed by Greptile

@imrja8
imrja8 requested a review from Siumauricio as a code owner November 5, 2026 16:20
@imrja8
imrja8 force-pushed the fix/4894-unused-le-certificates branch from a040f3a to 0c77d72 Compare November 5, 2026 16:27
Comment on lines +671 to +673
await backupAcmeContent(serverId, original);
await writeAcmeContent(serverId, `${JSON.stringify(storage, null, 2)}\n`);
await restartWithUndo(serverId, () => writeAcmeContent(serverId, original));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Cleanup overwrites newly saved certificates

removeOnce replaces acme.json while Traefik is still running. If Traefik saves a newly issued or renewed certificate between readAcmeContent and writeAcmeContent, cleanup overwrites it with the older snapshot. The SSH backup adds another network round trip to this window.

The retry checks only whether selected certificates returned, so it cannot detect a lost certificate for another domain. After restarting, Traefik must obtain that certificate again, which can break HTTPS if issuance fails. Exclude Traefik's writes while reading and replacing its storage.

Comment on lines +685 to +686
) =>
queueTraefikTask(serverId, async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Cloud cleanup jobs collide

Replacing whileTraefikStopped with queueTraefikTask removes the shared Docker marker that blocked cleanup from another Dokploy instance. If multiple Dokploy instances serve cloud requests, as imrja8 described, two requests can now edit the same remote server at once. Both remaining locks are in-memory maps.

The jobs also share acme.json.tmp. One can overwrite the other's changes or rename the file while the other is still writing it, leaving invalid certificate storage that Traefik cannot load. Keep a server-wide claim around the entire read, write, and restart sequence.

@imrja8 imrja8 closed this Nov 6, 2026
@imrja8
imrja8 deleted the fix/4894-unused-le-certificates branch November 6, 2026 05:47
@imrja8

imrja8 commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Will create a fresh PR, it got too wide..

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.

acme.json file is not cleaned up

1 participant