Skip to content

Commit

Permalink
Add BuildSpec validation to TaskSpec.
Browse files Browse the repository at this point in the history
dlorenc authored and knative-prow-robot committed Oct 10, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent b9e610b commit 4c7cf1f
Showing 2 changed files with 81 additions and 36 deletions.
34 changes: 21 additions & 13 deletions pkg/apis/pipeline/v1alpha1/task_validation.go
Original file line number Diff line number Diff line change
@@ -35,48 +35,56 @@ 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
}
}
}
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)
}
83 changes: 60 additions & 23 deletions pkg/apis/pipeline/v1alpha1/task_validation_test.go
Original file line number Diff line number Diff line change
@@ -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{},
},
},
},
}

0 comments on commit 4c7cf1f

Please sign in to comment.