fix: remove stale basketball-api and pal-e-app namespace references #450
No reviewers
Labels
No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
ldraney/pal-e-platform!450
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/remove-stale-namespace-refs"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Both
basketball-apiandpal-e-appnamespaces were deleted from the cluster but Terraform still referenced them as data sources, causing every CI apply to fail with "namespaces not found" errors. This PR removes all stale code references, orphaned dashboard JSON files, moved blocks, CI pipeline variables, and module wiring.Changes
pal_e_productiondata source,paledocs_db_urlsecret,basketball_apidata source,admin_app_user_provisionjob,admin_app_db_urlsecret,admin_app_db_url_westside_adminsecret mirror, andadmin_app_database_urllocalpal_e_production_namespace,admin_app_db_url_secret_name,admin_app_db_url_namespacesoutputsadmin_app_db_passwordandpaledocs_db_passwordvariables (no longer consumed)embedding_worker_metricsservice targeting pal-e-app namespacepal_e_production_namespacevariablepal_e_production_dashboardconfigmap,basketball_api_dashboardconfigmap,embedding_worker_service_monitor,embedding_alerts,payment_pipeline_alerts,gmail_oauth_expiry_alert, and basketball-api blackbox targetnetpol_basketball_apiresource and stalefromrules referencing basketball-api/pal-e-app in keycloak and postgres netpolsadmin_app_db_passwordandpaledocs_db_passwordfrom database module call,pal_e_production_namespacefrom ops module call, and stale moved blocks (pal_e_production_dashboard, embedding_worker_service_monitor, embedding_alerts, embedding_worker_metrics, paledocs_db_url)admin_app_db_passwordandpaledocs_db_passwordroot variablesbasketball-api-golden-signals.jsonandpal-e-app-golden-signals.jsonpaledocs_db_passwordandadmin_app_db_passwordfrom TF_SECRET_VARSTF_VAR_paledocs_db_passwordandTF_VAR_admin_app_db_passwordfrom both plan and apply stepsTest Plan
basketball_api,pal_e_production,admin_app_db,embedding_worker_metrics,embedding_alerts,payment_pipeline,gmail_oauth_expiry, orpaledocs_dbremain in any.tffiletofu state rmfor orphaned resources must be run manually before merge (requires cluster access)Review Checklist
tofu state rmfor all removed resources before mergingRelated Notes
westside_admindata source and itsadmin_app_db_url_westside_adminsecret were also removed since they depend on the basketball-api database URL local that no longer existsCloses #449
PR #450 Review
DOMAIN REVIEW
Tech stack: Terraform/HCL (Plugin Framework resources, modules, moved blocks), Woodpecker CI YAML, Makefile, Grafana dashboard JSON. This is a Terraform/k8s/IaC domain review.
Nature of change: Pure deletion PR -- 1,401 lines removed, 3 lines added (whitespace alignment in
terraform/main.tf). Removes all references to two decommissioned namespaces (basketball-api,pal-e-app) that were causing CI apply failures due to missing namespace data sources.Scope of deletions (verified across all 13 files):
TF_VAR_paledocs_db_password,TF_VAR_admin_app_db_passwordfrom plan + apply steps (4 removals)paledocs_db_password,admin_app_db_passwordfromTF_SECRET_VARSpaledocs_db_password,admin_app_db_passworddeclarationsmain.tffor database + ops modulespal_e_productiondata source,paledocs_db_urlsecret,basketball_apidata source,admin_app_user_provisionjob,admin_app_db_urlsecret,westside_admindata source + mirror secret,admin_app_database_urllocalpal_e_production_namespace,admin_app_db_url_secret_name,admin_app_db_url_namespacespaledocs_db_password,admin_app_db_passwordembedding_worker_metricsservice +pal_e_production_namespacevariablepal_e_production_dashboard,basketball_api_dashboardconfigmaps,embedding_worker_service_monitor,embedding_alerts,payment_pipeline_alerts,gmail_oauth_expiry_alertPrometheusRules, basketball-api blackbox targetnetpol_basketball_apientirely, removedbasketball-apifrom keycloak ingress, removedpal-e-app+basketball-apifrom postgres ingressbasketball-api-golden-signals.jsonandpal-e-app-golden-signals.jsonTerraform-specific assessment:
sensitive = trueannotations on removed variables were correct originally; no secrets exposure risk from this deletionState management concern (addressed in PR body):
The PR body correctly notes that
tofu state rmmust be run for orphaned resources before merge. This is the correct procedure -- Terraform will attempt to destroy resources it manages but can no longer reach (deleted namespaces), which would fail. The Review Checklist item for this is appropriate.BLOCKERS
None.
This is a pure deletion PR removing references to already-decommissioned infrastructure. There is no new functionality, so the "no tests for new functionality" blocker does not apply. The deletions are internally consistent -- every variable/output/resource removal is traced through the full module call chain.
NITS
Alignment whitespace (line 812-813): The only "additions" are re-aligning
cnpg_iam_user_nameandcnpg_iam_user_secretinterraform/main.tfafter removing the two lines above them. This is fine --tofu fmtcompliance. Not a concern.Comment cleanup in monitoring/main.tf (line 1207-1209): The removed basketball-api blackbox target also removed the comment
# - pal-e-app (redirects, expected non-2xx on /). Good -- stale comments were cleaned up alongside the code.Relationship to issues #411 and #412: The issue list shows open issues #411 ("Remove deprecated pal-e-app references from Terraform") and #412 ("Remove deprecated westside-admin references from Terraform"). This PR appears to address the substance of both. Consider closing those issues as well if this PR fully covers their scope, or note in their comments that #449/#450 subsumes them.
Unused Woodpecker repo secrets: The Review Checklist notes "Confirm Woodpecker repo secrets can be left in place (unused secrets are harmless)." This is correct -- unused secrets in Woodpecker settings do not cause errors. However, for hygiene, consider cleaning them up in a follow-up to avoid confusion.
SOP COMPLIANCE
fix: remove stale basketball-api and pal-e-app namespace references)fix/remove-stale-namespace-refs)Closes #449PROCESS OBSERVATIONS
tofu state rmis not run before merge, which the PR body explicitly calls out.tofu state rmfor all removed resources before merging" is a hard prerequisite. If state rm is not performed, the first apply after merge will attempt to destroy resources in non-existent namespaces and fail.VERDICT: APPROVED
PR #450 Review
Parent issue: #449
Scope: Remove all Terraform references to deleted
basketball-apiandpal-e-appk8s namespaces (13 files, +3/-1401)DOMAIN REVIEW
Stack: Terraform (HCL), Woodpecker CI YAML, Makefile, Grafana dashboard JSON
Deletion scope verified correct. Cross-referenced every removed resource against the codebase. The PR removes:
basketball_api,pal_e_production(pal-e-app),westside_admin-- all correct.westside-adminis not a standalone namespace; it only consumed a mirrored secret frombasketball-apiresources, so removing it here is correct.paledocs_db_url(deployed to pal-e-app ns),admin_app_db_url(basketball-api ns),admin_app_db_url_westside_admin(mirror) -- all scoped to deleted namespaces.admin_app_user_provision-- provisioned a Postgres role for basketball-api. Correct to remove.fromrules. Removed the entirenetpol_basketball_apiresource. Other valid entries (tailscale, monitoring, pal-e-docs, westside-contracts, westside-ai-assistant) are untouched in remaining policies.embedding_worker_metricsservice (pal-e-app namespace) and itspal_e_production_namespacevariable.paledocs_db_passwordandadmin_app_db_passwordat root and module levels, plus CI pipeline secret wiring.totargets are also being deleted in this PR. Safe to remove -- Terraform state migration already completed on prior applies; themovedblocks were only needed for the initial state rename.paledocs_db_passwordremoval is correct. This variable was ONLY used to construct thepaledocs-db-urlsecret deployed into thepal-e-appnamespace. Thepal-e-docsservice (separate namespace, still live) does NOT use this variable -- it connects to postgres through its own CNPG-managed credentials. No dangling dependency.No dangling references found. After applying this diff, no remaining Terraform code references
basketball-api,pal-e-app,admin_app_db_password,paledocs_db_password, orpal_e_production_namespace.State management: The commit message correctly notes "State rm for orphaned resources must be run separately before merge." This is critical --
tofu state rmmust be executed for all removed resources beforetofu apply, otherwise Terraform will attempt to destroy resources in non-existent namespaces and fail. Ensure this is documented in the test plan.BLOCKERS
None.
No new functionality (pure deletion), no user input handling, no secrets in code, no auth logic changes. All deletion targets are scoped to confirmed-dead namespaces.
NITS
secrets.auto.tfvars.examplenot updated. The example file (line 36:paledocs_db_password, line 41:admin_app_db_password) still lists both removed variables. While.tfvarsfiles are gitignored and the example is just a template, it should be updated so new contributors don't set variables that no longer exist. Low priority since it won't break anything.PR body does not follow the repo's PR template. The repo has a PR template at
.github/PULL_REQUEST_TEMPLATE.mdrequiring## Summary,## Discovered Scope,## Terraform Changes(withtofu planoutput,tofu fmt/tofu validatecheckboxes),## README Impact, and## Review Checklist. The PR currently has only a commit message as its description. Thetofu planoutput is especially valuable here to confirm the destroy plan matches expectations.SOP COMPLIANCE
## Summary,## Terraform Changeswith plan output,## Review Checklist-- all missing)tofu planoutput not included (important for deletion PRs to verify destroy targets)PROCESS OBSERVATIONS
tofu state rm), not in the code changes themselves.VERDICT: APPROVED
The code changes are correct, complete within their stated scope, and introduce no regressions. The two nits (example tfvars file, PR body template) are non-blocking. The critical operational step (state rm) is acknowledged in the commit message. Approve contingent on the state rm being executed before apply.