Unified Storage: Fix Create, Update and Delete wrt Resource Versions (#88183)

* add sqltemplate utilities, improve tests and documentation * bunch of things * remove unnecessary message * add queries * add queries * add queries * add folders support * fix diff * fix linters * fix diff * fix linters * fix linters * fix typo * fix linters * fix linters * fix linters * several fixes * several fixes * temporarily disable k8s integration tests for Entity Server * postpone some tests * postpone documentation changes * Fix bug in create * improve error reporting * fix PostgeSQL parameters * fix MySQL sqlmode * fix MySQL-5.7 * reduce but document the number of database connection options * remove unused code and improve docs
2024-06-05 14:23:32 -03:00
parent cbe7521a56
commit 6fcd7d9e03
43 changed files with 1867 additions and 1298 deletions
@@ -1,75 +1,105 @@
 package dbimpl

 import (
+	"cmp"
 	"fmt"
 	"strings"
 	"time"

-	"github.com/grafana/grafana/pkg/infra/tracing"
-	"github.com/grafana/grafana/pkg/services/sqlstore"
-	"github.com/grafana/grafana/pkg/setting"
-	"github.com/grafana/grafana/pkg/util"
+	"github.com/go-sql-driver/mysql"
 	"xorm.io/xorm"
+
+	"github.com/grafana/grafana/pkg/infra/tracing"
+	"github.com/grafana/grafana/pkg/services/store/entity/db"
 )

-func getEngineMySQL(cfgSection *setting.DynamicSection, tracer tracing.Tracer) (*xorm.Engine, error) {
-	dbHost := cfgSection.Key("db_host").MustString("")
-	dbName := cfgSection.Key("db_name").MustString("")
-	dbUser := cfgSection.Key("db_user").MustString("")
-	dbPass := cfgSection.Key("db_pass").MustString("")
+func getEngineMySQL(getter *sectionGetter, _ tracing.Tracer) (*xorm.Engine, error) {
+	config := mysql.NewConfig()
+	config.User = getter.String("db_user")
+	config.Passwd = getter.String("db_pass")
+	config.Net = "tcp"
+	config.Addr = getter.String("db_host")
+	config.DBName = getter.String("db_name")
+	config.Params = map[string]string{
+		// See: https://dev.mysql.com/doc/refman/en/sql-mode.html
+		"@@SESSION.sql_mode": "ANSI",
+	}
+	config.Collation = "utf8mb4_unicode_ci"
+	config.Loc = time.UTC
+	config.AllowNativePasswords = true
+	config.ClientFoundRows = true

-	// TODO: support all mysql connection options
-	protocol := "tcp"
-	if strings.HasPrefix(dbHost, "/") {
-		protocol = "unix"
+	// TODO: do we want to support these?
+	//	config.ServerPubKey = getter.String("db_server_pub_key")
+	//	config.TLSConfig = getter.String("db_tls_config_name")
+
+	if err := getter.Err(); err != nil {
+		return nil, fmt.Errorf("config error: %w", err)
 	}

-	connectionString := connectionStringMySQL(dbUser, dbPass, protocol, dbHost, dbName)
+	if strings.HasPrefix(config.Addr, "/") {
+		config.Net = "unix"
+	}

-	driverName := sqlstore.WrapDatabaseDriverWithHooks("mysql", tracer)
-	engine, err := xorm.NewEngine(driverName, connectionString)
+	// FIXME: get rid of xorm
+	engine, err := xorm.NewEngine(db.DriverMySQL, config.FormatDSN())
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("open database: %w", err)
 	}

 	engine.SetMaxOpenConns(0)
 	engine.SetMaxIdleConns(2)
-	engine.SetConnMaxLifetime(time.Second * time.Duration(14400))
+	engine.SetConnMaxLifetime(4 * time.Hour)

 	return engine, nil
 }

-func getEnginePostgres(cfgSection *setting.DynamicSection, tracer tracing.Tracer) (*xorm.Engine, error) {
-	dbHost := cfgSection.Key("db_host").MustString("")
-	dbName := cfgSection.Key("db_name").MustString("")
-	dbUser := cfgSection.Key("db_user").MustString("")
-	dbPass := cfgSection.Key("db_pass").MustString("")
-
-	// TODO: support all postgres connection options
-	dbSslMode := cfgSection.Key("db_sslmode").MustString("disable")
-
-	addr, err := util.SplitHostPortDefault(dbHost, "127.0.0.1", "5432")
-	if err != nil {
-		return nil, fmt.Errorf("invalid host specifier '%s': %w", dbHost, err)
+func getEnginePostgres(getter *sectionGetter, _ tracing.Tracer) (*xorm.Engine, error) {
+	dsnKV := map[string]string{
+		"user":     getter.String("db_user"),
+		"password": getter.String("db_pass"),
+		"dbname":   getter.String("db_name"),
+		"sslmode":  cmp.Or(getter.String("db_sslmode"), "disable"),
 	}

-	connectionString := connectionStringPostgres(dbUser, dbPass, addr.Host, addr.Port, dbName, dbSslMode)
+	// TODO: probably interesting:
+	//	"passfile", "statement_timeout", "lock_timeout", "connect_timeout"

-	driverName := sqlstore.WrapDatabaseDriverWithHooks("postgres", tracer)
-	engine, err := xorm.NewEngine(driverName, connectionString)
-	if err != nil {
-		return nil, err
+	// TODO: for CockroachDB, we probably need to use the following:
+	//	dsnKV["options"] = "-c enable_experimental_alter_column_type_general=true"
+	// Or otherwise specify it as:
+	//	dsnKV["enable_experimental_alter_column_type_general"] = "true"
+
+	// TODO: do we want to support these options in the DSN as well?
+	//	"sslkey", "sslcert", "sslrootcert", "sslpassword", "sslsni", "krbspn",
+	//	"krbsrvname", "target_session_attrs", "service", "servicefile"
+
+	// More on Postgres connection string parameters:
+	//	https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING
+
+	hostport := getter.String("db_host")
+
+	if err := getter.Err(); err != nil {
+		return nil, fmt.Errorf("config error: %w", err)
 	}
+
+	host, port, err := splitHostPortDefault(hostport, "127.0.0.1", "5432")
+	if err != nil {
+		return nil, fmt.Errorf("invalid db_host: %w", err)
+	}
+	dsnKV["host"] = host
+	dsnKV["port"] = port
+
+	dsn, err := MakeDSN(dsnKV)
+	if err != nil {
+		return nil, fmt.Errorf("error building DSN: %w", err)
+	}
+
+	// FIXME: get rid of xorm
+	engine, err := xorm.NewEngine(db.DriverPostgres, dsn)
+	if err != nil {
+		return nil, fmt.Errorf("open database: %w", err)
+	}
+
 	return engine, nil
 }
-
-func connectionStringMySQL(user, password, protocol, host, dbName string) string {
-	return fmt.Sprintf("%s:%s@%s(%s)/%s?collation=utf8mb4_unicode_ci&allowNativePasswords=true&clientFoundRows=true", user, password, protocol, host, dbName)
-}
-
-func connectionStringPostgres(user, password, host, port, dbName, sslMode string) string {
-	return fmt.Sprintf(
-		"user=%s password=%s host=%s port=%s dbname=%s sslmode=%s", // sslcert='%s' sslkey='%s' sslrootcert='%s'",
-		user, password, host, port, dbName, sslMode, // ss.dbCfg.ClientCertPath, ss.dbCfg.ClientKeyPath, ss.dbCfg.CaCertPath
-	)
-}
@@ -19,7 +19,10 @@ func TestGetEnginePostgresFromConfig(t *testing.T) {
 	s.Key("db_user").SetValue("user")
 	s.Key("db_password").SetValue("password")

-	engine, err := getEnginePostgres(cfg.SectionWithEnvOverrides("entity_api"), nil)
+	getter := &sectionGetter{
+		DynamicSection: cfg.SectionWithEnvOverrides("entity_api"),
+	}
+	engine, err := getEnginePostgres(getter, nil)

 	assert.NotNil(t, engine)
 	assert.NoError(t, err)
@@ -36,19 +39,11 @@ func TestGetEngineMySQLFromConfig(t *testing.T) {
 	s.Key("db_user").SetValue("user")
 	s.Key("db_password").SetValue("password")

-	engine, err := getEngineMySQL(cfg.SectionWithEnvOverrides("entity_api"), nil)
+	getter := &sectionGetter{
+		DynamicSection: cfg.SectionWithEnvOverrides("entity_api"),
+	}
+	engine, err := getEngineMySQL(getter, nil)

 	assert.NotNil(t, engine)
 	assert.NoError(t, err)
 }
-
-func TestGetConnectionStrings(t *testing.T) {
-	t.Run("generate mysql connection string", func(t *testing.T) {
-		expected := "user:password@tcp(localhost)/grafana?collation=utf8mb4_unicode_ci&allowNativePasswords=true&clientFoundRows=true"
-		assert.Equal(t, expected, connectionStringMySQL("user", "password", "tcp", "localhost", "grafana"))
-	})
-	t.Run("generate postgres connection string", func(t *testing.T) {
-		expected := "user=user password=password host=localhost port=5432 dbname=grafana sslmode=disable"
-		assert.Equal(t, expected, connectionStringPostgres("user", "password", "localhost", "5432", "grafana", "disable"))
-	})
-}
@@ -2,6 +2,7 @@ package dbimpl

 import (
 	"fmt"
+	"sync"

 	"github.com/dlmiddlecote/sqlstats"
 	"github.com/jmoiron/sqlx"
@@ -31,6 +32,9 @@ func ProvideEntityDB(db db.DB, cfg *setting.Cfg, features featuremgmt.FeatureTog
 }

 type EntityDB struct {
+	once    sync.Once
+	onceErr error
+
 	db       db.DB
 	features featuremgmt.FeatureToggles
 	engine   *xorm.Engine
@@ -45,42 +49,57 @@ func (db *EntityDB) Init() error {
 }

 func (db *EntityDB) GetEngine() (*xorm.Engine, error) {
+	db.once.Do(func() {
+		db.onceErr = db.init()
+	})
+
+	return db.engine, db.onceErr
+}
+
+func (db *EntityDB) init() error {
 	if db.engine != nil {
-		return db.engine, nil
+		return nil
 	}

 	var engine *xorm.Engine
 	var err error

-	cfgSection := db.cfg.SectionWithEnvOverrides("entity_api")
-	dbType := cfgSection.Key("db_type").MustString("")
+	getter := &sectionGetter{
+		DynamicSection: db.cfg.SectionWithEnvOverrides("entity_api"),
+	}
+
+	dbType := getter.Key("db_type").MustString("")

 	// if explicit connection settings are provided, use them
 	if dbType != "" {
 		if dbType == "postgres" {
-			engine, err = getEnginePostgres(cfgSection, db.tracer)
+			engine, err = getEnginePostgres(getter, db.tracer)
 			if err != nil {
-				return nil, err
+				return err
 			}

 			// FIXME: this config option is cockroachdb-specific, it's not supported by postgres
+			// FIXME: this only sets this option for the session that we get
+			// from the pool right now. A *sql.DB is a pool of connections,
+			// there is no guarantee that the session where this is run will be
+			// the same where we need to change the type of a column
 			_, err = engine.Exec("SET SESSION enable_experimental_alter_column_type_general=true")
 			if err != nil {
 				db.log.Error("error connecting to postgres", "msg", err.Error())
 				// FIXME: return nil, err
 			}
 		} else if dbType == "mysql" {
-			engine, err = getEngineMySQL(cfgSection, db.tracer)
+			engine, err = getEngineMySQL(getter, db.tracer)
 			if err != nil {
-				return nil, err
+				return err
 			}
-			_, err = engine.Exec("SELECT 1")
-			if err != nil {
-				return nil, err
+
+			if err = engine.Ping(); err != nil {
+				return err
 			}
 		} else {
 			// TODO: sqlite support
-			return nil, fmt.Errorf("invalid db type specified: %s", dbType)
+			return fmt.Errorf("invalid db type specified: %s", dbType)
 		}

 		// register sql stat metrics
@@ -89,7 +108,7 @@ func (db *EntityDB) GetEngine() (*xorm.Engine, error) {
 		}

 		// configure sql logging
-		debugSQL := cfgSection.Key("log_queries").MustBool(false)
+		debugSQL := getter.Key("log_queries").MustBool(false)
 		if !debugSQL {
 			engine.SetLogger(&xorm.DiscardLogger{})
 		} else {
@@ -98,10 +117,11 @@ func (db *EntityDB) GetEngine() (*xorm.Engine, error) {
 			engine.ShowSQL(true)
 			engine.ShowExecTime(true)
 		}
+
 		// otherwise, try to use the grafana db connection
 	} else {
 		if db.db == nil {
-			return nil, fmt.Errorf("no db connection provided")
+			return fmt.Errorf("no db connection provided")
 		}

 		engine = db.db.GetEngine()
@@ -109,12 +129,12 @@ func (db *EntityDB) GetEngine() (*xorm.Engine, error) {

 	db.engine = engine

-	if err := migrations.MigrateEntityStore(db, db.features); err != nil {
+	if err := migrations.MigrateEntityStore(engine, db.cfg, db.features); err != nil {
 		db.engine = nil
-		return nil, err
+		return fmt.Errorf("run migrations: %w", err)
 	}

-	return db.engine, nil
+	return nil
 }

 func (db *EntityDB) GetSession() (*session.SessionDB, error) {
@@ -0,0 +1,99 @@
+package dbimpl
+
+import (
+	"cmp"
+	"errors"
+	"fmt"
+	"net"
+	"strings"
+	"unicode/utf8"
+
+	"github.com/grafana/grafana/pkg/setting"
+)
+
+var (
+	ErrInvalidUTF8Sequence = errors.New("invalid UTF-8 sequence")
+)
+
+type sectionGetter struct {
+	*setting.DynamicSection
+	err error
+}
+
+func (g *sectionGetter) Err() error {
+	return g.err
+}
+
+func (g *sectionGetter) String(key string) string {
+	if g.err != nil {
+		return ""
+	}
+	v := g.DynamicSection.Key(key).MustString("")
+	if !utf8.ValidString(v) {
+		g.err = fmt.Errorf("value for key %q: %w", key, ErrInvalidUTF8Sequence)
+
+		return ""
+	}
+
+	return v
+}
+
+// MakeDSN creates a DSN from the given key/value pair. It validates the strings
+// form valid UTF-8 sequences and escapes values if needed.
+func MakeDSN(m map[string]string) (string, error) {
+	b := new(strings.Builder)
+
+	for k, v := range m {
+		if !utf8.ValidString(v) {
+			return "", fmt.Errorf("value for DSN key %q: %w", k,
+				ErrInvalidUTF8Sequence)
+		}
+		if v == "" {
+			continue
+		}
+
+		if b.Len() > 0 {
+			_ = b.WriteByte(' ')
+		}
+		_, _ = b.WriteString(k)
+		_ = b.WriteByte('=')
+		writeDSNValue(b, v)
+	}
+
+	return b.String(), nil
+}
+
+func writeDSNValue(b *strings.Builder, v string) {
+	numq := strings.Count(v, `'`)
+	numb := strings.Count(v, `\`)
+	if numq+numb == 0 && v != "" {
+		b.WriteString(v)
+
+		return
+	}
+	b.Grow(2 + numq + numb + len(v))
+
+	_ = b.WriteByte('\'')
+	for _, r := range v {
+		if r == '\\' || r == '\'' {
+			_ = b.WriteByte('\\')
+		}
+		_, _ = b.WriteRune(r)
+	}
+	_ = b.WriteByte('\'')
+}
+
+func splitHostPortDefault(hostport, defaultHost, defaultPort string) (string, string, error) {
+	host, port, err := net.SplitHostPort(hostport)
+	if err != nil {
+		// try appending the port
+		host, port, err = net.SplitHostPort(hostport + ":" + defaultPort)
+		if err != nil {
+			return "", "", fmt.Errorf("invalid hostport: %q", hostport)
+		}
+	}
+	host = cmp.Or(host, defaultHost)
+	port = cmp.Or(port, defaultPort)
+
+	return host, port, nil
+}