-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 issue in logic of modsec template #4793
Conversation
according to go templates: `(and ((not false) false))` == `true` the only way to remove the owasp rules from every location is to disable modsec on that location, or to enable owasp globally, both not-so-great choices. This commit fixes the logic issue by fixing the and-clause in the if-statement. As a result this reduces global resource usages when modsecurity is configured globally, but not on every location.
Hi @MMeent. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
Codecov Report
@@ Coverage Diff @@
## master #4793 +/- ##
=========================================
Coverage ? 58.59%
=========================================
Files ? 88
Lines ? 6702
Branches ? 0
=========================================
Hits ? 3927
Misses ? 2349
Partials ? 426 Continue to review full report at Codecov.
|
/lgtm @MMeent thanks! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, MMeent The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
according to go templates:
(and ((not false) false))
==true
the only way to remove the owasp rules from every location is to disable modsec on that location, or to enable owasp globally, both not-so-great choices.
This commit fixes the logic issue by fixing the and-clause in the if-statement. As a result this reduces global resource usages when modsecurity is configured globally, but not on every location.
What this PR does / why we need it:
The current logic in the template dictates that when you enable modsecurity globally, you either also enable the owasp rule sets globally, or the owasp rulesets are rendered into each location, increasing memory usage per-location without a way to turn this off.
Which issue this PR fixes fixes #4629
Special notes for your reviewer: If possible, please backport to 0.26.x