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

feature(k8s): add default events to k8s policies #3328

Conversation

josedonizetti
Copy link
Contributor

@josedonizetti josedonizetti commented Jul 19, 2023

1. Explain what the PR does

Fix #3323

Adds a new policy to have all default events collected by default into kubernetes.

2. Explain how to test it

# create a k8s cluster
minikube start

# deploy tracee
helm install tracee ./deploy/helm/tracee

# check it is running
kubectl get pods
kubectl logs -f ds/tracee

3. Other comments

@josedonizetti josedonizetti marked this pull request as ready for review July 19, 2023 18:55
Copy link
Contributor

@rafaeldtinoco rafaeldtinoco left a comment

Choose a reason for hiding this comment

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

Possibly something you agreed with someone else (based on the issue description and an "Okay" emoji from Itay). The change looks technically correct so I'm approving. I just wonder why this change is being made. I thought that the "detections only" policy was quite nice (since users might not be interested in many of these default events). What is the idea behind this change ? (Out of curiosity only, feel free to merge).

@josedonizetti
Copy link
Contributor Author

@rafaeldtinoco Good question. For me, the idea is to be consistent with tracee without policies. Why do we have so many default events if you boot tracee with the binary or docker, but not for k8s? The experiences should be consistent.
Why k8s is detections only? But for binary/docker we also have some other forensics events?

@rafaeldtinoco
Copy link
Contributor

Why k8s is detections only? But for binary/docker we also have some other forensics events?

Yep, @itaysk and @yanivagman to answer/decide. I'm not sure why we're mixing tracing and detection events in the default experience.

IMHO, "detections only" seems more appropriate to be the default but, TBH, maybe this should require feedback from users and/or something similar (and not something I would be the best person to opinionate).

Like I said, change is "technically correct", as long as people are on the same page, I'm good.

@josedonizetti
Copy link
Contributor Author

@rafaeldtinoco Yeah, also not sure about the mix for the default events, I think our goal is to have higher level as default events along signatures in the future, but we still didn't implement them, so we are using the closest events?

But I'm moving this to 0.18.0 so we can think a little more about it.

@itaysk
Copy link
Collaborator

itaysk commented Jul 25, 2023

I'm not sure why we're mixing tracing and detection events

the technical separation between "trace" and "signatures" was unnatural for users. for example "mem_prot_alert" is technically an event but logically a signature, "container created" was technically a signature but logically an event. Users went through lengths in order to get both streams, so instead of trying to justify this separation, we canceled it and made "everything as an event". Now there is no more "trace" or "signature" it's just events.

users might not be interested in many of these default events

I do agree that the selected default events are probably too verbose, I would suggest to add just execution events for now.

@josedonizetti
Copy link
Contributor Author

I do agree that the selected default events are probably too verbose, I would suggest to add just execution events for now.

Yes, that is why I think moving to 0.18.0 makes sense, because we will be adding process_executed and process_exit, we could have them + signatures are the defaults.

@rafaeldtinoco
Copy link
Contributor

for example "mem_prot_alert" is technically an event but logically a signature, "container created" was technically a signature but logically an event.

Indeed, indeed. Never stopped to think about this, super true.

because we will be adding process_executed and process_exit, we could have them + signatures are the defaults.

Yep. That would be better IMO.

@josedonizetti
Copy link
Contributor Author

I'm closing this PR while higher level events aren't implemented -> #2725, but there is a issue tracking it so we can implement it later #3323

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

k8s extend default policy
3 participants