-
Notifications
You must be signed in to change notification settings - Fork 912
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
Allow exact rule matches ruleset #1185
Conversation
Currently, when calling enable_rule, the provided rule name pattern is a substring match, that is if the rules file has a rule "My fantastic rule", and you call engine->enable_rule("fantastic", true), the rule will be enabled. This can cause problems if one rule name is a complete subset of another rule name e.g. rules "My rule" and "My rule is great", and calling engine->enable_rule("My rule", true). To allow for this case, add an alternate method enable_rule_exact() in both default ruleset and ruleset variants. In this case, the rule name must be an exact match. In the underlying ruleset code, add a "match_exact" option to falco_ruleset::enable() that denotes whether the substring is an exact or substring match. This doesn't change the default behavior of falco in any way, as the existing calls still use enable_rule(). Signed-off-by: Mark Stemm <[email protected]>
Previously, valgrind was complaining about the leaked token bucket. Signed-off-by: Mark Stemm <[email protected]>
A new unit test file test_rulesets adds tests for the following: - enabling/disabling rules based on substrings - enabling/disabling rules based on exact matches - enabling/disabling rules based on tags There are variants that test for default and non-default rulesets. Signed-off-by: Mark Stemm <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @mstemm!
LGTM
LGTM label has been added. Git tree hash: 67866854e56eafc7d7555e29655635ae02465d78
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fntlnz, leodido 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 |
@mstemm This change looks good to me. It's just a great contribution! Just a question: The way this API is exposed now is only from We might want to start with an issue to propose that. |
What type of PR is this?
/kind bug
Any specific area of the project related to this PR?
/area engine
/area tests
What this PR does / why we need it:
This adds unit tests for ruleset handling and adds the ability to only allow for exact matches when adding rules to the falco engine. The current behavior matches substrings. This is usually fine but there can be some cases where exact matches are better.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: