Secrets: Improve unified secrets migration and implement compatibility flag (#50463)

* Implement disableSecretsCompatibility flag

* Allow secret deletion right after migration

* Use dialect.Quote for secure_json_data on secret deletion

Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com>

* Set secure_json_data to NULL instead of empty json

* Run toggles_gen_test and use generated flag variable

* Add ID to delete data source secrets command on function call

Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com>

* Remove extra query to get datasource on secret deletion

* Fix linting issues with CHANGELOG.md

* Use empty json string when deleting secure json data

* Implement secret migration as a background process

* Refactor secret migration as a background service

* Refactor migration to be inside secret store

* Re-add secret deletion function removed on merge

* Try using transaction to fix db lock during tests

* Disable migration for pipeline debugging

* Try adding sleep to fix database lock

* Remove unecessary time sleep from migration

* Fix merge issue, replace models with datasources

* Try event listener approach

* Fix merge issue, replace models with datasources

* Fix linting issues with unchecked error

* Remove unecessary trainling new line

* Increase wait interval on background secret migration

* Rename secret store migration folder for consistency

* Convert background migration to blocking

* Fix number of arguments on server tests

* Check error value of secret migration provider

* Fix linting issue with method varaible

* Revert unintended change on background services

* Move secret migration service provider to wire.go

* Remove unecessary else from datasource service

* Move transaction inside loop on secret migration

* Remove unecessary GetServices function

* Remove unecessary interface after method removal

* Rename Run to Migrate on secret migration interface

* Rename secret migrations service variable on server

* Use MustBool on datasource secret migration

* Revert changes to GetDataSources

* Implement GetAllDataSources function

* Remove DeleteDataSourceSecrets function

* Move datasource secret migration to datasource service

* Remove unecessary properties from datasource secret migration

* Make DecryptLegacySecrets a private method

* Remove context canceled check on secret migrator

* Log error when fail to unmarshal datasource secret

* Add necessary fields to update command on migration

* Handle high availability on secret migration

* Use kvstore for datasource secret migration status

* Add error check for migration status set on kvstore

* Remove NewSecretMigrationService from server tests

* Use const for strings on datasource secrets migration

* Test all cases for datasources secret migrations

Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com>
This commit is contained in:
Guilherme Caulada
2022-07-12 17:27:37 -03:00
committed by GitHub
co-authored by Sofia Papagiannaki
parent a6b1090879
commit 2d8a91a846
15 changed files with 608 additions and 55 deletions
+27 -18
View File
@@ -24,6 +24,7 @@ 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/setting"
"golang.org/x/sync/errgroup"
@@ -43,9 +44,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,
) (*Server, error) {
statsCollectorService.RegisterProviders(usageStatsProvidersRegistry.GetServices())
s, err := newServer(opts, cfg, httpServer, roleRegistry, provisioningService, backgroundServiceProvider)
s, err := newServer(opts, cfg, httpServer, roleRegistry, provisioningService, backgroundServiceProvider, secretMigrationService)
if err != nil {
return nil, err
}
@@ -59,25 +61,27 @@ 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,
) (*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(),
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,
}
return s, nil
@@ -101,9 +105,10 @@ type Server struct {
buildBranch string
backgroundServices []registry.BackgroundService
HTTPServer *api.HTTPServer
roleRegistry accesscontrol.RoleRegistry
provisioningService provisioning.ProvisioningService
HTTPServer *api.HTTPServer
roleRegistry accesscontrol.RoleRegistry
provisioningService provisioning.ProvisioningService
secretMigrationService secretsMigrations.SecretMigrationService
}
// init initializes the server and its services.
@@ -128,6 +133,10 @@ func (s *Server) init() error {
return err
}
if err := s.secretMigrationService.Migrate(s.context); err != nil {
return err
}
return s.provisioningService.RunInitProvisioners(s.context)
}
+8 -1
View File
@@ -7,9 +7,12 @@ 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/ossaccesscontrol"
"github.com/grafana/grafana/pkg/services/secrets/kvstore/migrations"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/require"
)
@@ -47,7 +50,11 @@ func (s *testService) IsDisabled() bool {
func testServer(t *testing.T, services ...registry.BackgroundService) *Server {
t.Helper()
s, err := newServer(Options{}, setting.NewCfg(), nil, &ossaccesscontrol.OSSAccessControlService{}, nil, backgroundsvcs.NewBackgroundServiceRegistry(services...))
serverLockService := serverlock.ProvideService(sqlstore.InitTestDB(t))
secretMigrationService := &migrations.SecretMigrationServiceImpl{
ServerLockService: serverLockService,
}
s, err := newServer(Options{}, setting.NewCfg(), nil, &ossaccesscontrol.OSSAccessControlService{}, nil, backgroundsvcs.NewBackgroundServiceRegistry(services...), secretMigrationService)
require.NoError(t, err)
// Required to skip configuration initialization that causes
// DI errors in this test.
+4
View File
@@ -91,6 +91,7 @@ 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"
@@ -293,6 +294,9 @@ var wireBasicSet = wire.NewSet(
publicdashboardsApi.ProvideApi,
userimpl.ProvideService,
orgimpl.ProvideService,
datasourceservice.ProvideDataSourceMigrationService,
secretsMigrations.ProvideSecretMigrationService,
wire.Bind(new(secretsMigrations.SecretMigrationService), new(*secretsMigrations.SecretMigrationServiceImpl)),
)
var wireSet = wire.NewSet(