mirror of
https://github.com/go-gitea/gitea.git
synced 2026-09-08 14:03:24 +09:00
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`. <img width="1600" alt="image" src="https://github.com/user-attachments/assets/21b34e61-bf10-4af1-b9fd-4ee4e9fde049" /> 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 <zettat123@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: bircni <bircni@icloud.com> Co-authored-by: Giteabot <teabot@gitea.io>
This commit is contained in:
co-authored by
Copilot Autofix powered by AI
bircni
Giteabot
parent
1e9ea9c8f5
commit
60f66a9bfd
@@ -4,6 +4,7 @@
|
|||||||
package actions
|
package actions
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"cmp"
|
||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
"slices"
|
"slices"
|
||||||
@@ -671,18 +672,18 @@ func cancelOneJob(ctx context.Context, job *ActionRunJob) (*ActionRunJob, error)
|
|||||||
func cancelReusableCaller(ctx context.Context, caller *ActionRunJob) ([]*ActionRunJob, error) {
|
func cancelReusableCaller(ctx context.Context, caller *ActionRunJob) ([]*ActionRunJob, error) {
|
||||||
cancelledJobs := make([]*ActionRunJob, 0)
|
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)
|
attemptJobs, err := GetRunJobsByRunAndAttemptID(ctx, caller.RunID, caller.RunAttemptID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return cancelledJobs, err
|
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)
|
cancelled, err := cancelOneJob(ctx, c)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return cancelledJobs, err
|
return cancelledJobs, err
|
||||||
@@ -691,5 +692,11 @@ func cancelReusableCaller(ctx context.Context, caller *ActionRunJob) ([]*ActionR
|
|||||||
cancelledJobs = append(cancelledJobs, cancelled)
|
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
|
return cancelledJobs, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -131,3 +131,69 @@ func TestGetPriorAttemptChildrenByParent(t *testing.T) {
|
|||||||
assertAttempt1Children(t, out)
|
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")
|
||||||
|
}
|
||||||
|
|||||||
@@ -3774,6 +3774,7 @@
|
|||||||
"actions.runs.no_matching_online_runner_helper": "No matching online runner with label: %s",
|
"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_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.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.actor": "Actor",
|
||||||
"actions.runs.status": "Status",
|
"actions.runs.status": "Status",
|
||||||
"actions.runs.actors_no_select": "All actors",
|
"actions.runs.actors_no_select": "All actors",
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ import (
|
|||||||
"gitea.dev/modules/templates"
|
"gitea.dev/modules/templates"
|
||||||
"gitea.dev/modules/util"
|
"gitea.dev/modules/util"
|
||||||
shared_user "gitea.dev/routers/web/shared/user"
|
shared_user "gitea.dev/routers/web/shared/user"
|
||||||
|
actions_service "gitea.dev/services/actions"
|
||||||
"gitea.dev/services/context"
|
"gitea.dev/services/context"
|
||||||
"gitea.dev/services/convert"
|
"gitea.dev/services/convert"
|
||||||
|
|
||||||
@@ -208,12 +209,20 @@ func prepareWorkflowTemplate(ctx *context.Context, commit *git.Commit) (workflow
|
|||||||
if !hasJobWithoutNeeds && len(j.Needs()) == 0 {
|
if !hasJobWithoutNeeds && len(j.Needs()) == 0 {
|
||||||
hasJobWithoutNeeds = true
|
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 {
|
if workflow.ErrMsg == "" {
|
||||||
workflow.ErrMsg = ctx.Locale.TrString("actions.runs.no_job_without_needs")
|
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 emptyJobsNumber == len(wf.Jobs) {
|
||||||
|
workflow.ErrMsg = ctx.Locale.TrString("actions.runs.no_job")
|
||||||
|
}
|
||||||
}
|
}
|
||||||
workflows = append(workflows, workflow)
|
workflows = append(workflows, workflow)
|
||||||
}
|
}
|
||||||
@@ -352,7 +361,7 @@ func prepareWorkflowList(ctx *context.Context, workflows []WorkflowInfo, otherWo
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
for _, run := range runs {
|
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
|
continue
|
||||||
}
|
}
|
||||||
jobs, err := actions_model.GetLatestAttemptJobsByRepoAndRunID(ctx, run.RepoID, run.ID)
|
jobs, err := actions_model.GetLatestAttemptJobsByRepoAndRunID(ctx, run.RepoID, run.ID)
|
||||||
@@ -361,23 +370,31 @@ func prepareWorkflowList(ctx *context.Context, workflows []WorkflowInfo, otherWo
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
for _, job := range jobs {
|
for _, job := range jobs {
|
||||||
if !job.Status.IsWaiting() {
|
if !job.Status.In(actions_model.StatusWaiting, actions_model.StatusBlocked) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if err := actions.ValidateWorkflowContent(job.WorkflowPayload); err != nil {
|
if err := actions.ValidateWorkflowContent(job.WorkflowPayload); err != nil {
|
||||||
runErrors[run.ID] = ctx.Locale.TrString("actions.runs.invalid_workflow_helper", err.Error())
|
runErrors[run.ID] = ctx.Locale.TrString("actions.runs.invalid_workflow_helper", err.Error())
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
hasOnlineRunner := false
|
if job.CallUses != "" {
|
||||||
for _, runner := range runners {
|
if _, err := actions_service.ResolveUses(ctx, job.CallUses); err != nil {
|
||||||
if !runner.IsDisabled && runner.CanMatchLabels(job.RunsOn) {
|
runErrors[run.ID] = ctx.Locale.TrString("actions.runs.invalid_reusable_workflow_uses", err.Error())
|
||||||
hasOnlineRunner = true
|
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if !hasOnlineRunner {
|
if job.Status.IsWaiting() {
|
||||||
runErrors[run.ID] = ctx.Locale.TrString("actions.runs.no_matching_online_runner_helper", strings.Join(job.RunsOn, ","))
|
hasOnlineRunner := false
|
||||||
break
|
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
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ package actions
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"strings"
|
||||||
|
|
||||||
actions_model "gitea.dev/models/actions"
|
actions_model "gitea.dev/models/actions"
|
||||||
"gitea.dev/models/db"
|
"gitea.dev/models/db"
|
||||||
@@ -15,7 +16,9 @@ import (
|
|||||||
"gitea.dev/modules/actions/jobparser"
|
"gitea.dev/modules/actions/jobparser"
|
||||||
"gitea.dev/modules/container"
|
"gitea.dev/modules/container"
|
||||||
"gitea.dev/modules/gitrepo"
|
"gitea.dev/modules/gitrepo"
|
||||||
|
"gitea.dev/modules/httplib"
|
||||||
"gitea.dev/modules/json"
|
"gitea.dev/modules/json"
|
||||||
|
"gitea.dev/modules/setting"
|
||||||
api "gitea.dev/modules/structs"
|
api "gitea.dev/modules/structs"
|
||||||
"gitea.dev/modules/util"
|
"gitea.dev/modules/util"
|
||||||
"gitea.dev/services/convert"
|
"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)
|
return fmt.Errorf("parse caller job %d: %w", caller.ID, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// 3. Load called-workflow source.
|
// 3. Resolve `uses` and load called-workflow source.
|
||||||
ref, err := jobparser.ParseUses(parsedJob.Uses)
|
ref, err := ResolveUses(ctx, parsedJob.Uses)
|
||||||
if err != nil {
|
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)
|
content, contentSourceRepoID, contentSourceCommitSHA, err := loadReusableWorkflowSource(ctx, run, caller, ref)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -340,3 +343,20 @@ func insertCallerChildren(ctx context.Context, run *actions_model.ActionRun, att
|
|||||||
}
|
}
|
||||||
return nil
|
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)
|
||||||
|
}
|
||||||
|
|||||||
@@ -10,6 +10,9 @@ import (
|
|||||||
actions_model "gitea.dev/models/actions"
|
actions_model "gitea.dev/models/actions"
|
||||||
"gitea.dev/models/db"
|
"gitea.dev/models/db"
|
||||||
"gitea.dev/models/unittest"
|
"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/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
@@ -132,3 +135,44 @@ func buildCallerChain(t *testing.T, callerUses ...string) []*actions_model.Actio
|
|||||||
}
|
}
|
||||||
return jobs
|
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")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user