diff --git a/e2e/harness/harness.go b/e2e/harness/harness.go index 098bf41a..3631be27 100644 --- a/e2e/harness/harness.go +++ b/e2e/harness/harness.go @@ -33,6 +33,17 @@ var ( cliBinaryErr error ) +// normalizeCallbackStubPath returns the canonical path where a callback stub +// workflow should be placed so it matches the path the generator will reference. +// Cross-repo external refs (containing "@") are skipped - they are not stubs. +// All other paths are placed under .github/workflows/ using the base filename. +func normalizeCallbackStubPath(workflow string) string { + if strings.Contains(workflow, "@") { + return "" + } + return ".github/workflows/" + filepath.Base(workflow) +} + // Harness orchestrates E2E test execution type Harness struct { t *testing.T @@ -123,16 +134,22 @@ func (h *Harness) StageRepoFromConfig(ctx context.Context, config Config) error scenarioTag := scenarioTagFromTestName(h.t.Name()) for _, build := range config.Builds { if build.Workflow != "" { - files[build.Workflow] = generateStubWorkflow(build.Name, scenarioTag) + if p := normalizeCallbackStubPath(build.Workflow); p != "" { + files[p] = generateStubWorkflow(build.Name, scenarioTag) + } } } for _, deploy := range config.Deploys { if deploy.Workflow != "" { - files[deploy.Workflow] = generateStubWorkflow(deploy.Name, scenarioTag) + if p := normalizeCallbackStubPath(deploy.Workflow); p != "" { + files[p] = generateStubWorkflow(deploy.Name, scenarioTag) + } } } if config.Publish != nil && config.Publish.Workflow != "" { - files[config.Publish.Workflow] = generatePublishStubWorkflow(scenarioTag) + if p := normalizeCallbackStubPath(config.Publish.Workflow); p != "" { + files[p] = generatePublishStubWorkflow(scenarioTag) + } } // Create mock setup-cli action that installs CLI from repo diff --git a/internal/config/parse.go b/internal/config/parse.go index b84d3e80..5a821cce 100644 --- a/internal/config/parse.go +++ b/internal/config/parse.go @@ -178,6 +178,7 @@ func Validate(cfg *TrunkConfig) []string { } // workflow XOR run: exactly one must be set. errors = append(errors, validateWorkflowRunXOR(fmt.Sprintf("builds[%d]", i), b.Workflow, b.Run, b.Shell)...) + errors = append(errors, validateLocalCallbackWorkflowPath(fmt.Sprintf("builds[%d]", i), b.Workflow)...) // Reusable-workflow callbacks cannot carry job-control fields that GHA // rejects on a jobs..uses call. matrix: is builds-only. @@ -234,6 +235,7 @@ func Validate(cfg *TrunkConfig) []string { } // workflow XOR run: exactly one must be set. errors = append(errors, validateWorkflowRunXOR(fmt.Sprintf("deploys[%d]", i), d.Workflow, d.Run, d.Shell)...) + errors = append(errors, validateLocalCallbackWorkflowPath(fmt.Sprintf("deploys[%d]", i), d.Workflow)...) // Reusable-workflow callbacks cannot carry job-control fields that GHA // rejects on a jobs..uses call. rollout: is deploys-only. @@ -286,6 +288,7 @@ func Validate(cfg *TrunkConfig) []string { if cfg.Validate != nil { v := cfg.Validate errors = append(errors, validateWorkflowRunXOR("validate", v.Workflow, v.Run, v.Shell)...) + errors = append(errors, validateLocalCallbackWorkflowPath("validate", v.Workflow)...) isReusable := v.Workflow != "" errors = append(errors, validateJobControlFields("validate", isReusable, v.RunsOn, v.Concurrency)...) errors = append(errors, validatePermissions("validate", v.Permissions)...) diff --git a/internal/config/validate_local_callback_test.go b/internal/config/validate_local_callback_test.go new file mode 100644 index 00000000..bcb9afdf --- /dev/null +++ b/internal/config/validate_local_callback_test.go @@ -0,0 +1,84 @@ +package config + +import ( + "strings" + "testing" +) + +func TestValidateLocalCallbackWorkflowPath(t *testing.T) { + t.Parallel() + tests := []struct { + name string + prefix string + workflow string + wantErrs int + wantMsg string + }{ + { + name: "empty workflow accepted", + prefix: "builds[0]", + workflow: "", + wantErrs: 0, + }, + { + name: "bare filename accepted", + prefix: "builds[0]", + workflow: "build.yaml", + wantErrs: 0, + }, + { + name: "github workflows path accepted", + prefix: "builds[0]", + workflow: ".github/workflows/x.yaml", + wantErrs: 0, + }, + { + name: "dot-slash github workflows path accepted", + prefix: "builds[0]", + workflow: "./.github/workflows/x.yaml", + wantErrs: 0, + }, + { + name: "cross-repo external ref accepted", + prefix: "builds[0]", + workflow: "owner/repo/.github/workflows/x.yml@ref", + wantErrs: 0, + }, + { + name: "local subdir path rejected", + prefix: "deploys[0]", + workflow: "ci/build.yaml", + wantErrs: 1, + wantMsg: `deploys[0]: local callback workflow must be a .github/workflows/... path or a bare filename, got "ci/build.yaml"`, + }, + { + name: "dot-slash local path rejected", + prefix: "deploys[1]", + workflow: "./build.yaml", + wantErrs: 1, + wantMsg: `deploys[1]: local callback workflow must be a .github/workflows/... path or a bare filename, got "./build.yaml"`, + }, + { + name: "github non-workflows subdir rejected", + prefix: "builds[1]", + workflow: ".github/foo/x.yaml", + wantErrs: 1, + wantMsg: `builds[1]: local callback workflow must be a .github/workflows/... path or a bare filename, got ".github/foo/x.yaml"`, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + errs := validateLocalCallbackWorkflowPath(tc.prefix, tc.workflow) + if len(errs) != tc.wantErrs { + t.Errorf("got %d errors, want %d: %v", len(errs), tc.wantErrs, errs) + return + } + if tc.wantMsg != "" { + if len(errs) == 0 || !strings.Contains(errs[0], tc.wantMsg) { + t.Errorf("error message mismatch\ngot: %q\nwant: %q", errs, tc.wantMsg) + } + } + }) + } +} diff --git a/internal/config/validate_v1.go b/internal/config/validate_v1.go index 7c1a96f8..721727c1 100644 --- a/internal/config/validate_v1.go +++ b/internal/config/validate_v1.go @@ -3,6 +3,7 @@ package config import ( "fmt" "sort" + "strings" ) // Structural validation for the v1 reserved-shape fields. These rules are the @@ -96,6 +97,29 @@ func validateJobControlFields(prefix string, isReusableWorkflow bool, runsOn *Ru return errs } +// validateLocalCallbackWorkflowPath checks that a local callback workflow path +// is either a bare filename, a .github/workflows/... path, or a cross-repo +// external ref (containing "@"). Any other form is rejected because GitHub +// requires local reusable workflows to live under .github/workflows/. +func validateLocalCallbackWorkflowPath(prefix, workflow string) []string { + if workflow == "" { + return nil + } + // Cross-repo external refs contain "@" - always valid. + if strings.Contains(workflow, "@") { + return nil + } + // Bare filename (no "/") - valid; normalizeWorkflowPath will route it. + if !strings.Contains(workflow, "/") { + return nil + } + // .github/workflows/... path - valid. + if strings.HasPrefix(workflow, ".github/workflows/") || strings.HasPrefix(workflow, "./.github/workflows/") { + return nil + } + return []string{fmt.Sprintf("%s: local callback workflow must be a .github/workflows/... path or a bare filename, got %q", prefix, workflow)} +} + // validatePermissions checks permission scope keys and values. func validatePermissions(prefix string, perms map[string]string) []string { var errs []string diff --git a/internal/generate/generator.go b/internal/generate/generator.go index 2543715f..74845a82 100644 --- a/internal/generate/generator.go +++ b/internal/generate/generator.go @@ -18,12 +18,32 @@ import ( // hold a runner for six hours. Override per manifest via config.job_timeout_minutes. const DefaultJobTimeoutMinutes = 30 -// normalizeWorkflowPath adds ./ prefix to local workflow paths (required by GitHub Actions) +// normalizeWorkflowPath returns a GitHub-valid workflow path for a local callback. +// Cross-repo external refs (containing "@") are returned unchanged. +// Paths already under ./.github/workflows/ are returned unchanged. +// Paths starting with .github/workflows/ get the ./ prefix. +// Bare filenames (no "/") and any other local path are routed to +// ./.github/workflows/, which is where GitHub requires local reusable +// workflows to live. func normalizeWorkflowPath(path string) string { + // Cross-repo external refs contain "@" - leave them as-is. + if strings.Contains(path, "@") { + return path + } + // Already fully normalized. + if strings.HasPrefix(path, "./.github/workflows/") { + return path + } + // .github/workflows/x.yaml -> ./.github/workflows/x.yaml + if strings.HasPrefix(path, ".github/workflows/") { + return "./" + path + } + // .github// - legacy edge case, prepend ./ (prior behavior). if strings.HasPrefix(path, ".github/") { return "./" + path } - return path + // Bare filename or any other local path: route to canonical location. + return "./.github/workflows/" + filepath.Base(path) } // envGHAName returns the GitHub Environment name for a given cascade environment @@ -479,7 +499,11 @@ func (g *Generator) discoverOutputsAndInputs() error { continue } - path := filepath.Join(g.baseDir, cb.workflow) + // Read the stub from the normalized location so a bare filename + // (build.yaml) resolves to .github/workflows/build.yaml, which is where + // GitHub requires local reusable workflows to live and where the emitted + // uses: reference points. + path := filepath.Join(g.baseDir, normalizeWorkflowPath(cb.workflow)) data, err := os.ReadFile(path) if err != nil { return fmt.Errorf("reading workflow %s: %w", cb.workflow, err) diff --git a/internal/generate/generator_normalize_test.go b/internal/generate/generator_normalize_test.go new file mode 100644 index 00000000..63de9578 --- /dev/null +++ b/internal/generate/generator_normalize_test.go @@ -0,0 +1,112 @@ +package generate + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stablekernel/cascade/internal/config" +) + +func TestNormalizeWorkflowPath(t *testing.T) { + t.Parallel() + tests := []struct { + name string + input string + want string + }{ + { + name: "bare filename normalizes to github workflows dir", + input: "build.yaml", + want: "./.github/workflows/build.yaml", + }, + { + name: "github workflows path gets dot-slash prefix", + input: ".github/workflows/x.yaml", + want: "./.github/workflows/x.yaml", + }, + { + name: "already normalized path unchanged", + input: "./.github/workflows/x.yaml", + want: "./.github/workflows/x.yaml", + }, + { + name: "cross-repo external ref unchanged", + input: "owner/repo/.github/workflows/x.yml@ref", + want: "owner/repo/.github/workflows/x.yml@ref", + }, + { + name: "org satellite cross-repo ref unchanged", + input: "org/satellite/.github/workflows/deploy.yaml@v1", + want: "org/satellite/.github/workflows/deploy.yaml@v1", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := normalizeWorkflowPath(tc.input) + if got != tc.want { + t.Errorf("normalizeWorkflowPath(%q) = %q, want %q", tc.input, got, tc.want) + } + }) + } +} + +func TestNormalizeWorkflowPath_ActionlintClean(t *testing.T) { + // Verify that a bare-path build callback generates an actionlint-clean workflow. + actionlint, err := exec.LookPath("actionlint") + if err != nil { + // Try the known homebrew path. + actionlint = "/opt/homebrew/bin/actionlint" + if _, statErr := os.Stat(actionlint); statErr != nil { + t.Skip("actionlint not found; skipping actionlint integration test") + } + } + + // Stage bare-filename stub callback workflows at the normalized location + // (.github/workflows/), which is where the generator discovers + // inputs/outputs from and emits the uses: reference to. + dir := t.TempDir() + wfDir := filepath.Join(dir, ".github", "workflows") + if mkErr := os.MkdirAll(wfDir, 0o755); mkErr != nil { + t.Fatalf("MkdirAll: %v", mkErr) + } + stub := []byte("on:\n workflow_call:\n") + if writeErr := os.WriteFile(filepath.Join(wfDir, "build.yaml"), stub, 0o644); writeErr != nil { + t.Fatalf("WriteFile build stub: %v", writeErr) + } + if writeErr := os.WriteFile(filepath.Join(wfDir, "deploy.yaml"), stub, 0o644); writeErr != nil { + t.Fatalf("WriteFile deploy stub: %v", writeErr) + } + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"staging", "production"}, + Builds: []config.BuildConfig{ + {Name: "app", Workflow: "build.yaml", Triggers: []string{"src/**"}}, + }, + Deploys: []config.DeployConfig{ + {Name: "app", Workflow: "deploy.yaml", DependsOn: []string{"app"}}, + }, + } + + g := NewGenerator(cfg, dir) + wf, err := g.Generate() + if err != nil { + t.Fatalf("Generate: %v", err) + } + + path := filepath.Join(wfDir, "orchestrate.yaml") + if writeErr := os.WriteFile(path, []byte(wf), 0o644); writeErr != nil { + t.Fatalf("WriteFile: %v", writeErr) + } + + // Disable the shellcheck integration: this test governs workflow structure + // and uses: reference validity, not the style of cascade-owned run: scripts + // (which carry their own pre-existing SC2129-style notes). + out, runErr := exec.Command(actionlint, "-shellcheck=", path).CombinedOutput() + if runErr != nil { + t.Errorf("actionlint found errors in generated workflow:\n%s", out) + } +}