pal-e-docs

Review: Terraform state splitting -- modularize main.tf

review-436-2026-03-26 Review

review ready
Project

Verdict: READY

Final re-review (2026-03-26): The issue body for #197 has been fully rewritten to incorporate all 6 refinement items from the previous review. Sub-ticket #198 has been created and is tracked on the board. This ticket is agent-ready.

Refinement Resolution

# Original Concern Resolution Status
1 4 missing file targets (network-policies.tf, outputs.tf, providers.tf, versions.tf) All 4 added to "Files to modify" with accurate descriptions RESOLVED in body
2 ~25 orphaned resources not mapped to 8 modules 9th "ops" module added to architecture diagram, file targets, and AC count RESOLVED in body
3 CI pipeline detection logic non-trivial .woodpecker.yaml moved to "Files NOT to touch." Sub-ticket #198 created, board item #437 exists in backlog RESOLVED in body + sub-ticket
4 81 moved blocks = high risk for zero-downtime AC added: "All 81 moved blocks enumerated and verified." Constraint added: "Agent must generate full moved manifest before relocation" RESOLVED in body
5 Missing AC for network-policies.tf and outputs.tf 3 new ACs added: namespace refs, output addresses, 0 add/destroy RESOLVED in body
6 Provider count (5 vs 4) Corrected to "4 providers (kubernetes, helm, tailscale, minio)" in both Context and Architecture RESOLVED in body

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- references #184 incident, #196 symptom
  • [x] Repo -- forgejo_admin/pal-e-platform
  • [x] User Story -- two well-formed stories (isolated deploys, blast radius containment)
  • [x] Context -- clear problem statement with incident evidence
  • [x] Architecture -- current vs target diagram with migration path (3 steps)
  • [x] File Targets -- 9 module sets to create, 6 files to modify, 3 explicit exclusions
  • [x] Acceptance Criteria -- 11 ACs, all testable and verifiable
  • [x] Test Expectations -- 5 items including run command
  • [x] Constraints -- 8 constraints including lock=false and moved-block manifest
  • [x] Checklist -- 5 items including sub-ticket #198 confirmation
  • [x] Related -- #196, #194, #198, #191, project-pal-e-platform

Traceability

  • [x] story:superuser-deploy label -- platform operator isolated deploys
  • [x] story:platform-reliability label -- blast radius containment
  • [x] arch:terraform label -- maps to terraform control plane
  • [x] Forgejo issue -- #197, open
Traceability triangle complete.

File Targets

  • [x] terraform/main.tf -- verified: exists, 2551 lines (issue says ~2200, acceptable approximation)
  • [x] terraform/variables.tf -- verified: exists, 167 lines
  • [x] terraform/network-policies.tf -- verified: exists, 228 lines, now in body
  • [x] terraform/outputs.tf -- verified: exists, 54 lines (11 outputs), now in body
  • [x] terraform/providers.tf -- verified: exists, 21 lines (4 providers), now in body
  • [x] terraform/versions.tf -- verified: exists, 28 lines, now in body
  • [x] terraform/modules/ -- does NOT exist yet (to create), correct
  • [x] .woodpecker.yaml -- correctly moved to "Files NOT to touch" (deferred to #198)
  • [x] salt/ -- correctly excluded
  • [x] terraform/k3s.tfvars / secrets.auto.tfvars -- correctly excluded

Repo Placement

OK. All targets within forgejo_admin/pal-e-platform. No cross-repo concerns.

Dependencies

  • #196 (MinIO blocking Helm applies) -- open, symptom ticket. No hard dependency.
  • #194 (Bump agent parallel workflows) -- in_progress on board. No hard dependency.
  • #191 (Agent label routing) -- in_progress on board. Same Helm values block moves to modules/ci/. No blocker.
  • #198 (CI pipeline targeted apply) -- open, sub-ticket. Depends on this ticket. Tracked on board as #437.

Acceptance Criteria

All 11 ACs are well-formed, testable, and verifiable by an executing agent:
  • [x] 9 modules created (ci, storage, monitoring, forgejo, keycloak, harbor, networking, database, ops)
  • [x] Each module self-contained with main.tf, variables.tf, outputs.tf
  • [x] All 81 moved blocks enumerated and verified -- tofu plan shows 0 add/0 destroy
  • [x] Targeted plan for ci module only refreshes CI resources
  • [x] Targeted plan for storage module only refreshes MinIO resources
  • [x] Targeted apply for ci succeeds even when MinIO is unreachable
  • [x] Full apply (no target) still works
  • [x] Network policies reference correct namespaces post-migration
  • [x] All 11 outputs resolve to correct module resource addresses
  • [x] tofu validate passes
  • [x] tofu plan shows 0 changes after migration

Blast Radius

  • Dense cross-module dependencies (tailscale_operator referenced by 6+ modules) remain the biggest implementation risk.
  • Monitoring as dependency hub (kube-prometheus-stack referenced by ~12 resources).
  • 81 moved blocks -- now mitigated by AC requiring full enumeration before relocation.
  • pal-e-services confirmed safe (no remote_state references).

Recommendation

No action needed. All 6 refinement items have been incorporated into the issue body. Sub-ticket #198 is created and tracked. This ticket is ready to move from todo to next_up.