fix: Clear related caches on clusters update

This fixes an issue where the clustersNamespaces and the userNamespaces
caches were not updated when Clusters were deleted.

This was most noticeable for clustersNamespaces since the GetAll there
would return everything in the cache, dated or not.

For the userNamespaces cache this was hidden, since the GetAll in this
case would receive an up to date list of the cached clusters and then
use that to look up individual keys, returning them in a collated list.
This meant it _looked like_ the cache was updated, but it was only
showing us what we wanted to see; the cache was still quietly building
up.

The solution here is to generate a Hash based on an ordered list of the
names of current clusters. When updating the clustersNamespaces cache, we
compare the saved with the current hash, and clear the clustersNamespaces if
they differ.
We also clear the userNamespaces at this point; since the userNamespaces
cache is dependent on both the clusters and the clustersNamespaces
cache, it is simpler to clear everything together.

The `userNsList` fun has been pulled apart for testing purposes, but is
otherwise the same. The testing that the userNamespaces cache has been
cleared was harder, since, as I said above the GetAll does not _really_
get all, but "gets all based on this cluster list". I can expose new
methods and change this into a genuine List if that is preferred, but
have not done that for now.

Author: Callisto13

core/clustersmngr/clustersmngrfakes/fake_clients_factory.go

@@ -33,10 +33,14 @@ type ClientsFactory interface {
 	UpdateClusters(ctx context.Context) error
 	// UpdateNamespaces updates the namespaces all namespaces for all clusters
 	UpdateNamespaces(ctx context.Context) error
+	// UpdateUsers updates the namespaces all namespaces for all clusters
+	UpdateUsers(ctx context.Context, user *auth.UserPrincipal)
 	// GetServerClient returns the cluster client with gitops server permissions
 	GetServerClient(ctx context.Context) (Client, error)
 	// GetClustersNamespaces returns the namespaces for all clusters
 	GetClustersNamespaces() map[string][]v1.Namespace
+	// GetClustersNamespaces returns the namespaces for all clusters
+	GetUsersNamespaces(user *auth.UserPrincipal) map[string][]v1.Namespace
 	// Start starts go routines to keep clusters and namespaces lists up to date
 	Start(ctx context.Context)
 }
