From 814d7dc487f21d50a59e53f4ec9f22cf57cc784c Mon Sep 17 00:00:00 2001 From: Samantha Date: Thu, 13 Nov 2025 17:42:41 -0500 Subject: [PATCH 1/4] sa: Stop injecting max_statement_time and long_query_time into DSNs --- sa/database.go | 27 ------- sa/database_test.go | 86 ++++++---------------- test.sh | 1 + test/db.go | 17 ++++- test/secrets/backfiller_dburl | 1 - test/secrets/badkeyrevoker_dburl | 2 +- test/secrets/cert_checker_dburl | 2 +- test/secrets/expiration_mailer_dburl | 1 - test/secrets/incidents_dburl | 2 +- test/secrets/mailer_dburl | 1 - test/secrets/ocsp_responder_dburl | 1 - test/secrets/ocsp_responder_redis_password | 1 - test/secrets/revoker_dburl | 2 +- test/secrets/rocsp_tool_password | 1 - test/secrets/sa_dburl | 2 +- test/secrets/sa_ro_dburl | 2 +- test/vars/vars.go | 38 +++++++--- 17 files changed, 72 insertions(+), 115 deletions(-) delete mode 100644 test/secrets/backfiller_dburl delete mode 100644 test/secrets/expiration_mailer_dburl delete mode 100644 test/secrets/mailer_dburl delete mode 100644 test/secrets/ocsp_responder_dburl delete mode 100644 test/secrets/ocsp_responder_redis_password delete mode 100644 test/secrets/rocsp_tool_password diff --git a/sa/database.go b/sa/database.go index 0d06fac5f90..0f829453a8e 100644 --- a/sa/database.go +++ b/sa/database.go @@ -201,13 +201,6 @@ func adjustMySQLConfig(conf *mysql.Config) error { } } - // If a given parameter has the value "0", delete it from conf.Params. - omitZero := func(name string) { - if conf.Params[name] == "0" { - delete(conf.Params, name) - } - } - // Ensures that MySQL/MariaDB warnings are treated as errors. This // avoids a number of nasty edge conditions we could wander into. // Common things this discovers includes places where data being sent @@ -216,26 +209,6 @@ func adjustMySQLConfig(conf *mysql.Config) error { // . setDefault("sql_mode", "'STRICT_ALL_TABLES'") - // If a read timeout is set, we set max_statement_time to 95% of that, and - // long_query_time to 80% of that. That way we get logs of queries that are - // close to timing out but not yet doing so, and our queries get stopped by - // max_statement_time before timing out the read. This generates clearer - // errors, and avoids unnecessary reconnects. - // To override these values, set them in the DSN, e.g. - // `?max_statement_time=2`. A zero value in the DSN means these won't be - // sent on new connections. - if conf.ReadTimeout != 0 { - // In MariaDB, max_statement_time and long_query_time are both seconds, - // but can have up to microsecond granularity. - // Note: in MySQL (which we don't use), max_statement_time is millis. - readTimeout := conf.ReadTimeout.Seconds() - setDefault("max_statement_time", fmt.Sprintf("%.6f", readTimeout*0.95)) - setDefault("long_query_time", fmt.Sprintf("%.6f", readTimeout*0.80)) - } - - omitZero("max_statement_time") - omitZero("long_query_time") - // Finally, perform validation over all variables set by the DSN and via Boulder. for k, v := range conf.Params { err := checkMariaDBSystemVariables(k, v) diff --git a/sa/database_test.go b/sa/database_test.go index 1585c6d89cf..80c8d5b4426 100644 --- a/sa/database_test.go +++ b/sa/database_test.go @@ -4,6 +4,8 @@ import ( "context" "database/sql" "errors" + "fmt" + "net" "os" "path" "strings" @@ -17,23 +19,35 @@ import ( "github.com/letsencrypt/boulder/test/vars" ) +var dbAddr = func() string { + addr := os.Getenv("DB_ADDR") + if addr == "" { + panic("environment variable DB_ADDR must be set") + } + _, _, err := net.SplitHostPort(addr) + if err != nil { + panic(fmt.Sprintf("environment variable DB_ADDR (%s) is not a valid address with host and port: %s", addr, err)) + } + return addr +}() + func TestInvalidDSN(t *testing.T) { _, err := DBMapForTest("invalid") test.AssertError(t, err, "DB connect string missing the slash separating the database name") - DSN := "policy:password@tcp(boulder-proxysql:6033)/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&stringVarThatDoesntExist=%27whoopsidaisies" + DSN := "policy:password@tcp(" + dbAddr + ")/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&stringVarThatDoesntExist=%27whoopsidaisies" _, err = DBMapForTest(DSN) test.AssertError(t, err, "Variable does not exist in curated system var list, but didn't return an error and should have") - DSN = "policy:password@tcp(boulder-proxysql:6033)/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&concurrent_insert=2" + DSN = "policy:password@tcp(" + dbAddr + ")/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&concurrent_insert=2" _, err = DBMapForTest(DSN) test.AssertError(t, err, "Variable is unable to be set in the SESSION scope, but was declared") - DSN = "policy:password@tcp(boulder-proxysql:6033)/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&optimizer_switch=incorrect-quoted-string" + DSN = "policy:password@tcp(" + dbAddr + ")/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&optimizer_switch=incorrect-quoted-string" _, err = DBMapForTest(DSN) test.AssertError(t, err, "Variable declared with incorrect quoting") - DSN = "policy:password@tcp(boulder-proxysql:6033)/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&concurrent_insert=%272%27" + DSN = "policy:password@tcp(" + dbAddr + ")/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&concurrent_insert=%272%27" _, err = DBMapForTest(DSN) test.AssertError(t, err, "Integer enum declared, but should not have been quoted") } @@ -76,7 +90,7 @@ func TestDbSettings(t *testing.T) { } dsnFile := path.Join(t.TempDir(), "dbconnect") err := os.WriteFile(dsnFile, - []byte("sa@tcp(boulder-proxysql:6033)/boulder_sa_integration"), + []byte("sa@tcp("+dbAddr+")/boulder_sa_integration"), os.ModeAppend) test.AssertNotError(t, err, "writing dbconnect file") @@ -108,7 +122,7 @@ func TestDbSettings(t *testing.T) { // TODO: Change this to test `newDbMapFromMySQLConfig` instead? func TestNewDbMap(t *testing.T) { const mysqlConnectURL = "policy:password@tcp(boulder-proxysql:6033)/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms" - const expected = "policy:password@tcp(boulder-proxysql:6033)/boulder_policy_integration?clientFoundRows=true&parseTime=true&readTimeout=800ms&writeTimeout=800ms&long_query_time=0.640000&max_statement_time=0.760000&sql_mode=%27STRICT_ALL_TABLES%27" + const expected = "policy:password@tcp(boulder-proxysql:6033)/boulder_policy_integration?clientFoundRows=true&parseTime=true&readTimeout=800ms&writeTimeout=800ms&sql_mode=%27STRICT_ALL_TABLES%27" oldSQLOpen := sqlOpen defer func() { sqlOpen = oldSQLOpen @@ -146,27 +160,6 @@ func TestStrictness(t *testing.T) { } } -func TestTimeouts(t *testing.T) { - dbMap, err := DBMapForTest(vars.DBConnSA + "?max_statement_time=1") - if err != nil { - t.Fatal("Error setting up DB:", err) - } - // SLEEP is defined to return 1 if it was interrupted, but we want to actually - // get an error to simulate what would happen with a slow query. So we wrap - // the SLEEP in a subselect. - _, err = dbMap.ExecContext(ctx, `SELECT 1 FROM (SELECT SLEEP(5)) as subselect;`) - if err == nil { - t.Fatal("Expected error when running slow query, got none.") - } - - // We expect to get: - // Error 1969: Query execution was interrupted (max_statement_time exceeded) - // https://mariadb.com/kb/en/mariadb/mariadb-error-codes/ - if !strings.Contains(err.Error(), "Error 1969") { - t.Fatalf("Got wrong type of error: %s", err) - } -} - // TestAutoIncrementSchema tests that all of the tables in the boulder_* // databases that have auto_increment columns use BIGINT for the data type. Our // data is too big for INT. @@ -190,40 +183,7 @@ func TestAdjustMySQLConfig(t *testing.T) { conf := &mysql.Config{} err := adjustMySQLConfig(conf) test.AssertNotError(t, err, "unexpected err setting server variables") - test.AssertDeepEquals(t, conf.Params, map[string]string{ - "sql_mode": "'STRICT_ALL_TABLES'", - }) - - conf = &mysql.Config{ReadTimeout: 100 * time.Second} - err = adjustMySQLConfig(conf) - test.AssertNotError(t, err, "unexpected err setting server variables") - test.AssertDeepEquals(t, conf.Params, map[string]string{ - "sql_mode": "'STRICT_ALL_TABLES'", - "max_statement_time": "95.000000", - "long_query_time": "80.000000", - }) - - conf = &mysql.Config{ - ReadTimeout: 100 * time.Second, - Params: map[string]string{ - "max_statement_time": "0", - }, - } - err = adjustMySQLConfig(conf) - test.AssertNotError(t, err, "unexpected err setting server variables") - test.AssertDeepEquals(t, conf.Params, map[string]string{ - "sql_mode": "'STRICT_ALL_TABLES'", - "long_query_time": "80.000000", - }) - - conf = &mysql.Config{ - Params: map[string]string{ - "max_statement_time": "0", - }, - } - err = adjustMySQLConfig(conf) - test.AssertNotError(t, err, "unexpected err setting server variables") - test.AssertDeepEquals(t, conf.Params, map[string]string{ - "sql_mode": "'STRICT_ALL_TABLES'", - }) + test.Assert(t, conf.ParseTime, "ParseTime should be enabled") + test.Assert(t, conf.ClientFoundRows, "ClientFoundRows should be enabled") + test.AssertDeepEquals(t, conf.Params, map[string]string{"sql_mode": "'STRICT_ALL_TABLES'"}) } diff --git a/test.sh b/test.sh index 81a9dccfda9..2f55d25e97f 100755 --- a/test.sh +++ b/test.sh @@ -12,6 +12,7 @@ fi # Defaults # export RACE="false" +export DB_ADDR="boulder-proxysql:6033" STAGE="starting" STATUS="FAILURE" RUN=() diff --git a/test/db.go b/test/db.go index 13a4e5270f0..7044f84ee5d 100644 --- a/test/db.go +++ b/test/db.go @@ -5,11 +5,24 @@ import ( "database/sql" "fmt" "io" + "net" + "os" "testing" ) var ( - _ CleanUpDB = &sql.DB{} + _ CleanUpDB = &sql.DB{} + dbAddr = func() string { + addr := os.Getenv("DB_ADDR") + if addr == "" { + panic("environment variable DB_ADDR must be set") + } + _, _, err := net.SplitHostPort(addr) + if err != nil { + panic(fmt.Sprintf("environment variable DB_ADDR (%s) is not a valid address with host and port: %s", addr, err)) + } + return addr + }() ) // CleanUpDB is an interface with only what is needed to delete all @@ -40,7 +53,7 @@ func ResetIncidentsTestDatabase(t testing.TB) func() { } func resetTestDatabase(t testing.TB, ctx context.Context, dbPrefix string) func() { - db, err := sql.Open("mysql", fmt.Sprintf("test_setup@tcp(boulder-proxysql:6033)/%s_sa_test", dbPrefix)) + db, err := sql.Open("mysql", fmt.Sprintf("test_setup@tcp(%s)/%s_sa_test", dbAddr, dbPrefix)) if err != nil { t.Fatalf("Couldn't create db: %s", err) } diff --git a/test/secrets/backfiller_dburl b/test/secrets/backfiller_dburl deleted file mode 100644 index b62d870a545..00000000000 --- a/test/secrets/backfiller_dburl +++ /dev/null @@ -1 +0,0 @@ -sa@tcp(boulder-proxysql:6033)/boulder_sa_integration diff --git a/test/secrets/badkeyrevoker_dburl b/test/secrets/badkeyrevoker_dburl index 51f90c093be..db3796cccd8 100644 --- a/test/secrets/badkeyrevoker_dburl +++ b/test/secrets/badkeyrevoker_dburl @@ -1 +1 @@ -badkeyrevoker@tcp(boulder-proxysql:6033)/boulder_sa_integration +badkeyrevoker@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 diff --git a/test/secrets/cert_checker_dburl b/test/secrets/cert_checker_dburl index 16f6d8a8bf3..7e6aee421b7 100644 --- a/test/secrets/cert_checker_dburl +++ b/test/secrets/cert_checker_dburl @@ -1 +1 @@ -cert_checker@tcp(boulder-proxysql:6033)/boulder_sa_integration +cert_checker@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 diff --git a/test/secrets/expiration_mailer_dburl b/test/secrets/expiration_mailer_dburl deleted file mode 100644 index 615415cd8bb..00000000000 --- a/test/secrets/expiration_mailer_dburl +++ /dev/null @@ -1 +0,0 @@ -mailer@tcp(boulder-proxysql:6033)/boulder_sa_integration diff --git a/test/secrets/incidents_dburl b/test/secrets/incidents_dburl index 032afcfce71..31dede42932 100644 --- a/test/secrets/incidents_dburl +++ b/test/secrets/incidents_dburl @@ -1 +1 @@ -incidents_sa@tcp(boulder-proxysql:6033)/incidents_sa_integration?readTimeout=14s&timeout=1s +incidents_sa@tcp(boulder-proxysql:6033)/incidents_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 diff --git a/test/secrets/mailer_dburl b/test/secrets/mailer_dburl deleted file mode 100644 index 615415cd8bb..00000000000 --- a/test/secrets/mailer_dburl +++ /dev/null @@ -1 +0,0 @@ -mailer@tcp(boulder-proxysql:6033)/boulder_sa_integration diff --git a/test/secrets/ocsp_responder_dburl b/test/secrets/ocsp_responder_dburl deleted file mode 100644 index 4a789bad0b1..00000000000 --- a/test/secrets/ocsp_responder_dburl +++ /dev/null @@ -1 +0,0 @@ -ocsp_resp@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=800ms&writeTimeout=800ms&timeout=100ms diff --git a/test/secrets/ocsp_responder_redis_password b/test/secrets/ocsp_responder_redis_password deleted file mode 100644 index a132ec74b6a..00000000000 --- a/test/secrets/ocsp_responder_redis_password +++ /dev/null @@ -1 +0,0 @@ -0e5a4c8b5faaf3194c8ad83c3dd9a0dd8a75982b diff --git a/test/secrets/revoker_dburl b/test/secrets/revoker_dburl index 3e31508e869..b53be2b35d9 100644 --- a/test/secrets/revoker_dburl +++ b/test/secrets/revoker_dburl @@ -1 +1 @@ -revoker@tcp(boulder-proxysql:6033)/boulder_sa_integration +revoker@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 diff --git a/test/secrets/rocsp_tool_password b/test/secrets/rocsp_tool_password deleted file mode 100644 index f659bd3fc2e..00000000000 --- a/test/secrets/rocsp_tool_password +++ /dev/null @@ -1 +0,0 @@ -e4e9ce7845cb6adbbc44fb1d9deb05e6b4dc1386 diff --git a/test/secrets/sa_dburl b/test/secrets/sa_dburl index 4da95057bd5..84704344ffb 100644 --- a/test/secrets/sa_dburl +++ b/test/secrets/sa_dburl @@ -1 +1 @@ -sa@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s +sa@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 diff --git a/test/secrets/sa_ro_dburl b/test/secrets/sa_ro_dburl index 8e6cc85b50e..640ad40b39f 100644 --- a/test/secrets/sa_ro_dburl +++ b/test/secrets/sa_ro_dburl @@ -1 +1 @@ -sa_ro@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s +sa_ro@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 diff --git a/test/vars/vars.go b/test/vars/vars.go index deb2b56df95..ab6ad78574c 100644 --- a/test/vars/vars.go +++ b/test/vars/vars.go @@ -1,25 +1,41 @@ package vars -import "fmt" - -const ( - dbURL = "%s@tcp(boulder-proxysql:6033)/%s" +import ( + "fmt" + "net" + "os" ) +var dbAddr = func() string { + addr := os.Getenv("DB_ADDR") + if addr == "" { + panic("environment variable DB_ADDR must be set") + } + _, _, err := net.SplitHostPort(addr) + if err != nil { + panic(fmt.Sprintf("environment variable DB_ADDR (%s) is not a valid address with host and port: %s", addr, err)) + } + return addr +}() + +func dsn(user, database string) string { + return fmt.Sprintf("%s@tcp(%s)/%s", user, dbAddr, database) +} + var ( // DBConnSA is the sa database connection - DBConnSA = fmt.Sprintf(dbURL, "sa", "boulder_sa_test") + DBConnSA = dsn("sa", "boulder_sa_test") // DBConnSAMailer is the sa mailer database connection - DBConnSAMailer = fmt.Sprintf(dbURL, "mailer", "boulder_sa_test") + DBConnSAMailer = dsn("mailer", "boulder_sa_test") // DBConnSAFullPerms is the sa database connection with full perms - DBConnSAFullPerms = fmt.Sprintf(dbURL, "test_setup", "boulder_sa_test") + DBConnSAFullPerms = dsn("test_setup", "boulder_sa_test") // DBConnSAIntegrationFullPerms is the sa database connection for the // integration test DB, with full perms - DBConnSAIntegrationFullPerms = fmt.Sprintf(dbURL, "test_setup", "boulder_sa_integration") + DBConnSAIntegrationFullPerms = dsn("test_setup", "boulder_sa_integration") // DBInfoSchemaRoot is the root user and the information_schema connection. - DBInfoSchemaRoot = fmt.Sprintf(dbURL, "root", "information_schema") + DBInfoSchemaRoot = dsn("root", "information_schema") // DBConnIncidents is the incidents database connection. - DBConnIncidents = fmt.Sprintf(dbURL, "incidents_sa", "incidents_sa_test") + DBConnIncidents = dsn("incidents_sa", "incidents_sa_test") // DBConnIncidentsFullPerms is the incidents database connection with full perms. - DBConnIncidentsFullPerms = fmt.Sprintf(dbURL, "test_setup", "incidents_sa_test") + DBConnIncidentsFullPerms = dsn("test_setup", "incidents_sa_test") ) From 2611bc436b4dc4274684231f9a61533dbeb8f39a Mon Sep 17 00:00:00 2001 From: Samantha Date: Tue, 18 Nov 2025 12:31:06 -0500 Subject: [PATCH 2/4] Address comment. --- test/db.go | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/test/db.go b/test/db.go index 7044f84ee5d..a80cb0889a1 100644 --- a/test/db.go +++ b/test/db.go @@ -5,26 +5,13 @@ import ( "database/sql" "fmt" "io" - "net" - "os" "testing" -) -var ( - _ CleanUpDB = &sql.DB{} - dbAddr = func() string { - addr := os.Getenv("DB_ADDR") - if addr == "" { - panic("environment variable DB_ADDR must be set") - } - _, _, err := net.SplitHostPort(addr) - if err != nil { - panic(fmt.Sprintf("environment variable DB_ADDR (%s) is not a valid address with host and port: %s", addr, err)) - } - return addr - }() + "github.com/letsencrypt/boulder/test/vars" ) +var _ CleanUpDB = &sql.DB{} + // CleanUpDB is an interface with only what is needed to delete all // rows in all tables in a database plus close the database // connection. It is satisfied by *sql.DB. @@ -40,7 +27,7 @@ type CleanUpDB interface { // table as this is used by sql-migrate (https://github.com/rubenv/sql-migrate) // to track migrations. If it encounters an error it fails the tests. func ResetBoulderTestDatabase(t testing.TB) func() { - return resetTestDatabase(t, context.Background(), "boulder") + return resetTestDatabase(t, context.Background(), vars.DBConnSAFullPerms) } // ResetIncidentsTestDatabase returns a cleanup function which deletes all rows @@ -49,11 +36,11 @@ func ResetBoulderTestDatabase(t testing.TB) func() { // (https://github.com/rubenv/sql-migrate) to track migrations. If it encounters // an error it fails the tests. func ResetIncidentsTestDatabase(t testing.TB) func() { - return resetTestDatabase(t, context.Background(), "incidents") + return resetTestDatabase(t, context.Background(), vars.DBConnIncidentsFullPerms) } -func resetTestDatabase(t testing.TB, ctx context.Context, dbPrefix string) func() { - db, err := sql.Open("mysql", fmt.Sprintf("test_setup@tcp(%s)/%s_sa_test", dbAddr, dbPrefix)) +func resetTestDatabase(t testing.TB, ctx context.Context, dsn string) func() { + db, err := sql.Open("mysql", dsn) if err != nil { t.Fatalf("Couldn't create db: %s", err) } From 42f87c52d79ae4b68b40e7e5c9ae6ae7637e60ab Mon Sep 17 00:00:00 2001 From: Samantha Date: Wed, 19 Nov 2025 11:08:09 -0500 Subject: [PATCH 3/4] Address standup comments. --- sa/database.go | 12 ++++++++++++ test/secrets/badkeyrevoker_dburl | 2 +- test/secrets/cert_checker_dburl | 2 +- test/secrets/incidents_dburl | 2 +- test/secrets/revoker_dburl | 2 +- test/secrets/sa_dburl | 2 +- test/secrets/sa_ro_dburl | 2 +- 7 files changed, 18 insertions(+), 6 deletions(-) diff --git a/sa/database.go b/sa/database.go index 0f829453a8e..c307885b222 100644 --- a/sa/database.go +++ b/sa/database.go @@ -201,6 +201,13 @@ func adjustMySQLConfig(conf *mysql.Config) error { } } + omit := func(name string) { + _, ok := conf.Params[name] + if ok { + delete(conf.Params, name) + } + } + // Ensures that MySQL/MariaDB warnings are treated as errors. This // avoids a number of nasty edge conditions we could wander into. // Common things this discovers includes places where data being sent @@ -209,6 +216,11 @@ func adjustMySQLConfig(conf *mysql.Config) error { // . setDefault("sql_mode", "'STRICT_ALL_TABLES'") + // Omit max_statement_time and max_execution_time from the DSN. Query + // timeouts are managed exclusively by ProxySQL and/or Vitess. + omit("max_statement_time") + omit("max_execution_time") + // Finally, perform validation over all variables set by the DSN and via Boulder. for k, v := range conf.Params { err := checkMariaDBSystemVariables(k, v) diff --git a/test/secrets/badkeyrevoker_dburl b/test/secrets/badkeyrevoker_dburl index db3796cccd8..51f90c093be 100644 --- a/test/secrets/badkeyrevoker_dburl +++ b/test/secrets/badkeyrevoker_dburl @@ -1 +1 @@ -badkeyrevoker@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 +badkeyrevoker@tcp(boulder-proxysql:6033)/boulder_sa_integration diff --git a/test/secrets/cert_checker_dburl b/test/secrets/cert_checker_dburl index 7e6aee421b7..16f6d8a8bf3 100644 --- a/test/secrets/cert_checker_dburl +++ b/test/secrets/cert_checker_dburl @@ -1 +1 @@ -cert_checker@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 +cert_checker@tcp(boulder-proxysql:6033)/boulder_sa_integration diff --git a/test/secrets/incidents_dburl b/test/secrets/incidents_dburl index 31dede42932..032afcfce71 100644 --- a/test/secrets/incidents_dburl +++ b/test/secrets/incidents_dburl @@ -1 +1 @@ -incidents_sa@tcp(boulder-proxysql:6033)/incidents_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 +incidents_sa@tcp(boulder-proxysql:6033)/incidents_sa_integration?readTimeout=14s&timeout=1s diff --git a/test/secrets/revoker_dburl b/test/secrets/revoker_dburl index b53be2b35d9..3e31508e869 100644 --- a/test/secrets/revoker_dburl +++ b/test/secrets/revoker_dburl @@ -1 +1 @@ -revoker@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 +revoker@tcp(boulder-proxysql:6033)/boulder_sa_integration diff --git a/test/secrets/sa_dburl b/test/secrets/sa_dburl index 84704344ffb..4da95057bd5 100644 --- a/test/secrets/sa_dburl +++ b/test/secrets/sa_dburl @@ -1 +1 @@ -sa@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 +sa@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s diff --git a/test/secrets/sa_ro_dburl b/test/secrets/sa_ro_dburl index 640ad40b39f..8e6cc85b50e 100644 --- a/test/secrets/sa_ro_dburl +++ b/test/secrets/sa_ro_dburl @@ -1 +1 @@ -sa_ro@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s&max_statement_time=13&long_query_time=11 +sa_ro@tcp(boulder-proxysql:6033)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s From 71d49e59770628eca0d285620228c29cbd077be1 Mon Sep 17 00:00:00 2001 From: Samantha Date: Fri, 21 Nov 2025 16:12:11 -0500 Subject: [PATCH 4/4] Address comments. --- sa/database.go | 11 ++--------- sa/database_test.go | 27 ++++++--------------------- 2 files changed, 8 insertions(+), 30 deletions(-) diff --git a/sa/database.go b/sa/database.go index c307885b222..5d54d3aebfd 100644 --- a/sa/database.go +++ b/sa/database.go @@ -201,13 +201,6 @@ func adjustMySQLConfig(conf *mysql.Config) error { } } - omit := func(name string) { - _, ok := conf.Params[name] - if ok { - delete(conf.Params, name) - } - } - // Ensures that MySQL/MariaDB warnings are treated as errors. This // avoids a number of nasty edge conditions we could wander into. // Common things this discovers includes places where data being sent @@ -218,8 +211,8 @@ func adjustMySQLConfig(conf *mysql.Config) error { // Omit max_statement_time and max_execution_time from the DSN. Query // timeouts are managed exclusively by ProxySQL and/or Vitess. - omit("max_statement_time") - omit("max_execution_time") + delete(conf.Params, "max_statement_time") + delete(conf.Params, "max_execution_time") // Finally, perform validation over all variables set by the DSN and via Boulder. for k, v := range conf.Params { diff --git a/sa/database_test.go b/sa/database_test.go index 80c8d5b4426..95cfe07a977 100644 --- a/sa/database_test.go +++ b/sa/database_test.go @@ -4,8 +4,6 @@ import ( "context" "database/sql" "errors" - "fmt" - "net" "os" "path" "strings" @@ -19,35 +17,23 @@ import ( "github.com/letsencrypt/boulder/test/vars" ) -var dbAddr = func() string { - addr := os.Getenv("DB_ADDR") - if addr == "" { - panic("environment variable DB_ADDR must be set") - } - _, _, err := net.SplitHostPort(addr) - if err != nil { - panic(fmt.Sprintf("environment variable DB_ADDR (%s) is not a valid address with host and port: %s", addr, err)) - } - return addr -}() - func TestInvalidDSN(t *testing.T) { _, err := DBMapForTest("invalid") test.AssertError(t, err, "DB connect string missing the slash separating the database name") - DSN := "policy:password@tcp(" + dbAddr + ")/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&stringVarThatDoesntExist=%27whoopsidaisies" + DSN := "policy:password@tcp(foo-database:1337)/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&stringVarThatDoesntExist=%27whoopsidaisies" _, err = DBMapForTest(DSN) test.AssertError(t, err, "Variable does not exist in curated system var list, but didn't return an error and should have") - DSN = "policy:password@tcp(" + dbAddr + ")/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&concurrent_insert=2" + DSN = "policy:password@tcp(foo-database:1337)/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&concurrent_insert=2" _, err = DBMapForTest(DSN) test.AssertError(t, err, "Variable is unable to be set in the SESSION scope, but was declared") - DSN = "policy:password@tcp(" + dbAddr + ")/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&optimizer_switch=incorrect-quoted-string" + DSN = "policy:password@tcp(foo-database:1337)/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&optimizer_switch=incorrect-quoted-string" _, err = DBMapForTest(DSN) test.AssertError(t, err, "Variable declared with incorrect quoting") - DSN = "policy:password@tcp(" + dbAddr + ")/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&concurrent_insert=%272%27" + DSN = "policy:password@tcp(foo-database:1337)/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms&concurrent_insert=%272%27" _, err = DBMapForTest(DSN) test.AssertError(t, err, "Integer enum declared, but should not have been quoted") } @@ -89,9 +75,8 @@ func TestDbSettings(t *testing.T) { oldSetConnMaxIdleTime(db, connMaxIdleTime) } dsnFile := path.Join(t.TempDir(), "dbconnect") - err := os.WriteFile(dsnFile, - []byte("sa@tcp("+dbAddr+")/boulder_sa_integration"), - os.ModeAppend) + + err := os.WriteFile(dsnFile, []byte(vars.DBConnSA), os.ModeAppend) test.AssertNotError(t, err, "writing dbconnect file") config := cmd.DBConfig{