diff --git a/admission/rulebinding/cache/cache_test.go b/admission/rulebinding/cache/cache_test.go index b1fee7b..944659b 100644 --- a/admission/rulebinding/cache/cache_test.go +++ b/admission/rulebinding/cache/cache_test.go @@ -2,6 +2,7 @@ package cache import ( "context" + "sync" "testing" "github.com/goradd/maps" @@ -45,6 +46,66 @@ func TestNewCache(t *testing.T) { } } +func TestCacheConcurrentAccess(t *testing.T) { + for _, newCache := range []struct { + name string + new func() *RBCache + }{ + {name: "constructor", new: func() *RBCache { + return NewCache(nil, &rules.RuleCreatorMock{}, false) + }}, + {name: "partial literal", new: func() *RBCache { + return &RBCache{ruleCreator: &rules.RuleCreatorMock{}} + }}, + } { + t.Run(newCache.name, func(t *testing.T) { + c := newCache.new() + binding := &typesv1.RuntimeAlertRuleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: "binding", Namespace: "test"}, + Spec: typesv1.RuntimeAlertRuleBindingSpec{ + Rules: []typesv1.RuntimeAlertRuleBindingRule{{RuleID: "R2000"}}, + }, + } + object := &unstructured.Unstructured{} + object.SetNamespace("test") + ctx := t.Context() + start := make(chan struct{}) + var wg sync.WaitGroup + for _, operation := range []func(){ + func() { c.addRuleBinding(binding) }, + func() { c.deleteRuleBinding(uniqueName(binding)) }, + func() { c.ListRulesForObject(ctx, object) }, + c.RefreshRules, + } { + wg.Go(func() { + <-start + for range 64 { + operation() + } + }) + } + close(start) + wg.Wait() + + // Assert final behavior after the concurrent operations have finished. + c.addRuleBinding(binding) + beforeRefresh := c.ListRulesForObject(ctx, object) + if !assert.Len(t, beforeRefresh, 1) { + return + } + assert.Equal(t, "R2000", beforeRefresh[0].ID()) + c.RefreshRules() + afterRefresh := c.ListRulesForObject(ctx, object) + if assert.Len(t, afterRefresh, 1) { + assert.Equal(t, "R2000", afterRefresh[0].ID()) + assert.NotSame(t, beforeRefresh[0], afterRefresh[0], "refresh should recreate the rule") + } + c.deleteRuleBinding(uniqueName(binding)) + assert.Empty(t, c.ListRulesForObject(ctx, object)) + }) + } +} + func TestRuntimeObjAddHandler(t *testing.T) { type rules struct { ruleID string diff --git a/go.mod b/go.mod index bab1d72..78edaa6 100644 --- a/go.mod +++ b/go.mod @@ -361,3 +361,7 @@ replace github.com/project-copacetic/copacetic => github.com/anubhav06/copacetic // runtime-spec v1.3.0 changed LinuxPids.Limit from int64 to *int64, which breaks // containerd v1.7.32 under Go 1.25.8+. replace github.com/opencontainers/runtime-spec => github.com/opencontainers/runtime-spec v1.2.1 + +// Temporary concurrency fix: https://github.com/goradd/maps/pull/22 +// Remove this replacement when an upstream release includes the fix. +replace github.com/goradd/maps => github.com/matthyx/maps v0.0.0-20260915111345-e9181ad40421 diff --git a/go.sum b/go.sum index 57bcec8..c37f972 100644 --- a/go.sum +++ b/go.sum @@ -145,8 +145,6 @@ github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmV github.com/armon/go-metrics v0.3.10/go.mod h1:4O98XIr/9W0sxpJ8UaYkvjk10Iff7SnFrb4QAOwNTFc= github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= -github.com/armosec/armoapi-go v0.0.720 h1:mtxUw2wWPRSQWcUf89Eoc9J81SBIC0YaK66XqAXuhCQ= -github.com/armosec/armoapi-go v0.0.720/go.mod h1:9jAH0g8ZsryhiBDd/aNMX4+n10bGwTx/doWCyyjSxts= github.com/armosec/armoapi-go v0.0.761 h1:/idEh/lGFLGUIF64/ecuusCYHPzs7F6T/VQ9zFHEaxA= github.com/armosec/armoapi-go v0.0.761/go.mod h1:1l+70fBK09F7zI2jArrPUWVHaLkijg+sQutFTmE6HRs= github.com/armosec/gojay v1.2.17 h1:VSkLBQzD1c2V+FMtlGFKqWXNsdNvIKygTKJI9ysY8eM= @@ -587,8 +585,6 @@ github.com/gookit/color v1.2.5/go.mod h1:AhIE+pS6D4Ql0SQWbBeXPHw7gY0/sjHoA4s/n1K github.com/gookit/color v1.6.0 h1:JjJXBTk1ETNyqyilJhkTXJYYigHG24TM9Xa2M1xAhRA= github.com/gookit/color v1.6.0/go.mod h1:9ACFc7/1IpHGBW8RwuDm/0YEnhg3dwwXpoMsmtyHfjs= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= -github.com/goradd/maps v1.3.0 h1:toF7ALsgbjQBmmmRSACTAEO+9g2rApW8dU1WirFQyrE= -github.com/goradd/maps v1.3.0/go.mod h1:O3i5k17BAjHa9h5dzGWWfRJizF03umiBDZsNSqFdbVA= github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4= @@ -740,6 +736,8 @@ github.com/mailru/easyjson v0.9.0 h1:PrnmzHw7262yW8sTBwxi1PdJA3Iw/EKBa8psRf7d9a4 github.com/mailru/easyjson v0.9.0/go.mod h1:1+xMtQp2MRNVL/V1bOzuP3aP8VNwRW55fQUto+XFtTU= github.com/matthyx/inspektor-gadget v0.0.0-20260203101533-6ef87216d3dd h1:n8zR1L5t5UWzmQ/DgQ98DF/NrYJL7gUI57GkiDlyu9Y= github.com/matthyx/inspektor-gadget v0.0.0-20260203101533-6ef87216d3dd/go.mod h1:V4TgEmWo37K72pQvC7XuRQssysrxIIkrNX4TtEkgiE0= +github.com/matthyx/maps v0.0.0-20260915111345-e9181ad40421 h1:9XVp2iZdJ7AVWvlNb7e9AOgGzXUyZmX79GzNJDNOCLM= +github.com/matthyx/maps v0.0.0-20260915111345-e9181ad40421/go.mod h1:O3i5k17BAjHa9h5dzGWWfRJizF03umiBDZsNSqFdbVA= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= diff --git a/watcher/sbomwatcher_test.go b/watcher/sbomwatcher_test.go index 246b364..848d9e2 100644 --- a/watcher/sbomwatcher_test.go +++ b/watcher/sbomwatcher_test.go @@ -299,7 +299,15 @@ func TestHandleSBOMEvents_WlidArrivesLate(t *testing.T) { cmdCh := make(chan *apis.Command, 4) errorCh := make(chan error, 4) - go wh.HandleSBOMEvents(eventQueue, cmdCh, errorCh) + handlerDone := make(chan struct{}) + go func() { + defer close(handlerDone) + wh.HandleSBOMEvents(eventQueue, cmdCh, errorCh) + }() + t.Cleanup(func() { + eventQueue.Stop() + <-handlerDone + }) // Enqueue the SBOM while ImageToContainerData is empty. eventQueue.Enqueue(watch.Event{Type: watch.Added, Object: sbom}) @@ -332,9 +340,8 @@ func TestHandleSBOMEvents_WlidArrivesLate(t *testing.T) { // Bookkeeping should be cleared after success. key := sbom.Namespace + "/" + sbom.Name - assert.Equal(t, 0, wh.sbomRetryAttempts.Get(key), "retry counter must be cleared on success") - - eventQueue.Stop() + _, exists := wh.sbomRetryAttempts.Load(key) + assert.False(t, exists, "retry counter must be cleared on success") } // TestHandleSBOMEvents_WlidNeverArrives_ExhaustsRetries verifies the @@ -381,7 +388,17 @@ func TestHandleSBOMEvents_WlidNeverArrives_ExhaustsRetries(t *testing.T) { } }() - go wh.HandleSBOMEvents(eventQueue, cmdCh, errorCh) + handlerDone := make(chan struct{}) + go func() { + defer close(handlerDone) + wh.HandleSBOMEvents(eventQueue, cmdCh, errorCh) + }() + t.Cleanup(func() { + eventQueue.Stop() + <-handlerDone + close(cmdCh) + <-cmdDone + }) eventQueue.Enqueue(watch.Event{Type: watch.Added, Object: sbom}) @@ -400,11 +417,6 @@ func TestHandleSBOMEvents_WlidNeverArrives_ExhaustsRetries(t *testing.T) { // Bookkeeping must be cleared on exhaustion to avoid leaking memory if the // SBOM is later re-observed. key := sbom.Namespace + "/" + sbom.Name - assert.Equal(t, 0, wh.sbomRetryAttempts.Get(key), "retry counter must be cleared on exhaustion") - - eventQueue.Stop() - // HandleSBOMEvents closes cmdCh implicitly? No - it only closes errorCh. - // Close cmdCh manually so the drain goroutine exits, then wait. - close(cmdCh) - <-cmdDone + _, exists := wh.sbomRetryAttempts.Load(key) + assert.False(t, exists, "retry counter must be cleared on exhaustion") }