TEP-0118: Update TaskRun Validation for Matrix Include Params #6418

EmmaMunley · 2023-03-21T21:29:22Z

Changes

This PR updates TaskRun validation to also validate Matrix Include Params. With this change, we validate that all parameters declared in the TaskSpec are present in the TaskRun. It also validates that all parameters have values, parameter types match the specified type and object params have all the keys required.

Also added testing to bring test coverage to 100%.

Note: Matrix Include is still in preview mode.

/kind feature

Submitter Checklist

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

[N/A] Has Docs included if any changes are user facing
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
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
[N/A] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
[N/A] Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

EmmaMunley · 2023-03-21T21:32:24Z

/Assign @jerop @pritidesai
/Hold until #6407 merges
cc: @lbernick

This is branched off of #6407 to avoid merge conflicts.
Please only review these 2 files (the rest are part of #6407)
validate_taskrun.go
validate_taskrun_test.go

tekton-robot · 2023-03-21T21:35:33Z

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/reconciler/pipelinerun/resources/pipelinerunresolution.go	96.0%	96.0%	-0.0
pkg/reconciler/taskrun/validate_taskrun.go	97.8%	96.9%	-0.9

tekton-robot · 2023-03-21T21:38:01Z

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

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	96.0%	96.0%	-0.0
pkg/reconciler/taskrun/validate_taskrun.go	97.8%	96.9%	-0.9

EmmaMunley · 2023-03-22T16:09:48Z

Also not sure why the code coverage went down. I've compared the code coverage with the last PR to have file changes: #6150 and it looks like it has the exact same code coverage, except for whatever reason it is now highlighting in red this section of code (which is unchanged in this PR)

      if _, ok := neededParamsTypes[param.Name]; !ok {
                        // Ignore any missing params - this happens when extra params were
                        // passed to the task that aren't being used.
                        continue
                }

tekton-robot · 2023-03-22T17:24:57Z

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/reconciler/pipelinerun/resources/pipelinerunresolution.go	96.0%	96.0%	-0.0
pkg/reconciler/taskrun/validate_taskrun.go	97.8%	96.9%	-0.9

tekton-robot · 2023-03-22T17:25:06Z

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

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	96.0%	96.0%	-0.0
pkg/reconciler/taskrun/validate_taskrun.go	97.8%	96.9%	-0.9

tekton-robot · 2023-03-22T21:44:59Z

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/reconciler/taskrun/validate_taskrun.go	97.8%	98.4%	0.7

tekton-robot · 2023-03-22T21:46:23Z

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

File	Old Coverage	New Coverage	Delta
pkg/reconciler/taskrun/validate_taskrun.go	97.8%	98.4%	0.7

EmmaMunley · 2023-03-22T21:47:41Z

Also not sure why the code coverage went down. I've compared the code coverage with the last PR to have file changes: #6150 and it looks like it has the exact same code coverage, except for whatever reason it is now highlighting in red this section of code (which is unchanged in this PR)
      if _, ok := neededParamsTypes[param.Name]; !ok {
                        // Ignore any missing params - this happens when extra params were
                        // passed to the task that aren't being used.
                        continue
                }

Added additional testing

tekton-robot · 2023-03-22T22:02:52Z

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/reconciler/taskrun/validate_taskrun.go	97.8%	98.4%	0.7

tekton-robot · 2023-03-22T22:09:49Z

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

File	Old Coverage	New Coverage	Delta
pkg/reconciler/taskrun/validate_taskrun.go	97.8%	98.4%	0.7

pkg/reconciler/taskrun/validate_taskrun.go

pkg/reconciler/taskrun/validate_taskrun_test.go

tekton-robot · 2023-03-24T02:20:14Z

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/reconciler/taskrun/validate_taskrun.go	97.8%	100.0%	2.2

tekton-robot · 2023-03-24T02:27:54Z

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

File	Old Coverage	New Coverage	Delta
pkg/reconciler/taskrun/validate_taskrun.go	97.8%	100.0%	2.2

lbernick

Thanks Emma! Looks great!

pkg/reconciler/taskrun/validate_taskrun_test.go

lbernick · 2023-03-24T13:51:20Z

pkg/reconciler/taskrun/validate_taskrun_test.go

+			Value: *v1beta1.NewStructuredValues("array", "param"),
+		}},
+		matrix:  &v1beta1.Matrix{},
+		wantErr: "invalid input params for task : param types don't match the user-specified type: [foo]",


Suggest including more info in this error message comparing the type received to the type specified; see #6365 (comment) for more detail on why.
Although this might be better in a separate PR

pkg/reconciler/taskrun/validate_taskrun_test.go

tekton-robot · 2023-03-24T13:53:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lbernick

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~~ [lbernick]

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

jerop

/unhold

tekton-robot · 2023-03-27T13:16:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop, lbernick

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,lbernick]

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

This commit updates TaskRun validation to also validate Matrix Include Params. With this change, we validate that all parameters declared in the TaskSpec are present in the TaskRun. It also validates that all parameters have values, parameter types match the specified type and object params have all the keys required. Note: Matrix Include is still in preview mode.

