Custom task without api version return validation error

This commit closes tektoncd#6459. For a customtask reference in a pipelinetask, if the Kind is non-empty and not "Task" or "ClusterTask", then it should be a Custom task and api version should be set. If not then validation webhook should return error. TaskRun's taskRef validation will be handled separately in tektoncd#6557 Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com
bobcatfish · Apr 25, 2023 · 7d6cd81 · 7d6cd81
1 parent a2630a4
commit 7d6cd81
Show file tree

Hide file tree

Showing 7 changed files with 88 additions and 6 deletions.
diff --git a/docs/pipelines.md b/docs/pipelines.md
@@ -1555,6 +1555,8 @@ spec:
 
 This creates a `Run/CustomRun` of a custom task of type `Example` in the `example.dev` API group with the version `v1alpha1`.
 
+Validation error will be returned if `apiVersion` or `kind` is missing.
+
 You can also specify the `name` of a custom task resource object previously defined in the cluster.
 
 ```yaml

diff --git a/pkg/apis/pipeline/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go
@@ -311,7 +311,7 @@ func TestPipelineTask_Validate_Failure(t *testing.T) {
  expectedError apis.FieldError
  wc func(context.Context) context.Context
  }{{
- name: "invalid custom task without Kind",
+ name: "custom task reference in taskref missing apiversion Kind",
  p: PipelineTask{
  Name: "invalid-custom-task",
  TaskRef: &TaskRef{APIVersion: "example.com"},
@@ -320,7 +320,26 @@ func TestPipelineTask_Validate_Failure(t *testing.T) {
  Message: `invalid value: custom task ref must specify kind`,
  Paths: []string{"taskRef.kind"},
  },
- }}
+ }, {
+ name: "custom task reference in taskspec missing kind",
+ p: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{
+ TypeMeta: runtime.TypeMeta{
+ APIVersion: "example.com",
+ }}},
+ expectedError: *apis.ErrInvalidValue("custom task spec must specify kind", "taskSpec.kind"),
+ }, {
+ name: "custom task reference in taskref missing apiversion",
+ p: PipelineTask{Name: "foo", TaskRef: &TaskRef{Kind: "Example", Name: ""}},
+ expectedError: *apis.ErrInvalidValue("custom task ref must specify apiVersion", "taskRef.apiVersion"),
+ }, {
+ name: "custom task reference in taskspec missing apiversion",
+ p: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{
+ TypeMeta: runtime.TypeMeta{
+ Kind: "Example",
+ }}},
+ expectedError: *apis.ErrInvalidValue("custom task spec must specify apiVersion", "taskSpec.apiVersion"),
+ },
+ }
  for _, tt := range tests {
  t.Run(tt.name, func(t *testing.T) {
  ctx := context.Background()

diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go
@@ -126,11 +126,20 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) {
  errs = errs.Also(pt.validateRefOrSpec())
 
  errs = errs.Also(pt.validateEmbeddedOrType())
-
+ // taskKinds contains the kinds when the apiVersion is not set, they are not custom tasks,
+ // if apiVersion is set they are custom tasks.
+ taskKinds := map[TaskKind]bool{
+ "": true,
+ NamespacedTaskKind: true,
+ }
  // Pipeline task having taskRef/taskSpec with APIVersion is classified as custom task
  switch {
+ case pt.TaskRef != nil && !taskKinds[pt.TaskRef.Kind]:
+ errs = errs.Also(pt.validateCustomTask())
  case pt.TaskRef != nil && pt.TaskRef.APIVersion != "":
  errs = errs.Also(pt.validateCustomTask())
+ case pt.TaskSpec != nil && !taskKinds[TaskKind(pt.TaskSpec.Kind)]:
+ errs = errs.Also(pt.validateCustomTask())
  case pt.TaskSpec != nil && pt.TaskSpec.APIVersion != "":
  errs = errs.Also(pt.validateCustomTask())
  default:

diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go
@@ -69,6 +69,14 @@ func TestPipeline_Validate_Success(t *testing.T) {
  }},
  },
  },
+ }, {
+ name: "valid Task without apiversion",
+ p: &Pipeline{
+ ObjectMeta: metav1.ObjectMeta{Name: "pipeline"},
+ Spec: PipelineSpec{
+ Tasks: []PipelineTask{{Name: "foo", TaskRef: &TaskRef{Name: "bar", Kind: NamespacedTaskKind}}},
+ },
+ },
  }}
  for _, tt := range tests {
  t.Run(tt.name, func(t *testing.T) {

diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go
@@ -339,7 +339,7 @@ func TestPipelineTask_Validate_Failure(t *testing.T) {
  expectedError apis.FieldError
  wc func(context.Context) context.Context
  }{{
- name: "invalid custom task without Kind",
+ name: "custom task reference in taskref missing apiversion Kind",
  p: PipelineTask{
  Name: "invalid-custom-task",
  TaskRef: &TaskRef{APIVersion: "example.com"},
@@ -348,6 +348,24 @@ func TestPipelineTask_Validate_Failure(t *testing.T) {
  Message: `invalid value: custom task ref must specify kind`,
  Paths: []string{"taskRef.kind"},
  },
+ }, {
+ name: "custom task reference in taskspec missing kind",
+ p: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{
+ TypeMeta: runtime.TypeMeta{
+ APIVersion: "example.com",
+ }}},
+ expectedError: *apis.ErrInvalidValue("custom task spec must specify kind", "taskSpec.kind"),
+ }, {
+ name: "custom task reference in taskref missing apiversion",
+ p: PipelineTask{Name: "foo", TaskRef: &TaskRef{Kind: "Example", Name: ""}},
+ expectedError: *apis.ErrInvalidValue("custom task ref must specify apiVersion", "taskRef.apiVersion"),
+ }, {
+ name: "custom task reference in taskspec missing apiversion",
+ p: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{
+ TypeMeta: runtime.TypeMeta{
+ Kind: "Example",
+ }}},
+ expectedError: *apis.ErrInvalidValue("custom task spec must specify apiVersion", "taskSpec.apiVersion"),
  }, {
  name: "invalid bundle without bundle name",
  p: PipelineTask{

diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go
@@ -134,12 +134,22 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) {
  if pt.Resources != nil {
  errs = errs.Also(apis.ErrDisallowedFields("resources"))
  }
-
+ // taskKinds contains the kinds when the apiVersion is not set, they are not custom tasks,
+ // if apiVersion is set they are custom tasks.
+ taskKinds := map[TaskKind]bool{
+ "": true,
+ NamespacedTaskKind: true,
+ ClusterTaskKind: true,
+ }
  cfg := config.FromContextOrDefaults(ctx)
  // Pipeline task having taskRef/taskSpec with APIVersion is classified as custom task
  switch {
+ case pt.TaskRef != nil && !taskKinds[pt.TaskRef.Kind]:
+ errs = errs.Also(pt.validateCustomTask())
  case pt.TaskRef != nil && pt.TaskRef.APIVersion != "":
  errs = errs.Also(pt.validateCustomTask())
+ case pt.TaskSpec != nil && !taskKinds[TaskKind(pt.TaskSpec.Kind)]:
+ errs = errs.Also(pt.validateCustomTask())
  case pt.TaskSpec != nil && pt.TaskSpec.APIVersion != "":
  errs = errs.Also(pt.validateCustomTask())
  // If EnableTektonOCIBundles feature flag is on, validate bundle specifications
@@ -148,7 +158,7 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) {
  default:
  errs = errs.Also(pt.validateTask(ctx))
  }
- return
+ return //nolint:nakedret
 }
 
 func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldError) {

diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
@@ -71,6 +71,22 @@ func TestPipeline_Validate_Success(t *testing.T) {
  }},
  },
  },
+ }, {
+ name: "valid Task without apiversion",
+ p: &Pipeline{
+ ObjectMeta: metav1.ObjectMeta{Name: "pipeline"},
+ Spec: PipelineSpec{
+ Tasks: []PipelineTask{{Name: "foo", TaskRef: &TaskRef{Name: "bar", Kind: NamespacedTaskKind}}},
+ },
+ },
+ }, {
+ name: "valid Cluster Task without apiversion",
+ p: &Pipeline{
+ ObjectMeta: metav1.ObjectMeta{Name: "pipeline"},
+ Spec: PipelineSpec{
+ Tasks: []PipelineTask{{Name: "foo", TaskRef: &TaskRef{Name: "task", Kind: ClusterTaskKind}}},
+ },
+ },
  }}
  for _, tt := range tests {
  t.Run(tt.name, func(t *testing.T) {