diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index 19edfb4..4fd9be9 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -445,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 { @@ -468,13 +471,28 @@ 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 +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, "./") + isRemotePath, _ := regexp.MatchString(`^[^.](.+?/){2,}.+\.ya?ml@`, j.Uses) + hasVersion, _ := regexp.MatchString(`\.ya?ml@`, j.Uses) + + if isLocalPath { + return JobTypeReusableWorkflowLocal, nil + } else if isRemotePath && hasVersion { + return JobTypeReusableWorkflowRemote, nil + } + } + + 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 + + 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 8f8b7a9..292c0bf 100644 --- a/pkg/model/workflow_test.go +++ b/pkg/model/workflow_test.go @@ -147,20 +147,81 @@ jobs: runs-on: ubuntu-latest steps: - run: echo - remote-reusable-workflow: - runs-on: ubuntu-latest - uses: remote/repo/.github/workflows/workflow.yml@main - local-reusable-workflow: - runs-on: ubuntu-latest - uses: ./.github/workflows/workflow.yml + remote-reusable-workflow-yml: + uses: remote/repo/some/path/to/workflow.yml@main + remote-reusable-workflow-yaml: + 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: ./some/path/to/workflow.yml + local-reusable-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, 3) - 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.Len(t, workflow.Jobs, 6) + + jobType, err := workflow.Jobs["default-job"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeDefault, jobType) + + jobType, err = workflow.Jobs["remote-reusable-workflow-yml"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeReusableWorkflowRemote, jobType) + + jobType, err = workflow.Jobs["remote-reusable-workflow-yaml"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeReusableWorkflowRemote, jobType) + + jobType, err = workflow.Jobs["remote-reusable-workflow-custom-path"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeReusableWorkflowRemote, jobType) + + jobType, err = workflow.Jobs["local-reusable-workflow-yml"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeReusableWorkflowLocal, jobType) + + jobType, err = workflow.Jobs["local-reusable-workflow-yaml"].Type() + assert.Equal(t, nil, err) + assert.Equal(t, JobTypeReusableWorkflowLocal, jobType) +} + +func TestReadWorkflow_JobTypes_InvalidPath(t *testing.T) { + yaml := ` +name: invalid job definition + +jobs: + remote-reusable-workflow-missing-version: + uses: remote/repo/some/path/to/workflow.yml + remote-reusable-workflow-bad-extension: + uses: remote/repo/some/path/to/workflow.json + local-reusable-workflow-bad-extension: + uses: ./some/path/to/workflow.json + local-reusable-workflow-bad-path: + 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, 4) + + jobType, err := workflow.Jobs["remote-reusable-workflow-missing-version"].Type() + assert.Equal(t, JobTypeInvalid, jobType) + assert.NotEqual(t, nil, err) + + jobType, err = workflow.Jobs["remote-reusable-workflow-bad-extension"].Type() + assert.Equal(t, JobTypeInvalid, jobType) + assert.NotEqual(t, nil, err) + + jobType, err = workflow.Jobs["local-reusable-workflow-bad-extension"].Type() + assert.Equal(t, JobTypeInvalid, jobType) + assert.NotEqual(t, nil, err) + + jobType, err = workflow.Jobs["local-reusable-workflow-bad-path"].Type() + assert.Equal(t, JobTypeInvalid, jobType) + 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 fbc20a5..bcd36a8 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -454,16 +454,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 { @@ -475,7 +478,7 @@ func (rc *RunContext) Executor() common.Executor { return executor(ctx) } return nil - } + }, nil } func (rc *RunContext) containerImage(ctx context.Context) string { @@ -528,19 +531,24 @@ func (rc *RunContext) options(_ 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 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 } - if job.Type() != model.JobTypeDefault { - return true, nil - } - img := rc.platformImage(ctx) if img == "" { if job.RunsOn() == nil { diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index aad2541..ecb70e7 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -181,7 +181,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...))