pipeline icon indicating copy to clipboard operation
pipeline copied to clipboard

Published 20 hours ago • verified publisher icon tektoncd

Move parameter validation from `taskspec` to `taskrunspec` when propagating parameters

Open chitrangpatel opened this issue 3 years ago • 49 comments

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 taskspec and taskrunspec. The changes for validations in pipelinespec and pipelinerunspec will be added in a follow up 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 `taskspec` to `taskrunspec` when propagating parameters

chitrangpatel avatar Jul 14 '22 19:07 chitrangpatel

/kind bug

chitrangpatel avatar Jul 14 '22 19:07 chitrangpatel

cc @chuangw6

chitrangpatel avatar Jul 14 '22 19:07 chitrangpatel

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.6% 97.6% 0.0

tekton-robot avatar Jul 14 '22 19:07 tekton-robot

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/pipelinerun_validation.go 100.0% 81.7% -18.3
pkg/apis/pipeline/v1beta1/taskrun_validation.go 100.0% 92.9% -7.1

tekton-robot avatar Jul 15 '22 19:07 tekton-robot

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/pipelinerun_validation.go 100.0% 95.0% -5.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go 100.0% 98.0% -2.0

tekton-robot avatar Jul 15 '22 20:07 tekton-robot

/retest

chitrangpatel avatar Jul 15 '22 20:07 chitrangpatel

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/pipelinerun_validation.go 100.0% 96.7% -3.3
pkg/apis/pipeline/v1beta1/taskrun_validation.go 100.0% 99.0% -1.0

tekton-robot avatar Jul 15 '22 20:07 tekton-robot

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/pipeline_types.go 97.0% 97.0% 0.1
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go 100.0% 96.7% -3.3
pkg/apis/pipeline/v1beta1/task_validation.go 97.7% 97.3% -0.4
pkg/apis/pipeline/v1beta1/taskrun_validation.go 100.0% 99.0% -1.0
pkg/reconciler/taskrun/taskrun.go 80.3% 80.4% 0.1

tekton-robot avatar Jul 18 '22 13:07 tekton-robot

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/pipeline_types.go 97.0% 97.0% 0.1
pkg/apis/pipeline/v1beta1/task_validation.go 97.7% 97.3% -0.4
pkg/apis/pipeline/v1beta1/taskrun_validation.go 100.0% 98.0% -2.0
pkg/reconciler/taskrun/taskrun.go 80.3% 80.4% 0.1

tekton-robot avatar Jul 18 '22 14:07 tekton-robot

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/pipeline_types.go 97.0% 97.0% 0.1
pkg/apis/pipeline/v1beta1/task_validation.go 97.7% 98.1% 0.4
pkg/reconciler/taskrun/taskrun.go 80.3% 80.4% 0.1

tekton-robot avatar Jul 18 '22 15:07 tekton-robot

/retest

chitrangpatel avatar Jul 18 '22 16:07 chitrangpatel

/retest

chitrangpatel avatar Jul 18 '22 17:07 chitrangpatel

/retest

chitrangpatel avatar Jul 19 '22 15:07 chitrangpatel

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/pipeline_types.go 97.0% 97.0% 0.1
pkg/apis/pipeline/v1beta1/task_validation.go 97.7% 98.1% 0.4
pkg/reconciler/taskrun/taskrun.go 80.3% 80.4% 0.1

tekton-robot avatar Jul 19 '22 18:07 tekton-robot

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/pipeline_types.go 97.0% 97.0% 0.0
pkg/apis/pipeline/v1beta1/task_validation.go 97.7% 98.1% 0.4
pkg/reconciler/taskrun/taskrun.go 80.3% 80.4% 0.1

tekton-robot avatar Jul 19 '22 18:07 tekton-robot

/retest

chitrangpatel avatar Jul 19 '22 19:07 chitrangpatel

/kind bug

chitrangpatel avatar Jul 19 '22 19:07 chitrangpatel

/assign

jerop avatar Jul 19 '22 19:07 jerop

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% 98.1% 0.4
pkg/reconciler/pipelinerun/pipelinerun.go 85.8% 85.8% 0.0
pkg/reconciler/taskrun/taskrun.go 80.3% 80.4% 0.1

tekton-robot avatar Jul 20 '22 20:07 tekton-robot

/retest

chitrangpatel avatar Jul 20 '22 20:07 chitrangpatel

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% 98.1% 0.4
pkg/reconciler/pipelinerun/pipelinerun.go 85.8% 85.8% 0.0
pkg/reconciler/taskrun/taskrun.go 80.3% 80.4% 0.1

tekton-robot avatar Jul 20 '22 21:07 tekton-robot

/retest

chitrangpatel avatar Jul 21 '22 13:07 chitrangpatel

/retest

chitrangpatel avatar Jul 21 '22 13:07 chitrangpatel

/retest

chitrangpatel avatar Jul 21 '22 14:07 chitrangpatel

@jerop this is now ready for review. Thanks in advance 😄

chitrangpatel avatar Jul 21 '22 17:07 chitrangpatel

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.3% 98.1% 0.8
pkg/reconciler/pipelinerun/pipelinerun.go 85.8% 85.8% 0.0
pkg/reconciler/taskrun/taskrun.go 80.6% 80.7% 0.1

tekton-robot avatar Jul 28 '22 18:07 tekton-robot

@lbernick I accidentally re-requested review. Sorry about that. Please hold off until I implement your suggestions.

chitrangpatel avatar Jul 29 '22 15:07 chitrangpatel

thanks Chitrang! Please rebase, and make sure these changes are reflected in v1 as well

Hi @lbernick, should I only update the files in V1 that are similar to those inv1beta1 or also others? I ask because I implemented some changes in files like pipeline_types.go and pipelinerun_validation.go which don't exist in v1. Should they be added there as well?

chitrangpatel avatar Jul 29 '22 16:07 chitrangpatel

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.3% 98.1% 0.8
pkg/reconciler/pipelinerun/pipelinerun.go 85.8% 85.8% 0.0
pkg/reconciler/taskrun/taskrun.go 80.6% 80.7% 0.1

tekton-robot avatar Jul 29 '22 17:07 tekton-robot

should I only update the files in V1 that are similar to those inv1beta1 or also others? I ask because I implemented some changes in files like pipeline_types.go and pipelinerun_validation.go which don't exist in v1. Should they be added there as well?

Until we release v1 (and stop adding new features to v1beta1), we'll have to keep these two directories in sync. (This is a good reason to factor any non-api related logic out of pkg/apis/pipeline.) Any changes to v1beta1 should also be made to v1. The pipeline validation can be done in a separate PR though.

lbernick avatar Jul 29 '22 17:07 lbernick