forgejo_admin/pal-e-platform

Fork 1

Create Terraform-managed Postgres DB secret for pal-e-docs namespace #22

New issue

Closed

opened 2026-03-06 17:12:40 +00:00 by forgejo_admin · 2 comments

forgejo_admin commented

2026-03-06 17:12:40 +00:00

Owner

Plan

plan-2026-02-26-tf-modularize-postgres -- Phase 3, Part A (traceability only)

Repo

forgejo_admin/pal-e-platform

User Story

As a platform operator
I want the pal-e-docs Postgres connection string managed by Terraform
So that the app can connect to the CNPG cluster without manual kubectl secrets

Context

A CNPG Postgres cluster (pal-e-postgres) is running in the postgres namespace with a paledocs database and user. The pal-e-docs app runs in the pal-e-docs namespace and needs a secret containing the DATABASE_URL to connect cross-namespace.

This follows the established pattern: Terraform creates kubernetes_secret_v1 resources in app namespaces, just like harbor-creds in services.tf and cnpg-s3-creds in main.tf.

The existing credentials (already in the cluster):

Secret: paledocs-db-credentials in postgres namespace
Username: paledocs
Connection endpoint: pal-e-postgres-rw.postgres.svc.cluster.local:5432
Database: paledocs

File Targets

Files to modify:

terraform/variables.tf -- add paledocs_db_password variable (sensitive string)
terraform/main.tf -- add kubernetes_secret_v1.paledocs_db_url resource

Files NOT to touch:

terraform/k3s.tfvars -- gitignored, Lucas adds the value manually
Anything in salt/ -- not in scope

Acceptance Criteria

terraform/variables.tf has a new variable "paledocs_db_password" with sensitive = true and type = string
terraform/main.tf has a new kubernetes_secret_v1.paledocs_db_url resource that:
- Lives in the pal-e-docs namespace (reference existing namespace data source or hardcode "pal-e-docs")
- Has key DATABASE_URL with value postgresql://paledocs:${var.paledocs_db_password}@pal-e-postgres-rw.postgres.svc.cluster.local:5432/paledocs
tofu validate passes
tofu plan shows only the new secret resource being created (no unrelated drift)

Test Expectations

tofu fmt -check passes
tofu validate passes
tofu plan output included in PR description showing the new resource

Constraints

Follow the existing pattern in main.tf for kubernetes_secret_v1 resources (see cnpg_s3_creds around line 1086)
Use set_sensitive pattern if the value appears in plan output -- see type = "string" lesson in CLAUDE.md
Place the new resource in the CNPG section of main.tf (after the existing CNPG resources, before the next section)
The pal-e-docs namespace already exists (created by pal-e-services Terraform). Do NOT create it. Use a hardcoded namespace string "pal-e-docs".

Checklist

PR opened
tofu plan output in PR description
No unrelated changes

pal-e-docs project -- this secret enables the Postgres migration
Phase 3 Part B (pal-e-docs issue) depends on this being applied first

### Plan `plan-2026-02-26-tf-modularize-postgres` -- Phase 3, Part A (traceability only) ### Repo `forgejo_admin/pal-e-platform` ### User Story As a platform operator I want the pal-e-docs Postgres connection string managed by Terraform So that the app can connect to the CNPG cluster without manual kubectl secrets ### Context A CNPG Postgres cluster (`pal-e-postgres`) is running in the `postgres` namespace with a `paledocs` database and user. The pal-e-docs app runs in the `pal-e-docs` namespace and needs a secret containing the DATABASE_URL to connect cross-namespace. This follows the established pattern: Terraform creates `kubernetes_secret_v1` resources in app namespaces, just like `harbor-creds` in `services.tf` and `cnpg-s3-creds` in `main.tf`. The existing credentials (already in the cluster): - Secret: `paledocs-db-credentials` in `postgres` namespace - Username: `paledocs` - Connection endpoint: `pal-e-postgres-rw.postgres.svc.cluster.local:5432` - Database: `paledocs` ### File Targets Files to modify: - `terraform/variables.tf` -- add `paledocs_db_password` variable (sensitive string) - `terraform/main.tf` -- add `kubernetes_secret_v1.paledocs_db_url` resource Files NOT to touch: - `terraform/k3s.tfvars` -- gitignored, Lucas adds the value manually - Anything in `salt/` -- not in scope ### Acceptance Criteria - [ ] `terraform/variables.tf` has a new `variable "paledocs_db_password"` with `sensitive = true` and `type = string` - [ ] `terraform/main.tf` has a new `kubernetes_secret_v1.paledocs_db_url` resource that: - Lives in the `pal-e-docs` namespace (reference existing namespace data source or hardcode `"pal-e-docs"`) - Has key `DATABASE_URL` with value `postgresql://paledocs:${var.paledocs_db_password}@pal-e-postgres-rw.postgres.svc.cluster.local:5432/paledocs` - [ ] `tofu validate` passes - [ ] `tofu plan` shows only the new secret resource being created (no unrelated drift) ### Test Expectations - [ ] `tofu fmt -check` passes - [ ] `tofu validate` passes - [ ] `tofu plan` output included in PR description showing the new resource ### Constraints - Follow the existing pattern in `main.tf` for `kubernetes_secret_v1` resources (see `cnpg_s3_creds` around line 1086) - Use `set_sensitive` pattern if the value appears in plan output -- see `type = "string"` lesson in CLAUDE.md - Place the new resource in the CNPG section of `main.tf` (after the existing CNPG resources, before the next section) - The `pal-e-docs` namespace already exists (created by pal-e-services Terraform). Do NOT create it. Use a hardcoded namespace string `"pal-e-docs"`. ### Checklist - [ ] PR opened - [ ] `tofu plan` output in PR description - [ ] No unrelated changes ### Related - `pal-e-docs` project -- this secret enables the Postgres migration - Phase 3 Part B (pal-e-docs issue) depends on this being applied first

