fix(storage): make cockroachdb, mysql, mariadb and sql server work with gorm AutoMigrate

AGENTS.md

@@ -71,23 +71,30 @@ Run `make fmt` before committing; CI runs `make lint`.
 ```bash
 make test                 # Full module test run; TEST_DBS=sqlite (integration tests always SQLite)
 make test-sqlite          # Same as test — explicit SQLite-only, no Docker
-make test-all-db          # All 8 DBs via Docker (postgres, sqlite, mongodb, arangodb, scylladb, cassandradb, dynamodb, couchbase)
+make test-all-db          # All 12 DBs via Docker (postgres, cockroachdb, mysql, mariadb, sqlite, libsql, mongodb, arangodb, scylladb, cassandradb, dynamodb, couchbase)
 make smoke                # Release e2e smoke tests (build tag `smoke`, 5m timeout)
 
 # Single-DB targets (each spins up Docker, runs tests, tears down)
 make test-postgres
+make test-cockroachdb
+make test-mysql
+make test-mariadb
 make test-mongodb
 make test-scylladb
 make test-cassandradb
 make test-arangodb
 make test-dynamodb
 make test-couchbase
+make test-sqlserver        # Not in test-all-db: 2 upsert tests are known-red
+                          # (driver builds MERGE from the PK, ignores OnConflict.Columns)
 
 # Docker helpers for test-all-db
 make test-docker-up       # Start all test DB containers + Redis
 make test-cleanup         # Remove all test containers
-make test-cleanup-postgres | test-cleanup-mongodb | test-cleanup-scylladb | test-cleanup-cassandradb
+make test-cleanup-postgres | test-cleanup-cockroachdb | test-cleanup-mysql | test-cleanup-mariadb
+make test-cleanup-mongodb | test-cleanup-scylladb | test-cleanup-cassandradb
 make test-cleanup-arangodb | test-cleanup-dynamodb | test-cleanup-couchbase
+make test-cleanup-sqlserver
 ```
 
 **Test env vars**:

Makefile

@@ -7,6 +7,12 @@ DOCKER_IMAGE ?= quay.io/authorizer/authorizer:$(VERSION)
 # Integration tests and memory_store/db tests always use SQLite.
 # Redis memory_store tests run only when TEST_ENABLE_REDIS=1.
 GO_TEST_ALL := go test -p 1 -v ./...
+# Pinned to a release line so the suite does not move under us.
+COCKROACH_IMAGE := cockroachdb/cockroach:latest-v24.3
+MYSQL_IMAGE := mysql:8.0.40
+MARIADB_IMAGE := mariadb:11
+SQLSERVER_IMAGE := mcr.microsoft.com/mssql/server:2022-latest
+SQLSERVER_PASSWORD := Str0ng!Passw0rd
 
 .PHONY: all bootstrap build build-app build-dashboard build-local-image build-push-image trivy-scan
 
@@ -123,6 +129,33 @@ test-postgres: test-cleanup-postgres
 	docker rm -vf authorizer_postgres; \
 	exit $$status
 
