From 2e98770b0200989fcb38efe124ac42838bc2bbb9 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 10 May 2026 11:45:46 -0400 Subject: [PATCH 1/2] fix(wfctl): disambiguate iac_provider from impl-level provider in resource configs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plugin schemas like workflow-plugin-eventbus declare "provider" as the implementation identifier (nats, kafka, ...). wfctl's plan/apply/dispatch code paths conflated that with the iac.provider module reference, looking up the impl name as a module name and failing with: error: plan action for "bmw-eventbus" references provider "nats" which is not declared as an iac.provider module The infra.database resource ("provider: do-provider") works because there the field happens to name the iac.provider module directly. Resources where "provider" carries impl semantics (eventbus, etc.) need a separate field for IaC routing. This change: introduce `iac_provider` as the canonical field for selecting the iac.provider module to dispatch to. Read it via a new `resolveIaCProviderRef(map[string]any) string` helper that returns iac_provider first and falls back to provider for backward compatibility. Updated all 9 read sites: cmd/wfctl/infra.go:1034 (resolveProviderForSpec) cmd/wfctl/infra_provider_dispatch.go:118 (groupSpecsByProviderRef) cmd/wfctl/infra_apply.go:348/356 (state/spec helpers) cmd/wfctl/infra_apply.go:557/1340 (post-apply state record builds) cmd/wfctl/infra_apply.go:1167/1173 (apply group dispatch) cmd/wfctl/infra_apply_dryrun.go:156/226 cmd/wfctl/infra_destroy.go:69 cmd/wfctl/infra_status_drift.go:213 cloud.account / iac.provider module-internal `provider` reads (deploy.go, deploy_providers.go, ci_run_dryrun.go) are intentionally left alone — those refer to the impl-name field on iac.provider modules themselves (provider: digitalocean), not resource-level routing. Error messages updated to mention `iac_provider/provider` resolution path so operators can quickly identify the field set. Tests: TestResolveIaCProviderRef covers 8 cases including iac_provider-wins-over-provider, type-mismatch fallback, nil/empty. Existing Infra/Provider/Apply/Plan/Dispatch test suites green (no regressions). Unblocks BMW deploy: bmw-eventbus (provider="nats") now coexists with iac_provider="do-provider" in infra.yaml without requiring the eventbus plugin to drop its impl-name semantics. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/wfctl/infra.go | 24 +++++++- cmd/wfctl/infra_apply.go | 18 +++--- cmd/wfctl/infra_apply_dryrun.go | 4 +- cmd/wfctl/infra_destroy.go | 2 +- cmd/wfctl/infra_iac_provider_ref_test.go | 74 ++++++++++++++++++++++++ cmd/wfctl/infra_provider_dispatch.go | 8 +-- cmd/wfctl/infra_status_drift.go | 2 +- 7 files changed, 111 insertions(+), 21 deletions(-) create mode 100644 cmd/wfctl/infra_iac_provider_ref_test.go diff --git a/cmd/wfctl/infra.go b/cmd/wfctl/infra.go index f8607d565..8607d971d 100644 --- a/cmd/wfctl/infra.go +++ b/cmd/wfctl/infra.go @@ -1030,10 +1030,28 @@ func findInfraSpecByName(cfgFile, envName, name string) (interfaces.ResourceSpec return interfaces.ResourceSpec{}, fmt.Errorf("infra resource %q not found in %s", name, cfgFile) } +// resolveIaCProviderRef returns the iac.provider module name to dispatch to +// for a resource. Reads "iac_provider" first (canonical, disambiguates +// implementation from IaC routing), falls back to "provider" for backward +// compatibility. Resources whose plugin schema uses "provider" as the +// implementation identifier (e.g. infra.eventbus's provider="nats" | +// "kafka") should declare iac_provider explicitly. Resources where +// "provider" is itself the iac.provider module name (e.g. infra.database's +// provider="do-provider") continue to work via the fallback. +func resolveIaCProviderRef(cfg map[string]any) string { + if v, ok := cfg["iac_provider"].(string); ok && v != "" { + return v + } + if v, ok := cfg["provider"].(string); ok { + return v + } + return "" +} + func resolveProviderForSpec(cfgFile, envName string, spec interfaces.ResourceSpec) (string, map[string]any, error) { - moduleRef, _ := spec.Config["provider"].(string) + moduleRef := resolveIaCProviderRef(spec.Config) if moduleRef == "" { - return "", nil, fmt.Errorf("infra module %q (%s): missing required 'provider' field", spec.Name, spec.Type) + return "", nil, fmt.Errorf("infra module %q (%s): missing required 'iac_provider' or 'provider' field", spec.Name, spec.Type) } cfg, err := config.LoadFromFile(cfgFile) if err != nil { @@ -1060,7 +1078,7 @@ func resolveProviderForSpec(cfgFile, envName string, spec interfaces.ResourceSpe } return providerType, modCfg, nil } - return "", nil, fmt.Errorf("infra module %q references provider %q which is not declared as an iac.provider module", spec.Name, moduleRef) + return "", nil, fmt.Errorf("infra module %q references iac.provider module %q (resolved from iac_provider/provider) which is not declared in modules", spec.Name, moduleRef) } func isNoopStateStore(store infraStateStore) bool { diff --git a/cmd/wfctl/infra_apply.go b/cmd/wfctl/infra_apply.go index 0b4cb2d2e..a0bf93e03 100644 --- a/cmd/wfctl/infra_apply.go +++ b/cmd/wfctl/infra_apply.go @@ -345,16 +345,14 @@ func resourceStateProviderRef(st interfaces.ResourceState) string { if st.AppliedConfig == nil { return "" } - providerRef, _ := st.AppliedConfig["provider"].(string) - return providerRef + return resolveIaCProviderRef(st.AppliedConfig) } func resourceSpecProviderRef(spec interfaces.ResourceSpec) string { if spec.Config == nil { return "" } - providerRef, _ := spec.Config["provider"].(string) - return providerRef + return resolveIaCProviderRef(spec.Config) } // applyWithProviderAndStore computes a diff plan for the given specs against @@ -554,7 +552,7 @@ func applyWithProviderAndStore(ctx context.Context, provider interfaces.IaCProvi for i := range specs { if specs[i].Name == r.Name { appliedCfg = specs[i].Config - providerRef, _ = specs[i].Config["provider"].(string) + providerRef = resolveIaCProviderRef(specs[i].Config) dependencies = append([]string(nil), specs[i].DependsOn...) break } @@ -1166,22 +1164,22 @@ func applyFromPrecomputedPlan(ctx context.Context, plan interfaces.IaCPlan, cfgF for i := range plan.Actions { action := &plan.Actions[i] - moduleRef, _ := action.Resource.Config["provider"].(string) + moduleRef := resolveIaCProviderRef(action.Resource.Config) // delete actions from ComputePlan carry an empty Resource.Config — the // provider ref must be recovered from the recorded current state instead. if moduleRef == "" && action.Current != nil { moduleRef = action.Current.ProviderRef if moduleRef == "" { - moduleRef, _ = action.Current.AppliedConfig["provider"].(string) + moduleRef = resolveIaCProviderRef(action.Current.AppliedConfig) } } if moduleRef == "" { - return runHydrated, fmt.Errorf("plan action for %q: missing 'provider' field in resource config (delete actions require a current state record)", action.Resource.Name) + return runHydrated, fmt.Errorf("plan action for %q: missing 'iac_provider' or 'provider' field in resource config (delete actions require a current state record)", action.Resource.Name) } if _, exists := groups[moduleRef]; !exists { def, ok := providerDefs[moduleRef] if !ok { - return runHydrated, fmt.Errorf("plan action for %q references provider %q which is not declared as an iac.provider module", action.Resource.Name, moduleRef) + return runHydrated, fmt.Errorf("plan action for %q references iac.provider module %q (resolved from iac_provider/provider) which is not declared in modules", action.Resource.Name, moduleRef) } groups[moduleRef] = &actionGroup{provType: def.provType, provCfg: def.provCfg} groupOrder = append(groupOrder, moduleRef) @@ -1339,7 +1337,7 @@ func applyPrecomputedPlanWithStore(ctx context.Context, plan interfaces.IaCPlan, for i := range plan.Actions { if plan.Actions[i].Resource.Name == r.Name { appliedCfg = plan.Actions[i].Resource.Config - providerRef, _ = plan.Actions[i].Resource.Config["provider"].(string) + providerRef = resolveIaCProviderRef(plan.Actions[i].Resource.Config) dependencies = append([]string(nil), plan.Actions[i].Resource.DependsOn...) break } diff --git a/cmd/wfctl/infra_apply_dryrun.go b/cmd/wfctl/infra_apply_dryrun.go index 233c85053..450d761e9 100644 --- a/cmd/wfctl/infra_apply_dryrun.go +++ b/cmd/wfctl/infra_apply_dryrun.go @@ -153,7 +153,7 @@ func printDryRunJSON(cfgFile, envName string, plan interfaces.IaCPlan, providerG actions := make([]DryRunAction, 0, len(plan.Actions)) for i := range plan.Actions { a := &plan.Actions[i] - provRef, _ := a.Resource.Config["provider"].(string) + provRef := resolveIaCProviderRef(a.Resource.Config) actions = append(actions, DryRunAction{ Action: a.Action, ResourceName: a.Resource.Name, @@ -223,7 +223,7 @@ func collectProviderGroups(cfgFile, envName string, specs []interfaces.ResourceS if !strings.HasPrefix(spec.Type, "infra.") { continue } - moduleRef, _ := spec.Config["provider"].(string) + moduleRef := resolveIaCProviderRef(spec.Config) if moduleRef == "" { continue } diff --git a/cmd/wfctl/infra_destroy.go b/cmd/wfctl/infra_destroy.go index d6cf6df05..c228abd87 100644 --- a/cmd/wfctl/infra_destroy.go +++ b/cmd/wfctl/infra_destroy.go @@ -66,7 +66,7 @@ func destroyInfraModules(ctx context.Context, cfgFile, envName string) error { / moduleRef := resourceStateProviderRef(*st) if spec, ok := specByName[st.Name]; ok { if moduleRef == "" { - moduleRef, _ = spec.Config["provider"].(string) + moduleRef = resolveIaCProviderRef(spec.Config) } } diff --git a/cmd/wfctl/infra_iac_provider_ref_test.go b/cmd/wfctl/infra_iac_provider_ref_test.go new file mode 100644 index 000000000..d60e85d9e --- /dev/null +++ b/cmd/wfctl/infra_iac_provider_ref_test.go @@ -0,0 +1,74 @@ +package main + +import "testing" + +// TestResolveIaCProviderRef covers the disambiguation between an +// implementation-level "provider" field (e.g. infra.eventbus's provider="nats") +// and the iac.provider module reference. The canonical key is iac_provider; +// "provider" is the backward-compat fallback. +func TestResolveIaCProviderRef(t *testing.T) { + cases := []struct { + name string + cfg map[string]any + want string + }{ + { + name: "iac_provider_only", + cfg: map[string]any{"iac_provider": "do-provider"}, + want: "do-provider", + }, + { + name: "provider_only_back_compat", + cfg: map[string]any{"provider": "do-provider"}, + want: "do-provider", + }, + { + name: "iac_provider_wins_over_provider", + cfg: map[string]any{ + "iac_provider": "do-provider", + "provider": "nats", // implementation, ignored for IaC routing + }, + want: "do-provider", + }, + { + name: "empty_iac_provider_falls_back", + cfg: map[string]any{ + "iac_provider": "", + "provider": "do-provider", + }, + want: "do-provider", + }, + { + name: "neither_set", + cfg: map[string]any{}, + want: "", + }, + { + name: "nil_config", + cfg: nil, + want: "", + }, + { + name: "non_string_iac_provider_falls_back", + cfg: map[string]any{ + "iac_provider": 42, // type mismatch, treated as missing + "provider": "do-provider", + }, + want: "do-provider", + }, + { + name: "non_string_provider", + cfg: map[string]any{"provider": 42}, + want: "", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := resolveIaCProviderRef(tc.cfg) + if got != tc.want { + t.Errorf("resolveIaCProviderRef(%v) = %q; want %q", tc.cfg, got, tc.want) + } + }) + } +} diff --git a/cmd/wfctl/infra_provider_dispatch.go b/cmd/wfctl/infra_provider_dispatch.go index e5910be42..1e8f37ae9 100644 --- a/cmd/wfctl/infra_provider_dispatch.go +++ b/cmd/wfctl/infra_provider_dispatch.go @@ -115,18 +115,18 @@ func groupSpecsByProviderRef(specs []interfaces.ResourceSpec, defs map[string]pr for _, spec := range specs { var moduleRef string if spec.Config != nil { - moduleRef, _ = spec.Config["provider"].(string) + moduleRef = resolveIaCProviderRef(spec.Config) } if moduleRef == "" { - return nil, nil, fmt.Errorf("infra module %q (%s): missing required 'provider' field", spec.Name, spec.Type) + return nil, nil, fmt.Errorf("infra module %q (%s): missing required 'iac_provider' or 'provider' field", spec.Name, spec.Type) } if _, exists := groups[moduleRef]; !exists { def, ok := defs[moduleRef] if !ok { if _, isDisabled := disabled[moduleRef]; isDisabled { - return nil, nil, fmt.Errorf("infra module %q references provider %q which is disabled for environment %q", spec.Name, moduleRef, envName) + return nil, nil, fmt.Errorf("infra module %q references iac.provider %q which is disabled for environment %q", spec.Name, moduleRef, envName) } - return nil, nil, fmt.Errorf("infra module %q references provider %q which is not declared as an iac.provider module", spec.Name, moduleRef) + return nil, nil, fmt.Errorf("infra module %q references iac.provider module %q (resolved from iac_provider/provider) which is not declared in modules", spec.Name, moduleRef) } if def.provType == "" { return nil, nil, fmt.Errorf("provider module %q has no 'provider' type configured", moduleRef) diff --git a/cmd/wfctl/infra_status_drift.go b/cmd/wfctl/infra_status_drift.go index 31e5a9b00..46ccbe96a 100644 --- a/cmd/wfctl/infra_status_drift.go +++ b/cmd/wfctl/infra_status_drift.go @@ -210,7 +210,7 @@ func groupStatesByProvider(states []interfaces.ResourceState, cfgFile, envName s moduleRef := resourceStateProviderRef(*st) if spec, ok := specByName[st.Name]; ok { if moduleRef == "" { - moduleRef, _ = spec.Config["provider"].(string) + moduleRef = resolveIaCProviderRef(spec.Config) } } if moduleRef == "" { From 5da12c935f25b705b3e5a4b49331b60fb743c22c Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 10 May 2026 11:58:28 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20Copilot=20review=20on=20PR?= =?UTF-8?q?=20#620=20=E2=80=94=20preserve=20"iac.provider=20module"=20qual?= =?UTF-8?q?ifier=20in=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3 inline findings, all the same nit at infra.go:1081, infra_apply.go:1182, infra_provider_dispatch.go:129. My initial change widened the wording "not declared as an iac.provider module" → "not declared in modules". The lookup map is filtered to type=iac.provider modules; if a same-named module of a different type exists, "not declared in modules" is inaccurate. Restored the precise qualifier and tightened the field reference to read "iac_provider/provider field" so the operator knows which fields the lookup key was resolved from. Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/wfctl/infra.go | 2 +- cmd/wfctl/infra_apply.go | 2 +- cmd/wfctl/infra_provider_dispatch.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/wfctl/infra.go b/cmd/wfctl/infra.go index 8607d971d..62e476b68 100644 --- a/cmd/wfctl/infra.go +++ b/cmd/wfctl/infra.go @@ -1078,7 +1078,7 @@ func resolveProviderForSpec(cfgFile, envName string, spec interfaces.ResourceSpe } return providerType, modCfg, nil } - return "", nil, fmt.Errorf("infra module %q references iac.provider module %q (resolved from iac_provider/provider) which is not declared in modules", spec.Name, moduleRef) + return "", nil, fmt.Errorf("infra module %q references iac.provider module %q (resolved from iac_provider/provider field) which is not declared as an iac.provider module", spec.Name, moduleRef) } func isNoopStateStore(store infraStateStore) bool { diff --git a/cmd/wfctl/infra_apply.go b/cmd/wfctl/infra_apply.go index a0bf93e03..471cff2a2 100644 --- a/cmd/wfctl/infra_apply.go +++ b/cmd/wfctl/infra_apply.go @@ -1179,7 +1179,7 @@ func applyFromPrecomputedPlan(ctx context.Context, plan interfaces.IaCPlan, cfgF if _, exists := groups[moduleRef]; !exists { def, ok := providerDefs[moduleRef] if !ok { - return runHydrated, fmt.Errorf("plan action for %q references iac.provider module %q (resolved from iac_provider/provider) which is not declared in modules", action.Resource.Name, moduleRef) + return runHydrated, fmt.Errorf("plan action for %q references iac.provider module %q (resolved from iac_provider/provider field) which is not declared as an iac.provider module", action.Resource.Name, moduleRef) } groups[moduleRef] = &actionGroup{provType: def.provType, provCfg: def.provCfg} groupOrder = append(groupOrder, moduleRef) diff --git a/cmd/wfctl/infra_provider_dispatch.go b/cmd/wfctl/infra_provider_dispatch.go index 1e8f37ae9..b9c15c463 100644 --- a/cmd/wfctl/infra_provider_dispatch.go +++ b/cmd/wfctl/infra_provider_dispatch.go @@ -126,7 +126,7 @@ func groupSpecsByProviderRef(specs []interfaces.ResourceSpec, defs map[string]pr if _, isDisabled := disabled[moduleRef]; isDisabled { return nil, nil, fmt.Errorf("infra module %q references iac.provider %q which is disabled for environment %q", spec.Name, moduleRef, envName) } - return nil, nil, fmt.Errorf("infra module %q references iac.provider module %q (resolved from iac_provider/provider) which is not declared in modules", spec.Name, moduleRef) + return nil, nil, fmt.Errorf("infra module %q references iac.provider module %q (resolved from iac_provider/provider field) which is not declared as an iac.provider module", spec.Name, moduleRef) } if def.provType == "" { return nil, nil, fmt.Errorf("provider module %q has no 'provider' type configured", moduleRef)