forgejo_admin referenced this issue from a commit

2026-03-06 17:16:31 +00:00

Add Terraform-managed Postgres DB secret for pal-e-docs namespace

forgejo_admin referenced this issue

2026-03-06 17:17:09 +00:00

Add Terraform-managed Postgres DB secret for pal-e-docs namespace #23

forgejo_admin commented

2026-03-06 17:21:33 +00:00

Author

Owner

PR #23 Review

BLOCKERS

1. Namespace pal-e-docs does not exist in Terraform state and is hardcoded as a string literal.

The new resource at /home/ldraney/pal-e-platform/terraform/main.tf (line ~1100 in the branch) uses:

namespace = "pal-e-docs"

This namespace is not created anywhere in this codebase. Every other secret in main.tf references a Terraform-managed namespace via kubernetes_namespace_v1.<name>.metadata[0].name (e.g., cnpg_s3_creds uses kubernetes_namespace_v1.postgres.metadata[0].name). The PR body claims it "follows the established pattern used by cnpg_s3_creds" but the namespace handling does not follow that pattern.

If the namespace does not already exist in the cluster, tofu apply will fail. If it does exist (created outside Terraform), this is still an inconsistency -- all other namespaces are Terraform-managed. Either:

(a) Add a kubernetes_namespace_v1.paledocs resource and reference it, or
(b) If the namespace is intentionally managed outside this repo (e.g., by pal-e-services), document that explicitly and add a comment in the code explaining why this is a hardcoded string.

This is a blocker because it will either fail on apply or silently create a pattern inconsistency without justification.

NITS

1. No password length validation. Other password variables in variables.tf include validation blocks (e.g., harbor_admin_password requires >= 8 chars, minio_root_password requires >= 8 chars). The new paledocs_db_password variable has no validation. Consider adding a minimum length constraint for consistency and to catch empty/short passwords early.

2. DATABASE_URL string contains hardcoded hostname. The connection string hardcodes pal-e-postgres-rw.postgres.svc.cluster.local:5432. This is correct for the current CNPG cluster setup, but if the service name or namespace ever changes, this string will silently break. A comment noting the dependency on the CNPG cluster service name would help future maintainers. Non-blocking.

SOP COMPLIANCE

