-
Notifications
You must be signed in to change notification settings - Fork 36
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
Added event for default stable rule "Remove Bulk Data from Disk" #109
Conversation
Signed-off-by: GLVS Kiriti <[email protected]>
Kindly tell me if any changes required!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we need to clean up the temp dir.
|
||
func RemoveBulkDataFromDisk(h events.Helper) error { | ||
// Creates temporary data for testing, avoiding critical file deletion. | ||
tmpDir, err := os.MkdirTemp(os.TempDir(), "created-by-falco-event-generator") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tmpDir, err := os.MkdirTemp(os.TempDir(), "created-by-falco-event-generator") | |
tmpDir, err := os.MkdirTemp(os.TempDir(), "created-by-falco-event-generator") | |
defer os.RemoveAll(tmpDir) // clean up |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leogr why again defer os.RemoveAll(tmpDir) as we already removing it with shred command in the same code at last
event-generator/events/syscall/remove_bulk_data_from_disk.go
Lines 42 to 44 in da107c6
cmd := exec.Command("shred", "-u", tmpDir) | |
err = cmd.Run() | |
return err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not consider that, you're likely right.
Just one doubt: what happens if shred
fails? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
irrespective of shred exist or not ! the rule is triggering in this case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: GLVS Kiriti <[email protected]>
@leogr |
Signed-off-by: GLVSKiriti <[email protected]>
|
||
func RemoveBulkDataFromDisk(h events.Helper) error { | ||
// Creates temporary data for testing, avoiding critical file deletion. | ||
tmpDir, err := os.MkdirTemp(os.TempDir(), "created-by-falco-event-generator") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not consider that, you're likely right.
Just one doubt: what happens if shred
fails? 🤔
As with shred we can remove a file not a directory Signed-off-by: GLVS Kiriti <[email protected]>
Just updated and test is also successful now!!
IMO Instead of directly skipping action can we just run the command so that rule triggers? and if shred not exists clean up happens and remove the file by
|
My original idea was that we should skip the action whenever the preconditions are not met. Here's a clear example of a skipped action due to a missing utility: event-generator/events/syscall/schedule_cron_jobs.go Lines 35 to 41 in ddf4731
That being said, if just trying to execute the
In such a case, please just leave a code comment to explain why what we are doing. Thanks |
Signed-off-by: GLVS Kiriti <[email protected]>
Signed-off-by: GLVSKiriti <[email protected]>
Just added the comments! |
Co-authored-by: Leonardo Grasso <[email protected]> Signed-off-by: GLVSKiriti <[email protected]>
…eft side of := Signed-off-by: Leonardo Grasso <[email protected]>
LGTM label has been added. Git tree hash: b66fe657a922e9ba9a76595b8ff7b093333a0c53
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: GLVSKiriti, 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:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area events
What this PR does / why we need it:
Added an event for the default stable rule "Remove Bulk Data from Disk" which is very important to test because sometimes bulk data removal can be done with the intention to destroy data, possibly interrupting availability to systems.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: