From e874540714e8beb236c001b5e9ea0791901e5572 Mon Sep 17 00:00:00 2001 From: Josh McCullough Date: Fri, 12 May 2023 20:31:52 -0400 Subject: [PATCH 1/9] throw if `uses` is invalid --- pkg/model/workflow.go | 19 +++++++++++++++---- pkg/model/workflow_test.go | 18 +++++++++++------- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index e7d43e4b338..c6a96ec5fc9 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -468,11 +468,22 @@ func (j JobType) String() string { // Type returns the type of the job func (j *Job) Type() JobType { - if strings.HasPrefix(j.Uses, "./.github/workflows") && (strings.HasSuffix(j.Uses, ".yml") || strings.HasSuffix(j.Uses, ".yaml")) { - return JobTypeReusableWorkflowLocal - } else if !strings.HasPrefix(j.Uses, "./") && strings.Contains(j.Uses, ".github/workflows") && (strings.Contains(j.Uses, ".yml@") || strings.Contains(j.Uses, ".yaml@")) { - return JobTypeReusableWorkflowRemote + isYaml, _ := regexp.MatchString(`\.(ya?ml)(?:$|@)`, j.Uses) + + if isYaml { + isLocalPath := strings.HasPrefix(j.Uses, "./.github/workflows/") + isRemotePath := strings.Contains(j.Uses, "/.github/workflows/") + hasVersion, _ := regexp.MatchString(`\.ya?ml@`, j.Uses) + + if isLocalPath { + return JobTypeReusableWorkflowLocal + } else if isRemotePath && hasVersion { + return JobTypeReusableWorkflowRemote + } else { + log.Fatalf("`uses` key references invalid workflow path '%s'. Must start with './.github/workflows/' if it's a local workflow, or must start with '//.github/workflows/' and include an '@' if it's a remote workflow.", j.Uses) + } } + return JobTypeDefault } diff --git a/pkg/model/workflow_test.go b/pkg/model/workflow_test.go index 8f8b7a9e4b3..e2b5f90effd 100644 --- a/pkg/model/workflow_test.go +++ b/pkg/model/workflow_test.go @@ -147,20 +147,24 @@ jobs: runs-on: ubuntu-latest steps: - run: echo - remote-reusable-workflow: - runs-on: ubuntu-latest + remote-reusable-workflow-yml: uses: remote/repo/.github/workflows/workflow.yml@main - local-reusable-workflow: - runs-on: ubuntu-latest + remote-reusable-workflow-yaml: + uses: remote/repo/.github/workflows/workflow.yaml@main + local-reusable-workflow-yml: uses: ./.github/workflows/workflow.yml + local-reusable-workflow-yaml: + uses: ./.github/workflows/workflow.yaml ` workflow, err := ReadWorkflow(strings.NewReader(yaml)) assert.NoError(t, err, "read workflow should succeed") - assert.Len(t, workflow.Jobs, 3) + assert.Len(t, workflow.Jobs, 5) assert.Equal(t, workflow.Jobs["default-job"].Type(), JobTypeDefault) - assert.Equal(t, workflow.Jobs["remote-reusable-workflow"].Type(), JobTypeReusableWorkflowRemote) - assert.Equal(t, workflow.Jobs["local-reusable-workflow"].Type(), JobTypeReusableWorkflowLocal) + assert.Equal(t, workflow.Jobs["remote-reusable-workflow-yml"].Type(), JobTypeReusableWorkflowRemote) + assert.Equal(t, workflow.Jobs["remote-reusable-workflow-yaml"].Type(), JobTypeReusableWorkflowRemote) + assert.Equal(t, workflow.Jobs["local-reusable-workflow-yml"].Type(), JobTypeReusableWorkflowLocal) + assert.Equal(t, workflow.Jobs["local-reusable-workflow-yaml"].Type(), JobTypeReusableWorkflowLocal) } func TestReadWorkflow_StepsTypes(t *testing.T) { From 79776a8586350a5cbb8a74dd29beac0a1540e6ab Mon Sep 17 00:00:00 2001 From: Josh McCullough Date: Sun, 14 May 2023 15:33:16 -0400 Subject: [PATCH 2/9] update JobType to return error --- pkg/model/workflow.go | 44 +++++++++++++++---------- pkg/model/workflow_test.go | 67 +++++++++++++++++++++++++++++++++++--- pkg/runner/run_context.go | 22 +++++++++---- pkg/runner/runner.go | 8 ++++- 4 files changed, 110 insertions(+), 31 deletions(-) diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index c6a96ec5fc9..7396db67940 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -1,6 +1,7 @@ package model import ( + "errors" "fmt" "io" "reflect" @@ -444,14 +445,17 @@ func commonKeysMatch2(a map[string]interface{}, b map[string]interface{}, m map[ type JobType int const ( - // StepTypeRun is all steps that have a `run` attribute + // JobTypeDefault is all jobs that have a `run` attribute JobTypeDefault JobType = iota - // StepTypeReusableWorkflowLocal is all steps that have a `uses` that is a local workflow in the .github/workflows directory + // JobTypeReusableWorkflowLocal is all jobs that have a `uses` that is a local workflow in the .github/workflows directory JobTypeReusableWorkflowLocal - // JobTypeReusableWorkflowRemote is all steps that have a `uses` that references a workflow file in a github repo + // JobTypeReusableWorkflowRemote is all jobs that have a `uses` that references a workflow file in a github repo JobTypeReusableWorkflowRemote + + // JobTypeInvalid represents a job which is not configured correctly + JobTypeInvalid ) func (j JobType) String() string { @@ -467,24 +471,28 @@ func (j JobType) String() string { } // Type returns the type of the job -func (j *Job) Type() JobType { - isYaml, _ := regexp.MatchString(`\.(ya?ml)(?:$|@)`, j.Uses) - - if isYaml { - isLocalPath := strings.HasPrefix(j.Uses, "./.github/workflows/") - isRemotePath := strings.Contains(j.Uses, "/.github/workflows/") - hasVersion, _ := regexp.MatchString(`\.ya?ml@`, j.Uses) - - if isLocalPath { - return JobTypeReusableWorkflowLocal - } else if isRemotePath && hasVersion { - return JobTypeReusableWorkflowRemote - } else { - log.Fatalf("`uses` key references invalid workflow path '%s'. Must start with './.github/workflows/' if it's a local workflow, or must start with '//.github/workflows/' and include an '@' if it's a remote workflow.", j.Uses) +func (j *Job) Type() (JobType, error) { + isReusable := j.Uses != "" + + if isReusable { + isYaml, _ := regexp.MatchString(`\.(ya?ml)(?:$|@)`, j.Uses) + + if isYaml { + isLocalPath := strings.HasPrefix(j.Uses, "./.github/workflows/") + isRemotePath := strings.Contains(j.Uses, "/.github/workflows/") + hasVersion, _ := regexp.MatchString(`\.ya?ml@`, j.Uses) + + if isLocalPath { + return JobTypeReusableWorkflowLocal, nil + } else if isRemotePath && hasVersion { + return JobTypeReusableWorkflowRemote, nil + } } + + return JobTypeInvalid, errors.New(fmt.Sprintf("`uses` key references invalid workflow path '%s'. Must start with './.github/workflows/' if it's a local workflow, or must start with '//.github/workflows/' and include an '@' if it's a remote workflow.", j.Uses)) } - return JobTypeDefault + return JobTypeDefault, nil } // ContainerSpec is the specification of the container to use for the job diff --git a/pkg/model/workflow_test.go b/pkg/model/workflow_test.go index e2b5f90effd..64a9b69cb86 100644 --- a/pkg/model/workflow_test.go +++ b/pkg/model/workflow_test.go @@ -160,11 +160,68 @@ jobs: workflow, err := ReadWorkflow(strings.NewReader(yaml)) assert.NoError(t, err, "read workflow should succeed") assert.Len(t, workflow.Jobs, 5) - assert.Equal(t, workflow.Jobs["default-job"].Type(), JobTypeDefault) - assert.Equal(t, workflow.Jobs["remote-reusable-workflow-yml"].Type(), JobTypeReusableWorkflowRemote) - assert.Equal(t, workflow.Jobs["remote-reusable-workflow-yaml"].Type(), JobTypeReusableWorkflowRemote) - assert.Equal(t, workflow.Jobs["local-reusable-workflow-yml"].Type(), JobTypeReusableWorkflowLocal) - assert.Equal(t, workflow.Jobs["local-reusable-workflow-yaml"].Type(), JobTypeReusableWorkflowLocal) + + job, err := workflow.Jobs["default-job"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeDefault, job) + + job, err = workflow.Jobs["remote-reusable-workflow-yml"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeReusableWorkflowRemote, job) + + job, err = workflow.Jobs["remote-reusable-workflow-yaml"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeReusableWorkflowRemote, job) + + job, err = workflow.Jobs["local-reusable-workflow-yml"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeReusableWorkflowLocal, job) + + job, err = workflow.Jobs["local-reusable-workflow-yaml"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeReusableWorkflowLocal, job) +} + +func TestReadWorkflow_JobTypes_InvalidPath(t *testing.T) { + yaml := ` +name: invalid job definition + +jobs: + remote-reusable-workflow-missing-version: + uses: remote/repo/.github/workflows/workflow.yml + remote-reusable-workflow-bad-extension: + uses: remote/repo/.github/workflows/workflow.json + remote-reusable-workflow-bad-path: + uses: remote/repo/github/workflows/workflow.yaml@main + local-reusable-workflow-bad-extension: + uses: ./.github/workflows/workflow.json + local-reusable-workflow-bad-path: + uses: ./.github/workflow/workflow.yaml +` + + workflow, err := ReadWorkflow(strings.NewReader(yaml)) + assert.NoError(t, err, "read workflow should succeed") + assert.Len(t, workflow.Jobs, 5) + + job, err := workflow.Jobs["remote-reusable-workflow-missing-version"].Type() + assert.Equal(t, JobTypeInvalid, job) + assert.NotEqual(t, nil, err) + + job, err = workflow.Jobs["remote-reusable-workflow-bad-extension"].Type() + assert.Equal(t, JobTypeInvalid, job) + assert.NotEqual(t, nil, err) + + job, err = workflow.Jobs["remote-reusable-workflow-bad-path"].Type() + assert.Equal(t, JobTypeInvalid, job) + assert.NotEqual(t, nil, err) + + job, err = workflow.Jobs["local-reusable-workflow-bad-extension"].Type() + assert.Equal(t, JobTypeInvalid, job) + assert.NotEqual(t, nil, err) + + job, err = workflow.Jobs["local-reusable-workflow-bad-path"].Type() + assert.Equal(t, JobTypeInvalid, job) + assert.NotEqual(t, nil, err) } func TestReadWorkflow_StepsTypes(t *testing.T) { diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index acffb76dc95..f6c1d39311d 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -451,16 +451,19 @@ func (rc *RunContext) steps() []*model.Step { } // Executor returns a pipeline executor for all the steps in the job -func (rc *RunContext) Executor() common.Executor { +func (rc *RunContext) Executor() (common.Executor, error) { var executor common.Executor + var jobType, err = rc.Run.Job().Type() - switch rc.Run.Job().Type() { + switch jobType { case model.JobTypeDefault: executor = newJobExecutor(rc, &stepFactoryImpl{}, rc) case model.JobTypeReusableWorkflowLocal: executor = newLocalReusableWorkflowExecutor(rc) case model.JobTypeReusableWorkflowRemote: executor = newRemoteReusableWorkflowExecutor(rc) + case model.JobTypeInvalid: + return nil, err } return func(ctx context.Context) error { @@ -472,7 +475,7 @@ func (rc *RunContext) Executor() common.Executor { return executor(ctx) } return nil - } + }, nil } func (rc *RunContext) containerImage(ctx context.Context) string { @@ -525,16 +528,21 @@ func (rc *RunContext) options(ctx context.Context) string { func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) { job := rc.Run.Job() l := common.Logger(ctx) - runJob, err := EvalBool(ctx, rc.ExprEval, job.If.Value, exprparser.DefaultStatusCheckSuccess) - if err != nil { - return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", job.If.Value, err) + runJob, runJobErr := EvalBool(ctx, rc.ExprEval, job.If.Value, exprparser.DefaultStatusCheckSuccess) + jobType, jobTypeErr := job.Type() + + if runJobErr != nil { + return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", job.If.Value, runJobErr) } + if !runJob { l.WithField("jobResult", "skipped").Debugf("Skipping job '%s' due to '%s'", job.Name, job.If.Value) return false, nil } - if job.Type() != model.JobTypeDefault { + if jobType == model.JobTypeInvalid { + return false, jobTypeErr + } else if jobType != model.JobTypeDefault { return true, nil } diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index a47cf8b19dc..bb9377d94b7 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -147,7 +147,13 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor { } stageExecutor = append(stageExecutor, func(ctx context.Context) error { jobName := fmt.Sprintf("%-*s", maxJobNameLen, rc.String()) - return rc.Executor()(common.WithJobErrorContainer(WithJobLogger(ctx, rc.Run.JobID, jobName, rc.Config, &rc.Masks, matrix))) + executor, err := rc.Executor() + + if err != nil { + return err + } + + return executor(common.WithJobErrorContainer(WithJobLogger(ctx, rc.Run.JobID, jobName, rc.Config, &rc.Masks, matrix))) }) } pipeline = append(pipeline, common.NewParallelExecutor(maxParallel, stageExecutor...)) From 5a95bfef0b7872e06ad7d156f4ff31a9c9294bbd Mon Sep 17 00:00:00 2001 From: Josh McCullough Date: Mon, 15 May 2023 17:13:05 -0400 Subject: [PATCH 3/9] lint --- pkg/model/workflow.go | 3 +-- pkg/model/workflow_test.go | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index 7396db67940..97030ec5472 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -1,7 +1,6 @@ package model import ( - "errors" "fmt" "io" "reflect" @@ -489,7 +488,7 @@ func (j *Job) Type() (JobType, error) { } } - return JobTypeInvalid, errors.New(fmt.Sprintf("`uses` key references invalid workflow path '%s'. Must start with './.github/workflows/' if it's a local workflow, or must start with '//.github/workflows/' and include an '@' if it's a remote workflow.", j.Uses)) + return JobTypeInvalid, fmt.Errorf("`uses` key references invalid workflow path '%s'. Must start with './.github/workflows/' if it's a local workflow, or must start with '//.github/workflows/' and include an '@' if it's a remote workflow.", j.Uses) } return JobTypeDefault, nil diff --git a/pkg/model/workflow_test.go b/pkg/model/workflow_test.go index 64a9b69cb86..167ffaefc2c 100644 --- a/pkg/model/workflow_test.go +++ b/pkg/model/workflow_test.go @@ -182,6 +182,7 @@ jobs: assert.Equal(t, JobTypeReusableWorkflowLocal, job) } +//nolint:dupl func TestReadWorkflow_JobTypes_InvalidPath(t *testing.T) { yaml := ` name: invalid job definition @@ -224,6 +225,7 @@ jobs: assert.NotEqual(t, nil, err) } +// nolint:dupl func TestReadWorkflow_StepsTypes(t *testing.T) { yaml := ` name: invalid step definition From b8b3c8ef3d0a989e0ac2f0ac82dc8aaa3b877ced Mon Sep 17 00:00:00 2001 From: Josh McCullough Date: Mon, 15 May 2023 17:25:32 -0400 Subject: [PATCH 4/9] put //nolint:dupl on wrong test --- pkg/model/workflow_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/model/workflow_test.go b/pkg/model/workflow_test.go index 167ffaefc2c..3809166645f 100644 --- a/pkg/model/workflow_test.go +++ b/pkg/model/workflow_test.go @@ -138,6 +138,7 @@ jobs: }) } +// nolint:dupl func TestReadWorkflow_JobTypes(t *testing.T) { yaml := ` name: invalid job definition @@ -225,7 +226,6 @@ jobs: assert.NotEqual(t, nil, err) } -// nolint:dupl func TestReadWorkflow_StepsTypes(t *testing.T) { yaml := ` name: invalid step definition From 0060436233eacd9f0234acf995c415d29c5025d8 Mon Sep 17 00:00:00 2001 From: Josh McCullough Date: Wed, 17 May 2023 08:22:38 -0400 Subject: [PATCH 5/9] update error message to remove end punctuation --- pkg/model/workflow.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index 97030ec5472..63aaf3c0eb8 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -488,7 +488,7 @@ func (j *Job) Type() (JobType, error) { } } - return JobTypeInvalid, fmt.Errorf("`uses` key references invalid workflow path '%s'. Must start with './.github/workflows/' if it's a local workflow, or must start with '//.github/workflows/' and include an '@' if it's a remote workflow.", j.Uses) + return JobTypeInvalid, fmt.Errorf("`uses` key references invalid workflow path '%s'. Must start with './.github/workflows/' if it's a local workflow, or must start with '//.github/workflows/' and include an '@' if it's a remote workflow", j.Uses) } return JobTypeDefault, nil From 919982c81f594825f44fa4559caef701005f0de8 Mon Sep 17 00:00:00 2001 From: Josh McCullough Date: Wed, 17 May 2023 08:22:44 -0400 Subject: [PATCH 6/9] lint --- pkg/model/workflow_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/model/workflow_test.go b/pkg/model/workflow_test.go index 3809166645f..8d1311ff1fe 100644 --- a/pkg/model/workflow_test.go +++ b/pkg/model/workflow_test.go @@ -138,7 +138,7 @@ jobs: }) } -// nolint:dupl +//nolint:dupl func TestReadWorkflow_JobTypes(t *testing.T) { yaml := ` name: invalid job definition From 7d005c12cb496e39a64dcd7ed66f32dc7ddd56b2 Mon Sep 17 00:00:00 2001 From: Josh McCullough Date: Sun, 21 May 2023 13:26:36 -0400 Subject: [PATCH 7/9] update remote job type check --- pkg/model/workflow.go | 6 ++-- pkg/model/workflow_test.go | 69 +++++++++++++++++++------------------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index 63aaf3c0eb8..85de55c6f22 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -477,8 +477,8 @@ func (j *Job) Type() (JobType, error) { isYaml, _ := regexp.MatchString(`\.(ya?ml)(?:$|@)`, j.Uses) if isYaml { - isLocalPath := strings.HasPrefix(j.Uses, "./.github/workflows/") - isRemotePath := strings.Contains(j.Uses, "/.github/workflows/") + isLocalPath := strings.HasPrefix(j.Uses, "./") + isRemotePath, _ := regexp.MatchString(`^[^.](.+?/){2,}.+\.ya?ml@`, j.Uses) hasVersion, _ := regexp.MatchString(`\.ya?ml@`, j.Uses) if isLocalPath { @@ -488,7 +488,7 @@ func (j *Job) Type() (JobType, error) { } } - return JobTypeInvalid, fmt.Errorf("`uses` key references invalid workflow path '%s'. Must start with './.github/workflows/' if it's a local workflow, or must start with '//.github/workflows/' and include an '@' if it's a remote workflow", j.Uses) + return JobTypeInvalid, fmt.Errorf("`uses` key references invalid workflow path '%s'. Must start with './' if it's a local workflow, or must start with '//' and include an '@' if it's a remote workflow", j.Uses) } return JobTypeDefault, nil diff --git a/pkg/model/workflow_test.go b/pkg/model/workflow_test.go index 8d1311ff1fe..00bf2aa50ed 100644 --- a/pkg/model/workflow_test.go +++ b/pkg/model/workflow_test.go @@ -149,80 +149,79 @@ jobs: steps: - run: echo remote-reusable-workflow-yml: - uses: remote/repo/.github/workflows/workflow.yml@main + uses: remote/repo/some/path/to/workflow.yml@main remote-reusable-workflow-yaml: - uses: remote/repo/.github/workflows/workflow.yaml@main + uses: remote/repo/some/path/to/workflow.yaml@main + remote-reusable-workflow-custom-path: + uses: remote/repo/path/to/workflow.yml@main local-reusable-workflow-yml: - uses: ./.github/workflows/workflow.yml + uses: ./some/path/to/workflow.yml local-reusable-workflow-yaml: - uses: ./.github/workflows/workflow.yaml + uses: ./some/path/to/workflow.yaml ` workflow, err := ReadWorkflow(strings.NewReader(yaml)) assert.NoError(t, err, "read workflow should succeed") - assert.Len(t, workflow.Jobs, 5) + assert.Len(t, workflow.Jobs, 6) - job, err := workflow.Jobs["default-job"].Type() + jobType, err := workflow.Jobs["default-job"].Type() assert.Equal(t, nil, err) - assert.Equal(t, JobTypeDefault, job) + assert.Equal(t, JobTypeDefault, jobType) - job, err = workflow.Jobs["remote-reusable-workflow-yml"].Type() + jobType, err = workflow.Jobs["remote-reusable-workflow-yml"].Type() assert.Equal(t, nil, err) - assert.Equal(t, JobTypeReusableWorkflowRemote, job) + assert.Equal(t, JobTypeReusableWorkflowRemote, jobType) - job, err = workflow.Jobs["remote-reusable-workflow-yaml"].Type() + jobType, err = workflow.Jobs["remote-reusable-workflow-yaml"].Type() assert.Equal(t, nil, err) - assert.Equal(t, JobTypeReusableWorkflowRemote, job) + assert.Equal(t, JobTypeReusableWorkflowRemote, jobType) - job, err = workflow.Jobs["local-reusable-workflow-yml"].Type() + jobType, err = workflow.Jobs["remote-reusable-workflow-custom-path"].Type() assert.Equal(t, nil, err) - assert.Equal(t, JobTypeReusableWorkflowLocal, job) + assert.Equal(t, JobTypeReusableWorkflowRemote, jobType) - job, err = workflow.Jobs["local-reusable-workflow-yaml"].Type() + jobType, err = workflow.Jobs["local-reusable-workflow-yml"].Type() assert.Equal(t, nil, err) - assert.Equal(t, JobTypeReusableWorkflowLocal, job) + assert.Equal(t, JobTypeReusableWorkflowLocal, jobType) + + jobType, err = workflow.Jobs["local-reusable-workflow-yaml"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeReusableWorkflowLocal, jobType) } -//nolint:dupl func TestReadWorkflow_JobTypes_InvalidPath(t *testing.T) { yaml := ` name: invalid job definition jobs: remote-reusable-workflow-missing-version: - uses: remote/repo/.github/workflows/workflow.yml + uses: remote/repo/some/path/to/workflow.yml remote-reusable-workflow-bad-extension: - uses: remote/repo/.github/workflows/workflow.json - remote-reusable-workflow-bad-path: - uses: remote/repo/github/workflows/workflow.yaml@main + uses: remote/repo/some/path/to/workflow.json local-reusable-workflow-bad-extension: - uses: ./.github/workflows/workflow.json + uses: ./some/path/to/workflow.json local-reusable-workflow-bad-path: - uses: ./.github/workflow/workflow.yaml + uses: some/path/to/workflow.yaml ` workflow, err := ReadWorkflow(strings.NewReader(yaml)) assert.NoError(t, err, "read workflow should succeed") - assert.Len(t, workflow.Jobs, 5) - - job, err := workflow.Jobs["remote-reusable-workflow-missing-version"].Type() - assert.Equal(t, JobTypeInvalid, job) - assert.NotEqual(t, nil, err) + assert.Len(t, workflow.Jobs, 4) - job, err = workflow.Jobs["remote-reusable-workflow-bad-extension"].Type() - assert.Equal(t, JobTypeInvalid, job) + jobType, err := workflow.Jobs["remote-reusable-workflow-missing-version"].Type() + assert.Equal(t, JobTypeInvalid, jobType) assert.NotEqual(t, nil, err) - job, err = workflow.Jobs["remote-reusable-workflow-bad-path"].Type() - assert.Equal(t, JobTypeInvalid, job) + jobType, err = workflow.Jobs["remote-reusable-workflow-bad-extension"].Type() + assert.Equal(t, JobTypeInvalid, jobType) assert.NotEqual(t, nil, err) - job, err = workflow.Jobs["local-reusable-workflow-bad-extension"].Type() - assert.Equal(t, JobTypeInvalid, job) + jobType, err = workflow.Jobs["local-reusable-workflow-bad-extension"].Type() + assert.Equal(t, JobTypeInvalid, jobType) assert.NotEqual(t, nil, err) - job, err = workflow.Jobs["local-reusable-workflow-bad-path"].Type() - assert.Equal(t, JobTypeInvalid, job) + jobType, err = workflow.Jobs["local-reusable-workflow-bad-path"].Type() + assert.Equal(t, JobTypeInvalid, jobType) assert.NotEqual(t, nil, err) } From 921167bc1622f04c715c4c1a56a3a1c2610f1774 Mon Sep 17 00:00:00 2001 From: Josh McCullough Date: Sun, 21 May 2023 13:26:49 -0400 Subject: [PATCH 8/9] move if statement --- pkg/runner/run_context.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index f6c1d39311d..6c8b8e9b1ff 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -535,17 +535,17 @@ func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) { return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", job.If.Value, runJobErr) } - if !runJob { - l.WithField("jobResult", "skipped").Debugf("Skipping job '%s' due to '%s'", job.Name, job.If.Value) - return false, nil - } - if jobType == model.JobTypeInvalid { return false, jobTypeErr } else if jobType != model.JobTypeDefault { return true, nil } + if !runJob { + l.WithField("jobResult", "skipped").Debugf("Skipping job '%s' due to '%s'", job.Name, job.If.Value) + return false, nil + } + img := rc.platformImage(ctx) if img == "" { if job.RunsOn() == nil { From bc336131911000a6704d41e8087f031c0ba36f91 Mon Sep 17 00:00:00 2001 From: Josh McCullough Date: Sun, 21 May 2023 13:27:48 -0400 Subject: [PATCH 9/9] rm nolint:dupl ... we'll see how that goes --- pkg/model/workflow_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/model/workflow_test.go b/pkg/model/workflow_test.go index 00bf2aa50ed..292c0bfffba 100644 --- a/pkg/model/workflow_test.go +++ b/pkg/model/workflow_test.go @@ -138,7 +138,6 @@ jobs: }) } -//nolint:dupl func TestReadWorkflow_JobTypes(t *testing.T) { yaml := ` name: invalid job definition