falco
falco copied to clipboard
falcosecurity
fix(build): metrics compilation after refactoring into classes
What type of PR is this?
/kind cleanup
Any specific area of the project related to this PR?
/area build
What this PR does / why we need it:
Once falcosecurity/libs#1920 is merged, compilation of falco will be broken due to the new_metric method changing the class it lives in. This PR makes falco compatible with this change.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: The commit that changes the libs cmake files will be dropped and replaced with a commit that properly updates the libs commit and SHA once falcosecurity/libs#1920 is merged, please disregard these changes until that merge occurs.
Does this PR introduce a user-facing change?:
NONE
I'm a bit confused as to why those driver tests are failing, these PRs don't do any changes to driver code or its corresponding userspace components. Any ideas?
It happens sometimes; probably mismatch between kernel headers package and runner kernel (or bugged headers package); rnothing to worry about for you! Restarted :)
Kmod failed again, but I need to fix the cmake files after I merge the libs PR anyways. @FedeDP, what do you think? Am I OK to unhold the libs PR?
@Molter73 we usually have the habit to bump both libs and driver during the dev phase. Few times we didn't do it and it caused compatibility issues. If you agree we can just continue bumping both together?
I'm guessing the error I'm getting is related to this commit: https://github.com/falcosecurity/libs/commit/fac0ae442474bc1265d2ddc54a3f12557337a821
How do we want to handle it?
Yep! I already tagged Gerald in that PR to ask him to double check ;) Not a big deal, we can wait and then bump libs and driver here once the fix PR is merged.
/hold for possible patches https://github.com/falcosecurity/falco/issues/3271#issuecomment-2206918760. Do not merge prior to having made a decision.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Molter73 Once this PR has been reviewed and has the lgtm label, please assign mstemm for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
I'm guessing the 3 issues are due to some change that bled in with the latest libs bump, I was having some issues linking unit tests locally, I'll look into them when I get a minute.
@jasondellaluce looks like the unit test errors I got are due to some warning messages being changed in falcosecurity/libs#1831, could you double check I got those right in the last commit?
Now it looks like ASAN is not very happy, tried to run the tests locally with a debug build and the errors seem related to falcosecurity/libs#1992, would appreciate some confirmation.
@Molter73 AFAIK the ASan failures are fixed by https://github.com/falcosecurity/libs/pull/1992 , I'm writing some tests for those and will merge the PR soon. I'm not sure why the asan crashes started appearing now.
I'm gonna need some help understanding the last errors I'm getting, seems something has been broken with the legacy rules?
This is the same error we are gettng on #3283 . Needs further investigation... Also this and #3283 are basically the same, i'd close one of them (well this is older and we should keep this but the other is on an upstream branch that is much easier to work with to fix the test issues; let me know if you wish to keep this one, i'll close the other!)