[TEP-0091] return VerificationResult from GetPipeline

This commits updates GetPipeline to return
VerificationResult. VerificationResult is returned by VerifyResource in
remote resolution.

Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -406,10 +406,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
 		message := fmt.Sprintf("PipelineRun %s/%s awaiting remote resource", pr.Namespace, pr.Name)
 		pr.Status.MarkRunning(ReasonResolvingPipelineRef, message)
 		return nil
-	case errors.Is(err, trustedresources.ErrResourceVerificationFailed):
-		message := fmt.Sprintf("PipelineRun %s/%s referred pipeline failed signature verification", pr.Namespace, pr.Name)
-		pr.Status.MarkFailed(ReasonResourceVerificationFailed, message)
-		return controller.NewPermanentError(err)
 	case err != nil:
 		logger.Errorf("Failed to determine Pipeline spec to use for pipelinerun %s: %v", pr.Name, err)
 		pr.Status.MarkFailed(ReasonCouldntGetPipeline,
@@ -423,6 +419,12 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
 		}
 	}
 
+	if pipelineMeta.VerificationResult != nil && pipelineMeta.VerificationResult.VerificationResultType == trustedresources.VerificationError {
+		err := fmt.Errorf("PipelineRun %s/%s referred pipeline failed signature verification: %w", pr.Namespace, pr.Name, pipelineMeta.VerificationResult.Err)
+		pr.Status.MarkFailed(ReasonResourceVerificationFailed, err.Error())
+		return controller.NewPermanentError(err)
+	}
+
 	d, err := dag.Build(v1beta1.PipelineTaskList(pipelineSpec.Tasks), v1beta1.PipelineTaskList(pipelineSpec.Tasks).Deps())
 	if err != nil {
 		// This Run has failed, so we need to mark it as failed and stop reconciling it

pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go

@@ -23,36 +23,40 @@ import (
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	resolutionutil "github.com/tektoncd/pipeline/pkg/internal/resolution"
+	"github.com/tektoncd/pipeline/pkg/trustedresources"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // GetPipeline is a function used to retrieve Pipelines.
-type GetPipeline func(context.Context, string) (*v1beta1.Pipeline, *v1beta1.RefSource, error)
+// VerificationResult is the result from trusted resources if the feature is enabled.
+type GetPipeline func(context.Context, string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error)
 
 // GetPipelineData will retrieve the Pipeline metadata and Spec associated with the
 // provided PipelineRun. This can come from a reference Pipeline or from the PipelineRun's
 // metadata and embedded PipelineSpec.
 func GetPipelineData(ctx context.Context, pipelineRun *v1beta1.PipelineRun, getPipeline GetPipeline) (*resolutionutil.ResolvedObjectMeta, *v1beta1.PipelineSpec, error) {
 	pipelineMeta := metav1.ObjectMeta{}
 	var refSource *v1beta1.RefSource
+	var verificationResult *trustedresources.VerificationResult
 	pipelineSpec := v1beta1.PipelineSpec{}
 	switch {
 	case pipelineRun.Spec.PipelineRef != nil && pipelineRun.Spec.PipelineRef.Name != "":
 		// Get related pipeline for pipelinerun
-		p, source, err := getPipeline(ctx, pipelineRun.Spec.PipelineRef.Name)
+		p, source, vr, err := getPipeline(ctx, pipelineRun.Spec.PipelineRef.Name)
 		if err != nil {
 			return nil, nil, fmt.Errorf("error when listing pipelines for pipelineRun %s: %w", pipelineRun.Name, err)
 		}
 		pipelineMeta = p.PipelineMetadata()
 		pipelineSpec = p.PipelineSpec()
 		refSource = source
+		verificationResult = vr
 	case pipelineRun.Spec.PipelineSpec != nil:
 		pipelineMeta = pipelineRun.ObjectMeta
 		pipelineSpec = *pipelineRun.Spec.PipelineSpec
 		// TODO: if we want to set RefSource for embedded pipeline, set it here.
 		// https://github.com/tektoncd/pipeline/issues/5522
 	case pipelineRun.Spec.PipelineRef != nil && pipelineRun.Spec.PipelineRef.Resolver != "":
-		pipeline, source, err := getPipeline(ctx, "")
+		pipeline, source, vr, err := getPipeline(ctx, "")
 		switch {
 		case err != nil:
 			return nil, nil, err
@@ -63,13 +67,15 @@ func GetPipelineData(ctx context.Context, pipelineRun *v1beta1.PipelineRun, getP
 			pipelineSpec = pipeline.PipelineSpec()
 		}
 		refSource = source
+		verificationResult = vr
 	default:
 		return nil, nil, fmt.Errorf("pipelineRun %s not providing PipelineRef or PipelineSpec", pipelineRun.Name)
 	}
 
 	pipelineSpec.SetDefaults(ctx)
 	return &resolutionutil.ResolvedObjectMeta{
-		ObjectMeta: &pipelineMeta,
-		RefSource:  refSource,
+		ObjectMeta:         &pipelineMeta,
+		RefSource:          refSource,
+		VerificationResult: verificationResult,
 	}, &pipelineSpec, nil
 }

pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go

@@ -25,6 +25,7 @@ import (
 	cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	pipelinespec "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/pipelinespec"
+	"github.com/tektoncd/pipeline/pkg/trustedresources"
 	"github.com/tektoncd/pipeline/test/diff"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -53,8 +54,8 @@ func TestGetPipelineSpec_Ref(t *testing.T) {
 			},
 		},
 	}
-	gt := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
-		return pipeline, nil, nil
+	gt := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
+		return pipeline, nil, nil, nil
 	}
 	resolvedObjectMeta, pipelineSpec, err := pipelinespec.GetPipelineData(context.Background(), pr, gt)
 
@@ -91,8 +92,8 @@ func TestGetPipelineSpec_Embedded(t *testing.T) {
 			},
 		},
 	}
