Fix task parameter value substitution error due to propagateParams
#6004
Conversation
tekton-robot
added
release-note-none
|
Hi @chengjoey. Thanks for your PR. I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/kind bug |
|
/retest |
|
@chengjoey: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
The following is the coverage report on the affected files.
|
|
/ok-to-test |
tekton-robot
added
ok-to-test
|
The following is the coverage report on the affected files.
|
There was a problem hiding this comment.
Thanks @chengjoey for this PR!! See if you can reduce the complexity of the unit test. Left a suggestion.
|
/test check-pr-has-kind-label |
|
@chitrangpatel: The specified target(s) for
The following commands are available to trigger optional jobs:
Use In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
chengjoey
force-pushed
the
fix/pipelinerun-params
branch
from
f0db0e4
to
ca60566
Compare
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
/test pull-tekton-pipeline-go-coverage-df |
|
@afrittoli: The specified target(s) for
The following commands are available to trigger optional jobs:
Use In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
The following is the coverage report on the affected files.
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abayer 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 |
tekton-robot
added
the
approved
| Name: "start", | ||
| Value: v1beta1.ParamValue{ | ||
| Type: v1beta1.ParamTypeString, | ||
| StringVal: "$(tasks.start.results.Output)", |
There was a problem hiding this comment.
It might make sense to include a "start" task in the test case-- otherwise this param value is sort of confusing?
There was a problem hiding this comment.
thanks @lbernick , I added a previous-task-with-result task in the test case, and changed the param $(tasks.start.results.Output) to $(tasks.previous-task-with-result.results.Output), maybe it looks clearer
| @@ -251,38 +251,56 @@ func ApplyReplacements(p *v1beta1.PipelineSpec, replacements map[string]string, | |||
| if p.Finally[i].TaskRef != nil && p.Finally[i].TaskRef.Params != nil { | |||
| p.Finally[i].TaskRef.Params = replaceParamValues(p.Finally[i].TaskRef.Params, replacements, arrayReplacements, objectReplacements) | |||
| } | |||
| p.Finally[i], replacements, arrayReplacements, objectReplacements = propagateParams(p.Finally[i], replacements, arrayReplacements, objectReplacements) | |||
| p.Finally[i] = propagateParams(p.Finally[i], replacements, arrayReplacements, objectReplacements) | |||
There was a problem hiding this comment.
can you please also add a test case covering finally tasks?
There was a problem hiding this comment.
Ok, I added a test case to cover finally tasks
chengjoey
force-pushed
the
fix/pipelinerun-params
branch
from
ca60566
to
e87eda6
Compare
chengjoey
force-pushed
the
fix/pipelinerun-params
branch
from
e87eda6
to
f800d50
Compare
chengjoey
force-pushed
the
fix/pipelinerun-params
branch
from
f800d50
to
33d7eac
Compare
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
chengjoey
requested review from
lbernick
and removed request for
vdemeester and
abayer
|
The following is the coverage report on the affected files.
|
|
/test pull-tekton-pipeline-integration-tests |
|
/lgtm |
|
/cherry-pick release-v0.41.x |
|
@lbernick: new pull request created: #6032 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/cherry-pick release-v0.42.x |
|
/cherry-pick release-v0.43.x |
|
@lbernick: new pull request created: #6033 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@lbernick: new pull request created: #6034 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
afrittoli
added
the
needs-cherry-pick
|
We should backport this to the LTS release v0.41.x |
@afrittoli PTAL at #6032 I also opened cherry picks onto 0.42 and 0.43 since our docs state that we're still in the support period for those releases (I think we mistakenly specified 3 months of support for them) |
Changes
fix Tekton incorrectly maps pipeline parameters and task outputs with the same name
propagateParamsmodifications to replacements should be limited to this function so as not to interfere withthe parameters of subsequent tasks
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes