diff --git a/pkg/registry/apis/secret/secretkeeper/secretkeeper.go b/pkg/registry/apis/secret/secretkeeper/secretkeeper.go index dc51d5b3160..25241f5c15b 100644 --- a/pkg/registry/apis/secret/secretkeeper/secretkeeper.go +++ b/pkg/registry/apis/secret/secretkeeper/secretkeeper.go @@ -9,6 +9,7 @@ import ( "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/secretkeeper/sqlkeeper" + "github.com/grafana/grafana/pkg/setting" "github.com/prometheus/client_golang/prometheus" ) @@ -25,9 +26,10 @@ func ProvideService( encryptionManager contracts.EncryptionManager, migrationExecutor contracts.EncryptedValueMigrationExecutor, 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 ) (*OSSKeeperService, error) { - systemKeeper, err := sqlkeeper.NewSQLKeeper(tracer, encryptionManager, store, migrationExecutor, reg) + systemKeeper, err := sqlkeeper.NewSQLKeeper(tracer, encryptionManager, store, migrationExecutor, reg, cfg) if err != nil { return nil, fmt.Errorf("failed to create system keeper: %w", err) } diff --git a/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go b/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go index 43193befcea..f60dcc91188 100644 --- a/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go +++ b/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go @@ -66,7 +66,7 @@ func setupTestService(t *testing.T, cfg *setting.Cfg) (*OSSKeeperService, error) require.NoError(t, err) // 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) return keeperService, err diff --git a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go index a6f54d5674a..4117fac25e8 100644 --- a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go +++ b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go @@ -10,6 +10,7 @@ import ( "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/xkube" + "github.com/grafana/grafana/pkg/setting" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -30,16 +31,21 @@ func NewSQLKeeper( store contracts.EncryptedValueStorage, migrationExecutor contracts.EncryptedValueMigrationExecutor, reg prometheus.Registerer, + cfg *setting.Cfg, ) (*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. - // Periodically assess whether it is safe to remove - most likely for G13 should be fine. - log := logging.FromContext(context.Background()) - log.Debug("sqlkeeper: executing encrypted value store migration") - 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) + + // Only run the migration if running as an MT api server + if cfg.StackID == "" { + // 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. + // Periodically assess whether it is safe to remove - most likely for G13 should be fine. + log := logging.FromContext(context.Background()) + log.Debug("sqlkeeper: executing encrypted value store migration") + 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{ diff --git a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go index db5139c50ff..52a2d1484bb 100644 --- a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go +++ b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go @@ -1,6 +1,7 @@ package sqlkeeper_test import ( + "context" "testing" "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) 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 } diff --git a/pkg/registry/apis/secret/testutils/testutils.go b/pkg/registry/apis/secret/testutils/testutils.go index 2d1c94c2ea3..12495f92e01 100644 --- a/pkg/registry/apis/secret/testutils/testutils.go +++ b/pkg/registry/apis/secret/testutils/testutils.go @@ -40,7 +40,9 @@ import ( ) type SetupConfig struct { - KeeperService contracts.KeeperService + KeeperService contracts.KeeperService + StackID string + DataKeyMigrationExecutor contracts.EncryptedValueMigrationExecutor } func defaultSetupCfg() SetupConfig { @@ -95,6 +97,7 @@ func Setup(t *testing.T, opts ...func(*SetupConfig)) Sut { GCWorkerMaxBatchSize: 2, GCWorkerMaxConcurrentCleanups: 2, } + cfg.StackID = setupCfg.StackID store, err := encryptionstorage.ProvideDataKeyStorage(database, tracer, nil) require.NoError(t, err) @@ -126,9 +129,12 @@ func Setup(t *testing.T, opts ...func(*SetupConfig)) Sut { globalEncryptedValueStorage, err := encryptionstorage.ProvideGlobalEncryptedValueStorage(database, tracer) require.NoError(t, err) - // Initialize a noop migration executor for the sql keeper so it doesn't interfere with initialization - noopMigrationExecutor := &NoopMigrationExecutor{} - sqlKeeper, err := sqlkeeper.NewSQLKeeper(tracer, encryptionManager, encryptedValueStorage, noopMigrationExecutor, nil) + // Initialize a noop migration executor for the sql keeper so it doesn't interfere with initialization, or use the one provided + var fakeMigrationExecutor contracts.EncryptedValueMigrationExecutor = setupCfg.DataKeyMigrationExecutor + if fakeMigrationExecutor == nil { + fakeMigrationExecutor = &NoopMigrationExecutor{} + } + sqlKeeper, err := sqlkeeper.NewSQLKeeper(tracer, encryptionManager, encryptedValueStorage, fakeMigrationExecutor, nil, cfg) require.NoError(t, err) // Initialize a real migration executor for test diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index 06a246c2e1b..60473437d96 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -494,7 +494,7 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api if err != nil { 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 { return nil, err } @@ -1104,7 +1104,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac if err != nil { 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 { return nil, err }