Fix enum validation with multiple param references #7481
Conversation
Fixes [tektoncd#7476][7476]. TEP-0144 requires that the pipeline-level `enum` must be a subset of referenced task-level `enum`. Prior to this commit, the enum subset validation logic assumes that a task-level param only referenced only one pipeline-level `enum`, and does not support scenario where multiple pipeline-level `enums` are referenced (e.g., "$(params.p1) and $(params.p2)"). This commit adds the handling logic for such compound references, skipping the subset validation in this scenario as there is no directly associated params at pipeline level. /kind bug [7476]: tektoncd#7476
tekton-robot
added
release-note
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
/retest |
| params: | ||
| - name: message1 | ||
| enum: ["v1"] | ||
| - name: message2 | ||
| enum: ["v2"] | ||
| tasks: | ||
| - name: task1 | ||
| params: | ||
| - name: message | ||
| value: "$(params.message1) and $(params.message2)" | ||
| taskSpec: | ||
| params: message | ||
| enum: [...] # the message enum is not required to be a subset of message1 or message2 |
There was a problem hiding this comment.
I'm curious, if for the task, we have an enum that requires a url to beabc/foo, and the pipeline passes a $(params.url-part1)/$(params.url-part2) param to the task's param.
The PR will skip the validation right?
There was a problem hiding this comment.
Not really. There are 2 kinds of check we do
- Validate user input against enums. At pipeline level, we will validate
message1andmessage2individually; at task-level, we validate the value ofmessageis in it own enum (the compound value). This PR does not change this behavior. - Subset validation: prior to the change, we require that the pipeline-level param enum must be a subset of the task-level enum referencing it. If a task-level
paramreferenes a single pipeline-levelparam, it is easy and we still validate it. However, if it references multiple pipeline-level params, like this one, we will skip the subset validation.
There was a problem hiding this comment.
Oh this is the subset validaition, I thought it is the first one
tekton-robot
added
the
approved
|
Thanks for speedy solution, can we merge this fix @QuanZhang-William :) , as I needed to workaround (like using |
Hi @dohoangkhiem 👋 . I have pinged some reviewers to review and merge this PR. As a workaround, would disabling the param enum feature by setting |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JeromeJu, Yongxuanzhang 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
Fixes #7476. TEP-0144 requires that the pipeline-level
enummust be a subset of referenced task-levelenum.Prior to this commit, the enum subset validation logic assumes that a task-level param only referenced only one pipeline-level
enum, and does not support scenario where multiple pipeline-levelenumsare referenced (e.g., "$(params.p1) and $(params.p2)").This commit adds the handling logic for such compound references, skipping the subset validation in this scenario as there is no directly associated params at pipeline level.
/kind bug
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes