throw when invalid uses key is provided

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,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, "./.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 '<org>/<repo>/.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

pkg/model/workflow_test.go

@@ -147,20 +147,81 @@ 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, 5)
+
+	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, 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, 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) {

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
 	}
 

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