Resolving newline in TriggerBinding param

[jmcshane]

Closes #942

This PR resolves the bug by replacing all instances of the \n character
in triggerbinding params with \n, thus encoding into JSON as the bytes
92 110 rather than 10 or 13 10 depending on the system. This works in any
instance where the parameter is substituted as the JSON encoding will resolve
the parameter value after the byte array for the constructed rawMessage is
assembled. Therefore, anywhere that the newline character is in one of these
parameters, it will not encode appropriately in JSON.

This was determined based on the existing tests that use \n in the label fields.
Since the JSON encoder already operate successfully with those fields, we can look
at the resultant byte array when those tests are run and determine the appropriate
input that should be passed to the replace function in resource.go

Changes

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes tests (if functionality changed/added)
• Includes docs (if user facing)
• Commit messages follow commit message best practices
• Release notes block has been filled in or deleted (only if no user facing changes)

See the contribution guide for more details.

Release Notes

Resolves error that occurs when triggerbinding parameter has multiple lines

[tekton-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign khrm
You can assign the PR to them by writing /assign @khrm in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[tekton-robot]

Hi @jmcshane. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[jmcshane]

I couldn't find anywhere to document this as it appears that this is the desired behavior. One thing I did find is that if you are trying to do a multi-line JSON document, you do need to use the old-escape annotation. I put that in the new sink_test.go test to demonstrate the difference between a standard string with multiple lines and a JSON string

[jmcshane]

Closing in favor of a more general approach for encoding resources

[dibyom]

So, I was taking another look at this issue and I think we might want to merge this.

I tried out a few triggers with newline chars as well as some quotes. I found that #823 fixes the issues with escaping quotes -- I could successfully use a trigger with a param that contains \" . And with this PR, I could also successfully create params with newlines in them.

I tried a few triggers with other escaped chars ( e.g. \t ) and I did not run into the JSON encoding errors that we were running into before. So, while we can still do a general purpose escaping fix in tektoncd/community#321, I think just merging this PR should fix a bunch of use cases.