fix: target first environment in PR-preview dry-run - #196
Merged
Merged
Conversation
The generated cascade-pr-preview.yaml ran orchestrate setup with no --environment. For a manifest that declares environments, version calculation looks the empty value up in the environments list, finds no match, and fails the preview with environment "" not found, so the PR-preview workflow could never succeed for any multi-environment repo. Pass --environment with the first (lowest) environment, mirroring how the orchestrate workflow defaults its setup environment. Version calculation then resolves against that environment. A manifest with no environments still omits the flag and runs the no-environment path. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
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
1 participant
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.
Problem
The generated
cascade-pr-preview.yamlran its "Compute plan (dry-run)" step ascascade --dry-run orchestrate setup --config <manifest> --sha $HEAD_SHAwith no--environment. The flag defaults to"". For any manifest that declares environments,calculateVersionlooks the empty value up in the environments list, finds no match, and fails with:So the PR-preview workflow could never succeed for any multi-environment repo.
Fix
internal/generate/pr_preview.gowriteDeployDryRunStepnow emits--environment <environments[0]>when the manifest declares environments, mirroring how the orchestrate workflow defaults its setup environment (generator.gousesEnvironments[0]likewise). Version calculation resolves against that environment. A manifest with no environments still omits the flag and runs the no-environment path. The step stays read-only (--dry-runhard-coded) regardless of which environment it reports on.Verification
TestPRPreviewGenerator_DryRunResolvesEnvironment(multi-env asserts--environment staging; no-env asserts the flag is omitted). Written failing first, then green.16-pr-preview.yamlnow uses[staging, prod]and asserts--environment stagingin the generated workflow.environment "" not found); confirmedorchestrate setup --environment staging --dry-runresolvesv0.1.0-rc.0with exit 0 and no error.go build ./... && go test ./... && golangci-lint run ./...all green (1398 tests). e2e module builds and vets clean.