From 014577ba62335810245f1c25602263bd341dcc5e Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Mon, 21 Oct 2024 14:43:30 +0000 Subject: [PATCH 01/50] Adding base code Signed-off-by: Amit Schendel --- .../applicationprofilecache.go | 161 +++++++++++++++--- 1 file changed, 134 insertions(+), 27 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index 143dadb2..14f0768a 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -3,6 +3,7 @@ package applicationprofilecache import ( "context" "fmt" + "sync" "time" mapset "github.com/deckarep/golang-set/v2" @@ -13,7 +14,6 @@ import ( helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" "github.com/kubescape/k8s-interface/workloadinterface" "github.com/kubescape/node-agent/pkg/objectcache" - "github.com/kubescape/node-agent/pkg/utils" "github.com/kubescape/node-agent/pkg/watcher" "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" versioned "github.com/kubescape/storage/pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1" @@ -47,24 +47,31 @@ func newApplicationProfileState(ap *v1beta1.ApplicationProfile) applicationProfi } type ApplicationProfileCacheImpl struct { - containerToSlug maps.SafeMap[string, string] // cache the containerID to slug mapping, this will enable a quick lookup of the application profile - slugToAppProfile maps.SafeMap[string, *v1beta1.ApplicationProfile] // cache the application profile - slugToContainers maps.SafeMap[string, mapset.Set[string]] // cache the containerIDs that belong to the application profile, this will enable removing from cache AP without pods - slugToState maps.SafeMap[string, applicationProfileState] // cache the containerID to slug mapping, this will enable a quick lookup of the application profile - storageClient versioned.SpdxV1beta1Interface - allProfiles mapset.Set[string] // cache all the application profiles that are ready. this will enable removing from cache AP without pods that are running on the same node - nodeName string - maxDelaySeconds int // maximum delay in seconds before getting the full object from the storage + containerToSlug maps.SafeMap[string, string] // cache the containerID to slug mapping, this will enable a quick lookup of the application profile + slugToAppProfile maps.SafeMap[string, *v1beta1.ApplicationProfile] // cache the application profile + slugToContainers maps.SafeMap[string, mapset.Set[string]] // cache the containerIDs that belong to the application profile, this will enable removing from cache AP without pods + slugToState maps.SafeMap[string, applicationProfileState] // cache the containerID to slug mapping, this will enable a quick lookup of the application profile + storageClient versioned.SpdxV1beta1Interface + allProfiles mapset.Set[string] // cache all the application profiles that are ready. this will enable removing from cache AP without pods that are running on the same node + nodeName string + maxDelaySeconds int // maximum delay in seconds before getting the full object from the storage + userManagedProfiles maps.SafeMap[string, *v1beta1.ApplicationProfile] + pendingMergeProfiles maps.SafeMap[string, *v1beta1.ApplicationProfile] + mergeWaitGroup sync.WaitGroup + mergeTimeout time.Duration } func NewApplicationProfileCache(nodeName string, storageClient versioned.SpdxV1beta1Interface, maxDelaySeconds int) *ApplicationProfileCacheImpl { return &ApplicationProfileCacheImpl{ - nodeName: nodeName, - maxDelaySeconds: maxDelaySeconds, - storageClient: storageClient, - containerToSlug: maps.SafeMap[string, string]{}, - slugToContainers: maps.SafeMap[string, mapset.Set[string]]{}, - allProfiles: mapset.NewSet[string](), + nodeName: nodeName, + maxDelaySeconds: maxDelaySeconds, + storageClient: storageClient, + containerToSlug: maps.SafeMap[string, string]{}, + slugToContainers: maps.SafeMap[string, mapset.Set[string]]{}, + allProfiles: mapset.NewSet[string](), + userManagedProfiles: maps.SafeMap[string, *v1beta1.ApplicationProfile]{}, + pendingMergeProfiles: maps.SafeMap[string, *v1beta1.ApplicationProfile]{}, + mergeTimeout: time.Duration(maxDelaySeconds) * time.Second, } } @@ -213,16 +220,34 @@ func (ap *ApplicationProfileCacheImpl) removeContainer(containerID string) { } // ------------------ watch application profile methods ----------------------- -func (ap *ApplicationProfileCacheImpl) addApplicationProfile(_ context.Context, obj runtime.Object) { +func (ap *ApplicationProfileCacheImpl) addApplicationProfile(ctx context.Context, obj runtime.Object) { appProfile := obj.(*v1beta1.ApplicationProfile) apName := objectcache.MetaUniqueName(appProfile) + isUserManaged := appProfile.Annotations["kubescape.io/managed-by"] == "User" + + if isUserManaged { + ap.handleUserManagedProfile(ctx, appProfile, apName) + } else { + ap.handleNormalProfile(appProfile, apName) + } +} + +func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(ctx context.Context, appProfile *v1beta1.ApplicationProfile, apName string) { + ap.userManagedProfiles.Set(apName, appProfile) + + if normalProfile := ap.slugToAppProfile.Get(apName); normalProfile != nil { + ap.mergeProfiles(normalProfile, appProfile) + } else { + ap.pendingMergeProfiles.Set(apName, appProfile) + ap.waitForNormalProfile(ctx, apName) + } +} + +func (ap *ApplicationProfileCacheImpl) handleNormalProfile(appProfile *v1beta1.ApplicationProfile, apName string) { apState := newApplicationProfileState(appProfile) ap.slugToState.Set(apName, apState) - // the cache holds only completed application profiles. - // check if the application profile is completed - // if status was completed and now is not (e.g. mode changed from complete to partial), remove from cache if apState.status != helpersv1.Completed { if ap.slugToAppProfile.Has(apName) { ap.slugToAppProfile.Delete(apName) @@ -231,19 +256,101 @@ func (ap *ApplicationProfileCacheImpl) addApplicationProfile(_ context.Context, return } - // add to the cache ap.allProfiles.Add(apName) - if ap.slugToContainers.Has(apName) { - // get the full application profile from the storage - // the watch only returns the metadata - // avoid thundering herd problem by adding a random delay - time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { - ap.addFullApplicationProfile(appProfile, apName) - }) + if userManagedProfile := ap.userManagedProfiles.Get(apName); userManagedProfile != nil { + ap.mergeProfiles(appProfile, userManagedProfile) + } else if pendingProfile := ap.pendingMergeProfiles.Get(apName); pendingProfile != nil { + ap.mergeProfiles(appProfile, pendingProfile) + ap.pendingMergeProfiles.Delete(apName) + } else { + ap.addFullApplicationProfile(appProfile, apName) } } +func (ap *ApplicationProfileCacheImpl) mergeProfiles(normalProfile, userManagedProfile *v1beta1.ApplicationProfile) { + mergedProfile := ap.performMerge(normalProfile, userManagedProfile) + ap.slugToAppProfile.Set(objectcache.MetaUniqueName(mergedProfile), mergedProfile) + + if ap.slugToContainers.Has(objectcache.MetaUniqueName(mergedProfile)) { + for _, containerID := range ap.slugToContainers.Get(objectcache.MetaUniqueName(mergedProfile)).ToSlice() { + ap.containerToSlug.Set(containerID, objectcache.MetaUniqueName(mergedProfile)) + } + } + + logger.L().Debug("Merged user-managed profile with normal profile", + helpers.String("name", mergedProfile.GetName()), + helpers.String("namespace", mergedProfile.GetNamespace())) +} + +func (ap *ApplicationProfileCacheImpl) performMerge(normalProfile, userManagedProfile *v1beta1.ApplicationProfile) *v1beta1.ApplicationProfile { + mergedProfile := normalProfile.DeepCopy() + + // Merge spec + mergedProfile.Spec.Containers = ap.mergeContainers(mergedProfile.Spec.Containers, userManagedProfile.Spec.Containers) + mergedProfile.Spec.InitContainers = ap.mergeContainers(mergedProfile.Spec.InitContainers, userManagedProfile.Spec.InitContainers) + mergedProfile.Spec.EphemeralContainers = ap.mergeContainers(mergedProfile.Spec.EphemeralContainers, userManagedProfile.Spec.EphemeralContainers) + + // Remove the user-managed annotation + delete(mergedProfile.Annotations, "kubescape.io/managed-by") + + return mergedProfile +} + +func (ap *ApplicationProfileCacheImpl) mergeContainers(normalContainers, userManagedContainers []v1beta1.ApplicationProfileContainer) []v1beta1.ApplicationProfileContainer { + containerMap := make(map[string]*v1beta1.ApplicationProfileContainer) + + for i := range normalContainers { + containerMap[normalContainers[i].Name] = &normalContainers[i] + } + + for _, userContainer := range userManagedContainers { + if normalContainer, exists := containerMap[userContainer.Name]; exists { + ap.mergeContainer(normalContainer, &userContainer) + } else { + normalContainers = append(normalContainers, userContainer) + } + } + + return normalContainers +} + +func (ap *ApplicationProfileCacheImpl) mergeContainer(normalContainer, userContainer *v1beta1.ApplicationProfileContainer) { + normalContainer.Capabilities = append(normalContainer.Capabilities, userContainer.Capabilities...) + normalContainer.Execs = append(normalContainer.Execs, userContainer.Execs...) + normalContainer.Opens = append(normalContainer.Opens, userContainer.Opens...) + normalContainer.Syscalls = append(normalContainer.Syscalls, userContainer.Syscalls...) + normalContainer.Endpoints = append(normalContainer.Endpoints, userContainer.Endpoints...) +} + +func (ap *ApplicationProfileCacheImpl) waitForNormalProfile(ctx context.Context, apName string) { + ap.mergeWaitGroup.Add(1) + go func() { + defer ap.mergeWaitGroup.Done() + timer := time.NewTimer(ap.mergeTimeout) + defer timer.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-timer.C: + logger.L().Warning("Timeout waiting for normal profile", helpers.String("name", apName)) + ap.pendingMergeProfiles.Delete(apName) + return + default: + if normalProfile := ap.slugToAppProfile.Get(apName); normalProfile != nil { + userManagedProfile := ap.pendingMergeProfiles.Get(apName) + ap.mergeProfiles(normalProfile, userManagedProfile) + ap.pendingMergeProfiles.Delete(apName) + return + } + time.Sleep(100 * time.Millisecond) + } + } + }() +} + func (ap *ApplicationProfileCacheImpl) addFullApplicationProfile(appProfile *v1beta1.ApplicationProfile, apName string) { fullAP, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) if err != nil { From f00da4b0b8ad7fedd7506432b189ba71352fd271 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 08:48:28 +0000 Subject: [PATCH 02/50] Adding fixed code Signed-off-by: Amit Schendel --- .../applicationprofilecache.go | 139 +++++++-- .../applicationprofilecache_test.go | 289 +++++++++++++++++- 2 files changed, 388 insertions(+), 40 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index 14f0768a..dde45127 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -14,6 +14,7 @@ import ( helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers" "github.com/kubescape/k8s-interface/workloadinterface" "github.com/kubescape/node-agent/pkg/objectcache" + "github.com/kubescape/node-agent/pkg/utils" "github.com/kubescape/node-agent/pkg/watcher" "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" versioned "github.com/kubescape/storage/pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1" @@ -59,21 +60,40 @@ type ApplicationProfileCacheImpl struct { pendingMergeProfiles maps.SafeMap[string, *v1beta1.ApplicationProfile] mergeWaitGroup sync.WaitGroup mergeTimeout time.Duration + testMode bool } -func NewApplicationProfileCache(nodeName string, storageClient versioned.SpdxV1beta1Interface, maxDelaySeconds int) *ApplicationProfileCacheImpl { - return &ApplicationProfileCacheImpl{ +type CacheOption func(*ApplicationProfileCacheImpl) + +// Option to enable test mode +func WithTestMode() CacheOption { + return func(a *ApplicationProfileCacheImpl) { + a.testMode = true + } +} + +func NewApplicationProfileCache(nodeName string, storageClient versioned.SpdxV1beta1Interface, maxDelaySeconds int, opts ...CacheOption) *ApplicationProfileCacheImpl { + cache := &ApplicationProfileCacheImpl{ nodeName: nodeName, maxDelaySeconds: maxDelaySeconds, storageClient: storageClient, containerToSlug: maps.SafeMap[string, string]{}, + slugToAppProfile: maps.SafeMap[string, *v1beta1.ApplicationProfile]{}, slugToContainers: maps.SafeMap[string, mapset.Set[string]]{}, + slugToState: maps.SafeMap[string, applicationProfileState]{}, allProfiles: mapset.NewSet[string](), userManagedProfiles: maps.SafeMap[string, *v1beta1.ApplicationProfile]{}, pendingMergeProfiles: maps.SafeMap[string, *v1beta1.ApplicationProfile]{}, - mergeTimeout: time.Duration(maxDelaySeconds) * time.Second, + mergeTimeout: time.Minute * 1, + testMode: false, } + // Apply options + for _, opt := range opts { + opt(cache) + } + + return cache } // ------------------ objectcache.ApplicationProfileCache methods ----------------------- @@ -224,7 +244,7 @@ func (ap *ApplicationProfileCacheImpl) addApplicationProfile(ctx context.Context appProfile := obj.(*v1beta1.ApplicationProfile) apName := objectcache.MetaUniqueName(appProfile) - isUserManaged := appProfile.Annotations["kubescape.io/managed-by"] == "User" + isUserManaged := appProfile.Annotations != nil && appProfile.Annotations["kubescape.io/managed-by"] == "User" if isUserManaged { ap.handleUserManagedProfile(ctx, appProfile, apName) @@ -233,17 +253,6 @@ func (ap *ApplicationProfileCacheImpl) addApplicationProfile(ctx context.Context } } -func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(ctx context.Context, appProfile *v1beta1.ApplicationProfile, apName string) { - ap.userManagedProfiles.Set(apName, appProfile) - - if normalProfile := ap.slugToAppProfile.Get(apName); normalProfile != nil { - ap.mergeProfiles(normalProfile, appProfile) - } else { - ap.pendingMergeProfiles.Set(apName, appProfile) - ap.waitForNormalProfile(ctx, apName) - } -} - func (ap *ApplicationProfileCacheImpl) handleNormalProfile(appProfile *v1beta1.ApplicationProfile, apName string) { apState := newApplicationProfileState(appProfile) ap.slugToState.Set(apName, apState) @@ -259,12 +268,91 @@ func (ap *ApplicationProfileCacheImpl) handleNormalProfile(appProfile *v1beta1.A ap.allProfiles.Add(apName) if userManagedProfile := ap.userManagedProfiles.Get(apName); userManagedProfile != nil { - ap.mergeProfiles(appProfile, userManagedProfile) + if ap.testMode { + fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + ap.mergeProfiles(fullProfile, userManagedProfile) + } else { + time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { + fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + ap.mergeProfiles(fullProfile, userManagedProfile) + }) + } } else if pendingProfile := ap.pendingMergeProfiles.Get(apName); pendingProfile != nil { - ap.mergeProfiles(appProfile, pendingProfile) - ap.pendingMergeProfiles.Delete(apName) + if ap.testMode { + fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + ap.mergeProfiles(fullProfile, pendingProfile) + ap.pendingMergeProfiles.Delete(apName) + } else { + time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { + fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + ap.mergeProfiles(fullProfile, pendingProfile) + ap.pendingMergeProfiles.Delete(apName) + }) + } + } else { + if ap.slugToContainers.Has(apName) { + if ap.testMode { + // Sync mode execution for testing + fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + ap.slugToAppProfile.Set(apName, fullProfile) + for _, containerID := range ap.slugToContainers.Get(apName).ToSlice() { + ap.containerToSlug.Set(containerID, apName) + } + } else { + // Async delay to get the full object from the storage. + time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { + ap.addFullApplicationProfile(appProfile, apName) + }) + } + } + } +} + +func (ap *ApplicationProfileCacheImpl) addFullApplicationProfile(appProfile *v1beta1.ApplicationProfile, apName string) { + fullAP, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + + if ap.slugToContainers.Has(apName) { + ap.slugToAppProfile.Set(apName, fullAP) + for _, containerID := range ap.slugToContainers.Get(apName).ToSlice() { + ap.containerToSlug.Set(containerID, apName) + } + logger.L().Debug("added pod to application profile cache", helpers.String("name", apName)) + } +} + +func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(ctx context.Context, appProfile *v1beta1.ApplicationProfile, apName string) { + ap.userManagedProfiles.Set(apName, appProfile) + + if ap.slugToAppProfile.Has(apName) { + normalProfile := ap.slugToAppProfile.Get(apName) + ap.mergeProfiles(normalProfile, appProfile) } else { - ap.addFullApplicationProfile(appProfile, apName) + ap.pendingMergeProfiles.Set(apName, appProfile) + ap.waitForNormalProfile(ctx, apName) } } @@ -351,19 +439,6 @@ func (ap *ApplicationProfileCacheImpl) waitForNormalProfile(ctx context.Context, }() } -func (ap *ApplicationProfileCacheImpl) addFullApplicationProfile(appProfile *v1beta1.ApplicationProfile, apName string) { - fullAP, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) - if err != nil { - logger.L().Error("failed to get full application profile", helpers.Error(err)) - return - } - ap.slugToAppProfile.Set(apName, fullAP) - for _, i := range ap.slugToContainers.Get(apName).ToSlice() { - ap.containerToSlug.Set(i, apName) - } - logger.L().Debug("added pod to application profile cache", helpers.String("name", apName)) -} - func (ap *ApplicationProfileCacheImpl) deleteApplicationProfile(obj runtime.Object) { apName := objectcache.MetaUniqueName(obj.(metav1.Object)) ap.slugToAppProfile.Delete(apName) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go index eab2c8f4..bd8e4c2a 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go @@ -81,7 +81,7 @@ func Test_AddHandlers(t *testing.T) { t.Run(tt.name, func(t *testing.T) { tt.obj.(metav1.Object).SetNamespace("default") storageClient := fake.NewSimpleClientset().SpdxV1beta1() - ap := NewApplicationProfileCache("", storageClient, 0) + ap := NewApplicationProfileCache("", storageClient, 0, WithTestMode()) ap.slugToContainers.Set(tt.slug, mapset.NewSet[string]()) tt.f(ap, context.Background(), tt.obj) @@ -180,7 +180,7 @@ func Test_addApplicationProfile(t *testing.T) { storageClient := fake.NewSimpleClientset(runtimeObjs...).SpdxV1beta1() - ap := NewApplicationProfileCache("", storageClient, 0) + ap := NewApplicationProfileCache("", storageClient, 0, WithTestMode()) for i := range tt.preCreatedPods { ap.addPod(tt.preCreatedPods[i]) @@ -255,7 +255,7 @@ func Test_deleteApplicationProfile(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ap := NewApplicationProfileCache("", nil, 0) + ap := NewApplicationProfileCache("", nil, 0, WithTestMode()) ap.allProfiles.Append(tt.slugs...) for _, i := range tt.slugs { @@ -318,7 +318,7 @@ func Test_deletePod(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ap := NewApplicationProfileCache("", nil, 0) + ap := NewApplicationProfileCache("", nil, 0, WithTestMode()) for _, i := range tt.otherSlugs { ap.slugToContainers.Set(i, mapset.NewSet[string]()) ap.slugToAppProfile.Set(i, &v1beta1.ApplicationProfile{}) @@ -426,7 +426,7 @@ func Test_GetApplicationProfile(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ap := NewApplicationProfileCache("", fake.NewSimpleClientset().SpdxV1beta1(), 0) + ap := NewApplicationProfileCache("", fake.NewSimpleClientset().SpdxV1beta1(), 0, WithTestMode()) for _, c := range tt.pods { ap.containerToSlug.Set(c.containerID, c.slug) @@ -505,7 +505,7 @@ func Test_addApplicationProfile_existing(t *testing.T) { storageClient := fake.NewSimpleClientset(runtimeObjs...).SpdxV1beta1() - ap := NewApplicationProfileCache("", storageClient, 0) + ap := NewApplicationProfileCache("", storageClient, 0, WithTestMode()) // add pods for i := range tt.pods { @@ -583,7 +583,7 @@ func Test_getApplicationProfile(t *testing.T) { } func Test_WatchResources(t *testing.T) { - ap := NewApplicationProfileCache("test-node", nil, 0) + ap := NewApplicationProfileCache("test-node", nil, 0, WithTestMode()) expectedPodWatchResource := watcher.NewWatchResource(schema.GroupVersionResource{ Group: "", @@ -704,7 +704,7 @@ func Test_addPod(t *testing.T) { storageClient := fake.NewSimpleClientset(runtimeObjs...).SpdxV1beta1() - ap := NewApplicationProfileCache("", storageClient, 0) + ap := NewApplicationProfileCache("", storageClient, 0, WithTestMode()) ap.addApplicationProfile(context.Background(), tt.preCreatedAP) time.Sleep(1 * time.Second) // add is async @@ -734,3 +734,276 @@ func Test_addPod(t *testing.T) { }) } } + +func Test_MergeApplicationProfiles(t *testing.T) { + tests := []struct { + name string + normalProfile *v1beta1.ApplicationProfile + userProfile *v1beta1.ApplicationProfile + expectedMerged *v1beta1.ApplicationProfile + }{ + { + name: "merge profiles with different containers", + normalProfile: &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-profile", + Namespace: "default", + }, + Spec: v1beta1.ApplicationProfileSpec{ + Containers: []v1beta1.ApplicationProfileContainer{ + { + Name: "container1", + Capabilities: []string{ + "NET_ADMIN", + }, + Syscalls: []string{ + "open", + "read", + }, + }, + }, + }, + }, + userProfile: &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-profile", + Namespace: "default", + Annotations: map[string]string{ + "kubescape.io/managed-by": "User", + }, + }, + Spec: v1beta1.ApplicationProfileSpec{ + Containers: []v1beta1.ApplicationProfileContainer{ + { + Name: "container2", + Capabilities: []string{ + "SYS_ADMIN", + }, + Syscalls: []string{ + "write", + }, + }, + }, + }, + }, + expectedMerged: &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-profile", + Namespace: "default", + }, + Spec: v1beta1.ApplicationProfileSpec{ + Containers: []v1beta1.ApplicationProfileContainer{ + { + Name: "container1", + Capabilities: []string{ + "NET_ADMIN", + }, + Syscalls: []string{ + "open", + "read", + }, + }, + { + Name: "container2", + Capabilities: []string{ + "SYS_ADMIN", + }, + Syscalls: []string{ + "write", + }, + }, + }, + }, + }, + }, + { + name: "merge profiles with overlapping containers", + normalProfile: &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-profile", + Namespace: "default", + }, + Spec: v1beta1.ApplicationProfileSpec{ + Containers: []v1beta1.ApplicationProfileContainer{ + { + Name: "container1", + Capabilities: []string{ + "NET_ADMIN", + }, + Syscalls: []string{ + "open", + }, + Opens: []v1beta1.OpenCalls{ + { + Path: "/etc/config", + }, + }, + }, + }, + }, + }, + userProfile: &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-profile", + Namespace: "default", + Annotations: map[string]string{ + "kubescape.io/managed-by": "User", + }, + }, + Spec: v1beta1.ApplicationProfileSpec{ + Containers: []v1beta1.ApplicationProfileContainer{ + { + Name: "container1", + Capabilities: []string{ + "SYS_ADMIN", + }, + Syscalls: []string{ + "write", + }, + Opens: []v1beta1.OpenCalls{ + { + Path: "/etc/secret", + }, + }, + }, + }, + }, + }, + expectedMerged: &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-profile", + Namespace: "default", + }, + Spec: v1beta1.ApplicationProfileSpec{ + Containers: []v1beta1.ApplicationProfileContainer{ + { + Name: "container1", + Capabilities: []string{ + "NET_ADMIN", + "SYS_ADMIN", + }, + Syscalls: []string{ + "open", + "write", + }, + Opens: []v1beta1.OpenCalls{ + { + Path: "/etc/config", + }, + { + Path: "/etc/secret", + }, + }, + }, + }, + }, + }, + }, + { + name: "merge profiles with init containers", + normalProfile: &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-profile", + Namespace: "default", + }, + Spec: v1beta1.ApplicationProfileSpec{ + InitContainers: []v1beta1.ApplicationProfileContainer{ + { + Name: "init1", + Execs: []v1beta1.ExecCalls{ + { + Path: "mount", + }, + }, + }, + }, + }, + }, + userProfile: &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-profile", + Namespace: "default", + Annotations: map[string]string{ + "kubescape.io/managed-by": "User", + }, + }, + Spec: v1beta1.ApplicationProfileSpec{ + InitContainers: []v1beta1.ApplicationProfileContainer{ + { + Name: "init1", + Execs: []v1beta1.ExecCalls{ + { + Path: "chmod", + }, + }, + Syscalls: []string{ + "chmod", + }, + }, + }, + }, + }, + expectedMerged: &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-profile", + Namespace: "default", + }, + Spec: v1beta1.ApplicationProfileSpec{ + InitContainers: []v1beta1.ApplicationProfileContainer{ + { + Name: "init1", + Execs: []v1beta1.ExecCalls{ + { + Path: "mount", + }, + { + Path: "chmod", + }, + }, + Syscalls: []string{ + "chmod", + }, + }, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cache := NewApplicationProfileCache("test-node", nil, 0, WithTestMode()) + merged := cache.performMerge(tt.normalProfile, tt.userProfile) + + // Verify object metadata + assert.Equal(t, tt.expectedMerged.Name, merged.Name) + assert.Equal(t, tt.expectedMerged.Namespace, merged.Namespace) + + // Verify user-managed annotation is removed + _, hasAnnotation := merged.Annotations["kubescape.io/managed-by"] + assert.False(t, hasAnnotation) + + // Verify containers + assert.Equal(t, len(tt.expectedMerged.Spec.Containers), len(merged.Spec.Containers)) + for i, container := range tt.expectedMerged.Spec.Containers { + assert.Equal(t, container.Name, merged.Spec.Containers[i].Name) + assert.ElementsMatch(t, container.Capabilities, merged.Spec.Containers[i].Capabilities) + assert.ElementsMatch(t, container.Syscalls, merged.Spec.Containers[i].Syscalls) + assert.ElementsMatch(t, container.Opens, merged.Spec.Containers[i].Opens) + assert.ElementsMatch(t, container.Execs, merged.Spec.Containers[i].Execs) + assert.ElementsMatch(t, container.Endpoints, merged.Spec.Containers[i].Endpoints) + } + + // Verify init containers + assert.Equal(t, len(tt.expectedMerged.Spec.InitContainers), len(merged.Spec.InitContainers)) + for i, container := range tt.expectedMerged.Spec.InitContainers { + assert.Equal(t, container.Name, merged.Spec.InitContainers[i].Name) + assert.ElementsMatch(t, container.Capabilities, merged.Spec.InitContainers[i].Capabilities) + assert.ElementsMatch(t, container.Syscalls, merged.Spec.InitContainers[i].Syscalls) + assert.ElementsMatch(t, container.Opens, merged.Spec.InitContainers[i].Opens) + assert.ElementsMatch(t, container.Execs, merged.Spec.InitContainers[i].Execs) + assert.ElementsMatch(t, container.Endpoints, merged.Spec.InitContainers[i].Endpoints) + } + }) + } +} From 48d8c106e7335b3e8380324ebbb6e157da8140a2 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 09:27:53 +0000 Subject: [PATCH 03/50] Adding test for application profile merging Signed-off-by: Amit Schendel --- .github/workflows/component-tests.yaml | 1 + tests/component_test.go | 261 ++++++++++--------------- tests/resources/user-profile.yaml | 17 ++ 3 files changed, 124 insertions(+), 155 deletions(-) create mode 100644 tests/resources/user-profile.yaml diff --git a/.github/workflows/component-tests.yaml b/.github/workflows/component-tests.yaml index c9149da1..92639865 100644 --- a/.github/workflows/component-tests.yaml +++ b/.github/workflows/component-tests.yaml @@ -50,6 +50,7 @@ jobs: Test_08_ApplicationProfilePatching, Test_10_MalwareDetectionTest, Test_11_EndpointTest, + Test_12_MergingProfilesTest, # Test_10_DemoTest # Test_11_DuplicationTest ] diff --git a/tests/component_test.go b/tests/component_test.go index bf2cc4f6..1a86fa08 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -720,158 +720,109 @@ func getEndpoint(endpoints []v1beta1.HTTPEndpoint, endpoint v1beta1.HTTPEndpoint } -// func Test_10_DemoTest(t *testing.T) { -// start := time.Now() -// defer tearDownTest(t, start) - -// //testutils.IncreaseNodeAgentSniffingTime("2m") -// wl, err := testutils.NewTestWorkload("default", path.Join(utils.CurrentDir(), "resources/ping-app-role.yaml")) -// if err != nil { -// t.Errorf("Error creating role: %v", err) -// } - -// wl, err = testutils.NewTestWorkload("default", path.Join(utils.CurrentDir(), "resources/ping-app-role-binding.yaml")) -// if err != nil { -// t.Errorf("Error creating role binding: %v", err) -// } - -// wl, err = testutils.NewTestWorkload("default", path.Join(utils.CurrentDir(), "resources/ping-app-service.yaml")) -// if err != nil { -// t.Errorf("Error creating service: %v", err) -// } - -// wl, err = testutils.NewTestWorkload("default", path.Join(utils.CurrentDir(), "resources/ping-app.yaml")) -// if err != nil { -// t.Errorf("Error creating workload: %v", err) -// } -// assert.NoError(t, wl.WaitForReady(80)) -// _, _, err = wl.ExecIntoPod([]string{"sh", "-c", "ping 1.1.1.1 -c 4"}, "") -// err = wl.WaitForApplicationProfileCompletion(80) -// if err != nil { -// t.Errorf("Error waiting for application profile to be completed: %v", err) -// } -// // err = wl.WaitForNetworkNeighborhoodCompletion(80) -// // if err != nil { -// // t.Errorf("Error waiting for network neighborhood to be completed: %v", err) -// // } - -// // Do a ls command using command injection in the ping command -// _, _, err = wl.ExecIntoPod([]string{"sh", "-c", "ping 1.1.1.1 -c 4;ls"}, "ping-app") -// if err != nil { -// t.Errorf("Error executing remote command: %v", err) -// } - -// // Do a cat command using command injection in the ping command -// _, _, err = wl.ExecIntoPod([]string{"sh", "-c", "ping 1.1.1.1 -c 4;cat /run/secrets/kubernetes.io/serviceaccount/token"}, "ping-app") -// if err != nil { -// t.Errorf("Error executing remote command: %v", err) -// } - -// // Do an uname command using command injection in the ping command -// _, _, err = wl.ExecIntoPod([]string{"sh", "-c", "ping 1.1.1.1 -c 4;uname -m | sed 's/x86_64/amd64/g' | sed 's/aarch64/arm64/g'"}, "ping-app") -// if err != nil { -// t.Errorf("Error executing remote command: %v", err) -// } - -// // Download kubectl -// _, _, err = wl.ExecIntoPod([]string{"sh", "-c", "ping 1.1.1.1 -c 4;curl -LO \"https://dl.k8s.io/release/$(curl -L -s https://dl.k8s.io/release/stable.txt)/bin/linux/amd64/kubectl\""}, "ping-app") -// if err != nil { -// t.Errorf("Error executing remote command: %v", err) -// } - -// // Sleep for 10 seconds to wait for the kubectl download -// time.Sleep(10 * time.Second) - -// // Make kubectl executable -// _, _, err = wl.ExecIntoPod([]string{"sh", "-c", "ping 1.1.1.1 -c 4;chmod +x kubectl"}, "ping-app") -// if err != nil { -// t.Errorf("Error executing remote command: %v", err) -// } - -// // Get the pods in the cluster -// output, _, err := wl.ExecIntoPod([]string{"sh", "-c", "ping 1.1.1.1 -c 4;./kubectl --server https://kubernetes.default --insecure-skip-tls-verify --token $(cat /run/secrets/kubernetes.io/serviceaccount/token) get pods"}, "ping-app") -// if err != nil { -// t.Errorf("Error executing remote command: %v", err) -// } - -// // Check that the output contains the pod-ping-app pod -// assert.Contains(t, output, "ping-app", "Expected output to contain 'ping-app'") - -// // Get the alerts and check that the alerts are generated -// alerts, err := testutils.GetAlerts(wl.Namespace) -// if err != nil { -// t.Errorf("Error getting alerts: %v", err) -// } - -// // Validate that all alerts are signaled -// expectedAlerts := map[string]bool{ -// "Unexpected process launched": false, -// "Unexpected file access": false, -// "Kubernetes Client Executed": false, -// // "Exec from malicious source": false, -// "Exec Binary Not In Base Image": false, -// "Unexpected Service Account Token Access": false, -// // "Unexpected domain request": false, -// } - -// for _, alert := range alerts { -// ruleName, ruleOk := alert.Labels["rule_name"] -// if ruleOk { -// if _, exists := expectedAlerts[ruleName]; exists { -// expectedAlerts[ruleName] = true -// } -// } -// } - -// for ruleName, signaled := range expectedAlerts { -// if !signaled { -// t.Errorf("Expected alert '%s' was not signaled", ruleName) -// } -// } -// } - -// func Test_11_DuplicationTest(t *testing.T) { -// start := time.Now() -// defer tearDownTest(t, start) - -// ns := testutils.NewRandomNamespace() -// // wl, err := testutils.NewTestWorkload(ns.Name, path.Join(utils.CurrentDir(), "resources/deployment-multiple-containers.yaml")) -// wl, err := testutils.NewTestWorkload(ns.Name, path.Join(utils.CurrentDir(), "resources/ping-app.yaml")) -// if err != nil { -// t.Errorf("Error creating workload: %v", err) -// } -// assert.NoError(t, wl.WaitForReady(80)) - -// err = wl.WaitForApplicationProfileCompletion(80) -// if err != nil { -// t.Errorf("Error waiting for application profile to be completed: %v", err) -// } - -// // process launched from nginx container -// _, _, err = wl.ExecIntoPod([]string{"ls", "-a"}, "ping-app") -// if err != nil { -// t.Errorf("Error executing remote command: %v", err) -// } - -// time.Sleep(20 * time.Second) - -// alerts, err := testutils.GetAlerts(wl.Namespace) -// if err != nil { -// t.Errorf("Error getting alerts: %v", err) -// } - -// // Validate that unexpected process launched alert is signaled only once -// count := 0 -// for _, alert := range alerts { -// ruleName, ruleOk := alert.Labels["rule_name"] -// if ruleOk { -// if ruleName == "Unexpected process launched" { -// count++ -// } -// } -// } - -// testutils.AssertContains(t, alerts, "Unexpected process launched", "ls", "ping-app") - -// assert.Equal(t, 1, count, "Expected 1 alert of type 'Unexpected process launched' but got %d", count) -// } +func Test_12_MergingProfilesTest(t *testing.T) { + start := time.Now() + defer tearDownTest(t, start) + + ns := testutils.NewRandomNamespace() + wl, err := testutils.NewTestWorkload(ns.Name, path.Join(utils.CurrentDir(), "resources/deployment-multiple-containers.yaml")) + if err != nil { + t.Errorf("Error creating workload: %v", err) + } + assert.NoError(t, wl.WaitForReady(80)) + + assert.NoError(t, wl.WaitForApplicationProfile(80, "ready")) + assert.NoError(t, wl.WaitForNetworkNeighborhood(80, "ready")) + + // process launched from nginx container + _, _, err = wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") + + // network activity from server container + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") + + // network activity from nginx container + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") + + err = wl.WaitForApplicationProfileCompletion(80) + if err != nil { + t.Errorf("Error waiting for application profile to be completed: %v", err) + } + err = wl.WaitForNetworkNeighborhoodCompletion(80) + if err != nil { + t.Errorf("Error waiting for network neighborhood to be completed: %v", err) + } + + time.Sleep(10 * time.Second) + + appProfile, _ := wl.GetApplicationProfile() + appProfileJson, _ := json.Marshal(appProfile) + + t.Logf("application profile: %v", string(appProfileJson)) + + wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") // no alert expected + wl.ExecIntoPod([]string{"ls", "-l"}, "server") // alert expected + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // no alert expected + _, _, err = wl.ExecIntoPod([]string{"curl", "ebpf.io", "-m", "2"}, "nginx") // alert expected + + // Wait for the alert to be signaled + time.Sleep(30 * time.Second) + + alerts, err := testutils.GetAlerts(wl.Namespace) + if err != nil { + t.Errorf("Error getting alerts: %v", err) + } + + testutils.AssertContains(t, alerts, "Unexpected process launched", "ls", "server") + testutils.AssertNotContains(t, alerts, "Unexpected process launched", "ls", "nginx") + + testutils.AssertContains(t, alerts, "Unexpected domain request", "curl", "nginx") + testutils.AssertNotContains(t, alerts, "Unexpected domain request", "wget", "server") + + // check network neighborhood + nn, _ := wl.GetNetworkNeighborhood() + testutils.AssertNetworkNeighborhoodContains(t, nn, "nginx", []string{"kubernetes.io."}, []string{}) + testutils.AssertNetworkNeighborhoodNotContains(t, nn, "server", []string{"kubernetes.io."}, []string{}) + + testutils.AssertNetworkNeighborhoodContains(t, nn, "server", []string{"ebpf.io."}, []string{}) + testutils.AssertNetworkNeighborhoodNotContains(t, nn, "nginx", []string{"ebpf.io."}, []string{}) + + // Apply user profile with the same workload and check if the profiles are merged and that the alerts are no longer generated. + // The user profile will contain the exec calls from the nginx container and the network activity from the server container. + // Which will cause the alerts to not be generated (whitelist). + + // Apply user profile + exitCode := testutils.RunCommand("kubectl", "apply", "-f", path.Join(utils.CurrentDir(), "resources/user-profile.yaml")) + assert.Equal(t, 0, exitCode, "Error applying user profile") + + // Sleep for 5 seconds to allow the profile to be applied + time.Sleep(5 * time.Second) + + // Check if the alerts are no longer generated + // Trigger the same exec calls and network activity + wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") // no alert expected + wl.ExecIntoPod([]string{"ls", "-l"}, "server") // no alert expected + // _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // no alert expected + // _, _, err = wl.ExecIntoPod([]string{"curl", "ebpf.io", "-m", "2"}, "nginx") // no alert expected + + // Wait for the alert to be signaled + time.Sleep(5 * time.Second) + alerts, err = testutils.GetAlerts(wl.Namespace) + if err != nil { + t.Errorf("Error getting alerts: %v", err) + } + + // Check the count of each alert type if it's bigger than 2, it means that the alerts are still being generated + alertsCount := map[string]int{} + for _, alert := range alerts { + ruleName, ruleOk := alert.Labels["rule_name"] + if ruleOk { + alertsCount[ruleName]++ + } + } + + for ruleName, count := range alertsCount { + if count > 2 { + t.Errorf("Expected no alerts to be generated, but got %d alerts of type '%s'", count, ruleName) + } + } +} diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml new file mode 100644 index 00000000..40ae1e0e --- /dev/null +++ b/tests/resources/user-profile.yaml @@ -0,0 +1,17 @@ +apiVersion: spdx.softwarecomposition.kubescape.io/v1beta1 +kind: ApplicationProfile +metadata: + name: multiple-containers-deployment + annotations: + kubescape.io/managed-by: User # This marks it as a user-managed profile +spec: + containers: + - name: nginx + execs: + - ls # Allow ls command for nginx container + - name: server + execs: + - ls # Allow ls command for server container + - /bin/grpc_health_probe # Allow health probe for server container + initContainers: [] + ephemeralContainers: [] \ No newline at end of file From c7dc395a1773a1319134152032db5bf4b513c882 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 09:30:11 +0000 Subject: [PATCH 04/50] Updating test user profile Signed-off-by: Amit Schendel --- tests/resources/user-profile.yaml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml index 40ae1e0e..3df3d0a5 100644 --- a/tests/resources/user-profile.yaml +++ b/tests/resources/user-profile.yaml @@ -8,10 +8,16 @@ spec: containers: - name: nginx execs: - - ls # Allow ls command for nginx container + - path: ls + args: + - "-l" - name: server execs: - - ls # Allow ls command for server container - - /bin/grpc_health_probe # Allow health probe for server container + - path: ls + args: + - "-l" + - path: /bin/grpc_health_probe + args: + - "-addr=:9555" initContainers: [] ephemeralContainers: [] \ No newline at end of file From 893338bff07272aec0992c583084d17ded0759eb Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 10:39:20 +0000 Subject: [PATCH 05/50] Adding updated test Signed-off-by: Amit Schendel --- tests/component_test.go | 36 +++++++++++++++++++++++++------ tests/resources/user-profile.yaml | 2 +- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index 1a86fa08..288fe85a 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -6,9 +6,11 @@ import ( "context" "encoding/json" "fmt" + "os" "path" "slices" "sort" + "strings" "testing" "time" @@ -19,6 +21,7 @@ import ( spdxv1beta1client "github.com/kubescape/storage/pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1" "github.com/kubescape/storage/pkg/registry/file/dynamicpathdetector" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -790,8 +793,29 @@ func Test_12_MergingProfilesTest(t *testing.T) { // The user profile will contain the exec calls from the nginx container and the network activity from the server container. // Which will cause the alerts to not be generated (whitelist). - // Apply user profile - exitCode := testutils.RunCommand("kubectl", "apply", "-f", path.Join(utils.CurrentDir(), "resources/user-profile.yaml")) + // Modify the user profile name to match the existing application profile name + // Read the user profile template + userProfileBytes, err := os.ReadFile(path.Join(utils.CurrentDir(), "resources/user-profile.yaml")) + require.NoError(t, err, "Failed to read user profile template") + + // Replace the {name} placeholder with actual name + userProfileContent := strings.Replace(string(userProfileBytes), "{name}", appProfile.Name, 1) + + // Log the user profile content + t.Logf("Applying user profile:\n%s", userProfileContent) + + // Write to a temporary file + tmpFile, err := os.CreateTemp("", "user-profile-*.yaml") + require.NoError(t, err, "Failed to create temp file") + defer os.Remove(tmpFile.Name()) // Clean up + + _, err = tmpFile.WriteString(userProfileContent) + require.NoError(t, err, "Failed to write to temp file") + err = tmpFile.Close() + require.NoError(t, err, "Failed to close temp file") + + // Apply the modified user profile + exitCode := testutils.RunCommand("kubectl", "apply", "-f", tmpFile.Name()) assert.Equal(t, 0, exitCode, "Error applying user profile") // Sleep for 5 seconds to allow the profile to be applied @@ -805,13 +829,13 @@ func Test_12_MergingProfilesTest(t *testing.T) { // _, _, err = wl.ExecIntoPod([]string{"curl", "ebpf.io", "-m", "2"}, "nginx") // no alert expected // Wait for the alert to be signaled - time.Sleep(5 * time.Second) + time.Sleep(10 * time.Second) alerts, err = testutils.GetAlerts(wl.Namespace) if err != nil { t.Errorf("Error getting alerts: %v", err) } - // Check the count of each alert type if it's bigger than 2, it means that the alerts are still being generated + // Check the count of exec alert type if it's bigger than 2, it means that the alerts are still being generated alertsCount := map[string]int{} for _, alert := range alerts { ruleName, ruleOk := alert.Labels["rule_name"] @@ -821,8 +845,8 @@ func Test_12_MergingProfilesTest(t *testing.T) { } for ruleName, count := range alertsCount { - if count > 2 { - t.Errorf("Expected no alerts to be generated, but got %d alerts of type '%s'", count, ruleName) + if count > 2 && ruleName == "Unexpected process launched" { + t.Errorf("Alert '%s' was signaled %d times, expected 2", ruleName, count) } } } diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml index 3df3d0a5..23155d5b 100644 --- a/tests/resources/user-profile.yaml +++ b/tests/resources/user-profile.yaml @@ -1,7 +1,7 @@ apiVersion: spdx.softwarecomposition.kubescape.io/v1beta1 kind: ApplicationProfile metadata: - name: multiple-containers-deployment + name: {name} annotations: kubescape.io/managed-by: User # This marks it as a user-managed profile spec: From 4458cafabc544576d5999e3672615816bac47fd6 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 11:23:52 +0000 Subject: [PATCH 06/50] Adding fixed test and code Signed-off-by: Amit Schendel --- .../applicationprofilecache.go | 238 ++++++++++-------- .../applicationprofilecache_test.go | 12 +- tests/component_test.go | 114 ++++----- 3 files changed, 190 insertions(+), 174 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index dde45127..21dda9ce 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -3,6 +3,7 @@ package applicationprofilecache import ( "context" "fmt" + "strings" "sync" "time" @@ -19,6 +20,7 @@ import ( "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" versioned "github.com/kubescape/storage/pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -98,8 +100,135 @@ func NewApplicationProfileCache(nodeName string, storageClient versioned.SpdxV1b // ------------------ objectcache.ApplicationProfileCache methods ----------------------- +func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(ctx context.Context, appProfile *v1beta1.ApplicationProfile, apName string) { + // Store the user-managed profile + ap.userManagedProfiles.Set(apName, appProfile) + + // Get the corresponding base profile name + baseProfileName := getBaseProfileName(appProfile.GetName()) + baseProfileUniqueName := objectcache.UniqueName(appProfile.GetNamespace(), baseProfileName) + + // Clear existing cached base profile if it exists + ap.slugToAppProfile.Delete(baseProfileUniqueName) + + // Fetch fresh base profile from cluster + baseProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), baseProfileName) + if err != nil { + if !apierrors.IsNotFound(err) { + logger.L().Error("failed to get base application profile", + helpers.String("name", baseProfileName), + helpers.String("namespace", appProfile.GetNamespace()), + helpers.Error(err)) + } + // Store user-managed profile in pending merge map and wait for base profile + ap.pendingMergeProfiles.Set(baseProfileUniqueName, appProfile) + ap.waitForNormalProfile(ctx, baseProfileUniqueName) + return + } + + // Merge and cache the result + ap.mergeProfiles(baseProfile, appProfile) +} + +func (ap *ApplicationProfileCacheImpl) handleNormalProfile(appProfile *v1beta1.ApplicationProfile, apName string) { + apState := newApplicationProfileState(appProfile) + ap.slugToState.Set(apName, apState) + + if apState.status != helpersv1.Completed { + if ap.slugToAppProfile.Has(apName) { + ap.slugToAppProfile.Delete(apName) + ap.allProfiles.Remove(apName) + } + return + } + + ap.allProfiles.Add(apName) + + // Check for corresponding user-managed profile with ug- prefix + userManagedName := "ug-" + appProfile.GetName() + userManagedUniqueName := objectcache.UniqueName(appProfile.GetNamespace(), userManagedName) + + if userManagedProfile := ap.userManagedProfiles.Get(userManagedUniqueName); userManagedProfile != nil { + if ap.testMode { + fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + ap.mergeProfiles(fullProfile, userManagedProfile) + } else { + time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { + fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + ap.mergeProfiles(fullProfile, userManagedProfile) + }) + } + } else if pendingProfile := ap.pendingMergeProfiles.Get(apName); pendingProfile != nil { + if ap.testMode { + fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + ap.mergeProfiles(fullProfile, pendingProfile) + ap.pendingMergeProfiles.Delete(apName) + } else { + time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { + fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + ap.mergeProfiles(fullProfile, pendingProfile) + ap.pendingMergeProfiles.Delete(apName) + }) + } + } else { + if ap.slugToContainers.Has(apName) { + if ap.testMode { + fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + ap.slugToAppProfile.Set(apName, fullProfile) + for _, containerID := range ap.slugToContainers.Get(apName).ToSlice() { + ap.containerToSlug.Set(containerID, apName) + } + } else { + time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { + ap.addFullApplicationProfile(appProfile, apName) + }) + } + } + } +} + +func (ap *ApplicationProfileCacheImpl) addApplicationProfile(ctx context.Context, obj runtime.Object) { + appProfile := obj.(*v1beta1.ApplicationProfile) + apName := objectcache.MetaUniqueName(appProfile) + + isUserManaged := appProfile.Annotations != nil && + appProfile.Annotations["kubescape.io/managed-by"] == "User" && + isUserManagedByPrefix(appProfile.GetName()) + + if isUserManaged { + ap.handleUserManagedProfile(ctx, appProfile, apName) + } else { + ap.handleNormalProfile(appProfile, apName) + } +} + func (ap *ApplicationProfileCacheImpl) GetApplicationProfile(containerID string) *v1beta1.ApplicationProfile { if s := ap.containerToSlug.Get(containerID); s != "" { + // Check if there's a user-managed version first + userManagedSlug := "ug-" + s + if profile := ap.slugToAppProfile.Get(userManagedSlug); profile != nil { + return profile + } return ap.slugToAppProfile.Get(s) } return nil @@ -240,93 +369,6 @@ func (ap *ApplicationProfileCacheImpl) removeContainer(containerID string) { } // ------------------ watch application profile methods ----------------------- -func (ap *ApplicationProfileCacheImpl) addApplicationProfile(ctx context.Context, obj runtime.Object) { - appProfile := obj.(*v1beta1.ApplicationProfile) - apName := objectcache.MetaUniqueName(appProfile) - - isUserManaged := appProfile.Annotations != nil && appProfile.Annotations["kubescape.io/managed-by"] == "User" - - if isUserManaged { - ap.handleUserManagedProfile(ctx, appProfile, apName) - } else { - ap.handleNormalProfile(appProfile, apName) - } -} - -func (ap *ApplicationProfileCacheImpl) handleNormalProfile(appProfile *v1beta1.ApplicationProfile, apName string) { - apState := newApplicationProfileState(appProfile) - ap.slugToState.Set(apName, apState) - - if apState.status != helpersv1.Completed { - if ap.slugToAppProfile.Has(apName) { - ap.slugToAppProfile.Delete(apName) - ap.allProfiles.Remove(apName) - } - return - } - - ap.allProfiles.Add(apName) - - if userManagedProfile := ap.userManagedProfiles.Get(apName); userManagedProfile != nil { - if ap.testMode { - fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) - if err != nil { - logger.L().Error("failed to get full application profile", helpers.Error(err)) - return - } - ap.mergeProfiles(fullProfile, userManagedProfile) - } else { - time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { - fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) - if err != nil { - logger.L().Error("failed to get full application profile", helpers.Error(err)) - return - } - ap.mergeProfiles(fullProfile, userManagedProfile) - }) - } - } else if pendingProfile := ap.pendingMergeProfiles.Get(apName); pendingProfile != nil { - if ap.testMode { - fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) - if err != nil { - logger.L().Error("failed to get full application profile", helpers.Error(err)) - return - } - ap.mergeProfiles(fullProfile, pendingProfile) - ap.pendingMergeProfiles.Delete(apName) - } else { - time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { - fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) - if err != nil { - logger.L().Error("failed to get full application profile", helpers.Error(err)) - return - } - ap.mergeProfiles(fullProfile, pendingProfile) - ap.pendingMergeProfiles.Delete(apName) - }) - } - } else { - if ap.slugToContainers.Has(apName) { - if ap.testMode { - // Sync mode execution for testing - fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) - if err != nil { - logger.L().Error("failed to get full application profile", helpers.Error(err)) - return - } - ap.slugToAppProfile.Set(apName, fullProfile) - for _, containerID := range ap.slugToContainers.Get(apName).ToSlice() { - ap.containerToSlug.Set(containerID, apName) - } - } else { - // Async delay to get the full object from the storage. - time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { - ap.addFullApplicationProfile(appProfile, apName) - }) - } - } - } -} func (ap *ApplicationProfileCacheImpl) addFullApplicationProfile(appProfile *v1beta1.ApplicationProfile, apName string) { fullAP, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) @@ -344,18 +386,6 @@ func (ap *ApplicationProfileCacheImpl) addFullApplicationProfile(appProfile *v1b } } -func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(ctx context.Context, appProfile *v1beta1.ApplicationProfile, apName string) { - ap.userManagedProfiles.Set(apName, appProfile) - - if ap.slugToAppProfile.Has(apName) { - normalProfile := ap.slugToAppProfile.Get(apName) - ap.mergeProfiles(normalProfile, appProfile) - } else { - ap.pendingMergeProfiles.Set(apName, appProfile) - ap.waitForNormalProfile(ctx, apName) - } -} - func (ap *ApplicationProfileCacheImpl) mergeProfiles(normalProfile, userManagedProfile *v1beta1.ApplicationProfile) { mergedProfile := ap.performMerge(normalProfile, userManagedProfile) ap.slugToAppProfile.Set(objectcache.MetaUniqueName(mergedProfile), mergedProfile) @@ -483,3 +513,13 @@ func getSlug(p *corev1.Pod) (string, error) { } return slug, nil } + +// Helper function to check if a profile name is user-managed by prefix +func isUserManagedByPrefix(name string) bool { + return strings.HasPrefix(name, "ug-") +} + +// Helper function to get base profile name from user-managed profile name +func getBaseProfileName(userManagedName string) string { + return strings.TrimPrefix(userManagedName, "ug-") +} diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go index bd8e4c2a..61bc991c 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go @@ -766,7 +766,7 @@ func Test_MergeApplicationProfiles(t *testing.T) { }, userProfile: &v1beta1.ApplicationProfile{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-profile", + Name: "ug-test-profile", // Added ug- prefix Namespace: "default", Annotations: map[string]string{ "kubescape.io/managed-by": "User", @@ -788,7 +788,7 @@ func Test_MergeApplicationProfiles(t *testing.T) { }, expectedMerged: &v1beta1.ApplicationProfile{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-profile", + Name: "test-profile", // Keeps original name without ug- prefix Namespace: "default", }, Spec: v1beta1.ApplicationProfileSpec{ @@ -844,7 +844,7 @@ func Test_MergeApplicationProfiles(t *testing.T) { }, userProfile: &v1beta1.ApplicationProfile{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-profile", + Name: "ug-test-profile", // Added ug- prefix Namespace: "default", Annotations: map[string]string{ "kubescape.io/managed-by": "User", @@ -871,7 +871,7 @@ func Test_MergeApplicationProfiles(t *testing.T) { }, expectedMerged: &v1beta1.ApplicationProfile{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-profile", + Name: "test-profile", // Keeps original name without ug- prefix Namespace: "default", }, Spec: v1beta1.ApplicationProfileSpec{ @@ -921,7 +921,7 @@ func Test_MergeApplicationProfiles(t *testing.T) { }, userProfile: &v1beta1.ApplicationProfile{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-profile", + Name: "ug-test-profile", // Added ug- prefix Namespace: "default", Annotations: map[string]string{ "kubescape.io/managed-by": "User", @@ -945,7 +945,7 @@ func Test_MergeApplicationProfiles(t *testing.T) { }, expectedMerged: &v1beta1.ApplicationProfile{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-profile", + Name: "test-profile", // Keeps original name without ug- prefix Namespace: "default", }, Spec: v1beta1.ApplicationProfileSpec{ diff --git a/tests/component_test.go b/tests/component_test.go index 288fe85a..3990fd25 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -734,119 +734,95 @@ func Test_12_MergingProfilesTest(t *testing.T) { } assert.NoError(t, wl.WaitForReady(80)) + // Wait for initial profile creation assert.NoError(t, wl.WaitForApplicationProfile(80, "ready")) - assert.NoError(t, wl.WaitForNetworkNeighborhood(80, "ready")) - // process launched from nginx container + // Execute commands to generate initial profile _, _, err = wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") + require.NoError(t, err, "Failed to exec into nginx container") + _, _, err = wl.ExecIntoPod([]string{"ls", "-l"}, "server") + require.NoError(t, err, "Failed to exec into server container") - // network activity from server container - _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") - - // network activity from nginx container - _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") - + // Wait for profile completion err = wl.WaitForApplicationProfileCompletion(80) - if err != nil { - t.Errorf("Error waiting for application profile to be completed: %v", err) - } - err = wl.WaitForNetworkNeighborhoodCompletion(80) - if err != nil { - t.Errorf("Error waiting for network neighborhood to be completed: %v", err) - } + require.NoError(t, err, "Error waiting for application profile completion") + // Give time for the profile to be fully processed time.Sleep(10 * time.Second) - appProfile, _ := wl.GetApplicationProfile() + // Get the initial profile for reference + appProfile, err := wl.GetApplicationProfile() + require.NoError(t, err, "Failed to get application profile") appProfileJson, _ := json.Marshal(appProfile) + t.Logf("Initial application profile: %v", string(appProfileJson)) - t.Logf("application profile: %v", string(appProfileJson)) - - wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") // no alert expected - wl.ExecIntoPod([]string{"ls", "-l"}, "server") // alert expected - _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // no alert expected - _, _, err = wl.ExecIntoPod([]string{"curl", "ebpf.io", "-m", "2"}, "nginx") // alert expected + // Execute test commands that should trigger alerts + wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") // should be allowed (in profile) + wl.ExecIntoPod([]string{"ls", "-l"}, "server") // should trigger alert (not in profile) - // Wait for the alert to be signaled + // Wait for alerts to be generated time.Sleep(30 * time.Second) + // Verify initial alerts alerts, err := testutils.GetAlerts(wl.Namespace) - if err != nil { - t.Errorf("Error getting alerts: %v", err) - } + require.NoError(t, err, "Error getting alerts") testutils.AssertContains(t, alerts, "Unexpected process launched", "ls", "server") testutils.AssertNotContains(t, alerts, "Unexpected process launched", "ls", "nginx") - testutils.AssertContains(t, alerts, "Unexpected domain request", "curl", "nginx") - testutils.AssertNotContains(t, alerts, "Unexpected domain request", "wget", "server") - - // check network neighborhood - nn, _ := wl.GetNetworkNeighborhood() - testutils.AssertNetworkNeighborhoodContains(t, nn, "nginx", []string{"kubernetes.io."}, []string{}) - testutils.AssertNetworkNeighborhoodNotContains(t, nn, "server", []string{"kubernetes.io."}, []string{}) - - testutils.AssertNetworkNeighborhoodContains(t, nn, "server", []string{"ebpf.io."}, []string{}) - testutils.AssertNetworkNeighborhoodNotContains(t, nn, "nginx", []string{"ebpf.io."}, []string{}) - - // Apply user profile with the same workload and check if the profiles are merged and that the alerts are no longer generated. - // The user profile will contain the exec calls from the nginx container and the network activity from the server container. - // Which will cause the alerts to not be generated (whitelist). - - // Modify the user profile name to match the existing application profile name - // Read the user profile template + // Create and apply user-managed profile userProfileBytes, err := os.ReadFile(path.Join(utils.CurrentDir(), "resources/user-profile.yaml")) require.NoError(t, err, "Failed to read user profile template") - // Replace the {name} placeholder with actual name - userProfileContent := strings.Replace(string(userProfileBytes), "{name}", appProfile.Name, 1) - - // Log the user profile content + // Use ug- prefix for user-managed profile + userProfileContent := strings.Replace(string(userProfileBytes), "{name}", fmt.Sprintf("ug-%s", appProfile.Name), 1) t.Logf("Applying user profile:\n%s", userProfileContent) - // Write to a temporary file tmpFile, err := os.CreateTemp("", "user-profile-*.yaml") require.NoError(t, err, "Failed to create temp file") - defer os.Remove(tmpFile.Name()) // Clean up + defer os.Remove(tmpFile.Name()) _, err = tmpFile.WriteString(userProfileContent) require.NoError(t, err, "Failed to write to temp file") err = tmpFile.Close() require.NoError(t, err, "Failed to close temp file") - // Apply the modified user profile + // Apply user profile exitCode := testutils.RunCommand("kubectl", "apply", "-f", tmpFile.Name()) - assert.Equal(t, 0, exitCode, "Error applying user profile") + assert.Equal(t, 0, exitCode, "Failed to apply user profile") - // Sleep for 5 seconds to allow the profile to be applied - time.Sleep(5 * time.Second) + // Wait for profile merge to complete + time.Sleep(10 * time.Second) - // Check if the alerts are no longer generated - // Trigger the same exec calls and network activity - wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") // no alert expected - wl.ExecIntoPod([]string{"ls", "-l"}, "server") // no alert expected - // _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // no alert expected - // _, _, err = wl.ExecIntoPod([]string{"curl", "ebpf.io", "-m", "2"}, "nginx") // no alert expected + // Get merged profile for verification + mergedProfile, err := wl.GetApplicationProfile() + require.NoError(t, err, "Failed to get merged profile") + mergedProfileJson, _ := json.Marshal(mergedProfile) + t.Logf("Merged application profile: %v", string(mergedProfileJson)) - // Wait for the alert to be signaled + // Execute same commands - should not generate new alerts after merge + wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") + wl.ExecIntoPod([]string{"ls", "-l"}, "server") + + // Wait for potential alerts time.Sleep(10 * time.Second) - alerts, err = testutils.GetAlerts(wl.Namespace) - if err != nil { - t.Errorf("Error getting alerts: %v", err) - } - // Check the count of exec alert type if it's bigger than 2, it means that the alerts are still being generated + // Verify no new alerts were generated + newAlerts, err := testutils.GetAlerts(wl.Namespace) + require.NoError(t, err, "Error getting alerts") + + // Count alerts by rule name alertsCount := map[string]int{} - for _, alert := range alerts { - ruleName, ruleOk := alert.Labels["rule_name"] - if ruleOk { + for _, alert := range newAlerts { + if ruleName, ok := alert.Labels["rule_name"]; ok { alertsCount[ruleName]++ } } + // Verify alert counts haven't increased significantly for ruleName, count := range alertsCount { if count > 2 && ruleName == "Unexpected process launched" { - t.Errorf("Alert '%s' was signaled %d times, expected 2", ruleName, count) + t.Errorf("Alert '%s' was signaled %d times after merge, expected ≤ 2", ruleName, count) } } } From 08d7fb311604f095141c3403b0718ea8698e25d6 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 12:21:33 +0000 Subject: [PATCH 07/50] Simplified the logic Signed-off-by: Amit Schendel --- .../applicationprofilecache.go | 315 +++++++----------- .../applicationprofilecache_test.go | 28 +- 2 files changed, 130 insertions(+), 213 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index 21dda9ce..221e70d4 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "strings" - "sync" "time" mapset "github.com/deckarep/golang-set/v2" @@ -20,7 +19,6 @@ import ( "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" versioned "github.com/kubescape/storage/pkg/generated/clientset/versioned/typed/softwarecomposition/v1beta1" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -50,49 +48,28 @@ func newApplicationProfileState(ap *v1beta1.ApplicationProfile) applicationProfi } type ApplicationProfileCacheImpl struct { - containerToSlug maps.SafeMap[string, string] // cache the containerID to slug mapping, this will enable a quick lookup of the application profile - slugToAppProfile maps.SafeMap[string, *v1beta1.ApplicationProfile] // cache the application profile - slugToContainers maps.SafeMap[string, mapset.Set[string]] // cache the containerIDs that belong to the application profile, this will enable removing from cache AP without pods - slugToState maps.SafeMap[string, applicationProfileState] // cache the containerID to slug mapping, this will enable a quick lookup of the application profile - storageClient versioned.SpdxV1beta1Interface - allProfiles mapset.Set[string] // cache all the application profiles that are ready. this will enable removing from cache AP without pods that are running on the same node - nodeName string - maxDelaySeconds int // maximum delay in seconds before getting the full object from the storage - userManagedProfiles maps.SafeMap[string, *v1beta1.ApplicationProfile] - pendingMergeProfiles maps.SafeMap[string, *v1beta1.ApplicationProfile] - mergeWaitGroup sync.WaitGroup - mergeTimeout time.Duration - testMode bool + containerToSlug maps.SafeMap[string, string] // cache the containerID to slug mapping, this will enable a quick lookup of the application profile + slugToAppProfile maps.SafeMap[string, *v1beta1.ApplicationProfile] // cache the application profile + slugToContainers maps.SafeMap[string, mapset.Set[string]] // cache the containerIDs that belong to the application profile, this will enable removing from cache AP without pods + slugToState maps.SafeMap[string, applicationProfileState] // cache the containerID to slug mapping, this will enable a quick lookup of the application profile + storageClient versioned.SpdxV1beta1Interface + allProfiles mapset.Set[string] // cache all the application profiles that are ready. this will enable removing from cache AP without pods that are running on the same node + nodeName string + maxDelaySeconds int // maximum delay in seconds before getting the full object from the storage + userManagedProfiles maps.SafeMap[string, *v1beta1.ApplicationProfile] } -type CacheOption func(*ApplicationProfileCacheImpl) - -// Option to enable test mode -func WithTestMode() CacheOption { - return func(a *ApplicationProfileCacheImpl) { - a.testMode = true - } -} - -func NewApplicationProfileCache(nodeName string, storageClient versioned.SpdxV1beta1Interface, maxDelaySeconds int, opts ...CacheOption) *ApplicationProfileCacheImpl { +func NewApplicationProfileCache(nodeName string, storageClient versioned.SpdxV1beta1Interface, maxDelaySeconds int) *ApplicationProfileCacheImpl { cache := &ApplicationProfileCacheImpl{ - nodeName: nodeName, - maxDelaySeconds: maxDelaySeconds, - storageClient: storageClient, - containerToSlug: maps.SafeMap[string, string]{}, - slugToAppProfile: maps.SafeMap[string, *v1beta1.ApplicationProfile]{}, - slugToContainers: maps.SafeMap[string, mapset.Set[string]]{}, - slugToState: maps.SafeMap[string, applicationProfileState]{}, - allProfiles: mapset.NewSet[string](), - userManagedProfiles: maps.SafeMap[string, *v1beta1.ApplicationProfile]{}, - pendingMergeProfiles: maps.SafeMap[string, *v1beta1.ApplicationProfile]{}, - mergeTimeout: time.Minute * 1, - testMode: false, - } - - // Apply options - for _, opt := range opts { - opt(cache) + nodeName: nodeName, + maxDelaySeconds: maxDelaySeconds, + storageClient: storageClient, + containerToSlug: maps.SafeMap[string, string]{}, + slugToAppProfile: maps.SafeMap[string, *v1beta1.ApplicationProfile]{}, + slugToContainers: maps.SafeMap[string, mapset.Set[string]]{}, + slugToState: maps.SafeMap[string, applicationProfileState]{}, + allProfiles: mapset.NewSet[string](), + userManagedProfiles: maps.SafeMap[string, *v1beta1.ApplicationProfile]{}, } return cache @@ -100,37 +77,53 @@ func NewApplicationProfileCache(nodeName string, storageClient versioned.SpdxV1b // ------------------ objectcache.ApplicationProfileCache methods ----------------------- -func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(ctx context.Context, appProfile *v1beta1.ApplicationProfile, apName string) { - // Store the user-managed profile - ap.userManagedProfiles.Set(apName, appProfile) - - // Get the corresponding base profile name - baseProfileName := getBaseProfileName(appProfile.GetName()) +func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1beta1.ApplicationProfile) { + baseProfileName := strings.TrimPrefix(appProfile.GetName(), "ug-") baseProfileUniqueName := objectcache.UniqueName(appProfile.GetNamespace(), baseProfileName) - // Clear existing cached base profile if it exists - ap.slugToAppProfile.Delete(baseProfileUniqueName) + // Store the user-managed profile temporarily + ap.userManagedProfiles.Set(baseProfileUniqueName, appProfile) - // Fetch fresh base profile from cluster - baseProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), baseProfileName) - if err != nil { - if !apierrors.IsNotFound(err) { - logger.L().Error("failed to get base application profile", + // If we have the base profile cached, fetch a fresh copy and merge + if ap.slugToAppProfile.Has(baseProfileUniqueName) { + // Fetch fresh base profile from cluster + freshBaseProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), baseProfileName) + if err != nil { + logger.L().Error("failed to get fresh base profile for merging", helpers.String("name", baseProfileName), helpers.String("namespace", appProfile.GetNamespace()), helpers.Error(err)) + return } - // Store user-managed profile in pending merge map and wait for base profile - ap.pendingMergeProfiles.Set(baseProfileUniqueName, appProfile) - ap.waitForNormalProfile(ctx, baseProfileUniqueName) - return + + mergedProfile := ap.performMerge(freshBaseProfile, appProfile) + ap.slugToAppProfile.Set(baseProfileUniqueName, mergedProfile) + + // Clean up the user-managed profile after successful merge + ap.userManagedProfiles.Delete(baseProfileUniqueName) + + logger.L().Debug("merged user-managed profile with fresh base profile", + helpers.String("name", baseProfileName), + helpers.String("namespace", appProfile.GetNamespace())) } - // Merge and cache the result - ap.mergeProfiles(baseProfile, appProfile) + // If the base profile is not cached yet, the merge will be attempted when it's added. } -func (ap *ApplicationProfileCacheImpl) handleNormalProfile(appProfile *v1beta1.ApplicationProfile, apName string) { +func (ap *ApplicationProfileCacheImpl) addApplicationProfile(obj runtime.Object) { + appProfile := obj.(*v1beta1.ApplicationProfile) + apName := objectcache.MetaUniqueName(appProfile) + + isUserManaged := appProfile.Annotations != nil && + appProfile.Annotations["kubescape.io/managed-by"] == "User" && + strings.HasPrefix(appProfile.GetName(), "ug-") + + if isUserManaged { + ap.handleUserManagedProfile(appProfile) + return + } + + // Original behavior for normal profiles apState := newApplicationProfileState(appProfile) ap.slugToState.Set(apName, apState) @@ -144,81 +137,10 @@ func (ap *ApplicationProfileCacheImpl) handleNormalProfile(appProfile *v1beta1.A ap.allProfiles.Add(apName) - // Check for corresponding user-managed profile with ug- prefix - userManagedName := "ug-" + appProfile.GetName() - userManagedUniqueName := objectcache.UniqueName(appProfile.GetNamespace(), userManagedName) - - if userManagedProfile := ap.userManagedProfiles.Get(userManagedUniqueName); userManagedProfile != nil { - if ap.testMode { - fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) - if err != nil { - logger.L().Error("failed to get full application profile", helpers.Error(err)) - return - } - ap.mergeProfiles(fullProfile, userManagedProfile) - } else { - time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { - fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) - if err != nil { - logger.L().Error("failed to get full application profile", helpers.Error(err)) - return - } - ap.mergeProfiles(fullProfile, userManagedProfile) - }) - } - } else if pendingProfile := ap.pendingMergeProfiles.Get(apName); pendingProfile != nil { - if ap.testMode { - fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) - if err != nil { - logger.L().Error("failed to get full application profile", helpers.Error(err)) - return - } - ap.mergeProfiles(fullProfile, pendingProfile) - ap.pendingMergeProfiles.Delete(apName) - } else { - time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { - fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) - if err != nil { - logger.L().Error("failed to get full application profile", helpers.Error(err)) - return - } - ap.mergeProfiles(fullProfile, pendingProfile) - ap.pendingMergeProfiles.Delete(apName) - }) - } - } else { - if ap.slugToContainers.Has(apName) { - if ap.testMode { - fullProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) - if err != nil { - logger.L().Error("failed to get full application profile", helpers.Error(err)) - return - } - ap.slugToAppProfile.Set(apName, fullProfile) - for _, containerID := range ap.slugToContainers.Get(apName).ToSlice() { - ap.containerToSlug.Set(containerID, apName) - } - } else { - time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { - ap.addFullApplicationProfile(appProfile, apName) - }) - } - } - } -} - -func (ap *ApplicationProfileCacheImpl) addApplicationProfile(ctx context.Context, obj runtime.Object) { - appProfile := obj.(*v1beta1.ApplicationProfile) - apName := objectcache.MetaUniqueName(appProfile) - - isUserManaged := appProfile.Annotations != nil && - appProfile.Annotations["kubescape.io/managed-by"] == "User" && - isUserManagedByPrefix(appProfile.GetName()) - - if isUserManaged { - ap.handleUserManagedProfile(ctx, appProfile, apName) - } else { - ap.handleNormalProfile(appProfile, apName) + if ap.slugToContainers.Has(apName) { + time.AfterFunc(utils.RandomDuration(ap.maxDelaySeconds, time.Second), func() { + ap.addFullApplicationProfile(appProfile, apName) + }) } } @@ -266,7 +188,7 @@ func (ap *ApplicationProfileCacheImpl) AddHandler(ctx context.Context, obj runti if pod, ok := obj.(*corev1.Pod); ok { ap.addPod(pod) } else if appProfile, ok := obj.(*v1beta1.ApplicationProfile); ok { - ap.addApplicationProfile(ctx, appProfile) + ap.addApplicationProfile(appProfile) } } @@ -274,7 +196,7 @@ func (ap *ApplicationProfileCacheImpl) ModifyHandler(ctx context.Context, obj ru if pod, ok := obj.(*corev1.Pod); ok { ap.addPod(pod) } else if appProfile, ok := obj.(*v1beta1.ApplicationProfile); ok { - ap.addApplicationProfile(ctx, appProfile) + ap.addApplicationProfile(appProfile) } } @@ -377,28 +299,20 @@ func (ap *ApplicationProfileCacheImpl) addFullApplicationProfile(appProfile *v1b return } - if ap.slugToContainers.Has(apName) { - ap.slugToAppProfile.Set(apName, fullAP) - for _, containerID := range ap.slugToContainers.Get(apName).ToSlice() { - ap.containerToSlug.Set(containerID, apName) - } - logger.L().Debug("added pod to application profile cache", helpers.String("name", apName)) + // Check if there's a pending user-managed profile to merge + if ap.userManagedProfiles.Has(apName) { + userManagedProfile := ap.userManagedProfiles.Get(apName) + fullAP = ap.performMerge(fullAP, userManagedProfile) + // Clean up the user-managed profile after successful merge + ap.userManagedProfiles.Delete(apName) + logger.L().Debug("merged pending user-managed profile", helpers.String("name", apName)) } -} - -func (ap *ApplicationProfileCacheImpl) mergeProfiles(normalProfile, userManagedProfile *v1beta1.ApplicationProfile) { - mergedProfile := ap.performMerge(normalProfile, userManagedProfile) - ap.slugToAppProfile.Set(objectcache.MetaUniqueName(mergedProfile), mergedProfile) - if ap.slugToContainers.Has(objectcache.MetaUniqueName(mergedProfile)) { - for _, containerID := range ap.slugToContainers.Get(objectcache.MetaUniqueName(mergedProfile)).ToSlice() { - ap.containerToSlug.Set(containerID, objectcache.MetaUniqueName(mergedProfile)) - } + ap.slugToAppProfile.Set(apName, fullAP) + for _, i := range ap.slugToContainers.Get(apName).ToSlice() { + ap.containerToSlug.Set(i, apName) } - - logger.L().Debug("Merged user-managed profile with normal profile", - helpers.String("name", mergedProfile.GetName()), - helpers.String("namespace", mergedProfile.GetNamespace())) + logger.L().Debug("added pod to application profile cache", helpers.String("name", apName)) } func (ap *ApplicationProfileCacheImpl) performMerge(normalProfile, userManagedProfile *v1beta1.ApplicationProfile) *v1beta1.ApplicationProfile { @@ -441,41 +355,36 @@ func (ap *ApplicationProfileCacheImpl) mergeContainer(normalContainer, userConta normalContainer.Endpoints = append(normalContainer.Endpoints, userContainer.Endpoints...) } -func (ap *ApplicationProfileCacheImpl) waitForNormalProfile(ctx context.Context, apName string) { - ap.mergeWaitGroup.Add(1) - go func() { - defer ap.mergeWaitGroup.Done() - timer := time.NewTimer(ap.mergeTimeout) - defer timer.Stop() - - for { - select { - case <-ctx.Done(): - return - case <-timer.C: - logger.L().Warning("Timeout waiting for normal profile", helpers.String("name", apName)) - ap.pendingMergeProfiles.Delete(apName) - return - default: - if normalProfile := ap.slugToAppProfile.Get(apName); normalProfile != nil { - userManagedProfile := ap.pendingMergeProfiles.Get(apName) - ap.mergeProfiles(normalProfile, userManagedProfile) - ap.pendingMergeProfiles.Delete(apName) - return - } - time.Sleep(100 * time.Millisecond) - } - } - }() -} - func (ap *ApplicationProfileCacheImpl) deleteApplicationProfile(obj runtime.Object) { - apName := objectcache.MetaUniqueName(obj.(metav1.Object)) - ap.slugToAppProfile.Delete(apName) - ap.slugToState.Delete(apName) - ap.allProfiles.Remove(apName) + appProfile := obj.(*v1beta1.ApplicationProfile) + apName := objectcache.MetaUniqueName(appProfile) + + isUserManaged := appProfile.Annotations != nil && + appProfile.Annotations["kubescape.io/managed-by"] == "User" && + strings.HasPrefix(appProfile.GetName(), "ug-") - logger.L().Info("deleted application profile from cache", helpers.String("uniqueSlug", apName)) + if isUserManaged { + // For user-managed profiles, we need to use the base name for cleanup + baseProfileName := strings.TrimPrefix(appProfile.GetName(), "ug-") + baseProfileUniqueName := objectcache.UniqueName(appProfile.GetNamespace(), baseProfileName) + ap.userManagedProfiles.Delete(baseProfileUniqueName) + + logger.L().Debug("deleted user-managed profile from cache", + helpers.String("profileName", appProfile.GetName()), + helpers.String("baseProfile", baseProfileName)) + } else { + // For normal profiles, clean up all related data + ap.slugToAppProfile.Delete(apName) + ap.slugToState.Delete(apName) + ap.allProfiles.Remove(apName) + + // Log the deletion of normal profile + logger.L().Debug("deleted application profile from cache", + helpers.String("uniqueSlug", apName)) + } + + // Clean up any orphaned user-managed profiles + ap.cleanupOrphanedUserManagedProfiles() } func (ap *ApplicationProfileCacheImpl) getApplicationProfile(namespace, name string) (*v1beta1.ApplicationProfile, error) { @@ -514,12 +423,20 @@ func getSlug(p *corev1.Pod) (string, error) { return slug, nil } -// Helper function to check if a profile name is user-managed by prefix -func isUserManagedByPrefix(name string) bool { - return strings.HasPrefix(name, "ug-") -} - -// Helper function to get base profile name from user-managed profile name -func getBaseProfileName(userManagedName string) string { - return strings.TrimPrefix(userManagedName, "ug-") +// Add cleanup method for any orphaned user-managed profiles +func (ap *ApplicationProfileCacheImpl) cleanupOrphanedUserManagedProfiles() { + // This could be called periodically or during certain operations + ap.userManagedProfiles.Range(func(key string, value *v1beta1.ApplicationProfile) bool { + if ap.slugToAppProfile.Has(key) { + // If base profile exists but merge didn't happen for some reason, + // attempt merge again and cleanup + if baseProfile := ap.slugToAppProfile.Get(key); baseProfile != nil { + mergedProfile := ap.performMerge(baseProfile, value) + ap.slugToAppProfile.Set(key, mergedProfile) + ap.userManagedProfiles.Delete(key) + logger.L().Debug("cleaned up orphaned user-managed profile", helpers.String("name", key)) + } + } + return true + }) } diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go index 61bc991c..f8806142 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go @@ -81,7 +81,7 @@ func Test_AddHandlers(t *testing.T) { t.Run(tt.name, func(t *testing.T) { tt.obj.(metav1.Object).SetNamespace("default") storageClient := fake.NewSimpleClientset().SpdxV1beta1() - ap := NewApplicationProfileCache("", storageClient, 0, WithTestMode()) + ap := NewApplicationProfileCache("", storageClient, 0) ap.slugToContainers.Set(tt.slug, mapset.NewSet[string]()) tt.f(ap, context.Background(), tt.obj) @@ -180,16 +180,16 @@ func Test_addApplicationProfile(t *testing.T) { storageClient := fake.NewSimpleClientset(runtimeObjs...).SpdxV1beta1() - ap := NewApplicationProfileCache("", storageClient, 0, WithTestMode()) + ap := NewApplicationProfileCache("", storageClient, 0) for i := range tt.preCreatedPods { ap.addPod(tt.preCreatedPods[i]) } for i := range tt.preCreatedAP { - ap.addApplicationProfile(context.Background(), tt.preCreatedAP[i]) + ap.addApplicationProfile(tt.preCreatedAP[i]) } - ap.addApplicationProfile(context.Background(), tt.obj) + ap.addApplicationProfile(tt.obj) time.Sleep(1 * time.Second) // add is async // test if the application profile is added to the cache @@ -255,7 +255,7 @@ func Test_deleteApplicationProfile(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ap := NewApplicationProfileCache("", nil, 0, WithTestMode()) + ap := NewApplicationProfileCache("", nil, 0) ap.allProfiles.Append(tt.slugs...) for _, i := range tt.slugs { @@ -318,7 +318,7 @@ func Test_deletePod(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ap := NewApplicationProfileCache("", nil, 0, WithTestMode()) + ap := NewApplicationProfileCache("", nil, 0) for _, i := range tt.otherSlugs { ap.slugToContainers.Set(i, mapset.NewSet[string]()) ap.slugToAppProfile.Set(i, &v1beta1.ApplicationProfile{}) @@ -426,7 +426,7 @@ func Test_GetApplicationProfile(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ap := NewApplicationProfileCache("", fake.NewSimpleClientset().SpdxV1beta1(), 0, WithTestMode()) + ap := NewApplicationProfileCache("", fake.NewSimpleClientset().SpdxV1beta1(), 0) for _, c := range tt.pods { ap.containerToSlug.Set(c.containerID, c.slug) @@ -505,7 +505,7 @@ func Test_addApplicationProfile_existing(t *testing.T) { storageClient := fake.NewSimpleClientset(runtimeObjs...).SpdxV1beta1() - ap := NewApplicationProfileCache("", storageClient, 0, WithTestMode()) + ap := NewApplicationProfileCache("", storageClient, 0) // add pods for i := range tt.pods { @@ -513,9 +513,9 @@ func Test_addApplicationProfile_existing(t *testing.T) { ap.slugToContainers.Set(tt.pods[i].slug, mapset.NewSet(tt.pods[i].podName)) } - ap.addApplicationProfile(context.Background(), tt.obj1) + ap.addApplicationProfile(tt.obj1) time.Sleep(1 * time.Second) // add is async - ap.addApplicationProfile(context.Background(), tt.obj2) + ap.addApplicationProfile(tt.obj2) // test if the application profile is added to the cache if tt.storeInCache { @@ -583,7 +583,7 @@ func Test_getApplicationProfile(t *testing.T) { } func Test_WatchResources(t *testing.T) { - ap := NewApplicationProfileCache("test-node", nil, 0, WithTestMode()) + ap := NewApplicationProfileCache("test-node", nil, 0) expectedPodWatchResource := watcher.NewWatchResource(schema.GroupVersionResource{ Group: "", @@ -704,9 +704,9 @@ func Test_addPod(t *testing.T) { storageClient := fake.NewSimpleClientset(runtimeObjs...).SpdxV1beta1() - ap := NewApplicationProfileCache("", storageClient, 0, WithTestMode()) + ap := NewApplicationProfileCache("", storageClient, 0) - ap.addApplicationProfile(context.Background(), tt.preCreatedAP) + ap.addApplicationProfile(tt.preCreatedAP) time.Sleep(1 * time.Second) // add is async tt.obj.(metav1.Object).SetNamespace(namespace) @@ -972,7 +972,7 @@ func Test_MergeApplicationProfiles(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cache := NewApplicationProfileCache("test-node", nil, 0, WithTestMode()) + cache := NewApplicationProfileCache("test-node", nil, 0) merged := cache.performMerge(tt.normalProfile, tt.userProfile) // Verify object metadata From 821806241a7a883c340f234a8988968b4eb2d474 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 12:24:41 +0000 Subject: [PATCH 08/50] Adding a fix for application profile fetching Signed-off-by: Amit Schendel --- .../applicationprofilecache/applicationprofilecache.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index 221e70d4..acfb992a 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -146,13 +146,9 @@ func (ap *ApplicationProfileCacheImpl) addApplicationProfile(obj runtime.Object) func (ap *ApplicationProfileCacheImpl) GetApplicationProfile(containerID string) *v1beta1.ApplicationProfile { if s := ap.containerToSlug.Get(containerID); s != "" { - // Check if there's a user-managed version first - userManagedSlug := "ug-" + s - if profile := ap.slugToAppProfile.Get(userManagedSlug); profile != nil { - return profile - } return ap.slugToAppProfile.Get(s) } + return nil } From 96c60ae0eea08b60616a7c7ca0aded22765fb69e Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 12:28:52 +0000 Subject: [PATCH 09/50] Doing a deep copy to avoid modifications Signed-off-by: Amit Schendel --- .../applicationprofilecache/applicationprofilecache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index acfb992a..a11c6b24 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -82,7 +82,7 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be baseProfileUniqueName := objectcache.UniqueName(appProfile.GetNamespace(), baseProfileName) // Store the user-managed profile temporarily - ap.userManagedProfiles.Set(baseProfileUniqueName, appProfile) + ap.userManagedProfiles.Set(baseProfileUniqueName, appProfile.DeepCopy()) // If we have the base profile cached, fetch a fresh copy and merge if ap.slugToAppProfile.Has(baseProfileUniqueName) { From dfc42f67f613024afb258e3bf1d3cfbe2a699582 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 12:52:44 +0000 Subject: [PATCH 10/50] Adding fixed test Signed-off-by: Amit Schendel --- tests/component_test.go | 115 ++++++++++++++---------------- tests/resources/user-profile.yaml | 1 + 2 files changed, 55 insertions(+), 61 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index 3990fd25..c9d05e7c 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "os" + "os/exec" "path" "slices" "sort" @@ -727,102 +728,94 @@ func Test_12_MergingProfilesTest(t *testing.T) { start := time.Now() defer tearDownTest(t, start) + // PHASE 1: Setup workload and initial profile ns := testutils.NewRandomNamespace() wl, err := testutils.NewTestWorkload(ns.Name, path.Join(utils.CurrentDir(), "resources/deployment-multiple-containers.yaml")) - if err != nil { - t.Errorf("Error creating workload: %v", err) - } - assert.NoError(t, wl.WaitForReady(80)) - - // Wait for initial profile creation - assert.NoError(t, wl.WaitForApplicationProfile(80, "ready")) + require.NoError(t, err, "Failed to create workload") + require.NoError(t, wl.WaitForReady(80), "Workload failed to be ready") + require.NoError(t, wl.WaitForApplicationProfile(80, "ready"), "Application profile not ready") - // Execute commands to generate initial profile + // Generate initial profile data _, _, err = wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") require.NoError(t, err, "Failed to exec into nginx container") _, _, err = wl.ExecIntoPod([]string{"ls", "-l"}, "server") require.NoError(t, err, "Failed to exec into server container") - // Wait for profile completion - err = wl.WaitForApplicationProfileCompletion(80) - require.NoError(t, err, "Error waiting for application profile completion") - - // Give time for the profile to be fully processed - time.Sleep(10 * time.Second) - - // Get the initial profile for reference - appProfile, err := wl.GetApplicationProfile() - require.NoError(t, err, "Failed to get application profile") - appProfileJson, _ := json.Marshal(appProfile) - t.Logf("Initial application profile: %v", string(appProfileJson)) - - // Execute test commands that should trigger alerts - wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") // should be allowed (in profile) - wl.ExecIntoPod([]string{"ls", "-l"}, "server") // should trigger alert (not in profile) + require.NoError(t, wl.WaitForApplicationProfileCompletion(80), "Profile failed to complete") + time.Sleep(10 * time.Second) // Allow profile processing - // Wait for alerts to be generated - time.Sleep(30 * time.Second) + // Log initial profile state + initialProfile, err := wl.GetApplicationProfile() + require.NoError(t, err, "Failed to get initial profile") + initialProfileJSON, _ := json.Marshal(initialProfile) + t.Logf("Initial application profile:\n%s", string(initialProfileJSON)) - // Verify initial alerts - alerts, err := testutils.GetAlerts(wl.Namespace) - require.NoError(t, err, "Error getting alerts") + // PHASE 2: Verify initial alerts + t.Log("Testing initial alert generation...") + wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") // Expected: no alert + wl.ExecIntoPod([]string{"ls", "-l"}, "server") // Expected: alert + time.Sleep(30 * time.Second) // Wait for alert generation - testutils.AssertContains(t, alerts, "Unexpected process launched", "ls", "server") - testutils.AssertNotContains(t, alerts, "Unexpected process launched", "ls", "nginx") + initialAlerts, err := testutils.GetAlerts(wl.Namespace) + require.NoError(t, err, "Failed to get initial alerts") + testutils.AssertContains(t, initialAlerts, "Unexpected process launched", "ls", "server") + testutils.AssertNotContains(t, initialAlerts, "Unexpected process launched", "ls", "nginx") - // Create and apply user-managed profile + // PHASE 3: Apply user-managed profile + t.Log("Applying user-managed profile...") userProfileBytes, err := os.ReadFile(path.Join(utils.CurrentDir(), "resources/user-profile.yaml")) require.NoError(t, err, "Failed to read user profile template") - // Use ug- prefix for user-managed profile - userProfileContent := strings.Replace(string(userProfileBytes), "{name}", fmt.Sprintf("ug-%s", appProfile.Name), 1) - t.Logf("Applying user profile:\n%s", userProfileContent) + // Replace both name and namespace placeholders + userProfileContent := string(userProfileBytes) + userProfileContent = strings.Replace(userProfileContent, "{name}", fmt.Sprintf("ug-%s", initialProfile.Name), 1) + userProfileContent = strings.Replace(userProfileContent, "{namespace}", initialProfile.Namespace, 1) + + t.Logf("User profile content:\n%s", userProfileContent) + // Create and apply the user profile tmpFile, err := os.CreateTemp("", "user-profile-*.yaml") require.NoError(t, err, "Failed to create temp file") defer os.Remove(tmpFile.Name()) - _, err = tmpFile.WriteString(userProfileContent) - require.NoError(t, err, "Failed to write to temp file") - err = tmpFile.Close() - require.NoError(t, err, "Failed to close temp file") + err = os.WriteFile(tmpFile.Name(), []byte(userProfileContent), 0644) + require.NoError(t, err, "Failed to write user profile to file") - // Apply user profile - exitCode := testutils.RunCommand("kubectl", "apply", "-f", tmpFile.Name()) - assert.Equal(t, 0, exitCode, "Failed to apply user profile") + output, err := exec.Command("kubectl", "apply", "-f", tmpFile.Name()).CombinedOutput() + if err != nil { + t.Fatalf("Failed to apply user profile: %v\nOutput: %s", err, string(output)) + } - // Wait for profile merge to complete - time.Sleep(10 * time.Second) + // PHASE 4: Verify merged profile behavior + t.Log("Verifying merged profile behavior...") + time.Sleep(15 * time.Second) // Allow merge to complete - // Get merged profile for verification + // Log merged profile state mergedProfile, err := wl.GetApplicationProfile() require.NoError(t, err, "Failed to get merged profile") - mergedProfileJson, _ := json.Marshal(mergedProfile) - t.Logf("Merged application profile: %v", string(mergedProfileJson)) + mergedProfileJSON, _ := json.Marshal(mergedProfile) + t.Logf("Merged application profile:\n%s", string(mergedProfileJSON)) - // Execute same commands - should not generate new alerts after merge + // Test merged profile behavior wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") wl.ExecIntoPod([]string{"ls", "-l"}, "server") + time.Sleep(10 * time.Second) // Wait for potential alerts - // Wait for potential alerts - time.Sleep(10 * time.Second) - - // Verify no new alerts were generated - newAlerts, err := testutils.GetAlerts(wl.Namespace) - require.NoError(t, err, "Error getting alerts") + // Verify alert counts + finalAlerts, err := testutils.GetAlerts(wl.Namespace) + require.NoError(t, err, "Failed to get final alerts") - // Count alerts by rule name - alertsCount := map[string]int{} - for _, alert := range newAlerts { + alertCounts := make(map[string]int) + for _, alert := range finalAlerts { if ruleName, ok := alert.Labels["rule_name"]; ok { - alertsCount[ruleName]++ + alertCounts[ruleName]++ } } - // Verify alert counts haven't increased significantly - for ruleName, count := range alertsCount { + // Check for unexpected increase in alerts + for ruleName, count := range alertCounts { if count > 2 && ruleName == "Unexpected process launched" { - t.Errorf("Alert '%s' was signaled %d times after merge, expected ≤ 2", ruleName, count) + t.Errorf("Unexpected number of alerts for '%s': got %d, expected ≤ 2", ruleName, count) } } } diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml index 23155d5b..6d6e972d 100644 --- a/tests/resources/user-profile.yaml +++ b/tests/resources/user-profile.yaml @@ -2,6 +2,7 @@ apiVersion: spdx.softwarecomposition.kubescape.io/v1beta1 kind: ApplicationProfile metadata: name: {name} + namespace: {namespace} annotations: kubescape.io/managed-by: User # This marks it as a user-managed profile spec: From 781984374722ca1adadfdc297e52cc1f7c496dc9 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 12:55:21 +0000 Subject: [PATCH 11/50] Adding fixed test Signed-off-by: Amit Schendel --- tests/component_test.go | 22 +++++++++++++++++++--- tests/resources/user-profile.yaml | 25 +++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index c9d05e7c..46e2e9d1 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -766,10 +766,26 @@ func Test_12_MergingProfilesTest(t *testing.T) { userProfileBytes, err := os.ReadFile(path.Join(utils.CurrentDir(), "resources/user-profile.yaml")) require.NoError(t, err, "Failed to read user profile template") - // Replace both name and namespace placeholders + // Extract workload name from initial profile + workloadName := "" + if initialProfile.Labels != nil { + workloadName = initialProfile.Labels["kubescape.io/workload-name"] + } + + // Replace all placeholders in the template + replacements := map[string]string{ + "{name}": fmt.Sprintf("ug-%s", initialProfile.Name), + "{namespace}": initialProfile.Namespace, + "{hash}": initialProfile.Labels["kubescape.io/instance-template-hash"], + "{workload-name}": workloadName, + "{resource-version}": initialProfile.ResourceVersion, + "{wlid}": fmt.Sprintf("wlid://cluster/%s/deployment-%s", initialProfile.Namespace, workloadName), + } + userProfileContent := string(userProfileBytes) - userProfileContent = strings.Replace(userProfileContent, "{name}", fmt.Sprintf("ug-%s", initialProfile.Name), 1) - userProfileContent = strings.Replace(userProfileContent, "{namespace}", initialProfile.Namespace, 1) + for placeholder, value := range replacements { + userProfileContent = strings.Replace(userProfileContent, placeholder, value, 1) + } t.Logf("User profile content:\n%s", userProfileContent) diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml index 6d6e972d..dc1eab15 100644 --- a/tests/resources/user-profile.yaml +++ b/tests/resources/user-profile.yaml @@ -4,15 +4,32 @@ metadata: name: {name} namespace: {namespace} annotations: - kubescape.io/managed-by: User # This marks it as a user-managed profile + kubescape.io/managed-by: User + kubescape.io/resource-size: "1" + kubescape.io/wlid: {wlid} + labels: + kubescape.io/instance-template-hash: {hash} + kubescape.io/workload-api-group: apps + kubescape.io/workload-api-version: v1 + kubescape.io/workload-kind: Deployment + kubescape.io/workload-name: {workload-name} + kubescape.io/workload-namespace: {namespace} + kubescape.io/workload-resource-version: {resource-version} spec: containers: - name: nginx + imageID: "" + imageTag: "" execs: - path: ls args: - "-l" + seccompProfile: + spec: + defaultAction: "" - name: server + imageID: "" + imageTag: "" execs: - path: ls args: @@ -20,5 +37,9 @@ spec: - path: /bin/grpc_health_probe args: - "-addr=:9555" + seccompProfile: + spec: + defaultAction: "" initContainers: [] - ephemeralContainers: [] \ No newline at end of file + ephemeralContainers: [] +status: {} \ No newline at end of file From b1e3dd34f4c681943d4ba6efb48e8bbbbb416d6e Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 13:05:10 +0000 Subject: [PATCH 12/50] Fixing test Signed-off-by: Amit Schendel --- tests/component_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index 46e2e9d1..76c6f154 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -738,8 +738,6 @@ func Test_12_MergingProfilesTest(t *testing.T) { // Generate initial profile data _, _, err = wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") require.NoError(t, err, "Failed to exec into nginx container") - _, _, err = wl.ExecIntoPod([]string{"ls", "-l"}, "server") - require.NoError(t, err, "Failed to exec into server container") require.NoError(t, wl.WaitForApplicationProfileCompletion(80), "Profile failed to complete") time.Sleep(10 * time.Second) // Allow profile processing @@ -830,8 +828,8 @@ func Test_12_MergingProfilesTest(t *testing.T) { // Check for unexpected increase in alerts for ruleName, count := range alertCounts { - if count > 2 && ruleName == "Unexpected process launched" { - t.Errorf("Unexpected number of alerts for '%s': got %d, expected ≤ 2", ruleName, count) + if count > 1 && ruleName == "Unexpected process launched" { + t.Errorf("Unexpected number of alerts for '%s': got %d, expected ≤ 1", ruleName, count) } } } From ff295a3d800aaee1c7cc196ffbb6137dedbe181b Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 13:08:09 +0000 Subject: [PATCH 13/50] Fixing profile Signed-off-by: Amit Schendel --- tests/resources/user-profile.yaml | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml index dc1eab15..b579aa4a 100644 --- a/tests/resources/user-profile.yaml +++ b/tests/resources/user-profile.yaml @@ -9,17 +9,22 @@ metadata: kubescape.io/wlid: {wlid} labels: kubescape.io/instance-template-hash: {hash} - kubescape.io/workload-api-group: apps - kubescape.io/workload-api-version: v1 - kubescape.io/workload-kind: Deployment + kubescape.io/workload-api-group: "apps" + kubescape.io/workload-api-version: "v1" + kubescape.io/workload-kind: "Deployment" kubescape.io/workload-name: {workload-name} - kubescape.io/workload-namespace: {namespace} + kubescape.io/workload-namespace: "{namespace}" kubescape.io/workload-resource-version: {resource-version} spec: + architectures: ["amd64"] containers: - name: nginx imageID: "" imageTag: "" + capabilities: [] + opens: [] + syscalls: [] + endpoints: [] execs: - path: ls args: @@ -30,6 +35,10 @@ spec: - name: server imageID: "" imageTag: "" + capabilities: [] + opens: [] + syscalls: [] + endpoints: [] execs: - path: ls args: From 90ec95cb259c255658e5c8cfacfc43d1e1d81be3 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 13:37:00 +0000 Subject: [PATCH 14/50] Fixed test Signed-off-by: Amit Schendel --- tests/component_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index 76c6f154..5ffffec2 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -764,13 +764,14 @@ func Test_12_MergingProfilesTest(t *testing.T) { userProfileBytes, err := os.ReadFile(path.Join(utils.CurrentDir(), "resources/user-profile.yaml")) require.NoError(t, err, "Failed to read user profile template") - // Extract workload name from initial profile - workloadName := "" - if initialProfile.Labels != nil { - workloadName = initialProfile.Labels["kubescape.io/workload-name"] + // Extract required values from initial profile + workloadName := initialProfile.Labels["kubescape.io/workload-name"] + if workloadName == "" { + workloadName = strings.TrimPrefix(initialProfile.Name, "replicaset-") + workloadName = strings.TrimSuffix(workloadName, "-"+initialProfile.Labels["kubescape.io/instance-template-hash"]) } - // Replace all placeholders in the template + // Create replacements map with properly quoted values replacements := map[string]string{ "{name}": fmt.Sprintf("ug-%s", initialProfile.Name), "{namespace}": initialProfile.Namespace, From c3c078109c59ae633f28358d8b95d2fe536d58da Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 13:50:26 +0000 Subject: [PATCH 15/50] Fixing test Signed-off-by: Amit Schendel --- tests/component_test.go | 17 ++++------------- tests/resources/user-profile.yaml | 10 ---------- 2 files changed, 4 insertions(+), 23 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index 5ffffec2..f9b8524e 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -738,6 +738,8 @@ func Test_12_MergingProfilesTest(t *testing.T) { // Generate initial profile data _, _, err = wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") require.NoError(t, err, "Failed to exec into nginx container") + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") + require.NoError(t, err, "Failed to exec into server container") require.NoError(t, wl.WaitForApplicationProfileCompletion(80), "Profile failed to complete") time.Sleep(10 * time.Second) // Allow profile processing @@ -764,21 +766,10 @@ func Test_12_MergingProfilesTest(t *testing.T) { userProfileBytes, err := os.ReadFile(path.Join(utils.CurrentDir(), "resources/user-profile.yaml")) require.NoError(t, err, "Failed to read user profile template") - // Extract required values from initial profile - workloadName := initialProfile.Labels["kubescape.io/workload-name"] - if workloadName == "" { - workloadName = strings.TrimPrefix(initialProfile.Name, "replicaset-") - workloadName = strings.TrimSuffix(workloadName, "-"+initialProfile.Labels["kubescape.io/instance-template-hash"]) - } - // Create replacements map with properly quoted values replacements := map[string]string{ - "{name}": fmt.Sprintf("ug-%s", initialProfile.Name), - "{namespace}": initialProfile.Namespace, - "{hash}": initialProfile.Labels["kubescape.io/instance-template-hash"], - "{workload-name}": workloadName, - "{resource-version}": initialProfile.ResourceVersion, - "{wlid}": fmt.Sprintf("wlid://cluster/%s/deployment-%s", initialProfile.Namespace, workloadName), + "{name}": fmt.Sprintf("ug-%s", initialProfile.Name), + "{namespace}": initialProfile.Namespace, } userProfileContent := string(userProfileBytes) diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml index b579aa4a..997931f7 100644 --- a/tests/resources/user-profile.yaml +++ b/tests/resources/user-profile.yaml @@ -5,16 +5,6 @@ metadata: namespace: {namespace} annotations: kubescape.io/managed-by: User - kubescape.io/resource-size: "1" - kubescape.io/wlid: {wlid} - labels: - kubescape.io/instance-template-hash: {hash} - kubescape.io/workload-api-group: "apps" - kubescape.io/workload-api-version: "v1" - kubescape.io/workload-kind: "Deployment" - kubescape.io/workload-name: {workload-name} - kubescape.io/workload-namespace: "{namespace}" - kubescape.io/workload-resource-version: {resource-version} spec: architectures: ["amd64"] containers: From 8b760e12a214080739d36a32a1285555d89bdf4c Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 14:23:57 +0000 Subject: [PATCH 16/50] Adding fixed test Signed-off-by: Amit Schendel --- tests/component_test.go | 14 +++++--------- tests/resources/user-profile.yaml | 2 ++ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index f9b8524e..da0a9f37 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -796,16 +796,10 @@ func Test_12_MergingProfilesTest(t *testing.T) { t.Log("Verifying merged profile behavior...") time.Sleep(15 * time.Second) // Allow merge to complete - // Log merged profile state - mergedProfile, err := wl.GetApplicationProfile() - require.NoError(t, err, "Failed to get merged profile") - mergedProfileJSON, _ := json.Marshal(mergedProfile) - t.Logf("Merged application profile:\n%s", string(mergedProfileJSON)) - // Test merged profile behavior - wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") - wl.ExecIntoPod([]string{"ls", "-l"}, "server") - time.Sleep(10 * time.Second) // Wait for potential alerts + wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") // Expected: no alert + wl.ExecIntoPod([]string{"ls", "-l"}, "server") // Expected: no alert (user profile should suppress alert) + time.Sleep(10 * time.Second) // Wait for potential alerts // Verify alert counts finalAlerts, err := testutils.GetAlerts(wl.Namespace) @@ -821,6 +815,8 @@ func Test_12_MergingProfilesTest(t *testing.T) { // Check for unexpected increase in alerts for ruleName, count := range alertCounts { if count > 1 && ruleName == "Unexpected process launched" { + // Print all alerts for debugging + t.Logf("Final alerts:\n%v", finalAlerts) t.Errorf("Unexpected number of alerts for '%s': got %d, expected ≤ 1", ruleName, count) } } diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml index 997931f7..d61a7843 100644 --- a/tests/resources/user-profile.yaml +++ b/tests/resources/user-profile.yaml @@ -18,6 +18,7 @@ spec: execs: - path: ls args: + - "ls" - "-l" seccompProfile: spec: @@ -32,6 +33,7 @@ spec: execs: - path: ls args: + - "ls" - "-l" - path: /bin/grpc_health_probe args: From 5c9b9fae3a5e251930ee3eadee2de3fdc482844a Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 14:42:26 +0000 Subject: [PATCH 17/50] Adding debug information Signed-off-by: Amit Schendel --- .../applicationprofilecache/applicationprofilecache.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index a11c6b24..5c9d4ccc 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -346,6 +346,15 @@ func (ap *ApplicationProfileCacheImpl) mergeContainers(normalContainers, userMan func (ap *ApplicationProfileCacheImpl) mergeContainer(normalContainer, userContainer *v1beta1.ApplicationProfileContainer) { normalContainer.Capabilities = append(normalContainer.Capabilities, userContainer.Capabilities...) normalContainer.Execs = append(normalContainer.Execs, userContainer.Execs...) + + for _, exec := range userContainer.Execs { + logger.L().Debug("merged exec call", helpers.String("path", exec.Path)) + } + + for _, exec := range normalContainer.Execs { + logger.L().Debug("final exec call", helpers.String("path", exec.Path)) + } + normalContainer.Opens = append(normalContainer.Opens, userContainer.Opens...) normalContainer.Syscalls = append(normalContainer.Syscalls, userContainer.Syscalls...) normalContainer.Endpoints = append(normalContainer.Endpoints, userContainer.Endpoints...) From 991caf5f6738b146217a6695e05ac9cc058ae48a Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 14:43:40 +0000 Subject: [PATCH 18/50] Adding debug logs Signed-off-by: Amit Schendel --- .../applicationprofilecache/applicationprofilecache.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index 5c9d4ccc..87928211 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -99,6 +99,8 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be mergedProfile := ap.performMerge(freshBaseProfile, appProfile) ap.slugToAppProfile.Set(baseProfileUniqueName, mergedProfile) + logger.L().Debug("merged user-managed profile with fresh base profile", helpers.Interface("profile", mergedProfile)) + // Clean up the user-managed profile after successful merge ap.userManagedProfiles.Delete(baseProfileUniqueName) From 9cbed55a139518719be35ea1f72f0110499fb6c9 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 15:42:46 +0000 Subject: [PATCH 19/50] Fixing the paths Signed-off-by: Amit Schendel --- .../applicationprofilecache/applicationprofilecache.go | 10 ---------- tests/resources/user-profile.yaml | 8 ++++---- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index 87928211..2c4f8384 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -99,8 +99,6 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be mergedProfile := ap.performMerge(freshBaseProfile, appProfile) ap.slugToAppProfile.Set(baseProfileUniqueName, mergedProfile) - logger.L().Debug("merged user-managed profile with fresh base profile", helpers.Interface("profile", mergedProfile)) - // Clean up the user-managed profile after successful merge ap.userManagedProfiles.Delete(baseProfileUniqueName) @@ -349,14 +347,6 @@ func (ap *ApplicationProfileCacheImpl) mergeContainer(normalContainer, userConta normalContainer.Capabilities = append(normalContainer.Capabilities, userContainer.Capabilities...) normalContainer.Execs = append(normalContainer.Execs, userContainer.Execs...) - for _, exec := range userContainer.Execs { - logger.L().Debug("merged exec call", helpers.String("path", exec.Path)) - } - - for _, exec := range normalContainer.Execs { - logger.L().Debug("final exec call", helpers.String("path", exec.Path)) - } - normalContainer.Opens = append(normalContainer.Opens, userContainer.Opens...) normalContainer.Syscalls = append(normalContainer.Syscalls, userContainer.Syscalls...) normalContainer.Endpoints = append(normalContainer.Endpoints, userContainer.Endpoints...) diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml index d61a7843..f8b798ac 100644 --- a/tests/resources/user-profile.yaml +++ b/tests/resources/user-profile.yaml @@ -16,9 +16,9 @@ spec: syscalls: [] endpoints: [] execs: - - path: ls + - path: /usr/bin/ls args: - - "ls" + - "/usr/bin/ls" - "-l" seccompProfile: spec: @@ -31,9 +31,9 @@ spec: syscalls: [] endpoints: [] execs: - - path: ls + - path: /usr/bin/ls args: - - "ls" + - "/usr/bin/ls" - "-l" - path: /bin/grpc_health_probe args: From 698ed2d7fc0fc163aaafc5e521324052d6fb0886 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 15:57:18 +0000 Subject: [PATCH 20/50] Removing char Signed-off-by: Amit Schendel --- tests/resources/user-profile.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml index f8b798ac..bd6097a4 100644 --- a/tests/resources/user-profile.yaml +++ b/tests/resources/user-profile.yaml @@ -18,8 +18,8 @@ spec: execs: - path: /usr/bin/ls args: - - "/usr/bin/ls" - - "-l" + - /usr/bin/ls + - -l seccompProfile: spec: defaultAction: "" @@ -33,8 +33,8 @@ spec: execs: - path: /usr/bin/ls args: - - "/usr/bin/ls" - - "-l" + - /usr/bin/ls + - -l - path: /bin/grpc_health_probe args: - "-addr=:9555" From ca673f1814f5ea58dc587922a30a5fd30635ccad Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 16:13:42 +0000 Subject: [PATCH 21/50] Checking alert count Signed-off-by: Amit Schendel --- tests/component_test.go | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index da0a9f37..8ab710e2 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -758,6 +758,15 @@ func Test_12_MergingProfilesTest(t *testing.T) { initialAlerts, err := testutils.GetAlerts(wl.Namespace) require.NoError(t, err, "Failed to get initial alerts") + + // Record initial alert count + initialAlertCount := 0 + for _, alert := range initialAlerts { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected process launched" { + initialAlertCount++ + } + } + testutils.AssertContains(t, initialAlerts, "Unexpected process launched", "ls", "server") testutils.AssertNotContains(t, initialAlerts, "Unexpected process launched", "ls", "nginx") @@ -805,19 +814,23 @@ func Test_12_MergingProfilesTest(t *testing.T) { finalAlerts, err := testutils.GetAlerts(wl.Namespace) require.NoError(t, err, "Failed to get final alerts") - alertCounts := make(map[string]int) + // Only count new alerts (after the initial count) + newAlertCount := 0 for _, alert := range finalAlerts { - if ruleName, ok := alert.Labels["rule_name"]; ok { - alertCounts[ruleName]++ + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected process launched" { + newAlertCount++ } } - // Check for unexpected increase in alerts - for ruleName, count := range alertCounts { - if count > 1 && ruleName == "Unexpected process launched" { - // Print all alerts for debugging - t.Logf("Final alerts:\n%v", finalAlerts) - t.Errorf("Unexpected number of alerts for '%s': got %d, expected ≤ 1", ruleName, count) + t.Logf("Alert counts - Initial: %d, Final: %d", initialAlertCount, newAlertCount) + + if newAlertCount > initialAlertCount { + t.Logf("Full alert details:") + for _, alert := range finalAlerts { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected process launched" { + t.Logf("Alert: %+v", alert) + } } + t.Errorf("New alerts were generated after merge (Initial: %d, Final: %d)", initialAlertCount, newAlertCount) } } From 8fcdbe25e24a67999a2148da5e8d5d98dcc4fe44 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 16:27:21 +0000 Subject: [PATCH 22/50] Adding debug Signed-off-by: Amit Schendel --- .../applicationprofilecache.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index 2c4f8384..f99b9fb8 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -96,9 +96,19 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be return } + // Print the execs for debugging + for _, container := range freshBaseProfile.Spec.Containers { + logger.L().Debug("before execs", helpers.String("container", container.Name), helpers.String("execs", fmt.Sprintf("%v", container.Execs))) + } + mergedProfile := ap.performMerge(freshBaseProfile, appProfile) ap.slugToAppProfile.Set(baseProfileUniqueName, mergedProfile) + // Print the execs for debugging + for _, container := range mergedProfile.Spec.Containers { + logger.L().Debug("merged execs", helpers.String("container", container.Name), helpers.String("execs", fmt.Sprintf("%v", container.Execs))) + } + // Clean up the user-managed profile after successful merge ap.userManagedProfiles.Delete(baseProfileUniqueName) @@ -346,7 +356,6 @@ func (ap *ApplicationProfileCacheImpl) mergeContainers(normalContainers, userMan func (ap *ApplicationProfileCacheImpl) mergeContainer(normalContainer, userContainer *v1beta1.ApplicationProfileContainer) { normalContainer.Capabilities = append(normalContainer.Capabilities, userContainer.Capabilities...) normalContainer.Execs = append(normalContainer.Execs, userContainer.Execs...) - normalContainer.Opens = append(normalContainer.Opens, userContainer.Opens...) normalContainer.Syscalls = append(normalContainer.Syscalls, userContainer.Syscalls...) normalContainer.Endpoints = append(normalContainer.Endpoints, userContainer.Endpoints...) From 6746de198cace608aa96b40539fafd8acf082f34 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 16:47:41 +0000 Subject: [PATCH 23/50] Adding fixed code Signed-off-by: Amit Schendel --- .../applicationprofilecache.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index f99b9fb8..20bd4a32 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -336,15 +336,19 @@ func (ap *ApplicationProfileCacheImpl) performMerge(normalProfile, userManagedPr } func (ap *ApplicationProfileCacheImpl) mergeContainers(normalContainers, userManagedContainers []v1beta1.ApplicationProfileContainer) []v1beta1.ApplicationProfileContainer { - containerMap := make(map[string]*v1beta1.ApplicationProfileContainer) + // Create a map to store containers by name + containerMap := make(map[string]int) // map name to index in slice + // Store indices of normal containers for i := range normalContainers { - containerMap[normalContainers[i].Name] = &normalContainers[i] + containerMap[normalContainers[i].Name] = i } + // Merge or append user containers for _, userContainer := range userManagedContainers { - if normalContainer, exists := containerMap[userContainer.Name]; exists { - ap.mergeContainer(normalContainer, &userContainer) + if idx, exists := containerMap[userContainer.Name]; exists { + // Directly modify the container in the slice + ap.mergeContainer(&normalContainers[idx], &userContainer) } else { normalContainers = append(normalContainers, userContainer) } From c2517776e426ff477d986bbe5773d2900aff118f Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 17:08:36 +0000 Subject: [PATCH 24/50] copy Signed-off-by: Amit Schendel --- .../applicationprofilecache.go | 87 +++++++++++++++++-- 1 file changed, 82 insertions(+), 5 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index 20bd4a32..b3043447 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -322,19 +322,50 @@ func (ap *ApplicationProfileCacheImpl) addFullApplicationProfile(appProfile *v1b } func (ap *ApplicationProfileCacheImpl) performMerge(normalProfile, userManagedProfile *v1beta1.ApplicationProfile) *v1beta1.ApplicationProfile { + logger.L().Debug("starting profile merge", + helpers.String("normal-name", normalProfile.Name), + helpers.String("user-name", userManagedProfile.Name)) + + // Make a deep copy first mergedProfile := normalProfile.DeepCopy() + // Log container counts + logger.L().Debug("container counts", + helpers.Int("normal-containers", len(mergedProfile.Spec.Containers)), + helpers.Int("user-containers", len(userManagedProfile.Spec.Containers))) + // Merge spec mergedProfile.Spec.Containers = ap.mergeContainers(mergedProfile.Spec.Containers, userManagedProfile.Spec.Containers) mergedProfile.Spec.InitContainers = ap.mergeContainers(mergedProfile.Spec.InitContainers, userManagedProfile.Spec.InitContainers) mergedProfile.Spec.EphemeralContainers = ap.mergeContainers(mergedProfile.Spec.EphemeralContainers, userManagedProfile.Spec.EphemeralContainers) // Remove the user-managed annotation + if mergedProfile.Annotations == nil { + mergedProfile.Annotations = make(map[string]string) + } delete(mergedProfile.Annotations, "kubescape.io/managed-by") + // Log final container counts + logger.L().Debug("final container counts", + helpers.Int("merged-containers", len(mergedProfile.Spec.Containers))) + return mergedProfile } +// func (ap *ApplicationProfileCacheImpl) performMerge(normalProfile, userManagedProfile *v1beta1.ApplicationProfile) *v1beta1.ApplicationProfile { +// mergedProfile := normalProfile.DeepCopy() + +// // Merge spec +// mergedProfile.Spec.Containers = ap.mergeContainers(mergedProfile.Spec.Containers, userManagedProfile.Spec.Containers) +// mergedProfile.Spec.InitContainers = ap.mergeContainers(mergedProfile.Spec.InitContainers, userManagedProfile.Spec.InitContainers) +// mergedProfile.Spec.EphemeralContainers = ap.mergeContainers(mergedProfile.Spec.EphemeralContainers, userManagedProfile.Spec.EphemeralContainers) + +// // Remove the user-managed annotation +// delete(mergedProfile.Annotations, "kubescape.io/managed-by") + +// return mergedProfile +// } + func (ap *ApplicationProfileCacheImpl) mergeContainers(normalContainers, userManagedContainers []v1beta1.ApplicationProfileContainer) []v1beta1.ApplicationProfileContainer { // Create a map to store containers by name containerMap := make(map[string]int) // map name to index in slice @@ -358,13 +389,59 @@ func (ap *ApplicationProfileCacheImpl) mergeContainers(normalContainers, userMan } func (ap *ApplicationProfileCacheImpl) mergeContainer(normalContainer, userContainer *v1beta1.ApplicationProfileContainer) { - normalContainer.Capabilities = append(normalContainer.Capabilities, userContainer.Capabilities...) - normalContainer.Execs = append(normalContainer.Execs, userContainer.Execs...) - normalContainer.Opens = append(normalContainer.Opens, userContainer.Opens...) - normalContainer.Syscalls = append(normalContainer.Syscalls, userContainer.Syscalls...) - normalContainer.Endpoints = append(normalContainer.Endpoints, userContainer.Endpoints...) + logger.L().Debug("merging container", + helpers.String("container", normalContainer.Name), + helpers.Int("current-execs", len(normalContainer.Execs)), + helpers.Int("user-execs", len(userContainer.Execs))) + + // Create new slices and copy existing content + newCapabilities := make([]string, len(normalContainer.Capabilities)) + copy(newCapabilities, normalContainer.Capabilities) + newExecs := make([]v1beta1.ExecCalls, len(normalContainer.Execs)) + copy(newExecs, normalContainer.Execs) + newOpens := make([]v1beta1.OpenCalls, len(normalContainer.Opens)) + copy(newOpens, normalContainer.Opens) + newSyscalls := make([]string, len(normalContainer.Syscalls)) + copy(newSyscalls, normalContainer.Syscalls) + newEndpoints := make([]v1beta1.HTTPEndpoint, len(normalContainer.Endpoints)) + copy(newEndpoints, normalContainer.Endpoints) + + // Append new content to the new slices + newCapabilities = append(newCapabilities, userContainer.Capabilities...) + newExecs = append(newExecs, userContainer.Execs...) + newOpens = append(newOpens, userContainer.Opens...) + newSyscalls = append(newSyscalls, userContainer.Syscalls...) + newEndpoints = append(newEndpoints, userContainer.Endpoints...) + + // Assign new slices back to container + normalContainer.Capabilities = newCapabilities + normalContainer.Execs = newExecs + normalContainer.Opens = newOpens + normalContainer.Syscalls = newSyscalls + normalContainer.Endpoints = newEndpoints + + logger.L().Debug("after merge", + helpers.String("container", normalContainer.Name), + helpers.Int("final-execs", len(normalContainer.Execs))) + + // Log individual execs for verification + for i, exec := range normalContainer.Execs { + logger.L().Debug("exec entry", + helpers.String("container", normalContainer.Name), + helpers.Int("index", i), + helpers.String("path", exec.Path), + helpers.String("args", fmt.Sprintf("%v", exec.Args))) + } } +// func (ap *ApplicationProfileCacheImpl) mergeContainer(normalContainer, userContainer *v1beta1.ApplicationProfileContainer) { +// normalContainer.Capabilities = append(normalContainer.Capabilities, userContainer.Capabilities...) +// normalContainer.Execs = append(normalContainer.Execs, userContainer.Execs...) +// normalContainer.Opens = append(normalContainer.Opens, userContainer.Opens...) +// normalContainer.Syscalls = append(normalContainer.Syscalls, userContainer.Syscalls...) +// normalContainer.Endpoints = append(normalContainer.Endpoints, userContainer.Endpoints...) +// } + func (ap *ApplicationProfileCacheImpl) deleteApplicationProfile(obj runtime.Object) { appProfile := obj.(*v1beta1.ApplicationProfile) apName := objectcache.MetaUniqueName(appProfile) From c642ce1de2158dd8843347854d8545fb7abb8f90 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 17:26:31 +0000 Subject: [PATCH 25/50] Adding debug logs Signed-off-by: Amit Schendel --- .../applicationprofilecache.go | 95 +++---------------- 1 file changed, 13 insertions(+), 82 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index b3043447..b4beba01 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -84,6 +84,14 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be // Store the user-managed profile temporarily ap.userManagedProfiles.Set(baseProfileUniqueName, appProfile.DeepCopy()) + // For debugging purposes print the execs of the user-managed profile + for _, container := range appProfile.Spec.Containers { + logger.L().Debug("user-managed execs", helpers.String("container", container.Name), helpers.String("execs", fmt.Sprintf("%v", container.Execs))) + } + + // Print the user-managed profile + logger.L().Debug("added user-managed profile to cache", helpers.Interface("profile", appProfile)) + // If we have the base profile cached, fetch a fresh copy and merge if ap.slugToAppProfile.Has(baseProfileUniqueName) { // Fetch fresh base profile from cluster @@ -322,50 +330,19 @@ func (ap *ApplicationProfileCacheImpl) addFullApplicationProfile(appProfile *v1b } func (ap *ApplicationProfileCacheImpl) performMerge(normalProfile, userManagedProfile *v1beta1.ApplicationProfile) *v1beta1.ApplicationProfile { - logger.L().Debug("starting profile merge", - helpers.String("normal-name", normalProfile.Name), - helpers.String("user-name", userManagedProfile.Name)) - - // Make a deep copy first mergedProfile := normalProfile.DeepCopy() - // Log container counts - logger.L().Debug("container counts", - helpers.Int("normal-containers", len(mergedProfile.Spec.Containers)), - helpers.Int("user-containers", len(userManagedProfile.Spec.Containers))) - // Merge spec mergedProfile.Spec.Containers = ap.mergeContainers(mergedProfile.Spec.Containers, userManagedProfile.Spec.Containers) mergedProfile.Spec.InitContainers = ap.mergeContainers(mergedProfile.Spec.InitContainers, userManagedProfile.Spec.InitContainers) mergedProfile.Spec.EphemeralContainers = ap.mergeContainers(mergedProfile.Spec.EphemeralContainers, userManagedProfile.Spec.EphemeralContainers) // Remove the user-managed annotation - if mergedProfile.Annotations == nil { - mergedProfile.Annotations = make(map[string]string) - } delete(mergedProfile.Annotations, "kubescape.io/managed-by") - // Log final container counts - logger.L().Debug("final container counts", - helpers.Int("merged-containers", len(mergedProfile.Spec.Containers))) - return mergedProfile } -// func (ap *ApplicationProfileCacheImpl) performMerge(normalProfile, userManagedProfile *v1beta1.ApplicationProfile) *v1beta1.ApplicationProfile { -// mergedProfile := normalProfile.DeepCopy() - -// // Merge spec -// mergedProfile.Spec.Containers = ap.mergeContainers(mergedProfile.Spec.Containers, userManagedProfile.Spec.Containers) -// mergedProfile.Spec.InitContainers = ap.mergeContainers(mergedProfile.Spec.InitContainers, userManagedProfile.Spec.InitContainers) -// mergedProfile.Spec.EphemeralContainers = ap.mergeContainers(mergedProfile.Spec.EphemeralContainers, userManagedProfile.Spec.EphemeralContainers) - -// // Remove the user-managed annotation -// delete(mergedProfile.Annotations, "kubescape.io/managed-by") - -// return mergedProfile -// } - func (ap *ApplicationProfileCacheImpl) mergeContainers(normalContainers, userManagedContainers []v1beta1.ApplicationProfileContainer) []v1beta1.ApplicationProfileContainer { // Create a map to store containers by name containerMap := make(map[string]int) // map name to index in slice @@ -389,59 +366,13 @@ func (ap *ApplicationProfileCacheImpl) mergeContainers(normalContainers, userMan } func (ap *ApplicationProfileCacheImpl) mergeContainer(normalContainer, userContainer *v1beta1.ApplicationProfileContainer) { - logger.L().Debug("merging container", - helpers.String("container", normalContainer.Name), - helpers.Int("current-execs", len(normalContainer.Execs)), - helpers.Int("user-execs", len(userContainer.Execs))) - - // Create new slices and copy existing content - newCapabilities := make([]string, len(normalContainer.Capabilities)) - copy(newCapabilities, normalContainer.Capabilities) - newExecs := make([]v1beta1.ExecCalls, len(normalContainer.Execs)) - copy(newExecs, normalContainer.Execs) - newOpens := make([]v1beta1.OpenCalls, len(normalContainer.Opens)) - copy(newOpens, normalContainer.Opens) - newSyscalls := make([]string, len(normalContainer.Syscalls)) - copy(newSyscalls, normalContainer.Syscalls) - newEndpoints := make([]v1beta1.HTTPEndpoint, len(normalContainer.Endpoints)) - copy(newEndpoints, normalContainer.Endpoints) - - // Append new content to the new slices - newCapabilities = append(newCapabilities, userContainer.Capabilities...) - newExecs = append(newExecs, userContainer.Execs...) - newOpens = append(newOpens, userContainer.Opens...) - newSyscalls = append(newSyscalls, userContainer.Syscalls...) - newEndpoints = append(newEndpoints, userContainer.Endpoints...) - - // Assign new slices back to container - normalContainer.Capabilities = newCapabilities - normalContainer.Execs = newExecs - normalContainer.Opens = newOpens - normalContainer.Syscalls = newSyscalls - normalContainer.Endpoints = newEndpoints - - logger.L().Debug("after merge", - helpers.String("container", normalContainer.Name), - helpers.Int("final-execs", len(normalContainer.Execs))) - - // Log individual execs for verification - for i, exec := range normalContainer.Execs { - logger.L().Debug("exec entry", - helpers.String("container", normalContainer.Name), - helpers.Int("index", i), - helpers.String("path", exec.Path), - helpers.String("args", fmt.Sprintf("%v", exec.Args))) - } + normalContainer.Capabilities = append(normalContainer.Capabilities, userContainer.Capabilities...) + normalContainer.Execs = append(normalContainer.Execs, userContainer.Execs...) + normalContainer.Opens = append(normalContainer.Opens, userContainer.Opens...) + normalContainer.Syscalls = append(normalContainer.Syscalls, userContainer.Syscalls...) + normalContainer.Endpoints = append(normalContainer.Endpoints, userContainer.Endpoints...) } -// func (ap *ApplicationProfileCacheImpl) mergeContainer(normalContainer, userContainer *v1beta1.ApplicationProfileContainer) { -// normalContainer.Capabilities = append(normalContainer.Capabilities, userContainer.Capabilities...) -// normalContainer.Execs = append(normalContainer.Execs, userContainer.Execs...) -// normalContainer.Opens = append(normalContainer.Opens, userContainer.Opens...) -// normalContainer.Syscalls = append(normalContainer.Syscalls, userContainer.Syscalls...) -// normalContainer.Endpoints = append(normalContainer.Endpoints, userContainer.Endpoints...) -// } - func (ap *ApplicationProfileCacheImpl) deleteApplicationProfile(obj runtime.Object) { appProfile := obj.(*v1beta1.ApplicationProfile) apName := objectcache.MetaUniqueName(appProfile) From a15331503da1436e6b76b81bb51a0cb8bd8f4892 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 22 Oct 2024 17:50:24 +0000 Subject: [PATCH 26/50] Adding resource version Signed-off-by: Amit Schendel --- tests/resources/user-profile.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml index bd6097a4..9b67324a 100644 --- a/tests/resources/user-profile.yaml +++ b/tests/resources/user-profile.yaml @@ -3,6 +3,7 @@ kind: ApplicationProfile metadata: name: {name} namespace: {namespace} + resourceVersion: "1" # Start with "1" for new resources annotations: kubescape.io/managed-by: User spec: From 28f6f741285403c05d05b424b74f6001f3fed926 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Wed, 23 Oct 2024 05:55:08 +0000 Subject: [PATCH 27/50] Replacing the client to be the storage client Signed-off-by: Amit Schendel --- tests/component_test.go | 83 +++++++++++++++++++++++++++-------------- 1 file changed, 56 insertions(+), 27 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index 8ab710e2..2e489790 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -6,12 +6,9 @@ import ( "context" "encoding/json" "fmt" - "os" - "os/exec" "path" "slices" "sort" - "strings" "testing" "time" @@ -772,34 +769,66 @@ func Test_12_MergingProfilesTest(t *testing.T) { // PHASE 3: Apply user-managed profile t.Log("Applying user-managed profile...") - userProfileBytes, err := os.ReadFile(path.Join(utils.CurrentDir(), "resources/user-profile.yaml")) - require.NoError(t, err, "Failed to read user profile template") - - // Create replacements map with properly quoted values - replacements := map[string]string{ - "{name}": fmt.Sprintf("ug-%s", initialProfile.Name), - "{namespace}": initialProfile.Namespace, - } - - userProfileContent := string(userProfileBytes) - for placeholder, value := range replacements { - userProfileContent = strings.Replace(userProfileContent, placeholder, value, 1) + // Create the user-managed profile + userProfile := &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("ug-%s", initialProfile.Name), + Namespace: initialProfile.Namespace, + Annotations: map[string]string{ + "kubescape.io/managed-by": "User", + }, + }, + Spec: v1beta1.ApplicationProfileSpec{ + Architectures: []string{"amd64"}, + Containers: []v1beta1.ApplicationProfileContainer{ + { + Name: "nginx", + Execs: []v1beta1.ExecCalls{ + { + Path: "/usr/bin/ls", + Args: []string{"/usr/bin/ls", "-l"}, + }, + }, + SeccompProfile: v1beta1.SingleSeccompProfile{ + Spec: v1beta1.SingleSeccompProfileSpec{ + DefaultAction: "", + }, + }, + }, + { + Name: "server", + Execs: []v1beta1.ExecCalls{ + { + Path: "/usr/bin/ls", + Args: []string{"/usr/bin/ls", "-l"}, + }, + { + Path: "/bin/grpc_health_probe", + Args: []string{"-addr=:9555"}, + }, + }, + SeccompProfile: v1beta1.SingleSeccompProfile{ + Spec: v1beta1.SingleSeccompProfileSpec{ + DefaultAction: "", + }, + }, + }, + }, + }, } - t.Logf("User profile content:\n%s", userProfileContent) - - // Create and apply the user profile - tmpFile, err := os.CreateTemp("", "user-profile-*.yaml") - require.NoError(t, err, "Failed to create temp file") - defer os.Remove(tmpFile.Name()) + // Log the profile we're about to create + userProfileJSON, err := json.MarshalIndent(userProfile, "", " ") + require.NoError(t, err, "Failed to marshal user profile") + t.Logf("Creating user profile:\n%s", string(userProfileJSON)) - err = os.WriteFile(tmpFile.Name(), []byte(userProfileContent), 0644) - require.NoError(t, err, "Failed to write user profile to file") + // Get k8s client + k8sClient := k8sinterface.NewKubernetesApi() - output, err := exec.Command("kubectl", "apply", "-f", tmpFile.Name()).CombinedOutput() - if err != nil { - t.Fatalf("Failed to apply user profile: %v\nOutput: %s", err, string(output)) - } + // Create the user-managed profile + storageClient := spdxv1beta1client.NewForConfigOrDie(k8sClient.K8SConfig) + _, err = storageClient.ApplicationProfiles(ns.Name).Create(context.Background(), userProfile, metav1.CreateOptions{}) + require.NoError(t, err, "Failed to create user profile") // PHASE 4: Verify merged profile behavior t.Log("Verifying merged profile behavior...") From 69c8bad0448987ab3e6659012de3375bfb3ec971 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Wed, 23 Oct 2024 06:17:10 +0000 Subject: [PATCH 28/50] Fetching fullAP from the storage Signed-off-by: Amit Schendel --- .../applicationprofilecache/applicationprofilecache.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index b4beba01..fb45c56e 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -81,8 +81,15 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be baseProfileName := strings.TrimPrefix(appProfile.GetName(), "ug-") baseProfileUniqueName := objectcache.UniqueName(appProfile.GetNamespace(), baseProfileName) + // Get the full user managed profile from the storage + fullAP, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + if err != nil { + logger.L().Error("failed to get full application profile", helpers.Error(err)) + return + } + // Store the user-managed profile temporarily - ap.userManagedProfiles.Set(baseProfileUniqueName, appProfile.DeepCopy()) + ap.userManagedProfiles.Set(baseProfileUniqueName, fullAP) // For debugging purposes print the execs of the user-managed profile for _, container := range appProfile.Spec.Containers { From 42ea6d94c2fbd02eb304ea4b4afa08358b0129c3 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Wed, 23 Oct 2024 06:29:34 +0000 Subject: [PATCH 29/50] Adding fixed code Signed-off-by: Amit Schendel --- .../applicationprofilecache/applicationprofilecache.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index fb45c56e..87b7bcdf 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -92,12 +92,12 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be ap.userManagedProfiles.Set(baseProfileUniqueName, fullAP) // For debugging purposes print the execs of the user-managed profile - for _, container := range appProfile.Spec.Containers { + for _, container := range fullAP.Spec.Containers { logger.L().Debug("user-managed execs", helpers.String("container", container.Name), helpers.String("execs", fmt.Sprintf("%v", container.Execs))) } // Print the user-managed profile - logger.L().Debug("added user-managed profile to cache", helpers.Interface("profile", appProfile)) + logger.L().Debug("added user-managed profile to cache", helpers.Interface("profile", fullAP)) // If we have the base profile cached, fetch a fresh copy and merge if ap.slugToAppProfile.Has(baseProfileUniqueName) { @@ -116,7 +116,7 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be logger.L().Debug("before execs", helpers.String("container", container.Name), helpers.String("execs", fmt.Sprintf("%v", container.Execs))) } - mergedProfile := ap.performMerge(freshBaseProfile, appProfile) + mergedProfile := ap.performMerge(freshBaseProfile, fullAP) ap.slugToAppProfile.Set(baseProfileUniqueName, mergedProfile) // Print the execs for debugging From 148a2f5ec0706a4e2581a07bdbfb8b2f3840651a Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Wed, 23 Oct 2024 06:43:28 +0000 Subject: [PATCH 30/50] Adding update for the application profile Signed-off-by: Amit Schendel --- .../applicationprofilecache/applicationprofilecache.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index 87b7bcdf..1476e790 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -99,7 +99,8 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be // Print the user-managed profile logger.L().Debug("added user-managed profile to cache", helpers.Interface("profile", fullAP)) - // If we have the base profile cached, fetch a fresh copy and merge + // If we have the base profile cached, fetch a fresh copy and merge. + // If the base profile is not cached yet, the merge will be attempted when it's added. if ap.slugToAppProfile.Has(baseProfileUniqueName) { // Fetch fresh base profile from cluster freshBaseProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), baseProfileName) @@ -127,12 +128,13 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be // Clean up the user-managed profile after successful merge ap.userManagedProfiles.Delete(baseProfileUniqueName) + // Update the cached AP with the merged profile + ap.slugToAppProfile.Set(baseProfileUniqueName, mergedProfile) + logger.L().Debug("merged user-managed profile with fresh base profile", helpers.String("name", baseProfileName), helpers.String("namespace", appProfile.GetNamespace())) } - - // If the base profile is not cached yet, the merge will be attempted when it's added. } func (ap *ApplicationProfileCacheImpl) addApplicationProfile(obj runtime.Object) { From 98607680cd9af47430ac8fbc3effe21572a41214 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Wed, 23 Oct 2024 06:50:34 +0000 Subject: [PATCH 31/50] Fixing test data Signed-off-by: Amit Schendel --- .../applicationprofilecache/applicationprofilecache.go | 3 --- tests/component_test.go | 4 ++-- tests/resources/user-profile.yaml | 4 ++-- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index 1476e790..c75b8cfa 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -128,9 +128,6 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be // Clean up the user-managed profile after successful merge ap.userManagedProfiles.Delete(baseProfileUniqueName) - // Update the cached AP with the merged profile - ap.slugToAppProfile.Set(baseProfileUniqueName, mergedProfile) - logger.L().Debug("merged user-managed profile with fresh base profile", helpers.String("name", baseProfileName), helpers.String("namespace", appProfile.GetNamespace())) diff --git a/tests/component_test.go b/tests/component_test.go index 2e489790..1630a070 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -799,8 +799,8 @@ func Test_12_MergingProfilesTest(t *testing.T) { Name: "server", Execs: []v1beta1.ExecCalls{ { - Path: "/usr/bin/ls", - Args: []string{"/usr/bin/ls", "-l"}, + Path: "/bin/ls", + Args: []string{"/bin/ls", "-l"}, }, { Path: "/bin/grpc_health_probe", diff --git a/tests/resources/user-profile.yaml b/tests/resources/user-profile.yaml index 9b67324a..97a116f6 100644 --- a/tests/resources/user-profile.yaml +++ b/tests/resources/user-profile.yaml @@ -32,9 +32,9 @@ spec: syscalls: [] endpoints: [] execs: - - path: /usr/bin/ls + - path: /bin/ls args: - - /usr/bin/ls + - /bin/ls - -l - path: /bin/grpc_health_probe args: From 8815788d7d6f42687ac2502db7311871e2e291ed Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Wed, 23 Oct 2024 07:10:23 +0000 Subject: [PATCH 32/50] Adding a test for patching the application profile Signed-off-by: Amit Schendel --- .../applicationprofilecache.go | 15 ------ tests/component_test.go | 51 ++++++++++++++++++- 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index c75b8cfa..ae3930ca 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -91,11 +91,6 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be // Store the user-managed profile temporarily ap.userManagedProfiles.Set(baseProfileUniqueName, fullAP) - // For debugging purposes print the execs of the user-managed profile - for _, container := range fullAP.Spec.Containers { - logger.L().Debug("user-managed execs", helpers.String("container", container.Name), helpers.String("execs", fmt.Sprintf("%v", container.Execs))) - } - // Print the user-managed profile logger.L().Debug("added user-managed profile to cache", helpers.Interface("profile", fullAP)) @@ -112,19 +107,9 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be return } - // Print the execs for debugging - for _, container := range freshBaseProfile.Spec.Containers { - logger.L().Debug("before execs", helpers.String("container", container.Name), helpers.String("execs", fmt.Sprintf("%v", container.Execs))) - } - mergedProfile := ap.performMerge(freshBaseProfile, fullAP) ap.slugToAppProfile.Set(baseProfileUniqueName, mergedProfile) - // Print the execs for debugging - for _, container := range mergedProfile.Spec.Containers { - logger.L().Debug("merged execs", helpers.String("container", container.Name), helpers.String("execs", fmt.Sprintf("%v", container.Execs))) - } - // Clean up the user-managed profile after successful merge ap.userManagedProfiles.Delete(baseProfileUniqueName) diff --git a/tests/component_test.go b/tests/component_test.go index 1630a070..714d2c97 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -832,7 +832,7 @@ func Test_12_MergingProfilesTest(t *testing.T) { // PHASE 4: Verify merged profile behavior t.Log("Verifying merged profile behavior...") - time.Sleep(15 * time.Second) // Allow merge to complete + time.Sleep(5 * time.Second) // Allow merge to complete // Test merged profile behavior wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") // Expected: no alert @@ -862,4 +862,53 @@ func Test_12_MergingProfilesTest(t *testing.T) { } t.Errorf("New alerts were generated after merge (Initial: %d, Final: %d)", initialAlertCount, newAlertCount) } + + // PHASE 5: Check PATCH (removing the ls command from the user profile of the server container and triggering an alert) + t.Log("Patching user profile to remove ls command from server container...") + patchOperations := []utils.PatchOperation{ + {Op: "remove", Path: "/spec/containers/1/execs/0"}, + } + + patch, err := json.Marshal(patchOperations) + require.NoError(t, err, "Failed to marshal patch operations") + + _, err = storageClient.ApplicationProfiles(ns.Name).Patch(context.Background(), userProfile.Name, types.JSONPatchType, patch, metav1.PatchOptions{}) + require.NoError(t, err, "Failed to patch user profile") + + // Verify patched profile behavior + time.Sleep(5 * time.Second) // Allow merge to complete + + // Log the profile that was patched + patchedProfile, err := wl.GetApplicationProfile() + require.NoError(t, err, "Failed to get patched profile") + t.Logf("Patched application profile:\n%v", patchedProfile) + + // Test patched profile behavior + wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") // Expected: no alert + wl.ExecIntoPod([]string{"ls", "-l"}, "server") // Expected: alert (ls command removed from user profile) + time.Sleep(10 * time.Second) // Wait for potential alerts + + // Verify alert counts + finalAlerts, err = testutils.GetAlerts(wl.Namespace) + require.NoError(t, err, "Failed to get final alerts") + + // Only count new alerts (after the initial count) + newAlertCount = 0 + for _, alert := range finalAlerts { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected process launched" { + newAlertCount++ + } + } + + t.Logf("Alert counts - Initial: %d, Final: %d", initialAlertCount, newAlertCount) + + if newAlertCount <= initialAlertCount { + t.Logf("Full alert details:") + for _, alert := range finalAlerts { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected process launched" { + t.Logf("Alert: %+v", alert) + } + } + t.Errorf("New alerts were not generated after patch (Initial: %d, Final: %d)", initialAlertCount, newAlertCount) + } } From 371b8b4b27ff5c67ecd38af0d7e1ed95f33b6d65 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Wed, 23 Oct 2024 07:24:16 +0000 Subject: [PATCH 33/50] Removing debug logs Signed-off-by: Amit Schendel --- .../applicationprofilecache/applicationprofilecache.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index ae3930ca..d32e2c1e 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -91,9 +91,6 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be // Store the user-managed profile temporarily ap.userManagedProfiles.Set(baseProfileUniqueName, fullAP) - // Print the user-managed profile - logger.L().Debug("added user-managed profile to cache", helpers.Interface("profile", fullAP)) - // If we have the base profile cached, fetch a fresh copy and merge. // If the base profile is not cached yet, the merge will be attempted when it's added. if ap.slugToAppProfile.Has(baseProfileUniqueName) { From faa881df79cba95cd56f07ee7c372b3d10e83cfb Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Wed, 23 Oct 2024 07:57:25 +0000 Subject: [PATCH 34/50] Bring up the time to complete Signed-off-by: Amit Schendel --- tests/component_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index 714d2c97..ad03fe11 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -832,7 +832,7 @@ func Test_12_MergingProfilesTest(t *testing.T) { // PHASE 4: Verify merged profile behavior t.Log("Verifying merged profile behavior...") - time.Sleep(5 * time.Second) // Allow merge to complete + time.Sleep(15 * time.Second) // Allow merge to complete // Test merged profile behavior wl.ExecIntoPod([]string{"ls", "-l"}, "nginx") // Expected: no alert @@ -876,7 +876,7 @@ func Test_12_MergingProfilesTest(t *testing.T) { require.NoError(t, err, "Failed to patch user profile") // Verify patched profile behavior - time.Sleep(5 * time.Second) // Allow merge to complete + time.Sleep(15 * time.Second) // Allow merge to complete // Log the profile that was patched patchedProfile, err := wl.GetApplicationProfile() From 209381119c2f707f93e9586c8a51e3121a6f39bc Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Sun, 27 Oct 2024 08:58:10 +0000 Subject: [PATCH 35/50] Adding NN merging Signed-off-by: Amit Schendel --- .../networkneighborhoodcache.go | 315 ++++++++++++++++-- .../networkneighborhoodcache_test.go | 240 +++++++++++++ tests/component_test.go | 214 ++++++++++++ 3 files changed, 743 insertions(+), 26 deletions(-) diff --git a/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go b/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go index 6509ef61..4d0f16ef 100644 --- a/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go +++ b/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go @@ -3,6 +3,7 @@ package networkneighborhoodcache import ( "context" "fmt" + "strings" "time" mapset "github.com/deckarep/golang-set/v2" @@ -47,24 +48,26 @@ func newNetworkNeighborhoodState(nn *v1beta1.NetworkNeighborhood) networkNeighbo } type NetworkNeighborhoodCacheImpl struct { - containerToSlug maps.SafeMap[string, string] // cache the containerID to slug mapping, this will enable a quick lookup of the network neighborhood - slugToNetworkNeighborhood maps.SafeMap[string, *v1beta1.NetworkNeighborhood] // cache the network neighborhood - slugToContainers maps.SafeMap[string, mapset.Set[string]] // cache the containerIDs that belong to the network neighborhood, this will enable removing from cache NN without pods - slugToState maps.SafeMap[string, networkNeighborhoodState] // cache the containerID to slug mapping, this will enable a quick lookup of the network neighborhood - storageClient versioned.SpdxV1beta1Interface - allNetworkNeighborhoods mapset.Set[string] // cache all the NN that are ready. this will enable removing from cache NN without pods that are running on the same node - nodeName string - maxDelaySeconds int // maximum delay in seconds before getting the full object from the storage + containerToSlug maps.SafeMap[string, string] // cache the containerID to slug mapping, this will enable a quick lookup of the network neighborhood + slugToNetworkNeighborhood maps.SafeMap[string, *v1beta1.NetworkNeighborhood] // cache the network neighborhood + slugToContainers maps.SafeMap[string, mapset.Set[string]] // cache the containerIDs that belong to the network neighborhood, this will enable removing from cache NN without pods + slugToState maps.SafeMap[string, networkNeighborhoodState] // cache the containerID to slug mapping, this will enable a quick lookup of the network neighborhood + storageClient versioned.SpdxV1beta1Interface + allNetworkNeighborhoods mapset.Set[string] // cache all the NN that are ready. this will enable removing from cache NN without pods that are running on the same node + nodeName string + maxDelaySeconds int // maximum delay in seconds before getting the full object from the storage + userManagedNetworkNeighborhood maps.SafeMap[string, *v1beta1.NetworkNeighborhood] } func NewNetworkNeighborhoodCache(nodeName string, storageClient versioned.SpdxV1beta1Interface, maxDelaySeconds int) *NetworkNeighborhoodCacheImpl { return &NetworkNeighborhoodCacheImpl{ - nodeName: nodeName, - maxDelaySeconds: maxDelaySeconds, - storageClient: storageClient, - containerToSlug: maps.SafeMap[string, string]{}, - slugToContainers: maps.SafeMap[string, mapset.Set[string]]{}, - allNetworkNeighborhoods: mapset.NewSet[string](), + nodeName: nodeName, + maxDelaySeconds: maxDelaySeconds, + storageClient: storageClient, + containerToSlug: maps.SafeMap[string, string]{}, + slugToContainers: maps.SafeMap[string, mapset.Set[string]]{}, + allNetworkNeighborhoods: mapset.NewSet[string](), + userManagedNetworkNeighborhood: maps.SafeMap[string, *v1beta1.NetworkNeighborhood]{}, } } @@ -78,6 +81,215 @@ func (nn *NetworkNeighborhoodCacheImpl) GetNetworkNeighborhood(containerID strin return nil } +func (nn *NetworkNeighborhoodCacheImpl) handleUserManagedNN(netNeighborhood *v1beta1.NetworkNeighborhood) { + baseNNName := strings.TrimPrefix(netNeighborhood.GetName(), "ug-") + baseNNUniqueName := objectcache.UniqueName(netNeighborhood.GetNamespace(), baseNNName) + + // Get the full user managed network neighborhood from the storage + fullNN, err := nn.getNetworkNeighborhood(netNeighborhood.GetNamespace(), netNeighborhood.GetName()) + if err != nil { + logger.L().Error("failed to get full network neighborhood", helpers.Error(err)) + return + } + + // Store the user-managed network neighborhood temporarily + nn.userManagedNetworkNeighborhood.Set(baseNNUniqueName, fullNN) + + // If we have the base network neighborhood cached, fetch a fresh copy and merge. + // If the base network neighborhood is not cached yet, the merge will be attempted when it's added. + if nn.slugToNetworkNeighborhood.Has(baseNNUniqueName) { + // Fetch fresh base network neighborhood from cluster + freshBaseNN, err := nn.getNetworkNeighborhood(netNeighborhood.GetNamespace(), baseNNName) + if err != nil { + logger.L().Error("failed to get fresh base network neighborhood for merging", + helpers.String("name", baseNNName), + helpers.String("namespace", netNeighborhood.GetNamespace()), + helpers.Error(err)) + return + } + + mergedNN := nn.performMerge(freshBaseNN, fullNN) + nn.slugToNetworkNeighborhood.Set(baseNNUniqueName, mergedNN) + + // Clean up the user-managed network neighborhood after successful merge + nn.userManagedNetworkNeighborhood.Delete(baseNNUniqueName) + + logger.L().Debug("merged user-managed network neighborhood with fresh base network neighborhood", + helpers.String("name", baseNNName), + helpers.String("namespace", netNeighborhood.GetNamespace())) + } +} + +func (nn *NetworkNeighborhoodCacheImpl) performMerge(normalNN, userManagedNN *v1beta1.NetworkNeighborhood) *v1beta1.NetworkNeighborhood { + mergedNN := normalNN.DeepCopy() + + // Merge spec containers + mergedNN.Spec.Containers = nn.mergeContainers(mergedNN.Spec.Containers, userManagedNN.Spec.Containers) + mergedNN.Spec.InitContainers = nn.mergeContainers(mergedNN.Spec.InitContainers, userManagedNN.Spec.InitContainers) + mergedNN.Spec.EphemeralContainers = nn.mergeContainers(mergedNN.Spec.EphemeralContainers, userManagedNN.Spec.EphemeralContainers) + + // Merge LabelSelector + if userManagedNN.Spec.LabelSelector.MatchLabels != nil { + if mergedNN.Spec.LabelSelector.MatchLabels == nil { + mergedNN.Spec.LabelSelector.MatchLabels = make(map[string]string) + } + for k, v := range userManagedNN.Spec.LabelSelector.MatchLabels { + mergedNN.Spec.LabelSelector.MatchLabels[k] = v + } + } + mergedNN.Spec.LabelSelector.MatchExpressions = append( + mergedNN.Spec.LabelSelector.MatchExpressions, + userManagedNN.Spec.LabelSelector.MatchExpressions..., + ) + + // Remove the user-managed annotation + delete(mergedNN.Annotations, "kubescape.io/managed-by") + + return mergedNN +} + +func (nn *NetworkNeighborhoodCacheImpl) mergeContainers(normalContainers, userManagedContainers []v1beta1.NetworkNeighborhoodContainer) []v1beta1.NetworkNeighborhoodContainer { + // Create a map to store containers by name + containerMap := make(map[string]int) // map name to index in slice + + // Store indices of normal containers + for i := range normalContainers { + containerMap[normalContainers[i].Name] = i + } + + // Merge or append user containers + for _, userContainer := range userManagedContainers { + if idx, exists := containerMap[userContainer.Name]; exists { + // Directly modify the container in the slice + nn.mergeContainer(&normalContainers[idx], &userContainer) + } else { + normalContainers = append(normalContainers, userContainer) + } + } + + return normalContainers +} + +func (nn *NetworkNeighborhoodCacheImpl) mergeContainer(normalContainer, userContainer *v1beta1.NetworkNeighborhoodContainer) { + // Merge ingress rules + normalContainer.Ingress = nn.mergeNetworkNeighbors(normalContainer.Ingress, userContainer.Ingress) + + // Merge egress rules + normalContainer.Egress = nn.mergeNetworkNeighbors(normalContainer.Egress, userContainer.Egress) +} + +func (nn *NetworkNeighborhoodCacheImpl) mergeNetworkNeighbors(normalNeighbors, userNeighbors []v1beta1.NetworkNeighbor) []v1beta1.NetworkNeighbor { + // Use map to track existing neighbors by identifier + neighborMap := make(map[string]int) + for i, neighbor := range normalNeighbors { + neighborMap[neighbor.Identifier] = i + } + + // Merge or append user neighbors + for _, userNeighbor := range userNeighbors { + if idx, exists := neighborMap[userNeighbor.Identifier]; exists { + // Merge existing neighbor + normalNeighbors[idx] = nn.mergeNetworkNeighbor(normalNeighbors[idx], userNeighbor) + } else { + // Append new neighbor + normalNeighbors = append(normalNeighbors, userNeighbor) + } + } + + return normalNeighbors +} + +func (nn *NetworkNeighborhoodCacheImpl) mergeNetworkNeighbor(normal, user v1beta1.NetworkNeighbor) v1beta1.NetworkNeighbor { + merged := normal.DeepCopy() + + // Merge DNS names (removing duplicates) + dnsNamesSet := make(map[string]struct{}) + for _, dns := range normal.DNSNames { + dnsNamesSet[dns] = struct{}{} + } + for _, dns := range user.DNSNames { + dnsNamesSet[dns] = struct{}{} + } + merged.DNSNames = make([]string, 0, len(dnsNamesSet)) + for dns := range dnsNamesSet { + merged.DNSNames = append(merged.DNSNames, dns) + } + + // Merge ports based on patchMergeKey (name) + merged.Ports = nn.mergeNetworkPorts(merged.Ports, user.Ports) + + // Merge pod selector if provided + if user.PodSelector != nil { + if merged.PodSelector == nil { + merged.PodSelector = &metav1.LabelSelector{} + } + if user.PodSelector.MatchLabels != nil { + if merged.PodSelector.MatchLabels == nil { + merged.PodSelector.MatchLabels = make(map[string]string) + } + for k, v := range user.PodSelector.MatchLabels { + merged.PodSelector.MatchLabels[k] = v + } + } + merged.PodSelector.MatchExpressions = append( + merged.PodSelector.MatchExpressions, + user.PodSelector.MatchExpressions..., + ) + } + + // Merge namespace selector if provided + if user.NamespaceSelector != nil { + if merged.NamespaceSelector == nil { + merged.NamespaceSelector = &metav1.LabelSelector{} + } + if user.NamespaceSelector.MatchLabels != nil { + if merged.NamespaceSelector.MatchLabels == nil { + merged.NamespaceSelector.MatchLabels = make(map[string]string) + } + for k, v := range user.NamespaceSelector.MatchLabels { + merged.NamespaceSelector.MatchLabels[k] = v + } + } + merged.NamespaceSelector.MatchExpressions = append( + merged.NamespaceSelector.MatchExpressions, + user.NamespaceSelector.MatchExpressions..., + ) + } + + // Take the user's IP address if provided + if user.IPAddress != "" { + merged.IPAddress = user.IPAddress + } + + // Take the user's type if provided + if user.Type != "" { + merged.Type = user.Type + } + + return *merged +} + +func (nn *NetworkNeighborhoodCacheImpl) mergeNetworkPorts(normalPorts, userPorts []v1beta1.NetworkPort) []v1beta1.NetworkPort { + // Use map to track existing ports by name (patchMergeKey) + portMap := make(map[string]int) + for i, port := range normalPorts { + portMap[port.Name] = i + } + + // Merge or append user ports + for _, userPort := range userPorts { + if idx, exists := portMap[userPort.Name]; exists { + // Update existing port + normalPorts[idx] = userPort + } else { + // Append new port + normalPorts = append(normalPorts, userPort) + } + } + + return normalPorts +} + // ------------------ watcher.Adaptor methods ----------------------- // ------------------ watcher.WatchResources methods ----------------------- @@ -217,12 +429,18 @@ func (nn *NetworkNeighborhoodCacheImpl) addNetworkNeighborhood(_ context.Context netNeighborhood := obj.(*v1beta1.NetworkNeighborhood) nnName := objectcache.MetaUniqueName(netNeighborhood) + isUserManaged := netNeighborhood.Annotations != nil && + netNeighborhood.Annotations["kubescape.io/managed-by"] == "User" && + strings.HasPrefix(netNeighborhood.GetName(), "ug-") + + if isUserManaged { + nn.handleUserManagedNN(netNeighborhood) + return + } + nnState := newNetworkNeighborhoodState(netNeighborhood) nn.slugToState.Set(nnName, nnState) - // the cache holds only completed network neighborhoods. - // check if the network neighborhood is completed - // if status was completed and now is not (e.g. mode changed from complete to partial), remove from cache if nnState.status != helpersv1.Completed { if nn.slugToNetworkNeighborhood.Has(nnName) { nn.slugToNetworkNeighborhood.Delete(nnName) @@ -231,13 +449,9 @@ func (nn *NetworkNeighborhoodCacheImpl) addNetworkNeighborhood(_ context.Context return } - // add to the cache nn.allNetworkNeighborhoods.Add(nnName) if nn.slugToContainers.Has(nnName) { - // get the full network neighborhood from the storage - // the watch only returns the metadata - // avoid thundering herd problem by adding a random delay time.AfterFunc(utils.RandomDuration(nn.maxDelaySeconds, time.Second), func() { nn.addFullNetworkNeighborhood(netNeighborhood, nnName) }) @@ -250,6 +464,16 @@ func (nn *NetworkNeighborhoodCacheImpl) addFullNetworkNeighborhood(netNeighborho logger.L().Error("failed to get full network neighborhood", helpers.Error(err)) return } + + // Check if there's a pending user-managed network neighborhood to merge + if nn.userManagedNetworkNeighborhood.Has(nnName) { + userManagedNN := nn.userManagedNetworkNeighborhood.Get(nnName) + fullNN = nn.performMerge(fullNN, userManagedNN) + // Clean up the user-managed network neighborhood after successful merge + nn.userManagedNetworkNeighborhood.Delete(nnName) + logger.L().Debug("merged pending user-managed network neighborhood", helpers.String("name", nnName)) + } + nn.slugToNetworkNeighborhood.Set(nnName, fullNN) for _, i := range nn.slugToContainers.Get(nnName).ToSlice() { nn.containerToSlug.Set(i, nnName) @@ -258,12 +482,51 @@ func (nn *NetworkNeighborhoodCacheImpl) addFullNetworkNeighborhood(netNeighborho } func (nn *NetworkNeighborhoodCacheImpl) deleteNetworkNeighborhood(obj runtime.Object) { - nnName := objectcache.MetaUniqueName(obj.(metav1.Object)) - nn.slugToNetworkNeighborhood.Delete(nnName) - nn.slugToState.Delete(nnName) - nn.allNetworkNeighborhoods.Remove(nnName) + netNeighborhood := obj.(*v1beta1.NetworkNeighborhood) + nnName := objectcache.MetaUniqueName(netNeighborhood) - logger.L().Info("deleted network neighborhood from cache", helpers.String("uniqueSlug", nnName)) + isUserManaged := netNeighborhood.Annotations != nil && + netNeighborhood.Annotations["kubescape.io/managed-by"] == "User" && + strings.HasPrefix(netNeighborhood.GetName(), "ug-") + + if isUserManaged { + // For user-managed network neighborhoods, we need to use the base name for cleanup + baseNNName := strings.TrimPrefix(netNeighborhood.GetName(), "ug-") + baseNNUniqueName := objectcache.UniqueName(netNeighborhood.GetNamespace(), baseNNName) + nn.userManagedNetworkNeighborhood.Delete(baseNNUniqueName) + + logger.L().Debug("deleted user-managed network neighborhood from cache", + helpers.String("nnName", netNeighborhood.GetName()), + helpers.String("baseNN", baseNNName)) + } else { + // For normal network neighborhoods, clean up all related data + nn.slugToNetworkNeighborhood.Delete(nnName) + nn.slugToState.Delete(nnName) + nn.allNetworkNeighborhoods.Remove(nnName) + + logger.L().Debug("deleted network neighborhood from cache", + helpers.String("uniqueSlug", nnName)) + } + + // Clean up any orphaned user-managed network neighborhoods + nn.cleanupOrphanedUserManagedNNs() +} + +// Add cleanup method for orphaned user-managed network neighborhoods +func (nn *NetworkNeighborhoodCacheImpl) cleanupOrphanedUserManagedNNs() { + nn.userManagedNetworkNeighborhood.Range(func(key string, value *v1beta1.NetworkNeighborhood) bool { + if nn.slugToNetworkNeighborhood.Has(key) { + // If base network neighborhood exists but merge didn't happen for some reason, + // attempt merge again and cleanup + if baseNN := nn.slugToNetworkNeighborhood.Get(key); baseNN != nil { + mergedNN := nn.performMerge(baseNN, value) + nn.slugToNetworkNeighborhood.Set(key, mergedNN) + nn.userManagedNetworkNeighborhood.Delete(key) + logger.L().Debug("cleaned up orphaned user-managed network neighborhood", helpers.String("name", key)) + } + } + return true + }) } func (nn *NetworkNeighborhoodCacheImpl) getNetworkNeighborhood(namespace, name string) (*v1beta1.NetworkNeighborhood, error) { diff --git a/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache_test.go b/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache_test.go index acb96444..49f415be 100644 --- a/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache_test.go +++ b/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache_test.go @@ -734,3 +734,243 @@ func Test_addPod(t *testing.T) { }) } } + +func Test_performMerge(t *testing.T) { + tests := []struct { + name string + baseNN *v1beta1.NetworkNeighborhood + userNN *v1beta1.NetworkNeighborhood + expectedResult *v1beta1.NetworkNeighborhood + validateResults func(*testing.T, *v1beta1.NetworkNeighborhood) + }{ + { + name: "merge basic network neighbors", + baseNN: &v1beta1.NetworkNeighborhood{ + Spec: v1beta1.NetworkNeighborhoodSpec{ + Containers: []v1beta1.NetworkNeighborhoodContainer{ + { + Name: "container1", + Ingress: []v1beta1.NetworkNeighbor{ + { + Identifier: "ingress1", + Type: "http", + DNSNames: []string{"example.com"}, + Ports: []v1beta1.NetworkPort{ + {Name: "http", Protocol: "TCP", Port: ptr(int32(80))}, + }, + }, + }, + }, + }, + }, + }, + userNN: &v1beta1.NetworkNeighborhood{ + Spec: v1beta1.NetworkNeighborhoodSpec{ + Containers: []v1beta1.NetworkNeighborhoodContainer{ + { + Name: "container1", + Ingress: []v1beta1.NetworkNeighbor{ + { + Identifier: "ingress2", + Type: "https", + DNSNames: []string{"secure.example.com"}, + Ports: []v1beta1.NetworkPort{ + {Name: "https", Protocol: "TCP", Port: ptr(int32(443))}, + }, + }, + }, + }, + }, + }, + }, + validateResults: func(t *testing.T, result *v1beta1.NetworkNeighborhood) { + assert.Len(t, result.Spec.Containers, 1) + assert.Len(t, result.Spec.Containers[0].Ingress, 2) + + // Verify both ingress rules are present + ingressIdentifiers := []string{ + result.Spec.Containers[0].Ingress[0].Identifier, + result.Spec.Containers[0].Ingress[1].Identifier, + } + assert.Contains(t, ingressIdentifiers, "ingress1") + assert.Contains(t, ingressIdentifiers, "ingress2") + }, + }, + { + name: "merge label selectors", + baseNN: &v1beta1.NetworkNeighborhood{ + Spec: v1beta1.NetworkNeighborhoodSpec{ + LabelSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "base", + }, + }, + Containers: []v1beta1.NetworkNeighborhoodContainer{ + { + Name: "container1", + Egress: []v1beta1.NetworkNeighbor{ + { + Identifier: "egress1", + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "role": "db", + }, + }, + }, + }, + }, + }, + }, + }, + userNN: &v1beta1.NetworkNeighborhood{ + Spec: v1beta1.NetworkNeighborhoodSpec{ + LabelSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "env": "prod", + }, + }, + Containers: []v1beta1.NetworkNeighborhoodContainer{ + { + Name: "container1", + Egress: []v1beta1.NetworkNeighbor{ + { + Identifier: "egress1", + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "version": "v1", + }, + }, + }, + }, + }, + }, + }, + }, + validateResults: func(t *testing.T, result *v1beta1.NetworkNeighborhood) { + // Verify merged label selectors + assert.Equal(t, "base", result.Spec.LabelSelector.MatchLabels["app"]) + assert.Equal(t, "prod", result.Spec.LabelSelector.MatchLabels["env"]) + + // Verify merged pod selector in egress rule + container := result.Spec.Containers[0] + podSelector := container.Egress[0].PodSelector + assert.Equal(t, "db", podSelector.MatchLabels["role"]) + assert.Equal(t, "v1", podSelector.MatchLabels["version"]) + }, + }, + { + name: "merge network ports", + baseNN: &v1beta1.NetworkNeighborhood{ + Spec: v1beta1.NetworkNeighborhoodSpec{ + Containers: []v1beta1.NetworkNeighborhoodContainer{ + { + Name: "container1", + Egress: []v1beta1.NetworkNeighbor{ + { + Identifier: "egress1", + Ports: []v1beta1.NetworkPort{ + {Name: "http", Protocol: "TCP", Port: ptr(int32(80))}, + }, + }, + }, + }, + }, + }, + }, + userNN: &v1beta1.NetworkNeighborhood{ + Spec: v1beta1.NetworkNeighborhoodSpec{ + Containers: []v1beta1.NetworkNeighborhoodContainer{ + { + Name: "container1", + Egress: []v1beta1.NetworkNeighbor{ + { + Identifier: "egress1", + Ports: []v1beta1.NetworkPort{ + {Name: "http", Protocol: "TCP", Port: ptr(int32(8080))}, // Override existing port + {Name: "https", Protocol: "TCP", Port: ptr(int32(443))}, // Add new port + }, + }, + }, + }, + }, + }, + }, + validateResults: func(t *testing.T, result *v1beta1.NetworkNeighborhood) { + container := result.Spec.Containers[0] + ports := container.Egress[0].Ports + + // Verify ports are properly merged + assert.Len(t, ports, 2) + + // Find HTTP port - should be updated to 8080 + for _, port := range ports { + if port.Name == "http" { + assert.Equal(t, int32(8080), *port.Port) + } + if port.Name == "https" { + assert.Equal(t, int32(443), *port.Port) + } + } + }, + }, + { + name: "merge DNS names", + baseNN: &v1beta1.NetworkNeighborhood{ + Spec: v1beta1.NetworkNeighborhoodSpec{ + Containers: []v1beta1.NetworkNeighborhoodContainer{ + { + Name: "container1", + Egress: []v1beta1.NetworkNeighbor{ + { + Identifier: "egress1", + DNSNames: []string{"example.com", "api.example.com"}, + }, + }, + }, + }, + }, + }, + userNN: &v1beta1.NetworkNeighborhood{ + Spec: v1beta1.NetworkNeighborhoodSpec{ + Containers: []v1beta1.NetworkNeighborhoodContainer{ + { + Name: "container1", + Egress: []v1beta1.NetworkNeighbor{ + { + Identifier: "egress1", + DNSNames: []string{"api.example.com", "admin.example.com"}, + }, + }, + }, + }, + }, + }, + validateResults: func(t *testing.T, result *v1beta1.NetworkNeighborhood) { + container := result.Spec.Containers[0] + dnsNames := container.Egress[0].DNSNames + + // Verify DNS names are properly merged and deduplicated + assert.Len(t, dnsNames, 3) + assert.Contains(t, dnsNames, "example.com") + assert.Contains(t, dnsNames, "api.example.com") + assert.Contains(t, dnsNames, "admin.example.com") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + nn := NewNetworkNeighborhoodCache("test-node", nil, 0) + result := nn.performMerge(tt.baseNN, tt.userNN) + + if tt.validateResults != nil { + tt.validateResults(t, result) + } + }) + } +} + +// Helper function to create pointer to int32 +func ptr(i int32) *int32 { + return &i +} diff --git a/tests/component_test.go b/tests/component_test.go index ad03fe11..b87a2018 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -912,3 +912,217 @@ func Test_12_MergingProfilesTest(t *testing.T) { t.Errorf("New alerts were not generated after patch (Initial: %d, Final: %d)", initialAlertCount, newAlertCount) } } + +func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { + start := time.Now() + defer tearDownTest(t, start) + + // PHASE 1: Setup workload and initial network neighborhood + ns := testutils.NewRandomNamespace() + wl, err := testutils.NewTestWorkload(ns.Name, path.Join(utils.CurrentDir(), "resources/deployment-multiple-containers.yaml")) + require.NoError(t, err, "Failed to create workload") + require.NoError(t, wl.WaitForReady(80), "Workload failed to be ready") + require.NoError(t, wl.WaitForNetworkNeighborhood(80, "ready"), "Network neighborhood not ready") + + // Generate initial network data + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") + require.NoError(t, err, "Failed to exec wget in server container") + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") + require.NoError(t, err, "Failed to exec curl in nginx container") + + require.NoError(t, wl.WaitForNetworkNeighborhoodCompletion(80), "Network neighborhood failed to complete") + time.Sleep(10 * time.Second) // Allow network neighborhood processing + + // Log initial network neighborhood state + initialNN, err := wl.GetNetworkNeighborhood() + require.NoError(t, err, "Failed to get initial network neighborhood") + initialNNJSON, _ := json.Marshal(initialNN) + t.Logf("Initial network neighborhood:\n%s", string(initialNNJSON)) + + // PHASE 2: Verify initial alerts + t.Log("Testing initial alert generation...") + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert + _, _, err = wl.ExecIntoPod([]string{"curl", "example.com", "-m", "2"}, "server") // Expected: alert + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert + _, _, err = wl.ExecIntoPod([]string{"wget", "github.com", "-T", "2", "-t", "1"}, "nginx") // Expected: alert + time.Sleep(30 * time.Second) // Wait for alert generation + + initialAlerts, err := testutils.GetAlerts(wl.Namespace) + require.NoError(t, err, "Failed to get initial alerts") + + // Record initial alert count + initialAlertCount := 0 + for _, alert := range initialAlerts { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { + initialAlertCount++ + } + } + + testutils.AssertContains(t, initialAlerts, "Unexpected domain request", "curl", "server") + testutils.AssertNotContains(t, initialAlerts, "Unexpected domain request", "wget", "server") + testutils.AssertContains(t, initialAlerts, "Unexpected domain request", "wget", "nginx") + testutils.AssertNotContains(t, initialAlerts, "Unexpected domain request", "curl", "nginx") + + // PHASE 3: Apply user-managed network neighborhood + t.Log("Applying user-managed network neighborhood...") + userNN := &v1beta1.NetworkNeighborhood{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("ug-%s", initialNN.Name), + Namespace: initialNN.Namespace, + Annotations: map[string]string{ + "kubescape.io/managed-by": "User", + }, + }, + Spec: v1beta1.NetworkNeighborhoodSpec{ + Containers: []v1beta1.NetworkNeighborhoodContainer{ + { + Name: "nginx", + Egress: []v1beta1.NetworkNeighbor{ + { + Identifier: "nginx-github", + Type: "dns", + DNSNames: []string{"github.com."}, + Ports: []v1beta1.NetworkPort{ + { + Name: "http", + Protocol: "TCP", + Port: ptr(int32(80)), + }, + { + Name: "https", + Protocol: "TCP", + Port: ptr(int32(443)), + }, + }, + }, + }, + }, + { + Name: "server", + Egress: []v1beta1.NetworkNeighbor{ + { + Identifier: "server-example", + Type: "dns", + DNSNames: []string{"example.com."}, + Ports: []v1beta1.NetworkPort{ + { + Name: "http", + Protocol: "TCP", + Port: ptr(int32(80)), + }, + { + Name: "https", + Protocol: "TCP", + Port: ptr(int32(443)), + }, + }, + }, + }, + }, + }, + }, + } + + // Log the network neighborhood we're about to create + userNNJSON, err := json.MarshalIndent(userNN, "", " ") + require.NoError(t, err, "Failed to marshal user network neighborhood") + t.Logf("Creating user network neighborhood:\n%s", string(userNNJSON)) + + // Get k8s client + k8sClient := k8sinterface.NewKubernetesApi() + + // Create the user-managed network neighborhood + storageClient := spdxv1beta1client.NewForConfigOrDie(k8sClient.K8SConfig) + _, err = storageClient.NetworkNeighborhoods(ns.Name).Create(context.Background(), userNN, metav1.CreateOptions{}) + require.NoError(t, err, "Failed to create user network neighborhood") + + // PHASE 4: Verify merged network neighborhood behavior + t.Log("Verifying merged network neighborhood behavior...") + time.Sleep(15 * time.Second) // Allow merge to complete + + // Test merged network neighborhood behavior + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original) + _, _, err = wl.ExecIntoPod([]string{"curl", "example.com", "-m", "2"}, "server") // Expected: no alert (user added) + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original) + _, _, err = wl.ExecIntoPod([]string{"wget", "github.com", "-T", "2", "-t", "1"}, "nginx") // Expected: no alert (user added) + time.Sleep(10 * time.Second) // Wait for potential alerts + + // Verify alert counts + finalAlerts, err := testutils.GetAlerts(wl.Namespace) + require.NoError(t, err, "Failed to get final alerts") + + // Only count new alerts (after the initial count) + newAlertCount := 0 + for _, alert := range finalAlerts { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { + newAlertCount++ + } + } + + t.Logf("Alert counts - Initial: %d, Final: %d", initialAlertCount, newAlertCount) + + if newAlertCount > initialAlertCount { + t.Logf("Full alert details:") + for _, alert := range finalAlerts { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { + t.Logf("Alert: %+v", alert) + } + } + t.Errorf("New alerts were generated after merge (Initial: %d, Final: %d)", initialAlertCount, newAlertCount) + } + + // PHASE 5: Check PATCH (removing example.com from the server container and triggering an alert) + t.Log("Patching user network neighborhood to remove example.com from server container...") + patchOperations := []utils.PatchOperation{ + {Op: "remove", Path: "/spec/containers/1/egress/0"}, + } + + patch, err := json.Marshal(patchOperations) + require.NoError(t, err, "Failed to marshal patch operations") + + _, err = storageClient.NetworkNeighborhoods(ns.Name).Patch(context.Background(), userNN.Name, types.JSONPatchType, patch, metav1.PatchOptions{}) + require.NoError(t, err, "Failed to patch user network neighborhood") + + // Verify patched network neighborhood behavior + time.Sleep(15 * time.Second) // Allow merge to complete + + // Log the network neighborhood that was patched + patchedNN, err := wl.GetNetworkNeighborhood() + require.NoError(t, err, "Failed to get patched network neighborhood") + t.Logf("Patched network neighborhood:\n%v", patchedNN) + + // Test patched network neighborhood behavior + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert + _, _, err = wl.ExecIntoPod([]string{"curl", "example.com", "-m", "2"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert + _, _, err = wl.ExecIntoPod([]string{"wget", "github.com", "-T", "2", "-t", "1"}, "nginx") // Expected: no alert + time.Sleep(10 * time.Second) // Wait for potential alerts + + // Verify alert counts + finalAlerts, err = testutils.GetAlerts(wl.Namespace) + require.NoError(t, err, "Failed to get final alerts") + + // Only count new alerts (after the initial count) + newAlertCount = 0 + for _, alert := range finalAlerts { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { + newAlertCount++ + } + } + + t.Logf("Alert counts - Initial: %d, Final: %d", initialAlertCount, newAlertCount) + + if newAlertCount <= initialAlertCount { + t.Logf("Full alert details:") + for _, alert := range finalAlerts { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { + t.Logf("Alert: %+v", alert) + } + } + t.Errorf("New alerts were not generated after patch (Initial: %d, Final: %d)", initialAlertCount, newAlertCount) + } +} + +func ptr(i int32) *int32 { + return &i +} From 1c36e0a3dcfa9206982175b96c966824e1984029 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Sun, 27 Oct 2024 09:02:49 +0000 Subject: [PATCH 36/50] Adding test to component tests Signed-off-by: Amit Schendel --- .github/workflows/component-tests.yaml | 1 + tests/component_test.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/component-tests.yaml b/.github/workflows/component-tests.yaml index 92639865..931dde77 100644 --- a/.github/workflows/component-tests.yaml +++ b/.github/workflows/component-tests.yaml @@ -51,6 +51,7 @@ jobs: Test_10_MalwareDetectionTest, Test_11_EndpointTest, Test_12_MergingProfilesTest, + Test_13_MergingNetworkNeighborhoodTest, # Test_10_DemoTest # Test_11_DuplicationTest ] diff --git a/tests/component_test.go b/tests/component_test.go index b87a2018..a7b4ed3b 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -980,7 +980,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { Egress: []v1beta1.NetworkNeighbor{ { Identifier: "nginx-github", - Type: "dns", + Type: "external", DNSNames: []string{"github.com."}, Ports: []v1beta1.NetworkPort{ { @@ -1002,7 +1002,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { Egress: []v1beta1.NetworkNeighbor{ { Identifier: "server-example", - Type: "dns", + Type: "external", DNSNames: []string{"example.com."}, Ports: []v1beta1.NetworkPort{ { From f0e074855494a098a2ad91f148d0204c9f15f752 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Sun, 27 Oct 2024 09:46:57 +0000 Subject: [PATCH 37/50] Adding fixed test Signed-off-by: Amit Schendel --- .github/workflows/component-tests.yaml | 2 - tests/component_test.go | 78 +++++++++++--------------- 2 files changed, 34 insertions(+), 46 deletions(-) diff --git a/.github/workflows/component-tests.yaml b/.github/workflows/component-tests.yaml index 931dde77..5640d4bb 100644 --- a/.github/workflows/component-tests.yaml +++ b/.github/workflows/component-tests.yaml @@ -52,8 +52,6 @@ jobs: Test_11_EndpointTest, Test_12_MergingProfilesTest, Test_13_MergingNetworkNeighborhoodTest, - # Test_10_DemoTest - # Test_11_DuplicationTest ] steps: - name: Checkout code diff --git a/tests/component_test.go b/tests/component_test.go index a7b4ed3b..9ca1e1ef 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -941,10 +941,10 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { // PHASE 2: Verify initial alerts t.Log("Testing initial alert generation...") - _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert - _, _, err = wl.ExecIntoPod([]string{"curl", "example.com", "-m", "2"}, "server") // Expected: alert - _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert - _, _, err = wl.ExecIntoPod([]string{"wget", "github.com", "-T", "2", "-t", "1"}, "nginx") // Expected: alert + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original rule) + _, _, err = wl.ExecIntoPod([]string{"curl", "example.com", "-m", "2"}, "server") // Expected: alert (not allowed) + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original rule) + _, _, err = wl.ExecIntoPod([]string{"wget", "github.com", "-T", "2", "-t", "1"}, "nginx") // Expected: alert (not allowed) time.Sleep(30 * time.Second) // Wait for alert generation initialAlerts, err := testutils.GetAlerts(wl.Namespace) @@ -958,6 +958,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { } } + // Verify initial alerts testutils.AssertContains(t, initialAlerts, "Unexpected domain request", "curl", "server") testutils.AssertNotContains(t, initialAlerts, "Unexpected domain request", "wget", "server") testutils.AssertContains(t, initialAlerts, "Unexpected domain request", "wget", "nginx") @@ -974,6 +975,11 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { }, }, Spec: v1beta1.NetworkNeighborhoodSpec{ + LabelSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "multiple-containers-app", + }, + }, Containers: []v1beta1.NetworkNeighborhoodContainer{ { Name: "nginx", @@ -984,12 +990,12 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { DNSNames: []string{"github.com."}, Ports: []v1beta1.NetworkPort{ { - Name: "http", + Name: "TCP-80", Protocol: "TCP", Port: ptr(int32(80)), }, { - Name: "https", + Name: "TCP-443", Protocol: "TCP", Port: ptr(int32(443)), }, @@ -1006,12 +1012,12 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { DNSNames: []string{"example.com."}, Ports: []v1beta1.NetworkPort{ { - Name: "http", + Name: "TCP-80", Protocol: "TCP", Port: ptr(int32(80)), }, { - Name: "https", + Name: "TCP-443", Protocol: "TCP", Port: ptr(int32(443)), }, @@ -1023,55 +1029,46 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { }, } - // Log the network neighborhood we're about to create - userNNJSON, err := json.MarshalIndent(userNN, "", " ") - require.NoError(t, err, "Failed to marshal user network neighborhood") - t.Logf("Creating user network neighborhood:\n%s", string(userNNJSON)) - - // Get k8s client + // Create user-managed network neighborhood k8sClient := k8sinterface.NewKubernetesApi() - - // Create the user-managed network neighborhood storageClient := spdxv1beta1client.NewForConfigOrDie(k8sClient.K8SConfig) _, err = storageClient.NetworkNeighborhoods(ns.Name).Create(context.Background(), userNN, metav1.CreateOptions{}) require.NoError(t, err, "Failed to create user network neighborhood") - // PHASE 4: Verify merged network neighborhood behavior + // PHASE 4: Verify merged behavior (no new alerts) t.Log("Verifying merged network neighborhood behavior...") time.Sleep(15 * time.Second) // Allow merge to complete - // Test merged network neighborhood behavior _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original) _, _, err = wl.ExecIntoPod([]string{"curl", "example.com", "-m", "2"}, "server") // Expected: no alert (user added) _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original) _, _, err = wl.ExecIntoPod([]string{"wget", "github.com", "-T", "2", "-t", "1"}, "nginx") // Expected: no alert (user added) - time.Sleep(10 * time.Second) // Wait for potential alerts + time.Sleep(30 * time.Second) // Wait for potential alerts - // Verify alert counts - finalAlerts, err := testutils.GetAlerts(wl.Namespace) - require.NoError(t, err, "Failed to get final alerts") + mergedAlerts, err := testutils.GetAlerts(wl.Namespace) + require.NoError(t, err, "Failed to get alerts after merge") - // Only count new alerts (after the initial count) + // Count new alerts after merge newAlertCount := 0 - for _, alert := range finalAlerts { + for _, alert := range mergedAlerts { if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { newAlertCount++ } } - t.Logf("Alert counts - Initial: %d, Final: %d", initialAlertCount, newAlertCount) + t.Logf("Alert counts - Initial: %d, After merge: %d", initialAlertCount, newAlertCount) if newAlertCount > initialAlertCount { t.Logf("Full alert details:") - for _, alert := range finalAlerts { + for _, alert := range mergedAlerts { if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { t.Logf("Alert: %+v", alert) } } - t.Errorf("New alerts were generated after merge (Initial: %d, Final: %d)", initialAlertCount, newAlertCount) + t.Errorf("New alerts were generated after merge (Initial: %d, After merge: %d)", initialAlertCount, newAlertCount) } - // PHASE 5: Check PATCH (removing example.com from the server container and triggering an alert) + // PHASE 5: Remove permission via patch and verify alerts return t.Log("Patching user network neighborhood to remove example.com from server container...") patchOperations := []utils.PatchOperation{ {Op: "remove", Path: "/spec/containers/1/egress/0"}, @@ -1083,43 +1080,36 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { _, err = storageClient.NetworkNeighborhoods(ns.Name).Patch(context.Background(), userNN.Name, types.JSONPatchType, patch, metav1.PatchOptions{}) require.NoError(t, err, "Failed to patch user network neighborhood") - // Verify patched network neighborhood behavior time.Sleep(15 * time.Second) // Allow merge to complete - // Log the network neighborhood that was patched - patchedNN, err := wl.GetNetworkNeighborhood() - require.NoError(t, err, "Failed to get patched network neighborhood") - t.Logf("Patched network neighborhood:\n%v", patchedNN) - - // Test patched network neighborhood behavior + // Test alerts after patch _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert _, _, err = wl.ExecIntoPod([]string{"curl", "example.com", "-m", "2"}, "server") // Expected: alert (removed) _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert _, _, err = wl.ExecIntoPod([]string{"wget", "github.com", "-T", "2", "-t", "1"}, "nginx") // Expected: no alert - time.Sleep(10 * time.Second) // Wait for potential alerts + time.Sleep(30 * time.Second) // Wait for alerts - // Verify alert counts - finalAlerts, err = testutils.GetAlerts(wl.Namespace) + finalAlerts, err := testutils.GetAlerts(wl.Namespace) require.NoError(t, err, "Failed to get final alerts") - // Only count new alerts (after the initial count) - newAlertCount = 0 + // Count final alerts + finalAlertCount := 0 for _, alert := range finalAlerts { if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { - newAlertCount++ + finalAlertCount++ } } - t.Logf("Alert counts - Initial: %d, Final: %d", initialAlertCount, newAlertCount) + t.Logf("Alert counts - Initial: %d, Final: %d", initialAlertCount, finalAlertCount) - if newAlertCount <= initialAlertCount { + if finalAlertCount <= initialAlertCount { t.Logf("Full alert details:") for _, alert := range finalAlerts { if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { t.Logf("Alert: %+v", alert) } } - t.Errorf("New alerts were not generated after patch (Initial: %d, Final: %d)", initialAlertCount, newAlertCount) + t.Errorf("New alerts were not generated after patch (Initial: %d, Final: %d)", initialAlertCount, finalAlertCount) } } From 39977026b63a1e8ec705da63e746317c923dfa46 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Sun, 27 Oct 2024 09:54:29 +0000 Subject: [PATCH 38/50] Adding fixed test Signed-off-by: Amit Schendel --- tests/component_test.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index 9ca1e1ef..ae3eca0d 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -941,11 +941,11 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { // PHASE 2: Verify initial alerts t.Log("Testing initial alert generation...") - _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original rule) - _, _, err = wl.ExecIntoPod([]string{"curl", "example.com", "-m", "2"}, "server") // Expected: alert (not allowed) - _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original rule) - _, _, err = wl.ExecIntoPod([]string{"wget", "github.com", "-T", "2", "-t", "1"}, "nginx") // Expected: alert (not allowed) - time.Sleep(30 * time.Second) // Wait for alert generation + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original rule) + _, _, err = wl.ExecIntoPod([]string{"wget", "example.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (not allowed) + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original rule) + _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: alert (not allowed) + time.Sleep(30 * time.Second) // Wait for alert generation initialAlerts, err := testutils.GetAlerts(wl.Namespace) require.NoError(t, err, "Failed to get initial alerts") @@ -959,9 +959,9 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { } // Verify initial alerts - testutils.AssertContains(t, initialAlerts, "Unexpected domain request", "curl", "server") + testutils.AssertContains(t, initialAlerts, "Unexpected domain request", "wget", "server") testutils.AssertNotContains(t, initialAlerts, "Unexpected domain request", "wget", "server") - testutils.AssertContains(t, initialAlerts, "Unexpected domain request", "wget", "nginx") + testutils.AssertContains(t, initialAlerts, "Unexpected domain request", "curl", "nginx") testutils.AssertNotContains(t, initialAlerts, "Unexpected domain request", "curl", "nginx") // PHASE 3: Apply user-managed network neighborhood @@ -1039,11 +1039,11 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { t.Log("Verifying merged network neighborhood behavior...") time.Sleep(15 * time.Second) // Allow merge to complete - _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original) - _, _, err = wl.ExecIntoPod([]string{"curl", "example.com", "-m", "2"}, "server") // Expected: no alert (user added) - _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original) - _, _, err = wl.ExecIntoPod([]string{"wget", "github.com", "-T", "2", "-t", "1"}, "nginx") // Expected: no alert (user added) - time.Sleep(30 * time.Second) // Wait for potential alerts + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original) + _, _, err = wl.ExecIntoPod([]string{"wget", "example.com", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original) + _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert (user added) + time.Sleep(30 * time.Second) // Wait for potential alerts mergedAlerts, err := testutils.GetAlerts(wl.Namespace) require.NoError(t, err, "Failed to get alerts after merge") @@ -1083,11 +1083,11 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { time.Sleep(15 * time.Second) // Allow merge to complete // Test alerts after patch - _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert - _, _, err = wl.ExecIntoPod([]string{"curl", "example.com", "-m", "2"}, "server") // Expected: alert (removed) - _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert - _, _, err = wl.ExecIntoPod([]string{"wget", "github.com", "-T", "2", "-t", "1"}, "nginx") // Expected: no alert - time.Sleep(30 * time.Second) // Wait for alerts + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert + _, _, err = wl.ExecIntoPod([]string{"wget", "example.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert + _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert + time.Sleep(30 * time.Second) // Wait for alerts finalAlerts, err := testutils.GetAlerts(wl.Namespace) require.NoError(t, err, "Failed to get final alerts") From 0ae7baa912bb8b3addc36479a1cb7b172a38dad0 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Sun, 27 Oct 2024 10:13:14 +0000 Subject: [PATCH 39/50] Fixing test Signed-off-by: Amit Schendel --- tests/component_test.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index ae3eca0d..e2c83b25 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -941,11 +941,11 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { // PHASE 2: Verify initial alerts t.Log("Testing initial alert generation...") - _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original rule) - _, _, err = wl.ExecIntoPod([]string{"wget", "example.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (not allowed) - _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original rule) - _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: alert (not allowed) - time.Sleep(30 * time.Second) // Wait for alert generation + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original rule) + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (not allowed) + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original rule) + _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: alert (not allowed) + time.Sleep(30 * time.Second) // Wait for alert generation initialAlerts, err := testutils.GetAlerts(wl.Namespace) require.NoError(t, err, "Failed to get initial alerts") @@ -1009,7 +1009,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { { Identifier: "server-example", Type: "external", - DNSNames: []string{"example.com."}, + DNSNames: []string{"httpforever.com."}, Ports: []v1beta1.NetworkPort{ { Name: "TCP-80", @@ -1039,11 +1039,11 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { t.Log("Verifying merged network neighborhood behavior...") time.Sleep(15 * time.Second) // Allow merge to complete - _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original) - _, _, err = wl.ExecIntoPod([]string{"wget", "example.com", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) - _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original) - _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert (user added) - time.Sleep(30 * time.Second) // Wait for potential alerts + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original) + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original) + _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert (user added) + time.Sleep(30 * time.Second) // Wait for potential alerts mergedAlerts, err := testutils.GetAlerts(wl.Namespace) require.NoError(t, err, "Failed to get alerts after merge") @@ -1069,7 +1069,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { } // PHASE 5: Remove permission via patch and verify alerts return - t.Log("Patching user network neighborhood to remove example.com from server container...") + t.Log("Patching user network neighborhood to remove httpforever.com from server container...") patchOperations := []utils.PatchOperation{ {Op: "remove", Path: "/spec/containers/1/egress/0"}, } @@ -1083,11 +1083,11 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { time.Sleep(15 * time.Second) // Allow merge to complete // Test alerts after patch - _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert - _, _, err = wl.ExecIntoPod([]string{"wget", "example.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) - _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert - _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert - time.Sleep(30 * time.Second) // Wait for alerts + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert + _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert + time.Sleep(30 * time.Second) // Wait for alerts finalAlerts, err := testutils.GetAlerts(wl.Namespace) require.NoError(t, err, "Failed to get final alerts") From 9af0956ee368ea48b85abd8f1100c504fdba97c8 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Sun, 27 Oct 2024 10:40:18 +0000 Subject: [PATCH 40/50] Adding fixed test Signed-off-by: Amit Schendel --- tests/component_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index e2c83b25..8203172d 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -960,9 +960,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { // Verify initial alerts testutils.AssertContains(t, initialAlerts, "Unexpected domain request", "wget", "server") - testutils.AssertNotContains(t, initialAlerts, "Unexpected domain request", "wget", "server") testutils.AssertContains(t, initialAlerts, "Unexpected domain request", "curl", "nginx") - testutils.AssertNotContains(t, initialAlerts, "Unexpected domain request", "curl", "nginx") // PHASE 3: Apply user-managed network neighborhood t.Log("Applying user-managed network neighborhood...") From f1376407548dcd746270fc0a9106baa7d6bebaed Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Sun, 27 Oct 2024 12:15:29 +0000 Subject: [PATCH 41/50] Adding more alerts to make sure alert is sending Signed-off-by: Amit Schendel --- tests/component_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/component_test.go b/tests/component_test.go index 8203172d..d60f48f1 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -943,6 +943,8 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { t.Log("Testing initial alert generation...") _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original rule) _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (not allowed) + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (not allowed) + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (not allowed) _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original rule) _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: alert (not allowed) time.Sleep(30 * time.Second) // Wait for alert generation @@ -1081,7 +1083,11 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { time.Sleep(15 * time.Second) // Allow merge to complete // Test alerts after patch - _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert + // Try multiple times to ensure alert is removed + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert From 81e044060b8679103033d7f7d55d8f6740d4ad15 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Sun, 27 Oct 2024 12:29:42 +0000 Subject: [PATCH 42/50] Making sure the alert really got removed Signed-off-by: Amit Schendel --- tests/component_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/component_test.go b/tests/component_test.go index d60f48f1..aed77074 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -1039,7 +1039,11 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { t.Log("Verifying merged network neighborhood behavior...") time.Sleep(15 * time.Second) // Allow merge to complete - _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original) + _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original) + // Try multiple times to ensure alert is removed + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original) _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert (user added) From 2ef9abe47a827a6ef09b54f7cd1b2d426f5ff01e Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Mon, 28 Oct 2024 07:55:20 +0000 Subject: [PATCH 43/50] Fix CR comments Signed-off-by: Amit Schendel --- .../applicationprofilecache.go | 60 +++++++-------- .../applicationprofilecache_test.go | 74 ------------------- .../networkneighborhoodcache.go | 49 +++++------- 3 files changed, 45 insertions(+), 138 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index d32e2c1e..be2925d0 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -60,7 +60,7 @@ type ApplicationProfileCacheImpl struct { } func NewApplicationProfileCache(nodeName string, storageClient versioned.SpdxV1beta1Interface, maxDelaySeconds int) *ApplicationProfileCacheImpl { - cache := &ApplicationProfileCacheImpl{ + return &ApplicationProfileCacheImpl{ nodeName: nodeName, maxDelaySeconds: maxDelaySeconds, storageClient: storageClient, @@ -71,8 +71,6 @@ func NewApplicationProfileCache(nodeName string, storageClient versioned.SpdxV1b allProfiles: mapset.NewSet[string](), userManagedProfiles: maps.SafeMap[string, *v1beta1.ApplicationProfile]{}, } - - return cache } // ------------------ objectcache.ApplicationProfileCache methods ----------------------- @@ -82,14 +80,14 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be baseProfileUniqueName := objectcache.UniqueName(appProfile.GetNamespace(), baseProfileName) // Get the full user managed profile from the storage - fullAP, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) + userManagedProfile, err := ap.getApplicationProfile(appProfile.GetNamespace(), appProfile.GetName()) if err != nil { logger.L().Error("failed to get full application profile", helpers.Error(err)) return } // Store the user-managed profile temporarily - ap.userManagedProfiles.Set(baseProfileUniqueName, fullAP) + ap.userManagedProfiles.Set(baseProfileUniqueName, userManagedProfile) // If we have the base profile cached, fetch a fresh copy and merge. // If the base profile is not cached yet, the merge will be attempted when it's added. @@ -104,7 +102,7 @@ func (ap *ApplicationProfileCacheImpl) handleUserManagedProfile(appProfile *v1be return } - mergedProfile := ap.performMerge(freshBaseProfile, fullAP) + mergedProfile := ap.performMerge(freshBaseProfile, userManagedProfile) ap.slugToAppProfile.Set(baseProfileUniqueName, mergedProfile) // Clean up the user-managed profile after successful merge @@ -120,11 +118,7 @@ func (ap *ApplicationProfileCacheImpl) addApplicationProfile(obj runtime.Object) appProfile := obj.(*v1beta1.ApplicationProfile) apName := objectcache.MetaUniqueName(appProfile) - isUserManaged := appProfile.Annotations != nil && - appProfile.Annotations["kubescape.io/managed-by"] == "User" && - strings.HasPrefix(appProfile.GetName(), "ug-") - - if isUserManaged { + if isUserManagedProfile(appProfile) { ap.handleUserManagedProfile(appProfile) return } @@ -325,31 +319,29 @@ func (ap *ApplicationProfileCacheImpl) performMerge(normalProfile, userManagedPr mergedProfile.Spec.InitContainers = ap.mergeContainers(mergedProfile.Spec.InitContainers, userManagedProfile.Spec.InitContainers) mergedProfile.Spec.EphemeralContainers = ap.mergeContainers(mergedProfile.Spec.EphemeralContainers, userManagedProfile.Spec.EphemeralContainers) - // Remove the user-managed annotation - delete(mergedProfile.Annotations, "kubescape.io/managed-by") - return mergedProfile } func (ap *ApplicationProfileCacheImpl) mergeContainers(normalContainers, userManagedContainers []v1beta1.ApplicationProfileContainer) []v1beta1.ApplicationProfileContainer { - // Create a map to store containers by name - containerMap := make(map[string]int) // map name to index in slice - - // Store indices of normal containers - for i := range normalContainers { - containerMap[normalContainers[i].Name] = i + if len(userManagedContainers) != len(normalContainers) { + // If the number of containers don't match, we can't merge + logger.L().Error("failed to merge user-managed profile with base profile", + helpers.Int("normalContainers len", len(normalContainers)), + helpers.Int("userManagedContainers len", len(userManagedContainers)), + helpers.String("reason", "number of containers don't match")) + return normalContainers } - // Merge or append user containers - for _, userContainer := range userManagedContainers { - if idx, exists := containerMap[userContainer.Name]; exists { - // Directly modify the container in the slice - ap.mergeContainer(&normalContainers[idx], &userContainer) - } else { - normalContainers = append(normalContainers, userContainer) + // Assuming the normalContainers are already in the correct Pod order + // We'll merge user containers at their corresponding positions + for i := range normalContainers { + for _, userContainer := range userManagedContainers { + if normalContainers[i].Name == userContainer.Name { + ap.mergeContainer(&normalContainers[i], &userContainer) + break + } } } - return normalContainers } @@ -365,11 +357,7 @@ func (ap *ApplicationProfileCacheImpl) deleteApplicationProfile(obj runtime.Obje appProfile := obj.(*v1beta1.ApplicationProfile) apName := objectcache.MetaUniqueName(appProfile) - isUserManaged := appProfile.Annotations != nil && - appProfile.Annotations["kubescape.io/managed-by"] == "User" && - strings.HasPrefix(appProfile.GetName(), "ug-") - - if isUserManaged { + if isUserManagedProfile(appProfile) { // For user-managed profiles, we need to use the base name for cleanup baseProfileName := strings.TrimPrefix(appProfile.GetName(), "ug-") baseProfileUniqueName := objectcache.UniqueName(appProfile.GetNamespace(), baseProfileName) @@ -446,3 +434,9 @@ func (ap *ApplicationProfileCacheImpl) cleanupOrphanedUserManagedProfiles() { return true }) } + +func isUserManagedProfile(appProfile *v1beta1.ApplicationProfile) bool { + return appProfile.Annotations != nil && + appProfile.Annotations["kubescape.io/managed-by"] == "User" && + strings.HasPrefix(appProfile.GetName(), "ug-") +} diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go index f8806142..3c1797e5 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache_test.go @@ -742,80 +742,6 @@ func Test_MergeApplicationProfiles(t *testing.T) { userProfile *v1beta1.ApplicationProfile expectedMerged *v1beta1.ApplicationProfile }{ - { - name: "merge profiles with different containers", - normalProfile: &v1beta1.ApplicationProfile{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-profile", - Namespace: "default", - }, - Spec: v1beta1.ApplicationProfileSpec{ - Containers: []v1beta1.ApplicationProfileContainer{ - { - Name: "container1", - Capabilities: []string{ - "NET_ADMIN", - }, - Syscalls: []string{ - "open", - "read", - }, - }, - }, - }, - }, - userProfile: &v1beta1.ApplicationProfile{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ug-test-profile", // Added ug- prefix - Namespace: "default", - Annotations: map[string]string{ - "kubescape.io/managed-by": "User", - }, - }, - Spec: v1beta1.ApplicationProfileSpec{ - Containers: []v1beta1.ApplicationProfileContainer{ - { - Name: "container2", - Capabilities: []string{ - "SYS_ADMIN", - }, - Syscalls: []string{ - "write", - }, - }, - }, - }, - }, - expectedMerged: &v1beta1.ApplicationProfile{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-profile", // Keeps original name without ug- prefix - Namespace: "default", - }, - Spec: v1beta1.ApplicationProfileSpec{ - Containers: []v1beta1.ApplicationProfileContainer{ - { - Name: "container1", - Capabilities: []string{ - "NET_ADMIN", - }, - Syscalls: []string{ - "open", - "read", - }, - }, - { - Name: "container2", - Capabilities: []string{ - "SYS_ADMIN", - }, - Syscalls: []string{ - "write", - }, - }, - }, - }, - }, - }, { name: "merge profiles with overlapping containers", normalProfile: &v1beta1.ApplicationProfile{ diff --git a/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go b/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go index 4d0f16ef..d79e8d92 100644 --- a/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go +++ b/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go @@ -86,14 +86,14 @@ func (nn *NetworkNeighborhoodCacheImpl) handleUserManagedNN(netNeighborhood *v1b baseNNUniqueName := objectcache.UniqueName(netNeighborhood.GetNamespace(), baseNNName) // Get the full user managed network neighborhood from the storage - fullNN, err := nn.getNetworkNeighborhood(netNeighborhood.GetNamespace(), netNeighborhood.GetName()) + userManagedNN, err := nn.getNetworkNeighborhood(netNeighborhood.GetNamespace(), netNeighborhood.GetName()) if err != nil { logger.L().Error("failed to get full network neighborhood", helpers.Error(err)) return } // Store the user-managed network neighborhood temporarily - nn.userManagedNetworkNeighborhood.Set(baseNNUniqueName, fullNN) + nn.userManagedNetworkNeighborhood.Set(baseNNUniqueName, userManagedNN) // If we have the base network neighborhood cached, fetch a fresh copy and merge. // If the base network neighborhood is not cached yet, the merge will be attempted when it's added. @@ -108,7 +108,7 @@ func (nn *NetworkNeighborhoodCacheImpl) handleUserManagedNN(netNeighborhood *v1b return } - mergedNN := nn.performMerge(freshBaseNN, fullNN) + mergedNN := nn.performMerge(freshBaseNN, userManagedNN) nn.slugToNetworkNeighborhood.Set(baseNNUniqueName, mergedNN) // Clean up the user-managed network neighborhood after successful merge @@ -142,31 +142,20 @@ func (nn *NetworkNeighborhoodCacheImpl) performMerge(normalNN, userManagedNN *v1 userManagedNN.Spec.LabelSelector.MatchExpressions..., ) - // Remove the user-managed annotation - delete(mergedNN.Annotations, "kubescape.io/managed-by") - return mergedNN } func (nn *NetworkNeighborhoodCacheImpl) mergeContainers(normalContainers, userManagedContainers []v1beta1.NetworkNeighborhoodContainer) []v1beta1.NetworkNeighborhoodContainer { - // Create a map to store containers by name - containerMap := make(map[string]int) // map name to index in slice - - // Store indices of normal containers + // Assuming the normalContainers are already in the correct Pod order + // We'll merge user containers at their corresponding positions for i := range normalContainers { - containerMap[normalContainers[i].Name] = i - } - - // Merge or append user containers - for _, userContainer := range userManagedContainers { - if idx, exists := containerMap[userContainer.Name]; exists { - // Directly modify the container in the slice - nn.mergeContainer(&normalContainers[idx], &userContainer) - } else { - normalContainers = append(normalContainers, userContainer) + for _, userContainer := range userManagedContainers { + if normalContainers[i].Name == userContainer.Name { + nn.mergeContainer(&normalContainers[i], &userContainer) + break + } } } - return normalContainers } @@ -429,11 +418,7 @@ func (nn *NetworkNeighborhoodCacheImpl) addNetworkNeighborhood(_ context.Context netNeighborhood := obj.(*v1beta1.NetworkNeighborhood) nnName := objectcache.MetaUniqueName(netNeighborhood) - isUserManaged := netNeighborhood.Annotations != nil && - netNeighborhood.Annotations["kubescape.io/managed-by"] == "User" && - strings.HasPrefix(netNeighborhood.GetName(), "ug-") - - if isUserManaged { + if isUserManagedNN(netNeighborhood) { nn.handleUserManagedNN(netNeighborhood) return } @@ -485,11 +470,7 @@ func (nn *NetworkNeighborhoodCacheImpl) deleteNetworkNeighborhood(obj runtime.Ob netNeighborhood := obj.(*v1beta1.NetworkNeighborhood) nnName := objectcache.MetaUniqueName(netNeighborhood) - isUserManaged := netNeighborhood.Annotations != nil && - netNeighborhood.Annotations["kubescape.io/managed-by"] == "User" && - strings.HasPrefix(netNeighborhood.GetName(), "ug-") - - if isUserManaged { + if isUserManagedNN(netNeighborhood) { // For user-managed network neighborhoods, we need to use the base name for cleanup baseNNName := strings.TrimPrefix(netNeighborhood.GetName(), "ug-") baseNNUniqueName := objectcache.UniqueName(netNeighborhood.GetNamespace(), baseNNName) @@ -564,3 +545,9 @@ func getSlug(p *corev1.Pod) (string, error) { } return slug, nil } + +func isUserManagedNN(nn *v1beta1.NetworkNeighborhood) bool { + return nn.Annotations != nil && + nn.Annotations["kubescape.io/managed-by"] == "User" && + strings.HasPrefix(nn.GetName(), "ug-") +} From 0c481fa219dd79d9b2349e6526f7b3822fcd23e3 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Mon, 28 Oct 2024 08:09:42 +0000 Subject: [PATCH 44/50] Raising chances for dns request succeed Signed-off-by: Amit Schendel --- tests/component_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/component_test.go b/tests/component_test.go index aed77074..7b87d7e9 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -1092,6 +1092,12 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + time.Sleep(2 * time.Second) // Wait for alerts + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + time.Sleep(2 * time.Second) // Wait for alerts _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert From 5820596045b40c9b7997665edcac865f9917f376 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Mon, 28 Oct 2024 08:37:15 +0000 Subject: [PATCH 45/50] Adding more time for profile to be patched Signed-off-by: Amit Schendel --- tests/component_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index 7b87d7e9..7d9e7a7c 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -1084,7 +1084,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { _, err = storageClient.NetworkNeighborhoods(ns.Name).Patch(context.Background(), userNN.Name, types.JSONPatchType, patch, metav1.PatchOptions{}) require.NoError(t, err, "Failed to patch user network neighborhood") - time.Sleep(15 * time.Second) // Allow merge to complete + time.Sleep(20 * time.Second) // Allow merge to complete // Test alerts after patch _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert @@ -1097,7 +1097,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) - time.Sleep(2 * time.Second) // Wait for alerts + time.Sleep(2 * time.Second) // Wait for alerts _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert From 3d21f4287c73b67683201b036a7858259f9078b7 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Mon, 28 Oct 2024 08:59:40 +0000 Subject: [PATCH 46/50] Removing memory leak test since it's not working well Signed-off-by: Amit Schendel --- .github/workflows/component-tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/component-tests.yaml b/.github/workflows/component-tests.yaml index 5640d4bb..2c741b92 100644 --- a/.github/workflows/component-tests.yaml +++ b/.github/workflows/component-tests.yaml @@ -43,7 +43,7 @@ jobs: Test_01_BasicAlertTest, Test_02_AllAlertsFromMaliciousApp, Test_03_BasicLoadActivities, - Test_04_MemoryLeak, + # Test_04_MemoryLeak, Test_05_MemoryLeak_10K_Alerts, Test_06_KillProcessInTheMiddle, Test_07_RuleBindingApplyTest, From 5a9c95f4f4f42d347fe67cf3758b0280787ed6ea Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Mon, 28 Oct 2024 08:59:52 +0000 Subject: [PATCH 47/50] Fixed Test13 Signed-off-by: Amit Schendel --- tests/component_test.go | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index 7d9e7a7c..0eb19844 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -1009,7 +1009,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { { Identifier: "server-example", Type: "external", - DNSNames: []string{"httpforever.com."}, + DNSNames: []string{"info.cern.ch."}, Ports: []v1beta1.NetworkPort{ { Name: "TCP-80", @@ -1041,13 +1041,13 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert (original) // Try multiple times to ensure alert is removed - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) - _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original) - _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert (user added) - time.Sleep(30 * time.Second) // Wait for potential alerts + _, _, err = wl.ExecIntoPod([]string{"wget", "info.cern.ch", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) + _, _, err = wl.ExecIntoPod([]string{"wget", "info.cern.ch", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) + _, _, err = wl.ExecIntoPod([]string{"wget", "info.cern.ch", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) + _, _, err = wl.ExecIntoPod([]string{"wget", "info.cern.ch", "-T", "2", "-t", "1"}, "server") // Expected: no alert (user added) + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert (original) + _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert (user added) + time.Sleep(30 * time.Second) // Wait for potential alerts mergedAlerts, err := testutils.GetAlerts(wl.Namespace) require.NoError(t, err, "Failed to get alerts after merge") @@ -1073,7 +1073,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { } // PHASE 5: Remove permission via patch and verify alerts return - t.Log("Patching user network neighborhood to remove httpforever.com from server container...") + t.Log("Patching user network neighborhood to remove info.cern.ch from server container...") patchOperations := []utils.PatchOperation{ {Op: "remove", Path: "/spec/containers/1/egress/0"}, } @@ -1089,19 +1089,14 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { // Test alerts after patch _, _, err = wl.ExecIntoPod([]string{"wget", "ebpf.io", "-T", "2", "-t", "1"}, "server") // Expected: no alert // Try multiple times to ensure alert is removed - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) - time.Sleep(2 * time.Second) // Wait for alerts - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) - time.Sleep(2 * time.Second) // Wait for alerts - _, _, err = wl.ExecIntoPod([]string{"wget", "httpforever.com", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) - _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert - _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert - time.Sleep(30 * time.Second) // Wait for alerts + _, _, err = wl.ExecIntoPod([]string{"wget", "info.cern.ch", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"wget", "info.cern.ch", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"wget", "info.cern.ch", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"wget", "info.cern.ch", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"wget", "info.cern.ch", "-T", "2", "-t", "1"}, "server") // Expected: alert (removed) + _, _, err = wl.ExecIntoPod([]string{"curl", "kubernetes.io", "-m", "2"}, "nginx") // Expected: no alert + _, _, err = wl.ExecIntoPod([]string{"curl", "github.com", "-m", "2"}, "nginx") // Expected: no alert + time.Sleep(30 * time.Second) // Wait for alerts finalAlerts, err := testutils.GetAlerts(wl.Namespace) require.NoError(t, err, "Failed to get final alerts") From 7843e8a8e6066be55b9c1634032b6d8e4c3bff71 Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 29 Oct 2024 09:37:20 +0000 Subject: [PATCH 48/50] Adding a check for matching len Signed-off-by: Amit Schendel --- .../networkneighborhoodcache/networkneighborhoodcache.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go b/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go index d79e8d92..bcef01f7 100644 --- a/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go +++ b/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go @@ -146,6 +146,14 @@ func (nn *NetworkNeighborhoodCacheImpl) performMerge(normalNN, userManagedNN *v1 } func (nn *NetworkNeighborhoodCacheImpl) mergeContainers(normalContainers, userManagedContainers []v1beta1.NetworkNeighborhoodContainer) []v1beta1.NetworkNeighborhoodContainer { + if len(userManagedContainers) != len(normalContainers) { + logger.L().Error("user-managed containers count does not match normal containers count", + helpers.Int("userManagedCount", len(userManagedContainers)), + helpers.Int("normalCount", len(normalContainers)), + helpers.String("reason", "number of containers don't match")) + return normalContainers + } + // Assuming the normalContainers are already in the correct Pod order // We'll merge user containers at their corresponding positions for i := range normalContainers { From 1607fea678cfc8d18fa5cb20d79ed586bf39e7e0 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Tue, 29 Oct 2024 10:45:29 +0100 Subject: [PATCH 49/50] reorg networkneighborhoodcache to match applicationprofilecache Signed-off-by: Matthias Bertschy --- .../applicationprofilecache.go | 1 - .../networkneighborhoodcache.go | 413 +++++++++--------- 2 files changed, 207 insertions(+), 207 deletions(-) diff --git a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go index be2925d0..eae254ef 100644 --- a/pkg/objectcache/applicationprofilecache/applicationprofilecache.go +++ b/pkg/objectcache/applicationprofilecache/applicationprofilecache.go @@ -148,7 +148,6 @@ func (ap *ApplicationProfileCacheImpl) GetApplicationProfile(containerID string) if s := ap.containerToSlug.Get(containerID); s != "" { return ap.slugToAppProfile.Get(s) } - return nil } diff --git a/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go b/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go index bcef01f7..93fc1083 100644 --- a/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go +++ b/pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go @@ -69,18 +69,10 @@ func NewNetworkNeighborhoodCache(nodeName string, storageClient versioned.SpdxV1 allNetworkNeighborhoods: mapset.NewSet[string](), userManagedNetworkNeighborhood: maps.SafeMap[string, *v1beta1.NetworkNeighborhood]{}, } - } // ------------------ objectcache.NetworkNeighborhoodCache methods ----------------------- -func (nn *NetworkNeighborhoodCacheImpl) GetNetworkNeighborhood(containerID string) *v1beta1.NetworkNeighborhood { - if s := nn.containerToSlug.Get(containerID); s != "" { - return nn.slugToNetworkNeighborhood.Get(s) - } - return nil -} - func (nn *NetworkNeighborhoodCacheImpl) handleUserManagedNN(netNeighborhood *v1beta1.NetworkNeighborhood) { baseNNName := strings.TrimPrefix(netNeighborhood.GetName(), "ug-") baseNNUniqueName := objectcache.UniqueName(netNeighborhood.GetNamespace(), baseNNName) @@ -120,171 +112,40 @@ func (nn *NetworkNeighborhoodCacheImpl) handleUserManagedNN(netNeighborhood *v1b } } -func (nn *NetworkNeighborhoodCacheImpl) performMerge(normalNN, userManagedNN *v1beta1.NetworkNeighborhood) *v1beta1.NetworkNeighborhood { - mergedNN := normalNN.DeepCopy() - - // Merge spec containers - mergedNN.Spec.Containers = nn.mergeContainers(mergedNN.Spec.Containers, userManagedNN.Spec.Containers) - mergedNN.Spec.InitContainers = nn.mergeContainers(mergedNN.Spec.InitContainers, userManagedNN.Spec.InitContainers) - mergedNN.Spec.EphemeralContainers = nn.mergeContainers(mergedNN.Spec.EphemeralContainers, userManagedNN.Spec.EphemeralContainers) - - // Merge LabelSelector - if userManagedNN.Spec.LabelSelector.MatchLabels != nil { - if mergedNN.Spec.LabelSelector.MatchLabels == nil { - mergedNN.Spec.LabelSelector.MatchLabels = make(map[string]string) - } - for k, v := range userManagedNN.Spec.LabelSelector.MatchLabels { - mergedNN.Spec.LabelSelector.MatchLabels[k] = v - } - } - mergedNN.Spec.LabelSelector.MatchExpressions = append( - mergedNN.Spec.LabelSelector.MatchExpressions, - userManagedNN.Spec.LabelSelector.MatchExpressions..., - ) - - return mergedNN -} - -func (nn *NetworkNeighborhoodCacheImpl) mergeContainers(normalContainers, userManagedContainers []v1beta1.NetworkNeighborhoodContainer) []v1beta1.NetworkNeighborhoodContainer { - if len(userManagedContainers) != len(normalContainers) { - logger.L().Error("user-managed containers count does not match normal containers count", - helpers.Int("userManagedCount", len(userManagedContainers)), - helpers.Int("normalCount", len(normalContainers)), - helpers.String("reason", "number of containers don't match")) - return normalContainers - } - - // Assuming the normalContainers are already in the correct Pod order - // We'll merge user containers at their corresponding positions - for i := range normalContainers { - for _, userContainer := range userManagedContainers { - if normalContainers[i].Name == userContainer.Name { - nn.mergeContainer(&normalContainers[i], &userContainer) - break - } - } - } - return normalContainers -} - -func (nn *NetworkNeighborhoodCacheImpl) mergeContainer(normalContainer, userContainer *v1beta1.NetworkNeighborhoodContainer) { - // Merge ingress rules - normalContainer.Ingress = nn.mergeNetworkNeighbors(normalContainer.Ingress, userContainer.Ingress) - - // Merge egress rules - normalContainer.Egress = nn.mergeNetworkNeighbors(normalContainer.Egress, userContainer.Egress) -} - -func (nn *NetworkNeighborhoodCacheImpl) mergeNetworkNeighbors(normalNeighbors, userNeighbors []v1beta1.NetworkNeighbor) []v1beta1.NetworkNeighbor { - // Use map to track existing neighbors by identifier - neighborMap := make(map[string]int) - for i, neighbor := range normalNeighbors { - neighborMap[neighbor.Identifier] = i - } - - // Merge or append user neighbors - for _, userNeighbor := range userNeighbors { - if idx, exists := neighborMap[userNeighbor.Identifier]; exists { - // Merge existing neighbor - normalNeighbors[idx] = nn.mergeNetworkNeighbor(normalNeighbors[idx], userNeighbor) - } else { - // Append new neighbor - normalNeighbors = append(normalNeighbors, userNeighbor) - } - } - - return normalNeighbors -} - -func (nn *NetworkNeighborhoodCacheImpl) mergeNetworkNeighbor(normal, user v1beta1.NetworkNeighbor) v1beta1.NetworkNeighbor { - merged := normal.DeepCopy() +func (nn *NetworkNeighborhoodCacheImpl) addNetworkNeighborhood(_ context.Context, obj runtime.Object) { + netNeighborhood := obj.(*v1beta1.NetworkNeighborhood) + nnName := objectcache.MetaUniqueName(netNeighborhood) - // Merge DNS names (removing duplicates) - dnsNamesSet := make(map[string]struct{}) - for _, dns := range normal.DNSNames { - dnsNamesSet[dns] = struct{}{} - } - for _, dns := range user.DNSNames { - dnsNamesSet[dns] = struct{}{} - } - merged.DNSNames = make([]string, 0, len(dnsNamesSet)) - for dns := range dnsNamesSet { - merged.DNSNames = append(merged.DNSNames, dns) + if isUserManagedNN(netNeighborhood) { + nn.handleUserManagedNN(netNeighborhood) + return } - // Merge ports based on patchMergeKey (name) - merged.Ports = nn.mergeNetworkPorts(merged.Ports, user.Ports) - - // Merge pod selector if provided - if user.PodSelector != nil { - if merged.PodSelector == nil { - merged.PodSelector = &metav1.LabelSelector{} - } - if user.PodSelector.MatchLabels != nil { - if merged.PodSelector.MatchLabels == nil { - merged.PodSelector.MatchLabels = make(map[string]string) - } - for k, v := range user.PodSelector.MatchLabels { - merged.PodSelector.MatchLabels[k] = v - } - } - merged.PodSelector.MatchExpressions = append( - merged.PodSelector.MatchExpressions, - user.PodSelector.MatchExpressions..., - ) - } + nnState := newNetworkNeighborhoodState(netNeighborhood) + nn.slugToState.Set(nnName, nnState) - // Merge namespace selector if provided - if user.NamespaceSelector != nil { - if merged.NamespaceSelector == nil { - merged.NamespaceSelector = &metav1.LabelSelector{} - } - if user.NamespaceSelector.MatchLabels != nil { - if merged.NamespaceSelector.MatchLabels == nil { - merged.NamespaceSelector.MatchLabels = make(map[string]string) - } - for k, v := range user.NamespaceSelector.MatchLabels { - merged.NamespaceSelector.MatchLabels[k] = v - } + if nnState.status != helpersv1.Completed { + if nn.slugToNetworkNeighborhood.Has(nnName) { + nn.slugToNetworkNeighborhood.Delete(nnName) + nn.allNetworkNeighborhoods.Remove(nnName) } - merged.NamespaceSelector.MatchExpressions = append( - merged.NamespaceSelector.MatchExpressions, - user.NamespaceSelector.MatchExpressions..., - ) + return } - // Take the user's IP address if provided - if user.IPAddress != "" { - merged.IPAddress = user.IPAddress - } + nn.allNetworkNeighborhoods.Add(nnName) - // Take the user's type if provided - if user.Type != "" { - merged.Type = user.Type + if nn.slugToContainers.Has(nnName) { + time.AfterFunc(utils.RandomDuration(nn.maxDelaySeconds, time.Second), func() { + nn.addFullNetworkNeighborhood(netNeighborhood, nnName) + }) } - - return *merged } -func (nn *NetworkNeighborhoodCacheImpl) mergeNetworkPorts(normalPorts, userPorts []v1beta1.NetworkPort) []v1beta1.NetworkPort { - // Use map to track existing ports by name (patchMergeKey) - portMap := make(map[string]int) - for i, port := range normalPorts { - portMap[port.Name] = i - } - - // Merge or append user ports - for _, userPort := range userPorts { - if idx, exists := portMap[userPort.Name]; exists { - // Update existing port - normalPorts[idx] = userPort - } else { - // Append new port - normalPorts = append(normalPorts, userPort) - } +func (nn *NetworkNeighborhoodCacheImpl) GetNetworkNeighborhood(containerID string) *v1beta1.NetworkNeighborhood { + if s := nn.containerToSlug.Get(containerID); s != "" { + return nn.slugToNetworkNeighborhood.Get(s) } - - return normalPorts + return nil } // ------------------ watcher.Adaptor methods ----------------------- @@ -422,34 +283,6 @@ func (nn *NetworkNeighborhoodCacheImpl) removeContainer(containerID string) { } // ------------------ watch network neighborhood methods ----------------------- -func (nn *NetworkNeighborhoodCacheImpl) addNetworkNeighborhood(_ context.Context, obj runtime.Object) { - netNeighborhood := obj.(*v1beta1.NetworkNeighborhood) - nnName := objectcache.MetaUniqueName(netNeighborhood) - - if isUserManagedNN(netNeighborhood) { - nn.handleUserManagedNN(netNeighborhood) - return - } - - nnState := newNetworkNeighborhoodState(netNeighborhood) - nn.slugToState.Set(nnName, nnState) - - if nnState.status != helpersv1.Completed { - if nn.slugToNetworkNeighborhood.Has(nnName) { - nn.slugToNetworkNeighborhood.Delete(nnName) - nn.allNetworkNeighborhoods.Remove(nnName) - } - return - } - - nn.allNetworkNeighborhoods.Add(nnName) - - if nn.slugToContainers.Has(nnName) { - time.AfterFunc(utils.RandomDuration(nn.maxDelaySeconds, time.Second), func() { - nn.addFullNetworkNeighborhood(netNeighborhood, nnName) - }) - } -} func (nn *NetworkNeighborhoodCacheImpl) addFullNetworkNeighborhood(netNeighborhood *v1beta1.NetworkNeighborhood, nnName string) { fullNN, err := nn.getNetworkNeighborhood(netNeighborhood.GetNamespace(), netNeighborhood.GetName()) @@ -474,6 +307,174 @@ func (nn *NetworkNeighborhoodCacheImpl) addFullNetworkNeighborhood(netNeighborho logger.L().Debug("added pod to network neighborhood cache", helpers.String("name", nnName)) } +func (nn *NetworkNeighborhoodCacheImpl) performMerge(normalNN, userManagedNN *v1beta1.NetworkNeighborhood) *v1beta1.NetworkNeighborhood { + mergedNN := normalNN.DeepCopy() + + // Merge spec containers + mergedNN.Spec.Containers = nn.mergeContainers(mergedNN.Spec.Containers, userManagedNN.Spec.Containers) + mergedNN.Spec.InitContainers = nn.mergeContainers(mergedNN.Spec.InitContainers, userManagedNN.Spec.InitContainers) + mergedNN.Spec.EphemeralContainers = nn.mergeContainers(mergedNN.Spec.EphemeralContainers, userManagedNN.Spec.EphemeralContainers) + + // Merge LabelSelector + if userManagedNN.Spec.LabelSelector.MatchLabels != nil { + if mergedNN.Spec.LabelSelector.MatchLabels == nil { + mergedNN.Spec.LabelSelector.MatchLabels = make(map[string]string) + } + for k, v := range userManagedNN.Spec.LabelSelector.MatchLabels { + mergedNN.Spec.LabelSelector.MatchLabels[k] = v + } + } + mergedNN.Spec.LabelSelector.MatchExpressions = append( + mergedNN.Spec.LabelSelector.MatchExpressions, + userManagedNN.Spec.LabelSelector.MatchExpressions..., + ) + + return mergedNN +} + +func (nn *NetworkNeighborhoodCacheImpl) mergeContainers(normalContainers, userManagedContainers []v1beta1.NetworkNeighborhoodContainer) []v1beta1.NetworkNeighborhoodContainer { + if len(userManagedContainers) != len(normalContainers) { + // If the number of containers don't match, we can't merge + logger.L().Error("failed to merge user-managed profile with base profile", + helpers.Int("normalContainers len", len(normalContainers)), + helpers.Int("userManagedContainers len", len(userManagedContainers)), + helpers.String("reason", "number of containers don't match")) + return normalContainers + } + + // Assuming the normalContainers are already in the correct Pod order + // We'll merge user containers at their corresponding positions + for i := range normalContainers { + for _, userContainer := range userManagedContainers { + if normalContainers[i].Name == userContainer.Name { + nn.mergeContainer(&normalContainers[i], &userContainer) + break + } + } + } + return normalContainers +} + +func (nn *NetworkNeighborhoodCacheImpl) mergeContainer(normalContainer, userContainer *v1beta1.NetworkNeighborhoodContainer) { + // Merge ingress rules + normalContainer.Ingress = nn.mergeNetworkNeighbors(normalContainer.Ingress, userContainer.Ingress) + + // Merge egress rules + normalContainer.Egress = nn.mergeNetworkNeighbors(normalContainer.Egress, userContainer.Egress) +} + +func (nn *NetworkNeighborhoodCacheImpl) mergeNetworkNeighbors(normalNeighbors, userNeighbors []v1beta1.NetworkNeighbor) []v1beta1.NetworkNeighbor { + // Use map to track existing neighbors by identifier + neighborMap := make(map[string]int) + for i, neighbor := range normalNeighbors { + neighborMap[neighbor.Identifier] = i + } + + // Merge or append user neighbors + for _, userNeighbor := range userNeighbors { + if idx, exists := neighborMap[userNeighbor.Identifier]; exists { + // Merge existing neighbor + normalNeighbors[idx] = nn.mergeNetworkNeighbor(normalNeighbors[idx], userNeighbor) + } else { + // Append new neighbor + normalNeighbors = append(normalNeighbors, userNeighbor) + } + } + + return normalNeighbors +} + +func (nn *NetworkNeighborhoodCacheImpl) mergeNetworkNeighbor(normal, user v1beta1.NetworkNeighbor) v1beta1.NetworkNeighbor { + merged := normal.DeepCopy() + + // Merge DNS names (removing duplicates) + dnsNamesSet := make(map[string]struct{}) + for _, dns := range normal.DNSNames { + dnsNamesSet[dns] = struct{}{} + } + for _, dns := range user.DNSNames { + dnsNamesSet[dns] = struct{}{} + } + merged.DNSNames = make([]string, 0, len(dnsNamesSet)) + for dns := range dnsNamesSet { + merged.DNSNames = append(merged.DNSNames, dns) + } + + // Merge ports based on patchMergeKey (name) + merged.Ports = nn.mergeNetworkPorts(merged.Ports, user.Ports) + + // Merge pod selector if provided + if user.PodSelector != nil { + if merged.PodSelector == nil { + merged.PodSelector = &metav1.LabelSelector{} + } + if user.PodSelector.MatchLabels != nil { + if merged.PodSelector.MatchLabels == nil { + merged.PodSelector.MatchLabels = make(map[string]string) + } + for k, v := range user.PodSelector.MatchLabels { + merged.PodSelector.MatchLabels[k] = v + } + } + merged.PodSelector.MatchExpressions = append( + merged.PodSelector.MatchExpressions, + user.PodSelector.MatchExpressions..., + ) + } + + // Merge namespace selector if provided + if user.NamespaceSelector != nil { + if merged.NamespaceSelector == nil { + merged.NamespaceSelector = &metav1.LabelSelector{} + } + if user.NamespaceSelector.MatchLabels != nil { + if merged.NamespaceSelector.MatchLabels == nil { + merged.NamespaceSelector.MatchLabels = make(map[string]string) + } + for k, v := range user.NamespaceSelector.MatchLabels { + merged.NamespaceSelector.MatchLabels[k] = v + } + } + merged.NamespaceSelector.MatchExpressions = append( + merged.NamespaceSelector.MatchExpressions, + user.NamespaceSelector.MatchExpressions..., + ) + } + + // Take the user's IP address if provided + if user.IPAddress != "" { + merged.IPAddress = user.IPAddress + } + + // Take the user's type if provided + if user.Type != "" { + merged.Type = user.Type + } + + return *merged +} + +func (nn *NetworkNeighborhoodCacheImpl) mergeNetworkPorts(normalPorts, userPorts []v1beta1.NetworkPort) []v1beta1.NetworkPort { + // Use map to track existing ports by name (patchMergeKey) + portMap := make(map[string]int) + for i, port := range normalPorts { + portMap[port.Name] = i + } + + // Merge or append user ports + for _, userPort := range userPorts { + if idx, exists := portMap[userPort.Name]; exists { + // Update existing port + normalPorts[idx] = userPort + } else { + // Append new port + normalPorts = append(normalPorts, userPort) + } + } + + return normalPorts +} + func (nn *NetworkNeighborhoodCacheImpl) deleteNetworkNeighborhood(obj runtime.Object) { netNeighborhood := obj.(*v1beta1.NetworkNeighborhood) nnName := objectcache.MetaUniqueName(netNeighborhood) @@ -501,23 +502,6 @@ func (nn *NetworkNeighborhoodCacheImpl) deleteNetworkNeighborhood(obj runtime.Ob nn.cleanupOrphanedUserManagedNNs() } -// Add cleanup method for orphaned user-managed network neighborhoods -func (nn *NetworkNeighborhoodCacheImpl) cleanupOrphanedUserManagedNNs() { - nn.userManagedNetworkNeighborhood.Range(func(key string, value *v1beta1.NetworkNeighborhood) bool { - if nn.slugToNetworkNeighborhood.Has(key) { - // If base network neighborhood exists but merge didn't happen for some reason, - // attempt merge again and cleanup - if baseNN := nn.slugToNetworkNeighborhood.Get(key); baseNN != nil { - mergedNN := nn.performMerge(baseNN, value) - nn.slugToNetworkNeighborhood.Set(key, mergedNN) - nn.userManagedNetworkNeighborhood.Delete(key) - logger.L().Debug("cleaned up orphaned user-managed network neighborhood", helpers.String("name", key)) - } - } - return true - }) -} - func (nn *NetworkNeighborhoodCacheImpl) getNetworkNeighborhood(namespace, name string) (*v1beta1.NetworkNeighborhood, error) { return nn.storageClient.NetworkNeighborhoods(namespace).Get(context.Background(), name, metav1.GetOptions{}) } @@ -554,6 +538,23 @@ func getSlug(p *corev1.Pod) (string, error) { return slug, nil } +// Add cleanup method for orphaned user-managed network neighborhoods +func (nn *NetworkNeighborhoodCacheImpl) cleanupOrphanedUserManagedNNs() { + nn.userManagedNetworkNeighborhood.Range(func(key string, value *v1beta1.NetworkNeighborhood) bool { + if nn.slugToNetworkNeighborhood.Has(key) { + // If base network neighborhood exists but merge didn't happen for some reason, + // attempt merge again and cleanup + if baseNN := nn.slugToNetworkNeighborhood.Get(key); baseNN != nil { + mergedNN := nn.performMerge(baseNN, value) + nn.slugToNetworkNeighborhood.Set(key, mergedNN) + nn.userManagedNetworkNeighborhood.Delete(key) + logger.L().Debug("cleaned up orphaned user-managed network neighborhood", helpers.String("name", key)) + } + } + return true + }) +} + func isUserManagedNN(nn *v1beta1.NetworkNeighborhood) bool { return nn.Annotations != nil && nn.Annotations["kubescape.io/managed-by"] == "User" && From 26b3371b6af4b82ca7115d4dec8007a39064d2be Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Tue, 29 Oct 2024 10:15:59 +0000 Subject: [PATCH 50/50] Fixing test Signed-off-by: Amit Schendel --- tests/component_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/component_test.go b/tests/component_test.go index 0eb19844..839e6490 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -955,7 +955,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { // Record initial alert count initialAlertCount := 0 for _, alert := range initialAlerts { - if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" && alert.Labels["container_name"] == "server" { initialAlertCount++ } } @@ -1055,7 +1055,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { // Count new alerts after merge newAlertCount := 0 for _, alert := range mergedAlerts { - if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" && alert.Labels["container_name"] == "server" { newAlertCount++ } } @@ -1065,7 +1065,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { if newAlertCount > initialAlertCount { t.Logf("Full alert details:") for _, alert := range mergedAlerts { - if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" && alert.Labels["container_name"] == "server" { t.Logf("Alert: %+v", alert) } } @@ -1104,7 +1104,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { // Count final alerts finalAlertCount := 0 for _, alert := range finalAlerts { - if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" && alert.Labels["container_name"] == "server" { finalAlertCount++ } } @@ -1114,7 +1114,7 @@ func Test_13_MergingNetworkNeighborhoodTest(t *testing.T) { if finalAlertCount <= initialAlertCount { t.Logf("Full alert details:") for _, alert := range finalAlerts { - if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" { + if ruleName, ok := alert.Labels["rule_name"]; ok && ruleName == "Unexpected domain request" && alert.Labels["container_name"] == "server" { t.Logf("Alert: %+v", alert) } }