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(k8spspprocmount): exemptImages CEL bug #588

Merged

Conversation

julianKatz
Copy link
Contributor

I recently found (#584) that some K8sNativeValidation implementations of certain templates that iterate over and exempt containers by image had a bug preventing the exemption logic from working.

I've fixed that bug here by mapping from container struct to container.image string. I've also added a suite test to verify this. That case fails without the change to the CEL logic.

@julianKatz julianKatz requested a review from a team as a code owner August 28, 2024 22:00
@julianKatz julianKatz changed the title fix(k8spspprocmount): fix exemptImages support fix(k8spspprocmount): exemptImages CEL bug Aug 28, 2024
@julianKatz julianKatz force-pushed the k8spspprocmount-exemptImages branch from 99079f9 to 74ebb28 Compare August 29, 2024 21:25
@julianKatz
Copy link
Contributor Author

Thanks for the review @JaydipGabani! This is ready for another look.

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

Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

LGTM

I recently found (open-policy-agent#584) that some K8sNativeValidation implementations of
certain templates that iterate over and exempt containers by image had a
bug preventing the exemption logic from working.

I've fixed that bug here by mapping from `container` struct to
`container.image` string.  I've also added a suite test to verify this.
That case fails without the change to the CEL logic.

Signed-off-by: juliankatz <[email protected]>
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 4f6d2f5 into open-policy-agent:master Aug 30, 2024
22 checks passed
@julianKatz julianKatz deleted the k8spspprocmount-exemptImages branch August 30, 2024 20:10
@Markieta
Copy link

Markieta commented Aug 30, 2024

@julianKatz I just pulled this update and it seems to break functionality getting the following violations:

failed expression: (has(request.operation) && request.operation == "UPDATE") || size(variables.badContainers) == 0

Also some variants of:

ProcMount type is not allowed, container: kube-proxy. Allowed procMount types: default

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.

5 participants