From 60f66a9bfdb60414ca8470b9b9a77fe56e1f2b83 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Mon, 8 Jun 2026 00:39:06 -0600 Subject: [PATCH] enhance(actions): improve reusable workflow `uses` handling and cancellation (#37991) Follow up #37478 ## Changes 1. #37478 doesn't support absolute URL in `uses`. This PR provides partial support for URL-style reusable workflow references. A reusable workflow can now be referenced by an absolute URL, as long as it points to the local Gitea instance: ```yaml jobs: call: uses: https://your-gitea.example.com/OWNER/REPO/.gitea/workflows/ci.yaml@v1 ``` 2. Show an error message in the UI for invalid `uses`. image 3. Fix reusable caller cancellation issue. A reusable caller's status is aggregated from its children, so cancellation should processes a caller's descendants deepest-first. --------- Signed-off-by: Zettat123 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: bircni Co-authored-by: Giteabot --- models/actions/run_job.go | 21 ++++--- models/actions/run_job_test.go | 66 ++++++++++++++++++++++ options/locale/locale_en-US.json | 1 + routers/web/repo/actions/actions.go | 45 ++++++++++----- services/actions/reusable_workflow.go | 26 ++++++++- services/actions/reusable_workflow_test.go | 44 +++++++++++++++ 6 files changed, 179 insertions(+), 24 deletions(-) diff --git a/models/actions/run_job.go b/models/actions/run_job.go index caf66ca451..df01546fd8 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -4,6 +4,7 @@ package actions import ( + "cmp" "context" "fmt" "slices" @@ -671,18 +672,18 @@ func cancelOneJob(ctx context.Context, job *ActionRunJob) (*ActionRunJob, error) func cancelReusableCaller(ctx context.Context, caller *ActionRunJob) ([]*ActionRunJob, error) { cancelledJobs := make([]*ActionRunJob, 0) - if c, err := cancelOneJob(ctx, caller); err != nil { - return cancelledJobs, err - } else if c != nil { - cancelledJobs = append(cancelledJobs, c) - } - attemptJobs, err := GetRunJobsByRunAndAttemptID(ctx, caller.RunID, caller.RunAttemptID) if err != nil { return cancelledJobs, err } - for _, c := range CollectAllDescendantJobs(caller, attemptJobs) { + // Cancel descendants deepest-first, then the caller: a caller's status is aggregated from its children, + // so each child must reach its final state before its parent caller is re-aggregated. + // A child's ID always exceeds its parent's, so descending ID is a valid deepest-first order. + descendants := CollectAllDescendantJobs(caller, attemptJobs) + slices.SortFunc(descendants, func(a, b *ActionRunJob) int { return cmp.Compare(b.ID, a.ID) }) + + for _, c := range descendants { cancelled, err := cancelOneJob(ctx, c) if err != nil { return cancelledJobs, err @@ -691,5 +692,11 @@ func cancelReusableCaller(ctx context.Context, caller *ActionRunJob) ([]*ActionR cancelledJobs = append(cancelledJobs, cancelled) } } + + if c, err := cancelOneJob(ctx, caller); err != nil { + return cancelledJobs, err + } else if c != nil { + cancelledJobs = append(cancelledJobs, c) + } return cancelledJobs, nil } diff --git a/models/actions/run_job_test.go b/models/actions/run_job_test.go index a9e07ce0cf..4437b5906d 100644 --- a/models/actions/run_job_test.go +++ b/models/actions/run_job_test.go @@ -131,3 +131,69 @@ func TestGetPriorAttemptChildrenByParent(t *testing.T) { assertAttempt1Children(t, out) }) } + +// A reusable caller subtree with a Blocked descendant (e.g. a nested caller stuck on an invalid `uses:`) must aggregate to Cancelled, when the run is cancelled. +func TestCancelJobs_NestedBlockedReusableCaller(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + ctx := t.Context() + + run := &ActionRun{ + Title: "cancel-nested-caller", + RepoID: 4, + Index: 9701, + OwnerID: 1, + WorkflowID: "caller.yaml", + TriggerUserID: 1, + Ref: "refs/heads/master", + CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", + Event: "push", + TriggerEvent: "push", + EventPayload: "{}", + Status: StatusBlocked, + } + require.NoError(t, db.Insert(ctx, run)) + + attempt := &ActionRunAttempt{RepoID: run.RepoID, RunID: run.ID, Attempt: 1, TriggerUserID: 1, Status: StatusBlocked} + require.NoError(t, db.Insert(ctx, attempt)) + run.LatestAttemptID = attempt.ID + require.NoError(t, UpdateRun(ctx, run, "latest_attempt_id")) + + newJob := func(name string, attemptJobID, parentID int64, callUses string) *ActionRunJob { + job := &ActionRunJob{ + RunID: run.ID, + RunAttemptID: attempt.ID, + RepoID: run.RepoID, + OwnerID: run.OwnerID, + CommitSHA: run.CommitSHA, + Name: name, + JobID: name, + Attempt: 1, + Status: StatusBlocked, + AttemptJobID: attemptJobID, + IsReusableCaller: true, + CallUses: callUses, + ParentJobID: parentID, + } + require.NoError(t, db.Insert(ctx, job)) + return job + } + + // outer: a valid top-level caller that expanded; inner: a nested caller stuck Blocked (invalid uses, never expands). + outer := newJob("outer", 1, 0, "./.gitea/workflows/lib.yml") + inner := newJob("inner", 2, outer.ID, "https://other.example.com/o/r/.gitea/workflows/ci.yml@v1") + + // Cancel all jobs of the attempt, ordered by id (parent before child). + jobs, err := GetRunJobsByRunAndAttemptID(ctx, run.ID, attempt.ID) + require.NoError(t, err) + _, err = CancelJobs(ctx, jobs) + require.NoError(t, err) + + for _, j := range []*ActionRunJob{outer, inner} { + got := unittest.AssertExistsAndLoadBean(t, &ActionRunJob{ID: j.ID}) + assert.Equal(t, StatusCancelled, got.Status, "job %q should be cancelled", j.JobID) + } + gotAttempt := unittest.AssertExistsAndLoadBean(t, &ActionRunAttempt{ID: attempt.ID}) + assert.Equal(t, StatusCancelled, gotAttempt.Status, "attempt must aggregate to Cancelled") + gotRun := unittest.AssertExistsAndLoadBean(t, &ActionRun{ID: run.ID}) + assert.Equal(t, StatusCancelled, gotRun.Status, "run must aggregate to Cancelled, not stay Blocked") +} diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 51a9797742..9595baebed 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -3774,6 +3774,7 @@ "actions.runs.no_matching_online_runner_helper": "No matching online runner with label: %s", "actions.runs.no_job_without_needs": "The workflow must contain at least one job without dependencies.", "actions.runs.no_job": "The workflow must contain at least one job", + "actions.runs.invalid_reusable_workflow_uses": "Invalid reusable workflow \"uses\": %s", "actions.runs.actor": "Actor", "actions.runs.status": "Status", "actions.runs.actors_no_select": "All actors", diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index 5ae0d14a6f..9c5e1664de 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -27,6 +27,7 @@ import ( "gitea.dev/modules/templates" "gitea.dev/modules/util" shared_user "gitea.dev/routers/web/shared/user" + actions_service "gitea.dev/services/actions" "gitea.dev/services/context" "gitea.dev/services/convert" @@ -208,12 +209,20 @@ func prepareWorkflowTemplate(ctx *context.Context, commit *git.Commit) (workflow if !hasJobWithoutNeeds && len(j.Needs()) == 0 { hasJobWithoutNeeds = true } + if j.Uses != "" { + if _, err := actions_service.ResolveUses(ctx, j.Uses); err != nil { + workflow.ErrMsg = ctx.Locale.TrString("actions.runs.invalid_reusable_workflow_uses", err.Error()) + break + } + } } - if !hasJobWithoutNeeds { - workflow.ErrMsg = ctx.Locale.TrString("actions.runs.no_job_without_needs") - } - if emptyJobsNumber == len(wf.Jobs) { - workflow.ErrMsg = ctx.Locale.TrString("actions.runs.no_job") + if workflow.ErrMsg == "" { + if !hasJobWithoutNeeds { + workflow.ErrMsg = ctx.Locale.TrString("actions.runs.no_job_without_needs") + } + if emptyJobsNumber == len(wf.Jobs) { + workflow.ErrMsg = ctx.Locale.TrString("actions.runs.no_job") + } } workflows = append(workflows, workflow) } @@ -352,7 +361,7 @@ func prepareWorkflowList(ctx *context.Context, workflows []WorkflowInfo, otherWo return } for _, run := range runs { - if !run.Status.In(actions_model.StatusWaiting, actions_model.StatusRunning) { + if !run.Status.In(actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked) { continue } jobs, err := actions_model.GetLatestAttemptJobsByRepoAndRunID(ctx, run.RepoID, run.ID) @@ -361,23 +370,31 @@ func prepareWorkflowList(ctx *context.Context, workflows []WorkflowInfo, otherWo return } for _, job := range jobs { - if !job.Status.IsWaiting() { + if !job.Status.In(actions_model.StatusWaiting, actions_model.StatusBlocked) { continue } if err := actions.ValidateWorkflowContent(job.WorkflowPayload); err != nil { runErrors[run.ID] = ctx.Locale.TrString("actions.runs.invalid_workflow_helper", err.Error()) break } - hasOnlineRunner := false - for _, runner := range runners { - if !runner.IsDisabled && runner.CanMatchLabels(job.RunsOn) { - hasOnlineRunner = true + if job.CallUses != "" { + if _, err := actions_service.ResolveUses(ctx, job.CallUses); err != nil { + runErrors[run.ID] = ctx.Locale.TrString("actions.runs.invalid_reusable_workflow_uses", err.Error()) break } } - if !hasOnlineRunner { - runErrors[run.ID] = ctx.Locale.TrString("actions.runs.no_matching_online_runner_helper", strings.Join(job.RunsOn, ",")) - break + if job.Status.IsWaiting() { + hasOnlineRunner := false + for _, runner := range runners { + if !runner.IsDisabled && runner.CanMatchLabels(job.RunsOn) { + hasOnlineRunner = true + break + } + } + if !hasOnlineRunner { + runErrors[run.ID] = ctx.Locale.TrString("actions.runs.no_matching_online_runner_helper", strings.Join(job.RunsOn, ",")) + break + } } } } diff --git a/services/actions/reusable_workflow.go b/services/actions/reusable_workflow.go index 9b5ecdef6f..65a6acfbd0 100644 --- a/services/actions/reusable_workflow.go +++ b/services/actions/reusable_workflow.go @@ -6,6 +6,7 @@ package actions import ( "context" "fmt" + "strings" actions_model "gitea.dev/models/actions" "gitea.dev/models/db" @@ -15,7 +16,9 @@ import ( "gitea.dev/modules/actions/jobparser" "gitea.dev/modules/container" "gitea.dev/modules/gitrepo" + "gitea.dev/modules/httplib" "gitea.dev/modules/json" + "gitea.dev/modules/setting" api "gitea.dev/modules/structs" "gitea.dev/modules/util" "gitea.dev/services/convert" @@ -149,10 +152,10 @@ func expandReusableWorkflowCaller(ctx context.Context, run *actions_model.Action return fmt.Errorf("parse caller job %d: %w", caller.ID, err) } - // 3. Load called-workflow source. - ref, err := jobparser.ParseUses(parsedJob.Uses) + // 3. Resolve `uses` and load called-workflow source. + ref, err := ResolveUses(ctx, parsedJob.Uses) if err != nil { - return fmt.Errorf("parse uses %q: %w", parsedJob.Uses, err) + return fmt.Errorf("resolve uses %q: %w", parsedJob.Uses, err) } content, contentSourceRepoID, contentSourceCommitSHA, err := loadReusableWorkflowSource(ctx, run, caller, ref) if err != nil { @@ -340,3 +343,20 @@ func insertCallerChildren(ctx context.Context, run *actions_model.ActionRun, att } return nil } + +// ResolveUses normalizes and parses a reusable workflow `uses:` value. +// It first rewrites an absolute URL pointing to this instance into the cross-repo form (rejecting external URLs), +// then validates the syntax via jobparser.ParseUses. +func ResolveUses(ctx context.Context, uses string) (*jobparser.UsesRef, error) { + // Rewrite a local-instance URL to the equivalent cross-repo form "owner/repo/.gitea/workflows/file.yml@ref". + if strings.HasPrefix(uses, "http://") || strings.HasPrefix(uses, "https://") { + // ParseGiteaSiteURL returns nil for URLs that do not belong to this instance. + gsu := httplib.ParseGiteaSiteURL(ctx, uses) + if gsu == nil { + return nil, fmt.Errorf("unsupported reusable workflow URL %q: an absolute URL must point to this Gitea instance (%s)", uses, setting.AppURL) + } + // RoutePath is the instance-relative path (AppSubURL already stripped), e.g. "/owner/repo/.gitea/workflows/file.yml@ref". + uses = strings.TrimPrefix(gsu.RoutePath, "/") + } + return jobparser.ParseUses(uses) +} diff --git a/services/actions/reusable_workflow_test.go b/services/actions/reusable_workflow_test.go index cadb26a851..a7bb41ba8a 100644 --- a/services/actions/reusable_workflow_test.go +++ b/services/actions/reusable_workflow_test.go @@ -10,6 +10,9 @@ import ( actions_model "gitea.dev/models/actions" "gitea.dev/models/db" "gitea.dev/models/unittest" + "gitea.dev/modules/actions/jobparser" + "gitea.dev/modules/setting" + "gitea.dev/modules/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -132,3 +135,44 @@ func buildCallerChain(t *testing.T, callerUses ...string) []*actions_model.Actio } return jobs } + +func TestResolveUses(t *testing.T) { + defer test.MockVariableValue(&setting.AppURL, "https://gitea.example.com/sub/")() + defer test.MockVariableValue(&setting.AppSubURL, "/sub")() + ctx := t.Context() + + t.Run("LocalForms", func(t *testing.T) { + // Same-repo and cross-repo forms are not URLs and are parsed as-is. + ref, err := ResolveUses(ctx, "./.gitea/workflows/build.yml") + require.NoError(t, err) + assert.Equal(t, jobparser.UsesRef{Kind: jobparser.UsesKindLocalSameRepo, Path: ".gitea/workflows/build.yml"}, *ref) + + ref, err = ResolveUses(ctx, "owner/repo/.gitea/workflows/build.yml@v1") + require.NoError(t, err) + assert.Equal(t, jobparser.UsesRef{Kind: jobparser.UsesKindLocalCrossRepo, Owner: "owner", Repo: "repo", Path: ".gitea/workflows/build.yml", Ref: "v1"}, *ref) + }) + + t.Run("LocalInstanceURL", func(t *testing.T) { + // An absolute URL on this instance (incl. AppSubURL) resolves to the equivalent cross-repo ref. + ref, err := ResolveUses(ctx, "https://gitea.example.com/sub/owner/repo/.gitea/workflows/ci.yml@refs/heads/main") + require.NoError(t, err) + assert.Equal(t, jobparser.UsesRef{Kind: jobparser.UsesKindLocalCrossRepo, Owner: "owner", Repo: "repo", Path: ".gitea/workflows/ci.yml", Ref: "refs/heads/main"}, *ref) + }) + + t.Run("InvalidSyntax", func(t *testing.T) { + for _, in := range []string{ + "owner/.gitea/workflows/foo.yml", // missing repo segment + "owner/repo/.gitea/workflows/foo.yml", // missing @ref + "https://gitea.example.com/sub/repo/.gitea/workflows/ci.yml@refs/heads/main", // local absolute URL but missing owner + "not a valid uses at all", + } { + _, err := ResolveUses(ctx, in) + require.Error(t, err, "in = %s", in) + } + }) + + t.Run("ForeignURL", func(t *testing.T) { + _, err := ResolveUses(ctx, "https://other.gitea-example.com/owner/repo/.gitea/workflows/ci.yaml@v1") + assert.ErrorContains(t, err, "must point to this Gitea instance") + }) +}