Reject TaskRun taskRef with custom task kind but no apiVersion - #10457
lopster568 wants to merge 1 commit into
Conversation
|
|
|
/kind bug |
There was a problem hiding this comment.
Pull request overview
Adds admission-time validation for TaskRun.spec.taskRef to reject a non-default taskRef.kind when taskRef.apiVersion is missing, aligning TaskRun behavior with existing Pipeline-level custom-task validation and preventing confusing “not found” resolution errors.
Changes:
- Add
TaskRef.Validatechecks in bothv1andv1beta1to requireapiVersionwhenkindis non-default. - Add unit tests covering the newly invalid case and ensuring the explicit default kind remains valid.
- Fix an example TaskRun that used
kind: task(lowercase) so it remains valid under the new validation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/apis/pipeline/v1beta1/taskref_validation.go | Reject non-default kind without apiVersion for v1beta1 TaskRefs. |
| pkg/apis/pipeline/v1beta1/taskref_validation_test.go | Add valid/invalid cases for the new v1beta1 validation rule. |
| pkg/apis/pipeline/v1/taskref_validation.go | Reject non-default kind without apiVersion for v1 TaskRefs. |
| pkg/apis/pipeline/v1/taskref_validation_test.go | Add valid/invalid cases for the new v1 validation rule. |
| examples/v1/taskruns/beta/emit-array-results.yaml | Update TaskRun example to use kind: Task (capitalized). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vdemeester
left a comment
There was a problem hiding this comment.
SGTM, but we will need a very good release-note entry as it could possibly break users isn't it ?
|
Updated the release note to spell out who is affected and what to change. In practice that is leftover kind: ClusterTask manifests and wrong-case values like kind: task, which are accepted today and will be rejected at admission after this change. I modeled the note on the one in #9588, a similar validation tightening shipped as a bug fix with an action required entry. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
cc @tektoncd/core-maintainers |
A TaskRef that sets kind to a non-default value without apiVersion is not recognized as a Custom Task reference (IsCustomTask requires both fields), and the resolver ignores the kind entirely: the ref resolves as an ordinary namespaced Task when one exists, or fails at runtime with a confusing not-found error. Validate the pairing at admission time in TaskRef.Validate for v1 and v1beta1, mirroring the custom task validation Pipelines already perform, and correct the one example that set a lowercase kind on an ordinary task reference. Fixes tektoncd#6557 Signed-off-by: Roshan <rosh.s568@gmail.com>
ded12a6 to
7fc1d86
Compare
Changes
Setting
taskRef.kindto a non-default value withouttaskRef.apiVersioncurrently passes admission validation.TaskRef.IsCustomTask()requires both fields to be set, so such a ref is not treated as a Custom Task, and the resolver ignores the kind entirely: the TaskRun runs an ordinary namespaced Task when one with that name exists, or fails at resolution with a confusingtasks.tekton.dev "foo" not founderror. #6505 added the Pipeline-level checks for this (pipeline tasktaskRefand embeddedtaskSpec); the issue notes the standalone TaskRuntaskRefcase was left open.This adds the pairing check to
TaskRef.Validatein v1 and v1beta1, reusing the pre-existing "custom task ref must specify apiVersion" error from the Pipeline-level custom task validation. The Pipeline path is unaffected:PipelineTask.Validateroutes non-default kinds tovalidateCustomTaskbeforeTaskRef.Validateis reached, so no duplicate errors. An explicitkind: Taskwithout apiVersion stays valid, since the defaulting webhook writeskind: Taskinto every non-resolver taskRef. The reverse case (apiVersion set, kind empty) is deliberately not rejected: such refs resolve as namespaced Tasks today and rejecting them would break working configs. Also correctsexamples/v1/taskruns/beta/emit-array-results.yaml, which setkind: task(lowercase) on an ordinary task reference and would have been rejected by the new check.docs/taskruns.mddocuments onlytaskRef.namefor TaskRuns,taskRef.kindis not documented there, so no doc change is included.Fixes #6557
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes