diff --git a/go.mod b/go.mod index 690c7ae3eb5..44e96db12d9 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( gitea.com/go-chi/session v0.0.0-20260708011333-ebced8a7a2d6 gitea.com/lunny/dingtalk_webhook v0.0.0-20171025031554-e3534c89ef96 gitea.com/lunny/levelqueue v0.4.2-0.20230414023320-3c0159fe0fe4 - gitea.dev/actionslib v1.0.0 + gitea.dev/actionslib v1.2.1 gitea.dev/sdk v1.2.0 github.com/42wim/httpsig v1.2.4 github.com/42wim/sshsig v0.0.0-20260317195500-b9f38cf0d432 diff --git a/go.sum b/go.sum index 0d765ab98be..ca80207b1bd 100644 --- a/go.sum +++ b/go.sum @@ -22,8 +22,8 @@ gitea.com/lunny/levelqueue v0.4.2-0.20230414023320-3c0159fe0fe4 h1:IFT+hup2xejHq gitea.com/lunny/levelqueue v0.4.2-0.20230414023320-3c0159fe0fe4/go.mod h1:HBqmLbz56JWpfEGG0prskAV97ATNRoj5LDmPicD22hU= gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a h1:lSA0F4e9A2NcQSqGqTOXqu2aRi/XEQxDCBwM8yJtE6s= gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a/go.mod h1:EXuID2Zs0pAQhH8yz+DNjUbjppKQzKFAn28TMYPB6IU= -gitea.dev/actionslib v1.0.0 h1:l0oFJP+P4Ds1rlCI5zk618dYkuBc2mU7Gz5wPeG0lZY= -gitea.dev/actionslib v1.0.0/go.mod h1:6O8YHkqVTKSR0LL2e5VhIDePYzGTZCbfmSVqJWEhk9g= +gitea.dev/actionslib v1.2.1 h1:GL//K/0zIZV6h1OOksv1awvFVg0rg7fug2xWcG7mkG0= +gitea.dev/actionslib v1.2.1/go.mod h1:1+gqOKGSEPn2IFHgY8S3GC5Ld+Hn8jF3OxiFHQ/fpx4= gitea.dev/sdk v1.2.0 h1:avRtJl/nKCGispgSalo9czoZM9Rto1awnE0caNAoXGo= gitea.dev/sdk v1.2.0/go.mod h1:rfh5oNdIK24cbCREwIn1tqWKQW+IICXFGWJyebuOAOE= github.com/42wim/httpsig v1.2.4 h1:mI5bH0nm4xn7K18fo1K3okNDRq8CCJ0KbBYWyA6r8lU= diff --git a/models/actions/run_job.go b/models/actions/run_job.go index a3f38f6d155..003859725fd 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -10,6 +10,7 @@ import ( "slices" "time" + act_model "gitea.dev/actionslib/pkg/model" "gitea.dev/models/db" repo_model "gitea.dev/models/repo" "gitea.dev/modules/actions/jobparser" @@ -192,32 +193,26 @@ func (job *ActionRunJob) LoadAttributes(ctx context.Context) error { // ParseJob parses the job structure from the ActionRunJob.WorkflowPayload func (job *ActionRunJob) ParseJob() (*jobparser.Job, error) { - if job.IsMatrixDeferred { - // The needs were erased before the placeholder was persisted, so jobparser.Parse no longer - // recognises the raw matrix it still carries and would re-expand it: see ParseRawSingleWorkflow. - _, workflowJob, err := jobparser.ParseRawSingleWorkflow(job.WorkflowPayload) - if err != nil { - return nil, fmt.Errorf("job %d deferred matrix placeholder: unable to parse: %w", job.ID, err) - } - return workflowJob, nil - } - - // 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) + // read as stored, jobparser.Parse would evaluate the payload again and reset its strategy.job-index + _, workflowJob, err := jobparser.ParseRawSingleWorkflow(job.WorkflowPayload) if err != nil { return nil, fmt.Errorf("job %d single workflow: unable to parse: %w", job.ID, err) - } else if len(parsedWorkflows) != 1 { - 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.ID) } return workflowJob, nil } +func (job *ActionRunJob) IsMatrixSiblingOf(other *ActionRunJob) bool { + return job.JobID == other.JobID && job.ParentJobID == other.ParentJobID && job.ID != other.ID +} + +func (job *ActionRunJob) GetFailFast() (bool, error) { + parsed, err := job.ParseJob() + if err != nil { + return false, err + } + return (&act_model.Strategy{FailFastString: parsed.Strategy.FailFastString}).GetFailFast(), nil +} + func GetRunJobByRepoAndID(ctx context.Context, repoID, jobID int64) (*ActionRunJob, error) { var job ActionRunJob has, err := db.GetEngine(ctx).Where("id=? AND repo_id=?", jobID, repoID).Get(&job) @@ -663,6 +658,9 @@ func AggregateJobStatus(jobs []*ActionRunJob) Status { // statuses like cancelled/failure when no job is waiting or running. return StatusBlocked case hasCancelled: + if hasFailure && hasFailFastMatrixFailure(jobs) { + return StatusFailure + } return StatusCancelled case hasFailure: return StatusFailure @@ -671,6 +669,21 @@ func AggregateJobStatus(jobs []*ActionRunJob) Status { } } +func hasFailFastMatrixFailure(jobs []*ActionRunJob) bool { + for _, failed := range jobs { + if failed.Status != StatusFailure || failed.ContinueOnError || !slices.ContainsFunc(jobs, func(sibling *ActionRunJob) bool { + return sibling.IsMatrixSiblingOf(failed) && sibling.Status == StatusCancelled + }) { + continue + } + failFast, err := failed.GetFailFast() + if err == nil && failFast { + return true + } + } + return false +} + // CancelPreviousJobs cancels all previous jobs of the same repository, reference, workflow, and event. // It's useful when a new run is triggered, and all previous runs needn't be continued anymore. func CancelPreviousJobs(ctx context.Context, repoID int64, ref, workflowID string, event webhook_module.HookEventType) ([]*ActionRunJob, error) { diff --git a/models/actions/run_job_status_test.go b/models/actions/run_job_status_test.go index c1a44cc6d36..74b4cd8a105 100644 --- a/models/actions/run_job_status_test.go +++ b/models/actions/run_job_status_test.go @@ -51,7 +51,6 @@ func TestAggregateJobStatus(t *testing.T) { {[]Status{StatusSuccess, StatusRunning}, StatusRunning}, {[]Status{StatusSuccess, StatusBlocked}, StatusBlocked}, - // any cancelled, then cancelled {[]Status{StatusCancelled}, StatusCancelled}, {[]Status{StatusCancelled, StatusSuccess}, StatusCancelled}, {[]Status{StatusCancelled, StatusSkipped}, StatusCancelled}, diff --git a/models/actions/run_job_test.go b/models/actions/run_job_test.go index 6570ed60534..4b428c67079 100644 --- a/models/actions/run_job_test.go +++ b/models/actions/run_job_test.go @@ -308,8 +308,6 @@ jobs: `, matrix) } - // The shape Parse happens to survive, so only the name tells the two paths apart. What Parse makes - // of every other shape is asserted in TestParseRawSingleWorkflowRoundTripsDeferredPlaceholder. t.Run("a placeholder is read back, not re-expanded", func(t *testing.T) { job := &ActionRunJob{ID: 1, JobID: "build", IsMatrixDeferred: true, WorkflowPayload: payload("version: ${{ fromJson(needs.setup.outputs.m) }}")} parsed, err := job.ParseJob() @@ -318,13 +316,11 @@ jobs: assert.Equal(t, "build", parsed.Name) }) - t.Run("an expanded job still goes through the full parse", func(t *testing.T) { - job := &ActionRunJob{ID: 1, JobID: "build", WorkflowPayload: payload("version: [1]")} + t.Run("an expanded job keeps its stored job-index", func(t *testing.T) { + job := &ActionRunJob{ID: 1, JobID: "build", WorkflowPayload: payload("version: [1]\n job-index: 1\n job-total: 2")} parsed, err := job.ParseJob() require.NoError(t, err) - require.NotNil(t, parsed) - // Parse bakes the combination into the name, ParseRawSingleWorkflow would not. - assert.Equal(t, "build (1)", parsed.Name) + assert.Equal(t, []any{"build", 1, 2}, []any{parsed.Name, parsed.Strategy.JobIndex, parsed.Strategy.JobTotal}) }) } diff --git a/models/actions/runner.go b/models/actions/runner.go index 1e5e6329ec9..02db85015b9 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "slices" "strings" "time" @@ -15,7 +16,6 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/models/shared/types" user_model "gitea.dev/models/user" - "gitea.dev/modules/container" "gitea.dev/modules/optional" "gitea.dev/modules/setting" "gitea.dev/modules/timeutil" @@ -200,8 +200,7 @@ func (r *ActionRunner) GenerateAndFillToken() { // CanMatchLabels checks whether the runner's labels can match a job's "runs-on" // See https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#jobsjob_idruns-on func (r *ActionRunner) CanMatchLabels(jobRunsOn []string) bool { - runnerLabelSet := container.SetOf(r.AgentLabels...) - return runnerLabelSet.Contains(jobRunsOn...) // match all labels + return !slices.ContainsFunc(jobRunsOn, func(label string) bool { return !util.SliceContainsString(r.AgentLabels, label, true) }) } func init() { diff --git a/models/actions/runner_test.go b/models/actions/runner_test.go index f7b4cb507bd..77937db173f 100644 --- a/models/actions/runner_test.go +++ b/models/actions/runner_test.go @@ -81,3 +81,9 @@ func TestShouldPersistLastActive(t *testing.T) { }) } } + +func TestCanMatchLabelsCaseInsensitive(t *testing.T) { + runner := &ActionRunner{AgentLabels: []string{"self-hosted", "Linux", "X64"}} + assert.True(t, runner.CanMatchLabels([]string{"SELF-HOSTED", "linux"})) + assert.False(t, runner.CanMatchLabels([]string{"linux", "arm64"})) +} diff --git a/models/perm/access/actions_repo_permission_test.go b/models/perm/access/actions_repo_permission_test.go index c833adc64d1..ff5c3f5ae26 100644 --- a/models/perm/access/actions_repo_permission_test.go +++ b/models/perm/access/actions_repo_permission_test.go @@ -93,6 +93,17 @@ func TestGetActionsUserRepoPermission(t *testing.T) { // Fork PR never gets cross-repo access to other private repos assert.False(t, perm.CanRead(unit.TypeCode)) + + publicCaller := *repo2 + publicCaller.IsPrivate = false + run := &actions_model.ActionRun{RepoID: repo2.ID, Repo: &publicCaller} + allowed, err := CanReadWorkflowCrossRepo(ctx, repo15, run) + require.NoError(t, err) + assert.False(t, allowed) + run.Repo = repo2 + allowed, err = CanReadWorkflowCrossRepo(ctx, repo15, run) + require.NoError(t, err) + assert.True(t, allowed) }) t.Run("CollaborativeOwner_ForkPR_Denied", func(t *testing.T) { diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 89ce6dc604c..7ce3f4f0168 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -686,18 +686,16 @@ func CanReadWorkflowCrossRepo(ctx context.Context, targetRepo *repo_model.Reposi if err := run.LoadRepo(ctx); err != nil { return false, err } + if targetRepo.IsPrivate && !run.Repo.IsPrivate { + return false, nil + } - // (1) Same owner: always allowed (fork-PR scrubbing handled inside). + // (1) Same owner: allowed by owner policy (fork-PR scrubbing handled inside). if checkSameOwnerCrossRepoAccess(ctx, run.Repo, targetRepo, run.IsForkPullRequest) { return true, nil } - // (2) Cross-owner: respect the target repo's collaborative-owner allowlist on its Actions unit. - // The caller (run.Repo) must itself be private. The collaborative-owner grant is owner-level, so without this - // guard a public caller owned by a grantee could pull a private reusable workflow and expose its definition and - // logs in a publicly visible run; requiring a private caller keeps private content flowing private -> private. - // This is intentionally stricter than GitHub, which gates on the target repo's access setting (introduced in #32562): - // https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#allowing-access-to-components-in-a-private-repository + // (2) Cross-owner access requires a private caller and the target's collaborative-owner grant. if run.Repo.IsPrivate && !run.IsForkPullRequest { if actionsUnit, err := targetRepo.GetUnit(ctx, unit.TypeActions); err == nil { if actionsUnit.ActionsConfig().IsCollaborativeOwner(run.Repo.OwnerID) { diff --git a/modules/actions/jobparser/aliases.go b/modules/actions/jobparser/aliases.go index 664e6943c73..0faa26900af 100644 --- a/modules/actions/jobparser/aliases.go +++ b/modules/actions/jobparser/aliases.go @@ -4,6 +4,7 @@ package jobparser import ( + "bytes" "errors" "io" @@ -12,12 +13,6 @@ import ( "go.yaml.in/yaml/v4" ) -// maxExpandedNodes bounds how many nodes alias expansion may create. go-yaml's own alias guard does -// not cover us: it only counts while decoding into values, and a workflow is kept as raw yaml.Nodes. -const maxExpandedNodes = 50000 - -var errTooManyYamlNodes = errors.New("maximum YAML nodes exceeded") - // ReadWorkflow decodes a workflow file with its aliases expanded. Callers inspect the workflow's // raw nodes by kind, and an alias is a kind none of them expect. func ReadWorkflow(content []byte) (*model.Workflow, error) { @@ -33,7 +28,10 @@ func readWorkflowDoc(doc *yaml.Node) (*model.Workflow, error) { return nil, io.EOF // what a yaml decoder reports for an empty file } w := new(model.Workflow) - return w, doc.Decode(w) + if err := doc.Decode(w); err != nil { + return w, err + } + return w, validateJobConditions(w) } // decodeResolved is yaml.Unmarshal with aliases expanded first. @@ -54,68 +52,22 @@ func decodeYamlDoc(doc *yaml.Node, out any) error { // resolveYamlAliases parses content and replaces every alias with a copy of the node its anchor names. func resolveYamlAliases(content []byte) (*yaml.Node, error) { - doc := &yaml.Node{} - if err := yaml.Unmarshal(content, doc); err != nil { + doc, err := model.ReadWorkflowNode(bytes.NewReader(content)) + if err != nil && !errors.Is(err, io.EOF) { return nil, err } - budget := maxExpandedNodes - return doc, expandAliases(doc, &budget) -} - -// expandAliases replaces node's alias descendants in place. -func expandAliases(node *yaml.Node, budget *int) error { - node.Anchor = "" // a name for a node, not part of the workflow: keep it out of the payloads - if err := rejectMergeKeys(node); err != nil { - return err - } - for i, child := range node.Content { - if child.Kind != yaml.AliasNode { - if err := expandAliases(child, budget); err != nil { - return err - } - continue - } - copied, err := copyExpanded(child.Alias, budget) - if err != nil { - return err - } - node.Content[i] = copied - } - return nil -} - -// copyExpanded deep copies a node expandAliases already expanded and validated, since an anchor is -// declared before the alias naming it. An anchor aliased from inside itself is the exception, and -// recurses here until it exhausts budget. -func copyExpanded(node *yaml.Node, budget *int) (*yaml.Node, error) { - if *budget--; *budget < 0 { - return nil, errTooManyYamlNodes - } - if node.Kind == yaml.AliasNode { - return copyExpanded(node.Alias, budget) - } - - copied := *node - copied.Content = make([]*yaml.Node, len(node.Content)) - for i, child := range node.Content { - child, err := copyExpanded(child, budget) - if err != nil { - return nil, err - } - copied.Content[i] = child - } - return &copied, nil + return doc, rejectMergeKeys(doc) } // rejectMergeKeys refuses `<<: *anchor`, same as GitHub does func rejectMergeKeys(node *yaml.Node) error { - if node.Kind != yaml.MappingNode { - return nil - } - for i := 0; i < len(node.Content)-1; i += 2 { - if node.Content[i].Tag == "!!merge" { + for i, child := range node.Content { + if node.Kind == yaml.MappingNode && i%2 == 0 && child.Tag == "!!merge" { return errors.New("merge keys (`<<`) are not supported, alias the whole value instead") } + if err := rejectMergeKeys(child); err != nil { + return err + } } return nil } diff --git a/modules/actions/jobparser/aliases_test.go b/modules/actions/jobparser/aliases_test.go index 71f385e9e52..ee4a06b53a0 100644 --- a/modules/actions/jobparser/aliases_test.go +++ b/modules/actions/jobparser/aliases_test.go @@ -10,10 +10,11 @@ import ( "github.com/stretchr/testify/require" ) -func TestParseResolvesAliases(t *testing.T) { +func TestParseResolvesAliasesAndTimestamps(t *testing.T) { got, err := Parse([]byte(`on: push env: &common_env SHARED: "1" + DATE: 2026-03-15 jobs: a: runs-on: linux @@ -31,7 +32,7 @@ jobs: _, job := workflow.Job() var env map[string]string require.NoError(t, job.Env.Decode(&env)) - assert.Equal(t, map[string]string{"SHARED": "1"}, env) + assert.Equal(t, map[string]string{"SHARED": "1", "DATE": "2026-03-15"}, env) require.Len(t, job.Steps, 1) payload, err := workflow.Marshal() @@ -49,18 +50,6 @@ func TestParseRejectsAliases(t *testing.T) { name, wantErr string content []byte }{ - { - name: "nested aliases exceed the node limit", - content: []byte(`on: push -x0: &x0 [1, 2, 3, 4, 5, 6, 7, 8, 9] -x1: &x1 [*x0, *x0, *x0, *x0, *x0, *x0, *x0, *x0, *x0] -x2: &x2 [*x1, *x1, *x1, *x1, *x1, *x1, *x1, *x1, *x1] -x3: &x3 [*x2, *x2, *x2, *x2, *x2, *x2, *x2, *x2, *x2] -x4: &x4 [*x3, *x3, *x3, *x3, *x3, *x3, *x3, *x3, *x3] -jobs: {a: {runs-on: linux, steps: [{run: echo}]}} -`), - wantErr: "maximum YAML nodes exceeded", - }, { name: "anchor aliased from inside itself", content: job(" steps: &s [{run: echo}, *s]\n"), diff --git a/modules/actions/jobparser/interpeter.go b/modules/actions/jobparser/interpeter.go index 97bb6950de7..8768d7de6ca 100644 --- a/modules/actions/jobparser/interpeter.go +++ b/modules/actions/jobparser/interpeter.go @@ -15,21 +15,13 @@ import ( // see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability func NewInterpeter( jobID string, - job *model.Job, + strategy *Strategy, matrix map[string]any, gitCtx *model.GithubContext, results map[string]*JobResult, vars map[string]string, inputs map[string]any, ) exprparser.Interpreter { - strategy := make(map[string]any) - if job.Strategy != nil { - strategy["fail-fast"] = job.Strategy.GetFailFast() - if limit, declared, err := job.Strategy.ParseMaxParallel(); declared && err == nil { - strategy["max-parallel"] = limit - } - } - run := &model.Run{ Workflow: &model.Workflow{ Jobs: map[string]*model.Job{}, @@ -46,19 +38,6 @@ func NewInterpeter( } } - jobs := run.Workflow.Jobs - jobNeeds := run.Job().Needs() - - using := map[string]exprparser.Needs{} - for _, need := range jobNeeds { - if v, ok := jobs[need]; ok { - using[need] = exprparser.Needs{ - Outputs: v.Outputs, - Result: v.Result, - } - } - } - ee := &exprparser.EvaluationEnvironment{ Github: gitCtx, Env: nil, // no need @@ -69,9 +48,9 @@ func NewInterpeter( Steps: nil, // no need Runner: nil, // no need Secrets: nil, // no need - Strategy: strategy, + Strategy: strategy.context(), Matrix: matrix, - Needs: using, + Needs: exprparser.NeedsContext(run), Inputs: inputs, Vars: vars, } diff --git a/modules/actions/jobparser/jobparser.go b/modules/actions/jobparser/jobparser.go index 55c5a8be0de..912b1278548 100644 --- a/modules/actions/jobparser/jobparser.go +++ b/modules/actions/jobparser/jobparser.go @@ -17,28 +17,33 @@ import ( "go.yaml.in/yaml/v4" ) -// HasDeferredMatrix reports whether the job's matrix can only be expanded once its needs finish: -// it reads the needs context and the job has needs to resolve that context against. -// Parse emits such a job as a single placeholder rather than one job per combination, so every -// caller that persists a job must agree with Parse on this condition. +// HasDeferredMatrix reports whether the job's strategy, name, runs-on or continue-on-error need outputs before they can be resolved. func HasDeferredMatrix(job *Job) bool { - return len(job.Needs()) > 0 && rawMatrixReadsNeeds(&job.Strategy.RawMatrix) + return len(job.Needs()) > 0 && (nodeMatches(&job.Strategy.RawMatrix, expressionReadsNeeds) || expressionReadsNeeds(job.Strategy.RawExpression.Value) || + expressionReadsNeeds(job.Strategy.MaxParallelString) || expressionReadsNeeds(job.Strategy.FailFastString) || + expressionReadsNeeds(job.Name) || nodeMatches(&job.RawRunsOn, expressionReadsNeeds) || nodeMatches(&job.RawContinueOnError, expressionReadsNeeds)) } -func rawMatrixReadsNeeds(node *yaml.Node) bool { +func nodeMatches(node *yaml.Node, match func(string) bool) bool { if node.Kind == yaml.ScalarNode { - return expressionReadsNeeds(node.Value) + return match(node.Value) } - return slices.ContainsFunc(node.Content, rawMatrixReadsNeeds) + return slices.ContainsFunc(node.Content, func(child *yaml.Node) bool { return nodeMatches(child, match) }) } -// ParseRawSingleWorkflow decodes a SingleWorkflow payload into the workflow and its single job -// without expanding `strategy.matrix`. -// -// A deferred-matrix placeholder's payload still carries the raw, unevaluated matrix, which Parse -// would try to expand: depending on the matrix's shape that yields several workflows (a static -// vector crossed with the unevaluated expression) or an error (an `include`/`exclude` that is still -// a scalar), neither of which describes the one job the payload stands for. +func hasExpression(value string) bool { + return strings.Contains(value, "${{") +} + +// IfExpression wraps an `if:` that omits the `${{ }}`, which github.com evaluates as one expression anyway. +func IfExpression(value string) string { + if hasExpression(value) { + return value + } + return "${{ " + value + " }}" +} + +// ParseRawSingleWorkflow decodes a stored SingleWorkflow payload into the workflow and its single job as stored, without expanding or evaluating it again. func ParseRawSingleWorkflow(payload []byte) (*SingleWorkflow, *Job, error) { swf := &SingleWorkflow{} if err := decodeResolved(payload, swf); err != nil { @@ -62,29 +67,6 @@ func expressionReadsNeeds(value string) bool { return expreval.ReadsContext(value, "needs") } -// ExpressionReadsMatrix reports whether a job's `if:` reads the matrix context. -// A deferred-matrix placeholder has no combination yet, so such an expression cannot be decided. -func ExpressionReadsMatrix(ifValue string) bool { - return expreval.ReadsContext(asIfExpression(ifValue), "matrix") -} - -// ExpressionIgnoresNeedResults reports whether a job's `if:` calls always(), failure() or cancelled(), -// the status functions that run a job whatever its needs did rather than under the implicit success(). -// Keep in sync with act's exprparser, which owns the same list for the evaluation itself. -func ExpressionIgnoresNeedResults(ifValue string) bool { - return expreval.CallsFunction(asIfExpression(ifValue), "always", "failure", "cancelled") -} - -// asIfExpression wraps an `if:` that omits the `${{ }}`, which GitHub evaluates as one expression anyway. -// `if:` is the only field with that exception: every other value is interpolated, so a bare matrix or -// `runs-on` is a literal there and must not be parsed as an expression. -func asIfExpression(ifValue string) string { - if ifValue == "" || strings.Contains(ifValue, "${{") { - return ifValue - } - return "${{ " + ifValue + " }}" -} - func Parse(content []byte, options ...ParseOption) ([]*SingleWorkflow, error) { // The workflow is split into one document per job below, which would strand an alias whose // anchor lands in another one. @@ -132,28 +114,25 @@ func Parse(content []byte, options ...ParseOption) ([]*SingleWorkflow, error) { for i, id := range ids { job := jobs[i] - originJob := origin.GetJob(id) - - if originJob == nil { - return nil, fmt.Errorf("job %s not found in origin workflow", id) - } - var combos []*Job - if HasDeferredMatrix(job) { - // The matrix reads values that do not exist yet (a needs output), so emit a single - // placeholder keeping it raw. Re-parsing that placeholder's payload yields it again, - // and the server expands it once the needs finish. + if HasDeferredMatrix(job) || pc.gitContext == nil && (job.Strategy.RawExpression.Kind != 0 || nodeMatches(&job.Strategy.RawMatrix, hasExpression)) { placeholder := job.Clone() if placeholder.Name == "" { placeholder.Name = id } combos = []*Job{placeholder} } else { - matricxes, err := getMatrixes(originJob) + if pc.gitContext != nil { // callers without one, like commit status, only read the literal workflow + if err := job.Strategy.resolve(evaluator); err != nil { + return nil, fmt.Errorf("job %q: %w", id, err) + } + } + // Keep accepting empty exclude mappings for workflow compatibility, although GitHub rejects them. + matrixes, err := (&model.Job{Strategy: job.Strategy.actStrategy()}).GetMatrixes() if err != nil { return nil, fmt.Errorf("getMatrixes: %w", err) } - if combos, err = buildMatrixCombos(id, job, matricxes, originJob, pc.gitContext, results, pc.vars, pc.inputs); err != nil { + if combos, err = buildMatrixCombos(id, job, matrixes, pc.gitContext, results, pc.vars, pc.inputs); err != nil { return nil, err } } @@ -186,99 +165,120 @@ func (w *SingleWorkflow) CloneHeader() *SingleWorkflow { // maxCombinations caps how many combinations may be built: the values come from a needs output at // runtime, so the cap has to be enforced before one Job is materialized per combination. func ExpandMatrixWithNeeds(jobID string, job *Job, gitCtx *model.GithubContext, results map[string]*JobResult, vars map[string]string, inputs map[string]any, maxCombinations int) ([]*Job, error) { - actJob := &model.Job{Strategy: &model.Strategy{ - FailFastString: job.Strategy.FailFastString, - MaxParallelString: job.Strategy.MaxParallelString, - RawMatrix: job.Strategy.RawMatrix, - }} - - // Resolve fromJson(needs.*.outputs.*) and friends into concrete matrix values. - if err := expreval.New(NewInterpeter(jobID, actJob, nil, gitCtx, results, vars, inputs).Evaluate). - EvaluateYamlNode(&actJob.Strategy.RawMatrix); err != nil { - return nil, fmt.Errorf("evaluate matrix: %w", err) + job = job.Clone() + if err := job.Strategy.resolve(expreval.New(NewInterpeter(jobID, nil, nil, gitCtx, results, vars, inputs).Evaluate)); err != nil { + return nil, err } - matrixes, err := getMatrixes(actJob) + matrixes, err := (&model.Job{Strategy: job.Strategy.actStrategy()}).GetMatrixes() if err != nil { return nil, fmt.Errorf("getMatrixes: %w", err) } - // act collapses a matrix that yields no combination (an empty vector or include, everything - // excluded, a whole-matrix expression that is not a mapping) into one empty combination, which - // would run the job once unparameterized. GitHub rejects such a matrix, so reject it too. - if len(matrixes) == 1 && len(matrixes[0]) == 0 { - return nil, errors.New("matrix must define at least one vector") - } if len(matrixes) > maxCombinations { return nil, fmt.Errorf("matrix expands to %d combinations, exceeding the limit of %d", len(matrixes), maxCombinations) } - return buildMatrixCombos(jobID, job, matrixes, actJob, gitCtx, results, vars, inputs) + return buildMatrixCombos(jobID, job, matrixes, gitCtx, results, vars, inputs) } -// matrixesOf is this package's only entry to act's GetMatrixes, so that every caller is covered by -// the filter check below. A deferred placeholder is the first thing carrying a raw matrix this far, -// and the emitter reads its `if:` before expanding it. -func matrixesOf(job *model.Job) ([]map[string]any, error) { - if err := validateMatrixFilters(job); err != nil { - return nil, err - } - matrixes, err := job.GetMatrixes() - if err != nil { - return nil, fmt.Errorf("GetMatrixes: %w", err) - } - return matrixes, nil -} - -// validateMatrixFilters rejects an `include`/`exclude` that is not a list of mappings, so that the -// usual way to get there, an unevaluated ${{ }} expression that is still a scalar, is named as such -// instead of surfacing from the middle of the expansion. -func validateMatrixFilters(job *model.Job) error { - if job.Strategy == nil || job.Strategy.RawMatrix.Kind != yaml.MappingNode { - return nil - } - content := job.Strategy.RawMatrix.Content - for i := 0; i+1 < len(content); i += 2 { - name, value := content[i].Value, content[i+1] - if name != "include" && name != "exclude" { - continue +// resolve evaluates strategy values once and escapes expression-like results for the runner. +func (s *Strategy) resolve(evaluator expreval.Evaluator) error { + if s.RawExpression.Kind != 0 { + if err := model.DecodeEvaluated("strategy", s.RawExpression, evaluator.EvaluateYamlNode, s); err != nil { + return err } - entries := []*yaml.Node{value} - if value.Kind == yaml.SequenceNode { - entries = value.Content + if s.RawExpression.Kind != 0 { + return errors.New("strategy is not a map of strategy keys to values") } - for _, entry := range entries { - if entry.Kind != yaml.MappingNode { - return fmt.Errorf("matrix %s must be a list of mappings", name) + } else { + for _, value := range []*string{&s.FailFastString, &s.MaxParallelString} { + evaluated, err := evaluator.Interpolate(*value) + if err != nil { + return fmt.Errorf("evaluate strategy: %w", err) } + *value = evaluated + } + if err := evaluator.EvaluateYamlNode(&s.RawMatrix); err != nil { + return fmt.Errorf("evaluate matrix: %w", err) } } + s.FailFastString, s.MaxParallelString = escapeExpressions(s.FailFastString), escapeExpressions(s.MaxParallelString) return nil } +const escapedExpression = "${{ '$' }}{{" + +// escapeExpressions returns a template interpolating to value, since GitHub never evaluates the result of an evaluation again. +func escapeExpressions(value string) string { + return strings.ReplaceAll(value, "${{", escapedExpression) +} + +func unescapeExpressions(value string) string { + return strings.ReplaceAll(value, escapedExpression, "${{") +} + +func replaceScalars(node *yaml.Node, replace func(string) string) { + node.Value = replace(node.Value) + for _, child := range node.Content { + replaceScalars(child, replace) + } +} + // buildMatrixCombos builds one Job per matrix combination from src, baking the combination into the // strategy and interpolating the name, runs-on and continue-on-error with it. -func buildMatrixCombos(jobID string, src *Job, matrixes []map[string]any, actJob *model.Job, gitCtx *model.GithubContext, results map[string]*JobResult, vars map[string]string, inputs map[string]any) ([]*Job, error) { - srcRunsOn := src.RunsOn() +func buildMatrixCombos(jobID string, src *Job, matrixes []map[string]any, gitCtx *model.GithubContext, results map[string]*JobResult, vars map[string]string, inputs map[string]any) ([]*Job, error) { + srcRunsOn := model.RunsOnFromNode(src.RawRunsOn) + order, names := make([]int, len(matrixes)), make([]string, len(matrixes)) + for index, matrix := range matrixes { + order[index], names[index] = index, matrixName(matrix) + } + slices.SortStableFunc(order, func(a, b int) int { return strings.Compare(names[a], names[b]) }) combos := make([]*Job, 0, len(matrixes)) var err error - for _, matrix := range matrixes { + for _, index := range order { + matrix := matrixes[index] combo := src.Clone() if combo.Name == "" { combo.Name = jobID } combo.Strategy.RawMatrix = encodeMatrix(matrix) - evaluator := expreval.New(NewInterpeter(jobID, actJob, matrix, gitCtx, results, vars, inputs).Evaluate) - if combo.Name, err = nameWithMatrix(combo.Name, matrix, evaluator); err != nil { + replaceScalars(&combo.Strategy.RawMatrix, escapeExpressions) + if src.Strategy.RawMatrix.Kind != 0 { + combo.Strategy.JobIndex, combo.Strategy.JobTotal = index, len(matrixes) + } + evaluator := expreval.New(NewInterpeter(jobID, &combo.Strategy, matrix, gitCtx, results, vars, inputs).Evaluate) + if len(matrix) == 0 && gitCtx != nil { + combo.Name, err = evaluator.Interpolate(combo.Name) + combo.Name = escapeExpressions(combo.Name) + } else { + combo.Name, err = nameWithMatrix(combo.Name, matrix, evaluator) + } + if err != nil { return nil, fmt.Errorf("interpolate name for job %q: %w", jobID, err) } - runsOn := slices.Clone(srcRunsOn) - for i := range runsOn { - if runsOn[i], err = evaluator.Interpolate(runsOn[i]); err != nil { + if gitCtx != nil { // callers without one don't read runs-on + rawRunsOn := model.CloneYamlNode(src.RawRunsOn) + if err := evaluator.EvaluateYamlNode(&rawRunsOn); err != nil { return nil, fmt.Errorf("interpolate runs-on for job %q: %w", jobID, err) } + runsOn := model.RunsOnFromNode(rawRunsOn) + if len(runsOn) == 0 && len(srcRunsOn) > 0 { // match no runner rather than every runner + runsOn = []string{""} + } + for i := range runsOn { + runsOn[i] = escapeExpressions(runsOn[i]) + } + combo.RawRunsOn = model.RunsOnNode(runsOn, "") } - combo.RawRunsOn = encodeRunsOn(runsOn) if err := evaluator.EvaluateYamlNode(&combo.RawContinueOnError); err != nil { return nil, fmt.Errorf("evaluate continue-on-error for job %q: %w", jobID, err) } + if combo.RawContinueOnError.Kind != 0 { + var continueOnError bool + if err := combo.RawContinueOnError.Decode(&continueOnError); err == nil { + _ = combo.RawContinueOnError.Encode(continueOnError) + } else { + replaceScalars(&combo.RawContinueOnError, escapeExpressions) + } + } combos = append(combos, combo) } return combos, nil @@ -311,17 +311,6 @@ type parseContext struct { type ParseOption func(c *parseContext) -func getMatrixes(job *model.Job) ([]map[string]any, error) { - ret, err := matrixesOf(job) - if err != nil { - return nil, err - } - sort.Slice(ret, func(i, j int) bool { - return matrixName(ret[i]) < matrixName(ret[j]) - }) - return ret, nil -} - func encodeMatrix(matrix map[string]any) yaml.Node { if len(matrix) == 0 { return yaml.Node{} @@ -335,26 +324,17 @@ func encodeMatrix(matrix map[string]any) yaml.Node { return node } -func encodeRunsOn(runsOn []string) yaml.Node { - node := yaml.Node{} - if len(runsOn) == 1 { - _ = node.Encode(runsOn[0]) - } else { - _ = node.Encode(runsOn) - } - return node -} - func nameWithMatrix(name string, m map[string]any, evaluator expreval.Evaluator) (string, error) { if len(m) == 0 { return name, nil } if !strings.Contains(name, "${{") || !strings.Contains(name, "}}") { - return name + " " + matrixName(m), nil + return escapeExpressions(name + " " + matrixName(m)), nil } - return evaluator.Interpolate(name) + name, err := evaluator.Interpolate(name) + return escapeExpressions(name), err } func matrixName(m map[string]any) string { diff --git a/modules/actions/jobparser/jobparser_test.go b/modules/actions/jobparser/jobparser_test.go index 15fa3cf354e..3da5d2f45a0 100644 --- a/modules/actions/jobparser/jobparser_test.go +++ b/modules/actions/jobparser/jobparser_test.go @@ -8,6 +8,8 @@ import ( "strings" "testing" + "gitea.dev/actionslib/pkg/expreval" + "gitea.dev/actionslib/pkg/exprparser" "gitea.dev/actionslib/pkg/model" "github.com/stretchr/testify/assert" @@ -78,7 +80,7 @@ func TestParse(t *testing.T) { t.Run(tt.name, func(t *testing.T) { content := ReadTestdata(t, tt.name+".in.yaml") want := ReadTestdata(t, tt.name+".out.yaml") - got, err := Parse(content, tt.options...) + got, err := Parse(content, append([]ParseOption{WithGitContext(&model.GithubContext{})}, tt.options...)...) if tt.wantErr { require.Error(t, err) } @@ -112,8 +114,6 @@ func TestParse(t *testing.T) { } func TestParseDefersDynamicMatrix(t *testing.T) { - // A matrix referencing needs outputs yields one placeholder keeping the raw expression, rather - // than one job per resolvable static value. Any other matrix expands at plan time as usual. const workflow = ` on: push jobs: @@ -121,32 +121,28 @@ jobs: steps: [{run: echo}] build: %s - strategy: - matrix: - os: [a, b] - version: %s + %s steps: [{run: echo}] ` for _, tt := range []struct { name string needs string - version string + strategy string deferred bool want int }{ - {"needs outputs", "needs: setup", "${{ fromJson(needs.setup.outputs.v) }}", true, 1}, - {"static", "needs: setup", "[1, 2]", false, 4}, - // Without needs there is nothing to resolve the expression from later, so deferring would - // strand the job as a single combination that never expands. - {"expression without needs", "", `["${{ github.sha }}"]`, false, 2}, - // A context that is already available while planning must keep expanding there, otherwise - // such a workflow would silently lose the per-combination commit statuses it used to create. - {"expression over another context", "needs: setup", `["${{ github.sha }}"]`, false, 2}, - // The needs context is looked up in the parsed expression, not in the raw text. - {"needs inside a string literal", "needs: setup", `["${{ format('needs.setup.outputs.v {0}', github.sha) }}"]`, false, 2}, + {"needs outputs", "needs: setup", "strategy:\n matrix:\n os: [a, b]\n version: ${{ fromJson(needs.setup.outputs.v) }}", true, 1}, + {"static", "needs: setup", "strategy:\n matrix: {os: [a, b], version: [1, 2]}", false, 4}, + {"max-parallel over needs outputs", "needs: setup", "strategy:\n max-parallel: ${{ needs.setup.outputs.limit }}\n matrix: {os: [a, b], version: [1, 2]}", true, 1}, + {"fail-fast without matrix", "needs: setup", "strategy:\n fail-fast: ${{ needs.setup.outputs.fast }}", true, 1}, + {"whole strategy without matrix", "needs: setup", "strategy: ${{ fromJSON(needs.setup.outputs.strategy) }}", true, 1}, + {"expression without needs", "", "strategy:\n matrix:\n os: [a, b]\n version: [\"${{ github.sha }}\"]", false, 2}, + {"expression over another context", "needs: setup", "strategy:\n matrix:\n os: [a, b]\n version: [\"${{ github.sha }}\"]", false, 2}, + {"needs inside a string literal", "needs: setup", "strategy:\n matrix:\n os: [a, b]\n version: [\"${{ format('needs.setup.outputs.v {0}', github.sha) }}\"]", false, 2}, + {"vars", "needs: setup", "strategy:\n matrix:\n os: [a, b]\n version: ${{ fromJSON(vars.VERSIONS) }}", false, 4}, } { t.Run(tt.name, func(t *testing.T) { - result, err := Parse(fmt.Appendf(nil, workflow, tt.needs, tt.version)) + result, err := Parse(fmt.Appendf(nil, workflow, tt.needs, tt.strategy), WithGitContext(&model.GithubContext{}), WithVars(map[string]string{"VERSIONS": "[1, 2]"})) require.NoError(t, err) var builds []*Job @@ -161,6 +157,93 @@ jobs: } } +func TestParseWholeValueExpressions(t *testing.T) { + const workflow = ` +on: push +env: ${{ fromJSON(vars.ENV) }} +jobs: + setup: + runs-on: ubuntu-latest + steps: [{run: echo}] + build: + %s + runs-on: ${{ matrix.os }} + strategy: ${{ fromJSON(%s) }} + services: ${{ fromJSON(vars.SERVICES) }} + defaults: + run: ${{ fromJSON(vars.DEFAULTS) }} + steps: + - uses: actions/checkout@v4 + with: ${{ fromJSON(vars.WITH) }} +` + const strategy = `{"max-parallel": 1, "matrix": {"os": ["a", "b-${{ secrets.X }}"]}}` + runnerView := func(t *testing.T, job *model.Job) []any { + t.Helper() + runner := expreval.New(exprparser.NewInterpeter(&exprparser.EvaluationEnvironment{Secrets: map[string]string{"X": "leaked"}}, exprparser.Config{}).Evaluate) + require.NoError(t, runner.EvaluateYamlNode(&job.Strategy.RawMatrix)) + require.NoError(t, runner.EvaluateYamlNode(&job.RawRunsOn)) + matrixes, err := job.GetMatrixes() + require.NoError(t, err) + name, err := runner.Interpolate(job.Name) + require.NoError(t, err) + return []any{matrixes[0]["os"], name, job.RunsOn()} + } + payloads := func(t *testing.T, needs, source string, options ...ParseOption) []string { + t.Helper() + workflows, err := Parse(fmt.Appendf(nil, workflow, needs, source), options...) + require.NoError(t, err) + var payloads []string + for _, w := range workflows { + if id, _ := w.Job(); id == "build" { + payload, err := w.Marshal() + require.NoError(t, err) + payloads = append(payloads, string(payload)) + } + } + return payloads + } + planning := []ParseOption{WithGitContext(&model.GithubContext{}), WithVars(map[string]string{"STRATEGY": strategy})} + + planned := payloads(t, "", "vars.STRATEGY", planning...) + require.Len(t, planned, 2) + for _, raw := range []string{`max-parallel: "1"`, "env: ${{ fromJSON(vars.ENV) }}", "services: ${{ fromJSON(vars.SERVICES) }}", "run: ${{ fromJSON(vars.DEFAULTS) }}", "with: ${{ fromJSON(vars.WITH) }}"} { + assert.Contains(t, planned[0], raw) + } + for index, payload := range planned { + os := []string{"a", "b-${{ secrets.X }}"}[index] + read, err := model.ReadWorkflow(strings.NewReader(payload)) + require.NoError(t, err) + assert.Equal(t, []any{os, "build (" + os + ")", []string{os}}, runnerView(t, read.Jobs["build"])) + _, job, err := ParseRawSingleWorkflow([]byte(payload)) + require.NoError(t, err) + group, _, err := EvaluateConcurrency(&model.RawConcurrency{Group: "${{ matrix.os }}"}, "build", job, map[string]any{}, nil, nil, nil) + require.NoError(t, err) + assert.Equal(t, []any{"build (" + os + ")", []string{os}, os}, []any{job.DisplayName(), job.RunsOn(), group}) + _, err = Parse([]byte(payload)) + require.NoError(t, err) + } + + assert.Len(t, payloads(t, "", "vars.STRATEGY"), 1) + + deferred := payloads(t, "needs: setup", "needs.setup.outputs.strategy", planning...) + require.Len(t, deferred, 1) + _, placeholder, err := ParseRawSingleWorkflow([]byte(deferred[0])) + require.NoError(t, err) + expanded, err := ExpandMatrixWithNeeds("build", placeholder, &model.GithubContext{}, map[string]*JobResult{ + "build": {Needs: []string{"setup"}}, + "setup": {Result: "success", Outputs: map[string]string{"strategy": strategy}}, + }, nil, nil, 256) + require.NoError(t, err) + require.Len(t, expanded, 2) + assert.Equal(t, "1", expanded[0].Strategy.MaxParallelString) + assert.Equal(t, []any{"build (b-${{ secrets.X }})", []string{"b-${{ secrets.X }}"}}, []any{expanded[1].DisplayName(), expanded[1].RunsOn()}) + assert.Equal(t, []any{"b-${{ secrets.X }}", "build (b-${{ secrets.X }})", []string{"b-${{ secrets.X }}"}}, + runnerView(t, &model.Job{Name: expanded[1].Name, RawRunsOn: expanded[1].RawRunsOn, Strategy: expanded[1].Strategy.actStrategy()})) + + _, err = Parse(fmt.Appendf(nil, workflow, "", "vars.STRATEGY"), planning[0], WithVars(map[string]string{"STRATEGY": `"a"`})) + require.ErrorContains(t, err, "is not a map of strategy keys to values") +} + func TestParseInterpolatesRunName(t *testing.T) { workflow := func(runName string) []byte { return []byte("name: t\nrun-name: \"" + runName + "\"\non: push\njobs:\n build:\n runs-on: ubuntu-latest\n steps: [{run: echo}]\n") @@ -183,6 +266,9 @@ func TestParseInterpolatesRunName(t *testing.T) { require.NoError(t, err) require.Len(t, result, 1) assert.Equal(t, tt.want, result[0].RunName) + payload, err := result[0].Marshal() + require.NoError(t, err) + assert.NotContains(t, string(payload), "run-name") }) } @@ -194,7 +280,7 @@ func TestParseInterpolatesRunName(t *testing.T) { // a malformed part must not restructure the surrounding expression for _, runName := range []string{"${{ 1) && (2 }}", "run ${{ 1) && (2 }} now", "${{ 'a' }} ${{ b", "${{ 'a }}"} { _, err := Parse(workflow(runName), WithGitContext(&model.GithubContext{EventName: "push"})) - assert.ErrorContains(t, err, "interpolate run-name") + assert.Error(t, err, runName) } // callers such as commit status parse without a git context, leaving `github` a nil pointer @@ -204,6 +290,68 @@ func TestParseInterpolatesRunName(t *testing.T) { assert.Empty(t, result[0].RunName) } +func TestParseRunsOnFromJSONArray(t *testing.T) { + content := []byte("on: push\njobs:\n build:\n runs-on: ${{ fromJSON(vars.RUNNER) }}\n steps: [{run: echo}]\n") + _, err := Parse(content) + require.NoError(t, err) + for runner, want := range map[string][]string{`["self-hosted", "linux"]`: {"self-hosted", "linux"}, "[]": {""}} { + result, err := Parse(content, WithGitContext(&model.GithubContext{}), WithVars(map[string]string{"RUNNER": runner})) + require.NoError(t, err) + require.Len(t, result, 1) + _, job := result[0].Job() + assert.Equal(t, want, job.RunsOn(), runner) + } +} + +func TestJobFieldsWithoutMatrix(t *testing.T) { + const workflow = `on: push +jobs: + seed: + steps: [{run: echo}] + build: + name: build-${{ github.ref_name }} + continue-on-error: ${{ fromJSON(vars.CONTINUE) }} + steps: [{run: echo}] + target: + needs: seed + name: target-${{ needs.seed.outputs.runner }} + runs-on: ${{ needs.seed.outputs.runner }} + continue-on-error: ${{ needs.seed.outputs.tolerate == 'true' }} + strategy: ${{ fromJSON(needs.seed.outputs.strategy) }} + steps: [{run: echo}] +` + parse := func(t *testing.T, continueOnError string) map[string]*SingleWorkflow { + t.Helper() + parsed, err := Parse([]byte(workflow), WithGitContext(&model.GithubContext{RefName: "main"}), WithVars(map[string]string{"CONTINUE": continueOnError})) + require.NoError(t, err) + workflows := map[string]*SingleWorkflow{} + for _, parsedWorkflow := range parsed { + id, _ := parsedWorkflow.Job() + workflows[id] = parsedWorkflow + } + return workflows + } + workflows := parse(t, "true") + _, build := workflows["build"].Job() + assert.Equal(t, []any{"build-main", true}, []any{build.Name, build.GetContinueOnError()}) + payload, err := parse(t, `"${{ true }}"`)["build"].Marshal() + require.NoError(t, err) + assert.Contains(t, string(payload), "${{ '$' }}{{ true }}") + + _, placeholder := workflows["target"].Job() + require.True(t, HasDeferredMatrix(placeholder)) + expanded, err := ExpandMatrixWithNeeds("target", placeholder, &model.GithubContext{}, map[string]*JobResult{ + "target": {Needs: []string{"seed"}}, + "seed": {Result: "success", Outputs: map[string]string{"runner": "ubuntu-latest", "tolerate": "true", "strategy": `{"fail-fast":false,"max-parallel":3}`}}, + }, nil, nil, 256) + require.NoError(t, err) + require.Len(t, expanded, 1) + group, _, err := EvaluateConcurrency(&model.RawConcurrency{Group: "${{ strategy.job-index }}:${{ strategy.job-total }}:${{ strategy.fail-fast }}"}, "target", expanded[0], map[string]any{}, nil, nil, nil) + require.NoError(t, err) + assert.Equal(t, []any{"target-ubuntu-latest", []string{"ubuntu-latest"}, true, "3", "0:1:false"}, + []any{expanded[0].DisplayName(), expanded[0].RunsOn(), expanded[0].GetContinueOnError(), expanded[0].Strategy.MaxParallelString, group}) +} + func TestExpandMatrixWithNeeds(t *testing.T) { // matrixYAML is the YAML value of the `matrix:` key, so a case can replace the whole node. expandMax := func(t *testing.T, matrixYAML string, maxCombinations int) ([]*Job, error) { @@ -221,6 +369,7 @@ func TestExpandMatrixWithNeeds(t *testing.T) { "os": `["linux", "darwin"]`, "include": `[{"os":"linux","fast":true},{"os":"windows","fast":false}]`, "empty": "[]", + "limit": "3", }}, }, nil, nil, maxCombinations) } @@ -230,17 +379,18 @@ func TestExpandMatrixWithNeeds(t *testing.T) { } t.Run("expands the product and interpolates runs-on", func(t *testing.T) { - got, err := expand(t, "\n os: ${{ fromJson(needs.setup.outputs.os) }}\n version: ${{ fromJson(needs.setup.outputs.versions) }}\n") + got, err := expand(t, "\n os: ${{ fromJson(needs.setup.outputs.os) }}\n version: ${{ fromJson(needs.setup.outputs.versions) }}\nmax-parallel: ${{ needs.setup.outputs.limit }}\n") require.NoError(t, err) - names := make([]string, 0, len(got)) + indexes := make(map[string]int, len(got)) for _, combo := range got { - names = append(names, combo.Name) + indexes[combo.Name] = combo.Strategy.JobIndex assert.Contains(t, []string{"linux", "darwin"}, combo.RunsOn()[0]) + assert.Equal(t, "3", combo.Strategy.MaxParallelString) + assert.Equal(t, 4, combo.Strategy.JobTotal) } - // Dimensions are appended in key order, as GitHub names multi-dimension combinations. - assert.ElementsMatch(t, []string{ - "build (linux, 1.20)", "build (linux, 1.21)", "build (darwin, 1.20)", "build (darwin, 1.21)", - }, names) + assert.Equal(t, map[string]int{ + "build (linux, 1.20)": 0, "build (linux, 1.21)": 1, "build (darwin, 1.20)": 2, "build (darwin, 1.21)": 3, + }, indexes) }) t.Run("static and dynamic dimensions expand together, once", func(t *testing.T) { @@ -255,7 +405,6 @@ func TestExpandMatrixWithNeeds(t *testing.T) { assert.Len(t, got, 2) }) - // GitHub rejects a matrix that yields no combinations instead of running the job unparameterized. for _, tt := range []struct{ name, matrix, errHas string }{ {"empty vector", "\n version: ${{ fromJson(needs.setup.outputs.empty) }}\n", `Matrix vector "version" does not contain any values`}, {"empty include", "\n include: ${{ fromJson(needs.setup.outputs.empty) }}\n", "Matrix must define at least one vector"}, @@ -267,11 +416,24 @@ func TestExpandMatrixWithNeeds(t *testing.T) { }) } + t.Run("fully excluded matrix runs once without matrix values", func(t *testing.T) { + got, err := expand(t, "\n os: [linux]\n exclude:\n - os: linux\n") + require.NoError(t, err) + require.Len(t, got, 1) + assert.Equal(t, "build", got[0].Name) + assert.Equal(t, 1, got[0].Strategy.JobTotal) + }) + t.Run("unresolved need errors", func(t *testing.T) { _, err := expand(t, "\n v: ${{ fromJson(needs.missing.outputs.v) }}\n") require.ErrorContains(t, err, "evaluate matrix") }) + t.Run("invalid strategy field errors", func(t *testing.T) { + _, err := expand(t, "\n os: [linux]\nmax-parallel: ${{ fromJSON('bad') }}\n") + require.ErrorContains(t, err, "evaluate strategy") + }) + // The combination count comes from a runtime output, so it must be rejected before one Job per // combination is built rather than after. t.Run("too many combinations errors", func(t *testing.T) { @@ -280,40 +442,34 @@ func TestExpandMatrixWithNeeds(t *testing.T) { }) } -// evaluateJobIf builds a one-job workflow around the given `matrix:` value and `if:`, and decides it. -func evaluateJobIf(t *testing.T, matrixYAML, ifExpr string, deferred bool) (bool, error) { - t.Helper() - var strategy Strategy - require.NoError(t, yaml.Unmarshal(fmt.Appendf(nil, "matrix:\n %s\n", matrixYAML), &strategy)) - job := &Job{Name: "build", Strategy: strategy} - require.NoError(t, job.If.Encode(ifExpr)) - return EvaluateJobIfExpression("build", job, map[string]any{}, map[string]*JobResult{"build": {}}, nil, nil, deferred) +func TestReadWorkflowJobConditionContexts(t *testing.T) { + for condition, unavailable := range map[string]string{ + "matrix.os == 'a'": "matrix", + "'${{ strategy.job-index == 0 }}'": "strategy", + "'${{ github.ref }} ${{ secrets.X }}'": "secrets", + "github.event.matrix && gitea.ref && needs.a.result && vars.X && inputs.y && always() && fromJSON('true')": "", + } { + _, err := ReadWorkflow([]byte("jobs: {build: {if: " + condition + "}}")) + if unavailable == "" { + assert.NoError(t, err, condition) + } else { + assert.ErrorContains(t, err, "Unrecognized named-value: '"+unavailable+"'", condition) + } + } } func TestRejectsUnevaluatedMatrixFilters(t *testing.T) { - // An unevaluated expression is still a scalar, which is not a filter act can apply. - // Every entry point into act's matrix expansion must reject it. The - // expression here reads `vars`, which is available while planning, so the job is not deferred and - // nothing will ever resolve the filter: the error is the right answer at both entry points. - // A deferred placeholder is the other case, covered by TestEvaluateJobIfExpressionLeavesRawMatrixUnavailable. for _, filter := range []string{"include", "exclude"} { t.Run(filter, func(t *testing.T) { _, err := Parse(fmt.Appendf(nil, - "name: t\non: push\njobs:\n build:\n runs-on: ubuntu-latest\n strategy:\n matrix:\n os: [a]\n %s: ${{ fromJson(vars.MATRIX) }}\n steps: [{run: echo}]\n", filter)) - require.ErrorContains(t, err, "must be a list of mappings") - - _, err = evaluateJobIf(t, fmt.Sprintf("os: [a]\n %s: ${{ fromJson(vars.MATRIX) }}", filter), "${{ true }}", false) - require.ErrorContains(t, err, "must be a list of mappings") + "name: t\non: push\njobs:\n build:\n runs-on: ubuntu-latest\n strategy:\n matrix:\n os: [a]\n %s: ${{ fromJson(vars.MATRIX) }}\n steps: [{run: echo}]\n", filter), + WithGitContext(&model.GithubContext{}), WithVars(map[string]string{"MATRIX": `"a"`})) + require.ErrorContains(t, err, "is not a list of maps") }) } } func TestParseRawSingleWorkflowRoundTripsDeferredPlaceholder(t *testing.T) { - // The server persists a placeholder the way insertRunJob does: erase the needs, then marshal. - // Reading it back must yield that one job again. Parse cannot do it: it only keeps a matrix raw - // while the job still declares needs, so on the stored payload it falls through to expanding the - // raw matrix instead - which either fails or splits the placeholder into several workflows, and - // in both cases leaves the job unexpandable for good. const workflow = ` on: push jobs: @@ -327,21 +483,13 @@ jobs: %s steps: [{run: echo}] ` - for _, tt := range []struct { - name string - matrix string - parseCount int // what Parse makes of the stored payload - parseErrHas string // ... or the error it fails with - }{ - // The canonical GitHub dynamic-matrix idiom. validateMatrixFilters rejects the still-scalar expression outright. - {name: "include expression", matrix: "include: ${{ fromJson(needs.setup.outputs.m) }}", parseErrHas: "must be a list of mappings"}, - // An unevaluated expression is a scalar where a vector is required, rather than one - // combination holding the literal `${{ }}` text. - {name: "static vector and expression", matrix: "os: [a, b]\n version: ${{ fromJson(needs.setup.outputs.m) }}", parseErrHas: `Matrix vector "version" is not a list of values`}, - {name: "single expression vector", matrix: "version: ${{ fromJson(needs.setup.outputs.m) }}", parseErrHas: `Matrix vector "version" is not a list of values`}, + for name, matrix := range map[string]string{ + "include expression": "include: ${{ fromJson(needs.setup.outputs.m) }}", + "static vector and expression": "os: [a, b]\n version: ${{ fromJson(needs.setup.outputs.m) }}", + "single expression vector": "version: ${{ fromJson(needs.setup.outputs.m) }}", } { - t.Run(tt.name, func(t *testing.T) { - planned, err := Parse(fmt.Appendf(nil, workflow, tt.matrix)) + t.Run(name, func(t *testing.T) { + planned, err := Parse(fmt.Appendf(nil, workflow, matrix)) require.NoError(t, err) var payload []byte @@ -357,90 +505,15 @@ jobs: } require.NotEmpty(t, payload, "no placeholder was planned for build") - // The stored payload keeps the raw matrix, but no longer the needs that made Parse defer it. _, job, err := ParseRawSingleWorkflow(payload) require.NoError(t, err) assert.Equal(t, "build", job.Name) - // The needs are gone, which is exactly why Parse no longer defers this payload. assert.Empty(t, job.Needs()) assert.False(t, HasDeferredMatrix(job)) - // Guard the reason ParseRawSingleWorkflow exists, so a future Parse change cannot quietly - // make the placeholder re-expandable again without this being noticed. reparsed, err := Parse(payload) - if tt.parseErrHas != "" { - require.ErrorContains(t, err, tt.parseErrHas) - } else { - require.NoError(t, err) - assert.Len(t, reparsed, tt.parseCount) - } + require.NoError(t, err) + assert.Len(t, reparsed, 1) }) } } - -func TestEvaluateJobIfExpressionLeavesRawMatrixUnavailable(t *testing.T) { - // A placeholder's `if:` is read before its matrix can be resolved. `matrix.*` has to be absent - // there: binding it to the expression's own source text would decide the job against a value no - // combination ever has, and an include/exclude that is still a scalar cannot be read at all. - t.Run("include expression is not read", func(t *testing.T) { - run, err := evaluateJobIf(t, "include: ${{ fromJson(needs.setup.outputs.m) }}", "${{ true }}", true) - require.NoError(t, err) - assert.True(t, run) - }) - - t.Run("matrix context is null, not the raw expression", func(t *testing.T) { - const matrix = "version: ${{ fromJson(needs.setup.outputs.m) }}" - run, err := evaluateJobIf(t, matrix, "${{ matrix.version == null }}", true) - require.NoError(t, err) - assert.True(t, run) - - run, err = evaluateJobIf(t, matrix, "${{ matrix.version == '${{ fromJson(needs.setup.outputs.m) }}' }}", true) - require.NoError(t, err) - assert.False(t, run) - }) - - t.Run("an expanded job still reads its combination", func(t *testing.T) { - run, err := evaluateJobIf(t, "version: [1]", "${{ matrix.version == 1 }}", false) - require.NoError(t, err) - assert.True(t, run) - }) -} - -func TestExpressionReadsMatrix(t *testing.T) { - // Erring toward true only postpones the `if:` to the pass that has the combination, which decides it correctly anyway. - for value, want := range map[string]bool{ - "": false, - "true": false, // a bare literal is an expression too, it just reads nothing - "${{ always() }}": false, - "${{ needs.setup.result == 'ok' }}": false, - "${{ vars.MATRIX }}": false, // a name that merely looks like the context - "${{ matrix.os }}": true, - "${{ MATRIX.os }}": true, // contexts are case-insensitive - "${{ always() && matrix.os == 1 }}": true, - "${{ contains(matrix.tags, 'a') }}": true, - "${{ toJSON(matrix) }}": true, // the whole context, not a property of it - "${{ vars.A }}${{ matrix.os }}": true, // only the second of two expressions reads it - "${{ matrix.os == }}": true, // unparseable, postpone rather than decide it here - // An `if:` may omit the `${{ }}`, and is evaluated as one expression either way. - "matrix.os == 'a'": true, - "needs.setup.result == 'ok'": false, - } { - assert.Equal(t, want, ExpressionReadsMatrix(value), "value %q", value) - } -} - -func TestExpressionIgnoresNeedResults(t *testing.T) { - for value, want := range map[string]bool{ - "": false, - "${{ matrix.os == 'a' }}": false, - "${{ success() }}": false, // the implicit gate, so the fallback already matches it - "${{ always() }}": true, - "${{ ALWAYS() && matrix.os }}": true, // function names are case-insensitive - "${{ failure() }}": true, - "${{ cancelled() }}": true, - "always() && matrix.os == 'a'": true, // the brace-less form of the same gate - "${{ vars.always }}": false, - } { - assert.Equal(t, want, ExpressionIgnoresNeedResults(value), "value %q", value) - } -} diff --git a/modules/actions/jobparser/model.go b/modules/actions/jobparser/model.go index e95a123de33..a5b7b1d6897 100644 --- a/modules/actions/jobparser/model.go +++ b/modules/actions/jobparser/model.go @@ -14,18 +14,19 @@ import ( "gitea.dev/actionslib/pkg/model" "gitea.dev/modules/util" + "github.com/robfig/cron/v3" "go.yaml.in/yaml/v4" ) // SingleWorkflow is a workflow with single job and single matrix type SingleWorkflow struct { - Name string `yaml:"name,omitempty"` - RawOn yaml.Node `yaml:"on,omitempty"` - Env map[string]string `yaml:"env,omitempty"` - RawJobs yaml.Node `yaml:"jobs,omitempty"` - Defaults Defaults `yaml:"defaults,omitempty"` - RawPermissions yaml.Node `yaml:"permissions,omitempty"` - RunName string `yaml:"run-name,omitempty"` + Name string `yaml:"name,omitempty"` + RawOn yaml.Node `yaml:"on,omitempty"` + Env yaml.Node `yaml:"env,omitempty"` + RawJobs yaml.Node `yaml:"jobs,omitempty"` + Defaults Defaults `yaml:"defaults,omitempty"` + RawPermissions yaml.Node `yaml:"permissions,omitempty"` + RunName string `yaml:"run-name,omitempty"` } func (w *SingleWorkflow) Job() (string, *Job) { @@ -93,7 +94,9 @@ func (w *SingleWorkflow) Marshal() ([]byte, error) { var buf bytes.Buffer enc := yaml.NewEncoder(&buf) enc.SetIndent(2) - if err := enc.Encode(w); err != nil { + payload := *w + payload.RunName = "" // already interpolated into the run title, a runner would parse it as a template again + if err := enc.Encode(&payload); err != nil { return nil, err } if err := enc.Close(); err != nil { @@ -103,24 +106,24 @@ func (w *SingleWorkflow) Marshal() ([]byte, error) { } type Job struct { - Name string `yaml:"name,omitempty"` - RawNeeds yaml.Node `yaml:"needs,omitempty"` - RawRunsOn yaml.Node `yaml:"runs-on,omitempty"` - Env yaml.Node `yaml:"env,omitempty"` - If yaml.Node `yaml:"if,omitempty"` - Steps []*Step `yaml:"steps,omitempty"` - TimeoutMinutes string `yaml:"timeout-minutes,omitempty"` - RawContinueOnError yaml.Node `yaml:"continue-on-error,omitempty"` - Services map[string]*ContainerSpec `yaml:"services,omitempty"` - Strategy Strategy `yaml:"strategy,omitempty"` - RawContainer yaml.Node `yaml:"container,omitempty"` - Defaults Defaults `yaml:"defaults,omitempty"` - Outputs map[string]string `yaml:"outputs,omitempty"` - Uses string `yaml:"uses,omitempty"` - With map[string]any `yaml:"with,omitempty"` - RawSecrets yaml.Node `yaml:"secrets,omitempty"` - RawConcurrency *model.RawConcurrency `yaml:"concurrency,omitempty"` - RawPermissions yaml.Node `yaml:"permissions,omitempty"` + Name string `yaml:"name,omitempty"` + RawNeeds yaml.Node `yaml:"needs,omitempty"` + RawRunsOn yaml.Node `yaml:"runs-on,omitempty"` + Env yaml.Node `yaml:"env,omitempty"` + If yaml.Node `yaml:"if,omitempty"` + Steps []*Step `yaml:"steps,omitempty"` + TimeoutMinutes string `yaml:"timeout-minutes,omitempty"` + RawContinueOnError yaml.Node `yaml:"continue-on-error,omitempty"` + Services yaml.Node `yaml:"services,omitempty"` + Strategy Strategy `yaml:"strategy,omitempty"` + RawContainer yaml.Node `yaml:"container,omitempty"` + Defaults yaml.Node `yaml:"defaults,omitempty"` + Outputs map[string]string `yaml:"outputs,omitempty"` + Uses string `yaml:"uses,omitempty"` + With yaml.Node `yaml:"with,omitempty"` + RawSecrets yaml.Node `yaml:"secrets,omitempty"` + RawConcurrency *model.RawConcurrency `yaml:"concurrency,omitempty"` + RawPermissions yaml.Node `yaml:"permissions,omitempty"` } // GetContinueOnError decodes the continue-on-error field to a bool. @@ -171,8 +174,18 @@ func (j *Job) EraseNeeds() *Job { return j } +// RunsOn returns the labels Gitea matches runners against, unescaped like DisplayName. func (j *Job) RunsOn() []string { - return (&model.Job{RawRunsOn: j.RawRunsOn}).RunsOn() + runsOn := model.RunsOnFromNode(j.RawRunsOn) + for i, label := range runsOn { + runsOn[i] = unescapeExpressions(label) + } + return runsOn +} + +// DisplayName is the name Gitea stores, without the escaping the payload keeps for runners. +func (j *Job) DisplayName() string { + return util.EllipsisDisplayString(unescapeExpressions(j.Name), 255) } // BlockSafeString works around https://github.com/yaml/go-yaml/issues/399, quoting a value whose @@ -187,17 +200,17 @@ func (s BlockSafeString) MarshalYAML() (any, error) { } type Step struct { - ID string `yaml:"id,omitempty"` - If yaml.Node `yaml:"if,omitempty"` - Name BlockSafeString `yaml:"name,omitempty"` - Uses string `yaml:"uses,omitempty"` - Run BlockSafeString `yaml:"run,omitempty"` - WorkingDirectory string `yaml:"working-directory,omitempty"` - Shell string `yaml:"shell,omitempty"` - Env yaml.Node `yaml:"env,omitempty"` - With map[string]string `yaml:"with,omitempty"` - RawContinueOnError yaml.Node `yaml:"continue-on-error,omitempty"` // raw: the runner evaluates it with the steps context - TimeoutMinutes string `yaml:"timeout-minutes,omitempty"` + ID string `yaml:"id,omitempty"` + If yaml.Node `yaml:"if,omitempty"` + Name BlockSafeString `yaml:"name,omitempty"` + Uses string `yaml:"uses,omitempty"` + Run BlockSafeString `yaml:"run,omitempty"` + WorkingDirectory string `yaml:"working-directory,omitempty"` + Shell string `yaml:"shell,omitempty"` + Env yaml.Node `yaml:"env,omitempty"` + With yaml.Node `yaml:"with,omitempty"` + RawContinueOnError yaml.Node `yaml:"continue-on-error,omitempty"` // raw: the runner evaluates it with the steps context + TimeoutMinutes string `yaml:"timeout-minutes,omitempty"` } // UnmarshalYAML canonicalizes booleans like continue-on-error @@ -225,20 +238,45 @@ func (s *Step) String() string { }).String() } -type ContainerSpec struct { - Image string `yaml:"image,omitempty"` - Env map[string]string `yaml:"env,omitempty"` - Ports []string `yaml:"ports,omitempty"` - Volumes []string `yaml:"volumes,omitempty"` - Options string `yaml:"options,omitempty"` - Credentials map[string]string `yaml:"credentials,omitempty"` - Cmd []string `yaml:"cmd,omitempty"` -} - type Strategy struct { FailFastString string `yaml:"fail-fast,omitempty"` MaxParallelString string `yaml:"max-parallel,omitempty"` RawMatrix yaml.Node `yaml:"matrix,omitempty"` + JobIndex int `yaml:"job-index,omitempty"` // set by buildMatrixCombos, read back from its payload + JobTotal int `yaml:"job-total,omitempty"` + RawExpression yaml.Node `yaml:"-"` // a whole-value `strategy: ${{ }}`, see Strategy.resolve +} + +type rawStrategy Strategy + +func (s *Strategy) UnmarshalYAML(node *yaml.Node) error { + if node.Kind == yaml.ScalarNode { + *s = Strategy{RawExpression: *node} + return nil + } + return node.Decode((*rawStrategy)(s)) +} + +func (s Strategy) MarshalYAML() (any, error) { + if s.RawExpression.Kind != 0 { + return &s.RawExpression, nil + } + return rawStrategy(s), nil +} + +func (s Strategy) actStrategy() *model.Strategy { + return &model.Strategy{FailFastString: s.FailFastString, MaxParallelString: s.MaxParallelString, RawMatrix: s.RawMatrix} +} + +// context is the strategy context, combination 0 of 1 without a matrix as on GitHub, and without job-index for an unexpanded matrix. +func (s *Strategy) context() map[string]any { + switch { + case s == nil: + return exprparser.StrategyContext(nil, 0, 0) + case s.RawMatrix.Kind == 0 && s.RawExpression.Kind == 0: + return exprparser.StrategyContext(s.actStrategy(), 0, 1) + } + return exprparser.StrategyContext(s.actStrategy(), s.JobIndex, s.JobTotal) } type Defaults struct { @@ -250,20 +288,10 @@ type RunDefaults struct { WorkingDirectory string `yaml:"working-directory,omitempty"` } -type WorkflowDispatchInput struct { - Name string `yaml:"name"` - Description string `yaml:"description"` - Required bool `yaml:"required"` - Default string `yaml:"default"` - Type string `yaml:"type"` - Options []string `yaml:"options"` -} - type Event struct { Name string acts map[string][]string schedules []map[string]string - inputs []WorkflowDispatchInput } func (evt *Event) IsSchedule() bool { @@ -278,10 +306,6 @@ func (evt *Event) Schedules() []map[string]string { return evt.schedules } -func (evt *Event) Inputs() []WorkflowDispatchInput { - return evt.inputs -} - func ReadWorkflowRawConcurrency(content []byte) (*model.RawConcurrency, error) { w, err := ReadWorkflow(content) if err != nil { @@ -290,26 +314,30 @@ func ReadWorkflowRawConcurrency(content []byte) (*model.RawConcurrency, error) { return w.RawConcurrency, nil } -func EvaluateConcurrency(rc *model.RawConcurrency, jobID string, job *Job, gitCtx map[string]any, results map[string]*JobResult, vars map[string]string, inputs map[string]any) (string, bool, error) { - actJob := &model.Job{} +// newJobEvaluator evaluates against a stored job's contexts, with its single matrix combination. +func newJobEvaluator(jobID string, job *Job, gitCtx map[string]any, results map[string]*JobResult, vars map[string]string, inputs map[string]any) (expreval.Evaluator, error) { + var strategy *Strategy + var matrix map[string]any if job != nil { - actJob.Strategy = &model.Strategy{ - FailFastString: job.Strategy.FailFastString, - MaxParallelString: job.Strategy.MaxParallelString, - RawMatrix: job.Strategy.RawMatrix, + strategy = &job.Strategy + rawMatrix := model.CloneYamlNode(job.Strategy.RawMatrix) + replaceScalars(&rawMatrix, unescapeExpressions) + matrixes, err := (&model.Job{Strategy: &model.Strategy{RawMatrix: rawMatrix}}).GetMatrixes() + if err != nil { + return expreval.Evaluator{}, err + } + if len(matrixes[0]) > 0 { + matrix = matrixes[0] } } + return expreval.New(NewInterpeter(jobID, strategy, matrix, model.GithubContextFromMap(gitCtx), results, vars, inputs).Evaluate), nil +} - matrix := make(map[string]any) - matrixes, err := matrixesOf(actJob) +func EvaluateConcurrency(rc *model.RawConcurrency, jobID string, job *Job, gitCtx map[string]any, results map[string]*JobResult, vars map[string]string, inputs map[string]any) (string, bool, error) { + evaluator, err := newJobEvaluator(jobID, job, gitCtx, results, vars, inputs) if err != nil { return "", false, err } - if len(matrixes) > 0 { - matrix = matrixes[0] - } - - evaluator := expreval.New(NewInterpeter(jobID, actJob, matrix, toGitContext(gitCtx), results, vars, inputs).Evaluate) var node yaml.Node if err := node.Encode(rc); err != nil { return "", false, fmt.Errorf("failed to encode concurrency: %w", err) @@ -327,40 +355,6 @@ func EvaluateConcurrency(rc *model.RawConcurrency, jobID string, job *Job, gitCt return evaluated.Group, util.ParseYamlBool(evaluated.CancelInProgress), nil } -func toGitContext(input map[string]any) *model.GithubContext { - gitContext := &model.GithubContext{ - EventPath: asString(input["event_path"]), - Workflow: asString(input["workflow"]), - RunID: asString(input["run_id"]), - RunNumber: asString(input["run_number"]), - Actor: asString(input["actor"]), - Repository: asString(input["repository"]), - EventName: asString(input["event_name"]), - Sha: asString(input["sha"]), - Ref: asString(input["ref"]), - RefName: asString(input["ref_name"]), - RefType: asString(input["ref_type"]), - HeadRef: asString(input["head_ref"]), - BaseRef: asString(input["base_ref"]), - Token: asString(input["token"]), - Workspace: asString(input["workspace"]), - Action: asString(input["action"]), - ActionPath: asString(input["action_path"]), - ActionRef: asString(input["action_ref"]), - ActionRepository: asString(input["action_repository"]), - Job: asString(input["job"]), - RepositoryOwner: asString(input["repository_owner"]), - RetentionDays: asString(input["retention_days"]), - } - - event, ok := input["event"].(map[string]any) - if ok { - gitContext.Event = event - } - - return gitContext -} - // workflowCallEvent is only fired by another workflow's `uses:`, so it is excluded from trigger detection. const workflowCallEvent = "workflow_call" @@ -372,6 +366,9 @@ func ParseRawOn(rawOn *yaml.Node) ([]*Event, error) { if err != nil { return nil, err } + if rawOn.ShortTag() != "!!str" || val == "" { + return nil, fmt.Errorf("invalid event %q", val) + } if val == workflowCallEvent { return []*Event{}, nil } @@ -421,18 +418,24 @@ func ParseRawOn(rawOn *yaml.Node) ([]*Event, error) { } schedules := make([]map[string]string, len(t)) if k == "schedule" { + if len(t) == 0 { + return nil, errors.New("schedule must contain at least one cron entry") + } for i, tt := range t { vv, ok := tt.(map[string]any) if !ok { - return nil, fmt.Errorf("unknown on type(schedule): %#v", v) + return nil, errors.New("unknown on type(schedule)") } schedules[i] = make(map[string]string, len(vv)) for k, vvv := range vv { var ok bool if schedules[i][k], ok = vvv.(string); !ok { - return nil, fmt.Errorf("unknown on type(schedule): %#v", v) + return nil, errors.New("unknown on type(schedule)") } } + if _, err := cron.ParseStandard(schedules[i]["cron"]); err != nil { + return nil, fmt.Errorf("invalid cron %q: %w", schedules[i]["cron"], err) + } } } @@ -444,14 +447,14 @@ func ParseRawOn(rawOn *yaml.Node) ([]*Event, error) { schedules: schedules, }) case yaml.MappingNode: + // Keep combined include and ignore filters for existing Gitea workflows, although GitHub rejects them. acts := make(map[string][]string, len(v.Content)/2) - var inputs []WorkflowDispatchInput expectedKey := true var act string for _, content := range v.Content { if expectedKey { if content.Kind != yaml.ScalarNode { - return nil, fmt.Errorf("key type not string: %#v", content) + return nil, errors.New("key type not string") } act = "" err := content.Decode(&act) @@ -476,48 +479,23 @@ func ParseRawOn(rawOn *yaml.Node) ([]*Event, error) { acts[act] = []string{t} case yaml.MappingNode: if k != "workflow_dispatch" || act != "inputs" { - return nil, fmt.Errorf("map should only for workflow_dispatch but %s: %#v", act, content) + return nil, fmt.Errorf("map should only for workflow_dispatch but %s", act) } - - var key string - for i, vv := range content.Content { - if i%2 == 0 { - if vv.Kind != yaml.ScalarNode { - return nil, fmt.Errorf("key type not string: %#v", vv) - } - key = "" - if err := vv.Decode(&key); err != nil { - return nil, err - } - } else { - if vv.Kind != yaml.MappingNode { - return nil, fmt.Errorf("key type not map(%s): %#v", key, vv) - } - - input := WorkflowDispatchInput{} - if err := vv.Decode(&input); err != nil { - return nil, err - } - input.Name = key - inputs = append(inputs, input) - } + if err := content.Decode(new(map[string]model.WorkflowDispatchInput)); err != nil { + return nil, err } default: - return nil, fmt.Errorf("unknown on type: %#v", content) + return nil, fmt.Errorf("unknown on type for %s", act) } } expectedKey = !expectedKey } - if len(inputs) == 0 { - inputs = nil - } if len(acts) == 0 { acts = nil } res = append(res, &Event{ - Name: k, - acts: acts, - inputs: inputs, + Name: k, + acts: acts, }) default: return nil, fmt.Errorf("unknown on type: %v", v.Kind) @@ -529,35 +507,12 @@ func ParseRawOn(rawOn *yaml.Node) ([]*Event, error) { } } -// EvaluateJobIfExpression evaluates a job's `if:`. -func EvaluateJobIfExpression(jobID string, job *Job, gitCtx map[string]any, results map[string]*JobResult, vars map[string]string, inputs map[string]any, matrixDeferred bool) (bool, error) { - actJob := &model.Job{ - Strategy: &model.Strategy{ - FailFastString: job.Strategy.FailFastString, - MaxParallelString: job.Strategy.MaxParallelString, - RawMatrix: job.Strategy.RawMatrix, - }, +// EvaluateJobIfExpression evaluates a job's `if:`, which github.com decides before the matrix, so without the matrix and strategy contexts. +func EvaluateJobIfExpression(jobID string, job *Job, gitCtx map[string]any, results map[string]*JobResult, vars map[string]string, inputs map[string]any) (bool, error) { + if unavailable := unavailableContext(IfExpression(job.If.Value), jobConditionContexts); unavailable != "" { // only a job stored before its conditions were validated + return false, fmt.Errorf("job %s: Unrecognized named-value: '%s', update the workflow and trigger a new run", jobID, unavailable) } - // Each per-matrix job carries its single matrix combination in RawMatrix so resolve it and pass it in; - // otherwise `matrix.*` references in `if:` evaluate to null. - // GetMatrixes always returns at least one element (an empty map for a job without a matrix), - // so only a non-empty combination should populate `matrix.*`, leaving it nil otherwise. - // - // A deferred-matrix placeholder is the exception: its combinations do not exist yet, and reading the - // raw matrix here would either fail outright (an `include` that is still a scalar expression) or bind - // `matrix.*` to the expression's own source text. Leaving it nil is safe: the caller checks - // ExpressionReadsMatrix first, so an `if:` that reads `matrix.*` is deferred to the post-expansion pass. - var matrix map[string]any - if !matrixDeferred { - matrixes, err := matrixesOf(actJob) - if err != nil { - return false, err - } - if len(matrixes) > 0 && len(matrixes[0]) > 0 { - matrix = matrixes[0] - } - } - evaluator := expreval.New(NewInterpeter(jobID, actJob, matrix, toGitContext(gitCtx), results, vars, inputs).Evaluate) + evaluator := expreval.New(NewInterpeter(jobID, nil, nil, model.GithubContextFromMap(gitCtx), results, vars, inputs).Evaluate) return evaluator.EvalBool(job.If.Value, exprparser.DefaultStatusCheckSuccess) } @@ -593,12 +548,3 @@ func parseMappingNode[T any](node *yaml.Node) ([]string, []T, error) { return scalars, datas, nil } - -func asString(v any) string { - if v == nil { - return "" - } else if s, ok := v.(string); ok { - return s - } - return "" -} diff --git a/modules/actions/jobparser/model_test.go b/modules/actions/jobparser/model_test.go index e8d2421efc3..af708b3cc95 100644 --- a/modules/actions/jobparser/model_test.go +++ b/modules/actions/jobparser/model_test.go @@ -17,9 +17,18 @@ import ( func TestParseRawOn(t *testing.T) { kases := []struct { - input string - result []*Event + input string + result []*Event + wantErr bool }{ + {input: "on:\n push:\n branches:\n a: b", wantErr: true}, + {input: "on: [push, {pull_request: null}]", wantErr: true}, + {input: "on:", wantErr: true}, + {input: "on: 42", wantErr: true}, + {input: "jobs: {}", wantErr: true}, + {input: "on:\n schedule: []", wantErr: true}, + {input: "on:\n schedule:\n - {}", wantErr: true}, + {input: "on:\n schedule:\n - cron: nope", wantErr: true}, { input: "on: issue_comment", result: []*Event{ @@ -190,13 +199,14 @@ func TestParseRawOn(t *testing.T) { }, }, { - input: "on:\n schedule:\n - cron: '20 6 * * *'", + input: "on:\n schedule:\n - cron: '20 6 * * *'\n timezone: UTC", result: []*Event{ { Name: "schedule", schedules: []map[string]string{ { - "cron": "20 6 * * *", + "cron": "20 6 * * *", + "timezone": "UTC", }, }, }, @@ -228,28 +238,6 @@ func TestParseRawOn(t *testing.T) { result: []*Event{ { Name: "workflow_dispatch", - inputs: []WorkflowDispatchInput{ - { - Name: "logLevel", - Description: "Log level", - Required: true, - Default: "warning", - Type: "choice", - Options: []string{"info", "warning", "debug"}, - }, - { - Name: "tags", - Description: "Test scenario tags", - Required: false, - Type: "boolean", - }, - { - Name: "environment", - Description: "Environment to run tests against", - Type: "environment", - Required: true, - }, - }, }, { Name: "push", @@ -310,6 +298,10 @@ func TestParseRawOn(t *testing.T) { assert.NoError(t, err) events, err := ParseRawOn(&origin.RawOn) + if kase.wantErr { + assert.Error(t, err) + return + } assert.NoError(t, err) assert.Equal(t, kase.result, events, events) }) @@ -467,51 +459,6 @@ func TestParseMappingNode(t *testing.T) { } } -func TestEvaluateJobIfExpressionMatrix(t *testing.T) { - ifExprs := []string{ - `${{ contains(fromJSON('["linux","windows"]'), matrix.target) }}`, - `${{ contains('["linux","windows"]', matrix.target) }}`, - } - - want := map[string]bool{ - "build (linux)": true, - "build (windows)": true, - "build (macos)": false, - } - - for _, ifExpr := range ifExprs { - t.Run(ifExpr, func(t *testing.T) { - content := fmt.Sprintf(` -name: test -on: push -jobs: - build: - runs-on: ubuntu-latest - if: %s - strategy: - fail-fast: false - matrix: - target: [linux, windows, macos] - steps: - - run: echo ${{ matrix.target }} -`, ifExpr) - - swfs, err := Parse([]byte(content)) - require.NoError(t, err) - require.Len(t, swfs, 3) - - got := make(map[string]bool, len(swfs)) - for _, swf := range swfs { - id, job := swf.Job() - shouldRun, err := EvaluateJobIfExpression(id, job, map[string]any{}, map[string]*JobResult{id: {}}, nil, nil, false) - require.NoError(t, err) - got[job.Name] = shouldRun - } - assert.Equal(t, want, got) - }) - } -} - func TestEvaluateJobIfExpression(t *testing.T) { kases := []struct { name string @@ -567,9 +514,23 @@ jobs: "job1": {Result: kase.needResult}, "job2": {Needs: []string{"job1"}}, } - got, err := EvaluateJobIfExpression("job2", job2, map[string]any{}, results, nil, nil, false) + got, err := EvaluateJobIfExpression("job2", job2, map[string]any{}, results, nil, nil) require.NoError(t, err) assert.Equal(t, kase.expected, got) }) } + + t.Run("stored unavailable context", func(t *testing.T) { + for condition, wantErr := range map[string]string{ + "matrix.os == 'a'": "Unrecognized named-value: 'matrix'", + "${{ strategy.fail-fast }}": "Unrecognized named-value: 'strategy'", + "secrets.TOKEN != ''": "Unrecognized named-value: 'secrets'", + "${{ matrix.os == }}": "Unexpected end of expression", + } { + _, job, err := ParseRawSingleWorkflow(fmt.Appendf(nil, "jobs: {job2: {if: %q, strategy: {matrix: {os: [a]}}}}", condition)) + require.NoError(t, err) + _, err = EvaluateJobIfExpression("job2", job, map[string]any{}, map[string]*JobResult{"job2": {}}, nil, nil) + assert.ErrorContains(t, err, wantErr, condition) + } + }) } diff --git a/modules/actions/jobparser/testdata/continue_on_error_expr.out.yaml b/modules/actions/jobparser/testdata/continue_on_error_expr.out.yaml index 0d3afff878c..0e621b65796 100644 --- a/modules/actions/jobparser/testdata/continue_on_error_expr.out.yaml +++ b/modules/actions/jobparser/testdata/continue_on_error_expr.out.yaml @@ -10,6 +10,7 @@ jobs: matrix: experimental: - false + job-total: 2 --- name: test jobs: @@ -23,3 +24,5 @@ jobs: matrix: experimental: - true + job-index: 1 + job-total: 2 diff --git a/modules/actions/jobparser/testdata/job_name_with_matrix.out.yaml b/modules/actions/jobparser/testdata/job_name_with_matrix.out.yaml index 081e8d4abd6..ebceb976a57 100644 --- a/modules/actions/jobparser/testdata/job_name_with_matrix.out.yaml +++ b/modules/actions/jobparser/testdata/job_name_with_matrix.out.yaml @@ -14,6 +14,8 @@ jobs: - ubuntu-20.04 version: - 1.17 + job-index: 3 + job-total: 6 --- name: test jobs: @@ -31,6 +33,8 @@ jobs: - ubuntu-20.04 version: - 1.18 + job-index: 4 + job-total: 6 --- name: test jobs: @@ -48,6 +52,8 @@ jobs: - ubuntu-20.04 version: - 1.19 + job-index: 5 + job-total: 6 --- name: test jobs: @@ -65,6 +71,7 @@ jobs: - ubuntu-22.04 version: - 1.17 + job-total: 6 --- name: test jobs: @@ -82,6 +89,8 @@ jobs: - ubuntu-22.04 version: - 1.18 + job-index: 1 + job-total: 6 --- name: test jobs: @@ -99,3 +108,5 @@ jobs: - ubuntu-22.04 version: - 1.19 + job-index: 2 + job-total: 6 diff --git a/modules/actions/jobparser/testdata/multiple_matrix.out.yaml b/modules/actions/jobparser/testdata/multiple_matrix.out.yaml index e277cdd0eeb..0dcd7e5562d 100644 --- a/modules/actions/jobparser/testdata/multiple_matrix.out.yaml +++ b/modules/actions/jobparser/testdata/multiple_matrix.out.yaml @@ -14,6 +14,8 @@ jobs: - ubuntu-20.04 version: - 1.17 + job-index: 3 + job-total: 6 --- name: test jobs: @@ -31,6 +33,8 @@ jobs: - ubuntu-20.04 version: - 1.18 + job-index: 4 + job-total: 6 --- name: test jobs: @@ -48,6 +52,8 @@ jobs: - ubuntu-20.04 version: - 1.19 + job-index: 5 + job-total: 6 --- name: test jobs: @@ -65,6 +71,7 @@ jobs: - ubuntu-22.04 version: - 1.17 + job-total: 6 --- name: test jobs: @@ -82,6 +89,8 @@ jobs: - ubuntu-22.04 version: - 1.18 + job-index: 1 + job-total: 6 --- name: test jobs: @@ -99,3 +108,5 @@ jobs: - ubuntu-22.04 version: - 1.19 + job-index: 2 + job-total: 6 diff --git a/modules/actions/jobparser/uses.go b/modules/actions/jobparser/uses.go deleted file mode 100644 index 1519dc4f865..00000000000 --- a/modules/actions/jobparser/uses.go +++ /dev/null @@ -1,79 +0,0 @@ -// Copyright 2026 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package jobparser - -import ( - "errors" - "fmt" - "path" - "regexp" - "strings" -) - -// UsesKind enumerates the supported forms of a reusable workflow "uses:" value. -type UsesKind int - -const ( - // UsesKindLocalSameRepo is ".//foo.yml" or "$//foo.yml" - a path inside the calling repository. - // For example: "./.gitea/workflows/foo.yml" - UsesKindLocalSameRepo UsesKind = iota + 1 - // UsesKindLocalCrossRepo is "owner/repo//foo.yml@ref" - a workflow in another repo on the same instance. - // For example: "owner/repo/.gitea/workflows/foo.yml@ref" - UsesKindLocalCrossRepo -) - -// UsesRef is the parsed form of a reusable workflow "uses:" value. -type UsesRef struct { - Kind UsesKind - Owner string // empty for UsesKindLocalSameRepo - Repo string // empty for UsesKindLocalSameRepo - Path string // workflow file path inside the source repo - Ref string // git ref; empty for UsesKindLocalSameRepo -} - -var ( - reLocalSameRepo = regexp.MustCompile(`^[.$]/([^@]+\.ya?ml)$`) - reLocalCrossRepo = regexp.MustCompile(`^([-.\w]+)/([-.\w]+)/([^@]+\.ya?ml)@(.+)$`) -) - -// ParseUses parses the SYNTAX of a reusable workflow "uses:" value into a UsesRef. Two forms are supported: -// - ".//foo.yml" or "$//foo.yml" (UsesKindLocalSameRepo, no @ref) -// - "OWNER/REPO//foo.yml@REF" (UsesKindLocalCrossRepo) -// -// It deliberately does NOT validate that is an allowed workflow directory: the allowed directories are instance-configurable (WORKFLOW_DIRS / SCOPED_WORKFLOW_DIRS). -// The caller (services/actions.ResolveUses) enforces the directory allowlist. The returned Path is the cleaned, repo-relative file path. -func ParseUses(s string) (*UsesRef, error) { - s = strings.TrimSpace(s) - if s == "" { - return nil, errors.New("empty uses value") - } - - if strings.HasPrefix(s, "./") || strings.HasPrefix(s, "$/") { - m := reLocalSameRepo.FindStringSubmatch(s) - if m == nil { - return nil, fmt.Errorf(`invalid local "uses:" %q (expect .//.yml or $//.yml)`, s) - } - p := m[1] - if path.Clean(p) != p { - return nil, fmt.Errorf("invalid workflow path %q", s) - } - return &UsesRef{Kind: UsesKindLocalSameRepo, Path: p}, nil - } - - m := reLocalCrossRepo.FindStringSubmatch(s) - if m == nil { - return nil, fmt.Errorf(`invalid cross-repo "uses:" %q (expect owner/repo//.yml@ref)`, s) - } - p := m[3] - if path.Clean(p) != p { - return nil, fmt.Errorf("invalid workflow path %q", s) - } - return &UsesRef{ - Kind: UsesKindLocalCrossRepo, - Owner: m[1], - Repo: m[2], - Path: p, - Ref: m[4], - }, nil -} diff --git a/modules/actions/jobparser/uses_test.go b/modules/actions/jobparser/uses_test.go deleted file mode 100644 index c7da1edc939..00000000000 --- a/modules/actions/jobparser/uses_test.go +++ /dev/null @@ -1,195 +0,0 @@ -// Copyright 2026 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package jobparser - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestParseUses(t *testing.T) { - t.Run("LocalSameRepo", func(t *testing.T) { - cases := []struct { - name string - in string - want UsesRef - }{ - { - name: "gitea dir, .yml", - in: "./.gitea/workflows/build.yml", - want: UsesRef{Kind: UsesKindLocalSameRepo, Path: ".gitea/workflows/build.yml"}, - }, - { - name: "github dir, .yml", - in: "./.github/workflows/build.yml", - want: UsesRef{Kind: UsesKindLocalSameRepo, Path: ".github/workflows/build.yml"}, - }, - { - name: "gitea dir, .yaml", - in: "./.gitea/workflows/build.yaml", - want: UsesRef{Kind: UsesKindLocalSameRepo, Path: ".gitea/workflows/build.yaml"}, - }, - { - name: "filename containing dots is allowed", - in: "./.gitea/workflows/foo..bar.yml", - want: UsesRef{Kind: UsesKindLocalSameRepo, Path: ".gitea/workflows/foo..bar.yml"}, - }, - { - name: "nested subdirectory", - in: "./.gitea/workflows/sub/build.yml", - want: UsesRef{Kind: UsesKindLocalSameRepo, Path: ".gitea/workflows/sub/build.yml"}, - }, - { - // ParseUses is dir-agnostic; the allowed directories (WORKFLOW_DIRS / SCOPED_WORKFLOW_DIRS) are enforced by ResolveUses. - name: "scoped workflows dir parses", - in: "./.gitea/scoped_workflows/lib.yml", - want: UsesRef{Kind: UsesKindLocalSameRepo, Path: ".gitea/scoped_workflows/lib.yml"}, - }, - { - name: "non-default dir parses (allowlist enforced downstream)", - in: "./.gitea/custom_workflows/x.yaml", - want: UsesRef{Kind: UsesKindLocalSameRepo, Path: ".gitea/custom_workflows/x.yaml"}, - }, - { - name: "self-repo prefix", - in: "$/.gitea/workflows/build.yml", - want: UsesRef{Kind: UsesKindLocalSameRepo, Path: ".gitea/workflows/build.yml"}, - }, - { - name: "leading/trailing whitespace is trimmed", - in: " ./.gitea/workflows/build.yml ", - want: UsesRef{Kind: UsesKindLocalSameRepo, Path: ".gitea/workflows/build.yml"}, - }, - } - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - got, err := ParseUses(c.in) - require.NoError(t, err) - assert.Equal(t, c.want, *got) - }) - } - }) - - t.Run("LocalCrossRepo", func(t *testing.T) { - cases := []struct { - name string - in string - want UsesRef - }{ - { - name: "gitea dir, simple ref", - in: "owner/repo/.gitea/workflows/build.yml@v1", - want: UsesRef{ - Kind: UsesKindLocalCrossRepo, - Owner: "owner", - Repo: "repo", - Path: ".gitea/workflows/build.yml", - Ref: "v1", - }, - }, - { - name: "github dir, branch ref", - in: "owner/repo/.github/workflows/build.yml@main", - want: UsesRef{ - Kind: UsesKindLocalCrossRepo, - Owner: "owner", - Repo: "repo", - Path: ".github/workflows/build.yml", - Ref: "main", - }, - }, - { - name: ".yaml extension", - in: "owner/repo/.gitea/workflows/build.yaml@abc123", - want: UsesRef{ - Kind: UsesKindLocalCrossRepo, - Owner: "owner", - Repo: "repo", - Path: ".gitea/workflows/build.yaml", - Ref: "abc123", - }, - }, - { - name: "ref with slashes (refs/heads/feature)", - in: "owner/repo/.gitea/workflows/build.yml@refs/heads/feature", - want: UsesRef{ - Kind: UsesKindLocalCrossRepo, - Owner: "owner", - Repo: "repo", - Path: ".gitea/workflows/build.yml", - Ref: "refs/heads/feature", - }, - }, - { - name: "nested subdirectory under workflows", - in: "owner/repo/.gitea/workflows/sub/build.yml@v1", - want: UsesRef{ - Kind: UsesKindLocalCrossRepo, - Owner: "owner", - Repo: "repo", - Path: ".gitea/workflows/sub/build.yml", - Ref: "v1", - }, - }, - { - name: "scoped workflows dir parses (allowlist enforced by ResolveUses)", - in: "owner/repo/.gitea/scoped_workflows/lib.yml@v1", - want: UsesRef{ - Kind: UsesKindLocalCrossRepo, - Owner: "owner", - Repo: "repo", - Path: ".gitea/scoped_workflows/lib.yml", - Ref: "v1", - }, - }, - } - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - got, err := ParseUses(c.in) - require.NoError(t, err) - assert.Equal(t, c.want, *got) - }) - } - }) - - t.Run("Errors", func(t *testing.T) { - cases := []struct { - name string - in string - }{ - {name: "empty string", in: ""}, - {name: "whitespace only", in: " "}, - - // Same-repo malformed (note: a wrong *directory* parses and should be rejected by the caller) - {name: "same-repo with @ref", in: "./.gitea/workflows/build.yml@v1"}, - {name: "self-repo with @ref", in: "$/.gitea/workflows/build.yml@v1"}, - {name: "same-repo wrong extension", in: "./.gitea/workflows/build.txt"}, - {name: "same-repo missing extension", in: "./.gitea/workflows/build"}, - {name: "same-repo absolute path", in: "/.gitea/workflows/build.yml"}, - {name: "same-repo path traversal", in: "./.gitea/workflows/../escape.yml"}, - {name: "same-repo double slash", in: "./.gitea/workflows//build.yml"}, - {name: "same-repo redundant ./", in: "./.gitea/workflows/./build.yml"}, - - // Cross-repo malformed - {name: "cross-repo missing @ref", in: "owner/repo/.gitea/workflows/build.yml"}, - {name: "cross-repo empty ref", in: "owner/repo/.gitea/workflows/build.yml@"}, - {name: "cross-repo missing owner", in: "/repo/.gitea/workflows/build.yml@v1"}, - {name: "cross-repo missing repo", in: "owner//.gitea/workflows/build.yml@v1"}, - {name: "cross-repo wrong extension", in: "owner/repo/.gitea/workflows/build.txt@v1"}, - {name: "cross-repo path traversal", in: "owner/repo/.gitea/workflows/../escape.yml@v1"}, - {name: "cross-repo double slash in path", in: "owner/repo/.gitea/workflows//build.yml@v1"}, - // owner/repo with chars Gitea's name validators reject - {name: "cross-repo owner with space", in: "bad owner/repo/.gitea/workflows/build.yml@v1"}, - {name: "cross-repo repo with @", in: "owner/re@po/.gitea/workflows/build.yml@v1"}, - } - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - _, err := ParseUses(c.in) - assert.Error(t, err) - }) - } - }) -} diff --git a/modules/actions/jobparser/validate.go b/modules/actions/jobparser/validate.go new file mode 100644 index 00000000000..c054d30c40b --- /dev/null +++ b/modules/actions/jobparser/validate.go @@ -0,0 +1,112 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package jobparser + +import ( + "errors" + "fmt" + "slices" + + "gitea.dev/actionslib/pkg/expreval" + "gitea.dev/actionslib/pkg/exprparser" + "gitea.dev/actionslib/pkg/model" +) + +// jobConditionContexts are what github.com gives `jobs..if`, which it decides before the matrix, plus the `gitea` alias. +var jobConditionContexts = []string{"github", "gitea", "needs", "vars", "inputs"} + +func ValidateWorkflowStatic(content []byte) ([]*Event, error) { + doc, err := resolveYamlAliases(content) + if err != nil { + return nil, err + } + // Keep unknown and case-distinct keys accepted for existing Gitea workflows. + workflow, err := readWorkflowDoc(doc) + if err != nil { + return nil, err + } + events, err := ParseRawOn(&workflow.RawOn) + if err != nil { + return nil, err + } + if err := validateWorkflowStructure(workflow); err != nil { + return nil, err + } + var header struct { + RunName string `yaml:"run-name"` + } + if err := decodeYamlDoc(doc, &header); err != nil { + return nil, err + } + if unavailable := unavailableContext(header.RunName, []string{"github", "gitea", "inputs", "vars"}); unavailable != "" { + return nil, fmt.Errorf("run-name: Unrecognized named-value: '%s'", unavailable) + } + return events, nil +} + +func validateJobConditions(workflow *model.Workflow) error { + for id, job := range workflow.Jobs { + if job == nil { + continue + } + if unavailable := unavailableContext(IfExpression(job.If.Value), jobConditionContexts); unavailable != "" { + return fmt.Errorf("job %s: Unrecognized named-value: '%s'", id, unavailable) + } + } + return nil +} + +func unavailableContext(expression string, allowed []string) string { + var unavailable string + expreval.Match(expression, func(node exprparser.ExprNode) bool { + if variable, ok := node.(*exprparser.VariableNode); ok && !slices.ContainsFunc(allowed, func(name string) bool { return exprparser.OrdinalIgnoreCaseEqual(name, variable.Name) }) { + unavailable = variable.Name + } + return unavailable != "" + }) + return unavailable +} + +func validateWorkflowStructure(workflow *model.Workflow) error { + if len(workflow.Jobs) == 0 { + return errors.New("the workflow must contain at least one job") + } + for id, job := range workflow.Jobs { + if job == nil { + return fmt.Errorf("job %q has no configuration", id) + } + // a job without runs-on is accepted and runs on any runner, github.com rejects it + for _, dependency := range job.Needs() { + if _, ok := workflow.Jobs[dependency]; !ok { + return fmt.Errorf("job %q needs unknown job %q", id, dependency) + } + } + } + visited := make(map[string]bool, len(workflow.Jobs)) + visiting := make(map[string]bool, len(workflow.Jobs)) + var visit func(string) error + visit = func(id string) error { + if visiting[id] { + return fmt.Errorf("job %q has a dependency cycle", id) + } + if visited[id] { + return nil + } + visiting[id] = true + for _, dependency := range workflow.Jobs[id].Needs() { + if err := visit(dependency); err != nil { + return err + } + } + delete(visiting, id) + visited[id] = true + return nil + } + for id := range workflow.Jobs { + if err := visit(id); err != nil { + return err + } + } + return nil +} diff --git a/modules/actions/jobparser/workflow_call.go b/modules/actions/jobparser/workflow_call.go index bba4aeb54ef..05a0a88888d 100644 --- a/modules/actions/jobparser/workflow_call.go +++ b/modules/actions/jobparser/workflow_call.go @@ -9,274 +9,66 @@ import ( "regexp" "strings" - "gitea.dev/actionslib/pkg/expreval" - "gitea.dev/actionslib/pkg/exprparser" "gitea.dev/actionslib/pkg/model" - "gitea.dev/modules/container" - "gitea.dev/modules/util" "go.yaml.in/yaml/v4" ) -// InputType enumerates the allowed types for a workflow_call input. -type InputType string - -const ( - InputTypeString InputType = "string" - InputTypeBoolean InputType = "boolean" - InputTypeNumber InputType = "number" -) - -// InputSpec describes a single workflow_call input declaration. -type InputSpec struct { - Description string `yaml:"description"` - Required bool `yaml:"required"` - Default yaml.Node `yaml:"default"` - Type InputType `yaml:"type"` -} - -// SecretSpec describes a single workflow_call secret declaration. -type SecretSpec struct { - Description string `yaml:"description"` - Required bool `yaml:"required"` -} - -// OutputSpec describes a single workflow_call output declaration. -type OutputSpec struct { - Description string `yaml:"description"` - Value string `yaml:"value"` -} - -// WorkflowCallSpec is the parsed "on.workflow_call" schema of a called workflow. -type WorkflowCallSpec struct { - Inputs map[string]InputSpec - Secrets map[string]SecretSpec - Outputs map[string]OutputSpec -} - -// JobOutputs is the per-job-id outputs map used for evaluating workflow_call outputs. -type JobOutputs map[string]map[string]string - -// ParseWorkflowCallSpec extracts on.workflow_call.{inputs,secrets,outputs} from a workflow YAML. -// Returns an error if the workflow does not declare on.workflow_call at all. -func ParseWorkflowCallSpec(content []byte) (*WorkflowCallSpec, error) { - var doc struct { - On yaml.Node `yaml:"on"` - } - if err := decodeResolved(content, &doc); err != nil { - return nil, fmt.Errorf("parse workflow yaml: %w", err) - } - - wcNode, ok := findWorkflowCallNode(&doc.On) - if !ok { - return nil, errors.New("workflow does not declare on.workflow_call") - } - - spec := &WorkflowCallSpec{ - Inputs: map[string]InputSpec{}, - Secrets: map[string]SecretSpec{}, - Outputs: map[string]OutputSpec{}, - } - - if wcNode == nil || wcNode.Kind != yaml.MappingNode { - return spec, nil - } - - for i := 0; i+1 < len(wcNode.Content); i += 2 { - key := wcNode.Content[i] - val := wcNode.Content[i+1] - switch key.Value { - case "inputs": - if err := decodeWorkflowCallMapping(val, spec.Inputs); err != nil { - return nil, fmt.Errorf("parse workflow_call.inputs: %w", err) - } - case "secrets": - if err := decodeWorkflowCallMapping(val, spec.Secrets); err != nil { - return nil, fmt.Errorf("parse workflow_call.secrets: %w", err) - } - case "outputs": - if err := decodeWorkflowCallMapping(val, spec.Outputs); err != nil { - return nil, fmt.Errorf("parse workflow_call.outputs: %w", err) - } - } - } - - for name, in := range spec.Inputs { - if in.Type == "" { - return nil, fmt.Errorf("workflow_call input %q is missing required field \"type\"", name) - } - switch in.Type { - case InputTypeString, InputTypeBoolean, InputTypeNumber: - default: - return nil, fmt.Errorf("workflow_call input %q has unsupported type %q", name, in.Type) - } - } - - return spec, nil -} - -// findWorkflowCallNode walks the "on:" node and returns the value mapping (or nil) for "workflow_call". -// "ok" is true when the workflow declares workflow_call (even with an empty body). -func findWorkflowCallNode(on *yaml.Node) (val *yaml.Node, ok bool) { - if on == nil || on.Kind == 0 { - return nil, false - } - switch on.Kind { - case yaml.ScalarNode: - return nil, on.Value == "workflow_call" - case yaml.SequenceNode: - for _, item := range on.Content { - if item.Kind == yaml.ScalarNode && item.Value == "workflow_call" { - return nil, true - } - } - return nil, false - case yaml.MappingNode: - for i := 0; i+1 < len(on.Content); i += 2 { - k := on.Content[i] - v := on.Content[i+1] - if k.Value != "workflow_call" { - continue - } - if v.Kind == yaml.MappingNode { - return v, true - } - return nil, true - } - } - return nil, false -} - -func decodeWorkflowCallMapping[T any](node *yaml.Node, dst map[string]T) error { - if node == nil || node.Kind != yaml.MappingNode { - return nil - } - for i := 0; i+1 < len(node.Content); i += 2 { - name := node.Content[i].Value - var v T - if err := node.Content[i+1].Decode(&v); err != nil { - return fmt.Errorf("%q: %w", name, err) - } - dst[name] = v - } - return nil -} - -// EvaluateCallerWith evaluates the caller-side expressions in `job.With` against the provided contexts -func EvaluateCallerWith( +// ResolveCallerInputs evaluates the caller's `with` against its own contexts and types it by the called workflow's inputs. +func ResolveCallerInputs( jobID string, job *Job, + config *model.WorkflowCall, gitCtx map[string]any, results map[string]*JobResult, vars map[string]string, inputs map[string]any, ) (map[string]any, error) { - actJob := &model.Job{Strategy: &model.Strategy{ - FailFastString: job.Strategy.FailFastString, - MaxParallelString: job.Strategy.MaxParallelString, - RawMatrix: job.Strategy.RawMatrix, - }} - - var matrix map[string]any - matrixes, err := matrixesOf(actJob) + if job.With.Kind != 0 && job.With.Kind != yaml.MappingNode { + return nil, errors.New("caller with must be a mapping") + } + evaluator, err := newJobEvaluator(jobID, job, gitCtx, results, vars, inputs) if err != nil { return nil, fmt.Errorf("get caller %q matrix: %w", jobID, err) } - if len(matrixes) > 0 { - matrix = matrixes[0] + var with map[string]any + if err := model.DecodeEvaluated("with", job.With, evaluator.EvaluateYamlNode, &with); err != nil { + return nil, err } - - evaluator := expreval.New(NewInterpeter(jobID, actJob, matrix, toGitContext(gitCtx), results, vars, inputs).Evaluate) - - out := make(map[string]any, len(job.With)) - for k, raw := range job.With { - var evaluated any - switch v := raw.(type) { - case string: - node := yaml.Node{} - if err := node.Encode(v); err != nil { - return nil, fmt.Errorf("encode caller %q with[%q]: %w", jobID, k, err) - } - if err := evaluator.EvaluateYamlNode(&node); err != nil { - return nil, fmt.Errorf("evaluate caller %q with[%q]: %w", jobID, k, err) - } - if err := node.Decode(&evaluated); err != nil { - return nil, fmt.Errorf("decode caller %q with[%q]: %w", jobID, k, err) - } - default: - evaluated = v - } - out[k] = evaluated + declared := make(map[string]model.WorkflowCallInput, len(config.Inputs)) + for name, input := range config.Inputs { + declared[strings.ToLower(name)] = input } - return out, nil -} - -// MatchCallerInputsAgainstSpec checks the caller's already-evaluated `with:` values against the callee's declared `on.workflow_call.inputs` schema -func MatchCallerInputsAgainstSpec(spec *WorkflowCallSpec, evaluated map[string]any) (map[string]any, error) { - resolved := make(map[string]any, len(spec.Inputs)) - - // fill defaults first - for name, in := range spec.Inputs { - if in.Default.IsZero() { - continue - } - var defaultVal any - if err := in.Default.Decode(&defaultVal); err != nil { - return nil, fmt.Errorf("decode workflow_call input %q default: %w", name, err) - } - v, err := parseWorkflowCallInput(name, in.Type, defaultVal) - if err != nil { - return nil, err - } - resolved[name] = v - } - - for k, raw := range evaluated { - inputSpec, ok := spec.Inputs[k] + for name, value := range with { + input, ok := declared[strings.ToLower(name)] if !ok { - // ignore unknown "with:" keys - continue + continue // ignored as in released Gitea, github.com rejects an undeclared input } - converted, err := parseWorkflowCallInput(k, inputSpec.Type, raw) - if err != nil { - return nil, err + switch input.Type { + case "boolean": + if _, ok := value.(bool); !ok { + return nil, fmt.Errorf("input %s: expected a boolean, got %T", name, value) + } + case "number": + switch value.(type) { + case float64, int, int64, uint64: + default: + return nil, fmt.Errorf("input %s: expected a number, got %T", name, value) + } } - resolved[k] = converted } - - for name, in := range spec.Inputs { - if !in.Required { - continue - } - // resolved[name] is set when caller provided it OR when spec has a non-zero default - both satisfy "required". - if _, ok := resolved[name]; ok { - continue - } - return nil, fmt.Errorf("workflow_call input %q is required", name) - } - - return resolved, nil + return evaluator.ResolveWorkflowCallInputs(config, with) } -func parseWorkflowCallInput(name string, typ InputType, v any) (any, error) { - switch typ { - case InputTypeString: - return exprparser.CoerceToString(v), nil - case InputTypeBoolean: - // strict type matching: a boolean input only accepts a native bool, not a "true"/"false" string - if b, ok := v.(bool); ok { - return b, nil - } - return false, fmt.Errorf("workflow_call input %q expects boolean", name) - case InputTypeNumber: - // strict type matching: a number input rejects "123"/"3.14" strings. - if _, isString := v.(string); isString { - return 0.0, fmt.Errorf("workflow_call input %q expects number", name) - } - return util.ToFloat64(v) - default: - return nil, fmt.Errorf("workflow_call input %q has unsupported type %q", name, typ) +// ParseWorkflowCallConfig reads the workflow_call configuration of a called workflow, decoding only its `on`. +func ParseWorkflowCallConfig(content []byte) (*model.WorkflowCall, error) { + var workflow struct { + RawOn yaml.Node `yaml:"on"` } + if err := decodeResolved(content, &workflow); err != nil { + return nil, err + } + return (&model.Workflow{RawOn: workflow.RawOn}).ParseWorkflowCallConfig() } // SecretsInherit is the literal keyword used in a caller's `secrets: inherit` directive @@ -317,60 +109,3 @@ func ParseCallerSecrets(node yaml.Node) (inherit bool, mapping map[string]string } return false, out, nil } - -// ValidateCallerSecrets checks a caller's parsed explicit-mapping `secrets:` against the called workflow's declared `on.workflow_call.secrets` schema. -func ValidateCallerSecrets(spec *WorkflowCallSpec, mapping map[string]string) error { - if spec == nil { - return errors.New("ValidateCallerSecrets: nil workflow_call spec") - } - // Secret names are case-insensitive, so compare declared names and caller aliases upper-cased. - declaredNames := make(container.Set[string], len(spec.Secrets)) - for name := range spec.Secrets { - declaredNames.Add(strings.ToUpper(name)) - } - provided := make(container.Set[string], len(mapping)) - for alias := range mapping { - up := strings.ToUpper(alias) - provided.Add(up) - if !declaredNames.Contains(up) { - return fmt.Errorf("caller secret %q is not declared in the called workflow's on.workflow_call.secrets", alias) - } - } - for name, sec := range spec.Secrets { - if sec.Required && !provided.Contains(strings.ToUpper(name)) { - return fmt.Errorf("required secret %q is not provided by the caller", name) - } - } - return nil -} - -// EvaluateWorkflowCallOutputs evaluates a called workflow's "on.workflow_call.outputs..value" expressions against the provided contexts. -func EvaluateWorkflowCallOutputs(spec *WorkflowCallSpec, gitCtx *model.GithubContext, vars map[string]string, inputs map[string]any, jobOutputs JobOutputs) (map[string]string, error) { - if spec == nil || len(spec.Outputs) == 0 { - return map[string]string{}, nil - } - - jobsCtx := make(map[string]*model.WorkflowCallResult, len(jobOutputs)) - for jobID, outputs := range jobOutputs { - jobsCtx[jobID] = &model.WorkflowCallResult{Outputs: outputs} - } - - // See `on.workflow_call.outputs..value` in https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#context-availability - env := &exprparser.EvaluationEnvironment{ - Github: gitCtx, - Jobs: &jobsCtx, - Vars: vars, - Inputs: inputs, - } - evaluator := expreval.New(exprparser.NewInterpeter(env, exprparser.Config{}).Evaluate) - - out := make(map[string]string, len(spec.Outputs)) - for name, o := range spec.Outputs { - v, err := evaluator.Interpolate(o.Value) - if err != nil { - return nil, fmt.Errorf("workflow_call output %q: %w", name, err) - } - out[name] = v - } - return out, nil -} diff --git a/modules/actions/jobparser/workflow_call_test.go b/modules/actions/jobparser/workflow_call_test.go index 9b383b4df03..a4fe0145bed 100644 --- a/modules/actions/jobparser/workflow_call_test.go +++ b/modules/actions/jobparser/workflow_call_test.go @@ -4,7 +4,6 @@ package jobparser import ( - "maps" "testing" "gitea.dev/actionslib/pkg/model" @@ -14,235 +13,51 @@ import ( "go.yaml.in/yaml/v4" ) -func TestParseWorkflowCallSpec(t *testing.T) { - t.Run("malformed YAML surfaces a parse error", func(t *testing.T) { - // Mismatched flow-sequence brackets — yaml.Unmarshal must reject this. - _, err := ParseWorkflowCallSpec([]byte(`name: bad -on: [workflow_call -jobs: - noop: { } -`)) - require.Error(t, err) - }) - - t.Run("workflow without on.workflow_call is rejected", func(t *testing.T) { - notCallable := []byte(`name: ordinary -on: push -jobs: - noop: - runs-on: ubuntu-latest - steps: - - run: echo -`) - _, err := ParseWorkflowCallSpec(notCallable) - require.Error(t, err) - assert.Contains(t, err.Error(), "does not declare on.workflow_call") - }) - - t.Run("input missing the required type field is rejected", func(t *testing.T) { - content := callableWorkflow(t, `inputs: - x: - description: missing type -`) - _, err := ParseWorkflowCallSpec(content) - require.Error(t, err) - assert.Contains(t, err.Error(), `missing required field "type"`) - }) - - t.Run("inputs/secrets/outputs are decoded", func(t *testing.T) { - content := callableWorkflow(t, `inputs: - env: - type: string - required: true - secrets: - DEPLOY_KEY: - required: true - outputs: - sha: - value: ${{ jobs.build.outputs.commit }} -`) - spec, err := ParseWorkflowCallSpec(content) - require.NoError(t, err) - assert.Equal(t, InputTypeString, spec.Inputs["env"].Type) - assert.True(t, spec.Inputs["env"].Required) - assert.True(t, spec.Secrets["DEPLOY_KEY"].Required) - assert.Equal(t, "${{ jobs.build.outputs.commit }}", spec.Outputs["sha"].Value) - }) -} - -func TestEvaluateCallerWith(t *testing.T) { - t.Run("empty with: returns empty map", func(t *testing.T) { - out, err := EvaluateCallerWith("caller", &Job{}, nil, callerResults("caller", nil, nil), nil, nil) - require.NoError(t, err) - assert.Empty(t, out) - }) - - t.Run("non-string raw values pass through unchanged", func(t *testing.T) { - job := &Job{With: map[string]any{ - "already_bool": true, - "already_int": 42, - "already_slice": []any{"a", "b"}, - }} - out, err := EvaluateCallerWith("caller", job, nil, callerResults("caller", nil, nil), nil, nil) - require.NoError(t, err) - assert.Equal(t, true, out["already_bool"]) - assert.Equal(t, 42, out["already_int"]) - assert.Equal(t, []any{"a", "b"}, out["already_slice"]) - }) - - t.Run("expressions resolve against vars/inputs/results", func(t *testing.T) { - job := &Job{With: map[string]any{ - "env_name": "${{ vars.ENV }}", - "from_inputs": "${{ inputs.PARENT_VAR }}", - "from_needs": "${{ needs.upstream.outputs.commit }}", - }} - gitCtx := map[string]any{"event": map[string]any{}} - results := callerResults("caller", []string{"upstream"}, map[string]*JobResult{ - "upstream": {Result: "success", Outputs: map[string]string{"commit": "abc123"}}, - }) - vars := map[string]string{"ENV": "staging"} - inputs := map[string]any{"PARENT_VAR": "from-parent"} - out, err := EvaluateCallerWith("caller", job, gitCtx, results, vars, inputs) - require.NoError(t, err) - assert.Equal(t, "staging", out["env_name"]) - assert.Equal(t, "from-parent", out["from_inputs"]) - assert.Equal(t, "abc123", out["from_needs"]) - }) - - t.Run("matrix.X resolves to this caller row's matrix instance", func(t *testing.T) { - var rawMatrix yaml.Node - require.NoError(t, rawMatrix.Encode(map[string][]any{"target": {"staging"}})) - job := &Job{ - With: map[string]any{"env": "${{ matrix.target }}"}, - Strategy: Strategy{RawMatrix: rawMatrix}, - } - out, err := EvaluateCallerWith("caller", job, nil, callerResults("caller", nil, nil), nil, nil) - require.NoError(t, err) - assert.Equal(t, "staging", out["env"]) - }) -} - -func TestMatchCallerInputsAgainstSpec(t *testing.T) { - // mustParseSpec wraps ParseWorkflowCallSpec for test brevity. - mustParseSpec := func(t *testing.T, content []byte) *WorkflowCallSpec { - t.Helper() - spec, err := ParseWorkflowCallSpec(content) - require.NoError(t, err) - return spec +func TestResolveCallerInputs(t *testing.T) { + config := &model.WorkflowCall{Inputs: map[string]model.WorkflowCallInput{ + "env": {Type: "string"}, + "from_inputs": {Type: "string"}, + "from_needs": {Type: "string"}, + "count": {Type: "number"}, + "index": {Type: "number"}, + }} + results := map[string]*JobResult{ + "caller": {Needs: []string{"upstream"}}, + "upstream": {Result: "success", Outputs: map[string]string{"commit": "abc123"}}, } + var job Job + require.NoError(t, yaml.Unmarshal([]byte(`strategy: {matrix: {target: [staging]}, job-index: 1, job-total: 2} +with: {env: "${{ matrix.target }}", from_inputs: "${{ inputs.PARENT_VAR }}", from_needs: "${{ needs.upstream.outputs.commit }}", count: 42, index: "${{ strategy.job-index }}"}`), &job)) + out, err := ResolveCallerInputs("caller", &job, config, map[string]any{"event": map[string]any{}}, results, + nil, map[string]any{"PARENT_VAR": "from-parent"}) + require.NoError(t, err) + assert.Equal(t, map[string]any{ + "env": "staging", "from_inputs": "from-parent", "from_needs": "abc123", "count": 42.0, "index": 1.0, + }, out) - t.Run("default is filled when caller does not provide the input", func(t *testing.T) { - spec := mustParseSpec(t, callableWorkflow(t, `inputs: - greeting: - type: string - default: hi -`)) - out, err := MatchCallerInputsAgainstSpec(spec, nil) + for _, tt := range []struct { + with string + inputs map[string]model.WorkflowCallInput + want map[string]any + wantErr bool + }{ + {with: "with: {EXTRA: value}", want: map[string]any{}}, + {with: `with: ${{ fromJSON('{"env":"prod"}') }}`, wantErr: true}, + {inputs: map[string]model.WorkflowCallInput{"required": {Type: "string", Required: true, Default: "fallback"}}, want: map[string]any{"required": "fallback"}}, + {inputs: map[string]model.WorkflowCallInput{"derived": {Type: "string", Default: "${{ inputs.PARENT_VAR }}"}}, want: map[string]any{"derived": "from-parent"}}, + {with: "with: {value: 'false'}", inputs: map[string]model.WorkflowCallInput{"value": {Type: "boolean"}}, wantErr: true}, + {with: `with: {value: "${{ '5' }}"}`, inputs: map[string]model.WorkflowCallInput{"value": {Type: "number"}}, wantErr: true}, + } { + job = Job{} + require.NoError(t, yaml.Unmarshal([]byte(tt.with), &job)) + out, err := ResolveCallerInputs("caller", &job, &model.WorkflowCall{Inputs: tt.inputs}, nil, nil, nil, map[string]any{"PARENT_VAR": "from-parent"}) + if tt.wantErr { + require.Error(t, err, tt.with) + continue + } require.NoError(t, err) - assert.Equal(t, map[string]any{"greeting": "hi"}, out) - }) - - t.Run("caller-provided value wins over default", func(t *testing.T) { - spec := mustParseSpec(t, callableWorkflow(t, `inputs: - greeting: - type: string - default: hi -`)) - out, err := MatchCallerInputsAgainstSpec(spec, map[string]any{"greeting": "hello"}) - require.NoError(t, err) - assert.Equal(t, map[string]any{"greeting": "hello"}, out) - }) - - t.Run("required input must be provided", func(t *testing.T) { - spec := mustParseSpec(t, callableWorkflow(t, `inputs: - target: - type: string - required: true -`)) - _, err := MatchCallerInputsAgainstSpec(spec, nil) - require.Error(t, err) - assert.Contains(t, err.Error(), `"target" is required`) - }) - - t.Run("required input is satisfied by a default value", func(t *testing.T) { - spec := mustParseSpec(t, callableWorkflow(t, `inputs: - target: - type: string - required: true - default: prod -`)) - out, err := MatchCallerInputsAgainstSpec(spec, nil) - require.NoError(t, err) - assert.Equal(t, map[string]any{"target": "prod"}, out) - }) - - t.Run("boolean inputs accept native bool values and bool defaults", func(t *testing.T) { - spec := mustParseSpec(t, callableWorkflow(t, `inputs: - flag1: - type: boolean - flag2: - type: boolean - default: true - flag3: - type: boolean -`)) - out, err := MatchCallerInputsAgainstSpec(spec, map[string]any{ - "flag1": true, - "flag3": false, - }) - require.NoError(t, err) - assert.Equal(t, true, out["flag1"]) - assert.Equal(t, true, out["flag2"]) // from default - assert.Equal(t, false, out["flag3"]) - }) - - t.Run("boolean input rejects strings", func(t *testing.T) { - spec := mustParseSpec(t, callableWorkflow(t, `inputs: - flag: - type: boolean -`)) - _, err := MatchCallerInputsAgainstSpec(spec, map[string]any{"flag": "true"}) - require.Error(t, err) - assert.Contains(t, err.Error(), "expects boolean") - }) - - t.Run("number inputs accept native numeric values and number defaults", func(t *testing.T) { - spec := mustParseSpec(t, callableWorkflow(t, `inputs: - count: - type: number - ratio: - type: number - default: 0.5 -`)) - out, err := MatchCallerInputsAgainstSpec(spec, map[string]any{"count": 42}) - require.NoError(t, err) - assert.InDelta(t, 42.0, out["count"], 0) - assert.InDelta(t, 0.5, out["ratio"], 0) - }) - - t.Run("number input rejects strings", func(t *testing.T) { - spec := mustParseSpec(t, callableWorkflow(t, `inputs: - count: - type: number -`)) - _, err := MatchCallerInputsAgainstSpec(spec, map[string]any{"count": "42"}) - require.Error(t, err) - assert.Contains(t, err.Error(), "expects number") - }) - - t.Run("unknown caller-with key is silently dropped", func(t *testing.T) { - spec := mustParseSpec(t, callableWorkflow(t, `inputs: - known: - type: string - default: ok -`)) - out, err := MatchCallerInputsAgainstSpec(spec, map[string]any{ - "known": "yes", - "unknown": "ignored", - }) - require.NoError(t, err) - assert.Equal(t, map[string]any{"known": "yes"}, out) - }) + assert.Equal(t, tt.want, out) + } } func TestParseCallerSecrets(t *testing.T) { @@ -312,161 +127,3 @@ deploy_key: ${{ secrets.gitea_deploy_key }} assert.Contains(t, err.Error(), `must be of the form ${{ secrets.NAME }}`) }) } - -func TestValidateCallerSecrets(t *testing.T) { - specWith := func(secrets map[string]SecretSpec) *WorkflowCallSpec { - return &WorkflowCallSpec{Secrets: secrets} - } - - t.Run("explicit mapping with all required + only declared aliases is accepted", func(t *testing.T) { - spec := specWith(map[string]SecretSpec{ - "DEPLOY_KEY": {Required: true}, - "OPTIONAL": {}, - }) - mapping := map[string]string{ - "DEPLOY_KEY": "PROD_DEPLOY_KEY", - "OPTIONAL": "SOMETHING_ELSE", - } - require.NoError(t, ValidateCallerSecrets(spec, mapping)) - }) - - t.Run("alias not in callee schema is rejected", func(t *testing.T) { - spec := specWith(map[string]SecretSpec{"DEPLOY_KEY": {}}) - mapping := map[string]string{ - "DEPLOY_KEY": "PROD_DEPLOY_KEY", - "EXTRA": "SOMETHING_NOT_DECLARED", - } - err := ValidateCallerSecrets(spec, mapping) - require.Error(t, err) - assert.Contains(t, err.Error(), `caller secret "EXTRA"`) - assert.Contains(t, err.Error(), `not declared`) - }) - - t.Run("missing required secret is rejected", func(t *testing.T) { - spec := specWith(map[string]SecretSpec{ - "MUST_HAVE": {Required: true}, - "OPTIONAL": {}, - }) - mapping := map[string]string{"OPTIONAL": "X"} - err := ValidateCallerSecrets(spec, mapping) - require.Error(t, err) - assert.Contains(t, err.Error(), `required secret "MUST_HAVE"`) - assert.Contains(t, err.Error(), `not provided`) - }) - - t.Run("callee with no secrets schema accepts an empty mapping", func(t *testing.T) { - spec := specWith(map[string]SecretSpec{}) - require.NoError(t, ValidateCallerSecrets(spec, nil)) - require.NoError(t, ValidateCallerSecrets(spec, map[string]string{})) - }) - - t.Run("callee with no secrets schema rejects a non-empty mapping", func(t *testing.T) { - spec := specWith(map[string]SecretSpec{}) - err := ValidateCallerSecrets(spec, map[string]string{"X": "Y"}) - require.Error(t, err) - assert.Contains(t, err.Error(), `caller secret "X"`) - }) - - t.Run("name matching is case-insensitive", func(t *testing.T) { - // declared name and caller alias differ only in case; both should match. - spec := specWith(map[string]SecretSpec{"deploy_key": {Required: true}}) - mapping := map[string]string{"DEPLOY_KEY": "PROD_DEPLOY_KEY"} - require.NoError(t, ValidateCallerSecrets(spec, mapping)) - }) - - t.Run("nil spec is rejected", func(t *testing.T) { - err := ValidateCallerSecrets(nil, map[string]string{"X": "Y"}) - require.Error(t, err) - assert.Contains(t, err.Error(), "nil workflow_call spec") - }) -} - -func TestEvaluateWorkflowCallOutputs(t *testing.T) { - t.Run("nil spec returns empty map", func(t *testing.T) { - out, err := EvaluateWorkflowCallOutputs(nil, &model.GithubContext{}, nil, nil, nil) - require.NoError(t, err) - assert.Empty(t, out) - }) - - t.Run("spec with no outputs returns empty map", func(t *testing.T) { - spec := &WorkflowCallSpec{Outputs: map[string]OutputSpec{}} - out, err := EvaluateWorkflowCallOutputs(spec, &model.GithubContext{}, nil, nil, nil) - require.NoError(t, err) - assert.Empty(t, out) - }) - - t.Run("plain string value passes through unchanged", func(t *testing.T) { - spec := &WorkflowCallSpec{Outputs: map[string]OutputSpec{ - "name": {Value: "static-value"}, - }} - out, err := EvaluateWorkflowCallOutputs(spec, &model.GithubContext{}, nil, nil, nil) - require.NoError(t, err) - assert.Equal(t, map[string]string{"name": "static-value"}, out) - }) - - t.Run("output references jobs..outputs.", func(t *testing.T) { - spec := &WorkflowCallSpec{Outputs: map[string]OutputSpec{ - "sha": {Value: "${{ jobs.build.outputs.commit }}"}, - }} - jobOutputs := JobOutputs{ - "build": {"commit": "deadbeef"}, - } - out, err := EvaluateWorkflowCallOutputs(spec, &model.GithubContext{}, nil, nil, jobOutputs) - require.NoError(t, err) - assert.Equal(t, "deadbeef", out["sha"]) - }) - - t.Run("output references inputs.", func(t *testing.T) { - spec := &WorkflowCallSpec{Outputs: map[string]OutputSpec{ - "target": {Value: "${{ inputs.env_name }}"}, - }} - inputs := map[string]any{"env_name": "staging"} - out, err := EvaluateWorkflowCallOutputs(spec, &model.GithubContext{}, nil, inputs, nil) - require.NoError(t, err) - assert.Equal(t, "staging", out["target"]) - }) - - t.Run("multiple outputs are all evaluated", func(t *testing.T) { - spec := &WorkflowCallSpec{Outputs: map[string]OutputSpec{ - "static": {Value: "static-value"}, - "dynamic": {Value: "${{ vars.SUFFIX }}"}, - }} - vars := map[string]string{"SUFFIX": "abc"} - out, err := EvaluateWorkflowCallOutputs(spec, &model.GithubContext{}, vars, nil, nil) - require.NoError(t, err) - assert.Equal(t, "static-value", out["static"]) - assert.Equal(t, "abc", out["dynamic"]) - }) - - t.Run("expression referencing an undefined symbol surfaces an error", func(t *testing.T) { - spec := &WorkflowCallSpec{Outputs: map[string]OutputSpec{ - "bad": {Value: "${{ this.is.not.valid() }}"}, - }} - _, err := EvaluateWorkflowCallOutputs(spec, &model.GithubContext{}, nil, nil, nil) - require.Error(t, err) - assert.Contains(t, err.Error(), `output "bad"`) - }) -} - -// callableWorkflow returns a minimal valid called-workflow YAML with on.workflow_call. -func callableWorkflow(t *testing.T, body string) []byte { - t.Helper() - return []byte(`name: callable -on: - workflow_call: - ` + body + ` -jobs: - noop: - runs-on: ubuntu-latest - steps: - - run: "echo" -`) -} - -// callerResults returns the minimum results map shape that NewInterpeter expects -func callerResults(callerJobID string, callerNeeds []string, deps map[string]*JobResult) map[string]*JobResult { - out := make(map[string]*JobResult, len(deps)+1) - maps.Copy(out, deps) - out[callerJobID] = &JobResult{Needs: callerNeeds} - return out -} diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go index 2a1535faa35..0e14df432e6 100644 --- a/modules/actions/workflows.go +++ b/modules/actions/workflows.go @@ -4,6 +4,7 @@ package actions import ( + "bytes" "context" "fmt" "path" @@ -120,19 +121,19 @@ func GetContentFromEntry(ctx context.Context, gitRepo *git.Repository, entry *gi } func GetEventsFromContent(content []byte) ([]*jobparser.Event, error) { - workflow, err := jobparser.ReadWorkflow(content) - if err != nil { - return nil, err - } - events, err := jobparser.ParseRawOn(&workflow.RawOn) - if err != nil { - return nil, err - } - if err := ValidateWorkflowContent(content); err != nil { - return nil, err - } + events, _, err := readWorkflowEvents(content) + return events, err +} - return events, nil +// readWorkflowEvents also reports whether its error needs no run-time values to find, as a parse error of a workflow without expressions does. +func readWorkflowEvents(content []byte) (events []*jobparser.Event, static bool, err error) { + if events, err = jobparser.ValidateWorkflowStatic(content); err != nil { + return nil, true, err + } + if err = ValidateWorkflowContent(content); err != nil { + return nil, !bytes.Contains(content, []byte("${{")), err + } + return events, false, nil } // ValidateWorkflowContent catches structural errors (e.g. blank lines in run: | blocks) @@ -183,22 +184,26 @@ func DetectWorkflows( triggedEvent webhook_module.HookEventType, payload api.Payloader, detectSchedule bool, -) (workflows, schedules, filtered []*DetectedWorkflow, err error) { +) (workflows, schedules, filtered []*DetectedWorkflow, invalid map[string]error, err error) { _, entries, err := ListWorkflows(ctx, gitRepo, commit) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } + invalid = map[string]error{} for _, entry := range entries { content, err := GetContentFromEntry(ctx, gitRepo, entry) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } // one workflow may have multiple events - events, err := GetEventsFromContent(content) + events, static, err := readWorkflowEvents(content) if err != nil { log.Warn("ignore invalid workflow %q: %v", entry.Name(), err) + if static { + invalid[entry.Name()] = err + } continue } for _, evt := range events { @@ -231,7 +236,7 @@ func DetectWorkflows( } } - return workflows, schedules, filtered, nil + return workflows, schedules, filtered, invalid, nil } func DetectScheduledWorkflows(ctx context.Context, gitRepo *git.Repository, commit *git.Commit) ([]*DetectedWorkflow, error) { diff --git a/modules/actions/workflows_test.go b/modules/actions/workflows_test.go index 190d7e15951..27a9d6e35bd 100644 --- a/modules/actions/workflows_test.go +++ b/modules/actions/workflows_test.go @@ -13,6 +13,7 @@ import ( webhook_module "gitea.dev/modules/webhook" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func fullWorkflowContent(part string) []byte { @@ -27,6 +28,25 @@ jobs: `) } +func TestReadWorkflowEventsStaticErrors(t *testing.T) { + for content, static := range map[string]bool{ + "on: push\njobs: {}": true, + "on: push\njobs: {test: {needs: absent}}": true, + "on: push\njobs: {one: {needs: two}, two: {needs: one}}": true, + "on: push\njobs: {test: {strategy: {matrix: {os: []}}}}": true, + "on: push\nrun-name: ${{ secrets.TOKEN }}\njobs: {test: {}}": true, + "on: push\nrun-name: ${{ fromJSON(inputs.x) }}\njobs: {test: {steps: [{run: echo}]}}": false, + } { + _, gotStatic, err := readWorkflowEvents([]byte(content)) + require.Error(t, err, content) + assert.Equal(t, static, gotStatic, content) + } + for _, content := range []string{"on: push\njobs: {test: {steps: [{run: echo}]}}", "on: push\nrun-name: ${{ github.ref }}\njobs: {test: {}}"} { + _, _, err := readWorkflowEvents([]byte(content)) + assert.NoError(t, err, content) + } +} + func TestIsWorkflow(t *testing.T) { defer test.MockVariableValue(&setting.Actions.WorkflowDirs)() diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index 6995b9c1a83..56dc8955ab4 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -33,8 +33,6 @@ import ( actions_service "gitea.dev/services/actions" "gitea.dev/services/context" "gitea.dev/services/convert" - - "go.yaml.in/yaml/v4" ) const ( @@ -226,7 +224,7 @@ func prepareWorkflowTemplate(ctx *context.Context, commit *git.Commit) (workflow workflows = append(workflows, workflow) continue } - if err := actions.ValidateWorkflowContent(content); err != nil { + if _, err := actions.GetEventsFromContent(content); err != nil { workflow.ErrMsg = ctx.Locale.TrString("actions.runs.invalid_workflow_helper", err.Error()) workflows = append(workflows, workflow) continue @@ -441,12 +439,13 @@ func prepareWorkflowDispatchTemplate(ctx *context.Context, workflowInfos []Workf } ctx.Data["CurWorkflowExists"] = true - curWfDispatchCfg := workflowDispatchConfig(curWorkflow) + curWfDispatchCfg := curWorkflow.WorkflowDispatchConfig() if curWfDispatchCfg == nil { return } ctx.Data["WorkflowDispatchConfig"] = curWfDispatchCfg + ctx.Data["WorkflowDispatchInputs"] = curWorkflow.WorkflowDispatchInputs() branchOpts := git_model.FindBranchOptions{ RepoID: ctx.Repo.Repository.ID, @@ -750,91 +749,6 @@ func loadIsRefDeleted(ctx stdCtx.Context, repoID int64, runs actions_model.RunLi return nil } -type WorkflowDispatchInput struct { - Name string `yaml:"name"` - Description string `yaml:"description"` - Required bool `yaml:"required"` - Default string `yaml:"default"` - Type string `yaml:"type"` - Options []string `yaml:"options"` -} - -func (i WorkflowDispatchInput) IsDefaultTrue() bool { - return util.ParseYamlBool(i.Default) -} - -type WorkflowDispatch struct { - Inputs []WorkflowDispatchInput -} - -func workflowDispatchConfig(w *act_model.Workflow) *WorkflowDispatch { - switch w.RawOn.Kind { - case yaml.ScalarNode: - var val string - if !decodeNode(w.RawOn, &val) { - return nil - } - if val == "workflow_dispatch" { - return &WorkflowDispatch{} - } - case yaml.SequenceNode: - var val []string - if !decodeNode(w.RawOn, &val) { - return nil - } - if slices.Contains(val, "workflow_dispatch") { - return &WorkflowDispatch{} - } - case yaml.MappingNode: - var val map[string]yaml.Node - if !decodeNode(w.RawOn, &val) { - return nil - } - - workflowDispatchNode, found := val["workflow_dispatch"] - if !found { - return nil - } - - var workflowDispatch WorkflowDispatch - var workflowDispatchVal map[string]yaml.Node - if !decodeNode(workflowDispatchNode, &workflowDispatchVal) { - return &workflowDispatch - } - - inputsNode, found := workflowDispatchVal["inputs"] - if !found || inputsNode.Kind != yaml.MappingNode { - return &workflowDispatch - } - - i := 0 - for { - if i+1 >= len(inputsNode.Content) { - break - } - var input WorkflowDispatchInput - if decodeNode(*inputsNode.Content[i+1], &input) { - input.Name = inputsNode.Content[i].Value - workflowDispatch.Inputs = append(workflowDispatch.Inputs, input) - } - i += 2 - } - return &workflowDispatch - - default: - return nil - } - return nil -} - -func decodeNode(node yaml.Node, out any) bool { - if err := node.Decode(out); err != nil { - log.Warn("Failed to decode node %v into %T: %v", node, out, err) - return false - } - return true -} - func actionsListRedirectURL(repoLink, workflow, scopedWorkflowSourceRepoID, actor, status, branch string) string { return fmt.Sprintf("%s/actions?workflow=%s&scoped_workflow_source_repo_id=%s&actor=%s&status=%s&branch=%s", repoLink, diff --git a/routers/web/repo/actions/actions_test.go b/routers/web/repo/actions/actions_test.go index 5fcf59ada50..7d4bebf0073 100644 --- a/routers/web/repo/actions/actions_test.go +++ b/routers/web/repo/actions/actions_test.go @@ -6,10 +6,8 @@ package actions import ( "net/http" "net/http/httptest" - "strings" "testing" - act_model "gitea.dev/actionslib/pkg/model" actions_model "gitea.dev/models/actions" "gitea.dev/models/db" repo_model "gitea.dev/models/repo" @@ -21,150 +19,6 @@ import ( "github.com/stretchr/testify/assert" ) -func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) { - yaml := ` - name: local-action-docker-url - ` - workflow, err := act_model.ReadWorkflow(strings.NewReader(yaml)) - assert.NoError(t, err, "read workflow should succeed") - workflowDispatch := workflowDispatchConfig(workflow) - assert.Nil(t, workflowDispatch) - - yaml = ` - name: local-action-docker-url - on: push - ` - workflow, err = act_model.ReadWorkflow(strings.NewReader(yaml)) - assert.NoError(t, err, "read workflow should succeed") - workflowDispatch = workflowDispatchConfig(workflow) - assert.Nil(t, workflowDispatch) - - yaml = ` - name: local-action-docker-url - on: workflow_dispatch - ` - workflow, err = act_model.ReadWorkflow(strings.NewReader(yaml)) - assert.NoError(t, err, "read workflow should succeed") - workflowDispatch = workflowDispatchConfig(workflow) - assert.NotNil(t, workflowDispatch) - assert.Nil(t, workflowDispatch.Inputs) - - yaml = ` - name: local-action-docker-url - on: [push, pull_request] - ` - workflow, err = act_model.ReadWorkflow(strings.NewReader(yaml)) - assert.NoError(t, err, "read workflow should succeed") - workflowDispatch = workflowDispatchConfig(workflow) - assert.Nil(t, workflowDispatch) - - yaml = ` - name: local-action-docker-url - on: - push: - pull_request: - ` - workflow, err = act_model.ReadWorkflow(strings.NewReader(yaml)) - assert.NoError(t, err, "read workflow should succeed") - workflowDispatch = workflowDispatchConfig(workflow) - assert.Nil(t, workflowDispatch) - - yaml = ` - name: local-action-docker-url - on: [push, workflow_dispatch] - ` - workflow, err = act_model.ReadWorkflow(strings.NewReader(yaml)) - assert.NoError(t, err, "read workflow should succeed") - workflowDispatch = workflowDispatchConfig(workflow) - assert.NotNil(t, workflowDispatch) - assert.Nil(t, workflowDispatch.Inputs) - - yaml = ` - name: local-action-docker-url - on: - - push - - workflow_dispatch - ` - workflow, err = act_model.ReadWorkflow(strings.NewReader(yaml)) - assert.NoError(t, err, "read workflow should succeed") - workflowDispatch = workflowDispatchConfig(workflow) - assert.NotNil(t, workflowDispatch) - assert.Nil(t, workflowDispatch.Inputs) - - yaml = ` - name: local-action-docker-url - on: - push: - pull_request: - workflow_dispatch: - inputs: - ` - workflow, err = act_model.ReadWorkflow(strings.NewReader(yaml)) - assert.NoError(t, err, "read workflow should succeed") - workflowDispatch = workflowDispatchConfig(workflow) - assert.NotNil(t, workflowDispatch) - assert.Nil(t, workflowDispatch.Inputs) - - yaml = ` - name: local-action-docker-url - on: - push: - pull_request: - workflow_dispatch: - inputs: - logLevel: - description: 'Log level' - required: true - default: 'warning' - type: choice - options: - - info - - warning - - debug - boolean_default_true: - description: 'Test scenario tags' - required: true - type: boolean - default: true - boolean_default_false: - description: 'Test scenario tags' - required: true - type: boolean - default: false - ` - - workflow, err = act_model.ReadWorkflow(strings.NewReader(yaml)) - assert.NoError(t, err, "read workflow should succeed") - workflowDispatch = workflowDispatchConfig(workflow) - assert.NotNil(t, workflowDispatch) - assert.Equal(t, WorkflowDispatchInput{ - Name: "logLevel", - Default: "warning", - Description: "Log level", - Options: []string{ - "info", - "warning", - "debug", - }, - Required: true, - Type: "choice", - }, workflowDispatch.Inputs[0]) - assert.Equal(t, WorkflowDispatchInput{ - Name: "boolean_default_true", - Default: "true", - Description: "Test scenario tags", - Required: true, - Type: "boolean", - }, workflowDispatch.Inputs[1]) - assert.Equal(t, WorkflowDispatchInput{ - Name: "boolean_default_false", - Default: "false", - Description: "Test scenario tags", - Required: true, - Type: "boolean", - }, workflowDispatch.Inputs[2]) -} - func Test_loadIsRefDeleted(t *testing.T) { unittest.PrepareTestEnv(t) diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 55163cc94ee..13161f6a80c 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -594,7 +594,7 @@ func fillViewRunResponseSummary(ctx *context_module.Context, resp *ViewResponse, // Hide the Cancel button once a cancel is already in cancelling progress resp.State.Run.CanCancel = isLatestAttempt && !resp.State.Run.Done && !effectiveStatus.IsCancelling() && ctx.Repo.Permission.CanWrite(unit.TypeActions) resp.State.Run.CanApprove = isLatestAttempt && run.NeedApproval && ctx.Repo.Permission.CanWrite(unit.TypeActions) - resp.State.Run.CanRerun = isLatestAttempt && resp.State.Run.Done && ctx.Repo.Permission.CanWrite(unit.TypeActions) + resp.State.Run.CanRerun = isLatestAttempt && resp.State.Run.Done && len(jobs) > 0 && ctx.Repo.Permission.CanWrite(unit.TypeActions) resp.State.Run.CanDeleteArtifact = resp.State.Run.Done && ctx.Repo.Permission.CanWrite(unit.TypeActions) if resp.State.Run.CanRerun { for _, job := range jobs { @@ -682,7 +682,7 @@ func fillViewRunResponseSummary(ctx *context_module.Context, resp *ViewResponse, return } if len(summaries) > 0 { - jobNameByID := make(map[int64]string, len(jobs)) + jobNameByID := map[int64]string{0: run.WorkflowID} // a workflow-level summary, such as an invalid workflow file for _, j := range jobs { jobNameByID[j.ID] = j.Name } @@ -964,6 +964,10 @@ func Rerun(ctx *context_module.Context) { if !checkRunRerunAllowed(ctx, run) { return } + if len(jobs) == 0 { + ctx.JSONError(ctx.Locale.Tr("actions.runs.no_job")) + return + } currentJob, hasPathParam := findCurrentJobByPathParam(ctx, jobs) if hasPathParam && currentJob == nil { @@ -1097,7 +1101,7 @@ func getRunViewLink(run *actions_model.ActionRun, attempt *actions_model.ActionR } // getCurrentRunJobsByPathParam resolves the current run view context from path parameters, including the run, optional attempt, and jobs to render. -// Any error will be written to the ctx, empty jobs will also result in 404 error, then the return values are all nil. +// Any error will be written to the ctx, then the return values are all nil. func getCurrentRunJobsByPathParam(ctx *context_module.Context) (*actions_model.ActionRun, *actions_model.ActionRunAttempt, []*actions_model.ActionRunJob) { run := getCurrentRunByPathParam(ctx) if ctx.Written() { @@ -1164,10 +1168,6 @@ func getCurrentRunJobsByPathParam(ctx *context_module.Context) (*actions_model.A ctx.ServerError("get current jobs", err) return nil, nil, nil } - if len(jobs) == 0 { - ctx.NotFound(nil) - return nil, nil, nil - } jobs.SortMatrixGroupsByName() for _, job := range jobs { diff --git a/routers/web/repo/view_file.go b/routers/web/repo/view_file.go index dead532c74d..4e2db924030 100644 --- a/routers/web/repo/view_file.go +++ b/routers/web/repo/view_file.go @@ -186,7 +186,7 @@ func prepareFileView(ctx *context.Context, entry *git.TreeEntry) { if err != nil { log.Error("actions.GetContentFromEntry: %v", err) } - if workFlowErr := actions.ValidateWorkflowContent(content); workFlowErr != nil { + if _, workFlowErr := actions.GetEventsFromContent(content); workFlowErr != nil { ctx.Data["FileError"] = ctx.Locale.Tr("actions.runs.invalid_workflow_helper", workFlowErr.Error()) } } else if issue_service.IsCodeOwnerFile(ctx.Repo.TreePath) { diff --git a/routers/web/shared/actions/scoped_workflows.go b/routers/web/shared/actions/scoped_workflows.go index 14d4efa7c64..db5100dba58 100644 --- a/routers/web/shared/actions/scoped_workflows.go +++ b/routers/web/shared/actions/scoped_workflows.go @@ -181,7 +181,7 @@ func deriveScopedStatusContexts(prefix, displayName string, content []byte, even if job == nil { continue } - jobName := util.EllipsisDisplayString(job.Name, 255) // run creation truncates job names the same way + jobName := job.DisplayName() for _, ev := range eventNames { ctxName := actions_module.ScopedWorkflowStatusContextName(prefix, displayName, jobName, ev) if seen.Contains(ctxName) { diff --git a/routers/web/shared/actions/scoped_workflows_test.go b/routers/web/shared/actions/scoped_workflows_test.go index 7d96c431ee5..a51861bdc68 100644 --- a/routers/web/shared/actions/scoped_workflows_test.go +++ b/routers/web/shared/actions/scoped_workflows_test.go @@ -13,7 +13,7 @@ import ( ) func TestDeriveScopedStatusContexts(t *testing.T) { - t.Run("jobs x events; job name is its name: or its id", func(t *testing.T) { + t.Run("jobs x events; job name is its unescaped name: or its id", func(t *testing.T) { content := []byte(`name: CI on: [push, pull_request] jobs: @@ -26,6 +26,14 @@ jobs: runs-on: ubuntu-latest steps: - run: echo + test: + name: "${{ matrix.os }} ${{ '${{' }}" + strategy: + matrix: + os: [linux] + runs-on: ubuntu-latest + steps: + - run: echo `) events, err := actions_module.GetEventsFromContent(content) require.NoError(t, err) @@ -35,6 +43,8 @@ jobs: "org/src: CI / lint (pull_request)", "org/src: CI / Build It (push)", "org/src: CI / Build It (pull_request)", + "org/src: CI / linux ${{ (push)", + "org/src: CI / linux ${{ (pull_request)", }, got) }) diff --git a/services/actions/cancel.go b/services/actions/cancel.go index b0b4c52d63e..83b56bd392c 100644 --- a/services/actions/cancel.go +++ b/services/actions/cancel.go @@ -7,8 +7,10 @@ import ( "context" "fmt" + "gitea.dev/actionslib/pkg/expreval" actions_model "gitea.dev/models/actions" "gitea.dev/models/db" + "gitea.dev/modules/actions/jobparser" ) // CancelRun cancels a run's cancellable jobs and returns the run's post-cancellation state. @@ -26,7 +28,7 @@ func ForceCancelRun(ctx context.Context, run *actions_model.ActionRun, jobs []*a func cancelRun(ctx context.Context, run *actions_model.ActionRun, jobs []*actions_model.ActionRunJob, force bool) (*actions_model.ActionRun, error) { var updatedJobs []*actions_model.ActionRunJob if err := db.WithTx(ctx, func(ctx context.Context) (err error) { - updatedJobs, err = actions_model.CancelJobs(ctx, jobs, force) + updatedJobs, err = actions_model.CancelJobs(ctx, cancellableJobs(run, jobs, force), force) if err != nil { return fmt.Errorf("CancelJobs: %w", err) } @@ -52,3 +54,28 @@ func cancelRun(ctx context.Context, run *actions_model.ActionRun, jobs []*action } return reloaded, nil } + +func cancellableJobs(run *actions_model.ActionRun, jobs []*actions_model.ActionRunJob, force bool) []*actions_model.ActionRunJob { + if force || (run.Started.IsZero() && !run.Status.In(actions_model.StatusRunning, actions_model.StatusCancelling)) { + return jobs + } + toCancel := make([]*actions_model.ActionRunJob, 0, len(jobs)) + for _, job := range jobs { + if !runsAfterCancellation(job) { + toCancel = append(toCancel, job) + } + } + return toCancel +} + +func runsAfterCancellation(job *actions_model.ActionRunJob) bool { + if job.Status.IsDone() { + return false + } + parsed, err := job.ParseJob() + if err != nil || parsed.If.Value == "" { + return false + } + condition := jobparser.IfExpression(parsed.If.Value) + return expreval.CallsFunction(condition, "always") && !expreval.CallsFunction(condition, "cancelled") +} diff --git a/services/actions/clear_tasks.go b/services/actions/clear_tasks.go index 982108229e2..4f1bc8d61ae 100644 --- a/services/actions/clear_tasks.go +++ b/services/actions/clear_tasks.go @@ -67,7 +67,7 @@ func shouldBlockJobByConcurrency(ctx context.Context, job *actions_model.ActionR return true, nil } - if job.ConcurrencyGroup == "" || job.ConcurrencyCancel { + if job.ConcurrencyGroup == "" { return false, nil } @@ -97,7 +97,7 @@ func PrepareToStartJobWithConcurrency(ctx context.Context, job *actions_model.Ac } func shouldBlockRunByConcurrency(ctx context.Context, attempt *actions_model.ActionRunAttempt) (bool, error) { - if attempt.ConcurrencyGroup == "" || attempt.ConcurrencyCancel { + if attempt.ConcurrencyGroup == "" { return false, nil } diff --git a/services/actions/clear_tasks_test.go b/services/actions/clear_tasks_test.go index be7ff26602a..ab654f8985e 100644 --- a/services/actions/clear_tasks_test.go +++ b/services/actions/clear_tasks_test.go @@ -60,6 +60,31 @@ func createConflictingCancellingJob(t *testing.T, concurrencyGroup string, runIn return job } +func TestCancellableJobs(t *testing.T) { + jobs := []*actions_model.ActionRunJob{ + {ID: 1, JobID: "always", Status: actions_model.StatusRunning, WorkflowPayload: []byte(`jobs: {always: {if: "always() && needs.build.result == 'cancelled'"}}`)}, + {ID: 2, JobID: "ordinary", Status: actions_model.StatusBlocked, WorkflowPayload: []byte(`jobs: {ordinary: {}}`)}, + {ID: 3, JobID: "not-cancelled", Status: actions_model.StatusBlocked, WorkflowPayload: []byte(`jobs: {not-cancelled: {if: "always() && !cancelled()"}}`)}, + {ID: 4, JobID: "done", Status: actions_model.StatusSuccess, WorkflowPayload: []byte(`jobs: {done: {if: "${{ always() }}"}}`)}, + } + for _, test := range []struct { + name string + started timeutil.TimeStamp + status actions_model.Status + force bool + want []*actions_model.ActionRunJob + }{ + {name: "pending run", want: jobs}, + {name: "started run", started: 1, want: jobs[1:]}, + {name: "legacy running run", status: actions_model.StatusRunning, want: jobs[1:]}, + {name: "force cancellation", started: 1, force: true, want: jobs}, + } { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.want, cancellableJobs(&actions_model.ActionRun{Started: test.started, Status: test.status}, jobs, test.force)) + }) + } +} + func TestShouldBlockJobByConcurrency_CancellingJobBlocks(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) @@ -76,6 +101,10 @@ func TestShouldBlockJobByConcurrency_CancellingJobBlocks(t *testing.T) { shouldBlock, err := shouldBlockJobByConcurrency(t.Context(), job) require.NoError(t, err) assert.True(t, shouldBlock) + job.ConcurrencyCancel = true + shouldBlock, err = shouldBlockJobByConcurrency(t.Context(), job) + require.NoError(t, err) + assert.True(t, shouldBlock) } func TestShouldBlockRunByConcurrency_CancellingJobBlocks(t *testing.T) { @@ -92,6 +121,10 @@ func TestShouldBlockRunByConcurrency_CancellingJobBlocks(t *testing.T) { shouldBlock, err := shouldBlockRunByConcurrency(t.Context(), attempt) require.NoError(t, err) assert.True(t, shouldBlock) + attempt.ConcurrencyCancel = true + shouldBlock, err = shouldBlockRunByConcurrency(t.Context(), attempt) + require.NoError(t, err) + assert.True(t, shouldBlock) } // TestStopEndlessTasksSkipsCancelling verifies that a task running its post-cancel cleanup is not diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index debacde1177..fd5d794334d 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -236,7 +236,7 @@ func CreateSkippedCommitStatusForFilteredWorkflow(ctx context.Context, repo *rep if job == nil { continue } - jobName := util.EllipsisDisplayString(job.Name, 255) // run creation truncates job names the same way + jobName := job.DisplayName() ctxName := actions_module.WorkflowStatusContextName(displayName, jobName, statusEvent) if scopedPrefix != "" { ctxName = actions_module.ScopedWorkflowStatusContextName(scopedPrefix, displayName, jobName, statusEvent) diff --git a/services/actions/concurrency.go b/services/actions/concurrency.go index 3d9f860ff8d..ba66a84433f 100644 --- a/services/actions/concurrency.go +++ b/services/actions/concurrency.go @@ -11,6 +11,7 @@ import ( actions_model "gitea.dev/models/actions" "gitea.dev/modules/actions/jobparser" "gitea.dev/modules/setting" + "gitea.dev/modules/util" "go.yaml.in/yaml/v4" ) @@ -79,7 +80,7 @@ func EvaluateJobConcurrencyFillModel(ctx context.Context, run *actions_model.Act 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) + return util.NewInvalidArgumentErrorf("%v", err) } actionRunJob.IsConcurrencyEvaluated = true return nil diff --git a/services/actions/context.go b/services/actions/context.go index f9cccfbb598..7755ad3e5c2 100644 --- a/services/actions/context.go +++ b/services/actions/context.go @@ -4,10 +4,15 @@ package actions import ( + "cmp" "context" "fmt" + "maps" + "slices" "strconv" + "gitea.dev/actionslib/pkg/expreval" + "gitea.dev/actionslib/pkg/exprparser" "gitea.dev/actionslib/pkg/model" actions_model "gitea.dev/models/actions" "gitea.dev/models/db" @@ -96,6 +101,11 @@ func GenerateGiteaContext(ctx context.Context, run *actions_model.ActionRun, att "workflow": run.WorkflowID, // string, The name of the workflow. If the workflow file doesn't specify a name, the value of this property is the full path of the workflow file in the repository. "workspace": "", // string, The default working directory on the runner for steps, and the default location of your repository when using the checkout action. + "actor_id": strconv.FormatInt(run.TriggerUserID, 10), + "repository_id": strconv.FormatInt(run.RepoID, 10), + "repository_owner_id": strconv.FormatInt(run.Repo.OwnerID, 10), + "workflow_sha": run.WorkflowCommitSHA, + // additional contexts "gitea_default_actions_url": setting.Actions.DefaultActionsURL.URL(), } @@ -164,9 +174,9 @@ type TaskNeed struct { // FindTaskNeeds finds the `needs` for the task by the task's job. // Lookup is scoped to the same ParentJobID. -func FindTaskNeeds(ctx context.Context, job *actions_model.ActionRunJob) (map[string]*TaskNeed, error) { +func FindTaskNeeds(ctx context.Context, job *actions_model.ActionRunJob) (map[string]*TaskNeed, map[string][]*actions_model.ActionRunJob, error) { if len(job.Needs) == 0 { - return nil, nil //nolint:nilnil // return nil when the job has no needs + return nil, nil, nil } needs := container.SetOf(job.Needs...) @@ -178,7 +188,7 @@ func FindTaskNeeds(ctx context.Context, job *actions_model.ActionRunJob) (map[st jobs, err := db.Find[actions_model.ActionRunJob](ctx, findOpts) if err != nil { - return nil, fmt.Errorf("FindRunJobs: %w", err) + return nil, nil, fmt.Errorf("FindRunJobs: %w", err) } jobIDJobs := make(map[string][]*actions_model.ActionRunJob) @@ -199,9 +209,10 @@ func FindTaskNeeds(ctx context.Context, job *actions_model.ActionRunJob) (map[st if !needs.Contains(jobID) { continue } + sortJobsByCompletion(jobsWithSameID) var jobOutputs map[string]string for _, candidate := range jobsWithSameID { - if !candidate.Status.IsDone() { + if !candidate.Status.IsDone() || candidate.IsReusableCaller && candidate.Status != actions_model.StatusSuccess { continue } var outputs map[string]string @@ -212,7 +223,7 @@ func FindTaskNeeds(ctx context.Context, job *actions_model.ActionRunJob) (map[st outputs, err = loadJobTaskOutputs(ctx, candidate) } if err != nil { - return nil, err + return nil, nil, err } if len(jobOutputs) == 0 { jobOutputs = outputs @@ -225,7 +236,7 @@ func FindTaskNeeds(ctx context.Context, job *actions_model.ActionRunJob) (map[st Result: actions_model.AggregateJobStatus(jobsWithSameID), } } - return ret, nil + return ret, jobIDJobs, nil } // computeReusableCallerOutputs returns the workflow_call outputs of a reusable caller by recursing into its child subtree. @@ -240,7 +251,7 @@ func computeReusableCallerOutputs(ctx context.Context, caller *actions_model.Act if err := caller.LoadRun(ctx); err != nil { return nil, err } - wcSpec, err := jobparser.ParseWorkflowCallSpec(caller.ReusableWorkflowContent) + wcSpec, err := jobparser.ParseWorkflowCallConfig(caller.ReusableWorkflowContent) if err != nil { return nil, err } @@ -249,7 +260,8 @@ func computeReusableCallerOutputs(ctx context.Context, caller *actions_model.Act } // Per-job outputs over the children of this caller. - jobOutputs := make(jobparser.JobOutputs, len(directChildren)) + sortJobsByCompletion(directChildren) + jobOutputs := make(map[string]*model.WorkflowCallResult, len(directChildren)) for _, child := range directChildren { var outs map[string]string switch { @@ -262,10 +274,9 @@ func computeReusableCallerOutputs(ctx context.Context, caller *actions_model.Act return nil, err } if existing, ok := jobOutputs[child.JobID]; ok { - jobOutputs[child.JobID] = mergeTwoOutputs(outs, existing) - } else { - jobOutputs[child.JobID] = outs + outs = mergeTwoOutputs(outs, existing.Outputs) } + jobOutputs[child.JobID] = &model.WorkflowCallResult{Outputs: outs} } // build contexts for evaluating outputs @@ -288,7 +299,19 @@ func computeReusableCallerOutputs(ctx context.Context, caller *actions_model.Act } } - return jobparser.EvaluateWorkflowCallOutputs(wcSpec, gitCtx.ToGitHubContext(), vars, inputs, jobOutputs) + // See `on.workflow_call.outputs..value` in https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#context-availability + return expreval.New(exprparser.NewInterpeter(&exprparser.EvaluationEnvironment{ + Github: gitCtx.ToGitHubContext(), + Jobs: &jobOutputs, + Vars: vars, + Inputs: inputs, + }, exprparser.Config{}).Evaluate).EvaluateWorkflowCallOutputs(wcSpec) +} + +func sortJobsByCompletion(jobs []*actions_model.ActionRunJob) { + slices.SortFunc(jobs, func(left, right *actions_model.ActionRunJob) int { + return cmp.Or(cmp.Compare(left.Stopped, right.Stopped), cmp.Compare(left.ID, right.ID)) + }) } // loadJobTaskOutputs returns the task-output map of `job`. @@ -312,56 +335,18 @@ func loadJobTaskOutputs(ctx context.Context, job *actions_model.ActionRunJob) (m // Values with the same output name may be overridden. The user should ensure the output names are unique. // See https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#using-job-outputs-in-a-matrix-job func mergeTwoOutputs(o1, o2 map[string]string) map[string]string { - ret := make(map[string]string, len(o1)) + ret := make(map[string]string, len(o1)+len(o2)) + maps.Copy(ret, o2) for k1, v1 := range o1 { - if len(v1) > 0 { + if len(v1) > 0 || ret[k1] == "" { ret[k1] = v1 - } else { - ret[k1] = o2[k1] } } return ret } -func contextMapValueOrDefault[T any](m map[string]any, key string, defaultValue T) T { - if value, ok := m[key]; ok { - if v, ok := value.(T); ok { - return v - } - } - return defaultValue -} - func (g *GiteaContext) ToGitHubContext() *model.GithubContext { - return &model.GithubContext{ - Event: contextMapValueOrDefault(*g, "event", map[string]any(nil)), - EventPath: contextMapValueOrDefault(*g, "event_path", ""), - Workflow: contextMapValueOrDefault(*g, "workflow", ""), - RunID: contextMapValueOrDefault(*g, "run_id", ""), - RunNumber: contextMapValueOrDefault(*g, "run_number", ""), - Actor: contextMapValueOrDefault(*g, "actor", ""), - Repository: contextMapValueOrDefault(*g, "repository", ""), - EventName: contextMapValueOrDefault(*g, "event_name", ""), - Sha: contextMapValueOrDefault(*g, "sha", ""), - Ref: contextMapValueOrDefault(*g, "ref", ""), - RefName: contextMapValueOrDefault(*g, "ref_name", ""), - RefType: contextMapValueOrDefault(*g, "ref_type", ""), - HeadRef: contextMapValueOrDefault(*g, "head_ref", ""), - BaseRef: contextMapValueOrDefault(*g, "base_ref", ""), - Token: "", // deliberately omitted for security - Workspace: contextMapValueOrDefault(*g, "workspace", ""), - Action: contextMapValueOrDefault(*g, "action", ""), - ActionPath: contextMapValueOrDefault(*g, "action_path", ""), - ActionRef: contextMapValueOrDefault(*g, "action_ref", ""), - ActionRepository: contextMapValueOrDefault(*g, "action_repository", ""), - Job: contextMapValueOrDefault(*g, "job", ""), - JobName: "", // not present in GiteaContext - RepositoryOwner: contextMapValueOrDefault(*g, "repository_owner", ""), - RetentionDays: contextMapValueOrDefault(*g, "retention_days", ""), - RunnerPerflog: "", // not present in GiteaContext - RunnerTrackingID: "", // not present in GiteaContext - ServerURL: contextMapValueOrDefault(*g, "server_url", ""), - APIURL: contextMapValueOrDefault(*g, "api_url", ""), - GraphQLURL: contextMapValueOrDefault(*g, "graphql_url", ""), - } + githubCtx := model.GithubContextFromMap(*g) + githubCtx.Token = "" // deliberately omitted for security + return githubCtx } diff --git a/services/actions/context_test.go b/services/actions/context_test.go index b61a605f8f6..9049de2c523 100644 --- a/services/actions/context_test.go +++ b/services/actions/context_test.go @@ -293,20 +293,24 @@ func TestComputeReusableCallerOutputs(t *testing.T) { assert.Equal(t, map[string]string{"bubbled": "bubble-value"}, out) }) - t.Run("matrix children with same JobID prefer non-empty values", func(t *testing.T) { + t.Run("matrix children combine outputs by completion order while ignoring empty values", func(t *testing.T) { run := insertRun(t, "matrix-out.yaml") caller := insertCaller(t, run, "caller", 0, `on: workflow_call: outputs: foo: value: ${{ jobs.matrix.outputs.foo }} + bar: + value: ${{ jobs.matrix.outputs.bar }} `, "") - insertChildJobAndTask(t, run, "matrix", caller.ID, map[string]string{"foo": ""}) - insertChildJobAndTask(t, run, "matrix", caller.ID, map[string]string{"foo": "filled"}) + later := insertChildJobAndTask(t, run, "matrix", caller.ID, map[string]string{"foo": "latest", "bar": "kept"}) + earlier := insertChildJobAndTask(t, run, "matrix", caller.ID, map[string]string{"foo": "earlier"}) + empty := insertChildJobAndTask(t, run, "matrix", caller.ID, map[string]string{"foo": ""}) + later.Stopped, earlier.Stopped, empty.Stopped = 200, 100, 300 - out, err := computeReusableCallerOutputs(ctx, caller, childrenByParentOfRun(t, run.ID)) + out, err := computeReusableCallerOutputs(ctx, caller, map[int64][]*actions_model.ActionRunJob{caller.ID: {later, earlier, empty}}) require.NoError(t, err) - assert.Equal(t, map[string]string{"foo": "filled"}, out) + assert.Equal(t, map[string]string{"foo": "latest", "bar": "kept"}, out) }) } @@ -316,7 +320,7 @@ func TestFindTaskNeeds(t *testing.T) { task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 51}) job := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: task.JobID}) - ret, err := FindTaskNeeds(t.Context(), job) + ret, _, err := FindTaskNeeds(t.Context(), job) assert.NoError(t, err) assert.Len(t, ret, 1) assert.Contains(t, ret, "job1") diff --git a/services/actions/helper.go b/services/actions/helper.go index 9f963cb5a4e..2e3a9d504c6 100644 --- a/services/actions/helper.go +++ b/services/actions/helper.go @@ -96,18 +96,12 @@ func pullRequestTargetBaseSHA(run *actions_model.ActionRun) (string, bool) { func evaluateJobIf(ctx context.Context, run *actions_model.ActionRun, attempt *actions_model.ActionRunAttempt, job *actions_model.ActionRunJob, vars map[string]string, allNeedsSucceed bool) (bool, error) { parsedJob, err := job.ParseJob() if err != nil { - return false, err + return false, upsertJobErrorSummary(ctx, job, "if", err) } // Empty `if:` reduces to implicit `success()` - true iff every need finished as Success. if len(parsedJob.If.Value) == 0 { return allNeedsSucceed, nil } - // A deferred-matrix placeholder has no combination yet, so an `if:` reading `matrix.*` can only be - // decided by the emitter's post-expansion pass, against each combination's own values. - // always()/failure()/cancelled() opt out of the needs gate this falls back to. - if job.IsMatrixDeferred && jobparser.ExpressionReadsMatrix(parsedJob.If.Value) { - return allNeedsSucceed || jobparser.ExpressionIgnoresNeedResults(parsedJob.If.Value), nil - } jobResults, err := findJobNeedsAndFillJobResults(ctx, job) if err != nil { return false, err @@ -125,24 +119,53 @@ func evaluateJobIf(ctx context.Context, run *actions_model.ActionRun, attempt *a return false, err } gitCtx := GenerateGiteaContext(ctx, run, attempt, job) - return jobparser.EvaluateJobIfExpression(job.JobID, parsedJob, gitCtx, jobResults, vars, inputs, job.IsMatrixDeferred) + gitCtx["job"] = "" // github.com decides a job's `if:` before the job exists + shouldStart, err := jobparser.EvaluateJobIfExpression(job.JobID, parsedJob, gitCtx, jobResults, vars, inputs) + if err != nil { + return false, upsertJobErrorSummary(ctx, job, "if", err) + } + return shouldStart, nil +} + +func upsertJobErrorSummary(ctx context.Context, job *actions_model.ActionRunJob, key string, err error) error { + content := fmt.Sprintf("Error when evaluating `%s` for job `%s`.\n\n```\n%v\n```\n", key, job.JobID, err) + return actions_model.UpsertActionRunJobSummary(ctx, job.RepoID, job.RunID, job.RunAttemptID, job.ID, 0, actions_model.JobSummaryContentTypeMarkdown, []byte(content)) } func findJobNeedsAndFillJobResults(ctx context.Context, job *actions_model.ActionRunJob) (map[string]*jobparser.JobResult, error) { - taskNeeds, err := FindTaskNeeds(ctx, job) + taskNeeds, jobsByID, err := FindTaskNeeds(ctx, job) if err != nil { return nil, fmt.Errorf("find task needs: %w", err) } - jobResults := make(map[string]*jobparser.JobResult, len(taskNeeds)) + jobResults := make(map[string]*jobparser.JobResult, len(taskNeeds)+1) for jobID, taskNeed := range taskNeeds { - jobResult := &jobparser.JobResult{ + jobResults[jobID] = &jobparser.JobResult{ Result: taskNeed.Result.String(), Outputs: taskNeed.Outputs, } - jobResults[jobID] = jobResult } jobResults[job.JobID] = &jobparser.JobResult{ Needs: job.Needs, } + if len(job.Needs) == 0 { + return jobResults, nil + } + + queue := append([]string(nil), job.Needs...) + for len(queue) > 0 { + jobID := queue[0] + queue = queue[1:] + if len(jobsByID[jobID]) == 0 { + continue + } + if jobResults[jobID] == nil { + jobResults[jobID] = &jobparser.JobResult{Result: actions_model.AggregateJobStatus(jobsByID[jobID]).String()} + } + if jobResults[jobID].Needs != nil { + continue + } + jobResults[jobID].Needs = jobsByID[jobID][0].Needs + queue = append(queue, jobResults[jobID].Needs...) + } return jobResults, nil } diff --git a/services/actions/helper_test.go b/services/actions/helper_test.go index dae2a517bb6..5475775ed5b 100644 --- a/services/actions/helper_test.go +++ b/services/actions/helper_test.go @@ -4,7 +4,6 @@ package actions import ( - "fmt" "testing" actions_model "gitea.dev/models/actions" @@ -17,52 +16,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestEvaluateJobIfDefersMatrixExpression(t *testing.T) { - // A placeholder's `if:` reading `matrix.*` can only be decided per combination once the matrix is expanded. - - emptyRun := &actions_model.ActionRun{} // if the gate is removed, the emptyRun will cause an error - deferredJob := func(ifExpr string) *actions_model.ActionRunJob { - return &actions_model.ActionRunJob{ - ID: 1, JobID: "build", Needs: []string{"setup"}, IsMatrixDeferred: true, - WorkflowPayload: fmt.Appendf(nil, `name: test -on: push -jobs: - build: - runs-on: ubuntu-latest - if: %s - strategy: - matrix: - value: ${{ fromJson(needs.setup.outputs.m) }} - steps: - - run: echo -`, ifExpr), - } - } - - for _, tt := range []struct { - ifExpr string - // wantNeedsFailed is what the `if:` decides when the needs did not all succeed. - wantNeedsFailed bool - }{ - // These hold only for a real combination, so none can be decided before the matrix expands, - // and the fallback is the needs gate: a job whose needs did not succeed is still skipped. - {ifExpr: "${{ matrix.value == 1 }}"}, - {ifExpr: "matrix.value == 1"}, // an `if:` may omit the `${{ }}` - // always() asks to run whatever the needs did, so it must not fall back to their gate. - {ifExpr: "${{ always() && matrix.value == 1 }}", wantNeedsFailed: true}, - } { - t.Run(tt.ifExpr, func(t *testing.T) { - got, err := evaluateJobIf(t.Context(), emptyRun, nil, deferredJob(tt.ifExpr), nil, true) - require.NoError(t, err) - assert.True(t, got, "must reach the expansion that gates each combination on its own values") - - got, err = evaluateJobIf(t.Context(), emptyRun, nil, deferredJob(tt.ifExpr), nil, false) - require.NoError(t, err) - assert.Equal(t, tt.wantNeedsFailed, got) - }) - } -} - func TestDispatchInputsForRunJobs(t *testing.T) { // a child carries the callee's `on: workflow_call`, so only a top-level job answers for the run run := &actions_model.ActionRun{Event: "workflow_dispatch", EventPayload: `{"inputs":{"deploy":"true"}}`} diff --git a/services/actions/invalid_workflow.go b/services/actions/invalid_workflow.go new file mode 100644 index 00000000000..172de91b4d0 --- /dev/null +++ b/services/actions/invalid_workflow.go @@ -0,0 +1,71 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + "context" + "fmt" + + actions_model "gitea.dev/models/actions" + "gitea.dev/models/db" + "gitea.dev/models/unit" + "gitea.dev/modules/commitstatus" + "gitea.dev/modules/git" + "gitea.dev/modules/json" + "gitea.dev/modules/log" + "gitea.dev/modules/timeutil" + "gitea.dev/modules/util" +) + +func handleInvalidWorkflows(ctx context.Context, input *notifyInput, ref git.RefName, commit *git.Commit, invalid map[string]error) { + if len(invalid) == 0 { + return + } + payload, err := json.Marshal(input.Payload) + if err != nil { + log.Error("marshal event payload: %v", err) + return + } + actionsConfig := input.Repo.MustGetUnit(ctx, unit.TypeActions).ActionsConfig() + for entryName, parseErr := range invalid { + if actionsConfig.IsWorkflowDisabled(entryName) { + continue + } + now := timeutil.TimeStampNow() + run := &actions_model.ActionRun{ + Title: util.EllipsisDisplayString(commit.MessageTitle(), 255), RepoID: input.Repo.ID, Repo: input.Repo, OwnerID: input.Repo.OwnerID, + WorkflowID: entryName, TriggerUserID: input.Doer.ID, TriggerUser: input.Doer, Ref: ref.String(), + CommitSHA: commit.ID.String(), Event: input.Event, TriggerEvent: string(input.Event), EventPayload: string(payload), + WorkflowRepoID: input.Repo.ID, WorkflowCommitSHA: commit.ID.String(), Status: actions_model.StatusFailure, Started: now, Stopped: now, + } + if err := db.WithTx(ctx, func(ctx context.Context) error { + if run.Index, err = db.GetNextResourceIndex(ctx, "action_run_index", run.RepoID); err != nil { + return err + } + if err := db.Insert(ctx, run); err != nil { + return err + } + attempt := &actions_model.ActionRunAttempt{RepoID: run.RepoID, RunID: run.ID, Attempt: 1, TriggerUserID: run.TriggerUserID, Status: run.Status, Started: now, Stopped: now} + if err := db.Insert(ctx, attempt); err != nil { + return err + } + run.LatestAttemptID = attempt.ID + if err := actions_model.UpdateRun(ctx, run, "latest_attempt_id"); err != nil { + return err + } + content := fmt.Sprintf("**Invalid workflow file: %s**\n\n```\n%v\n```\n", entryName, parseErr) + return db.Insert(ctx, &actions_model.ActionRunJobSummary{ + RepoID: run.RepoID, RunID: run.ID, RunAttemptID: attempt.ID, Content: content, ContentSize: int64(len(content)), ContentType: actions_model.JobSummaryContentTypeMarkdown, + }) + }); err != nil { + log.Error("insert run for invalid workflow %q: %v", entryName, err) + continue + } + if err := createWorkflowCommitStatus(ctx, run.Repo, run.CommitSHA, entryName+" ("+run.TriggerEvent+")", run.WorkflowID, + commitstatus.CommitStatusFailure, run.Link(), "Invalid workflow file"); err != nil { + log.Error("create commit status for invalid workflow %q: %v", entryName, err) + } + NotifyWorkflowRunStatusUpdate(ctx, run) + } +} diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index ca0482ab5a3..b4a3156572f 100644 --- a/services/actions/job_emitter.go +++ b/services/actions/job_emitter.go @@ -17,6 +17,7 @@ import ( "gitea.dev/modules/queue" "gitea.dev/modules/setting" "gitea.dev/modules/timeutil" + "gitea.dev/modules/util" "xorm.io/builder" ) @@ -281,13 +282,26 @@ func checkJobsOfCurrentRunAttempt(ctx context.Context, run *actions_model.Action if err != nil { return nil, err } - resolver := newJobStatusResolver(jobs, vars) - + var resolver *jobStatusResolver expandedAnyCaller := false if err = db.WithTx(ctx, func(ctx context.Context) error { for _, job := range jobs { job.Run = run } + cancelledJobs, err := cancelFailedMatrixSiblings(ctx, jobs) + if err != nil { + return err + } + result.CancelledJobs = append(result.CancelledJobs, cancelledJobs...) + for _, cancelledJob := range cancelledJobs { + for _, job := range jobs { + if job.ID == cancelledJob.ID { + job.Status = cancelledJob.Status + break + } + } + } + resolver = newJobStatusResolver(jobs, vars) updates, err := resolver.Resolve(ctx) if err != nil { @@ -310,6 +324,9 @@ func checkJobsOfCurrentRunAttempt(ctx context.Context, run *actions_model.Action if n, uerr := actions_model.UpdateRunJob(ctx, job, builder.Eq{"status": actions_model.StatusBlocked, "is_expanded": false}, "status", "stopped"); uerr != nil { return fmt.Errorf("mark unexpandable caller %d failed: %w", job.ID, uerr) } else if n == 1 { + if err := upsertJobErrorSummary(ctx, job, "uses", err); err != nil { + return err + } log.Warn("unexpandable caller %d has been marked as failed", job.ID) result.UpdatedJobs = append(result.UpdatedJobs, job) // Re-emit so the failed caller's dependents get resolved on the next pass. @@ -323,10 +340,12 @@ func checkJobsOfCurrentRunAttempt(ctx context.Context, run *actions_model.Action } else { expandedAnyCaller = true } - case actions_model.StatusSkipped: - job.Status = actions_model.StatusSkipped - if _, err := actions_model.UpdateRunJob(ctx, job, nil, "status"); err != nil { + case actions_model.StatusSkipped, actions_model.StatusFailure: + job.Status = status + if n, err := actions_model.UpdateRunJob(ctx, job, builder.Eq{"status": actions_model.StatusBlocked}, "status"); err != nil { return err + } else if n == 1 { + result.UpdatedJobs = append(result.UpdatedJobs, job) } } continue @@ -352,10 +371,31 @@ func checkJobsOfCurrentRunAttempt(ctx context.Context, run *actions_model.Action if expandedAnyCaller || resolver.matrixChanged { result.RunIDsToReEmit = append(result.RunIDsToReEmit, run.ID) } - result.CancelledJobs = resolver.cancelledJobs + result.CancelledJobs = append(result.CancelledJobs, resolver.cancelledJobs...) return result, nil } +func cancelFailedMatrixSiblings(ctx context.Context, jobs actions_model.ActionJobList) ([]*actions_model.ActionRunJob, error) { + var toCancel []*actions_model.ActionRunJob + for _, failed := range jobs { + if failed.Status != actions_model.StatusFailure || failed.ContinueOnError { + continue + } + siblings := slices.DeleteFunc(slices.Clone(jobs), func(sibling *actions_model.ActionRunJob) bool { + return !sibling.IsMatrixSiblingOf(failed) || sibling.Status.IsDone() || slices.Contains(toCancel, sibling) + }) + if len(siblings) == 0 { + continue + } + if failFast, err := failed.GetFailFast(); err != nil { + return nil, fmt.Errorf("parse failed matrix job %d: %w", failed.ID, err) + } else if failFast { + toCancel = append(toCancel, siblings...) + } + } + return actions_model.CancelJobs(ctx, toCancel, false) +} + type jobStatusResolver struct { statuses map[int64]actions_model.Status // sortedIDs are the keys of statuses, so blocked jobs are resolved in insertion order. @@ -487,13 +527,9 @@ func (r *jobStatusResolver) resolve(ctx context.Context) (map[int64]actions_mode continue } - // Decide whether the job runs at all before expanding a deferred matrix: a job whose needs - // failed or were skipped has to be skipped too, not failed for a matrix those needs never - // produced the outputs for. An `if:` that reads `matrix.*` cannot be decided this early, so - // evaluateJobIf reduces it to that needs gate and the pass below decides it per combination. + // decide before expanding, so failed needs skip the job instead of failing its matrix shouldStartJob, err := evaluateJobIf(ctx, actionRunJob.Run, nil, actionRunJob, r.vars, allSucceed) if err != nil { - // TODO: surface deterministic expression errors to users by failing the job with a message. log.Error("evaluateJobIf failed, job will stay blocked: job: %d, err: %v", id, err) continue } @@ -503,7 +539,6 @@ func (r *jobStatusResolver) resolve(ctx context.Context) (map[int64]actions_mode } // Expand a needs-dependent matrix now that its needs are done and the job is going to run. - wasDeferred := actionRunJob.IsMatrixDeferred siblings, err := expandDeferredMatrix(ctx, actionRunJob, r.vars) if err != nil { // Aborting the pass is required: once the placeholder is claimed as the first combination, @@ -525,19 +560,6 @@ func (r *jobStatusResolver) resolve(ctx context.Context) (map[int64]actions_mode if len(siblings) > 0 { r.matrixChanged, r.matrixInserted = true, true } - if wasDeferred { - // This row is now the first combination, and the `if:` can be evaluated. - // Gate it on its own combination here, as the siblings will be on the next pass. - shouldStartJob, err := evaluateJobIf(ctx, actionRunJob.Run, nil, actionRunJob, r.vars, allSucceed) - if err != nil { - log.Error("evaluateJobIf failed after matrix expansion, job will stay blocked: job: %d, err: %v", id, err) - continue - } - if !shouldStartJob { - ret[id] = actions_model.StatusSkipped - continue - } - } // A slot-starved job cannot start, skip the following checks. if !slots.available(actionRunJob) { @@ -545,11 +567,13 @@ func (r *jobStatusResolver) resolve(ctx context.Context) (map[int64]actions_mode } // update concurrency and check whether the job can run now - err = updateConcurrencyEvaluationForJobWithNeeds(ctx, actionRunJob, r.vars) - if err != nil { - // The err can be caused by different cases: database error, or syntax error, or the needed jobs haven't completed - // At the moment there is no way to distinguish them. - // TODO: if workflow or concurrency expression has syntax error, there should be a user error message, need to show it to end users + if err := updateConcurrencyEvaluationForJobWithNeeds(ctx, actionRunJob, r.vars); errors.Is(err, util.ErrInvalidArgument) { + if err := upsertJobErrorSummary(ctx, actionRunJob, "concurrency", err); err != nil { + return nil, err + } + ret[id] = actions_model.StatusFailure + continue + } else if err != nil { log.Debug("updateConcurrencyEvaluationForJobWithNeeds failed, this job will stay blocked: job: %d, err: %v", id, err) continue } @@ -587,7 +611,7 @@ func updateConcurrencyEvaluationForJobWithNeeds(ctx context.Context, actionRunJo } } if err := EvaluateJobConcurrencyFillModel(ctx, actionRunJob.Run, attempt, actionRunJob, vars, nil); err != nil { - return fmt.Errorf("evaluate job concurrency: %w", err) + return err } if _, err := actions_model.UpdateRunJob(ctx, actionRunJob, nil, "concurrency_group", "concurrency_cancel", "is_concurrency_evaluated"); err != nil { diff --git a/services/actions/job_emitter_test.go b/services/actions/job_emitter_test.go index 431ae53c679..9e2edc6d244 100644 --- a/services/actions/job_emitter_test.go +++ b/services/actions/job_emitter_test.go @@ -35,6 +35,7 @@ func Test_jobStatusResolver_Resolve(t *testing.T) { run *actions_model.ActionRun // defaults to stubRun jobs actions_model.ActionJobList want map[int64]actions_model.Status + note string }{ { name: "no blocked", @@ -80,6 +81,15 @@ func Test_jobStatusResolver_Resolve(t *testing.T) { 3: actions_model.StatusSkipped, }, }, + { + name: "failure checks transitive needs after a skipped job", + jobs: actions_model.ActionJobList{ + {ID: 1, JobID: "fail", Status: actions_model.StatusFailure}, + {ID: 2, JobID: "skipped", Status: actions_model.StatusSkipped, Needs: []string{"fail"}}, + {ID: 3, JobID: "recover", Status: actions_model.StatusBlocked, Needs: []string{"skipped"}, WorkflowPayload: []byte(`jobs: {recover: {if: "${{ failure() }}"}}`)}, + }, + want: map[int64]actions_model.Status{3: actions_model.StatusWaiting}, + }, { name: "loop need", jobs: actions_model.ActionJobList{ @@ -145,6 +155,24 @@ jobs: }, want: map[int64]actions_model.Status{2: actions_model.StatusSkipped}, }, + { + name: "invalid job `if` is skipped with an annotation", + jobs: actions_model.ActionJobList{ + {ID: 1, RepoID: 1, JobID: "job1", Status: actions_model.StatusSuccess}, + {ID: 2, RepoID: 1, JobID: "job2", Status: actions_model.StatusBlocked, Needs: []string{"job1"}, WorkflowPayload: []byte("jobs: {job2: {if: '${{ fromJSON(needs.job1.outputs.x) }}'}}")}, + }, + want: map[int64]actions_model.Status{2: actions_model.StatusSkipped}, + note: "Error when evaluating `if` for job `job2`.", + }, + { + name: "invalid job `concurrency` fails the job with an annotation", + jobs: actions_model.ActionJobList{ + {ID: 1, RepoID: 1, JobID: "job1", Status: actions_model.StatusSuccess}, + {ID: 2, RepoID: 1, JobID: "job2", Status: actions_model.StatusBlocked, Needs: []string{"job1"}, RawConcurrency: "group: ${{ fromJSON(needs.job1.outputs.cfg).group }}", WorkflowPayload: []byte("jobs: {job2: {}}")}, + }, + want: map[int64]actions_model.Status{2: actions_model.StatusFailure}, + note: "Error when evaluating `concurrency` for job `job2`.", + }, { name: "max-parallel: a freed slot promotes the lowest blocked job id", jobs: actions_model.ActionJobList{ @@ -256,13 +284,14 @@ jobs: } assert.NoError(t, unittest.PrepareTestDatabase()) ctx := t.Context() - stubRun := &actions_model.ActionRun{TriggerUser: &user_model.User{}, Repo: &repo_model.Repository{}} + stubRun := &actions_model.ActionRun{TriggerUser: &user_model.User{}, Repo: &repo_model.Repository{Owner: &user_model.User{}}} for i, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Each subtest gets a unique RunID / RunAttemptID so jobs from different subtests don't bleed into each other's FindTaskNeeds queries runID := int64(9001 + i) attemptID := int64(9001 + i) run := util.IfZero(tt.run, stubRun) + require.NoError(t, db.Insert(ctx, &actions_model.ActionRunAttempt{ID: attemptID, RepoID: 1, RunID: runID})) // Insert each test job (letting the DB assign IDs) and remember the testID -> dbID mapping so we can translate the expected map. idMap := make(map[int64]int64, len(tt.jobs)) @@ -292,6 +321,56 @@ jobs: got, err := r.Resolve(ctx) require.NoError(t, err) assert.Equal(t, want, got) + if tt.note != "" { + summaries, err := actions_model.ListActionRunJobSummaries(ctx, 1, runID, attemptID, 0) + require.NoError(t, err) + require.Len(t, summaries, 1) + assert.Contains(t, summaries[0].Content, tt.note) + } + }) + } +} + +func Test_cancelFailedMatrixSiblings(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + for index, test := range []struct { + name string + failFast bool + continueOnError bool + wantCancelled bool + }{ + {name: "fail fast cancels a queued combination", failFast: true, wantCancelled: true}, + {name: "disabled fail fast keeps a queued combination", failFast: false}, + {name: "continued failure keeps a queued combination", failFast: true, continueOnError: true}, + } { + t.Run(test.name, func(t *testing.T) { + ctx := t.Context() + run := &actions_model.ActionRun{RepoID: 1, OwnerID: 1, TriggerUserID: 1, WorkflowID: "matrix.yml", Index: int64(12000 + index), Status: actions_model.StatusRunning} + require.NoError(t, db.Insert(ctx, run)) + attempt := &actions_model.ActionRunAttempt{RepoID: 1, RunID: run.ID, Attempt: 1, Status: actions_model.StatusRunning} + require.NoError(t, db.Insert(ctx, attempt)) + _, err := db.Exec(ctx, "UPDATE `action_run` SET latest_attempt_id = ? WHERE id = ?", attempt.ID, run.ID) + require.NoError(t, err) + jobs := actions_model.ActionJobList{ + {RunID: run.ID, RunAttemptID: attempt.ID, RepoID: 1, OwnerID: 1, JobID: "matrix", Status: actions_model.StatusFailure, ContinueOnError: test.continueOnError, WorkflowPayload: fmt.Appendf(nil, "jobs: {matrix: {strategy: {fail-fast: %t}}}", test.failFast)}, + {RunID: run.ID, RunAttemptID: attempt.ID, RepoID: 1, OwnerID: 1, JobID: "matrix", Status: actions_model.StatusWaiting}, + } + for _, job := range jobs { + require.NoError(t, db.Insert(ctx, job)) + } + cancelled, err := cancelFailedMatrixSiblings(ctx, jobs) + require.NoError(t, err) + if test.wantCancelled { + require.Len(t, cancelled, 1) + assert.Equal(t, jobs[1].ID, cancelled[0].ID) + assert.Equal(t, actions_model.StatusCancelled, cancelled[0].Status) + run, err = actions_model.GetRunByRepoAndID(ctx, 1, run.ID) + require.NoError(t, err) + assert.Equal(t, actions_model.StatusFailure, run.Status) + } else { + assert.Empty(t, cancelled) + assert.Equal(t, actions_model.StatusWaiting, jobs[1].Status) + } }) } } @@ -525,6 +604,37 @@ func Test_checkJobsOfCurrentRunAttempt_NeedApprovalKeepsJobsBlocked(t *testing.T assert.Equal(t, actions_model.StatusBlocked, unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: job.ID}).Status) } +func Test_checkJobsOfCurrentRunAttempt_SkippedCallerIsUpdated(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + ctx := t.Context() + + run := &actions_model.ActionRun{ + RepoID: 4, OwnerID: 1, TriggerUserID: 1, + WorkflowID: "test.yml", Index: 9914, Ref: "refs/heads/main", + Status: actions_model.StatusBlocked, + } + assert.NoError(t, db.Insert(ctx, run)) + attempt := &actions_model.ActionRunAttempt{ + RepoID: 4, RunID: run.ID, Attempt: 1, Status: actions_model.StatusBlocked, + } + assert.NoError(t, db.Insert(ctx, attempt)) + _, err := db.Exec(ctx, "UPDATE `action_run` SET latest_attempt_id = ? WHERE id = ?", attempt.ID, run.ID) + assert.NoError(t, err) + run.LatestAttemptID = attempt.ID + caller := &actions_model.ActionRunJob{ + RunID: run.ID, RunAttemptID: attempt.ID, AttemptJobID: 1, + RepoID: 4, OwnerID: 1, JobID: "caller", Name: "caller", Status: actions_model.StatusBlocked, + IsReusableCaller: true, WorkflowPayload: []byte("jobs: {caller: {if: false, uses: ./.gitea/workflows/called.yml}}"), + } + assert.NoError(t, db.Insert(ctx, caller)) + + result, err := checkJobsOfCurrentRunAttempt(ctx, run) + assert.NoError(t, err) + require.Len(t, result.UpdatedJobs, 1) + assert.Equal(t, caller.ID, result.UpdatedJobs[0].ID) + assert.Equal(t, actions_model.StatusSkipped, unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: caller.ID}).Status) +} + // Test_checkRunConcurrency_HeldGroupDoesNotWake verifies that only an unoccupied concurrency group can wake up a blocked run/job. func Test_checkRunConcurrency_HeldGroupDoesNotWake(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/services/actions/matrix.go b/services/actions/matrix.go index 30e9bc9f6eb..f7490576fd0 100644 --- a/services/actions/matrix.go +++ b/services/actions/matrix.go @@ -119,9 +119,10 @@ func expandDeferredMatrix(ctx context.Context, job *actions_model.ActionRunJob, if err != nil { return fmt.Errorf("marshal expanded job: %w", err) } - dst.Name = util.EllipsisDisplayString(combo.Name, 255) + dst.Name = combo.DisplayName() dst.WorkflowPayload, dst.RunsOn = payload, combo.RunsOn() dst.ContinueOnError = combo.GetContinueOnError() + dst.MaxParallel = parseMaxParallel(job.JobID, combo.Strategy.MaxParallelString) return nil } @@ -174,7 +175,7 @@ func expandDeferredMatrix(ctx context.Context, job *actions_model.ActionRunJob, job.IsMatrixDeferred = false affected, err := actions_model.UpdateRunJob(ctx, job, builder.Eq{"is_matrix_deferred": true, "status": actions_model.StatusBlocked}, - "name", "workflow_payload", "runs_on", "continue_on_error", "is_matrix_deferred") + "name", "workflow_payload", "runs_on", "continue_on_error", "max_parallel", "is_matrix_deferred") if err != nil { return nil, fmt.Errorf("claim placeholder of job %d: %w", job.ID, err) } @@ -208,10 +209,11 @@ func restoreDeferredMatrixPlaceholder(clone *actions_model.ActionRunJob) error { if err != nil { return fmt.Errorf("parse deferred matrix payload: %w", err) } - clone.Name = util.EllipsisDisplayString(parsed.Name, 255) + clone.Name = parsed.DisplayName() clone.WorkflowPayload = slices.Clone(clone.DeferredMatrixPayload) clone.RunsOn = parsed.RunsOn() clone.ContinueOnError = parsed.GetContinueOnError() + clone.MaxParallel = parseMaxParallel(clone.JobID, parsed.Strategy.MaxParallelString) clone.IsMatrixDeferred = true return nil } diff --git a/services/actions/matrix_test.go b/services/actions/matrix_test.go index aac0d477204..9f931ee5f4f 100644 --- a/services/actions/matrix_test.go +++ b/services/actions/matrix_test.go @@ -23,28 +23,25 @@ var testRunIndex int64 = 9100 // setupDeferredMatrixJob plants a completed `generate` job exposing outputs and the blocked `build` // placeholder that depends on them, and returns the placeholder. Both are children of a reusable // workflow caller, the case where a sibling losing ParentJobID would break needs resolution. -// jobIf is the `build` job's `if:` expression, omitted entirely when empty. -func setupDeferredMatrixJob(t *testing.T, matrixValue, jobIf string, outputs map[string]string) *actions_model.ActionRunJob { +func setupDeferredMatrixJob(t *testing.T, matrixValue string, outputs map[string]string) *actions_model.ActionRunJob { t.Helper() ctx := t.Context() - ifLine := "" - if jobIf != "" { - ifLine = " if: " + jobIf + "\n" - } // The `build` job takes its matrix from `generate`'s outputs, so Parse defers it. workflows, err := jobparser.Parse(fmt.Appendf(nil, ` on: push jobs: generate: + runs-on: ubuntu-latest steps: [{run: echo}] build: needs: generate -%s strategy: + runs-on: ubuntu-latest + strategy: matrix: value: %s steps: [{run: echo}] -`, ifLine, matrixValue)) +`, matrixValue)) require.NoError(t, err) var placeholder *jobparser.SingleWorkflow for _, workflow := range workflows { @@ -106,13 +103,12 @@ func TestExpandDeferredMatrix(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) t.Run("expands into siblings", func(t *testing.T) { - job := setupDeferredMatrixJob(t, "${{ fromJson(needs.generate.outputs.values) }}", "", map[string]string{"values": `["a","b","c"]`}) + job := setupDeferredMatrixJob(t, "${{ fromJson(needs.generate.outputs.values) }}", map[string]string{"values": `["a","b","c"]`}) siblings, err := expandDeferredMatrix(t.Context(), job, nil) require.NoError(t, err) require.Len(t, siblings, 2) - // The placeholder is reused as the first combination and stays blocked for the `if:` gate. assert.Equal(t, "build (a)", job.Name) assert.False(t, job.IsMatrixDeferred) assert.Equal(t, actions_model.StatusBlocked, job.Status) @@ -135,6 +131,11 @@ func TestExpandDeferredMatrix(t *testing.T) { assert.Equal(t, "build (a)", reloaded.Name) assert.False(t, reloaded.IsMatrixDeferred) assert.NotEmpty(t, reloaded.DeferredMatrixPayload, "the claim must not erase the raw payload") + reloaded.MaxParallel = 3 + require.NoError(t, restoreDeferredMatrixPlaceholder(reloaded)) + assert.Equal(t, "build", reloaded.Name) + assert.Zero(t, reloaded.MaxParallel) + assert.True(t, reloaded.IsMatrixDeferred) }) // A matrix that can never produce runnable combinations fails the job instead of rolling the @@ -168,7 +169,7 @@ func TestExpandDeferredMatrix(t *testing.T) { if outputs == nil { outputs = map[string]string{"values": `["a","b","c"]`} } - job := setupDeferredMatrixJob(t, tt.matrixValue, "", outputs) + job := setupDeferredMatrixJob(t, tt.matrixValue, outputs) if tt.prepare != nil { tt.prepare(t, job) } @@ -185,40 +186,14 @@ func TestExpandDeferredMatrix(t *testing.T) { } } -// TestDeferredMatrixResolverGating covers the resolver deciding a placeholder's fate around the -// expansion. A need that did not succeed leaves no outputs to build the matrix from, so the job is -// skipped like any other job with such a need rather than failed over a matrix it never had to -// evaluate, and no combination is inserted. A job that does run is then gated by its own -// combination: the placeholder reused as the first one is judged by `matrix.*`, not by the raw -// expression the `if:` would have seen before expansion. -func TestDeferredMatrixResolverGating(t *testing.T) { +func TestDeferredMatrixResolverSkipsWithoutSucceededNeeds(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) - for _, tt := range []struct { - name string - needStatus actions_model.Status - jobIf string - outputs map[string]string - wantBuilds []string - }{ - {name: "failed need", needStatus: actions_model.StatusFailure, wantBuilds: []string{"build"}}, - {name: "skipped need", needStatus: actions_model.StatusSkipped, wantBuilds: []string{"build"}}, - { - name: "`if:` gated per combination", needStatus: actions_model.StatusSuccess, - jobIf: "${{ matrix.value == 'b' }}", outputs: map[string]string{"values": `["a","b"]`}, - wantBuilds: []string{"build (a)", "build (b)"}, - }, - { - // The same gate without the `${{ }}`, which must expand rather than skip the whole job. - name: "brace-less `if:` gated per combination", needStatus: actions_model.StatusSuccess, - jobIf: "matrix.value == 'b'", outputs: map[string]string{"values": `["a","b"]`}, - wantBuilds: []string{"build (a)", "build (b)"}, - }, - } { - t.Run(tt.name, func(t *testing.T) { + for _, needStatus := range []actions_model.Status{actions_model.StatusFailure, actions_model.StatusSkipped} { + t.Run(needStatus.String(), func(t *testing.T) { ctx := t.Context() - job := setupDeferredMatrixJob(t, "${{ fromJson(needs.generate.outputs.values) }}", tt.jobIf, tt.outputs) - _, err := db.Exec(ctx, "UPDATE `action_run_job` SET status = ? WHERE run_id = ? AND job_id = ?", int(tt.needStatus), job.RunID, "generate") + job := setupDeferredMatrixJob(t, "${{ fromJson(needs.generate.outputs.values) }}", nil) + _, err := db.Exec(ctx, "UPDATE `action_run_job` SET status = ? WHERE run_id = ? AND job_id = ?", int(needStatus), job.RunID, "generate") require.NoError(t, err) jobs := runJobs(t, job.RunID, job.RunAttemptID) @@ -234,7 +209,7 @@ func TestDeferredMatrixResolverGating(t *testing.T) { names = append(names, runJob.Name) } } - assert.ElementsMatch(t, tt.wantBuilds, names) + assert.Equal(t, []string{"build"}, names) }) } } @@ -243,9 +218,7 @@ func TestDeferredMatrixResolverDefersDependents(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) ctx := t.Context() - // `build (a)` is skipped by the `if:` once it carries its own combination, `build (b)` runs. - build := setupDeferredMatrixJob(t, "${{ fromJson(needs.generate.outputs.values) }}", - "${{ matrix.value != 'a' }}", map[string]string{"values": `["a","b"]`}) + build := setupDeferredMatrixJob(t, "${{ fromJson(needs.generate.outputs.values) }}", map[string]string{"values": `["a","b"]`}) attemptJobID, err := actions_model.GetNextAttemptJobID(ctx, build.RunID) require.NoError(t, err) @@ -262,7 +235,7 @@ func TestDeferredMatrixResolverDefersDependents(t *testing.T) { updates, err := newJobStatusResolver(jobs, nil).Resolve(ctx) require.NoError(t, err) - assert.Equal(t, actions_model.StatusSkipped, updates[build.ID], "the combination the `if:` excludes") + assert.Equal(t, actions_model.StatusWaiting, updates[build.ID], "the placeholder runs as `build (a)`") assert.NotContains(t, updates, report.ID, "report must wait for the re-emit, which sees `build (b)` too") // The sibling the pass would otherwise have resolved report against is there, and still to run. diff --git a/services/actions/max_parallel.go b/services/actions/max_parallel.go index 703a11385bb..dd0e1227775 100644 --- a/services/actions/max_parallel.go +++ b/services/actions/max_parallel.go @@ -4,36 +4,20 @@ package actions import ( - "math" - "strconv" "strings" + "gitea.dev/actionslib/pkg/model" actions_model "gitea.dev/models/actions" "gitea.dev/modules/log" ) // parseMaxParallel returns strategy.max-parallel for a job, 0 meaning unlimited. -// GitHub accepts any YAML number here and casts it to an int, so 1.5 truncates to 1. -// Expressions are not evaluated yet and fall back to unlimited. func parseMaxParallel(jobID, maxParallelString string) int { - if maxParallelString == "" { - return 0 + limit, err := model.Strategy{MaxParallelString: maxParallelString}.MaxParallel() + if err != nil && !strings.Contains(maxParallelString, "${{") { // a deferred matrix placeholder's expression is evaluated at expansion + log.Warn("job %s: %v, treating as unlimited", jobID, err) } - maxParallel, err := strconv.ParseFloat(maxParallelString, 64) - if err != nil || math.IsNaN(maxParallel) { - // Both fall back to unlimited, but an expression is a gap in Gitea while a non-number is the - // author's mistake, so dropping the cap must not be reported the same way. - if strings.Contains(maxParallelString, "${{") { - // TODO: evaluate it against the contexts `if:` and the matrix already resolve, so that an - // expression can actually cap a job instead of quietly disabling the cap. - log.Debug("job %s: max-parallel %q is an expression, which is not evaluated yet: treating as unlimited", jobID, maxParallelString) - } else { - log.Warn("job %s: max-parallel %q is not a number, treating as unlimited", jobID, maxParallelString) - } - return 0 - } - // a run can never hold more jobs than MaxJobNumPerRun, so clamping there keeps the cast total - return int(min(max(maxParallel, 0), actions_model.MaxJobNumPerRun)) + return min(limit, actions_model.MaxJobNumPerRun) } // maxParallelSlots counts the jobs holding a max-parallel slot. Slots are scoped by ParentJobID as diff --git a/services/actions/max_parallel_test.go b/services/actions/max_parallel_test.go index 936646bcec2..a90b6834aaf 100644 --- a/services/actions/max_parallel_test.go +++ b/services/actions/max_parallel_test.go @@ -25,12 +25,12 @@ func TestParseMaxParallel(t *testing.T) { {"3", 3}, {"0", 0}, {"-1", 0}, - {"1.5", 1}, // GitHub casts the YAML number to int - {"-1.5", 0}, // truncates to -1, which means unlimited - {"1e3", 256}, // clamped to MaxJobNumPerRun - {"nan", 0}, // must not reach the int cast - {"${{ vars.n }}", 0}, // expressions are not evaluated yet, logged as such - {"abc", 0}, // a plain workflow error, warned about rather than hidden + {"1.5", 1}, // GitHub casts the YAML number to int + {"-1.5", 0}, // truncates to -1, which means unlimited + {"1e3", 256}, // clamped to MaxJobNumPerRun + {"nan", 0}, // must not reach the int cast + {"${{ vars.n }}", 0}, + {"abc", 0}, // a plain workflow error, warned about rather than hidden } for _, tt := range tests { assert.Equal(t, tt.want, parseMaxParallel("job", tt.input), "input %q", tt.input) @@ -44,7 +44,7 @@ jobs: build: runs-on: ubuntu-latest strategy: - max-parallel: 2 + max-parallel: ${{ fromJSON('2') }} matrix: version: [1, 2, 3, 4, 5] steps: diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 700750ccf2b..c67b6fb1026 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -188,7 +188,7 @@ func notify(ctx context.Context, input *notifyInput) error { var detectedWorkflows []*actions_module.DetectedWorkflow var filteredWorkflows []*actions_module.DetectedWorkflow actionsConfig := input.Repo.MustGetUnit(ctx, unit_model.TypeActions).ActionsConfig() - workflows, schedules, filtered, err := actions_module.DetectWorkflows(ctx, gitRepo, commit, + workflows, schedules, filtered, invalid, err := actions_module.DetectWorkflows(ctx, gitRepo, commit, input.Event, input.Payload, shouldDetectSchedules, @@ -234,7 +234,7 @@ func notify(ctx context.Context, input *notifyInput) error { if err != nil { return fmt.Errorf("gitRepo.GetCommit: %w", err) } - baseWorkflows, _, baseFiltered, err := actions_module.DetectWorkflows(ctx, gitRepo, baseCommit, input.Event, input.Payload, false) + baseWorkflows, _, baseFiltered, _, err := actions_module.DetectWorkflows(ctx, gitRepo, baseCommit, input.Event, input.Payload, false) if err != nil { return fmt.Errorf("DetectWorkflows: %w", err) } @@ -268,6 +268,10 @@ func notify(ctx context.Context, input *notifyInput) error { } } + if input.Event == webhook_module.HookEventPush { + handleInvalidWorkflows(ctx, input, ref, commit, invalid) + } + if err := handleWorkflows(ctx, detectedWorkflows, commit, input, ref); err != nil { return err } diff --git a/services/actions/reusable_workflow.go b/services/actions/reusable_workflow.go index 173a28d8fd7..641cb21efdc 100644 --- a/services/actions/reusable_workflow.go +++ b/services/actions/reusable_workflow.go @@ -9,11 +9,13 @@ import ( "fmt" "strings" + "gitea.dev/actionslib/pkg/model" actions_model "gitea.dev/models/actions" "gitea.dev/models/db" perm_model "gitea.dev/models/perm" access_model "gitea.dev/models/perm/access" repo_model "gitea.dev/models/repo" + "gitea.dev/models/unit" actions_module "gitea.dev/modules/actions" "gitea.dev/modules/actions/jobparser" "gitea.dev/modules/container" @@ -23,15 +25,13 @@ import ( "gitea.dev/modules/log" "gitea.dev/modules/setting" api "gitea.dev/modules/structs" - "gitea.dev/modules/util" "gitea.dev/services/convert" "xorm.io/builder" ) -// MaxReusableCallLevels caps how deep a reusable workflow can nest: -// a top-level caller may have at most MaxReusableCallLevels nested callers below it. -const MaxReusableCallLevels = 9 +// MaxReusableCallLevels allows nine calls across ten workflows, including the top-level workflow. +const MaxReusableCallLevels = 8 // checkRunJobLimit rejects an expansion that would push the attempt over actions_model.MaxJobNumPerRun. // checkCallerChain bounds nesting *depth*, but a reusable graph also fans out in *breadth*: without a @@ -49,13 +49,13 @@ func checkRunJobLimit(ctx context.Context, runID, attemptID int64, adding int) e // loadReusableWorkflowSource resolves the workflow file referenced by a caller's `uses:` and returns its raw bytes, // along with the (repo_id, commit_sha) the file was loaded from. -func loadReusableWorkflowSource(ctx context.Context, run *actions_model.ActionRun, caller *actions_model.ActionRunJob, ref *jobparser.UsesRef) (content []byte, sourceRepoID int64, sourceCommitSHA string, err error) { +func loadReusableWorkflowSource(ctx context.Context, run *actions_model.ActionRun, caller *actions_model.ActionRunJob, ref *model.ReusableWorkflowUses) (content []byte, sourceRepoID int64, sourceCommitSHA string, err error) { if err := run.LoadAttributes(ctx); err != nil { return nil, 0, "", err } - switch ref.Kind { - case jobparser.UsesKindLocalSameRepo: + switch { + case ref.IsLocal(): // `./` and `$/` are resolved against the workflow file containing the `uses:` - i.e. the caller's own source repo + commit. callerRepo, err := repo_model.GetRepositoryByID(ctx, caller.WorkflowSourceRepoID) if err != nil { @@ -71,8 +71,12 @@ func loadReusableWorkflowSource(ctx context.Context, run *actions_model.ActionRu } return bytes, callerRepo.ID, resolvedSHA, nil - case jobparser.UsesKindLocalCrossRepo: + default: + unavailable := fmt.Errorf("reusable workflow repository %s/%s does not exist or is not readable", ref.Owner, ref.Repo) // the same for both, so a run cannot tell whether a private one exists repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, ref.Owner, ref.Repo) + if repo_model.IsErrRepoNotExist(err) { + return nil, 0, "", unavailable + } if err != nil { return nil, 0, "", fmt.Errorf("look up cross-repo workflow source %q: %w", ref.Owner+"/"+ref.Repo, err) } @@ -81,12 +85,7 @@ func loadReusableWorkflowSource(ctx context.Context, run *actions_model.ActionRu return nil, 0, "", err } if !ok { - if run.IsScopedRun { - // A scoped workflow's cross-repo "uses:" is resolved with the consuming repo's read permission, - // so the referenced repo must be readable by every consumer. Make that explicit in the failure. - return nil, 0, "", fmt.Errorf("no permission to read reusable workflow %s/%s: a scoped workflow's cross-repo \"uses:\" is resolved with the consuming repository %q read permission", ref.Owner, ref.Repo, run.Repo.FullName()) - } - return nil, 0, "", fmt.Errorf("no permission to read reusable workflow from %s/%s", ref.Owner, ref.Repo) + return nil, 0, "", unavailable } bytes, resolvedSHA, err := readWorkflowFromRepo(ctx, repo, ref.Ref, ref.Path) if err != nil { @@ -94,7 +93,6 @@ func loadReusableWorkflowSource(ctx context.Context, run *actions_model.ActionRu } return bytes, repo.ID, resolvedSHA, nil } - return nil, 0, "", fmt.Errorf("unsupported uses kind %d", ref.Kind) } // resolveSameRepoWorkflowSourceCommit returns the commit to read a same-repo reusable workflow from. @@ -132,16 +130,12 @@ func readWorkflowFromRepo(ctx context.Context, repo *repo_model.Repository, refO // checkCallerChain walks `caller`'s ancestor chain (via ParentJobID) and: // - rejects cycles (caller.CallUses appearing in any ancestor's CallUses) // - enforces MaxReusableCallLevels on the number of ancestors above `caller` -// -// Cycle detection is intentionally *syntactic* (string equality on canonicalCallUses), not semantic. -// So `owner/repo/lib.yml@v1` and `owner/repo/lib.yml@refs/heads/v1` resolving to the same commit are NOT treated as the same node. -// Going semantic (Owner, Repo, Path, ResolvedSHA tuples) would require extra git reads. func checkCallerChain(ctx context.Context, caller *actions_model.ActionRunJob) error { if caller.ParentJobID == 0 { return nil // top-level caller: depth 0, no ancestors to walk } - visited := container.SetOf(canonicalCallUses(caller.CallUses)) + visited := container.SetOf(canonicalCallUses(caller)) depth := 0 current := caller @@ -155,19 +149,41 @@ func checkCallerChain(ctx context.Context, caller *actions_model.ActionRunJob) e if depth > MaxReusableCallLevels { return fmt.Errorf("reusable workflow call exceeds the maximum nesting level of %d at %q", MaxReusableCallLevels, caller.CallUses) } - if current.IsReusableCaller && current.CallUses != "" && !visited.Add(canonicalCallUses(current.CallUses)) { + if current.IsReusableCaller && current.CallUses != "" && !visited.Add(canonicalCallUses(current)) { return fmt.Errorf("reusable workflow call cycle detected: %q", current.CallUses) } } return nil } -// canonicalCallUses folds the two same-repo prefixes into one key, because `$/x.yml` and `./x.yml` name the same file. -func canonicalCallUses(uses string) string { - if ref, err := jobparser.ParseUses(uses); err == nil && ref.Kind == jobparser.UsesKindLocalSameRepo { - return "./" + ref.Path +func checkResolvedCallerCycle(ctx context.Context, caller *actions_model.ActionRunJob, sourceRepoID int64, sourceCommitSHA, path string) error { + for current := caller; current.ParentJobID != 0; { + parent, err := actions_model.GetRunJobByRunAndID(ctx, current.RunID, current.ParentJobID) + if err != nil { + return fmt.Errorf("walk caller chain: %w", err) + } + ref, err := ResolveUses(ctx, parent.CallUses) + if err != nil { + return fmt.Errorf("resolve ancestor uses %q: %w", parent.CallUses, err) + } + if current.WorkflowSourceRepoID == sourceRepoID && current.WorkflowSourceCommitSHA == sourceCommitSHA && ref.Path == path { + return fmt.Errorf("reusable workflow call cycle detected: %q", caller.CallUses) + } + current = parent } - return uses + return nil +} + +// canonicalCallUses keys a call by its parsed form, so the `$/` and `self:` spellings match the plain ones. +func canonicalCallUses(job *actions_model.ActionRunJob) string { + ref, err := model.ParseReusableWorkflowUses(job.CallUses) + if err != nil { + return job.CallUses + } + if ref.IsLocal() { + return fmt.Sprintf("./%s@%d:%s", ref.Path, job.WorkflowSourceRepoID, job.WorkflowSourceCommitSHA) + } + return ref.Owner + "/" + ref.Repo + "/" + ref.Path + "@" + ref.Ref } // expandReusableWorkflowCaller loads and parses the target reusable workflow and inserts the caller's direct child jobs. @@ -204,9 +220,12 @@ func expandReusableWorkflowCaller(ctx context.Context, run *actions_model.Action if err != nil { return err } + if err := checkResolvedCallerCycle(ctx, caller, contentSourceRepoID, contentSourceCommitSHA, ref.Path); err != nil { + return err + } // 4. Parse the called workflow's spec (used by both secret validation and input evaluation). - wcSpec, err := jobparser.ParseWorkflowCallSpec(content) + wcSpec, err := jobparser.ParseWorkflowCallConfig(content) if err != nil { return fmt.Errorf("parse called workflow spec: %w", err) } @@ -220,7 +239,7 @@ func expandReusableWorkflowCaller(ctx context.Context, run *actions_model.Action // so required-secret presence cannot be verified at expansion time and a missing required secret will surface at job runtime. // This matches GitHub Actions' behavior. if !inherit { - if err := jobparser.ValidateCallerSecrets(wcSpec, secretsMap); err != nil { + if err := wcSpec.ValidateSecrets(secretsMap); err != nil { return fmt.Errorf("caller %q secrets: %w", caller.JobID, err) } } @@ -238,7 +257,7 @@ func expandReusableWorkflowCaller(ctx context.Context, run *actions_model.Action // 6. Evaluate caller's `with:`, then match against the callee schema. workflowCallInputs := map[string]any{} - if len(wcSpec.Inputs) > 0 { + if len(wcSpec.Inputs) > 0 || parsedJob.With.Kind != 0 { jobResults, err := findJobNeedsAndFillJobResults(ctx, caller) if err != nil { return fmt.Errorf("find caller needs: %w", err) @@ -248,14 +267,7 @@ func expandReusableWorkflowCaller(ctx context.Context, run *actions_model.Action return err } callerGitCtx := GenerateGiteaContext(ctx, run, attempt, caller) - evaluated, err := jobparser.EvaluateCallerWith( - caller.JobID, parsedJob, - callerGitCtx, jobResults, vars, parentInputs, - ) - if err != nil { - return fmt.Errorf("evaluate caller with: %w", err) - } - workflowCallInputs, err = jobparser.MatchCallerInputsAgainstSpec(wcSpec, evaluated) + workflowCallInputs, err = jobparser.ResolveCallerInputs(caller.JobID, parsedJob, wcSpec, callerGitCtx, jobResults, vars, parentInputs) if err != nil { return fmt.Errorf("caller %q inputs: %w", caller.JobID, err) } @@ -306,6 +318,23 @@ func expandReusableWorkflowCaller(ctx context.Context, run *actions_model.Action // insertCallerChildren parses the called workflow with the caller's resolved inputs and inserts each parsed job. func insertCallerChildren(ctx context.Context, run *actions_model.ActionRun, attempt *actions_model.ActionRunAttempt, caller *actions_model.ActionRunJob, content []byte, sourceRepoID int64, sourceCommitSHA string, vars map[string]string, inputs map[string]any) error { + callerPermissions := caller.TokenPermissions + if callerPermissions == nil { + actionsUnit, err := run.Repo.GetUnit(ctx, unit.TypeActions) + if err != nil { + return fmt.Errorf("load caller repository Actions settings: %w", err) + } + if config := actionsUnit.ActionsConfig(); config.OverrideOwnerConfig { + callerPermissions = new(config.GetDefaultTokenPermissions()) + } else { + ownerConfig, err := actions_model.GetOwnerActionsConfig(ctx, run.OwnerID) + if err != nil { + return fmt.Errorf("load caller owner Actions settings: %w", err) + } + callerPermissions = new(ownerConfig.GetDefaultTokenPermissions()) + } + } + // Parse the called workflow with the caller's `inputs` gitCtx := GenerateGiteaContext(ctx, run, attempt, nil) if event, ok := gitCtx["event"].(map[string]any); ok { @@ -349,7 +378,7 @@ func insertCallerChildren(ctx context.Context, run *actions_model.ActionRun, att return fmt.Errorf("marshal child %q under caller %d: %w", jobID, caller.ID, err) } - parsedChild.Name = util.EllipsisDisplayString(parsedChild.Name, 255) + parsedChild.Name = parsedChild.DisplayName() // AttemptJobID: prefer a prior-attempt match and fall back to a fresh allocator value for newly-appearing logical jobs. var attemptJobID int64 @@ -388,7 +417,9 @@ func insertCallerChildren(ctx context.Context, run *actions_model.ActionRun, att child.DeferredMatrixPayload = payload } if perms := ExtractJobPermissionsFromWorkflow(sw, parsedChild); perms != nil { - child.TokenPermissions = perms + child.TokenPermissions = new(repo_model.ClampActionsTokenPermissions(*perms, *callerPermissions)) + } else { + child.TokenPermissions = callerPermissions } if parsedChild.Uses != "" { child.IsReusableCaller = true @@ -403,8 +434,8 @@ func insertCallerChildren(ctx context.Context, run *actions_model.ActionRun, att // 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) { +// then validates the syntax via model.ParseReusableWorkflowUses. +func ResolveUses(ctx context.Context, uses string) (*model.ReusableWorkflowUses, 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. @@ -415,11 +446,10 @@ func ResolveUses(ctx context.Context, uses string) (*jobparser.UsesRef, error) { // RoutePath is the instance-relative path (AppSubURL already stripped), e.g. "/owner/repo/.gitea/workflows/file.yml@ref". uses = strings.TrimPrefix(gsu.RoutePath, "/") } - ref, err := jobparser.ParseUses(uses) + ref, err := model.ParseReusableWorkflowUses(uses) if err != nil { return nil, err } - // jobparser only validates syntax; enforce the (instance-configurable) directory allowlist here. if !actions_module.IsWorkflowOrScopedWorkflow(ref.Path) { return nil, fmt.Errorf(`"uses:" path %q must be under a configured workflow directory (WORKFLOW_DIRS or SCOPED_WORKFLOW_DIRS)`, ref.Path) } diff --git a/services/actions/reusable_workflow_test.go b/services/actions/reusable_workflow_test.go index 1528e991220..41a33d6564c 100644 --- a/services/actions/reusable_workflow_test.go +++ b/services/actions/reusable_workflow_test.go @@ -7,11 +7,11 @@ import ( "fmt" "testing" + "gitea.dev/actionslib/pkg/model" actions_model "gitea.dev/models/actions" "gitea.dev/models/db" "gitea.dev/models/unittest" actions_module "gitea.dev/modules/actions" - "gitea.dev/modules/actions/jobparser" "gitea.dev/modules/json" "gitea.dev/modules/setting" api "gitea.dev/modules/structs" @@ -55,6 +55,7 @@ func TestCheckCallerChain_Cycle(t *testing.T) { ) err := checkCallerChain(t.Context(), chain[len(chain)-1]) assert.ErrorContains(t, err, "cycle detected") + assert.Equal(t, canonicalCallUses(&actions_model.ActionRunJob{CallUses: "owner/repo/.gitea/workflows/a.yml@v1"}), canonicalCallUses(&actions_model.ActionRunJob{CallUses: "self:owner/repo/.gitea/workflows/a.yml@v1"})) }) t.Run("NoCycle", func(t *testing.T) { @@ -67,6 +68,37 @@ func TestCheckCallerChain_Cycle(t *testing.T) { ) require.NoError(t, checkCallerChain(t.Context(), chain[len(chain)-1])) }) + + t.Run("SameLocalPathInOtherRepo", func(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + chain := buildCallerChain(t, + "./.gitea/workflows/a.yml", + "owner/lib/.gitea/workflows/lib.yml@v1", + "./.gitea/workflows/a.yml", + ) + leaf := chain[len(chain)-1] + leaf.WorkflowSourceRepoID = 2 + require.NoError(t, checkCallerChain(t.Context(), leaf)) + }) + + t.Run("ResolvedIdentityCycle", func(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + chain := buildCallerChain(t, + "./.gitea/workflows/a.yml", + "owner/repo/.gitea/workflows/b.yml@v1", + "owner/repo/.gitea/workflows/a.yml@v1", + ) + chain[1].WorkflowSourceRepoID = 4 + chain[1].WorkflowSourceCommitSHA = "first-commit" + _, err := actions_model.UpdateRunJob(t.Context(), chain[1], nil, "workflow_source_repo_id", "workflow_source_commit_sha") + require.NoError(t, err) + chain[2].WorkflowSourceRepoID = 5 + chain[2].WorkflowSourceCommitSHA = "second-commit" + + require.NoError(t, checkCallerChain(t.Context(), chain[2])) + require.ErrorContains(t, checkResolvedCallerCycle(t.Context(), chain[2], 4, "first-commit", ".gitea/workflows/a.yml"), "cycle detected") + require.NoError(t, checkResolvedCallerCycle(t.Context(), chain[2], 4, "other-commit", ".gitea/workflows/a.yml")) + }) } func TestCheckCallerChain_DepthLimit(t *testing.T) { @@ -162,11 +194,11 @@ func TestResolveUses(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) + assert.Equal(t, model.ReusableWorkflowUses{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) + assert.Equal(t, model.ReusableWorkflowUses{Owner: "owner", Repo: "repo", Path: ".gitea/workflows/build.yml", Ref: "v1"}, *ref) }) t.Run("DirectoryAllowlist", func(t *testing.T) { @@ -179,11 +211,17 @@ func TestResolveUses(t *testing.T) { require.NoError(t, err) assert.Equal(t, ".gitea/scoped_workflows/lib.yml", ref.Path) + ref, err = ResolveUses(ctx, "self:owner/repo/.gitea/scoped_workflows/lib.yml@v1") + require.NoError(t, err) + assert.Equal(t, ".gitea/scoped_workflows/lib.yml", ref.Path) + // A directory that is neither WORKFLOW_DIRS nor SCOPED_WORKFLOW_DIRS parses but is rejected by the allowlist. _, err = ResolveUses(ctx, "./not-workflows/build.yml") require.Error(t, err) _, err = ResolveUses(ctx, "owner/repo/lib/build.yml@v1") require.Error(t, err) + _, err = ResolveUses(ctx, "self:owner/repo/lib/build.yml@v1") + require.Error(t, err) }) t.Run("ConfigurableWorkflowDirs", func(t *testing.T) { @@ -201,7 +239,7 @@ func TestResolveUses(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) + assert.Equal(t, model.ReusableWorkflowUses{Owner: "owner", Repo: "repo", Path: ".gitea/workflows/ci.yml", Ref: "refs/heads/main"}, *ref) }) t.Run("InvalidSyntax", func(t *testing.T) { @@ -222,6 +260,16 @@ func TestResolveUses(t *testing.T) { }) } +func TestLoadReusableWorkflowSourceFailsAlikeForMissingAndPrivateRepo(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + run := &actions_model.ActionRun{RepoID: 4, TriggerUserID: 1} + for _, repoName := range []string{"missing", "repo3"} { + _, _, _, err := loadReusableWorkflowSource(t.Context(), run, nil, &model.ReusableWorkflowUses{Owner: "org3", Repo: repoName, Path: ".gitea/workflows/build.yml", Ref: "main"}) + assert.EqualError(t, err, "reusable workflow repository org3/"+repoName+" does not exist or is not readable") + } +} + func TestCheckRunJobLimit(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) diff --git a/services/actions/run.go b/services/actions/run.go index 340cedee5c9..2379b3d8c97 100644 --- a/services/actions/run.go +++ b/services/actions/run.go @@ -200,7 +200,7 @@ func insertRunJob(ctx context.Context, run *actions_model.ActionRun, runAttempt return nil, nil, false, fmt.Errorf("alloc attempt_job_id: %w", err) } - job.Name = util.EllipsisDisplayString(job.Name, 255) + job.Name = job.DisplayName() runJob := &actions_model.ActionRunJob{ RunID: run.ID, RunAttemptID: runAttempt.ID, diff --git a/services/actions/task.go b/services/actions/task.go index e63a0ff4660..35d31788b20 100644 --- a/services/actions/task.go +++ b/services/actions/task.go @@ -180,7 +180,7 @@ func generateTaskContext(ctx context.Context, t *actions_model.ActionTask) (*str } func findTaskNeeds(ctx context.Context, taskJob *actions_model.ActionRunJob) (map[string]*runnerv1.TaskNeed, error) { - taskNeeds, err := FindTaskNeeds(ctx, taskJob) + taskNeeds, _, err := FindTaskNeeds(ctx, taskJob) if err != nil { return nil, err } diff --git a/services/actions/workflow.go b/services/actions/workflow.go index 46c0ef57ab6..b02a6dd4381 100644 --- a/services/actions/workflow.go +++ b/services/actions/workflow.go @@ -140,9 +140,12 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re return 0, err } + if _, err := jobparser.ValidateWorkflowStatic(content); err != nil { + return 0, util.ErrorWrapTranslatable(util.NewInvalidArgumentErrorf("invalid workflow %q: %v", workflowID, err), "actions.runs.invalid_workflow_helper", err.Error()) + } workflow, err := jobparser.ReadWorkflow(content) if err != nil { - return 0, fmt.Errorf("failed to unmarshal workflow content: %w", err) + return 0, util.ErrorWrapTranslatable(util.NewInvalidArgumentErrorf("invalid workflow %q: %v", workflowID, err), "actions.runs.invalid_workflow_helper", err.Error()) } // get inputs from post workflowDispatch := workflow.WorkflowDispatchConfig() diff --git a/templates/repo/actions/workflow_dispatch_inputs.tmpl b/templates/repo/actions/workflow_dispatch_inputs.tmpl index 279a43bf0df..e352bb03004 100644 --- a/templates/repo/actions/workflow_dispatch_inputs.tmpl +++ b/templates/repo/actions/workflow_dispatch_inputs.tmpl @@ -8,7 +8,7 @@ {{end}} {{else}} - {{range $item := .WorkflowDispatchConfig.Inputs}} + {{range $item := .WorkflowDispatchInputs}}
{{if eq .Type "choice"}} @@ -19,12 +19,12 @@ {{else if eq .Type "boolean"}} {{else if eq .Type "number"}} - + {{else}} diff --git a/tests/integration/actions_dynamic_matrix_test.go b/tests/integration/actions_dynamic_matrix_test.go index 817708e8af8..8c9cf81ed12 100644 --- a/tests/integration/actions_dynamic_matrix_test.go +++ b/tests/integration/actions_dynamic_matrix_test.go @@ -22,16 +22,6 @@ import ( "github.com/stretchr/testify/require" ) -// TestDynamicMatrixEvaluation covers a job whose matrix references ${{ needs.*.outputs.* }}: it is -// planned as a single placeholder and expanded once its dependency completes. `build` exercises the -// expansion, `report` that a downstream job sees the combinations' outputs, `gated` and `partial` the -// `if:` gates on either side of it, and `static` that a matrix expanded at plan time is left alone. -// `included` covers the placeholder shapes that only survive as long as nothing re-parses their -// payload: `include:` is still a scalar there, which act refuses to read at all. -// `partial` and `strict` gate on `matrix.*` from either direction: neither may be decided before the -// combinations exist, or the whole job is skipped instead of the combinations the gate excludes. -// A full rerun then re-derives the matrix from the new attempt's outputs instead of reusing the -// previous combinations, keeping the AttemptJobID of every combination that recurs. func TestDynamicMatrixEvaluation(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) @@ -93,24 +83,6 @@ jobs: value: ${{ fromJson(needs.generate.outputs.matrix) }} steps: - run: echo "${{ matrix.value }}" - partial: - needs: [generate] - if: ${{ matrix.value != 1 }} - runs-on: ubuntu-latest - strategy: - matrix: - value: ${{ fromJson(needs.generate.outputs.matrix) }} - steps: - - run: echo "${{ matrix.value }}" - strict: - needs: [generate] - if: matrix.value == 1 - runs-on: ubuntu-latest - strategy: - matrix: - value: ${{ fromJson(needs.generate.outputs.matrix) }} - steps: - - run: echo "${{ matrix.value }}" report: needs: [build] runs-on: ubuntu-latest @@ -151,20 +123,12 @@ jobs: return names } - seen := execAttempt(t, 9) + seen := execAttempt(t, 7) firstAttemptIDs := maps.Clone(attemptJobIDs) - // `gated` is decided before the matrix is touched, so it never expands and is skipped as one job; - // `partial (1)` and `strict (2)` are decided afterwards, each against its own combination. assert.ElementsMatch(t, []string{ - "build (1)", "build (2)", "included (x)", "included (y)", - "static (a)", "static (b)", "partial (2)", "strict (1)", "report", + "build (1)", "build (2)", "included (x)", "included (y)", "static (a)", "static (b)", "report", }, seen) - for _, name := range []string{"partial (1)", "strict (2)"} { - skipped := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: run.ID, Name: name}) - assert.Equal(t, actions_model.StatusSkipped, skipped.Status) - } - // A gate reading `matrix.*` must not be decided against the unexpanded placeholder. - unittest.AssertNotExistsBean(t, &actions_model.ActionRunJob{RunID: run.ID, Name: "strict"}) + assert.Equal(t, actions_model.StatusSkipped, unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: run.ID, Name: "gated"}).Status) buildNeed, ok := reportTask.Needs["build"] require.True(t, ok, "report must see the expanded combinations as its 'build' need") @@ -186,10 +150,9 @@ jobs: }) // The matrix now yields a third value, while `static` is cloned as-is rather than collapsed. - seenRerun := execAttempt(t, 11) + seenRerun := execAttempt(t, 8) assert.ElementsMatch(t, []string{ - "build (1)", "build (2)", "build (3)", "included (x)", "included (y)", - "static (a)", "static (b)", "partial (2)", "partial (3)", "strict (1)", "report", + "build (1)", "build (2)", "build (3)", "included (x)", "included (y)", "static (a)", "static (b)", "report", }, seenRerun) for name, firstID := range firstAttemptIDs { assert.Equal(t, firstID, attemptJobIDs[name], "%s keeps its AttemptJobID across attempts", name) diff --git a/tests/integration/actions_invalid_workflow_test.go b/tests/integration/actions_invalid_workflow_test.go new file mode 100644 index 00000000000..91898a49306 --- /dev/null +++ b/tests/integration/actions_invalid_workflow_test.go @@ -0,0 +1,74 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "fmt" + "net/http" + "net/url" + "testing" + + actions_model "gitea.dev/models/actions" + auth_model "gitea.dev/models/auth" + "gitea.dev/models/db" + git_model "gitea.dev/models/git" + "gitea.dev/models/unittest" + user_model "gitea.dev/models/user" + "gitea.dev/modules/commitstatus" + "gitea.dev/modules/json" + "gitea.dev/routers/web/repo/actions" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestActionsInvalidWorkflowPush(t *testing.T) { + onGiteaRun(t, func(t *testing.T, _ *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + for _, testCase := range []struct { + name string + content string + wantErrors []string + }{ + {"expression", "on: push\nrun-name: '${{ github.ref'\njobs: {check: {if: unknown.x}}\n", []string{"Unrecognized named-value: 'unknown'", "unclosed expression"}}, + {"trigger", "on:\njobs: {check: {runs-on: ubuntu-latest, steps: [{run: echo hello}]}}\n", []string{"invalid event"}}, + } { + t.Run(testCase.name, func(t *testing.T) { + repo := createActionsTestRepo(t, token, "invalid-workflow-"+testCase.name, false) + createWorkflowFile(t, token, user.Name, repo.Name, ".gitea/workflows/invalid.yml", + getWorkflowCreateFileOptions(user, repo.DefaultBranch, "invalid workflow", testCase.content)) + + runs, err := db.Find[actions_model.ActionRun](t.Context(), actions_model.FindRunOptions{RepoID: repo.ID}) + require.NoError(t, err) + require.Len(t, runs, 1) + run := runs[0] + assert.Equal(t, actions_model.StatusFailure, run.Status) + + statuses, err := git_model.GetLatestCommitStatus(t.Context(), repo.ID, run.CommitSHA, db.ListOptionsAll) + require.NoError(t, err) + require.Len(t, statuses, 1) + assert.Equal(t, commitstatus.CommitStatusFailure, statuses[0].State) + + view := session.MakeRequest(t, NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d", user.Name, repo.Name, run.ID)), http.StatusOK) + var viewResponse actions.ViewResponse + require.NoError(t, json.Unmarshal(view.Body.Bytes(), &viewResponse)) + assert.Empty(t, viewResponse.State.Run.Jobs) + require.Len(t, viewResponse.State.Run.JobSummaries, 1) + assert.Equal(t, "invalid.yml", viewResponse.State.Run.JobSummaries[0].JobName) + assert.Contains(t, string(viewResponse.State.Run.JobSummaries[0].SummaryHTML), "Invalid workflow file: invalid.yml") + actionsPage := session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("/%s/%s/actions", user.Name, repo.Name)), http.StatusOK) + filePage := session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("/%s/%s/src/branch/%s/.gitea/workflows/invalid.yml", user.Name, repo.Name, repo.DefaultBranch)), http.StatusOK) + for _, wantError := range testCase.wantErrors { + assert.Contains(t, string(viewResponse.State.Run.JobSummaries[0].SummaryHTML), wantError) + assert.Contains(t, actionsPage.Body.String(), wantError) + assert.Contains(t, filePage.Body.String(), wantError) + } + + session.MakeRequest(t, NewRequest(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/rerun", user.Name, repo.Name, run.ID)), http.StatusBadRequest) + }) + } + }) +} diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index e3c458d31b2..8409719da9d 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -260,6 +260,7 @@ jobs: output_2: ${{ steps.gen_output.outputs.output_2 }} output_3: ${{ steps.gen_output.outputs.output_3 }} strategy: + fail-fast: false matrix: version: [1, 2, 3] steps: diff --git a/tests/integration/actions_reusable_workflow_test.go b/tests/integration/actions_reusable_workflow_test.go index 77b858ec342..e1de6a7ca94 100644 --- a/tests/integration/actions_reusable_workflow_test.go +++ b/tests/integration/actions_reusable_workflow_test.go @@ -14,7 +14,9 @@ import ( runnerv1 "gitea.dev/actionslib/runner/v1" actions_model "gitea.dev/models/actions" auth_model "gitea.dev/models/auth" + perm_model "gitea.dev/models/perm" repo_model "gitea.dev/models/repo" + "gitea.dev/models/unit" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" actions_module "gitea.dev/modules/actions" @@ -81,6 +83,8 @@ on: jobs: reusable1_job1: + permissions: + contents: write runs-on: ubuntu-latest steps: - run: echo 'reusable1_job1' @@ -135,6 +139,8 @@ jobs: caller_job2: needs: [caller_job1] + permissions: + contents: read uses: './.gitea/workflows/reusable1.yaml' with: str_input: 'from_caller_job2' @@ -204,6 +210,8 @@ jobs: _, r1Job1, _ := getTaskAndJobAndRunByTaskID(t, r1Job1Task.Id) assert.Equal(t, "reusable1_job1", r1Job1.JobID) assert.Equal(t, callerJob2ID, r1Job1.ParentJobID) + require.NotNil(t, r1Job1.TokenPermissions) + assert.Equal(t, perm_model.AccessModeRead, r1Job1.TokenPermissions.UnitAccessModes[unit.TypeCode]) payload := getWorkflowCallPayloadFromTask(t, r1Job1Task) if assert.Len(t, payload.Inputs, 5) { assert.Equal(t, "from_caller_job2", payload.Inputs["str_input"]) @@ -253,6 +261,8 @@ jobs: r1Job3AttemptJobID = r1Job3.AttemptJobID r2Job1 := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: runID, JobID: "reusable2_job1"}) assert.Equal(t, r1Job3ID, r2Job1.ParentJobID) + require.NotNil(t, r2Job1.TokenPermissions) + assert.Equal(t, perm_model.AccessModeRead, r2Job1.TokenPermissions.UnitAccessModes[unit.TypeCode]) r2Job1AttemptJobID = r2Job1.AttemptJobID r2Job1Task := defaultRunner.fetchTask(t) // for reusable2_job1 @@ -575,7 +585,7 @@ jobs: assert.Equal(t, 0, unittest.GetCount(t, &actions_model.ActionRun{RepoID: repo.ID})) }) - t.Run("Nested caller with missing callee fails instead of blocking", func(t *testing.T) { + t.Run("Nested caller with missing callee fails with the error as summary instead of blocking", func(t *testing.T) { // When the expansion hits a terminal error (e.g. missing callee), the emitter must fail the caller and let the run finish as failed, not retry the expansion forever. apiRepo := createActionsTestRepo(t, user2Token, "nested-caller-missing-callee", false) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID}) @@ -616,6 +626,9 @@ jobs: assert.Equal(t, actions_model.StatusFailure, finalRun.Status) runner.fetchNoTask(t) // no task scheduled for the failed caller; the run is not stuck + summary, err := actions_model.GetActionRunJobSummary(t.Context(), repo.ID, run.ID, badCaller.RunAttemptID, badCaller.ID, 0) + require.NoError(t, err) + assert.Contains(t, summary.Content, "does-not-exist.yml") }) t.Run("Fork PR with secrets: inherit does not leak base repo secrets", func(t *testing.T) {