Secrets: Improve error handling for secrets manager plugin (#54811)

* Improve error handling for secrets manager plugin

* Update sever lock log for secret migration to Error

* Move plugin started check to a separate function

* Fix nil pointer exception on HasPluginStarted

pkg/services/secrets/kvstore/kvstore.go

@@ -40,7 +40,7 @@ func ProvideService(
 		secretsPlugin, err = StartAndReturnPlugin(pluginsManager, ctx)
 		namespacedKVStore := GetNamespacedKVStore(kvstore)
 		if err != nil || secretsPlugin == nil {
-			logger.Error("failed to start remote secrets management plugin", "msg", err.Error())
+			logger.Error("failed to start remote secrets management plugin")
 			if isFatal, readErr := IsPluginStartupErrorFatal(ctx, 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")

pkg/services/secrets/kvstore/migrations/from_plugin_mig.go

@@ -2,6 +2,7 @@ package migrations
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/grafana/grafana/pkg/infra/kvstore"
@@ -13,6 +14,8 @@ import (
 	"github.com/grafana/grafana/pkg/setting"
 )
 
+var errPluginUnavailable = errors.New("remote secret management plugin is unavailable")
+
 // MigrateFromPluginService This migrator will handle migration of the configured plugin secrets back to Grafana unified secrets
 type MigrateFromPluginService struct {
 	cfg            *setting.Cfg
@@ -44,8 +47,7 @@ func (s *MigrateFromPluginService) Migrate(ctx context.Context) error {
 	// access the plugin directly
 	plugin, err := secretskvs.StartAndReturnPlugin(s.manager, context.Background())
 	if err != nil {
-		logger.Error("Error retrieiving plugin", "error", err.Error())
-		return err
+		return errPluginUnavailable
 	}
 	// Get full list of secrets from the plugin
 	res, err := plugin.GetAllSecrets(ctx, &secretsmanagerplugin.GetAllSecretsRequest{})

pkg/services/secrets/kvstore/migrations/migrator.go

@@ -67,7 +67,7 @@ func (s *SecretMigrationProviderImpl) Run(ctx context.Context) error {
 // This should only be called once at startup
 func (s *SecretMigrationProviderImpl) Migrate(ctx context.Context) error {
 	// Start migration services.
-	return s.ServerLockService.LockExecuteAndRelease(ctx, actionName, time.Minute*10, func(context.Context) {
+	err := s.ServerLockService.LockExecuteAndRelease(ctx, actionName, time.Minute*10, func(context.Context) {
 		for _, service := range s.services {
 			serviceName := reflect.TypeOf(service).String()
 			logger.Debug("Starting secret migration service", "service", serviceName)
@@ -78,6 +78,10 @@ func (s *SecretMigrationProviderImpl) Migrate(ctx context.Context) error {
 			logger.Debug("Finished secret migration service", "service", serviceName)
 		}
 	})
+	if err != nil {
+		logger.Error("Server lock for secret migration already exists")
+	}
+	return nil
 }
 
 // TriggerPluginMigration Kick off a migration to or from the plugin. This will block until all services have exited.

pkg/services/secrets/kvstore/migrations/to_plugin_mig.go

@@ -45,7 +45,9 @@ func ProvideMigrateToPluginService(
 }
 
 func (s *MigrateToPluginService) Migrate(ctx context.Context) error {
-	if err := secretskvs.EvaluateRemoteSecretsPlugin(ctx, s.manager, s.cfg); err == nil {
+	err := secretskvs.EvaluateRemoteSecretsPlugin(ctx, s.manager, s.cfg)
+	hasStarted := secretskvs.HasPluginStarted(ctx, s.manager)
+	if err == nil && hasStarted {
 		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.
 		tmpStore, err := secretskvs.GetUnwrappedStoreFromCache(s.secretsStore)

pkg/services/secrets/kvstore/plugin.go

@@ -19,8 +19,8 @@ import (
 var (
 	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`")
+	errPluginDisabledByConfig = errors.New("remote secret management plugin disabled because the property `secrets.use_plugin` is not set to `true`")
+	errPluginNotInstalled     = errors.New("remote secret management plugin disabled because there is no installed plugin of type `secretsmanager`")
 )
 
 // SecretsKVStorePlugin provides a key/value store backed by the Grafana plugin gRPC interface
@@ -267,6 +267,10 @@ func EvaluateRemoteSecretsPlugin(ctx context.Context, mg plugins.SecretsPluginMa
 	return nil
 }
 
+func HasPluginStarted(ctx context.Context, mg plugins.SecretsPluginManager) bool {
+	return mg.SecretsManager(ctx) != nil && mg.SecretsManager(ctx).SecretsManager != nil
+}
+
 func StartAndReturnPlugin(mg plugins.SecretsPluginManager, ctx context.Context) (smp.SecretsManagerPlugin, error) {
 	var err error
 	startupOnce.Do(func() {