Skip to content

test: postgres-backed e2e - #707

Merged
ValgulNecron merged 4 commits into
masterfrom
claude/013-postgres-e2e
Oct 16, 2026
Merged

ValgulNecron merged 4 commits into
masterfrom
claude/013-postgres-e2e

Conversation

@ValgulNecron

@ValgulNecron ValgulNecron commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Before: the Postgres store was only tested at the DB layer (api (postgres)), and nothing showed that a database stuck at the frozen 001–012 migrations upgrades cleanly. No E2E run used a Postgres-backed API (spec 013 gaps G-23, G-24).

After: a new e2e postgres / amd64 (kind) job brings up kind with PostgreSQL 17 and installs the chart with api.db.driver=postgres. It checks that the API Deployment runs with --db-driver=postgres, then runs the api-auth, api-roles and api-rbac buckets against it. api (postgres) also runs the new TestPostgres_MigrateFromFrozenBaseline, which applies only migrations/postgres 001–012, writes a user, a role binding, a share link and an audit row with the 012-era columns, migrates to head, and checks all four rows read back unchanged and every version is recorded.

How:

  • deploy/kind/postgres.yaml holds the StatefulSet, Service and Secret, using the same image digest as the api (postgres) service.
  • deploy/kind/e2e.sh takes an optional fourth argument, the profile. The postgres profile applies that fixture, waits for pg_isready, retags gameplane-test/api:e2e-postgres under the shared tag, and adds the driver and DSN to the Helm install.
  • The CI job restarts the API between buckets. The login rate limiter is in memory, and each bucket is sized to one fresh limiter's budget.
  • newPostgresStore is split so the new test can open an unmigrated schema. The existing tests behave the same.

Chart fix found by the new job (valgul chose "Fix in #707"): the chart passed the database to the API only as container args, so kubectl exec deploy/gameplane-api -- /api bootstrap-admin (docs/install.md) fell back to SQLite on a PostgreSQL install and created the admin in a throwaway emptyDir file. charts/gameplane/templates/api.yaml now also sets GAMEPLANE_DB_DRIVER and GAMEPLANE_DB_DSN on the api container.

Spec 013 tasks T054–T057. G-23 and G-24 flip after the first green run (T058).

Two deviations from the task text:

  • The contract says e2e.sh up "$CLUSTER" postgres, but the third argument is already the image tag, so the job calls e2e.sh up "$CLUSTER" e2e postgres.
  • The restart steps between buckets are an addition to the three go test steps in T057.

Generated by Claude Code

Summary by CodeRabbit

  • Improvements
    • PostgreSQL settings are now passed consistently to the API when deploying with a configured database.
    • The local end-to-end test setup now supports a PostgreSQL profile, including checks that the database is ready before tests run.
  • Testing
    • Added end-to-end validation of API behavior with PostgreSQL and checks that upgrades from an earlier database version preserve existing data.

Spec 013 US5 (T054-T057): TestPostgres_MigrateFromFrozenBaseline upgrades
a schema stopped at the frozen 001-012 set to head and checks rows survive;
deploy/kind/postgres.yaml and an e2e.sh postgres profile run the API on
PostgreSQL in kind; the new e2e postgres job re-runs api-auth, api-roles
and api-rbac against it, restarting the API between buckets so each gets
a fresh in-memory login limiter.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
@ValgulNecron ValgulNecron added area: api REST/WebSocket gateway area: e2e kind-based end-to-end suite type: test Test additions or changes labels Oct 15, 2026 — with Claude
@ValgulNecron ValgulNecron self-assigned this Oct 15, 2026
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: GameplanePanel/Gameplane/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c968896d-11c3-46e0-a866-90cf74fbceb5
📥 Commits

Reviewing files that changed from the base of the PR and between 63308ed and 8c0410f.

📒 Files selected for processing: 7
  • .github/workflows/ci.yaml
  • api/internal/db/db_postgres_test.go
  • charts/gameplane/templates/api.yaml
  • deploy/kind/e2e.sh
  • deploy/kind/postgres.yaml
  • specs/013-expand-test-coverage/coverage-gaps.md
  • specs/013-expand-test-coverage/tasks.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

