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/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 5b8466f08..c59fd397e 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -1783,7 +1783,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 @@ -2473,9 +2473,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: @@ -2483,7 +2482,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: @@ -2518,23 +2517,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 \ @@ -2542,6 +2543,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 \ @@ -2549,6 +2551,13 @@ 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: | + 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" \ + 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 @@ -2556,6 +2565,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 @@ -2969,6 +2995,10 @@ 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'); + 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/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/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) + } +} 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/api/cmd/main.go b/api/cmd/main.go index afae1ac68..cc4eac51d 100644 --- a/api/cmd/main.go +++ b/api/cmd/main.go @@ -13,6 +13,7 @@ import ( "os/signal" "strconv" "strings" + "sync" "syscall" "time" @@ -109,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 != "" { @@ -196,7 +201,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 +283,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 @@ -307,6 +310,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 @@ -378,6 +389,9 @@ func 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) @@ -445,18 +459,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, @@ -498,6 +524,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) } @@ -530,6 +561,7 @@ type config struct { addr string metricsAddr string dbDriver string + postgresHA bool dbDSN string logLevel string @@ -608,6 +640,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/runtime_postgres_test.go b/api/cmd/runtime_postgres_test.go new file mode 100644 index 000000000..ccd40b816 --- /dev/null +++ b/api/cmd/runtime_postgres_test.go @@ -0,0 +1,400 @@ +//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/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" + "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. +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 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] + 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() + 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, rbac.StreamGrantValidator(nil)) + 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) + // 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/cmd/workers.go b/api/cmd/workers.go new file mode 100644 index 000000000..3b33652d9 --- /dev/null +++ b/api/cmd/workers.go @@ -0,0 +1,89 @@ +package main + +import ( + "context" + "database/sql" + "errors" + "log/slog" + "time" + + "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 +// 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/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/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/local.go b/api/internal/auth/local.go index 20951e06d..199b2c1d3 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.WriteCode(w, req, http.StatusServiceUnavailable, err) + 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..47de2eb02 100644 --- a/api/internal/auth/oidc.go +++ b/api/internal/auth/oidc.go @@ -65,7 +65,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 +347,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 +454,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.WriteCode(w, req, http.StatusServiceUnavailable, err) + return + } + if helmOverride != nil { resolvePolicy = effectiveHelmPolicy(o.policy, helmOverride) } } @@ -526,8 +531,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 +545,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 +565,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 +633,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 +656,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 +664,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 +680,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 +687,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/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..3624f40d5 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 || !ok { - return defaultProviders(), sha256.Sum256(nil) + if err != nil && r.store.Driver == "postgres" { + return nil, [sha256.Size]byte{}, err + } + if !ok { + 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 || !ok { - return nil + if err != nil && r.store.Driver == "postgres" { + return nil, err + } + if !ok { + 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..c8983007a 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,32 @@ 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) + } + 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) + } } } - }() + } } // DeleteForUser drops every session row owned by the given user. Call @@ -184,6 +196,48 @@ func (s *SessionStore) Authenticate(next 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) { + 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) + valid := err == nil && active.ID == initial.ID && active.Role == initial.Role && + reflect.DeepEqual(active.Perms, initial.Perms) && grantValid(check, active.ID) + stop() + if !valid { + 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 +278,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/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..f0bf20acb 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,48 @@ 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) { + 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 + } + 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 +130,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 +141,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 +152,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 +242,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 +287,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 +301,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.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/api/internal/db/db_postgres_test.go b/api/internal/db/db_postgres_test.go index fddfaf672..187c67ddb 100644 --- a/api/internal/db/db_postgres_test.go +++ b/api/internal/db/db_postgres_test.go @@ -559,3 +559,79 @@ 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) + 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} { + 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/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/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/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/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/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/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 } 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..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" @@ -90,8 +92,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" { @@ -142,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: 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/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..290ef1e36 --- /dev/null +++ b/api/internal/rbac/stream_grant_test.go @@ -0,0 +1,108 @@ +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, 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) + } + 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/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 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) diff --git a/charts/gameplane/templates/api.yaml b/charts/gameplane/templates/api.yaml index 9cd5c188f..c95d50af6 100644 --- a/charts/gameplane/templates/api.yaml +++ b/charts/gameplane/templates/api.yaml @@ -6,6 +6,7 @@ {{- end }} {{- end }} {{- if (eq (include "gameplane.apiEnabled" .) "true") }} +{{- $ha := (.Values.api.postgresHA | default dict).enabled }} apiVersion: v1 kind: ServiceAccount metadata: @@ -202,13 +203,16 @@ 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 } - # 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. @@ -224,6 +228,7 @@ spec: # runs pre-upgrade. Both halves are load-bearing; the e2e upgrade # test covers them. rollingUpdate: null + {{- end }} template: metadata: labels: @@ -232,9 +237,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 @@ -253,6 +270,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 and (not .Values.api.standalone) (not $rbac.remoteClusters) }} - --remote-clusters=false {{- end }} @@ -492,9 +514,15 @@ 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: /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 875039e84..c1a4f2230 100644 --- a/charts/gameplane/values.yaml +++ b/charts/gameplane/values.yaml @@ -195,17 +195,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 @@ -233,8 +234,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/charts/tests/check_standalone.py b/charts/tests/check_standalone.py index 903462d1b..e14545056 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}, @@ -54,8 +54,10 @@ 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" - 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 +74,26 @@ 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}} + assert dep["minReadySeconds"] == 5 + pod = dep["template"]["spec"] + 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" + 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 check_native_permissions(): 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..1a2c897a4 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 @@ -34,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 @@ -204,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 - </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 @@ -304,7 +291,7 @@ EOF echo "creating gameplane-system namespace (pre-Helm, for the fake-OIDC fixture)" kubectl create namespace gameplane-system --dry-run=client -o yaml | kubectl apply -f - - if [ "${PROFILE}" = postgres ]; then + if [[ "${PROFILE}" == postgres || "${PROFILE}" == postgres-ha ]]; then echo "deploying PostgreSQL (postgres profile)" kubectl apply -f "${HERE}/postgres.yaml" kubectl rollout status --namespace gameplane-system \ @@ -400,9 +387,13 @@ EOF --set networkPolicies.probePorts[0].protocol=TCP --set networkPolicies.probePorts[0].port=25566) fi - if [ "${PROFILE}" = postgres ]; then + if [[ "${PROFILE}" == postgres || "${PROFILE}" == postgres-ha ]]; then DB_SETS=(--set api.db.driver=postgres --set api.db.existingSecret=gameplane-postgres) fi + if [[ "${PROFILE}" == postgres-ha ]]; then + kubectl label nodes "${CLUSTER}-worker" "${CLUSTER}-worker2" gameplane-ha=worker + DB_SETS+=(--set-string api.nodeSelector.gameplane-ha=worker) + fi echo "helm upgrade --install gameplane" # Bump the API container's memory limit above the chart default of diff --git a/deploy/kind/recover-postgres-wal.sh b/deploy/kind/recover-postgres-wal.sh new file mode 100644 index 000000000..7cf2c1f1f --- /dev/null +++ b/deploy/kind/recover-postgres-wal.sh @@ -0,0 +1,101 @@ +#!/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 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",persistentVolumeClaim:{claimName:$name}}]}}' | "${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; +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"}}}' +"${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/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')" diff --git a/docs/dependencies.md b/docs/dependencies.md index 0dfcf1e26..30905d75c 100644 --- a/docs/dependencies.md +++ b/docs/dependencies.md @@ -431,19 +431,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 d63613596..9e722ace8 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, production-tested) or `postgres` [experimental] (requires an api image built with `-tags postgres`; the published image is built without it). The CI `api (postgres)` job and the PostgreSQL E2E buckets, including `postgres-availability` (`TestUpgrade_PostgresAvailability`), exercise it. +- `api.db.driver` — `sqlite` (default, production-tested) or `postgres` (experimental). The API image includes both drivers. Native PostgreSQL deployments can opt into multiple API replicas after the HA-capable image completes its singleton rollout. The CI `api (postgres)` job and the `postgres-availability`, `postgres-pitr` and `postgres-ha` E2E suites exercise this configuration. - `api.db.dsn` — connection string; 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. @@ -1053,14 +1053,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 @@ -1075,10 +1075,38 @@ 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. 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/docs/panel-recovery.md b/docs/panel-recovery.md index d8e075f95..9261b292b 100644 --- a/docs/panel-recovery.md +++ b/docs/panel-recovery.md @@ -175,3 +175,87 @@ 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 + +### 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. 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 +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. 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 +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. 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 + +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. + +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. diff --git a/test/e2e/buckets.sh b/test/e2e/buckets.sh index 0a145dbe6..2acdc82bf 100755 --- a/test/e2e/buckets.sh +++ b/test/e2e/buckets.sh @@ -389,6 +389,9 @@ TestUpgrade_PostgresAvailability EOF } +bucket_postgres_pitr() { printf '%s\n' TestPanel_PostgresPointInTimeRecovery; } +bucket_postgres_ha() { printf '%s\n' TestPanel_PostgresActiveActiveFailover; } + # 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 @@ -462,7 +465,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 postgres-ha telemetry capture-overhead optional-components restricted-workloads wake-hostport network-isolation management-placement backup-preparation monitoring recovery } list_bucket() { @@ -479,6 +482,8 @@ list_bucket() { multicluster) bucket_multicluster ;; upgrade) bucket_upgrade ;; postgres-availability) bucket_postgres_availability ;; + postgres-pitr) bucket_postgres_pitr ;; + postgres-ha) bucket_postgres_ha ;; telemetry) bucket_telemetry ;; capture-overhead) bucket_capture_overhead ;; optional-components) bucket_optional_components ;; 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 new file mode 100644 index 000000000..f67a2d7a4 --- /dev/null +++ b/test/e2e/postgres_ha_e2e_test.go @@ -0,0 +1,520 @@ +//go:build e2e + +package e2e + +import ( + "context" + "encoding/json" + "fmt" + "net" + "net/http" + "net/url" + "os/exec" + "strconv" + "strings" + "sync" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +const haNamespace = "gameplane-system" + +// This test takes down an actual worker and the fixture database. Never run it +// on a developer's current cluster or share its fault domain with other tests. +func TestPanel_PostgresActiveActiveFailover(t *testing.T) { + if envInstance.Context != "kind-gameplane-postgres-ha" || envInstance.ClusterName != "gameplane-postgres-ha" { + t.Skip("requires the dedicated postgres-ha kind cluster") + } + ctx := t.Context() + envInstance.BootstrapAdmin(t, adminUsername, adminPassword) + admin := envInstance.APIClient(t, adminUsername, adminPassword) + defer admin.Close() + chart, err := findRepoRel("charts/gameplane") + if err != nil { + t.Fatal(err) + } + upgrade := func(args ...string) error { + cmd := append([]string{"upgrade", "gameplane", chart, "--kube-context", envInstance.Context, "--namespace", haNamespace, + "--reuse-values", "--wait", "--timeout", "4m", "--set", "api.postgresHA.enabled=true", "--set", "api.replicas=2"}, args...) + if out, err := exec.CommandContext(ctx, "helm", cmd...).CombinedOutput(); err != nil { + return fmt.Errorf("upgrade HA release: %w\n%s", err, out) + } + return nil + } + // The singleton is running the freshly built image, and has passed that + // image's DB-backed capability probe before we enable concurrent writers. + assertAPIStatus(t, admin, "/readyz/postgres-ha", http.StatusOK) + 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") + haConcurrentWrites(t, clients, id) + haAudit(t, clients[0]) + haAudit(t, clients[1]) + haSessionAndLimiter(t, clients) + + dbPod, err := envInstance.K8s.CoreV1().Pods(haNamespace).Get(ctx, "gameplane-postgres-0", metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + dbNode := dbPod.Spec.NodeName + stable := haServiceClient(t, dbNode, admin) + envInstance.Eventually(t, 30*time.Second, func() (bool, string) { + return haStatus(ctx, stable, "/users/me", http.StatusOK) + }) + // Unlike a kubectl Service port-forward, a NodePort exercises the real + // kube-proxy endpoint set across Pod replacements. Keep using the cookie + // issued by the singleton and make real SQL-backed requests. + finish := haObserveService(ctx, stable) + if err := upgrade("--set-string", "api.logLevel=debug"); err != nil { + _, _ = finish() + t.Fatal(err) + } + samples, err := finish() + if err != nil || samples < 2 { + t.Fatalf("rolling upgrade: %d successful service requests: %v", samples, err) + } + next := haPodPair(t) + for _, old := range pair { + if old.UID == next[0].UID || old.UID == next[1].UID { + t.Fatal("rolling upgrade did not replace both API Pods") + } + } + t.Logf("rolling upgrade served %d authenticated requests, replacing %s/%s with %s/%s", samples, pair[0].Name, pair[1].Name, next[0].Name, next[1].Name) + clients = haDirectClients(t, next, admin) + haEvictWorkerLeader(t, next, clients, id) + pair = haPodPair(t) + clients = haDirectClients(t, pair, admin) + victim := 0 + if pair[0].Spec.NodeName == dbNode { + victim = 1 + } else { + victim = 0 + } + node := pair[victim].Spec.NodeName + if node != "gameplane-postgres-ha-worker" && node != "gameplane-postgres-ha-worker2" { + t.Fatalf("unexpected fault node %q", node) + } + if node == dbNode { + t.Fatal("the node loss drill must leave the database reachable") + } + restart := func(ctx context.Context) error { + out, err := exec.CommandContext(ctx, "docker", "unpause", node).CombinedOutput() + if err != nil { + return fmt.Errorf("resume fixture worker: %w %s", err, out) + } + return nil + } + if out, err := exec.CommandContext(ctx, "docker", "pause", node).CombinedOutput(); err != nil { + t.Fatalf("pause fixture worker: %v %s", err, out) + } + paused := true + defer func() { + if paused { + cleanup, stop := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second) + defer stop() + if err := restart(cleanup); err != nil { + t.Error(err) + } + } + }() + haPatch(t, clients[1-victim], id, "survived worker loss") + envInstance.Eventually(t, 105*time.Second, func() (bool, string) { + current, err := envInstance.K8s.CoreV1().Nodes().Get(ctx, node, metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + for _, condition := range current.Status.Conditions { + if condition.Type == corev1.NodeReady && condition.Status != corev1.ConditionTrue { + return haStatus(ctx, stable, "/users/me", http.StatusOK) + } + } + return false, "awaiting detected node loss and authenticated service failover" + }) + if err := restart(ctx); err != nil { + t.Fatal(err) + } + paused = false + t.Logf("worker %s was unavailable while %s served authenticated writes", node, pair[1-victim].Name) + 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 { + t.Fatalf("game or persistent world stopped progressing during the database outage: before=%v after=%v", identity, current) + } + 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 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" + // 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) { + t.Helper() + ctx := t.Context() + // Match the two-int advisory lock used by the database-backed worker + // election. Inspect the live primary, independently of API-side logging. + const holder = `SELECT host(a.client_addr) FROM pg_locks l JOIN pg_stat_activity a USING(pid) +WHERE l.locktype='advisory' AND l.classid=1196447563 AND l.objid=hashtext(current_schema())::oid AND l.objsubid=2 AND l.granted` + leader := -1 + readLeader := func() (string, error) { + out, err := envInstance.Kubectl(ctx, "exec", "-n", haNamespace, "gameplane-postgres-0", "--", "psql", "-XAt", "-v", "ON_ERROR_STOP=1", "-U", "gameplane", "-d", "gameplane", "-c", holder) + return strings.TrimSpace(out), err + } + envInstance.Eventually(t, time.Minute, func() (bool, string) { + ip, err := readLeader() + for i, pod := range pods { + if err == nil && ip == pod.Status.PodIP { + leader = i + return true, "" + } + } + return false, fmt.Sprintf("expected one worker owner, got %q: %v", ip, err) + }) + node := pods[leader].Spec.NodeName + if out, err := envInstance.Kubectl(ctx, "cordon", node); err != nil { + t.Fatalf("cordon worker owner before eviction: %v %s", err, out) + } + defer func() { + cleanup, stop := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second) + defer stop() + if out, err := envInstance.Kubectl(cleanup, "uncordon", node); err != nil { + t.Errorf("restore scheduling to worker node: %v %s", err, out) + } + }() + // Required hostname anti-affinity and the cordon leave only the existing + // standby able to take over. Exercise the PDB via a normal eviction. + err := envInstance.K8s.CoreV1().Pods(haNamespace).EvictV1(ctx, &policyv1.Eviction{ + ObjectMeta: metav1.ObjectMeta{Name: pods[leader].Name, Namespace: haNamespace}, + }) + if err != nil { + t.Fatal(err) + } + standby := 1 - leader + haPatch(t, clients[standby], id, "survived worker leader eviction") + envInstance.Eventually(t, time.Minute, func() (bool, string) { + ip, err := readLeader() + return err == nil && ip == pods[standby].Status.PodIP, fmt.Sprintf("worker failover owner %q: %v", ip, err) + }) + t.Logf("worker lock transferred from %s to %s", pods[leader].Name, pods[standby].Name) +} + +func haPodPair(t *testing.T) []corev1.Pod { + t.Helper() + var ready []corev1.Pod + envInstance.Eventually(t, 3*time.Minute, func() (bool, string) { + pods, err := envInstance.K8s.CoreV1().Pods(haNamespace).List(t.Context(), metav1.ListOptions{LabelSelector: "app.kubernetes.io/name=gameplane-api"}) + if err != nil { + return false, err.Error() + } + ready = nil + for _, p := range pods.Items { + for _, c := range p.Status.Conditions { + if p.DeletionTimestamp == nil && c.Type == corev1.PodReady && c.Status == corev1.ConditionTrue { + ready = append(ready, p) + } + } + } + return len(ready) == 2 && ready[0].Spec.NodeName != ready[1].Spec.NodeName, fmt.Sprintf("ready API replicas: %d", len(ready)) + }) + return ready +} + +func haDirectClients(t *testing.T, pods []corev1.Pod, admin *APIClient) []*APIClient { + t.Helper() + var clients []*APIClient + for _, pod := range pods { + port, stop := envInstance.PortForward(t, haNamespace, "pod/"+pod.Name, 8000) + t.Cleanup(stop) + client := &APIClient{BaseURL: fmt.Sprintf("http://127.0.0.1:%d", port), HTTP: admin.HTTP, CSRF: admin.CSRF} + assertAPIStatus(t, client, "/users/me", http.StatusOK) + clients = append(clients, client) + } + return clients +} + +func haPatch(t *testing.T, client *APIClient, id, displayName string) { + t.Helper() + 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) + } +} + +func haConcurrentWrites(t *testing.T, clients []*APIClient, id string) { + t.Helper() + var writers sync.WaitGroup + for n, client := range clients { + writers.Go(func() { + for i := range 6 { + 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 + } + } + }) + } + writers.Wait() +} + +func haAudit(t *testing.T, client *APIClient) { + t.Helper() + body := assertAPIStatus(t, client, "/admin/audit/verify", http.StatusOK) + var audit struct { + OK bool + Checked int + } + if err := json.Unmarshal(body, &audit); err != nil || !audit.OK || audit.Checked < 12 { + t.Fatalf("cross-writer audit integrity: %s", body) + } +} + +func haSessionAndLimiter(t *testing.T, clients []*APIClient) { + t.Helper() + // A fresh viewer session is first read (and potentially cached) on A. + // 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}} + 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 || 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) + a, _ := url.Parse(first.BaseURL) + 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)) + 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}} + 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 || status != want { + t.Fatalf("shared username login budget, attempt %d: %v %v (want %d)", attempt+1, err, status, want) + } + } +} + +func haServiceClient(t *testing.T, nodeName string, admin *APIClient) *APIClient { + t.Helper() + api := envInstance.K8s.CoreV1() + svc, err := api.Services(haNamespace).Create(t.Context(), &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "ha-drill"}, + Spec: corev1.ServiceSpec{Type: corev1.ServiceTypeNodePort, Selector: map[string]string{"app.kubernetes.io/name": "gameplane-api"}, + Ports: []corev1.ServicePort{{Name: "http", Port: 80, TargetPort: intstr.FromString("http")}}}, + }, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + node, err := api.Nodes().Get(t.Context(), nodeName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + for _, addr := range node.Status.Addresses { + if addr.Type == corev1.NodeInternalIP { + base := &url.URL{Scheme: "http", Host: net.JoinHostPort(addr.Address, strconv.Itoa(int(svc.Spec.Ports[0].NodePort)))} + u, _ := url.Parse(admin.BaseURL) + cli := &http.Client{Jar: newInsecureCookieJar(), Timeout: 3 * time.Second} + cli.Jar.SetCookies(base, admin.HTTP.Jar.Cookies(u)) + return &APIClient{BaseURL: base.String(), CSRF: admin.CSRF, HTTP: cli} + } + } + t.Fatalf("node %q has no reachable InternalIP", nodeName) + return nil +} + +func haStatus(ctx context.Context, client *APIClient, path string, want int) (bool, string) { + status, body, err := tlDo(ctx, client, http.MethodGet, path, nil) + if err != nil { + return false, err.Error() + } + return status == want, fmt.Sprintf("%s returned %d: %s", path, status, body) +} + +func haObserveService(ctx context.Context, stable *APIClient) func() (int, error) { + stop := make(chan struct{}) + done := make(chan struct{}) + var count int + var failure error + go func() { + defer close(done) + tick := time.NewTicker(250 * time.Millisecond) + defer tick.Stop() + for { + if ok, why := haStatus(ctx, stable, "/users/me", http.StatusOK); !ok { + failure = fmt.Errorf("service continuity: %s", why) + return + } + count++ + select { + case <-stop: + return + case <-ctx.Done(): + failure = ctx.Err() + return + case <-tick.C: + } + } + }() + return func() (int, error) { close(stop); <-done; return count, failure } +} + +func haDatabaseOutage(t *testing.T, clients []*APIClient, id string) { + t.Helper() + setScale := func(ctx context.Context, n int32) error { + sts := envInstance.K8s.AppsV1().StatefulSets(haNamespace) + scale, err := sts.GetScale(ctx, "gameplane-postgres", metav1.GetOptions{}) + if err != nil { + return err + } + scale.Spec.Replicas = n + _, err = sts.UpdateScale(ctx, "gameplane-postgres", scale, metav1.UpdateOptions{}) + return err + } + if err := setScale(t.Context(), 0); err != nil { + t.Fatal(err) + } + restored := false + defer func() { + if !restored { + ctx, stop := context.WithTimeout(context.WithoutCancel(t.Context()), 30*time.Second) + defer stop() + if err := setScale(ctx, 1); err != nil { + t.Error(err) + } + } + }() + for _, client := range clients { + envInstance.Eventually(t, time.Minute, func() (bool, string) { + return haStatus(t.Context(), client, "/readyz/postgres-ha", http.StatusServiceUnavailable) + }) + assertAPIStatus(t, client, "/healthz", http.StatusOK) + 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 { + t.Fatal(err) + } + restored = true + for _, client := range clients { + envInstance.Eventually(t, 2*time.Minute, func() (bool, string) { + return haStatus(t.Context(), client, "/readyz/postgres-ha", http.StatusOK) + }) + body := assertAPIStatus(t, client, "/users", http.StatusOK) + if strings.Contains(string(body), "database-unavailable") { + t.Fatal("an unavailable write was reported failed but committed") + } + haPatch(t, client, id, "database-recovered") + } +} diff --git a/test/e2e/postgres_pitr_e2e_test.go b/test/e2e/postgres_pitr_e2e_test.go new file mode 100644 index 000000000..fd96f3252 --- /dev/null +++ b/test/e2e/postgres_pitr_e2e_test.go @@ -0,0 +1,162 @@ +//go:build e2e + +package e2e + +import ( + "context" + "crypto/sha256" + "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) + } + // 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, name, metav1.GetOptions{}) + if err != nil || key.UID == "" || len(key.Data[field]) == 0 { + t.Fatalf("read native credential %s: %v", name, err) + } + 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 := 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) + 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") + 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) + 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("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) + 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 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) +} diff --git a/test/e2e/suite-components.json b/test/e2e/suite-components.json index d2f3e6746..fc0cfce38 100644 --- a/test/e2e/suite-components.json +++ b/test/e2e/suite-components.json @@ -56,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_", 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