Provide error for invalid bindings

[jmcshane]

Changes

TriggerBinding values do not yet have any validation on their values, but we have situations, as
discussed in #385, where the binding value is somewhat nonsensical. In templates, it is possible that
a nesting situation may take place within a TaskRun script, but for a binding parameter value it makes
sense to avoid these types of situations.

This PR currently only avoids the situations where expressions are directly nested, meaning
dollar-parens immediately followed by dollar-parens. This would not detect a situation where they are
separated by some characters

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

TriggerBinding error on invalid Tekton template values

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/template/event.go	100.0%	97.8%	-2.2
pkg/template/jsonpath.go	93.3%	93.8%	0.4

[dibyom]

Looks like build test is failing with:

pkg/template/jsonpath.go:187:11: `if` block ends with a `return` statement, so drop this `else` and outdent its block (golint)
			} else {
			       ^

[jmcshane]

So, this happens at processing time, but I wonder if we want to change this to a validation error? It might be an issue in existing installs, but an error at triggerbinding creation time might make more sense than during the HTTP request.

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/template/event.go	100.0%	97.8%	-2.2
pkg/template/jsonpath.go	93.3%	93.8%	0.4

[dibyom]

So, this happens at processing time, but I wonder if we want to change this to a validation error? It might be an issue in existing installs, but an error at triggerbinding creation time might make more sense than during the HTTP request.

Agreed. That makes more sense.

[jmcshane]

Updated tests and Validation function (as well as validation error messages). The HTTP request processing remains completely the same, we will just reject binding values that have nested tekton expressions within them.

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/trigger_binding_validation.go	100.0%	93.5%	-6.5

[dibyom]

question: is it possible to reuse this bit from the jsonpath.go? Maybe this bit could be extracted to its own function? If that's too much for this PR, I'm happy to merge this as is.

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dibyom

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [dibyom]

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

[dibyom]

/lgtm