Validate Task for undefined variables

A Task can fail at "runtime", when the variables are interpolated based on resources and parameters. Adding validation for those inexistent variable shorten the loop when trying to write a Pipeline. Signed-off-by: Vincent Demeester <[email protected]>
tektoncd · Jan 22, 2019 · 8a10953 · 8a10953
1 parent e8caf46
commit 8a10953
Show file tree

Hide file tree

Showing 2 changed files with 156 additions and 3 deletions.
diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go
@@ -18,9 +18,11 @@ package v1alpha1
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/knative/pkg/apis"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/util/validation"
 )
@@ -76,9 +78,108 @@ func (ts *TaskSpec) Validate() *apis.FieldError {
 			}
 		}
 	}
+
+	if err := validateInputParameterVariables(ts.Steps, ts.Inputs); err != nil {
+		return err
+	}
+	if err := validateResourceVariables(ts.Steps, ts.Inputs, ts.Outputs); err != nil {
+		return err
+	}
 	return nil
 }
 
+func validateInputParameterVariables(steps []corev1.Container, inputs *Inputs) *apis.FieldError {
+	parameterNames := map[string]struct{}{}
+	if inputs != nil {
+		for _, p := range inputs.Params {
+			parameterNames[p.Name] = struct{}{}
+		}
+	}
+	return validateVariables(steps, "params", parameterNames)
+}
+
+func validateResourceVariables(steps []corev1.Container, inputs *Inputs, outputs *Outputs) *apis.FieldError {
+	resourceNames := map[string]struct{}{}
+	if inputs != nil {
+		for _, r := range inputs.Resources {
+			resourceNames[r.Name] = struct{}{}
+		}
+	}
+	if outputs != nil {
+		for _, r := range outputs.Resources {
+			resourceNames[r.Name] = struct{}{}
+		}
+	}
+	return validateVariables(steps, "resources", resourceNames)
+}
+
+func validateVariables(steps []corev1.Container, prefix string, vars map[string]struct{}) *apis.FieldError {
+	for _, step := range steps {
+		if err := validateVariable("name", step.Name, prefix, vars); err != nil {
+			return err
+		}
+		if err := validateVariable("image", step.Image, prefix, vars); err != nil {
+			return err
+		}
+		if err := validateVariable("workingDir", step.WorkingDir, prefix, vars); err != nil {
+			return err
+		}
+		for i, cmd := range step.Command {
+			if err := validateVariable(fmt.Sprintf("command[%d]", i), cmd, prefix, vars); err != nil {
+				return err
+			}
+		}
+		for i, arg := range step.Args {
+			if err := validateVariable(fmt.Sprintf("arg[%d]", i), arg, prefix, vars); err != nil {
+				return err
+			}
+		}
+		for _, env := range step.Env {
+			if err := validateVariable(fmt.Sprintf("env[%s]", env.Name), env.Value, prefix, vars); err != nil {
+				return err
+			}
+		}
+		for i, v := range step.VolumeMounts {
+			if err := validateVariable(fmt.Sprintf("volumeMount[%d].Name", i), v.Name, prefix, vars); err != nil {
+				return err
+			}
+			if err := validateVariable(fmt.Sprintf("volumeMount[%d].MountPath", i), v.MountPath, prefix, vars); err != nil {
+				return err
+			}
+			if err := validateVariable(fmt.Sprintf("volumeMount[%d].SubPath", i), v.SubPath, prefix, vars); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func validateVariable(name, value, prefix string, vars map[string]struct{}) *apis.FieldError {
+	if v, present := extractVariable(value, prefix); present {
+		if _, ok := vars[v]; !ok {
+			return &apis.FieldError{
+				Message: fmt.Sprintf("non-existent variable in %q for step %s", value, name),
+				Paths:   []string{"taskspec.steps." + name},
+			}
+		}
+	}
+	return nil
+}
+
+func extractVariable(s, prefix string) (string, bool) {
+	re := regexp.MustCompile("\\${(?:inputs|outputs)\\." + prefix + "\\.(.*)}")
+	if re.MatchString(s) {
+		// ${inputs.resources.foo} with prefix=resources -> [${inputs.resources.foo foo}]
+		// ${inputs.resources.foo.bar} with prefix=resources -> [${inputs.resources.foo foo.bar}]
+		v := re.FindStringSubmatch(s)[1]
+		// foo -> foo
+		// foo.bar -> foo
+		// foo.bar.baz -> foo
+		return strings.SplitN(v, ".", 2)[0], true
+	}
+	return "", false
+}
+
 func checkForDuplicates(resources []TaskResource, path string) *apis.FieldError {
 	encountered := map[string]struct{}{}
 	for _, r := range resources {

diff --git a/pkg/apis/pipeline/v1alpha1/task_validation_test.go b/pkg/apis/pipeline/v1alpha1/task_validation_test.go
@@ -39,8 +39,9 @@ var invalidBuildSteps = []corev1.Container{{
 
 func TestTaskSpec_Validate(t *testing.T) {
 	type fields struct {
-		Inputs  *Inputs
-		Outputs *Outputs
+		Inputs     *Inputs
+		Outputs    *Outputs
+		BuildSteps []corev1.Container
 	}
 	tests := []struct {
 		name   string
@@ -58,13 +59,15 @@ func TestTaskSpec_Validate(t *testing.T) {
 					},
 				},
 			},
+			BuildSteps: validBuildSteps,
 		},
 	}, {
 		name: "valid outputs",
 		fields: fields{
 			Outputs: &Outputs{
 				Resources: []TaskResource{validResource},
 			},
+			BuildSteps: validBuildSteps,
 		},
 	}, {
 		name: "both valid",
@@ -75,14 +78,37 @@ func TestTaskSpec_Validate(t *testing.T) {
 			Outputs: &Outputs{
 				Resources: []TaskResource{validResource},
 			},
+			BuildSteps: validBuildSteps,
+		},
+	}, {
+		name: "valid template variable",
+		fields: fields{
+			Inputs: &Inputs{
+				Resources: []TaskResource{{
+					Name: "foo",
+					Type: PipelineResourceTypeImage,
+				}},
+				Params: []TaskParam{{
+					Name: "baz",
+				}},
+			},
+			Outputs: &Outputs{
+				Resources: []TaskResource{validResource},
+			},
+			BuildSteps: []corev1.Container{{
+				Name:       "mystep",
+				Image:      "${inputs.resources.foo.url}",
+				Args:       []string{"--flag=${inputs.params.baz}"},
+				WorkingDir: "/foo/bar/${outputs.resources.source}",
+			}},
 		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ts := &TaskSpec{
 				Inputs:  tt.fields.Inputs,
 				Outputs: tt.fields.Outputs,
-				Steps:   validBuildSteps,
+				Steps:   tt.fields.BuildSteps,
 			}
 			if err := ts.Validate(); err != nil {
 				t.Errorf("TaskSpec.Validate() = %v", err)
@@ -195,6 +221,32 @@ func TestTaskSpec_ValidateError(t *testing.T) {
 			},
 			BuildSteps: invalidBuildSteps,
 		},
+	}, {
+		name: "inexistent input param variable",
+		fields: fields{
+			BuildSteps: []corev1.Container{{
+				Name:  "mystep",
+				Image: "myimage",
+				Args:  []string{"--flag=${inputs.params.inexistent}"},
+			}},
+		},
+	}, {
+		name: "inexistent input resource variable",
+		fields: fields{
+			BuildSteps: []corev1.Container{{
+				Name:  "mystep",
+				Image: "myimage:${inputs.resources.inputs}",
+			}},
+		},
+	}, {
+		name: "inexistent output param variable",
+		fields: fields{
+			BuildSteps: []corev1.Container{{
+				Name:       "mystep",
+				Image:      "myimage",
+				WorkingDir: "/foo/bar/${outputs.resources.inexistent}",
+			}},
+		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {