fix: avoid panic when used pipelineRef or pipelineSpec in pipeline task #7722

l-qing · 2024-02-28T05:11:23Z

This issue has been present since v0.52
6150191

Currently, the pipelineRef and pipelineSpec are only in preview mode and not yet supported. If a user has configured this field and enabled alpha features, it might bypass validation and enter into controller logic. It is now necessary to implement relevant checks within the controller logic to clearly prompt the user, instead of causing the program to panic.

Field definition

pipeline/pkg/apis/pipeline/v1/pipeline_types.go

Lines 239 to 247 in 9be03e2

    
           // PipelineRef is a reference to a pipeline definition 
        
           // Note: PipelineRef is in preview mode and not yet supported 
        
           // +optional 
        
           PipelineRef *PipelineRef `json:"pipelineRef,omitempty"` 
        
           // PipelineSpec is a specification of a pipeline 
        
           // Note: PipelineSpec is in preview mode and not yet supported 
        
           // +optional 
        
           PipelineSpec *PipelineSpec `json:"pipelineSpec,omitempty"`

validation logic

pipeline/pkg/apis/pipeline/v1/pipeline_validation.go

Lines 291 to 305 in 9be03e2

    
           // check the length of nonNilFields 
        
           // if one of taskRef or taskSpec or pipelineRef or pipelineSpec is specified, 
        
           // the length of nonNilFields should exactly be 1 
        
           if len(nonNilFields) > 1 { 
        
           	errs = errs.Also(apis.ErrGeneric("expected exactly one, got multiple", nonNilFields...)) 
        
           } else if len(nonNilFields) == 0 { 
        
           	cfg := config.FromContextOrDefaults(ctx) 
        
           	// check for TaskRef or TaskSpec or PipelineRef or PipelineSpec with alpha feature flag 
        
           	if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields { 
        
           		errs = errs.Also(apis.ErrMissingOneOf(taskRef, taskSpec, pipelineRef, pipelineSpec)) 
        
           	} else { 
        
           		// check for taskRef and taskSpec with beta/stable feature flag 
        
           		errs = errs.Also(apis.ErrMissingOneOf(taskRef, taskSpec)) 
        
           	} 
        
           }

Changes

Submitter Checklist

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

Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
Has Tests included if any functionality added or changed
pre-commit Passed
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
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

fix: avoid panic when used pipelineRef or pipelineSpec in pipeline task

/kind bug

tekton-robot · 2024-02-28T05:11:33Z

Hi @l-qing. 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.

tekton-robot · 2024-02-28T09:52:07Z

@l-qing: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-tekton-pipeline-go-coverage-df

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.

tekton-robot · 2024-02-28T10:02: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/reconciler/pipelinerun/resources/pipelinerunresolution.go	96.7%	96.7%	0.0

tekton-robot · 2024-02-28T10:11:58Z

@l-qing: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test check-pr-has-kind-label

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.

tekton-robot · 2024-02-28T10:28:05Z

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.7%	96.7%	0.0

afrittoli

/ok-to-test

afrittoli

Thanks for this.
/approve

tekton-robot · 2024-02-28T14:50: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/pipelinerun/resources/pipelinerunresolution.go	96.7%	96.7%	0.0

tekton-robot · 2024-02-28T16:03:15Z

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.7%	96.7%	0.0

tekton-robot · 2024-02-28T16:07:51Z

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.7%	96.7%	0.0

l-qing · 2024-02-29T00:55:44Z

I encountered this e2e test error, which seems to be an environment issue, or a problem with the dependent image. But I'm not sure which code repository to check.

2024/02/29 00:32:15 error during command execution:error processing import paths in "config/webhook.yaml": error resolving image references: 
GET https://9236a389bd48b984df91adc1bc924620.r2.cloudflarestorage.com/chainguard-images-prod/sha256%3Aa910da37a1da1a04eefdefb2cda6a5df4d92d6ae0b5da8a602a0905315f63358?X-Amz-Algorithm=REDACTED&X-Amz-Credential=REDACTED&X-Amz-Date=REDACTED&X-Amz-Expires=REDACTED&X-Amz-Signature=REDACTED&X-Amz-SignedHeaders=REDACTED&x-id=REDACTED: 
unexpected status code 403

detail

