datastore: MySQL/PostgreSQL driver correctness & connection safety

[Jakob3xD]

Summary

Several correctness and resource-safety bugs in the relational datastore drivers (internal/datastore/mysql.go, postgresql.go). All were confirmed by reading the code.

Findings

1. PostgreSQL connection options: pointer aliased instead of struct-copied (High)

internal/datastore/postgresql.go:109-114

opt := &pg.Options{ ... }
fn := func(dbName string) *pg.DB {
    o := opt        // copies the POINTER, not the struct
    o.Database = dbName
    return pg.Connect(o)
}

o := opt aliases the shared *pg.Options that the root connection pg.Connect(opt) (line 117) also uses. Every switchDatabaseFn call mutates opt.Database globally. Under concurrent reconciles on the same PG datastore this is a data race on opt.Database, and GrantPrivileges/DeleteDB/Migrate can run against the wrong database.

Fix: dereference-copy the struct: o := *opt; o.Database = dbName; return pg.Connect(&o).

2. MySQL migration switches to namespace_name instead of the configured schema (High)

internal/datastore/mysql.go:62,84 (vs 50-51)

DBExists/CreateDB use tcp.Status.Storage.Setup.Schema, but the USE statements build the DB name raw from tcp.GetNamespace()/tcp.GetName():

c.db.ExecContext(ctx, fmt.Sprintf("USE %s_%s", tcp.GetNamespace(), tcp.GetName()))

This does not match GetDefaultDatastoreSchema() = normalizeNamespaceName() which replaces -→_, and diverges entirely once a custom DataStoreSchema is set. For TCPs with a dash in namespace/name, or a custom schema, migration switches to a wrong/non-existent DB → silent data corruption/loss or migration failure.

Fix: use tcp.Status.Storage.Setup.Schema consistently for the USE statements.

3. RegisterTLSConfig uses a process-global constant key (High)

internal/datastore/mysql.go:114-121

tlsKey := "mysql"
mysql.RegisterTLSConfig(tlsKey, config.TLSConfig)

The key is constant for all MySQL datastores, but RegisterTLSConfig writes into a process-global map. Two datastores with different CAs/client certs overwrite each other → a connection can validate against the wrong CA or present the wrong client certificate (last-write-wins / race).

Fix: use a per-datastore unique key (name/UID) and deregister after use.

4. GrantPrivilegesExists leaks a connection on the match path (Medium)

internal/datastore/mysql.go:230-255

rows, err := c.db.Query(statementShowGrantsStatement) //nolint:sqlclosecheck
...
for rows.Next() {
    ...
    if grant == expected { return true, nil }  // returns without rows.Close()
}

No defer rows.Close(); the match path (the common success case) returns early leaving rows open. The //nolint:sqlclosecheck masks exactly this. Called on every reconcile (datastore_setup.go:265) → gradual connection-pool exhaustion.

Fix: add defer rows.Close() right after the error check and remove the nolint.

---

🤖 Reported as part of a Claude Code audit.