Review: Harbor pull creds use stale westside-ror robot account after rename to westside-basketball

review-1613-2026-06-25 Review

review ready

Verdict: READY

Re-review after refinements addressing 5 issues from initial NEEDS_REFINEMENT verdict. All 5 issues resolved.

Previous Issues Resolution

  • [x] pal-e-deployments overlay added to scope -- issue body now lists "In scope (pal-e-deployments)" with overlay path update
  • [x] State management approach documented -- issue body now includes "State management approach" section with moved blocks strategy and tofu plan verification
  • [x] keycloak_clients map key added to scope -- issue body now lists keycloak_clients rename with corresponding moved block in keycloak.tf
  • [x] arch-terraform note -- referenced in Related section. Note does not yet exist in pal-e-docs (search returns empty), but the issue body references it. Acceptable: creating the arch note is a separate documentation task, not a blocker for this bug fix.
  • [x] story label changed to WS-S1 -- board item now shows story:WS-S1 (superadmin IaC reproducibility), correct alignment for an infrastructure bug

Template Completeness

  • [x] Type -- Bug
  • [x] Lineage -- Regression from westside-ror rename
  • [x] Repo -- ldraney/pal-e-services (primary), ldraney/pal-e-deployments (overlay fix)
  • [x] What Broke -- ImagePullBackOff with 401 Unauthorized, root cause identified
  • [x] Repro Steps -- 5-step reproduction
  • [x] Expected Behavior -- New pod should pull successfully
  • [x] Environment -- Cluster, namespace, pull secret username, Terraform key
  • [x] Scope -- Detailed scope section covering both repos, with state management approach
  • [x] Acceptance Criteria -- 6 criteria listed
  • [x] Related -- project, parent issue, westside-rename note, arch-terraform reference

Traceability

  • [x] story:WS-S1 label -- "As superadmin, I want to deploy platform changes via IaC so that infrastructure is reproducible and auditable"
  • [x] story note verified -- found in project-westside-basketball user-stories section (Superadmin block)
  • [x] arch:terraform label -- terraform component
  • [ ] arch note NOT YET CREATED -- arch-terraform note does not exist in pal-e-docs. However, this is a documentation gap, not a scope blocker for a 2pt bug fix. The issue body correctly references it in Related.
  • [x] Forgejo issue -- ldraney/pal-e-services#148, open

File Targets

  • [x] terraform/k3s.tfvars services map -- verified: key is westside-ror at line 336 (in .example), with image_repo = "westside-ror/app", forgejo_repo = "ldraney/westside-ror", source_path = "overlays/westside-ror/prod". All need updating to westside-basketball.
  • [x] terraform/k3s.tfvars keycloak_clients map -- verified: key is westside-ror at line 152 (in .example) with client_id = "westside-ror". Needs key rename + moved block.
  • [x] terraform/services.tf -- verified: 6 resources iterate var.services (harbor_project, service_ci, service_pull, kubernetes_namespace_v1, harbor_creds, argocd_application). Note: service_funnel is filtered by v.funnel and westside-ror has funnel = false, so only 6 moved blocks needed in services.tf, not 7.
  • [x] terraform/keycloak.tf -- verified: existing moved block precedent (westside-app -> westside-landing). 2 resources need moved blocks: keycloak_openid_client, keycloak_openid_user_realm_role_protocol_mapper (since include_realm_roles_mapper = true).
  • [x] overlays/westside-ror/prod/kustomization.yaml (pal-e-deployments) -- verified: image reference already updated to harbor.tail5b443a.ts.net/westside-basketball/app. The overlay does NOT remove imagePullSecrets (base deployment includes them). Directory rename is the main change needed.

Repo Placement

Correct. Primary fix in ldraney/pal-e-services (Terraform). Secondary fix in ldraney/pal-e-deployments (overlay directory rename). Both repos identified in issue body. Single Forgejo issue is appropriate since the pal-e-deployments change is a simple directory rename.

Dependencies

  • Board item #1614 (TLS handshake bug) is a sibling sprint:5 unplanned bug -- independent fix, no blocker.
  • This ticket blocks Sprint 4+ deployments. Correctly references westside-basketball#54 (Keycloak auth, merged but not deployed).
  • No upstream dependencies. No depends: label needed.

Acceptance Criteria

6 criteria listed. Assessment:
  • [x] AC1: "tofu plan shows 0 destroy" -- verifiable by running tofu plan and checking output
  • [x] AC2: "Harbor pull secret uses correct robot account" -- verifiable via kubectl get secret
  • [x] AC3: "New pod pulls successfully" -- verifiable via kubectl get pods
  • [x] AC4: "Sprint 4 auth changes are live" -- verifiable by testing Keycloak login flow
  • [x] AC5: "keycloak_clients map key updated without disrupting existing client" -- verifiable via tofu plan + Keycloak admin console
  • [x] AC6: "No regression in other service deployments" -- verifiable by checking all other service pods
All criteria are agent-verifiable. No missing criteria.

Blast Radius

  • The moved blocks strategy mitigates destroy/recreate risk. With correct moved blocks, tofu plan should show only in-place updates.
  • Keycloak client rename has precedent (westside-app -> westside-landing moved blocks already exist in keycloak.tf).
  • No other services affected -- all other service keys match their image_repo prefixes.
  • Minor detail: the issue body says "moved blocks in services.tf, harbor.tf" but harbor.tf does not exist as a separate file -- Harbor resources live in services.tf. The issue should say "services.tf and keycloak.tf" for moved blocks. This is cosmetic and does not affect implementation correctness since the agent will find the resources in services.tf regardless.

Decomposition Assessment

4 files across 2 repos, 6 AC. Borderline on the 5-minute rule. However, the core fix is a single logical operation (rename map keys + add moved blocks), and the pal-e-deployments change is a trivial directory rename. The tofu plan verification is the gating step. No decomposition needed -- this is a cohesive 2pt bug fix that an agent can complete in one pass.

Recommendations

  • [BODY] Minor: issue body says "moved blocks in services.tf, harbor.tf" but harbor.tf does not contain service resources. Should say "services.tf and keycloak.tf". Non-blocking -- agent will find the correct files.
  • [BODY] Minor: issue body does not explicitly mention updating forgejo_repo from ldraney/westside-ror to ldraney/westside-basketball or source_path from overlays/westside-ror/prod to overlays/westside-basketball/prod in the services map. These are implicit in "rename services map key" but worth calling out. Non-blocking.
Neither recommendation is blocking. The scope is clear enough for an agent to implement correctly.