Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(k8sPSPHostFilesystem): null-check on volumes #595

Merged

Conversation

julianKatz
Copy link
Contributor

A K8sNativeValidation implementation of this template was added in #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.

@julianKatz julianKatz requested a review from a team as a code owner September 4, 2024 20:18
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]>
@julianKatz julianKatz force-pushed the fix-psp-host-filesystem branch from b0d012c to 8caaa18 Compare September 4, 2024 20:19
Copy link
Contributor

@JaydipGabani JaydipGabani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

Copy link
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@julianKatz julianKatz merged commit 799d77b into open-policy-agent:master Sep 4, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants