fix!: remove list and watch on secrets. Fixes #8534

manifests/cluster-install/argo-server-rbac/argo-server-clusterole.yaml

@@ -18,8 +18,6 @@ rules:
     verbs:
       - get
       - create
-      - list
-      - watch
   - apiGroups:
       - ""
     resources:

manifests/install.yaml

@@ -944,8 +944,6 @@ rules:
   verbs:
   - get
   - create
-  - list
-  - watch
 - apiGroups:
   - ""
   resources:

manifests/namespace-install.yaml

@@ -853,8 +853,6 @@ rules:
   verbs:
   - get
   - create
-  - list
-  - watch
 - apiGroups:
   - ""
   resources:

manifests/namespace-install/argo-server-rbac/argo-server-role.yaml

@@ -18,8 +18,6 @@ rules:
     verbs:
       - get
       - create
-      - list
-      - watch
   - apiGroups:
       - ""
     resources:

manifests/quick-start-minimal.yaml

@@ -882,8 +882,6 @@ rules:
   verbs:
   - get
   - create
-  - list
-  - watch
 - apiGroups:
   - ""
   resources:

manifests/quick-start-mysql.yaml

@@ -882,8 +882,6 @@ rules:
   verbs:
   - get
   - create
-  - list
-  - watch
 - apiGroups:
   - ""
   resources:

manifests/quick-start-postgres.yaml

@@ -882,8 +882,6 @@ rules:
   verbs:
   - get
   - create
-  - list
-  - watch
 - apiGroups:
   - ""
   resources:

server/auth/gatekeeper.go

@@ -318,7 +318,7 @@ func (s *gatekeeper) authorizationForServiceAccount(serviceAccount *corev1.Servi
 	if len(serviceAccount.Secrets) == 0 {
 		return "", fmt.Errorf("expected at least one secret for SSO RBAC service account: %s", serviceAccount.GetName())
 	}
-	secret, err := s.cache.SecretLister.Secrets(serviceAccount.GetNamespace()).Get(serviceAccount.Secrets[0].Name)
+	secret, err := s.cache.GetSecret(serviceAccount.GetNamespace(), serviceAccount.Secrets[0].Name)
 	if err != nil {
 		return "", fmt.Errorf("failed to get service account secret: %w", err)
 	}

server/cache/cache.go

@@ -4,23 +4,57 @@ import (
 	"context"
 	"time"
 
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	v1 "k8s.io/client-go/listers/core/v1"
 )
 
 type ResourceCache struct {
+	ctx         context.Context
+	secretCache *timedCache[string, *corev1.Secret]
+	client      kubernetes.Interface
 	v1.ServiceAccountLister
-	v1.SecretLister
 }
 
-func NewResourceCache(client kubernetes.Interface, ctx context.Context, namespace string) *ResourceCache {
-	informerFactory := informers.NewSharedInformerFactoryWithOptions(client, time.Minute*20, informers.WithNamespace(namespace))
+func NewResourceCacheWithTimeout(client kubernetes.Interface, ctx context.Context, namespace string, timeout time.Duration) *ResourceCache {
+	informerFactory := informers.NewSharedInformerFactoryWithOptions(client, timeout, informers.WithNamespace(namespace))
 	cache := &ResourceCache{
+		ctx:                  ctx,
+		secretCache:          NewTimedCache[string, *corev1.Secret](timeout, 2000),
+		client:               client,
 		ServiceAccountLister: informerFactory.Core().V1().ServiceAccounts().Lister(),
-		SecretLister:         informerFactory.Core().V1().Secrets().Lister(),
 	}
 	informerFactory.Start(ctx.Done())
 	informerFactory.WaitForCacheSync(ctx.Done())
 	return cache
 }
+
+func NewResourceCache(client kubernetes.Interface, ctx context.Context, namespace string) *ResourceCache {
+	return NewResourceCacheWithTimeout(client, ctx, namespace, time.Minute*20)
+}
+
+func (c *ResourceCache) GetSecret(namespace string, secretName string) (*corev1.Secret, error) {
+	cacheKey := c.getSecretCacheKey(namespace, secretName)
+	if secret, ok := c.secretCache.Get(cacheKey); ok {
+		return secret, nil
+	}
+
+	secret, err := c.getSecretFromServer(namespace, secretName)
+	if err != nil {
+		return nil, err
+	}
+
+	c.secretCache.Add(cacheKey, secret)
+	return secret, nil
+}
+
+func (c *ResourceCache) getSecretFromServer(namespace string, secretName string) (*corev1.Secret, error) {
+	return c.client.CoreV1().Secrets(namespace).Get(c.ctx, secretName, metav1.GetOptions{})
+}
+
+func (c *ResourceCache) getSecretCacheKey(namespace string, secretName string) string {
+	return namespace + ":secret:" + secretName
+}

server/cache/cache_test.go

@@ -87,7 +87,7 @@ func TestServer_K8sUtilsCache(t *testing.T) {
 		assert.Equal(t, 1, len(sa))
 		assert.True(t, checkServiceAccountExists(sa, "sa3"))
 
-		secrets, _ := cache.SecretLister.Secrets("ns1").List(labels.Everything())
-		assert.Equal(t, 1, len(secrets))
+		secret, _ := cache.GetSecret("ns1", "s1")
+		assert.NotNil(t, secret)
 	})
 }

