diff --git a/pkg/services/sqlstore/database_config.go b/pkg/services/sqlstore/database_config.go index 9105859ca03..71d37bd603c 100644 --- a/pkg/services/sqlstore/database_config.go +++ b/pkg/services/sqlstore/database_config.go @@ -235,3 +235,33 @@ func buildExtraConnectionString(sep rune, urlQueryParams map[string][]string) st } return sb.String() } + +func validateReplicaConfigs(primary *DatabaseConfig, cfgs []*DatabaseConfig) error { + if cfgs == nil { + return errors.New("cfg cannot be nil") + } + + // Return multiple errors so we can fix them all at once! + var result error + + // Check for duplicate connection strings + seen := make(map[string]struct{}) + seen[primary.ConnectionString] = struct{}{} + for _, cfg := range cfgs { + if _, ok := seen[cfg.ConnectionString]; ok { + result = errors.Join(result, errors.New("duplicate connection string")) + } else { + seen[cfg.ConnectionString] = struct{}{} + } + } + + // Verify that every database is the same type and version, and that it matches the primary database. + for _, cfg := range cfgs { + if cfg.Type != primary.Type { + result = errors.Join(result, fmt.Errorf("the replicas must have the same database type as the primary database (%s != %s)", primary.Type, cfg.Type)) + break // Only need to report this once + } + } + + return result +} diff --git a/pkg/services/sqlstore/database_config_test.go b/pkg/services/sqlstore/database_config_test.go index cd13fc3b8fa..08174d8307e 100644 --- a/pkg/services/sqlstore/database_config_test.go +++ b/pkg/services/sqlstore/database_config_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/ini.v1" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" @@ -221,3 +222,71 @@ func TestBuildConnectionStringPostgres(t *testing.T) { }) } } + +func TestValidateReplicaConfigs(t *testing.T) { + t.Run("valid config", func(t *testing.T) { + inicfg, err := ini.Load([]byte(testReplCfg)) + require.NoError(t, err) + cfg, err := setting.NewCfgFromINIFile(inicfg) + require.NoError(t, err) + + dbCfgs, err := NewRODatabaseConfigs(cfg, nil) + require.NoError(t, err) + + err = validateReplicaConfigs(&DatabaseConfig{Type: "mysql"}, dbCfgs) + require.NoError(t, err) + }) + + t.Run("invalid config: primary database type mismatch", func(t *testing.T) { + // valid repl config, the issue is that the primary has a different type + inicfg, err := ini.Load([]byte(testReplCfg)) + require.NoError(t, err) + cfg, err := setting.NewCfgFromINIFile(inicfg) + require.NoError(t, err) + + dbCfgs, err := NewRODatabaseConfigs(cfg, nil) + require.NoError(t, err) + + err = validateReplicaConfigs(&DatabaseConfig{Type: "postgres"}, dbCfgs) + require.Error(t, err) + + if uw, ok := err.(interface{ Unwrap() []error }); ok { + errs := uw.Unwrap() + require.Equal(t, 1, len(errs)) + } + }) + + t.Run("invalid repl config", func(t *testing.T) { + // Type mismatch + duplicate hosts + inicfg, err := ini.Load([]byte(invalidReplCfg)) + require.NoError(t, err) + cfg, err := setting.NewCfgFromINIFile(inicfg) + require.NoError(t, err) + + dbCfgs, err := NewRODatabaseConfigs(cfg, nil) + require.NoError(t, err) + + err = validateReplicaConfigs(&DatabaseConfig{Type: "mysql"}, dbCfgs) + require.Error(t, err) + + if uw, ok := err.(interface{ Unwrap() []error }); ok { + errs := uw.Unwrap() + require.Equal(t, 2, len(errs)) + } + }) +} + +// This cfg has a duplicate host for repls 0 and 1, and a type mismatch in repl 2 +var invalidReplCfg = ` +[database_replicas] +type = mysql +name = grafana +user = grafana +password = password +host = 127.0.0.1:3306 +[database_replicas.one] = +type = mysql +host = 127.0.0.1:3306 +[database_replicas.two] = +type = postgres +host = 127.0.0.1:3308` diff --git a/pkg/services/sqlstore/replstore.go b/pkg/services/sqlstore/replstore.go index d245b036d7a..ff30b10f60b 100644 --- a/pkg/services/sqlstore/replstore.go +++ b/pkg/services/sqlstore/replstore.go @@ -2,6 +2,7 @@ package sqlstore import ( "errors" + "fmt" "regexp" "sync/atomic" "time" @@ -84,6 +85,10 @@ func ProvideServiceWithReadReplica(primary *SQLStore, cfg *setting.Cfg, return nil, err } + if err := validateReplicaConfigs(primary.dbCfg, replCfgs); err != nil { + return nil, fmt.Errorf("failed to validate replica configurations: %w", err) + } + if len(replCfgs) > 0 { replStore.repls = make([]*SQLStore, len(replCfgs)) } diff --git a/pkg/services/sqlstore/replstore_test.go b/pkg/services/sqlstore/replstore_test.go index b9084804341..8fa50625cdc 100644 --- a/pkg/services/sqlstore/replstore_test.go +++ b/pkg/services/sqlstore/replstore_test.go @@ -32,26 +32,28 @@ func TestReplStore_ReadReplica(t *testing.T) { } func TestNewRODatabaseConfig(t *testing.T) { - inicfg, err := ini.Load([]byte(replCfg)) - require.NoError(t, err) - cfg, err := setting.NewCfgFromINIFile(inicfg) - require.NoError(t, err) + t.Run("valid config", func(t *testing.T) { + inicfg, err := ini.Load([]byte(testReplCfg)) + require.NoError(t, err) + cfg, err := setting.NewCfgFromINIFile(inicfg) + require.NoError(t, err) - dbCfgs, err := NewRODatabaseConfigs(cfg, nil) - require.NoError(t, err) + dbCfgs, err := NewRODatabaseConfigs(cfg, nil) + require.NoError(t, err) - var connStr = func(port int) string { - return fmt.Sprintf("grafana:password@tcp(127.0.0.1:%d)/grafana?collation=utf8mb4_unicode_ci&allowNativePasswords=true&clientFoundRows=true", port) - } - - for i, c := range dbCfgs { - if !cmp.Equal(c.ConnectionString, connStr(i+3306)) { - t.Errorf("wrong result for connection string %d.\nGot: %s,\nWant: %s", i, c.ConnectionString, connStr(i+3306)) + var connStr = func(port int) string { + return fmt.Sprintf("grafana:password@tcp(127.0.0.1:%d)/grafana?collation=utf8mb4_unicode_ci&allowNativePasswords=true&clientFoundRows=true", port) } - } + + for i, c := range dbCfgs { + if !cmp.Equal(c.ConnectionString, connStr(i+3306)) { + t.Errorf("wrong result for connection string %d.\nGot: %s,\nWant: %s", i, c.ConnectionString, connStr(i+3306)) + } + } + }) } -var replCfg = ` +var testReplCfg = ` [database_replicas] type = mysql name = grafana