diff --git a/models/actions/pre_execution_errors.go b/models/actions/pre_execution_errors.go index 64c9b17727..000d59d71d 100644 --- a/models/actions/pre_execution_errors.go +++ b/models/actions/pre_execution_errors.go @@ -30,6 +30,7 @@ const ( ErrorCodeIncompleteWithMissingOutput ErrorCodeIncompleteWithMissingMatrixDimension ErrorCodeIncompleteWithUnknownCause + ErrorCodeUnknownJobInNeeds ) func TranslatePreExecutionError(lang translation.Locale, run *ActionRun) string { @@ -69,6 +70,8 @@ func TranslatePreExecutionError(lang translation.Locale, run *ActionRun) string 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...) + case ErrorCodeUnknownJobInNeeds: + return lang.TrString("actions.workflow.unknown_job_in_needs", run.PreExecutionErrorDetails...) } return fmt.Sprintf(" 0 if err := rerunJob(ctx, j, shouldBlock); err != nil { @@ -550,6 +559,11 @@ func Rerun(ctx *app_context.Context) { var redirectURL string for _, j := range rerunJobs { + if !j.CanBeRerun() { + ctx.JSONError(ctx.Locale.Tr("actions.workflow.job_rerun_impossible")) + return + } + // jobs other than the specified one should be set to "blocked" status shouldBlock := j.JobID != job.JobID if err := rerunJob(ctx, j, shouldBlock); err != nil { diff --git a/routers/web/repo/actions/view_test.go b/routers/web/repo/actions/view_test.go index 8d8e82a007..04d32f191b 100644 --- a/routers/web/repo/actions/view_test.go +++ b/routers/web/repo/actions/view_test.go @@ -520,31 +520,48 @@ func TestActionsViewRedirectToLatestAttempt(t *testing.T) { } func TestActionsRerun(t *testing.T) { - defer unittest.OverrideFixtures("routers/web/repo/actions/TestActionsRerun")() - unittest.PrepareTestEnv(t) - tests := []struct { name string runIndex int64 jobIndex int64 expectedCode int expectedURL string + expectedBody string }{ { - name: "rerun all", - runIndex: 138574, - jobIndex: -1, - expectedURL: "https://try.gitea.io/user2/repo1/actions/runs/138574/jobs/0/attempt/3", + name: "rerun all", + runIndex: 138574, + jobIndex: -1, + expectedCode: 200, + expectedURL: "https://try.gitea.io/user2/repo1/actions/runs/138574/jobs/0/attempt/3", }, { - name: "rerun job", - runIndex: 138574, - jobIndex: 2, - expectedURL: "https://try.gitea.io/user2/repo1/actions/runs/138574/jobs/2/attempt/6", + name: "rerun job", + runIndex: 138574, + jobIndex: 2, + expectedCode: 200, + expectedURL: "https://try.gitea.io/user2/repo1/actions/runs/138574/jobs/2/attempt/6", + }, + { + name: "rerun workflow that cannot be run", + runIndex: 138575, + jobIndex: -1, + expectedCode: 400, + expectedBody: "{\"errorMessage\":\"actions.workflow.rerun_impossible\",\"renderFormat\":\"html\"}\n", + }, + { + name: "rerun job that cannot be run", + runIndex: 138575, + jobIndex: 1, + expectedCode: 400, + expectedBody: "{\"errorMessage\":\"actions.workflow.job_rerun_impossible\",\"renderFormat\":\"html\"}\n", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + defer unittest.OverrideFixtures("routers/web/repo/actions/TestActionsRerun")() + unittest.PrepareTestEnv(t) + ctx, resp := contexttest.MockContext(t, "user2/repo1/actions/runs/138574/rerun") contexttest.LoadUser(t, ctx, 2) contexttest.LoadRepo(t, ctx, 1) @@ -554,16 +571,22 @@ func TestActionsRerun(t *testing.T) { } Rerun(ctx) - require.Equal(t, http.StatusOK, resp.Result().StatusCode, "failure in Rerun(): %q", resp.Body.String()) - var actual redirectObject - err := json.Unmarshal(resp.Body.Bytes(), &actual) - require.NoError(t, err) + if tt.expectedCode < 300 { + require.Equal(t, tt.expectedCode, resp.Result().StatusCode, "failure in Rerun(): %q", resp.Body.String()) - // Note: this test isn't doing any functional testing of the Rerun handler's actual ability to set up a job - // rerun. This test was added when the redirect to the correct `attempt` was added and only covers that - // addition at this time. - assert.Equal(t, redirectObject{Redirect: tt.expectedURL}, actual) + var actual redirectObject + err := json.Unmarshal(resp.Body.Bytes(), &actual) + require.NoError(t, err) + + // Note: this test isn't doing any functional testing of the Rerun handler's actual ability to set up a job + // rerun. This test was added when the redirect to the correct `attempt` was added and only covers that + // addition at this time. + assert.Equal(t, redirectObject{Redirect: tt.expectedURL}, actual) + } else { + require.Equal(t, tt.expectedCode, resp.Result().StatusCode) + assert.Equal(t, tt.expectedBody, resp.Body.String()) + } }) } } diff --git a/services/actions/TestActions_consistencyCheckRun/action_run.yml b/services/actions/TestActions_consistencyCheckRun/action_run.yml index 321600adf9..0872f69b3a 100644 --- a/services/actions/TestActions_consistencyCheckRun/action_run.yml +++ b/services/actions/TestActions_consistencyCheckRun/action_run.yml @@ -88,3 +88,21 @@ updated: 1683636626 need_approval: 0 approved_by: 0 +- + id: 905 + title: "waiting workflow_dispatch run" + repo_id: 63 + owner_id: 2 + workflow_id: "running.yaml" + index: 9 + trigger_user_id: 2 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 5 # waiting + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 diff --git a/services/actions/TestActions_consistencyCheckRun/action_run_job.yml b/services/actions/TestActions_consistencyCheckRun/action_run_job.yml index 8d0adf3f58..e917d7d9a3 100644 --- a/services/actions/TestActions_consistencyCheckRun/action_run_job.yml +++ b/services/actions/TestActions_consistencyCheckRun/action_run_job.yml @@ -172,3 +172,47 @@ incomplete_runs_on_matrix: dimension: platform-oops-wrong-dimension incomplete_matrix: true +- + id: 607 + run_id: 905 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: job_1 + attempt: 1 + job_id: job_1 + task_id: 0 + status: 7 # blocked + runs_on: '["fedora"]' + needs: '[]' + workflow_payload: | + "on": + push: + jobs: + job_1: + runs-on: fedora + steps: + - run: echo "OK!" +- + id: 608 + run_id: 905 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: job_2 + attempt: 1 + job_id: job_2 + task_id: 0 + status: 7 # blocked + runs_on: '["fedora"]' + needs: '["unknown","Job_1"]' # invalid capitalization of job_1 + workflow_payload: | + "on": + push: + jobs: + job_2: + runs-on: fedora + steps: + - run: echo "OK!" diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index b23918e42f..593e17517a 100644 --- a/services/actions/job_emitter.go +++ b/services/actions/job_emitter.go @@ -136,6 +136,10 @@ type jobStatusResolver struct { jobMap map[int64]*actions_model.ActionRunJob } +// unknownJobID stores the ID of an unknown job that might be referenced in the workflow. The ID can be any number as +// long it does not match the ID of an existing job. +var unknownJobID int64 = -1 + func newJobStatusResolver(jobs actions_model.ActionJobList) *jobStatusResolver { idToJobs := make(map[string][]*actions_model.ActionRunJob, len(jobs)) jobMap := make(map[int64]*actions_model.ActionRunJob) @@ -149,8 +153,14 @@ func newJobStatusResolver(jobs actions_model.ActionJobList) *jobStatusResolver { for _, job := range jobs { statuses[job.ID] = job.Status for _, need := range job.Needs { - for _, v := range idToJobs[need] { - needs[job.ID] = append(needs[job.ID], v.ID) + neededJobs, ok := idToJobs[need] + if ok { + for _, v := range neededJobs { + needs[job.ID] = append(needs[job.ID], v.ID) + } + } else { + // Handles the case of an unknown job being referenced in `needs`, for example, `needs: ["unknown"]`. + needs[job.ID] = append(needs[job.ID], unknownJobID) } } } diff --git a/services/actions/job_emitter_test.go b/services/actions/job_emitter_test.go index 7d5853ce1a..44e9163102 100644 --- a/services/actions/job_emitter_test.go +++ b/services/actions/job_emitter_test.go @@ -231,6 +231,30 @@ __metadata: 3: actions_model.StatusFailure, }, }, + { + name: "blocked if needs are unknown", + jobs: actions_model.ActionJobList{ + {ID: 1, JobID: "build", Status: actions_model.StatusSuccess, Needs: []string{}}, + {ID: 2, JobID: "test", Status: actions_model.StatusBlocked, Needs: []string{"build", "unknown"}}, + }, + want: map[int64]actions_model.Status{}, + }, + { + name: "blocked if needs are unknown despite always()", + jobs: actions_model.ActionJobList{ + {ID: 1, JobID: "build", Status: actions_model.StatusSuccess, Needs: []string{}}, + {ID: 45, JobID: "test", Needs: []string{"build", "unknown"}, Status: actions_model.StatusBlocked, WorkflowPayload: []byte(` +on: push +jobs: + test: + runs-on: ubuntu-latest + needs: [build, unknown] + if: always() + steps: [] +`)}, + }, + want: map[int64]actions_model.Status{}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/services/actions/run.go b/services/actions/run.go index ed795f2a63..bee0b1581d 100644 --- a/services/actions/run.go +++ b/services/actions/run.go @@ -97,11 +97,17 @@ func FailRunPreExecutionError(ctx context.Context, run *actions_model.ActionRun, // Perform pre-execution checks that would affect the ability for a job to reach an executing stage. func consistencyCheckRun(ctx context.Context, run *actions_model.ActionRun) error { + var jobs actions_model.ActionJobList jobs, err := actions_model.GetRunJobsByRunID(ctx, run.ID) if err != nil { return err } + validJobIDs := jobs.GetJobIDs() for _, job := range jobs { + if unknownJobIDs, ok := job.AllNeedsExist(validJobIDs); !ok { + return FailRunPreExecutionError(ctx, run, actions_model.ErrorCodeUnknownJobInNeeds, + []any{job.JobID, strings.Join(unknownJobIDs, ", ")}) + } if stop, err := checkJobWillRevisit(ctx, job); err != nil { return err } else if stop { diff --git a/services/actions/run_test.go b/services/actions/run_test.go index e8a198c446..61e580aed1 100644 --- a/services/actions/run_test.go +++ b/services/actions/run_test.go @@ -137,6 +137,12 @@ func TestActions_consistencyCheckRun(t *testing.T) { name: "consistent: matrix missing dimension but matrix is dynamic", runID: 904, }, + { + name: "unknown job in needs", + runID: 905, + preExecutionError: actions_model.ErrorCodeUnknownJobInNeeds, + preExecutionErrorDetails: []any{"job_2", "unknown, Job_1"}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {