From 5c4a96bcb797c7d013b878f1a3ceb74f9a834c64 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 7 Apr 2023 16:31:03 +0800 Subject: [PATCH] Avoid using `log.Fatal` in `pkg/*` (#39) Follow https://github.com/nektos/act/pull/1705 Reviewed-on: https://gitea.com/gitea/act/pulls/39 Reviewed-by: Lunny Xiao Co-authored-by: Jason Song Co-committed-by: Jason Song --- pkg/jobparser/jobparser.go | 15 +++++++--- pkg/model/workflow.go | 58 ++++++++++++++++++++++---------------- pkg/model/workflow_test.go | 16 ++++++++--- pkg/runner/runner.go | 10 +++++-- 4 files changed, 63 insertions(+), 36 deletions(-) diff --git a/pkg/jobparser/jobparser.go b/pkg/jobparser/jobparser.go index f062bce..cd84651 100644 --- a/pkg/jobparser/jobparser.go +++ b/pkg/jobparser/jobparser.go @@ -42,7 +42,11 @@ func Parse(content []byte, options ...ParseOption) ([]*SingleWorkflow, error) { } for i, id := range ids { job := jobs[i] - for _, matrix := range getMatrixes(origin.GetJob(id)) { + matricxes, err := getMatrixes(origin.GetJob(id)) + if err != nil { + return nil, fmt.Errorf("getMatrixes: %w", err) + } + for _, matrix := range matricxes { job := job.Clone() if job.Name == "" { job.Name = id @@ -89,12 +93,15 @@ type parseContext struct { type ParseOption func(c *parseContext) -func getMatrixes(job *model.Job) []map[string]interface{} { - ret := job.GetMatrixes() +func getMatrixes(job *model.Job) ([]map[string]interface{}, error) { + ret, err := job.GetMatrixes() + if err != nil { + return nil, fmt.Errorf("GetMatrixes: %w", err) + } sort.Slice(ret, func(i, j int) bool { return matrixName(ret[i]) < matrixName(ret[j]) }) - return ret + return ret, nil } func encodeMatrix(matrix map[string]interface{}) yaml.Node { diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index 38aed62..a77a688 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -271,15 +271,13 @@ func (j *Job) Container() *ContainerSpec { switch j.RawContainer.Kind { case yaml.ScalarNode: val = new(ContainerSpec) - err := j.RawContainer.Decode(&val.Image) - if err != nil { - log.Fatal(err) + if !decodeNode(j.RawContainer, &val.Image) { + return nil } case yaml.MappingNode: val = new(ContainerSpec) - err := j.RawContainer.Decode(val) - if err != nil { - log.Fatal(err) + if !decodeNode(j.RawContainer, val) { + return nil } } return val @@ -290,16 +288,14 @@ func (j *Job) Needs() []string { switch j.RawNeeds.Kind { case yaml.ScalarNode: var val string - err := j.RawNeeds.Decode(&val) - if err != nil { - log.Fatal(err) + if !decodeNode(j.RawNeeds, &val) { + return nil } return []string{val} case yaml.SequenceNode: var val []string - err := j.RawNeeds.Decode(&val) - if err != nil { - log.Fatal(err) + if !decodeNode(j.RawNeeds, &val) { + return nil } return val } @@ -311,16 +307,14 @@ func (j *Job) RunsOn() []string { switch j.RawRunsOn.Kind { case yaml.ScalarNode: var val string - err := j.RawRunsOn.Decode(&val) - if err != nil { - log.Fatal(err) + if !decodeNode(j.RawRunsOn, &val) { + return nil } return []string{val} case yaml.SequenceNode: var val []string - err := j.RawRunsOn.Decode(&val) - if err != nil { - log.Fatal(err) + if !decodeNode(j.RawRunsOn, &val) { + return nil } return val } @@ -330,8 +324,8 @@ func (j *Job) RunsOn() []string { func environment(yml yaml.Node) map[string]string { env := make(map[string]string) if yml.Kind == yaml.MappingNode { - if err := yml.Decode(&env); err != nil { - log.Fatal(err) + if !decodeNode(yml, &env) { + return nil } } return env @@ -346,8 +340,8 @@ func (j *Job) Environment() map[string]string { func (j *Job) Matrix() map[string][]interface{} { if j.Strategy.RawMatrix.Kind == yaml.MappingNode { var val map[string][]interface{} - if err := j.Strategy.RawMatrix.Decode(&val); err != nil { - log.Fatal(err) + if !decodeNode(j.Strategy.RawMatrix, &val) { + return nil } return val } @@ -358,7 +352,7 @@ func (j *Job) Matrix() map[string][]interface{} { // It skips includes and hard fails excludes for non-existing keys // //nolint:gocyclo -func (j *Job) GetMatrixes() []map[string]interface{} { +func (j *Job) GetMatrixes() ([]map[string]interface{}, error) { matrixes := make([]map[string]interface{}, 0) if j.Strategy != nil { j.Strategy.FailFast = j.Strategy.GetFailFast() @@ -409,7 +403,7 @@ func (j *Job) GetMatrixes() []map[string]interface{} { excludes = append(excludes, e) } else { // We fail completely here because that's what GitHub does for non-existing matrix keys, fail on exclude, silent skip on include - log.Fatalf("The workflow is not valid. Matrix exclude key '%s' does not match any key within the matrix", k) + return nil, fmt.Errorf("the workflow is not valid. Matrix exclude key %q does not match any key within the matrix", k) } } } @@ -454,7 +448,7 @@ func (j *Job) GetMatrixes() []map[string]interface{} { } else { matrixes = append(matrixes, make(map[string]interface{})) } - return matrixes + return matrixes, nil } func commonKeysMatch(a map[string]interface{}, b map[string]interface{}) bool { @@ -699,3 +693,17 @@ func (w *Workflow) GetJobIDs() []string { } return ids } + +var OnDecodeNodeError = func(node yaml.Node, out interface{}, err error) { + log.Fatalf("Failed to decode node %v into %T: %v", node, out, err) +} + +func decodeNode(node yaml.Node, out interface{}) bool { + if err := node.Decode(out); err != nil { + if OnDecodeNodeError != nil { + OnDecodeNodeError(node, out, err) + } + return false + } + return true +} diff --git a/pkg/model/workflow_test.go b/pkg/model/workflow_test.go index 5a0656d..3729add 100644 --- a/pkg/model/workflow_test.go +++ b/pkg/model/workflow_test.go @@ -332,25 +332,33 @@ func TestReadWorkflow_Strategy(t *testing.T) { wf := p.Stages[0].Runs[0].Workflow job := wf.Jobs["strategy-only-max-parallel"] - assert.Equal(t, job.GetMatrixes(), []map[string]interface{}{{}}) + matrixes, err := job.GetMatrixes() + assert.NoError(t, err) + assert.Equal(t, matrixes, []map[string]interface{}{{}}) assert.Equal(t, job.Matrix(), map[string][]interface{}(nil)) assert.Equal(t, job.Strategy.MaxParallel, 2) assert.Equal(t, job.Strategy.FailFast, true) job = wf.Jobs["strategy-only-fail-fast"] - assert.Equal(t, job.GetMatrixes(), []map[string]interface{}{{}}) + matrixes, err = job.GetMatrixes() + assert.NoError(t, err) + assert.Equal(t, matrixes, []map[string]interface{}{{}}) assert.Equal(t, job.Matrix(), map[string][]interface{}(nil)) assert.Equal(t, job.Strategy.MaxParallel, 4) assert.Equal(t, job.Strategy.FailFast, false) job = wf.Jobs["strategy-no-matrix"] - assert.Equal(t, job.GetMatrixes(), []map[string]interface{}{{}}) + matrixes, err = job.GetMatrixes() + assert.NoError(t, err) + assert.Equal(t, matrixes, []map[string]interface{}{{}}) assert.Equal(t, job.Matrix(), map[string][]interface{}(nil)) assert.Equal(t, job.Strategy.MaxParallel, 2) assert.Equal(t, job.Strategy.FailFast, false) job = wf.Jobs["strategy-all"] - assert.Equal(t, job.GetMatrixes(), + matrixes, err = job.GetMatrixes() + assert.NoError(t, err) + assert.Equal(t, matrixes, []map[string]interface{}{ {"datacenter": "site-c", "node-version": "14.x", "site": "staging"}, {"datacenter": "site-c", "node-version": "16.x", "site": "staging"}, diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index c0595e6..651d03e 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -7,11 +7,11 @@ import ( "os" "time" - log "github.com/sirupsen/logrus" - "github.com/nektos/act/pkg/common" "github.com/nektos/act/pkg/container" "github.com/nektos/act/pkg/model" + + log "github.com/sirupsen/logrus" ) // Runner provides capabilities to run GitHub actions @@ -136,7 +136,11 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor { log.Errorf("Error while evaluating matrix: %v", err) } } - matrixes := job.GetMatrixes() + matrixes, err := job.GetMatrixes() + if err != nil { + log.Errorf("Error while get job's matrix: %v", err) + // fall back to empty matrixes + } maxParallel := 4 if job.Strategy != nil { maxParallel = job.Strategy.MaxParallel