From 4828bd88f2e530bf6969b01001c32c43d090d2bb Mon Sep 17 00:00:00 2001 From: jp-ayyappan Date: Thu, 28 May 2026 16:25:42 -0400 Subject: [PATCH 1/3] fix(ers): lazily register database/sql driver in SQL ERS provider Previously the SQL provider required consumers to add a blank driver import (e.g. _ "github.com/lib/pq") to their own binaries, causing the driver to be registered globally at init time even when the SQL ERS tier was not configured. This change moves driver registration into NewProvider() using pgx/v5/stdlib (already a direct dependency of this module), guarded by a sync.Mutex and a sql.Drivers() pre-check to prevent duplicate-register panics when consumers have already registered the driver themselves. The registration is now lazy: it happens only when a SQL ERS provider is actually instantiated, and only if the driver has not already been registered by the consumer or another code path. Fixes: https://github.com/opentdf/platform/issues/3539 --- .../providers/sql/sql_provider.go | 51 +++++++++++++++++-- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/service/entityresolution/multi-strategy/providers/sql/sql_provider.go b/service/entityresolution/multi-strategy/providers/sql/sql_provider.go index 47f4328edc..86425a21cb 100644 --- a/service/entityresolution/multi-strategy/providers/sql/sql_provider.go +++ b/service/entityresolution/multi-strategy/providers/sql/sql_provider.go @@ -5,15 +5,52 @@ import ( "database/sql" "fmt" "strings" + "sync" - // Database drivers would be imported here: - // _ "github.com/lib/pq" // PostgreSQL driver - // _ "github.com/go-sql-driver/mysql" // MySQL driver - // _ "github.com/mattn/go-sqlite3" // SQLite driver - + "github.com/jackc/pgx/v5/stdlib" "github.com/opentdf/platform/service/entityresolution/multi-strategy/types" ) +var ( + // driverRegMu guards lazy driver registration to prevent duplicate-register panics. + driverRegMu sync.Mutex + registeredDrivers = make(map[string]struct{}) +) + +// ensureDriverRegistered lazily registers the named database/sql driver the first +// time a SQL provider for that driver is created. This avoids the need for +// consumers to add blank driver imports to their own binaries. +// +// Uses pgx/v5/stdlib for postgres (already a platform dependency). Other drivers +// (mysql, sqlite) are not currently auto-registered and must be imported by the +// consumer. Consumers that have already registered the driver themselves are +// handled gracefully via a sql.Drivers() pre-check. +func ensureDriverRegistered(driver string) { + driverRegMu.Lock() + defer driverRegMu.Unlock() + + if _, ok := registeredDrivers[driver]; ok { + return + } + + // Check whether the driver was already registered externally (e.g. via a + // blank import in the consumer binary) before attempting to register it. + for _, d := range sql.Drivers() { + if d == driver { + registeredDrivers[driver] = struct{}{} + return + } + } + + switch strings.ToLower(driver) { + case "postgres": + sql.Register("postgres", stdlib.GetDefaultDriver()) + registeredDrivers[driver] = struct{}{} + } + // mysql and sqlite require imports not present in this module's dependencies. + // Add cases here when those drivers are added to go.mod. +} + // Provider implements the Provider interface for SQL databases type Provider struct { name string @@ -24,6 +61,10 @@ type Provider struct { // NewProvider creates a new SQL provider func NewProvider(ctx context.Context, name string, config Config) (*Provider, error) { + // Register the database/sql driver for this provider's configured driver name + // if it has not already been registered. + ensureDriverRegistered(config.Driver) + provider := &Provider{ name: name, config: config, From b2cf82196f303c9a83deea7d0929599d4d4da895 Mon Sep 17 00:00:00 2001 From: jp-ayyappan Date: Thu, 28 May 2026 17:49:06 -0400 Subject: [PATCH 2/3] fix(ers): normalize driver name to lowercase before registration checks Both the registeredDrivers map lookup and the sql.Drivers() pre-check were case-sensitive, while the switch used strings.ToLower. A consumer passing driver: Postgres (capital P) would bypass the pre-check and trigger a duplicate sql.Register panic. Normalize to strings.ToLower(strings.TrimSpace(driver)) at the top of ensureDriverRegistered and use strings.EqualFold for the sql.Drivers() comparison so all case variants map to a single canonical key. Addresses review feedback from gemini-code-assist and coderabbitai. --- .../multi-strategy/providers/sql/sql_provider.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/service/entityresolution/multi-strategy/providers/sql/sql_provider.go b/service/entityresolution/multi-strategy/providers/sql/sql_provider.go index 86425a21cb..dbba2252b8 100644 --- a/service/entityresolution/multi-strategy/providers/sql/sql_provider.go +++ b/service/entityresolution/multi-strategy/providers/sql/sql_provider.go @@ -26,6 +26,10 @@ var ( // consumer. Consumers that have already registered the driver themselves are // handled gracefully via a sql.Drivers() pre-check. func ensureDriverRegistered(driver string) { + // Normalize to lowercase so "Postgres", "POSTGRES", and "postgres" all resolve + // to the same registered driver name. sql.Register is case-sensitive. + driver = strings.ToLower(strings.TrimSpace(driver)) + driverRegMu.Lock() defer driverRegMu.Unlock() @@ -35,14 +39,15 @@ func ensureDriverRegistered(driver string) { // Check whether the driver was already registered externally (e.g. via a // blank import in the consumer binary) before attempting to register it. + // Use strings.EqualFold so the pre-check is also case-insensitive. for _, d := range sql.Drivers() { - if d == driver { + if strings.EqualFold(d, driver) { registeredDrivers[driver] = struct{}{} return } } - switch strings.ToLower(driver) { + switch driver { case "postgres": sql.Register("postgres", stdlib.GetDefaultDriver()) registeredDrivers[driver] = struct{}{} From 0eed6d935914b91bde5775b016035a39a05a4412 Mon Sep 17 00:00:00 2001 From: jp-ayyappan Date: Thu, 28 May 2026 18:24:26 -0400 Subject: [PATCH 3/3] fix(ers): normalize config.Driver in NewProvider before sql.Open ensureDriverRegistered already normalizes its input to lowercase, but NewProvider was still passing the raw config.Driver to sql.Open. A mixed-case driver (e.g. 'Postgres') would pass through ensureDriverRegistered correctly but then fail sql.Open with 'unknown driver' since database/sql matches driver names by exact string. Normalize config.Driver at the top of NewProvider so both ensureDriverRegistered and sql.Open receive the canonical lowercase name. Addresses review feedback from coderabbitai and gemini-code-assist. --- .../multi-strategy/providers/sql/sql_provider.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/service/entityresolution/multi-strategy/providers/sql/sql_provider.go b/service/entityresolution/multi-strategy/providers/sql/sql_provider.go index dbba2252b8..d9c8c4c181 100644 --- a/service/entityresolution/multi-strategy/providers/sql/sql_provider.go +++ b/service/entityresolution/multi-strategy/providers/sql/sql_provider.go @@ -66,6 +66,11 @@ type Provider struct { // NewProvider creates a new SQL provider func NewProvider(ctx context.Context, name string, config Config) (*Provider, error) { + // Normalize the driver name so "Postgres", "POSTGRES", and "postgres" all + // resolve correctly through ensureDriverRegistered and sql.Open, both of + // which use case-sensitive driver name matching. + config.Driver = strings.ToLower(strings.TrimSpace(config.Driver)) + // Register the database/sql driver for this provider's configured driver name // if it has not already been registered. ensureDriverRegistered(config.Driver)