move trusted resources verification after we resolve the remote resou…

…rces This PR moves the trusted resources verification to readRuntimeObjectAsTask and readRuntimeObjectAsPipline, the reasons we need this change include 1) unblock the work for v1, since v1 will mutate, validate and convert the resources, the mutation will break trusted resources verification thus we need to verify right after we resolve the remote resources. 2) Prepare the support for verifying different api versions. This commit also makes it clear that currently we only support verification for remote resources. Signed-off-by: Yongxuan Zhang [email protected]
tektoncd · May 5, 2023 · 7ddb9a4 · 7ddb9a4
1 parent 09d422c
commit 7ddb9a4
Show file tree

Hide file tree

Showing 11 changed files with 396 additions and 254 deletions.
diff --git 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
@@ -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
@@ -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