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.
- Update the docstring for `Kind` and `APIVersion` in `TaskRef`.

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

Author: jerop

docs/pipeline-api.md

@@ -4487,7 +4487,10 @@ TaskKind
 </em>
 </td>
 <td>
-<p>TaskKind indicates the kind of the task, namespaced or cluster scoped.</p>
+<p>TaskKind indicates the Kind of the Task:
+1. Namespaced Task when Kind is set to &ldquo;Task&rdquo;
+2. Cluster-Scoped Task when Kind is set to &ldquo;ClusterTask&rdquo;
+3. Custom Task when Kind is non-empty and APIVersion is non-empty</p>
 </td>
 </tr>
 <tr>
@@ -4499,7 +4502,8 @@ string
 </td>
 <td>
 <em>(Optional)</em>
-<p>API version of the referent</p>
+<p>API version of the referent
+Note: A Task with non-empty APIVersion and Kind is considered a Custom Task</p>
 </td>
 </tr>
 <tr>
@@ -11953,7 +11957,9 @@ TaskKind
 </em>
 </td>
 <td>
-<p>TaskKind indicates the kind of the task, namespaced or cluster scoped.</p>
+<p>TaskKind indicates the Kind of the Task:
+1. Namespaced Task when Kind is set to &ldquo;Task&rdquo;
+2. Custom Task when Kind is non-empty and APIVersion is non-empty</p>
 </td>
 </tr>
 <tr>
@@ -11965,7 +11971,8 @@ string
 </td>
 <td>
 <em>(Optional)</em>
-<p>API version of the referent</p>
+<p>API version of the referent
+Note: A Task with non-empty APIVersion and Kind is considered a Custom Task</p>
 </td>
 </tr>
 <tr>

pkg/apis/pipeline/v1/openapi_generated.go

@@ -203,6 +203,11 @@ type PipelineTask struct {
 	Timeout *metav1.Duration `json:"timeout,omitempty"`
 }
 
+// IsCustomTask checks whether an embedded TaskSpec is a Custom Task
+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.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{
+				APIVersion: "example/v0",
+			},
+		},
+		want: false,
+	}, {
+		name: "custom task - APIVersion and Kind are set",
+		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/pipeline_types_test.go

@@ -1654,11 +1654,11 @@
       "type": "object",
       "properties": {
         "apiVersion": {
-          "description": "API version of the referent",
+          "description": "API version of the referent Note: A Task with non-empty APIVersion and Kind is considered a Custom Task",
           "type": "string"
         },
         "kind": {
-          "description": "TaskKind indicates the kind of the task, namespaced or cluster scoped.",
+          "description": "TaskKind indicates the Kind of the Task: 1. Namespaced Task when Kind is set to \"Task\" 2. Cluster-Scoped Task when Kind is set to \"ClusterTask\" 3. Custom Task when Kind is non-empty and APIVersion is non-empty",
           "type": "string"
         },
         "name": {

pkg/apis/pipeline/v1/swagger.json

@@ -20,9 +20,13 @@ package v1
 type TaskRef struct {
 	// Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names
 	Name string `json:"name,omitempty"`
-	// TaskKind indicates the kind of the task, namespaced or cluster scoped.
+	// TaskKind indicates the Kind of the Task:
+	// 1. Namespaced Task when Kind is set to "Task"
+	// 2. Cluster-Scoped Task when Kind is set to "ClusterTask"
+	// 3. Custom Task when Kind is non-empty and APIVersion is non-empty
 	Kind TaskKind `json:"kind,omitempty"`
 	// API version of the referent
+	// Note: A Task with non-empty APIVersion and Kind is considered a Custom Task
 	// +optional
 	APIVersion string `json:"apiVersion,omitempty"`
 
@@ -40,3 +44,8 @@ const (
 	// NamespacedTaskKind indicates that the task type has a namespaced scope.
 	NamespacedTaskKind TaskKind = "Task"
 )
+
+// IsCustomTask checks whether the reference is to a Custom Task
+func (tr *TaskRef) IsCustomTask() bool {
+	return tr != nil && tr.APIVersion != "" && tr.Kind != ""
+}

pkg/apis/pipeline/v1/taskref_types.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/v1/taskref_types_test.go

@@ -209,6 +209,11 @@ type PipelineTask struct {
 	Timeout *metav1.Duration `json:"timeout,omitempty"`
 }
 
+// IsCustomTask checks whether an embedded TaskSpec is a Custom Task
+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/openapi_generated.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{
+				APIVersion: "example/v0",
+			},
+		},
+		want: false,
+	}, {
+		name: "custom task - APIVersion and Kind are set",
+		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)
+			}
+		})
+	}
+}