+ ./test/e2e-tests.sh
================================
==== SETTING UP ENVIRONMENT ====
================================
>> Deploying Tekton Pipelines
2024/02/29 00:32:08 No matching credentials were found, falling back on anonymous
2024/02/29 00:32:13 Using base distroless.dev/static:latest@sha256:02c5e4a18e90b16c039b4bcfe593bfb0b62373d9e7e9fa53e5de3fb0044d9f77 for github.com/tektoncd/pipeline/cmd/webhook
2024/02/29 00:32:13 Using base distroless.dev/static:latest@sha256:02c5e4a18e90b16c039b4bcfe593bfb0b62373d9e7e9fa53e5de3fb0044d9f77 for github.com/tektoncd/pipeline/cmd/events
2024/02/29 00:32:14 Using base distroless.dev/static:latest@sha256:02c5e4a18e90b16c039b4bcfe593bfb0b62373d9e7e9fa53e5de3fb0044d9f77 for github.com/tektoncd/pipeline/cmd/controller
2024/02/29 00:32:14 Using base distroless.dev/static:latest@sha256:02c5e4a18e90b16c039b4bcfe593bfb0b62373d9e7e9fa53e5de3fb0044d9f77 for github.com/tektoncd/pipeline/cmd/resolvers
2024/02/29 00:32:14 Building github.com/tektoncd/pipeline/cmd/events for linux/amd64
2024/02/29 00:32:14 Unexpected error running "go build": context canceled
2024/02/29 00:32:14 Using base distroless.dev/static:latest@sha256:02c5e4a18e90b16c039b4bcfe593bfb0b62373d9e7e9fa53e5de3fb0044d9f77 for github.com/tektoncd/pipeline/cmd/nop
2024/02/29 00:32:14 Building github.com/tektoncd/pipeline/cmd/controller for linux/amd64
2024/02/29 00:32:14 Unexpected error running "go build": context canceled
2024/02/29 00:32:15 Building github.com/tektoncd/pipeline/cmd/resolvers for linux/amd64
2024/02/29 00:32:15 Unexpected error running "go build": context canceled
2024/02/29 00:32:15 Using base distroless.dev/static:latest@sha256:02c5e4a18e90b16c039b4bcfe593bfb0b62373d9e7e9fa53e5de3fb0044d9f77 for github.com/tektoncd/pipeline/cmd/workingdirinit
2024/02/29 00:32:15 Building github.com/tektoncd/pipeline/cmd/nop for linux/amd64
2024/02/29 00:32:15 Unexpected error running "go build": context canceled
2024/02/29 00:32:15 Using base distroless.dev/static:latest@sha256:02c5e4a18e90b16c039b4bcfe593bfb0b62373d9e7e9fa53e5de3fb0044d9f77 for github.com/tektoncd/pipeline/cmd/sidecarlogresults
2024/02/29 00:32:15 Building github.com/tektoncd/pipeline/cmd/workingdirinit for linux/amd64
2024/02/29 00:32:15 Unexpected error running "go build": context canceled
2024/02/29 00:32:15 Using base distroless.dev/static:latest@sha256:02c5e4a18e90b16c039b4bcfe593bfb0b62373d9e7e9fa53e5de3fb0044d9f77 for github.com/tektoncd/pipeline/cmd/entrypoint
2024/02/29 00:32:15 Building github.com/tektoncd/pipeline/cmd/sidecarlogresults for linux/amd64
2024/02/29 00:32:15 Unexpected error running "go build": context canceled
2024/02/29 00:32:15 Building github.com/tektoncd/pipeline/cmd/entrypoint for linux/amd64
2024/02/29 00:32:15 Unexpected error running "go build": context canceled
Error: error processing import paths in "config/webhook.yaml": error resolving image references: GET https://9236a389bd48b984df91adc1bc9[246](https://prow.tekton.dev/view/gs/tekton-prow/pr-logs/pull/tektoncd_pipeline/7722/pull-tekton-pipeline-alpha-integration-tests/1762998278112153600#1:build-log.txt%3A246)20.r2.cloudflarestorage.com/chainguard-images-prod/sha[256](https://prow.tekton.dev/view/gs/tekton-prow/pr-logs/pull/tektoncd_pipeline/7722/pull-tekton-pipeline-alpha-integration-tests/1762998278112153600#1:build-log.txt%3A256)%3Aa910da37a1da1a04eefdefb2cda6a5df4d92d6ae0b5da8a602a0905315f63358?X-Amz-Algorithm=REDACTED&X-Amz-Credential=REDACTED&X-Amz-Date=REDACTED&X-Amz-Expires=REDACTED&X-Amz-Signature=REDACTED&X-Amz-SignedHeaders=REDACTED&x-id=REDACTED: unexpected status code 403 Forbidden: ExpiredRequestRequest has expired
2024/02/29 00:32:15 error during command execution:error processing import paths in "config/webhook.yaml": error resolving image references: GET https://9236a389bd48b984df91adc1bc924620.r2.cloudflarestorage.com/chainguard-images-prod/sha256%3Aa910da37a1da1a04eefdefb2cda6a5df4d92d6ae0b5da8a602a0905315f63358?X-Amz-Algorithm=REDACTED&X-Amz-Credential=REDACTED&X-Amz-Date=REDACTED&X-Amz-Expires=REDACTED&X-Amz-Signature=REDACTED&X-Amz-SignedHeaders=REDACTED&x-id=REDACTED: unexpected status code 403 Forbidden: ExpiredRequestRequest has expired
ERROR: Pipeline image resolve failed

