Support JSONPath in TriggerBindings

This commit switches TriggerBinding params to use JSONPath instead of GJSON. To keep the commit small, removing existing GJSON will be part of a separate commit. Signed-off-by: Dibyo Mukherjee <[email protected]>
tektoncd · Dec 3, 2019 · 077ea13 · 077ea13
1 parent b6abf88
commit 077ea13
Show file tree

Hide file tree

Showing 5 changed files with 135 additions and 103 deletions.
diff --git a/docs/triggerbindings.md b/docs/triggerbindings.md
@@ -33,46 +33,44 @@ Each parameter has a `name` and a `value`.
 
 ## Event Variable Interpolation
 
-In order to parse generic events as efficiently as possible,
-[GJSON](https://github.com/tidwall/gjson) is used internally. As a result, the
-binding [path syntax](https://github.com/tidwall/gjson#path-syntax) differs
-slightly from standard JSON. As of now, the following patterns are supported
-within `TriggerBinding` parameter value interpolation: -
-`\$\(body(\.[[:alnum:]/_\-\.\\]+|\.#\([[:alnum:]=<>%!"\*_-]+\)#??)*\)` -
-`\$\(header(\.[[:alnum:]_\-]+)?\)`
+TriggerBindings can access values from the HTTP JSON body and the headers using
+JSONPath expressions. The expressions have to be wrapped in `$()` but can omit
+the curly braces `{}` and the leading `.`. 
+
+These are all valid expressions:
+```shell script
+$(body.key1)
+$(.body.key)
+$({.body.key1})
+$({body.key)
+```
+
+These are invalid expressions:
+```shell script
+{.body.key1} # INVALID - Not wrapped in $()
+$({body) # INVALID - Ending curly brace absent
+```
+
 
-### Body
+### Examples
 
-HTTP Post request body data can be referenced using variable interpolation. Text
-in the form of `$(body.X.Y.Z)` is replaced by the body data at JSON path
-`X.Y.Z`.
+``` shell script
 
 `$(body)` is replaced by the entire body.
 
-The following are some example variable interpolation replacements:
-``` $(body)
--> "{\"key1\": \"value1\", \"key2\": {\"key3\": \"value3\"}, \"key4\":
-[\"value4\", \"value5\"]}"
+$(body) -> "{"key1": "value1", "key2": {"key3": "value3"}, "key4": ["value4", "value5"]}"
 
 $(body.key1) -> "value1"
 
-$(body.key2) -> "{\"key3\": \"value3\"}"
+$(body.key2) -> "{"key3": "value3"}"
 
 $(body.key2.key3) -> "value3"
 
-$(body.key4.0) -> "value4"
-```
-
-### Header
+$(body.key4[0]) -> "value4"
 
-HTTP Post request header data can be referenced using variable interpolation.
-Text in the form of `$(header.X)` is replaced by the event's header named `X`.
+# $(header) is replaced by all of the headers from the event.
 
-`$(header)` is replaced by all of the headers from the event.
-
-The following are some example variable interpolation replacements:
-```
-$(header) -> "{\"One\":[\"one\"], \"Two\":[\"one\",\"two\",\"three\"]}"
+$(header) -> "{"One":["one"], "Two":["one","two","three"]}"
 
 $(header.One) -> "one"
 

diff --git a/pkg/template/event.go b/pkg/template/event.go
@@ -169,26 +169,63 @@ func getHeaderValue(header map[string][]string, headerName string) (string, erro
 func ResolveParams(bindings []*triggersv1.TriggerBinding, body []byte, header map[string][]string, params []pipelinev1.ParamSpec) ([]pipelinev1.Param, error) {
 	out, err := MergeBindingParams(bindings)
 	if err != nil {
-		return nil, xerrors.Errorf("error merging trigger params: %v", err)
+		return nil, xerrors.Errorf("error merging trigger params: %q", err)
 	}
-	out, err = ApplyBodyToParams(body, out)
+	event, err := newEvent(body, header)
 	if err != nil {
-		return nil, xerrors.Errorf("error applying body to trigger params: %s", err)
+		return nil, xerrors.Errorf("failed to create Event: %q", err)
 	}
-	out, err = ApplyHeaderToParams(header, out)
+	out, err = ApplyEventValuesToParams(out, event)
 	if err != nil {
-		return nil, xerrors.Errorf("error applying header to trigger params: %s", err)
+		return nil, xerrors.Errorf("failed to ApplyEventValuesToParams: %q", err)
 	}
-
 	return MergeInDefaultParams(out, params), nil
 }
 
+// ResolveResources resolves a templated resource by replacing params with their values.
 func ResolveResources(template *triggersv1.TriggerTemplate, params []pipelinev1.Param) []json.RawMessage {
 	resources := make([]json.RawMessage, len(template.Spec.ResourceTemplates))
-	uid := UID()
 	for i := range template.Spec.ResourceTemplates {
 		resources[i] = ApplyParamsToResourceTemplate(params, template.Spec.ResourceTemplates[i].RawMessage)
-		resources[i] = ApplyUIDToResourceTemplate(resources[i], uid)
+		resources[i] = ApplyUIDToResourceTemplate(resources[i], UID())
 	}
 	return resources
 }
+
+// event represents a HTTP event that Triggers processes
+type event struct {
+	Header map[string]string `json:"header"`
+	Body   interface{}       `json:"body"`
+}
+
+// newEvent returns a new Event from HTTP headers and body
+func newEvent(body []byte, headers map[string][]string) (*event, error) {
+	var data interface{}
+	if len(body) > 0 {
+		if err := json.Unmarshal(body, &data); err != nil {
+			return nil, xerrors.Errorf("failed to unmarshal request body: %q", err)
+		}
+	}
+	joinedHeaders := make(map[string]string, len(headers))
+	for k, v := range headers {
+		joinedHeaders[k] = strings.Join(v, ",")
+	}
+
+	return &event{
+		Header: joinedHeaders,
+		Body:   data,
+	}, nil
+}
+
+// ApplyEventValuesToParams returns a slice of Params with the JSONPath variables replaced
+// with values from the event body and headers.
+func ApplyEventValuesToParams(params []pipelinev1.Param, ec *event) ([]pipelinev1.Param, error) {
+	for idx, p := range params {
+		val, err := ParseJSONPath(ec, p.Value.StringVal)
+		if err != nil {
+			return nil, xerrors.Errorf("failed to replace JSONPath value for param %s: %s: %q", p.Name, p.Value, err)
+		}
+		params[idx].Value = pipelinev1.ArrayOrString{Type: pipelinev1.ParamTypeString, StringVal: val}
+	}
+	return params, nil
+}
diff --git a/pkg/template/event_test.go b/pkg/template/event_test.go
@@ -663,7 +663,7 @@ func Test_NewResources(t *testing.T) {
 	}{{
 		name: "empty",
 		args: args{
-			body:   json.RawMessage{},
+			body:   []byte{},
 			header: map[string][]string{},
 			binding: ResolvedTrigger{
 				TriggerTemplate: bldr.TriggerTemplate("tt", "namespace"),
@@ -799,7 +799,7 @@ func Test_NewResources(t *testing.T) {
 		},
 		want: []json.RawMessage{
 			json.RawMessage(`{"rt1": "bar-cbhtc", "cbhtc": "cbhtc"}`),
-			json.RawMessage(`{"rt2": "default2-cbhtc"}`),
+			json.RawMessage(`{"rt2": "default2-bsvjp"}`),
 			json.RawMessage(`{"rt3": "rt3"}`),
 		},
 	}, {
@@ -866,89 +866,85 @@ func Test_NewResources_error(t *testing.T) {
 		header   map[string][]string
 		elParams []pipelinev1.Param
 		binding  ResolvedTrigger
-	}{
-		{
-			name: "bodypath not found in body",
-			body: json.RawMessage(`{"foo": "bar"}`),
-			binding: ResolvedTrigger{
-				TriggerTemplate: bldr.TriggerTemplate("tt", "namespace",
-					bldr.TriggerTemplateSpec(
-						bldr.TriggerTemplateParam("param1", "description", ""),
-					),
+	}{{
+		name: "bodypath not found in body",
+		body: json.RawMessage(`{"foo": "bar"}`),
+		binding: ResolvedTrigger{
+			TriggerTemplate: bldr.TriggerTemplate("tt", "namespace",
+				bldr.TriggerTemplateSpec(
+					bldr.TriggerTemplateParam("param1", "description", ""),
 				),
-				TriggerBindings: []*triggersv1.TriggerBinding{
-					bldr.TriggerBinding("tb", "namespace",
-						bldr.TriggerBindingSpec(
-							bldr.TriggerBindingParam("param1", "$(body.bogusvalue)"),
-						),
+			),
+			TriggerBindings: []*triggersv1.TriggerBinding{
+				bldr.TriggerBinding("tb", "namespace",
+					bldr.TriggerBindingSpec(
+						bldr.TriggerBindingParam("param1", "$(body.bogusvalue)"),
 					),
-				},
+				),
 			},
 		},
-		{
-			name:   "header not found in event",
-			body:   json.RawMessage(`{"foo": "bar"}`),
-			header: map[string][]string{"One": {"one"}},
-			binding: ResolvedTrigger{
-				TriggerTemplate: bldr.TriggerTemplate("tt", "namespace",
-					bldr.TriggerTemplateSpec(
-						bldr.TriggerTemplateParam("param1", "description", ""),
-					),
+	}, {
+		name:   "header not found in event",
+		body:   json.RawMessage(`{"foo": "bar"}`),
+		header: map[string][]string{"One": {"one"}},
+		binding: ResolvedTrigger{
+			TriggerTemplate: bldr.TriggerTemplate("tt", "namespace",
+				bldr.TriggerTemplateSpec(
+					bldr.TriggerTemplateParam("param1", "description", ""),
 				),
-				TriggerBindings: []*triggersv1.TriggerBinding{
-					bldr.TriggerBinding("tb", "namespace",
-						bldr.TriggerBindingSpec(
-							bldr.TriggerBindingParam("param1", "$(header.bogusvalue)"),
-						),
+			),
+			TriggerBindings: []*triggersv1.TriggerBinding{
+				bldr.TriggerBinding("tb", "namespace",
+					bldr.TriggerBindingSpec(
+						bldr.TriggerBindingParam("param1", "$(header.bogusvalue)"),
 					),
-				},
+				),
 			},
 		},
-		{
-			name: "merge params error",
-			elParams: []pipelinev1.Param{
-				{
-					Name:  "param1",
-					Value: pipelinev1.ArrayOrString{StringVal: "value1", Type: pipelinev1.ParamTypeString},
-				},
+	}, {
+		name: "merge params error",
+		elParams: []pipelinev1.Param{
+			{
+				Name:  "param1",
+				Value: pipelinev1.ArrayOrString{StringVal: "value1", Type: pipelinev1.ParamTypeString},
 			},
-			binding: ResolvedTrigger{
-				TriggerTemplate: bldr.TriggerTemplate("tt", "namespace",
-					bldr.TriggerTemplateSpec(
-						bldr.TriggerTemplateParam("param1", "description", ""),
-					),
+		},
+		binding: ResolvedTrigger{
+			TriggerTemplate: bldr.TriggerTemplate("tt", "namespace",
+				bldr.TriggerTemplateSpec(
+					bldr.TriggerTemplateParam("param1", "description", ""),
 				),
-				TriggerBindings: []*triggersv1.TriggerBinding{
-					bldr.TriggerBinding("tb", "namespace",
-						bldr.TriggerBindingSpec(
-							bldr.TriggerBindingParam("param1", "$(body.bogusvalue)"),
-						),
+			),
+			TriggerBindings: []*triggersv1.TriggerBinding{
+				bldr.TriggerBinding("tb", "namespace",
+					bldr.TriggerBindingSpec(
+						bldr.TriggerBindingParam("param1", "$(body.bogusvalue)"),
 					),
-				},
+				),
 			},
 		},
-		{
-			name: "conflicting bindings",
-			binding: ResolvedTrigger{
-				TriggerTemplate: bldr.TriggerTemplate("tt", "namespace",
-					bldr.TriggerTemplateSpec(
-						bldr.TriggerTemplateParam("param1", "description", ""),
-					),
+	}, {
+		name: "conflicting bindings",
+		binding: ResolvedTrigger{
+			TriggerTemplate: bldr.TriggerTemplate("tt", "namespace",
+				bldr.TriggerTemplateSpec(
+					bldr.TriggerTemplateParam("param1", "description", ""),
 				),
-				TriggerBindings: []*triggersv1.TriggerBinding{
-					bldr.TriggerBinding("tb", "namespace",
-						bldr.TriggerBindingSpec(
-							bldr.TriggerBindingParam("param1", "foo"),
-						),
+			),
+			TriggerBindings: []*triggersv1.TriggerBinding{
+				bldr.TriggerBinding("tb", "namespace",
+					bldr.TriggerBindingSpec(
+						bldr.TriggerBindingParam("param1", "foo"),
 					),
-					bldr.TriggerBinding("tb2", "namespace",
-						bldr.TriggerBindingSpec(
-							bldr.TriggerBindingParam("param1", "bar"),
-						),
+				),
+				bldr.TriggerBinding("tb2", "namespace",
+					bldr.TriggerBindingSpec(
+						bldr.TriggerBindingParam("param1", "bar"),
 					),
-				},
+				),
 			},
 		},
+	},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/pkg/template/resource.go b/pkg/template/resource.go
@@ -42,7 +42,7 @@ type ResolvedTrigger struct {
 type getTriggerBinding func(name string, options metav1.GetOptions) (*triggersv1.TriggerBinding, error)
 type getTriggerTemplate func(name string, options metav1.GetOptions) (*triggersv1.TriggerTemplate, error)
 
-// ResolveBindings takes in a trigger containing object refs to bindings and
+// ResolveTrigger takes in a trigger containing object refs to bindings and
 // templates and resolves them to their underlying values.
 func ResolveTrigger(trigger triggersv1.EventListenerTrigger, getTB getTriggerBinding, getTT getTriggerTemplate) (ResolvedTrigger, error) {
 	tb := make([]*triggersv1.TriggerBinding, 0, len(trigger.Bindings))

diff --git a/third_party/VENDOR-LICENSE b/third_party/VENDOR-LICENSE
@@ -7517,3 +7517,4 @@ Import: github.com/tektoncd/triggers/vendor/knative.dev/pkg
    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.
+
Original file line number	Diff line number	Diff line change
Expand Up		@@ -7517,3 +7517,4 @@ Import: github.com/tektoncd/triggers/vendor/knative.dev/pkg
		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.