⏰ Context from checks skipped due to timeout: 29
  • GitHub Check: e2e api-rbac / amd64 (kind)
  • GitHub Check: e2e api-agent / amd64 (kind)
  • GitHub Check: e2e telemetry / amd64 (kind)
  • GitHub Check: e2e optional-components / amd64 (kind)
  • GitHub Check: e2e api-roles / amd64 (kind)
  • GitHub Check: e2e upgrade / amd64 (kind)
  • GitHub Check: e2e api-auth / amd64 (kind)
  • GitHub Check: e2e game bot / TestGameServer_MinecraftJavaBot_Joined (kind)
  • GitHub Check: e2e game bot / TestGameServer_WakeOnConnect_PingDoesNotWake (kind)
  • GitHub Check: e2e game bot / TestGameServer_GarrysModBot_Query (kind)
  • GitHub Check: e2e game bot / TestGameServer_TerrariaBot_Joined (kind)
  • GitHub Check: e2e game bot / TestGameServer_WakeOnConnect_UnarmedNoSentinel (kind)
  • GitHub Check: e2e api-mods / amd64 (kind)
  • GitHub Check: e2e game bot / TestGameServer_WakeOnConnect_LoginWakes (kind)
  • GitHub Check: e2e capture overhead (kind)
  • GitHub Check: e2e operator / amd64 (kind)
  • GitHub Check: e2e postgres / amd64 (kind)
  • GitHub Check: e2e web live / amd64 (kind)
  • GitHub Check: e2e multicluster / amd64 (kind x2)
  • GitHub Check: vuln (images)
  • GitHub Check: e2e api-mods / arm64 (kind)
  • GitHub Check: e2e api-auth / arm64 (kind)
  • GitHub Check: e2e optional-components / arm64 (kind)
  • GitHub Check: e2e telemetry / arm64 (kind)
  • GitHub Check: e2e api-agent / arm64 (kind)
  • GitHub Check: e2e multicluster / arm64 (kind x2)
  • GitHub Check: e2e api-roles / arm64 (kind)
  • GitHub Check: e2e operator / arm64 (kind)
  • GitHub Check: e2e web live / arm64 (kind)
🧰 Additional context used
📓 Path-based instructions: 4
CRITICAL: Every workflow MUST follow hardened-CI practices: All external actions MUST be pinned to a full 40-hex commit SHA (not a tag or branch) Each action SHA pin MUST include an inline version comment for human readability Workflows MUS...

⚙️ CodeRabbit configuration file

Files:

  • .github/workflows/ci.yaml
CRITICAL: Flag test deletions or weakening: A test removed or commented out (sign: test file size decreased) A test case removed from a table-driven test A test condition relaxed (e.g., comparison changed from == to >) A test assertion remo...

⚙️ CodeRabbit configuration file

Files:

  • api/internal/db/db_postgres_test.go
CRITICAL: Behavior changes must update the relevant module's specs.md.

⚙️ CodeRabbit configuration file

Files:

  • api/internal/db/db_postgres_test.go
CRITICAL: Gameplane's house rules forbid in-source lint suppressions.

⚙️ CodeRabbit configuration file

Files:

  • api/internal/db/db_postgres_test.go
🪛 Betterleaks (1.9.0)
deploy/kind/postgres.yaml

[high] 19-19: Detected a password embedded in a service connection URI, which may expose direct access to the referenced service.

(generic-credential-uri)

[high] 17-17: Detected a potential hardcoded password literal, which may expose account credentials.

(generic-password)

🪛 Checkov (3.3.20)
deploy/kind/postgres.yaml

[medium] 34-79: Containers should not run with allowPrivilegeEscalation

(CKV_K8S_20)

[medium] 34-79: Minimize the admission of root containers

(CKV_K8S_23)

[medium] 19-20: Basic Auth Credentials

(CKV_SECRET_4)

🪛 LanguageTool
specs/013-expand-test-coverage/tasks.md

