mirror of
https://github.com/juanfont/headscale.git
synced 2026-11-04 04:34:07 +09:00
db: disable GORM's prepared statement cache on SQLite
database.gorm.prepare_stmt is now ignored, with a startup warning.
This commit is contained in:
1 parent
170ae26ad7
commit
05ca808491
5 files changed
+117
-11
No files matched your search
@@ -187,9 +187,6 @@ database:
|
||||
|
||||
# GORM configuration settings.
|
||||
gorm:
|
||||
# Enable prepared statements.
|
||||
prepare_stmt: true
|
||||
|
||||
# Enable parameterized queries.
|
||||
parameterized_queries: true
|
||||
|
||||
|
||||
+6
-2
@@ -439,11 +439,15 @@ 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.
|
||||
// An outside query can publish a pending cached statement while waiting
|
||||
// for that connection. If the transaction requests the same statement,
|
||||
// it waits for preparation, deadlocking both callers. The regression
|
||||
// test TestSQLiteSameQueryInAndOutsideTransaction covers this ordering.
|
||||
db, err := gorm.Open(
|
||||
sqlite.Open(connectionURL),
|
||||
&gorm.Config{
|
||||
PrepareStmt: cfg.Gorm.PrepareStmt,
|
||||
Logger: dbLogger,
|
||||
Logger: dbLogger,
|
||||
},
|
||||
)
|
||||
if err != nil {
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
Reference in new issue
Block a user