Added Skip Logic: Matrix parameters cannot be empty arrays #6140

EmmaMunley · 2023-02-09T18:35:40Z

Changes

Added a check in Pipeline validation to verify that matrix parameters are not just arrays, but that they are not empty arrays. This won't change existing functionality/capabilities, but will get rid of the panic in the reconciler.
This addresses issues: #6071

/kind bug

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
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

tekton-robot · 2023-02-09T18:41:06Z

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/param_types.go	98.2%	98.3%	0.0

tekton-robot · 2023-02-09T18:42:55Z

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/apis/pipeline/v1beta1/param_types.go	98.2%	98.3%	0.0

tekton-robot · 2023-02-09T18:54:32Z

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/param_types.go	98.2%	98.3%	0.0

tekton-robot · 2023-02-09T18:56:24Z

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/apis/pipeline/v1beta1/param_types.go	98.2%	98.3%	0.0

tekton-robot · 2023-02-09T18:57:24Z

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/apis/pipeline/v1beta1/param_types.go	98.2%	98.3%	0.0

tekton-robot · 2023-02-09T19:22:38Z

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/param_types.go	98.3%	98.3%	0.0
pkg/apis/pipeline/v1beta1/param_types.go	98.2%	98.3%	0.0

jerop

Thank you @EmmaMunley!

Please update the documentation: https://github.com/tektoncd/pipeline/blob/main/docs/matrix.md
You referenced issue #6137 but it should be #6071

jerop · 2023-02-09T19:26:27Z

cc @abayer

pritidesai · 2023-02-09T19:32:13Z

thanks @EmmaMunley for the PR, I have updated the PR description with the right issue number.

@abayer and @jerop, please review the comment at #6071 (comment)

tekton-robot · 2023-02-09T19:35:10Z

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/apis/pipeline/v1/param_types.go	98.3%	98.3%	0.0
pkg/apis/pipeline/v1beta1/param_types.go	98.2%	98.3%	0.0

tekton-robot · 2023-02-09T19:35:14Z

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/apis/pipeline/v1/param_types.go	98.3%	98.3%	0.0
pkg/apis/pipeline/v1beta1/param_types.go	98.2%	98.3%	0.0

tekton-robot · 2023-02-09T19:47:20Z

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/apis/pipeline/v1/param_types.go	98.3%	98.3%	0.0
pkg/apis/pipeline/v1beta1/param_types.go	98.2%	98.3%	0.0

EmmaMunley · 2023-02-09T19:48:19Z

thanks @EmmaMunley for the PR, I have updated the PR description with the right issue number.

@abayer and @jerop, please review the comment at #6071 (comment)

Thank you @pritidesai. I've updated the matrix documentation.

tekton-robot · 2023-02-09T19:50:22Z

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/param_types.go	98.3%	98.3%	0.0
pkg/apis/pipeline/v1beta1/param_types.go	98.2%	98.3%	0.0

tekton-robot · 2023-02-09T19:51: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/apis/pipeline/v1/param_types.go	98.3%	98.3%	0.0
pkg/apis/pipeline/v1beta1/param_types.go	98.2%	98.3%	0.0

tekton-robot · 2023-02-09T19:51:28Z

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/apis/pipeline/v1/param_types.go	98.3%	98.3%	0.0
pkg/apis/pipeline/v1beta1/param_types.go	98.2%	98.3%	0.0

tekton-robot · 2023-02-09T19:56:17Z

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/apis/pipeline/v1/param_types.go	98.3%	98.3%	0.0
pkg/apis/pipeline/v1beta1/param_types.go	98.2%	98.3%	0.0

dibyom · 2023-03-27T20:03:34Z

/test pull-tekton-pipeline-alpha-integration-tests

tekton-robot · 2023-03-28T18:03:36Z

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.3%	0.4

tekton-robot · 2023-03-28T18:04:42Z

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.3%	0.4

tekton-robot · 2023-03-29T18:54: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.3%	0.4

tekton-robot · 2023-03-29T18:56: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/pipelinerun/resources/pipelinerunresolution.go	96.0%	96.3%	0.4

EmmaMunley · 2023-03-29T19:21:58Z

/assign @dibyom

pkg/reconciler/taskrun/validate_taskrun.go

dibyom · 2023-03-29T21:12:22Z

Overall LGTM! Just one question around adding another unit test

tekton-robot · 2023-03-30T14:37:07Z

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.3%	0.4

