Core: periodically prune orphan Vault Clients

Previously, the CachingClientFactory would keep all cached Vault Clients
alive even if no longer had any referring objects.

This change adds a new periodic function that will prune any Vault
client that has no referring objects.

- Don't prune new clients.

api/v1beta1/vaultpkisecret_types.go

@@ -137,6 +137,9 @@ type VaultPKISecretStatus struct {
 	SecretMAC string `json:"secretMAC,omitempty"`
 	Valid     bool   `json:"valid"`
 	Error     string `json:"error"`
+	// VaultClientMeta contains the status of the Vault client and is used during
+	// resource reconciliation.
+	VaultClientMeta VaultClientMeta `json:"vaultClientMeta,omitempty"`
 }
 
 //+kubebuilder:object:root=true

api/v1beta1/vaultstaticsecret_types.go

@@ -66,6 +66,9 @@ type VaultStaticSecretStatus struct {
 	// The SecretMac is also used to detect drift in the Destination Secret's Data.
 	// If drift is detected the data will be synced to the Destination.
 	SecretMAC string `json:"secretMAC,omitempty"`
+	// VaultClientMeta contains the status of the Vault client and is used during
+	// resource reconciliation.
+	VaultClientMeta VaultClientMeta `json:"vaultClientMeta,omitempty"`
 }
 
 //+kubebuilder:object:root=true

chart/crds/secrets.hashicorp.com_vaultpkisecrets.yaml

@@ -374,6 +374,21 @@ spec:
                 type: string
               valid:
                 type: boolean
+              vaultClientMeta:
+                description: |-
+                  VaultClientMeta contains the status of the Vault client and is used during
+                  resource reconciliation.
+                properties:
+                  cacheKey:
+                    description: CacheKey is the unique key used to identify the client
+                      cache.
+                    type: string
+                  id:
+                    description: |-
+                      ID is the Vault ID of the authenticated client. The ID should never contain
+                      any sensitive information.
+                    type: string
+                type: object
             required:
             - error
             - lastGeneration

chart/crds/secrets.hashicorp.com_vaultstaticsecrets.yaml

@@ -295,6 +295,21 @@ spec:
                   The SecretMac is also used to detect drift in the Destination Secret's Data.
                   If drift is detected the data will be synced to the Destination.
                 type: string
+              vaultClientMeta:
+                description: |-
+                  VaultClientMeta contains the status of the Vault client and is used during
+                  resource reconciliation.
+                properties:
+                  cacheKey:
+                    description: CacheKey is the unique key used to identify the client
+                      cache.
+                    type: string
+                  id:
+                    description: |-
+                      ID is the Vault ID of the authenticated client. The ID should never contain
+                      any sensitive information.
+                    type: string
+                type: object
             required:
             - lastGeneration
             type: object

config/crd/bases/secrets.hashicorp.com_vaultpkisecrets.yaml

@@ -374,6 +374,21 @@ spec:
                 type: string
               valid:
                 type: boolean
+              vaultClientMeta:
+                description: |-
+                  VaultClientMeta contains the status of the Vault client and is used during
+                  resource reconciliation.
+                properties:
+                  cacheKey:
+                    description: CacheKey is the unique key used to identify the client
+                      cache.
+                    type: string
+                  id:
+                    description: |-
+                      ID is the Vault ID of the authenticated client. The ID should never contain
+                      any sensitive information.
+                    type: string
+                type: object
             required:
             - error
             - lastGeneration

config/crd/bases/secrets.hashicorp.com_vaultstaticsecrets.yaml

@@ -295,6 +295,21 @@ spec:
                   The SecretMac is also used to detect drift in the Destination Secret's Data.
                   If drift is detected the data will be synced to the Destination.
                 type: string
+              vaultClientMeta:
+                description: |-
+                  VaultClientMeta contains the status of the Vault client and is used during
+                  resource reconciliation.
+                properties:
+                  cacheKey:
+                    description: CacheKey is the unique key used to identify the client
+                      cache.
+                    type: string
+                  id:
+                    description: |-
+                      ID is the Vault ID of the authenticated client. The ID should never contain
+                      any sensitive information.
+                    type: string
+                type: object
             required:
             - lastGeneration
             type: object

controllers/vaultdynamicsecret_controller_test.go

@@ -992,6 +992,8 @@ func TestVaultDynamicSecretReconciler_computePostSyncHorizon(t *testing.T) {
 	}
 }
 
+var _ vault.Client = (*stubVaultClient)(nil)
+
 type stubVaultClient struct {
 	vault.Client
 	cacheKey           vault.ClientCacheKey

controllers/vaultpkisecret_controller.go

@@ -175,6 +175,10 @@ func (r *VaultPKISecretReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		}, nil
 	}
 
