diff --git a/docs/trusted-resources.md b/docs/trusted-resources.md index 282f7dce654..f8fca4f74d5 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 and PipelineRun 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/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index dee03436b64..4c20b536a21 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -375,11 +375,11 @@ func (c *Reconciler) resolvePipelineState( return nil, controller.NewPermanentError(err) } if resolvedTask.ResolvedTask != nil && resolvedTask.ResolvedTask.VerificationResult != nil { - vr := resolvedTask.ResolvedTask.VerificationResult - if vr.VerificationResultType == trustedresources.VerificationError { - err := fmt.Errorf("PipelineRun %s/%s referred pipeline failed signature verification for task %s : %w", pr.Namespace, pr.Name, resolvedTask.ResolvedTask.TaskName, vr.Err) + cond, err := conditionFromVerificationResult(resolvedTask.ResolvedTask.VerificationResult, pr, task.Name) + pr.Status.SetCondition(cond) + if err != nil { pr.Status.MarkFailed(ReasonResourceVerificationFailed, err.Error()) - return nil, controller.NewPermanentError(vr.Err) + return nil, controller.NewPermanentError(err) } } pst = append(pst, resolvedTask) @@ -419,10 +419,13 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get } } - if pipelineMeta.VerificationResult != nil && pipelineMeta.VerificationResult.VerificationResultType == trustedresources.VerificationError { - err := fmt.Errorf("PipelineRun %s/%s referred pipeline failed signature verification: %w", pr.Namespace, pr.Name, pipelineMeta.VerificationResult.Err) - pr.Status.MarkFailed(ReasonResourceVerificationFailed, err.Error()) - return controller.NewPermanentError(err) + if pipelineMeta.VerificationResult != nil { + cond, err := conditionFromVerificationResult(pipelineMeta.VerificationResult, pr, pipelineMeta.Name) + pr.Status.SetCondition(cond) + if err != nil { + pr.Status.MarkFailed(ReasonResourceVerificationFailed, err.Error()) + return controller.NewPermanentError(err) + } } d, err := dag.Build(v1beta1.PipelineTaskList(pipelineSpec.Tasks), v1beta1.PipelineTaskList(pipelineSpec.Tasks).Deps()) @@ -1411,3 +1414,32 @@ func updatePipelineRunStatusFromChildRefs(logger *zap.SugaredLogger, pr *v1beta1 } pr.Status.ChildReferences = newChildRefs } + +// conditionFromVerificationResult returns the ConditionTrustedResourcesVerified condition based on the VerificationResult, err is returned when the VerificationResult type is VerificationError +func conditionFromVerificationResult(verificationResult *trustedresources.VerificationResult, pr *v1beta1.PipelineRun, resourceName string) (*apis.Condition, error) { + var condition *apis.Condition + var err error + switch verificationResult.VerificationResultType { + case trustedresources.VerificationError: + err = fmt.Errorf("PipelineRun %s/%s referred resource %s failed signature verification: %w", pr.Namespace, pr.Name, resourceName, verificationResult.Err) + condition = &apis.Condition{ + Type: trustedresources.ConditionTrustedResourcesVerified, + Status: corev1.ConditionFalse, + Message: err.Error(), + } + case trustedresources.VerificationWarn: + condition = &apis.Condition{ + Type: trustedresources.ConditionTrustedResourcesVerified, + Status: corev1.ConditionFalse, + Message: verificationResult.Err.Error(), + } + case trustedresources.VerificationPass: + condition = &apis.Condition{ + Type: trustedresources.ConditionTrustedResourcesVerified, + Status: corev1.ConditionTrue, + } + case trustedresources.VerificationSkip: + // do nothing + } + return condition, err +} diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index edbf15bf241..004916d8b91 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -46,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" @@ -11609,28 +11611,46 @@ spec: pipelineRef: resolver: %s `, 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("failed 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 { @@ -11659,6 +11679,10 @@ spec: reconciledRun, _ := prt.reconcileRun("foo", "test-pipelinerun", []string{}, false) checkPipelineRunConditionStatusAndReason(t, reconciledRun, corev1.ConditionUnknown, v1beta1.PipelineRunReasonRunning.String()) + gotVerificationCondition := reconciledRun.Status.GetCondition(trustedresources.ConditionTrustedResourcesVerified) + if d := cmp.Diff(tc.wantTrustedResourcesCondition, gotVerificationCondition, ignoreLastTransitionTime); d != "" { + t.Error(diff.PrintWantGot(d)) + } }) } } @@ -11838,6 +11862,10 @@ spec: reconciledRun, _ := prt.reconcileRun("foo", "test-pipelinerun", []string{}, true) checkPipelineRunConditionStatusAndReason(t, reconciledRun, corev1.ConditionFalse, ReasonResourceVerificationFailed) + 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/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..a5bd2ab7a06 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("failed 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 ccd1c0cc65c..788e8ad595d 100644 --- a/pkg/trustedresources/verify.go +++ b/pkg/trustedresources/verify.go @@ -34,12 +34,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 ( @@ -205,7 +208,7 @@ func verifyResource(ctx context.Context, resource metav1.Object, k8s kubernetes. for _, p := range warnPolicies { verifiers, err := verifier.FromPolicy(ctx, k8s, p) if err != nil { - warn := fmt.Errorf("fails to get verifiers for resource %s from namespace %s: %w", resource.GetName(), resource.GetNamespace(), err) + warn := fmt.Errorf("failed to get verifiers for resource %s from namespace %s: %w", resource.GetName(), resource.GetNamespace(), err) logger.Warnf(warn.Error()) return VerificationResult{VerificationResultType: VerificationWarn, Err: warn} }