From 13ed94f5b7aceca07f9a0e0b17e0b084da4f349a Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Wed, 16 Jul 2025 08:46:36 +0000 Subject: [PATCH] feat!: add the validate argument to reading workflows (#180) This is a followup of https://code.forgejo.org/forgejo/act/pulls/170 so that it is possible to read a workflow without validation. It is not uncommon for Forgejo to read a workflow just to extract a few information from it, knowing it has been validated before. It would be a performance regression if schema validation happened in these cases. This is a port of https://github.com/nektos/act/pull/2717/files It is a breaking change in the context of Forgejo and Forgejo runner because it will need to add the new `validate` argument when reading workflows. Co-authored-by: ChristopherHX Reviewed-on: https://code.forgejo.org/forgejo/act/pulls/180 Reviewed-by: Michael Kriese Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- act/artifacts/server_test.go | 2 +- act/jobparser/jobparser.go | 4 +-- act/jobparser/jobparser_test.go | 2 +- act/jobparser/model_test.go | 9 +++--- act/model/planner.go | 6 ++-- act/model/planner_test.go | 2 +- act/model/workflow.go | 11 +++++-- act/model/workflow_test.go | 52 ++++++++++++++++----------------- act/runner/reusable_workflow.go | 2 +- act/runner/run_context_test.go | 2 +- act/runner/runner_test.go | 16 +++++----- 11 files changed, 57 insertions(+), 51 deletions(-) diff --git a/act/artifacts/server_test.go b/act/artifacts/server_test.go index aeeb0598..9dec551e 100644 --- a/act/artifacts/server_test.go +++ b/act/artifacts/server_test.go @@ -297,7 +297,7 @@ func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) { runner, err := runner.New(runnerConfig) assert.Nil(t, err, tjfi.workflowPath) - planner, err := model.NewWorkflowPlanner(fullWorkflowPath, true) + planner, err := model.NewWorkflowPlanner(fullWorkflowPath, true, false) assert.Nil(t, err, fullWorkflowPath) plan, err := planner.PlanEvent(tjfi.eventName) diff --git a/act/jobparser/jobparser.go b/act/jobparser/jobparser.go index b8c206a3..437a3317 100644 --- a/act/jobparser/jobparser.go +++ b/act/jobparser/jobparser.go @@ -11,8 +11,8 @@ import ( "github.com/nektos/act/pkg/model" ) -func Parse(content []byte, options ...ParseOption) ([]*SingleWorkflow, error) { - origin, err := model.ReadWorkflow(bytes.NewReader(content)) +func Parse(content []byte, validate bool, options ...ParseOption) ([]*SingleWorkflow, error) { + origin, err := model.ReadWorkflow(bytes.NewReader(content), validate) if err != nil { return nil, fmt.Errorf("model.ReadWorkflow: %w", err) } diff --git a/act/jobparser/jobparser_test.go b/act/jobparser/jobparser_test.go index 5ecf9934..02b58def 100644 --- a/act/jobparser/jobparser_test.go +++ b/act/jobparser/jobparser_test.go @@ -47,7 +47,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, false, tt.options...) if tt.wantErr { require.Error(t, err) } diff --git a/act/jobparser/model_test.go b/act/jobparser/model_test.go index 45f8e9b2..7c73172b 100644 --- a/act/jobparser/model_test.go +++ b/act/jobparser/model_test.go @@ -208,7 +208,7 @@ func TestParseRawOn(t *testing.T) { } for _, kase := range kases { t.Run(kase.input, func(t *testing.T) { - origin, err := model.ReadWorkflow(strings.NewReader(kase.input)) + origin, err := model.ReadWorkflow(strings.NewReader(kase.input), false) assert.NoError(t, err) events, err := ParseRawOn(&origin.RawOn) @@ -222,7 +222,7 @@ func TestSingleWorkflow_SetJob(t *testing.T) { t.Run("erase needs", func(t *testing.T) { content := ReadTestdata(t, "erase_needs.in.yaml") want := ReadTestdata(t, "erase_needs.out.yaml") - swf, err := Parse(content) + swf, err := Parse(content, false) require.NoError(t, err) builder := &strings.Builder{} for _, v := range swf { @@ -249,8 +249,7 @@ func TestParseMappingNode(t *testing.T) { { input: "on:\n push:\n branches:\n - master", scalars: []string{"push"}, - datas: []interface { - }{ + datas: []interface{}{ map[string]interface{}{ "branches": []interface{}{"master"}, }, @@ -313,7 +312,7 @@ func TestParseMappingNode(t *testing.T) { for _, test := range tests { t.Run(test.input, func(t *testing.T) { - workflow, err := model.ReadWorkflow(strings.NewReader(test.input)) + workflow, err := model.ReadWorkflow(strings.NewReader(test.input), false) assert.NoError(t, err) scalars, datas, err := parseMappingNode[interface{}](&workflow.RawOn) diff --git a/act/model/planner.go b/act/model/planner.go index 71d0ea36..6ea5f753 100644 --- a/act/model/planner.go +++ b/act/model/planner.go @@ -56,7 +56,7 @@ type WorkflowFiles struct { } // NewWorkflowPlanner will load a specific workflow, all workflows from a directory or all workflows from a directory and its subdirectories -func NewWorkflowPlanner(path string, noWorkflowRecurse bool) (WorkflowPlanner, error) { +func NewWorkflowPlanner(path string, noWorkflowRecurse, validate bool) (WorkflowPlanner, error) { path, err := filepath.Abs(path) if err != nil { return nil, err @@ -127,7 +127,7 @@ func NewWorkflowPlanner(path string, noWorkflowRecurse bool) (WorkflowPlanner, e } log.Debugf("Reading workflow '%s'", f.Name()) - workflow, err := ReadWorkflow(f) + workflow, err := ReadWorkflow(f, validate) if err != nil { _ = f.Close() if err == io.EOF { @@ -171,7 +171,7 @@ func NewSingleWorkflowPlanner(name string, f io.Reader) (WorkflowPlanner, error) wp := new(workflowPlanner) log.Debugf("Reading workflow %s", name) - workflow, err := ReadWorkflow(f) + workflow, err := ReadWorkflow(f, false) if err != nil { if err == io.EOF { return nil, fmt.Errorf("unable to read workflow '%s': file is empty: %w", name, err) diff --git a/act/model/planner_test.go b/act/model/planner_test.go index 2857c2c8..57e7443f 100644 --- a/act/model/planner_test.go +++ b/act/model/planner_test.go @@ -31,7 +31,7 @@ func TestPlanner(t *testing.T) { assert.NoError(t, err, workdir) for _, table := range tables { fullWorkflowPath := filepath.Join(workdir, table.workflowPath) - _, err = NewWorkflowPlanner(fullWorkflowPath, table.noWorkflowRecurse) + _, err = NewWorkflowPlanner(fullWorkflowPath, table.noWorkflowRecurse, false) if table.errorMessage == "" { assert.NoError(t, err, "WorkflowPlanner should exit without any error") } else { diff --git a/act/model/workflow.go b/act/model/workflow.go index f71e636c..815124bb 100644 --- a/act/model/workflow.go +++ b/act/model/workflow.go @@ -94,7 +94,9 @@ func (w *Workflow) OnSchedule() []string { return []string{} } -func (w *Workflow) UnmarshalYAML(node *yaml.Node) error { +type WorkflowValidate Workflow + +func (w *WorkflowValidate) UnmarshalYAML(node *yaml.Node) error { // Validate the schema before deserializing it into our model if err := (&schema.Node{ Definition: "workflow-root", @@ -733,7 +735,12 @@ func (s *Step) Type() StepType { } // ReadWorkflow returns a list of jobs for a given workflow file reader -func ReadWorkflow(in io.Reader) (*Workflow, error) { +func ReadWorkflow(in io.Reader, validate bool) (*Workflow, error) { + if validate { + w := new(WorkflowValidate) + err := yaml.NewDecoder(in).Decode(w) + return (*Workflow)(w), err + } w := new(Workflow) err := yaml.NewDecoder(in).Decode(w) return w, err diff --git a/act/model/workflow_test.go b/act/model/workflow_test.go index cb92250e..adb61d1b 100644 --- a/act/model/workflow_test.go +++ b/act/model/workflow_test.go @@ -23,7 +23,7 @@ jobs: - uses: ./actions/docker-url ` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") schedules := workflow.OnEvent("schedule") @@ -48,7 +48,7 @@ jobs: - uses: ./actions/docker-url ` - workflow, err = ReadWorkflow(strings.NewReader(yaml)) + workflow, err = ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") newSchedules = workflow.OnSchedule() @@ -66,7 +66,7 @@ jobs: - uses: ./actions/docker-url ` - workflow, err = ReadWorkflow(strings.NewReader(yaml)) + workflow, err = ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") newSchedules = workflow.OnSchedule() @@ -83,7 +83,7 @@ jobs: - uses: ./actions/docker-url ` - workflow, err = ReadWorkflow(strings.NewReader(yaml)) + workflow, err = ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") newSchedules = workflow.OnSchedule() @@ -102,7 +102,7 @@ jobs: - uses: ./actions/docker-url ` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") assert.Len(t, workflow.On(), 1) @@ -146,7 +146,7 @@ jobs: - run: echo hi `, testCase.snippet) - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), true) if testCase.err != "" { assert.ErrorContains(t, err, testCase.err) } else { @@ -172,7 +172,7 @@ jobs: - uses: ./actions/docker-url ` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") assert.Len(t, workflow.On(), 2) @@ -198,7 +198,7 @@ jobs: - uses: ./actions/docker-url ` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") assert.Len(t, workflow.On(), 2) assert.Contains(t, workflow.On(), "push") @@ -219,7 +219,7 @@ jobs: foo: {{ a }} ` - _, err := ReadWorkflow(strings.NewReader(yaml)) + _, err := ReadWorkflow(strings.NewReader(yaml), true) assert.ErrorContains(t, err, "Line: 11 Column 16: Expected a scalar got mapping") } @@ -235,7 +235,7 @@ jobs: steps: - uses: ./actions/docker-url` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") assert.Equal(t, workflow.Jobs["test"].RunsOn(), []string{"ubuntu-latest"}) } @@ -253,7 +253,7 @@ jobs: steps: - uses: ./actions/docker-url` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") assert.Equal(t, workflow.Jobs["test"].RunsOn(), []string{"ubuntu-latest", "linux"}) } @@ -278,7 +278,7 @@ jobs: - uses: ./actions/docker-url ` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") assert.Len(t, workflow.Jobs, 2) assert.Contains(t, workflow.Jobs["test"].Container().Image, "nginx:latest") @@ -308,7 +308,7 @@ jobs: - uses: ./actions/docker-url ` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") assert.Len(t, workflow.Jobs, 1) @@ -346,7 +346,7 @@ jobs: uses: ./some/path/to/workflow.yaml ` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") assert.Len(t, workflow.Jobs, 6) @@ -390,7 +390,7 @@ jobs: uses: some/path/to/workflow.yaml ` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") assert.Len(t, workflow.Jobs, 4) @@ -432,7 +432,7 @@ jobs: uses: ./local-action ` - _, err := ReadWorkflow(strings.NewReader(yaml)) + _, err := ReadWorkflow(strings.NewReader(yaml), true) assert.Error(t, err, "read workflow should fail") } @@ -464,7 +464,7 @@ jobs: echo "${{ needs.test1.outputs.some-b-key }}" ` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") assert.Len(t, workflow.Jobs, 2) @@ -479,7 +479,7 @@ jobs: } func TestReadWorkflow_Strategy(t *testing.T) { - w, err := NewWorkflowPlanner("testdata/strategy/push.yml", true) + w, err := NewWorkflowPlanner("testdata/strategy/push.yml", true, false) assert.NoError(t, err) p, err := w.PlanJob("strategy-only-max-parallel") @@ -569,7 +569,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) { yaml := ` name: local-action-docker-url ` - workflow, err := ReadWorkflow(strings.NewReader(yaml)) + workflow, err := ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") workflowDispatch := workflow.WorkflowDispatchConfig() assert.Nil(t, workflowDispatch) @@ -578,7 +578,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) { name: local-action-docker-url on: push ` - workflow, err = ReadWorkflow(strings.NewReader(yaml)) + workflow, err = ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") workflowDispatch = workflow.WorkflowDispatchConfig() assert.Nil(t, workflowDispatch) @@ -587,7 +587,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) { name: local-action-docker-url on: workflow_dispatch ` - workflow, err = ReadWorkflow(strings.NewReader(yaml)) + workflow, err = ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") workflowDispatch = workflow.WorkflowDispatchConfig() assert.NotNil(t, workflowDispatch) @@ -597,7 +597,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) { name: local-action-docker-url on: [push, pull_request] ` - workflow, err = ReadWorkflow(strings.NewReader(yaml)) + workflow, err = ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") workflowDispatch = workflow.WorkflowDispatchConfig() assert.Nil(t, workflowDispatch) @@ -606,7 +606,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) { name: local-action-docker-url on: [push, workflow_dispatch] ` - workflow, err = ReadWorkflow(strings.NewReader(yaml)) + workflow, err = ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") workflowDispatch = workflow.WorkflowDispatchConfig() assert.NotNil(t, workflowDispatch) @@ -618,7 +618,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) { - push - workflow_dispatch ` - workflow, err = ReadWorkflow(strings.NewReader(yaml)) + workflow, err = ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") workflowDispatch = workflow.WorkflowDispatchConfig() assert.NotNil(t, workflowDispatch) @@ -630,7 +630,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) { push: pull_request: ` - workflow, err = ReadWorkflow(strings.NewReader(yaml)) + workflow, err = ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") workflowDispatch = workflow.WorkflowDispatchConfig() assert.Nil(t, workflowDispatch) @@ -652,7 +652,7 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) { - warning - debug ` - workflow, err = ReadWorkflow(strings.NewReader(yaml)) + workflow, err = ReadWorkflow(strings.NewReader(yaml), false) assert.NoError(t, err, "read workflow should succeed") workflowDispatch = workflow.WorkflowDispatchConfig() assert.NotNil(t, workflowDispatch) diff --git a/act/runner/reusable_workflow.go b/act/runner/reusable_workflow.go index 9d873653..312d4150 100644 --- a/act/runner/reusable_workflow.go +++ b/act/runner/reusable_workflow.go @@ -152,7 +152,7 @@ func cloneIfRequired(rc *RunContext, remoteReusableWorkflow remoteReusableWorkfl func newReusableWorkflowExecutor(rc *RunContext, directory string, workflow string) common.Executor { return func(ctx context.Context) error { - planner, err := model.NewWorkflowPlanner(path.Join(directory, workflow), true) + planner, err := model.NewWorkflowPlanner(path.Join(directory, workflow), true, false) if err != nil { return err } diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index 19993f08..48608684 100644 --- a/act/runner/run_context_test.go +++ b/act/runner/run_context_test.go @@ -738,7 +738,7 @@ jobs: steps: - run: echo ok ` - workflow, err := model.ReadWorkflow(strings.NewReader(yaml)) + workflow, err := model.ReadWorkflow(strings.NewReader(yaml), true) require.NoError(t, err) testCases := []struct { diff --git a/act/runner/runner_test.go b/act/runner/runner_test.go index 6be2a9af..303b6c08 100644 --- a/act/runner/runner_test.go +++ b/act/runner/runner_test.go @@ -52,7 +52,7 @@ func init() { } func TestNoWorkflowsFoundByPlanner(t *testing.T) { - planner, err := model.NewWorkflowPlanner("res", true) + planner, err := model.NewWorkflowPlanner("res", true, false) assert.NoError(t, err) out := log.StandardLogger().Out @@ -72,7 +72,7 @@ func TestNoWorkflowsFoundByPlanner(t *testing.T) { } func TestGraphMissingEvent(t *testing.T) { - planner, err := model.NewWorkflowPlanner("testdata/issue-1595/no-event.yml", true) + planner, err := model.NewWorkflowPlanner("testdata/issue-1595/no-event.yml", true, false) assert.NoError(t, err) out := log.StandardLogger().Out @@ -90,7 +90,7 @@ func TestGraphMissingEvent(t *testing.T) { } func TestGraphMissingFirst(t *testing.T) { - planner, err := model.NewWorkflowPlanner("testdata/issue-1595/no-first.yml", true) + planner, err := model.NewWorkflowPlanner("testdata/issue-1595/no-first.yml", true, false) assert.NoError(t, err) plan, err := planner.PlanEvent("push") @@ -100,7 +100,7 @@ func TestGraphMissingFirst(t *testing.T) { } func TestGraphWithMissing(t *testing.T) { - planner, err := model.NewWorkflowPlanner("testdata/issue-1595/missing.yml", true) + planner, err := model.NewWorkflowPlanner("testdata/issue-1595/missing.yml", true, false) assert.NoError(t, err) out := log.StandardLogger().Out @@ -119,7 +119,7 @@ func TestGraphWithMissing(t *testing.T) { func TestGraphWithSomeMissing(t *testing.T) { log.SetLevel(log.DebugLevel) - planner, err := model.NewWorkflowPlanner("testdata/issue-1595/", true) + planner, err := model.NewWorkflowPlanner("testdata/issue-1595/", true, false) assert.NoError(t, err) out := log.StandardLogger().Out @@ -137,7 +137,7 @@ func TestGraphWithSomeMissing(t *testing.T) { } func TestGraphEvent(t *testing.T) { - planner, err := model.NewWorkflowPlanner("testdata/basic", true) + planner, err := model.NewWorkflowPlanner("testdata/basic", true, false) assert.NoError(t, err) plan, err := planner.PlanEvent("push") @@ -196,7 +196,7 @@ func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config runner, err := New(runnerConfig) assert.Nil(t, err, j.workflowPath) - planner, err := model.NewWorkflowPlanner(fullWorkflowPath, true) + planner, err := model.NewWorkflowPlanner(fullWorkflowPath, true, false) if err != nil { assert.Error(t, err, j.errorMessage) } else { @@ -659,7 +659,7 @@ func TestRunWithService(t *testing.T) { runner, err := New(runnerConfig) assert.NoError(t, err, workflowPath) - planner, err := model.NewWorkflowPlanner(fmt.Sprintf("testdata/%s", workflowPath), true) + planner, err := model.NewWorkflowPlanner(fmt.Sprintf("testdata/%s", workflowPath), true, false) assert.NoError(t, err, workflowPath) plan, err := planner.PlanEvent(eventName)