+# CockroachDB is Postgres-wire compatible and runs through the same GORM SQL
+# provider, so this target exists to prove that compatibility actually holds
+# (AutoMigrate, transactions, LIKE anchoring) rather than being assumed.
+test-cockroachdb: test-cleanup-cockroachdb
+	docker run -d --name authorizer_cockroachdb -p 26257:26257 $(COCKROACH_IMAGE) start-single-node --insecure
+	sh scripts/wait-for-sql.sh authorizer_cockroachdb cockroachdb && \
+	{ go clean --testcache; TEST_DBS="cockroachdb" $(GO_TEST_ALL); }; \
+	status=$$?; \
+	docker rm -vf authorizer_cockroachdb; \
+	exit $$status
+
+test-mysql: test-cleanup-mysql
+	docker run -d --name authorizer_mysql -p 3306:3306 -e MYSQL_ROOT_PASSWORD=password -e MYSQL_DATABASE=authorizer_test $(MYSQL_IMAGE)
+	sh scripts/wait-for-sql.sh authorizer_mysql mysql && \
+	{ go clean --testcache; TEST_DBS="mysql" $(GO_TEST_ALL); }; \
+	status=$$?; \
+	docker rm -vf authorizer_mysql; \
+	exit $$status
+
+test-mariadb: test-cleanup-mariadb
+	docker run -d --name authorizer_mariadb -p 3307:3306 -e MARIADB_ROOT_PASSWORD=password -e MARIADB_DATABASE=authorizer_test $(MARIADB_IMAGE)
+	sh scripts/wait-for-sql.sh authorizer_mariadb mariadb && \
+	{ go clean --testcache; TEST_DBS="mariadb" $(GO_TEST_ALL); }; \
+	status=$$?; \
+	docker rm -vf authorizer_mariadb; \
+	exit $$status
+
 test-sqlite:
 	go clean --testcache && TEST_DBS="sqlite" $(GO_TEST_ALL)
 
@@ -224,7 +257,10 @@ test-all-db: test-cleanup test-docker-up
 	@# the && chain a probe failure is captured by status and still cleans up.
 	sh scripts/wait-for-cql.sh authorizer_scylla_db && \
 	sh scripts/wait-for-cql.sh authorizer_cassandra_db && \
-	{ go clean --testcache; TEST_DBS="couchbase,postgres,sqlite,mongodb,arangodb,scylladb,cassandradb,dynamodb" $(GO_TEST_ALL); }; \
+	sh scripts/wait-for-sql.sh authorizer_mysql mysql && \
+	sh scripts/wait-for-sql.sh authorizer_mariadb mariadb && \
+	sh scripts/wait-for-sql.sh authorizer_cockroachdb cockroachdb && \
+	{ go clean --testcache; TEST_DBS="couchbase,postgres,cockroachdb,mysql,mariadb,sqlite,libsql,mongodb,arangodb,scylladb,cassandradb,dynamodb" $(GO_TEST_ALL); }; \
 	status=$$?; \
 	$(MAKE) test-cleanup; \
 	exit $$status
@@ -233,6 +269,11 @@ test-all-db: test-cleanup test-docker-up
 test-docker-up:
 	docker run -d --name authorizer_redis -p 6380:6379 redis
 	docker run -d --name authorizer_postgres -p 5434:5432 -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=postgres postgres
+	@# Only 26257 (SQL) is published. Cockroach's admin UI is on 8080, which is
+	@# the port `make dev` binds.
+	docker run -d --name authorizer_cockroachdb -p 26257:26257 $(COCKROACH_IMAGE) start-single-node --insecure
+	docker run -d --name authorizer_mysql -p 3306:3306 -e MYSQL_ROOT_PASSWORD=password -e MYSQL_DATABASE=authorizer_test $(MYSQL_IMAGE)
+	docker run -d --name authorizer_mariadb -p 3307:3306 -e MARIADB_ROOT_PASSWORD=password -e MARIADB_DATABASE=authorizer_test $(MARIADB_IMAGE)
 	docker run -d --name authorizer_mongodb_db -p 27017:27017 mongo:4.4.15
 	docker run -d --name authorizer_scylla_db -p 9042:9042 scylladb/scylla
 	docker run -d --name authorizer_cassandra_db -p 9142:9042 -e MAX_HEAP_SIZE=512M -e HEAP_NEWSIZE=128M cassandra:4.1
@@ -252,10 +293,16 @@ test-docker-up:
 	@# there costs nothing: the probe returns in 0s once CQL answers.
 	-sh scripts/wait-for-cql.sh authorizer_scylla_db
 	-sh scripts/wait-for-cql.sh authorizer_cassandra_db
