Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.18.9] - 2026-04-24

### Fixed

- **`wfctl ci run --phase deploy` now uses env-resolved module name** — `pluginDeployProvider.resourceName` was populated from `m.Name` (base config name) instead of `ResolvedModule.Name` (env-override lifted from `Config["name"]`). When infra apply had env-renamed a module (e.g. BMW's `bmw-app` → `bmw-staging` for staging), the deploy phase used the base name for `driver.Read` lookup, didn't find the resource, and went down the Create path — producing duplicate DO resources. Same class as v0.18.7 Task #32 fix, but in the ci-run code path. Root cause from BMW deploy run 24888583717.
- **`infra_output` source resolution now applies env override to module name** — `secrets.generate[].source: "bmw-database.uri"` now resolves to the env-resolved state key (e.g. `bmw-staging-db`) when `--env staging` is set, matching how infra apply persists state. Closes task #56.

### Tests

- `cmd/wfctl/deploy_providers_env_test.go` — `TestPluginDeployProvider_UsesEnvResolvedName`, `TestPluginDeployProvider_FallsBackToModuleNameWhenNoEnv`
- `cmd/wfctl/infra_secrets_env_test.go` — `TestInfraOutput_EnvResolvesModuleSource`, `TestInfraOutput_NoEnvUsesBaseName`
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The v0.18.9 changelog test list omits TestInfraOutput_ExplicitlyDisabledModuleErrors, which was added in cmd/wfctl/infra_secrets_env_test.go. Please update this line to include the third test (or list the file without enumerating specific test names) so the changelog stays accurate.

Suggested change
- `cmd/wfctl/infra_secrets_env_test.go``TestInfraOutput_EnvResolvesModuleSource`, `TestInfraOutput_NoEnvUsesBaseName`
- `cmd/wfctl/infra_secrets_env_test.go``TestInfraOutput_EnvResolvesModuleSource`, `TestInfraOutput_NoEnvUsesBaseName`, `TestInfraOutput_ExplicitlyDisabledModuleErrors`

Copilot uses AI. Check for mistakes.

## [0.18.8] - 2026-04-24

### Fixed
Expand Down
49 changes: 25 additions & 24 deletions cmd/wfctl/deploy_providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -703,18 +703,19 @@ func newPluginDeployProvider(providerName string, wfCfg *config.WorkflowConfig,
return nil, fmt.Errorf("unsupported deploy provider %q (built-ins: kubernetes, docker, aws-ecs; to use a plugin provider, declare an iac.provider module in your workflow config)%s", providerName, fmt.Sprintf(hint, providerName))
}

// resolveModCfg returns the effective config map for m after applying the
// resolveModule returns the effective ResolvedModule for m after applying the
// per-env overlay (when envName is set). ok=false means the module is
// explicitly deleted for this env and should be skipped.
resolveModCfg := func(m *config.ModuleConfig) (map[string]any, bool) {
// Callers must read resolved.Name (not m.Name) to get the env-overridden identity.
resolveModule := func(m *config.ModuleConfig) (*config.ResolvedModule, bool) {
if envName == "" {
return m.Config, true
return &config.ResolvedModule{
Name: m.Name,
Type: m.Type,
Config: m.Config,
}, true
}
resolved, ok := m.ResolveForEnv(envName)
if !ok {
return nil, false
}
return resolved.Config, true
return m.ResolveForEnv(envName)
}

// Find the iac.provider module matching the requested provider name.
Expand All @@ -725,14 +726,14 @@ func newPluginDeployProvider(providerName string, wfCfg *config.WorkflowConfig,
if m.Type != "iac.provider" {
continue
}
cfg, ok := resolveModCfg(m)
resolved, ok := resolveModule(m)
if !ok {
continue
}
cfgProvider, _ := cfg["provider"].(string)
if cfgProvider == providerName || m.Name == providerName {
providerModName = m.Name
providerModCfg = cfg
cfgProvider, _ := resolved.Config["provider"].(string)
if cfgProvider == providerName || resolved.Name == providerName {
providerModName = resolved.Name
providerModCfg = resolved.Config
break
}
}
Expand Down Expand Up @@ -761,14 +762,14 @@ func newPluginDeployProvider(providerName string, wfCfg *config.WorkflowConfig,
if m.Type != target {
continue
}
cfg, ok := resolveModCfg(m)
resolved, ok := resolveModule(m)
if !ok {
continue
}
if p, _ := cfg["provider"].(string); p == providerModName {
resourceName = m.Name
resourceType = m.Type
resourceCfg = cfg
if p, _ := resolved.Config["provider"].(string); p == providerModName {
resourceName = resolved.Name
resourceType = resolved.Type
resourceCfg = resolved.Config
return true
}
}
Expand All @@ -786,16 +787,16 @@ func newPluginDeployProvider(providerName string, wfCfg *config.WorkflowConfig,
if m.Type == "iac.provider" || m.Type == "" {
continue
}
cfg, ok := resolveModCfg(m)
resolved, ok := resolveModule(m)
if !ok {
continue
}
if p, _ := cfg["provider"].(string); p == providerModName {
if p, _ := resolved.Config["provider"].(string); p == providerModName {
fmt.Fprintf(os.Stderr, "warning: no deploy-target module (%v) found for provider %q; falling back to first infra module %q (type %q)\n",
deployTargetTypes, providerModName, m.Name, m.Type)
resourceName = m.Name
resourceType = m.Type
resourceCfg = cfg
deployTargetTypes, providerModName, resolved.Name, resolved.Type)
resourceName = resolved.Name
resourceType = resolved.Type
resourceCfg = resolved.Config
break
}
}
Expand Down
73 changes: 73 additions & 0 deletions cmd/wfctl/deploy_providers_env_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package main

import (
"testing"

"github.com/GoCodeAlone/workflow/config"
)

func TestPluginDeployProvider_UsesEnvResolvedName(t *testing.T) {
wfCfg := &config.WorkflowConfig{
Modules: []config.ModuleConfig{
{
Name: "do-provider",
Type: "iac.provider",
Config: map[string]any{"provider": "digitalocean"},
},
{
Name: "bmw-app",
Type: "infra.container_service",
Config: map[string]any{"provider": "do-provider"},
Environments: map[string]*config.InfraEnvironmentResolution{
"staging": {
Config: map[string]any{"name": "bmw-staging"},
},
},
},
},
}

dp, err := newPluginDeployProvider("digitalocean", wfCfg, "staging")
if err != nil {
t.Fatalf("newPluginDeployProvider: %v", err)
}

pdp, ok := dp.(*pluginDeployProvider)
if !ok {
t.Fatalf("expected *pluginDeployProvider, got %T", dp)
}
if pdp.resourceName != "bmw-staging" {
t.Errorf("resourceName = %q, want %q (env-resolved name)", pdp.resourceName, "bmw-staging")
}
}

func TestPluginDeployProvider_FallsBackToModuleNameWhenNoEnv(t *testing.T) {
wfCfg := &config.WorkflowConfig{
Modules: []config.ModuleConfig{
{
Name: "do-provider",
Type: "iac.provider",
Config: map[string]any{"provider": "digitalocean"},
},
{
Name: "bmw-app",
Type: "infra.container_service",
Config: map[string]any{"provider": "do-provider"},
// NOTE: no Environments block — base name should be used
},
},
}

dp, err := newPluginDeployProvider("digitalocean", wfCfg, "")
if err != nil {
t.Fatalf("newPluginDeployProvider: %v", err)
}

pdp, ok := dp.(*pluginDeployProvider)
if !ok {
t.Fatalf("expected *pluginDeployProvider, got %T", dp)
}
if pdp.resourceName != "bmw-app" {
t.Errorf("resourceName = %q, want %q (base module name when no env)", pdp.resourceName, "bmw-app")
}
}
19 changes: 18 additions & 1 deletion cmd/wfctl/infra.go
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,24 @@ func runInfraApply(args []string) error {
return fmt.Errorf("resolve secrets provider for infra_output sync: %w", err)
}
states := loadCurrentState(cfgFile, envName)
return syncInfraOutputSecrets(ctx, secretsCfg, secretsProvider, states)
// Only reload the workflow config when env resolution is actually needed:
// it is needed only when --env is set AND at least one infra_output secret
// generator is configured (otherwise syncInfraOutputSecrets is a no-op for
// env resolution regardless).
var wfCfg *config.WorkflowConfig
if envName != "" {
for _, g := range secretsCfg.Generate {
if g.Type == "infra_output" {
var loadErr error
wfCfg, loadErr = config.LoadFromFile(cfgFile)
if loadErr != nil {
return fmt.Errorf("load config for infra_output env resolution: %w", loadErr)
}
break
}
}
}
Comment on lines +832 to +844
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

secretsCfg can be nil here, which would panic when evaluating secretsCfg.Generate. Since syncInfraOutputSecrets already treats a nil secrets config as a no-op, add an explicit guard (e.g., if envName != "" && secretsCfg != nil { ... }) before ranging over secretsCfg.Generate.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declined — secretsCfg cannot be nil at this point. Line 820 guards: if err != nil || secretsCfg == nil { return err }. Execution only reaches the optimization block when secretsCfg is non-nil. Adding a redundant guard would mislead future readers into thinking nil is possible here. b50e3f3 is correct as-is.

return syncInfraOutputSecrets(ctx, secretsCfg, secretsProvider, states, wfCfg, envName)
}

func runInfraStatus(args []string) error {
Expand Down
79 changes: 73 additions & 6 deletions cmd/wfctl/infra_output_secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import (
"context"
"errors"
"fmt"
"sort"
"strings"

"github.com/GoCodeAlone/workflow/config"
"github.com/GoCodeAlone/workflow/interfaces"
"github.com/GoCodeAlone/workflow/secrets"
)
Expand All @@ -23,10 +26,78 @@ func buildStateOutputsMap(states []interfaces.ResourceState) map[string]map[stri
return m
}

// resolveInfraOutput resolves a single "module.field" source string against the
// pre-loaded state outputs map, applying per-env module name resolution so that
// a source like "bmw-database.uri" finds the state keyed by the env-resolved
// name (e.g. "bmw-staging-db") when --env staging renames the module.
//
// wfCfg may be nil (e.g. tests that only care about base-name resolution).
// When envName is empty no resolution is performed and the source module name
// is used verbatim.
func resolveInfraOutput(wfCfg *config.WorkflowConfig, source, envName string, stateOutputs map[string]map[string]any) (string, error) {
if source == "" {
return "", fmt.Errorf("infra_output: source is required (format: \"module.field\")")
}
dot := strings.Index(source, ".")
if dot < 1 || dot >= len(source)-1 {
return "", fmt.Errorf("infra_output: invalid source %q: expected \"module.field\" format", source)
}
moduleName := source[:dot]
field := source[dot+1:]

// Apply env resolution: the state was persisted under the env-resolved name.
if envName != "" && wfCfg != nil {
for i := range wfCfg.Modules {
m := &wfCfg.Modules[i]
if m.Name != moduleName {
continue
}
resolved, ok := m.ResolveForEnv(envName)
if !ok {
return "", fmt.Errorf("infra_output: module %q is explicitly disabled for environment %q — cannot read infra_output from a disabled module", moduleName, envName)
}
if resolved.Name != "" {
moduleName = resolved.Name
}
break
}
}

if stateOutputs == nil {
return "", fmt.Errorf("infra_output: state outputs not available for source %q — did infra apply succeed?", source)
}
outputs, ok := stateOutputs[moduleName]
if !ok {
return "", fmt.Errorf("infra_output: module %q not found in state (available: %s)", moduleName, strings.Join(stateKeys(stateOutputs), ", "))
}
val, ok := outputs[field]
if !ok {
return "", fmt.Errorf("infra_output: field %q not found in outputs of module %q", field, moduleName)
}
s, ok := val.(string)
if !ok {
return "", fmt.Errorf("infra_output: output field %q of module %q is %T, expected string", field, moduleName, val)
}
return s, nil
Comment on lines +66 to +81
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolveInfraOutput re-implements the existing secrets.GenerateSecret "infra_output" generator logic (source parsing, state lookup, type assertion, and related error messages). This duplicates behavior across packages and risks drift/inconsistent user-facing errors. Consider reusing the existing generator by doing only the env-specific module-name resolution here (including the explicit-disabled error), then calling secrets.GenerateSecret("infra_output", ...) with an updated source and the _state_outputs map. That keeps infra_output semantics in one place and preserves existing error wording/prefixes.

Suggested change
if stateOutputs == nil {
return "", fmt.Errorf("infra_output: state outputs not available for source %q — did infra apply succeed?", source)
}
outputs, ok := stateOutputs[moduleName]
if !ok {
return "", fmt.Errorf("infra_output: module %q not found in state (available: %s)", moduleName, strings.Join(stateKeys(stateOutputs), ", "))
}
val, ok := outputs[field]
if !ok {
return "", fmt.Errorf("infra_output: field %q not found in outputs of module %q", field, moduleName)
}
s, ok := val.(string)
if !ok {
return "", fmt.Errorf("infra_output: output field %q of module %q is %T, expected string", field, moduleName, val)
}
return s, nil
return secrets.GenerateSecret(
"infra_output",
map[string]any{
"source": moduleName + "." + field,
},
map[string]any{
"_state_outputs": stateOutputs,
},
)

Copilot uses AI. Check for mistakes.
}

// stateKeys returns the sorted keys of a state outputs map for error messages.
func stateKeys(m map[string]map[string]any) []string {
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
sort.Strings(keys)
return keys
Comment thread
intel352 marked this conversation as resolved.
}

// syncInfraOutputSecrets writes infra_output-typed secrets after a successful
// apply. It skips secrets that already exist in the provider so idempotent
// re-runs never overwrite live values.
func syncInfraOutputSecrets(ctx context.Context, secretsCfg *SecretsConfig, provider secrets.Provider, states []interfaces.ResourceState) error {
// wfCfg and envName are used to resolve source module names through per-env
// overrides so that "bmw-database.uri" finds "bmw-staging-db" in state when
// --env staging renames the module.
func syncInfraOutputSecrets(ctx context.Context, secretsCfg *SecretsConfig, provider secrets.Provider, states []interfaces.ResourceState, wfCfg *config.WorkflowConfig, envName string) error {
if secretsCfg == nil {
return nil
}
Expand Down Expand Up @@ -89,11 +160,7 @@ func syncInfraOutputSecrets(ctx context.Context, secretsCfg *SecretsConfig, prov
continue
}

genConfig := map[string]any{
"source": gen.Source,
"_state_outputs": stateOutputs,
}
value, err := generateSecret(ctx, "infra_output", genConfig)
value, err := resolveInfraOutput(wfCfg, gen.Source, envName, stateOutputs)
if err != nil {
return fmt.Errorf("generate infra_output secret %q: %w", gen.Key, err)
}
Expand Down
18 changes: 9 additions & 9 deletions cmd/wfctl/infra_output_secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestBuildStateOutputsMap_Empty(t *testing.T) {

func TestSyncInfraOutputSecrets_NilConfig(t *testing.T) {
p := newSimpleProvider()
err := syncInfraOutputSecrets(context.Background(), nil, p, sampleStates())
err := syncInfraOutputSecrets(context.Background(), nil, p, sampleStates(), nil, "")
if err != nil {
t.Fatalf("nil config should be no-op: %v", err)
}
Expand All @@ -125,7 +125,7 @@ func TestSyncInfraOutputSecrets_NoInfraOutputGens(t *testing.T) {
{Key: "JWT_SECRET", Type: "random_hex", Length: 32},
},
}
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates())
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates(), nil, "")
if err != nil {
t.Fatalf("no infra_output generators should be no-op: %v", err)
}
Expand All @@ -141,7 +141,7 @@ func TestSyncInfraOutputSecrets_WritesSecret(t *testing.T) {
{Key: "STAGING_DATABASE_URL", Type: "infra_output", Source: "bmw-database.uri"},
},
}
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates())
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates(), nil, "")
if err != nil {
t.Fatalf("syncInfraOutputSecrets: %v", err)
}
Expand All @@ -158,7 +158,7 @@ func TestSyncInfraOutputSecrets_WritesMultiple(t *testing.T) {
{Key: "REDIS_URL", Type: "infra_output", Source: "bmw-cache.url"},
},
}
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates())
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates(), nil, "")
if err != nil {
t.Fatalf("syncInfraOutputSecrets: %v", err)
}
Expand All @@ -178,7 +178,7 @@ func TestSyncInfraOutputSecrets_SkipsExisting(t *testing.T) {
{Key: "DATABASE_URL", Type: "infra_output", Source: "bmw-database.uri"},
},
}
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates())
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates(), nil, "")
if err != nil {
t.Fatalf("syncInfraOutputSecrets: %v", err)
}
Expand All @@ -198,7 +198,7 @@ func TestSyncInfraOutputSecrets_WriteOnlyProviderSkips(t *testing.T) {
{Key: "DATABASE_URL", Type: "infra_output", Source: "bmw-database.uri"},
},
}
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates())
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates(), nil, "")
if err != nil {
t.Fatalf("syncInfraOutputSecrets: %v", err)
}
Expand All @@ -217,7 +217,7 @@ func TestSyncInfraOutputSecrets_WriteOnlyProviderWrites(t *testing.T) {
{Key: "DATABASE_URL", Type: "infra_output", Source: "bmw-database.uri"},
},
}
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates())
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates(), nil, "")
if err != nil {
t.Fatalf("syncInfraOutputSecrets: %v", err)
}
Expand All @@ -233,7 +233,7 @@ func TestSyncInfraOutputSecrets_MissingModule(t *testing.T) {
{Key: "X", Type: "infra_output", Source: "nonexistent.uri"},
},
}
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates())
err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates(), nil, "")
if err == nil {
t.Fatal("expected error for missing module in state")
}
Expand All @@ -246,7 +246,7 @@ func TestSyncInfraOutputSecrets_EmptyStates(t *testing.T) {
{Key: "X", Type: "infra_output", Source: "bmw-database.uri"},
},
}
err := syncInfraOutputSecrets(context.Background(), cfg, p, nil)
err := syncInfraOutputSecrets(context.Background(), cfg, p, nil, nil, "")
if err == nil {
t.Fatal("expected error when state has no matching module")
}
Expand Down
Loading
Loading