[TEP-0091] add condition for trusted resources

This commit adds the ConditionTrustedResourcesVerified for trusted
resources into pipelinerun/taskrun status to show if the resources passes verification. Failing
to verify will add a false condition to the taskrun/pipelinerun
status and passing the verification will add a true status condition.

Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com

docs/trusted-resources.md

@@ -67,6 +67,49 @@ data:
  **Notes:**
  * To skip the verification: make sure no policies exist and `trusted-resources-verification-no-match-policy` is set to `warn` or `ignore`.
  * To enable the verification: install [VerificationPolicy](#config-key-at-verificationpolicy) to match the resources.
+ * Setting the feature flag will have impact on the [condition].(#taskrun-and-pipelinerun-status-update)
+
+ #### TaskRun and PipelineRun status update
+Trusted resources will update the taskrun/pipelinerun’s [condition](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties) to indicate if it passes verification or not.
+
+The following tables illustrate how the conditions are impacted by feature flag and verification result. Note that if not `true` or `false` means this case doesn't update the corresponding condition.
+**No Matching Policies:**
+|                             | `Conditions.TrustedResourcesVerified` | `Conditions.Succeeded` |
+|-----------------------------|---------------------------------------|------------------------|
+| `no-match-policy`: "ignore" |                                       |                        |
+| `no-match-policy`: "warn"   | False                                 |                        |
+| `no-match-policy`: "fail"   | False                                 | False                  |
+
+**Matching Policies(no matter what `trusted-resources-verification-no-match-policy` value is):**
+|                          | `Conditions.TrustedResourcesVerified` | `Conditions.Succeeded` |
+|--------------------------|---------------------------------------|------------------------|
+| all policies pass        | True                                  |                        |
+| any enforce policy fails | False                                 | False                  |
+| only warn policies fail  | False                                 |                        |
+
+A successful sample `TrustedResourcesVerified` condition is:
+```yaml
+status:
+  conditions:
+  - lastTransitionTime: "2023-03-01T18:17:05Z"
+    message: Trusted resource verification passed
+    status: "True"
+    type: TrustedResourcesVerified
+```
+
+Failed sample `TrustedResourcesVerified` and `Succeeded` conditions are:
+```yaml
+status:
+  conditions:
+  - lastTransitionTime: "2023-03-01T18:17:05Z"
+    message: resource verification failed # This will be filled with detailed error message.
+    status: "False"
+    type: TrustedResourcesVerified
+  - lastTransitionTime: "2023-03-01T18:17:10Z"
+    message: resource verification failed
+    status: "False"
+    type: Succeeded
+```
 
 Or patch the new values:
 ```bash

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -346,7 +346,7 @@ func (c *Reconciler) resolvePipelineState(
 			return r, nil
 		}
 
-		resolvedTask, err := resources.ResolvePipelineTask(ctx,
+		resolvedTask, verificationResult, err := resources.ResolvePipelineTask(ctx,
 			*pr,
 			fn,
 			func(name string) (*v1beta1.TaskRun, error) {
@@ -365,6 +365,11 @@ func (c *Reconciler) resolvePipelineState(
 			if errors.Is(err, trustedresources.ErrResourceVerificationFailed) {
 				message := fmt.Sprintf("PipelineRun %s/%s referred task %s failed signature verification", pr.Namespace, pr.Name, task.Name)
 				pr.Status.MarkFailed(ReasonResourceVerificationFailed, message)
+				pr.Status.SetCondition(&apis.Condition{
+					Type:    trustedresources.ConditionTrustedResourcesVerified,
+					Status:  corev1.ConditionFalse,
+					Message: message,
+				})
 				return nil, controller.NewPermanentError(err)
 			}
 			var nfErr *resources.TaskNotFoundError
@@ -379,6 +384,25 @@ func (c *Reconciler) resolvePipelineState(
 			}
 			return nil, controller.NewPermanentError(err)
 		}
+		if verificationResult != nil {
+			switch verificationResult.VerificationResultType {
+			case trustedresources.IgnoreNoMatchPolicy:
+				// do nothing
+			case trustedresources.WarnNoMatchPolicy, trustedresources.WarnModeVerificationFail:
+				pr.Status.SetCondition(&apis.Condition{
+					Type:    trustedresources.ConditionTrustedResourcesVerified,
+					Status:  corev1.ConditionFalse,
+					Message: verificationResult.Message,
+				})
+			case trustedresources.VerificationPass:
+				pr.Status.SetCondition(&apis.Condition{
+					Type:    trustedresources.ConditionTrustedResourcesVerified,
+					Status:  corev1.ConditionTrue,
+					Message: verificationResult.Message,
+				})
+			}
+		}
+
 		pst = append(pst, resolvedTask)
 	}
 	return pst, nil
@@ -397,7 +421,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
 		return nil
 	}
 
-	pipelineMeta, pipelineSpec, err := rprp.GetPipelineData(ctx, pr, getPipelineFunc)
+	pipelineMeta, pipelineSpec, verificationResult, err := rprp.GetPipelineData(ctx, pr, getPipelineFunc)
 	switch {
 	case errors.Is(err, remote.ErrRequestInProgress):
 		message := fmt.Sprintf("PipelineRun %s/%s awaiting remote resource", pr.Namespace, pr.Name)
@@ -406,6 +430,11 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
 	case errors.Is(err, trustedresources.ErrResourceVerificationFailed):
 		message := fmt.Sprintf("PipelineRun %s/%s referred pipeline failed signature verification", pr.Namespace, pr.Name)
 		pr.Status.MarkFailed(ReasonResourceVerificationFailed, message)
+		pr.Status.SetCondition(&apis.Condition{
+			Type:    trustedresources.ConditionTrustedResourcesVerified,
+			Status:  corev1.ConditionFalse,
+			Message: message,
+		})
 		return controller.NewPermanentError(err)
 	case err != nil:
 		logger.Errorf("Failed to determine Pipeline spec to use for pipelinerun %s: %v", pr.Name, err)
@@ -419,6 +448,24 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
 			logger.Errorf("Failed to store PipelineSpec on PipelineRun.Status for pipelinerun %s: %v", pr.Name, err)
 		}
 	}
+	if verificationResult != nil {
+		switch verificationResult.VerificationResultType {
+		case trustedresources.IgnoreNoMatchPolicy:
+			// do nothing
+		case trustedresources.WarnNoMatchPolicy, trustedresources.WarnModeVerificationFail:
+			pr.Status.SetCondition(&apis.Condition{
+				Type:    trustedresources.ConditionTrustedResourcesVerified,
+				Status:  corev1.ConditionFalse,
+				Message: verificationResult.Message,
+			})
+		case trustedresources.VerificationPass:
+			pr.Status.SetCondition(&apis.Condition{
+				Type:    trustedresources.ConditionTrustedResourcesVerified,
+				Status:  corev1.ConditionTrue,
+				Message: verificationResult.Message,
+			})
+		}
+	}
 
 	d, err := dag.Build(v1beta1.PipelineTaskList(pipelineSpec.Tasks), v1beta1.PipelineTaskList(pipelineSpec.Tasks).Deps())
 	if err != nil {

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -35,6 +35,7 @@ import (
 	"github.com/google/go-containerregistry/pkg/registry"
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	"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"
@@ -45,6 +46,8 @@ import (
 	"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/trustedresources/verifier"
 	"github.com/tektoncd/pipeline/test"
 	"github.com/tektoncd/pipeline/test/diff"
 	"github.com/tektoncd/pipeline/test/names"
@@ -11394,8 +11397,75 @@ spec:
 	if err != nil {
 		t.Fatal("fail to marshal task", err)
 	}
-
-	prs := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(`
+	noMatchPolicy := []*v1alpha1.VerificationPolicy{{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "no-match",
+			Namespace: ts.Namespace,
+		},
+		Spec: v1alpha1.VerificationPolicySpec{
+			Resources: []v1alpha1.ResourcePattern{{Pattern: "no-match"}},
+		}}}
+	warnPolicy := []*v1alpha1.VerificationPolicy{{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "warn-policy",
+			Namespace: ts.Namespace,
+		},
+		Spec: v1alpha1.VerificationPolicySpec{
+			Resources: []v1alpha1.ResourcePattern{{Pattern: ".*"}},
+			Mode:      v1alpha1.ModeWarn,
+		}}}
+	failNoMatchCondition := &apis.Condition{
+		Type:    trustedresources.ConditionTrustedResourcesVerified,
+		Status:  corev1.ConditionFalse,
+		Message: trustedresources.ErrNoMatchedPolicies.Error(),
+	}
+	passCondition := &apis.Condition{
+		Type:   trustedresources.ConditionTrustedResourcesVerified,
+		Status: corev1.ConditionTrue,
+	}
+	failNoKeysCondition := &apis.Condition{
+		Type:    trustedresources.ConditionTrustedResourcesVerified,
+		Status:  corev1.ConditionFalse,
+		Message: verifier.ErrEmptyPublicKeys.Error(),
+	}
+	/*
+		This test covers 4 cases where the trusted resources return no error.
+		1. no matching policies and no-match-policy ignore
+		2. no matching policies and no-match-policy warn
+		3. all policies pass
+		4. only warn policies fail
+	*/
+	testCases := []struct {
+		name                              string
+		task                              []*v1beta1.Task
+		noMatchPolicy                     string
+		verificationPolicies              []*v1alpha1.VerificationPolicy
+		expectedTrustedResourcesCondition *apis.Condition
+	}{{
+		name:                              "ignore no match policy",
+		noMatchPolicy:                     config.IgnoreNoMatchPolicy,
+		verificationPolicies:              noMatchPolicy,
+		expectedTrustedResourcesCondition: nil,
+	}, {
+		name:                              "warn no match policy",
+		noMatchPolicy:                     config.WarnNoMatchPolicy,
+		verificationPolicies:              noMatchPolicy,
+		expectedTrustedResourcesCondition: failNoMatchCondition,
+	}, {
+		name:                              "pass enforce policy",
+		noMatchPolicy:                     config.FailNoMatchPolicy,
+		verificationPolicies:              vps,
+		expectedTrustedResourcesCondition: passCondition,
+	}, {
+		name:                              "only fail warn policy",
+		noMatchPolicy:                     config.FailNoMatchPolicy,
+		verificationPolicies:              warnPolicy,
+		expectedTrustedResourcesCondition: failNoKeysCondition,
+	},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			pr := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(`
 metadata:
   name: test-pipelinerun
   namespace: foo
@@ -11405,30 +11475,36 @@ spec:
     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,
-			},
-		},
-	}
+			cms := []*corev1.ConfigMap{
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()},
+					Data: map[string]string{
+						"trusted-resources-verification-no-match-policy": tc.noMatchPolicy,
+					},
+				},
+			}
 
-	pipelineReq := getResolvedResolutionRequest(t, resolverName, signedPipelineBytes, prs.Namespace, prs.Name)
-	taskReq := getResolvedResolutionRequest(t, resolverName, signedTaskBytes, prs.Namespace, prs.Name+"-"+ps.Spec.Tasks[0].Name)
+			pipelineReq := getResolvedResolutionRequest(t, resolverName, signedPipelineBytes, pr.Namespace, pr.Name)
+			taskReq := getResolvedResolutionRequest(t, resolverName, signedTaskBytes, pr.Namespace, pr.Name+"-"+ps.Spec.Tasks[0].Name)
 
-	d := test.Data{
-		PipelineRuns:         []*v1beta1.PipelineRun{prs},
-		VerificationPolicies: vps,
-		ConfigMaps:           cms,
-		ResolutionRequests:   []*resolutionv1beta1.ResolutionRequest{&pipelineReq, &taskReq},
-	}
-	prt := newPipelineRunTest(t, d)
-	defer prt.Cancel()
+			d := test.Data{
+				PipelineRuns:         []*v1beta1.PipelineRun{pr},
+				VerificationPolicies: tc.verificationPolicies,
+				ConfigMaps:           cms,
+				ResolutionRequests:   []*resolutionv1beta1.ResolutionRequest{&pipelineReq, &taskReq},
+			}
+			prt := newPipelineRunTest(t, d)
+			defer prt.Cancel()
 
-	reconciledRun, _ := prt.reconcileRun("foo", "test-pipelinerun", []string{}, false)
+			reconciledRun, _ := prt.reconcileRun("foo", "test-pipelinerun", []string{}, false)
 
-	checkPipelineRunConditionStatusAndReason(t, reconciledRun, corev1.ConditionUnknown, v1beta1.PipelineRunReasonRunning.String())
+			checkPipelineRunConditionStatusAndReason(t, reconciledRun, corev1.ConditionUnknown, v1beta1.PipelineRunReasonRunning.String())
+			verificationCondition := reconciledRun.Status.GetCondition(trustedresources.ConditionTrustedResourcesVerified)
+			if d := cmp.Diff(tc.expectedTrustedResourcesCondition, verificationCondition, ignoreLastTransitionTime); d != "" {
+				t.Error(diff.PrintWantGot(d))
+			}
+		})
+	}
 }
 
 func TestReconcile_verifyResolvedPipeline_Error(t *testing.T) {
@@ -11606,6 +11682,10 @@ spec:
 
 			reconciledRun, _ := prt.reconcileRun("foo", "test-pipelinerun", []string{}, true)
 			checkPipelineRunConditionStatusAndReason(t, reconciledRun, corev1.ConditionFalse, ReasonResourceVerificationFailed)
+			verificationCondition := reconciledRun.Status.GetCondition(trustedresources.ConditionTrustedResourcesVerified)
+			if verificationCondition == nil || verificationCondition.Status != corev1.ConditionFalse {
+				t.Errorf("Expected to have false condition, but had %v", verificationCondition)
+			}
 		})
 	}
 }

pkg/reconciler/pipelinerun/pipelinespec/pipelinespec.go

@@ -23,53 +23,57 @@ import (
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	resolutionutil "github.com/tektoncd/pipeline/pkg/internal/resolution"
+	"github.com/tektoncd/pipeline/pkg/trustedresources"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // GetPipeline is a function used to retrieve Pipelines.
-type GetPipeline func(context.Context, string) (*v1beta1.Pipeline, *v1beta1.RefSource, error)
+type GetPipeline func(context.Context, string) (*v1beta1.Pipeline, *v1beta1.RefSource, *trustedresources.VerificationResult, error)
 
 // GetPipelineData will retrieve the Pipeline metadata and Spec associated with the
 // provided PipelineRun. This can come from a reference Pipeline or from the PipelineRun's
 // metadata and embedded PipelineSpec.
-func GetPipelineData(ctx context.Context, pipelineRun *v1beta1.PipelineRun, getPipeline GetPipeline) (*resolutionutil.ResolvedObjectMeta, *v1beta1.PipelineSpec, error) {
+func GetPipelineData(ctx context.Context, pipelineRun *v1beta1.PipelineRun, getPipeline GetPipeline) (*resolutionutil.ResolvedObjectMeta, *v1beta1.PipelineSpec, *trustedresources.VerificationResult, error) {
 	pipelineMeta := metav1.ObjectMeta{}
 	var refSource *v1beta1.RefSource
+	var verificationResult *trustedresources.VerificationResult
 	pipelineSpec := v1beta1.PipelineSpec{}
 	switch {
 	case pipelineRun.Spec.PipelineRef != nil && pipelineRun.Spec.PipelineRef.Name != "":
 		// Get related pipeline for pipelinerun
-		p, source, err := getPipeline(ctx, pipelineRun.Spec.PipelineRef.Name)
+		p, source, vr, err := getPipeline(ctx, pipelineRun.Spec.PipelineRef.Name)
 		if err != nil {
-			return nil, nil, fmt.Errorf("error when listing pipelines for pipelineRun %s: %w", pipelineRun.Name, err)
+			return nil, nil, nil, fmt.Errorf("error when listing pipelines for pipelineRun %s: %w", pipelineRun.Name, err)
 		}
 		pipelineMeta = p.PipelineMetadata()
 		pipelineSpec = p.PipelineSpec()
 		refSource = source
+		verificationResult = vr
 	case pipelineRun.Spec.PipelineSpec != nil:
 		pipelineMeta = pipelineRun.ObjectMeta
 		pipelineSpec = *pipelineRun.Spec.PipelineSpec
 		// TODO: if we want to set RefSource for embedded pipeline, set it here.
 		// https://github.com/tektoncd/pipeline/issues/5522
 	case pipelineRun.Spec.PipelineRef != nil && pipelineRun.Spec.PipelineRef.Resolver != "":
-		pipeline, source, err := getPipeline(ctx, "")
+		pipeline, source, vr, err := getPipeline(ctx, "")
 		switch {
 		case err != nil:
-			return nil, nil, err
+			return nil, nil, nil, err
 		case pipeline == nil:
-			return nil, nil, errors.New("resolution of remote resource completed successfully but no pipeline was returned")
+			return nil, nil, nil, errors.New("resolution of remote resource completed successfully but no pipeline was returned")
 		default:
 			pipelineMeta = pipeline.PipelineMetadata()
 			pipelineSpec = pipeline.PipelineSpec()
+			verificationResult = vr
 		}
 		refSource = source
 	default:
-		return nil, nil, fmt.Errorf("pipelineRun %s not providing PipelineRef or PipelineSpec", pipelineRun.Name)
+		return nil, nil, nil, fmt.Errorf("pipelineRun %s not providing PipelineRef or PipelineSpec", pipelineRun.Name)
 	}
 
 	pipelineSpec.SetDefaults(ctx)
 	return &resolutionutil.ResolvedObjectMeta{
 		ObjectMeta: &pipelineMeta,
 		RefSource:  refSource,
-	}, &pipelineSpec, nil
+	}, &pipelineSpec, verificationResult, nil
 }