Branch named after issue (22-paledocs-db-secret references issue #22)
PR body has: Summary, Changes, Test Plan, Related
Related section references the plan slug (plan-2026-02-26-tf-modularize-postgres)
No secrets committed (password passed via variable, marked sensitive = true)
No unnecessary file changes (2 files, 19 additions, 0 deletions -- tightly scoped)
Commit messages are descriptive (PR title matches scope)
tofu plan output included in PR body

VERDICT: NOT APPROVED

One blocker: the pal-e-docs namespace is not Terraform-managed in this repo and the hardcoded string breaks the established pattern without explanation. Resolve the namespace question (create it in Terraform or document why it is external) before merge.

## PR #23 Review ### BLOCKERS **1. Namespace `pal-e-docs` does not exist in Terraform state and is hardcoded as a string literal.** The new resource at `/home/ldraney/pal-e-platform/terraform/main.tf` (line ~1100 in the branch) uses: ```hcl namespace = "pal-e-docs" ``` This namespace is not created anywhere in this codebase. Every other secret in main.tf references a Terraform-managed namespace via `kubernetes_namespace_v1.<name>.metadata[0].name` (e.g., `cnpg_s3_creds` uses `kubernetes_namespace_v1.postgres.metadata[0].name`). The PR body claims it "follows the established pattern used by `cnpg_s3_creds`" but the namespace handling does not follow that pattern. If the namespace does not already exist in the cluster, `tofu apply` will fail. If it does exist (created outside Terraform), this is still an inconsistency -- all other namespaces are Terraform-managed. Either: - (a) Add a `kubernetes_namespace_v1.paledocs` resource and reference it, or - (b) If the namespace is intentionally managed outside this repo (e.g., by pal-e-services), document that explicitly and add a comment in the code explaining why this is a hardcoded string. This is a blocker because it will either fail on apply or silently create a pattern inconsistency without justification. ### NITS **1. No password length validation.** Other password variables in `variables.tf` include validation blocks (e.g., `harbor_admin_password` requires >= 8 chars, `minio_root_password` requires >= 8 chars). The new `paledocs_db_password` variable has no validation. Consider adding a minimum length constraint for consistency and to catch empty/short passwords early. **2. DATABASE_URL string contains hardcoded hostname.** The connection string hardcodes `pal-e-postgres-rw.postgres.svc.cluster.local:5432`. This is correct for the current CNPG cluster setup, but if the service name or namespace ever changes, this string will silently break. A comment noting the dependency on the CNPG cluster service name would help future maintainers. Non-blocking. ### SOP COMPLIANCE - [x] Branch named after issue (`22-paledocs-db-secret` references issue #22) - [x] PR body has: Summary, Changes, Test Plan, Related - [x] Related section references the plan slug (`plan-2026-02-26-tf-modularize-postgres`) - [x] No secrets committed (password passed via variable, marked `sensitive = true`) - [x] No unnecessary file changes (2 files, 19 additions, 0 deletions -- tightly scoped) - [x] Commit messages are descriptive (PR title matches scope) - [x] `tofu plan` output included in PR body ### VERDICT: NOT APPROVED One blocker: the `pal-e-docs` namespace is not Terraform-managed in this repo and the hardcoded string breaks the established pattern without explanation. Resolve the namespace question (create it in Terraform or document why it is external) before merge.

forgejo_admin commented

2026-03-06 17:24:14 +00:00

Author

Owner

PR #23 Review (Re-review)

BLOCKERS

None. The previously identified blocker (hardcoded namespace string) has been resolved.

Fix verification: The dev added a data "kubernetes_namespace_v1" "pal_e_docs" data source to look up the externally managed pal-e-docs namespace, and the secret resource now references it via data.kubernetes_namespace_v1.pal_e_docs.metadata[0].name. This is the correct approach -- since pal-e-docs is managed by pal-e-services (not this bootstrap repo), a data source (read-only lookup) is more appropriate than a resource (which would imply ownership). Good design decision.

NITS

The paledocs_db_password variable has no length validation, unlike harbor_admin_password and minio_root_password which enforce minimum lengths. Consider adding a minimum length constraint for consistency. Non-blocking.

SOP COMPLIANCE

Branch named after issue (22-paledocs-db-secret references issue #22)
PR body follows template (Summary, Changes, Test Plan, Related)
Related references plan slug (plan-2026-02-26-tf-modularize-postgres)
tofu plan output included in PR body
No secrets committed
No unnecessary file changes (2 files, both relevant)
Commit messages are descriptive

VERDICT: APPROVED

## PR #23 Review (Re-review) ### BLOCKERS None. The previously identified blocker (hardcoded namespace string) has been resolved. **Fix verification:** The dev added a `data "kubernetes_namespace_v1" "pal_e_docs"` data source to look up the externally managed `pal-e-docs` namespace, and the secret resource now references it via `data.kubernetes_namespace_v1.pal_e_docs.metadata[0].name`. This is the correct approach -- since `pal-e-docs` is managed by pal-e-services (not this bootstrap repo), a `data` source (read-only lookup) is more appropriate than a `resource` (which would imply ownership). Good design decision. ### NITS - The `paledocs_db_password` variable has no length validation, unlike `harbor_admin_password` and `minio_root_password` which enforce minimum lengths. Consider adding a minimum length constraint for consistency. Non-blocking. ### SOP COMPLIANCE - [x] Branch named after issue (`22-paledocs-db-secret` references issue #22) - [x] PR body follows template (Summary, Changes, Test Plan, Related) - [x] Related references plan slug (`plan-2026-02-26-tf-modularize-postgres`) - [x] tofu plan output included in PR body - [x] No secrets committed - [x] No unnecessary file changes (2 files, both relevant) - [x] Commit messages are descriptive ### VERDICT: APPROVED

forgejo_admin closed this issue

2026-03-06 17:30:35 +00:00