tektoncd / triggers

Event triggering with Tekton!
Apache License 2.0
558 stars 420 forks source link

TriggerTemplate errors with \n character #942

Open dibyom opened 3 years ago

dibyom commented 3 years ago

Expected Behavior

Users should be able to use a Trigger with binding or template param values that contain the newline \n character.

Actual Behavior

TriggerTemplate errors out. I tried a few cases cases -- 1) TriggerTemplate with a default param that contains a newline, 2) with the annotation triggers.tekton.dev/old-escape-quotes: "true" added, and 3) with a binding value that contains the new line param.

Steps to Reproduce the Problem

apiVersion: triggers.tekton.dev/v1alpha1
kind: Trigger
metadata:
  name: newline-params
spec:
  bindings:
    - name: extra
      value: |
        {
          "type": "section",
          "foo": "bar"
        }
  template:
    ref: "newline-params"
---
apiVersion: triggers.tekton.dev/v1alpha1
kind: TriggerTemplate
metadata:
  name: newline-params
  annotations:
spec:
  params:
  - name: "extra"
    default: |
        {
            "type": "section"
        }
  resourcetemplates:
    - apiVersion: tekton.dev/v1alpha1
      kind: TaskRun
      metadata:
        generateName: newline-params-
      spec:
        taskSpec:
          steps:
          - name: "hellothere"
            image: ubuntu
            script: echo "$(tt.params.extra)"
---
apiVersion: triggers.tekton.dev/v1alpha1
kind: EventListener
metadata:
  name: listener-triggerref
spec:
  serviceAccountName: tekton-triggers-example-sa
  triggers:
    - triggerRef: "newline-params"

Additional Info

Related:

Error logs

