fix: incorrect templating not producing error

tektoncd · Apr 17, 2019 · 8f5dcbc · 8f5dcbc
1 parent 385a537
commit 8f5dcbc
Show file tree

Hide file tree

Showing 4 changed files with 118 additions and 23 deletions.
diff --git a/pkg/apis/pipeline/v1alpha1/resource_types.go b/pkg/apis/pipeline/v1alpha1/resource_types.go
@@ -164,3 +164,19 @@ func ResourceFromType(r *PipelineResource) (PipelineResourceInterface, error) {
 	}
 	return nil, fmt.Errorf("%s is an invalid or unimplemented PipelineResource", r.Spec.Type)
 }
+
+// AttributesFromType returns a list of available attributes that can be replaced
+// through templating.
+func AttributesFromType(prt PipelineResourceType) ([]string, error) {
+	r := &PipelineResource{}
+	r.Spec.Type = prt
+	resource, err := ResourceFromType(r)
+	if err != nil {
+		return nil, err
+	}
+	keys := []string{}
+	for k := range resource.Replacements() {
+		keys = append(keys, k)
+	}
+	return keys, nil
+}
diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go
@@ -132,67 +132,110 @@ func validateInputParameterVariables(steps []corev1.Container, inputs *Inputs) *
 			parameterNames[p.Name] = struct{}{}
 		}
 	}
-	return validateVariables(steps, "params", parameterNames)
+	return validateVariables(steps, "params", "inputs.", parameterNames)
 }
 
 func validateResourceVariables(steps []corev1.Container, inputs *Inputs, outputs *Outputs) *apis.FieldError {
-	resourceNames := map[string]struct{}{}
+	inputVars, err := getInputResourceVariables(inputs)
+	if err != nil {
+		return err
+	}
+	outputVars, err := getOutputResourceVariables(outputs)
+	if err != nil {
+		return err
+	}
+	err = validateVariables(steps, "resources", "inputs.", inputVars)
+	if err != nil {
+		return err
+	}
+	return validateVariables(steps, "resources", "outputs.", outputVars)
+}
+
+func getInputResourceVariables(inputs *Inputs) (map[string]struct{}, *apis.FieldError) {
+	vars := map[string]struct{}{}
 	if inputs != nil {
 		for _, r := range inputs.Resources {
-			resourceNames[r.Name] = struct{}{}
+			attrs, err := AttributesFromType(r.Type)
+			if err != nil {
+				return nil, &apis.FieldError{
+					Message: fmt.Sprintf("invalid resource type %s", r.Type),
+					Paths:   []string{"taskspec.inputs.resources." + r.Name},
+					Details: err.Error(),
+				}
+			}
+			for _, a := range attrs {
+				rv := r.Name + "." + a
+				vars[rv] = struct{}{}
+			}
 		}
 	}
+	return vars, nil
+}
+
+func getOutputResourceVariables(outputs *Outputs) (map[string]struct{}, *apis.FieldError) {
+	vars := map[string]struct{}{}
 	if outputs != nil {
 		for _, r := range outputs.Resources {
-			resourceNames[r.Name] = struct{}{}
+			attrs, err := AttributesFromType(r.Type)
+			if err != nil {
+				return nil, &apis.FieldError{
+					Message: fmt.Sprintf("invalid resource type %s", r.Type),
+					Paths:   []string{"taskspec.outputs.resources." + r.Name},
+					Details: err.Error(),
+				}
+			}
+			for _, a := range attrs {
+				rv := r.Name + "." + a
+				vars[rv] = struct{}{}
+			}
 		}
 	}
-	return validateVariables(steps, "resources", resourceNames)
+	return vars, nil
 }
 
