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(k8srequiredlabels): CEL broke when allowedRegex was empty #583

Merged

Conversation

julianKatz
Copy link
Contributor

The CEL implementation of this policy always expected the allowedRegex value to be set for a given key value in the contraint parameters.

Existing users of the template could (via rego) require only that a given label key exist, but assert nothing about the value of that label.

This change makes the CEL implementation uphold that same contract.

The CEL implementation of this policy always expected the `allowedRegex`
value to be set for a given `key` value in the contraint parameters.

Existing users of the template could (via rego) require only that a
given label key exist, but assert nothing about the value of that label.

This change makes the CEL implementation uphold that same contract.

Signed-off-by: juliankatz <[email protected]>
@julianKatz julianKatz requested a review from a team as a code owner August 26, 2024 21:25
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

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.

Thanks for catching @julianKatz!

This highlights the value of the Rego engine unit tests (e.g. https://github.com/open-policy-agent/gatekeeper-library/blob/master/src/general/requiredlabels/src_test.rego), as they provide far more code coverage than just the universal integration tests(https://github.com/open-policy-agent/gatekeeper-library/blob/master/library/general/requiredlabels/suite.yaml).

@julianKatz julianKatz requested a review from a team August 26, 2024 21:58
@apeabody apeabody merged commit caf342f into open-policy-agent:master Aug 26, 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.

4 participants