Allow event filtering based on tracing policy #1867

ioandr · 2023-12-11T01:30:31Z

This PR extends the list of filters by implementing the policy name filter. This allows Tetragon to filter events based on the name of a tracing policy.

~~It also improves Tetragon's docs~~ (see #1965) and extends test suites as needed.

Closes #1855

tetra: Add event filter based on tracing policy name

netlify · 2023-12-11T01:31:00Z

✅ Deploy Preview for tetragon ready!

Name	Link
🔨 Latest commit	`fee4776`
🔍 Latest deploy log	https://app.netlify.com/sites/tetragon/deploys/65a5771630912d0008d50f0e
😎 Deploy Preview	https://deploy-preview-1867--tetragon.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

kkourt · 2023-12-20T10:34:19Z

Thanks! started the tests, and added a release note!

mtardy

Hey!! Thanks a lot for those patches, I think they all make sense and I would like to merge them!

This is a bit of a nit but the commits are not really related, would it be possible to split this PR into two PRs? Maybe this one for the filtering based on the tracing policy name, and another one for the docs? If this is too annoying tell me and we'll figure something else.

Also if you can rebase since we fixed something in the CI in the meantime. Sorry for the delay, it was the Christmas period! 🎄

api/v1/tetragon/events.proto

docs/content/en/docs/getting-started/install-k8s.md

willfindlay · 2024-01-08T15:39:16Z

I'm also good with merging this once @mtardy's comments have been addressed.

jrfastab · 2024-01-08T20:31:39Z

+1 appears useful please address feedback and we can get this merged! :)

ioandr · 2024-01-10T08:30:30Z

Hey!! Thanks a lot for those patches, I think they all make sense and I would like to merge them!

That's good to hear, thanks!

This is a bit of a nit but the commits are not really related, would it be possible to split this PR into two PRs? Maybe this one for the filtering based on the tracing policy name, and another one for the docs? If this is too annoying tell me and we'll figure something else.

Sure, I can split changes into 2 PRs.

Also if you can rebase since we fixed something in the CI in the meantime.

Definitely, will do.

kkourt

Thanks!

Everything looks good to me.

I'd prefer if the last patch was split into multiple patches, such as

documentation fixes (typos)
core code changes
tetra CLI

But I don't think we should block the PR on this.

Could you please address the comments from everyone and have the documentation change also work in bash?

Other than that, good to go from my side!

docs/content/en/docs/getting-started/install-k8s.md

ioandr · 2024-01-12T00:44:34Z

Update:

I moved doc-related changes to a new PR - see docs: fix typos, fix k8s install guide for zsh and update Cilium SW demo #1965
I broke down remaining changes introduced by the original PR into 3 commits as discussed above
I rebased over latest main and force-pushed to the branch of the original PR
I did not have the time to re-test functionality, I can do that in the following days (if needed)

kkourt

Many thanks! LGTM

I did not have the time to re-test functionality, I can do that in the following days (if needed)

Is there testing that is not covered by the unit tests?
As far as I can see, there were no changes in the code (other than the rebasing) and you have unit tests so, at least from my side, it's good to merge.

ioandr · 2024-01-12T19:05:04Z

Is there testing that is not covered by the unit tests? As far as I can see, there were no changes in the code (other than the rebasing) and you have unit tests so, at least from my side, it's good to merge.

Indeed, just mentioned this for completeness. I'm good to proceed.

ioandr · 2024-01-12T19:44:34Z

@mtardy now that your comments have been addressed maybe approve this?

mtardy

Thanks a lot, and thanks for adding tests, let's merge that :)

cmd/tetra/getevents/getevents.go

mtardy · 2024-01-15T10:11:41Z

I can't merge because of this @willfindlay, could you review it?

tetragon/.github/CODEOWNERS

Lines 4 to 5 in 0a9b71e

    
           # Request a review from William when codegen is edited 
        
           /cmd/protoc-gen-go-tetragon/ @willfindlay

