fix: preserve secrets.generate vars outside env_vars to prevent DesiredHash mismatch#559
Conversation
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
…esiredHash mismatch
Add ExpandEnvInMapPreservingVars to config/env_expand.go that, in addition to
preserving entire key submaps (env_vars, env_vars_secret, secret_env_vars), also
preserves individual ${VAR} references whose variable name appears in the
cfg.Secrets.Generate list. This ensures desiredStateHash produces identical output
regardless of whether generated secret variables are present in the environment.
Fixes the "plan stale: config hash mismatch" error that occurs when a
secrets.generate key (e.g. STAGING_PG_PASSWORD) is referenced in a non-env_vars
config field (e.g. Droplet user_data cloud-init script): at plan time the var is
absent so os.ExpandEnv returns "", at apply time it holds the generated value,
causing the hash to diverge.
- config/env_expand.go: add ExpandEnvInMapPreservingVars + helpers
- config/env_expand_test.go: tests for new function including hash-consistency check
- cmd/wfctl/infra.go: add secretGenKeys helper; use ExpandEnvInMapPreservingVars in
parseInfraResourceSpecs and planResourcesForEnv
- cmd/wfctl/infra_plan_env_vars_preserve_test.go: regression test for user_data hash mismatch
- cmd/wfctl/testdata/infra-with-env-var-refs.yaml: add secrets.generate + droplet with user_data
Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/643f6bb1-a20d-4468-94a6-ccebf7c70d3c
Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes infra plan/apply desiredStateHash mismatches caused by eager env expansion of secrets.generate variables outside the existing preserved submaps (e.g., user_data cloud-init blocks), which could otherwise lead to spurious “plan stale: config hash mismatch” errors.
Changes:
- Added
ExpandEnvInMapPreservingVarsto preserve specific variable names as literal${VAR}while still expanding all other env vars outside preserved submaps. - Updated wfctl infra planning code to preserve
cfg.Secrets.Generate[].Keyvariables during config expansion. - Added unit + regression tests and updated infra test fixture to cover the hash-consistency scenario.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| config/env_expand.go | Adds env-expansion helper that preserves specific variable names as literals while keeping preserved submap behavior. |
| config/env_expand_test.go | Adds unit tests for var-name preservation behavior (nil input, precedence, nested slices, hash consistency). |
| cmd/wfctl/infra.go | Extracts secrets.generate keys and uses them during env expansion for infra planning/spec parsing. |
| cmd/wfctl/infra_plan_env_vars_preserve_test.go | Adds regression test asserting desiredStateHash stability when generated secret env var is set vs unset. |
| cmd/wfctl/testdata/infra-with-env-var-refs.yaml | Adds secrets.generate and a droplet user_data reference to a generated secret for regression coverage. |
| cmd/wfctl/dryrun_test.go | Removes trailing whitespace at EOF. |
| cmd/wfctl/ci_run_dryrun.go | Formatting/alignment-only changes to struct field indentation. |
|
@copilot apply changes based on the comments in this thread |
| // ExpandEnvInMapPreservingVars is like ExpandEnvInMapPreservingKeys but adds | ||
| // a second dimension of preservation: individual ${VAR} / $VAR references | ||
| // whose variable name appears in preserveVarNames are emitted as the literal | ||
| // "${name}" instead of being substituted from the process environment. | ||
| // | ||
| // Use case: plan-time serialisation of resource specs where a known set of | ||
| // secret variable names (e.g. cfg.Secrets.Generate keys) must produce | ||
| // hash-identical output regardless of whether the variable is present in the | ||
| // current environment. Without this, fields such as user_data that contain | ||
| // ${SECRET_VAR} produce different hashes at plan time (var unset → empty | ||
| // substitution) and apply time (var set → actual value), causing a spurious | ||
| // "plan stale: config hash mismatch". | ||
| // | ||
| // Precedence: preserveKeys takes priority — if a map key is in preserveKeys | ||
| // the entire subtree is deep-copied as-is (no expansion at all, matching | ||
| // ExpandEnvInMapPreservingKeys semantics). preserveVarNames only affects | ||
| // string values in portions of the tree that are NOT inside a preserved-key | ||
| // subtree. | ||
| func ExpandEnvInMapPreservingVars(m map[string]any, preserveKeys []string, preserveVarNames []string) map[string]any { | ||
| if m == nil { | ||
| return nil | ||
| } | ||
| preserveK := make(map[string]struct{}, len(preserveKeys)) | ||
| for _, k := range preserveKeys { | ||
| preserveK[k] = struct{}{} | ||
| } | ||
| preserveV := make(map[string]struct{}, len(preserveVarNames)) | ||
| for _, v := range preserveVarNames { | ||
| preserveV[v] = struct{}{} | ||
| } | ||
| return expandEnvInMapWithPreserveVars(m, preserveK, preserveV) | ||
| } | ||
|
|
||
| func expandEnvInMapWithPreserveVars(m map[string]any, preserveK, preserveV map[string]struct{}) map[string]any { | ||
| out := make(map[string]any, len(m)) | ||
| for k, v := range m { | ||
| if _, isPreservedKey := preserveK[k]; isPreservedKey { | ||
| out[k] = deepCopyValue(v) | ||
| continue | ||
| } | ||
| out[k] = expandEnvInValueWithPreserveVars(v, preserveK, preserveV) | ||
| } | ||
| return out | ||
| } | ||
|
|
||
| func expandEnvInValueWithPreserveVars(v any, preserveK, preserveV map[string]struct{}) any { | ||
| switch val := v.(type) { | ||
| case string: | ||
| if len(preserveV) == 0 { | ||
| return os.ExpandEnv(val) | ||
| } | ||
| return os.Expand(val, func(name string) string { | ||
| if _, ok := preserveV[name]; ok { | ||
| return "${" + name + "}" | ||
| } | ||
| return os.Getenv(name) | ||
| }) |
… test Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/2787b760-c8b6-4eed-9e60-c87107dade39 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Done in f55b82f. Replaced |
|
@copilot apply changes based on the comments in this thread |
…rvingVars Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/b37c58c4-6c11-4a8b-8a39-cbfa685f1bb3 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Added |
infraPreserveKeysonly prevented${VAR}expansion insideenv_vars/env_vars_secret/secret_env_varssubmaps. Any other field referencing asecrets.generatekey (e.g. Dropletuser_datacloud-init) was expanded byos.ExpandEnv— to""at plan time (var absent) and to the actual value at apply time — producing divergentdesiredStateHashvalues and a spuriousplan stale: config hash mismatch.Changes
config/env_expand.go—ExpandEnvInMapPreservingVars(m, preserveKeys, preserveVarNames): extends the existing key-submap preservation with a second axis. For string values outside preserved submaps, usesos.Expandwith a custom mapping that returns the literal${NAME}for any var inpreserveVarNames, leaving all other vars to resolve normally.cmd/wfctl/infra.go—secretGenKeys(cfg)extractsKeyfields fromcfg.Secrets.Generate. BothparseInfraResourceSpecsandplanResourcesForEnvnow callExpandEnvInMapPreservingVarswith these keys so the hash is identical whether the generated secret is in the environment or not.config/env_expand_test.go— unit tests covering nil input, hash consistency (set vs unset), non-secret vars still expand,preserveKeystakes priority, nested slices.cmd/wfctl/infra_plan_env_vars_preserve_test.go+testdata/infra-with-env-var-refs.yaml— regression test:POSTGRES_PASSWORD(insecrets.generate) referenced in a Dropletuser_datablock produces the samedesiredStateHashwhether the var is set or not.