dbconn: take RDS TLS from the driver, and drop rejectReadOnly

go.mod

@@ -4,7 +4,7 @@ go 1.26.6
 
 require (
 	github.com/alecthomas/kong v1.16.1
-	github.com/block/mysql v0.0.0-20260906201522-a3178f8dca69
+	github.com/block/mysql v0.0.0-20260906224346-ee0a93fe50d6
 	github.com/go-ini/ini v1.67.0
 	github.com/go-mysql-org/go-mysql v1.16.1-0.20260731133054-6f853f178dc3
 	github.com/google/uuid v1.6.0

go.sum

@@ -6,8 +6,8 @@ github.com/alecthomas/kong v1.16.1 h1:ixhCt93XkJ98kGposQ54+bl0IK6XwqB40AsMynU7Z8
 github.com/alecthomas/kong v1.16.1/go.mod h1:wrlbXem1CWqUV5Vbmss5ISYhsVPkBb1Yo7YKJghju2I=
 github.com/alecthomas/repr v0.5.2 h1:SU73FTI9D1P5UNtvseffFSGmdNci/O6RsqzeXJtP0Qs=
 github.com/alecthomas/repr v0.5.2/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4=
-github.com/block/mysql v0.0.0-20260906201522-a3178f8dca69 h1:rCWVZKT5PdrdfosvMavnnfUBAdtaM3iepvADt5pMSJI=
-github.com/block/mysql v0.0.0-20260906201522-a3178f8dca69/go.mod h1:KEo73lbxXs9cFlq+x3Z35UqGg3MTxAPfjDOR/ob/iik=
+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/coreos/go-semver v0.3.1 h1:yi21YpKnrx1gt5R+la8n5WgS0kCrsPp33dmEyHReZr4=
 github.com/coreos/go-semver v0.3.1/go.mod h1:irMmmIw/7yzSRPWryHsK7EYSg09caPQL03VsM8rvUec=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

pkg/change/tls_test.go

@@ -98,9 +98,9 @@ func TestTLSConfigurationLogic(t *testing.T) {
 				if dbconn.IsRDSHost(client.host) {
 					tlsConfig = dbconn.NewTLSConfig()
 				} else {
-					// For testing, use embedded RDS bundle since we don't have actual cert files
-					certData := dbconn.GetEmbeddedRDSBundle()
-					tlsConfig = dbconn.NewCustomTLSConfig(certData, client.dbConfig.TLSMode)
+					// nil cert data means "use the RDS roots", which is what
+					// this test wants: it has no cert files of its own.
+					tlsConfig = dbconn.NewCustomTLSConfig(nil, client.dbConfig.TLSMode)
 				}
 
 				if tlsConfig != nil {

pkg/datasync/runner.go

@@ -95,10 +95,9 @@ type Runner struct {
 	// nothing pays only for the discarded values.
 	metricsSink metrics.Sink
 	cancelFunc  context.CancelFunc
-	// sourceDBConfig connects to the read-only source: ForceKill and
-	// RejectReadOnly are disabled (see Run). targetDBConfig connects to the
-	// writable target and keeps the standard safe defaults — most importantly
-	// RejectReadOnly=true for Aurora-failover safety.
+	// sourceDBConfig connects to the read-only source, with ForceKill disabled
+	// (see Run). targetDBConfig connects to the writable target and keeps the
+	// standard safe defaults.
 	sourceDBConfig *dbconn.DBConfig
 	targetDBConfig *dbconn.DBConfig
 
@@ -250,25 +249,31 @@ func (r *Runner) Run(ctx context.Context) error {
 	// on the source schema; the built-in MySQL binlog client additionally
 	// needs REPLICATION SLAVE/CLIENT (validated on Start) and RELOAD, because
 	// it issues FLUSH BINARY LOGS to establish its start position. Disable the
-	// two dbConfig behaviours that would otherwise demand more:
+	// one dbConfig behaviour that would otherwise demand more:
 	//   - ForceKill needs CONNECTION_ADMIN/PROCESS + performance_schema, and
 	//     is only used to break metadata locks during cutover — which sync
 	//     never does.
-	//   - RejectReadOnly is an Aurora-failover guard that turns a read-only
-	//     server error into driver.ErrBadConn; sync's source is read-only by
-	//     design (e.g. a Vitess/PlanetScale replica), so it must not fire.
+	//
+	// There used to be a second, RejectReadOnly=false, on the grounds that the
+	// source is read-only by design (e.g. a Vitess/PlanetScale replica). That
+	// reasoned from the wrong axis: 1290/1792/1836 are raised by *writes*, not
+	// by connecting to a read-only server, and everything sync sends the source
+	// succeeds against a super_read_only MySQL — including the binlog client's
+	// FLUSH BINARY LOGS. The option is gone from the driver, and its absence
+	// costs sync nothing. If sync ever does write to its source, that is a bug,
+	// and the rejection now surfaces it instead of hiding it.
 	r.sourceDBConfig.ForceKill = false
-	r.sourceDBConfig.RejectReadOnly = false
 	r.sourceDBConfig.MaxOpenConnections = r.sync.MaxConnections
 
 	// The target is written to (table creation, the copy/apply, the
 	// checkpoint, and CREATE DATABASE on the admin connection), so it keeps the
-	// standard safe defaults — crucially RejectReadOnly=true, so that if the
-	// target Aurora fails over and we land on a demoted, now-read-only primary,
-	// writes turn into driver.ErrBadConn and the pool reconnects instead of
-	// silently erroring. Only the relaxations the target genuinely shares with
-	// the source are applied (no cutover here either, so ForceKill is left at
-	// its default but never fires).
+	// standard safe defaults. It is also the side that actually benefits from
+	// the driver's read-only rejection: if the target Aurora fails over and we
+	// land on a demoted, now-read-only primary, those writes turn into
+	// driver.ErrBadConn and the pool reconnects instead of silently erroring.
+	// Only the relaxations the target genuinely shares with the source are
+	// applied (no cutover here either, so ForceKill is left at its default but
+	// never fires).
 	r.targetDBConfig = dbconn.NewDBConfig()
 	r.targetDBConfig.MaxOpenConnections = r.sync.MaxConnections
 

pkg/dbconn/conn.go

@@ -4,12 +4,10 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"database/sql"
-	_ "embed"
 	"errors"
 	"fmt"
 	"log/slog"
 	"os"
-	"regexp"
 	"strconv"
 	"strings"
 	"sync"
@@ -35,6 +33,12 @@ const (
 	requiredTLSConfigName = "required"
 	verifyCATLSConfigName = "verify_ca"
 	verifyIDTLSConfigName = "verify_identity"
+
+	// tlsDisabledConfigName is the DSN's explicit "no TLS" value. Unlike the
+	// names above it is not something this package registers — the driver
+	// understands it directly — and unlike an empty tls= it survives the
+	// driver's RDS auto-TLS. See the DISABLED branch of newDSN.
+	tlsDisabledConfigName = "false"
 )
 
 // maxConnLifetime is the default maximum lifetime for pooled connections.
@@ -68,33 +72,62 @@ func SetPoolSize(db *sql.DB, n int) {
 	}
 }
 
-// rdsAddr matches Amazon RDS hostnames with optional :port suffix.
-// It's used to automatically load the Amazon RDS CA and enable TLS.
-// The leading \. ensures only legitimate *.rds.amazonaws.com subdomains match,
-// preventing subdomain spoofing attacks (e.g., fake-rds.amazonaws.com).
-var (
-	rdsAddr = regexp.MustCompile(`\.rds\.amazonaws\.com(:\d+)?$`)
-	once    sync.Once
-	// https://truststore.pki.rds.amazonaws.com/global/global-bundle.pem
-	//go:embed rdsGlobalBundle.pem
-	rdsGlobalBundle []byte
-)
+var once sync.Once
 
+// IsRDSHost reports whether host is an Amazon RDS or Aurora endpoint.
+//
+// [DriverName] applies verified TLS to such a host by itself, so the
+// database/sql paths in this package no longer need to ask. It stays exported
+// and in use for two reasons: [GetTLSConfigForBinlog] serves the go-mysql
+// binlog client, which is not a database/sql connection and so is not covered
+// by the driver; and block/schemabot calls it to decide a TLS mode.
+//
+// Two things changed by delegating. The match is now case-insensitive, which is
+// a fix — DNS is case-insensitive, nothing normalizes the host, and a
+// hostname that arrives uppercased is the same endpoint. And GovCloud and
+// China endpoints now report false, because the bundle behind
+// [NewTLSConfig] contains no roots for either partition, so verifying against
+// it could only ever fail. Reach those with --tls-certificate-path and that
+// partition's own bundle.
 func IsRDSHost(host string) bool {
-	return rdsAddr.MatchString(host)
+	return mysql.IsRDSAddr(host)
 }
 
-// NewTLSConfig creates a TLS config using the embedded RDS global bundle
+// NewTLSConfig returns a TLS config that verifies an Amazon RDS or Aurora
+// server against the RDS root bundle.
+//
+// The bundle and the pool behind this used to be spirit's own — an embedded
+// copy of global-bundle.pem plus an x509.CertPool built from it. Both now come
+// from [DriverName], which carries the same bundle for its own auto-TLS, so
+// there is one copy to refresh instead of two that can drift apart.
+//
+// Each call returns a config with its own RootCAs, so callers may modify the
+// result (GetTLSConfigForBinlog sets ServerName on it). It also pins
+// MinVersion to TLS 1.2, which spirit's version did not.
 func NewTLSConfig() *tls.Config {
-	caCertPool := x509.NewCertPool()
-	caCertPool.AppendCertsFromPEM(rdsGlobalBundle)
-	return &tls.Config{RootCAs: caCertPool}
+	return mysql.RDSTLSConfig()
 }
 
-// NewCustomTLSConfig creates a TLS config based on SSL mode and certificate data
+// NewCustomTLSConfig creates a TLS config based on SSL mode and certificate data.
+//
+// An empty certData means "use the RDS roots", which is the fallback for a
+// host that is not recognizably RDS and for which no --tls-certificate-path was
+// given. That fallback is inherited behaviour and is rarely what anyone wants:
+// a non-RDS server will not present an RDS-issued certificate, so VERIFY_CA
+// and VERIFY_IDENTITY against it fail by construction. It is preserved here
+// rather than changed, because tightening it is a behaviour change that
+// belongs in its own commit.
 func NewCustomTLSConfig(certData []byte, sslMode string) *tls.Config {
-	caCertPool := x509.NewCertPool()
-	caCertPool.AppendCertsFromPEM(certData)
+	var caCertPool *x509.CertPool
+	if len(certData) == 0 {
+		// The driver's RDS roots, and a private copy of the pool: the switch
+		// below hands it to callers who may append to it, and x509.CertPool has
+		// no copy-on-write.
+		caCertPool = mysql.RDSTLSConfig().RootCAs
+	} else {
+		caCertPool = x509.NewCertPool()
+		caCertPool.AppendCertsFromPEM(certData)
+	}
 
 	switch strings.ToUpper(sslMode) {
 	case "DISABLED":
@@ -172,11 +205,11 @@ func LoadCertificateFromFile(filePath string) ([]byte, error) {
 	return os.ReadFile(filePath)
 }
 
-// GetEmbeddedRDSBundle returns the embedded RDS certificate bundle
-func GetEmbeddedRDSBundle() []byte {
-	return rdsGlobalBundle
-}
-
+// initRDSTLS registers the RDS trust store under rdsTLSConfigName.
+//
+// The registration survives the driver's own auto-TLS because the name is part
+// of this package's contract, not an internal detail: EnhanceDSNWithTLS returns
+// DSNs carrying tls=rds, and block/schemabot opens them.
 func initRDSTLS() error {
 	var err error
 	once.Do(func() {
@@ -195,10 +228,9 @@ func initCustomTLS(config *DBConfig) error {
 		if err != nil {
 			return err
 		}
-	} else {
-		// Use embedded RDS bundle as fallback
-		certData = rdsGlobalBundle
 	}
+	// Otherwise certData stays nil, which NewCustomTLSConfig reads as "use the
+	// RDS roots" — the same fallback as before, now sourced from the driver.
 
 	tlsConfig := NewCustomTLSConfig(certData, config.TLSMode)
 	if tlsConfig != nil {
@@ -247,8 +279,15 @@ func newDSN(dsn string, config *DBConfig) (string, error) {
 	if cfg.TLSConfig == "" {
 		switch strings.ToUpper(config.TLSMode) {
 		case "DISABLED":
-			// No TLS - explicitly clear any TLS configuration
-			cfg.TLSConfig = ""
+			// No TLS — and it has to be said out loud rather than left blank.
+			//
+			// [DriverName] applies verified TLS to an RDS address when the DSN
+			// asks for nothing, so leaving TLSConfig empty here would hand
+			// --tls-mode=DISABLED users on RDS a TLS connection: exactly the
+			// opposite of what they asked for, from a driver upgrade, with no
+			// error to notice. "false" is the DSN's explicit off switch, which
+			// the driver honours and its auto-TLS declines to override.
+			cfg.TLSConfig = tlsDisabledConfigName
 
 		case "REQUIRED", "VERIFY_CA", "VERIFY_IDENTITY":
 			// TLS with certificate selection - determine which certificate to use
@@ -266,7 +305,7 @@ func newDSN(dsn string, config *DBConfig) (string, error) {
 				}
 				cfg.TLSConfig = rdsTLSConfigName
 			default:
-				// Use embedded RDS bundle as fallback for non-RDS hosts
+				// Use the RDS roots as fallback for non-RDS hosts
 				if err = initCustomTLS(config); err != nil {
 					return "", err
 				}
@@ -278,14 +317,14 @@ func newDSN(dsn string, config *DBConfig) (string, error) {
 
 		default:
 			// PREFERRED and unknown modes - use permissive TLS behavior
-			// For RDS hosts, use RDS certificate. For others, use embedded RDS bundle as fallback
+			// For RDS hosts, use RDS certificate. For others, use the RDS roots as fallback
 			if IsRDSHost(cfg.Addr) {
 				if err = initRDSTLS(); err != nil {
 					return "", err
 				}
 				cfg.TLSConfig = rdsTLSConfigName
 			} else {
-				// Use embedded RDS bundle as fallback for non-RDS hosts
+				// Use the RDS roots as fallback for non-RDS hosts
 				if err = initCustomTLS(config); err != nil {
 					return "", err
 				}
@@ -330,19 +369,20 @@ func newDSN(dsn string, config *DBConfig) (string, error) {
 
 	// Set driver options directly on the config struct.
 	cfg.Collation = "utf8mb4_bin"
-	// So that we recycle the connection if we inadvertently connect to an old primary which is now a read only replica.
-	// This behaviour has been observed during blue/green upgrades and failover on AWS Aurora.
-	// See also: https://github.com/go-sql-driver/mysql?tab=readme-ov-file#rejectreadonly
-	//
-	// Disabled for an injected, read-only change.Source (a Vitess/PlanetScale
-	// VStream import) via DBConfig.RejectReadOnly: that source is a read-only
-	// replica on purpose, and rejectReadOnly would turn its read-only
-	// responses into "driver: bad connection".
-	cfg.RejectReadOnly = config.RejectReadOnly
+	// Note: there is no rejectReadOnly to set. [DriverName] recycles a
+	// connection that reports a read-only error unconditionally — the option
+	// and its default-off are gone — so the blue/green and Aurora-failover
+	// protection spirit used to opt into is now simply how the driver behaves.
 	cfg.InterpolateParams = config.InterpolateParams
-	// Allow cleartext password authentication only when TLS is configured
-	// (required for AWS RDS IAM auth, safe because the connection uses TLS).
-	cfg.AllowCleartextPasswords = cfg.TLSConfig != ""
+	// Allow cleartext password authentication only when the connection is
+	// actually encrypted (required for AWS RDS IAM auth, safe because the
+	// connection uses TLS).
+	//
+	// Checking against tlsDisabledConfigName as well as "" is load-bearing:
+	// DISABLED now writes tls=false rather than leaving the field empty, and a
+	// bare `!= ""` would read that as "TLS is on" and start sending passwords
+	// in the clear over a plaintext connection.
+	cfg.AllowCleartextPasswords = cfg.TLSConfig != "" && cfg.TLSConfig != tlsDisabledConfigName
 	cfg.AllowNativePasswords = true
 
 	return cfg.FormatDSN(), nil
@@ -590,8 +630,6 @@ func GetTLSConfigForBinlog(config *DBConfig, host string) (*tls.Config, error) {
 			if err != nil {
 				return nil, fmt.Errorf("failed to load TLS certificate: %w", err)
 			}
-		} else {
-			certData = GetEmbeddedRDSBundle()
 		}
 		tlsConfig = NewCustomTLSConfig(certData, config.TLSMode)
 
@@ -612,8 +650,6 @@ func GetTLSConfigForBinlog(config *DBConfig, host string) (*tls.Config, error) {
 				if err != nil {
 					return nil, fmt.Errorf("failed to load TLS certificate: %w", err)
 				}
-			} else {
-				certData = GetEmbeddedRDSBundle()
 			}
 			tlsConfig = NewCustomTLSConfig(certData, config.TLSMode)
 		}
@@ -629,8 +665,6 @@ func GetTLSConfigForBinlog(config *DBConfig, host string) (*tls.Config, error) {
 			if err != nil {
 				return nil, fmt.Errorf("failed to load TLS certificate: %w", err)
 			}
-		} else {
-			certData = GetEmbeddedRDSBundle()
 		}
 		tlsConfig = NewCustomTLSConfig(certData, config.TLSMode)
 
@@ -645,8 +679,6 @@ func GetTLSConfigForBinlog(config *DBConfig, host string) (*tls.Config, error) {
 			if err != nil {
 				return nil, fmt.Errorf("failed to load TLS certificate: %w", err)
 			}
-		} else {
-			certData = GetEmbeddedRDSBundle()
 		}
 		tlsConfig = NewCustomTLSConfig(certData, config.TLSMode)
 
@@ -662,8 +694,6 @@ func GetTLSConfigForBinlog(config *DBConfig, host string) (*tls.Config, error) {
 			if err != nil {
 				return nil, fmt.Errorf("failed to load TLS certificate: %w", err)
 			}
-		} else {
-			certData = GetEmbeddedRDSBundle()
 		}
 		tlsConfig = NewCustomTLSConfig(certData, config.TLSMode)
 	}