In the meantime @ioandr if you can ever rebase this on main (I'm sorry I wanted to merge this now), I fixed the CI issue that made vmtests fail early.

Signed-off-by: Ioannis Androulidakis <[email protected]>

Extend the list of filters with a new one to support filtering events based on the name of a tracing policy. Closes cilium#1855 Signed-off-by: Ioannis Androulidakis <[email protected]>

Closes cilium#1855 Signed-off-by: Ioannis Androulidakis <[email protected]>

mtardy · 2024-01-16T09:16:34Z

Thanks for the rebase, everything is green, now just waiting on William's ack.

willfindlay · 2024-01-16T14:05:35Z

Merged, thanks

In cilium#1867, the `--policy-names` flag was added to filter events based on the tracing policy. However, the filter was only appled to `kprobe` events. This patch extends the filter to support all events types: `tracepoint`, `uprobe` and `lsm`. Signed-off-by: Justin Chen <[email protected]>

In cilium#1867, the `--policy-names` flag was added to filter events based on the tracing policy. However, the filter was only appled to `kprobe` events. This patch extends the filter to support all events types: `kprobe`, `tracepoint`, `uprobe` and `lsm`. Signed-off-by: Justin Chen <[email protected]>

In #1867, the `--policy-names` flag was added to filter events based on the tracing policy. However, the filter was only appled to `kprobe` events. This patch extends the filter to support all events types: `kprobe`, `tracepoint`, `uprobe` and `lsm`. Signed-off-by: Justin Chen <[email protected]>

ioandr force-pushed the feature-i-filter-policies branch 2 times, most recently from 8ad79e5 to b14a096 Compare December 20, 2023 01:14

ioandr marked this pull request as ready for review December 20, 2023 01:15

ioandr requested review from a team, mtardy and willfindlay as code owners December 20, 2023 01:15

kkourt self-requested a review December 20, 2023 10:26

kkourt added release-note/misc This PR makes changes that have no direct user impact. release-note/minor This PR introduces a minor user-visible change and removed release-note/misc This PR makes changes that have no direct user impact. labels Dec 20, 2023

mtardy reviewed Jan 4, 2024

View reviewed changes

api/v1/tetragon/events.proto Outdated Show resolved Hide resolved

docs/content/en/docs/getting-started/install-k8s.md Outdated Show resolved Hide resolved

kkourt requested changes Jan 10, 2024

View reviewed changes

docs/content/en/docs/getting-started/install-k8s.md Outdated Show resolved Hide resolved

ioandr force-pushed the feature-i-filter-policies branch from b14a096 to 6d05c15 Compare January 12, 2024 00:38

kkourt approved these changes Jan 12, 2024

View reviewed changes

ioandr mentioned this pull request Jan 12, 2024

docs: fix typos, fix k8s install guide for zsh and update Cilium SW demo #1965

Merged

mtardy approved these changes Jan 15, 2024

View reviewed changes

cmd/tetra/getevents/getevents.go Show resolved Hide resolved

mtardy added the area/userspace Related to userspace Tetragon logic label Jan 15, 2024

ioandr added 3 commits January 15, 2024 19:17

api: Fix typos in comment

f2b2cb0

Signed-off-by: Ioannis Androulidakis <[email protected]>

pkg/filters: Implement policy name filter

7d050ab

Extend the list of filters with a new one to support filtering events based on the name of a tracing policy. Closes cilium#1855 Signed-off-by: Ioannis Androulidakis <[email protected]>

cmd/tetra: Add option to filter events based on policy names

fee4776

Closes cilium#1855 Signed-off-by: Ioannis Androulidakis <[email protected]>

ioandr force-pushed the feature-i-filter-policies branch from 6d05c15 to fee4776 Compare January 15, 2024 18:19

willfindlay approved these changes Jan 16, 2024

View reviewed changes

willfindlay merged commit 5361282 into cilium:main Jan 16, 2024
37 checks passed

justin0u0 mentioned this pull request Oct 25, 2024

tetra: fix --policy-names to apply all event types #3044

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow event filtering based on tracing policy #1867

Allow event filtering based on tracing policy #1867

ioandr commented Dec 11, 2023 •

edited by lambdanis

Loading

netlify bot commented Dec 11, 2023 •

edited

Loading

kkourt commented Dec 20, 2023

mtardy left a comment •

edited

Loading

willfindlay commented Jan 8, 2024

jrfastab commented Jan 8, 2024

ioandr commented Jan 10, 2024 •

edited

Loading

kkourt left a comment

ioandr commented Jan 12, 2024

kkourt left a comment

ioandr commented Jan 12, 2024

ioandr commented Jan 12, 2024

mtardy left a comment

mtardy commented Jan 15, 2024

mtardy commented Jan 16, 2024

willfindlay commented Jan 16, 2024

Allow event filtering based on tracing policy #1867

Allow event filtering based on tracing policy #1867

Conversation

ioandr commented Dec 11, 2023 • edited by lambdanis Loading

netlify bot commented Dec 11, 2023 • edited Loading

✅ Deploy Preview for tetragon ready!

kkourt commented Dec 20, 2023

mtardy left a comment • edited Loading

Choose a reason for hiding this comment

willfindlay commented Jan 8, 2024

jrfastab commented Jan 8, 2024

ioandr commented Jan 10, 2024 • edited Loading

kkourt left a comment

Choose a reason for hiding this comment

ioandr commented Jan 12, 2024

kkourt left a comment

Choose a reason for hiding this comment

ioandr commented Jan 12, 2024

ioandr commented Jan 12, 2024

mtardy left a comment

Choose a reason for hiding this comment

mtardy commented Jan 15, 2024

mtardy commented Jan 16, 2024

willfindlay commented Jan 16, 2024

ioandr commented Dec 11, 2023 •

edited by lambdanis

Loading

netlify bot commented Dec 11, 2023 •

edited

Loading

mtardy left a comment •

edited

Loading

ioandr commented Jan 10, 2024 •

edited

Loading