Preserve escaped characters when parsing json path 🦟

pkg/resources/create.go

@@ -63,7 +63,7 @@ func Create(logger *zap.SugaredLogger, rt json.RawMessage, triggerName, eventID,
 	// Assume the TriggerResourceTemplate is valid (it has an apiVersion and Kind)
 	data := new(unstructured.Unstructured)
 	if err := data.UnmarshalJSON(rt); err != nil {
-		return err
+		return fmt.Errorf("couldn't unmarshal json: %v", err)
 	}
 
 	data = AddLabels(data, map[string]string{
@@ -81,7 +81,7 @@ func Create(logger *zap.SugaredLogger, rt json.RawMessage, triggerName, eventID,
 	// Resolve resource kind to the underlying API Resource type.
 	apiResource, err := FindAPIResource(data.GetAPIVersion(), data.GetKind(), c)
 	if err != nil {
-		return err
+		return fmt.Errorf("couldn't find API resource for json: %v", err)
 	}
 
 	name := data.GetName()
@@ -96,8 +96,12 @@ func Create(logger *zap.SugaredLogger, rt json.RawMessage, triggerName, eventID,
 		Resource: apiResource.Name,
 	}
 
-	_, err = dc.Resource(gvr).Namespace(namespace).Create(data, metav1.CreateOptions{})
-	return err
+	logger.Infof("For event ID %q creating resource %v", eventID, gvr)
+
+	if _, err := dc.Resource(gvr).Namespace(namespace).Create(data, metav1.CreateOptions{}); err != nil {
+		return fmt.Errorf("couldn't create resource with group version kind %q: %v", gvr, err)
+	}
+	return nil
 }
 
 // AddLabels adds autogenerated Tekton labels to created resources.

pkg/resources/create_test.go

@@ -140,22 +140,12 @@ func TestCreateResource(t *testing.T) {
 	logger, _ := logging.NewLogger("", "")
 
 	tests := []struct {
-		name     string
-		resource pipelinev1.PipelineResource
-		want     pipelinev1.PipelineResource
+		name string
+		json []byte
+		want pipelinev1.PipelineResource
 	}{{
 		name: "PipelineResource without namespace",
-		resource: pipelinev1.PipelineResource{
-			TypeMeta: metav1.TypeMeta{
-				APIVersion: "tekton.dev/v1alpha1",
-				Kind:       "PipelineResource",
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name:   "my-pipelineresource",
-				Labels: map[string]string{"woriginal-label-1": "label-1"},
-			},
-			Spec: pipelinev1.PipelineResourceSpec{},
-		},
+		json: json.RawMessage(`{"kind":"PipelineResource","apiVersion":"tekton.dev/v1alpha1","metadata":{"name":"my-pipelineresource","creationTimestamp":null,"labels":{"woriginal-label-1":"label-1"}},"spec":{"type":"","params":[{"name":"foo","value":"bar\r\nbaz"}]},"status":{}}`),
 		want: pipelinev1.PipelineResource{
 			TypeMeta: metav1.TypeMeta{
 				APIVersion: "tekton.dev/v1alpha1",
@@ -170,22 +160,16 @@ func TestCreateResource(t *testing.T) {
 					eventIDLabel:        eventID,
 				},
 			},
-			Spec: pipelinev1.PipelineResourceSpec{},
+			Spec: pipelinev1.PipelineResourceSpec{
+				Params: []pipelinev1.ResourceParam{{
+					Name:  "foo",
+					Value: "bar\r\nbaz",
+				}},
+			},
 		},
 	}, {
 		name: "PipelineResource with namespace",
-		resource: pipelinev1.PipelineResource{
-			TypeMeta: metav1.TypeMeta{
-				APIVersion: "tekton.dev/v1alpha1",
-				Kind:       "PipelineResource",
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Namespace: "foo",
-				Name:      "my-pipelineresource",
-				Labels:    map[string]string{"woriginal-label-1": "label-1"},
-			},
-			Spec: pipelinev1.PipelineResourceSpec{},
-		},
+		json: json.RawMessage(`{"kind":"PipelineResource","apiVersion":"tekton.dev/v1alpha1","metadata":{"name":"my-pipelineresource","namespace":"foo","creationTimestamp":null,"labels":{"woriginal-label-1":"label-1"}},"spec":{"type":"","params":null},"status":{}}`),
 		want: pipelinev1.PipelineResource{
 			TypeMeta: metav1.TypeMeta{
 				APIVersion: "tekton.dev/v1alpha1",
@@ -207,12 +191,7 @@ func TestCreateResource(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			dynamicClient.ClearActions()
-
-			b, err := json.Marshal(tt.resource)
-			if err != nil {
-				t.Fatalf("error marshalling resource: %v", tt.resource)
-			}
-			if err := Create(logger, b, triggerName, eventID, elName, elNamespace, kubeClient.Discovery(), dynamicSet); err != nil {
+			if err := Create(logger, tt.json, triggerName, eventID, elName, elNamespace, kubeClient.Discovery(), dynamicSet); err != nil {
 				t.Errorf("createResource() returned error: %s", err)
 			}
 

pkg/sink/sink.go

@@ -128,10 +128,10 @@ func (r Sink) HandleEvent(response http.ResponseWriter, request *http.Request) {
 				result <- http.StatusAccepted
 				return
 			}
-			log.Info("params: %+v", params)
+			log.Infof("params: %+v", params)
 			res := template.ResolveResources(rt.TriggerTemplate, params)
 			if err := r.createResources(res, t.Name, eventID); err != nil {
-				log.Error(err)
+				log.Errorf("Could not create resources for %q: %v", t.Name, err)
 			}
 			result <- http.StatusCreated
 		}(t)

pkg/sink/sink_test.go

@@ -24,6 +24,7 @@ import (
 	"net/http/httptest"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/google/go-cmp/cmp"
 	pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
@@ -56,9 +57,24 @@ func init() {
 	template.UID = func() string { return eventID }
 }
 
+func waitTimeout(wg *sync.WaitGroup, timeout time.Duration) bool {
+	c := make(chan struct{})
+	go func() {
+		defer close(c)
+		wg.Wait()
+	}()
+	select {
+	case <-c:
+		return false
+	case <-time.After(timeout):
+		return true
+	}
+}
+
 func TestHandleEvent(t *testing.T) {
 	namespace := "foo"
-	eventBody := json.RawMessage(`{"head_commit": {"id": "testrevision"}, "repository": {"url": "testurl"}}`)
+	eventBody := json.RawMessage(`{"head_commit": {"id": "testrevision"}, "repository": {"url": "testurl"}, "foo": "bar\t\r\nbaz昨"}`)
+
 	pipelineResource := pipelinev1.PipelineResource{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "tekton.dev/v1alpha1",
@@ -75,6 +91,7 @@ func TestHandleEvent(t *testing.T) {
 				{Name: "url", Value: "$(params.url)"},
 				{Name: "revision", Value: "$(params.revision)"},
 				{Name: "contenttype", Value: "$(params.contenttype)"},
+				{Name: "foo", Value: "$(params.foo)"},
 			},
 		},
 	}
@@ -89,13 +106,15 @@ func TestHandleEvent(t *testing.T) {
 			bldr.TriggerTemplateParam("revision", "", ""),
 			bldr.TriggerTemplateParam("appLabel", "", "foo"),
 			bldr.TriggerTemplateParam("contenttype", "", ""),
+			bldr.TriggerTemplateParam("foo", "", ""),
 			bldr.TriggerResourceTemplate(json.RawMessage(pipelineResourceBytes)),
 		))
 	tb := bldr.TriggerBinding("my-triggerbinding", namespace,
 		bldr.TriggerBindingSpec(
 			bldr.TriggerBindingParam("url", "$(body.repository.url)"),
 			bldr.TriggerBindingParam("revision", "$(body.head_commit.id)"),
 			bldr.TriggerBindingParam("contenttype", "$(header.Content-Type)"),
+			bldr.TriggerBindingParam("foo", "$(body.foo)"),
 		))
 	el := bldr.EventListener("my-eventlistener", namespace,
 		bldr.EventListenerSpec(bldr.EventListenerTrigger("my-triggerbinding", "my-triggertemplate", "v1alpha1")))
@@ -166,7 +185,11 @@ func TestHandleEvent(t *testing.T) {
 		t.Errorf("did not get expected response back -want,+got: %s", diff)
 	}
 
-	wg.Wait()
+	// We expect that the EventListener will be able to immediately handle the event so we
+	// can use a very short timeout
+	if waitTimeout(&wg, time.Second) {
+		t.Fatalf("timed out waiting for reactor to fire")
+	}
 	wantResource := pipelinev1.PipelineResource{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "tekton.dev/v1alpha1",
@@ -188,6 +211,7 @@ func TestHandleEvent(t *testing.T) {
 				{Name: "url", Value: "testurl"},
 				{Name: "revision", Value: "testrevision"},
 				{Name: "contenttype", Value: "application/json"},
+				{Name: "foo", Value: "bar\t\r\nbaz昨"},
 			},
 		},
 	}

pkg/template/event_test.go

@@ -21,13 +21,15 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
+	"github.com/tektoncd/triggers/test"
 	bldr "github.com/tektoncd/triggers/test/builder"
 	"k8s.io/apimachinery/pkg/util/rand"
 )
 
-// TODO(#252): Split NewResourcesTests into separate tests for ResolveParams and ResolveResources
+// TODO(#252): Split testcases from NewResourcesTests into TestResolveParams and TestResolveResources
 func Test_NewResources(t *testing.T) {
 	type args struct {
 		body    []byte
@@ -315,6 +317,56 @@ func Test_NewResources(t *testing.T) {
 	}
 }
 
+func TestResolveParams(t *testing.T) {
+	tests := []struct {
+		name     string
+		body     []byte
+		params   []pipelinev1.ParamSpec
+		bindings []*triggersv1.TriggerBinding
+		want     []pipelinev1.Param
+	}{{
+		name: "values with newlines",
+		body: json.RawMessage(`{"foo": "bar\r\nbaz"}`),
+		params: []pipelinev1.ParamSpec{{
+			Name: "param1",
+		}, {
+			Name: "param2",
+		}},
+		bindings: []*triggersv1.TriggerBinding{
+			bldr.TriggerBinding("tb", "namespace",
+				bldr.TriggerBindingSpec(
+					bldr.TriggerBindingParam("param1", "qux"),
+					bldr.TriggerBindingParam("param2", "$(body.foo)"),
+				),
+			),
+		},
+		want: []pipelinev1.Param{{
+			Name: "param1",
+			Value: pipelinev1.ArrayOrString{
+				StringVal: "qux",
+				Type:      pipelinev1.ParamTypeString,
+			},
+		}, {
+			Name: "param2",
+			Value: pipelinev1.ArrayOrString{
+				StringVal: "bar\\r\\nbaz",
+				Type:      pipelinev1.ParamTypeString,
+			},
+		}},
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			params, err := ResolveParams(tt.bindings, tt.body, map[string][]string{}, tt.params)
+			if err != nil {
+				t.Fatalf("ResolveParams() returned unexpected error: %s", err)
+			}
+			if diff := cmp.Diff(tt.want, params, cmpopts.SortSlices(test.CompareParams)); diff != "" {
+				t.Errorf("didn't get expected params -want + got: %s", diff)
+			}
+		})
+	}
+}
+
 func convertJSONRawMessagesToString(rawMessages []json.RawMessage) []string {
 	stringMessages := make([]string, len(rawMessages))
 	for i := range rawMessages {

pkg/template/jsonpath.go

@@ -89,7 +89,16 @@ func textValue(v reflect.Value) ([]byte, error) {
 
 	switch t.Kind() {
 	// evalToText() returns <map> ....; return JSON string instead.
-	case reflect.Map, reflect.Slice:
+	case reflect.String:
+		b, err := json.Marshal(v.Interface())
+		if err != nil {
+			return nil, fmt.Errorf("unable to marshal string value %v: %v", v, err)
+		}
+		// A valid json string is surrounded by quotation marks; we are using this function to
+		// create a representation of the json value that can be embedded in a CRD definition and
+		// we want to leave it up to the user if they want the surrounding quotation marks or not.
+		return b[1 : len(b)-1], nil
+	case reflect.Map, reflect.Slice, reflect.Int:
 		return json.Marshal(v.Interface())
 	default:
 		return evalToText(v)

pkg/template/jsonpath_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 )
 
-var objects = `{"a":"v","c":{"d":"e"},"empty": "","null": null, "number": 42}`
+var objects = `{"a":"v\r\n烈","c":{"d":"e"},"empty": "","null": null, "number": 42}`
 var arrays = `[{"a": "b"}, {"c": "d"}, {"e": "f"}]`
 
 // Checks that we print JSON strings when the JSONPath selects
@@ -41,12 +41,7 @@ func TestParseJSONPath(t *testing.T) {
 		name: "string values",
 		in:   objectBody,
 		expr: "$(body.a)",
-		want: "v",
-	}, {
-		name: "string values",
-		in:   objectBody,
-		expr: "$(body.a)",
-		want: "v",
+		want: "v\\r\\n烈",
 	}, {
 		name: "empty string",
 		in:   objectBody,

pkg/template/resource_test.go

@@ -25,16 +25,12 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 	pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
+	"github.com/tektoncd/triggers/test"
 	bldr "github.com/tektoncd/triggers/test/builder"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/rand"
 )
 
-// Allows us to sort Params by Name
-func compareParam(x, y pipelinev1.Param) bool {
-	return x.Name < y.Name
-}
-
 func Test_MergeInDefaultParams(t *testing.T) {
 	var (
 		oneParam = pipelinev1.Param{
@@ -114,7 +110,7 @@ func Test_MergeInDefaultParams(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := MergeInDefaultParams(tt.args.params, tt.args.paramSpecs)
-			if diff := cmp.Diff(tt.want, got, cmpopts.SortSlices(compareParam)); diff != "" {
+			if diff := cmp.Diff(tt.want, got, cmpopts.SortSlices(test.CompareParams)); diff != "" {
 				t.Errorf("MergeInDefaultParams(): -want +got: %s", diff)
 			}
 		})