+	-sh scripts/wait-for-sql.sh authorizer_mysql mysql
+	-sh scripts/wait-for-sql.sh authorizer_mariadb mariadb
+	-sh scripts/wait-for-sql.sh authorizer_cockroachdb cockroachdb
 
 # Remove all test database containers
 test-cleanup:
 	-docker rm -vf authorizer_postgres
+	-docker rm -vf authorizer_cockroachdb
+	-docker rm -vf authorizer_mysql
+	-docker rm -vf authorizer_mariadb
 	-docker rm -vf authorizer_scylla_db
 	-docker rm -vf authorizer_cassandra_db
 	-docker rm -vf authorizer_mongodb_db
@@ -266,6 +313,28 @@ test-cleanup:
 
 test-cleanup-postgres:
 	-docker rm -vf authorizer_postgres
+test-cleanup-cockroachdb:
+	-docker rm -vf authorizer_cockroachdb
+test-sqlserver: test-cleanup-sqlserver
+	@# Not part of test-all-db: two upsert tests are known-red on SQL Server
+	@# because gorm.io/driver/sqlserver builds its MERGE predicate from the
+	@# primary key and ignores clause.OnConflict.Columns, so an upsert keyed on
+	@# a composite unique index inserts a second row instead of replacing one.
+	@# Tracked separately from the AutoMigrate fixes; everything else passes.
+	docker run -d --name authorizer_sqlserver -p 1433:1433 \
+		-e ACCEPT_EULA=Y -e MSSQL_SA_PASSWORD='$(SQLSERVER_PASSWORD)' $(SQLSERVER_IMAGE)
+	sh scripts/wait-for-sql.sh authorizer_sqlserver sqlserver && \
+	docker exec authorizer_sqlserver /opt/mssql-tools18/bin/sqlcmd -C -S localhost -U sa \
+		-P '$(SQLSERVER_PASSWORD)' -Q "CREATE DATABASE authorizer_test" && \
+	{ go clean --testcache; TEST_DBS="sqlserver" $(GO_TEST_ALL); }; \
+	status=$$?; \
+	docker rm -vf authorizer_sqlserver; \
+	exit $$status
+
+test-cleanup-mysql:
+	-docker rm -vf authorizer_mysql
+test-cleanup-mariadb:
+	-docker rm -vf authorizer_mariadb
 test-cleanup-mongodb:
 	-docker rm -vf authorizer_mongodb_db
 test-cleanup-scylladb:
@@ -278,6 +347,8 @@ test-cleanup-dynamodb:
 	-docker rm -vf authorizer_dynamodb
 test-cleanup-couchbase:
 	-docker rm -vf authorizer_couchbase
+test-cleanup-sqlserver:
+	-docker rm -vf authorizer_sqlserver
 generate-graphql:
 	go run github.com/99designs/gqlgen --verbose generate && go mod tidy
 generate-db-template:

internal/storage/db/sql/cockroachdb.go

