Skip to content

Commit

Permalink
Add actual validation to the task validation webhook skeleton.
Browse files Browse the repository at this point in the history
This PR adds the following validation:
- Task inputs and outputs must have a valid Type
- Task inputs and outputs must not have a duplicated name.
  • Loading branch information
dlorenc authored and knative-prow-robot committed Oct 10, 2018
1 parent f7bfbd4 commit b9e610b
Show file tree
Hide file tree
Showing 3 changed files with 226 additions and 1 deletion.
2 changes: 2 additions & 0 deletions pkg/apis/pipeline/v1alpha1/resource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ const (
PipelineResourceTypeImage PipelineResourceType = "image"
)

var AllResourceTypes = []PipelineResourceType{PipelineResourceTypeGit, PipelineResourceTypeGCS, PipelineResourceTypeImage}

// PipelineResourceInterface interface to be implemented by different PipelineResource types
type PipelineResourceInterface interface {
GetName() string
Expand Down
47 changes: 46 additions & 1 deletion pkg/apis/pipeline/v1alpha1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package v1alpha1

import (
"fmt"

"github.com/knative/pkg/apis"
"k8s.io/apimachinery/pkg/api/equality"
)
Expand All @@ -25,13 +27,56 @@ func (t *Task) Validate() *apis.FieldError {
if err := validateObjectMetadata(t.GetObjectMeta()); err != nil {
return err.ViaField("metadata")
}
return nil
return t.Spec.Validate()
}

func (ts *TaskSpec) Validate() *apis.FieldError {
if equality.Semantic.DeepEqual(ts, &TaskSpec{}) {
return apis.ErrMissingField(apis.CurrentField)
}

// 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 {
return err
}
}
if err := checkForDuplicates(ts.Inputs.Sources, "taskspec.Inputs.Sources.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 {
return err
}
if err := checkForDuplicates(ts.Outputs.Sources, "taskspec.Outputs.Sources.Name"); err != nil {
return err
}
}
}
return nil
}

func checkForDuplicates(sources []Source, path string) *apis.FieldError {
encountered := map[string]struct{}{}
for _, s := range sources {
if _, ok := encountered[s.Name]; ok {
return apis.ErrMultipleOneOf(path)
}
encountered[s.Name] = struct{}{}
}
return nil
}

func validateResourceType(s Source, path string) *apis.FieldError {
for _, allowed := range AllResourceTypes {
if s.Type == allowed {
return nil
}
}
return apis.ErrInvalidValue(string(s.Type), path)
}
178 changes: 178 additions & 0 deletions pkg/apis/pipeline/v1alpha1/task_validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
/*
Copyright 2018 The Knative Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1

import (
"testing"

buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1"
)

var validSource = Source{
Name: "source",
Type: "git",
}

func TestTaskSpec_Validate(t *testing.T) {
type fields struct {
Inputs *Inputs
Outputs *Outputs
BuildSpec *buildv1alpha1.BuildSpec
}
tests := []struct {
name string
fields fields
}{
{
name: "valid inputs",
fields: fields{
Inputs: &Inputs{
Sources: []Source{validSource},
},
},
},
{
name: "valid outputs",
fields: fields{
Outputs: &Outputs{
Sources: []Source{validSource},
},
},
},
{
name: "both valid",
fields: fields{
Inputs: &Inputs{
Sources: []Source{validSource},
},
Outputs: &Outputs{
Sources: []Source{validSource},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ts := &TaskSpec{
Inputs: tt.fields.Inputs,
Outputs: tt.fields.Outputs,
BuildSpec: tt.fields.BuildSpec,
}
if err := ts.Validate(); err != nil {
t.Errorf("TaskSpec.Validate() = %v", err)
}
})
}
}

func TestTaskSpec_ValidateError(t *testing.T) {
type fields struct {
Inputs *Inputs
Outputs *Outputs
BuildSpec *buildv1alpha1.BuildSpec
}
tests := []struct {
name string
fields fields
}{
{
name: "nil",
},
{
name: "one invalid input",
fields: fields{
Inputs: &Inputs{
Sources: []Source{
{
Name: "source",
Type: "what",
},
validSource,
},
},
Outputs: &Outputs{
Sources: []Source{
validSource,
},
},
},
},
{
name: "one invalid output",
fields: fields{
Inputs: &Inputs{
Sources: []Source{
validSource,
},
},
Outputs: &Outputs{
Sources: []Source{
{
Name: "who",
Type: "what",
},
validSource,
},
},
},
},
{
name: "duplicated inputs",
fields: fields{
Inputs: &Inputs{
Sources: []Source{
validSource,
validSource,
},
},
Outputs: &Outputs{
Sources: []Source{
validSource,
},
},
},
},
{
name: "duplicated outputs",
fields: fields{
Inputs: &Inputs{
Sources: []Source{
validSource,
},
},
Outputs: &Outputs{
Sources: []Source{
validSource,
validSource,
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ts := &TaskSpec{
Inputs: tt.fields.Inputs,
Outputs: tt.fields.Outputs,
BuildSpec: tt.fields.BuildSpec,
}
if err := ts.Validate(); err == nil {
t.Errorf("TaskSpec.Validate() did not return error.")
}
})
}
}

0 comments on commit b9e610b

Please sign in to comment.