@@ -48,6 +52,8 @@ type clientsFactory struct {
 
 	// list of clusters returned by the clusters fetcher
 	clusters *Clusters
+	// string containing ordered list of cluster names, used to refresh dependent caches
+	clustersHash string
 	// the lists of all namespaces of each cluster
 	clustersNamespaces *ClustersNamespaces
 	// lists of namespaces accessible by the user on every cluster
@@ -124,6 +130,8 @@ func (cf *clientsFactory) UpdateNamespaces(ctx context.Context) error {
 		return err
 	}
 
+	cf.syncCaches()
+
 	wg := sync.WaitGroup{}
 
 	for clusterName, c := range clients {
@@ -147,6 +155,20 @@ func (cf *clientsFactory) UpdateNamespaces(ctx context.Context) error {
 	return nil
 }
 
+func (cf *clientsFactory) GetClustersNamespaces() map[string][]v1.Namespace {
+	return cf.clustersNamespaces.namespaces
+}
+
+func (cf *clientsFactory) syncCaches() {
+	newHash := cf.clusters.Hash(cf.clusters.Get())
+
+	if newHash != cf.clustersHash {
+		cf.clustersNamespaces.Clear()
+		cf.usersNamespaces.Clear()
+		cf.clustersHash = newHash
+	}
+}
+
 func (cf *clientsFactory) GetImpersonatedClient(ctx context.Context, user *auth.UserPrincipal) (Client, error) {
 	pool := NewClustersClientsPool()
 
@@ -171,26 +193,7 @@ func (cf *clientsFactory) GetServerClient(ctx context.Context) (Client, error) {
 	return NewClient(pool, cf.clustersNamespaces.namespaces), nil
 }
 
-func (cf *clientsFactory) GetClustersNamespaces() map[string][]v1.Namespace {
-	return cf.clustersNamespaces.namespaces
-}
-
-func restConfigFromCluster(cluster Cluster) *rest.Config {
-	return &rest.Config{
-		Host:            cluster.Server,
-		BearerToken:     cluster.BearerToken,
-		TLSClientConfig: cluster.TLSConfig,
-		QPS:             ClientQPS,
-		Burst:           ClientBurst,
-	}
-}
-
-func (cf *clientsFactory) userNsList(ctx context.Context, user *auth.UserPrincipal) map[string][]v1.Namespace {
-	userNamespaces := cf.usersNamespaces.GetAll(user, cf.clusters.Get())
-	if len(userNamespaces) > 0 {
-		return userNamespaces
-	}
-
+func (cf *clientsFactory) UpdateUsers(ctx context.Context, user *auth.UserPrincipal) {
 	wg := sync.WaitGroup{}
 
 	for _, cluster := range cf.clusters.Get() {
@@ -211,10 +214,23 @@ func (cf *clientsFactory) userNsList(ctx context.Context, user *auth.UserPrincip
 	}
 
 	wg.Wait()
+}
 
+func (cf *clientsFactory) GetUsersNamespaces(user *auth.UserPrincipal) map[string][]v1.Namespace {
 	return cf.usersNamespaces.GetAll(user, cf.clusters.Get())
 }
 
+func (cf *clientsFactory) userNsList(ctx context.Context, user *auth.UserPrincipal) map[string][]v1.Namespace {
+	userNamespaces := cf.GetUsersNamespaces(user)
+	if len(userNamespaces) > 0 {
+		return userNamespaces
+	}
+
+	cf.UpdateUsers(ctx, user)
+
+	return cf.GetUsersNamespaces(user)
+}
+
 func impersonatedConfig(cluster Cluster, user *auth.UserPrincipal) *rest.Config {
 	shallowCopy := *restConfigFromCluster(cluster)
 
@@ -240,3 +256,13 @@ func clientsForClusters(clusters []Cluster) (map[string]client.Client, error) {
 
 	return clients, nil
 }
+
+func restConfigFromCluster(cluster Cluster) *rest.Config {
+	return &rest.Config{
+		Host:            cluster.Server,
+		BearerToken:     cluster.BearerToken,
+		TLSClientConfig: cluster.TLSConfig,
+		QPS:             ClientQPS,
+		Burst:           ClientBurst,
+	}
+}

core/clustersmngr/factory.go

@@ -2,6 +2,8 @@ package clustersmngr
 
 import (
 	"fmt"
+	"sort"
+	"strings"
 	"sync"
 
 	"github.com/cheshir/ttlcache"
@@ -28,6 +30,18 @@ func (c *Clusters) Get() []Cluster {
 	return c.clusters
 }
 
+func (c *Clusters) Hash(clusters []Cluster) string {
+	names := []string{}
+
+	for _, cluster := range clusters {
+		names = append(names, cluster.Name)
+	}
+
+	sort.Strings(names)
+
+	return strings.Join(names, "")
+}
+
 type ClustersNamespaces struct {
 	sync.RWMutex
 	namespaces map[string][]v1.Namespace
@@ -44,6 +58,13 @@ func (cn *ClustersNamespaces) Set(cluster string, namespaces []v1.Namespace) {
 	cn.namespaces[cluster] = namespaces
 }
 
+func (cn *ClustersNamespaces) Clear() {
+	cn.Lock()
+	defer cn.Unlock()
+
+	cn.namespaces = make(map[string][]v1.Namespace)
+}
+
 func (cn *ClustersNamespaces) Get(cluster string) []v1.Namespace {
 	cn.Lock()
 	defer cn.Unlock()
@@ -67,6 +88,8 @@ func (un *UsersNamespaces) Set(user *auth.UserPrincipal, cluster string, nsList
 	un.Cache.Set(un.cacheKey(user, cluster), nsList, userNamespaceTTL)
 }
 
+// GetAll will return all namespace mappings based on the list of clusters provided.
+// The cache very well may contain more, but this List is targeted.
 func (un *UsersNamespaces) GetAll(user *auth.UserPrincipal, clusters []Cluster) map[string][]v1.Namespace {
 	namespaces := map[string][]v1.Namespace{}
 
@@ -79,6 +102,10 @@ func (un *UsersNamespaces) GetAll(user *auth.UserPrincipal, clusters []Cluster)
 	return namespaces
 }
 
+func (un *UsersNamespaces) Clear() {
+	un.Cache.Clear()
+}
+
 func (u UsersNamespaces) cacheKey(user *auth.UserPrincipal, cluster string) uint64 {
 	return ttlcache.StringKey(fmt.Sprintf("%s:%s", user.ID, cluster))
 }

core/clustersmngr/factory_caches.go

@@ -71,4 +71,8 @@ func TestClustersNamespaces(t *testing.T) {
 	cs.Set(clusterName, []v1.Namespace{ns})
 
 	g.Expect(cs.Get(clusterName)).To(Equal([]v1.Namespace{ns}))
+
+	cs.Clear()
+
+	g.Expect(cs.Get(clusterName)).To(HaveLen(0))
 }