only run the migration as an MT api server

This commit is contained in:
Michael Mandrus 2025-10-06 15:26:28 -04:00
parent 4346561129
commit a0604517d9
6 changed files with 59 additions and 17 deletions

View File

@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/registry/apis/secret" "github.com/grafana/grafana/pkg/registry/apis/secret"
"github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts"
"github.com/grafana/grafana/pkg/registry/apis/secret/secretkeeper/sqlkeeper" "github.com/grafana/grafana/pkg/registry/apis/secret/secretkeeper/sqlkeeper"
"github.com/grafana/grafana/pkg/setting"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
) )
@ -25,9 +26,10 @@ func ProvideService(
encryptionManager contracts.EncryptionManager, encryptionManager contracts.EncryptionManager,
migrationExecutor contracts.EncryptedValueMigrationExecutor, migrationExecutor contracts.EncryptedValueMigrationExecutor,
reg prometheus.Registerer, reg prometheus.Registerer,
cfg *setting.Cfg,
_ *secret.DependencyRegisterer, // noop import so wire runs DB migrations before instantiating this service -- can be nil when manually instantiating _ *secret.DependencyRegisterer, // noop import so wire runs DB migrations before instantiating this service -- can be nil when manually instantiating
) (*OSSKeeperService, error) { ) (*OSSKeeperService, error) {
systemKeeper, err := sqlkeeper.NewSQLKeeper(tracer, encryptionManager, store, migrationExecutor, reg) systemKeeper, err := sqlkeeper.NewSQLKeeper(tracer, encryptionManager, store, migrationExecutor, reg, cfg)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to create system keeper: %w", err) return nil, fmt.Errorf("failed to create system keeper: %w", err)
} }

View File

@ -66,7 +66,7 @@ func setupTestService(t *testing.T, cfg *setting.Cfg) (*OSSKeeperService, error)
require.NoError(t, err) require.NoError(t, err)
// Initialize the keeper service // Initialize the keeper service
keeperService, err := ProvideService(tracer, encValueStore, encryptionManager, &testutils.NoopMigrationExecutor{}, nil, nil) keeperService, err := ProvideService(tracer, encValueStore, encryptionManager, &testutils.NoopMigrationExecutor{}, nil, cfg, nil)
require.NoError(t, err) require.NoError(t, err)
return keeperService, err return keeperService, err

View File

@ -10,6 +10,7 @@ import (
"github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts"
"github.com/grafana/grafana/pkg/registry/apis/secret/secretkeeper/metrics" "github.com/grafana/grafana/pkg/registry/apis/secret/secretkeeper/metrics"
"github.com/grafana/grafana/pkg/registry/apis/secret/xkube" "github.com/grafana/grafana/pkg/registry/apis/secret/xkube"
"github.com/grafana/grafana/pkg/setting"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace" "go.opentelemetry.io/otel/trace"
@ -30,16 +31,21 @@ func NewSQLKeeper(
store contracts.EncryptedValueStorage, store contracts.EncryptedValueStorage,
migrationExecutor contracts.EncryptedValueMigrationExecutor, migrationExecutor contracts.EncryptedValueMigrationExecutor,
reg prometheus.Registerer, reg prometheus.Registerer,
cfg *setting.Cfg,
) (*SQLKeeper, error) { ) (*SQLKeeper, error) {
// Run the encrypted value store migration before anything else, otherwise operations may fail
// TODO: This does not need to be here forever, but we may currently have on-prem deployments using GSM, so it needs to be here for now. // Only run the migration if running as an MT api server
// Periodically assess whether it is safe to remove - most likely for G13 should be fine. if cfg.StackID == "" {
log := logging.FromContext(context.Background()) // Run the encrypted value store migration before anything else, otherwise operations may fail
log.Debug("sqlkeeper: executing encrypted value store migration") // TODO: This does not need to be here forever, but we may currently have on-prem deployments using GSM, so it needs to be here for now.
rowsAffected, err := migrationExecutor.Execute(context.Background()) // Periodically assess whether it is safe to remove - most likely for G13 should be fine.
log.Debug("sqlkeeper: encrypted value store migration completed", "rows_affected", rowsAffected) log := logging.FromContext(context.Background())
if err != nil { log.Debug("sqlkeeper: executing encrypted value store migration")
return nil, fmt.Errorf("error encountered during encrypted value store migration: %w", err) rowsAffected, err := migrationExecutor.Execute(context.Background())
log.Debug("sqlkeeper: encrypted value store migration completed", "rows_affected", rowsAffected)
if err != nil {
return nil, fmt.Errorf("error encountered during encrypted value store migration: %w", err)
}
} }
return &SQLKeeper{ return &SQLKeeper{

View File

@ -1,6 +1,7 @@
package sqlkeeper_test package sqlkeeper_test
import ( import (
"context"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@ -146,4 +147,31 @@ func Test_SQLKeeperSetup(t *testing.T) {
err = sut.SQLKeeper.Update(t.Context(), nil, namespace1, "non_existing_name", version1, plaintext2) err = sut.SQLKeeper.Update(t.Context(), nil, namespace1, "non_existing_name", version1, plaintext2)
require.Error(t, err) require.Error(t, err)
}) })
t.Run("data key migration only runs if the stack ID is not set", func(t *testing.T) {
t.Parallel()
m := &mockMigrationExecutor{}
testutils.Setup(t, testutils.WithMutateCfg(func(cfg *testutils.SetupConfig) {
cfg.StackID = "test-123"
cfg.DataKeyMigrationExecutor = m
}))
assert.False(t, m.wasExecuted)
testutils.Setup(t, testutils.WithMutateCfg(func(cfg *testutils.SetupConfig) {
cfg.StackID = ""
cfg.DataKeyMigrationExecutor = m
}))
assert.True(t, m.wasExecuted)
})
}
type mockMigrationExecutor struct {
wasExecuted bool
}
func (m *mockMigrationExecutor) Execute(ctx context.Context) (int, error) {
m.wasExecuted = true
return 0, nil
} }

