TEP-0107: Implement Parameter Propagation #4845

chitrangpatel · 2022-05-06T20:59:31Z

Tekton Pipelines resources are verbose mostly because of explicitly propagating Parameters. Implicit Parameters feature was added to reduce the verbosity. However, there are challenges caused by mutating specifications to support Implicit Parameters. This PR builds on this prior work by propagating Parameters without mutating specifications to improve usability of Tekton Pipelines. This addresses features proposed in TEP-0107.

Changes

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Docs included if any changes are user facing
Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Release notes block below has been filled in
(if there are no user facing changes, use release note "NONE")

Release Notes

Parameters are propagated in embedded specifications without mutations.

tekton-robot · 2022-05-06T20:59:46Z

Hi @chitrangpatel. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

dibyom · 2022-05-09T15:32:04Z

Hmmm...weird that we need an ok-to-test here given @chitrangpatel was added to the org in tektoncd/community#692

@afrittoli do you think our peribolos sync is broken?

dibyom · 2022-05-09T19:15:10Z

/ok-to-test

tekton-robot · 2022-05-09T19:20:53Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1beta1/task_validation.go	97.0%	97.1%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	100.0%	91.0%	-9.0
pkg/reconciler/taskrun/validate_resources.go	96.0%	96.9%	0.8

dibyom · 2022-05-10T14:19:41Z

/assign @jerop

jerop

thank you @chitrangpatel!

please add tests for the changes in this PR - here are the guidelines for tests in tekton pipelines: https://github.com/tektoncd/community/blob/main/standards.md#tests

pkg/reconciler/pipelinerun/resources/apply.go

chitrangpatel · 2022-05-10T18:42:30Z

thank you @chitrangpatel!

please add tests for the changes in this PR - here are the guidelines for tests in tekton pipelines: https://github.com/tektoncd/community/blob/main/standards.md#tests

Thanks @jerop! I added examples and unit tests.

afrittoli · 2022-05-10T20:42:29Z

Hmmm...weird that we need an ok-to-test here given @chitrangpatel was added to the org in tektoncd/community#692

@afrittoli do you think our peribolos sync is broken?

I don't see @chitrangpatel in the org - something went wrong with the CI job probably.
I sent an invite manually now.

We should add a nightly job to peribolos to make sure things stay in sync.

jerop · 2022-05-10T20:44:52Z

/retest

jerop

please add documentation as well

just to make sure this is not missed, are you planning to remove the existing implicit parameters in a follow up pr?

tekton-robot · 2022-05-10T20:47:18Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1beta1/task_validation.go	97.0%	97.1%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	100.0%	99.1%	-0.9
pkg/reconciler/taskrun/validate_resources.go	96.0%	96.9%	0.8

chitrangpatel · 2022-05-10T20:55:28Z

please add documentation as well

Yes, will do.

just to make sure this is not missed, are you planning to remove the existing implicit parameters in a follow up pr?

Are you referring to the alpha feature? Changes here? I'm a bit confused. I think I need to know what needs to be removed regarding existing implicit parameters. I guess, I can enable the alpha feature and see it for myself.

chitrangpatel · 2022-05-10T21:16:04Z

/retest

dibyom · 2022-05-18T14:56:36Z

/retest

Tekton Pipelines resources are verbose mostly because of explicitly propagating Parameters. Implicit Parameters feature was added to reduce the verbosity. However, there are challenges caused by mutating specifications to support Implicit Parameters. This PR builds on this prior work by propagating Parameters without mutating specifications to improve usability of Tekton Pipelines. Removed existing implicit params behind alpha features. Parameter propagation is behind the `alpha` feature gate.

chitrangpatel · 2022-05-26T15:06:53Z

@jerop test cases with parameter of type array to apply_test.go added.

tekton-robot · 2022-05-26T15:09:04Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1beta1/task_validation.go	97.7%	97.7%	0.0

chitrangpatel · 2022-05-26T15:10:50Z

/test tekton-pipeline-unit-tests

jerop

thank you @chitrangpatel!

