Extract full parameter name when using dots in bracket notation.

[chitrangpatel]

Prior to this, when using $params["<NAME>"] or $params['<NAME>'],
if NAME contained dots(.) (eg. org.foo.blah), only the first part (ie. org) would be
extracted, instead of the entire nameorg.foo.blah. This PR now properly fetches the
entire name and throws an error if using dots (.) without single
or double quotes.

The following references show whats valid and whats not:

$params.org.foo.blah : Not Valid: the webhook will throw a validation error.
$params["org.foo.blah"]: Valid: Properly extracts the entire NAME (org.foo.blah).
$params['org.foo.blah']: Valid: Properly extracts the entire NAME (org.foo.blah).

Changes

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

• Docs included if any changes are user facing
• Tests included if any functionality added or changed
• Follows the commit message standard
• Meets the Tekton contributor standards (including
  functionality, content, code)
• Release notes block below has been filled in
  (if there are no user facing changes, use release note "NONE")

Release Notes

Extract full parameter name when using dots inside single/double quotes.

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/substitution/substitution.go	54.5%	55.7%	1.1

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/substitution/substitution.go	54.5%	55.7%	1.1

[chitrangpatel]

/assign @imjasonh

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/substitution/substitution.go	54.5%	55.7%	1.1

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

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 [vdemeester]

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

[chitrangpatel]

/retest

[imjasonh]

/test pull-tekton-pipeline-integration-tests
/test pull-tekton-pipeline-alpha-integration-tests

[imjasonh]

There's now some overly aggressive validation of previously valid params, that rejects the otherwise valid resources.input.foo.path:

    cluster_resource_test.go:72: Failed to create Task `cluster-resource-kvkmoqgu`: admission webhook "validation.webhook.pipeline.tekton.dev" denied the request: validation failed: Invalid referencing of parameters in $(resources.inputs.target-cluster.path)/kubeconfig !!! You can only use the dots inside single or double quotes. eg. $(params["org.foo.blah"]) or $(params['org.foo.blah']) are valid references but NOT $params.org.foo.blah.: spec.steps[0].args[0]

[chitrangpatel]

There's now some overly aggressive validation of previously valid params, that rejects the otherwise valid resources.input.foo.path:

    cluster_resource_test.go:72: Failed to create Task `cluster-resource-kvkmoqgu`: admission webhook "validation.webhook.pipeline.tekton.dev" denied the request: validation failed: Invalid referencing of parameters in $(resources.inputs.target-cluster.path)/kubeconfig !!! You can only use the dots inside single or double quotes. eg. $(params["org.foo.blah"]) or $(params['org.foo.blah']) are valid references but NOT $params.org.foo.blah.: spec.steps[0].args[0]

Hmm. Is the rule of using dots with the dot notation (eg. $params.org.foo.blah) invalid only when accessing parameters (ie. when the prefix is params)? Does it not apply to any other case (like resources.input.target-cluster.path or resources.output.target-cluster.path or anything else that we might use)? If so, I can throw an error only in case of params... and not any other type.

Alternatively, we can choose to not throw an error at all. But in that case, $params.org.foo.blah will become completely valid.

[imjasonh]

I'd probably say only resources.* is allowed to have 4 components, params.* must have two components or use bracket notation. Any other prefix might be invalid, unless I'm missing some cases (entirely possible).

[chitrangpatel]

I'd probably say only resources.* is allowed to have 4 components, params.* must have two components or use bracket notation. Any other prefix might be invalid, unless I'm missing some cases (entirely possible).

Thanks @imjasonh , I updated the code to reflect this. Since we are unsure of other prefixes, I am not throwing an error so that I don't break existing code.

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/substitution/substitution.go	54.5%	57.3%	2.7

[chitrangpatel]

/retest

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/substitution/substitution.go	54.5%	57.3%	2.7

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/substitution/substitution.go	54.5%	58.3%	3.8

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/substitution/substitution.go	54.5%	57.9%	3.3

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/substitution/substitution.go	54.5%	57.9%	3.3

[chitrangpatel]

@imjasonh All the integration tests now pass. I think it should be ok now 🤞.

[imjasonh]

/lgtm

This seems to work in my downstream tests. Thanks for fixing this! 👍