Fix Validation Error Merging StepTemplates with StepRef #7982
Conversation
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.
|
chitrangpatel
force-pushed
the
fix-validation-stepactions-steptemplates
branch
from
a30fabd
to
86fc5f7
Compare
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
This PR fixes the validation error when using StepTemplates with Steps referencing StepActions. Fixes: tektoncd#7981
chitrangpatel
force-pushed
the
fix-validation-stepactions-steptemplates
branch
from
86fc5f7
to
d6e2559
Compare
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
| @@ -46,6 +46,11 @@ func MergeStepsWithStepTemplate(template *StepTemplate, steps []Step) ([]Step, e | |||
| } | |||
|
|
|||
| for i, s := range steps { | |||
| // If the stepaction has not been fetched yet then do not merge. | |||
There was a problem hiding this comment.
I am not sure I'm following here - why does if s.Ref is empty determine if the stepaction has been fetched?
There was a problem hiding this comment.
Because users can either provide a ref to StepActions (in which case it is not nil) or inline the actionable fields e.g. image, command etc. (in which case ref is nil). There is already a validation check that users cannot provide both ref and image (for example).
There was a problem hiding this comment.
And at webhook validation time, we have not yet done the remote resolution.
There was a problem hiding this comment.
At runtime, after we fetch the StepActions, we inline the content and set ref to nil explicitly. So when we validate at runtime, we can actually do this after the StepAction has been fetched. https://github.com/tektoncd/pipeline/blob/main/pkg/reconciler/taskrun/resources/taskspec.go#L145
JeromeJu
changed the title
tekton-robot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JeromeJu, 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 |
Prior to this, when validating a Step, we were merging the StepTemplate fields into the Step. For inlined-Steps,
this is fine. However, when referencing a StepAction, we end up merging StepTemplate with a StepRef.
This leads to broken validation. In reality this should be skipped at web-hook validation since the
StepAction has not been fetched yet and so there is not enough information at that time to do this validation correctly.
This is what this PR fixes. See the issue below for more details.
Fixes #7981
Changes
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
/kind bug