NETOBSERV-1617: use filter with packets capture feature#54
NETOBSERV-1617: use filter with packets capture feature#54openshift-merge-bot[bot] merged 1 commit intonetobserv:mainfrom
Conversation
|
@msherif1234: This pull request references NETOBSERV-1617 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.17.0" version, but no target version was set. DetailsIn 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. |
msherif1234
force-pushed
the
update-pcap
branch
3 times, most recently
from
531f684 to
8752b5c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
=======================================
Coverage 28.99% 28.99%
=======================================
Files 8 8
Lines 1045 1045
=======================================
Hits 303 303
Misses 720 720
Partials 22 22
Flags with carried forward coverage won't be shown. Click here to find out more. |
scripts/functions.sh
Outdated
| for option in $options; do | ||
| key="${option%%=*}" | ||
| value="${option#*=}" | ||
| case "$key" in | ||
| --enable_pca) # Enable packet capture agent | ||
| if [[ "$value" == "true" || "$value" == "false" ]]; then | ||
| edit_manifest "pca_enable" "$value" "$manifest" | ||
| else | ||
| echo "invalid value for --pca_enable" | ||
| fi | ||
| ;; | ||
| --direction) # Configure filter direction | ||
| if [[ "$value" == "Ingress" || "$value" == "Egress" ]]; then | ||
| edit_manifest "filter_direction" "$value" "$manifest" | ||
| else | ||
| echo "invalid value for --direction" | ||
| fi | ||
| ;; | ||
| --cidr) # Configure filter CIDR | ||
| edit_manifest "filter_cidr" "$value" "$manifest" | ||
| ;; | ||
| --protocol) # Configure filter protocol | ||
| if [[ "$value" == "TCP" || "$value" == "UDP" || "$value" == "SCTP" || "$value" == "ICMP" || "$value" == "ICMPv6" ]]; then | ||
| edit_manifest "filter_protocol" "$value" "$manifest" | ||
| else | ||
| echo "invalid value for --protocol" | ||
| fi | ||
| ;; | ||
| --sport) # Configure filter source port | ||
| edit_manifest "filter_sport" "$value" "$manifest" | ||
| ;; | ||
| --dport) # Configure filter destination port | ||
| edit_manifest "filter_dport" "$value" "$manifest" | ||
| ;; | ||
| --port) # Configure filter port | ||
| edit_manifest "filter_port" "$value" "$manifest" | ||
| ;; | ||
| --sport_range) # Configure filter source port range | ||
| edit_manifest "filter_sport_range" "$value" "$manifest" | ||
| ;; | ||
| --dport_range) # Configure filter destination port range | ||
| edit_manifest "filter_dport_range" "$value" "$manifest" | ||
| ;; | ||
| --port_range) # Configure filter port range | ||
| edit_manifest "filter_port_range" "$value" "$manifest" | ||
| ;; | ||
| --icmp_type) # ICMP type | ||
| edit_manifest "filter_icmp_type" "$value" "$manifest" | ||
| ;; | ||
| --icmp_code) # ICMP code | ||
| edit_manifest "filter_icmp_code" "$value" "$manifest" | ||
| ;; | ||
| --peer_ip) # Peer IP | ||
| edit_manifest "filter_peer_ip" "$value" "$manifest" | ||
| ;; | ||
| --action) # Filter action | ||
| if [[ "$value" == "Accept" || "$value" == "Reject" ]]; then | ||
| edit_manifest "filter_action" "$value" "$manifest" | ||
| else | ||
| echo "invalid value for --action" | ||
| fi | ||
| ;; | ||
| *) # Invalid option | ||
| echo "Invalid option: $key" >&2 | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| done |
There was a problem hiding this comment.
Can't we merge that loop in a single function to avoid code duplicate ?
We can add check for options not available in both flows / packets commands stopping the script
There was a problem hiding this comment.
ok will gen common function
scripts/functions.sh
Outdated
| --enable_pca) # Enable packet capture agent | ||
| if [[ "$value" == "true" || "$value" == "false" ]]; then | ||
| edit_manifest "pca_enable" "$value" "$manifest" | ||
| else | ||
| echo "invalid value for --pca_enable" | ||
| fi | ||
| ;; |
There was a problem hiding this comment.
Not sure to get the point of this option.
To run a packet capture, the user is supposed to type oc netobserv packets right ?
There was a problem hiding this comment.
Yes the above default is true the only use of this as way to disable pca which I don’t see a use for it in production see the new doc
There was a problem hiding this comment.
I will drop this option
|
@msherif1234: This pull request references NETOBSERV-1617 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.17.0" version, but no target version was set. DetailsIn 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. |
jpinsonneau
left a comment
jpinsonneau
left a comment
There was a problem hiding this comment.
Code looks good, thanks @msherif1234 !
|
/ok-to-test |
|
New image: It will expire after two weeks. To use this build, update your commands using: USER=netobserv VERSION=2fd8052 make commands |
|
/retest |
|
New changes are detected. LGTM label has been removed. |
|
fixed the help strings to match with the new pca fmt for e2e to pass |
Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>
|
New changes are detected. LGTM label has been removed. |
|
[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 DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Was this also meant to be |
4 participants
Description
add filter capability for pca feature
Example from running on
KindclusterNETOBSERV_AGENT_IMAGE=quay.io/netobserv/netobserv-ebpf-agent:f525832 ./build/oc-netobserv packets --protocol="TCP" --port=443output pcap
Dependencies
netobserv/netobserv-ebpf-agent#359
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.