Skip to content

Commit

Permalink
Allow brackets inside Tekton expressions
Browse files Browse the repository at this point in the history
We were previously using a regex to extract expressions within
TriggerBindings. The regex only extracted between the initial `$(`
and the first occurrence of the `)`. This meant that some valid
JSONPath expressions that contained brackets (e.g. filters) were
incorrectly extracted.

This commit fixes the issue by splitting the string on `$(` and then
extracting until the first "unbalanced" occurrence of `)`.

Fixes #365

Co-Authored-By: Andrea Frittoli <[email protected]>
Signed-off-by: Dibyo Mukherjee <[email protected]>
  • Loading branch information
dibyom and afrittoli committed Jan 23, 2020
1 parent 4ca05e0 commit 67955d5
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/template/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func applyEventValuesToParams(params []pipelinev1.Param, body []byte, header htt
for idx, p := range params {
pValue := p.Value.StringVal
// Find all expressions wrapped in $() from the value
expressions := tektonVar.FindAllString(pValue, -1)
expressions := findTektonExpressions(pValue)
for _, expr := range expressions {
val, err := ParseJSONPath(event, expr)
if err != nil {
Expand Down
10 changes: 10 additions & 0 deletions pkg/template/event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,16 @@ func TestApplyEventValuesToParams(t *testing.T) {
bldr.Param("foo", `val1`),
bldr.Param("bar", `val2`),
},
}, {
name: "Array filters",
body: json.RawMessage(`{"child":[{"a": "b", "w": "1"}, {"a": "c", "w": "2"}, {"a": "d", "w": "3"}]}`),
params: []pipelinev1.Param{bldr.Param("a", "$(body.child[?(@.a == 'd')].w)")},
want: []pipelinev1.Param{bldr.Param("a", "3")},
}, {
name: "filters + multiple JSONPath expressions",
body: json.RawMessage(`{"child":[{"a": "b", "w": "1"}, {"a": "c", "w": "2"}, {"a": "d", "w": "3"}]}`),
params: []pipelinev1.Param{bldr.Param("a", "$(body.child[?(@.a == 'd')].w) : $(body.child[0].a)")},
want: []pipelinev1.Param{bldr.Param("a", "3 : b")},
}}

for _, tt := range tests {
Expand Down
34 changes: 34 additions & 0 deletions pkg/template/jsonpath.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,37 @@ func relaxedJSONPathExpression(pathExpression string) (string, error) {
func isTektonExpr(expr string) bool {
return tektonVar.MatchString(expr)
}

// findTektonExpressions searches for and returns a slice of
// all substrings that are wrapped in $()
func findTektonExpressions(in string) []string {
results := []string{}

// No expressions to return
if !strings.Contains(in, "$(") {
return results
}
// Splits string on $( to find potential Tekton expressions
maybeExpressions := strings.Split(in, "$(")
for _, e := range maybeExpressions[1:] { // Split always returns at least one element
// Iterate until we find the first unbalanced )
numOpenBrackets := 0
for i, ch := range e {
switch ch {
case '(':
numOpenBrackets++
case ')':
numOpenBrackets--
if numOpenBrackets < 0 {
results = append(results, fmt.Sprintf("$(%s)", e[:i]))
}
default:
continue
}
if numOpenBrackets < 0 {
break
}
}
}
return results
}
59 changes: 59 additions & 0 deletions pkg/template/jsonpath_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ func TestParseJSONPath(t *testing.T) {
in: `{"body":["", null, "thing"]}`,
expr: "$(body[:2])",
want: `["", null]`,
}, {
name: "Array filters",
in: `{"body":{"child":[{"a": "b", "w": "1"}, {"a": "c", "w": "2"}, {"a": "d", "w": "3"}]}}`,
expr: "$(body.child[?(@.a == 'd')].w)",
want: "3",
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -213,3 +218,57 @@ func TestRelaxedJSONPathExpression_Error(t *testing.T) {
})
}
}

func TestFindTektonExpressions(t *testing.T) {
tcs := []struct {
in string
want []string
}{{
in: "$(body.blah)",
want: []string{"$(body.blah)"},
}, {
in: "$(body.blah)-$(header.*)",
want: []string{"$(body.blah)", "$(header.*)"},
}, {
in: "start:$(body.blah)//middle//$(header.one)-end",
want: []string{"$(body.blah)", "$(header.one)"},
}, {
in: "start:$(body.[?(@.a == 'd')])-$(body.another-one)",
want: []string{"$(body.[?(@.a == 'd')])", "$(body.another-one)"},
}, {
in: "$(this)-$(not-this",
want: []string{"$(this)"},
}, {
in: "$body.)",
want: []string{},
}, {
in: "($(body.blah))-and-$(body.foo)",
want: []string{"$(body.blah)", "$(body.foo)"},
}, {
in: "(staticvalue)$(body.blah)",
want: []string{"$(body.blah)"},
}, {
in: "asd)$(asd",
want: []string{},
}, {
in: "onlystatic",
want: []string{},
}, {
in: "",
want: []string{},
}, {
in: "$())))",
want: []string{"$()"},
}, {
in: "$($())",
want: []string{"$()"},
}}

for _, tc := range tcs {
t.Run(tc.in, func(t *testing.T) {
if diff := cmp.Diff(tc.want, findTektonExpressions(tc.in)); diff != "" {
t.Fatalf("error -want/+got: %s", diff)
}
})
}
}

0 comments on commit 67955d5

Please sign in to comment.