vdemeester · 2024-02-29T15:27:00Z

@l-qing yes this is identified, @imjasonh is looking into it 👼🏼. As of now, we "just" need to be patient 😅

tekton-robot · 2024-03-02T02:39:49Z

@l-qing: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-tekton-pipeline-alpha-integration-tests
/test pull-tekton-pipeline-beta-integration-tests
/test pull-tekton-pipeline-build-tests
/test pull-tekton-pipeline-integration-tests
/test pull-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:

/test pull-tekton-pipeline-go-coverage-df

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.

l-qing · 2024-03-02T02:40:54Z

/cc @vdemeester

tekton-robot · 2024-03-02T02:48: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.7%	96.7%	0.0

fix tektoncd#7720 Currently, the `pipelineRef` and `pipelineSpec` are only in preview mode and not yet supported. If a user has configured this field and enabled alpha features, it might bypass validation and enter into controller logic. It is now necessary to implement relevant checks within the controller logic to clearly prompt the user, instead of causing the program to panic.

tekton-robot · 2024-03-03T02:30: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/pipelinerun/resources/pipelinerunresolution.go	96.7%	96.7%	0.0

tekton-robot · 2024-03-03T02:35: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/pipelinerun/resources/pipelinerunresolution.go	96.7%	96.7%	0.0

vdemeester

/lgtm

tekton-robot · 2024-03-04T16:10:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli, vdemeester

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~~ [afrittoli,vdemeester]

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

vdemeester · 2024-03-04T16:10:59Z

/cherry-pick release-v0.56.x

tekton-robot · 2024-03-04T16:11:02Z

@vdemeester: once the present PR merges, I will cherry-pick it on top of release-v0.56.x in a new PR and assign it to you.

In response to this:

/cherry-pick release-v0.56.x

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.

vdemeester · 2024-03-04T16:11:03Z

/cherry-pick release-v0.53.x

tekton-robot · 2024-03-04T16:11:06Z

@vdemeester: once the present PR merges, I will cherry-pick it on top of release-v0.53.x in a new PR and assign it to you.

In response to this:

/cherry-pick release-v0.53.x

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.

tekton-robot · 2024-03-04T16:12:16Z

@vdemeester: new pull request created: #7733

In response to this:

/cherry-pick release-v0.56.x

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.

tekton-robot · 2024-03-04T16:12:54Z

@vdemeester: new pull request created: #7734

In response to this:

/cherry-pick release-v0.53.x

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.

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. labels Feb 28, 2024

tekton-robot requested review from afrittoli and pritidesai February 28, 2024 05:11

tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 28, 2024

l-qing force-pushed the fix/panic-when-use-pipelineref-in-pipeline-task branch from 4d09050 to c600657 Compare February 28, 2024 10:17

afrittoli reviewed Feb 28, 2024

View reviewed changes

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 Feb 28, 2024

afrittoli reviewed Feb 28, 2024

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 28, 2024

l-qing force-pushed the fix/panic-when-use-pipelineref-in-pipeline-task branch from c600657 to 48ba28c Compare February 28, 2024 15:58

