update taskrun condition based on VerificationResult

This commits updates taskrun condition based on VerificationResult to add TrustedResourcesVerified condition. The condition will be marked as false if verification policy fails, or no matching policies when feature flag is set to fail. The condition will be set to true if verification passes. No condition is added when verification is skipped. Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com
tektoncd · Jun 1, 2023 · ee36af9 · ee36af9
1 parent 5995b54
commit ee36af9
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 20 deletions.
diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go
@@ -345,10 +345,31 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1
 		}
 	}
 
-	if taskMeta.VerificationResult != nil && taskMeta.VerificationResult.VerificationResultType == trustedresources.VerificationError {
-		logger.Errorf("TaskRun %s/%s referred task failed signature verification", tr.Namespace, tr.Name)
-		tr.Status.MarkResourceFailed(podconvert.ReasonResourceVerificationFailed, taskMeta.VerificationResult.Err)
-		return nil, nil, controller.NewPermanentError(taskMeta.VerificationResult.Err)
+	if taskMeta.VerificationResult != nil {
+		switch taskMeta.VerificationResult.VerificationResultType {
+		case trustedresources.VerificationError:
+			logger.Errorf("TaskRun %s/%s referred task failed signature verification", tr.Namespace, tr.Name)
+			tr.Status.MarkResourceFailed(podconvert.ReasonResourceVerificationFailed, taskMeta.VerificationResult.Err)
+			tr.Status.SetCondition(&apis.Condition{
+				Type:    trustedresources.ConditionTrustedResourcesVerified,
+				Status:  corev1.ConditionFalse,
+				Message: taskMeta.VerificationResult.Err.Error(),
+			})
+			return nil, nil, controller.NewPermanentError(taskMeta.VerificationResult.Err)
+		case trustedresources.VerificationSkip:
+			// do nothing
+		case trustedresources.VerificationWarn:
+			tr.Status.SetCondition(&apis.Condition{
+				Type:    trustedresources.ConditionTrustedResourcesVerified,
+				Status:  corev1.ConditionFalse,
+				Message: taskMeta.VerificationResult.Err.Error(),
+			})
+		case trustedresources.VerificationPass:
+			tr.Status.SetCondition(&apis.Condition{
+				Type:   trustedresources.ConditionTrustedResourcesVerified,
+				Status: corev1.ConditionTrue,
+			})
+		}
 	}
 
 	rtr := &resources.ResolvedTask{

diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go
@@ -49,6 +49,7 @@ import (
 	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/pkg/workspace"
 	"github.com/tektoncd/pipeline/test"
 	"github.com/tektoncd/pipeline/test/diff"
@@ -4958,27 +4959,47 @@ spec:
 status:
   podName: the-pod
 `, resolverName))
+
+	failNoMatchCondition := &apis.Condition{
+		Type:    trustedresources.ConditionTrustedResourcesVerified,
+		Status:  corev1.ConditionFalse,
+		Message: fmt.Sprintf("failed to get matched policies: %s: no matching policies are found for resource: %s against source: %s", trustedresources.ErrNoMatchedPolicies, ts.Name, ""),
+	}
+	passCondition := &apis.Condition{
+		Type:   trustedresources.ConditionTrustedResourcesVerified,
+		Status: corev1.ConditionTrue,
+	}
+	failNoKeysCondition := &apis.Condition{
+		Type:    trustedresources.ConditionTrustedResourcesVerified,
+		Status:  corev1.ConditionFalse,
+		Message: fmt.Sprintf("fails to get verifiers for resource %s from namespace %s: %s", ts.Name, ts.Namespace, verifier.ErrEmptyPublicKeys),
+	}
 	testCases := []struct {
-		name                 string
-		task                 []*v1beta1.Task
-		noMatchPolicy        string
-		verificationPolicies []*v1alpha1.VerificationPolicy
+		name                          string
+		task                          []*v1beta1.Task
+		noMatchPolicy                 string
+		verificationPolicies          []*v1alpha1.VerificationPolicy
+		wantTrustedResourcesCondition *apis.Condition
 	}{{
-		name:                 "ignore no match policy",
-		noMatchPolicy:        config.IgnoreNoMatchPolicy,
-		verificationPolicies: noMatchPolicy,
+		name:                          "ignore no match policy",
+		noMatchPolicy:                 config.IgnoreNoMatchPolicy,
+		verificationPolicies:          noMatchPolicy,
+		wantTrustedResourcesCondition: nil,
 	}, {
-		name:                 "warn no match policy",
-		noMatchPolicy:        config.WarnNoMatchPolicy,
-		verificationPolicies: noMatchPolicy,
+		name:                          "warn no match policy",
+		noMatchPolicy:                 config.WarnNoMatchPolicy,
+		verificationPolicies:          noMatchPolicy,
+		wantTrustedResourcesCondition: failNoMatchCondition,
 	}, {
-		name:                 "pass enforce policy",
-		noMatchPolicy:        config.FailNoMatchPolicy,
-		verificationPolicies: vps,
+		name:                          "pass enforce policy",
+		noMatchPolicy:                 config.FailNoMatchPolicy,
+		verificationPolicies:          vps,
+		wantTrustedResourcesCondition: passCondition,
 	}, {
-		name:                 "only fail warn policy",
-		noMatchPolicy:        config.FailNoMatchPolicy,
-		verificationPolicies: warnPolicy,
+		name:                          "only fail warn policy",
+		noMatchPolicy:                 config.FailNoMatchPolicy,
+		verificationPolicies:          warnPolicy,
+		wantTrustedResourcesCondition: failNoKeysCondition,
 	},
 	}
 	for _, tc := range testCases {
@@ -5018,6 +5039,10 @@ status:
 			if condition != nil && condition.Reason != v1beta1.TaskRunReasonRunning.String() {
 				t.Errorf("Expected reason %q but was %s", v1beta1.TaskRunReasonRunning.String(), condition.Reason)
 			}
+			gotVerificationCondition := reconciledRun.Status.GetCondition(trustedresources.ConditionTrustedResourcesVerified)
+			if d := cmp.Diff(tc.wantTrustedResourcesCondition, gotVerificationCondition, ignoreLastTransitionTime); d != "" {
+				t.Error(diff.PrintWantGot(d))
+			}
 		})
 	}
 }
@@ -5122,6 +5147,10 @@ status:
 			if condition.Type != apis.ConditionSucceeded || condition.Status != corev1.ConditionFalse || condition.Reason != podconvert.ReasonResourceVerificationFailed {
 				t.Errorf("Expected TaskRun to fail with reason \"%s\" but it did not. Final conditions were:\n%#v", podconvert.ReasonResourceVerificationFailed, tr.Status.Conditions)
 			}
+			gotVerificationCondition := reconciledRun.Status.GetCondition(trustedresources.ConditionTrustedResourcesVerified)
+			if gotVerificationCondition == nil || gotVerificationCondition.Status != corev1.ConditionFalse {
+				t.Errorf("Expected to have false condition, but had %v", gotVerificationCondition)
+			}
 		})
 	}
 }

diff --git a/pkg/trustedresources/verify.go b/pkg/trustedresources/verify.go
@@ -33,12 +33,15 @@ import (
 	"github.com/tektoncd/pipeline/pkg/trustedresources/verifier"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
+	"knative.dev/pkg/apis"
 	"knative.dev/pkg/logging"
 )
 
 const (
 	// SignatureAnnotation is the key of signature in annotation map
 	SignatureAnnotation = "tekton.dev/signature"
+	// ConditionTrustedResourcesVerified specifies that the resources pass trusted resources verification or not.
+	ConditionTrustedResourcesVerified apis.ConditionType = "TrustedResourcesVerified"
 )
 
 const (