feat: NetworkNeighbor wildcard surface (v0.0.2)#30
Closed
entlein wants to merge 8 commits into
Closed
Conversation
added 4 commits
May 10, 2026 12:56
Runtime counterpart to spec §5.7 (v0.0.2). Profile entries are matched disjunctively — any single entry hit means match. Each entry MAY be: - a literal IP (canonicalised by net.ParseIP, IPv4 ↔ ::ffff:v4 unified) - a CIDR (a.b.c.d/n, validated by net.ParseCIDR) - the '*' sentinel for any IP (sugar for 0.0.0.0/0 ∪ ::/0) Public API: CompileIP([]string) *IPMatcher // compile-once for hot paths (*IPMatcher).Match(string) bool // amortised match MatchIP([]string, string) bool // cold-path convenience The compiled-matcher pattern eliminates per-call allocs from net.ParseCIDR on the hot path — node-agent's nn.* CEL functions call this on every network event captured by the eBPF tracer. Tests cover: - literal equality (incl. IPv4/IPv6 canonicalisation, ::ffff: mapping) - CIDR membership (incl. /32 == literal, edge addresses) - '*' sentinel (and rejection of empty/garbage observations) - 0.0.0.0/0 + ::/0 RFC-aligned alternatives (do NOT cross address families) - malformed entry skipping (admission validates, runtime defends) - mixed-list disjunction Benchmarks (arm64, hot-path compiled form): Literal : 12.73 ns/op 0 allocs CIDR : 18.48 ns/op 0 allocs 10-entry miss : 59.10 ns/op 0 allocs All well under the README target of <200 ns/op for IP literal/CIDR.
…psis
Runtime counterpart to spec §5.8 (v0.0.2). Profile entries are matched
disjunctively against an observed FQDN. Each entry's labels are walked
left-to-right with positional token semantics:
literal label : byte-equality (case-insensitive)
leading '*' : matches EXACTLY ONE label (RFC 4592)
trailing '*' : matches ONE OR MORE labels (project extension)
'⋯' anywhere : matches EXACTLY ONE label (DynamicLabel)
'**' : reserved/recursive — admission rejects, runtime drops
Trailing dot is normalised on both sides (entry MAY or MAY NOT have it).
Empty inner labels (e.g. 'foo..bar') are treated as malformed and skipped.
Public API:
CompileDNS([]string) *DNSMatcher
(*DNSMatcher).Match(string) bool
MatchDNS([]string, string) bool // cold-path convenience
Tests cover:
- literal equality (case-insensitive, exact-label-count)
- trailing-dot normalisation in all four combinations
- RFC 4592 leading '*' (exactly one label)
- mid '⋯' DynamicLabel (the kubernetes service-FQDN case)
- trailing '*' extension (one or more, never zero)
- disjunction across entry list
- malformed input rejection ('**', empty labels, lone '*')
- compiled-form reuse contract
Benchmarks (arm64, hot-path compiled form):
Literal : 47.68 ns/op 1 alloc (label-split)
Leading wildcard : 53.20 ns/op 1 alloc
Deep name (10 lbl): 144.9 ns/op 1 alloc
Long mixed list : 103.3 ns/op 1 alloc
All under the README target of <600 ns/op for DNS wildcards.
Adds IPAddresses []string to the NetworkNeighbor schema (internal +
v1beta1) as the v0.0.2 list-form replacement for the deprecated singular
IPAddress field. Each entry MAY be a literal IP, a CIDR, or '*'.
Hand-edited generated code (deepcopy, conversion, protobuf) because
the codegen pipeline isn't run as part of build; verified by a
new round-trip test that pins the protobuf wire contract for field
number 9.
Validation lives in two places:
pkg/registry/file/networkmatch/validate.go
- ValidateIPEntry : literal | CIDR | '*'
- ValidateDNSEntry : literal | leading-* | trailing-* | mid-⋯
rejects '**', empty inner labels, mid-* without ⋯
pkg/registry/softwarecomposition/networkneighborhood/strategy.go
REST Validate() walks every neighbor in containers/initContainers/
ephemeralContainers × egress/ingress and surfaces a field.Invalid
error per malformed entry. Apiserver translates this into a 400
so the user gets immediate feedback at write time.
Runtime matchers (MatchIP/MatchDNS) keep their tolerance of malformed
entries — a misconfigured profile that slips past admission won't kill
the detection path.
New tests:
- protobuf round-trip (field 9 survives Marshal → Unmarshal)
- ValidateIPEntry / ValidateDNSEntry edge cases
- REST strategy Validate rejects malformed entries with the right
field paths
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds networkmatch (IP/DNS matchers, validators, tests, benchmarks), extends NetworkNeighbor with list-form IPAddresses (keeps deprecated singular field), and plugs admission-time validation into NetworkNeighborhoodStrategy. Changesnetworkmatch Package: IP and DNS Matching Implementation
NetworkNeighbor API Extension
Admission Strategy Integration
Build Environment Configuration
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/registry/softwarecomposition/networkneighborhood/strategy.go (1)
174-187:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd network profile entry validation to ValidateUpdate method.
ValidateUpdate omits the
validateNetworkProfileEntriescall present in the Validate method, allowing malformedipAddressesordnsNamesto bypass admission checks during updates.Suggested fix
func (NetworkNeighborhoodStrategy) ValidateUpdate(_ context.Context, obj, _ runtime.Object) field.ErrorList { ap := obj.(*softwarecomposition.NetworkNeighborhood) allErrors := field.ErrorList{} if err := utils.ValidateCompletionAnnotation(ap.Annotations); err != nil { allErrors = append(allErrors, err) } if err := utils.ValidateStatusAnnotation(ap.Annotations); err != nil { allErrors = append(allErrors, err) } + + allErrors = append(allErrors, validateNetworkProfileEntries(&ap.Spec)...) return allErrors }🤖 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/registry/softwarecomposition/networkneighborhood/strategy.go` around lines 174 - 187, ValidateUpdate currently skips validating network profile entries, allowing bad ipAddresses/dnsNames on updates; update the NetworkNeighborhoodStrategy.ValidateUpdate function to cast obj to *softwarecomposition.NetworkNeighborhood, call validateNetworkProfileEntries(ap.Spec.NetworkProfileEntries) (same helper used by Validate), and append any returned field.Errors to allErrors (alongside the existing ValidateCompletionAnnotation and ValidateStatusAnnotation checks) before returning.
🤖 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/apis/softwarecomposition/v1beta1/network_types_protobuf_test.go`:
- Around line 62-63: The test currently ignores errors from withField.Marshal()
and withoutField.Marshal(), so replace the ignored returns with proper error
checks: capture the error when calling Marshal on withField and withoutField
(e.g., a, err := withField.Marshal() / b, err := withoutField.Marshal()), and
fail the test immediately (use t.Fatalf or t.Fatalf-like helper) if either
marshal returns a non-nil error, including the error text in the failure
message; ensure you reference the existing symbols Marshal, withField,
withoutField, and the test-local variables a and b when adding these checks.
In `@pkg/registry/file/networkmatch/README.md`:
- Around line 7-9: The README currently links to the spec URL ending in
"spec-v0.0.1" but the text describes v0.0.2 semantics; update the link string in
the README (the URL containing "spec-v0.0.1") to point to "spec-v0.0.2" so the
documented spec version matches the described feature contract.
- Around line 44-46: The README's documentation for MatchDNS is incorrect about
normalization; update the comment describing MatchDNS (the line above func
MatchDNS) to state that both profile entries and observedName are normalized by
stripping any trailing dot (not by appending one) before comparison so the
wording matches the runtime behavior of MatchDNS.
In `@pkg/registry/softwarecomposition/networkneighborhood/strategy_test.go`:
- Around line 411-414: The test currently only checks the number of validation
errors from s.Validate(context.TODO(), makeNN(tc.neighbor)) (errs) against
tc.wantErrors; update the test to also assert the expected admission field paths
for each error to ensure the field-path contract is preserved. Modify the
table-driven test cases to include an expected list of field paths (e.g.,
tc.wantFields or tc.wantErrorPaths) and after calling s.Validate, iterate errs
to extract each error's Field or Path value and compare the set/order against
tc.wantErrorPaths, failing the test with a descriptive message if they differ;
keep the existing len(errs) check but add this additional assertion so
s.Validate, makeNN, and err contents are verified.
In `@pkg/registry/softwarecomposition/networkneighborhood/strategy.go`:
- Around line 142-147: The admission currently only validates the slice
n.IPAddresses; also validate the deprecated singular field n.IPAddress using the
same validator to prevent malformed addresses from bypassing checks. After the
loop that uses networkmatch.ValidateIPEntry for n.IPAddresses (and the existing
ipsPath), call networkmatch.ValidateIPEntry on n.IPAddress when it is
set/non-empty, and if it returns an error append a field.Invalid for
nPath.Child("ipAddress") (mirroring the existing error construction) so both the
new and deprecated fields are covered.
---
Outside diff comments:
In `@pkg/registry/softwarecomposition/networkneighborhood/strategy.go`:
- Around line 174-187: ValidateUpdate currently skips validating network profile
entries, allowing bad ipAddresses/dnsNames on updates; update the
NetworkNeighborhoodStrategy.ValidateUpdate function to cast obj to
*softwarecomposition.NetworkNeighborhood, call
validateNetworkProfileEntries(ap.Spec.NetworkProfileEntries) (same helper used
by Validate), and append any returned field.Errors to allErrors (alongside the
existing ValidateCompletionAnnotation and ValidateStatusAnnotation checks)
before returning.
🪄 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: ea849c34-26ea-4391-8874-92bc385906e4
⛔ Files ignored due to path filters (1)
pkg/apis/softwarecomposition/v1beta1/generated.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (18)
.gitignorepkg/apis/softwarecomposition/network_types.gopkg/apis/softwarecomposition/v1beta1/network_types.gopkg/apis/softwarecomposition/v1beta1/network_types_protobuf_test.gopkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.gopkg/apis/softwarecomposition/v1beta1/zz_generated.deepcopy.gopkg/apis/softwarecomposition/zz_generated.deepcopy.gopkg/registry/file/networkmatch/README.mdpkg/registry/file/networkmatch/bench_test.gopkg/registry/file/networkmatch/doc.gopkg/registry/file/networkmatch/match_dns.gopkg/registry/file/networkmatch/match_dns_test.gopkg/registry/file/networkmatch/match_ip.gopkg/registry/file/networkmatch/match_ip_test.gopkg/registry/file/networkmatch/validate.gopkg/registry/file/networkmatch/validate_test.gopkg/registry/softwarecomposition/networkneighborhood/strategy.gopkg/registry/softwarecomposition/networkneighborhood/strategy_test.go
Five findings, all legit, all fixed: - ValidateUpdate now also runs validateNetworkProfileEntries, so malformed ipAddresses / dnsNames can't land via PUT after a clean POST. New TestValidateUpdate_NetworkProfileEntries pins this. - validateNeighborList now also validates the deprecated singular IPAddress field while it remains supported. Bypass via the old form is closed. - TestValidate_NetworkProfileEntries asserts each error's field path (multiset, order-insensitive) instead of just the count. Field-path contract is now pinned: if validation starts emitting errors on the wrong path, downstream tooling that surfaces these to users will catch it here first. New case for deprecated-singular path. - TestNetworkNeighbor_IPAddresses_EmptyOmitted now checks Marshal errors with t.Fatalf instead of swallowing them via _. - README links to spec-v0.0.2 (was v0.0.1) and DNS normalisation wording matches runtime: trailing dot stripped, labels lowercased (was incorrectly described as 'appended').
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/registry/file/networkmatch/README.md`:
- Line 17: Replace the British spelling "canonicalisation" with the American
spelling "canonicalization" in the README table cell containing the text
"Literal | byte-equality after canonicalisation (net.IP) | byte-equality after
trailing-dot normalisation" so the phrase becomes "byte-equality after
canonicalization (net.IP)"; update any other occurrences of "canonicalisation"
in the same README to maintain consistent American spelling across the document.
In `@pkg/registry/softwarecomposition/networkneighborhood/strategy_test.go`:
- Around line 475-478: The test currently only asserts len(errs) from
s.ValidateUpdate(context.TODO(), bad, bad); instead, assert that the set (or
list) of field paths reported by errs matches the expected field paths to
prevent regressions emitting errors on wrong fields; locate the call to
s.ValidateUpdate and the errs variable and replace the lone length check with a
check that extracts each error's field path (from the validation error type
returned), compare to an expected slice/set of paths (e.g., expectedPaths :=
{...}) and fail if they differ or if any expected path is missing.
In `@pkg/registry/softwarecomposition/networkneighborhood/strategy.go`:
- Around line 123-127: The current loop in NetworkNeighborhood's validation
iterates a map (using keys "containers","initContainers","ephemeralContainers")
causing nondeterministic ordering of the returned field.ErrorList; change the
map to an ordered slice of structs (e.g., []struct{name string, group
[]softwarecomposition.NetworkNeighborhoodContainer} or similar) and iterate that
slice in the same loop body so spec.Containers, spec.InitContainers and
spec.EphemeralContainers are validated in a stable, deterministic order; update
any references to groupName/group accordingly so the rest of the validation
logic remains unchanged.
🪄 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: 660b88ce-0af2-4e5a-b037-423ed963d18b
📒 Files selected for processing (4)
pkg/apis/softwarecomposition/v1beta1/network_types_protobuf_test.gopkg/registry/file/networkmatch/README.mdpkg/registry/softwarecomposition/networkneighborhood/strategy.gopkg/registry/softwarecomposition/networkneighborhood/strategy_test.go
Three findings, two fixed, one skipped: - validateNetworkProfileEntries now iterates an ordered []struct of (groupName, items) pairs rather than a map. Go map iteration is non-deterministic and admission errors flow to clients via the apiserver — stable ordering keeps error messages reproducible. - TestValidateUpdate_NetworkProfileEntries asserts each error's field path (multiset, order-insensitive). Same pattern already applied to TestValidate_NetworkProfileEntries; this closes the parallel gap. Skipped: - README.md "canonicalisation" → "canonicalization" (Trivial nit). Project already uses British spelling consistently across other docs; changing one occurrence creates inconsistency without fixing anything. Tests pass on the new ordering — note the test uses an order-insensitive multiset comparison, so the deterministic-order fix is a separate concern from the test (the test would pass with either ordering, but the runtime is now stable for downstream consumers).
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/registry/softwarecomposition/networkneighborhood/strategy.go`:
- Around line 163-168: The loop validates n.DNSNames but misses the deprecated
singular n.DNS; add a check mirroring the ipAddress pattern by validating n.DNS
when non-empty using networkmatch.ValidateDNSEntry and appending a field.Invalid
to errs with nPath.Child("dns") on error; place this check alongside the
existing dnsNames loop (use symbols n.DNS, n.DNSNames, dnsPath,
networkmatch.ValidateDNSEntry, field.Invalid, and nPath.Child) so the deprecated
field is validated consistently.
🪄 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: 731ab064-db9d-4da2-955f-97d594db47fb
📒 Files selected for processing (2)
pkg/registry/softwarecomposition/networkneighborhood/strategy.gopkg/registry/softwarecomposition/networkneighborhood/strategy_test.go
Closes a parity gap with IPAddress (which round-1 already validates). Now both deprecated singular fields receive the same admission grammar check while they remain supported. Test extended with a deprecated-DNS path case mirroring the existing IPAddress one.
|
Summary:
|
Reverses the round-2 skip on the CodeRabbit nitpick (PR #30, README line 17). Files added in this branch now use American spelling (canonicalize/normalize/behavior) matching the Go ecosystem convention that 'canonicalisation' originally diverged from. Files touched: - pkg/registry/file/networkmatch/README.md - pkg/registry/file/networkmatch/match_ip.go (comment) - pkg/registry/file/networkmatch/match_dns.go (comment) - pkg/registry/file/networkmatch/match_ip_test.go (comment) No behaviour change.
|
Summary:
|
5 tasks
Author
|
closing as it is superseded by PR 31 |
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
Adds wildcard support to NetworkNeighbor profile entries — CIDRs, RFC 4592 leading-
*, mid-⋯(DynamicLabel), trailing-*, and an*any-IP sentinel — so user-defined NetworkNeighborhoods can express realistic SaaS endpoints (*.stripe.com.), cluster service FQDNs (kubernetes.⋯.svc.cluster.local.), and pod-CIDR ranges (10.244.0.0/16) without enumerating every literal address.Implements spec sections §5.7 (IP) and §5.8 (DNS) of the v0.0.2 BoB spec.
What's new
`pkg/registry/file/networkmatch/` — new runtime matcher package
Schema — `IPAddresses []string` added to `NetworkNeighbor` (internal + v1beta1) as the v0.0.2 list-form replacement for the deprecated singular `IPAddress`. Each entry MAY be a literal IP, a CIDR, or `*`. Protobuf field number 9; round-trip test pins the wire contract.
REST strategy validation — admission rejects malformed `IPAddresses[]` and `DNSNames[]` entries with field-path errors (recursive `**`, malformed CIDRs, mid-position bare `*`, empty inner labels).
Why these specific tokens
Backward compatibility
Test plan
Companion PR
User-facing fixtures (20 YAML examples covering every edge case) and the runtime CEL rewiring live on the node-agent companion branch `feat/network-wildcards`.