fix: keep valid cert symlinks until docker-gen generates its data

[JamBalaya56562]

What

Stop the companion from briefly serving the default certificate (and breaking clients such as Nextcloud sync) every time it is recreated. Closes #956.

Root cause

app/start.sh launches letsencrypt_service and docker-gen in parallel. letsencrypt_service runs update_certs immediately, but at that point docker-gen has usually not yet generated /app/letsencrypt_service_data (the issue thread shows the tell-tale Warning: /app/letsencrypt_service_data not found).

With no data file, update_certs proceeds with an empty set of enabled domains and reaches cleanup_links. There, every existing *.crt symlink is classified as "disabled" (no enabled domain matches it) and, because each managed cert directory contains a .companion marker, all of them are deleted. docker-gen then generates the data, the loop re-runs, and the symlinks are recreated one domain at a time via acme.sh (RENEW_SKIP) + create_links — during which nginx serves the default certificate.

This only requires nginx-proxy to already be up while the companion restarts (the check_nginx_proxy_container_run gate passes), which is exactly the "recreate the companion on image update" scenario from the report.

Fix

Return early from update_certs while neither /app/letsencrypt_service_data nor /app/letsencrypt_user_data exists, before any path that can reach cleanup_links. The existing symlinks are left untouched, so nginx keeps serving the real certificates; the loop is re-triggered via SIGUSR1 (signal_le_service) as soon as docker-gen writes the data file, and verification/renewal then happens normally.

The guard keys on file existence, not on the container count: docker-gen always writes the data file even with zero proxied containers, so the legitimate "all containers removed" cleanup still runs.

Test

New deterministic certs_persistence integration test (registered in test/config.sh and the CI matrix, runs under 2containers and 3containers): it issues a cert, removes /app/letsencrypt_service_data, runs update_certs synchronously (via the script'''s --source-only mode), and asserts the symlink is still present.

Verified locally

With a managed cert + symlinks in place and the data files removed: update_certs hits the new guard and the symlink is preserved, whereas calling the unguarded cleanup_links directly (the old reachable behavior) removes it — confirming both the mechanism and the fix.

Out of scope (deliberately)

Migrating only the /etc/nginx/certs/<domain>/ directories without their top-level *.crt symlinks: there is no symlink to preserve, so issuance recreates it on the next loop as today. A pre-validation relink pass could close that too but is left as a follow-up to keep this change minimal.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR prevents nginx-proxy from briefly serving the default certificate during acme-companion restarts by avoiding deletion of existing managed *.crt symlinks until docker-gen has generated /app/letsencrypt_service_data (fixing #956).

Changes:

• Gate cleanup_links in update_certs on the existence of /app/letsencrypt_service_data to avoid pruning symlinks before docker-gen’s first render.
• Add a new deterministic integration test (certs_persistence) to ensure symlinks survive a data-less update_certs run.
• Register the new test in the test suite config and CI matrix.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
app/letsencrypt_service	Skips cleanup_links unless docker-gen service data exists, preventing premature symlink deletion on startup/recreate.
test/tests/certs_persistence/run.sh	New integration test that asserts symlink persistence when service/user data files are missing.
test/config.sh	Registers certs_persistence in the global integration test list.
.github/workflows/test.yml	Adds certs_persistence to the GitHub Actions integration test matrix.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[buchdag]

The PR seems to break the renew_private_keys test.

[JamBalaya56562]

This looks like the flaky renew_private_keys test rather than a regression from this PR:

• The failure is purely timing: Certificate for le1.wtf was not renewed within 30s (not a functional error such as a changed/unreadable key).
• This PR only gates cleanup_links on [[ -f /app/letsencrypt_service_data ]]. In the 2containers renew_private_keys setup docker-gen always writes that file, so the gate is a no-op there and the renewal path (update_cert → acme.sh --issue) is unchanged.
• The same renewal code is green on fix: handle multiple nginx-proxy replicas sharing a label #1275 (unrelated to cert symlinks); renew_private_keys also failed there on an earlier run before turning green, which is the flaky pattern.

Root cause: renew_private_keys polls the renewal for only 30s while the rest of the suite waits 120s (wait_for_symlink, container_restart, etc.), so it occasionally times out under CI load — a re-run should make it pass.

To fix the flakiness for good I'd suggest bumping that poll from 30s to 120s to align with the rest of the suite. Happy to do it — would you prefer it in this PR, or as a separate follow-up so this one stays focused on #956?

[buchdag]

The test does not seem flaky, it was run five times in a row in CI and failed each time.

Edit : it passed on the sixth try 🤷

[buchdag]

@JamBalaya56562 going forward please dot not remove mentions that those PRs where made using Claude Code from either the PR description or the commit co-authors.