diff --git a/internal/cli/upload_test.go b/internal/cli/upload_test.go index 6f226192..40586f47 100644 --- a/internal/cli/upload_test.go +++ b/internal/cli/upload_test.go @@ -1746,11 +1746,11 @@ func TestRunUploadEmitsReusableInputFailuresAsActionableFailingSteps(t *testing. if len(pipeline.Steps) != 1 || !isGeneratedFailureCommand(pipeline.Steps[0].Command) { t.Fatalf("reusable input failure pipeline = %#v", pipeline.Steps) } - primary := `Reusable workflow input "target" uses an unsupported needs expression. Use exactly needs..outputs. for a string input.` + primary := `Reusable workflow input "target" uses a needs expression in an unsupported form. Pass the whole value as exactly ${{ needs..outputs. }}, with nothing around it. Only string inputs can take a needs value, and Buildkite resolves it before the called job runs, so the reference has to be the entire value rather than part of a larger expression. If you need a computed input from job outputs, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.` detail := `Reusable-workflow input "target" is not statically resolvable: unsupported compile-time context "needs"` message := string(failureArtifactForStep(pipeline.Steps[0].Plugins, runner.uploaded, "messages")) annotation := string(failureArtifactForStep(pipeline.Steps[0].Plugins, runner.uploaded, "annotations")) - if !strings.Contains(message, primary) || !strings.Contains(message, "detail: "+detail) || !strings.Contains(annotation, "Reusable workflow input "target" uses an unsupported needs expression.") || !strings.Contains(annotation, "Use exactly needs.<job>.outputs.<name>") || !strings.Contains(annotation, strings.ReplaceAll(detail, `"`, """)) || len(pipeline.Steps[0].Notify) != 1 || strings.Contains(pipeline.Steps[0].Notify[0].GitHubCheck.Output.Summary, "Reusable workflow input "target" uses a needs expression in an unsupported form.") || !strings.Contains(annotation, "Pass the whole value as exactly ${{ needs.<job>.outputs.<name> }}") || !strings.Contains(annotation, "github.com/buildkite/buildkite-gha") || !strings.Contains(annotation, strings.ReplaceAll(detail, `"`, """)) || len(pipeline.Steps[0].Notify) != 1 || strings.Contains(pipeline.Steps[0].Notify[0].GitHubCheck.Output.Summary, "> "$GITHUB_OUTPUT" + call: + needs: prepare + uses: ./.github/workflows/reusable.yml + with: + target: prefix-${{ needs.prepare.outputs.target }} +`) + writeWorkflow(t, repository, "reusable.yml", "on:\n workflow_call:\n inputs:\n target:\n type: string\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - run: true\n") + _, err := Compile(path, readFile(t, path), readFile(t, smokePath("events", "push.json"))) + if err == nil { + t.Fatal("Compile() error = nil, want unsupported needs input finding") + } + var finding *ProcessingFinding + wantMessage := `Reusable workflow input "target" uses a needs expression in an unsupported form. Pass the whole value as exactly ${{ needs..outputs. }}, with nothing around it. Only string inputs can take a needs value, and Buildkite resolves it before the called job runs, so the reference has to be the entire value rather than part of a larger expression. If you need a computed input from job outputs, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.` + wantDetail := `Reusable-workflow input "target" is not statically resolvable: unsupported compile-time context "needs"` + if !errors.As(err, &finding) || finding.Message != wantMessage || finding.Detail != wantDetail || finding.Path != "./.github/workflows/caller.yml" || finding.Line != 14 || finding.Column != 15 || finding.Job != "call" { + t.Fatalf("Compile() finding = %#v", finding) + } + }) + + t.Run("value unavailable before jobs run", func(t *testing.T) { + repository := t.TempDir() + path := writeWorkflow(t, repository, "caller.yml", `on: push +jobs: + call: + uses: ./.github/workflows/reusable.yml + with: + target: ${{ steps.prepare.outputs.target }} +`) + writeWorkflow(t, repository, "reusable.yml", "on:\n workflow_call:\n inputs:\n target:\n type: string\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - run: true\n") + _, err := Compile(path, readFile(t, path), readFile(t, smokePath("events", "push.json"))) + var finding *ProcessingFinding + wantMessage := `Reusable workflow input "target" uses a value that is unavailable before jobs run. Replace it with a literal or an expression that does not depend on job results.` + if err == nil || !errors.As(err, &finding) || finding.Message != wantMessage { + t.Fatalf("Compile() finding = %#v", finding) + } + }) + t.Run("call condition", func(t *testing.T) { repository := t.TempDir() path := writeWorkflow(t, repository, "caller.yml", "on: push\njobs:\n call:\n if: github.ref == 'refs/heads/main'\n uses: ./.github/workflows/reusable.yml\n") diff --git a/internal/compiler/reusable.go b/internal/compiler/reusable.go index c3933996..3747edeb 100644 --- a/internal/compiler/reusable.go +++ b/internal/compiler/reusable.go @@ -308,10 +308,26 @@ func (resolver *reusableResolver) resolve(ctx context.Context, current reusableW } calleeSource, source, err := resolver.loadReusableWorkflow(ctx, current, call.Uses) if err != nil { + var finding *ProcessingFinding + if errors.As(err, &finding) { + attributed := *finding + attributed.Path, attributed.Line, attributed.Column, attributed.Job = path, call.Span.Start.Line, call.Span.Start.Column, job.ID + attributed.Err = locatedJobWrappedError(path, job, call.Span.Start.Line, call.Span.Start.Column, "", finding.Err) + return reusableResolution{}, &attributed + } return reusableResolution{}, locatedJobWrappedError(path, job, call.Span.Start.Line, call.Span.Start.Column, "", err) } if (call.InheritSecrets || len(call.Secrets) != 0) && calleeSource.identity.kind != "workspace" { - return reusableResolution{}, locatedJobError(path, job, call.Span.Start.Line, call.Span.Start.Column, "secret forwarding is supported only for repository-local reusable workflows") + message := fmt.Sprintf("A secrets: map cannot forward secrets to a workflow in another repository. Reusable workflow %q is outside this repository, so no secrets were forwarded. Retrieve each secret by name with buildkite-agent secret get NAME in the jobs of that workflow, or copy the workflow into this repository's .github/workflows and use a secrets: map with a ./ call. If you need explicit secret mappings across repositories, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.", calleeSource.displayPath) + if call.InheritSecrets { + message = fmt.Sprintf("secrets: inherit cannot forward secrets to a workflow in another repository. Reusable workflow %q is outside this repository, so no secrets were forwarded. Retrieve each secret by name with buildkite-agent secret get NAME in the jobs of that workflow, or copy the workflow into this repository's .github/workflows and use secrets: inherit with a ./ call. If you need secrets: inherit across repositories, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.", calleeSource.displayPath) + } + return reusableResolution{}, &ProcessingFinding{ + Stage: StageGraph, Code: CodeGraphInvalid, Category: "compatibility", + Path: path, Line: call.Span.Start.Line, Column: call.Span.Start.Column, Job: job.ID, + Message: message, + Err: locatedJobError(path, job, call.Span.Start.Line, call.Span.Start.Column, message), + } } if cycle := resolver.cycle(calleeSource); cycle != "" { return reusableResolution{}, locatedJobError(path, job, call.Span.Start.Line, call.Span.Start.Column, "reusable-workflow cycle detected: "+cycle) @@ -750,7 +766,7 @@ func resolveCallInputs(path string, job workflow.Job, call *workflow.ReusableWor if need, _, ok := deferredNeedReference(text); ok { message = fmt.Sprintf("Reusable workflow input %q references job %q, but the call does not list it in needs. Add %q to the reusable-workflow call's needs.", name, need, need) } else if strings.Contains(err.Error(), `unsupported compile-time context "needs"`) { - message = fmt.Sprintf("Reusable workflow input %q uses an unsupported needs expression. Use exactly needs..outputs. for a string input.", name) + message = fmt.Sprintf("Reusable workflow input %q uses a needs expression in an unsupported form. Pass the whole value as exactly ${{ needs..outputs. }}, with nothing around it. Only string inputs can take a needs value, and Buildkite resolves it before the called job runs, so the reference has to be the entire value rather than part of a larger expression. If you need a computed input from job outputs, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.", name) } return reusableInputs{}, &ProcessingFinding{ Stage: StageGraph, Code: CodeGraphInvalid, Category: "compatibility", diff --git a/internal/compiler/reusable_remote_test.go b/internal/compiler/reusable_remote_test.go index 20359b38..56ebfec1 100644 --- a/internal/compiler/reusable_remote_test.go +++ b/internal/compiler/reusable_remote_test.go @@ -181,26 +181,30 @@ jobs: } } -func TestCompileRejectsSecretInheritanceIntoRemoteReusableWorkflows(t *testing.T) { +func TestCompileRejectsSecretForwardingIntoRemoteReusableWorkflows(t *testing.T) { for _, test := range []struct { - name string - caller string - remote string + name string + caller string + remote string + wantMessage string }{ { - name: "direct remote call", - caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n secrets: inherit\n", - remote: "on: workflow_call\njobs:\n test:\n runs-on: ubuntu-latest\n steps: [{run: true}]\n", + name: "inherited remote call", + caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n secrets: inherit\n", + remote: "on: workflow_call\njobs:\n test:\n runs-on: ubuntu-latest\n steps: [{run: true}]\n", + wantMessage: `secrets: inherit cannot forward secrets to a workflow in another repository. Reusable workflow "owner/workflows/.github/workflows/ci.yml@v1" is outside this repository, so no secrets were forwarded. Retrieve each secret by name with buildkite-agent secret get NAME in the jobs of that workflow, or copy the workflow into this repository's .github/workflows and use secrets: inherit with a ./ call. If you need secrets: inherit across repositories, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`, }, { - name: "explicit remote call", - caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n secrets:\n token: ${{ secrets.SOURCE }}\n", - remote: "on:\n workflow_call:\n secrets:\n token:\njobs:\n test:\n runs-on: ubuntu-latest\n steps: [{run: true}]\n", + name: "explicit remote call", + caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n secrets:\n token: ${{ secrets.SOURCE }}\n", + remote: "on:\n workflow_call:\n secrets:\n token:\njobs:\n test:\n runs-on: ubuntu-latest\n steps: [{run: true}]\n", + wantMessage: `A secrets: map cannot forward secrets to a workflow in another repository. Reusable workflow "owner/workflows/.github/workflows/ci.yml@v1" is outside this repository, so no secrets were forwarded. Retrieve each secret by name with buildkite-agent secret get NAME in the jobs of that workflow, or copy the workflow into this repository's .github/workflows and use a secrets: map with a ./ call. If you need explicit secret mappings across repositories, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`, }, { - name: "local path within remote repository", - caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n", - remote: "on: workflow_call\njobs:\n nested:\n uses: ./.github/workflows/nested.yml\n secrets: inherit\n", + name: "local path within remote repository", + caller: "on: push\njobs:\n call:\n uses: owner/workflows/.github/workflows/ci.yml@v1\n", + remote: "on: workflow_call\njobs:\n nested:\n uses: ./.github/workflows/nested.yml\n secrets: inherit\n", + wantMessage: `secrets: inherit cannot forward secrets to a workflow in another repository. Reusable workflow "owner/workflows/.github/workflows/nested.yml@v1" is outside this repository, so no secrets were forwarded. Retrieve each secret by name with buildkite-agent secret get NAME in the jobs of that workflow, or copy the workflow into this repository's .github/workflows and use secrets: inherit with a ./ call. If you need secrets: inherit across repositories, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`, }, } { t.Run(test.name, func(t *testing.T) { @@ -212,8 +216,12 @@ func TestCompileRejectsSecretInheritanceIntoRemoteReusableWorkflows(t *testing.T options := defaultOptions() options.RepositorySource = MemoizeRepositorySource(newFakeReusableRepositorySource(t, map[string]string{"owner/workflows": remoteRoot})) _, err := CompileWithOptions(callerPath, readFile(t, callerPath), pushEvent(t), options) - if err == nil || !strings.Contains(err.Error(), "secret forwarding is supported only for repository-local reusable workflows") { - t.Fatalf("CompileWithOptions() error = %v, want remote inheritance rejection", err) + var finding *ProcessingFinding + if err == nil || !errors.As(err, &finding) { + t.Fatalf("CompileWithOptions() error = %v, want remote forwarding finding", err) + } + if finding.Message != test.wantMessage || finding.Detail != "" || finding.Path != "./.github/workflows/caller.yml" || finding.Line != 4 || finding.Column != 11 || finding.Job != "call" { + t.Fatalf("CompileWithOptions() finding = %#v", finding) } }) } @@ -446,11 +454,15 @@ func TestCompileRemoteReusableWorkflowLimitsAndDiagnostics(t *testing.T) { }) for _, test := range []struct { - name string - uses string - want string + name string + uses string + want string + wantMessage string }{ - {name: "dynamic", uses: "${{ inputs.workflow }}", want: "runtime-dependent"}, + { + name: "dynamic", uses: "${{ inputs.workflow }}", want: "reusable workflow path cannot be an expression", + wantMessage: `Reusable workflow path cannot be an expression. "${{ inputs.workflow }}" is only known once the build is running, and the workflow file has to be read before that. Name the file directly, for example ./.github/workflows/ci.yml, or org/shared/.github/workflows/ci.yml@v1. If you need a computed workflow path, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.`, + }, {name: "nested path", uses: "owner/repo/.github/workflows/nested/ci.yml@v1", want: "directly under .github/workflows"}, {name: "wrong directory", uses: "owner/repo/workflows/ci.yml@v1", want: "directly under .github/workflows"}, {name: "non YAML", uses: "owner/repo/.github/workflows/ci.json@v1", want: "must end in .yml or .yaml"}, @@ -465,6 +477,12 @@ func TestCompileRemoteReusableWorkflowLimitsAndDiagnostics(t *testing.T) { if err == nil || !strings.Contains(err.Error(), test.want) || len(fake.references()) != 0 { t.Fatalf("CompileWithOptions() error/calls = %v / %#v, want %q before source access", err, fake.references(), test.want) } + if test.wantMessage != "" { + var finding *ProcessingFinding + if !errors.As(err, &finding) || finding.Message != test.wantMessage || finding.Detail != "" || finding.Path != "./.github/workflows/dynamic.yml" || finding.Line != 4 || finding.Column != 11 || finding.Job != "call" { + t.Fatalf("CompileWithOptions() finding = %#v", finding) + } + } }) } @@ -475,9 +493,30 @@ func TestCompileRemoteReusableWorkflowLimitsAndDiagnostics(t *testing.T) { options := defaultOptions() options.RepositorySource = MemoizeRepositorySource(fake) _, err := CompileWithOptions(callerPath, readFile(t, callerPath), event, options) - if err == nil || !strings.Contains(err.Error(), `public reusable workflow "owner/private/.github/workflows/ci.yml@v1" was not found or is not public`) { + if err == nil || !strings.Contains(err.Error(), `public reusable workflow "owner/private/.github/workflows/ci.yml@v1" could not be read`) { t.Fatalf("CompileWithOptions() error = %v, want non-enumerating source error", err) } + var finding *ProcessingFinding + wantMessage := `Reusable workflow could not be read. "owner/private/.github/workflows/ci.yml@v1" is either private or does not exist. Only public workflows can be called across repositories. Check the path, or copy the workflow into this repository's .github/workflows and call it with a ./ path. If you need private cross-repository calls, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.` + if !errors.As(err, &finding) || finding.Message != wantMessage || finding.Detail != "" || finding.Path != "./.github/workflows/private.yml" || finding.Line != 4 || finding.Column != 11 || finding.Job != "call" { + t.Fatalf("CompileWithOptions() finding = %#v", finding) + } + }) + + t.Run("missing workflow in public repository", func(t *testing.T) { + callerPath := writeWorkflow(t, callerRoot, "missing.yml", "on: push\njobs:\n call:\n uses: owner/public/.github/workflows/absent.yml@v1\n") + publicRoot := t.TempDir() + options := defaultOptions() + options.RepositorySource = MemoizeRepositorySource(newFakeReusableRepositorySource(t, map[string]string{"owner/public": publicRoot})) + _, err := CompileWithOptions(callerPath, readFile(t, callerPath), event, options) + if err == nil || !strings.Contains(err.Error(), `public reusable workflow "owner/public/.github/workflows/absent.yml@v1" could not be read`) { + t.Fatalf("CompileWithOptions() error = %v, want non-enumerating missing workflow error", err) + } + var finding *ProcessingFinding + wantMessage := `Reusable workflow could not be read. "owner/public/.github/workflows/absent.yml@v1" is either private or does not exist. Only public workflows can be called across repositories. Check the path, or copy the workflow into this repository's .github/workflows and call it with a ./ path. If you need private cross-repository calls, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.` + if !errors.As(err, &finding) || finding.Message != wantMessage || finding.Detail != "" || finding.Path != "./.github/workflows/missing.yml" || finding.Line != 4 || finding.Column != 11 || finding.Job != "call" { + t.Fatalf("CompileWithOptions() finding = %#v", finding) + } }) t.Run("cancellation", func(t *testing.T) { diff --git a/internal/compiler/reusable_source.go b/internal/compiler/reusable_source.go index 5a85da92..ce777c8c 100644 --- a/internal/compiler/reusable_source.go +++ b/internal/compiler/reusable_source.go @@ -82,7 +82,11 @@ func localReusableWorkflowSource(workflowPath string) (reusableWorkflowSource, e func (resolver *reusableResolver) loadReusableWorkflow(ctx context.Context, parent reusableWorkflowSource, uses string) (reusableWorkflowSource, []byte, error) { if strings.Contains(uses, "${{") { - return reusableWorkflowSource{}, nil, fmt.Errorf("reusable workflow %q is runtime-dependent; only literal local or public GitHub references are supported", uses) + return reusableWorkflowSource{}, nil, &ProcessingFinding{ + Stage: StageGraph, Code: CodeGraphInvalid, Category: "compatibility", + Message: fmt.Sprintf("Reusable workflow path cannot be an expression. %q is only known once the build is running, and the workflow file has to be read before that. Name the file directly, for example ./.github/workflows/ci.yml, or org/shared/.github/workflows/ci.yml@v1. If you need a computed workflow path, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.", uses), + Err: fmt.Errorf("reusable workflow path cannot be an expression: %q", uses), + } } if strings.HasPrefix(uses, "./") { return resolver.loadLocalReusableWorkflow(parent, uses) @@ -153,7 +157,7 @@ func (resolver *reusableResolver) loadRemoteReusableWorkflow(ctx context.Context if err != nil { var notPublic *actionsource.NotPublicError if errors.As(err, ¬Public) { - return reusableWorkflowSource{}, nil, fmt.Errorf("public reusable workflow %q was not found or is not public", uses) + return reusableWorkflowSource{}, nil, unavailablePublicReusableWorkflowError(uses) } return reusableWorkflowSource{}, nil, fmt.Errorf("resolve public reusable workflow %q: %w", uses, err) } @@ -175,6 +179,9 @@ func (resolver *reusableResolver) loadRemoteReusableWorkflow(ctx context.Context } info, err := os.Lstat(filePath) if err != nil || !info.Mode().IsRegular() { + if errors.Is(err, os.ErrNotExist) { + return reusableWorkflowSource{}, nil, unavailablePublicReusableWorkflowError(uses) + } if err == nil { err = fmt.Errorf("selected workflow is not a regular file") } @@ -196,6 +203,14 @@ func (resolver *reusableResolver) loadRemoteReusableWorkflow(ctx context.Context }, source, nil } +func unavailablePublicReusableWorkflowError(uses string) error { + return &ProcessingFinding{ + Stage: StageGraph, Code: CodeGraphInvalid, Category: "compatibility", + Message: fmt.Sprintf("Reusable workflow could not be read. %q is either private or does not exist. Only public workflows can be called across repositories. Check the path, or copy the workflow into this repository's .github/workflows and call it with a ./ path. If you need private cross-repository calls, log an issue on github.com/buildkite/buildkite-gha so we can prioritise it.", uses), + Err: fmt.Errorf("public reusable workflow %q could not be read", uses), + } +} + func reusableWorkflowPath(value string, requireYAML bool) (string, error) { if value == "" || strings.HasPrefix(value, "/") || strings.Contains(value, "\\") || path.Clean(value) != value || path.Dir(value) != ".github/workflows" { return "", fmt.Errorf("must name a file directly under .github/workflows")