tekton-robot requested a review from vdemeester March 2, 2024 02:40

l-qing force-pushed the fix/panic-when-use-pipelineref-in-pipeline-task branch from 48ba28c to 8215339 Compare March 3, 2024 02:25

vdemeester approved these changes Mar 4, 2024

View reviewed changes

tekton-robot assigned vdemeester Mar 4, 2024

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

tekton-robot merged commit 3757e9d into tektoncd:main Mar 4, 2024
13 checks passed

tekton-robot mentioned this pull request Mar 4, 2024

[release-v0.56.x] fix: avoid panic when used pipelineRef or pipelineSpec in pipeline task #7733

Merged

tekton-robot mentioned this pull request Mar 4, 2024

[release-v0.53.x] fix: avoid panic when used pipelineRef or pipelineSpec in pipeline task #7734

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: avoid panic when used pipelineRef or pipelineSpec in pipeline task #7722

fix: avoid panic when used pipelineRef or pipelineSpec in pipeline task #7722

l-qing commented Feb 28, 2024 •

edited

Loading

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

afrittoli left a comment

afrittoli left a comment

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

l-qing commented Feb 29, 2024

vdemeester commented Feb 29, 2024

tekton-robot commented Mar 2, 2024

l-qing commented Mar 2, 2024

tekton-robot commented Mar 2, 2024

tekton-robot commented Mar 3, 2024

tekton-robot commented Mar 3, 2024

vdemeester left a comment

tekton-robot commented Mar 4, 2024

vdemeester commented Mar 4, 2024

tekton-robot commented Mar 4, 2024

vdemeester commented Mar 4, 2024

tekton-robot commented Mar 4, 2024

tekton-robot commented Mar 4, 2024

tekton-robot commented Mar 4, 2024

	// PipelineRef is a reference to a pipeline definition
	// Note: PipelineRef is in preview mode and not yet supported
	// +optional
	PipelineRef *PipelineRef `json:"pipelineRef,omitempty"`

	// PipelineSpec is a specification of a pipeline
	// Note: PipelineSpec is in preview mode and not yet supported
	// +optional
	PipelineSpec *PipelineSpec `json:"pipelineSpec,omitempty"`

	// check the length of nonNilFields
	// if one of taskRef or taskSpec or pipelineRef or pipelineSpec is specified,
	// the length of nonNilFields should exactly be 1
	if len(nonNilFields) > 1 {
	errs = errs.Also(apis.ErrGeneric("expected exactly one, got multiple", nonNilFields...))
	} else if len(nonNilFields) == 0 {
	cfg := config.FromContextOrDefaults(ctx)
	// check for TaskRef or TaskSpec or PipelineRef or PipelineSpec with alpha feature flag
	if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields {
	errs = errs.Also(apis.ErrMissingOneOf(taskRef, taskSpec, pipelineRef, pipelineSpec))
	} else {
	// check for taskRef and taskSpec with beta/stable feature flag
	errs = errs.Also(apis.ErrMissingOneOf(taskRef, taskSpec))
	}
	}

fix: avoid panic when used pipelineRef or pipelineSpec in pipeline task #7722

fix: avoid panic when used pipelineRef or pipelineSpec in pipeline task #7722

Conversation

l-qing commented Feb 28, 2024 • edited Loading

Field definition

validation logic

Changes

Submitter Checklist

Release Notes

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

afrittoli left a comment

Choose a reason for hiding this comment

afrittoli left a comment

Choose a reason for hiding this comment

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

tekton-robot commented Feb 28, 2024

l-qing commented Feb 29, 2024

vdemeester commented Feb 29, 2024

tekton-robot commented Mar 2, 2024

l-qing commented Mar 2, 2024

tekton-robot commented Mar 2, 2024

tekton-robot commented Mar 3, 2024

tekton-robot commented Mar 3, 2024

vdemeester left a comment

Choose a reason for hiding this comment

tekton-robot commented Mar 4, 2024

vdemeester commented Mar 4, 2024

tekton-robot commented Mar 4, 2024

vdemeester commented Mar 4, 2024

tekton-robot commented Mar 4, 2024

tekton-robot commented Mar 4, 2024

tekton-robot commented Mar 4, 2024

l-qing commented Feb 28, 2024 •

edited

Loading