diff --git a/pkg/apis/pipeline/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go index f9e00a0213d..65ec4bc3ec2 100644 --- a/pkg/apis/pipeline/v1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1/pipeline_types_test.go @@ -447,7 +447,7 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) { expectedError apis.FieldError }{{ name: "custom task - taskRef without kind", - task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "example.dev/v0", Kind: "", Name: ""}}, + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "example/v0", Kind: "", Name: ""}}, expectedError: apis.FieldError{ Message: `invalid value: custom task ref must specify kind`, Paths: []string{"taskRef.kind"}, @@ -456,7 +456,7 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) { name: "custom task - taskSpec without kind", task: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ TypeMeta: runtime.TypeMeta{ - APIVersion: "example.dev/v0", + APIVersion: "example/v0", Kind: "", }, }}, @@ -483,6 +483,39 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) { Message: `invalid value: custom task ref must specify apiVersion`, Paths: []string{"taskRef.apiVersion"}, }, + }, { + name: "custom task - taskRef with invalid apiVersion format", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "invalid-api-version", Kind: "some-kind", Name: ""}}, + expectedError: apis.FieldError{ + Message: `invalid value: invalid apiVersion format "invalid-api-version", must be in the format "group/version"`, + Paths: []string{"taskRef.apiVersion"}, + }, + }, { + name: "custom task - taskSpec with invalid apiVersion format", + task: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "no-slash-no-dot", + Kind: "some-kind", + }, + }}, + expectedError: apis.FieldError{ + Message: `invalid value: invalid apiVersion format "no-slash-no-dot", must be in the format "group/version"`, + Paths: []string{"taskSpec.apiVersion"}, + }, + }, { + name: "custom task - taskRef with empty group in apiVersion", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "/v1", Kind: "some-kind", Name: ""}}, + expectedError: apis.FieldError{ + Message: `invalid value: invalid apiVersion format "/v1", must be in the format "group/version"`, + Paths: []string{"taskRef.apiVersion"}, + }, + }, { + name: "custom task - taskRef with empty version in apiVersion", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "example/", Kind: "some-kind", Name: ""}}, + expectedError: apis.FieldError{ + Message: `invalid value: invalid apiVersion format "example/", must be in the format "group/version"`, + Paths: []string{"taskRef.apiVersion"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -497,6 +530,38 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) { } } +func TestPipelineTask_ValidateCustomTask_ValidAPIVersion(t *testing.T) { + tests := []struct { + name string + task PipelineTask + }{{ + name: "custom task - valid apiVersion with group/version", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "example.dev/v1", Kind: "Example", Name: "example"}}, + }, { + name: "custom task - valid apiVersion with multi-level group", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "custom.tekton.dev/v1beta1", Kind: "Custom", Name: "custom"}}, + }, { + name: "custom task - valid apiVersion without dots in group", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "example/v1", Kind: "Example", Name: "example"}}, + }, { + name: "custom task - valid apiVersion in taskSpec", + task: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "example.io/v2", + Kind: "CustomTask", + }, + }}, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.task.validateCustomTask() + if err != nil { + t.Errorf("PipelineTask.validateCustomTask() returned unexpected error: %v", err) + } + }) + } +} + func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { tests := []struct { name string @@ -691,7 +756,7 @@ func TestPipelineTask_Validate_Failure(t *testing.T) { name: "custom task reference in taskref missing apiversion Kind", p: PipelineTask{ Name: "invalid-custom-task", - TaskRef: &TaskRef{APIVersion: "example.com"}, + TaskRef: &TaskRef{APIVersion: "example.com/v1"}, }, expectedError: apis.FieldError{ Message: `invalid value: custom task ref must specify kind`, @@ -701,7 +766,7 @@ func TestPipelineTask_Validate_Failure(t *testing.T) { name: "custom task reference in taskspec missing kind", p: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ TypeMeta: runtime.TypeMeta{ - APIVersion: "example.com", + APIVersion: "example.com/v1", }}}, expectedError: *apis.ErrInvalidValue("custom task spec must specify kind", "taskSpec.kind"), }, { diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 8d109c10fab..0037d2973dc 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -336,6 +336,19 @@ func (pt PipelineTask) validateRefOrSpec(ctx context.Context) (errs *apis.FieldE return errs } +// isValidAPIVersion validates the format of an apiVersion string. +// Valid formats are "group/version" where both group and version are non-empty. +// For custom tasks, apiVersion must always be in the "group/version" format. +func isValidAPIVersion(apiVersion string) bool { + parts := strings.Split(apiVersion, "/") + if len(parts) != 2 { + return false + } + group := parts[0] + version := parts[1] + return group != "" && version != "" +} + // validateCustomTask validates custom task specifications - checking kind and fail if not yet supported features specified func (pt PipelineTask) validateCustomTask() (errs *apis.FieldError) { if pt.TaskRef != nil && pt.TaskRef.Kind == "" { @@ -344,10 +357,19 @@ func (pt PipelineTask) validateCustomTask() (errs *apis.FieldError) { if pt.TaskSpec != nil && pt.TaskSpec.Kind == "" { errs = errs.Also(apis.ErrInvalidValue("custom task spec must specify kind", "taskSpec.kind")) } - if pt.TaskRef != nil && pt.TaskRef.APIVersion == "" { + // Validate apiVersion format for custom tasks + if pt.TaskRef != nil && pt.TaskRef.APIVersion != "" { + if !isValidAPIVersion(pt.TaskRef.APIVersion) { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("invalid apiVersion format %q, must be in the format \"group/version\"", pt.TaskRef.APIVersion), "taskRef.apiVersion")) + } + } else if pt.TaskRef != nil { errs = errs.Also(apis.ErrInvalidValue("custom task ref must specify apiVersion", "taskRef.apiVersion")) } - if pt.TaskSpec != nil && pt.TaskSpec.APIVersion == "" { + if pt.TaskSpec != nil && pt.TaskSpec.APIVersion != "" { + if !isValidAPIVersion(pt.TaskSpec.APIVersion) { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("invalid apiVersion format %q, must be in the format \"group/version\"", pt.TaskSpec.APIVersion), "taskSpec.apiVersion")) + } + } else if pt.TaskSpec != nil { errs = errs.Also(apis.ErrInvalidValue("custom task spec must specify apiVersion", "taskSpec.apiVersion")) } return errs diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go index bf7d19b72ea..5ad16a02d35 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go @@ -447,7 +447,7 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) { expectedError apis.FieldError }{{ name: "custom task - taskRef without kind", - task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "example.dev/v0", Kind: "", Name: ""}}, + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "example/v0", Kind: "", Name: ""}}, expectedError: apis.FieldError{ Message: `invalid value: custom task ref must specify kind`, Paths: []string{"taskRef.kind"}, @@ -456,7 +456,7 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) { name: "custom task - taskSpec without kind", task: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ TypeMeta: runtime.TypeMeta{ - APIVersion: "example.dev/v0", + APIVersion: "example/v0", Kind: "", }, }}, @@ -483,6 +483,39 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) { Message: `invalid value: custom task ref must specify apiVersion`, Paths: []string{"taskRef.apiVersion"}, }, + }, { + name: "custom task - taskRef with invalid apiVersion format", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "invalid-api-version", Kind: "some-kind", Name: ""}}, + expectedError: apis.FieldError{ + Message: `invalid value: invalid apiVersion format "invalid-api-version", must be in the format "group/version"`, + Paths: []string{"taskRef.apiVersion"}, + }, + }, { + name: "custom task - taskSpec with invalid apiVersion format", + task: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "no-slash-no-dot", + Kind: "some-kind", + }, + }}, + expectedError: apis.FieldError{ + Message: `invalid value: invalid apiVersion format "no-slash-no-dot", must be in the format "group/version"`, + Paths: []string{"taskSpec.apiVersion"}, + }, + }, { + name: "custom task - taskRef with empty group in apiVersion", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "/v1", Kind: "some-kind", Name: ""}}, + expectedError: apis.FieldError{ + Message: `invalid value: invalid apiVersion format "/v1", must be in the format "group/version"`, + Paths: []string{"taskRef.apiVersion"}, + }, + }, { + name: "custom task - taskRef with empty version in apiVersion", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "example/", Kind: "some-kind", Name: ""}}, + expectedError: apis.FieldError{ + Message: `invalid value: invalid apiVersion format "example/", must be in the format "group/version"`, + Paths: []string{"taskRef.apiVersion"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -497,6 +530,38 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) { } } +func TestPipelineTask_ValidateCustomTask_ValidAPIVersion(t *testing.T) { + tests := []struct { + name string + task PipelineTask + }{{ + name: "custom task - valid apiVersion with group/version", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "example.dev/v1", Kind: "Example", Name: "example"}}, + }, { + name: "custom task - valid apiVersion with multi-level group", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "custom.tekton.dev/v1beta1", Kind: "Custom", Name: "custom"}}, + }, { + name: "custom task - valid apiVersion without dots in group", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "example/v1", Kind: "Example", Name: "example"}}, + }, { + name: "custom task - valid apiVersion in taskSpec", + task: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "example.io/v2", + Kind: "CustomTask", + }, + }}, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.task.validateCustomTask() + if err != nil { + t.Errorf("PipelineTask.validateCustomTask() returned unexpected error: %v", err) + } + }) + } +} + func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { tests := []struct { name string @@ -687,7 +752,7 @@ func TestPipelineTask_Validate_Failure(t *testing.T) { name: "custom task reference in taskref missing apiversion Kind", p: PipelineTask{ Name: "invalid-custom-task", - TaskRef: &TaskRef{APIVersion: "example.com"}, + TaskRef: &TaskRef{APIVersion: "example.com/v1"}, }, expectedError: apis.FieldError{ Message: `invalid value: custom task ref must specify kind`, @@ -697,7 +762,7 @@ func TestPipelineTask_Validate_Failure(t *testing.T) { name: "custom task reference in taskspec missing kind", p: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ TypeMeta: runtime.TypeMeta{ - APIVersion: "example.com", + APIVersion: "example.com/v1", }, }}, expectedError: *apis.ErrInvalidValue("custom task spec must specify kind", "taskSpec.kind"), @@ -846,7 +911,7 @@ func TestPipelineTaskList_Validate(t *testing.T) { name: "validate all valid custom task, and regular task", tasks: PipelineTaskList{{ Name: "valid-custom-task", - TaskRef: &TaskRef{APIVersion: "example.com", Kind: "custom"}, + TaskRef: &TaskRef{APIVersion: "example.com/v1", Kind: "custom"}, }, { Name: "valid-task", TaskRef: &TaskRef{Name: "task"}, @@ -856,7 +921,7 @@ func TestPipelineTaskList_Validate(t *testing.T) { name: "validate list of tasks with valid custom task and invalid regular task", tasks: PipelineTaskList{{ Name: "valid-custom-task", - TaskRef: &TaskRef{APIVersion: "example.com", Kind: "custom"}, + TaskRef: &TaskRef{APIVersion: "example.com/v1", Kind: "custom"}, }, { Name: "invalid-task-without-name", TaskRef: &TaskRef{Name: ""}, @@ -867,7 +932,7 @@ func TestPipelineTaskList_Validate(t *testing.T) { name: "validate all invalid tasks - custom task and regular task", tasks: PipelineTaskList{{ Name: "invalid-custom-task", - TaskRef: &TaskRef{APIVersion: "example.com"}, + TaskRef: &TaskRef{APIVersion: "example.com/v1"}, }, { Name: "invalid-task", TaskRef: &TaskRef{Name: ""}, diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index e0c9b7365e2..abb967d1872 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -341,6 +341,19 @@ func (pt PipelineTask) validateRefOrSpec(ctx context.Context) (errs *apis.FieldE return errs } +// isValidAPIVersion validates the format of an apiVersion string. +// Valid formats are "group/version" where both group and version are non-empty. +// For custom tasks, apiVersion must always be in the "group/version" format. +func isValidAPIVersion(apiVersion string) bool { + parts := strings.Split(apiVersion, "/") + if len(parts) != 2 { + return false + } + group := parts[0] + version := parts[1] + return group != "" && version != "" +} + // validateCustomTask validates custom task specifications - checking kind and fail if not yet supported features specified func (pt PipelineTask) validateCustomTask() (errs *apis.FieldError) { if pt.TaskRef != nil && pt.TaskRef.Kind == "" { @@ -349,10 +362,19 @@ func (pt PipelineTask) validateCustomTask() (errs *apis.FieldError) { if pt.TaskSpec != nil && pt.TaskSpec.Kind == "" { errs = errs.Also(apis.ErrInvalidValue("custom task spec must specify kind", "taskSpec.kind")) } - if pt.TaskRef != nil && pt.TaskRef.APIVersion == "" { + // Validate apiVersion format for custom tasks + if pt.TaskRef != nil && pt.TaskRef.APIVersion != "" { + if !isValidAPIVersion(pt.TaskRef.APIVersion) { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("invalid apiVersion format %q, must be in the format \"group/version\"", pt.TaskRef.APIVersion), "taskRef.apiVersion")) + } + } else if pt.TaskRef != nil { errs = errs.Also(apis.ErrInvalidValue("custom task ref must specify apiVersion", "taskRef.apiVersion")) } - if pt.TaskSpec != nil && pt.TaskSpec.APIVersion == "" { + if pt.TaskSpec != nil && pt.TaskSpec.APIVersion != "" { + if !isValidAPIVersion(pt.TaskSpec.APIVersion) { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("invalid apiVersion format %q, must be in the format \"group/version\"", pt.TaskSpec.APIVersion), "taskSpec.apiVersion")) + } + } else if pt.TaskSpec != nil { errs = errs.Also(apis.ErrInvalidValue("custom task spec must specify apiVersion", "taskSpec.apiVersion")) } return errs