Adds Basic Support for Star Array Notation #2085
Conversation
googlebot
added
the
cla: yes
tekton-robot
added
the
size/L
|
The following is the coverage report on pkg/.
|
tekton-robot
added
area/api
|
Whoops -- didn't see this til right now. Yep you bet. |
when referring to an array reference that we want to expand inside an existing array we should use star syntax. For example: ["a", "b", "$(params.myArray[*])", "d"] to avoid confusion when in the future we want to add the array itself as an array element. Basic support changes are in substitution.go All other changes are for unit tests
|
The following is the coverage report on pkg/.
|
Updating the docs however for now we are not breaking existing use of arrays without star-expansion, and are not mentioning the old approach.
|
The following is the coverage report on pkg/.
|
|
/retest |
| @@ -576,7 +576,7 @@ Param values from resources can also be accessed using [variable substitution](. | |||
|
|
|||
| ##### Variable Substitution with Parameters of Type `Array` | |||
|
|
|||
| Referenced parameters of type `array` will expand to insert the array elements in the reference string's spot. | |||
| Referenced parameters of type `array` can be expanded using 'star-expansion' by adding `[*]` to the named parameter to insert the array elements in the reference string's spot. | |||
There was a problem hiding this comment.
What happens if I don't use the * with an array param? Is that an error or will it simply not expand the values out?
Reading the issue, it seems like both the old style and the * expansion should work!
There was a problem hiding this comment.
Yes with caveats. Right now the behaviour is identical as the star expansion is effectively an alias, but longer term when we add proper type support if you just reference the array directly then the array itself will be added. Whether or not that is an error is going to depend on the type we unmarshal into (see #1530 for where we're heading).
There was a problem hiding this comment.
As it turn out right now you can't assign an array parameter to an actual array field, so afaik in all cases we currently have, we should just use star-expansion. Talking to @afrittoli earlier, I'll also update the catalog tasks, but that is not desperately urgent for now other than to illustrate good practices.
tekton-robot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dibyom, 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:
Approvers can indicate their approval by writing |
Changes
When referring to an array reference that we want to expand inside an existing array we should use star syntax.
For example: ["a", "b", "$(params.myArray[*])", "d"] to avoid confusion when in the future we want to add the array itself as an array element.
Basic support changes are in substitution.go
All other changes are for unit tests
Fixes #2041
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmddir, please updatethe release Task to build and release this image.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.