resources: Fail on non-string template labels.

[wlynch]

Changes

Although ObjectMeta defines labels as a map[string]string, Unstructured
types allow for map[string]interface{}, which means that users can set
non-string values like ints in their configurations, and we will parse it
with no issues.

We discovered that the behavior of
unstructured.GetLabels
will silently ignore errors, returning an empty label set if a
non-string value is included. When used in triggers, this looks like we
are completely ignoring the user provided labels.

This change replaces GetLabels with it's underlying implementation, and bubbles
back the non-string error if it is encountered.

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)
• [n/a] 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 Note

NONE

[wlynch]

/kind bug

[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/resources/create.go	81.8%	81.2%	-0.6

[dibyom]

Nice -- one thing I was thinking was that we could do a similar check in the resourceTemplate validation as well...so that if a user creates a resource with the wrong type, we catch it at creation time vs run time. What do you think?

/approve

[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

[wlynch]

Nice -- one thing I was thinking was that we could do a similar check in the resourceTemplate validation as well...so that if a user creates a resource with the wrong type, we catch it at creation time vs run time. What do you think?

That's probably going to be more work than I want to commit to for this PR. 😅

TriggerResourceTemplates are currently stored as runtime.RawExtensions. We likely want to convert this to an unstructured.Unstructured, and I'm not sure what the scope of this change would entail (particularly in ensuring backwards compatibility). Alternatively, we'd could unmarshal the message in the webhook and in the controller, but that seems wasteful.

Worth noting - if we move forward with #697 this validation should happen automatically.

[dibyom]

Ahh, missed the runtime.RawExtension vs unstructured.Unstructured difference. I don't think it would be too bad converting them in the controller/webhook but I agree that is outside the scope of this PR.
/lgtm