-	gt := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
-		return nil, nil, errors.New("shouldn't be called")
+	gt := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
+		return nil, nil, nil, errors.New("shouldn't be called")
 	}
 	resolvedObjectMeta, pipelineSpec, err := pipelinespec.GetPipelineData(context.Background(), pr, gt)
 
@@ -119,8 +120,8 @@ func TestGetPipelineSpec_Invalid(t *testing.T) {
 			Name: "mypipelinerun",
 		},
 	}
-	gt := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
-		return nil, nil, errors.New("shouldn't be called")
+	gt := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
+		return nil, nil, nil, errors.New("shouldn't be called")
 	}
 	_, _, err := pipelinespec.GetPipelineData(context.Background(), tr, gt)
 	if err == nil {
@@ -217,11 +218,11 @@ func TestGetPipelineData_ResolutionSuccess(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			ctx := cfgtesting.SetDefaults(context.Background(), t, tc.defaults)
-			getPipeline := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
+			getPipeline := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
 				return &v1beta1.Pipeline{
 					ObjectMeta: *tc.sourceMeta.DeepCopy(),
 					Spec:       *tc.sourceSpec.DeepCopy(),
-				}, tc.refSource.DeepCopy(), nil
+				}, tc.refSource.DeepCopy(), nil, nil
 			}
 
 			resolvedObjectMeta, resolvedPipelineSpec, err := pipelinespec.GetPipelineData(ctx, tc.pr, getPipeline)
@@ -253,8 +254,8 @@ func TestGetPipelineSpec_Error(t *testing.T) {
 			},
 		},
 	}
-	gt := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
-		return nil, nil, errors.New("something went wrong")
+	gt := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
+		return nil, nil, nil, errors.New("something went wrong")
 	}
 	_, _, err := pipelinespec.GetPipelineData(context.Background(), tr, gt)
 	if err == nil {
@@ -275,8 +276,8 @@ func TestGetPipelineData_ResolutionError(t *testing.T) {
 			},
 		},
 	}
