feat: NetworkNeighbor wildcard runtime (v0.0.2)#41
Open
entlein wants to merge 10 commits into
Open
Conversation
added 5 commits
May 10, 2026 12:49
… surface Living documentation for the feat/network-wildcards work. Each fixture is a complete, kubectl-applicable NetworkNeighborhood document exercising ONE edge case in the v0.0.2 wildcard surface. Test_34 (forthcoming) consumes them directly; users learning the syntax can copy-paste them as authoritative examples. Coverage: 01 — IPv4 literal in ipAddresses[] 02 — IPv6 literal (canonicalisation) 03 — IPv4 CIDR 04 — IPv6 CIDR 05 — '*' sentinel for ANY IP (with discouragement annotation) 06 — 0.0.0.0/0 + ::/0 (RFC-aligned alternative to '*') 07 — mixed list (literal + CIDR + sentinel) 08 — backward-compat singular ipAddress 09 — DNS literal 10 — DNS leading '*' (RFC 4592) 11 — DNS mid '⋯' (DynamicIdentifier) 12 — DNS trailing '*' (one or more, never zero) 13 — trailing-dot normalisation 14 — '**' recursive — admission MUST reject 15 — egress + ingress on same container, direction isolation 16 — egress: [] NONE (declared zero-egress) 17 — realistic Stripe API + cluster DNS 18 — Kubernetes service-FQDN via mid '⋯' (the user's case) 19 — port + protocol + CIDR composed 20 — multi-container pod, different rules per container README.md indexes all fixtures and lists the wildcard token vocabulary. Each fixture's header comment lists the edge case, expected outcomes, match path, spec reference, and operational guidance. Ready to be consumed by node-agent's Test_34_NetworkWildcardSurface (forthcoming) and by storage's networkmatch unit tests via testdata-style references.
Replaces byte-equality with the v0.0.2 wildcard-aware matchers from
storage's pkg/registry/file/networkmatch — applied symmetrically to
all six nn.* CEL functions (egress + ingress mirror images):
nn.was_address_in_egress / _in_ingress
nn.is_domain_in_egress / _in_ingress
nn.was_address_port_protocol_in_egress / _in_ingress
Each function now walks BOTH the deprecated singular field
(IPAddress / DNS, byte-equality, back-compat) AND the new plural
field (IPAddresses / DNSNames, wildcard-aware) on each NetworkNeighbor
entry. A profile that uses only the deprecated form behaves exactly
as before; a profile that uses the new form gains CIDR + wildcard
matching with no rule-side changes required.
Two helpers (neighborMatchesIP / neighborMatchesDNS) factor the
two-list walk so the six call sites stay readable. Compiled-form
caching of the matcher across calls is deferred to a follow-up — the
existing cel functionCache still memoises (containerID, observed)
tuples, so the per-call MatchIP/MatchDNS overhead only fires on
cache misses.
Tests cover:
- CIDR membership across egress/ingress
- '*' sentinel for any IP
- leading-* DNS wildcard (RFC 4592, exactly one label)
- mid-⋯ DynamicLabel (the kubernetes service-FQDN case)
- trailing-dot resilience
- direction isolation (egress and ingress lists are walked
independently — same address allowed on one direction
must NOT match the other)
- back-compat: deprecated singular IPAddress/DNS still works
- mixed: profile with one entry using singular, another using plural
- composed match: CIDR + port + protocol on the granular variant
go.mod: temporary local-path replace for kubescape/storage so the
node-agent picks up the in-flight feat/network-wildcards work; user
flips back to fork ref before pushing.
TestFixturesParse: every YAML under tests/resources/network-wildcards/ parses against the v1beta1 NetworkNeighborhood schema. The fixtures double as authoritative user-facing syntax documentation, so a fixture that fails to parse is a documentation bug. TestFixturesMatchExpectedBehaviour: representative observed→match triples for each major edge case (literal IP, CIDR, '*' sentinel, deprecated singular IPAddress, leading-* DNS RFC 4592, mid-⋯ DynamicLabel, direction isolation between egress and ingress) are exercised through the actual nn.* CEL functions. If a fixture's header comment says '10.1.2.3 → match' and the matcher disagrees, ONE of them is wrong; this test pins both. True end-to-end Test_34_NetworkWildcardSurface (kubectl-applies the fixtures against a live cluster) belongs in the iximiuz lab; that job is left for the lab pass once the storage + node-agent images ship via the fork CI.
Local replace points at ../storage so the fork ref isn't fetched. User reverts both go.mod and go.sum before pushing the branch.
Updates the storage replace to a pseudo-version on the fork that includes the v0.0.2 wildcard surface (pkg/registry/file/networkmatch/, IPAddresses schema field, REST validation). Build and tests stay green against the pinned ref. The .claude/ entry on .gitignore prevents the agent state directory from being tracked accidentally.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request refactors CEL IP/DNS matching to shared helpers that support plural CIDR and DNS wildcard fields while preserving deprecated singular-field parity; adds explicit port-range validation; introduces comprehensive unit tests and helpers; and adds 20 NetworkNeighborhood fixture YAMLs with README plus minor .gitignore and go.mod updates. ChangesNetwork Wildcard Matching Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/rulemanager/cel/libraries/networkneighborhood/network.go (1)
171-191:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject out-of-range ports before narrowing to
int32.
int64→int32narrowing can wrap here. For example, an invalid CEL value like4294967739becomes443after conversion, so this matcher can incorrectly returntruefor a bogus port. GuardportIntto the valid TCP/UDP range before comparing.Proposed fix
portInt, ok := port.Value().(int64) if !ok { return types.MaybeNoSuchOverloadErr(port) } + if portInt < 0 || portInt > 65535 { + return types.Bool(false) + } protocolStr, ok := protocol.Value().(string) if !ok { return types.MaybeNoSuchOverloadErr(protocol) } @@ - for _, portInfo := range egress.Ports { - if portInfo.Protocol == v1beta1.Protocol(protocolStr) && portInfo.Port != nil && *portInfo.Port == int32(portInt) { + expectedPort := int32(portInt) + for _, portInfo := range egress.Ports { + if portInfo.Protocol == v1beta1.Protocol(protocolStr) && portInfo.Port != nil && *portInfo.Port == expectedPort { return types.Bool(true) } } @@ portInt, ok := port.Value().(int64) if !ok { return types.MaybeNoSuchOverloadErr(port) } + if portInt < 0 || portInt > 65535 { + return types.Bool(false) + } protocolStr, ok := protocol.Value().(string) if !ok { return types.MaybeNoSuchOverloadErr(protocol) } @@ - for _, portInfo := range ingress.Ports { - if portInfo.Protocol == v1beta1.Protocol(protocolStr) && portInfo.Port != nil && *portInfo.Port == int32(portInt) { + expectedPort := int32(portInt) + for _, portInfo := range ingress.Ports { + if portInfo.Protocol == v1beta1.Protocol(protocolStr) && portInfo.Port != nil && *portInfo.Port == expectedPort { return types.Bool(true) } }Also applies to: 213-233
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/rulemanager/cel/libraries/networkneighborhood/network.go` around lines 171 - 191, The code narrows port.Value() from int64 to int32 which can wrap; validate portInt is within the TCP/UDP range before comparing or converting: check portInt >= 0 && portInt <= 65535 and return types.MaybeNoSuchOverloadErr(port) (or similar error) if out of range, then safely cast to int32 for comparison with *portInfo.Port; apply the same guard for the second occurrence around lines 213-233 (the portInt handling used with cp.Spec.Egress and portInfo.Port).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/rulemanager/cel/libraries/networkneighborhood/fixtures_test.go`:
- Around line 52-53: The test uses yaml.Unmarshal which ignores unknown fields
and can let fixture typos slip; replace the call to yaml.Unmarshal(data, &nn)
with yaml.UnmarshalStrict(data, &nn) in the fixture parsing block (the one
assigning err and calling require.NoError(t, err, "fixture %s must parse against
v1beta1 schema", name)) so the test fails on unknown fields; ensure any
necessary yaml package provides UnmarshalStrict and adjust imports if needed.
- Around line 155-159: The test case that builds ipChecks currently only asserts
wasAddressInEgress; update the test in fixtures_test.go to also assert
wasAddressInIngress for each entry so both directions are validated: for the
"8.8.8.8" entry assert wasAddressInEgress == true and wasAddressInIngress ==
false, and for the "10.244.5.5" entry assert wasAddressInEgress == false and
wasAddressInIngress == true (or the expected inverse), ensuring the loop that
iterates ipChecks checks both wasAddressInEgress and wasAddressInIngress for
every ipCheck element.
In `@pkg/rulemanager/cel/libraries/networkneighborhood/network.go`:
- Around line 34-43: The neighborMatchesDNS function currently compares the
deprecated neighbor.DNS via simple string equality which diverges from DNSNames
behavior; update it so that when neighbor.DNS is non-empty you call
networkmatch.MatchDNS with a single-element slice (e.g.,
networkmatch.MatchDNS([]string{neighbor.DNS}, observed)) so the same
normalization/trailing-dot logic is applied as for DNSNames, and add a
regression test covering neighbor.DNS matching with and without trailing dots to
ensure parity with DNSNames.
In `@tests/resources/network-wildcards/README.md`:
- Around line 5-8: Update the README sentence that currently states every
`*.yaml` is "kubectl-applicable" to note that one fixture in the set is
intentionally rejected at admission and should not be applied; mention that the
test harness (Test_34_NetworkWildcardSurface) consumes all fixtures including an
admission-rejected example and point readers to the manifest indexed at line 48
as the intentionally-rejected case so users don’t attempt to kubectl-apply it.
---
Outside diff comments:
In `@pkg/rulemanager/cel/libraries/networkneighborhood/network.go`:
- Around line 171-191: The code narrows port.Value() from int64 to int32 which
can wrap; validate portInt is within the TCP/UDP range before comparing or
converting: check portInt >= 0 && portInt <= 65535 and return
types.MaybeNoSuchOverloadErr(port) (or similar error) if out of range, then
safely cast to int32 for comparison with *portInfo.Port; apply the same guard
for the second occurrence around lines 213-233 (the portInt handling used with
cp.Spec.Egress and portInfo.Port).
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 6afa11bf-3914-4be4-b861-58eff302d9bf
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (26)
.gitignorego.modpkg/rulemanager/cel/libraries/networkneighborhood/fixtures_test.gopkg/rulemanager/cel/libraries/networkneighborhood/network.gopkg/rulemanager/cel/libraries/networkneighborhood/wildcard_test.gotests/resources/network-wildcards/01-literal-ipv4.yamltests/resources/network-wildcards/02-literal-ipv6.yamltests/resources/network-wildcards/03-cidr-ipv4.yamltests/resources/network-wildcards/04-cidr-ipv6.yamltests/resources/network-wildcards/05-any-ip-sentinel.yamltests/resources/network-wildcards/06-any-as-cidr.yamltests/resources/network-wildcards/07-mixed-ip-list.yamltests/resources/network-wildcards/08-deprecated-ipaddress.yamltests/resources/network-wildcards/09-dns-literal.yamltests/resources/network-wildcards/10-dns-leading-wildcard.yamltests/resources/network-wildcards/11-dns-mid-ellipsis.yamltests/resources/network-wildcards/12-dns-trailing-star.yamltests/resources/network-wildcards/13-dns-trailing-dot-normalisation.yamltests/resources/network-wildcards/14-recursive-star-rejected.yamltests/resources/network-wildcards/15-egress-and-ingress.yamltests/resources/network-wildcards/16-egress-none.yamltests/resources/network-wildcards/17-realistic-stripe-api.yamltests/resources/network-wildcards/18-cluster-dns-via-mid-ellipsis.yamltests/resources/network-wildcards/19-port-protocol-with-cidr.yamltests/resources/network-wildcards/20-multi-container-mixed-wildcards.yamltests/resources/network-wildcards/README.md
Five findings, all legit, all fixed: - Port range guard (Major): wasAddressPortProtocolInEgress/Ingress now reject portInt outside [0, 65535] BEFORE narrowing to int32. Without this, a CEL value like 4294967739 wraps to 443 and would falsely match a port-443 entry. New TestWasAddressPortProtocolInEgress_ PortWrapRejected pins the contract. - neighborMatchesDNS now routes the deprecated singular DNS field through MatchDNS (single-element slice) instead of raw string equality, so back-compat behaviour gets the same trailing-dot stripping + lowercasing as the new DNSNames[]. New TestIsDomainInEgress_DeprecatedDNS_TrailingDotParity pins this. - Direction-isolation fixture test now exercises BOTH wasAddressInEgress and wasAddressInIngress for each observation, via a new ipBothCheck struct. The prior version only checked egress, so a regression that broke ingress matching would have slipped through. - TestFixturesParse uses yaml.UnmarshalStrict so a typo in any user- facing fixture (the YAML files double as documentation) fails the test instead of silently parsing. - README clarifies that fixture 14 is intentionally rejected at admission and shouldn't be kubectl-applied — points readers at the index entry so they don't try to use it as a template. Also bumps the storage replace to e1263bf6, which carries storage's CR fixes (deprecated IPAddress validation, ValidateUpdate now also runs network-profile validation, field-path assertions in admission tests).
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
added 2 commits
May 10, 2026 19:42
Pulls in storage's CR round-2 fixes: deterministic admission error ordering across container groups, and field-path assertions on the ValidateUpdate test.
Pulls in storage's deprecated-DNS validation parity fix.
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/rulemanager/cel/libraries/networkneighborhood/fixtures_test.go`:
- Around line 10-12: Remove the unused imports "github.com/goradd/maps",
"github.com/kubescape/node-agent/pkg/objectcache", and
"github.com/kubescape/node-agent/pkg/objectcache/v1" from fixtures_test.go and
delete the corresponding blank identifier workaround declarations (the _ = ...
lines that reference those packages around lines 244-246); tests already use
buildLibWithContainer from wildcard_test.go in the same package so those imports
and blank identifier lines are unnecessary—simply delete the three import
entries and the three blank identifier lines to clean up the file.
In `@pkg/rulemanager/cel/libraries/networkneighborhood/network.go`:
- Around line 19-29: The deprecated IPAddress field in neighborMatchesIP is
compared using byte-equality while DNS uses MatchDNS normalization; clarify this
intentional asymmetry by either (a) adding a brief comment inside
neighborMatchesIP explaining that IPAddress uses strict byte-equality by design
(so formats like 192.168.001.1 or alternate IPv6 representations will not
match), or (b) if you want consistent behavior, route the deprecated IPAddress
through networkmatch.MatchIP (and remove the direct equality check) so both
deprecated and list fields use the same normalization; reference
neighborMatchesIP and networkmatch.MatchIP when making the change.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 96818701-da40-4299-af40-7865277ce05b
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
go.modpkg/rulemanager/cel/libraries/networkneighborhood/fixtures_test.gopkg/rulemanager/cel/libraries/networkneighborhood/network.gopkg/rulemanager/cel/libraries/networkneighborhood/wildcard_test.gotests/resources/network-wildcards/README.md
Two findings, both nitpick-level, both applied: - Remove the unused 'maps', 'objectcache', 'objectcachev1' imports from fixtures_test.go along with the blank-identifier _ = ... lines at the bottom that existed only to silence the unused-import error. buildLibWithContainer is defined in wildcard_test.go (same package), so fixtures_test.go has no real need for those imports. - Route the deprecated singular IPAddress through networkmatch.MatchIP for symmetry with the deprecated singular DNS (which round 1 already routed through MatchDNS). Both deprecated fields now get the same canonicalisation (IPv6 expanded forms, IPv4-mapped IPv6) as the new list fields. New TestWasAddressInEgress_DeprecatedIPAddress_ IPv6Canonicalisation pins this.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/rulemanager/cel/libraries/networkneighborhood/network.go`:
- Around line 20-25: The deprecated neighbor.IPAddress is routed through
networkmatch.MatchIP which accepts wildcards/CIDRs, but tests only cover
literal/canonical IPs; add explicit coverage or documentation to define
backward-compat behavior. Either (A) add unit tests exercising
neighbor.IPAddress set to "*" and CIDR like "0.0.0.0/8" (mirror
TestWasAddressInEgress_AnyIPSentinel and TestWasAddressInEgress_* patterns) to
assert they match/ don’t match as intended, or (B) update the code comment near
neighbor.IPAddress and existing tests (e.g.,
TestWasAddressInEgress_LegacySingularStillWorks,
TestWasAddressInEgress_DeprecatedIPAddress_IPv6Canonicalisation) to state that
deprecated IPAddress only guarantees literal/canonical matching and should not
be relied on for wildcard/CIDR semantics; choose one approach and add the
corresponding test or comment to make the compatibility contract explicit.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: c593d69d-c819-4111-bbc5-4ffe92eb0ed5
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.modpkg/rulemanager/cel/libraries/networkneighborhood/fixtures_test.gopkg/rulemanager/cel/libraries/networkneighborhood/network.gopkg/rulemanager/cel/libraries/networkneighborhood/wildcard_test.go
…nd 3) CR caught that the round-2 routing of deprecated IPAddress through MatchIP had a documentation gap: existing tests only proved literal + canonical (IPv6) matching, never the wildcard/CIDR semantics that MatchIP now also enables on the deprecated field. Adds TestWasAddressInEgress_DeprecatedIPAddress_AcceptsWildcardAndCIDR which pins the contract: deprecated singular field accepts the SAME wildcard token vocabulary as the new list form — '*' sentinel, CIDRs, 0.0.0.0/0 and ::/0 alternatives. Comment on neighborMatchesIP documents this is intentional unification, not accidental.
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup Effectiveness (AFTER only)
Event Counters
|
This was referenced May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Companion PR to `storage#30`. Wires the v0.0.2 wildcard matchers into the runtime CEL functions and ships 20 user-facing YAML fixtures covering every edge case of the new surface.
What's new
Fixtures (`tests/resources/network-wildcards/`)
20 kubectl-applicable NetworkNeighborhood documents. Each fixture exercises ONE edge case and double as authoritative syntax documentation. Index in the directory's `README.md`. Coverage:
CEL function rewiring (`pkg/rulemanager/cel/libraries/networkneighborhood/network.go`)
All six `nn.*` functions now route IP comparisons through `storage/networkmatch.MatchIP` and DNS comparisons through `MatchDNS`. Mirror image applied symmetrically to egress + ingress:
Each function walks BOTH the deprecated singular field (back-compat) AND the new plural list (wildcard-aware). Two helpers — `neighborMatchesIP` and `neighborMatchesDNS` — factor the two-list walk so the six call sites stay readable. The existing CEL `functionCache` continues to memoise (containerID, observed) tuples, so the wildcard-match overhead only fires on cache misses.
Backward compatibility
Test plan
Storage dep
`go.mod` replace pinned to the storage fork ref carrying the networkmatch package. Once `storage#30` merges, this PR's replace will be updated to the post-merge SHA.