Split up and refactor isCustomTask

In this change, we clean up the `isCustomTask` function:
- Remove unused `context.Context`
- Remove the check that either `TaskRef` or `TaskSpec` is specified
because we check that way before - see [code].
- Define the check that `TaskRef` is a `CustomTask` as a member function
of `TaskRef`; and add tests for the member function.
- Define the check that `TaskSpec` is a `CustomTask` as a member function
of `TaskSpec`; and add tests for the member function.

There are no user-facing changes in this commit.

[code]: https://github.com/tektoncd/pipeline/blob/b7d815a9d8528547994f65da097050f472bbb8b2/pkg/apis/pipeline/v1beta1/pipeline_types.go#L216-L227

pkg/apis/pipeline/v1/pipeline_types.go

@@ -203,6 +203,10 @@ type PipelineTask struct {
 	Timeout *metav1.Duration `json:"timeout,omitempty"`
 }
 
+func (et *EmbeddedTask) IsCustomTask() bool {
+	return et != nil && et.APIVersion != "" && et.Kind != ""
+}
+
 // validateRefOrSpec validates at least one of taskRef or taskSpec is specified
 func (pt PipelineTask) validateRefOrSpec() (errs *apis.FieldError) {
 	// can't have both taskRef and taskSpec at the same time

pkg/apis/pipeline/v1/pipeline_types_test.go

@@ -965,3 +965,48 @@ func TestPipelineTask_IsMatrixed(t *testing.T) {
 		})
 	}
 }