-	getPipeline := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
-		return nil, nil, errors.New("something went wrong")
+	getPipeline := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
+		return nil, nil, nil, errors.New("something went wrong")
 	}
 	ctx := context.Background()
 	_, _, err := pipelinespec.GetPipelineData(ctx, pr, getPipeline)
@@ -298,8 +299,8 @@ func TestGetPipelineData_ResolvedNilPipeline(t *testing.T) {
 			},
 		},
 	}
-	getPipeline := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
-		return nil, nil, nil
+	getPipeline := func(ctx context.Context, n string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
+		return nil, nil, nil, nil
 	}
 	ctx := context.Background()
 	_, _, err := pipelinespec.GetPipelineData(ctx, pr, getPipeline)

pkg/reconciler/pipelinerun/resources/pipelineref.go

@@ -49,7 +49,7 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien
 	// if the spec is already in the status, do not try to fetch it again, just use it as source of truth.
 	// Same for the RefSource field in the Status.Provenance.
 	if pipelineRun.Status.PipelineSpec != nil {
-		return func(_ context.Context, name string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
+		return func(_ context.Context, name string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
 			var refSource *v1beta1.RefSource
 			if pipelineRun.Status.Provenance != nil {
 				refSource = pipelineRun.Status.Provenance.RefSource
@@ -60,28 +60,28 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien
 					Namespace: namespace,
 				},
 				Spec: *pipelineRun.Status.PipelineSpec,
-			}, refSource, nil
+			}, refSource, nil, nil
 		}
 	}
 
 	switch {
 	case cfg.FeatureFlags.EnableTektonOCIBundles && pr != nil && pr.Bundle != "":
 		// Return an inline function that implements GetTask by calling Resolver.Get with the specified task type and
 		// casting it to a PipelineObject.
-		return func(ctx context.Context, name string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
+		return func(ctx context.Context, name string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
 			// If there is a bundle url at all, construct an OCI resolver to fetch the pipeline.
 			kc, err := k8schain.New(ctx, k8s, k8schain.Options{
 				Namespace:          namespace,
 				ServiceAccountName: pipelineRun.Spec.ServiceAccountName,
 			})
 			if err != nil {
-				return nil, nil, fmt.Errorf("failed to get keychain: %w", err)
+				return nil, nil, nil, fmt.Errorf("failed to get keychain: %w", err)
 			}
 			resolver := oci.NewResolver(pr.Bundle, kc)
 			return resolvePipeline(ctx, resolver, name, k8s, verificationPolicies)
 		}
 	case pr != nil && pr.Resolver != "" && requester != nil:
-		return func(ctx context.Context, name string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
+		return func(ctx context.Context, name string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
 			stringReplacements, arrayReplacements, objectReplacements := paramsFromPipelineRun(ctx, pipelineRun)
 			for k, v := range GetContextReplacements("", pipelineRun) {
 				stringReplacements[k] = v
@@ -110,61 +110,57 @@ type LocalPipelineRefResolver struct {
 // return an error if it can't find an appropriate Pipeline for any reason.
 // TODO: if we want to set RefSource for in-cluster pipeline, set it here.
 // https://github.com/tektoncd/pipeline/issues/5522
-func (l *LocalPipelineRefResolver) GetPipeline(ctx context.Context, name string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
+func (l *LocalPipelineRefResolver) GetPipeline(ctx context.Context, name string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
 	// If we are going to resolve this reference locally, we need a namespace scope.
 	if l.Namespace == "" {
-		return nil, nil, fmt.Errorf("Must specify namespace to resolve reference to pipeline %s", name)
+		return nil, nil, nil, fmt.Errorf("Must specify namespace to resolve reference to pipeline %s", name)
 	}
 
 	pipeline, err := l.Tektonclient.TektonV1beta1().Pipelines(l.Namespace).Get(ctx, name, metav1.GetOptions{})
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
-	return pipeline, nil, nil
+	return pipeline, nil, nil, nil
 }
 
 // resolvePipeline accepts an impl of remote.Resolver and attempts to
 // fetch a pipeline with given name and verify the v1beta1 pipeline if trusted resources is enabled.
-// An error is returned if the resolution doesn't work, the verification fails
+// An error is returned if the remoteresource doesn't work
+// A VerificationResult is returned if trusted resources is enabled, VerificationResult contains the result type and err.
 // or the returned data isn't a valid *v1beta1.Pipeline.
-func resolvePipeline(ctx context.Context, resolver remote.Resolver, name string, k8s kubernetes.Interface, verificationPolicies []*v1alpha1.VerificationPolicy) (*v1beta1.Pipeline, *v1beta1.RefSource, error) {
+func resolvePipeline(ctx context.Context, resolver remote.Resolver, name string, k8s kubernetes.Interface, verificationPolicies []*v1alpha1.VerificationPolicy) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error) {
 	obj, refSource, err := resolver.Get(ctx, "pipeline", name)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
-	pipelineObj, err := readRuntimeObjectAsPipeline(ctx, obj, k8s, refSource, verificationPolicies)
+	pipelineObj, vr, err := readRuntimeObjectAsPipeline(ctx, obj, k8s, refSource, verificationPolicies)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
-	return pipelineObj, refSource, nil
+	return pipelineObj, refSource, vr, nil
 }
 
 // readRuntimeObjectAsPipeline tries to convert a generic runtime.Object
 // into a *v1beta1.Pipeline type so that its meta and spec fields
 // can be read. v1 object will be converted to v1beta1 and returned.
 // v1beta1 Pipeline will be verified if trusted resources is enabled
+// A VerificationResult is returned if trusted resources is enabled, VerificationResult contains the result type and err.
 // An error is returned if the given object is not a
-// PipelineObject, trusted resources verification fails or if there is an error validating or upgrading an
+// PipelineObject or if there is an error validating or upgrading an
 // older PipelineObject into its v1beta1 equivalent.
 // TODO(#5541): convert v1beta1 obj to v1 once we use v1 as the stored version
-func readRuntimeObjectAsPipeline(ctx context.Context, obj runtime.Object, k8s kubernetes.Interface, refSource *v1beta1.RefSource, verificationPolicies []*v1alpha1.VerificationPolicy) (*v1beta1.Pipeline, error) {
+func readRuntimeObjectAsPipeline(ctx context.Context, obj runtime.Object, k8s kubernetes.Interface, refSource *v1beta1.RefSource, verificationPolicies []*v1alpha1.VerificationPolicy) (*v1beta1.Pipeline, *trustedresources.VerificationResult, error) {
 	switch obj := obj.(type) {
 	case *v1beta1.Pipeline:
 		// Verify the Pipeline once we fetch from the remote resolution, mutating, validation and conversion of the pipeline should happen after the verification, since signatures are based on the remote pipeline contents
 		vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies)
-		if vr.VerificationResultType == trustedresources.VerificationError {
-			if vr.Err != nil {
-				return nil, fmt.Errorf("Pipeline verification failed for object %s: %w", obj.GetName(), vr.Err)
-			}
-			return nil, fmt.Errorf("Pipeline verification failed for object %s", obj.GetName())
-		}
-		return obj, nil
+		return obj, &vr, 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 Pipeline %s: %w", obj.GetName(), err)
+			return nil, nil, fmt.Errorf("invalid Pipeline %s: %w", obj.GetName(), err)
 		}
 		t := &v1beta1.Pipeline{
 			TypeMeta: metav1.TypeMeta{
@@ -173,10 +169,10 @@ func readRuntimeObjectAsPipeline(ctx context.Context, obj runtime.Object, k8s ku
 			},
 		}
 		if err := t.ConvertFrom(ctx, obj); err != nil {
-			return nil, fmt.Errorf("failed to convert obj %s into Pipeline", obj.GetObjectKind().GroupVersionKind().String())
+			return nil, nil, fmt.Errorf("failed to convert obj %s into Pipeline", obj.GetObjectKind().GroupVersionKind().String())
 		}
-		return t, nil
+		return t, nil, nil
 	}
 
-	return nil, errors.New("resource is not a pipeline")
+	return nil, nil, errors.New("resource is not a pipeline")
 }