chore: refactor SecretMigrationProviderImpl as a dskit module (#71944)

* chore: refactor SecretMigrationProviderImpl as a dskit service

pkg/modules/dependencies.go

@@ -13,14 +13,16 @@ const (
 	HTTPServer string = "http-server"
 	// Provisioning sets up Grafana with preconfigured datasources, dashboards, etc.
 	Provisioning string = "provisioning"
+	// SecretMigrator handles legacy secrets migrations
+	SecretMigrator string = "secret-migrator"
 )
 
 // dependencyMap defines Module Targets => Dependencies
 var dependencyMap = map[string][]string{
 	BackgroundServices: {Provisioning, HTTPServer},
+	CertGenerator:      {},
+	GrafanaAPIServer:   {CertGenerator},
+	Provisioning:       {SecretMigrator},
 
-	CertGenerator:    {},
+	All: {BackgroundServices},
-	GrafanaAPIServer: {CertGenerator},
-
-	All: {Provisioning, HTTPServer, BackgroundServices},
 }

pkg/modules/registry/registry.go

@@ -10,6 +10,7 @@ import (
 	"github.com/grafana/grafana/pkg/server/backgroundsvcs"
 	grafanaapiserver "github.com/grafana/grafana/pkg/services/grafana-apiserver"
 	"github.com/grafana/grafana/pkg/services/provisioning"
+	"github.com/grafana/grafana/pkg/services/secrets/kvstore/migrations"
 )
 
 type Registry interface{}
@@ -26,6 +27,7 @@ func ProvideRegistry(
 	certGenerator certgenerator.ServiceInterface,
 	httpServer *api.HTTPServer,
 	provisioningService *provisioning.ProvisioningServiceImpl,
+	secretsMigrator *migrations.SecretMigrationProviderImpl,
 ) *registry {
 	return newRegistry(
 		log.New("modules.registry"),
@@ -35,6 +37,7 @@ func ProvideRegistry(
 		certGenerator,
 		httpServer,
 		provisioningService,
+		secretsMigrator,
 	)
 }
 

pkg/server/backgroundsvcs/background_services.go

@@ -28,7 +28,6 @@ import (
 	publicdashboardsmetric "github.com/grafana/grafana/pkg/services/publicdashboards/metric"
 	"github.com/grafana/grafana/pkg/services/rendering"
 	"github.com/grafana/grafana/pkg/services/searchV2"
-	secretsMigrations "github.com/grafana/grafana/pkg/services/secrets/kvstore/migrations"
 	secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	samanager "github.com/grafana/grafana/pkg/services/serviceaccounts/manager"
@@ -48,7 +47,7 @@ func ProvideBackgroundServiceRegistry(
 	pluginsUpdateChecker *updatechecker.PluginsService, metrics *metrics.InternalMetricsService,
 	secretsService *secretsManager.SecretsService, remoteCache *remotecache.RemoteCache, StorageService store.StorageService, searchService searchV2.SearchService, entityEventsService store.EntityEventsService,
 	saService *samanager.ServiceAccountsService, authInfoService *authinfoservice.Implementation,
-	grpcServerProvider grpcserver.Provider, secretMigrationProvider secretsMigrations.SecretMigrationProvider, loginAttemptService *loginattemptimpl.Service,
+	grpcServerProvider grpcserver.Provider, loginAttemptService *loginattemptimpl.Service,
 	bundleService *supportbundlesimpl.Service,
 	publicDashboardsMetric *publicdashboardsmetric.Service,
 	keyRetriever *dynamic.KeyRetriever,
@@ -84,7 +83,6 @@ func ProvideBackgroundServiceRegistry(
 		saService,
 		authInfoService,
 		processManager,
-		secretMigrationProvider,
 		loginAttemptService,
 		bundleService,
 		publicDashboardsMetric,

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

@@ -5,9 +5,11 @@ import (
 	"reflect"
 	"time"
 
+	"github.com/grafana/dskit/services"
+
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/serverlock"
-	"github.com/grafana/grafana/pkg/registry"
+	"github.com/grafana/grafana/pkg/modules"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -21,15 +23,21 @@ type SecretMigrationService interface {
 }
 
 type SecretMigrationProvider interface {
-	registry.BackgroundService
 	TriggerPluginMigration(ctx context.Context, toPlugin bool) error
 }
 
 type SecretMigrationProviderImpl struct {
-	services                 []SecretMigrationService
+	migServices              []SecretMigrationService
 	ServerLockService        *serverlock.ServerLockService
 	migrateToPluginService   *MigrateToPluginService
 	migrateFromPluginService *MigrateFromPluginService
+
+	// SecretMigrationProviderImpl is a dskit module Note on dskit module usage:
+	// The SecretMigrationProviderImpl iterates over several service's
+	// Migration() method sequentially. dskit has the concept of a service
+	// Manager which launches services. We could use the Manager here, but it
+	// seems heavyweight given that these services only log errors.
+	*services.BasicService
 }
 
 func ProvideSecretMigrationProvider(
@@ -39,27 +47,30 @@ func ProvideSecretMigrationProvider(
 	migrateToPluginService *MigrateToPluginService,
 	migrateFromPluginService *MigrateFromPluginService,
 ) *SecretMigrationProviderImpl {
-	services := make([]SecretMigrationService, 0)
+	migServices := make([]SecretMigrationService, 0)
-	services = append(services, dataSourceSecretMigrationService)
+	migServices = append(migServices, dataSourceSecretMigrationService)
 	// Plugin migration should always be last; should either migrate to or from, not both
 	// This is because the migrateTo checks for use_plugin = true, in which case we should always
 	// migrate by default to ensure users don't lose access to secrets. If migration has
 	// already occurred, the migrateTo function will be called but it won't do anything
 	if cfg.SectionWithEnvOverrides("secrets").Key("migrate_from_plugin").MustBool(false) {
-		services = append(services, migrateFromPluginService)
+		migServices = append(migServices, migrateFromPluginService)
 	} else {
-		services = append(services, migrateToPluginService)
+		migServices = append(migServices, migrateToPluginService)
 	}
 
-	return &SecretMigrationProviderImpl{
+	s := &SecretMigrationProviderImpl{
 		ServerLockService:        serverLockService,
-		services:                 services,
+		migServices:              migServices,
 		migrateToPluginService:   migrateToPluginService,
 		migrateFromPluginService: migrateFromPluginService,
 	}
+
+	s.BasicService = services.NewIdleService(s.start, nil).WithName(modules.SecretMigrator)
+	return s
 }
 
-func (s *SecretMigrationProviderImpl) Run(ctx context.Context) error {
+func (s *SecretMigrationProviderImpl) start(ctx context.Context) error {
 	return s.Migrate(ctx)
 }
 
@@ -68,7 +79,7 @@ func (s *SecretMigrationProviderImpl) Run(ctx context.Context) error {
 func (s *SecretMigrationProviderImpl) Migrate(ctx context.Context) error {
 	// Start migration services.
 	err := s.ServerLockService.LockExecuteAndRelease(ctx, actionName, time.Minute*10, func(context.Context) {
-		for _, service := range s.services {
+		for _, service := range s.migServices {
 			serviceName := reflect.TypeOf(service).String()
 			logger.Debug("Starting secret migration service", "service", serviceName)
 			err := service.Migrate(ctx)