Chore (sqlstore): add validation and testing for repl config (#90683)

* add some validation and testing for repl cf
* connection strings are secrets

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
+}

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`

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))
 	}

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))
+	t.Run("valid config", func(t *testing.T) {
-	require.NoError(t, err)
+		inicfg, err := ini.Load([]byte(testReplCfg))
-	cfg, err := setting.NewCfgFromINIFile(inicfg)
+		require.NoError(t, err)
-	require.NoError(t, err)
+		cfg, err := setting.NewCfgFromINIFile(inicfg)
+		require.NoError(t, err)
 
-	dbCfgs, err := NewRODatabaseConfigs(cfg, nil)
+		dbCfgs, err := NewRODatabaseConfigs(cfg, nil)
-	require.NoError(t, err)
+		require.NoError(t, err)
 
-	var connStr = func(port int) string {
+		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)
+			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))
 		}
-	}
+
+		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