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

.github/workflows/unit-tls.yml

@@ -0,0 +1,53 @@
+name: TLS/DSN unit tests (no database)
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - master
+      - main
+  pull_request:
+permissions:
+  contents: read
+
+# Spirit's TLS and DSN construction decides whether a connection is encrypted
+# and whether its server certificate is verified. Every one of those tests runs
+# against pure functions and needs no database — but until this workflow, the
+# only jobs that ran `go test` at all were the mysql*-docker matrices, so the
+# assertions were gated behind Docker, a MySQL boot, and each of those
+# workflows' documentation-only short-circuit.
+#
+# This job runs them on their own, on every pull request, with nothing to
+# provision. Keep it that way: a test matched by the filter below must not
+# require a server, or this workflow starts failing for reasons that have
+# nothing to do with TLS.
+jobs:
+  tls-dsn:
+    name: tls/dsn
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v7
+
+      - uses: actions/setup-go@v7.0.0
+        with:
+          go-version: "1.26.6"
+          # No module cache. This workflow also runs on release tags, and a
+          # cache restored there could have been written from a lower-trust
+          # context — the cache-poisoning class zizmor flags. linter.yml
+          # disables it for the same reason. The job compiles one package, so
+          # the cache buys almost nothing.
+          cache: false
+
+      # The filter is deliberately broad. It over-matches into neighbouring
+      # DSN-handling tests, which is the safe direction: a new TLS test whose
+      # name this misses is silently unprotected, whereas an extra pure test
+      # costs a second.
+      #
+      # This is pkg/dbconn only, and that is not the whole TLS surface:
+      # pkg/migration asserts these properties too (TestReplicaTLSEnhancement),
+      # but its TestMain provisions a server, so those stay in the mysql*-docker
+      # matrices. Do not read a green tick here as "the TLS assertions passed" —
+      # it means the ones that need no database did.
+      - name: TLS and DSN tests
+        run: go test -race -count=1 -run 'TLS|DSN|Certificate|RDS|Verify|Strict|Disabled|Registered' ./pkg/dbconn/

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