pal-e-docs

Review: Migrate westside-admin harbor-creds SOPS to terraform (Option A)

review-1120-2026-05-02 Review

review approved
Project

Verdict: APPROVED

Note: Both READY and APPROVED are accepted as passing verdicts by the check-board-advance hook.
This is the FIRST review under refined Option A scope (post-architectural reframing 2026-05-02). The original "rotate the placeholder" framing was scoped out; the refined scope is the correct unit of work. Minor [BODY] polish recommended before tofu apply runs, but nothing blocks the PR from being cut.

Template Completeness

  • [x] ### Type — Bug
  • [x] ### Lineage — links validation note + parent #144 + layered debugging history
  • [x] ### Repo — pal-e-deployments (overlay) + coordinated pal-e-services (terraform)
  • [x] ### What Broke — placeholder string, base64 error, dual-IaC root cause explained
  • [x] ### Repro Steps — three concrete commands (kubectl pod state, secret decode, Harbor API)
  • [x] ### Expected Behavior — TWO coordinated changes with explicit sequence
  • [x] ### Environment — service, cluster, Harbor project id, target robot name, refs
  • [x] ### Acceptance Criteria — 8 items, mix of file-presence + Harbor API + cluster state + funnel
  • [x] ### Constraint — secrets path, -target= scoping, -lock=false convention reference
  • [x] ### Related — parent #144, sibling layers (#137, westside-admin#13), validation note, sop-harbor-robot-import
  • [x] ### Scope — narrow (westside-admin only), explicit out-of-scope

Traceability

  • [x] story:admin-row-crud label — verified entry in project-westside-admin user-stories table (story note story-westside-admin-admin-row-crud)
  • [x] arch:harbor label — verified row "Harbor registry" in arch-deployment-westside-admin Components table (column "arch: label" = arch:harbor)
  • [x] type:bug label — matches refined Type (Bug) in body
  • [x] blocks:deploy-chain label — accurate; this is the LAST blocker on westside-admin's M1 bootstrap arc
  • [x] Forgejo issue — forgejo_admin/pal-e-deployments#143, state: open
  • [x] Architectural parent forgejo_admin/pal-e-deployments#144 — verified open, body explicitly names westside-admin as "first concrete consumer" with #143 as the migration ticket

File Targets

All paths verified against origin/main of pal-e-deployments and the live ~/pal-e-services/terraform/ tree:
  • [x] overlays/westside-admin/prod/kustomization.yaml — line 8 of resources: list contains - harbor-creds.enc.yaml. Confirmed.
  • [x] overlays/westside-admin/prod/harbor-creds.enc.yaml — file exists, SOPS-encrypted with age recipient age15ct78fr4scv4vxzj3k6q76wshywzlu0mdc64a624e264dst7zfaq6tjzjr, lastmodified 2026-04-25. Confirmed.
  • [x] ~/pal-e-services/terraform/services.tf — resource "kubernetes_secret_v1" "harbor_creds" at lines 94-115 with for_each = var.services. Resource composes auths from harbor_robot_account.service_pull[each.key].secret. Confirmed.
  • [x] ~/pal-e-services/terraform/services.tf — resource "harbor_robot_account" "service_pull" at lines 57-77, name "${each.key}-pull", project-scoped. Resolves to robot robot$westside-admin+westside-admin-pull. Matches issue body.
  • [x] ~/pal-e-services/terraform/k3s.tfvars — services map at lines 199-308 includes westside-admin at lines 288-295 (forgejo_repo, image_repo=westside-admin/app, port 3000, funnel=false, source_path=overlays/westside-admin/prod). Confirmed. funnel=false is correct because the overlay manages its own ingress (ingress.yaml present).
  • [x] ~/secrets/pal-e-services/secrets.env — HARBOR_ADMIN_PASSWORD present (also duplicated in k3s.tfvars line 2 — terraform reads from tfvars).

Repo Placement

OK. The overlay PR is correctly filed against forgejo_admin/pal-e-deployments. The terraform apply step is correctly framed as a separate operator action against forgejo_admin/pal-e-services (no PR needed for the apply itself — the .tf code is already on main and unchanged; this is a state-only operation). No multi-repo coordination required beyond the documented sequence.

