fix(infra): --refresh-outputs reconciles stale routed secrets instead of skipping them#646
Merged
Merged
Conversation
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
…ly requested Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/79783eb9-2faa-4eb2-988d-7a7d6b5e813c Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Update wfctl refresh-outputs to reconcile existing routed secrets
fix(infra): --refresh-outputs reconciles stale routed secrets instead of skipping them
May 13, 2026
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the infra apply post-processing path so --refresh-outputs reconciles configured infra_output secrets instead of skipping existing values, helping keep routed output secrets aligned after provider/resource recreation.
Changes:
- Threads
refreshOutputsFlagfromrunInfraApplyintosyncInfraOutputSecrets. - Adds refresh-mode reconciliation behavior for readable and write-only secret providers.
- Adds unit coverage for refresh-mode update/create/unchanged/write-only cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
cmd/wfctl/infra.go |
Passes the CLI --refresh-outputs flag into infra output secret synchronization. |
cmd/wfctl/infra_output_secrets.go |
Implements refresh-mode secret reconciliation and logging semantics. |
cmd/wfctl/infra_output_secrets_test.go |
Updates existing call sites and adds refresh-mode unit tests. |
| } | ||
| } | ||
| return syncInfraOutputSecrets(ctx, secretsCfg, secretsProvider, states, wfCfg, envName, runHydrated) | ||
| return syncInfraOutputSecrets(ctx, secretsCfg, secretsProvider, states, wfCfg, envName, runHydrated, refreshOutputsFlag) |
Comment on lines
+290
to
+295
| err := syncInfraOutputSecrets(context.Background(), cfg, p, sampleStates(), nil, "", nil, true) | ||
| if err != nil { | ||
| t.Fatalf("syncInfraOutputSecrets: %v", err) | ||
| } | ||
| if p.data["DATABASE_URL"] != current { | ||
| t.Errorf("unchanged secret should not be modified: got %q", p.data["DATABASE_URL"]) |
Contributor
|
@copilot apply changes based on the comments in this thread Test coverage is also lax: #646 (comment) |
…-outputs integration tests Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/185a3026-2d6d-4ef1-a04c-82efba142cb5 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Contributor
Author
Addressed both review items in ee2f50b:
|
Contributor
Author
Both issues are addressed in commit
|
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.
When
wfctl infra apply --refresh-outputsran after a provider was recreated, routed secrets (e.g.COMPUTE_GITHUB_RUNNER_PROVIDER_URL) that already existed in the secrets provider were logged as"already exists — skipped"and left with their stale values, causing downstream failures.Changes
syncInfraOutputSecrets— addsrefreshOutputs boolparameter that switches from skip-if-exists to reconcile semantics:Setonly when value is identical ("unchanged"), otherwise updates ("updated from infra output")Setsince comparison is impossible ("updated from infra output")"created from infra output")refreshOutputs=false): unchanged — skips existing secrets ("already exists — skipped")runInfraApply— passesrefreshOutputsFlagthrough tosyncInfraOutputSecretsso the post-apply secret sync honours the operator's explicit intent.Log output (refresh mode)
Invariants preserved
--refresh-outputsalways reconciles managed output routes, even when the secret already exists.