From f25c7f6ddd41c0f3ebf023517e73f02e065d3e79 Mon Sep 17 00:00:00 2001 From: Guilherme Caulada Date: Thu, 25 Aug 2022 18:04:44 -0300 Subject: [PATCH] Chore: Refactor secrets kvstore to organize testing and migrations (#54249) * Refactor migrations and tests for secrets kvstore * Use fake secrets store as a shortcut on tests * Update wire * Use global migration logger * Fix ds proxy tests * Fix linting issues * Rename data source test setup function --- pkg/api/pluginproxy/ds_proxy_test.go | 110 ++++++----- pkg/cmd/grafana-cli/runner/wire.go | 4 +- pkg/server/wire.go | 6 +- .../datasources/service/datasource_service.go | 12 +- .../service/datasource_service_test.go | 69 ++++--- pkg/services/query/query_test.go | 14 +- pkg/services/secrets/kvstore/kvstore.go | 15 +- .../kvstore/migrations/datasource_mig.go} | 14 +- .../migrations/datasource_mig_test.go} | 78 ++++---- .../from_plugin_mig.go} | 46 ++--- .../from_plugin_mig_test.go} | 18 +- .../secrets/kvstore/migrations/migrator.go | 12 +- .../to_plugin_mig.go} | 38 ++-- .../to_plugin_mig_test.go} | 75 +++++-- pkg/services/secrets/kvstore/model.go | 1 + .../kvstore/{remote_plugin.go => plugin.go} | 12 +- .../kvstore/plugin_fatal_crash_test.go | 184 ------------------ pkg/services/secrets/kvstore/plugin_test.go | 74 +++++++ pkg/services/secrets/kvstore/sql.go | 33 ++-- pkg/services/secrets/kvstore/sql_test.go | 33 +--- pkg/services/secrets/kvstore/test_helpers.go | 85 +++++--- pkg/tsdb/legacydata/service/service_test.go | 11 +- 22 files changed, 468 insertions(+), 476 deletions(-) rename pkg/services/{datasources/service/secrets_mig.go => secrets/kvstore/migrations/datasource_mig.go} (87%) rename pkg/services/{datasources/service/secrets_mig_test.go => secrets/kvstore/migrations/datasource_mig_test.go} (77%) rename pkg/services/secrets/kvstore/{migrate_from_plugin.go => migrations/from_plugin_mig.go} (61%) rename pkg/services/secrets/kvstore/{migrate_from_plugin_test.go => migrations/from_plugin_mig_test.go} (87%) rename pkg/services/secrets/kvstore/{migrate_to_plugin.go => migrations/to_plugin_mig.go} (61%) rename pkg/services/secrets/kvstore/{migrate_to_plugin_test.go => migrations/to_plugin_mig_test.go} (53%) rename pkg/services/secrets/kvstore/{remote_plugin.go => plugin.go} (95%) delete mode 100644 pkg/services/secrets/kvstore/plugin_fatal_crash_test.go create mode 100644 pkg/services/secrets/kvstore/plugin_test.go diff --git a/pkg/api/pluginproxy/ds_proxy_test.go b/pkg/api/pluginproxy/ds_proxy_test.go index 04b9fffd670..fdbe5138d36 100644 --- a/pkg/api/pluginproxy/ds_proxy_test.go +++ b/pkg/api/pluginproxy/ds_proxy_test.go @@ -21,6 +21,7 @@ import ( "github.com/grafana/grafana/pkg/api/datasource" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/httpclient" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins" @@ -32,8 +33,9 @@ import ( "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/services/secrets/fakes" - "github.com/grafana/grafana/pkg/services/secrets/kvstore" - secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" + secretskvs "github.com/grafana/grafana/pkg/services/secrets/kvstore" + secretsmng "github.com/grafana/grafana/pkg/services/secrets/manager" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" @@ -96,8 +98,9 @@ func TestDataSourceProxy_routeRule(t *testing.T) { }) setting.SecretKey = "password" //nolint:goconst - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) key, err := secretsService.Encrypt(context.Background(), []byte("123"), secrets.WithoutScope()) require.NoError(t, err) @@ -249,8 +252,9 @@ func TestDataSourceProxy_routeRule(t *testing.T) { }) setting.SecretKey = "password" - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) key, err := secretsService.Encrypt(context.Background(), []byte("123"), secrets.WithoutScope()) require.NoError(t, err) @@ -348,8 +352,9 @@ func TestDataSourceProxy_routeRule(t *testing.T) { ds := &datasources.DataSource{Url: "htttp://graphite:8080", Type: datasources.DS_GRAPHITE} ctx := &models.ReqContext{} - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "/render", &setting.Cfg{BuildVersion: "5.3.0"}, httpClientProvider, &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -374,8 +379,9 @@ func TestDataSourceProxy_routeRule(t *testing.T) { ctx := &models.ReqContext{} var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -399,8 +405,9 @@ func TestDataSourceProxy_routeRule(t *testing.T) { ctx := &models.ReqContext{} var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -428,8 +435,9 @@ func TestDataSourceProxy_routeRule(t *testing.T) { ctx := &models.ReqContext{} var pluginRoutes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, pluginRoutes, ctx, "", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -452,8 +460,9 @@ func TestDataSourceProxy_routeRule(t *testing.T) { } ctx := &models.ReqContext{} var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "/path/to/folder/", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -502,8 +511,9 @@ func TestDataSourceProxy_routeRule(t *testing.T) { } var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "/path/to/folder/", &setting.Cfg{}, httpClientProvider, &mockAuthToken, dsService, tracer) require.NoError(t, err) @@ -556,8 +566,9 @@ func TestDataSourceProxy_routeRule(t *testing.T) { }) t.Run("When proxying data source proxy should handle authentication", func(t *testing.T) { - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) tests := []*testCase{ createAuthTest(t, secretsStore, datasources.DS_INFLUXDB_08, "http://localhost:9090", authTypePassword, authCheckQuery), @@ -637,8 +648,9 @@ func TestDataSourceProxy_requestHandling(t *testing.T) { t.Run("When response header Set-Cookie is not set should remove proxied Set-Cookie header", func(t *testing.T) { ctx, ds := setUp(t) var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "/render", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -656,8 +668,9 @@ func TestDataSourceProxy_requestHandling(t *testing.T) { }, }) var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "/render", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -671,8 +684,9 @@ func TestDataSourceProxy_requestHandling(t *testing.T) { t.Run("When response should set Content-Security-Policy header", func(t *testing.T) { ctx, ds := setUp(t) var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "/render", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -694,8 +708,9 @@ func TestDataSourceProxy_requestHandling(t *testing.T) { }, }) var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "/render", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -720,8 +735,9 @@ func TestDataSourceProxy_requestHandling(t *testing.T) { ctx.Req = httptest.NewRequest("GET", "/api/datasources/proxy/1/path/%2Ftest%2Ftest%2F?query=%2Ftest%2Ftest%2F", nil) var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "/path/%2Ftest%2Ftest%2F", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -745,8 +761,9 @@ func TestDataSourceProxy_requestHandling(t *testing.T) { ctx.Req = httptest.NewRequest("GET", "/api/datasources/proxy/1/path/%2Ftest%2Ftest%2F?query=%2Ftest%2Ftest%2F", nil) var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "/path/%2Ftest%2Ftest%2F", &setting.Cfg{}, httpClientProvider, &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -770,8 +787,9 @@ func TestNewDataSourceProxy_InvalidURL(t *testing.T) { cfg := &setting.Cfg{} tracer := tracing.InitializeTracerForTest() var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) _, err := NewDataSourceProxy(&ds, routes, &ctx, "api/method", cfg, httpclient.NewProvider(), &oauthtoken.Service{}, dsService, tracer) require.Error(t, err) @@ -791,8 +809,9 @@ func TestNewDataSourceProxy_ProtocolLessURL(t *testing.T) { tracer := tracing.InitializeTracerForTest() var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) _, err := NewDataSourceProxy(&ds, routes, &ctx, "api/method", cfg, httpclient.NewProvider(), &oauthtoken.Service{}, dsService, tracer) @@ -834,8 +853,9 @@ func TestNewDataSourceProxy_MSSQL(t *testing.T) { } var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) p, err := NewDataSourceProxy(&ds, routes, &ctx, "api/method", cfg, httpclient.NewProvider(), &oauthtoken.Service{}, dsService, tracer) if tc.err == nil { @@ -861,8 +881,9 @@ func getDatasourceProxiedRequest(t *testing.T, ctx *models.ReqContext, cfg *sett tracer := tracing.InitializeTracerForTest() var routes []*plugins.Route - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(ds, routes, ctx, "", cfg, httpclient.NewProvider(), &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) @@ -916,7 +937,7 @@ const ( authCheckHeader = "header" ) -func createAuthTest(t *testing.T, secretsStore kvstore.SecretsKVStore, dsType string, url string, authType string, authCheck string) *testCase { +func createAuthTest(t *testing.T, secretsStore secretskvs.SecretsKVStore, dsType string, url string, authType string, authCheck string) *testCase { // Basic user:password base64AuthHeader := "Basic dXNlcjpwYXNzd29yZA==" @@ -975,7 +996,7 @@ func createAuthTest(t *testing.T, secretsStore kvstore.SecretsKVStore, dsType st return test } -func runDatasourceAuthTest(t *testing.T, secretsService secrets.Service, secretsStore kvstore.SecretsKVStore, cfg *setting.Cfg, test *testCase) { +func runDatasourceAuthTest(t *testing.T, secretsService secrets.Service, secretsStore secretskvs.SecretsKVStore, cfg *setting.Cfg, test *testCase) { ctx := &models.ReqContext{} tracer := tracing.InitializeTracerForTest() @@ -1021,8 +1042,9 @@ func Test_PathCheck(t *testing.T) { return ctx, req } ctx, _ := setUp() - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) proxy, err := NewDataSourceProxy(&datasources.DataSource{}, routes, ctx, "b", &setting.Cfg{}, httpclient.NewProvider(), &oauthtoken.Service{}, dsService, tracer) require.NoError(t, err) diff --git a/pkg/cmd/grafana-cli/runner/wire.go b/pkg/cmd/grafana-cli/runner/wire.go index f8da47e9c8f..56e5e497553 100644 --- a/pkg/cmd/grafana-cli/runner/wire.go +++ b/pkg/cmd/grafana-cli/runner/wire.go @@ -306,8 +306,8 @@ var wireSet = wire.NewSet( publicdashboardsApi.ProvideApi, userimpl.ProvideService, orgimpl.ProvideService, - datasourceservice.ProvideDataSourceMigrationService, - secretsStore.ProvideMigrateToPluginService, + secretsMigrations.ProvideDataSourceMigrationService, + secretsMigrations.ProvideMigrateToPluginService, secretsMigrations.ProvideSecretMigrationService, wire.Bind(new(secretsMigrations.SecretMigrationService), new(*secretsMigrations.SecretMigrationServiceImpl)), userauthimpl.ProvideService, diff --git a/pkg/server/wire.go b/pkg/server/wire.go index ca05dff73f7..8cde320c7fd 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -318,9 +318,9 @@ var wireBasicSet = wire.NewSet( orgimpl.ProvideService, tempuserimpl.ProvideService, loginattemptimpl.ProvideService, - datasourceservice.ProvideDataSourceMigrationService, - secretsStore.ProvideMigrateToPluginService, - secretsStore.ProvideMigrateFromPluginService, + secretsMigrations.ProvideDataSourceMigrationService, + secretsMigrations.ProvideMigrateToPluginService, + secretsMigrations.ProvideMigrateFromPluginService, secretsMigrations.ProvideSecretMigrationService, wire.Bind(new(secretsMigrations.SecretMigrationService), new(*secretsMigrations.SecretMigrationServiceImpl)), userauthimpl.ProvideService, diff --git a/pkg/services/datasources/service/datasource_service.go b/pkg/services/datasources/service/datasource_service.go index 35ee939321a..b98f62d68e1 100644 --- a/pkg/services/datasources/service/datasource_service.go +++ b/pkg/services/datasources/service/datasource_service.go @@ -78,8 +78,6 @@ type DataSourceRetriever interface { GetDataSource(ctx context.Context, query *datasources.GetDataSourceQuery) error } -const secretType = "datasource" - // NewNameScopeResolver provides an ScopeAttributeResolver able to // translate a scope prefixed with "datasources:name:" into an uid based scope. func NewNameScopeResolver(db DataSourceRetriever) (string, accesscontrol.ScopeAttributeResolver) { @@ -165,7 +163,7 @@ func (s *Service) AddDataSource(ctx context.Context, cmd *datasources.AddDataSou return err } - return s.SecretsStore.Set(ctx, cmd.OrgId, cmd.Name, secretType, string(secret)) + return s.SecretsStore.Set(ctx, cmd.OrgId, cmd.Name, kvstore.DataSourceSecretType, string(secret)) } if err := s.SQLStore.AddDataSource(ctx, cmd); err != nil { @@ -197,7 +195,7 @@ func (s *Service) AddDataSource(ctx context.Context, cmd *datasources.AddDataSou func (s *Service) DeleteDataSource(ctx context.Context, cmd *datasources.DeleteDataSourceCommand) error { return s.SQLStore.InTransaction(ctx, func(ctx context.Context) error { cmd.UpdateSecretFn = func() error { - return s.SecretsStore.Del(ctx, cmd.OrgID, cmd.Name, secretType) + return s.SecretsStore.Del(ctx, cmd.OrgID, cmd.Name, kvstore.DataSourceSecretType) } return s.SQLStore.DeleteDataSource(ctx, cmd) @@ -230,13 +228,13 @@ func (s *Service) UpdateDataSource(ctx context.Context, cmd *datasources.UpdateD } if query.Result.Name != cmd.Name { - err := s.SecretsStore.Rename(ctx, cmd.OrgId, query.Result.Name, secretType, cmd.Name) + err := s.SecretsStore.Rename(ctx, cmd.OrgId, query.Result.Name, kvstore.DataSourceSecretType, cmd.Name) if err != nil { return err } } - return s.SecretsStore.Set(ctx, cmd.OrgId, cmd.Name, secretType, string(secret)) + return s.SecretsStore.Set(ctx, cmd.OrgId, cmd.Name, kvstore.DataSourceSecretType, string(secret)) } } @@ -299,7 +297,7 @@ func (s *Service) GetTLSConfig(ctx context.Context, ds *datasources.DataSource, func (s *Service) DecryptedValues(ctx context.Context, ds *datasources.DataSource) (map[string]string, error) { decryptedValues := make(map[string]string) - secret, exist, err := s.SecretsStore.Get(ctx, ds.OrgId, ds.Name, secretType) + secret, exist, err := s.SecretsStore.Get(ctx, ds.OrgId, ds.Name, kvstore.DataSourceSecretType) if err != nil { return nil, err } diff --git a/pkg/services/datasources/service/datasource_service_test.go b/pkg/services/datasources/service/datasource_service_test.go index b29974fc695..f45bcf994f2 100644 --- a/pkg/services/datasources/service/datasource_service_test.go +++ b/pkg/services/datasources/service/datasource_service_test.go @@ -15,14 +15,16 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/httpclient" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/services/secrets/fakes" - "github.com/grafana/grafana/pkg/services/secrets/kvstore" - secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" + secretskvs "github.com/grafana/grafana/pkg/services/secrets/kvstore" + secretsmng "github.com/grafana/grafana/pkg/services/secrets/manager" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" ) @@ -195,8 +197,9 @@ func TestService_GetHttpTransport(t *testing.T) { Type: "Kubernetes", } - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) rt1, err := dsService.GetHTTPTransport(context.Background(), &ds, provider) @@ -229,8 +232,9 @@ func TestService_GetHttpTransport(t *testing.T) { sjson := simplejson.New() sjson.Set("tlsAuthWithCACert", true) - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) ds := datasources.DataSource{ @@ -277,8 +281,9 @@ func TestService_GetHttpTransport(t *testing.T) { sjson := simplejson.New() sjson.Set("tlsAuth", true) - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) ds := datasources.DataSource{ @@ -296,7 +301,7 @@ func TestService_GetHttpTransport(t *testing.T) { }) require.NoError(t, err) - err = secretsStore.Set(context.Background(), ds.OrgId, ds.Name, secretType, string(secureJsonData)) + err = secretsStore.Set(context.Background(), ds.OrgId, ds.Name, secretskvs.DataSourceSecretType, string(secureJsonData)) require.NoError(t, err) rt, err := dsService.GetHTTPTransport(context.Background(), &ds, provider) @@ -322,8 +327,9 @@ func TestService_GetHttpTransport(t *testing.T) { sjson.Set("tlsAuthWithCACert", true) sjson.Set("serverName", "server-name") - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) ds := datasources.DataSource{ @@ -340,7 +346,7 @@ func TestService_GetHttpTransport(t *testing.T) { }) require.NoError(t, err) - err = secretsStore.Set(context.Background(), ds.OrgId, ds.Name, secretType, string(secureJsonData)) + err = secretsStore.Set(context.Background(), ds.OrgId, ds.Name, secretskvs.DataSourceSecretType, string(secureJsonData)) require.NoError(t, err) rt, err := dsService.GetHTTPTransport(context.Background(), &ds, provider) @@ -364,8 +370,9 @@ func TestService_GetHttpTransport(t *testing.T) { sjson := simplejson.New() sjson.Set("tlsSkipVerify", true) - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) ds := datasources.DataSource{ @@ -396,8 +403,9 @@ func TestService_GetHttpTransport(t *testing.T) { "httpHeaderName1": "Authorization", }) - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) ds := datasources.DataSource{ @@ -414,7 +422,7 @@ func TestService_GetHttpTransport(t *testing.T) { }) require.NoError(t, err) - err = secretsStore.Set(context.Background(), ds.OrgId, ds.Name, secretType, string(secureJsonData)) + err = secretsStore.Set(context.Background(), ds.OrgId, ds.Name, secretskvs.DataSourceSecretType, string(secureJsonData)) require.NoError(t, err) headers := dsService.getCustomHeaders(sjson, map[string]string{"httpHeaderValue1": "Bearer xf5yhfkpsnmgo"}) @@ -462,8 +470,9 @@ func TestService_GetHttpTransport(t *testing.T) { "timeout": 19, }) - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) ds := datasources.DataSource{ @@ -496,8 +505,9 @@ func TestService_GetHttpTransport(t *testing.T) { sjson, err := simplejson.NewJson([]byte(`{ "sigV4Auth": true }`)) require.NoError(t, err) - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) ds := datasources.DataSource{ @@ -532,8 +542,9 @@ func TestService_getTimeout(t *testing.T) { {jsonData: simplejson.NewFromAny(map[string]interface{}{"timeout": "2"}), expectedTimeout: 2 * time.Second}, } - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) for _, tc := range testCases { @@ -552,8 +563,9 @@ func TestService_GetDecryptedValues(t *testing.T) { Type: "prometheus", } - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := ProvideService(nil, secretsService, secretsStore, nil, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) jsonData := map[string]string{ @@ -577,8 +589,9 @@ func TestService_GetDecryptedValues(t *testing.T) { Type: "prometheus", } - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) dsService := ProvideService(nil, secretsService, secretsStore, nil, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) jsonData := map[string]string{ @@ -587,7 +600,7 @@ func TestService_GetDecryptedValues(t *testing.T) { jsonString, err := json.Marshal(jsonData) require.NoError(t, err) - err = secretsStore.Set(context.Background(), ds.OrgId, ds.Name, secretType, string(jsonString)) + err = secretsStore.Set(context.Background(), ds.OrgId, ds.Name, secretskvs.DataSourceSecretType, string(jsonString)) require.NoError(t, err) values, err := dsService.DecryptedValues(context.Background(), ds) diff --git a/pkg/services/query/query_test.go b/pkg/services/query/query_test.go index 567d9b87f48..e0db5a3467b 100644 --- a/pkg/services/query/query_test.go +++ b/pkg/services/query/query_test.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/plugins" acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/datasources" @@ -19,8 +20,9 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/query" "github.com/grafana/grafana/pkg/services/secrets/fakes" - "github.com/grafana/grafana/pkg/services/secrets/kvstore" - secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" + secretskvs "github.com/grafana/grafana/pkg/services/secrets/kvstore" + secretsmng "github.com/grafana/grafana/pkg/services/secrets/manager" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" ) @@ -106,8 +108,10 @@ func setup(t *testing.T) *testContext { tc := &fakeOAuthTokenService{} rv := &fakePluginRequestValidator{} - ss := kvstore.SetupTestService(t) - ssvc := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + ss := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) + ssvc := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) ds := dsSvc.ProvideService(nil, ssvc, ss, nil, featuremgmt.WithFeatures(), acmock.New(), acmock.NewMockedPermissionsService()) return &testContext{ @@ -122,7 +126,7 @@ func setup(t *testing.T) *testContext { type testContext struct { pluginContext *fakePluginClient - secretStore kvstore.SecretsKVStore + secretStore secretskvs.SecretsKVStore dataSourceCache *fakeDataSourceCache oauthTokenService *fakeOAuthTokenService pluginRequestValidator *fakePluginRequestValidator diff --git a/pkg/services/secrets/kvstore/kvstore.go b/pkg/services/secrets/kvstore/kvstore.go index 09b7b2b767d..85fd583794d 100644 --- a/pkg/services/secrets/kvstore/kvstore.go +++ b/pkg/services/secrets/kvstore/kvstore.go @@ -29,25 +29,18 @@ func ProvideService( ) (SecretsKVStore, error) { var logger = log.New("secrets.kvstore") var store SecretsKVStore - store = &secretsKVStoreSQL{ - sqlStore: sqlStore, - secretsService: secretsService, - log: logger, - decryptionCache: decryptionCache{ - cache: make(map[int64]cachedDecrypted), - }, - } + store = NewSQLSecretsKVStore(sqlStore, secretsService, logger) err := EvaluateRemoteSecretsPlugin(pluginsManager, cfg) if err != nil { logger.Debug(err.Error()) } else { // Attempt to start the plugin var secretsPlugin secretsmanagerplugin.SecretsManagerPlugin - secretsPlugin, err = startAndReturnPlugin(pluginsManager, context.Background()) + secretsPlugin, err = StartAndReturnPlugin(pluginsManager, context.Background()) namespacedKVStore := GetNamespacedKVStore(kvstore) if err != nil || secretsPlugin == nil { logger.Error("failed to start remote secrets management plugin", "msg", err.Error()) - if isFatal, readErr := isPluginStartupErrorFatal(context.Background(), namespacedKVStore); isFatal || readErr != nil { + if isFatal, readErr := IsPluginStartupErrorFatal(context.Background(), namespacedKVStore); isFatal || readErr != nil { // plugin error was fatal or there was an error determining if the error was fatal logger.Error("secrets management plugin is required to start -- exiting app") if readErr != nil { @@ -56,7 +49,7 @@ func ProvideService( return nil, err } } else { - // as the plugin is installed, secretsKVStoreSQL is now replaced with + // as the plugin is installed, SecretsKVStoreSQL is now replaced with // an instance of secretsKVStorePlugin with the sql store as a fallback // (used for migration and in case a secret is not found). store = &secretsKVStorePlugin{ diff --git a/pkg/services/datasources/service/secrets_mig.go b/pkg/services/secrets/kvstore/migrations/datasource_mig.go similarity index 87% rename from pkg/services/datasources/service/secrets_mig.go rename to pkg/services/secrets/kvstore/migrations/datasource_mig.go index 84fa2bcc739..71f9d74cd1c 100644 --- a/pkg/services/datasources/service/secrets_mig.go +++ b/pkg/services/secrets/kvstore/migrations/datasource_mig.go @@ -1,13 +1,13 @@ -package service +package migrations import ( "context" "fmt" "github.com/grafana/grafana/pkg/infra/kvstore" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" + secretskvs "github.com/grafana/grafana/pkg/services/secrets/kvstore" ) const ( @@ -23,7 +23,6 @@ type DataSourceSecretMigrationService struct { dataSourcesService datasources.DataSourceService kvStore *kvstore.NamespacedKVStore features featuremgmt.FeatureToggles - log log.Logger } func ProvideDataSourceMigrationService( @@ -33,9 +32,8 @@ func ProvideDataSourceMigrationService( ) *DataSourceSecretMigrationService { return &DataSourceSecretMigrationService{ dataSourcesService: dataSourcesService, - kvStore: kvstore.WithNamespace(kvStore, 0, secretType), + kvStore: kvstore.WithNamespace(kvStore, 0, secretskvs.DataSourceSecretType), features: features, - log: log.New("secrets.migration"), } } @@ -44,7 +42,7 @@ func (s *DataSourceSecretMigrationService) Migrate(ctx context.Context) error { if err != nil { return err } - s.log.Debug(fmt.Sprint("secret migration status is ", migrationStatus)) + logger.Debug(fmt.Sprint("secret migration status is ", migrationStatus)) // If this flag is true, delete secrets from the legacy secrets store as they are migrated disableSecretsCompatibility := s.features.IsEnabled(featuremgmt.FlagDisableSecretsCompatibility) // If migration hasn't happened, migrate to unified secrets and keep copy in legacy @@ -55,7 +53,7 @@ func (s *DataSourceSecretMigrationService) Migrate(ctx context.Context) error { needMigration := migrationStatus != completeSecretMigrationValue && disableSecretsCompatibility if needCompatibility || needMigration { - s.log.Debug("performing secret migration", "needs migration", needMigration, "needs compatibility", needCompatibility) + logger.Debug("performing secret migration", "needs migration", needMigration, "needs compatibility", needCompatibility) query := &datasources.GetAllDataSourcesQuery{} err := s.dataSourcesService.GetAllDataSources(ctx, query) if err != nil { @@ -100,7 +98,7 @@ func (s *DataSourceSecretMigrationService) Migrate(ctx context.Context) error { if err != nil { return err } - s.log.Debug(fmt.Sprint("set secret migration status to ", newMigStatus)) + logger.Debug(fmt.Sprint("set secret migration status to ", newMigStatus)) } return nil diff --git a/pkg/services/datasources/service/secrets_mig_test.go b/pkg/services/secrets/kvstore/migrations/datasource_mig_test.go similarity index 77% rename from pkg/services/datasources/service/secrets_mig_test.go rename to pkg/services/secrets/kvstore/migrations/datasource_mig_test.go index 3c9054dd895..6b4152434be 100644 --- a/pkg/services/datasources/service/secrets_mig_test.go +++ b/pkg/services/secrets/kvstore/migrations/datasource_mig_test.go @@ -1,30 +1,32 @@ -package service +package migrations import ( "context" "testing" "github.com/grafana/grafana/pkg/infra/kvstore" + "github.com/grafana/grafana/pkg/infra/log" acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/datasources" + dsservice "github.com/grafana/grafana/pkg/services/datasources/service" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/secrets/fakes" - secretsStore "github.com/grafana/grafana/pkg/services/secrets/kvstore" - secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" + secretskvs "github.com/grafana/grafana/pkg/services/secrets/kvstore" + secretsmng "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/assert" ) -func SetupTestMigrationService(t *testing.T, sqlStore *sqlstore.SQLStore, kvStore kvstore.KVStore, secretsStore secretsStore.SecretsKVStore, compatibility bool) *DataSourceSecretMigrationService { +func SetupTestDataSourceSecretMigrationService(t *testing.T, sqlStore *sqlstore.SQLStore, kvStore kvstore.KVStore, secretsStore secretskvs.SecretsKVStore, compatibility bool) *DataSourceSecretMigrationService { t.Helper() cfg := &setting.Cfg{} features := featuremgmt.WithFeatures() if !compatibility { features = featuremgmt.WithFeatures(featuremgmt.FlagDisableSecretsCompatibility, true) } - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) - dsService := ProvideService(sqlStore, secretsService, secretsStore, cfg, features, acmock.New().WithDisabled(), acmock.NewMockedPermissionsService()) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + dsService := dsservice.ProvideService(sqlStore, secretsService, secretsStore, cfg, features, acmock.New().WithDisabled(), acmock.NewMockedPermissionsService()) migService := ProvideDataSourceMigrationService(dsService, kvStore, features) return migService } @@ -33,8 +35,9 @@ func TestMigrate(t *testing.T) { t.Run("should migrate from legacy to unified without compatibility", func(t *testing.T) { sqlStore := sqlstore.InitTestDB(t) kvStore := kvstore.ProvideService(sqlStore) - secretsStore := secretsStore.SetupTestService(t) - migService := SetupTestMigrationService(t, sqlStore, kvStore, secretsStore, false) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) + migService := SetupTestDataSourceSecretMigrationService(t, sqlStore, kvStore, secretsStore, false) dataSourceName := "Test" dataSourceOrg := int64(1) @@ -60,13 +63,13 @@ func TestMigrate(t *testing.T) { assert.NotEmpty(t, query.Result.SecureJsonData) // Check if the migration status key is empty - value, exist, err := kvStore.Get(context.Background(), 0, secretType, secretMigrationStatusKey) + value, exist, err := kvStore.Get(context.Background(), 0, secretskvs.DataSourceSecretType, secretMigrationStatusKey) assert.NoError(t, err) assert.Empty(t, value) assert.False(t, exist) // Check that the secret is not present on the secret store - value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretType) + value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretskvs.DataSourceSecretType) assert.NoError(t, err) assert.Empty(t, value) assert.False(t, exist) @@ -83,13 +86,13 @@ func TestMigrate(t *testing.T) { assert.Empty(t, query.Result.SecureJsonData) // Check if the secret was added to the secret store - value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretType) + value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretskvs.DataSourceSecretType) assert.NoError(t, err) assert.NotEmpty(t, value) assert.True(t, exist) // Check if the migration status key was set - value, exist, err = kvStore.Get(context.Background(), 0, secretType, secretMigrationStatusKey) + value, exist, err = kvStore.Get(context.Background(), 0, secretskvs.DataSourceSecretType, secretMigrationStatusKey) assert.NoError(t, err) assert.Equal(t, completeSecretMigrationValue, value) assert.True(t, exist) @@ -98,8 +101,9 @@ func TestMigrate(t *testing.T) { t.Run("should migrate from legacy to unified with compatibility", func(t *testing.T) { sqlStore := sqlstore.InitTestDB(t) kvStore := kvstore.ProvideService(sqlStore) - secretsStore := secretsStore.SetupTestService(t) - migService := SetupTestMigrationService(t, sqlStore, kvStore, secretsStore, true) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) + migService := SetupTestDataSourceSecretMigrationService(t, sqlStore, kvStore, secretsStore, true) dataSourceName := "Test" dataSourceOrg := int64(1) @@ -125,13 +129,13 @@ func TestMigrate(t *testing.T) { assert.NotEmpty(t, query.Result.SecureJsonData) // Check if the migration status key is empty - value, exist, err := kvStore.Get(context.Background(), 0, secretType, secretMigrationStatusKey) + value, exist, err := kvStore.Get(context.Background(), 0, secretskvs.DataSourceSecretType, secretMigrationStatusKey) assert.NoError(t, err) assert.Empty(t, value) assert.False(t, exist) // Check that the secret is not present on the secret store - value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretType) + value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretskvs.DataSourceSecretType) assert.NoError(t, err) assert.Empty(t, value) assert.False(t, exist) @@ -148,13 +152,13 @@ func TestMigrate(t *testing.T) { assert.NotEmpty(t, query.Result.SecureJsonData) // Check if the secret was added to the secret store - value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretType) + value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretskvs.DataSourceSecretType) assert.NoError(t, err) assert.NotEmpty(t, value) assert.True(t, exist) // Check if the migration status key was set - value, exist, err = kvStore.Get(context.Background(), 0, secretType, secretMigrationStatusKey) + value, exist, err = kvStore.Get(context.Background(), 0, secretskvs.DataSourceSecretType, secretMigrationStatusKey) assert.NoError(t, err) assert.Equal(t, compatibleSecretMigrationValue, value) assert.True(t, exist) @@ -163,8 +167,9 @@ func TestMigrate(t *testing.T) { t.Run("should replicate from unified to legacy for compatibility", func(t *testing.T) { sqlStore := sqlstore.InitTestDB(t) kvStore := kvstore.ProvideService(sqlStore) - secretsStore := secretsStore.SetupTestService(t) - migService := SetupTestMigrationService(t, sqlStore, kvStore, secretsStore, false) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) + migService := SetupTestDataSourceSecretMigrationService(t, sqlStore, kvStore, secretsStore, false) dataSourceName := "Test" dataSourceOrg := int64(1) @@ -190,13 +195,13 @@ func TestMigrate(t *testing.T) { assert.NotEmpty(t, query.Result.SecureJsonData) // Check if the migration status key is empty - value, exist, err := kvStore.Get(context.Background(), 0, secretType, secretMigrationStatusKey) + value, exist, err := kvStore.Get(context.Background(), 0, secretskvs.DataSourceSecretType, secretMigrationStatusKey) assert.NoError(t, err) assert.Empty(t, value) assert.False(t, exist) // Check that the secret is not present on the secret store - value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretType) + value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretskvs.DataSourceSecretType) assert.NoError(t, err) assert.Empty(t, value) assert.False(t, exist) @@ -213,19 +218,19 @@ func TestMigrate(t *testing.T) { assert.Empty(t, query.Result.SecureJsonData) // Check if the secret was added to the secret store - value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretType) + value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretskvs.DataSourceSecretType) assert.NoError(t, err) assert.NotEmpty(t, value) assert.True(t, exist) // Check if the migration status key was set - value, exist, err = kvStore.Get(context.Background(), 0, secretType, secretMigrationStatusKey) + value, exist, err = kvStore.Get(context.Background(), 0, secretskvs.DataSourceSecretType, secretMigrationStatusKey) assert.NoError(t, err) assert.Equal(t, completeSecretMigrationValue, value) assert.True(t, exist) // Run the migration with compatibility - migService = SetupTestMigrationService(t, sqlStore, kvStore, secretsStore, true) + migService = SetupTestDataSourceSecretMigrationService(t, sqlStore, kvStore, secretsStore, true) err = migService.Migrate(context.Background()) assert.NoError(t, err) @@ -237,13 +242,13 @@ func TestMigrate(t *testing.T) { assert.NotEmpty(t, query.Result.SecureJsonData) // Check if the secret was added to the secret store - value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretType) + value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretskvs.DataSourceSecretType) assert.NoError(t, err) assert.NotEmpty(t, value) assert.True(t, exist) // Check if the migration status key was set - value, exist, err = kvStore.Get(context.Background(), 0, secretType, secretMigrationStatusKey) + value, exist, err = kvStore.Get(context.Background(), 0, secretskvs.DataSourceSecretType, secretMigrationStatusKey) assert.NoError(t, err) assert.Equal(t, compatibleSecretMigrationValue, value) assert.True(t, exist) @@ -252,8 +257,9 @@ func TestMigrate(t *testing.T) { t.Run("should delete from legacy to remove compatibility", func(t *testing.T) { sqlStore := sqlstore.InitTestDB(t) kvStore := kvstore.ProvideService(sqlStore) - secretsStore := secretsStore.SetupTestService(t) - migService := SetupTestMigrationService(t, sqlStore, kvStore, secretsStore, true) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) + migService := SetupTestDataSourceSecretMigrationService(t, sqlStore, kvStore, secretsStore, true) dataSourceName := "Test" dataSourceOrg := int64(1) @@ -279,13 +285,13 @@ func TestMigrate(t *testing.T) { assert.NotEmpty(t, query.Result.SecureJsonData) // Check if the migration status key is empty - value, exist, err := kvStore.Get(context.Background(), 0, secretType, secretMigrationStatusKey) + value, exist, err := kvStore.Get(context.Background(), 0, secretskvs.DataSourceSecretType, secretMigrationStatusKey) assert.NoError(t, err) assert.Empty(t, value) assert.False(t, exist) // Check that the secret is not present on the secret store - value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretType) + value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretskvs.DataSourceSecretType) assert.NoError(t, err) assert.Empty(t, value) assert.False(t, exist) @@ -302,19 +308,19 @@ func TestMigrate(t *testing.T) { assert.NotEmpty(t, query.Result.SecureJsonData) // Check if the secret was added to the secret store - value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretType) + value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretskvs.DataSourceSecretType) assert.NoError(t, err) assert.NotEmpty(t, value) assert.True(t, exist) // Check if the migration status key was set - value, exist, err = kvStore.Get(context.Background(), 0, secretType, secretMigrationStatusKey) + value, exist, err = kvStore.Get(context.Background(), 0, secretskvs.DataSourceSecretType, secretMigrationStatusKey) assert.NoError(t, err) assert.Equal(t, compatibleSecretMigrationValue, value) assert.True(t, exist) // Run the migration without compatibility - migService = SetupTestMigrationService(t, sqlStore, kvStore, secretsStore, false) + migService = SetupTestDataSourceSecretMigrationService(t, sqlStore, kvStore, secretsStore, false) err = migService.Migrate(context.Background()) assert.NoError(t, err) @@ -326,13 +332,13 @@ func TestMigrate(t *testing.T) { assert.Empty(t, query.Result.SecureJsonData) // Check if the secret was added to the secret store - value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretType) + value, exist, err = secretsStore.Get(context.Background(), dataSourceOrg, dataSourceName, secretskvs.DataSourceSecretType) assert.NoError(t, err) assert.NotEmpty(t, value) assert.True(t, exist) // Check if the migration status key was set - value, exist, err = kvStore.Get(context.Background(), 0, secretType, secretMigrationStatusKey) + value, exist, err = kvStore.Get(context.Background(), 0, secretskvs.DataSourceSecretType, secretMigrationStatusKey) assert.NoError(t, err) assert.Equal(t, completeSecretMigrationValue, value) assert.True(t, exist) diff --git a/pkg/services/secrets/kvstore/migrate_from_plugin.go b/pkg/services/secrets/kvstore/migrations/from_plugin_mig.go similarity index 61% rename from pkg/services/secrets/kvstore/migrate_from_plugin.go rename to pkg/services/secrets/kvstore/migrations/from_plugin_mig.go index c20db3b3a0b..160fc82b03a 100644 --- a/pkg/services/secrets/kvstore/migrate_from_plugin.go +++ b/pkg/services/secrets/kvstore/migrations/from_plugin_mig.go @@ -1,15 +1,14 @@ -package kvstore +package migrations import ( "context" "fmt" - "sync" "github.com/grafana/grafana/pkg/infra/kvstore" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/backendplugin/secretsmanagerplugin" "github.com/grafana/grafana/pkg/services/secrets" + secretskvs "github.com/grafana/grafana/pkg/services/secrets/kvstore" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" ) @@ -17,7 +16,6 @@ import ( // MigrateFromPluginService This migrator will handle migration of the configured plugin secrets back to Grafana unified secrets type MigrateFromPluginService struct { cfg *setting.Cfg - logger log.Logger sqlStore sqlstore.Store secretsService secrets.Service manager plugins.SecretsPluginManager @@ -34,7 +32,6 @@ func ProvideMigrateFromPluginService( ) *MigrateFromPluginService { return &MigrateFromPluginService{ cfg: cfg, - logger: log.New("sec-plugin-mig"), sqlStore: sqlStore, secretsService: secretsService, manager: manager, @@ -43,43 +40,36 @@ func ProvideMigrateFromPluginService( } func (s *MigrateFromPluginService) Migrate(ctx context.Context) error { - s.logger.Debug("starting migration of plugin secrets to unified secrets") + logger.Debug("starting migration of plugin secrets to unified secrets") // access the plugin directly - plugin, err := startAndReturnPlugin(s.manager, context.Background()) + plugin, err := secretskvs.StartAndReturnPlugin(s.manager, context.Background()) if err != nil { - s.logger.Error("Error retrieiving plugin", "error", err.Error()) + logger.Error("Error retrieiving plugin", "error", err.Error()) return err } // Get full list of secrets from the plugin res, err := plugin.GetAllSecrets(ctx, &secretsmanagerplugin.GetAllSecretsRequest{}) if err != nil { - s.logger.Error("Failed to retrieve all secrets from plugin") + logger.Error("Failed to retrieve all secrets from plugin") return err } totalSecrets := len(res.Items) - s.logger.Debug("retrieved all secrets from plugin", "num secrets", totalSecrets) + logger.Debug("retrieved all secrets from plugin", "num secrets", totalSecrets) // create a secret sql store manually - secretsSql := &secretsKVStoreSQL{ - sqlStore: s.sqlStore, - secretsService: s.secretsService, - log: s.logger, - decryptionCache: decryptionCache{ - cache: make(map[int64]cachedDecrypted), - }, - } + secretsSql := secretskvs.NewSQLSecretsKVStore(s.sqlStore, s.secretsService, logger) for i, item := range res.Items { - s.logger.Debug(fmt.Sprintf("Migrating secret %d of %d", i+1, totalSecrets), "current", i+1, "secretCount", totalSecrets) + logger.Debug(fmt.Sprintf("Migrating secret %d of %d", i+1, totalSecrets), "current", i+1, "secretCount", totalSecrets) // Add to sql store err = secretsSql.Set(ctx, item.Key.OrgId, item.Key.Namespace, item.Key.Type, item.Value) if err != nil { - s.logger.Error("Error adding secret to unified secrets", "orgId", item.Key.OrgId, + logger.Error("Error adding secret to unified secrets", "orgId", item.Key.OrgId, "namespace", item.Key.Namespace, "type", item.Key.Type) return err } } for i, item := range res.Items { - s.logger.Debug(fmt.Sprintf("Cleaning secret %d of %d", i+1, totalSecrets), "current", i+1, "secretCount", totalSecrets) + logger.Debug(fmt.Sprintf("Cleaning secret %d of %d", i+1, totalSecrets), "current", i+1, "secretCount", totalSecrets) // Delete from the plugin _, err := plugin.DeleteSecret(ctx, &secretsmanagerplugin.DeleteSecretRequest{ KeyDescriptor: &secretsmanagerplugin.Key{ @@ -88,28 +78,28 @@ func (s *MigrateFromPluginService) Migrate(ctx context.Context) error { Type: item.Key.Type, }}) if err != nil { - s.logger.Error("Error deleting secret from plugin after migration", "orgId", item.Key.OrgId, + logger.Error("Error deleting secret from plugin after migration", "orgId", item.Key.OrgId, "namespace", item.Key.Namespace, "type", item.Key.Type) continue } } - s.logger.Debug("Completed migration of secrets from plugin") + logger.Debug("Completed migration of secrets from plugin") // The plugin is no longer needed at the moment - err = setPluginStartupErrorFatal(ctx, GetNamespacedKVStore(s.kvstore), false) + err = secretskvs.SetPluginStartupErrorFatal(ctx, secretskvs.GetNamespacedKVStore(s.kvstore), false) if err != nil { - s.logger.Error("Failed to remove plugin error fatal flag", "error", err.Error()) + logger.Error("Failed to remove plugin error fatal flag", "error", err.Error()) } // Reset the fatal flag setter in case another secret is created on the plugin - fatalFlagOnce = sync.Once{} + secretskvs.ResetPlugin() - s.logger.Debug("Shutting down secrets plugin now that migration is complete") + logger.Debug("Shutting down secrets plugin now that migration is complete") // if `use_plugin` wasn't set, stop the plugin after migration if !s.cfg.SectionWithEnvOverrides("secrets").Key("use_plugin").MustBool(false) { err := s.manager.SecretsManager().Stop(ctx) if err != nil { // Log a warning but don't throw an error - s.logger.Error("Error stopping secrets plugin after migration", "error", err.Error()) + logger.Error("Error stopping secrets plugin after migration", "error", err.Error()) } } return nil diff --git a/pkg/services/secrets/kvstore/migrate_from_plugin_test.go b/pkg/services/secrets/kvstore/migrations/from_plugin_mig_test.go similarity index 87% rename from pkg/services/secrets/kvstore/migrate_from_plugin_test.go rename to pkg/services/secrets/kvstore/migrations/from_plugin_mig_test.go index 6add17254f5..a611f086c2a 100644 --- a/pkg/services/secrets/kvstore/migrate_from_plugin_test.go +++ b/pkg/services/secrets/kvstore/migrations/from_plugin_mig_test.go @@ -1,4 +1,4 @@ -package kvstore +package migrations import ( "context" @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/plugins/backendplugin/secretsmanagerplugin" "github.com/grafana/grafana/pkg/services/secrets/fakes" + secretskvs "github.com/grafana/grafana/pkg/services/secrets/kvstore" secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" @@ -39,13 +40,13 @@ func TestPluginSecretMigrationService_MigrateFromPlugin(t *testing.T) { } // Set up services used in migration -func setupTestMigrateFromPluginService(t *testing.T) (*MigrateFromPluginService, secretsmanagerplugin.SecretsManagerPlugin, *secretsKVStoreSQL) { +func setupTestMigrateFromPluginService(t *testing.T) (*MigrateFromPluginService, secretsmanagerplugin.SecretsManagerPlugin, *secretskvs.SecretsKVStoreSQL) { t.Helper() // this is to init the sql secret store inside the migration sqlStore := sqlstore.InitTestDB(t) secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) - manager := NewFakeSecretsPluginManager(t, false) + manager := secretskvs.NewFakeSecretsPluginManager(t, false) migratorService := ProvideMigrateFromPluginService( setting.NewCfg(), sqlStore, @@ -54,14 +55,7 @@ func setupTestMigrateFromPluginService(t *testing.T) (*MigrateFromPluginService, kvstore.ProvideService(sqlStore), ) - secretsSql := &secretsKVStoreSQL{ - sqlStore: sqlStore, - secretsService: secretsService, - log: log.New("test.logger"), - decryptionCache: decryptionCache{ - cache: make(map[int64]cachedDecrypted), - }, - } + secretsSql := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) return migratorService, manager.SecretsManager().SecretsManager, secretsSql } @@ -88,7 +82,7 @@ func validatePluginSecretsWereDeleted(t *testing.T, plugin secretsmanagerplugin. } // validates that secrets are in sql -func validateSecretWasStoredInSql(t *testing.T, sqlStore *secretsKVStoreSQL, ctx context.Context, orgId int64, namespace string, typ string, expectedValue string) { +func validateSecretWasStoredInSql(t *testing.T, sqlStore *secretskvs.SecretsKVStoreSQL, ctx context.Context, orgId int64, namespace string, typ string, expectedValue string) { t.Helper() res, exists, err := sqlStore.Get(ctx, orgId, namespace, typ) require.NoError(t, err) diff --git a/pkg/services/secrets/kvstore/migrations/migrator.go b/pkg/services/secrets/kvstore/migrations/migrator.go index 19934a2b449..3bef598ce16 100644 --- a/pkg/services/secrets/kvstore/migrations/migrator.go +++ b/pkg/services/secrets/kvstore/migrations/migrator.go @@ -7,8 +7,6 @@ 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" "github.com/grafana/grafana/pkg/setting" ) @@ -24,16 +22,16 @@ type SecretMigrationService interface { type SecretMigrationServiceImpl struct { services []SecretMigrationService ServerLockService *serverlock.ServerLockService - migrateToPluginService *kvstore.MigrateToPluginService - migrateFromPluginService *kvstore.MigrateFromPluginService + migrateToPluginService *MigrateToPluginService + migrateFromPluginService *MigrateFromPluginService } func ProvideSecretMigrationService( cfg *setting.Cfg, serverLockService *serverlock.ServerLockService, - dataSourceSecretMigrationService *datasources.DataSourceSecretMigrationService, - migrateToPluginService *kvstore.MigrateToPluginService, - migrateFromPluginService *kvstore.MigrateFromPluginService, + dataSourceSecretMigrationService *DataSourceSecretMigrationService, + migrateToPluginService *MigrateToPluginService, + migrateFromPluginService *MigrateFromPluginService, ) *SecretMigrationServiceImpl { services := make([]SecretMigrationService, 0) services = append(services, dataSourceSecretMigrationService) diff --git a/pkg/services/secrets/kvstore/migrate_to_plugin.go b/pkg/services/secrets/kvstore/migrations/to_plugin_mig.go similarity index 61% rename from pkg/services/secrets/kvstore/migrate_to_plugin.go rename to pkg/services/secrets/kvstore/migrations/to_plugin_mig.go index 2c3df279d2c..dfa1d36267a 100644 --- a/pkg/services/secrets/kvstore/migrate_to_plugin.go +++ b/pkg/services/secrets/kvstore/migrations/to_plugin_mig.go @@ -1,4 +1,4 @@ -package kvstore +package migrations import ( "context" @@ -6,9 +6,9 @@ import ( "fmt" "github.com/grafana/grafana/pkg/infra/kvstore" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/secrets" + secretskvs "github.com/grafana/grafana/pkg/services/secrets/kvstore" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" ) @@ -16,9 +16,8 @@ import ( // MigrateToPluginService This migrator will handle migration of datasource secrets (aka Unified secrets) // into the plugin secrets configured type MigrateToPluginService struct { - secretsStore SecretsKVStore + secretsStore secretskvs.SecretsKVStore cfg *setting.Cfg - logger log.Logger sqlStore sqlstore.Store secretsService secrets.Service kvstore kvstore.KVStore @@ -26,7 +25,7 @@ type MigrateToPluginService struct { } func ProvideMigrateToPluginService( - secretsStore SecretsKVStore, + secretsStore secretskvs.SecretsKVStore, cfg *setting.Cfg, sqlStore sqlstore.Store, secretsService secrets.Service, @@ -36,7 +35,6 @@ func ProvideMigrateToPluginService( return &MigrateToPluginService{ secretsStore: secretsStore, cfg: cfg, - logger: log.New("secret.migration.plugin"), sqlStore: sqlStore, secretsService: secretsService, kvstore: kvstore, @@ -45,8 +43,8 @@ func ProvideMigrateToPluginService( } func (s *MigrateToPluginService) Migrate(ctx context.Context) error { - if err := EvaluateRemoteSecretsPlugin(s.manager, s.cfg); err == nil { - s.logger.Debug("starting migration of unified secrets to the plugin") + if err := secretskvs.EvaluateRemoteSecretsPlugin(s.manager, s.cfg); err == nil { + logger.Debug("starting migration of unified secrets to the plugin") // we need to get the fallback store since in this scenario the secrets store would be the plugin. fallbackStore := s.secretsStore.Fallback() if fallbackStore == nil { @@ -54,10 +52,10 @@ func (s *MigrateToPluginService) Migrate(ctx context.Context) error { } // before we start migrating, check see if plugin startup failures were already fatal - namespacedKVStore := GetNamespacedKVStore(s.kvstore) - wasFatal, err := isPluginStartupErrorFatal(ctx, namespacedKVStore) + namespacedKVStore := secretskvs.GetNamespacedKVStore(s.kvstore) + wasFatal, err := secretskvs.IsPluginStartupErrorFatal(ctx, namespacedKVStore) if err != nil { - s.logger.Warn("unable to determine whether plugin startup failures are fatal - continuing migration anyway.") + logger.Warn("unable to determine whether plugin startup failures are fatal - continuing migration anyway.") } allSec, err := fallbackStore.GetAll(ctx) @@ -66,35 +64,35 @@ func (s *MigrateToPluginService) Migrate(ctx context.Context) error { } totalSec := len(allSec) // We just set it again as the current secret store should be the plugin secret - s.logger.Debug(fmt.Sprintf("Total amount of secrets to migrate: %d", totalSec)) + logger.Debug(fmt.Sprintf("Total amount of secrets to migrate: %d", totalSec)) for i, sec := range allSec { - s.logger.Debug(fmt.Sprintf("Migrating secret %d of %d", i+1, totalSec), "current", i+1, "secretCount", totalSec) + logger.Debug(fmt.Sprintf("Migrating secret %d of %d", i+1, totalSec), "current", i+1, "secretCount", totalSec) err = s.secretsStore.Set(ctx, *sec.OrgId, *sec.Namespace, *sec.Type, sec.Value) if err != nil { return err } } - s.logger.Debug("migrated unified secrets to plugin", "number of secrets", totalSec) + logger.Debug("migrated unified secrets to plugin", "number of secrets", totalSec) // as no err was returned, when we delete all the secrets from the sql store for index, sec := range allSec { - s.logger.Debug(fmt.Sprintf("Cleaning secret %d of %d", index+1, totalSec), "current", index+1, "secretCount", totalSec) + logger.Debug(fmt.Sprintf("Cleaning secret %d of %d", index+1, totalSec), "current", index+1, "secretCount", totalSec) err = fallbackStore.Del(ctx, *sec.OrgId, *sec.Namespace, *sec.Type) if err != nil { - s.logger.Error("plugin migrator encountered error while deleting unified secrets") + logger.Error("plugin migrator encountered error while deleting unified secrets") if index == 0 && !wasFatal { // old unified secrets still exists, so plugin startup errors are still not fatal, unless they were before we started - err := setPluginStartupErrorFatal(ctx, namespacedKVStore, false) + err := secretskvs.SetPluginStartupErrorFatal(ctx, namespacedKVStore, false) if err != nil { - s.logger.Error("error reverting plugin failure fatal status", "error", err.Error()) + logger.Error("error reverting plugin failure fatal status", "error", err.Error()) } else { - s.logger.Debug("application will continue to function without the secrets plugin") + logger.Debug("application will continue to function without the secrets plugin") } } return err } } - s.logger.Debug("deleted unified secrets after migration", "number of secrets", totalSec) + logger.Debug("deleted unified secrets after migration", "number of secrets", totalSec) } return nil } diff --git a/pkg/services/secrets/kvstore/migrate_to_plugin_test.go b/pkg/services/secrets/kvstore/migrations/to_plugin_mig_test.go similarity index 53% rename from pkg/services/secrets/kvstore/migrate_to_plugin_test.go rename to pkg/services/secrets/kvstore/migrations/to_plugin_mig_test.go index ecf4071b2ed..e273eaca59f 100644 --- a/pkg/services/secrets/kvstore/migrate_to_plugin_test.go +++ b/pkg/services/secrets/kvstore/migrations/to_plugin_mig_test.go @@ -1,15 +1,19 @@ -package kvstore +package migrations import ( "context" + "errors" "testing" "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/secrets/fakes" + secretskvs "github.com/grafana/grafana/pkg/services/secrets/kvstore" secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/ini.v1" ) @@ -43,14 +47,32 @@ func TestPluginSecretMigrationService_MigrateToPlugin(t *testing.T) { }) } -func addSecretToSqlStore(t *testing.T, sqlSecretStore *secretsKVStoreSQL, ctx context.Context, orgId int64, namespace1 string, typ string, value string) { +// With fatal flag unset, do a migration with backwards compatibility disabled. When unified secrets are deleted, return an error on the first deletion +// Should result in the fatal flag remaining unset +func TestFatalPluginErr_MigrationTestWithErrorDeletingUnifiedSecrets(t *testing.T) { + p, err := secretskvs.SetupFatalCrashTest(t, false, false, true) + assert.NoError(t, err) + + migration := setupTestMigratorServiceWithDeletionError(t, p.SecretsKVStore, &mockstore.SQLStoreMock{ + ExpectedError: errors.New("random error"), + }, p.KVStore) + err = migration.Migrate(context.Background()) + assert.Error(t, err) + assert.Equal(t, "mocked del error", err.Error()) + + isFatal, err := secretskvs.IsPluginStartupErrorFatal(context.Background(), secretskvs.GetNamespacedKVStore(p.KVStore)) + assert.NoError(t, err) + assert.False(t, isFatal) +} + +func addSecretToSqlStore(t *testing.T, sqlSecretStore *secretskvs.SecretsKVStoreSQL, ctx context.Context, orgId int64, namespace1 string, typ string, value string) { t.Helper() err := sqlSecretStore.Set(ctx, orgId, namespace1, typ, value) require.NoError(t, err) } // validates that secrets on the sql store were deleted. -func validateSqlSecretWasDeleted(t *testing.T, sqlSecretStore *secretsKVStoreSQL, ctx context.Context, orgId int64, namespace1 string, typ string) { +func validateSqlSecretWasDeleted(t *testing.T, sqlSecretStore *secretskvs.SecretsKVStoreSQL, ctx context.Context, orgId int64, namespace1 string, typ string) { t.Helper() res, err := sqlSecretStore.Keys(ctx, orgId, namespace1, typ) require.NoError(t, err) @@ -58,7 +80,7 @@ func validateSqlSecretWasDeleted(t *testing.T, sqlSecretStore *secretsKVStoreSQL } // validates that secrets should be on the plugin -func validateSecretWasStoredInPlugin(t *testing.T, secretsStore SecretsKVStore, ctx context.Context, orgId int64, namespace1 string, typ string) { +func validateSecretWasStoredInPlugin(t *testing.T, secretsStore secretskvs.SecretsKVStore, ctx context.Context, orgId int64, namespace1 string, typ string) { t.Helper() resPlugin, err := secretsStore.Keys(ctx, orgId, namespace1, typ) require.NoError(t, err) @@ -66,7 +88,7 @@ func validateSecretWasStoredInPlugin(t *testing.T, secretsStore SecretsKVStore, } // Set up services used in migration -func setupTestMigrateToPluginService(t *testing.T) (*MigrateToPluginService, SecretsKVStore, *secretsKVStoreSQL) { +func setupTestMigrateToPluginService(t *testing.T) (*MigrateToPluginService, secretskvs.SecretsKVStore, *secretskvs.SecretsKVStoreSQL) { t.Helper() rawCfg := ` @@ -77,12 +99,12 @@ func setupTestMigrateToPluginService(t *testing.T) (*MigrateToPluginService, Sec require.NoError(t, err) cfg := &setting.Cfg{Raw: raw} // this would be the plugin - mocked at the moment - secretsStoreForPlugin := NewFakeSecretsKVStore() + secretsStoreForPlugin := secretskvs.NewFakeSecretsKVStore() // this is to init the sql secret store inside the migration sqlStore := sqlstore.InitTestDB(t) secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) - manager := NewFakeSecretsPluginManager(t, false) + manager := secretskvs.NewFakeSecretsPluginManager(t, false) migratorService := ProvideMigrateToPluginService( secretsStoreForPlugin, cfg, @@ -92,16 +114,39 @@ func setupTestMigrateToPluginService(t *testing.T) (*MigrateToPluginService, Sec manager, ) - secretsSql := &secretsKVStoreSQL{ - sqlStore: sqlStore, - secretsService: secretsService, - log: log.New("test.logger"), - decryptionCache: decryptionCache{ - cache: make(map[int64]cachedDecrypted), - }, - } + secretsSql := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) err = secretsStoreForPlugin.SetFallback(secretsSql) require.NoError(t, err) return migratorService, secretsStoreForPlugin, secretsSql } + +func setupTestMigratorServiceWithDeletionError( + t *testing.T, + secretskv secretskvs.SecretsKVStore, + sqlStore sqlstore.Store, + kvstore kvstore.KVStore, +) *MigrateToPluginService { + t.Helper() + secretskvs.ResetPlugin() + cfg := secretskvs.SetupTestConfig(t) + secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + manager := secretskvs.NewFakeSecretsPluginManager(t, false) + migratorService := ProvideMigrateToPluginService( + secretskv, + cfg, + sqlStore, + secretsService, + kvstore, + manager, + ) + fallback := secretskvs.NewFakeSecretsKVStore() + var orgId int64 = 1 + str := "random string" + err := fallback.Set(context.Background(), orgId, str, str, "bogus") + require.NoError(t, err) + fallback.DeletionError(true) + err = secretskv.SetFallback(fallback) + require.NoError(t, err) + return migratorService +} diff --git a/pkg/services/secrets/kvstore/model.go b/pkg/services/secrets/kvstore/model.go index 9e7db1b0c5d..463fd7c2563 100644 --- a/pkg/services/secrets/kvstore/model.go +++ b/pkg/services/secrets/kvstore/model.go @@ -7,6 +7,7 @@ import ( const ( QuitOnPluginStartupFailureKey = "quit_on_secrets_plugin_startup_failure" PluginNamespace = "secretsmanagerplugin" + DataSourceSecretType = "datasource" ) // Item stored in k/v store. diff --git a/pkg/services/secrets/kvstore/remote_plugin.go b/pkg/services/secrets/kvstore/plugin.go similarity index 95% rename from pkg/services/secrets/kvstore/remote_plugin.go rename to pkg/services/secrets/kvstore/plugin.go index 2d2c39b4e7b..35b6c954536 100644 --- a/pkg/services/secrets/kvstore/remote_plugin.go +++ b/pkg/services/secrets/kvstore/plugin.go @@ -191,10 +191,10 @@ func updateFatalFlag(ctx context.Context, skv secretsKVStorePlugin) { fatalFlagOnce.Do(func() { skv.log.Debug("Updating plugin startup error fatal flag") var err error - if isFatal, _ := isPluginStartupErrorFatal(ctx, skv.kvstore); !isFatal && skv.backwardsCompatibilityDisabled { - err = setPluginStartupErrorFatal(ctx, skv.kvstore, true) + if isFatal, _ := IsPluginStartupErrorFatal(ctx, skv.kvstore); !isFatal && skv.backwardsCompatibilityDisabled { + err = SetPluginStartupErrorFatal(ctx, skv.kvstore, true) } else if isFatal && !skv.backwardsCompatibilityDisabled { - err = setPluginStartupErrorFatal(ctx, skv.kvstore, false) + err = SetPluginStartupErrorFatal(ctx, skv.kvstore, false) } if err != nil { skv.log.Error("failed to set plugin error fatal flag", err.Error()) @@ -210,7 +210,7 @@ func GetNamespacedKVStore(kv kvstore.KVStore) *kvstore.NamespacedKVStore { return kvstore.WithNamespace(kv, kvstore.AllOrganizations, PluginNamespace) } -func isPluginStartupErrorFatal(ctx context.Context, kvstore *kvstore.NamespacedKVStore) (bool, error) { +func IsPluginStartupErrorFatal(ctx context.Context, kvstore *kvstore.NamespacedKVStore) (bool, error) { _, exists, err := kvstore.Get(ctx, QuitOnPluginStartupFailureKey) if err != nil { return false, errors.New(fmt.Sprint("error retrieving key ", QuitOnPluginStartupFailureKey, " from kvstore. error: ", err.Error())) @@ -218,7 +218,7 @@ func isPluginStartupErrorFatal(ctx context.Context, kvstore *kvstore.NamespacedK return exists, nil } -func setPluginStartupErrorFatal(ctx context.Context, kvstore *kvstore.NamespacedKVStore, isFatal bool) error { +func SetPluginStartupErrorFatal(ctx context.Context, kvstore *kvstore.NamespacedKVStore, isFatal bool) error { if !isFatal { return kvstore.Del(ctx, QuitOnPluginStartupFailureKey) } @@ -237,7 +237,7 @@ func EvaluateRemoteSecretsPlugin(mg plugins.SecretsPluginManager, cfg *setting.C return nil } -func startAndReturnPlugin(mg plugins.SecretsPluginManager, ctx context.Context) (smp.SecretsManagerPlugin, error) { +func StartAndReturnPlugin(mg plugins.SecretsPluginManager, ctx context.Context) (smp.SecretsManagerPlugin, error) { var err error startupOnce.Do(func() { err = mg.SecretsManager().Start(ctx) diff --git a/pkg/services/secrets/kvstore/plugin_fatal_crash_test.go b/pkg/services/secrets/kvstore/plugin_fatal_crash_test.go deleted file mode 100644 index 2fb5d2d3a20..00000000000 --- a/pkg/services/secrets/kvstore/plugin_fatal_crash_test.go +++ /dev/null @@ -1,184 +0,0 @@ -package kvstore - -import ( - "context" - "errors" - "sync" - "testing" - - "github.com/grafana/grafana/pkg/infra/kvstore" - "github.com/grafana/grafana/pkg/plugins" - "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/services/sqlstore/mockstore" - "github.com/grafana/grafana/pkg/setting" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "gopkg.in/ini.v1" -) - -// Set fatal flag to true, then simulate a plugin start failure -// Should result in an error from the secret store provider -func TestFatalPluginErr_PluginFailsToStartWithFatalFlagSet(t *testing.T) { - p, err := setupFatalCrashTest(t, true, true, false) - assert.Error(t, err) - assert.Equal(t, "mocked failed to start", err.Error()) - assert.Nil(t, p.secretsKVStore) -} - -// Set fatal flag to false, then simulate a plugin start failure -// Should result in the secret store provider returning the sql impl -func TestFatalPluginErr_PluginFailsToStartWithFatalFlagNotSet(t *testing.T) { - p, err := setupFatalCrashTest(t, true, false, false) - assert.NoError(t, err) - require.IsType(t, &CachedKVStore{}, p.secretsKVStore) - - cachedKv, _ := p.secretsKVStore.(*CachedKVStore) - assert.IsType(t, &secretsKVStoreSQL{}, cachedKv.GetUnwrappedStore()) -} - -// With fatal flag not set, store a secret in the plugin while backwards compatibility is disabled -// Should result in the fatal flag going from unset -> set to true -func TestFatalPluginErr_FatalFlagGetsSetWithBackwardsCompatDisabled(t *testing.T) { - p, err := setupFatalCrashTest(t, false, false, true) - assert.NoError(t, err) - require.NotNil(t, p.secretsKVStore) - - err = p.secretsKVStore.Set(context.Background(), 0, "datasource", "postgres", "my secret") - assert.NoError(t, err) - - isFatal, err := isPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(p.kvstore)) - assert.NoError(t, err) - assert.True(t, isFatal) -} - -// With fatal flag set, retrieve a secret from the plugin while backwards compatibility is enabled -// Should result in the fatal flag going from set to true -> unset -func TestFatalPluginErr_FatalFlagGetsUnSetWithBackwardsCompatEnabled(t *testing.T) { - p, err := setupFatalCrashTest(t, false, true, false) - assert.NoError(t, err) - require.NotNil(t, p.secretsKVStore) - - // setup - store secret and manually bypassing the remote plugin impl - _, err = p.pluginManager.SecretsManager().SecretsManager.SetSecret(context.Background(), &secretsmanagerplugin.SetSecretRequest{ - KeyDescriptor: &secretsmanagerplugin.Key{ - OrgId: 0, - Namespace: "postgres", - Type: "datasource", - }, - Value: "bogus", - }) - assert.NoError(t, err) - - // retrieve the secret and check values - val, exists, err := p.secretsKVStore.Get(context.Background(), 0, "postgres", "datasource") - assert.NoError(t, err) - assert.NotNil(t, val) - assert.True(t, exists) - - isFatal, err := isPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(p.kvstore)) - assert.NoError(t, err) - assert.False(t, isFatal) -} - -// With fatal flag unset, do a migration with backwards compatibility disabled. When unified secrets are deleted, return an error on the first deletion -// Should result in the fatal flag remaining unset -func TestFatalPluginErr_MigrationTestWithErrorDeletingUnifiedSecrets(t *testing.T) { - p, err := setupFatalCrashTest(t, false, false, true) - assert.NoError(t, err) - - migration := setupTestMigratorServiceWithDeletionError(t, p.secretsKVStore, &mockstore.SQLStoreMock{ - ExpectedError: errors.New("random error"), - }, p.kvstore) - err = migration.Migrate(context.Background()) - assert.Error(t, err) - assert.Equal(t, "mocked del error", err.Error()) - - isFatal, err := isPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(p.kvstore)) - assert.NoError(t, err) - assert.False(t, isFatal) -} - -func setupFatalCrashTest( - t *testing.T, - shouldFailOnStart bool, - isPluginErrorFatal bool, - isBackwardsCompatDisabled bool, -) (fatalCrashTestFields, error) { - t.Helper() - fatalFlagOnce = sync.Once{} - startupOnce = sync.Once{} - cfg := setupTestConfig(t) - sqlStore := sqlstore.InitTestDB(t) - secretService := fakes.FakeSecretsService{} - kvstore := kvstore.ProvideService(sqlStore) - if isPluginErrorFatal { - _ = setPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(kvstore), true) - } - features := NewFakeFeatureToggles(t, isBackwardsCompatDisabled) - manager := NewFakeSecretsPluginManager(t, shouldFailOnStart) - svc, err := ProvideService(sqlStore, secretService, manager, kvstore, features, cfg) - t.Cleanup(func() { - fatalFlagOnce = sync.Once{} - }) - return fatalCrashTestFields{ - secretsKVStore: svc, - pluginManager: manager, - kvstore: kvstore, - sqlStore: sqlStore, - }, err -} - -type fatalCrashTestFields struct { - secretsKVStore SecretsKVStore - pluginManager plugins.SecretsPluginManager - kvstore kvstore.KVStore - sqlStore *sqlstore.SQLStore -} - -func setupTestMigratorServiceWithDeletionError( - t *testing.T, - secretskv SecretsKVStore, - sqlStore sqlstore.Store, - kvstore kvstore.KVStore, -) *MigrateToPluginService { - t.Helper() - fatalFlagOnce = sync.Once{} - startupOnce = sync.Once{} - cfg := setupTestConfig(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) - manager := NewFakeSecretsPluginManager(t, false) - migratorService := ProvideMigrateToPluginService( - secretskv, - cfg, - sqlStore, - secretsService, - kvstore, - manager, - ) - fallback := NewFakeSecretsKVStore() - var orgId int64 = 1 - str := "random string" - fallback.store[Key{ - OrgId: orgId, - Type: str, - Namespace: str, - }] = "bogus" - fallback.delError = true - err := secretskv.SetFallback(fallback) - require.NoError(t, err) - return migratorService -} - -func setupTestConfig(t *testing.T) *setting.Cfg { - t.Helper() - rawCfg := ` - [secrets] - use_plugin = true - ` - raw, err := ini.Load([]byte(rawCfg)) - require.NoError(t, err) - return &setting.Cfg{Raw: raw} -} diff --git a/pkg/services/secrets/kvstore/plugin_test.go b/pkg/services/secrets/kvstore/plugin_test.go new file mode 100644 index 00000000000..2655bf9c740 --- /dev/null +++ b/pkg/services/secrets/kvstore/plugin_test.go @@ -0,0 +1,74 @@ +package kvstore + +import ( + "context" + "testing" + + "github.com/grafana/grafana/pkg/plugins/backendplugin/secretsmanagerplugin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Set fatal flag to true, then simulate a plugin start failure +// Should result in an error from the secret store provider +func TestFatalPluginErr_PluginFailsToStartWithFatalFlagSet(t *testing.T) { + p, err := SetupFatalCrashTest(t, true, true, false) + assert.Error(t, err) + assert.Equal(t, "mocked failed to start", err.Error()) + assert.Nil(t, p.SecretsKVStore) +} + +// Set fatal flag to false, then simulate a plugin start failure +// Should result in the secret store provider returning the sql impl +func TestFatalPluginErr_PluginFailsToStartWithFatalFlagNotSet(t *testing.T) { + p, err := SetupFatalCrashTest(t, true, false, false) + assert.NoError(t, err) + require.IsType(t, &CachedKVStore{}, p.SecretsKVStore) + + cachedKv, _ := p.SecretsKVStore.(*CachedKVStore) + assert.IsType(t, &SecretsKVStoreSQL{}, cachedKv.GetUnwrappedStore()) +} + +// With fatal flag not set, store a secret in the plugin while backwards compatibility is disabled +// Should result in the fatal flag going from unset -> set to true +func TestFatalPluginErr_FatalFlagGetsSetWithBackwardsCompatDisabled(t *testing.T) { + p, err := SetupFatalCrashTest(t, false, false, true) + assert.NoError(t, err) + require.NotNil(t, p.SecretsKVStore) + + err = p.SecretsKVStore.Set(context.Background(), 0, "datasource", "postgres", "my secret") + assert.NoError(t, err) + + isFatal, err := IsPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(p.KVStore)) + assert.NoError(t, err) + assert.True(t, isFatal) +} + +// With fatal flag set, retrieve a secret from the plugin while backwards compatibility is enabled +// Should result in the fatal flag going from set to true -> unset +func TestFatalPluginErr_FatalFlagGetsUnSetWithBackwardsCompatEnabled(t *testing.T) { + p, err := SetupFatalCrashTest(t, false, true, false) + assert.NoError(t, err) + require.NotNil(t, p.SecretsKVStore) + + // setup - store secret and manually bypassing the remote plugin impl + _, err = p.PluginManager.SecretsManager().SecretsManager.SetSecret(context.Background(), &secretsmanagerplugin.SetSecretRequest{ + KeyDescriptor: &secretsmanagerplugin.Key{ + OrgId: 0, + Namespace: "postgres", + Type: "datasource", + }, + Value: "bogus", + }) + assert.NoError(t, err) + + // retrieve the secret and check values + val, exists, err := p.SecretsKVStore.Get(context.Background(), 0, "postgres", "datasource") + assert.NoError(t, err) + assert.NotNil(t, val) + assert.True(t, exists) + + isFatal, err := IsPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(p.KVStore)) + assert.NoError(t, err) + assert.False(t, isFatal) +} diff --git a/pkg/services/secrets/kvstore/sql.go b/pkg/services/secrets/kvstore/sql.go index ca3da39e4b1..0853130801c 100644 --- a/pkg/services/secrets/kvstore/sql.go +++ b/pkg/services/secrets/kvstore/sql.go @@ -12,8 +12,8 @@ import ( "github.com/grafana/grafana/pkg/services/sqlstore" ) -// secretsKVStoreSQL provides a key/value store backed by the Grafana database -type secretsKVStoreSQL struct { +// SecretsKVStoreSQL provides a key/value store backed by the Grafana database +type SecretsKVStoreSQL struct { log log.Logger sqlStore sqlstore.Store secretsService secrets.Service @@ -35,8 +35,19 @@ var ( errFallbackNotAllowed = errors.New("fallback not allowed for sql secret store") ) +func NewSQLSecretsKVStore(sqlStore sqlstore.Store, secretsService secrets.Service, logger log.Logger) *SecretsKVStoreSQL { + return &SecretsKVStoreSQL{ + sqlStore: sqlStore, + secretsService: secretsService, + log: logger, + decryptionCache: decryptionCache{ + cache: make(map[int64]cachedDecrypted), + }, + } +} + // Get an item from the store -func (kv *secretsKVStoreSQL) Get(ctx context.Context, orgId int64, namespace string, typ string) (string, bool, error) { +func (kv *SecretsKVStoreSQL) Get(ctx context.Context, orgId int64, namespace string, typ string) (string, bool, error) { item := Item{ OrgId: &orgId, Namespace: &namespace, @@ -72,7 +83,7 @@ func (kv *secretsKVStoreSQL) Get(ctx context.Context, orgId int64, namespace str } // Set an item in the store -func (kv *secretsKVStoreSQL) Set(ctx context.Context, orgId int64, namespace string, typ string, value string) error { +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.Error("error encrypting secret value", "orgId", orgId, "type", typ, "namespace", namespace, "err", err) @@ -130,7 +141,7 @@ func (kv *secretsKVStoreSQL) Set(ctx context.Context, orgId int64, namespace str } // Del deletes an item from the store. -func (kv *secretsKVStoreSQL) Del(ctx context.Context, orgId int64, namespace string, typ string) error { +func (kv *SecretsKVStoreSQL) Del(ctx context.Context, orgId int64, namespace string, typ string) error { err := kv.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { item := Item{ OrgId: &orgId, @@ -164,7 +175,7 @@ func (kv *secretsKVStoreSQL) Del(ctx context.Context, orgId int64, namespace str // Keys get all keys for a given namespace. To query for all // organizations the constant 'kvstore.AllOrganizations' can be passed as orgId. -func (kv *secretsKVStoreSQL) Keys(ctx context.Context, orgId int64, namespace string, typ string) ([]Key, error) { +func (kv *SecretsKVStoreSQL) Keys(ctx context.Context, orgId int64, namespace string, typ string) ([]Key, error) { var keys []Key err := kv.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { query := dbSession.Where("namespace = ?", namespace).And("type = ?", typ) @@ -177,7 +188,7 @@ func (kv *secretsKVStoreSQL) Keys(ctx context.Context, orgId int64, namespace st } // Rename an item in the store -func (kv *secretsKVStoreSQL) Rename(ctx context.Context, orgId int64, namespace string, typ string, newNamespace string) error { +func (kv *SecretsKVStoreSQL) Rename(ctx context.Context, orgId int64, namespace string, typ string, newNamespace string) error { return kv.sqlStore.WithTransactionalDbSession(ctx, func(dbSession *sqlstore.DBSession) error { item := Item{ OrgId: &orgId, @@ -211,7 +222,7 @@ func (kv *secretsKVStoreSQL) Rename(ctx context.Context, orgId int64, namespace // 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) { +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) @@ -233,15 +244,15 @@ func (kv *secretsKVStoreSQL) GetAll(ctx context.Context) ([]Item, error) { return items, err } -func (kv *secretsKVStoreSQL) Fallback() SecretsKVStore { +func (kv *SecretsKVStoreSQL) Fallback() SecretsKVStore { return nil } -func (kv *secretsKVStoreSQL) SetFallback(_ SecretsKVStore) error { +func (kv *SecretsKVStoreSQL) SetFallback(_ SecretsKVStore) error { return errFallbackNotAllowed } -func (kv *secretsKVStoreSQL) getDecryptedValue(ctx context.Context, item Item) ([]byte, error) { +func (kv *SecretsKVStoreSQL) getDecryptedValue(ctx context.Context, item Item) ([]byte, error) { kv.decryptionCache.Lock() defer kv.decryptionCache.Unlock() var decryptedValue []byte diff --git a/pkg/services/secrets/kvstore/sql_test.go b/pkg/services/secrets/kvstore/sql_test.go index 48604955fc2..07e62eceb00 100644 --- a/pkg/services/secrets/kvstore/sql_test.go +++ b/pkg/services/secrets/kvstore/sql_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/secrets/database" + "github.com/grafana/grafana/pkg/services/secrets/fakes" "github.com/grafana/grafana/pkg/services/secrets/manager" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/stretchr/testify/assert" @@ -24,27 +24,10 @@ func (t *TestCase) Value() string { return fmt.Sprintf("%d:%s:%s:%d", t.OrgId, t.Namespace, t.Type, t.Revision) } -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) + sqlStore := sqlstore.InitTestDB(t) + secretsService := manager.SetupTestService(t, fakes.NewFakeSecretsStore()) + kv := NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) ctx := context.Background() @@ -175,7 +158,9 @@ func TestSecretsKVStoreSQL(t *testing.T) { }) t.Run("listing existing keys", func(t *testing.T) { - kv := setupTestService(t) + sqlStore := sqlstore.InitTestDB(t) + secretsService := manager.SetupTestService(t, fakes.NewFakeSecretsStore()) + kv := NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) ctx := context.Background() @@ -248,7 +233,9 @@ func TestSecretsKVStoreSQL(t *testing.T) { }) t.Run("getting all secrets", func(t *testing.T) { - kv := setupTestService(t) + sqlStore := sqlstore.InitTestDB(t) + secretsService := manager.SetupTestService(t, fakes.NewFakeSecretsStore()) + kv := NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) ctx := context.Background() diff --git a/pkg/services/secrets/kvstore/test_helpers.go b/pkg/services/secrets/kvstore/test_helpers.go index 0acc818dd7d..adf444efd85 100644 --- a/pkg/services/secrets/kvstore/test_helpers.go +++ b/pkg/services/secrets/kvstore/test_helpers.go @@ -3,38 +3,22 @@ package kvstore import ( "context" "errors" + "sync" "testing" - "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/backendplugin" "github.com/grafana/grafana/pkg/plugins/backendplugin/secretsmanagerplugin" "github.com/grafana/grafana/pkg/services/featuremgmt" - "github.com/grafana/grafana/pkg/services/secrets/database" - "github.com/grafana/grafana/pkg/services/secrets/manager" + "github.com/grafana/grafana/pkg/services/secrets/fakes" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/require" "google.golang.org/grpc" + "gopkg.in/ini.v1" ) -func SetupTestService(t *testing.T) SecretsKVStore { - 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 -} - // In memory kv store used for testing type FakeSecretsKVStore struct { store map[Key]string @@ -46,6 +30,10 @@ func NewFakeSecretsKVStore() *FakeSecretsKVStore { return &FakeSecretsKVStore{store: make(map[Key]string)} } +func (f *FakeSecretsKVStore) DeletionError(shouldErr bool) { + f.delError = shouldErr +} + 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 != "" @@ -247,3 +235,56 @@ func (pc *fakePluginClient) Start(_ context.Context) error { func (pc *fakePluginClient) Stop(_ context.Context) error { return nil } + +func SetupFatalCrashTest( + t *testing.T, + shouldFailOnStart bool, + isPluginErrorFatal bool, + isBackwardsCompatDisabled bool, +) (fatalCrashTestFields, error) { + t.Helper() + fatalFlagOnce = sync.Once{} + startupOnce = sync.Once{} + cfg := SetupTestConfig(t) + sqlStore := sqlstore.InitTestDB(t) + secretService := fakes.FakeSecretsService{} + kvstore := kvstore.ProvideService(sqlStore) + if isPluginErrorFatal { + _ = SetPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(kvstore), true) + } + features := NewFakeFeatureToggles(t, isBackwardsCompatDisabled) + manager := NewFakeSecretsPluginManager(t, shouldFailOnStart) + svc, err := ProvideService(sqlStore, secretService, manager, kvstore, features, cfg) + t.Cleanup(func() { + fatalFlagOnce = sync.Once{} + }) + return fatalCrashTestFields{ + SecretsKVStore: svc, + PluginManager: manager, + KVStore: kvstore, + SqlStore: sqlStore, + }, err +} + +type fatalCrashTestFields struct { + SecretsKVStore SecretsKVStore + PluginManager plugins.SecretsPluginManager + KVStore kvstore.KVStore + SqlStore *sqlstore.SQLStore +} + +func SetupTestConfig(t *testing.T) *setting.Cfg { + t.Helper() + rawCfg := ` + [secrets] + use_plugin = true + ` + raw, err := ini.Load([]byte(rawCfg)) + require.NoError(t, err) + return &setting.Cfg{Raw: raw} +} + +func ResetPlugin() { + fatalFlagOnce = sync.Once{} + startupOnce = sync.Once{} +} diff --git a/pkg/tsdb/legacydata/service/service_test.go b/pkg/tsdb/legacydata/service/service_test.go index 6cd8ef9f686..90ebc640eb1 100644 --- a/pkg/tsdb/legacydata/service/service_test.go +++ b/pkg/tsdb/legacydata/service/service_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/plugins" acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/datasources" @@ -15,8 +16,9 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/oauthtoken" "github.com/grafana/grafana/pkg/services/secrets/fakes" - "github.com/grafana/grafana/pkg/services/secrets/kvstore" - secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" + secretskvs "github.com/grafana/grafana/pkg/services/secrets/kvstore" + secretsmng "github.com/grafana/grafana/pkg/services/secrets/manager" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tsdb/legacydata" ) @@ -40,8 +42,9 @@ func TestHandleRequest(t *testing.T) { actualReq = req return backend.NewQueryDataResponse(), nil } - secretsStore := kvstore.SetupTestService(t) - secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + sqlStore := sqlstore.InitTestDB(t) + secretsService := secretsmng.SetupTestService(t, fakes.NewFakeSecretsStore()) + secretsStore := secretskvs.NewSQLSecretsKVStore(sqlStore, secretsService, log.New("test.logger")) datasourcePermissions := acmock.NewMockedPermissionsService() dsService := datasourceservice.ProvideService(nil, secretsService, secretsStore, cfg, featuremgmt.WithFeatures(), acmock.New(), datasourcePermissions) s := ProvideService(client, nil, dsService)