pipeline
pipeline copied to clipboard
tektoncd
Move validation from `pipelinespec` to `pipelinerunspec` when propagating parameters
Prior to this, propagating parameters only skipped webhook validation
for params defined in script. As a result, when users tried to propagate
params to other fields like args or command, etc. an webhook
validation was raised. This PR address issue https://github.com/tektoncd/pipeline/issues/5141.
This PR addresses validations in pipelinespec and pipelinerunspec. The changes for validations in taskspec and taskrunspec are in a separate PR.
Changes
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
- [ ] Has Docs included if any changes are user facing
- [x] Has Tests included if any functionality added or changed
- [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)
- [ ] Release notes contains the string "action required" if the change requires additional action from users switching to the new release
Release Notes
Move parameter validation from `pipelinespec` to `pipelinerunspec` when propagating parameters
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/v1/pipeline_validation.go | 99.4% | 99.4% | 0.0 |
| pkg/reconciler/pipelinerun/pipelinerun.go | 86.0% | 86.0% | 0.0 |
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/v1/pipeline_validation.go | 99.4% | 99.4% | 0.0 |
| pkg/reconciler/pipelinerun/pipelinerun.go | 85.7% | 85.7% | 0.0 |
[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
- ~~OWNERS~~ [jerop]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
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/v1/pipeline_validation.go | 99.4% | 99.4% | 0.0 |
| pkg/reconciler/pipelinerun/pipelinerun.go | 85.7% | 85.7% | 0.0 |
@chitrangpatel: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:
/test pull-tekton-pipeline-alpha-integration-tests/test pull-tekton-pipeline-build-tests/test pull-tekton-pipeline-integration-tests/test 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:
/retest timeout flake again 🙁
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.
/retest
@chitrangpatel /retest won't work if you have something else in the same line
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/v1/pipeline_validation.go | 99.4% | 99.4% | 0.0 |
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/v1/pipeline_validation.go | 99.4% | 99.4% | 0.0 |
| pkg/reconciler/pipelinerun/pipelinerun.go | 85.8% | 85.8% | 0.0 |
It seems that some ArrayOrString params in https://github.com/tektoncd/pipeline/blob/main/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go/#L130 should be updated to ParamValue.
cc @Yongxuanzhang @chitrangpatel Could you please help confirm here?
I agree. I don't recall which PR was merged first. There is backwards compatibility which is why the tests work. However, we should make it ParamValue. I can do that in one shot in a separate PR.