[uncategorized] ~379-~379: The official name of this software platform is spelled with a capital “H”.
Context: ... named e2e postgres / amd64 (kind) to .github/workflows/ci.yaml, modeled on `e2e-cap...

(GITHUB)

🔇 Additional comments: 3
api/internal/db/db_postgres_test.go: 1

10-10: LGTM!

Also applies to: 30-40, 426-557

specs/013-expand-test-coverage/coverage-gaps.md: 1

3-3: LGTM!

Also applies to: 30-31

specs/013-expand-test-coverage/tasks.md: 1

366-366: LGTM!

Also applies to: 372-373, 379-379, 388-389

Walkthrough

Adds PostgreSQL migration regression coverage and a PostgreSQL-backed Kind profile. CI runs selected API E2E buckets against PostgreSQL and includes the job in its report.

Changes

PostgreSQL validation

Layer / File(s) Summary
Frozen migration regression
api/internal/db/db_postgres_test.go, specs/013-expand-test-coverage/coverage-gaps.md, specs/013-expand-test-coverage/tasks.md
The test helpers now distinguish an unmigrated schema from a migrated store. A regression test applies the frozen migrations, seeds representative records, migrates to head, and checks migration records and retained data. The coverage summary records the migration gap as closed.
PostgreSQL Kind profile
charts/gameplane/templates/api.yaml, deploy/kind/e2e.sh, deploy/kind/postgres.yaml, specs/013-expand-test-coverage/coverage-gaps.md, specs/013-expand-test-coverage/tasks.md
The Kind script accepts a PostgreSQL profile, deploys and checks the PostgreSQL fixture, and passes its DSN and driver to Helm. The API Deployment receives the database environment variables. The coverage summary records the E2E gap as closed.
PostgreSQL E2E CI job
.github/workflows/ci.yaml
CI adds an amd64 job that starts the PostgreSQL profile, checks the API database argument, and runs the api-auth, api-roles, and api-rbac buckets with API restarts between buckets. The report waits for and lists the job result.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Other

Suggested labels: type: ci, type: docs, area: specs

Merge Risk: ⚪ Minimal · up to 8c041

The PostgreSQL test buckets use the intended cluster. No merge-blocking issue was identified; merge after normal checks.

Security architecture risk: 🔵 Low · up to 8c041

The changes are narrowly scoped to test infrastructure and making administrator commands use the configured database. No expanded production permissions were found. The test cluster has no explicit end-of-job teardown, so cleanup depends on the worker lifecycle.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The newly provisioned database and its known credentials are confined to the E2E cluster. Compromise of the tested API would expose that database and the API's existing Kubernetes authority within the test cluster. The new job grants only contents:read, and the inspected configuration supplies no production database credential.

Trust Boundaries and Controls

  • inferred — The production trust transition remains release-controlled database configuration consumed by a privileged administrator command. Supplying its existing environment contract changes the default target database, not who may execute the command. A caller already able to execute bootstrap-admin could explicitly provide the same database flags, and the DSN was already present in pod arguments.

Resilience and Maintainability Implications

  • observed — Initial administrator creation and its role binding remain separate writes, whereas forced reset transactionally updates credentials, role binding and session eviction. The separate-write failure state and lack of cross-process migration serialization predate this PR; the comparison did not establish increased attacker authority or production rollout concurrency.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (5 skipped: 5… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: adding PostgreSQL-backed end-to-end testing.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (5 skipped: 5 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Collaborator Author

e2e postgres / amd64 (kind) fails on 989be03. Every test that logs in as the bootstrapped admin gets login 401: invalid credentials, while the OIDC tests pass (job log).

Cause: this is a real chart bug that the new job exposes. The chart passes the database to the API only as container args (--db-driver/--db-dsn, charts/gameplane/templates/api.yaml:289-290). bootstrap-admin run through kubectl exec deploy/gameplane-api -- /api bootstrap-admin (as in docs/install.md:94) doesn't see those args, so it falls back to the GAMEPLANE_DB_DRIVER/GAMEPLANE_DB_DSN env defaults: SQLite at /data/gameplane.db. On a Postgres install, that file is an emptyDir. The admin lands in a throwaway SQLite file, and the API on Postgres never sees it.

Proposed fix: also set GAMEPLANE_DB_DRIVER and GAMEPLANE_DB_DSN as env on the api container, so execed subcommands use the same database. That's a production chart change, so it waits for valgul's sign-off before I push it.


Generated by Claude Code

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

CI Report - 8c0410f

run 38826485921 - attempt 1

0 failed - 33 passed - 0 skipped - 60m36s elapsed

Coverage

Module Measured Gate Margin
web L95.6 S94.05 B89.13 F91.5 L92 S92 B82 F76 +2.05

Changed

Area Files +/-
api/ 1 +144/-3
deploy/ 2 +136/-3
.github/ 1 +79/-1
specs/ 2 +9/-8
charts/ 1 +8/-0

Slowest jobs

Job Duration
e2e telemetry / arm64 (kind) 24m5s
e2e telemetry / amd64 (kind) 23m18s
e2e capture overhead (kind) 12m40s
build e2e images 12m15s
e2e multicluster / arm64 (kind x2) 9m52s

e2e buckets run

api-agent, api-auth, api-mods, api-rbac, api-roles, bot-fast, multicluster, operator, optional-components, ratelimit, telemetry, upgrade

bootstrap-admin and other subcommands run through kubectl exec don't see
the container's --db-driver/--db-dsn args, so on a PostgreSQL install they
fell back to the SQLite default in the emptyDir and the new admin never
reached the real database. Set GAMEPLANE_DB_DRIVER and GAMEPLANE_DB_DSN
from the same values.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
@ValgulNecron ValgulNecron added area: chart Helm chart and deployment type: fix Bug fix labels Oct 15, 2026 — with Claude

Copy link
Copy Markdown
Collaborator Author

build e2e images (arm64) failed at go mod download for the e2e-mcp-server target. The cause was a Go module proxy transport error (proxy.golang.org … apiextensions-apiserver@v0.37.0.zip: stream error … INTERNAL_ERROR; received from peer). It happened while dependencies were being fetched, before anything this PR changes was built or tested. This is the same error #704 hit. I'm re-running the failed jobs once.


Generated by Claude Code

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
@ValgulNecron
ValgulNecron marked this pull request as ready for review October 15, 2026 23:56

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review


Generated by Claude Code

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Collaborator Author

CodeRabbit's Docstring Coverage warning (75%) counts the one-line e2e-postgres matcher in JOB_MATCHERS in .github/workflows/ci.yaml. None of its roughly 40 sibling matchers has a comment, and valgul chose to leave it as is. The three Go functions this PR touches all have doc comments.


Generated by Claude Code

@ValgulNecron
ValgulNecron merged commit ae8bd5a into master Oct 16, 2026
100 checks passed
ValgulNecron pushed a commit that referenced this pull request Oct 16, 2026
Brings in #701 (chart CRDs moved to the gameplane-crds subchart), #619,
#704, #706, #707, #708 and #709. No conflicts; hack/sync-chart-crds.sh
leaves the subchart CRD copies unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: api REST/WebSocket gateway area: chart Helm chart and deployment area: e2e kind-based end-to-end suite type: fix Bug fix type: test Test additions or changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants