From 75cb38faa6f5b18c5b86a3ecfde7b1b41cdbddcf Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Wed, 31 Dec 2025 19:04:35 +0100 Subject: [PATCH] feat: support reusable workflow expansion when `with` or `strategy.matrix` contains ${{ needs... }} (#10647) This change allows the `with:` field of a reusable workflow to reference a previous job, such as `with: { some-input: "${{ needs.other-job.outputs.other-output }}" }`. `strategy.matrix` can also reference `${{ needs... }}`. When a job is parsed and encounters this situation, the outer job of the workflow is marked with a field `incomplete_with` (or `incomplete_matrix`), indicating to Forgejo that it can't be executed as-is and the other jobs in its `needs` list need to be completed first. And then in `job_emitter.go` when one job is completed, it checks if other jobs had a `needs` reference to it and unblocks those jobs -- but if they're marked with `incomplete_with` then they can be sent back through the job parser, with the now-available job outputs, to be expanded into the correct definition of the job. The core functionality for this already exists to allow `runs-on` and `strategy.matrix` to reference the outputs of other jobs, but it is expanded upon here to include `with` for reusable workflows. There is one known defect in this implementation, but it has a limited scope -- if this code path is used to expand a nested reusable workflow, then the `${{ input.... }}` context will be incorrect. This will require an update to the jobparser in runner version 12.4.0, and so it is out-of-scope of this PR. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). - **end-to-end test:** will require the noted "known defect" to be resolved, but tests are authored at https://code.forgejo.org/forgejo/end-to-end/compare/main...mfenniak:expand-reusable-workflows-needs ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10647 Reviewed-by: Andreas Ahlenstorf Co-authored-by: Mathieu Fenniak Co-committed-by: Mathieu Fenniak --- models/actions/pre_execution_errors.go | 12 + models/actions/pre_execution_errors_test.go | 36 ++- models/actions/run.go | 2 +- models/actions/run_job.go | 22 +- models/actions/run_job_test.go | 56 +++- models/actions/run_test.go | 48 +++ options/locale_next/locale_en-US.json | 8 +- .../action_run.yml | 133 +++++++++ .../action_run_job.yml | 274 +++++++++++++++++- .../action_task_output.yml | 10 + services/actions/commit_status.go | 4 +- services/actions/commit_status_test.go | 8 +- services/actions/job_emitter.go | 145 +++++++-- services/actions/job_emitter_test.go | 268 ++++++++++++++++- services/actions/run.go | 10 +- 15 files changed, 971 insertions(+), 65 deletions(-) diff --git a/models/actions/pre_execution_errors.go b/models/actions/pre_execution_errors.go index 28cd589ef9..64c9b17727 100644 --- a/models/actions/pre_execution_errors.go +++ b/models/actions/pre_execution_errors.go @@ -26,6 +26,10 @@ const ( ErrorCodeIncompleteRunsOnMissingOutput ErrorCodeIncompleteRunsOnMissingMatrixDimension ErrorCodeIncompleteRunsOnUnknownCause + ErrorCodeIncompleteWithMissingJob + ErrorCodeIncompleteWithMissingOutput + ErrorCodeIncompleteWithMissingMatrixDimension + ErrorCodeIncompleteWithUnknownCause ) func TranslatePreExecutionError(lang translation.Locale, run *ActionRun) string { @@ -57,6 +61,14 @@ func TranslatePreExecutionError(lang translation.Locale, run *ActionRun) string return lang.TrString("actions.workflow.incomplete_runson_missing_matrix_dimension", run.PreExecutionErrorDetails...) case ErrorCodeIncompleteRunsOnUnknownCause: return lang.TrString("actions.workflow.incomplete_runson_unknown_cause", run.PreExecutionErrorDetails...) + case ErrorCodeIncompleteWithMissingJob: + return lang.TrString("actions.workflow.incomplete_with_missing_job", run.PreExecutionErrorDetails...) + case ErrorCodeIncompleteWithMissingOutput: + return lang.TrString("actions.workflow.incomplete_with_missing_output", run.PreExecutionErrorDetails...) + case ErrorCodeIncompleteWithMissingMatrixDimension: + return lang.TrString("actions.workflow.incomplete_with_missing_matrix_dimension", run.PreExecutionErrorDetails...) + case ErrorCodeIncompleteWithUnknownCause: + return lang.TrString("actions.workflow.incomplete_with_unknown_cause", run.PreExecutionErrorDetails...) } return fmt.Sprintf(" 0 || run.NeedApproval || v.IncompleteMatrix || v.IncompleteRunsOn { + if len(needs) > 0 || run.NeedApproval || v.IncompleteMatrix || v.IncompleteRunsOn || v.IncompleteWith { status = StatusBlocked } else { status = StatusWaiting diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 7d943f1410..0e59b931b5 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -260,7 +260,7 @@ func (job *ActionRunJob) ClearCachedWorkflowPayload() { // Checks whether the target job is an `(incomplete matrix)` job that will be blocked until the matrix is complete, and // then regenerated and deleted. If it is incomplete, and if the information is available, the specific job and/or // output that causes it to be incomplete will be returned as well. -func (job *ActionRunJob) IsIncompleteMatrix() (bool, *jobparser.IncompleteNeeds, error) { +func (job *ActionRunJob) HasIncompleteMatrix() (bool, *jobparser.IncompleteNeeds, error) { jobWorkflow, err := job.DecodeWorkflowPayload() if err != nil { return false, nil, fmt.Errorf("failure decoding workflow payload: %w", err) @@ -270,7 +270,7 @@ func (job *ActionRunJob) IsIncompleteMatrix() (bool, *jobparser.IncompleteNeeds, // Checks whether the target job has a `runs-on` field with an expression that requires an input from another job. The // job will be blocked until the other job is complete, and then regenerated and deleted. -func (job *ActionRunJob) IsIncompleteRunsOn() (bool, *jobparser.IncompleteNeeds, *jobparser.IncompleteMatrix, error) { +func (job *ActionRunJob) HasIncompleteRunsOn() (bool, *jobparser.IncompleteNeeds, *jobparser.IncompleteMatrix, error) { jobWorkflow, err := job.DecodeWorkflowPayload() if err != nil { return false, nil, nil, fmt.Errorf("failure decoding workflow payload: %w", err) @@ -278,6 +278,15 @@ func (job *ActionRunJob) IsIncompleteRunsOn() (bool, *jobparser.IncompleteNeeds, return jobWorkflow.IncompleteRunsOn, jobWorkflow.IncompleteRunsOnNeeds, jobWorkflow.IncompleteRunsOnMatrix, nil } +// Check whether the target job was generated as a result of expanding a reusable workflow. +func (job *ActionRunJob) IsWorkflowCallInnerJob() (bool, error) { + jobWorkflow, err := job.DecodeWorkflowPayload() + if err != nil { + return false, fmt.Errorf("failure decoding workflow payload: %w", err) + } + return jobWorkflow.Metadata.WorkflowCallParent != "", nil +} + // Check whether this job is a caller of a reusable workflow -- in other words, the real work done in this job is in // spawned child jobs, not this job. func (job *ActionRunJob) IsWorkflowCallOuterJob() (bool, error) { @@ -288,11 +297,12 @@ func (job *ActionRunJob) IsWorkflowCallOuterJob() (bool, error) { return jobWorkflow.Metadata.WorkflowCallID != "", nil } -// Check whether the target job was generated as a result of expanding a reusable workflow. -func (job *ActionRunJob) IsWorkflowCallInnerJob() (bool, error) { +// Checks whether the target job has a `with` field with an expression that requires an input from another job. The job +// will be blocked until the other job is complete, and then regenerated and deleted. +func (job *ActionRunJob) HasIncompleteWith() (bool, *jobparser.IncompleteNeeds, *jobparser.IncompleteMatrix, error) { jobWorkflow, err := job.DecodeWorkflowPayload() if err != nil { - return false, fmt.Errorf("failure decoding workflow payload: %w", err) + return false, nil, nil, fmt.Errorf("failure decoding workflow payload: %w", err) } - return jobWorkflow.Metadata.WorkflowCallParent != "", nil + return jobWorkflow.IncompleteWith, jobWorkflow.IncompleteWithNeeds, jobWorkflow.IncompleteWithMatrix, nil } diff --git a/models/actions/run_job_test.go b/models/actions/run_job_test.go index 231a17f463..7c204d4eb9 100644 --- a/models/actions/run_job_test.go +++ b/models/actions/run_job_test.go @@ -72,7 +72,7 @@ func TestActionRunJob_HTMLURL(t *testing.T) { } } -func TestActionRunJob_IsIncompleteMatrix(t *testing.T) { +func TestActionRunJob_HasIncompleteMatrix(t *testing.T) { tests := []struct { name string job ActionRunJob @@ -100,7 +100,7 @@ func TestActionRunJob_IsIncompleteMatrix(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - isIncomplete, needs, err := tt.job.IsIncompleteMatrix() + isIncomplete, needs, err := tt.job.HasIncompleteMatrix() if tt.errContains != "" { assert.ErrorContains(t, err, tt.errContains) } else { @@ -112,7 +112,7 @@ func TestActionRunJob_IsIncompleteMatrix(t *testing.T) { } } -func TestActionRunJob_IsIncompleteRunsOn(t *testing.T) { +func TestActionRunJob_HasIncompleteRunsOn(t *testing.T) { tests := []struct { name string job ActionRunJob @@ -147,7 +147,7 @@ func TestActionRunJob_IsIncompleteRunsOn(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - isIncomplete, needs, matrix, err := tt.job.IsIncompleteRunsOn() + isIncomplete, needs, matrix, err := tt.job.HasIncompleteRunsOn() if tt.errContains != "" { assert.ErrorContains(t, err, tt.errContains) } else { @@ -233,3 +233,51 @@ func TestActionRunJob_IsWorkflowCallInnerJob(t *testing.T) { }) } } + +func TestActionRunJob_HasIncompleteWith(t *testing.T) { + tests := []struct { + name string + job ActionRunJob + isIncomplete bool + needs *jobparser.IncompleteNeeds + matrix *jobparser.IncompleteMatrix + errContains string + }{ + { + name: "normal workflow", + job: ActionRunJob{WorkflowPayload: []byte("name: workflow")}, + isIncomplete: false, + }, + { + name: "incomplete_with workflow", + job: ActionRunJob{WorkflowPayload: []byte("name: workflow\nincomplete_with: true\nincomplete_with_needs: { job: abc }")}, + needs: &jobparser.IncompleteNeeds{Job: "abc"}, + isIncomplete: true, + }, + { + name: "incomplete_with workflow", + job: ActionRunJob{WorkflowPayload: []byte("name: workflow\nincomplete_with: true\nincomplete_with_matrix: { dimension: abc }")}, + matrix: &jobparser.IncompleteMatrix{Dimension: "abc"}, + isIncomplete: true, + }, + { + name: "unparseable workflow", + job: ActionRunJob{WorkflowPayload: []byte("name: []\nincomplete_with: true")}, + errContains: "failure unmarshaling WorkflowPayload to SingleWorkflow: yaml: unmarshal errors", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + isIncomplete, needs, matrix, err := tt.job.HasIncompleteWith() + if tt.errContains != "" { + assert.ErrorContains(t, err, tt.errContains) + } else { + require.NoError(t, err) + assert.Equal(t, tt.isIncomplete, isIncomplete) + assert.Equal(t, tt.needs, needs) + assert.Equal(t, tt.matrix, matrix) + } + }) + } +} diff --git a/models/actions/run_test.go b/models/actions/run_test.go index a0fdfddbd6..5c98f453eb 100644 --- a/models/actions/run_test.go +++ b/models/actions/run_test.go @@ -327,3 +327,51 @@ jobs: }) } } + +func TestActionRun_IncompleteWith(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + pullRequestPosterID := int64(4) + repoID := int64(10) + pullRequestID := int64(2) + runDoesNotNeedApproval := &ActionRun{ + RepoID: repoID, + PullRequestID: pullRequestID, + PullRequestPosterID: pullRequestPosterID, + } + + workflowRaw := []byte(` +jobs: + outer-job: + with: + some_input: ${{ needs.other-job.outputs.some-output }} + uses: ./.forgejo/workflows/reusable.yml +`) + workflows, err := jobparser.Parse(workflowRaw, false, + jobparser.WithJobOutputs(map[string]map[string]string{}), + jobparser.ExpandLocalReusableWorkflows(func(job *jobparser.Job, path string) ([]byte, error) { + return []byte(` +on: + workflow_call: + inputs: + some_input: + type: string +jobs: + inner-job: + runs-on: debian + steps: [] +`), nil + })) + require.NoError(t, err) + require.True(t, workflows[0].IncompleteWith) // must be set for this test scenario to be valid + + require.NoError(t, InsertRun(t.Context(), runDoesNotNeedApproval, workflows)) + + jobs, err := db.Find[ActionRunJob](t.Context(), FindRunJobOptions{RunID: runDoesNotNeedApproval.ID}) + require.NoError(t, err) + require.Len(t, jobs, 1) + job := jobs[0] + + // Expect job with an incomplete with to be StatusBlocked: + assert.Equal(t, StatusBlocked, job.Status) +} diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index 38d7d18e7e..e85f8e311e 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -219,12 +219,16 @@ "actions.workflow.event_detection_error": "Unable to parse supported events in workflow: %v", "actions.workflow.persistent_incomplete_matrix": "Unable to evaluate `strategy.matrix` of job %[1]s due to a `needs` expression that was invalid. It may reference a job that is not in it's 'needs' list (%[2]s), or an output that doesn't exist on one of those jobs.", "actions.workflow.incomplete_matrix_missing_job": "Unable to evaluate `strategy.matrix` of job %[1]s: job %[2]s is not in the `needs` list of job %[1]s (%[3]s).", - "actions.workflow.incomplete_matrix_missing_output": "Unable to evaluate `strategy.matrix` of job %[1]s: job %[2]s does not have an output %[3]s.", + "actions.workflow.incomplete_matrix_missing_output": "Unable to evaluate `strategy.matrix` of job %[1]s: job %[2]s is missing output %[3]s.", "actions.workflow.incomplete_matrix_unknown_cause": "Unable to evaluate `strategy.matrix` of job %[1]s: unknown error.", "actions.workflow.incomplete_runson_missing_job": "Unable to evaluate `runs-on` of job %[1]s: job %[2]s is not in the `needs` list of job %[1]s (%[3]s).", - "actions.workflow.incomplete_runson_missing_output": "Unable to evaluate `runs-on` of job %[1]s: job %[2]s does not have an output %[3]s.", + "actions.workflow.incomplete_runson_missing_output": "Unable to evaluate `runs-on` of job %[1]s: job %[2]s is missing output %[3]s.", "actions.workflow.incomplete_runson_missing_matrix_dimension": "Unable to evaluate `runs-on` of job %[1]s: matrix dimension %[2]s does not exist.", "actions.workflow.incomplete_runson_unknown_cause": "Unable to evaluate `runs-on` of job %[1]s: unknown error.", + "actions.workflow.incomplete_with_missing_job": "Unable to evaluate `with` of job %[1]s: job %[2]s is not in the `needs` list of job %[1]s (%[3]s).", + "actions.workflow.incomplete_with_missing_output": "Unable to evaluate `with` of job %[1]s: job %[2]s is missing output %[3]s.", + "actions.workflow.incomplete_with_missing_matrix_dimension": "Unable to evaluate `with` of job %[1]s: matrix dimension %[2]s does not exist.", + "actions.workflow.incomplete_with_unknown_cause": "Unable to evaluate `with` of job %[1]s: unknown error.", "actions.workflow.pre_execution_error": "Workflow was not executed due to an error that blocked the execution attempt.", "pulse.n_active_issues": { "one": "%s active issue", diff --git a/services/actions/Test_tryHandleIncompleteMatrix/action_run.yml b/services/actions/Test_tryHandleIncompleteMatrix/action_run.yml index 2bbd44b405..06af0d3014 100644 --- a/services/actions/Test_tryHandleIncompleteMatrix/action_run.yml +++ b/services/actions/Test_tryHandleIncompleteMatrix/action_run.yml @@ -283,3 +283,136 @@ need_approval: 0 approved_by: 0 concurrency_group: abc123 +- + id: 915 + title: "running workflow_dispatch run" + repo_id: 63 + owner_id: 2 + workflow_id: "running.yaml" + index: 19 + trigger_user_id: 2 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 6 # running + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 + concurrency_group: abc123 +- + id: 916 + title: "running workflow_dispatch run" + repo_id: 63 + owner_id: 2 + workflow_id: "running.yaml" + index: 20 + trigger_user_id: 2 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 6 # running + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 + concurrency_group: abc123 +- + id: 917 + title: "running workflow_dispatch run" + repo_id: 63 + owner_id: 2 + workflow_id: "running.yaml" + index: 21 + trigger_user_id: 2 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 6 # running + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 + concurrency_group: abc123 +- + id: 918 + title: "running workflow_dispatch run" + repo_id: 63 + owner_id: 2 + workflow_id: "running.yaml" + index: 22 + trigger_user_id: 2 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 6 # running + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 + concurrency_group: abc123 +- + id: 919 + title: "running workflow_dispatch run" + repo_id: 63 + owner_id: 2 + workflow_id: "running.yaml" + index: 23 + trigger_user_id: 2 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 6 # running + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 + concurrency_group: abc123 +- + id: 920 + title: "running workflow_dispatch run" + repo_id: 63 + owner_id: 2 + workflow_id: "running.yaml" + index: 24 + trigger_user_id: 2 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 6 # running + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 + concurrency_group: abc123 +- + id: 921 + title: "running workflow_dispatch run" + repo_id: 63 + owner_id: 2 + workflow_id: "running.yaml" + index: 25 + trigger_user_id: 2 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 6 # running + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 + concurrency_group: abc123 diff --git a/services/actions/Test_tryHandleIncompleteMatrix/action_run_job.yml b/services/actions/Test_tryHandleIncompleteMatrix/action_run_job.yml index a352f54ee6..19cb5f2d59 100644 --- a/services/actions/Test_tryHandleIncompleteMatrix/action_run_job.yml +++ b/services/actions/Test_tryHandleIncompleteMatrix/action_run_job.yml @@ -7,7 +7,7 @@ is_fork_pull_request: 0 name: job_1 attempt: 0 - job_id: job_1 + job_id: produce-artifacts task_id: 0 status: 7 # blocked runs_on: '["fedora"]' @@ -35,7 +35,7 @@ is_fork_pull_request: 0 name: job_1 attempt: 0 - job_id: job_1 + job_id: produce-artifacts task_id: 0 status: 7 # blocked runs_on: '["fedora"]' @@ -76,7 +76,7 @@ is_fork_pull_request: 0 name: job_1 attempt: 0 - job_id: job_1 + job_id: produce-artifacts task_id: 0 status: 7 # blocked runs_on: '["fedora"]' @@ -117,7 +117,7 @@ is_fork_pull_request: 0 name: job_1 attempt: 0 - job_id: job_1 + job_id: produce-artifacts task_id: 0 status: 7 # blocked runs_on: '["fedora"]' @@ -159,7 +159,7 @@ is_fork_pull_request: 0 name: job_1 attempt: 0 - job_id: job_1 + job_id: produce-artifacts task_id: 0 status: 7 # blocked runs_on: '["fedora"]' @@ -200,7 +200,7 @@ is_fork_pull_request: 0 name: job_1 attempt: 0 - job_id: job_1 + job_id: run-tests task_id: 0 status: 7 # blocked runs_on: '["fedora"]' @@ -243,7 +243,7 @@ is_fork_pull_request: 0 name: job_1 attempt: 0 - job_id: job_1 + job_id: run-tests task_id: 0 status: 7 # blocked runs_on: '["fedora"]' @@ -283,7 +283,7 @@ is_fork_pull_request: 0 name: job_1 attempt: 0 - job_id: job_1 + job_id: scalar-job task_id: 0 status: 7 # blocked runs_on: '["fedora"]' @@ -328,7 +328,7 @@ is_fork_pull_request: 0 name: job_1 attempt: 0 - job_id: job_1 + job_id: produce-artifacts task_id: 0 status: 7 # blocked runs_on: '["fedora"]' @@ -596,3 +596,259 @@ task_id: 107 status: 1 # success runs_on: '["fedora"]' + +- + id: 629 + run_id: 915 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: consume-runs-on + attempt: 0 + job_id: consume-runs-on + task_id: 0 + status: 7 # blocked + runs_on: '[]' + needs: '["define-workflow-call"]' + workflow_payload: | + "on": + push: + jobs: + consume-runs-on: + name: consume-runs-on + uses: some-repo/some-org/.forgejo/workflows/reusable.yml@non-existent-reference + with: + workflow_input: ${{ needs.define-workflow-call.outputs.workflow_input }} + incomplete_with: true + +- + id: 630 + run_id: 916 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: perform-workflow-call + attempt: 0 + job_id: perform-workflow-call + task_id: 0 + status: 7 # blocked + runs_on: '[]' + needs: '["define-workflow-call"]' + workflow_payload: | + "on": + push: + jobs: + perform-workflow-call: + name: perform-workflow-call + uses: some-repo/some-org/.forgejo/workflows/reusable.yml@simple + with: + workflow_input: ${{ needs.define-workflow-call.outputs.workflow_input }} + incomplete_with: true +- + id: 631 + run_id: 916 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: define-workflow-call + attempt: 0 + job_id: define-workflow-call + task_id: 108 + status: 1 # success + runs_on: '["fedora"]' + +- + id: 632 + run_id: 917 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: perform-workflow-call + attempt: 0 + job_id: perform-workflow-call + task_id: 0 + status: 7 # blocked + runs_on: '[]' + needs: '["define-workflow-call"]' + workflow_payload: | + "on": + push: + jobs: + perform-workflow-call: + name: perform-workflow-call + uses: some-repo/some-org/.forgejo/workflows/reusable.yml@more-incomplete + with: + workflow_input: ${{ needs.define-workflow-call.outputs.workflow_input }} + incomplete_with: true +- + id: 633 + run_id: 917 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: define-workflow-call + attempt: 0 + job_id: define-workflow-call + task_id: 109 + status: 1 # success + runs_on: '["fedora"]' + +- + id: 634 + run_id: 918 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: perform-workflow-call + attempt: 0 + job_id: perform-workflow-call + task_id: 0 + status: 7 # blocked + runs_on: '[]' + needs: '["define-workflow-call"]' + workflow_payload: | + "on": + push: + jobs: + perform-workflow-call: + name: perform-workflow-call + uses: some-repo/some-org/.forgejo/workflows/reusable.yml@more-incomplete + with: + workflow_input: ${{ needs.oops-i-misspelt-the-job-id.outputs.workflow_input }} + incomplete_with: true +- + id: 635 + run_id: 918 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: define-workflow-call + attempt: 0 + job_id: define-workflow-call + task_id: 109 + status: 1 # success + runs_on: '["fedora"]' + +- + id: 636 + run_id: 919 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: perform-workflow-call + attempt: 0 + job_id: perform-workflow-call + task_id: 0 + status: 7 # blocked + runs_on: '[]' + needs: '["define-workflow-call"]' + workflow_payload: | + "on": + push: + jobs: + perform-workflow-call: + name: perform-workflow-call + uses: some-repo/some-org/.forgejo/workflows/reusable.yml@more-incomplete + with: + workflow_input: ${{ needs.define-workflow-call.outputs.output-doesnt-exist }} + incomplete_with: true +- + id: 637 + run_id: 919 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: define-workflow-call + attempt: 0 + job_id: define-workflow-call + task_id: 109 + status: 1 # success + runs_on: '["fedora"]' + +- + id: 638 + run_id: 920 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: perform-workflow-call + attempt: 0 + job_id: perform-workflow-call + task_id: 0 + status: 7 # blocked + runs_on: '[]' + needs: '["define-workflow-call"]' + workflow_payload: | + "on": + push: + jobs: + perform-workflow-call: + name: perform-workflow-call + uses: some-repo/some-org/.forgejo/workflows/reusable.yml@more-incomplete + with: + workflow_input: ${{ matrix.dimension-oops-error }} + strategy: + matrix: + dimension1: [ abc, def ] + incomplete_with: true +- + id: 639 + run_id: 920 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: define-workflow-call + attempt: 0 + job_id: define-workflow-call + task_id: 109 + status: 1 # success + runs_on: '["fedora"]' + +- + id: 640 + run_id: 921 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: perform-workflow-call + attempt: 0 + job_id: perform-workflow-call + task_id: 0 + status: 7 # blocked + runs_on: '[]' + needs: '["define-workflow-call"]' + workflow_payload: | + "on": + push: + jobs: + perform-workflow-call: + name: perform-workflow-call + uses: ./.forgejo/workflows/reusable.yml + with: + workflow_input: ${{ needs.define-workflow-call.outputs.workflow_input }} + incomplete_with: true +- + id: 641 + run_id: 921 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: define-workflow-call + attempt: 0 + job_id: define-workflow-call + task_id: 108 + status: 1 # success + runs_on: '["fedora"]' diff --git a/services/actions/Test_tryHandleIncompleteMatrix/action_task_output.yml b/services/actions/Test_tryHandleIncompleteMatrix/action_task_output.yml index 6633ee72ee..35c4676504 100644 --- a/services/actions/Test_tryHandleIncompleteMatrix/action_task_output.yml +++ b/services/actions/Test_tryHandleIncompleteMatrix/action_task_output.yml @@ -48,3 +48,13 @@ task_id: 107 output_key: run-on-this output_value: nixos-25.11 +- + id: 110 + task_id: 108 + output_key: workflow_input + output_value: my-workflow-input +- + id: 111 + task_id: 109 + output_key: workflow_input + output_value: my-workflow-input diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index e5093301ea..7f86dbecc1 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -33,8 +33,8 @@ func CreateCommitStatus(ctx context.Context, jobs ...*actions_model.ActionRunJob } func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) error { - if incompleteMatrix, _, err := job.IsIncompleteMatrix(); err != nil { - return fmt.Errorf("job IsIncompleteMatrix: %w", err) + if incompleteMatrix, _, err := job.HasIncompleteMatrix(); err != nil { + return fmt.Errorf("job HasIncompleteMatrix: %w", err) } else if incompleteMatrix { // Don't create commit statuses for incomplete matrix jobs because they are never completed. return nil diff --git a/services/actions/commit_status_test.go b/services/actions/commit_status_test.go index 0872a9d7df..372448e321 100644 --- a/services/actions/commit_status_test.go +++ b/services/actions/commit_status_test.go @@ -22,17 +22,17 @@ func TestCreateCommitStatus_IncompleteMatrix(t *testing.T) { err := createCommitStatus(t.Context(), job) require.ErrorContains(t, err, "object does not exist [id: 7a3858dc7f059543a8807a8b551304b7e362a7ef") - // Transition from IsIncompleteMatrix()=false to true... - isIncomplete, _, err := job.IsIncompleteMatrix() + // Transition from HasIncompleteMatrix()=false to true... + isIncomplete, _, err := job.HasIncompleteMatrix() require.NoError(t, err) require.False(t, isIncomplete) job.WorkflowPayload = append(job.WorkflowPayload, "\nincomplete_matrix: true\n"...) job.ClearCachedWorkflowPayload() - isIncomplete, _, err = job.IsIncompleteMatrix() + isIncomplete, _, err = job.HasIncompleteMatrix() require.NoError(t, err) require.True(t, isIncomplete) - // Now there should be no error since createCommitStatus will exit early due to the IsIncompleteMatrix() flag. + // Now there should be no error since createCommitStatus will exit early due to the HasIncompleteMatrix() flag. err = createCommitStatus(t.Context(), job) require.NoError(t, err) } diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index 4e61029878..167083a028 100644 --- a/services/actions/job_emitter.go +++ b/services/actions/job_emitter.go @@ -168,7 +168,20 @@ func (r *jobStatusResolver) resolve() map[int64]actions_model.Status { // success/failure. checkJobsOfRun will do additional work in these cases to "finish" the workflow call // job as well. if allSucceed { - ret[id] = actions_model.StatusSuccess + isIncompleteMatrix, _, _ := r.jobMap[id].HasIncompleteMatrix() + isIncompleteWith, _, _, _ := r.jobMap[id].HasIncompleteWith() + if isIncompleteMatrix || isIncompleteWith { + // The `needs` of this job are done. For an outer workflow call, that usually means that the + // inner jobs are done. But if the job is incomplete, that means that the `needs` that were + // required to define the job are done, and now the job can be expanded with the missing values + // that come from `${{ needs... }}`. By putting this job into `Waiting` state, it will go into + // `tryHandleIncompleteMatrix` to be reparsed, replaced with a full job definition, with new + // `needs` that contain its inner jobs: + ret[id] = actions_model.StatusWaiting + } else { + // This job is done by virtue of its inner jobs being done successfully. + ret[id] = actions_model.StatusSuccess + } } else { ret[id] = actions_model.StatusFailure } @@ -201,17 +214,22 @@ func (r *jobStatusResolver) resolve() map[int64]actions_model.Status { // Invoked once a job has all its `needs` parameters met and is ready to transition to waiting, this may expand the // job's `strategy.matrix` into multiple new jobs. func tryHandleIncompleteMatrix(ctx context.Context, blockedJob *actions_model.ActionRunJob, jobsInRun []*actions_model.ActionRunJob) (bool, error) { - incompleteMatrix, _, err := blockedJob.IsIncompleteMatrix() + incompleteMatrix, _, err := blockedJob.HasIncompleteMatrix() if err != nil { - return false, fmt.Errorf("job IsIncompleteMatrix: %w", err) + return false, fmt.Errorf("job HasIncompleteMatrix: %w", err) } - incompleteRunsOn, _, _, err := blockedJob.IsIncompleteRunsOn() + incompleteRunsOn, _, _, err := blockedJob.HasIncompleteRunsOn() if err != nil { - return false, fmt.Errorf("job IsIncompleteRunsOn: %w", err) + return false, fmt.Errorf("job HasIncompleteRunsOn: %w", err) } - if !incompleteMatrix && !incompleteRunsOn { + incompleteWith, _, _, err := blockedJob.HasIncompleteWith() + if err != nil { + return false, fmt.Errorf("job HasIncompleteWith: %w", err) + } + + if !incompleteMatrix && !incompleteRunsOn && !incompleteWith { // Not relevant to attempt re-parsing the job if it wasn't marked as Incomplete[...] previously. return false, nil } @@ -247,13 +265,28 @@ func tryHandleIncompleteMatrix(ctx context.Context, blockedJob *actions_model.Ac // Re-parse the blocked job, providing all the other completed jobs' outputs, to turn this incomplete job into // one-or-more new jobs: + expandLocalReusableWorkflow, expandCleanup := lazyRepoExpandLocalReusableWorkflow(ctx, blockedJob.RepoID, blockedJob.CommitSHA) + defer expandCleanup() newJobWorkflows, err := jobparser.Parse(blockedJob.WorkflowPayload, false, jobparser.WithJobOutputs(jobOutputs), jobparser.WithWorkflowNeeds(blockedJob.Needs), jobparser.SupportIncompleteRunsOn(), + jobparser.ExpandLocalReusableWorkflows(expandLocalReusableWorkflow), + jobparser.ExpandInstanceReusableWorkflows(expandInstanceReusableWorkflows(ctx)), ) if err != nil { - return false, fmt.Errorf("failure re-parsing SingleWorkflow: %w", err) + // Reparsing errors are quite rare here since we were already able to parse this workflow in the past to + // generate `blockedJob`, but it would be possible with a remote reusable workflow if the reference disappears + // from the remote repo -- eg. it was `@v1` and the `v1` tag was removed. + if err := FailRunPreExecutionError( + ctx, + blockedJob.Run, + actions_model.ErrorCodeJobParsingError, + []any{err.Error()}); err != nil { + return false, fmt.Errorf("setting run into PreExecutionError state failed: %w", err) + } + // Return `true` to skip running this job in this invalid state + return true, nil } // Even though every job in the `needs` list is done, perform a consistency check if the job was still unable to be @@ -261,20 +294,51 @@ func tryHandleIncompleteMatrix(ctx context.Context, blockedJob *actions_model.Ac // reported back to the user for them to correct their workflow, so we slip this notification into // PreExecutionError. for _, swf := range newJobWorkflows { - if swf.IncompleteMatrix { - errorCode, errorDetails := persistentIncompleteMatrixError(blockedJob, swf.IncompleteMatrixNeeds) - if err := FailRunPreExecutionError(ctx, blockedJob.Run, errorCode, errorDetails); err != nil { - return false, fmt.Errorf("failure when marking run with error: %w", err) + // If the re-evaluated job has the same job ID as the input job, and it's still incomplete, then we'll consider + // it to be a "persistent incomplete" job with some error that needs to be reported to the user. If the + // re-evaluated job has a different job ID, then it's likely an expanded job -- such as from a reusable workflow + // -- which could have it's own `needs` that allows it to expand into a correct job in the future. + jobID, job := swf.Job() + if jobID == blockedJob.JobID { + if swf.IncompleteMatrix { + errorCode, errorDetails := persistentIncompleteMatrixError(blockedJob, swf.IncompleteMatrixNeeds) + if err := FailRunPreExecutionError(ctx, blockedJob.Run, errorCode, errorDetails); err != nil { + return false, fmt.Errorf("setting run into PreExecutionError state failed: %w", err) + } + // Return `true` to skip running this job in this invalid state + return true, nil + } else if swf.IncompleteRunsOn { + errorCode, errorDetails := persistentIncompleteRunsOnError(blockedJob, swf.IncompleteRunsOnNeeds, swf.IncompleteRunsOnMatrix) + if err := FailRunPreExecutionError(ctx, blockedJob.Run, errorCode, errorDetails); err != nil { + return false, fmt.Errorf("setting run into PreExecutionError state failed: %w", err) + } + // Return `true` to skip running this job in this invalid state + return true, nil + } else if swf.IncompleteWith { + errorCode, errorDetails := persistentIncompleteWithError(blockedJob, swf.IncompleteWithNeeds, swf.IncompleteWithMatrix) + if err := FailRunPreExecutionError(ctx, blockedJob.Run, errorCode, errorDetails); err != nil { + return false, fmt.Errorf("setting run into PreExecutionError state failed: %w", err) + } + // Return `true` to skip running this job in this invalid state + return true, nil } - // Return `true` to skip running this job in this invalid state - return true, nil - } else if swf.IncompleteRunsOn { - errorCode, errorDetails := persistentIncompleteRunsOnError(blockedJob, swf.IncompleteRunsOnNeeds, swf.IncompleteRunsOnMatrix) - if err := FailRunPreExecutionError(ctx, blockedJob.Run, errorCode, errorDetails); err != nil { - return false, fmt.Errorf("failure when marking run with error: %w", err) + + // When `InsertRunJobs` is run on a job (including `blockedJob` when it was persisted), the `needs` are + // removed from it's WorkflowPayload and moved up to the database record so that Forgejo can manage ordering + // the run execution. Now that `blockedJob` has been changed from incomplete->complete by reparsing it and + // providing its inputs, it would have been reparsed with an empty `needs` entry because of this earlier + // removal. And the returned record could have its own new `needs` if it was a reusable workflow with inner + // jobs. So, merge the old database list of needs with the new reparsed list of needs, and store them in + // the new `SingleWorkflow` which will be paseed to `InsertRunJobs` where it will be ripped out again. + // + // This is only relevant for `blockedJob` and not for any other generated jobs since they wouldn't have yet + // gone through `InsertRunJobs` to have this mutation performed. + newNeeds := append(job.Needs(), blockedJob.Needs...) + _ = job.RawNeeds.Encode(newNeeds) + err := swf.SetJob(jobID, job) + if err != nil { + return false, fmt.Errorf("failure to update needs in job: %w", err) } - // Return `true` to skip running this job in this invalid state - return true, nil } } @@ -375,6 +439,49 @@ func persistentIncompleteRunsOnError(job *actions_model.ActionRunJob, incomplete return errorCode, errorDetails } +func persistentIncompleteWithError(job *actions_model.ActionRunJob, incompleteNeeds *jobparser.IncompleteNeeds, incompleteMatrix *jobparser.IncompleteMatrix) (actions_model.PreExecutionError, []any) { + var errorCode actions_model.PreExecutionError + var errorDetails []any + + // `incompleteMatrix` tells us which dimension of a matrix was accessed that was missing + if incompleteMatrix != nil { + dimension := incompleteMatrix.Dimension + errorCode = actions_model.ErrorCodeIncompleteWithMissingMatrixDimension + errorDetails = []any{ + job.JobID, + dimension, + } + return errorCode, errorDetails + } + + // `incompleteNeeds` tells us what part of a `${{ needs... }}` expression was missing + if incompleteNeeds != nil { + jobRef := incompleteNeeds.Job // always provided + outputRef := incompleteNeeds.Output // missing if the entire job wasn't present + if outputRef != "" { + errorCode = actions_model.ErrorCodeIncompleteWithMissingOutput + errorDetails = []any{ + job.JobID, + jobRef, + outputRef, + } + } else { + errorCode = actions_model.ErrorCodeIncompleteWithMissingJob + errorDetails = []any{ + job.JobID, + jobRef, + strings.Join(job.Needs, ", "), + } + } + return errorCode, errorDetails + } + + // Not sure why we ended up in `IncompleteWith` when nothing was marked as incomplete + errorCode = actions_model.ErrorCodeIncompleteWithUnknownCause + errorDetails = []any{job.JobID} + return errorCode, errorDetails +} + // When a workflow call outer job's dependencies are completed, `tryHandleWorkflowCallOuterJob` will complete the job // without actually executing it. It will not be dispatched it to a runner. There's no job execution logic, but we need // to update state of a few things -- particularly workflow outputs. diff --git a/services/actions/job_emitter_test.go b/services/actions/job_emitter_test.go index 0bfa10d7ea..14310703e6 100644 --- a/services/actions/job_emitter_test.go +++ b/services/actions/job_emitter_test.go @@ -4,6 +4,8 @@ package actions import ( + "context" + "errors" "slices" "testing" @@ -12,6 +14,7 @@ import ( "forgejo.org/models/unittest" "forgejo.org/modules/test" + "code.forgejo.org/forgejo/runner/v12/act/jobparser" "code.forgejo.org/forgejo/runner/v12/act/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -154,6 +157,58 @@ __metadata: 3: actions_model.StatusSuccess, }, }, + { + name: "unblocked workflow call outer job, incomplete `with`", + jobs: actions_model.ActionJobList{ + {ID: 1, JobID: "job0", Status: actions_model.StatusSuccess, Needs: []string{}}, + {ID: 2, JobID: "job1", Status: actions_model.StatusBlocked, Needs: []string{"job0"}, WorkflowPayload: []byte( + ` +name: test +on: push +jobs: + job2: + if: false + uses: ./.forgejo/workflows/reusable.yml + with: + something: ${{ needs.job0.outputs.something }} +incomplete_with: true +incomplete_with_needs: + job: job0 + output: something +__metadata: + workflow_call_id: b5a9f46f1f2513d7777fde50b169d323a6519e349cc175484c947ac315a209ed +`)}, + }, + want: map[int64]actions_model.Status{ + 2: actions_model.StatusWaiting, + }, + }, + { + name: "unblocked workflow call outer job, incomplete `strategy.matrix`", + jobs: actions_model.ActionJobList{ + {ID: 1, JobID: "job0", Status: actions_model.StatusSuccess, Needs: []string{}}, + {ID: 2, JobID: "job1", Status: actions_model.StatusBlocked, Needs: []string{"job0"}, WorkflowPayload: []byte( + ` +name: test +on: push +jobs: + job2: + if: false + uses: ./.forgejo/workflows/reusable.yml + strategy: + matrix: ${{ fromJSON(needs.job0.outputs.something) }} +incomplete_matrix: true +incomplete_matrix_needs: + job: job0 + output: something +__metadata: + workflow_call_id: b5a9f46f1f2513d7777fde50b169d323a6519e349cc175484c947ac315a209ed +`)}, + }, + want: map[int64]actions_model.Status{ + 2: actions_model.StatusWaiting, + }, + }, { name: "unblocked workflow call outer job with internal failure", jobs: actions_model.ActionJobList{ @@ -184,21 +239,67 @@ __metadata: } } +const testWorkflowCallSimpleExpansion = ` +on: + workflow_call: + inputs: + workflow_input: + type: string +jobs: + inner_job: + name: "inner ${{ inputs.workflow_input }}" + runs-on: debian-latest + steps: + - run: echo ${{ inputs.workflow_input }} +` + +const testWorkflowCallMoreIncompleteExpansion = ` +on: + workflow_call: + inputs: + workflow_input: + type: string +jobs: + define-runs-on: + name: "inner define-runs-on ${{ inputs.workflow_input }}" + runs-on: docker + outputs: + scalar-value: ${{ steps.define.outputs.scalar }} + steps: + - id: define + run: | + echo 'scalar=scalar value' >> "$FORGEJO_OUTPUT" + scalar-job: + name: "inner incomplete-job ${{ inputs.workflow_input }}" + runs-on: ${{ needs.define-runs-on.outputs.scalar-value }} + needs: define-runs-on + steps: [] +` + func Test_tryHandleIncompleteMatrix(t *testing.T) { // Shouldn't get any decoding errors during this test -- pop them up from a log warning to a test fatal error. defer test.MockVariableValue(&model.OnDecodeNodeError, func(node yaml.Node, out any, err error) { t.Fatalf("Failed to decode node %v into %T: %v", node, out, err) })() + type localReusableWorkflowCallArgs struct { + repoID int64 + commitSHA string + path string + } + tests := []struct { - name string - runJobID int64 - errContains string - consumed bool - runJobNames []string - preExecutionError actions_model.PreExecutionError - preExecutionErrorDetails []any - runsOn map[string][]string + name string + runJobID int64 + errContains string + consumed bool + runJobNames []string + preExecutionError actions_model.PreExecutionError + preExecutionErrorDetails []any + runsOn map[string][]string + needs map[string][]string + expectIncompleteJob []string + localReusableWorkflowCallArgs *localReusableWorkflowCallArgs }{ { name: "not incomplete", @@ -219,7 +320,7 @@ func Test_tryHandleIncompleteMatrix(t *testing.T) { name: "missing needs for strategy.matrix evaluation", runJobID: 605, preExecutionError: actions_model.ErrorCodeIncompleteMatrixMissingJob, - preExecutionErrorDetails: []any{"job_1", "define-matrix-2", "define-matrix-1"}, + preExecutionErrorDetails: []any{"produce-artifacts", "define-matrix-2", "define-matrix-1"}, }, { name: "matrix expanded to 0 jobs", @@ -279,7 +380,7 @@ func Test_tryHandleIncompleteMatrix(t *testing.T) { name: "missing needs output for strategy.matrix evaluation", runJobID: 615, preExecutionError: actions_model.ErrorCodeIncompleteMatrixMissingOutput, - preExecutionErrorDetails: []any{"job_1", "define-matrix-1", "colours-intentional-mistake"}, + preExecutionErrorDetails: []any{"produce-artifacts", "define-matrix-1", "colours-intentional-mistake"}, }, { name: "runs-on evaluation with needs", @@ -356,12 +457,126 @@ func Test_tryHandleIncompleteMatrix(t *testing.T) { preExecutionError: actions_model.ErrorCodeIncompleteRunsOnMissingMatrixDimension, preExecutionErrorDetails: []any{"consume-runs-on", "dimension-oops-error"}, }, + { + name: "workflow call remote reference unavailable", + runJobID: 629, + preExecutionError: actions_model.ErrorCodeJobParsingError, + preExecutionErrorDetails: []any{"unable to read instance workflow \"some-repo/some-org/.forgejo/workflows/reusable.yml@non-existent-reference\": someone deleted that reference maybe"}, + }, + { + name: "workflow call with needs expansion", + runJobID: 630, + consumed: true, + runJobNames: []string{ + "define-workflow-call", + "inner my-workflow-input", + "perform-workflow-call", + }, + needs: map[string][]string{ + "define-workflow-call": nil, + "inner my-workflow-input": nil, + "perform-workflow-call": {"define-workflow-call", "perform-workflow-call.inner_job"}, + }, + }, + // Before reusable workflow expansion, there weren't any cases where evaluating a job in the job emitter could + // result in more incomplete jobs being generated (other than errors). This is the first such case -- run job + // ID 632 references reusable workflow "more-incomplete" which generates more incomplete jobs. + { + name: "workflow call generates more incomplete jobs", + runJobID: 632, + consumed: true, + runJobNames: []string{ + "define-workflow-call", + "inner define-runs-on my-workflow-input", + "inner incomplete-job my-workflow-input", + "perform-workflow-call", + }, + runsOn: map[string][]string{ + "define-workflow-call": {"fedora"}, + "perform-workflow-call": {}, + "inner define-runs-on my-workflow-input": {"docker"}, + "inner incomplete-job my-workflow-input": {"${{ needs[format('{0}.{1}', 'perform-workflow-call', 'define-runs-on')].outputs.scalar-value }}"}, + }, + needs: map[string][]string{ + "define-workflow-call": nil, + "inner define-runs-on my-workflow-input": nil, + "inner incomplete-job my-workflow-input": {"perform-workflow-call.define-runs-on"}, + "perform-workflow-call": { + "define-workflow-call", + "perform-workflow-call.define-runs-on", + "perform-workflow-call.scalar-job", + }, + }, + expectIncompleteJob: []string{"inner incomplete-job my-workflow-input"}, + }, + { + name: "missing needs job for workflow call evaluation", + runJobID: 634, + preExecutionError: actions_model.ErrorCodeIncompleteWithMissingJob, + preExecutionErrorDetails: []any{"perform-workflow-call", "oops-i-misspelt-the-job-id", "define-workflow-call"}, + }, + { + name: "missing needs output for workflow call evaluation", + runJobID: 636, + preExecutionError: actions_model.ErrorCodeIncompleteWithMissingOutput, + preExecutionErrorDetails: []any{"perform-workflow-call", "define-workflow-call", "output-doesnt-exist"}, + }, + { + name: "missing matrix dimension for workflow call evaluation", + runJobID: 638, + preExecutionError: actions_model.ErrorCodeIncompleteWithMissingMatrixDimension, + preExecutionErrorDetails: []any{"perform-workflow-call", "dimension-oops-error"}, + }, + { + name: "local workflow call with needs expansion", + runJobID: 640, + consumed: true, + runJobNames: []string{ + "define-workflow-call", + "inner my-workflow-input", + "perform-workflow-call", + }, + localReusableWorkflowCallArgs: &localReusableWorkflowCallArgs{ + repoID: 63, + commitSHA: "97f29ee599c373c729132a5c46a046978311e0ee", + path: "./.forgejo/workflows/reusable.yml", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { defer unittest.OverrideFixtures("services/actions/Test_tryHandleIncompleteMatrix")() require.NoError(t, unittest.PrepareTestDatabase()) + // Mock access to reusable workflows, both local and remote + var localReusableCalled []*localReusableWorkflowCallArgs + var cleanupCallCount int + defer test.MockVariableValue(&lazyRepoExpandLocalReusableWorkflow, + func(ctx context.Context, repoID int64, commitSHA string) (jobparser.LocalWorkflowFetcher, CleanupFunc) { + fetcher := func(job *jobparser.Job, path string) ([]byte, error) { + localReusableCalled = append(localReusableCalled, &localReusableWorkflowCallArgs{repoID, commitSHA, path}) + return []byte(testWorkflowCallSimpleExpansion), nil + } + cleanup := func() { + cleanupCallCount++ + } + return fetcher, cleanup + })() + defer test.MockVariableValue(&expandInstanceReusableWorkflows, + func(ctx context.Context) jobparser.InstanceWorkflowFetcher { + return func(job *jobparser.Job, ref *model.NonLocalReusableWorkflowReference) ([]byte, error) { + switch ref.Ref { + case "non-existent-reference": + return nil, errors.New("someone deleted that reference maybe") + case "simple": + return []byte(testWorkflowCallSimpleExpansion), nil + case "more-incomplete": + return []byte(testWorkflowCallMoreIncompleteExpansion), nil + } + return nil, errors.New("unknown workflow reference") + } + })() + blockedJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: tt.runJobID}) jobsInRun, err := db.Find[actions_model.ActionRunJob](t.Context(), actions_model.FindRunJobOptions{RunID: blockedJob.RunID}) @@ -381,7 +596,7 @@ func Test_tryHandleIncompleteMatrix(t *testing.T) { // expectations are that the ActionRun has an empty PreExecutionError actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: blockedJob.RunID}) - assert.EqualValues(t, 0, actionRun.PreExecutionErrorCode) + assert.EqualValues(t, 0, actionRun.PreExecutionErrorCode, "PreExecutionError Details: %#v", actionRun.PreExecutionErrorDetails) // compare jobs that exist with `runJobNames` to ensure new jobs are inserted: allJobsInRun, err := db.Find[actions_model.ActionRunJob](t.Context(), actions_model.FindRunJobOptions{RunID: blockedJob.RunID}) @@ -404,6 +619,37 @@ func Test_tryHandleIncompleteMatrix(t *testing.T) { } } } + + if tt.needs != nil { + for _, j := range allJobsInRun { + expected, ok := tt.needs[j.Name] + if assert.Truef(t, ok, "unable to find runsOn[%q] in test case", j.Name) { + slices.Sort(j.Needs) + slices.Sort(expected) + assert.Equalf(t, expected, j.Needs, "comparing needs expectations for job %q", j.Name) + } + } + } + + if tt.expectIncompleteJob != nil { + for _, j := range allJobsInRun { + if slices.Contains(tt.expectIncompleteJob, j.Name) { + m, _, err := j.HasIncompleteMatrix() + require.NoError(t, err) + r, _, _, err := j.HasIncompleteRunsOn() + require.NoError(t, err) + w, _, _, err := j.HasIncompleteWith() + require.NoError(t, err) + assert.True(t, m || r || w, "job %s was expected to still be marked as incomplete", j.Name) + } + } + } + + if tt.localReusableWorkflowCallArgs != nil { + require.Len(t, localReusableCalled, 1) + assert.Equal(t, tt.localReusableWorkflowCallArgs, localReusableCalled[0]) + assert.Equal(t, 1, cleanupCallCount) + } } else if tt.preExecutionError != 0 { // expectations are that the ActionRun has a populated PreExecutionError, is marked as failed actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: blockedJob.RunID}) diff --git a/services/actions/run.go b/services/actions/run.go index 9bc93bc68e..64228938f8 100644 --- a/services/actions/run.go +++ b/services/actions/run.go @@ -126,7 +126,7 @@ func checkJobWillRevisit(ctx context.Context, job *actions_model.ActionRunJob) ( // Check to ensure that a job marked with `IncompleteMatrix` doesn't refer to a job that it doesn't have listed in // `needs`. If that state is discovered, fail the job and mark a PreExecutionError on the run. - isIncompleteMatrix, matrixNeeds, err := job.IsIncompleteMatrix() + isIncompleteMatrix, matrixNeeds, err := job.HasIncompleteMatrix() if err != nil { return false, err } @@ -164,11 +164,11 @@ func checkJobWillRevisit(ctx context.Context, job *actions_model.ActionRunJob) ( func checkJobRunsOnStaticMatrixError(ctx context.Context, job *actions_model.ActionRunJob) (bool, error) { // If a job has a `runs-on` field that references a matrix dimension like `runs-on: ${{ matrix.platorm }}`, and - // `platform` is not part of the job's matrix at all, then it will be tagged as `IsIncompleteRunsOn` and will be + // `platform` is not part of the job's matrix at all, then it will be tagged as `HasIncompleteRunsOn` and will be // blocked forever. This only applies if the matrix is static -- that is, the job isn't also tagged - // `IsIncompleteMatrix` and the matrix is yet to be fully defined. + // `HasIncompleteMatrix` and the matrix is yet to be fully defined. - isIncompleteRunsOn, _, matrixReference, err := job.IsIncompleteRunsOn() + isIncompleteRunsOn, _, matrixReference, err := job.HasIncompleteRunsOn() if err != nil { return false, err } else if !isIncompleteRunsOn || matrixReference == nil { @@ -176,7 +176,7 @@ func checkJobRunsOnStaticMatrixError(ctx context.Context, job *actions_model.Act return false, nil } - isIncompleteMatrix, _, err := job.IsIncompleteMatrix() + isIncompleteMatrix, _, err := job.HasIncompleteMatrix() if err != nil { return false, err } else if isIncompleteMatrix {