pal-e-docs

Review: CI pipeline targeted apply (depends on #197)

review-437-2026-03-27 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type header -- Feature
  • [x] Lineage -- Sub-ticket of #197
  • [x] Repo -- forgejo_admin/pal-e-platform
  • [x] User Story -- platform operator, targeted applies
  • [x] Context -- explains monolith problem, 328-line pipeline
  • [x] File Targets -- .woodpecker.yaml (modify), modules/ and main.tf (do not touch)
  • [x] Acceptance Criteria -- 6 criteria
  • [x] Test Expectations -- 3 scenarios + pipeline log verification
  • [x] Constraints -- dependency on #197, preserve existing CI, -lock=false
  • [x] Checklist -- standard 3-item
  • [x] Related -- #197, #196, project-pal-e-platform

Traceability

  • [x] story:superuser-deploy label -- present on board item #437
  • [x] arch:ci-pipeline label -- present on board item #437
  • [x] arch:terraform label -- present on board item #437
  • [x] Forgejo issue -- #198, open, well-formed

File Targets

  • [x] .woodpecker.yaml -- verified: exists, 327 lines, contains plan step (line 29), apply step (line 121), cross-pillar-review step (line 219). All three steps need module-aware logic.
  • [x] terraform/modules/ -- verified: 9 modules exist (ci, database, forgejo, harbor, keycloak, monitoring, networking, ops, storage). Do-not-touch confirmed.
  • [x] terraform/main.tf -- verified: 509 lines, root orchestrator with module composition + moved{} blocks. Do-not-touch confirmed.
  • [x] terraform/variables.tf, terraform/providers.tf -- verified: exist. Fallback trigger files confirmed.

Repo Placement

Correct. Issue #198 is filed on forgejo_admin/pal-e-platform, which owns both .woodpecker.yaml and the terraform/ directory. Single-repo change.

Dependencies

  • [x] #197 (Terraform state splitting) -- CLOSED. Board item #436 is in done. All 9 modules exist on disk. Dependency satisfied.
  • [x] #196 (MinIO blocking applies) -- Referenced as root symptom. In todo column (board item #435). This ticket is the permanent fix path; #196 documents the symptom. No hard dependency.
  • #184 (Harbor connectivity timeout) -- In in_progress. Not a dependency but shares the arch:ci-pipeline label. Concurrent changes to .woodpecker.yaml could cause merge conflicts.

Acceptance Criteria

Partially testable, but has a correctness gap.
  • [x] "Detects which terraform/modules/X/ changed" -- testable via git diff in CI
  • [x] "Runs tofu apply -target=module.X" -- testable via pipeline logs
  • [x] "Falls back to full apply if root files changed" -- testable
  • [x] "Plan step similarly targets changed modules" -- testable
  • [x] "Kubeconfig, secrets, lock retry preserved" -- testable
  • [ ] "No regression on existing CI behavior" -- vague. Should specify: plan comments still post, lock retry still works, cross-pillar-review still triggers, IPv6 disable still runs.

Blast Radius

CRITICAL FINDING: Inter-module dependency graph undermines isolated applies.
The ticket assumes tofu apply -target=module.X will only refresh module X's provider. This is not how Terraform -target works with cross-module references. Examining main.tf:
  • module.networking consumes outputs from 6 other modules (monitoring, forgejo, ci, harbor, storage, keycloak)
  • module.ci has depends_on = [module.forgejo, module.database] and uses module.storage outputs
  • module.database uses module.storage outputs
  • module.ops uses module.storage and module.database outputs, has depends_on = [module.storage]
When you run tofu apply -target=module.ci, Terraform will still refresh module.storage (for cnpg_iam outputs), module.forgejo, and module.database. If MinIO is down, -target=module.ci will still fail because it must refresh module.storage to resolve the dependency.
The only modules that can be truly isolated are leaf modules with no cross-module inputs: module.monitoring, module.forgejo, module.keycloak. All others pull in the storage/database dependency chain.
This does not invalidate the ticket -- targeted applies still provide value (faster plans, smaller blast radius on actual changes). But the user story's promise ("a MinIO hiccup doesn't block a Woodpecker Helm change") is only partially fulfilled by -target alone. The ticket should acknowledge this limitation.
No sibling repo blast radius. This is the only repo with Terraform CI. The cross-pillar-review step will auto-generate a review issue on merge.

Recommendation

Two issues must be resolved before moving to next_up:
  • Add a constraint or context paragraph acknowledging that -target=module.X does NOT fully isolate provider connectivity due to cross-module output references. The agent must understand this to avoid writing incorrect fallback logic or making false promises in PR descriptions. Specifically: targeting module.ci will still refresh module.storage, module.forgejo, and module.database.
  • Sharpen "no regression" acceptance criterion into specific testable items: plan comments post correctly, lock retry logic works, cross-pillar-review step triggers, IPv6 disable runs in all steps, all 15+ secret env vars preserved.
No decomposition needed -- single file target, single agent pass. But the agent needs accurate mental model of what -target actually does.