new(metrics): add file sha256sum metrics for loaded config and rules files #3187

incertum · 2024-05-08T12:41:47Z

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

/kind release

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area build

/area engine

/area tests

/area proposals

/area CI

What this PR does / why we need it:

This PR adds the sha256sum for each loaded config and rules file as individual metric. These metrics complement existing informational metrics such as the Falco version or kernelrelease of the host and especially help to track deployment upgrade convergence and integrity. When hot reloading Falco and having watch config files setting enabled, the state is re-initialized and as such the new sha256sum is calculated.

Note: This PR only adds the new metrics, thus deferring future metrics code consolidations to the next release dev cycle.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new(metrics): add file sha256sum metrics for loaded config and rules files

github-actions · 2024-05-08T12:42:23Z

This PR may bring feature or behavior changes in the Falco engine and may require the engine version to be bumped.

Please double check userspace/engine/falco_engine_version.h file. See versioning for FALCO_ENGINE_VERSION.

/hold

incertum · 2024-05-08T12:46:48Z

/milestone 0.38.0

incertum · 2024-05-09T16:16:49Z

This is now ready for review.

FedeDP · 2024-05-10T06:51:52Z

userspace/falco/app/actions/load_rules_files.cpp

@@ -83,6 +84,9 @@ falco::app::run_result falco::app::actions::load_rules_files(falco::app::state&
 		{
 			falco_logger::log(falco_logger::level::WARNING,res->as_string(true, rc) + "\n");
 		}
+#if defined(__linux__) and !defined(MINIMAL_BUILD) and !defined(__EMSCRIPTEN__)
+		s.config->m_loaded_rules_filenames_sha256sum.push_back(falco::utils::calculate_file_sha256sum(filename));


What if m_loaded_rules_filenames_sha256sum (and m_loaded_configs_filenames_sha256sum) was instead a unoredered_set<string>string, where key is filename and value is sha256sum?
This would allow metrics and stats writer to just iterate over m_loaded_rules_filenames_sha256sum (and m_loaded_configs_filenames) keys.

If that's more desirable, no problems to change it.

I think it would simplify a bit the falco_metrics and stats_writer loops, making them more readable.

FedeDP

Question: why is all of this linux only?

incertum · 2024-05-10T07:42:31Z

Question: why is all of this linux only?

Main purpose of metrics is at runtime when running Falco on Linux. I am not sure why we would need it when we load a capture file on macOS or Windows. In addition, most metrics already only work on Linux. More thoughts?

Support for macOS or Windows likely requires a different approach as that openssl lib I am using is not available.

FedeDP · 2024-05-10T07:45:19Z

I was thinking if running Falco on eg: windows with plugins and their rules, one could still want the shasums in the metrics.

Support for macOS or Windows likely requires a different approach as that openssl lib I am using is not available.

No problem then, we can introduce it later if someone needs it!

incertum · 2024-05-10T07:47:41Z

I was thinking if running Falco on eg: windows with plugins and their rules, one could still want the shasums in the metrics.

Support for macOS or Windows likely requires a different approach as that openssl lib I am using is not available.

No problem then, we can introduce it later if someone needs it!

Yeah, right now it's actually not even working well for plugins only even on Linux. Needless to say, metrics still requires lots of work over the next n releases.

FedeDP

/approve

poiana · 2024-05-10T08:39:37Z

LGTM label has been added.

Git tree hash: dcb2a70aa3d6111480fd1c016a4db4f24a8b665d

FedeDP · 2024-05-13T08:38:49Z

CI Build / test-dev-packages / test-packages (pull_request) Failing after 4m

I am investigating the TestFalco_Legacy_KernelUpgrade failure since it also fails on #3191

Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

…files Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

Co-authored-by: Federico Di Pierro <nierro92@gmail.com> Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

FedeDP

/approve

poiana · 2024-05-13T21:39:20Z

LGTM label has been added.

Git tree hash: dc0266cc27417ce5e855648589ac81944db7ea34

poiana · 2024-05-14T08:07:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, incertum, 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~~ [FedeDP,incertum,leogr]

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

leogr · 2024-05-14T08:08:06Z

This PR may bring feature or behavior changes in the Falco engine and may require the engine version to be bumped.

Please double check userspace/engine/falco_engine_version.h file. See versioning for FALCO_ENGINE_VERSION.

/hold

false positive
/unhold

poiana added do-not-merge/work-in-progress release-note kind/feature dco-signoff: no area/engine approved labels May 8, 2024

poiana requested review from FedeDP and Kaizhe May 8, 2024 12:41

poiana added the size/L label May 8, 2024

poiana added the do-not-merge/hold label May 8, 2024

incertum force-pushed the sha256-config-rules-files-metrics branch from f9607c8 to 0cc454d Compare May 8, 2024 12:43

poiana added dco-signoff: yes and removed dco-signoff: no labels May 8, 2024

poiana added this to the 0.38.0 milestone May 8, 2024

incertum mentioned this pull request May 8, 2024

Bug: segfault on hot reload of configs in current falco master #3188

Closed

incertum force-pushed the sha256-config-rules-files-metrics branch from 0cc454d to 9c5a8b6 Compare May 9, 2024 16:12

incertum changed the title ~~wip: new(metrics): add file sha256sum metrics for loaded config and rules files~~ new(metrics): add file sha256sum metrics for loaded config and rules files May 9, 2024

poiana removed the do-not-merge/work-in-progress label May 9, 2024

FedeDP reviewed May 10, 2024

View reviewed changes

FedeDP approved these changes May 10, 2024

View reviewed changes

poiana assigned FedeDP May 10, 2024

poiana added the lgtm label May 10, 2024

incertum and others added 5 commits May 13, 2024 16:34

new(utils): add new helper to calculate file sha256sum

b71230e

Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

new(metrics): add file sha256sum metrics for loaded config and rules …

7cc49fa

…files Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

cleanup(metrics): use filesystem lib to derive file names + build fix

30d5ca2

Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

chore: fix non linux build metrics

1067bf1

Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

cleanup(metrics): use map for config and rules filenames sha256 tracking

f9c7dc1

Co-authored-by: Federico Di Pierro <nierro92@gmail.com> Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

incertum force-pushed the sha256-config-rules-files-metrics branch from 8c2d430 to f9c7dc1 Compare May 13, 2024 16:34

poiana removed the lgtm label May 13, 2024

poiana requested a review from FedeDP May 13, 2024 16:34

FedeDP approved these changes May 13, 2024

View reviewed changes

poiana added the lgtm label May 13, 2024

leogr approved these changes May 14, 2024

View reviewed changes

poiana assigned leogr May 14, 2024

poiana removed the do-not-merge/hold label May 14, 2024

poiana merged commit 60e6798 into falcosecurity:master May 14, 2024
35 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new(metrics): add file sha256sum metrics for loaded config and rules files #3187

new(metrics): add file sha256sum metrics for loaded config and rules files #3187

incertum commented May 8, 2024 •

edited

github-actions bot commented May 8, 2024

incertum commented May 8, 2024

incertum commented May 9, 2024

FedeDP May 10, 2024

incertum May 10, 2024

FedeDP May 10, 2024

incertum May 10, 2024

FedeDP left a comment

incertum commented May 10, 2024

FedeDP commented May 10, 2024

incertum commented May 10, 2024

FedeDP left a comment

poiana commented May 10, 2024

FedeDP commented May 13, 2024

FedeDP left a comment

poiana commented May 13, 2024

poiana commented May 14, 2024

leogr commented May 14, 2024

new(metrics): add file sha256sum metrics for loaded config and rules files #3187

new(metrics): add file sha256sum metrics for loaded config and rules files #3187

Conversation

incertum commented May 8, 2024 • edited

github-actions bot commented May 8, 2024

incertum commented May 8, 2024

incertum commented May 9, 2024

FedeDP May 10, 2024

Choose a reason for hiding this comment

incertum May 10, 2024

Choose a reason for hiding this comment

FedeDP May 10, 2024

Choose a reason for hiding this comment

incertum May 10, 2024

Choose a reason for hiding this comment

FedeDP left a comment

Choose a reason for hiding this comment

incertum commented May 10, 2024

FedeDP commented May 10, 2024

incertum commented May 10, 2024

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented May 10, 2024

FedeDP commented May 13, 2024

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented May 13, 2024

poiana commented May 14, 2024

leogr commented May 14, 2024

incertum commented May 8, 2024 •

edited