From a21e2836a123b2ddf85f5f943f778fb3550c37af Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Wed, 22 Jul 2026 13:09:46 +0200 Subject: [PATCH 1/7] feat(storage): add CIDR-based collapsing for NetworkNeighbors Node-agent bakes the literal destination IP into the NetworkNeighbor identifier hash for external traffic, so every distinct observed IP produces a separate entry - a real profile hit 8,687 such entries and was flagged too-large. Extend the existing path-collapsing pattern (CollapseConfiguration CRD, TTL-cached provider, PreSave hook) to NetworkNeighbor: group entries by (Type, DNS, NamespaceSelector, PodSelector), aggregate IPv4 hosts into CIDR blocks once a configurable count threshold is exceeded, bounded by a configurable breadth floor. Held-stable CIDRs and a deterministic collapsed Identifier keep the pass idempotent across repeated saves. Policy generation now consumes the plural IPAddresses field so generated NetworkPolicies still reflect collapsed ranges, with known-server enrichment preserved for bare IPs. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Matthias Bertschy --- docs/features/networkneighbors-collapsing.md | 87 ++++++ .../softwarecomposition/collapse_types.go | 9 + .../networkpolicy/v2/networkpolicy.go | 110 +++++++- .../networkpolicy/v2/networkpolicy_test.go | 222 +++++++++++++++ .../v1beta1/collapse_types.go | 13 + .../v1beta1/generated.pb.go | 48 ++++ .../v1beta1/generated.proto | 15 + .../v1beta1/zz_generated.conversion.go | 4 + pkg/apiserver/apiserver.go | 4 +- .../v1beta1/collapseconfigurationspec.go | 27 ++ pkg/generated/openapi/zz_generated.openapi.go | 14 + .../file/containerprofile_processor.go | 4 +- .../file/containerprofile_processor_test.go | 39 +++ .../collapse_config_from_crd.go | 22 +- .../tests/collapse_config_crd_test.go | 2 + .../file/dynamicpathdetector/types.go | 6 + .../file/networkneighborhood_ipcollapse.go | 228 ++++++++++++++++ .../networkneighborhood_ipcollapse_test.go | 258 ++++++++++++++++++ .../file/networkneighborhood_processor.go | 49 +++- .../networkneighborhood_processor_test.go | 38 +++ .../collapseconfiguration/strategy.go | 13 +- .../collapseconfiguration/strategy_test.go | 58 ++++ 22 files changed, 1256 insertions(+), 14 deletions(-) create mode 100644 docs/features/networkneighbors-collapsing.md create mode 100644 pkg/registry/file/networkneighborhood_ipcollapse.go create mode 100644 pkg/registry/file/networkneighborhood_ipcollapse_test.go diff --git a/docs/features/networkneighbors-collapsing.md b/docs/features/networkneighbors-collapsing.md new file mode 100644 index 000000000..5b2cb2cd8 --- /dev/null +++ b/docs/features/networkneighbors-collapsing.md @@ -0,0 +1,87 @@ +# NetworkNeighbors CIDR-based collapsing + +## Summary + +When a node-agent observes external traffic from a workload, it records each distinct destination IP as a separate `NetworkNeighbor` entry. Since the `Identifier` includes the IP address, high-traffic profiles targeting a range of IPs (e.g. a cloud provider's IP block) can explode into thousands of entries — a real-world case hit **8,687 entries** for a single prefix. + +The collapsing pass groups `NetworkNeighbor` entries that differ only by IP (same `Type`, `DNS`, namespace selector, pod selector) and replaces them with a small number of **CIDR-bearing entries** when a group exceeds a configurable threshold. CIDR aggregation is bounded by a configurable floor (minimum prefix length / maximum breadth), so output size is predictable even for scattered IP distributions. + +Two new fields in the `CollapseConfiguration` CRD control this: +- `NetworkIPGroupThreshold` (default 50): group size threshold above which IP collapsing is triggered. +- `NetworkCIDRFloorBits` (default 16): minimum CIDR prefix length — no emitted block is ever broader than `/`. + +The pass is a **fixpoint**: running it twice on the same input is guaranteed to produce the same output, so collapsed entries remain stable across successive saves without oscillation or duplication. + +## Why it matters + +Before collapsing, external-traffic profiles with 8,687 individual IP entries consumed excessive storage, slowed queries, and produced NetworkPolicies with thousands of rules — most of which could be expressed as a handful of CIDR blocks. + +Collapsing reduces entry count by orders of magnitude for traffic aimed at cloud IP blocks (e.g. `100.68.24.0/22`, `16.15.183.0/24`) while preserving policy correctness: generated NetworkPolicies still include the same destination ranges, just expressed more efficiently as CIDR blocks instead of `/32` host routes. + +## How it works + +**Grouping**: Entries are grouped by a deterministic key over `(Type, DNS, NamespaceSelector, PodSelector)`. Entries in the same group differ only by IP address. + +**Classification**: Each IP value is classified as: +- **IPv4 host literal** (e.g., `192.168.1.5`): aggregatable, fed to the CIDR algorithm. +- **CIDR block** (e.g., `192.168.0.0/24`): treated as an already-covering range, held stable (not re-parsed or re-tightened) to preserve idempotency. +- **`"*"` sentinel or IPv6**: pass-through, never aggregated. + +**Aggregation**: If a group's count of aggregatable IPv4 host addresses exceeds `NetworkIPGroupThreshold`: +1. Compute the smallest covering prefix (common leading bits across all hosts). +2. If that prefix's length is at least `NetworkCIDRFloorBits` (e.g., `/16` or longer), emit it as-is. +3. If it would be broader than `NetworkCIDRFloorBits`, split the group into floor-length buckets (e.g., `/16` buckets) and emit one CIDR entry per non-empty bucket. + +**Output entries**: Each emitted CIDR entry carries: +- The computed CIDR block(s) in `IPAddresses`. +- The group's **full merged/deduped DNS names and ports** replicated onto every bucket (so each output entry is independently correlated with its ports and DNS for policy generation). +- The group's shared singular `DNS` value (constant across the group by construction, used for policy metadata). +- An `Identifier` derived from the group key plus sorted CIDR list — so the identifier-merge pass on a subsequent save recognizes and re-merges the same collapsed entry instead of duplicating it. +- Empty `IPAddress` (singular field), non-empty `IPAddresses` (plural field). + +**Policy generation**: `GenerateNetworkPolicy`'s rule generators (`generateEgressRule`, `generateIngressRule`) now consume the plural `IPAddresses` field: +- **CIDR block** (e.g., `192.168.0.0/16`): becomes an `IPBlock` peer with empty `OriginalIP` (no single original IP for a range). +- **Bare IPv4** (e.g., `192.168.1.5`): mirrored through the existing singular-path logic exactly, including known-server enrichment and `/32` formatting. +- **`"*"` sentinel**: becomes `0.0.0.0/0`. +- **IPv6**: skipped (out of scope for v1). + +## Scope / limitations + +**Held-stable CIDRs do not retroactively re-narrow when floor is tightened**: If an operator later changes `NetworkCIDRFloorBits` from 16 to 24 (smaller blocks, higher precision), already-emitted `/16` blocks are held stable for idempotency and won't be split retroactively. They persist until that group naturally re-collapses (e.g., new IPs arrive, triggering re-aggregation). This is an intentional trade-off: predictable idempotency wins over floor freshness for held entries. + +**New host IPs inside an already-held CIDR are not immediately absorbed**: If a `/16` block covers `192.168.0.0/16` and new traffic arrives to `192.168.5.100` (which falls inside that CIDR), the new IP persists as a separate entry until its own group independently exceeds the threshold. Entry-count creep is bounded by `NetworkIPGroupThreshold` and the existing merge logic, so this is not unbounded. + +**CIDR/`"*"` peers skip known-server enrichment**: A CIDR block is a range, not a single IP, so it cannot be looked up in the known-servers registry. CIDR and `"*"` entries produce bare `IPBlock` peers without the `PolicyRef` name/server enrichment that singular IPs enjoy. Bare-IP elements of the plural `IPAddresses` field retain full known-server matching identical to the singular-field path. + +**IPv4 only for v1**: IPv6 addresses pass through uncollapsed; future work may add IPv6 support. + +## Configuration + +Both fields are part of the existing `CollapseConfiguration` CRD singleton (`default`), using the same zero-means-default semantics as the existing path-collapsing thresholds: + +```yaml +apiVersion: spdx.softwarecomposition.kubescape.io/v1beta1 +kind: CollapseConfiguration +metadata: + name: default +spec: + # ... existing fields (OpenDynamicThreshold, EndpointDynamicThreshold, CollapseConfigs) ... + + # IP collapsing thresholds (optional; omit or set to 0 for defaults) + networkIPGroupThreshold: 50 # Collapse groups of 50+ hosts + networkCIDRFloorBits: 16 # No block narrower than /16 +``` + +Zero or omitted values use the compiled-in defaults (50 and 16 respectively). No operator restart is required; the provider reads the singleton at each request. + +## Verifying + +**Entry count**: Compare entry counts before and after enabling the feature on a real profile with external traffic. A profile with 8,687 external-traffic entries should drop to a small number (typically hundreds or fewer CIDR entries, depending on traffic distribution). + +**CIDR breadth**: Inspect emitted `NetworkNeighbor` entries; no single `IPAddresses` CIDR block should exceed the configured floor (default `/16`). + +**Idempotency**: Run the collapse pass twice in succession (e.g., two sequential saves) on the same profile and assert the second pass's output is byte-identical to the first (fixpoint). + +**Policy generation**: Feed a collapsed `NetworkNeighborhood` through `GenerateNetworkPolicy` and verify the generated NetworkPolicy includes `IPBlock` rules covering the collapsed CIDRs (test CIDR, bare-IP, and `"*"` cases). Confirm no rule has ports without at least one corresponding peer. + +**Known-server enrichment**: Verify that bare-IP entries of `IPAddresses` still receive known-server `PolicyRef` enrichment (identical to singular-field behavior), while CIDR and `"*"` entries produce bare `IPBlock` peers without enrichment. diff --git a/pkg/apis/softwarecomposition/collapse_types.go b/pkg/apis/softwarecomposition/collapse_types.go index 7c8360785..89dd44f8f 100644 --- a/pkg/apis/softwarecomposition/collapse_types.go +++ b/pkg/apis/softwarecomposition/collapse_types.go @@ -57,6 +57,15 @@ type CollapseConfigurationSpec struct { // longest-prefix-wins. It REPLACES the compiled-in defaults wholesale // (no merge); include any default prefix you want to keep. CollapseConfigs []CollapseConfigEntry + // NetworkIPGroupThreshold is the count threshold above which a group of + // NetworkNeighbor entries (sharing Type/DNS/selectors, differing only by + // IP) gets CIDR-collapsed. Omitted or 0 means "use the compiled-in + // default" (a literal 0 would collapse every group of size 1). + NetworkIPGroupThreshold int32 + // NetworkCIDRFloorBits is the minimum CIDR prefix length (maximum + // breadth) a single aggregated block may have. Omitted or 0 means "use + // the compiled-in default". + NetworkCIDRFloorBits int32 } // CollapseConfigEntry is one per-prefix threshold override. diff --git a/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go b/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go index 84f504cfc..59a6fe7b2 100644 --- a/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go +++ b/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go @@ -7,12 +7,14 @@ import ( "encoding/hex" "fmt" "net" + "net/netip" "sort" "strings" helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" "github.com/kubescape/storage/pkg/apis/softwarecomposition" "github.com/kubescape/storage/pkg/apis/softwarecomposition/networkpolicy" + "github.com/kubescape/storage/pkg/registry/file/networkmatch" "github.com/kubescape/go-logger" "github.com/kubescape/go-logger/helpers" @@ -337,7 +339,15 @@ func generateEgressRule(neighbor softwarecomposition.NetworkNeighbor, knownServe } } - if neighbor.IPAddress != "" { + skipPorts := false + if len(neighbor.IPAddresses) > 0 { + peers, refs := buildIPAddressesPeers(neighbor.IPAddresses, neighbor.DNS, knownServers) + egressRule.To = append(egressRule.To, peers...) + policyRefs = append(policyRefs, refs...) + if len(peers) == 0 && neighbor.PodSelector == nil && neighbor.NamespaceSelector == nil { + skipPorts = true + } + } else if neighbor.IPAddress != "" { // look if this IP is part of any known server if entries, contains := knownServers.Contains(net.ParseIP(neighbor.IPAddress)); contains { for _, entry := range entries { @@ -377,6 +387,10 @@ func generateEgressRule(neighbor softwarecomposition.NetworkNeighbor, knownServe } } + if skipPorts { + return egressRule, policyRefs + } + portMap := make(map[PortProtocolKey]bool) for _, networkPort := range neighbor.Ports { protocol := v1.Protocol(strings.ToUpper(string(networkPort.Protocol))) @@ -416,7 +430,15 @@ func generateIngressRule(neighbor softwarecomposition.NetworkNeighbor, knownServ } } - if neighbor.IPAddress != "" { + skipPorts := false + if len(neighbor.IPAddresses) > 0 { + peers, refs := buildIPAddressesPeers(neighbor.IPAddresses, neighbor.DNS, knownServers) + ingressRule.From = append(ingressRule.From, peers...) + policyRefs = append(policyRefs, refs...) + if len(peers) == 0 && neighbor.PodSelector == nil && neighbor.NamespaceSelector == nil { + skipPorts = true + } + } else if neighbor.IPAddress != "" { // look if this IP is part of any known server if entries, ok := knownServers.Contains(net.ParseIP(neighbor.IPAddress)); ok { for _, entry := range entries { @@ -455,6 +477,10 @@ func generateIngressRule(neighbor softwarecomposition.NetworkNeighbor, knownServ } } + if skipPorts { + return ingressRule, policyRefs + } + portMap := make(map[PortProtocolKey]bool) for _, networkPort := range neighbor.Ports { protocol := v1.Protocol(strings.ToUpper(string(networkPort.Protocol))) @@ -473,6 +499,86 @@ func generateIngressRule(neighbor softwarecomposition.NetworkNeighbor, knownServ return ingressRule, policyRefs } +// buildIPAddressesPeers builds NetworkPolicyPeer/PolicyRef pairs from the plural +// NetworkNeighbor.IPAddresses field, shared by generateEgressRule/generateIngressRule. +func buildIPAddressesPeers(ipAddresses []string, dns string, knownServers softwarecomposition.IKnownServersFinder) ([]softwarecomposition.NetworkPolicyPeer, []softwarecomposition.PolicyRef) { + var peers []softwarecomposition.NetworkPolicyPeer + var policyRefs []softwarecomposition.PolicyRef + + for _, entry := range ipAddresses { + if prefix, err := netip.ParsePrefix(entry); err == nil { + if !prefix.Addr().Is4() { + continue // IPv6 CIDR, out of scope (AC9) + } + peers = append(peers, softwarecomposition.NetworkPolicyPeer{ + IPBlock: &softwarecomposition.IPBlock{CIDR: entry}, + }) + if dns != "" { + // no single original IP for a CIDR range + policyRefs = append(policyRefs, softwarecomposition.PolicyRef{ + DNS: dns, + IPBlock: entry, + OriginalIP: "", + }) + } + continue + } + + if entry == networkmatch.AnyIPSentinel { + const anyCIDR = "0.0.0.0/0" + peers = append(peers, softwarecomposition.NetworkPolicyPeer{ + IPBlock: &softwarecomposition.IPBlock{CIDR: anyCIDR}, + }) + if dns != "" { + policyRefs = append(policyRefs, softwarecomposition.PolicyRef{ + DNS: dns, + IPBlock: anyCIDR, + OriginalIP: "", + }) + } + continue + } + + addr, err := netip.ParseAddr(entry) + if err != nil || !addr.Is4() { + continue // IPv6 or unparseable, out of scope (AC9) + } + + // bare IPv4: mirror the singular IPAddress path exactly, including known-server enrichment + if entries, contains := knownServers.Contains(net.ParseIP(entry)); contains { + for _, ks := range entries { + peers = append(peers, softwarecomposition.NetworkPolicyPeer{ + IPBlock: &softwarecomposition.IPBlock{CIDR: ks.GetIPBlock()}, + }) + + policyRef := softwarecomposition.PolicyRef{ + Name: ks.GetName(), + OriginalIP: entry, + IPBlock: ks.GetIPBlock(), + Server: ks.GetServer(), + } + if dns != "" { + policyRef.DNS = dns + } + policyRefs = append(policyRefs, policyRef) + } + } else { + ipBlock := getSingleIP(entry) + peers = append(peers, softwarecomposition.NetworkPolicyPeer{IPBlock: ipBlock}) + + if dns != "" { + policyRefs = append(policyRefs, softwarecomposition.PolicyRef{ + DNS: dns, + IPBlock: ipBlock.CIDR, + OriginalIP: entry, + }) + } + } + } + + return peers, policyRefs +} + func getSingleIP(ipAddress string) *softwarecomposition.IPBlock { ipBlock := &softwarecomposition.IPBlock{CIDR: ipAddress + "/32"} return ipBlock diff --git a/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy_test.go b/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy_test.go index 3cab9d181..490f2a075 100644 --- a/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy_test.go +++ b/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy_test.go @@ -2026,6 +2026,228 @@ func TestGenerateNetworkPolicy(t *testing.T) { } } +func TestGenerateEgressRule_IPAddresses(t *testing.T) { + tcpPort80 := []softwarecomposition.NetworkPort{ + {Port: ptrToInt32(80), Protocol: softwarecomposition.ProtocolTCP, Name: "TCP-80"}, + } + + tests := []struct { + name string + neighbor softwarecomposition.NetworkNeighbor + knownServers []softwarecomposition.KnownServer + expectedTo []softwarecomposition.NetworkPolicyPeer + expectedRefs []softwarecomposition.PolicyRef + expectNoPorts bool + }{ + { + name: "CIDR only produces IPBlock peer", + neighbor: softwarecomposition.NetworkNeighbor{ + IPAddresses: []string{"10.0.0.0/16"}, + Ports: tcpPort80, + }, + expectedTo: []softwarecomposition.NetworkPolicyPeer{ + {IPBlock: &softwarecomposition.IPBlock{CIDR: "10.0.0.0/16"}}, + }, + expectedRefs: []softwarecomposition.PolicyRef{}, + }, + { + name: "any sentinel produces 0.0.0.0/0", + neighbor: softwarecomposition.NetworkNeighbor{ + IPAddresses: []string{"*"}, + Ports: tcpPort80, + }, + expectedTo: []softwarecomposition.NetworkPolicyPeer{ + {IPBlock: &softwarecomposition.IPBlock{CIDR: "0.0.0.0/0"}}, + }, + expectedRefs: []softwarecomposition.PolicyRef{}, + }, + { + name: "bare IPv4 with known server match gets enrichment", + neighbor: softwarecomposition.NetworkNeighbor{ + IPAddresses: []string{"1.2.3.4"}, + Ports: tcpPort80, + }, + knownServers: []softwarecomposition.KnownServer{ + {Spec: softwarecomposition.KnownServerSpec{ + {IPBlock: "1.2.3.0/24", Name: "known-server", Server: "server-1"}, + }}, + }, + expectedTo: []softwarecomposition.NetworkPolicyPeer{ + {IPBlock: &softwarecomposition.IPBlock{CIDR: "1.2.3.0/24"}}, + }, + expectedRefs: []softwarecomposition.PolicyRef{ + {Name: "known-server", OriginalIP: "1.2.3.4", IPBlock: "1.2.3.0/24", Server: "server-1"}, + }, + }, + { + name: "bare IPv4 without known server match behaves like singular path", + neighbor: softwarecomposition.NetworkNeighbor{ + IPAddresses: []string{"5.6.7.8"}, + DNS: "example.com", + Ports: tcpPort80, + }, + expectedTo: []softwarecomposition.NetworkPolicyPeer{ + {IPBlock: &softwarecomposition.IPBlock{CIDR: "5.6.7.8/32"}}, + }, + expectedRefs: []softwarecomposition.PolicyRef{ + {DNS: "example.com", IPBlock: "5.6.7.8/32", OriginalIP: "5.6.7.8"}, + }, + }, + { + name: "all-IPv6 with no selector yields zero peers and no ports", + neighbor: softwarecomposition.NetworkNeighbor{ + IPAddresses: []string{"2001:db8::1"}, + Ports: tcpPort80, + }, + expectedTo: nil, + expectedRefs: []softwarecomposition.PolicyRef{}, + expectNoPorts: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + knownServers := softwarecomposition.NewKnownServersFinderImpl(tt.knownServers) + rule, refs := generateEgressRule(tt.neighbor, knownServers) + + assert.Equal(t, tt.expectedTo, rule.To) + assert.Equal(t, tt.expectedRefs, refs) + if tt.expectNoPorts { + assert.Empty(t, rule.Ports) + } else { + assert.NotEmpty(t, rule.Ports) + } + }) + } +} + +func TestGenerateIngressRule_IPAddresses(t *testing.T) { + tcpPort80 := []softwarecomposition.NetworkPort{ + {Port: ptrToInt32(80), Protocol: softwarecomposition.ProtocolTCP, Name: "TCP-80"}, + } + + tests := []struct { + name string + neighbor softwarecomposition.NetworkNeighbor + knownServers []softwarecomposition.KnownServer + expectedFrom []softwarecomposition.NetworkPolicyPeer + expectedRefs []softwarecomposition.PolicyRef + expectNoPorts bool + }{ + { + name: "CIDR only produces IPBlock peer", + neighbor: softwarecomposition.NetworkNeighbor{ + IPAddresses: []string{"10.0.0.0/16"}, + Ports: tcpPort80, + }, + expectedFrom: []softwarecomposition.NetworkPolicyPeer{ + {IPBlock: &softwarecomposition.IPBlock{CIDR: "10.0.0.0/16"}}, + }, + expectedRefs: []softwarecomposition.PolicyRef{}, + }, + { + name: "any sentinel produces 0.0.0.0/0", + neighbor: softwarecomposition.NetworkNeighbor{ + IPAddresses: []string{"*"}, + Ports: tcpPort80, + }, + expectedFrom: []softwarecomposition.NetworkPolicyPeer{ + {IPBlock: &softwarecomposition.IPBlock{CIDR: "0.0.0.0/0"}}, + }, + expectedRefs: []softwarecomposition.PolicyRef{}, + }, + { + name: "bare IPv4 with known server match gets enrichment", + neighbor: softwarecomposition.NetworkNeighbor{ + IPAddresses: []string{"1.2.3.4"}, + Ports: tcpPort80, + }, + knownServers: []softwarecomposition.KnownServer{ + {Spec: softwarecomposition.KnownServerSpec{ + {IPBlock: "1.2.3.0/24", Name: "known-server", Server: "server-1"}, + }}, + }, + expectedFrom: []softwarecomposition.NetworkPolicyPeer{ + {IPBlock: &softwarecomposition.IPBlock{CIDR: "1.2.3.0/24"}}, + }, + expectedRefs: []softwarecomposition.PolicyRef{ + {Name: "known-server", OriginalIP: "1.2.3.4", IPBlock: "1.2.3.0/24", Server: "server-1"}, + }, + }, + { + name: "bare IPv4 without known server match behaves like singular path", + neighbor: softwarecomposition.NetworkNeighbor{ + IPAddresses: []string{"5.6.7.8"}, + DNS: "example.com", + Ports: tcpPort80, + }, + expectedFrom: []softwarecomposition.NetworkPolicyPeer{ + {IPBlock: &softwarecomposition.IPBlock{CIDR: "5.6.7.8/32"}}, + }, + expectedRefs: []softwarecomposition.PolicyRef{ + {DNS: "example.com", IPBlock: "5.6.7.8/32", OriginalIP: "5.6.7.8"}, + }, + }, + { + name: "all-IPv6 with no selector yields zero peers and no ports", + neighbor: softwarecomposition.NetworkNeighbor{ + IPAddresses: []string{"2001:db8::1"}, + Ports: tcpPort80, + }, + expectedFrom: nil, + expectedRefs: []softwarecomposition.PolicyRef{}, + expectNoPorts: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + knownServers := softwarecomposition.NewKnownServersFinderImpl(tt.knownServers) + rule, refs := generateIngressRule(tt.neighbor, knownServers) + + assert.Equal(t, tt.expectedFrom, rule.From) + assert.Equal(t, tt.expectedRefs, refs) + if tt.expectNoPorts { + assert.Empty(t, rule.Ports) + } else { + assert.NotEmpty(t, rule.Ports) + } + }) + } +} + +// TestGenerateEgressRule_IPAddressVsIPAddresses confirms a bare-IP element of the +// plural IPAddresses field produces the same peer/PolicyRef shape as an equivalent +// singular IPAddress entry, including known-server enrichment (AC12). +func TestGenerateEgressRule_IPAddressVsIPAddresses(t *testing.T) { + knownServers := softwarecomposition.NewKnownServersFinderImpl([]softwarecomposition.KnownServer{ + {Spec: softwarecomposition.KnownServerSpec{ + {IPBlock: "1.2.3.0/24", Name: "known-server", Server: "server-1"}, + }}, + }) + + singular := softwarecomposition.NetworkNeighbor{ + IPAddress: "1.2.3.4", + DNS: "example.com", + Ports: []softwarecomposition.NetworkPort{ + {Port: ptrToInt32(80), Protocol: softwarecomposition.ProtocolTCP, Name: "TCP-80"}, + }, + } + plural := softwarecomposition.NetworkNeighbor{ + IPAddresses: []string{"1.2.3.4"}, + DNS: "example.com", + Ports: []softwarecomposition.NetworkPort{ + {Port: ptrToInt32(80), Protocol: softwarecomposition.ProtocolTCP, Name: "TCP-80"}, + }, + } + + singularRule, singularRefs := generateEgressRule(singular, knownServers) + pluralRule, pluralRefs := generateEgressRule(plural, knownServers) + + assert.Equal(t, singularRule.To, pluralRule.To) + assert.Equal(t, singularRefs, pluralRefs) +} + func TestGetSingleIP(t *testing.T) { ipAddress := "192.168.1.1" expected := &softwarecomposition.IPBlock{CIDR: "192.168.1.1/32"} diff --git a/pkg/apis/softwarecomposition/v1beta1/collapse_types.go b/pkg/apis/softwarecomposition/v1beta1/collapse_types.go index 9462810e6..8ae04d828 100644 --- a/pkg/apis/softwarecomposition/v1beta1/collapse_types.go +++ b/pkg/apis/softwarecomposition/v1beta1/collapse_types.go @@ -65,6 +65,19 @@ type CollapseConfigurationSpec struct { // +listType=map // +listMapKey=prefix CollapseConfigs []CollapseConfigEntry `json:"collapseConfigs,omitempty" protobuf:"bytes,3,rep,name=collapseConfigs"` + // NetworkIPGroupThreshold is the count threshold above which a group of + // NetworkNeighbor entries (sharing Type/DNS/selectors, differing only by + // IP) gets CIDR-collapsed. Optional: when omitted (decodes to 0) or + // explicitly set to 0, the deflate path uses the compiled-in default + // rather than a literal 0 — a 0 threshold would collapse every group of + // size 1. See CollapseSettingsFromCRD. + // +optional + NetworkIPGroupThreshold int32 `json:"networkIPGroupThreshold,omitempty" protobuf:"varint,4,opt,name=networkIPGroupThreshold"` + // NetworkCIDRFloorBits is the minimum CIDR prefix length (maximum + // breadth) a single aggregated block may have. Optional with the same + // omitted/0-means-compiled-default semantics as NetworkIPGroupThreshold. + // +optional + NetworkCIDRFloorBits int32 `json:"networkCIDRFloorBits,omitempty" protobuf:"varint,5,opt,name=networkCIDRFloorBits"` } // CollapseConfigEntry is one per-prefix threshold override. diff --git a/pkg/apis/softwarecomposition/v1beta1/generated.pb.go b/pkg/apis/softwarecomposition/v1beta1/generated.pb.go index ead6f7c08..d52aafa2a 100644 --- a/pkg/apis/softwarecomposition/v1beta1/generated.pb.go +++ b/pkg/apis/softwarecomposition/v1beta1/generated.pb.go @@ -1058,6 +1058,12 @@ func (m *CollapseConfigurationSpec) MarshalToSizedBuffer(dAtA []byte) (int, erro _ = i var l int _ = l + i = encodeVarintGenerated(dAtA, i, uint64(m.NetworkCIDRFloorBits)) + i-- + dAtA[i] = 0x28 + i = encodeVarintGenerated(dAtA, i, uint64(m.NetworkIPGroupThreshold)) + i-- + dAtA[i] = 0x20 if len(m.CollapseConfigs) > 0 { for iNdEx := len(m.CollapseConfigs) - 1; iNdEx >= 0; iNdEx-- { { @@ -9260,6 +9266,8 @@ func (m *CollapseConfigurationSpec) Size() (n int) { n += 1 + l + sovGenerated(uint64(l)) } } + n += 1 + sovGenerated(uint64(m.NetworkIPGroupThreshold)) + n += 1 + sovGenerated(uint64(m.NetworkCIDRFloorBits)) return n } @@ -12447,6 +12455,8 @@ func (this *CollapseConfigurationSpec) String() string { `OpenDynamicThreshold:` + fmt.Sprintf("%v", this.OpenDynamicThreshold) + `,`, `EndpointDynamicThreshold:` + fmt.Sprintf("%v", this.EndpointDynamicThreshold) + `,`, `CollapseConfigs:` + repeatedStringForCollapseConfigs + `,`, + `NetworkIPGroupThreshold:` + fmt.Sprintf("%v", this.NetworkIPGroupThreshold) + `,`, + `NetworkCIDRFloorBits:` + fmt.Sprintf("%v", this.NetworkCIDRFloorBits) + `,`, `}`, }, "") return s @@ -16848,6 +16858,44 @@ func (m *CollapseConfigurationSpec) Unmarshal(dAtA []byte) error { return err } iNdEx = postIndex + case 4: + if wireType != 0 { + return fmt.Errorf("proto: wrong wireType = %d for field NetworkIPGroupThreshold", wireType) + } + m.NetworkIPGroupThreshold = 0 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowGenerated + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + m.NetworkIPGroupThreshold |= int32(b&0x7F) << shift + if b < 0x80 { + break + } + } + case 5: + if wireType != 0 { + return fmt.Errorf("proto: wrong wireType = %d for field NetworkCIDRFloorBits", wireType) + } + m.NetworkCIDRFloorBits = 0 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowGenerated + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + m.NetworkCIDRFloorBits |= int32(b&0x7F) << shift + if b < 0x80 { + break + } + } default: iNdEx = preIndex skippy, err := skipGenerated(dAtA[iNdEx:]) diff --git a/pkg/apis/softwarecomposition/v1beta1/generated.proto b/pkg/apis/softwarecomposition/v1beta1/generated.proto index 739ab9039..8aa1beecb 100644 --- a/pkg/apis/softwarecomposition/v1beta1/generated.proto +++ b/pkg/apis/softwarecomposition/v1beta1/generated.proto @@ -193,6 +193,21 @@ message CollapseConfigurationSpec { // +listType=map // +listMapKey=prefix repeated CollapseConfigEntry collapseConfigs = 3; + + // NetworkIPGroupThreshold is the count threshold above which a group of + // NetworkNeighbor entries (sharing Type/DNS/selectors, differing only by + // IP) gets CIDR-collapsed. Optional: when omitted (decodes to 0) or + // explicitly set to 0, the deflate path uses the compiled-in default + // rather than a literal 0 — a 0 threshold would collapse every group of + // size 1. See CollapseSettingsFromCRD. + // +optional + optional int32 networkIPGroupThreshold = 4; + + // NetworkCIDRFloorBits is the minimum CIDR prefix length (maximum + // breadth) a single aggregated block may have. Optional with the same + // omitted/0-means-compiled-default semantics as NetworkIPGroupThreshold. + // +optional + optional int32 networkCIDRFloorBits = 5; } message Component { diff --git a/pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go b/pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go index 353a757af..bbf89dfe5 100644 --- a/pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go +++ b/pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go @@ -2102,6 +2102,8 @@ func autoConvert_v1beta1_CollapseConfigurationSpec_To_softwarecomposition_Collap out.OpenDynamicThreshold = in.OpenDynamicThreshold out.EndpointDynamicThreshold = in.EndpointDynamicThreshold out.CollapseConfigs = *(*[]softwarecomposition.CollapseConfigEntry)(unsafe.Pointer(&in.CollapseConfigs)) + out.NetworkIPGroupThreshold = in.NetworkIPGroupThreshold + out.NetworkCIDRFloorBits = in.NetworkCIDRFloorBits return nil } @@ -2114,6 +2116,8 @@ func autoConvert_softwarecomposition_CollapseConfigurationSpec_To_v1beta1_Collap out.OpenDynamicThreshold = in.OpenDynamicThreshold out.EndpointDynamicThreshold = in.EndpointDynamicThreshold out.CollapseConfigs = *(*[]CollapseConfigEntry)(unsafe.Pointer(&in.CollapseConfigs)) + out.NetworkIPGroupThreshold = in.NetworkIPGroupThreshold + out.NetworkCIDRFloorBits = in.NetworkCIDRFloorBits return nil } diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index e57af1df2..506dd78d2 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -146,6 +146,7 @@ func (c completedConfig) New() (*WardleServer, error) { // read the CR, processors are baked into the storage backend. applicationProfileProcessor := file.NewApplicationProfileProcessor(c.ExtraConfig.StorageConfig) containerProfileProcessor := file.NewContainerProfileProcessor(c.ExtraConfig.StorageConfig, c.ExtraConfig.CleanupHandler) + networkNeighborhoodProcessor := file.NewNetworkNeighborhoodProcessor(c.ExtraConfig.StorageConfig) var ( storageImpl = file.NewStorageImpl(c.ExtraConfig.OsFs, file.DefaultStorageRoot, c.ExtraConfig.Pool, c.ExtraConfig.WatchDispatcher, Scheme) @@ -153,7 +154,7 @@ func (c completedConfig) New() (*WardleServer, error) { applicationProfileStorageBackend = file.NewStorageImplWithCollector(c.ExtraConfig.OsFs, file.DefaultStorageRoot, c.ExtraConfig.Pool, c.ExtraConfig.WatchDispatcher, Scheme, applicationProfileProcessor) applicationProfileStorageImpl = file.NewApplicationProfileStorage(applicationProfileStorageBackend) containerProfileStorageImpl = file.NewContainerProfileRESTStorage(file.NewStorageImplWithCollector(c.ExtraConfig.OsFs, file.DefaultStorageRoot, c.ExtraConfig.Pool, c.ExtraConfig.WatchDispatcher, Scheme, containerProfileProcessor)) - networkNeighborhoodStorageImpl = file.NewNetworkNeighborhoodStorage(file.NewStorageImplWithCollector(c.ExtraConfig.OsFs, file.DefaultStorageRoot, c.ExtraConfig.Pool, c.ExtraConfig.WatchDispatcher, Scheme, file.NewNetworkNeighborhoodProcessor(c.ExtraConfig.StorageConfig))) + networkNeighborhoodStorageImpl = file.NewNetworkNeighborhoodStorage(file.NewStorageImplWithCollector(c.ExtraConfig.OsFs, file.DefaultStorageRoot, c.ExtraConfig.Pool, c.ExtraConfig.WatchDispatcher, Scheme, networkNeighborhoodProcessor)) configScanStorageImpl = file.NewConfigurationScanSummaryStorage(storageImpl) vulnerabilitySummaryStorage = file.NewVulnerabilitySummaryStorage(storageImpl) generatedNetworkPolicyStorage = file.NewGeneratedNetworkPolicyStorage(storageImpl, networkNeighborhoodStorageImpl) @@ -181,6 +182,7 @@ func (c completedConfig) New() (*WardleServer, error) { collapseSettingsFromCRD := file.NewCRDCollapseSettingsProvider(applicationProfileStorageBackend) applicationProfileProcessor.SetCollapseSettings(collapseSettingsFromCRD) containerProfileProcessor.CollapseSettings = collapseSettingsFromCRD + networkNeighborhoodProcessor.SetCollapseSettings(collapseSettingsFromCRD) apiGroupInfo.VersionedResourcesStorageMap["v1beta1"] = map[string]rest.Storage{ "applicationprofiles": ep(applicationprofile.NewREST, applicationProfileStorageImpl), "collapseconfigurations": ep(collapseconfiguration.NewREST), diff --git a/pkg/generated/applyconfiguration/softwarecomposition/v1beta1/collapseconfigurationspec.go b/pkg/generated/applyconfiguration/softwarecomposition/v1beta1/collapseconfigurationspec.go index 2ad0dabad..989201f36 100644 --- a/pkg/generated/applyconfiguration/softwarecomposition/v1beta1/collapseconfigurationspec.go +++ b/pkg/generated/applyconfiguration/softwarecomposition/v1beta1/collapseconfigurationspec.go @@ -43,6 +43,17 @@ type CollapseConfigurationSpecApplyConfiguration struct { // built-in /etc, /opt, /var/run (etc.) overrides — include them // explicitly if you want them to remain in effect. CollapseConfigs []CollapseConfigEntryApplyConfiguration `json:"collapseConfigs,omitempty"` + // NetworkIPGroupThreshold is the count threshold above which a group of + // NetworkNeighbor entries (sharing Type/DNS/selectors, differing only by + // IP) gets CIDR-collapsed. Optional: when omitted (decodes to 0) or + // explicitly set to 0, the deflate path uses the compiled-in default + // rather than a literal 0 — a 0 threshold would collapse every group of + // size 1. See CollapseSettingsFromCRD. + NetworkIPGroupThreshold *int32 `json:"networkIPGroupThreshold,omitempty"` + // NetworkCIDRFloorBits is the minimum CIDR prefix length (maximum + // breadth) a single aggregated block may have. Optional with the same + // omitted/0-means-compiled-default semantics as NetworkIPGroupThreshold. + NetworkCIDRFloorBits *int32 `json:"networkCIDRFloorBits,omitempty"` } // CollapseConfigurationSpecApplyConfiguration constructs a declarative configuration of the CollapseConfigurationSpec type for use with @@ -79,3 +90,19 @@ func (b *CollapseConfigurationSpecApplyConfiguration) WithCollapseConfigs(values } return b } + +// WithNetworkIPGroupThreshold sets the NetworkIPGroupThreshold field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the NetworkIPGroupThreshold field is set to the value of the last call. +func (b *CollapseConfigurationSpecApplyConfiguration) WithNetworkIPGroupThreshold(value int32) *CollapseConfigurationSpecApplyConfiguration { + b.NetworkIPGroupThreshold = &value + return b +} + +// WithNetworkCIDRFloorBits sets the NetworkCIDRFloorBits field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the NetworkCIDRFloorBits field is set to the value of the last call. +func (b *CollapseConfigurationSpecApplyConfiguration) WithNetworkCIDRFloorBits(value int32) *CollapseConfigurationSpecApplyConfiguration { + b.NetworkCIDRFloorBits = &value + return b +} diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index 2b458e1ca..7c9f2d3a4 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -925,6 +925,20 @@ func schema_pkg_apis_softwarecomposition_v1beta1_CollapseConfigurationSpec(ref c }, }, }, + "networkIPGroupThreshold": { + SchemaProps: spec.SchemaProps{ + Description: "NetworkIPGroupThreshold is the count threshold above which a group of NetworkNeighbor entries (sharing Type/DNS/selectors, differing only by IP) gets CIDR-collapsed. Optional: when omitted (decodes to 0) or explicitly set to 0, the deflate path uses the compiled-in default rather than a literal 0 — a 0 threshold would collapse every group of size 1. See CollapseSettingsFromCRD.", + Type: []string{"integer"}, + Format: "int32", + }, + }, + "networkCIDRFloorBits": { + SchemaProps: spec.SchemaProps{ + Description: "NetworkCIDRFloorBits is the minimum CIDR prefix length (maximum breadth) a single aggregated block may have. Optional with the same omitted/0-means-compiled-default semantics as NetworkIPGroupThreshold.", + Type: []string{"integer"}, + Format: "int32", + }, + }, }, }, }, diff --git a/pkg/registry/file/containerprofile_processor.go b/pkg/registry/file/containerprofile_processor.go index db4a8ee10..0fd79279e 100644 --- a/pkg/registry/file/containerprofile_processor.go +++ b/pkg/registry/file/containerprofile_processor.go @@ -911,8 +911,8 @@ func DeflateContainerProfileSpec(container softwarecomposition.ContainerProfileS MatchLabels: container.MatchLabels, MatchExpressions: DeflateLabelSelectorRequirement(container.MatchExpressions), }, - Ingress: deflateNetworkNeighbors(container.Ingress), - Egress: deflateNetworkNeighbors(container.Egress), + Ingress: deflateNetworkNeighbors(container.Ingress, settings), + Egress: deflateNetworkNeighbors(container.Egress, settings), } } diff --git a/pkg/registry/file/containerprofile_processor_test.go b/pkg/registry/file/containerprofile_processor_test.go index 384d059e9..bacfb1629 100644 --- a/pkg/registry/file/containerprofile_processor_test.go +++ b/pkg/registry/file/containerprofile_processor_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "os" "sync" "testing" @@ -13,6 +14,7 @@ import ( helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" "github.com/kubescape/storage/pkg/apis/softwarecomposition" "github.com/kubescape/storage/pkg/generated/clientset/versioned/scheme" + "github.com/kubescape/storage/pkg/registry/file/dynamicpathdetector" "github.com/kubescape/storage/pkg/utils" "github.com/spf13/afero" "github.com/stretchr/testify/assert" @@ -23,6 +25,43 @@ import ( "zombiezen.com/go/sqlite/sqlitemigration" ) +func TestDeflateContainerProfileSpec_NetworkNeighborsCollapse(t *testing.T) { + const hostCount = 60 + newIngress := func() []softwarecomposition.NetworkNeighbor { + ingress := make([]softwarecomposition.NetworkNeighbor, 0, hostCount) + for i := 1; i <= hostCount; i++ { + ingress = append(ingress, softwarecomposition.NetworkNeighbor{ + Identifier: fmt.Sprintf("external-%d", i), + Type: "external", + IPAddress: fmt.Sprintf("10.0.0.%d", i), + Ports: []softwarecomposition.NetworkPort{{Name: "80"}}, + }) + } + return ingress + } + + settings := dynamicpathdetector.CollapseSettings{ + NetworkIPGroupThreshold: 10, + NetworkCIDRFloorBits: 24, + } + + container := softwarecomposition.ContainerProfileSpec{ + Ingress: newIngress(), + } + + result := DeflateContainerProfileSpec(container, nil, settings) + + assert.Len(t, result.Ingress, 1, "expected all same-group host IPs to collapse into a single CIDR entry") + assert.Empty(t, result.Ingress[0].IPAddress) + assert.Equal(t, []string{"10.0.0.0/26"}, result.Ingress[0].IPAddresses) + assert.Equal(t, []softwarecomposition.NetworkPort{{Name: "80"}}, result.Ingress[0].Ports) + + // Confirm both call sites (NetworkNeighborhoodProcessor's deflateNetworkNeighbors + // and DeflateContainerProfileSpec's) collapse identically given the same settings. + directResult := deflateNetworkNeighbors(newIngress(), settings) + assert.Equal(t, directResult, result.Ingress) +} + func TestConsolidateData(t *testing.T) { // Prepare pool and connection pool := NewTestPool("/tmp") diff --git a/pkg/registry/file/dynamicpathdetector/collapse_config_from_crd.go b/pkg/registry/file/dynamicpathdetector/collapse_config_from_crd.go index 6bd43e79a..0e24d46e6 100644 --- a/pkg/registry/file/dynamicpathdetector/collapse_config_from_crd.go +++ b/pkg/registry/file/dynamicpathdetector/collapse_config_from_crd.go @@ -47,6 +47,8 @@ type CollapseSettings struct { OpenDynamicThreshold int EndpointDynamicThreshold int CollapseConfigs []CollapseConfig + NetworkIPGroupThreshold int + NetworkCIDRFloorBits int } // DefaultCollapseSettings returns the built-in baseline. The returned @@ -59,6 +61,8 @@ func DefaultCollapseSettings() CollapseSettings { OpenDynamicThreshold: OpenDynamicThreshold, EndpointDynamicThreshold: EndpointDynamicThreshold, CollapseConfigs: DefaultCollapseConfigs(), + NetworkIPGroupThreshold: NetworkIPGroupThreshold, + NetworkCIDRFloorBits: NetworkCIDRFloorBits, } } @@ -74,7 +78,11 @@ func DefaultCollapseSettings() CollapseSettings { // >= 1 child" — updateNodeStats collapses on Count > threshold — silently // flattening every open/endpoint in every profile to a single ⋯. Treat a // non-positive global threshold as "use the compiled-in default" instead, -// matching the absent-CR fallback the provider already performs. +// matching the absent-CR fallback the provider already performs. The same +// non-positive-means-default guard applies to NetworkIPGroupThreshold and +// NetworkCIDRFloorBits: a literal 0 group threshold would CIDR-collapse +// every NetworkNeighbor group, and a literal 0 floor would forbid any +// aggregated CIDR block. func CollapseSettingsFromCRD(crd *softwarecomposition.CollapseConfiguration) CollapseSettings { if crd == nil { return DefaultCollapseSettings() @@ -87,6 +95,14 @@ func CollapseSettingsFromCRD(crd *softwarecomposition.CollapseConfiguration) Col if endpoint <= 0 { endpoint = EndpointDynamicThreshold } + networkIPGroup := int(crd.Spec.NetworkIPGroupThreshold) + if networkIPGroup <= 0 { + networkIPGroup = NetworkIPGroupThreshold + } + networkCIDRFloor := int(crd.Spec.NetworkCIDRFloorBits) + if networkCIDRFloor <= 0 { + networkCIDRFloor = NetworkCIDRFloorBits + } configs := make([]CollapseConfig, len(crd.Spec.CollapseConfigs)) for i, entry := range crd.Spec.CollapseConfigs { configs[i] = CollapseConfig{ @@ -98,6 +114,8 @@ func CollapseSettingsFromCRD(crd *softwarecomposition.CollapseConfiguration) Col OpenDynamicThreshold: open, EndpointDynamicThreshold: endpoint, CollapseConfigs: configs, + NetworkIPGroupThreshold: networkIPGroup, + NetworkCIDRFloorBits: networkCIDRFloor, } } @@ -121,6 +139,8 @@ func CRDFromCollapseSettings(name string, settings CollapseSettings) *softwareco OpenDynamicThreshold: clampInt32(settings.OpenDynamicThreshold), EndpointDynamicThreshold: clampInt32(settings.EndpointDynamicThreshold), CollapseConfigs: entries, + NetworkIPGroupThreshold: clampInt32(settings.NetworkIPGroupThreshold), + NetworkCIDRFloorBits: clampInt32(settings.NetworkCIDRFloorBits), }, } } diff --git a/pkg/registry/file/dynamicpathdetector/tests/collapse_config_crd_test.go b/pkg/registry/file/dynamicpathdetector/tests/collapse_config_crd_test.go index dde04d2d9..be4efdf4b 100644 --- a/pkg/registry/file/dynamicpathdetector/tests/collapse_config_crd_test.go +++ b/pkg/registry/file/dynamicpathdetector/tests/collapse_config_crd_test.go @@ -198,6 +198,8 @@ func TestCollapseSettings_FullRoundTrip(t *testing.T) { Spec: softwarecomposition.CollapseConfigurationSpec{ OpenDynamicThreshold: 50, EndpointDynamicThreshold: 100, + NetworkIPGroupThreshold: 50, + NetworkCIDRFloorBits: 16, CollapseConfigs: []softwarecomposition.CollapseConfigEntry{ {Prefix: "/etc", Threshold: 100}, {Prefix: "/var/run", Threshold: 50}, diff --git a/pkg/registry/file/dynamicpathdetector/types.go b/pkg/registry/file/dynamicpathdetector/types.go index 6e62c261f..87be18a49 100644 --- a/pkg/registry/file/dynamicpathdetector/types.go +++ b/pkg/registry/file/dynamicpathdetector/types.go @@ -21,9 +21,15 @@ const ( // OpenDynamicThreshold is the fallback threshold used by AnalyzeOpens when // no more-specific CollapseConfig matches the walked path prefix. // EndpointDynamicThreshold is the counterpart for AnalyzeEndpoints. +// NetworkIPGroupThreshold is the count threshold above which a group of +// NetworkNeighbor entries differing only by IP gets CIDR-collapsed. +// NetworkCIDRFloorBits is the minimum CIDR prefix length (maximum breadth) +// a single aggregated block may have. const ( OpenDynamicThreshold = 50 EndpointDynamicThreshold = 100 + NetworkIPGroupThreshold = 50 + NetworkCIDRFloorBits = 16 ) // --- Collapse configuration --- diff --git a/pkg/registry/file/networkneighborhood_ipcollapse.go b/pkg/registry/file/networkneighborhood_ipcollapse.go new file mode 100644 index 000000000..347822d4f --- /dev/null +++ b/pkg/registry/file/networkneighborhood_ipcollapse.go @@ -0,0 +1,228 @@ +package file + +import ( + "crypto/sha256" + "encoding/hex" + "net/netip" + "sort" + "strings" + + "github.com/kubescape/storage/pkg/apis/softwarecomposition" + "github.com/kubescape/storage/pkg/registry/file/dynamicpathdetector" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ipCollapseFieldSep = "\x00" + +// collapseIPGroups aggregates NetworkNeighbor entries that differ only by IP +// into a small number of CIDR-bearing entries. Entries are grouped by +// (Type, DNS, NamespaceSelector, PodSelector); within a group whose count of +// aggregatable IPv4 host addresses exceeds settings.NetworkIPGroupThreshold, +// those hosts are replaced by covering CIDR block(s) no broader than +// settings.NetworkCIDRFloorBits. +// +// The pass is a fixpoint (AC10): already-collapsed CIDR values and the "*" +// sentinel / IPv6 values are treated as pass-through and are never re-parsed as +// host IPs or re-tightened, and collapsed output carries a deterministic +// Identifier, so a second run — whose groups now hold only CIDRs and thus have +// zero aggregatable hosts — leaves everything untouched. +func collapseIPGroups(entries []softwarecomposition.NetworkNeighbor, settings dynamicpathdetector.CollapseSettings) []softwarecomposition.NetworkNeighbor { + if entries == nil { + return nil + } + + threshold := settings.NetworkIPGroupThreshold + if threshold <= 0 { + threshold = dynamicpathdetector.NetworkIPGroupThreshold + } + floorBits := settings.NetworkCIDRFloorBits + if floorBits <= 0 || floorBits > 32 { + floorBits = dynamicpathdetector.NetworkCIDRFloorBits + } + + type group struct { + key string + entries []softwarecomposition.NetworkNeighbor + repType softwarecomposition.CommunicationType + repDNS string + repNsSel *metav1.LabelSelector + repPodSel *metav1.LabelSelector + } + + order := make([]string, 0) + groups := make(map[string]*group) + for _, e := range entries { + key := neighborGroupKey(e) + g, ok := groups[key] + if !ok { + g = &group{ + key: key, + repType: e.Type, + repDNS: e.DNS, + repNsSel: e.NamespaceSelector, + repPodSel: e.PodSelector, + } + groups[key] = g + order = append(order, key) + } + g.entries = append(g.entries, e) + } + + out := make([]softwarecomposition.NetworkNeighbor, 0, len(entries)) + for _, key := range order { + g := groups[key] + + hosts, passthrough := classifyGroupAddresses(g.entries) + if len(hosts) <= threshold { + out = append(out, g.entries...) + continue + } + + cidrs := aggregateHosts(hosts, floorBits) + values := append(cidrs, passthrough...) + sort.Strings(values) + + var dnsNames []string + var ports []softwarecomposition.NetworkPort + for _, e := range g.entries { + dnsNames = append(dnsNames, e.DNSNames...) + ports = append(ports, e.Ports...) + } + dnsNames = DeflateSortString(dnsNames) + ports = DeflateStringer(ports) + + for _, v := range values { + out = append(out, softwarecomposition.NetworkNeighbor{ + Identifier: collapsedIdentifier(g.repType, g.repDNS, g.repNsSel, g.repPodSel, []string{v}), + Type: g.repType, + DNS: g.repDNS, + DNSNames: append([]string(nil), dnsNames...), + Ports: append([]softwarecomposition.NetworkPort(nil), ports...), + PodSelector: g.repPodSel, + NamespaceSelector: g.repNsSel, + IPAddress: "", + IPAddresses: []string{v}, + }) + } + } + return out +} + +// classifyGroupAddresses splits a group's address values into aggregatable IPv4 +// host addresses (deduped) and pass-through values held verbatim. An entry's +// value comes from the singular IPAddress when set, otherwise from each element +// of IPAddresses. CIDRs, the "*" sentinel, IPv6 and unparseable values are +// pass-through and are never fed to aggregation, which is what makes the pass a +// fixpoint on already-collapsed input. +func classifyGroupAddresses(entries []softwarecomposition.NetworkNeighbor) ([]netip.Addr, []string) { + seenHost := map[netip.Addr]struct{}{} + seenPass := map[string]struct{}{} + var hosts []netip.Addr + var passthrough []string + + classify := func(v string) { + if v == "" { + return + } + if addr, err := netip.ParseAddr(v); err == nil && addr.Is4() { + if _, ok := seenHost[addr]; !ok { + seenHost[addr] = struct{}{} + hosts = append(hosts, addr) + } + return + } + if _, ok := seenPass[v]; !ok { + seenPass[v] = struct{}{} + passthrough = append(passthrough, v) + } + } + + for _, e := range entries { + if e.IPAddress != "" { + classify(e.IPAddress) + continue + } + for _, v := range e.IPAddresses { + classify(v) + } + } + return hosts, passthrough +} + +// aggregateHosts returns the CIDR block(s) covering the given IPv4 hosts. If the +// hosts share a common prefix at least as long as floorBits it is emitted as a +// single block; otherwise each host is bucketed into a floorBits-length prefix +// so no emitted block is ever broader than the floor. +func aggregateHosts(hosts []netip.Addr, floorBits int) []string { + if len(hosts) == 0 { + return nil + } + if commonLen := commonPrefixLen(hosts); commonLen >= floorBits { + return []string{netip.PrefixFrom(hosts[0], commonLen).Masked().String()} + } + seen := map[string]struct{}{} + var out []string + for _, addr := range hosts { + cidr := netip.PrefixFrom(addr, floorBits).Masked().String() + if _, ok := seen[cidr]; !ok { + seen[cidr] = struct{}{} + out = append(out, cidr) + } + } + return out +} + +// commonPrefixLen returns the number of leading bits shared by every address in +// the set. All addresses are assumed to be IPv4. +func commonPrefixLen(addrs []netip.Addr) int { + base := addrs[0].As4() + common := 32 + for _, addr := range addrs[1:] { + b := addr.As4() + n := 0 + for i := 0; i < 4 && n < common; i++ { + x := base[i] ^ b[i] + if x == 0 { + n += 8 + continue + } + for bit := 7; bit >= 0; bit-- { + if x&(1< common prefix exactly the floor (16) -> one block. + var in []softwarecomposition.NetworkNeighbor + for i := 0; i < 60; i++ { + in = append(in, hostNeighbor(fmt.Sprintf("10.1.%d.0", i*4))) + } + + out := collapseIPGroups(in, testSettings()) + + require.Len(t, out, 1) + assert.Equal(t, []string{"10.1.0.0/16"}, out[0].IPAddresses) + assert.Empty(t, out[0].IPAddress) +} + +func TestCollapseIPGroups_AboveThresholdBroaderThanFloorBuckets(t *testing.T) { + // 60 hosts spread across many /16s -> common prefix broader than floor -> + // floor-bucket into distinct /16 blocks, none broader than the floor. + var in []softwarecomposition.NetworkNeighbor + for i := 0; i < 60; i++ { + in = append(in, hostNeighbor(fmt.Sprintf("%d.%d.0.1", 10+i, i))) + } + + out := collapseIPGroups(in, testSettings()) + + assert.Greater(t, len(out), 1) + for _, e := range out { + require.Len(t, e.IPAddresses, 1) + p, err := netip.ParsePrefix(e.IPAddresses[0]) + require.NoError(t, err) + assert.GreaterOrEqual(t, p.Bits(), 16, "no emitted block may be broader than the floor") + } +} + +func TestCollapseIPGroups_MixedGroupsNotCrossMerged(t *testing.T) { + var in []softwarecomposition.NetworkNeighbor + for i := 0; i < 60; i++ { + e := hostNeighbor(fmt.Sprintf("10.2.0.%d", i)) + e.Type = softwarecomposition.CommunicationTypeEgress + e.DNS = "egress.example" + in = append(in, e) + } + for i := 0; i < 60; i++ { + e := hostNeighbor(fmt.Sprintf("10.2.0.%d", i)) + e.Type = softwarecomposition.CommunicationTypeIngress + e.DNS = "ingress.example" + in = append(in, e) + } + + out := collapseIPGroups(in, testSettings()) + + dnsSeen := map[string]softwarecomposition.CommunicationType{} + for _, e := range out { + if prev, ok := dnsSeen[e.DNS]; ok { + assert.Equal(t, prev, e.Type) + } + dnsSeen[e.DNS] = e.Type + } + assert.Contains(t, dnsSeen, "egress.example") + assert.Contains(t, dnsSeen, "ingress.example") +} + +func TestCollapseIPGroups_DifferentSelectorsNotMerged(t *testing.T) { + sel := func(v string) *metav1.LabelSelector { + return &metav1.LabelSelector{MatchLabels: map[string]string{"app": v}} + } + var in []softwarecomposition.NetworkNeighbor + for i := 0; i < 60; i++ { + e := hostNeighbor(fmt.Sprintf("10.3.0.%d", i)) + e.PodSelector = sel("a") + in = append(in, e) + } + for i := 0; i < 60; i++ { + e := hostNeighbor(fmt.Sprintf("10.3.0.%d", i)) + e.PodSelector = sel("b") + in = append(in, e) + } + + out := collapseIPGroups(in, testSettings()) + + require.Len(t, out, 2) + selectors := map[string]bool{} + for _, e := range out { + require.NotNil(t, e.PodSelector) + selectors[e.PodSelector.MatchLabels["app"]] = true + } + assert.True(t, selectors["a"]) + assert.True(t, selectors["b"]) +} + +func TestCollapseIPGroups_RealWorldShapeOrdersOfMagnitude(t *testing.T) { + var in []softwarecomposition.NetworkNeighbor + // ~500 IPs clustered in 100.68.x.x + for i := 0; i < 250; i++ { + in = append(in, hostNeighbor(fmt.Sprintf("100.68.%d.%d", i/256, i%256))) + } + // ~250 IPs clustered in 16.15.183.x plus neighboring /24s + for i := 0; i < 250; i++ { + in = append(in, hostNeighbor(fmt.Sprintf("16.15.%d.%d", 180+i/256, i%256))) + } + + out := collapseIPGroups(in, testSettings()) + + assert.Less(t, len(out), 10) + assert.Less(t, len(out), len(in)/50) + for _, e := range out { + require.Len(t, e.IPAddresses, 1) + p, err := netip.ParsePrefix(e.IPAddresses[0]) + require.NoError(t, err) + assert.GreaterOrEqual(t, p.Bits(), 16) + } +} + +func TestCollapseIPGroups_Idempotent(t *testing.T) { + var in []softwarecomposition.NetworkNeighbor + for i := 0; i < 80; i++ { + e := hostNeighbor(fmt.Sprintf("100.68.%d.%d", i/64, i%64)) + e.Ports = []softwarecomposition.NetworkPort{{Name: "tcp-443"}} + in = append(in, e) + } + // already-collapsed CIDR carried in the plural field + in = append(in, softwarecomposition.NetworkNeighbor{ + Type: softwarecomposition.CommunicationTypeEgress, + DNS: "example.com", + IPAddresses: []string{"200.0.0.0/16"}, + }) + // "*" sentinel + in = append(in, softwarecomposition.NetworkNeighbor{ + Type: softwarecomposition.CommunicationTypeEgress, + DNS: "example.com", + IPAddresses: []string{"*"}, + }) + // IPv6 entry + in = append(in, softwarecomposition.NetworkNeighbor{ + Type: softwarecomposition.CommunicationTypeEgress, + DNS: "example.com", + IPAddress: "2001:db8::1", + }) + + once := collapseIPGroups(in, testSettings()) + twice := collapseIPGroups(once, testSettings()) + + assert.Equal(t, once, twice, "collapseIPGroups must be a fixpoint") + + // pass-through values survived + var values []string + for _, e := range once { + values = append(values, e.IPAddresses...) + } + assert.Contains(t, values, "*") + assert.Contains(t, values, "200.0.0.0/16") + assert.Contains(t, values, "2001:db8::1") +} + +func TestCollapseIPGroups_FieldContract(t *testing.T) { + var in []softwarecomposition.NetworkNeighbor + for i := 0; i < 60; i++ { + in = append(in, hostNeighbor(fmt.Sprintf("10.4.0.%d", i))) + } + + out := collapseIPGroups(in, testSettings()) + + for _, e := range out { + assert.NotEmpty(t, e.IPAddresses) + assert.Empty(t, e.IPAddress) + assert.NotEmpty(t, e.Identifier) + } +} + +func TestCollapseIPGroups_MultiBucketReplicatesDNSNamesAndPorts(t *testing.T) { + var in []softwarecomposition.NetworkNeighbor + for i := 0; i < 60; i++ { + e := hostNeighbor(fmt.Sprintf("%d.%d.0.1", 20+i, i)) + e.DNSNames = []string{fmt.Sprintf("host-%d.example", i)} + e.Ports = []softwarecomposition.NetworkPort{{Name: fmt.Sprintf("tcp-%d", 8000+i)}} + in = append(in, e) + } + + out := collapseIPGroups(in, testSettings()) + + require.Greater(t, len(out), 1) + first := out[0] + require.NotEmpty(t, first.DNSNames) + require.NotEmpty(t, first.Ports) + for _, e := range out { + assert.Equal(t, first.DNSNames, e.DNSNames, "every bucket entry gets the full merged DNSNames") + assert.Equal(t, first.Ports, e.Ports, "every bucket entry gets the full merged Ports") + } +} + +func TestCollapseIPGroups_IPv6PassThrough(t *testing.T) { + var in []softwarecomposition.NetworkNeighbor + for i := 0; i < 60; i++ { + in = append(in, hostNeighbor(fmt.Sprintf("10.5.0.%d", i))) + } + in = append(in, softwarecomposition.NetworkNeighbor{ + Type: softwarecomposition.CommunicationTypeEgress, + DNS: "example.com", + IPAddress: "2001:db8::42", + }) + + out := collapseIPGroups(in, testSettings()) + + var values []string + for _, e := range out { + values = append(values, e.IPAddresses...) + } + assert.Contains(t, values, "2001:db8::42") +} + +func TestCollapseIPGroups_NilInput(t *testing.T) { + assert.Nil(t, collapseIPGroups(nil, testSettings())) +} diff --git a/pkg/registry/file/networkneighborhood_processor.go b/pkg/registry/file/networkneighborhood_processor.go index 15d852b85..f7430218f 100644 --- a/pkg/registry/file/networkneighborhood_processor.go +++ b/pkg/registry/file/networkneighborhood_processor.go @@ -9,19 +9,49 @@ import ( "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" "github.com/kubescape/storage/pkg/apis/softwarecomposition" "github.com/kubescape/storage/pkg/config" + "github.com/kubescape/storage/pkg/registry/file/dynamicpathdetector" "k8s.io/apimachinery/pkg/runtime" ) type NetworkNeighborhoodProcessor struct { maxNetworkNeighborhoodSize int + // collapseSettings is the lookup hook the deflate path consults for + // per-prefix thresholds. Defaults to dynamicpathdetector.DefaultCollapseSettings; + // production wiring may override via SetCollapseSettings to a provider that + // reads the cluster-scoped CollapseConfiguration "default" CR. + collapseSettings dynamicpathdetector.CollapseSettingsProvider } func NewNetworkNeighborhoodProcessor(cfg config.Config) *NetworkNeighborhoodProcessor { return &NetworkNeighborhoodProcessor{ maxNetworkNeighborhoodSize: cfg.MaxNetworkNeighborhoodSize, + collapseSettings: dynamicpathdetector.DefaultCollapseSettings, } } +// SetCollapseSettings overrides the provider the deflate path uses to fetch +// effective thresholds. Pass dynamicpathdetector.DefaultCollapseSettings to +// fall back to compiled-in defaults; production wiring passes a provider +// that reads the CollapseConfiguration CR. +func (a *NetworkNeighborhoodProcessor) SetCollapseSettings(p dynamicpathdetector.CollapseSettingsProvider) { + if p == nil { + a.collapseSettings = dynamicpathdetector.DefaultCollapseSettings + return + } + a.collapseSettings = p +} + +// effectiveCollapseSettings is the safe accessor for the deflate path. It +// returns the result of the configured provider, or — when the processor +// was constructed without using NewNetworkNeighborhoodProcessor (zero-value +// field, no factory call) — the compiled-in defaults. +func (a NetworkNeighborhoodProcessor) effectiveCollapseSettings() dynamicpathdetector.CollapseSettings { + if a.collapseSettings == nil { + return dynamicpathdetector.DefaultCollapseSettings() + } + return a.collapseSettings() +} + var _ Processor = (*NetworkNeighborhoodProcessor)(nil) func (a NetworkNeighborhoodProcessor) AfterCreate(_ context.Context, _ runtime.Object) error { @@ -40,10 +70,12 @@ func (a NetworkNeighborhoodProcessor) PreSave(_ context.Context, object runtime. // size is the sum of all ingress/egress in all containers var size int + settings := a.effectiveCollapseSettings() + // Define a function to process a slice of containers processContainers := func(containers []softwarecomposition.NetworkNeighborhoodContainer) []softwarecomposition.NetworkNeighborhoodContainer { for i, container := range containers { - containers[i] = deflateNetworkNeighborhoodContainer(container) + containers[i] = deflateNetworkNeighborhoodContainer(container, settings) size += len(containers[i].Ingress) size += len(containers[i].Egress) } @@ -70,18 +102,23 @@ func (a NetworkNeighborhoodProcessor) PreSave(_ context.Context, object runtime. func (a NetworkNeighborhoodProcessor) SetStorage(_ ContainerProfileStorage) {} -func deflateNetworkNeighborhoodContainer(container softwarecomposition.NetworkNeighborhoodContainer) softwarecomposition.NetworkNeighborhoodContainer { +func deflateNetworkNeighborhoodContainer(container softwarecomposition.NetworkNeighborhoodContainer, settings dynamicpathdetector.CollapseSettings) softwarecomposition.NetworkNeighborhoodContainer { return softwarecomposition.NetworkNeighborhoodContainer{ Name: container.Name, - Ingress: deflateNetworkNeighbors(container.Ingress), - Egress: deflateNetworkNeighbors(container.Egress), + Ingress: deflateNetworkNeighbors(container.Ingress, settings), + Egress: deflateNetworkNeighbors(container.Egress, settings), } } // NetworkNeighbors are merged on Identifier // DNSNames are deduplicated // Ports are merged on Name -func deflateNetworkNeighbors(in []softwarecomposition.NetworkNeighbor) []softwarecomposition.NetworkNeighbor { +// Then, groups of entries differing only by IP are collapsed into CIDR-bearing +// entries once their count exceeds settings.NetworkIPGroupThreshold (see +// collapseIPGroups). That second pass is a fixpoint (AC10): re-running it on +// its own output leaves already-collapsed entries untouched, so repeated saves +// are idempotent. +func deflateNetworkNeighbors(in []softwarecomposition.NetworkNeighbor, settings dynamicpathdetector.CollapseSettings) []softwarecomposition.NetworkNeighbor { if in == nil { return nil } @@ -102,5 +139,5 @@ func deflateNetworkNeighbors(in []softwarecomposition.NetworkNeighbor) []softwar out[i].DNSNames = DeflateSortString(out[i].DNSNames) out[i].Ports = DeflateStringer(out[i].Ports) } - return out + return collapseIPGroups(out, settings) } diff --git a/pkg/registry/file/networkneighborhood_processor_test.go b/pkg/registry/file/networkneighborhood_processor_test.go index a9e909978..67d8eba77 100644 --- a/pkg/registry/file/networkneighborhood_processor_test.go +++ b/pkg/registry/file/networkneighborhood_processor_test.go @@ -8,6 +8,7 @@ import ( "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" "github.com/kubescape/storage/pkg/apis/softwarecomposition" "github.com/kubescape/storage/pkg/config" + "github.com/kubescape/storage/pkg/registry/file/dynamicpathdetector" "github.com/stretchr/testify/assert" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -130,3 +131,40 @@ func TestNetworkNeighborhoodProcessor_PreSave(t *testing.T) { }) } } + +func TestNetworkNeighborhoodProcessor_PreSave_IPCollapse(t *testing.T) { + const hostCount = 60 + ingress := make([]softwarecomposition.NetworkNeighbor, 0, hostCount) + for i := 1; i <= hostCount; i++ { + ingress = append(ingress, softwarecomposition.NetworkNeighbor{ + Identifier: fmt.Sprintf("external-%d", i), + Type: "external", + IPAddress: fmt.Sprintf("10.0.0.%d", i), + Ports: []softwarecomposition.NetworkPort{{Name: "80"}}, + }) + } + profile := &softwarecomposition.NetworkNeighborhood{ + ObjectMeta: v1.ObjectMeta{Annotations: map[string]string{}}, + Spec: softwarecomposition.NetworkNeighborhoodSpec{ + Containers: []softwarecomposition.NetworkNeighborhoodContainer{ + {Name: "container1", Ingress: ingress}, + }, + }, + } + + a := NewNetworkNeighborhoodProcessor(config.Config{MaxNetworkNeighborhoodSize: 40000}) + a.SetCollapseSettings(func() dynamicpathdetector.CollapseSettings { + return dynamicpathdetector.CollapseSettings{ + NetworkIPGroupThreshold: 10, + NetworkCIDRFloorBits: 24, + } + }) + + assert.NoError(t, a.PreSave(context.TODO(), profile)) + + got := profile.Spec.Containers[0].Ingress + assert.Len(t, got, 1, "expected all same-group host IPs to collapse into a single CIDR entry") + assert.Empty(t, got[0].IPAddress) + assert.Equal(t, []string{"10.0.0.0/26"}, got[0].IPAddresses) + assert.Equal(t, []softwarecomposition.NetworkPort{{Name: "80"}}, got[0].Ports) +} diff --git a/pkg/registry/softwarecomposition/collapseconfiguration/strategy.go b/pkg/registry/softwarecomposition/collapseconfiguration/strategy.go index f5c2cd443..f065d4538 100644 --- a/pkg/registry/softwarecomposition/collapseconfiguration/strategy.go +++ b/pkg/registry/softwarecomposition/collapseconfiguration/strategy.go @@ -127,13 +127,16 @@ func (CollapseConfigurationStrategy) WarningsOnUpdate(_ context.Context, _, _ ru // rejects duplicate prefixes (which would silently produce a non-deterministic // longest-prefix-wins outcome at runtime). // -// The two global thresholds are optional and only rejected when negative: 0 +// The global thresholds (OpenDynamicThreshold, EndpointDynamicThreshold, +// NetworkIPGroupThreshold) are optional and only rejected when negative: 0 // (or an omitted field) is a valid "use the compiled-in default" sentinel, // honored by CollapseSettingsFromCRD. This is deliberately more permissive // than the per-prefix entries (which require >= 1): a per-prefix entry is an // explicit override and has no default to fall back to, whereas an omitted // global threshold must not be treated as a literal 0 that collapses -// everything. +// everything. NetworkCIDRFloorBits shares the same 0-means-default sentinel +// but additionally caps the valid non-zero range at [1,32], since it encodes +// a CIDR prefix length rather than an unbounded count. func validateCollapseConfigurationSpec(spec *softwarecomposition.CollapseConfigurationSpec, fp *field.Path) field.ErrorList { var errs field.ErrorList if spec.OpenDynamicThreshold < 0 { @@ -142,6 +145,12 @@ func validateCollapseConfigurationSpec(spec *softwarecomposition.CollapseConfigu if spec.EndpointDynamicThreshold < 0 { errs = append(errs, field.Invalid(fp.Child("endpointDynamicThreshold"), spec.EndpointDynamicThreshold, "must be >= 0 (0 means use the compiled-in default)")) } + if spec.NetworkIPGroupThreshold < 0 { + errs = append(errs, field.Invalid(fp.Child("networkIPGroupThreshold"), spec.NetworkIPGroupThreshold, "must be >= 0 (0 means use the compiled-in default)")) + } + if spec.NetworkCIDRFloorBits < 0 || spec.NetworkCIDRFloorBits > 32 { + errs = append(errs, field.Invalid(fp.Child("networkCIDRFloorBits"), spec.NetworkCIDRFloorBits, "must be 0 (use the compiled-in default) or in the range [1,32]")) + } seen := make(map[string]int, len(spec.CollapseConfigs)) cfgsPath := fp.Child("collapseConfigs") for i, e := range spec.CollapseConfigs { diff --git a/pkg/registry/softwarecomposition/collapseconfiguration/strategy_test.go b/pkg/registry/softwarecomposition/collapseconfiguration/strategy_test.go index f8224bf7e..7ebc4b021 100644 --- a/pkg/registry/softwarecomposition/collapseconfiguration/strategy_test.go +++ b/pkg/registry/softwarecomposition/collapseconfiguration/strategy_test.go @@ -45,6 +45,8 @@ func TestValidate_Valid(t *testing.T) { Spec: softwarecomposition.CollapseConfigurationSpec{ OpenDynamicThreshold: 50, EndpointDynamicThreshold: 100, + NetworkIPGroupThreshold: 50, + NetworkCIDRFloorBits: 16, CollapseConfigs: []softwarecomposition.CollapseConfigEntry{ {Prefix: "/etc", Threshold: 100}, {Prefix: "/var/log", Threshold: 50}, @@ -71,6 +73,62 @@ func TestValidate_NegativeThresholds(t *testing.T) { } } +func TestValidate_NetworkFieldsZeroMeansDefault(t *testing.T) { + s := NewStrategy(newScheme()) + cc := &softwarecomposition.CollapseConfiguration{ + Spec: softwarecomposition.CollapseConfigurationSpec{ + NetworkIPGroupThreshold: 0, + NetworkCIDRFloorBits: 0, + }, + } + if errs := s.Validate(context.Background(), cc); len(errs) != 0 { + t.Fatalf("expected zero network fields to be accepted as use-default, got: %v", errs) + } +} + +func TestValidate_NetworkIPGroupThresholdNegative(t *testing.T) { + s := NewStrategy(newScheme()) + cc := &softwarecomposition.CollapseConfiguration{ + Spec: softwarecomposition.CollapseConfigurationSpec{ + NetworkIPGroupThreshold: -1, + }, + } + errs := s.Validate(context.Background(), cc) + if len(errs) != 1 { + t.Fatalf("expected 1 error for negative NetworkIPGroupThreshold, got %d: %v", len(errs), errs) + } +} + +func TestValidate_NetworkCIDRFloorBitsRange(t *testing.T) { + s := NewStrategy(newScheme()) + for _, tc := range []struct { + name string + value int32 + wantErr bool + }{ + {"zero-is-default", 0, false}, + {"min-valid", 1, false}, + {"max-valid", 32, false}, + {"negative", -1, true}, + {"above-32", 33, true}, + } { + t.Run(tc.name, func(t *testing.T) { + cc := &softwarecomposition.CollapseConfiguration{ + Spec: softwarecomposition.CollapseConfigurationSpec{ + NetworkCIDRFloorBits: tc.value, + }, + } + errs := s.Validate(context.Background(), cc) + if tc.wantErr && len(errs) == 0 { + t.Fatalf("expected an error for NetworkCIDRFloorBits=%d, got none", tc.value) + } + if !tc.wantErr && len(errs) != 0 { + t.Fatalf("expected no error for NetworkCIDRFloorBits=%d, got: %v", tc.value, errs) + } + }) + } +} + func TestValidate_EntryRules(t *testing.T) { s := NewStrategy(newScheme()) cc := &softwarecomposition.CollapseConfiguration{ From 72504c8665af674b31cc8aca74c0fa4e1311ba47 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Wed, 22 Jul 2026 13:10:07 +0200 Subject: [PATCH 2/7] fix(networkpolicy): dedupe merged peers and fix compareNP self-comparison compareNP compared p1's Egress/Ingress against itself instead of p2, making every compareNP-based assertion in TestGenerateNetworkPolicy a no-op (pre-existing, unrelated to any recent feature work). Fixing the comparison surfaced a real latent bug: mergeIngressRulesByPorts and mergeEgressRulesByPorts appended every IPBlock peer sharing a port key without deduping, so the same IP declared across multiple distinct NetworkNeighbor entries (e.g. seen from different containers) ended up duplicated 2-3x in the merged rule. Harmless semantically but clearly unintended. Add containsIPBlockPeer and use it at both merge sites, and correct the 3 stale test fixtures that had assumed a merged-multi-port rule shape the merge functions never actually produce (confirmed by the existing, always-passing TestMergeIngressRulesByPorts/ TestMergeEgressRulesByPorts, which document the real one-rule-per-port design). A separate, unrelated flaky test (TestGenerateNetworkPolicy/ real_duplicate_bug_test) was found during investigation: hash() uses encoding/gob on a struct containing a map, so Go's randomized map iteration order occasionally produces two different hashes for the same logical rule. Not fixed here - left as a follow-up. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Matthias Bertschy --- .../networkpolicy/v2/networkpolicy.go | 22 +++++- .../networkpolicy/v2/networkpolicy_test.go | 71 ++++++++++++++----- 2 files changed, 74 insertions(+), 19 deletions(-) diff --git a/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go b/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go index 59a6fe7b2..6dff2314c 100644 --- a/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go +++ b/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go @@ -163,6 +163,18 @@ func listEgressNetworkNeighbors(nn *softwarecomposition.NetworkNeighborhood) []s } +// containsIPBlockPeer reports whether peers already contains an entry with the given CIDR. +// Used to avoid duplicate peer entries when merging rules that reference the same IP +// from multiple distinct NetworkNeighbor entries (e.g. the same peer seen across containers). +func containsIPBlockPeer(peers []softwarecomposition.NetworkPolicyPeer, cidr string) bool { + for _, existing := range peers { + if existing.IPBlock != nil && existing.IPBlock.CIDR == cidr { + return true + } + } + return false +} + func mergeIngressRulesByPorts(rules []softwarecomposition.NetworkPolicyIngressRule) []softwarecomposition.NetworkPolicyIngressRule { type PortProtocolKey struct { Port int32 @@ -196,7 +208,10 @@ func mergeIngressRulesByPorts(rules []softwarecomposition.NetworkPolicyIngressRu keys = append(keys, key) } for _, peer := range rule.From { - if peer.IPBlock != nil { + if peer.IPBlock == nil { + continue + } + if !containsIPBlockPeer(merged[key], peer.IPBlock.CIDR) { merged[key] = append(merged[key], peer) } } @@ -279,7 +294,10 @@ func mergeEgressRulesByPorts(rules []softwarecomposition.NetworkPolicyEgressRule keys = append(keys, key) } for _, peer := range rule.To { - if peer.IPBlock != nil { + if peer.IPBlock == nil { + continue + } + if !containsIPBlockPeer(merged[key], peer.IPBlock.CIDR) { merged[key] = append(merged[key], peer) } } diff --git a/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy_test.go b/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy_test.go index 490f2a075..c489d9be7 100644 --- a/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy_test.go +++ b/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy_test.go @@ -253,6 +253,17 @@ func TestGenerateNetworkPolicy(t *testing.T) { Port: ptr.To(int32(80)), Protocol: &protocolTCP, }, + }, + From: []softwarecomposition.NetworkPolicyPeer{ + { + IPBlock: &softwarecomposition.IPBlock{ + CIDR: "10.0.0.1/32", + }, + }, + }, + }, + { + Ports: []softwarecomposition.NetworkPolicyPort{ { Port: ptr.To(int32(443)), Protocol: &protocolTCP, @@ -946,6 +957,17 @@ func TestGenerateNetworkPolicy(t *testing.T) { Port: ptr.To(int32(80)), Protocol: &protocolTCP, }, + }, + From: []softwarecomposition.NetworkPolicyPeer{ + { + IPBlock: &softwarecomposition.IPBlock{ + CIDR: "10.0.0.1/32", + }, + }, + }, + }, + { + Ports: []softwarecomposition.NetworkPolicyPort{ { Port: ptr.To(int32(443)), Protocol: &protocolTCP, @@ -1237,16 +1259,29 @@ func TestGenerateNetworkPolicy(t *testing.T) { Port: ptr.To(int32(80)), Protocol: &protocolTCP, }, + }, + From: []softwarecomposition.NetworkPolicyPeer{ { - Port: ptr.To(int32(90)), - Protocol: &protocolTCP, + IPBlock: &softwarecomposition.IPBlock{ + CIDR: "10.0.0.1/32", + }, }, { - Port: ptr.To(int32(100)), - Protocol: &protocolTCP, + IPBlock: &softwarecomposition.IPBlock{ + CIDR: "10.0.0.3/32", + }, }, { - Port: ptr.To(int32(443)), + IPBlock: &softwarecomposition.IPBlock{ + CIDR: "10.0.0.4/32", + }, + }, + }, + }, + { + Ports: []softwarecomposition.NetworkPolicyPort{ + { + Port: ptr.To(int32(90)), Protocol: &protocolTCP, }, }, @@ -1261,34 +1296,36 @@ func TestGenerateNetworkPolicy(t *testing.T) { { Ports: []softwarecomposition.NetworkPolicyPort{ { - Port: ptr.To(int32(80)), - Protocol: &protocolTCP, - }, - { - Port: ptr.To(int32(443)), + Port: ptr.To(int32(100)), Protocol: &protocolTCP, }, }, From: []softwarecomposition.NetworkPolicyPeer{ { IPBlock: &softwarecomposition.IPBlock{ - CIDR: "10.0.0.3/32", + CIDR: "10.0.0.1/32", }, }, }, }, { Ports: []softwarecomposition.NetworkPolicyPort{ - { - Port: ptr.To(int32(80)), - Protocol: &protocolTCP, - }, { Port: ptr.To(int32(443)), Protocol: &protocolTCP, }, }, From: []softwarecomposition.NetworkPolicyPeer{ + { + IPBlock: &softwarecomposition.IPBlock{ + CIDR: "10.0.0.1/32", + }, + }, + { + IPBlock: &softwarecomposition.IPBlock{ + CIDR: "10.0.0.3/32", + }, + }, { IPBlock: &softwarecomposition.IPBlock{ CIDR: "10.0.0.4/32", @@ -2824,10 +2861,10 @@ func compareNP(p1, p2 *softwarecomposition.GeneratedNetworkPolicy) error { return fmt.Errorf("one of the policies is nil") } - if err := compareEgress(p1.Spec.Spec.Egress, p1.Spec.Spec.Egress); err != nil { + if err := compareEgress(p1.Spec.Spec.Egress, p2.Spec.Spec.Egress); err != nil { return fmt.Errorf("Spec is different. p1.Spec.Spec.Egress: %v, p2.Spec.Spec.Egress: %v", p1.Spec.Spec.Egress, p2.Spec.Spec.Egress) } - if err := compareIngress(p1.Spec.Spec.Ingress, p1.Spec.Spec.Ingress); err != nil { + if err := compareIngress(p1.Spec.Spec.Ingress, p2.Spec.Spec.Ingress); err != nil { return fmt.Errorf("Spec is different. p1.Spec.Spec.Ingress: %v, p2.Spec.Spec.Ingress: %v", p1.Spec.Spec.Ingress, p2.Spec.Spec.Ingress) } From a5b922ed32203a55f38eae426169c3700520f3b8 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Wed, 22 Jul 2026 13:26:14 +0200 Subject: [PATCH 3/7] fix(networkpolicy): make rule-dedup hash deterministic hash() gob-encoded rules/policyRefs to dedupe them, but gob's map encoding follows Go's randomized map iteration order. A rule containing a LabelSelector's MatchLabels map could therefore hash differently across otherwise-identical calls, so GenerateNetworkPolicy's dedup-by- hash sometimes kept a duplicate identical rule and sometimes didn't - reproduced reliably under `go test -run TestGenerateNetworkPolicy -count=40` (3/40 failures on TestGenerateNetworkPolicy/ real_duplicate_bug_test before this fix, 0/100 after). This was real product nondeterminism (generated NetworkPolicies weren't byte-stable run-to-run), not just test flakiness. Switch hash() to json.Marshal, which sorts map keys, giving a stable encoding regardless of map iteration order. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Matthias Bertschy --- .../networkpolicy/v2/networkpolicy.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go b/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go index 6dff2314c..9febd5121 100644 --- a/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go +++ b/pkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.go @@ -1,10 +1,9 @@ package networkpolicy import ( - "bytes" "crypto/sha256" - "encoding/gob" "encoding/hex" + "encoding/json" "fmt" "net" "net/netip" @@ -622,12 +621,16 @@ func IsAvailable(nn *softwarecomposition.NetworkNeighborhood) bool { } } +// hash must be a deterministic function of s's contents: gob's map encoding follows Go's +// randomized map iteration order, so gob-encoding a struct containing a map (e.g. a +// LabelSelector's MatchLabels) previously produced a different byte sequence - and thus a +// different hash - across otherwise-identical calls. json.Marshal sorts map keys, giving a +// stable encoding regardless of map iteration order. func hash(s any) (string, error) { - - var b bytes.Buffer - if err := gob.NewEncoder(&b).Encode(s); err != nil { + b, err := json.Marshal(s) + if err != nil { return "", err } - vv := sha256.Sum256(b.Bytes()) + vv := sha256.Sum256(b) return hex.EncodeToString(vv[:]), nil } From 714296906486fb4efebc51dcb082be5290a91b88 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Thu, 23 Jul 2026 10:37:10 +0200 Subject: [PATCH 4/7] fix(storage): default NetworkCIDRFloorBits to /24 instead of /16 /16 delivers the most aggressive entry-count reduction (the real 8,687-entry motivating case collapses to ~10 entries) but at the cost of much broader, less precise CIDR blocks. Measured against that same real-world IP set, a /24 floor still collapses it to ~341 entries (down from 8,687) while keeping blocks 256x tighter. 341 is an acceptable trade-off, so make /24 the default; /16 remains available for operators who want maximum compaction. Also fill in the two new fields in the sample CollapseConfiguration YAML, which was never updated when they were introduced. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Matthias Bertschy --- .../collapseconfiguration-default-sample.yaml | 7 ++++++- docs/features/networkneighbors-collapsing.md | 18 ++++++++++-------- pkg/registry/file/dynamicpathdetector/types.go | 2 +- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/artifacts/collapseconfiguration-default-sample.yaml b/artifacts/collapseconfiguration-default-sample.yaml index 64c7b8b41..9aea1f95b 100644 --- a/artifacts/collapseconfiguration-default-sample.yaml +++ b/artifacts/collapseconfiguration-default-sample.yaml @@ -12,7 +12,7 @@ # taste, but any default you omit here stops being applied. Delete the # resource entirely to fall back to the compiled-in defaults. # -# The two global thresholds are optional: omit them (or set 0) to use the +# The global thresholds are optional: omit them (or set 0) to use the # compiled-in defaults. Do NOT set them to 0 expecting "no collapsing" — 0 is # treated as "use default", because a literal 0 would collapse everything. # @@ -38,3 +38,8 @@ spec: threshold: 50 - prefix: /app threshold: 50 + # NetworkNeighbor IP-collapsing thresholds — see docs/features/networkneighbors-collapsing.md. + # Group size above which NetworkNeighbor entries differing only by IP get CIDR-collapsed. + networkIPGroupThreshold: 50 + # Minimum CIDR prefix length (maximum breadth) a single aggregated block may have. + networkCIDRFloorBits: 24 diff --git a/docs/features/networkneighbors-collapsing.md b/docs/features/networkneighbors-collapsing.md index 5b2cb2cd8..edcbf5b75 100644 --- a/docs/features/networkneighbors-collapsing.md +++ b/docs/features/networkneighbors-collapsing.md @@ -8,7 +8,7 @@ The collapsing pass groups `NetworkNeighbor` entries that differ only by IP (sam Two new fields in the `CollapseConfiguration` CRD control this: - `NetworkIPGroupThreshold` (default 50): group size threshold above which IP collapsing is triggered. -- `NetworkCIDRFloorBits` (default 16): minimum CIDR prefix length — no emitted block is ever broader than `/`. +- `NetworkCIDRFloorBits` (default 24): minimum CIDR prefix length — no emitted block is ever broader than `/`. The pass is a **fixpoint**: running it twice on the same input is guaranteed to produce the same output, so collapsed entries remain stable across successive saves without oscillation or duplication. @@ -29,8 +29,8 @@ Collapsing reduces entry count by orders of magnitude for traffic aimed at cloud **Aggregation**: If a group's count of aggregatable IPv4 host addresses exceeds `NetworkIPGroupThreshold`: 1. Compute the smallest covering prefix (common leading bits across all hosts). -2. If that prefix's length is at least `NetworkCIDRFloorBits` (e.g., `/16` or longer), emit it as-is. -3. If it would be broader than `NetworkCIDRFloorBits`, split the group into floor-length buckets (e.g., `/16` buckets) and emit one CIDR entry per non-empty bucket. +2. If that prefix's length is at least `NetworkCIDRFloorBits` (e.g., `/24` or longer), emit it as-is. +3. If it would be broader than `NetworkCIDRFloorBits`, split the group into floor-length buckets (e.g., `/24` buckets) and emit one CIDR entry per non-empty bucket. **Output entries**: Each emitted CIDR entry carries: - The computed CIDR block(s) in `IPAddresses`. @@ -47,9 +47,9 @@ Collapsing reduces entry count by orders of magnitude for traffic aimed at cloud ## Scope / limitations -**Held-stable CIDRs do not retroactively re-narrow when floor is tightened**: If an operator later changes `NetworkCIDRFloorBits` from 16 to 24 (smaller blocks, higher precision), already-emitted `/16` blocks are held stable for idempotency and won't be split retroactively. They persist until that group naturally re-collapses (e.g., new IPs arrive, triggering re-aggregation). This is an intentional trade-off: predictable idempotency wins over floor freshness for held entries. +**Held-stable CIDRs do not retroactively re-narrow when floor is tightened**: If an operator later changes `NetworkCIDRFloorBits` from 24 to 28 (smaller blocks, higher precision), already-emitted `/24` blocks are held stable for idempotency and won't be split retroactively. They persist until that group naturally re-collapses (e.g., new IPs arrive, triggering re-aggregation). This is an intentional trade-off: predictable idempotency wins over floor freshness for held entries. -**New host IPs inside an already-held CIDR are not immediately absorbed**: If a `/16` block covers `192.168.0.0/16` and new traffic arrives to `192.168.5.100` (which falls inside that CIDR), the new IP persists as a separate entry until its own group independently exceeds the threshold. Entry-count creep is bounded by `NetworkIPGroupThreshold` and the existing merge logic, so this is not unbounded. +**New host IPs inside an already-held CIDR are not immediately absorbed**: If a `/24` block covers `192.168.0.0/24` and new traffic arrives to `192.168.0.200` (which falls inside that CIDR), the new IP persists as a separate entry until its own group independently exceeds the threshold. Entry-count creep is bounded by `NetworkIPGroupThreshold` and the existing merge logic, so this is not unbounded. **CIDR/`"*"` peers skip known-server enrichment**: A CIDR block is a range, not a single IP, so it cannot be looked up in the known-servers registry. CIDR and `"*"` entries produce bare `IPBlock` peers without the `PolicyRef` name/server enrichment that singular IPs enjoy. Bare-IP elements of the plural `IPAddresses` field retain full known-server matching identical to the singular-field path. @@ -69,16 +69,18 @@ spec: # IP collapsing thresholds (optional; omit or set to 0 for defaults) networkIPGroupThreshold: 50 # Collapse groups of 50+ hosts - networkCIDRFloorBits: 16 # No block narrower than /16 + networkCIDRFloorBits: 24 # No block narrower than /24 ``` -Zero or omitted values use the compiled-in defaults (50 and 16 respectively). No operator restart is required; the provider reads the singleton at each request. +Zero or omitted values use the compiled-in defaults (50 and 24 respectively). No operator restart is required; the provider reads the singleton at each request. + +The default floor was chosen to favor precision over maximum compaction: on the real-world 8,687-entry case that motivated this feature, a `/24` floor collapses it to ~341 entries (a `/16` floor would collapse to ~10, but at the cost of much broader, less precise CIDR blocks). Lower `networkCIDRFloorBits` (e.g. `16`) for more aggressive compaction if entry count matters more than block precision in your environment. ## Verifying **Entry count**: Compare entry counts before and after enabling the feature on a real profile with external traffic. A profile with 8,687 external-traffic entries should drop to a small number (typically hundreds or fewer CIDR entries, depending on traffic distribution). -**CIDR breadth**: Inspect emitted `NetworkNeighbor` entries; no single `IPAddresses` CIDR block should exceed the configured floor (default `/16`). +**CIDR breadth**: Inspect emitted `NetworkNeighbor` entries; no single `IPAddresses` CIDR block should exceed the configured floor (default `/24`). **Idempotency**: Run the collapse pass twice in succession (e.g., two sequential saves) on the same profile and assert the second pass's output is byte-identical to the first (fixpoint). diff --git a/pkg/registry/file/dynamicpathdetector/types.go b/pkg/registry/file/dynamicpathdetector/types.go index 87be18a49..40e728777 100644 --- a/pkg/registry/file/dynamicpathdetector/types.go +++ b/pkg/registry/file/dynamicpathdetector/types.go @@ -29,7 +29,7 @@ const ( OpenDynamicThreshold = 50 EndpointDynamicThreshold = 100 NetworkIPGroupThreshold = 50 - NetworkCIDRFloorBits = 16 + NetworkCIDRFloorBits = 24 ) // --- Collapse configuration --- From 183f910a05d7e649f63f1207ae90604cf9c6dea3 Mon Sep 17 00:00:00 2001 From: entlein Date: Thu, 23 Jul 2026 19:54:17 +0200 Subject: [PATCH 5/7] first implementation, not optimized Signed-off-by: entlein --- .../file/networkneighborhood_ipcollapse.go | 62 ++++++++++++++++++- .../networkneighborhood_ipcollapse_test.go | 49 +++++++++++++++ 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/pkg/registry/file/networkneighborhood_ipcollapse.go b/pkg/registry/file/networkneighborhood_ipcollapse.go index 347822d4f..8b67ad770 100644 --- a/pkg/registry/file/networkneighborhood_ipcollapse.go +++ b/pkg/registry/file/networkneighborhood_ipcollapse.go @@ -79,8 +79,12 @@ func collapseIPGroups(entries []softwarecomposition.NetworkNeighbor, settings dy } cidrs := aggregateHosts(hosts, floorBits) - values := append(cidrs, passthrough...) - sort.Strings(values) + // Merge the freshly aggregated CIDR(s) with the group's already-collapsed + // pass-through CIDRs into a minimal set. Without this, incremental + // learning re-collapses newly observed hosts to a CIDR that duplicates or + // nests inside a block already held from an earlier save, producing garbage + // like [52.216.0.0/26, 52.216.0.0/26, 52.216.0.0/27]. + values := minimizeCIDRs(append(cidrs, passthrough...)) var dnsNames []string var ports []softwarecomposition.NetworkPort @@ -172,6 +176,60 @@ func aggregateHosts(hosts []netip.Addr, floorBits int) []string { return out } +// minimizeCIDRs reduces a set of address values to the smallest equivalent set: +// CIDR values are deduplicated and any prefix wholly contained in a broader +// prefix of the set is dropped. This keeps incremental re-collapsing a fixpoint +// on the CIDR set — freshly aggregated blocks that duplicate or nest inside a +// group's already-collapsed pass-through CIDRs are absorbed rather than +// accumulated. Non-CIDR values (the "*" sentinel, IPv6, unparseable) are held +// verbatim and deduplicated. The result is sorted. +func minimizeCIDRs(values []string) []string { + seenPfx := map[string]struct{}{} + seenOther := map[string]struct{}{} + var prefixes []netip.Prefix + var others []string + for _, v := range values { + if p, err := netip.ParsePrefix(v); err == nil { + m := p.Masked() + key := m.String() + if _, ok := seenPfx[key]; ok { + continue + } + seenPfx[key] = struct{}{} + prefixes = append(prefixes, m) + continue + } + if _, ok := seenOther[v]; ok { + continue + } + seenOther[v] = struct{}{} + others = append(others, v) + } + + out := make([]string, 0, len(prefixes)+len(others)) + for i, a := range prefixes { + contained := false + for j, b := range prefixes { + if i == j { + continue + } + // A broader prefix (fewer bits) that covers a's network address + // subsumes a; drop a. Equal-width prefixes never subsume each other + // and identical prefixes are already deduplicated above. + if b.Bits() < a.Bits() && b.Contains(a.Addr()) { + contained = true + break + } + } + if !contained { + out = append(out, a.String()) + } + } + out = append(out, others...) + sort.Strings(out) + return out +} + // commonPrefixLen returns the number of leading bits shared by every address in // the set. All addresses are assumed to be IPv4. func commonPrefixLen(addrs []netip.Addr) int { diff --git a/pkg/registry/file/networkneighborhood_ipcollapse_test.go b/pkg/registry/file/networkneighborhood_ipcollapse_test.go index beefdcda5..9be155558 100644 --- a/pkg/registry/file/networkneighborhood_ipcollapse_test.go +++ b/pkg/registry/file/networkneighborhood_ipcollapse_test.go @@ -256,3 +256,52 @@ func TestCollapseIPGroups_IPv6PassThrough(t *testing.T) { func TestCollapseIPGroups_NilInput(t *testing.T) { assert.Nil(t, collapseIPGroups(nil, testSettings())) } + +func TestCollapseIPGroups_IncrementalReCollapseDeduplicatesAndAbsorbs(t *testing.T) { + // Regression for the incremental-learning garbage [/26, /26, /27]: a group + // that already holds collapsed CIDRs from earlier saves (a /27 and a /26) + // plus freshly observed hosts that re-aggregate to 52.216.0.0/26 must + // converge to exactly one 52.216.0.0/26 — the duplicate /26 deduplicated and + // the nested /27 absorbed — instead of accumulating all three entries. + settings := dynamicpathdetector.CollapseSettings{ + NetworkIPGroupThreshold: 5, + NetworkCIDRFloorBits: 16, + } + cidr := func(c string) softwarecomposition.NetworkNeighbor { + return softwarecomposition.NetworkNeighbor{ + Type: softwarecomposition.CommunicationTypeEgress, + DNS: "example.com", + IPAddresses: []string{c}, + } + } + in := []softwarecomposition.NetworkNeighbor{ + cidr("52.216.0.0/27"), + cidr("52.216.0.0/26"), + } + for _, h := range []string{"52.216.0.1", "52.216.0.10", "52.216.0.20", "52.216.0.40", "52.216.0.55", "52.216.0.60"} { + in = append(in, hostNeighbor(h)) + } + + out := collapseIPGroups(in, settings) + + var cidrs []string + for _, e := range out { + cidrs = append(cidrs, e.IPAddresses...) + assert.Empty(t, e.IPAddress) + } + assert.Equal(t, []string{"52.216.0.0/26"}, cidrs, "must converge to a single covering /26, not [/26 /26 /27]") +} + +func TestMinimizeCIDRs_DropsDuplicatesAndSubsumed(t *testing.T) { + // exact duplicate + nested prefix + an unrelated block + non-CIDR sentinel + got := minimizeCIDRs([]string{ + "52.216.0.0/26", "52.216.0.0/27", "52.216.0.0/26", + "10.0.0.0/24", "*", + }) + assert.Equal(t, []string{"*", "10.0.0.0/24", "52.216.0.0/26"}, got) +} + +func TestMinimizeCIDRs_KeepsDisjointEqualWidth(t *testing.T) { + got := minimizeCIDRs([]string{"10.1.0.0/24", "10.2.0.0/24"}) + assert.Equal(t, []string{"10.1.0.0/24", "10.2.0.0/24"}, got) +} From c20f4371ba5bbca9b83207b0166cc6dc019b0eac Mon Sep 17 00:00:00 2001 From: entlein Date: Thu, 23 Jul 2026 20:44:54 +0200 Subject: [PATCH 6/7] grouping CIDRs to avoid dedup Signed-off-by: entlein --- go.mod | 2 +- .../file/containerprofile_processor_test.go | 4 +- .../file/dynamicpathdetector/types.go | 10 + .../file/networkneighborhood_ipcollapse.go | 197 ++++++++---------- ...tworkneighborhood_ipcollapse_bench_test.go | 63 ++++++ .../networkneighborhood_ipcollapse_test.go | 126 +++++++---- .../networkneighborhood_processor_test.go | 4 +- 7 files changed, 255 insertions(+), 151 deletions(-) create mode 100644 pkg/registry/file/networkneighborhood_ipcollapse_bench_test.go diff --git a/go.mod b/go.mod index 46719c0ba..af8e3606a 100644 --- a/go.mod +++ b/go.mod @@ -31,6 +31,7 @@ require ( go.uber.org/goleak v1.3.0 go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.1 + go4.org/netipx v0.0.0-20231129151722-fdeea329fbba golang.org/x/sync v0.20.0 golang.org/x/text v0.37.0 k8s.io/api v0.35.0 @@ -191,7 +192,6 @@ require ( go.opentelemetry.io/proto/otlp v1.10.0 // indirect go.yaml.in/yaml/v2 v2.4.3 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - go4.org/netipx v0.0.0-20231129151722-fdeea329fbba // indirect golang.org/x/crypto v0.52.0 // indirect golang.org/x/exp v0.0.0-20251023183803-a4bb9ffd2546 // indirect golang.org/x/mod v0.35.0 // indirect diff --git a/pkg/registry/file/containerprofile_processor_test.go b/pkg/registry/file/containerprofile_processor_test.go index bacfb1629..af9fe4199 100644 --- a/pkg/registry/file/containerprofile_processor_test.go +++ b/pkg/registry/file/containerprofile_processor_test.go @@ -26,10 +26,10 @@ import ( ) func TestDeflateContainerProfileSpec_NetworkNeighborsCollapse(t *testing.T) { - const hostCount = 60 + const hostCount = 64 // a fully-observed /26 (10.0.0.0..10.0.0.63) newIngress := func() []softwarecomposition.NetworkNeighbor { ingress := make([]softwarecomposition.NetworkNeighbor, 0, hostCount) - for i := 1; i <= hostCount; i++ { + for i := 0; i < hostCount; i++ { ingress = append(ingress, softwarecomposition.NetworkNeighbor{ Identifier: fmt.Sprintf("external-%d", i), Type: "external", diff --git a/pkg/registry/file/dynamicpathdetector/types.go b/pkg/registry/file/dynamicpathdetector/types.go index 40e728777..7489b943c 100644 --- a/pkg/registry/file/dynamicpathdetector/types.go +++ b/pkg/registry/file/dynamicpathdetector/types.go @@ -25,11 +25,21 @@ const ( // NetworkNeighbor entries differing only by IP gets CIDR-collapsed. // NetworkCIDRFloorBits is the minimum CIDR prefix length (maximum breadth) // a single aggregated block may have. +// NetworkMaxCIDRSplitBits caps, PER prefix, how far a single cover block broader +// than the floor is split into floor-width children: up to 2^NetworkMaxCIDRSplitBits +// blocks (4096 here). A prefix whose split would exceed that is kept as-is rather +// than exploding the entry list. This bounds ONE block's fan-out (not the sum +// across a group — the whole neighborhood is separately capped by +// MaxNetworkNeighborhoodSize); it only bites when a held pass-through block is +// much broader than a tightened floor (e.g. a /16 under a /28 floor -> 4096 +// children; a /16 under a /24 floor is only 256). Not currently exposed as a +// CollapseConfiguration field. const ( OpenDynamicThreshold = 50 EndpointDynamicThreshold = 100 NetworkIPGroupThreshold = 50 NetworkCIDRFloorBits = 24 + NetworkMaxCIDRSplitBits = 12 ) // --- Collapse configuration --- diff --git a/pkg/registry/file/networkneighborhood_ipcollapse.go b/pkg/registry/file/networkneighborhood_ipcollapse.go index 8b67ad770..2dd5a02f3 100644 --- a/pkg/registry/file/networkneighborhood_ipcollapse.go +++ b/pkg/registry/file/networkneighborhood_ipcollapse.go @@ -9,6 +9,7 @@ import ( "github.com/kubescape/storage/pkg/apis/softwarecomposition" "github.com/kubescape/storage/pkg/registry/file/dynamicpathdetector" + "go4.org/netipx" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -18,14 +19,15 @@ const ipCollapseFieldSep = "\x00" // into a small number of CIDR-bearing entries. Entries are grouped by // (Type, DNS, NamespaceSelector, PodSelector); within a group whose count of // aggregatable IPv4 host addresses exceeds settings.NetworkIPGroupThreshold, -// those hosts are replaced by covering CIDR block(s) no broader than -// settings.NetworkCIDRFloorBits. +// those hosts plus any already-collapsed pass-through CIDRs are replaced by the +// minimal EXACT CIDR cover of exactly those addresses (see coverPrefixes), with +// no block broader than settings.NetworkCIDRFloorBits. The cover never +// over-approximates to a block the workload did not actually reach. // -// The pass is a fixpoint (AC10): already-collapsed CIDR values and the "*" -// sentinel / IPv6 values are treated as pass-through and are never re-parsed as -// host IPs or re-tightened, and collapsed output carries a deterministic -// Identifier, so a second run — whose groups now hold only CIDRs and thus have -// zero aggregatable hosts — leaves everything untouched. +// The pass is a fixpoint: an exact cover re-covered is itself, and the "*" +// sentinel / bare IPv6 values are pass-through held verbatim, so a second run — +// whose groups now hold only CIDRs and thus have zero aggregatable hosts — +// leaves everything untouched. func collapseIPGroups(entries []softwarecomposition.NetworkNeighbor, settings dynamicpathdetector.CollapseSettings) []softwarecomposition.NetworkNeighbor { if entries == nil { return nil @@ -78,13 +80,25 @@ func collapseIPGroups(entries []softwarecomposition.NetworkNeighbor, settings dy continue } - cidrs := aggregateHosts(hosts, floorBits) - // Merge the freshly aggregated CIDR(s) with the group's already-collapsed - // pass-through CIDRs into a minimal set. Without this, incremental - // learning re-collapses newly observed hosts to a CIDR that duplicates or - // nests inside a block already held from an earlier save, producing garbage - // like [52.216.0.0/26, 52.216.0.0/26, 52.216.0.0/27]. - values := minimizeCIDRs(append(cidrs, passthrough...)) + // Split pass-through into already-collapsed CIDRs (folded into the cover) + // and non-CIDR sentinels ("*", bare IPv6, unparseable) held verbatim. + var cidrPass []netip.Prefix + var sentinels []string + for _, v := range passthrough { + if p, err := netip.ParsePrefix(v); err == nil { + cidrPass = append(cidrPass, p.Masked()) + } else { + sentinels = append(sentinels, v) + } + } + + // Exact minimal CIDR cover of the hosts plus already-held CIDRs, capped at + // the floor. Because it is an exact cover, incremental re-collapsing is a + // fixpoint and never accumulates duplicate or nested blocks — the bug that + // produced [52.216.0.0/26, 52.216.0.0/26, 52.216.0.0/27]. + values := coverPrefixes(hosts, cidrPass, floorBits) + values = append(values, sentinels...) + sort.Strings(values) var dnsNames []string var ports []softwarecomposition.NetworkPort @@ -112,12 +126,13 @@ func collapseIPGroups(entries []softwarecomposition.NetworkNeighbor, settings dy return out } -// classifyGroupAddresses splits a group's address values into aggregatable IPv4 -// host addresses (deduped) and pass-through values held verbatim. An entry's -// value comes from the singular IPAddress when set, otherwise from each element -// of IPAddresses. CIDRs, the "*" sentinel, IPv6 and unparseable values are -// pass-through and are never fed to aggregation, which is what makes the pass a -// fixpoint on already-collapsed input. +// classifyGroupAddresses splits a group's address values into aggregatable host +// addresses (bare IPv4 or IPv6, deduped) and pass-through values held verbatim. +// An entry's value comes from the singular IPAddress when set, otherwise from +// each element of IPAddresses. The "*" sentinel and unparseable values are +// pass-through; already-collapsed CIDRs are pass-through here but the caller +// folds them back into the exact cover. Both address families are aggregated — +// netipx covers IPv4 and IPv6 alike. func classifyGroupAddresses(entries []softwarecomposition.NetworkNeighbor) ([]netip.Addr, []string) { seenHost := map[netip.Addr]struct{}{} seenPass := map[string]struct{}{} @@ -128,7 +143,7 @@ func classifyGroupAddresses(entries []softwarecomposition.NetworkNeighbor) ([]ne if v == "" { return } - if addr, err := netip.ParseAddr(v); err == nil && addr.Is4() { + if addr, err := netip.ParseAddr(v); err == nil { if _, ok := seenHost[addr]; !ok { seenHost[addr] = struct{}{} hosts = append(hosts, addr) @@ -153,110 +168,72 @@ func classifyGroupAddresses(entries []softwarecomposition.NetworkNeighbor) ([]ne return hosts, passthrough } -// aggregateHosts returns the CIDR block(s) covering the given IPv4 hosts. If the -// hosts share a common prefix at least as long as floorBits it is emitted as a -// single block; otherwise each host is bucketed into a floorBits-length prefix -// so no emitted block is ever broader than the floor. -func aggregateHosts(hosts []netip.Addr, floorBits int) []string { - if len(hosts) == 0 { +// coverPrefixes returns the minimal set of CIDR strings that covers EXACTLY the +// given IPv4 host addresses together with the group's already-collapsed +// pass-through CIDRs, capped so no prefix is broader than floorBits. netipx does +// the aggregation — deduplicating, dropping subsumed prefixes and merging +// adjacent siblings into the minimal exact cover in near-linear time — and any +// resulting prefix broader than the floor is then split into floorBits-wide +// children (all fully covered, since the parent lies wholly within the set). +// +// Because the cover is exact, it never over-approximates to a block the workload +// did not actually reach, and re-running on already-collapsed input is a +// fixpoint: no duplicate or nested blocks can accumulate across incremental +// saves. The result is sorted. +func coverPrefixes(hosts []netip.Addr, cidrPass []netip.Prefix, floorBits int) []string { + if len(hosts) == 0 && len(cidrPass) == 0 { return nil } - if commonLen := commonPrefixLen(hosts); commonLen >= floorBits { - return []string{netip.PrefixFrom(hosts[0], commonLen).Masked().String()} + var b netipx.IPSetBuilder + for _, h := range hosts { + b.Add(h) } - seen := map[string]struct{}{} - var out []string - for _, addr := range hosts { - cidr := netip.PrefixFrom(addr, floorBits).Masked().String() - if _, ok := seen[cidr]; !ok { - seen[cidr] = struct{}{} - out = append(out, cidr) - } + for _, p := range cidrPass { + b.AddPrefix(p) } - return out -} - -// minimizeCIDRs reduces a set of address values to the smallest equivalent set: -// CIDR values are deduplicated and any prefix wholly contained in a broader -// prefix of the set is dropped. This keeps incremental re-collapsing a fixpoint -// on the CIDR set — freshly aggregated blocks that duplicate or nest inside a -// group's already-collapsed pass-through CIDRs are absorbed rather than -// accumulated. Non-CIDR values (the "*" sentinel, IPv6, unparseable) are held -// verbatim and deduplicated. The result is sorted. -func minimizeCIDRs(values []string) []string { - seenPfx := map[string]struct{}{} - seenOther := map[string]struct{}{} - var prefixes []netip.Prefix - var others []string - for _, v := range values { - if p, err := netip.ParsePrefix(v); err == nil { - m := p.Masked() - key := m.String() - if _, ok := seenPfx[key]; ok { - continue - } - seenPfx[key] = struct{}{} - prefixes = append(prefixes, m) - continue - } - if _, ok := seenOther[v]; ok { - continue - } - seenOther[v] = struct{}{} - others = append(others, v) + set, err := b.IPSet() + if err != nil || set == nil { + return nil } - out := make([]string, 0, len(prefixes)+len(others)) - for i, a := range prefixes { - contained := false - for j, b := range prefixes { - if i == j { - continue - } - // A broader prefix (fewer bits) that covers a's network address - // subsumes a; drop a. Equal-width prefixes never subsume each other - // and identical prefixes are already deduplicated above. - if b.Bits() < a.Bits() && b.Contains(a.Addr()) { - contained = true - break - } - } - if !contained { - out = append(out, a.String()) + var out []string + for _, p := range set.Prefixes() { + // The floor is an IPv4 breadth cap; IPv6 covers are emitted as-is (a /24 + // floor is meaningless for v6, whose covers are already narrow). + if p.Addr().Is4() && p.Bits() < floorBits { + out = append(out, splitToFloor(p, floorBits)...) + continue } + out = append(out, p.String()) } - out = append(out, others...) sort.Strings(out) return out } -// commonPrefixLen returns the number of leading bits shared by every address in -// the set. All addresses are assumed to be IPv4. -func commonPrefixLen(addrs []netip.Addr) int { - base := addrs[0].As4() - common := 32 - for _, addr := range addrs[1:] { - b := addr.As4() - n := 0 - for i := 0; i < 4 && n < common; i++ { - x := base[i] ^ b[i] - if x == 0 { - n += 8 - continue - } - for bit := 7; bit >= 0; bit-- { - if x&(1< dynamicpathdetector.NetworkMaxCIDRSplitBits { + return []string{p.String()} + } + count := 1 << shift + out := make([]string, 0, count) + child := netip.PrefixFrom(p.Addr(), floorBits).Masked() + for i := 0; i < count; i++ { + out = append(out, child.String()) + next := netipx.RangeOfPrefix(child).To().Next() + if !next.IsValid() { break } - if n < common { - common = n - } + child = netip.PrefixFrom(next, floorBits).Masked() } - return common + return out } func neighborGroupKey(n softwarecomposition.NetworkNeighbor) string { diff --git a/pkg/registry/file/networkneighborhood_ipcollapse_bench_test.go b/pkg/registry/file/networkneighborhood_ipcollapse_bench_test.go new file mode 100644 index 000000000..f6dfdeaf2 --- /dev/null +++ b/pkg/registry/file/networkneighborhood_ipcollapse_bench_test.go @@ -0,0 +1,63 @@ +package file + +import ( + "fmt" + "net/netip" + "testing" + + "github.com/kubescape/storage/pkg/apis/softwarecomposition" + "github.com/kubescape/storage/pkg/registry/file/dynamicpathdetector" +) + +func cidrNeighbor(c string) softwarecomposition.NetworkNeighbor { + return softwarecomposition.NetworkNeighbor{ + Type: softwarecomposition.CommunicationTypeEgress, + DNS: "example.com", + IPAddresses: []string{c}, + } +} + +// BenchmarkCollapseIPGroups measures CPU/allocations of the full deflate path on +// a realistic incremental-learning snapshot: 200 contiguous hosts in a /24 plus +// two already-collapsed pass-through CIDRs from earlier saves, at a /16 and a +// /24 floor (the latter exercising the floor-cap split). +func BenchmarkCollapseIPGroups(b *testing.B) { + var in []softwarecomposition.NetworkNeighbor + for i := 0; i < 200; i++ { + in = append(in, hostNeighbor(fmt.Sprintf("52.216.%d.%d", i/256, i%256))) + } + in = append(in, cidrNeighbor("52.216.4.0/24"), cidrNeighbor("52.216.0.0/16")) + + for _, floor := range []int{16, 24} { + settings := dynamicpathdetector.CollapseSettings{NetworkIPGroupThreshold: 5, NetworkCIDRFloorBits: floor} + b.Run(fmt.Sprintf("floor%d", floor), func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = collapseIPGroups(in, settings) + } + }) + } +} + +// BenchmarkCoverPrefixes isolates the netipx exact-cover step: 256 scattered +// hosts across a /16 plus two pass-through CIDRs, at a /16 and a /24 floor. +func BenchmarkCoverPrefixes(b *testing.B) { + hosts := make([]netip.Addr, 0, 256) + for i := 0; i < 256; i++ { + hosts = append(hosts, netip.AddrFrom4([4]byte{52, 216, byte(i), byte((i * 7) % 256)})) + } + cidrPass := []netip.Prefix{ + netip.MustParsePrefix("52.216.4.0/24"), + netip.MustParsePrefix("52.216.128.0/17"), + } + for _, floor := range []int{16, 24} { + b.Run(fmt.Sprintf("floor%d", floor), func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = coverPrefixes(hosts, cidrPass, floor) + } + }) + } +} diff --git a/pkg/registry/file/networkneighborhood_ipcollapse_test.go b/pkg/registry/file/networkneighborhood_ipcollapse_test.go index 9be155558..90cc1ae74 100644 --- a/pkg/registry/file/networkneighborhood_ipcollapse_test.go +++ b/pkg/registry/file/networkneighborhood_ipcollapse_test.go @@ -44,23 +44,23 @@ func TestCollapseIPGroups_BelowThresholdUntouched(t *testing.T) { } func TestCollapseIPGroups_AboveThresholdSingleCoveringCIDR(t *testing.T) { - // 60 hosts spread across the full third octet of 10.1.0.0/16 (0..236, - // spanning the top bit) -> common prefix exactly the floor (16) -> one block. + // A fully-observed /24 (all 256 hosts) exact-covers to exactly one /24 block. var in []softwarecomposition.NetworkNeighbor - for i := 0; i < 60; i++ { - in = append(in, hostNeighbor(fmt.Sprintf("10.1.%d.0", i*4))) + for i := 0; i < 256; i++ { + in = append(in, hostNeighbor(fmt.Sprintf("10.1.5.%d", i))) } out := collapseIPGroups(in, testSettings()) require.Len(t, out, 1) - assert.Equal(t, []string{"10.1.0.0/16"}, out[0].IPAddresses) + assert.Equal(t, []string{"10.1.5.0/24"}, out[0].IPAddresses) assert.Empty(t, out[0].IPAddress) } -func TestCollapseIPGroups_AboveThresholdBroaderThanFloorBuckets(t *testing.T) { - // 60 hosts spread across many /16s -> common prefix broader than floor -> - // floor-bucket into distinct /16 blocks, none broader than the floor. +func TestCollapseIPGroups_ScatteredHostsStayGranularAndCappedAtFloor(t *testing.T) { + // 60 lone hosts, each in its own /16 -> exact cover keeps them granular (one + // /32 apiece, since none are adjacent); no emitted block is broader than the + // floor, and the cover never over-approximates to a covering block. var in []softwarecomposition.NetworkNeighbor for i := 0; i < 60; i++ { in = append(in, hostNeighbor(fmt.Sprintf("%d.%d.0.1", 10+i, i))) @@ -110,12 +110,12 @@ func TestCollapseIPGroups_DifferentSelectorsNotMerged(t *testing.T) { return &metav1.LabelSelector{MatchLabels: map[string]string{"app": v}} } var in []softwarecomposition.NetworkNeighbor - for i := 0; i < 60; i++ { + for i := 0; i < 64; i++ { // a full /26 per selector -> one exact block each e := hostNeighbor(fmt.Sprintf("10.3.0.%d", i)) e.PodSelector = sel("a") in = append(in, e) } - for i := 0; i < 60; i++ { + for i := 0; i < 64; i++ { e := hostNeighbor(fmt.Sprintf("10.3.0.%d", i)) e.PodSelector = sel("b") in = append(in, e) @@ -135,17 +135,18 @@ func TestCollapseIPGroups_DifferentSelectorsNotMerged(t *testing.T) { func TestCollapseIPGroups_RealWorldShapeOrdersOfMagnitude(t *testing.T) { var in []softwarecomposition.NetworkNeighbor - // ~500 IPs clustered in 100.68.x.x - for i := 0; i < 250; i++ { - in = append(in, hostNeighbor(fmt.Sprintf("100.68.%d.%d", i/256, i%256))) + // 256 IPs fully covering 100.68.0.0/24 + for i := 0; i < 256; i++ { + in = append(in, hostNeighbor(fmt.Sprintf("100.68.0.%d", i))) } - // ~250 IPs clustered in 16.15.183.x plus neighboring /24s - for i := 0; i < 250; i++ { - in = append(in, hostNeighbor(fmt.Sprintf("16.15.%d.%d", 180+i/256, i%256))) + // 256 IPs fully covering 16.15.180.0/24 + for i := 0; i < 256; i++ { + in = append(in, hostNeighbor(fmt.Sprintf("16.15.180.%d", i))) } out := collapseIPGroups(in, testSettings()) + // 512 contiguous hosts exact-cover to a handful of blocks (two /24s here). assert.Less(t, len(out), 10) assert.Less(t, len(out), len(in)/50) for _, e := range out { @@ -175,7 +176,7 @@ func TestCollapseIPGroups_Idempotent(t *testing.T) { DNS: "example.com", IPAddresses: []string{"*"}, }) - // IPv6 entry + // IPv6 entry — a lone v6 host exact-covers to its /128 in = append(in, softwarecomposition.NetworkNeighbor{ Type: softwarecomposition.CommunicationTypeEgress, DNS: "example.com", @@ -187,14 +188,14 @@ func TestCollapseIPGroups_Idempotent(t *testing.T) { assert.Equal(t, once, twice, "collapseIPGroups must be a fixpoint") - // pass-through values survived + // pass-through + covered values survived var values []string for _, e := range once { values = append(values, e.IPAddresses...) } assert.Contains(t, values, "*") assert.Contains(t, values, "200.0.0.0/16") - assert.Contains(t, values, "2001:db8::1") + assert.Contains(t, values, "2001:db8::1/128") } func TestCollapseIPGroups_FieldContract(t *testing.T) { @@ -233,16 +234,17 @@ func TestCollapseIPGroups_MultiBucketReplicatesDNSNamesAndPorts(t *testing.T) { } } -func TestCollapseIPGroups_IPv6PassThrough(t *testing.T) { +func TestCollapseIPGroups_IPv6Aggregated(t *testing.T) { + // A full IPv6 /120 (256 contiguous v6 hosts) exact-covers to that /120, and a + // lone v6 host to its /128 — alongside a v4 group, in one mixed-family pass. var in []softwarecomposition.NetworkNeighbor + for i := 0; i < 256; i++ { + in = append(in, hostNeighbor(fmt.Sprintf("2606:4700:0:1::%x", i))) + } + in = append(in, hostNeighbor("2001:db8::42")) for i := 0; i < 60; i++ { in = append(in, hostNeighbor(fmt.Sprintf("10.5.0.%d", i))) } - in = append(in, softwarecomposition.NetworkNeighbor{ - Type: softwarecomposition.CommunicationTypeEgress, - DNS: "example.com", - IPAddress: "2001:db8::42", - }) out := collapseIPGroups(in, testSettings()) @@ -250,7 +252,45 @@ func TestCollapseIPGroups_IPv6PassThrough(t *testing.T) { for _, e := range out { values = append(values, e.IPAddresses...) } - assert.Contains(t, values, "2001:db8::42") + assert.Contains(t, values, "2606:4700:0:1::/120", "contiguous v6 hosts aggregate") + assert.Contains(t, values, "2001:db8::42/128", "lone v6 host covers to /128") +} + +func TestCoverPrefixes_IPv6ExactAndMerge(t *testing.T) { + // Two adjacent v6 /33 halves merge into the parent /32 (Cloudflare 2606:4700::/32), + // independent of any IPv4 floor. + got := coverPrefixes(nil, []netip.Prefix{ + netip.MustParsePrefix("2606:4700::/33"), + netip.MustParsePrefix("2606:4700:8000::/33"), + }, 24) + assert.Equal(t, []string{"2606:4700::/32"}, got) +} + +// TestCoverPrefixes_RealCloudRangesDedupAndMerge feeds netipx the kind of messy, +// overlapping, non-aggregated CIDR lists cloud providers publish (a subsumed +// range, two adjacent siblings that merge, and disjoint blocks across families) +// and asserts the minimal exact cover. +func TestCoverPrefixes_RealCloudRangesDedupAndMerge(t *testing.T) { + pass := []netip.Prefix{ + // AWS S3 us-east-1: 52.216.0.0/15 subsumes the more specific 52.216.4.0/24 + netip.MustParsePrefix("52.216.0.0/15"), + netip.MustParsePrefix("52.216.4.0/24"), + // Cloudflare: 104.16.0.0/13 subsumes 104.16.0.0/14 + netip.MustParsePrefix("104.16.0.0/13"), + netip.MustParsePrefix("104.16.0.0/14"), + // Cloudflare v6 siblings that merge to a /31 + netip.MustParsePrefix("2606:4700::/32"), + netip.MustParsePrefix("2606:4701::/32"), + } + // Permissive floor (/8) so the cap does not split these broad blocks — this + // isolates the dedup/merge behavior (the floor cap has its own test). + got := coverPrefixes(nil, pass, 8) + // sorted lexicographically (the collapse output order) + assert.Equal(t, []string{ + "104.16.0.0/13", + "2606:4700::/31", + "52.216.0.0/15", + }, got) } func TestCollapseIPGroups_NilInput(t *testing.T) { @@ -292,16 +332,30 @@ func TestCollapseIPGroups_IncrementalReCollapseDeduplicatesAndAbsorbs(t *testing assert.Equal(t, []string{"52.216.0.0/26"}, cidrs, "must converge to a single covering /26, not [/26 /26 /27]") } -func TestMinimizeCIDRs_DropsDuplicatesAndSubsumed(t *testing.T) { - // exact duplicate + nested prefix + an unrelated block + non-CIDR sentinel - got := minimizeCIDRs([]string{ - "52.216.0.0/26", "52.216.0.0/27", "52.216.0.0/26", - "10.0.0.0/24", "*", - }) - assert.Equal(t, []string{"*", "10.0.0.0/24", "52.216.0.0/26"}, got) +func TestCoverPrefixes_ExactNoOverApproximation(t *testing.T) { + // Three non-adjacent hosts cover exactly {.1,.2,.3} — never a single /30 + // (which would admit the unobserved .0). + hosts := []netip.Addr{ + netip.MustParseAddr("52.216.0.1"), + netip.MustParseAddr("52.216.0.2"), + netip.MustParseAddr("52.216.0.3"), + } + got := coverPrefixes(hosts, nil, 16) + assert.Equal(t, []string{"52.216.0.1/32", "52.216.0.2/31"}, got) +} + +func TestCoverPrefixes_MergesAdjacentSiblings(t *testing.T) { + // The two /25 halves of a /24 merge into the single parent /24. + got := coverPrefixes(nil, []netip.Prefix{ + netip.MustParsePrefix("10.0.0.0/25"), + netip.MustParsePrefix("10.0.0.128/25"), + }, 16) + assert.Equal(t, []string{"10.0.0.0/24"}, got) } -func TestMinimizeCIDRs_KeepsDisjointEqualWidth(t *testing.T) { - got := minimizeCIDRs([]string{"10.1.0.0/24", "10.2.0.0/24"}) - assert.Equal(t, []string{"10.1.0.0/24", "10.2.0.0/24"}, got) +func TestCoverPrefixes_FloorCapSplitsBroadBlock(t *testing.T) { + // A pass-through /22 under a /24 floor splits into its four /24 children; + // none is broader than the floor. + got := coverPrefixes(nil, []netip.Prefix{netip.MustParsePrefix("10.9.0.0/22")}, 24) + assert.Equal(t, []string{"10.9.0.0/24", "10.9.1.0/24", "10.9.2.0/24", "10.9.3.0/24"}, got) } diff --git a/pkg/registry/file/networkneighborhood_processor_test.go b/pkg/registry/file/networkneighborhood_processor_test.go index 67d8eba77..6d5e0fc7c 100644 --- a/pkg/registry/file/networkneighborhood_processor_test.go +++ b/pkg/registry/file/networkneighborhood_processor_test.go @@ -133,9 +133,9 @@ func TestNetworkNeighborhoodProcessor_PreSave(t *testing.T) { } func TestNetworkNeighborhoodProcessor_PreSave_IPCollapse(t *testing.T) { - const hostCount = 60 + const hostCount = 64 // a fully-observed /26 (10.0.0.0..10.0.0.63) ingress := make([]softwarecomposition.NetworkNeighbor, 0, hostCount) - for i := 1; i <= hostCount; i++ { + for i := 0; i < hostCount; i++ { ingress = append(ingress, softwarecomposition.NetworkNeighbor{ Identifier: fmt.Sprintf("external-%d", i), Type: "external", From ac8721e489f5f9f5edf77b65924dbd144f4d9760 Mon Sep 17 00:00:00 2001 From: entlein Date: Fri, 24 Jul 2026 12:32:14 +0200 Subject: [PATCH 7/7] rewrote to address review, lets retest Signed-off-by: entlein --- .../file/networkneighborhood_ipcollapse.go | 135 ++++++++++++++---- .../networkneighborhood_ipcollapse_test.go | 51 ++++--- 2 files changed, 138 insertions(+), 48 deletions(-) diff --git a/pkg/registry/file/networkneighborhood_ipcollapse.go b/pkg/registry/file/networkneighborhood_ipcollapse.go index 2dd5a02f3..8a21bd1c0 100644 --- a/pkg/registry/file/networkneighborhood_ipcollapse.go +++ b/pkg/registry/file/networkneighborhood_ipcollapse.go @@ -19,15 +19,16 @@ const ipCollapseFieldSep = "\x00" // into a small number of CIDR-bearing entries. Entries are grouped by // (Type, DNS, NamespaceSelector, PodSelector); within a group whose count of // aggregatable IPv4 host addresses exceeds settings.NetworkIPGroupThreshold, -// those hosts plus any already-collapsed pass-through CIDRs are replaced by the -// minimal EXACT CIDR cover of exactly those addresses (see coverPrefixes), with -// no block broader than settings.NetworkCIDRFloorBits. The cover never -// over-approximates to a block the workload did not actually reach. +// those hosts plus any already-collapsed pass-through CIDRs are replaced by +// covering CIDR blocks no broader than settings.NetworkCIDRFloorBits (see +// coverPrefixes). Output size is bounded by the number of distinct floor-length +// networks the workload actually reached, not by the host count: scattered hosts +// collapse to one block per floor network rather than one /32 apiece. // -// The pass is a fixpoint: an exact cover re-covered is itself, and the "*" -// sentinel / bare IPv6 values are pass-through held verbatim, so a second run — -// whose groups now hold only CIDRs and thus have zero aggregatable hosts — -// leaves everything untouched. +// The pass is a fixpoint: collapsed blocks re-fed through the same aggregation +// converge to themselves, and the "*" sentinel / bare IPv6 values are +// pass-through held verbatim, so a second run — whose groups now hold only CIDRs +// and thus have zero aggregatable hosts — leaves everything untouched. func collapseIPGroups(entries []softwarecomposition.NetworkNeighbor, settings dynamicpathdetector.CollapseSettings) []softwarecomposition.NetworkNeighbor { if entries == nil { return nil @@ -126,13 +127,15 @@ func collapseIPGroups(entries []softwarecomposition.NetworkNeighbor, settings dy return out } -// classifyGroupAddresses splits a group's address values into aggregatable host -// addresses (bare IPv4 or IPv6, deduped) and pass-through values held verbatim. -// An entry's value comes from the singular IPAddress when set, otherwise from -// each element of IPAddresses. The "*" sentinel and unparseable values are -// pass-through; already-collapsed CIDRs are pass-through here but the caller -// folds them back into the exact cover. Both address families are aggregated — -// netipx covers IPv4 and IPv6 alike. +// classifyGroupAddresses splits a group's address values into aggregatable IPv4 +// host addresses (deduped) and pass-through values held verbatim. An entry's +// value comes from the singular IPAddress when set, otherwise from each element +// of IPAddresses. CIDRs, the "*" sentinel, IPv6 and unparseable values are +// pass-through (already-collapsed CIDRs are folded back into the cover by the +// caller; the rest are held verbatim). Only IPv4 hosts are aggregated: policy +// generation (buildIPAddressesPeers) skips non-IPv4 entries, so collapsing IPv6 +// hosts into a CIDR would silently drop them — and any ports — from the derived +// NetworkPolicy. IPv6 is therefore kept as individual pass-through entries. func classifyGroupAddresses(entries []softwarecomposition.NetworkNeighbor) ([]netip.Addr, []string) { seenHost := map[netip.Addr]struct{}{} seenPass := map[string]struct{}{} @@ -143,7 +146,7 @@ func classifyGroupAddresses(entries []softwarecomposition.NetworkNeighbor) ([]ne if v == "" { return } - if addr, err := netip.ParseAddr(v); err == nil { + if addr, err := netip.ParseAddr(v); err == nil && addr.Is4() { if _, ok := seenHost[addr]; !ok { seenHost[addr] = struct{}{} hosts = append(hosts, addr) @@ -168,28 +171,36 @@ func classifyGroupAddresses(entries []softwarecomposition.NetworkNeighbor) ([]ne return hosts, passthrough } -// coverPrefixes returns the minimal set of CIDR strings that covers EXACTLY the -// given IPv4 host addresses together with the group's already-collapsed -// pass-through CIDRs, capped so no prefix is broader than floorBits. netipx does -// the aggregation — deduplicating, dropping subsumed prefixes and merging -// adjacent siblings into the minimal exact cover in near-linear time — and any -// resulting prefix broader than the floor is then split into floorBits-wide -// children (all fully covered, since the parent lies wholly within the set). +// coverPrefixes returns the set of CIDR strings covering the given IPv4 host +// addresses together with the group's already-collapsed pass-through CIDRs, with +// no block broader than floorBits and a bounded entry count. // -// Because the cover is exact, it never over-approximates to a block the workload -// did not actually reach, and re-running on already-collapsed input is a -// fixpoint: no duplicate or nested blocks can accumulate across incremental -// saves. The result is sorted. +// It works in two stages. First the raw hosts are aggregated to the floor +// (aggregateHostsToFloor): a group of hosts sharing a common prefix at least as +// long as the floor collapses to that single tight block — kept tighter than the +// floor when the traffic really is that tight, e.g. a fully-observed /26 — while +// scattered hosts are bucketed into their floor-length networks so the output is +// bounded by the number of distinct floor networks reached, not the host count. +// Second, those host blocks are folded together with the already-held CIDRs +// through netipx, which deduplicates, drops subsumed prefixes and merges adjacent +// siblings into a canonical set. That fold is what fixes incremental-learning +// garbage like [52.216.0.0/26, 52.216.0.0/26, 52.216.0.0/27] — the duplicate +// deduplicated and the nested /27 absorbed — and makes re-collapsing a fixpoint. +// +// Any IPv4 block still broader than the floor after the fold (a held block from a +// coarser prior floor, or floor networks that merged into a shorter parent) is +// split back into floorBits-wide children. IPv6 has no floor and is emitted as +// covered. The result is sorted. func coverPrefixes(hosts []netip.Addr, cidrPass []netip.Prefix, floorBits int) []string { if len(hosts) == 0 && len(cidrPass) == 0 { return nil } var b netipx.IPSetBuilder - for _, h := range hosts { - b.Add(h) + for _, p := range aggregateHostsToFloor(hosts, floorBits) { + b.AddPrefix(p) } for _, p := range cidrPass { - b.AddPrefix(p) + b.AddPrefix(p.Masked()) } set, err := b.IPSet() if err != nil || set == nil { @@ -210,6 +221,70 @@ func coverPrefixes(hosts []netip.Addr, cidrPass []netip.Prefix, floorBits int) [ return out } +// aggregateHostsToFloor collapses raw host addresses into CIDR blocks no broader +// than floorBits. IPv4 hosts sharing a common prefix at least as long as the +// floor collapse to that single common block (kept tighter than the floor when +// the traffic is genuinely that tight, e.g. a fully-observed /26); otherwise each +// host is bucketed into its floor-length network, so scattered traffic yields at +// most one block per distinct floor network. IPv6 hosts — which the caller keeps +// out of aggregation, but which are handled defensively here — are emitted as +// individual host prefixes and never widened by the IPv4 floor. +func aggregateHostsToFloor(hosts []netip.Addr, floorBits int) []netip.Prefix { + var v4 []netip.Addr + var out []netip.Prefix + for _, h := range hosts { + if h.Is4() { + v4 = append(v4, h) + } else { + out = append(out, netip.PrefixFrom(h, h.BitLen()).Masked()) + } + } + if len(v4) == 0 { + return out + } + if commonLen := commonPrefixLen(v4); commonLen >= floorBits { + return append(out, netip.PrefixFrom(v4[0], commonLen).Masked()) + } + seen := make(map[netip.Prefix]struct{}, len(v4)) + for _, h := range v4 { + p := netip.PrefixFrom(h, floorBits).Masked() + if _, ok := seen[p]; !ok { + seen[p] = struct{}{} + out = append(out, p) + } + } + return out +} + +// commonPrefixLen returns the number of leading bits shared by every address in +// the set. All addresses are assumed to be IPv4. +func commonPrefixLen(addrs []netip.Addr) int { + base := addrs[0].As4() + common := 32 + for _, addr := range addrs[1:] { + b := addr.As4() + n := 0 + for i := 0; i < 4 && n < common; i++ { + x := base[i] ^ b[i] + if x == 0 { + n += 8 + continue + } + for bit := 7; bit >= 0; bit-- { + if x&(1< exact cover keeps them granular (one - // /32 apiece, since none are adjacent); no emitted block is broader than the - // floor, and the cover never over-approximates to a covering block. +func TestCollapseIPGroups_ScatteredHostsBucketedToFloor(t *testing.T) { + // 60 lone hosts, each in its own /16, do not share a common prefix as long as + // the floor, so each is bucketed into its floor-length (/16) network. Output + // is one block per distinct floor network — bounded by the number of networks + // reached, not the host count — and no block is broader than the floor. var in []softwarecomposition.NetworkNeighbor for i := 0; i < 60; i++ { in = append(in, hostNeighbor(fmt.Sprintf("%d.%d.0.1", 10+i, i))) @@ -68,12 +69,12 @@ func TestCollapseIPGroups_ScatteredHostsStayGranularAndCappedAtFloor(t *testing. out := collapseIPGroups(in, testSettings()) - assert.Greater(t, len(out), 1) + require.Len(t, out, 60, "one bucket per distinct /16") for _, e := range out { require.Len(t, e.IPAddresses, 1) p, err := netip.ParsePrefix(e.IPAddresses[0]) require.NoError(t, err) - assert.GreaterOrEqual(t, p.Bits(), 16, "no emitted block may be broader than the floor") + assert.Equal(t, 16, p.Bits(), "each lone host is bucketed into its floor-length network") } } @@ -146,7 +147,9 @@ func TestCollapseIPGroups_RealWorldShapeOrdersOfMagnitude(t *testing.T) { out := collapseIPGroups(in, testSettings()) - // 512 contiguous hosts exact-cover to a handful of blocks (two /24s here). + // The two /24s fall in different /16s and share no common prefix as long as + // the /16 floor, so each is bucketed into its floor network: a handful of + // blocks (two /16s here), orders of magnitude below the host count. assert.Less(t, len(out), 10) assert.Less(t, len(out), len(in)/50) for _, e := range out { @@ -176,7 +179,8 @@ func TestCollapseIPGroups_Idempotent(t *testing.T) { DNS: "example.com", IPAddresses: []string{"*"}, }) - // IPv6 entry — a lone v6 host exact-covers to its /128 + // IPv6 entry — held as a pass-through value verbatim (IPv6 is not aggregated, + // since policy generation consumes only IPv4 collapsed entries) in = append(in, softwarecomposition.NetworkNeighbor{ Type: softwarecomposition.CommunicationTypeEgress, DNS: "example.com", @@ -195,7 +199,7 @@ func TestCollapseIPGroups_Idempotent(t *testing.T) { } assert.Contains(t, values, "*") assert.Contains(t, values, "200.0.0.0/16") - assert.Contains(t, values, "2001:db8::1/128") + assert.Contains(t, values, "2001:db8::1") } func TestCollapseIPGroups_FieldContract(t *testing.T) { @@ -234,9 +238,13 @@ func TestCollapseIPGroups_MultiBucketReplicatesDNSNamesAndPorts(t *testing.T) { } } -func TestCollapseIPGroups_IPv6Aggregated(t *testing.T) { - // A full IPv6 /120 (256 contiguous v6 hosts) exact-covers to that /120, and a - // lone v6 host to its /128 — alongside a v4 group, in one mixed-family pass. +func TestCollapseIPGroups_IPv6NotAggregatedHeldPassThrough(t *testing.T) { + // IPv6 hosts are NOT aggregated into CIDRs: policy generation + // (buildIPAddressesPeers) consumes only IPv4 collapsed entries, so folding + // IPv6 hosts into an IPv6 CIDR would silently drop them — and their ports — + // from the derived NetworkPolicy. They are held as individual pass-through + // values instead, even when contiguous, while the co-located v4 group still + // collapses normally. var in []softwarecomposition.NetworkNeighbor for i := 0; i < 256; i++ { in = append(in, hostNeighbor(fmt.Sprintf("2606:4700:0:1::%x", i))) @@ -252,8 +260,12 @@ func TestCollapseIPGroups_IPv6Aggregated(t *testing.T) { for _, e := range out { values = append(values, e.IPAddresses...) } - assert.Contains(t, values, "2606:4700:0:1::/120", "contiguous v6 hosts aggregate") - assert.Contains(t, values, "2001:db8::42/128", "lone v6 host covers to /128") + // v6 hosts survive verbatim, never merged into a /120 or /128 CIDR + assert.Contains(t, values, "2606:4700:0:1::0", "contiguous v6 hosts stay individual") + assert.Contains(t, values, "2001:db8::42", "lone v6 host stays verbatim") + assert.NotContains(t, values, "2606:4700:0:1::/120", "v6 must not be aggregated into a CIDR") + // the co-located IPv4 group still collapses (a fully-observed /26) + assert.Contains(t, values, "10.5.0.0/26", "co-located IPv4 group still collapses") } func TestCoverPrefixes_IPv6ExactAndMerge(t *testing.T) { @@ -332,16 +344,19 @@ func TestCollapseIPGroups_IncrementalReCollapseDeduplicatesAndAbsorbs(t *testing assert.Equal(t, []string{"52.216.0.0/26"}, cidrs, "must converge to a single covering /26, not [/26 /26 /27]") } -func TestCoverPrefixes_ExactNoOverApproximation(t *testing.T) { - // Three non-adjacent hosts cover exactly {.1,.2,.3} — never a single /30 - // (which would admit the unobserved .0). +func TestCoverPrefixes_HostsCollapseToCommonPrefixWhenTighterThanFloor(t *testing.T) { + // Hosts sharing a common prefix at least as long as the floor collapse to that + // single common block. {.1,.2,.3} share a /30, which is tighter than the /16 + // floor, so they aggregate to 52.216.0.0/30 (bounding the entry count to one + // rather than emitting a /32 and a /31). The block is capped at the floor, but + // the workload's own common prefix is honored when it is already narrower. hosts := []netip.Addr{ netip.MustParseAddr("52.216.0.1"), netip.MustParseAddr("52.216.0.2"), netip.MustParseAddr("52.216.0.3"), } got := coverPrefixes(hosts, nil, 16) - assert.Equal(t, []string{"52.216.0.1/32", "52.216.0.2/31"}, got) + assert.Equal(t, []string{"52.216.0.0/30"}, got) } func TestCoverPrefixes_MergesAdjacentSiblings(t *testing.T) {