falco
falco copied to clipboard
Support condition parse errors in rule loading results
In #2098, we reworked how rules loading errors/warnings were returned to provide a richer set of information, including locations/context for the errors/warnings.
That did not include locations within condition expressions, though. When parsing a condition expression resulted in a warning/error, the location simply pointed to the condition property of the rule.
This commit improves this to handle parse errors:
- When libsinsp::filter::parser::parse() throws an exception, use get_pos() to get the position within the condition string.
- Add a new context() constructor that takes a filter pos_info instead of a YAML::Mark.
Doing this required some small changes to the context struct, though. Previously, the name (e.g. yaml filename) was held separate from the list of locations. However, condition strings don't directly reflect any content in the yaml file. Yaml block scalars remove whitespace/unwrap strings. Macro references/list references are replaced with their contents.
To handle this, move the "name" (e.g. filename) from the context into each location. This allows a chain of locations to start with file positions but transition to offsets within a condition expression.
Also allow a context to contain an alternate content string which is used to build the snippet. For contexts related to condition strings, the content is the condition.
Finally, when printing snippets that contain very long lines (> a static const 160 chars), instead of printing the entire line, print the 160 chars surrounding the position.
Signed-off-by: Mark Stemm [email protected]
What type of PR is this?
Uncomment one (or more)
/kind <>
lines:
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
If contributing rules or changes to rules, please make sure to also uncomment one of the following line:
/kind rule-update
/kind rule-create
Any specific area of the project related to this PR?
Uncomment one (or more)
/area <>
lines:
/area build
/area engine
/area rules
/area tests
/area proposals
What this PR does / why we need it:
Add better support for parse errors in rule conditions when displaying rule load errors.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
NONE
/milestone 0.33.0
The test failure was not related to these changes, and was actually a design flaw in the earlier PR that wasn't detected until I changed how very long snippet lines were returned.
If reading rules files A and B, it's possible that reading file A has no warnings, but after reading file B there are warnings in file A, because B overrides macros/rules/lists from A. Two files that show this are https://github.com/falcosecurity/falco/blob/master/test/rules/single_rule.yaml and https://github.com/falcosecurity/falco/blob/master/test/rules/override_macro.yaml. override_macro overrides the macro is_cat, but doing that causes the list used in the original definition of is_cat to become unused.
I'm going to fix that in a separate PR. It's going to have a lot of the changes that are in this PR (moving filenames into the context positions, handling long snippet lines), but it won't have the specific changes for handling condition parse errors.
So putting this one on hold for now.
LGTM label has been added.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: FedeDP, jasondellaluce, mstemm
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [FedeDP,jasondellaluce,mstemm]
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment