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 2, 2023
1 parent 5995b54 commit 1fe170b
Show file tree
Hide file tree
Showing 6 changed files with 209 additions and 45 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
56 changes: 48 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,30 @@ 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
switch verificationResult.VerificationResultType {
case trustedresources.VerificationError:
err := fmt.Errorf("PipelineRun %s/%s referred task %s failed signature verification: %w", pr.Namespace, pr.Name, task.Name, verificationResult.Err)
pr.Status.MarkFailed(ReasonResourceVerificationFailed, err.Error())
return nil, controller.NewPermanentError(vr.Err)
pr.Status.SetCondition(&apis.Condition{
Type: trustedresources.ConditionTrustedResourcesVerified,
Status: corev1.ConditionFalse,
Message: err.Error(),
})
return nil, controller.NewPermanentError(err)
case trustedresources.VerificationSkip:
// do nothing
case trustedresources.VerificationWarn:
pr.Status.SetCondition(&apis.Condition{
Type: trustedresources.ConditionTrustedResourcesVerified,
Status: corev1.ConditionFalse,
Message: verificationResult.Err.Error(),
})
case trustedresources.VerificationPass:
pr.Status.SetCondition(&apis.Condition{
Type: trustedresources.ConditionTrustedResourcesVerified,
Status: corev1.ConditionTrue,
})
}
}
pst = append(pst, resolvedTask)
Expand Down Expand Up @@ -419,10 +438,31 @@ 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
switch verificationResult.VerificationResultType {
case trustedresources.VerificationError:
err := fmt.Errorf("PipelineRun %s/%s referred pipeline failed signature verification: %w", pr.Namespace, pr.Name, verificationResult.Err)
pr.Status.MarkFailed(ReasonResourceVerificationFailed, err.Error())
pr.Status.SetCondition(&apis.Condition{
Type: trustedresources.ConditionTrustedResourcesVerified,
Status: corev1.ConditionFalse,
Message: err.Error(),
})
return controller.NewPermanentError(err)
case trustedresources.VerificationSkip:
// do nothing
case trustedresources.VerificationWarn:
pr.Status.SetCondition(&apis.Condition{
Type: trustedresources.ConditionTrustedResourcesVerified,
Status: corev1.ConditionFalse,
Message: verificationResult.Err.Error(),
})
case trustedresources.VerificationPass:
pr.Status.SetCondition(&apis.Condition{
Type: trustedresources.ConditionTrustedResourcesVerified,
})
}
}

d, err := dag.Build(v1beta1.PipelineTaskList(pipelineSpec.Tasks), v1beta1.PipelineTaskList(pipelineSpec.Tasks).Deps())
Expand Down
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("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 {
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("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 {
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
3 changes: 3 additions & 0 deletions 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

0 comments on commit 1fe170b

Please sign in to comment.