From b2bde3117f64598459e35fce20aa496e28fbffad Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Tue, 14 Jul 2026 19:38:37 +0200 Subject: [PATCH] fix: invalidate workload identity cache when SveltosCluster spec changes getWorkloadIdentityRestConfig caches the built rest.Config per cluster in an in-memory wiCache, keyed only by namespace/name. Eviction is only wired up in each component's SveltosCluster delete handler nothing invalidates the cache on update. So editing spec.workloadIdentity had no effect until the cached cloud token neared its natural expiry The fix consist in hashing spec.workloadIdentity and store it alongside the cached rest.Config. The cache is now only considered valid when both the hash matches the current spec and it isn't near expiry --- lib/clusterproxy/export_test.go | 16 +++- lib/clusterproxy/workload_identity.go | 31 +++++++- lib/clusterproxy/workload_identity_test.go | 91 ++++++++++++++++++++-- 3 files changed, 124 insertions(+), 14 deletions(-) diff --git a/lib/clusterproxy/export_test.go b/lib/clusterproxy/export_test.go index 984311a..0d83f92 100644 --- a/lib/clusterproxy/export_test.go +++ b/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) { diff --git a/lib/clusterproxy/workload_identity.go b/lib/clusterproxy/workload_identity.go index 67dba1b..6318d5a 100644 --- a/lib/clusterproxy/workload_identity.go +++ b/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 { diff --git a/lib/clusterproxy/workload_identity_test.go b/lib/clusterproxy/workload_identity_test.go index 3e5d4a9..7864a8a 100644 --- a/lib/clusterproxy/workload_identity_test.go +++ b/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() {