Skip to content

Refactor: extract shared iac.provider discovery + grouping helper used by plan and apply #530

Description

@intel352

Background

Surfaced by Copilot's review of PR #528 (W-3b): cmd/wfctl/infra_plan_provider.go::computePlanForInfraSpecs duplicates the iac.provider discovery + grouping logic in cmd/wfctl/infra_apply.go::applyInfraModules. Both walk cfg.Modules, filter by iac.provider, group resource specs by provider ref, count provider types for the fallback heuristic, etc.

Why this exists today: apply was the original; plan was added in W-3b mirroring it intentionally so the v2 dispatch path (W-3b T3.6b) could honestly emit Replace actions before apply. The duplication was accepted in-PR to keep W-3b scope-locked (per workspace memory feedback_implementer_scope_bleed).

Risk

Plan and apply can drift over time on:

  • env-var resolution rules
  • disabled-provider handling
  • grouping order
  • provider-type-counts fallback heuristic
  • error wrapping of plugin-load failures

Drift here is silent and only surfaces when an operator's plan and apply produce different action sets for the same input.

Proposed shape

Extract a shared helper in cmd/wfctl/infra_provider_dispatch.go (or similar):

// resolveProviderDefs walks cfg.Modules, filters `iac.provider`
// modules, expands env vars in cfg, and returns provType -> providerDef.
func resolveProviderDefs(cfg *engine.Config, envName string) (map[string]providerDef, error)

// groupSpecsByProviderRef walks resource specs, looks up the matching
// providerDef via `cfg.provider` field, and returns moduleRef ->
// planGroup{ moduleRef, provType, provCfg, specs[] } in stable order.
func groupSpecsByProviderRef(specs []interfaces.ResourceSpec, defs map[string]providerDef) ([]string, map[string]*planGroup, error)

// loadProvidersForGroups iterates groups in order, calls
// resolveIaCProvider per group, and returns map[moduleRef]*loadedProvider
// (with closer + error). Caller decides whether to fail-fast or
// surface partial results.
func loadProvidersForGroups(ctx context.Context, groups map[string]*planGroup) (map[string]*loadedProvider, error)

Then computePlanForInfraSpecs and applyInfraModules both call into these helpers, eliminating the duplication.

Acceptance

  • Single helper used by both plan and apply paths
  • Existing test coverage migrated (no test deletion)
  • New unit test asserts plan and apply produce the same provider grouping for the same input config
  • No behavior change for v1 plugins (grouping output identical)

Source

Copilot inline comment on PR #528 round 5: #528 (comment)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions