diff --git a/docs/trusted-resources.md b/docs/trusted-resources.md index f36d11b4cc7..282f7dce654 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 is an alpha feature and supports `v1beta1` version of `Task` and `Pipeline`. + +**Note**: Currently, trusted resources only support verifying Tekton resources that come from remote places i.e. git, OCI registry and Artifact Hub. To use [cluster resolver](./cluster-resolver.md) for in-cluster resources, make sure to set all default values for the resources before applied to cluster, because the mutating webhook will update the default fields if not given and fail the verification. 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 e2d103fe392..4fc89006216 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 ab0409736b3..f2a96d371d5 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -36,6 +36,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + resolutionv1beta1 "github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1" resolutionutil "github.com/tektoncd/pipeline/pkg/internal/resolution" "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent" "github.com/tektoncd/pipeline/pkg/reconciler/events/k8sevent" @@ -43,6 +44,7 @@ import ( ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" "github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim" resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common" + remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource" "github.com/tektoncd/pipeline/test" "github.com/tektoncd/pipeline/test/diff" "github.com/tektoncd/pipeline/test/names" @@ -11347,30 +11349,8 @@ spec: } func TestReconcile_verifyResolvedPipeline_Success(t *testing.T) { - names.TestingSeed() - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() + resolverName := "foobar" - 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,21 +11365,63 @@ 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) } + signedTaskBytes, err := yaml.Marshal(signedTask) + if err != nil { + t.Fatal("fail to marshal task", err) + } + + ps := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: test-pipeline + namespace: foo +spec: + tasks: + - name: test-1 + taskRef: + resolver: %s +`, resolverName)) + signedPipeline, err := test.GetSignedPipeline(ps, signer, "test-pipeline") if err != nil { t.Fatal("fail to sign pipeline", err) } + signedPipelineBytes, err := yaml.Marshal(signedPipeline) + if err != nil { + t.Fatal("fail to marshal task", err) + } + + prs := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` +metadata: + name: test-pipelinerun + namespace: foo + selfLink: /pipeline/1234 +spec: + pipelineRef: + resolver: %s +`, resolverName)) + + cms := []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "trusted-resources-verification-no-match-policy": config.FailNoMatchPolicy, + }, + }, + } + + pipelineReq := getResolvedResolutionRequest(t, resolverName, signedPipelineBytes, prs.Namespace, prs.Name) + taskReq := getResolvedResolutionRequest(t, resolverName, signedTaskBytes, prs.Namespace, prs.Name+"-"+ps.Spec.Tasks[0].Name) d := test.Data{ PipelineRuns: []*v1beta1.PipelineRun{prs}, - Pipelines: []*v1beta1.Pipeline{signedPipeline}, - Tasks: []*v1beta1.Task{signedTask}, VerificationPolicies: vps, + ConfigMaps: cms, + ResolutionRequests: []*resolutionv1beta1.ResolutionRequest{&pipelineReq, &taskReq}, } prt := newPipelineRunTest(t, d) defer prt.Cancel() @@ -11410,31 +11432,10 @@ spec: } func TestReconcile_verifyResolvedPipeline_Error(t *testing.T) { - names.TestingSeed() - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() + resolverName := "foobar" - 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, ` + // Case1: unsigned Pipeline refers to unsigned Task + unsignedTask := parse.MustParseV1beta1Task(t, ` metadata: name: test-task namespace: foo @@ -11447,78 +11448,188 @@ spec: - name: foo value: bar `) + unsignedTaskBytes, err := yaml.Marshal(unsignedTask) + if err != nil { + t.Fatal("fail to marshal task", err) + } - signer, _, vps := test.SetupMatchAllVerificationPolicies(t, prs.Namespace) - signedTask, err := test.GetSignedTask(ts, signer, "test-task") + unsignedPipeline := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: test-pipeline + namespace: foo +spec: + tasks: + - name: test-1 + taskRef: + resolver: %s +`, resolverName)) + unsignedPipelineBytes, err := yaml.Marshal(unsignedPipeline) + if err != nil { + t.Fatal("fail to marshal task", err) + } + + // Case2: signed Pipeline refers to unsigned Task + signer, _, vps := test.SetupMatchAllVerificationPolicies(t, unsignedTask.Namespace) + signedPipelineWithUnsignedTask, err := test.GetSignedPipeline(unsignedPipeline, signer, "test-pipeline") + if err != nil { + t.Fatal("fail to sign pipeline", err) + } + signedPipelineWithUnsignedTaskBytes, err := yaml.Marshal(signedPipelineWithUnsignedTask) + if err != nil { + t.Fatal("fail to marshal task", err) + } + + // Case3: signed Pipeline refers to modified Task + signedTask, err := test.GetSignedTask(unsignedTask, signer, "test-task") if err != nil { t.Fatal("fail to sign task", err) } - signedPipeline, err := test.GetSignedPipeline(ps, signer, "test-pipeline") + signedTaskBytes, err := yaml.Marshal(signedTask) + if err != nil { + t.Fatal("fail to marshal task", err) + } + modifiedTask := signedTask.DeepCopy() + if modifiedTask.Annotations == nil { + modifiedTask.Annotations = make(map[string]string) + } + modifiedTask.Annotations["random"] = "attack" + modifiedTaskBytes, err := yaml.Marshal(modifiedTask) + if err != nil { + t.Fatal("fail to marshal task", err) + } + + ps := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: test-pipeline + namespace: foo +spec: + tasks: + - name: test-1 + taskRef: + resolver: %s +`, resolverName)) + signedPipelineWithModifiedTask, err := test.GetSignedPipeline(ps, signer, "test-pipeline") if err != nil { t.Fatal("fail to sign pipeline", err) } + signedPipelineWithModifiedTaskBytes, err := yaml.Marshal(signedPipelineWithModifiedTask) + if err != nil { + t.Fatal("fail to marshal task", err) + } - tamperedTask := signedTask.DeepCopy() - if tamperedTask.Annotations == nil { - tamperedTask.Annotations = make(map[string]string) + // Case4: modified Pipeline refers to signed Task + ps = parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: test-pipeline + namespace: foo +spec: + tasks: + - name: test-1 + taskRef: + resolver: %s +`, resolverName)) + signedPipeline, err := test.GetSignedPipeline(ps, signer, "test-pipeline") + if err != nil { + t.Fatal("fail to sign pipeline", err) + } + modifiedPipeline := signedPipeline.DeepCopy() + if modifiedPipeline.Annotations == nil { + modifiedPipeline.Annotations = make(map[string]string) + } + modifiedPipeline.Annotations["random"] = "attack" + modifiedPipelineBytes, err := yaml.Marshal(modifiedPipeline) + if err != nil { + t.Fatal("fail to marshal task", err) } - tamperedTask.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 + name string + pipelinerun []*v1beta1.PipelineRun + pipelineBytes []byte + taskBytes []byte }{ { - name: "unsigned pipeline fails verification", - pipelinerun: []*v1beta1.PipelineRun{prs}, - pipeline: []*v1beta1.Pipeline{ps}, + name: "unsigned pipeline fails verification", + pipelineBytes: unsignedPipelineBytes, + taskBytes: unsignedTaskBytes, }, { - 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", + pipelineBytes: signedPipelineWithUnsignedTaskBytes, + taskBytes: unsignedTaskBytes, }, { - 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", + pipelineBytes: signedPipelineWithModifiedTaskBytes, + taskBytes: modifiedTaskBytes, }, { - 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", + pipelineBytes: modifiedPipelineBytes, + taskBytes: signedTaskBytes, }, } - for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + prs := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` +metadata: + name: test-pipelinerun + namespace: foo + selfLink: /pipeline/1234 +spec: + pipelineRef: + resolver: %s + serviceAccountName: default +`, resolverName)) + + pipelineReq := getResolvedResolutionRequest(t, resolverName, tc.pipelineBytes, prs.Namespace, prs.Name) + taskReq := getResolvedResolutionRequest(t, resolverName, tc.taskBytes, prs.Namespace, prs.Name+"-"+ps.Spec.Tasks[0].Name) + d := test.Data{ - PipelineRuns: tc.pipelinerun, - Pipelines: tc.pipeline, - Tasks: tc.task, + PipelineRuns: []*v1beta1.PipelineRun{prs}, + ConfigMaps: cms, VerificationPolicies: vps, + ResolutionRequests: []*resolutionv1beta1.ResolutionRequest{&pipelineReq, &taskReq}, } prt := newPipelineRunTest(t, d) defer prt.Cancel() reconciledRun, _ := prt.reconcileRun("foo", "test-pipelinerun", []string{}, true) - checkPipelineRunConditionStatusAndReason(t, reconciledRun, corev1.ConditionFalse, ReasonResourceVerificationFailed) }) } } +// getResolvedResolutionRequest is a helper function to return the ResolutionRequest and the data is filled with resourceBytes, +// the ResolutionRequest's name is generated by resolverName, namespace and runName. +func getResolvedResolutionRequest(t *testing.T, resolverName string, resourceBytes []byte, namespace string, runName string) resolutionv1beta1.ResolutionRequest { + t.Helper() + name, err := remoteresource.GenerateDeterministicName(resolverName, namespace+"/"+runName, nil) + if err != nil { + t.Errorf("error generating name for %s/%s/%s: %v", resolverName, namespace, runName, err) + } + rr := resolutionv1beta1.ResolutionRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + CreationTimestamp: metav1.Time{Time: time.Now()}, + }, + } + rr.Status.ResolutionRequestStatusFields.Data = base64.StdEncoding.Strict().EncodeToString(resourceBytes) + rr.Status.MarkSucceeded() + return rr +} + 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..b905fab9d05 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,22 @@ 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: + // Verify the Pipeline once we fetch from the remote resolution, mutating, validation and conversion of the pipeline should happen after the verification, since signatures are based on the remote pipeline contents + err := trustedresources.VerifyPipeline(ctx, obj, k8s, refSource, verificationPolicies) + if err != nil { + return nil, fmt.Errorf("remote Pipeline verification failed for object %s: %w", obj.GetName(), err) + } return obj, nil case *v1.Pipeline: + // TODO(#6356): Support V1 Task verification t := &v1beta1.Pipeline{ TypeMeta: metav1.TypeMeta{ Kind: "Pipeline", @@ -183,7 +165,7 @@ func readRuntimeObjectAsPipeline(ctx context.Context, obj runtime.Object) (*v1be }, } if err := t.ConvertFrom(ctx, obj); err != nil { - return nil, err + return nil, fmt.Errorf("failed to convert obj %s into Pipeline", obj.GetObjectKind().GroupVersionKind().String()) } return t, nil } 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..95a0730f1de 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 fails +// 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,21 @@ 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: + // Verify the Task once we fetch from the remote resolution, mutating, validation and conversion of the task should happen after the verification, since signatures are based on the remote task contents + err := trustedresources.VerifyTask(ctx, obj, k8s, refSource, verificationPolicies) + if err != nil { + return nil, fmt.Errorf("remote Task verification failed for object %s: %w", obj.GetName(), 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", @@ -201,7 +188,7 @@ func readRuntimeObjectAsTask(ctx context.Context, obj runtime.Object) (*v1beta1. }, } if err := t.ConvertFrom(ctx, obj); err != nil { - return nil, err + return nil, fmt.Errorf("failed to convert obj %s into Task", obj.GetObjectKind().GroupVersionKind().String()) } return t, nil } 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..23c891969d6 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -37,6 +37,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + resolutionv1beta1 "github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1" resolutionutil "github.com/tektoncd/pipeline/pkg/internal/resolution" podconvert "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent" @@ -45,6 +46,7 @@ import ( ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" "github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim" resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common" + remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource" "github.com/tektoncd/pipeline/pkg/trustedresources" "github.com/tektoncd/pipeline/pkg/workspace" "github.com/tektoncd/pipeline/test" @@ -72,6 +74,7 @@ import ( pkgreconciler "knative.dev/pkg/reconciler" "knative.dev/pkg/system" _ "knative.dev/pkg/system/testing" // Setup system.Namespace() + "sigs.k8s.io/yaml" ) const ( @@ -1603,17 +1606,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 @@ -4876,10 +4879,7 @@ status: } func TestReconcile_verifyResolvedTask_Success(t *testing.T) { - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() - + resolverName := "foobar" ts := parse.MustParseV1beta1Task(t, ` metadata: name: test-task @@ -4894,7 +4894,18 @@ spec: image: myimage name: mycontainer `) - tr := parse.MustParseV1beta1TaskRun(t, ` + + 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) + } + signedTaskBytes, err := yaml.Marshal(signedTask) + if err != nil { + t.Fatal("fail to marshal task", err) + } + + tr := parse.MustParseV1beta1TaskRun(t, fmt.Sprintf(` metadata: name: test-taskrun namespace: foo @@ -4903,16 +4914,11 @@ spec: - name: myarg value: foo taskRef: - name: test-task + resolver: %s + serviceAccountName: default 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) - } +`, resolverName)) cms := []*corev1.ConfigMap{ { @@ -4922,12 +4928,12 @@ status: }, }, } - + rr := getResolvedResolutionRequest(t, resolverName, signedTaskBytes, tr.Namespace, tr.Name) d := test.Data{ TaskRuns: []*v1beta1.TaskRun{tr}, - Tasks: []*v1beta1.Task{signedTask}, ConfigMaps: cms, VerificationPolicies: vps, + ResolutionRequests: []*resolutionv1beta1.ResolutionRequest{&rr}, } testAssets, cancel := getTaskRunController(t, d) @@ -4938,14 +4944,22 @@ status: if ok, _ := controller.IsRequeueKey(err); !ok { t.Errorf("Error reconciling TaskRun. Got error %v", err) } + 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) + } + condition := tr.Status.GetCondition(apis.ConditionSucceeded) + if condition == nil || condition.Status != corev1.ConditionUnknown { + t.Errorf("Expected fresh TaskRun to have in progress status, but had %v", condition) + } + if condition != nil && condition.Reason != v1beta1.TaskRunReasonRunning.String() { + t.Errorf("Expected reason %q but was %s", v1beta1.TaskRunReasonRunning.String(), condition.Reason) + } } func TestReconcile_verifyResolvedTask_Error(t *testing.T) { - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() - - ts := parse.MustParseV1beta1Task(t, ` + resolverName := "foobar" + unsignedTask := parse.MustParseV1beta1Task(t, ` metadata: name: test-task namespace: foo @@ -4959,66 +4973,73 @@ 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 -`) + unsignedTaskBytes, err := yaml.Marshal(unsignedTask) + if err != nil { + t.Fatal("fail to marshal task", err) + } - signer, _, vps := test.SetupMatchAllVerificationPolicies(t, tr.Namespace) - signedTask, err := test.GetSignedTask(ts, signer, "test-task") + signer, _, vps := test.SetupMatchAllVerificationPolicies(t, unsignedTask.Namespace) + signedTask, err := test.GetSignedTask(unsignedTask, signer, "test-task") if err != nil { t.Fatal("fail to sign task", err) } - tamperedTask := signedTask.DeepCopy() - if tamperedTask.Annotations == nil { - tamperedTask.Annotations = make(map[string]string) + modifiedTask := signedTask.DeepCopy() + if modifiedTask.Annotations == nil { + modifiedTask.Annotations = make(map[string]string) + } + modifiedTask.Annotations["random"] = "attack" + modifiedTaskBytes, err := yaml.Marshal(modifiedTask) + if err != nil { + t.Fatal("fail to marshal task", err) } - tamperedTask.Annotations["random"] = "attack" 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", }, }, } testCases := []struct { - name string - taskrun []*v1beta1.TaskRun - task []*v1beta1.Task - expectedError error + name string + taskBytes []byte }{ { - name: "unsigned task fails verification", - task: []*v1beta1.Task{ts}, - expectedError: trustedresources.ErrResourceVerificationFailed, + name: "unsigned task fails verification", + taskBytes: unsignedTaskBytes, }, { - name: "modified task fails verification", - task: []*v1beta1.Task{tamperedTask}, - expectedError: trustedresources.ErrResourceVerificationFailed, + name: "modified task fails verification", + taskBytes: modifiedTaskBytes, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + tr := parse.MustParseV1beta1TaskRun(t, fmt.Sprintf(` +metadata: + name: test-taskrun + namespace: foo +spec: + params: + - name: myarg + value: foo + taskRef: + resolver: %s + name: test-task +status: + podName: the-pod +`, resolverName)) + rr := getResolvedResolutionRequest(t, resolverName, tc.taskBytes, tr.Namespace, tr.Name) d := test.Data{ TaskRuns: []*v1beta1.TaskRun{tr}, - Tasks: tc.task, ConfigMaps: cms, VerificationPolicies: vps, + ResolutionRequests: []*resolutionv1beta1.ResolutionRequest{&rr}, } testAssets, cancel := getTaskRunController(t, d) @@ -5026,10 +5047,10 @@ status: createServiceAccount(t, testAssets, tr.Spec.ServiceAccountName, tr.Namespace) 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) + if !errors.Is(err, trustedresources.ErrResourceVerificationFailed) { + t.Errorf("Reconcile got %v but want %v", err, trustedresources.ErrResourceVerificationFailed) } - 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) } @@ -5040,3 +5061,23 @@ status: }) } } + +// getResolvedResolutionRequest is a helper function to return the ResolutionRequest and the data is filled with resourceBytes, +// the ResolutionRequest's name is generated by resolverName, namespace and runName. +func getResolvedResolutionRequest(t *testing.T, resolverName string, resourceBytes []byte, namespace string, runName string) resolutionv1beta1.ResolutionRequest { + t.Helper() + name, err := remoteresource.GenerateDeterministicName(resolverName, namespace+"/"+runName, nil) + if err != nil { + t.Errorf("error generating name for %s/%s/%s: %v", resolverName, namespace, runName, err) + } + rr := resolutionv1beta1.ResolutionRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + CreationTimestamp: metav1.Time{Time: time.Now()}, + }, + } + rr.Status.ResolutionRequestStatusFields.Data = base64.StdEncoding.Strict().EncodeToString(resourceBytes) + rr.Status.MarkSucceeded() + return rr +} diff --git a/pkg/trustedresources/verify.go b/pkg/trustedresources/verify.go index 1e6b729f942..2b805278a54 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 contains the source information of the task. +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 contains the source information of the pipeline. +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 {