Skip to content

Commit

Permalink
Adress some review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@dibyom
dibyom committed Dec 4, 2019
1 parent 077ea13 commit 58c20e5
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 72 deletions.
5 changes: 3 additions & 2 deletions pkg/template/event.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package template

import (
"encoding/json"
"net/http"
"regexp"
"strings"

Expand Down Expand Up @@ -166,7 +167,7 @@ func getHeaderValue(header map[string][]string, headerName string) (string, erro

// 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: %q", err)
Expand Down Expand Up @@ -199,7 +200,7 @@ type event struct {
}

// newEvent returns a new Event from HTTP headers and body
func newEvent(body []byte, headers map[string][]string) (*event, error) {
func newEvent(body []byte, headers http.Header) (*event, error) {
var data interface{}
if len(body) > 0 {
if err := json.Unmarshal(body, &data); err != nil {
Expand Down
130 changes: 67 additions & 63 deletions pkg/template/event_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ func Test_NewResources(t *testing.T) {
}{{
name: "empty",
args: args{
body: []byte{},
body: json.RawMessage{},
header: map[string][]string{},
binding: ResolvedTrigger{
TriggerTemplate: bldr.TriggerTemplate("tt", "namespace"),
Expand Down Expand Up @@ -799,7 +799,7 @@ func Test_NewResources(t *testing.T) {
},
want: []json.RawMessage{
json.RawMessage(`{"rt1": "bar-cbhtc", "cbhtc": "cbhtc"}`),
json.RawMessage(`{"rt2": "default2-bsvjp"}`),
json.RawMessage(`{"rt2": "default2-cbhtc"}`),
json.RawMessage(`{"rt3": "rt3"}`),
},
}, {
Expand Down Expand Up @@ -866,85 +866,89 @@ 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", ""),
),
),
TriggerBindings: []*triggersv1.TriggerBinding{
bldr.TriggerBinding("tb", "namespace",
bldr.TriggerBindingSpec(
bldr.TriggerBindingParam("param1", "$(body.bogusvalue)"),
}{
{
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)"),
),
),
},
},
},
}, {
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)"),
{
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)"),
),
),
},
},
},
}, {
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", ""),
),
),
TriggerBindings: []*triggersv1.TriggerBinding{
bldr.TriggerBinding("tb", "namespace",
bldr.TriggerBindingSpec(
bldr.TriggerBindingParam("param1", "$(body.bogusvalue)"),
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)"),
),
),
},
},
},
}, {
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"),
{
name: "conflicting bindings",
binding: ResolvedTrigger{
TriggerTemplate: bldr.TriggerTemplate("tt", "namespace",
bldr.TriggerTemplateSpec(
bldr.TriggerTemplateParam("param1", "description", ""),
),
),
bldr.TriggerBinding("tb2", "namespace",
bldr.TriggerBindingSpec(
bldr.TriggerBindingParam("param1", "bar"),
TriggerBindings: []*triggersv1.TriggerBinding{
bldr.TriggerBinding("tb", "namespace",
bldr.TriggerBindingSpec(
bldr.TriggerBindingParam("param1", "foo"),
),
),
),
bldr.TriggerBinding("tb2", "namespace",
bldr.TriggerBindingSpec(
bldr.TriggerBindingParam("param1", "bar"),
),
),
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
19 changes: 12 additions & 7 deletions pkg/template/jsonpath.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,17 @@ import (
)

var (
// tektonVar is a regular expression for strings surrounded by $()
tektonVar = regexp.MustCompile(`^\$\((.*)\)$`)

// jsonRegexp is a regular expression for JSONPath expressions
// with or without the enclosing {} and the leading . inside the curly
// braces e.g. 'a.b' or '.a.b' or '{a.b}' or '{.a.b}
jsonRegexp = regexp.MustCompile(`^\{\.?([^{}]+)\}$|^\.?([^{}]+)$`)
)

// ParseJSONPath extracts a subset of the given JSON input
// using the provided JSONPath expression.
func ParseJSONPath(input interface{}, expr string) (string, error) {
j := jsonpath.New("").AllowMissingKeys(false)
buf := new(bytes.Buffer)
Expand All @@ -37,8 +44,8 @@ func ParseJSONPath(input interface{}, expr string) (string, error) {
return "", err
}

for ix := range fullResults {
if err := printResults(buf, fullResults[ix]); err != nil {
for _, r := range fullResults {
if err := printResults(buf, r); err != nil {
return "", err
}
}
Expand Down Expand Up @@ -82,11 +89,7 @@ func textValue(v reflect.Value) ([]byte, error) {
switch t.Kind() {
// evalToText() returns <map> ....; return JSON string instead.
case reflect.Map, reflect.Slice:
text, err := json.Marshal(v.Interface())
if err != nil {
return nil, err
}
return text, nil
return json.Marshal(v.Interface())
default:
return evalToText(v)
}
Expand Down Expand Up @@ -122,6 +125,8 @@ func TektonJSONPathExpression(expr string) (string, error) {
// * {.metadata.name} (complete expression)
// And transforms them all into a valid jsonpath expression:
// {.metadata.name}
// This function has been copied as-is from
// https://github.com/kubernetes/kubectl/blob/c273777957bd657233cf867892fb061a6498dab8/pkg/cmd/get/customcolumn.go#L47
func relaxedJSONPathExpression(pathExpression string) (string, error) {
if len(pathExpression) == 0 {
return pathExpression, nil
Expand Down

0 comments on commit 58c20e5

Please sign in to comment.