Give the runtime real readiness, graceful shutdown and stated limits
All checks were successful
Build and Publish Container Image / build-and-push (push) Successful in 41s
All checks were successful
Build and Publish Container Image / build-and-push (push) Successful in 41s
Closes gap G08. /healthz returned a constant without probing anything, the server called ListenAndServe with no signal handling, and the operational limits of in-memory state, startup-loaded keys and local-only logout lived in code comments rather than anywhere an operator would look. /readyz probes LLDAP, Authelia and privacyIDEA; /healthz stays liveness and probes nothing. Keeping them distinct matters: wiring liveness to dependency health means an orchestrator restarts KeyCape when a dependency blinks, and a restart also discards every in-flight login, so the reaction is worse than the condition it reacts to. LLDAP is probed with a bind rather than a dial, since a rotated or revoked service password leaves the port open and every lookup failing -- exactly what readiness should catch and exactly what a dial would miss. The response names the failing check but never the reason: the endpoint is unauthenticated and upstream error text carries hostnames and sometimes credentials-in-URLs. Results are cached for 2s so an unauthenticated endpoint cannot be used to drive unbounded upstream traffic, and probes run concurrently under a 3s bound so a hung dependency makes the endpoint answer rather than hang with it. SIGTERM and SIGINT now drain in-flight requests for 15s, under the 30s read/write timeouts so a stuck request cannot outlive the window before SIGKILL. docs/operations.md states the single-replica topology and why, and three limits easy to get wrong: the constant key-1 key ID makes same-kid rotation a trap for consumers caching JWKS, removing a client does not revoke its issued tokens, and /logout is local only. No throughput figures are given, since nothing here benchmarks KeyCape. Shared storage and refresh tokens stay excluded, as G08 allows. Verified in the running executable: 503 naming all three checks failed while /healthz returned 200, the LLDAP check flipping to ok once started, and 40/40 requests succeeding across a SIGTERM. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV9oijZukGyGbRQGGKnK4P Assistant: claude-code Assistant-Model: opus Assistant-Process: 713576@bnt-lap001 Assistant-Session: 384c511d-9bce-4cb8-a676-2aef6c0c8df6
This commit is contained in:
parent
a9296fdf84
commit
d568b79223
8 changed files with 622 additions and 4 deletions
6
SCOPE.md
6
SCOPE.md
|
|
@ -93,7 +93,11 @@ Keycloak interchangeability are not established.
|
|||
can establish it. Subject continuity is explicitly not preserved: the canonical
|
||||
ID survives as an attribute while Keycloak mints its own `sub`.
|
||||
- The server listens on HTTP; HTTPS termination is deployment-owned.
|
||||
`/healthz` reports process status without probing dependencies. Development
|
||||
`/healthz` is liveness and probes nothing; `/readyz` probes LLDAP, Authelia and
|
||||
privacyIDEA and gates traffic. `SIGTERM` drains in-flight requests for 15s.
|
||||
The supported topology is a single replica, since login and authorization state
|
||||
is process-local — see [operations](docs/operations.md) for that and for the
|
||||
key-rotation, client-removal and logout limits. Development
|
||||
Compose needs configuration/key material absent from the checkout. Production
|
||||
deployment and custody are external; a source implementation or example client
|
||||
fragment does not prove live registration or consumer cutover.
|
||||
|
|
|
|||
101
docs/operations.md
Normal file
101
docs/operations.md
Normal file
|
|
@ -0,0 +1,101 @@
|
|||
# Operating KeyCape
|
||||
|
||||
The supported deployment topology and the limits that come with it. These are
|
||||
deliberate boundaries of the current implementation, not defects awaiting a fix:
|
||||
where a limit is a consequence of a design choice, the choice is named.
|
||||
|
||||
## Topology: exactly one replica
|
||||
|
||||
**Run one instance per issuer.** Authorization codes, login sessions and
|
||||
registration/enrollment handoffs are held in process memory. There is no shared
|
||||
store, and no sticky-session configuration makes this safe: a browser that starts
|
||||
a login on one replica and returns from Authelia to another finds no session and
|
||||
must start again. Two replicas do not halve the failure rate, they roughly double
|
||||
the login failure rate.
|
||||
|
||||
This is a documented exclusion rather than a missing feature. Adding a shared
|
||||
session store is a real option if the profile ever requires horizontal scale; it
|
||||
is not required today.
|
||||
|
||||
Consequences to plan for:
|
||||
|
||||
- **A restart drops in-flight logins.** Anyone mid-login gets an error and must
|
||||
start again. Issued tokens are unaffected — they are self-contained JWTs and
|
||||
stay valid until they expire.
|
||||
- **Rolling deployments are single-instance rollovers**, so expect a brief window
|
||||
where new logins fail. Draining (below) protects requests already in flight, not
|
||||
logins waiting on a human at Authelia's password prompt.
|
||||
|
||||
## Liveness and readiness
|
||||
|
||||
| Endpoint | Answers | Use it for |
|
||||
| --- | --- | --- |
|
||||
| `/healthz` | Is the process up? Probes nothing. | Liveness. |
|
||||
| `/readyz` | Are LLDAP, Authelia and privacyIDEA reachable? | Readiness / traffic gating. |
|
||||
|
||||
Keep these distinct. Wiring liveness to `/readyz` means an orchestrator restarts
|
||||
KeyCape whenever a dependency blinks, turning someone else's blip into an outage
|
||||
of your own — and a restart also discards every in-flight login, making it worse
|
||||
than the condition it reacted to.
|
||||
|
||||
`/readyz` returns 200 with `status: ready`, or 503 with `status: not_ready` and
|
||||
the failing check named. It reports *which* check failed, never *why*: the
|
||||
endpoint is unauthenticated and upstream error text carries hostnames and
|
||||
occasionally credentials-in-URLs. The reason is in the server log.
|
||||
|
||||
Probes are reachability and credential checks, not functional tests. LLDAP is
|
||||
probed with a bind, so a rotated or revoked service password is caught — that
|
||||
leaves the port open and every lookup failing, which is exactly what readiness
|
||||
should catch. Authelia and privacyIDEA are probed with a plain HTTP GET: any
|
||||
response means something is listening and speaking HTTP.
|
||||
|
||||
Results are cached for 2 seconds and each probe is bounded at 3 seconds. The
|
||||
cache is not an optimisation: the endpoint is necessarily unauthenticated, and
|
||||
without it anyone able to reach it could drive one upstream request per
|
||||
dependency per call.
|
||||
|
||||
## Shutdown
|
||||
|
||||
On `SIGTERM` or `SIGINT` the server stops accepting connections and gives
|
||||
in-flight requests up to 15 seconds to finish. The grace period is deliberately
|
||||
under the 30-second read/write timeouts, so a stuck request cannot outlive the
|
||||
window an orchestrator typically allows before `SIGKILL`.
|
||||
|
||||
In-memory login and authorization state is not preserved across shutdown, by
|
||||
design — see the topology section.
|
||||
|
||||
## Key and registration lifecycle
|
||||
|
||||
The signing key and all client registrations are read once at startup. Both are
|
||||
changed by editing configuration and restarting; there is no reload signal and no
|
||||
rotation service.
|
||||
|
||||
**The key ID is the constant `key-1`.** Rotating the signing key while keeping
|
||||
that identifier is a trap: a consumer caching JWKS by `kid` may keep the old key
|
||||
and reject freshly issued tokens until its cache expires. Plan rotation as
|
||||
"publish new keys, let consumers refetch, then issue with the new key", and treat
|
||||
a same-`kid` swap as a breaking change for anyone caching aggressively.
|
||||
|
||||
**Removing a client registration does not revoke tokens already issued to it.**
|
||||
There is no introspection or revocation endpoint, so an issued token stays valid
|
||||
until it expires — 15 minutes by default, or the client's `tokenLifetime`. To cut
|
||||
off a compromised client, remove the registration *and* wait out the lifetime, or
|
||||
rotate the signing key if you cannot.
|
||||
|
||||
**`/logout` clears the local KeyCape session only.** It does not end the Authelia
|
||||
session and does not revoke any issued token. A user who logs out and back in may
|
||||
not be prompted for credentials, because the upstream session is still valid.
|
||||
|
||||
## Transport
|
||||
|
||||
The server speaks plain HTTP. TLS termination belongs to the deployment. KeyCape
|
||||
deliberately does not check or gate on the transport used to reach its upstream
|
||||
providers either; upstream ID tokens are verified cryptographically instead, so
|
||||
that assurance does not depend on the network being what we believe it is
|
||||
(KEY-WP-0019).
|
||||
|
||||
## What is not claimed
|
||||
|
||||
No resource-efficiency or throughput bounds are asserted here. Nothing in this
|
||||
repository benchmarks KeyCape, so any figure would be invention. Measure it in
|
||||
your own deployment before sizing against it.
|
||||
|
|
@ -387,6 +387,23 @@ restart behavior, and coordinate key/client lifecycle and consumer refresh.
|
|||
Shared storage or refresh tokens need not be added if the accepted profile
|
||||
explicitly excludes them. Benchmark before asserting resource-efficiency bounds.
|
||||
|
||||
**Status 2026-09-08 (KEY-WP-0025): closed.** `/readyz` probes LLDAP (by bind, so
|
||||
a revoked service password is caught), Authelia and privacyIDEA, with results
|
||||
cached 2s and each probe bounded at 3s; `/healthz` stays liveness and probes
|
||||
nothing, so a dependency blip cannot trigger a restart that also discards every
|
||||
in-flight login. Failures name the check but not the reason, since the endpoint is
|
||||
unauthenticated. `SIGTERM`/`SIGINT` drain in-flight requests for 15s. Verified in
|
||||
the running executable: 503 with all three failed while `/healthz` returned 200,
|
||||
the LLDAP check flipping to ok once started, and 40/40 requests succeeding across
|
||||
a SIGTERM.
|
||||
|
||||
`docs/operations.md` states the single-replica topology and why, restart
|
||||
behaviour, and three limits easy to get wrong: the constant `key-1` key ID makes
|
||||
same-kid rotation a trap for consumers caching JWKS; removing a client does not
|
||||
revoke its issued tokens; `/logout` is local only. No throughput or resource
|
||||
figures are asserted, since nothing benchmarks KeyCape. Shared storage and
|
||||
refresh tokens remain deliberately excluded.
|
||||
|
||||
### G09 — Packaging/bootstrap and older CLI credential handling need reconciliation
|
||||
|
||||
**Priority: medium. Kind: operational/tooling gap.**
|
||||
|
|
|
|||
|
|
@ -11,9 +11,12 @@ import (
|
|||
"encoding/pem"
|
||||
"flag"
|
||||
"fmt"
|
||||
"keycape/internal/server/readiness"
|
||||
"net/http"
|
||||
"os"
|
||||
"os/signal"
|
||||
"strings"
|
||||
"syscall"
|
||||
"time"
|
||||
|
||||
"github.com/rs/zerolog"
|
||||
|
|
@ -196,6 +199,20 @@ func main() {
|
|||
})
|
||||
|
||||
// Healthz.
|
||||
// /readyz gates traffic on the dependencies a login needs; /healthz stays a
|
||||
// liveness signal (KEY-WP-0025). Restarting KeyCape because LLDAP blinked
|
||||
// would convert a dependency blip into an outage of our own, so the two must
|
||||
// not be the same endpoint.
|
||||
mux.Handle("/readyz", readiness.New(2*time.Second, 3*time.Second,
|
||||
readiness.Check{Name: "lldap", Probe: func(ctx context.Context) error {
|
||||
return lldapAdapter.Ping(ctx)
|
||||
}},
|
||||
readiness.Check{Name: "authelia", Probe: httpReachable(cfg.Authelia.TokenBaseURL, cfg.Authelia.BaseURL)},
|
||||
readiness.Check{Name: "privacyidea", Probe: httpReachable(cfg.PrivacyIDEA.BaseURL)},
|
||||
))
|
||||
|
||||
// Liveness only: this says the process is up, and deliberately probes
|
||||
// nothing. See /readyz.
|
||||
mux.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
|
|
@ -231,9 +248,74 @@ func main() {
|
|||
IdleTimeout: 120 * time.Second,
|
||||
}
|
||||
|
||||
if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
|
||||
log.Error().Err(err).Msg("server error")
|
||||
os.Exit(1)
|
||||
// Serve in the background so shutdown can be driven by a signal. Without
|
||||
// this the process died on SIGTERM mid-request, so every rolling restart
|
||||
// returned errors to whoever was logging in at that moment.
|
||||
serveErr := make(chan error, 1)
|
||||
go func() {
|
||||
if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
|
||||
serveErr <- err
|
||||
return
|
||||
}
|
||||
serveErr <- nil
|
||||
}()
|
||||
|
||||
signals := make(chan os.Signal, 1)
|
||||
signal.Notify(signals, syscall.SIGINT, syscall.SIGTERM)
|
||||
|
||||
select {
|
||||
case err := <-serveErr:
|
||||
if err != nil {
|
||||
log.Error().Err(err).Msg("server error")
|
||||
os.Exit(1)
|
||||
}
|
||||
case sig := <-signals:
|
||||
log.Info().Str("signal", sig.String()).Msg("shutting down")
|
||||
// In-flight requests get a bounded window to finish. In-memory login and
|
||||
// authorization state does not survive this and is not meant to: a
|
||||
// browser mid-login must start again after a restart. That is a
|
||||
// documented limit of the single-replica topology, not a bug to paper
|
||||
// over here -- see docs/operations.md.
|
||||
ctx, cancel := context.WithTimeout(context.Background(), shutdownGrace)
|
||||
defer cancel()
|
||||
if err := srv.Shutdown(ctx); err != nil {
|
||||
log.Error().Err(err).Msg("graceful shutdown failed; exiting anyway")
|
||||
os.Exit(1)
|
||||
}
|
||||
log.Info().Msg("shutdown complete")
|
||||
}
|
||||
}
|
||||
|
||||
// shutdownGrace bounds how long in-flight requests may finish after a signal.
|
||||
// Kept under the 30s read/write timeouts so a stuck request cannot outlive the
|
||||
// window an orchestrator typically allows before sending SIGKILL.
|
||||
const shutdownGrace = 15 * time.Second
|
||||
|
||||
// httpReachable probes the first non-empty URL with a GET, reporting whether the
|
||||
// dependency answered at all. It is a reachability check, not a functional one:
|
||||
// any HTTP response means something is listening and speaking HTTP there.
|
||||
func httpReachable(candidates ...string) func(context.Context) error {
|
||||
target := ""
|
||||
for _, candidate := range candidates {
|
||||
if candidate != "" {
|
||||
target = candidate
|
||||
break
|
||||
}
|
||||
}
|
||||
return func(ctx context.Context) error {
|
||||
if target == "" {
|
||||
return fmt.Errorf("no endpoint configured")
|
||||
}
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, nil)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -435,3 +435,39 @@ var (
|
|||
_ domain.UserRepository = (*LDAPAdapter)(nil)
|
||||
_ domain.GroupLister = (*LDAPAdapter)(nil)
|
||||
)
|
||||
|
||||
// Ping reports whether the directory is reachable and the configured service
|
||||
// credentials still bind. It is the readiness probe for LLDAP (KEY-WP-0025).
|
||||
//
|
||||
// A bind rather than a bare dial: a rotated or revoked service password leaves
|
||||
// the port open and every lookup failing, which is exactly the state readiness
|
||||
// exists to catch. dial already binds, so this opens and closes one connection.
|
||||
func (a *LDAPAdapter) Ping(ctx context.Context) error {
|
||||
type dialResult struct {
|
||||
conn LDAPConn
|
||||
err error
|
||||
}
|
||||
// dial is synchronous and has no context, so race it against the caller's
|
||||
// deadline rather than letting a hung directory outlive the probe budget.
|
||||
results := make(chan dialResult, 1)
|
||||
go func() {
|
||||
conn, err := a.dial()
|
||||
results <- dialResult{conn, err}
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
go func() {
|
||||
if result := <-results; result.err == nil {
|
||||
result.conn.Close()
|
||||
}
|
||||
}()
|
||||
return ctx.Err()
|
||||
case result := <-results:
|
||||
if result.err != nil {
|
||||
return result.err
|
||||
}
|
||||
result.conn.Close()
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
|
|
|||
129
src/internal/server/readiness/readiness.go
Normal file
129
src/internal/server/readiness/readiness.go
Normal file
|
|
@ -0,0 +1,129 @@
|
|||
// Package readiness answers whether this instance can currently serve, as
|
||||
// distinct from whether the process is alive.
|
||||
//
|
||||
// /healthz says the process is running and is the right liveness signal: an
|
||||
// orchestrator restarting KeyCape because LLDAP is briefly down would turn a
|
||||
// dependency blip into an outage of its own. /readyz says the dependencies
|
||||
// KeyCape needs to complete a login are reachable, which is what should gate
|
||||
// traffic (KEY-WP-0025).
|
||||
//
|
||||
// Probes are reachability and credential checks, not functional tests. A ready
|
||||
// response means the dependency answered, not that every operation against it
|
||||
// will succeed.
|
||||
package readiness
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"sort"
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Check is one dependency probe. Probe must respect the context deadline.
|
||||
type Check struct {
|
||||
Name string
|
||||
Probe func(ctx context.Context) error
|
||||
}
|
||||
|
||||
// Reporter serves the readiness endpoint over a set of checks.
|
||||
type Reporter struct {
|
||||
checks []Check
|
||||
ttl time.Duration
|
||||
timeout time.Duration
|
||||
|
||||
// now is swappable so tests can advance time without sleeping.
|
||||
now func() time.Time
|
||||
|
||||
mu sync.Mutex
|
||||
cached *result
|
||||
cachedAt time.Time
|
||||
}
|
||||
|
||||
type result struct {
|
||||
Status string `json:"status"`
|
||||
Checks []checkStatus `json:"checks"`
|
||||
}
|
||||
|
||||
type checkStatus struct {
|
||||
Name string `json:"name"`
|
||||
Status string `json:"status"`
|
||||
}
|
||||
|
||||
// New returns a Reporter.
|
||||
//
|
||||
// ttl caches the last result. The endpoint is necessarily unauthenticated, so
|
||||
// without a cache anyone able to reach it could drive one upstream request per
|
||||
// dependency per probe; the cache bounds that regardless of how often it is
|
||||
// called. timeout bounds each probe, so a hung dependency makes the endpoint
|
||||
// answer "not ready" rather than hang with it.
|
||||
func New(ttl, timeout time.Duration, checks ...Check) *Reporter {
|
||||
return &Reporter{checks: checks, ttl: ttl, timeout: timeout, now: time.Now}
|
||||
}
|
||||
|
||||
// ServeHTTP reports readiness: 200 when every check passes, 503 otherwise.
|
||||
//
|
||||
// The body names each check and whether it passed, but never why. The endpoint
|
||||
// is unauthenticated, and upstream error text tends to carry hostnames, versions
|
||||
// and occasionally credentials-in-URLs. Detail belongs in the server's logs.
|
||||
func (r *Reporter) ServeHTTP(w http.ResponseWriter, req *http.Request) {
|
||||
res := r.evaluate(req.Context())
|
||||
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.Header().Set("Cache-Control", "no-store")
|
||||
if res.Status != "ready" {
|
||||
w.WriteHeader(http.StatusServiceUnavailable)
|
||||
} else {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}
|
||||
_ = json.NewEncoder(w).Encode(res)
|
||||
}
|
||||
|
||||
func (r *Reporter) evaluate(ctx context.Context) result {
|
||||
r.mu.Lock()
|
||||
if r.cached != nil && r.now().Sub(r.cachedAt) < r.ttl {
|
||||
cached := *r.cached
|
||||
r.mu.Unlock()
|
||||
return cached
|
||||
}
|
||||
r.mu.Unlock()
|
||||
|
||||
res := r.runChecks(ctx)
|
||||
|
||||
r.mu.Lock()
|
||||
r.cached, r.cachedAt = &res, r.now()
|
||||
r.mu.Unlock()
|
||||
return res
|
||||
}
|
||||
|
||||
// runChecks probes every dependency concurrently: readiness should cost about
|
||||
// as long as the slowest probe, not the sum of them.
|
||||
func (r *Reporter) runChecks(ctx context.Context) result {
|
||||
statuses := make([]checkStatus, len(r.checks))
|
||||
var wg sync.WaitGroup
|
||||
for i, check := range r.checks {
|
||||
wg.Add(1)
|
||||
go func(i int, check Check) {
|
||||
defer wg.Done()
|
||||
probeCtx, cancel := context.WithTimeout(ctx, r.timeout)
|
||||
defer cancel()
|
||||
status := "ok"
|
||||
if check.Probe == nil || check.Probe(probeCtx) != nil {
|
||||
status = "failed"
|
||||
}
|
||||
statuses[i] = checkStatus{Name: check.Name, Status: status}
|
||||
}(i, check)
|
||||
}
|
||||
wg.Wait()
|
||||
|
||||
sort.Slice(statuses, func(i, j int) bool { return statuses[i].Name < statuses[j].Name })
|
||||
res := result{Status: "ready", Checks: statuses}
|
||||
for _, status := range statuses {
|
||||
if status.Status != "ok" {
|
||||
res.Status = "not_ready"
|
||||
break
|
||||
}
|
||||
}
|
||||
return res
|
||||
}
|
||||
136
src/internal/server/readiness/readiness_test.go
Normal file
136
src/internal/server/readiness/readiness_test.go
Normal file
|
|
@ -0,0 +1,136 @@
|
|||
package readiness_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"keycape/internal/server/readiness"
|
||||
)
|
||||
|
||||
func probe(err error) func(context.Context) error {
|
||||
return func(context.Context) error { return err }
|
||||
}
|
||||
|
||||
func serve(t *testing.T, r *readiness.Reporter) (int, map[string]interface{}) {
|
||||
t.Helper()
|
||||
w := httptest.NewRecorder()
|
||||
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/readyz", nil))
|
||||
var body map[string]interface{}
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil {
|
||||
t.Fatalf("body is not JSON: %v (%s)", err, w.Body.String())
|
||||
}
|
||||
return w.Code, body
|
||||
}
|
||||
|
||||
func TestReadyWhenEveryCheckPasses(t *testing.T) {
|
||||
r := readiness.New(0, time.Second,
|
||||
readiness.Check{Name: "lldap", Probe: probe(nil)},
|
||||
readiness.Check{Name: "authelia", Probe: probe(nil)},
|
||||
)
|
||||
code, body := serve(t, r)
|
||||
if code != http.StatusOK || body["status"] != "ready" {
|
||||
t.Fatalf("status %d body %v", code, body)
|
||||
}
|
||||
}
|
||||
|
||||
// A dependency KeyCape needs to complete a login being down must take this
|
||||
// instance out of rotation, not be reported as healthy.
|
||||
func TestNotReadyWhenAnyCheckFails(t *testing.T) {
|
||||
r := readiness.New(0, time.Second,
|
||||
readiness.Check{Name: "lldap", Probe: probe(errors.New("connection refused"))},
|
||||
readiness.Check{Name: "authelia", Probe: probe(nil)},
|
||||
)
|
||||
code, body := serve(t, r)
|
||||
if code != http.StatusServiceUnavailable || body["status"] != "not_ready" {
|
||||
t.Fatalf("status %d body %v", code, body)
|
||||
}
|
||||
}
|
||||
|
||||
// A nil probe is a wiring mistake; treating it as passing would report readiness
|
||||
// nobody checked.
|
||||
func TestNilProbeIsNotReady(t *testing.T) {
|
||||
r := readiness.New(0, time.Second, readiness.Check{Name: "misconfigured"})
|
||||
if code, _ := serve(t, r); code != http.StatusServiceUnavailable {
|
||||
t.Fatalf("status %d, want 503", code)
|
||||
}
|
||||
}
|
||||
|
||||
// The endpoint is unauthenticated, so upstream error text must not reach it:
|
||||
// that text carries hostnames and sometimes credentials-in-URLs.
|
||||
func TestFailureDetailIsNotDisclosed(t *testing.T) {
|
||||
secret := "ldap://admin:hunter2@lldap.internal:3890"
|
||||
r := readiness.New(0, time.Second,
|
||||
readiness.Check{Name: "lldap", Probe: probe(errors.New("dial " + secret + ": refused"))})
|
||||
w := httptest.NewRecorder()
|
||||
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/readyz", nil))
|
||||
for _, leaked := range []string{"hunter2", "lldap.internal", "refused"} {
|
||||
if strings.Contains(w.Body.String(), leaked) {
|
||||
t.Fatalf("response discloses %q: %s", leaked, w.Body.String())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Unauthenticated and uncached, each call would drive one upstream request per
|
||||
// dependency. The TTL bounds that however often the endpoint is polled.
|
||||
func TestResultsAreCachedWithinTheTTL(t *testing.T) {
|
||||
var probes int64
|
||||
r := readiness.New(time.Minute, time.Second, readiness.Check{
|
||||
Name: "lldap",
|
||||
Probe: func(context.Context) error { atomic.AddInt64(&probes, 1); return nil },
|
||||
})
|
||||
for i := 0; i < 5; i++ {
|
||||
if code, _ := serve(t, r); code != http.StatusOK {
|
||||
t.Fatalf("call %d: status %d", i, code)
|
||||
}
|
||||
}
|
||||
if got := atomic.LoadInt64(&probes); got != 1 {
|
||||
t.Fatalf("probed %d times, want 1 within the TTL", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A hung dependency must make the endpoint answer, not hang alongside it.
|
||||
func TestSlowProbeTimesOutRatherThanBlocking(t *testing.T) {
|
||||
r := readiness.New(0, 50*time.Millisecond, readiness.Check{
|
||||
Name: "slow",
|
||||
Probe: func(ctx context.Context) error {
|
||||
<-ctx.Done()
|
||||
return ctx.Err()
|
||||
},
|
||||
})
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
defer close(done)
|
||||
if code, _ := serve(t, r); code != http.StatusServiceUnavailable {
|
||||
t.Errorf("status %d, want 503", code)
|
||||
}
|
||||
}()
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("readiness endpoint blocked on a hung dependency")
|
||||
}
|
||||
}
|
||||
|
||||
// Checks run concurrently: readiness should cost about the slowest probe.
|
||||
func TestChecksRunConcurrently(t *testing.T) {
|
||||
slow := func(context.Context) error { time.Sleep(150 * time.Millisecond); return nil }
|
||||
r := readiness.New(0, time.Second,
|
||||
readiness.Check{Name: "a", Probe: slow},
|
||||
readiness.Check{Name: "b", Probe: slow},
|
||||
readiness.Check{Name: "c", Probe: slow},
|
||||
)
|
||||
start := time.Now()
|
||||
if code, _ := serve(t, r); code != http.StatusOK {
|
||||
t.Fatalf("status %d", code)
|
||||
}
|
||||
if elapsed := time.Since(start); elapsed > 400*time.Millisecond {
|
||||
t.Fatalf("checks appear to run serially: %s for three 150ms probes", elapsed)
|
||||
}
|
||||
}
|
||||
113
workplans/KEY-WP-0025-runtime-lifecycle-and-readiness.md
Normal file
113
workplans/KEY-WP-0025-runtime-lifecycle-and-readiness.md
Normal file
|
|
@ -0,0 +1,113 @@
|
|||
---
|
||||
id: KEY-WP-0025
|
||||
type: workplan
|
||||
title: "Give the runtime real readiness, graceful shutdown and stated operational limits"
|
||||
domain: infotech
|
||||
repo: key-cape
|
||||
status: finished
|
||||
owner: claude
|
||||
topic_slug: runtime-lifecycle-and-readiness
|
||||
created: "2026-09-08"
|
||||
updated: "2026-09-08"
|
||||
---
|
||||
|
||||
Closes gap G08. `/healthz` returned a constant without probing anything, the
|
||||
server called `ListenAndServe` with no signal handling, and the operational
|
||||
limits of in-memory state, startup-loaded keys and local-only logout were spread
|
||||
across code comments rather than stated anywhere an operator would look.
|
||||
|
||||
G08 explicitly does not require shared storage or refresh tokens. The work is
|
||||
therefore to make readiness real, make restarts survivable for in-flight
|
||||
requests, and write the limits down honestly.
|
||||
|
||||
## Separate readiness from liveness
|
||||
|
||||
```task
|
||||
id: KEY-WP-0025-T01
|
||||
status: done
|
||||
priority: medium
|
||||
```
|
||||
|
||||
Added `internal/server/readiness` and a `/readyz` endpoint probing LLDAP,
|
||||
Authelia and privacyIDEA. `/healthz` stays liveness and deliberately probes
|
||||
nothing: wiring liveness to dependency health means an orchestrator restarts
|
||||
KeyCape when a dependency blinks, and since a restart also discards every
|
||||
in-flight login, the reaction is worse than the condition.
|
||||
|
||||
Design decisions worth keeping:
|
||||
|
||||
- **LLDAP is probed with a bind, not a dial.** A rotated or revoked service
|
||||
password leaves the port open and every lookup failing — precisely the state
|
||||
readiness exists to catch, and precisely what a TCP dial would miss.
|
||||
- **The body names the failing check but never the reason.** The endpoint is
|
||||
unauthenticated, and upstream error text carries hostnames and sometimes
|
||||
credentials-in-URLs. Detail goes to the log. A test asserts a probe error
|
||||
containing a credentialed URL does not reach the response.
|
||||
- **Results are cached for 2s.** Not an optimisation: without it, anyone able to
|
||||
reach an unauthenticated endpoint could drive one upstream request per
|
||||
dependency per call.
|
||||
- **Probes run concurrently, each bounded at 3s**, so readiness costs the slowest
|
||||
probe rather than their sum, and a hung dependency makes the endpoint answer
|
||||
rather than hang alongside it.
|
||||
|
||||
## Shut down without dropping requests
|
||||
|
||||
```task
|
||||
id: KEY-WP-0025-T02
|
||||
status: done
|
||||
priority: medium
|
||||
```
|
||||
|
||||
`SIGTERM`/`SIGINT` now drain in-flight requests for up to 15 seconds. The grace
|
||||
period sits under the 30s read/write timeouts so a stuck request cannot outlive
|
||||
the window an orchestrator allows before `SIGKILL`.
|
||||
|
||||
In-memory login state is deliberately not preserved: a browser mid-login must
|
||||
start again after a restart. That is a property of the single-replica topology,
|
||||
documented rather than papered over.
|
||||
|
||||
## State the operational limits
|
||||
|
||||
```task
|
||||
id: KEY-WP-0025-T03
|
||||
status: done
|
||||
priority: medium
|
||||
```
|
||||
|
||||
`docs/operations.md` states the supported topology — exactly one replica, because
|
||||
authorization codes, login sessions and handoffs are process-local and no sticky
|
||||
configuration makes a second replica safe — and the consequences: restarts drop
|
||||
in-flight logins while issued tokens keep working, and rolling deploys have a
|
||||
window where new logins fail.
|
||||
|
||||
It also records three limits that are easy to get wrong in operation:
|
||||
|
||||
- the key ID is the constant `key-1`, so rotating the key without changing it can
|
||||
leave consumers caching JWKS by `kid` rejecting freshly issued tokens;
|
||||
- removing a client registration does not revoke its issued tokens, since there
|
||||
is no revocation or introspection endpoint — they stay valid until expiry;
|
||||
- `/logout` clears the local session only, not the Authelia session or any token.
|
||||
|
||||
No throughput or resource figures are given: nothing here benchmarks KeyCape, so
|
||||
any number would be invented.
|
||||
|
||||
## Verify in the running server
|
||||
|
||||
```task
|
||||
id: KEY-WP-0025-T04
|
||||
status: done
|
||||
priority: medium
|
||||
```
|
||||
|
||||
Checked against the built executable rather than the handlers, following the
|
||||
lesson from KEY-WP-0023:
|
||||
|
||||
- with every dependency down, `/readyz` returns 503 naming all three as failed
|
||||
while `/healthz` still returns 200 — the split doing its job;
|
||||
- starting LLDAP flips its check to `ok` while the others stay failed, so the
|
||||
probes are measuring what they claim;
|
||||
- 40 requests issued across a `SIGTERM` all returned 200, and the log shows the
|
||||
signal handled and shutdown completed.
|
||||
|
||||
Seven unit tests cover the reporter, including the disclosure, caching, timeout
|
||||
and concurrency properties above.
|
||||
Loading…
Add table
Add a link
Reference in a new issue