feat(san-cert): decouple services in handshake batches, bound renewal deferral

[mhenrixon]

Summary

Three independent hardenings from the SAN grouping / primary-domain audit, closing the structural half of it (#101/#103 closed the availability half):

• internal/server/san_cert_manager.go — handshake batches are partitioned by service, exactly as the dynamic issuer always was (and by directory since fix(san-cert): honor per-service --tls-staging in the SAN cert manager #100): a certificate never spans services, so an operator's primary domain cannot spend its whole life coupled to hosts another service controls. Membership is decided by the authoritative owner map (registeredDomains), not the pending map, whose entries restored from failed batches carry no service name.
• internal/server/domain_renewal.go — certificates covering a deploy-registered host compact away quarantined/unreachable members 14 days before expiry instead of 7 (registeredQuarantineCompactionWindow). Tenant domains are individually expendable; the operator's own names are not, and a flapping batch-mate must no longer push their renewal into the final week. Tenant-only certificates keep the tight 7-day window — and the identical-set renewal exemption it preserves.
• internal/metrics/metrics.go + domain_renewal.go — deferred renewals are now a gauge, kamal_proxy_certificate_renewals_deferred, set on every reconcile (all four deferral paths report; a completed or failed order clears). Operators can alert on the coupling before the compaction window instead of discovering it during an outage.

Not included, per the issue's own note: the cosmetic CN reorder (browsers ignore CN; optional).

Closes #102

Test plan

• TestSANCertManager_HandshakeBatchNeverMixesServices — same-service mates batch, other services keep their pending slots and order separately
• TestCertRenewer_RegisteredCertCompactsEarlierThanDynamic — registered cert compacts at 10d, still defers at 20d; dynamic-only cert still defers at 10d
• TestCertRenewer_ReportsDeferredRenewals — gauge reports 1 while deferred, clears to 0 once the member recovers and the renewal lands
• gofmt/go vet clean, make test green, go test -race ./internal/server/ clean, make lint 0 issues, make build clean

Deviations & judgment calls

• 14 days, not 21. The issue offered 14–21; 14 doubles the recovery margin while keeping deferral (and the renewal-exemption preservation it exists for) meaningful. It is a constant, trivially revisited.
• The compaction window is decided by the certificate's renewable membership (compactionWindowFor(allowed)): one registered domain anywhere in the set gives the whole certificate the wider window — mixed legacy certs err on the side of the operator's names.
• The gauge counts certificates, not domains, and is set every reconcile from scratch — no increment/decrement bookkeeping to drift. Deferral is reported from all four defer paths in renew() (all-quarantined, partial-quarantine wait, unreachable wait, all-preflight-failed); a placed order — even one that then fails at the CA — counts as not deferred, because the failure paths have their own signal (IncCertificateRenewals(domain, false)).
• One overlap to know about: the per-service partition further shrinks handshake batch sizes, which compounds with fix(san-cert): serve the held certificate while its replacement issues #103's single-domain failsafe replacement path. Both are deliberate trades of batching efficiency for blast-radius isolation; the renewer remains the whole-batch mechanism.

---

Summary by cubic

Partitions SAN handshake batches by service, widens the renewal compaction window for certificates with registered domains, and exposes deferred renewals as a gauge. Previously, batches could mix services, all certs compacted 7 days before expiry, and deferred renewals were only logged.

• Old vs new behavior:
  • Handshake batching: previously could include other services if directories matched; now batches only include same-service domains using the authoritative registeredDomains owner map. Expect smaller, safer batches.
  • Renewal compaction: previously 7 days for all certs; now 14 days for any certificate that includes a deploy-registered domain, while tenant-only certs remain at 7 days. This may forfeit identical-set renewal in some registered cases to avoid last-week deferrals.
  • Metrics: adds kamal_proxy_certificate_renewals_deferred set during each reconcile and cleared when an order is placed or fails, enabling proactive alerting on coupling. Addresses Decouple deploy-registered domains from cross-service SAN batches and bound renewal deferral #102 requirements.

Review notes

• In provisionCertificate, confirm pending selection filters by both directory and registeredDomains service owner; pending entries restored from failed batches carry no service, so the owner map is authoritative.
• In domain_renewal, verify compactionWindowFor uses isRegisteredDomain to choose 14d vs 7d and that all defer paths return and count toward the gauge.
• In metrics, the tracker interface gains SetDeferredRenewals; Prometheus registers the kamal_proxy_certificate_renewals_deferred gauge.

Rollout

• Add alerting on kamal_proxy_certificate_renewals_deferred. No config or data migration required.

^{Written for commit dbd065e. Summary will update on new commits.}

[cubic-dev-ai]

3 issues found across 7 files

Confidence score: 3/5

• In internal/server/san_cert_manager.go, the shared _batch_ waiter can be released by an unrelated concurrent order while this service’s domain was filtered out of that in-flight batch, so callers may think provisioning completed when their domain is still pending. Tighten the batch/wait handshake so waiters are keyed to the specific domains/order they depend on.
• In internal/server/domain_renewal.go (compactionWindowFor), wildcard-covered deploy hosts can be classified as tenant-only because matching uses the wildcard member string instead of the concrete registered host, which can delay compaction for deploy domains. Normalize or expand wildcard membership checks against registered hosts to preserve deploy timing behavior.
• In internal/server/domain_renewal.go reconcile flow, early return on r.ctx.Err() skips metrics.Tracker.SetDeferredRenewals(deferred), leaving a stale non-zero gauge and masking true deferred-renewal state during cancellations. Ensure the metric is updated on all exit paths (for example via a defer) to keep observability accurate.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/server/san_cert_manager.go">

<violation number="1" location="internal/server/san_cert_manager.go:568">
P1: When another service is provisioning concurrently, this filter leaves its domain out of the in-flight batch, but the shared `_batch_` waiter still returns immediately after that unrelated order. The waiting handshake therefore fails with `ErrCertNotFound` instead of starting or retrying its own batch. Use service-scoped provisioning single-flight keys, or retry batch acquisition after a waiter completes without obtaining this domain.</violation>
</file>

<file name="internal/server/domain_renewal.go">

<violation number="1" location="internal/server/domain_renewal.go:181">
P3: When the renewer context is cancelled mid-loop, reconcile returns at the `r.ctx.Err() != nil` guard before `metrics.Tracker.SetDeferredRenewals(deferred)` runs, so the gauge keeps a stale non-zero value instead of clearing. The gauge is only guaranteed correct when reconcile completes; on the shutdown path it can report a deferred count that no longer reflects reality.</violation>

<violation number="2" location="internal/server/domain_renewal.go:190">
P2: When a wildcard certificate covers a deploy-registered host, `compactionWindowFor` treats it as tenant-only because the certificate member is the wildcard string, not the concrete registered host. That can defer compaction until seven days before expiry and leave the registered host dependent on a quarantined batch member; apply the registered window when a wildcard member covers any renewable registered domain.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P1: When another service is provisioning concurrently, this filter leaves its domain out of the in-flight batch, but the shared _batch_ waiter still returns immediately after that unrelated order. The waiting handshake therefore fails with ErrCertNotFound instead of starting or retrying its own batch. Use service-scoped provisioning single-flight keys, or retry batch acquisition after a waiter completes without obtaining this domain.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/server/san_cert_manager.go, line 568:

<comment>When another service is provisioning concurrently, this filter leaves its domain out of the in-flight batch, but the shared `_batch_` waiter still returns immediately after that unrelated order. The waiting handshake therefore fails with `ErrCertNotFound` instead of starting or retrying its own batch. Use service-scoped provisioning single-flight keys, or retry batch acquisition after a waiter completes without obtaining this domain.</comment>

<file context>
@@ -559,6 +563,11 @@ func (m *SANCertManager) provisionCertificate(ctx context.Context, domain string
 		}
+		// The owner map is authoritative — a pending entry restored from a
+		// failed batch carries no service name.
+		if m.registeredDomains[pendingDomain] != batchService {
+			continue
+		}
</file context>

[cubic-dev-ai]

P2: When a wildcard certificate covers a deploy-registered host, compactionWindowFor treats it as tenant-only because the certificate member is the wildcard string, not the concrete registered host. That can defer compaction until seven days before expiry and leave the registered host dependent on a quarantined batch member; apply the registered window when a wildcard member covers any renewable registered domain.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/server/domain_renewal.go, line 190:

<comment>When a wildcard certificate covers a deploy-registered host, `compactionWindowFor` treats it as tenant-only because the certificate member is the wildcard string, not the concrete registered host. That can defer compaction until seven days before expiry and leave the registered host dependent on a quarantined batch member; apply the registered window when a wildcard member covers any renewable registered domain.</comment>

<file context>
@@ -161,13 +172,28 @@ func (r *certRenewer) reconcile() {
+// deploy-registered host compact a week earlier than tenant-only ones.
+func (r *certRenewer) compactionWindowFor(domains []string) time.Duration {
+	for _, domain := range domains {
+		if r.manager.isRegisteredDomain(domain) {
+			return registeredQuarantineCompactionWindow
+		}
</file context>

[cubic-dev-ai]

P3: When the renewer context is cancelled mid-loop, reconcile returns at the r.ctx.Err() != nil guard before metrics.Tracker.SetDeferredRenewals(deferred) runs, so the gauge keeps a stale non-zero value instead of clearing. The gauge is only guaranteed correct when reconcile completes; on the shutdown path it can report a deferred count that no longer reflects reality.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/server/domain_renewal.go, line 181:

<comment>When the renewer context is cancelled mid-loop, reconcile returns at the `r.ctx.Err() != nil` guard before `metrics.Tracker.SetDeferredRenewals(deferred)` runs, so the gauge keeps a stale non-zero value instead of clearing. The gauge is only guaranteed correct when reconcile completes; on the shutdown path it can report a deferred count that no longer reflects reality.</comment>

<file context>
@@ -161,13 +172,28 @@ func (r *certRenewer) reconcile() {
 		}
 	}
 
+	metrics.Tracker.SetDeferredRenewals(deferred)
 	r.reportMetrics()
 }
</file context>