From 7924d3b3b58dbfea8d4dd60bb8d7c55506dd90c4 Mon Sep 17 00:00:00 2001 From: Guilherme Caulada Date: Wed, 10 Aug 2022 16:47:03 -0300 Subject: [PATCH] Secrets: Move license check to secret store plugin (#53185) * Move license check to secret store plugin * Use shouldUseRemoteSecretsPlugin for migration * Encapsulate plugin startup logic in sync.Once * Remove global logger from startup functions * Fix issues with wire and secrets plugin check * Remove todo for plugin fatal error * Rename fatalErr variable to be less confusing * Fix merge conflicts * Fix issue with grafana-cli wire and opentsdb * Remove duplicated import on remote plugin * Rename plugin check in favor of error return value * Remove unnecessary import on grafana-cli wireexts_oss * Remove unnecessary import on grafana wireexts_oss * Reset sync.Once during test setup * Remove unrelated opentsdb change on grafana-cli wire * Readd opentsdb change on grafana-cli wire --- pkg/api/frontendsettings.go | 4 +- pkg/api/frontendsettings_test.go | 2 - pkg/api/http_server.go | 8 +-- pkg/cmd/grafana-cli/runner/wire.go | 2 + pkg/cmd/grafana-cli/runner/wireexts_oss.go | 3 - pkg/server/wireexts_oss.go | 3 - pkg/services/secrets/kvstore/kvstore.go | 51 ++++++--------- .../kvstore/plugin_fatal_crash_test.go | 41 +++++++----- pkg/services/secrets/kvstore/plugin_mig.go | 9 +-- .../secrets/kvstore/plugin_mig_test.go | 5 +- pkg/services/secrets/kvstore/remote_plugin.go | 51 ++++++++++++++- .../secrets/kvstore/remote_plugin_check.go | 34 ---------- pkg/services/secrets/kvstore/test_helpers.go | 65 +++++++++++-------- 13 files changed, 145 insertions(+), 133 deletions(-) delete mode 100644 pkg/services/secrets/kvstore/remote_plugin_check.go diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index d8ec0222878..1f7fe21582d 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/services/pluginsettings" + "github.com/grafana/grafana/pkg/services/secrets/kvstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tsdb/grafanads" "github.com/grafana/grafana/pkg/util" @@ -87,6 +88,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *models.ReqContext) (map[string]i } hasAccess := accesscontrol.HasAccess(hs.AccessControl, c) + secretsManagerPluginEnabled := kvstore.EvaluateRemoteSecretsPlugin(hs.secretsPluginManager, hs.Cfg) == nil jsonObj := map[string]interface{}{ "defaultDatasource": defaultDS, @@ -154,7 +156,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *models.ReqContext) (map[string]i "featureToggles": hs.Features.GetEnabled(c.Req.Context()), "rendererAvailable": hs.RenderService.IsAvailable(), "rendererVersion": hs.RenderService.Version(), - "secretsManagerPluginEnabled": hs.remoteSecretsCheck.ShouldUseRemoteSecretsPlugin(), + "secretsManagerPluginEnabled": secretsManagerPluginEnabled, "http2Enabled": hs.Cfg.Protocol == setting.HTTP2Scheme, "sentry": hs.Cfg.Sentry, "grafanaJavascriptAgent": hs.Cfg.GrafanaJavascriptAgent, diff --git a/pkg/api/frontendsettings_test.go b/pkg/api/frontendsettings_test.go index 3e21a994c10..26c3a939090 100644 --- a/pkg/api/frontendsettings_test.go +++ b/pkg/api/frontendsettings_test.go @@ -13,7 +13,6 @@ import ( pluginSettings "github.com/grafana/grafana/pkg/services/pluginsettings/service" "github.com/grafana/grafana/pkg/services/rendering" "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" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/updatechecker" @@ -56,7 +55,6 @@ func setupTestEnvironment(t *testing.T, cfg *setting.Cfg, features *featuremgmt. grafanaUpdateChecker: &updatechecker.GrafanaService{}, AccessControl: accesscontrolmock.New().WithDisabled(), PluginSettings: pluginSettings.ProvideService(sqlStore, secretsService), - remoteSecretsCheck: &kvstore.OSSRemoteSecretsPluginCheck{}, } m := web.New() diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index f2ac79c0edc..705dd12f3a4 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -74,7 +74,6 @@ import ( "github.com/grafana/grafana/pkg/services/search" "github.com/grafana/grafana/pkg/services/searchusers" "github.com/grafana/grafana/pkg/services/secrets" - secretsKV "github.com/grafana/grafana/pkg/services/secrets/kvstore" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/shorturls" "github.com/grafana/grafana/pkg/services/sqlstore" @@ -140,7 +139,7 @@ type HTTPServer struct { Listener net.Listener EncryptionService encryption.Internal SecretsService secrets.Service - remoteSecretsCheck secretsKV.UseRemoteSecretsPluginCheck + secretsPluginManager plugins.SecretsPluginManager DataSourcesService datasources.DataSourceService cleanUpService *cleanup.CleanUpService tracer tracing.Tracer @@ -211,7 +210,8 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi teamsPermissionsService accesscontrol.TeamPermissionsService, folderPermissionsService accesscontrol.FolderPermissionsService, dashboardPermissionsService accesscontrol.DashboardPermissionsService, dashboardVersionService dashver.Service, starService star.Service, csrfService csrf.Service, coremodels *registry.Base, - playlistService playlist.Service, apiKeyService apikey.Service, kvStore kvstore.KVStore, secretsMigrator secrets.Migrator, remoteSecretsCheck secretsKV.UseRemoteSecretsPluginCheck, + playlistService playlist.Service, apiKeyService apikey.Service, kvStore kvstore.KVStore, + secretsMigrator secrets.Migrator, secretsPluginManager plugins.SecretsPluginManager, publicDashboardsApi *publicdashboardsApi.Api, userService user.Service) (*HTTPServer, error) { web.Env = cfg.Env m := web.New() @@ -267,7 +267,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi SocialService: socialService, EncryptionService: encryptionService, SecretsService: secretsService, - remoteSecretsCheck: remoteSecretsCheck, + secretsPluginManager: secretsPluginManager, DataSourcesService: dataSourcesService, searchUsersService: searchUsersService, ldapGroups: ldapGroups, diff --git a/pkg/cmd/grafana-cli/runner/wire.go b/pkg/cmd/grafana-cli/runner/wire.go index 792c13fefcd..a3d2818de43 100644 --- a/pkg/cmd/grafana-cli/runner/wire.go +++ b/pkg/cmd/grafana-cli/runner/wire.go @@ -129,6 +129,7 @@ import ( "github.com/grafana/grafana/pkg/tsdb/loki" "github.com/grafana/grafana/pkg/tsdb/mssql" "github.com/grafana/grafana/pkg/tsdb/mysql" + "github.com/grafana/grafana/pkg/tsdb/opentsdb" "github.com/grafana/grafana/pkg/tsdb/postgres" "github.com/grafana/grafana/pkg/tsdb/prometheus" "github.com/grafana/grafana/pkg/tsdb/tempo" @@ -318,6 +319,7 @@ var wireSet = wire.NewSet( wire.Bind(new(sqlstore.Store), new(*sqlstore.SQLStore)), wire.Bind(new(db.DB), new(*sqlstore.SQLStore)), prefimpl.ProvideService, + opentsdb.ProvideService, ) func Initialize(cfg *setting.Cfg) (Runner, error) { diff --git a/pkg/cmd/grafana-cli/runner/wireexts_oss.go b/pkg/cmd/grafana-cli/runner/wireexts_oss.go index 2373203a3c1..ddbc19a5091 100644 --- a/pkg/cmd/grafana-cli/runner/wireexts_oss.go +++ b/pkg/cmd/grafana-cli/runner/wireexts_oss.go @@ -32,7 +32,6 @@ import ( "github.com/grafana/grafana/pkg/services/provisioning" "github.com/grafana/grafana/pkg/services/searchusers" "github.com/grafana/grafana/pkg/services/searchusers/filters" - secretsStore "github.com/grafana/grafana/pkg/services/secrets/kvstore" "github.com/grafana/grafana/pkg/services/sqlstore/migrations" "github.com/grafana/grafana/pkg/services/thumbs" "github.com/grafana/grafana/pkg/services/validations" @@ -88,8 +87,6 @@ var wireExtsSet = wire.NewSet( wire.Bind(new(registry.UsageStatsProvidersRegistry), new(*usagestatssvcs.UsageStatsProvidersRegistry)), ossaccesscontrol.ProvideDatasourcePermissionsService, wire.Bind(new(accesscontrol.DatasourcePermissionsService), new(*ossaccesscontrol.DatasourcePermissionsService)), - secretsStore.ProvideRemotePluginCheck, - wire.Bind(new(secretsStore.UseRemoteSecretsPluginCheck), new(*secretsStore.OSSRemoteSecretsPluginCheck)), encryptionprovider.ProvideEncryptionProvider, wire.Bind(new(encryption.Provider), new(encryptionprovider.Provider)), ) diff --git a/pkg/server/wireexts_oss.go b/pkg/server/wireexts_oss.go index 8f7fe3fb088..2fa20af111d 100644 --- a/pkg/server/wireexts_oss.go +++ b/pkg/server/wireexts_oss.go @@ -31,7 +31,6 @@ import ( "github.com/grafana/grafana/pkg/services/provisioning" "github.com/grafana/grafana/pkg/services/searchusers" "github.com/grafana/grafana/pkg/services/searchusers/filters" - secretsStore "github.com/grafana/grafana/pkg/services/secrets/kvstore" "github.com/grafana/grafana/pkg/services/sqlstore/migrations" "github.com/grafana/grafana/pkg/services/thumbs" "github.com/grafana/grafana/pkg/services/validations" @@ -86,8 +85,6 @@ var wireExtsBasicSet = wire.NewSet( wire.Bind(new(registry.UsageStatsProvidersRegistry), new(*usagestatssvcs.UsageStatsProvidersRegistry)), ossaccesscontrol.ProvideDatasourcePermissionsService, wire.Bind(new(accesscontrol.DatasourcePermissionsService), new(*ossaccesscontrol.DatasourcePermissionsService)), - secretsStore.ProvideRemotePluginCheck, - wire.Bind(new(secretsStore.UseRemoteSecretsPluginCheck), new(*secretsStore.OSSRemoteSecretsPluginCheck)), ) var wireExtsSet = wire.NewSet( diff --git a/pkg/services/secrets/kvstore/kvstore.go b/pkg/services/secrets/kvstore/kvstore.go index 14444387df1..7cd2a776589 100644 --- a/pkg/services/secrets/kvstore/kvstore.go +++ b/pkg/services/secrets/kvstore/kvstore.go @@ -2,15 +2,16 @@ package kvstore import ( "context" - "errors" - "fmt" "time" "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/featuremgmt" "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/setting" ) const ( @@ -21,12 +22,13 @@ const ( func ProvideService( sqlStore sqlstore.Store, secretsService secrets.Service, - remoteCheck UseRemoteSecretsPluginCheck, + pluginsManager plugins.SecretsPluginManager, kvstore kvstore.KVStore, features featuremgmt.FeatureToggles, + cfg *setting.Cfg, ) (SecretsKVStore, error) { + var logger = log.New("secrets.kvstore") var store SecretsKVStore - logger := log.New("secrets.kvstore") store = &secretsKVStoreSQL{ sqlStore: sqlStore, secretsService: secretsService, @@ -35,21 +37,24 @@ func ProvideService( cache: make(map[int64]cachedDecrypted), }, } - if remoteCheck.ShouldUseRemoteSecretsPlugin() { + err := EvaluateRemoteSecretsPlugin(pluginsManager, cfg) + if err != nil { + logger.Debug(err.Error()) + } else { // Attempt to start the plugin - secretsPlugin, err := remoteCheck.StartAndReturnPlugin(context.Background()) + var secretsPlugin secretsmanagerplugin.SecretsManagerPlugin + secretsPlugin, err = startAndReturnPlugin(pluginsManager, context.Background()) namespacedKVStore := GetNamespacedKVStore(kvstore) if err != nil || secretsPlugin == nil { - if isFatal, err2 := isPluginStartupErrorFatal(context.Background(), namespacedKVStore); isFatal || err2 != nil { + logger.Error("failed to start remote secrets management plugin", "msg", err.Error()) + 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 err2 != nil { - // TODO decide whether an error here should actually crash the app - return nil, err2 + if readErr != nil { + return nil, readErr } return nil, err } - logger.Error("error starting secrets plugin, falling back to SQL implementation") } else { store = &secretsKVStorePlugin{ secretsPlugin: secretsPlugin, @@ -59,7 +64,9 @@ func ProvideService( backwardsCompatibilityDisabled: features.IsEnabled(featuremgmt.FlagDisableSecretsCompatibility), } } - } else { + } + + if err != nil { logger.Debug("secrets kvstore is using the default (SQL) implementation for secrets management") } @@ -117,23 +124,3 @@ func (kv *FixedKVStore) Rename(ctx context.Context, newNamespace string) error { kv.Namespace = newNamespace return nil } - -// Helpers to determine whether plugin startup failures are fatal to the app -func GetNamespacedKVStore(kv kvstore.KVStore) *kvstore.NamespacedKVStore { - return kvstore.WithNamespace(kv, kvstore.AllOrganizations, PluginNamespace) -} - -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())) - } - return exists, nil -} - -func setPluginStartupErrorFatal(ctx context.Context, kvstore *kvstore.NamespacedKVStore, isFatal bool) error { - if !isFatal { - return kvstore.Del(ctx, QuitOnPluginStartupFailureKey) - } - return kvstore.Set(ctx, QuitOnPluginStartupFailureKey, "true") -} diff --git a/pkg/services/secrets/kvstore/plugin_fatal_crash_test.go b/pkg/services/secrets/kvstore/plugin_fatal_crash_test.go index ffa4b5afd91..501934ab89e 100644 --- a/pkg/services/secrets/kvstore/plugin_fatal_crash_test.go +++ b/pkg/services/secrets/kvstore/plugin_fatal_crash_test.go @@ -80,25 +80,23 @@ func TestFatalPluginErr_MigrationTestWithErrorDeletingUnifiedSecrets(t *testing. func setupFatalCrashTest( t *testing.T, - shouldRemoteCheckError bool, + shouldFailOnStart bool, isPluginErrorFatal bool, isBackwardsCompatDisabled bool, ) (SecretsKVStore, kvstore.KVStore, *sqlstore.SQLStore, error) { t.Helper() + fatalFlagOnce = sync.Once{} + startupOnce = sync.Once{} + cfg := setupTestConfig(t) sqlStore := sqlstore.InitTestDB(t) secretService := fakes.FakeSecretsService{} - var remoteCheck *mockRemoteSecretsPluginCheck - if shouldRemoteCheckError { - remoteCheck = provideMockRemotePluginCheckWithErr() - } else { - remoteCheck = provideMockRemotePluginCheck() - } kvstore := kvstore.ProvideService(sqlStore) if isPluginErrorFatal { _ = setPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(kvstore), true) } features := NewFakeFeatureToggles(t, isBackwardsCompatDisabled) - svc, err := ProvideService(sqlStore, secretService, remoteCheck, kvstore, features) + manager := NewFakeSecretsPluginManager(t, shouldFailOnStart) + svc, err := ProvideService(sqlStore, secretService, manager, kvstore, features, cfg) t.Cleanup(func() { fatalFlagOnce = sync.Once{} }) @@ -112,15 +110,9 @@ func setupTestMigratorServiceWithDeletionError( kvstore kvstore.KVStore, ) *PluginSecretMigrationService { t.Helper() - rawCfg := ` - [secrets] - use_plugin = true - migrate_to_plugin = true - ` - raw, err := ini.Load([]byte(rawCfg)) - require.NoError(t, err) - cfg := &setting.Cfg{Raw: raw} - remoteCheck := provideMockRemotePluginCheck() + fatalFlagOnce = sync.Once{} + startupOnce = sync.Once{} + cfg := setupTestConfig(t) secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) getAllFuncOverride := func(ctx context.Context) ([]Item, error) { items := make([]Item, 0) @@ -135,15 +127,28 @@ func setupTestMigratorServiceWithDeletionError( }) return items, nil } + manager := NewFakeSecretsPluginManager(t, false) migratorService := ProvidePluginSecretMigrationService( secretskv, cfg, sqlStore, secretsService, - remoteCheck, kvstore, + manager, ) // TODO refactor Migrator to allow us to override the entire sqlstore with a mock instead migratorService.overrideGetAllFunc(getAllFuncOverride) return migratorService } + +func setupTestConfig(t *testing.T) *setting.Cfg { + t.Helper() + rawCfg := ` + [secrets] + use_plugin = true + migrate_to_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_mig.go b/pkg/services/secrets/kvstore/plugin_mig.go index a1c48f0315a..f1d3986fa45 100644 --- a/pkg/services/secrets/kvstore/plugin_mig.go +++ b/pkg/services/secrets/kvstore/plugin_mig.go @@ -6,6 +6,7 @@ import ( "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" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" @@ -19,8 +20,8 @@ type PluginSecretMigrationService struct { logger log.Logger sqlStore sqlstore.Store secretsService secrets.Service - remoteCheck UseRemoteSecretsPluginCheck kvstore kvstore.KVStore + manager plugins.SecretsPluginManager getAllFunc func(ctx context.Context) ([]Item, error) } @@ -29,8 +30,8 @@ func ProvidePluginSecretMigrationService( cfg *setting.Cfg, sqlStore sqlstore.Store, secretsService secrets.Service, - remoteCheck UseRemoteSecretsPluginCheck, kvstore kvstore.KVStore, + manager plugins.SecretsPluginManager, ) *PluginSecretMigrationService { return &PluginSecretMigrationService{ secretsStore: secretsStore, @@ -38,14 +39,14 @@ func ProvidePluginSecretMigrationService( logger: log.New("sec-plugin-mig"), sqlStore: sqlStore, secretsService: secretsService, - remoteCheck: remoteCheck, kvstore: kvstore, + manager: manager, } } func (s *PluginSecretMigrationService) Migrate(ctx context.Context) error { // Check if we should migrate to plugin - default false - if s.cfg.SectionWithEnvOverrides("secrets").Key("migrate_to_plugin").MustBool(false) && s.remoteCheck.ShouldUseRemoteSecretsPlugin() { + if err := EvaluateRemoteSecretsPlugin(s.manager, s.cfg); err == nil { s.logger.Debug("starting migration of unified secrets to the plugin") // we need to instantiate the secretsKVStore as this is not on wire, and in this scenario, // the secrets store would be the plugin. diff --git a/pkg/services/secrets/kvstore/plugin_mig_test.go b/pkg/services/secrets/kvstore/plugin_mig_test.go index dd6ea614654..ee35084ef59 100644 --- a/pkg/services/secrets/kvstore/plugin_mig_test.go +++ b/pkg/services/secrets/kvstore/plugin_mig_test.go @@ -77,19 +77,18 @@ func setupTestMigratorService(t *testing.T) (*PluginSecretMigrationService, Secr cfg := &setting.Cfg{Raw: raw} // this would be the plugin - mocked at the moment secretsStoreForPlugin := NewFakeSecretsKVStore() - // Mocked remote plugin check, always return true - remoteCheck := provideMockRemotePluginCheck() // this is to init the sql secret store inside the migration sqlStore := sqlstore.InitTestDB(t) secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + manager := NewFakeSecretsPluginManager(t, false) migratorService := ProvidePluginSecretMigrationService( secretsStoreForPlugin, cfg, sqlStore, secretsService, - remoteCheck, kvstore.ProvideService(sqlStore), + manager, ) secretsSql := &secretsKVStoreSQL{ diff --git a/pkg/services/secrets/kvstore/remote_plugin.go b/pkg/services/secrets/kvstore/remote_plugin.go index a038ca7ce83..cfcbc4e1408 100644 --- a/pkg/services/secrets/kvstore/remote_plugin.go +++ b/pkg/services/secrets/kvstore/remote_plugin.go @@ -2,17 +2,24 @@ package kvstore import ( "context" + "errors" + "fmt" "sync" "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/plugins" smp "github.com/grafana/grafana/pkg/plugins/backendplugin/secretsmanagerplugin" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/secrets" + "github.com/grafana/grafana/pkg/setting" ) var ( - fatalFlagOnce sync.Once + fatalFlagOnce sync.Once + startupOnce sync.Once + errPluginDisabledByConfig = errors.New("remote secret managements plugin disabled because the property `secrets.use_plugin` is not set to `true`") + errPluginNotInstalled = errors.New("remote secret managements plugin disabled because there is no installed plugin of type `secretsmanager`") ) // secretsKVStorePlugin provides a key/value store backed by the Grafana plugin gRPC interface @@ -163,3 +170,45 @@ func updateFatalFlag(ctx context.Context, skv secretsKVStorePlugin) { func wrapUserFriendlySecretError(ufe string) datasources.ErrDatasourceSecretsPluginUserFriendly { return datasources.ErrDatasourceSecretsPluginUserFriendly{Err: ufe} } + +func GetNamespacedKVStore(kv kvstore.KVStore) *kvstore.NamespacedKVStore { + return kvstore.WithNamespace(kv, kvstore.AllOrganizations, PluginNamespace) +} + +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())) + } + return exists, nil +} + +func setPluginStartupErrorFatal(ctx context.Context, kvstore *kvstore.NamespacedKVStore, isFatal bool) error { + if !isFatal { + return kvstore.Del(ctx, QuitOnPluginStartupFailureKey) + } + return kvstore.Set(ctx, QuitOnPluginStartupFailureKey, "true") +} + +func EvaluateRemoteSecretsPlugin(mg plugins.SecretsPluginManager, cfg *setting.Cfg) error { + usePlugin := cfg.SectionWithEnvOverrides("secrets").Key("use_plugin").MustBool() + if !usePlugin { + return errPluginDisabledByConfig + } + pluginInstalled := mg.SecretsManager() != nil + if !pluginInstalled { + return errPluginNotInstalled + } + return nil +} + +func startAndReturnPlugin(mg plugins.SecretsPluginManager, ctx context.Context) (smp.SecretsManagerPlugin, error) { + var err error + startupOnce.Do(func() { + err = mg.SecretsManager().Start(ctx) + }) + if err != nil { + return nil, err + } + return mg.SecretsManager().SecretsManager, nil +} diff --git a/pkg/services/secrets/kvstore/remote_plugin_check.go b/pkg/services/secrets/kvstore/remote_plugin_check.go deleted file mode 100644 index a415bdbb68c..00000000000 --- a/pkg/services/secrets/kvstore/remote_plugin_check.go +++ /dev/null @@ -1,34 +0,0 @@ -package kvstore - -import ( - "context" - - "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/plugins/backendplugin/secretsmanagerplugin" -) - -type UseRemoteSecretsPluginCheck interface { - ShouldUseRemoteSecretsPlugin() bool - StartAndReturnPlugin(ctx context.Context) (secretsmanagerplugin.SecretsManagerPlugin, error) -} - -type OSSRemoteSecretsPluginCheck struct { - log log.Logger -} - -func ProvideRemotePluginCheck() *OSSRemoteSecretsPluginCheck { - return &OSSRemoteSecretsPluginCheck{ - log: log.New("ossremotesecretsplugincheck"), - } -} - -func (c OSSRemoteSecretsPluginCheck) ShouldUseRemoteSecretsPlugin() bool { - return false -} - -func (c OSSRemoteSecretsPluginCheck) StartAndReturnPlugin(ctx context.Context) (secretsmanagerplugin.SecretsManagerPlugin, error) { - c.log.Warn("OSSRemoteSecretsPluginCheck.StartAndReturnPlugin() was called by mistake. Secrets Manager plugins are enterprise only.") - return nil, nil -} - -var _ UseRemoteSecretsPluginCheck = OSSRemoteSecretsPluginCheck{} diff --git a/pkg/services/secrets/kvstore/test_helpers.go b/pkg/services/secrets/kvstore/test_helpers.go index f75041563b6..82792a4fe9f 100644 --- a/pkg/services/secrets/kvstore/test_helpers.go +++ b/pkg/services/secrets/kvstore/test_helpers.go @@ -6,6 +6,8 @@ import ( "testing" "github.com/grafana/grafana/pkg/infra/log" + "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" @@ -98,34 +100,6 @@ func (f fakeFeatureToggles) IsEnabled(feature string) bool { return f.returnValue } -// Fake Remote Secrets Plugin Check -type mockRemoteSecretsPluginCheck struct { - UseRemoteSecretsPluginCheck - shouldReturnError bool -} - -func provideMockRemotePluginCheck() *mockRemoteSecretsPluginCheck { - return &mockRemoteSecretsPluginCheck{} -} - -func provideMockRemotePluginCheckWithErr() *mockRemoteSecretsPluginCheck { - return &mockRemoteSecretsPluginCheck{ - shouldReturnError: true, - } -} - -func (c *mockRemoteSecretsPluginCheck) ShouldUseRemoteSecretsPlugin() bool { - return true -} - -func (c *mockRemoteSecretsPluginCheck) StartAndReturnPlugin(ctx context.Context) (secretsmanagerplugin.SecretsManagerPlugin, error) { - var err error - if c.shouldReturnError { - err = errors.New("bogus") - } - return &fakeGRPCSecretsPlugin{}, err -} - // Fake grpc secrets plugin impl type fakeGRPCSecretsPlugin struct{} @@ -156,3 +130,38 @@ func (c *fakeGRPCSecretsPlugin) RenameSecret(ctx context.Context, in *secretsman var _ SecretsKVStore = FakeSecretsKVStore{} var _ secretsmanagerplugin.SecretsManagerPlugin = &fakeGRPCSecretsPlugin{} + +// Fake plugin manager +type fakePluginManager struct { + shouldFailOnStart bool +} + +func (mg *fakePluginManager) SecretsManager() *plugins.Plugin { + p := &plugins.Plugin{ + SecretsManager: &fakeGRPCSecretsPlugin{}, + } + p.RegisterClient(&fakePluginClient{ + shouldFailOnStart: mg.shouldFailOnStart, + }) + return p +} + +func NewFakeSecretsPluginManager(t *testing.T, shouldFailOnStart bool) plugins.SecretsPluginManager { + t.Helper() + return &fakePluginManager{ + shouldFailOnStart: shouldFailOnStart, + } +} + +// Fake plugin client +type fakePluginClient struct { + shouldFailOnStart bool + backendplugin.Plugin +} + +func (pc *fakePluginClient) Start(_ context.Context) error { + if pc.shouldFailOnStart { + return errors.New("failed to start") + } + return nil +}