diff --git a/CHANGELOG.md b/CHANGELOG.md index ef487ffa..55b8397b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,16 @@ A `Migration` section is added to any release that bumps `schema_version`. ### Fixed +- **verify:** `cascade verify` (and the generated `cascade-drift-check.yaml`, + which runs it) always re-planned every generator assuming the default + `action` `--cli-install` mode, so a repo generated with + `--cli-install=binary` reported every Setup CLI step as spurious drift, + permanently, with no way to reconcile it. `verify` now accepts its own + `--cli-install` flag, threaded through to every generator `Plan` builds, and + the drift-check generator's emitted `cascade verify` invocation now passes + `--cli-install=binary` when that is the mode it was generated in. Action-mode + output is unchanged. + - **release:** A release cut that materializes a git tag now fails closed when the tag already exists at a different commit, instead of treating the `422 reference already exists` response as an unconditional success. The old diff --git a/internal/generate/drift_check.go b/internal/generate/drift_check.go index 05a94f84..196276ae 100644 --- a/internal/generate/drift_check.go +++ b/internal/generate/drift_check.go @@ -153,7 +153,16 @@ func (g *DriftCheckGenerator) writeCheckJob(sb *strings.Builder) { sb.WriteString(" - name: Check for workflow drift\n") sb.WriteString(" run: |\n") sb.WriteString(" set +e\n") - fmt.Fprintf(sb, " cascade verify --config %s > drift-report.txt 2>&1\n", g.getManifestFilePath()) + // --cli-install is omitted in action mode (the default) so existing + // manifests keep byte-identical output; verify's own --cli-install + // default already matches. Binary mode must say so explicitly, or verify + // silently re-plans every file assuming action mode and reports spurious + // drift on every Setup CLI step. + if g.installMode == cliInstallModeBinary { + fmt.Fprintf(sb, " cascade verify --config %s --cli-install=binary > drift-report.txt 2>&1\n", g.getManifestFilePath()) + } else { + fmt.Fprintf(sb, " cascade verify --config %s > drift-report.txt 2>&1\n", g.getManifestFilePath()) + } sb.WriteString(" echo $? > drift-exit.txt\n") sb.WriteString(" set -e\n") sb.WriteString(" cat drift-report.txt\n") diff --git a/internal/generate/drift_check_test.go b/internal/generate/drift_check_test.go index b2b0df79..3df638d9 100644 --- a/internal/generate/drift_check_test.go +++ b/internal/generate/drift_check_test.go @@ -192,3 +192,28 @@ func TestDriftCheckGenerator_Actionlint(t *testing.T) { out, runErr := cmd.CombinedOutput() assert.NoError(t, runErr, "actionlint reported issues:\n%s", string(out)) } + +// TestDriftCheckGenerator_ActionMode_OmitsCLIInstallFlag proves the default +// (action-mode) verify invocation is byte-identical to before --cli-install +// existed: no downstream manifest's committed cascade-drift-check.yaml changes +// just because this generator learned a new flag. +func TestDriftCheckGenerator_ActionMode_OmitsCLIInstallFlag(t *testing.T) { + g := NewDriftCheckGenerator(driftCheckConfig(false), t.TempDir()) + content, err := g.Generate() + require.NoError(t, err) + assert.Contains(t, content, "cascade verify --config .github/manifest.yaml > drift-report.txt") + assert.NotContains(t, content, "--cli-install") +} + +// TestDriftCheckGenerator_BinaryMode_PassesCLIInstallFlag proves a +// binary-mode-generated repo's own drift-check workflow invokes verify with +// the matching --cli-install=binary flag, so verify re-plans the repo in the +// same mode it was generated in instead of silently assuming action mode and +// reporting every Setup CLI step as drift. +func TestDriftCheckGenerator_BinaryMode_PassesCLIInstallFlag(t *testing.T) { + g := NewDriftCheckGenerator(driftCheckConfig(false), t.TempDir()) + g.setInstallMode(cliInstallModeBinary) + content, err := g.Generate() + require.NoError(t, err) + assert.Contains(t, content, "cascade verify --config .github/manifest.yaml --cli-install=binary > drift-report.txt") +} diff --git a/internal/generate/plan.go b/internal/generate/plan.go index 547ebed9..7060135a 100644 --- a/internal/generate/plan.go +++ b/internal/generate/plan.go @@ -41,6 +41,13 @@ type PlanOptions struct { // verify/generate-workflow --own-repo invocation sets this; it is not a // manifest field. OwnRepo bool + // CLIInstall selects how every planned generator emits its "Setup CLI" + // step: "" or "action" (default) for the setup-cli composite action, or + // "binary" for the self-contained install. Mirrors generate-workflow's + // --cli-install flag; verify passes its own --cli-install here so a + // binary-mode-generated repo can be planned back for comparison instead + // of always assuming the action-mode default. + CLIInstall string } // Plan resolves the manifest and returns the complete set of files the generate @@ -65,6 +72,14 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { return nil, fmt.Errorf("parsing config: %w", err) } + // Parsed once and applied to every generator below via setInstallMode, + // mirroring exactly what the generate-workflow command does (command.go) + // so Plan can never disagree with generate-workflow on a binary-mode repo. + installMode, err := parseCLIInstallMode(opts.CLIInstall) + if err != nil { + return nil, fmt.Errorf("parsing --cli-install: %w", err) + } + if opts.PinOverridesPath != "" { if err := ApplyDiskPinOverrides(cfg, opts.PinOverridesPath); err != nil { return nil, fmt.Errorf("applying pin overrides: %w", err) @@ -113,6 +128,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { } var content string for _, t := range orchTargets { + t.Gen.setInstallMode(installMode) content, err = t.Gen.Generate() if err != nil { return nil, fmt.Errorf("generating orchestrate workflow: %w", err) @@ -125,7 +141,9 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // out to one promote-.yaml per component; otherwise a single // promote.yaml, byte-identical to today. if cfg.IsSingleEnvironment() { - content, err = NewReleaseGenerator(cfg, baseDir).Generate() + relGen := NewReleaseGenerator(cfg, baseDir) + relGen.setInstallMode(installMode) + content, err = relGen.Generate() if err != nil { return nil, fmt.Errorf("generating release workflow: %w", err) } @@ -136,6 +154,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { return nil, perr } for _, t := range promoteTargets { + t.Gen.setInstallMode(installMode) content, err = t.Gen.Generate() if err != nil { return nil, fmt.Errorf("generating promote workflow: %w", err) @@ -146,7 +165,9 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // 3. external-update -> .github/workflows/external-update.yaml when primary. if cfg.IsPrimary() { - content, err = NewExternalUpdateGenerator(cfg, baseDir).Generate() + extGen := NewExternalUpdateGenerator(cfg, baseDir) + extGen.setInstallMode(installMode) + content, err = extGen.Generate() if err != nil { return nil, fmt.Errorf("generating external-update workflow: %w", err) } @@ -155,6 +176,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // 4. validate-check -> .github/workflows/cascade-validate.yaml when enabled. if gen := NewValidateCheckGenerator(cfg, baseDir); gen.Enabled() { + gen.setInstallMode(installMode) content, err = gen.Generate() if err != nil { return nil, fmt.Errorf("generating validate-check workflow: %w", err) @@ -164,6 +186,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // 5. merge-queue -> .github/workflows/cascade-merge-queue.yaml when enabled. if gen := NewMergeQueueGenerator(cfg, baseDir); gen.Enabled() { + gen.setInstallMode(installMode) content, err = gen.Generate() if err != nil { return nil, fmt.Errorf("generating merge-queue workflow: %w", err) @@ -179,6 +202,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { return nil, err } for _, t := range hfTargets { + t.Gen.setInstallMode(installMode) content, err = t.Gen.Generate() if err != nil { return nil, fmt.Errorf("generating hotfix workflow: %w", err) @@ -194,6 +218,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { return nil, err } for _, t := range rbTargets { + t.Gen.setInstallMode(installMode) content, err = t.Gen.Generate() if err != nil { return nil, fmt.Errorf("generating rollback workflow: %w", err) @@ -203,7 +228,9 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // 8. pr-preview -> .github/workflows/cascade-pr-preview.yaml when enabled. if cfg.PRPreview.IsEnabled() { - content, err = NewPRPreviewGenerator(cfg, baseDir).Generate() + previewGen := NewPRPreviewGenerator(cfg, baseDir) + previewGen.setInstallMode(installMode) + content, err = previewGen.Generate() if err != nil { return nil, fmt.Errorf("generating pr-preview workflow: %w", err) } @@ -213,6 +240,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // 9. drift-check -> .github/workflows/cascade-drift-check.yaml when enabled, // plus the fork-safe comment companion when drift_check.comment is set. if gen := NewDriftCheckGenerator(cfg, baseDir); gen.Enabled() { + gen.setInstallMode(installMode) content, err = gen.Generate() if err != nil { return nil, fmt.Errorf("generating drift-check workflow: %w", err) @@ -234,6 +262,7 @@ func Plan(opts PlanOptions) ([]PlannedFile, error) { // sees the same two files generate writes and reports no drift on a clean // tree. if gen := NewReconcileGenerator(cfg, baseDir); gen.Enabled() { + gen.setInstallMode(installMode) content, err = gen.Generate() if err != nil { return nil, fmt.Errorf("generating reconcile-check workflow: %w", err) diff --git a/internal/verify/command.go b/internal/verify/command.go index 0056842e..f3660f19 100644 --- a/internal/verify/command.go +++ b/internal/verify/command.go @@ -38,6 +38,7 @@ verify is read-only: it never writes files, runs git, or modifies the repo.`, cmd.Flags().BoolVarP(&o.Quiet, "quiet", "q", false, "Suppress the per-file report body; only set the exit code") cmd.Flags().BoolVar(&o.AllowOrphans, "allow-orphans", false, "Do not report cascade-owned workflow files that are no longer in the plan as drift") cmd.Flags().BoolVar(&o.OwnRepo, "own-repo", false, "Verify against cascade's own-repo release-plumbing variant (tag-only manage-release, non-triggering tag-create). Maintainer-only; never used by a downstream manifest.") + cmd.Flags().StringVar(&o.CLIInstall, "cli-install", "action", "How the committed workflows install the cascade CLI: \"action\" (setup-cli composite action, default) or \"binary\" (self-contained install). Must match the mode generate-workflow --cli-install used, or every Setup CLI step reports as spurious drift.") return cmd } diff --git a/internal/verify/verify.go b/internal/verify/verify.go index a65457b3..a10e504e 100644 --- a/internal/verify/verify.go +++ b/internal/verify/verify.go @@ -78,6 +78,11 @@ type Options struct { // the own-repo output; without this, verify would compute the plain variant // and report the deliberate own-repo differences as spurious drift. OwnRepo bool + // CLIInstall mirrors generate-workflow's --cli-install flag ("" / "action" + // or "binary"). It must match the mode the committed files were actually + // generated with, or every planned file whose Setup CLI step differs by + // mode reports as spurious drift. + CLIInstall string } // Run compares every file the manifest would generate against the bytes @@ -103,6 +108,7 @@ func Run(o Options, stdout, stderr io.Writer) error { OutputPath: o.OutputPath, PromoteOutputPath: o.PromoteOutputPath, OwnRepo: o.OwnRepo, + CLIInstall: o.CLIInstall, }) if err != nil { return operational(fmt.Errorf("planning workflows: %w", err)) diff --git a/internal/verify/verify_test.go b/internal/verify/verify_test.go index 2695235b..e7e355cf 100644 --- a/internal/verify/verify_test.go +++ b/internal/verify/verify_test.go @@ -71,27 +71,89 @@ func newRepo(t *testing.T) string { // path is absolute so Plan resolves the manifest, base directory, and emitted // files without consulting the process working directory. func planOpts(dir string) generate.PlanOptions { + return planOptsWithCLIInstall(dir, "") +} + +// planOptsWithCLIInstall is planOpts with an explicit --cli-install mode, for +// tests that need to plan (or re-plan) a repo generated in binary mode. +func planOptsWithCLIInstall(dir, cliInstall string) generate.PlanOptions { return generate.PlanOptions{ ConfigPath: filepath.Join(dir, ".github", "manifest.yaml"), ManifestKey: config.DefaultManifestKey, ActionFolder: "manage-release", OutputPath: filepath.Join(dir, ".github", "workflows", "orchestrate.yaml"), PromoteOutputPath: filepath.Join(dir, ".github", "workflows", "promote.yaml"), + CLIInstall: cliInstall, } } // opts builds the verify options for a repo rooted at dir, mirroring planOpts so // the verify run reads the same absolute paths the plan emitted. func opts(dir string) Options { + return optsWithCLIInstall(dir, "") +} + +// optsWithCLIInstall is opts with an explicit --cli-install mode, mirroring +// planOptsWithCLIInstall so a verify Run reads back what a matching Plan wrote. +func optsWithCLIInstall(dir, cliInstall string) Options { return Options{ ConfigPath: filepath.Join(dir, ".github", "manifest.yaml"), ManifestKey: config.DefaultManifestKey, ActionFolder: "manage-release", OutputPath: filepath.Join(dir, ".github", "workflows", "orchestrate.yaml"), PromoteOutputPath: filepath.Join(dir, ".github", "workflows", "promote.yaml"), + CLIInstall: cliInstall, } } +// newRepoWithCLIInstall mirrors newRepo but plans and materializes the repo +// using the given --cli-install mode, so tests can build a binary-mode +// generated fixture the same way a real adopter's repo would look. +func newRepoWithCLIInstall(t *testing.T, cliInstall string) string { + t.Helper() + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".github", "workflows"), 0o755)) + + stubs := map[string]string{ + ".github/workflows/image-build.yaml": "" + + "name: Image Build\non:\n workflow_call:\n inputs:\n os:\n type: string\n", + ".github/workflows/bundle-build.yaml": "" + + "name: Bundle Build\non:\n workflow_call:\n inputs:\n image:\n type: string\n", + ".github/workflows/deploy.yaml": "" + + "name: Deploy\non:\n workflow_call:\n inputs:\n environment:\n type: string\n", + } + for path, body := range stubs { + require.NoError(t, os.WriteFile(filepath.Join(dir, path), []byte(body), 0o644)) + } + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: config.EnvNames("dev", "staging", "prod"), + Builds: []config.BuildConfig{ + {Name: "image", Workflow: ".github/workflows/image-build.yaml", Triggers: []string{"src/**"}}, + }, + Deploys: []config.DeployConfig{ + {Name: "app", Workflow: ".github/workflows/deploy.yaml", Triggers: []string{"src/**"}, DependsOn: []string{"image"}}, + }, + } + manifest := map[string]any{config.DefaultManifestKey: config.CICDFile{Config: cfg}} + body, err := yaml.Marshal(manifest) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, ".github", "manifest.yaml"), body, 0o644)) + + planned, err := generate.Plan(planOptsWithCLIInstall(dir, cliInstall)) + require.NoError(t, err) + for _, p := range planned { + path := p.Path + if !filepath.IsAbs(path) { + path = filepath.Join(dir, path) + } + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(p.Content), 0o644)) + } + return dir +} + func TestRun_CleanRepo_NoDrift(t *testing.T) { t.Parallel() dir := newRepo(t) @@ -349,3 +411,33 @@ func TestErrDrift_ExitCodeOne(t *testing.T) { require.ErrorAs(t, error(ErrDrift), &ec) require.Equal(t, 1, ec.ExitCode()) } + +// TestRun_CLIInstallBinary_NoDriftWhenModeMatches proves verify can correctly +// check a repo generated with --cli-install=binary: without CLIInstall wired +// through to the underlying Plan, every generator silently defaults to action +// mode internally, so a binary-mode repo would report spurious drift on every +// file whose Setup CLI step differs by mode even though nothing is out of +// sync. +func TestRun_CLIInstallBinary_NoDriftWhenModeMatches(t *testing.T) { + t.Parallel() + dir := newRepoWithCLIInstall(t, "binary") + + var out, errOut bytes.Buffer + err := Run(optsWithCLIInstall(dir, "binary"), &out, &errOut) + require.NoError(t, err, "a clean binary-mode repo must verify with no drift when CLIInstall matches; report:\n%s", errOut.String()) + require.Contains(t, out.String(), "no drift") +} + +// TestRun_CLIInstallBinary_MismatchedModeIsRealDrift is the control for the +// test above: a binary-mode repo checked WITHOUT CLIInstall set (defaulting to +// action) must still report drift, proving the two modes produce genuinely +// different bytes and the prior test isn't passing by coincidence. +func TestRun_CLIInstallBinary_MismatchedModeIsRealDrift(t *testing.T) { + t.Parallel() + dir := newRepoWithCLIInstall(t, "binary") + + var out, errOut bytes.Buffer + err := Run(opts(dir), &out, &errOut) // opts(dir) leaves CLIInstall unset (action) + require.Error(t, err) + require.True(t, errors.Is(err, ErrDrift), "action-mode verify against a binary-mode repo must report drift, got %v", err) +}