Add validation for beta features in v1 remote Tasks/Pipelines

Currently, when a user creates a v1 Task or Pipeline using beta features
on their cluster, we validate that "enable-api-fields" is set to "alpha"
or "beta", and if not, reject the Task or Pipeline.

However, no such validation is done for v1 remote Tasks or Pipelines.
This is because we validate remote Task and Pipeline specs in the reconciler
after converting them to v1beta1. This commit adds the same validation to remote v1
Tasks and Pipelines that we are already performing for local v1 Tasks and Pipelines.

pkg/apis/pipeline/v1/pipeline_validation.go

@@ -70,7 +70,7 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	errs = errs.Also(validatePipelineContextVariables(ps.Tasks).ViaField("tasks"))
 	errs = errs.Also(validatePipelineContextVariables(ps.Finally).ViaField("finally"))
 	errs = errs.Also(validateExecutionStatusVariables(ps.Tasks, ps.Finally))
-	errs = errs.Also(ps.validateBetaFields(ctx))
+	errs = errs.Also(ps.ValidateBetaFields(ctx))
 	// Validate the pipeline's workspaces.
 	errs = errs.Also(validatePipelineWorkspacesDeclarations(ps.Workspaces))
 	// Validate the pipeline's results
@@ -84,9 +84,9 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	return errs
 }
 
-// validateBetaFields returns an error if the Pipeline spec uses beta features but does not
+// ValidateBetaFields returns an error if the Pipeline spec uses beta features but does not
 // have "enable-api-fields" set to "alpha" or "beta".
-func (ps *PipelineSpec) validateBetaFields(ctx context.Context) *apis.FieldError {
+func (ps *PipelineSpec) ValidateBetaFields(ctx context.Context) *apis.FieldError {
 	var errs *apis.FieldError
 	// Object parameters
 	for i, p := range ps.Params {
@@ -133,7 +133,7 @@ func (pt *PipelineTask) validateBetaFields(ctx context.Context) *apis.FieldError
 			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.params", config.BetaAPIFields))
 		}
 	} else if pt.TaskSpec != nil {
-		errs = errs.Also(pt.TaskSpec.validateBetaFields(ctx))
+		errs = errs.Also(pt.TaskSpec.ValidateBetaFields(ctx))
 	}
 	return errs
 }

pkg/apis/pipeline/v1/task_validation.go

@@ -90,16 +90,16 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	errs = errs.Also(validateSidecarNames(ts.Sidecars))
 	errs = errs.Also(ValidateParameterTypes(ctx, ts.Params).ViaField("params"))
 	errs = errs.Also(ValidateParameterVariables(ctx, ts.Steps, ts.Params))
-	errs = errs.Also(ts.validateBetaFields(ctx))
+	errs = errs.Also(ts.ValidateBetaFields(ctx))
 	errs = errs.Also(validateTaskContextVariables(ctx, ts.Steps))
 	errs = errs.Also(validateTaskResultsVariables(ctx, ts.Steps, ts.Results))
 	errs = errs.Also(validateResults(ctx, ts.Results).ViaField("results"))
 	return errs
 }
 
-// validateBetaFields returns an error if the Task spec uses beta features but does not
+// ValidateBetaFields returns an error if the Task spec uses beta features but does not
 // have "enable-api-fields" set to "alpha" or "beta".
