fix(dstack-ingress): let RENEW_DAYS_BEFORE be unset again

[kvinwang]

RENEW_DAYS_BEFORE could be turned on but not off.

The value is written into certbot's lineage config (renew_before_expiry), which lives in the certificate volume and outlives the container. apply_renewal_window returned early when the variable was absent, so removing it from the environment left the previous value in place — the certificate stayed permanently due and every later pass performed a real renewal.

Its own docstring claimed the opposite:

Leaving it unset keeps certbot's own default (30 days).
...
days = os.environ.get("RENEW_DAYS_BEFORE", "").strip()
if not days:
    return          # ← the lineage keeps whatever was written last time

and both README and TESTING.md tell you to unset it once you have seen the renewal branch you came for.

What changed

• Clearing the variable removes the active renew_before_expiry line, so the client default applies again. certbot's own commented-out template is left alone.
• The config is only rewritten when it would actually change, so a quiet pass no longer touches the file.
• _renewal_conf_path() extracted so the rewrite can be tested without /etc/letsencrypt.
• README says what unsetting does.

Testing

New scripts/tests/test_certman.py, 9 cases: writing, replacing without duplicating, clearing, leaving the commented template alone, invalid values ignored, missing lineage, wildcard lineage naming, plus the same duplicate-definition guard test_dnsguide.py uses.

The regression case was checked both ways — it fails against the previous code and passes against this one:

FAIL: test_unsetting_removes_a_previously_written_value

How it turned up

Testing 2.3 (dstacktee/dstack-ingress:2.3). After exercising the forced-renewal branch with RENEW_DAYS_BEFORE=365, removing the variable left the lineage due, so the next passes renewed for real and two of them hit certbot's hard 300 s cap — which on dns-01 fails the bootstrap pass before haproxy serves anything. That cap is a separate, longer-standing problem; PR to follow.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[kvinwang]

Ran this through an independent review. Two findings taken, one declined with reasoning. Pushed as 813b8cf.

Taken — the config was rewritten non-atomically. open(path, "w") truncates the live lineage config before anything replaces it, so a crash or a full disk mid-write leaves certbot a config it cannot parse — and a lineage it cannot parse is one it cannot renew. The except OSError could only report that; the content was already gone. Now written to a temp file, fsynced, and os.replaced over the original.

Declined, documented instead — clearing is not ownership-aware. With the variable unset we drop any active renew_before_expiry, including one a human put there; nothing distinguishes the two. Adding an ownership marker would mean maintaining state purely to support a case this image does not create — the volume is container-managed. The docstring now states the invariant and the way out (set RENEW_DAYS_BEFORE to the value you want rather than leaving it unset).

Out of scope, filed separately — wildcards never reach this code. certificate_exists() checks /etc/letsencrypt/live/*.example.com/fullchain.pem, and os.path.isfile does not glob, while certbot stores the lineage under the bare name. Confirmed in certbot 5.7.0 itself:

# client.py _choose_lineagename
elif sans[0].is_wildcard():
    # Don't make files and directories starting with *.
    lineagename = str(sans[0])[2:]

So run_action("auto") always takes the obtain branch for a wildcard, apply_renewal_window is never called, and RENEW_DAYS_BEFORE has no effect on wildcard lineages at all. That predates this PR and is a bigger behaviour change than belongs here.

Tests: 11 cases, up from 9 — no scratch file left behind, and a write that cannot even create the temp file leaves the original byte-identical. Each case now gets its own directory, since one of them makes that directory unwritable.