+
+func TestEmbeddedTask_IsCustomTask(t *testing.T) {
+	tests := []struct {
+		name string
+		et   *EmbeddedTask
+		want bool
+	}{{
+		name: "not a custom task - apiVersion and kind are not set",
+		et:   &EmbeddedTask{},
+		want: false,
+	}, {
+		name: "not a custom task - apiVersion is not set",
+		et: &EmbeddedTask{
+			TypeMeta: runtime.TypeMeta{
+				Kind: "Example",
+			},
+		},
+		want: false,
+	}, {
+		name: "not a custom task - kind is not set",
+		et: &EmbeddedTask{
+			TypeMeta: runtime.TypeMeta{
+				Kind: "Example",
+			},
+		},
+		want: false,
+	}, {
+		name: "custom task",
+		et: &EmbeddedTask{
+			TypeMeta: runtime.TypeMeta{
+				Kind:       "Example",
+				APIVersion: "example/v0",
+			},
+		},
+		want: true,
+	},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.et.IsCustomTask(); got != tt.want {
+				t.Errorf("IsCustomTask() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/apis/pipeline/v1/taskref_types.go

@@ -40,3 +40,7 @@ const (
 	// NamespacedTaskKind indicates that the task type has a namespaced scope.
 	NamespacedTaskKind TaskKind = "Task"
 )
+
+func (tr *TaskRef) IsCustomTask() bool {
+	return tr != nil && tr.APIVersion != "" && tr.Kind != ""
+}

pkg/apis/pipeline/v1/taskref_types_test.go

@@ -0,0 +1,54 @@
+package v1
+
+import "testing"
+
+func TestTaskRef_IsCustomTask(t *testing.T) {
+	tests := []struct {
+		name string
+		tr   *TaskRef
+		want bool
+	}{{
+		name: "not a custom task - apiVersion and Kind are not set",
+		tr: &TaskRef{
+			Name: "foo",
+		},
+		want: false,
+	}, {
+		name: "not a custom task - apiVersion is not set",
+		tr: &TaskRef{
+			Name: "foo",
+			Kind: "Example",
+		},
+		want: false,
+	}, {
+		name: "not a custom task - kind is not set",
+		tr: &TaskRef{
+			Name:       "foo",
+			APIVersion: "example/v0",
+		},
+		want: false,
+	}, {
+		name: "custom task with name",
+		tr: &TaskRef{
+			Name:       "foo",
+			Kind:       "Example",
+			APIVersion: "example/v0",
+		},
+		want: true,
+	}, {
+		name: "custom task without name",
+		tr: &TaskRef{
+			Kind:       "Example",
+			APIVersion: "example/v0",
+		},
+		want: true,
+	},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.tr.IsCustomTask(); got != tt.want {
+				t.Errorf("IsCustomTask() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/apis/pipeline/v1beta1/pipeline_types.go

@@ -209,6 +209,10 @@ type PipelineTask struct {
 	Timeout *metav1.Duration `json:"timeout,omitempty"`
 }
 
+func (et *EmbeddedTask) IsCustomTask() bool {
+	return et != nil && et.APIVersion != "" && et.Kind != ""
+}
+
 // validateRefOrSpec validates at least one of taskRef or taskSpec is specified
 func (pt PipelineTask) validateRefOrSpec() (errs *apis.FieldError) {
 	// can't have both taskRef and taskSpec at the same time

pkg/apis/pipeline/v1beta1/pipeline_types_test.go

@@ -939,3 +939,48 @@ func TestPipelineTask_IsMatrixed(t *testing.T) {
 		})
 	}
 }
+
+func TestEmbeddedTask_IsCustomTask(t *testing.T) {
+	tests := []struct {
+		name string
+		et   *EmbeddedTask
+		want bool
+	}{{
+		name: "not a custom task - apiVersion and kind are not set",
+		et:   &EmbeddedTask{},
+		want: false,
+	}, {
+		name: "not a custom task - apiVersion is not set",
+		et: &EmbeddedTask{
+			TypeMeta: runtime.TypeMeta{
+				Kind: "Example",
+			},
+		},
+		want: false,
+	}, {
+		name: "not a custom task - kind is not set",
+		et: &EmbeddedTask{
+			TypeMeta: runtime.TypeMeta{
+				Kind: "Example",
+			},
+		},
+		want: false,
+	}, {
+		name: "custom task",
+		et: &EmbeddedTask{
+			TypeMeta: runtime.TypeMeta{
+				Kind:       "Example",
+				APIVersion: "example/v0",
+			},
+		},
+		want: true,
+	},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.et.IsCustomTask(); got != tt.want {
+				t.Errorf("IsCustomTask() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/apis/pipeline/v1beta1/taskref_types.go

@@ -49,3 +49,7 @@ const (
 	// ClusterTaskKind indicates that task type has a cluster scope.
 	ClusterTaskKind TaskKind = "ClusterTask"
 )
+
+func (tr *TaskRef) IsCustomTask() bool {
+	return tr != nil && tr.APIVersion != "" && tr.Kind != ""
+}

pkg/apis/pipeline/v1beta1/taskref_types_test.go

@@ -0,0 +1,54 @@
+package v1beta1
+
+import "testing"
+
+func TestTaskRef_IsCustomTask(t *testing.T) {
+	tests := []struct {
+		name string
+		tr   *TaskRef
+		want bool
+	}{{
+		name: "not a custom task - apiVersion and Kind are not set",
+		tr: &TaskRef{
+			Name: "foo",
+		},
+		want: false,
+	}, {
+		name: "not a custom task - apiVersion is not set",
+		tr: &TaskRef{
+			Name: "foo",
+			Kind: "Example",
+		},
+		want: false,
+	}, {
+		name: "not a custom task - kind is not set",
+		tr: &TaskRef{
+			Name:       "foo",
+			APIVersion: "example/v0",
+		},
+		want: false,
+	}, {
+		name: "custom task with name",
+		tr: &TaskRef{
+			Name:       "foo",
+			Kind:       "Example",
+			APIVersion: "example/v0",
+		},
+		want: true,
+	}, {
+		name: "custom task without name",
+		tr: &TaskRef{
+			Kind:       "Example",
+			APIVersion: "example/v0",
+		},
+		want: true,
+	},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.tr.IsCustomTask(); got != tt.want {
+				t.Errorf("IsCustomTask() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go

@@ -574,15 +574,6 @@ func ValidateTaskRunSpecs(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) erro
 	return nil
 }
 
-func isCustomTask(ctx context.Context, rpt ResolvedPipelineTask) bool {
-	invalidSpec := rpt.PipelineTask.TaskRef != nil && rpt.PipelineTask.TaskSpec != nil
-	isTaskRefCustomTask := rpt.PipelineTask.TaskRef != nil && rpt.PipelineTask.TaskRef.APIVersion != "" &&
-		rpt.PipelineTask.TaskRef.Kind != ""
-	isTaskSpecCustomTask := rpt.PipelineTask.TaskSpec != nil && rpt.PipelineTask.TaskSpec.APIVersion != "" &&
-		rpt.PipelineTask.TaskSpec.Kind != ""
-	return !invalidSpec && (isTaskRefCustomTask || isTaskSpecCustomTask)
-}
-
 // ResolvePipelineTask retrieves a single Task's instance using the getTask to fetch
 // the spec. If it is unable to retrieve an instance of a referenced Task, it  will return
 // an error, otherwise it returns a list of all the Tasks retrieved.  It will retrieve
@@ -598,7 +589,7 @@ func ResolvePipelineTask(
 	rpt := ResolvedPipelineTask{
 		PipelineTask: &pipelineTask,
 	}
-	rpt.CustomTask = isCustomTask(ctx, rpt)
+	rpt.CustomTask = rpt.PipelineTask.TaskRef.IsCustomTask() || rpt.PipelineTask.TaskSpec.IsCustomTask()
 	switch {
 	case rpt.IsCustomTask() && rpt.PipelineTask.IsMatrixed():
 		rpt.RunObjectNames = getNamesOfRuns(pipelineRun.Status.ChildReferences, pipelineTask.Name, pipelineRun.Name, pipelineTask.Matrix.CountCombinations())

pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go

@@ -2284,21 +2284,6 @@ func TestIsCustomTask(t *testing.T) {
 			},
 		},
 		want: true,
-	}, {
-		name: "custom both taskRef and taskSpec",
-		pt: v1beta1.PipelineTask{
-			TaskRef: &v1beta1.TaskRef{
-				APIVersion: "example.dev/v0",
-				Kind:       "Sample",
-			},
-			TaskSpec: &v1beta1.EmbeddedTask{
-				TypeMeta: runtime.TypeMeta{
-					APIVersion: "example.dev/v0",
-					Kind:       "Sample",
-				},
-			},
-		},
-		want: false,
 	}, {
 		name: "custom taskRef missing kind",
 		pt: v1beta1.PipelineTask{