tekton-robot · 2022-05-26T16:03:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop

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:

~~OWNERS~~ [jerop]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jerop · 2022-05-26T16:15:24Z

@tektoncd/core-maintainers please take a look 🙏🏾

abayer · 2022-05-26T17:50:26Z

/lgtm

@chitrangpatel

Marking TEP-0107 as implemented, thank you @chitrangpatel! Pull Requests: - tektoncd/pipeline#4906 - tektoncd/pipeline#4845

@chitrangpatel

Marking TEP-0107 as implemented, thank you @chitrangpatel! Pull Requests: - tektoncd/pipeline#4906 - tektoncd/pipeline#4845

@chitrangpatel

Marking TEP-0107 as implemented, thank you @chitrangpatel! Pull Requests: - tektoncd/pipeline#4906 - tektoncd/pipeline#4845

@chitrangpatel

Marking TEP-0107 as implemented, thank you @chitrangpatel! Pull Requests: - tektoncd/pipeline#4906 - tektoncd/pipeline#4845

Propagated `Parameters` was designed and implemented to replace Implicit `Parameters` to remove the mutation of specifications. In this change, we update the alpha features table to remove the Implicit `Parameters` and the broken link to its documentation, and replace them with Propagated `Parameters`. TEPs: - https://github.com/tektoncd/community/blob/main/teps/0107-propagating-parameters.md - https://github.com/tektoncd/community/blob/main/teps/0023-implicit-mapping.md PRs: - tektoncd#4906 - tektoncd#4845

Propagated `Parameters` was designed and implemented to replace Implicit `Parameters` to remove the mutation of specifications. In this change, we update the alpha features table to remove the Implicit `Parameters` and the broken link to its documentation, and replace them with Propagated `Parameters`. TEPs: - https://github.com/tektoncd/community/blob/main/teps/0107-propagating-parameters.md - https://github.com/tektoncd/community/blob/main/teps/0023-implicit-mapping.md PRs: - #4906 - #4845

tekton-robot requested review from afrittoli and dibyom May 6, 2022 20:59

chitrangpatel force-pushed the TEP-0107-Propagating-Parameters branch from 3f884da to b1e4824 Compare May 6, 2022 21:13

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 6, 2022

chitrangpatel force-pushed the TEP-0107-Propagating-Parameters branch from b1e4824 to be4850b Compare May 9, 2022 14:32

tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 9, 2022

tekton-robot assigned jerop May 10, 2022

jerop changed the title ~~TEP-0107: Propagating Parameters: Implemented implicit parameter propagation.~~ TEP-0107: Propagating Parameters May 10, 2022

jerop reviewed May 10, 2022

View reviewed changes

pkg/reconciler/pipelinerun/resources/apply.go Outdated Show resolved Hide resolved

chitrangpatel force-pushed the TEP-0107-Propagating-Parameters branch from be4850b to 14c67fa Compare May 10, 2022 18:41

jerop reviewed May 10, 2022

View reviewed changes

chitrangpatel force-pushed the TEP-0107-Propagating-Parameters branch from 14c67fa to eb93c5f Compare May 11, 2022 18:10

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 11, 2022

chitrangpatel force-pushed the TEP-0107-Propagating-Parameters branch from bfa2f34 to cbea59f Compare May 26, 2022 15:06

jerop approved these changes May 26, 2022

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 26, 2022

jerop changed the title ~~Implement Parameter Propagation~~ TEP-0107: Implement Parameter Propagation May 26, 2022

tekton-robot assigned abayer May 26, 2022

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 26, 2022

tekton-robot merged commit 30b2bcb into tektoncd:main May 26, 2022

jerop added a commit to jerop/community that referenced this pull request May 26, 2022

TEP-0107: Propagating Parameters

3ff9a9d

Marking TEP-0107 as implemented, thank you @chitrangpatel! Pull Requests: - tektoncd/pipeline#4906 - tektoncd/pipeline#4845

jerop mentioned this pull request May 26, 2022

