Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inconsistent ClusterTask references in V1 #6587

Closed
lbernick opened this issue Apr 26, 2023 · 2 comments · Fixed by #6588
Closed

inconsistent ClusterTask references in V1 #6587

lbernick opened this issue Apr 26, 2023 · 2 comments · Fixed by #6588
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@lbernick
Copy link
Member

lbernick commented Apr 26, 2023

Our docs state that "a v1beta1 clustertask can still be referenced in a v1 pipeline".

The following taskrun succeeds:

apiVersion: tekton.dev/v1
kind: TaskRun
metadata:
  name: v1-taskrun
spec:
  taskRef:
    name: my-cluster-task
    kind: ClusterTask

However, the following Pipeline cannot be created, even though the TaskRef is the same:

apiVersion: tekton.dev/v1
kind: Pipeline
metadata:
  name: v1-pipeline
spec:
  tasks:
  - name: first
    taskRef:
      name: my-cluster-task
      kind: ClusterTask

Error message is

Error from server (BadRequest): error when creating "/Users/leebernick/tkndefs/clustertask/v1pipelinerun.yaml": admission webhook "validation.webhook.pipeline.tekton.dev" denied the request: validation failed: invalid value: custom task ref must specify apiVersion: spec.tasks[0].taskRef.apiVersion

(introduced in #6505) -- fyi @jerop @Yongxuanzhang.

Labeling this as a bug since our docs are inconsistent with implementation, and TaskRef is inconsistent between TaskRun and PipelineRun (a separate issue #6557 was created for TaskRun taskrefs, but in hindsight I think it would have been better to treat all TaskRefs the same).

I think we need to decide whether ClusterTasks can be referenced from v1 TaskRefs (I vote yes).

If so, we should fix this validation. There shouldn't be any work required when swapping the v1 storage version, i.e. I don't think we need to reopen #6552, since we should be able to update this function here to convert a clustertask into a v1 Task instead of a v1beta1 Task. (FYI @JeromeJu @dibyom)

If not, we need to update docs and disallow clustertask references in validation. I'm not sure if we want to clarify the error message returned here, or whether we'd want to treat kind "ClusterTask" as a custom task reference (this could be a bit confusing). Note that ClusterTask support cannot be removed until July.

@lbernick lbernick added the kind/bug Categorizes issue or PR as related to a bug. label Apr 26, 2023
@dibyom
Copy link
Member

dibyom commented Apr 26, 2023

I think we need to decide whether ClusterTasks can be referenced from v1 TaskRefs (I vote yes).

Yes, we need to support this since the earliest EOL for cluster tasks is July.

If so, we should fix this validation. There shouldn't be any work required when swapping the v1 storage version, i.e. I don't think we need to reopen #6552, since we should be able to update this function here to convert a clustertask into a v1 Task instead of a v1beta1 Task. (FYI @JeromeJu @dibyom)

Nice!!

@Yongxuanzhang
Copy link
Member

Is it because we didn't add cluster task in v1's validation funciton?:

	// taskKinds contains the kinds when the apiVersion is not set, they are not custom tasks,
	// if apiVersion is set they are custom tasks.
	taskKinds := map[TaskKind]bool{
		"":                 true,
		NamespacedTaskKind: true,
		ClusterTaskKind:    true,
	}

I added at first but @jerop said we don't have cluster task in v1.

To address this issue I think we need to add it right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants