Skip to content

policyfilter improvements#1038

Merged
kkourt merged 13 commits into
mainfrom
pr/kkourt/policyfilter-improvements
Jun 1, 2023
Merged

policyfilter improvements#1038
kkourt merged 13 commits into
mainfrom
pr/kkourt/policyfilter-improvements

Conversation

@kkourt

@kkourt kkourt commented May 31, 2023

Copy link
Copy Markdown
Contributor

This PR includes several improvements for polcyfilter.

  • fix an issue where we did not removed policies from the policyfilter state
  • add e2e tests for namespaced policies and pod label filters
  • list policies in bugtool
  • adds some (simple for now) metrics

There were some modifications needed in the e2e tests for the second bullet. Please review patch-by-patch.

@kkourt
kkourt requested a review from a team as a code owner May 31, 2023 11:13
@kkourt
kkourt requested a review from tpapagian May 31, 2023 11:13
@kkourt
kkourt marked this pull request as draft May 31, 2023 11:13
@kkourt
kkourt force-pushed the pr/kkourt/policyfilter-improvements branch 4 times, most recently from 055bc63 to 725b3cf Compare May 31, 2023 12:42
@kkourt
kkourt marked this pull request as ready for review May 31, 2023 13:28

@willfindlay willfindlay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

e2e tests and e2e changes LGTM. Thanks a lot for the bug fixes btw 🚀

Comment thread tests/e2e/checker/rpcchecker.go
Comment thread tests/e2e/helpers/grpc/grpc.go
Comment thread pkg/metrics/policyfilter/policyfilter.go Outdated

@tpapagian tpapagian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

Comment thread cmd/tetra/tracingpolicy/tracingpolicy.go
Comment thread pkg/sensors/manager.go
kkourt added 13 commits June 1, 2023 11:06
Testing indicated that we were not removing the policy from the
policyfilter state, when the sensor manager  was deleting a policy. We
do so in this patch.

We add two more fields in the collection struct: an error and the filter
id. We use the error to mark errors when we try to delete a policy, and
the filter id to delete the policy from the policyfilter state.

For now, we expose these fields in the Info() field of
TracingPolicyStatus. Eventually, we probably want to have them as part
of the proto spec.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Set the wait flag to wait when for the pod to be ready when starting it.
We 've observed flakes on the skeleton test, so they could be
potentially caused by starting the test when the pod is not ready yet.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
While we check whether the checker implements GetRemainingChecks, we
still call the method if it does implement it which leads to a nil
dereference. This patch voids that by returning.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Previous version of RPCChecker failed when we hit the time limit or the
event limit. This breaks the MultiEventChecker semantics and makes it
difficult to check for the absence of events. This patch, changes
RPCChecker's behavior so that if it terminates due to a time or event
limit, it returns the value of FinalCheck().

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
This patch adds operation metrics to the policfilter operations. We
maintain a counter using the subsystem (currently pod-handlers or
rthooks), the operation, and the error (including the case where error
was nil) as labels.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
This patch adds a WaitForTracingPolicy function that: connects to all
tetragon pods via the gRPC port that was port-forwarded, and ensures
that a policy with the provided name exists in the agent.

This is going to be used in subsequent patches.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
policyfilter is the underlying mechanism for implementing namespaced
policies and pod label fitlers. Enable it for our e2e tests.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Add an e2e test for namespaced policies.

The test:
- creates two namespaces and start a pod in each of them
- installs a policy for monitoring syscalls in one of them
- checks that we get events only from that namespace, and not the other.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
This patch adds a pod label filter test to the policyfilter e2e test.

Idea is:
- create a namespace
- install a namespaced policy with pod label filter in that namespace (on sys_exit)
- create two pods, one that matches the policy and one that does not
- check that we only receive events from the first

We use sys_exit here to make it easier to dinguish with the namespace
test which uses sys_enter.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
The code that reads the flag, uses KeyOutput (which is set to "output"),
so use the same in the code that sets the flag.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
This patch adds an --output flag for listing tracing policies in a JSON
format.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
A test failure had goroutines blocked on starting the stt manager.

stt manager is no longer used. It was also based on the old sensor
design and we would need to rethink how to implement it. For now, just
remove it.

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
@kkourt
kkourt force-pushed the pr/kkourt/policyfilter-improvements branch from 725b3cf to 99d81ba Compare June 1, 2023 09:11
@kkourt
kkourt merged commit e64e1ac into main Jun 1, 2023
@kkourt
kkourt deleted the pr/kkourt/policyfilter-improvements branch June 1, 2023 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants