From 7ddb9a4277dd1de0c4ab5c0d153bcae542204aad Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Thu, 4 May 2023 21:00:34 +0000 Subject: [PATCH] move trusted resources verification after we resolve the remote resources This PR moves the trusted resources verification to readRuntimeObjectAsTask and readRuntimeObjectAsPipline, the reasons we need this change include 1) unblock the work for v1, since v1 will mutate, validate and convert the resources, the mutation will break trusted resources verification thus we need to verify right after we resolve the remote resources. 2) Prepare the support for verifying different api versions. This commit also makes it clear that currently we only support verification for remote resources. Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com --- docs/trusted-resources.md | 4 +- pkg/reconciler/pipelinerun/pipelinerun.go | 4 +- .../pipelinerun/pipelinerun_test.go | 254 +++++++++++++----- .../pipelinerun/resources/pipelineref.go | 55 ++-- .../pipelinerun/resources/pipelineref_test.go | 44 ++- pkg/reconciler/taskrun/resources/taskref.go | 54 ++-- .../taskrun/resources/taskref_test.go | 30 +-- pkg/reconciler/taskrun/taskrun_test.go | 95 ++++--- pkg/trustedresources/verify.go | 16 +- pkg/trustedresources/verify_test.go | 48 ++-- test/trusted_resources_test.go | 46 +++- 11 files changed, 396 insertions(+), 254 deletions(-) diff --git a/docs/trusted-resources.md b/docs/trusted-resources.md index f36d11b4cc7..c143d844de8 100644 --- a/docs/trusted-resources.md +++ b/docs/trusted-resources.md @@ -14,7 +14,9 @@ weight: 312 ## Overview -Trusted Resources is a feature which can be used to sign Tekton Resources and verify them. Details of design can be found at [TEP--0091](https://github.com/tektoncd/community/blob/main/teps/0091-trusted-resources.md). This feature is under `alpha` version and support `v1beta1` version of `Task` and `Pipeline`. +Trusted Resources is a feature which can be used to sign Tekton Resources and verify them. Details of design can be found at [TEP--0091](https://github.com/tektoncd/community/blob/main/teps/0091-trusted-resources.md). This feature is under `alpha` version and support `v1beta1` version of `Task` and `Pipeline`. + +**Note**: trusted resources support verification of resources from OCI bundle or remote resolution, to use [cluster resolver](./cluster-resolver.md) make sure the resources all default values set before applied to cluster, otherwise the verification will fail due to the mutating webhook. Verification failure will mark corresponding taskrun/pipelinerun as Failed status and stop the execution. diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 39f89587e43..96fe7b966b7 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -220,7 +220,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) if err != nil { return fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %w", pr.Namespace, err) } - getPipelineFunc := resources.GetVerifiedPipelineFunc(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, pr, vp) + getPipelineFunc := resources.GetPipelineFunc(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, pr, vp) if pr.IsDone() { pr.SetDefaults(ctx) @@ -331,7 +331,7 @@ func (c *Reconciler) resolvePipelineState( if err != nil { return nil, fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %w", pr.Namespace, err) } - fn := tresources.GetVerifiedTaskFunc(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, pr, task.TaskRef, trName, pr.Namespace, pr.Spec.ServiceAccountName, vp) + fn := tresources.GetTaskFunc(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, pr, task.TaskRef, trName, pr.Namespace, pr.Spec.ServiceAccountName, vp) getRunObjectFunc := func(name string) (v1beta1.RunObject, error) { r, err := c.customRunLister.CustomRuns(pr.Namespace).Get(name) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index b2ac6502d8c..e610171aeb9 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -11351,26 +11351,14 @@ func TestReconcile_verifyResolvedPipeline_Success(t *testing.T) { ctx := context.Background() ctx, cancel := context.WithCancel(ctx) defer cancel() + // Set up a fake registry to push an image to. + s := httptest.NewServer(registry.New()) + defer s.Close() + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } - prs := parse.MustParseV1beta1PipelineRun(t, ` -metadata: - name: test-pipelinerun - namespace: foo - selfLink: /pipeline/1234 -spec: - pipelineRef: - name: test-pipeline -`) - ps := parse.MustParseV1beta1Pipeline(t, ` -metadata: - name: test-pipeline - namespace: foo -spec: - tasks: - - name: test-1 - taskRef: - name: test-task -`) ts := parse.MustParseV1beta1Task(t, ` metadata: name: test-task @@ -11385,23 +11373,64 @@ spec: value: bar `) - signer, _, vps := test.SetupMatchAllVerificationPolicies(t, prs.Namespace) + signer, _, vps := test.SetupMatchAllVerificationPolicies(t, ts.Namespace) signedTask, err := test.GetSignedTask(ts, signer, "test-task") if err != nil { t.Fatal("fail to sign task", err) } + ref, err := test.CreateImage(u.Host+"/"+signedTask.Name, signedTask) + if err != nil { + t.Fatalf("failed to upload image with simple task: %s", err.Error()) + } + + ps := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: test-pipeline + namespace: foo +spec: + tasks: + - name: test-1 + taskRef: + bundle: %s + name: test-task +`, ref)) + signedPipeline, err := test.GetSignedPipeline(ps, signer, "test-pipeline") if err != nil { t.Fatal("fail to sign pipeline", err) } + ref, err = test.CreateImage(u.Host+"/"+signedPipeline.Name, signedPipeline) + if err != nil { + t.Fatalf("failed to upload image with simple task: %s", err.Error()) + } + prs := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` +metadata: + name: test-pipelinerun + namespace: foo + selfLink: /pipeline/1234 +spec: + pipelineRef: + bundle: %s + name: test-pipeline +`, ref)) + + cms := []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "trusted-resources-verification-no-match-policy": config.FailNoMatchPolicy, + "enable-tekton-oci-bundles": "true", + }, + }, + } d := test.Data{ PipelineRuns: []*v1beta1.PipelineRun{prs}, - Pipelines: []*v1beta1.Pipeline{signedPipeline}, - Tasks: []*v1beta1.Task{signedTask}, VerificationPolicies: vps, + ConfigMaps: cms, } prt := newPipelineRunTest(t, d) + createServiceAccount(t, prt.TestAssets, prs.Spec.ServiceAccountName, prs.Namespace) defer prt.Cancel() reconciledRun, _ := prt.reconcileRun("foo", "test-pipelinerun", []string{}, false) @@ -11415,25 +11444,15 @@ func TestReconcile_verifyResolvedPipeline_Error(t *testing.T) { ctx, cancel := context.WithCancel(ctx) defer cancel() - prs := parse.MustParseV1beta1PipelineRun(t, ` -metadata: - name: test-pipelinerun - namespace: foo - selfLink: /pipeline/1234 -spec: - pipelineRef: - name: test-pipeline -`) - ps := parse.MustParseV1beta1Pipeline(t, ` -metadata: - name: test-pipeline - namespace: foo -spec: - tasks: - - name: test-1 - taskRef: - name: test-task -`) + // Set up a fake registry to push an image to. + s := httptest.NewServer(registry.New()) + defer s.Close() + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + + // Case1: unsigned Pipeline refers to unsigned Task ts := parse.MustParseV1beta1Task(t, ` metadata: name: test-task @@ -11447,69 +11466,147 @@ spec: - name: foo value: bar `) + // Upload the simple task to the registry + unsignedTaskref, err := test.CreateImage(u.Host+"/"+ts.Name, ts) + if err != nil { + t.Fatalf("failed to upload image with simple task: %s", err.Error()) + } + unsignedPipeline := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: test-pipeline + namespace: foo +spec: + tasks: + - name: test-1 + taskRef: + bundle: %s + name: test-task +`, unsignedTaskref)) - signer, _, vps := test.SetupMatchAllVerificationPolicies(t, prs.Namespace) + // Case2: signed Pipeline refers to unsigned Task + signer, _, vps := test.SetupMatchAllVerificationPolicies(t, ts.Namespace) + signedPipelineWithUnsignedTask, err := test.GetSignedPipeline(unsignedPipeline, signer, "test-pipeline") + if err != nil { + t.Fatal("fail to sign pipeline", err) + } + + // Case3: signed Pipeline refers to modified Task signedTask, err := test.GetSignedTask(ts, signer, "test-task") if err != nil { t.Fatal("fail to sign task", err) } - signedPipeline, err := test.GetSignedPipeline(ps, signer, "test-pipeline") + modifiedTask := signedTask.DeepCopy() + if modifiedTask.Annotations == nil { + modifiedTask.Annotations = make(map[string]string) + } + modifiedTask.Annotations["random"] = "attack" + // Upload the simple task to the registry + modifiedTaskref, err := test.CreateImage(u.Host+"/"+modifiedTask.Name, modifiedTask) + if err != nil { + t.Fatalf("failed to upload image with simple task: %s", err.Error()) + } + ps := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: test-pipeline + namespace: foo +spec: + tasks: + - name: test-1 + taskRef: + bundle: %s + name: test-task +`, modifiedTaskref)) + signedPipelineWithModifiedTask, err := test.GetSignedPipeline(ps, signer, "test-pipeline") if err != nil { t.Fatal("fail to sign pipeline", err) } - tamperedTask := signedTask.DeepCopy() - if tamperedTask.Annotations == nil { - tamperedTask.Annotations = make(map[string]string) + // Case4: modified Pipeline refers to signed Task + // Upload the simple task to the registry + signedTaskref, err := test.CreateImage(u.Host+"/"+signedTask.Name, signedTask) + if err != nil { + t.Fatalf("failed to upload image with simple task: %s", err.Error()) + } + ps = parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: test-pipeline + namespace: foo +spec: + tasks: + - name: test-1 + taskRef: + bundle: %s + name: test-task +`, signedTaskref)) + signedPipeline, err := test.GetSignedPipeline(ps, signer, "test-pipeline") + if err != nil { + t.Fatal("fail to sign pipeline", err) } - tamperedTask.Annotations["random"] = "attack" + modifiedPipeline := signedPipeline.DeepCopy() + if modifiedPipeline.Annotations == nil { + modifiedPipeline.Annotations = make(map[string]string) + } + modifiedPipeline.Annotations["random"] = "attack" - tamperedPipeline := signedPipeline.DeepCopy() - if tamperedPipeline.Annotations == nil { - tamperedPipeline.Annotations = make(map[string]string) + cms := []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "trusted-resources-verification-no-match-policy": config.FailNoMatchPolicy, + "enable-tekton-oci-bundles": "true", + }, + }, } - tamperedPipeline.Annotations["random"] = "attack" testCases := []struct { name string pipelinerun []*v1beta1.PipelineRun - pipeline []*v1beta1.Pipeline - task []*v1beta1.Task + pipeline *v1beta1.Pipeline }{ { - name: "unsigned pipeline fails verification", - pipelinerun: []*v1beta1.PipelineRun{prs}, - pipeline: []*v1beta1.Pipeline{ps}, + name: "unsigned pipeline fails verification", + pipeline: unsignedPipeline, }, { - name: "signed pipeline with unsigned task fails verification", - pipelinerun: []*v1beta1.PipelineRun{prs}, - pipeline: []*v1beta1.Pipeline{signedPipeline}, - task: []*v1beta1.Task{ts}, + name: "signed pipeline with unsigned task fails verification", + pipeline: signedPipelineWithUnsignedTask, }, { - name: "signed pipeline with modified task fails verification", - pipelinerun: []*v1beta1.PipelineRun{prs}, - pipeline: []*v1beta1.Pipeline{signedPipeline}, - task: []*v1beta1.Task{tamperedTask}, + name: "signed pipeline with modified task fails verification", + pipeline: signedPipelineWithModifiedTask, }, { - name: "modified pipeline with signed task fails verification", - pipelinerun: []*v1beta1.PipelineRun{prs}, - pipeline: []*v1beta1.Pipeline{tamperedPipeline}, - task: []*v1beta1.Task{signedTask}, + name: "modified pipeline with signed task fails verification", + pipeline: modifiedPipeline, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + ref, err := test.CreateImage(u.Host+"/"+tc.pipeline.Name, tc.pipeline) + if err != nil { + t.Fatalf("failed to upload image with simple task: %s", err.Error()) + } + + prs := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` +metadata: + name: test-pipelinerun + namespace: foo + selfLink: /pipeline/1234 +spec: + pipelineRef: + name: test-pipeline + bundle: %s +`, ref)) + d := test.Data{ - PipelineRuns: tc.pipelinerun, - Pipelines: tc.pipeline, - Tasks: tc.task, + PipelineRuns: []*v1beta1.PipelineRun{prs}, + ConfigMaps: cms, VerificationPolicies: vps, } + prt := newPipelineRunTest(t, d) + createServiceAccount(t, prt.TestAssets, prs.Spec.ServiceAccountName, prs.Namespace) defer prt.Cancel() reconciledRun, _ := prt.reconcileRun("foo", "test-pipelinerun", []string{}, true) @@ -11519,6 +11616,21 @@ spec: } } +func createServiceAccount(t *testing.T, assets test.Assets, name string, namespace string) { + t.Helper() + if name == "" { + name = "default" + } + if _, err := assets.Clients.Kube.CoreV1().ServiceAccounts(namespace).Create(assets.Ctx, &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + }, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } +} + func TestReconcileForPipelineRunCreateRunFailed(t *testing.T) { // TestReconcileForPipelineRunCreateRunFailed runs "Reconcile" on a PipelineRun that has permanent error. // It verifies that reconcile is successful, no TaskRun is created, the PipelineTask is marked as CreateRunFailed, and the diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go index 7b73755e1d2..bbd86515d91 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref.go @@ -41,7 +41,8 @@ import ( // 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. -func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, pipelineRun *v1beta1.PipelineRun) rprp.GetPipeline { +// OCI bundle and remote resolution pipelines will be verified by trusted resources if the feature is enabled +func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, pipelineRun *v1beta1.PipelineRun, verificationPolicies []*v1alpha1.VerificationPolicy) rprp.GetPipeline { cfg := config.FromContextOrDefaults(ctx) pr := pipelineRun.Spec.PipelineRef namespace := pipelineRun.Namespace @@ -77,7 +78,7 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien return nil, nil, fmt.Errorf("failed to get keychain: %w", err) } resolver := oci.NewResolver(pr.Bundle, kc) - return resolvePipeline(ctx, resolver, name) + 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) { @@ -87,7 +88,7 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien } replacedParams := replaceParamValues(pr.Params, stringReplacements, arrayReplacements, objectReplacements) resolver := resolution.NewResolver(requester, pipelineRun, string(pr.Resolver), "", "", replacedParams) - return resolvePipeline(ctx, resolver, name) + return resolvePipeline(ctx, resolver, name, k8s, verificationPolicies) } default: // Even if there is no pipeline ref, we should try to return a local resolver. @@ -99,32 +100,6 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien } } -// GetVerifiedPipelineFunc is a wrapper of GetPipelineFunc and return the function to -// verify the pipeline if there are matching verification policies -func GetVerifiedPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, pipelineRun *v1beta1.PipelineRun, verificationpolicies []*v1alpha1.VerificationPolicy) rprp.GetPipeline { - get := GetPipelineFunc(ctx, k8s, tekton, requester, pipelineRun) - return func(context.Context, string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) { - p, s, err := get(ctx, pipelineRun.Spec.PipelineRef.Name) - if err != nil { - return nil, nil, fmt.Errorf("failed to get pipeline: %w", err) - } - // if the pipeline is in status, then it has been verified and no need to verify again - if pipelineRun.Status.PipelineSpec != nil { - return p, s, nil - } - var refSource string - if s != nil { - refSource = s.URI - } - if err := trustedresources.VerifyPipeline(ctx, p, k8s, refSource, verificationpolicies); err != nil { - // FixMe: the below %v should be %w (and the nolint pragma removed) - // but making that change causes e2e test failures. - return nil, nil, fmt.Errorf("GetVerifiedPipelineFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err) //nolint:errorlint - } - return p, s, nil - } -} - // LocalPipelineRefResolver uses the current cluster to resolve a pipeline reference. type LocalPipelineRefResolver struct { Namespace string @@ -149,17 +124,17 @@ func (l *LocalPipelineRefResolver) GetPipeline(ctx context.Context, name string) } // resolvePipeline accepts an impl of remote.Resolver and attempts to -// fetch a pipeline with given name. An error is returned if the -// resolution doesn't work or the returned data isn't a valid -// *v1beta1.Pipeline. -func resolvePipeline(ctx context.Context, resolver remote.Resolver, name string) (*v1beta1.Pipeline, *v1beta1.RefSource, error) { +// 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 +// 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) { obj, refSource, err := resolver.Get(ctx, "pipeline", name) if err != nil { return nil, nil, err } - pipelineObj, err := readRuntimeObjectAsPipeline(ctx, obj) + pipelineObj, err := readRuntimeObjectAsPipeline(ctx, obj, k8s, refSource, verificationPolicies) if err != nil { - return nil, nil, fmt.Errorf("failed to convert obj %s into Pipeline", obj.GetObjectKind().GroupVersionKind().String()) + return nil, nil, err } return pipelineObj, refSource, nil } @@ -167,15 +142,21 @@ func resolvePipeline(ctx context.Context, resolver remote.Resolver, name string) // 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 // An error is returned if the given object is not a -// PipelineObject or if there is an error validating or upgrading an +// PipelineObject, trusted resources verification fails 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) (*v1beta1.Pipeline, error) { +func readRuntimeObjectAsPipeline(ctx context.Context, obj runtime.Object, k8s kubernetes.Interface, refSource *v1beta1.RefSource, verificationPolicies []*v1alpha1.VerificationPolicy) (*v1beta1.Pipeline, error) { switch obj := obj.(type) { case *v1beta1.Pipeline: + err := trustedresources.VerifyPipeline(ctx, obj, k8s, refSource, verificationPolicies) + if err != nil { + return nil, err + } return obj, nil case *v1.Pipeline: + // TODO(#6356): Support V1 Task verification t := &v1beta1.Pipeline{ TypeMeta: metav1.TypeMeta{ Kind: "Pipeline", diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go index 2b7c025ce7d..8f31dbe1563 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go @@ -207,7 +207,7 @@ func TestGetPipelineFunc(t *testing.T) { PipelineRef: tc.ref, ServiceAccountName: "default", }, - }) + }, nil /*VerificationPolicies*/) if err != nil { t.Fatalf("failed to get pipeline fn: %s", err.Error()) } @@ -272,7 +272,7 @@ func TestGetPipelineFuncSpecAlreadyFetched(t *testing.T) { Spec: pipelineSpec, } - fn := resources.GetPipelineFunc(ctx, kubeclient, tektonclient, nil, pipelineRun) + fn := resources.GetPipelineFunc(ctx, kubeclient, tektonclient, nil, pipelineRun, nil /*VerificationPolicies*/) actualPipeline, actualRefSource, err := fn(ctx, name) if err != nil { t.Fatalf("failed to call pipelinefn: %s", err.Error()) @@ -322,7 +322,7 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) { PipelineRef: pipelineRef, ServiceAccountName: "default", }, - }) + }, nil /*VerificationPolicies*/) resolvedPipeline, resolvedRefSource, err := fn(ctx, pipelineRef.Name) if err != nil { @@ -387,7 +387,7 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { Value: *v1beta1.NewStructuredValues("bar"), }}, }, - }) + }, nil /*VerificationPolicies*/) resolvedPipeline, resolvedRefSource, err := fn(ctx, pipelineRef.Name) if err != nil { @@ -428,7 +428,7 @@ func TestGetPipelineFunc_RemoteResolution_ReplacedParams(t *testing.T) { Value: *v1beta1.NewStructuredValues("banana"), }}, }, - }) + }, nil /*VerificationPolicies*/) _, _, err = fnNotMatching(ctx, pipelineRefNotMatching.Name) if err == nil { @@ -453,13 +453,13 @@ func TestGetPipelineFunc_RemoteResolutionInvalidData(t *testing.T) { PipelineRef: pipelineRef, ServiceAccountName: "default", }, - }) + }, nil /*VerificationPolicies*/) if _, _, err := fn(ctx, pipelineRef.Name); err == nil { t.Fatalf("expected error due to invalid pipeline data but saw none") } } -func TestGetVerifiedPipelineFunc_Success(t *testing.T) { +func TestGetPipelineFunc_VerifySuccess(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. @@ -618,7 +618,7 @@ func TestGetVerifiedPipelineFunc_Success(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { ctx = test.SetupTrustedResourceConfig(ctx, tc.verificationNoMatchPolicy) - fn := resources.GetVerifiedPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.pipelinerun, tc.policies) + fn := resources.GetPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.pipelinerun, tc.policies) resolvedPipeline, source, err := fn(ctx, pipelineRef.Name) if err != nil { @@ -634,7 +634,7 @@ func TestGetVerifiedPipelineFunc_Success(t *testing.T) { } } -func TestGetVerifiedPipelineFunc_VerifyError(t *testing.T) { +func TestGetPipelineFunc_VerifyError(t *testing.T) { ctx := context.Background() tektonclient := fake.NewSimpleClientset() signer, _, k8sclient, vps := test.SetupVerificationPolicies(t) @@ -689,51 +689,43 @@ func TestGetVerifiedPipelineFunc_VerifyError(t *testing.T) { name string requester *test.Requester verificationNoMatchPolicy string - expected runtime.Object expectedErr error }{ { name: "unsigned pipeline fails verification with fail no match policy", requester: requesterUnsigned, verificationNoMatchPolicy: config.FailNoMatchPolicy, - expected: (*v1beta1.Pipeline)(nil), expectedErr: trustedresources.ErrResourceVerificationFailed, }, { name: "unsigned pipeline fails verification with warn no match policy", requester: requesterUnsigned, verificationNoMatchPolicy: config.WarnNoMatchPolicy, - expected: (*v1beta1.Pipeline)(nil), expectedErr: trustedresources.ErrResourceVerificationFailed, }, { name: "unsigned pipeline fails verification with ignore no match policy", requester: requesterUnsigned, verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, - expected: (*v1beta1.Pipeline)(nil), expectedErr: trustedresources.ErrResourceVerificationFailed, }, { name: "modified pipeline fails verification with fail no match policy", requester: requesterModified, verificationNoMatchPolicy: config.FailNoMatchPolicy, - expected: (*v1beta1.Pipeline)(nil), expectedErr: trustedresources.ErrResourceVerificationFailed, }, { name: "modified pipeline fails verification with warn no match policy", requester: requesterModified, verificationNoMatchPolicy: config.WarnNoMatchPolicy, - expected: (*v1beta1.Pipeline)(nil), expectedErr: trustedresources.ErrResourceVerificationFailed, }, { name: "modified pipeline fails verification with ignore no match policy", requester: requesterModified, verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, - expected: (*v1beta1.Pipeline)(nil), expectedErr: trustedresources.ErrResourceVerificationFailed, }, { name: "unmatched pipeline fails with fail no match policy", requester: requesterUnmatched, verificationNoMatchPolicy: config.FailNoMatchPolicy, - expected: (*v1beta1.Pipeline)(nil), - expectedErr: trustedresources.ErrResourceVerificationFailed, + expectedErr: trustedresources.ErrNoMatchedPolicies, }, } for _, tc := range testcases { @@ -746,13 +738,13 @@ func TestGetVerifiedPipelineFunc_VerifyError(t *testing.T) { ServiceAccountName: "default", }, } - fn := resources.GetVerifiedPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, pr, vps) + 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("GetVerifiedPipelineFunc got %v, want %v", err, tc.expectedErr) + t.Errorf("GetPipelineFunc got %v, want %v", err, tc.expectedErr) } - if d := cmp.Diff(resolvedPipeline, tc.expected); d != "" { + if d := cmp.Diff(resolvedPipeline, (*v1beta1.Pipeline)(nil)); d != "" { t.Errorf("resolvedPipeline did not match: %s", diff.PrintWantGot(d)) } if resolvedRefSource != nil { @@ -762,7 +754,7 @@ func TestGetVerifiedPipelineFunc_VerifyError(t *testing.T) { } } -func TestGetVerifiedPipelineFunc_GetFuncError(t *testing.T) { +func TestGetPipelineFunc_GetFuncError(t *testing.T) { ctx := context.Background() tektonclient := fake.NewSimpleClientset() _, k8sclient, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources") @@ -811,13 +803,13 @@ func TestGetVerifiedPipelineFunc_GetFuncError(t *testing.T) { name: "get error when oci bundle return error", requester: requesterUnsigned, pipelinerun: *prBundleError, - expectedErr: fmt.Errorf(`failed to get pipeline: failed to get keychain: serviceaccounts "default" not found`), + expectedErr: fmt.Errorf(`failed to get keychain: serviceaccounts "default" not found`), }, { name: "get error when remote resolution return error", requester: requesterUnsigned, pipelinerun: *prResolutionError, - expectedErr: fmt.Errorf("failed to get pipeline: error accessing data from remote resource: %w", resolvedUnsigned.DataErr), + expectedErr: fmt.Errorf("error accessing data from remote resource: %w", resolvedUnsigned.DataErr), }, } for _, tc := range testcases { @@ -835,11 +827,11 @@ func TestGetVerifiedPipelineFunc_GetFuncError(t *testing.T) { store.OnConfigChanged(featureflags) ctx = store.ToContext(ctx) - fn := resources.GetVerifiedPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.pipelinerun, vps) + fn := resources.GetPipelineFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.pipelinerun, vps) _, _, err = fn(ctx, tc.pipelinerun.Spec.PipelineRef.Name) if d := cmp.Diff(err.Error(), tc.expectedErr.Error()); d != "" { - t.Errorf("GetVerifiedPipelineFunc got %v, want %v", err, tc.expectedErr) + t.Errorf("GetPipelineFunc got %v, want %v", err, tc.expectedErr) } }) } diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index ec62065322b..67dd3901021 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -58,7 +58,8 @@ func GetTaskKind(taskrun *v1beta1.TaskRun) v1beta1.TaskKind { // also requires a kubeclient, tektonclient, namespace, and service account in case it needs to find that task in // cluster or authorize against an external repositroy. It will figure out whether it needs to look in the cluster or in // a remote image to fetch the reference. It will also return the "kind" of the task being referenced. -func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, taskrun *v1beta1.TaskRun, verificationpolicies []*v1alpha1.VerificationPolicy) GetTask { +// OCI bundle and remote resolution tasks will be verified by trusted resources if the feature is enabled +func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, taskrun *v1beta1.TaskRun, verificationPolicies []*v1alpha1.VerificationPolicy) GetTask { // 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 taskrun.Status.TaskSpec != nil { @@ -76,37 +77,16 @@ func GetTaskFuncFromTaskRun(ctx context.Context, k8s kubernetes.Interface, tekto }, refSource, nil } } - return GetVerifiedTaskFunc(ctx, k8s, tekton, requester, taskrun, taskrun.Spec.TaskRef, taskrun.Name, taskrun.Namespace, taskrun.Spec.ServiceAccountName, verificationpolicies) -} - -// GetVerifiedTaskFunc is a wrapper of GetTaskFunc and return the function to verify the task -// if there are matching verification policies -func GetVerifiedTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, - owner kmeta.OwnerRefable, taskref *v1beta1.TaskRef, trName string, namespace, saName string, verificationpolicies []*v1alpha1.VerificationPolicy) GetTask { - get := GetTaskFunc(ctx, k8s, tekton, requester, owner, taskref, trName, namespace, saName) - - return func(context.Context, string) (*v1beta1.Task, *v1beta1.RefSource, error) { - t, s, err := get(ctx, taskref.Name) - if err != nil { - return nil, nil, fmt.Errorf("failed to get task: %w", err) - } - var refSource string - if s != nil { - refSource = s.URI - } - if err := trustedresources.VerifyTask(ctx, t, k8s, refSource, verificationpolicies); err != nil { - return nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err) //nolint:errorlint - } - return t, s, nil - } + return GetTaskFunc(ctx, k8s, tekton, requester, taskrun, taskrun.Spec.TaskRef, taskrun.Name, taskrun.Namespace, taskrun.Spec.ServiceAccountName, verificationPolicies) } // GetTaskFunc is a factory function that will use the given TaskRef as context to return a valid GetTask function. It // also requires a kubeclient, tektonclient, namespace, and service account in case it needs to find that task in // cluster or authorize against an external repositroy. It will figure out whether it needs to look in the cluster or in // a remote image to fetch the reference. It will also return the "kind" of the task being referenced. +// OCI bundle and remote resolution tasks will be verified by trusted resources if the feature is enabled func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset.Interface, requester remoteresource.Requester, - owner kmeta.OwnerRefable, tr *v1beta1.TaskRef, trName string, namespace, saName string) GetTask { + owner kmeta.OwnerRefable, tr *v1beta1.TaskRef, trName string, namespace, saName string, verificationPolicies []*v1alpha1.VerificationPolicy) GetTask { cfg := config.FromContextOrDefaults(ctx) kind := v1beta1.NamespacedTaskKind if tr != nil && tr.Kind != "" { @@ -128,7 +108,7 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset } resolver := oci.NewResolver(tr.Bundle, kc) - return resolveTask(ctx, resolver, name, kind, k8s) + return resolveTask(ctx, resolver, name, kind, k8s, verificationPolicies) } case tr != nil && tr.Resolver != "" && requester != nil: // Return an inline function that implements GetTask by calling Resolver.Get with the specified task type and @@ -148,7 +128,7 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset replacedParams = append(replacedParams, tr.Params...) } resolver := resolution.NewResolver(requester, owner, string(tr.Resolver), trName, namespace, replacedParams) - return resolveTask(ctx, resolver, name, kind, k8s) + return resolveTask(ctx, resolver, name, kind, k8s, verificationPolicies) } default: @@ -163,19 +143,19 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset } // resolveTask accepts an impl of remote.Resolver and attempts to -// fetch a task with given name. An error is returned if the -// remoteresource doesn't work or the returned data isn't a valid -// *v1beta1.Task. -func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kind v1beta1.TaskKind, k8s kubernetes.Interface) (*v1beta1.Task, *v1beta1.RefSource, error) { +// fetch a task with given name and verify the v1beta1 task if trusted resources is enabled. +// An error is returned if the remoteresource doesn't work, the verification failed +// or the returned data isn't a valid *v1beta1.Task. +func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kind v1beta1.TaskKind, k8s kubernetes.Interface, verificationPolicies []*v1alpha1.VerificationPolicy) (*v1beta1.Task, *v1beta1.RefSource, error) { // Because the resolver will only return references with the same kind (eg ClusterTask), this will ensure we // don't accidentally return a Task with the same name but different kind. obj, refSource, err := resolver.Get(ctx, strings.TrimSuffix(strings.ToLower(string(kind)), "s"), name) if err != nil { return nil, nil, err } - taskObj, err := readRuntimeObjectAsTask(ctx, obj) + taskObj, err := readRuntimeObjectAsTask(ctx, obj, k8s, refSource, verificationPolicies) if err != nil { - return nil, nil, fmt.Errorf("failed to convert obj %s into Task", obj.GetObjectKind().GroupVersionKind().String()) + return nil, nil, err } return taskObj, refSource, nil } @@ -186,14 +166,20 @@ func resolveTask(ctx context.Context, resolver remote.Resolver, name string, kin // An error is returned if the given object is not a Task nor a ClusterTask // or if there is an error validating or upgrading an older TaskObject into // its v1beta1 equivalent. +// v1beta1 task will be verified by trusted resources if the feature is enabled // TODO(#5541): convert v1beta1 obj to v1 once we use v1 as the stored version -func readRuntimeObjectAsTask(ctx context.Context, obj runtime.Object) (*v1beta1.Task, error) { +func readRuntimeObjectAsTask(ctx context.Context, obj runtime.Object, k8s kubernetes.Interface, refSource *v1beta1.RefSource, verificationPolicies []*v1alpha1.VerificationPolicy) (*v1beta1.Task, error) { switch obj := obj.(type) { case *v1beta1.Task: + err := trustedresources.VerifyTask(ctx, obj, k8s, refSource, verificationPolicies) + if err != nil { + return nil, err + } return obj, nil case *v1beta1.ClusterTask: return convertClusterTaskToTask(*obj), nil case *v1.Task: + // TODO(#6356): Support V1 Task verification t := &v1beta1.Task{ TypeMeta: metav1.TypeMeta{ Kind: "Task", diff --git a/pkg/reconciler/taskrun/resources/taskref_test.go b/pkg/reconciler/taskrun/resources/taskref_test.go index f83561c12a7..a739c20a8f1 100644 --- a/pkg/reconciler/taskrun/resources/taskref_test.go +++ b/pkg/reconciler/taskrun/resources/taskref_test.go @@ -465,7 +465,7 @@ func TestGetTaskFunc(t *testing.T) { TaskRef: tc.ref, }, } - fn := resources.GetTaskFunc(ctx, kubeclient, tektonclient, nil, trForFunc, tc.ref, "", "default", "default") + fn := resources.GetTaskFunc(ctx, kubeclient, tektonclient, nil, trForFunc, tc.ref, "", "default", "default", nil /*VerificationPolicies*/) task, refSource, err := fn(ctx, tc.ref.Name) if err != nil { @@ -584,7 +584,7 @@ func TestGetTaskFunc_RemoteResolution(t *testing.T) { ServiceAccountName: "default", }, } - fn := resources.GetTaskFunc(ctx, nil, nil, requester, tr, tr.Spec.TaskRef, "", "default", "default") + 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 { @@ -650,7 +650,7 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) { }}, }, } - fn := resources.GetTaskFunc(ctx, nil, nil, requester, tr, tr.Spec.TaskRef, "", "default", "default") + 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 { @@ -692,7 +692,7 @@ func TestGetTaskFunc_RemoteResolution_ReplacedParams(t *testing.T) { }}, }, } - fnNotMatching := resources.GetTaskFunc(ctx, nil, nil, requester, trNotMatching, trNotMatching.Spec.TaskRef, "", "default", "default") + fnNotMatching := resources.GetTaskFunc(ctx, nil, nil, requester, trNotMatching, trNotMatching.Spec.TaskRef, "", "default", "default", nil /*VerificationPolicies*/) _, _, err = fnNotMatching(ctx, taskRefNotMatching.Name) if err == nil { @@ -718,13 +718,13 @@ func TestGetPipelineFunc_RemoteResolutionInvalidData(t *testing.T) { ServiceAccountName: "default", }, } - fn := resources.GetTaskFunc(ctx, nil, nil, requester, tr, tr.Spec.TaskRef, "", "default", "default") + fn := resources.GetTaskFunc(ctx, nil, nil, requester, tr, tr.Spec.TaskRef, "", "default", "default", nil /*VerificationPolicies*/) if _, _, err := fn(ctx, taskRef.Name); err == nil { t.Fatalf("expected error due to invalid pipeline data but saw none") } } -func TestGetVerifiedTaskFunc_Success(t *testing.T) { +func TestGetTaskFunc_VerifySuccess(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. @@ -835,7 +835,7 @@ func TestGetVerifiedTaskFunc_Success(t *testing.T) { ServiceAccountName: "default", }, } - fn := resources.GetVerifiedTaskFunc(ctx, k8sclient, tektonclient, tc.requester, tr, tr.Spec.TaskRef, "", "default", "default", tc.policies) + fn := resources.GetTaskFunc(ctx, k8sclient, tektonclient, tc.requester, tr, tr.Spec.TaskRef, "", "default", "default", tc.policies) resolvedTask, refSource, err := fn(ctx, taskRef.Name) @@ -854,7 +854,7 @@ func TestGetVerifiedTaskFunc_Success(t *testing.T) { } } -func TestGetVerifiedTaskFunc_VerifyError(t *testing.T) { +func TestGetTaskFunc_VerifyError(t *testing.T) { ctx := context.Background() signer, _, k8sclient, vps := test.SetupVerificationPolicies(t) tektonclient := fake.NewSimpleClientset() @@ -952,7 +952,7 @@ func TestGetVerifiedTaskFunc_VerifyError(t *testing.T) { requester: requesterUnmatched, verificationNoMatchPolicy: config.FailNoMatchPolicy, expected: nil, - expectedErr: trustedresources.ErrResourceVerificationFailed, + expectedErr: trustedresources.ErrNoMatchedPolicies, }, } for _, tc := range testcases { @@ -965,12 +965,12 @@ func TestGetVerifiedTaskFunc_VerifyError(t *testing.T) { ServiceAccountName: "default", }, } - fn := resources.GetVerifiedTaskFunc(ctx, k8sclient, tektonclient, tc.requester, tr, tr.Spec.TaskRef, "", "default", "default", vps) + fn := resources.GetTaskFunc(ctx, k8sclient, tektonclient, tc.requester, tr, tr.Spec.TaskRef, "", "default", "default", vps) resolvedTask, resolvedRefSource, err := fn(ctx, taskRef.Name) if !errors.Is(err, tc.expectedErr) { - t.Errorf("GetVerifiedTaskFunc got %v but want %v", err, tc.expectedErr) + t.Errorf("GetTaskFunc got %v but want %v", err, tc.expectedErr) } if d := cmp.Diff(tc.expected, resolvedTask); d != "" { @@ -984,7 +984,7 @@ func TestGetVerifiedTaskFunc_VerifyError(t *testing.T) { } } -func TestGetVerifiedTaskFunc_GetFuncError(t *testing.T) { +func TestGetTaskFunc_GetFuncError(t *testing.T) { ctx := context.Background() _, k8sclient, vps := test.SetupMatchAllVerificationPolicies(t, "trusted-resources") tektonclient := fake.NewSimpleClientset() @@ -1033,13 +1033,13 @@ func TestGetVerifiedTaskFunc_GetFuncError(t *testing.T) { name: "get error when oci bundle return error", requester: requesterUnsigned, taskrun: *trBundleError, - expectedErr: fmt.Errorf(`failed to get task: failed to get keychain: serviceaccounts "default" not found`), + expectedErr: fmt.Errorf(`failed to get keychain: serviceaccounts "default" not found`), }, { name: "get error when remote resolution return error", requester: requesterUnsigned, taskrun: *trResolutionError, - expectedErr: fmt.Errorf("failed to get task: error accessing data from remote resource: %w", resolvedUnsigned.DataErr), + expectedErr: fmt.Errorf("error accessing data from remote resource: %w", resolvedUnsigned.DataErr), }, } for _, tc := range testcases { @@ -1057,7 +1057,7 @@ func TestGetVerifiedTaskFunc_GetFuncError(t *testing.T) { store.OnConfigChanged(featureflags) ctx = store.ToContext(ctx) - fn := resources.GetVerifiedTaskFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.taskrun, tc.taskrun.Spec.TaskRef, "", "default", "default", vps) + fn := resources.GetTaskFunc(ctx, k8sclient, tektonclient, tc.requester, &tc.taskrun, tc.taskrun.Spec.TaskRef, "", "default", "default", vps) _, _, err = fn(ctx, tc.taskrun.Spec.TaskRef.Name) diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 65bb2e93bd7..5710c5221ff 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -1603,17 +1603,17 @@ status: - reason: ToBeRetried status: Unknown type: Succeeded - message: "error when listing tasks for taskRun test-taskrun-run-retry-prepare-failure: failed to get task: tasks.tekton.dev \"test-task\" not found" + message: "error when listing tasks for taskRun test-taskrun-run-retry-prepare-failure: tasks.tekton.dev \"test-task\" not found" retriesStatus: - conditions: - reason: TaskRunResolutionFailed status: "False" type: "Succeeded" - message: "error when listing tasks for taskRun test-taskrun-run-retry-prepare-failure: failed to get task: tasks.tekton.dev \"test-task\" not found" + message: "error when listing tasks for taskRun test-taskrun-run-retry-prepare-failure: tasks.tekton.dev \"test-task\" not found" startTime: "2021-12-31T23:59:59Z" completionTime: "2022-01-01T00:00:00Z" `) - prepareError = fmt.Errorf("1 error occurred:\n\t* error when listing tasks for taskRun test-taskrun-run-retry-prepare-failure: failed to get task: tasks.tekton.dev \"test-task\" not found") + prepareError = fmt.Errorf("1 error occurred:\n\t* error when listing tasks for taskRun test-taskrun-run-retry-prepare-failure: tasks.tekton.dev \"test-task\" not found") toFailOnReconcileFailureTaskRun = parse.MustParseV1beta1TaskRun(t, ` metadata: name: test-taskrun-results-type-mismatched @@ -4894,7 +4894,27 @@ spec: image: myimage name: mycontainer `) - tr := parse.MustParseV1beta1TaskRun(t, ` + // Set up a fake registry to push an image to. + s := httptest.NewServer(registry.New()) + defer s.Close() + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + + signer, _, vps := test.SetupMatchAllVerificationPolicies(t, ts.Namespace) + signedTask, err := test.GetSignedTask(ts, signer, "test-task") + if err != nil { + t.Fatal("fail to sign task", err) + } + + // Upload the simple task to the registry for our taskRunBundle TaskRun. + ref, err := test.CreateImage(u.Host+"/"+signedTask.Name, signedTask) + if err != nil { + t.Fatalf("failed to upload image with simple task: %s", err.Error()) + } + + tr := parse.MustParseV1beta1TaskRun(t, fmt.Sprintf(` metadata: name: test-taskrun namespace: foo @@ -4903,29 +4923,24 @@ spec: - name: myarg value: foo taskRef: + bundle: %s name: test-task status: podName: the-pod -`) - - signer, _, vps := test.SetupMatchAllVerificationPolicies(t, tr.Namespace) - signedTask, err := test.GetSignedTask(ts, signer, "test-task") - if err != nil { - t.Fatal("fail to sign task", err) - } +`, ref)) cms := []*corev1.ConfigMap{ { ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, Data: map[string]string{ "trusted-resources-verification-no-match-policy": config.FailNoMatchPolicy, + "enable-tekton-oci-bundles": "true", }, }, } d := test.Data{ TaskRuns: []*v1beta1.TaskRun{tr}, - Tasks: []*v1beta1.Task{signedTask}, ConfigMaps: cms, VerificationPolicies: vps, } @@ -4959,21 +4974,8 @@ spec: image: myimage name: mycontainer `) - tr := parse.MustParseV1beta1TaskRun(t, ` -metadata: - name: test-taskrun - namespace: foo -spec: - params: - - name: myarg - value: foo - taskRef: - name: test-task -status: - podName: the-pod -`) - signer, _, vps := test.SetupMatchAllVerificationPolicies(t, tr.Namespace) + signer, _, vps := test.SetupMatchAllVerificationPolicies(t, ts.Namespace) signedTask, err := test.GetSignedTask(ts, signer, "test-task") if err != nil { t.Fatal("fail to sign task", err) @@ -4990,33 +4992,62 @@ status: ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, Data: map[string]string{ "trusted-resources-verification-no-match-policy": config.FailNoMatchPolicy, + "enable-tekton-oci-bundles": "true", }, }, } + // Set up a fake registry to push an image to. + s := httptest.NewServer(registry.New()) + defer s.Close() + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + testCases := []struct { name string taskrun []*v1beta1.TaskRun - task []*v1beta1.Task + task *v1beta1.Task expectedError error }{ { name: "unsigned task fails verification", - task: []*v1beta1.Task{ts}, + task: ts, expectedError: trustedresources.ErrResourceVerificationFailed, }, { name: "modified task fails verification", - task: []*v1beta1.Task{tamperedTask}, + task: tamperedTask, expectedError: trustedresources.ErrResourceVerificationFailed, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + // Upload the simple task to the registry + ref, err := test.CreateImage(u.Host+"/"+tc.task.Name, tc.task) + if err != nil { + t.Fatalf("failed to upload image with simple task: %s", err.Error()) + } + + tr := parse.MustParseV1beta1TaskRun(t, fmt.Sprintf(` +metadata: + name: test-taskrun + namespace: foo +spec: + params: + - name: myarg + value: foo + taskRef: + bundle: %s + name: test-task +status: + podName: the-pod +`, ref)) + d := test.Data{ TaskRuns: []*v1beta1.TaskRun{tr}, - Tasks: tc.task, ConfigMaps: cms, VerificationPolicies: vps, } @@ -5024,12 +5055,12 @@ status: testAssets, cancel := getTaskRunController(t, d) defer cancel() createServiceAccount(t, testAssets, tr.Spec.ServiceAccountName, tr.Namespace) - err := testAssets.Controller.Reconciler.Reconcile(testAssets.Ctx, getRunName(tr)) + err = testAssets.Controller.Reconciler.Reconcile(testAssets.Ctx, getRunName(tr)) if !errors.Is(err, tc.expectedError) { t.Errorf("Reconcile got %v but want %v", err, tc.expectedError) } - tr, err := testAssets.Clients.Pipeline.TektonV1beta1().TaskRuns(tr.Namespace).Get(testAssets.Ctx, tr.Name, metav1.GetOptions{}) + tr, err = testAssets.Clients.Pipeline.TektonV1beta1().TaskRuns(tr.Namespace).Get(testAssets.Ctx, tr.Name, metav1.GetOptions{}) if err != nil { t.Fatalf("getting updated taskrun: %v", err) } diff --git a/pkg/trustedresources/verify.go b/pkg/trustedresources/verify.go index 1e6b729f942..5be3d23cc36 100644 --- a/pkg/trustedresources/verify.go +++ b/pkg/trustedresources/verify.go @@ -45,8 +45,12 @@ const ( // Skip the verification when no policies are found and trusted-resources-verification-no-match-policy is set to ignore or warn // Return an error when no policies are found and trusted-resources-verification-no-match-policy is set to fail, // or the resource fails to pass matched enforce verification policy -// refSourceURI is from RefSource.URI, which will be used to match policy patterns. k8s is used to fetch secret from cluster -func VerifyTask(ctx context.Context, taskObj *v1beta1.Task, k8s kubernetes.Interface, refSourceURI string, verificationpolicies []*v1alpha1.VerificationPolicy) error { +// refSource is used to match policy patterns by using refSource.URI. k8s is used to fetch secret from cluster +func VerifyTask(ctx context.Context, taskObj *v1beta1.Task, k8s kubernetes.Interface, refSource *v1beta1.RefSource, verificationpolicies []*v1alpha1.VerificationPolicy) error { + var refSourceURI string + if refSource != nil { + refSourceURI = refSource.URI + } matchedPolicies, err := getMatchedPolicies(taskObj.TaskMetadata().Name, refSourceURI, verificationpolicies) if err != nil { if errors.Is(err, ErrNoMatchedPolicies) { @@ -81,8 +85,12 @@ func VerifyTask(ctx context.Context, taskObj *v1beta1.Task, k8s kubernetes.Inter // Skip the verification when no policies are found and trusted-resources-verification-no-match-policy is set to ignore or warn // Return an error when no policies are found and trusted-resources-verification-no-match-policy is set to fail, // or the resource fails to pass matched enforce verification policy -// refSourceURI is from RefSource.URI, which will be used to match policy patterns. k8s is used to fetch secret from cluster -func VerifyPipeline(ctx context.Context, pipelineObj *v1beta1.Pipeline, k8s kubernetes.Interface, refSourceURI string, verificationpolicies []*v1alpha1.VerificationPolicy) error { +// refSource is used to match policy patterns by using refSource.URI. k8s is used to fetch secret from cluster +func VerifyPipeline(ctx context.Context, pipelineObj *v1beta1.Pipeline, k8s kubernetes.Interface, refSource *v1beta1.RefSource, verificationpolicies []*v1alpha1.VerificationPolicy) error { + var refSourceURI string + if refSource != nil { + refSourceURI = refSource.URI + } matchedPolicies, err := getMatchedPolicies(pipelineObj.PipelineMetadata().Name, refSourceURI, verificationpolicies) if err != nil { if errors.Is(err, ErrNoMatchedPolicies) { diff --git a/pkg/trustedresources/verify_test.go b/pkg/trustedresources/verify_test.go index 8b847e9b723..04c9bc4ba2c 100644 --- a/pkg/trustedresources/verify_test.go +++ b/pkg/trustedresources/verify_test.go @@ -200,48 +200,48 @@ func TestVerifyTask_Success(t *testing.T) { tcs := []struct { name string task *v1beta1.Task - source string + source *v1beta1.RefSource signer signature.SignerVerifier verificationNoMatchPolicy string verificationPolicies []*v1alpha1.VerificationPolicy }{{ name: "signed git source task passes verification", task: signedTask, - source: "git+https://github.com/tektoncd/catalog.git", + source: &v1beta1.RefSource{URI: "git+https://github.com/tektoncd/catalog.git"}, verificationNoMatchPolicy: config.FailNoMatchPolicy, verificationPolicies: vps, }, { name: "signed bundle source task passes verification", task: signedTask, - source: "gcr.io/tekton-releases/catalog/upstream/git-clone", + source: &v1beta1.RefSource{URI: "gcr.io/tekton-releases/catalog/upstream/git-clone"}, verificationNoMatchPolicy: config.FailNoMatchPolicy, verificationPolicies: vps, }, { name: "signed task with sha384 key", task: signedTask384, - source: "gcr.io/tekton-releases/catalog/upstream/sha384", + source: &v1beta1.RefSource{URI: "gcr.io/tekton-releases/catalog/upstream/sha384"}, verificationNoMatchPolicy: config.FailNoMatchPolicy, verificationPolicies: []*v1alpha1.VerificationPolicy{sha384Vp}, }, { name: "ignore no match policy skips verification when no matching policies", task: unsignedTask, - source: mismatchedSource, + source: &v1beta1.RefSource{URI: mismatchedSource}, verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, }, { name: "warn no match policy skips verification when no matching policies", task: unsignedTask, - source: mismatchedSource, + source: &v1beta1.RefSource{URI: mismatchedSource}, verificationNoMatchPolicy: config.WarnNoMatchPolicy, }, { name: "unsigned task matches warn policy doesn't fail verification", task: unsignedTask, - source: "git+https://github.com/tektoncd/catalog.git", + source: &v1beta1.RefSource{URI: "git+https://github.com/tektoncd/catalog.git"}, verificationNoMatchPolicy: config.FailNoMatchPolicy, verificationPolicies: []*v1alpha1.VerificationPolicy{warnPolicy}, }, { name: "modified task matches warn policy doesn't fail verification", task: modifiedTask, - source: "git+https://github.com/tektoncd/catalog.git", + source: &v1beta1.RefSource{URI: "git+https://github.com/tektoncd/catalog.git"}, verificationNoMatchPolicy: config.FailNoMatchPolicy, verificationPolicies: []*v1alpha1.VerificationPolicy{warnPolicy}, }} @@ -277,37 +277,37 @@ func TestVerifyTask_Error(t *testing.T) { tcs := []struct { name string task *v1beta1.Task - source string + source *v1beta1.RefSource verificationPolicy []*v1alpha1.VerificationPolicy expectedError error }{{ name: "unsigned Task fails verification", task: unsignedTask, - source: "git+https://github.com/tektoncd/catalog.git", + source: &v1beta1.RefSource{URI: "git+https://github.com/tektoncd/catalog.git"}, verificationPolicy: vps, expectedError: ErrResourceVerificationFailed, }, { name: "modified Task fails verification", task: tamperedTask, - source: matchingSource, + source: &v1beta1.RefSource{URI: matchingSource}, verificationPolicy: vps, expectedError: ErrResourceVerificationFailed, }, { name: "task not matching pattern fails verification", task: signedTask, - source: mismatchedSource, + source: &v1beta1.RefSource{URI: mismatchedSource}, verificationPolicy: vps, expectedError: ErrNoMatchedPolicies, }, { name: "verification fails with empty policy", task: tamperedTask, - source: matchingSource, + source: &v1beta1.RefSource{URI: matchingSource}, verificationPolicy: []*v1alpha1.VerificationPolicy{}, expectedError: ErrNoMatchedPolicies, }, { name: "Verification fails with regex error", task: signedTask, - source: "git+https://github.com/tektoncd/catalog.git", + source: &v1beta1.RefSource{URI: "git+https://github.com/tektoncd/catalog.git"}, verificationPolicy: []*v1alpha1.VerificationPolicy{ { ObjectMeta: metav1.ObjectMeta{ @@ -324,7 +324,7 @@ func TestVerifyTask_Error(t *testing.T) { }, { name: "Verification fails with error from policy", task: signedTask, - source: "git+https://github.com/tektoncd/catalog.git", + source: &v1beta1.RefSource{URI: "git+https://github.com/tektoncd/catalog.git"}, verificationPolicy: []*v1alpha1.VerificationPolicy{ { ObjectMeta: metav1.ObjectMeta{ @@ -369,27 +369,27 @@ func TestVerifyPipeline_Success(t *testing.T) { tcs := []struct { name string pipeline *v1beta1.Pipeline - source string + source *v1beta1.RefSource verificationNoMatchPolicy string }{{ name: "signed git source pipeline passes verification", pipeline: signedPipeline, - source: "git+https://github.com/tektoncd/catalog.git", + source: &v1beta1.RefSource{URI: "git+https://github.com/tektoncd/catalog.git"}, verificationNoMatchPolicy: config.FailNoMatchPolicy, }, { name: "signed bundle source pipeline passes verification", pipeline: signedPipeline, - source: "gcr.io/tekton-releases/catalog/upstream/git-clone", + source: &v1beta1.RefSource{URI: "gcr.io/tekton-releases/catalog/upstream/git-clone"}, verificationNoMatchPolicy: config.FailNoMatchPolicy, }, { name: "ignore no match policy skips verification when no matching policies", pipeline: unsignedPipeline, - source: mismatchedSource, + source: &v1beta1.RefSource{URI: mismatchedSource}, verificationNoMatchPolicy: config.IgnoreNoMatchPolicy, }, { name: "warn no match policy skips verification when no matching policies", pipeline: unsignedPipeline, - source: mismatchedSource, + source: &v1beta1.RefSource{URI: mismatchedSource}, verificationNoMatchPolicy: config.WarnNoMatchPolicy, }} for _, tc := range tcs { @@ -422,22 +422,22 @@ func TestVerifyPipeline_Error(t *testing.T) { tcs := []struct { name string pipeline *v1beta1.Pipeline - source string + source *v1beta1.RefSource verificationPolicy []*v1alpha1.VerificationPolicy }{{ name: "Tampered Task Fails Verification with tampered content", pipeline: tamperedPipeline, - source: matchingSource, + source: &v1beta1.RefSource{URI: matchingSource}, verificationPolicy: vps, }, { name: "Task Not Matching Pattern Fails Verification", pipeline: signedPipeline, - source: mismatchedSource, + source: &v1beta1.RefSource{URI: mismatchedSource}, verificationPolicy: vps, }, { name: "Verification fails with regex error", pipeline: signedPipeline, - source: "git+https://github.com/tektoncd/catalog.git", + source: &v1beta1.RefSource{URI: "git+https://github.com/tektoncd/catalog.git"}, verificationPolicy: []*v1alpha1.VerificationPolicy{ { ObjectMeta: metav1.ObjectMeta{ diff --git a/test/trusted_resources_test.go b/test/trusted_resources_test.go index 5646d973078..7026c997d76 100644 --- a/test/trusted_resources_test.go +++ b/test/trusted_resources_test.go @@ -112,9 +112,16 @@ spec: tasks: - name: task taskRef: - name: %s + resolver: cluster kind: Task -`, helpers.ObjectNameForTest(t), namespace, signedTask.Name)) + params: + - name: kind + value: task + - name: name + value: %s + - name: namespace + value: %s +`, helpers.ObjectNameForTest(t), namespace, signedTask.Name, namespace)) signedPipeline, err := GetSignedPipeline(pipeline, signer, "signedpipeline") if err != nil { @@ -131,8 +138,16 @@ metadata: namespace: %s spec: pipelineRef: - name: %s -`, helpers.ObjectNameForTest(t), namespace, signedPipeline.Name)) + resolver: cluster + kind: Pipeline + params: + - name: kind + value: pipeline + - name: name + value: %s + - name: namespace + value: %s +`, helpers.ObjectNameForTest(t), namespace, signedPipeline.Name, namespace)) t.Logf("Creating PipelineRun %s", pr.Name) if _, err := c.V1beta1PipelineRunClient.Create(ctx, pr, metav1.CreateOptions{}); err != nil { @@ -213,9 +228,16 @@ spec: tasks: - name: task taskRef: - name: %s + resolver: cluster kind: Task -`, helpers.ObjectNameForTest(t), namespace, signedTask.Name)) + params: + - name: kind + value: task + - name: name + value: %s + - name: namespace + value: %s +`, helpers.ObjectNameForTest(t), namespace, signedTask.Name, namespace)) signedPipeline, err := GetSignedPipeline(pipeline, signer, "signedpipeline") if err != nil { @@ -232,8 +254,16 @@ metadata: namespace: %s spec: pipelineRef: - name: %s -`, helpers.ObjectNameForTest(t), namespace, signedPipeline.Name)) + resolver: cluster + kind: Pipeline + params: + - name: kind + value: pipeline + - name: name + value: %s + - name: namespace + value: %s +`, helpers.ObjectNameForTest(t), namespace, signedPipeline.Name, namespace)) t.Logf("Creating PipelineRun %s", pr.Name) if _, err := c.V1beta1PipelineRunClient.Create(ctx, pr, metav1.CreateOptions{}); err != nil {