8000 Dependencies tree manager by AlonZivony · Pull Request #3931 · aquasecurity/tracee · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Dependencies tree manager #3931

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 4, 2024
Merged

Dependencies tree manager #3931

merged 1 commit into from
Apr 4, 2024

Conversation

AlonZivony
Copy link
Contributor
@AlonZivony AlonZivony commented Mar 26, 2024 •
edited
Loading

1. Explain what the PR does

Create a manager object for the dependencies between event.
It helps to follow what events are really used, and will enable to effectively cancel and add events and probes.

This is a first step towards my current goal of having fallbacks for dependencies, to have a proper solution for the process_execute_failed event.
I will create another PR after this one that support the fallback of dependencies, and afterwards work with it on the event.

The work on this PR exposed multiple problems or I might even call them bugs in our current code:

  1. When an event fails, we just cancel all of its dependencies events even though they might be chosen or a dependency of another event. However, we don't cancel events that depend on it. This PR address this issue - Fix Events cancellation removes chosen dependencies events #3932
  2. We don't cancel events properly upon failure. We don't detach their probes nor remove them from the policies filter maps. this wasn't addressed in this PR, but this PR exposes an easy way to do these things - using the watcher functions we can make sure to update everything upon each change to the events and their decencies. This PR does not address this issue, only creates the framework to solve it in future PRs

2. Explain how to test it

3. Other comments

@AlonZivony AlonZivony force-pushed the feature/managing-dependencies branch from 5571903 to f9d83d8 Compare March 26, 2024 11:10
@AlonZivony AlonZivony changed the title Before using the dependencies tree Dependencies tree manager Mar 26, 2024
@AlonZivony AlonZivony force-pushed the feature/managing-dependencies branch 4 times, most recently from f41827f to e896171 Compare March 28, 2024 11:57
@AlonZivony
Copy link
Contributor Author

The current objects are not thread safe.
Do you want me to fix them to be or add some documentation about it?
Currently it doesn't matter, but in the future it might do.

@AlonZivony AlonZivony force-pushed the feature/managing-dependencies branch 2 times, most recently from 7f3537d to 703e2b3 Compare March 28, 2024 17:04
@geyslan
Copy link
Member
geyslan commented Apr 1, 2024

@AlonZivony I'll be on this soon.

geyslan
geyslan approved these changes Apr 1, 2024
View reviewed changes
Copy link
Member
@geyslan geyslan left a comment

Choose a reason for hiding this comment

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

I reviewed the code and tests and didn't find any apparent flaws. I also ran the unit tests locally and checked the manager generated fields values with some prints. LGTM.

E2E 1433 is green. 👍🏼

Furthermore, put a few comments for discussion.

@yanivagman
Copy link
Collaborator

I'm going to review this soon. Please wait with merging it

@AlonZivony
events: add dependencies tree manager
c87c2a9 
Create a manager object for the dependencies between event.
It helps to follow what events are really used, and will enable to effectively cancel and add events and probes.
@AlonZivony AlonZivony force-pushed the feature/managing-dependencies branch from 703e2b3 to c87c2a9 Compare April 3, 2024 12:01
@yanivagman yanivagman merged commit d5899a0 into aquasecurity:main Apr 4, 2024
32 checks passed
@AlonZivony AlonZivony mentioned this pull request Apr 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geyslan geyslan geyslan approved these changes

@yanivagman yanivagman yanivagman left review comments

Assignees

@AlonZivony AlonZivony

Labels
area/ebpf area/events area/testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Events cancellation removes chosen dependencies events
3 participants
@AlonZivony @geyslan @yanivagman
0