diff --git a/pkg/infra/kvstore/kvstore_test.go b/pkg/infra/kvstore/kvstore_test.go index c35ded9dc21..6cfe6cb18ea 100644 --- a/pkg/infra/kvstore/kvstore_test.go +++ b/pkg/infra/kvstore/kvstore_test.go @@ -5,11 +5,10 @@ import ( "fmt" "testing" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func createTestableKVStore(t *testing.T) KVStore { diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 4c781ad3c14..119ee1f2613 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -297,6 +297,7 @@ var wireBasicSet = wire.NewSet( userimpl.ProvideService, orgimpl.ProvideService, datasourceservice.ProvideDataSourceMigrationService, + secretsStore.ProvidePluginSecretMigrationService, secretsMigrations.ProvideSecretMigrationService, wire.Bind(new(secretsMigrations.SecretMigrationService), new(*secretsMigrations.SecretMigrationServiceImpl)), userauthimpl.ProvideService, diff --git a/pkg/services/secrets/kvstore/helpers.go b/pkg/services/secrets/kvstore/helpers.go index c00923ca8ea..ab6089b965a 100644 --- a/pkg/services/secrets/kvstore/helpers.go +++ b/pkg/services/secrets/kvstore/helpers.go @@ -1,6 +1,7 @@ package kvstore import ( + "context" "testing" "github.com/grafana/grafana/pkg/infra/log" @@ -27,3 +28,54 @@ func SetupTestService(t *testing.T) SecretsKVStore { return kv } + +// In memory kv store used for testing +type FakeSecretsKVStore struct { + store map[Key]string +} + +func NewFakeSecretsKVStore() FakeSecretsKVStore { + return FakeSecretsKVStore{store: make(map[Key]string)} +} + +func (f FakeSecretsKVStore) Get(ctx context.Context, orgId int64, namespace string, typ string) (string, bool, error) { + value := f.store[buildKey(orgId, namespace, typ)] + found := value != "" + return value, found, nil +} + +func (f FakeSecretsKVStore) Set(ctx context.Context, orgId int64, namespace string, typ string, value string) error { + f.store[buildKey(orgId, namespace, typ)] = value + return nil +} + +func (f FakeSecretsKVStore) Del(ctx context.Context, orgId int64, namespace string, typ string) error { + delete(f.store, buildKey(orgId, namespace, typ)) + return nil +} + +func (f FakeSecretsKVStore) Keys(ctx context.Context, orgId int64, namespace string, typ string) ([]Key, error) { + res := make([]Key, 0) + for k := range f.store { + if k.OrgId == orgId && k.Namespace == namespace && k.Type == typ { + res = append(res, k) + } + } + return res, nil +} + +func (f FakeSecretsKVStore) Rename(ctx context.Context, orgId int64, namespace string, typ string, newNamespace string) error { + f.store[buildKey(orgId, newNamespace, typ)] = f.store[buildKey(orgId, namespace, typ)] + delete(f.store, buildKey(orgId, namespace, typ)) + return nil +} + +func buildKey(orgId int64, namespace string, typ string) Key { + return Key{ + OrgId: orgId, + Namespace: namespace, + Type: typ, + } +} + +var _ SecretsKVStore = FakeSecretsKVStore{} diff --git a/pkg/services/secrets/kvstore/kvstore.go b/pkg/services/secrets/kvstore/kvstore.go index 2a0f8a7f2af..377e014f9d2 100644 --- a/pkg/services/secrets/kvstore/kvstore.go +++ b/pkg/services/secrets/kvstore/kvstore.go @@ -37,8 +37,9 @@ func ProvideService(sqlStore sqlstore.Store, secretsService secrets.Service, rem log: logger, } } + } else { + logger.Debug("secrets kvstore is using the default (SQL) implementation for secrets management") } - logger.Debug("secrets kvstore is using the default (SQL) implementation for secrets management") return NewCachedKVStore(store, 5*time.Second, 5*time.Minute) } diff --git a/pkg/services/secrets/kvstore/migrations/migrator.go b/pkg/services/secrets/kvstore/migrations/migrator.go index 14c3c6f0b07..08dc9d20ada 100644 --- a/pkg/services/secrets/kvstore/migrations/migrator.go +++ b/pkg/services/secrets/kvstore/migrations/migrator.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/serverlock" datasources "github.com/grafana/grafana/pkg/services/datasources/service" + "github.com/grafana/grafana/pkg/services/secrets/kvstore" ) var logger = log.New("secret.migration") @@ -25,16 +26,20 @@ type SecretMigrationServiceImpl struct { func ProvideSecretMigrationService( serverLockService *serverlock.ServerLockService, dataSourceSecretMigrationService *datasources.DataSourceSecretMigrationService, + pluginSecretMigrationService *kvstore.PluginSecretMigrationService, ) *SecretMigrationServiceImpl { + services := make([]SecretMigrationService, 0) + services = append(services, dataSourceSecretMigrationService) + // pluginMigrationService should always be the last one + services = append(services, pluginSecretMigrationService) + return &SecretMigrationServiceImpl{ ServerLockService: serverLockService, - Services: []SecretMigrationService{ - dataSourceSecretMigrationService, - }, + Services: services, } } -// Run migration services. This will block until all services have exited. +// Migrate Run migration services. This will block until all services have exited. func (s *SecretMigrationServiceImpl) Migrate(ctx context.Context) error { // Start migration services. return s.ServerLockService.LockAndExecute(ctx, "migrate secrets to unified secrets", time.Minute*10, func(context.Context) { diff --git a/pkg/services/secrets/kvstore/plugin_mig.go b/pkg/services/secrets/kvstore/plugin_mig.go new file mode 100644 index 00000000000..73cd082eef8 --- /dev/null +++ b/pkg/services/secrets/kvstore/plugin_mig.go @@ -0,0 +1,75 @@ +package kvstore + +import ( + "context" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/secrets" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/setting" +) + +// PluginSecretMigrationService This migrator will handle migration of datasource secrets (aka Unified secrets) +// into the plugin secrets configured +type PluginSecretMigrationService struct { + secretsStore SecretsKVStore + cfg *setting.Cfg + logger log.Logger + sqlStore sqlstore.Store + secretsService secrets.Service + remoteCheck UseRemoteSecretsPluginCheck +} + +func ProvidePluginSecretMigrationService( + secretsStore SecretsKVStore, + cfg *setting.Cfg, + sqlStore sqlstore.Store, + secretsService secrets.Service, + remoteCheck UseRemoteSecretsPluginCheck, +) *PluginSecretMigrationService { + return &PluginSecretMigrationService{ + secretsStore: secretsStore, + cfg: cfg, + logger: log.New("sec-plugin-mig"), + sqlStore: sqlStore, + secretsService: secretsService, + remoteCheck: remoteCheck, + } +} + +func (s *PluginSecretMigrationService) Migrate(ctx context.Context) error { + // Check if we should migrate to plugin - default false + if s.cfg.SectionWithEnvOverrides("secrets").Key("migrate_to_plugin").MustBool(false) && + s.remoteCheck.ShouldUseRemoteSecretsPlugin() { + // we need to instantiate the secretsKVStore as this is not on wire, and in this scenario, + // the secrets store would be the plugin. + secretsSql := &secretsKVStoreSQL{ + sqlStore: s.sqlStore, + secretsService: s.secretsService, + log: s.logger, + decryptionCache: decryptionCache{ + cache: make(map[int64]cachedDecrypted), + }, + } + + allSec, err := secretsSql.GetAll(ctx) + if err != nil { + return nil + } + // We just set it again as the current secret store should be the plugin secret + for _, sec := range allSec { + err = s.secretsStore.Set(ctx, *sec.OrgId, *sec.Namespace, *sec.Type, sec.Value) + if err != nil { + return err + } + } + // as no err was returned, when we delete all the secrets from the sql store + for _, sec := range allSec { + err = secretsSql.Del(ctx, *sec.OrgId, *sec.Namespace, *sec.Type) + if err != nil { + return err + } + } + } + return nil +} diff --git a/pkg/services/secrets/kvstore/plugin_mig_test.go b/pkg/services/secrets/kvstore/plugin_mig_test.go new file mode 100644 index 00000000000..32350d1e0b2 --- /dev/null +++ b/pkg/services/secrets/kvstore/plugin_mig_test.go @@ -0,0 +1,121 @@ +package kvstore + +import ( + "context" + "testing" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/plugins/backendplugin/secretsmanagerplugin" + "github.com/grafana/grafana/pkg/services/secrets/fakes" + secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/require" + "gopkg.in/ini.v1" +) + +// This tests will create a mock sql database and an inmemory +// implementation of the secret manager to simulate the plugin. +func TestPluginSecretMigrationService_Migrate(t *testing.T) { + ctx := context.Background() + + t.Run("migration run ok - 2 secrets migrated", func(t *testing.T) { + // --- SETUP + migratorService, secretsStore, sqlSecretStore := setupTestMigratorService(t) + var orgId int64 = 1 + namespace1, namespace2 := "namespace-test", "namespace-test2" + typ := "type-test" + value := "SUPER_SECRET" + + addSecretToSqlStore(t, sqlSecretStore, ctx, orgId, namespace1, typ, value) + addSecretToSqlStore(t, sqlSecretStore, ctx, orgId, namespace2, typ, value) + + // --- EXECUTION + err := migratorService.Migrate(ctx) + require.NoError(t, err) + + // --- VALIDATIONS + validateSecretWasDeleted(t, sqlSecretStore, ctx, orgId, namespace1, typ) + validateSecretWasDeleted(t, sqlSecretStore, ctx, orgId, namespace2, typ) + + validateSecretWasStoreInPlugin(t, secretsStore, ctx, orgId, namespace1, typ) + validateSecretWasStoreInPlugin(t, secretsStore, ctx, orgId, namespace1, typ) + }) +} + +func addSecretToSqlStore(t *testing.T, sqlSecretStore *secretsKVStoreSQL, ctx context.Context, orgId int64, namespace1 string, typ string, value string) { + err := sqlSecretStore.Set(ctx, orgId, namespace1, typ, value) + require.NoError(t, err) +} + +// validates that secrets on the sql store were deleted. +func validateSecretWasDeleted(t *testing.T, sqlSecretStore *secretsKVStoreSQL, ctx context.Context, orgId int64, namespace1 string, typ string) { + res, err := sqlSecretStore.Keys(ctx, orgId, namespace1, typ) + require.NoError(t, err) + require.Equal(t, 0, len(res)) +} + +// validates that secrets should be on the plugin +func validateSecretWasStoreInPlugin(t *testing.T, secretsStore SecretsKVStore, ctx context.Context, orgId int64, namespace1 string, typ string) { + resPlugin, err := secretsStore.Keys(ctx, orgId, namespace1, typ) + require.NoError(t, err) + require.Equal(t, 1, len(resPlugin)) +} + +// +func setupTestMigratorService(t *testing.T) (*PluginSecretMigrationService, SecretsKVStore, *secretsKVStoreSQL) { + t.Helper() + + rawCfg := ` + [secrets] + use_plugin = true + migrate_to_plugin = true + ` + raw, err := ini.Load([]byte(rawCfg)) + require.NoError(t, err) + cfg := &setting.Cfg{Raw: raw} + // this would be the plugin - mocked at the moment + secretsStoreForPlugin := NewFakeSecretsKVStore() + // Mocked remote plugin check, always return true + remoteCheck := provideMockRemotePluginCheck() + + // this is to init the sql secret store inside the migration + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + + migratorService := ProvidePluginSecretMigrationService( + secretsStoreForPlugin, + cfg, + sqlStore, + secretsService, + remoteCheck, + ) + + secretsSql := &secretsKVStoreSQL{ + sqlStore: sqlStore, + secretsService: secretsService, + log: log.New("test.logger"), + decryptionCache: decryptionCache{ + cache: make(map[int64]cachedDecrypted), + }, + } + + return migratorService, secretsStoreForPlugin, secretsSql +} + +// +type mockRemoteSecretsPluginCheck struct { + UseRemoteSecretsPluginCheck +} + +func provideMockRemotePluginCheck() *mockRemoteSecretsPluginCheck { + return &mockRemoteSecretsPluginCheck{} +} + +func (c *mockRemoteSecretsPluginCheck) ShouldUseRemoteSecretsPlugin() bool { + return true +} + +func (c *mockRemoteSecretsPluginCheck) GetPlugin() (secretsmanagerplugin.SecretsManagerPlugin, error) { + return nil, nil +} diff --git a/pkg/services/secrets/kvstore/sql.go b/pkg/services/secrets/kvstore/sql.go index ef64f29463a..0c613629933 100644 --- a/pkg/services/secrets/kvstore/sql.go +++ b/pkg/services/secrets/kvstore/sql.go @@ -44,11 +44,11 @@ func (kv *secretsKVStoreSQL) Get(ctx context.Context, orgId int64, namespace str err := kv.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { has, err := dbSession.Get(&item) if err != nil { - kv.log.Debug("error getting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) + kv.log.Error("error getting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) return err } if !has { - kv.log.Debug("secret value not found", "orgId", orgId, "type", typ, "namespace", namespace) + kv.log.Error("secret value not found", "orgId", orgId, "type", typ, "namespace", namespace) return nil } isFound = true @@ -66,13 +66,13 @@ func (kv *secretsKVStoreSQL) Get(ctx context.Context, orgId int64, namespace str decodedValue, err := b64.DecodeString(item.Value) if err != nil { - kv.log.Debug("error decoding secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) + kv.log.Error("error decoding secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) return string(decryptedValue), isFound, err } decryptedValue, err = kv.secretsService.Decrypt(ctx, decodedValue) if err != nil { - kv.log.Debug("error decrypting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) + kv.log.Error("error decrypting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) return string(decryptedValue), isFound, err } @@ -90,7 +90,7 @@ func (kv *secretsKVStoreSQL) Get(ctx context.Context, orgId int64, namespace str func (kv *secretsKVStoreSQL) Set(ctx context.Context, orgId int64, namespace string, typ string, value string) error { encryptedValue, err := kv.secretsService.Encrypt(ctx, []byte(value), secrets.WithoutScope()) if err != nil { - kv.log.Debug("error encrypting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) + kv.log.Error("error encrypting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) return err } encodedValue := b64.EncodeToString(encryptedValue) @@ -103,7 +103,7 @@ func (kv *secretsKVStoreSQL) Set(ctx context.Context, orgId int64, namespace str has, err := dbSession.Get(&item) if err != nil { - kv.log.Debug("error checking secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) + kv.log.Error("error checking secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) return err } @@ -119,7 +119,7 @@ func (kv *secretsKVStoreSQL) Set(ctx context.Context, orgId int64, namespace str // if item already exists we update it _, err = dbSession.ID(item.Id).Update(&item) if err != nil { - kv.log.Debug("error updating secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) + kv.log.Error("error updating secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) } else { kv.decryptionCache.Lock() defer kv.decryptionCache.Unlock() @@ -136,7 +136,7 @@ func (kv *secretsKVStoreSQL) Set(ctx context.Context, orgId int64, namespace str item.Created = item.Updated _, err = dbSession.Insert(&item) if err != nil { - kv.log.Debug("error inserting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) + kv.log.Error("error inserting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) } else { kv.log.Debug("secret value inserted", "orgId", orgId, "type", typ, "namespace", namespace) } @@ -155,7 +155,7 @@ func (kv *secretsKVStoreSQL) Del(ctx context.Context, orgId int64, namespace str has, err := dbSession.Get(&item) if err != nil { - kv.log.Debug("error checking secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) + kv.log.Error("error checking secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) return err } @@ -163,7 +163,7 @@ func (kv *secretsKVStoreSQL) Del(ctx context.Context, orgId int64, namespace str // if item exists we delete it _, err = dbSession.ID(item.Id).Delete(&item) if err != nil { - kv.log.Debug("error deleting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) + kv.log.Error("error deleting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) } else { kv.decryptionCache.Lock() defer kv.decryptionCache.Unlock() @@ -202,7 +202,7 @@ func (kv *secretsKVStoreSQL) Rename(ctx context.Context, orgId int64, namespace has, err := dbSession.Get(&item) if err != nil { - kv.log.Debug("error checking secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) + kv.log.Error("error checking secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) return err } @@ -213,7 +213,7 @@ func (kv *secretsKVStoreSQL) Rename(ctx context.Context, orgId int64, namespace // if item already exists we update it _, err = dbSession.ID(item.Id).Update(&item) if err != nil { - kv.log.Debug("error updating secret namespace", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) + kv.log.Error("error updating secret namespace", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) } else { kv.log.Debug("secret namespace updated", "orgId", orgId, "type", typ, "namespace", namespace) } @@ -223,3 +223,50 @@ func (kv *secretsKVStoreSQL) Rename(ctx context.Context, orgId int64, namespace return err }) } + +// GetAll this returns all the secrets stored in the database. This is not part of the kvstore interface as we +// only need it for migration from sql to plugin at this moment +func (kv *secretsKVStoreSQL) GetAll(ctx context.Context) ([]Item, error) { + var items []Item + err := kv.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { + return dbSession.Find(&items) + }) + if err != nil { + kv.log.Error("error getting all the items", "err", err) + return nil, err + } + + // decrypting value + kv.decryptionCache.Lock() + defer kv.decryptionCache.Unlock() + for i := range items { + var decryptedValue []byte + if cache, ok := kv.decryptionCache.cache[items[i].Id]; ok && items[i].Updated.Equal(cache.updated) { + kv.log.Debug("got secret value from decryption cache", "orgId", items[i].OrgId, "type", items[i].Type, "namespace", items[i].Namespace) + items[i].Value = cache.value + continue + } + + decodedValue, err := b64.DecodeString(items[i].Value) + if err != nil { + kv.log.Error("error decoding secret value", "orgId", items[i].OrgId, "type", items[i].Type, "namespace", items[i].Namespace, "err", err) + items[i].Value = string(decryptedValue) + continue + } + + decryptedValue, err = kv.secretsService.Decrypt(ctx, decodedValue) + if err != nil { + kv.log.Error("error decrypting secret value", "orgId", items[i].OrgId, "type", items[i].Type, "namespace", items[i].Namespace, "err", err) + items[i].Value = string(decryptedValue) + continue + } + + items[i].Value = string(decryptedValue) + kv.decryptionCache.cache[items[i].Id] = cachedDecrypted{ + updated: items[i].Updated, + value: string(decryptedValue), + } + } + + return items, err +} diff --git a/pkg/services/secrets/kvstore/kvstore_test.go b/pkg/services/secrets/kvstore/sql_test.go similarity index 70% rename from pkg/services/secrets/kvstore/kvstore_test.go rename to pkg/services/secrets/kvstore/sql_test.go index 9b6bff4a371..48604955fc2 100644 --- a/pkg/services/secrets/kvstore/kvstore_test.go +++ b/pkg/services/secrets/kvstore/sql_test.go @@ -5,6 +5,10 @@ import ( "fmt" "testing" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/secrets/database" + "github.com/grafana/grafana/pkg/services/secrets/manager" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -20,8 +24,27 @@ func (t *TestCase) Value() string { return fmt.Sprintf("%d:%s:%s:%d", t.OrgId, t.Namespace, t.Type, t.Revision) } -func TestKVStore(t *testing.T) { - kv := SetupTestService(t) +func setupTestService(t *testing.T) *secretsKVStoreSQL { + t.Helper() + + sqlStore := sqlstore.InitTestDB(t) + store := database.ProvideSecretsStore(sqlstore.InitTestDB(t)) + secretsService := manager.SetupTestService(t, store) + + kv := &secretsKVStoreSQL{ + sqlStore: sqlStore, + log: log.New("secrets.kvstore"), + secretsService: secretsService, + decryptionCache: decryptionCache{ + cache: make(map[int64]cachedDecrypted), + }, + } + + return kv +} + +func TestSecretsKVStoreSQL(t *testing.T) { + kv := setupTestService(t) ctx := context.Background() @@ -152,7 +175,7 @@ func TestKVStore(t *testing.T) { }) t.Run("listing existing keys", func(t *testing.T) { - kv := SetupTestService(t) + kv := setupTestService(t) ctx := context.Background() @@ -223,4 +246,71 @@ func TestKVStore(t *testing.T) { require.NoError(t, err, "querying a not existing namespace should not throw an error") require.Len(t, keys, 0, "querying a not existing namespace should return an empty slice") }) + + t.Run("getting all secrets", func(t *testing.T) { + kv := setupTestService(t) + + ctx := context.Background() + + namespace, typ := "listtest", "listtest" + + testCases := []*TestCase{ + { + OrgId: 1, + Type: typ, + Namespace: namespace, + }, + { + OrgId: 2, + Type: typ, + Namespace: namespace, + }, + { + OrgId: 3, + Type: typ, + Namespace: namespace, + }, + { + OrgId: 4, + Type: typ, + Namespace: namespace, + }, + { + OrgId: 1, + Type: typ, + Namespace: "other_key", + }, + { + OrgId: 4, + Type: typ, + Namespace: "another_one", + }, + } + + for _, tc := range testCases { + err := kv.Set(ctx, tc.OrgId, tc.Namespace, tc.Type, tc.Value()) + require.NoError(t, err) + } + + secrets, err := kv.GetAll(ctx) + + require.NoError(t, err) + require.Len(t, secrets, 6) + + found := 0 + + for _, s := range secrets { + for _, tc := range testCases { + if *s.OrgId == tc.OrgId && + *s.Namespace == tc.Namespace && + *s.Type == tc.Type { + require.Equal(t, tc.Value(), s.Value, "secret found but value is not equals") + found++ + break + } + } + } + + require.Equal(t, 6, found, "querying for all secrets should return 6 records") + }) }