From 4c7cf1ff2f9781ff227820528a6f29153b27249f Mon Sep 17 00:00:00 2001 From: dlorenc Date: Tue, 9 Oct 2018 12:11:00 -0700 Subject: [PATCH] Add BuildSpec validation to TaskSpec. --- pkg/apis/pipeline/v1alpha1/task_validation.go | 34 +++++--- .../pipeline/v1alpha1/task_validation_test.go | 83 ++++++++++++++----- 2 files changed, 81 insertions(+), 36 deletions(-) diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go index a046442e3c1..287da6b0530 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation.go @@ -35,25 +35,33 @@ func (ts *TaskSpec) Validate() *apis.FieldError { return apis.ErrMissingField(apis.CurrentField) } + // A Task must have a valid BuildSpec. + if ts.BuildSpec == nil { + return apis.ErrMissingField("taskspec.BuildSpec") + } + if err := ts.BuildSpec.Validate(); err != nil { + return err + } + // A task doesn't have to have inputs or outputs, but if it does they must be valid. // A task can't duplicate input or output names. if ts.Inputs != nil { - for _, source := range ts.Inputs.Sources { - if err := validateResourceType(source, fmt.Sprintf("taskspec.Inputs.Sources.%s.Type", source.Name)); err != nil { + for _, resource := range ts.Inputs.Resources { + if err := validateResourceType(resource, fmt.Sprintf("taskspec.Inputs.Resources.%s.Type", resource.Name)); err != nil { return err } } - if err := checkForDuplicates(ts.Inputs.Sources, "taskspec.Inputs.Sources.Name"); err != nil { + if err := checkForDuplicates(ts.Inputs.Resources, "taskspec.Inputs.Resources.Name"); err != nil { return err } } if ts.Outputs != nil { - for _, source := range ts.Outputs.Sources { - if err := validateResourceType(source, fmt.Sprintf("taskspec.Outputs.Sources.%s.Type", source.Name)); err != nil { + for _, source := range ts.Outputs.Resources { + if err := validateResourceType(source, fmt.Sprintf("taskspec.Outputs.Resources.%s.Type", source.Name)); err != nil { return err } - if err := checkForDuplicates(ts.Outputs.Sources, "taskspec.Outputs.Sources.Name"); err != nil { + if err := checkForDuplicates(ts.Outputs.Resources, "taskspec.Outputs.Resources.Name"); err != nil { return err } } @@ -61,22 +69,22 @@ func (ts *TaskSpec) Validate() *apis.FieldError { return nil } -func checkForDuplicates(sources []Source, path string) *apis.FieldError { +func checkForDuplicates(resources []TaskResource, path string) *apis.FieldError { encountered := map[string]struct{}{} - for _, s := range sources { - if _, ok := encountered[s.Name]; ok { + for _, r := range resources { + if _, ok := encountered[r.Name]; ok { return apis.ErrMultipleOneOf(path) } - encountered[s.Name] = struct{}{} + encountered[r.Name] = struct{}{} } return nil } -func validateResourceType(s Source, path string) *apis.FieldError { +func validateResourceType(r TaskResource, path string) *apis.FieldError { for _, allowed := range AllResourceTypes { - if s.Type == allowed { + if r.Type == allowed { return nil } } - return apis.ErrInvalidValue(string(s.Type), path) + return apis.ErrInvalidValue(string(r.Type), path) } diff --git a/pkg/apis/pipeline/v1alpha1/task_validation_test.go b/pkg/apis/pipeline/v1alpha1/task_validation_test.go index 56041d08d6a..0044df4dd5a 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation_test.go @@ -19,14 +19,25 @@ package v1alpha1 import ( "testing" + corev1 "k8s.io/api/core/v1" + buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1" ) -var validSource = Source{ +var validResource = TaskResource{ Name: "source", Type: "git", } +var validBuild = &buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{ + { + Name: "mystep", + Image: "myimage", + }, + }, +} + func TestTaskSpec_Validate(t *testing.T) { type fields struct { Inputs *Inputs @@ -41,27 +52,30 @@ func TestTaskSpec_Validate(t *testing.T) { name: "valid inputs", fields: fields{ Inputs: &Inputs{ - Sources: []Source{validSource}, + Resources: []TaskResource{validResource}, }, + BuildSpec: validBuild, }, }, { name: "valid outputs", fields: fields{ Outputs: &Outputs{ - Sources: []Source{validSource}, + Resources: []TaskResource{validResource}, }, + BuildSpec: validBuild, }, }, { name: "both valid", fields: fields{ Inputs: &Inputs{ - Sources: []Source{validSource}, + Resources: []TaskResource{validResource}, }, Outputs: &Outputs{ - Sources: []Source{validSource}, + Resources: []TaskResource{validResource}, }, + BuildSpec: validBuild, }, }, } @@ -92,74 +106,97 @@ func TestTaskSpec_ValidateError(t *testing.T) { { name: "nil", }, + { + name: "no build", + fields: fields{ + Inputs: &Inputs{ + Resources: []TaskResource{validResource}, + }, + }, + }, { name: "one invalid input", fields: fields{ Inputs: &Inputs{ - Sources: []Source{ + Resources: []TaskResource{ { Name: "source", Type: "what", }, - validSource, + validResource, }, }, Outputs: &Outputs{ - Sources: []Source{ - validSource, + Resources: []TaskResource{ + validResource, }, }, + BuildSpec: validBuild, }, }, { name: "one invalid output", fields: fields{ Inputs: &Inputs{ - Sources: []Source{ - validSource, + Resources: []TaskResource{ + validResource, }, }, Outputs: &Outputs{ - Sources: []Source{ + Resources: []TaskResource{ { Name: "who", Type: "what", }, - validSource, + validResource, }, }, + BuildSpec: validBuild, }, }, { name: "duplicated inputs", fields: fields{ Inputs: &Inputs{ - Sources: []Source{ - validSource, - validSource, + Resources: []TaskResource{ + validResource, + validResource, }, }, Outputs: &Outputs{ - Sources: []Source{ - validSource, + Resources: []TaskResource{ + validResource, }, }, + BuildSpec: validBuild, }, }, { name: "duplicated outputs", fields: fields{ Inputs: &Inputs{ - Sources: []Source{ - validSource, + Resources: []TaskResource{ + validResource, }, }, Outputs: &Outputs{ - Sources: []Source{ - validSource, - validSource, + Resources: []TaskResource{ + validResource, + validResource, }, }, + BuildSpec: validBuild, + }, + }, + { + name: "invalid build", + fields: fields{ + Inputs: &Inputs{ + Resources: []TaskResource{validResource}, + }, + BuildSpec: &buildv1alpha1.BuildSpec{ + Steps: []corev1.Container{}, + }, }, }, }