From 3c53da9f751cd7ce71837599c539f99df92b554b Mon Sep 17 00:00:00 2001 From: Kevin Conner Date: Fri, 26 Apr 2024 11:28:10 -0700 Subject: [PATCH] Update error handling to propagate cleanly the nested exception Signed-off-by: Kevin Conner --- k8s/evals.go | 33 ++++++++++++--- k8s/testdata/vap/broken1 policy.yaml | 23 ++++++++++ k8s/testdata/vap/broken1 updated.yaml | 61 +++++++++++++++++++++++++++ k8s/validatingadmissionpolicy_test.go | 24 +++++++++++ 4 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 k8s/testdata/vap/broken1 policy.yaml create mode 100644 k8s/testdata/vap/broken1 updated.yaml diff --git a/k8s/evals.go b/k8s/evals.go index 75ddc00..05ae0be 100644 --- a/k8s/evals.go +++ b/k8s/evals.go @@ -15,6 +15,7 @@ package k8s import ( + "fmt" "reflect" "github.com/google/cel-go/cel" @@ -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 @@ -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, diff --git a/k8s/testdata/vap/broken1 policy.yaml b/k8s/testdata/vap/broken1 policy.yaml new file mode 100644 index 0000000..cfbcb82 --- /dev/null +++ b/k8s/testdata/vap/broken1 policy.yaml @@ -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" diff --git a/k8s/testdata/vap/broken1 updated.yaml b/k8s/testdata/vap/broken1 updated.yaml new file mode 100644 index 0000000..31f17e1 --- /dev/null +++ b/k8s/testdata/vap/broken1 updated.yaml @@ -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 \ No newline at end of file diff --git a/k8s/validatingadmissionpolicy_test.go b/k8s/validatingadmissionpolicy_test.go index a620eca..e2edf7b 100644 --- a/k8s/validatingadmissionpolicy_test.go +++ b/k8s/validatingadmissionpolicy_test.go @@ -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) {