View File

@ -40,7 +40,9 @@ import (
) )
type SetupConfig struct { type SetupConfig struct {
KeeperService contracts.KeeperService KeeperService contracts.KeeperService
StackID string
DataKeyMigrationExecutor contracts.EncryptedValueMigrationExecutor
} }
func defaultSetupCfg() SetupConfig { func defaultSetupCfg() SetupConfig {
@ -95,6 +97,7 @@ func Setup(t *testing.T, opts ...func(*SetupConfig)) Sut {
GCWorkerMaxBatchSize: 2, GCWorkerMaxBatchSize: 2,
GCWorkerMaxConcurrentCleanups: 2, GCWorkerMaxConcurrentCleanups: 2,
} }
cfg.StackID = setupCfg.StackID
store, err := encryptionstorage.ProvideDataKeyStorage(database, tracer, nil) store, err := encryptionstorage.ProvideDataKeyStorage(database, tracer, nil)
require.NoError(t, err) require.NoError(t, err)
@ -126,9 +129,12 @@ func Setup(t *testing.T, opts ...func(*SetupConfig)) Sut {
globalEncryptedValueStorage, err := encryptionstorage.ProvideGlobalEncryptedValueStorage(database, tracer) globalEncryptedValueStorage, err := encryptionstorage.ProvideGlobalEncryptedValueStorage(database, tracer)
require.NoError(t, err) require.NoError(t, err)
// Initialize a noop migration executor for the sql keeper so it doesn't interfere with initialization // Initialize a noop migration executor for the sql keeper so it doesn't interfere with initialization, or use the one provided
noopMigrationExecutor := &NoopMigrationExecutor{} var fakeMigrationExecutor contracts.EncryptedValueMigrationExecutor = setupCfg.DataKeyMigrationExecutor
sqlKeeper, err := sqlkeeper.NewSQLKeeper(tracer, encryptionManager, encryptedValueStorage, noopMigrationExecutor, nil) if fakeMigrationExecutor == nil {
fakeMigrationExecutor = &NoopMigrationExecutor{}
}
sqlKeeper, err := sqlkeeper.NewSQLKeeper(tracer, encryptionManager, encryptedValueStorage, fakeMigrationExecutor, nil, cfg)
require.NoError(t, err) require.NoError(t, err)
// Initialize a real migration executor for test // Initialize a real migration executor for test

View File

@ -494,7 +494,7 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api
if err != nil { if err != nil {
return nil, err return nil, err
} }
ossKeeperService, err := secretkeeper.ProvideService(tracer, encryptedValueStorage, encryptionManager, encryptedValueMigrationExecutor, registerer, dependencyRegisterer) ossKeeperService, err := secretkeeper.ProvideService(tracer, encryptedValueStorage, encryptionManager, encryptedValueMigrationExecutor, registerer, cfg, dependencyRegisterer)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -1104,7 +1104,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac
if err != nil { if err != nil {
return nil, err return nil, err
} }
ossKeeperService, err := secretkeeper.ProvideService(tracer, encryptedValueStorage, encryptionManager, encryptedValueMigrationExecutor, registerer, dependencyRegisterer) ossKeeperService, err := secretkeeper.ProvideService(tracer, encryptedValueStorage, encryptionManager, encryptedValueMigrationExecutor, registerer, cfg, dependencyRegisterer)
if err != nil { if err != nil {
return nil, err return nil, err
} }