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

refactor(userspace/engine): decoupling ruleset reading, parsing, and compilation steps #1970

Merged
merged 5 commits into from
Apr 15, 2022

Conversation

jasondellaluce
Copy link
Contributor

@jasondellaluce jasondellaluce commented Apr 13, 2022

What type of PR is this?

/kind cleanup

/kind design

/kind feature

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:

This is PR is not supposed to bring any breaking changes. The goal of this is to de-couple the logic related to ruleset file/content loading in the Falco Engine. With this, YAML reading and rule parsing/compilation are operated by the two distinct classes rule_reader and rule_loader. This not only makes the code more clean, but will also allow us to swap YAML library in the future, or even support alternative methods of loading rulesets.

On top of that, this also adds the optional feature of clearing the rule loader state with a clear_loader method in the engine, which is helpful to free some memory once all rule files have been loaded and when such information is not used anymore.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

refactor(userspace/engine): decoupling ruleset reading, parsing, and compilation steps

The rule_loader is now simply responsible of collecting list/macro/rule definitions and then compiling them as falco_rules. The ruleset file reading code will be moved to another class

Signed-off-by: Jason Dellaluce <[email protected]>
The rule_reader class is responsible of parsing the YAML ruleset text and of using the rule_loader
to store the new definition in the internal state. This is a first step towards separating the YAML
reading logic from the rule parsing one. Potentially, this will allow us to read rulesets from another
YAML library or from something different than YAML files too.

Signed-off-by: Jason Dellaluce <[email protected]>
Copy link
Contributor

@mstemm mstemm left a comment

Choose a reason for hiding this comment

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

This already is looking like a much cleaner way to interact with rules files!

test/falco_tests.yaml Outdated Show resolved Hide resolved
userspace/engine/falco_engine.h Outdated Show resolved Hide resolved
userspace/engine/rule_loader.h Outdated Show resolved Hide resolved
userspace/engine/rule_loader.h Show resolved Hide resolved
userspace/engine/rule_loader.h Show resolved Hide resolved
userspace/engine/rule_reader.cpp Outdated Show resolved Hide resolved
Once all rule files have been loaded, and all the rules have been compiled into filters and inserted in the engine rulesets, the loader definitions are maintained in memory without really being used. This commit adds a convenience method to clear the loader state and free-up some memory when engine consumers do not require such information in memory anymore.

Signed-off-by: Jason Dellaluce <[email protected]>
@poiana poiana added the lgtm label Apr 14, 2022
@poiana
Copy link
Contributor

poiana commented Apr 14, 2022

LGTM label has been added.

Git tree hash: 1b5b7fa644dd814ba25ce4f4a4267a1d79c6baf8

Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

LGTM

/approve

@poiana
Copy link
Contributor

poiana commented Apr 15, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jasondellaluce, leogr

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:
  • OWNERS [jasondellaluce,leogr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana merged commit f638706 into master Apr 15, 2022
@poiana poiana deleted the poc/rule-loader-decouplin branch April 15, 2022 08:55
@jasondellaluce
Copy link
Contributor Author

/milestone 0.32.0

@poiana poiana added this to the 0.32.0 milestone Apr 15, 2022
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