You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
SAN batch resilience: eviction grace + shrink guard, renewal preflight for all members, deploy-path quarantine
Problem / Goal
The WM3 deployment feeds ~1100 tenant domains through tls-domains-source, and far from all of them are live or should be issued for. The issuance pipeline already defends well — a failed ACME order never touches an existing certificate (adoptCertificate runs only on success), per-domain quarantine with escalating backoff exists, never-issued domains are preflight-probed before joining an order, and renewal compacts a batch without its quarantined members near expiry. There is no Traefik-style "bad primary": a cert's identity is a hash of its sorted domain set, no member is privileged.
Three gaps remain where the Traefik failure mode ("one dead domain kills the whole SAN batch", or worse) can still occur:
Gap B (worst): estate wipe by a bad source poll.applyDomains replaces a service's domain set wholesale. One poll returning {"domains": []} (or a truncated list) evicts everything, and the next hourly renewer reconcile deletes the certificates and private keys for every cert with no remaining domains. When the source recovers, 1100 domains need reissuance — straight into Let's Encrypt rate limits, with handshake failures until certs return.
Gap A: renewal blind spot for dead domains. The preflight probe is skipped for domains that already hold a certificate (!HasCertificate(domain) guard). A departed customer whose DNS points elsewhere fails the renewal order at ACME. If lego's error names the domain, quarantine + compaction save the batch. If the error is not per-domain parseable (network error, account rate limit, generic failure), handleRenewalFailure quarantines nobody — so compaction never triggers and the doomed batch retries every reconcile until the whole certificate expires, taking every member down with it.
Gap C: deploy/handshake batch has no quarantine.provisionCertificate batches ALL pending deploy-registered hosts into one synchronous order. No preflight, no quarantine, no culprit-splitting: one typo'd --host fails the batch, everything goes back to pending, and the same doomed batch retries on every handshake (bounded only by the token bucket).
Done looks like: a bad poll can never delete a still-valid certificate; a dead domain can never sink a renewal order or cause unbounded retries; a bad deploy host can never starve certs for its batch siblings; make test green with table-driven coverage for each scenario.
Context (read these first)
internal/server/dynamic_domains.go — DynamicDomainManager: owns quarantine, issuer, renewer, per-service pollers. applyDomains (~line 322) is where a poll replaces the domain set — the shrink guard goes here. preflightProbe (~line 413) is the nonce self-probe (HTTP, 5s timeout, no redirects). NewDynamicDomainManager wires Preflight into both issuer and renewer configs.
internal/server/domain_renewal.go — certRenewer. reconcile (~line 136) removes certs with zero renewable domains — the grace period changes this. renew (~line 186) is where probe-all-members goes, before the order; note the existing defer-vs-compact logic around quarantineCompactionWindow (7 days). handleRenewalFailure (~line 328) is the asymmetric failure handler that can quarantine nobody. topUpBatch (~line 285) already probes never-issued top-ups — the pattern to extend.
internal/server/domain_issuer.go — handleObtainFailure (~line 372) falls back to quarantining ALL domains when failedDomainsFromError finds no culprits; the renewal path lacks this fallback. failedDomainsFromError (~line 416) parses lego's "<domain>: <cause>" lines.
internal/server/domain_quarantine.go — the quarantine: RecordFailure(domain, kind) with quarantineACME / quarantinePreflight ladders, Filter, Clear. Reuse it; do not invent a second mechanism.
internal/server/san_cert_manager.go — provisionCertificate (~line 473) is the deploy/handshake batch path (Gap C). adoptCertificate (~line 556) is success-only adoption. This file is 886 lines, already over the 800-line ceiling — new deploy-path guard code goes in a new file, not here.
internal/server/san_cert_dynamic.go — SetDynamicDomains, DomainAllowed, removeCertificate (deletes cert dir from disk), HasCertificate/HasValidCertificate.
internal/server/domain_source.go — parseDomainList validates entries (RFC 1123, no wildcards, 10k cap) but happily returns a legitimate empty list.
PR feat(cert-store): certificate store export/import for disaster recovery #95 (feat(cert-store): certificate store export/import for disaster recovery) — merge it first. It touches san_cert_manager.go and san_cert_dynamic.go (moves saveCertificate/adoptCertificate/removeCertificate under stateMu, staged tmp+rename writes). This plan's diff must be written on top of it, not in parallel.
Existing tests to extend: domain_renewal_test.go, domain_issuer_test.go, san_cert_manager_test.go; dynamic-domain manager tests live alongside dynamic_domains.go.
Decision
Three coordinated protections, all reusing the existing quarantine/preflight machinery rather than adding a backup/rollback layer:
Eviction grace + shrink guard (Gap B). Certificates are never deleted before their own NotAfter: an evicted domain simply stops renewing, and its cert keeps serving until natural expiry (GetCertificate already serves still-valid certs for evicted domains). Independently, applyDomains refuses to apply the removals from a poll that shrinks a service's set by more than a threshold, until consecutive polls confirm the shrink; additions always apply immediately.
Renewal preflight for all members + probe-based culprit identification (Gap A).renew probes every batch member before placing the order; unreachable members are quarantined (quarantinePreflight) and the existing defer/compact logic does the rest. On an order failure whose error names no domains, probe the members to find culprits; if probing identifies none, quarantine all members (matching the issuer's existing fallback) so retries ride the backoff ladder instead of looping hourly.
Deploy-path preflight + quarantine (Gap C).provisionCertificate gains the same treatment: batched pending domains (other than the handshake-triggering one) are probed and quarantine-filtered before the order; on failure, culprits are quarantined and only survivors return to pending.
Why not backup/restore-and-retry around every change: issuance is already transactional in the direction that matters — certs are adopted only on success, never deleted on failure, state files are tmp+rename atomic, and PR #95 covers disaster recovery for host loss. A pre-change snapshot would protect against nothing the above doesn't, while adding I/O on every order. Rejected.
Why not per-service flags for the shrink threshold: constants with sane defaults (30% shrink threshold, 3 confirming polls) keep the surface small; a flag can be added later if a real deployment needs different numbers. Rejected for now (avoid over-engineering).
Why thresholded hold instead of refusing empty lists only: a truncated-but-nonempty list (500 of 1100 domains) is the same bug with the same blast radius; a percentage threshold catches both.
Settled in interview:
Gap B: BOTH grace period (no cert deleted before its own expiry) AND shrink guard (thresholded hold with multi-poll confirmation). Not one or the other.
Gap A: probe ALL members before every renewal order (not only probe-on-failure). Probe-based culprit identification on unparseable failures is included.
Gap C: in scope for this plan, same treatment as the dynamic path.
Work on top of merged PR #95. RED → GREEN per step; table-driven tests, testify, no live ACME (fake obtainers as in existing domain_renewal_test.go).
1. Eviction grace period (internal/server/domain_renewal.go)
Tests first in domain_renewal_test.go: (a) a cert whose domains were all evicted is NOT removed while unexpired; (b) it IS removed once NotAfter has passed; (c) a superseded cert (domains remapped to a newer cert) likewise lingers until expiry and is then removed; (d) an evicted-but-unexpired cert is never renewed.
Change reconcile: when renewableDomains(cert) is empty, remove only if now is past cert.NotAfter; otherwise skip with a debug log. Remove the duplicate zero-domain removal branch at the top of renew (it becomes unreachable — reconcile no longer calls renew for empty sets).
Note in code why: a domain source outage must never destroy keys for still-valid certificates.
Constants: shrinkGuardThreshold = 0.30 (fraction of previous set removed in one poll that triggers the hold), shrinkGuardConfirmations = 3 (consecutive polls that must confirm the shrink before removals apply).
Tests first: (a) empty poll against a 100-domain set applies no removals and keeps the manager's dynamic set intact; (b) a poll removing 10% applies immediately; (c) a poll removing 50% holds removals, additions in the same poll still apply; (d) 3 consecutive confirming polls then apply the removals; (e) a recovering poll (domains return) cancels the hold and resets the counter; (f) the hold state is visible in Status().
In applyDomains: compute removed against the previous set (diffDomains already does). If len(previous) > 0 and len(removed) > shrinkGuardThreshold * len(previous): apply previous ∪ added to the manager instead of the polled set, track a per-service hold (pending removals + confirmation count) in serviceDomainState or a sibling in-memory map, log at Warn every held poll. When the same removals persist for shrinkGuardConfirmations consecutive polls, apply them. Memory-only hold state is fine: a restart reloads the last applied (unshrunk) set, which restarts the hold conservatively.
Surface the hold in Status() / DomainsServiceStatus so kamal-proxy domains shows held_removals + count, and operators can see a stuck source.
Keep dynamic_domains.go under the 800-line ceiling — if the guard logic pushes it, extract to internal/server/dynamic_domains_shrink.go.
3. Renewal preflight for all members (internal/server/domain_renewal.go)
Tests first: (a) an unreachable member is quarantined before the order and the order proceeds without it only inside the compaction window — outside it, renewal defers (existing semantics); (b) all-members-unreachable defers renewal and places no order; (c) reachable members probe clean and the order contains exactly them; (d) probe is skipped entirely when Preflight is nil; (e) wildcard identifiers (*.example.com) are never probed (nothing listens on a literal wildcard) — pass them through un-probed.
In renew, after quarantine.Filter and before topUpBatch: probe each remaining non-wildcard domain via r.config.Preflight; failures get RecordFailure(domain, quarantinePreflight) and drop from the order. Then re-apply the existing defer-vs-compact decision against the updated quarantine picture (a probe failure far from expiry defers the renewal, preserving today's "wait for recovery rather than shrink the set" behavior).
Remove the now-redundant !m.manager.HasCertificate(domain) probe guard in topUpBatch? No — keep it: top-ups are probed there before joining; members are probed in renew. Just ensure no domain is probed twice in one pass.
4. Probe-based culprit identification on unparseable failures (shared by issuer + renewer)
Tests first: (a) renewal failure with a lego error naming dead.example.com quarantines exactly it (existing behavior, keep green); (b) renewal failure with an unparseable error probes members, quarantines the unreachable ones; (c) unparseable error + all probes pass ⇒ quarantine all members (quarantineACME), matching handleObtainFailure's fallback; (d) same for the issuer path, replacing its blanket fallback with probe-first.
Extract a helper (new file internal/server/domain_failure.go or alongside failedDomainsFromError in domain_issuer.go if it stays small): identifyFailedDomains(err, domains, preflight) []string — parse first, probe second, all as last resort. Use it from both handleObtainFailure and handleRenewalFailure. This also fixes the renewal path's quarantine-nobody hole directly.
5. Deploy-path preflight + quarantine (Gap C) — new file internal/server/san_cert_batch_guard.go (do NOT grow san_cert_manager.go, it is over the ceiling)
Wiring: SANCertManager gains optional hooks (e.g. SetIssuanceGuard(preflight func(string) error, quarantine *domainQuarantine)) installed by NewDynamicDomainManager next to the existing SetDynamicCertRequester call. Verify DynamicDomainManager is constructed unconditionally at boot (router/run wiring) — if it is only built when domain sources exist, construct it unconditionally so deploy-path protection does not depend on a dynamic-domains flag.
Tests first in san_cert_manager_test.go (or a new san_cert_batch_guard_test.go): (a) a pending domain failing preflight is left out of the handshake batch and quarantined, the rest issue; (b) the handshake-triggering domain itself is never dropped by the guard (it gets its shot; ACME failure then quarantines it via step 4's helper); (c) a quarantined pending domain is skipped without probing; (d) on order failure, culprits are quarantined and only survivors return to pending; (e) nil hooks (guard not installed) preserve today's behavior exactly.
In provisionCertificate: filter the collected pending domains (not the trigger) through quarantine + preflight before planIssuanceDomains; on obtain failure, run identifyFailedDomains, quarantine culprits, restorePending only survivors.
6. Docs + race pass
README: short section under the existing TLS docs on the shrink guard and eviction grace (operator-visible behavior: domains status shows held removals; certs for departed domains persist until expiry).
go test -race ./internal/server/ — this touches renewer/issuer/manager shared state.
gofmt -l internal/ cmd/, make lint, make test.
Verification gates
make test — all green (go test ./...)
go test -race ./internal/server/ — clean (renewer/issuer/quarantine are cross-goroutine)
gofmt -l internal/ cmd/ — empty output
make lint — 0 issues (golangci-lint v2.11.3 per ci.yml)
make build — bin/kamal-proxy builds clean
make docker && docker run --rm kamal-proxy kamal-proxy -h — image smoke test (cert-manager surface touched)
Scenario check (unit-level, no live ACME): simulate the WM3 wipe — 100 dynamic domains with certs, one empty poll, advance reconcile ⇒ zero certs deleted, hold visible in status, recovery poll restores the set with zero new orders.
NO edits to Dockerfile/Makefile/script/release (upstream's); script/release-dash untouched.
NO renaming of the kamal-proxy module/binary/RPC service/socket.
NO commits on main; nothing lands there.
NO changes to the wildcard collapse logic (planIssuanceDomains) or the solver-selection strategy (obtainCertificate).
NO gem-side (../kamal) changes — this is entirely proxy-internal behavior with no new flags to plumb.
Execution
Merge PR #95 into dash first. Then branch feature/san-batch-resilience off dash, implement steps 1–6 in order (each is independently green), PR against dash. Steps 1+2 (Gap B) are the highest-value slice if the work needs to ship in two PRs.
SAN batch resilience: eviction grace + shrink guard, renewal preflight for all members, deploy-path quarantine
Problem / Goal
The WM3 deployment feeds ~1100 tenant domains through
tls-domains-source, and far from all of them are live or should be issued for. The issuance pipeline already defends well — a failed ACME order never touches an existing certificate (adoptCertificateruns only on success), per-domain quarantine with escalating backoff exists, never-issued domains are preflight-probed before joining an order, and renewal compacts a batch without its quarantined members near expiry. There is no Traefik-style "bad primary": a cert's identity is a hash of its sorted domain set, no member is privileged.Three gaps remain where the Traefik failure mode ("one dead domain kills the whole SAN batch", or worse) can still occur:
applyDomainsreplaces a service's domain set wholesale. One poll returning{"domains": []}(or a truncated list) evicts everything, and the next hourly renewer reconcile deletes the certificates and private keys for every cert with no remaining domains. When the source recovers, 1100 domains need reissuance — straight into Let's Encrypt rate limits, with handshake failures until certs return.!HasCertificate(domain)guard). A departed customer whose DNS points elsewhere fails the renewal order at ACME. If lego's error names the domain, quarantine + compaction save the batch. If the error is not per-domain parseable (network error, account rate limit, generic failure),handleRenewalFailurequarantines nobody — so compaction never triggers and the doomed batch retries every reconcile until the whole certificate expires, taking every member down with it.provisionCertificatebatches ALL pending deploy-registered hosts into one synchronous order. No preflight, no quarantine, no culprit-splitting: one typo'd--hostfails the batch, everything goes back to pending, and the same doomed batch retries on every handshake (bounded only by the token bucket).Done looks like: a bad poll can never delete a still-valid certificate; a dead domain can never sink a renewal order or cause unbounded retries; a bad deploy host can never starve certs for its batch siblings;
make testgreen with table-driven coverage for each scenario.Context (read these first)
internal/server/dynamic_domains.go—DynamicDomainManager: owns quarantine, issuer, renewer, per-service pollers.applyDomains(~line 322) is where a poll replaces the domain set — the shrink guard goes here.preflightProbe(~line 413) is the nonce self-probe (HTTP, 5s timeout, no redirects).NewDynamicDomainManagerwiresPreflightinto both issuer and renewer configs.internal/server/domain_renewal.go—certRenewer.reconcile(~line 136) removes certs with zero renewable domains — the grace period changes this.renew(~line 186) is where probe-all-members goes, before the order; note the existing defer-vs-compact logic aroundquarantineCompactionWindow(7 days).handleRenewalFailure(~line 328) is the asymmetric failure handler that can quarantine nobody.topUpBatch(~line 285) already probes never-issued top-ups — the pattern to extend.internal/server/domain_issuer.go—handleObtainFailure(~line 372) falls back to quarantining ALL domains whenfailedDomainsFromErrorfinds no culprits; the renewal path lacks this fallback.failedDomainsFromError(~line 416) parses lego's"<domain>: <cause>"lines.internal/server/domain_quarantine.go— the quarantine:RecordFailure(domain, kind)withquarantineACME/quarantinePreflightladders,Filter,Clear. Reuse it; do not invent a second mechanism.internal/server/san_cert_manager.go—provisionCertificate(~line 473) is the deploy/handshake batch path (Gap C).adoptCertificate(~line 556) is success-only adoption. This file is 886 lines, already over the 800-line ceiling — new deploy-path guard code goes in a new file, not here.internal/server/san_cert_dynamic.go—SetDynamicDomains,DomainAllowed,removeCertificate(deletes cert dir from disk),HasCertificate/HasValidCertificate.internal/server/domain_source.go—parseDomainListvalidates entries (RFC 1123, no wildcards, 10k cap) but happily returns a legitimate empty list.feat(cert-store): certificate store export/import for disaster recovery) — merge it first. It touchessan_cert_manager.goandsan_cert_dynamic.go(movessaveCertificate/adoptCertificate/removeCertificateunderstateMu, staged tmp+rename writes). This plan's diff must be written on top of it, not in parallel.domain_renewal_test.go,domain_issuer_test.go,san_cert_manager_test.go; dynamic-domain manager tests live alongsidedynamic_domains.go.Decision
Three coordinated protections, all reusing the existing quarantine/preflight machinery rather than adding a backup/rollback layer:
NotAfter: an evicted domain simply stops renewing, and its cert keeps serving until natural expiry (GetCertificate already serves still-valid certs for evicted domains). Independently,applyDomainsrefuses to apply the removals from a poll that shrinks a service's set by more than a threshold, until consecutive polls confirm the shrink; additions always apply immediately.renewprobes every batch member before placing the order; unreachable members are quarantined (quarantinePreflight) and the existing defer/compact logic does the rest. On an order failure whose error names no domains, probe the members to find culprits; if probing identifies none, quarantine all members (matching the issuer's existing fallback) so retries ride the backoff ladder instead of looping hourly.provisionCertificategains the same treatment: batched pending domains (other than the handshake-triggering one) are probed and quarantine-filtered before the order; on failure, culprits are quarantined and only survivors return to pending.Why not backup/restore-and-retry around every change: issuance is already transactional in the direction that matters — certs are adopted only on success, never deleted on failure, state files are tmp+rename atomic, and PR #95 covers disaster recovery for host loss. A pre-change snapshot would protect against nothing the above doesn't, while adding I/O on every order. Rejected.
Why not per-service flags for the shrink threshold: constants with sane defaults (30% shrink threshold, 3 confirming polls) keep the surface small; a flag can be added later if a real deployment needs different numbers. Rejected for now (avoid over-engineering).
Why thresholded hold instead of refusing empty lists only: a truncated-but-nonempty list (500 of 1100 domains) is the same bug with the same blast radius; a percentage threshold catches both.
Settled in interview:
Implementation steps
Work on top of merged PR #95. RED → GREEN per step; table-driven tests,
testify, no live ACME (fake obtainers as in existingdomain_renewal_test.go).1. Eviction grace period (
internal/server/domain_renewal.go)domain_renewal_test.go: (a) a cert whose domains were all evicted is NOT removed while unexpired; (b) it IS removed onceNotAfterhas passed; (c) a superseded cert (domains remapped to a newer cert) likewise lingers until expiry and is then removed; (d) an evicted-but-unexpired cert is never renewed.reconcile: whenrenewableDomains(cert)is empty, remove only ifnowis pastcert.NotAfter; otherwise skip with a debug log. Remove the duplicate zero-domain removal branch at the top ofrenew(it becomes unreachable —reconcileno longer callsrenewfor empty sets).2. Shrink guard (
internal/server/dynamic_domains.go)shrinkGuardThreshold = 0.30(fraction of previous set removed in one poll that triggers the hold),shrinkGuardConfirmations = 3(consecutive polls that must confirm the shrink before removals apply).Status().applyDomains: computeremovedagainst the previous set (diffDomains already does). Iflen(previous) > 0andlen(removed) > shrinkGuardThreshold * len(previous): applyprevious ∪ addedto the manager instead of the polled set, track a per-service hold (pending removals + confirmation count) inserviceDomainStateor a sibling in-memory map, log at Warn every held poll. When the same removals persist forshrinkGuardConfirmationsconsecutive polls, apply them. Memory-only hold state is fine: a restart reloads the last applied (unshrunk) set, which restarts the hold conservatively.Status()/DomainsServiceStatussokamal-proxy domainsshowsheld_removals+ count, and operators can see a stuck source.dynamic_domains.gounder the 800-line ceiling — if the guard logic pushes it, extract tointernal/server/dynamic_domains_shrink.go.3. Renewal preflight for all members (
internal/server/domain_renewal.go)Preflightis nil; (e) wildcard identifiers (*.example.com) are never probed (nothing listens on a literal wildcard) — pass them through un-probed.renew, afterquarantine.Filterand beforetopUpBatch: probe each remaining non-wildcard domain viar.config.Preflight; failures getRecordFailure(domain, quarantinePreflight)and drop from the order. Then re-apply the existing defer-vs-compact decision against the updated quarantine picture (a probe failure far from expiry defers the renewal, preserving today's "wait for recovery rather than shrink the set" behavior).!m.manager.HasCertificate(domain)probe guard intopUpBatch? No — keep it: top-ups are probed there before joining; members are probed inrenew. Just ensure no domain is probed twice in one pass.4. Probe-based culprit identification on unparseable failures (shared by issuer + renewer)
dead.example.comquarantines exactly it (existing behavior, keep green); (b) renewal failure with an unparseable error probes members, quarantines the unreachable ones; (c) unparseable error + all probes pass ⇒ quarantine all members (quarantineACME), matchinghandleObtainFailure's fallback; (d) same for the issuer path, replacing its blanket fallback with probe-first.internal/server/domain_failure.goor alongsidefailedDomainsFromErrorindomain_issuer.goif it stays small):identifyFailedDomains(err, domains, preflight) []string— parse first, probe second, all as last resort. Use it from bothhandleObtainFailureandhandleRenewalFailure. This also fixes the renewal path's quarantine-nobody hole directly.5. Deploy-path preflight + quarantine (Gap C) — new file
internal/server/san_cert_batch_guard.go(do NOT growsan_cert_manager.go, it is over the ceiling)SANCertManagergains optional hooks (e.g.SetIssuanceGuard(preflight func(string) error, quarantine *domainQuarantine)) installed byNewDynamicDomainManagernext to the existingSetDynamicCertRequestercall. VerifyDynamicDomainManageris constructed unconditionally at boot (router/run wiring) — if it is only built when domain sources exist, construct it unconditionally so deploy-path protection does not depend on a dynamic-domains flag.san_cert_manager_test.go(or a newsan_cert_batch_guard_test.go): (a) a pending domain failing preflight is left out of the handshake batch and quarantined, the rest issue; (b) the handshake-triggering domain itself is never dropped by the guard (it gets its shot; ACME failure then quarantines it via step 4's helper); (c) a quarantined pending domain is skipped without probing; (d) on order failure, culprits are quarantined and only survivors return to pending; (e) nil hooks (guard not installed) preserve today's behavior exactly.provisionCertificate: filter the collected pending domains (not the trigger) through quarantine + preflight beforeplanIssuanceDomains; on obtain failure, runidentifyFailedDomains, quarantine culprits,restorePendingonly survivors.6. Docs + race pass
domainsstatus shows held removals; certs for departed domains persist until expiry).go test -race ./internal/server/— this touches renewer/issuer/manager shared state.gofmt -l internal/ cmd/,make lint,make test.Verification gates
make test— all green (go test ./...)go test -race ./internal/server/— clean (renewer/issuer/quarantine are cross-goroutine)gofmt -l internal/ cmd/— empty outputmake lint— 0 issues (golangci-lint v2.11.3 per ci.yml)make build—bin/kamal-proxybuilds cleanmake docker && docker run --rm kamal-proxy kamal-proxy -h— image smoke test (cert-manager surface touched)Out of scope
script/release(upstream's);script/release-dashuntouched.kamal-proxymodule/binary/RPC service/socket.main; nothing lands there.planIssuanceDomains) or the solver-selection strategy (obtainCertificate).../kamal) changes — this is entirely proxy-internal behavior with no new flags to plumb.Execution
Merge PR #95 into
dashfirst. Then branchfeature/san-batch-resilienceoffdash, implement steps 1–6 in order (each is independently green), PR againstdash. Steps 1+2 (Gap B) are the highest-value slice if the work needs to ship in two PRs.