Skip to content

Commit

Permalink
Split up and refactor isCustomTask
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jerop committed Mar 29, 2023
1 parent 538fee3 commit bdd28a1
Show file tree
Hide file tree
Showing 15 changed files with 247 additions and 39 deletions.
15 changes: 11 additions & 4 deletions docs/pipeline-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down Expand Up @@ -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>
Expand All @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/apis/pipeline/v1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 45 additions & 0 deletions pkg/apis/pipeline/v1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
11 changes: 10 additions & 1 deletion pkg/apis/pipeline/v1/taskref_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand All @@ -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 != ""
}
54 changes: 54 additions & 0 deletions pkg/apis/pipeline/v1/taskref_types_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1beta1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 45 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1beta1/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -2168,15 +2168,15 @@
"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"
},
"bundle": {
"description": "Bundle url reference to a Tekton Bundle.\n\nDeprecated: Please use ResolverRef with the bundles resolver instead.",
"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. Custom Task when Kind is non-empty and APIVersion is non-empty",
"type": "string"
},
"name": {
Expand Down
10 changes: 9 additions & 1 deletion pkg/apis/pipeline/v1beta1/taskref_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ package v1beta1
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. 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"`
// Bundle url reference to a Tekton Bundle.
Expand All @@ -49,3 +52,8 @@ const (
// ClusterTaskKind indicates that task type has a cluster scope.
ClusterTaskKind TaskKind = "ClusterTask"
)

// IsCustomTask checks whether the reference is to a Custom Task
func (tr *TaskRef) IsCustomTask() bool {
return tr != nil && tr.APIVersion != "" && tr.Kind != ""
}
54 changes: 54 additions & 0 deletions pkg/apis/pipeline/v1beta1/taskref_types_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
Loading

0 comments on commit bdd28a1

Please sign in to comment.