From f95fa034a8560a03bb3acb8e551cf8f5a6f5f5f5 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Thu, 1 Jun 2023 22:44:49 +0000 Subject: [PATCH] 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 --- docs/trusted-resources.md | 43 ++++++++++++++++++ pkg/reconciler/taskrun/taskrun.go | 29 ++++++++++-- pkg/reconciler/taskrun/taskrun_test.go | 61 +++++++++++++++++++------- pkg/trustedresources/verify.go | 3 ++ 4 files changed, 116 insertions(+), 20 deletions(-) diff --git a/docs/trusted-resources.md b/docs/trusted-resources.md index 282f7dce654..1ba5f58656b 100644 --- a/docs/trusted-resources.md +++ b/docs/trusted-resources.md @@ -73,6 +73,49 @@ Or patch the new values: kubectl patch configmap feature-flags -n tekton-pipelines -p='{"data":{"trusted-resources-verification-no-match-policy":"fail"}} ``` + #### TaskRun status update +Trusted resources will update the taskrun'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 +``` + #### Config key at VerificationPolicy VerificationPolicy supports SecretRef or encoded public key data. diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 100ee1dd258..75f6dcd7e8b 100644 --- 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 index 0bbf49b5ecc..131b534c310 100644 --- 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 index cdf1c23a02e..971348285e0 100644 --- 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 (