fix(verify): thread --cli-install mode through Plan and drift-check#662
Merged
Conversation
verify (and the generated cascade-drift-check.yaml, which runs it) always re-planned every generator assuming the default action install mode: Plan never called setInstallMode on any generator, unlike generate-workflow's own command.go, which does so for all eleven generator types before rendering. A repo generated with --cli-install=binary would therefore report every Setup CLI step as spurious drift, forever, since nothing could tell verify which mode to check against. Fixed by mirroring generate-workflow's own pattern exactly: PlanOptions gained a CLIInstall field, parsed once in Plan and applied via setInstallMode to every generator Plan constructs, so a binary-mode repo now verifies clean when told --cli-install=binary. Added the matching --cli-install flag to the verify command itself. Also fixed the other half of the same gap: cascade-drift-check.yaml's own emitted "cascade verify" invocation never passed --cli-install, so even a correctly-generated binary-mode repo's own CI would still report drift at runtime. The generator now emits --cli-install=binary when its own installMode is binary, and omits the flag entirely in the default action mode (byte-identical output for every existing manifest, confirmed by the full byte-identical regression suite still passing unchanged). Verified: two new verify tests prove a binary-mode repo verifies clean when the mode is told, and reports real drift when it isn't (the control, proving the two modes produce genuinely different bytes rather than the test passing by coincidence). Two new drift-check generator tests lock in the flag being present in binary mode and absent in action mode. Full repo test suite, go vet, golangci-lint, and gofmt all pass.
Contributor
|
All PR Validation checks passed. |
TestChangelog_UnreleasedCoversUserFacingCommits correctly caught this was missing from the prior 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
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.
TLDR
verify(and the generatedcascade-drift-check.yaml, which runs it) always re-planned every generator assuming the defaultactioninstall mode, sincePlannever calledsetInstallModeon any generator (unlikegenerate-workflow's owncommand.go, which does for all eleven generator types).--cli-install=binarywould therefore report every Setup CLI step as spurious drift, permanently, with no way to tellverifywhich mode to check against.Plan/verifynow accept--cli-install, and the drift-check generator's own emittedcascade verifyinvocation now passes--cli-install=binarywhen that's the mode it was generated in (byte-identical output preserved for the default action-mode case).Why / value
--cli-install=binary(added in feat(generate): add --cli-install=binary for allowlist-free CLI install #653) was otherwise unusable together withverify/drift_checkat all -- a real capability gap for any adopter whose org's Actions allowlist blocks thesetup-clicomposite action.cfacorp/voice-ordering):cascade verifyreported drift on every file the moment--cli-install=binarywas used, with no flag to reconcile it.Evidence
--cli-install=binaryTestRun_CLIInstallBinary_NoDriftWhenModeMatchesinternal/verify/verify_test.go; passes.TestRun_CLIInstallBinary_MismatchedModeIsRealDriftcascade-drift-check.yaml's own emittedcascade verifyline passes--cli-install=binarywhen generated in that modeTestDriftCheckGenerator_BinaryMode_PassesCLIInstallFlaginternal/generate/drift_check_test.go; passes.TestDriftCheckGenerator_ActionMode_OmitsCLIInstallFlagplus the full existing byte-identical regression suitego build ./...,go vet ./...,golangci-lint run,gofmt -l,go test ./...