2 Commits

Author	SHA1	Message	Date
shankar0123	a41fc2d75c	feat(ratelimit): Phase 13 Sprint 13.3 — wire backend selector + scheduler janitor + docs + helm (ARCH-M1 closure complete) ... Phase 13 Sprint 13.3 — the completion half of the ARCH-M1 substantive close. Sprint 13.2 shipped the Postgres-backed sliding-window limiter + multi-replica integration test; Sprint 13.3 wires the 6 call sites in cmd/server/main.go through the operator- chosen backend selector, adds the rate_limit_buckets scheduler janitor sweep, rewrites the observability doc, exposes the env-var in the helm chart, and promotes the multi-replica integration test to a required CI status check. Signature ground-truth (sprint 13.2 + 13.3) =========================================== Prompt-template signatures: `Allow(key string) error` and "5 call sites." Actual repo: `Allow(key string, now time.Time) error` and 6 NewSlidingWindowLimiter call sites in cmd/server/main.go (the prompt miscounted the second EST per-principal arm). Per CLAUDE.md "the repo is truth," matched the live shape. What changed ============ internal/config/server.go (+40 LOC): - Added `SlidingWindowBackend string` + `SlidingWindowJanitorInterval time.Duration` to RateLimitConfig with full operator-facing documentation of the two valid values (memory|postgres) + when-to-use-which decision tree. internal/config/config.go (+27 LOC): - Load() reads CERTCTL_RATE_LIMIT_BACKEND (default "memory") + CERTCTL_RATE_LIMIT_JANITOR_INTERVAL (default 5m). - Validate() rejects anything other than ""/"memory"/"postgres" (empty = memory equivalence for test-built Configs that bypass Load()). Janitor interval must be ≥ 1 minute when set. - Failure modes return clear ::error:: with the env-var name + the valid values, so an operator typo ("postgress" → memory in a 3-replica cluster) fails fast at startup. internal/ratelimit/factory.go (NEW, 67 LOC): - NewLimiter(backend, db, maxN, window, mapCap) Limiter — single factory the 6 cmd/server/main.go call sites route through. - Drop-in signature: same maxN/window/mapCap as NewSlidingWindowLimiter (mapCap accepted + ignored for postgres — the rate_limit_buckets table grows until the janitor sweeps). - Defensive panic on unknown backend (config.Validate is SoT; this is belt-and-suspenders). internal/ratelimit/postgres_gc.go (NEW, 73 LOC): - PostgresGC struct + NewPostgresGC + GarbageCollect. - Single-statement DELETE FROM rate_limit_buckets WHERE updated_at < NOW() - maxWindow. Idempotent. - maxWindow <= 0 is a no-op (operator opt-out). internal/scheduler/scheduler.go (+90 LOC): - New RateLimitGarbageCollector interface (mirrors the ACMEGarbageCollector / SessionGarbageCollector contracts). - rateLimitGC field + rateLimitGCInterval + rateLimitGCRunning on Scheduler. - SetRateLimitGarbageCollector(gc) + SetRateLimitGCInterval(d) Setters following the existing acmeGC/sessionGC pattern. - rateLimitGCLoop() — JitteredTicker + atomic.Bool guard + per-tick context.WithTimeout(1m). Logs row count at Debug. - Loop counted in the Start() WaitGroup only when the GC is non-nil; cmd/server/main.go skips SetRateLimitGarbageCollector when backend=memory so the loop never launches for that case. cmd/server/main.go (35 LOC diff): - All 6 ratelimit.NewSlidingWindowLimiter call sites now route through ratelimit.NewLimiter(cfg.RateLimit.SlidingWindowBackend, db, ...). Grep verification post-fix returns ZERO hits. - Six sites: breakglass loginLimiter (580), ocspLimiter (1003), exportLimiter (1068), EST failed-basic (1535), EST per-principal SCEP-mTLS arm (1591), EST per-principal SCEP arm (1613). The intune.NewPerDeviceRateLimiter site at line 1823 stays unmoved — its inner type-alias wrapper is the prompt's out-of-scope (cmd/server/*.go only). - Conditionally constructs PostgresGC + wires the scheduler janitor when backend=postgres; logs the wiring decision either way so operators see "rate-limit GC sweep enabled (postgres backend)" or "in-memory backend self-prunes" in the boot log. internal/api/handler/{est,export,certificates,auth_breakglass}.go: - Replaced 5 *ratelimit.SlidingWindowLimiter field/Setter types with ratelimit.Limiter (the interface). Allow() satisfies the same call shape on both backends; the in-memory tests that construct *SlidingWindowLimiter still compile because the concrete type satisfies the interface (compile-time check in internal/ratelimit/limiter.go pins this). docs/operator/observability.md (176 LOC diff): - Replaced the "per-process, in-memory, reset-on-restart, not shared across replicas" paragraph with the new configurable-backend section: operator decision tree, backend internals (memory vs postgres), janitor description, falsifiable closure proof (the Sprint 13.2 integration test name + invocation), helm chart wiring example. - Updated inventory to reflect the actual handler file paths + actual cap configurations (the prior doc said "60s window" for several limiters that actually use 60m / 24h windows). - Doc smoke confirmed: grep -c 'per-process, in-memory, reset-on-restart' docs/operator/observability.md = 0. deploy/helm/certctl/values.yaml + templates/server-configmap.yaml + templates/server-deployment.yaml: - Exposed server.rateLimiting.backend (default "memory") + server.rateLimiting.janitorInterval (default "5m") under the existing rateLimiting block. - ConfigMap renders both as rate-limit-backend + rate-limit-janitor-interval keys. - Deployment wires CERTCTL_RATE_LIMIT_BACKEND + CERTCTL_RATE_LIMIT_JANITOR_INTERVAL env vars from the configmap. - Helm render: `helm template deploy/helm/certctl --set server.rateLimiting.backend=postgres` shows the env-var on the server-deployment.yaml output. .github/workflows/ci.yml (+12 LOC): - Added a new step in the Go Build & Test job that runs the Sprint 13.2 multi-replica integration test (TestRateLimit_PostgresBackend_CapEnforcedAcrossReplicas) with -tags=integration -race -timeout=300s. Fails the CI status check if the cross-replica row lock ever stops arbitrating across replicas — the ARCH-M1 closure regression gate. Verification (all green locally; postgres integration via CI) ============================================================ $ grep -nE 'NewSlidingWindowLimiter' cmd/server/*.go (zero hits — Sprint 13.3 receipt) $ go test -short -count=1 \ ./internal/config/... ./internal/ratelimit/... \ ./internal/scheduler/... ./internal/api/handler/... \ ./cmd/server/... ok internal/config 1.177s ok internal/ratelimit 0.007s ok internal/scheduler 9.165s ok internal/api/handler 6.245s ok cmd/server 0.390s $ staticcheck ./internal/ratelimit/... ./internal/scheduler/... \ ./internal/config/... ./internal/api/handler/... ./cmd/server/... (clean) $ gofmt -l internal/ cmd/server/ (clean) $ grep -c 'per-process, in-memory, reset-on-restart' \ docs/operator/observability.md 0 (doc smoke — the audit's verbatim phrasing is gone) $ bash scripts/ci-guards/G-3-env-docs-drift.sh G-3 env-docs-drift: clean. $ bash scripts/ci-guards/complete-path-config-coverage.sh OK — every CERTCTL_* env var (197) has at least one non-config- package consumer. Selector contract verified — config.Validate() rejects any value other than ""/memory/postgres at startup with a clear error message. Sprint 13.4 next (ARCH-H1 OpenAPI authoring batch 1) is on a different axis; ARCH-M1 closure is complete with this commit modulo the Sprint 13.7 audit-HTML flip + zero-floor pin. Closes: ARCH-M1 substantive remediation. The cross-replica rate- limit-cap-enforcement gap that the audit recommended deferring to v3 is closed; operators with server.replicas > 1 flip CERTCTL_RATE_LIMIT_BACKEND=postgres and get exactly-cap enforcement across the cluster (proved by the multi-replica integration test now gating CI).	2026-05-14 11:52:13 +00:00
shankar0123	57b539c378	docs(b12): observability reference + Postgres backup runbook ... Closes acquisition-diligence Bundle 12 — Observability, DR, Operations Receipts, And Performance Proof. Source IDs: D5, D6, D8, T9, finding 7, OPS-H1, OPS-M1, OPS-M2, LOW-7. Two new operator-facing references; both non-audit-framed per the Bundle 5 doc-placement policy. docs/operator/observability.md — single canonical statement of what certctl emits, what it doesn't, and what survives a restart: - Metrics surface: both /api/v1/metrics (JSON) and /api/v1/metrics/prometheus (text exposition v0.0.4); inventory of certctl_certificate_* gauges + certctl_issuance_duration_seconds per-issuer-type histogram + certctl_uptime_seconds. - Prometheus library vs hand-rolled exposition: explicit scope statement — hand-rolled fmt.Fprintf is intentional for v2.x given the shallow metric surface; client_golang migration tracked as v3 item (closes OPS-M1). - Tracing: explicit deferral — no OTel SDK setup, OTel packages are indirect-only in go.mod, no spans, no OTLP exporter; tracked as v3 item; in the meantime structured logs carry request_id and certctl_issuance_duration_seconds carries the per-issuer latency signal (closes OPS-M2). - Logging: structured JSON via log/slog; CERTCTL_LOG_LEVEL control; no key material / bearer tokens / session cookies in log lines. - Rate-limit semantics under restarts + replicas: per-process, in-memory, reset-on-restart, NOT shared across replicas; full inventory of the 5 limiter call sites (break-glass login, SCEP/Intune per-device, EST per-principal CSR, EST HTTP-Basic source-IP, ACME per-account); multi-replica + sticky-session implications; database-backed sliding window deferred to v3 (closes D8). - Performance harness scope: cross-references the explicit 'What it explicitly does NOT measure' list in deploy/test/loadtest/README.md (closes LOW-7 + finding 7). docs/operator/runbooks/postgres-backup.md — operator-runnable backup procedure: - Inventory of what to back up (DB + operator-managed file material that lives outside the DB: CA keys, RA keys, OCSP responder keys, trust bundles). - Logical backup recipe with docker-compose + Kubernetes variants, integrity verification step, off-host storage step. - Physical / PITR recipe pointing at pgbackrest / wal-g (certctl ships nothing here — standard PostgreSQL DBA work). - Three sample automation paths (in-cluster Postgres → S3 CronJob, managed Postgres PITR, self-hosted VM systemd timer + restic). - Quarterly restore-dry-run procedure. - Helm CronJob template deliberately not shipped — three documented reasons (deployment topology / secret-management integration / off-host storage all vary by operator) plus roadmap entry for shipping a starter template when a real operator asks for one (closes D6 + OPS-H1). Both new docs wired into docs/README.md Operator + Runbooks tables. D5 (ServiceMonitor) and T9 (canonical k6 load-test) were already shipped in Bundle 3 (deploy/helm/certctl/templates/servicemonitor.yaml) and in deploy/test/loadtest/ + .github/workflows/loadtest.yml respectively; this bundle doesn't touch them — it just records the closure in the audit HTML. Verified: bash scripts/ci-guards/G-3-env-docs-drift.sh # PASS bash scripts/ci-guards/doc-rot-detector.sh # PASS All 35 scripts/ci-guards/*.sh green.	2026-05-13 02:09:11 +00:00