Skip to content

Commit

Permalink
Update error handling to propagate cleanly the nested exception
Browse files Browse the repository at this point in the history
Signed-off-by: Kevin Conner <[email protected]>
  • Loading branch information
knrc committed Apr 26, 2024
1 parent 338273a commit 3c53da9
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 6 deletions.
33 changes: 27 additions & 6 deletions k8s/evals.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package k8s

import (
"fmt"
"reflect"

"github.com/google/cel-go/cel"
Expand All @@ -24,6 +25,32 @@ import (
"google.golang.org/protobuf/types/known/structpb"
)

type evalResponseError struct {
error
cause error
}

func newEvalResponseErr(operation, expression string, err error) *evalResponse {
switch celErr := err.(type) {
case *types.Err:
underlying := celErr.Unwrap()
switch evalErr := underlying.(type) {
case *evalResponseError:
return &evalResponse{
val: types.WrapErr(&evalResponseError{fmt.Errorf("unexpected error %s expression '%s', caused by nested exception: '%s'", operation, expression, evalErr.cause), evalErr.cause}),
}
default:
return &evalResponse{
val: types.WrapErr(&evalResponseError{fmt.Errorf("unexpected error %s expression %s: %s", operation, expression, underlying), underlying}),
}
}
default:
return &evalResponse{
val: types.WrapErr(&evalResponseError{fmt.Errorf("unexpected error %s expression %s: %s", operation, expression, err), err}),
}
}
}

type evalResponse struct {
name string
val ref.Val
Expand All @@ -34,12 +61,6 @@ type evalResponse struct {

type evalResponses []*evalResponse

func newEvalResponseErr(operation, expression string, err error) *evalResponse {
return &evalResponse{
val: types.NewErr("Unexpected error %s expression %s: %v", operation, expression, err),
}
}

func newEvalResponse(name string, exprEval ref.Val, details *cel.EvalDetails, message string, messageVal ref.Val) *evalResponse {
return &evalResponse{
name: name,
Expand Down
23 changes: 23 additions & 0 deletions k8s/testdata/vap/broken1 policy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
apiVersion: admissionregistration.k8s.io/v1alpha1
kind: ValidatingAdmissionPolicy
metadata:
name: "test-variable-access"
spec:
failurePolicy: Fail
matchConstraints:
resourceRules:
- apiGroups: ["apps"]
apiVersions: ["v1"]
operations: ["CREATE", "UPDATE"]
resources: ["deployments"]
variables:
- name: foo
expression: "'foo' in object.spec.template.metadata.labels ? object.spec.template.metadata.labels['foo'] : 'default'"
- name: containers
# deliberately misspelling 'spec' as 'spc'
expression: "object.spec.template.spc.containers"
validations:
- expression: variables.foo == 'default' && variables.containers.all(c, c.image.startsWith("test"))
auditAnnotations:
- key: "foo-label"
valueExpression: "'Label for foo is set to ' + variables.foo"
61 changes: 61 additions & 0 deletions k8s/testdata/vap/broken1 updated.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
apiVersion: apps/v1
kind: Deployment
metadata:
annotations:
deployment.kubernetes.io/revision: "1"
creationTimestamp: "2023-10-02T15:26:06Z"
generation: 1
labels:
app: kubernetes-bootcamp
name: kubernetes-bootcamp
namespace: default
resourceVersion: "246826"
uid: dcdda63b-1611-467d-8927-43e3c73bc963
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
app: kubernetes-bootcamp
strategy:
rollingUpdate:
maxSurge: 25%
maxUnavailable: 25%
type: RollingUpdate
template:
metadata:
creationTimestamp: null
labels:
app: kubernetes-bootcamp
spec:
containers:
- image: gcr.io/google-samples/kubernetes-bootcamp:v1
imagePullPolicy: IfNotPresent
name: kubernetes-bootcamp
resources: {}
terminationMessagePath: /dev/termination-log
terminationMessagePolicy: File
dnsPolicy: ClusterFirst
restartPolicy: Always
schedulerName: default-scheduler
securityContext: {}
terminationGracePeriodSeconds: 30
status:
conditions:
- lastTransitionTime: "2023-10-02T15:26:08Z"
lastUpdateTime: "2023-10-02T15:26:08Z"
message: Deployment does not have minimum availability.
reason: MinimumReplicasUnavailable
status: "False"
type: Available
- lastTransitionTime: "2023-10-02T15:26:07Z"
lastUpdateTime: "2023-10-02T15:26:10Z"
message: ReplicaSet "kubernetes-bootcamp-855d5cc575" is progressing.
reason: ReplicaSetUpdated
status: "True"
type: Progressing
observedGeneration: 1
replicas: 1
unavailableReplicas: 1
updatedReplicas: 1
24 changes: 24 additions & 0 deletions k8s/validatingadmissionpolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,30 @@ func TestValidationEval(t *testing.T) {
}},
Cost: uint64ptr(19),
},
}, {
name: "test a broken expression within variables, expression should fail with no audit annotation",
policy: "broken1 policy.yaml",
orig: "",
updated: "broken1 updated.yaml",
expected: k8s.EvalResponse{
ValidationVariables: []*k8s.EvalVariable{{
Name: "foo",
Value: "default",
Cost: uint64ptr(6),
}, {
Name: "containers",
Error: strptr("unexpected error evaluating expression containers: no such key: spc"),
}},
Validations: []*k8s.EvalResult{{
Error: strptr("unexpected error evaluating expression 'variables.foo == 'default' && variables.containers.all(c, c.image.startsWith(\"test\"))', caused by nested exception: 'no such key: spc'"),
}},
AuditAnnotations: []*k8s.EvalResult{{
Name: strptr("foo-label"),
Message: "Label for foo is set to default",
Cost: uint64ptr(2),
}},
Cost: uint64ptr(8),
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit 3c53da9

Please sign in to comment.