fix: invalidate workload identity cache when SveltosCluster spec changes

lib/clusterproxy/export_test.go

@@ -20,6 +20,8 @@ import (
 	"time"
 
 	"k8s.io/client-go/rest"
+
+	libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1"
 )
 
 const (
@@ -32,12 +34,20 @@ var (
 	IsSveltosClusterInPullMode = isSveltosClusterInPullMode
 )
 
-// StoreTestWiCache inserts a pre-built entry into the workload identity cache.
+// StoreTestWiCache inserts a pre-built entry into the workload identity cache, hashed from wi
+// so it is only treated as valid for that exact WorkloadIdentityConfig.
 // For use in tests only.
-func StoreTestWiCache(namespace, name string, cfg *rest.Config, expiresAt time.Time) {
-	wiCache.Store(wiCacheKey(namespace, name), cachedRestConfig{config: cfg, expiresAt: expiresAt})
+func StoreTestWiCache(namespace, name string, cfg *rest.Config, expiresAt time.Time,
+	wi *libsveltosv1beta1.WorkloadIdentityConfig) {
+
+	specHash, _ := workloadIdentityConfigHash(wi)
+	wiCache.Store(wiCacheKey(namespace, name), cachedRestConfig{config: cfg, expiresAt: expiresAt, specHash: specHash})
 }
 
+// WorkloadIdentityConfigHashForTest calls the internal workloadIdentityConfigHash function.
+// For use in tests only.
+var WorkloadIdentityConfigHashForTest = workloadIdentityConfigHash
+
 // LoadTestWiCache returns the cached rest.Config and expiry for the given cluster.
 // For use in tests only.
 func LoadTestWiCache(namespace, name string) (*rest.Config, time.Time, bool) {

lib/clusterproxy/workload_identity.go

@@ -18,7 +18,9 @@ package clusterproxy
 
 import (
 	"context"
+	"crypto/sha256"
 	"encoding/base64"
+	"encoding/json"
 	"fmt"
 	"net/http"
 	"os"
@@ -66,6 +68,10 @@ const (
 type cachedRestConfig struct {
 	config    *rest.Config
 	expiresAt time.Time
+	// specHash is the hash of the WorkloadIdentityConfig this entry was built from.
+	// A mismatch means spec.workloadIdentity changed since this entry was cached, so
+	// it must not be reused even though it has not expired yet.
+	specHash string
 }
 
 var (
@@ -84,6 +90,20 @@ func wiCacheKey(namespace, name string) string {
 	return namespace + "/" + name
 }
 
+// workloadIdentityConfigHash returns a stable hash of a WorkloadIdentityConfig, used to
+// detect when spec.workloadIdentity changes (provider, endpoint, cloud-specific fields,
+// or which CA Secret is referenced) so a stale cached rest.Config is not reused.
+// It does not detect the referenced CA Secret's content changing while keeping the same
+// name; that is a narrower, separate problem left for a follow-up.
+func workloadIdentityConfigHash(wi *libsveltosv1beta1.WorkloadIdentityConfig) (string, error) {
+	data, err := json.Marshal(wi)
+	if err != nil {
+		return "", err
+	}
+	hash := sha256.Sum256(data)
+	return fmt.Sprintf("%x", hash), nil
+}
+
 // getWorkloadIdentityRestConfig returns a rest.Config for a SveltosCluster that
 // uses cloud provider workload identity. Results are cached and proactively
 // refreshed when they approach expiry; concurrent calls for the same cluster are
@@ -98,10 +118,15 @@ func getWorkloadIdentityRestConfig(
 
 	key := wiCacheKey(clusterNamespace, clusterName)
 
-	// Fast path: valid cached entry.
+	specHash, err := workloadIdentityConfigHash(wi)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to hash workload identity config")
+	}
+
+	// Fast path: valid cached entry for the current spec.
 	if v, ok := wiCache.Load(key); ok {
 		entry := v.(cachedRestConfig)
-		if time.Until(entry.expiresAt) > wiRefreshThreshold {
+		if entry.specHash == specHash && time.Until(entry.expiresAt) > wiRefreshThreshold {
 			return entry.config, nil
 		}
 	}
@@ -133,7 +158,7 @@ func getWorkloadIdentityRestConfig(
 			return nil, err
 		}
 
-		wiCache.Store(key, cachedRestConfig{config: cfg, expiresAt: expiresAt})
+		wiCache.Store(key, cachedRestConfig{config: cfg, expiresAt: expiresAt, specHash: specHash})
 		return result{cfg: cfg}, nil
 	})
 	if err != nil {

lib/clusterproxy/workload_identity_test.go

@@ -35,6 +35,10 @@ import (
 const (
 	wiTestCachedEndpoint = "https://cached.example.com"
 	wiTestCASecretName   = "my-ca"
+	wiTestEndpoint       = "https://example.com"
+	wiTestGCPProjectID   = "proj"
+	wiTestGCPCluster     = "cluster"
+	wiTestGCPLocation    = "us-central1"
 )
 
 var _ = Describe("WorkloadIdentity cache", func() {
@@ -48,8 +52,12 @@ var _ = Describe("WorkloadIdentity cache", func() {
 	})
 
 	It("EvictWorkloadIdentityCache removes the entry", func() {
-		cfg := &rest.Config{Host: "https://example.com"}
-		clusterproxy.StoreTestWiCache(ns, name, cfg, time.Now().Add(time.Hour))
+		cfg := &rest.Config{Host: wiTestEndpoint}
+		wi := &libsveltosv1beta1.WorkloadIdentityConfig{
+			Provider: libsveltosv1beta1.WorkloadIdentityProviderGCP,
+			Endpoint: wiTestEndpoint,
+		}
+		clusterproxy.StoreTestWiCache(ns, name, cfg, time.Now().Add(time.Hour), wi)
 
 		_, _, ok := clusterproxy.LoadTestWiCache(ns, name)
 		Expect(ok).To(BeTrue())
@@ -61,18 +69,19 @@ var _ = Describe("WorkloadIdentity cache", func() {
 	})
 
 	It("GetSveltosKubernetesRestConfig returns cached config when not near expiry", func() {
-		cached := &rest.Config{Host: wiTestCachedEndpoint}
-		clusterproxy.StoreTestWiCache(ns, name, cached, time.Now().Add(time.Hour))
-
 		wi := &libsveltosv1beta1.WorkloadIdentityConfig{
 			Provider: libsveltosv1beta1.WorkloadIdentityProviderGCP,
 			Endpoint: wiTestCachedEndpoint,
 			GCP: &libsveltosv1beta1.GCPWorkloadIdentityConfig{
-				ProjectID:   "proj",
-				ClusterName: "cluster",
-				Location:    "us-central1",
+				ProjectID:   wiTestGCPProjectID,
+				ClusterName: wiTestGCPCluster,
+				Location:    wiTestGCPLocation,
 			},
 		}
+
+		cached := &rest.Config{Host: wiTestCachedEndpoint}
+		clusterproxy.StoreTestWiCache(ns, name, cached, time.Now().Add(time.Hour), wi)
+
 		sveltosCluster := &libsveltosv1beta1.SveltosCluster{
 			ObjectMeta: metav1.ObjectMeta{
 				Namespace: ns,
@@ -90,6 +99,72 @@ var _ = Describe("WorkloadIdentity cache", func() {
 		Expect(err).To(BeNil())
 		Expect(got).To(Equal(cached))
 	})
+
+	It("GetSveltosKubernetesRestConfig does not reuse a cached config once workloadIdentity spec changes", func() {
+		oldWi := &libsveltosv1beta1.WorkloadIdentityConfig{
+			Provider: libsveltosv1beta1.WorkloadIdentityProviderGCP,
+			Endpoint: wiTestCachedEndpoint,
+			GCP: &libsveltosv1beta1.GCPWorkloadIdentityConfig{
+				ProjectID:   wiTestGCPProjectID,
+				ClusterName: wiTestGCPCluster,
+				Location:    wiTestGCPLocation,
+			},
+		}
+		cached := &rest.Config{Host: wiTestCachedEndpoint}
+		clusterproxy.StoreTestWiCache(ns, name, cached, time.Now().Add(time.Hour), oldWi)
+
+		// Same cluster, but the CA secret reference was added/changed: this must be treated
+		// as a different spec, not the one the cached entry was built from.
+		newWi := oldWi.DeepCopy()
+		newWi.CASecretRef = &corev1.LocalObjectReference{Name: wiTestCASecretName}
+
+		sveltosCluster := &libsveltosv1beta1.SveltosCluster{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: ns,
+				Name:      name,
+			},
+			Spec: libsveltosv1beta1.SveltosClusterSpec{
+				WorkloadIdentity: newWi,
+			},
+		}
+
+		c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(sveltosCluster).Build()
+		logger := logr.Discard()
+
+		// The stale cached config must not be returned. Without real cloud credentials in this
+		// test environment the slow path is expected to fail obtaining a token, which is itself
+		// proof the fast path was correctly bypassed.
+		got, err := clusterproxy.GetSveltosKubernetesRestConfig(ctx, logger, c, ns, name)
+		Expect(err).ToNot(BeNil())
+		Expect(got).ToNot(Equal(cached))
+	})
+})
+
+var _ = Describe("workloadIdentityConfigHash", func() {
+	It("returns the same hash for equal configs and a different hash when the spec changes", func() {
+		wi := &libsveltosv1beta1.WorkloadIdentityConfig{
+			Provider: libsveltosv1beta1.WorkloadIdentityProviderGCP,
+			Endpoint: wiTestCachedEndpoint,
+			GCP: &libsveltosv1beta1.GCPWorkloadIdentityConfig{
+				ProjectID:   wiTestGCPProjectID,
+				ClusterName: wiTestGCPCluster,
+				Location:    wiTestGCPLocation,
+			},
+		}
+
+		h1, err := clusterproxy.WorkloadIdentityConfigHashForTest(wi)
+		Expect(err).To(BeNil())
+
+		h2, err := clusterproxy.WorkloadIdentityConfigHashForTest(wi.DeepCopy())
+		Expect(err).To(BeNil())
+		Expect(h1).To(Equal(h2))
+
+		withCA := wi.DeepCopy()
+		withCA.CASecretRef = &corev1.LocalObjectReference{Name: wiTestCASecretName}
+		h3, err := clusterproxy.WorkloadIdentityConfigHashForTest(withCA)
+		Expect(err).To(BeNil())
+		Expect(h3).ToNot(Equal(h1))
+	})
 })
 
 var _ = Describe("WorkloadIdentity CA secret", func() {