pal-e-docs

Review: Critical: Migrate basketball-api Postgres to CNPG

review-417-2026-03-26 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — standalone, discovered during #184
  • [x] Repo — lists 3 repos (pal-e-platform, pal-e-deployments, basketball-api)
  • [x] User Story — WS-S5, clearly stated
  • [x] What Broke — thorough description of the data safety gap
  • [x] Repro Steps — 4 kubectl commands
  • [x] Expected Behavior — stated
  • [x] Environment — detailed (PG versions, DB size, namespace info)
  • [x] Acceptance Criteria — 6 criteria
  • [x] Related — references #184, project-westside-basketball, sop-postgres-restore
  • [x] Architecture diagram — before/after ASCII diagrams included
  • [x] Migration Steps — 7-step migration plan (bonus, not required by template)
  • [x] Test Expectations — specific SQL queries + kubectl checks (bonus)
  • [x] Constraints — PG version gap, zero downtime, deletion gate (bonus)
  • [ ] File Targets — present but has issues (see below)

Traceability

  • [x] story:WS-S5 label — present on board item #417
  • [x] arch:postgres label — present on board item #417
  • [x] Forgejo issue — forgejo_admin/pal-e-platform#187, open
Traceability triangle is complete.

File Targets

  • [x] terraform/main.tf — verified: exists, contains CNPG operator, backup verify CronJob, MinIO CNPG credentials, and woodpecker CNPG cluster as reference patterns. However, the pal-e-postgres shared cluster definition is NOT in this file (see Repo Placement).
  • [~] ~/pal-e-deployments/basketball-api/ — ISSUE: path in ticket is wrong. Actual path is overlays/basketball-api/prod/. Contains kustomization.yaml, deployment-patch.yaml, postgres.yaml, pvc.yaml, harbor-creds.enc.yaml. The standalone postgres Deployment + PVC + Service are in postgres.yaml. The DATABASE_URL is in deployment-patch.yaml line 30: postgresql://basketball:$(POSTGRES_PASSWORD)@postgres:5432/basketball.
  • [x] ~/basketball-api/src/basketball_api/config.py — verified: exists, line 5 shows default database_url, uses env_prefix = "BASKETBALL_" so env var BASKETBALL_DATABASE_URL overrides. Connection string change is purely a kustomize patch change, not a code change.
  • [x] ~/basketball-api/k8s/deployment.yaml — verified: exists, line 42 has BASKETBALL_DATABASE_URL pointing at postgres:5432. Note: this file is the raw k8s manifest, but ArgoCD deploys from pal-e-deployments kustomize overlays. The actual change target is the kustomize overlay, not this file.

Repo Placement

ISSUE: The ticket is filed on pal-e-platform and lists 3 repos, but there is a critical gap:
  • The pal-e-postgres CNPG Cluster (the "shared cluster" referenced in the ticket) is NOT defined in terraform/main.tf. It exists in the running k8s cluster (pal-e-docs connects to pal-e-postgres-rw.postgres.svc.cluster.local) but its manifest is not managed by Terraform in either pal-e-platform or pal-e-services. The ticket assumes adding a database to this cluster is a terraform change, but the cluster definition's location is undocumented.
  • The Woodpecker CNPG cluster IS defined in terraform/main.tf (line 1549), but that's a separate per-service cluster in the woodpecker namespace, not a shared cluster in the postgres namespace.
  • The ticket should clarify: is the basketball database going on the existing pal-e-postgres cluster (requires finding/documenting its manifest), or is a new dedicated CNPG cluster being created (like woodpecker-db)?

Dependencies

  • Board item #411 (in_progress) — "Bug: Harbor connectivity timeout from Woodpecker CI agent" — this is the #184 CI blocker that triggered discovery. Not a direct blocker for this ticket, but if CI is broken, the PR can't be validated.
  • Network policy — The postgres namespace NetworkPolicy (network-policies.tf line 161-183) currently allows ingress only from pal-e-docs, cnpg-system, and monitoring. The basketball-api namespace is NOT in the allow list. The ticket's Constraints section mentions "Network policy in postgres namespace must allow traffic from basketball-api namespace" but does not list this as a file target or acceptance criterion. This is a required change that's undocumented in the scope.
  • CNPG S3 credentials — If basketball uses the existing pal-e-postgres cluster, no new S3 creds are needed (the cluster already has Barman configured). If a new cluster is created, new S3 creds + MinIO policy may be needed.

Acceptance Criteria

  • [x] "basketball database exists on CNPG shared cluster" — verifiable via kubectl exec + psql
  • [x] "basketball-api connects to CNPG, all endpoints functional" — verifiable via health check + API calls
  • [x] "Daily Barman backup covers basketball database" — verifiable, but note: CNPG backups are cluster-level, not database-level. If basketball is on the shared cluster, existing ScheduledBackup covers it automatically.
  • [~] "cnpg-backup-verify CronJob validates basketball data restores correctly" — MISLEADING. The CronJob (main.tf line 2420) checks WAL freshness in MinIO by prefix ("pal-e-postgres", "woodpecker"). It does NOT do database-level restore validation. It verifies that backup objects exist and are recent. The criterion should say "cnpg-backup-verify CronJob passes with basketball data included in pal-e-postgres backups."
  • [x] "Old standalone postgres Deployment + PVC removed" — verifiable
  • [x] "Zero downtime" — testable by monitoring health endpoint during cutover
  • [ ] MISSING: "Network policy updated to allow basketball-api namespace ingress to postgres namespace" — required for connectivity
  • [ ] MISSING: "Row count validation pre/post migration" — mentioned in Constraints/Test Expectations but not in AC

Blast Radius

Two other services have the identical standalone postgres pattern with zero backup coverage:
  • pal-e-deployments/overlays/mcd-tracker/prod/postgres.yaml — postgres:16-alpine, standalone Deployment, same risk profile
  • pal-e-deployments/overlays/pal-e-mail/prod/postgres.yaml — postgres:16-alpine, standalone Deployment, same risk profile
These should get their own tickets (discovered scope). The fix pattern from this ticket should be documented as a repeatable playbook for mcd-tracker and pal-e-mail.

Recommendation

Four items must be addressed before this ticket is READY:
  • Clarify CNPG cluster definition location — Where is the pal-e-postgres CNPG Cluster manifest? Is it a raw kubectl apply, a Helm release, or unmanaged? The ticket must specify how the basketball database + user get created (CNPG initdb won't work on an existing cluster — need to use CREATE DATABASE + CREATE USER via psql, or define a new CNPG cluster).
  • Add network policy change to file targets + acceptance criteria — terraform/network-policies.tf line 175-179 must add basketball-api namespace. This is a required change currently missing from scope.
  • Fix file target paths — ~/pal-e-deployments/basketball-api/ should be ~/pal-e-deployments/overlays/basketball-api/prod/. Clarify that ~/basketball-api/k8s/deployment.yaml is NOT the deploy target (ArgoCD uses the kustomize overlay).
  • Correct the backup verification acceptance criterion — The CronJob checks WAL freshness by prefix, not per-database restore. Reword to match actual behavior.
Additionally, two discovered-scope items should be filed as separate tickets:
  • mcd-tracker standalone postgres migration to CNPG
  • pal-e-mail standalone postgres migration to CNPG