Adding TriggerBinding validation for parameter values

This eliminates the concern about nested Tekton expressions that could possibly be parsed in sink processing.

pkg/apis/triggers/v1alpha1/trigger_binding_validation.go

@@ -18,29 +18,71 @@ package v1alpha1
 
 import (
 	"context"
+	"fmt"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/util/sets"
 	"knative.dev/pkg/apis"
 )
 
 // Validate TriggerBinding.
 func (tb *TriggerBinding) Validate(ctx context.Context) *apis.FieldError {
-	return tb.Spec.Validate(ctx)
+	return tb.Spec.Validate(ctx).ViaField("spec")
 }
 
 // Validate TriggerBindingSpec.
 func (s *TriggerBindingSpec) Validate(ctx context.Context) *apis.FieldError {
-	return validateParams(s.Params)
+	return validateParams(s.Params).ViaField("params")
 }
 
 func validateParams(params []Param) *apis.FieldError {
 	// Ensure there aren't multiple params with the same name.
 	seen := sets.NewString()
-	for _, param := range params {
+	for i, param := range params {
 		if seen.Has(param.Name) {
-			return apis.ErrMultipleOneOf("spec.params")
+			return apis.ErrMultipleOneOf(fmt.Sprintf("[%d].name", i))
 		}
 		seen.Insert(param.Name)
+		errs := validateParamValue(param.Value).ViaField(fmt.Sprintf("[%d]", i))
+		if errs != nil {
+			return errs
+		}
 	}
 	return nil
 }
+
+func validateParamValue(in string) *apis.FieldError {
+	if !strings.Contains(in, "$(") {
+		return nil
+	}
+	// Splits string on $( to find potential Tekton expressions
+	maybeExpressions := strings.Split(in, "$(")
+	terminated := true
+	for _, e := range maybeExpressions[1:] { // Split always returns at least one element
+		// Iterate until we find the first unbalanced )
+		numOpenBrackets := 0
+		if !terminated {
+			return apis.ErrInvalidValue(in, "value")
+		}
+		terminated = false
+		for _, ch := range e {
+			switch ch {
+			case '(':
+				numOpenBrackets++
+			case ')':
+				numOpenBrackets--
+				if numOpenBrackets < 0 {
+					terminated = true
+				}
+			default:
+				continue
+			}
+			if numOpenBrackets < 0 {
+				terminated = true
+				break
+			}
+		}
+	}
+	return nil
+
+}

pkg/apis/triggers/v1alpha1/trigger_binding_validation_test.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
 	bldr "github.com/tektoncd/triggers/test/builder"
 )
@@ -37,7 +38,8 @@ func Test_TriggerBindingValidate(t *testing.T) {
 			bldr.TriggerBindingSpec(
 				bldr.TriggerBindingParam("param1", "$(body.input1)"),
 				bldr.TriggerBindingParam("param2", "$(body.input2)"),
-				bldr.TriggerBindingParam("param3", "$(body.input3)"),
+				bldr.TriggerBindingParam("param3", "$(body.(input3))"),
+				bldr.TriggerBindingParam("param4", "static-input"),
 			)),
 	}, {
 		name: "multiple params case sensitive",
@@ -47,6 +49,12 @@ func Test_TriggerBindingValidate(t *testing.T) {
 				bldr.TriggerBindingParam("PARAM1", "$(body.input2)"),
 				bldr.TriggerBindingParam("Param1", "$(body.input3)"),
 			)),
+	}, {
+		name: "multiple expressions in one body",
+		tb: bldr.TriggerBinding("name", "namespace",
+			bldr.TriggerBindingSpec(
+				bldr.TriggerBindingParam("param1", "$(body.input1)-$(body.input2)"),
+			)),
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -59,8 +67,9 @@ func Test_TriggerBindingValidate(t *testing.T) {
 
 func Test_TriggerBindingValidate_error(t *testing.T) {
 	tests := []struct {
-		name string
-		tb   *v1alpha1.TriggerBinding
+		name   string
+		tb     *v1alpha1.TriggerBinding
+		errMsg string
 	}{{
 		name: "duplicate params",
 		tb: bldr.TriggerBinding("name", "namespace",
@@ -69,12 +78,38 @@ func Test_TriggerBindingValidate_error(t *testing.T) {
 				bldr.TriggerBindingParam("param1", "$(body.param1)"),
 				bldr.TriggerBindingParam("param3", "$(body.param1)"),
 			)),
+		errMsg: "expected exactly one, got both: spec.params[1].name",
+	}, {
+		name: "invalid parameter",
+		tb: bldr.TriggerBinding("name", "namespace",
+			bldr.TriggerBindingSpec(
+				bldr.TriggerBindingParam("param1", "$($(body.param1))"),
+			)),
+		errMsg: "invalid value: $($(body.param1)): spec.params[0].value",
+	}, {
+		name: "invalid parameter further nested",
+		tb: bldr.TriggerBinding("name", "namespace",
+			bldr.TriggerBindingSpec(
+				bldr.TriggerBindingParam("param1", "$(body.test-$(body.param1))"),
+			)),
+		errMsg: "invalid value: $(body.test-$(body.param1)): spec.params[0].value",
+	}, {
+		name: "invalid parameter triple nested",
+		tb: bldr.TriggerBinding("name", "namespace",
+			bldr.TriggerBindingSpec(
+				bldr.TriggerBindingParam("param1", "$($($(body.param1)))"),
+			)),
+		errMsg: "invalid value: $($($(body.param1))): spec.params[0].value",
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if err := tt.tb.Validate(context.Background()); err == nil {
+			err := tt.tb.Validate(context.Background())
+			if err == nil {
 				t.Errorf("TriggerBinding.Validate() expected error for TriggerBinding: %v", tt.tb)
 			}
+			if diff := cmp.Diff(tt.errMsg, err.Error()); diff != "" {
+				t.Errorf("-want +got: %s", diff)
+			}
 		})
 	}
 }