-
Notifications
You must be signed in to change notification settings - Fork 6
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
NETOBSERV-1609: parse flow filter configs and use it to update manifest #30
Conversation
4551ec3
to
fea3833
Compare
4b21561
to
40bcff6
Compare
@msherif1234: This pull request references NETOBSERV-1609 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #30 +/- ##
=======================================
Coverage 31.25% 31.25%
=======================================
Files 5 5
Lines 944 944
=======================================
Hits 295 295
Misses 629 629
Partials 20 20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
scripts/update_flowcapture.sh
Outdated
if ! command -v yq &> /dev/null | ||
then | ||
echo "yq binary not found, installing... " | ||
go install -mod='' github.com/mikefarah/yq/[email protected] | ||
fi |
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 would like to avoid as much as possible dependencies in the script
That will help to keep cross platform compatibility
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 removed it from the script and added to make prereqs
target
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.
ok thanks !
69736b6
to
c040d28
Compare
c040d28
to
e6b29cf
Compare
@msherif1234: This pull request references NETOBSERV-1609 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
@@ -57,6 +52,9 @@ function clusterIsReady() { | |||
fi | |||
} | |||
|
|||
MANIFEST_FILE="flow-capture.yml" |
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.
If you manipulate the file the inject.sh script will not work anymore 🤔
The goal is to embedd these yamls into variables and have a single shell as oc / krew plugin
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 update the PR to read the yaml after been updated by inject.sh lmk if that works
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.
ok I see the trick now. I'm not a huge fan of saving the flow-capture.yml
in the output
folder but that works on every situation now 👍
Would it be better to put that file in /tmp/
folder ?
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.
What about the PCAP filters ?
Don't we have extra ones too ?
e6b29cf
to
328ee75
Compare
no flow filter capability for agent flow engine the pcap hs its own cli |
328ee75
to
f7de8c8
Compare
@msherif1234: This pull request references NETOBSERV-1609 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
f7de8c8
to
dd2a8af
Compare
It would be awesome to have PCAP filters too ! Even if it's only a subset of what we have for flows. It would allow to run this CLI on huge clusters without exploding resources consumption. |
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.
It seems the filters are not correctly applyed when I run your example:
Running network-observability-cli as Flow Capture
Log level: info
Collection filters: --interfaces=eth0 --enable=true --protocol=TCP --cidr=0.0.0.0/0 --port-range=80-100 --peer-ip=1.1.1.1 --action=Accept
Showing last: 35 Use Up / Down keyboard arrows to increase / decrease limit
Display: Standard Use Left / Right keyboard arrows to cycle views
Enrichment: None Use Page Up / Page Down keyboard keys to cycle enrichment scopes
Time SrcAddr SrcPort DstAddr DstPort Dir Interfaces Proto Dscp Bytes Packets
07:55:22.707000 3f06:91fe:ac12:2:af4:108:c030:270f 49200 45c3:87b4:831f:786b:8018:fb:ba14:0 9999 n/a unknown TCP 20 510B 1
07:55:22.707000 3f06:93ca:ac12:2:af4:108:c030:270f 49200 45c3:8992:831f:7889:8018:fb:b847:0 9999 n/a unknown TCP 20 49B 1
07:55:22.713000 4006:469b:af4:101:af4:108:550:270f 1360 ee50:3969:26e3:31ac:8018:fb:1828:0 9999 n/a unknown TCP 20 49B 1
07:55:22.713000 4006:469b:af4:101:af4:108:550:270f 1360 ee50:3969:26e3:31ac:8018:fb:1828:0 9999 n/a unknown TCP 20 49B 1
07:55:22.720000 3f06:93c7:ac12:2:af4:108:c030:270f 49200 45c3:8b9f:831f:78dc:8018:fb:b847:0 9999 n/a unknown TCP 20 49B 1
07:55:22.726000 4006:4698:af4:101:af4:108:550:270f 1360 ee50:3b7b:26e3:31ff:8018:fb:1828:0 9999 n/a unknown TCP 20 49B 1
07:55:22.726000 4006:4698:af4:101:af4:108:550:270f 1360 ee50:3b7b:26e3:31ff:8018:fb:1828:0 9999 n/a unknown TCP 20 49B 1
07:55:22.732000 3f06:93b9:ac12:2:af4:108:c030:270f 49200 45c3:8bb0:831f:7900:8018:fb:b854:0 9999 n/a unknown TCP 20 62B 1
07:55:22.738000 4006:4695:af4:101:af4:108:550:270f 1360 ee50:3d87:26e3:3252:8018:fb:1828:0 9999 n/a unknown TCP 20 49B 1
07:55:22.744000 3f06:93b6:ac12:2:af4:108:c030:270f 49200 45c3:8dbf:831f:7953:8018:fb:b854:0 9999 n/a unknown TCP 20 62B 1
07:55:22.756000 3f06:93b3:ac12:2:af4:108:c030:270f 49200 45c3:8fcc:831f:79a6:8018:fb:b854:0 9999 n/a unknown TCP 20 62B 1
07:55:22.756000 3f06:91f4:ac12:2:af4:108:c030:270f 49200 45c3:8fea:831f:79b7:8018:fb:ba12:0 9999 n/a unknown TCP 20 508B 1
07:55:22.762000 4006:468f:af4:101:af4:108:550:270f 1360 ee50:419f:26e3:32f8:8018:fb:1828:0 9999 n/a unknown TCP 20 49B 1
07:55:22.779000 3f06:93ad:ac12:2:af4:108:c030:270f 49200 45c3:93e4:831f:7a4c:8018:fb:b854:0 9999 n/a unknown TCP 20 62B 1
07:55:22.785000 4006:9691:ac12:1:ac12:3:95e2:192b 38370 f672:44d3:9a82:b5bf:8010:a30:584f:0 6443 n/a unknown TCP 20 32B 1
07:55:22.785000 4006:2d50:ac12:3:ac12:4:8838:280a 34872 21e:64a9:f6bf:76:8010:aa0:5852:0 10250 n/a unknown TCP 20 32B 1
07:55:22.785000 4006:2d52:ac12:3:ac12:4:8838:280a 34872 21e:64a9:f6be:ee50:8010:aa0:5852:0 10250 n/a unknown TCP 20 32B 1
07:55:22.785000 4006:2d53:ac12:3:ac12:4:8838:280a 34872 21e:64a9:f6be:eabc:8010:aa0:5852:0 10250 n/a eth0 TCP 20 32B 1
07:55:22.785000 4006:4689:af4:101:af4:108:550:270f 1360 ee50:45bb:26e3:339e:8018:fb:1828:0 9999 n/a unknown TCP 20 49B 1
07:55:22.785000 4006:2d51:ac12:3:ac12:4:8838:280a 34872 21e:64a9:f6be:f0be:8010:aa0:5852:0 10250 n/a eth0 TCP 20 32B 1
07:55:22.785000 4006:2d53:ac12:3:ac12:4:8838:280a 34872 21e:64a9:f6be:eabc:8010:aa0:5852:0 10250 n/a unknown TCP 20 32B 1
07:55:22.785000 4006:2d4f:ac12:3:ac12:4:8838:280a 34872 21e:64a9:f6bf:86f:8010:aa0:5852:0 10250 n/a unknown TCP 20 32B 1
07:55:22.785000 4006:9692:ac12:1:ac12:3:95e2:192b 38370 f672:44d3:9a82:a607:8010:a30:584f:0 6443 n/a vetha40d0d3a TCP 20 32B 1
07:55:22.785000 4006:44b8:af4:101:af4:108:550:270f 1360 ee50:43d8:26e3:3380:8018:fb:19fa:0 9999 n/a unknown TCP 20 515B 1
07:55:22.785000 4006:968d:ac12:1:ac12:3:95e2:192b 38370 f672:44d3:9a82:c81f:8010:a30:584f:0 6443 n/a unknown TCP 20 32B 1
07:55:22.785000 4006:2d54:ac12:3:ac12:4:8838:280a 34872 21e:64a9:f6be:e8e2:8010:aa0:5852:0 10250 n/a unknown TCP 20 32B 1
07:55:22.785000 4006:9696:ac12:1:ac12:3:95e2:192b 38370 f672:44d3:9a82:9e2b:8010:a30:584f:0 6443 n/a vetha40d0d3a TCP 20 32B 1
07:55:22.785000 4006:9694:ac12:1:ac12:3:95e2:192b 38370 f672:44d3:9a82:a023:8010:a30:584f:0 6443 n/a unknown TCP 20 32B 1
07:55:22.791000 3f06:93aa:ac12:2:af4:108:c030:270f 49200 45c3:95f0:831f:7a9f:8018:fb:b854:0 9999 n/a unknown TCP 20 62B 1
07:55:22.796000 4006:467b:af4:101:af4:108:550:270f 1360 ee50:45cc:26e3:33c2:8018:fb:1835:0 9999 n/a unknown TCP 20 62B 1
07:55:22.796000 4006:4686:af4:101:af4:108:550:270f 1360 ee50:47c6:26e3:33f1:8018:fb:1828:0 9999 n/a unknown TCP 20 49B 1
07:55:22.796000 4006:4686:af4:101:af4:108:550:270f 1360 ee50:47c6:26e3:33f1:8018:fb:1828:0 9999 n/a unknown TCP 20 49B 1
07:55:22.811000 4006:4678:af4:101:af4:108:550:270f 1360 ee50:47d7:26e3:3415:8018:fb:1835:0 9999 n/a unknown TCP 20 62B 1
07:55:22.811000 4006:4678:af4:101:af4:108:550:270f 1360 ee50:47d7:26e3:3415:8018:fb:1835:0 9999 n/a unknown TCP 20 62B 1
07:55:22.812000 4006:4683:af4:101:af4:108:550:270f 1360 ee50:49d2:26e3:3444:8018:fb:1828:0 9999 n/a unknown TCP 20 49B 1
---------------- ---------------------------------------- ------ ---------------------------------------- ------ ---------- ---------------- ------ -------- ------ ------
Type anything to filter incoming flows in view
I was expecting only eth0 interfaces on ports between 80 to 100 in this view. Did I missed something ?
scripts/functions.sh
Outdated
--enable) # Enable flow filter | ||
if [[ "$value" == "true" || "$value" == "false" ]]; then | ||
edit_manifest "filter_enable" "$value" "$manifest" | ||
else | ||
echo "invalid value for --enable" | ||
fi | ||
;; |
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.
What's the point having this ?
As soon as we have one+ filter, we should automatically set it to true
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.
its like most of the agent feature to enable it u need to explicitly enable it
scripts/update_flowcapture.sh
Outdated
if ! command -v yq &> /dev/null | ||
then | ||
echo "yq binary not found, installing... " | ||
go install -mod='' github.com/mikefarah/yq/[email protected] | ||
fi |
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.
ok thanks !
@@ -57,6 +52,9 @@ function clusterIsReady() { | |||
fi | |||
} | |||
|
|||
MANIFEST_FILE="flow-capture.yml" |
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.
ok I see the trick now. I'm not a huge fan of saving the flow-capture.yml
in the output
folder but that works on every situation now 👍
Would it be better to put that file in /tmp/
folder ?
Yeah my example was just to test config very unlikely peer ip will match u can remove it from config and see, also not sure if |
flow filter and pca configs might collide maybe can be a future task if we can find a proper use case |
dd2a8af
to
88b0040
Compare
88b0040
to
5bdcc9e
Compare
Signed-off-by: Mohamed Mahmoud <[email protected]>
5bdcc9e
to
a508962
Compare
Looks good @msherif1234 ! Thanks ! |
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: 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 |
Description
how we can pass in flow filter configs and use it to update flow collector manifests
Example
Dependencies
n/a
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.