@@ -0,0 +1,63 @@
+package sql
+
+import (
+	"fmt"
+
+	"gorm.io/driver/postgres"
+	"gorm.io/gorm"
+	"gorm.io/gorm/schema"
+)
+
+// cockroachDialector is the Postgres dialector with one migration behaviour
+// changed. It embeds postgres.Dialector rather than the gorm.Dialector
+// interface so the driver's optional interfaces — SavePoint/RollbackTo, which
+// gorm type-asserts for nested transactions — stay promoted.
+type cockroachDialector struct {
+	postgres.Dialector
+}
+
+// newCockroachDialector returns the Postgres dialector for dbURL with the
+// CockroachDB migrator behaviour layered on.
+func newCockroachDialector(dbURL string) gorm.Dialector {
+	return cockroachDialector{postgres.Dialector{Config: &postgres.Config{DSN: dbURL}}}
+}
+
+// Migrator keeps the driver's own migrator — and with it every method gorm
+// expects to find, including the BuildIndexOptions it type-asserts with no
+// ok-check — and replaces only the unique reconciliation.
+func (d cockroachDialector) Migrator(db *gorm.DB) gorm.Migrator {
+	return cockroachMigrator{d.Dialector.Migrator(db).(postgres.Migrator)}
+}
+
+type cockroachMigrator struct {
+	postgres.Migrator
+}
+
+// MigrateColumnUnique leaves the unique indexes the models declare alone; see
+// skipUniqueIndexDrop for why CockroachDB needs this and PostgreSQL does not.
+func (m cockroachMigrator) MigrateColumnUnique(value any, field *schema.Field, columnType gorm.ColumnType) error {
+	if skipUniqueIndexDrop(field, columnType) {
+		return nil
+	}
+	return m.Migrator.MigrateColumnUnique(value, field, columnType)
+}
+
+// dropUniqueConstraint removes a single-column UNIQUE constraint on
+// CockroachDB, where `ALTER TABLE ... DROP CONSTRAINT` — what
+// Migrator.DropConstraint emits — is rejected outright:
+//
+//	unimplemented: cannot drop UNIQUE constraint "authorizer_users_email_key"
+//	using ALTER TABLE DROP CONSTRAINT, use DROP INDEX CASCADE instead
+//	(SQLSTATE 0A000)
+//
+// A UNIQUE constraint there is an index, so it has to go in those terms.
+// Without this the legacy-uniqueness cleanup silently does nothing on an
+// upgraded v1 database, which leaves UNIQUE on authorizer_otps.phone_number —
+// where v2 writes the empty string for every email OTP, so the second one
+// would be rejected.
+//
+// The identifiers are internal: table comes from a fixed list and constraint
+// from this database's own information_schema, never from a request.
+func dropUniqueConstraint(db *gorm.DB, table, constraint string) error {
+	return db.Exec(fmt.Sprintf(`DROP INDEX IF EXISTS %q@%q CASCADE`, table, constraint)).Error
+}

internal/storage/db/sql/migrator.go

@@ -0,0 +1,39 @@
+package sql
+
+import (
+	"gorm.io/gorm"
+	"gorm.io/gorm/schema"
+)
+
+// skipUniqueIndexDrop reports whether AutoMigrate is about to drop a unique
+// index the models declare, and should be left alone.
+//
+// GORM reconciles single-column uniqueness by comparing what the catalog
+// reports against the field's `unique` tag. On a disagreement it issues
+// DROP CONSTRAINT "uni_<table>_<column>" — a NamingStrategy-derived name that
+// only ever exists for a constraint GORM itself created from a `unique` tag.
+//
+// PostgreSQL lists only real UNIQUE constraints in
+// information_schema.table_constraints, so the two agree and nothing happens.
+// CockroachDB lists UNIQUE *indexes* there too, and MySQL reports
+// COLUMN_KEY='UNI' for them, so on both every `gorm:"uniqueIndex"` field looks
+// like a constraint the model no longer wants — and startup dies on a name that
+// was never created:
+//
+//	cockroachdb: constraint "uni_authorizer_oauth_states_state_key" of relation
+//	             "authorizer_oauth_states" does not exist (SQLSTATE 42704)
+//	mysql:       Error 1091 (42000): Can't DROP
+//	             'uni_authorizer_oauth_states_state_key'
+//
+// Those indexes are declared by the models and are meant to be there, so the
+// drop is skipped. The create direction — a `gorm:"unique"` field whose
+// constraint is genuinely missing — is always delegated untouched.
+//
+// Each affected driver wraps its own concrete migrator (see cockroachdb.go and
+// mysql.go) rather than the gorm.Migrator interface: gorm type-asserts
+// migrator.BuildIndexOptionsInterface with no ok-check, so a wrapper that
+// embeds the interface panics the moment a table with an index is created.
+func skipUniqueIndexDrop(field *schema.Field, columnType gorm.ColumnType) bool {
+	unique, ok := columnType.Unique()
+	return ok && unique && !field.Unique && !field.PrimaryKey
+}

