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 <christopher.homberger@web.de>
Reviewed-on: https://code.forgejo.org/forgejo/act/pulls/180
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>

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)

act/jobparser/jobparser.go

@@ -11,8 +11,8 @@ import (
 	"github.com/nektos/act/pkg/model"
 )
 
-func Parse(content []byte, options ...ParseOption) ([]*SingleWorkflow, error) {
+func Parse(content []byte, validate bool, options ...ParseOption) ([]*SingleWorkflow, error) {
-	origin, err := model.ReadWorkflow(bytes.NewReader(content))
+	origin, err := model.ReadWorkflow(bytes.NewReader(content), validate)
 	if err != nil {
 		return nil, fmt.Errorf("model.ReadWorkflow: %w", err)
 	}

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)
 			}

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)

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)

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 {

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

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)

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
 		}

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 {

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)