fix: preserve required secrets in infra plans#746
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates wfctl infra planning to treat more “declared secrets” as plan-time preserved ${VAR} references, preventing secret literals from entering infra plans and keeping desired-state hashes stable across plan/apply.
Changes:
- Expand the set of preserved variables to include top-level
secrets.entriesandsecrets.*modules (secrets.requires/secrets.generate) during infra plan-time env expansion. - Extend runtime-only secret blocking to cover externally supplied required secrets (not just generated non-
infra_outputsecrets), so plan-time substitution won’t embed them into plans. - Add regressions covering required-secret preservation and desired-state hash stability.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/wfctl/testdata/infra-with-env-var-refs.yaml | Adds a secrets.requires module and references a required secret in user_data to exercise plan-time preservation. |
| cmd/wfctl/infra.go | Replaces secretGenKeys with declaredSecretKeys and adds parsing for secret keys from top-level secrets and secrets.* modules. |
| cmd/wfctl/infra_resolve_state.go | Broadens runtime-only secret key detection to include secrets.entries and secrets.requires declarations. |
| cmd/wfctl/infra_resolve_state_test.go | Adds coverage ensuring required secrets and module-generated secrets are handled correctly at plan time. |
| cmd/wfctl/infra_plan_env_vars_preserve_test.go | Adds a hash-stability regression ensuring required secrets remain templated in user_data. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
secrets.entriesand modulesecrets.requires/secrets.generatekeys as declared secrets during infra plan-time env expansion.${VAR}references at plan time so F3 does not see secret literals and desired-state hashes stay stable.WFCOMPUTE_VALIDATION_TOKEN-style required secrets and hash stability.Red/green proof
With fix reverted: