From 4804bb3dfb8c4b07bf5ab0195eb0e546fae707b2 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 10 Oct 2025 23:46:41 +0200 Subject: [PATCH 1/4] Refactor ActionRunJob parsing into a reusable function --- models/actions/run_job.go | 15 +++++++++++++++ models/actions/task.go | 8 ++------ services/actions/concurrency.go | 12 +++--------- services/actions/job_emitter.go | 7 +++---- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 9c7c3658d77c3..aea73e88b2554 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -13,6 +13,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" + "github.com/nektos/act/pkg/jobparser" "xorm.io/builder" ) @@ -99,6 +100,20 @@ func (job *ActionRunJob) LoadAttributes(ctx context.Context) error { return job.Run.LoadAttributes(ctx) } +// Load the job structure from the ActionRunJob +func (job *ActionRunJob) ParseJob() (*jobparser.Job, error) { + // singleWorkflows is created from an ActionJob, which always contains exactly a single job's YAML definition. + // Ideally it shouldn't be called "Workflow", it is just a job with global workflow fields + trigger + parsedWorkflows, err := jobparser.Parse(job.WorkflowPayload) + if err != nil { + return nil, fmt.Errorf("parse workflow of job %d: %w", job.ID, err) + } else if len(parsedWorkflows) != 1 { + return nil, fmt.Errorf("workflow of job %d: not single workflow", job.ID) + } + _, workflowJob := parsedWorkflows[0].Job() + return workflowJob, nil +} + func GetRunJobByID(ctx context.Context, id int64) (*ActionRunJob, error) { var job ActionRunJob has, err := db.GetEngine(ctx).Where("id=?", id).Get(&job) diff --git a/models/actions/task.go b/models/actions/task.go index c1306a8418248..7417af8b45186 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -21,7 +21,6 @@ import ( runnerv1 "code.gitea.io/actions-proto-go/runner/v1" lru "github.com/hashicorp/golang-lru/v2" - "github.com/nektos/act/pkg/jobparser" "google.golang.org/protobuf/types/known/timestamppb" "xorm.io/builder" ) @@ -278,13 +277,10 @@ func CreateTaskForRunner(ctx context.Context, runner *ActionRunner) (*ActionTask return nil, false, err } - parsedWorkflows, err := jobparser.Parse(job.WorkflowPayload) + workflowJob, err := job.ParseJob() if err != nil { - return nil, false, fmt.Errorf("parse workflow of job %d: %w", job.ID, err) - } else if len(parsedWorkflows) != 1 { - return nil, false, fmt.Errorf("workflow of job %d: not single workflow", job.ID) + return nil, false, fmt.Errorf("load job %d: %w", job.ID, err) } - _, workflowJob := parsedWorkflows[0].Job() if _, err := e.Insert(task); err != nil { return nil, false, err diff --git a/services/actions/concurrency.go b/services/actions/concurrency.go index 2910e47739557..090830270943a 100644 --- a/services/actions/concurrency.go +++ b/services/actions/concurrency.go @@ -5,7 +5,6 @@ package actions import ( "context" - "errors" "fmt" actions_model "code.gitea.io/gitea/models/actions" @@ -91,17 +90,12 @@ func EvaluateJobConcurrencyFillModel(ctx context.Context, run *actions_model.Act return fmt.Errorf("get inputs: %w", err) } - // singleWorkflows is created from an ActionJob, which always contains exactly a single job's YAML definition. - // Ideally it shouldn't be called "Workflow", it is just a job with global workflow fields + trigger - singleWorkflows, err := jobparser.Parse(actionRunJob.WorkflowPayload) + workflowJob, err := actionRunJob.ParseJob() if err != nil { - return fmt.Errorf("parse single workflow: %w", err) - } else if len(singleWorkflows) != 1 { - return errors.New("not single workflow") + return fmt.Errorf("load job %d: %w", actionRunJob.ID, err) } - _, singleWorkflowJob := singleWorkflows[0].Job() - actionRunJob.ConcurrencyGroup, actionRunJob.ConcurrencyCancel, err = jobparser.EvaluateConcurrency(&rawConcurrency, actionRunJob.JobID, singleWorkflowJob, actionsJobCtx, jobResults, vars, inputs) + actionRunJob.ConcurrencyGroup, actionRunJob.ConcurrencyCancel, err = jobparser.EvaluateConcurrency(&rawConcurrency, actionRunJob.JobID, workflowJob, actionsJobCtx, jobResults, vars, inputs) if err != nil { return fmt.Errorf("evaluate concurrency: %w", err) } diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index c6578eae65163..762d28e9cc238 100644 --- a/services/actions/job_emitter.go +++ b/services/actions/job_emitter.go @@ -18,7 +18,6 @@ import ( "code.gitea.io/gitea/modules/util" notify_service "code.gitea.io/gitea/services/notify" - "github.com/nektos/act/pkg/jobparser" "xorm.io/builder" ) @@ -305,9 +304,9 @@ func (r *jobStatusResolver) resolveCheckNeeds(id int64) (allDone, allSucceed boo } func (r *jobStatusResolver) resolveJobHasIfCondition(actionRunJob *actions_model.ActionRunJob) (hasIf bool) { - if wfJobs, _ := jobparser.Parse(actionRunJob.WorkflowPayload); len(wfJobs) == 1 { - _, wfJob := wfJobs[0].Job() - hasIf = len(wfJob.If.Value) > 0 + // FIXME evaluate this on the server side + if job, err := actionRunJob.ParseJob(); err == nil { + return len(job.If.Value) > 0 } return hasIf } From b13db78cfea1e9dc658c7f420a17faaf763870ef Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 10 Oct 2025 23:55:54 +0200 Subject: [PATCH 2/4] fix fmt --- models/actions/run_job.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/actions/run_job.go b/models/actions/run_job.go index aea73e88b2554..95a4a4056a7fb 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -13,8 +13,8 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" - "github.com/nektos/act/pkg/jobparser" + "github.com/nektos/act/pkg/jobparser" "xorm.io/builder" ) From 0c14f305da6e89b8bd46ee6e7753c9574ab797e4 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 11 Oct 2025 08:27:33 +0800 Subject: [PATCH 3/4] check whether parsedWorkflows[0].Job() is nil --- models/actions/run_job.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 95a4a4056a7fb..57379c4e251e9 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -100,17 +100,21 @@ func (job *ActionRunJob) LoadAttributes(ctx context.Context) error { return job.Run.LoadAttributes(ctx) } -// Load the job structure from the ActionRunJob +// ParseJob parses the job structure from the ActionRunJob.WorkflowPayload func (job *ActionRunJob) ParseJob() (*jobparser.Job, error) { - // singleWorkflows is created from an ActionJob, which always contains exactly a single job's YAML definition. + // job.WorkflowPayload is a SingleWorkflow created from an ActionRun's workflow, which exactly contains this job's YAML definition. // Ideally it shouldn't be called "Workflow", it is just a job with global workflow fields + trigger parsedWorkflows, err := jobparser.Parse(job.WorkflowPayload) if err != nil { - return nil, fmt.Errorf("parse workflow of job %d: %w", job.ID, err) + return nil, fmt.Errorf("job %d single workflow: unable to parse: %w", job.ID, err) } else if len(parsedWorkflows) != 1 { - return nil, fmt.Errorf("workflow of job %d: not single workflow", job.ID) + return nil, fmt.Errorf("job %d single workflow: not single workflow", job.ID) } _, workflowJob := parsedWorkflows[0].Job() + if workflowJob == nil { + // it shouldn't happen, and since the callers don't check nil, so return an error instead of nil + return nil, util.ErrorWrap(util.ErrNotExist, "job %d single workflow: payload doesn't contain a job", job.JobID) + } return workflowJob, nil } From 7dd0bc0d25f194a717d3094e52ca46a16016140b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 11 Oct 2025 08:37:02 +0800 Subject: [PATCH 4/4] fix typo --- models/actions/run_job.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 57379c4e251e9..f72a7040e3359 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -113,7 +113,7 @@ func (job *ActionRunJob) ParseJob() (*jobparser.Job, error) { _, workflowJob := parsedWorkflows[0].Job() if workflowJob == nil { // it shouldn't happen, and since the callers don't check nil, so return an error instead of nil - return nil, util.ErrorWrap(util.ErrNotExist, "job %d single workflow: payload doesn't contain a job", job.JobID) + return nil, util.ErrorWrap(util.ErrNotExist, "job %d single workflow: payload doesn't contain a job", job.ID) } return workflowJob, nil }