TEP-0107: Propagating Parameters - Implemented tektoncd/community#712

Merged

jerop added a commit to jerop/community that referenced this pull request May 26, 2022

TEP-0107: Propagating Parameters

c3194ce

Marking TEP-0107 as implemented, thank you @chitrangpatel! Pull Requests: - tektoncd/pipeline#4906 - tektoncd/pipeline#4845

jerop added a commit to jerop/community that referenced this pull request May 26, 2022

TEP-0107: Propagating Parameters - Implemented

f0a2104

Marking TEP-0107 as implemented, thank you @chitrangpatel! Pull Requests: - tektoncd/pipeline#4906 - tektoncd/pipeline#4845

tekton-robot pushed a commit to tektoncd/community that referenced this pull request May 26, 2022

TEP-0107: Propagating Parameters - Implemented

6011246

Marking TEP-0107 as implemented, thank you @chitrangpatel! Pull Requests: - tektoncd/pipeline#4906 - tektoncd/pipeline#4845

This was referenced Jun 9, 2022

Updated Tekton Pipeline to v0.36.0 buildsec/frsca#237

Merged

Tekton Pipeline v0.36.0 causes error in buildpacks pipeline buildpacks/tekton-integration#36

Closed

Buildpacks pipeline v0.2 tektoncd/catalog#992

Merged

jerop mentioned this pull request Jun 16, 2022

Propagated Parameters replaced Implicit Parameters #4994

Merged

3 tasks

chuangw6 mentioned this pull request Jul 14, 2022

TEP0107 & TEP0075 - Issues with propagating object parameters #5140

Closed

pietervincken mentioned this pull request Jul 18, 2022

Unable to use array parameter substitution in pipeline #5149

Closed

chuangw6 mentioned this pull request Jul 19, 2022

Move parameter validation from taskspec to taskrunspec when propagating parameters #5143

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TEP-0107: Implement Parameter Propagation #4845

TEP-0107: Implement Parameter Propagation #4845

chitrangpatel commented May 6, 2022 •

edited

Loading

tekton-robot commented May 6, 2022

dibyom commented May 9, 2022

dibyom commented May 9, 2022

tekton-robot commented May 9, 2022

dibyom commented May 10, 2022

jerop left a comment

chitrangpatel commented May 10, 2022

afrittoli commented May 10, 2022

jerop commented May 10, 2022

jerop left a comment •

edited

Loading

tekton-robot commented May 10, 2022

chitrangpatel commented May 10, 2022 •

edited

Loading

chitrangpatel commented May 10, 2022

dibyom commented May 18, 2022

chitrangpatel commented May 26, 2022

tekton-robot commented May 26, 2022

chitrangpatel commented May 26, 2022

jerop left a comment

tekton-robot commented May 26, 2022

jerop commented May 26, 2022

abayer commented May 26, 2022

TEP-0107: Implement Parameter Propagation #4845

TEP-0107: Implement Parameter Propagation #4845

Conversation

chitrangpatel commented May 6, 2022 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented May 6, 2022

dibyom commented May 9, 2022

dibyom commented May 9, 2022

tekton-robot commented May 9, 2022

dibyom commented May 10, 2022

jerop left a comment

Choose a reason for hiding this comment

chitrangpatel commented May 10, 2022

afrittoli commented May 10, 2022

jerop commented May 10, 2022

jerop left a comment • edited Loading

Choose a reason for hiding this comment

tekton-robot commented May 10, 2022

chitrangpatel commented May 10, 2022 • edited Loading

chitrangpatel commented May 10, 2022

dibyom commented May 18, 2022

chitrangpatel commented May 26, 2022

tekton-robot commented May 26, 2022

chitrangpatel commented May 26, 2022

jerop left a comment

Choose a reason for hiding this comment

tekton-robot commented May 26, 2022

jerop commented May 26, 2022

abayer commented May 26, 2022

chitrangpatel commented May 6, 2022 •

edited

Loading

jerop left a comment •

edited

Loading

chitrangpatel commented May 10, 2022 •

edited

Loading