From 2929d980978b2d0a47f2b012b57b60d86300e304 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 12:08:52 +0200 Subject: [PATCH 01/19] fix(cpm): re-resolve in-cluster peer IP at profile flush (SUB-8865) When Inspektor Gadget's KubeIPResolver misses an in-cluster peer pod during initial connection startup races, it emits endpoint.k8s.kind = 'raw'. Previously, non-pod/svc endpoints were permanently recorded as external raw IP neighbors, resulting in invalid NetworkNeighborhood/ContainerProfile entries and broken generated NetworkPolicy selectors. This fix: 1. Re-resolves raw endpoints via k8sInventory (and k8sObjectCache fallback) at both network event reporting and profile flush time. 2. Defers raw private IP endpoints by one profile flush when not immediately resolvable, giving the Kubernetes informer time to receive the pod status. 3. Falls back to external IP only after deferral elapses or on forced profile saves (e.g. container termination or max sniffing time). 4. Cleans up pod template hashes on resolved pod selectors. Signed-off-by: Matthias Bertschy --- .../v1/container_data.go | 188 ++++++-- .../v1/container_data_test.go | 8 +- .../v1/containerprofile_manager.go | 26 +- .../v1/containerprofile_manager_test.go | 4 +- .../v1/event_reporting.go | 2 + .../v1/event_reporting_test.go | 14 +- pkg/containerprofilemanager/v1/monitoring.go | 18 +- .../v1/peer_resolution_test.go | 405 ++++++++++++++++++ 8 files changed, 615 insertions(+), 50 deletions(-) create mode 100644 pkg/containerprofilemanager/v1/peer_resolution_test.go diff --git a/pkg/containerprofilemanager/v1/container_data.go b/pkg/containerprofilemanager/v1/container_data.go index 5d25611fc6..c41e29a76c 100644 --- a/pkg/containerprofilemanager/v1/container_data.go +++ b/pkg/containerprofilemanager/v1/container_data.go @@ -1,20 +1,24 @@ package containerprofilemanager import ( + "net" "sort" + "github.com/DmitriyVTitov/size" mapset "github.com/deckarep/golang-set/v2" + "github.com/inspektor-gadget/inspektor-gadget/pkg/operators/common" "github.com/kubescape/go-logger" "github.com/kubescape/go-logger/helpers" "github.com/kubescape/k8s-interface/k8sinterface" "github.com/kubescape/node-agent/pkg/dnsmanager" "github.com/kubescape/node-agent/pkg/k8sclient" + "github.com/kubescape/node-agent/pkg/objectcache" "github.com/kubescape/node-agent/pkg/utils" "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// emptyEvents clears all event data +// emptyEvents clears all event data, but retains deferred network events for re-resolution func (cd *containerData) emptyEvents() { cd.size.Store(0) cd.capabilites = nil @@ -24,7 +28,20 @@ func (cd *containerData) emptyEvents() { cd.opens = nil cd.rulePolicies = nil cd.callStacks = nil - cd.networks = nil + if cd.deferredNetworks != nil && cd.deferredNetworks.Cardinality() > 0 { + cd.networks = cd.deferredNetworks.Clone() + cd.prevDeferredNetworks = cd.deferredNetworks.Clone() + cd.deferredNetworks = nil + var retainedSize int64 + for _, ev := range cd.networks.ToSlice() { + retainedSize += int64(size.Of(ev) + networkNeighborIncrement(cd, ev)) + } + cd.size.Store(retainedSize) + } else { + cd.networks = nil + cd.prevDeferredNetworks = nil + cd.deferredNetworks = nil + } cd.servicePorts = nil if cd.watchedContainerData != nil { cd.lastReportedCompletion = string(cd.watchedContainerData.GetCompletionStatus()) @@ -159,19 +176,80 @@ func (cd *containerData) getCallStacks() []v1beta1.IdentifiedCallStack { return callStacks } +func isPrivateIP(ipStr string) bool { + ip := net.ParseIP(ipStr) + return ip != nil && ip.IsPrivate() +} + +func resolveEndpoint( + event *NetworkEvent, + k8sInventory common.K8sInventoryCache, + k8sObjectCache objectcache.K8sObjectCache, +) { + if event.Destination.Kind == EndpointKindPod || event.Destination.Kind == EndpointKindService { + return + } + ip := event.Destination.IPAddress + if ip == "" || ip == "127.0.0.1" { + return + } + + if k8sInventory != nil { + if pod := k8sInventory.GetPodByIp(ip); pod != nil && !pod.Spec.HostNetwork { + event.Destination.Kind = EndpointKindPod + event.Destination.Name = pod.Name + event.Destination.Namespace = pod.Namespace + event.SetDestinationPodLabels(pod.Labels) + return + } + if svc := k8sInventory.GetSvcByIp(ip); svc != nil { + event.Destination.Kind = EndpointKindService + event.Destination.Name = svc.Name + event.Destination.Namespace = svc.Namespace + event.SetDestinationPodLabels(svc.Labels) + return + } + } + + if k8sObjectCache != nil { + for _, pod := range k8sObjectCache.GetPods() { + if pod != nil && !pod.Spec.HostNetwork && pod.Status.PodIP == ip { + event.Destination.Kind = EndpointKindPod + event.Destination.Name = pod.Name + event.Destination.Namespace = pod.Namespace + event.SetDestinationPodLabels(pod.Labels) + return + } + } + } +} + // getIngressNetworkNeighbors returns ingress network neighbors for this container -func (cd *containerData) getIngressNetworkNeighbors(containerID string, namespace string, k8sClient k8sclient.K8sClientInterface, dnsResolverClient dnsmanager.DNSResolver) []v1beta1.NetworkNeighbor { +func (cd *containerData) getIngressNetworkNeighbors( + containerID string, + namespace string, + k8sClient k8sclient.K8sClientInterface, + dnsResolverClient dnsmanager.DNSResolver, + k8sInventory common.K8sInventoryCache, + k8sObjectCache objectcache.K8sObjectCache, + forceSend bool, +) []v1beta1.NetworkNeighbor { var ingress []v1beta1.NetworkNeighbor if cd.networks == nil { return ingress } + seen := make(map[string]struct{}) for _, event := range cd.networks.ToSlice() { if event.PktType == utils.HostPktType { - neighbor := cd.createNetworkNeighbor(containerID, event, namespace, k8sClient, dnsResolverClient) + neighbor := cd.createNetworkNeighbor(containerID, event, namespace, k8sClient, dnsResolverClient, k8sInventory, k8sObjectCache, forceSend) if neighbor == nil { continue } + if _, ok := seen[neighbor.Identifier]; ok { + continue + } + seen[neighbor.Identifier] = struct{}{} ingress = append(ingress, *neighbor) } } @@ -180,18 +258,31 @@ func (cd *containerData) getIngressNetworkNeighbors(containerID string, namespac } // getEgressNetworkNeighbors returns egress network neighbors for this container -func (cd *containerData) getEgressNetworkNeighbors(containerID string, namespace string, k8sClient k8sclient.K8sClientInterface, dnsResolverClient dnsmanager.DNSResolver) []v1beta1.NetworkNeighbor { +func (cd *containerData) getEgressNetworkNeighbors( + containerID string, + namespace string, + k8sClient k8sclient.K8sClientInterface, + dnsResolverClient dnsmanager.DNSResolver, + k8sInventory common.K8sInventoryCache, + k8sObjectCache objectcache.K8sObjectCache, + forceSend bool, +) []v1beta1.NetworkNeighbor { var egress []v1beta1.NetworkNeighbor if cd.networks == nil { return egress } + seen := make(map[string]struct{}) for _, event := range cd.networks.ToSlice() { if event.PktType != utils.HostPktType { - neighbor := cd.createNetworkNeighbor(containerID, event, namespace, k8sClient, dnsResolverClient) + neighbor := cd.createNetworkNeighbor(containerID, event, namespace, k8sClient, dnsResolverClient, k8sInventory, k8sObjectCache, forceSend) if neighbor == nil { continue } + if _, ok := seen[neighbor.Identifier]; ok { + continue + } + seen[neighbor.Identifier] = struct{}{} egress = append(egress, *neighbor) } } @@ -200,7 +291,18 @@ func (cd *containerData) getEgressNetworkNeighbors(containerID string, namespace } // createNetworkNeighbor creates a network neighbor from a network event -func (cd *containerData) createNetworkNeighbor(containerID string, networkEvent NetworkEvent, namespace string, k8sClient k8sclient.K8sClientInterface, dnsResolverClient dnsmanager.DNSResolver) *v1beta1.NetworkNeighbor { +func (cd *containerData) createNetworkNeighbor( + containerID string, + networkEvent NetworkEvent, + namespace string, + k8sClient k8sclient.K8sClientInterface, + dnsResolverClient dnsmanager.DNSResolver, + k8sInventory common.K8sInventoryCache, + k8sObjectCache objectcache.K8sObjectCache, + forceSend bool, +) *v1beta1.NetworkNeighbor { + resolveEndpoint(&networkEvent, k8sInventory, k8sObjectCache) + var neighborEntry v1beta1.NetworkNeighbor enforcementPorts := []uint16{networkEvent.Port} @@ -220,22 +322,24 @@ func (cd *containerData) createNetworkNeighbor(containerID string, networkEvent } else if networkEvent.Destination.Kind == EndpointKindService { // For service, we need to retrieve it and use its selector - svc, err := k8sClient.GetWorkload(networkEvent.Destination.Namespace, "Service", networkEvent.Destination.Name) // TODO: use IG inventory as this can generate a lot of API calls. - if err != nil { - logger.L().Warning("failed to get service", - helpers.String("reason", err.Error()), - helpers.String("service name", networkEvent.Destination.Name)) - return nil - } - serviceWorkload = svc - var selector map[string]string - if svc.GetName() == "kubernetes" && svc.GetNamespace() == "default" { - // The default service has no selectors, in addition, we want to save the default service address - selector = svc.GetLabels() - neighborEntry.IPAddress = networkEvent.Destination.IPAddress - } else { - selector = svc.GetServiceSelector() + if k8sClient != nil { + svc, err := k8sClient.GetWorkload(networkEvent.Destination.Namespace, "Service", networkEvent.Destination.Name) // TODO: use IG inventory as this can generate a lot of API calls. + if err != nil { + logger.L().Warning("failed to get service", + helpers.String("reason", err.Error()), + helpers.String("service name", networkEvent.Destination.Name)) + return nil + } + serviceWorkload = svc + + if svc.GetName() == "kubernetes" && svc.GetNamespace() == "default" { + // The default service has no selectors, in addition, we want to save the default service address + selector = svc.GetLabels() + neighborEntry.IPAddress = networkEvent.Destination.IPAddress + } else { + selector = svc.GetServiceSelector() + } } if len(selector) == 0 { @@ -257,6 +361,19 @@ func (cd *containerData) createNetworkNeighbor(containerID string, networkEvent // No need to generate for localhost return nil } + + // If the IP is inside the cluster pod CIDR / private IP and could not be resolved, + // defer by one flush before persisting as an external IP, unless forceSend is true + // or it has already been deferred once. + if isPrivateIP(networkEvent.Destination.IPAddress) && !forceSend && cd != nil && + (cd.prevDeferredNetworks == nil || !cd.prevDeferredNetworks.Contains(networkEvent)) { + if cd.deferredNetworks == nil { + cd.deferredNetworks = mapset.NewSet[NetworkEvent]() + } + cd.deferredNetworks.Add(networkEvent) + return nil + } + neighborEntry.IPAddress = networkEvent.Destination.IPAddress if dnsResolverClient != nil { @@ -268,17 +385,22 @@ func (cd *containerData) createNetworkNeighbor(containerID string, networkEvent } } - if ports, ok := cd.servicePorts[networkEvent]; ok { - enforcementPorts = ports - } else if networkEvent.Destination.Kind == EndpointKindService && serviceWorkload != nil && k8sClient != nil { - enforcementPorts = resolveServiceEnforcementPorts( - k8sClient, - networkEvent.Destination.Namespace, - networkEvent.Destination.Name, - serviceWorkload, - networkEvent.Port, - networkEvent.Protocol, - ) + if cd != nil && cd.servicePorts != nil { + if ports, ok := cd.servicePorts[networkEvent]; ok { + enforcementPorts = ports + } + } + if len(enforcementPorts) == 1 && enforcementPorts[0] == networkEvent.Port { + if networkEvent.Destination.Kind == EndpointKindService && serviceWorkload != nil && k8sClient != nil { + enforcementPorts = resolveServiceEnforcementPorts( + k8sClient, + networkEvent.Destination.Namespace, + networkEvent.Destination.Name, + serviceWorkload, + networkEvent.Port, + networkEvent.Protocol, + ) + } } neighborEntry.Ports = buildNetworkPorts(networkEvent.Protocol, enforcementPorts) diff --git a/pkg/containerprofilemanager/v1/container_data_test.go b/pkg/containerprofilemanager/v1/container_data_test.go index 57d0f52776..b8876f6a03 100644 --- a/pkg/containerprofilemanager/v1/container_data_test.go +++ b/pkg/containerprofilemanager/v1/container_data_test.go @@ -270,7 +270,7 @@ func TestCreateNetworkNeighbor_ServiceTargetPortMatrix(t *testing.T) { } cd := &containerData{} - neighbor := cd.createNetworkNeighbor("", tc.event, "default", client, nil) + neighbor := cd.createNetworkNeighbor("", tc.event, "default", client, nil, nil, nil, false) require.NotNil(t, neighbor) require.Equal(t, map[string]string{"app": "api"}, neighbor.PodSelector.MatchLabels) require.Equal(t, tc.wantPorts, networkPortValues(neighbor.Ports)) @@ -298,7 +298,7 @@ func TestCreateNetworkNeighbor_NonServiceDestinationsUnchanged(t *testing.T) { }, } podEvent.SetDestinationPodLabels(map[string]string{"app": "web"}) - podNeighbor := cd.createNetworkNeighbor("", podEvent, "default", nil, nil) + podNeighbor := cd.createNetworkNeighbor("", podEvent, "default", nil, nil, nil, nil, false) require.NotNil(t, podNeighbor) require.Equal(t, []int32{8080}, networkPortValues(podNeighbor.Ports)) @@ -310,7 +310,7 @@ func TestCreateNetworkNeighbor_NonServiceDestinationsUnchanged(t *testing.T) { IPAddress: "93.184.216.34", }, } - rawNeighbor := cd.createNetworkNeighbor("", rawEvent, "default", nil, nil) + rawNeighbor := cd.createNetworkNeighbor("", rawEvent, "default", nil, nil, nil, nil, false) require.NotNil(t, rawNeighbor) require.Equal(t, []int32{443}, networkPortValues(rawNeighbor.Ports)) } @@ -326,7 +326,7 @@ func TestGenerateNetworkPolicy_ServiceTargetPortRoundTrip(t *testing.T) { cd := &containerData{} event := serviceNetworkEvent(80, "tcp") - neighbor := cd.createNetworkNeighbor("", event, "default", client, nil) + neighbor := cd.createNetworkNeighbor("", event, "default", client, nil, nil, nil, false) require.NotNil(t, neighbor) egressPorts := make([]softwarecomposition.NetworkPort, 0, len(neighbor.Ports)) diff --git a/pkg/containerprofilemanager/v1/containerprofile_manager.go b/pkg/containerprofilemanager/v1/containerprofile_manager.go index ad53cb054f..6528ceeb33 100644 --- a/pkg/containerprofilemanager/v1/containerprofile_manager.go +++ b/pkg/containerprofilemanager/v1/containerprofile_manager.go @@ -13,6 +13,7 @@ import ( mapset "github.com/deckarep/golang-set/v2" "github.com/goradd/maps" containercollection "github.com/inspektor-gadget/inspektor-gadget/pkg/container-collection" + "github.com/inspektor-gadget/inspektor-gadget/pkg/operators/common" "github.com/kubescape/go-logger" "github.com/kubescape/go-logger/helpers" "github.com/kubescape/node-agent/pkg/config" @@ -61,8 +62,10 @@ type containerData struct { opens *maps.SafeMap[string, mapset.Set[string]] // Map of opens, key is file path rulePolicies *maps.SafeMap[string, *v1beta1.RulePolicy] // Map of rule policies, key is rule ID callStacks *maps.SafeMap[string, *v1beta1.IdentifiedCallStack] // Map of callstacks, key is SHA256 hash - networks mapset.Set[NetworkEvent] - droppedEvents bool // Indicates if any events were dropped during monitoring + networks mapset.Set[NetworkEvent] + deferredNetworks mapset.Set[NetworkEvent] + prevDeferredNetworks mapset.Set[NetworkEvent] + droppedEvents bool // Indicates if any events were dropped during monitoring // Service port snapshots keep report-time accounting and serialization consistent. servicePorts map[NetworkEvent][]uint16 @@ -86,6 +89,7 @@ type ContainerProfileManager struct { cfg config.Config k8sClient k8sclient.K8sClientInterface k8sObjectCache objectcache.K8sObjectCache + k8sInventory common.K8sInventoryCache storageClient storage.ProfileCreator dnsResolverClient dnsmanager.DNSResolver seccompManager seccompmanager.SeccompManagerClient @@ -122,6 +126,11 @@ func (cpm *ContainerProfileManager) SetCompletionNotifier(n objectcache.Completi cpm.completionNotifier = n } +// SetK8sInventory sets the k8s inventory cache (primarily used in tests) +func (cpm *ContainerProfileManager) SetK8sInventory(k8sInventory common.K8sInventoryCache) { + cpm.k8sInventory = k8sInventory +} + // SetSyscallFlusher implements containerprofilemanager.ContainerProfileManagerClient. func (cpm *ContainerProfileManager) SetSyscallFlusher(flush func()) { cpm.syscallFlusher.Store(&flush) @@ -162,6 +171,15 @@ func NewContainerProfileManager( lifecycleTracker: otelsetup.NewProfileLifecycleTracker(), } + if cfg.KubernetesMode { + if k8sInventory, err := common.GetK8sInventoryCache(); err == nil && k8sInventory != nil { + containerProfileManager.k8sInventory = k8sInventory + k8sInventory.Start() + } else if err != nil { + logger.L().Debug("failed to initialize k8s inventory cache in container profile manager", helpers.Error(err)) + } + } + // Initialize queue queueDir := os.Getenv("QUEUE_DIR") if queueDir == "" { @@ -221,6 +239,10 @@ func (cpm *ContainerProfileManager) Close() { if cpm.queueData != nil { _ = cpm.queueData.Close() } + + if cpm.k8sInventory != nil { + cpm.k8sInventory.Stop() + } } var _ containerprofilemanager.ContainerProfileManagerClient = (*ContainerProfileManager)(nil) diff --git a/pkg/containerprofilemanager/v1/containerprofile_manager_test.go b/pkg/containerprofilemanager/v1/containerprofile_manager_test.go index f537c53845..dd228c9b68 100644 --- a/pkg/containerprofilemanager/v1/containerprofile_manager_test.go +++ b/pkg/containerprofilemanager/v1/containerprofile_manager_test.go @@ -573,11 +573,11 @@ func TestContainerDataMethods(t *testing.T) { assert.Empty(t, callStacks) // Test getIngressNetworkNeighbors with nil networks - ingress := cd.getIngressNetworkNeighbors("", "default", nil, nil) + ingress := cd.getIngressNetworkNeighbors("", "default", nil, nil, nil, nil, false) assert.Empty(t, ingress) // Test getEgressNetworkNeighbors with nil networks - egress := cd.getEgressNetworkNeighbors("", "default", nil, nil) + egress := cd.getEgressNetworkNeighbors("", "default", nil, nil, nil, nil, false) assert.Empty(t, egress) } diff --git a/pkg/containerprofilemanager/v1/event_reporting.go b/pkg/containerprofilemanager/v1/event_reporting.go index 7c746071e9..2d5b233b6c 100644 --- a/pkg/containerprofilemanager/v1/event_reporting.go +++ b/pkg/containerprofilemanager/v1/event_reporting.go @@ -353,6 +353,8 @@ func (cpm *ContainerProfileManager) ReportNetworkEvent(containerID string, event networkEvent.SetPodLabels(event.GetPodLabels()) networkEvent.SetDestinationPodLabels(dstEndpoint.PodLabels) + resolveEndpoint(&networkEvent, cpm.k8sInventory, cpm.k8sObjectCache) + // Skip if we already saved this event if data.networks.Contains(networkEvent) { return 0, nil diff --git a/pkg/containerprofilemanager/v1/event_reporting_test.go b/pkg/containerprofilemanager/v1/event_reporting_test.go index 9e314d91d6..d590a808d8 100644 --- a/pkg/containerprofilemanager/v1/event_reporting_test.go +++ b/pkg/containerprofilemanager/v1/event_reporting_test.go @@ -117,7 +117,7 @@ func TestNetworkNeighborIncrementCoversMaxDNSName(t *testing.T) { } cd := &containerData{} - neighbor := cd.createNetworkNeighbor("", networkEvent, "default", nil, fakeDNSResolver{domain: maxDNSName}) + neighbor := cd.createNetworkNeighbor("", networkEvent, "default", nil, fakeDNSResolver{domain: maxDNSName}, nil, nil, false) if !assert.NotNil(t, neighbor) { return } @@ -157,7 +157,7 @@ func TestNetworkNeighborIncrementCoversSelectorPayload(t *testing.T) { // neighbor. watchedContainerData.Namespace is what networkNeighborIncrement reads to make // the same "different namespace" call createNetworkNeighbor's own namespace arg does below. cd := &containerData{watchedContainerData: &objectcache.WatchedContainerData{Namespace: "default"}} - neighbor := cd.createNetworkNeighbor("", networkEvent, "default", nil, nil) + neighbor := cd.createNetworkNeighbor("", networkEvent, "default", nil, nil, nil, nil, false) if !assert.NotNil(t, neighbor) { return } @@ -252,7 +252,7 @@ func TestCreateNetworkNeighbor_EmptyContainerIDWithWatchedContainerData(t *testi } resolver := &trackingDNSResolver{} - neighbor := cd.createNetworkNeighbor("", networkEvent, "default", nil, resolver) + neighbor := cd.createNetworkNeighbor("", networkEvent, "default", nil, resolver, nil, nil, false) assert.NotNil(t, neighbor) assert.Equal(t, "", resolver.lastContainerID, "empty containerID must be preserved without falling back to watchedContainerData") assert.Equal(t, "93.184.216.34", resolver.lastIPAddress) @@ -279,7 +279,7 @@ func TestReportNetworkEventServicePortMultiplicity(t *testing.T) { DstPort: 80, Proto: "tcp", PktType: utils.OutgoingPktType, } cpm.ReportNetworkEvent("container1", event) - neighbor := entry.data.createNetworkNeighbor("", serviceNetworkEvent(80, "tcp"), "default", client, nil) + neighbor := entry.data.createNetworkNeighbor("", serviceNetworkEvent(80, "tcp"), "default", client, nil, nil, nil, false) require.NotNil(t, neighbor) require.Equal(t, []int32{8080, 9090, 10000}, networkPortValues(neighbor.Ports)) // Isolate the port budget so unused selector headroom cannot hide an undercount. @@ -305,13 +305,13 @@ func TestReportNetworkEventServicePortMultiplicity(t *testing.T) { // Endpoint changes after reporting must not change the budgeted port list. require.NoError(t, client.kubeClient.DiscoveryV1().EndpointSlices("default").Delete(context.Background(), "c", metav1.DeleteOptions{})) - neighbor = entry.data.createNetworkNeighbor("", serviceNetworkEvent(80, "tcp"), "default", client, nil) + neighbor = entry.data.createNetworkNeighbor("", serviceNetworkEvent(80, "tcp"), "default", client, nil, nil, nil, false) require.Equal(t, []int32{8080, 9090, 10000}, networkPortValues(neighbor.Ports)) // A new profile batch resolves fresh ports instead of keeping the old snapshot. entry.data.emptyEvents() cpm.ReportNetworkEvent("container1", event) - neighbor = entry.data.createNetworkNeighbor("", serviceNetworkEvent(80, "tcp"), "default", client, nil) + neighbor = entry.data.createNetworkNeighbor("", serviceNetworkEvent(80, "tcp"), "default", client, nil, nil, nil, false) require.Equal(t, []int32{8080, 9090}, networkPortValues(neighbor.Ports)) require.Less(t, entry.data.size.Load(), recordedSize) } @@ -349,7 +349,7 @@ func TestCreateNetworkNeighbor_StatefulSetPeerStripsPodIdentityLabels(t *testing }) cd := &containerData{} - neighbor := cd.createNetworkNeighbor("", networkEvent, "default", nil, nil) + neighbor := cd.createNetworkNeighbor("", networkEvent, "default", nil, nil, nil, nil, false) if !assert.NotNil(t, neighbor) { return } diff --git a/pkg/containerprofilemanager/v1/monitoring.go b/pkg/containerprofilemanager/v1/monitoring.go index 96ecbb78ee..40654c2305 100644 --- a/pkg/containerprofilemanager/v1/monitoring.go +++ b/pkg/containerprofilemanager/v1/monitoring.go @@ -302,8 +302,8 @@ func (cpm *ContainerProfileManager) saveContainerProfile(watchedContainer *objec Endpoints: containerData.getEndpoints(), PolicyByRuleId: containerData.getRulePolicies(), IdentifiedCallStacks: containerData.getCallStacks(), - Egress: containerData.getEgressNetworkNeighbors(watchedContainer.ContainerID, container.K8s.Namespace, cpm.k8sClient, cpm.dnsResolverClient), - Ingress: containerData.getIngressNetworkNeighbors(watchedContainer.ContainerID, container.K8s.Namespace, cpm.k8sClient, cpm.dnsResolverClient), + Egress: containerData.getEgressNetworkNeighbors(watchedContainer.ContainerID, container.K8s.Namespace, cpm.k8sClient, cpm.dnsResolverClient, cpm.k8sInventory, cpm.k8sObjectCache, forceSend), + Ingress: containerData.getIngressNetworkNeighbors(watchedContainer.ContainerID, container.K8s.Namespace, cpm.k8sClient, cpm.dnsResolverClient, cpm.k8sInventory, cpm.k8sObjectCache, forceSend), LabelSelector: metav1.LabelSelector{ MatchLabels: watchedContainer.ParentWorkloadSelector.MatchLabels, MatchExpressions: watchedContainer.ParentWorkloadSelector.MatchExpressions, @@ -311,6 +311,20 @@ func (cpm *ContainerProfileManager) saveContainerProfile(watchedContainer *objec }, } + if !forceSend && containerData.deferredNetworks != nil && containerData.deferredNetworks.Cardinality() > 0 && + len(containerProfile.Spec.Capabilities) == 0 && + len(containerProfile.Spec.Execs) == 0 && + len(containerProfile.Spec.Opens) == 0 && + len(containerProfile.Spec.Syscalls) == 0 && + len(containerProfile.Spec.Endpoints) == 0 && + len(containerProfile.Spec.PolicyByRuleId) == 0 && + len(containerProfile.Spec.IdentifiedCallStacks) == 0 && + len(containerProfile.Spec.Egress) == 0 && + len(containerProfile.Spec.Ingress) == 0 { + containerData.emptyEvents() + return nil + } + if err := cpm.enqueueContainerProfile(containerProfile, watchedContainer.ContainerID); err != nil { // Empty the container data to prevent reporting the same data again containerData.emptyEvents() diff --git a/pkg/containerprofilemanager/v1/peer_resolution_test.go b/pkg/containerprofilemanager/v1/peer_resolution_test.go new file mode 100644 index 0000000000..0e97bd04e4 --- /dev/null +++ b/pkg/containerprofilemanager/v1/peer_resolution_test.go @@ -0,0 +1,405 @@ +package containerprofilemanager + +import ( + "testing" + + "github.com/inspektor-gadget/inspektor-gadget/pkg/operators/common" + "github.com/inspektor-gadget/inspektor-gadget/pkg/types" + "github.com/kubescape/node-agent/pkg/objectcache" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +type stubK8sObjectCache struct { + objectcache.K8sObjectCacheMock + pods []*corev1.Pod +} + +func (s *stubK8sObjectCache) GetPods() []*corev1.Pod { + return s.pods +} + +type mockK8sInventory struct { + podsByIP map[string]*common.SlimPod + svcsByIP map[string]*common.SlimService +} + +func newMockK8sInventory() *mockK8sInventory { + return &mockK8sInventory{ + podsByIP: make(map[string]*common.SlimPod), + svcsByIP: make(map[string]*common.SlimService), + } +} + +func (m *mockK8sInventory) Start() {} +func (m *mockK8sInventory) Stop() {} + +func (m *mockK8sInventory) GetPods() []*common.SlimPod { + var pods []*common.SlimPod + for _, p := range m.podsByIP { + pods = append(pods, p) + } + return pods +} + +func (m *mockK8sInventory) GetPodByName(namespace string, name string) *common.SlimPod { + for _, p := range m.podsByIP { + if p.Namespace == namespace && p.Name == name { + return p + } + } + return nil +} + +func (m *mockK8sInventory) GetPodByIp(ip string) *common.SlimPod { + if m.podsByIP == nil { + return nil + } + return m.podsByIP[ip] +} + +func (m *mockK8sInventory) GetSvcs() []*common.SlimService { + var svcs []*common.SlimService + for _, s := range m.svcsByIP { + svcs = append(svcs, s) + } + return svcs +} + +func (m *mockK8sInventory) GetSvcByName(namespace string, name string) *common.SlimService { + for _, s := range m.svcsByIP { + if s.Namespace == namespace && s.Name == name { + return s + } + } + return nil +} + +func (m *mockK8sInventory) GetSvcByIp(ip string) *common.SlimService { + if m.svcsByIP == nil { + return nil + } + return m.svcsByIP[ip] +} + +func TestCreateNetworkNeighbor_RawPodIP_ResolvedViaK8sInventory(t *testing.T) { + inv := newMockK8sInventory() + inv.podsByIP["10.244.0.14"] = &common.SlimPod{ + SlimObjectMeta: common.SlimObjectMeta{ + Name: "wikijs-5b7c844697-x9k2v", + Namespace: "default", + Labels: map[string]string{ + "app": "wikijs", + "pod-template-hash": "5b7c844697", + }, + }, + Spec: common.SlimPodSpec{ + HostNetwork: false, + }, + Status: common.SlimPodStatus{ + PodIP: "10.244.0.14", + }, + } + + cd := &containerData{} + rawEvent := NetworkEvent{ + Port: 3306, + Protocol: "tcp", + PktType: utils.HostPktType, // ingress to mariadb + Destination: Destination{ + Kind: EndpointKindRaw, + IPAddress: "10.244.0.14", + }, + } + + neighbor := cd.createNetworkNeighbor("", rawEvent, "default", nil, nil, inv, nil, false) + require.NotNil(t, neighbor) + assert.Equal(t, InternalTrafficType, string(neighbor.Type)) + assert.Empty(t, neighbor.IPAddress, "pod neighbor must not have raw ipAddress set") + require.NotNil(t, neighbor.PodSelector) + assert.Equal(t, map[string]string{"app": "wikijs"}, neighbor.PodSelector.MatchLabels) + assert.NotContains(t, neighbor.PodSelector.MatchLabels, "pod-template-hash") + assert.Nil(t, neighbor.NamespaceSelector, "same namespace should have nil namespaceSelector") + require.Len(t, neighbor.Ports, 1) + assert.Equal(t, int32(3306), *neighbor.Ports[0].Port) +} + +func TestCreateNetworkNeighbor_RawPodIP_CrossNamespace(t *testing.T) { + inv := newMockK8sInventory() + inv.podsByIP["10.244.0.14"] = &common.SlimPod{ + SlimObjectMeta: common.SlimObjectMeta{ + Name: "wikijs-abcde", + Namespace: "client-ns", + Labels: map[string]string{ + "app": "wikijs", + }, + }, + Spec: common.SlimPodSpec{ + HostNetwork: false, + }, + Status: common.SlimPodStatus{ + PodIP: "10.244.0.14", + }, + } + + cd := &containerData{} + rawEvent := NetworkEvent{ + Port: 3306, + Protocol: "tcp", + PktType: utils.HostPktType, + Destination: Destination{ + Kind: EndpointKindRaw, + IPAddress: "10.244.0.14", + }, + } + + neighbor := cd.createNetworkNeighbor("", rawEvent, "server-ns", nil, nil, inv, nil, false) + require.NotNil(t, neighbor) + assert.Equal(t, InternalTrafficType, string(neighbor.Type)) + require.NotNil(t, neighbor.PodSelector) + assert.Equal(t, map[string]string{"app": "wikijs"}, neighbor.PodSelector.MatchLabels) + require.NotNil(t, neighbor.NamespaceSelector) + assert.Equal(t, map[string]string{"kubernetes.io/metadata.name": "client-ns"}, neighbor.NamespaceSelector.MatchLabels) +} + +func TestCreateNetworkNeighbor_RawPodIP_ResolvedViaK8sObjectCache(t *testing.T) { + mockCache := &stubK8sObjectCache{ + pods: []*corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "wikijs-pod", + Namespace: "default", + Labels: map[string]string{"app": "wikijs"}, + }, + Spec: corev1.PodSpec{ + HostNetwork: false, + }, + Status: corev1.PodStatus{ + PodIP: "10.244.0.14", + }, + }, + }, + } + + cd := &containerData{} + rawEvent := NetworkEvent{ + Port: 3306, + Protocol: "tcp", + PktType: utils.HostPktType, + Destination: Destination{ + Kind: EndpointKindRaw, + IPAddress: "10.244.0.14", + }, + } + + neighbor := cd.createNetworkNeighbor("", rawEvent, "default", nil, nil, nil, mockCache, false) + require.NotNil(t, neighbor) + assert.Equal(t, InternalTrafficType, string(neighbor.Type)) + require.NotNil(t, neighbor.PodSelector) + assert.Equal(t, map[string]string{"app": "wikijs"}, neighbor.PodSelector.MatchLabels) +} + +func TestCreateNetworkNeighbor_RawServiceIP_ResolvedViaK8sInventory(t *testing.T) { + inv := newMockK8sInventory() + inv.svcsByIP["10.96.0.42"] = &common.SlimService{ + SlimObjectMeta: common.SlimObjectMeta{ + Name: "api-svc", + Namespace: "default", + }, + Spec: common.SlimServiceSpec{ + ClusterIP: "10.96.0.42", + }, + } + + service := newServiceWorkload("api-svc", map[string]any{"app": "api"}, map[string]any{ + "port": 80, "targetPort": 8080, "protocol": "TCP", + }) + client := &servicePortTestClient{ + service: service, + kubeClient: fake.NewClientset(), + } + + cd := &containerData{} + rawEvent := NetworkEvent{ + Port: 80, + Protocol: "tcp", + PktType: utils.OutgoingPktType, + Destination: Destination{ + Kind: EndpointKindRaw, + IPAddress: "10.96.0.42", + }, + } + + neighbor := cd.createNetworkNeighbor("", rawEvent, "default", client, nil, inv, nil, false) + require.NotNil(t, neighbor) + assert.Equal(t, InternalTrafficType, string(neighbor.Type)) + require.NotNil(t, neighbor.PodSelector) + assert.Equal(t, map[string]string{"app": "api"}, neighbor.PodSelector.MatchLabels) +} + +func TestCreateNetworkNeighbor_RawPrivateIP_DeferredOnIntermediateFlush(t *testing.T) { + cd := &containerData{} + rawEvent := NetworkEvent{ + Port: 3306, + Protocol: "tcp", + PktType: utils.HostPktType, + Destination: Destination{ + Kind: EndpointKindRaw, + IPAddress: "10.244.0.14", + }, + } + + // 1. First flush: IP is private (10.244.0.14) and inventory does not have it yet. + // Intermediate flush (forceSend = false). + neighbor := cd.createNetworkNeighbor("", rawEvent, "default", nil, nil, nil, nil, false) + assert.Nil(t, neighbor, "raw private IP should be deferred on first intermediate flush") + require.NotNil(t, cd.deferredNetworks) + assert.True(t, cd.deferredNetworks.Contains(rawEvent)) + + // 2. emptyEvents preserves deferred networks for next flush + cd.emptyEvents() + assert.Nil(t, cd.deferredNetworks) + require.NotNil(t, cd.networks) + assert.True(t, cd.networks.Contains(rawEvent)) + require.NotNil(t, cd.prevDeferredNetworks) + assert.True(t, cd.prevDeferredNetworks.Contains(rawEvent)) + + // 3. Second flush: inventory now has the pod! + inv := newMockK8sInventory() + inv.podsByIP["10.244.0.14"] = &common.SlimPod{ + SlimObjectMeta: common.SlimObjectMeta{ + Name: "wikijs", + Namespace: "default", + Labels: map[string]string{"app": "wikijs"}, + }, + Status: common.SlimPodStatus{PodIP: "10.244.0.14"}, + } + + neighbor = cd.createNetworkNeighbor("", rawEvent, "default", nil, nil, inv, nil, false) + require.NotNil(t, neighbor, "re-resolved to pod on second flush") + assert.Equal(t, InternalTrafficType, string(neighbor.Type)) + assert.Equal(t, map[string]string{"app": "wikijs"}, neighbor.PodSelector.MatchLabels) +} + +func TestCreateNetworkNeighbor_RawPrivateIP_EmittedExternalIfNeverResolves(t *testing.T) { + cd := &containerData{} + rawEvent := NetworkEvent{ + Port: 3306, + Protocol: "tcp", + PktType: utils.OutgoingPktType, + Destination: Destination{ + Kind: EndpointKindRaw, + IPAddress: "10.50.1.20", // off-cluster private IP + }, + } + + // Flush 1: deferred + neighbor := cd.createNetworkNeighbor("", rawEvent, "default", nil, nil, nil, nil, false) + assert.Nil(t, neighbor) + + // emptyEvents moves it to prevDeferredNetworks + cd.emptyEvents() + + // Flush 2: already deferred once, now emitted as external + neighbor = cd.createNetworkNeighbor("", rawEvent, "default", nil, nil, nil, nil, false) + require.NotNil(t, neighbor) + assert.Equal(t, ExternalTrafficType, string(neighbor.Type)) + assert.Equal(t, "10.50.1.20", neighbor.IPAddress) +} + +func TestCreateNetworkNeighbor_PublicIP_EmittedExternalImmediately(t *testing.T) { + cd := &containerData{} + rawEvent := NetworkEvent{ + Port: 443, + Protocol: "tcp", + PktType: utils.OutgoingPktType, + Destination: Destination{ + Kind: EndpointKindRaw, + IPAddress: "93.184.216.34", // public IP + }, + } + + neighbor := cd.createNetworkNeighbor("", rawEvent, "default", nil, nil, nil, nil, false) + require.NotNil(t, neighbor) + assert.Equal(t, ExternalTrafficType, string(neighbor.Type)) + assert.Equal(t, "93.184.216.34", neighbor.IPAddress) + assert.Nil(t, cd.deferredNetworks) +} + +func TestReportNetworkEvent_ImmediateResolutionWhenAvailableInInventory(t *testing.T) { + cpm, entry := newTestManager(t, "container1") + inv := newMockK8sInventory() + inv.podsByIP["10.244.0.14"] = &common.SlimPod{ + SlimObjectMeta: common.SlimObjectMeta{ + Name: "wikijs", + Namespace: "default", + Labels: map[string]string{"app": "wikijs"}, + }, + Status: common.SlimPodStatus{PodIP: "10.244.0.14"}, + } + cpm.SetK8sInventory(inv) + + event := &utils.StructEvent{ + DstEndpoint: types.L3Endpoint{ + Addr: "10.244.0.14", + Kind: types.EndpointKindRaw, // inspector gadget emitted raw + }, + DstPort: 3306, + Proto: "tcp", + PktType: utils.HostPktType, + } + + cpm.ReportNetworkEvent("container1", event) + + // Verify that networks stored the event directly resolved to EndpointKindPod + slice := entry.data.networks.ToSlice() + require.Len(t, slice, 1) + assert.Equal(t, EndpointKindPod, slice[0].Destination.Kind) + assert.Equal(t, "wikijs", slice[0].Destination.Name) + assert.Equal(t, map[string]string{"app": "wikijs"}, slice[0].GetDestinationPodLabels()) +} + +func TestMonitoring_ReResolutionAtProfileFlush(t *testing.T) { + cpm, entry := newTestManager(t, "container1") + inv := newMockK8sInventory() + cpm.SetK8sInventory(inv) + + // Step 1: Network event arrived when pod was NOT yet in inventory + event := &utils.StructEvent{ + DstEndpoint: types.L3Endpoint{ + Addr: "10.244.0.14", + Kind: types.EndpointKindRaw, + }, + DstPort: 3306, + Proto: "tcp", + PktType: utils.HostPktType, + } + cpm.ReportNetworkEvent("container1", event) + + // Event is stored as raw + slice := entry.data.networks.ToSlice() + require.Len(t, slice, 1) + assert.Equal(t, EndpointKindRaw, slice[0].Destination.Kind) + + // Step 2: Informer catches up before saveProfile flush! + inv.podsByIP["10.244.0.14"] = &common.SlimPod{ + SlimObjectMeta: common.SlimObjectMeta{ + Name: "wikijs-789", + Namespace: "default", + Labels: map[string]string{"app": "wikijs"}, + }, + Status: common.SlimPodStatus{PodIP: "10.244.0.14"}, + } + + // Step 3: Profile generation resolves raw peer via k8sInventory + ingress := entry.data.getIngressNetworkNeighbors("mariadb", "default", nil, nil, cpm.k8sInventory, cpm.k8sObjectCache, false) + require.Len(t, ingress, 1) + assert.Equal(t, InternalTrafficType, string(ingress[0].Type)) + assert.Empty(t, ingress[0].IPAddress) + require.NotNil(t, ingress[0].PodSelector) + assert.Equal(t, map[string]string{"app": "wikijs"}, ingress[0].PodSelector.MatchLabels) +} From 37fb999a6cb64aa7b021b48118c1b6be479f1f55 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 12:38:39 +0200 Subject: [PATCH 02/19] fix(cpm): address Copilot review 5413180027 Preserve report timestamps across deferred-only flushes, merge distinct neighbor ports, and honor cached service-port snapshots. Add regression coverage for all three findings. Signed-off-by: Matthias Bertschy --- .../v1/container_data.go | 39 ++++++---- pkg/containerprofilemanager/v1/monitoring.go | 27 +++---- .../v1/monitoring_timestamp_test.go | 73 +++++++++++++++++++ .../v1/peer_resolution_test.go | 57 +++++++++++++++ 4 files changed, 169 insertions(+), 27 deletions(-) create mode 100644 pkg/containerprofilemanager/v1/monitoring_timestamp_test.go diff --git a/pkg/containerprofilemanager/v1/container_data.go b/pkg/containerprofilemanager/v1/container_data.go index c41e29a76c..1d22ba9122 100644 --- a/pkg/containerprofilemanager/v1/container_data.go +++ b/pkg/containerprofilemanager/v1/container_data.go @@ -2,6 +2,7 @@ package containerprofilemanager import ( "net" + "slices" "sort" "github.com/DmitriyVTitov/size" @@ -239,18 +240,14 @@ func (cd *containerData) getIngressNetworkNeighbors( return ingress } - seen := make(map[string]struct{}) + seen := make(map[string]int) for _, event := range cd.networks.ToSlice() { if event.PktType == utils.HostPktType { neighbor := cd.createNetworkNeighbor(containerID, event, namespace, k8sClient, dnsResolverClient, k8sInventory, k8sObjectCache, forceSend) if neighbor == nil { continue } - if _, ok := seen[neighbor.Identifier]; ok { - continue - } - seen[neighbor.Identifier] = struct{}{} - ingress = append(ingress, *neighbor) + ingress = appendNetworkNeighbor(ingress, seen, *neighbor) } } @@ -272,24 +269,36 @@ func (cd *containerData) getEgressNetworkNeighbors( return egress } - seen := make(map[string]struct{}) + seen := make(map[string]int) for _, event := range cd.networks.ToSlice() { if event.PktType != utils.HostPktType { neighbor := cd.createNetworkNeighbor(containerID, event, namespace, k8sClient, dnsResolverClient, k8sInventory, k8sObjectCache, forceSend) if neighbor == nil { continue } - if _, ok := seen[neighbor.Identifier]; ok { - continue - } - seen[neighbor.Identifier] = struct{}{} - egress = append(egress, *neighbor) + egress = appendNetworkNeighbor(egress, seen, *neighbor) } } return egress } +// appendNetworkNeighbor merges all observed ports for neighbors with the same identity. +func appendNetworkNeighbor(neighbors []v1beta1.NetworkNeighbor, seen map[string]int, neighbor v1beta1.NetworkNeighbor) []v1beta1.NetworkNeighbor { + if index, ok := seen[neighbor.Identifier]; ok { + for _, port := range neighbor.Ports { + if !slices.ContainsFunc(neighbors[index].Ports, func(existing v1beta1.NetworkPort) bool { + return existing.Name == port.Name + }) { + neighbors[index].Ports = append(neighbors[index].Ports, port) + } + } + return neighbors + } + seen[neighbor.Identifier] = len(neighbors) + return append(neighbors, neighbor) +} + // createNetworkNeighbor creates a network neighbor from a network event func (cd *containerData) createNetworkNeighbor( containerID string, @@ -385,12 +394,14 @@ func (cd *containerData) createNetworkNeighbor( } } - if cd != nil && cd.servicePorts != nil { + hasPortSnapshot := false + if cd != nil { if ports, ok := cd.servicePorts[networkEvent]; ok { + hasPortSnapshot = true enforcementPorts = ports } } - if len(enforcementPorts) == 1 && enforcementPorts[0] == networkEvent.Port { + if !hasPortSnapshot { if networkEvent.Destination.Kind == EndpointKindService && serviceWorkload != nil && k8sClient != nil { enforcementPorts = resolveServiceEnforcementPorts( k8sClient, diff --git a/pkg/containerprofilemanager/v1/monitoring.go b/pkg/containerprofilemanager/v1/monitoring.go index 40654c2305..7719506741 100644 --- a/pkg/containerprofilemanager/v1/monitoring.go +++ b/pkg/containerprofilemanager/v1/monitoring.go @@ -268,23 +268,17 @@ func (cpm *ContainerProfileManager) saveContainerProfile(watchedContainer *objec return nil } - // Update timestamps before saving - watchedContainer.PreviousReportTimestamp = watchedContainer.CurrentReportTimestamp - watchedContainer.CurrentReportTimestamp = time.Now() - containerProfile := &v1beta1.ContainerProfile{ Name: slug, Namespace: container.K8s.Namespace, Annotations: map[string]string{ - helpersv1.InstanceIDMetadataKey: watchedContainer.InstanceID.GetStringFormatted(), - helpersv1.WlidMetadataKey: watchedContainer.Wlid, - helpersv1.CompletionMetadataKey: string(watchedContainer.GetCompletionStatus()), - helpersv1.StatusMetadataKey: string(watchedContainer.GetStatus()), - helpersv1.ContainerTypeMetadataKey: watchedContainer.ContainerType.String(), - helpersv1.ReportSeriesIdMetadataKey: watchedContainer.SeriesID, - helpersv1.PreviousReportTimestampMetadataKey: watchedContainer.PreviousReportTimestamp.String(), - helpersv1.ReportTimestampMetadataKey: watchedContainer.CurrentReportTimestamp.String(), - helpersv1.OtelSpanIDMetadataKey: cpm.lifecycleTracker.LearningSpanID(watchedContainer.ContainerID), + helpersv1.InstanceIDMetadataKey: watchedContainer.InstanceID.GetStringFormatted(), + helpersv1.WlidMetadataKey: watchedContainer.Wlid, + helpersv1.CompletionMetadataKey: string(watchedContainer.GetCompletionStatus()), + helpersv1.StatusMetadataKey: string(watchedContainer.GetStatus()), + helpersv1.ContainerTypeMetadataKey: watchedContainer.ContainerType.String(), + helpersv1.ReportSeriesIdMetadataKey: watchedContainer.SeriesID, + helpersv1.OtelSpanIDMetadataKey: cpm.lifecycleTracker.LearningSpanID(watchedContainer.ContainerID), // Full W3C traceparent so kubescape/storage can create a properly // parented child span for the aggregation step. helpersv1.OtelTraceparentMetadataKey: cpm.lifecycleTracker.LearningTraceparent(watchedContainer.ContainerID), @@ -325,6 +319,13 @@ func (cpm *ContainerProfileManager) saveContainerProfile(watchedContainer *objec return nil } + // Advance the report chain only after deciding to emit this profile. + // Deferred-only flushes must not create a link to a report that was skipped. + watchedContainer.PreviousReportTimestamp = watchedContainer.CurrentReportTimestamp + watchedContainer.CurrentReportTimestamp = time.Now() + containerProfile.Annotations[helpersv1.PreviousReportTimestampMetadataKey] = watchedContainer.PreviousReportTimestamp.String() + containerProfile.Annotations[helpersv1.ReportTimestampMetadataKey] = watchedContainer.CurrentReportTimestamp.String() + if err := cpm.enqueueContainerProfile(containerProfile, watchedContainer.ContainerID); err != nil { // Empty the container data to prevent reporting the same data again containerData.emptyEvents() diff --git a/pkg/containerprofilemanager/v1/monitoring_timestamp_test.go b/pkg/containerprofilemanager/v1/monitoring_timestamp_test.go new file mode 100644 index 0000000000..52c0fc817f --- /dev/null +++ b/pkg/containerprofilemanager/v1/monitoring_timestamp_test.go @@ -0,0 +1,73 @@ +package containerprofilemanager + +import ( + "context" + "testing" + "time" + + mapset "github.com/deckarep/golang-set/v2" + helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" + "github.com/kubescape/node-agent/pkg/config" + "github.com/kubescape/node-agent/pkg/hostidentity" + "github.com/kubescape/node-agent/pkg/seccompmanager" + "github.com/kubescape/node-agent/pkg/storage" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSaveContainerProfile_DeferredOnlyPreservesReportTimestamps(t *testing.T) { + for _, priorReport := range []bool{false, true} { + name := "first report" + if priorReport { + name = "after prior report" + } + t.Run(name, func(t *testing.T) { + t.Setenv("QUEUE_DIR", t.TempDir()) + storageClient := &storage.StorageHttpClientMock{} + cpm, err := NewContainerProfileManager(context.Background(), config.Config{}, nil, nil, + storageClient, nil, &seccompmanager.SeccompManagerMock{}, nil, nil, nil) + require.NoError(t, err) + t.Cleanup(cpm.Close) + + watched := hostidentity.BuildHostWatchedContainerData("node-1") + container := hostContainerWithIdentity(newHostPseudoContainer(), watched, "kubescape") + data := &containerData{watchedContainerData: watched} + wantReports := 1 + if priorReport { + data.syscalls = mapset.NewSet("openat") + require.NoError(t, cpm.saveContainerProfile(watched, container, data, false)) + wantReports++ + } + previous, current := watched.PreviousReportTimestamp, watched.CurrentReportTimestamp + data.networks = mapset.NewSet(NetworkEvent{ + Port: 443, Protocol: "tcp", PktType: utils.OutgoingPktType, + Destination: Destination{Kind: EndpointKindRaw, IPAddress: "10.50.1.20"}, + }) + + // An unresolved private peer skips this flush but must not create a link + // to a report that was never queued. + require.NoError(t, cpm.saveContainerProfile(watched, container, data, false)) + assert.Equal(t, previous, watched.PreviousReportTimestamp) + assert.Equal(t, current, watched.CurrentReportTimestamp) + require.NotNil(t, data.networks) + require.Equal(t, 1, data.networks.Cardinality()) + + // The next flush emits the retained peer with the last emitted timestamp. + require.NoError(t, cpm.saveContainerProfile(watched, container, data, false)) + assert.Equal(t, current, watched.PreviousReportTimestamp) + require.Eventually(t, func() bool { + return len(storageClient.ContainerProfilesSnapshot()) == wantReports + }, 8*time.Second, 10*time.Millisecond) + profiles := storageClient.ContainerProfilesSnapshot() + last := profiles[len(profiles)-1] + assert.Equal(t, current.String(), last.Annotations[helpersv1.PreviousReportTimestampMetadataKey]) + assert.Equal(t, watched.CurrentReportTimestamp.String(), last.Annotations[helpersv1.ReportTimestampMetadataKey]) + require.Len(t, last.Spec.Egress, 1) + assert.Equal(t, "10.50.1.20", last.Spec.Egress[0].IPAddress) + if priorReport { + assert.Equal(t, profiles[0].Annotations[helpersv1.ReportTimestampMetadataKey], last.Annotations[helpersv1.PreviousReportTimestampMetadataKey]) + } + }) + } +} diff --git a/pkg/containerprofilemanager/v1/peer_resolution_test.go b/pkg/containerprofilemanager/v1/peer_resolution_test.go index 0e97bd04e4..7844174e60 100644 --- a/pkg/containerprofilemanager/v1/peer_resolution_test.go +++ b/pkg/containerprofilemanager/v1/peer_resolution_test.go @@ -3,6 +3,11 @@ package containerprofilemanager import ( "testing" + mapset "github.com/deckarep/golang-set/v2" + "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" + discoveryv1 "k8s.io/api/discovery/v1" + "k8s.io/utils/ptr" + "github.com/inspektor-gadget/inspektor-gadget/pkg/operators/common" "github.com/inspektor-gadget/inspektor-gadget/pkg/types" "github.com/kubescape/node-agent/pkg/objectcache" @@ -403,3 +408,55 @@ func TestMonitoring_ReResolutionAtProfileFlush(t *testing.T) { require.NotNil(t, ingress[0].PodSelector) assert.Equal(t, map[string]string{"app": "wikijs"}, ingress[0].PodSelector.MatchLabels) } + +func TestNetworkNeighbors_MergeDistinctPortsAfterResolution(t *testing.T) { + for _, direction := range []string{utils.HostPktType, utils.OutgoingPktType} { + t.Run(direction, func(t *testing.T) { + inv := newMockK8sInventory() + inv.podsByIP["10.244.0.14"] = &common.SlimPod{SlimObjectMeta: common.SlimObjectMeta{ + Name: "peer", Namespace: "default", Labels: map[string]string{"app": "peer"}, + }} + cd := &containerData{networks: mapset.NewSet[NetworkEvent]()} + raw := NetworkEvent{Port: 80, Protocol: "tcp", PktType: direction, + Destination: Destination{Kind: EndpointKindRaw, IPAddress: "10.244.0.14"}} + cd.networks.Add(raw) + resolved := raw + resolveEndpoint(&resolved, inv, nil) + cd.networks.Add(resolved) // Same peer and port from a later, resolved observation. + resolved.Port = 443 + cd.networks.Add(resolved) + resolved.Port = 80 + resolved.Protocol = "udp" + cd.networks.Add(resolved) + var neighbors []v1beta1.NetworkNeighbor + if direction == utils.HostPktType { + neighbors = cd.getIngressNetworkNeighbors("", "default", nil, nil, inv, nil, false) + } else { + neighbors = cd.getEgressNetworkNeighbors("", "default", nil, nil, inv, nil, false) + } + require.Len(t, neighbors, 1) + names := make([]string, 0, len(neighbors[0].Ports)) + for _, port := range neighbors[0].Ports { + names = append(names, port.Name) + } + require.ElementsMatch(t, []string{"tcp-80", "tcp-443", "udp-80"}, names) + }) + } +} + +func TestCreateNetworkNeighbor_PreservesSnapshotEqualToObservedPort(t *testing.T) { + event := serviceNetworkEvent(80, "tcp") + client := &servicePortTestClient{ + service: newServiceWorkload("api", map[string]any{"app": "api"}, map[string]any{ + "name": "web", "port": 80, "targetPort": "http", "protocol": "TCP", + }), + kubeClient: fake.NewClientset(newEndpointSlice("api-new", "api", discoveryv1.EndpointPort{ + Name: ptr.To("web"), Port: ptr.To(int32(8080)), Protocol: ptr.To(corev1.ProtocolTCP), + })), + } + cd := &containerData{servicePorts: map[NetworkEvent][]uint16{event: {80}}} + neighbor := cd.createNetworkNeighbor("", event, "default", client, nil, nil, nil, false) + require.NotNil(t, neighbor) + require.Equal(t, []int32{80}, networkPortValues(neighbor.Ports)) + require.Empty(t, client.kubeClient.Actions(), "cached snapshots must not query changed EndpointSlices") +} From 1564663aa4f8881baab74c21d0806dfd0f7b6534 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 12:53:54 +0200 Subject: [PATCH 03/19] fix(cpm): address Copilot review 5413292601 Retain raw IP observations when Service promotion fails, index fallback pod lookups, and apply the configured estimated size budget after late peer resolution using existing queue splits. Cover fallback retries, cache lifecycle, expanded Service ports, and report chain preservation. Signed-off-by: Matthias Bertschy --- .../v1/container_data.go | 52 +++++----- .../v1/late_resolution_size.go | 16 +++ .../v1/late_resolution_size_test.go | 97 ++++++++++++++++++ pkg/containerprofilemanager/v1/monitoring.go | 10 +- .../v1/peer_resolution_test.go | 49 +++++++++- .../v1/queue/containerprofile_budget_test.go | 51 ++++++++++ .../v1/queue/containerprofile_queue.go | 41 ++++++-- .../integration_helpers_test.go | 2 + .../containerprofilecache/reconciler_test.go | 1 + pkg/objectcache/k8scache/k8scache.go | 57 ++++++++++- pkg/objectcache/k8scache/pod_ip_test.go | 98 +++++++++++++++++++ pkg/objectcache/k8scache_interface.go | 15 +++ pkg/objectcache/v1/mock.go | 11 +++ 13 files changed, 462 insertions(+), 38 deletions(-) create mode 100644 pkg/containerprofilemanager/v1/late_resolution_size.go create mode 100644 pkg/containerprofilemanager/v1/late_resolution_size_test.go create mode 100644 pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go create mode 100644 pkg/objectcache/k8scache/pod_ip_test.go diff --git a/pkg/containerprofilemanager/v1/container_data.go b/pkg/containerprofilemanager/v1/container_data.go index 1d22ba9122..248f46006f 100644 --- a/pkg/containerprofilemanager/v1/container_data.go +++ b/pkg/containerprofilemanager/v1/container_data.go @@ -213,14 +213,12 @@ func resolveEndpoint( } if k8sObjectCache != nil { - for _, pod := range k8sObjectCache.GetPods() { - if pod != nil && !pod.Spec.HostNetwork && pod.Status.PodIP == ip { - event.Destination.Kind = EndpointKindPod - event.Destination.Name = pod.Name - event.Destination.Namespace = pod.Namespace - event.SetDestinationPodLabels(pod.Labels) - return - } + if pod := k8sObjectCache.GetPodByIP(ip); pod != nil && !pod.Spec.HostNetwork { + event.Destination.Kind = EndpointKindPod + event.Destination.Name = pod.Name + event.Destination.Namespace = pod.Namespace + event.SetDestinationPodLabels(pod.Labels) + return } } } @@ -310,6 +308,7 @@ func (cd *containerData) createNetworkNeighbor( k8sObjectCache objectcache.K8sObjectCache, forceSend bool, ) *v1beta1.NetworkNeighbor { + originalEvent := networkEvent resolveEndpoint(&networkEvent, k8sInventory, k8sObjectCache) var neighborEntry v1beta1.NetworkNeighbor @@ -338,22 +337,25 @@ func (cd *containerData) createNetworkNeighbor( logger.L().Warning("failed to get service", helpers.String("reason", err.Error()), helpers.String("service name", networkEvent.Destination.Name)) - return nil - } - serviceWorkload = svc - - if svc.GetName() == "kubernetes" && svc.GetNamespace() == "default" { - // The default service has no selectors, in addition, we want to save the default service address - selector = svc.GetLabels() - neighborEntry.IPAddress = networkEvent.Destination.IPAddress - } else { - selector = svc.GetServiceSelector() + } else if svc != nil { + serviceWorkload = svc + + if svc.GetName() == "kubernetes" && svc.GetNamespace() == "default" { + // The default service has no selectors, in addition, we want to save the default service address + selector = svc.GetLabels() + neighborEntry.IPAddress = networkEvent.Destination.IPAddress + } else { + selector = svc.GetServiceSelector() + } } } if len(selector) == 0 { - // TODO: check if we need to handle services with no selectors - return nil + // Preserve observed IP traffic when promotion cannot provide a selector. + if networkEvent.Destination.IPAddress == "" { + return nil + } + networkEvent.Destination.Kind = EndpointKindRaw } else { neighborEntry.PodSelector = &metav1.LabelSelector{ MatchLabels: selector, @@ -365,7 +367,9 @@ func (cd *containerData) createNetworkNeighbor( } } - } else { + } + + if networkEvent.Destination.Kind != EndpointKindPod && networkEvent.Destination.Kind != EndpointKindService { if networkEvent.Destination.IPAddress == "127.0.0.1" { // No need to generate for localhost return nil @@ -375,11 +379,11 @@ func (cd *containerData) createNetworkNeighbor( // defer by one flush before persisting as an external IP, unless forceSend is true // or it has already been deferred once. if isPrivateIP(networkEvent.Destination.IPAddress) && !forceSend && cd != nil && - (cd.prevDeferredNetworks == nil || !cd.prevDeferredNetworks.Contains(networkEvent)) { + (cd.prevDeferredNetworks == nil || !cd.prevDeferredNetworks.Contains(originalEvent)) { if cd.deferredNetworks == nil { cd.deferredNetworks = mapset.NewSet[NetworkEvent]() } - cd.deferredNetworks.Add(networkEvent) + cd.deferredNetworks.Add(originalEvent) return nil } @@ -395,7 +399,7 @@ func (cd *containerData) createNetworkNeighbor( } hasPortSnapshot := false - if cd != nil { + if cd != nil && networkEvent.Destination.Kind == EndpointKindService { if ports, ok := cd.servicePorts[networkEvent]; ok { hasPortSnapshot = true enforcementPorts = ports diff --git a/pkg/containerprofilemanager/v1/late_resolution_size.go b/pkg/containerprofilemanager/v1/late_resolution_size.go new file mode 100644 index 0000000000..4478d20762 --- /dev/null +++ b/pkg/containerprofilemanager/v1/late_resolution_size.go @@ -0,0 +1,16 @@ +package containerprofilemanager + +// Raw observations may acquire selectors and additional Service ports only at +// flush time. Apply the size budget to their materialized profile, which also +// freezes those ports across any subsequent queue splits. +func hasUnresolvedNetworkPeers(data *containerData) bool { + if data.networks == nil { + return false + } + for _, event := range data.networks.ToSlice() { + if event.Destination.Kind != EndpointKindPod && event.Destination.Kind != EndpointKindService { + return true + } + } + return false +} diff --git a/pkg/containerprofilemanager/v1/late_resolution_size_test.go b/pkg/containerprofilemanager/v1/late_resolution_size_test.go new file mode 100644 index 0000000000..9f16a6a0b2 --- /dev/null +++ b/pkg/containerprofilemanager/v1/late_resolution_size_test.go @@ -0,0 +1,97 @@ +package containerprofilemanager + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + "github.com/DmitriyVTitov/size" + mapset "github.com/deckarep/golang-set/v2" + "github.com/inspektor-gadget/inspektor-gadget/pkg/operators/common" + helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" + "github.com/kubescape/node-agent/pkg/config" + "github.com/kubescape/node-agent/pkg/hostidentity" + "github.com/kubescape/node-agent/pkg/seccompmanager" + "github.com/kubescape/node-agent/pkg/storage" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/require" + discoveryv1 "k8s.io/api/discovery/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func TestSaveContainerProfile_LateResolutionSizeBudget(t *testing.T) { + for _, kind := range []EndpointKind{EndpointKindPod, EndpointKindService} { + t.Run(string(kind), func(t *testing.T) { + t.Setenv("QUEUE_DIR", t.TempDir()) + sink := &storage.StorageHttpClientMock{} + manager, err := NewContainerProfileManager(context.Background(), config.Config{}, nil, nil, sink, nil, &seccompmanager.SeccompManagerMock{}, nil, nil, nil) + require.NoError(t, err) + t.Cleanup(manager.Close) + watched := hostidentity.BuildHostWatchedContainerData("node-1") + container := hostContainerWithIdentity(newHostPseudoContainer(), watched, "kubescape") + data := &containerData{watchedContainerData: watched, networks: mapset.NewSet[NetworkEvent]()} + inventory := newMockK8sInventory() + manager.k8sInventory = inventory + labels := map[string]string{} + serviceLabels := map[string]any{} + for i := range 40 { + key := fmt.Sprintf("label-%d", i) + labels[key] = strings.Repeat("v", 63) + serviceLabels[key] = labels[key] + } + if kind == EndpointKindService { + manager.k8sClient = &servicePortTestClient{service: newServiceWorkload("api", serviceLabels, map[string]any{"name": "web", "port": 80, "targetPort": "http", "protocol": "TCP"}), kubeClient: fake.NewClientset()} + } + for i := range 2 { + ip := fmt.Sprintf("10.0.0.%d", i+1) + namespace := fmt.Sprintf("peer-%d", i) + event := NetworkEvent{Port: 80, Protocol: "tcp", PktType: utils.OutgoingPktType, Destination: Destination{Kind: EndpointKindRaw, IPAddress: ip}} + data.networks.Add(event) + data.size.Add(int64(size.Of(event) + networkNeighborIncrement(data, event))) + if kind == EndpointKindService { + for j, port := range []int32{8080, 9090, 10000} { + slice := newEndpointSlice(fmt.Sprintf("slice-%d", j), "api", discoveryv1.EndpointPort{Name: new("web"), Port: new(port)}) + slice.Namespace = namespace + _, err := manager.k8sClient.(*servicePortTestClient).kubeClient.DiscoveryV1().EndpointSlices(namespace).Create(context.Background(), slice, metav1.CreateOptions{}) + require.NoError(t, err) + } + } + meta := common.SlimObjectMeta{Name: "api", Namespace: namespace, Labels: labels} + if kind == EndpointKindPod { + inventory.podsByIP[ip] = &common.SlimPod{SlimObjectMeta: meta, Status: common.SlimPodStatus{PodIP: ip}} + } else { + inventory.svcsByIP[ip] = &common.SlimService{SlimObjectMeta: meta, Spec: common.SlimServiceSpec{ClusterIP: ip}} + } + } + // Each resolved peer fits, but their combined labels exceed the raw-IP budget. + neighbors := data.getEgressNetworkNeighbors(watched.ContainerID, container.K8s.Namespace, manager.k8sClient, nil, inventory, nil, false) + require.Len(t, neighbors, 2) + manager.cfg.MaxTsProfileSize = int64(size.Of(neighbors[0])*3/2 + 1000) + require.Less(t, data.size.Load(), manager.cfg.MaxTsProfileSize) + require.Greater(t, int64(size.Of(neighbors)), manager.cfg.MaxTsProfileSize) + require.NoError(t, manager.saveContainerProfile(watched, container, data, false)) + if kind == EndpointKindService { + for i := range 2 { + require.NoError(t, manager.k8sClient.(*servicePortTestClient).kubeClient.DiscoveryV1().EndpointSlices(fmt.Sprintf("peer-%d", i)).Delete(context.Background(), "slice-2", metav1.DeleteOptions{})) + } + } + require.Eventually(t, func() bool { return len(sink.ContainerProfilesSnapshot()) >= 2 }, 16*time.Second, 10*time.Millisecond) + profiles := sink.ContainerProfilesSnapshot() + require.Len(t, profiles, 2) + previous := time.Time{}.String() + for _, profile := range profiles { + require.LessOrEqual(t, int64(size.Of(profile.Spec)), manager.cfg.MaxTsProfileSize) + require.Len(t, profile.Spec.Egress, 1) + if kind == EndpointKindService { + require.Equal(t, []int32{8080, 9090, 10000}, networkPortValues(profile.Spec.Egress[0].Ports)) + } + require.Equal(t, previous, profile.Annotations[helpersv1.PreviousReportTimestampMetadataKey]) + previous = profile.Annotations[helpersv1.ReportTimestampMetadataKey] + } + require.Equal(t, watched.CurrentReportTimestamp.String(), previous) + }) + } +} diff --git a/pkg/containerprofilemanager/v1/monitoring.go b/pkg/containerprofilemanager/v1/monitoring.go index 7719506741..41e87916ca 100644 --- a/pkg/containerprofilemanager/v1/monitoring.go +++ b/pkg/containerprofilemanager/v1/monitoring.go @@ -326,10 +326,16 @@ func (cpm *ContainerProfileManager) saveContainerProfile(watchedContainer *objec containerProfile.Annotations[helpersv1.PreviousReportTimestampMetadataKey] = watchedContainer.PreviousReportTimestamp.String() containerProfile.Annotations[helpersv1.ReportTimestampMetadataKey] = watchedContainer.CurrentReportTimestamp.String() - if err := cpm.enqueueContainerProfile(containerProfile, watchedContainer.ContainerID); err != nil { + var enqueueErr error + if hasUnresolvedNetworkPeers(containerData) { + enqueueErr = cpm.queueData.EnqueueWithSizeLimit(containerProfile, watchedContainer.ContainerID, cpm.cfg.MaxTsProfileSize) + } else { + enqueueErr = cpm.enqueueContainerProfile(containerProfile, watchedContainer.ContainerID) + } + if enqueueErr != nil { // Empty the container data to prevent reporting the same data again containerData.emptyEvents() - return err + return enqueueErr } cpm.lifecycleTracker.OnEntrySaved(watchedContainer.ContainerID, containerData.droppedEvents) diff --git a/pkg/containerprofilemanager/v1/peer_resolution_test.go b/pkg/containerprofilemanager/v1/peer_resolution_test.go index 7844174e60..507a4113cf 100644 --- a/pkg/containerprofilemanager/v1/peer_resolution_test.go +++ b/pkg/containerprofilemanager/v1/peer_resolution_test.go @@ -1,6 +1,7 @@ package containerprofilemanager import ( + "errors" "testing" mapset "github.com/deckarep/golang-set/v2" @@ -21,13 +22,24 @@ import ( type stubK8sObjectCache struct { objectcache.K8sObjectCacheMock - pods []*corev1.Pod + pods []*corev1.Pod + getPodsCalls int } func (s *stubK8sObjectCache) GetPods() []*corev1.Pod { + s.getPodsCalls++ return s.pods } +func (s *stubK8sObjectCache) GetPodByIP(ip string) *corev1.Pod { + for _, pod := range s.pods { + if pod != nil && pod.Status.PodIP == ip { + return pod + } + } + return nil +} + type mockK8sInventory struct { podsByIP map[string]*common.SlimPod svcsByIP map[string]*common.SlimService @@ -206,6 +218,7 @@ func TestCreateNetworkNeighbor_RawPodIP_ResolvedViaK8sObjectCache(t *testing.T) assert.Equal(t, InternalTrafficType, string(neighbor.Type)) require.NotNil(t, neighbor.PodSelector) assert.Equal(t, map[string]string{"app": "wikijs"}, neighbor.PodSelector.MatchLabels) + require.Zero(t, mockCache.getPodsCalls, "fallback lookup must use the IP index") } func TestCreateNetworkNeighbor_RawServiceIP_ResolvedViaK8sInventory(t *testing.T) { @@ -460,3 +473,37 @@ func TestCreateNetworkNeighbor_PreservesSnapshotEqualToObservedPort(t *testing.T require.Equal(t, []int32{80}, networkPortValues(neighbor.Ports)) require.Empty(t, client.kubeClient.Actions(), "cached snapshots must not query changed EndpointSlices") } + +func TestCreateNetworkNeighbor_ServicePromotionPreservesRawFallback(t *testing.T) { + for _, lookupFailure := range []bool{false, true} { + name := "selectorless service" + if lookupFailure { + name = "lookup failure" + } + t.Run(name, func(t *testing.T) { + inv := newMockK8sInventory() + inv.svcsByIP["10.96.0.42"] = &common.SlimService{SlimObjectMeta: common.SlimObjectMeta{Name: "api", Namespace: "default"}} + client := &servicePortTestClient{service: newServiceWorkload("api", nil)} + if lookupFailure { + client.getErr = errors.New("transient lookup failure") + } + for _, kind := range []EndpointKind{EndpointKindRaw, EndpointKindService} { + event := NetworkEvent{Port: 80, Protocol: "tcp", PktType: utils.OutgoingPktType, + Destination: Destination{Kind: kind, IPAddress: "10.96.0.42", Namespace: "default", Name: "api"}} + cd := &containerData{} + require.Nil(t, cd.createNetworkNeighbor("", event, "default", client, nil, inv, nil, false)) + require.NotNil(t, cd.deferredNetworks, "failed promotion must retain the observation") + require.True(t, cd.deferredNetworks.Contains(event)) + cd.emptyEvents() + neighbor := cd.createNetworkNeighbor("", event, "default", client, nil, inv, nil, false) + require.NotNil(t, neighbor, "retry is bounded to one flush") + require.Equal(t, ExternalTrafficType, string(neighbor.Type)) + require.Equal(t, "10.96.0.42", neighbor.IPAddress) + require.Equal(t, []int32{80}, networkPortValues(neighbor.Ports)) + final := (&containerData{}).createNetworkNeighbor("", event, "default", client, nil, inv, nil, true) + require.NotNil(t, final, "forced final flush must preserve raw IP") + require.Equal(t, "10.96.0.42", final.IPAddress) + } + }) + } +} diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go new file mode 100644 index 0000000000..ac3d793da6 --- /dev/null +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go @@ -0,0 +1,51 @@ +package queue + +import ( + "context" + "strings" + "testing" + "time" + + helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" + "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" + "github.com/stretchr/testify/require" +) + +func TestQueueSizeBudgetRetainsUnsplittableAndDepthLimitedData(t *testing.T) { + for _, count := range []int{1, 8} { + name := "unsplittable" + if count > 1 { + name = "depth limit" + } + t.Run(name, func(t *testing.T) { + creator := &MockProfileCreator{} + q, err := NewQueueData(context.Background(), creator, QueueConfig{QueueName: "budget", QueueDir: t.TempDir(), MaxQueueSize: 100, ItemsPerSegment: 10, MaxSplitDepth: 2}) + require.NoError(t, err) + defer q.Close() + original := &v1beta1.ContainerProfile{Name: "profile", Annotations: map[string]string{ + helpersv1.PreviousReportTimestampMetadataKey: time.Time{}.String(), + helpersv1.ReportTimestampMetadataKey: time.Now().String(), + }} + for i := range count { + original.Spec.Syscalls = append(original.Spec.Syscalls, strings.Repeat(string(rune('a'+i)), 100)) + } + require.NoError(t, q.EnqueueWithSizeLimit(original, "container", 1)) + // Process deterministically without the queue goroutine. The inherited budget + // must split again on the second pass, then send despite the remaining overage. + for range 3 { + q.processAllItems() + } + profiles := creator.CreatedProfiles() + require.Len(t, profiles, min(count, 4)) + var syscalls []string + var rows []tsRow + for _, p := range profiles { + syscalls = append(syscalls, p.Spec.Syscalls...) + rows = append(rows, tsRow{PreviousReportTimestamp: p.Annotations[helpersv1.PreviousReportTimestampMetadataKey], ReportTimestamp: p.Annotations[helpersv1.ReportTimestampMetadataKey]}) + } + require.ElementsMatch(t, original.Spec.Syscalls, syscalls) + assertChainIsLinear(t, rows, original.Annotations[helpersv1.PreviousReportTimestampMetadataKey], original.Annotations[helpersv1.ReportTimestampMetadataKey]) + require.Zero(t, q.GetQueueSize()) + }) + } +} diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go index 2790caa395..b56b76b8f9 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go @@ -11,6 +11,7 @@ import ( "sync/atomic" "time" + "github.com/DmitriyVTitov/size" "github.com/joncrlsn/dque" "github.com/kubescape/go-logger" "github.com/kubescape/go-logger/helpers" @@ -65,7 +66,11 @@ type QueuedContainerProfile struct { // retryable error. Items persisted before this field existed decode with Attempts // at zero, so they simply get a full budget of retries. Attempts int `json:"attempts"` - // SplitDepth counts how many times this item's lineage has been halved after an HTTP 413. + // MaxProfileSize is an optional in-memory size estimate for profiles whose + // network peers expanded after collection. Zero disables proactive splitting. + MaxProfileSize int64 `json:"maxProfileSize,omitempty"` + // SplitDepth counts how many times this item's lineage has been halved, either + // proactively for MaxProfileSize or after an HTTP 413. // Items persisted before this field existed decode with SplitDepth at zero. SplitDepth int `json:"splitDepth"` // IsStitch marks a metadata-only chunk emitted in place of a chunk that was dropped or @@ -283,12 +288,20 @@ func (qd *QueueData) Start() { // Enqueue adds a new container profile to the queue with LRU eviction func (qd *QueueData) Enqueue(profile *v1beta1.ContainerProfile, containerID string) error { + return qd.EnqueueWithSizeLimit(profile, containerID, 0) +} + +// EnqueueWithSizeLimit applies an estimated size budget before sending to storage. +// The existing splitter preserves the materialized data and report chain; profiles +// that cannot be split within the depth limit are still offered to storage. +func (qd *QueueData) EnqueueWithSizeLimit(profile *v1beta1.ContainerProfile, containerID string, maxProfileSize int64) error { qd.mu.Lock() defer qd.mu.Unlock() if err := qd.enqueueLocked(&QueuedContainerProfile{ - Profile: profile, - ContainerID: containerID, + Profile: profile, + ContainerID: containerID, + MaxProfileSize: maxProfileSize, }); err != nil { return err } @@ -547,6 +560,17 @@ processLoop: qd.releaseStitch() } + if !queuedProfile.IsStitch && queuedProfile.MaxProfileSize > 0 && + queuedProfile.SplitDepth < qd.maxSplitDepth && + int64(size.Of(queuedProfile.Profile.Spec)) > queuedProfile.MaxProfileSize { + if a, b, ok := splitProfile(queuedProfile.Profile); ok { + qd.splits.Add(1) + qd.metrics.ReportContainerProfileSplit() + qd.requeueSplit(queuedProfile, a, b) + continue + } + } + // Attempt to create the profile err = qd.creator.CreateContainerProfileDirect(queuedProfile.Profile) if err != nil { @@ -650,11 +674,12 @@ func (qd *QueueData) requeueImmediate(queuedProfile *QueuedContainerProfile) { func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1.ContainerProfile) { half := func(profile *v1beta1.ContainerProfile) *QueuedContainerProfile { return &QueuedContainerProfile{ - Profile: profile, - ContainerID: parent.ContainerID, - Attempts: parent.Attempts, - SplitDepth: parent.SplitDepth + 1, - IsStitch: false, + Profile: profile, + ContainerID: parent.ContainerID, + Attempts: parent.Attempts, + SplitDepth: parent.SplitDepth + 1, + MaxProfileSize: parent.MaxProfileSize, + IsStitch: false, } } diff --git a/pkg/objectcache/containerprofilecache/integration_helpers_test.go b/pkg/objectcache/containerprofilecache/integration_helpers_test.go index 6895a5813e..b9cfbd1378 100644 --- a/pkg/objectcache/containerprofilecache/integration_helpers_test.go +++ b/pkg/objectcache/containerprofilecache/integration_helpers_test.go @@ -99,6 +99,8 @@ func (k *stubK8sCache) GetPodStatus(_, _ string) *corev1.PodStatus { return nil func (k *stubK8sCache) GetApiServerIpAddress() string { return "" } func (k *stubK8sCache) GetPods() []*corev1.Pod { return nil } +func (k *stubK8sCache) GetPodByIP(_ string) *corev1.Pod { return nil } + func (k *stubK8sCache) SetSharedContainerData(id string, d *objectcache.WatchedContainerData) { k.mu.Lock() defer k.mu.Unlock() diff --git a/pkg/objectcache/containerprofilecache/reconciler_test.go b/pkg/objectcache/containerprofilecache/reconciler_test.go index f591c0a772..3b9a6a952c 100644 --- a/pkg/objectcache/containerprofilecache/reconciler_test.go +++ b/pkg/objectcache/containerprofilecache/reconciler_test.go @@ -55,6 +55,7 @@ func (k *controllableK8sCache) GetPodSpec(_, _ string) *corev1.PodSpec { ret func (k *controllableK8sCache) GetPodStatus(_, _ string) *corev1.PodStatus { return nil } func (k *controllableK8sCache) GetApiServerIpAddress() string { return "" } func (k *controllableK8sCache) GetPods() []*corev1.Pod { return nil } +func (k *controllableK8sCache) GetPodByIP(_ string) *corev1.Pod { return nil } func (k *controllableK8sCache) SetSharedContainerData(_ string, _ *objectcache.WatchedContainerData) { } func (k *controllableK8sCache) GetSharedContainerData(_ string) *objectcache.WatchedContainerData { diff --git a/pkg/objectcache/k8scache/k8scache.go b/pkg/objectcache/k8scache/k8scache.go index 1f9b11e7f3..539a37f08c 100644 --- a/pkg/objectcache/k8scache/k8scache.go +++ b/pkg/objectcache/k8scache/k8scache.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "sync" "github.com/kubescape/node-agent/pkg/k8sclient" "github.com/kubescape/node-agent/pkg/objectcache" @@ -24,6 +25,8 @@ type K8sObjectCacheImpl struct { nodeName string k8sClient k8sclient.K8sClientInterface pods maps.SafeMap[string, *corev1.Pod] + podMu sync.RWMutex + podsByIP map[string]*corev1.Pod apiServerIpAddress string containerIDToSharedData maps.SafeMap[string, *objectcache.WatchedContainerData] } @@ -73,6 +76,13 @@ func (k *K8sObjectCacheImpl) GetPod(namespace, podName string) *corev1.Pod { return nil } +// GetPodByIP returns a cached pod by its primary or secondary IP in constant time. +func (k *K8sObjectCacheImpl) GetPodByIP(ip string) *corev1.Pod { + k.podMu.RLock() + defer k.podMu.RUnlock() + return k.podsByIP[ip] +} + func (k *K8sObjectCacheImpl) GetApiServerIpAddress() string { return k.apiServerIpAddress } @@ -99,19 +109,60 @@ func (k *K8sObjectCacheImpl) DeleteSharedContainerData(containerID string) { func (k *K8sObjectCacheImpl) AddHandler(_ context.Context, obj runtime.Object) { if pod, ok := obj.(*corev1.Pod); ok { - k.pods.Set(podKey(pod.GetNamespace(), pod.GetName()), pod) + k.storePod(pod) } } func (k *K8sObjectCacheImpl) ModifyHandler(_ context.Context, obj runtime.Object) { if pod, ok := obj.(*corev1.Pod); ok { - k.pods.Set(podKey(pod.GetNamespace(), pod.GetName()), pod) + k.storePod(pod) } } func (k *K8sObjectCacheImpl) DeleteHandler(_ context.Context, obj runtime.Object) { if pod, ok := obj.(*corev1.Pod); ok { - k.pods.Delete(podKey(pod.GetNamespace(), pod.GetName())) + k.podMu.Lock() + defer k.podMu.Unlock() + key := podKey(pod.GetNamespace(), pod.GetName()) + current, ok := k.pods.Load(key) + if !ok || current.UID != pod.UID { + return + } + k.removePodIPs(current) + k.pods.Delete(key) + } +} + +func (k *K8sObjectCacheImpl) storePod(pod *corev1.Pod) { + k.podMu.Lock() + defer k.podMu.Unlock() + key := podKey(pod.GetNamespace(), pod.GetName()) + if previous, ok := k.pods.Load(key); ok { + k.removePodIPs(previous) + } + k.pods.Set(key, pod) + if k.podsByIP == nil { + k.podsByIP = make(map[string]*corev1.Pod) + } + if pod.Status.PodIP != "" { + k.podsByIP[pod.Status.PodIP] = pod + } + for _, ip := range pod.Status.PodIPs { + if ip.IP != "" { + k.podsByIP[ip.IP] = pod + } + } +} + +// removePodIPs requires podMu and preserves IPs already assigned to another pod. +func (k *K8sObjectCacheImpl) removePodIPs(pod *corev1.Pod) { + if k.podsByIP[pod.Status.PodIP] == pod { + delete(k.podsByIP, pod.Status.PodIP) + } + for _, ip := range pod.Status.PodIPs { + if k.podsByIP[ip.IP] == pod { + delete(k.podsByIP, ip.IP) + } } } diff --git a/pkg/objectcache/k8scache/pod_ip_test.go b/pkg/objectcache/k8scache/pod_ip_test.go new file mode 100644 index 0000000000..fd78de4cf1 --- /dev/null +++ b/pkg/objectcache/k8scache/pod_ip_test.go @@ -0,0 +1,98 @@ +package k8scache + +import ( + "context" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +func indexedPod(name, uid, ip string) *corev1.Pod { + return &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: name, UID: types.UID(uid)}, Status: corev1.PodStatus{PodIP: ip}} +} + +func TestPodIPIndexLifecycle(t *testing.T) { + k := &K8sObjectCacheImpl{} + ctx := context.Background() + pod := indexedPod("pod", "first", "10.0.0.1") + pod.Status.PodIPs = []corev1.PodIP{{IP: "10.0.0.1"}, {IP: "fd00::1"}} + assert.Nil(t, k.GetPodByIP("10.0.0.1")) + k.AddHandler(ctx, pod) + assert.Same(t, pod, k.GetPodByIP("10.0.0.1")) + assert.Same(t, pod, k.GetPodByIP("fd00::1")) + assert.Nil(t, k.GetPodByIP("")) + updated := pod.DeepCopy() + updated.Status.PodIP = "10.0.0.2" + updated.Status.PodIPs = nil + k.ModifyHandler(ctx, updated) + assert.Nil(t, k.GetPodByIP("10.0.0.1")) + assert.Nil(t, k.GetPodByIP("fd00::1")) + assert.Same(t, updated, k.GetPodByIP("10.0.0.2")) + // Delete payloads may predate the latest status update. + k.DeleteHandler(ctx, pod) + assert.Nil(t, k.GetPodByIP("10.0.0.2")) + assert.Nil(t, k.GetPod("default", "pod")) +} + +func TestPodIPIndexReuse(t *testing.T) { + for _, sameName := range []bool{false, true} { + t.Run(map[bool]string{false: "different pods", true: "recreated pod"}[sameName], func(t *testing.T) { + k := &K8sObjectCacheImpl{} + ctx := context.Background() + old := indexedPod("old", "first", "10.0.0.1") + replacement := indexedPod("new", "second", old.Status.PodIP) + if sameName { + replacement.Name = old.Name + } + k.AddHandler(ctx, old) + k.AddHandler(ctx, replacement) + k.DeleteHandler(ctx, old) + assert.Same(t, replacement, k.GetPodByIP(old.Status.PodIP)) + assert.Same(t, replacement, k.GetPod("default", replacement.Name)) + k.DeleteHandler(ctx, replacement) + assert.Nil(t, k.GetPodByIP(old.Status.PodIP)) + }) + } +} + +func TestPodIPIndexUpdatePreservesReusedIP(t *testing.T) { + k := &K8sObjectCacheImpl{} + ctx := context.Background() + old := indexedPod("old", "first", "10.0.0.1") + replacement := indexedPod("new", "second", "10.0.0.1") + k.AddHandler(ctx, old) + k.AddHandler(ctx, replacement) + updated := old.DeepCopy() + updated.Status.PodIP = "" + updated.Status.PodIPs = []corev1.PodIP{{IP: "fd00::1"}, {IP: ""}} + k.ModifyHandler(ctx, updated) + assert.Same(t, replacement, k.GetPodByIP("10.0.0.1")) + assert.Same(t, updated, k.GetPodByIP("fd00::1")) + assert.Nil(t, k.GetPodByIP("")) + k.DeleteHandler(ctx, updated) + assert.Same(t, replacement, k.GetPodByIP("10.0.0.1")) + assert.Nil(t, k.GetPodByIP("fd00::1")) +} + +func TestPodIPIndexConcurrentAccess(t *testing.T) { + k := &K8sObjectCacheImpl{} + var wg sync.WaitGroup + for i := 0; i < 4; i++ { + wg.Add(1) + go func() { + defer wg.Done() + pod := indexedPod("pod", "uid", "10.0.0.1") + for j := 0; j < 100; j++ { + k.AddHandler(context.Background(), pod) + k.GetPodByIP(pod.Status.PodIP) + k.ModifyHandler(context.Background(), pod) + k.DeleteHandler(context.Background(), pod) + } + }() + } + wg.Wait() +} diff --git a/pkg/objectcache/k8scache_interface.go b/pkg/objectcache/k8scache_interface.go index 68647d85b0..95827f3d4a 100644 --- a/pkg/objectcache/k8scache_interface.go +++ b/pkg/objectcache/k8scache_interface.go @@ -10,6 +10,7 @@ type K8sObjectCache interface { GetPodStatus(namespace, podName string) *corev1.PodStatus GetApiServerIpAddress() string GetPods() []*corev1.Pod + GetPodByIP(ip string) *corev1.Pod GetPod(namespace, podName string) *corev1.Pod SetSharedContainerData(containerID string, data *WatchedContainerData) GetSharedContainerData(containerID string) *WatchedContainerData @@ -34,6 +35,20 @@ func (k *K8sObjectCacheMock) GetPodStatus(_, _ string) *corev1.PodStatus { func (k *K8sObjectCacheMock) GetPod(_, _ string) *corev1.Pod { return &corev1.Pod{Spec: k.PodSpec, Status: k.PodStatus} } +func (k *K8sObjectCacheMock) GetPodByIP(ip string) *corev1.Pod { + if ip == "" { + return nil + } + if k.PodStatus.PodIP == ip { + return k.GetPod("", "") + } + for _, podIP := range k.PodStatus.PodIPs { + if podIP.IP == ip { + return k.GetPod("", "") + } + } + return nil +} func (k *K8sObjectCacheMock) GetApiServerIpAddress() string { return k.ApiServerIpAddress } diff --git a/pkg/objectcache/v1/mock.go b/pkg/objectcache/v1/mock.go index 30fffed084..2b0f40ba9f 100644 --- a/pkg/objectcache/v1/mock.go +++ b/pkg/objectcache/v1/mock.go @@ -275,6 +275,17 @@ func (r *RuleObjectCacheMock) GetPods() []*corev1.Pod { return []*corev1.Pod{{Spec: *r.podSpec, Status: *r.podStatus}} } +func (r *RuleObjectCacheMock) GetPodByIP(ip string) *corev1.Pod { + if ip == "" || r.podStatus == nil { + return nil + } + k := objectcache.K8sObjectCacheMock{PodStatus: *r.podStatus} + if r.podSpec != nil { + k.PodSpec = *r.podSpec + } + return k.GetPodByIP(ip) +} + func (r *RuleObjectCacheMock) SetSharedContainerData(containerID string, data *objectcache.WatchedContainerData) { r.ContainerIDToSharedData.Set(containerID, data) } From a0c188bb860b86ec4c3ac496dd34133e5f7867a0 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 13:05:13 +0200 Subject: [PATCH 04/19] fix(cpm): address Copilot review 5413479461 Preserve replacement Pod IP ownership across old status updates and retain captured Service ports for deferred observations. Add regressions for primary/secondary IP reuse and EndpointSlice changes between flushes. Signed-off-by: Matthias Bertschy --- .../v1/container_data.go | 11 ++++- .../v1/peer_resolution_test.go | 28 +++++++++++++ pkg/objectcache/k8scache/k8scache.go | 27 +++++++++--- pkg/objectcache/k8scache/pod_ip_test.go | 41 +++++++++++++++++++ 4 files changed, 100 insertions(+), 7 deletions(-) diff --git a/pkg/containerprofilemanager/v1/container_data.go b/pkg/containerprofilemanager/v1/container_data.go index 248f46006f..1d40d303a7 100644 --- a/pkg/containerprofilemanager/v1/container_data.go +++ b/pkg/containerprofilemanager/v1/container_data.go @@ -33,6 +33,15 @@ func (cd *containerData) emptyEvents() { cd.networks = cd.deferredNetworks.Clone() cd.prevDeferredNetworks = cd.deferredNetworks.Clone() cd.deferredNetworks = nil + // Retained observations must keep the ports captured at ingestion. + for event := range cd.servicePorts { + if !cd.networks.Contains(event) { + delete(cd.servicePorts, event) + } + } + if len(cd.servicePorts) == 0 { + cd.servicePorts = nil + } var retainedSize int64 for _, ev := range cd.networks.ToSlice() { retainedSize += int64(size.Of(ev) + networkNeighborIncrement(cd, ev)) @@ -42,8 +51,8 @@ func (cd *containerData) emptyEvents() { cd.networks = nil cd.prevDeferredNetworks = nil cd.deferredNetworks = nil + cd.servicePorts = nil } - cd.servicePorts = nil if cd.watchedContainerData != nil { cd.lastReportedCompletion = string(cd.watchedContainerData.GetCompletionStatus()) cd.lastReportedStatus = string(cd.watchedContainerData.GetStatus()) diff --git a/pkg/containerprofilemanager/v1/peer_resolution_test.go b/pkg/containerprofilemanager/v1/peer_resolution_test.go index 507a4113cf..35fff85234 100644 --- a/pkg/containerprofilemanager/v1/peer_resolution_test.go +++ b/pkg/containerprofilemanager/v1/peer_resolution_test.go @@ -4,6 +4,7 @@ import ( "errors" "testing" + "github.com/DmitriyVTitov/size" mapset "github.com/deckarep/golang-set/v2" "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" discoveryv1 "k8s.io/api/discovery/v1" @@ -507,3 +508,30 @@ func TestCreateNetworkNeighbor_ServicePromotionPreservesRawFallback(t *testing.T }) } } + +func TestEmptyEvents_RetainsDeferredServicePortSnapshot(t *testing.T) { + event := serviceNetworkEvent(80, "tcp") + event.Destination.IPAddress = "10.96.0.42" + discarded := serviceNetworkEvent(443, "tcp") + cd := &containerData{servicePorts: map[NetworkEvent][]uint16{event: {8080, 9090}, discarded: {8443}}} + client := &servicePortTestClient{getErr: errors.New("transient lookup failure")} + require.Nil(t, cd.createNetworkNeighbor("", event, "default", client, nil, nil, nil, false)) + cd.emptyEvents() + require.Equal(t, map[NetworkEvent][]uint16{event: {8080, 9090}}, cd.servicePorts) + require.Equal(t, int64(size.Of(event)+networkNeighborIncrement(cd, event)), cd.size.Load()) + + // EndpointSlices change while the Service lookup recovers. + client.getErr = nil + client.service = newServiceWorkload("api", map[string]any{"app": "api"}, map[string]any{ + "name": "web", "port": 80, "targetPort": "http", "protocol": "TCP", + }) + client.kubeClient = fake.NewClientset(newEndpointSlice("api-new", "api", discoveryv1.EndpointPort{ + Name: ptr.To("web"), Port: ptr.To(int32(10000)), Protocol: ptr.To(corev1.ProtocolTCP), + })) + neighbor := cd.createNetworkNeighbor("", event, "default", client, nil, nil, nil, false) + require.NotNil(t, neighbor) + require.Equal(t, []int32{8080, 9090}, networkPortValues(neighbor.Ports)) + require.Empty(t, client.kubeClient.Actions()) + cd.emptyEvents() + require.Nil(t, cd.servicePorts, "snapshots clear when their observations are emitted") +} diff --git a/pkg/objectcache/k8scache/k8scache.go b/pkg/objectcache/k8scache/k8scache.go index 539a37f08c..6e7a6b9deb 100644 --- a/pkg/objectcache/k8scache/k8scache.go +++ b/pkg/objectcache/k8scache/k8scache.go @@ -137,20 +137,35 @@ func (k *K8sObjectCacheImpl) storePod(pod *corev1.Pod) { k.podMu.Lock() defer k.podMu.Unlock() key := podKey(pod.GetNamespace(), pod.GetName()) - if previous, ok := k.pods.Load(key); ok { + previous, ok := k.pods.Load(key) + if ok { k.removePodIPs(previous) } k.pods.Set(key, pod) if k.podsByIP == nil { k.podsByIP = make(map[string]*corev1.Pod) } - if pod.Status.PodIP != "" { - k.podsByIP[pod.Status.PodIP] = pod + indexIP := func(ip string) { + if ip == "" { + return + } + // A status update retaining an old IP must not reclaim it after reuse. + // New pods and newly assigned IPs can replace the previous owner. + if k.podsByIP[ip] != nil && previous != nil && previous.UID == pod.UID { + if previous.Status.PodIP == ip { + return + } + for _, oldIP := range previous.Status.PodIPs { + if oldIP.IP == ip { + return + } + } + } + k.podsByIP[ip] = pod } + indexIP(pod.Status.PodIP) for _, ip := range pod.Status.PodIPs { - if ip.IP != "" { - k.podsByIP[ip.IP] = pod - } + indexIP(ip.IP) } } diff --git a/pkg/objectcache/k8scache/pod_ip_test.go b/pkg/objectcache/k8scache/pod_ip_test.go index fd78de4cf1..3f5b2a049a 100644 --- a/pkg/objectcache/k8scache/pod_ip_test.go +++ b/pkg/objectcache/k8scache/pod_ip_test.go @@ -96,3 +96,44 @@ func TestPodIPIndexConcurrentAccess(t *testing.T) { } wg.Wait() } + +func TestPodIPIndexRetainedReassignedIP(t *testing.T) { + for _, secondary := range []bool{false, true} { + t.Run(map[bool]string{false: "primary", true: "secondary"}[secondary], func(t *testing.T) { + k := &K8sObjectCacheImpl{} + ctx := context.Background() + ip := "10.0.0.1" + old := indexedPod("old", "first", ip) + if secondary { + ip = "fd00::1" + old.Status.PodIPs = []corev1.PodIP{{IP: old.Status.PodIP}, {IP: ip}} + } + replacement := indexedPod("new", "second", ip) + k.AddHandler(ctx, old) + k.AddHandler(ctx, replacement) + updated := old.DeepCopy() + updated.Labels = map[string]string{"updated": "true"} + k.ModifyHandler(ctx, updated) + assert.Same(t, replacement, k.GetPodByIP(ip)) + k.DeleteHandler(ctx, updated) + assert.Same(t, replacement, k.GetPodByIP(ip)) + }) + } +} + +func TestPodIPIndexNewAssignmentTakesOwnership(t *testing.T) { + k := &K8sObjectCacheImpl{} + ctx := context.Background() + old := indexedPod("old", "first", "10.0.0.1") + replacement := indexedPod("new", "second", "10.0.0.2") + k.AddHandler(ctx, old) + k.AddHandler(ctx, replacement) + updated := replacement.DeepCopy() + updated.Status.PodIP = old.Status.PodIP + k.ModifyHandler(ctx, updated) + assert.Same(t, updated, k.GetPodByIP(old.Status.PodIP)) + assert.Nil(t, k.GetPodByIP(replacement.Status.PodIP)) + recreated := indexedPod(replacement.Name, "third", old.Status.PodIP) + k.AddHandler(ctx, recreated) + assert.Same(t, recreated, k.GetPodByIP(old.Status.PodIP)) +} From 8baa483423a6b6986e51a41065315a7904415c8f Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 13:15:38 +0200 Subject: [PATCH 05/19] perf(cpm): address Copilot review 5413610359 Use per-neighbor port membership sets instead of scanning merged port slices. Add a 4000-port scan benchmark preserving every observed port; measured flush time improves from approximately 14 ms to 2.8 ms. Signed-off-by: Matthias Bertschy --- .../v1/container_data.go | 27 ++++++++++++------- .../v1/peer_resolution_test.go | 15 +++++++++++ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/pkg/containerprofilemanager/v1/container_data.go b/pkg/containerprofilemanager/v1/container_data.go index 1d40d303a7..1ffe90d4b5 100644 --- a/pkg/containerprofilemanager/v1/container_data.go +++ b/pkg/containerprofilemanager/v1/container_data.go @@ -2,7 +2,6 @@ package containerprofilemanager import ( "net" - "slices" "sort" "github.com/DmitriyVTitov/size" @@ -247,7 +246,7 @@ func (cd *containerData) getIngressNetworkNeighbors( return ingress } - seen := make(map[string]int) + seen := make(map[string]networkNeighborIndex) for _, event := range cd.networks.ToSlice() { if event.PktType == utils.HostPktType { neighbor := cd.createNetworkNeighbor(containerID, event, namespace, k8sClient, dnsResolverClient, k8sInventory, k8sObjectCache, forceSend) @@ -276,7 +275,7 @@ func (cd *containerData) getEgressNetworkNeighbors( return egress } - seen := make(map[string]int) + seen := make(map[string]networkNeighborIndex) for _, event := range cd.networks.ToSlice() { if event.PktType != utils.HostPktType { neighbor := cd.createNetworkNeighbor(containerID, event, namespace, k8sClient, dnsResolverClient, k8sInventory, k8sObjectCache, forceSend) @@ -290,19 +289,27 @@ func (cd *containerData) getEgressNetworkNeighbors( return egress } +type networkNeighborIndex struct { + index int + ports map[string]struct{} +} + // appendNetworkNeighbor merges all observed ports for neighbors with the same identity. -func appendNetworkNeighbor(neighbors []v1beta1.NetworkNeighbor, seen map[string]int, neighbor v1beta1.NetworkNeighbor) []v1beta1.NetworkNeighbor { - if index, ok := seen[neighbor.Identifier]; ok { +func appendNetworkNeighbor(neighbors []v1beta1.NetworkNeighbor, seen map[string]networkNeighborIndex, neighbor v1beta1.NetworkNeighbor) []v1beta1.NetworkNeighbor { + if entry, ok := seen[neighbor.Identifier]; ok { for _, port := range neighbor.Ports { - if !slices.ContainsFunc(neighbors[index].Ports, func(existing v1beta1.NetworkPort) bool { - return existing.Name == port.Name - }) { - neighbors[index].Ports = append(neighbors[index].Ports, port) + if _, exists := entry.ports[port.Name]; !exists { + neighbors[entry.index].Ports = append(neighbors[entry.index].Ports, port) + entry.ports[port.Name] = struct{}{} } } return neighbors } - seen[neighbor.Identifier] = len(neighbors) + ports := make(map[string]struct{}, len(neighbor.Ports)) + for _, port := range neighbor.Ports { + ports[port.Name] = struct{}{} + } + seen[neighbor.Identifier] = networkNeighborIndex{index: len(neighbors), ports: ports} return append(neighbors, neighbor) } diff --git a/pkg/containerprofilemanager/v1/peer_resolution_test.go b/pkg/containerprofilemanager/v1/peer_resolution_test.go index 35fff85234..8027e236af 100644 --- a/pkg/containerprofilemanager/v1/peer_resolution_test.go +++ b/pkg/containerprofilemanager/v1/peer_resolution_test.go @@ -535,3 +535,18 @@ func TestEmptyEvents_RetainsDeferredServicePortSnapshot(t *testing.T) { cd.emptyEvents() require.Nil(t, cd.servicePorts, "snapshots clear when their observations are emitted") } + +func BenchmarkNetworkNeighborsPortScan(b *testing.B) { + cd := &containerData{networks: mapset.NewSet[NetworkEvent]()} + for port := 1; port <= 4000; port++ { + cd.networks.Add(NetworkEvent{Port: uint16(port), Protocol: "tcp", PktType: utils.OutgoingPktType, + Destination: Destination{Kind: EndpointKindRaw, IPAddress: "93.184.216.34"}}) + } + b.ResetTimer() + for b.Loop() { + neighbors := cd.getEgressNetworkNeighbors("", "default", nil, nil, nil, nil, false) + if len(neighbors) != 1 || len(neighbors[0].Ports) != 4000 { + b.Fatal("port-scan observations were lost") + } + } +} From 3d5860ead0c6bea0a408bce441a517ea314fbe8c Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 13:25:04 +0200 Subject: [PATCH 06/19] fix(cpm): address Copilot review 5413716935 Split ports within a single merged neighbor so HTTP 413 retries preserve every observed connection. Retain peer identity and report chain; cover ingress, egress, proactive budgets, odd port counts, and mixed profile fields. Signed-off-by: Matthias Bertschy --- .../containerprofile_ports_split_test.go | 78 +++++++++++++++++++ .../v1/queue/containerprofile_split.go | 38 +++++++-- .../v1/queue/containerprofile_split_test.go | 26 ++++++- 3 files changed, 132 insertions(+), 10 deletions(-) create mode 100644 pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go new file mode 100644 index 0000000000..9a6b334555 --- /dev/null +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go @@ -0,0 +1,78 @@ +package queue + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/DmitriyVTitov/size" + helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" + "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type portLimitedCreator struct{ accepted []*v1beta1.ContainerProfile } + +func (c *portLimitedCreator) CreateContainerProfileDirect(p *v1beta1.ContainerProfile) error { + for _, n := range append(append([]v1beta1.NetworkNeighbor{}, p.Spec.Ingress...), p.Spec.Egress...) { + if len(n.Ports) > 2 { + return genericStatusError(http.StatusRequestEntityTooLarge) + } + } + c.accepted = append(c.accepted, p.DeepCopy()) + return nil +} + +func portSplitNeighbor() v1beta1.NetworkNeighbor { + n := v1beta1.NetworkNeighbor{Identifier: "peer", Type: "internal", PodSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "api"}}} + for i := range 8 { + n.Ports = append(n.Ports, v1beta1.NetworkPort{Name: fmt.Sprintf("TCP-%d", 8000+i), Protocol: "TCP", Port: new(int32(8000 + i))}) + } + return n +} + +func TestQueueSplitsSingleNeighborPorts(t *testing.T) { + for _, direction := range []string{"ingress", "egress"} { + for _, proactive := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/proactive=%t", direction, proactive), func(t *testing.T) { + creator := &portLimitedCreator{} + q, err := NewQueueData(context.Background(), creator, QueueConfig{QueueName: "ports", QueueDir: t.TempDir(), MaxQueueSize: 100, ItemsPerSegment: 10}) + require.NoError(t, err) + defer q.Close() + profile := testProfile() + neighbor := portSplitNeighbor() + if direction == "ingress" { + profile.Spec.Ingress = []v1beta1.NetworkNeighbor{neighbor} + } else { + profile.Spec.Egress = []v1beta1.NetworkNeighbor{neighbor} + } + if proactive { + require.NoError(t, q.EnqueueWithSizeLimit(profile, "container", int64(size.Of(profile.Spec)-size.Of(neighbor.Ports)/2))) + } else { + require.NoError(t, q.Enqueue(profile, "container")) + } + for range DefaultMaxSplitDepth + 2 { + q.processAllItems() + } + var ports []v1beta1.NetworkPort + var rows []tsRow + for _, p := range creator.accepted { + peers := p.Spec.Ingress + if direction == "egress" { + peers = p.Spec.Egress + } + require.Len(t, peers, 1, "must deliver peer data instead of a replacement stitch") + require.Equal(t, neighbor.Identifier, peers[0].Identifier) + require.Equal(t, neighbor.PodSelector, peers[0].PodSelector) + ports = append(ports, peers[0].Ports...) + rows = append(rows, tsRow{PreviousReportTimestamp: p.Annotations[helpersv1.PreviousReportTimestampMetadataKey], ReportTimestamp: p.Annotations[helpersv1.ReportTimestampMetadataKey]}) + } + require.ElementsMatch(t, neighbor.Ports, ports) + assertChainIsLinear(t, rows, profile.Annotations[helpersv1.PreviousReportTimestampMetadataKey], profile.Annotations[helpersv1.ReportTimestampMetadataKey]) + require.Zero(t, q.GetQueueSize()) + }) + } + } +} diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_split.go b/pkg/containerprofilemanager/v1/queue/containerprofile_split.go index 4e8bd2d629..c35b1d10df 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_split.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_split.go @@ -77,13 +77,13 @@ func splitProfile(p *v1beta1.ContainerProfile) (*v1beta1.ContainerProfile, *v1be a.Spec.Syscalls, b.Spec.Syscalls = halve(p.Spec.Syscalls) a.Spec.Endpoints, b.Spec.Endpoints = halve(p.Spec.Endpoints) a.Spec.IdentifiedCallStacks, b.Spec.IdentifiedCallStacks = halve(p.Spec.IdentifiedCallStacks) - a.Spec.Ingress, b.Spec.Ingress = halve(p.Spec.Ingress) - a.Spec.Egress, b.Spec.Egress = halve(p.Spec.Egress) + a.Spec.Ingress, b.Spec.Ingress = halveNeighbors(p.Spec.Ingress) + a.Spec.Egress, b.Spec.Egress = halveNeighbors(p.Spec.Egress) a.Spec.PolicyByRuleId, b.Spec.PolicyByRuleId = halvePolicies(p.Spec.PolicyByRuleId) - // Every field with len <= 1 leaves its single element in a, so b can come out empty even - // though p had two or more elements. Move one element across, otherwise recursion on a - // would not strictly reduce and the split would make no progress. + // Fields with one indivisible element leave it in a, so b can come out empty + // even though p had two or more elements. Move one element across, otherwise + // recursion on a would not strictly reduce and the split would make no progress. if countPartitionableElements(&b.Spec) == 0 { moveOneElement(&a.Spec, &b.Spec) } @@ -223,7 +223,7 @@ func isZeroTimeString(s string) bool { } // countPartitionableElements returns the total number of elements across every list and -// map field that splitProfile partitions. +// map field that splitProfile partitions, counting each neighbor port separately. func countPartitionableElements(spec *v1beta1.ContainerProfileSpec) int { return len(spec.Capabilities) + len(spec.Execs) + @@ -231,11 +231,33 @@ func countPartitionableElements(spec *v1beta1.ContainerProfileSpec) int { len(spec.Syscalls) + len(spec.Endpoints) + len(spec.IdentifiedCallStacks) + - len(spec.Ingress) + - len(spec.Egress) + + countNeighborElements(spec.Ingress) + + countNeighborElements(spec.Egress) + len(spec.PolicyByRuleId) } +// A peer without ports still carries an identity that must be retained. +func countNeighborElements(neighbors []v1beta1.NetworkNeighbor) int { + count := 0 + for _, neighbor := range neighbors { + count += max(1, len(neighbor.Ports)) + } + return count +} + +// Split peer lists first. Once a single peer remains, its ports can still be +// partitioned while copying the identity into both chunks for storage to merge. +func halveNeighbors(neighbors []v1beta1.NetworkNeighbor) ([]v1beta1.NetworkNeighbor, []v1beta1.NetworkNeighbor) { + if len(neighbors) != 1 || len(neighbors[0].Ports) <= 1 { + return halve(neighbors) + } + a, b := neighbors[0].DeepCopy(), neighbors[0].DeepCopy() + mid := (len(a.Ports) + 1) / 2 + a.Ports = a.Ports[:mid] + b.Ports = b.Ports[mid:] + return []v1beta1.NetworkNeighbor{*a}, []v1beta1.NetworkNeighbor{*b} +} + // moveOneElement transfers a single element from the first non-empty partitionable field of // from into to, so that a split of a spec with at least two elements never yields an empty half. func moveOneElement(from, to *v1beta1.ContainerProfileSpec) { diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go index 7e5df3e04c..3a5a922680 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go @@ -153,10 +153,20 @@ func elementSignatures(spec *v1beta1.ContainerProfileSpec) []string { out = append(out, "callstack:"+string(c.CallID)) } for _, n := range spec.Ingress { - out = append(out, "ingress:"+n.Identifier) + if len(n.Ports) == 0 { + out = append(out, "ingress:"+n.Identifier) + } + for _, port := range n.Ports { + out = append(out, "ingress:"+n.Identifier+":"+port.Name) + } } for _, n := range spec.Egress { - out = append(out, "egress:"+n.Identifier) + if len(n.Ports) == 0 { + out = append(out, "egress:"+n.Identifier) + } + for _, port := range n.Ports { + out = append(out, "egress:"+n.Identifier+":"+port.Name) + } } for k := range spec.PolicyByRuleId { out = append(out, "policy:"+k) @@ -193,6 +203,18 @@ func TestSplitProfile_PartitionsWithoutLossOrDuplication(t *testing.T) { {"egress", func(p *v1beta1.ContainerProfile) { p.Spec.Egress = []v1beta1.NetworkNeighbor{{Identifier: "a"}, {Identifier: "b"}, {Identifier: "c"}} }}, + {"merged neighbor ports", func(p *v1beta1.ContainerProfile) { + neighbor := portSplitNeighbor() + neighbor.Ports = neighbor.Ports[:7] + p.Spec.Ingress = []v1beta1.NetworkNeighbor{neighbor} + p.Spec.Egress = []v1beta1.NetworkNeighbor{neighbor} + }}, + {"mixed peers and fields", func(p *v1beta1.ContainerProfile) { + neighbor := portSplitNeighbor() + p.Spec.Ingress = []v1beta1.NetworkNeighbor{neighbor} + p.Spec.Egress = []v1beta1.NetworkNeighbor{neighbor, {Identifier: "second"}} + p.Spec.Capabilities = []string{"CAP_SYS_ADMIN"} + }}, {"policyByRuleId", func(p *v1beta1.ContainerProfile) { p.Spec.PolicyByRuleId = map[string]v1beta1.RulePolicy{ "a": {AllowedProcesses: []string{"x"}}, From 50bc912b9b502016e1171db57776cafcc9aa16fe Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 13:36:28 +0200 Subject: [PATCH 07/19] fix(cpm): address CodeRabbit review 5413700702 Admit proactive splits only when both halves fit under the queue lock; send the original without evicting pending data when capacity is insufficient. Add regressions for full queues and producer contention, and document PR-touched functions for the review coverage check. Signed-off-by: Matthias Bertschy --- .../v1/container_data.go | 2 + .../v1/container_data_test.go | 3 + .../v1/containerprofile_manager.go | 16 +-- .../v1/containerprofile_manager_test.go | 1 + .../v1/event_reporting_test.go | 2 + .../v1/late_resolution_size_test.go | 1 + .../v1/monitoring_timestamp_test.go | 1 + .../v1/peer_resolution_test.go | 28 ++++- .../v1/queue/containerprofile_budget_test.go | 1 + .../containerprofile_ports_split_test.go | 3 + .../v1/queue/containerprofile_queue.go | 21 +++- .../containerprofile_queue_errors_test.go | 2 +- .../containerprofile_queue_proactive_test.go | 108 ++++++++++++++++++ .../v1/queue/containerprofile_split_test.go | 1 + .../integration_helpers_test.go | 1 + .../containerprofilecache/reconciler_test.go | 4 +- pkg/objectcache/k8scache/k8scache.go | 4 + pkg/objectcache/k8scache/pod_ip_test.go | 7 ++ pkg/objectcache/k8scache_interface.go | 2 + pkg/objectcache/v1/mock.go | 1 + 20 files changed, 192 insertions(+), 17 deletions(-) create mode 100644 pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go diff --git a/pkg/containerprofilemanager/v1/container_data.go b/pkg/containerprofilemanager/v1/container_data.go index 1ffe90d4b5..7e6d9e53a5 100644 --- a/pkg/containerprofilemanager/v1/container_data.go +++ b/pkg/containerprofilemanager/v1/container_data.go @@ -185,11 +185,13 @@ func (cd *containerData) getCallStacks() []v1beta1.IdentifiedCallStack { return callStacks } +// isPrivateIP reports whether a valid address belongs to a private IPv4 or IPv6 range. func isPrivateIP(ipStr string) bool { ip := net.ParseIP(ipStr) return ip != nil && ip.IsPrivate() } +// resolveEndpoint resolves unknown peers from inventory, then the pod cache, excluding host-network pods. func resolveEndpoint( event *NetworkEvent, k8sInventory common.K8sInventoryCache, diff --git a/pkg/containerprofilemanager/v1/container_data_test.go b/pkg/containerprofilemanager/v1/container_data_test.go index b8876f6a03..493029c928 100644 --- a/pkg/containerprofilemanager/v1/container_data_test.go +++ b/pkg/containerprofilemanager/v1/container_data_test.go @@ -121,6 +121,7 @@ func serviceNetworkEvent(port uint16, protocol string) NetworkEvent { } } +// TestCreateNetworkNeighbor_ServiceTargetPortMatrix checks Service port remapping and observed-port fallbacks across protocols and endpoint sources. func TestCreateNetworkNeighbor_ServiceTargetPortMatrix(t *testing.T) { tests := []struct { name string @@ -284,6 +285,7 @@ func TestCreateNetworkNeighbor_ServiceTargetPortMatrix(t *testing.T) { } } +// TestCreateNetworkNeighbor_NonServiceDestinationsUnchanged checks that pod and raw-IP peers retain their observed ports. func TestCreateNetworkNeighbor_NonServiceDestinationsUnchanged(t *testing.T) { cd := &containerData{} @@ -315,6 +317,7 @@ func TestCreateNetworkNeighbor_NonServiceDestinationsUnchanged(t *testing.T) { require.Equal(t, []int32{443}, networkPortValues(rawNeighbor.Ports)) } +// TestGenerateNetworkPolicy_ServiceTargetPortRoundTrip checks that generated policies use the backend target port instead of the Service port. func TestGenerateNetworkPolicy_ServiceTargetPortRoundTrip(t *testing.T) { service := newServiceWorkload("api", map[string]any{"app.kubernetes.io/name": "api"}, map[string]any{ "port": 80, "targetPort": 8080, "protocol": "TCP", diff --git a/pkg/containerprofilemanager/v1/containerprofile_manager.go b/pkg/containerprofilemanager/v1/containerprofile_manager.go index 6528ceeb33..ee17e83d4a 100644 --- a/pkg/containerprofilemanager/v1/containerprofile_manager.go +++ b/pkg/containerprofilemanager/v1/containerprofile_manager.go @@ -55,13 +55,13 @@ type containerData struct { timer *time.Timer // For max sniffing time // Events reported for this container that need to be saved to the profile - capabilites mapset.Set[string] - syscalls mapset.Set[string] - endpoints *maps.SafeMap[string, *v1beta1.HTTPEndpoint] - execs *maps.SafeMap[string, []string] // Map of execs, key is SHA256 hash - opens *maps.SafeMap[string, mapset.Set[string]] // Map of opens, key is file path - rulePolicies *maps.SafeMap[string, *v1beta1.RulePolicy] // Map of rule policies, key is rule ID - callStacks *maps.SafeMap[string, *v1beta1.IdentifiedCallStack] // Map of callstacks, key is SHA256 hash + capabilites mapset.Set[string] + syscalls mapset.Set[string] + endpoints *maps.SafeMap[string, *v1beta1.HTTPEndpoint] + execs *maps.SafeMap[string, []string] // Map of execs, key is SHA256 hash + opens *maps.SafeMap[string, mapset.Set[string]] // Map of opens, key is file path + rulePolicies *maps.SafeMap[string, *v1beta1.RulePolicy] // Map of rule policies, key is rule ID + callStacks *maps.SafeMap[string, *v1beta1.IdentifiedCallStack] // Map of callstacks, key is SHA256 hash networks mapset.Set[NetworkEvent] deferredNetworks mapset.Set[NetworkEvent] prevDeferredNetworks mapset.Set[NetworkEvent] @@ -221,7 +221,7 @@ func NewContainerProfileManager( return containerProfileManager, nil } -// Stop stops the container profile manager +// Close stops container timers, the persistent queue, and the Kubernetes inventory. func (cpm *ContainerProfileManager) Close() { // Stop all container timers and clear container map cpm.containersMu.Lock() diff --git a/pkg/containerprofilemanager/v1/containerprofile_manager_test.go b/pkg/containerprofilemanager/v1/containerprofile_manager_test.go index dd228c9b68..1a2f1d11fa 100644 --- a/pkg/containerprofilemanager/v1/containerprofile_manager_test.go +++ b/pkg/containerprofilemanager/v1/containerprofile_manager_test.go @@ -545,6 +545,7 @@ func TestContainerProfileManagerCreation(t *testing.T) { assert.NotNil(t, cpm.maxSniffTimeNotificationChan) } +// TestContainerDataMethods checks that an empty container produces no profile events or network neighbors. func TestContainerDataMethods(t *testing.T) { cd := &containerData{} diff --git a/pkg/containerprofilemanager/v1/event_reporting_test.go b/pkg/containerprofilemanager/v1/event_reporting_test.go index d590a808d8..dc867c32f0 100644 --- a/pkg/containerprofilemanager/v1/event_reporting_test.go +++ b/pkg/containerprofilemanager/v1/event_reporting_test.go @@ -236,6 +236,7 @@ func (r *trackingDNSResolver) ResolveContainerProcessToCloudServices(string, uin return nil } +// TestCreateNetworkNeighbor_EmptyContainerIDWithWatchedContainerData checks that DNS lookup preserves an explicitly empty container ID. func TestCreateNetworkNeighbor_EmptyContainerIDWithWatchedContainerData(t *testing.T) { cd := &containerData{ watchedContainerData: &objectcache.WatchedContainerData{ @@ -259,6 +260,7 @@ func TestCreateNetworkNeighbor_EmptyContainerIDWithWatchedContainerData(t *testi assert.Equal(t, "resolved.domain", neighbor.DNS) } +// TestReportNetworkEventServicePortMultiplicity checks that all backend ports count toward the size budget and stay fixed within a batch. func TestReportNetworkEventServicePortMultiplicity(t *testing.T) { cpm, entry := newTestManager(t, "container1") client := &servicePortTestClient{ diff --git a/pkg/containerprofilemanager/v1/late_resolution_size_test.go b/pkg/containerprofilemanager/v1/late_resolution_size_test.go index 9f16a6a0b2..b15974aadd 100644 --- a/pkg/containerprofilemanager/v1/late_resolution_size_test.go +++ b/pkg/containerprofilemanager/v1/late_resolution_size_test.go @@ -22,6 +22,7 @@ import ( "k8s.io/client-go/kubernetes/fake" ) +// TestSaveContainerProfile_LateResolutionSizeBudget checks that resolved peers split within budget while preserving ports and report order. func TestSaveContainerProfile_LateResolutionSizeBudget(t *testing.T) { for _, kind := range []EndpointKind{EndpointKindPod, EndpointKindService} { t.Run(string(kind), func(t *testing.T) { diff --git a/pkg/containerprofilemanager/v1/monitoring_timestamp_test.go b/pkg/containerprofilemanager/v1/monitoring_timestamp_test.go index 52c0fc817f..9ec760a279 100644 --- a/pkg/containerprofilemanager/v1/monitoring_timestamp_test.go +++ b/pkg/containerprofilemanager/v1/monitoring_timestamp_test.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/require" ) +// TestSaveContainerProfile_DeferredOnlyPreservesReportTimestamps checks that a skipped flush leaves no timestamp link to an unqueued report. func TestSaveContainerProfile_DeferredOnlyPreservesReportTimestamps(t *testing.T) { for _, priorReport := range []bool{false, true} { name := "first report" diff --git a/pkg/containerprofilemanager/v1/peer_resolution_test.go b/pkg/containerprofilemanager/v1/peer_resolution_test.go index 8027e236af..984eed81b7 100644 --- a/pkg/containerprofilemanager/v1/peer_resolution_test.go +++ b/pkg/containerprofilemanager/v1/peer_resolution_test.go @@ -27,11 +27,13 @@ type stubK8sObjectCache struct { getPodsCalls int } +// GetPods records list calls so tests can detect fallback scans. func (s *stubK8sObjectCache) GetPods() []*corev1.Pod { s.getPodsCalls++ return s.pods } +// GetPodByIP finds a test pod by primary IP without calling GetPods. func (s *stubK8sObjectCache) GetPodByIP(ip string) *corev1.Pod { for _, pod := range s.pods { if pod != nil && pod.Status.PodIP == ip { @@ -46,6 +48,7 @@ type mockK8sInventory struct { svcsByIP map[string]*common.SlimService } +// newMockK8sInventory creates mutable pod and Service indexes for simulating inventory updates. func newMockK8sInventory() *mockK8sInventory { return &mockK8sInventory{ podsByIP: make(map[string]*common.SlimPod), @@ -53,9 +56,13 @@ func newMockK8sInventory() *mockK8sInventory { } } +// Start is a no-op because tests populate the inventory directly. func (m *mockK8sInventory) Start() {} -func (m *mockK8sInventory) Stop() {} +// Stop is a no-op because the test inventory has no background workers. +func (m *mockK8sInventory) Stop() {} + +// GetPods returns the pods currently present in the test inventory. func (m *mockK8sInventory) GetPods() []*common.SlimPod { var pods []*common.SlimPod for _, p := range m.podsByIP { @@ -64,6 +71,7 @@ func (m *mockK8sInventory) GetPods() []*common.SlimPod { return pods } +// GetPodByName finds a test pod by namespace and name, returning nil when absent. func (m *mockK8sInventory) GetPodByName(namespace string, name string) *common.SlimPod { for _, p := range m.podsByIP { if p.Namespace == namespace && p.Name == name { @@ -73,6 +81,7 @@ func (m *mockK8sInventory) GetPodByName(namespace string, name string) *common.S return nil } +// GetPodByIp looks up a test pod by IP, returning nil when absent. func (m *mockK8sInventory) GetPodByIp(ip string) *common.SlimPod { if m.podsByIP == nil { return nil @@ -80,6 +89,7 @@ func (m *mockK8sInventory) GetPodByIp(ip string) *common.SlimPod { return m.podsByIP[ip] } +// GetSvcs returns the Services currently present in the test inventory. func (m *mockK8sInventory) GetSvcs() []*common.SlimService { var svcs []*common.SlimService for _, s := range m.svcsByIP { @@ -88,6 +98,7 @@ func (m *mockK8sInventory) GetSvcs() []*common.SlimService { return svcs } +// GetSvcByName finds a test Service by namespace and name, returning nil when absent. func (m *mockK8sInventory) GetSvcByName(namespace string, name string) *common.SlimService { for _, s := range m.svcsByIP { if s.Namespace == namespace && s.Name == name { @@ -97,6 +108,7 @@ func (m *mockK8sInventory) GetSvcByName(namespace string, name string) *common.S return nil } +// GetSvcByIp looks up a test Service by IP, returning nil when absent. func (m *mockK8sInventory) GetSvcByIp(ip string) *common.SlimService { if m.svcsByIP == nil { return nil @@ -104,6 +116,7 @@ func (m *mockK8sInventory) GetSvcByIp(ip string) *common.SlimService { return m.svcsByIP[ip] } +// TestCreateNetworkNeighbor_RawPodIP_ResolvedViaK8sInventory checks that inventory resolution replaces a raw IP with stable workload labels. func TestCreateNetworkNeighbor_RawPodIP_ResolvedViaK8sInventory(t *testing.T) { inv := newMockK8sInventory() inv.podsByIP["10.244.0.14"] = &common.SlimPod{ @@ -146,6 +159,7 @@ func TestCreateNetworkNeighbor_RawPodIP_ResolvedViaK8sInventory(t *testing.T) { assert.Equal(t, int32(3306), *neighbor.Ports[0].Port) } +// TestCreateNetworkNeighbor_RawPodIP_CrossNamespace checks that resolved peers in another namespace receive a namespace selector. func TestCreateNetworkNeighbor_RawPodIP_CrossNamespace(t *testing.T) { inv := newMockK8sInventory() inv.podsByIP["10.244.0.14"] = &common.SlimPod{ @@ -184,6 +198,7 @@ func TestCreateNetworkNeighbor_RawPodIP_CrossNamespace(t *testing.T) { assert.Equal(t, map[string]string{"kubernetes.io/metadata.name": "client-ns"}, neighbor.NamespaceSelector.MatchLabels) } +// TestCreateNetworkNeighbor_RawPodIP_ResolvedViaK8sObjectCache checks that fallback resolution uses the IP lookup without listing all pods. func TestCreateNetworkNeighbor_RawPodIP_ResolvedViaK8sObjectCache(t *testing.T) { mockCache := &stubK8sObjectCache{ pods: []*corev1.Pod{ @@ -222,6 +237,7 @@ func TestCreateNetworkNeighbor_RawPodIP_ResolvedViaK8sObjectCache(t *testing.T) require.Zero(t, mockCache.getPodsCalls, "fallback lookup must use the IP index") } +// TestCreateNetworkNeighbor_RawServiceIP_ResolvedViaK8sInventory checks that a raw Service IP resolves to its workload selector. func TestCreateNetworkNeighbor_RawServiceIP_ResolvedViaK8sInventory(t *testing.T) { inv := newMockK8sInventory() inv.svcsByIP["10.96.0.42"] = &common.SlimService{ @@ -260,6 +276,7 @@ func TestCreateNetworkNeighbor_RawServiceIP_ResolvedViaK8sInventory(t *testing.T assert.Equal(t, map[string]string{"app": "api"}, neighbor.PodSelector.MatchLabels) } +// TestCreateNetworkNeighbor_RawPrivateIP_DeferredOnIntermediateFlush checks that deferred private peers resolve after the inventory catches up. func TestCreateNetworkNeighbor_RawPrivateIP_DeferredOnIntermediateFlush(t *testing.T) { cd := &containerData{} rawEvent := NetworkEvent{ @@ -304,6 +321,7 @@ func TestCreateNetworkNeighbor_RawPrivateIP_DeferredOnIntermediateFlush(t *testi assert.Equal(t, map[string]string{"app": "wikijs"}, neighbor.PodSelector.MatchLabels) } +// TestCreateNetworkNeighbor_RawPrivateIP_EmittedExternalIfNeverResolves checks that unresolved private peers become external after one deferred flush. func TestCreateNetworkNeighbor_RawPrivateIP_EmittedExternalIfNeverResolves(t *testing.T) { cd := &containerData{} rawEvent := NetworkEvent{ @@ -330,6 +348,7 @@ func TestCreateNetworkNeighbor_RawPrivateIP_EmittedExternalIfNeverResolves(t *te assert.Equal(t, "10.50.1.20", neighbor.IPAddress) } +// TestCreateNetworkNeighbor_PublicIP_EmittedExternalImmediately checks that public IP peers bypass deferral. func TestCreateNetworkNeighbor_PublicIP_EmittedExternalImmediately(t *testing.T) { cd := &containerData{} rawEvent := NetworkEvent{ @@ -349,6 +368,7 @@ func TestCreateNetworkNeighbor_PublicIP_EmittedExternalImmediately(t *testing.T) assert.Nil(t, cd.deferredNetworks) } +// TestReportNetworkEvent_ImmediateResolutionWhenAvailableInInventory checks that ingestion stores pod identity when inventory already contains the peer. func TestReportNetworkEvent_ImmediateResolutionWhenAvailableInInventory(t *testing.T) { cpm, entry := newTestManager(t, "container1") inv := newMockK8sInventory() @@ -382,6 +402,7 @@ func TestReportNetworkEvent_ImmediateResolutionWhenAvailableInInventory(t *testi assert.Equal(t, map[string]string{"app": "wikijs"}, slice[0].GetDestinationPodLabels()) } +// TestMonitoring_ReResolutionAtProfileFlush checks that profile generation resolves raw peers added to inventory after ingestion. func TestMonitoring_ReResolutionAtProfileFlush(t *testing.T) { cpm, entry := newTestManager(t, "container1") inv := newMockK8sInventory() @@ -423,6 +444,7 @@ func TestMonitoring_ReResolutionAtProfileFlush(t *testing.T) { assert.Equal(t, map[string]string{"app": "wikijs"}, ingress[0].PodSelector.MatchLabels) } +// TestNetworkNeighbors_MergeDistinctPortsAfterResolution checks that converging peer identities retain distinct ports and protocols without duplicates. func TestNetworkNeighbors_MergeDistinctPortsAfterResolution(t *testing.T) { for _, direction := range []string{utils.HostPktType, utils.OutgoingPktType} { t.Run(direction, func(t *testing.T) { @@ -458,6 +480,7 @@ func TestNetworkNeighbors_MergeDistinctPortsAfterResolution(t *testing.T) { } } +// TestCreateNetworkNeighbor_PreservesSnapshotEqualToObservedPort checks that a cached port matching the observation bypasses changed EndpointSlices. func TestCreateNetworkNeighbor_PreservesSnapshotEqualToObservedPort(t *testing.T) { event := serviceNetworkEvent(80, "tcp") client := &servicePortTestClient{ @@ -475,6 +498,7 @@ func TestCreateNetworkNeighbor_PreservesSnapshotEqualToObservedPort(t *testing.T require.Empty(t, client.kubeClient.Actions(), "cached snapshots must not query changed EndpointSlices") } +// TestCreateNetworkNeighbor_ServicePromotionPreservesRawFallback checks that unusable Service selectors retain raw traffic with bounded deferral. func TestCreateNetworkNeighbor_ServicePromotionPreservesRawFallback(t *testing.T) { for _, lookupFailure := range []bool{false, true} { name := "selectorless service" @@ -509,6 +533,7 @@ func TestCreateNetworkNeighbor_ServicePromotionPreservesRawFallback(t *testing.T } } +// TestEmptyEvents_RetainsDeferredServicePortSnapshot checks that deferred observations retain their port snapshot and size until emitted. func TestEmptyEvents_RetainsDeferredServicePortSnapshot(t *testing.T) { event := serviceNetworkEvent(80, "tcp") event.Destination.IPAddress = "10.96.0.42" @@ -536,6 +561,7 @@ func TestEmptyEvents_RetainsDeferredServicePortSnapshot(t *testing.T) { require.Nil(t, cd.servicePorts, "snapshots clear when their observations are emitted") } +// BenchmarkNetworkNeighborsPortScan measures merging 4,000 ports for one peer and checks that no observations are lost. func BenchmarkNetworkNeighborsPortScan(b *testing.B) { cd := &containerData{networks: mapset.NewSet[NetworkEvent]()} for port := 1; port <= 4000; port++ { diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go index ac3d793da6..6a79877649 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" ) +// TestQueueSizeBudgetRetainsUnsplittableAndDepthLimitedData verifies estimate-driven splits preserve data and timestamps through depth and size limits. func TestQueueSizeBudgetRetainsUnsplittableAndDepthLimitedData(t *testing.T) { for _, count := range []int{1, 8} { name := "unsplittable" diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go index 9a6b334555..bf4c91d450 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go @@ -15,6 +15,7 @@ import ( type portLimitedCreator struct{ accepted []*v1beta1.ContainerProfile } +// CreateContainerProfileDirect rejects peers above the port limit and records independent copies of accepted profiles. func (c *portLimitedCreator) CreateContainerProfileDirect(p *v1beta1.ContainerProfile) error { for _, n := range append(append([]v1beta1.NetworkNeighbor{}, p.Spec.Ingress...), p.Spec.Egress...) { if len(n.Ports) > 2 { @@ -25,6 +26,7 @@ func (c *portLimitedCreator) CreateContainerProfileDirect(p *v1beta1.ContainerPr return nil } +// portSplitNeighbor builds a single peer with enough ports to require successive queue splits. func portSplitNeighbor() v1beta1.NetworkNeighbor { n := v1beta1.NetworkNeighbor{Identifier: "peer", Type: "internal", PodSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "api"}}} for i := range 8 { @@ -33,6 +35,7 @@ func portSplitNeighbor() v1beta1.NetworkNeighbor { return n } +// TestQueueSplitsSingleNeighborPorts verifies HTTP 413 and proactive splits deliver every port with a continuous report chain. func TestQueueSplitsSingleNeighborPorts(t *testing.T) { for _, direction := range []string{"ingress", "egress"} { for _, proactive := range []bool{false, true} { diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go index b56b76b8f9..cf80801289 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go @@ -563,10 +563,9 @@ processLoop: if !queuedProfile.IsStitch && queuedProfile.MaxProfileSize > 0 && queuedProfile.SplitDepth < qd.maxSplitDepth && int64(size.Of(queuedProfile.Profile.Spec)) > queuedProfile.MaxProfileSize { - if a, b, ok := splitProfile(queuedProfile.Profile); ok { + if a, b, ok := splitProfile(queuedProfile.Profile); ok && qd.requeueSplit(queuedProfile, a, b, false) { qd.splits.Add(1) qd.metrics.ReportContainerProfileSplit() - qd.requeueSplit(queuedProfile, a, b) continue } } @@ -613,7 +612,7 @@ processLoop: qd.splits.Add(1) qd.metrics.ReportContainerProfileSplit() - qd.requeueSplit(queuedProfile, a, b) + qd.requeueSplit(queuedProfile, a, b, true) } case failureRetryable: @@ -670,8 +669,13 @@ func (qd *QueueData) requeueImmediate(queuedProfile *QueuedContainerProfile) { // Both halves inherit parent.Attempts, take SplitDepth = parent.SplitDepth+1, and are explicitly // IsStitch = false (the zero value - stated because a half must always remain splittable). // +// With allowEviction false, insufficient capacity leaves the queue untouched and returns +// false so the caller can send the original profile. Capacity is checked under the same lock +// as both enqueues, preventing concurrent producers from taking either reserved slot. A true +// return means the split was attempted, including enqueue failures handled below. +// // Callers must NOT hold qd.mu. -func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1.ContainerProfile) { +func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1.ContainerProfile, allowEviction bool) bool { half := func(profile *v1beta1.ContainerProfile) *QueuedContainerProfile { return &QueuedContainerProfile{ Profile: profile, @@ -686,6 +690,10 @@ func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1. qd.mu.Lock() defer qd.mu.Unlock() + if !allowEviction && qd.maxQueueSize-qd.queue.Size() < 2 { + return false + } + if err := qd.enqueueLocked(half(a)); err != nil { // The parent was already dequeued, so neither half reaches the queue: this is a // total loss of the chunk, not just a fork, and must be at least as loud as the @@ -712,7 +720,7 @@ func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1. helpers.String("containerID", parent.ContainerID)) qd.metrics.ReportContainerProfileChunkDropped(string(dropReasonStitchBacklogExhausted)) - return + return true } stitch := qd.newStitchFor(parent, false) @@ -725,7 +733,7 @@ func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1. qd.metrics.ReportContainerProfileChunkDropped(string(dropReasonEnqueueFailed)) } - return + return true } if err := qd.enqueueLocked(half(b)); err != nil { @@ -739,6 +747,7 @@ func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1. qd.chunksDropped.Add(1) qd.metrics.ReportContainerProfileChunkDropped(string(dropReasonEnqueueFailed)) } + return true } // dropChunk discards a queued chunk that cannot be delivered as-is: because it was rejected for diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue_errors_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_errors_test.go index 42a95abcf5..657ad4c7eb 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_queue_errors_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_errors_test.go @@ -906,7 +906,7 @@ func TestRequeueSplit_QueueNotRunningDropsBothHalvesAndAttemptsStitch(t *testing qd.running = false qd.mu.Unlock() - qd.requeueSplit(queuedParent, a, b) + qd.requeueSplit(queuedParent, a, b, true) assert.Equal(t, 0, qd.GetQueueSize(), "neither half nor the stitch can land while the queue isn't running") assert.Equal(t, int64(1), qd.chunksDropped.Load(), diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go new file mode 100644 index 0000000000..25e2bd6213 --- /dev/null +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go @@ -0,0 +1,108 @@ +package queue + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestProactiveSplitPreservesFullQueue verifies that optional splitting never evicts +// pending data when dequeuing the original freed only one slot. +func TestProactiveSplitPreservesFullQueue(t *testing.T) { + for _, capacity := range []int{1, 3} { + t.Run(fmt.Sprint(capacity), func(t *testing.T) { + creator := &MockProfileCreator{} + qd, err := NewQueueData(context.Background(), creator, QueueConfig{QueueDir: t.TempDir(), MaxQueueSize: capacity}) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, qd.Close()) }) + + parent := testProfile() + parent.Spec.Capabilities = []string{"cap-a", "cap-b"} + _, _, ok := splitProfile(parent) + require.True(t, ok) + require.NoError(t, qd.EnqueueWithSizeLimit(parent, "parent", 1)) + for i := 1; i < capacity; i++ { + pending := testProfile() + pending.Name = fmt.Sprintf("pending-%d", i) + pending.Spec.Capabilities = []string{"pending-capability"} + require.NoError(t, qd.Enqueue(pending, pending.Name)) + } + + qd.processAllItems() + + created := creator.CreatedProfiles() + require.Len(t, created, capacity) + assert.Equal(t, parent, created[0], "capacity pressure must send the intact original") + for i := 1; i < capacity; i++ { + assert.Equal(t, fmt.Sprintf("pending-%d", i), created[i].Name) + assert.Equal(t, []string{"pending-capability"}, created[i].Spec.Capabilities) + } + assert.Zero(t, qd.splits.Load()) + assert.Zero(t, qd.chunksDropped.Load()) + assert.Zero(t, qd.GetQueueSize()) + }) + } +} + +// TestProactiveSplitChecksCapacityAtAdmission verifies that a producer consuming +// capacity before split admission cannot cause the split to evict pending data. +func TestProactiveSplitChecksCapacityAtAdmission(t *testing.T) { + qd, err := NewQueueData(context.Background(), &MockProfileCreator{}, QueueConfig{QueueDir: t.TempDir(), MaxQueueSize: 3}) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, qd.Close()) }) + parent := testProfile() + parent.Spec.Capabilities = []string{"cap-a", "cap-b"} + a, b, ok := splitProfile(parent) + require.True(t, ok) + pending := testProfile() + require.NoError(t, qd.Enqueue(pending, "pending")) + require.Equal(t, 2, qd.maxQueueSize-qd.GetQueueSize()) + + // A producer holds the admission lock while the split waits. Its enqueue uses + // the same locked path as Enqueue and consumes one of the two available slots. + qd.mu.Lock() + started := make(chan struct{}) + admitted := make(chan bool, 1) + go func() { + close(started) + admitted <- qd.requeueSplit(&QueuedContainerProfile{Profile: parent}, a, b, false) + }() + <-started + incoming := testProfile() + err = qd.enqueueLocked(&QueuedContainerProfile{Profile: incoming, ContainerID: "incoming"}) + qd.mu.Unlock() + require.NoError(t, err) + require.False(t, <-admitted) + assert.Zero(t, qd.chunksDropped.Load()) + require.Equal(t, 2, qd.GetQueueSize()) + for _, id := range []string{"pending", "incoming"} { + item, err := qd.queue.Dequeue() + require.NoError(t, err) + assert.Equal(t, id, item.(*QueuedContainerProfile).ContainerID) + } +} + +// TestProactiveSplitUsesAvailableCapacity verifies that both halves are delivered +// when proactive splitting has enough queue capacity. +func TestProactiveSplitUsesAvailableCapacity(t *testing.T) { + creator := &MockProfileCreator{} + qd, err := NewQueueData(context.Background(), creator, QueueConfig{QueueDir: t.TempDir(), MaxQueueSize: 2}) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, qd.Close()) }) + parent := testProfile() + parent.Spec.Capabilities = []string{"cap-a", "cap-b"} + require.NoError(t, qd.EnqueueWithSizeLimit(parent, "parent", 1)) + qd.processAllItems() + assert.Empty(t, creator.CreatedProfiles()) + assert.Equal(t, int64(1), qd.splits.Load()) + require.Equal(t, 2, qd.GetQueueSize()) + qd.processAllItems() + created := creator.CreatedProfiles() + require.Len(t, created, 2) + assert.Equal(t, parent.Spec.Capabilities, append(created[0].Spec.Capabilities, created[1].Spec.Capabilities...)) + assert.Zero(t, qd.chunksDropped.Load()) + assert.Zero(t, qd.GetQueueSize()) +} diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go index 3a5a922680..75dab3a3db 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go @@ -174,6 +174,7 @@ func elementSignatures(spec *v1beta1.ContainerProfileSpec) []string { return out } +// TestSplitProfile_PartitionsWithoutLossOrDuplication checks that split halves contain every observation exactly once, including peer ports. func TestSplitProfile_PartitionsWithoutLossOrDuplication(t *testing.T) { tests := []struct { name string diff --git a/pkg/objectcache/containerprofilecache/integration_helpers_test.go b/pkg/objectcache/containerprofilecache/integration_helpers_test.go index b9cfbd1378..b03ae6acac 100644 --- a/pkg/objectcache/containerprofilecache/integration_helpers_test.go +++ b/pkg/objectcache/containerprofilecache/integration_helpers_test.go @@ -99,6 +99,7 @@ func (k *stubK8sCache) GetPodStatus(_, _ string) *corev1.PodStatus { return nil func (k *stubK8sCache) GetApiServerIpAddress() string { return "" } func (k *stubK8sCache) GetPods() []*corev1.Pod { return nil } +// GetPodByIP returns no peer because these cache tests do not model IP resolution. func (k *stubK8sCache) GetPodByIP(_ string) *corev1.Pod { return nil } func (k *stubK8sCache) SetSharedContainerData(id string, d *objectcache.WatchedContainerData) { diff --git a/pkg/objectcache/containerprofilecache/reconciler_test.go b/pkg/objectcache/containerprofilecache/reconciler_test.go index 3b9a6a952c..8793643a51 100644 --- a/pkg/objectcache/containerprofilecache/reconciler_test.go +++ b/pkg/objectcache/containerprofilecache/reconciler_test.go @@ -55,7 +55,9 @@ func (k *controllableK8sCache) GetPodSpec(_, _ string) *corev1.PodSpec { ret func (k *controllableK8sCache) GetPodStatus(_, _ string) *corev1.PodStatus { return nil } func (k *controllableK8sCache) GetApiServerIpAddress() string { return "" } func (k *controllableK8sCache) GetPods() []*corev1.Pod { return nil } -func (k *controllableK8sCache) GetPodByIP(_ string) *corev1.Pod { return nil } + +// GetPodByIP returns no peer because these cache tests do not model IP resolution. +func (k *controllableK8sCache) GetPodByIP(_ string) *corev1.Pod { return nil } func (k *controllableK8sCache) SetSharedContainerData(_ string, _ *objectcache.WatchedContainerData) { } func (k *controllableK8sCache) GetSharedContainerData(_ string) *objectcache.WatchedContainerData { diff --git a/pkg/objectcache/k8scache/k8scache.go b/pkg/objectcache/k8scache/k8scache.go index 6e7a6b9deb..f499c97e24 100644 --- a/pkg/objectcache/k8scache/k8scache.go +++ b/pkg/objectcache/k8scache/k8scache.go @@ -107,18 +107,21 @@ func (k *K8sObjectCacheImpl) DeleteSharedContainerData(containerID string) { k.containerIDToSharedData.Delete(containerID) } +// AddHandler adds pod objects to both name and IP indexes, ignoring other resource types. func (k *K8sObjectCacheImpl) AddHandler(_ context.Context, obj runtime.Object) { if pod, ok := obj.(*corev1.Pod); ok { k.storePod(pod) } } +// ModifyHandler updates both pod indexes and removes stale IP mappings. func (k *K8sObjectCacheImpl) ModifyHandler(_ context.Context, obj runtime.Object) { if pod, ok := obj.(*corev1.Pod); ok { k.storePod(pod) } } +// DeleteHandler removes the current pod and its IP mappings only when the deleted UID matches. func (k *K8sObjectCacheImpl) DeleteHandler(_ context.Context, obj runtime.Object) { if pod, ok := obj.(*corev1.Pod); ok { k.podMu.Lock() @@ -133,6 +136,7 @@ func (k *K8sObjectCacheImpl) DeleteHandler(_ context.Context, obj runtime.Object } } +// storePod updates pod indexes under lock while preserving IPs reassigned to another pod. func (k *K8sObjectCacheImpl) storePod(pod *corev1.Pod) { k.podMu.Lock() defer k.podMu.Unlock() diff --git a/pkg/objectcache/k8scache/pod_ip_test.go b/pkg/objectcache/k8scache/pod_ip_test.go index 3f5b2a049a..f94291c9ff 100644 --- a/pkg/objectcache/k8scache/pod_ip_test.go +++ b/pkg/objectcache/k8scache/pod_ip_test.go @@ -11,10 +11,12 @@ import ( "k8s.io/apimachinery/pkg/types" ) +// indexedPod builds a pod with explicit identity and primary IP for index lifecycle tests. func indexedPod(name, uid, ip string) *corev1.Pod { return &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: name, UID: types.UID(uid)}, Status: corev1.PodStatus{PodIP: ip}} } +// TestPodIPIndexLifecycle checks primary and secondary IP updates and deletion using an older status payload. func TestPodIPIndexLifecycle(t *testing.T) { k := &K8sObjectCacheImpl{} ctx := context.Background() @@ -38,6 +40,7 @@ func TestPodIPIndexLifecycle(t *testing.T) { assert.Nil(t, k.GetPod("default", "pod")) } +// TestPodIPIndexReuse checks that deleting an old pod preserves the pod that reused its IP. func TestPodIPIndexReuse(t *testing.T) { for _, sameName := range []bool{false, true} { t.Run(map[bool]string{false: "different pods", true: "recreated pod"}[sameName], func(t *testing.T) { @@ -59,6 +62,7 @@ func TestPodIPIndexReuse(t *testing.T) { } } +// TestPodIPIndexUpdatePreservesReusedIP checks that updating or deleting an old pod cannot remove a reassigned IP. func TestPodIPIndexUpdatePreservesReusedIP(t *testing.T) { k := &K8sObjectCacheImpl{} ctx := context.Background() @@ -78,6 +82,7 @@ func TestPodIPIndexUpdatePreservesReusedIP(t *testing.T) { assert.Nil(t, k.GetPodByIP("fd00::1")) } +// TestPodIPIndexConcurrentAccess exercises concurrent informer mutations and IP lookups for race detection. func TestPodIPIndexConcurrentAccess(t *testing.T) { k := &K8sObjectCacheImpl{} var wg sync.WaitGroup @@ -97,6 +102,7 @@ func TestPodIPIndexConcurrentAccess(t *testing.T) { wg.Wait() } +// TestPodIPIndexRetainedReassignedIP checks that stale status updates cannot reclaim reused primary or secondary IPs. func TestPodIPIndexRetainedReassignedIP(t *testing.T) { for _, secondary := range []bool{false, true} { t.Run(map[bool]string{false: "primary", true: "secondary"}[secondary], func(t *testing.T) { @@ -121,6 +127,7 @@ func TestPodIPIndexRetainedReassignedIP(t *testing.T) { } } +// TestPodIPIndexNewAssignmentTakesOwnership checks that newly assigned IPs and recreated pods take ownership of their index entries. func TestPodIPIndexNewAssignmentTakesOwnership(t *testing.T) { k := &K8sObjectCacheImpl{} ctx := context.Background() diff --git a/pkg/objectcache/k8scache_interface.go b/pkg/objectcache/k8scache_interface.go index 95827f3d4a..064979ad3b 100644 --- a/pkg/objectcache/k8scache_interface.go +++ b/pkg/objectcache/k8scache_interface.go @@ -35,6 +35,8 @@ func (k *K8sObjectCacheMock) GetPodStatus(_, _ string) *corev1.PodStatus { func (k *K8sObjectCacheMock) GetPod(_, _ string) *corev1.Pod { return &corev1.Pod{Spec: k.PodSpec, Status: k.PodStatus} } + +// GetPodByIP returns the mock pod for a matching primary or secondary IP, or nil. func (k *K8sObjectCacheMock) GetPodByIP(ip string) *corev1.Pod { if ip == "" { return nil diff --git a/pkg/objectcache/v1/mock.go b/pkg/objectcache/v1/mock.go index 2b0f40ba9f..76e79755d5 100644 --- a/pkg/objectcache/v1/mock.go +++ b/pkg/objectcache/v1/mock.go @@ -275,6 +275,7 @@ func (r *RuleObjectCacheMock) GetPods() []*corev1.Pod { return []*corev1.Pod{{Spec: *r.podSpec, Status: *r.podStatus}} } +// GetPodByIP resolves the configured mock pod by primary or secondary IP, or returns nil. func (r *RuleObjectCacheMock) GetPodByIP(ip string) *corev1.Pod { if ip == "" || r.podStatus == nil { return nil From 59734909aeff5dee25f0ff52fce27b65e178fcdf Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 13:45:50 +0200 Subject: [PATCH 08/19] fix(cpm): address Copilot review 5413961676 Skip optional split admission after queue shutdown and deliver the intact original. Cover shutdown with both insufficient and sufficient split capacity. Signed-off-by: Matthias Bertschy --- .../v1/queue/containerprofile_queue.go | 9 +++--- .../containerprofile_queue_proactive_test.go | 32 +++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go index cf80801289..7ffa9256e3 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go @@ -669,9 +669,10 @@ func (qd *QueueData) requeueImmediate(queuedProfile *QueuedContainerProfile) { // Both halves inherit parent.Attempts, take SplitDepth = parent.SplitDepth+1, and are explicitly // IsStitch = false (the zero value - stated because a half must always remain splittable). // -// With allowEviction false, insufficient capacity leaves the queue untouched and returns -// false so the caller can send the original profile. Capacity is checked under the same lock -// as both enqueues, preventing concurrent producers from taking either reserved slot. A true +// With allowEviction false, shutdown or insufficient capacity leaves the queue untouched +// and returns false so the caller can send the original profile. Admission is checked under +// the same lock as both enqueues, preventing shutdown or concurrent producers from +// invalidating the decision before either half is queued. A true // return means the split was attempted, including enqueue failures handled below. // // Callers must NOT hold qd.mu. @@ -690,7 +691,7 @@ func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1. qd.mu.Lock() defer qd.mu.Unlock() - if !allowEviction && qd.maxQueueSize-qd.queue.Size() < 2 { + if !allowEviction && (!qd.running || qd.maxQueueSize-qd.queue.Size() < 2) { return false } diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go index 25e2bd6213..44738567f8 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go @@ -106,3 +106,35 @@ func TestProactiveSplitUsesAvailableCapacity(t *testing.T) { assert.Zero(t, qd.chunksDropped.Load()) assert.Zero(t, qd.GetQueueSize()) } + +// TestProactiveSplitSendsOriginalDuringShutdown verifies that stopping queue admission +// while processing is in flight preserves the original payload for its direct send. +func TestProactiveSplitSendsOriginalDuringShutdown(t *testing.T) { + for _, capacity := range []int{1, 2} { + t.Run(fmt.Sprint(capacity), func(t *testing.T) { + creator := &MockProfileCreator{} + qd, err := NewQueueData(context.Background(), creator, QueueConfig{QueueDir: t.TempDir(), MaxQueueSize: capacity}) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, qd.Close()) }) + parent := testProfile() + parent.Spec.Capabilities = []string{"cap-a", "cap-b"} + _, _, ok := splitProfile(parent) + require.True(t, ok) + require.NoError(t, qd.EnqueueWithSizeLimit(parent, "parent", 1)) + + // Close disables admission before waiting for the processor to finish. + // Reproduce that state without closing the disk queue underneath processing. + qd.mu.Lock() + qd.running = false + qd.mu.Unlock() + qd.processAllItems() + + created := creator.CreatedProfiles() + require.Len(t, created, 1) + assert.Equal(t, parent, created[0]) + assert.Zero(t, qd.splits.Load()) + assert.Zero(t, qd.chunksDropped.Load()) + assert.Zero(t, qd.GetQueueSize()) + }) + } +} From 1eedb87188025ab41a4534d9e5648ac7e913eac5 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 14:02:51 +0200 Subject: [PATCH 09/19] fix(queue): preserve optional split data on enqueue errors Address Copilot review 5414063623 by sending the original or unsent half directly after optional enqueue failures. Add real filesystem-failure regressions for both cases. Signed-off-by: Matthias Bertschy --- .../v1/queue/containerprofile_queue.go | 47 +++++++---- .../containerprofile_queue_proactive_test.go | 81 ++++++++++++++++++- 2 files changed, 110 insertions(+), 18 deletions(-) diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go index 7ffa9256e3..1e026a1101 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go @@ -563,10 +563,16 @@ processLoop: if !queuedProfile.IsStitch && queuedProfile.MaxProfileSize > 0 && queuedProfile.SplitDepth < qd.maxSplitDepth && int64(size.Of(queuedProfile.Profile.Spec)) > queuedProfile.MaxProfileSize { - if a, b, ok := splitProfile(queuedProfile.Profile); ok && qd.requeueSplit(queuedProfile, a, b, false) { - qd.splits.Add(1) - qd.metrics.ReportContainerProfileSplit() - continue + if a, b, ok := splitProfile(queuedProfile.Profile); ok { + fallback := qd.requeueSplit(queuedProfile, a, b, false) + if fallback != queuedProfile { + qd.splits.Add(1) + qd.metrics.ReportContainerProfileSplit() + } + if fallback == nil { + continue + } + queuedProfile = fallback } } @@ -669,14 +675,14 @@ func (qd *QueueData) requeueImmediate(queuedProfile *QueuedContainerProfile) { // Both halves inherit parent.Attempts, take SplitDepth = parent.SplitDepth+1, and are explicitly // IsStitch = false (the zero value - stated because a half must always remain splittable). // -// With allowEviction false, shutdown or insufficient capacity leaves the queue untouched -// and returns false so the caller can send the original profile. Admission is checked under -// the same lock as both enqueues, preventing shutdown or concurrent producers from -// invalidating the decision before either half is queued. A true -// return means the split was attempted, including enqueue failures handled below. +// With allowEviction false, shutdown, insufficient capacity, or first-half enqueue failure +// returns parent for direct delivery. If only the second enqueue fails, it returns that half +// with its split metadata for direct delivery, preserving the already queued first half. +// Admission is checked under the same lock as both enqueues. A nil return means the split was +// handled; allowEviction true retains the HTTP 413 drop/repair policy on enqueue failures. // // Callers must NOT hold qd.mu. -func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1.ContainerProfile, allowEviction bool) bool { +func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1.ContainerProfile, allowEviction bool) *QueuedContainerProfile { half := func(profile *v1beta1.ContainerProfile) *QueuedContainerProfile { return &QueuedContainerProfile{ Profile: profile, @@ -692,10 +698,15 @@ func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1. defer qd.mu.Unlock() if !allowEviction && (!qd.running || qd.maxQueueSize-qd.queue.Size() < 2) { - return false + return parent } if err := qd.enqueueLocked(half(a)); err != nil { + if !allowEviction { + logger.L().Warning("failed to enqueue optional split, sending original container profile", + helpers.String("name", parent.Profile.Name), helpers.Error(err)) + return parent + } // The parent was already dequeued, so neither half reaches the queue: this is a // total loss of the chunk, not just a fork, and must be at least as loud as the // second-half case below. Unlike that case, nothing of the parent's data survives @@ -721,7 +732,7 @@ func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1. helpers.String("containerID", parent.ContainerID)) qd.metrics.ReportContainerProfileChunkDropped(string(dropReasonStitchBacklogExhausted)) - return true + return nil } stitch := qd.newStitchFor(parent, false) @@ -734,10 +745,16 @@ func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1. qd.metrics.ReportContainerProfileChunkDropped(string(dropReasonEnqueueFailed)) } - return true + return nil } - if err := qd.enqueueLocked(half(b)); err != nil { + second := half(b) + if err := qd.enqueueLocked(second); err != nil { + if !allowEviction { + logger.L().Warning("failed to enqueue second optional split half, sending it directly", + helpers.String("name", b.Name), helpers.Error(err)) + return second + } // Exactly one half of a pair survived, which forks the container's report chain. logger.L().Warning("failed to enqueue the second half of a split container profile, its report chain is now forked", helpers.String("name", b.Name), @@ -748,7 +765,7 @@ func (qd *QueueData) requeueSplit(parent *QueuedContainerProfile, a, b *v1beta1. qd.chunksDropped.Add(1) qd.metrics.ReportContainerProfileChunkDropped(string(dropReasonEnqueueFailed)) } - return true + return nil } // dropChunk discards a queued chunk that cannot be delivered as-is: because it was rejected for diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go index 44738567f8..9c50343dad 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go @@ -3,8 +3,13 @@ package queue import ( "context" "fmt" + "os" + "path/filepath" "testing" + helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" + "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -65,17 +70,19 @@ func TestProactiveSplitChecksCapacityAtAdmission(t *testing.T) { // the same locked path as Enqueue and consumes one of the two available slots. qd.mu.Lock() started := make(chan struct{}) - admitted := make(chan bool, 1) + fallback := make(chan *QueuedContainerProfile, 1) go func() { close(started) - admitted <- qd.requeueSplit(&QueuedContainerProfile{Profile: parent}, a, b, false) + fallback <- qd.requeueSplit(&QueuedContainerProfile{Profile: parent}, a, b, false) }() <-started incoming := testProfile() err = qd.enqueueLocked(&QueuedContainerProfile{Profile: incoming, ContainerID: "incoming"}) qd.mu.Unlock() require.NoError(t, err) - require.False(t, <-admitted) + unsent := <-fallback + require.NotNil(t, unsent) + require.Same(t, parent, unsent.Profile) assert.Zero(t, qd.chunksDropped.Load()) require.Equal(t, 2, qd.GetQueueSize()) for _, id := range []string{"pending", "incoming"} { @@ -138,3 +145,71 @@ func TestProactiveSplitSendsOriginalDuringShutdown(t *testing.T) { }) } } + +// recoveringDiskCreator restores segment creation when storage receives a profile, +// allowing each regression to verify the complete delivery after an enqueue failure. +type recoveringDiskCreator struct { + MockProfileCreator + blockedSegment string +} + +// CreateContainerProfileDirect clears the injected disk failure and records delivery. +func (c *recoveringDiskCreator) CreateContainerProfileDirect(profile *v1beta1.ContainerProfile) error { + if c.blockedSegment != "" { + if err := os.Remove(c.blockedSegment); err != nil { + return err + } + c.blockedSegment = "" + } + return c.MockProfileCreator.CreateContainerProfileDirect(profile) +} + +// TestProactiveSplitPreservesDataOnDiskFailure verifies that optional splitting sends +// the original on first-half failure and only the unqueued half on second-half failure. +func TestProactiveSplitPreservesDataOnDiskFailure(t *testing.T) { + for _, itemsPerSegment := range []int{2, 3} { + t.Run(fmt.Sprint(itemsPerSegment), func(t *testing.T) { + dir := t.TempDir() + creator := &recoveringDiskCreator{} + qd, err := NewQueueData(context.Background(), creator, QueueConfig{QueueDir: dir, MaxQueueSize: 4, ItemsPerSegment: itemsPerSegment}) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, qd.Close()) }) + parent := testProfile() + parent.Spec.Capabilities = []string{"cap-a", "cap-b"} + require.NoError(t, qd.EnqueueWithSizeLimit(parent, "parent", 1)) + pending := testProfile() + pending.Name = "pending" + require.NoError(t, qd.Enqueue(pending, "pending")) + + // dque starts at segment 1. Keep a pending item there so dequeuing the + // parent does not rotate segments. Two slots force the first-half enqueue + // to create segment 2; three slots defer that failure to the second half. + blocked := filepath.Join(dir, DefaultQueueName, "0000000000002.dque") + require.NoError(t, os.Mkdir(blocked, 0700)) + creator.blockedSegment = blocked + qd.processAllItems() + require.Empty(t, creator.blockedSegment, "fallback must reach storage before pending data is dequeued") + qd.processAllItems() + + created := creator.CreatedProfiles() + if itemsPerSegment == 2 { + require.Len(t, created, 2) + assert.Equal(t, parent, created[0]) + assert.Zero(t, qd.splits.Load()) + } else { + require.Len(t, created, 3) + // The second half is sent immediately; the first remains queued until + // the next processing pass. Their payloads and chain still cover parent. + a, b := created[2], created[0] + assert.Equal(t, parent.Spec.Capabilities, append(a.Spec.Capabilities, b.Spec.Capabilities...)) + assert.Equal(t, parent.Annotations[helpersv1.PreviousReportTimestampMetadataKey], a.Annotations[helpersv1.PreviousReportTimestampMetadataKey]) + assert.Equal(t, a.Annotations[helpersv1.ReportTimestampMetadataKey], b.Annotations[helpersv1.PreviousReportTimestampMetadataKey]) + assert.Equal(t, parent.Annotations[helpersv1.ReportTimestampMetadataKey], b.Annotations[helpersv1.ReportTimestampMetadataKey]) + assert.Equal(t, int64(1), qd.splits.Load()) + } + assert.Equal(t, pending, created[1]) + assert.Zero(t, qd.chunksDropped.Load()) + assert.Zero(t, qd.GetQueueSize()) + }) + } +} From 88c1c21508d134f4a5e7d7bca1bc5d4c1afa1db4 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 14:13:44 +0200 Subject: [PATCH 10/19] fix(queue): split dominant peer ports before depth exhaustion Address Copilot review 5414255433 by partitioning neighbors at the midpoint of their total port observations. Verify skewed ingress and egress batches preserve every observation and report chain under the default HTTP 413 split-depth limit. Signed-off-by: Matthias Bertschy --- .../containerprofile_ports_split_test.go | 83 +++++++++++++++++++ .../v1/queue/containerprofile_split.go | 30 +++++-- 2 files changed, 104 insertions(+), 9 deletions(-) diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go index bf4c91d450..c488e52038 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go @@ -79,3 +79,86 @@ func TestQueueSplitsSingleNeighborPorts(t *testing.T) { } } } + +// skewedPortNeighbors places one port-heavy peer among peers with one or no ports. +func skewedPortNeighbors(heavyIndex int) []v1beta1.NetworkNeighbor { + neighbors := make([]v1beta1.NetworkNeighbor, 16) + for i := range neighbors { + neighbors[i] = portSplitNeighbor() + neighbors[i].Identifier = fmt.Sprintf("peer-%d", i) + neighbors[i].Ports = neighbors[i].Ports[:i%2] + } + neighbors[heavyIndex] = portSplitNeighbor() + return neighbors +} + +// TestSplitProfileBalancesSkewedNeighborPorts verifies weighted splitting preserves +// observation order, zero-port peers, identities, and the unmodified input. +func TestSplitProfileBalancesSkewedNeighborPorts(t *testing.T) { + for _, direction := range []string{"ingress", "egress"} { + for _, heavyIndex := range []int{0, 8, 15} { + t.Run(fmt.Sprintf("%s/heavy=%d", direction, heavyIndex), func(t *testing.T) { + profile := testProfile() + neighbors := skewedPortNeighbors(heavyIndex) + if direction == "ingress" { + profile.Spec.Ingress = neighbors + } else { + profile.Spec.Egress = neighbors + } + before := profile.DeepCopy() + a, b, ok := splitProfile(profile) + require.True(t, ok) + require.Equal(t, 12, countPartitionableElements(&a.Spec)) + require.Equal(t, 11, countPartitionableElements(&b.Spec)) + require.Equal(t, elementSignatures(&profile.Spec), append(elementSignatures(&a.Spec), elementSignatures(&b.Spec)...)) + aPeers, bPeers := a.Spec.Ingress, b.Spec.Ingress + if direction == "egress" { + aPeers, bPeers = a.Spec.Egress, b.Spec.Egress + } + require.LessOrEqual(t, len(aPeers)+len(bPeers), len(neighbors)+1) + if aPeers[len(aPeers)-1].Identifier == bPeers[0].Identifier { + // Only a boundary peer is duplicated, with independent identity and ports. + aPeers[len(aPeers)-1].PodSelector.MatchLabels["app"] = "changed" + *aPeers[len(aPeers)-1].Ports[0].Port = 1 + require.Equal(t, "api", bPeers[0].PodSelector.MatchLabels["app"]) + } + require.Equal(t, before, profile) + }) + } + } +} + +// TestQueueSplitsSkewedNeighborPortsWithinDefaultDepth verifies a heavy peer is +// split before peer isolation exhausts the HTTP 413 retry lineage's depth budget. +func TestQueueSplitsSkewedNeighborPortsWithinDefaultDepth(t *testing.T) { + for _, direction := range []string{"ingress", "egress"} { + for _, heavyIndex := range []int{0, 8, 15} { + t.Run(fmt.Sprintf("%s/heavy=%d", direction, heavyIndex), func(t *testing.T) { + creator := &portLimitedCreator{} + q, err := NewQueueData(context.Background(), creator, QueueConfig{QueueDir: t.TempDir(), MaxQueueSize: 100}) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, q.Close()) }) + profile := testProfile() + if direction == "ingress" { + profile.Spec.Ingress = skewedPortNeighbors(heavyIndex) + } else { + profile.Spec.Egress = skewedPortNeighbors(heavyIndex) + } + require.NoError(t, q.Enqueue(profile, "container")) + for range DefaultMaxSplitDepth + 2 { + q.processAllItems() + } + var observations []string + var rows []tsRow + for _, accepted := range creator.accepted { + observations = append(observations, elementSignatures(&accepted.Spec)...) + rows = append(rows, tsRow{PreviousReportTimestamp: accepted.Annotations[helpersv1.PreviousReportTimestampMetadataKey], ReportTimestamp: accepted.Annotations[helpersv1.ReportTimestampMetadataKey]}) + } + require.ElementsMatch(t, elementSignatures(&profile.Spec), observations) + assertChainIsLinear(t, rows, profile.Annotations[helpersv1.PreviousReportTimestampMetadataKey], profile.Annotations[helpersv1.ReportTimestampMetadataKey]) + require.Zero(t, q.chunksDropped.Load()) + require.Zero(t, q.GetQueueSize()) + }) + } + } +} diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_split.go b/pkg/containerprofilemanager/v1/queue/containerprofile_split.go index c35b1d10df..e979f183f4 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_split.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_split.go @@ -245,17 +245,29 @@ func countNeighborElements(neighbors []v1beta1.NetworkNeighbor) int { return count } -// Split peer lists first. Once a single peer remains, its ports can still be -// partitioned while copying the identity into both chunks for storage to merge. +// halveNeighbors partitions the ordered observations at half their total weight, +// counting every port and each portless peer once. Only a peer crossing that boundary +// is copied into both chunks, so large port lists consume the same split-depth budget +// as other observations without duplicating every peer's identity. func halveNeighbors(neighbors []v1beta1.NetworkNeighbor) ([]v1beta1.NetworkNeighbor, []v1beta1.NetworkNeighbor) { - if len(neighbors) != 1 || len(neighbors[0].Ports) <= 1 { - return halve(neighbors) + remaining := (countNeighborElements(neighbors) + 1) / 2 + for i := range neighbors { + if remaining == 0 { + return neighbors[:i], neighbors[i:] + } + weight := max(1, len(neighbors[i].Ports)) + if remaining < weight { + left, right := neighbors[i].DeepCopy(), neighbors[i].DeepCopy() + left.Ports = left.Ports[:remaining] + right.Ports = right.Ports[remaining:] + a := append([]v1beta1.NetworkNeighbor(nil), neighbors[:i]...) + a = append(a, *left) + b := append([]v1beta1.NetworkNeighbor{*right}, neighbors[i+1:]...) + return a, b + } + remaining -= weight } - a, b := neighbors[0].DeepCopy(), neighbors[0].DeepCopy() - mid := (len(a.Ports) + 1) / 2 - a.Ports = a.Ports[:mid] - b.Ports = b.Ports[mid:] - return []v1beta1.NetworkNeighbor{*a}, []v1beta1.NetworkNeighbor{*b} + return neighbors, nil } // moveOneElement transfers a single element from the first non-empty partitionable field of From 5b4fb863833fd8ab9a270f4c706e5628b4af60c8 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 14:27:14 +0200 Subject: [PATCH 11/19] fix(queue): balance peer splits by payload bytes Address Copilot review 5414381382 by accounting for peer identity and selector size alongside port payloads. Prefer whole-peer cuts on ties and include duplicated identity costs in interior-port cuts. Regressions verify complete HTTP 413 delivery under actual protobuf size limits. Signed-off-by: Matthias Bertschy --- .../containerprofile_ports_split_test.go | 120 +++++++++++++++++- .../v1/queue/containerprofile_split.go | 75 ++++++++--- 2 files changed, 173 insertions(+), 22 deletions(-) diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go index c488e52038..c78a0fa230 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_ports_split_test.go @@ -2,8 +2,10 @@ package queue import ( "context" + "encoding/json" "fmt" "net/http" + "strings" "testing" "github.com/DmitriyVTitov/size" @@ -92,7 +94,7 @@ func skewedPortNeighbors(heavyIndex int) []v1beta1.NetworkNeighbor { return neighbors } -// TestSplitProfileBalancesSkewedNeighborPorts verifies weighted splitting preserves +// TestSplitProfileBalancesSkewedNeighborPorts verifies byte-balanced splitting preserves // observation order, zero-port peers, identities, and the unmodified input. func TestSplitProfileBalancesSkewedNeighborPorts(t *testing.T) { for _, direction := range []string{"ingress", "egress"} { @@ -108,13 +110,19 @@ func TestSplitProfileBalancesSkewedNeighborPorts(t *testing.T) { before := profile.DeepCopy() a, b, ok := splitProfile(profile) require.True(t, ok) - require.Equal(t, 12, countPartitionableElements(&a.Spec)) - require.Equal(t, 11, countPartitionableElements(&b.Spec)) + require.Equal(t, 23, countPartitionableElements(&a.Spec)+countPartitionableElements(&b.Spec)) require.Equal(t, elementSignatures(&profile.Spec), append(elementSignatures(&a.Spec), elementSignatures(&b.Spec)...)) aPeers, bPeers := a.Spec.Ingress, b.Spec.Ingress if direction == "egress" { aPeers, bPeers = a.Spec.Egress, b.Spec.Egress } + encodedParent, err := json.Marshal(neighbors) + require.NoError(t, err) + encodedA, err := json.Marshal(aPeers) + require.NoError(t, err) + encodedB, err := json.Marshal(bPeers) + require.NoError(t, err) + require.LessOrEqual(t, max(len(encodedA), len(encodedB)), 2*len(encodedParent)/3) require.LessOrEqual(t, len(aPeers)+len(bPeers), len(neighbors)+1) if aPeers[len(aPeers)-1].Identifier == bPeers[0].Identifier { // Only a boundary peer is duplicated, with independent identity and ports. @@ -134,7 +142,7 @@ func TestQueueSplitsSkewedNeighborPortsWithinDefaultDepth(t *testing.T) { for _, direction := range []string{"ingress", "egress"} { for _, heavyIndex := range []int{0, 8, 15} { t.Run(fmt.Sprintf("%s/heavy=%d", direction, heavyIndex), func(t *testing.T) { - creator := &portLimitedCreator{} + creator := &byteLimitedCreator{} q, err := NewQueueData(context.Background(), creator, QueueConfig{QueueDir: t.TempDir(), MaxQueueSize: 100}) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, q.Close()) }) @@ -144,6 +152,15 @@ func TestQueueSplitsSkewedNeighborPortsWithinDefaultDepth(t *testing.T) { } else { profile.Spec.Egress = skewedPortNeighbors(heavyIndex) } + limitProfile := testProfile() + limitedPeer := portSplitNeighbor() + limitedPeer.Ports = limitedPeer.Ports[:4] + if direction == "ingress" { + limitProfile.Spec.Ingress = []v1beta1.NetworkNeighbor{limitedPeer} + } else { + limitProfile.Spec.Egress = []v1beta1.NetworkNeighbor{limitedPeer} + } + creator.limit = limitProfile.Size() require.NoError(t, q.Enqueue(profile, "container")) for range DefaultMaxSplitDepth + 2 { q.processAllItems() @@ -162,3 +179,98 @@ func TestQueueSplitsSkewedNeighborPortsWithinDefaultDepth(t *testing.T) { } } } + +// byteLimitedCreator models storage's transport rejection using encoded protobuf size. +type byteLimitedCreator struct { + limit int + accepted []*v1beta1.ContainerProfile +} + +// CreateContainerProfileDirect rejects oversized wire payloads and records accepted copies. +func (c *byteLimitedCreator) CreateContainerProfileDirect(p *v1beta1.ContainerProfile) error { + if p.Size() > c.limit { + return genericStatusError(http.StatusRequestEntityTooLarge) + } + c.accepted = append(c.accepted, p.DeepCopy()) + return nil +} + +// TestQueueSplitsSelectorHeavyNeighbors verifies numerous large identities are balanced +// even when another peer's large port count would dominate an observation-count cut. +func TestQueueSplitsSelectorHeavyNeighbors(t *testing.T) { + for _, direction := range []string{"ingress", "egress"} { + t.Run(direction, func(t *testing.T) { + profile := testProfile() + heavyPorts := portSplitNeighbor() + heavyPorts.Ports = nil + for i := range 512 { + heavyPorts.Ports = append(heavyPorts.Ports, v1beta1.NetworkPort{Name: fmt.Sprintf("TCP-%d", 8000+i), Protocol: "TCP", Port: new(int32(8000 + i))}) + } + neighbors := []v1beta1.NetworkNeighbor{heavyPorts} + for i := range 40 { + peer := portSplitNeighbor() + peer.Identifier = fmt.Sprintf("selector-peer-%d", i) + peer.Ports = peer.Ports[:1] + for j := range 1500 { + peer.PodSelector.MatchLabels[fmt.Sprintf("label-%04d", j)] = strings.Repeat("v", 63) + } + neighbors = append(neighbors, peer) + } + if direction == "ingress" { + profile.Spec.Ingress = neighbors + } else { + profile.Spec.Egress = neighbors + } + creator := &byteLimitedCreator{limit: 3 * 1024 * 1024} + require.Greater(t, profile.Size(), creator.limit) + q, err := NewQueueData(context.Background(), creator, QueueConfig{QueueDir: t.TempDir(), MaxQueueSize: 100}) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, q.Close()) }) + require.NoError(t, q.Enqueue(profile, "container")) + for range DefaultMaxSplitDepth + 2 { + q.processAllItems() + } + var observations []string + var rows []tsRow + for _, accepted := range creator.accepted { + peers := accepted.Spec.Ingress + if direction == "egress" { + peers = accepted.Spec.Egress + } + for _, peer := range peers { + var original *v1beta1.NetworkNeighbor + for i := range neighbors { + if neighbors[i].Identifier == peer.Identifier { + original = &neighbors[i] + break + } + } + require.NotNil(t, original) + require.Equal(t, *original, peer, "selectors and ports must stay intact") + } + observations = append(observations, elementSignatures(&accepted.Spec)...) + rows = append(rows, tsRow{PreviousReportTimestamp: accepted.Annotations[helpersv1.PreviousReportTimestampMetadataKey], ReportTimestamp: accepted.Annotations[helpersv1.ReportTimestampMetadataKey]}) + } + require.ElementsMatch(t, elementSignatures(&profile.Spec), observations) + assertChainIsLinear(t, rows, profile.Annotations[helpersv1.PreviousReportTimestampMetadataKey], profile.Annotations[helpersv1.ReportTimestampMetadataKey]) + require.Zero(t, q.chunksDropped.Load()) + require.Zero(t, q.GetQueueSize()) + }) + } +} + +// TestHalveNeighborsAvoidsDuplicatingLargeIdentity verifies that splitting a peer's +// ports is rejected when copying its selector would make the larger half larger. +func TestHalveNeighborsAvoidsDuplicatingLargeIdentity(t *testing.T) { + heavy := portSplitNeighbor() + heavy.Ports = heavy.Ports[:2] + for i := range 40 { + heavy.PodSelector.MatchLabels[fmt.Sprintf("label-%d", i)] = strings.Repeat("v", 63) + } + small := portSplitNeighbor() + small.Identifier = "small" + small.Ports = small.Ports[:1] + a, b := halveNeighbors([]v1beta1.NetworkNeighbor{heavy, small}) + require.Equal(t, []v1beta1.NetworkNeighbor{heavy}, a) + require.Equal(t, []v1beta1.NetworkNeighbor{small}, b) +} diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_split.go b/pkg/containerprofilemanager/v1/queue/containerprofile_split.go index e979f183f4..e10f309f52 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_split.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_split.go @@ -245,29 +245,68 @@ func countNeighborElements(neighbors []v1beta1.NetworkNeighbor) int { return count } -// halveNeighbors partitions the ordered observations at half their total weight, -// counting every port and each portless peer once. Only a peer crossing that boundary -// is copied into both chunks, so large port lists consume the same split-depth budget -// as other observations without duplicating every peer's identity. +// halveNeighbors chooses the ordered cut with the smallest larger half, estimating +// bytes from JSON as splitProfile's progress guard does. Whole-peer cuts are preferred +// on ties. Interior port cuts charge the duplicated peer identity to both halves, so +// large selectors are balanced without needlessly copying them. Each neighbor and +// port is encoded once; scoring all cuts is linear in the number of observations. func halveNeighbors(neighbors []v1beta1.NetworkNeighbor) ([]v1beta1.NetworkNeighbor, []v1beta1.NetworkNeighbor) { - remaining := (countNeighborElements(neighbors) + 1) / 2 + if len(neighbors) == 0 { + return neighbors, nil + } + sizes := make([]int, len(neighbors)) + total := 0 for i := range neighbors { - if remaining == 0 { - return neighbors[:i], neighbors[i:] + encoded, _ := json.Marshal(neighbors[i]) + // Include one separator byte per peer; the list brackets add the same + // constant to each candidate and therefore do not affect the choice. + sizes[i] = len(encoded) + 1 + total += sizes[i] + } + + bestSize, cutPeer, cutPort := total, len(neighbors), 0 + prefix := 0 + for i := 1; i < len(neighbors); i++ { + prefix += sizes[i-1] + if candidate := max(prefix, total-prefix); candidate < bestSize { + bestSize, cutPeer = candidate, i } - weight := max(1, len(neighbors[i].Ports)) - if remaining < weight { - left, right := neighbors[i].DeepCopy(), neighbors[i].DeepCopy() - left.Ports = left.Ports[:remaining] - right.Ports = right.Ports[remaining:] - a := append([]v1beta1.NetworkNeighbor(nil), neighbors[:i]...) - a = append(a, *left) - b := append([]v1beta1.NetworkNeighbor{*right}, neighbors[i+1:]...) - return a, b + } + + prefix = 0 + for i, neighbor := range neighbors { + if len(neighbor.Ports) > 1 { + portSizes := make([]int, len(neighbor.Ports)) + portsTotal := 0 + for j, port := range neighbor.Ports { + encoded, _ := json.Marshal(port) + portSizes[j] = len(encoded) + 1 + portsTotal += portSizes[j] + } + portsPrefix := 0 + for j := 1; j < len(neighbor.Ports); j++ { + portsPrefix += portSizes[j-1] + // Removing ports also removes one comma per port while each + // nonempty half retains the peer's full identity and ports wrapper. + left := prefix + sizes[i] - (portsTotal - portsPrefix) + right := total - prefix - portsPrefix + if candidate := max(left, right); candidate < bestSize { + bestSize, cutPeer, cutPort = candidate, i, j + } + } } - remaining -= weight + prefix += sizes[i] + } + if cutPort == 0 { + return neighbors[:cutPeer], neighbors[cutPeer:] } - return neighbors, nil + left, right := neighbors[cutPeer].DeepCopy(), neighbors[cutPeer].DeepCopy() + left.Ports = left.Ports[:cutPort] + right.Ports = right.Ports[cutPort:] + a := append([]v1beta1.NetworkNeighbor(nil), neighbors[:cutPeer]...) + a = append(a, *left) + b := append([]v1beta1.NetworkNeighbor{*right}, neighbors[cutPeer+1:]...) + return a, b } // moveOneElement transfers a single element from the first non-empty partitionable field of From 23c1a952686a8f59d58e255584c0d31709f072f7 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 14:33:31 +0200 Subject: [PATCH 12/19] fix(cpm): budget all materialized profile payloads Address CodeRabbit review 5414468354 by applying the queue size limit after full peer materialization, including already-resolved Services. Remove the unused unresolved-peer predicate and verify full selector/port preservation with bounded chunks and continuous timestamps. Signed-off-by: Matthias Bertschy --- .../v1/late_resolution_size.go | 16 --------- .../v1/late_resolution_size_test.go | 36 ++++++++++++++----- pkg/containerprofilemanager/v1/monitoring.go | 8 ++--- 3 files changed, 29 insertions(+), 31 deletions(-) delete mode 100644 pkg/containerprofilemanager/v1/late_resolution_size.go diff --git a/pkg/containerprofilemanager/v1/late_resolution_size.go b/pkg/containerprofilemanager/v1/late_resolution_size.go deleted file mode 100644 index 4478d20762..0000000000 --- a/pkg/containerprofilemanager/v1/late_resolution_size.go +++ /dev/null @@ -1,16 +0,0 @@ -package containerprofilemanager - -// Raw observations may acquire selectors and additional Service ports only at -// flush time. Apply the size budget to their materialized profile, which also -// freezes those ports across any subsequent queue splits. -func hasUnresolvedNetworkPeers(data *containerData) bool { - if data.networks == nil { - return false - } - for _, event := range data.networks.ToSlice() { - if event.Destination.Kind != EndpointKindPod && event.Destination.Kind != EndpointKindService { - return true - } - } - return false -} diff --git a/pkg/containerprofilemanager/v1/late_resolution_size_test.go b/pkg/containerprofilemanager/v1/late_resolution_size_test.go index b15974aadd..a1db8a7462 100644 --- a/pkg/containerprofilemanager/v1/late_resolution_size_test.go +++ b/pkg/containerprofilemanager/v1/late_resolution_size_test.go @@ -24,8 +24,16 @@ import ( // TestSaveContainerProfile_LateResolutionSizeBudget checks that resolved peers split within budget while preserving ports and report order. func TestSaveContainerProfile_LateResolutionSizeBudget(t *testing.T) { - for _, kind := range []EndpointKind{EndpointKindPod, EndpointKindService} { - t.Run(string(kind), func(t *testing.T) { + for _, tc := range []struct { + name string + peerKind EndpointKind + eventKind EndpointKind + }{ + {name: "raw pod", peerKind: EndpointKindPod, eventKind: EndpointKindRaw}, + {name: "raw service", peerKind: EndpointKindService, eventKind: EndpointKindRaw}, + {name: "resolved service", peerKind: EndpointKindService, eventKind: EndpointKindService}, + } { + t.Run(tc.name, func(t *testing.T) { t.Setenv("QUEUE_DIR", t.TempDir()) sink := &storage.StorageHttpClientMock{} manager, err := NewContainerProfileManager(context.Background(), config.Config{}, nil, nil, sink, nil, &seccompmanager.SeccompManagerMock{}, nil, nil, nil) @@ -43,16 +51,24 @@ func TestSaveContainerProfile_LateResolutionSizeBudget(t *testing.T) { labels[key] = strings.Repeat("v", 63) serviceLabels[key] = labels[key] } - if kind == EndpointKindService { + if tc.peerKind == EndpointKindService { manager.k8sClient = &servicePortTestClient{service: newServiceWorkload("api", serviceLabels, map[string]any{"name": "web", "port": 80, "targetPort": "http", "protocol": "TCP"}), kubeClient: fake.NewClientset()} } for i := range 2 { ip := fmt.Sprintf("10.0.0.%d", i+1) namespace := fmt.Sprintf("peer-%d", i) - event := NetworkEvent{Port: 80, Protocol: "tcp", PktType: utils.OutgoingPktType, Destination: Destination{Kind: EndpointKindRaw, IPAddress: ip}} + event := NetworkEvent{Port: 80, Protocol: "tcp", PktType: utils.OutgoingPktType, Destination: Destination{Kind: tc.eventKind, IPAddress: ip}} + if tc.eventKind == EndpointKindService { + event.Destination.Name = "api" + event.Destination.Namespace = namespace + if data.servicePorts == nil { + data.servicePorts = make(map[NetworkEvent][]uint16) + } + data.servicePorts[event] = []uint16{8080, 9090, 10000} + } data.networks.Add(event) data.size.Add(int64(size.Of(event) + networkNeighborIncrement(data, event))) - if kind == EndpointKindService { + if tc.peerKind == EndpointKindService { for j, port := range []int32{8080, 9090, 10000} { slice := newEndpointSlice(fmt.Sprintf("slice-%d", j), "api", discoveryv1.EndpointPort{Name: new("web"), Port: new(port)}) slice.Namespace = namespace @@ -61,20 +77,20 @@ func TestSaveContainerProfile_LateResolutionSizeBudget(t *testing.T) { } } meta := common.SlimObjectMeta{Name: "api", Namespace: namespace, Labels: labels} - if kind == EndpointKindPod { + if tc.peerKind == EndpointKindPod { inventory.podsByIP[ip] = &common.SlimPod{SlimObjectMeta: meta, Status: common.SlimPodStatus{PodIP: ip}} } else { inventory.svcsByIP[ip] = &common.SlimService{SlimObjectMeta: meta, Spec: common.SlimServiceSpec{ClusterIP: ip}} } } - // Each resolved peer fits, but their combined labels exceed the raw-IP budget. + // Each materialized peer fits, but their combined selectors exceed the estimate. neighbors := data.getEgressNetworkNeighbors(watched.ContainerID, container.K8s.Namespace, manager.k8sClient, nil, inventory, nil, false) require.Len(t, neighbors, 2) manager.cfg.MaxTsProfileSize = int64(size.Of(neighbors[0])*3/2 + 1000) require.Less(t, data.size.Load(), manager.cfg.MaxTsProfileSize) require.Greater(t, int64(size.Of(neighbors)), manager.cfg.MaxTsProfileSize) require.NoError(t, manager.saveContainerProfile(watched, container, data, false)) - if kind == EndpointKindService { + if tc.peerKind == EndpointKindService { for i := range 2 { require.NoError(t, manager.k8sClient.(*servicePortTestClient).kubeClient.DiscoveryV1().EndpointSlices(fmt.Sprintf("peer-%d", i)).Delete(context.Background(), "slice-2", metav1.DeleteOptions{})) } @@ -86,7 +102,9 @@ func TestSaveContainerProfile_LateResolutionSizeBudget(t *testing.T) { for _, profile := range profiles { require.LessOrEqual(t, int64(size.Of(profile.Spec)), manager.cfg.MaxTsProfileSize) require.Len(t, profile.Spec.Egress, 1) - if kind == EndpointKindService { + require.NotNil(t, profile.Spec.Egress[0].PodSelector) + require.Equal(t, labels, profile.Spec.Egress[0].PodSelector.MatchLabels) + if tc.peerKind == EndpointKindService { require.Equal(t, []int32{8080, 9090, 10000}, networkPortValues(profile.Spec.Egress[0].Ports)) } require.Equal(t, previous, profile.Annotations[helpersv1.PreviousReportTimestampMetadataKey]) diff --git a/pkg/containerprofilemanager/v1/monitoring.go b/pkg/containerprofilemanager/v1/monitoring.go index 41e87916ca..f5197d2c13 100644 --- a/pkg/containerprofilemanager/v1/monitoring.go +++ b/pkg/containerprofilemanager/v1/monitoring.go @@ -326,12 +326,8 @@ func (cpm *ContainerProfileManager) saveContainerProfile(watchedContainer *objec containerProfile.Annotations[helpersv1.PreviousReportTimestampMetadataKey] = watchedContainer.PreviousReportTimestamp.String() containerProfile.Annotations[helpersv1.ReportTimestampMetadataKey] = watchedContainer.CurrentReportTimestamp.String() - var enqueueErr error - if hasUnresolvedNetworkPeers(containerData) { - enqueueErr = cpm.queueData.EnqueueWithSizeLimit(containerProfile, watchedContainer.ContainerID, cpm.cfg.MaxTsProfileSize) - } else { - enqueueErr = cpm.enqueueContainerProfile(containerProfile, watchedContainer.ContainerID) - } + // Enforce the budget after selectors and Service ports have been materialized. + enqueueErr := cpm.queueData.EnqueueWithSizeLimit(containerProfile, watchedContainer.ContainerID, cpm.cfg.MaxTsProfileSize) if enqueueErr != nil { // Empty the container data to prevent reporting the same data again containerData.emptyEvents() From bb966008a84cabdf59dc44c48a787628e628c5c0 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 14:42:30 +0200 Subject: [PATCH 13/19] fix(cpm): retry Service ports after failed ingestion lookup Address Copilot review 5414587242 by caching Service enforcement ports only after successful non-nil workload lookup. Regression tests cover lookup errors and nil results recovering to the target port, while retaining successful observed-port snapshots. Signed-off-by: Matthias Bertschy --- .../v1/event_reporting.go | 20 ++++----- .../v1/peer_resolution_test.go | 42 +++++++++++++++++++ 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/pkg/containerprofilemanager/v1/event_reporting.go b/pkg/containerprofilemanager/v1/event_reporting.go index 2d5b233b6c..2520902b79 100644 --- a/pkg/containerprofilemanager/v1/event_reporting.go +++ b/pkg/containerprofilemanager/v1/event_reporting.go @@ -360,19 +360,17 @@ func (cpm *ContainerProfileManager) ReportNetworkEvent(containerID string, event return 0, nil } - if networkEvent.Destination.Kind == EndpointKindService { - ports := []uint16{networkEvent.Port} - if cpm.k8sClient != nil { - svc, err := cpm.k8sClient.GetWorkload(networkEvent.Destination.Namespace, "Service", networkEvent.Destination.Name) - if err == nil { - ports = resolveServiceEnforcementPorts(cpm.k8sClient, networkEvent.Destination.Namespace, - networkEvent.Destination.Name, svc, networkEvent.Port, networkEvent.Protocol) + if networkEvent.Destination.Kind == EndpointKindService && cpm.k8sClient != nil { + svc, err := cpm.k8sClient.GetWorkload(networkEvent.Destination.Namespace, "Service", networkEvent.Destination.Name) + // Failed lookups leave no snapshot so flush-time resolution can retry. + if err == nil && svc != nil { + ports := resolveServiceEnforcementPorts(cpm.k8sClient, networkEvent.Destination.Namespace, + networkEvent.Destination.Name, svc, networkEvent.Port, networkEvent.Protocol) + if data.servicePorts == nil { + data.servicePorts = make(map[NetworkEvent][]uint16) } + data.servicePorts[networkEvent] = ports } - if data.servicePorts == nil { - data.servicePorts = make(map[NetworkEvent][]uint16) - } - data.servicePorts[networkEvent] = ports } data.networks.Add(networkEvent) diff --git a/pkg/containerprofilemanager/v1/peer_resolution_test.go b/pkg/containerprofilemanager/v1/peer_resolution_test.go index 984eed81b7..18858058fa 100644 --- a/pkg/containerprofilemanager/v1/peer_resolution_test.go +++ b/pkg/containerprofilemanager/v1/peer_resolution_test.go @@ -576,3 +576,45 @@ func BenchmarkNetworkNeighborsPortScan(b *testing.B) { } } } + +// TestReportNetworkEventRetriesFailedServiceLookup verifies transient lookup failures +// leave ports unresolved, while a successful observed-port fallback stays authoritative. +func TestReportNetworkEventRetriesFailedServiceLookup(t *testing.T) { + for _, scenario := range []string{"lookup error", "nil workload", "successful observed-port fallback"} { + t.Run(scenario, func(t *testing.T) { + cpm, entry := newTestManager(t, "container1") + inv := newMockK8sInventory() + inv.svcsByIP["10.96.0.42"] = &common.SlimService{SlimObjectMeta: common.SlimObjectMeta{Name: "api", Namespace: "default"}} + cpm.SetK8sInventory(inv) + client := &servicePortTestClient{service: newServiceWorkload("api", map[string]any{"app": "api"})} + switch scenario { + case "lookup error": + client.getErr = errors.New("transient Service lookup failure") + case "nil workload": + client.service = nil + } + cpm.k8sClient = client + cpm.ReportNetworkEvent("container1", &utils.StructEvent{ + DstEndpoint: types.L3Endpoint{Addr: "10.96.0.42", Kind: types.EndpointKindRaw}, + DstPort: 80, Proto: "tcp", PktType: utils.OutgoingPktType, + }) + events := entry.data.networks.ToSlice() + require.Len(t, events, 1) + require.Equal(t, EndpointKindService, events[0].Destination.Kind) + _, snapshotted := entry.data.servicePorts[events[0]] + assert.Equal(t, scenario == "successful observed-port fallback", snapshotted) + + client.getErr = nil + client.service = newServiceWorkload("api", map[string]any{"app": "api"}, map[string]any{ + "name": "web", "port": 80, "targetPort": 8080, "protocol": "TCP", + }) + neighbors := entry.data.getEgressNetworkNeighbors("", "default", client, nil, inv, nil, false) + require.Len(t, neighbors, 1) + want := int32(8080) + if scenario == "successful observed-port fallback" { + want = 80 + } + require.Equal(t, []int32{want}, networkPortValues(neighbors[0].Ports)) + }) + } +} From 87499c4e7d1ed41456301a68fe909471273901a6 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 15:41:49 +0200 Subject: [PATCH 14/19] fix(queue): reserve final split level for storage rejection Address CodeRabbit review 5415198967 by retaining a reactive split level when proactive splitting can grow protobuf metadata. Regressions cover accepted parents and authoritative HTTP 413 recovery at depth limits one and four. Signed-off-by: Matthias Bertschy --- .../v1/queue/containerprofile_budget_test.go | 5 +- .../v1/queue/containerprofile_queue.go | 4 +- .../containerprofile_queue_proactive_test.go | 68 +++++++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go index 6a79877649..9206256b36 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_budget_test.go @@ -20,7 +20,7 @@ func TestQueueSizeBudgetRetainsUnsplittableAndDepthLimitedData(t *testing.T) { } t.Run(name, func(t *testing.T) { creator := &MockProfileCreator{} - q, err := NewQueueData(context.Background(), creator, QueueConfig{QueueName: "budget", QueueDir: t.TempDir(), MaxQueueSize: 100, ItemsPerSegment: 10, MaxSplitDepth: 2}) + q, err := NewQueueData(context.Background(), creator, QueueConfig{QueueName: "budget", QueueDir: t.TempDir(), MaxQueueSize: 100, ItemsPerSegment: 10, MaxSplitDepth: 3}) require.NoError(t, err) defer q.Close() original := &v1beta1.ContainerProfile{Name: "profile", Annotations: map[string]string{ @@ -32,7 +32,8 @@ func TestQueueSizeBudgetRetainsUnsplittableAndDepthLimitedData(t *testing.T) { } require.NoError(t, q.EnqueueWithSizeLimit(original, "container", 1)) // Process deterministically without the queue goroutine. The inherited budget - // must split again on the second pass, then send despite the remaining overage. + // must split again on the second pass, then reserve the final level for HTTP 413 + // and send despite the remaining estimated overage. for range 3 { q.processAllItems() } diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go index 1e026a1101..8819603dc6 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go @@ -560,8 +560,10 @@ processLoop: qd.releaseStitch() } + // Reserve the final split level for a real storage rejection: optional splitting + // can grow protobuf timestamp metadata even when the JSON estimate shrinks. if !queuedProfile.IsStitch && queuedProfile.MaxProfileSize > 0 && - queuedProfile.SplitDepth < qd.maxSplitDepth && + queuedProfile.SplitDepth+1 < qd.maxSplitDepth && int64(size.Of(queuedProfile.Profile.Spec)) > queuedProfile.MaxProfileSize { if a, b, ok := splitProfile(queuedProfile.Profile); ok { fallback := qd.requeueSplit(queuedProfile, a, b, false) diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go index 9c50343dad..385bb7cb96 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go @@ -213,3 +213,71 @@ func TestProactiveSplitPreservesDataOnDiskFailure(t *testing.T) { }) } } + +// finalDepthCreator records requests while applying a protobuf payload size limit. +type finalDepthCreator struct { + byteLimitedCreator + attempted []*v1beta1.ContainerProfile +} + +// CreateContainerProfileDirect records the attempted payload before applying storage's cap. +func (c *finalDepthCreator) CreateContainerProfileDirect(profile *v1beta1.ContainerProfile) error { + c.attempted = append(c.attempted, profile.DeepCopy()) + return c.byteLimitedCreator.CreateContainerProfileDirect(profile) +} + +// TestProactiveSplitReservesFinalDepth verifies storage sees a still-acceptable parent +// before a final optional split can grow its protobuf payload through timestamp metadata. +func TestProactiveSplitReservesFinalDepth(t *testing.T) { + for _, maxDepth := range []int{1, DefaultMaxSplitDepth} { + for _, requiresSplit := range []bool{false, true} { + t.Run(fmt.Sprintf("depth=%d/requiresSplit=%t", maxDepth, requiresSplit), func(t *testing.T) { + parent := testProfile() + parent.Annotations[helpersv1.PreviousReportTimestampMetadataKey] = "2026-10-05 10:59:59.99975 +0000 UTC" + parent.Annotations[helpersv1.ReportTimestampMetadataKey] = "2026-10-05 11:00:00 +0000 UTC" + if requiresSplit { + parent.Spec.Ingress = []v1beta1.NetworkNeighbor{portSplitNeighbor()} + } else { + parent.Spec.Opens = []v1beta1.OpenCalls{{Path: "/a"}} + parent.Spec.Syscalls = []string{"poll"} + } + a, b, ok := splitProfile(parent) + require.True(t, ok) + creator := &finalDepthCreator{byteLimitedCreator: byteLimitedCreator{limit: parent.Size()}} + if requiresSplit { + creator.limit = max(a.Size(), b.Size()) + require.Less(t, creator.limit, parent.Size()) + } else { + require.Greater(t, max(a.Size(), b.Size()), parent.Size(), "JSON progress can still enlarge protobuf metadata") + } + q, err := NewQueueData(context.Background(), creator, QueueConfig{QueueDir: t.TempDir(), MaxSplitDepth: maxDepth}) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, q.Close()) }) + q.mu.Lock() + err = q.enqueueLocked(&QueuedContainerProfile{Profile: parent, ContainerID: "container", SplitDepth: maxDepth - 1, MaxProfileSize: 1}) + q.mu.Unlock() + require.NoError(t, err) + for range 3 { + q.processAllItems() + } + require.NotEmpty(t, creator.attempted) + require.Equal(t, parent, creator.attempted[0], "the final split level must first try storage") + var observations []string + var rows []tsRow + for _, accepted := range creator.accepted { + observations = append(observations, elementSignatures(&accepted.Spec)...) + rows = append(rows, tsRow{PreviousReportTimestamp: accepted.Annotations[helpersv1.PreviousReportTimestampMetadataKey], ReportTimestamp: accepted.Annotations[helpersv1.ReportTimestampMetadataKey]}) + } + require.ElementsMatch(t, elementSignatures(&parent.Spec), observations) + assertChainIsLinear(t, rows, parent.Annotations[helpersv1.PreviousReportTimestampMetadataKey], parent.Annotations[helpersv1.ReportTimestampMetadataKey]) + require.Zero(t, q.chunksDropped.Load()) + require.Zero(t, q.GetQueueSize()) + if requiresSplit { + require.Equal(t, int64(1), q.splits.Load(), "HTTP 413 retains the final split level") + } else { + require.Zero(t, q.splits.Load()) + } + }) + } + } +} From 3800bc9836a6a47516ff3853156bbe3f7352d1c3 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 15:41:49 +0200 Subject: [PATCH 15/19] fix(queue): isolate recursive split slice capacities Prevent singleton redistribution from appending into a queued sibling through shared slice capacity. Regressions reproduce sibling corruption for ordinary lists and whole-peer neighbor partitions. Signed-off-by: Matthias Bertschy --- .../v1/queue/containerprofile_split.go | 7 ++--- .../v1/queue/containerprofile_split_test.go | 26 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_split.go b/pkg/containerprofilemanager/v1/queue/containerprofile_split.go index e10f309f52..b62a2505fc 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_split.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_split.go @@ -298,7 +298,7 @@ func halveNeighbors(neighbors []v1beta1.NetworkNeighbor) ([]v1beta1.NetworkNeigh prefix += sizes[i] } if cutPort == 0 { - return neighbors[:cutPeer], neighbors[cutPeer:] + return neighbors[:cutPeer:cutPeer], neighbors[cutPeer:len(neighbors):len(neighbors)] } left, right := neighbors[cutPeer].DeepCopy(), neighbors[cutPeer].DeepCopy() left.Ports = left.Ports[:cutPort] @@ -386,7 +386,8 @@ func freshOneTimeSlug(name string) string { return base + suffix } -// halve returns the first ceil(len(s)/2) elements of s and the rest. +// halve returns the first ceil(len(s)/2) elements of s and the rest, limiting +// capacity so singleton redistribution cannot append into a queued sibling's range. func halve[T any](s []T) ([]T, []T) { if len(s) == 0 { return nil, nil @@ -394,7 +395,7 @@ func halve[T any](s []T) ([]T, []T) { mid := (len(s) + 1) / 2 - return s[:mid], s[mid:] + return s[:mid:mid], s[mid:len(s):len(s)] } // halvePolicies partitions m by sorted key, so the partition is deterministic across runs. diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go index 75dab3a3db..6b98932e77 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_split_test.go @@ -16,6 +16,32 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// TestRecursiveSplitPreservesSibling verifies recursive singleton redistribution +// cannot overwrite an unsent sibling or the original profile through spare slice capacity. +func TestRecursiveSplitPreservesSibling(t *testing.T) { + for _, kind := range []string{"lists", "neighbors"} { + t.Run(kind, func(t *testing.T) { + parent := testProfile() + if kind == "lists" { + parent.Spec.Capabilities = []string{strings.Repeat("A", 128), strings.Repeat("B", 128)} + parent.Spec.Syscalls = []string{strings.Repeat("read", 32), strings.Repeat("write", 32)} + } else { + parent.Spec.Ingress = []v1beta1.NetworkNeighbor{{Identifier: strings.Repeat("ingress-A", 32)}, {Identifier: strings.Repeat("ingress-B", 32)}} + parent.Spec.Egress = []v1beta1.NetworkNeighbor{{Identifier: strings.Repeat("egress-A", 32)}, {Identifier: strings.Repeat("egress-B", 32)}} + } + original := parent.DeepCopy() + a, b, ok := splitProfile(parent) + require.True(t, ok) + sibling := b.DeepCopy() + // This split redistributes one singleton into its empty second half. + // Even a rejected split must leave the queued sibling intact. + splitProfile(a) + require.Equal(t, sibling, b) + require.Equal(t, original, parent) + }) + } +} + // tsRow is the reduced shape of storage's TimeSeriesContainers that // consolidateContinuousTimeSeries actually branches on. type tsRow struct { From 0254c9705dc3e8dc6ee320c8f198633d91edfda2 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 15:56:44 +0200 Subject: [PATCH 16/19] fix(queue): reject optional splits that grow protobuf payloads Address Copilot review 5415462616 by checking both child protobuf sizes before proactive admission. A depth-zero regression preserves a storage-acceptable parent whose JSON-smaller child would lose an observation after HTTP 413. Signed-off-by: Matthias Bertschy --- .../v1/queue/containerprofile_queue.go | 8 +++-- .../containerprofile_queue_proactive_test.go | 34 +++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go index 8819603dc6..c02c502bcb 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue.go @@ -560,12 +560,14 @@ processLoop: qd.releaseStitch() } - // Reserve the final split level for a real storage rejection: optional splitting - // can grow protobuf timestamp metadata even when the JSON estimate shrinks. + // Reserve the final split level for a real storage rejection. if !queuedProfile.IsStitch && queuedProfile.MaxProfileSize > 0 && queuedProfile.SplitDepth+1 < qd.maxSplitDepth && int64(size.Of(queuedProfile.Profile.Spec)) > queuedProfile.MaxProfileSize { - if a, b, ok := splitProfile(queuedProfile.Profile); ok { + // JSON progress can hide protobuf timestamp growth. Optional splitting + // must not turn an acceptable parent into a larger wire payload. + parentWireSize := queuedProfile.Profile.Size() + if a, b, ok := splitProfile(queuedProfile.Profile); ok && a.Size() <= parentWireSize && b.Size() <= parentWireSize { fallback := qd.requeueSplit(queuedProfile, a, b, false) if fallback != queuedProfile { qd.splits.Add(1) diff --git a/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go index 385bb7cb96..891b6117d8 100644 --- a/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go +++ b/pkg/containerprofilemanager/v1/queue/containerprofile_queue_proactive_test.go @@ -281,3 +281,37 @@ func TestProactiveSplitReservesFinalDepth(t *testing.T) { } } } + +// TestProactiveSplitPreservesWireSizeAtInitialDepth verifies optional splitting cannot +// turn a storage-acceptable parent into an oversized, indivisible protobuf child. +func TestProactiveSplitPreservesWireSizeAtInitialDepth(t *testing.T) { + parent := testProfile() + parent.Annotations[helpersv1.PreviousReportTimestampMetadataKey] = "2026-10-05 10:59:59.99975 +0000 UTC" + parent.Annotations[helpersv1.ReportTimestampMetadataKey] = "2026-10-05 11:00:00 +0000 UTC" + parent.Spec.Opens = []v1beta1.OpenCalls{{Path: "/a"}} + parent.Spec.Syscalls = []string{"poll"} + a, b, ok := splitProfile(parent) + require.True(t, ok) + require.Greater(t, max(a.Size(), b.Size()), parent.Size()) + + creator := &finalDepthCreator{byteLimitedCreator: byteLimitedCreator{limit: parent.Size()}} + q, err := NewQueueData(context.Background(), creator, QueueConfig{QueueDir: t.TempDir()}) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, q.Close()) }) + require.NoError(t, q.EnqueueWithSizeLimit(parent, "container", 1)) + for range DefaultMaxSplitDepth + 2 { + q.processAllItems() + } + + require.Zero(t, q.chunksDropped.Load(), "an optional split must not create a storage rejection") + require.Len(t, creator.attempted, 1) + require.Equal(t, parent, creator.attempted[0]) + require.Len(t, creator.accepted, 1) + require.Equal(t, elementSignatures(&parent.Spec), elementSignatures(&creator.accepted[0].Spec)) + assertChainIsLinear(t, []tsRow{{ + PreviousReportTimestamp: creator.accepted[0].Annotations[helpersv1.PreviousReportTimestampMetadataKey], + ReportTimestamp: creator.accepted[0].Annotations[helpersv1.ReportTimestampMetadataKey], + }}, parent.Annotations[helpersv1.PreviousReportTimestampMetadataKey], parent.Annotations[helpersv1.ReportTimestampMetadataKey]) + require.Zero(t, q.splits.Load()) + require.Zero(t, q.GetQueueSize()) +} From 39730cf90e44723091ea7ce501275467160e0708 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 15:56:44 +0200 Subject: [PATCH 17/19] fix(cpm): preserve retry windows and pending status reports Address maintainer review 5415214805 by retaining per-event deferral deadlines across rapid size-triggered saves and emitting unreported metadata transitions even when peers remain deferred. Cover deadline expiry, informer recovery, forced flushes, pruning, status updates, and report-chain continuity. Signed-off-by: Matthias Bertschy --- .../v1/container_data.go | 59 +++++++++--- .../v1/containerprofile_manager.go | 4 + pkg/containerprofilemanager/v1/monitoring.go | 6 +- .../v1/monitoring_timestamp_test.go | 89 +++++++++++++++++++ .../v1/peer_resolution_test.go | 58 ++++++++++++ 5 files changed, 201 insertions(+), 15 deletions(-) diff --git a/pkg/containerprofilemanager/v1/container_data.go b/pkg/containerprofilemanager/v1/container_data.go index 7e6d9e53a5..f1401ee34c 100644 --- a/pkg/containerprofilemanager/v1/container_data.go +++ b/pkg/containerprofilemanager/v1/container_data.go @@ -3,6 +3,7 @@ package containerprofilemanager import ( "net" "sort" + "time" "github.com/DmitriyVTitov/size" mapset "github.com/deckarep/golang-set/v2" @@ -41,6 +42,14 @@ func (cd *containerData) emptyEvents() { if len(cd.servicePorts) == 0 { cd.servicePorts = nil } + for event := range cd.networkDeferredUntil { + if !cd.networks.Contains(event) { + delete(cd.networkDeferredUntil, event) + } + } + if len(cd.networkDeferredUntil) == 0 { + cd.networkDeferredUntil = nil + } var retainedSize int64 for _, ev := range cd.networks.ToSlice() { retainedSize += int64(size.Of(ev) + networkNeighborIncrement(cd, ev)) @@ -51,6 +60,7 @@ func (cd *containerData) emptyEvents() { cd.prevDeferredNetworks = nil cd.deferredNetworks = nil cd.servicePorts = nil + cd.networkDeferredUntil = nil } if cd.watchedContainerData != nil { cd.lastReportedCompletion = string(cd.watchedContainerData.GetCompletionStatus()) @@ -71,12 +81,14 @@ func (cd *containerData) isEmpty() bool { return false } - if cd.watchedContainerData == nil { - return true - } + return !cd.hasUnreportedStatusChange() +} - return cd.lastReportedCompletion == string(cd.watchedContainerData.GetCompletionStatus()) && - cd.lastReportedStatus == string(cd.watchedContainerData.GetStatus()) +// hasUnreportedStatusChange reports whether a metadata-only update still needs saving. +func (cd *containerData) hasUnreportedStatusChange() bool { + return cd.watchedContainerData != nil && + (cd.lastReportedCompletion != string(cd.watchedContainerData.GetCompletionStatus()) || + cd.lastReportedStatus != string(cd.watchedContainerData.GetStatus())) } // getCapabilities returns a sorted slice of capabilities @@ -315,6 +327,32 @@ func appendNetworkNeighbor(neighbors []v1beta1.NetworkNeighbor, seen map[string] return append(neighbors, neighbor) } +// deferNetworkEvent retains an unresolved observation until its first deadline. +// Without a configured duration, retain the existing one-flush retry behavior. +func (cd *containerData) deferNetworkEvent(event NetworkEvent) bool { + if cd.networkDeferralDuration > 0 { + now := time.Now() + deadline, exists := cd.networkDeferredUntil[event] + if !exists { + if cd.networkDeferredUntil == nil { + cd.networkDeferredUntil = make(map[NetworkEvent]time.Time) + } + deadline = now.Add(cd.networkDeferralDuration) + cd.networkDeferredUntil[event] = deadline + } + if !now.Before(deadline) { + return false + } + } else if cd.prevDeferredNetworks != nil && cd.prevDeferredNetworks.Contains(event) { + return false + } + if cd.deferredNetworks == nil { + cd.deferredNetworks = mapset.NewSet[NetworkEvent]() + } + cd.deferredNetworks.Add(event) + return true +} + // createNetworkNeighbor creates a network neighbor from a network event func (cd *containerData) createNetworkNeighbor( containerID string, @@ -393,15 +431,8 @@ func (cd *containerData) createNetworkNeighbor( return nil } - // If the IP is inside the cluster pod CIDR / private IP and could not be resolved, - // defer by one flush before persisting as an external IP, unless forceSend is true - // or it has already been deferred once. - if isPrivateIP(networkEvent.Destination.IPAddress) && !forceSend && cd != nil && - (cd.prevDeferredNetworks == nil || !cd.prevDeferredNetworks.Contains(originalEvent)) { - if cd.deferredNetworks == nil { - cd.deferredNetworks = mapset.NewSet[NetworkEvent]() - } - cd.deferredNetworks.Add(originalEvent) + // Let inventory catch up before persisting unresolved private traffic as raw IP. + if isPrivateIP(networkEvent.Destination.IPAddress) && !forceSend && cd != nil && cd.deferNetworkEvent(originalEvent) { return nil } diff --git a/pkg/containerprofilemanager/v1/containerprofile_manager.go b/pkg/containerprofilemanager/v1/containerprofile_manager.go index ee17e83d4a..96b21c6a19 100644 --- a/pkg/containerprofilemanager/v1/containerprofile_manager.go +++ b/pkg/containerprofilemanager/v1/containerprofile_manager.go @@ -67,6 +67,10 @@ type containerData struct { prevDeferredNetworks mapset.Set[NetworkEvent] droppedEvents bool // Indicates if any events were dropped during monitoring + // Positive durations give unresolved peers an informer catch-up window across rapid saves. + networkDeferralDuration time.Duration + networkDeferredUntil map[NetworkEvent]time.Time + // Service port snapshots keep report-time accounting and serialization consistent. servicePorts map[NetworkEvent][]uint16 diff --git a/pkg/containerprofilemanager/v1/monitoring.go b/pkg/containerprofilemanager/v1/monitoring.go index f5197d2c13..6b9ae9b9b8 100644 --- a/pkg/containerprofilemanager/v1/monitoring.go +++ b/pkg/containerprofilemanager/v1/monitoring.go @@ -267,6 +267,9 @@ func (cpm *ContainerProfileManager) saveContainerProfile(watchedContainer *objec if containerData.isEmpty() && !forceSend { // TODO: Also check if the seccomp profile is new (currently not implemented) return nil } + // Size-triggered flushes must give unresolved peers the same minimum retry + // window as interval-triggered flushes. + containerData.networkDeferralDuration = cpm.cfg.UpdateDataPeriod containerProfile := &v1beta1.ContainerProfile{ Name: slug, @@ -305,7 +308,8 @@ func (cpm *ContainerProfileManager) saveContainerProfile(watchedContainer *objec }, } - if !forceSend && containerData.deferredNetworks != nil && containerData.deferredNetworks.Cardinality() > 0 && + if !forceSend && !containerData.hasUnreportedStatusChange() && + containerData.deferredNetworks != nil && containerData.deferredNetworks.Cardinality() > 0 && len(containerProfile.Spec.Capabilities) == 0 && len(containerProfile.Spec.Execs) == 0 && len(containerProfile.Spec.Opens) == 0 && diff --git a/pkg/containerprofilemanager/v1/monitoring_timestamp_test.go b/pkg/containerprofilemanager/v1/monitoring_timestamp_test.go index 9ec760a279..2ff7ee9010 100644 --- a/pkg/containerprofilemanager/v1/monitoring_timestamp_test.go +++ b/pkg/containerprofilemanager/v1/monitoring_timestamp_test.go @@ -6,9 +6,11 @@ import ( "time" mapset "github.com/deckarep/golang-set/v2" + "github.com/inspektor-gadget/inspektor-gadget/pkg/operators/common" helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" "github.com/kubescape/node-agent/pkg/config" "github.com/kubescape/node-agent/pkg/hostidentity" + "github.com/kubescape/node-agent/pkg/objectcache" "github.com/kubescape/node-agent/pkg/seccompmanager" "github.com/kubescape/node-agent/pkg/storage" "github.com/kubescape/node-agent/pkg/utils" @@ -41,6 +43,9 @@ func TestSaveContainerProfile_DeferredOnlyPreservesReportTimestamps(t *testing.T wantReports++ } previous, current := watched.PreviousReportTimestamp, watched.CurrentReportTimestamp + // Isolate a deferred-only flush with no unreported lifecycle change. + data.lastReportedCompletion = string(watched.GetCompletionStatus()) + data.lastReportedStatus = string(watched.GetStatus()) data.networks = mapset.NewSet(NetworkEvent{ Port: 443, Protocol: "tcp", PktType: utils.OutgoingPktType, Destination: Destination{Kind: EndpointKindRaw, IPAddress: "10.50.1.20"}, @@ -72,3 +77,87 @@ func TestSaveContainerProfile_DeferredOnlyPreservesReportTimestamps(t *testing.T }) } } + +// TestSaveContainerProfile_RapidFlushesRetainUnresolvedPeer verifies back-to-back +// size flushes keep a peer retryable until the informer can supply its identity. +func TestSaveContainerProfile_RapidFlushesRetainUnresolvedPeer(t *testing.T) { + t.Setenv("QUEUE_DIR", t.TempDir()) + sink := &storage.StorageHttpClientMock{} + cpm, err := NewContainerProfileManager(context.Background(), config.Config{UpdateDataPeriod: time.Minute}, nil, nil, + sink, nil, &seccompmanager.SeccompManagerMock{}, nil, nil, nil) + require.NoError(t, err) + t.Cleanup(cpm.Close) + inventory := newMockK8sInventory() + cpm.k8sInventory = inventory + watched := hostidentity.BuildHostWatchedContainerData("node-1") + container := hostContainerWithIdentity(newHostPseudoContainer(), watched, "kubescape") + data := &containerData{ + watchedContainerData: watched, + lastReportedCompletion: string(watched.GetCompletionStatus()), + lastReportedStatus: string(watched.GetStatus()), + networks: mapset.NewSet(NetworkEvent{Port: 443, Protocol: "tcp", PktType: utils.OutgoingPktType, + Destination: Destination{Kind: EndpointKindRaw, IPAddress: "10.50.1.20"}}), + } + previous := watched.CurrentReportTimestamp + for range 2 { + require.NoError(t, cpm.saveContainerProfile(watched, container, data, false)) + require.Equal(t, previous, watched.CurrentReportTimestamp) + require.Equal(t, 1, data.networks.Cardinality()) + require.Empty(t, sink.ContainerProfilesSnapshot()) + } + inventory.podsByIP["10.50.1.20"] = &common.SlimPod{ + SlimObjectMeta: common.SlimObjectMeta{Name: "late", Namespace: "peer", Labels: map[string]string{"app": "late"}}, + Status: common.SlimPodStatus{PodIP: "10.50.1.20"}, + } + require.NoError(t, cpm.saveContainerProfile(watched, container, data, false)) + require.Eventually(t, func() bool { return len(sink.ContainerProfilesSnapshot()) == 1 }, 8*time.Second, 10*time.Millisecond) + profile := sink.ContainerProfilesSnapshot()[0] + require.Len(t, profile.Spec.Egress, 1) + require.Empty(t, profile.Spec.Egress[0].IPAddress) + require.Equal(t, map[string]string{"app": "late"}, profile.Spec.Egress[0].PodSelector.MatchLabels) + require.Equal(t, previous.String(), profile.Annotations[helpersv1.PreviousReportTimestampMetadataKey]) + require.Empty(t, data.networkDeferredUntil) +} + +// TestSaveContainerProfile_DeferredOnlyReportsStatusChanges verifies deferred peers +// cannot hide a completion or status transition, and the later peer follows that report. +func TestSaveContainerProfile_DeferredOnlyReportsStatusChanges(t *testing.T) { + for _, transition := range []string{"dropped events", "status"} { + t.Run(transition, func(t *testing.T) { + t.Setenv("QUEUE_DIR", t.TempDir()) + sink := &storage.StorageHttpClientMock{} + cpm, err := NewContainerProfileManager(context.Background(), config.Config{}, nil, nil, + sink, nil, &seccompmanager.SeccompManagerMock{}, nil, nil, nil) + require.NoError(t, err) + t.Cleanup(cpm.Close) + watched := hostidentity.BuildHostWatchedContainerData("node-1") + container := hostContainerWithIdentity(newHostPseudoContainer(), watched, "kubescape") + data := &containerData{ + watchedContainerData: watched, + lastReportedCompletion: string(watched.GetCompletionStatus()), + lastReportedStatus: string(watched.GetStatus()), + networks: mapset.NewSet(NetworkEvent{Port: 443, Protocol: "tcp", PktType: utils.OutgoingPktType, + Destination: Destination{Kind: EndpointKindRaw, IPAddress: "10.50.1.20"}}), + } + if transition == "dropped events" { + data.droppedEvents = true + } else { + watched.SetStatus(objectcache.WatchedContainerStatusCompleted) + } + previous := watched.CurrentReportTimestamp + require.NoError(t, cpm.saveContainerProfile(watched, container, data, false)) + require.Eventually(t, func() bool { return len(sink.ContainerProfilesSnapshot()) == 1 }, 8*time.Second, 10*time.Millisecond) + first := sink.ContainerProfilesSnapshot()[0] + require.Empty(t, first.Spec.Egress) + require.Equal(t, string(watched.GetCompletionStatus()), first.Annotations[helpersv1.CompletionMetadataKey]) + require.Equal(t, string(watched.GetStatus()), first.Annotations[helpersv1.StatusMetadataKey]) + require.Equal(t, previous.String(), first.Annotations[helpersv1.PreviousReportTimestampMetadataKey]) + require.Equal(t, 1, data.networks.Cardinality()) + require.NoError(t, cpm.saveContainerProfile(watched, container, data, true)) + require.Eventually(t, func() bool { return len(sink.ContainerProfilesSnapshot()) == 2 }, 8*time.Second, 10*time.Millisecond) + last := sink.ContainerProfilesSnapshot()[1] + require.Len(t, last.Spec.Egress, 1) + require.Equal(t, first.Annotations[helpersv1.ReportTimestampMetadataKey], last.Annotations[helpersv1.PreviousReportTimestampMetadataKey]) + }) + } +} diff --git a/pkg/containerprofilemanager/v1/peer_resolution_test.go b/pkg/containerprofilemanager/v1/peer_resolution_test.go index 18858058fa..5af1955f95 100644 --- a/pkg/containerprofilemanager/v1/peer_resolution_test.go +++ b/pkg/containerprofilemanager/v1/peer_resolution_test.go @@ -3,6 +3,7 @@ package containerprofilemanager import ( "errors" "testing" + "time" "github.com/DmitriyVTitov/size" mapset "github.com/deckarep/golang-set/v2" @@ -618,3 +619,60 @@ func TestReportNetworkEventRetriesFailedServiceLookup(t *testing.T) { }) } } + +// TestNetworkDeferralSurvivesRapidFlushes verifies split-triggered saves cannot consume +// the inventory catch-up window, and expiry, resolution, or final flush releases the event. +func TestNetworkDeferralSurvivesRapidFlushes(t *testing.T) { + for _, kind := range []EndpointKind{EndpointKindRaw, EndpointKindService} { + for _, finish := range []string{"expired", "resolved", "forced"} { + t.Run(string(kind)+"/"+finish, func(t *testing.T) { + event := serviceNetworkEvent(80, "tcp") + event.Destination.Kind = kind + event.Destination.IPAddress = "10.96.0.42" + discarded := serviceNetworkEvent(443, "tcp") + client := &servicePortTestClient{service: newServiceWorkload("api", nil)} + inv := newMockK8sInventory() + cd := &containerData{networks: mapset.NewSet(event), networkDeferralDuration: time.Minute, + networkDeferredUntil: map[NetworkEvent]time.Time{discarded: time.Now().Add(time.Minute)}, + servicePorts: map[NetworkEvent][]uint16{event: {8080}, discarded: {8443}}} + require.Empty(t, cd.getEgressNetworkNeighbors("", "default", client, nil, inv, nil, false)) + cd.emptyEvents() + deadline := cd.networkDeferredUntil[event] + for range 3 { + require.Empty(t, cd.getEgressNetworkNeighbors("", "default", client, nil, inv, nil, false), "rapid saves must keep waiting for inventory") + cd.emptyEvents() + require.True(t, cd.networks.Contains(event)) + require.Equal(t, map[NetworkEvent]time.Time{event: deadline}, cd.networkDeferredUntil, "retain only pending deadlines without extending them") + require.Equal(t, map[NetworkEvent][]uint16{event: {8080}}, cd.servicePorts) + } + require.False(t, deadline.IsZero()) + forceSend := finish == "forced" + switch finish { + case "expired": + cd.networkDeferredUntil[event] = time.Now().Add(-time.Second) + case "resolved": + if kind == EndpointKindService { + client.service = newServiceWorkload("api", map[string]any{"app": "api"}) + } else { + inv.podsByIP[event.Destination.IPAddress] = &common.SlimPod{SlimObjectMeta: common.SlimObjectMeta{Name: "api", Namespace: "default", Labels: map[string]string{"app": "api"}}, Status: common.SlimPodStatus{PodIP: event.Destination.IPAddress}} + } + } + neighbors := cd.getEgressNetworkNeighbors("", "default", client, nil, inv, nil, forceSend) + require.Len(t, neighbors, 1) + if finish == "resolved" { + require.NotNil(t, neighbors[0].PodSelector) + require.Equal(t, "api", neighbors[0].PodSelector.MatchLabels["app"]) + if kind == EndpointKindService { + require.Equal(t, []int32{8080}, networkPortValues(neighbors[0].Ports)) + } + } else { + require.Equal(t, event.Destination.IPAddress, neighbors[0].IPAddress) + } + cd.emptyEvents() + require.Nil(t, cd.networks) + require.Nil(t, cd.networkDeferredUntil) + require.Nil(t, cd.servicePorts) + }) + } + } +} From 76e138030414dfc7f4e79f57550da30fb56abf52 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 16:20:34 +0200 Subject: [PATCH 18/19] fix(profiles): bound pressure flush work to fresh events Address Copilot review 5415699069 by separating fresh network batches from deferred retries. Preserve interval retries, deadlines, cached ports, and status reports. Signed-off-by: Matthias Bertschy --- .../v1/container_data.go | 62 ++++++-- .../v1/containerprofile_manager.go | 7 +- .../v1/event_reporting.go | 4 + pkg/containerprofilemanager/v1/monitoring.go | 14 +- .../v1/monitoring_pressure_test.go | 134 ++++++++++++++++++ .../v1/peer_resolution_test.go | 5 +- 6 files changed, 207 insertions(+), 19 deletions(-) create mode 100644 pkg/containerprofilemanager/v1/monitoring_pressure_test.go diff --git a/pkg/containerprofilemanager/v1/container_data.go b/pkg/containerprofilemanager/v1/container_data.go index f1401ee34c..a318c686d3 100644 --- a/pkg/containerprofilemanager/v1/container_data.go +++ b/pkg/containerprofilemanager/v1/container_data.go @@ -5,7 +5,6 @@ import ( "sort" "time" - "github.com/DmitriyVTitov/size" mapset "github.com/deckarep/golang-set/v2" "github.com/inspektor-gadget/inspektor-gadget/pkg/operators/common" "github.com/kubescape/go-logger" @@ -29,7 +28,38 @@ func (cd *containerData) emptyEvents() { cd.opens = nil cd.rulePolicies = nil cd.callStacks = nil - if cd.deferredNetworks != nil && cd.deferredNetworks.Cardinality() > 0 { + if cd.networkFlushForSize { + // Pressure flushes only consumed the active batch. Keep untouched pending + // peers in place rather than scanning or cloning their growing backlog. + if cd.activeNetworks != nil { + for _, event := range cd.activeNetworks.ToSlice() { + if cd.deferredNetworks != nil && cd.deferredNetworks.Contains(event) { + if cd.prevDeferredNetworks == nil { + cd.prevDeferredNetworks = mapset.NewSet[NetworkEvent]() + } + cd.prevDeferredNetworks.Add(event) + continue + } + cd.networks.Remove(event) + if cd.prevDeferredNetworks != nil { + cd.prevDeferredNetworks.Remove(event) + } + delete(cd.servicePorts, event) + delete(cd.networkDeferredUntil, event) + } + } + cd.deferredNetworks = nil + if cd.networks != nil && cd.networks.Cardinality() == 0 { + cd.networks = nil + cd.prevDeferredNetworks = nil + } + if len(cd.servicePorts) == 0 { + cd.servicePorts = nil + } + if len(cd.networkDeferredUntil) == 0 { + cd.networkDeferredUntil = nil + } + } else if cd.deferredNetworks != nil && cd.deferredNetworks.Cardinality() > 0 { cd.networks = cd.deferredNetworks.Clone() cd.prevDeferredNetworks = cd.deferredNetworks.Clone() cd.deferredNetworks = nil @@ -50,11 +80,6 @@ func (cd *containerData) emptyEvents() { if len(cd.networkDeferredUntil) == 0 { cd.networkDeferredUntil = nil } - var retainedSize int64 - for _, ev := range cd.networks.ToSlice() { - retainedSize += int64(size.Of(ev) + networkNeighborIncrement(cd, ev)) - } - cd.size.Store(retainedSize) } else { cd.networks = nil cd.prevDeferredNetworks = nil @@ -62,6 +87,7 @@ func (cd *containerData) emptyEvents() { cd.servicePorts = nil cd.networkDeferredUntil = nil } + cd.activeNetworks = nil if cd.watchedContainerData != nil { cd.lastReportedCompletion = string(cd.watchedContainerData.GetCompletionStatus()) cd.lastReportedStatus = string(cd.watchedContainerData.GetStatus()) @@ -70,6 +96,7 @@ func (cd *containerData) emptyEvents() { // isEmpty returns true if the container data is empty func (cd *containerData) isEmpty() bool { + networks := cd.networkEventsForFlush(false) if cd.capabilites != nil || cd.syscalls != nil || cd.endpoints != nil || @@ -77,7 +104,7 @@ func (cd *containerData) isEmpty() bool { cd.opens != nil || cd.rulePolicies != nil || cd.callStacks != nil || - cd.networks != nil { + (networks != nil && networks.Cardinality() > 0) { return false } @@ -245,6 +272,15 @@ func resolveEndpoint( } } +// networkEventsForFlush selects fresh observations for pressure saves and all retained +// observations for interval or final saves, without copying either set. +func (cd *containerData) networkEventsForFlush(forceSend bool) mapset.Set[NetworkEvent] { + if cd.networkFlushForSize && !forceSend { + return cd.activeNetworks + } + return cd.networks +} + // getIngressNetworkNeighbors returns ingress network neighbors for this container func (cd *containerData) getIngressNetworkNeighbors( containerID string, @@ -256,12 +292,13 @@ func (cd *containerData) getIngressNetworkNeighbors( forceSend bool, ) []v1beta1.NetworkNeighbor { var ingress []v1beta1.NetworkNeighbor - if cd.networks == nil { + networks := cd.networkEventsForFlush(forceSend) + if networks == nil { return ingress } seen := make(map[string]networkNeighborIndex) - for _, event := range cd.networks.ToSlice() { + for _, event := range networks.ToSlice() { if event.PktType == utils.HostPktType { neighbor := cd.createNetworkNeighbor(containerID, event, namespace, k8sClient, dnsResolverClient, k8sInventory, k8sObjectCache, forceSend) if neighbor == nil { @@ -285,12 +322,13 @@ func (cd *containerData) getEgressNetworkNeighbors( forceSend bool, ) []v1beta1.NetworkNeighbor { var egress []v1beta1.NetworkNeighbor - if cd.networks == nil { + networks := cd.networkEventsForFlush(forceSend) + if networks == nil { return egress } seen := make(map[string]networkNeighborIndex) - for _, event := range cd.networks.ToSlice() { + for _, event := range networks.ToSlice() { if event.PktType != utils.HostPktType { neighbor := cd.createNetworkNeighbor(containerID, event, namespace, k8sClient, dnsResolverClient, k8sInventory, k8sObjectCache, forceSend) if neighbor == nil { diff --git a/pkg/containerprofilemanager/v1/containerprofile_manager.go b/pkg/containerprofilemanager/v1/containerprofile_manager.go index 96b21c6a19..cc986c5e81 100644 --- a/pkg/containerprofilemanager/v1/containerprofile_manager.go +++ b/pkg/containerprofilemanager/v1/containerprofile_manager.go @@ -48,7 +48,8 @@ type containerData struct { monitorDone chan struct{} monitorDoneOnce sync.Once - // Apparent size + // Apparent size of observations collected since the last flush; deferred peers + // remain in networks without being charged to each new active batch. size atomic.Int64 // Cleanup resources @@ -62,7 +63,9 @@ type containerData struct { opens *maps.SafeMap[string, mapset.Set[string]] // Map of opens, key is file path rulePolicies *maps.SafeMap[string, *v1beta1.RulePolicy] // Map of rule policies, key is rule ID callStacks *maps.SafeMap[string, *v1beta1.IdentifiedCallStack] // Map of callstacks, key is SHA256 hash - networks mapset.Set[NetworkEvent] + networks mapset.Set[NetworkEvent] // Union used for deduplication and interval/final retries. + activeNetworks mapset.Set[NetworkEvent] // Newly collected observations since the last flush. + networkFlushForSize bool // Size-triggered saves visit only activeNetworks. deferredNetworks mapset.Set[NetworkEvent] prevDeferredNetworks mapset.Set[NetworkEvent] droppedEvents bool // Indicates if any events were dropped during monitoring diff --git a/pkg/containerprofilemanager/v1/event_reporting.go b/pkg/containerprofilemanager/v1/event_reporting.go index 2520902b79..ce33395e34 100644 --- a/pkg/containerprofilemanager/v1/event_reporting.go +++ b/pkg/containerprofilemanager/v1/event_reporting.go @@ -374,6 +374,10 @@ func (cpm *ContainerProfileManager) ReportNetworkEvent(containerID string, event } data.networks.Add(networkEvent) + if data.activeNetworks == nil { + data.activeNetworks = mapset.NewSet[NetworkEvent]() + } + data.activeNetworks.Add(networkEvent) return size.Of(networkEvent) + networkNeighborIncrement(data, networkEvent), nil }) diff --git a/pkg/containerprofilemanager/v1/monitoring.go b/pkg/containerprofilemanager/v1/monitoring.go index 6b9ae9b9b8..2780cf8e26 100644 --- a/pkg/containerprofilemanager/v1/monitoring.go +++ b/pkg/containerprofilemanager/v1/monitoring.go @@ -161,7 +161,7 @@ func (cpm *ContainerProfileManager) monitorContainer(container *containercollect return ContainerReachedMaxTime case errors.Is(err, ProfileRequiresSplit): - if err := cpm.saveProfile(watchedContainer, container, false); err != nil { + if err := cpm.saveProfileForSize(watchedContainer, container); err != nil { if handledErr := cpm.handleSaveProfileError(err, watchedContainer, container, data); handledErr != nil { return handledErr } @@ -236,6 +236,16 @@ func (cpm *ContainerProfileManager) saveProfile(watchedContainer *objectcache.Wa }) } +// saveProfileForSize flushes newly collected data while leaving pending network +// retries for the interval or final flush. The entry lock guards the flush mode. +func (cpm *ContainerProfileManager) saveProfileForSize(watchedContainer *objectcache.WatchedContainerData, container *containercollection.Container) error { + return cpm.withContainerNoSizeUpdate(watchedContainer.ContainerID, func(data *containerData) error { + data.networkFlushForSize = true + defer func() { data.networkFlushForSize = false }() + return cpm.saveContainerProfile(watchedContainer, container, data, false) + }) +} + // saveContainerProfile saves the container profile to storage func (cpm *ContainerProfileManager) saveContainerProfile(watchedContainer *objectcache.WatchedContainerData, container *containercollection.Container, containerData *containerData, forceSend bool) error { if watchedContainer == nil { @@ -309,7 +319,7 @@ func (cpm *ContainerProfileManager) saveContainerProfile(watchedContainer *objec } if !forceSend && !containerData.hasUnreportedStatusChange() && - containerData.deferredNetworks != nil && containerData.deferredNetworks.Cardinality() > 0 && + containerData.networks != nil && containerData.networks.Cardinality() > 0 && len(containerProfile.Spec.Capabilities) == 0 && len(containerProfile.Spec.Execs) == 0 && len(containerProfile.Spec.Opens) == 0 && diff --git a/pkg/containerprofilemanager/v1/monitoring_pressure_test.go b/pkg/containerprofilemanager/v1/monitoring_pressure_test.go new file mode 100644 index 0000000000..9a2cee5275 --- /dev/null +++ b/pkg/containerprofilemanager/v1/monitoring_pressure_test.go @@ -0,0 +1,134 @@ +package containerprofilemanager + +import ( + "context" + "fmt" + "maps" + "testing" + "time" + + "github.com/inspektor-gadget/inspektor-gadget/pkg/operators/common" + "github.com/inspektor-gadget/inspektor-gadget/pkg/types" + helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" + "github.com/kubescape/node-agent/pkg/config" + "github.com/kubescape/node-agent/pkg/hostidentity" + "github.com/kubescape/node-agent/pkg/objectcache" + "github.com/kubescape/node-agent/pkg/seccompmanager" + "github.com/kubescape/node-agent/pkg/storage" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/client-go/kubernetes/fake" +) + +// pressureInventory counts resolution attempts to expose repeated backlog processing. +type pressureInventory struct { + *mockK8sInventory + lookups int +} + +// GetPodByIp counts each lookup while retaining the mutable informer fixture. +func (i *pressureInventory) GetPodByIp(ip string) *common.SlimPod { + i.lookups++ + return i.mockK8sInventory.GetPodByIp(ip) +} + +// TestPressureFlushDoesNotReprocessDeferredBacklog verifies pressure work follows +// fresh batches while periodic, status-only, and forced saves retain delivery guarantees. +func TestPressureFlushDoesNotReprocessDeferredBacklog(t *testing.T) { + t.Setenv("QUEUE_DIR", t.TempDir()) + sink := &storage.StorageHttpClientMock{} + cpm, err := NewContainerProfileManager(context.Background(), config.Config{UpdateDataPeriod: time.Hour, MaxTsProfileSize: 10 * 1024 * 1024}, nil, nil, + sink, nil, &seccompmanager.SeccompManagerMock{}, nil, nil, nil) + require.NoError(t, err) + t.Cleanup(cpm.Close) + inventory := &pressureInventory{mockK8sInventory: newMockK8sInventory()} + cpm.k8sInventory = inventory + client := &servicePortTestClient{service: newServiceWorkload("api", nil, + map[string]any{"port": 443, "targetPort": 8080, "protocol": "TCP"}), kubeClient: fake.NewClientset()} + cpm.k8sClient = client + inventory.svcsByIP["10.60.3.1"] = &common.SlimService{SlimObjectMeta: common.SlimObjectMeta{Name: "api", Namespace: "peer"}} + watched := hostidentity.BuildHostWatchedContainerData("node-1") + watched.SyncChannel = make(chan error, 8) + container := hostContainerWithIdentity(newHostPseudoContainer(), watched, "kubescape") + data := &containerData{watchedContainerData: watched, + lastReportedCompletion: string(watched.GetCompletionStatus()), lastReportedStatus: string(watched.GetStatus())} + require.True(t, cpm.addContainerEntryIfAbsent(watched.ContainerID, &ContainerEntry{data: data})) + report := func(ip string) { + cpm.ReportNetworkEvent(watched.ContainerID, &utils.StructEvent{ + DstEndpoint: types.L3Endpoint{Addr: ip}, DstPort: 443, Proto: "tcp", PktType: utils.OutgoingPktType}) + } + for i := range 64 { + report(fmt.Sprintf("10.60.1.%d", i+1)) + } + report("10.60.3.1") + cpm.cfg.MaxTsProfileSize = data.size.Load() / 4 + require.Greater(t, data.size.Load(), cpm.cfg.MaxTsProfileSize) + pressureSave := func() error { return cpm.saveProfileForSize(watched, container) } + require.NoError(t, pressureSave()) + assert.Zero(t, data.size.Load(), "retained bytes must not trigger the next pressure flush") + require.Equal(t, 65, data.networks.Cardinality()) + deadlines := maps.Clone(data.networkDeferredUntil) + snapshots := maps.Clone(data.servicePorts) + require.Len(t, snapshots, 1) + for i := range 3 { + report(fmt.Sprintf("10.60.2.%d", i+1)) + assert.Empty(t, watched.SyncChannel, "one small fresh event must not retrigger an oversized deferred batch") + inventory.lookups = 0 + require.NoError(t, pressureSave()) + assert.Equal(t, 1, inventory.lookups, "pressure saves must resolve only the fresh event") + assert.Zero(t, data.size.Load()) + for event, deadline := range deadlines { + require.Equal(t, deadline, data.networkDeferredUntil[event]) + } + for event, ports := range snapshots { + require.Equal(t, ports, data.servicePorts[event]) + } + } + // A stale pressure signal must not retry pending peers or emit an unchanged row. + inventory.lookups = 0 + require.NoError(t, pressureSave()) + assert.Zero(t, inventory.lookups) + require.Empty(t, sink.ContainerProfilesSnapshot()) + for _, event := range data.networks.ToSlice() { + if event.Destination.Kind == EndpointKindService { + continue + } + ip := event.Destination.IPAddress + inventory.podsByIP[ip] = &common.SlimPod{ + SlimObjectMeta: common.SlimObjectMeta{Name: "ready", Namespace: "peer", Labels: map[string]string{"app": "ready"}}, + Status: common.SlimPodStatus{PodIP: ip}, + } + } + client.service = newServiceWorkload("api", map[string]any{"app": "ready"}, + map[string]any{"port": 443, "targetPort": 8080, "protocol": "TCP"}) + inventory.lookups = 0 + require.NoError(t, cpm.saveProfile(watched, container, false)) + require.Equal(t, 67, inventory.lookups, "periodic saves must retry the complete pending batch") + require.Nil(t, data.networks) + require.Empty(t, data.networkDeferredUntil) + require.Empty(t, data.servicePorts) + require.Eventually(t, func() bool { return len(sink.ContainerProfilesSnapshot()) == 1 }, 8*time.Second, 10*time.Millisecond) + first := sink.ContainerProfilesSnapshot()[0] + require.Len(t, first.Spec.Egress, 1) + require.ElementsMatch(t, []int32{443, 8080}, networkPortValues(first.Spec.Egress[0].Ports)) + // Pending-only status transitions must still emit, and a final save releases the peer. + report("10.60.4.1") + require.NoError(t, pressureSave()) + watched.SetCompletionStatus(objectcache.WatchedContainerCompletionStatusPartial) + inventory.lookups = 0 + require.NoError(t, pressureSave()) + assert.Zero(t, inventory.lookups) + require.Eventually(t, func() bool { return len(sink.ContainerProfilesSnapshot()) == 2 }, 8*time.Second, 10*time.Millisecond) + metadata := sink.ContainerProfilesSnapshot()[1] + require.Empty(t, metadata.Spec.Egress) + require.Equal(t, string(objectcache.WatchedContainerCompletionStatusPartial), metadata.Annotations[helpersv1.CompletionMetadataKey]) + require.NoError(t, cpm.saveProfile(watched, container, true)) + require.Eventually(t, func() bool { return len(sink.ContainerProfilesSnapshot()) == 3 }, 8*time.Second, 10*time.Millisecond) + last := sink.ContainerProfilesSnapshot()[2] + require.Len(t, last.Spec.Egress, 1) + require.Equal(t, "10.60.4.1", last.Spec.Egress[0].IPAddress) + require.Equal(t, first.Annotations[helpersv1.ReportTimestampMetadataKey], metadata.Annotations[helpersv1.PreviousReportTimestampMetadataKey]) + require.Equal(t, metadata.Annotations[helpersv1.ReportTimestampMetadataKey], last.Annotations[helpersv1.PreviousReportTimestampMetadataKey]) + assert.Nil(t, data.networks) +} diff --git a/pkg/containerprofilemanager/v1/peer_resolution_test.go b/pkg/containerprofilemanager/v1/peer_resolution_test.go index 5af1955f95..2bf688c135 100644 --- a/pkg/containerprofilemanager/v1/peer_resolution_test.go +++ b/pkg/containerprofilemanager/v1/peer_resolution_test.go @@ -5,7 +5,6 @@ import ( "testing" "time" - "github.com/DmitriyVTitov/size" mapset "github.com/deckarep/golang-set/v2" "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" discoveryv1 "k8s.io/api/discovery/v1" @@ -534,7 +533,7 @@ func TestCreateNetworkNeighbor_ServicePromotionPreservesRawFallback(t *testing.T } } -// TestEmptyEvents_RetainsDeferredServicePortSnapshot checks that deferred observations retain their port snapshot and size until emitted. +// TestEmptyEvents_RetainsDeferredServicePortSnapshot checks that deferred observations retain their port snapshot without recharging the next batch. func TestEmptyEvents_RetainsDeferredServicePortSnapshot(t *testing.T) { event := serviceNetworkEvent(80, "tcp") event.Destination.IPAddress = "10.96.0.42" @@ -544,7 +543,7 @@ func TestEmptyEvents_RetainsDeferredServicePortSnapshot(t *testing.T) { require.Nil(t, cd.createNetworkNeighbor("", event, "default", client, nil, nil, nil, false)) cd.emptyEvents() require.Equal(t, map[NetworkEvent][]uint16{event: {8080, 9090}}, cd.servicePorts) - require.Equal(t, int64(size.Of(event)+networkNeighborIncrement(cd, event)), cd.size.Load()) + require.Zero(t, cd.size.Load(), "retained peers must not consume the next active batch budget") // EndpointSlices change while the Service lookup recovers. client.getErr = nil From 449c7e65da5bffff5a8375d8ede0b1feaed82408 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 5 Oct 2026 16:38:41 +0200 Subject: [PATCH 19/19] fix(profiles): cap deferred network retention Address Copilot review 5416095376 with incremental backlog byte accounting. Deliver overflow using raw-IP fallback and reclaim admission capacity when peers resolve or expire. Signed-off-by: Matthias Bertschy --- .../v1/container_data.go | 56 +++++++-- .../v1/containerprofile_manager.go | 4 + pkg/containerprofilemanager/v1/monitoring.go | 1 + .../v1/monitoring_pressure_test.go | 5 +- .../v1/network_deferral_budget_test.go | 113 ++++++++++++++++++ 5 files changed, 165 insertions(+), 14 deletions(-) create mode 100644 pkg/containerprofilemanager/v1/network_deferral_budget_test.go diff --git a/pkg/containerprofilemanager/v1/container_data.go b/pkg/containerprofilemanager/v1/container_data.go index a318c686d3..625742a926 100644 --- a/pkg/containerprofilemanager/v1/container_data.go +++ b/pkg/containerprofilemanager/v1/container_data.go @@ -5,6 +5,7 @@ import ( "sort" "time" + "github.com/DmitriyVTitov/size" mapset "github.com/deckarep/golang-set/v2" "github.com/inspektor-gadget/inspektor-gadget/pkg/operators/common" "github.com/kubescape/go-logger" @@ -46,6 +47,7 @@ func (cd *containerData) emptyEvents() { } delete(cd.servicePorts, event) delete(cd.networkDeferredUntil, event) + cd.releaseDeferredNetworkSize(event) } } cd.deferredNetworks = nil @@ -80,12 +82,19 @@ func (cd *containerData) emptyEvents() { if len(cd.networkDeferredUntil) == 0 { cd.networkDeferredUntil = nil } + for event := range cd.networkDeferredSizes { + if !cd.networks.Contains(event) { + cd.releaseDeferredNetworkSize(event) + } + } } else { cd.networks = nil cd.prevDeferredNetworks = nil cd.deferredNetworks = nil cd.servicePorts = nil cd.networkDeferredUntil = nil + cd.networkDeferredSizes = nil + cd.networkDeferredSize = 0 } cd.activeNetworks = nil if cd.watchedContainerData != nil { @@ -365,25 +374,48 @@ func appendNetworkNeighbor(neighbors []v1beta1.NetworkNeighbor, seen map[string] return append(neighbors, neighbor) } -// deferNetworkEvent retains an unresolved observation until its first deadline. -// Without a configured duration, retain the existing one-flush retry behavior. +// releaseDeferredNetworkSize removes one consumed observation from the independent +// backlog budget. Pressure cleanup calls this only for events in its active batch. +func (cd *containerData) releaseDeferredNetworkSize(event NetworkEvent) { + if estimate, exists := cd.networkDeferredSizes[event]; exists { + cd.networkDeferredSize -= estimate + delete(cd.networkDeferredSizes, event) + if len(cd.networkDeferredSizes) == 0 { + cd.networkDeferredSizes = nil + } + } +} + +// deferNetworkEvent retains an unresolved observation until its first deadline, +// provided the independent backlog budget has room. Overflow falls through to raw +// delivery. Nonpositive limits preserve the uncapped behavior of zero-config callers. func (cd *containerData) deferNetworkEvent(event NetworkEvent) bool { + now := time.Now() + deadline, hasDeadline := cd.networkDeferredUntil[event] if cd.networkDeferralDuration > 0 { - now := time.Now() - deadline, exists := cd.networkDeferredUntil[event] - if !exists { - if cd.networkDeferredUntil == nil { - cd.networkDeferredUntil = make(map[NetworkEvent]time.Time) - } - deadline = now.Add(cd.networkDeferralDuration) - cd.networkDeferredUntil[event] = deadline - } - if !now.Before(deadline) { + if hasDeadline && !now.Before(deadline) { return false } } else if cd.prevDeferredNetworks != nil && cd.prevDeferredNetworks.Contains(event) { return false } + if _, accounted := cd.networkDeferredSizes[event]; !accounted && cd.networkDeferredSizeLimit > 0 { + estimate := int64(size.Of(event) + networkNeighborIncrement(cd, event)) + if estimate > cd.networkDeferredSizeLimit-cd.networkDeferredSize { + return false + } + if cd.networkDeferredSizes == nil { + cd.networkDeferredSizes = make(map[NetworkEvent]int64) + } + cd.networkDeferredSizes[event] = estimate + cd.networkDeferredSize += estimate + } + if cd.networkDeferralDuration > 0 && !hasDeadline { + if cd.networkDeferredUntil == nil { + cd.networkDeferredUntil = make(map[NetworkEvent]time.Time) + } + cd.networkDeferredUntil[event] = now.Add(cd.networkDeferralDuration) + } if cd.deferredNetworks == nil { cd.deferredNetworks = mapset.NewSet[NetworkEvent]() } diff --git a/pkg/containerprofilemanager/v1/containerprofile_manager.go b/pkg/containerprofilemanager/v1/containerprofile_manager.go index cc986c5e81..f85874dd6e 100644 --- a/pkg/containerprofilemanager/v1/containerprofile_manager.go +++ b/pkg/containerprofilemanager/v1/containerprofile_manager.go @@ -73,6 +73,10 @@ type containerData struct { // Positive durations give unresolved peers an informer catch-up window across rapid saves. networkDeferralDuration time.Duration networkDeferredUntil map[NetworkEvent]time.Time + // Deferred admission is tracked independently from the active flush budget. + networkDeferredSizeLimit int64 + networkDeferredSize int64 + networkDeferredSizes map[NetworkEvent]int64 // Service port snapshots keep report-time accounting and serialization consistent. servicePorts map[NetworkEvent][]uint16 diff --git a/pkg/containerprofilemanager/v1/monitoring.go b/pkg/containerprofilemanager/v1/monitoring.go index 2780cf8e26..39dd4d3d41 100644 --- a/pkg/containerprofilemanager/v1/monitoring.go +++ b/pkg/containerprofilemanager/v1/monitoring.go @@ -280,6 +280,7 @@ func (cpm *ContainerProfileManager) saveContainerProfile(watchedContainer *objec // Size-triggered flushes must give unresolved peers the same minimum retry // window as interval-triggered flushes. containerData.networkDeferralDuration = cpm.cfg.UpdateDataPeriod + containerData.networkDeferredSizeLimit = cpm.cfg.MaxTsProfileSize containerProfile := &v1beta1.ContainerProfile{ Name: slug, diff --git a/pkg/containerprofilemanager/v1/monitoring_pressure_test.go b/pkg/containerprofilemanager/v1/monitoring_pressure_test.go index 9a2cee5275..c0a048f77d 100644 --- a/pkg/containerprofilemanager/v1/monitoring_pressure_test.go +++ b/pkg/containerprofilemanager/v1/monitoring_pressure_test.go @@ -62,8 +62,9 @@ func TestPressureFlushDoesNotReprocessDeferredBacklog(t *testing.T) { report(fmt.Sprintf("10.60.1.%d", i+1)) } report("10.60.3.1") - cpm.cfg.MaxTsProfileSize = data.size.Load() / 4 - require.Greater(t, data.size.Load(), cpm.cfg.MaxTsProfileSize) + // Keep the backlog below its independent admission limit; each flush must + // still leave its bytes out of the active budget and avoid retrying it. + cpm.cfg.MaxTsProfileSize = 2 * data.size.Load() pressureSave := func() error { return cpm.saveProfileForSize(watched, container) } require.NoError(t, pressureSave()) assert.Zero(t, data.size.Load(), "retained bytes must not trigger the next pressure flush") diff --git a/pkg/containerprofilemanager/v1/network_deferral_budget_test.go b/pkg/containerprofilemanager/v1/network_deferral_budget_test.go new file mode 100644 index 0000000000..e1e25e0d66 --- /dev/null +++ b/pkg/containerprofilemanager/v1/network_deferral_budget_test.go @@ -0,0 +1,113 @@ +package containerprofilemanager + +import ( + "maps" + "testing" + "time" + + "github.com/DmitriyVTitov/size" + mapset "github.com/deckarep/golang-set/v2" + "github.com/inspektor-gadget/inspektor-gadget/pkg/operators/common" + "github.com/stretchr/testify/require" +) + +// TestNetworkDeferralBudgetEmitsOverflow verifies repeated fresh batches cannot +// grow deferred memory beyond its budget and overflow observations still reach output. +func TestNetworkDeferralBudgetEmitsOverflow(t *testing.T) { + for _, kind := range []EndpointKind{EndpointKindRaw, EndpointKindService} { + for _, final := range []string{"expired", "forced"} { + t.Run(string(kind)+"/"+final, func(t *testing.T) { + cd := &containerData{networkDeferralDuration: time.Hour, networkFlushForSize: true, networks: mapset.NewSet[NetworkEvent]()} + client := &servicePortTestClient{service: newServiceWorkload("api", nil)} + event := serviceNetworkEvent(80, "tcp") + event.Destination.Kind = kind + event.Destination.IPAddress = "10.96.0.42" + if kind == EndpointKindService { + cd.servicePorts = map[NetworkEvent][]uint16{event: {8080, 9090}} + } + cd.networkDeferredSizeLimit = 2*int64(size.Of(event)+networkNeighborIncrement(cd, event)) + 1 + var emitted []int32 + var deadlines map[NetworkEvent]time.Time + for i := range 10 { + current := event + current.Port = uint16(80 + i) + cd.networks.Add(current) + cd.activeNetworks = mapset.NewSet(current) + if kind == EndpointKindService { + cd.servicePorts[current] = []uint16{8080, 9090} + } + for _, neighbor := range cd.getEgressNetworkNeighbors("", "default", client, nil, nil, nil, false) { + require.Equal(t, current.Destination.IPAddress, neighbor.IPAddress) + emitted = append(emitted, networkPortValues(neighbor.Ports)...) + } + cd.emptyEvents() + require.LessOrEqual(t, cd.networks.Cardinality(), 2, "each pressure flush must leave the backlog bounded") + require.LessOrEqual(t, cd.networkDeferredSize, cd.networkDeferredSizeLimit) + require.Zero(t, cd.size.Load()) + if i == 1 { + deadlines = maps.Clone(cd.networkDeferredUntil) + } + if i > 1 { + require.Equal(t, deadlines, cd.networkDeferredUntil, "overflow must not change admitted deadlines") + } + } + require.Equal(t, 2, cd.networks.Cardinality()) + require.ElementsMatch(t, []int32{82, 83, 84, 85, 86, 87, 88, 89}, emitted) + require.Len(t, cd.networkDeferredSizes, 2) + if kind == EndpointKindService { + require.Len(t, cd.servicePorts, 2) + } + cd.networkFlushForSize = false + if final == "expired" { + for event := range cd.networkDeferredUntil { + cd.networkDeferredUntil[event] = time.Now().Add(-time.Second) + } + } + for _, neighbor := range cd.getEgressNetworkNeighbors("", "default", client, nil, nil, nil, final == "forced") { + emitted = append(emitted, networkPortValues(neighbor.Ports)...) + } + cd.emptyEvents() + require.ElementsMatch(t, []int32{80, 81, 82, 83, 84, 85, 86, 87, 88, 89}, emitted) + require.Nil(t, cd.networks) + require.Nil(t, cd.networkDeferredSizes) + require.Nil(t, cd.networkDeferredUntil) + require.Nil(t, cd.servicePorts) + require.Zero(t, cd.networkDeferredSize) + }) + } + } +} + +// TestNetworkDeferralBudgetReclaimsResolvedEntries verifies periodic delivery frees +// exactly the consumed peer's budget while another pending peer keeps its deadline. +func TestNetworkDeferralBudgetReclaimsResolvedEntries(t *testing.T) { + first := serviceNetworkEvent(80, "tcp") + first.Destination.Kind = EndpointKindRaw + first.Destination.IPAddress = "10.96.0.41" + second := first + second.Destination.IPAddress = "10.96.0.42" + cd := &containerData{networkDeferralDuration: time.Hour, networks: mapset.NewSet(first, second)} + cost := int64(size.Of(first) + networkNeighborIncrement(cd, first)) + cd.networkDeferredSizeLimit = 2 * cost + inv := newMockK8sInventory() + require.Empty(t, cd.getEgressNetworkNeighbors("", "default", nil, nil, inv, nil, false)) + cd.emptyEvents() + require.Equal(t, 2*cost, cd.networkDeferredSize) + deadline := cd.networkDeferredUntil[second] + inv.podsByIP[first.Destination.IPAddress] = &common.SlimPod{SlimObjectMeta: common.SlimObjectMeta{Name: "api", Namespace: "default", Labels: map[string]string{"app": "api"}}, Status: common.SlimPodStatus{PodIP: first.Destination.IPAddress}} + require.Len(t, cd.getEgressNetworkNeighbors("", "default", nil, nil, inv, nil, false), 1) + cd.emptyEvents() + require.Equal(t, cost, cd.networkDeferredSize) + require.Equal(t, map[NetworkEvent]int64{second: cost}, cd.networkDeferredSizes) + require.Equal(t, deadline, cd.networkDeferredUntil[second]) + third := first + third.Destination.IPAddress = "10.96.0.43" + cd.networks.Add(third) + cd.activeNetworks = mapset.NewSet(third) + cd.networkFlushForSize = true + require.Empty(t, cd.getEgressNetworkNeighbors("", "default", nil, nil, inv, nil, false), "released capacity must admit the new peer") + cd.emptyEvents() + require.Equal(t, 2*cost, cd.networkDeferredSize) + require.Equal(t, 2, cd.networks.Cardinality()) + require.Equal(t, deadline, cd.networkDeferredUntil[second]) +}