fix(node-agent): fix ContainerProfile size accounting in ReportSyscall and ReportNetworkEvent

.gitignore

@@ -5,4 +5,4 @@ temp
 resources/ebpf/falco/*
 node-agent
 __pycache__
-tracers.tar
+tracers.tar

pkg/containerprofilemanager/v1/event_reporting.go

@@ -1,6 +1,7 @@
 package containerprofilemanager
 
 import (
+	"crypto/sha256"
 	"errors"
 	"reflect"
 	"regexp"
@@ -15,10 +16,96 @@ import (
 	"github.com/kubescape/node-agent/pkg/utils"
 	"github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1"
 	"github.com/kubescape/storage/pkg/registry/file/dynamicpathdetector"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
 )
 
 var procRegex = regexp.MustCompile(`^/proc/\d+`)
 
+// neighborFixedOverhead is the number of bytes every v1beta1.NetworkNeighbor gains over its
+// source NetworkEvent regardless of Destination.Kind: a generated Identifier hash and a Type
+// string. Both are exactly bounded, so they're measured once rather than guessed:
+// Identifier is hex.EncodeToString of a sha256 sum (always 2*sha256.Size bytes), Type is the
+// longer of "internal"/"external".
+var neighborFixedOverhead = size.Of(strings.Repeat("f", sha256.Size*2)) + size.Of(ExternalTrafficType)
+
+// maxDNSNameEstimate budgets the one field that's genuinely unknowable at report time on
+// container_data.go's raw/DNS branch: DNS resolution happens at serialization, not when the
+// event is reported. RFC 1035 §3.1 bounds an encoded domain name to 253 bytes;
+// createNetworkNeighbor stores it twice (DNS and DNSNames[0]).
+var maxDNSNameEstimate = func() int {
+	maxDNSName := strings.Repeat("a", 253)
+	return size.Of(maxDNSName) + size.Of([]string{maxDNSName})
+}()
+
+// maxBudgetedServiceLabels bounds maxServiceSelectorEstimate below. Kubernetes Services are
+// conventionally selected on a handful of short labels (e.g. "app: foo"), unlike Pods which
+// can carry many more, so this is deliberately smaller than a Pod label budget would be.
+const maxBudgetedServiceLabels = 6
+
+// maxServiceSelectorEstimate budgets the other field genuinely unknowable at report time:
+// on the Service branch, svc.GetServiceSelector() is fetched from the k8s API at
+// serialization time and has no relationship to anything on the raw event. Each of
+// maxBudgetedServiceLabels labels is sized at Kubernetes' per-label maximum (253-byte key,
+// 63-byte value) as generous headroom.
+var maxServiceSelectorEstimate = func() int {
+	maxLabelKey := strings.Repeat("k", 253)
+	maxLabelValue := strings.Repeat("v", 63)
+	labels := make(map[string]string, maxBudgetedServiceLabels)
+	for i := 0; i < maxBudgetedServiceLabels; i++ {
+		// Trailing rune only exists to keep the map keys distinct; length is still ~maxLabelKey.
+		labels[maxLabelKey+string(rune('a'+i))] = maxLabelValue
+	}
+	return size.Of(&metav1.LabelSelector{MatchLabels: labels})
+}()
+
+// networkNeighborIncrement estimates the additional bytes createNetworkNeighbor()
+// (container_data.go) adds beyond the raw NetworkEvent when it builds the eventual
+// v1beta1.NetworkNeighbor. createNetworkNeighbor takes exactly one branch per
+// Destination.Kind, so only that branch's cost is charged - summing every branch
+// unconditionally, as an earlier version of this function did, overcounted by 6-20x and
+// turned the split path from #866 into the normal case instead of a rare backstop.
+//
+// PodSelector on the Pod branch is not budgeted as a guess: its label *bytes* are already on
+// the meter via Destination.PodLabels, a string field size.Of(networkEvent) counts by the
+// caller, but re-shaping that string into map[string]string costs real additional bytes (Go
+// map bucket overhead), so this measures that wrapper delta exactly from the same data
+// filterLabels/GetDestinationPodLabels would produce, rather than assuming it's zero or
+// guessing a label count. Ports and NamespaceSelector are similarly computed exactly from
+// fields already on the event (Port/Protocol, and the destination namespace compared against
+// the container's own), not estimated, since nothing about their content is deferred to
+// serialization.
+func networkNeighborIncrement(data *containerData, networkEvent NetworkEvent) int {
+	est := neighborFixedOverhead + size.Of([]v1beta1.NetworkPort{{
+		Name:     generatePortIdentifierFromEvent(networkEvent),
+		Protocol: v1beta1.Protocol(networkEvent.Protocol),
+		Port:     ptr.To(int32(networkEvent.Port)),
+	}})
+
+	sourceNamespace := ""
+	if data.watchedContainerData != nil {
+		sourceNamespace = data.watchedContainerData.Namespace
+	}
+	if namespaceLabels := getNamespaceMatchLabels(networkEvent.Destination.Namespace, sourceNamespace); namespaceLabels != nil {
+		est += size.Of(&metav1.LabelSelector{MatchLabels: namespaceLabels})
+	}
+
+	switch networkEvent.Destination.Kind {
+	case EndpointKindService:
+		est += maxServiceSelectorEstimate
+	case EndpointKindPod:
+		// The label bytes are already on the meter via Destination.PodLabels; only charge the
+		// extra cost of wrapping them into a LabelSelector's map, and never a negative one.
+		podSelector := &metav1.LabelSelector{MatchLabels: filterLabels(networkEvent.GetDestinationPodLabels())}
+		if delta := size.Of(podSelector) - size.Of(networkEvent.Destination.PodLabels); delta > 0 {
+			est += delta
+		}
+	default:
+		est += maxDNSNameEstimate
+	}
+	return est
+}
+
 // ReportCapability reports a capability event for a container
 func (cpm *ContainerProfileManager) ReportCapability(containerID, capability string) {
 	err := cpm.withContainer(containerID, func(data *containerData) (int, error) {
@@ -273,7 +360,7 @@ func (cpm *ContainerProfileManager) ReportNetworkEvent(containerID string, event
 		}
 
 		data.networks.Add(networkEvent)
-		return size.Of(networkEvent), nil
+		return size.Of(networkEvent) + networkNeighborIncrement(data, networkEvent), nil
 	})
 
 	cpm.logEventError(err, "network", containerID)
@@ -299,7 +386,13 @@ func (cpm *ContainerProfileManager) ReportSyscall(containerID string, syscall st
 		if data.syscalls == nil {
 			data.syscalls = mapset.NewSet[string]()
 		}
-		return data.syscalls.Append(syscall), nil
+		// Append returns the number of elements newly added (0 or 1 here), not their
+		// serialized size - using it directly mixed element-count units into a
+		// byte-size accumulator and made syscalls contribute ~0 to MaxTsProfileSize.
+		if data.syscalls.Append(syscall) == 0 {
+			return 0, nil
+		}
+		return size.Of(syscall), nil
 	})
 
 	cpm.logEventError(err, "syscalls", containerID)

pkg/containerprofilemanager/v1/event_reporting_test.go

@@ -1,6 +1,154 @@
 package containerprofilemanager
 
-import "testing"
+import (
+	"strings"
+	"testing"
+
+	"github.com/DmitriyVTitov/size"
+	mapset "github.com/deckarep/golang-set/v2"
+	"github.com/inspektor-gadget/inspektor-gadget/pkg/types"
+	"github.com/kubescape/node-agent/pkg/config"
+	"github.com/kubescape/node-agent/pkg/objectcache"
+	"github.com/kubescape/node-agent/pkg/utils"
+	"github.com/stretchr/testify/assert"
+)
+
+// fakeDNSResolver resolves every address to a fixed domain, so tests can exercise
+// createNetworkNeighbor's DNS branch without a real dnsmanager.
+type fakeDNSResolver struct{ domain string }
+
+func (f fakeDNSResolver) ResolveIPAddress(string) (string, bool) { return f.domain, true }
+func (f fakeDNSResolver) ResolveContainerProcessToCloudServices(string, uint32) mapset.Set[string] {
+	return nil
+}
+
+// newTestManager builds a ContainerProfileManager with a single, pre-registered
+// container entry, large enough MaxTsProfileSize that these tests never trip the
+// split path, and no watchedContainerData - so a threshold crossing is a no-op
+// instead of blocking on an unbuffered SyncChannel send.
+func newTestManager(t *testing.T, containerID string) (*ContainerProfileManager, *ContainerEntry) {
+	t.Helper()
+	cpm := &ContainerProfileManager{
+		cfg:        config.Config{MaxTsProfileSize: 10 * 1024 * 1024},
+		containers: map[string]*ContainerEntry{},
+	}
+	entry := &ContainerEntry{data: &containerData{}}
+	cpm.addContainerEntry(containerID, entry)
+	return cpm, entry
+}
+
+func TestReportSyscallSizeAccounting(t *testing.T) {
+	cpm, entry := newTestManager(t, "container1")
+
+	cpm.ReportSyscall("container1", "execve")
+	assert.Equal(t, int64(size.Of("execve")), entry.data.size.Load(),
+		"size must grow by the syscall's byte size, not by the set's element-count delta")
+
+	// Re-reporting an already-known syscall is a set-dedup no-op and must not grow the estimate.
+	cpm.ReportSyscall("container1", "execve")
+	assert.Equal(t, int64(size.Of("execve")), entry.data.size.Load())
+
+	cpm.ReportSyscall("container1", "openat")
+	assert.Equal(t, int64(size.Of("execve")+size.Of("openat")), entry.data.size.Load())
+}
+
+func TestReportNetworkEventSizeAccounting(t *testing.T) {
+	cpm, entry := newTestManager(t, "container1")
+
+	event := &utils.StructEvent{
+		DstEndpoint: types.L3Endpoint{
+			Addr: "10.0.0.5",
+		},
+		DstPort: 8080,
+		Proto:   "tcp",
+		PktType: utils.OutgoingPktType,
+	}
+
+	cpm.ReportNetworkEvent("container1", event)
+
+	networkEvent := NetworkEvent{
+		Port:     8080,
+		Protocol: "tcp",
+		PktType:  utils.OutgoingPktType,
+		Destination: Destination{
+			IPAddress: "10.0.0.5",
+		},
+	}
+	want := int64(size.Of(networkEvent) + networkNeighborIncrement(entry.data, networkEvent))
+	assert.Equal(t, want, entry.data.size.Load(),
+		"estimate must include the networkNeighborIncrement surcharge for the identifier/Ports/DNS fields createNetworkNeighbor adds at serialization time")
+
+	// Re-reporting the identical event is a set-dedup no-op and must not grow the estimate.
+	cpm.ReportNetworkEvent("container1", event)
+	assert.Equal(t, want, entry.data.size.Load())
+}
+
+// TestNetworkNeighborIncrementCoversMaxDNSName confirms the report-time estimate does not
+// undercount a NetworkNeighbor carrying the longest legal DNS name (RFC 1035 §3.1, 253
+// bytes) once DNS resolution actually runs at serialization time.
+func TestNetworkNeighborIncrementCoversMaxDNSName(t *testing.T) {
+	maxDNSName := strings.Repeat("a", 253)
+
+	networkEvent := NetworkEvent{
+		Port:     443,
+		Protocol: "tcp",
+		PktType:  utils.OutgoingPktType,
+		Destination: Destination{
+			Kind:      EndpointKindRaw,
+			IPAddress: "203.0.113.10",
+		},
+	}
+
+	cd := &containerData{}
+	neighbor := cd.createNetworkNeighbor(networkEvent, "default", nil, fakeDNSResolver{domain: maxDNSName})
+	if !assert.NotNil(t, neighbor) {
+		return
+	}
+
+	estimate := size.Of(networkEvent) + networkNeighborIncrement(cd, networkEvent)
+	assert.GreaterOrEqual(t, estimate, size.Of(*neighbor),
+		"report-time estimate must cover a resolved NetworkNeighbor with the longest legal DNS name")
+}
+
+// TestNetworkNeighborIncrementCoversSelectorPayload confirms the report-time estimate does
+// not undercount a NetworkNeighbor whose PodSelector/NamespaceSelector are populated from
+// the destination pod's labels at serialization time.
+func TestNetworkNeighborIncrementCoversSelectorPayload(t *testing.T) {
+	podLabels := map[string]string{
+		"app.kubernetes.io/name":       "web",
+		"app.kubernetes.io/instance":   "web-abc123",
+		"app.kubernetes.io/version":    "1.4.2",
+		"app.kubernetes.io/component":  "frontend",
+		"app.kubernetes.io/part-of":    "shop",
+		"app.kubernetes.io/managed-by": "helm",
+	}
+
+	networkEvent := NetworkEvent{
+		Port:     8080,
+		Protocol: "tcp",
+		PktType:  utils.OutgoingPktType,
+		Destination: Destination{
+			Kind:      EndpointKindPod,
+			Namespace: "other-ns",
+			Name:      "web",
+		},
+	}
+	networkEvent.SetDestinationPodLabels(podLabels)
+
+	// The container's own namespace ("default") differs from the destination's ("other-ns"),
+	// so both PodSelector and NamespaceSelector get populated - matching a real cross-namespace
+	// 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)
+	if !assert.NotNil(t, neighbor) {
+		return
+	}
+
+	estimate := size.Of(networkEvent) + networkNeighborIncrement(cd, networkEvent)
+	assert.GreaterOrEqual(t, estimate, size.Of(*neighbor),
+		"report-time estimate must cover a NetworkNeighbor with a populated selector payload")
+}
 
 func TestResolveExecPath(t *testing.T) {
 	tests := []struct {