pal-e-docs

Review: Critical: Re-establish orphaned CNPG cluster manifest

review-423-2026-03-26 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — standalone, discovered during #187 review
  • [x] Repo — forgejo_admin/pal-e-services
  • [x] What Broke — clear description of orphaned manifest with impact list
  • [x] Repro Steps — 4 concrete steps, all verified
  • [x] Expected Behavior — clear target state
  • [x] Environment — cluster, version, storage, backup, databases all documented
  • [x] Acceptance Criteria — 5 criteria listed
  • [x] Related — 3 blocked issues + removed commit reference
  • [x] Architecture diagram — excellent current vs target state visualization
  • [x] Cluster Spec — reconstructed from removed commit
  • [x] File Targets — present
  • [x] Constraints — critical safety notes about live cluster

Traceability

  • [ ] story:X label — missing, but this is foundational infrastructure repair. Acceptable.
  • [x] arch:postgres label — present on board item #423
  • [x] Forgejo issue — forgejo_admin/pal-e-services#33, open

File Targets

  • [x] ~/pal-e-services/ — verified: no pal-e-postgres references exist (grep confirms orphaned state)
  • [x] ~/pal-e-platform/terraform/main.tf — verified: only connection string references remain (line 2206: DATABASE_URL, line 2465: backup verification). No Cluster resource.
  • [x] Live cluster — verified: kubectl get clusters.postgresql.cnpg.io -n postgres shows pal-e-postgres healthy, 1/1 ready, 24d age
  • [x] pal-e-services repo structure — verified: terraform directory exists with main.tf, services.tf, providers.tf. Has kubernetes provider configured. Pattern exists for kubernetes_manifest resources.

Repo Placement

Correct. The issue is filed on pal-e-services (where the manifest should land). The architectural separation decision is documented: platform-level resources (namespace, S3 creds, MinIO bucket, CNPG operator) stay in pal-e-platform; app-level resources (CNPG clusters) move to pal-e-services. The woodpecker CNPG cluster in pal-e-platform is the exception (it IS a platform resource).
Note: the issue correctly identifies that network policy changes for new namespace access remain in pal-e-platform. No cross-repo issue needed for this ticket itself, but the dependent issues (#187, #189, #190) will need pal-e-platform network policy PRs.

Dependencies

  • Blocks #417 (board item) — Critical: Migrate basketball-api Postgres to CNPG (pal-e-platform#187, open)
  • Blocks #419 (board item) — Critical: Migrate mcd-tracker Postgres to CNPG (pal-e-platform#189, open)
  • Blocks #420 (board item) — Critical: Migrate pal-e-mail Postgres to CNPG (pal-e-platform#190, open)
  • No blockers — this ticket has no upstream dependencies. The CNPG operator, namespace, S3 creds, and MinIO bucket all exist in pal-e-platform and are healthy.
  • Dependencies are well-documented in the issue body.

Acceptance Criteria

  • [x] AC1: "CNPG cluster manifest exists in pal-e-services repo" — clear, verifiable
  • [x] AC2: "Manifest matches running cluster spec (verified via kubectl diff)" — clear, verifiable
  • [x] AC3: "kubectl apply is idempotent" — clear, verifiable
  • [ ] AC4: "Manifest supports adding additional databases" — MISLEADING. The issue's own Constraints section correctly notes that bootstrap.initdb only runs on initial creation and new databases use SQL. AC4 implies the manifest itself enables adding databases, but adding databases to an existing CNPG cluster is a SQL operation, not a manifest change. This AC should be reworded to: "Documentation explains that new databases require SQL creation, not manifest changes" or removed entirely since it's covered by the Constraints section.
  • [x] AC5: "Documentation updated: which repo owns which CNPG resource" — clear, verifiable

Blast Radius

  • DISCOVERED SCOPE: Orphaned ScheduledBackup. The pal-e-postgres-daily ScheduledBackup is ALSO running in the postgres namespace with no manifest in any repo. It was likely removed alongside the Cluster manifest in the same commit. This must be included in the fix — a CNPG cluster manifest without its ScheduledBackup is incomplete. The ScheduledBackup spec (from live cluster): schedule 0 0 2 * * * (daily 02:00 UTC), method barmanObjectStore, backupOwnerReference: cluster.
  • Backup verification CronJob — pal-e-platform main.tf (line 2465) already references pal-e-postgres in its backup verification loop. This will continue working since it checks MinIO objects, not the manifest. No impact.
  • pal-e-docs DATABASE_URL — pal-e-platform main.tf (line 2206) references pal-e-postgres-rw.postgres.svc.cluster.local. This connection string will remain valid. No impact.
  • No similar orphaned patterns found in other repos for this specific issue class.

Recommendation

Two items need refinement before this ticket is READY:
  • Add the orphaned ScheduledBackup to scope. The pal-e-postgres-daily ScheduledBackup is equally orphaned and must be re-established alongside the Cluster manifest. Add it to the Architecture diagram (current state shows it missing), add it to File Targets, and add an AC: "ScheduledBackup manifest exists in pal-e-services and matches running state."
  • Reword AC4. Change "Manifest supports adding additional databases" to "Documentation explains that new databases require SQL creation on the running cluster, not manifest changes to bootstrap.initdb" — or simply remove it, since the Constraints section already covers this and it's the dependent issues' responsibility.