diff --git a/api/v1beta1/vaultpkisecret_types.go b/api/v1beta1/vaultpkisecret_types.go index 9bf9c92e..e6fb956a 100644 --- a/api/v1beta1/vaultpkisecret_types.go +++ b/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 diff --git a/api/v1beta1/vaultstaticsecret_types.go b/api/v1beta1/vaultstaticsecret_types.go index 211465fe..6d4f26b1 100644 --- a/api/v1beta1/vaultstaticsecret_types.go +++ b/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 diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index d05c83cc..f7ce438f 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1373,6 +1373,7 @@ func (in *VaultPKISecretSpec) DeepCopy() *VaultPKISecretSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VaultPKISecretStatus) DeepCopyInto(out *VaultPKISecretStatus) { *out = *in + out.VaultClientMeta = in.VaultClientMeta } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VaultPKISecretStatus. @@ -1503,6 +1504,7 @@ func (in *VaultStaticSecretSpec) DeepCopy() *VaultStaticSecretSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VaultStaticSecretStatus) DeepCopyInto(out *VaultStaticSecretStatus) { *out = *in + out.VaultClientMeta = in.VaultClientMeta } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VaultStaticSecretStatus. diff --git a/chart/crds/secrets.hashicorp.com_vaultpkisecrets.yaml b/chart/crds/secrets.hashicorp.com_vaultpkisecrets.yaml index a7ed16f9..5f575734 100644 --- a/chart/crds/secrets.hashicorp.com_vaultpkisecrets.yaml +++ b/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 diff --git a/chart/crds/secrets.hashicorp.com_vaultstaticsecrets.yaml b/chart/crds/secrets.hashicorp.com_vaultstaticsecrets.yaml index 986e0423..c0781e0b 100644 --- a/chart/crds/secrets.hashicorp.com_vaultstaticsecrets.yaml +++ b/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 diff --git a/config/crd/bases/secrets.hashicorp.com_vaultpkisecrets.yaml b/config/crd/bases/secrets.hashicorp.com_vaultpkisecrets.yaml index a7ed16f9..5f575734 100644 --- a/config/crd/bases/secrets.hashicorp.com_vaultpkisecrets.yaml +++ b/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 diff --git a/config/crd/bases/secrets.hashicorp.com_vaultstaticsecrets.yaml b/config/crd/bases/secrets.hashicorp.com_vaultstaticsecrets.yaml index 986e0423..c0781e0b 100644 --- a/config/crd/bases/secrets.hashicorp.com_vaultstaticsecrets.yaml +++ b/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 diff --git a/controllers/vaultdynamicsecret_controller_test.go b/controllers/vaultdynamicsecret_controller_test.go index e0ca9fc3..efe6f384 100644 --- a/controllers/vaultdynamicsecret_controller_test.go +++ b/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 diff --git a/controllers/vaultpkisecret_controller.go b/controllers/vaultpkisecret_controller.go index de0b90a7..943c4fbc 100644 --- a/controllers/vaultpkisecret_controller.go +++ b/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) { diff --git a/controllers/vaultstaticsecret_controller.go b/controllers/vaultstaticsecret_controller.go index 36d91897..b6587bee 100644 --- a/controllers/vaultstaticsecret_controller.go +++ b/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) diff --git a/docs/api/api-reference.md b/docs/api/api-reference.md index f8f4e2a1..1df5ac68 100644 --- a/docs/api/api-reference.md +++ b/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 | | --- | --- | --- | --- | diff --git a/internal/vault/cache.go b/internal/vault/cache.go index 37c8d658..edc92aa6 100644 --- a/internal/vault/cache.go +++ b/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) { diff --git a/internal/vault/client.go b/internal/vault/client.go index cba04578..adb0ce5e 100644 --- a/internal/vault/client.go +++ b/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 } diff --git a/internal/vault/client_factory.go b/internal/vault/client_factory.go index ffcaa58e..278a47ee 100644 --- a/internal/vault/client_factory.go +++ b/internal/vault/client_factory.go @@ -40,6 +40,8 @@ const ( ClientCallbackOnCacheRemoval ) +var defaultPruneOrphanAge = 5 * time.Minute + func (o ClientCallbackOn) String() string { switch o { case ClientCallbackOnLifetimeWatcherDone: @@ -139,7 +141,8 @@ type cachingClientFactory struct { // encClientLock is a lock for the encryption client. It is used to ensure that // only one encryption client is created. This is necessary because the // encryption client is not stored in the cache. - encClientLock sync.RWMutex + encClientLock sync.RWMutex + orphanPrunerCancel context.CancelFunc } // Start method for cachingClientFactory starts the lifetime watcher handler. @@ -148,6 +151,7 @@ func (m *cachingClientFactory) Start(ctx context.Context) { defer m.mu.Unlock() m.onceDoWatcher.Do(func() { m.startClientCallbackHandler(ctx) + m.startOrphanClientPruner(ctx) }) } @@ -245,15 +249,24 @@ func (m *cachingClientFactory) pruneStorage(ctx context.Context, client ctrlclie // onClientEvict should be called whenever an eviction from the ClientCache occurs. // It should always call Client.Close() to prevent leaking Go routines. func (m *cachingClientFactory) onClientEvict(ctx context.Context, client ctrlclient.Client, cacheKey ClientCacheKey, c Client) { - logger := m.logger.WithValues("cacheKey", cacheKey) + logger := m.logger.WithName("onClientEvict").WithValues("cacheKey", cacheKey) logger.Info("Handling client cache eviction") c.Close(m.revokeOnEvict) if m.storageEnabled() && m.pruneStorageOnEvict { - if count, err := m.pruneStorage(ctx, client, cacheKey); err != nil { - logger.Error(err, "Failed to remove Client from storage") - } else { - logger.Info("Pruned storage", "count", count) + m.encClientLock.RLock() + if m.clientCacheKeyEncrypt == cacheKey { + m.encClientLock.RUnlock() + m.encClientLock.Lock() + m.clientCacheKeyEncrypt = "" + m.encClientLock.Unlock() + } + if m.pruneStorageOnEvict { + if count, err := m.pruneStorage(ctx, client, cacheKey); err != nil { + logger.Error(err, "Failed to remove Client from storage") + } else { + logger.Info("Pruned storage", "count", count) + } } } @@ -659,8 +672,9 @@ func (m *cachingClientFactory) storageEncryptionClient(ctx context.Context, clie c, ok := m.cache.Get(m.clientCacheKeyEncrypt) if !ok { - return nil, fmt.Errorf("expected Client for storage encryption not found in the cache, "+ - "cacheKey=%s", m.clientCacheKeyEncrypt) + // TODO: reconsider the approach to getting a client for storage encryption. + m.clientCacheKeyEncrypt = "" + return m.storageEncryptionClient(ctx, client) } if cached { @@ -689,15 +703,15 @@ func (m *cachingClientFactory) incrementRequestCounter(operation string, err err } func (m *cachingClientFactory) startClientCallbackHandler(ctx context.Context) { + logger := log.FromContext(ctx).WithName("clientCallbackHandler") if m.callbackHandlerCancel != nil { - m.logger.Info("Already started") + logger.Info("Already started") return } callbackCtx, cancel := context.WithCancel(ctx) m.callbackHandlerCancel = cancel - logger := log.FromContext(ctx).WithName("clientCallbackHandler") logger.Info("Starting client callback handler") go func() { @@ -793,18 +807,142 @@ func (m *cachingClientFactory) callClientCallbacks(ctx context.Context, c Client } } +// startOrphanClientPruner starts a go routine that will periodically prune +// orphaned clients from the cache. An orphaned client is a client that is not +// associated with any of secretsv1beta1.VaultStaticSecret, +// secretsv1beta1.VaultPKISecret, secretsv1beta1.VaultDynamicSecret. +func (m *cachingClientFactory) startOrphanClientPruner(ctx context.Context) { + logger := log.FromContext(ctx).WithName("orphanClientPruner") + if m.orphanPrunerCancel != nil { + logger.Info("Already started") + } + + ctx_, cancel := context.WithCancel(ctx) + m.orphanPrunerCancel = cancel + // TODO: make period a command line option + ticker := time.NewTicker(30 * time.Second) + go func() { + defer func() { + m.orphanPrunerCancel = nil + }() + for { + select { + case <-ctx_.Done(): + logger.Info("Done") + return + case <-ticker.C: + if count, err := m.pruneOrphanClients(ctx); err != nil { + logger.Error(err, "Prune orphan Vault Clients") + } else { + logger.Info("Prune orphan Vault Clients", "count", count) + } + } + } + }() +} + +// pruneOrphanClients will remove all clients from the cache that are not +// associated with any of the following custom resources: +// secretsv1beta1.VaultStaticSecret, secretsv1beta1.VaultPKISecret, +// secretsv1beta1.VaultDynamicSecret. +func (m *cachingClientFactory) pruneOrphanClients(ctx context.Context) (int, error) { + m.mu.Lock() + defer m.mu.Unlock() + + logger := m.logger.WithName("pruneOrphanClients") + + currentClientCacheKeys := map[ClientCacheKey]empty{} + addCurrentClientCacheKeys := func(meta secretsv1beta1.VaultClientMeta) { + if meta.CacheKey != "" { + key := ClientCacheKey(meta.CacheKey) + currentClientCacheKeys[key] = empty{} + } + } + + var vssList secretsv1beta1.VaultStaticSecretList + err := m.ctrlClient.List(ctx, &vssList) + if err != nil { + return 0, err + } + + for _, o := range vssList.Items { + addCurrentClientCacheKeys(o.Status.VaultClientMeta) + } + + var vpsList secretsv1beta1.VaultPKISecretList + err = m.ctrlClient.List(ctx, &vpsList) + if err != nil { + return 0, err + } + for _, o := range vpsList.Items { + addCurrentClientCacheKeys(o.Status.VaultClientMeta) + } + + var vdsList secretsv1beta1.VaultDynamicSecretList + err = m.ctrlClient.List(ctx, &vdsList) + if err != nil { + return 0, err + } + for _, o := range vdsList.Items { + addCurrentClientCacheKeys(o.Status.VaultClientMeta) + } + + var toPrune []ClientCacheKey + for _, c := range m.cache.Values() { + key, err := c.GetCacheKey() + if err != nil { + continue + } + + if _, ok := currentClientCacheKeys[key]; !ok { + if key == m.clientCacheKeyEncrypt { + continue + } + stat := c.Stat() + if stat == nil { + continue + } + // prune clients that have not been created in the last 5 minutes, this gives + // time for any referring resource to update their + // .status.vaultClientMeta.cacheKey + if stat.Age() >= defaultPruneOrphanAge { + toPrune = append(toPrune, key) + } + } + } + + // TODO: ensure that this does not block forever... + var count int + wg := sync.WaitGroup{} + wg.Add(len(toPrune)) + for _, key := range toPrune { + count++ + go func() { + defer wg.Done() + m.cache.Remove(key) + m.removeClientLock(key) + }() + } + wg.Wait() + + logger.V(consts.LogLevelDebug).Info( + "Pruned orphaned clients", "count", count, "pruned", toPrune) + return count, nil +} + // NewCachingClientFactory returns a CachingClientFactory with ClientCache initialized. // The ClientCache's onEvictCallback is registered with the factory's onClientEvict(), // to ensure any evictions are handled by the factory (this is very important). func NewCachingClientFactory(ctx context.Context, client ctrlclient.Client, cacheStorage ClientCacheStorage, config *CachingClientFactoryConfig) (CachingClientFactory, error) { factory := &cachingClientFactory{ - storage: cacheStorage, - recorder: config.Recorder, - persist: config.Persist, - ctrlClient: client, - callbackHandlerCh: make(chan *ClientCallbackHandlerRequest), - encryptionRequired: config.StorageConfig.EnforceEncryption, - clientLocks: make(map[ClientCacheKey]*sync.RWMutex, config.ClientCacheSize), + storage: cacheStorage, + recorder: config.Recorder, + persist: config.Persist, + ctrlClient: client, + pruneStorageOnEvict: true, + callbackHandlerCh: make(chan *ClientCallbackHandlerRequest), + encryptionRequired: config.StorageConfig.EnforceEncryption, + clientLocks: make(map[ClientCacheKey]*sync.RWMutex, config.ClientCacheSize), logger: zap.New().WithName("clientCacheFactory").WithValues( "persist", config.Persist, "enforceEncryption", config.StorageConfig.EnforceEncryption, @@ -932,3 +1070,5 @@ type nullEventRecorder struct { } func (n *nullEventRecorder) Event(_ runtime.Object, _, _, _ string) {} + +type empty struct{} diff --git a/internal/vault/client_factory_test.go b/internal/vault/client_factory_test.go index cf6240db..f4b331ba 100644 --- a/internal/vault/client_factory_test.go +++ b/internal/vault/client_factory_test.go @@ -5,12 +5,24 @@ package vault import ( "context" + "errors" + "fmt" "sync" "testing" "time" + lru "github.com/hashicorp/golang-lru/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + secretsv1beta1 "github.com/hashicorp/vault-secrets-operator/api/v1beta1" + "github.com/hashicorp/vault-secrets-operator/internal/credentials/provider" ) func Test_cachingClientFactory_RegisterClientCallbackHandler(t *testing.T) { @@ -300,3 +312,245 @@ func Test_cachingClientFactory_callClientCallbacks(t *testing.T) { }) } } + +func Test_cachingClientFactory_pruneOrphanClients(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + type keyTest struct { + key ClientCacheKey + creationTimeOffset time.Duration + } + + newCache := func(size int, keyTests ...keyTest) *clientCache { + lruCache, err := lru.NewWithEvict[ClientCacheKey, Client](size, nil) + require.NoError(t, err) + c := &clientCache{ + cache: lruCache, + } + for _, k := range keyTests { + c.cache.Add(k.key, &stubClient{ + cacheKey: k.key, + clientStat: &ClientStat{ + createTime: time.Now().Add(k.creationTimeOffset), + }, + }) + } + return c + } + + clientBuilder := newClientBuilder() + schemeLessBuilder := fake.NewClientBuilder() + tests := []struct { + name string + cache ClientCache + c ctrlclient.Client + want int + createFunc func(t *testing.T, c ctrlclient.Client) error + wantClientCacheKeys []ClientCacheKey + wantErr assert.ErrorAssertionFunc + }{ + { + name: "empty-cache", + c: clientBuilder.Build(), + cache: newCache(1), + wantErr: assert.NoError, + }, + { + name: "no-referring-objects-purge", + c: clientBuilder.Build(), + cache: newCache(1, + keyTest{ + key: "kubernetes-123456", + creationTimeOffset: -defaultPruneOrphanAge, + }, + ), + want: 1, + wantErr: assert.NoError, + }, + { + name: "prune-some", + c: clientBuilder.Build(), + createFunc: func(t *testing.T, c ctrlclient.Client) error { + t.Helper() + var errs error + for _, o := range []ctrlclient.Object{ + &secretsv1beta1.VaultStaticSecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vss-1", + Namespace: "default", + }, + Status: secretsv1beta1.VaultStaticSecretStatus{ + VaultClientMeta: secretsv1beta1.VaultClientMeta{ + CacheKey: "kubernetes-123456", + }, + }, + }, + } { + errs = errors.Join(errs, c.Create(ctx, o)) + } + return errs + }, + cache: newCache(2, + keyTest{ + key: "kubernetes-123455", + creationTimeOffset: -defaultPruneOrphanAge, + }, + keyTest{ + key: "kubernetes-123456", + creationTimeOffset: -defaultPruneOrphanAge, + }, + ), + wantClientCacheKeys: []ClientCacheKey{ + ClientCacheKey("kubernetes-123456"), + }, + want: 1, + wantErr: assert.NoError, + }, + { + name: "none", + c: clientBuilder.Build(), + cache: newCache(1, + keyTest{ + key: "kubernetes-123456", + creationTimeOffset: -defaultPruneOrphanAge, + }, + ), + createFunc: func(t *testing.T, c ctrlclient.Client) error { + t.Helper() + var errs error + for _, o := range []ctrlclient.Object{ + &secretsv1beta1.VaultStaticSecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vss-1", + Namespace: "default", + }, + Status: secretsv1beta1.VaultStaticSecretStatus{ + VaultClientMeta: secretsv1beta1.VaultClientMeta{ + CacheKey: "kubernetes-123456", + }, + }, + }, + &secretsv1beta1.VaultPKISecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vps-1", + Namespace: "default", + }, + Status: secretsv1beta1.VaultPKISecretStatus{ + VaultClientMeta: secretsv1beta1.VaultClientMeta{ + CacheKey: "kubernetes-123456", + }, + }, + }, + &secretsv1beta1.VaultDynamicSecret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vds-1", + Namespace: "default", + }, + Status: secretsv1beta1.VaultDynamicSecretStatus{ + VaultClientMeta: secretsv1beta1.VaultClientMeta{ + CacheKey: "kubernetes-123456", + }, + }, + }, + } { + errs = errors.Join(errs, c.Create(ctx, o)) + } + return errs + }, + wantErr: assert.NoError, + wantClientCacheKeys: []ClientCacheKey{ + ClientCacheKey("kubernetes-123456"), + }, + want: 0, + }, + { + name: "no-prune-recent", + c: clientBuilder.Build(), + cache: newCache(2, + keyTest{ + key: "kubernetes-123455", + creationTimeOffset: -(defaultPruneOrphanAge - time.Second*1), + }, + keyTest{ + key: "kubernetes-123456", + creationTimeOffset: -(defaultPruneOrphanAge - time.Second*1), + }, + ), + wantClientCacheKeys: []ClientCacheKey{ + ClientCacheKey("kubernetes-123455"), + ClientCacheKey("kubernetes-123456"), + }, + want: 0, + wantErr: assert.NoError, + }, + { + name: "vss-scheme-not-set", + c: schemeLessBuilder.Build(), + cache: newCache(1, + keyTest{ + key: "kubernetes-123456", + creationTimeOffset: -defaultPruneOrphanAge, + }, + ), + want: 0, + wantClientCacheKeys: []ClientCacheKey{ + ClientCacheKey("kubernetes-123456"), + }, + wantErr: func(t assert.TestingT, err error, msgAndArgs ...interface{}) bool { + return assert.ErrorContains(t, err, "no kind is registered for the type v1beta1.VaultStaticSecret") + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := &cachingClientFactory{ + cache: tt.cache, + ctrlClient: tt.c, + } + + if tt.createFunc != nil { + require.NoError(t, tt.createFunc(t, tt.c)) + } + + got, err := m.pruneOrphanClients(ctx) + if !tt.wantErr(t, err, fmt.Sprintf("pruneOrphanClients(%v)", ctx)) { + return + } + assert.Equalf(t, tt.want, got, "pruneOrphanClients(%v)", ctx) + assert.ElementsMatchf(t, tt.wantClientCacheKeys, m.cache.Keys(), "pruneOrphanClients(%v)", ctx) + }) + } +} + +// newClientBuilder returns a new fake.ClientBuilder with the necessary schemes. +// copied from helpers +func newClientBuilder() *fake.ClientBuilder { + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(secretsv1beta1.AddToScheme(scheme)) + return fake.NewClientBuilder().WithScheme(scheme) +} + +var _ Client = (*stubClient)(nil) + +type stubClient struct { + Client + cacheKey ClientCacheKey + credentialProvider provider.CredentialProviderBase + isClone bool + clientStat *ClientStat +} + +func (c *stubClient) GetCacheKey() (ClientCacheKey, error) { + return c.cacheKey, nil +} + +func (c *stubClient) IsClone() bool { + return c.isClone +} + +func (c *stubClient) Stat() *ClientStat { + return c.clientStat +}