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

update!: bump libs version, and support latest plugin features, add --nodriver option #2552

Merged
merged 21 commits into from
May 19, 2023

Conversation

jasondellaluce
Copy link
Contributor

What type of PR is this?

/kind cleanup

/kind feature

Any specific area of the project related to this PR?

/area build

/area engine

What this PR does / why we need it:

This PR updates the libs and driver dependencies to the latest mainline of falcosecurity/libs. This brings in all the latest changes of the plugin framework, including the new plugin API major version change 3.0.0 (which can be seen as a breaking changes, old plugins based on the API 2.X.X will be rejected by future Falco versions).

This also supports plugins with the new event parsing and async events capabilities. Accordingly, a new --nodriver option has been introduced to open live captures without drivers. This can be used when loading with plugins generating system events to instruct Falco about using the plugins for the system events capture instead of one of the drivers.

Also, this carries a couple of fixes in Falco, such as properly constructing the config object (which caused spurious test failures), and making all the different open options mutually exclusive (e.g. -u, --moder-bpf, etc...).

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

update!: bump libs version, and support latest plugin features, add --nodriver option

@jasondellaluce
Copy link
Contributor Author

/milestone 0.35.0

@poiana poiana added this to the 0.35.0 milestone May 17, 2023
@poiana poiana requested review from Kaizhe and leogr May 17, 2023 16:37
@poiana poiana added the size/L label May 17, 2023
Andreagit97
Andreagit97 previously approved these changes May 17, 2023
Copy link
Member

@Andreagit97 Andreagit97 left a comment

Choose a reason for hiding this comment

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

/approve

@@ -147,6 +149,19 @@ bool options::parse(int argc, char **argv, std::string &errstr)

list_fields = m_cmdline_parsed.count("list") > 0 ? true : false;

int open_modes = 0;
open_modes += !trace_filename.empty();
Copy link
Member

Choose a reason for hiding this comment

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

it seems like a joke that every engine is opened in a different way but that's the sad truth 😂 maybe one day we will us an enum

@@ -117,7 +117,7 @@ falco::app::run_result falco::app::actions::init_falco_engine(falco::app::state&
auto manager = s.offline_inspector->get_plugin_manager();
for (const auto &p : manager->plugins())
{
if (p->caps() & CAP_SOURCING)
if (p->caps() & CAP_SOURCING && p->id() != 0)
Copy link
Member

Choose a reason for hiding this comment

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

this check is used in more than one place probably at a certain point we could create an helper for it

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, i think a small method would help making the code more readable.

@poiana
Copy link
Contributor

poiana commented May 17, 2023

LGTM label has been added.

Git tree hash: cc849d088cf43451e1403e2cb3069136d442da7a

cmake/modules/driver.cmake Outdated Show resolved Hide resolved
…34bc8f5aef

Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
…ourcing system events

Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
leogr
leogr previously approved these changes May 18, 2023
@poiana
Copy link
Contributor

poiana commented May 18, 2023

LGTM label has been added.

Git tree hash: 44081af7b5289184c96f2f74b475bcc666d9de2c

Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
&& r.warn_evttypes)
auto evttypes = libsinsp::filter::ast::ppm_event_codes(ast.get());
evttypes.insert(ppm_event_code::PPME_ASYNCEVENT_E);
if ((evttypes.empty() || evttypes.size() > 100) && r.warn_evttypes)
Copy link
Contributor

Choose a reason for hiding this comment

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

evvtypes.empty() can't ever be true here since we've just added PPME_ASYNCEVENT_E, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks!

jasondellaluce and others added 3 commits May 19, 2023 08:56
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Co-authored-by: Grzegorz Nosek <grzegorz.nosek@sysdig.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

All green!
/approve

Great job @jasondellaluce !

@poiana
Copy link
Contributor

poiana commented May 19, 2023

LGTM label has been added.

Git tree hash: a179f875a2c7dc5f3ff3451aa27cd65365b080c5

Copy link
Member

@Andreagit97 Andreagit97 left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented May 19, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, jasondellaluce

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 [Andreagit97,FedeDP,jasondellaluce]

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 2818f09 into master May 19, 2023
@poiana poiana deleted the update/libs-sync branch May 19, 2023 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants