Add webhook validation for remote Tasks

pkg/pod/status.go

@@ -51,6 +51,10 @@ const (
 	// that taskrun failed runtime validation
 	ReasonFailedValidation = "TaskRunValidationFailed"
 
+	// ReasonTaskFailedValidation indicated that the reason for failure status is
+	// that task failed runtime validation
+	ReasonTaskFailedValidation = "TaskValidationFailed"
+
 	// ReasonExceededResourceQuota indicates that the TaskRun failed to create a pod due to
 	// a ResourceQuota in the namespace
 	ReasonExceededResourceQuota = "ExceededResourceQuota"

pkg/reconciler/apiserver/apiserver.go

@@ -0,0 +1,80 @@
+package apiserver
+
+import (
+	"context"
+	"errors"
+	"fmt"
+
+	"github.com/google/uuid"
+	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
+	clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+var (
+	ErrReferencedObjectValidationFailed = errors.New("validation failed for referenced object")
+	ErrCouldntValidateObjectRetryable   = errors.New("retryable error validating referenced object")
+	ErrCouldntValidateObjectPermanent   = errors.New("permanent error validating referenced object")
+)
+
+// DryRunValidate validates the obj by issuing a dry-run create request for it in the given namespace.
+// This allows validating admission webhooks to process the object without actually creating it.
+// obj must be a v1/v1beta1 Task or Pipeline.
+func DryRunValidate(ctx context.Context, namespace string, obj runtime.Object, tekton clientset.Interface) error {
+	dryRunObjName := uuid.NewString() // Use a randomized name for the Pipeline/Task in case there is already another Pipeline/Task of the same name
+
+	switch obj := obj.(type) {
+	case *v1.Pipeline:
+		dryRunObj := obj.DeepCopy()
+		dryRunObj.Name = dryRunObjName
+		dryRunObj.Namespace = namespace // Make sure the namespace is the same as the PipelineRun
+		if _, err := tekton.TektonV1().Pipelines(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
+			return handleDryRunCreateErr(err, obj.Name)
+		}
+	case *v1beta1.Pipeline:
+		dryRunObj := obj.DeepCopy()
+		dryRunObj.Name = dryRunObjName
+		dryRunObj.Namespace = namespace // Make sure the namespace is the same as the PipelineRun
+		if _, err := tekton.TektonV1beta1().Pipelines(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
+			return handleDryRunCreateErr(err, obj.Name)
+		}
+
+	case *v1.Task:
+		dryRunObj := obj.DeepCopy()
+		dryRunObj.Name = dryRunObjName
+		dryRunObj.Namespace = namespace // Make sure the namespace is the same as the TaskRun
+		if _, err := tekton.TektonV1().Tasks(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
+			return handleDryRunCreateErr(err, obj.Name)
+		}
+	case *v1beta1.Task:
+		dryRunObj := obj.DeepCopy()
+		dryRunObj.Name = dryRunObjName
+		dryRunObj.Namespace = namespace // Make sure the namespace is the same as the TaskRun
+		if _, err := tekton.TektonV1beta1().Tasks(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
+			return handleDryRunCreateErr(err, obj.Name)
+		}
+	default:
+		return fmt.Errorf("unsupported object GVK %s", obj.GetObjectKind().GroupVersionKind())
+	}
+	return nil
+}
+
+func handleDryRunCreateErr(err error, objectName string) error {
+	var errType error
+	switch {
+	case apierrors.IsBadRequest(err): // Object rejected by validating webhook
+		errType = ErrReferencedObjectValidationFailed
+	case apierrors.IsInvalid(err), apierrors.IsMethodNotSupported(err):
+		errType = ErrCouldntValidateObjectPermanent
+	case apierrors.IsTimeout(err), apierrors.IsServerTimeout(err), apierrors.IsTooManyRequests(err):
+		errType = ErrCouldntValidateObjectRetryable
+	default:
+		// Assume unknown errors are retryable
+		// Additional errors can be added to the switch statements as needed
+		errType = ErrCouldntValidateObjectRetryable
+	}
+	return fmt.Errorf("%w %s: %s", errType, objectName, err.Error())
+}

pkg/reconciler/apiserver/apiserver_test.go

@@ -0,0 +1,141 @@
+package apiserver_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
+	"github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake"
+	"github.com/tektoncd/pipeline/pkg/reconciler/apiserver"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	ktesting "k8s.io/client-go/testing"
+)
+
+func TestDryRunCreate_Valid_DifferentGVKs(t *testing.T) {
+	tcs := []struct {
+		name    string
+		obj     runtime.Object
+		wantErr bool
+	}{{
+		name: "v1 task",
+		obj:  &v1.Task{},
+	}, {
+		name: "v1beta1 task",
+		obj:  &v1beta1.Task{},
+	}, {
+		name: "v1 pipeline",
+		obj:  &v1.Pipeline{},
+	}, {
+		name: "v1beta1 pipeline",
+		obj:  &v1beta1.Pipeline{},
+	}, {
+		name:    "unsupported gvk",
+		obj:     &v1beta1.ClusterTask{},
+		wantErr: true,
+	}}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			tektonclient := fake.NewSimpleClientset()
+			err := apiserver.DryRunValidate(context.Background(), "default", tc.obj, tektonclient)
+			if (err != nil) != tc.wantErr {
+				t.Errorf("wantErr was %t but got err %v", tc.wantErr, err)
+			}
+		})
+	}
+}
+
+func TestDryRunCreate_Invalid_DifferentGVKs(t *testing.T) {
+	tcs := []struct {
+		name    string
+		obj     runtime.Object
+		wantErr error
+	}{{
+		name:    "v1 task",
+		obj:     &v1.Task{},
+		wantErr: apiserver.ErrReferencedObjectValidationFailed,
+	}, {
+		name:    "v1beta1 task",
+		obj:     &v1beta1.Task{},
+		wantErr: apiserver.ErrReferencedObjectValidationFailed,
+	}, {
+		name:    "v1 pipeline",
+		obj:     &v1.Pipeline{},
+		wantErr: apiserver.ErrReferencedObjectValidationFailed,
+	}, {
+		name:    "v1beta1 pipeline",
+		obj:     &v1beta1.Pipeline{},
+		wantErr: apiserver.ErrReferencedObjectValidationFailed,
+	}, {
+		name:    "unsupported gvk",
+		obj:     &v1beta1.ClusterTask{},
+		wantErr: cmpopts.AnyError,
+	}}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			tektonclient := fake.NewSimpleClientset()
+			tektonclient.PrependReactor("create", "tasks", func(action ktesting.Action) (bool, runtime.Object, error) {
+				return true, nil, apierrors.NewBadRequest("bad request")
+			})
+			tektonclient.PrependReactor("create", "pipelines", func(action ktesting.Action) (bool, runtime.Object, error) {
+				return true, nil, apierrors.NewBadRequest("bad request")
+			})
+			err := apiserver.DryRunValidate(context.Background(), "default", tc.obj, tektonclient)
+			if d := cmp.Diff(tc.wantErr, err, cmpopts.EquateErrors()); d != "" {
+				t.Errorf("wrong error: %s", d)
+			}
+		})
+	}
+}
+
+func TestDryRunCreate_DifferentErrTypes(t *testing.T) {
+	tcs := []struct {
+		name       string
+		webhookErr error
+		wantErr    error
+	}{{
+		name:    "no error",
+		wantErr: nil,
+	}, {
+		name:       "bad request",
+		webhookErr: apierrors.NewBadRequest("bad request"),
+		wantErr:    apiserver.ErrReferencedObjectValidationFailed,
+	}, {
+		name:       "invalid",
+		webhookErr: apierrors.NewInvalid(schema.GroupKind{Group: "tekton.dev/v1", Kind: "Task"}, "task", field.ErrorList{}),
+		wantErr:    apiserver.ErrCouldntValidateObjectPermanent,
+	}, {
+		name:       "not supported",
+		webhookErr: apierrors.NewMethodNotSupported(schema.GroupResource{}, "create"),
+		wantErr:    apiserver.ErrCouldntValidateObjectPermanent,
+	}, {
+		name:       "timeout",
+		webhookErr: apierrors.NewTimeoutError("timeout", 5),
+		wantErr:    apiserver.ErrCouldntValidateObjectRetryable,
+	}, {
+		name:       "server timeout",
+		webhookErr: apierrors.NewServerTimeout(schema.GroupResource{}, "create", 5),
+		wantErr:    apiserver.ErrCouldntValidateObjectRetryable,
+	}, {
+		name:       "too many requests",
+		webhookErr: apierrors.NewTooManyRequests("foo", 5),
+		wantErr:    apiserver.ErrCouldntValidateObjectRetryable,
+	}}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			tektonclient := fake.NewSimpleClientset()
+			tektonclient.PrependReactor("create", "tasks", func(action ktesting.Action) (bool, runtime.Object, error) {
+				return true, nil, tc.webhookErr
+			})
+			err := apiserver.DryRunValidate(context.Background(), "default", &v1.Task{}, tektonclient)
+			if d := cmp.Diff(tc.wantErr, err, cmpopts.EquateErrors()); d != "" {
+				t.Errorf("wrong error: %s", d)
+			}
+		})
+	}
+}

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -40,6 +40,7 @@ import (
 	resolutionutil "github.com/tektoncd/pipeline/pkg/internal/resolution"
 	"github.com/tektoncd/pipeline/pkg/pipelinerunmetrics"
 	tknreconciler "github.com/tektoncd/pipeline/pkg/reconciler"
+	"github.com/tektoncd/pipeline/pkg/reconciler/apiserver"
 	"github.com/tektoncd/pipeline/pkg/reconciler/events"
 	"github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent"
 	"github.com/tektoncd/pipeline/pkg/reconciler/pipeline/dag"
@@ -408,10 +409,10 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel
 		message := fmt.Sprintf("PipelineRun %s/%s awaiting remote resource", pr.Namespace, pr.Name)
 		pr.Status.MarkRunning(ReasonResolvingPipelineRef, message)
 		return nil
-	case errors.Is(err, resources.ErrReferencedPipelineValidationFailed), errors.Is(err, resources.ErrCouldntValidatePipelinePermanent):
+	case errors.Is(err, apiserver.ErrReferencedObjectValidationFailed), errors.Is(err, apiserver.ErrCouldntValidateObjectPermanent):
 		pr.Status.MarkFailed(ReasonFailedValidation, err.Error())
 		return controller.NewPermanentError(err)
-	case errors.Is(err, resources.ErrCouldntValidatePipelineRetryable):
+	case errors.Is(err, apiserver.ErrCouldntValidateObjectRetryable):
 		return err
 	case err != nil:
 		logger.Errorf("Failed to determine Pipeline spec to use for pipelinerun %s: %v", pr.Name, err)

pkg/reconciler/pipelinerun/resources/pipelineref.go

@@ -21,28 +21,21 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/google/uuid"
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
+	"github.com/tektoncd/pipeline/pkg/reconciler/apiserver"
 	rprp "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/pipelinespec"
 	"github.com/tektoncd/pipeline/pkg/remote"
 	"github.com/tektoncd/pipeline/pkg/remote/resolution"
 	remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource"
 	"github.com/tektoncd/pipeline/pkg/trustedresources"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
 )
 