internal/storage/db/sql/mysql.go

@@ -0,0 +1,81 @@
+package sql
+
+import (
+	"fmt"
+
+	"gorm.io/driver/mysql"
+	"gorm.io/gorm"
+	"gorm.io/gorm/schema"
+)
+
+// indexedStringSize is the column width GORM's mysql driver already gives an
+// unsized string that carries an `index` or `unique` tag: 191 characters, the
+// widest utf8mb4 column InnoDB can index under the 767-byte prefix limit.
+const indexedStringSize = 191
+
+// mysqlDialector is the MySQL dialector with two migration behaviours changed.
+// It embeds mysql.Dialector (which itself holds a *Config, so Initialize's
+// writes are visible here) rather than the gorm.Dialector interface, so the
+// driver's methods all stay promoted.
+type mysqlDialector struct {
+	mysql.Dialector
+}
+
+// newMySQLDialector returns the MySQL dialector for dbURL with the MySQL
+// migration fixes layered on. It goes through mysql.New so the driver fills in
+// DSNConfig from the DSN, as Initialize expects.
+func newMySQLDialector(dbURL string) gorm.Dialector {
+	return mysqlDialector{*mysql.New(mysql.Config{DSN: dbURL}).(*mysql.Dialector)}
+}
+
+// Migrator keeps the driver's own migrator — and with it every method gorm
+// expects to find, including the BuildIndexOptions it type-asserts with no
+// ok-check — and replaces only the unique reconciliation.
+//
+// Config.Dialector is repointed at this wrapper because
+// migrator.Migrator.DataTypeOf resolves column types through it: left as the
+// driver built it, every DDL type decision would route back to the unwrapped
+// dialector and skip DataTypeOf below.
+func (d mysqlDialector) Migrator(db *gorm.DB) gorm.Migrator {
+	m := d.Dialector.Migrator(db).(mysql.Migrator)
+	// Spelled out: mysql.Migrator embeds both migrator.Migrator and the
+	// driver's Dialector, and each carries a Config, so a bare m.Config is
+	// ambiguous.
+	m.Migrator.Config.Dialector = d
+	return mysqlMigrator{m}
+}
+
+type mysqlMigrator struct {
+	mysql.Migrator
+}
+
+// MigrateColumnUnique leaves the unique indexes the models declare alone; see
+// skipUniqueIndexDrop.
+func (m mysqlMigrator) MigrateColumnUnique(value any, field *schema.Field, columnType gorm.ColumnType) error {
+	if skipUniqueIndexDrop(field, columnType) {
+		return nil
+	}
+	return m.Migrator.MigrateColumnUnique(value, field, columnType)
+}
+
+// DataTypeOf gives an unsized string column under a `uniqueIndex` a width, so
+// MySQL can actually index it.
+//
+// The driver sizes an unsized string to varchar(191) when the field carries
+// `index` or `unique`, but it checks only those two tags — a field tagged
+// `uniqueIndex` (named or composite) is missed and becomes `longtext`. MySQL
+// then rejects the table outright, because a TEXT column needs an explicit key
+// length to be indexed, and the server cannot create its schema at all:
+//
+//	Error 1170 (42000): BLOB/TEXT column 'client_id' used in key specification
+//	without a key length
+//
+// Same width the driver picks for the tags it does recognise, so an indexed
+// column is sized consistently however its index was declared.
+func (d mysqlDialector) DataTypeOf(field *schema.Field) string {
+	if field.DataType == schema.String && field.Size == 0 &&
+		field.TagSettings["UNIQUEINDEX"] != "" && d.DefaultStringSize == 0 {
+		return fmt.Sprintf("varchar(%d)", indexedStringSize)
+	}
+	return d.Dialector.DataTypeOf(field)
+}