tekton-robot · 2023-03-27T14:32:43Z

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/reconciler/taskrun/validate_taskrun.go	97.8%	100.0%	2.2

tekton-robot · 2023-03-27T14:34:47Z

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

File	Old Coverage	New Coverage	Delta
pkg/reconciler/taskrun/validate_taskrun.go	97.8%	100.0%	2.2

tekton-robot · 2023-03-27T14:34:56Z

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

File	Old Coverage	New Coverage	Delta
pkg/reconciler/taskrun/validate_taskrun.go	97.8%	100.0%	2.2

jerop

/lgtm

jerop · 2023-03-27T16:41:52Z

/test pull-tekton-pipeline-go-coverage

tekton-robot · 2023-03-27T16:47:43Z

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/reconciler/taskrun/validate_taskrun.go	97.8%	100.0%	2.2

tekton-robot added release-note-none Denotes a PR that doesnt merit a release note. kind/feature Categorizes issue or PR as related to a new feature. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 21, 2023

tekton-robot requested review from imjasonh and wlynch March 21, 2023 21:29

tekton-robot assigned jerop Mar 21, 2023

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 21, 2023

tekton-robot assigned pritidesai Mar 21, 2023

EmmaMunley mentioned this pull request Mar 21, 2023

Refactor Matrix Implementation #6407

Merged

7 tasks

EmmaMunley mentioned this pull request Mar 21, 2023

TEP-0118: Implement Fanning Out logic to support Matrix Include Parameters in a Task Run #6341

Merged

7 tasks

jerop added this to the Pipelines v0.47 milestone Mar 22, 2023

EmmaMunley force-pushed the validate-taskrun-matrix-include branch from de08dd5 to 6605a41 Compare March 22, 2023 17:18

EmmaMunley mentioned this pull request Mar 22, 2023

TEP-0118: Matrix with Explicit Combinations of Parameters #6351

Closed

16 tasks

EmmaMunley force-pushed the validate-taskrun-matrix-include branch from 6605a41 to dc39b2b Compare March 22, 2023 21:16

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 22, 2023

EmmaMunley force-pushed the validate-taskrun-matrix-include branch 2 times, most recently from bf005e5 to a866dc6 Compare March 22, 2023 21:39

EmmaMunley force-pushed the validate-taskrun-matrix-include branch from a866dc6 to 0b29a67 Compare March 22, 2023 21:57

lbernick reviewed Mar 23, 2023

View reviewed changes

lbernick self-assigned this Mar 23, 2023

EmmaMunley force-pushed the validate-taskrun-matrix-include branch from 750641e to afe0a98 Compare March 24, 2023 02:14

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 24, 2023

EmmaMunley requested review from lbernick and jerop and removed request for lbernick and jerop March 24, 2023 13:35

lbernick approved these changes Mar 24, 2023

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 24, 2023

jerop approved these changes Mar 27, 2023

View reviewed changes

tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 27, 2023

EmmaMunley force-pushed the validate-taskrun-matrix-include branch from afe0a98 to d1f8e96 Compare March 27, 2023 14:27

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 27, 2023

jerop reviewed Mar 27, 2023

View reviewed changes

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2023

tekton-robot merged commit 8638170 into tektoncd:main Mar 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TEP-0118: Update TaskRun Validation for Matrix Include Params #6418

TEP-0118: Update TaskRun Validation for Matrix Include Params #6418

EmmaMunley commented Mar 21, 2023 •

edited

Loading

EmmaMunley commented Mar 21, 2023

tekton-robot commented Mar 21, 2023

tekton-robot commented Mar 21, 2023

EmmaMunley commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

EmmaMunley commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

tekton-robot commented Mar 24, 2023

tekton-robot commented Mar 24, 2023

lbernick left a comment

lbernick Mar 24, 2023

tekton-robot commented Mar 24, 2023

jerop left a comment

tekton-robot commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

jerop left a comment

jerop commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

TEP-0118: Update TaskRun Validation for Matrix Include Params #6418

TEP-0118: Update TaskRun Validation for Matrix Include Params #6418

Conversation

EmmaMunley commented Mar 21, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

EmmaMunley commented Mar 21, 2023

tekton-robot commented Mar 21, 2023

tekton-robot commented Mar 21, 2023

EmmaMunley commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

EmmaMunley commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

tekton-robot commented Mar 22, 2023

tekton-robot commented Mar 24, 2023

tekton-robot commented Mar 24, 2023

lbernick left a comment

Choose a reason for hiding this comment

lbernick Mar 24, 2023

Choose a reason for hiding this comment

tekton-robot commented Mar 24, 2023

jerop left a comment

Choose a reason for hiding this comment

tekton-robot commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

jerop left a comment

Choose a reason for hiding this comment

jerop commented Mar 27, 2023

tekton-robot commented Mar 27, 2023

EmmaMunley commented Mar 21, 2023 •

edited

Loading