{"level":"info","ts":"2021-02-04T17:39:02.730Z","logger":"eventlistener","caller":"sink/sink.go:234","msg":"ResolvedParams : [{Name:extra Value:{\n  \"type\": \"section\"\n}\n}]","knative.dev/controller":"eventlistener","/triggers-eventid":"khdmd","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"234","function":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger"}}
{"level":"error","ts":"2021-02-04T17:39:02.730Z","logger":"eventlistener","caller":"sink/sink.go:337","msg":"problem creating obj: &errors.errorString{s:\"couldn't unmarshal json from the TriggerTemplate: invalid character '\\\\n' in string literal\"}","knative.dev/controller":"eventlistener","/triggers-eventid":"khdmd","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"337","function":"github.com/tektoncd/triggers/pkg/sink.Sink.CreateResources"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.CreateResources\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:337\ngithub.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:236\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent.func1\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:125"}
{"level":"error","ts":"2021-02-04T17:39:02.730Z","logger":"eventlistener","caller":"sink/sink.go:237","msg":"couldn't unmarshal json from the TriggerTemplate: invalid character '\\n' in string literal","knative.dev/controller":"eventlistener","/triggers-eventid":"khdmd","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"237","function":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:237\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent.func1\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:125"}
{"level":"error","ts":"2021-02-04T17:42:14.604Z","logger":"eventlistener","caller":"sink/sink.go:177","msg":"Error getting Trigger trigger in Namespace default: trigger.triggers.tekton.dev \"trigger\" not found","knative.dev/controller":"eventlistener","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"177","function":"github.com/tektoncd/triggers/pkg/sink.Sink.merge"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.merge\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:177\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:116\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2042\nnet/http.(*ServeMux).ServeHTTP\n\tnet/http/server.go:2417\nnet/http.(*timeoutHandler).ServeHTTP.func1\n\tnet/http/server.go:3272"}
{"level":"info","ts":"2021-02-04T17:42:14.604Z","logger":"eventlistener","caller":"sink/sink.go:234","msg":"ResolvedParams : [{Name:extra Value:{\n    \"type\": \"section\"\n}\n}]","knative.dev/controller":"eventlistener","/triggers-eventid":"85dd2","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"234","function":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger"}}
{"level":"error","ts":"2021-02-04T17:42:14.605Z","logger":"eventlistener","caller":"sink/sink.go:337","msg":"problem creating obj: &errors.errorString{s:\"couldn't unmarshal json from the TriggerTemplate: invalid character '\\\\n' in string literal\"}","knative.dev/controller":"eventlistener","/triggers-eventid":"85dd2","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"337","function":"github.com/tektoncd/triggers/pkg/sink.Sink.CreateResources"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.CreateResources\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:337\ngithub.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:236\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent.func1\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:125"}
{"level":"error","ts":"2021-02-04T17:42:14.605Z","logger":"eventlistener","caller":"sink/sink.go:237","msg":"couldn't unmarshal json from the TriggerTemplate: invalid character '\\n' in string literal","knative.dev/controller":"eventlistener","/triggers-eventid":"85dd2","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"237","function":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:237\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent.func1\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:125"}
{"level":"error","ts":"2021-02-04T17:43:43.815Z","logger":"eventlistener","caller":"sink/sink.go:177","msg":"Error getting Trigger trigger in Namespace default: trigger.triggers.tekton.dev \"trigger\" not found","knative.dev/controller":"eventlistener","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"177","function":"github.com/tektoncd/triggers/pkg/sink.Sink.merge"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.merge\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:177\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:116\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2042\nnet/http.(*ServeMux).ServeHTTP\n\tnet/http/server.go:2417\nnet/http.(*timeoutHandler).ServeHTTP.func1\n\tnet/http/server.go:3272"}
{"level":"info","ts":"2021-02-04T17:43:43.815Z","logger":"eventlistener","caller":"sink/sink.go:234","msg":"ResolvedParams : [{Name:extra Value:{\n    \"type\": \"section\"\n}\n}]","knative.dev/controller":"eventlistener","/triggers-eventid":"zdb5r","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"234","function":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger"}}
{"level":"error","ts":"2021-02-04T17:43:43.815Z","logger":"eventlistener","caller":"sink/sink.go:337","msg":"problem creating obj: &errors.errorString{s:\"couldn't unmarshal json from the TriggerTemplate: invalid character '\\\\n' in string literal\"}","knative.dev/controller":"eventlistener","/triggers-eventid":"zdb5r","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"337","function":"github.com/tektoncd/triggers/pkg/sink.Sink.CreateResources"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.CreateResources\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:337\ngithub.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:236\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent.func1\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:125"}
{"level":"error","ts":"2021-02-04T17:43:43.815Z","logger":"eventlistener","caller":"sink/sink.go:237","msg":"couldn't unmarshal json from the TriggerTemplate: invalid character '\\n' in string literal","knative.dev/controller":"eventlistener","/triggers-eventid":"zdb5r","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"237","function":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:237\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent.func1\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:125"}
{"level":"error","ts":"2021-02-04T17:45:25.452Z","logger":"eventlistener","caller":"sink/sink.go:177","msg":"Error getting Trigger trigger in Namespace default: trigger.triggers.tekton.dev \"trigger\" not found","knative.dev/controller":"eventlistener","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"177","function":"github.com/tektoncd/triggers/pkg/sink.Sink.merge"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.merge\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:177\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:116\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2042\nnet/http.(*ServeMux).ServeHTTP\n\tnet/http/server.go:2417\nnet/http.(*timeoutHandler).ServeHTTP.func1\n\tnet/http/server.go:3272"}
{"level":"info","ts":"2021-02-04T17:45:25.452Z","logger":"eventlistener","caller":"sink/sink.go:234","msg":"ResolvedParams : [{Name:extra Value:{\n  \"type\": \"section\",\n  \"foo\": \"bar\"\n}\n}]","knative.dev/controller":"eventlistener","/triggers-eventid":"42fnn","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"234","function":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger"}}
{"level":"error","ts":"2021-02-04T17:45:25.452Z","logger":"eventlistener","caller":"sink/sink.go:337","msg":"problem creating obj: &errors.errorString{s:\"couldn't unmarshal json from the TriggerTemplate: invalid character '\\\\n' in string literal\"}","knative.dev/controller":"eventlistener","/triggers-eventid":"42fnn","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"337","function":"github.com/tektoncd/triggers/pkg/sink.Sink.CreateResources"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.CreateResources\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:337\ngithub.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:236\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent.func1\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:125"}
{"level":"error","ts":"2021-02-04T17:45:25.452Z","logger":"eventlistener","caller":"sink/sink.go:237","msg":"couldn't unmarshal json from the TriggerTemplate: invalid character '\\n' in string literal","knative.dev/controller":"eventlistener","/triggers-eventid":"42fnn","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"237","function":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:237\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent.func1\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:125"}
{"level":"error","ts":"2021-02-04T17:51:55.054Z","logger":"eventlistener","caller":"sink/sink.go:177","msg":"Error getting Trigger trigger in Namespace default: trigger.triggers.tekton.dev \"trigger\" not found","knative.dev/controller":"eventlistener","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"177","function":"github.com/tektoncd/triggers/pkg/sink.Sink.merge"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.merge\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:177\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:116\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2042\nnet/http.(*ServeMux).ServeHTTP\n\tnet/http/server.go:2417\nnet/http.(*timeoutHandler).ServeHTTP.func1\n\tnet/http/server.go:3272"}
{"level":"info","ts":"2021-02-04T17:51:55.054Z","logger":"eventlistener","caller":"sink/sink.go:234","msg":"ResolvedParams : [{Name:extra Value:{\n  \"type\": \"section\",\n  \"foo\": \"bar\"\n}\n}]","knative.dev/controller":"eventlistener","/triggers-eventid":"bxfn5","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"234","function":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger"}}
{"level":"error","ts":"2021-02-04T17:51:55.054Z","logger":"eventlistener","caller":"sink/sink.go:337","msg":"problem creating obj: &errors.errorString{s:\"couldn't unmarshal json from the TriggerTemplate: invalid character '\\\\n' in string literal\"}","knative.dev/controller":"eventlistener","/triggers-eventid":"bxfn5","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"337","function":"github.com/tektoncd/triggers/pkg/sink.Sink.CreateResources"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.CreateResources\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:337\ngithub.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:236\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent.func1\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:125"}
{"level":"error","ts":"2021-02-04T17:51:55.054Z","logger":"eventlistener","caller":"sink/sink.go:237","msg":"couldn't unmarshal json from the TriggerTemplate: invalid character '\\n' in string literal","knative.dev/controller":"eventlistener","/triggers-eventid":"bxfn5","/trigger":"newline-params","logging.googleapis.com/labels":{},"logging.googleapis.com/sourceLocation":{"file":"github.com/tektoncd/triggers/pkg/sink/sink.go","line":"237","function":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger"},"stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:237\ngithub.com/tektoncd/triggers/pkg/sink.Sink.HandleEvent.func1\n\tgithub.com/tektoncd/triggers/pkg/sink/sink.go:125"}
jmcshane commented 3 years ago

So @dibyom it looks like newlines that are set as parameters passed in at https://github.com/tektoncd/triggers/blob/master/pkg/resources/create.go#L69 need to be escaped in order to pass as newlines. Unfortunately, it looks like you have to know where that newline string goes in the resolved template. If it shows up in a pipelinev1.ArrayOrString parameter, it needs to be escaped there before it is populated. As you can see in sink_test.go, when it is passed into a label this issue isn't present.

Basically, this is about how the triggertemplate is populated with values. You need to know the context of the type that the resolved string is being placed into.

jmcshane commented 3 years ago

The problem with finding this context is that triggers replaces the byte arrays in a json.RawMessage, so that context isn't available: https://github.com/tektoncd/triggers/blob/master/pkg/template/resource.go#L135

jmcshane commented 3 years ago

Scratch that, we can just encode this by escaping the newline anywhere the param is replaced. Submitted PR #953

tekton-robot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale with a justification. Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with /close with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

dibyom commented 3 years ago

So, in https://github.com/tektoncd/triggers/pull/953#issuecomment-861729659 I had suggested that we revive and merge #953. I did a bit more digging and it looks like the problem with newline characters is restricted mainly to YAML multilines i.e. either trigger binding values or trigger template param defaults containing YAML multiline strings. If the incoming event body contains a \n, Trigger processing does not fail

tekton-robot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale with a justification. Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with /close with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

tekton-robot commented 3 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten with a justification. Rotten issues close after an additional 30d of inactivity. If this issue is safe to close now please do so with /close with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

dibyom commented 3 years ago

(This is still a issue we need to fix)

lbernick commented 2 years ago

/priority important-longterm

OSobky commented 2 years ago

Now I am facing a similar problem when I have string array in the JSON body sent to Event Listener

dibyom commented 2 years ago

hi @OSobky do you have a quick example of your use case that is failing?

OSobky commented 2 years ago

Hey @dibyom, Sorry for the late reply. Sure!

Basically, I was trying to send a request with an array of strings in the JSON body like the following: { "PARAMETERS" : ["-P", "alpha=0.555" , "-P" ,"l1_ratio=0.222"] }

However, the event listener generates the following error: {"level":"error","logger":"eventlistener","caller":"sink/sink.go:339","msg":"problem creating obj: &errors.errorString{s:\"couldn't unmarshal json from the TriggerTemplate: invalid character '-' after object key:value pair\"}","knative.dev/controller":"eventlistener","/triggers-eventid":"7c6eac30-50a8-4ef9-8db6-877d6734d170","/trigger":"triggers","stacktrace":"github.com/tektoncd/triggers/pkg/sink.Sink.CreateResources\n\t/opt/app-root/src/go/src/github.com/tektoncd/triggers/pkg/sink/sink.go:339\ngithub.com/tektoncd/triggers/pkg/sink.Sink.processTrigger\n\t/opt/approot/src/go/src/github.com/tektoncd/triggers/pkg/sink/sink.go:238\ngithub.com/tektoncd/triggers/pkg/sink.Sik.HandleEvent.func1\n\t/opt/app-root/src/go/src/github.com/tektoncd/triggers/pkg/sink/sink.go:127"}

Let me know if you need more detailed example

tekton-robot commented 2 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale with a justification. Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with /close with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.