tekton-robot · 2023-03-30T14:38:21Z

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.3%	0.4

If a matrix parameter contains an empty array it will be skipped - no PipelineTask will be created - this follows the same logic for when expressions.

tekton-robot · 2023-03-30T17:27:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dibyom

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

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

tekton-robot · 2023-03-30T17:27:22Z

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.3%	0.4

tekton-robot · 2023-03-30T17:28:29Z

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.3%	0.4

jerop

/lgtm

tekton-robot added release-note-none Denotes a PR that doesnt merit a release note. kind/bug Categorizes issue or PR as related to a bug. labels Feb 9, 2023

tekton-robot requested review from dibyom and jerop February 9, 2023 18:35

tekton-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 9, 2023

EmmaMunley force-pushed the matrix-param branch from ac24741 to 5e39dfb Compare February 9, 2023 18:49

tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 9, 2023

EmmaMunley force-pushed the matrix-param branch from 5c26aae to 0180ffc Compare February 9, 2023 19:17

jerop reviewed Feb 9, 2023

View reviewed changes

jerop added this to the Pipelines v0.45 milestone Feb 9, 2023

EmmaMunley force-pushed the matrix-param branch 2 times, most recently from 8e3113c to f30249a Compare February 9, 2023 19:44

EmmaMunley force-pushed the matrix-param branch from 54d985a to cc1fd14 Compare February 9, 2023 20:12

jerop self-assigned this Mar 22, 2023

EmmaMunley force-pushed the matrix-param branch from 1c7969b to ce90b71 Compare March 28, 2023 17:57

EmmaMunley force-pushed the matrix-param branch from ce90b71 to d47b321 Compare March 29, 2023 18:49

tekton-robot assigned dibyom Mar 29, 2023

dibyom reviewed Mar 29, 2023

View reviewed changes

pkg/reconciler/taskrun/validate_taskrun.go Show resolved Hide resolved

EmmaMunley force-pushed the matrix-param branch from d47b321 to 9031421 Compare March 30, 2023 14:30

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2023

EmmaMunley force-pushed the matrix-param branch from 9031421 to ba67df1 Compare March 30, 2023 17:20

Added Skip Logic: Matrix parameters cannot be empty arrays

a13f53f

If a matrix parameter contains an empty array it will be skipped - no PipelineTask will be created - this follows the same logic for when expressions.

EmmaMunley force-pushed the matrix-param branch from ba67df1 to a13f53f Compare March 30, 2023 17:21

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2023

dibyom approved these changes Mar 30, 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 30, 2023

jerop reviewed Mar 30, 2023

View reviewed changes

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

jerop linked an issue Mar 30, 2023 that may be closed by this pull request

PipelineRun reconciler panics for a matrix parameter with an empty array value #6071

Closed

tekton-robot merged commit 697c54b into tektoncd:main Mar 30, 2023

EmmaMunley mentioned this pull request Apr 18, 2023

Add matrix support for using references to entire PipelineRun array parameters #6516

Merged

7 tasks

Added Skip Logic: Matrix parameters cannot be empty arrays #6140

Added Skip Logic: Matrix parameters cannot be empty arrays #6140

Uh oh!

Conversation

EmmaMunley commented Feb 9, 2023 • edited by pritidesai Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Submitter Checklist

Release Notes

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

jerop left a comment

Choose a reason for hiding this comment

Uh oh!

jerop commented Feb 9, 2023

Uh oh!

pritidesai commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

EmmaMunley commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

tekton-robot commented Feb 9, 2023

Uh oh!

dibyom commented Mar 27, 2023

Uh oh!

tekton-robot commented Mar 28, 2023

Uh oh!

tekton-robot commented Mar 28, 2023

Uh oh!

tekton-robot commented Mar 29, 2023

Uh oh!

tekton-robot commented Mar 29, 2023

Uh oh!

EmmaMunley commented Mar 29, 2023

Uh oh!

Uh oh!

dibyom commented Mar 29, 2023

Uh oh!

tekton-robot commented Mar 30, 2023

Uh oh!

tekton-robot commented Mar 30, 2023

Uh oh!

tekton-robot commented Mar 30, 2023

Uh oh!

tekton-robot commented Mar 30, 2023

Uh oh!

tekton-robot commented Mar 30, 2023

Uh oh!

jerop left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

EmmaMunley commented Feb 9, 2023 •

edited by pritidesai

Loading