Skip to content

Commit

Permalink
update taskrun and pipelinerun condition based on VerificationResult
Browse files Browse the repository at this point in the history 
This commits updates taskrun and pipelinerun 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 [email protected]
  • Loading branch information
@Yongxuanzhang
Yongxuanzhang committed Jun 5, 2023
1 parent 5995b54 commit d080cf3
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 46 deletions.
43 changes: 43 additions & 0 deletions docs/trusted-resources.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
47 changes: 39 additions & 8 deletions pkg/reconciler/pipelinerun/pipelinerun.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,11 +375,12 @@ 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)
verificationResult := resolvedTask.ResolvedTask.VerificationResult
cond, err := conditionFromVerificationResult(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)
Expand Down Expand Up @@ -419,10 +420,14 @@ 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 {
verificationResult := pipelineMeta.VerificationResult
cond, err := conditionFromVerificationResult(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())
Expand Down Expand Up @@ -1411,3 +1416,29 @@ func updatePipelineRunStatusFromChildRefs(logger *zap.SugaredLogger, pr *v1beta1
}
pr.Status.ChildReferences = newChildRefs
}

func conditionFromVerificationResult(verificationResult *trustedresources.VerificationResult, pr *v1beta1.PipelineRun, resourceName string) (*apis.Condition, 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)
return &apis.Condition{
Type: trustedresources.ConditionTrustedResourcesVerified,
Status: corev1.ConditionFalse,
Message: err.Error(),
}, controller.NewPermanentError(err)
case trustedresources.VerificationWarn:
return &apis.Condition{
Type: trustedresources.ConditionTrustedResourcesVerified,
Status: corev1.ConditionFalse,
Message: verificationResult.Err.Error(),
}, nil
case trustedresources.VerificationPass:
return &apis.Condition{
Type: trustedresources.ConditionTrustedResourcesVerified,
Status: corev1.ConditionTrue,
}, nil
default:
// trustedresources.VerificationSkip falls into default, for this case we don't update the status
return nil, nil
}
}
62 changes: 45 additions & 17 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
}
})
}
}
Expand Down Expand Up @@ -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)
}
})
}
}
Expand Down
29 changes: 25 additions & 4 deletions pkg/reconciler/taskrun/taskrun.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
61 changes: 45 additions & 16 deletions pkg/reconciler/taskrun/taskrun_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
}
})
}
}
Expand Down Expand Up @@ -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)
}
})
}
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/trustedresources/verify.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -191,7 +194,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}
}
Expand Down

0 comments on commit d080cf3

Please sign in to comment.