NETOBSERV-2471: Allow space in filtered values#1393
Conversation
|
@jotak: This pull request references NETOBSERV-2471 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.22.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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated module/vendor dependency versions; removed large vendored libraries (goflow2, pflag) and many k8s clientcmd/auth files; allowed spaces in Loki filter values and expanded tests; added TLS-related frontend columns/filters and sample-config changes; small golang.org/x/* adjustments. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
@jotak: This pull request references NETOBSERV-2471 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.22.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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/loki/flow_query.go (1)
25-26: Comment is stale; consider clarifying hyphen placement.The comment on line 25 doesn't mention that spaces are now allowed. Also, placing the hyphen explicitly at the end of the character class (e.g.,
[\w _.,"*:/-]) would make the regex intent clearer.Suggested fix
-// can contains only alphanumeric / '-' / '_' / '.' / ',' / '"' / '*' / ':' / '/' characteres -var filterRegexpValidation = regexp.MustCompile(`^[\w -_.,\"*:/]*$`) +// can contain only alphanumeric / space / '-' / '_' / '.' / ',' / '"' / '*' / ':' / '/' characters +var filterRegexpValidation = regexp.MustCompile(`^[\w _.,"*:/-]*$`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/loki/flow_query.go` around lines 25 - 26, The comment for filterRegexpValidation is outdated and the hyphen inside the character class is ambiguous; update the comment to note that spaces are allowed and clarify allowed characters, and adjust the regexp in filterRegexpValidation to place the hyphen at the end of the class (e.g., move '-' to the end) so its intent is unambiguous while preserving existing allowed characters (alphanumeric, space, '_', '.', ',', '"', '*', ':', '/'); ensure the variable name filterRegexpValidation is updated with the new regex and the comment reflects the exact allowed set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vendor/modules.txt`:
- Around line 80-82: The replace directive currently points to a personal fork
"github.com/jotak/netobserv-agent v0.0.0-20260331075254-f72ba0ce9bde" instead of
the official module; switch the replacement back to the official module name
"github.com/netobserv/netobserv-ebpf-agent" and use a proper released semantic
version (e.g., v1.11.2-community or a newer official release) rather than the
pseudo-version, then run dependency update (go mod tidy / vendor refresh) so
vendor/modules.txt (the line showing "github.com/netobserv/netobserv-ebpf-agent
... => github.com/jotak/netobserv-agent ...") is regenerated with the official
release.
---
Nitpick comments:
In `@pkg/loki/flow_query.go`:
- Around line 25-26: The comment for filterRegexpValidation is outdated and the
hyphen inside the character class is ambiguous; update the comment to note that
spaces are allowed and clarify allowed characters, and adjust the regexp in
filterRegexpValidation to place the hyphen at the end of the class (e.g., move
'-' to the end) so its intent is unambiguous while preserving existing allowed
characters (alphanumeric, space, '_', '.', ',', '"', '*', ':', '/'); ensure the
variable name filterRegexpValidation is updated with the new regex and the
comment reflects the exact allowed set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9dbfc6df-f8c7-4e66-9063-da8d75c67907
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumvendor/github.com/netsampler/goflow2/pb/flow.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (104)
go.modpkg/decoders/network-events.gopkg/loki/flow_query.gopkg/loki/query_test.govendor/github.com/netobserv/flowlogs-pipeline/pkg/api/ingest_ipfix.govendor/github.com/netobserv/flowlogs-pipeline/pkg/utils/kubernetes.govendor/github.com/netobserv/netobserv-ebpf-agent/pkg/utils/network_events.govendor/github.com/netobserv/netobserv-ebpf-agent/pkg/utils/networkevents/network_events.govendor/github.com/netobserv/netobserv-ebpf-agent/pkg/utils/utils.govendor/github.com/netsampler/goflow2/LICENSEvendor/github.com/netsampler/goflow2/decoders/netflow/ipfix.govendor/github.com/netsampler/goflow2/decoders/netflow/netflow.govendor/github.com/netsampler/goflow2/decoders/netflow/nfv9.govendor/github.com/netsampler/goflow2/decoders/netflow/packet.govendor/github.com/netsampler/goflow2/decoders/netflow/templates/templates.govendor/github.com/netsampler/goflow2/decoders/netflowlegacy/netflow.govendor/github.com/netsampler/goflow2/decoders/netflowlegacy/packet.govendor/github.com/netsampler/goflow2/decoders/sflow/datastructure.govendor/github.com/netsampler/goflow2/decoders/sflow/packet.govendor/github.com/netsampler/goflow2/decoders/sflow/sflow.govendor/github.com/netsampler/goflow2/decoders/utils/utils.govendor/github.com/netsampler/goflow2/pb/flow.protovendor/github.com/netsampler/goflow2/producer/producer_nf.govendor/github.com/netsampler/goflow2/producer/producer_nflegacy.govendor/github.com/netsampler/goflow2/producer/producer_sf.govendor/github.com/netsampler/goflow2/producer/reflect.govendor/github.com/spf13/pflag/.editorconfigvendor/github.com/spf13/pflag/.gitignorevendor/github.com/spf13/pflag/.golangci.yamlvendor/github.com/spf13/pflag/.travis.ymlvendor/github.com/spf13/pflag/LICENSEvendor/github.com/spf13/pflag/README.mdvendor/github.com/spf13/pflag/bool.govendor/github.com/spf13/pflag/bool_func.govendor/github.com/spf13/pflag/bool_slice.govendor/github.com/spf13/pflag/bytes.govendor/github.com/spf13/pflag/count.govendor/github.com/spf13/pflag/duration.govendor/github.com/spf13/pflag/duration_slice.govendor/github.com/spf13/pflag/errors.govendor/github.com/spf13/pflag/flag.govendor/github.com/spf13/pflag/float32.govendor/github.com/spf13/pflag/float32_slice.govendor/github.com/spf13/pflag/float64.govendor/github.com/spf13/pflag/float64_slice.govendor/github.com/spf13/pflag/func.govendor/github.com/spf13/pflag/golangflag.govendor/github.com/spf13/pflag/int.govendor/github.com/spf13/pflag/int16.govendor/github.com/spf13/pflag/int32.govendor/github.com/spf13/pflag/int32_slice.govendor/github.com/spf13/pflag/int64.govendor/github.com/spf13/pflag/int64_slice.govendor/github.com/spf13/pflag/int8.govendor/github.com/spf13/pflag/int_slice.govendor/github.com/spf13/pflag/ip.govendor/github.com/spf13/pflag/ip_slice.govendor/github.com/spf13/pflag/ipmask.govendor/github.com/spf13/pflag/ipnet.govendor/github.com/spf13/pflag/ipnet_slice.govendor/github.com/spf13/pflag/string.govendor/github.com/spf13/pflag/string_array.govendor/github.com/spf13/pflag/string_slice.govendor/github.com/spf13/pflag/string_to_int.govendor/github.com/spf13/pflag/string_to_int64.govendor/github.com/spf13/pflag/string_to_string.govendor/github.com/spf13/pflag/text.govendor/github.com/spf13/pflag/time.govendor/github.com/spf13/pflag/uint.govendor/github.com/spf13/pflag/uint16.govendor/github.com/spf13/pflag/uint32.govendor/github.com/spf13/pflag/uint64.govendor/github.com/spf13/pflag/uint8.govendor/github.com/spf13/pflag/uint_slice.govendor/golang.org/x/net/http2/writesched_priority_rfc9218.govendor/golang.org/x/sys/plan9/syscall_plan9.govendor/golang.org/x/sys/unix/ztypes_linux.govendor/golang.org/x/sys/windows/aliases.govendor/golang.org/x/sys/windows/syscall_windows.govendor/golang.org/x/term/terminal.govendor/k8s.io/client-go/tools/auth/OWNERSvendor/k8s.io/client-go/tools/auth/clientauth.govendor/k8s.io/client-go/tools/clientcmd/api/latest/latest.govendor/k8s.io/client-go/tools/clientcmd/api/v1/conversion.govendor/k8s.io/client-go/tools/clientcmd/api/v1/defaults.govendor/k8s.io/client-go/tools/clientcmd/api/v1/doc.govendor/k8s.io/client-go/tools/clientcmd/api/v1/register.govendor/k8s.io/client-go/tools/clientcmd/api/v1/types.govendor/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.govendor/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.deepcopy.govendor/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.defaults.govendor/k8s.io/client-go/tools/clientcmd/auth_loaders.govendor/k8s.io/client-go/tools/clientcmd/client_config.govendor/k8s.io/client-go/tools/clientcmd/config.govendor/k8s.io/client-go/tools/clientcmd/doc.govendor/k8s.io/client-go/tools/clientcmd/flag.govendor/k8s.io/client-go/tools/clientcmd/helpers.govendor/k8s.io/client-go/tools/clientcmd/loader.govendor/k8s.io/client-go/tools/clientcmd/merge.govendor/k8s.io/client-go/tools/clientcmd/merged_client_builder.govendor/k8s.io/client-go/tools/clientcmd/overrides.govendor/k8s.io/client-go/tools/clientcmd/validation.govendor/k8s.io/client-go/util/homedir/homedir.govendor/modules.txt
💤 Files with no reviewable changes (92)
- vendor/github.com/spf13/pflag/.editorconfig
- vendor/github.com/spf13/pflag/.travis.yml
- vendor/k8s.io/client-go/tools/auth/clientauth.go
- vendor/github.com/netsampler/goflow2/LICENSE
- vendor/github.com/spf13/pflag/.golangci.yaml
- vendor/github.com/spf13/pflag/LICENSE
- vendor/github.com/netsampler/goflow2/pb/flow.proto
- vendor/github.com/spf13/pflag/.gitignore
- vendor/github.com/netsampler/goflow2/decoders/netflow/templates/templates.go
- vendor/github.com/spf13/pflag/string_slice.go
- vendor/github.com/spf13/pflag/bool_func.go
- vendor/k8s.io/client-go/tools/clientcmd/api/v1/doc.go
- vendor/github.com/netsampler/goflow2/decoders/sflow/packet.go
- vendor/k8s.io/client-go/tools/auth/OWNERS
- vendor/k8s.io/client-go/tools/clientcmd/flag.go
- vendor/github.com/netobserv/netobserv-ebpf-agent/pkg/utils/utils.go
- vendor/github.com/spf13/pflag/uint8.go
- vendor/github.com/netsampler/goflow2/decoders/netflowlegacy/packet.go
- vendor/k8s.io/client-go/tools/clientcmd/api/v1/register.go
- vendor/k8s.io/client-go/tools/clientcmd/helpers.go
- vendor/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.defaults.go
- vendor/github.com/spf13/pflag/string_to_int64.go
- vendor/github.com/spf13/pflag/int_slice.go
- vendor/github.com/spf13/pflag/text.go
- vendor/github.com/netobserv/netobserv-ebpf-agent/pkg/utils/network_events.go
- vendor/golang.org/x/sys/windows/syscall_windows.go
- vendor/github.com/spf13/pflag/uint64.go
- vendor/k8s.io/client-go/tools/clientcmd/api/v1/defaults.go
- vendor/github.com/netsampler/goflow2/producer/producer_nflegacy.go
- vendor/github.com/spf13/pflag/func.go
- vendor/github.com/netobserv/flowlogs-pipeline/pkg/utils/kubernetes.go
- vendor/k8s.io/client-go/util/homedir/homedir.go
- vendor/github.com/spf13/pflag/uint16.go
- vendor/k8s.io/client-go/tools/clientcmd/api/latest/latest.go
- vendor/k8s.io/client-go/tools/clientcmd/auth_loaders.go
- vendor/github.com/netsampler/goflow2/decoders/netflow/nfv9.go
- vendor/github.com/spf13/pflag/ip_slice.go
- vendor/github.com/spf13/pflag/README.md
- vendor/github.com/spf13/pflag/int64_slice.go
- vendor/github.com/spf13/pflag/float32_slice.go
- vendor/github.com/spf13/pflag/bool_slice.go
- vendor/github.com/spf13/pflag/bool.go
- vendor/github.com/netsampler/goflow2/decoders/sflow/sflow.go
- vendor/github.com/spf13/pflag/int.go
- vendor/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go
- vendor/k8s.io/client-go/tools/clientcmd/merged_client_builder.go
- vendor/github.com/spf13/pflag/uint32.go
- vendor/k8s.io/client-go/tools/clientcmd/merge.go
- vendor/github.com/spf13/pflag/errors.go
- vendor/github.com/spf13/pflag/duration.go
- vendor/github.com/spf13/pflag/int8.go
- vendor/github.com/spf13/pflag/int16.go
- vendor/github.com/netsampler/goflow2/decoders/utils/utils.go
- vendor/github.com/netsampler/goflow2/decoders/netflow/ipfix.go
- vendor/github.com/spf13/pflag/time.go
- vendor/k8s.io/client-go/tools/clientcmd/doc.go
- vendor/github.com/spf13/pflag/int32_slice.go
- vendor/github.com/spf13/pflag/golangflag.go
- vendor/github.com/spf13/pflag/int64.go
- vendor/github.com/spf13/pflag/ip.go
- vendor/github.com/spf13/pflag/float32.go
- vendor/k8s.io/client-go/tools/clientcmd/config.go
- vendor/github.com/spf13/pflag/count.go
- vendor/github.com/spf13/pflag/duration_slice.go
- vendor/k8s.io/client-go/tools/clientcmd/api/v1/types.go
- vendor/github.com/spf13/pflag/int32.go
- vendor/github.com/netsampler/goflow2/producer/reflect.go
- vendor/github.com/netsampler/goflow2/decoders/netflow/netflow.go
- vendor/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.deepcopy.go
- vendor/github.com/netsampler/goflow2/decoders/netflowlegacy/netflow.go
- vendor/github.com/spf13/pflag/ipnet.go
- vendor/github.com/spf13/pflag/bytes.go
- vendor/k8s.io/client-go/tools/clientcmd/api/v1/conversion.go
- vendor/github.com/spf13/pflag/flag.go
- vendor/github.com/netsampler/goflow2/producer/producer_sf.go
- vendor/github.com/netsampler/goflow2/decoders/sflow/datastructure.go
- vendor/github.com/spf13/pflag/float64_slice.go
- vendor/github.com/spf13/pflag/ipnet_slice.go
- vendor/github.com/spf13/pflag/uint_slice.go
- vendor/k8s.io/client-go/tools/clientcmd/client_config.go
- vendor/github.com/spf13/pflag/string_to_int.go
- vendor/github.com/spf13/pflag/float64.go
- vendor/github.com/netsampler/goflow2/producer/producer_nf.go
- vendor/k8s.io/client-go/tools/clientcmd/validation.go
- vendor/github.com/spf13/pflag/ipmask.go
- vendor/k8s.io/client-go/tools/clientcmd/overrides.go
- vendor/github.com/spf13/pflag/string_array.go
- vendor/github.com/spf13/pflag/string.go
- vendor/k8s.io/client-go/tools/clientcmd/loader.go
- vendor/github.com/spf13/pflag/string_to_string.go
- vendor/github.com/spf13/pflag/uint.go
- vendor/github.com/netsampler/goflow2/decoders/netflow/packet.go
vendor/modules.txt
Outdated
| # github.com/netobserv/netobserv-ebpf-agent v1.11.2-community => github.com/jotak/netobserv-agent v0.0.0-20260331075254-f72ba0ce9bde | ||
| ## explicit; go 1.25.0 | ||
| github.com/netobserv/netobserv-ebpf-agent/pkg/utils | ||
| # github.com/netsampler/goflow2 v1.3.7 | ||
| ## explicit; go 1.18 | ||
| github.com/netsampler/goflow2/decoders/netflow | ||
| github.com/netsampler/goflow2/decoders/netflow/templates | ||
| github.com/netsampler/goflow2/decoders/netflowlegacy | ||
| github.com/netsampler/goflow2/decoders/sflow | ||
| github.com/netsampler/goflow2/decoders/utils | ||
| github.com/netsampler/goflow2/pb | ||
| github.com/netsampler/goflow2/producer | ||
| github.com/netobserv/netobserv-ebpf-agent/pkg/utils/networkevents |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the dependent PR in netobserv-ebpf-agent is merged
gh pr view 815 --repo netobserv/netobserv-ebpf-agent --json state,mergedAtRepository: netobserv/netobserv-web-console
Length of output: 173
🏁 Script executed:
# First, locate and examine the go.mod file
find . -name "go.mod" -type f | head -5Repository: netobserv/netobserv-web-console
Length of output: 83
🏁 Script executed:
# Search for references to the netobserv-agent fork and related TODOs/notes
rg "jotak/netobserv-agent|netobserv-ebpf-agent" -i --type-list | head -20Repository: netobserv/netobserv-web-console
Length of output: 464
🏁 Script executed:
# Look for any related comments or context about this replacement
rg "replace.*netobserv|fork|temporary|development" -i -A 2 -B 2Repository: netobserv/netobserv-web-console
Length of output: 50388
🏁 Script executed:
cat -n go.mod | head -100Repository: netobserv/netobserv-web-console
Length of output: 3531
🏁 Script executed:
# Search for any TODO or comments related to this fork replacement
rg "TODO|FIXME|temporary|develop" go.mod -i -B 2 -A 2Repository: netobserv/netobserv-web-console
Length of output: 57
🏁 Script executed:
# Search for references to netobserv-agent or related fork context
rg "jotak|netobserv-agent" -i --type goRepository: netobserv/netobserv-web-console
Length of output: 57
Replace directive points to a personal fork with a pseudo-version.
The module is replaced with github.com/jotak/netobserv-agent using a development pseudo-version (v0.0.0-20260331075254-f72ba0ce9bde). Ensure this is updated to the official github.com/netobserv/netobserv-ebpf-agent release before merging to main.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vendor/modules.txt` around lines 80 - 82, The replace directive currently
points to a personal fork "github.com/jotak/netobserv-agent
v0.0.0-20260331075254-f72ba0ce9bde" instead of the official module; switch the
replacement back to the official module name
"github.com/netobserv/netobserv-ebpf-agent" and use a proper released semantic
version (e.g., v1.11.2-community or a newer official release) rather than the
pseudo-version, then run dependency update (go mod tidy / vendor refresh) so
vendor/modules.txt (the line showing "github.com/netobserv/netobserv-ebpf-agent
... => github.com/jotak/netobserv-agent ...") is regenerated with the official
release.
jotak
added
the
ok-to-test
|
@jotak: This pull request references NETOBSERV-2471 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.22.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. |
|
New image: quay.io/netobserv/network-observability-console-plugin:ea51880
It will expire in two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=ea51880 make set-plugin-image |
github-actions
bot
removed
the
ok-to-test
jotak
added
the
ok-to-test
|
@jotak: This pull request references NETOBSERV-2471 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.22.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. |
|
New image: quay.io/netobserv/network-observability-console-plugin:502d5d0
It will expire in two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=502d5d0 make set-plugin-image |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/components/drawer/record/record-panel.tsx (1)
238-267: Implementation follows established patterns.The function correctly mirrors
getGenericFilterbut handles multi-value scenarios. One optional style improvement:♻️ Use ES6 shorthand property syntax
- const filterValues: FilterValue[] = values.map(v => ({v: v})); + const filterValues: FilterValue[] = values.map(v => ({ v }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/drawer/record/record-panel.tsx` around lines 238 - 267, The implementation of getGenericAnyOfFilter should use ES6 object property shorthand where possible to make the code cleaner; update object literals like { def: def, compare: FilterCompare.equal } and { v: v } to use shorthand ({ def, compare: FilterCompare.equal } and { v }) and similarly other places constructing filter objects so they use shorthand properties (e.g., newFilters.push({ def, compare: FilterCompare.equal, values: filterValues }) and the returned object with isDelete) while keeping all logic in getGenericAnyOfFilter, references to filterDefinitions, filters, setFilters, filterKey, filterValues and isDelete intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/src/components/drawer/record/record-panel.tsx`:
- Around line 238-267: The implementation of getGenericAnyOfFilter should use
ES6 object property shorthand where possible to make the code cleaner; update
object literals like { def: def, compare: FilterCompare.equal } and { v: v } to
use shorthand ({ def, compare: FilterCompare.equal } and { v }) and similarly
other places constructing filter objects so they use shorthand properties (e.g.,
newFilters.push({ def, compare: FilterCompare.equal, values: filterValues }) and
the returned object with isDelete) while keeping all logic in
getGenericAnyOfFilter, references to filterDefinitions, filters, setFilters,
filterKey, filterValues and isDelete intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6228387a-391f-48b3-a1d1-e69a1d4e4082
📒 Files selected for processing (3)
pkg/loki/flow_query.goweb/src/components/drawer/record/record-panel.tsxweb/src/utils/columns.ts
✅ Files skipped from review due to trivial changes (1)
- pkg/loki/flow_query.go
github-actions
bot
removed
the
ok-to-test
jotak
added
the
ok-to-test
|
@jotak: This pull request references NETOBSERV-2471 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.22.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. |
|
New image: quay.io/netobserv/network-observability-console-plugin:7034ec3
It will expire in two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=7034ec3 make set-plugin-image |
|
/label qe-approved |
|
@jotak: This pull request references NETOBSERV-2471 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.22.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. |
|
|
||
| // can contains only alphanumeric / '-' / '_' / '.' / ',' / '"' / '*' / ':' / '/' characteres | ||
| var filterRegexpValidation = regexp.MustCompile(`^[\w-_.,\"*:/]*$`) | ||
| var filterRegexpValidation = regexp.MustCompile(`^[\w -_.,\"*:/]*$`) |
There was a problem hiding this comment.
nit: add the space in the comment above
| ifdirs = 'IfDirections', | ||
| flowdirints = 'FlowDirInts', | ||
| udns = 'Udns', | ||
| tlstypes = 'TLSTypes', |
There was a problem hiding this comment.
Can you please update https://github.com/netobserv/netobserv-web-console/blob/main/config/sample-config.yaml too ?
github-actions
bot
removed
the
ok-to-test
|
@jotak: This pull request references NETOBSERV-2471 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.22.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. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/sample-config.yaml (1)
986-997:⚠️ Potential issue | 🟡 MinorUpdate the examples to match the new multi-value input.
This block now advertises comma-separated values, but the examples still say
Specify a single .... The same mismatch is repeated in the mirrored source/destination/name/owner/node examples below.Suggested wording
examples: |- - Specify a single kubernetes name following these rules: + Specify one or more kubernetes names following these rules: + - Separate values with commas; spaces after commas are allowed, e.g. netobserv, kube-system - Containing any alphanumeric, hyphen, underscrore or dot character🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/sample-config.yaml` around lines 986 - 997, The examples text under the placeholder/hint for the kubernetes filters (e.g., the block with id: src_namespace and the mirrored source/destination/name/owner/node blocks) still says "Specify a single kubernetes name" but the input now accepts comma-separated multi-values; update the wording to reflect multi-value input (e.g., "Specify one or more kubernetes names separated by commas, following these rules:") and keep the same bullet rules and examples; apply the same phrasing change to each mirrored block (src_namespace and its destination/name/owner/node counterparts) so the documentation matches the new multi-value behaviour.
🧹 Nitpick comments (1)
config/sample-config.yaml (1)
692-714: KeeptlsTrackingin sync with the typed feature model.These additions introduce a new feature literal, but
web/src/model/config.ts:8-17still omitstlsTrackingfromFeature. That leaves this gate as an unchecked config string.Suggested follow-up
export type Feature = | 'multiCluster' | 'zones' | 'pktDrop' | 'dnsTracking' | 'flowRTT' | 'udnMapping' | 'packetTranslation' | 'networkEvents' - | 'ipsec'; + | 'ipsec' + | 'tlsTracking';Also applies to: 1402-1425
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/sample-config.yaml` around lines 692 - 714, The config added a new feature literal "tlsTracking" but the typed Feature model (the Feature type/enum in web/src/model/config.ts, currently defined around the top of that file) doesn't include it; update the Feature definition to include "tlsTracking" (or add it to the Feature enum/union) so the config string is type-checked, and scan usages of Feature (e.g., any code referencing Feature, validateFeature, or feature gates) to ensure they accept the new value as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/sample-config.yaml`:
- Line 28: Uncommented sample Alertmanager URL currently sits at the same
indentation as prometheus.url and will be parsed as a second prometheus.url;
move/uncomment that line so it is nested under the alertManager key (i.e., make
it prometheus.alertManager.url by indenting the "url: https://localhost:9094"
line under the existing alertManager block) so the URL is associated with
alertManager rather than the top-level prometheus.url.
- Around line 1523-1526: The YAML fragment contains a duplicated key
"placeholder" (same value 'E.g: NoError, NXDomain, NotAuth') which will break
strict YAML parsers; remove the second occurrence (the duplicate "placeholder"
adjacent to the "hint" and "examples" keys) so only one "placeholder" key
remains in the block that includes the "hint" and "examples" entries.
---
Outside diff comments:
In `@config/sample-config.yaml`:
- Around line 986-997: The examples text under the placeholder/hint for the
kubernetes filters (e.g., the block with id: src_namespace and the mirrored
source/destination/name/owner/node blocks) still says "Specify a single
kubernetes name" but the input now accepts comma-separated multi-values; update
the wording to reflect multi-value input (e.g., "Specify one or more kubernetes
names separated by commas, following these rules:") and keep the same bullet
rules and examples; apply the same phrasing change to each mirrored block
(src_namespace and its destination/name/owner/node counterparts) so the
documentation matches the new multi-value behaviour.
---
Nitpick comments:
In `@config/sample-config.yaml`:
- Around line 692-714: The config added a new feature literal "tlsTracking" but
the typed Feature model (the Feature type/enum in web/src/model/config.ts,
currently defined around the top of that file) doesn't include it; update the
Feature definition to include "tlsTracking" (or add it to the Feature
enum/union) so the config string is type-checked, and scan usages of Feature
(e.g., any code referencing Feature, validateFeature, or feature gates) to
ensure they accept the new value as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60c7a961-cef6-487c-96b5-dc9569f9aec0
📒 Files selected for processing (2)
config/sample-config.yamlpkg/loki/flow_query.go
✅ Files skipped from review due to trivial changes (1)
- pkg/loki/flow_query.go
| # tokenPath: /tmp/oc-token.txt | ||
| alertManager: | ||
| # url: https://localhost:9094 | ||
| # url: https://localhost:9094 |
There was a problem hiding this comment.
Indent the sample Alertmanager URL under alertManager.
Uncommenting Line 28 as-is turns it into a second prometheus.url, not prometheus.alertManager.url.
Suggested fix
alertManager:
- # url: https://localhost:9094
+ # url: https://localhost:9094📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # url: https://localhost:9094 | |
| alertManager: | |
| # url: https://localhost:9094 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/sample-config.yaml` at line 28, Uncommented sample Alertmanager URL
currently sits at the same indentation as prometheus.url and will be parsed as a
second prometheus.url; move/uncomment that line so it is nested under the
alertManager key (i.e., make it prometheus.alertManager.url by indenting the
"url: https://localhost:9094" line under the existing alertManager block) so the
URL is associated with alertManager rather than the top-level prometheus.url.
| placeholder: 'E.g: NoError, NXDomain, NotAuth' | ||
| hint: Add DNS RCODE name filter. | ||
| placeholder: 'E.g: NoError, NXDomain, NotAuth' | ||
| examples: |- |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
from pathlib import Path
path = Path("config/sample-config.yaml")
lines = path.read_text().splitlines()
start = next(i for i, line in enumerate(lines) if line.strip() == "- id: dns_flag_response_code")
block = []
for line in lines[start:]:
if block and line.startswith(" - id: "):
break
block.append(line)
print("dns_flag_response_code block:")
for idx, line in enumerate(block, start=start + 1):
print(f"{idx}: {line}")
counts = {}
for line in block[1:]:
stripped = line.strip()
if not stripped or stripped.startswith("#") or ":" not in stripped:
continue
key = stripped.split(":", 1)[0]
counts[key] = counts.get(key, 0) + 1
dups = {k: v for k, v in counts.items() if v > 1}
print("\nduplicate keys:", dups)
PYRepository: netobserv/netobserv-web-console
Length of output: 761
Remove the duplicated placeholder key on line 1525.
Line 1523 and 1525 both define placeholder: 'E.g: NoError, NXDomain, NotAuth'. Strict YAML loaders reject duplicate keys; permissive ones silently keep only the last value. Remove line 1525.
Suggested fix
- id: dns_flag_response_code
name: DNS Response Code
component: autocomplete
placeholder: 'E.g: NoError, NXDomain, NotAuth'
hint: Add DNS RCODE name filter.
- placeholder: 'E.g: NoError, NXDomain, NotAuth'
examples: |-🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 1525-1525: duplication of key "placeholder" in mapping
(key-duplicates)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/sample-config.yaml` around lines 1523 - 1526, The YAML fragment
contains a duplicated key "placeholder" (same value 'E.g: NoError, NXDomain,
NotAuth') which will break strict YAML parsers; remove the second occurrence
(the duplicate "placeholder" adjacent to the "hint" and "examples" keys) so only
one "placeholder" key remains in the block that includes the "hint" and
"examples" entries.
There was a problem hiding this comment.
@jotak seems like coderabbit said something useful here 👆
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
CI run failure was also useful :)
|
@jotak: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
5 participants
Description
Allow space in filtered values
Dependencies
Checklist
Summary by CodeRabbit
Bug Fixes
New Features
Improvements
Chores