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..2f4083e4d3e 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 @@ -152,14 +127,14 @@ func (l *LocalPipelineRefResolver) GetPipeline(ctx context.Context, name string) // 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) { +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 } @@ -170,10 +145,15 @@ func resolvePipeline(ctx context.Context, resolver remote.Resolver, name string) // An error is returned if the given object is not a // PipelineObject or if there is an error validating or upgrading an // older PipelineObject into its v1beta1 equivalent. +// v1beta1 pipeline 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 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: t := &v1beta1.Pipeline{ diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go index 2b7c025ce7d..1533af538a5 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_Success(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..ce14cb5f96d 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: @@ -166,16 +146,16 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset // 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) { +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,10 +166,15 @@ 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 diff --git a/pkg/reconciler/taskrun/resources/taskref_test.go b/pkg/reconciler/taskrun/resources/taskref_test.go index f83561c12a7..ec9ae704289 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_Success(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..e1ceb372792 100644 --- a/pkg/trustedresources/verify.go +++ b/pkg/trustedresources/verify.go @@ -46,7 +46,11 @@ const ( // 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 { +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) { @@ -82,7 +86,11 @@ func VerifyTask(ctx context.Context, taskObj *v1beta1.Task, k8s kubernetes.Inter // 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 { +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 {