allowing CIDRs, wildcards and Plural in IP and DNS#806
Conversation
Signed-off-by: entlein <einentlein@gmail.com>
|
Warning Review limit reached
More reviews will be available in 59 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (28)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review Collective re-review request after applying your atomic-split advisory + rabbit-feedback fixes across the full SBOB upstream PR set. Each PR is now scoped to a single tier per your dependency map: Tier 0 (opens):
Tier 2 (execs):
Tier 3 (network):
Tier 4 (tamper/sign):
Umbrellas:
Please confirm atomicity per your earlier advisory and flag any remaining cross-PR coupling we missed. |
|
On your atomicity question for the full SBOB set: the tier structure you've described looks well-scoped. A few cross-PR coupling observations specific to this PR's position: Tier 3 coupling checks (PR
Tier 4 coupling risk
General atomicity verdict No cross-tier coupling jumps are apparent from the tier descriptions alone. The re-review will surface any remaining intra-PR issues in ✅ Actions performedReview triggered.
|
| "github.com/kubescape/node-agent/pkg/objectcache" | ||
| "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/cache" | ||
| "github.com/kubescape/node-agent/pkg/rulemanager/profilehelper" | ||
| "github.com/kubescape/storage/pkg/registry/file/networkmatch" |
There was a problem hiding this comment.
Blocking: this introduces github.com/kubescape/storage/pkg/registry/file/networkmatch, but this branch still pins github.com/kubescape/storage v0.0.258, and that version does not ship this package. go test ./pkg/rulemanager/cel/libraries/networkneighborhood/... currently fails immediately with no required module provides package github.com/kubescape/storage/pkg/registry/file/networkmatch.
Can we either bump github.com/kubescape/storage in this PR or add a temporary replace to the sister storage branch so the PR builds in this repo as-is?
There was a problem hiding this comment.
replace github.com/kubescape/storage => github.com/k8sstormcenter/storage v0.0.240-0.20260527173728-46f37d32b969 , please remove before merge :)
matthyx
left a comment
There was a problem hiding this comment.
Requesting changes for one blocking issue: this PR imports github.com/kubescape/storage/pkg/registry/file/networkmatch, but the repo still pins github.com/kubescape/storage v0.0.258, which does not contain that package. As a result, go test ./pkg/rulemanager/cel/libraries/networkneighborhood/... fails before the new tests can run.
I left the inline comment on the import; once the storage dependency is wired to a version/replace that includes networkmatch, I’m happy to re-review.
Resolves matthyx review on network.go:12 (2026-05-27).
…mcenter/node-agent into upstream-pr/sbob-network
matthyx
left a comment
There was a problem hiding this comment.
Rechecked the updated branch: the original missing-package blocker is fixed, but the new github.com/kubescape/storage replace still leaves this PR unbuildable.
On a fresh checkout, go test ./pkg/rulemanager/cel/libraries/networkneighborhood/... now stops with go: updates to go.mod needed; to update it: go mod tidy, so go.mod/go.sum are not in a buildable state as committed.
If I let Go resolve modules (GOFLAGS=-mod=mod go build ./pkg/rulemanager/cel/libraries/networkneighborhood/...), the dependency graph shifts and the build then fails in github.com/containerd/containerd/oci with cannot use limit (variable of type int64) as *int64 value in assignment.
So the branch still needs a storage version / dependency set that builds cleanly in node-agent as committed, without requiring local module rewrites.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Overview
network-wildcards: DNS leading/trailing/mid + CIDR + IPAddresses + "*" sentinel
Additional Information
CEL
networkneighborhoodhelpers now queryprojected fields with wildcard/CIDR/DNS-wildcard support and the v0.0.2
spec semantics (port/protocol checks degrade to address-only in v1
when only a CIDR is declared).
*(RFC 4592 single-label wildcard)*(subdomain-prefix wildcard)⋯(interior dynamic segment)*any-IP sentinelIPAddresseslist (deprecated single-IP field) preserved on thedecode path
How to Test
20 declarative fixture YAMLs under
tests/resources/network-wildcards/pin the wildcard contract end-to-end:
*any-IP and*as CIDRIPAddressesfield path⋯, trailing-**rejection⋯Has sister PR in storage