Skip to content

Commit

Permalink
fix(k8sPSPHostFilesystem): null-check on volumes
Browse files Browse the repository at this point in the history
A K8sNativeValidation implementation of this template was added in open-policy-agent#547.

When testing it, I found that a Pod lacking the `volumes` field would
yield a null-pointer style error on the CEL expression:

```
unexpected number of violations: got 1 violations but want none: got messages [expression '(has(request.operation) && request.operation == "UPDATE") || size(variables.badHostPaths) == 0' resulted in error: composited variable "badHostPaths" fails to evaluate: composited variable "volumes" fails to evaluate: no such key: volumes]
```

This PR adds a `has(` check to prevent that null pointer, and adds a
suite test case that fails without the code change.

Signed-off-by: juliankatz <[email protected]>
  • Loading branch information
julianKatz committed Sep 4, 2024
1 parent 598df74 commit b0d012c
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
apiVersion: constraints.gatekeeper.sh/v1beta1
kind: K8sPSPHostFilesystem
metadata:
name: no-host-paths
spec:
match:
kinds:
- apiGroups: [""]
kinds: ["Pod"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: v1
kind: Pod
metadata:
name: nginx-no-volumes
spec:
containers:
- name: nginx
image: nginx
12 changes: 12 additions & 0 deletions library/pod-security-policy/host-filesystem/suite.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,15 @@ tests:
object: samples/psp-host-filesystem/update.yaml
assertions:
- violations: no
- name: no-host-paths
template: template.yaml
constraint: samples/no-host-paths/constraint.yaml
cases:
- name: previously-allowed-path-disallowed
object: samples/psp-host-filesystem/example_allowed.yaml
assertions:
- violations: yes
- name: no-volumes-is-allowed
object: samples/no-host-paths/example_allowed_no_volumes.yaml
assertions:
- violations: no
2 changes: 1 addition & 1 deletion library/pod-security-policy/host-filesystem/template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ spec:
!has(variables.params.allowedHostPaths) ? [] : variables.params.allowedHostPaths
- name: volumes
expression: |
variables.anyObject.spec.volumes.filter(volume, has(volume.hostPath))
!has(variables.anyObject.spec.volumes) ? [] : variables.anyObject.spec.volumes.filter(volume, has(volume.hostPath))
- name: badHostPaths
expression: |
variables.volumes.filter(volume,
Expand Down
4 changes: 2 additions & 2 deletions src/pod-security-policy/host-filesystem/src.cel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ variables:
!has(variables.params.allowedHostPaths) ? [] : variables.params.allowedHostPaths
- name: volumes
expression: |
variables.anyObject.spec.volumes.filter(volume, has(volume.hostPath))
!has(variables.anyObject.spec.volumes) ? [] : variables.anyObject.spec.volumes.filter(volume, has(volume.hostPath))
- name: badHostPaths
expression: |
variables.volumes.filter(volume,
Expand All @@ -25,4 +25,4 @@ variables:
).map(volume, "{ hostPath: { path : " + volume.hostPath.path + " }, name: " + volume.name + "}").map(volume, "HostPath volume " + volume + " is not allowed, pod: " + object.metadata.name + ". Allowed path: " + variables.allowedPaths.map(path, path.pathPrefix + ", readOnly: " + (path.readOnly ? "true" : "false") + "}").join(", "))
validations:
- expression: '(has(request.operation) && request.operation == "UPDATE") || size(variables.badHostPaths) == 0'
messageExpression: 'variables.badHostPaths.join("\n")'
messageExpression: 'variables.badHostPaths.join("\n")'

0 comments on commit b0d012c

Please sign in to comment.