-var (
-	ErrReferencedPipelineValidationFailed = errors.New("validation failed for referenced Pipeline")
-	ErrCouldntValidatePipelineRetryable   = errors.New("retryable error validating referenced Pipeline")
-	ErrCouldntValidatePipelinePermanent   = errors.New("permanent error validating referenced Pipeline")
-)
-
 // GetPipelineFunc is a factory function that will use the given PipelineRef to return a valid GetPipeline function that
 // looks up the pipeline. It uses as context a k8s client, tekton client, namespace, and service account name to return
 // the pipeline. It knows whether it needs to look in the cluster or in a remote location to fetch the reference.
@@ -150,11 +143,8 @@ func readRuntimeObjectAsPipeline(ctx context.Context, namespace string, obj runt
 		// Validation must happen before the v1beta1 Pipeline is converted into the storage version of the API,
 		// since validation of beta features differs between v1 and v1beta1
 		// TODO(#6592): Decouple API versioning from feature versioning
-		dryRunObj := obj.DeepCopy()
-		dryRunObj.Name = uuid.NewString() // Use a randomized name for the Pipeline in case there is already another Pipeline of the same name
-		dryRunObj.Namespace = namespace   // Make sure the namespace is the same as the PipelineRun
-		if _, err := tekton.TektonV1beta1().Pipelines(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
-			return nil, nil, handleDryRunCreateErr(err, obj.Name)
+		if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil {
+			return nil, nil, err
 		}
 		p := &v1.Pipeline{
 			TypeMeta: metav1.TypeMeta{
@@ -170,30 +160,10 @@ func readRuntimeObjectAsPipeline(ctx context.Context, namespace string, obj runt
 		vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies)
 		// Issue a dry-run request to create the remote Pipeline, so that it can undergo validation from validating admission webhooks
 		// without actually creating the Pipeline on the cluster
-		dryRunObj := obj.DeepCopy()
-		dryRunObj.Name = uuid.NewString() // Use a randomized name for the Pipeline in case there is already another Pipeline of the same name
-		dryRunObj.Namespace = namespace   // Make sure the namespace is the same as the PipelineRun
-		if _, err := tekton.TektonV1().Pipelines(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
-			return nil, nil, handleDryRunCreateErr(err, obj.Name)
+		if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil {
+			return nil, nil, err
 		}
 		return obj, &vr, nil
 	}
 	return nil, nil, errors.New("resource is not a pipeline")
 }
-
-func handleDryRunCreateErr(err error, objectName string) error {
-	var errType error
-	switch {
-	case apierrors.IsBadRequest(err): // Pipeline rejected by validating webhook
-		errType = ErrReferencedPipelineValidationFailed
-	case apierrors.IsInvalid(err), apierrors.IsMethodNotSupported(err):
-		errType = ErrCouldntValidatePipelinePermanent
-	case apierrors.IsTimeout(err), apierrors.IsServerTimeout(err), apierrors.IsTooManyRequests(err):
-		errType = ErrCouldntValidatePipelineRetryable
-	default:
-		// Assume unknown errors are retryable
-		// Additional errors can be added to the switch statements as needed
-		errType = ErrCouldntValidatePipelineRetryable
-	}
-	return fmt.Errorf("%w %s: %s", errType, objectName, err.Error())
-}

pkg/reconciler/pipelinerun/resources/pipelineref_test.go

@@ -36,6 +36,7 @@ import (
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake"
 	clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake"
+	"github.com/tektoncd/pipeline/pkg/reconciler/apiserver"
 	"github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources"
 	ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing"
 	"github.com/tektoncd/pipeline/pkg/trustedresources"
@@ -397,7 +398,7 @@ func TestGetPipelineFunc_RemoteResolution_ValidationFailure(t *testing.T) {
 			})
 
 			resolvedPipeline, resolvedRefSource, _, err := fn(ctx, pipelineRef.Name)
-			if !errors.Is(err, resources.ErrReferencedPipelineValidationFailed) {
+			if !errors.Is(err, apiserver.ErrReferencedObjectValidationFailed) {
 				t.Errorf("expected RemotePipelineValidationFailed error but got none")
 			}
 			if resolvedPipeline != nil {