From 4c074d312becba63ebbab576b2bd5251caef4b3c Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 17 Oct 2026 12:18:01 +0000 Subject: [PATCH] db: disable GORM's prepared statement cache on SQLite database.gorm.prepare_stmt is now ignored, with a startup warning. --- config-example.yaml | 3 -- hscontrol/db/db.go | 4 +- hscontrol/db/db_test.go | 69 ++++++++++++++++++++++++++++++++++ hscontrol/types/config.go | 11 +++--- hscontrol/types/config_test.go | 37 ++++++++++++++++++ 5 files changed, 113 insertions(+), 11 deletions(-) diff --git a/config-example.yaml b/config-example.yaml index d6a995d2e..1c1766b27 100644 --- a/config-example.yaml +++ b/config-example.yaml @@ -187,9 +187,6 @@ database: # GORM configuration settings. gorm: - # Enable prepared statements. - prepare_stmt: true - # Enable parameterized queries. parameterized_queries: true diff --git a/hscontrol/db/db.go b/hscontrol/db/db.go index aadbb3f7f..c606b5e1c 100644 --- a/hscontrol/db/db.go +++ b/hscontrol/db/db.go @@ -439,11 +439,11 @@ func openDB(cfg types.DatabaseConfig) (*gorm.DB, error) { return nil, fmt.Errorf("building sqlite connection URL: %w", err) } + // No PrepareStmt: its cache can wait on the one connection a transaction holds. db, err := gorm.Open( sqlite.Open(connectionURL), &gorm.Config{ - PrepareStmt: cfg.Gorm.PrepareStmt, - Logger: dbLogger, + Logger: dbLogger, }, ) if err != nil { diff --git a/hscontrol/db/db_test.go b/hscontrol/db/db_test.go index 2cd0d9edc..d3cb2d1d0 100644 --- a/hscontrol/db/db_test.go +++ b/hscontrol/db/db_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/juanfont/headscale/hscontrol/types" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gorm.io/gorm" @@ -22,6 +23,74 @@ import ( // TODO(kradalby): remove in 0.31 with the credentials migration. const legacyPlaintextKey = "plaintextlegacykey0000000000000000000000000000" +func TestSQLiteSameQueryInAndOutsideTransaction(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + configPath := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(configPath, []byte(` +server_url: https://example.com +prefixes: + v4: 100.64.0.0/10 +noise: + private_key_path: noise_private.key +dns: + magic_dns: false + override_local_dns: false +database: + type: sqlite3 + sqlite: + path: headscale.db + gorm: + prepare_stmt: true +`), 0o600)) + require.NoError(t, types.LoadConfig(configPath, true)) + + cfg, err := types.LoadServerConfig() + require.NoError(t, err) + hsdb, err := NewHeadscaleDatabase(cfg) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, hsdb.Close()) }) + + user, err := hsdb.CreateUser(types.User{Name: "test-user"}) + require.NoError(t, err) + sqlDB, err := hsdb.DB.DB() + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + + hsdb.DB = hsdb.DB.WithContext(ctx) + poolResult := make(chan error, 1) + + err = hsdb.Read(func(rx *gorm.DB) error { + waitCount := sqlDB.Stats().WaitCount + + go func() { + _, err := hsdb.GetUserByID(types.UserID(user.ID)) + poolResult <- err + }() + + require.Eventually(t, func() bool { + return sqlDB.Stats().WaitCount > waitCount + }, time.Second, time.Millisecond) + + _, err := GetUserByID(rx, types.UserID(user.ID)) + + return err + }) + require.NoError(t, err) + + select { + case err := <-poolResult: + require.NoError(t, err) + case <-ctx.Done(): + t.Fatal( + "pool query did not finish after the transaction released its connection", + ) + } +} + // TestSQLiteMigrationAndDataValidation tests specific SQLite migration scenarios // and validates data integrity after migration. All migrations that require data validation // should be added here. diff --git a/hscontrol/types/config.go b/hscontrol/types/config.go index 29fa7283d..8792498d2 100644 --- a/hscontrol/types/config.go +++ b/hscontrol/types/config.go @@ -285,7 +285,6 @@ type GormConfig struct { SlowThreshold time.Duration SkipErrRecordNotFound bool ParameterizedQueries bool - PrepareStmt bool } type DatabaseConfig struct { @@ -760,6 +759,10 @@ func validateServerConfigInto(v *configValidator) { // Removed: oidc.expiry -> node.expiry depr.fatalIfSet("oidc.expiry", "node.expiry") + // Ignored: database.gorm.prepare_stmt. A warning, not a fatal, so configs + // that shipped with it set still start. + depr.warn("database.gorm.prepare_stmt") + // OIDC is activated by setting oidc.issuer (see app.go), not by a // dedicated oidc.enabled key. Gate validation on the real activation // condition so a misconfiguration fails at startup. @@ -1237,7 +1240,6 @@ func databaseConfig() DatabaseConfig { skipErrRecordNotFound := viper.GetBool("database.gorm.skip_err_record_not_found") slowThreshold := time.Duration(viper.GetInt64("database.gorm.slow_threshold")) * time.Millisecond parameterizedQueries := viper.GetBool("database.gorm.parameterized_queries") - prepareStmt := viper.GetBool("database.gorm.prepare_stmt") switch type_ { case DatabaseSqlite, DatabasePostgres: @@ -1254,7 +1256,6 @@ func databaseConfig() DatabaseConfig { SkipErrRecordNotFound: skipErrRecordNotFound, SlowThreshold: slowThreshold, ParameterizedQueries: parameterizedQueries, - PrepareStmt: prepareStmt, }, Sqlite: SqliteConfig{ Path: util.AbsolutePathFromConfigPath( @@ -1955,8 +1956,6 @@ func (d *deprecator) warnNoAlias(newKey, oldKey string) { // warn records a non-blocking warning when oldKey is set, with no // replacement to point at. -// -//nolint:unused func (d *deprecator) warn(oldKey string) { if viper.IsSet(oldKey) { d.addWarn(deprecation{OldKey: oldKey}) @@ -1977,7 +1976,7 @@ func (d *deprecator) Apply(v *configValidator) { continue } - log.Warn().Msgf("configuration key %q is deprecated", w.OldKey) + log.Warn().Msgf("configuration key %q is ignored", w.OldKey) } for _, f := range d.fatals { diff --git a/hscontrol/types/config_test.go b/hscontrol/types/config_test.go index 2fd135fc9..a5f1d2202 100644 --- a/hscontrol/types/config_test.go +++ b/hscontrol/types/config_test.go @@ -1,6 +1,7 @@ package types import ( + "bytes" "encoding/json" "fmt" "io/fs" @@ -16,6 +17,8 @@ import ( hsdns "github.com/juanfont/headscale/hscontrol/dns" "github.com/juanfont/headscale/hscontrol/util" "github.com/libdns/cloudflare" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -362,6 +365,40 @@ func TestReadConfigFromEnv(t *testing.T) { } } +func TestPreparedStatementConfigIsIgnoredWithWarning(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + var logs bytes.Buffer + + previousLogger := log.Logger + previousLevel := zerolog.GlobalLevel() + + zerolog.SetGlobalLevel(zerolog.WarnLevel) + t.Cleanup(func() { zerolog.SetGlobalLevel(previousLevel) }) + + log.Logger = zerolog.New(&logs) + + t.Cleanup(func() { log.Logger = previousLogger }) + + configPath := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(configPath, []byte(` +server_url: https://example.com +noise: + private_key_path: noise_private.key +dns: + magic_dns: false + override_local_dns: false +database: + type: sqlite3 + gorm: + prepare_stmt: true +`), 0o600)) + require.NoError(t, LoadConfig(configPath, true)) + require.NoError(t, validateServerConfig()) + assert.Contains(t, logs.String(), "database.gorm.prepare_stmt") +} + func TestTLSConfigValidation(t *testing.T) { tmpDir := t.TempDir()