-func (ts *TaskSpec) validateBetaFields(ctx context.Context) *apis.FieldError {
+func (ts *TaskSpec) ValidateBetaFields(ctx context.Context) *apis.FieldError {
 	var errs *apis.FieldError
 	// Object parameters
 	for i, p := range ts.Params {

pkg/reconciler/pipelinerun/resources/pipelineref.go

@@ -161,6 +161,11 @@ func readRuntimeObjectAsPipeline(ctx context.Context, obj runtime.Object, k8s ku
 		return obj, nil
 	case *v1.Pipeline:
 		// TODO(#6356): Support V1 Task verification
+		// Validation of beta fields must happen before the V1 Pipeline is converted into the storage version of the API.
+		// TODO(#6592): Decouple API versioning from feature versioning
+		if err := obj.Spec.ValidateBetaFields(ctx); err != nil {
+			return nil, fmt.Errorf("invalid remote Pipeline %s: %w", obj.GetName(), err)
+		}
 		t := &v1beta1.Pipeline{
 			TypeMeta: metav1.TypeMeta{
 				Kind:       "Pipeline",

pkg/reconciler/pipelinerun/resources/pipelineref_test.go

@@ -291,26 +291,46 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) {
 	ctx := context.Background()
 	cfg := config.FromContextOrDefaults(ctx)
 	ctx = config.ToContext(ctx, cfg)
-	pipeline := parse.MustParseV1beta1Pipeline(t, pipelineYAMLString)
 	pipelineRef := &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}
 
 	testcases := []struct {
 		name         string
 		pipelineYAML string
+		wantPipeline *v1beta1.Pipeline
+		wantErr      bool
 	}{{
 		name: "v1beta1 pipeline",
 		pipelineYAML: strings.Join([]string{
 			"kind: Pipeline",
 			"apiVersion: tekton.dev/v1beta1",
 			pipelineYAMLString,
 		}, "\n"),
+		wantPipeline: parse.MustParseV1beta1Pipeline(t, pipelineYAMLString),
+	}, {
+		name: "v1beta1 pipeline with beta features",
+		pipelineYAML: strings.Join([]string{
+			"kind: Pipeline",
+			"apiVersion: tekton.dev/v1beta1",
+			pipelineYAMLStringWithBetaFeatures,
+		}, "\n"),
+		wantPipeline: parse.MustParseV1beta1Pipeline(t, pipelineYAMLStringWithBetaFeatures),
 	}, {
 		name: "v1 pipeline",
 		pipelineYAML: strings.Join([]string{
 			"kind: Pipeline",
 			"apiVersion: tekton.dev/v1",
 			pipelineYAMLString,
 		}, "\n"),
+		wantPipeline: parse.MustParseV1beta1Pipeline(t, pipelineYAMLString),
+	}, {
+		name: "v1 pipeline with beta features",
+		pipelineYAML: strings.Join([]string{
+			"kind: Pipeline",
+			"apiVersion: tekton.dev/v1",
+			pipelineYAMLStringWithBetaFeatures,
+		}, "\n"),
+		wantPipeline: nil,
+		wantErr:      true,
 	}}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -325,16 +345,22 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) {
 			}, nil /*VerificationPolicies*/)
 
 			resolvedPipeline, resolvedRefSource, err := fn(ctx, pipelineRef.Name)
-			if err != nil {
-				t.Fatalf("failed to call pipelinefn: %s", err.Error())
-			}
-
-			if diff := cmp.Diff(pipeline, resolvedPipeline); diff != "" {
-				t.Error(diff)
-			}
-
-			if d := cmp.Diff(sampleRefSource, resolvedRefSource); d != "" {
-				t.Errorf("refSources did not match: %s", diff.PrintWantGot(d))
+			if tc.wantErr {
+				if err == nil {
+					t.Errorf("expected an error when calling pipelinefunc but got none")
+				}
+			} else {
+				if err != nil {
+					t.Fatalf("failed to call pipelinefn: %s", err.Error())
+				}
+
+				if diff := cmp.Diff(tc.wantPipeline, resolvedPipeline); diff != "" {
+					t.Error(diff)
+				}
+
+				if d := cmp.Diff(sampleRefSource, resolvedRefSource); d != "" {
+					t.Errorf("refSources did not match: %s", diff.PrintWantGot(d))
+				}
 			}
 		})
 	}
@@ -900,3 +926,16 @@ spec:
         script: |
           echo "hello world!"
 `
+
+var pipelineYAMLStringWithBetaFeatures = `
+metadata:
+  name: foo
+spec:
+  tasks:
+  - name: task1
+    taskRef:
+      resolver: git
+      params:
+      - name: name
+        value: test-task
+`

pkg/reconciler/taskrun/resources/taskref.go

@@ -184,6 +184,11 @@ func readRuntimeObjectAsTask(ctx context.Context, obj runtime.Object, k8s kubern
 		return convertClusterTaskToTask(*obj), nil
 	case *v1.Task:
 		// TODO(#6356): Support V1 Task verification
+		// Validation of beta fields must happen before the V1 Task is converted into the storage version of the API.
+		// TODO(#6592): Decouple API versioning from feature versioning
+		if err := obj.Spec.ValidateBetaFields(ctx); err != nil {
+			return nil, fmt.Errorf("invalid remote Task %s: %w", obj.GetName(), err)
+		}
 		t := &v1beta1.Task{
 			TypeMeta: metav1.TypeMeta{
 				Kind:       "Task",

pkg/reconciler/taskrun/resources/taskref_test.go

@@ -551,26 +551,45 @@ func TestGetTaskFunc_RemoteResolution(t *testing.T) {
 	ctx := context.Background()
 	cfg := config.FromContextOrDefaults(ctx)
 	ctx = config.ToContext(ctx, cfg)
-	task := parse.MustParseV1beta1Task(t, taskYAMLString)
 	taskRef := &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}
 
 	testcases := []struct {
 		name     string
 		taskYAML string
+		wantTask *v1beta1.Task
+		wantErr  bool
 	}{{
 		name: "v1beta1 task",
 		taskYAML: strings.Join([]string{
 			"kind: Task",
 			"apiVersion: tekton.dev/v1beta1",
 			taskYAMLString,
 		}, "\n"),
+		wantTask: parse.MustParseV1beta1Task(t, taskYAMLString),
+	}, {
+		name: "v1beta1 task with beta features",
+		taskYAML: strings.Join([]string{
+			"kind: Task",
+			"apiVersion: tekton.dev/v1beta1",
+			taskYAMLStringWithBetaFeatures,
+		}, "\n"),
+		wantTask: parse.MustParseV1beta1Task(t, taskYAMLStringWithBetaFeatures),
 	}, {
 		name: "v1 task",
 		taskYAML: strings.Join([]string{
 			"kind: Task",
 			"apiVersion: tekton.dev/v1",
 			taskYAMLString,
 		}, "\n"),
+		wantTask: parse.MustParseV1beta1Task(t, taskYAMLString),
+	}, {
+		name: "v1 task with beta features",
+		taskYAML: strings.Join([]string{
+			"kind: Task",
+			"apiVersion: tekton.dev/v1",
+			taskYAMLStringWithBetaFeatures,
+		}, "\n"),
+		wantErr: true,
 	}}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -586,16 +605,22 @@ func TestGetTaskFunc_RemoteResolution(t *testing.T) {
 			fn := resources.GetTaskFunc(ctx, nil, nil, requester, tr, tr.Spec.TaskRef, "", "default", "default", nil /*VerificationPolicies*/)
 
 			resolvedTask, resolvedRefSource, err := fn(ctx, taskRef.Name)
-			if err != nil {
-				t.Fatalf("failed to call pipelinefn: %s", err.Error())
-			}
-
-			if d := cmp.Diff(sampleRefSource, resolvedRefSource); d != "" {
-				t.Errorf("refSources did not match: %s", diff.PrintWantGot(d))
-			}
-
-			if d := cmp.Diff(task, resolvedTask); d != "" {
-				t.Errorf("resolvedTask did not match: %s", diff.PrintWantGot(d))
+			if tc.wantErr {
+				if err == nil {
+					t.Fatalf("expected an error when calling taskfunc but got none")
+				}
+			} else {
+				if err != nil {
+					t.Fatalf("failed to call taskfn: %s", err.Error())
+				}
+
+				if d := cmp.Diff(sampleRefSource, resolvedRefSource); d != "" {
+					t.Errorf("refSources did not match: %s", diff.PrintWantGot(d))
+				}
+
+				if d := cmp.Diff(tc.wantTask, resolvedTask); d != "" {
+					t.Errorf("resolvedTask did not match: %s", diff.PrintWantGot(d))
+				}
 			}
 		})
 	}
@@ -1078,3 +1103,17 @@ spec:
     script: |
       echo "hello world!"
 `
+
+var taskYAMLStringWithBetaFeatures = `
+metadata:
+  name: foo
+spec:
+  steps:
+  - name: step1
+    image: ubuntu
+    script: |
+      echo "hello world!"
+  results:
+  - name: array-result
+    type: array
+`