Dependencies

  • Architectural parent: forgejo_admin/pal-e-deployments#144 — open, lists westside-admin as first migration. This ticket is documented as the proof-of-pattern; lessons feed back to #144.
  • Layered prerequisites already done: layer-1 (westside-admin#13, image tag 40-zeros fix) and layer-2 (#137, PSA admission) are done/qa. This is layer-3, the last visible blocker.
  • No in-progress blockers: board scan shows nothing in in_progress; this ticket is the only item in backlog on board-westside-admin and is the gating M1 ticket.
  • Downstream: 13 more overlays carry harbor-creds.enc.yaml (basketball-api, gcal-scheduler, mcd-tracker, mcd-tracker-app, pal-e-app, pal-e-docs, pal-e-mail, pal-e-production, platform-validation, twitch-2k-wager, westside-ai-assistant, westside-streamlit, westsidekingsandqueens). Out-of-scope per Option A; tracked under #144.
  • No CI dependency: ~/pal-e-services/ has no .woodpecker.yaml or other CI config. The tofu apply is unambiguously a manual operator step. The issue body's "Terraform state access (likely remote backend; verify before apply)" is slightly imprecise — current state is LOCAL (per pal-e-platform/CLAUDE.md: "Local terraform state for now. Remote backend is a post-bootstrap concern."). Worth tightening (see [BODY] recs below).

Acceptance Criteria

8 AC items, all testable by an agent with kubectl + curl + Harbor admin creds + ArgoCD CLI. Strong coverage on outcomes (Harbor API for robot existence, dockerconfigjson parses, deployment 1/1 ready, funnel responds 200). Two minor gaps (called out below as [BODY] polish, not blockers):
  • No explicit AC for "no-drift verification" — i.e., tofu plan -target='harbor_robot_account.service_pull["westside-admin"]' -target='kubernetes_secret_v1.harbor_creds["westside-admin"]' -lock=false returns "0 to add, 0 to change, 0 to destroy" AFTER the apply. User flagged this as item #4. Add as AC #9.
  • No explicit AC for "ArgoCD shows no OutOfSync on westside-admin Application" — verifies that the overlay change + terraform-managed Secret reach a stable steady state. Add as AC #10.

Blast Radius

  • Cluster: westside-admin pod is already in CreateContainerError. Removing the placeholder Secret cannot make it worse. Brief gap (overlay prune → tofu apply) is non-regressive. Acknowledged in body.
  • Other services: -target= on two specific keys means kubernetes_secret_v1.harbor_creds["westside-admin"] and harbor_robot_account.service_pull["westside-admin"] only. terraform's -target follows transitive dependencies, so it will ALSO read/touch harbor_project.service["westside-admin"] and kubernetes_namespace_v1.service["westside-admin"] (referenced via interpolations on lines 75 and 99). Both already exist and should plan as no-op. Other services' state is not touched. Acceptable — but worth a [BODY] note that operator should review the plan diff before applying.
  • Harbor: ZERO project-scoped robots currently exist (verified live: GET /api/v2.0/robots?page_size=50 returned only robot$image-updater). This will be the FIRST harbor_robot_account.service_pull to apply in the current Harbor instance. Pattern is unproven in production — that's the intentional value of doing westside-admin first under the Option A migration.
  • Robot count claim: issue body says "other 11 services migrate" but actual overlay count carrying harbor-creds.enc.yaml is 14 total (13 remaining after westside-admin). Cosmetic — does not affect this ticket's execution.

Decomposition Assessment

Within the 5-minute / <3 file targets / <5 AC heuristics:
  • File targets: 2 (kustomization.yaml edit, harbor-creds.enc.yaml deletion) — under the 3-file ceiling.
  • AC items: 8 (the heuristic ceiling is 5, but 6 of these are post-merge cluster verifications that the validate skill will run automatically, not implementation work).
  • Implementation work for the dev agent is exactly 2 file changes in one repo. Estimated 2-3 minutes including PR scaffold.
  • Operator step (tofu apply) is a separate action, properly sequenced post-merge.
No decomposition needed.

Recommendation

APPROVED. Move 1120 to todo. The four [BODY] polish items below are nits — not blockers — and can be appended as a comment on #143 or absorbed during dev/validate without forcing a re-review:
  • [BODY] Add AC #9: "tofu plan -target='harbor_robot_account.service_pull[\"westside-admin\"]' -target='kubernetes_secret_v1.harbor_creds[\"westside-admin\"]' -lock=false after the apply returns 0 changes (no drift)."
  • [BODY] Add AC #10: "ArgoCD shows Application westside-admin as Synced with no OutOfSync resources after both changes settle."
  • [BODY] Constraint section: replace "Terraform state access (likely remote backend; verify before apply)" with "Terraform state is LOCAL (per pal-e-platform/CLAUDE.md); operator runs tofu apply from ~/pal-e-services/terraform/ with the local state file."
  • [BODY] Add to Constraint: "Operator must review the tofu plan diff before applying — -target= follows transitive dependencies, so harbor_project.service[\"westside-admin\"] and kubernetes_namespace_v1.service[\"westside-admin\"] will also appear in the plan (both already exist; expect no-op). Abort if anything else changes."
No [LABEL], [SCOPE], or [DECOMPOSE] recommendations.

Architectural Concerns Evaluated (per request)

  • SOPS-prune-before-tofu-apply gap: NOT a regression. Pod is already broken with CreateContainerError; removing the placeholder Secret does not change that. Acknowledged in body. No action needed.
  • Unexpected terraform state touches: -target follows transitive deps. The two named resources pull in harbor_project.service["westside-admin"] and kubernetes_namespace_v1.service["westside-admin"]. Both exist and should plan as no-op. Other for_each keys are NOT re-evaluated by -target (that's the whole point of the flag). Acceptable; covered by [BODY] rec to review plan before apply.
  • Per-overlay PR vs. separate tofu apply ticket: I considered whether the operator-run tofu apply needs its own Forgejo issue. Recommendation: NO — keep it as part of #143's expected behavior. Reasons: (a) the apply has no code artifact, only a state transition; (b) splitting it would introduce a coordination handoff that doubles the chance the sequence is mis-ordered; (c) the validate-ticket skill can verify both changes in one pass. If we want a paper trail of WHO ran the apply and when, that belongs in a comment on #143 or a one-line entry in validation-143-{date}, not a separate ticket.

Live Evidence Captured

  • git ls-tree origin/main overlays/westside-admin/prod/: kustomization.yaml, harbor-creds.enc.yaml, deployment-patch.yaml, ingress.yaml, namespace.yaml, westside-admin-secrets.enc.yaml all present.
  • GET https://harbor.tail5b443a.ts.net/api/v2.0/robots?page_size=50: returns 1 robot (robot$image-updater). ZERO project-scoped robots exist. ?q=name%3D~westside-admin: 0 results. Confirms terraform's harbor_robot_account.service_pull["westside-admin"] has never successfully applied.
  • cat ~/secrets/pal-e-services/secrets.env | grep HARBOR_ADMIN_PASSWORD: present.
  • find ~/pal-e-services -name ".woodpecker*": nothing. tofu apply is manual.
  • Parent #144 body: explicitly names #143 as "first concrete consumer" and "first overlay to migrate."