From 4b3dd7f748603dcf3c5b5f7889724aa74cb21396 Mon Sep 17 00:00:00 2001 From: Dibyo Mukherjee Date: Tue, 10 Dec 2019 11:15:48 -0500 Subject: [PATCH] Support JSONPath in TriggerBindings This commit switches the expression syntax in TriggerBindings from GJson to JSONPath. Fixes #178 --- docs/triggerbindings.md | 48 +- pkg/template/event.go | 203 +++------ pkg/template/event_test.go | 817 ++++++---------------------------- pkg/template/resource.go | 11 +- pkg/template/resource_test.go | 12 + test/eventlistener_test.go | 5 +- 6 files changed, 223 insertions(+), 873 deletions(-) diff --git a/docs/triggerbindings.md b/docs/triggerbindings.md index cef345c54f..0265a88eaa 100644 --- a/docs/triggerbindings.md +++ b/docs/triggerbindings.md @@ -33,46 +33,40 @@ 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 wrapped in `$()`. -### Body +These are all valid expressions: +```shell script +$(body.key1) +$(.body.key) +``` + +These are invalid expressions: +```shell script +.body.key1 # INVALID - Not wrapped in $() +$({body) # INVALID - Ending curly brace absent +``` + +### 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 index 63bbe2887f..437a1c4b1e 100644 --- a/pkg/template/event.go +++ b/pkg/template/event.go @@ -18,177 +18,82 @@ package template import ( "encoding/json" - "regexp" + "fmt" + "net/http" "strings" pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" - "github.com/tidwall/gjson" - "golang.org/x/xerrors" ) -var ( - // bodyPathVarRegex determines valid body path variables - // The body regular expression allows for a subset of GJSON syntax, the mininum - // required to navigate through dictionaries, query arrays and support - // namespaced label names e.g. tekton.dev/eventlistener - bodyPathVarRegex = regexp.MustCompile(`\$\(body(\.[[:alnum:]/_\-\.\\]+|\.#\([[:alnum:]=<>%!"\*_-]+\)#??)*\)`) - - // The headers regular expression allows for simple navigation down a hierarchy - // of dictionaries - headerVarRegex = regexp.MustCompile(`\$\(header(\.[[:alnum:]_\-]+)?\)`) -) - -// getBodyPathFromVar returns the body path given an body path variable -// $(body.my.path) -> my.path -// $(body) returns an empty string "" because there is no body path -func getBodyPathFromVar(bodyPathVar string) string { - // Assume bodyPathVar matches the bodyPathVarRegex - if bodyPathVar == "$(body)" { - return "" - } - return strings.TrimSuffix(strings.TrimPrefix(bodyPathVar, "$(body."), ")") -} - -// getHeaderFromVar returns the header given a header variable -// $(header.example) -> example -func getHeaderFromVar(headerVar string) string { - // Assume headerVar matches the headerVarRegex - if headerVar == "$(header)" { - return "" - } - - return strings.TrimSuffix(strings.TrimPrefix(headerVar, "$(header."), ")") -} - -// ApplyBodyToParams returns the params with each body path variable replaced -// with the appropriate data from the body. Returns an error when the body -// path variable is not found in the body. -func ApplyBodyToParams(body []byte, params []pipelinev1.Param) ([]pipelinev1.Param, error) { - for i := range params { - param, err := applyBodyToParam(body, params[i]) - if err != nil { - return nil, err - } - params[i] = param - } - return params, nil -} - -// applyBodyToParam returns the param with each body path variable replaced -// with the appropriate data from the body. Returns an error when the body -// path variable is not found in the body. -func applyBodyToParam(body []byte, param pipelinev1.Param) (pipelinev1.Param, error) { - // Get each body path variable in the param - bodyPathVars := bodyPathVarRegex.FindAllString(param.Value.StringVal, -1) - for _, bodyPathVar := range bodyPathVars { - bodyPath := getBodyPathFromVar(bodyPathVar) - bodyPathValue, err := getBodyPathValue(body, bodyPath) - if err != nil { - return param, err - } - param.Value.StringVal = strings.Replace(param.Value.StringVal, bodyPathVar, bodyPathValue, -1) - } - return param, nil -} - -// getBodyPathValue returns the value of the bodyPath in the body. An error -// is returned if the bodyPath is not found in the body. -func getBodyPathValue(body []byte, bodyPath string) (string, error) { - var bodyPathValue string - if bodyPath == "" { - // $(body) has an empty bodyPath, so use the entire body as the bodyValue - bodyPathValue = string(body) - } else { - bodyPathResult := gjson.GetBytes(body, bodyPath) - if bodyPathResult.Index == 0 { - return "", xerrors.Errorf("Error body path %s not found in the body %s", bodyPath, string(body)) - } - bodyPathValue = bodyPathResult.String() - if bodyPathResult.Type == gjson.Null { - bodyPathValue = "null" - } - } - return strings.Replace(bodyPathValue, `"`, `\"`, -1), nil -} - -// ApplyHeaderToParams returns the params with each header variable replaced -// with the appropriate header value. Returns an error when the header variable -// is not found. -func ApplyHeaderToParams(header map[string][]string, params []pipelinev1.Param) ([]pipelinev1.Param, error) { - for i := range params { - param, err := applyHeaderToParam(header, params[i]) - if err != nil { - return nil, err - } - params[i] = param - } - return params, nil -} - -// applyHeaderToParam returns the param with each header variable replaced -// with the appropriate header value. Returns an error when the header variable -// is not found. -func applyHeaderToParam(header map[string][]string, param pipelinev1.Param) (pipelinev1.Param, error) { - // Get each header variable in the param - headerVars := headerVarRegex.FindAllString(param.Value.StringVal, -1) - for _, headerVar := range headerVars { - headerName := getHeaderFromVar(headerVar) - headerValue, err := getHeaderValue(header, headerName) - if err != nil { - return param, err - } - param.Value.StringVal = strings.Replace(param.Value.StringVal, headerVar, headerValue, -1) - } - return param, nil -} - -// getHeaderValue returns a string representation of the headerName in the event -// header. An error is returned if the headerName is not found in the header. -func getHeaderValue(header map[string][]string, headerName string) (string, error) { - var headerValue string - if headerName == "" { - // $(header) has an empty headerName, so use all the headers in the headerValue - b, err := json.Marshal(&header) - if err != nil { - return "", xerrors.Errorf("Error marshalling header %s: %s", header, err) - } - headerValue = string(b) - } else { - value, ok := header[headerName] - if !ok { - return "", xerrors.Errorf("Error headerName %s not found in the event header %s", headerName, header) - } - headerValue = strings.Join(value, " ") - } - return strings.Replace(headerValue, `"`, `\"`, -1), nil -} - // ResolveParams takes a given trigger binding and produces the resulting // resource params. -func ResolveParams(bindings []*triggersv1.TriggerBinding, body []byte, header map[string][]string, params []pipelinev1.ParamSpec) ([]pipelinev1.Param, error) { +func ResolveParams(bindings []*triggersv1.TriggerBinding, body []byte, header http.Header, 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, fmt.Errorf("error merging trigger params: %w", 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, fmt.Errorf("failed to create Event: %w", 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, fmt.Errorf("failed to ApplyEventValuesToParams: %w", 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 http.Header) (*event, error) { + var data interface{} + if len(body) > 0 { + if err := json.Unmarshal(body, &data); err != nil { + return nil, fmt.Errorf("failed to unmarshal request body: %w", 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 { + pValue := p.Value.StringVal + // Find all expressions wrapped in $() from the value + expressions := tektonVar.FindAllString(pValue, -1) + for _, expr := range expressions { + val, err := ParseJSONPath(ec, expr) + if err != nil { + return nil, fmt.Errorf("failed to replace JSONPath value for param %s: %s: %w", p.Name, p.Value, err) + } + pValue = strings.ReplaceAll(pValue, expr, val) + } + params[idx].Value = pipelinev1.ArrayOrString{Type: pipelinev1.ParamTypeString, StringVal: pValue} + } + return params, nil +} diff --git a/pkg/template/event_test.go b/pkg/template/event_test.go index b7c35585e5..ce3ee80e4b 100644 --- a/pkg/template/event_test.go +++ b/pkg/template/event_test.go @@ -18,7 +18,6 @@ package template import ( "encoding/json" - "strings" "testing" "github.com/google/go-cmp/cmp" @@ -28,628 +27,7 @@ import ( "k8s.io/apimachinery/pkg/util/rand" ) -func TestBodyPathVarRegex(t *testing.T) { - tests := []string{ - "$(body)", - "$(body.a-b)", - "$(body.a1)", - "$(body.a.b)", - "$(body.a.b.c)", - "$(body.1.b.c\\.e/f)", - "$(body.#(a==b))", - "$(body.#(a>1)#)", - "$(body.#(a%\"D*\")#.c)", - "$(body.#(a!%\"D*\").c)", - } - for _, bodyPathVar := range tests { - t.Run(bodyPathVar, func(t *testing.T) { - if !bodyPathVarRegex.MatchString(bodyPathVar) { - t.Errorf("bodyPathVarRegex.MatchString(%s) = false, want = true", bodyPathVar) - } - }) - } -} - -func TestBodyPathVarRegex_invalid(t *testing.T) { - tests := []string{ - "$body", - "$[body]", - "${body}", - "$(body.)", - "$(body.@)", - "$(body.$a)", - "$(body#a)", - "$(body@#)", - "body.a", - "body", - "${{body}", - "${body", - } - for _, bodyPathVar := range tests { - t.Run(bodyPathVar, func(t *testing.T) { - if bodyPathVarRegex.MatchString(bodyPathVar) { - t.Errorf("bodyPathVarRegex.MatchString(%s) = true, want = false", bodyPathVar) - } - }) - } -} - -func TestHeaderVarRegex(t *testing.T) { - tests := []string{ - "$(header)", - "$(header.a-b)", - "$(header.a1)", - } - for _, headerVar := range tests { - t.Run(headerVar, func(t *testing.T) { - if !headerVarRegex.MatchString(headerVar) { - t.Errorf("headerVarRegex.MatchString(%s) = false, want = true", headerVar) - } - }) - } -} - -func TestHeaderVarRegex_invalid(t *testing.T) { - tests := []string{ - "$(header.a.b)", - "$(header.a.b.c)", - "$header", - "$[header]", - "${header}", - "$(header.)", - "$(header..)", - "$(header.$a)", - "header.a", - "header", - "${{header}", - "${header", - } - for _, headerVar := range tests { - t.Run(headerVar, func(t *testing.T) { - if headerVarRegex.MatchString(headerVar) { - t.Errorf("headerVarRegex.MatchString(%s) = true, want = false", headerVar) - } - }) - } -} - -func TestGetBodyPathFromVar(t *testing.T) { - tests := []struct { - bodyPathVar string - want string - }{ - {bodyPathVar: "$(body)", want: ""}, - {bodyPathVar: "$(body.a-b)", want: "a-b"}, - {bodyPathVar: "$(body.a1)", want: "a1"}, - {bodyPathVar: "$(body.a.b)", want: "a.b"}, - {bodyPathVar: "$(body.a.b.c)", want: "a.b.c"}, - } - for _, tt := range tests { - t.Run(tt.bodyPathVar, func(t *testing.T) { - if bodyPath := getBodyPathFromVar(tt.bodyPathVar); bodyPath != tt.want { - t.Errorf("getBodyPathFromVar() = %s, want = %s", bodyPath, tt.want) - } - }) - } -} - -func TestGetHeaderFromVar(t *testing.T) { - tests := []struct { - headerVar string - want string - }{ - {headerVar: "$(header)", want: ""}, - {headerVar: "$(header.a-b)", want: "a-b"}, - {headerVar: "$(header.a1)", want: "a1"}, - {headerVar: "$(header.a.b)", want: "a.b"}, - } - for _, tt := range tests { - t.Run(tt.headerVar, func(t *testing.T) { - if header := getHeaderFromVar(tt.headerVar); header != tt.want { - t.Errorf("getHeaderFromVar() = %s, want = %s", header, tt.want) - } - }) - } -} - -func Test_getBodyPathValue(t *testing.T) { - body := `{"empty": "", "null": null, "one": "one", "two": {"two": "twovalue"}, "three": {"three": {"three": {"three": {"three": "threevalue"}}}}}` - bodyJSON := json.RawMessage(body) - type args struct { - body []byte - bodyPath string - } - tests := []struct { - args args - want string - }{{ - args: args{ - body: bodyJSON, - bodyPath: "", - }, - want: strings.Replace(body, `"`, `\"`, -1), - }, { - args: args{ - body: bodyJSON, - bodyPath: "one", - }, - want: "one", - }, { - args: args{ - body: bodyJSON, - bodyPath: "two", - }, - want: `{\"two\": \"twovalue\"}`, - }, { - args: args{ - body: bodyJSON, - bodyPath: "three.three.three.three.three", - }, - want: "threevalue", - }, { - args: args{ - body: bodyJSON, - bodyPath: "empty", - }, - want: "", - }, { - args: args{ - body: bodyJSON, - bodyPath: "null", - }, - want: "null", - }} - for _, tt := range tests { - t.Run(tt.args.bodyPath, func(t *testing.T) { - got, err := getBodyPathValue(tt.args.body, tt.args.bodyPath) - if err != nil { - t.Errorf("getBodyPathValue() error: %s", err) - } else if diff := cmp.Diff(tt.want, got); diff != "" { - t.Errorf("getBodyPathValue(): -want +got: %s", diff) - } - }) - } -} - -func Test_getBodyPathValue_error(t *testing.T) { - bodyJSON := json.RawMessage(`{"one": "onevalue", "two": {"two": "twovalue"}, "three": {"three": {"three": {"three": {"three": "threevalue"}}}}}`) - tests := []struct { - body []byte - bodyPath string - }{{ - body: bodyJSON, - bodyPath: "boguspath", - }, { - body: bodyJSON, - bodyPath: "two.bogus", - }, { - body: bodyJSON, - bodyPath: "three.three.bogus.three", - }, - } - for _, tt := range tests { - t.Run(tt.bodyPath, func(t *testing.T) { - got, err := getBodyPathValue(tt.body, tt.bodyPath) - if err == nil { - t.Errorf("getBodyPathValue() did not return error when expected; got: %s", got) - } - }) - } -} - -func Test_getHeaderValue(t *testing.T) { - header := map[string][]string{"one": {"one"}, "two": {"one", "two"}, "three": {"one", "two", "three"}} - type args struct { - header map[string][]string - headerName string - } - tests := []struct { - args args - want string - }{{ - args: args{ - header: header, - headerName: "", - }, - want: `{\"one\":[\"one\"],\"three\":[\"one\",\"two\",\"three\"],\"two\":[\"one\",\"two\"]}`, - }, { - args: args{ - header: header, - headerName: "one", - }, - want: "one", - }, { - args: args{ - header: header, - headerName: "two", - }, - want: "one two", - }, { - args: args{ - header: header, - headerName: "three", - }, - want: "one two three", - }} - for _, tt := range tests { - t.Run(tt.args.headerName, func(t *testing.T) { - got, err := getHeaderValue(tt.args.header, tt.args.headerName) - if err != nil { - t.Errorf("getHeaderValue() error: %s", err) - } else if diff := cmp.Diff(tt.want, got); diff != "" { - t.Errorf("getHeaderValue(): -want +got: %s", diff) - } - }) - } -} - -func Test_getHeaderValue_error(t *testing.T) { - header := map[string][]string{"one": {"one"}} - tests := []struct { - header map[string][]string - headerName string - }{{ - header: header, - headerName: "bogusheadername", - }} - for _, tt := range tests { - t.Run(tt.headerName, func(t *testing.T) { - got, err := getHeaderValue(tt.header, tt.headerName) - if err == nil { - t.Errorf("getHeaderValue() did not return error when expected; got: %s", got) - } - }) - } -} - -var ( - testBodyJSON = json.RawMessage(`{"one": "onevalue", "two": {"two": "twovalue"}, "three": {"three": {"three": {"three": {"three": "threevalue"}}}}}`) - paramNoBodyPathVar = pipelinev1.Param{ - Name: "paramNoBodyPathVar", - Value: pipelinev1.ArrayOrString{StringVal: "bar"}, - } - wantParamNoBodyPathVar = pipelinev1.Param{ - Name: "paramNoBodyPathVar", - Value: pipelinev1.ArrayOrString{StringVal: "bar"}, - } - paramOneBodyPathVar = pipelinev1.Param{ - Name: "paramOneBodyPathVar", - Value: pipelinev1.ArrayOrString{StringVal: "bar-$(body.one)-bar"}, - } - wantParamOneBodyPathVar = pipelinev1.Param{ - Name: "paramOneBodyPathVar", - Value: pipelinev1.ArrayOrString{StringVal: "bar-onevalue-bar"}, - } - paramMultipleIdenticalBodyPathVars = pipelinev1.Param{ - Name: "paramMultipleIdenticalBodyPathVars", - Value: pipelinev1.ArrayOrString{StringVal: "bar-$(body.one)-$(body.one)-$(body.one)-bar"}, - } - wantParamMultipleIdenticalBodyPathVars = pipelinev1.Param{ - Name: "paramMultipleIdenticalBodyPathVars", - Value: pipelinev1.ArrayOrString{StringVal: "bar-onevalue-onevalue-onevalue-bar"}, - } - paramMultipleUniqueBodyPathVars = pipelinev1.Param{ - Name: "paramMultipleUniqueBodyPathVars", - Value: pipelinev1.ArrayOrString{StringVal: "bar-$(body.one)-$(body.two.two)-$(body.three.three.three.three.three)-bar"}, - } - wantParamMultipleUniqueBodyPathVars = pipelinev1.Param{ - Name: "paramMultipleUniqueBodyPathVars", - Value: pipelinev1.ArrayOrString{StringVal: "bar-onevalue-twovalue-threevalue-bar"}, - } - paramSubobjectBodyPathVar = pipelinev1.Param{ - Name: "paramSubobjectBodyPathVar", - Value: pipelinev1.ArrayOrString{StringVal: "bar-$(body.three)-bar"}, - } - wantParamSubobjectBodyPathVar = pipelinev1.Param{ - Name: "paramSubobjectBodyPathVar", - Value: pipelinev1.ArrayOrString{StringVal: `bar-{\"three\": {\"three\": {\"three\": {\"three\": \"threevalue\"}}}}-bar`}, - } - paramEntireBodyPathVar = pipelinev1.Param{ - Name: "paramEntireBodyPathVar", - Value: pipelinev1.ArrayOrString{StringVal: "bar-$(body)-bar"}, - } - wantParamEntireBodyPathVar = pipelinev1.Param{ - Name: "paramEntireBodyPathVar", - Value: pipelinev1.ArrayOrString{StringVal: `bar-{\"one\": \"onevalue\", \"two\": {\"two\": \"twovalue\"}, \"three\": {\"three\": {\"three\": {\"three\": {\"three\": \"threevalue\"}}}}}-bar`}, - } - paramOneBogusBodyPathVar = pipelinev1.Param{ - Name: "paramOneBogusBodyPathVar", - Value: pipelinev1.ArrayOrString{StringVal: "bar-$(body.bogus.path)-bar"}, - } - paramMultipleBogusBodyPathVars = pipelinev1.Param{ - Name: "paramMultipleBogusBodyPathVars", - Value: pipelinev1.ArrayOrString{StringVal: "bar-$(body.bogus.path)-$(body.two.bogus)-$(body.three.bogus)-bar"}, - } -) - -func Test_applyBodyToParam(t *testing.T) { - type args struct { - body []byte - param pipelinev1.Param - } - tests := []struct { - args args - want pipelinev1.Param - }{{ - args: args{body: []byte{}, param: paramNoBodyPathVar}, - want: wantParamNoBodyPathVar, - }, { - args: args{body: testBodyJSON, param: paramOneBodyPathVar}, - want: wantParamOneBodyPathVar, - }, { - args: args{body: testBodyJSON, param: paramMultipleIdenticalBodyPathVars}, - want: wantParamMultipleIdenticalBodyPathVars, - }, { - args: args{body: testBodyJSON, param: paramMultipleUniqueBodyPathVars}, - want: wantParamMultipleUniqueBodyPathVars, - }, { - args: args{body: testBodyJSON, param: paramEntireBodyPathVar}, - want: wantParamEntireBodyPathVar, - }, { - args: args{body: testBodyJSON, param: paramSubobjectBodyPathVar}, - want: wantParamSubobjectBodyPathVar, - }} - for _, tt := range tests { - t.Run(tt.args.param.Value.StringVal, func(t *testing.T) { - got, err := applyBodyToParam(tt.args.body, tt.args.param) - if err != nil { - t.Errorf("applyBodyToParam() error = %v", err) - } else if diff := cmp.Diff(tt.want, got); diff != "" { - t.Errorf("applyBodyToParam(): -want +got: %s", diff) - } - }) - } -} - -func Test_applyBodyToParam_error(t *testing.T) { - tests := []struct { - body []byte - param pipelinev1.Param - }{{ - body: testBodyJSON, - param: paramOneBogusBodyPathVar, - }, { - body: testBodyJSON, - param: paramMultipleBogusBodyPathVars, - }} - for _, tt := range tests { - t.Run(tt.param.Value.StringVal, func(t *testing.T) { - got, err := applyBodyToParam(tt.body, tt.param) - if err == nil { - t.Errorf("applyBodyToParam() did not return error when expected; got: %v", got) - } - }) - } -} - -func Test_ApplyBodyToParams(t *testing.T) { - type args struct { - body []byte - params []pipelinev1.Param - } - tests := []struct { - name string - args args - want []pipelinev1.Param - }{{ - name: "empty params", - args: args{ - body: testBodyJSON, - params: []pipelinev1.Param{}, - }, - want: []pipelinev1.Param{}, - }, { - name: "one param", - args: args{ - body: testBodyJSON, - params: []pipelinev1.Param{paramOneBodyPathVar}, - }, - want: []pipelinev1.Param{wantParamOneBodyPathVar}, - }, { - name: "multiple params", - args: args{ - body: testBodyJSON, - params: []pipelinev1.Param{ - paramOneBodyPathVar, - paramMultipleUniqueBodyPathVars, - paramSubobjectBodyPathVar, - }, - }, - want: []pipelinev1.Param{ - wantParamOneBodyPathVar, - wantParamMultipleUniqueBodyPathVars, - wantParamSubobjectBodyPathVar, - }, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := ApplyBodyToParams(tt.args.body, tt.args.params) - if err != nil { - t.Errorf("ApplyBodyToParams() error = %v", err) - return - } - if diff := cmp.Diff(tt.want, got); diff != "" { - t.Errorf("ApplyBodyToParams(): -want +got: %s", diff) - } - }) - } -} - -func Test_ApplyBodyToParams_error(t *testing.T) { - type args struct { - body []byte - params []pipelinev1.Param - } - tests := []struct { - name string - args args - }{{ - name: "error one bodypath not found", - args: args{ - body: testBodyJSON, - params: []pipelinev1.Param{ - paramOneBogusBodyPathVar, - paramMultipleUniqueBodyPathVars, - paramSubobjectBodyPathVar, - }, - }, - }, { - name: "error multiple bodypaths not found", - args: args{ - body: testBodyJSON, - params: []pipelinev1.Param{ - paramOneBogusBodyPathVar, - paramMultipleBogusBodyPathVars, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := ApplyBodyToParams(tt.args.body, tt.args.params) - if err == nil { - t.Errorf("ApplyBodyToParams() did not return error when expected; got: %v", got) - } - }) - } -} - -func Test_applyHeaderToParams(t *testing.T) { - header := map[string][]string{"one": {"one"}, "two": {"one", "two"}, "three": {"one", "two", "three"}} - type args struct { - header map[string][]string - param pipelinev1.Param - } - tests := []struct { - name string - args args - want pipelinev1.Param - }{{ - name: "empty", - args: args{ - header: header, - param: pipelinev1.Param{}, - }, - want: pipelinev1.Param{}, - }, { - name: "no header vars", - args: args{ - header: header, - param: pipelinev1.Param{ - Name: "noHeaderVars", - Value: pipelinev1.ArrayOrString{StringVal: "foo"}, - }, - }, - want: pipelinev1.Param{ - Name: "noHeaderVars", - Value: pipelinev1.ArrayOrString{StringVal: "foo"}, - }, - }, { - name: "one header var", - args: args{ - header: header, - param: pipelinev1.Param{ - Name: "oneHeaderVar", - Value: pipelinev1.ArrayOrString{StringVal: "$(header.one)"}, - }, - }, - want: pipelinev1.Param{ - Name: "oneHeaderVar", - Value: pipelinev1.ArrayOrString{StringVal: "one"}, - }, - }, { - name: "multiple header vars", - args: args{ - header: header, - param: pipelinev1.Param{ - Name: "multipleHeaderVars", - Value: pipelinev1.ArrayOrString{StringVal: "$(header.one)-$(header.two)-$(header.three)"}, - }, - }, - want: pipelinev1.Param{ - Name: "multipleHeaderVars", - Value: pipelinev1.ArrayOrString{StringVal: `one-one two-one two three`}, - }, - }, { - name: "identical header vars", - args: args{ - header: header, - param: pipelinev1.Param{ - Name: "identicalHeaderVars", - Value: pipelinev1.ArrayOrString{StringVal: "$(header.one)-$(header.one)-$(header.one)"}, - }, - }, - want: pipelinev1.Param{ - Name: "identicalHeaderVars", - Value: pipelinev1.ArrayOrString{StringVal: `one-one-one`}, - }, - }, { - name: "entire header var", - args: args{ - header: header, - param: pipelinev1.Param{ - Name: "entireHeaderVar", - Value: pipelinev1.ArrayOrString{StringVal: "$(header)"}, - }, - }, - want: pipelinev1.Param{ - Name: "entireHeaderVar", - Value: pipelinev1.ArrayOrString{StringVal: `{\"one\":[\"one\"],\"three\":[\"one\",\"two\",\"three\"],\"two\":[\"one\",\"two\"]}`}, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := applyHeaderToParam(tt.args.header, tt.args.param) - if err != nil { - t.Errorf("applyHeaderToParam() error = %v", err) - return - } - if diff := cmp.Diff(tt.want, got); diff != "" { - t.Errorf("applyHeaderToParam(): -want +got: %s", diff) - } - }) - } -} - -func Test_applyHeaderToParams_error(t *testing.T) { - header := map[string][]string{"one": {"one"}} - type args struct { - header map[string][]string - param pipelinev1.Param - } - tests := []struct { - name string - args args - }{{ - name: "error header not found", - args: args{ - header: header, - param: pipelinev1.Param{ - Name: "oneBogusHeader", - Value: pipelinev1.ArrayOrString{StringVal: "$(header.bogus)"}, - }, - }, - }, { - name: "error multiple headers not found", - args: args{ - header: header, - param: pipelinev1.Param{ - Name: "multipleBogusHeaders", - Value: pipelinev1.ArrayOrString{StringVal: "$(header.one)-$(header.bogus1)-$(header.bogus2)-$(header.bogus3)"}, - }, - }, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := applyHeaderToParam(tt.args.header, tt.args.param) - if err == nil { - t.Errorf("applyHeaderToParam() did not return error when expected; got: %v", got) - } - }) - } -} - +// TODO(#252): Split NewResourcesTests into separate tests for ResolveParams and ResolveResources func Test_NewResources(t *testing.T) { type args struct { body []byte @@ -799,7 +177,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"}`), }, }, { @@ -832,8 +210,69 @@ func Test_NewResources(t *testing.T) { want: []json.RawMessage{ json.RawMessage(`{"rt1": "bar-1"}`), }, - }, - } + }, { + name: "bindings with static values", + args: args{ + body: json.RawMessage(`{"foo": "bar"}`), + binding: ResolvedTrigger{ + TriggerTemplate: bldr.TriggerTemplate("tt", "ns", bldr.TriggerTemplateSpec( + bldr.TriggerTemplateParam("p1", "", ""), + bldr.TriggerTemplateParam("p2", "", ""), + bldr.TriggerResourceTemplate(json.RawMessage(`{"p1": "$(params.p1)", "p2": "$(params.p2)"}`)), + ), + ), + TriggerBindings: []*triggersv1.TriggerBinding{ + bldr.TriggerBinding("tb", "ns", bldr.TriggerBindingSpec( + bldr.TriggerBindingParam("p1", "static_value"), + bldr.TriggerBindingParam("p2", "$(body.foo)"), + )), + }, + }, + }, + want: []json.RawMessage{ + json.RawMessage(`{"p1": "static_value", "p2": "bar"}`), + }, + }, { + name: "bindings with combination of static values ", + args: args{ + body: json.RawMessage(`{"foo": "fooValue", "bar": "barValue"}`), + binding: ResolvedTrigger{ + TriggerTemplate: bldr.TriggerTemplate("tt", "ns", bldr.TriggerTemplateSpec( + bldr.TriggerTemplateParam("p1", "", ""), + bldr.TriggerResourceTemplate(json.RawMessage(`{"p1": "$(params.p1)"`)), + ), + ), + TriggerBindings: []*triggersv1.TriggerBinding{ + bldr.TriggerBinding("tb", "ns", bldr.TriggerBindingSpec( + bldr.TriggerBindingParam("p1", "Event values are - foo: $(body.foo); bar: $(body.bar)"), + )), + }, + }, + }, + want: []json.RawMessage{ + json.RawMessage(`{"p1": "Event values are - foo: fooValue; bar: barValue"`), + }, + }, { + name: "event value is JSON string", + args: args{ + body: json.RawMessage(`{"a": "b"}`), + binding: ResolvedTrigger{ + TriggerTemplate: bldr.TriggerTemplate("tt", "ns", bldr.TriggerTemplateSpec( + bldr.TriggerTemplateParam("p1", "", ""), + bldr.TriggerResourceTemplate(json.RawMessage(`{"p1": "$(params.p1)"}`)), + ), + ), + TriggerBindings: []*triggersv1.TriggerBinding{ + bldr.TriggerBinding("tb", "ns", bldr.TriggerBindingSpec( + bldr.TriggerBindingParam("p1", "$(body)"), + )), + }, + }, + }, + want: []json.RawMessage{ + json.RawMessage(`{"p1": "{\"a\":\"b\"}"}`), + }, + }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // This seeds Uid() to return 'cbhtc' @@ -866,89 +305,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 index 4a503da4fc..6148872965 100644 --- a/pkg/template/resource.go +++ b/pkg/template/resource.go @@ -20,10 +20,10 @@ import ( "bytes" "encoding/json" "fmt" + "strings" pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" - "golang.org/x/xerrors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/rand" @@ -49,7 +49,7 @@ func ResolveTrigger(trigger triggersv1.EventListenerTrigger, getTB getTriggerBin for _, b := range trigger.Bindings { tb2, err := getTB(b.Name, metav1.GetOptions{}) if err != nil { - return ResolvedTrigger{}, xerrors.Errorf("error getting TriggerBinding %s: %s", b.Name, err) + return ResolvedTrigger{}, fmt.Errorf("error getting TriggerBinding %s: %w", b.Name, err) } tb = append(tb, tb2) } @@ -57,7 +57,7 @@ func ResolveTrigger(trigger triggersv1.EventListenerTrigger, getTB getTriggerBin ttName := trigger.Template.Name tt, err := getTT(ttName, metav1.GetOptions{}) if err != nil { - return ResolvedTrigger{}, xerrors.Errorf("Error getting TriggerTemplate %s: %s", ttName, err) + return ResolvedTrigger{}, fmt.Errorf("error getting TriggerTemplate %s: %w", ttName, err) } return ResolvedTrigger{TriggerBindings: tb, TriggerTemplate: tt}, nil } @@ -92,7 +92,10 @@ func ApplyParamsToResourceTemplate(params []pipelinev1.Param, rt json.RawMessage func applyParamToResourceTemplate(param pipelinev1.Param, rt json.RawMessage) json.RawMessage { // Assume the param is valid paramVariable := fmt.Sprintf("$(params.%s)", param.Name) - return bytes.Replace(rt, []byte(paramVariable), []byte(param.Value.StringVal), -1) + // Escape quotes so that that JSON strings can be appended to regular strings. + // See #257 for discussion on this behavior. + paramValue := strings.Replace(param.Value.StringVal, `"`, `\"`, -1) + return bytes.Replace(rt, []byte(paramVariable), []byte(paramValue), -1) } // UID generates a random string like the Kubernetes apiserver generateName metafield postfix. diff --git a/pkg/template/resource_test.go b/pkg/template/resource_test.go index 6dceff829c..9fb0f9fbc3 100644 --- a/pkg/template/resource_test.go +++ b/pkg/template/resource_test.go @@ -176,6 +176,18 @@ func Test_applyParamToResourceTemplate(t *testing.T) { rt: rtMultipleParamVars, }, want: wantRtMultipleParamVars, + }, { + name: "espcae quotes in param val", + args: args{ + param: pipelinev1.Param{ + Name: "p1", + Value: pipelinev1.ArrayOrString{ + StringVal: `{"a":"b"}`, + }, + }, + rt: json.RawMessage(`{"foo": "$(params.p1)"}`), + }, + want: json.RawMessage(`{"foo": "{\"a\":\"b\"}"}`), }, } for _, tt := range tests { diff --git a/test/eventlistener_test.go b/test/eventlistener_test.go index 300aa5fcb5..cf0c8e8485 100644 --- a/test/eventlistener_test.go +++ b/test/eventlistener_test.go @@ -24,6 +24,7 @@ import ( "fmt" "net/http" "os/exec" + "strings" "testing" "time" @@ -244,8 +245,8 @@ func TestEventListenerCreate(t *testing.T) { Spec: v1alpha1.PipelineResourceSpec{ Type: "git", Params: []v1alpha1.ResourceParam{ - {Name: "body", Value: `{"one": "zonevalue", "two": {"name": "zfoo", "value": "bar"}}`}, - {Name: "header", Value: `{"Accept-Encoding":["gzip"],"Content-Length":["61"],"Content-Type":["application/json"],"User-Agent":["Go-http-client/1.1"]}`}, + {Name: "body", Value: strings.Replace(`{"one": "zonevalue", "two": {"name": "zfoo", "value": "bar"}}`, " ", "", -1)}, + {Name: "header", Value: `{"Accept-Encoding":"gzip","Content-Length":"61","Content-Type":"application/json","User-Agent":"Go-http-client/1.1"}`}, }, }, }