server/cache/timed_cache.go

@@ -0,0 +1,49 @@
+package cache
+
+import (
+	"time"
+
+	"k8s.io/utils/lru"
+)
+
+type timedCache[Key comparable, Value any] struct {
+	timeout time.Duration
+	cache   *lru.Cache
+}
+
+type timeValueHolder struct {
+	createTime time.Time
+	value      interface{}
+}
+
+func NewTimedCache[key comparable, value any](timeout time.Duration, size int) *timedCache[key, value] {
+	return &timedCache[key, value]{
+		timeout: timeout,
+		cache:   lru.New(size),
+	}
+}
+
+func (c *timedCache[Key, Value]) Get(key Key) (Value, bool) {
+	if data, ok := c.cache.Get(key); ok {
+		holder := data.(*timeValueHolder)
+		deadline := holder.createTime.Add(c.timeout)
+		if c.getCurrentTime().Before(deadline) {
+			if value, ok := holder.value.(Value); ok {
+				return value, true
+			}
+		}
+		c.cache.Remove(key)
+	}
+	return *new(Value), false
+}
+
+func (c *timedCache[Key, Value]) Add(key Key, value Value) {
+	c.cache.Add(key, &timeValueHolder{
+		createTime: c.getCurrentTime(),
+		value:      value,
+	})
+}
+
+func (c *timedCache[Key, Value]) getCurrentTime() time.Time {
+	return time.Now().UTC()
+}

server/cache/timed_cache_test.go

@@ -0,0 +1,54 @@
+package cache
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewTimedCache(t *testing.T) {
+
+	t.Run("NewTimedCache should return a new instance", func(t *testing.T) {
+		cache := NewTimedCache[string, string](time.Second, 1)
+		assert.NotNil(t, cache)
+	})
+
+	t.Run("TimedCache should cache based on LRU size", func(t *testing.T) {
+		cache := NewTimedCache[string, string](time.Second*10, 2)
+		cache.Add("one", "one")
+		cache.Add("two", "two")
+
+		// Both "one" and "two" should be available since maxSize is 2
+		_, ok := cache.Get("one")
+		assert.True(t, ok)
+
+		_, ok = cache.Get("two")
+		assert.True(t, ok)
+
+		// "three" should be available since its newly added
+		cache.Add("three", "three")
+		_, ok = cache.Get("three")
+		assert.True(t, ok)
+
+		// "one" should not be available since maxSize is 2
+		_, ok = cache.Get("one")
+		assert.False(t, ok)
+	})
+
+	t.Run("TimedCache should cache based on timeout", func(t *testing.T) {
+		cache := NewTimedCache[string, string](time.Millisecond*5, 2)
+		cache.Add("one", "one")
+
+		_, ok := cache.Get("one")
+		assert.True(t, ok)
+
+		time.Sleep(time.Millisecond * 10)
+
+		// "one" should not be available since timeout is 5 ms
+		_, ok = cache.Get("one")
+		assert.False(t, ok)
+
+	})
+
+}