From 38126d16e211a014b19ae1c2d7c96b753b878d1e Mon Sep 17 00:00:00 2001 From: grzesuav Date: Sat, 22 Jan 2022 17:29:12 +0100 Subject: [PATCH] fix(customize): #414 - Use 'UID' as cache key to avoid collisions between objects in different namespaces Closes #414 Signed-off-by: grzesuav --- pkg/controller/common/customize/cache.go | 14 ++++++++++---- pkg/controller/common/customize/manager.go | 4 ++-- pkg/controller/common/customize/manager_test.go | 4 ++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/pkg/controller/common/customize/cache.go b/pkg/controller/common/customize/cache.go index 3ca34778a4..e809fbca37 100644 --- a/pkg/controller/common/customize/cache.go +++ b/pkg/controller/common/customize/cache.go @@ -19,6 +19,8 @@ package customize import ( "time" + "k8s.io/apimachinery/pkg/types" + "zgo.at/zcache" ) @@ -42,17 +44,17 @@ type customizeResponseCacheEntry struct { } // Add adds a given response for given parent and its generation -func (responseCache *ResponseCache) Add(name string, parentGeneration int64, response *CustomizeHookResponse) { +func (responseCache *ResponseCache) Add(uid types.UID, parentGeneration int64, response *CustomizeHookResponse) { responseCacheEntry := customizeResponseCacheEntry{ parentGeneration: parentGeneration, cachedResponse: response, } - responseCache.cache.Set(name, &responseCacheEntry, zcache.DefaultExpiration) + responseCache.cache.Set(toKey(uid), &responseCacheEntry, zcache.DefaultExpiration) } // Get returns response from cache or nil when not found -func (responseCache *ResponseCache) Get(name string, parentGeneration int64) *CustomizeHookResponse { - value, found := responseCache.cache.Get(name) +func (responseCache *ResponseCache) Get(uid types.UID, parentGeneration int64) *CustomizeHookResponse { + value, found := responseCache.cache.Get(toKey(uid)) if !found { return nil } @@ -62,3 +64,7 @@ func (responseCache *ResponseCache) Get(name string, parentGeneration int64) *Cu } return responseCacheEntry.cachedResponse } + +func toKey(uid types.UID) string { + return string(uid) +} diff --git a/pkg/controller/common/customize/manager.go b/pkg/controller/common/customize/manager.go index f523e30781..f2272331c6 100644 --- a/pkg/controller/common/customize/manager.go +++ b/pkg/controller/common/customize/manager.go @@ -116,7 +116,7 @@ func (rm *Manager) Stop() { } func (rm *Manager) getCachedCustomizeHookResponse(parent *unstructured.Unstructured) *CustomizeHookResponse { - return rm.customizeCache.Get(parent.GetName(), parent.GetGeneration()) + return rm.customizeCache.Get(parent.GetUID(), parent.GetGeneration()) } func (rm *Manager) getCustomizeHookResponse(parent *unstructured.Unstructured) (*CustomizeHookResponse, error) { @@ -133,7 +133,7 @@ func (rm *Manager) getCustomizeHookResponse(parent *unstructured.Unstructured) ( return nil, err } - rm.customizeCache.Add(parent.GetName(), parent.GetGeneration(), &response) + rm.customizeCache.Add(parent.GetUID(), parent.GetGeneration(), &response) return &response, nil } } diff --git a/pkg/controller/common/customize/manager_test.go b/pkg/controller/common/customize/manager_test.go index 8c874c98df..e40a10f35f 100644 --- a/pkg/controller/common/customize/manager_test.go +++ b/pkg/controller/common/customize/manager_test.go @@ -90,7 +90,7 @@ func TestGetRelatedObject_requestResponse(t *testing.T) { customizeManagerWithFakeController.customizeHook = NewHookExecutorStub(expectedResponse) parent := &unstructured.Unstructured{} - parent.SetName("othertest") + parent.SetUID("some") parent.SetGeneration(1) response, err := customizeManagerWithFakeController.getCustomizeHookResponse(parent) @@ -103,7 +103,7 @@ func TestGetRelatedObject_requestResponse(t *testing.T) { t.Errorf("Response should be equal to %v, got %v", expectedResponse, response) } - if customizeManagerWithFakeController.customizeCache.Get("othertest", 1) == nil { + if customizeManagerWithFakeController.customizeCache.Get("some", 1) == nil { t.Error("Expected not nil here, response should be cached") } }