Call VDS callbacks on VaultAuth and VaultConnection changes

controllers/vaultauth_controller.go

@@ -142,8 +142,9 @@ func (r *VaultAuthReconciler) updateStatus(ctx context.Context, o *secretsv1beta
 func (r *VaultAuthReconciler) handleFinalizer(ctx context.Context, o *secretsv1beta1.VaultAuth) (ctrl.Result, error) {
 	if controllerutil.ContainsFinalizer(o, vaultAuthFinalizer) {
 		if _, err := r.ClientFactory.Prune(ctx, r.Client, o, vault.CachingClientFactoryPruneRequest{
-			FilterFunc:   filterAllCacheRefs,
-			PruneStorage: true,
+			FilterFunc:          filterAllCacheRefs,
+			PruneStorage:        true,
+			SkipClientCallbacks: true,
 		}); err != nil {
 			return ctrl.Result{}, err
 		}

controllers/vaultconnection_controller.go

@@ -136,8 +136,9 @@ func (r *VaultConnectionReconciler) updateStatus(ctx context.Context, o *secrets
 func (r *VaultConnectionReconciler) handleFinalizer(ctx context.Context, o *secretsv1beta1.VaultConnection) (ctrl.Result, error) {
 	if controllerutil.ContainsFinalizer(o, vaultConnectionFinalizer) {
 		if _, err := r.ClientFactory.Prune(ctx, r.Client, o, vault.CachingClientFactoryPruneRequest{
-			FilterFunc:   filterAllCacheRefs,
-			PruneStorage: true,
+			FilterFunc:          filterAllCacheRefs,
+			PruneStorage:        true,
+			SkipClientCallbacks: true,
 		}); err != nil {
 			return ctrl.Result{}, err
 		}

internal/vault/cache.go

@@ -21,7 +21,7 @@ type ClientCache interface {
 	Add(Client) (bool, error)
 	Remove(ClientCacheKey) bool
 	Len() int
-	Prune(filterFunc ClientCachePruneFilterFunc) []ClientCacheKey
+	Prune(filterFunc ClientCachePruneFilterFunc) []Client
 	Contains(key ClientCacheKey) bool
 	Purge() []ClientCacheKey
 }
@@ -134,13 +134,13 @@ func (c *clientCache) Remove(key ClientCacheKey) bool {
 	return removed
 }
 
-func (c *clientCache) Prune(filterFunc ClientCachePruneFilterFunc) []ClientCacheKey {
-	var pruned []ClientCacheKey
+func (c *clientCache) Prune(filterFunc ClientCachePruneFilterFunc) []Client {
+	var pruned []Client
 	for _, k := range c.cache.Keys() {
 		if client, ok := c.cache.Peek(k); ok {
 			if filterFunc(client) {
 				if c.remove(k, client) {
-					pruned = append(pruned, k)
+					pruned = append(pruned, client)
 				}
 			}
 		}
@@ -158,6 +158,10 @@ func (c *clientCache) remove(key ClientCacheKey, client Client) bool {
 }
 
 func (c *clientCache) pruneClones(cacheKey ClientCacheKey) {
+	if c.cloneCache == nil {
+		return
+	}
+
 	for _, k := range c.cloneCache.Keys() {
 		if !strings.HasPrefix(k.String(), cacheKey.String()) {
 			continue

internal/vault/cache_test.go

@@ -13,6 +13,8 @@ import (
 )
 
 func Test_clientCache_Prune(t *testing.T) {
+	t.Parallel()
+
 	dummyCallbackFunc := func(_ ClientCacheKey, _ Client) {}
 	cacheSize := 10
 
@@ -46,29 +48,33 @@ func Test_clientCache_Prune(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			c := &clientCache{}
 			cache, err := lru.NewWithEvict[ClientCacheKey, Client](cacheSize, dummyCallbackFunc)
 			require.NoError(t, err)
-			c.cache = cache
+			cloneCache, err := lru.New[ClientCacheKey, Client](cacheSize)
+			require.NoError(t, err)
 
-			var expectedPrunedKeys []ClientCacheKey
+			c := &clientCache{
+				cache:      cache,
+				cloneCache: cloneCache,
+			}
+			var expectedPrunedClients []Client
 			for i := 0; i < tt.cacheLen; i++ {
+				id := fmt.Sprintf("key%d", i)
 				client := &defaultClient{
-					// for simplicity, client is clone. pruneClones() should be tested separately
-					isClone: true,
+					id: id,
 				}
-				key := ClientCacheKey(fmt.Sprintf("key%d", i))
 				if tt.filterFuncReturnsTrue {
-					expectedPrunedKeys = append(expectedPrunedKeys, key)
+					expectedPrunedClients = append(expectedPrunedClients, client)
 				}
-				c.cache.Add(key, client)
+				c.cache.Add(ClientCacheKey(id), client)
 			}
-			assert.Equal(t, tt.cacheLen, c.cache.Len(), "unexpected cache len before calling Prune()")
-			keys := c.Prune(func(Client) bool {
+			assert.Equal(t, tt.cacheLen, c.cache.Len(),
+				"unexpected cache len before calling Prune()")
+			actual := c.Prune(func(Client) bool {
 				return tt.filterFuncReturnsTrue
 			})
-			assert.EqualValues(t, expectedPrunedKeys, keys)
-			assert.Equal(t, tt.cacheLen-len(expectedPrunedKeys), c.cache.Len())
+			assert.EqualValues(t, expectedPrunedClients, actual)
+			assert.Equal(t, tt.cacheLen-len(expectedPrunedClients), c.cache.Len())
 		})
 	}
 }

internal/vault/client_factory.go

@@ -66,6 +66,9 @@ type clientCacheObjectFilterFunc func(cur, other ctrlclient.Object) bool
 type CachingClientFactoryPruneRequest struct {
 	FilterFunc   clientCacheObjectFilterFunc
 	PruneStorage bool
+	// SkipClientCallbacks will prevent the ClientCallbackHandlers from being called
+	// when a Client is pruned.
+	SkipClientCallbacks bool
 }
 
 type CachingClientFactory interface {
@@ -156,24 +159,29 @@ func (m *cachingClientFactory) Prune(ctx context.Context, client ctrlclient.Clie
 		return 0, fmt.Errorf("client removal not supported for type %T", cur)
 	}
 
-	if !req.PruneStorage {
-		return 0, nil
-	}
-	return m.prune(ctx, client, filter)
+	return m.prune(ctx, client, filter, req.SkipClientCallbacks)
 }
 
-func (m *cachingClientFactory) prune(ctx context.Context, client ctrlclient.Client, filter ClientCachePruneFilterFunc) (int, error) {
+func (m *cachingClientFactory) prune(ctx context.Context, client ctrlclient.Client, filter ClientCachePruneFilterFunc, skipCallbacks bool) (int, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
 	// prune the client cache for filter, pruned is a slice of cache keys
 	pruned := m.cache.Prune(filter)
 	var errs error
-	// for all cache entries pruned, remove the corresponding storage entries.
-	if m.storageEnabled() {
-		for _, key := range pruned {
-			if _, err := m.pruneStorage(ctx, client, key); err != nil {
-				errs = errors.Join(errs, err)
+	if !skipCallbacks {
+		for _, c := range pruned {
+			// the callback handler will remove the client from the storage
+			m.callbackHandlerCh <- c
+		}
+	} else {
+		// for all cache entries pruned, remove the corresponding storage entries.
+		if m.storageEnabled() {
+			for _, c := range pruned {
+				key, _ := c.GetCacheKey()
+				if _, err := m.pruneStorage(ctx, client, key); err != nil {
+					errs = errors.Join(errs, err)
+				}
 			}
 		}
 	}

test/integration/vaultdynamicsecret_integration_test.go

@@ -28,6 +28,7 @@ import (
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	secretsv1beta1 "github.com/hashicorp/vault-secrets-operator/api/v1beta1"
+	"github.com/hashicorp/vault-secrets-operator/internal/common"
 	"github.com/hashicorp/vault-secrets-operator/internal/consts"
 	"github.com/hashicorp/vault-secrets-operator/internal/helpers"
 )
@@ -662,13 +663,69 @@ func TestVaultDynamicSecret_vaultClientCallback(t *testing.T) {
 	}
 
 	tests := []struct {
-		name     string
-		create   int
-		expected map[string]int
+		name        string
+		create      int
+		triggerFunc func(t *testing.T, reconciledObjs []*secretsv1beta1.VaultDynamicSecret)
+		expected    map[string]int
 	}{
 		{
 			name:   "create-only",
 			create: 25,
+			triggerFunc: func(t *testing.T, _ []*secretsv1beta1.VaultDynamicSecret) {
+				t.Helper()
+				cfg := api.DefaultConfig()
+				cfg.Address = vaultAddr
+				vc, err := api.NewClient(cfg)
+				assert.NoError(t, err)
+				vc.SetToken(vaultToken)
+				if outputs.Namespace != "" {
+					vc.SetNamespace(outputs.Namespace)
+				}
+
+				deleteEntitiesBySAPrefix(t, vc, ctx, outputs.K8sNamespace, "sa-create-only-test-vcc")
+			},
+		},
+		{
+			name:   "create-only-vault-auth-update",
+			create: 25,
+			triggerFunc: func(t *testing.T, reconciledObjs []*secretsv1beta1.VaultDynamicSecret) {
+				t.Helper()
+				for _, obj := range reconciledObjs {
+					authObj, err := common.GetVaultAuth(ctx, crdClient, ctrlclient.ObjectKey{
+						Namespace: obj.Namespace,
+						Name:      obj.Spec.VaultAuthRef,
+					})
+					if assert.NoError(t, err) {
+						authObj.Spec.Kubernetes.TokenAudiences = []string{"vault", "test"}
+						assert.NoError(t, crdClient.Update(ctx, authObj))
+					}
+				}
+			},
+		},
+		{
+			name:   "create-only-vault-conn-update",
+			create: 25,
+			triggerFunc: func(t *testing.T, reconciledObjs []*secretsv1beta1.VaultDynamicSecret) {
+				t.Helper()
+				for _, obj := range reconciledObjs {
+					authObj, err := common.GetVaultAuth(ctx, crdClient, ctrlclient.ObjectKey{
+						Namespace: obj.Namespace,
+						Name:      obj.Spec.VaultAuthRef,
+					})
+					if assert.NoError(t, err) {
+						connObj, err := common.GetVaultConnection(ctx, crdClient, ctrlclient.ObjectKey{
+							Namespace: obj.Namespace,
+							Name:      authObj.Spec.VaultConnectionRef,
+						})
+						if assert.NoError(t, err) {
+							connObj.Spec.Headers = map[string]string{
+								"X-test-it": "true",
+							}
+							assert.NoError(t, crdClient.Update(ctx, connObj))
+						}
+					}
+				}
+			},
 		},
 	}
 
@@ -695,15 +752,27 @@ func TestVaultDynamicSecret_vaultClientCallback(t *testing.T) {
 				}
 				createObj(sa)
 
+				connObj := &secretsv1beta1.VaultConnection{
+					ObjectMeta: v1.ObjectMeta{
+						Namespace: outputs.K8sNamespace,
+						Name:      fmt.Sprintf("va-%s", nameSuffix),
+					},
+					Spec: secretsv1beta1.VaultConnectionSpec{
+						Address: testVaultAddress,
+					},
+				}
+				createObj(connObj)
+
 				authObj := &secretsv1beta1.VaultAuth{
 					ObjectMeta: v1.ObjectMeta{
 						Namespace: outputs.K8sNamespace,
 						Name:      fmt.Sprintf("va-%s", nameSuffix),
 					},
 					Spec: secretsv1beta1.VaultAuthSpec{
-						Namespace: outputs.Namespace,
-						Method:    "kubernetes",
-						Mount:     outputs.AuthMount,
+						Namespace:          outputs.Namespace,
+						Method:             "kubernetes",
+						Mount:              outputs.AuthMount,
+						VaultConnectionRef: connObj.GetName(),
 						Kubernetes: &secretsv1beta1.VaultAuthConfigKubernetes{
 							Role:           outputs.AuthRole,
 							ServiceAccount: sa.ObjectMeta.Name,
@@ -753,17 +822,8 @@ func TestVaultDynamicSecret_vaultClientCallback(t *testing.T) {
 				return
 			}
 
-			cfg := api.DefaultConfig()
-			cfg.Address = vaultAddr
-			vc, err := api.NewClient(cfg)
-			assert.NoError(t, err)
-			vc.SetToken(vaultToken)
-			if outputs.Namespace != "" {
-				vc.SetNamespace(outputs.Namespace)
-			}
-
 			t.Logf("Running rotation tests on %d reconciled objects", len(reconciledObjs))
-			deleteEntitiesBySAPrefix(t, vc, ctx, outputs.K8sNamespace, "sa-"+testSuffix)
+			tt.triggerFunc(t, reconciledObjs)
 			if !t.Failed() {
 				for idx, obj := range reconciledObjs {
 					t.Run(fmt.Sprintf("create-dest-%d", idx), func(t *testing.T) {