tektoncd
Propagate params in pipelines
Prior to this, we allowed parameter propagation in an inlined pipelinerun. However, within a pipeline, we requrie a verbose spec. This was an oversight as indicated in https://github.com/tektoncd/pipeline/issues/7901. This PR fixes that issue by updating the validation logic in the webhook.
Fixes https://github.com/tektoncd/pipeline/issues/7901.
Changes
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
- [ ] Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
- [x] Has Tests included if any functionality added or changed
- [x] pre-commit Passed
- [x] Follows the commit message standard
- [x] Meets the Tekton contributor standards (including functionality, content, code)
- [x] Has a kind label. You can add one by adding a comment on this PR that contains
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep - [x] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
- [ ] Release notes contains the string "action required" if the change requires additional action from users switching to the new release
Release Notes
Enable propagating params in Pipelines.
/kind bug
Thanks for the release-note! Shall we also add the changes to somewhere https://github.com/tektoncd/pipeline/blob/main/docs/pipelines.md#specifying-parameters?
Thanks for the release-note! Shall we also add the changes to somewhere https://github.com/tektoncd/pipeline/blob/main/docs/pipelines.md#specifying-parameters?
Done. PTAL again 🙏 Thanks!
@chitrangpatel: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:
/test pull-tekton-pipeline-alpha-integration-tests/test pull-tekton-pipeline-beta-integration-tests/test pull-tekton-pipeline-build-tests/test pull-tekton-pipeline-integration-tests/test pull-tekton-pipeline-unit-tests
The following commands are available to trigger optional jobs:
/test pull-tekton-pipeline-go-coverage
Use /test all to run all jobs.
In response to this:
/test pull-tekton-pipeline-go-coverage-df
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.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: JeromeJu
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [JeromeJu]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
On second thought - do we need an integration test for this?
I threw in an example test for this. If that's not ok, I can work on an integration test too. The trick is that it needs us to apply a pipeline separately on the cluster. The web hook was the one stopping its admission. I already added unit tests for that. The example test also allows us to test the web hook admission more easily. It also provides a nice example :)
The controller logic has not changed at all I think so the status and everything else remains the same. And that is already tested by the current e2e test for propagated params.
I tend to agree with the issue #7901 to be an extension of the capability of the existing API.
While this PR generally lgtm, shall we also update the TEP0107 to reflect so?
Happy to do that!
/hold
cc @afrittoli I put a hold so that we can all be on the same page here. Let's discuss it on the issue or in the API WG on Monday or offline.
/hold cancel Synced offline with @afrittoli! We are all on the same page now that this is an oversight and should be fixed.
Thanks, this looks good! Could you add a "negative" test to see that propagation on a referenced task is not allowed?
Done! PTAL!