-func validateVariables(steps []corev1.Container, prefix string, vars map[string]struct{}) *apis.FieldError {
+func validateVariables(steps []corev1.Container, prefix, contextPrefix string, vars map[string]struct{}) *apis.FieldError {
 	for _, step := range steps {
-		if err := validateTaskVariable("name", step.Name, prefix, vars); err != nil {
+		if err := validateTaskVariable("name", step.Name, prefix, contextPrefix, vars); err != nil {
 			return err
 		}
-		if err := validateTaskVariable("image", step.Image, prefix, vars); err != nil {
+		if err := validateTaskVariable("image", step.Image, prefix, contextPrefix, vars); err != nil {
 			return err
 		}
-		if err := validateTaskVariable("workingDir", step.WorkingDir, prefix, vars); err != nil {
+		if err := validateTaskVariable("workingDir", step.WorkingDir, prefix, contextPrefix, vars); err != nil {
 			return err
 		}
 		for i, cmd := range step.Command {
-			if err := validateTaskVariable(fmt.Sprintf("command[%d]", i), cmd, prefix, vars); err != nil {
+			if err := validateTaskVariable(fmt.Sprintf("command[%d]", i), cmd, prefix, contextPrefix, vars); err != nil {
 				return err
 			}
 		}
 		for i, arg := range step.Args {
-			if err := validateTaskVariable(fmt.Sprintf("arg[%d]", i), arg, prefix, vars); err != nil {
+			if err := validateTaskVariable(fmt.Sprintf("arg[%d]", i), arg, prefix, contextPrefix, vars); err != nil {
 				return err
 			}
 		}
 		for _, env := range step.Env {
-			if err := validateTaskVariable(fmt.Sprintf("env[%s]", env.Name), env.Value, prefix, vars); err != nil {
+			if err := validateTaskVariable(fmt.Sprintf("env[%s]", env.Name), env.Value, prefix, contextPrefix, vars); err != nil {
 				return err
 			}
 		}
 		for i, v := range step.VolumeMounts {
-			if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].Name", i), v.Name, prefix, vars); err != nil {
+			if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].Name", i), v.Name, prefix, contextPrefix, vars); err != nil {
 				return err
 			}
-			if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].MountPath", i), v.MountPath, prefix, vars); err != nil {
+			if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].MountPath", i), v.MountPath, prefix, contextPrefix, vars); err != nil {
 				return err
 			}
-			if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].SubPath", i), v.SubPath, prefix, vars); err != nil {
+			if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].SubPath", i), v.SubPath, prefix, contextPrefix, vars); err != nil {
 				return err
 			}
 		}
 	}
 	return nil
 }
 
-func validateTaskVariable(name, value, prefix string, vars map[string]struct{}) *apis.FieldError {
-	return templating.ValidateVariable(name, value, prefix, "(?:inputs|outputs).", "step", "taskspec.steps", vars)
+func validateTaskVariable(name, value, prefix, contextPrefix string, vars map[string]struct{}) *apis.FieldError {
+	return templating.ValidateVariable(name, value, prefix, contextPrefix, "step", "taskspec.steps", vars)
 }
 
 func checkForDuplicates(resources []TaskResource, path string) *apis.FieldError {

diff --git a/pkg/apis/pipeline/v1alpha1/task_validation_test.go b/pkg/apis/pipeline/v1alpha1/task_validation_test.go
@@ -105,7 +105,7 @@ func TestTaskSpec_Validate(t *testing.T) {
 				Name:       "mystep",
 				Image:      "${inputs.resources.foo.url}",
 				Args:       []string{"--flag=${inputs.params.baz} && ${input.params.foo-is-baz}"},
-				WorkingDir: "/foo/bar/${outputs.resources.source}",
+				WorkingDir: "/foo/bar/${outputs.resources.source.name}",
 			}},
 		},
 	}}
@@ -321,7 +321,46 @@ func TestTaskSpec_ValidateError(t *testing.T) {
 			Message: `non-existent variable in "${inputs.params.foo} && ${inputs.params.inexistent}" for step arg[0]`,
 			Paths:   []string{"taskspec.steps.arg[0]"},
 		},
-	}}
+	}, {
+		name: "invalid resource variable usage",
+		fields: fields{
+			Inputs: &Inputs{
+				Resources: []TaskResource{{
+					Name: "foo",
+					Type: PipelineResourceTypeImage,
+				}},
+			},
+			BuildSteps: []corev1.Container{{
+				Name:  "mystep",
+				Image: "myimage",
+				Args:  []string{"${inputs.resources.foo}"},
+			}},
+		},
+		expectedError: apis.FieldError{
+			Message: `non-existent variable in "${inputs.resources.foo}" for step arg[0]`,
+			Paths:   []string{"taskspec.steps.arg[0]"},
+		},
+	},
+		{
+			name: "inexistent output param variable",
+			fields: fields{
+				Inputs: &Inputs{
+					Resources: []TaskResource{{
+						Name: "foo",
+						Type: PipelineResourceTypeImage,
+					}},
+				},
+				BuildSteps: []corev1.Container{{
+					Name:  "mystep",
+					Image: "myimage",
+					Args:  []string{"${outputs.resources.foo.url}"},
+				}},
+			},
+			expectedError: apis.FieldError{
+				Message: `non-existent variable in "${outputs.resources.foo.url}" for step arg[0]`,
+				Paths:   []string{"taskspec.steps.arg[0]"},
+			},
+		}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ts := &TaskSpec{

diff --git a/pkg/templating/templating.go b/pkg/templating/templating.go
@@ -47,10 +47,7 @@ func extractVariablesFromString(s, prefix string) ([]string, bool) {
 	vars := make([]string, len(matches))
 	for i, match := range matches {
 		groups := matchGroups(match, re)
-		// foo -> foo
-		// foo.bar -> foo
-		// foo.bar.baz -> foo
-		vars[i] = strings.SplitN(groups["var"], ".", 2)[0]
+		vars[i] = groups["var"]
 	}
 	return vars, true
 }