Secrets: Convert secret migration to a background service (#54676)

* Convert secret migration to a background service

* Fix merge conflicts

* Return concrete type for secret migration provider
This commit is contained in:
Guilherme Caulada 2022-09-06 16:21:25 -03:00 committed by GitHub
parent 0f549a44b0
commit 20589f76f4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 48 additions and 55 deletions

View File

@ -147,7 +147,7 @@ type HTTPServer struct {
secretsPluginManager plugins.SecretsPluginManager
secretsStore secretsKV.SecretsKVStore
secretsMigrator secrets.Migrator
secretsPluginMigrator *spm.SecretMigrationServiceImpl
secretsPluginMigrator spm.SecretMigrationProvider
DataSourcesService datasources.DataSourceService
cleanUpService *cleanup.CleanUpService
tracer tracing.Tracer
@ -225,7 +225,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
starService star.Service, csrfService csrf.Service, coremodels *registry.Base,
playlistService playlist.Service, apiKeyService apikey.Service, kvStore kvstore.KVStore,
secretsMigrator secrets.Migrator, secretsPluginManager plugins.SecretsPluginManager, secretsService secrets.Service,
secretsPluginMigrator *spm.SecretMigrationServiceImpl, secretsStore secretsKV.SecretsKVStore,
secretsPluginMigrator spm.SecretMigrationProvider, secretsStore secretsKV.SecretsKVStore,
publicDashboardsApi *publicdashboardsApi.Api, userService user.Service, tempUserService tempUser.Service, loginAttemptService loginAttempt.Service, orgService org.Service,
accesscontrolService accesscontrol.Service, dashboardThumbsService dashboardThumbs.Service,
) (*HTTPServer, error) {

View File

@ -103,7 +103,6 @@ import (
"github.com/grafana/grafana/pkg/services/secrets"
secretsDatabase "github.com/grafana/grafana/pkg/services/secrets/database"
secretsStore "github.com/grafana/grafana/pkg/services/secrets/kvstore"
secretsMigrations "github.com/grafana/grafana/pkg/services/secrets/kvstore/migrations"
secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager"
secretsMigrator "github.com/grafana/grafana/pkg/services/secrets/migrator"
"github.com/grafana/grafana/pkg/services/serviceaccounts"
@ -318,10 +317,6 @@ var wireSet = wire.NewSet(
publicdashboardsApi.ProvideApi,
userimpl.ProvideService,
orgimpl.ProvideService,
secretsMigrations.ProvideDataSourceMigrationService,
secretsMigrations.ProvideMigrateToPluginService,
secretsMigrations.ProvideSecretMigrationService,
wire.Bind(new(secretsMigrations.SecretMigrationService), new(*secretsMigrations.SecretMigrationServiceImpl)),
userauthimpl.ProvideService,
ngmetrics.ProvideServiceForTest,
wire.Bind(new(sqlstore.TeamStore), new(*sqlstore.SQLStore)),

View File

@ -23,6 +23,7 @@ import (
"github.com/grafana/grafana/pkg/services/provisioning"
"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"
@ -42,6 +43,7 @@ func ProvideBackgroundServiceRegistry(
secretsService *secretsManager.SecretsService, remoteCache *remotecache.RemoteCache,
thumbnailsService thumbs.Service, StorageService store.StorageService, searchService searchV2.SearchService, entityEventsService store.EntityEventsService,
saService *samanager.ServiceAccountsService, authInfoService *authinfoservice.Implementation,
secretMigrationProvider secretsMigrations.SecretMigrationProvider,
// Need to make sure these are initialized, is there a better place to put them?
_ dashboardsnapshots.Service, _ *alerting.AlertNotificationService,
_ serviceaccounts.Service, _ *guardian.Provider,
@ -73,6 +75,7 @@ func ProvideBackgroundServiceRegistry(
saService,
authInfoService,
processManager,
secretMigrationProvider,
)
}

View File

@ -23,7 +23,6 @@ import (
"github.com/grafana/grafana/pkg/login/social"
"github.com/grafana/grafana/pkg/registry"
"github.com/grafana/grafana/pkg/services/provisioning"
secretsMigrations "github.com/grafana/grafana/pkg/services/secrets/kvstore/migrations"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
@ -44,10 +43,10 @@ type Options struct {
func New(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistry accesscontrol.RoleRegistry,
provisioningService provisioning.ProvisioningService, backgroundServiceProvider registry.BackgroundServiceRegistry,
usageStatsProvidersRegistry registry.UsageStatsProvidersRegistry, statsCollectorService *statscollector.Service,
secretMigrationService secretsMigrations.SecretMigrationService, userService user.Service, loginAttemptService loginattempt.Service,
userService user.Service, loginAttemptService loginattempt.Service,
) (*Server, error) {
statsCollectorService.RegisterProviders(usageStatsProvidersRegistry.GetServices())
s, err := newServer(opts, cfg, httpServer, roleRegistry, provisioningService, backgroundServiceProvider, secretMigrationService, userService, loginAttemptService)
s, err := newServer(opts, cfg, httpServer, roleRegistry, provisioningService, backgroundServiceProvider, userService, loginAttemptService)
if err != nil {
return nil, err
}
@ -60,30 +59,28 @@ func New(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistr
}
func newServer(opts Options, cfg *setting.Cfg, httpServer *api.HTTPServer, roleRegistry accesscontrol.RoleRegistry,
provisioningService provisioning.ProvisioningService, backgroundServiceProvider registry.BackgroundServiceRegistry,
secretMigrationService secretsMigrations.SecretMigrationService, userService user.Service, loginAttemptService loginattempt.Service,
provisioningService provisioning.ProvisioningService, backgroundServiceProvider registry.BackgroundServiceRegistry, userService user.Service, loginAttemptService loginattempt.Service,
) (*Server, error) {
rootCtx, shutdownFn := context.WithCancel(context.Background())
childRoutines, childCtx := errgroup.WithContext(rootCtx)
s := &Server{
context: childCtx,
childRoutines: childRoutines,
HTTPServer: httpServer,
provisioningService: provisioningService,
roleRegistry: roleRegistry,
shutdownFn: shutdownFn,
shutdownFinished: make(chan struct{}),
log: log.New("server"),
cfg: cfg,
pidFile: opts.PidFile,
version: opts.Version,
commit: opts.Commit,
buildBranch: opts.BuildBranch,
backgroundServices: backgroundServiceProvider.GetServices(),
secretMigrationService: secretMigrationService,
userService: userService,
loginAttemptService: loginAttemptService,
context: childCtx,
childRoutines: childRoutines,
HTTPServer: httpServer,
provisioningService: provisioningService,
roleRegistry: roleRegistry,
shutdownFn: shutdownFn,
shutdownFinished: make(chan struct{}),
log: log.New("server"),
cfg: cfg,
pidFile: opts.PidFile,
version: opts.Version,
commit: opts.Commit,
buildBranch: opts.BuildBranch,
backgroundServices: backgroundServiceProvider.GetServices(),
userService: userService,
loginAttemptService: loginAttemptService,
}
return s, nil
@ -107,12 +104,11 @@ type Server struct {
buildBranch string
backgroundServices []registry.BackgroundService
HTTPServer *api.HTTPServer
roleRegistry accesscontrol.RoleRegistry
provisioningService provisioning.ProvisioningService
secretMigrationService secretsMigrations.SecretMigrationService
userService user.Service
loginAttemptService loginattempt.Service
HTTPServer *api.HTTPServer
roleRegistry accesscontrol.RoleRegistry
provisioningService provisioning.ProvisioningService
userService user.Service
loginAttemptService loginattempt.Service
}
// init initializes the server and its services.
@ -137,10 +133,6 @@ func (s *Server) init() error {
return err
}
if err := s.secretMigrationService.Migrate(s.context); err != nil {
return err
}
return s.provisioningService.RunInitProvisioners(s.context)
}

View File

@ -7,12 +7,9 @@ import (
"testing"
"time"
"github.com/grafana/grafana/pkg/infra/serverlock"
"github.com/grafana/grafana/pkg/registry"
"github.com/grafana/grafana/pkg/server/backgroundsvcs"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/secrets/kvstore/migrations"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/require"
@ -51,11 +48,7 @@ func (s *testService) IsDisabled() bool {
func testServer(t *testing.T, services ...registry.BackgroundService) *Server {
t.Helper()
serverLockService := serverlock.ProvideService(sqlstore.InitTestDB(t))
secretMigrationService := &migrations.SecretMigrationServiceImpl{
ServerLockService: serverLockService,
}
s, err := newServer(Options{}, setting.NewCfg(), nil, &acimpl.Service{}, nil, backgroundsvcs.NewBackgroundServiceRegistry(services...), secretMigrationService, usertest.NewUserServiceFake(), nil)
s, err := newServer(Options{}, setting.NewCfg(), nil, &acimpl.Service{}, nil, backgroundsvcs.NewBackgroundServiceRegistry(services...), usertest.NewUserServiceFake(), nil)
require.NoError(t, err)
// Required to skip configuration initialization that causes
// DI errors in this test.

View File

@ -334,8 +334,8 @@ var wireBasicSet = wire.NewSet(
secretsMigrations.ProvideDataSourceMigrationService,
secretsMigrations.ProvideMigrateToPluginService,
secretsMigrations.ProvideMigrateFromPluginService,
secretsMigrations.ProvideSecretMigrationService,
wire.Bind(new(secretsMigrations.SecretMigrationService), new(*secretsMigrations.SecretMigrationServiceImpl)),
secretsMigrations.ProvideSecretMigrationProvider,
wire.Bind(new(secretsMigrations.SecretMigrationProvider), new(*secretsMigrations.SecretMigrationProviderImpl)),
userauthimpl.ProvideService,
acimpl.ProvideAccessControl,
wire.Bind(new(accesscontrol.AccessControl), new(*acimpl.AccessControl)),

View File

@ -7,6 +7,7 @@ import (
"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/setting"
)
@ -19,20 +20,25 @@ type SecretMigrationService interface {
Migrate(ctx context.Context) error
}
type SecretMigrationServiceImpl struct {
type SecretMigrationProvider interface {
registry.BackgroundService
TriggerPluginMigration(ctx context.Context, toPlugin bool) error
}
type SecretMigrationProviderImpl struct {
services []SecretMigrationService
ServerLockService *serverlock.ServerLockService
migrateToPluginService *MigrateToPluginService
migrateFromPluginService *MigrateFromPluginService
}
func ProvideSecretMigrationService(
func ProvideSecretMigrationProvider(
cfg *setting.Cfg,
serverLockService *serverlock.ServerLockService,
dataSourceSecretMigrationService *DataSourceSecretMigrationService,
migrateToPluginService *MigrateToPluginService,
migrateFromPluginService *MigrateFromPluginService,
) *SecretMigrationServiceImpl {
) *SecretMigrationProviderImpl {
services := make([]SecretMigrationService, 0)
services = append(services, dataSourceSecretMigrationService)
// Plugin migration should always be last; should either migrate to or from, not both
@ -45,7 +51,7 @@ func ProvideSecretMigrationService(
services = append(services, migrateToPluginService)
}
return &SecretMigrationServiceImpl{
return &SecretMigrationProviderImpl{
ServerLockService: serverLockService,
services: services,
migrateToPluginService: migrateToPluginService,
@ -53,9 +59,13 @@ func ProvideSecretMigrationService(
}
}
func (s *SecretMigrationProviderImpl) Run(ctx context.Context) error {
return s.Migrate(ctx)
}
// Migrate Run migration services. This will block until all services have exited.
// This should only be called once at startup
func (s *SecretMigrationServiceImpl) Migrate(ctx context.Context) error {
func (s *SecretMigrationProviderImpl) Migrate(ctx context.Context) error {
// Start migration services.
return s.ServerLockService.LockExecuteAndRelease(ctx, actionName, time.Minute*10, func(context.Context) {
for _, service := range s.services {
@ -71,7 +81,7 @@ func (s *SecretMigrationServiceImpl) Migrate(ctx context.Context) error {
}
// TriggerPluginMigration Kick off a migration to or from the plugin. This will block until all services have exited.
func (s *SecretMigrationServiceImpl) TriggerPluginMigration(ctx context.Context, toPlugin bool) error {
func (s *SecretMigrationProviderImpl) TriggerPluginMigration(ctx context.Context, toPlugin bool) error {
// Don't migrate if there is already one happening
return s.ServerLockService.LockExecuteAndRelease(ctx, actionName, time.Minute*10, func(context.Context) {
var err error