diff --git a/.golangci.yaml b/.golangci.yaml index 56f39d186..fc2f1c704 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -13,7 +13,20 @@ linters: - bodyclose # unclosed HTTP response bodies - usetesting # enforce t.Context() over context.Background() in tests - unparam # unused/constant function parameters and results + - depguard # banned imports (see settings) settings: + # SchemaBot links exactly one MySQL driver. The deny list is what keeps that + # true: nothing about a second one fails to compile, and the way it breaks + # is silent (see pkg/mysqlerr.Number), so the invariant needs an enforcer + # rather than a comment. + depguard: + rules: + main: + deny: + - pkg: github.com/go-sql-driver/mysql + desc: 'import github.com/block/mysql instead (registered as driver name "block-mysql"). Two MySQL drivers in one binary define two field-identical but distinct *mysql.MySQLError types, and errors.As against one returns false for the other, so a classifier keeps compiling and silently stops recognizing deadlocks. Read codes through pkg/mysqlerr.' + - pkg: github.com/go-mysql/hotswap-dsn-driver + desc: 'use mysqlconn.OpenReloadable, which reloads credentials per pool on block/mysql. That driver embeds upstream go-sql-driver/mysql and cannot be pointed at the fork.' errcheck: exclude-functions: - (io.Closer).Close # bodyclose linter handles HTTP resp.Body; other Close() checked by closeandlog analyzer diff --git a/e2e/consumermodule/go.mod b/e2e/consumermodule/go.mod index baed0fed8..23e55f92f 100644 --- a/e2e/consumermodule/go.mod +++ b/e2e/consumermodule/go.mod @@ -36,8 +36,9 @@ require ( github.com/aws/aws-sdk-go-v2/service/sts v1.43.3 // indirect github.com/aws/smithy-go v1.27.7 // indirect github.com/beorn7/perks v1.0.1 // indirect + github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6 // indirect github.com/block/pg-sprite v0.2.0 // indirect - github.com/block/spirit v0.16.1-0.20260903162727-fc5f1dfb0a40 // indirect + github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c // indirect github.com/bradleyfalzon/ghinstallation/v2 v2.18.0 // indirect github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect @@ -48,8 +49,6 @@ require ( github.com/go-logr/logr v1.4.4 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-mysql-org/go-mysql v1.16.1-0.20260731133054-6f853f178dc3 // indirect - github.com/go-mysql/hotswap-dsn-driver v1.0.1 // indirect - github.com/go-sql-driver/mysql v1.10.0 // indirect github.com/goccy/go-json v0.10.6 // indirect github.com/gofri/go-github-ratelimit/v2 v2.0.2 // indirect github.com/golang-jwt/jwt/v4 v4.5.2 // indirect @@ -126,6 +125,4 @@ replace github.com/block/schemabot => ../.. // Mirror the parent module's replace directives; replaces do not propagate // across module boundaries. -replace vitess.io/vitess => github.com/block/vitess v0.0.0-20260703150944-881ec2298245 - -replace github.com/pingcap/tidb/pkg/parser => github.com/block/tidb/pkg/parser v0.0.0-20260506200501-e528fd979fc8 +replace vitess.io/vitess => github.com/block/vitess v0.0.0-20260907005807-88d15fda31ea diff --git a/e2e/consumermodule/go.sum b/e2e/consumermodule/go.sum index a424923d1..4c4ad53a4 100644 --- a/e2e/consumermodule/go.sum +++ b/e2e/consumermodule/go.sum @@ -48,14 +48,14 @@ github.com/aws/smithy-go v1.27.7 h1:Zgj5z4LfcDYoQIVk+n/yGdTkP/2y6ZT5vYxe0fp7bqE= github.com/aws/smithy-go v1.27.7/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6 h1:GvubwsqXHanJkhBotCs4XdEmSnwzhHQe7DVGrn+NFok= +github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6/go.mod h1:KEo73lbxXs9cFlq+x3Z35UqGg3MTxAPfjDOR/ob/iik= github.com/block/pg-sprite v0.2.0 h1:H6w/MNJf1rc7XtdVEI0Sq63I2+MkiifPgkS3qfZ9Rz8= github.com/block/pg-sprite v0.2.0/go.mod h1:vZxHdTMrCOPAYgswveB7PSjOaOuRgnDLGRw6WoOizRg= -github.com/block/spirit v0.16.1-0.20260903162727-fc5f1dfb0a40 h1:fEnxgrBNGJj4CtcYLfcQ2uVeqzTP9/9ZsUdKdP4Wb74= -github.com/block/spirit v0.16.1-0.20260903162727-fc5f1dfb0a40/go.mod h1:DmRuKoQODH6VReVLEsfMSgYPuVLUi051cx8ya1WylWM= -github.com/block/tidb/pkg/parser v0.0.0-20260506200501-e528fd979fc8 h1:+OfdTacrEyjlqcRUpBFX9uJ6ROBq6cUjwY4DClhnsdU= -github.com/block/tidb/pkg/parser v0.0.0-20260506200501-e528fd979fc8/go.mod h1:zDLDsfNBU5+L6T4J9/OgWAHc/WZvMUjbpgHqQ/t3yKo= -github.com/block/vitess v0.0.0-20260703150944-881ec2298245 h1:R7e7uAxl6WIZpeY957JDsrZtuihck6vm7QgRooI295U= -github.com/block/vitess v0.0.0-20260703150944-881ec2298245/go.mod h1:tOLnFt2ryuSGSYZ9NxLjsRhYrWxGBxz/z0zxrvuWYwE= +github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c h1:Gdd1vWs0UKLlvq84+4wMveyls7QkFqhQDpF21KR2cIA= +github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c/go.mod h1:Lg97/e4zr2X3AQXRUrDVAQZOVqFDZ09px503h4v/Yss= +github.com/block/vitess v0.0.0-20260907005807-88d15fda31ea h1:t9VROoN/aCzwwh7Ap90TLijZz3rZHZ6HUgWymC+yVS8= +github.com/block/vitess v0.0.0-20260907005807-88d15fda31ea/go.mod h1:193fxGVSfNHDStC08wSg/RzROVHmMh6XoT+QdfSoLjY= github.com/bradleyfalzon/ghinstallation/v2 v2.18.0 h1:WPqnN6NS9XvYlOgZQAIseN7Z1uAiE+UxgDKlW7FvFuU= github.com/bradleyfalzon/ghinstallation/v2 v2.18.0/go.mod h1:gpoSwwWc4biE49F7n+roCcpkEkZ1Qr9soZ2ESvMiouU= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= @@ -103,12 +103,8 @@ github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/go-mysql-org/go-mysql v1.16.1-0.20260731133054-6f853f178dc3 h1:ZLbHAIwCSSBhXzeM3IHrKYKlghU2WGQ4NETbld7iiyY= github.com/go-mysql-org/go-mysql v1.16.1-0.20260731133054-6f853f178dc3/go.mod h1:VjBTZTTDKL8OMXUAhNbg3VHaVVq9HOXJEBLpAKBFIfE= -github.com/go-mysql/hotswap-dsn-driver v1.0.1 h1:Ssm8Gqk3DLkZSyEuFOh1g1ReKBwhLiuYF0Qy4eb0zvo= -github.com/go-mysql/hotswap-dsn-driver v1.0.1/go.mod h1:YjeTwHrsEcFpzJDfOFO95lPZACK/TVo/xtu4Qgi2uC8= github.com/go-ole/go-ole v1.3.0 h1:Dt6ye7+vXGIKZ7Xtk4s6/xVdGDQynvom7xCFEdWr6uE= github.com/go-ole/go-ole v1.3.0/go.mod h1:5LS6F96DhAwUc7C+1HLexzMXY1xGRSryjyPPKW6zv78= -github.com/go-sql-driver/mysql v1.10.0 h1:Q+1LV8DkHJvSYAdR83XzuhDaTykuDx0l6fkXxoWCWfw= -github.com/go-sql-driver/mysql v1.10.0/go.mod h1:M+cqaI7+xxXGG9swrdeUIoPG3Y3KCkF0pZej+SK+nWk= github.com/goccy/go-json v0.10.6 h1:p8HrPJzOakx/mn/bQtjgNjdTcN+/S6FcG2CTtQOrHVU= github.com/goccy/go-json v0.10.6/go.mod h1:oq7eo15ShAhp70Anwd5lgX2pLfOS3QCiwU/PULtXL6M= github.com/gofri/go-github-ratelimit/v2 v2.0.2 h1:gS8wAS1jTmlWGdTjAM7KIpsLjwY1S0S/gKK5hthfSXM= @@ -198,6 +194,8 @@ github.com/pingcap/errors v0.11.5-0.20260310054046-9c8b3586e4b2 h1:cLgCk5mwDG9lD github.com/pingcap/errors v0.11.5-0.20260310054046-9c8b3586e4b2/go.mod h1:ktAJCA9lxrHHjVyVl2pKJFvzBnq2eZbb+CUOjBRPlXo= github.com/pingcap/log v1.1.1-0.20260227082333-572e590d08f1 h1:A2bEfgSb7hLwR9mxDszgGKweF+xY9YoTDG+8RjdFjDE= github.com/pingcap/log v1.1.1-0.20260227082333-572e590d08f1/go.mod h1:pxfz2oJfAuhwrb3/rcLqD//GS/5gRP4gD022iP3cEO0= +github.com/pingcap/tidb/pkg/parser v0.0.0-20260504140133-511dba1dbe17 h1:cfAVPis6GP6lxQgm1WGaNGi4rVXTB4KDvYf96LjqRCM= +github.com/pingcap/tidb/pkg/parser v0.0.0-20260504140133-511dba1dbe17/go.mod h1:zDLDsfNBU5+L6T4J9/OgWAHc/WZvMUjbpgHqQ/t3yKo= github.com/planetscale/planetscale-go v0.155.0 h1:KYFRWFn9d5BeZc++4DF0wS+mlRQ4efrAy+6Zw/1kzXs= github.com/planetscale/planetscale-go v0.155.0/go.mod h1:paQCI5SgquuoewvMQM7R+r1XJO868bdP6/ihGidYRM0= github.com/planetscale/vtprotobuf v0.6.1-0.20250313105119-ba97887b0a25 h1:S1hI5JiKP7883xBzZAr1ydcxrKNSVNm7+3+JwjxZEsg= diff --git a/e2e/grpc/grpc_test.go b/e2e/grpc/grpc_test.go index 476eb8dd0..8fa3c400a 100644 --- a/e2e/grpc/grpc_test.go +++ b/e2e/grpc/grpc_test.go @@ -48,10 +48,10 @@ import ( "testing" "time" + _ "github.com/block/mysql" "github.com/block/schemabot/e2e/testutil" "github.com/block/schemabot/pkg/state" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -61,7 +61,7 @@ func TestMain(m *testing.M) { // Clean up SchemaBot's state tables to ensure fresh state dsn := os.Getenv("E2E_SCHEMABOT_MYSQL_DSN") if dsn != "" { - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err == nil { rows, err := db.QueryContext(context.Background(), "SHOW TABLES") if err == nil { @@ -86,7 +86,7 @@ func TestMain(m *testing.M) { if ternDSN == "" { continue } - db, err := sql.Open("mysql", ternDSN) + db, err := sql.Open("block-mysql", ternDSN) if err != nil { continue } diff --git a/e2e/grpc/helpers_test.go b/e2e/grpc/helpers_test.go index 899fa4964..57ea560e9 100644 --- a/e2e/grpc/helpers_test.go +++ b/e2e/grpc/helpers_test.go @@ -15,10 +15,10 @@ import ( "testing" "time" + _ "github.com/block/mysql" "github.com/block/schemabot/e2e/testutil" "github.com/block/schemabot/pkg/state" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/require" ) @@ -385,7 +385,7 @@ func grpcEnsureNoActiveChange(t *testing.T, database, env string) { func grpcClearSchemabotState(t *testing.T) { t.Helper() dsn := grpcSchemabotMySQLDSN(t) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { t.Logf("warning: could not open schemabot db to clear state: %v", err) return @@ -418,7 +418,7 @@ func grpcClearTernStorage(t *testing.T, env string) { testappDSN := grpcTernMySQLDSN(t, env) ternDSN := strings.Replace(testappDSN, "/testapp", "/tern", 1) - db, err := sql.Open("mysql", ternDSN) + db, err := sql.Open("block-mysql", ternDSN) if err != nil { t.Logf("warning: could not open tern storage db (%s): %v", env, err) return @@ -452,7 +452,7 @@ func grpcClearTernStorage(t *testing.T, env string) { func grpcCreateTestTable(t *testing.T, env, tableName, ddl string) { t.Helper() dsn := grpcTernMySQLDSN(t, env) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoErrorf(t, err, "open tern mysql (%s)", env) _, err = db.ExecContext(t.Context(), ddl) @@ -460,7 +460,7 @@ func grpcCreateTestTable(t *testing.T, env, tableName, ddl string) { _ = db.Close() t.Cleanup(func() { - db2, err := sql.Open("mysql", dsn) + db2, err := sql.Open("block-mysql", dsn) if err != nil { return } @@ -487,7 +487,7 @@ func grpcSeedRows(t *testing.T, env, tableName, columns, valueTemplate string, r func grpcColumnExists(t *testing.T, env, tableName, columnName string) bool { t.Helper() dsn := grpcTernMySQLDSN(t, env) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoErrorf(t, err, "open tern mysql (%s)", env) defer utils.CloseAndLog(db) diff --git a/e2e/grpc/multideploy_test.go b/e2e/grpc/multideploy_test.go index 1f6736c69..4a47e74df 100644 --- a/e2e/grpc/multideploy_test.go +++ b/e2e/grpc/multideploy_test.go @@ -14,11 +14,11 @@ import ( "testing" "time" + "github.com/block/mysql" "github.com/block/schemabot/e2e/testutil" "github.com/block/schemabot/pkg/state" "github.com/block/schemabot/pkg/storage" "github.com/block/spirit/pkg/utils" - "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -117,7 +117,7 @@ func multiDeployTernMySQLDSN(t *testing.T, deployment string) string { func multiDeployCreateTestTable(t *testing.T, deployment, tableName, ddl string) { t.Helper() dsn := multiDeployTernMySQLDSN(t, deployment) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoErrorf(t, err, "open tern mysql (%s)", deployment) // Defer close immediately so the handle is reclaimed even if the create // below fails a require.* assertion, and so close errors are logged. @@ -126,7 +126,7 @@ func multiDeployCreateTestTable(t *testing.T, deployment, tableName, ddl string) require.NoErrorf(t, err, "create table %s on %s", tableName, deployment) t.Cleanup(func() { - db2, err := sql.Open("mysql", dsn) + db2, err := sql.Open("block-mysql", dsn) if err != nil { t.Logf("cleanup: open tern mysql (%s): %v", deployment, err) return @@ -153,7 +153,7 @@ func multiDeployCreateTestTable(t *testing.T, deployment, tableName, ddl string) func multiDeploySeedRows(t *testing.T, deployment, tableName, columns, valueTemplate string, rowCount int) { t.Helper() dsn := multiDeployTernMySQLDSN(t, deployment) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoErrorf(t, err, "open tern mysql (%s)", deployment) defer utils.CloseAndLog(db) @@ -228,7 +228,7 @@ func multiDeployTernStorageDSN(t *testing.T, deployment string) string { func multiDeployClearTernStorage(t *testing.T, deployments ...string) { t.Helper() for _, d := range deployments { - db, err := sql.Open("mysql", multiDeployTernStorageDSN(t, d)) + db, err := sql.Open("block-mysql", multiDeployTernStorageDSN(t, d)) require.NoErrorf(t, err, "cleanup: open tern storage db (%s)", d) func() { defer utils.CloseAndLog(db) @@ -293,7 +293,7 @@ func multiDeployClearTernStorage(t *testing.T, deployments ...string) { // applies row on the deployment's Tern. func multiDeploySpendRecoveryBudget(t *testing.T, deployment string) { t.Helper() - db, err := sql.Open("mysql", multiDeployTernStorageDSN(t, deployment)) + db, err := sql.Open("block-mysql", multiDeployTernStorageDSN(t, deployment)) require.NoErrorf(t, err, "open tern storage db (%s)", deployment) t.Cleanup(func() { utils.CloseAndLog(db) }) require.NoErrorf(t, db.PingContext(t.Context()), "ping tern storage db (%s)", deployment) diff --git a/e2e/k8s/dataplane_progress_ownership_test.go b/e2e/k8s/dataplane_progress_ownership_test.go index d5f6d1ef9..bbb52d687 100644 --- a/e2e/k8s/dataplane_progress_ownership_test.go +++ b/e2e/k8s/dataplane_progress_ownership_test.go @@ -234,7 +234,7 @@ func waitForPodRowCopyInFlight(t *testing.T, client ternv1.TernClient, applyID, // indexExists reports whether the named index is present on the table. func indexExists(t *testing.T, dsn, tableName, indexName string) bool { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(t.Context())) diff --git a/e2e/k8s/k8s_test.go b/e2e/k8s/k8s_test.go index f459cdba4..d2331cd96 100644 --- a/e2e/k8s/k8s_test.go +++ b/e2e/k8s/k8s_test.go @@ -54,7 +54,7 @@ import ( "testing" "time" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" "github.com/block/schemabot/e2e/testutil" "github.com/block/schemabot/pkg/apitypes" @@ -523,7 +523,7 @@ func TestK8s_PlanApply_CreateTable(t *testing.T) { // Register cleanup for the table the apply created t.Cleanup(func() { - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { return } diff --git a/e2e/local/apply_wait_test.go b/e2e/local/apply_wait_test.go index e48852df0..ff7b9b4ab 100644 --- a/e2e/local/apply_wait_test.go +++ b/e2e/local/apply_wait_test.go @@ -92,7 +92,7 @@ func applyTimeoutDiagnostics(applyID string) string { if dsn == "" { return "diagnostics: E2E_MYSQL_DSN not set" } - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { return fmt.Sprintf("diagnostics: open schemabot db: %v", err) } diff --git a/e2e/local/helpers_test.go b/e2e/local/helpers_test.go index d1e8dfd22..8ca2f0b73 100644 --- a/e2e/local/helpers_test.go +++ b/e2e/local/helpers_test.go @@ -83,7 +83,7 @@ func newSchemaDir(t *testing.T) string { func openTestappStaging(t *testing.T) *sql.DB { t.Helper() dsn := testappStagingDSN(t) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open testapp staging db") t.Cleanup(func() { utils.CloseAndLog(db) }) return db @@ -298,7 +298,7 @@ func clearSchemaBotStateImpl() { if schemabotDSN == "" { return } - db, err := sql.Open("mysql", schemabotDSN) + db, err := sql.Open("block-mysql", schemabotDSN) if err != nil { return } @@ -326,7 +326,7 @@ func clearSchemaBotStateImpl() { func markApplyHeartbeatStale(t *testing.T, applyID string) { t.Helper() - db, err := sql.Open("mysql", mysqlDSN(t)) + db, err := sql.Open("block-mysql", mysqlDSN(t)) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(t.Context())) @@ -398,7 +398,7 @@ func uniqueTableName(prefix string) string { func createTestTable(t *testing.T, tableName, ddlStmt string) { t.Helper() dsn := testappStagingDSN(t) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open db") defer utils.CloseAndLog(db) @@ -406,7 +406,7 @@ func createTestTable(t *testing.T, tableName, ddlStmt string) { require.NoErrorf(t, err, "create table %s", tableName) t.Cleanup(func() { - db2, err := sql.Open("mysql", dsn) + db2, err := sql.Open("block-mysql", dsn) if err != nil { return } @@ -426,7 +426,7 @@ func createTestTable(t *testing.T, tableName, ddlStmt string) { func dropTestTable(t *testing.T, tableName string) { t.Helper() dsn := testappStagingDSN(t) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { return } @@ -440,7 +440,7 @@ func dropTestTable(t *testing.T, tableName string) { func writeBaseFixtureSchemas(t *testing.T, schemaDir string) { t.Helper() dsn := testappStagingDSN(t) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { return } @@ -459,7 +459,7 @@ func writeBaseFixtureSchemas(t *testing.T, schemaDir string) { func writeExistingTablesSchema(t *testing.T, schemaDir string) { t.Helper() dsn := testappStagingDSN(t) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { return } diff --git a/e2e/local/local_test.go b/e2e/local/local_test.go index 330585b3a..e8b71b6d1 100644 --- a/e2e/local/local_test.go +++ b/e2e/local/local_test.go @@ -21,8 +21,8 @@ import ( "testing" "time" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -49,7 +49,7 @@ func TestMain(m *testing.M) { dsn := os.Getenv("E2E_TESTAPP_STAGING_DSN") if dsn != "" { - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err == nil { // Find all tables that are NOT base fixtures rows, err := db.QueryContext(context.Background(), ` @@ -116,7 +116,7 @@ func TestLocal_SchemaBot_Health(t *testing.T) { func TestLocal_SchemaBot_SchemaApplied(t *testing.T) { dsn := mysqlDSN(t) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "connect to MySQL") defer utils.CloseAndLog(db) @@ -142,7 +142,7 @@ func TestLocal_SchemaBot_SchemaApplied(t *testing.T) { func TestLocal_Demo_TestAppTablesCreated(t *testing.T) { // Skip this test if demo tables don't exist (requires 'make demo' first) stagingDSN := testappStagingDSN(t) - db, err := sql.Open("mysql", stagingDSN) + db, err := sql.Open("block-mysql", stagingDSN) if err != nil { t.Skip("Cannot connect to staging database") } @@ -170,7 +170,7 @@ func TestLocal_Demo_TestAppTablesCreated(t *testing.T) { }) t.Run("production", func(t *testing.T) { - prodDB, err := sql.Open("mysql", productionDSN) + prodDB, err := sql.Open("block-mysql", productionDSN) require.NoError(t, err, "connect to production MySQL") defer utils.CloseAndLog(prodDB) @@ -1221,7 +1221,7 @@ func TestLocal_Demo_FullValidation(t *testing.T) { require.Equalf(t, http.StatusOK, resp.StatusCode, "SchemaBot health check failed") schemabotDSN := mysqlDSN(t) - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "connect to schemabot MySQL") defer utils.CloseAndLog(schemabotDB) @@ -1237,7 +1237,7 @@ func TestLocal_Demo_FullValidation(t *testing.T) { } stagingDSN := testappStagingDSN(t) - stagingDB, err := sql.Open("mysql", stagingDSN) + stagingDB, err := sql.Open("block-mysql", stagingDSN) require.NoError(t, err, "connect to staging MySQL") defer utils.CloseAndLog(stagingDB) @@ -1259,7 +1259,7 @@ func TestLocal_Demo_FullValidation(t *testing.T) { } productionDSN := testappProductionDSN(t) - productionDB, err := sql.Open("mysql", productionDSN) + productionDB, err := sql.Open("block-mysql", productionDSN) require.NoError(t, err, "connect to production MySQL") defer utils.CloseAndLog(productionDB) diff --git a/e2e/testutil/db.go b/e2e/testutil/db.go index 0882a8c96..b5869e9e8 100644 --- a/e2e/testutil/db.go +++ b/e2e/testutil/db.go @@ -10,10 +10,10 @@ import ( "testing" "time" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/lint" "github.com/block/spirit/pkg/statement" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/require" ) @@ -21,7 +21,7 @@ import ( // plane's storage or target database. The handle is closed when the test ends. func OpenMySQL(t *testing.T, dsn string) *sql.DB { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open mysql") t.Cleanup(func() { utils.CloseAndLog(db) }) require.NoError(t, db.PingContext(t.Context()), "ping mysql") @@ -33,7 +33,7 @@ func OpenMySQL(t *testing.T, dsn string) *sql.DB { // after the test context is cancelled. func CreateTestTable(t *testing.T, dsn, tableName, ddl string) func() { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open mysql for create table") require.NoError(t, db.PingContext(t.Context()), "ping mysql for create table") @@ -42,7 +42,7 @@ func CreateTestTable(t *testing.T, dsn, tableName, ddl string) func() { require.NoError(t, err, "create table %s", tableName) return func() { - db2, err := sql.Open("mysql", dsn) + db2, err := sql.Open("block-mysql", dsn) if err != nil { return } @@ -112,7 +112,7 @@ func SeedRows(t *testing.T, dsn, tableName, columns, valueTemplate string, rowCo t.Helper() require.Positive(t, rowCount, "seed row count for %s", tableName) require.LessOrEqual(t, rowCount, maxSeedRows, "seed row count for %s", tableName) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open mysql for seeding") defer utils.CloseAndLog(db) @@ -152,7 +152,7 @@ func sequenceGenerator(rowCount int) string { // typically called from t.Cleanup where t.Context() is already canceled. func ClearAllTables(t *testing.T, dsn string) { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { t.Logf("warning: could not open db to clear tables: %v", err) return diff --git a/go.mod b/go.mod index 769cd156d..2e5a7ae39 100644 --- a/go.mod +++ b/go.mod @@ -9,8 +9,9 @@ require ( github.com/aws/aws-sdk-go-v2/credentials v1.19.14 github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.44.5 github.com/aws/aws-sdk-go-v2/service/sts v1.43.3 + github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6 github.com/block/pg-sprite v0.2.0 - github.com/block/spirit v0.16.1-0.20260903162727-fc5f1dfb0a40 + github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c github.com/bradleyfalzon/ghinstallation/v2 v2.18.0 github.com/charmbracelet/bubbles v1.0.0 github.com/charmbracelet/bubbletea v1.3.10 @@ -18,8 +19,6 @@ require ( github.com/coreos/go-oidc/v3 v3.18.0 github.com/go-jose/go-jose/v4 v4.1.4 github.com/go-mysql-org/go-mysql v1.16.1-0.20260731133054-6f853f178dc3 - github.com/go-mysql/hotswap-dsn-driver v1.0.1 - github.com/go-sql-driver/mysql v1.10.0 github.com/gofri/go-github-ratelimit/v2 v2.0.2 github.com/google/go-github/v86 v86.0.0 github.com/google/uuid v1.6.0 @@ -140,6 +139,7 @@ require ( github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-ole/go-ole v1.3.0 // indirect + github.com/go-sql-driver/mysql v1.10.0 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/goccy/go-json v0.10.6 // indirect github.com/godbus/dbus/v5 v5.2.2 // indirect @@ -274,7 +274,4 @@ require ( ) // needed for Strata and vtcombo OnlineDDL suppport -replace vitess.io/vitess => github.com/block/vitess v0.0.0-20260703150944-881ec2298245 - -// needed for SPATIAL index support in Spirit v0.13.0 -replace github.com/pingcap/tidb/pkg/parser => github.com/block/tidb/pkg/parser v0.0.0-20260506200501-e528fd979fc8 +replace vitess.io/vitess => github.com/block/vitess v0.0.0-20260907005807-88d15fda31ea diff --git a/go.sum b/go.sum index c235324b1..c34e9eb82 100644 --- a/go.sum +++ b/go.sum @@ -115,14 +115,14 @@ github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6 h1:GvubwsqXHanJkhBotCs4XdEmSnwzhHQe7DVGrn+NFok= +github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6/go.mod h1:KEo73lbxXs9cFlq+x3Z35UqGg3MTxAPfjDOR/ob/iik= github.com/block/pg-sprite v0.2.0 h1:H6w/MNJf1rc7XtdVEI0Sq63I2+MkiifPgkS3qfZ9Rz8= github.com/block/pg-sprite v0.2.0/go.mod h1:vZxHdTMrCOPAYgswveB7PSjOaOuRgnDLGRw6WoOizRg= -github.com/block/spirit v0.16.1-0.20260903162727-fc5f1dfb0a40 h1:fEnxgrBNGJj4CtcYLfcQ2uVeqzTP9/9ZsUdKdP4Wb74= -github.com/block/spirit v0.16.1-0.20260903162727-fc5f1dfb0a40/go.mod h1:DmRuKoQODH6VReVLEsfMSgYPuVLUi051cx8ya1WylWM= -github.com/block/tidb/pkg/parser v0.0.0-20260506200501-e528fd979fc8 h1:+OfdTacrEyjlqcRUpBFX9uJ6ROBq6cUjwY4DClhnsdU= -github.com/block/tidb/pkg/parser v0.0.0-20260506200501-e528fd979fc8/go.mod h1:zDLDsfNBU5+L6T4J9/OgWAHc/WZvMUjbpgHqQ/t3yKo= -github.com/block/vitess v0.0.0-20260703150944-881ec2298245 h1:R7e7uAxl6WIZpeY957JDsrZtuihck6vm7QgRooI295U= -github.com/block/vitess v0.0.0-20260703150944-881ec2298245/go.mod h1:tOLnFt2ryuSGSYZ9NxLjsRhYrWxGBxz/z0zxrvuWYwE= +github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c h1:Gdd1vWs0UKLlvq84+4wMveyls7QkFqhQDpF21KR2cIA= +github.com/block/spirit v0.17.1-0.20260907005557-10804bbe247c/go.mod h1:Lg97/e4zr2X3AQXRUrDVAQZOVqFDZ09px503h4v/Yss= +github.com/block/vitess v0.0.0-20260907005807-88d15fda31ea h1:t9VROoN/aCzwwh7Ap90TLijZz3rZHZ6HUgWymC+yVS8= +github.com/block/vitess v0.0.0-20260907005807-88d15fda31ea/go.mod h1:193fxGVSfNHDStC08wSg/RzROVHmMh6XoT+QdfSoLjY= github.com/bndr/gotabulate v1.1.2 h1:yC9izuZEphojb9r+KYL4W9IJKO/ceIO8HDwxMA24U4c= github.com/bndr/gotabulate v1.1.2/go.mod h1:0+8yUgaPTtLRTjf49E8oju7ojpU11YmXyvq1LbPAb3U= github.com/bradleyfalzon/ghinstallation/v2 v2.18.0 h1:WPqnN6NS9XvYlOgZQAIseN7Z1uAiE+UxgDKlW7FvFuU= @@ -236,8 +236,6 @@ github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/go-mysql-org/go-mysql v1.16.1-0.20260731133054-6f853f178dc3 h1:ZLbHAIwCSSBhXzeM3IHrKYKlghU2WGQ4NETbld7iiyY= github.com/go-mysql-org/go-mysql v1.16.1-0.20260731133054-6f853f178dc3/go.mod h1:VjBTZTTDKL8OMXUAhNbg3VHaVVq9HOXJEBLpAKBFIfE= -github.com/go-mysql/hotswap-dsn-driver v1.0.1 h1:Ssm8Gqk3DLkZSyEuFOh1g1ReKBwhLiuYF0Qy4eb0zvo= -github.com/go-mysql/hotswap-dsn-driver v1.0.1/go.mod h1:YjeTwHrsEcFpzJDfOFO95lPZACK/TVo/xtu4Qgi2uC8= github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= github.com/go-ole/go-ole v1.3.0 h1:Dt6ye7+vXGIKZ7Xtk4s6/xVdGDQynvom7xCFEdWr6uE= github.com/go-ole/go-ole v1.3.0/go.mod h1:5LS6F96DhAwUc7C+1HLexzMXY1xGRSryjyPPKW6zv78= @@ -522,6 +520,8 @@ github.com/pingcap/errors v0.11.5-0.20260310054046-9c8b3586e4b2 h1:cLgCk5mwDG9lD github.com/pingcap/errors v0.11.5-0.20260310054046-9c8b3586e4b2/go.mod h1:ktAJCA9lxrHHjVyVl2pKJFvzBnq2eZbb+CUOjBRPlXo= github.com/pingcap/log v1.1.1-0.20260227082333-572e590d08f1 h1:A2bEfgSb7hLwR9mxDszgGKweF+xY9YoTDG+8RjdFjDE= github.com/pingcap/log v1.1.1-0.20260227082333-572e590d08f1/go.mod h1:pxfz2oJfAuhwrb3/rcLqD//GS/5gRP4gD022iP3cEO0= +github.com/pingcap/tidb/pkg/parser v0.0.0-20260504140133-511dba1dbe17 h1:cfAVPis6GP6lxQgm1WGaNGi4rVXTB4KDvYf96LjqRCM= +github.com/pingcap/tidb/pkg/parser v0.0.0-20260504140133-511dba1dbe17/go.mod h1:zDLDsfNBU5+L6T4J9/OgWAHc/WZvMUjbpgHqQ/t3yKo= github.com/pires/go-proxyproto v0.11.0 h1:gUQpS85X/VJMdUsYyEgyn59uLJvGqPhJV5YvG68wXH4= github.com/pires/go-proxyproto v0.11.0/go.mod h1:ZKAAyp3cgy5Y5Mo4n9AlScrkCZwUy0g3Jf+slqQVcuU= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= diff --git a/integration/cli_test.go b/integration/cli_test.go index f11b89120..575d9eebe 100644 --- a/integration/cli_test.go +++ b/integration/cli_test.go @@ -17,8 +17,8 @@ import ( "testing" "time" + "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -154,7 +154,7 @@ func TestCLI_OnboardPullsLiveMySQLSchema(t *testing.T) { cfg, err := mysql.ParseDSN(targetDSN) require.NoError(t, err) cfg.DBName = dbName - db, err := sql.Open("mysql", cfg.FormatDSN()) + db, err := sql.Open("block-mysql", cfg.FormatDSN()) require.NoError(t, err, "open target database") defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(t.Context()), "ping target database") @@ -587,7 +587,7 @@ CREATE TABLE items ( // Insert rows into the table to make the copy phase take longer t.Run("insert_test_data", func(t *testing.T) { targetDSN := strings.Replace(targetDSN, "/target_test", "/"+dbName, 1) - db, err := sql.Open("mysql", targetDSN) + db, err := sql.Open("block-mysql", targetDSN) require.NoError(t, err, "open target db") defer utils.CloseAndLog(db) @@ -704,7 +704,7 @@ CREATE TABLE items ( // Verify the schema change was applied t.Run("verify_indexes_exist", func(t *testing.T) { targetDSN := strings.Replace(targetDSN, "/target_test", "/"+dbName, 1) - db, err := sql.Open("mysql", targetDSN) + db, err := sql.Open("block-mysql", targetDSN) require.NoError(t, err, "open target db") defer utils.CloseAndLog(db) @@ -756,7 +756,7 @@ CREATE TABLE items ( // before the stop command can intervene. t.Run("insert_test_data", func(t *testing.T) { targetDSN := strings.Replace(targetDSN, "/target_test", "/"+dbName, 1) - db, err := sql.Open("mysql", targetDSN) + db, err := sql.Open("block-mysql", targetDSN) require.NoError(t, err, "open target db") defer utils.CloseAndLog(db) @@ -870,7 +870,7 @@ func startSchemaBotLocal(t *testing.T) string { t.Helper() // Connect to SchemaBot storage and clear stale state from prior tests. - db, err := sql.Open("mysql", schemabotDSN) + db, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") t.Cleanup(func() { utils.CloseAndLog(db) }) clearStorageDB(t, db) @@ -941,7 +941,7 @@ func startSchemaBotLocalDB(t *testing.T, dbName string) string { t.Helper() // Connect to SchemaBot storage and clear stale state from prior tests. - db, err := sql.Open("mysql", schemabotDSN) + db, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") t.Cleanup(func() { utils.CloseAndLog(db) }) clearStorageDB(t, db) @@ -949,7 +949,7 @@ func startSchemaBotLocalDB(t *testing.T, dbName string) string { storage := mysqlstore.New(db) // Create the target database - targetDB, err := sql.Open("mysql", targetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", targetDSN+"&multiStatements=true") require.NoError(t, err, "open target db connection") t.Cleanup(func() { ctx, cancel := testutil.CleanupContext(30 * time.Second) @@ -1025,7 +1025,7 @@ func startSchemaBotWithGRPC(t *testing.T) string { t.Helper() // Connect to SchemaBot storage and clear stale state from prior tests. - db, err := sql.Open("mysql", schemabotDSN) + db, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") t.Cleanup(func() { utils.CloseAndLog(db) }) clearStorageDB(t, db) diff --git a/integration/grpc_integration_test.go b/integration/grpc_integration_test.go index 6779a7059..525995998 100644 --- a/integration/grpc_integration_test.go +++ b/integration/grpc_integration_test.go @@ -16,8 +16,8 @@ import ( "testing" "time" + "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -95,7 +95,7 @@ func startSchemaBot(t *testing.T, ternGRPCAddr string) string { logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) // Create MySQL storage for SchemaBot - db, err := sql.Open("mysql", schemabotDSN) + db, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") t.Cleanup(func() { utils.CloseAndLog(db) }) storage := schemabotmysql.New(db) @@ -188,7 +188,7 @@ func TestGRPC_ExternalID_StoredOnApply(t *testing.T) { ctx := t.Context() // Create a unique target database for this test - targetDB, err := sql.Open("mysql", targetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", targetDSN+"&multiStatements=true") require.NoError(t, err, "open target db") defer utils.CloseAndLog(targetDB) @@ -211,7 +211,7 @@ func TestGRPC_ExternalID_StoredOnApply(t *testing.T) { require.NoError(t, err, "start tern grpc") // Create SchemaBot with its own storage and a GRPCClient to remote Tern - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") defer utils.CloseAndLog(schemabotDB) schemabotStorage := schemabotmysql.New(schemabotDB) @@ -325,7 +325,7 @@ func TestGRPC_TaskStateUpdatedOnCompletion(t *testing.T) { // Create a unique target database for this test. The close cleanup is // registered before the drop cleanup so it runs after it (cleanups run // LIFO): the drop still has a live handle. - targetDB, err := sql.Open("mysql", targetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", targetDSN+"&multiStatements=true") require.NoError(t, err, "open target db") t.Cleanup(func() { utils.CloseAndLog(targetDB) }) @@ -356,7 +356,7 @@ func TestGRPC_TaskStateUpdatedOnCompletion(t *testing.T) { // Create SchemaBot with its own storage and a GRPCClient to remote Tern. // The GRPCClient must have storage so pollForCompletion can update tasks. - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") defer utils.CloseAndLog(schemabotDB) schemabotStorage := schemabotmysql.New(schemabotDB) @@ -487,7 +487,7 @@ func TestGRPC_FailedTableErrorSurfacesInTaskRecord(t *testing.T) { // Create a unique target database for this test. The close cleanup is // registered before the drop cleanup so it runs after it (cleanups run // LIFO): the drop still has a live handle. - targetDB, err := sql.Open("mysql", targetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", targetDSN+"&multiStatements=true") require.NoError(t, err, "open target db") t.Cleanup(func() { utils.CloseAndLog(targetDB) }) require.NoError(t, targetDB.PingContext(ctx), "ping target db") @@ -532,7 +532,7 @@ func TestGRPC_FailedTableErrorSurfacesInTaskRecord(t *testing.T) { // storage, which closes this DB, and every Tern client it was given. These // closes only cover an early failure before the service exists, so they // discard the error the service's own close makes inevitable. - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") defer func() { _ = schemabotDB.Close() }() require.NoError(t, schemabotDB.PingContext(ctx), "ping schemabot db") @@ -634,7 +634,7 @@ func TestGRPC_FailedTableErrorSurfacesInTaskRecord(t *testing.T) { // so the first retryable failure goes through the real expiry pass // immediately, settling the remote apply and its tasks to the same terminal // states exhaustion would reach, with the engine error intact. - ternDB, err := sql.Open("mysql", ternStorageDSN) + ternDB, err := sql.Open("block-mysql", ternStorageDSN) require.NoError(t, err, "open tern storage db") t.Cleanup(func() { utils.CloseAndLog(ternDB) }) require.NoError(t, ternDB.PingContext(ctx), "ping tern storage db") @@ -789,7 +789,7 @@ func TestGRPC_ServerSideTargetPlan(t *testing.T) { func TestGRPC_ServerSideDeploymentStoredOnApply(t *testing.T) { ctx := t.Context() - targetDB, err := sql.Open("mysql", targetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", targetDSN+"&multiStatements=true") require.NoError(t, err, "open target db") defer utils.CloseAndLog(targetDB) @@ -809,7 +809,7 @@ func TestGRPC_ServerSideDeploymentStoredOnApply(t *testing.T) { ternGRPCAddr, err := startTernGRPC(ctx, appDSN, ternStorageDSN) require.NoError(t, err, "start tern grpc") - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") defer utils.CloseAndLog(schemabotDB) schemabotStorage := schemabotmysql.New(schemabotDB) diff --git a/integration/hybrid_mode_test.go b/integration/hybrid_mode_test.go index 9f280a786..cb5360075 100644 --- a/integration/hybrid_mode_test.go +++ b/integration/hybrid_mode_test.go @@ -13,7 +13,7 @@ import ( "testing" "time" - mysql "github.com/go-sql-driver/mysql" + mysql "github.com/block/mysql" "github.com/block/spirit/pkg/utils" "github.com/stretchr/testify/assert" @@ -51,7 +51,7 @@ func TestHybridMode_LocalAndNamedRemoteTargets(t *testing.T) { // Create two target databases: one for local mode, one for gRPC mode // ========================================================================= - targetDB, err := sql.Open("mysql", targetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", targetDSN+"&multiStatements=true") require.NoError(t, err, "open target db") t.Cleanup(func() { utils.CloseAndLog(targetDB) }) @@ -92,7 +92,7 @@ func TestHybridMode_LocalAndNamedRemoteTargets(t *testing.T) { // Set up SchemaBot with hybrid config: databases + tern_deployments // ========================================================================= - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") clearStorageDB(t, schemabotDB) schemabotStorage := mysqlstore.New(schemabotDB) @@ -320,7 +320,7 @@ func TestHybridMode_LocalAndNamedRemoteTargets(t *testing.T) { // Test 8: Verify the actual tables exist in their respective databases // ========================================================================= - localDB, err := sql.Open("mysql", localDSN) + localDB, err := sql.Open("block-mysql", localDSN) require.NoError(t, err, "open local target db") t.Cleanup(func() { utils.CloseAndLog(localDB) }) @@ -329,7 +329,7 @@ func TestHybridMode_LocalAndNamedRemoteTargets(t *testing.T) { require.NoError(t, err, "local_items table should exist in local database") assert.Equal(t, "local_items", localTableName) - grpcTargetDB, err := sql.Open("mysql", grpcTargetDSN) + grpcTargetDB, err := sql.Open("block-mysql", grpcTargetDSN) require.NoError(t, err, "open grpc target db") t.Cleanup(func() { utils.CloseAndLog(grpcTargetDB) }) @@ -388,7 +388,7 @@ func startTernGRPCForDB(t *testing.T, appDSN, dbName string) (string, error) { logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) // Open Tern storage (reuse the shared tern storage container) - storageDB, err := sql.Open("mysql", ternStorageDSN) + storageDB, err := sql.Open("block-mysql", ternStorageDSN) if err != nil { return "", fmt.Errorf("open tern storage: %w", err) } diff --git a/integration/operator_test.go b/integration/operator_test.go index 3a985b9a6..0906d3144 100644 --- a/integration/operator_test.go +++ b/integration/operator_test.go @@ -88,7 +88,7 @@ func newOperatorClaimFixture(t *testing.T, appDBPrefix string) *operatorClaimFix t.Helper() appDBName, _ := createTestDB(t, appDBPrefix) - storageDB, err := sql.Open("mysql", schemabotDSN) + storageDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err) require.NoError(t, storageDB.PingContext(t.Context())) clearStorageDB(t, storageDB) @@ -133,7 +133,7 @@ func TestOperator_BasicClaimAndResume(t *testing.T) { ts.Service.StopOperator() // Remove the table so the second plan contains DDL that recovery can resume. - targetConn, err := sql.Open("mysql", appDSN) + targetConn, err := sql.Open("block-mysql", appDSN) require.NoError(t, err) require.NoError(t, targetConn.PingContext(ctx)) defer utils.CloseAndLog(targetConn) @@ -177,7 +177,7 @@ func TestOperator_BasicClaimAndResume(t *testing.T) { require.NoError(t, err) require.NoError(t, ts.Storage.ApplyOperations().MarkStarted(ctx, staleOpID)) - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) defer utils.CloseAndLog(schemabotDB) @@ -416,7 +416,7 @@ func TestOperator_ReconcilesClaimedOperationWhoseParentSettled(t *testing.T) { // driver that was running it is no longer presumed live. Written directly // because every storage write path refreshes updated_at, which is the // heartbeat itself. - storageDB, err := sql.Open("mysql", schemabotDSN) + storageDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") require.NoError(t, storageDB.PingContext(ctx)) t.Cleanup(func() { @@ -473,7 +473,7 @@ func TestOperator_StartResumesStoppedApplyWhoseDeploymentsNeverStarted(t *testin waitForState(t, "http://"+ts.Addr, firstApplyID, "completed", 20*time.Second) ts.Service.StopOperator() - targetConn, err := sql.Open("mysql", appDSN) + targetConn, err := sql.Open("block-mysql", appDSN) require.NoError(t, err) require.NoError(t, targetConn.PingContext(ctx)) defer utils.CloseAndLog(targetConn) @@ -615,7 +615,7 @@ func TestOperator_ProjectsApplyLeftBehindItsSettledOperations(t *testing.T) { // Age the apply's heartbeat past the lease staleness window so the crashed // drive is no longer presumed live. Written directly because every storage // write path refreshes updated_at, which is the heartbeat itself. - storageDB, err := sql.Open("mysql", schemabotDSN) + storageDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") require.NoError(t, storageDB.PingContext(ctx)) t.Cleanup(func() { @@ -787,7 +787,7 @@ func TestOperator_OperationDeploymentDrivesByOperationDeployment(t *testing.T) { waitForState(t, "http://"+ts.Addr, applyID, "completed", 20*time.Second) ts.Service.StopOperator() - targetConn, err := sql.Open("mysql", appDSN) + targetConn, err := sql.Open("block-mysql", appDSN) require.NoError(t, err) require.NoError(t, targetConn.PingContext(ctx)) defer utils.CloseAndLog(targetConn) @@ -856,7 +856,7 @@ func TestOperator_OperationDeploymentDrivesByOperationDeployment(t *testing.T) { // Age both rows so the operation-claim loop leases the operation on its // first poll. - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) defer utils.CloseAndLog(schemabotDB) @@ -967,7 +967,7 @@ func TestOperator_OperationMissingDeploymentFailsClosed(t *testing.T) { // Age both rows so the operation-claim loop leases the operation on its // first poll. - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) defer utils.CloseAndLog(schemabotDB) @@ -1068,7 +1068,7 @@ func TestOperator_OperationWithoutTasksFailsClosed(t *testing.T) { // Age both rows so the operation-claim loop leases the operation and its // parent apply on the first poll. - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) defer utils.CloseAndLog(schemabotDB) @@ -1373,7 +1373,7 @@ func TestOperator_MultipleWorkersResumeDifferentTargets(t *testing.T) { db1Name, db1DSN := createTestDB(t, "multi_worker_a_") db2Name, db2DSN := createTestDB(t, "multi_worker_b_") - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) clearStorageDB(t, schemabotDB) diff --git a/integration/resolve_apply_id_test.go b/integration/resolve_apply_id_test.go index 84722c2d8..8e6a9f182 100644 --- a/integration/resolve_apply_id_test.go +++ b/integration/resolve_apply_id_test.go @@ -44,7 +44,7 @@ func TestRemoteApplyID_ControlOperations(t *testing.T) { ternGRPCAddr := grpcAddr // 2. Create SchemaBot with real MySQL storage + GRPCClient. - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") t.Cleanup(func() { utils.CloseAndLog(schemabotDB) }) @@ -155,7 +155,7 @@ func TestRemoteApplyID_ControlOperations(t *testing.T) { // 8. Verify the table was actually created. testdbDSN := strings.Replace(targetDSN, "/target_test", "/testdb", 1) - appDB, err := sql.Open("mysql", testdbDSN) + appDB, err := sql.Open("block-mysql", testdbDSN) require.NoError(t, err) t.Cleanup(func() { utils.CloseAndLog(appDB) }) diff --git a/integration/serve_boot_retry_test.go b/integration/serve_boot_retry_test.go index 4c67148e6..78cffbd7c 100644 --- a/integration/serve_boot_retry_test.go +++ b/integration/serve_boot_retry_test.go @@ -12,7 +12,7 @@ import ( "testing" "time" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" "github.com/stretchr/testify/require" "github.com/block/schemabot/pkg/api" diff --git a/integration/setup_test.go b/integration/setup_test.go index b7281f54a..68cc543b0 100644 --- a/integration/setup_test.go +++ b/integration/setup_test.go @@ -21,8 +21,8 @@ import ( "testing" "time" + "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - "github.com/go-sql-driver/mysql" "github.com/testcontainers/testcontainers-go" "google.golang.org/grpc" "google.golang.org/grpc/connectivity" @@ -146,7 +146,7 @@ func startMySQLContainer(ctx context.Context, baseName, dbName string, schemaFS return nil, fmt.Errorf("build mysql dsn: %w", err) } - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { _ = container.Terminate(ctx) return nil, fmt.Errorf("open db for schema: %w", err) @@ -227,7 +227,7 @@ func startTernGRPC(ctx context.Context, targetDSN, storageDSN string) (grpcAddre adminCfg.MultiStatements = true // Create the target database before starting the remote Tern service. - targetDB, err := sql.Open("mysql", adminCfg.FormatDSN()) + targetDB, err := sql.Open("block-mysql", adminCfg.FormatDSN()) if err != nil { return "", fmt.Errorf("open target db: %w", err) } @@ -244,7 +244,7 @@ func startTernGRPC(ctx context.Context, targetDSN, storageDSN string) (grpcAddre clientDSN := clientCfg.FormatDSN() // Open Tern storage (separate from SchemaBot storage, simulates production architecture) - storageDB, err := sql.Open("mysql", storageDSN) + storageDB, err := sql.Open("block-mysql", storageDSN) if err != nil { return "", fmt.Errorf("open storage db: %w", err) } diff --git a/integration/status_cli_test.go b/integration/status_cli_test.go index 1784adc59..e8f409416 100644 --- a/integration/status_cli_test.go +++ b/integration/status_cli_test.go @@ -195,7 +195,7 @@ type statusApplySeed struct { func startStatusOnlySchemaBot(t *testing.T) (string, *schemabotmysql.Storage) { t.Helper() - db, err := sql.Open("mysql", schemabotDSN) + db, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") clearStorageDB(t, db) diff --git a/integration/workflow_test.go b/integration/workflow_test.go index 8cd07c3b5..835005368 100644 --- a/integration/workflow_test.go +++ b/integration/workflow_test.go @@ -44,7 +44,7 @@ type testServer struct { func createTestDB(t *testing.T, prefix string) (appDBName, appDSN string) { t.Helper() - targetDB, err := sql.Open("mysql", targetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", targetDSN+"&multiStatements=true") require.NoError(t, err, "open target db") appDBName = prefix + fmt.Sprintf("%d", time.Now().UnixNano()%10000) @@ -74,7 +74,7 @@ func startTestServerWithOperatorInterval(t *testing.T, appDBName, appDSN string, logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelInfo})) - schemabotDB, err := sql.Open("mysql", schemabotDSN) + schemabotDB, err := sql.Open("block-mysql", schemabotDSN) require.NoError(t, err, "open schemabot db") clearStorageDB(t, schemabotDB) store := mysqlstore.New(schemabotDB) @@ -287,7 +287,7 @@ func TestFullWorkflow_Spirit_PlanApplyVerify(t *testing.T) { waitForState(t, "http://"+ts.Addr, applyID, "completed", 10*time.Second) // Step 3: Verify the table exists in the target database - targetConn, err := sql.Open("mysql", appDSN) + targetConn, err := sql.Open("block-mysql", appDSN) require.NoError(t, err, "open target connection") defer func() { _ = targetConn.Close() }() @@ -357,7 +357,7 @@ func TestFullWorkflow_Spirit_DDLScenarios(t *testing.T) { ts := startTestServer(t, appDBName, appDSN) // Connect to app database for verification queries - appDB, err := sql.Open("mysql", appDSN) + appDB, err := sql.Open("block-mysql", appDSN) require.NoError(t, err, "open app db") defer func() { _ = appDB.Close() }() @@ -637,7 +637,7 @@ func TestFullWorkflow_Spirit_UnsafeChangeDetection(t *testing.T) { ts := startTestServer(t, appDBName, appDSN) // Connect to app database for setup - appDB, err := sql.Open("mysql", appDSN) + appDB, err := sql.Open("block-mysql", appDSN) require.NoError(t, err, "open app db") defer func() { _ = appDB.Close() }() @@ -808,7 +808,7 @@ func TestCLI_PlanApply(t *testing.T) { ts := startTestServer(t, appDBName, appDSN) // Connect to app database for verification - appDB, err := sql.Open("mysql", appDSN) + appDB, err := sql.Open("block-mysql", appDSN) require.NoError(t, err, "open app db") defer func() { _ = appDB.Close() }() @@ -965,7 +965,7 @@ func TestFullWorkflow_Spirit_DDLWithProgress(t *testing.T) { ts := startTestServer(t, appDBName, appDSN) // Connect to app database for seeding and verification - appDB, err := sql.Open("mysql", appDSN) + appDB, err := sql.Open("block-mysql", appDSN) require.NoError(t, err, "open app db") defer func() { _ = appDB.Close() }() @@ -1463,7 +1463,7 @@ CREATE TABLE ccc_cancelled ( assert.Equal(t, state.Task.Pending, tableStates["ccc_cancelled"], "ccc_cancelled") // Verify aaa_first table was actually created in DB (partial success committed) - targetDB, err := sql.Open("mysql", targetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", targetDSN+"&multiStatements=true") require.NoError(t, err, "open target db") defer func() { _ = targetDB.Close() }() diff --git a/pkg/api/config.go b/pkg/api/config.go index 23e15249c..b74f888f7 100644 --- a/pkg/api/config.go +++ b/pkg/api/config.go @@ -17,6 +17,7 @@ import ( "unicode" "unicode/utf8" + gomysql "github.com/block/mysql" "github.com/block/schemabot/pkg/engine" postgresengine "github.com/block/schemabot/pkg/engine/postgres" "github.com/block/schemabot/pkg/engine/spirit" @@ -27,7 +28,6 @@ import ( "github.com/block/schemabot/pkg/schema" "github.com/block/schemabot/pkg/secrets" "github.com/block/schemabot/pkg/storage" - gomysql "github.com/go-sql-driver/mysql" "gopkg.in/yaml.v3" ) diff --git a/pkg/api/config_test.go b/pkg/api/config_test.go index 40f15ba3a..72bfe4002 100644 --- a/pkg/api/config_test.go +++ b/pkg/api/config_test.go @@ -11,7 +11,7 @@ import ( "testing" "time" - gomysql "github.com/go-sql-driver/mysql" + gomysql "github.com/block/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/yaml.v3" diff --git a/pkg/api/enqueue_authorized_apply_integration_test.go b/pkg/api/enqueue_authorized_apply_integration_test.go index a8bea212a..d7be98bcb 100644 --- a/pkg/api/enqueue_authorized_apply_integration_test.go +++ b/pkg/api/enqueue_authorized_apply_integration_test.go @@ -10,8 +10,8 @@ import ( "testing" "time" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -35,7 +35,7 @@ func TestEnqueueAuthorizedApplyQueuesDurableApplyAgainstStorage(t *testing.T) { dsn := newStorageDatabaseWithSchema(t).DSN logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database") require.NoError(t, db.PingContext(ctx), "failed to ping database") t.Cleanup(func() { utils.CloseAndLog(db) }) @@ -150,7 +150,7 @@ func TestEnqueueAuthorizedApplyRecordsAuthenticatedCaller(t *testing.T) { dsn := newStorageDatabaseWithSchema(t).DSN logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database") require.NoError(t, db.PingContext(ctx), "failed to ping database") t.Cleanup(func() { utils.CloseAndLog(db) }) diff --git a/pkg/api/ensure_schema_integration_test.go b/pkg/api/ensure_schema_integration_test.go index b72255021..41da14202 100644 --- a/pkg/api/ensure_schema_integration_test.go +++ b/pkg/api/ensure_schema_integration_test.go @@ -13,8 +13,8 @@ import ( "testing" "time" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/api/mysql_shared_integration_test.go b/pkg/api/mysql_shared_integration_test.go index 8bbbdd659..ff85f5f36 100644 --- a/pkg/api/mysql_shared_integration_test.go +++ b/pkg/api/mysql_shared_integration_test.go @@ -14,8 +14,8 @@ import ( "testing" "time" + mysqldriver "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - mysqldriver "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/require" "github.com/testcontainers/testcontainers-go" "github.com/testcontainers/testcontainers-go/modules/mysql" @@ -106,7 +106,7 @@ func startSharedMySQL(ctx context.Context) (*mysql.MySQLContainer, string, error // pingMySQL opens a throwaway pool on dsn and runs the bounded readiness ping. func pingMySQL(ctx context.Context, dsn string) error { - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { return fmt.Errorf("open shared mysql: %w", err) } @@ -142,7 +142,7 @@ func newStorageDatabase(t *testing.T) storageDatabase { rootDSN := sharedMySQLDSN(t) name := fmt.Sprintf("api_test_%d", storageDatabaseSeq.Add(1)) - admin, err := sql.Open("mysql", rootDSN) + admin, err := sql.Open("block-mysql", rootDSN) require.NoError(t, err, "open shared mysql") defer utils.CloseAndLog(admin) _, err = admin.ExecContext(ctx, fmt.Sprintf("CREATE DATABASE `%s`", name)) @@ -173,7 +173,7 @@ func newStorageDatabaseWithSchema(t *testing.T) storageDatabase { // handle instead so the handle has a single owner. func openStorageDB(t *testing.T, dsn string) *sql.DB { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open storage database") t.Cleanup(func() { utils.CloseAndLog(db) }) require.NoError(t, testutil.PingMySQL(t.Context(), db), "ping storage database") diff --git a/pkg/api/operator_multi_operation_integration_test.go b/pkg/api/operator_multi_operation_integration_test.go index 811108c4b..c84a601df 100644 --- a/pkg/api/operator_multi_operation_integration_test.go +++ b/pkg/api/operator_multi_operation_integration_test.go @@ -12,7 +12,7 @@ import ( "testing" "time" - _ "github.com/go-sql-driver/mysql" + _ "github.com/block/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/api/pending_drops_cleaner_integration_test.go b/pkg/api/pending_drops_cleaner_integration_test.go index de6adbfcb..cbc8b32c2 100644 --- a/pkg/api/pending_drops_cleaner_integration_test.go +++ b/pkg/api/pending_drops_cleaner_integration_test.go @@ -10,7 +10,7 @@ import ( "testing" "time" - _ "github.com/go-sql-driver/mysql" + _ "github.com/block/mysql" "github.com/stretchr/testify/require" "github.com/block/schemabot/pkg/pendingdrops" diff --git a/pkg/api/rollback_plan_integration_test.go b/pkg/api/rollback_plan_integration_test.go index 2872537ee..2f56a0a11 100644 --- a/pkg/api/rollback_plan_integration_test.go +++ b/pkg/api/rollback_plan_integration_test.go @@ -9,8 +9,8 @@ import ( "testing" "time" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -26,7 +26,7 @@ func TestExecuteRollbackPlanForApplyUsesRequestedApplyOriginalFiles(t *testing.T dsn := newStorageDatabaseWithSchema(t).DSN logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) t.Cleanup(func() { diff --git a/pkg/api/service_integration_test.go b/pkg/api/service_integration_test.go index da8918ea6..56c214aec 100644 --- a/pkg/api/service_integration_test.go +++ b/pkg/api/service_integration_test.go @@ -8,8 +8,8 @@ import ( "os" "testing" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -32,7 +32,7 @@ func TestNew_Integration(t *testing.T) { }, } - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database") require.NoError(t, db.PingContext(ctx), "failed to ping database") @@ -47,7 +47,7 @@ func TestNew_Integration(t *testing.T) { t.Run("invalid DSN ping fails", func(t *testing.T) { // Test that connecting to an invalid MySQL server fails appropriately. // This tests the database connection logic that main.go now handles. - db, err := sql.Open("mysql", "invalid:invalid@tcp(localhost:12345)/invalid") + db, err := sql.Open("block-mysql", "invalid:invalid@tcp(localhost:12345)/invalid") if err != nil { // sql.Open may fail for malformed DSN - that's fine return diff --git a/pkg/api/telemetry_integration_test.go b/pkg/api/telemetry_integration_test.go index 271b5e83f..1629674c7 100644 --- a/pkg/api/telemetry_integration_test.go +++ b/pkg/api/telemetry_integration_test.go @@ -12,8 +12,8 @@ import ( "strings" "testing" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" @@ -31,7 +31,7 @@ func TestMetricsAfterRequests(t *testing.T) { dsn := newStorageDatabaseWithSchema(t).DSN logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) diff --git a/pkg/connreload/connreload.go b/pkg/connreload/connreload.go new file mode 100644 index 000000000..e510e4e5f --- /dev/null +++ b/pkg/connreload/connreload.go @@ -0,0 +1,289 @@ +// Package connreload provides a database/sql connector whose credentials +// survive rotation of the underlying secret. +// +// When a new physical connection is refused because the server rejected its +// credentials — the signature of a password rotated out from under a running +// pod — the connector re-resolves its DSN and retries once, so rotation is +// transparent and does not require a restart. Established connections +// authenticated before the rotation keep working; only new dials take the +// reload path. +// +// secret rotated ──► new conn ──► credentials refused +// │ +// ▼ +// reload: re-resolve DSN (re-read secret) +// │ (on error: keep current credentials) +// ▼ +// retry with fresh credentials ──► success +// +// Nothing here is engine-specific. A caller supplies two small functions — +// Resolve, which turns a raw DSN into the driver.Connector that dials it, and +// Refused, which recognizes its driver's credentials-refused error — and this +// package owns everything about *when* to reload: at most one reload in flight, +// one reload per generation of credentials however many dials failed against +// it, a cooldown so a secrets-backend outage cannot be amplified into one +// resolve per rejected dial, and a reload that runs detached so a hung secret +// resolution cannot pin a pool connection slot. +// +// It exists as one implementation because that scheduling is the subtle part +// and the part worth testing once. SchemaBot's MySQL and PostgreSQL storage +// pools resolve their DSN through the same secrets machinery, so a difference +// in how aggressively they re-resolve it would say nothing about either engine. +package connreload + +import ( + "context" + "database/sql/driver" + "fmt" + "log/slog" + "sync" + "time" +) + +// DefaultCooldown bounds how often a failing reload is retried. After a reload +// fails, further refused dials within this window surface their dial error +// without invoking Reload again, so a secrets-backend outage costs at most one +// resolve attempt per window instead of one per refused dial. The window is +// also armed when a reload succeeds but the dial retrying with the reloaded +// credentials is refused too — a backend that keeps answering with a credential +// the server rejects (stale secret sync, dropped grant, a user the rotation +// renamed) likewise costs one resolve per window, not one per connection. +const DefaultCooldown = 30 * time.Second + +// Config describes one reloadable pool. Resolve, Refused and Reload are +// required. +type Config struct { + // Resolve turns a raw DSN into the connector that dials it. It is called + // once for the DSN passed to New and once per successful Reload, never per + // dial, so it is the right place to put DSN normalization and connector + // construction however expensive they are. Returning an error rejects the + // DSN: at New time that fails the open, and on the reload path it keeps the + // pool on its current credentials. + Resolve func(dsn string) (driver.Connector, error) + + // Refused reports whether a dial error is the server rejecting the + // credentials the connection presented, as opposed to any other failure. It + // must be specific: every error it accepts costs a secret resolution, and + // every error it accepts that no rotation can fix costs one per cooldown + // window forever. It must also unwrap, because database/sql wraps. + Refused func(error) bool + + // Reload re-resolves the raw DSN, typically by re-reading a mounted secret. + // It is passed to Resolve, so it returns the DSN in the same form New was + // given. An error keeps the current credentials. + // + // It runs on its own goroutine, detached from the dial that elected it, and + // may block: a hung Reload delays the pool's credential refresh but cannot + // block healthy dials or hold a pool connection slot. A panic is recovered + // and treated as a failed reload. + Reload func() (string, error) + + // Driver is what the pool's Driver method reports. database/sql uses it + // only for driver-level feature detection, so it may be the driver's + // zero-value instance. + Driver driver.Driver + + // Name identifies the pool in log records. Optional. + Name string + + // Cooldown overrides DefaultCooldown. Non-positive means DefaultCooldown. + Cooldown time.Duration + + // now is the clock, for tests. nil means time.Now. + now func() time.Time +} + +// Connector dials with the most recently resolved credentials and refreshes +// them, at most once per failed attempt, when a dial is refused. gen counts +// credential swaps so concurrent failed dials trigger a single reload: a dial +// that failed against an already-superseded generation retries with the current +// credentials instead of reloading again. +type Connector struct { + cfg Config + + mu sync.Mutex + current driver.Connector + gen uint64 + lastReloadFail time.Time + reloading chan struct{} // non-nil while a reload for the current generation is in flight; closed when it finishes +} + +var _ driver.Connector = (*Connector)(nil) + +// New resolves dsn and returns a connector that dials it, reloading +// credentials as described on the package. A dsn Resolve rejects is returned as +// an error, so a bad DSN fails when the pool is opened rather than on first +// use. +func New(dsn string, cfg Config) (*Connector, error) { + if cfg.Resolve == nil || cfg.Refused == nil || cfg.Reload == nil { + return nil, fmt.Errorf("connreload: Resolve, Refused and Reload are required") + } + initial, err := cfg.Resolve(dsn) + if err != nil { + return nil, err + } + return &Connector{cfg: cfg, current: initial}, nil +} + +func (c *Connector) Connect(ctx context.Context) (driver.Conn, error) { + current, gen := c.snapshot() + conn, err := current.Connect(ctx) + if err == nil || !c.cfg.Refused(err) { + return conn, err + } + fresh, freshGen, ok := c.refresh(ctx, gen) + if !ok { + // Reload failed; surface the refusal that triggered it. + return nil, err + } + conn, err = fresh.Connect(ctx) + if err != nil && c.cfg.Refused(err) { + // The freshly resolved credentials are no better: the secret store + // keeps answering with a credential the server rejects. Arm the + // cooldown so subsequent refused dials back off instead of resolving + // once per connection. + c.armCooldown(freshGen) + c.log().Warn("dial with reloaded credentials was also refused; backing off further reloads", "error", err) + } + return conn, err +} + +func (c *Connector) Driver() driver.Driver { return c.cfg.Driver } + +// log returns the logger for this pool, tagged with its name when it has one. +func (c *Connector) log() *slog.Logger { + if c.cfg.Name == "" { + return slog.Default() + } + return slog.With("pool", c.cfg.Name) +} + +func (c *Connector) cooldown() time.Duration { + if c.cfg.Cooldown > 0 { + return c.cfg.Cooldown + } + return DefaultCooldown +} + +// clock returns the current time, honoring the test seam. +func (c *Connector) clock() time.Time { + if c.cfg.now != nil { + return c.cfg.now() + } + return time.Now() +} + +func (c *Connector) snapshot() (driver.Connector, uint64) { + c.mu.Lock() + defer c.mu.Unlock() + return c.current, c.gen +} + +// armCooldown starts a cooldown window as if a reload had failed, bounding +// resolve traffic when reloads succeed but the credentials they return keep +// being refused. refusedGen is the generation whose credentials were refused: +// when the connector has already advanced past it, the arm is a stale verdict +// on superseded credentials and must not suppress the newer generation's +// reloads. +func (c *Connector) armCooldown(refusedGen uint64) { + c.mu.Lock() + defer c.mu.Unlock() + if c.gen != refusedGen { + c.log().Debug("skipping cooldown arm: credentials advanced past the refused generation", + "refused_gen", refusedGen, "current_gen", c.gen) + return + } + c.lastReloadFail = c.clock() +} + +// refresh resolves fresh credentials after a dial using generation failedGen +// was refused. When another dial already swapped the credentials, the current +// ones are returned without reloading again. A reload or resolve error keeps +// the current credentials and reports false, so a transient resolve failure +// cannot wedge the pool; it also arms the cooldown so a secrets-backend outage +// is retried once per window, not once per refused dial. On success it also +// returns the generation the returned connector belongs to, so a later refusal +// of it can be attributed to the right generation. +// +// The Reload callback runs detached from every dial — outside the connector +// mutex and on its own goroutine — so a hung secret resolution can neither +// block healthy dials from snapshotting the current credentials nor pin the +// dial that elected it: database/sql counts a dial against the pool's +// connection budget before Connect runs, so a pinned dial would hold a pool +// slot for as long as the reload hangs. The reloading guard keeps it to one +// reload in flight at a time: every same-generation failure, the electing dial +// included, waits for the reload's outcome — or gives up when its own dial +// context ends, surfacing the dial error. +func (c *Connector) refresh(ctx context.Context, failedGen uint64) (driver.Connector, uint64, bool) { + for { + c.mu.Lock() + if c.gen != failedGen { + current, gen := c.current, c.gen + c.mu.Unlock() + return current, gen, true + } + if !c.lastReloadFail.IsZero() && c.clock().Sub(c.lastReloadFail) < c.cooldown() { + c.mu.Unlock() + c.log().Debug("skipping DSN reload during cooldown after a failed reload; surfacing the dial error") + return nil, 0, false + } + done := c.reloading + if done == nil { + done = make(chan struct{}) + c.reloading = done + go c.runReload(done) + } + c.mu.Unlock() + select { + case <-done: + // The reload finished; re-check the connector state to pick up the + // swapped credentials or the armed cooldown. + case <-ctx.Done(): + c.log().Debug("dial context ended while waiting for an in-flight DSN reload; surfacing the dial error") + return nil, 0, false + } + } +} + +// runReload invokes Reload and resolves its DSN outside the connector mutex, +// then publishes the outcome under it: success swaps the credentials, advances +// the generation, and clears the cooldown; failure arms the cooldown. It runs +// on its own goroutine, detached from the dial that elected it, so waiters +// observe the outcome through the connector state rather than a return value. +// The publish runs in a defer so the reloading guard is released and waiters +// are unblocked even if Reload panics; the panic is recovered and treated as a +// failed reload — a detached goroutine has no caller to propagate it to, and a +// panicking secret resolver must leave the pool on its current credentials, +// not crash the process. +func (c *Connector) runReload(done chan struct{}) { + var fresh driver.Connector + defer func() { + if r := recover(); r != nil { + c.log().Error("DSN reload after a refused dial panicked; keeping current credentials", "panic", r) + } + c.mu.Lock() + defer c.mu.Unlock() + c.reloading = nil + close(done) + if fresh == nil { + c.lastReloadFail = c.clock() + return + } + c.current = fresh + c.gen++ + c.lastReloadFail = time.Time{} + c.log().Info("reloaded credentials after a refused dial") + }() + + dsn, err := c.cfg.Reload() + if err != nil { + c.log().Error("DSN reload after a refused dial failed; keeping current credentials", "error", err) + return + } + resolved, err := c.cfg.Resolve(dsn) + if err != nil { + c.log().Error("resolving the reloaded DSN failed; keeping current credentials", "error", err) + return + } + fresh = resolved +} diff --git a/pkg/connreload/connreload_test.go b/pkg/connreload/connreload_test.go new file mode 100644 index 000000000..13783ffb7 --- /dev/null +++ b/pkg/connreload/connreload_test.go @@ -0,0 +1,689 @@ +package connreload + +import ( + "context" + "database/sql/driver" + "errors" + "fmt" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The fakes below stand in for a driver. Nothing in this package knows what a +// DSN means, so a test driver can be as simple as "the DSN is the credential": +// every dial records the DSN it was resolved from, and errRefused is the only +// error the connector is told to treat as a credentials refusal. + +var errRefused = errors.New("credentials refused") + +func refused(err error) bool { return errors.Is(err, errRefused) } + +// stubConn is the minimal driver.Conn a fake dial can hand back. +type stubConn struct{} + +func (stubConn) Prepare(string) (driver.Stmt, error) { return nil, errors.New("not implemented") } +func (stubConn) Close() error { return nil } +func (stubConn) Begin() (driver.Tx, error) { return nil, errors.New("not implemented") } + +type stubDriver struct{} + +func (stubDriver) Open(string) (driver.Conn, error) { return nil, errors.New("not implemented") } + +// fakeConnector dials for one resolved DSN, recording each attempt into the +// shared slice and taking its outcome from results, consumed in order across +// every generation. +type fakeConnector struct { + t *testing.T + dsn string + dials *[]string + results []error + mu *sync.Mutex +} + +func (f *fakeConnector) Connect(context.Context) (driver.Conn, error) { + f.mu.Lock() + *f.dials = append(*f.dials, f.dsn) + i := len(*f.dials) - 1 + f.mu.Unlock() + // assert (not require): this runs on the dialing goroutine, and testify's + // FailNow is only valid on the test goroutine. The error return fails the + // dial cleanly instead. + if !assert.Less(f.t, i, len(f.results), "unexpected extra dial attempt") { + return nil, errors.New("unexpected extra dial attempt") + } + if err := f.results[i]; err != nil { + return nil, err + } + return stubConn{}, nil +} + +func (f *fakeConnector) Driver() driver.Driver { return stubDriver{} } + +// dialRecorder builds a Resolve that hands out fakeConnectors sharing one +// attempt log, and returns the log. +func dialRecorder(t *testing.T, results []error) (func(string) (driver.Connector, error), *[]string) { + t.Helper() + var dials []string + var mu sync.Mutex + resolve := func(dsn string) (driver.Connector, error) { + return &fakeConnector{t: t, dsn: dsn, dials: &dials, results: results, mu: &mu}, nil + } + return resolve, &dials +} + +// newTestConnector builds a Connector over the fake driver. results are the +// dial outcomes in order; reload supplies the rotated DSN. +func newTestConnector(t *testing.T, dsn string, results []error, reload func() (string, error)) (*Connector, *[]string) { + t.Helper() + resolve, dials := dialRecorder(t, results) + c, err := New(dsn, Config{ + Resolve: resolve, + Refused: refused, + Reload: reload, + Driver: stubDriver{}, + Name: "test", + }) + require.NoError(t, err) + return c, dials +} + +func TestNewRequiresCallbacks(t *testing.T) { + resolve := func(string) (driver.Connector, error) { return nil, nil } + reload := func() (string, error) { return "", nil } + + _, err := New("dsn", Config{Refused: refused, Reload: reload}) + require.Error(t, err, "a nil Resolve must not produce a connector that panics on first dial") + _, err = New("dsn", Config{Resolve: resolve, Reload: reload}) + require.Error(t, err) + _, err = New("dsn", Config{Resolve: resolve, Refused: refused}) + require.Error(t, err) +} + +func TestNewRejectsUnresolvableDSN(t *testing.T) { + resolveErr := errors.New("malformed DSN") + _, err := New("bad", Config{ + Resolve: func(string) (driver.Connector, error) { return nil, resolveErr }, + Refused: refused, + Reload: func() (string, error) { return "", nil }, + }) + require.ErrorIs(t, err, resolveErr, "a bad boot DSN must fail the open, not the first dial") +} + +func TestReloadsOnRefusal(t *testing.T) { + var reloads atomic.Int32 + c, dials := newTestConnector(t, "old", []error{errRefused, nil, nil}, func() (string, error) { + reloads.Add(1) + return "rotated", nil + }) + + conn, err := c.Connect(t.Context()) + require.NoError(t, err) + require.NotNil(t, conn) + assert.Equal(t, []string{"old", "rotated"}, *dials, "retry must dial with the reloaded credentials") + assert.Equal(t, int32(1), reloads.Load()) + + // The reloaded credentials stick for subsequent dials without another reload. + conn, err = c.Connect(t.Context()) + require.NoError(t, err) + require.NotNil(t, conn) + assert.Equal(t, []string{"old", "rotated", "rotated"}, *dials) + assert.Equal(t, int32(1), reloads.Load()) +} + +func TestKeepsCredentialsWhenReloadFails(t *testing.T) { + c, dials := newTestConnector(t, "old", []error{errRefused}, func() (string, error) { + return "", errors.New("secret backend unavailable") + }) + + conn, err := c.Connect(t.Context()) + require.ErrorIs(t, err, errRefused, "the original refusal surfaces, not the reload error") + assert.Nil(t, conn) + assert.Equal(t, []string{"old"}, *dials, "no retry without fresh credentials") + + _, gen := c.snapshot() + assert.Equal(t, uint64(0), gen, "current credentials are kept") +} + +func TestIgnoresNonRefusalErrors(t *testing.T) { + dialErr := errors.New("connection refused") + reloadCalled := false + c, dials := newTestConnector(t, "old", []error{dialErr}, func() (string, error) { + reloadCalled = true + return "rotated", nil + }) + + _, err := c.Connect(t.Context()) + require.ErrorIs(t, err, dialErr) + assert.False(t, reloadCalled, "only a credentials refusal may trigger a reload") + assert.Equal(t, []string{"old"}, *dials) +} + +func TestSurfacesRetryFailure(t *testing.T) { + var reloads atomic.Int32 + c, dials := newTestConnector(t, "old", []error{errRefused, errRefused}, func() (string, error) { + reloads.Add(1) + return "rotated", nil + }) + + // The reload succeeds but the retry dial is also refused — for example a + // reloaded secret that is itself stale. The retry's error surfaces and the + // reload runs exactly once for the failed attempt. + conn, err := c.Connect(t.Context()) + require.ErrorIs(t, err, errRefused) + assert.Nil(t, conn) + assert.Equal(t, []string{"old", "rotated"}, *dials, "the retry dials with the reloaded credentials") + assert.Equal(t, int32(1), reloads.Load()) +} + +// A reloaded DSN that Resolve rejects keeps the pool on the working +// generation, rather than swapping in credentials nothing can dial with. +func TestRejectsUnresolvableReloadedDSN(t *testing.T) { + var dials []string + var mu sync.Mutex + resolveErr := errors.New("malformed DSN") + c, err := New("old", Config{ + Resolve: func(dsn string) (driver.Connector, error) { + if dsn != "old" { + return nil, resolveErr + } + return &fakeConnector{t: t, dsn: dsn, dials: &dials, results: []error{nil}, mu: &mu}, nil + }, + Refused: refused, + Reload: func() (string, error) { return "rotated", nil }, + }) + require.NoError(t, err) + + fresh, _, ok := c.refresh(t.Context(), 0) + require.False(t, ok) + assert.Nil(t, fresh) + + _, gen := c.snapshot() + assert.Equal(t, uint64(0), gen, "the pool stays on the working generation") +} + +func TestRefreshConcurrent(t *testing.T) { + var reloads atomic.Int32 + c, _ := newTestConnector(t, "old", nil, func() (string, error) { + reloads.Add(1) + return "rotated", nil + }) + + // Concurrent dials that failed on the same generation trigger exactly one + // reload; the rest reuse the swapped credentials. + var wg sync.WaitGroup + for range 8 { + wg.Go(func() { + _, gen, ok := c.refresh(t.Context(), 0) + assert.True(t, ok) + assert.Equal(t, uint64(1), gen) + }) + } + wg.Wait() + assert.Equal(t, int32(1), reloads.Load(), "concurrent same-generation failures must reload once") +} + +func TestRefreshDedupesStaleGeneration(t *testing.T) { + var reloads atomic.Int32 + c, _ := newTestConnector(t, "old", nil, func() (string, error) { + reloads.Add(1) + return "rotated", nil + }) + + _, gen, ok := c.refresh(t.Context(), 0) + require.True(t, ok) + require.Equal(t, uint64(1), gen) + require.Equal(t, int32(1), reloads.Load()) + + // A dial that failed against the already-superseded generation reuses the + // swapped credentials instead of reloading again. + _, gen, ok = c.refresh(t.Context(), 0) + require.True(t, ok) + assert.Equal(t, uint64(1), gen) + assert.Equal(t, int32(1), reloads.Load(), "stale-generation refresh must not reload") +} + +// A reload that succeeds but returns credentials the server still refuses arms +// the cooldown too: without it, each refused dial advances the generation and +// triggers a fresh resolve — one per new connection — for as long as the secret +// store keeps answering with a refused credential. +func TestArmsCooldownWhenReloadedCredentialsRefused(t *testing.T) { + var reloads atomic.Int32 + c, dials := newTestConnector(t, "old", + []error{errRefused, errRefused, errRefused, errRefused, errRefused}, + func() (string, error) { + reloads.Add(1) + return "stale", nil + }) + clock := time.Now() + c.cfg.now = func() time.Time { return clock } + + // The dial fails, the reload succeeds, and the retry is refused too: the + // cooldown arms. + _, err := c.Connect(t.Context()) + require.Error(t, err) + require.Equal(t, int32(1), reloads.Load()) + assert.Equal(t, []string{"old", "stale"}, *dials) + + // The next refused dial surfaces without another resolve. + _, err = c.Connect(t.Context()) + require.Error(t, err) + assert.Equal(t, int32(1), reloads.Load(), "a refused reloaded credential must not cost one resolve per connection") + assert.Equal(t, []string{"old", "stale", "stale"}, *dials) + + // After the window elapses the reload is retried; another refused retry + // re-arms the cooldown. + clock = clock.Add(DefaultCooldown) + _, err = c.Connect(t.Context()) + require.Error(t, err) + assert.Equal(t, int32(2), reloads.Load()) + assert.Equal(t, []string{"old", "stale", "stale", "stale", "stale"}, *dials) +} + +func TestReloadCooldown(t *testing.T) { + var reloads atomic.Int32 + c, _ := newTestConnector(t, "old", nil, func() (string, error) { + reloads.Add(1) + if reloads.Load() < 3 { + return "", errors.New("secret backend unavailable") + } + return "rotated", nil + }) + clock := time.Now() + c.cfg.now = func() time.Time { return clock } + + // The first failed reload arms the cooldown. + _, _, ok := c.refresh(t.Context(), 0) + require.False(t, ok) + require.Equal(t, int32(1), reloads.Load()) + + // Failed dials inside the window surface without reloading again. + _, _, ok = c.refresh(t.Context(), 0) + require.False(t, ok) + assert.Equal(t, int32(1), reloads.Load(), "reload must not run during the cooldown") + + // After the window elapses the reload is retried; another failure re-arms. + clock = clock.Add(DefaultCooldown) + _, _, ok = c.refresh(t.Context(), 0) + require.False(t, ok) + require.Equal(t, int32(2), reloads.Load()) + + // A successful reload swaps credentials and clears the cooldown. + clock = clock.Add(DefaultCooldown) + _, _, ok = c.refresh(t.Context(), 0) + require.True(t, ok) + require.Equal(t, int32(3), reloads.Load()) + assert.True(t, c.lastReloadFail.IsZero(), "a successful reload must clear the cooldown") +} + +func TestCooldownOverride(t *testing.T) { + var reloads atomic.Int32 + resolve, _ := dialRecorder(t, nil) + c, err := New("old", Config{ + Resolve: resolve, + Refused: refused, + Reload: func() (string, error) { reloads.Add(1); return "", errors.New("unavailable") }, + Cooldown: time.Hour, + }) + require.NoError(t, err) + clock := time.Now() + c.cfg.now = func() time.Time { return clock } + + _, _, ok := c.refresh(t.Context(), 0) + require.False(t, ok) + require.Equal(t, int32(1), reloads.Load()) + + // The default window would have elapsed; the override's has not. + clock = clock.Add(2 * DefaultCooldown) + _, _, ok = c.refresh(t.Context(), 0) + require.False(t, ok) + assert.Equal(t, int32(1), reloads.Load(), "Cooldown must override DefaultCooldown") +} + +// waitClosed fails the test when ch does not close within a bounded deadline. +func waitClosed(t *testing.T, ch <-chan struct{}, msg string) { + t.Helper() + select { + case <-ch: + case <-time.After(5 * time.Second): + t.Fatal(msg) + } +} + +// hungReload returns a Reload that signals started, then blocks until release +// closes before returning result and err. +func hungReload(started, release chan struct{}, reloads *atomic.Int32, result string, err error) func() (string, error) { + return func() (string, error) { + reloads.Add(1) + close(started) + <-release + return result, err + } +} + +func TestSnapshotNotBlockedByHungReload(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + var reloads atomic.Int32 + c, _ := newTestConnector(t, "old", nil, hungReload(started, release, &reloads, "rotated", nil)) + + var wg sync.WaitGroup + wg.Go(func() { + _, gen, ok := c.refresh(t.Context(), 0) + assert.True(t, ok) + assert.Equal(t, uint64(1), gen) + }) + waitClosed(t, started, "reload never started") + + // The hung reload must not hold the connector mutex: healthy dials keep + // snapshotting the current credentials while a new one resolves. + snapshotDone := make(chan struct{}) + go func() { + defer close(snapshotDone) + _, gen := c.snapshot() + assert.Equal(t, uint64(0), gen) + }() + waitClosed(t, snapshotDone, "snapshot blocked behind an in-flight reload") + + close(release) + wg.Wait() + assert.Equal(t, int32(1), reloads.Load()) +} + +func TestConnectNotBlockedByHungReload(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + var reloads atomic.Int32 + c, dials := newTestConnector(t, "old", []error{errRefused, nil, nil}, + hungReload(started, release, &reloads, "rotated", nil)) + + var wg sync.WaitGroup + wg.Go(func() { + conn, err := c.Connect(t.Context()) + assert.NoError(t, err) + assert.NotNil(t, conn) + }) + waitClosed(t, started, "reload never started") + + // A dial that authenticates with the current credentials completes while + // the refused dial's reload hangs on secret resolution. + connected := make(chan struct{}) + go func() { + defer close(connected) + conn, err := c.Connect(t.Context()) + assert.NoError(t, err) + assert.NotNil(t, conn) + }() + waitClosed(t, connected, "healthy dial blocked behind an in-flight reload") + + close(release) + wg.Wait() + assert.Equal(t, []string{"old", "old", "rotated"}, *dials) + assert.Equal(t, int32(1), reloads.Load()) +} + +func TestRefreshWaiterRespectsDialContext(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + var reloads atomic.Int32 + c, _ := newTestConnector(t, "old", nil, hungReload(started, release, &reloads, "rotated", nil)) + + var wg sync.WaitGroup + wg.Go(func() { + _, _, ok := c.refresh(t.Context(), 0) + assert.True(t, ok) + }) + waitClosed(t, started, "reload never started") + + // A waiter whose dial context ends while the leader's reload hangs gives up + // and surfaces its dial error instead of blocking indefinitely. + ctx, cancel := context.WithCancel(t.Context()) + waiterDone := make(chan struct{}) + go func() { + defer close(waiterDone) + fresh, _, ok := c.refresh(ctx, 0) + assert.False(t, ok) + assert.Nil(t, fresh) + }() + cancel() + waitClosed(t, waiterDone, "waiter did not honor its dial context") + + close(release) + wg.Wait() + assert.Equal(t, int32(1), reloads.Load(), "the canceled waiter must not trigger its own reload") +} + +// The dial that elects a reload is not pinned by it: the reload runs detached, +// so cancelling the electing dial's context returns it promptly with its dial +// error — it cannot hold a pool connection slot for as long as a hung secret +// resolution takes — while the reload finishes in the background and publishes +// the rotated credentials for later dials. +func TestRefreshElectingDialRespectsDialContext(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + var reloads atomic.Int32 + c, _ := newTestConnector(t, "old", nil, hungReload(started, release, &reloads, "rotated", nil)) + + ctx, cancel := context.WithCancel(t.Context()) + electorDone := make(chan struct{}) + go func() { + defer close(electorDone) + fresh, _, ok := c.refresh(ctx, 0) + assert.False(t, ok) + assert.Nil(t, fresh) + }() + waitClosed(t, started, "reload never started") + cancel() + waitClosed(t, electorDone, "the electing dial did not honor its own dial context") + + // The detached reload finishes and publishes: a later same-generation + // refresh picks up the rotated credentials without reloading again. + close(release) + _, gen, ok := c.refresh(t.Context(), 0) + require.True(t, ok) + assert.Equal(t, uint64(1), gen) + assert.Equal(t, int32(1), reloads.Load(), "the abandoned reload's outcome must be reused, not re-resolved") +} + +func TestRefreshWaiterObservesLeaderFailure(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + var reloads atomic.Int32 + c, _ := newTestConnector(t, "old", nil, + hungReload(started, release, &reloads, "", errors.New("secret backend unavailable"))) + + // The clock seam doubles as an entered-refresh gate: an expired cooldown + // stamp makes every refresh iteration consult the clock under the mutex, so + // the second consult is the waiter's. Once past it, the waiter can only + // reach the select on the leader's done channel — releasing the leader + // after that pins the wake-then-recheck path instead of leaving it to the + // scheduler. + c.lastReloadFail = time.Now().Add(-2 * DefaultCooldown) + var clockCalls atomic.Int32 + waiterEntered := make(chan struct{}) + c.cfg.now = func() time.Time { + if clockCalls.Add(1) == 2 { + close(waiterEntered) + } + return time.Now() + } + + var wg sync.WaitGroup + wg.Go(func() { + _, _, ok := c.refresh(t.Context(), 0) + assert.False(t, ok) + }) + waitClosed(t, started, "reload never started") + + // A same-generation waiter observes the leader's failed reload through the + // armed cooldown and reports failure without reloading again. + waiterDone := make(chan struct{}) + go func() { + defer close(waiterDone) + fresh, _, ok := c.refresh(t.Context(), 0) + assert.False(t, ok) + assert.Nil(t, fresh) + }() + waitClosed(t, waiterEntered, "waiter never entered refresh") + close(release) + wg.Wait() + waitClosed(t, waiterDone, "waiter did not observe the leader's failed reload") + assert.Equal(t, int32(1), reloads.Load(), "the waiter must not reload during the cooldown the failure armed") +} + +// A Reload that panics must not wedge the connector or crash the process: the +// recover in the publish defer converts the panic into a failed reload — the +// reloading guard is released, waiters are unblocked, the cooldown is armed, +// and the current credentials are kept. +func TestReloadPanicUnblocksWaiters(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + var reloads atomic.Int32 + c, _ := newTestConnector(t, "old", nil, func() (string, error) { + reloads.Add(1) + close(started) + <-release + panic("secret backend panicked") + }) + + var wg sync.WaitGroup + wg.Go(func() { + fresh, _, ok := c.refresh(t.Context(), 0) + assert.False(t, ok, "a panicking reload must surface as a failed reload") + assert.Nil(t, fresh) + }) + waitClosed(t, started, "reload never started") + + // A same-generation waiter blocked on the reload's outcome is unblocked by + // the publish defer and observes the armed cooldown. + waiterDone := make(chan struct{}) + go func() { + defer close(waiterDone) + fresh, _, ok := c.refresh(t.Context(), 0) + assert.False(t, ok) + assert.Nil(t, fresh) + }() + close(release) + wg.Wait() + waitClosed(t, waiterDone, "waiter was not unblocked by the panicking reload") + + // The guard is released and the cooldown armed: a later same-generation + // refresh backs off without reloading rather than deadlocking on a stale + // guard. + _, _, ok := c.refresh(t.Context(), 0) + assert.False(t, ok) + assert.Equal(t, int32(1), reloads.Load(), "the panicked reload must arm the cooldown; no further reloads") + c.mu.Lock() + defer c.mu.Unlock() + assert.Nil(t, c.reloading, "the reloading guard must be released after a panic") +} + +// A refusal verdict on superseded credentials must not arm the cooldown: while +// a retry dial is in flight, another rotation can swap in newer credentials, +// and stamping the late refusal onto that newer generation would suppress its +// legitimate reloads for a full window. +func TestStaleRefusalDoesNotArmCooldown(t *testing.T) { + var c *Connector + var dials atomic.Int32 + resolve := func(string) (driver.Connector, error) { + return connectFunc(func(context.Context) (driver.Conn, error) { + switch dials.Add(1) { + case 1: + // The initial dial is refused, triggering the reload. + return nil, errRefused + case 2: + // While the retry with reloaded credentials is in flight, a + // concurrent rotation advances the generation; the retry then + // comes back refused — a verdict on already-superseded + // credentials. + c.mu.Lock() + c.gen++ + c.mu.Unlock() + return nil, errRefused + default: + return stubConn{}, nil + } + }), nil + } + var err error + c, err = New("old", Config{ + Resolve: resolve, + Refused: refused, + Reload: func() (string, error) { return "rotated", nil }, + }) + require.NoError(t, err) + + _, err = c.Connect(t.Context()) + require.Error(t, err) + c.mu.Lock() + defer c.mu.Unlock() + assert.True(t, c.lastReloadFail.IsZero(), "a stale refusal must not arm the cooldown against the newer generation") +} + +// connectFunc adapts a dial function to driver.Connector. +type connectFunc func(context.Context) (driver.Conn, error) + +func (f connectFunc) Connect(ctx context.Context) (driver.Conn, error) { return f(ctx) } +func (connectFunc) Driver() driver.Driver { return stubDriver{} } + +func TestDriverIsReported(t *testing.T) { + c, _ := newTestConnector(t, "old", nil, func() (string, error) { return "", nil }) + assert.Equal(t, stubDriver{}, c.Driver()) +} + +// Two pools must not share reload state. The driver this package replaced +// registered its reload callback process-globally, so opening a second +// reloadable pool silently repointed the first one's credential reload at the +// second one's secret. +func TestPoolsDoNotShareReloadState(t *testing.T) { + var first, second atomic.Int32 + firstConn, _ := newTestConnector(t, "first-old", []error{errRefused, nil}, func() (string, error) { + first.Add(1) + return "first-new", nil + }) + secondConn, _ := newTestConnector(t, "second-old", []error{errRefused, nil}, func() (string, error) { + second.Add(1) + return "second-new", nil + }) + + _, err := firstConn.Connect(t.Context()) + require.NoError(t, err) + assert.Equal(t, int32(1), first.Load()) + assert.Equal(t, int32(0), second.Load(), "one pool's refusal must not reload another pool's secret") + + _, err = secondConn.Connect(t.Context()) + require.NoError(t, err) + assert.Equal(t, int32(1), first.Load()) + assert.Equal(t, int32(1), second.Load()) +} + +func TestUnnamedPoolLogsWithoutPanicking(t *testing.T) { + // Name is optional; the logger path must handle both. + resolve, _ := dialRecorder(t, nil) + c, err := New("old", Config{ + Resolve: resolve, + Refused: refused, + Reload: func() (string, error) { return "", errors.New("unavailable") }, + }) + require.NoError(t, err) + require.NotNil(t, c.log()) + + _, _, ok := c.refresh(t.Context(), 0) + assert.False(t, ok) +} + +func TestConnectSurfacesResolveErrorShape(t *testing.T) { + // Sanity: a wrapped refusal is still a refusal, because Refused unwraps. + c, dials := newTestConnector(t, "old", + []error{fmt.Errorf("dial tcp: %w", errRefused), nil}, + func() (string, error) { return "rotated", nil }) + + conn, err := c.Connect(t.Context()) + require.NoError(t, err) + require.NotNil(t, conn) + assert.Equal(t, []string{"old", "rotated"}, *dials) +} diff --git a/pkg/engine/planetscale/apply.go b/pkg/engine/planetscale/apply.go index e76b9bf39..0b684921d 100644 --- a/pkg/engine/planetscale/apply.go +++ b/pkg/engine/planetscale/apply.go @@ -10,7 +10,7 @@ import ( "sync/atomic" "time" - mysql "github.com/go-sql-driver/mysql" + mysql "github.com/block/mysql" ps "github.com/planetscale/planetscale-go/planetscale" "github.com/block/spirit/pkg/table" @@ -600,7 +600,7 @@ func (e *Engine) applyKeyspaceChangesOnce(ctx context.Context, sc engine.SchemaC } dsn := mysqlCfg.FormatDSN() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { return fmt.Errorf("open branch connection for %s: %w", sc.Namespace, err) } diff --git a/pkg/engine/planetscale/branch.go b/pkg/engine/planetscale/branch.go index e843b9cd8..eaaa51283 100644 --- a/pkg/engine/planetscale/branch.go +++ b/pkg/engine/planetscale/branch.go @@ -9,7 +9,7 @@ import ( "sync" "time" - mysql "github.com/go-sql-driver/mysql" + mysql "github.com/block/mysql" ps "github.com/planetscale/planetscale-go/planetscale" "github.com/block/spirit/pkg/statement" @@ -167,7 +167,7 @@ func (e *Engine) fetchBranchSchemaViaMySQL(ctx context.Context, password *ps.Dat g.Go(func() error { ksCfg := mysqlCfg.Clone() ksCfg.DBName = ks - db, err := sql.Open("mysql", ksCfg.FormatDSN()) + db, err := sql.Open("block-mysql", ksCfg.FormatDSN()) if err != nil { return fmt.Errorf("open branch MySQL for keyspace %s: %w", ks, err) } diff --git a/pkg/engine/planetscale/planetscale.go b/pkg/engine/planetscale/planetscale.go index edd8a2800..848b3c671 100644 --- a/pkg/engine/planetscale/planetscale.go +++ b/pkg/engine/planetscale/planetscale.go @@ -387,7 +387,7 @@ import ( "sync" "time" - mysql "github.com/go-sql-driver/mysql" + mysql "github.com/block/mysql" ps "github.com/planetscale/planetscale-go/planetscale" "github.com/block/spirit/pkg/utils" @@ -433,7 +433,7 @@ var ( var deployState = state.DeployRequest // formatDeployRequestError builds a detailed error message for a failed deploy request, -// including any lint errors from PlanetScale's validation. +// including any lint errors from PlanetScale's validation. sadscan:disable kingfisher.planetscale.2 func formatDeployRequestError(dr *ps.DeployRequest) string { var b strings.Builder fmt.Fprintf(&b, "deploy request #%d failed during preparation (state: %s)", dr.Number, dr.DeploymentState) @@ -666,7 +666,7 @@ func (e *Engine) getVtgateDB(ctx context.Context, dsn string) (*sql.DB, error) { return db, nil } - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { return nil, fmt.Errorf("open vtgate: %w", err) } diff --git a/pkg/engine/planetscale/planetscale_test.go b/pkg/engine/planetscale/planetscale_test.go index 454efef11..291406f65 100644 --- a/pkg/engine/planetscale/planetscale_test.go +++ b/pkg/engine/planetscale/planetscale_test.go @@ -6,7 +6,7 @@ import ( "testing" "time" - mysql "github.com/go-sql-driver/mysql" + mysql "github.com/block/mysql" ps "github.com/planetscale/planetscale-go/planetscale" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/engine/planetscale/tls.go b/pkg/engine/planetscale/tls.go index 576a09934..28a7dae35 100644 --- a/pkg/engine/planetscale/tls.go +++ b/pkg/engine/planetscale/tls.go @@ -7,7 +7,7 @@ import ( "os" "sync/atomic" - mysql "github.com/go-sql-driver/mysql" + mysql "github.com/block/mysql" ) // mtlsConfigName is the Go MySQL driver TLS config name registered by RegisterMTLS. diff --git a/pkg/engine/spirit/control.go b/pkg/engine/spirit/control.go index aa466f9f3..7c81ce273 100644 --- a/pkg/engine/spirit/control.go +++ b/pkg/engine/spirit/control.go @@ -12,7 +12,7 @@ import ( "fmt" "strings" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" "github.com/block/spirit/pkg/utils" diff --git a/pkg/engine/spirit/direct.go b/pkg/engine/spirit/direct.go index 8fb3aab06..89405abaa 100644 --- a/pkg/engine/spirit/direct.go +++ b/pkg/engine/spirit/direct.go @@ -15,15 +15,16 @@ import ( "strconv" "time" + "github.com/block/mysql" "github.com/block/spirit/pkg/dbconn/sqlescape" "github.com/block/spirit/pkg/migration/check" "github.com/block/spirit/pkg/statement" "github.com/block/spirit/pkg/utils" - "github.com/go-sql-driver/mysql" "github.com/block/schemabot/pkg/engine" "github.com/block/schemabot/pkg/metrics" "github.com/block/schemabot/pkg/mysqlconn" + "github.com/block/schemabot/pkg/mysqlerr" "github.com/block/schemabot/pkg/ui" ) @@ -268,8 +269,10 @@ const erLockWaitTimeout = 1205 // the session's bounded lock_wait_timeout expired while the statement queued // behind existing lock holders. func isLockWaitTimeout(err error) bool { - var mysqlErr *mysql.MySQLError - return errors.As(err, &mysqlErr) && mysqlErr.Number == erLockWaitTimeout + // Read through mysqlerr rather than asserting a driver type: two MySQL + // drivers are linked and their error types are not interchangeable. See + // pkg/mysqlerr/number.go. + return mysqlerr.Is(err, erLockWaitTimeout) } // directStatementProgress tracks one direct-routed statement's lifecycle for diff --git a/pkg/engine/spirit/existing_copy.go b/pkg/engine/spirit/existing_copy.go index 511da6574..586507ca3 100644 --- a/pkg/engine/spirit/existing_copy.go +++ b/pkg/engine/spirit/existing_copy.go @@ -24,9 +24,9 @@ import ( "strings" "time" + "github.com/block/mysql" "github.com/block/spirit/pkg/checkpoint" "github.com/block/spirit/pkg/utils" - "github.com/go-sql-driver/mysql" "github.com/block/schemabot/pkg/ddl" "github.com/block/schemabot/pkg/engine" diff --git a/pkg/engine/spirit/failure_reason_test.go b/pkg/engine/spirit/failure_reason_test.go index a629c1b17..7229fc8a8 100644 --- a/pkg/engine/spirit/failure_reason_test.go +++ b/pkg/engine/spirit/failure_reason_test.go @@ -7,7 +7,7 @@ import ( "net" "testing" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" "github.com/stretchr/testify/assert" "github.com/block/schemabot/pkg/engine" diff --git a/pkg/engine/spirit/helpers.go b/pkg/engine/spirit/helpers.go index 09bd86d16..bcbdb5e02 100644 --- a/pkg/engine/spirit/helpers.go +++ b/pkg/engine/spirit/helpers.go @@ -6,8 +6,8 @@ import ( "sort" "strings" + "github.com/block/mysql" "github.com/block/spirit/pkg/statement" - "github.com/go-sql-driver/mysql" "github.com/block/schemabot/pkg/schema" ) diff --git a/pkg/engine/spirit/spirit_integration_test.go b/pkg/engine/spirit/spirit_integration_test.go index 36e5ccf3e..a566b94ab 100644 --- a/pkg/engine/spirit/spirit_integration_test.go +++ b/pkg/engine/spirit/spirit_integration_test.go @@ -29,7 +29,7 @@ import ( "github.com/block/schemabot/pkg/schema" "github.com/block/schemabot/pkg/testutil" - drivermysql "github.com/go-sql-driver/mysql" + drivermysql "github.com/block/mysql" ) // Shared test infrastructure @@ -117,7 +117,7 @@ func tableSchemaNames(schemas []table.TableSchema) []string { func setupTestMySQL(t *testing.T) (string, *sql.DB) { t.Helper() - db, err := sql.Open("mysql", sharedDSN) + db, err := sql.Open("block-mysql", sharedDSN) require.NoError(t, err, "connect to mysql") t.Cleanup(func() { utils.CloseAndLog(db) }) @@ -2200,7 +2200,7 @@ func containsHelper(s, substr string) bool { // and report the deferred-cutover signal as absent. func TestEngine_StatelessControlAddressesDSNSchema(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", sharedDSN) + db, err := sql.Open("block-mysql", sharedDSN) require.NoError(t, err, "open database") defer utils.CloseAndLog(db) @@ -2214,7 +2214,7 @@ func TestEngine_StatelessControlAddressesDSNSchema(t *testing.T) { t.Cleanup(func() { cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(t.Context()), 30*time.Second) defer cancel() - cleanupDB, cleanupErr := sql.Open("mysql", sharedDSN) + cleanupDB, cleanupErr := sql.Open("block-mysql", sharedDSN) require.NoError(t, cleanupErr, "open database for stateless control cleanup") defer utils.CloseAndLog(cleanupDB) _, cleanupErr = cleanupDB.ExecContext(cleanupCtx, "DROP DATABASE IF EXISTS `"+physicalSchema+"`") diff --git a/pkg/etre/resolver_test.go b/pkg/etre/resolver_test.go index 6f1cdaad0..1405734c9 100644 --- a/pkg/etre/resolver_test.go +++ b/pkg/etre/resolver_test.go @@ -4,7 +4,7 @@ import ( "context" "testing" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" "github.com/square/etre" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/inventory/connection_assembler.go b/pkg/inventory/connection_assembler.go index 8ea295b5f..e7c47d73e 100644 --- a/pkg/inventory/connection_assembler.go +++ b/pkg/inventory/connection_assembler.go @@ -8,8 +8,8 @@ import ( "net/url" "strings" + "github.com/block/mysql" "github.com/block/spirit/pkg/dbconn" - "github.com/go-sql-driver/mysql" ) // ConnectionAssembler turns a resolved endpoint and credentials into the @@ -302,7 +302,7 @@ const ( // identifier is a routing and display key, while this name is what every // PlanetScale API call must address. MetadataDatabase = "database" - // MetadataTokenName is the PlanetScale service token id. + // MetadataTokenName is the PlanetScale service token id. sadscan:disable kingfisher.planetscale.2 MetadataTokenName = "token_name" // MetadataTokenValue is the PlanetScale service token secret. MetadataTokenValue = "token_value" diff --git a/pkg/inventory/connection_assembler_test.go b/pkg/inventory/connection_assembler_test.go index 44468c1f6..04266a650 100644 --- a/pkg/inventory/connection_assembler_test.go +++ b/pkg/inventory/connection_assembler_test.go @@ -4,7 +4,7 @@ import ( "net/url" "testing" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -210,7 +210,7 @@ func TestVitessConnectionAssemblerCustomDatabaseAttribute(t *testing.T) { // The PlanetScale database name attribute is required: without it every API // call would fall back to addressing the database by its registered -// identifier, which is an arbitrary routing key rather than a PlanetScale name. +// identifier, which is an arbitrary routing key rather than a PlanetScale name. sadscan:disable kingfisher.planetscale.2 func TestVitessConnectionAssemblerRequiresDatabase(t *testing.T) { _, _, err := VitessConnectionAssembler{}.Assemble( "", @@ -339,7 +339,7 @@ func TestPostgresConnectionAssemblerBuildsLibpqURL(t *testing.T) { &Credentials{Username: "pgsprite_engine", Password: "s3cret"}, ) require.NoError(t, err) - assert.Equal(t, "postgresql://pgsprite_engine:s3cret@orders.cluster-abc.us-east-1.rds.amazonaws.com:5432/orders?sslmode=verify-full", dsn) + assert.Equal(t, "postgresql://pgsprite_engine:s3cret@orders.cluster-abc.us-east-1.rds.amazonaws.com:5432/orders?sslmode=verify-full", dsn) // sadscan:disable np.postgres.1 assert.Equal(t, map[string]string{ "extra": "field", MetadataPostgresCARef: PostgresCARefEmbeddedRDSGlobal, diff --git a/pkg/inventory/static.go b/pkg/inventory/static.go index e61caaaa6..6844f50f2 100644 --- a/pkg/inventory/static.go +++ b/pkg/inventory/static.go @@ -8,7 +8,7 @@ import ( "strconv" "strings" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" "github.com/block/schemabot/pkg/secrets" ) diff --git a/pkg/inventory/static_test.go b/pkg/inventory/static_test.go index 304995d01..e6d8d4b33 100644 --- a/pkg/inventory/static_test.go +++ b/pkg/inventory/static_test.go @@ -5,7 +5,7 @@ import ( "strings" "testing" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -566,7 +566,7 @@ func TestStaticResolverResolveTargetDSNFromPostgres(t *testing.T) { assert.Equal(t, "orders-prod", got.Target) assert.Equal(t, "postgres", got.DatabaseType) - assert.Equal(t, "postgresql://pgsprite_engine:s3cret@orders.cluster-abc.us-east-1.rds.amazonaws.com:5432/orders?sslmode=verify-full", got.DSN) + assert.Equal(t, "postgresql://pgsprite_engine:s3cret@orders.cluster-abc.us-east-1.rds.amazonaws.com:5432/orders?sslmode=verify-full", got.DSN) // sadscan:disable np.postgres.1 assert.Equal(t, map[string]string{ "extra": "field", MetadataPostgresCARef: PostgresCARefEmbeddedRDSGlobal, @@ -617,7 +617,7 @@ func TestStaticResolverDSNFromPostgresDefaults(t *testing.T) { got, err := resolver.ResolveTarget(t.Context(), Request{Target: "orders-prod"}) require.NoError(t, err) - assert.Equal(t, "postgresql://pgsprite_engine:s3cret@orders.cluster-abc.us-east-1.rds.amazonaws.com:5432/orders?sslmode=verify-full", got.DSN) + assert.Equal(t, "postgresql://pgsprite_engine:s3cret@orders.cluster-abc.us-east-1.rds.amazonaws.com:5432/orders?sslmode=verify-full", got.DSN) // sadscan:disable np.postgres.1 assert.Equal(t, PostgresCARefEmbeddedRDSGlobal, got.Metadata[MetadataPostgresCARef]) } diff --git a/pkg/localscale/handlers_branches.go b/pkg/localscale/handlers_branches.go index 416ac51d2..541c358c2 100644 --- a/pkg/localscale/handlers_branches.go +++ b/pkg/localscale/handlers_branches.go @@ -183,7 +183,7 @@ func (s *Server) snapshotBranch(ctx context.Context, backend *databaseBackend, o // Open a connection to the backend's mysqld for branch database creation. // Each org/database has its own managed cluster with its own mysqld. s.logger.Info("branch snapshot: opening backend mysqld", "dsn_prefix", backend.mysqlDSNBase) - backendDB, err := sql.Open("mysql", backend.mysqlDSNBase) + backendDB, err := sql.Open("block-mysql", backend.mysqlDSNBase) if err != nil { return fmt.Errorf("open backend mysqld: %w", err) } @@ -216,7 +216,7 @@ func (s *Server) snapshotBranch(ctx context.Context, backend *databaseBackend, o // Execute CREATE TABLEs in branch database (on the backend's mysqld) if len(stmts) > 0 { - branchDB, err := sql.Open("mysql", backend.mysqlDSNBase+branchDBName(branchName, keyspace)) + branchDB, err := sql.Open("block-mysql", backend.mysqlDSNBase+branchDBName(branchName, keyspace)) if err != nil { return fmt.Errorf("open branch database %s: %w", keyspace, err) } diff --git a/pkg/localscale/helpers.go b/pkg/localscale/helpers.go index d8ce6e1c5..db8b25e8b 100644 --- a/pkg/localscale/helpers.go +++ b/pkg/localscale/helpers.go @@ -320,7 +320,7 @@ func (s *Server) getBranchSchemaFromBackend(ctx context.Context, backend *databa } func (s *Server) getBranchSchemaWithDSN(ctx context.Context, dsnBase, branch, keyspace string) ([]string, error) { - db, err := sql.Open("mysql", dsnBase+branchDBName(branch, keyspace)) + db, err := sql.Open("block-mysql", dsnBase+branchDBName(branch, keyspace)) if err != nil { return nil, fmt.Errorf("open branch db %s/%s: %w", branch, keyspace, err) } @@ -578,7 +578,7 @@ func (s *Server) getDeployRequestInfo(ctx context.Context, ref deployRequest) (* // and shuts down any TCP proxy associated with the branch. Branches live on // the backend's mysqld (not the metadata DB), so we connect there for the DROP. func (s *Server) dropBranchDatabases(ctx context.Context, backend *databaseBackend, branch string) { - db, err := sql.Open("mysql", backend.mysqlDSNBase) + db, err := sql.Open("block-mysql", backend.mysqlDSNBase) if err != nil { s.logger.Error("dropBranchDatabases: open backend", "branch", branch, "error", err) return @@ -760,7 +760,7 @@ func (s *Server) openBranchDB(ctx context.Context, branch, keyspace string) (*sq } dsn := backend.mysqlDSNBase + branchDBName(branch, keyspace) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { return nil, fmt.Errorf("open branch db %s/%s: %w", branch, keyspace, err) } @@ -859,7 +859,7 @@ const onlineDDLSidecarTable = "schema_migrations" func (s *Server) waitForOnlineDDLReady(ctx context.Context) error { deadline := time.Now().Add(onlineDDLReadyTimeout) for key, backend := range s.backends { - db, err := sql.Open("mysql", backend.mysqlDSNBase) + db, err := sql.Open("block-mysql", backend.mysqlDSNBase) if err != nil { return fmt.Errorf("connect to mysqld for %s/%s: %w", key.org, key.database, err) } diff --git a/pkg/localscale/managed.go b/pkg/localscale/managed.go index ee3d9100c..ce9cdf1a0 100644 --- a/pkg/localscale/managed.go +++ b/pkg/localscale/managed.go @@ -207,7 +207,7 @@ func startManagedCluster(ctx context.Context, keyspaces []KeyspaceConfig, logger // on the managed cluster's mysqld. vttest's init_db.sql only grants vt_dba@'localhost' // (socket-only), so we need a separate user for TCP access (branch proxy upstream). func createManagedTCPUser(mysqlDSNBase string, logger *slog.Logger) error { - db, err := sql.Open("mysql", mysqlDSNBase) + db, err := sql.Open("block-mysql", mysqlDSNBase) if err != nil { return fmt.Errorf("connect to mysqld: %w", err) } @@ -338,7 +338,7 @@ func startManagedClusters( // and returns a *sql.DB connected to it. func createManagedMetadataDB(ctx context.Context, mysqlDSNBase string) (*sql.DB, string, error) { // Connect without database to create it. - rootDB, err := sql.Open("mysql", mysqlDSNBase) + rootDB, err := sql.Open("block-mysql", mysqlDSNBase) if err != nil { return nil, "", fmt.Errorf("connect to mysqld for metadata: %w", err) } @@ -354,7 +354,7 @@ func createManagedMetadataDB(ctx context.Context, mysqlDSNBase string) (*sql.DB, // Connect to the localscale database. dsn := mysqlDSNBase + "localscale" - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { return nil, "", fmt.Errorf("connect to localscale database: %w", err) } diff --git a/pkg/localscale/planetscale_recovery_integration_test.go b/pkg/localscale/planetscale_recovery_integration_test.go index 71116aa76..b768af4dc 100644 --- a/pkg/localscale/planetscale_recovery_integration_test.go +++ b/pkg/localscale/planetscale_recovery_integration_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" ps "github.com/planetscale/planetscale-go/planetscale" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/localscale/proxy.go b/pkg/localscale/proxy.go index 675525a14..1d1659a32 100644 --- a/pkg/localscale/proxy.go +++ b/pkg/localscale/proxy.go @@ -229,7 +229,7 @@ func (p *branchProxy) closeClientConns() { func (p *branchProxy) handleConn(clientConn net.Conn) { // Open a dedicated upstream connection for this client. - upstreamDB, err := sql.Open("mysql", p.upstreamDSN) + upstreamDB, err := sql.Open("block-mysql", p.upstreamDSN) if err != nil { p.logger.Error("proxy: open upstream", "error", err) utils.CloseAndLog(clientConn) diff --git a/pkg/localscale/server.go b/pkg/localscale/server.go index dd55a516f..e9c1f996b 100644 --- a/pkg/localscale/server.go +++ b/pkg/localscale/server.go @@ -34,9 +34,9 @@ import ( "syscall" "time" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/table" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/block/schemabot/pkg/ddl" localscaleschema "github.com/block/schemabot/pkg/localscale/schema" @@ -238,7 +238,7 @@ func New(ctx context.Context, cfg Config) (*Server, error) { vtgateDBs := make(map[string]*sql.DB) for _, ks := range dbCfg.Keyspaces { dsn := fmt.Sprintf("root@tcp(%s)/%s", mc.vtgateMySQLAddr, ks.Name) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { closeDatabaseBackend(vtctld, vtgateDBs) return nil, fmt.Errorf("connect to vtgate keyspace %s (%s/%s): %w", ks.Name, orgName, dbName, err) @@ -252,7 +252,7 @@ func New(ctx context.Context, cfg Config) (*Server, error) { } // Create unscoped vtgate DB pool (no default keyspace) for shard-targeted connections. - unscopedDB, err := sql.Open("mysql", fmt.Sprintf("root@tcp(%s)/", mc.vtgateMySQLAddr)) + unscopedDB, err := sql.Open("block-mysql", fmt.Sprintf("root@tcp(%s)/", mc.vtgateMySQLAddr)) if err != nil { closeDatabaseBackend(vtctld, vtgateDBs) return nil, fmt.Errorf("connect unscoped vtgate for %s/%s: %w", orgName, dbName, err) diff --git a/pkg/localscale/server_deploy_integration_test.go b/pkg/localscale/server_deploy_integration_test.go index 4621ca511..f3ae7caf4 100644 --- a/pkg/localscale/server_deploy_integration_test.go +++ b/pkg/localscale/server_deploy_integration_test.go @@ -10,8 +10,8 @@ import ( "testing" "time" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" ps "github.com/planetscale/planetscale-go/planetscale" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -355,7 +355,7 @@ func TestBranchDDLError(t *testing.T) { // Apply invalid DDL — table doesn't exist dsn := fmt.Sprintf("%s:%s@tcp(%s)/%s", pw.Username, pw.PlainText, pw.Hostname, "testapp_sharded") - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open branch MySQL") require.NoError(t, db.PingContext(ctx), "ping branch MySQL") _, err = db.ExecContext(ctx, "ALTER TABLE nonexistent_table ADD COLUMN x INT") diff --git a/pkg/localscale/server_integration_test.go b/pkg/localscale/server_integration_test.go index cbbff5560..de6bc4dfc 100644 --- a/pkg/localscale/server_integration_test.go +++ b/pkg/localscale/server_integration_test.go @@ -18,8 +18,8 @@ import ( "testing" "time" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" ps "github.com/planetscale/planetscale-go/planetscale" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -554,7 +554,7 @@ func applyBranchDDL(t *testing.T, ctx context.Context, branchName string, ddl ma for keyspace, stmts := range ddl { dsn := fmt.Sprintf("%s:%s@tcp(%s)/%s", pw.Username, pw.PlainText, pw.Hostname, keyspace) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open branch MySQL for %s", keyspace) require.NoError(t, db.PingContext(ctx), "ping branch MySQL for %s", keyspace) for _, stmt := range stmts { diff --git a/pkg/localscale/tls_integration_test.go b/pkg/localscale/tls_integration_test.go index c51f3ec21..427f10d12 100644 --- a/pkg/localscale/tls_integration_test.go +++ b/pkg/localscale/tls_integration_test.go @@ -12,8 +12,8 @@ import ( "testing" "time" + "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - "github.com/go-sql-driver/mysql" ps "github.com/planetscale/planetscale-go/planetscale" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -113,7 +113,7 @@ func TestMTLS_BranchConnection(t *testing.T) { // Connect to branch proxy via mTLS dsn := fmt.Sprintf("%s:%s@tcp(%s)/testkeyspace?tls=test-mtls&interpolateParams=true", pw.Username, pw.PlainText, pw.Hostname) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) diff --git a/pkg/mysqlconn/mysqlconn.go b/pkg/mysqlconn/mysqlconn.go index 6dae3446c..ee399fbb7 100644 --- a/pkg/mysqlconn/mysqlconn.go +++ b/pkg/mysqlconn/mysqlconn.go @@ -1,15 +1,13 @@ package mysqlconn import ( - "context" "database/sql" "fmt" - "log/slog" "time" + "github.com/block/mysql" + "github.com/block/schemabot/pkg/connreload" "github.com/block/spirit/pkg/dbconn" - dsndriver "github.com/go-mysql/hotswap-dsn-driver" - "github.com/go-sql-driver/mysql" ) var openSQL = sql.Open @@ -55,10 +53,20 @@ func WithConnectTimeout(d time.Duration) Option { } } -// hotswapDriverName is Daniel Nichter's (https://github.com/daniel-nichter) -// hot-swap DSN driver, a drop-in replacement for github.com/go-sql-driver/mysql -// that re-reads credentials on an access-denied error. See OpenReloadable. -const hotswapDriverName = "mysql-hotswap-dsn" +// driverName is block/mysql, Block's fork of go-sql-driver/mysql. It registers +// itself as "block-mysql" rather than "mysql" so that a binary whose dependency +// graph still reaches upstream can link both without two sql.Register calls +// colliding under one name. SchemaBot's own graph no longer reaches upstream at +// all, but the fork's registered name is not SchemaBot's to choose. +// +// Every pool in this package dials through this one driver, and that is +// load-bearing rather than tidy: ConnectionDSN injects tls=rds, and a tls= +// value is a *name* that only resolves inside the registry of the driver +// package that registered it. Spirit registers "rds" into block/mysql. A pool +// opened through any other MySQL driver — as the reloadable pool once was, via +// a hot-swap driver that embedded upstream — cannot resolve the name and fails +// to open against an RDS host at all. +const driverName = "block-mysql" // Open returns a MySQL connection using the same target-DSN normalization as // Spirit. Options customize the DSN (for example WithConnectTimeout) before the @@ -68,68 +76,52 @@ func Open(dsn string, opts ...Option) (*sql.DB, error) { if err != nil { return nil, err } - db, err := openSQL("mysql", connectionDSN) + db, err := openSQL(driverName, connectionDSN) if err != nil { return nil, fmt.Errorf("open MySQL connection: %w", err) } return db, nil } -// OpenReloadable opens a connection whose credentials survive rotation of the -// underlying secret. When a new connection is rejected with MySQL error 1045 +// OpenReloadable opens a connection pool whose credentials survive rotation of +// the underlying secret. When a new connection is refused with MySQL error 1045 // (access denied) — the signature of a password that was rotated out from under -// a running pod — the driver calls reload to fetch a freshly resolved DSN -// (re-reading the mounted secret) and retries, so rotation is transparent and -// does not require a restart. reload returns the raw DSN; transport settings -// are re-applied here. A reload error keeps the current credentials so a -// transient resolve failure cannot wedge the pool with an empty DSN. +// a running pod — the pool calls reload to fetch a freshly resolved DSN +// (re-reading the mounted secret) and retries once, so rotation is transparent +// and does not require a restart. reload returns the raw DSN; transport +// settings and options are re-applied here. A reload error keeps the current +// credentials so a transient resolve failure cannot wedge the pool, and starts +// a short cooldown during which further refused dials skip the reload — the DSN +// may resolve through a remote secrets backend, and an outage there must not +// turn every refused dial into a resolve call. // // secret rotated ──► new conn ──► 1045 access denied // │ // ▼ // reload: re-resolve DSN (re-read secret) -// │ (on error: keep current DSN) +// │ (on error: keep current credentials) // ▼ // retry with fresh credentials ──► success // -// The reload callback is registered process-global on the hot-swap driver and -// applies to every connection opened with that driver; each OpenReloadable call -// replaces it. OpenReloadable is the only path that opens with the hot-swap -// driver, so reserve it for the single long-lived storage pool. Target-database -// connections use Open, whose credentials come from the apply request rather -// than the storage secret. +// Established connections authenticated before the rotation keep working; only +// new physical connections take the reload path. reload runs only after an +// access-denied failure — never per connection — so a DSN resolved through a +// remote secrets backend is not re-fetched on every dial. The callback belongs +// to this pool alone, so opening two reloadable pools does not have one +// silently inherit the other's credentials. Reserve OpenReloadable for the +// single long-lived storage pool; target-database connections use Open, whose +// credentials come from the apply request rather than the storage secret. +// +// The scheduling — how many reloads a burst of refused dials costs, and how a +// failing secrets backend is backed off — is pkg/connreload's and is shared +// with the PostgreSQL storage pool; see reloadConfig for the MySQL-specific +// half. func OpenReloadable(dsn string, reload func() (string, error), opts ...Option) (*sql.DB, error) { - connectionDSN, err := ConnectionDSN(dsn, opts...) - if err != nil { - return nil, err - } - dsndriver.SetHotswapFunc(func(_ context.Context, _ string) string { - return reloadConnectionDSN(reload, opts...) - }) - db, err := openSQL(hotswapDriverName, connectionDSN) + connector, err := connreload.New(dsn, reloadConfig(reload, opts)) if err != nil { return nil, fmt.Errorf("open reloadable MySQL connection: %w", err) } - return db, nil -} - -// reloadConnectionDSN resolves a fresh DSN and re-applies transport settings for -// the hot-swap driver. It returns "" — meaning "keep the current DSN" — when the -// reload or transport step fails, so a transient error cannot wedge the pool -// with an empty DSN. -func reloadConnectionDSN(reload func() (string, error), opts ...Option) string { - rawDSN, err := reload() - if err != nil { - slog.Error("reload storage DSN after access-denied failed; keeping current credentials", "error", err) - return "" - } - reloadedDSN, err := ConnectionDSN(rawDSN, opts...) - if err != nil { - slog.Error("apply transport settings to reloaded storage DSN failed; keeping current credentials", "error", err) - return "" - } - slog.Info("reloaded storage credentials after access-denied error") - return reloadedDSN + return sql.OpenDB(connector), nil } // ConnectionDSN returns a MySQL DSN with required connection settings applied diff --git a/pkg/mysqlconn/mysqlconn_test.go b/pkg/mysqlconn/mysqlconn_test.go index 17ab9a113..af2a47b94 100644 --- a/pkg/mysqlconn/mysqlconn_test.go +++ b/pkg/mysqlconn/mysqlconn_test.go @@ -6,7 +6,7 @@ import ( "testing" "time" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -195,56 +195,12 @@ func TestOpenNormalizesRDSDSNBeforeOpening(t *testing.T) { _, err := Open("spirit:secret@tcp(database.cluster-abc123.us-west-2.rds.amazonaws.com:3306)/app?parseTime=true") require.ErrorIs(t, err, openErr) - assert.Equal(t, "mysql", gotDriver) + // Not "mysql": that name belongs to upstream go-sql-driver. Nothing in + // SchemaBot's graph registers it any more, so opening under it would fail + // outright — but a dependency that reaches upstream again would make it + // resolve to the wrong driver silently, which is what this pins. + assert.Equal(t, "block-mysql", gotDriver) cfg, parseErr := mysql.ParseDSN(gotDSN) require.NoError(t, parseErr) assert.Equal(t, "rds", cfg.TLSConfig) } - -func TestOpenReloadableUsesHotswapDriver(t *testing.T) { - originalOpenSQL := openSQL - t.Cleanup(func() { openSQL = originalOpenSQL }) - - openErr := errors.New("stop before network connection") - var gotDriver string - openSQL = func(driverName, _ string) (*sql.DB, error) { - gotDriver = driverName - return nil, openErr - } - - _, err := OpenReloadable("spirit:secret@tcp(127.0.0.1:3306)/app", func() (string, error) { - return "", nil - }) - - require.ErrorIs(t, err, openErr) - assert.Equal(t, hotswapDriverName, gotDriver) -} - -func TestReloadConnectionDSN(t *testing.T) { - t.Run("re-applies RDS transport to the reloaded DSN", func(t *testing.T) { - got := reloadConnectionDSN(func() (string, error) { - return "spirit:rotated@tcp(database.cluster-abc123.us-west-2.rds.amazonaws.com:3306)/app", nil - }) - - cfg, err := mysql.ParseDSN(got) - require.NoError(t, err) - assert.Equal(t, "rotated", cfg.Passwd) - assert.Equal(t, "rds", cfg.TLSConfig) - }) - - t.Run("keeps current DSN when reload fails", func(t *testing.T) { - got := reloadConnectionDSN(func() (string, error) { - return "", errors.New("secret file unreadable") - }) - - assert.Empty(t, got) - }) - - t.Run("keeps current DSN when the reloaded DSN is unparseable", func(t *testing.T) { - got := reloadConnectionDSN(func() (string, error) { - return "not-a-valid-dsn", nil - }) - - assert.Empty(t, got) - }) -} diff --git a/pkg/mysqlconn/reloadable.go b/pkg/mysqlconn/reloadable.go new file mode 100644 index 000000000..3c6a909ea --- /dev/null +++ b/pkg/mysqlconn/reloadable.go @@ -0,0 +1,67 @@ +package mysqlconn + +import ( + "database/sql/driver" + + "github.com/block/mysql" + "github.com/block/schemabot/pkg/connreload" + "github.com/block/schemabot/pkg/mysqlerr" +) + +// erAccessDenied is MySQL's ER_ACCESS_DENIED_ERROR, returned when the server +// rejects the credentials a connection presented. It is the signature of a +// password rotated out from under a running pod, and it is the only code that +// triggers a credential reload. +// +// ER_ACCESS_DENIED_NO_PASSWORD_ERROR (1698) is deliberately not included. It +// means the account authenticates by something other than the password sent — +// auth_socket, or an account with no password at all — which is a grant shape +// no rotation of the secret changes, so a reload could only re-resolve the same +// credential and surface the same refusal a cooldown window later. +const erAccessDenied = 1045 + +// newConnector builds the connector that dials physical connections for a +// config. It is a seam so tests can exercise the reload path without a server. +var newConnector = mysql.NewConnector + +// resolveConnector normalizes a raw DSN the way every SchemaBot-managed MySQL +// connection is normalized (see ConnectionDSN), applies the caller's options, +// and builds the connector that dials it. It is the Resolve half of the +// reloadable pool: it runs once at open and once per reload, never per dial, so +// a reloaded DSN the driver refuses is rejected when it is published rather +// than on every dial that follows. +func resolveConnector(dsn string, opts ...Option) (driver.Connector, error) { + connectionDSN, err := ConnectionDSN(dsn, opts...) + if err != nil { + return nil, err + } + cfg, err := mysql.ParseDSN(connectionDSN) + if err != nil { + return nil, err + } + return newConnector(cfg) +} + +// isAccessDenied reports whether err is the server rejecting the connection's +// credentials. The code is read through mysqlerr.Number rather than by +// asserting a driver error type, so a dial error wrapped on the way up — which +// database/sql and the pool above it both do — is still recognized. +func isAccessDenied(err error) bool { + return mysqlerr.Is(err, erAccessDenied) +} + +// reloadConfig describes the MySQL storage pool to pkg/connreload. Everything +// about *when* to reload — one reload per generation of credentials however +// many dials failed against it, a cooldown so a secrets-backend outage is not +// amplified into one resolve per refused dial, a reload detached from the dial +// that elected it — lives there and is shared with the PostgreSQL pool. What is +// MySQL's, and all that is MySQL's, is the two functions below. +func reloadConfig(reload func() (string, error), opts []Option) connreload.Config { + return connreload.Config{ + Resolve: func(dsn string) (driver.Connector, error) { return resolveConnector(dsn, opts...) }, + Refused: isAccessDenied, + Reload: reload, + Driver: mysql.MySQLDriver{}, + Name: "mysql-storage", + } +} diff --git a/pkg/mysqlconn/reloadable_test.go b/pkg/mysqlconn/reloadable_test.go new file mode 100644 index 000000000..12f606585 --- /dev/null +++ b/pkg/mysqlconn/reloadable_test.go @@ -0,0 +1,228 @@ +package mysqlconn + +import ( + "context" + "database/sql/driver" + "errors" + "fmt" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/block/mysql" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The scheduling of reloads is pkg/connreload's and is tested there. What is +// tested here is the MySQL-specific half: which error means "the server +// rejected these credentials", and that a reloaded DSN gets the same +// normalization and options as the DSN the pool was opened with. + +// stubConn is the minimal driver.Conn a fake dial can hand back. +type stubConn struct{} + +func (stubConn) Prepare(string) (driver.Stmt, error) { return nil, errors.New("not implemented") } +func (stubConn) Close() error { return nil } +func (stubConn) Begin() (driver.Tx, error) { return nil, errors.New("not implemented") } + +// fakeConnector dials for one resolved config, recording the password of every +// attempt into the shared log and taking its outcome from results, consumed in +// order across every generation. +type fakeConnector struct { + t *testing.T + password string + passwords *[]string + results []error + mu *sync.Mutex +} + +func (f *fakeConnector) Connect(context.Context) (driver.Conn, error) { + f.mu.Lock() + *f.passwords = append(*f.passwords, f.password) + i := len(*f.passwords) - 1 + f.mu.Unlock() + // assert (not require): this runs on the dialing goroutine, and testify's + // FailNow is only valid on the test goroutine. The error return fails the + // dial cleanly instead. + if !assert.Less(f.t, i, len(f.results), "unexpected extra dial attempt") { + return nil, errors.New("unexpected extra dial attempt") + } + if err := f.results[i]; err != nil { + return nil, err + } + return stubConn{}, nil +} + +func (f *fakeConnector) Driver() driver.Driver { return mysql.MySQLDriver{} } + +// fakeDial replaces the newConnector seam and returns the log recording the +// password of every dial attempt, in order. +func fakeDial(t *testing.T, results []error) *[]string { + t.Helper() + var passwords []string + var mu sync.Mutex + original := newConnector + t.Cleanup(func() { newConnector = original }) + newConnector = func(cfg *mysql.Config) (driver.Connector, error) { + return &fakeConnector{t: t, password: cfg.Passwd, passwords: &passwords, results: results, mu: &mu}, nil + } + return &passwords +} + +func accessDeniedError() error { + return fmt.Errorf("connect: %w", &mysql.MySQLError{ + Number: erAccessDenied, + Message: "Access denied for user 'schemabot'@'10.0.0.1' (using password: YES)", + }) +} + +func TestIsAccessDenied(t *testing.T) { + assert.True(t, isAccessDenied(&mysql.MySQLError{Number: erAccessDenied})) + // database/sql wraps, so recognizing a bare error is not enough. + assert.True(t, isAccessDenied(accessDeniedError())) + // 1698 is access-denied for an account that authenticates by something + // other than the password sent, which no rotation of the secret changes. + assert.False(t, isAccessDenied(&mysql.MySQLError{Number: 1698})) + // Any other MySQL error is not a rotation signal: a deadlock or an unknown + // database must not cost a secret resolution. + assert.False(t, isAccessDenied(&mysql.MySQLError{Number: 1049})) + assert.False(t, isAccessDenied(&mysql.MySQLError{Number: 1213})) + assert.False(t, isAccessDenied(errors.New("connection refused"))) + assert.False(t, isAccessDenied(nil)) +} + +func TestResolveConnectorAppliesOptionsAndNormalization(t *testing.T) { + // A reloaded raw DSN gets the same treatment as the boot DSN, because both + // go through resolveConnector: caller options are re-applied, an RDS host + // gets TLS injected, and the required settings every managed connection + // carries are re-imposed. The assertions read the config the seam is + // handed, which is what the pool will dial with. + original := newConnector + t.Cleanup(func() { newConnector = original }) + var got *mysql.Config + newConnector = func(cfg *mysql.Config) (driver.Connector, error) { + got = cfg + return nil, nil + } + + _, err := resolveConnector( + "schemabot:rotated@tcp(database.cluster-abc123.us-west-2.rds.amazonaws.com:3306)/app", + WithConnectTimeout(7*time.Second), + ) + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, "rotated", got.Passwd) + assert.Equal(t, 7*time.Second, got.Timeout, "options must flow through the resolve path") + assert.Equal(t, "rds", got.TLSConfig, "an RDS DSN must get TLS injected") + assert.Equal(t, defaultWriteTimeout, got.WriteTimeout, "required settings must be imposed") + assert.True(t, got.InterpolateParams) +} + +func TestResolveConnectorRejectsBadDSN(t *testing.T) { + _, err := resolveConnector("not-a-dsn") + require.Error(t, err) +} + +// The reloadable storage pool must reach an RDS host *with TLS* — not merely +// open. This is the case that was broken and could not be seen: `tls=rds` is a +// name, and a name only resolves inside the registry of the driver package +// that registered it. Spirit registers it into block/mysql, and the reloadable +// pool used to be opened with a hot-swap driver that embedded upstream +// go-sql-driver, whose registry has no "rds" — so a MySQL storage pool on an +// RDS host failed to open at all and the server did not start. No CI job points +// storage at an *.rds.amazonaws.com address, so the break was host-shaped +// rather than code-shaped and nothing in the suite could catch it. +// +// Both pools now dial through block/mysql, so there is one registry and the +// name resolves on both paths. The assertions are on the resolved trust rather +// than on the DSN text, because the failure this replaces would have been +// equally invisible to a test that only checked the pool opened: dropping the +// tls= injection also "opens", by connecting in the clear. +func TestReloadablePoolReachesRDSHostWithVerifiedTLS(t *testing.T) { + const host = "sb.cluster-abc123.us-west-2.rds.amazonaws.com" + const rawDSN = "u:p@tcp(" + host + ":3306)/schemabot" + + // First half: the DSN this pool is opened with names a TLS config, and the + // driver the pool dials through resolves that name. This is the assertion + // the bug would have failed — the name is registered in block/mysql, and a + // pool dialing through any other MySQL driver cannot resolve it. + // + // It asserts the mechanism, not just the outcome, and that is deliberate: + // block/mysql applies RDS TLS on its own for an RDS address even with no + // tls= at all, so the resolved-trust assertions below hold either way and + // cannot tell the two apart. Dropping the injection and leaning on the + // driver's own auto-TLS may well be right later; this line is here so that + // is a decision someone makes, rather than a silent change in which layer + // is responsible for encrypting the connection that carries every + // credential and lease. + connectionDSN, err := ConnectionDSN(rawDSN) + require.NoError(t, err) + assert.Contains(t, connectionDSN, "tls=rds", "the RDS TLS config name must be injected") + named, err := mysql.ParseDSN(connectionDSN) + require.NoError(t, err, "the driver this pool dials through cannot resolve the injected TLS config name") + require.NotNil(t, named.TLS) + + original := newConnector + t.Cleanup(func() { newConnector = original }) + var dialed *mysql.Config + var dials atomic.Int32 + newConnector = func(cfg *mysql.Config) (driver.Connector, error) { + dialed = cfg + return connectFunc(func(context.Context) (driver.Conn, error) { + dials.Add(1) + return stubConn{}, nil + }), nil + } + + db, err := OpenReloadable(rawDSN, func() (string, error) { + return "", errors.New("no rotation in this test") + }) + require.NoError(t, err, "the storage pool must open against an RDS host") + t.Cleanup(func() { assert.NoError(t, db.Close()) }) + + require.NoError(t, db.PingContext(t.Context())) + require.Equal(t, int32(1), dials.Load()) + + // Second half: what the pool actually dials with authenticates the server — + // real roots, the right ServerName, no skipped verification, no plaintext + // fallback. A pool that merely opened, in the clear, fails every line here. + require.NotNil(t, dialed) + require.NotNil(t, dialed.TLS, "the pool would connect in the clear") + assert.Equal(t, host, dialed.TLS.ServerName) + assert.False(t, dialed.TLS.InsecureSkipVerify, "the RDS trust store must actually be verified against") + assert.NotNil(t, dialed.TLS.RootCAs, "verification against a nil root pool is the ambient system store, which has no RDS roots") + assert.False(t, dialed.AllowFallbackToPlaintext, "a plaintext fallback would make the TLS above optional") +} + +// connectFunc adapts a dial function to driver.Connector. +type connectFunc func(context.Context) (driver.Conn, error) + +func (f connectFunc) Connect(ctx context.Context) (driver.Conn, error) { return f(ctx) } +func (connectFunc) Driver() driver.Driver { return mysql.MySQLDriver{} } + +// End to end through pkg/connreload: a pool whose first dial is refused +// re-resolves its DSN and retries with the rotated password, without the +// caller doing anything. +func TestOpenReloadableRotatesCredentialsOnAccessDenied(t *testing.T) { + passwords := fakeDial(t, []error{accessDeniedError(), nil}) + var reloads atomic.Int32 + + db, err := OpenReloadable("schemabot:old@tcp(localhost:3306)/app", func() (string, error) { + reloads.Add(1) + return "schemabot:rotated@tcp(localhost:3306)/app", nil + }) + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, db.Close()) }) + + require.NoError(t, db.PingContext(t.Context())) + assert.Equal(t, []string{"old", "rotated"}, *passwords, "the retry must dial with the reloaded credentials") + assert.Equal(t, int32(1), reloads.Load()) +} + +func TestOpenReloadableRejectsBadDSN(t *testing.T) { + _, err := OpenReloadable("not-a-dsn", func() (string, error) { return "", nil }) + require.Error(t, err) + assert.Contains(t, err.Error(), "open reloadable MySQL connection") +} diff --git a/pkg/mysqlerr/mysqlerr.go b/pkg/mysqlerr/mysqlerr.go index 03e14fefa..bfc2f02f0 100644 --- a/pkg/mysqlerr/mysqlerr.go +++ b/pkg/mysqlerr/mysqlerr.go @@ -25,7 +25,7 @@ import ( "regexp" "strconv" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" ) // Generic is what an unrecognized failure reports. It says where the reason is @@ -126,9 +126,10 @@ func codeScanRegion(msg string) string { // this package owns. err is read for its error code and is never carried into // the result, so callers are free to pass an error wrapping anything. func Reason(err error) string { - var mysqlErr *mysql.MySQLError - if errors.As(err, &mysqlErr) { - return render(int(mysqlErr.Number)) + // Number, not a type assertion: a target error can arrive from either + // linked driver. See number.go. + if number, ok := Number(err); ok { + return render(int(number)) } // Context errors are classified ahead of the connection probes below, // because both probes would otherwise claim them. An exceeded deadline diff --git a/pkg/mysqlerr/mysqlerr_test.go b/pkg/mysqlerr/mysqlerr_test.go index 342a1b20e..a343043fb 100644 --- a/pkg/mysqlerr/mysqlerr_test.go +++ b/pkg/mysqlerr/mysqlerr_test.go @@ -11,7 +11,7 @@ import ( "strings" "testing" - "github.com/go-sql-driver/mysql" + "github.com/block/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) diff --git a/pkg/mysqlerr/number.go b/pkg/mysqlerr/number.go new file mode 100644 index 000000000..0d290bd85 --- /dev/null +++ b/pkg/mysqlerr/number.go @@ -0,0 +1,42 @@ +package mysqlerr + +import ( + "errors" + "slices" + + "github.com/block/mysql" +) + +// Number reports the MySQL server error code carried by err. +// +// It exists so no call site asserts a driver's error type directly. SchemaBot +// links exactly one MySQL driver — block/mysql — and that is a property worth +// keeping rather than assuming: it stopped being true once, when the +// credential-reloading storage pool went through a hot-swap DSN driver that +// embedded upstream go-sql-driver/mysql and could not be pointed at the fork. +// The two *mysql.MySQLError structs are field-identical and carry the same +// codes, but they are distinct types in distinct packages, so errors.As against +// one silently returns false for the other. Silently is the problem: a retry +// classifier that checks only one type does not fail loudly on the other, it +// just stops recognizing deadlocks and starts surfacing them as permanent +// errors. Reading the code through here means a second driver re-entering the +// graph is a change to one function rather than a hunt through every classifier +// in the repo. +func Number(err error) (uint16, bool) { + var mysqlErr *mysql.MySQLError + if errors.As(err, &mysqlErr) { + return mysqlErr.Number, true + } + return 0, false +} + +// Is reports whether err is a MySQL server error with any of the given codes. +// It is the common shape of a Number call and exists so callers testing a +// fixed set of codes do not each re-derive it. +func Is(err error, codes ...uint16) bool { + number, ok := Number(err) + if !ok { + return false + } + return slices.Contains(codes, number) +} diff --git a/pkg/mysqlerr/number_test.go b/pkg/mysqlerr/number_test.go new file mode 100644 index 000000000..31af549f0 --- /dev/null +++ b/pkg/mysqlerr/number_test.go @@ -0,0 +1,43 @@ +package mysqlerr + +import ( + "errors" + "fmt" + "testing" + + "github.com/block/mysql" + "github.com/stretchr/testify/require" +) + +func TestNumberReadsDriverErrors(t *testing.T) { + const deadlock = 1213 + + t.Run("driver error", func(t *testing.T) { + number, ok := Number(&mysql.MySQLError{Number: deadlock, Message: "Deadlock found"}) + require.True(t, ok) + require.Equal(t, uint16(deadlock), number) + }) + + t.Run("wrapped", func(t *testing.T) { + // database/sql and every layer above it wrap, so unwrapping is not + // optional — this is the reason call sites must not type-assert. + number, ok := Number(fmt.Errorf("exec: %w", &mysql.MySQLError{Number: deadlock})) + require.True(t, ok, "wrapped error was not unwrapped") + require.Equal(t, uint16(deadlock), number) + }) + + t.Run("not a MySQL error", func(t *testing.T) { + _, ok := Number(errors.New("connection refused")) + require.False(t, ok) + _, ok = Number(nil) + require.False(t, ok) + }) +} + +func TestIsMatchesAnyCode(t *testing.T) { + err := error(&mysql.MySQLError{Number: 1205}) + require.True(t, Is(err, 1213, 1205), "lock-wait timeout not matched among several codes") + require.False(t, Is(err, 1213, 1062), "matched a code the error does not carry") + require.False(t, Is(errors.New("nope"), 1205)) + require.False(t, Is(err), "no codes must never match") +} diff --git a/pkg/namedlock/namedlock_integration_test.go b/pkg/namedlock/namedlock_integration_test.go index 8f794a753..52ccd7d95 100644 --- a/pkg/namedlock/namedlock_integration_test.go +++ b/pkg/namedlock/namedlock_integration_test.go @@ -17,7 +17,7 @@ import ( "github.com/block/schemabot/pkg/testutil" - _ "github.com/go-sql-driver/mysql" + _ "github.com/block/mysql" _ "github.com/jackc/pgx/v5/stdlib" ) @@ -97,7 +97,7 @@ type lockerCase struct { // the DSNs. func lockerCases() []lockerCase { return []lockerCase{ - {name: "mysql", locker: MySQL{}, driver: "mysql", dsn: mysqlDSN}, + {name: "mysql", locker: MySQL{}, driver: "block-mysql", dsn: mysqlDSN}, {name: "postgres", locker: Postgres{}, driver: "pgx", dsn: postgresDSN}, } } diff --git a/pkg/pendingdrops/cleaner_integration_test.go b/pkg/pendingdrops/cleaner_integration_test.go index a9d6d2222..a7b91b21e 100644 --- a/pkg/pendingdrops/cleaner_integration_test.go +++ b/pkg/pendingdrops/cleaner_integration_test.go @@ -20,7 +20,7 @@ import ( "github.com/block/schemabot/pkg/namedlock" "github.com/block/schemabot/pkg/testutil" - _ "github.com/go-sql-driver/mysql" + _ "github.com/block/mysql" ) var sharedDSN string @@ -52,7 +52,7 @@ func TestMain(m *testing.M) { func setupCleanerTest(t *testing.T) *sql.DB { t.Helper() - db, err := sql.Open("mysql", sharedDSN) + db, err := sql.Open("block-mysql", sharedDSN) require.NoError(t, err, "connect to mysql") t.Cleanup(func() { utils.CloseAndLog(db) }) require.NoError(t, testutil.PingMySQL(t.Context(), db), "reach mysql") diff --git a/pkg/postgresconn/postgresconn.go b/pkg/postgresconn/postgresconn.go index 299655677..9d84bd5e1 100644 --- a/pkg/postgresconn/postgresconn.go +++ b/pkg/postgresconn/postgresconn.go @@ -7,30 +7,33 @@ package postgresconn import ( - "context" "crypto/x509" "database/sql" "database/sql/driver" "errors" "fmt" - "log/slog" "net/url" "regexp" "strings" "sync" "time" + "github.com/block/mysql" + "github.com/block/schemabot/pkg/connreload" "github.com/block/spirit/pkg/dbconn" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/stdlib" ) -// connectConfig dials one physical connection for the given config. It is a -// seam so tests can exercise the credential-reload path without a server. -var connectConfig = func(ctx context.Context, cfg pgx.ConnConfig) (driver.Conn, error) { - return stdlib.GetConnector(cfg).Connect(ctx) -} +// getConnector returns the connector that dials physical connections for the +// given config. It is a seam so tests can exercise the credential-reload path +// without a server. It is narrower than stdlib.GetConnector on purpose: the +// variadic options are not used here, and a seam whose signature is exactly +// what the package needs is one a fake cannot get subtly wrong. +var getConnector func(pgx.ConnConfig) driver.Connector = defaultConnector + +func defaultConnector(cfg pgx.ConnConfig) driver.Connector { return stdlib.GetConnector(cfg) } // Option customizes the parsed PostgreSQL config before the pool is opened. // Options are applied in connectionConfig, so they flow through Open, @@ -125,194 +128,45 @@ func Open(dsn string, opts ...Option) (*sql.DB, error) { // OpenReloadable for the single long-lived storage pool; target-database // connections use Open, whose credentials come from the apply request rather // than the storage secret. +// The scheduling — how many reloads a burst of rejected dials costs, and how a +// failing secrets backend is backed off — is pkg/connreload's and is shared +// with the MySQL storage pool; see reloadConfig for the PostgreSQL-specific +// half. func OpenReloadable(dsn string, reload func() (string, error), opts ...Option) (*sql.DB, error) { - cfg, err := connectionConfig(dsn, opts...) + connector, err := connreload.New(dsn, reloadConfig(reload, opts)) if err != nil { return nil, err } - return sql.OpenDB(&reloadableConnector{cfg: cfg, reload: reload, opts: opts}), nil -} - -// reloadCooldown bounds how often a failing reload is retried. After a reload -// fails, further authentication-failed dials within this window surface their -// dial error without invoking reload again, so a secrets-backend outage costs -// at most one resolve attempt per window instead of one per rejected dial. -// The window is also armed when a reload succeeds but the dial retrying with -// the reloaded credentials is rejected too — a backend that keeps answering -// with a credential the server refuses (stale secret sync, dropped role, -// pg_hba mismatch) likewise costs one resolve per window, not one per -// connection. -const reloadCooldown = 30 * time.Second - -// reloadableConnector dials with the most recently resolved credentials and -// refreshes them, at most once per failed attempt, when a dial is rejected as -// unauthenticated. gen counts credential swaps so concurrent failed dials -// trigger a single reload: a dial that failed with an already-superseded -// config retries with the current one instead of reloading again. -type reloadableConnector struct { - reload func() (string, error) - opts []Option - now func() time.Time // test seam; nil means time.Now - - mu sync.Mutex - cfg *pgx.ConnConfig - gen uint64 - lastReloadFail time.Time - reloading chan struct{} // non-nil while a reload for the current generation is in flight; closed when it finishes + return sql.OpenDB(connector), nil } -var _ driver.Connector = (*reloadableConnector)(nil) - -func (c *reloadableConnector) Connect(ctx context.Context) (driver.Conn, error) { - cfg, gen := c.snapshot() - conn, err := connectConfig(ctx, *cfg) - if err == nil || !isAuthError(err) { - return conn, err - } - fresh, freshGen, ok := c.refresh(ctx, gen) - if !ok { - // Reload failed; surface the authentication error that triggered it. +// resolveConnector normalizes a raw DSN the way every SchemaBot-managed +// PostgreSQL connection is normalized (see connectionConfig), applies the +// caller's options, and returns the connector that dials it. It is the Resolve +// half of the reloadable pool: it runs once at open and once per reload, never +// per dial. +func resolveConnector(dsn string, opts ...Option) (driver.Connector, error) { + cfg, err := connectionConfig(dsn, opts...) + if err != nil { return nil, err } - conn, err = connectConfig(ctx, *fresh) - if err != nil && isAuthError(err) { - // The freshly resolved credentials are no better: the secret store - // keeps answering with a credential the server refuses. Arm the - // cooldown so subsequent rejected dials back off instead of - // resolving once per connection. - c.armReloadCooldown(freshGen) - slog.Warn("dial with reloaded storage credentials was also rejected; backing off further reloads", "error", err) - } - return conn, err -} - -// armReloadCooldown starts a reloadCooldown window as if a reload had failed, -// bounding resolve traffic when reloads succeed but the credentials they -// return keep being rejected. rejectedGen is the generation whose credentials -// were rejected: when the connector has already advanced past it, the arm is -// a stale verdict on superseded credentials and must not suppress the newer -// generation's reloads. -func (c *reloadableConnector) armReloadCooldown(rejectedGen uint64) { - c.mu.Lock() - defer c.mu.Unlock() - if c.gen != rejectedGen { - slog.Debug("skipping reload cooldown arm: credentials advanced past the rejected generation", - "rejected_gen", rejectedGen, "current_gen", c.gen) - return - } - c.lastReloadFail = c.clock() -} - -func (c *reloadableConnector) Driver() driver.Driver { return stdlib.GetDefaultDriver() } - -func (c *reloadableConnector) snapshot() (*pgx.ConnConfig, uint64) { - c.mu.Lock() - defer c.mu.Unlock() - return c.cfg, c.gen -} - -// clock returns the current time, honoring the test seam. -func (c *reloadableConnector) clock() time.Time { - if c.now != nil { - return c.now() - } - return time.Now() + return getConnector(*cfg), nil } -// refresh resolves fresh credentials after a dial using generation failedGen -// was rejected as unauthenticated. When another dial already swapped the -// config, the current one is returned without reloading again. A reload or -// parse error keeps the current config and reports false, so a transient -// resolve failure cannot wedge the pool; it also arms reloadCooldown so a -// secrets-backend outage is retried once per window, not once per rejected -// dial. On success it also returns the generation the returned config -// belongs to, so a later rejection of that config can be attributed to the -// right generation. -// -// The reload callback runs detached from every dial — outside the connector -// mutex and on its own goroutine — so a hung secret resolution can neither -// block healthy dials from snapshotting the current config nor pin the dial -// that elected it: database/sql counts a dial against the pool's connection -// budget before Connect runs, so a pinned dial would hold a pool slot for as -// long as the reload hangs. The reloading guard keeps it to one reload in -// flight at a time: every same-generation failure, the electing dial -// included, waits for the reload's outcome — or gives up when its own dial -// context ends, surfacing the dial error. -func (c *reloadableConnector) refresh(ctx context.Context, failedGen uint64) (*pgx.ConnConfig, uint64, bool) { - for { - c.mu.Lock() - if c.gen != failedGen { - cfg, gen := c.cfg, c.gen - c.mu.Unlock() - return cfg, gen, true - } - if !c.lastReloadFail.IsZero() && c.clock().Sub(c.lastReloadFail) < reloadCooldown { - c.mu.Unlock() - slog.Debug("skipping storage DSN reload during cooldown after a failed reload; surfacing the dial error") - return nil, 0, false - } - done := c.reloading - if done == nil { - done = make(chan struct{}) - c.reloading = done - go c.runReload(done) - } - c.mu.Unlock() - select { - case <-done: - // The reload finished; re-check the connector state to pick up - // the swapped config or the armed cooldown. - case <-ctx.Done(): - slog.Debug("dial context ended while waiting for an in-flight storage DSN reload; surfacing the dial error") - return nil, 0, false - } +// reloadConfig describes the PostgreSQL storage pool to pkg/connreload. +// Everything about *when* to reload lives there and is shared with the MySQL +// pool. What is PostgreSQL's, and all that is PostgreSQL's, is the two +// functions below. +func reloadConfig(reload func() (string, error), opts []Option) connreload.Config { + return connreload.Config{ + Resolve: func(dsn string) (driver.Connector, error) { return resolveConnector(dsn, opts...) }, + Refused: isAuthError, + Reload: reload, + Driver: stdlib.GetDefaultDriver(), + Name: "postgres-storage", } } -// runReload invokes the reload callback and parses its DSN outside the -// connector mutex, then publishes the outcome under it: success swaps the -// config, advances the generation, and clears the cooldown; failure arms -// reloadCooldown. It runs on its own goroutine, detached from the dial that -// elected it, so waiters observe the outcome through the connector state -// rather than a return value. The publish runs in a defer so the reloading -// guard is released and waiters are unblocked even if the callback panics; -// the panic is recovered and treated as a failed reload — a detached -// goroutine has no caller to propagate it to, and a panicking secret -// resolver must leave the pool on its current credentials, not crash the -// process. -func (c *reloadableConnector) runReload(done chan struct{}) { - var fresh *pgx.ConnConfig - defer func() { - if r := recover(); r != nil { - slog.Error("reload storage DSN after authentication failure panicked; keeping current credentials", "panic", r) - } - c.mu.Lock() - defer c.mu.Unlock() - c.reloading = nil - close(done) - if fresh == nil { - c.lastReloadFail = c.clock() - return - } - c.cfg = fresh - c.gen++ - c.lastReloadFail = time.Time{} - slog.Info("reloaded storage credentials after authentication failure") - }() - - rawDSN, err := c.reload() - if err != nil { - slog.Error("reload storage DSN after authentication failure failed; keeping current credentials", "error", err) - return - } - cfg, err := connectionConfig(rawDSN, c.opts...) - if err != nil { - slog.Error("parse reloaded storage DSN failed; keeping current credentials", "error", err) - return - } - fresh = cfg -} - // isAuthError reports whether err is the server rejecting the connection's // credentials: 28P01 (invalid_password) is the rotation signature, and 28000 // (invalid_authorization_specification) covers rotation strategies that swap @@ -397,9 +251,19 @@ func connectionConfig(dsn string, opts ...Option) (*pgx.ConnConfig, error) { // system trust store carries, so verifying an RDS connection requires this // pool explicitly. The pool is built once and shared: it is read-only after // construction. +// +// The bundle comes from block/mysql, which is now the one copy of it in the +// dependency graph — spirit used to embed its own and expose the bytes, and +// dropped both when it started delegating to the driver. The pool is Postgres's +// here, but the roots are the same: RDS issues from the same private Amazon +// CAs regardless of engine. +// +// RDSTLSConfig clones its pool per call, so taking RootCAs off it does not +// alias anything the MySQL side is using — appending here could not widen trust +// for MySQL connections even if a caller tried. var rdsRootPool = sync.OnceValues(func() (*x509.CertPool, error) { - pool := x509.NewCertPool() - if !pool.AppendCertsFromPEM(dbconn.GetEmbeddedRDSBundle()) { + pool := mysql.RDSTLSConfig().RootCAs + if pool == nil { return nil, fmt.Errorf("embedded RDS global CA bundle contains no usable certificates") } return pool, nil @@ -443,7 +307,7 @@ func hasRuntimeParam(params map[string]string, key string) bool { // counterpart of the TLS mode mysqlconn injects for RDS MySQL targets. An // explicit sslmode — including disable — always wins, and non-RDS hosts are // left untouched. Both DSN forms are handled: URL -// (postgres://user:pass@host/db) and keyword/value (host=... user=...). +// (postgres://user:pass@host/db) and keyword/value (host=... user=...). sadscan:disable np.postgres.1 // RDS detection considers only the DSN's first host: a multi-host DSN whose // RDS host is a fallback gets no injection, so spell out sslmode explicitly // in multi-host DSNs. diff --git a/pkg/postgresconn/postgresconn_test.go b/pkg/postgresconn/postgresconn_test.go index ca1ca7264..6bf92841c 100644 --- a/pkg/postgresconn/postgresconn_test.go +++ b/pkg/postgresconn/postgresconn_test.go @@ -34,7 +34,7 @@ func TestConnectionDSN(t *testing.T) { }{ { name: "RDS URL host gets sslmode=require", - dsn: "postgres://schemabot:secret@database.cluster-abc123.us-west-2.rds.amazonaws.com:5432/app", + dsn: "postgres://schemabot:secret@database.cluster-abc123.us-west-2.rds.amazonaws.com:5432/app", // sadscan:disable np.postgres.1 want: "postgres://schemabot:secret@database.cluster-abc123.us-west-2.rds.amazonaws.com:5432/app?sslmode=require", }, { @@ -74,7 +74,7 @@ func TestConnectionDSN(t *testing.T) { }, { name: "non-RDS URL host is unchanged", - dsn: "postgres://schemabot:secret@localhost:5432/app", + dsn: "postgres://schemabot:secret@localhost:5432/app", // sadscan:disable np.postgres.1 want: "postgres://schemabot:secret@localhost:5432/app", }, { @@ -92,7 +92,7 @@ func TestConnectionDSN(t *testing.T) { }, { name: "invalid DSN returns context", - dsn: "postgres://schemabot:secret@localhost:not-a-port/app", + dsn: "postgres://schemabot:secret@localhost:not-a-port/app", // sadscan:disable np.postgres.1 wantErrSub: "parse PostgreSQL DSN", }, } @@ -120,7 +120,7 @@ func TestConnectionDSN(t *testing.T) { // global CA bundle — the ambient system trust store does not carry the // private Amazon RDS roots and would fail every handshake. func TestConnectionConfigVerifiesRDSHostsWithEmbeddedRoots(t *testing.T) { - cfg, err := connectionConfig("postgres://schemabot:secret@db.example.rds.amazonaws.com:5432/app?sslmode=verify-full") + cfg, err := connectionConfig("postgres://schemabot:secret@db.example.rds.amazonaws.com:5432/app?sslmode=verify-full") // sadscan:disable np.postgres.1 require.NoError(t, err) require.NotNil(t, cfg.TLSConfig) assert.False(t, cfg.TLSConfig.InsecureSkipVerify) @@ -128,7 +128,7 @@ func TestConnectionConfigVerifiesRDSHostsWithEmbeddedRoots(t *testing.T) { // DNS names are case-insensitive: an uppercase RDS endpoint gets the same // roots. - cfg, err = connectionConfig("postgres://schemabot:secret@DB.EXAMPLE.RDS.AMAZONAWS.COM:5432/app?sslmode=verify-full") + cfg, err = connectionConfig("postgres://schemabot:secret@DB.EXAMPLE.RDS.AMAZONAWS.COM:5432/app?sslmode=verify-full") // sadscan:disable np.postgres.1 require.NoError(t, err) require.NotNil(t, cfg.TLSConfig) assert.NotNil(t, cfg.TLSConfig.RootCAs) @@ -144,7 +144,7 @@ func TestConnectionConfigVerifiesRDSHostsWithEmbeddedRoots(t *testing.T) { // The embedded bundle holds RDS roots only: a non-RDS host gets no // implicit trust material. - cfg, err = connectionConfig("postgres://schemabot:secret@db.internal.example:5432/app?sslmode=verify-full") + cfg, err = connectionConfig("postgres://schemabot:secret@db.internal.example:5432/app?sslmode=verify-full") // sadscan:disable np.postgres.1 require.NoError(t, err) require.NotNil(t, cfg.TLSConfig) assert.Nil(t, cfg.TLSConfig.RootCAs) @@ -199,7 +199,7 @@ func writeSelfSignedCA(t *testing.T) string { // the DSN installed, and a DSN that negotiates no TLS has nothing to pin. func TestWithRootCAsPinsVerificationTrust(t *testing.T) { roots := x509.NewCertPool() - cfg, err := connectionConfig("postgres://schemabot:secret@db.cluster-abc123.eu-west-1.rds.amazonaws.com:5432/app?sslmode=verify-full", WithRootCAs(roots)) + cfg, err := connectionConfig("postgres://schemabot:secret@db.cluster-abc123.eu-west-1.rds.amazonaws.com:5432/app?sslmode=verify-full", WithRootCAs(roots)) // sadscan:disable np.postgres.1 require.NoError(t, err) require.NotNil(t, cfg.TLSConfig) assert.Same(t, roots, cfg.TLSConfig.RootCAs) @@ -210,7 +210,7 @@ func TestWithRootCAsPinsVerificationTrust(t *testing.T) { // the bundle the caller named. func TestWithRootCAsClearsFallbacks(t *testing.T) { roots := x509.NewCertPool() - cfg, err := connectionConfig("postgres://schemabot:secret@postgres.internal.example:5432/app?sslmode=prefer", WithRootCAs(roots)) + cfg, err := connectionConfig("postgres://schemabot:secret@postgres.internal.example:5432/app?sslmode=prefer", WithRootCAs(roots)) // sadscan:disable np.postgres.1 require.NoError(t, err) require.NotNil(t, cfg.TLSConfig) assert.Same(t, roots, cfg.TLSConfig.RootCAs) @@ -357,6 +357,11 @@ func TestConnectionConfigHonorsPGTZ(t *testing.T) { assert.Equal(t, "America/Los_Angeles", cfg.RuntimeParams["timezone"]) } +// The scheduling of credential reloads is pkg/connreload's and is tested +// there. What is tested here is the PostgreSQL-specific half: which error +// means "the server rejected these credentials", and that a reloaded DSN gets +// the same normalization and options as the DSN the pool was opened with. + // stubConn is the minimal driver.Conn a fake dial can hand back. type stubConn struct{} @@ -364,26 +369,50 @@ func (stubConn) Prepare(string) (driver.Stmt, error) { return nil, errors.New("n func (stubConn) Close() error { return nil } func (stubConn) Begin() (driver.Tx, error) { return nil, errors.New("not implemented") } -// fakeDial replaces the connectConfig seam for the test and records the -// password of every dial attempt. Each attempt's outcome comes from results, -// consumed in order. +// fakeConnector dials for one resolved config, recording the password of every +// attempt into the shared log and taking its outcome from results, consumed in +// order across every generation. +type fakeConnector struct { + t *testing.T + password string + passwords *[]string + results []error + mu *sync.Mutex +} + +func (f *fakeConnector) Connect(context.Context) (driver.Conn, error) { + f.mu.Lock() + *f.passwords = append(*f.passwords, f.password) + i := len(*f.passwords) - 1 + f.mu.Unlock() + // assert (not require): this runs on the dialing goroutine, and testify's + // FailNow is only valid on the test goroutine. The error return fails the + // dial cleanly instead. + if !assert.Less(f.t, i, len(f.results), "unexpected extra dial attempt") { + return nil, errors.New("unexpected extra dial attempt") + } + if err := f.results[i]; err != nil { + return nil, err + } + return stubConn{}, nil +} + +func (f *fakeConnector) Driver() driver.Driver { return stubDriver{} } + +type stubDriver struct{} + +func (stubDriver) Open(string) (driver.Conn, error) { return nil, errors.New("not implemented") } + +// fakeDial replaces the getConnector seam and returns the log recording the +// password of every dial attempt, in order. func fakeDial(t *testing.T, results []error) *[]string { t.Helper() var passwords []string - original := connectConfig - t.Cleanup(func() { connectConfig = original }) - connectConfig = func(_ context.Context, cfg pgx.ConnConfig) (driver.Conn, error) { - passwords = append(passwords, cfg.Password) - // assert (not require): this seam runs on the dialing goroutine, and - // testify's FailNow is only valid on the test goroutine. The error - // return fails the dial cleanly instead. - if !assert.Less(t, len(passwords)-1, len(results), "unexpected extra dial attempt") { - return nil, errors.New("unexpected extra dial attempt") - } - if err := results[len(passwords)-1]; err != nil { - return nil, err - } - return stubConn{}, nil + var mu sync.Mutex + original := getConnector + t.Cleanup(func() { getConnector = original }) + getConnector = func(cfg pgx.ConnConfig) driver.Connector { + return &fakeConnector{t: t, password: cfg.Password, passwords: &passwords, results: results, mu: &mu} } return &passwords } @@ -392,198 +421,59 @@ func authError() error { return fmt.Errorf("connect: %w", &pgconn.PgError{Code: "28P01", Message: "password authentication failed"}) } -func newReloadableConnector(t *testing.T, dsn string, reload func() (string, error)) *reloadableConnector { - t.Helper() - cfg, err := connectionConfig(dsn) - require.NoError(t, err) - return &reloadableConnector{cfg: cfg, reload: reload} -} - -func TestReloadableConnectorReloadsOnAuthFailure(t *testing.T) { - passwords := fakeDial(t, []error{authError(), nil, nil}) - var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", func() (string, error) { - reloads.Add(1) - return "postgres://schemabot:rotated@localhost:5432/app", nil - }) - - conn, err := c.Connect(t.Context()) - require.NoError(t, err) - require.NotNil(t, conn) - assert.Equal(t, []string{"old", "rotated"}, *passwords, "retry must dial with the reloaded credentials") - assert.Equal(t, int32(1), reloads.Load()) - - // The reloaded credentials stick for subsequent dials without another reload. - conn, err = c.Connect(t.Context()) - require.NoError(t, err) - require.NotNil(t, conn) - assert.Equal(t, []string{"old", "rotated", "rotated"}, *passwords) - assert.Equal(t, int32(1), reloads.Load()) -} - -func TestReloadableConnectorKeepsCredentialsWhenReloadFails(t *testing.T) { - passwords := fakeDial(t, []error{authError()}) - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", func() (string, error) { - return "", errors.New("secret backend unavailable") - }) - - conn, err := c.Connect(t.Context()) - require.Error(t, err) - assert.Nil(t, conn) - assert.Contains(t, err.Error(), "password authentication failed", "the original auth error surfaces, not the reload error") - assert.Equal(t, []string{"old"}, *passwords, "no retry without fresh credentials") - - cfg, gen := c.snapshot() - assert.Equal(t, "old", cfg.Password, "current credentials are kept") - assert.Equal(t, uint64(0), gen) -} - -func TestReloadableConnectorIgnoresNonAuthErrors(t *testing.T) { - dialErr := errors.New("connection refused") - passwords := fakeDial(t, []error{dialErr}) - reloadCalled := false - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", func() (string, error) { - reloadCalled = true - return "postgres://schemabot:rotated@localhost:5432/app", nil - }) - - _, err := c.Connect(t.Context()) - require.ErrorIs(t, err, dialErr) - assert.False(t, reloadCalled, "a non-authentication failure must not trigger a reload") - assert.Equal(t, []string{"old"}, *passwords) +func TestIsAuthError(t *testing.T) { + assert.True(t, isAuthError(&pgconn.PgError{Code: "28P01"})) + assert.True(t, isAuthError(&pgconn.PgError{Code: "28000"})) + // database/sql wraps, so recognizing a bare error is not enough. + assert.True(t, isAuthError(authError())) + assert.False(t, isAuthError(&pgconn.PgError{Code: "55P03"})) + assert.False(t, isAuthError(errors.New("connection refused"))) + assert.False(t, isAuthError(nil)) } -func TestReloadableConnectorSurfacesRetryFailure(t *testing.T) { - passwords := fakeDial(t, []error{authError(), authError()}) - var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", func() (string, error) { - reloads.Add(1) - return "postgres://schemabot:rotated@localhost:5432/app", nil - }) - - // The reload succeeds but the retry dial is also rejected — for example a - // reloaded secret that is itself stale. The retry's error surfaces and the - // reload runs exactly once for the failed attempt. - conn, err := c.Connect(t.Context()) - require.Error(t, err) - assert.Nil(t, conn) - assert.Contains(t, err.Error(), "password authentication failed") - assert.Equal(t, []string{"old", "rotated"}, *passwords, "the retry dials with the reloaded credentials") - assert.Equal(t, int32(1), reloads.Load()) -} +func TestResolveConnectorAppliesOptionsAndNormalization(t *testing.T) { + // A reloaded raw DSN gets the same treatment as the boot DSN, because both + // go through resolveConnector: caller options are re-applied and an RDS + // host gets sslmode=require injected. The assertions read the config the + // seam is handed, which is what the pool will dial with. + original := getConnector + t.Cleanup(func() { getConnector = original }) + var got pgx.ConnConfig + getConnector = func(cfg pgx.ConnConfig) driver.Connector { + got = cfg + return nil + } -func TestReloadableConnectorReloadReappliesOptionsAndNormalization(t *testing.T) { - // The reloaded raw DSN gets the same treatment as the boot DSN: caller - // options are re-applied and an RDS host gets TLS injected. - opts := []Option{WithConnectTimeout(7 * time.Second)} - cfg, err := connectionConfig("postgres://schemabot:old@localhost:5432/app", opts...) + _, err := resolveConnector( + "postgres://schemabot:rotated@database.cluster-abc123.us-west-2.rds.amazonaws.com:5432/app", // sadscan:disable np.postgres.1 + WithConnectTimeout(7*time.Second), + ) require.NoError(t, err) - c := &reloadableConnector{cfg: cfg, opts: opts, reload: func() (string, error) { - return "postgres://schemabot:rotated@database.cluster-abc123.us-west-2.rds.amazonaws.com:5432/app", nil - }} - - fresh, _, ok := c.refresh(t.Context(), 0) - require.True(t, ok) - assert.Equal(t, "rotated", fresh.Password) - assert.Equal(t, 7*time.Second, fresh.ConnectTimeout, "options must flow through the reload path") - assert.NotNil(t, fresh.TLSConfig, "a reloaded RDS DSN must get sslmode=require injected") + assert.Equal(t, "rotated", got.Password) + assert.Equal(t, 7*time.Second, got.ConnectTimeout, "options must flow through the resolve path") + assert.NotNil(t, got.TLSConfig, "a reloaded RDS DSN must get sslmode=require injected") // sslmode=prefer would also set TLSConfig but keep a plaintext fallback; // require is distinguished by that fallback's absence. - assert.Empty(t, fresh.Fallbacks, "sslmode=require must not leave a plaintext fallback") + assert.Empty(t, got.Fallbacks, "sslmode=require must not leave a plaintext fallback") } -func TestReloadableConnectorRefreshConcurrent(t *testing.T) { +// End to end through pkg/connreload: a pool whose first dial is rejected +// re-resolves its DSN and retries with the rotated password, without the +// caller doing anything. +func TestOpenReloadableRotatesCredentialsOnAuthFailure(t *testing.T) { + passwords := fakeDial(t, []error{authError(), nil}) var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", func() (string, error) { - reloads.Add(1) - return "postgres://schemabot:rotated@localhost:5432/app", nil - }) - // Concurrent dials that failed on the same generation trigger exactly one - // reload; the rest reuse the swapped config. - var wg sync.WaitGroup - for range 8 { - wg.Go(func() { - cfg, _, ok := c.refresh(t.Context(), 0) - assert.True(t, ok) - assert.Equal(t, "rotated", cfg.Password) - }) - } - wg.Wait() - assert.Equal(t, int32(1), reloads.Load(), "concurrent same-generation failures must reload once") -} - -// A reload that succeeds but returns credentials the server still rejects -// arms the cooldown too: without it, each rejected dial advances the -// generation and triggers a fresh resolve — one per new connection — for as -// long as the secret store keeps answering with a refused credential. -func TestReloadableConnectorArmsCooldownWhenReloadedCredentialsRejected(t *testing.T) { - passwords := fakeDial(t, []error{authError(), authError(), authError(), authError(), authError()}) - var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", func() (string, error) { + db, err := OpenReloadable("postgres://schemabot:old@localhost:5432/app", func() (string, error) { // sadscan:disable np.postgres.1 reloads.Add(1) - return "postgres://schemabot:stale@localhost:5432/app", nil + return "postgres://schemabot:rotated@localhost:5432/app", nil // sadscan:disable np.postgres.1 }) - clock := time.Now() - c.now = func() time.Time { return clock } - - // The dial fails, the reload succeeds, and the retry is rejected too: - // the cooldown arms. - _, err := c.Connect(t.Context()) - require.Error(t, err) - require.Equal(t, int32(1), reloads.Load()) - assert.Equal(t, []string{"old", "stale"}, *passwords) - - // The next rejected dial surfaces without another resolve. - _, err = c.Connect(t.Context()) - require.Error(t, err) - assert.Equal(t, int32(1), reloads.Load(), "a rejected reloaded credential must not cost one resolve per connection") - assert.Equal(t, []string{"old", "stale", "stale"}, *passwords) - - // After the window elapses the reload is retried; another rejected retry - // re-arms the cooldown. - clock = clock.Add(reloadCooldown) - _, err = c.Connect(t.Context()) - require.Error(t, err) - assert.Equal(t, int32(2), reloads.Load()) - assert.Equal(t, []string{"old", "stale", "stale", "stale", "stale"}, *passwords) -} + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, db.Close()) }) -func TestReloadableConnectorReloadCooldown(t *testing.T) { - var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", func() (string, error) { - reloads.Add(1) - if reloads.Load() < 3 { - return "", errors.New("secret backend unavailable") - } - return "postgres://schemabot:rotated@localhost:5432/app", nil - }) - clock := time.Now() - c.now = func() time.Time { return clock } - - // The first failed reload arms the cooldown. - _, _, ok := c.refresh(t.Context(), 0) - require.False(t, ok) - require.Equal(t, int32(1), reloads.Load()) - - // Failed dials inside the window surface without reloading again. - _, _, ok = c.refresh(t.Context(), 0) - require.False(t, ok) - assert.Equal(t, int32(1), reloads.Load(), "reload must not run during the cooldown") - - // After the window elapses the reload is retried; another failure re-arms. - clock = clock.Add(reloadCooldown) - _, _, ok = c.refresh(t.Context(), 0) - require.False(t, ok) - require.Equal(t, int32(2), reloads.Load()) - - // A successful reload swaps credentials and clears the cooldown. - clock = clock.Add(reloadCooldown) - cfg, _, ok := c.refresh(t.Context(), 0) - require.True(t, ok) - assert.Equal(t, "rotated", cfg.Password) - require.Equal(t, int32(3), reloads.Load()) - assert.True(t, c.lastReloadFail.IsZero(), "a successful reload must clear the cooldown") + require.NoError(t, db.PingContext(t.Context())) + assert.Equal(t, []string{"old", "rotated"}, *passwords, "the retry must dial with the reloaded credentials") + assert.Equal(t, int32(1), reloads.Load()) } func TestDSNParseErrorsRedactCredentials(t *testing.T) { @@ -604,322 +494,15 @@ func TestDSNParseErrorsRedactCredentials(t *testing.T) { assert.NotContains(t, err.Error(), password) }) - t.Run("refresh with unparseable reloaded DSN", func(t *testing.T) { - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", func() (string, error) { - return badDSN, nil - }) - _, _, ok := c.refresh(t.Context(), 0) - require.False(t, ok) - }) -} - -func TestReloadableConnectorRefreshDedupesConcurrentFailures(t *testing.T) { - var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", func() (string, error) { - reloads.Add(1) - return "postgres://schemabot:rotated@localhost:5432/app", nil - }) - - cfg, _, ok := c.refresh(t.Context(), 0) - require.True(t, ok) - assert.Equal(t, "rotated", cfg.Password) - require.Equal(t, int32(1), reloads.Load()) - - // A dial that failed against the already-superseded generation reuses the - // swapped config instead of reloading again. - cfg, _, ok = c.refresh(t.Context(), 0) - require.True(t, ok) - assert.Equal(t, "rotated", cfg.Password) - assert.Equal(t, int32(1), reloads.Load(), "stale-generation refresh must not reload") -} - -// waitClosed fails the test when ch does not close within a bounded deadline. -func waitClosed(t *testing.T, ch <-chan struct{}, msg string) { - t.Helper() - select { - case <-ch: - case <-time.After(5 * time.Second): - t.Fatal(msg) - } -} - -// hungReload returns a reload callback that signals started, then blocks -// until release closes before returning result and err. -func hungReload(started, release chan struct{}, reloads *atomic.Int32, result string, err error) func() (string, error) { - return func() (string, error) { - reloads.Add(1) - close(started) - <-release - return result, err - } -} - -func TestReloadableConnectorSnapshotNotBlockedByHungReload(t *testing.T) { - started := make(chan struct{}) - release := make(chan struct{}) - var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", - hungReload(started, release, &reloads, "postgres://schemabot:rotated@localhost:5432/app", nil)) - - var wg sync.WaitGroup - wg.Go(func() { - cfg, _, ok := c.refresh(t.Context(), 0) - assert.True(t, ok) - assert.Equal(t, "rotated", cfg.Password) - }) - waitClosed(t, started, "reload never started") - - // The hung reload must not hold the connector mutex: healthy dials keep - // snapshotting the current config while credentials resolve. - snapshotDone := make(chan struct{}) - go func() { - defer close(snapshotDone) - cfg, gen := c.snapshot() - assert.Equal(t, "old", cfg.Password) - assert.Equal(t, uint64(0), gen) - }() - waitClosed(t, snapshotDone, "snapshot blocked behind an in-flight reload") - - close(release) - wg.Wait() - assert.Equal(t, int32(1), reloads.Load()) -} - -func TestReloadableConnectorConnectNotBlockedByHungReload(t *testing.T) { - passwords := fakeDial(t, []error{authError(), nil, nil}) - started := make(chan struct{}) - release := make(chan struct{}) - var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", - hungReload(started, release, &reloads, "postgres://schemabot:rotated@localhost:5432/app", nil)) - - var wg sync.WaitGroup - wg.Go(func() { - conn, err := c.Connect(t.Context()) - assert.NoError(t, err) - assert.NotNil(t, conn) - }) - waitClosed(t, started, "reload never started") - - // A dial that authenticates with the current credentials completes while - // the rejected dial's reload hangs on secret resolution. - connected := make(chan struct{}) - go func() { - defer close(connected) - conn, err := c.Connect(t.Context()) - assert.NoError(t, err) - assert.NotNil(t, conn) - }() - waitClosed(t, connected, "healthy dial blocked behind an in-flight reload") - - close(release) - wg.Wait() - assert.Equal(t, []string{"old", "old", "rotated"}, *passwords) - assert.Equal(t, int32(1), reloads.Load()) -} - -func TestReloadableConnectorRefreshWaiterRespectsDialContext(t *testing.T) { - started := make(chan struct{}) - release := make(chan struct{}) - var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", - hungReload(started, release, &reloads, "postgres://schemabot:rotated@localhost:5432/app", nil)) - - var wg sync.WaitGroup - wg.Go(func() { - cfg, _, ok := c.refresh(t.Context(), 0) - assert.True(t, ok) - assert.Equal(t, "rotated", cfg.Password) - }) - waitClosed(t, started, "reload never started") - - // A waiter whose dial context ends while the leader's reload hangs gives - // up and surfaces its dial error instead of blocking indefinitely. - ctx, cancel := context.WithCancel(t.Context()) - waiterDone := make(chan struct{}) - go func() { - defer close(waiterDone) - cfg, _, ok := c.refresh(ctx, 0) - assert.False(t, ok) - assert.Nil(t, cfg) - }() - cancel() - waitClosed(t, waiterDone, "waiter did not honor its dial context") - - close(release) - wg.Wait() - assert.Equal(t, int32(1), reloads.Load(), "the canceled waiter must not trigger its own reload") -} - -// The dial that elects a reload is not pinned by it: the reload runs -// detached, so cancelling the electing dial's context returns it promptly -// with its dial error — it cannot hold a pool connection slot for as long as -// a hung secret resolution takes — while the reload finishes in the -// background and publishes the rotated credentials for later dials. -func TestReloadableConnectorRefreshElectingDialRespectsDialContext(t *testing.T) { - started := make(chan struct{}) - release := make(chan struct{}) - var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", - hungReload(started, release, &reloads, "postgres://schemabot:rotated@localhost:5432/app", nil)) - - ctx, cancel := context.WithCancel(t.Context()) - electorDone := make(chan struct{}) - go func() { - defer close(electorDone) - cfg, _, ok := c.refresh(ctx, 0) - assert.False(t, ok) - assert.Nil(t, cfg) - }() - waitClosed(t, started, "reload never started") - cancel() - waitClosed(t, electorDone, "the electing dial did not honor its own dial context") - - // The detached reload finishes and publishes: a later same-generation - // refresh picks up the rotated credentials without reloading again. - close(release) - cfg, _, ok := c.refresh(t.Context(), 0) - require.True(t, ok) - assert.Equal(t, "rotated", cfg.Password) - assert.Equal(t, int32(1), reloads.Load(), "the abandoned reload's outcome must be reused, not re-resolved") -} - -func TestReloadableConnectorRefreshWaiterObservesLeaderFailure(t *testing.T) { - started := make(chan struct{}) - release := make(chan struct{}) - var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", - hungReload(started, release, &reloads, "", errors.New("secret backend unavailable"))) - - // The clock seam doubles as an entered-refresh gate: an expired cooldown - // stamp makes every refresh iteration consult the clock under the mutex, - // so the second consult is the waiter's. Once past it, the waiter can - // only reach the select on the leader's done channel — releasing the - // leader after that pins the wake-then-recheck path instead of leaving - // it to the scheduler. - c.lastReloadFail = time.Now().Add(-2 * reloadCooldown) - var clockCalls atomic.Int32 - waiterEntered := make(chan struct{}) - c.now = func() time.Time { - if clockCalls.Add(1) == 2 { - close(waiterEntered) - } - return time.Now() - } - - var wg sync.WaitGroup - wg.Go(func() { - _, _, ok := c.refresh(t.Context(), 0) - assert.False(t, ok) - }) - waitClosed(t, started, "reload never started") - - // A same-generation waiter observes the leader's failed reload through - // the armed cooldown and reports failure without reloading again. - waiterDone := make(chan struct{}) - go func() { - defer close(waiterDone) - cfg, _, ok := c.refresh(t.Context(), 0) - assert.False(t, ok) - assert.Nil(t, cfg) - }() - waitClosed(t, waiterEntered, "waiter never entered refresh") - close(release) - wg.Wait() - waitClosed(t, waiterDone, "waiter did not observe the leader's failed reload") - assert.Equal(t, int32(1), reloads.Load(), "the waiter must not reload during the cooldown the failure armed") -} - -// A reload callback that panics must not wedge the connector or crash the -// process: the recover in the publish defer converts the panic into a failed -// reload — the reloading guard is released, waiters are unblocked, the -// cooldown is armed, and the current credentials are kept. -func TestReloadableConnectorReloadPanicUnblocksWaiters(t *testing.T) { - started := make(chan struct{}) - release := make(chan struct{}) - var reloads atomic.Int32 - c := newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", func() (string, error) { - reloads.Add(1) - close(started) - <-release - panic("secret backend panicked") - }) - - var wg sync.WaitGroup - wg.Go(func() { - cfg, _, ok := c.refresh(t.Context(), 0) - assert.False(t, ok, "a panicking reload must surface as a failed reload") - assert.Nil(t, cfg) + t.Run("OpenReloadable", func(t *testing.T) { + _, err := OpenReloadable(badDSN, func() (string, error) { return "", nil }) + require.Error(t, err) + assert.NotContains(t, err.Error(), password) }) - waitClosed(t, started, "reload never started") - - // A same-generation waiter blocked on the reload's outcome is unblocked - // by the publish defer and observes the armed cooldown. - waiterDone := make(chan struct{}) - go func() { - defer close(waiterDone) - cfg, _, ok := c.refresh(t.Context(), 0) - assert.False(t, ok) - assert.Nil(t, cfg) - }() - close(release) - wg.Wait() - waitClosed(t, waiterDone, "waiter was not unblocked by the panicking reload") - - // The guard is released and the cooldown armed: a later same-generation - // refresh backs off without reloading rather than deadlocking on a stale - // guard. - _, _, ok := c.refresh(t.Context(), 0) - assert.False(t, ok) - assert.Equal(t, int32(1), reloads.Load(), "the panicked reload must arm the cooldown; no further reloads") - c.mu.Lock() - defer c.mu.Unlock() - assert.Nil(t, c.reloading, "the reloading guard must be released after a panic") -} -// A rejection verdict on superseded credentials must not arm the cooldown: -// while a retry dial is in flight, another rotation can swap in newer -// credentials, and stamping the late rejection onto that newer generation -// would suppress its legitimate reloads for a full window. -func TestReloadableConnectorStaleRejectionDoesNotArmCooldown(t *testing.T) { - var c *reloadableConnector - original := connectConfig - t.Cleanup(func() { connectConfig = original }) - var dials atomic.Int32 - connectConfig = func(_ context.Context, _ pgx.ConnConfig) (driver.Conn, error) { - switch dials.Add(1) { - case 1: - // The initial dial is rejected, triggering the reload. - return nil, authError() - case 2: - // While the retry with reloaded credentials is in flight, a - // concurrent rotation advances the generation; the retry then - // comes back rejected — a verdict on already-superseded - // credentials. - c.mu.Lock() - c.gen++ - c.mu.Unlock() - return nil, authError() - default: - return stubConn{}, nil - } - } - c = newReloadableConnector(t, "postgres://schemabot:old@localhost:5432/app", func() (string, error) { - return "postgres://schemabot:rotated@localhost:5432/app", nil + t.Run("resolveConnector", func(t *testing.T) { + _, err := resolveConnector(badDSN) + require.Error(t, err) + assert.NotContains(t, err.Error(), password) }) - - _, err := c.Connect(t.Context()) - require.Error(t, err) - c.mu.Lock() - defer c.mu.Unlock() - assert.True(t, c.lastReloadFail.IsZero(), "a stale rejection must not arm the cooldown against the newer generation") -} - -func TestIsAuthError(t *testing.T) { - assert.True(t, isAuthError(&pgconn.PgError{Code: "28P01"})) - assert.True(t, isAuthError(&pgconn.PgError{Code: "28000"})) - assert.True(t, isAuthError(fmt.Errorf("wrapped: %w", &pgconn.PgError{Code: "28P01"}))) - assert.False(t, isAuthError(&pgconn.PgError{Code: "55P03"})) - assert.False(t, isAuthError(errors.New("connection refused"))) - assert.False(t, isAuthError(nil)) } diff --git a/pkg/serve/serve.go b/pkg/serve/serve.go index 67d6288dd..6918b69eb 100644 --- a/pkg/serve/serve.go +++ b/pkg/serve/serve.go @@ -21,8 +21,8 @@ import ( "syscall" "time" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "google.golang.org/grpc" diff --git a/pkg/serve/serve_close_test.go b/pkg/serve/serve_close_test.go index 844b99e28..c73462160 100644 --- a/pkg/serve/serve_close_test.go +++ b/pkg/serve/serve_close_test.go @@ -8,8 +8,8 @@ import ( "net/http/httptest" "testing" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -37,7 +37,7 @@ func TestServerCloseSucceedsWhenTelemetryFlushFails(t *testing.T) { // A lazily-opened handle: never connected, so svc.Close can close it // without a reachable database. - db, err := sql.Open("mysql", "schemabot@tcp(127.0.0.1:1)/schemabot") + db, err := sql.Open("block-mysql", "schemabot@tcp(127.0.0.1:1)/schemabot") require.NoError(t, err) // srv.Close (via svc.Close) owns the handle; this cleanup only prevents a // leak when the test fails before Close runs. diff --git a/pkg/storage/internal/sqlstore/applies_test.go b/pkg/storage/internal/sqlstore/applies_test.go index 872f8962e..e6bcd6953 100644 --- a/pkg/storage/internal/sqlstore/applies_test.go +++ b/pkg/storage/internal/sqlstore/applies_test.go @@ -3176,7 +3176,7 @@ func TestApplyStore_ClaimApplyByIDConcurrentPendingClaims(t *testing.T) { const drivers = 16 stores := make([]*Storage, drivers) for i := range drivers { - db, openErr := sql.Open("mysql", testDSNChangedRows) + db, openErr := sql.Open("block-mysql", testDSNChangedRows) require.NoError(t, openErr) db.SetMaxOpenConns(1) db.SetMaxIdleConns(1) @@ -3958,7 +3958,7 @@ func createTestApplyWithStateEnvDeployment(t *testing.T, store *Storage, lock *s // DB error tests func TestApplyStore_Create_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -3971,7 +3971,7 @@ func TestApplyStore_Create_DBError(t *testing.T) { } func TestApplyStore_Get_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -3981,7 +3981,7 @@ func TestApplyStore_Get_DBError(t *testing.T) { } func TestApplyStore_GetByApplyIdentifier_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -3991,7 +3991,7 @@ func TestApplyStore_GetByApplyIdentifier_DBError(t *testing.T) { } func TestApplyStore_GetByLock_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4001,7 +4001,7 @@ func TestApplyStore_GetByLock_DBError(t *testing.T) { } func TestApplyStore_GetInProgress_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4011,7 +4011,7 @@ func TestApplyStore_GetInProgress_DBError(t *testing.T) { } func TestApplyStore_Update_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4021,7 +4021,7 @@ func TestApplyStore_Update_DBError(t *testing.T) { } func TestApplyStore_Delete_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4031,7 +4031,7 @@ func TestApplyStore_Delete_DBError(t *testing.T) { } func TestApplyStore_DeleteByPR_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4041,7 +4041,7 @@ func TestApplyStore_DeleteByPR_DBError(t *testing.T) { } func TestApplyStore_GetByDatabase_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4051,7 +4051,7 @@ func TestApplyStore_GetByDatabase_DBError(t *testing.T) { } func TestApplyStore_GetByPR_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4061,7 +4061,7 @@ func TestApplyStore_GetByPR_DBError(t *testing.T) { } func TestApplyStore_GetByPlan_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) diff --git a/pkg/storage/internal/sqlstore/apply_operations_test.go b/pkg/storage/internal/sqlstore/apply_operations_test.go index ade584eb2..0faf20c29 100644 --- a/pkg/storage/internal/sqlstore/apply_operations_test.go +++ b/pkg/storage/internal/sqlstore/apply_operations_test.go @@ -1604,7 +1604,7 @@ func TestApplyOperationStore_FindNextApplyOperation_ConcurrentClaimsStoppedWithP const drivers = 16 stores := make([]*Storage, drivers) for i := range drivers { - db, openErr := sql.Open("mysql", testDSNChangedRows) + db, openErr := sql.Open("block-mysql", testDSNChangedRows) require.NoError(t, openErr) db.SetMaxOpenConns(1) db.SetMaxIdleConns(1) @@ -2475,7 +2475,7 @@ func TestApplyOperationStore_FindNextApplyOperation_ConcurrentClaims(t *testing. const drivers = 16 stores := make([]*Storage, drivers) for i := range drivers { - db, openErr := sql.Open("mysql", testDSNChangedRows) + db, openErr := sql.Open("block-mysql", testDSNChangedRows) require.NoError(t, openErr) db.SetMaxOpenConns(1) db.SetMaxIdleConns(1) @@ -2643,7 +2643,7 @@ func TestApplyOperationStore_FindNextApplyOperation_ConcurrentDriversClaimDistin const drivers = 16 stores := make([]*Storage, drivers) for i := range drivers { - db, openErr := sql.Open("mysql", testDSNChangedRows) + db, openErr := sql.Open("block-mysql", testDSNChangedRows) require.NoError(t, openErr) db.SetMaxOpenConns(1) db.SetMaxIdleConns(1) @@ -4760,7 +4760,7 @@ func assertApplyOperationState(t *testing.T, store *Storage, id int64, expected } func TestApplyOperationStore_Heartbeat_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4802,7 +4802,7 @@ func TestApplyOperationStore_DeleteByApply(t *testing.T) { // DB error tests — mirror the pattern used by apply_comments_test.go. func TestApplyOperationStore_Insert_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4814,7 +4814,7 @@ func TestApplyOperationStore_Insert_DBError(t *testing.T) { } func TestApplyOperationStore_Get_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4824,7 +4824,7 @@ func TestApplyOperationStore_Get_DBError(t *testing.T) { } func TestApplyOperationStore_GetByApplyAndDeployment_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4834,7 +4834,7 @@ func TestApplyOperationStore_GetByApplyAndDeployment_DBError(t *testing.T) { } func TestApplyOperationStore_ListByApply_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4844,7 +4844,7 @@ func TestApplyOperationStore_ListByApply_DBError(t *testing.T) { } func TestApplyOperationStore_UpdateState_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4854,7 +4854,7 @@ func TestApplyOperationStore_UpdateState_DBError(t *testing.T) { } func TestApplyOperationStore_MarkStarted_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4864,7 +4864,7 @@ func TestApplyOperationStore_MarkStarted_DBError(t *testing.T) { } func TestApplyOperationStore_MarkCompleted_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4874,7 +4874,7 @@ func TestApplyOperationStore_MarkCompleted_DBError(t *testing.T) { } func TestApplyOperationStore_MarkFailed_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) @@ -4884,7 +4884,7 @@ func TestApplyOperationStore_MarkFailed_DBError(t *testing.T) { } func TestApplyOperationStore_DeleteByApply_DBError(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) diff --git a/pkg/storage/internal/sqlstore/checks_test.go b/pkg/storage/internal/sqlstore/checks_test.go index 290e89c14..c29142f8e 100644 --- a/pkg/storage/internal/sqlstore/checks_test.go +++ b/pkg/storage/internal/sqlstore/checks_test.go @@ -119,7 +119,7 @@ func TestCheckStore_RepeatedPlanResultLandsUnderChangedRows(t *testing.T) { func newChangedRowsStore(t *testing.T) *Storage { t.Helper() - db, err := sql.Open("mysql", testDSNChangedRows) + db, err := sql.Open("block-mysql", testDSNChangedRows) require.NoError(t, err) require.NoError(t, db.PingContext(t.Context())) t.Cleanup(func() { diff --git a/pkg/storage/internal/sqlstore/error_classifier.go b/pkg/storage/internal/sqlstore/error_classifier.go index f6d8f6e5f..bf10e82bc 100644 --- a/pkg/storage/internal/sqlstore/error_classifier.go +++ b/pkg/storage/internal/sqlstore/error_classifier.go @@ -4,7 +4,7 @@ import ( "errors" "strings" - gomysql "github.com/go-sql-driver/mysql" + "github.com/block/schemabot/pkg/mysqlerr" "github.com/jackc/pgx/v5/pgconn" ) @@ -36,17 +36,19 @@ func NewMySQLErrorClassifier() ErrorClassifier { return mysqlErrorClassifier{} } +// The codes are read through mysqlerr.Number rather than by asserting a +// driver's error type. Asserting a type is what silently broke here before: the +// credential-reloading storage pool went through a hot-swap DSN driver that +// embedded upstream go-sql-driver, so it returned a *mysql.MySQLError of a type +// no errors.As against block/mysql's could match — and the failure mode was +// every deadlock from that pool classified as non-retryable, not an error +// anyone would see. See mysqlerr.Number. func (mysqlErrorClassifier) IsRetryableConflict(err error) bool { - var mysqlErr *gomysql.MySQLError - if !errors.As(err, &mysqlErr) { - return false - } - return mysqlErr.Number == mysqlErrDeadlock || mysqlErr.Number == mysqlErrLockWaitTimeout + return mysqlerr.Is(err, mysqlErrDeadlock, mysqlErrLockWaitTimeout) } func (mysqlErrorClassifier) IsDuplicateKey(err error) bool { - var mysqlErr *gomysql.MySQLError - if errors.As(err, &mysqlErr) && mysqlErr.Number == mysqlErrDuplicateKey { + if mysqlerr.Is(err, mysqlErrDuplicateKey) { return true } // Defend against driver errors flattened to strings with %v in a call path. diff --git a/pkg/storage/internal/sqlstore/error_classifier_test.go b/pkg/storage/internal/sqlstore/error_classifier_test.go index 02ce3b4d1..965f52242 100644 --- a/pkg/storage/internal/sqlstore/error_classifier_test.go +++ b/pkg/storage/internal/sqlstore/error_classifier_test.go @@ -5,7 +5,7 @@ import ( "fmt" "testing" - gomysql "github.com/go-sql-driver/mysql" + gomysql "github.com/block/mysql" "github.com/jackc/pgx/v5/pgconn" "github.com/stretchr/testify/assert" ) diff --git a/pkg/storage/internal/sqlstore/locks_test.go b/pkg/storage/internal/sqlstore/locks_test.go index 471922071..b47f50c1a 100644 --- a/pkg/storage/internal/sqlstore/locks_test.go +++ b/pkg/storage/internal/sqlstore/locks_test.go @@ -8,7 +8,7 @@ import ( "sync" "testing" - _ "github.com/go-sql-driver/mysql" + _ "github.com/block/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -26,7 +26,7 @@ func TestLockStore_Acquire_SameOwnerConcurrent(t *testing.T) { const drivers = 16 stores := make([]*Storage, drivers) for i := range drivers { - db, openErr := sql.Open("mysql", testDSNChangedRows) + db, openErr := sql.Open("block-mysql", testDSNChangedRows) require.NoError(t, openErr) db.SetMaxOpenConns(1) db.SetMaxIdleConns(1) @@ -83,7 +83,7 @@ func TestLockStore_Acquire_RefreshSameOwnerValueAlreadyMatches(t *testing.T) { clearTables(t) ctx := t.Context() - db, err := sql.Open("mysql", testDSNChangedRows) + db, err := sql.Open("block-mysql", testDSNChangedRows) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, db.Close()) @@ -267,7 +267,7 @@ func TestLockStore_UpdateSameSecondSucceeds(t *testing.T) { // Pin to a single connection so the frozen session timestamp persists across // the seed INSERT, the touch UPDATE, and the re-read. - db, err := sql.Open("mysql", testDSNChangedRows) + db, err := sql.Open("block-mysql", testDSNChangedRows) require.NoError(t, err) db.SetMaxOpenConns(1) db.SetMaxIdleConns(1) @@ -305,7 +305,7 @@ func TestLockStore_UpdateSameSecondSucceeds(t *testing.T) { } func TestStorage_Close(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) store := NewMySQL(db) @@ -326,7 +326,7 @@ func TestStorage_Ping(t *testing.T) { } func TestStorage_Ping_Error(t *testing.T) { - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) diff --git a/pkg/storage/internal/sqlstore/mysql_test.go b/pkg/storage/internal/sqlstore/mysql_test.go index c5f0c9c8c..131fbc3c8 100644 --- a/pkg/storage/internal/sqlstore/mysql_test.go +++ b/pkg/storage/internal/sqlstore/mysql_test.go @@ -9,8 +9,8 @@ import ( "os" "testing" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/require" "github.com/testcontainers/testcontainers-go" @@ -53,7 +53,7 @@ func TestMain(m *testing.M) { os.Exit(1) } - testDB, err = sql.Open("mysql", testDSN) + testDB, err = sql.Open("block-mysql", testDSN) if err != nil { fmt.Fprintf(os.Stderr, "Failed to connect to MySQL: %v\n", err) os.Exit(1) diff --git a/pkg/storage/internal/sqlstore/parity_test.go b/pkg/storage/internal/sqlstore/parity_test.go index e016a8f22..dcc8cf4d1 100644 --- a/pkg/storage/internal/sqlstore/parity_test.go +++ b/pkg/storage/internal/sqlstore/parity_test.go @@ -6,7 +6,7 @@ import ( "database/sql" "testing" - _ "github.com/go-sql-driver/mysql" + _ "github.com/block/mysql" "github.com/stretchr/testify/require" "github.com/block/schemabot/pkg/storage" @@ -25,7 +25,7 @@ func (mysqlHarness) NewStorage(t *testing.T) storage.Storage { func (mysqlHarness) NewUnreachableStorage(t *testing.T) storage.Storage { t.Helper() - db, err := sql.Open("mysql", testDSN) + db, err := sql.Open("block-mysql", testDSN) require.NoError(t, err) require.NoError(t, db.Close()) return NewMySQL(db) diff --git a/pkg/storage/internal/sqlstore/retry_test.go b/pkg/storage/internal/sqlstore/retry_test.go index 72fbb0294..d4b397674 100644 --- a/pkg/storage/internal/sqlstore/retry_test.go +++ b/pkg/storage/internal/sqlstore/retry_test.go @@ -6,7 +6,7 @@ import ( "fmt" "testing" - gomysql "github.com/go-sql-driver/mysql" + gomysql "github.com/block/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) diff --git a/pkg/storage/internal/sqlstore/webhook_events_test.go b/pkg/storage/internal/sqlstore/webhook_events_test.go index 479a89186..d0313e120 100644 --- a/pkg/storage/internal/sqlstore/webhook_events_test.go +++ b/pkg/storage/internal/sqlstore/webhook_events_test.go @@ -9,8 +9,8 @@ import ( "testing" "time" + _ "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -403,7 +403,7 @@ func TestWebhookEventStore_HeartbeatTreatsUnchangedMatchingLeaseAsSuccess(t *tes clearTables(t) ctx := t.Context() - db, err := sql.Open("mysql", testDSNChangedRows) + db, err := sql.Open("block-mysql", testDSNChangedRows) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) t.Cleanup(func() { require.NoError(t, db.Close()) }) @@ -435,7 +435,7 @@ func TestWebhookEventStore_TerminalWritesAreIdempotentOnRetry(t *testing.T) { clearTables(t) ctx := t.Context() - db, err := sql.Open("mysql", testDSNChangedRows) + db, err := sql.Open("block-mysql", testDSNChangedRows) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) t.Cleanup(func() { require.NoError(t, db.Close()) }) diff --git a/pkg/tern/grpc_control_rejection_integration_test.go b/pkg/tern/grpc_control_rejection_integration_test.go index ac1d90f64..d4cbf71ad 100644 --- a/pkg/tern/grpc_control_rejection_integration_test.go +++ b/pkg/tern/grpc_control_rejection_integration_test.go @@ -83,7 +83,7 @@ func dispatchQueuedApplyWithOptions(t *testing.T, stor storage.Storage, client * // test whose stub engine stands in for a real one sets the column directly. func setApplyEngine(t *testing.T, dsn string, applyID int64, engine string) { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open database to set the apply engine") defer utils.CloseAndLog(db) _, err = db.ExecContext(t.Context(), "UPDATE applies SET engine = ? WHERE id = ?", engine, applyID) @@ -374,7 +374,7 @@ func failLocallyQueuedControlRequest(t *testing.T, dsn string, stor storage.Stor }) require.NoError(t, err, "queue the %s control request", operation) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open database to settle the control request") defer utils.CloseAndLog(db) _, err = db.ExecContext(t.Context(), ` diff --git a/pkg/tern/grpc_retryable_pause_integration_test.go b/pkg/tern/grpc_retryable_pause_integration_test.go index be50fe523..73926da42 100644 --- a/pkg/tern/grpc_retryable_pause_integration_test.go +++ b/pkg/tern/grpc_retryable_pause_integration_test.go @@ -13,8 +13,8 @@ import ( "testing" "time" + drivermysql "github.com/block/mysql" "github.com/block/spirit/pkg/utils" - drivermysql "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc" @@ -108,7 +108,7 @@ func createControlPlaneStorage(t *testing.T, dpDSN string) storage.Storage { adminCfg := *cfg adminCfg.DBName = "" - adminDB, err := sql.Open("mysql", adminCfg.FormatDSN()) + adminDB, err := sql.Open("block-mysql", adminCfg.FormatDSN()) require.NoError(t, err, "open admin connection for control-plane storage database") defer utils.CloseAndLog(adminDB) require.NoError(t, adminDB.PingContext(t.Context()), "ping admin connection") @@ -120,7 +120,7 @@ func createControlPlaneStorage(t *testing.T, dpDSN string) storage.Storage { bootCfg := *cfg bootCfg.DBName = databaseName bootCfg.MultiStatements = true - bootDB, err := sql.Open("mysql", bootCfg.FormatDSN()) + bootDB, err := sql.Open("block-mysql", bootCfg.FormatDSN()) require.NoError(t, err, "open bootstrap connection for control-plane storage") defer utils.CloseAndLog(bootDB) require.NoError(t, bootDB.PingContext(t.Context()), "ping bootstrap connection") @@ -138,7 +138,7 @@ func createControlPlaneStorage(t *testing.T, dpDSN string) storage.Storage { storeCfg := *cfg storeCfg.DBName = databaseName - db, err := sql.Open("mysql", storeCfg.FormatDSN()) + db, err := sql.Open("block-mysql", storeCfg.FormatDSN()) require.NoError(t, err, "open control-plane storage connection") require.NoError(t, db.PingContext(t.Context()), "ping control-plane storage connection") return mysqlstore.New(db) diff --git a/pkg/tern/local_apply_adopt_integration_test.go b/pkg/tern/local_apply_adopt_integration_test.go index 62050748d..946433c2e 100644 --- a/pkg/tern/local_apply_adopt_integration_test.go +++ b/pkg/tern/local_apply_adopt_integration_test.go @@ -39,7 +39,7 @@ func newAdoptTestFixture(t *testing.T, desired map[string]string) *adoptTestFixt cleanupTasks(t, dsn) cleanupTestTables(t, dsn) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) t.Cleanup(func() { utils.CloseAndLog(db) }) require.NoError(t, db.PingContext(t.Context())) diff --git a/pkg/tern/local_client.go b/pkg/tern/local_client.go index c129b9504..1ae6faf61 100644 --- a/pkg/tern/local_client.go +++ b/pkg/tern/local_client.go @@ -99,10 +99,10 @@ import ( "sync" "time" + "github.com/block/mysql" "github.com/block/spirit/pkg/statement" spirittable "github.com/block/spirit/pkg/table" "github.com/block/spirit/pkg/utils" - "github.com/go-sql-driver/mysql" ps "github.com/planetscale/planetscale-go/planetscale" "github.com/block/schemabot/pkg/ddl" diff --git a/pkg/tern/local_client_integration_test.go b/pkg/tern/local_client_integration_test.go index 8b08f2dbd..b6db8ba6e 100644 --- a/pkg/tern/local_client_integration_test.go +++ b/pkg/tern/local_client_integration_test.go @@ -13,9 +13,9 @@ import ( "testing" "time" + drivermysql "github.com/block/mysql" "github.com/block/spirit/pkg/table" "github.com/block/spirit/pkg/utils" - drivermysql "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/testcontainers/testcontainers-go/modules/mysql" @@ -63,7 +63,7 @@ func TestMain(m *testing.M) { } // Wait for MySQL to be ready - db, err := sql.Open("mysql", sharedDSN) + db, err := sql.Open("block-mysql", sharedDSN) if err != nil { _ = sharedContainer.Terminate(ctx) log.Fatalf("failed to open database: %v", err) @@ -102,7 +102,7 @@ func setupMySQLContainer(t *testing.T) (*mysql.MySQLContainer, string) { // cleanupTestTables removes test tables to avoid conflicts between tests func cleanupTestTables(t *testing.T, dsn string) { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database for cleanup") defer utils.CloseAndLog(db) @@ -117,7 +117,7 @@ func cleanupTestTables(t *testing.T, dsn string) { // This is needed because tasks from previous tests can affect tests that expect no active schema change. func cleanupTasks(t *testing.T, dsn string) { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database for task cleanup") defer utils.CloseAndLog(db) @@ -188,7 +188,7 @@ func setupStorageSchema(t *testing.T, dsn string) { // Requires setupStorageSchema to have been called first. func createStorage(t *testing.T, dsn string) storage.Storage { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database for storage") return mysqlstore.New(db) } @@ -203,7 +203,7 @@ func createStorage(t *testing.T, dsn string) storage.Storage { func buildSchemaWithAllTables(t *testing.T, dsn string, testTableSchemas map[string]string) map[string]string { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database for schema building") defer utils.CloseAndLog(db) @@ -571,7 +571,7 @@ func TestLocalClient_PullSchemaLoadsLiveMySQLSchema(t *testing.T) { container, dsn := setupMySQLContainer(t) _ = container // container is managed by TestMain - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open database") defer utils.CloseAndLog(db) @@ -580,7 +580,7 @@ func TestLocalClient_PullSchemaLoadsLiveMySQLSchema(t *testing.T) { t.Cleanup(func() { cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(t.Context()), 30*time.Second) defer cancel() - cleanupDB, cleanupErr := sql.Open("mysql", dsn) + cleanupDB, cleanupErr := sql.Open("block-mysql", dsn) require.NoError(t, cleanupErr, "open database for pull schema cleanup") defer utils.CloseAndLog(cleanupDB) _, cleanupErr = cleanupDB.ExecContext(cleanupCtx, "DROP TABLE IF EXISTS `pull_schema_users`, `pull_schema_users_archive_2026_06_12`") @@ -689,7 +689,7 @@ func TestLocalClient_PullSchemaCatalogForeignKeysAndGeneratedColumns(t *testing. container, dsn := setupMySQLContainer(t) _ = container - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open database") defer utils.CloseAndLog(db) @@ -700,7 +700,7 @@ func TestLocalClient_PullSchemaCatalogForeignKeysAndGeneratedColumns(t *testing. t.Cleanup(func() { cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(t.Context()), 30*time.Second) defer cancel() - cleanupDB, cleanupErr := sql.Open("mysql", dsn) + cleanupDB, cleanupErr := sql.Open("block-mysql", dsn) require.NoError(t, cleanupErr, "open database for catalog cleanup") defer utils.CloseAndLog(cleanupDB) _, cleanupErr = cleanupDB.ExecContext(cleanupCtx, dropStmt) @@ -776,7 +776,7 @@ func TestLocalClient_PullSchemaDiscoversNonReservedNamespaces(t *testing.T) { container, dsn := setupMySQLContainer(t) _ = container // container is managed by TestMain - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open database") defer utils.CloseAndLog(db) @@ -794,7 +794,7 @@ func TestLocalClient_PullSchemaDiscoversNonReservedNamespaces(t *testing.T) { t.Cleanup(func() { cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(t.Context()), 30*time.Second) defer cancel() - cleanupDB, cleanupErr := sql.Open("mysql", dsn) + cleanupDB, cleanupErr := sql.Open("block-mysql", dsn) require.NoError(t, cleanupErr, "open database for namespace discovery cleanup") defer utils.CloseAndLog(cleanupDB) for _, stmt := range []string{ @@ -850,7 +850,7 @@ func TestLocalClient_PullSchemaOverridesReadPhysicalSchema(t *testing.T) { container, dsn := setupMySQLContainer(t) _ = container // container is managed by TestMain - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open database") defer utils.CloseAndLog(db) @@ -874,7 +874,7 @@ func TestLocalClient_PullSchemaOverridesReadPhysicalSchema(t *testing.T) { t.Cleanup(func() { cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(t.Context()), 30*time.Second) defer cancel() - cleanupDB, cleanupErr := sql.Open("mysql", dsn) + cleanupDB, cleanupErr := sql.Open("block-mysql", dsn) require.NoError(t, cleanupErr, "open database for schema override pull cleanup") defer utils.CloseAndLog(cleanupDB) for _, stmt := range []string{ @@ -964,7 +964,7 @@ func TestLocalClient_ApplySchemaOverridesTargetPhysicalSchema(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open database") defer utils.CloseAndLog(db) @@ -987,7 +987,7 @@ func TestLocalClient_ApplySchemaOverridesTargetPhysicalSchema(t *testing.T) { t.Cleanup(func() { cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(t.Context()), 30*time.Second) defer cancel() - cleanupDB, cleanupErr := sql.Open("mysql", dsn) + cleanupDB, cleanupErr := sql.Open("block-mysql", dsn) require.NoError(t, cleanupErr, "open database for schema override apply cleanup") defer utils.CloseAndLog(cleanupDB) for _, stmt := range []string{ @@ -1062,7 +1062,7 @@ func TestLocalClient_Plan(t *testing.T) { ctx := t.Context() // Create initial table - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database") defer utils.CloseAndLog(db) @@ -1184,7 +1184,7 @@ func TestLocalClient_Apply(t *testing.T) { ctx := t.Context() // Create initial table - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database") defer utils.CloseAndLog(db) @@ -1260,7 +1260,7 @@ func TestLocalClient_Apply_IdempotentDispatch(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database") defer utils.CloseAndLog(db) @@ -1365,7 +1365,7 @@ func TestLocalClient_Apply_WritesApplyOperationRow(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database") defer utils.CloseAndLog(db) @@ -2173,7 +2173,7 @@ func TestLocalClient_ResumeApplyGroupedFinalSchemaCheckCompletesWithoutReapply(t require.NoError(t, err) assert.Nil(t, pendingStart) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(ctx)) @@ -2210,7 +2210,7 @@ func TestLocalClient_ResumeApplyDeferredCutoverRecoveryPreservesCutoverReadyStor stor := createStorage(t, dsn) defer utils.CloseAndLog(stor) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(ctx)) @@ -2220,7 +2220,7 @@ func TestLocalClient_ResumeApplyDeferredCutoverRecoveryPreservesCutoverReadyStor require.NoError(t, err) t.Cleanup(func() { cleanupCtx := context.WithoutCancel(t.Context()) - cleanupDB, cleanupErr := sql.Open("mysql", dsn) + cleanupDB, cleanupErr := sql.Open("block-mysql", dsn) require.NoError(t, cleanupErr) defer utils.CloseAndLog(cleanupDB) _, cleanupErr = cleanupDB.ExecContext(cleanupCtx, "DROP TABLE IF EXISTS `_spirit_sentinel`") @@ -2445,7 +2445,7 @@ func TestLocalClient_ResumeApplyDeferredCutoverFailureMarksApplyRetryable(t *tes stor := createStorage(t, dsn) defer utils.CloseAndLog(stor) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(ctx)) @@ -2455,7 +2455,7 @@ func TestLocalClient_ResumeApplyDeferredCutoverFailureMarksApplyRetryable(t *tes require.NoError(t, err) t.Cleanup(func() { cleanupCtx := context.WithoutCancel(t.Context()) - cleanupDB, cleanupErr := sql.Open("mysql", dsn) + cleanupDB, cleanupErr := sql.Open("block-mysql", dsn) require.NoError(t, cleanupErr) defer utils.CloseAndLog(cleanupDB) _, cleanupErr = cleanupDB.ExecContext(cleanupCtx, "DROP TABLE IF EXISTS `_spirit_sentinel`") @@ -2573,7 +2573,7 @@ func TestLocalClient_ResumeApplyDeferredCutoverAbsentSentinelReconcilesCompleted stor := createStorage(t, dsn) defer utils.CloseAndLog(stor) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(ctx)) @@ -2684,7 +2684,7 @@ func TestLocalClient_ResumeApplyDeferredCutoverAbsentSentinelFailsWhenWorkRemain stor := createStorage(t, dsn) defer utils.CloseAndLog(stor) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(ctx)) @@ -2927,7 +2927,7 @@ func TestLocalClient_ResumeApplyGroupedStartRequestFailsWhenEngineRejects(t *tes require.NoError(t, err) assert.Nil(t, pendingStart) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(ctx)) @@ -3419,7 +3419,7 @@ func TestLocalClient_Apply_MultiTableSequential(t *testing.T) { ctx := t.Context() // Create two initial tables - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "failed to open database") defer utils.CloseAndLog(db) @@ -3559,7 +3559,7 @@ func TestLocalClient_StartApplyHeartbeat(t *testing.T) { _ = container setupStorageSchema(t, dsn) - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) @@ -3625,7 +3625,7 @@ func TestLocalClient_Apply_AtomicHeartbeat(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) @@ -3811,7 +3811,7 @@ func TestLocalClient_Apply_SequentialNamespaceMatchesTask(t *testing.T) { ctx := t.Context() // Create a table to alter - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) _, err = db.ExecContext(ctx, "CREATE TABLE users (id INT PRIMARY KEY)") require.NoError(t, err) @@ -3829,7 +3829,7 @@ func TestLocalClient_Apply_SequentialNamespaceMatchesTask(t *testing.T) { defer utils.CloseAndLog(client) // Load current schema - dbConn, err := sql.Open("mysql", dsn) + dbConn, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(dbConn) diff --git a/pkg/tern/local_control_cancel_settle_integration_test.go b/pkg/tern/local_control_cancel_settle_integration_test.go index 54159538f..461a750c4 100644 --- a/pkg/tern/local_control_cancel_settle_integration_test.go +++ b/pkg/tern/local_control_cancel_settle_integration_test.go @@ -91,7 +91,7 @@ func seedRunningApplyWithTask(t *testing.T, stor storage.Storage, databaseType, // hands the apply to a fresh claim in production. func expireApplyLease(t *testing.T, dsn string, applyID int64) { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open database to expire the apply lease") defer utils.CloseAndLog(db) _, err = db.ExecContext(t.Context(), diff --git a/pkg/tern/local_control_multiop_resume_integration_test.go b/pkg/tern/local_control_multiop_resume_integration_test.go index e682c4cc4..f96a1defd 100644 --- a/pkg/tern/local_control_multiop_resume_integration_test.go +++ b/pkg/tern/local_control_multiop_resume_integration_test.go @@ -194,7 +194,7 @@ func newMultiOpResumeFixture(t *testing.T, taskStates []string) *multiOpResumeFi tasks = append(tasks, task) } - leaseDB, err := sql.Open("mysql", dsn) + leaseDB, err := sql.Open("block-mysql", dsn) require.NoError(t, err) t.Cleanup(func() { utils.CloseAndLog(leaseDB) }) require.NoError(t, leaseDB.PingContext(ctx)) diff --git a/pkg/tern/local_dispatch_attach_integration_test.go b/pkg/tern/local_dispatch_attach_integration_test.go index 501c13cae..de57f6d23 100644 --- a/pkg/tern/local_dispatch_attach_integration_test.go +++ b/pkg/tern/local_dispatch_attach_integration_test.go @@ -32,7 +32,7 @@ func setupAttachDispatchClient(t *testing.T) (storage.Storage, *LocalClient, str ctx := t.Context() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) _, err = db.ExecContext(ctx, "CREATE TABLE users (id INT PRIMARY KEY)") diff --git a/pkg/tern/local_dispatch_shard_integration_test.go b/pkg/tern/local_dispatch_shard_integration_test.go index c12d0a516..8bc3add48 100644 --- a/pkg/tern/local_dispatch_shard_integration_test.go +++ b/pkg/tern/local_dispatch_shard_integration_test.go @@ -36,7 +36,7 @@ func TestLocalClient_ShardScopedDispatchDrivesItsTasks(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(db) _, err = db.ExecContext(ctx, "CREATE TABLE users (id INT PRIMARY KEY)") diff --git a/pkg/tern/local_resume_engine_logging_integration_test.go b/pkg/tern/local_resume_engine_logging_integration_test.go index 3bb9ea299..b3446e3e5 100644 --- a/pkg/tern/local_resume_engine_logging_integration_test.go +++ b/pkg/tern/local_resume_engine_logging_integration_test.go @@ -35,7 +35,7 @@ func TestLocalClient_ResumedDriveCapturesEngineLogs(t *testing.T) { cleanupTestTables(t, dsn) ctx := t.Context() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err, "open target database") defer utils.CloseAndLog(db) _, err = db.ExecContext(ctx, "CREATE TABLE users (id INT PRIMARY KEY)") diff --git a/pkg/tern/shard_writethrough_integration_test.go b/pkg/tern/shard_writethrough_integration_test.go index 9b80ac0f0..62361b686 100644 --- a/pkg/tern/shard_writethrough_integration_test.go +++ b/pkg/tern/shard_writethrough_integration_test.go @@ -71,7 +71,7 @@ func TestWriteShardProgressPersistsPerShardTasksUnderLease(t *testing.T) { require.NoError(t, err) // Stamp the operation lease the operator drive holds. - leaseDB, err := sql.Open("mysql", dsn) + leaseDB, err := sql.Open("block-mysql", dsn) require.NoError(t, err) defer utils.CloseAndLog(leaseDB) require.NoError(t, leaseDB.PingContext(ctx)) diff --git a/pkg/testutil/mysql.go b/pkg/testutil/mysql.go index 9074b064e..981e4b663 100644 --- a/pkg/testutil/mysql.go +++ b/pkg/testutil/mysql.go @@ -8,12 +8,23 @@ import ( "strings" "time" - _ "github.com/go-sql-driver/mysql" // database/sql driver for the readiness probe below + _ "github.com/block/mysql" // database/sql driver for the readiness probe below "github.com/moby/moby/api/types/network" "github.com/testcontainers/testcontainers-go" "github.com/testcontainers/testcontainers-go/wait" ) +// driverName is the database/sql driver the readiness probe below opens with. +// +// It has to name whatever the blank import above registers, and block/mysql +// registers "block-mysql" rather than "mysql". Nothing in SchemaBot registers +// "mysql" any more, so the old literal failed the wait strategy with `unknown +// driver "mysql"` before a single test ran — and it failed inside a container +// start hook, which surfaces as the whole package failing rather than as a bad +// driver name. Named, so the import and the string that depends on it cannot +// drift apart again. +const driverName = "block-mysql" + // mysqlRootPassword is the root password every test MySQL container is started // with, and that MySQLDSN builds connection strings against. const mysqlRootPassword = "testpassword" @@ -110,7 +121,7 @@ func MySQLContainerRequest(image, database string) testcontainers.ContainerReque "MYSQL_DATABASE": database, }, Tmpfs: mysqlDatadirTmpfs(), - WaitingFor: wait.ForSQL(mysqlPort+"/tcp", "mysql", func(host string, port network.Port) string { + WaitingFor: wait.ForSQL(mysqlPort+"/tcp", driverName, func(host string, port network.Port) string { return fmt.Sprintf("root:%s@tcp(%s:%s)/%s", mysqlRootPassword, host, port.Port(), database) }).WithStartupTimeout(mysqlStartupTimeout), } diff --git a/pkg/webhook/apply_check_records_integration_test.go b/pkg/webhook/apply_check_records_integration_test.go index 43f7fbcce..7601b22af 100644 --- a/pkg/webhook/apply_check_records_integration_test.go +++ b/pkg/webhook/apply_check_records_integration_test.go @@ -32,7 +32,7 @@ import ( func TestUpdateCheckRecordForApplyStart_ConvergesWhenApplyAlreadyTerminal(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", e2eSchemabotDSN) + db, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) t.Cleanup(func() { assert.NoError(t, db.Close()) }) @@ -134,7 +134,7 @@ func TestUpdateCheckRecordForApplyStart_ConvergesWhenApplyAlreadyTerminal(t *tes func TestUpdateCheckRecordForApplyStart_KeepsInProgressForRunningApply(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", e2eSchemabotDSN) + db, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) t.Cleanup(func() { assert.NoError(t, db.Close()) }) @@ -219,7 +219,7 @@ func TestUpdateCheckRecordForApplyStart_KeepsInProgressForRunningApply(t *testin func TestUpdateCheckRecordForApplyStart_RollbackOnConcludedAggregatePublishesFreshCheckRun(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", e2eSchemabotDSN) + db, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) t.Cleanup(func() { assert.NoError(t, db.Close()) }) diff --git a/pkg/webhook/apply_comment_integration_test.go b/pkg/webhook/apply_comment_integration_test.go index e22bfa2de..9fd742645 100644 --- a/pkg/webhook/apply_comment_integration_test.go +++ b/pkg/webhook/apply_comment_integration_test.go @@ -162,7 +162,7 @@ func TestE2EApplyCommentLifecycle(t *testing.T) { ctx := t.Context() // Set up SchemaBot storage - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { utils.CloseAndLog(schemabotDB) }) @@ -812,7 +812,7 @@ func setupApplyCommentFixture(t *testing.T, p applyCommentFixtureParams) *applyC t.Helper() ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) // svc.Close below owns closing the store's DB; this early-failure safety // close is redundant once svc exists, so discard its guaranteed @@ -2388,7 +2388,7 @@ func (s *failingCommentSupersedeStorage) heal() { s.healed.Store(true) } func TestE2EReconcileMissingSummaryCommentsPostsSummary(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { utils.CloseAndLog(schemabotDB) }) @@ -2607,7 +2607,7 @@ func seedReconcileScenario(t *testing.T, st storage.Storage, schemabotDB *sql.DB func TestE2EReconcileMissingSummaryCommentsRepairsStoppedApply(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) // Redundant early-exit closer: svc owns the storage (and this handle) and // closes it below, so discard the guaranteed already-closed error. @@ -2649,7 +2649,7 @@ func TestE2EReconcileMissingSummaryCommentsRepairsStoppedApply(t *testing.T) { func TestE2EReconcileMissingSummaryCommentsRespectsFreshClaim(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) // Redundant early-exit closer: svc owns the storage (and this handle) and // closes it below, so discard the guaranteed already-closed error. @@ -2693,7 +2693,7 @@ func TestE2EReconcileMissingSummaryCommentsRespectsFreshClaim(t *testing.T) { func TestE2EAggregateTerminalObserverClaimsSummaryExactlyOnce(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { utils.CloseAndLog(schemabotDB) }) st := mysqlstore.New(schemabotDB) @@ -2746,7 +2746,7 @@ func TestE2EAggregateTerminalObserverClaimsSummaryExactlyOnce(t *testing.T) { func TestE2EApplyCommentUpsertOnResume(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { _ = schemabotDB.Close() }) @@ -2853,7 +2853,7 @@ func TestE2EApplyCommentUpsertOnResume(t *testing.T) { func TestE2ECommentObserverSkipsTerminalSideEffectsAfterLeaseLoss(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { utils.CloseAndLog(schemabotDB) }) @@ -2987,7 +2987,7 @@ func TestE2ECommentObserverSkipsTerminalSideEffectsAfterLeaseLoss(t *testing.T) // TestE2EEditTrackedCommentNotFound tests that editing a non-existent comment is handled gracefully. func TestE2EEditTrackedCommentNotFound(t *testing.T) { - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { _ = schemabotDB.Close() }) @@ -3036,7 +3036,7 @@ func TestE2EEditTrackedCommentNotFound(t *testing.T) { func TestE2EInitialProgressCommentFinalizedForFastApply(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { utils.CloseAndLog(schemabotDB) }) st := mysqlstore.New(schemabotDB) diff --git a/pkg/webhook/apply_integration_test.go b/pkg/webhook/apply_integration_test.go index d6c817826..44d84955f 100644 --- a/pkg/webhook/apply_integration_test.go +++ b/pkg/webhook/apply_integration_test.go @@ -1773,7 +1773,7 @@ func TestE2EApplyStaleBaseSchemaOutranksUnsafePrompt(t *testing.T) { svc := setupE2EService(t, dbName) // The live target already carries the column the newer base commit added. - targetDB, err := sql.Open("mysql", e2eTargetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", e2eTargetDSN+"&multiStatements=true") require.NoError(t, err) defer utils.CloseAndLog(targetDB) _, err = targetDB.ExecContext(t.Context(), @@ -1868,7 +1868,7 @@ func TestE2EApplyConfirmStaleBaseSchemaAtFinalGateOutranksUnsafePrompt(t *testin // The live target already carries the column the newer base commit added, // so the re-plan from the stale snapshot emits a destructive DROP COLUMN. - targetDB, err := sql.Open("mysql", e2eTargetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", e2eTargetDSN+"&multiStatements=true") require.NoError(t, err) defer utils.CloseAndLog(targetDB) _, err = targetDB.ExecContext(t.Context(), @@ -3132,7 +3132,7 @@ func seedTargetTable(t *testing.T, dbName, ddl string) { t.Helper() appDSN := strings.Replace(e2eTargetDSN, "/target_test", "/"+dbName, 1) + "&multiStatements=true" - db, err := sql.Open("mysql", appDSN) + db, err := sql.Open("block-mysql", appDSN) require.NoError(t, err) defer utils.CloseAndLog(db) diff --git a/pkg/webhook/auto_plan_integration_test.go b/pkg/webhook/auto_plan_integration_test.go index ae7bccc6f..3769109fa 100644 --- a/pkg/webhook/auto_plan_integration_test.go +++ b/pkg/webhook/auto_plan_integration_test.go @@ -22,7 +22,7 @@ import ( "testing" "time" - mysql "github.com/go-sql-driver/mysql" + mysql "github.com/block/mysql" gh "github.com/google/go-github/v86/github" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -993,7 +993,7 @@ func TestE2EAutoPlanNoChangesSkipsComment(t *testing.T) { // Pre-create the table so there are no changes ctx := t.Context() appDSN := strings.Replace(e2eTargetDSN, "/target_test", "/"+dbName, 1) + "&multiStatements=true" - db, err := sql.Open("mysql", appDSN) + db, err := sql.Open("block-mysql", appDSN) require.NoError(t, err) _, err = db.ExecContext(ctx, "CREATE TABLE `users` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `name` varchar(255) NOT NULL,\n PRIMARY KEY (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci") require.NoError(t, err) @@ -1582,7 +1582,7 @@ func TestE2EAutoPlanWithOnlySchemaBotConfigChangeClearsRollbackCheck(t *testing. cfg, err := mysql.ParseDSN(e2eTargetDSN) require.NoError(t, err) cfg.DBName = dbName - db, err := sql.Open("mysql", cfg.FormatDSN()) + db, err := sql.Open("block-mysql", cfg.FormatDSN()) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(ctx)) diff --git a/pkg/webhook/blocked_gate_integration_test.go b/pkg/webhook/blocked_gate_integration_test.go index 85fd26c3e..3cf3b319b 100644 --- a/pkg/webhook/blocked_gate_integration_test.go +++ b/pkg/webhook/blocked_gate_integration_test.go @@ -29,7 +29,7 @@ const pkSwapSchema = "CREATE TABLE `users` (\n" + // database so the plan produces the refused primary-key reshape. func seedPKSwapTargetTable(t *testing.T, dbName string) { t.Helper() - db, err := sql.Open("mysql", driftDSN(t, dbName)) + db, err := sql.Open("block-mysql", driftDSN(t, dbName)) require.NoError(t, err) defer func() { _ = db.Close() }() _, err = db.ExecContext(t.Context(), "CREATE TABLE `users` (\n"+ diff --git a/pkg/webhook/check_records_refused_plan_test.go b/pkg/webhook/check_records_refused_plan_test.go index 51d300cf5..b7ece410a 100644 --- a/pkg/webhook/check_records_refused_plan_test.go +++ b/pkg/webhook/check_records_refused_plan_test.go @@ -28,7 +28,7 @@ import ( func TestPlanCheckWriteRefusedByInFlightApply(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", e2eSchemabotDSN) + db, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) t.Cleanup(func() { _ = db.Close() }) diff --git a/pkg/webhook/check_records_rollback_test.go b/pkg/webhook/check_records_rollback_test.go index ae99095d3..f9321cd4d 100644 --- a/pkg/webhook/check_records_rollback_test.go +++ b/pkg/webhook/check_records_rollback_test.go @@ -31,7 +31,7 @@ import ( func TestRefreshChecksForTerminalApply_CompletedRollbackIsActionRequired(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", e2eSchemabotDSN) + db, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) t.Cleanup(func() { _ = db.Close() }) diff --git a/pkg/webhook/check_records_stopped_test.go b/pkg/webhook/check_records_stopped_test.go index c9eaa830c..4dd05522c 100644 --- a/pkg/webhook/check_records_stopped_test.go +++ b/pkg/webhook/check_records_stopped_test.go @@ -27,7 +27,7 @@ import ( func TestUpdateCheckRecordForApplyResult_StoppedThenCompleted(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", e2eSchemabotDSN) + db, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) t.Cleanup(func() { _ = db.Close() }) diff --git a/pkg/webhook/comment_authority_integration_test.go b/pkg/webhook/comment_authority_integration_test.go index 44649de0b..3953386f5 100644 --- a/pkg/webhook/comment_authority_integration_test.go +++ b/pkg/webhook/comment_authority_integration_test.go @@ -37,7 +37,7 @@ func seedOperationScopedApply(t *testing.T, repo, database string) *operationSco t.Helper() ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { utils.CloseAndLog(schemabotDB) }) require.NoError(t, schemabotDB.PingContext(ctx)) diff --git a/pkg/webhook/control_integration_test.go b/pkg/webhook/control_integration_test.go index 7aa6feb16..2d85dfc79 100644 --- a/pkg/webhook/control_integration_test.go +++ b/pkg/webhook/control_integration_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + _ "github.com/block/mysql" "github.com/block/schemabot/pkg/api" ghclient "github.com/block/schemabot/pkg/github" ternv1 "github.com/block/schemabot/pkg/proto/ternv1" @@ -21,7 +22,6 @@ import ( "github.com/block/schemabot/pkg/storage/mysqlstore" "github.com/block/schemabot/pkg/tern" "github.com/block/spirit/pkg/utils" - _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -31,7 +31,7 @@ import ( // and preserves each caller in apply logs for incident triage. func TestE2EStopCommandRecordsDurableRequest(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) @@ -144,7 +144,7 @@ func TestE2EStopCommandRecordsDurableRequest(t *testing.T) { // command records permanent cancel intent and preserves the caller in apply logs. func TestE2ECancelCommandRecordsDurableRequest(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) @@ -210,7 +210,7 @@ func TestE2ECancelCommandRecordsDurableRequest(t *testing.T) { // active local schema change but this process does not own the Spirit runner. func TestE2EStopCommandQueuesDeferredCutoverLocalApplyWithoutRunner(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) @@ -302,7 +302,7 @@ func TestE2EStopCommandQueuesDeferredCutoverLocalApplyWithoutRunner(t *testing.T // claiming execution from the webhook process. func TestE2EStartCommandRecordsDurableRequest(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) @@ -390,7 +390,7 @@ func TestE2EStartCommandRecordsDurableRequest(t *testing.T) { // does not record a durable start request. func TestE2EStartCommandRejectsCompletedApply(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) @@ -457,7 +457,7 @@ func TestE2EStartCommandRejectsCompletedApply(t *testing.T) { // environment, then leaves the operator owner to perform the data-plane action. func TestE2ECutoverCommandRecordsDurableRequest(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) @@ -539,7 +539,7 @@ func TestE2ECutoverCommandRecordsDurableRequest(t *testing.T) { // PlanetScale apply in its revert window. func TestE2ESkipRevertCommandAcceptsApplyID(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) @@ -613,7 +613,7 @@ func TestE2ESkipRevertCommandAcceptsApplyID(t *testing.T) { // immediate attempt cannot land. func TestE2ERevertCommandRevertsApply(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) @@ -697,7 +697,7 @@ func TestE2ERevertCommandRevertsApply(t *testing.T) { // operator's stop intent and surfacing the rejection back to the PR. func TestE2ECutoverCommandRejectsPendingStop(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) diff --git a/pkg/webhook/copy_discard_gate_integration_test.go b/pkg/webhook/copy_discard_gate_integration_test.go index 29fdb115b..ff07a12f5 100644 --- a/pkg/webhook/copy_discard_gate_integration_test.go +++ b/pkg/webhook/copy_discard_gate_integration_test.go @@ -48,7 +48,7 @@ func seedAbandonedCopy(t *testing.T, dbName string) { // the schema the PR declares plans as a single ALTER against it. func seedPreChangeEvents(t *testing.T, dbName string) { t.Helper() - db, err := sql.Open("mysql", driftDSN(t, dbName)) + db, err := sql.Open("block-mysql", driftDSN(t, dbName)) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(t.Context()), "connect to target") @@ -66,7 +66,7 @@ func seedPreChangeEvents(t *testing.T, dbName string) { // PR's plan will hand the engine. func seedCopyArtifacts(t *testing.T, dbName string) { t.Helper() - db, err := sql.Open("mysql", driftDSN(t, dbName)) + db, err := sql.Open("block-mysql", driftDSN(t, dbName)) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(t.Context()), "connect to target") @@ -91,7 +91,7 @@ func seedCopyArtifacts(t *testing.T, dbName string) { // both behind for the copy to still be resumable. func requireCopyIntact(t *testing.T, dbName string) { t.Helper() - db, err := sql.Open("mysql", driftDSN(t, dbName)) + db, err := sql.Open("block-mysql", driftDSN(t, dbName)) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(t.Context()), "connect to target") @@ -214,7 +214,7 @@ func seedTwoTableTargetWithOneCopy(t *testing.T, dbName string) { t.Helper() seedPreChangeEvents(t, dbName) - db, err := sql.Open("mysql", driftDSN(t, dbName)) + db, err := sql.Open("block-mysql", driftDSN(t, dbName)) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(t.Context()), "connect to target") @@ -604,7 +604,7 @@ func TestE2EApplyConfirmProceedsWhenDiscardWasDisclosed(t *testing.T) { // The copy the operator agreed to lose is gone, and the index they asked for // is on the table. - db, err := sql.Open("mysql", driftDSN(t, dbName)) + db, err := sql.Open("block-mysql", driftDSN(t, dbName)) require.NoError(t, err) defer utils.CloseAndLog(db) require.NoError(t, db.PingContext(t.Context())) diff --git a/pkg/webhook/direct_gate_integration_test.go b/pkg/webhook/direct_gate_integration_test.go index 768dabf3f..f4390f532 100644 --- a/pkg/webhook/direct_gate_integration_test.go +++ b/pkg/webhook/direct_gate_integration_test.go @@ -24,7 +24,7 @@ import ( // ordinal order, so tests can assert whether the reshape landed on the target. func appPrimaryKeyColumns(t *testing.T, dbName, tableName string) []string { t.Helper() - db, err := sql.Open("mysql", driftDSN(t, dbName)) + db, err := sql.Open("block-mysql", driftDSN(t, dbName)) require.NoError(t, err) defer func() { _ = db.Close() }() rows, err := db.QueryContext(t.Context(), ` diff --git a/pkg/webhook/failure_logs_integration_test.go b/pkg/webhook/failure_logs_integration_test.go index 4eff3ba84..294fb9175 100644 --- a/pkg/webhook/failure_logs_integration_test.go +++ b/pkg/webhook/failure_logs_integration_test.go @@ -28,7 +28,7 @@ import ( func TestE2EFailedApplySummaryCarriesRecentLogs(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) t.Cleanup(func() { utils.CloseAndLog(schemabotDB) }) diff --git a/pkg/webhook/fanout_two_deployment_integration_test.go b/pkg/webhook/fanout_two_deployment_integration_test.go index 446528d33..453e5f109 100644 --- a/pkg/webhook/fanout_two_deployment_integration_test.go +++ b/pkg/webhook/fanout_two_deployment_integration_test.go @@ -76,7 +76,7 @@ func TestE2EFanOutRollbackOwnerActsSiblingSilent(t *testing.T) { ctx := t.Context() appDSN := strings.Replace(e2eTargetDSN, "/target_test", "/"+dbName, 1) + "&multiStatements=true" - db, err := sql.Open("mysql", appDSN) + db, err := sql.Open("block-mysql", appDSN) require.NoError(t, err) _, err = db.ExecContext(ctx, "CREATE TABLE `users` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `name` varchar(255) NOT NULL,\n PRIMARY KEY (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci") require.NoError(t, err) @@ -159,7 +159,7 @@ func TestE2EFanOutRollbackOwnerActsSiblingSilent(t *testing.T) { // the acceptance comment; a sibling tenant deployment stays silent. func TestE2EFanOutStopOwnerActsSiblingSilent(t *testing.T) { ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, schemabotDB.PingContext(ctx)) diff --git a/pkg/webhook/plan_change_ownership_integration_test.go b/pkg/webhook/plan_change_ownership_integration_test.go index 88bfb93d5..4ff356d51 100644 --- a/pkg/webhook/plan_change_ownership_integration_test.go +++ b/pkg/webhook/plan_change_ownership_integration_test.go @@ -471,7 +471,7 @@ func applyDeclaredSchemaForPullRequest(t *testing.T, svc *api.Service, dbName st // so the plan sees a drop for an object storage has no history for. func seedOwnershipReconcileTable(t *testing.T, dbName string) { t.Helper() - db, err := sql.Open("mysql", driftDSN(t, dbName)) + db, err := sql.Open("block-mysql", driftDSN(t, dbName)) require.NoError(t, err) defer func() { _ = db.Close() }() _, err = db.ExecContext(t.Context(), "CREATE TABLE `reconcile_state` (\n"+ @@ -487,7 +487,7 @@ func seedOwnershipReconcileTable(t *testing.T, dbName string) { // reads only what this run records. func resetOwnershipHistory(t *testing.T, dbName string) { t.Helper() - db, err := sql.Open("mysql", e2eSchemabotDSN) + db, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) defer func() { _ = db.Close() }() ctx := t.Context() diff --git a/pkg/webhook/plan_comment_retire_integration_test.go b/pkg/webhook/plan_comment_retire_integration_test.go index 7afe7ca75..b1b079441 100644 --- a/pkg/webhook/plan_comment_retire_integration_test.go +++ b/pkg/webhook/plan_comment_retire_integration_test.go @@ -202,7 +202,7 @@ func setupPlanCommentHandler(t *testing.T, repo string, deleteUnactioned bool) ( t.Helper() ctx := t.Context() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) // Redundant close for early-exit leak safety: svc.Close below owns the // handle (the store is built over it), so this close is expected to see an diff --git a/pkg/webhook/plan_drift_integration_test.go b/pkg/webhook/plan_drift_integration_test.go index cc295e94c..75c0b25ad 100644 --- a/pkg/webhook/plan_drift_integration_test.go +++ b/pkg/webhook/plan_drift_integration_test.go @@ -20,7 +20,7 @@ import ( "os" "testing" - mysql "github.com/go-sql-driver/mysql" + mysql "github.com/block/mysql" gh "github.com/google/go-github/v86/github" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -68,7 +68,7 @@ type deploymentSpec struct { // leaks the handle. func openDriftDB(t *testing.T, dsn string) *sql.DB { t.Helper() - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) require.NoError(t, err) t.Cleanup(func() { _ = db.Close() }) require.NoError(t, db.PingContext(t.Context())) @@ -143,7 +143,7 @@ func setupE2EReviewDriftService(t *testing.T, dbName string, specs []deploymentS // by the time cleanup runs, which would otherwise leave databases behind. t.Cleanup(func() { dropCtx := context.WithoutCancel(t.Context()) - db, err := sql.Open("mysql", adminDSN) + db, err := sql.Open("block-mysql", adminDSN) if err != nil { t.Logf("drift cleanup: open admin db to drop %s: %v", physicalDB, err) return diff --git a/pkg/webhook/plan_integration_test.go b/pkg/webhook/plan_integration_test.go index 64546cf6b..e6071dc5c 100644 --- a/pkg/webhook/plan_integration_test.go +++ b/pkg/webhook/plan_integration_test.go @@ -404,7 +404,7 @@ func TestE2EPlanNoChanges(t *testing.T) { // Create the table in the target DB first so the plan finds no changes ctx := t.Context() appDSN := strings.Replace(e2eTargetDSN, "/target_test", "/"+dbName, 1) + "&multiStatements=true" - db, err := sql.Open("mysql", appDSN) + db, err := sql.Open("block-mysql", appDSN) require.NoError(t, err) _, err = db.ExecContext(ctx, "CREATE TABLE `users` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `name` varchar(255) NOT NULL,\n PRIMARY KEY (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci") require.NoError(t, err) @@ -738,7 +738,7 @@ func TestE2EMultiEnvPlanDifferentChanges(t *testing.T) { // Pre-create the table in staging so staging has no changes, but production still does ctx := t.Context() appDSNStaging := strings.Replace(e2eTargetDSN, "/target_test", "/"+dbName+"_staging", 1) + "&multiStatements=true" - db, err := sql.Open("mysql", appDSNStaging) + db, err := sql.Open("block-mysql", appDSNStaging) require.NoError(t, err) _, err = db.ExecContext(ctx, "CREATE TABLE `users` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `name` varchar(255) NOT NULL,\n PRIMARY KEY (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci") require.NoError(t, err) @@ -801,14 +801,14 @@ func TestE2EPlanUsesServerSideTarget(t *testing.T) { ctx := t.Context() // Create the app database on the target - targetDB, err := sql.Open("mysql", e2eTargetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", e2eTargetDSN+"&multiStatements=true") require.NoError(t, err) _, err = targetDB.ExecContext(ctx, "CREATE DATABASE IF NOT EXISTS `"+dbName+"`") require.NoError(t, err) _ = targetDB.Close() t.Cleanup(func() { - db, err := sql.Open("mysql", e2eTargetDSN+"&multiStatements=true") + db, err := sql.Open("block-mysql", e2eTargetDSN+"&multiStatements=true") if err == nil { _, _ = db.ExecContext(t.Context(), "DROP DATABASE IF EXISTS `"+dbName+"`") _ = db.Close() @@ -818,7 +818,7 @@ func TestE2EPlanUsesServerSideTarget(t *testing.T) { appDSN := strings.Replace(e2eTargetDSN, "/target_test", "/"+dbName, 1) logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { _ = schemabotDB.Close() }) diff --git a/pkg/webhook/rollback_integration_test.go b/pkg/webhook/rollback_integration_test.go index 4cd55de0d..ee55fa5f2 100644 --- a/pkg/webhook/rollback_integration_test.go +++ b/pkg/webhook/rollback_integration_test.go @@ -20,7 +20,7 @@ import ( "testing" "time" - mysql "github.com/go-sql-driver/mysql" + mysql "github.com/block/mysql" gh "github.com/google/go-github/v86/github" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -43,7 +43,7 @@ func TestE2ERollbackPlanViaWebhook(t *testing.T) { // Step 1: Create an initial table in the target DB (the "before" state) appDSN := strings.Replace(e2eTargetDSN, "/target_test", "/"+dbName, 1) + "&multiStatements=true" - db, err := sql.Open("mysql", appDSN) + db, err := sql.Open("block-mysql", appDSN) require.NoError(t, err) _, err = db.ExecContext(ctx, "CREATE TABLE `users` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `name` varchar(255) NOT NULL,\n PRIMARY KEY (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci") require.NoError(t, err) @@ -262,7 +262,7 @@ func TestE2ERollbackConfirmExecutesAndPostsComments(t *testing.T) { cfg.DBName = dbName cfg.MultiStatements = true appDSN := cfg.FormatDSN() - db, err := sql.Open("mysql", appDSN) + db, err := sql.Open("block-mysql", appDSN) require.NoError(t, err) _, err = db.ExecContext(ctx, "CREATE TABLE `users` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `name` varchar(255) NOT NULL,\n PRIMARY KEY (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci") require.NoError(t, err) @@ -508,7 +508,7 @@ func TestE2ERollbackConfirmUpdatesCheckToActionRequired(t *testing.T) { require.NoError(t, err) cfg.DBName = dbName cfg.MultiStatements = true - db, err := sql.Open("mysql", cfg.FormatDSN()) + db, err := sql.Open("block-mysql", cfg.FormatDSN()) require.NoError(t, err) _, err = db.ExecContext(ctx, "CREATE TABLE `users` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `name` varchar(255) NOT NULL,\n PRIMARY KEY (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci") require.NoError(t, err) diff --git a/pkg/webhook/terminal_apply_head_publish_test.go b/pkg/webhook/terminal_apply_head_publish_test.go index cdd39f4a6..442691dc8 100644 --- a/pkg/webhook/terminal_apply_head_publish_test.go +++ b/pkg/webhook/terminal_apply_head_publish_test.go @@ -40,7 +40,7 @@ import ( func TestRefreshChecksForTerminalApplyPublishesOnPRHead(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", e2eSchemabotDSN) + db, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) t.Cleanup(func() { _ = db.Close() }) diff --git a/pkg/webhook/vschema_only_check_integration_test.go b/pkg/webhook/vschema_only_check_integration_test.go index 28a447d7a..3434839ca 100644 --- a/pkg/webhook/vschema_only_check_integration_test.go +++ b/pkg/webhook/vschema_only_check_integration_test.go @@ -30,7 +30,7 @@ import ( func TestUpsertPlanCheckRecord_VSchemaOnlyPlanRequiresApply(t *testing.T) { ctx := t.Context() - db, err := sql.Open("mysql", e2eSchemabotDSN) + db, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) require.NoError(t, db.PingContext(ctx)) t.Cleanup(func() { assert.NoError(t, db.Close()) }) diff --git a/pkg/webhook/webhook_integration_test.go b/pkg/webhook/webhook_integration_test.go index 28e7bed97..5beb34e79 100644 --- a/pkg/webhook/webhook_integration_test.go +++ b/pkg/webhook/webhook_integration_test.go @@ -81,7 +81,7 @@ import ( "testing" "time" - mysql "github.com/go-sql-driver/mysql" + mysql "github.com/block/mysql" gh "github.com/google/go-github/v86/github" "github.com/stretchr/testify/require" "github.com/testcontainers/testcontainers-go" @@ -205,14 +205,14 @@ func setupE2EServiceOpts(t *testing.T, appDBName string, opts e2eServiceOpts) *a if databaseType == storage.DatabaseTypeMySQL { // Create the app database on the target. - targetDB, err := sql.Open("mysql", e2eTargetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", e2eTargetDSN+"&multiStatements=true") require.NoError(t, err) _, err = targetDB.ExecContext(ctx, "CREATE DATABASE IF NOT EXISTS `"+appDBName+"`") require.NoError(t, err) _ = targetDB.Close() t.Cleanup(func() { - db, err := sql.Open("mysql", e2eTargetDSN+"&multiStatements=true") + db, err := sql.Open("block-mysql", e2eTargetDSN+"&multiStatements=true") if err == nil { _, _ = db.ExecContext(t.Context(), "DROP DATABASE IF EXISTS `"+appDBName+"`") _ = db.Close() @@ -229,7 +229,7 @@ func setupE2EServiceOpts(t *testing.T, appDBName string, opts e2eServiceOpts) *a require.NotEmpty(t, appDSN, "target DSN is required for database type %s", databaseType) logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { _ = schemabotDB.Close() }) @@ -1001,7 +1001,7 @@ func setupE2EServiceMultiEnv(t *testing.T, appDBName string) *api.Service { t.Helper() ctx := t.Context() - targetDB, err := sql.Open("mysql", e2eTargetDSN+"&multiStatements=true") + targetDB, err := sql.Open("block-mysql", e2eTargetDSN+"&multiStatements=true") require.NoError(t, err) stagingDB := appDBName + "_staging" @@ -1014,7 +1014,7 @@ func setupE2EServiceMultiEnv(t *testing.T, appDBName string) *api.Service { _ = targetDB.Close() t.Cleanup(func() { - db, err := sql.Open("mysql", e2eTargetDSN+"&multiStatements=true") + db, err := sql.Open("block-mysql", e2eTargetDSN+"&multiStatements=true") if err == nil { _, _ = db.ExecContext(t.Context(), "DROP DATABASE IF EXISTS `"+stagingDB+"`") _, _ = db.ExecContext(t.Context(), "DROP DATABASE IF EXISTS `"+productionDB+"`") @@ -1026,7 +1026,7 @@ func setupE2EServiceMultiEnv(t *testing.T, appDBName string) *api.Service { productionDSN := strings.Replace(e2eTargetDSN, "/target_test", "/"+productionDB, 1) logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { _ = schemabotDB.Close() }) @@ -1094,7 +1094,7 @@ func startE2EMySQLContainer(ctx context.Context, baseName, dbName string, schema _ = container.Terminate(ctx) return nil, fmt.Errorf("build mysql dsn: %w", err) } - db, err := sql.Open("mysql", dsn) + db, err := sql.Open("block-mysql", dsn) if err != nil { _ = container.Terminate(ctx) return nil, fmt.Errorf("open db: %w", err) @@ -1173,7 +1173,7 @@ func setupE2EServiceWithAllowedEnvs(t *testing.T, allowedEnvs []string) *api.Ser func setupE2EServiceWithConfig(t *testing.T, serverConfig *api.ServerConfig) *api.Service { t.Helper() - schemabotDB, err := sql.Open("mysql", e2eSchemabotDSN) + schemabotDB, err := sql.Open("block-mysql", e2eSchemabotDSN) require.NoError(t, err) t.Cleanup(func() { _ = schemabotDB.Close() }) diff --git a/pkg/webhook/webhook_misc_integration_test.go b/pkg/webhook/webhook_misc_integration_test.go index 6400f397f..918e56b7b 100644 --- a/pkg/webhook/webhook_misc_integration_test.go +++ b/pkg/webhook/webhook_misc_integration_test.go @@ -44,7 +44,7 @@ func TestE2EApplyCreateDualWritesApplyOperationRow(t *testing.T) { // Seed the target so the plan produces a real DDL change. appDSN := strings.Replace(e2eTargetDSN, "/target_test", "/"+dbName, 1) + "&multiStatements=true" - db, err := sql.Open("mysql", appDSN) + db, err := sql.Open("block-mysql", appDSN) require.NoError(t, err) _, err = db.ExecContext(ctx, "CREATE TABLE `users` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `name` varchar(255) NOT NULL,\n PRIMARY KEY (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci") require.NoError(t, err)