+	clientCacheKey, _ := c.GetCacheKey()
+	o.Status.VaultClientMeta.CacheKey = clientCacheKey.String()
+	o.Status.VaultClientMeta.ID = c.ID()
+
 	resp, err := c.Write(ctx, vault.NewWriteRequest(path, o.GetIssuerAPIData()))
 	if err != nil {
 		if vault.IsForbiddenError(err) {

controllers/vaultstaticsecret_controller.go

@@ -79,6 +79,10 @@ func (r *VaultStaticSecretReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		return ctrl.Result{}, err
 	}
 
+	clientCacheKey, _ := c.GetCacheKey()
+	o.Status.VaultClientMeta.CacheKey = clientCacheKey.String()
+	o.Status.VaultClientMeta.ID = c.ID()
+
 	var requeueAfter time.Duration
 	if o.Spec.RefreshAfter != "" {
 		d, err := parseDurationString(o.Spec.RefreshAfter, ".spec.refreshAfter", 0)

docs/api/api-reference.md

@@ -804,6 +804,8 @@ sync the secret. This status is used during resource reconciliation.
 
 _Appears in:_
 - [VaultDynamicSecretStatus](#vaultdynamicsecretstatus)
+- [VaultPKISecretStatus](#vaultpkisecretstatus)
+- [VaultStaticSecretStatus](#vaultstaticsecretstatus)
 
 | Field | Description | Default | Validation |
 | --- | --- | --- | --- |

internal/vault/cache.go

@@ -20,6 +20,8 @@ type ClientCache interface {
 	Get(ClientCacheKey) (Client, bool)
 	Add(Client) (bool, error)
 	Remove(ClientCacheKey) bool
+	Keys() []ClientCacheKey
+	Values() []Client
 	Len() int
 	Prune(filterFunc ClientCachePruneFilterFunc) []Client
 	Contains(key ClientCacheKey) bool
@@ -40,11 +42,19 @@ type clientCache struct {
 	missCloneCounter   prometheus.Counter
 }
 
+func (c *clientCache) Keys() []ClientCacheKey {
+	return c.cache.Keys()
+}
+
+func (c *clientCache) Values() []Client {
+	return c.cache.Values()
+}
+
 // Purge all Clients from the cache. Useful when shutting down a
 // CachingClientFactory.
 func (c *clientCache) Purge() []ClientCacheKey {
 	var purged []ClientCacheKey
-	for _, key := range c.cache.Keys() {
+	for _, key := range c.Keys() {
 		client, ok := c.Get(key)
 		if !ok {
 			continue
@@ -72,19 +82,27 @@ func (c *clientCache) Len() int {
 func (c *clientCache) Get(key ClientCacheKey) (Client, bool) {
 	if key.IsClone() {
 		if client, ok := c.cloneCache.Get(key); ok {
-			c.hitCloneCounter.Inc()
+			if c.hitCloneCounter != nil {
+				c.hitCloneCounter.Inc()
+			}
 			return client, ok
 		} else {
-			c.missCloneCounter.Inc()
+			if c.missCloneCounter != nil {
+				c.missCloneCounter.Inc()
+			}
 		}
 		return nil, false
 	}
 
 	if client, ok := c.cache.Get(key); ok {
-		c.hitCounter.Inc()
+		if c.hitCounter != nil {
+			c.hitCounter.Inc()
+		}
 		return client, ok
 	} else {
-		c.missCounter.Inc()
+		if c.missCounter != nil {
+			c.missCounter.Inc()
+		}
 		return nil, false
 	}
 }
@@ -136,7 +154,7 @@ func (c *clientCache) Remove(key ClientCacheKey) bool {
 
 func (c *clientCache) Prune(filterFunc ClientCachePruneFilterFunc) []Client {
 	var pruned []Client
-	for _, k := range c.cache.Keys() {
+	for _, k := range c.Keys() {
 		if client, ok := c.cache.Peek(k); ok {
 			if filterFunc(client) {
 				if c.remove(k, client) {

internal/vault/client.go

@@ -26,6 +26,26 @@ import (
 	"github.com/hashicorp/vault-secrets-operator/internal/metrics"
 )
 
+type ClientStat struct {
+	// createTime is the time the client was created.
+	createTime time.Time
+	mu         sync.RWMutex
+}
+
+// Age returns the duration since the client was created.
+func (m *ClientStat) Age() time.Duration {
+	m.mu.RLock()
+	defer m.mu.RUnlock()
+	return time.Since(m.createTime)
+}
+
+// Reset the client's creation time to the current time.
+func (m *ClientStat) Reset() {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	m.createTime = time.Now()
+}
+
 type ClientOptions struct {
 	SkipRenewal   bool
 	WatcherDoneCh chan<- *ClientCallbackHandlerRequest
@@ -170,6 +190,7 @@ type Client interface {
 	SetNamespace(string)
 	Tainted() bool
 	Untaint() bool
+	Stat() *ClientStat
 }
 
 var _ Client = (*defaultClient)(nil)
@@ -193,6 +214,11 @@ type defaultClient struct {
 	once               sync.Once
 	mu                 sync.RWMutex
 	id                 string
+	clientStat         *ClientStat
+}
+
+func (c *defaultClient) Stat() *ClientStat {
+	return c.clientStat
 }
 
 // Untaint the client, marking it as untainted. This should be done after the
@@ -772,6 +798,9 @@ func (c *defaultClient) init(ctx context.Context, client ctrlclient.Client,
 	c.connObj = connObj
 	c.watcherDoneCh = opts.WatcherDoneCh
 
+	c.clientStat = &ClientStat{}
+	c.clientStat.Reset()
+
 	return nil
 }