From c449e365f24977afed4fb59cb77e8520b36a75f8 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 00:21:01 -0500 Subject: [PATCH 01/23] feat(api): qualify the release API image with both database drivers Signed-off-by: Baylem --- .github/actions/build-e2e-images/action.yml | 14 ++------------ api/Dockerfile | 4 ++-- charts/gameplane/values.yaml | 3 +-- containers/docker-bake.hcl | 9 --------- deploy/kind/e2e.sh | 17 ++--------------- docs/dependencies.md | 17 ++++------------- docs/install.md | 2 +- 7 files changed, 12 insertions(+), 54 deletions(-) diff --git a/.github/actions/build-e2e-images/action.yml b/.github/actions/build-e2e-images/action.yml index 3d355653a..ed3071b76 100644 --- a/.github/actions/build-e2e-images/action.yml +++ b/.github/actions/build-e2e-images/action.yml @@ -1,9 +1,9 @@ name: build e2e images description: > Build the operator/api/agent/sentinel/capture-sidecar/telemetry-receiver/fakeoidc/mcp-server/audit-syslog-bridge/tunnel-frp - (+ gameprobe and the postgres-tagged api) images the kind e2e suites load into the cluster, tagged + (+ gameprobe) images the kind e2e suites load into the cluster, tagged gameplane-test/:e2e exactly like make e2e-images, via buildx bake - — then `docker save` all twelve into a single tar uploaded as an artifact + — then `docker save` all eleven into a single tar uploaded as an artifact (e2e-images-). The heavy e2e jobs load that tar instead of each recompiling the same binaries. This is the ONLY builder, so it always writes the layer cache back (the old single-writer export dance is gone). @@ -52,15 +52,6 @@ runs: set: | e2e-gameprobe.cache-from=type=gha,scope=e2e-gameprobe-${{ inputs.arch }} e2e-gameprobe.cache-to=type=gha,mode=max,scope=e2e-gameprobe-${{ inputs.arch }} - - name: bake postgres-tagged api image - uses: docker/bake-action@d3418bd7d0e9324001bca92fa8ba175ea7e6dc9b # v7.3.0 - with: - files: containers/docker-bake.hcl - targets: e2e-api-postgres - load: true - set: | - e2e-api-postgres.cache-from=type=gha,scope=e2e-api-postgres-${{ inputs.arch }} - e2e-api-postgres.cache-to=type=gha,mode=max,scope=e2e-api-postgres-${{ inputs.arch }} - name: save images to a single tar shell: bash run: | @@ -76,7 +67,6 @@ runs: gameplane-test/audit-syslog-bridge:e2e \ gameplane-test/tunnel-frp:e2e \ gameplane-test/gameprobe:e2e \ - gameplane-test/api:e2e-postgres \ -o "$RUNNER_TEMP/e2e-images.tar" - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 with: diff --git a/api/Dockerfile b/api/Dockerfile index a387d4e2c..4ba31c5bc 100644 --- a/api/Dockerfile +++ b/api/Dockerfile @@ -23,8 +23,8 @@ ARG VERSION=dev # QEMU emulation: multi-arch release builds of the arm64 image were slow # enough to hit the release job timeout. ARG TARGETARCH -# Build tags: "postgres" for the e2e-postgres image; empty by default so the release build is unchanged. -ARG GO_TAGS="" +# Both drivers ship in the multi-architecture API image. SQLite remains the runtime default. +ARG GO_TAGS="postgres" RUN CGO_ENABLED=0 GOOS=linux GOARCH=${TARGETARCH} go build \ -tags "${GO_TAGS}" \ -trimpath \ diff --git a/charts/gameplane/values.yaml b/charts/gameplane/values.yaml index 1a2640675..f27a9bccc 100644 --- a/charts/gameplane/values.yaml +++ b/charts/gameplane/values.yaml @@ -213,8 +213,7 @@ api: driver: sqlite # sqlite | postgres dsn: "file:/data/gameplane.db?_pragma=journal_mode(WAL)" # An existing Secret in the release namespace takes precedence over dsn. - # For postgres, store the connection string there and use an API image - # built with -tags postgres. Helm only needs the Secret name and key. + # For postgres, provision the full connection string there. The release API includes both drivers. existingSecret: "" secretKey: dsn diff --git a/containers/docker-bake.hcl b/containers/docker-bake.hcl index fbc31ee77..9102aa44f 100644 --- a/containers/docker-bake.hcl +++ b/containers/docker-bake.hcl @@ -76,15 +76,6 @@ target "e2e-tunnel-frp" { tags = ["gameplane-test/tunnel-frp:e2e"] } -# The API built with the "postgres" build tag (spec 013 US3). Baked on its own, -# like gameprobe, so the default e2e group keeps its SQLite-only api image. -target "e2e-api-postgres" { - context = "." - dockerfile = "api/Dockerfile" - args = { GO_TAGS = "postgres" } - tags = ["gameplane-test/api:e2e-postgres"] -} - # The headless protocol bot the game-bot job runs inside the cluster. It is # deliberately outside the "e2e" group: only that one job needs it, and the # other e2e buckets shouldn't pay to build it. diff --git a/deploy/kind/e2e.sh b/deploy/kind/e2e.sh index 9b060d9d4..254796069 100755 --- a/deploy/kind/e2e.sh +++ b/deploy/kind/e2e.sh @@ -17,10 +17,8 @@ # the GAMEPLANE_E2E_TAG env var. # # Profile defaults to "default". The "postgres" profile (spec 013 US5) runs -# the API on PostgreSQL: it applies deploy/kind/postgres.yaml, retags the -# postgres-tagged api image as gameplane-test/api: before loading, and -# installs with api.db.driver=postgres and the DSN from the -# gameplane-postgres Secret. The retag replaces the local SQLite api image. +# the standard API image against deploy/kind/postgres.yaml using the DSN +# referenced by the gameplane-postgres Secret. set -euo pipefail @@ -232,17 +230,6 @@ EOF install_metallb apply_metallb_pools - # The chart has one image.tag for every component, so the postgres - # profile loads the postgres-tagged api build under the shared tag. - if [ "${PROFILE}" = postgres ]; then - if ! docker image inspect gameplane-test/api:e2e-postgres >/dev/null 2>&1; then - echo "missing local image gameplane-test/api:e2e-postgres (bake target e2e-api-postgres)" >&2 - exit 1 - fi - echo "retagging gameplane-test/api:e2e-postgres as gameplane-test/api:${TAG}" - docker tag gameplane-test/api:e2e-postgres "gameplane-test/api:${TAG}" - fi - echo "loading gameplane-test/{operator,api,agent,sentinel,capture-sidecar,telemetry-receiver,mcp-server,audit-syslog-bridge,tunnel-frp}:${TAG} images into kind" for img in operator api agent sentinel capture-sidecar telemetry-receiver mcp-server audit-syslog-bridge tunnel-frp; do if ! docker image inspect "gameplane-test/${img}:${TAG}" >/dev/null 2>&1; then diff --git a/docs/dependencies.md b/docs/dependencies.md index d43326fc9..aa0a367fd 100644 --- a/docs/dependencies.md +++ b/docs/dependencies.md @@ -428,19 +428,10 @@ The Helm chart itself (`charts/gameplane/Chart.yaml`) declares **no chart new module for a single read-only GET was judged not worth the extra dependency surface — the same reasoning that keeps `netguard`, `gameaction`, `audit-syslog-bridge`, and `telemetry-receiver` this lean. -- **Persistence is driver-selectable, but Postgres needs a rebuild.** - `api/internal/db` registers `modernc.org/sqlite` (pure-Go, default) - unconditionally, and compiles in `github.com/jackc/pgx/v5`'s `stdlib` - driver only under the `postgres` build tag (`db_postgres.go`); without - that tag, `db_nopostgres.go` stands in and `Open(ctx, "postgres", dsn)` - returns `"postgres support not compiled in — rebuild with -tags - postgres"`. Confirmed: `api/Dockerfile` (and thus the published - `ghcr.io/.../api` image) does **not** pass `-tags postgres` — this is - intentional and documented, not an oversight: `charts/gameplane/values.yaml`'s - `api.db.driver` comment explicitly tells an operator who wants Postgres [experimental] - to set `dsn` *and* rebuild the image with `-tags postgres` themselves. - So `pgx/v5` is a real, live dependency, but it ships dormant in the - default binary. +- **Persistence drivers.** The release API image includes pgx and modernc SQLite. + SQLite is the default at runtime. The PostgreSQL kind lane exercises the same + API build used by the SQLite lane. Custom Go builds may omit the `postgres` + tag to produce a SQLite-only binary. - **netguard's two policies, three importers.** Both `IsAllowed` (permissive, for admin-configured infrastructure) and `IsPublic` (strict, for user-supplied targets) are used by operator (module sources), API gateway diff --git a/docs/install.md b/docs/install.md index fa372c215..3d667e787 100644 --- a/docs/install.md +++ b/docs/install.md @@ -256,7 +256,7 @@ Top-level knobs (see `values.yaml` for the full list): GameServer enters Pending with a `PVCProvisioningFailed` condition (visible in the dashboard); no pod starts until resolved. Example: `--set operator.gameDataStorage.storageClassName=fast-nvme` -- `api.db.driver` — `sqlite` (default) or `postgres` (experimental). PostgreSQL requires an API image built with `-tags postgres`. +- `api.db.driver` — `sqlite` (default) or `postgres` (experimental). The API image includes both drivers. Use one API replica with PostgreSQL. - `api.db.dsn` — literal connection string. The SQLite default persists to a PVC. - `api.db.existingSecret` and `api.db.secretKey` — name of a pre-provisioned Secret in the release namespace and the key holding the complete DSN (default key `dsn`). A Secret reference takes precedence over the literal DSN for both the API and `bootstrap-admin`. A missing Secret or key prevents the API container from starting. - `api.storage.existingClaim` — pre-existing PVC for the API's SQLite database (default `""`). PostgreSQL standalone deployments create no local database PVC or mount. The chart annotates `gameplane-api-data` with `helm.sh/resource-policy: keep` so switching to an existing claim preserves the previous PVC. From 808873683d91a78baa5e745c184fc31342519ef6 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 00:28:55 -0500 Subject: [PATCH 02/23] test(recovery): fence PostgreSQL and verify restored access at a named WAL boundary Signed-off-by: Baylem --- .github/workflows/ci.yaml | 8 ++ deploy/kind/recover-postgres-wal.sh | 94 ++++++++++++++++++++++ docs/panel-recovery.md | 23 ++++++ test/e2e/buckets.sh | 5 +- test/e2e/postgres_pitr_e2e_test.go | 116 ++++++++++++++++++++++++++++ 5 files changed, 245 insertions(+), 1 deletion(-) create mode 100644 deploy/kind/recover-postgres-wal.sh create mode 100644 test/e2e/postgres_pitr_e2e_test.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 8f25bc91d..b22bb014b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -2525,6 +2525,12 @@ jobs: go test -tags=e2e -timeout 15m -v -parallel 1 -run "$RUN_REGEX" ./... - name: verify Secret-backed PostgreSQL configuration after upgrade run: bash deploy/kind/check-postgres-secret.sh "$CTX" + - name: run go e2e (postgres-pitr) + run: | + RUN_REGEX="$(./test/e2e/buckets.sh regex postgres-pitr)" + cd test/e2e + GAMEPLANE_E2E_REUSE_CLUSTER=1 GAMEPLANE_E2E_CLUSTER="$CLUSTER" \ + go test -tags=e2e -timeout 15m -v -parallel 1 -run "$RUN_REGEX" ./... - name: dump cluster state on failure (e2e-postgres) if: failure() uses: ./.github/actions/dump-cluster-state @@ -2945,6 +2951,8 @@ jobs: if (/^e2e postgres \//.test(n) && Array.isArray(j.steps)) { const availability = j.steps.find((s) => s.name === 'run go e2e (PostgreSQL singleton upgrade and outage recovery)'); if (availability && availability.conclusion === 'success') bucketSet.add('postgres-availability'); + const pitr = j.steps.find((s) => s.name === 'run go e2e (postgres-pitr)'); + if (pitr && pitr.conclusion === 'success') bucketSet.add('postgres-pitr'); } // The ratelimit bucket runs as a trailing step inside the // api-auth job leg (deliberately, so it drains the shared diff --git a/deploy/kind/recover-postgres-wal.sh b/deploy/kind/recover-postgres-wal.sh new file mode 100644 index 000000000..8b15a42ac --- /dev/null +++ b/deploy/kind/recover-postgres-wal.sh @@ -0,0 +1,94 @@ +#!/usr/bin/env bash +# Fenced physical recovery drill for the disposable PostgreSQL kind job. +# The restored database runs on a new volume. The original claim is retained. +set -euo pipefail +context=$1 +viewer=$2 +report=$3 +[[ $context == kind-gameplane-e2e && $viewer =~ ^[1-9][0-9]*$ ]] +k=(kubectl --context "$context" -n gameplane-system) +fixture_label=$("${k[@]}" get sts gameplane-postgres -o jsonpath='{.metadata.labels.app\.kubernetes\.io/component}') +[[ $fixture_label == e2e-fixture ]] +source=gameplane-postgres-0 +target=postgres-pitr +sql() { + "${k[@]}" exec -i "$1" -- psql -X -qAt -v ON_ERROR_STOP=1 -U gameplane -d gameplane "${@:2}" +} +# Allocate the replacement before fencing. It has a distinct Service selector +# and cannot receive application connections until reconciliation has finished. +"${k[@]}" get sts gameplane-postgres -o json | jq --arg name "$target" '{ + apiVersion:"v1",kind:"Pod",metadata:{name:$name,labels:{"app.kubernetes.io/name":$name}}, + spec:{automountServiceAccountToken:false,restartPolicy:"Never", + securityContext:{runAsNonRoot:true,runAsUser:70,runAsGroup:70,fsGroup:70,seccompProfile:{type:"RuntimeDefault"}}, + containers:[{name:"postgres",image:.spec.template.spec.containers[0].image, + command:["sh","-ec","mkdir -p /data/db /data/wal; chmod 0700 /data/db; while [ ! -f /data/start ]; do sleep 1; done; exec postgres -D /data/db"], + securityContext:{allowPrivilegeEscalation:false,capabilities:{drop:["ALL"]}}, + volumeMounts:[{name:"recovery",mountPath:"/data"}]}], + volumes:[{name:"recovery",emptyDir:{}}]}}' | "${k[@]}" create -f - +"${k[@]}" wait --for=condition=Ready "pod/$target" --timeout=120s +"${k[@]}" exec "$target" -- mkdir -p /data/db /data/wal + +# Pin WAL on the original instance between the physical base backup and loss. +sql "$source" -c "SELECT pg_create_physical_replication_slot('gameplane_pitr', true)" >/dev/null +first=$(sql "$source" -c "SELECT pg_walfile_name(restart_lsn) FROM pg_replication_slots WHERE slot_name='gameplane_pitr'") +"${k[@]}" exec "$source" -- sh -ec ' + backup=$(mktemp -d) + pg_basebackup -U gameplane -D "$backup" -Fp -X stream -c fast + tar -C "$backup" -cf - . +' | + "${k[@]}" exec -i "$target" -- tar -xf - -C /data/db +"${k[@]}" exec "$target" -- pg_verifybackup /data/db +# This identity exists only in WAL after the base backup. The external +# revocation ledger says it was revoked after the recovery target. +sql "$source" -v viewer="$viewer" <<'SQL' +INSERT INTO oidc_links(user_id,issuer,subject) VALUES (:viewer,'https://pitr.invalid','revoked-subject'); +INSERT INTO oidc_links(user_id,issuer,subject) VALUES (:viewer,'https://retained.pitr.invalid','retained-subject'); +SQL +point=$(sql "$source" -c "SELECT pg_create_restore_point('gameplane_fenced_restore')") +point_time=$(date +%s) +sql "$source" <<'SQL' +DELETE FROM oidc_links WHERE issuer='https://pitr.invalid' AND subject='revoked-subject'; +INSERT INTO users(username,role) VALUES ('pitr-after-target','viewer'); +SQL +loss_time=$(date +%s) +"${k[@]}" scale deployment/gameplane-api --current-replicas=1 --replicas=0 +"${k[@]}" wait --for=delete pod -l app.kubernetes.io/name=gameplane-api --timeout=120s +last=$(sql "$source" -c 'SELECT pg_walfile_name(pg_switch_wal())') +[[ $first =~ ^[0-9A-F]{24}$ && $last =~ ^[0-9A-F]{24}$ ]] +# Only closed segments are copied. The slot keeps all files in range. +"${k[@]}" exec "$source" -- sh -ec ' + cd "$PGDATA/pg_wal" + for file in ????????????????????????; do + [ "$file" \< "$1" ] || [ "$file" \> "$2" ] || echo "$file" + done | tar -cf - -T - +' sh "$first" "$last" | "${k[@]}" exec -i "$target" -- tar -xf - -C /data/wal +"${k[@]}" scale sts/gameplane-postgres --current-replicas=1 --replicas=0 +"${k[@]}" wait --for=delete "pod/$source" --timeout=120s + +"${k[@]}" exec -i "$target" -- sh -ec 'cat >> /data/db/postgresql.auto.conf; touch /data/db/recovery.signal /data/start' <<'CONFIG' +restore_command = 'cp /data/wal/%f %p' +recovery_target_name = 'gameplane_fenced_restore' +recovery_target_action = 'promote' +CONFIG +for _ in {1..90}; do + if [[ $(sql "$target" -c 'SELECT NOT pg_is_in_recovery()' 2>/dev/null) == t ]]; then break; fi + sleep 1 +done +[[ $(sql "$target" -c 'SELECT NOT pg_is_in_recovery()') == t ]] +[[ $(sql "$target" -c "SELECT count(*) FROM users WHERE username='pitr-after-target'") == 0 ]] +[[ $(sql "$target" -c "SELECT count(*) FROM oidc_links WHERE issuer='https://pitr.invalid' AND subject='revoked-subject' AND user_id=$viewer") == 1 ]] +sql "$target" <<'SQL' +BEGIN; +DELETE FROM oidc_links WHERE issuer='https://pitr.invalid' AND subject='revoked-subject'; +DELETE FROM sessions; +DELETE FROM api_tokens; +UPDATE share_links SET revoked_at=to_char(now() AT TIME ZONE 'UTC','YYYY-MM-DD"T"HH24:MI:SS"Z"') WHERE revoked_at IS NULL; +COMMIT; +SQL +[[ $(sql "$target" -c "SELECT count(*) FROM oidc_links WHERE issuer='https://pitr.invalid'") == 0 ]] +[[ $(sql "$target" -c "SELECT count(*) FROM oidc_links WHERE issuer='https://retained.pitr.invalid' AND subject='retained-subject' AND user_id=$viewer") == 1 ]] +"${k[@]}" patch service gameplane-postgres --type=merge -p '{"spec":{"selector":{"app.kubernetes.io/name":"postgres-pitr"}}}' +"${k[@]}" scale deployment/gameplane-api --current-replicas=0 --replicas=1 +"${k[@]}" rollout status deployment/gameplane-api --timeout=180s +jq -n --arg lsn "$point" --argjson point "$point_time" --argjson loss "$loss_time" \ + '{restore_target_lsn:$lsn,recovery_point_age_seconds:($loss-$point),loss_unix_seconds:$loss}' > "$report" diff --git a/docs/panel-recovery.md b/docs/panel-recovery.md index d8e075f95..63fced6ee 100644 --- a/docs/panel-recovery.md +++ b/docs/panel-recovery.md @@ -175,3 +175,26 @@ rows. Use repeated passing runs and your own production-shaped drills to select and demonstrate targets. This fixture covers SQLite panel recovery, not storage cluster loss, PostgreSQL failover, a particular game's save format, or a complete Kubernetes disaster recovery exercise. + +## PostgreSQL physical recovery + +The PostgreSQL CI lane runs the release API image with one writer. After the +availability and Secret checks, the dedicated PITR step takes a physical base +backup, writes authorization state, creates a named WAL restore point, and +records a subsequent revocation and a user created after that point. It fences +the old API and database before promoting a copy from WAL. In the isolated +cluster it keeps the old database claim for inspection. In production, retain +the physical backup and archived WAL on storage independent of the database. + +Before reconnecting the API, the drill reapplies the recorded IdP revocation +and invalidates restored sessions, tokens and shares. It then checks positive +and denied requests, the old session, the audit chain, the owner on the live +GameServer, the agent credential and a retained world marker. The CI log +records the PostgreSQL target LSN, elapsed recovery-point age, time from loss +to restored authorization and game verification, and the retained resource +UIDs. Use actual successful job output when reporting recovery measurements. + +Both backends require one API writer. Fence the original process and storage +before promotion. Active-active deployment would additionally require +database-level audit and last-manager serialization, coordinated migrations +and background jobs, and cross-replica sessions and rate limits. diff --git a/test/e2e/buckets.sh b/test/e2e/buckets.sh index c4a7afdec..9e21d6dd4 100755 --- a/test/e2e/buckets.sh +++ b/test/e2e/buckets.sh @@ -388,6 +388,8 @@ TestUpgrade_PostgresAvailability EOF } +bucket_postgres_pitr() { printf '%s\n' TestPanel_PostgresPointInTimeRecovery; } + # One sequential lifecycle test that runs `helm upgrade --reuse-values` (1m # report interval, dashboard token, public summary, no per-source ingest # limit) on its own bucket cluster, so it must not share a cluster with other @@ -461,7 +463,7 @@ bucket_network_isolation() { } bucket_names() { - printf '%s\n' operator api-auth api-roles api-rbac api-agent api-mods ratelimit bot-fast bot-heavy multicluster upgrade postgres-availability telemetry capture-overhead optional-components restricted-workloads wake-hostport network-isolation management-placement backup-preparation monitoring recovery + printf '%s\n' operator api-auth api-roles api-rbac api-agent api-mods ratelimit bot-fast bot-heavy multicluster upgrade postgres-availability postgres-pitr telemetry capture-overhead optional-components restricted-workloads wake-hostport network-isolation management-placement backup-preparation monitoring recovery } list_bucket() { @@ -478,6 +480,7 @@ list_bucket() { multicluster) bucket_multicluster ;; upgrade) bucket_upgrade ;; postgres-availability) bucket_postgres_availability ;; + postgres-pitr) bucket_postgres_pitr ;; telemetry) bucket_telemetry ;; capture-overhead) bucket_capture_overhead ;; optional-components) bucket_optional_components ;; diff --git a/test/e2e/postgres_pitr_e2e_test.go b/test/e2e/postgres_pitr_e2e_test.go new file mode 100644 index 000000000..fcc73bc07 --- /dev/null +++ b/test/e2e/postgres_pitr_e2e_test.go @@ -0,0 +1,116 @@ +//go:build e2e + +package e2e + +import ( + "encoding/json" + "net/http" + "os" + "os/exec" + "path/filepath" + "strconv" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Runs last in the disposable PostgreSQL cluster. The original DB stays fenced. +func TestPanel_PostgresPointInTimeRecovery(t *testing.T) { + ctx := t.Context() + if envInstance.Context != "kind-gameplane-e2e" { + t.Fatal("PITR requires the disposable PostgreSQL kind fixture") + } + fixture, err := envInstance.K8s.AppsV1().StatefulSets("gameplane-system").Get(ctx, "gameplane-postgres", metav1.GetOptions{}) + if err != nil || fixture.Labels["app.kubernetes.io/component"] != "e2e-fixture" { + t.Fatalf("PostgreSQL fixture is unavailable: %v", err) + } + envInstance.BootstrapAdmin(t, adminUsername, adminPassword) + admin := envInstance.APIClient(t, adminUsername, adminPassword) + t.Cleanup(admin.Close) + user, password, id := envInstance.CreateUser(t, admin, "viewer", "pitr-viewer") + userID, err := strconv.ParseInt(id, 10, 64) + if err != nil { + t.Fatal(err) + } + game, gameUID := recoveryGame(t, "pitr") + const ns = "gameplane-games" + claimID := func() string { + t.Helper() + pvc, err := envInstance.K8s.CoreV1().PersistentVolumeClaims(ns).Get(ctx, game+"-data", metav1.GetOptions{}) + if err != nil || pvc.UID == "" { + t.Fatalf("read game data claim: %v", err) + } + return string(pvc.UID) + } + keyID := func() string { + t.Helper() + key, err := envInstance.K8s.CoreV1().Secrets("gameplane-system").Get(ctx, "gameplane-agent-ca", metav1.GetOptions{}) + if err != nil || key.UID == "" { + t.Fatalf("read native agent CA identity: %v", err) + } + return string(key.UID) + } + f := recoveryFixture{t: t} + f.request(admin, http.MethodPost, "/servers/"+game+":transfer?namespace="+ns, map[string]any{"userId": userID}, http.StatusNoContent) + beforePod, beforeClaim, beforeKey := recoveryGameIdentity(t, game), claimID(), keyID() + beforeProgress := recoveryCounter(t, game) + reader := envInstance.APIClient(t, user, password) + t.Cleanup(reader.Close) + f.request(reader, http.MethodGet, "/users/me", nil, http.StatusOK) + f.request(reader, http.MethodGet, "/users", nil, http.StatusForbidden) + + script, err := findRepoRel("deploy/kind/recover-postgres-wal.sh") + if err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "wal.json") + out, err := exec.CommandContext(ctx, "bash", script, envInstance.Context, id, path).CombinedOutput() + if err != nil { + t.Fatalf("fenced database recovery: %v\n%s", err, out) + } + post := envInstance.APIClient(t, user, password) + t.Cleanup(post.Close) + stale := &APIClient{BaseURL: post.BaseURL, HTTP: reader.HTTP, CSRF: reader.CSRF} + f.request(stale, http.MethodGet, "/users/me", nil, http.StatusUnauthorized) + me := f.request(post, http.MethodGet, "/users/me", nil, http.StatusOK) + if extractIntField(string(me), "id") != id { + t.Fatal("restored account ID does not match the recorded owner") + } + f.waitServer(post, "/servers?namespace="+ns, gameUID) + f.request(post, http.MethodGet, "/users", nil, http.StatusForbidden) + restoredAdmin := envInstance.APIClient(t, adminUsername, adminPassword) + t.Cleanup(restoredAdmin.Close) + marker := f.request(restoredAdmin, http.MethodGet, "/servers/"+game+"/files/read?namespace="+ns+"&path=%2Frecovery-marker", nil, http.StatusOK) + if string(marker) != "retained\n" { + t.Fatal("agent-backed read returned a different save marker") + } + var audit struct { + OK bool + Checked int + } + raw := f.request(restoredAdmin, http.MethodGet, "/admin/audit/verify", nil, http.StatusOK) + if json.Unmarshal(raw, &audit) != nil || !audit.OK || audit.Checked == 0 { + t.Fatal("restored audit chain failed verification") + } + gs, err := envInstance.Dyn.Resource(gameServerGVR).Namespace(ns).Get(ctx, game, metav1.GetOptions{}) + if err != nil || string(gs.GetUID()) != gameUID || gs.GetAnnotations()["gameplane.local/owner-id"] != id { + t.Fatalf("restored owner does not match the live GameServer: %v", err) + } + if recoveryGameIdentity(t, game) != beforePod || claimID() != beforeClaim || keyID() != beforeKey || recoveryCounter(t, game) <= beforeProgress { + t.Fatal("game process, save, claim, native CA identity or progress changed unexpectedly") + } + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var metrics struct { + LSN string `json:"restore_target_lsn"` + Age int64 `json:"recovery_point_age_seconds"` + Loss int64 `json:"loss_unix_seconds"` + } + if err := json.Unmarshal(b, &metrics); err != nil || metrics.Loss <= 0 || metrics.LSN == "" { + t.Fatalf("invalid PITR report: %v", err) + } + t.Logf("PostgreSQL PITR passed: target LSN=%s, panel recovery point age=%ds, panel RTO=%ds, live game retained CR=%s PVC=%s Pod=%s and advanced its persistent counter", metrics.LSN, metrics.Age, time.Now().Unix()-metrics.Loss, gameUID, beforeClaim, beforePod.PodUID) +} From 9c73a3b2d103a19dbd9d9adc0b38c5282ed066f3 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 00:53:08 -0500 Subject: [PATCH 03/23] feat(chart): stage native PostgreSQL API replicas across workers Signed-off-by: Baylem --- api/cmd/health.go | 10 +++- api/cmd/health_test.go | 1 + charts/gameplane/templates/api.yaml | 26 +++++++--- .../gameplane/templates/crd-apply-hook.yaml | 2 +- .../templates/poddisruptionbudgets.yaml | 4 +- charts/gameplane/templates/validate.yaml | 25 +++++++++- charts/gameplane/values.yaml | 11 +++-- charts/tests/check_standalone.py | 21 +++++++- docs/install.md | 48 ++++++++++++++----- 9 files changed, 117 insertions(+), 31 deletions(-) diff --git a/api/cmd/health.go b/api/cmd/health.go index 56bae3836..56aa9ec39 100644 --- a/api/cmd/health.go +++ b/api/cmd/health.go @@ -20,7 +20,7 @@ func mountHealthRoutes(r chi.Router, store *db.Store) { _, _ = w.Write([]byte("ok")) }) pingSlot := make(chan struct{}, 1) - r.Get("/readyz", func(w http.ResponseWriter, req *http.Request) { + ready := func(w http.ResponseWriter, req *http.Request) { select { case pingSlot <- struct{}{}: defer func() { <-pingSlot }() @@ -35,5 +35,11 @@ func mountHealthRoutes(r chi.Router, store *db.Store) { return } _, _ = w.Write([]byte("ok")) - }) + } + r.Get("/readyz", ready) + // Used to stage a PostgreSQL single writer before enabling rolling + // replicas. Older API images cannot pass this capability probe. + if store.Driver == "postgres" { + r.Get("/readyz/postgres-ha", ready) + } } diff --git a/api/cmd/health_test.go b/api/cmd/health_test.go index b51d2b225..f39596627 100644 --- a/api/cmd/health_test.go +++ b/api/cmd/health_test.go @@ -30,6 +30,7 @@ func TestHealthRoutesDatabaseUnavailable(t *testing.T) { } } check("/readyz", http.StatusOK, "ok") + check("/readyz/postgres-ha", http.StatusNotFound, "404 page not found\n") if err := store.Close(); err != nil { t.Fatal(err) } diff --git a/charts/gameplane/templates/api.yaml b/charts/gameplane/templates/api.yaml index 45810eb43..9777d070f 100644 --- a/charts/gameplane/templates/api.yaml +++ b/charts/gameplane/templates/api.yaml @@ -5,6 +5,7 @@ {{- end }} {{- end }} {{- if (eq (include "gameplane.apiEnabled" .) "true") }} +{{- $ha := (.Values.api.postgresHA | default dict).enabled }} apiVersion: v1 kind: ServiceAccount metadata: @@ -210,11 +211,11 @@ spec: replicas: {{ .Values.api.replicas }} selector: matchLabels: { app.kubernetes.io/name: gameplane-api } - # Every mode and database driver requires one API process: audit-chain and - # user-management locks are per-process. Recreate accepts a short outage to - # stop the old writer before starting its replacement. It is not fencing - # against forced Pod deletion or a partitioned node. strategy: + {{- if $ha }} + type: RollingUpdate + rollingUpdate: { maxUnavailable: 1, maxSurge: 0 } + {{- else }} type: Recreate # Explicit null, not omission: it states that this Deployment has no # rolling-update config, so nothing can merge one back in. @@ -230,6 +231,7 @@ spec: # runs pre-upgrade. Both halves are load-bearing; the e2e upgrade # test covers them. rollingUpdate: null + {{- end }} template: metadata: labels: @@ -238,9 +240,21 @@ spec: {{- if not .Values.api.standalone }} annotations: checksum/mtls: {{ include "gameplane.mtlsChecksum" . }} + {{- if eq .Values.api.db.driver "postgres" }} + gameplane.local/postgres-ha-protocol: "1" + {{- end }} {{- end }} spec: - {{- include "gameplane.placement" .Values.api | nindent 6 }} + {{- $placement := deepCopy .Values.api }} + {{- if $ha }} + {{- $affinity := $placement.affinity | default dict }} + {{- $anti := $affinity.podAntiAffinity | default dict }} + {{- $term := dict "topologyKey" "kubernetes.io/hostname" "labelSelector" (dict "matchLabels" (dict "app.kubernetes.io/name" "gameplane-api" "app.kubernetes.io/instance" .Release.Name)) }} + {{- $_ := set $anti "requiredDuringSchedulingIgnoredDuringExecution" (append ($anti.requiredDuringSchedulingIgnoredDuringExecution | default list) $term) }} + {{- $_ := set $affinity "podAntiAffinity" $anti }} + {{- $_ := set $placement "affinity" $affinity }} + {{- end }} + {{- include "gameplane.placement" $placement | nindent 6 }} serviceAccountName: gameplane-api {{- if .Values.api.standalone }} automountServiceAccountToken: false @@ -497,7 +511,7 @@ spec: livenessProbe: { httpGet: { path: /healthz, port: http }, initialDelaySeconds: 15 } # The DB ping has a 2s deadline. Tolerate brief contention on SQLite's # single connection before removing the only API endpoint. - readinessProbe: { httpGet: { path: /readyz, port: http }, initialDelaySeconds: 5, timeoutSeconds: 3, periodSeconds: 5, failureThreshold: 3 } + readinessProbe: { httpGet: { path: {{ ternary "/readyz/postgres-ha" "/readyz" (and (not .Values.api.standalone) (eq .Values.api.db.driver "postgres")) }}, port: http }, initialDelaySeconds: 5, timeoutSeconds: 3, periodSeconds: 5, failureThreshold: 3 } resources: {{ toYaml .Values.api.resources | nindent 12 }} securityContext: allowPrivilegeEscalation: false diff --git a/charts/gameplane/templates/crd-apply-hook.yaml b/charts/gameplane/templates/crd-apply-hook.yaml index d02015347..589bf6280 100644 --- a/charts/gameplane/templates/crd-apply-hook.yaml +++ b/charts/gameplane/templates/crd-apply-hook.yaml @@ -205,7 +205,7 @@ spec: {{- /* The target may be enabled for the first time during this upgrade. Only an existing Deployment needs migration; a lookup error fails rendering, while NotFound safely skips the patch. */}} - {{- if and (eq (include "gameplane.apiEnabled" .) "true") .Release.IsUpgrade (lookup "apps/v1" "Deployment" .Release.Namespace "gameplane-api") }} + {{- if and (eq (include "gameplane.apiEnabled" .) "true") (not (.Values.api.postgresHA | default dict).enabled) .Release.IsUpgrade (lookup "apps/v1" "Deployment" .Release.Namespace "gameplane-api") }} initContainers: # Clear the stale spec.strategy.rollingUpdate left by releases that # predate the Recreate strategy — see the Role above for why Helm diff --git a/charts/gameplane/templates/poddisruptionbudgets.yaml b/charts/gameplane/templates/poddisruptionbudgets.yaml index 35667e143..80075b25f 100644 --- a/charts/gameplane/templates/poddisruptionbudgets.yaml +++ b/charts/gameplane/templates/poddisruptionbudgets.yaml @@ -12,11 +12,13 @@ (dict "name" "telemetry-receiver" "config" $receiver "enabled" (and $apiEnabled $receiver.enabled (not .Values.api.telemetry.endpoint))) -}} {{- range $components -}} {{- $pdb := .config.pdb | default dict -}} -{{- if and .enabled $pdb.enabled -}} +{{- $ha := and (eq .name "api") ($.Values.api.postgresHA | default dict).enabled -}} +{{- if and .enabled (or $ha $pdb.enabled) -}} {{- $minimum := not (kindIs "invalid" $pdb.minAvailable) -}} {{- $maximum := not (kindIs "invalid" $pdb.maxUnavailable) -}} {{- if eq .name "api" -}} {{- if or $minimum (and $maximum (ne (toString $pdb.maxUnavailable) "1")) -}} +{{- if $ha -}}{{- fail "api.postgresHA requires an API disruption budget of maxUnavailable: 1" -}}{{- end -}} {{- fail "api.pdb must allow the singleton API to be evicted: use maxUnavailable: 1 without minAvailable" -}} {{- end -}} {{- else if and $minimum $maximum -}} diff --git a/charts/gameplane/templates/validate.yaml b/charts/gameplane/templates/validate.yaml index e96f3bc26..d1adf2b42 100644 --- a/charts/gameplane/templates/validate.yaml +++ b/charts/gameplane/templates/validate.yaml @@ -65,8 +65,29 @@ {{- if not (regexMatch "^v1\\.[0-9]+$" (.Values.podSecurity.version | default "v1.34")) }} {{- fail "podSecurity.version must pin a Kubernetes minor such as v1.34" }} {{- end }} -{{- if and (eq (include "gameplane.apiEnabled" .) "true") (ne (toString .Values.api.replicas) "1") }} -{{- fail "api requires api.replicas=1; multiple API processes are not supported" }} +{{- $ha := .Values.api.postgresHA | default dict }} +{{- if and (hasKey $ha "enabled") (not (kindIs "bool" $ha.enabled)) }} +{{- fail "api.postgresHA.enabled must be a boolean" }} +{{- end }} +{{- if $ha.enabled }} +{{- if not (and $nativeAPI (eq .Values.api.db.driver "postgres") .Values.api.db.existingSecret) }} +{{- fail "api.postgresHA.enabled requires native API mode, api.db.driver=postgres and api.db.existingSecret" }} +{{- end }} +{{- if not (and (regexMatch "^[1-9][0-9]*$" (toString .Values.api.replicas)) (ge (int64 .Values.api.replicas) 2)) }} +{{- fail "api.postgresHA.enabled requires at least two integer api.replicas" }} +{{- end }} +{{- $live := lookup "apps/v1" "Deployment" .Release.Namespace "gameplane-api" }} +{{- if and $live (ne (index ($live.spec.template.metadata.annotations | default dict) "gameplane.local/postgres-ha-protocol") "1") }} +{{- fail "First upgrade at api.replicas=1 with api.postgresHA.enabled=false and the new PostgreSQL API image, then wait for the singleton rollout before enabling HA" }} +{{- end }} +{{- if and $live (eq (int $live.spec.replicas) 1) }} +{{- $status := $live.status | default dict }} +{{- if or (ne (int64 $status.observedGeneration) (int64 $live.metadata.generation)) (ne (int $status.updatedReplicas) 1) (ne (int $status.availableReplicas) 1) (ne (int $status.replicas) 1) (gt (int $status.terminatingReplicas) 0) }} +{{- fail "Wait for the HA-capable PostgreSQL singleton rollout and its database readiness check before enabling additional writers" }} +{{- end }} +{{- end }} +{{- else if and (eq (include "gameplane.apiEnabled" .) "true") (ne (toString .Values.api.replicas) "1") }} +{{- fail "api requires api.replicas=1 unless native PostgreSQL HA is enabled" }} {{- end }} {{- if .Values.api.standalone }} {{- if ne (include "gameplane.apiEnabled" .) "true" }} diff --git a/charts/gameplane/values.yaml b/charts/gameplane/values.yaml index f27a9bccc..567acb3c7 100644 --- a/charts/gameplane/values.yaml +++ b/charts/gameplane/values.yaml @@ -175,17 +175,18 @@ api: # unless operator.controlPlaneNamespace already names an external namespace. # An explicit namespace must already exist and be dedicated to these Secrets. credentialsNamespace: "" - # Must be 1 in every enabled API install, including PostgreSQL. Audit-chain - # and user-management locks are per-process. Recreate rollouts stop the old - # API before starting its replacement and accept a brief panel outage. + # One replica with Recreate unless native PostgreSQL HA is enabled. replicas: 1 + postgresHA: + # Requires a Secret-backed PostgreSQL DSN, 2+ eligible workers and one + # API replica per worker. Stage the current image at one replica first. + enabled: false nodeSelector: {} tolerations: [] affinity: {} topologySpreadConstraints: [] priorityClassName: "" - # The API is a singleton: eviction causes downtime. A blocking budget would - # hang voluntary node drains, so only maxUnavailable: 1 is supported here. + # Optional for a singleton; HA always installs maxUnavailable: 1. pdb: { enabled: false, maxUnavailable: 1 } logLevel: info # API log verbosity (debug|info|warn|error), passed as GAMEPLANE_LOG_LEVEL # Port of the API's Prometheus metrics listener. It is separate from the diff --git a/charts/tests/check_standalone.py b/charts/tests/check_standalone.py index f0ed920c3..9b21181c7 100644 --- a/charts/tests/check_standalone.py +++ b/charts/tests/check_standalone.py @@ -43,7 +43,7 @@ def check_packaged_crds(example): def check_api_availability(): - """All API modes stay singleton, migrate old strategies, and probe the DB.""" + """Singleton upgrades and explicit native PostgreSQL HA.""" for standalone in (False, True): for driver in ("sqlite", "postgres"): values = {"operator": {"enabled": not standalone}, @@ -55,7 +55,8 @@ def check_api_availability(): assert deployment["spec"]["strategy"] == {"type": "Recreate", "rollingUpdate": None} container = next(c for c in deployment["spec"]["template"]["spec"]["containers"] if c["name"] == "api") assert container["livenessProbe"]["httpGet"]["path"] == "/healthz" - assert container["readinessProbe"]["httpGet"]["path"] == "/readyz" + expected_probe = "/readyz/postgres-ha" if driver == "postgres" and not standalone else "/readyz" + assert container["readinessProbe"]["httpGet"]["path"] == expected_probe assert container["readinessProbe"]["failureThreshold"] == 3 assert container["readinessProbe"]["periodSeconds"] == 5 assert container["readinessProbe"]["timeoutSeconds"] > 2 @@ -72,6 +73,22 @@ def check_api_availability(): render(values, failure="api requires api.replicas=1") # A remote installation with no API has no singleton requirement. render({"api": {"enabled": False, "replicas": 0}}) + ha = {"replicas": 2, "postgresHA": {"enabled": True}, "db": {"driver": "postgres", "existingSecret": "panel-dsn"}} + for upgrade in (False, True): + objects = render({"api": ha}, upgrade=upgrade) + dep = one(objects, "Deployment", "gameplane-api")["spec"] + assert dep["strategy"] == {"type": "RollingUpdate", "rollingUpdate": {"maxSurge": 0, "maxUnavailable": 1}} + pod = dep["template"]["spec"] + term = pod["affinity"]["podAntiAffinity"]["requiredDuringSchedulingIgnoredDuringExecution"][0] + assert term["topologyKey"] == "kubernetes.io/hostname" + assert term["labelSelector"]["matchLabels"]["app.kubernetes.io/name"] == "gameplane-api" + assert one(objects, "PodDisruptionBudget", "gameplane-api")["spec"]["maxUnavailable"] == 1 + assert all("persistentVolumeClaim" not in volume for volume in pod["volumes"]) + for driver in ("sqlite", "postgres"): + render({"api": {**ha, "db": {"driver": driver}}}, failure="requires native API mode") + render({"api": {**ha, "standalone": True}, "operator": {"enabled": False}}, failure="requires native API mode") + for replicas in (0, 1, -1, 1.5): + render({"api": {**ha, "replicas": replicas}}, failure="at least two integer") def main(): diff --git a/docs/install.md b/docs/install.md index 3d667e787..f3bec9cef 100644 --- a/docs/install.md +++ b/docs/install.md @@ -256,7 +256,7 @@ Top-level knobs (see `values.yaml` for the full list): GameServer enters Pending with a `PVCProvisioningFailed` condition (visible in the dashboard); no pod starts until resolved. Example: `--set operator.gameDataStorage.storageClassName=fast-nvme` -- `api.db.driver` — `sqlite` (default) or `postgres` (experimental). The API image includes both drivers. Use one API replica with PostgreSQL. +- `api.db.driver` — `sqlite` (default) or `postgres` (experimental). The API image includes both drivers. Native PostgreSQL deployments can opt into multiple API replicas once the HA-capable image has completed its singleton rollout. - `api.db.dsn` — literal connection string. The SQLite default persists to a PVC. - `api.db.existingSecret` and `api.db.secretKey` — name of a pre-provisioned Secret in the release namespace and the key holding the complete DSN (default key `dsn`). A Secret reference takes precedence over the literal DSN for both the API and `bootstrap-admin`. A missing Secret or key prevents the API container from starting. - `api.storage.existingClaim` — pre-existing PVC for the API's SQLite database (default `""`). PostgreSQL standalone deployments create no local database PVC or mount. The chart annotates `gameplane-api-data` with `helm.sh/resource-policy: keep` so switching to an existing claim preserves the previous PVC. @@ -1051,14 +1051,14 @@ and the existing database is used instead — no data loss. Nothing else needs to happen; the upgrade proceeds normally. -### API upgrades (brief downtime) +### API rollout and PostgreSQL availability -Every API Deployment uses `Recreate`: the old pod terminates before its -replacement starts. This includes combined and standalone installations with -either SQLite or PostgreSQL. The chart rejects `api.replicas` other than `1`. -Audit-chain and user-management locks are per-process; PostgreSQL and a shared -standalone key do not make multiple API writers supported. Plan a brief panel -outage on upgrade. Existing games and their operator continue independently. +The default API deployment runs one Pod and uses `Recreate`. The old Pod +terminates before its replacement starts. Plan a brief panel outage during a +singleton upgrade. Existing games and their operator continue independently. +SQLite requires one writer. Standalone installations also require one API: +the panel encryption key defaults to a private file on its retained PVC, and +standalone remote-cluster operation has only been qualified with one process. Upgrading an older combined PostgreSQL release now changes its default RollingUpdate strategy to Recreate. The existing pre-upgrade CRD hook also @@ -1073,10 +1073,34 @@ kubectl --namespace gameplane-system patch deployment gameplane-api --type=merge Recreate controls normal Kubernetes rollouts; it does not fence a writer on a partitioned node after forced Pod deletion. Stop or fence that old process -before allowing a replacement to access the database. Multi-replica API HA is -a separate feature requiring database-level transaction serialization for -audit append/prune and user-management invariants, plus coordinated background -work and rate limiting. +before allowing a replacement to access the database. + +For the native API with PostgreSQL, provision a shared writable PostgreSQL +endpoint through `api.db.existingSecret`. Supply at least as many eligible +worker nodes as API replicas. Back up PostgreSQL and test recovery before +changing writers. To upgrade an existing release, perform two distinct syncs: + +1. Deploy the HA-capable chart and API image with `api.replicas=1` and + `api.postgresHA.enabled=false`. Wait for `kubectl -n gameplane-system + rollout status deployment/gameplane-api`. Check that the running Pod's + readiness probe is `/readyz/postgres-ha` and that the Pod is Ready. This + stage serializes the transition from an older single-writer binary. Helm + checks the live deployment during the second upgrade. Offline GitOps + renderers must enforce the two syncs and wait for health between them. +2. Set `api.postgresHA.enabled=true` and `api.replicas=2` or higher. The + scheduler places each writer on a different hostname. A rolling upgrade + replaces one Pod at a time and needs no surge capacity. The API disruption + budget allows one unavailable replica. Keep a spare eligible worker if a + failed node must be replaced before it returns. + +Inspect `kubectl -n gameplane-system get pods -l app.kubernetes.io/name=gameplane-api +-o wide` to confirm the node assignments. A database outage removes the API +Pods from ready endpoints; liveness remains independent so they can reconnect +when PostgreSQL returns. Replicating the API still requires a separate +PostgreSQL failover and point-in-time recovery plan. To return to a single +writer, first deploy the same HA-capable binary with `api.replicas=1` and +`api.postgresHA.enabled=false`, then wait for the singleton rollout before +changing the application version. ### Moving a PostgreSQL DSN into a Secret From 9cbd1d88a33808acc53cbd0c35566434bc62cc11 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 00:54:39 -0500 Subject: [PATCH 04/23] fix(api): serialize authorization, audit and migrations across replicas Signed-off-by: Baylem --- api/internal/audit/audit.go | 38 ++--- api/internal/audit/transaction.go | 7 + api/internal/auth/oidc.go | 46 +++--- .../auth/oidc_management_lock_test.go | 21 ++- api/internal/db/audit_postgres_test.go | 61 ++++++++ api/internal/db/db.go | 68 +++++---- api/internal/db/db_postgres_test.go | 70 +++++++++ api/internal/db/db_test.go | 32 ++++ api/internal/db/rbac.go | 44 +++++- api/internal/db/telemetry.go | 10 +- api/internal/db/users.go | 15 +- api/internal/handlers/binding_permissions.go | 6 +- api/internal/handlers/privileged_grants.go | 20 +-- .../privileged_grants_security_test.go | 44 +++--- api/internal/handlers/roles.go | 67 ++++----- api/internal/handlers/users.go | 140 +++++++++++------- 16 files changed, 462 insertions(+), 227 deletions(-) diff --git a/api/internal/audit/audit.go b/api/internal/audit/audit.go index a360fd2e3..fc3a436b2 100644 --- a/api/internal/audit/audit.go +++ b/api/internal/audit/audit.go @@ -357,14 +357,7 @@ func computeHash(prevHash string, e Event) string { // headConfigKey) in the same transaction, so the newest-row anchor Verify // checks against is never out of sync with what was actually committed. // -// Single-writer assumption: the chain gate serializes the read-prev/compute/insert -// sequence against other requests in this process. That's correct for a -// single API replica — the only deployment topology this covers today (see -// docs/architecture.md). A multi-replica deployment would let two processes -// each read the same "previous row" concurrently and insert two rows -// chained to the same prev_hash, silently forking the chain; making that -// safe needs a DB-level serialization point (e.g. a Postgres advisory lock -// keyed on the audit_events table) in addition to this in-process gate. +// chainTransaction bounds local waiters and serializes PostgreSQL writers across API replicas. func (a *Auditor) insertChained(ctx context.Context, ts, actor, method, path, target, reason string, status int, ip string) error { return a.chainTransaction(ctx, "insert", 2*time.Second, func(ctx context.Context, tx *sql.Tx) error { return a.appendEvent(ctx, tx, Event{TS: ts, Actor: actor, Method: method, Path: path, Target: target, Status: status, IP: ip, Reason: reason}) @@ -468,10 +461,21 @@ type VerifyResult struct { // to complain about a missing successor), so after the walk it separately // checks audit.head — see verifyHead. func (a *Auditor) Verify(ctx context.Context) (VerifyResult, error) { + // Read the checkpoint, rows and head from the same point in time even + // when another replica is writing or pruning the chain. + var opts *sql.TxOptions + if a.db.Driver == "postgres" { + opts = &sql.TxOptions{Isolation: sql.LevelRepeatableRead, ReadOnly: true} + } + tx, err := a.db.DB.BeginTx(ctx, opts) + if err != nil { + return VerifyResult{}, fmt.Errorf("begin audit verification: %w", err) + } + defer func() { _ = tx.Rollback() }() expectedPrev := "" var afterID int64 - if raw, ok, err := a.db.ConfigValue(ctx, chainConfigKey); err != nil { + if raw, ok, err := db.ConfigValueTx(ctx, tx, chainConfigKey); err != nil { return VerifyResult{}, fmt.Errorf("load audit checkpoint: %w", err) } else if ok { var cp chainCheckpoint @@ -482,7 +486,7 @@ func (a *Auditor) Verify(ctx context.Context) (VerifyResult, error) { afterID = cp.ID } - rows, err := a.db.DB.QueryContext(ctx, + rows, err := tx.QueryContext(ctx, `SELECT id, ts, actor, method, path, COALESCE(target,''), status, COALESCE(ip,''), COALESCE(reason,''), COALESCE(prev_hash,''), COALESCE(hash,'') FROM audit_events WHERE id > ? AND hash IS NOT NULL AND hash <> '' @@ -518,16 +522,12 @@ func (a *Auditor) Verify(ctx context.Context) (VerifyResult, error) { if err := rows.Err(); err != nil { return VerifyResult{}, fmt.Errorf("iterate audit chain: %w", err) } - // Release the connection before verifyHead issues its own query: sqlite - // is opened with SetMaxOpenConns(1) (see db.Open), so a second query - // while rows is still open would deadlock waiting for a connection the - // still-open rows is holding. Close is idempotent, so the deferred call - // above stays as a safety net for the early returns above. + // Finish reading before the next query on this transaction. if err := rows.Close(); err != nil { return VerifyResult{}, fmt.Errorf("close audit chain rows: %w", err) } - if brk, err := a.verifyHead(ctx, afterID, checked); err != nil { + if brk, err := verifyHead(ctx, tx, afterID, checked); err != nil { return VerifyResult{}, err } else if brk != nil { return *brk, nil @@ -550,8 +550,8 @@ func (a *Auditor) Verify(ctx context.Context) (VerifyResult, error) { // row, so it is not treated as a break (this is what makes a full-table // prune, which deletes even the row the head points at, a non-event rather // than a false positive). -func (a *Auditor) verifyHead(ctx context.Context, checkpointID, checked int64) (*VerifyResult, error) { - raw, ok, err := a.db.ConfigValue(ctx, headConfigKey) +func verifyHead(ctx context.Context, tx *sql.Tx, checkpointID, checked int64) (*VerifyResult, error) { + raw, ok, err := db.ConfigValueTx(ctx, tx, headConfigKey) if err != nil { return nil, fmt.Errorf("load audit head: %w", err) } @@ -569,7 +569,7 @@ func (a *Auditor) verifyHead(ctx context.Context, checkpointID, checked int64) ( } var hash string - err = a.db.DB.QueryRowContext(ctx, + err = tx.QueryRowContext(ctx, `SELECT COALESCE(hash,'') FROM audit_events WHERE id = ?`, head.LastID, ).Scan(&hash) switch { diff --git a/api/internal/audit/transaction.go b/api/internal/audit/transaction.go index 464db6b96..73dabdbb9 100644 --- a/api/internal/audit/transaction.go +++ b/api/internal/audit/transaction.go @@ -77,6 +77,13 @@ func (a *Auditor) chainTransaction(ctx context.Context, operation string, budget return fmt.Errorf("begin audit %s: %w", operation, err) } defer func() { _ = tx.Rollback() }() + // All replicas take this transaction-scoped lock before reading the head. + // Ordinary readers remain concurrent with appends and pruning. + if a.db.Driver == "postgres" { + if _, err := tx.ExecContext(txCtx, "LOCK TABLE audit_events IN SHARE ROW EXCLUSIVE MODE"); err != nil { + return fmt.Errorf("lock audit chain: %w", err) + } + } if err := write(txCtx, tx); err != nil { return err } diff --git a/api/internal/auth/oidc.go b/api/internal/auth/oidc.go index 19d26201c..fb63b4272 100644 --- a/api/internal/auth/oidc.go +++ b/api/internal/auth/oidc.go @@ -526,8 +526,13 @@ func (o *OIDC) resolveOrLinkUser( if o.db == nil { return nil, nil, errors.New("oidc: no store attached") } + tx, err := o.db.BeginUserManagement(ctx) + if err != nil { + return nil, nil, err + } + defer func() { _ = tx.Rollback() }() var u User - err := o.db.DB.QueryRowContext(ctx, ` + err = tx.QueryRowContext(ctx, ` SELECT u.id, u.username, u.display_name, u.email, u.role FROM users u JOIN oidc_links l ON l.user_id = u.id WHERE l.issuer = ? AND l.subject = ?`, issuer, sub, @@ -535,13 +540,16 @@ func (o *OIDC) resolveOrLinkUser( if err == nil { // Existing user — re-evaluate role if mappings are configured. if syncRole { - outcome, err := o.syncUserRole(ctx, u.ID, role, matchedGroup) + outcome, err := o.syncUserRole(ctx, tx, u.ID, role, matchedGroup) if err != nil { return nil, nil, fmt.Errorf("sync role for user %d: %w", u.ID, err) } if outcome.Applied { u.Role = outcome.NewRole } + if err := tx.Commit(); err != nil { + return nil, nil, err + } return &u, outcome, nil } // No role mappings configured — return user as-is with no re-evaluation. @@ -552,16 +560,14 @@ func (o *OIDC) resolveOrLinkUser( MatchedGroup: "none", }, nil } + if !errors.Is(err, sql.ErrNoRows) { + return nil, nil, fmt.Errorf("read OIDC identity: %w", err) + } // First login — create user + link in a single tx. baseUsername := email if baseUsername == "" { baseUsername = sub } - tx, err := o.db.DB.BeginTx(ctx, nil) - if err != nil { - return nil, nil, err - } - defer func() { _ = tx.Rollback() }() // Find a username that doesn't collide with an existing local user. // Suffix with a short piece of the OIDC subject on conflict — keeps @@ -622,15 +628,11 @@ func (o *OIDC) resolveOrLinkUser( // // Re-evaluation only runs when the effective policy (post-helmOverride merge) has RoleMappings // configured. With no mappings, the user's role is assigned once at first login and never re-evaluated. -func (o *OIDC) syncUserRole(ctx context.Context, userID int64, newRole, matchedGroup string) (*RoleAssignmentOutcome, error) { - // Share the same guard as manual role changes and account deletion. - // Keep the role/count reads and the transaction in one critical section. - unlock := o.db.LockUserManagement() - defer unlock() - +func (o *OIDC) syncUserRole(ctx context.Context, tx *sql.Tx, userID int64, newRole, matchedGroup string) (*RoleAssignmentOutcome, error) { + roles := db.Roles{Reader: tx} // Fetch the user's current role to report in the outcome. var currentRole string - err := o.db.DB.QueryRowContext(ctx, `SELECT role FROM users WHERE id = ?`, userID).Scan(¤tRole) + err := tx.QueryRowContext(ctx, `SELECT role FROM users WHERE id = ?`, userID).Scan(¤tRole) if err != nil { return nil, fmt.Errorf("fetch current role: %w", err) } @@ -649,7 +651,7 @@ func (o *OIDC) syncUserRole(ctx context.Context, userID int64, newRole, matchedG } // Check if the new role grants user-management capability. - newGrantsManage, err := o.db.RoleGrantsUserManagement(ctx, newRole) + newGrantsManage, err := roles.RoleGrantsUserManagement(ctx, newRole) if err != nil { return outcome, fmt.Errorf("check target role: %w", err) } @@ -657,12 +659,12 @@ func (o *OIDC) syncUserRole(ctx context.Context, userID int64, newRole, matchedG // If the new role does NOT grant user management, check if the user currently manages users. // If so, check if they're the last one — if they are, block the demotion. if !newGrantsManage { - managesNow, err := o.db.UserManagesUsers(ctx, userID) + managesNow, err := roles.UserManagesUsers(ctx, userID) if err != nil { return outcome, fmt.Errorf("check current role: %w", err) } if managesNow { - count, err := o.db.UserManagerCount(ctx) + count, err := roles.UserManagerCount(ctx) if err != nil { return outcome, fmt.Errorf("count user managers: %w", err) } @@ -673,13 +675,6 @@ func (o *OIDC) syncUserRole(ctx context.Context, userID int64, newRole, matchedG } } - // Update the user's role in a transaction. - tx, err := o.db.DB.BeginTx(ctx, nil) - if err != nil { - return outcome, fmt.Errorf("begin: %w", err) - } - defer func() { _ = tx.Rollback() }() - if _, err := tx.ExecContext(ctx, `UPDATE users SET role = ? WHERE id = ?`, newRole, userID); err != nil { return outcome, fmt.Errorf("update role: %w", err) @@ -687,9 +682,6 @@ func (o *OIDC) syncUserRole(ctx context.Context, userID int64, newRole, matchedG if err := o.db.SetClusterRoleBinding(ctx, tx, userID, scope.DefaultCluster, newRole); err != nil { return outcome, err } - if err := tx.Commit(); err != nil { - return outcome, fmt.Errorf("commit: %w", err) - } outcome.Applied = true return outcome, nil diff --git a/api/internal/auth/oidc_management_lock_test.go b/api/internal/auth/oidc_management_lock_test.go index de13f5b0b..e0950606a 100644 --- a/api/internal/auth/oidc_management_lock_test.go +++ b/api/internal/auth/oidc_management_lock_test.go @@ -2,7 +2,6 @@ package auth import ( "context" - "sync" "testing" "time" ) @@ -20,15 +19,19 @@ func TestOIDCResyncWaitsForConcurrentUserManagement(t *testing.T) { if err := store.DB.QueryRowContext(ctx, `SELECT id FROM users WHERE username = 'oidc-admin'`).Scan(&userID); err != nil { t.Fatal(err) } + if _, err := store.DB.ExecContext(ctx, `INSERT INTO oidc_links(user_id, issuer, subject, email) VALUES (?, 'test', 'subject', 'admin@example.com')`, userID); err != nil { + t.Fatal(err) + } o := &OIDC{} o.AttachStore(store) // A manual management operation owns the guard while it removes the // other administrator. OIDC must wait before reading the manager count. - unlock := store.LockUserManagement() - var once sync.Once - release := func() { once.Do(unlock) } - defer release() + tx, err := store.BeginUserManagement(ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = tx.Rollback() }() type result struct { outcome *RoleAssignmentOutcome err error @@ -37,7 +40,7 @@ func TestOIDCResyncWaitsForConcurrentUserManagement(t *testing.T) { done := make(chan result, 1) go func() { close(started) - outcome, err := o.syncUserRole(ctx, userID, "viewer", "viewers") + _, outcome, err := o.resolveOrLinkUser(ctx, "test", "subject", "admin@example.com", "Admin", "viewer", "viewers", true) done <- result{outcome, err} }() <-started @@ -47,10 +50,12 @@ func TestOIDCResyncWaitsForConcurrentUserManagement(t *testing.T) { case <-time.After(100 * time.Millisecond): // Bound this negative assertion; do not release the guard early. } - if _, err := store.DB.ExecContext(ctx, `DELETE FROM users WHERE username = 'other-admin'`); err != nil { + if _, err := tx.ExecContext(ctx, `DELETE FROM users WHERE username = 'other-admin'`); err != nil { + t.Fatal(err) + } + if err := tx.Commit(); err != nil { t.Fatal(err) } - release() select { case got := <-done: if got.err != nil { diff --git a/api/internal/db/audit_postgres_test.go b/api/internal/db/audit_postgres_test.go index 8725a76ef..62d181ab9 100644 --- a/api/internal/db/audit_postgres_test.go +++ b/api/internal/db/audit_postgres_test.go @@ -166,3 +166,64 @@ func TestPostgresAuditBlackholeRecovery(t *testing.T) { } verify(1) // New row chains off the checkpoint. Lost attempts were not replayed. } + +func TestPostgresAuditConcurrentReplicas(t *testing.T) { + ctx := t.Context() + store := db.NewPostgresTestStore(t) + first, second := audit.New(store), audit.New(store) + write := func(writer *audit.Auditor) error { + return writer.WriteSync(ctx, http.MethodPost, "/servers", "test", "", http.StatusCreated) + } + if err := write(first); err != nil { + t.Fatal(err) + } + // A relation lock held by another server connection must fence both + // append and prune, while verification can still read the previous state. + tx, err := store.DB.BeginTx(ctx, nil) + if err != nil { + t.Fatal(err) + } + defer func() { _ = tx.Rollback() }() + if _, err := tx.ExecContext(ctx, "LOCK TABLE audit_events IN SHARE ROW EXCLUSIVE MODE"); err != nil { + t.Fatal(err) + } + short, stop := context.WithTimeout(ctx, 150*time.Millisecond) + err = second.WriteSync(short, http.MethodPost, "/servers", "", "", 200) + stop() + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("unfenced append: %v", err) + } + short, stop = context.WithTimeout(ctx, 150*time.Millisecond) + _, err = first.Prune(short, time.Now().Add(time.Hour)) + stop() + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("unfenced prune: %v", err) + } + result, err := second.Verify(ctx) + if err != nil || !result.OK || result.Checked != 1 { + t.Fatalf("verify during lock: %+v, %v", result, err) + } + if err := tx.Rollback(); err != nil { + t.Fatal(err) + } + var writers sync.WaitGroup + for _, writer := range []*audit.Auditor{first, second} { + writers.Go(func() { + for range 20 { + if err := write(writer); err != nil { + t.Error(err) + return + } + if state, err := writer.Verify(ctx); err != nil || !state.OK { + t.Errorf("verify during append: %+v, %v", state, err) + return + } + } + }) + } + writers.Wait() + result, err = first.Verify(ctx) + if err != nil || !result.OK || result.Checked != 41 { + t.Fatalf("concurrent chain: %+v, %v", result, err) + } +} diff --git a/api/internal/db/db.go b/api/internal/db/db.go index ffed7bc42..a8e34d7a7 100644 --- a/api/internal/db/db.go +++ b/api/internal/db/db.go @@ -23,7 +23,6 @@ import ( "path/filepath" "sort" "strings" - "sync" "time" // Pure-Go SQLite driver; registered as "sqlite" via database/sql. @@ -39,9 +38,8 @@ var migrations embed.FS // Store wraps the database connection and driver name, providing methods // for migrations and access to user/session/audit data. type Store struct { - DB *sql.DB - Driver string - userMgmtMu sync.Mutex // Serializes changes that can remove a user's user-management access. + DB *sql.DB + Driver string } // Open connects to a database using the specified driver and DSN. @@ -80,26 +78,41 @@ func Open(ctx context.Context, driver, dsn string) (*Store, error) { // Close closes the database connection. func (s *Store) Close() error { return s.DB.Close() } -// LockUserManagement serializes changes that can remove a user's -// user-management access (role permission edits, primary-role changes, -// user deletion). Hold it from the last-user-manager check through the -// commit of the change so two requests can't both pass the check. It -// covers one API process; every database driver requires a single API replica. -func (s *Store) LockUserManagement() (unlock func()) { - s.userMgmtMu.Lock() - return s.userMgmtMu.Unlock +// BeginUserManagement serializes role, account and binding checks with their +// writes. SQLite uses one connection. PostgreSQL releases the schema-local +// advisory lock on commit, rollback or loss of the server connection. +func (s *Store) BeginUserManagement(ctx context.Context) (*sql.Tx, error) { + return s.beginSerialized(ctx, "gameplane.user-management") +} + +func (s *Store) beginSerialized(ctx context.Context, purpose string) (*sql.Tx, error) { + tx, err := s.DB.BeginTx(ctx, nil) + if err != nil { + return nil, err + } + if s.Driver == "postgres" { + if _, err := tx.ExecContext(ctx, "SELECT pg_advisory_xact_lock(hashtext(current_schema()::text), hashtext(?))", purpose); err != nil { + _ = tx.Rollback() + return nil, fmt.Errorf("acquire %s lock: %w", purpose, err) + } + } + return tx, nil } // Migrate applies every pending migration for s.Driver, in version order: // first the driver's legacy set (migrations//, versions 001-012, // written per dialect), then the shared set (migrations/common/, 013 // onward, portable SQL that both drivers run unchanged). See -// migrations/README.md. Each file runs in a single transaction; failures -// are fatal. Afterwards it seeds the telemetry_state row once (see +// migrations/README.md. The pending files and initial seed commit atomically. It seeds the telemetry_state row once (see // seedTelemetryState), which is why it first notes whether the database was // fresh. func (s *Store) Migrate(ctx context.Context) error { - if _, err := s.DB.ExecContext(ctx, + tx, err := s.beginSerialized(ctx, "gameplane.migrations") + if err != nil { + return fmt.Errorf("begin migrations: %w", err) + } + defer func() { _ = tx.Rollback() }() + if _, err := tx.ExecContext(ctx, `CREATE TABLE IF NOT EXISTS schema_migrations (version TEXT PRIMARY KEY, applied_at TEXT NOT NULL)`, ); err != nil { return fmt.Errorf("create schema_migrations: %w", err) @@ -110,7 +123,7 @@ func (s *Store) Migrate(ctx context.Context) error { // upgrade. Counting here, before anything is applied, keeps the answer // right whichever entrypoint (bootstrap-admin or serve) migrates first. var recorded int - if err := s.DB.QueryRowContext(ctx, `SELECT COUNT(*) FROM schema_migrations`).Scan(&recorded); err != nil { + if err := tx.QueryRowContext(ctx, `SELECT COUNT(*) FROM schema_migrations`).Scan(&recorded); err != nil { return fmt.Errorf("count schema_migrations: %w", err) } fresh := recorded == 0 @@ -121,7 +134,7 @@ func (s *Store) Migrate(ctx context.Context) error { } for _, f := range files { - applied, err := s.migrationApplied(ctx, f.name) + applied, err := migrationApplied(ctx, tx, f.name) if err != nil { return fmt.Errorf("migration %s: %w", f.name, err) } @@ -132,11 +145,14 @@ func (s *Store) Migrate(ctx context.Context) error { if err != nil { return fmt.Errorf("read migration %s: %w", f.path, err) } - if err := s.runMigration(ctx, f.name, string(content)); err != nil { + if err := applyMigration(ctx, tx, f.name, string(content)); err != nil { return fmt.Errorf("migration %s: %w", f.name, err) } } - return s.seedTelemetryState(ctx, fresh) + if err := seedTelemetryState(ctx, tx, fresh); err != nil { + return err + } + return tx.Commit() } // Migration directories inside the embedded FS. legacyMigrationDirs maps a @@ -219,9 +235,9 @@ func migrationVersion(name string) string { return name[:i] } -func (s *Store) migrationApplied(ctx context.Context, name string) (bool, error) { +func migrationApplied(ctx context.Context, q Reader, name string) (bool, error) { var v string - err := s.DB.QueryRowContext(ctx, `SELECT version FROM schema_migrations WHERE version=?`, name).Scan(&v) + err := q.QueryRowContext(ctx, `SELECT version FROM schema_migrations WHERE version=?`, name).Scan(&v) if errors.Is(err, sql.ErrNoRows) { return false, nil } @@ -264,13 +280,7 @@ func NormalizeTimestamp(raw string) string { return raw } -func (s *Store) runMigration(ctx context.Context, name, sqlText string) error { - tx, err := s.DB.BeginTx(ctx, nil) - if err != nil { - return err - } - defer func() { _ = tx.Rollback() }() - +func applyMigration(ctx context.Context, tx *sql.Tx, name, sqlText string) error { for _, stmt := range splitStatements(sqlText) { if isCommentOnly(stmt) { continue @@ -284,7 +294,7 @@ func (s *Store) runMigration(ctx context.Context, name, sqlText string) error { ); err != nil { return err } - return tx.Commit() + return nil } // isCommentOnly reports whether stmt holds nothing but blank lines and diff --git a/api/internal/db/db_postgres_test.go b/api/internal/db/db_postgres_test.go index fddfaf672..6d73407e4 100644 --- a/api/internal/db/db_postgres_test.go +++ b/api/internal/db/db_postgres_test.go @@ -559,3 +559,73 @@ func TestPostgres_MigrateFromFrozenBaseline(t *testing.T) { ts, actor, method, path, target, status, ip, prevHash, rowHash, reason) } } + +// postgresPeer opens an independent pool on the same schema as the first API. +func postgresPeer(t *testing.T, original *Store) *Store { + t.Helper() + var schema string + if err := original.DB.QueryRowContext(t.Context(), "SELECT current_schema()").Scan(&schema); err != nil { + t.Fatal(err) + } + peer, err := Open(t.Context(), "postgres", dsnWithSearchPath(os.Getenv(postgresDSNEnv), schema)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = peer.Close() }) + return peer +} + +func TestPostgres_ConcurrentMigrationAndManagement(t *testing.T) { + ctx := t.Context() + first := openPostgresSchema(t) + second := postgresPeer(t, first) + start := make(chan struct{}) + results := make(chan error, 2) + for _, s := range []*Store{first, second} { + go func() { <-start; results <- s.Migrate(ctx) }() + } + close(start) + for range 2 { + if err := <-results; err != nil { + t.Fatal(err) + } + } + st, err := first.GetTelemetryState(ctx) + if err != nil || st.ConsentSource != TelemetryConsentDefault { + t.Fatalf("fresh concurrent migrations seeded %+v, %v", st, err) + } + a := pgInsertUser(t, first, "manager-a", "admin") + b := pgInsertUser(t, second, "manager-b", "admin") + tx, err := first.BeginUserManagement(ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = tx.Rollback() }() + if err := first.DeleteUserTx(ctx, tx, a); err != nil { + t.Fatal(err) + } + // The peer must not read an old count before the first deletion commits. + short, cancel := context.WithTimeout(ctx, 150*time.Millisecond) + defer cancel() + if peerTx, err := second.BeginUserManagement(short); err == nil { + _ = peerTx.Rollback() + t.Fatal("peer entered user management during an uncommitted deletion") + } else if short.Err() == nil { + t.Fatalf("lock returned before deadline: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + peerTx, err := second.BeginUserManagement(ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = peerTx.Rollback() }() + roles := Roles{Reader: peerTx} + if managed, err := roles.UserManagesUsers(ctx, b); err != nil || !managed { + t.Fatalf("remaining manager=%t, err=%v", managed, err) + } + if count, err := roles.UserManagerCount(ctx); err != nil || count != 1 { + t.Fatalf("peer saw %d managers after acquiring lock: %v", count, err) + } +} diff --git a/api/internal/db/db_test.go b/api/internal/db/db_test.go index c5227a2e2..96371908f 100644 --- a/api/internal/db/db_test.go +++ b/api/internal/db/db_test.go @@ -1,7 +1,9 @@ package db import ( + "context" "database/sql" + "fmt" "os" "path/filepath" "strings" @@ -704,3 +706,33 @@ func TestNormalizeTimestamp_ConvertsNaive(t *testing.T) { }) } } + +func (s *Store) migrationApplied(ctx context.Context, name string) (bool, error) { + return migrationApplied(ctx, s.DB, name) +} + +func (s *Store) runMigration(ctx context.Context, name, sqlText string) error { + tx, err := s.DB.BeginTx(ctx, nil) + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + + if err := applyMigration(ctx, tx, name, sqlText); err != nil { + return err + } + return tx.Commit() +} + +func (s *Store) seedTelemetryState(ctx context.Context, fresh bool) error { + tx, err := s.DB.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("seed telemetry state: begin: %w", err) + } + defer func() { _ = tx.Rollback() }() + + if err := seedTelemetryState(ctx, tx, fresh); err != nil { + return err + } + return tx.Commit() +} diff --git a/api/internal/db/rbac.go b/api/internal/db/rbac.go index b714a0a51..ec38fac2f 100644 --- a/api/internal/db/rbac.go +++ b/api/internal/db/rbac.go @@ -47,10 +47,24 @@ func (s *Store) DeleteUserBindings(ctx context.Context, ex Execer, userID int64) return nil } +// Reader is the common read interface of a pool and a transaction. +type Reader interface { + QueryRowContext(context.Context, string, ...any) *sql.Row + QueryContext(context.Context, string, ...any) (*sql.Rows, error) +} + +// Roles reads authorization state from the caller's chosen transaction or pool. +type Roles struct{ Reader } + // RoleExists reports whether a role with the given name exists. func (s *Store) RoleExists(ctx context.Context, name string) (bool, error) { + return (Roles{s.DB}).RoleExists(ctx, name) +} + +// RoleExists reports whether the named role exists in this snapshot. +func (r Roles) RoleExists(ctx context.Context, name string) (bool, error) { var x int - err := s.DB.QueryRowContext(ctx, `SELECT 1 FROM roles WHERE name = ?`, name).Scan(&x) + err := r.QueryRowContext(ctx, `SELECT 1 FROM roles WHERE name = ?`, name).Scan(&x) if errors.Is(err, sql.ErrNoRows) { return false, nil } @@ -63,8 +77,13 @@ func (s *Store) RoleExists(ctx context.Context, name string) (bool, error) { // RoleGrantsUserManagement reports whether the named role can manage // users (holds users:manage or the "*" wildcard). func (s *Store) RoleGrantsUserManagement(ctx context.Context, role string) (bool, error) { + return (Roles{s.DB}).RoleGrantsUserManagement(ctx, role) +} + +// RoleGrantsUserManagement checks the role's user-management permission. +func (r Roles) RoleGrantsUserManagement(ctx context.Context, role string) (bool, error) { var n int - err := s.DB.QueryRowContext(ctx, + err := r.QueryRowContext(ctx, `SELECT COUNT(*) FROM role_permissions WHERE role_name = ? AND permission IN ('users:manage', '*')`, role).Scan(&n) if err != nil { @@ -77,8 +96,13 @@ func (s *Store) RoleGrantsUserManagement(ctx context.Context, role string) (bool // role can manage users. No cluster filter: control-plane permissions are // not partitioned per cluster, so any cluster-wide binding confers users:manage. func (s *Store) UserManagesUsers(ctx context.Context, userID int64) (bool, error) { + return (Roles{s.DB}).UserManagesUsers(ctx, userID) +} + +// UserManagesUsers checks the user's current primary role. +func (r Roles) UserManagesUsers(ctx context.Context, userID int64) (bool, error) { var n int - err := s.DB.QueryRowContext(ctx, ` + err := r.QueryRowContext(ctx, ` SELECT COUNT(*) FROM users u JOIN role_permissions rp ON rp.role_name = u.role @@ -93,8 +117,13 @@ func (s *Store) UserManagesUsers(ctx context.Context, userID int64) (bool, error // manage users. Used to refuse demoting/deleting the last such user so // the cluster never loses all user administration. func (s *Store) UserManagerCount(ctx context.Context) (int, error) { + return (Roles{s.DB}).UserManagerCount(ctx) +} + +// UserManagerCount counts the users with user-management permission. +func (r Roles) UserManagerCount(ctx context.Context) (int, error) { var n int - err := s.DB.QueryRowContext(ctx, ` + err := r.QueryRowContext(ctx, ` SELECT COUNT(DISTINCT u.id) FROM users u JOIN role_permissions rp ON rp.role_name = u.role @@ -110,8 +139,13 @@ func (s *Store) UserManagerCount(ctx context.Context) (int, error) { // A role edit that removes users:manage uses it to check that some other // user can still manage users afterwards. func (s *Store) UserManagerCountExcludingRole(ctx context.Context, role string) (int, error) { + return (Roles{s.DB}).UserManagerCountExcludingRole(ctx, role) +} + +// UserManagerCountExcludingRole counts the managers whose primary role differs. +func (r Roles) UserManagerCountExcludingRole(ctx context.Context, role string) (int, error) { var n int - err := s.DB.QueryRowContext(ctx, ` + err := r.QueryRowContext(ctx, ` SELECT COUNT(DISTINCT u.id) FROM users u JOIN role_permissions rp ON rp.role_name = u.role diff --git a/api/internal/db/telemetry.go b/api/internal/db/telemetry.go index 675ba8f25..319ad2ffa 100644 --- a/api/internal/db/telemetry.go +++ b/api/internal/db/telemetry.go @@ -49,16 +49,11 @@ type TelemetryState struct { // schema_migrations held no rows when Migrate started. Running it again, for // example bootstrap-admin followed by serve, finds the row and changes // nothing, so a fresh install never flips to legacy. -func (s *Store) seedTelemetryState(ctx context.Context, fresh bool) error { +func seedTelemetryState(ctx context.Context, tx *sql.Tx, fresh bool) error { source := TelemetryConsentLegacy if fresh { source = TelemetryConsentDefault } - tx, err := s.DB.BeginTx(ctx, nil) - if err != nil { - return fmt.Errorf("seed telemetry state: begin: %w", err) - } - defer func() { _ = tx.Rollback() }() res, err := tx.ExecContext(ctx, `INSERT INTO telemetry_state(id, consent_source) VALUES (?, ?) ON CONFLICT (id) DO NOTHING`, @@ -91,9 +86,6 @@ func (s *Store) seedTelemetryState(ctx context.Context, fresh bool) error { } } } - if err := tx.Commit(); err != nil { - return fmt.Errorf("seed telemetry state: commit: %w", err) - } return nil } diff --git a/api/internal/db/users.go b/api/internal/db/users.go index fa1ed7544..666aaaa37 100644 --- a/api/internal/db/users.go +++ b/api/internal/db/users.go @@ -2,6 +2,7 @@ package db import ( "context" + "database/sql" "fmt" "time" ) @@ -20,6 +21,17 @@ func (s *Store) DeleteUser(ctx context.Context, userID int64) error { } defer func() { _ = tx.Rollback() }() + if err := s.DeleteUserTx(ctx, tx, userID); err != nil { + return err + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("delete user: commit: %w", err) + } + return nil +} + +// DeleteUserTx removes the account inside an existing management transaction. +func (s *Store) DeleteUserTx(ctx context.Context, tx *sql.Tx, userID int64) error { revokedAt := time.Now().UTC().Format(time.RFC3339) if _, err := tx.ExecContext(ctx, `UPDATE share_links SET revoked_at = ? WHERE created_by = ? AND revoked_at IS NULL`, @@ -45,8 +57,5 @@ func (s *Store) DeleteUser(ctx context.Context, userID int64) error { if _, err := tx.ExecContext(ctx, `DELETE FROM users WHERE id = ?`, userID); err != nil { return fmt.Errorf("delete user: users row: %w", err) } - if err := tx.Commit(); err != nil { - return fmt.Errorf("delete user: commit: %w", err) - } return nil } diff --git a/api/internal/handlers/binding_permissions.go b/api/internal/handlers/binding_permissions.go index efefa18d3..8d1ce8f89 100644 --- a/api/internal/handlers/binding_permissions.go +++ b/api/internal/handlers/binding_permissions.go @@ -22,9 +22,9 @@ func remoteWidePermissionsAllowed(permissions []string) bool { return true } -// Callers hold LockUserManagement through this read and their binding write. -func remoteWideRoleAllowed(ctx context.Context, store *db.Store, role string) (bool, error) { - rows, err := store.DB.QueryContext(ctx, `SELECT permission FROM role_permissions WHERE role_name = ?`, role) +// Callers use a management transaction for this read and their binding write. +func remoteWideRoleAllowed(ctx context.Context, q db.Reader, role string) (bool, error) { + rows, err := q.QueryContext(ctx, `SELECT permission FROM role_permissions WHERE role_name = ?`, role) if err != nil { return false, fmt.Errorf("load remote role permissions: %w", err) } diff --git a/api/internal/handlers/privileged_grants.go b/api/internal/handlers/privileged_grants.go index e6ef40bd3..b087ee4ec 100644 --- a/api/internal/handlers/privileged_grants.go +++ b/api/internal/handlers/privileged_grants.go @@ -36,9 +36,9 @@ func isFullAdmin(u *auth.User) bool { } // roleIsPrivileged reports whether role grants a privileged permission. -func roleIsPrivileged(ctx context.Context, store *db.Store, role string) (bool, error) { +func roleIsPrivileged(ctx context.Context, q db.Reader, role string) (bool, error) { var n int - if err := store.DB.QueryRowContext(ctx, + if err := q.QueryRowContext(ctx, `SELECT COUNT(*) FROM role_permissions WHERE role_name = ? AND permission IN ('*', 'users:manage', 'roles:manage', 'config:manage', 'cluster:manage')`, role).Scan(&n); err != nil { @@ -49,9 +49,9 @@ func roleIsPrivileged(ctx context.Context, store *db.Store, role string) (bool, // userIsPrivileged reports whether any role binding of the user grants a // privileged permission. -func userIsPrivileged(ctx context.Context, store *db.Store, userID int64) (bool, error) { +func userIsPrivileged(ctx context.Context, q db.Reader, userID int64) (bool, error) { var n int - if err := store.DB.QueryRowContext(ctx, + if err := q.QueryRowContext(ctx, `SELECT COUNT(*) FROM user_role_bindings b JOIN role_permissions rp ON rp.role_name = b.role_name WHERE b.user_id = ? AND rp.permission IN ('*', 'users:manage', 'roles:manage', 'config:manage', 'cluster:manage')`, @@ -63,13 +63,13 @@ func userIsPrivileged(ctx context.Context, store *db.Store, userID int64) (bool, // addsPrivileged reports whether perms contains a privileged permission that // role does not grant yet. -func addsPrivileged(ctx context.Context, store *db.Store, role string, perms []string) (bool, error) { +func addsPrivileged(ctx context.Context, q db.Reader, role string, perms []string) (bool, error) { for _, p := range perms { if !isPrivilegedPermission(p) { continue } var n int - if err := store.DB.QueryRowContext(ctx, + if err := q.QueryRowContext(ctx, `SELECT COUNT(*) FROM role_permissions WHERE role_name = ? AND permission = ?`, role, p).Scan(&n); err != nil { return false, fmt.Errorf("check permission %q on role %q: %w", p, role, err) @@ -83,11 +83,11 @@ func addsPrivileged(ctx context.Context, store *db.Store, role string, perms []s // allowRoleGrant answers 403 and returns false when a caller who is not a // full administrator tries to give someone a privileged role. -func allowRoleGrant(w http.ResponseWriter, req *http.Request, store *db.Store, role string) bool { +func allowRoleGrant(w http.ResponseWriter, req *http.Request, q db.Reader, role string) bool { if isFullAdmin(auth.UserFromContext(req.Context())) { return true } - privileged, err := roleIsPrivileged(req.Context(), store, role) + privileged, err := roleIsPrivileged(req.Context(), q, role) if err != nil { httperr.Write(w, req, err) return false @@ -103,12 +103,12 @@ func allowRoleGrant(w http.ResponseWriter, req *http.Request, store *db.Store, r // full administrator tries to change another account that holds a privileged // permission. Callers may still change their own account; the self-demotion // guards apply there. -func allowTargetChange(w http.ResponseWriter, req *http.Request, store *db.Store, targetID int64) bool { +func allowTargetChange(w http.ResponseWriter, req *http.Request, q db.Reader, targetID int64) bool { u := auth.UserFromContext(req.Context()) if isFullAdmin(u) || (u != nil && u.ID == targetID) { return true } - privileged, err := userIsPrivileged(req.Context(), store, targetID) + privileged, err := userIsPrivileged(req.Context(), q, targetID) if err != nil { httperr.Write(w, req, err) return false diff --git a/api/internal/handlers/privileged_grants_security_test.go b/api/internal/handlers/privileged_grants_security_test.go index 8007080f2..081aeb398 100644 --- a/api/internal/handlers/privileged_grants_security_test.go +++ b/api/internal/handlers/privileged_grants_security_test.go @@ -85,19 +85,19 @@ func TestPrivGrant_RoleIsPrivileged(t *testing.T) { seedRole(t, store, "pg-role-plain", "servers:read") ctx := t.Context() - if ok, err := roleIsPrivileged(ctx, store, "pg-role-config"); err != nil || !ok { + if ok, err := roleIsPrivileged(ctx, store.DB, "pg-role-config"); err != nil || !ok { t.Fatalf("config:manage role: ok=%v err=%v, want true", ok, err) } - if ok, err := roleIsPrivileged(ctx, store, "pg-role-plain"); err != nil || ok { + if ok, err := roleIsPrivileged(ctx, store.DB, "pg-role-plain"); err != nil || ok { t.Fatalf("plain role: ok=%v err=%v, want false", ok, err) } - if ok, err := roleIsPrivileged(ctx, store, "pg-role-missing"); err != nil || ok { + if ok, err := roleIsPrivileged(ctx, store.DB, "pg-role-missing"); err != nil || ok { t.Fatalf("missing role: ok=%v err=%v, want false", ok, err) } closed := newTestStore(t) _ = closed.Close() - if _, err := roleIsPrivileged(ctx, closed, "pg-role-config"); err == nil { + if _, err := roleIsPrivileged(ctx, closed.DB, "pg-role-config"); err == nil { t.Fatal("closed store: want error") } } @@ -113,19 +113,19 @@ func TestPrivGrant_UserIsPrivileged(t *testing.T) { unbound := seedUser(t, store, "pg-unbound", "viewer", "") ctx := t.Context() - if ok, err := userIsPrivileged(ctx, store, admin); err != nil || !ok { + if ok, err := userIsPrivileged(ctx, store.DB, admin); err != nil || !ok { t.Fatalf("bound privileged user: ok=%v err=%v, want true", ok, err) } - if ok, err := userIsPrivileged(ctx, store, viewer); err != nil || ok { + if ok, err := userIsPrivileged(ctx, store.DB, viewer); err != nil || ok { t.Fatalf("bound plain user: ok=%v err=%v, want false", ok, err) } - if ok, err := userIsPrivileged(ctx, store, unbound); err != nil || ok { + if ok, err := userIsPrivileged(ctx, store.DB, unbound); err != nil || ok { t.Fatalf("unbound user: ok=%v err=%v, want false", ok, err) } closed := newTestStore(t) _ = closed.Close() - if _, err := userIsPrivileged(ctx, closed, admin); err == nil { + if _, err := userIsPrivileged(ctx, closed.DB, admin); err == nil { t.Fatal("closed store: want error") } } @@ -136,21 +136,21 @@ func TestPrivGrant_AddsPrivileged(t *testing.T) { ctx := t.Context() // config:manage is not on the role yet, so it is an addition. - if ok, err := addsPrivileged(ctx, store, "pg-add-role", []string{"servers:read", "config:manage"}); err != nil || !ok { + if ok, err := addsPrivileged(ctx, store.DB, "pg-add-role", []string{"servers:read", "config:manage"}); err != nil || !ok { t.Fatalf("new privileged perm: ok=%v err=%v, want true", ok, err) } // roles:manage is already granted, so keeping it is not an addition. - if ok, err := addsPrivileged(ctx, store, "pg-add-role", []string{"roles:manage"}); err != nil || ok { + if ok, err := addsPrivileged(ctx, store.DB, "pg-add-role", []string{"roles:manage"}); err != nil || ok { t.Fatalf("already granted privileged perm: ok=%v err=%v, want false", ok, err) } // Non-privileged permissions never count. - if ok, err := addsPrivileged(ctx, store, "pg-add-role", []string{"servers:write"}); err != nil || ok { + if ok, err := addsPrivileged(ctx, store.DB, "pg-add-role", []string{"servers:write"}); err != nil || ok { t.Fatalf("non-privileged perm: ok=%v err=%v, want false", ok, err) } closed := newTestStore(t) _ = closed.Close() - if _, err := addsPrivileged(ctx, closed, "pg-add-role", []string{"users:manage"}); err == nil { + if _, err := addsPrivileged(ctx, closed.DB, "pg-add-role", []string{"users:manage"}); err == nil { t.Fatal("closed store with privileged perm: want error") } } @@ -162,19 +162,19 @@ func TestPrivGrant_AllowRoleGrant(t *testing.T) { // Full administrator may grant a privileged role. rr := httptest.NewRecorder() - if !allowRoleGrant(rr, pgGuardRequest(t, &auth.User{ID: 1, Role: rbac.RoleAdmin}), store, "pg-grant-priv") { + if !allowRoleGrant(rr, pgGuardRequest(t, &auth.User{ID: 1, Role: rbac.RoleAdmin}), store.DB, "pg-grant-priv") { t.Fatal("full admin refused a privileged grant") } // Delegated manager may grant an unprivileged role. rr = httptest.NewRecorder() - if !allowRoleGrant(rr, pgGuardRequest(t, pgUser(2, "users:manage")), store, "pg-grant-plain") { + if !allowRoleGrant(rr, pgGuardRequest(t, pgUser(2, "users:manage")), store.DB, "pg-grant-plain") { t.Fatal("delegated manager refused an unprivileged grant") } // Delegated manager may not grant a privileged role. rr = httptest.NewRecorder() - if allowRoleGrant(rr, pgGuardRequest(t, pgUser(2, "users:manage")), store, "pg-grant-priv") { + if allowRoleGrant(rr, pgGuardRequest(t, pgUser(2, "users:manage")), store.DB, "pg-grant-priv") { t.Fatal("delegated manager allowed to grant a privileged role") } if rr.Code != http.StatusForbidden { @@ -183,7 +183,7 @@ func TestPrivGrant_AllowRoleGrant(t *testing.T) { // An anonymous caller is treated as a non-admin. rr = httptest.NewRecorder() - if allowRoleGrant(rr, pgGuardRequest(t, nil), store, "pg-grant-priv") { + if allowRoleGrant(rr, pgGuardRequest(t, nil), store.DB, "pg-grant-priv") { t.Fatal("anonymous caller allowed to grant a privileged role") } if rr.Code != http.StatusForbidden { @@ -194,7 +194,7 @@ func TestPrivGrant_AllowRoleGrant(t *testing.T) { closed := newTestStore(t) _ = closed.Close() rr = httptest.NewRecorder() - if allowRoleGrant(rr, pgGuardRequest(t, pgUser(3, "users:manage")), closed, "pg-grant-priv") { + if allowRoleGrant(rr, pgGuardRequest(t, pgUser(3, "users:manage")), closed.DB, "pg-grant-priv") { t.Fatal("grant allowed although the role check failed") } } @@ -208,7 +208,7 @@ func TestPrivGrant_AllowTargetChange(t *testing.T) { // A delegated manager cannot change a privileged account. rr := httptest.NewRecorder() - if allowTargetChange(rr, pgGuardRequest(t, pgUser(10, "users:manage")), store, privileged) { + if allowTargetChange(rr, pgGuardRequest(t, pgUser(10, "users:manage")), store.DB, privileged) { t.Fatal("delegated manager allowed to change a privileged account") } if rr.Code != http.StatusForbidden { @@ -217,19 +217,19 @@ func TestPrivGrant_AllowTargetChange(t *testing.T) { // A delegated manager may change an ordinary account. rr = httptest.NewRecorder() - if !allowTargetChange(rr, pgGuardRequest(t, pgUser(10, "users:manage")), store, plain) { + if !allowTargetChange(rr, pgGuardRequest(t, pgUser(10, "users:manage")), store.DB, plain) { t.Fatal("delegated manager refused an ordinary account") } // A caller may always change their own account, even a privileged one. rr = httptest.NewRecorder() - if !allowTargetChange(rr, pgGuardRequest(t, pgUser(privileged, "users:manage")), store, privileged) { + if !allowTargetChange(rr, pgGuardRequest(t, pgUser(privileged, "users:manage")), store.DB, privileged) { t.Fatal("self change refused") } // A full administrator may change any account. rr = httptest.NewRecorder() - if !allowTargetChange(rr, pgGuardRequest(t, &auth.User{ID: 1, Role: rbac.RoleAdmin}), store, privileged) { + if !allowTargetChange(rr, pgGuardRequest(t, &auth.User{ID: 1, Role: rbac.RoleAdmin}), store.DB, privileged) { t.Fatal("full admin refused a privileged account change") } @@ -237,7 +237,7 @@ func TestPrivGrant_AllowTargetChange(t *testing.T) { closed := newTestStore(t) _ = closed.Close() rr = httptest.NewRecorder() - if allowTargetChange(rr, pgGuardRequest(t, pgUser(10, "users:manage")), closed, privileged) { + if allowTargetChange(rr, pgGuardRequest(t, pgUser(10, "users:manage")), closed.DB, privileged) { t.Fatal("change allowed although the privilege check failed") } } diff --git a/api/internal/handlers/roles.go b/api/internal/handlers/roles.go index ddf019184..63d465406 100644 --- a/api/internal/handlers/roles.go +++ b/api/internal/handlers/roles.go @@ -137,7 +137,7 @@ func (h *roleHandler) create(w http.ResponseWriter, req *http.Request) { } } - tx, err := h.db.DB.BeginTx(req.Context(), nil) + tx, err := h.db.BeginUserManagement(req.Context()) if err != nil { httperr.Write(w, req, err) return @@ -169,7 +169,18 @@ type roleUpdateReq struct { func (h *roleHandler) update(w http.ResponseWriter, req *http.Request) { name := chi.URLParam(req, "name") - builtin, found, err := h.roleBuiltin(req, name) + var body roleUpdateReq + if err := json.NewDecoder(req.Body).Decode(&body); err != nil { + httperr.WriteMessage(w, req, messages.New("server.bad_request", nil)) + return + } + tx, err := h.db.BeginUserManagement(req.Context()) + if err != nil { + httperr.Write(w, req, err) + return + } + defer func() { _ = tx.Rollback() }() + builtin, found, err := roleBuiltin(req, tx, name) if err != nil { httperr.Write(w, req, err) return @@ -185,22 +196,13 @@ func (h *roleHandler) update(w http.ResponseWriter, req *http.Request) { return } - var body roleUpdateReq - if err := json.NewDecoder(req.Body).Decode(&body); err != nil { - httperr.WriteMessage(w, req, messages.New("server.bad_request", nil)) - return - } if body.Permissions != nil { if msg, ok := validatePermissions(*body.Permissions); !ok { httperr.WriteCode(w, req, http.StatusBadRequest, errors.New(msg)) return } - // Lock to serialize with writes so the last-user-manager check can't race, - // and so the privileged-permission check reads what the write replaces. - unlock := h.db.LockUserManagement() - defer unlock() if !isFullAdmin(auth.UserFromContext(req.Context())) { - adds, err := addsPrivileged(req.Context(), h.db, name, *body.Permissions) + adds, err := addsPrivileged(req.Context(), tx, name, *body.Permissions) if err != nil { httperr.Write(w, req, err) return @@ -212,7 +214,7 @@ func (h *roleHandler) update(w http.ResponseWriter, req *http.Request) { } if !remoteWidePermissionsAllowed(*body.Permissions) { var remoteBindings int - if err := h.db.DB.QueryRowContext(req.Context(), + if err := tx.QueryRowContext(req.Context(), `SELECT COUNT(*) FROM user_role_bindings WHERE role_name = ? AND namespace = '*' AND cluster <> ?`, name, scope.DefaultCluster).Scan(&remoteBindings); err != nil { httperr.Write(w, req, err) @@ -223,7 +225,7 @@ func (h *roleHandler) update(w http.ResponseWriter, req *http.Request) { return } } - if msg, err := h.userManagementGuard(req, name, *body.Permissions); err != nil { + if msg, err := userManagementGuard(req, db.Roles{Reader: tx}, name, *body.Permissions); err != nil { httperr.Write(w, req, err) return } else if msg != "" { @@ -232,13 +234,6 @@ func (h *roleHandler) update(w http.ResponseWriter, req *http.Request) { } } - tx, err := h.db.DB.BeginTx(req.Context(), nil) - if err != nil { - httperr.Write(w, req, err) - return - } - defer func() { _ = tx.Rollback() }() - if body.Description != nil { if _, err := tx.ExecContext(req.Context(), `UPDATE roles SET description = ?, updated_at = ? WHERE name = ?`, @@ -286,9 +281,13 @@ func (h *roleHandler) del(w http.ResponseWriter, req *http.Request) { name := chi.URLParam(req, "name") // Keep the in-use check and deletion atomic with supplemental grants. // Otherwise deleting/recreating a role could widen an orphaned binding. - unlock := h.db.LockUserManagement() - defer unlock() - builtin, found, err := h.roleBuiltin(req, name) + tx, err := h.db.BeginUserManagement(req.Context()) + if err != nil { + httperr.Write(w, req, err) + return + } + defer func() { _ = tx.Rollback() }() + builtin, found, err := roleBuiltin(req, tx, name) if err != nil { httperr.Write(w, req, err) return @@ -302,7 +301,7 @@ func (h *roleHandler) del(w http.ResponseWriter, req *http.Request) { return } var inUse int - if err := h.db.DB.QueryRowContext(req.Context(), + if err := tx.QueryRowContext(req.Context(), `SELECT COUNT(*) FROM user_role_bindings WHERE role_name = ?`, name).Scan(&inUse); err != nil { httperr.Write(w, req, err) return @@ -312,12 +311,6 @@ func (h *roleHandler) del(w http.ResponseWriter, req *http.Request) { return } - tx, err := h.db.DB.BeginTx(req.Context(), nil) - if err != nil { - httperr.Write(w, req, err) - return - } - defer func() { _ = tx.Rollback() }() // sqlite has no FK cascade, so drop the permission rows explicitly. if _, err := tx.ExecContext(req.Context(), `DELETE FROM role_permissions WHERE role_name = ?`, name); err != nil { httperr.Write(w, req, err) @@ -334,9 +327,9 @@ func (h *roleHandler) del(w http.ResponseWriter, req *http.Request) { w.WriteHeader(http.StatusNoContent) } -func (h *roleHandler) roleBuiltin(req *http.Request, name string) (builtin, found bool, err error) { +func roleBuiltin(req *http.Request, q db.Reader, name string) (builtin, found bool, err error) { var b int - err = h.db.DB.QueryRowContext(req.Context(), `SELECT builtin FROM roles WHERE name = ?`, name).Scan(&b) + err = q.QueryRowContext(req.Context(), `SELECT builtin FROM roles WHERE name = ?`, name).Scan(&b) if errors.Is(err, sql.ErrNoRows) { return false, false, nil } @@ -367,11 +360,11 @@ func validatePermissions(perms []string) (string, bool) { // either the role is the caller's own primary role, or every user who can // manage users holds that role. An empty message means the change may // proceed. -func (h *roleHandler) userManagementGuard(req *http.Request, role string, perms []string) (string, error) { +func userManagementGuard(req *http.Request, roles db.Roles, role string, perms []string) (string, error) { if permsGrantUserManagement(perms) { return "", nil } - grantsNow, err := h.db.RoleGrantsUserManagement(req.Context(), role) + grantsNow, err := roles.RoleGrantsUserManagement(req.Context(), role) if err != nil { return "", err } @@ -381,11 +374,11 @@ func (h *roleHandler) userManagementGuard(req *http.Request, role string, perms if caller := auth.UserFromContext(req.Context()); caller != nil && caller.Role == role { return "cannot remove your own user-management access", nil } - total, err := h.db.UserManagerCount(req.Context()) + total, err := roles.UserManagerCount(req.Context()) if err != nil { return "", err } - others, err := h.db.UserManagerCountExcludingRole(req.Context(), role) + others, err := roles.UserManagerCountExcludingRole(req.Context(), role) if err != nil { return "", err } diff --git a/api/internal/handlers/users.go b/api/internal/handlers/users.go index 3edca91a4..d744834f6 100644 --- a/api/internal/handlers/users.go +++ b/api/internal/handlers/users.go @@ -136,16 +136,6 @@ func (h *userHandler) create(w http.ResponseWriter, req *http.Request) { if body.Role == "" { body.Role = "viewer" } - if ok, err := h.db.RoleExists(req.Context(), body.Role); err != nil { - httperr.Write(w, req, err) - return - } else if !ok { - httperr.WriteMessage(w, req, messages.New("server.users.invalid_role", nil)) - return - } - if !allowRoleGrant(w, req, h.db, body.Role) { - return - } var hash string if body.Password != "" { if len(body.Password) < auth.MinPasswordLen { @@ -159,11 +149,27 @@ func (h *userHandler) create(w http.ResponseWriter, req *http.Request) { } hash = h2 } + tx, err := h.db.BeginUserManagement(req.Context()) + if err != nil { + httperr.Write(w, req, err) + return + } + defer func() { _ = tx.Rollback() }() + if ok, err := (db.Roles{Reader: tx}).RoleExists(req.Context(), body.Role); err != nil { + httperr.Write(w, req, err) + return + } else if !ok { + httperr.WriteMessage(w, req, messages.New("server.users.invalid_role", nil)) + return + } + if !allowRoleGrant(w, req, tx, body.Role) { + return + } // RETURNING id instead of LastInsertId: pgx's database/sql driver has // no LastInsertId, and both SQLite and Postgres support RETURNING. var id int64 var createdAt string - if err := h.db.DB.QueryRowContext(req.Context(), + if err := tx.QueryRowContext(req.Context(), `INSERT INTO users(username, display_name, email, role, pw_hash) VALUES (?, ?, ?, ?, ?) RETURNING id, created_at`, body.Username, body.DisplayName, body.Email, body.Role, nullable(hash), ).Scan(&id, &createdAt); err != nil { @@ -173,7 +179,11 @@ func (h *userHandler) create(w http.ResponseWriter, req *http.Request) { // Mirror the primary role into a cluster-wide ("*") role binding so // RBAC resolves the new user's permissions. Without this the user has // no effective permissions at all. - if err := h.db.SetClusterRoleBinding(req.Context(), nil, id, scope.DefaultCluster, body.Role); err != nil { + if err := h.db.SetClusterRoleBinding(req.Context(), tx, id, scope.DefaultCluster, body.Role); err != nil { + httperr.Write(w, req, err) + return + } + if err := tx.Commit(); err != nil { httperr.Write(w, req, err) return } @@ -536,18 +546,22 @@ func (h *userHandler) del(w http.ResponseWriter, req *http.Request) { httperr.WriteMessage(w, req, messages.New("server.users.cannot_delete_self", nil)) return } - if !allowTargetChange(w, req, h.db, id) { + tx, err := h.db.BeginUserManagement(req.Context()) + if err != nil { + httperr.Write(w, req, err) + return + } + defer func() { _ = tx.Rollback() }() + roles := db.Roles{Reader: tx} + if !allowTargetChange(w, req, tx, id) { return } - // Lock to serialize with writes so the last-user-manager check can't race. - unlock := h.db.LockUserManagement() - defer unlock() // Don't delete the last user who can manage users. - if managesNow, err := h.db.UserManagesUsers(req.Context(), id); err != nil { + if managesNow, err := roles.UserManagesUsers(req.Context(), id); err != nil { httperr.Write(w, req, err) return } else if managesNow { - count, err := h.db.UserManagerCount(req.Context()) + count, err := roles.UserManagerCount(req.Context()) if err != nil { httperr.Write(w, req, err) return @@ -560,7 +574,11 @@ func (h *userHandler) del(w http.ResponseWriter, req *http.Request) { // One transaction removes the account and every row tied to it (SSO // links, preferences, sessions, bindings) and revokes its share links: // sqlite runs without FK cascade, so nothing else would. - if err := h.db.DeleteUser(req.Context(), id); err != nil { + if err := h.db.DeleteUserTx(req.Context(), tx, id); err != nil { + httperr.Write(w, req, err) + return + } + if err := tx.Commit(); err != nil { httperr.Write(w, req, err) return } @@ -593,8 +611,15 @@ func (h *userHandler) update(w http.ResponseWriter, req *http.Request) { return } } + tx, err := h.db.BeginUserManagement(req.Context()) + if err != nil { + httperr.Write(w, req, err) + return + } + defer func() { _ = tx.Rollback() }() + roles := db.Roles{Reader: tx} if body.Role != nil { - if ok, err := h.db.RoleExists(req.Context(), *body.Role); err != nil { + if ok, err := roles.RoleExists(req.Context(), *body.Role); err != nil { httperr.Write(w, req, err) return } else if !ok { @@ -602,18 +627,15 @@ func (h *userHandler) update(w http.ResponseWriter, req *http.Request) { return } } - if !allowTargetChange(w, req, h.db, id) { + if !allowTargetChange(w, req, tx, id) { return } - if body.Role != nil && !allowRoleGrant(w, req, h.db, *body.Role) { + if body.Role != nil && !allowRoleGrant(w, req, tx, *body.Role) { return } caller := auth.UserFromContext(req.Context()) if body.Role != nil { - // Lock to serialize with writes so the last-user-manager check can't race. - unlock := h.db.LockUserManagement() - defer unlock() - newGrantsManage, err := h.db.RoleGrantsUserManagement(req.Context(), *body.Role) + newGrantsManage, err := roles.RoleGrantsUserManagement(req.Context(), *body.Role) if err != nil { httperr.Write(w, req, err) return @@ -629,11 +651,11 @@ func (h *userHandler) update(w http.ResponseWriter, req *http.Request) { // System-wide safety net: never demote the last user who can manage // users, or the cluster loses all user administration. if !newGrantsManage { - if managesNow, err := h.db.UserManagesUsers(req.Context(), id); err != nil { + if managesNow, err := roles.UserManagesUsers(req.Context(), id); err != nil { httperr.Write(w, req, err) return } else if managesNow { - count, err := h.db.UserManagerCount(req.Context()) + count, err := roles.UserManagerCount(req.Context()) if err != nil { httperr.Write(w, req, err) return @@ -646,13 +668,6 @@ func (h *userHandler) update(w http.ResponseWriter, req *http.Request) { } } - tx, err := h.db.DB.BeginTx(req.Context(), nil) - if err != nil { - httperr.Write(w, req, err) - return - } - defer func() { _ = tx.Rollback() }() - if body.DisplayName != nil { if _, err := tx.ExecContext(req.Context(), `UPDATE users SET display_name = ?, updated_at = ? WHERE id = ?`, @@ -735,7 +750,7 @@ func (h *userHandler) resetPassword(w http.ResponseWriter, req *http.Request) { httperr.WriteMessage(w, req, messages.New("server.users.password_too_short", nil)) return } - if !allowTargetChange(w, req, h.db, id) { + if !allowTargetChange(w, req, h.db.DB, id) { return } hash, err := auth.HashPassword(req.Context(), body.Password) @@ -747,12 +762,15 @@ func (h *userHandler) resetPassword(w http.ResponseWriter, req *http.Request) { // Bumping session_epoch invalidates every session issued before this // reset, including one a concurrent login that verified the old password // is still creating; deleting the rows in the same transaction is cleanup. - tx, err := h.db.DB.BeginTx(req.Context(), nil) + tx, err := h.db.BeginUserManagement(req.Context()) if err != nil { httperr.Write(w, req, err) return } defer func() { _ = tx.Rollback() }() + if !allowTargetChange(w, req, tx, id) { + return + } res, err := tx.ExecContext(req.Context(), `UPDATE users SET pw_hash = ?, session_epoch = session_epoch + 1, updated_at = ? WHERE id = ?`, hash, db.NowTimestamp(), id) @@ -855,24 +873,24 @@ func (h *userHandler) addBinding(w http.ResponseWriter, req *http.Request) { httperr.WriteMessage(w, req, messages.New("server.cluster_forbidden", nil)) return } - if remoteWide { - // Serialize permission validation + insertion with role edits/deletion, - // so a safe role cannot become a control-plane grant between them. - unlock := h.db.LockUserManagement() - defer unlock() + tx, err := h.db.BeginUserManagement(req.Context()) + if err != nil { + httperr.Write(w, req, err) + return } - if ok, err := h.db.RoleExists(req.Context(), body.RoleName); err != nil { + defer func() { _ = tx.Rollback() }() + if ok, err := (db.Roles{Reader: tx}).RoleExists(req.Context(), body.RoleName); err != nil { httperr.Write(w, req, err) return } else if !ok { httperr.WriteMessage(w, req, messages.New("server.users.invalid_role", nil)) return } - if !allowRoleGrant(w, req, h.db, body.RoleName) || !allowTargetChange(w, req, h.db, id) { + if !allowRoleGrant(w, req, tx, body.RoleName) || !allowTargetChange(w, req, tx, id) { return } if remoteWide { - if allowed, err := remoteWideRoleAllowed(req.Context(), h.db, body.RoleName); err != nil { + if allowed, err := remoteWideRoleAllowed(req.Context(), tx, body.RoleName); err != nil { httperr.Write(w, req, err) return } else if !allowed { @@ -880,7 +898,7 @@ func (h *userHandler) addBinding(w http.ResponseWriter, req *http.Request) { return } } - if ok, err := h.userExists(req.Context(), id); err != nil { + if ok, err := userExists(req.Context(), tx, id); err != nil { httperr.Write(w, req, err) return } else if !ok { @@ -888,7 +906,7 @@ func (h *userHandler) addBinding(w http.ResponseWriter, req *http.Request) { return } var dup int - if err := h.db.DB.QueryRowContext(req.Context(), + if err := tx.QueryRowContext(req.Context(), `SELECT COUNT(*) FROM user_role_bindings WHERE user_id = ? AND role_name = ? AND cluster = ? AND namespace = ?`, id, body.RoleName, body.Cluster, body.Namespace).Scan(&dup); err != nil { httperr.Write(w, req, err) @@ -898,12 +916,16 @@ func (h *userHandler) addBinding(w http.ResponseWriter, req *http.Request) { httperr.WriteMessage(w, req, messages.New("server.users.binding_already_exists", nil)) return } - if _, err := h.db.DB.ExecContext(req.Context(), + if _, err := tx.ExecContext(req.Context(), `INSERT INTO user_role_bindings(user_id, role_name, cluster, namespace) VALUES (?, ?, ?, ?)`, id, body.RoleName, body.Cluster, body.Namespace); err != nil { httperr.Write(w, req, err) return } + if err := tx.Commit(); err != nil { + httperr.Write(w, req, err) + return + } h.invalidateSessions(req, id, "role binding added") writeJSONCreated(w, bindingDTO(body)) } @@ -928,15 +950,19 @@ func (h *userHandler) deleteBinding(w http.ResponseWriter, req *http.Request) { httperr.WriteMessage(w, req, messages.New("server.cluster_forbidden", nil)) return } - if !allowTargetChange(w, req, h.db, id) { + tx, err := h.db.BeginUserManagement(req.Context()) + if err != nil { + httperr.Write(w, req, err) + return + } + defer func() { _ = tx.Rollback() }() + if !allowTargetChange(w, req, tx, id) { return } if namespace == "*" { - unlock := h.db.LockUserManagement() - defer unlock() // Legacy externally provisioned global grants are not supplemental // remote readers; retain their previous removal protection. - if allowed, err := remoteWideRoleAllowed(req.Context(), h.db, role); err != nil { + if allowed, err := remoteWideRoleAllowed(req.Context(), tx, role); err != nil { httperr.Write(w, req, err) return } else if !allowed { @@ -944,7 +970,7 @@ func (h *userHandler) deleteBinding(w http.ResponseWriter, req *http.Request) { return } } - res, err := h.db.DB.ExecContext(req.Context(), + res, err := tx.ExecContext(req.Context(), `DELETE FROM user_role_bindings WHERE user_id = ? AND role_name = ? AND cluster = ? AND namespace = ?`, id, role, cluster, namespace) if err != nil { @@ -955,13 +981,17 @@ func (h *userHandler) deleteBinding(w http.ResponseWriter, req *http.Request) { httperr.WriteMessage(w, req, messages.New("server.users.binding_not_found", nil)) return } + if err := tx.Commit(); err != nil { + httperr.Write(w, req, err) + return + } h.invalidateSessions(req, id, "role binding removed") w.WriteHeader(http.StatusNoContent) } -func (h *userHandler) userExists(ctx context.Context, id int64) (bool, error) { +func userExists(ctx context.Context, q db.Reader, id int64) (bool, error) { var x int - err := h.db.DB.QueryRowContext(ctx, `SELECT 1 FROM users WHERE id = ?`, id).Scan(&x) + err := q.QueryRowContext(ctx, `SELECT 1 FROM users WHERE id = ?`, id).Scan(&x) if errors.Is(err, sql.ErrNoRows) { return false, nil } From ef025cfd67f2ecc168e361bfb2fbffd763801c67 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 01:01:04 -0500 Subject: [PATCH 05/23] test(api): qualify active-active PostgreSQL with pod, worker and database loss Signed-off-by: Baylem --- .github/workflows/ci.yaml | 21 +- charts/gameplane/templates/api.yaml | 9 + charts/tests/check_standalone.py | 2 + deploy/kind/e2e.sh | 14 +- test/e2e/buckets.sh | 4 +- test/e2e/postgres_ha_e2e_test.go | 426 ++++++++++++++++++++++++++++ test/e2e/suite-components.json | 1 + 7 files changed, 470 insertions(+), 7 deletions(-) create mode 100644 test/e2e/postgres_ha_e2e_test.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b22bb014b..e25b59bb8 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -2459,7 +2459,7 @@ jobs: needs: [changes, build-images] if: (needs.changes.outputs.e2e == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-postgres') runs-on: ubuntu-latest - timeout-minutes: 75 # Boots PostgreSQL and runs auth, roles, RBAC, then availability. + timeout-minutes: 105 # Sequential singleton/PITR and isolated multi-worker failover drills. permissions: contents: read env: @@ -2538,6 +2538,23 @@ jobs: context: ${{ env.CTX }} namespaces: "gameplane-system,gameplane-games" include-previous-logs: "true" + - name: replace the PITR cluster with an isolated HA cluster + run: | + ./deploy/kind/e2e.sh down "$CLUSTER" + ./deploy/kind/e2e.sh up gameplane-postgres-ha e2e postgres-ha + - name: run go e2e (postgres-ha) + run: | + RUN_REGEX="$(./test/e2e/buckets.sh regex postgres-ha)" + cd test/e2e + GAMEPLANE_E2E_REUSE_CLUSTER=1 GAMEPLANE_E2E_CLUSTER=gameplane-postgres-ha \ + go test -tags=e2e -timeout 20m -v -parallel 1 -run "$RUN_REGEX" ./... + - name: dump the HA cluster on failure + if: failure() + uses: ./.github/actions/dump-cluster-state + with: + context: kind-gameplane-postgres-ha + namespaces: "gameplane-system,gameplane-games" + include-previous-logs: "true" # Smoke-tests the dev ingress-nginx manifest pin (deploy/kind/up.sh, # INGRESS_NGINX_VERSION) on the same Kind node config `make dev-up` uses @@ -2953,6 +2970,8 @@ jobs: if (availability && availability.conclusion === 'success') bucketSet.add('postgres-availability'); const pitr = j.steps.find((s) => s.name === 'run go e2e (postgres-pitr)'); if (pitr && pitr.conclusion === 'success') bucketSet.add('postgres-pitr'); + const ha = j.steps.find((s) => s.name === 'run go e2e (postgres-ha)'); + if (ha && ha.conclusion === 'success') bucketSet.add('postgres-ha'); } // The ratelimit bucket runs as a trailing step inside the // api-auth job leg (deliberately, so it drains the shared diff --git a/charts/gameplane/templates/api.yaml b/charts/gameplane/templates/api.yaml index 9777d070f..e89e71828 100644 --- a/charts/gameplane/templates/api.yaml +++ b/charts/gameplane/templates/api.yaml @@ -209,6 +209,9 @@ metadata: labels: {{- include "gameplane.labels" . | nindent 4 }} spec: replicas: {{ .Values.api.replicas }} + {{- if $ha }} + minReadySeconds: 5 + {{- end }} selector: matchLabels: { app.kubernetes.io/name: gameplane-api } strategy: @@ -509,6 +512,12 @@ spec: - { name: http, containerPort: 8000 } - { name: metrics, containerPort: {{ .Values.api.metricsPort | default 9090 }} } livenessProbe: { httpGet: { path: /healthz, port: http }, initialDelaySeconds: 15 } + {{- if $ha }} + # Leave time for the terminating endpoint to leave kube-proxy's + # backend set before SIGTERM starts the HTTP server's shutdown. + lifecycle: + preStop: { sleep: { seconds: 5 } } + {{- end }} # The DB ping has a 2s deadline. Tolerate brief contention on SQLite's # single connection before removing the only API endpoint. readinessProbe: { httpGet: { path: {{ ternary "/readyz/postgres-ha" "/readyz" (and (not .Values.api.standalone) (eq .Values.api.db.driver "postgres")) }}, port: http }, initialDelaySeconds: 5, timeoutSeconds: 3, periodSeconds: 5, failureThreshold: 3 } diff --git a/charts/tests/check_standalone.py b/charts/tests/check_standalone.py index 9b21181c7..189fb99a6 100644 --- a/charts/tests/check_standalone.py +++ b/charts/tests/check_standalone.py @@ -78,7 +78,9 @@ def check_api_availability(): objects = render({"api": ha}, upgrade=upgrade) dep = one(objects, "Deployment", "gameplane-api")["spec"] assert dep["strategy"] == {"type": "RollingUpdate", "rollingUpdate": {"maxSurge": 0, "maxUnavailable": 1}} + assert dep["minReadySeconds"] == 5 pod = dep["template"]["spec"] + assert next(c for c in pod["containers"] if c["name"] == "api")["lifecycle"]["preStop"]["sleep"]["seconds"] == 5 term = pod["affinity"]["podAntiAffinity"]["requiredDuringSchedulingIgnoredDuringExecution"][0] assert term["topologyKey"] == "kubernetes.io/hostname" assert term["labelSelector"]["matchLabels"]["app.kubernetes.io/name"] == "gameplane-api" diff --git a/deploy/kind/e2e.sh b/deploy/kind/e2e.sh index 254796069..1a2c897a4 100755 --- a/deploy/kind/e2e.sh +++ b/deploy/kind/e2e.sh @@ -32,9 +32,9 @@ kindnet | cilium) ;; *) echo "unsupported E2E CNI: ${CNI}" >&2; exit 2 ;; esac case "${PROFILE}" in -default | postgres | two-node | management) ;; +default | postgres | postgres-ha | two-node | management) ;; *) - echo "unknown profile '${PROFILE}' (want default, postgres, two-node or management)" >&2 + echo "unknown profile '${PROFILE}' (want default, postgres, postgres-ha, two-node or management)" >&2 exit 2 ;; esac @@ -202,7 +202,7 @@ up) extra_nodes="" if [[ "${PROFILE}" == "two-node" ]]; then extra_nodes=" - role: worker" - elif [[ "${PROFILE}" == "management" ]]; then + elif [[ "${PROFILE}" == "management" || "${PROFILE}" == "postgres-ha" ]]; then extra_nodes=$' - role: worker\n - role: worker' fi kind create cluster --name "${CLUSTER}" --wait "${kind_wait}" --config - < Date: Wed, 11 Nov 2026 01:02:04 -0500 Subject: [PATCH 06/23] feat(api): coordinate authentication and periodic workers across PostgreSQL replicas Signed-off-by: Baylem --- .github/workflows/ci.yaml | 2 +- api/cmd/main.go | 39 ++- api/cmd/runtime_postgres_test.go | 227 ++++++++++++++++++ api/cmd/workers.go | 80 ++++++ api/internal/auth/local.go | 34 +-- .../auth/local_login_device_security_test.go | 2 +- api/internal/auth/oidc.go | 12 +- api/internal/auth/oidc_rolemap_test.go | 6 +- api/internal/auth/postgres_runtime.go | 111 +++++++++ api/internal/auth/ratelimit.go | 37 +-- api/internal/auth/registry.go | 118 ++++++--- api/internal/auth/registry_test.go | 51 ++++ api/internal/auth/sessions.go | 74 +++++- .../db/migrations/common/020_api_runtime.sql | 12 + docs/panel-recovery.md | 30 +++ 15 files changed, 747 insertions(+), 88 deletions(-) create mode 100644 api/cmd/runtime_postgres_test.go create mode 100644 api/cmd/workers.go create mode 100644 api/internal/auth/postgres_runtime.go create mode 100644 api/internal/db/migrations/common/020_api_runtime.sql diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b22bb014b..70b2aae81 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -1775,7 +1775,7 @@ jobs: working-directory: api env: GAMEPLANE_TEST_POSTGRES_DSN: postgres://postgres@localhost:5432/gameplane_test?sslmode=disable - run: go test -tags postgres -count=1 ./internal/db/... ./internal/controlplane/... + run: go test -tags postgres -count=1 ./internal/db/... ./internal/controlplane/... ./cmd # --------------------------------------------------------------------- # kind e2e — one job per suite, each with its own throwaway cluster on diff --git a/api/cmd/main.go b/api/cmd/main.go index a4a08e944..4be86cc05 100644 --- a/api/cmd/main.go +++ b/api/cmd/main.go @@ -13,6 +13,7 @@ import ( "os/signal" "strconv" "strings" + "sync" "syscall" "time" @@ -196,7 +197,6 @@ func main() { } sessions := auth.NewSessionStore(store) - sessions.StartGC(ctx, time.Hour) localAuth := auth.NewLocal(store) // The Helm-flag OIDC provider (if any) is built once at startup; the // registry lists it as the read-only "helm" provider next to the @@ -279,7 +279,6 @@ func main() { // configured in Admin Settings. Best-effort mirror — never gates // reconciliation or requests. notifier := notify.New(store, k8s, credentialsNS) - go notifier.RunWithRegistry(ctx, reg) // Cluster registry: watch Cluster CRDs and populate the client registry // with remote cluster connections. Single-cluster installs only have the @@ -305,6 +304,14 @@ func main() { r := chi.NewRouter() r.Use(middleware.RequestID, middleware.Recoverer) + if store.Driver == "postgres" { + shared, err := auth.PostgresRuntime(ctx, store) + if err != nil { + logger.Error("initialize shared authentication state", "err", err) + os.Exit(1) + } + r.Use(shared) + } // Client IP middleware runs before rate limiting and audit so the determined // IP is used for rate-limit buckets and audit records. X-Forwarded-For is // read only when the TCP peer is inside the trusted proxy networks, which @@ -371,6 +378,9 @@ func main() { // Protected API r.Group(func(p chi.Router) { p.Use(sessions.Authenticate) + if store.Driver == "postgres" { + p.Use(sessions.RevalidateStreams(isStreamingRequest, 5*time.Second)) + } // A broad per-IP cap on writes keeps one authenticated client from // pegging the DB or k8s API. Reads go through unlimited (the main // bottleneck there is the dashboard's own polling). @@ -439,18 +449,30 @@ func main() { // disabled or none. Otherwise it sends nothing while the admin's basic // toggle is off or, on a fresh install, until an admin was shown the // notice (spec 022 FR-004); each report slot is claimed in the database. - go telemetry.New(telemetry.Config{ + reporter := telemetry.New(telemetry.Config{ Dest: telemetryDest, Interval: cfg.telemetryInterval, Auth: cfg.telemetryAuth, Deps: telemetryDeps, - }).Run(ctx) + }) // Opt-in audit-event retention. Off by default (0 days = keep forever); // when set, a daily sweep prunes events past the window so the table // doesn't grow unbounded on long-lived installs. - go auditor.RunRetention(ctx, - time.Duration(cfg.auditRetentionDays)*24*time.Hour, 24*time.Hour, logger) + workersDone := make(chan struct{}) + go func() { + defer close(workersDone) + runPanelWorkers(ctx, store, 2*time.Second, logger, func(leaderCtx context.Context) { + var jobs sync.WaitGroup + jobs.Go(func() { sessions.RunGC(leaderCtx, time.Hour) }) + jobs.Go(func() { notify.New(store, k8s, credentialsNS).RunWithRegistry(leaderCtx, reg) }) + jobs.Go(func() { reporter.Run(leaderCtx) }) + jobs.Go(func() { + auditor.RunRetention(leaderCtx, time.Duration(cfg.auditRetentionDays)*24*time.Hour, 24*time.Hour, logger) + }) + jobs.Wait() + }) + }() srv := &http.Server{ Addr: cfg.addr, @@ -492,6 +514,11 @@ func main() { shutCtx, shutCancel := context.WithTimeout(context.Background(), 10*time.Second) defer shutCancel() _ = srv.Shutdown(shutCtx) + select { + case <-workersDone: + case <-shutCtx.Done(): + logger.Warn("panel workers did not stop before shutdown deadline") + } if metricsSrv != nil { _ = metricsSrv.Shutdown(shutCtx) } diff --git a/api/cmd/runtime_postgres_test.go b/api/cmd/runtime_postgres_test.go new file mode 100644 index 000000000..db77913c5 --- /dev/null +++ b/api/cmd/runtime_postgres_test.go @@ -0,0 +1,227 @@ +//go:build postgres + +package main + +import ( + "context" + "crypto/rand" + "encoding/hex" + "fmt" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/GameplanePanel/gameplane/api/internal/auth" + "github.com/GameplanePanel/gameplane/api/internal/db" +) + +// Each instance has its own pool and auth middleware, like two API processes. +func runtimeReplicas(t *testing.T) [2]*db.Store { + t.Helper() + dsn := os.Getenv("GAMEPLANE_TEST_POSTGRES_DSN") + if dsn == "" { + t.Skip("GAMEPLANE_TEST_POSTGRES_DSN not set") + } + open := func(dsn string) *db.Store { + s, err := db.Open(t.Context(), "postgres", dsn) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = s.Close() }) + return s + } + admin := open(dsn) + var suffix [6]byte + if _, err := rand.Read(suffix[:]); err != nil { + t.Fatal(err) + } + schema := "gp_runtime_" + hex.EncodeToString(suffix[:]) + if _, err := admin.DB.ExecContext(t.Context(), "CREATE SCHEMA "+schema); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if _, err := admin.DB.ExecContext(context.WithoutCancel(t.Context()), "DROP SCHEMA "+schema+" CASCADE"); err != nil { + t.Error(err) + } + }) + if strings.Contains(dsn, "://") { + sep := "?" + if strings.Contains(dsn, "?") { + sep = "&" + } + dsn += sep + "search_path=" + schema + } else { + dsn += " search_path=" + schema + } + stores := [2]*db.Store{open(dsn), open(dsn)} + if err := stores[0].Migrate(t.Context()); err != nil { + t.Fatal(err) + } + return stores +} + +func TestPostgresRuntimeAcrossReplicas(t *testing.T) { + stores := runtimeReplicas(t) + ctx := t.Context() + hash, err := auth.HashPassword(ctx, "replica-test") + if err != nil { + t.Fatal(err) + } + if _, err = stores[0].DB.ExecContext(ctx, `INSERT INTO users(username,display_name,email,pw_hash) VALUES ('qa','','',?)`, hash); err != nil { + t.Fatal(err) + } + var muxes [2]http.Handler + started, ended := make(chan struct{}), make(chan struct{}) + for i, s := range stores { + runtime, err := auth.PostgresRuntime(ctx, s) + if err != nil { + t.Fatal(err) + } + sessions := auth.NewSessionStore(s) + mux := http.NewServeMux() + login := auth.NewLocal(s).HandleLogin(sessions, auth.NewRegistry(s, nil, nil, "", nil)) + mux.Handle("POST /login", auth.LoginLimiter.Middleware(login)) + mux.Handle("POST /logout", sessions.Authenticate(sessions.HandleLogout())) + mux.Handle("GET /me", sessions.Authenticate(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(200) }))) + revalidate := sessions.RevalidateStreams(func(*http.Request) bool { return true }, 20*time.Millisecond) + stream := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + close(started) + <-r.Context().Done() + close(ended) + }) + mux.Handle("GET /stream", sessions.Authenticate(revalidate(stream))) + mux.Handle("GET /hit", auth.LoginLimiter.Middleware(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(204) }))) + muxes[i] = runtime(mux) + } + call := func(replica int, method, path, ip, password string, cookies ...*http.Cookie) *httptest.ResponseRecorder { + body := fmt.Sprintf(`{"username":"qa","password":%q}`, password) + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req.RemoteAddr = ip + ":1234" + req.Header.Set("Content-Type", "application/json") + for _, c := range cookies { + req.AddCookie(c) + if c.Name == "gameplane_csrf" { + req.Header.Set("X-Gameplane-CSRF", c.Value) + } + } + rec := httptest.NewRecorder() + muxes[replica].ServeHTTP(rec, req) + return rec + } + // 32 simultaneous requests admitted to the same IP must share exactly 10 tokens. + var group sync.WaitGroup + var admitted, limited atomic.Int32 + for i := range 32 { + group.Go(func() { + switch call(i%2, "GET", "/hit", "192.0.2.1", "").Code { + case http.StatusNoContent: + admitted.Add(1) + case http.StatusTooManyRequests: + limited.Add(1) + } + }) + } + group.Wait() + if admitted.Load() != 10 || limited.Load() != 22 { + t.Fatalf("race admitted=%d limited=%d", admitted.Load(), limited.Load()) + } + login := call(0, "POST", "/login", "192.0.2.2", "replica-test") + if login.Code != 200 { + t.Fatalf("login status %d", login.Code) + } + response := login.Result() + defer func() { _ = response.Body.Close() }() + cookies := response.Cookies() + for i := range 5 { + if got := call(i%2, "POST", "/login", fmt.Sprintf("192.0.2.%d", 10+i), "wrong").Code; got != 401 { + t.Fatalf("wrong password %d: %d", i, got) + } + } + if got := call(1, "POST", "/login", "192.0.2.20", "replica-test").Code; got != 429 { + t.Fatalf("exhausted user budget: %d", got) + } + if got := call(1, "POST", "/login", "192.0.2.21", "replica-test", &http.Cookie{Name: "gameplane_login_device", Value: "forged"}).Code; got != 429 { + t.Fatalf("forged known-device cookie: %d", got) + } + if got := call(1, "POST", "/login", "192.0.2.22", "replica-test", cookies...).Code; got != 200 { + t.Fatalf("recognized device on peer: %d", got) + } + if got := call(1, "GET", "/me", "192.0.2.23", "", cookies...).Code; got != 200 { + t.Fatalf("cross-replica session: %d", got) + } + go call(0, "GET", "/stream", "192.0.2.23", "", cookies...) + waitSignal(t, started) + if got := call(1, "POST", "/logout", "192.0.2.23", "", cookies...).Code; got != 204 { + t.Fatalf("logout on peer: %d", got) + } + waitSignal(t, ended) + if got := call(0, "GET", "/me", "192.0.2.23", "", cookies...).Code; got != 401 { + t.Fatalf("revoked session on original replica: %d", got) + } + if _, err := stores[1].DB.ExecContext(ctx, `INSERT INTO config(key,value) VALUES('auth','{"providers":[{"name":"local","kind":"local","enabled":false}]}') ON CONFLICT(key) DO UPDATE SET value=EXCLUDED.value`); err != nil { + t.Fatal(err) + } + if got := call(0, "POST", "/login", "192.0.2.24", "replica-test", cookies...).Code; got != 403 { + t.Fatalf("provider disabled on peer: %d", got) + } + if err := stores[0].Close(); err != nil { + t.Fatal(err) + } + if got := call(0, "GET", "/hit", "192.0.2.40", "").Code; got != 503 { + t.Fatalf("lost database created local budget: %d", got) + } +} + +func waitSignal(t *testing.T, signal <-chan struct{}) { + t.Helper() + select { + case <-signal: + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for replica") + } +} + +func TestPostgresWorkersRecoverWhenLeaderBackendDies(t *testing.T) { + stores := runtimeReplicas(t) + first, cancelFirst := context.WithCancel(t.Context()) + defer cancelFirst() + second, cancelSecond := context.WithCancel(t.Context()) + defer cancelSecond() + leader, stopped, standby, finished := make(chan struct{}), make(chan struct{}), make(chan struct{}), make(chan struct{}) + go runPanelWorkers(first, stores[0], 100*time.Millisecond, slog.Default(), func(ctx context.Context) { + close(leader) + <-ctx.Done() + cancelFirst() + close(stopped) + }) + waitSignal(t, leader) + go func() { + defer close(finished) + runPanelWorkers(second, stores[1], 100*time.Millisecond, slog.Default(), func(ctx context.Context) { + close(standby) + <-ctx.Done() + }) + }() + select { + case <-standby: + t.Fatal("both replicas became leader while original lock was held") + case <-time.After(300 * time.Millisecond): + } + var killed bool + err := stores[1].DB.QueryRowContext(t.Context(), `SELECT pg_terminate_backend(pid) FROM pg_locks + WHERE locktype='advisory' AND database=(SELECT oid FROM pg_database WHERE datname=current_database()) + AND classid=1196447563 AND objid=hashtext(current_schema())::oid AND objsubid=2 AND granted`).Scan(&killed) + if err != nil || !killed { + t.Fatalf("terminate original leader connection: %v, %v", killed, err) + } + waitSignal(t, stopped) + waitSignal(t, standby) + cancelSecond() + waitSignal(t, finished) +} diff --git a/api/cmd/workers.go b/api/cmd/workers.go new file mode 100644 index 000000000..33992cbb8 --- /dev/null +++ b/api/cmd/workers.go @@ -0,0 +1,80 @@ +package main + +import ( + "context" + "database/sql" + "errors" + "log/slog" + "time" + + "github.com/GameplanePanel/gameplane/api/internal/db" +) + +// runPanelWorkers keeps the install's periodic jobs on one replica. A read- +// committed transaction pins its PostgreSQL backend even with transaction +// pooling. The transaction-scoped lock is released on rollback or backend +// disconnect. Keeping workers out of this transaction leaves their writes +// short-lived and avoids holding a repeatable-read snapshot through a tenure. +func runPanelWorkers(ctx context.Context, store *db.Store, interval time.Duration, logger *slog.Logger, workers func(context.Context)) { + if store.Driver != "postgres" { + workers(ctx) + return + } + ticker := time.NewTicker(interval) + defer ticker.Stop() + for ctx.Err() == nil { + if err := workerTenure(ctx, store.DB, interval, workers); err != nil && ctx.Err() == nil { + logger.Warn("panel worker election", "err", err) + } + select { + case <-ctx.Done(): + return + case <-ticker.C: + } + } +} + +func workerTenure(ctx context.Context, database *sql.DB, interval time.Duration, workers func(context.Context)) error { + // The transaction's context outlives the workers on an orderly shutdown. + // Otherwise database/sql could release the lock while they were draining. + txCtx, cancelTx := context.WithCancel(context.WithoutCancel(ctx)) + defer cancelTx() + // Bound pool checkout and the initial BEGIN even during failover. + deadline := time.AfterFunc(interval, cancelTx) + tx, err := database.BeginTx(txCtx, &sql.TxOptions{Isolation: sql.LevelReadCommitted, ReadOnly: true}) + deadline.Stop() + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + probe, cancelProbe := context.WithTimeout(ctx, interval) + var elected bool + // The first key reserves Gameplane worker election ('GPWK'); the second + // partitions installations that use distinct schemas in one database. + err = tx.QueryRowContext(probe, `SELECT pg_try_advisory_xact_lock(1196447563,hashtext(current_schema()))`).Scan(&elected) + cancelProbe() + if err != nil || !elected { + return err + } + workerCtx, cancelWorkers := context.WithCancel(ctx) + done := make(chan struct{}) + go func() { defer close(done); workers(workerCtx) }() + defer func() { cancelWorkers(); <-done }() + heartbeat := time.NewTicker(interval) + defer heartbeat.Stop() + for { + select { + case <-ctx.Done(): + return ctx.Err() + case <-done: + return errors.New("panel workers stopped before their tenure ended") + case <-heartbeat.C: + ping, cancel := context.WithTimeout(ctx, interval) + _, err := tx.ExecContext(ping, "SELECT 1") + cancel() + if err != nil { + return err + } + } + } +} diff --git a/api/internal/auth/local.go b/api/internal/auth/local.go index 20951e06d..41d841363 100644 --- a/api/internal/auth/local.go +++ b/api/internal/auth/local.go @@ -110,9 +110,16 @@ type loginResp struct { // provider-level, not per-user, so it can't become an enumeration oracle. func (l *Local) HandleLogin(sessions *SessionStore, reg *Registry) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { - if reg != nil && !reg.LocalEnabled(req.Context()) { - httperr.WriteCode(w, req, http.StatusForbidden, errors.New("login method disabled")) - return + if reg != nil { + enabled, err := reg.localEnabled(req.Context()) + if err != nil { + httperr.WriteMessage(w, req, messages.New("server.generic.503", nil)) + return + } + if !enabled { + httperr.WriteCode(w, req, http.StatusForbidden, errors.New("login method disabled")) + return + } } // Enforce JSON content-type — a cross-site form POST can carry // Cookie but not application/json (without CORS preflight), so @@ -137,9 +144,7 @@ func (l *Local) HandleLogin(sessions *SessionStore, reg *Registry) http.HandlerF // login-device cookie and skips the per-username bucket (the per-IP // LoginLimiter still applies), so failed logins from elsewhere can't // lock the account's owner out. - if body.Username != "" && !knownLoginDevice(req, body.Username) && !LoginUserLimiter.AllowUser(body.Username) { - w.Header().Set("Retry-After", "30") - httperr.WriteCode(w, req, http.StatusTooManyRequests, errors.New("too many requests")) + if body.Username != "" && !knownLoginDevice(req, body.Username) && !LoginUserLimiter.requestBudget(w, req, body.Username) { return } // Every branch below that returns 401 must first spend the argon2 @@ -177,7 +182,7 @@ func (l *Local) HandleLogin(sessions *SessionStore, reg *Registry) http.HandlerF } setSessionCookie(w, token, sessionTTL) setCSRFCookie(w, csrf, sessionTTL) - setLoginDeviceCookie(w, body.Username) + setLoginDeviceCookie(w, req, body.Username) // Attribute the audit row to the user who just signed in; the // request itself is unauthenticated, so it would read "anonymous". SetActor(req.Context(), u.Username) @@ -206,9 +211,8 @@ func (l *Local) fetchUser(ctx context.Context, username string) (*User, string, } // loginDeviceCookie marks a browser that has signed in to an account -// before. Its value is an HMAC of the username under a per-process key, so -// it can't be forged and reveals nothing; a restart only means the next -// login from each browser counts against the per-username bucket once. +// before. SQLite signs it with a process-local key. PostgreSQL replicas +// share a stored signing key and authenticate the cookie's expiry. const loginDeviceCookie = "gameplane_login_device" // loginDeviceTTL is how long a browser stays known after a sign-in. @@ -220,8 +224,8 @@ var loginDeviceKey = func() []byte { return b }() -func loginDeviceToken(username string) string { - mac := hmac.New(sha256.New, loginDeviceKey) +func loginDeviceToken(username string, key []byte) string { + mac := hmac.New(sha256.New, key) _, _ = mac.Write([]byte(username)) return base64.RawURLEncoding.EncodeToString(mac.Sum(nil)) } @@ -233,13 +237,13 @@ func knownLoginDevice(req *http.Request, username string) bool { if err != nil { return false } - return hmac.Equal([]byte(c.Value), []byte(loginDeviceToken(username))) + return validDeviceCookie(req.Context(), username, c.Value) } -func setLoginDeviceCookie(w http.ResponseWriter, username string) { +func setLoginDeviceCookie(w http.ResponseWriter, req *http.Request, username string) { http.SetCookie(w, &http.Cookie{ Name: loginDeviceCookie, - Value: loginDeviceToken(username), + Value: issueDeviceCookie(req.Context(), username), Path: "/auth/login", MaxAge: loginDeviceTTL, HttpOnly: true, diff --git a/api/internal/auth/local_login_device_security_test.go b/api/internal/auth/local_login_device_security_test.go index 472ae30be..82d680f35 100644 --- a/api/internal/auth/local_login_device_security_test.go +++ b/api/internal/auth/local_login_device_security_test.go @@ -10,7 +10,7 @@ import ( // same account only. func TestLoginDevice_CookieRoundTrip(t *testing.T) { rr := httptest.NewRecorder() - setLoginDeviceCookie(rr, "dev-alice") + setLoginDeviceCookie(rr, httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/auth/login", nil), "dev-alice") var issued *http.Cookie for _, c := range rr.Result().Cookies() { diff --git a/api/internal/auth/oidc.go b/api/internal/auth/oidc.go index 19d26201c..2cce5ac98 100644 --- a/api/internal/auth/oidc.go +++ b/api/internal/auth/oidc.go @@ -14,6 +14,7 @@ import ( "github.com/GameplanePanel/gameplane/api/internal/db" "github.com/GameplanePanel/gameplane/api/internal/httperr" + "github.com/GameplanePanel/gameplane/api/internal/messages" "github.com/GameplanePanel/gameplane/api/internal/scope" "github.com/GameplanePanel/gameplane/netguard" ) @@ -65,7 +66,7 @@ type OIDC struct { db *db.Store auditWriteSync func(ctx context.Context, method, path, target, reason string, status int) error providerName string - getHelmRoleOverrides func(context.Context) *RoleMappings + getHelmRoleOverrides func(context.Context) (*RoleMappings, error) } // NewOIDCWithPolicy is NewOIDC carrying a group→role mapping policy. The @@ -347,7 +348,7 @@ func (o *OIDC) SetProviderName(name string) { o.providerName = name } // AttachHelmRoleOverridesFunc attaches a function that reads the current Helm-seeded // role-mapping overrides from the auth config row. Called on every login for the Helm // provider to deliver SC-007 (no restart needed after admin edit). -func (o *OIDC) AttachHelmRoleOverridesFunc(fn func(context.Context) *RoleMappings) { +func (o *OIDC) AttachHelmRoleOverridesFunc(fn func(context.Context) (*RoleMappings, error)) { o.getHelmRoleOverrides = fn } @@ -454,7 +455,12 @@ func (o *OIDC) HandleCallbackAt(sessions *SessionStore, cookiePath string) http. // on every login attempt (SC-007: no cache, read at login time). resolvePolicy := o.policy if o.providerName == HelmProviderName && o.getHelmRoleOverrides != nil { - if helmOverride := o.getHelmRoleOverrides(req.Context()); helmOverride != nil { + helmOverride, err := o.getHelmRoleOverrides(req.Context()) + if err != nil { + httperr.WriteMessage(w, req, messages.New("server.generic.503", nil)) + return + } + if helmOverride != nil { resolvePolicy = effectiveHelmPolicy(o.policy, helmOverride) } } diff --git a/api/internal/auth/oidc_rolemap_test.go b/api/internal/auth/oidc_rolemap_test.go index e26a0ee93..d3443ca88 100644 --- a/api/internal/auth/oidc_rolemap_test.go +++ b/api/internal/auth/oidc_rolemap_test.go @@ -1119,8 +1119,8 @@ func TestHandleCallback_HelmOverride_LiveRead_SC007(t *testing.T) { // Mutable override source: held in a closure so we can mutate it between logins. var helmOverride *RoleMappings - getOverride := func(_ context.Context) *RoleMappings { - return helmOverride + getOverride := func(_ context.Context) (*RoleMappings, error) { + return helmOverride, nil } o, err := NewOIDCWithPolicy(context.Background(), idp.issuer(), "client-1", "secret", @@ -1216,7 +1216,7 @@ func TestHandleCallback_HelmOverrideMappingsReEvaluateWithoutHelmMappings(t *tes t.Fatalf("NewOIDCWithPolicy: %v", err) } o.SetProviderName(HelmProviderName) - o.AttachHelmRoleOverridesFunc(func(context.Context) *RoleMappings { return override }) + o.AttachHelmRoleOverridesFunc(func(context.Context) (*RoleMappings, error) { return override, nil }) rec := &auditWriteRecorder{} o.AttachAuditWriteSyncFunc(rec.write) diff --git a/api/internal/auth/postgres_runtime.go b/api/internal/auth/postgres_runtime.go new file mode 100644 index 000000000..21a2e7984 --- /dev/null +++ b/api/internal/auth/postgres_runtime.go @@ -0,0 +1,111 @@ +package auth + +import ( + "context" + "crypto/hmac" + "crypto/rand" + "crypto/sha256" + "database/sql" + "encoding/hex" + "errors" + "fmt" + "math" + "net/http" + "strconv" + "strings" + "time" + + "github.com/GameplanePanel/gameplane/api/internal/db" + "github.com/GameplanePanel/gameplane/api/internal/httperr" + "github.com/GameplanePanel/gameplane/api/internal/messages" +) + +type runtimeKey struct{} + +type sharedAuth struct { + store *db.Store + deviceKey []byte +} + +func issueDeviceCookie(ctx context.Context, username string) string { + state, _ := ctx.Value(runtimeKey{}).(*sharedAuth) + if state == nil { + return loginDeviceToken(username, loginDeviceKey) + } + expires := strconv.FormatInt(time.Now().Add(loginDeviceTTL*time.Second).Unix(), 10) + return expires + "." + loginDeviceToken(username+"\x00"+expires, state.deviceKey) +} + +func validDeviceCookie(ctx context.Context, username, value string) bool { + state, _ := ctx.Value(runtimeKey{}).(*sharedAuth) + if state == nil { + return hmac.Equal([]byte(value), []byte(loginDeviceToken(username, loginDeviceKey))) + } + expires, signature, ok := strings.Cut(value, ".") + deadline, err := strconv.ParseInt(expires, 10, 64) + return ok && err == nil && deadline > time.Now().Unix() && hmac.Equal([]byte(signature), []byte(loginDeviceToken(username+"\x00"+expires, state.deviceKey))) +} + +// PostgresRuntime shares budgets and known-device signatures across all API replicas. +func PostgresRuntime(ctx context.Context, store *db.Store) (func(http.Handler) http.Handler, error) { + candidate := make([]byte, 32) + if _, err := rand.Read(candidate); err != nil { + return nil, err + } + _, err := store.DB.ExecContext(ctx, `INSERT INTO api_runtime_keys (purpose,key_hex) VALUES ('login_device',?) ON CONFLICT (purpose) DO NOTHING`, hex.EncodeToString(candidate)) + if err != nil { + return nil, fmt.Errorf("initialize login device key: %w", err) + } + var encoded string + if err := store.DB.QueryRowContext(ctx, `SELECT key_hex FROM api_runtime_keys WHERE purpose='login_device'`).Scan(&encoded); err != nil { + return nil, fmt.Errorf("read login device key: %w", err) + } + key, err := hex.DecodeString(encoded) + if err != nil || len(key) != 32 { + return nil, errors.New("invalid login device key in database") + } + state := &sharedAuth{store: store, deviceKey: key} + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), runtimeKey{}, state))) + }) + }, nil +} + +// check consumes a token using PostgreSQL's clock and row-level conflict handling. +// next_us is the database time at which all previously spent tokens refill. +// An exhausted bucket leaves its timestamp unchanged. Losing a database never +// creates a new local budget. +func (t *TokenBucket) check(r *http.Request, key string) (bool, error) { + state, _ := r.Context().Value(runtimeKey{}).(*sharedAuth) + if state == nil { + return t.Allow(key), nil + } + sum := sha256.Sum256([]byte(key)) + step := int64(math.Round(float64(time.Second/time.Microsecond) / t.rate)) + var next int64 + err := state.store.DB.QueryRowContext(r.Context(), ` + INSERT INTO api_rate_buckets (budget, subject_hash, next_us) + VALUES (?, ?, floor(extract(epoch FROM statement_timestamp())*1000000)::bigint + CAST(? AS bigint)) + ON CONFLICT (budget,subject_hash) DO UPDATE + SET next_us=GREATEST(api_rate_buckets.next_us+CAST(? AS bigint), EXCLUDED.next_us) + WHERE api_rate_buckets.next_us <= EXCLUDED.next_us+CAST(? AS bigint) + RETURNING next_us`, t.name, hex.EncodeToString(sum[:]), step, step, (int64(t.burst)-2)*step).Scan(&next) + if errors.Is(err, sql.ErrNoRows) { + return false, nil + } + return err == nil, err +} + +func (t *TokenBucket) requestBudget(w http.ResponseWriter, r *http.Request, key string) bool { + ok, err := t.check(r, key) + if err != nil { + httperr.WriteMessage(w, r, messages.New("server.generic.503", nil)) + return false + } + if !ok { + w.Header().Set("Retry-After", "30") + httperr.WriteCode(w, r, http.StatusTooManyRequests, errors.New("too many requests")) + } + return ok +} diff --git a/api/internal/auth/ratelimit.go b/api/internal/auth/ratelimit.go index 2501e8281..2a09c13ba 100644 --- a/api/internal/auth/ratelimit.go +++ b/api/internal/auth/ratelimit.go @@ -1,7 +1,6 @@ package auth import ( - "errors" "net" "net/http" "strconv" @@ -21,9 +20,10 @@ import ( // // Sized for the login path, where legitimate traffic is tiny but // attackers will fire as fast as the argon2 cost allows. Keeping this -// in-process is fine for single-replica deployments; HA setups with -// multiple API pods should replace this with Redis later. +// in-process handles SQLite. PostgreSQL requests use the same budget names, +// rates and bursts through the shared store. type TokenBucket struct { + name string rate float64 // tokens per second burst float64 @@ -46,6 +46,12 @@ func newTokenBucket(rate, burst float64) *TokenBucket { } } +func namedBucket(name string, rate, burst float64) *TokenBucket { + b := newTokenBucket(rate, burst) + b.name = name + return b +} + // Allow returns true if the key has tokens available and reserves one, false otherwise. func (t *TokenBucket) Allow(key string) bool { t.mu.Lock() @@ -100,9 +106,7 @@ func (t *TokenBucket) Middleware(next http.Handler) http.Handler { clientIP = host } } - if !t.Allow(clientIP) { - w.Header().Set("Retry-After", "30") - httperr.WriteCode(w, req, http.StatusTooManyRequests, errors.New("too many requests")) + if !t.requestBudget(w, req, clientIP) { return } next.ServeHTTP(w, req) @@ -112,35 +116,35 @@ func (t *TokenBucket) Middleware(next http.Handler) http.Handler { // LoginLimiter caps login attempts per client IP. 5/min with a burst of // 10 tolerates accidental retries but starves a brute-forcer of argon2 // cycles quickly. -var LoginLimiter = newTokenBucket(5.0/60.0, 10) +var LoginLimiter = namedBucket("login_ip", 5.0/60.0, 10) // LoginUserLimiter caps login attempts per *username*, layered on top of // LoginLimiter. Without it, an attacker spread across a botnet would // still burn through usernames at full speed because each IP has its // own bucket. 3/min with a burst of 6 blunts that without tripping on // a user fat-fingering their password. -var LoginUserLimiter = newTokenBucket(3.0/60.0, 6) +var LoginUserLimiter = namedBucket("login_user", 3.0/60.0, 6) // OIDCCallbackLimiter guards /auth/oidc/callback. Each redirect back // from the IdP triggers a token exchange + DB writes; flooding it can // overwhelm the IdP and the local DB. Tight burst; generous refill. -var OIDCCallbackLimiter = newTokenBucket(10.0/60.0, 10) +var OIDCCallbackLimiter = namedBucket("oidc", 10.0/60.0, 10) // NotifyTestLimiter guards the notification test-send endpoint. Each hit // dials an admin-configured external endpoint synchronously, so it gets a // far tighter budget than ordinary mutations: enough to poke every sink // while setting things up, not enough to use the API as a POST cannon. -var NotifyTestLimiter = newTokenBucket(12.0/60.0, 3) +var NotifyTestLimiter = namedBucket("notify_test", 12.0/60.0, 3) // MutationLimiter caps authenticated writes per IP. The login bucket is // narrow (argon2 is expensive); this one is broad — 60/min with 60 // burst — and only exists to keep a single client from pegging the // database on a mass create/delete loop. -var MutationLimiter = newTokenBucket(1.0, 60) // 60/min refill, 60 burst +var MutationLimiter = namedBucket("mutation", 1.0, 60) // 60/min refill, 60 burst // NewFleetReadLimiter creates a budget shared by all fleet reads on one router: // 60/min sustained, burst 10. It is independent of login and mutation budgets. -func NewFleetReadLimiter() *TokenBucket { return newTokenBucket(1.0, 10) } +func NewFleetReadLimiter() *TokenBucket { return namedBucket("fleet_read", 1.0, 10) } // UserMiddleware enforces a bucket keyed by the authenticated account rather // than IP or session, so separate users behind one proxy do not share a budget. @@ -151,7 +155,12 @@ func (t *TokenBucket) UserMiddleware(next http.Handler) http.Handler { httperr.WriteMessage(w, req, messages.New("server.common.unauthenticated", nil)) return } - if !t.AllowUser(strconv.FormatInt(u.ID, 10)) { + allowed, err := t.check(req, strconv.FormatInt(u.ID, 10)) + if err != nil { + httperr.WriteMessage(w, req, messages.New("server.generic.503", nil)) + return + } + if !allowed { w.Header().Set("Retry-After", "1") httperr.WriteMessage(w, req, messages.New("server.common.too_many_requests", nil)) return @@ -165,7 +174,7 @@ func (t *TokenBucket) UserMiddleware(next http.Handler) http.Handler { // mutations but higher than login attempts. 20/min with burst of 20 allows // legitimate players to retry connection attempts while keeping brute-force // costs reasonable. -var ShareLimiter = newTokenBucket(20.0/60.0, 20) +var ShareLimiter = namedBucket("public_share", 20.0/60.0, 20) // AllowUser is a helper for rate-limit checks keyed on something other // than the IP (e.g. username). Returns the same bool as Middleware's diff --git a/api/internal/auth/registry.go b/api/internal/auth/registry.go index 2b79dfb76..aaf25ed4b 100644 --- a/api/internal/auth/registry.go +++ b/api/internal/auth/registry.go @@ -168,9 +168,7 @@ func NewRegistry(store *db.Store, secrets SecretReader, legacy *OIDC, legacyLabe // helm role overrides function so it can be read on every login (SC-007). if legacy != nil { legacy.AttachStore(store) - legacy.AttachHelmRoleOverridesFunc(func(ctx context.Context) *RoleMappings { - return reg.HelmRoleOverrides(ctx) - }) + legacy.AttachHelmRoleOverridesFunc(reg.HelmRoleOverrides) } return reg } @@ -188,57 +186,72 @@ func (r *Registry) AttachAuditWriteSyncFunc(fn func(ctx context.Context, method, } // defaultProviders is the provider set of an install whose auth config -// row was never written (or cannot be parsed): local login enabled. A -// corrupted row must degrade to a working login page, not a lockout. +// row was never written: local login enabled. SQLite also uses this +// default when recovering from a malformed row. func defaultProviders() []Provider { return []Provider{{Name: "local", Kind: "local", Enabled: true}} } // snapshot returns the parsed provider list plus the hash of the raw row -// it came from. Missing or malformed rows yield the fail-open default. -func (r *Registry) snapshot(ctx context.Context) ([]Provider, [sha256.Size]byte) { +// it came from. PostgreSQL callers distinguish a fresh install from an +// unavailable or corrupted policy. +func (r *Registry) snapshot(ctx context.Context) ([]Provider, [sha256.Size]byte, error) { raw, ok, err := r.store.ConfigValue(ctx, "auth") + if err != nil && r.store.Driver == "postgres" { + return nil, [sha256.Size]byte{}, err + } if err != nil || !ok { - return defaultProviders(), sha256.Sum256(nil) + return defaultProviders(), sha256.Sum256(nil), nil } var cfg struct { Providers []Provider `json:"providers"` } if json.Unmarshal([]byte(raw), &cfg) != nil || len(cfg.Providers) == 0 { - return defaultProviders(), sha256.Sum256(nil) + if r.store.Driver == "postgres" { + return nil, [sha256.Size]byte{}, errors.New("invalid authentication configuration") + } + return defaultProviders(), sha256.Sum256(nil), nil } - return cfg.Providers, sha256.Sum256([]byte(raw)) + return cfg.Providers, sha256.Sum256([]byte(raw)), nil } // HelmRoleOverrides returns the current helmOverride.roleMappings from the // "auth" config row, if present. It parses the row to extract the optional // per-role override lists that the login path uses to compute the effective -// Helm policy. Returns nil if the auth row is missing, malformed, or carries -// no helmOverride. -func (r *Registry) HelmRoleOverrides(ctx context.Context) *RoleMappings { +// Helm policy. An unavailable or malformed PostgreSQL row aborts the callback. +func (r *Registry) HelmRoleOverrides(ctx context.Context) (*RoleMappings, error) { raw, ok, err := r.store.ConfigValue(ctx, "auth") + if err != nil && r.store.Driver == "postgres" { + return nil, err + } if err != nil || !ok { - return nil + return nil, nil } var cfg struct { HelmOverride *struct { RoleMappings *RoleMappings `json:"roleMappings"` } `json:"helmOverride"` } - if json.Unmarshal([]byte(raw), &cfg) != nil { - return nil + if err := json.Unmarshal([]byte(raw), &cfg); err != nil { + if r.store.Driver == "postgres" { + return nil, err + } + return nil, nil } if cfg.HelmOverride == nil || cfg.HelmOverride.RoleMappings == nil { - return nil + return nil, nil } - return cfg.HelmOverride.RoleMappings + return cfg.HelmOverride.RoleMappings, nil } // Enabled lists the providers the login page may offer: enabled DB // providers plus the synthetic Helm provider when the flags are set. // Row read only — no Secret fetch, no discovery. func (r *Registry) Enabled(ctx context.Context) []Provider { - providers, _ := r.snapshot(ctx) + providers, _, err := r.snapshot(ctx) + if err != nil { + return nil + } out := make([]Provider, 0, len(providers)+1) for _, p := range providers { if p.Enabled { @@ -264,18 +277,25 @@ func (r *Registry) Enabled(ctx context.Context) []Provider { } // LocalEnabled reports whether local (username/password) login is -// allowed. A missing or unreadable config row means yes — fresh installs -// must be able to log in. +// allowed. Fresh installs enable local login. func (r *Registry) LocalEnabled(ctx context.Context) bool { - providers, _ := r.snapshot(ctx) + enabled, _ := r.localEnabled(ctx) + return enabled +} + +func (r *Registry) localEnabled(ctx context.Context) (bool, error) { + providers, _, err := r.snapshot(ctx) + if err != nil { + return false, err + } for _, p := range providers { if p.Kind == "local" { - return p.Enabled + return p.Enabled, nil } } // Row exists but carries no local entry (pre-v2 hand-edited config): // fail open for the same reason a missing row does. - return true + return true, nil } // OIDCFor resolves the named provider to a ready OIDC client, building @@ -289,7 +309,10 @@ func (r *Registry) OIDCFor(ctx context.Context, name string) (*OIDC, error) { } return r.legacy, nil } - providers, hash := r.snapshot(ctx) + providers, hash, err := r.snapshot(ctx) + if err != nil { + return nil, err + } var found *Provider for i := range providers { if providers[i].Name == name { @@ -300,6 +323,21 @@ func (r *Registry) OIDCFor(ctx context.Context, name string) (*OIDC, error) { if found == nil || !found.Enabled || found.Kind == "local" { return nil, fmt.Errorf("%q: %w", name, ErrUnknownProvider) } + var input providerInputs + if r.store.Driver == "postgres" { + // Discovery is expensive, but Secrets and the redirect URL can change + // independently of this provider row on any API replica. + input, err = r.providerInputs(ctx, *found) + if err != nil { + return nil, err + } + h := sha256.New() + _, _ = h.Write(hash[:]) + secretHash := sha256.Sum256([]byte(input.secret)) + _, _ = h.Write(secretHash[:]) + _, _ = h.Write([]byte(input.redirect)) + copy(hash[:], h.Sum(nil)) + } r.mu.Lock() entry, cached := r.built[name] @@ -314,31 +352,46 @@ func (r *Registry) OIDCFor(ctx context.Context, name string) (*OIDC, error) { } } - o, err := r.build(ctx, *found) + if r.store.Driver != "postgres" { + input, err = r.providerInputs(ctx, *found) + } + var o *OIDC + if err == nil { + o, err = r.build(ctx, *found, input) + } r.mu.Lock() r.built[name] = builtEntry{oidc: o, err: err, builtAt: r.now(), cfgHash: hash} r.mu.Unlock() return o, err } -func (r *Registry) build(ctx context.Context, p Provider) (*OIDC, error) { +type providerInputs struct { + secret string + redirect string +} + +func (r *Registry) providerInputs(ctx context.Context, p Provider) (providerInputs, error) { secretName := p.ConfigRef if secretName == "" { secretName = authProviderPrefix + p.Name } data, err := r.secrets(ctx, secretName) if err != nil { - return nil, fmt.Errorf("provider %q: %w", p.Name, err) + return providerInputs{}, fmt.Errorf("provider %q: %w", p.Name, err) } clientSecret := string(data["clientSecret"]) if clientSecret == "" { - return nil, fmt.Errorf(`provider %q: secret %s has no "clientSecret" key`, p.Name, secretName) + return providerInputs{}, fmt.Errorf(`provider %q: secret %s has no "clientSecret" key`, p.Name, secretName) } redirect, err := r.redirectURL(ctx, p.Name) if err != nil { - return nil, fmt.Errorf("provider %q: %w", p.Name, err) + return providerInputs{}, fmt.Errorf("provider %q: %w", p.Name, err) } - o, err := NewOIDCWithPolicy(ctx, p.Issuer, p.ClientID, clientSecret, redirect, &ProviderPolicy{ + return providerInputs{secret: clientSecret, redirect: redirect}, nil +} + +func (r *Registry) build(ctx context.Context, p Provider, input providerInputs) (*OIDC, error) { + o, err := NewOIDCWithPolicy(ctx, p.Issuer, p.ClientID, input.secret, input.redirect, &ProviderPolicy{ Scopes: p.Scopes, GroupsClaim: p.GroupsClaim, RoleMappings: p.RoleMappings, @@ -348,9 +401,6 @@ func (r *Registry) build(ctx context.Context, p Provider) (*OIDC, error) { return nil, fmt.Errorf("provider %q: discover issuer: %w", p.Name, err) } o.AttachStore(r.store) - o.AttachHelmRoleOverridesFunc(func(ctx context.Context) *RoleMappings { - return r.HelmRoleOverrides(ctx) - }) r.mu.Lock() auditFn := r.auditWriteSync r.mu.Unlock() diff --git a/api/internal/auth/registry_test.go b/api/internal/auth/registry_test.go index d837772a1..d62d26795 100644 --- a/api/internal/auth/registry_test.go +++ b/api/internal/auth/registry_test.go @@ -157,6 +157,57 @@ func TestRegistry_UnknownDisabledLocal(t *testing.T) { } } +func TestRegistryPostgresCacheTracksProviderCredentialsAndRedirect(t *testing.T) { + s := newAuthDB(t) + seedExternalURL(t, s) + idp := newFakeIDP(t, "gameplane") + seedConfigRow(t, s, "auth", authRow(providerRow("corp", idp.issuer(), true))) + // The registry uses portable config queries. Select its PostgreSQL cache + // policy here so fake issuer discovery and credential rotation run in unit CI. + s.Driver = "postgres" + secrets := secretFor("corp") + registries := []*Registry{ + NewRegistry(s, staticSecrets(secrets), nil, "", nil), + NewRegistry(s, staticSecrets(secrets), nil, "", nil), + } + resolve := func(reg *Registry) *OIDC { + t.Helper() + o, err := reg.OIDCFor(t.Context(), "corp") + if err != nil { + t.Fatal(err) + } + return o + } + first, second := resolve(registries[0]), resolve(registries[1]) + if resolve(registries[1]) != second { + t.Fatal("unchanged discovery was rebuilt") + } + secrets["gameplane-auth-corp"]["clientSecret"] = []byte("rotated") + for i, reg := range registries { + o := resolve(reg) + if o == first || o == second || o.oauth.ClientSecret != "rotated" { + t.Fatalf("replica %d used stale credentials", i) + } + } + seedConfigRow(t, s, "general", `{"externalURL":"https://new.example"}`) + for i, reg := range registries { + if resolve(reg).oauth.RedirectURL != "https://new.example/auth/oidc/corp/callback" { + t.Fatalf("replica %d used stale redirect", i) + } + } + delete(secrets, "gameplane-auth-corp") + if _, err := registries[1].OIDCFor(t.Context(), "corp"); err == nil { + t.Fatal("removed Secret reused cached issuer") + } + seedConfigRow(t, s, "auth", "malformed") + if registries[0].LocalEnabled(t.Context()) { + t.Fatal("malformed PostgreSQL policy enabled local login") + } + if _, err := registries[1].HelmRoleOverrides(t.Context()); err == nil { + t.Fatal("malformed PostgreSQL row removed Helm restrictions") + } +} + func TestRegistry_BuildErrorBacksOff(t *testing.T) { s := newAuthDB(t) seedExternalURL(t, s) diff --git a/api/internal/auth/sessions.go b/api/internal/auth/sessions.go index d6fef2a75..130587289 100644 --- a/api/internal/auth/sessions.go +++ b/api/internal/auth/sessions.go @@ -10,6 +10,7 @@ import ( "errors" "log/slog" "net/http" + "reflect" "time" "github.com/GameplanePanel/gameplane/api/internal/db" @@ -105,21 +106,29 @@ func (s *SessionStore) createAtEpoch(ctx context.Context, userID, epoch int64) ( // expired sessions only get cleaned on the unlikely event that their // exact token is re-looked-up — the table grows forever in practice. func (s *SessionStore) StartGC(ctx context.Context, interval time.Duration) { + go s.RunGC(ctx, interval) +} + +// RunGC sweeps expired sessions and fully refilled PostgreSQL rate buckets until cancelled. +func (s *SessionStore) RunGC(ctx context.Context, interval time.Duration) { if interval <= 0 { interval = time.Hour } - go func() { - t := time.NewTicker(interval) - defer t.Stop() - for { - select { - case <-ctx.Done(): - return - case <-t.C: - s.gcOnce(ctx) + t := time.NewTicker(interval) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + s.gcOnce(ctx) + if s.db.Driver == "postgres" { + if _, err := s.db.DB.ExecContext(ctx, `DELETE FROM api_rate_buckets WHERE next_us < floor(extract(epoch FROM statement_timestamp())*1000000)::bigint`); err != nil && ctx.Err() == nil { + slog.Warn("rate limit gc", "err", err) + } } } - }() + } } // DeleteForUser drops every session row owned by the given user. Call @@ -184,6 +193,46 @@ func (s *SessionStore) Authenticate(next http.Handler) http.Handler { }) } +// RevalidateStreams bounds how long an established console or stream can +// outlive its session or permissions. Run it immediately after Authenticate. +func (s *SessionStore) RevalidateStreams(isStream func(*http.Request) bool, interval time.Duration) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + if !isStream(req) { + next.ServeHTTP(w, req) + return + } + initial := UserFromContext(req.Context()) + cookie, err := req.Cookie(sessionCookie) + if err != nil || initial == nil { + httperr.WriteMessage(w, req, messages.New("server.common.unauthenticated", nil)) + return + } + ctx, cancel := context.WithCancel(req.Context()) + defer cancel() + go func() { + ticker := time.NewTicker(interval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + check, stop := context.WithTimeout(ctx, interval) + active, _, err := s.lookup(check, cookie.Value) + stop() + if err != nil || active.ID != initial.ID || active.Role != initial.Role || !reflect.DeepEqual(active.Perms, initial.Perms) { + cancel() + return + } + } + } + }() + next.ServeHTTP(w, req.WithContext(ctx)) + }) + } +} + func (s *SessionStore) lookup(ctx context.Context, token string) (*User, string, error) { digest := sessionDigest(token) var ( @@ -224,7 +273,10 @@ func (s *SessionStore) lookup(ctx context.Context, token string) (*User, string, func (s *SessionStore) HandleLogout() http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { if c, err := req.Cookie(sessionCookie); err == nil { - _, _ = s.db.DB.ExecContext(req.Context(), `DELETE FROM sessions WHERE token = ?`, sessionDigest(c.Value)) + if _, err := s.db.DB.ExecContext(req.Context(), `DELETE FROM sessions WHERE token = ?`, sessionDigest(c.Value)); err != nil { + httperr.WriteMessage(w, req, messages.New("server.generic.503", nil)) + return + } } clearCookie(w, sessionCookie) // The CSRF cookie is JS-readable while set (see setCSRFCookie), but diff --git a/api/internal/db/migrations/common/020_api_runtime.sql b/api/internal/db/migrations/common/020_api_runtime.sql new file mode 100644 index 000000000..114ba2de3 --- /dev/null +++ b/api/internal/db/migrations/common/020_api_runtime.sql @@ -0,0 +1,12 @@ +CREATE TABLE api_rate_buckets ( + budget TEXT NOT NULL, + subject_hash TEXT NOT NULL, + next_us BIGINT NOT NULL, + PRIMARY KEY (budget, subject_hash) +); +CREATE INDEX api_rate_buckets_expiry ON api_rate_buckets(next_us); + +CREATE TABLE api_runtime_keys ( + purpose TEXT PRIMARY KEY, + key_hex TEXT NOT NULL +); diff --git a/docs/panel-recovery.md b/docs/panel-recovery.md index 63fced6ee..fa2ff56e3 100644 --- a/docs/panel-recovery.md +++ b/docs/panel-recovery.md @@ -178,6 +178,36 @@ Kubernetes disaster recovery exercise. ## PostgreSQL physical recovery +### Active-active API state + +All API replicas must use the same writable PostgreSQL database and schema, +the same configuration and credentials, and the same trusted ingress proxy +ranges. Protected traffic and authentication budgets go to the primary. A +session created on either API works on both, and logout and credential changes +affect the next request on either replica. Long-lived consoles, event streams +and transfers recheck their session and permissions every five seconds. An +unavailable database cancels the stream after a further five-second query +deadline. Provider edits, Secret rotation and redirect changes are read on the +next dashboard-managed OIDC handshake. + +The API persists login-device signatures and rate budgets with the rest of the +database. Preserve these tables in backups. To invalidate known-device cookies +after disaster recovery, stop all API replicas, delete the `login_device` row +from `api_runtime_keys`, then restart. A recognized device bypasses only the +per-account password budget and still needs the password and per-IP budget. + +Each install elects one replica for notification watches, session and budget +cleanup, usage reports, and audit retention. Reserve capacity for one additional +database connection per active leader. The leader holds a read-committed +transaction advisory lock for its tenure and heartbeats every two seconds. +The database or pooler's maximum transaction age must accommodate this +connection. After a backend loss, the replica cancels these workers and the +standby retries election every two seconds. Notification sinks receive +best-effort delivery across a failover. Fence the old writable primary before +enabling the new primary. + +### Physical recovery drill + The PostgreSQL CI lane runs the release API image with one writer. After the availability and Secret checks, the dedicated PITR step takes a physical base backup, writes authorization state, creates a named WAL restore point, and From 16978d78f04b84cd1eea90d1aa9d40f5255832d2 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 01:02:09 -0500 Subject: [PATCH 07/23] test(postgres): verify persistent recovery and game continuity during PITR Signed-off-by: Baylem --- deploy/kind/recover-postgres-wal.sh | 7 ++- test/e2e/postgres_pitr_e2e_test.go | 68 ++++++++++++++++++++++++----- 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/deploy/kind/recover-postgres-wal.sh b/deploy/kind/recover-postgres-wal.sh index 8b15a42ac..a68feabf9 100644 --- a/deploy/kind/recover-postgres-wal.sh +++ b/deploy/kind/recover-postgres-wal.sh @@ -16,15 +16,20 @@ sql() { } # Allocate the replacement before fencing. It has a distinct Service selector # and cannot receive application connections until reconciliation has finished. +"${k[@]}" get pvc "data-$source" -o json | jq --arg name "$target" '{ + apiVersion:"v1",kind:"PersistentVolumeClaim",metadata:{name:$name}, + spec:{accessModes:.spec.accessModes,storageClassName:.spec.storageClassName, + resources:{requests:{storage:.spec.resources.requests.storage}}}}' | "${k[@]}" create -f - "${k[@]}" get sts gameplane-postgres -o json | jq --arg name "$target" '{ apiVersion:"v1",kind:"Pod",metadata:{name:$name,labels:{"app.kubernetes.io/name":$name}}, spec:{automountServiceAccountToken:false,restartPolicy:"Never", securityContext:{runAsNonRoot:true,runAsUser:70,runAsGroup:70,fsGroup:70,seccompProfile:{type:"RuntimeDefault"}}, containers:[{name:"postgres",image:.spec.template.spec.containers[0].image, + ports:.spec.template.spec.containers[0].ports, command:["sh","-ec","mkdir -p /data/db /data/wal; chmod 0700 /data/db; while [ ! -f /data/start ]; do sleep 1; done; exec postgres -D /data/db"], securityContext:{allowPrivilegeEscalation:false,capabilities:{drop:["ALL"]}}, volumeMounts:[{name:"recovery",mountPath:"/data"}]}], - volumes:[{name:"recovery",emptyDir:{}}]}}' | "${k[@]}" create -f - + volumes:[{name:"recovery",persistentVolumeClaim:{claimName:$name}}]}}' | "${k[@]}" create -f - "${k[@]}" wait --for=condition=Ready "pod/$target" --timeout=120s "${k[@]}" exec "$target" -- mkdir -p /data/db /data/wal diff --git a/test/e2e/postgres_pitr_e2e_test.go b/test/e2e/postgres_pitr_e2e_test.go index fcc73bc07..fd96f3252 100644 --- a/test/e2e/postgres_pitr_e2e_test.go +++ b/test/e2e/postgres_pitr_e2e_test.go @@ -3,6 +3,8 @@ package e2e import ( + "context" + "crypto/sha256" "encoding/json" "net/http" "os" @@ -43,17 +45,24 @@ func TestPanel_PostgresPointInTimeRecovery(t *testing.T) { } return string(pvc.UID) } - keyID := func() string { + // Check public certificate bytes as well as object identity: an in-place + // replacement would preserve the Kubernetes UID. + type credential struct { + uid string + certificate [32]byte + } + keyID := func(name, field string) credential { t.Helper() - key, err := envInstance.K8s.CoreV1().Secrets("gameplane-system").Get(ctx, "gameplane-agent-ca", metav1.GetOptions{}) - if err != nil || key.UID == "" { - t.Fatalf("read native agent CA identity: %v", err) + key, err := envInstance.K8s.CoreV1().Secrets("gameplane-system").Get(ctx, name, metav1.GetOptions{}) + if err != nil || key.UID == "" || len(key.Data[field]) == 0 { + t.Fatalf("read native credential %s: %v", name, err) } - return string(key.UID) + return credential{string(key.UID), sha256.Sum256(key.Data[field])} } f := recoveryFixture{t: t} f.request(admin, http.MethodPost, "/servers/"+game+":transfer?namespace="+ns, map[string]any{"userId": userID}, http.StatusNoContent) - beforePod, beforeClaim, beforeKey := recoveryGameIdentity(t, game), claimID(), keyID() + beforePod, beforeClaim := recoveryGameIdentity(t, game), claimID() + beforeCA, beforeClient := keyID("gameplane-agent-ca", "ca.crt"), keyID("gameplane-agent-client", "tls.crt") beforeProgress := recoveryCounter(t, game) reader := envInstance.APIClient(t, user, password) t.Cleanup(reader.Close) @@ -65,9 +74,46 @@ func TestPanel_PostgresPointInTimeRecovery(t *testing.T) { t.Fatal(err) } path := filepath.Join(t.TempDir(), "wal.json") - out, err := exec.CommandContext(ctx, "bash", script, envInstance.Context, id, path).CombinedOutput() - if err != nil { - t.Fatalf("fenced database recovery: %v\n%s", err, out) + drillCtx, cancel := context.WithCancel(ctx) + defer cancel() + cmd := exec.CommandContext(drillCtx, "bash", script, envInstance.Context, id, path) + type drillResult struct { + output []byte + err error + } + finished := make(chan drillResult, 1) + go func() { out, err := cmd.CombinedOutput(); finished <- drillResult{out, err} }() + tick := time.NewTicker(2 * time.Second) + defer tick.Stop() + last, lastSample := beforeProgress, time.Now() + var maxGap time.Duration + samples := 0 + sampleGame := func() { + t.Helper() + next := recoveryCounter(t, game) + if next < last { + t.Fatalf("lost acknowledged game progress: %d -> %d", last, next) + } + now := time.Now() + maxGap = max(maxGap, now.Sub(lastSample)) + last, lastSample = next, now + samples++ + } +drill: + for { + select { + case <-tick.C: + sampleGame() + case result := <-finished: + if result.err != nil { + t.Fatalf("fenced database recovery: %v\n%s", result.err, result.output) + } + sampleGame() + break drill + } + } + if samples < 2 || last <= beforeProgress { + t.Fatal("insufficient live game progress during recovery") } post := envInstance.APIClient(t, user, password) t.Cleanup(post.Close) @@ -97,7 +143,7 @@ func TestPanel_PostgresPointInTimeRecovery(t *testing.T) { if err != nil || string(gs.GetUID()) != gameUID || gs.GetAnnotations()["gameplane.local/owner-id"] != id { t.Fatalf("restored owner does not match the live GameServer: %v", err) } - if recoveryGameIdentity(t, game) != beforePod || claimID() != beforeClaim || keyID() != beforeKey || recoveryCounter(t, game) <= beforeProgress { + if recoveryGameIdentity(t, game) != beforePod || claimID() != beforeClaim || keyID("gameplane-agent-ca", "ca.crt") != beforeCA || keyID("gameplane-agent-client", "tls.crt") != beforeClient || recoveryCounter(t, game) <= beforeProgress { t.Fatal("game process, save, claim, native CA identity or progress changed unexpectedly") } b, err := os.ReadFile(path) @@ -112,5 +158,5 @@ func TestPanel_PostgresPointInTimeRecovery(t *testing.T) { if err := json.Unmarshal(b, &metrics); err != nil || metrics.Loss <= 0 || metrics.LSN == "" { t.Fatalf("invalid PITR report: %v", err) } - t.Logf("PostgreSQL PITR passed: target LSN=%s, panel recovery point age=%ds, panel RTO=%ds, live game retained CR=%s PVC=%s Pod=%s and advanced its persistent counter", metrics.LSN, metrics.Age, time.Now().Unix()-metrics.Loss, gameUID, beforeClaim, beforePod.PodUID) + t.Logf("PostgreSQL PITR passed: target LSN=%s, panel recovery point age=%ds, panel RTO=%ds, live game retained CR=%s PVC=%s Pod=%s without a failed game probe or lost acknowledged counter (samples=%d, largest observation gap=%s)", metrics.LSN, metrics.Age, time.Now().Unix()-metrics.Loss, gameUID, beforeClaim, beforePod.PodUID, samples, maxGap) } From 4ffcad20a8abd73d54e6c33ee460949327c53307 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 01:03:48 -0500 Subject: [PATCH 08/23] fix(api): fence incompatible PostgreSQL HA writers before database startup Signed-off-by: Baylem --- api/cmd/main.go | 12 ++++++++++++ api/cmd/workers.go | 9 +++++++++ api/cmd/workers_test.go | 35 +++++++++++++++++++++++++++++++++++ docs/panel-recovery.md | 3 +++ 4 files changed, 59 insertions(+) create mode 100644 api/cmd/workers_test.go diff --git a/api/cmd/main.go b/api/cmd/main.go index 4be86cc05..71a9c8d98 100644 --- a/api/cmd/main.go +++ b/api/cmd/main.go @@ -110,6 +110,10 @@ func main() { } logger = newLogger(cfg.logLevel) slog.SetDefault(logger) + if err := cfg.validatePostgresHA(); err != nil { + logger.Error("invalid active-active deployment", "err", err) + os.Exit(2) + } // Validate OIDC configuration flags. if cfg.oidcDefaultRole != "" { @@ -550,6 +554,7 @@ type config struct { addr string metricsAddr string dbDriver string + postgresHA bool dbDSN string logLevel string @@ -628,6 +633,13 @@ func (c *config) bindFlags(fs *flag.FlagSet) { fs.StringVar(&c.logLevel, "log-level", envOr("GAMEPLANE_LOG_LEVEL", "info"), "log verbosity: debug, info, warn, or error") fs.StringVar(&c.dbDriver, "db-driver", envOr("GAMEPLANE_DB_DRIVER", "sqlite"), "sqlite or postgres") + fs.Func("postgres-ha-protocol", "require the named native PostgreSQL active-active protocol", func(v string) error { + if v != "1" { + return fmt.Errorf("unsupported PostgreSQL HA protocol %q (supported: 1)", v) + } + c.postgresHA = true + return nil + }) fs.StringVar(&c.dbDSN, "db-dsn", envOr("GAMEPLANE_DB_DSN", "file:/data/gameplane.db?_pragma=journal_mode(WAL)"), "DSN") fs.StringVar(&c.oidcIssuer, "oidc-issuer", envOr("GAMEPLANE_OIDC_ISSUER", ""), "OIDC issuer URL") fs.StringVar(&c.oidcClientID, "oidc-client-id", envOr("GAMEPLANE_OIDC_CLIENT_ID", ""), "OIDC client id") diff --git a/api/cmd/workers.go b/api/cmd/workers.go index 33992cbb8..3b33652d9 100644 --- a/api/cmd/workers.go +++ b/api/cmd/workers.go @@ -10,6 +10,15 @@ import ( "github.com/GameplanePanel/gameplane/api/internal/db" ) +// The HA chart passes a capability flag that old binaries reject before +// they open storage. The standalone control plane has its own key lifecycle. +func (c config) validatePostgresHA() error { + if c.postgresHA && (c.dbDriver != "postgres" || c.standalone) { + return errors.New("--postgres-ha-protocol requires a native PostgreSQL API") + } + return nil +} + // runPanelWorkers keeps the install's periodic jobs on one replica. A read- // committed transaction pins its PostgreSQL backend even with transaction // pooling. The transaction-scoped lock is released on rollback or backend diff --git a/api/cmd/workers_test.go b/api/cmd/workers_test.go new file mode 100644 index 000000000..d35aa4d9d --- /dev/null +++ b/api/cmd/workers_test.go @@ -0,0 +1,35 @@ +package main + +import ( + "flag" + "testing" +) + +func TestHAProtocolRequiresNativePostgres(t *testing.T) { + for _, tc := range []struct { + name string + args []string + valid bool + }{ + {"sqlite", []string{"--db-driver=sqlite"}, true}, + {"native postgres", []string{"--db-driver=postgres", "--standalone=false", "--postgres-ha-protocol=1"}, true}, + {"old protocol", []string{"--db-driver=postgres", "--postgres-ha-protocol=0"}, false}, + {"empty protocol", []string{"--db-driver=postgres", "--postgres-ha-protocol="}, false}, + {"future protocol", []string{"--db-driver=postgres", "--postgres-ha-protocol=2"}, false}, + {"sqlite HA", []string{"--db-driver=sqlite", "--postgres-ha-protocol=1"}, false}, + {"standalone HA", []string{"--db-driver=postgres", "--standalone", "--postgres-ha-protocol=1"}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + var cfg config + flags := flag.NewFlagSet("serve", flag.ContinueOnError) + cfg.bindFlags(flags) + err := flags.Parse(tc.args) + if err == nil { + err = cfg.validatePostgresHA() + } + if (err == nil) != tc.valid { + t.Fatalf("configuration validity=%v, err=%v", err == nil, err) + } + }) + } +} diff --git a/docs/panel-recovery.md b/docs/panel-recovery.md index fa2ff56e3..e806c835a 100644 --- a/docs/panel-recovery.md +++ b/docs/panel-recovery.md @@ -183,6 +183,9 @@ Kubernetes disaster recovery exercise. All API replicas must use the same writable PostgreSQL database and schema, the same configuration and credentials, and the same trusted ingress proxy ranges. Protected traffic and authentication budgets go to the primary. A +chart-managed HA Pod starts with `--postgres-ha-protocol=1`. A legacy +binary rejects this flag before opening the database. To downgrade to a +version without this protocol, return to one writer before changing images. A session created on either API works on both, and logout and credential changes affect the next request on either replica. Long-lived consoles, event streams and transfers recheck their session and permissions every five seconds. An From 99ecae3dc4aecf4c755f2f30ff31112394d47c31 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 01:05:55 -0500 Subject: [PATCH 09/23] test(postgres): wait for shared login budgets between independent suites Signed-off-by: Baylem --- .github/workflows/ci.yaml | 85 ++++++++++++----------- deploy/kind/wait-postgres-login-budget.sh | 10 +++ 2 files changed, 54 insertions(+), 41 deletions(-) create mode 100644 deploy/kind/wait-postgres-login-budget.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 329fb0c80..0e4c66d13 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -5,10 +5,10 @@ on: branches: [master] # Every master push runs the full suite, including docs/submodule changes. pull_request: - # Docs-only changes can't affect anything the suites test — don't spend + # Docs-only changes can't affect anything the suites test — don't spend # kind minutes on a markdown diff. Any non-ignored file in the change # set still triggers the workflow. "website" (no slash) matches the - # submodule gitlink itself — a plain submodule pointer bump; the /** + # submodule gitlink itself — a plain submodule pointer bump; the /** # variant covers any future in-tree files under that path. # paths + negations (not paths-ignore, which cannot re-include) so that a # change touching only a specs.md still runs make check-specs (ruling D5). @@ -66,8 +66,8 @@ permissions: # All jobs run on GitHub-hosted runners: each job gets its own isolated # VM, so jobs never contend for memory or ports and there is nothing to # serialize. The e2e suites are split into independent jobs, each with -# its own kind cluster, so the slowest suite — not the sum of all of -# them — sets the wall-clock time. +# its own kind cluster, so the slowest suite — not the sum of all of +# them — sets the wall-clock time. jobs: # Path-filter gate. Computes which parts of the tree a change touches so the # heavy jobs below can skip when they can't be affected (a web-only PR drops @@ -350,7 +350,7 @@ jobs: # (docs/dependencies.md -> CHANGELOG.md, docs/contributing.md # -> AGENTS.md and specs/012-.../outreach.md, README.md -> # LICENSE, signing/cosign.pub, docs/key-rotation.md -> the legacy key - # files) — renaming or moving any of these must run check-links. + # files) — renaming or moving any of these must run check-links. - 'CHANGELOG.md' - 'AGENTS.md' - 'CLAUDE.md' @@ -474,13 +474,13 @@ jobs: # # IMPORTANT: this deliberately does NOT extract the binary with `docker cp`. # `docker cp` (copy out of a container) unpacks the tar stream client-side, - # as the invoking non-root runner user, outside any container/namespace — + # as the invoking non-root runner user, outside any container/namespace — # and writing the security.capability xattr onto a real file requires the # CAP_SETFCAP privilege that process does not have, so the xattr silently # fails to apply. That makes `docker cp` + getcap report the capability # missing unconditionally, regardless of whether the image is actually # correct, which would fail this gate every run and permanently redden the - # branch — the worst possible outcome for a job whose only purpose is to + # branch — the worst possible outcome for a job whose only purpose is to # prove this property. Instead we `docker save` the image (the capability # xattr is preserved as a PAX/SCHILY.xattr record inside the layer # tarball itself) and extract that tarball ourselves with @@ -491,8 +491,8 @@ jobs: # exact same reason `docker cp` is: restoring the security.capability xattr # to a real file requires privilege the plain `runner` user does not have, # regardless of whether the archive is well-formed. So the extraction (and - # the getcap read of the result) run under `sudo` — GitHub-hosted runners - # grant the runner user passwordless sudo — which is the only way this job + # the getcap read of the result) run under `sudo` — GitHub-hosted runners + # grant the runner user passwordless sudo — which is the only way this job # can actually distinguish "capability present" from "capability absent" # instead of failing unconditionally either way. capture-sidecar-setcap-proof: @@ -513,7 +513,7 @@ jobs: sudo apt-get install -y --no-install-recommends libcap2-bin # Same Dockerfile, same repo-root context, same buildx-backed action as # publish-edge.yaml/release.yaml use for the real capture-sidecar image - # — single-platform (amd64, matching this runner) and loaded locally + # — single-platform (amd64, matching this runner) and loaded locally # instead of pushed. - name: build capture-sidecar image (matches publish workflows) uses: docker/build-push-action@c3c9e263c25d99ce0380d002d59b67737d91b0dc # v7.4.0 @@ -555,7 +555,7 @@ jobs: # Materialize the listing before grepping it: `tar -tf | grep -q` # lets grep exit on its first match and SIGPIPE the still-writing # tar process, which under `set -o pipefail` turns into a - # nonzero pipeline status indistinguishable from "no match" — + # nonzero pipeline status indistinguishable from "no match" — # silently skipping the layer that actually holds the binary. listing="$(tar -tf "$candidate" 2>/dev/null || true)" if ! printf '%s\n' "$listing" | grep -Eq '(^|/)capture-sidecar$'; then @@ -566,7 +566,7 @@ jobs: mkdir -p "$EXTRACT_DIR" # sudo: restoring the security.capability xattr to a real file # requires CAP_SETFCAP, which the unprivileged runner user does - # not have — see the job-level comment for why this can't run + # not have — see the job-level comment for why this can't run # as plain `tar`. sudo tar --xattrs --xattrs-include='security.capability' -xf "$candidate" -C "$EXTRACT_DIR" @@ -818,7 +818,7 @@ jobs: run: | for attempt in 1 2 3 4; do if go mod download; then exit 0; fi - echo "go mod download failed (attempt ${attempt}/5) — retrying in $((attempt * 30))s" + echo "go mod download failed (attempt ${attempt}/5) — retrying in $((attempt * 30))s" sleep $((attempt * 30)) done go mod download @@ -863,7 +863,7 @@ jobs: needs: [changes] if: (needs.changes.outputs.go == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'go') # amd64 on the standard hosted runner; arm64 on the free public-repo - # arm runner. Unit + envtest run on both arches every PR — this is where + # arm runner. Unit + envtest run on both arches every PR — this is where # real arch bugs (build tags, sqlite codepaths, struct alignment) surface. runs-on: ${{ matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-latest' }} timeout-minutes: 20 @@ -908,7 +908,7 @@ jobs: run: | for attempt in 1 2 3 4; do if go mod download; then exit 0; fi - echo "go mod download failed (attempt ${attempt}/5) — retrying in $((attempt * 30))s" + echo "go mod download failed (attempt ${attempt}/5) — retrying in $((attempt * 30))s" sleep $((attempt * 30)) done go mod download @@ -976,7 +976,7 @@ jobs: } for attempt in 1 2 3; do if fetch; then exit 0; fi - echo "coverage tool fetch failed (attempt ${attempt}/4) — retrying in $((attempt * 30))s" + echo "coverage tool fetch failed (attempt ${attempt}/4) — retrying in $((attempt * 30))s" sleep $((attempt * 30)) done fetch @@ -1130,8 +1130,8 @@ jobs: HEAD_SHA: ${{ github.event.pull_request.head.sha }} RUN_SHA: ${{ github.sha }} # steps.cover.outcome ignores this step's own continue-on-error - # (it has none) and reflects whether `npm run test:cover` — the - # vitest coverage-threshold gate — actually passed, so a + # (it has none) and reflects whether `npm run test:cover` — the + # vitest coverage-threshold gate — actually passed, so a # below-threshold run posts "failure" instead of a hard-coded # "success" that contradicts the red `web` job next to it. COVER_OUTCOME: ${{ steps.cover.outcome }} @@ -1486,7 +1486,7 @@ jobs: - name: pre-upgrade hook targets the rendered API Deployment name (F-213) # A release installed under a non-default name used to render a hook # Role whose resourceNames named a Deployment - # ({{ .Release.Name }}-api) the chart never actually creates — the + # ({{ .Release.Name }}-api) the chart never actually creates — the # API Deployment is always named "gameplane-api" literally # (api.yaml), unlike the surrounding Role/RoleBinding names. Any such # release could install but never upgrade. @@ -1583,7 +1583,7 @@ jobs: # values.yaml embeds a copy of the signing public key so the operator # can verify official module bundles. Nothing regenerates it, so a key # rotation that updates only the root file leaves the chart verifying - # against the retired key — every official bundle then fails to install. + # against the retired key — every official bundle then fails to install. # The release workflows check the root key against the signing secret; # this checks the chart's copy against the root key. run: | @@ -1601,7 +1601,7 @@ jobs: sys.stdout.write(data if data.endswith('\n') else data + '\n') PY if ! diff -u signing/cosign.pub /tmp/chart-cosign.pub; then - echo "::error::charts/gameplane/values.yaml cosignPublicKey does not match signing/cosign.pub — update both when rotating the signing key" + echo "::error::charts/gameplane/values.yaml cosignPublicKey does not match signing/cosign.pub — update both when rotating the signing key" exit 1 fi echo "chart cosign key matches signing/cosign.pub" @@ -1695,7 +1695,7 @@ jobs: # These are in test/e2e but are untagged Go unit tests, not integration # tests, so they run without a cluster or build tags. Gated on e2e # changes since the tests live in the e2e directory (covered by e2etree - # filter). This job intentionally runs no coverage — test/e2e has no + # filter). This job intentionally runs no coverage — test/e2e has no # .testcoverage.yml and is excluded from the coverage gates. go-e2e-unit: name: go (test/e2e) @@ -1761,7 +1761,7 @@ jobs: run: | for attempt in 1 2 3 4; do if go mod download; then exit 0; fi - echo "go mod download failed (attempt ${attempt}/5) — retrying in $((attempt * 30))s" + echo "go mod download failed (attempt ${attempt}/5) — retrying in $((attempt * 30))s" sleep $((attempt * 30)) done go mod download @@ -1778,7 +1778,7 @@ jobs: run: go test -tags postgres -count=1 ./internal/db/... ./internal/controlplane/... ./cmd # --------------------------------------------------------------------- - # kind e2e — one job per suite, each with its own throwaway cluster on + # kind e2e — one job per suite, each with its own throwaway cluster on # its own VM. No `needs` edges: a broken unit test doesn't gate e2e # feedback, and the per-ref concurrency group above still cancels # superseded runs. VMs are discarded after the job, so there is no @@ -1835,7 +1835,7 @@ jobs: strategy: fail-fast: false matrix: - # Buckets are cut by login pressure, not feature area — see the + # Buckets are cut by login pressure, not feature area — see the # header comment in test/e2e/buckets.sh. The suite runs with # t.Parallel(); `parallel` bounds in-flight tests per bucket # (operator does zero logins and is wait-dominated, so it runs @@ -1988,7 +1988,7 @@ jobs: strategy: fail-fast: false matrix: - # Buckets are cut by login pressure, not feature area — see the + # Buckets are cut by login pressure, not feature area — see the # header comment in test/e2e/buckets.sh. The suite runs with # t.Parallel(); `parallel` bounds in-flight tests per bucket # (operator does zero logins and is wait-dominated, so it runs @@ -2082,7 +2082,7 @@ jobs: run: ./deploy/kind/e2e.sh up "$CLUSTER" e2e - name: bring up cluster B + helm install # A second, fully independent kind cluster with the same chart - # installed — it needs its own operator to reconcile the GameServer + # installed — it needs its own operator to reconcile the GameServer # this test dispatches to it. kind attaches every cluster's nodes to # the same docker bridge network ("kind"), so cluster A's api/ # operator pods can reach cluster B's control-plane container by its @@ -2136,7 +2136,7 @@ jobs: # as multicluster: its cluster is set up differently. Every other job # installs the working-tree chart; this one installs the PUBLISHED chart and # PUBLISHED GHCR images of the previous release, then upgrades onto the tree - # — so it cannot share a cluster with anything, and needs network access to + # — so it cannot share a cluster with anything, and needs network access to # ghcr.io that the other kind jobs do not. e2e-upgrade: @@ -2317,11 +2317,11 @@ jobs: needs: [changes, build-images] if: (needs.changes.outputs.e2e == 'true' && needs.changes.outputs.e2e-bots-any == 'true') || contains(fromJSON(needs.changes.outputs.ci-jobs || '[]'), 'e2e-game-bot') # amd64 only: this boots the external itzg/minecraft-server (multi-arch) AND - # passivelemon/terraria-docker, whose arm64 support is unverified upstream — + # passivelemon/terraria-docker, whose arm64 support is unverified upstream — # so an arm64 leg would fail on the Terraria pull. Add an arch axis here once # an arm-capable Terraria image is confirmed/pinned. runs-on: ubuntu-latest - # Two serial real-game boots (Minecraft, then Terraria — deliberately + # Two serial real-game boots (Minecraft, then Terraria — deliberately # not parallel: two game servers booting at once OOM-starve the node). timeout-minutes: 60 # Pulls multi-GB game images, boots real servers, runs protocol joins. permissions: @@ -2449,9 +2449,8 @@ jobs: # Re-runs the api-auth, api-roles and api-rbac buckets against a # Postgres-backed API (spec 013 US5, OD-3). amd64 only. The API is restarted - # between buckets: its login rate limiter lives in memory and each bucket is - # sized to one fresh limiter's budget (see test/e2e/buckets.sh). The restart - # also means each later bucket runs on data the earlier ones left in Postgres. + # between buckets to check persistence. PostgreSQL owns login budgets, + # so independent suites wait for natural refill using the database clock. # A final serial bucket migrates the Deployment strategy and exercises a # PostgreSQL outage/recovery against that populated installation. e2e-postgres: @@ -2494,23 +2493,25 @@ jobs: GAMEPLANE_E2E_REUSE_CLUSTER=1 \ GAMEPLANE_E2E_CLUSTER="$CLUSTER" \ go test -tags=e2e -timeout 15m -v -parallel 4 -run "$RUN_REGEX" ./... - - name: restart the API (fresh login limiter) + - name: restart the API between suites run: | kubectl --context "$CTX" --namespace gameplane-system rollout restart deployment/gameplane-api kubectl --context "$CTX" --namespace gameplane-system rollout status deployment/gameplane-api --timeout=180s - name: run go e2e (api-roles on postgres) run: | + bash deploy/kind/wait-postgres-login-budget.sh "$CTX" RUN_REGEX="$(./test/e2e/buckets.sh regex api-roles)" cd test/e2e && \ GAMEPLANE_E2E_REUSE_CLUSTER=1 \ GAMEPLANE_E2E_CLUSTER="$CLUSTER" \ go test -tags=e2e -timeout 10m -v -parallel 3 -run "$RUN_REGEX" ./... - - name: restart the API again (fresh login limiter) + - name: restart the API again between suites run: | kubectl --context "$CTX" --namespace gameplane-system rollout restart deployment/gameplane-api kubectl --context "$CTX" --namespace gameplane-system rollout status deployment/gameplane-api --timeout=180s - name: run go e2e (api-rbac on postgres) run: | + bash deploy/kind/wait-postgres-login-budget.sh "$CTX" RUN_REGEX="$(./test/e2e/buckets.sh regex api-rbac)" cd test/e2e && \ GAMEPLANE_E2E_REUSE_CLUSTER=1 \ @@ -2518,6 +2519,7 @@ jobs: go test -tags=e2e -timeout 10m -v -parallel 4 -run "$RUN_REGEX" ./... - name: run go e2e (PostgreSQL singleton upgrade and outage recovery) run: | + bash deploy/kind/wait-postgres-login-budget.sh "$CTX" RUN_REGEX="$(./test/e2e/buckets.sh regex postgres-availability)" cd test/e2e && \ GAMEPLANE_E2E_REUSE_CLUSTER=1 \ @@ -2527,6 +2529,7 @@ jobs: run: bash deploy/kind/check-postgres-secret.sh "$CTX" - name: run go e2e (postgres-pitr) run: | + bash deploy/kind/wait-postgres-login-budget.sh "$CTX" RUN_REGEX="$(./test/e2e/buckets.sh regex postgres-pitr)" cd test/e2e GAMEPLANE_E2E_REUSE_CLUSTER=1 GAMEPLANE_E2E_CLUSTER="$CLUSTER" \ @@ -2558,10 +2561,10 @@ jobs: # Smoke-tests the dev ingress-nginx manifest pin (deploy/kind/up.sh, # INGRESS_NGINX_VERSION) on the same Kind node config `make dev-up` uses - # (deploy/kind/cluster.yaml — the ingress-ready control-plane node label + # (deploy/kind/cluster.yaml — the ingress-ready control-plane node label # and the 8080/8443/30565 extraPortMappings the ingress-nginx Kind # manifest's controller Service depends on). Nothing else in CI applies - # this manifest (deploy/kind/e2e.sh deliberately skips ingress-nginx — see + # this manifest (deploy/kind/e2e.sh deliberately skips ingress-nginx — see # its header comment), so a pin bump whose kind manifest moved, or whose # controller no longer reaches Available on the runner's Kind node image, # was previously only caught by a developer running `make dev-up` (#521). @@ -2590,7 +2593,7 @@ jobs: - name: read ingress-nginx version pin from up.sh id: pin # Single source of truth: deploy/kind/up.sh's INGRESS_NGINX_VERSION= - # line. Never duplicate the version here — read it back instead, so + # line. Never duplicate the version here — read it back instead, so # this job smoke-tests whatever `make dev-up` actually installs. run: | VERSION="$(sed -n 's/^INGRESS_NGINX_VERSION="\(.*\)"$/\1/p' deploy/kind/up.sh)" @@ -2602,7 +2605,7 @@ jobs: echo "version=${VERSION}" >> "$GITHUB_OUTPUT" - name: create kind cluster (dev config) # Same node config make dev-up uses (deploy/kind/cluster.yaml), not - # e2e.sh's single-node config — the ingress-ready node label and the + # e2e.sh's single-node config — the ingress-ready node label and the # host port mappings it carries are exactly what this job is meant # to exercise. run: kind create cluster --name "$CLUSTER" --config deploy/kind/cluster.yaml @@ -2640,7 +2643,7 @@ jobs: # # `if: always()` so a red run still gets a report. Every data-gathering and # publishing step is defensive (set +e, continue-on-error) so a broken - # report can never turn a green build red or hide a real failure — a fork + # report can never turn a green build red or hide a real failure — a fork # PR's read-only token, for example, makes the comment step a no-op but # never fails the job. report: @@ -2975,7 +2978,7 @@ jobs: } // The ratelimit bucket runs as a trailing step inside the // api-auth job leg (deliberately, so it drains the shared - // login buckets last), not as its own job — so it never + // login buckets last), not as its own job — so it never // matches a job-name regex. Read the step-level conclusion: // the step is skipped (not "success") when the api-auth // bucket step itself failed first. diff --git a/deploy/kind/wait-postgres-login-budget.sh b/deploy/kind/wait-postgres-login-budget.sh new file mode 100644 index 000000000..c7cf2db30 --- /dev/null +++ b/deploy/kind/wait-postgres-login-budget.sh @@ -0,0 +1,10 @@ +#!/usr/bin/env bash +# Each independent suite gets the full published login budget. Use the +# database clock and leave the security state intact across process restarts. +set -euo pipefail +context=${1:?kind context required} +kubectl --context "$context" -n gameplane-system exec gameplane-postgres-0 -- \ + psql -XAt -v ON_ERROR_STOP=1 -U gameplane -d gameplane -c " + SELECT pg_sleep(GREATEST(0, COALESCE(max(next_us), 0) / 1000000.0 - + extract(epoch FROM statement_timestamp()))::double precision) + FROM api_rate_buckets WHERE budget IN ('login_ip', 'login_user')" From e5a9db89825dbaf25f82a29008a8adb3964edd20 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 01:06:57 -0500 Subject: [PATCH 10/23] fix(chart): fence legacy PostgreSQL binaries before they can write Signed-off-by: Baylem --- charts/gameplane/templates/api.yaml | 5 +++ charts/tests/check_standalone.py | 5 ++- docs/install.md | 6 +++- test/e2e/postgres_ha_e2e_test.go | 47 +++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 2 deletions(-) diff --git a/charts/gameplane/templates/api.yaml b/charts/gameplane/templates/api.yaml index e89e71828..817463044 100644 --- a/charts/gameplane/templates/api.yaml +++ b/charts/gameplane/templates/api.yaml @@ -276,6 +276,11 @@ spec: image: {{ include "gameplane.apiImage" . }} imagePullPolicy: {{ .Values.image.pullPolicy }} args: + {{- if $ha }} + # An old image must exit during flag parsing, before opening the + # shared database or starting workers, even when its Pod is unready. + - --postgres-ha-protocol=1 + {{- end }} {{- if .Values.api.standalone }} - --standalone - {{ printf "--panel-key-file=/keys/%s" ((.Values.api.panelKey | default dict).fileName | default "panel.key") | quote }} diff --git a/charts/tests/check_standalone.py b/charts/tests/check_standalone.py index 189fb99a6..0ef74af7d 100644 --- a/charts/tests/check_standalone.py +++ b/charts/tests/check_standalone.py @@ -54,6 +54,7 @@ def check_api_availability(): assert deployment["spec"]["replicas"] == 1 assert deployment["spec"]["strategy"] == {"type": "Recreate", "rollingUpdate": None} container = next(c for c in deployment["spec"]["template"]["spec"]["containers"] if c["name"] == "api") + assert not any(a.startswith("--postgres-ha-protocol") for a in container["args"]) assert container["livenessProbe"]["httpGet"]["path"] == "/healthz" expected_probe = "/readyz/postgres-ha" if driver == "postgres" and not standalone else "/readyz" assert container["readinessProbe"]["httpGet"]["path"] == expected_probe @@ -80,7 +81,9 @@ def check_api_availability(): assert dep["strategy"] == {"type": "RollingUpdate", "rollingUpdate": {"maxSurge": 0, "maxUnavailable": 1}} assert dep["minReadySeconds"] == 5 pod = dep["template"]["spec"] - assert next(c for c in pod["containers"] if c["name"] == "api")["lifecycle"]["preStop"]["sleep"]["seconds"] == 5 + container = next(c for c in pod["containers"] if c["name"] == "api") + assert container["lifecycle"]["preStop"]["sleep"]["seconds"] == 5 + assert "--postgres-ha-protocol=1" in container["args"] term = pod["affinity"]["podAntiAffinity"]["requiredDuringSchedulingIgnoredDuringExecution"][0] assert term["topologyKey"] == "kubernetes.io/hostname" assert term["labelSelector"]["matchLabels"]["app.kubernetes.io/name"] == "gameplane-api" diff --git a/docs/install.md b/docs/install.md index f3bec9cef..1b5a17771 100644 --- a/docs/install.md +++ b/docs/install.md @@ -1100,7 +1100,11 @@ when PostgreSQL returns. Replicating the API still requires a separate PostgreSQL failover and point-in-time recovery plan. To return to a single writer, first deploy the same HA-capable binary with `api.replicas=1` and `api.postgresHA.enabled=false`, then wait for the singleton rollout before -changing the application version. +changing the application version. HA Pods pass `--postgres-ha-protocol=1`; +older images exit at flag parsing before connecting to the database. A Helm +rollback reapplies the target revision's recorded manifests, including its own +strategy and arguments. Complete the singleton transition and confirm all old +HA Pods have terminated before rolling back to a chart that predates the flag. ### Moving a PostgreSQL DSN into a Secret diff --git a/test/e2e/postgres_ha_e2e_test.go b/test/e2e/postgres_ha_e2e_test.go index 5e5595ac8..610edbb4e 100644 --- a/test/e2e/postgres_ha_e2e_test.go +++ b/test/e2e/postgres_ha_e2e_test.go @@ -52,6 +52,7 @@ func TestPanel_PostgresActiveActiveFailover(t *testing.T) { if err := upgrade(); err != nil { t.Fatal(err) } + haLegacyBinaryFence(t) pair := haPodPair(t) clients := haDirectClients(t, pair, admin) _, _, id := envInstance.CreateUser(t, clients[0], "viewer", "ha-writer") @@ -153,6 +154,52 @@ func TestPanel_PostgresActiveActiveFailover(t *testing.T) { } haAudit(t, clients[0]) haAudit(t, clients[1]) + if err := upgrade("--set", "api.postgresHA.enabled=false", "--set", "api.replicas=1"); err != nil { + t.Fatal(err) + } + // Scale-down status alone may omit terminating Pods. Wait for complete + // termination before an operator could switch to an old release. + envInstance.Eventually(t, 2*time.Minute, func() (bool, string) { + live, err := envInstance.K8s.CoreV1().Pods(haNamespace).List(ctx, metav1.ListOptions{LabelSelector: "app.kubernetes.io/name=gameplane-api"}) + if err != nil || len(live.Items) != 1 { + return false, fmt.Sprintf("waiting for singleton termination: %v", err) + } + args := strings.Join(live.Items[0].Spec.Containers[0].Args, " ") + if strings.Contains(args, "--postgres-ha-protocol") { + return false, "waiting for singleton process argument set" + } + return haStatus(ctx, stable, "/users/me", http.StatusOK) + }) +} + +func haLegacyBinaryFence(t *testing.T) { + t.Helper() + const name = "ha-legacy-binary" + // Official 0.4.0 OCI index. The invalid DSN keeps the probe isolated from + // production data and distinguishes flag parsing from any DB-open attempt. + const legacy = "ghcr.io/gameplanepanel/gameplane/api@sha256:774229ab7f2d9ecdd0d43a742cd833124c299780b4623308208d87d4b3586f59" + no := false + pods := envInstance.K8s.CoreV1().Pods(haNamespace) + _, err := pods.Create(t.Context(), &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: corev1.PodSpec{RestartPolicy: corev1.RestartPolicyNever, AutomountServiceAccountToken: &no, + Containers: []corev1.Container{{Name: "api", Image: legacy, Args: []string{"--postgres-ha-protocol=1", "--db-driver=postgres", "--db-dsn=invalid"}}}}, + }, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + envInstance.Eventually(t, 2*time.Minute, func() (bool, string) { + pod, err := pods.Get(t.Context(), name, metav1.GetOptions{}) + if err != nil || len(pod.Status.ContainerStatuses) != 1 { + return false, fmt.Sprintf("waiting for legacy API: %v", err) + } + ended := pod.Status.ContainerStatuses[0].State.Terminated + return ended != nil && ended.ExitCode != 0, fmt.Sprintf("waiting for historical binary to reject HA argument: %v", pod.Status.Phase) + }) + logs, err := pods.GetLogs(name, &corev1.PodLogOptions{}).DoRaw(t.Context()) + if err != nil || !strings.Contains(string(logs), "flag provided but not defined: -postgres-ha-protocol") { + t.Fatalf("legacy binary did not reject the protocol at flag parsing: %v %s", err, logs) + } } func haEvictWorkerLeader(t *testing.T, pods []corev1.Pod, clients []*APIClient, id string) { From 05ff0f3de70ecac561a071a25b8809a06f03ad5d Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 01:09:06 -0500 Subject: [PATCH 11/23] fix(api): commit bootstrap administrator and role binding atomically Signed-off-by: Baylem --- api/cmd/bootstrap.go | 43 +++++++++++++++++++++------------------ api/cmd/bootstrap_test.go | 21 +++++++++++++++++++ 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/api/cmd/bootstrap.go b/api/cmd/bootstrap.go index 2a9c243a1..04454b707 100644 --- a/api/cmd/bootstrap.go +++ b/api/cmd/bootstrap.go @@ -84,14 +84,19 @@ func bootstrapAdmin(ctx context.Context, args []string, stdin io.Reader, stderr display = bf.username } + tx, err := store.BeginUserManagement(ctx) + if err != nil { + return fmt.Errorf("begin admin update: %w", err) + } + defer func() { _ = tx.Rollback() }() var existingID int64 - row := store.DB.QueryRowContext(ctx, `SELECT id FROM users WHERE username = ?`, bf.username) + row := tx.QueryRowContext(ctx, `SELECT id FROM users WHERE username = ?`, bf.username) switch err := row.Scan(&existingID); { case errors.Is(err, sql.ErrNoRows): // RETURNING id instead of LastInsertId: pgx's database/sql driver // has no LastInsertId, and both SQLite and Postgres support RETURNING. var newID int64 - if err := store.DB.QueryRowContext(ctx, + if err := tx.QueryRowContext(ctx, `INSERT INTO users(username, display_name, email, role, pw_hash) VALUES (?, ?, ?, 'admin', ?) RETURNING id`, bf.username, display, bf.email, hash, ).Scan(&newID); err != nil { @@ -100,9 +105,12 @@ func bootstrapAdmin(ctx context.Context, args []string, stdin io.Reader, stderr // Mirror the admin role into a cluster-wide role binding so RBAC // resolves the user's permissions (this runs after Migrate, so the // migration's backfill never saw this row). - if err := store.SetClusterRoleBinding(ctx, nil, newID, scope.DefaultCluster, "admin"); err != nil { + if err := store.SetClusterRoleBinding(ctx, tx, newID, scope.DefaultCluster, "admin"); err != nil { return fmt.Errorf("bind admin role: %w", err) } + if err := tx.Commit(); err != nil { + return fmt.Errorf("commit admin creation: %w", err) + } _, _ = fmt.Fprintf(stderr, "bootstrap-admin: created user %q\n", bf.username) return nil case err != nil: @@ -113,15 +121,6 @@ func bootstrapAdmin(ctx context.Context, args []string, stdin io.Reader, stderr return fmt.Errorf("user %q already exists; pass --force to overwrite password and promote to admin", bf.username) } - // Wrap password, role, and session eviction in a transaction so they - // commit together: no session outlives a reset that succeeded and - // nothing changes if any step fails. - tx, err := store.DB.BeginTx(ctx, nil) - if err != nil { - return fmt.Errorf("begin reset: %w", err) - } - defer func() { _ = tx.Rollback() }() - if _, err := tx.ExecContext(ctx, `UPDATE users SET pw_hash = ?, session_epoch = session_epoch + 1, role = 'admin', display_name = ?, email = ?, updated_at = ? @@ -220,16 +219,20 @@ func enableLocalLogin(ctx context.Context, store *db.Store, stderr io.Writer) er if err != nil { return fmt.Errorf("encode auth config: %w", err) } - if _, err := store.DB.ExecContext(ctx, - `INSERT INTO config(key, value, updated_at) - VALUES ('auth', ?, ?) - ON CONFLICT(key) DO UPDATE SET - value = excluded.value, - updated_at = excluded.updated_at`, - string(canon), db.NowTimestamp(), - ); err != nil { + // Refuse to overwrite an identity-provider change made on another API + // replica while this command was decoding the old configuration. + res, err := store.DB.ExecContext(ctx, `UPDATE config SET value = ?, updated_at = ? WHERE key = 'auth' AND value = ?`, + string(canon), db.NowTimestamp(), raw) + if err != nil { return fmt.Errorf("write auth config: %w", err) } + n, err := res.RowsAffected() + if err != nil { + return fmt.Errorf("confirm auth config write: %w", err) + } + if n != 1 { + return errors.New("auth config changed concurrently; retry enabling local login") + } _, _ = fmt.Fprintln(stderr, "bootstrap-admin: local login re-enabled in the auth config") return nil } diff --git a/api/cmd/bootstrap_test.go b/api/cmd/bootstrap_test.go index 65eb488d3..ad027ebbb 100644 --- a/api/cmd/bootstrap_test.go +++ b/api/cmd/bootstrap_test.go @@ -476,3 +476,24 @@ func TestBootstrap_ForceResetIsAllOrNothing(t *testing.T) { t.Fatalf("session count=%d want 1 (unchanged)", sessionCount) } } + +func TestBootstrap_NewAdminAndRoleBindingCommitTogether(t *testing.T) { + dsn := dsnIn(t) + auth.SetFastHashParams(t) + s := mustOpen(t, dsn) + ctx := t.Context() + if err := s.Migrate(ctx); err != nil { + t.Fatal(err) + } + if _, err := s.DB.ExecContext(ctx, `CREATE TRIGGER fail_admin_grant BEFORE INSERT ON user_role_bindings BEGIN SELECT RAISE(ABORT, 'blocked'); END`); err != nil { + t.Fatal(err) + } + t.Setenv("GAMEPLANE_ADMIN_PASSWORD", "") + if _, err := runBootstrap(t, dsn, "", "--username=recovery-admin", "--password=new-password"); err == nil { + t.Fatal("bootstrap accepted a failed administrator role binding") + } + var count int + if err := s.DB.QueryRowContext(ctx, "SELECT COUNT(*) FROM users WHERE username='recovery-admin'").Scan(&count); err != nil || count != 0 { + t.Fatalf("partial bootstrap left %d admin accounts: %v", count, err) + } +} From 2169957265ef855019103e618144e8fac22ab68d Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 01:11:27 -0500 Subject: [PATCH 12/23] fix(postgres): rotate remembered-device keys before reopening a restored panel Signed-off-by: Baylem --- deploy/kind/recover-postgres-wal.sh | 2 ++ docs/panel-recovery.md | 19 +++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/deploy/kind/recover-postgres-wal.sh b/deploy/kind/recover-postgres-wal.sh index a68feabf9..7cf2c1f1f 100644 --- a/deploy/kind/recover-postgres-wal.sh +++ b/deploy/kind/recover-postgres-wal.sh @@ -87,9 +87,11 @@ BEGIN; DELETE FROM oidc_links WHERE issuer='https://pitr.invalid' AND subject='revoked-subject'; DELETE FROM sessions; DELETE FROM api_tokens; +DELETE FROM api_runtime_keys WHERE purpose='login_device'; UPDATE share_links SET revoked_at=to_char(now() AT TIME ZONE 'UTC','YYYY-MM-DD"T"HH24:MI:SS"Z"') WHERE revoked_at IS NULL; COMMIT; SQL +[[ $(sql "$target" -c "SELECT count(*) FROM api_runtime_keys WHERE purpose='login_device'") == 0 ]] [[ $(sql "$target" -c "SELECT count(*) FROM oidc_links WHERE issuer='https://pitr.invalid'") == 0 ]] [[ $(sql "$target" -c "SELECT count(*) FROM oidc_links WHERE issuer='https://retained.pitr.invalid' AND subject='retained-subject' AND user_id=$viewer") == 1 ]] "${k[@]}" patch service gameplane-postgres --type=merge -p '{"spec":{"selector":{"app.kubernetes.io/name":"postgres-pitr"}}}' diff --git a/docs/panel-recovery.md b/docs/panel-recovery.md index e806c835a..0e2353aae 100644 --- a/docs/panel-recovery.md +++ b/docs/panel-recovery.md @@ -227,7 +227,18 @@ records the PostgreSQL target LSN, elapsed recovery-point age, time from loss to restored authorization and game verification, and the retained resource UIDs. Use actual successful job output when reporting recovery measurements. -Both backends require one API writer. Fence the original process and storage -before promotion. Active-active deployment would additionally require -database-level audit and last-manager serialization, coordinated migrations -and background jobs, and cross-replica sessions and rate limits. +During physical recovery, fence every API process and the old writable +PostgreSQL primary. Keep ingress closed while you reconcile IdP subjects, +owners and grants with the external revocation record, invalidate sessions +and API tokens, revoke share links and rotate the login-device key. Validate +a permitted action and a denied action against the recovered account. Check +the agent client certificate and CA, then read a known world marker through +the agent. The drill restores onto a separate PVC and samples the running +game throughout the outage. Its largest sampling gap bounds what the probe +can observe. + +Start one API against the recovered primary. After verifying the audit chain, +live game identity and retained claims, native PostgreSQL deployments can +resume their configured replica count. The HA CI drill exercises a two-writer +rolling upgrade, session revocation, election after replica loss, loss of one +worker node and a PostgreSQL outage in a separate three-node cluster. From e0a52c4a6ce1d9704e0d4782eb1720e5d83c211b Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 01:14:35 -0500 Subject: [PATCH 13/23] fix(api): deduplicate notification dispatch during PostgreSQL leader loss Signed-off-by: Baylem --- api/cmd/runtime_postgres_test.go | 25 +++++++++ api/internal/auth/sessions.go | 3 ++ .../common/021_notification_claims.sql | 10 ++++ api/internal/db/runtime_claims.go | 23 ++++++++ api/internal/notify/notify.go | 36 +++++++++---- api/internal/notify/poll_standalone.go | 3 +- api/internal/notify/runtime_test.go | 54 +++++++++++++++++++ api/internal/notify/watch.go | 13 +++-- docs/panel-recovery.md | 11 +++- 9 files changed, 159 insertions(+), 19 deletions(-) create mode 100644 api/internal/db/migrations/common/021_notification_claims.sql create mode 100644 api/internal/db/runtime_claims.go create mode 100644 api/internal/notify/runtime_test.go diff --git a/api/cmd/runtime_postgres_test.go b/api/cmd/runtime_postgres_test.go index db77913c5..3c8251e99 100644 --- a/api/cmd/runtime_postgres_test.go +++ b/api/cmd/runtime_postgres_test.go @@ -222,6 +222,31 @@ func TestPostgresWorkersRecoverWhenLeaderBackendDies(t *testing.T) { } waitSignal(t, stopped) waitSignal(t, standby) + // Killing only the lock's backend leaves the former replica's other + // database connections and already-queued observations alive. Both + // replicas may attempt the same sink after ownership changes. + var sent atomic.Int32 + var attempts sync.WaitGroup + for _, replica := range stores { + attempts.Go(func() { + won, err := replica.ClaimNotification(t.Context(), "", "server-uid", "271", "server.unhealthy", "ops") + if err != nil { + t.Error(err) + return + } + if won { + sent.Add(1) + } + }) + } + attempts.Wait() + if sent.Load() != 1 { + t.Fatalf("dispatched one transition %d times after backend loss", sent.Load()) + } + won, err := stores[1].ClaimNotification(t.Context(), "", "server-uid", "272", "server.unhealthy", "ops") + if err != nil || !won { + t.Fatalf("distinct object update lost after failover: %v", err) + } cancelSecond() waitSignal(t, finished) } diff --git a/api/internal/auth/sessions.go b/api/internal/auth/sessions.go index 130587289..08e7e32f3 100644 --- a/api/internal/auth/sessions.go +++ b/api/internal/auth/sessions.go @@ -126,6 +126,9 @@ func (s *SessionStore) RunGC(ctx context.Context, interval time.Duration) { if _, err := s.db.DB.ExecContext(ctx, `DELETE FROM api_rate_buckets WHERE next_us < floor(extract(epoch FROM statement_timestamp())*1000000)::bigint`); err != nil && ctx.Err() == nil { slog.Warn("rate limit gc", "err", err) } + if _, err := s.db.DB.ExecContext(ctx, `DELETE FROM notification_claims WHERE claimed_at < CURRENT_TIMESTAMP - INTERVAL '7 days'`); err != nil && ctx.Err() == nil { + slog.Warn("notification claim gc", "err", err) + } } } } diff --git a/api/internal/db/migrations/common/021_notification_claims.sql b/api/internal/db/migrations/common/021_notification_claims.sql new file mode 100644 index 000000000..8379d310b --- /dev/null +++ b/api/internal/db/migrations/common/021_notification_claims.sql @@ -0,0 +1,10 @@ +CREATE TABLE notification_claims ( + cluster TEXT NOT NULL, + object_uid TEXT NOT NULL, + object_version TEXT NOT NULL, + event_type TEXT NOT NULL, + sink_name TEXT NOT NULL, + claimed_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (cluster, object_uid, object_version, event_type, sink_name) +); +CREATE INDEX notification_claims_expiry ON notification_claims(claimed_at); diff --git a/api/internal/db/runtime_claims.go b/api/internal/db/runtime_claims.go new file mode 100644 index 000000000..03b85cebf --- /dev/null +++ b/api/internal/db/runtime_claims.go @@ -0,0 +1,23 @@ +package db + +import ( + "context" + "errors" +) + +// ClaimNotification reserves one observed Kubernetes transition per sink. +// The transaction commits before any external HTTP or SMTP delivery begins. +func (s *Store) ClaimNotification(ctx context.Context, cluster, uid, version, eventType, sink string) (bool, error) { + if uid == "" || version == "" { + return false, errors.New("notification is missing its Kubernetes identity") + } + result, err := s.DB.ExecContext(ctx, `INSERT INTO notification_claims + (cluster, object_uid, object_version, event_type, sink_name) VALUES (?, ?, ?, ?, ?) + ON CONFLICT (cluster, object_uid, object_version, event_type, sink_name) DO NOTHING`, + cluster, uid, version, eventType, sink) + if err != nil { + return false, err + } + changed, err := result.RowsAffected() + return changed == 1, err +} diff --git a/api/internal/notify/notify.go b/api/internal/notify/notify.go index 078648749..5b4f22b19 100644 --- a/api/internal/notify/notify.go +++ b/api/internal/notify/notify.go @@ -49,17 +49,19 @@ const queueSize = 256 // (language, title, text); they are additive and every field above keeps its // name and value. type Event struct { - Cluster string `json:"cluster,omitempty"` - Type EventType `json:"type"` - TS string `json:"ts"` - Kind string `json:"kind,omitempty"` // CRD kind: GameServer | Backup | Restore - Name string `json:"name,omitempty"` - Namespace string `json:"namespace,omitempty"` - Reason string `json:"reason,omitempty"` - Message string `json:"message,omitempty"` - Instance string `json:"instance,omitempty"` - Test bool `json:"test,omitempty"` - lang string // BCP 47 rendering language; "" renders English. Never serialized. + Cluster string `json:"cluster,omitempty"` + Type EventType `json:"type"` + TS string `json:"ts"` + Kind string `json:"kind,omitempty"` // CRD kind: GameServer | Backup | Restore + Name string `json:"name,omitempty"` + Namespace string `json:"namespace,omitempty"` + Reason string `json:"reason,omitempty"` + Message string `json:"message,omitempty"` + Instance string `json:"instance,omitempty"` + Test bool `json:"test,omitempty"` + lang string // BCP 47 rendering language; "" renders English. Never serialized. + objectUID string + objectVersion string } // Notifier watches CRD status transitions and delivers matching events to @@ -153,6 +155,18 @@ func (n *Notifier) fanOut(ctx context.Context, e Event) { } ev := e ev.lang = resolveLanguage(s.Language, def) + if n.store.Driver == "postgres" { + claimCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + won, err := n.store.ClaimNotification(claimCtx, e.Cluster, e.objectUID, e.objectVersion, string(e.Type), s.Name) + cancel() + if err != nil { + deliveries.WithLabelValues(s.Kind, "failed").Inc() + slog.Warn("notification dispatch claim failed", "sink", s.Name, "err", err) + } + if !won || err != nil { + continue + } + } n.deliverWithRetry(ctx, s, secret, ev) } } diff --git a/api/internal/notify/poll_standalone.go b/api/internal/notify/poll_standalone.go index b21feb7b4..bc1c3a54f 100644 --- a/api/internal/notify/poll_standalone.go +++ b/api/internal/notify/poll_standalone.go @@ -90,8 +90,7 @@ func (n *Notifier) pollClusterSnapshot(ctx context.Context, k *kube.Client, clus events = phaseEvents(resourceKind, old, obj) } for _, e := range events { - e.Cluster = cluster - n.Enqueue(stampTS(e)) + n.enqueueObserved(e, cluster, obj) } } if kind == "servers" { diff --git a/api/internal/notify/runtime_test.go b/api/internal/notify/runtime_test.go new file mode 100644 index 000000000..4af6d03af --- /dev/null +++ b/api/internal/notify/runtime_test.go @@ -0,0 +1,54 @@ +package notify + +import ( + "net/http" + "net/http/httptest" + "sync" + "sync/atomic" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + kubefake "k8s.io/client-go/kubernetes/fake" + + "github.com/GameplanePanel/gameplane/api/internal/kube" +) + +func TestPostgresNotifierDispatchesEachObservedVersionOncePerSink(t *testing.T) { + store := newTestStore(t) + putConfig(t, store, "notifications", `{"sinks":[{"name":"ops","kind":"webhook","enabled":true,"configRef":"hook","events":["backup.failed"]}]}`) + // The claim statement is portable. The cmd PostgreSQL test races real + // independent pgx pools after terminating the worker leader's backend. + store.Driver = "postgres" + var sent atomic.Int32 + endpoint := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { sent.Add(1); w.WriteHeader(http.StatusNoContent) })) + defer endpoint.Close() + secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "hook", Namespace: "panel", Labels: map[string]string{SinkSecretLabel: "true"}}, Data: map[string][]byte{"url": []byte(endpoint.URL)}} + replicas := []*Notifier{ + New(store, &kube.Client{Typed: kubefake.NewClientset(secret.DeepCopy())}, "panel"), + New(store, &kube.Client{Typed: kubefake.NewClientset(secret.DeepCopy())}, "panel"), + } + for _, n := range replicas { + n.client = testNotifier().client + } + obj := &unstructured.Unstructured{} + obj.SetUID("uid") + obj.SetResourceVersion("37") + event := Event{Type: EventBackupFailed, Kind: "Backup", Name: "world", Namespace: "games"} + var calls sync.WaitGroup + for _, n := range replicas { + calls.Go(func() { n.enqueueObserved(event, "edge", obj); n.fanOut(t.Context(), <-n.ch) }) + } + calls.Wait() + if got := sent.Load(); got != 1 { + t.Fatalf("HTTP deliveries from overlapping workers: %d", got) + } + obj.SetResourceVersion("38") + n := replicas[1] + n.enqueueObserved(event, "edge", obj) + n.fanOut(t.Context(), <-n.ch) + if got := sent.Load(); got != 2 { + t.Fatalf("distinct update lost after prior delivery: %d", got) + } +} diff --git a/api/internal/notify/watch.go b/api/internal/notify/watch.go index 9062d8e51..8cf5c9e93 100644 --- a/api/internal/notify/watch.go +++ b/api/internal/notify/watch.go @@ -58,8 +58,7 @@ func (n *Notifier) runClusterWatchers(ctx context.Context, k *kube.Client, clust } mu.Unlock() for _, e := range events { - e.Cluster = cluster - n.Enqueue(stampTS(e)) + n.enqueueObserved(e, cluster, newU) } }, DeleteFunc: func(obj any) { @@ -88,8 +87,7 @@ func (n *Notifier) runClusterWatchers(ctx context.Context, k *kube.Client, clust return } for _, e := range phaseEvents(kind, oldU, newU) { - e.Cluster = cluster - n.Enqueue(stampTS(e)) + n.enqueueObserved(e, cluster, newU) } }, }); err != nil { @@ -104,6 +102,13 @@ func (n *Notifier) runClusterWatchers(ctx context.Context, k *kube.Client, clust factory.WaitForCacheSync(ctx.Done()) } +func (n *Notifier) enqueueObserved(e Event, cluster string, object *unstructured.Unstructured) { + e.Cluster = cluster + e.objectUID = string(object.GetUID()) + e.objectVersion = object.GetResourceVersion() + n.Enqueue(stampTS(e)) +} + func stampTS(e Event) Event { e.TS = time.Now().UTC().Format(time.RFC3339) return e diff --git a/docs/panel-recovery.md b/docs/panel-recovery.md index e806c835a..8ff1e420f 100644 --- a/docs/panel-recovery.md +++ b/docs/panel-recovery.md @@ -205,8 +205,15 @@ database connection per active leader. The leader holds a read-committed transaction advisory lock for its tenure and heartbeats every two seconds. The database or pooler's maximum transaction age must accommodate this connection. After a backend loss, the replica cancels these workers and the -standby retries election every two seconds. Notification sinks receive -best-effort delivery across a failover. Fence the old writable primary before +standby retries election every two seconds. A prior worker can still have a +request in flight. PostgreSQL reserves each notification's cluster, object UID, +resource version, event type and sink before delivery, so overlapping workers +cannot start two deliveries of the same observation. Claims expire after seven +days. An API crash between reservation and delivery can lose that notification. +HTTP or SMTP retries after an ambiguous transport response can still duplicate +a message at the receiver. Telemetry claims each report slot in SQL, retention +uses the audit-chain lock, and expired session/budget deletion is idempotent. +Fence the old writable primary before enabling the new primary. ### Physical recovery drill From 76d57e9b5c3fef6950a3c24241ddd7caed25a682 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 01:18:20 -0500 Subject: [PATCH 14/23] docs(api): describe native PostgreSQL writer and rate-limit contracts Signed-off-by: Baylem --- api/specs.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/specs.md b/api/specs.md index 6b1320475..ddfc8d9b5 100644 --- a/api/specs.md +++ b/api/specs.md @@ -123,7 +123,7 @@ The HTTP server listens on `:8000` (configurable) with these route groups. Prome - `/readyz` — GET: database readiness, a `PingContext` with a two-second deadline. At most one ping runs at once; concurrent probes return a generic 503 without queueing more database work. Success returns `ok`; unavailable or busy returns only `not ready`. The chart probes every five seconds with a three-second timeout; three consecutive failed probes remove readiness, tolerating brief SQLite connection contention. `/healthz` remains independent of the database, so outages do not trigger liveness restarts. **Protected (authenticated + RBAC):** -- All six `/fleet/*` reads share a per-authenticated-user token bucket: 60 requests/minute sustained, burst 10. Exhausted requests receive 429 with `Retry-After: 1` before cluster discovery or resource listing. Sessions and routes share the account's budget; users behind the same IP do not. This per-process limit bounds request frequency without removing owner/collaborator scans; multiple API replicas have independent budgets. +- All six `/fleet/*` reads share a per-authenticated-user token bucket: 60 requests/minute sustained, burst 10. Exhausted requests receive 429 with `Retry-After: 1` before cluster discovery or resource listing. Sessions and routes share the account's budget; users behind the same IP do not. The SQLite API keeps the budget in memory. PostgreSQL replicas share the same account budget and still enforce owner/collaborator checks. - `/namespaces` — GET: namespaces the caller may read servers in (scope.AllowedNamespaces filtered by servers:read on the resolved `?cluster=`); lets the dashboard fan out `/servers?namespace=` across every namespace it can see instead of only scope.Resolve's default (F-263) - `/fleet/servers`, `/fleet/backups`, `/fleet/schedules`, `/fleet/restores` — GET, authenticated: unified reads across registered clients and persisted registrations, restricted to configured allowed namespaces. Optional exact `cluster` and `namespace` filters narrow the backend read; omission means all eligible scopes, independent of the old selected-cluster default. Duplicate filters, wildcard IDs and invalid namespaces are rejected. A valid unknown or unauthorized cluster filter returns an empty filtered result without confirming registration existence. - Resource envelope: `{items:[{target:{cluster,namespace,name,uid},resource,permissions,access?}],partial,issues,totalReturned,scopes?}`. `resource` is the original Kubernetes object; server heartbeat freshness is projected as on existing reads. `target.uid` distinguishes replacement objects in row/cache identity; it does not add mutation preconditions to existing CRUD. `permissions` is the sorted catalogue of namespaced permissions actually held at that target, never a union of other clusters' grants. @@ -531,7 +531,7 @@ audit:read, config:read, config:manage (cluster-scoped) 3. **Three-role baseline RBAC:** admin/operator/viewer roles reproduce historical permission matrix exactly 4. **Multi-dimensional RBAC:** namespace + cluster + owner/collaborator dimensions; cluster gating prevents cross-cluster privilege escalation 5. **Append-only migrations:** database schema mutations are irreversible (migrations 001-014); no down-migrations -6. **Login rate limiting:** `/auth/login` is per-IP (burst 10, 5/min, `LoginLimiter`) plus a per-username throttle layered on top (burst 6, 3/min, `LoginUserLimiter` in `auth/local.go`), which a browser carrying a valid `gameplane_login_device` cookie for that username (set on a successful login, HMAC under a per-process key) skips; the OIDC callback routes (`/auth/oidc/{provider}/callback`, legacy `/auth/oidc/callback`) are per-IP only (burst 10, 10/min via `OIDCCallbackLimiter`), no per-user dimension +6. **Login rate limiting:** `/auth/login` is per-IP (burst 10, 5/min, `LoginLimiter`) plus a per-username throttle layered on top (burst 6, 3/min, `LoginUserLimiter` in `auth/local.go`), which a browser carrying a valid `gameplane_login_device` cookie for that username (set on a successful login, HMAC using a process-local SQLite key or a shared PostgreSQL key with a signed expiry) skips; the OIDC callback routes (`/auth/oidc/{provider}/callback`, legacy `/auth/oidc/callback`) are per-IP only (burst 10, 10/min via `OIDCCallbackLimiter`), no per-user dimension 7. **Audit hash-chain:** each audit_events row includes hash of previous row (prev_hash) + its own content hash (hash); detects DB-level UPDATE/DELETE tampering 8. **Audit pagination is bounded:** The `Auditor.Page(ctx, limit)` method clamps the untrusted `limit` parameter to a maximum of 500 entries (`MaxAuditPageSize`). Clamping occurs at both the API handler layer (api/internal/handlers/audit.go line 25) and the store layer (api/internal/audit/audit.go lines 820–822) so untrusted input is bounded at the earliest opportunity and again at use time. The allocated slice is always created with capacity within the bound (`make([]Event, 0, limit)` after clamping), guaranteeing that no untrusted client input can cause unbounded memory allocation regardless of how the limit value flows through the system. 9. **Session CSRF protection:** CSRF token paired with session token; validated on state-changing requests. The CSRF cookie is deliberately **not** `HttpOnly` — the SPA reads it via JS and echoes it back as the `X-Gameplane-CSRF` header (double-submit pattern); making it `HttpOnly` would break the protection it's providing. The session cookie itself stays `HttpOnly`. Cookie *clearing* (logout) always sends `HttpOnly` regardless of the original cookie, since a delete carries no value for a script to read and the browser matches the clear on Name/Domain/Path alone. This is why the `.golangci.yml` gosec G124 exclusion is scoped narrowly to `api/internal/auth/sessions.go`. @@ -571,8 +571,8 @@ Verify from `/api/go.mod`. ### Database driver selection - **Production (default):** `modernc.org/sqlite` — file-based, WAL mode, tested -- **Experimental:** PostgreSQL via `jackc/pgx/v5` — compile with `-tags=postgres`. The `api (postgres)` CI job covers the store. The `e2e postgres` job runs API auth/roles/RBAC plus the `postgres-availability` bucket: populated RollingUpdate-to-Recreate migration, singleton Pod observation, audit integrity, and database outage/recovery without restarting the API or replacing the session. -- Every enabled API install requires exactly one replica, for both drivers and both combined/standalone modes. Helm renders `Recreate` with `rollingUpdate: null`; the combined-install pre-upgrade hook clears an existing rolling-update strategy for either driver. Recreate prevents ordinary rollout overlap, but cannot fence a process on a partitioned node after force deletion. HA requires database-level serialization of audit/user-management transactions and coordinated background work/rate limiting before multiple writers are supported. +- The release image includes PostgreSQL through the `postgres` Go build tag. The PostgreSQL integration job uses a real database for migration, authorization, rate-limit, audit and notification concurrency. The kind job exercises the same image during populated singleton upgrade, fenced WAL replay and multi-replica failover. +- SQLite, standalone and default native PostgreSQL installations run one API with `Recreate`. Native installations can enable `api.postgresHA.enabled` with a Secret-backed DSN and two or more replicas. The installed singleton must first pass the HA protocol readiness check. PostgreSQL serializes migrations, authorization changes and audit-chain updates inside their write transactions. An elected replica runs periodic jobs. Follow the staged rollout and recovery procedure. [Installation](../docs/install.md#api-rollout-and-postgresql-availability) · [PostgreSQL recovery](../docs/panel-recovery.md#postgresql-physical-recovery) - Protected-route session lookup distinguishes missing, expired and corrupt sessions (generic 401) from database/session-permission lookup failures (generic 503). Both deny access. Operational failures do not delete sessions or clear cookies; access recovers with the same cookie when the store recovers. Missing-cookie responses remain 401 without consulting storage, and login privacy is unchanged. - Driver selected at startup via `--db-driver` (sqlite|postgres) + `--db-dsn` - SQLite DSNs automatically receive a `busy_timeout(5000)` pragma if one is not already specified, allowing multiple processes (e.g., the API and bootstrap-admin) sharing the same file to wait for locks instead of failing immediately From 1c735ca0c21461b7ef23d97505e316ce41a217c9 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 01:20:38 -0500 Subject: [PATCH 15/23] test(e2e): group PostgreSQL qualification with upgrade suites Signed-off-by: Baylem --- test/e2e/suite-components.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/suite-components.json b/test/e2e/suite-components.json index b720f1f49..fc0cfce38 100644 --- a/test/e2e/suite-components.json +++ b/test/e2e/suite-components.json @@ -16,7 +16,6 @@ "test/e2e/suite-components.json" ], "suites": [ - { "tests": "^TestPanel_Postgres(ActiveActiveFailover|PointInTimeRecovery)$", "components": ["api", "operator", "agent"] }, { "tests": "^TestColdWakeFleet_ConcurrentJoins$", "components": ["operator", "sentinel", "gameproto", "agent"], "paths": ["test/e2e/wake_burst.py", "test/e2e/cmd/join-once/", "test/e2e/testdata/wake-burst/"] }, { "tests": "^TestGameServer_HostportWakeStableEndpoint$", "components": ["operator", "api", "agent", "sentinel", "gameproto"] }, { "tests": "^(TestPanel_OfflineRecovery|TestAPIClient_DoContextResponseBody|TestAPIClient_ExtractIntField)$", "components": ["operator", "api", "agent"], @@ -57,6 +56,7 @@ "paths": ["test/e2e/internal/minecraft-java/", "test/e2e/internal/probe/", "test/e2e/gameprobe_job.go"] }, { "tests": "^TestMultiCluster_", "components": ["operator", "api", "agent", "capture-sidecar"] }, { "tests": "^TestUpgrade_", "components": ["operator", "api", "agent"] }, + { "tests": "^TestPanel_Postgres(ActiveActiveFailover|PointInTimeRecovery)$", "components": ["api", "operator", "agent"] }, { "tests": "^TestTelemetry", "components": ["operator", "api", "telemetry-receiver", "telemetryschema"], "paths": ["test/e2e/telemetry_e2e_helpers_test.go", "test/e2e/telemetry_client_test.go", "test/e2e/telemetry_pow_e2e_test.go"] }, { "tests": "^TestOptionalComponents_", From 763108bb2b94b8c48e8b5cbd4b70141dd6c2caa1 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 02:52:49 -0500 Subject: [PATCH 16/23] fix(api): reject read-only PostgreSQL sessions during failover Signed-off-by: Baylem --- api/cmd/runtime_postgres_test.go | 72 ++++++++++++++++++++++++++++++++ api/internal/db/db_postgres.go | 28 ++++++++++++- docs/panel-recovery.md | 7 +++- 3 files changed, 104 insertions(+), 3 deletions(-) diff --git a/api/cmd/runtime_postgres_test.go b/api/cmd/runtime_postgres_test.go index 3c8251e99..66fad57af 100644 --- a/api/cmd/runtime_postgres_test.go +++ b/api/cmd/runtime_postgres_test.go @@ -17,6 +17,11 @@ import ( "testing" "time" + "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" + "github.com/jackc/pgx/v5/stdlib" + "github.com/GameplanePanel/gameplane/api/internal/auth" "github.com/GameplanePanel/gameplane/api/internal/db" ) @@ -66,6 +71,73 @@ func runtimeReplicas(t *testing.T) [2]*db.Store { return stores } +func TestPostgresReadOnlyFence(t *testing.T) { + ctx := t.Context() + fixture := runtimeReplicas(t)[0] + var schema string + if err := fixture.DB.QueryRowContext(ctx, "SELECT current_schema()").Scan(&schema); err != nil { + t.Fatal(err) + } + cfg, err := pgx.ParseConfig(os.Getenv("GAMEPLANE_TEST_POSTGRES_DSN")) + if err != nil { + t.Fatal("invalid PostgreSQL fixture configuration") + } + cfg.RuntimeParams["search_path"] = schema + var fenced atomic.Bool + cfg.AfterConnect = func(ctx context.Context, conn *pgconn.PgConn) error { + if !fenced.Load() { + return nil + } + _, err := conn.Exec(ctx, "SET default_transaction_read_only=on").ReadAll() + return err + } + dsn := stdlib.RegisterConnConfig(cfg) + t.Cleanup(func() { stdlib.UnregisterConnConfig(dsn) }) + store, err := db.Open(ctx, "postgres", dsn) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + store.DB.SetMaxOpenConns(1) + var id int64 + if err := store.DB.QueryRowContext(ctx, "INSERT INTO users(username,display_name,email,role) VALUES ('fence-viewer','','','viewer') RETURNING id").Scan(&id); err != nil { + t.Fatal(err) + } + sessions := auth.NewSessionStore(store) + token, _, err := sessions.Create(ctx, id) + if err != nil { + t.Fatal(err) + } + r := chi.NewRouter() + mountHealthRoutes(r, store) + r.With(sessions.Authenticate).Get("/protected", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) + check := func(path string, want int) { + t.Helper() + req := httptest.NewRequestWithContext(ctx, http.MethodGet, path, nil) + req.AddCookie(&http.Cookie{Name: "gameplane_session", Value: token}) + resp := httptest.NewRecorder() + r.ServeHTTP(resp, req) + if resp.Code != want { + t.Fatalf("%s: got HTTP %d, want %d", path, resp.Code, want) + } + } + check("/protected", http.StatusOK) + check("/readyz/postgres-ha", http.StatusOK) + // Fence the already pooled connection and all future dials without + // changing a global setting on the shared test database. + fenced.Store(true) + if _, err := store.DB.ExecContext(ctx, "SET default_transaction_read_only=on"); err != nil { + t.Fatal(err) + } + check("/protected", http.StatusServiceUnavailable) + check("/readyz", http.StatusServiceUnavailable) + check("/readyz/postgres-ha", http.StatusServiceUnavailable) + check("/healthz", http.StatusOK) + fenced.Store(false) + check("/readyz/postgres-ha", http.StatusOK) + check("/protected", http.StatusOK) +} + func TestPostgresRuntimeAcrossReplicas(t *testing.T) { stores := runtimeReplicas(t) ctx := t.Context() diff --git a/api/internal/db/db_postgres.go b/api/internal/db/db_postgres.go index 883c7fc15..3cfcf80d4 100644 --- a/api/internal/db/db_postgres.go +++ b/api/internal/db/db_postgres.go @@ -10,6 +10,8 @@ import ( "fmt" "time" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/stdlib" ) @@ -33,6 +35,7 @@ func openPostgres(ctx context.Context, dsn string) (*Store, error) { // them (context-aware exec/query, transactions with options, named-value // checking, session reset) on the wrapper. type pgxConn interface { + Conn() *pgx.Conn driver.Conn driver.ConnBeginTx driver.ConnPrepareContext @@ -77,17 +80,38 @@ func (c rebindConnector) Connect(ctx context.Context) (driver.Conn, error) { _ = conn.Close() return nil, fmt.Errorf("pgx connection %T lacks a required database/sql driver interface", conn) } + if err := pgconn.ValidateConnectTargetSessionAttrsReadWrite(ctx, pc.Conn().PgConn()); err != nil { + _ = conn.Close() + return nil, fmt.Errorf("check writable PostgreSQL connection: %w", err) + } return rebindConn{pgxConn: pc}, nil } func (c rebindConnector) Driver() driver.Driver { return c.base.Driver() } -// rebindConn forwards everything to the pgx connection except the four -// entry points that take a query string, which it rebinds first. +// rebindConn translates query placeholders and checks the database role +// when database/sql lends an idle connection to a new caller. type rebindConn struct { pgxConn } +// An existing connection can remain reachable after the database has been +// fenced read-only. Stale sessions and permissions must not authorize direct +// requests or streams while Kubernetes withdraws an unready API endpoint. +func (c rebindConn) ResetSession(ctx context.Context) error { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + if err := c.pgxConn.ResetSession(ctx); err != nil { + return err + } + if err := pgconn.ValidateConnectTargetSessionAttrsReadWrite(ctx, c.Conn().PgConn()); err != nil { + // database/sql only discards a borrowed connection for ErrBadConn. + // A replacement must pass the same check in Connect. + return errors.Join(driver.ErrBadConn, err) + } + return nil +} + func (c rebindConn) Prepare(query string) (driver.Stmt, error) { return c.pgxConn.PrepareContext(context.Background(), Rebind("postgres", query)) } diff --git a/docs/panel-recovery.md b/docs/panel-recovery.md index 3022779af..aaedd5973 100644 --- a/docs/panel-recovery.md +++ b/docs/panel-recovery.md @@ -182,7 +182,12 @@ Kubernetes disaster recovery exercise. All API replicas must use the same writable PostgreSQL database and schema, the same configuration and credentials, and the same trusted ingress proxy -ranges. Protected traffic and authentication budgets go to the primary. A +ranges. For a multi-host DSN use `target_session_attrs=read-write` so pgx +can select a writable host. The API checks PostgreSQL's transaction mode +on each new connection and each checkout from its pool. A reachable but +read-only endpoint fails readiness and cannot authorize a new request or +revalidate an existing stream. The database platform still must fence the +old primary before promoting another one. A chart-managed HA Pod starts with `--postgres-ha-protocol=1`. A legacy binary rejects this flag before opening the database. To downgrade to a version without this protocol, return to one writer before changing images. A From 0e69df050115118294fcd955ec114a73497e0b13 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 03:57:47 -0500 Subject: [PATCH 17/23] fix(auth): preserve provider fallback using config lookup presence Signed-off-by: Baylem --- api/internal/auth/registry.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/internal/auth/registry.go b/api/internal/auth/registry.go index aaf25ed4b..3624f40d5 100644 --- a/api/internal/auth/registry.go +++ b/api/internal/auth/registry.go @@ -200,7 +200,7 @@ func (r *Registry) snapshot(ctx context.Context) ([]Provider, [sha256.Size]byte, if err != nil && r.store.Driver == "postgres" { return nil, [sha256.Size]byte{}, err } - if err != nil || !ok { + if !ok { return defaultProviders(), sha256.Sum256(nil), nil } var cfg struct { @@ -224,7 +224,7 @@ func (r *Registry) HelmRoleOverrides(ctx context.Context) (*RoleMappings, error) if err != nil && r.store.Driver == "postgres" { return nil, err } - if err != nil || !ok { + if !ok { return nil, nil } var cfg struct { From 59fa184a486c25b95c14a7a355ad33463c013622 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 04:01:53 -0500 Subject: [PATCH 18/23] fix(e2e): close and validate HA qualification responses Signed-off-by: Baylem --- test/e2e/postgres_ha_e2e_test.go | 34 +++++++++++++------------- test/e2e/telemetry_e2e_helpers_test.go | 4 +-- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/test/e2e/postgres_ha_e2e_test.go b/test/e2e/postgres_ha_e2e_test.go index 610edbb4e..1e4424157 100644 --- a/test/e2e/postgres_ha_e2e_test.go +++ b/test/e2e/postgres_ha_e2e_test.go @@ -288,8 +288,8 @@ func haDirectClients(t *testing.T, pods []corev1.Pod, admin *APIClient) []*APICl func haPatch(t *testing.T, client *APIClient, id, displayName string) { t.Helper() - resp, body, err := client.DoContext(t.Context(), http.MethodPatch, "/users/"+id, map[string]string{"displayName": displayName}) - if err != nil || resp.StatusCode != http.StatusOK { + status, body, err := tlDo(t.Context(), client, http.MethodPatch, "/users/"+id, map[string]string{"displayName": displayName}) + if err != nil || status != http.StatusOK { t.Fatalf("write through API replica: %v %s", err, body) } } @@ -300,8 +300,8 @@ func haConcurrentWrites(t *testing.T, clients []*APIClient, id string) { for n, client := range clients { writers.Go(func() { for i := range 6 { - resp, body, err := client.DoContext(t.Context(), http.MethodPatch, "/users/"+id, map[string]string{"displayName": fmt.Sprintf("writer-%d-%d", n, i)}) - if err != nil || resp.StatusCode != http.StatusOK { + status, body, err := tlDo(t.Context(), client, http.MethodPatch, "/users/"+id, map[string]string{"displayName": fmt.Sprintf("writer-%d-%d", n, i)}) + if err != nil || status != http.StatusOK { t.Errorf("writer %d: %v %s", n, err, body) return } @@ -329,9 +329,9 @@ func haSessionAndLimiter(t *testing.T, clients []*APIClient) { // Retain the original cookie in a second jar when B deletes its copy. username, password, _ := envInstance.CreateUser(t, clients[0], "viewer", "ha-session") first := &APIClient{BaseURL: clients[0].BaseURL, HTTP: &http.Client{Jar: newInsecureCookieJar(), Timeout: 5 * time.Second}} - resp, body, err := first.Post("/auth/login", map[string]string{"username": username, "password": password}) + status, body, err := tlDo(t.Context(), first, http.MethodPost, "/auth/login", map[string]string{"username": username, "password": password}) var login struct{ CSRF string } - if err != nil || resp.StatusCode != 200 || json.Unmarshal(body, &login) != nil || login.CSRF == "" { + if err != nil || status != 200 || json.Unmarshal(body, &login) != nil || login.CSRF == "" { t.Fatalf("viewer login on A did not return an authenticated session: %v", err) } assertAPIStatus(t, first, "/users/me", 200) @@ -339,20 +339,20 @@ func haSessionAndLimiter(t *testing.T, clients []*APIClient) { b, _ := url.Parse(clients[1].BaseURL) second := &APIClient{BaseURL: clients[1].BaseURL, CSRF: login.CSRF, HTTP: &http.Client{Jar: newInsecureCookieJar(), Timeout: 5 * time.Second}} second.HTTP.Jar.SetCookies(b, first.HTTP.Jar.Cookies(a)) - resp, _, err = second.Post("/auth/logout", nil) - if err != nil || resp.StatusCode/100 != 2 { - t.Fatalf("logout on B: %v %v", err, resp) + status, _, err = tlDo(t.Context(), second, http.MethodPost, "/auth/logout", nil) + if err != nil || status/100 != 2 { + t.Fatalf("logout on B: %v %v", err, status) } assertAPIStatus(t, first, "/users/me", http.StatusUnauthorized) for attempt := range 7 { anonymous := &APIClient{BaseURL: clients[attempt%2].BaseURL, HTTP: &http.Client{Timeout: 5 * time.Second}} - resp, _, err := anonymous.Post("/auth/login", map[string]string{"username": "ha-unknown", "password": "invalid"}) + status, _, err := tlDo(t.Context(), anonymous, http.MethodPost, "/auth/login", map[string]string{"username": "ha-unknown", "password": "invalid"}) want := http.StatusUnauthorized if attempt == 6 { want = http.StatusTooManyRequests } - if err != nil || resp.StatusCode != want { - t.Fatalf("shared username login budget, attempt %d: %v %v (want %d)", attempt+1, err, resp, want) + if err != nil || status != want { + t.Fatalf("shared username login budget, attempt %d: %v %v (want %d)", attempt+1, err, status, want) } } } @@ -386,11 +386,11 @@ func haServiceClient(t *testing.T, nodeName string, admin *APIClient) *APIClient } func haStatus(ctx context.Context, client *APIClient, path string, want int) (bool, string) { - resp, body, err := client.DoContext(ctx, http.MethodGet, path, nil) + status, body, err := tlDo(ctx, client, http.MethodGet, path, nil) if err != nil { return false, err.Error() } - return resp.StatusCode == want, fmt.Sprintf("%s returned %d: %s", path, resp.StatusCode, body) + return status == want, fmt.Sprintf("%s returned %d: %s", path, status, body) } func haObserveService(ctx context.Context, stable *APIClient) func() (int, error) { @@ -451,9 +451,9 @@ func haDatabaseOutage(t *testing.T, clients []*APIClient, id string) { return haStatus(t.Context(), client, "/readyz/postgres-ha", http.StatusServiceUnavailable) }) assertAPIStatus(t, client, "/healthz", http.StatusOK) - resp, _, err := client.DoContext(t.Context(), http.MethodPatch, "/users/"+id, map[string]string{"displayName": "database-unavailable"}) - if err != nil || resp.StatusCode != http.StatusServiceUnavailable { - t.Fatalf("database outage must reject authenticated writes: %v %v", err, resp) + status, _, err := tlDo(t.Context(), client, http.MethodPatch, "/users/"+id, map[string]string{"displayName": "database-unavailable"}) + if err != nil || status != http.StatusServiceUnavailable { + t.Fatalf("database outage must reject authenticated writes: %v %v", err, status) } } if err := setScale(t.Context(), 1); err != nil { diff --git a/test/e2e/telemetry_e2e_helpers_test.go b/test/e2e/telemetry_e2e_helpers_test.go index cb488897a..1a411a351 100644 --- a/test/e2e/telemetry_e2e_helpers_test.go +++ b/test/e2e/telemetry_e2e_helpers_test.go @@ -245,8 +245,8 @@ func tlDo(ctx context.Context, c *APIClient, method, path string, body any) (int return 0, nil, fmt.Errorf("do %s %s: %w", method, path, err) } defer func() { _ = resp.Body.Close() }() - rb, _ := io.ReadAll(resp.Body) - return resp.StatusCode, rb, nil + rb, err := io.ReadAll(resp.Body) + return resp.StatusCode, rb, err } // helmUpgradeReuse runs `helm upgrade --reuse-values` with the given --set From a3a59e6b8a16d88bfed5622846156df421fbc7d4 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 04:07:20 -0500 Subject: [PATCH 19/23] fix(auth): mask unavailable provider policy through the generic error path Signed-off-by: Baylem --- api/internal/auth/local.go | 2 +- api/internal/auth/oidc.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/api/internal/auth/local.go b/api/internal/auth/local.go index 41d841363..199b2c1d3 100644 --- a/api/internal/auth/local.go +++ b/api/internal/auth/local.go @@ -113,7 +113,7 @@ func (l *Local) HandleLogin(sessions *SessionStore, reg *Registry) http.HandlerF if reg != nil { enabled, err := reg.localEnabled(req.Context()) if err != nil { - httperr.WriteMessage(w, req, messages.New("server.generic.503", nil)) + httperr.WriteCode(w, req, http.StatusServiceUnavailable, err) return } if !enabled { diff --git a/api/internal/auth/oidc.go b/api/internal/auth/oidc.go index b1ca522c3..47de2eb02 100644 --- a/api/internal/auth/oidc.go +++ b/api/internal/auth/oidc.go @@ -14,7 +14,6 @@ import ( "github.com/GameplanePanel/gameplane/api/internal/db" "github.com/GameplanePanel/gameplane/api/internal/httperr" - "github.com/GameplanePanel/gameplane/api/internal/messages" "github.com/GameplanePanel/gameplane/api/internal/scope" "github.com/GameplanePanel/gameplane/netguard" ) @@ -457,7 +456,7 @@ func (o *OIDC) HandleCallbackAt(sessions *SessionStore, cookiePath string) http. if o.providerName == HelmProviderName && o.getHelmRoleOverrides != nil { helmOverride, err := o.getHelmRoleOverrides(req.Context()) if err != nil { - httperr.WriteMessage(w, req, messages.New("server.generic.503", nil)) + httperr.WriteCode(w, req, http.StatusServiceUnavailable, err) return } if helmOverride != nil { From f4fed0f683a9d3aa3ddd08b99aefaa1b6d81ee7e Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 08:16:04 -0500 Subject: [PATCH 20/23] fix(notify): preserve standalone dispatch identity across remote polls Signed-off-by: Baylem --- api/internal/db/notify_postgres_test.go | 113 ++++++++++++++++++++ api/internal/notify/poll_standalone.go | 11 ++ api/internal/notify/poll_standalone_test.go | 3 +- 3 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 api/internal/db/notify_postgres_test.go diff --git a/api/internal/db/notify_postgres_test.go b/api/internal/db/notify_postgres_test.go new file mode 100644 index 000000000..596b3790f --- /dev/null +++ b/api/internal/db/notify_postgres_test.go @@ -0,0 +1,113 @@ +//go:build postgres + +package db_test + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" + + "github.com/GameplanePanel/gameplane/api/internal/controlplane" + "github.com/GameplanePanel/gameplane/api/internal/db" + "github.com/GameplanePanel/gameplane/api/internal/kube" + "github.com/GameplanePanel/gameplane/api/internal/notify" + "github.com/GameplanePanel/gameplane/api/internal/scope" +) + +func TestPostgresStandaloneNotificationPollDeliversDistinctVersions(t *testing.T) { + ctx := t.Context() + store := db.NewPostgresTestStore(t) + control, err := controlplane.New(ctx, store, filepath.Join(t.TempDir(), "panel.key")) + if err != nil { + t.Fatal(err) + } + delivered := make(chan notify.Event, 8) + sink := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + var event notify.Event + if err := json.NewDecoder(req.Body).Decode(&event); err != nil { + t.Errorf("decode webhook: %v", err) + w.WriteHeader(http.StatusBadRequest) + return + } + delivered <- event + w.WriteHeader(http.StatusNoContent) + })) + defer sink.Close() + _, err = control.Secrets("panel").Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "ops-hook", Namespace: "panel", Labels: map[string]string{notify.SinkSecretLabel: "true"}}, + Data: map[string][]byte{"url": []byte(sink.URL)}, + }, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + cfg := `{"sinks":[{"name":"ops","kind":"webhook","enabled":true,"configRef":"ops-hook","events":["backup.failed"]}]}` + if _, err := store.DB.ExecContext(ctx, "INSERT INTO config(key,value) VALUES ('notifications',?) ON CONFLICT(key) DO UPDATE SET value=excluded.value", cfg); err != nil { + t.Fatal(err) + } + + // List through the real Kubernetes wire decoder and the standalone poller. + // A repeated snapshot of the first failure must keep its dispatch identity. + // Two opaque versions with the same long prefix must still be distinguishable. + prefix := strings.Repeat("opaque/", 40) + snapshots := []struct{ phase, version, message string }{ + {"Running", "initial", ""}, {"Failed", prefix + "a", "first"}, + {"Running", "initial", ""}, {"Failed", prefix + "a", "first"}, + {"Running", "retried", ""}, {"Failed", prefix + "b", "second"}, + } + var polls atomic.Int32 + api := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.Header().Set("Content-Type", "application/json") + if req.URL.Path != "/apis/gameplane.local/v1alpha1/namespaces/"+scope.DefaultNamespace+"/backups" { + fmt.Fprint(w, `{"apiVersion":"v1","kind":"List","items":[]}`) + return + } + s := snapshots[min(int(polls.Add(1))-1, len(snapshots)-1)] + fmt.Fprintf(w, `{"apiVersion":"gameplane.local/v1alpha1","kind":"BackupList","items":[{ + "apiVersion":"gameplane.local/v1alpha1","kind":"Backup", + "metadata":{"namespace":%q,"name":"world","uid":"backup-uid","resourceVersion":%q}, + "status":{"phase":%q,"message":%q}}]}`, scope.DefaultNamespace, s.version, s.phase, s.message) + })) + defer api.Close() + remote, err := kube.New(&rest.Config{Host: api.URL}) + if err != nil { + t.Fatal(err) + } + reg := kube.NewRegistry("edge") + reg.Set("edge", remote) + workerCtx, cancel := context.WithTimeout(ctx, time.Minute) + done := make(chan struct{}) + go func() { + defer close(done) + notify.New(store, control, "panel").RunWithRegistry(workerCtx, reg) + }() + defer func() { cancel(); <-done }() + for _, want := range []string{"first", "second"} { + select { + case event := <-delivered: + if event.Cluster != "edge" || event.Type != notify.EventBackupFailed || event.Message != want { + t.Fatalf("wanted %s failure once, got %+v", want, event) + } + case <-workerCtx.Done(): + t.Fatalf("missing %s webhook after %d remote lists", want, polls.Load()) + } + } + var claims, longest int + if err := store.DB.QueryRowContext(ctx, `SELECT COUNT(*), MAX(octet_length(object_version)) + FROM notification_claims WHERE cluster='edge' AND object_uid='backup-uid' AND sink_name='ops'`).Scan(&claims, &longest); err != nil { + t.Fatal(err) + } + if claims != 2 || longest >= len(prefix) { + t.Fatalf("got %d claims with longest key %d bytes", claims, longest) + } +} diff --git a/api/internal/notify/poll_standalone.go b/api/internal/notify/poll_standalone.go index bc1c3a54f..13183a14f 100644 --- a/api/internal/notify/poll_standalone.go +++ b/api/internal/notify/poll_standalone.go @@ -2,7 +2,9 @@ package notify import ( "context" + "crypto/sha256" "errors" + "fmt" "log/slog" "strings" "time" @@ -141,6 +143,15 @@ func notificationProjection(obj *unstructured.Unstructured) *unstructured.Unstru out.SetName(boundedNotificationText(obj.GetName(), 253)) out.SetNamespace(boundedNotificationText(obj.GetNamespace(), 63)) out.SetUID(obj.GetUID()) + // Resource versions are opaque dispatch identities. Keep short versions + // verbatim; hashing longer ones bounds the snapshot without conflating + // distinct updates that share a prefix. The tagged digest is longer than + // any verbatim key, so the two representations cannot collide. + version := obj.GetResourceVersion() + if len(version) > sha256.Size*2 { + version = fmt.Sprintf("sha256:%x", sha256.Sum256([]byte(version))) + } + out.SetResourceVersion(version) phase, _, _ := unstructured.NestedString(obj.Object, "status", "phase") message, _, _ := unstructured.NestedString(obj.Object, "status", "message") conditions := []any{} diff --git a/api/internal/notify/poll_standalone_test.go b/api/internal/notify/poll_standalone_test.go index 6f3d5272e..b8c146d02 100644 --- a/api/internal/notify/poll_standalone_test.go +++ b/api/internal/notify/poll_standalone_test.go @@ -77,13 +77,14 @@ func TestStandaloneNotificationPollPreservesTransitions(t *testing.T) { } failed := gsObj(scope.DefaultNamespace, "game", "Failed", "False", "Failed", "ImagePullFailed", "cannot pull") failed.SetUID("original") + failed.SetResourceVersion("rv:edge/37") if _, err := k.Dynamic.Resource(kube.GVRs["servers"]).Namespace(scope.DefaultNamespace).Update(t.Context(), failed, metav1.UpdateOptions{}); err != nil { t.Fatal(err) } n.pollClusterSnapshot(t.Context(), k, "remote", state) select { case e := <-n.ch: - if e.Cluster != "remote" || e.Type != EventServerUnhealthy || e.Reason != "ImagePullFailed" { + if e.Cluster != "remote" || e.Type != EventServerUnhealthy || e.Reason != "ImagePullFailed" || e.objectVersion != "rv:edge/37" { t.Fatalf("wrong event: %+v", e) } default: From c0a5a077143e3953799057329c7980d1db0c8d38 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 14:35:01 -0500 Subject: [PATCH 21/23] fix(api): refresh snapshots after PostgreSQL advisory locks Signed-off-by: Baylem --- api/cmd/runtime_postgres_test.go | 75 +++++++++++++++++++++++++++++ api/internal/db/db.go | 9 +++- api/internal/db/db_postgres_test.go | 6 +++ 3 files changed, 89 insertions(+), 1 deletion(-) diff --git a/api/cmd/runtime_postgres_test.go b/api/cmd/runtime_postgres_test.go index 66fad57af..298351dac 100644 --- a/api/cmd/runtime_postgres_test.go +++ b/api/cmd/runtime_postgres_test.go @@ -24,6 +24,7 @@ import ( "github.com/GameplanePanel/gameplane/api/internal/auth" "github.com/GameplanePanel/gameplane/api/internal/db" + "github.com/GameplanePanel/gameplane/api/internal/handlers" ) // Each instance has its own pool and auth middleware, like two API processes. @@ -71,6 +72,80 @@ func runtimeReplicas(t *testing.T) [2]*db.Store { return stores } +func TestPostgresWaitingDemotionCannotRemoveLastManager(t *testing.T) { + stores := runtimeReplicas(t) + ctx, cancel := context.WithTimeout(t.Context(), 15*time.Second) + defer cancel() + for _, store := range stores { + // Keep the session default across calls, as with a role's isolation setting. + store.DB.SetMaxOpenConns(1) + if _, err := store.DB.ExecContext(ctx, "SET default_transaction_isolation = 'repeatable read'"); err != nil { + t.Fatal(err) + } + } + if _, err := stores[0].DB.ExecContext(ctx, `INSERT INTO users(id, username, display_name, email, role) VALUES + (101, 'manager-a', '', '', 'admin'), (102, 'manager-b', '', '', 'admin')`); err != nil { + t.Fatal(err) + } + var waiterPID int + if err := stores[1].DB.QueryRowContext(ctx, "SELECT pg_backend_pid()").Scan(&waiterPID); err != nil { + t.Fatal(err) + } + first, err := stores[0].BeginUserManagement(ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = first.Rollback() }() + if _, err := first.ExecContext(ctx, "UPDATE users SET role='viewer' WHERE id=101"); err != nil { + t.Fatal(err) + } + + // This request was authenticated before the other replica's demotion commits. + router := chi.NewRouter() + handlers.MountUsers(router, stores[1], auth.NewSessionStore(stores[1]), nil) + caller := auth.WithUser(ctx, &auth.User{ID: 101, Role: "admin"}) + req := httptest.NewRequestWithContext(caller, http.MethodPatch, "/users/102", strings.NewReader(`{"role":"viewer"}`)) + req.Header.Set("Accept", "application/problem+json") + resp := httptest.NewRecorder() + finished := make(chan struct{}) + go func() { + defer close(finished) + router.ServeHTTP(resp, req) + }() + defer func() { cancel(); <-finished }() + + // Observe the exact two-key advisory lock before releasing the first writer. + for { + var waiting bool + err := first.QueryRowContext(ctx, `SELECT EXISTS (SELECT FROM pg_locks WHERE pid = ? + AND locktype = 'advisory' AND NOT granted AND objsubid = 2 + AND classid = hashtext(current_schema()::text)::oid + AND objid = hashtext('gameplane.user-management')::oid)`, waiterPID).Scan(&waiting) + if err != nil { + t.Fatal(err) + } + if waiting { + break + } + select { + case <-finished: + t.Fatalf("peer returned before waiting on the lock: HTTP %d", resp.Code) + case <-time.After(10 * time.Millisecond): + } + } + if err := first.Commit(); err != nil { + t.Fatal(err) + } + <-finished + count, err := stores[0].UserManagerCount(ctx) + if err != nil { + t.Fatal(err) + } + if resp.Code != http.StatusBadRequest || !strings.Contains(resp.Body.String(), "server.users.cannot_demote_last_manager") || count != 1 { + t.Fatalf("waiting demotion: HTTP %d, body %s, remaining managers %d", resp.Code, resp.Body.String(), count) + } +} + func TestPostgresReadOnlyFence(t *testing.T) { ctx := t.Context() fixture := runtimeReplicas(t)[0] diff --git a/api/internal/db/db.go b/api/internal/db/db.go index a8e34d7a7..f0bf20acb 100644 --- a/api/internal/db/db.go +++ b/api/internal/db/db.go @@ -86,7 +86,14 @@ func (s *Store) BeginUserManagement(ctx context.Context) (*sql.Tx, error) { } func (s *Store) beginSerialized(ctx context.Context, purpose string) (*sql.Tx, error) { - tx, err := s.DB.BeginTx(ctx, nil) + var opts *sql.TxOptions + if s.Driver == "postgres" { + // The advisory-lock SELECT can wait behind a committing replica. The + // next statement must see that commit even if the role defaults to + // REPEATABLE READ, which fixes its snapshot before the wait. + opts = &sql.TxOptions{Isolation: sql.LevelReadCommitted} + } + tx, err := s.DB.BeginTx(ctx, opts) if err != nil { return nil, err } diff --git a/api/internal/db/db_postgres_test.go b/api/internal/db/db_postgres_test.go index 6d73407e4..187c67ddb 100644 --- a/api/internal/db/db_postgres_test.go +++ b/api/internal/db/db_postgres_test.go @@ -579,6 +579,12 @@ func TestPostgres_ConcurrentMigrationAndManagement(t *testing.T) { ctx := t.Context() first := openPostgresSchema(t) second := postgresPeer(t, first) + for _, s := range []*Store{first, second} { + s.DB.SetMaxOpenConns(1) + if _, err := s.DB.ExecContext(ctx, "SET default_transaction_isolation = 'repeatable read'"); err != nil { + t.Fatal(err) + } + } start := make(chan struct{}) results := make(chan error, 2) for _, s := range []*Store{first, second} { From aa89dadf5ac571c9dfda8b8c799a304d2c2350be Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 14:35:11 -0500 Subject: [PATCH 22/23] fix(rbac): revoke live server streams when ownership access changes Signed-off-by: Baylem --- api/cmd/main.go | 6 +- api/cmd/runtime_postgres_test.go | 3 +- api/internal/auth/sessions.go | 10 ++- api/internal/rbac/rbac.go | 26 ++---- api/internal/rbac/server_identity.go | 49 +++++++++- api/internal/rbac/server_identity_test.go | 12 ++- api/internal/rbac/stream_grant_test.go | 105 ++++++++++++++++++++++ docs/panel-recovery.md | 13 ++- test/e2e/panel_recovery_e2e_test.go | 2 +- test/e2e/postgres_ha_e2e_test.go | 47 ++++++++++ 10 files changed, 236 insertions(+), 37 deletions(-) create mode 100644 api/internal/rbac/stream_grant_test.go diff --git a/api/cmd/main.go b/api/cmd/main.go index 4160a5c55..cc4eac51d 100644 --- a/api/cmd/main.go +++ b/api/cmd/main.go @@ -384,14 +384,14 @@ func main() { // Protected API r.Group(func(p chi.Router) { p.Use(sessions.Authenticate) - if store.Driver == "postgres" { - p.Use(sessions.RevalidateStreams(isStreamingRequest, 5*time.Second)) - } // A broad per-IP cap on writes keeps one authenticated client from // pegging the DB or k8s API. Reads go through unlimited (the main // bottleneck there is the dashboard's own polling). p.Use(mutationRateLimit) p.Use(rbac.Middleware(reg)) + if store.Driver == "postgres" { + p.Use(sessions.RevalidateStreams(isStreamingRequest, 5*time.Second, rbac.StreamGrantValidator(reg))) + } handlers.MountResources(p, reg, store) handlers.MountFleet(p, reg, store) diff --git a/api/cmd/runtime_postgres_test.go b/api/cmd/runtime_postgres_test.go index 298351dac..ccd40b816 100644 --- a/api/cmd/runtime_postgres_test.go +++ b/api/cmd/runtime_postgres_test.go @@ -25,6 +25,7 @@ import ( "github.com/GameplanePanel/gameplane/api/internal/auth" "github.com/GameplanePanel/gameplane/api/internal/db" "github.com/GameplanePanel/gameplane/api/internal/handlers" + "github.com/GameplanePanel/gameplane/api/internal/rbac" ) // Each instance has its own pool and auth middleware, like two API processes. @@ -236,7 +237,7 @@ func TestPostgresRuntimeAcrossReplicas(t *testing.T) { mux.Handle("POST /login", auth.LoginLimiter.Middleware(login)) mux.Handle("POST /logout", sessions.Authenticate(sessions.HandleLogout())) mux.Handle("GET /me", sessions.Authenticate(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(200) }))) - revalidate := sessions.RevalidateStreams(func(*http.Request) bool { return true }, 20*time.Millisecond) + revalidate := sessions.RevalidateStreams(func(*http.Request) bool { return true }, 20*time.Millisecond, rbac.StreamGrantValidator(nil)) stream := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { close(started) <-r.Context().Done() diff --git a/api/internal/auth/sessions.go b/api/internal/auth/sessions.go index 08e7e32f3..c8983007a 100644 --- a/api/internal/auth/sessions.go +++ b/api/internal/auth/sessions.go @@ -196,9 +196,9 @@ func (s *SessionStore) Authenticate(next http.Handler) http.Handler { }) } -// RevalidateStreams bounds how long an established console or stream can -// outlive its session or permissions. Run it immediately after Authenticate. -func (s *SessionStore) RevalidateStreams(isStream func(*http.Request) bool, interval time.Duration) func(http.Handler) http.Handler { +// RevalidateStreams bounds how long a stream can outlive its session, SQL +// permissions, or a resource grant bound by the preceding RBAC middleware. +func (s *SessionStore) RevalidateStreams(isStream func(*http.Request) bool, interval time.Duration, grantValid func(context.Context, int64) bool) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { if !isStream(req) { @@ -223,8 +223,10 @@ func (s *SessionStore) RevalidateStreams(isStream func(*http.Request) bool, inte case <-ticker.C: check, stop := context.WithTimeout(ctx, interval) active, _, err := s.lookup(check, cookie.Value) + valid := err == nil && active.ID == initial.ID && active.Role == initial.Role && + reflect.DeepEqual(active.Perms, initial.Perms) && grantValid(check, active.ID) stop() - if err != nil || active.ID != initial.ID || active.Role != initial.Role || !reflect.DeepEqual(active.Perms, initial.Perms) { + if !valid { cancel() return } diff --git a/api/internal/rbac/rbac.go b/api/internal/rbac/rbac.go index 4025f35aa..6f7ea3c94 100644 --- a/api/internal/rbac/rbac.go +++ b/api/internal/rbac/rbac.go @@ -133,18 +133,9 @@ func Middleware(fetch ServerFetcher) func(http.Handler) http.Handler { httperr.WriteMessage(w, req, messages.New("server.forbidden", nil)) return } - obj, err := fetch.GetServer(req.Context(), cl, ns, name) - if err == nil && obj != nil { - role := ownershipRole(obj, u.ID) - if role != roleNone { - // Grant to owner, or to collaborator if not owner-only. - if role == roleOwner || !ownerOnlyOperation(req.Method, verb, isExact) { - identity := ServerIdentity{Cluster: cl, Namespace: ns, Name: name, UID: string(obj.GetUID())} - req = req.WithContext(context.WithValue(req.Context(), serverIdentityKey{}, identity)) - next.ServeHTTP(w, req) - return - } - } + if ctx, granted := bindServerGrant(req.Context(), fetch, cl, ns, name, u.ID, ownerOnlyOperation(req.Method, verb, isExact)); granted { + next.ServeHTTP(w, req.WithContext(ctx)) + return } } } @@ -156,19 +147,14 @@ func Middleware(fetch ServerFetcher) func(http.Handler) http.Handler { // the caller holds the route's namespace permission. if Namespaced(r.perm) && !u.Can("*", true, cl, ns) { if name, verb, isExact, ok := parseServerPath(req.URL.Path); ok && ownerOnlyOperation(req.Method, verb, isExact) { - if fetch == nil { - httperr.WriteMessage(w, req, messages.New("server.forbidden", nil)) - return - } - obj, err := fetch.GetServer(req.Context(), cl, ns, name) - if err != nil || obj == nil || ownershipRole(obj, u.ID) != roleOwner { + ctx, granted := bindServerGrant(req.Context(), fetch, cl, ns, name, u.ID, true) + if !granted { httperr.WriteMessage(w, req, messages.New("server.forbidden", nil)) return } // This grant still depends on owning this exact object, even // though the namespace permission check already passed. - identity := ServerIdentity{Cluster: cl, Namespace: ns, Name: name, UID: string(obj.GetUID())} - req = req.WithContext(context.WithValue(req.Context(), serverIdentityKey{}, identity)) + req = req.WithContext(ctx) } } next.ServeHTTP(w, req) diff --git a/api/internal/rbac/server_identity.go b/api/internal/rbac/server_identity.go index d266babb6..0a6b4f413 100644 --- a/api/internal/rbac/server_identity.go +++ b/api/internal/rbac/server_identity.go @@ -3,6 +3,8 @@ package rbac import ( "context" "errors" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) // ServerIdentity is the exact resource that granted owner/collaborator access. @@ -15,6 +17,49 @@ type ServerIdentity struct { type serverIdentityKey struct{} +type serverGrant struct { + ServerIdentity + ownerOnly bool +} + +func (g serverGrant) permits(obj *unstructured.Unstructured, userID int64) bool { + if obj == nil { + return false + } + role := ownershipRole(obj, userID) + return role == roleOwner || (!g.ownerOnly && role == roleCollaborator) +} + +func bindServerGrant(ctx context.Context, fetch ServerFetcher, cluster, ns, name string, userID int64, ownerOnly bool) (context.Context, bool) { + if fetch == nil { + return ctx, false + } + obj, err := fetch.GetServer(ctx, cluster, ns, name) + grant := serverGrant{ownerOnly: ownerOnly} + if err != nil || !grant.permits(obj, userID) { + return ctx, false + } + grant.ServerIdentity = ServerIdentity{Cluster: cluster, Namespace: ns, Name: name, UID: string(obj.GetUID())} + return context.WithValue(ctx, serverIdentityKey{}, grant), true +} + +// StreamGrantValidator rechecks the exact resource grant used to open a stream. +// A namespace-wide SQL permission needs no additional Kubernetes lookup. +func StreamGrantValidator(fetch ServerFetcher) func(context.Context, int64) bool { + return func(ctx context.Context, userID int64) bool { + grant, bound := ctx.Value(serverIdentityKey{}).(serverGrant) + if !bound { + return true + } + if fetch == nil { + return false + } + obj, err := fetch.GetServer(ctx, grant.Cluster, grant.Namespace, grant.Name) + return err == nil && grant.permits(obj, userID) && + ValidateServerIdentity(ctx, grant.Cluster, obj.GetNamespace(), obj.GetName(), string(obj.GetUID())) == nil + } +} + // ErrServerIdentityChanged rejects a resource different from the one authorized. var ErrServerIdentityChanged = errors.New("authorized server identity changed") @@ -22,8 +67,8 @@ var ErrServerIdentityChanged = errors.New("authorized server identity changed") // Namespace-wide permissions remain unbound unless an owner-only operation also // requires ownership of the individual server. func BoundServerIdentity(ctx context.Context) (ServerIdentity, bool) { - identity, ok := ctx.Value(serverIdentityKey{}).(ServerIdentity) - return identity, ok + grant, ok := ctx.Value(serverIdentityKey{}).(serverGrant) + return grant.ServerIdentity, ok } // ValidateServerIdentity prevents a later lookup from silently rebinding an diff --git a/api/internal/rbac/server_identity_test.go b/api/internal/rbac/server_identity_test.go index 834dbc613..d920a9fd7 100644 --- a/api/internal/rbac/server_identity_test.go +++ b/api/internal/rbac/server_identity_test.go @@ -52,7 +52,7 @@ func TestOwnershipGrantBindsExactServerIdentity(t *testing.T) { func TestIdentityBindingChecksEveryTargetDimension(t *testing.T) { identity := ServerIdentity{Cluster: "remote", Namespace: "games", Name: "same-name", UID: "uid"} - ctx := context.WithValue(t.Context(), serverIdentityKey{}, identity) + ctx := context.WithValue(t.Context(), serverIdentityKey{}, serverGrant{ServerIdentity: identity}) for _, observed := range []ServerIdentity{ {Cluster: "local", Namespace: "games", Name: "same-name", UID: "uid"}, {Cluster: "remote", Namespace: "other", Name: "same-name", UID: "uid"}, @@ -66,7 +66,7 @@ func TestIdentityBindingChecksEveryTargetDimension(t *testing.T) { if err := ValidateServerIdentity(t.Context(), "remote", "games", "same-name", "replacement"); err != nil { t.Fatalf("unbound namespace grant changed: %v", err) } - emptyUID := context.WithValue(t.Context(), serverIdentityKey{}, ServerIdentity{Cluster: "remote", Namespace: "games", Name: "same-name"}) + emptyUID := context.WithValue(t.Context(), serverIdentityKey{}, serverGrant{ServerIdentity: ServerIdentity{Cluster: "remote", Namespace: "games", Name: "same-name"}}) if err := ValidateServerIdentity(emptyUID, "remote", "games", "same-name", ""); err == nil { t.Fatal("empty resource identity accepted") } @@ -78,6 +78,9 @@ func TestNamespacePermissionDoesNotBecomeOwnershipBound(t *testing.T) { if _, bound := BoundServerIdentity(req.Context()); bound { t.Fatal("namespace permission unexpectedly bound to server") } + if !StreamGrantValidator(nil)(req.Context(), user.ID) { + t.Fatal("namespace permission required a Kubernetes lookup") + } w.WriteHeader(http.StatusNoContent) })) req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/servers/same-name/files/write?cluster=remote", nil) @@ -101,6 +104,11 @@ func TestOwnerOnlyNamespacePermissionStillBindsOwnershipIdentity(t *testing.T) { if err := ValidateServerIdentity(req.Context(), "remote", scope.DefaultNamespace, "same-name", "replacement-uid"); !errors.Is(err, ErrServerIdentityChanged) { t.Errorf("owner-only namespace grant rebound to replacement: %v", err) } + transferred := server.DeepCopy() + transferred.SetAnnotations(map[string]string{"gameplane.local/collaborators": "42"}) + if StreamGrantValidator(identityFetcher{transferred})(req.Context(), user.ID) { + t.Error("former owner kept an owner-only grant as a collaborator") + } w.WriteHeader(http.StatusNoContent) })) method := http.MethodDelete diff --git a/api/internal/rbac/stream_grant_test.go b/api/internal/rbac/stream_grant_test.go new file mode 100644 index 000000000..b8b515ea1 --- /dev/null +++ b/api/internal/rbac/stream_grant_test.go @@ -0,0 +1,105 @@ +package rbac + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/coder/websocket" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/GameplanePanel/gameplane/api/internal/auth" + "github.com/GameplanePanel/gameplane/api/internal/scope" +) + +type streamServerFetcher struct { + current atomic.Pointer[unstructured.Unstructured] + reads atomic.Int32 + checked chan struct{} +} + +func (f *streamServerFetcher) GetServer(_ context.Context, cluster, ns, name string) (*unstructured.Unstructured, error) { + if obj := f.current.Load(); obj != nil && cluster == "remote" && ns == scope.DefaultNamespace && name == obj.GetName() { + if f.reads.Add(1) == 2 { // admission, then the first live recheck + close(f.checked) + } + return obj, nil + } + return nil, errors.New("server unavailable") +} + +func (*streamServerFetcher) IDs() []string { return []string{scope.DefaultCluster, "remote"} } + +func TestStreamLosesServerGrant(t *testing.T) { + db := migratedStore(t) + if _, err := db.DB.ExecContext(t.Context(), `INSERT INTO users(id,username,display_name,email) VALUES (42,'player','','')`); err != nil { + t.Fatal(err) + } + sessions := auth.NewSessionStore(db) + token, _, err := sessions.Create(t.Context(), 42) + if err != nil { + t.Fatal(err) + } + obj := &unstructured.Unstructured{} + obj.SetName("game") + obj.SetNamespace(scope.DefaultNamespace) + obj.SetUID("original") + obj.SetAnnotations(map[string]string{"gameplane.local/collaborators": "42"}) + for _, change := range []string{"removed", "replaced", "unavailable"} { + t.Run(change, func(t *testing.T) { + fetch := &streamServerFetcher{checked: make(chan struct{})} + fetch.current.Store(obj) + stream := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + conn, err := websocket.Accept(w, r, nil) + if err != nil { + return + } + defer conn.CloseNow() + <-conn.CloseRead(r.Context()).Done() + }) + revalidate := sessions.RevalidateStreams(func(*http.Request) bool { return true }, 100*time.Millisecond, StreamGrantValidator(fetch)) + server := httptest.NewServer(sessions.Authenticate(Middleware(fetch)(revalidate(stream)))) + defer server.Close() + header := http.Header{} + header.Set("Cookie", (&http.Cookie{Name: "gameplane_session", Value: token}).String()) + conn, _, err := websocket.Dial(t.Context(), strings.Replace(server.URL, "http://", "ws://", 1)+"/ws/servers/game/console?cluster=remote", &websocket.DialOptions{HTTPHeader: header}) + if err != nil { + t.Fatal(err) + } + defer conn.CloseNow() + active := conn.CloseRead(t.Context()) + select { + case <-active.Done(): + t.Fatal("socket closed while the collaborator was authorized") + case <-fetch.checked: + case <-time.After(2 * time.Second): + t.Fatal("established console did not recheck its live server grant") + } + ping, stop := context.WithTimeout(t.Context(), time.Second) + defer stop() + if err := conn.Ping(ping); err != nil { + t.Fatalf("socket unusable while the grant was valid: %v", err) + } + next := obj.DeepCopy() + switch change { + case "removed": + next.SetAnnotations(nil) + case "replaced": + next.SetUID("replacement") + case "unavailable": + next = nil + } + fetch.current.Store(next) + select { + case <-active.Done(): + case <-time.After(2 * time.Second): + t.Fatal("established console remained open after losing its live server grant") + } + }) + } +} diff --git a/docs/panel-recovery.md b/docs/panel-recovery.md index aaedd5973..9261b292b 100644 --- a/docs/panel-recovery.md +++ b/docs/panel-recovery.md @@ -193,10 +193,15 @@ binary rejects this flag before opening the database. To downgrade to a version without this protocol, return to one writer before changing images. A session created on either API works on both, and logout and credential changes affect the next request on either replica. Long-lived consoles, event streams -and transfers recheck their session and permissions every five seconds. An -unavailable database cancels the stream after a further five-second query -deadline. Provider edits, Secret rotation and redirect changes are read on the -next dashboard-managed OIDC handshake. +and transfers recheck their session and permissions every five seconds. Each +stream authorized through GameServer ownership or collaboration adds one +Kubernetes GET per check on the selected cluster to verify the original UID and +current grant. Namespace-wide permissions require only the database check. +A revoked grant or failed lookup cancels the stream on that check. The database +and Kubernetes reads share a five-second deadline, so an outage just after a +successful check can take up to ten seconds to cancel the stream. Provider edits, +Secret rotation and redirect changes are read on the next dashboard-managed OIDC +handshake. The API persists login-device signatures and rate budgets with the rest of the database. Preserve these tables in backups. To invalidate known-device cookies diff --git a/test/e2e/panel_recovery_e2e_test.go b/test/e2e/panel_recovery_e2e_test.go index bc471b2df..e461f4117 100644 --- a/test/e2e/panel_recovery_e2e_test.go +++ b/test/e2e/panel_recovery_e2e_test.go @@ -577,7 +577,7 @@ func recoveryGame(t *testing.T, suffix string) (string, string) { t.Helper() name := "recovery-game-" + suffix applyBusyboxTemplate(t, name) - patch, err := json.Marshal(map[string]any{"spec": map[string]any{"command": []string{"sh", "-c", "echo retained > /data/recovery-marker; n=0; while true; do n=$((n+1)); echo $n > /data/recovery-counter.next; mv /data/recovery-counter.next /data/recovery-counter; sleep 1; done"}}}) + patch, err := json.Marshal(map[string]any{"spec": map[string]any{"command": []string{"sh", "-c", "echo retained | tee /data/recovery-marker; n=0; while true; do n=$((n+1)); echo $n > /data/recovery-counter.next; mv /data/recovery-counter.next /data/recovery-counter; sleep 1; done"}}}) if err != nil { t.Fatal(err) } diff --git a/test/e2e/postgres_ha_e2e_test.go b/test/e2e/postgres_ha_e2e_test.go index 1e4424157..f67a2d7a4 100644 --- a/test/e2e/postgres_ha_e2e_test.go +++ b/test/e2e/postgres_ha_e2e_test.go @@ -147,6 +147,7 @@ func TestPanel_PostgresActiveActiveFailover(t *testing.T) { pair = haPodPair(t) clients = haDirectClients(t, pair, admin) game, _ := recoveryGame(t, "ha") + haRevokeServerStream(t, pair[0].Name, clients[1], game) identity, before := recoveryGameIdentity(t, game), recoveryCounter(t, game) haDatabaseOutage(t, clients, id) if current := recoveryGameIdentity(t, game); current != identity || recoveryCounter(t, game) <= before { @@ -172,6 +173,52 @@ func TestPanel_PostgresActiveActiveFailover(t *testing.T) { }) } +func haRevokeServerStream(t *testing.T, readerPod string, adminB *APIClient, game string) { + t.Helper() + fixture := &recoveryFixture{t: t, ns: haNamespace} + role := "ha-stream-" + strconv.FormatInt(time.Now().UnixNano(), 36) + fixture.request(adminB, http.MethodPost, "/roles", map[string]string{"name": role}, http.StatusCreated) + username, password, _ := envInstance.CreateUser(t, adminB, role, "ha-stream") + readerA := fixture.login(readerPod, username, password) + path := "/servers/" + game + const ns = "?namespace=gameplane-games" + assertAPIStatus(t, readerA, path+ns, http.StatusForbidden) + setCollaborators := func(names ...string) { + fixture.request(adminB, http.MethodPut, path+":collaborators"+ns, map[string]any{"usernames": names}, http.StatusNoContent) + } + setCollaborators(username) + assertAPIStatus(t, readerA, path+ns, http.StatusOK) + + // Prove the real Pod log handler on A has reached this game before B + // removes the Kubernetes grant. The session itself stays valid throughout. + conn, closeConn := dialAuthedWS(t, readerA, "/ws"+path+"/logs/pod"+ns+"&from=end") + defer closeConn() + before, cancel := context.WithTimeout(t.Context(), 20*time.Second) + defer cancel() + for { + _, msg, err := conn.Read(before) + if err != nil { + t.Fatalf("authorized collaborator could not read game logs: %v", err) + } + if strings.TrimSpace(string(msg)) == "retained" { + break + } + } + setCollaborators() + after, stop := context.WithTimeout(t.Context(), 15*time.Second) + defer stop() + for { + if _, _, err := conn.Read(after); err != nil { + if after.Err() != nil { + t.Fatal("revoked collaborator's established log stream stayed open") + } + break + } + } + assertAPIStatus(t, readerA, path+ns, http.StatusForbidden) + assertAPIStatus(t, readerA, "/users/me", http.StatusOK) +} + func haLegacyBinaryFence(t *testing.T) { t.Helper() const name = "ha-legacy-binary" From 48b02e8f7f834788faeac79fcfd5952f23477b8d Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 17:25:51 -0500 Subject: [PATCH 23/23] test(rbac): close WebSocket handshake responses in grant regression Signed-off-by: Baylem --- api/internal/rbac/stream_grant_test.go | 5 ++++- changelog/unreleased/911-postgres-ha.md | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/911-postgres-ha.md diff --git a/api/internal/rbac/stream_grant_test.go b/api/internal/rbac/stream_grant_test.go index b8b515ea1..290ef1e36 100644 --- a/api/internal/rbac/stream_grant_test.go +++ b/api/internal/rbac/stream_grant_test.go @@ -67,7 +67,10 @@ func TestStreamLosesServerGrant(t *testing.T) { defer server.Close() header := http.Header{} header.Set("Cookie", (&http.Cookie{Name: "gameplane_session", Value: token}).String()) - conn, _, err := websocket.Dial(t.Context(), strings.Replace(server.URL, "http://", "ws://", 1)+"/ws/servers/game/console?cluster=remote", &websocket.DialOptions{HTTPHeader: header}) + conn, resp, err := websocket.Dial(t.Context(), strings.Replace(server.URL, "http://", "ws://", 1)+"/ws/servers/game/console?cluster=remote", &websocket.DialOptions{HTTPHeader: header}) + if resp != nil && resp.Body != nil { + defer resp.Body.Close() + } if err != nil { t.Fatal(err) } diff --git a/changelog/unreleased/911-postgres-ha.md b/changelog/unreleased/911-postgres-ha.md new file mode 100644 index 000000000..f55d0e7d5 --- /dev/null +++ b/changelog/unreleased/911-postgres-ha.md @@ -0,0 +1,5 @@ +# PostgreSQL availability and recovery + +Kubernetes-native installations can run two or more API replicas on separate nodes against one writable PostgreSQL primary. To upgrade, first roll out the HA-capable API with one replica and a Secret-backed DSN. After the singleton is ready, enable `api.postgresHA.enabled` and set `api.replicas` to at least `2`. SQLite and standalone installations continue to use one API replica. [Helm upgrade instructions](../../docs/install.md#api-rollout-and-postgresql-availability) + +For point-in-time recovery, keep the physical base backup and archived WAL on storage separate from the database volume. The recovery drill fences the old writer, restores to a named WAL point, and reapplies authorization revocations before the API resumes serving requests. [Panel recovery procedure](../../docs/panel-recovery.md)