From d5185f8ab98c68075bde2903080304e22228e0ba Mon Sep 17 00:00:00 2001 From: Guilherme Caulada Date: Wed, 29 Jun 2022 08:00:24 -0700 Subject: [PATCH] Secrets: Implement unified secrets short lived cache (#51275) * Implement unified secrets short lived cache * Improve debug logging for unified secrets cache * Re-add decryption cache to sql secret kvstore * Remove cache from remote secret store plugin * Revert secret store helpers implementation * Remove cache from secret store plugin struct * Update secret store cache to implement interface * Set secret store cache value on get * Fix issues with sql secret store decryption cache * Increase clean up interval on cached secret store --- pkg/services/secrets/kvstore/cache.go | 78 +++++++++++++++++++++++++ pkg/services/secrets/kvstore/kvstore.go | 33 ++++++----- pkg/services/secrets/kvstore/sql.go | 9 ++- 3 files changed, 103 insertions(+), 17 deletions(-) create mode 100644 pkg/services/secrets/kvstore/cache.go diff --git a/pkg/services/secrets/kvstore/cache.go b/pkg/services/secrets/kvstore/cache.go new file mode 100644 index 00000000000..6fd62da9302 --- /dev/null +++ b/pkg/services/secrets/kvstore/cache.go @@ -0,0 +1,78 @@ +package kvstore + +import ( + "context" + "fmt" + "time" + + "github.com/grafana/grafana/pkg/infra/localcache" + "github.com/grafana/grafana/pkg/infra/log" +) + +type CachedKVStore struct { + log log.Logger + cache *localcache.CacheService + store SecretsKVStore +} + +func NewCachedKVStore(store SecretsKVStore, defaultExpiration time.Duration, cleanupInterval time.Duration) *CachedKVStore { + return &CachedKVStore{ + log: log.New("secrets.kvstore"), + cache: localcache.New(defaultExpiration, cleanupInterval), + store: store, + } +} + +func (kv *CachedKVStore) Get(ctx context.Context, orgId int64, namespace string, typ string) (string, bool, error) { + key := fmt.Sprint(orgId, namespace, typ) + if value, ok := kv.cache.Get(key); ok { + kv.log.Debug("got secret value from cache", "orgId", orgId, "type", typ, "namespace", namespace) + return fmt.Sprint(value), true, nil + } + value, ok, err := kv.store.Get(ctx, orgId, namespace, typ) + if err != nil { + return "", false, err + } + if ok { + kv.cache.SetDefault(key, value) + } + return value, ok, err +} + +func (kv *CachedKVStore) Set(ctx context.Context, orgId int64, namespace string, typ string, value string) error { + err := kv.store.Set(ctx, orgId, namespace, typ, value) + if err != nil { + return err + } + key := fmt.Sprint(orgId, namespace, typ) + kv.cache.SetDefault(key, value) + return nil +} + +func (kv *CachedKVStore) Del(ctx context.Context, orgId int64, namespace string, typ string) error { + err := kv.store.Del(ctx, orgId, namespace, typ) + if err != nil { + return err + } + key := fmt.Sprint(orgId, namespace, typ) + kv.cache.Delete(key) + return nil +} + +func (kv *CachedKVStore) Keys(ctx context.Context, orgId int64, namespace string, typ string) ([]Key, error) { + return kv.store.Keys(ctx, orgId, namespace, typ) +} + +func (kv *CachedKVStore) Rename(ctx context.Context, orgId int64, namespace string, typ string, newNamespace string) error { + err := kv.store.Rename(ctx, orgId, namespace, typ, newNamespace) + if err != nil { + return err + } + key := fmt.Sprint(orgId, namespace, typ) + if value, ok := kv.cache.Get(key); ok { + newKey := fmt.Sprint(orgId, newNamespace, typ) + kv.cache.SetDefault(newKey, value) + kv.cache.Delete(key) + } + return nil +} diff --git a/pkg/services/secrets/kvstore/kvstore.go b/pkg/services/secrets/kvstore/kvstore.go index d314ea4d909..2a0f8a7f2af 100644 --- a/pkg/services/secrets/kvstore/kvstore.go +++ b/pkg/services/secrets/kvstore/kvstore.go @@ -2,6 +2,7 @@ package kvstore import ( "context" + "time" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/secrets" @@ -14,22 +15,9 @@ const ( ) func ProvideService(sqlStore sqlstore.Store, secretsService secrets.Service, remoteCheck UseRemoteSecretsPluginCheck) SecretsKVStore { + var store SecretsKVStore logger := log.New("secrets.kvstore") - if remoteCheck.ShouldUseRemoteSecretsPlugin() { - logger.Debug("secrets kvstore is using a remote plugin for secrets management") - secretsPlugin, err := remoteCheck.GetPlugin() - if err != nil { - logger.Error("plugin client was nil, falling back to SQL implementation") - } else { - return &secretsKVStorePlugin{ - secretsPlugin: secretsPlugin, - secretsService: secretsService, - log: logger, - } - } - } - logger.Debug("secrets kvstore is using the default (SQL) implementation for secrets management") - return &secretsKVStoreSQL{ + store = &secretsKVStoreSQL{ sqlStore: sqlStore, secretsService: secretsService, log: logger, @@ -37,6 +25,21 @@ func ProvideService(sqlStore sqlstore.Store, secretsService secrets.Service, rem cache: make(map[int64]cachedDecrypted), }, } + if remoteCheck.ShouldUseRemoteSecretsPlugin() { + logger.Debug("secrets kvstore is using a remote plugin for secrets management") + secretsPlugin, err := remoteCheck.GetPlugin() + if err != nil { + logger.Error("plugin client was nil, falling back to SQL implementation") + } else { + store = &secretsKVStorePlugin{ + secretsPlugin: secretsPlugin, + secretsService: secretsService, + log: logger, + } + } + } + logger.Debug("secrets kvstore is using the default (SQL) implementation for secrets management") + return NewCachedKVStore(store, 5*time.Second, 5*time.Minute) } // SecretsKVStore is an interface for k/v store. diff --git a/pkg/services/secrets/kvstore/sql.go b/pkg/services/secrets/kvstore/sql.go index 08b1c9fe257..ef64f29463a 100644 --- a/pkg/services/secrets/kvstore/sql.go +++ b/pkg/services/secrets/kvstore/sql.go @@ -52,7 +52,6 @@ func (kv *secretsKVStoreSQL) Get(ctx context.Context, orgId int64, namespace str return nil } isFound = true - kv.log.Debug("got secret value", "orgId", orgId, "type", typ, "namespace", namespace) return nil }) @@ -60,7 +59,8 @@ func (kv *secretsKVStoreSQL) Get(ctx context.Context, orgId int64, namespace str kv.decryptionCache.Lock() defer kv.decryptionCache.Unlock() - if cache, present := kv.decryptionCache.cache[item.Id]; present && item.Updated.Equal(cache.updated) { + if cache, ok := kv.decryptionCache.cache[item.Id]; ok && item.Updated.Equal(cache.updated) { + kv.log.Debug("got secret value from decryption cache", "orgId", orgId, "type", typ, "namespace", namespace) return cache.value, isFound, err } @@ -82,6 +82,7 @@ func (kv *secretsKVStoreSQL) Get(ctx context.Context, orgId int64, namespace str } } + kv.log.Debug("got secret value", "orgId", orgId, "type", typ, "namespace", namespace) return string(decryptedValue), isFound, err } @@ -120,6 +121,8 @@ func (kv *secretsKVStoreSQL) Set(ctx context.Context, orgId int64, namespace str if err != nil { kv.log.Debug("error updating secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) } else { + kv.decryptionCache.Lock() + defer kv.decryptionCache.Unlock() kv.decryptionCache.cache[item.Id] = cachedDecrypted{ updated: item.Updated, value: value, @@ -162,6 +165,8 @@ func (kv *secretsKVStoreSQL) Del(ctx context.Context, orgId int64, namespace str if err != nil { kv.log.Debug("error deleting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) } else { + kv.decryptionCache.Lock() + defer kv.decryptionCache.Unlock() delete(kv.decryptionCache.cache, item.Id) kv.log.Debug("secret value deleted", "orgId", orgId, "type", typ, "namespace", namespace) }