From 2ee1ac78344357b3b5923c115202496aef13b10a Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Mon, 29 May 2023 17:29:37 +0000 Subject: [PATCH] [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 | 10 +- .../pipelinerun/pipelinespec/pipelinespec.go | 16 +- .../pipelinespec/pipelinespec_test.go | 29 ++-- .../pipelinerun/resources/pipelineref.go | 52 +++--- .../pipelinerun/resources/pipelineref_test.go | 152 ++++++++++-------- 5 files changed, 139 insertions(+), 120 deletions(-) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 9511a065f79..5959c989c9d 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/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 diff --git a/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go b/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go index 370be55470c..1061cfa901c 100644 --- a/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go +++ b/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go @@ -23,11 +23,13 @@ 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 @@ -35,24 +37,26 @@ type GetPipeline func(context.Context, string) (*v1beta1.Pipeline, *v1beta1.RefS 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 } diff --git a/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go b/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go index d086371b028..9e11a47cf60 100644 --- a/pkg/reconciler/pipelinerun/pipelinespec/pipelinespec_test.go +++ b/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) diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go index 94ca6cac36b..48a62ea555f 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref.go +++ b/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,7 +60,7 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien Namespace: namespace, }, Spec: *pipelineRun.Status.PipelineSpec, - }, refSource, nil + }, refSource, nil, nil } } @@ -68,20 +68,20 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien 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") } diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go index 363b816f37e..f0e2f6e30a4 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go @@ -107,7 +107,7 @@ func TestLocalPipelineRef(t *testing.T) { Tektonclient: tektonclient, } - resolvedPipeline, resolvedRefSource, err := lc.GetPipeline(ctx, tc.ref.Name) + resolvedPipeline, resolvedRefSource, _, err := lc.GetPipeline(ctx, tc.ref.Name) if tc.wantErr && err == nil { t.Fatal("Expected error but found nil instead") } else if !tc.wantErr && err != nil { @@ -212,7 +212,7 @@ func TestGetPipelineFunc(t *testing.T) { t.Fatalf("failed to get pipeline fn: %s", err.Error()) } - pipeline, refSource, err := fn(ctx, tc.ref.Name) + pipeline, refSource, _, err := fn(ctx, tc.ref.Name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } @@ -273,7 +273,7 @@ func TestGetPipelineFuncSpecAlreadyFetched(t *testing.T) { } fn := resources.GetPipelineFunc(ctx, kubeclient, tektonclient, nil, pipelineRun, nil /*VerificationPolicies*/) - actualPipeline, actualRefSource, err := fn(ctx, name) + actualPipeline, actualRefSource, _, err := fn(ctx, name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } @@ -344,7 +344,7 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) { }, }, nil /*VerificationPolicies*/) - resolvedPipeline, resolvedRefSource, err := fn(ctx, pipelineRef.Name) + resolvedPipeline, resolvedRefSource, _, err := fn(ctx, pipelineRef.Name) if tc.wantErr { if err == nil { t.Errorf("expected an error when calling pipelinefunc but got none") @@ -415,7 +415,7 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { }, }, nil /*VerificationPolicies*/) - resolvedPipeline, resolvedRefSource, err := fn(ctx, pipelineRef.Name) + resolvedPipeline, resolvedRefSource, _, err := fn(ctx, pipelineRef.Name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) } @@ -456,7 +456,7 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { }, }, nil /*VerificationPolicies*/) - _, _, err = fnNotMatching(ctx, pipelineRefNotMatching.Name) + _, _, _, err = fnNotMatching(ctx, pipelineRefNotMatching.Name) if err == nil { t.Fatal("expected error for non-matching params, did not get one") } @@ -480,15 +480,12 @@ func TestGetPipelineFunc_RemoteResolutionInvalidData(t *testing.T) { ServiceAccountName: "default", }, }, nil /*VerificationPolicies*/) - if _, _, err := fn(ctx, pipelineRef.Name); err == nil { + if _, _, _, err := fn(ctx, pipelineRef.Name); err == nil { t.Fatalf("expected error due to invalid pipeline data but saw none") } } func TestGetPipelineFunc_VerifyNoError(t *testing.T) { - // This test case tests the success cases of trusted-resources-verification-no-match-policy when it is set to - // fail: passed matching policy verification - // warn and ignore: no matching policies. ctx := context.Background() signer, _, k8sclient, vps := test.SetupVerificationPolicies(t) tektonclient := fake.NewSimpleClientset() @@ -568,61 +565,68 @@ func TestGetPipelineFunc_VerifyNoError(t *testing.T) { requesterUnsignedMatched := test.NewRequester(resolvedUnsignedMatched, nil) testcases := []struct { - name string - requester *test.Requester - verificationNoMatchPolicy string - pipelinerun v1beta1.PipelineRun - policies []*v1alpha1.VerificationPolicy - expected runtime.Object - expectedRefSource *v1beta1.RefSource + name string + requester *test.Requester + verificationNoMatchPolicy string + pipelinerun v1beta1.PipelineRun + policies []*v1alpha1.VerificationPolicy + expected runtime.Object + expectedRefSource *v1beta1.RefSource + expectedVerificationResult *trustedresources.VerificationResult }{{ - name: "signed pipeline with matching policy pass verification with enforce no match policy", - requester: requesterMatched, - verificationNoMatchPolicy: config.FailNoMatchPolicy, - pipelinerun: pr, - policies: vps, - expected: signedPipeline, - expectedRefSource: matchPolicyRefSource, + name: "signed pipeline with matching policy pass verification with enforce no match policy", + requester: requesterMatched, + verificationNoMatchPolicy: config.FailNoMatchPolicy, + pipelinerun: pr, + policies: vps, + expected: signedPipeline, + expectedRefSource: matchPolicyRefSource, + expectedVerificationResult: &trustedresources.VerificationResult{VerificationResultType: trustedresources.VerificationPass}, }, { - name: "signed pipeline with matching policy pass verification with warn no match policy", - requester: requesterMatched, - verificationNoMatchPolicy: config.WarnNoMatchPolicy, - pipelinerun: pr, - policies: vps, - expected: signedPipeline, - expectedRefSource: matchPolicyRefSource, + name: "signed pipeline with matching policy pass verification with warn no match policy", + requester: requesterMatched, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + pipelinerun: pr, + policies: vps, + expected: signedPipeline, + expectedRefSource: matchPolicyRefSource, + expectedVerificationResult: &trustedresources.VerificationResult{VerificationResultType: trustedresources.VerificationPass}, }, { - name: "signed pipeline with matching policy pass verification with ignore no match policy", - requester: requesterMatched, - verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, - pipelinerun: pr, - policies: vps, - expected: signedPipeline, - expectedRefSource: matchPolicyRefSource, + name: "signed pipeline with matching policy pass verification with ignore no match policy", + requester: requesterMatched, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + pipelinerun: pr, + policies: vps, + expected: signedPipeline, + expectedRefSource: matchPolicyRefSource, + expectedVerificationResult: &trustedresources.VerificationResult{VerificationResultType: trustedresources.VerificationPass}, }, { - name: "warn unsigned pipeline without matching policies", - requester: requesterUnmatched, - verificationNoMatchPolicy: config.WarnNoMatchPolicy, - pipelinerun: pr, - policies: vps, - expected: unsignedPipeline, - expectedRefSource: noMatchPolicyRefSource, + name: "warn unsigned pipeline without matching policies", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.WarnNoMatchPolicy, + pipelinerun: pr, + policies: vps, + expected: unsignedPipeline, + expectedRefSource: noMatchPolicyRefSource, + expectedVerificationResult: &trustedresources.VerificationResult{VerificationResultType: trustedresources.VerificationWarn, Err: trustedresources.ErrNoMatchedPolicies}, }, { - name: "unsigned pipeline fails warn mode policies doesn't return error", - requester: requesterUnsignedMatched, - verificationNoMatchPolicy: config.FailNoMatchPolicy, - pipelinerun: pr, - policies: vps, - expected: unsignedPipeline, - expectedRefSource: warnPolicyRefSource, + name: "unsigned pipeline fails warn mode policies doesn't return error", + requester: requesterUnsignedMatched, + verificationNoMatchPolicy: config.FailNoMatchPolicy, + pipelinerun: pr, + policies: vps, + expected: unsignedPipeline, + expectedRefSource: warnPolicyRefSource, + expectedVerificationResult: &trustedresources.VerificationResult{VerificationResultType: trustedresources.VerificationWarn, Err: trustedresources.ErrResourceVerificationFailed}, }, { - name: "ignore unsigned pipeline without matching policies", - requester: requesterUnmatched, - verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, - pipelinerun: pr, - policies: vps, - expected: unsignedPipeline, - expectedRefSource: noMatchPolicyRefSource, + name: "ignore unsigned pipeline without matching policies", + requester: requesterUnmatched, + verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, + pipelinerun: pr, + policies: vps, + expected: unsignedPipeline, + expectedRefSource: noMatchPolicyRefSource, + expectedVerificationResult: &trustedresources.VerificationResult{VerificationResultType: trustedresources.VerificationSkip}, }, { name: "signed pipeline in status no need to verify", requester: requesterMatched, @@ -636,7 +640,8 @@ func TestGetPipelineFunc_VerifyNoError(t *testing.T) { }, Spec: signedPipeline.Spec, }, - expectedRefSource: noMatchPolicyRefSource, + expectedRefSource: noMatchPolicyRefSource, + expectedVerificationResult: nil, }, } for _, tc := range testcases { @@ -644,7 +649,7 @@ func TestGetPipelineFunc_VerifyNoError(t *testing.T) { ctx = test.SetupTrustedResourceConfig(ctx, tc.verificationNoMatchPolicy) fn := resources.GetPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.pipelinerun, tc.policies) - resolvedPipeline, source, err := fn(ctx, pipelineRef.Name) + resolvedPipeline, source, vr, err := fn(ctx, pipelineRef.Name) if err != nil { t.Fatalf("Received unexpected error ( %#v )", err) } @@ -654,6 +659,15 @@ func TestGetPipelineFunc_VerifyNoError(t *testing.T) { if d := cmp.Diff(tc.expectedRefSource, source); d != "" { t.Errorf("configSources did not match: %s", diff.PrintWantGot(d)) } + if tc.expectedVerificationResult == nil { + if vr != nil { + t.Errorf("VerificationResult did not match: want %v, got %v", tc.expectedVerificationResult, vr) + } + return + } + if tc.expectedVerificationResult.VerificationResultType != vr.VerificationResultType && errors.Is(vr.Err, tc.expectedVerificationResult.Err) { + t.Errorf("VerificationResult did not match: want %v, got %v", tc.expectedVerificationResult, vr) + } }) } } @@ -764,15 +778,15 @@ func TestGetPipelineFunc_VerifyError(t *testing.T) { } fn := resources.GetPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, pr, vps) - resolvedPipeline, resolvedRefSource, err := fn(ctx, pipelineRef.Name) - if !errors.Is(err, tc.expectedErr) { - t.Errorf("GetPipelineFunc got %v, want %v", err, tc.expectedErr) + _, _, vr, err := fn(ctx, pipelineRef.Name) + if err != nil { + t.Errorf("want err nil but got %v", err) } - if d := cmp.Diff(resolvedPipeline, (*v1beta1.Pipeline)(nil)); d != "" { - t.Errorf("resolvedPipeline did not match: %s", diff.PrintWantGot(d)) + if !errors.Is(vr.Err, tc.expectedErr) { + t.Errorf("GetPipelineFunc got %v, want %v", err, tc.expectedErr) } - if resolvedRefSource != nil { - t.Errorf("got %v, but expected refSource is nil", resolvedRefSource) + if vr.VerificationResultType != trustedresources.VerificationError { + t.Errorf("VerificationResultType mismatch, want %d got %d", trustedresources.VerificationError, vr.VerificationResultType) } }) } @@ -853,7 +867,7 @@ func TestGetPipelineFunc_GetFuncError(t *testing.T) { fn := resources.GetPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.pipelinerun, vps) - _, _, err = fn(ctx, tc.pipelinerun.Spec.PipelineRef.Name) + _, _, _, err = fn(ctx, tc.pipelinerun.Spec.PipelineRef.Name) if d := cmp.Diff(err.Error(), tc.expectedErr.Error()); d != "" { t.Errorf("GetPipelineFunc got %v, want %v", err, tc.expectedErr) }