From 9fda3fb77da9389e613f3795aed1776d0ca1fde9 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 11 Feb 2019 10:46:07 -0800 Subject: [PATCH] Switch to DSNs (#4044) * Switch to DSNs We used to use "mysql+tcp://" URLs but we don't need those anymore, and there aren't any more of them in prod. * Fix test. --- cmd/config_test.go | 4 +-- cmd/testdata/test_dburl | 2 +- cmd/testdata/test_dburl_newline | 2 +- sa/database.go | 51 ---------------------------- sa/database_test.go | 4 +-- test/secrets/backfiller_dburl | 2 +- test/secrets/cert_checker_dburl | 2 +- test/secrets/expiration_mailer_dburl | 2 +- test/secrets/mailer_dburl | 2 +- test/secrets/ocsp_updater_dburl | 2 +- test/secrets/purger_dburl | 2 +- test/secrets/revoker_dburl | 2 +- test/secrets/sa_dburl | 2 +- 13 files changed, 14 insertions(+), 65 deletions(-) diff --git a/cmd/config_test.go b/cmd/config_test.go index 3d4567d1f..62e4b4ddd 100644 --- a/cmd/config_test.go +++ b/cmd/config_test.go @@ -18,12 +18,12 @@ func TestDBConfigURL(t *testing.T) { { // Test with one config file that has no trailing newline conf: DBConfig{DBConnectFile: "testdata/test_dburl"}, - expected: "mysql+tcp://test@testhost:3306/testDB?readTimeout=800ms&writeTimeout=800ms", + expected: "test@tcp(testhost:3306)/testDB?readTimeout=800ms&writeTimeout=800ms", }, { // Test with a config file that *has* a trailing newline conf: DBConfig{DBConnectFile: "testdata/test_dburl_newline"}, - expected: "mysql+tcp://test@testhost:3306/testDB?readTimeout=800ms&writeTimeout=800ms", + expected: "test@tcp(testhost:3306)/testDB?readTimeout=800ms&writeTimeout=800ms", }, } diff --git a/cmd/testdata/test_dburl b/cmd/testdata/test_dburl index 8207e8b42..c43b16c5d 100644 --- a/cmd/testdata/test_dburl +++ b/cmd/testdata/test_dburl @@ -1 +1 @@ -mysql+tcp://test@testhost:3306/testDB?readTimeout=800ms&writeTimeout=800ms +test@tcp(testhost:3306)/testDB?readTimeout=800ms&writeTimeout=800ms diff --git a/cmd/testdata/test_dburl_newline b/cmd/testdata/test_dburl_newline index 1e5ac734e..f2395d918 100644 --- a/cmd/testdata/test_dburl_newline +++ b/cmd/testdata/test_dburl_newline @@ -1,2 +1,2 @@ -mysql+tcp://test@testhost:3306/testDB?readTimeout=800ms&writeTimeout=800ms +test@tcp(testhost:3306)/testDB?readTimeout=800ms&writeTimeout=800ms diff --git a/sa/database.go b/sa/database.go index 792f1f0ad..84837d189 100644 --- a/sa/database.go +++ b/sa/database.go @@ -3,8 +3,6 @@ package sa import ( "database/sql" "fmt" - "net/url" - "strings" "time" "github.com/go-sql-driver/mysql" @@ -22,12 +20,6 @@ import ( func NewDbMap(dbConnect string, maxOpenConns int) (*gorp.DbMap, error) { var err error var config *mysql.Config - if strings.HasPrefix(dbConnect, "mysql+tcp://") { - dbConnect, err = recombineCustomMySQLURL(dbConnect) - if err != nil { - return nil, err - } - } config, err = mysql.ParseDSN(dbConnect) if err != nil { @@ -104,49 +96,6 @@ func adjustMySQLConfig(conf *mysql.Config) *mysql.Config { return conf } -// recombineCustomMySQLURL transforms the legacy database URLs into the -// URL-like strings expected by the mysql database driver. -// -// In the past, changes to the connection string were achieved by passing it -// into url.Parse and editing the query string that way, so the string had to -// be a valid URL. The mysql driver needs the Host data to be wrapped in -// "tcp()" but url.Parse will escape the parentheses and the mysql driver -// doesn't understand them. So, we couldn't have "tcp()" in the configs, but -// couldn't leave it out before passing it to the mysql driver. Similarly, the -// driver needs the password and username unescaped. The compromise was to do -// the leg work if the connection string's scheme is a fake one called -// "mysql+tcp://". -// -// Upon the addition of -// https://godoc.org/github.com/go-sql-driver/mysql#Config, this was no longer -// necessary, as the changes could be made on the decomposed struct version of -// the connection url. This method converts the old format into the format -// expected by the library. -func recombineCustomMySQLURL(dbConnect string) (string, error) { - dbConnect = strings.TrimSpace(dbConnect) - dbURL, err := url.Parse(dbConnect) - if err != nil { - return "", err - } - - if dbURL.Scheme != "mysql+tcp" { - format := "given database connection string was not a mysql+tcp:// URL, was %#v" - return "", fmt.Errorf(format, dbURL.Scheme) - } - - user := dbURL.User.Username() - passwd, hasPass := dbURL.User.Password() - dbConn := "" - if user != "" { - dbConn = url.QueryEscape(user) - } - if hasPass { - dbConn += ":" + passwd - } - dbConn += "@tcp(" + dbURL.Host + ")" - return dbConn + dbURL.EscapedPath() + "?" + dbURL.RawQuery, nil -} - // SetSQLDebug enables GORP SQL-level Debugging func SetSQLDebug(dbMap *gorp.DbMap, log blog.Logger) { dbMap.TraceOn("SQL: ", &SQLLogger{log}) diff --git a/sa/database_test.go b/sa/database_test.go index f504347ba..d267506bd 100644 --- a/sa/database_test.go +++ b/sa/database_test.go @@ -37,7 +37,7 @@ func TestMaxOpenConns(t *testing.T) { } func TestNewDbMap(t *testing.T) { - const mysqlConnectURL = "mysql+tcp://policy:password@boulder-mysql:3306/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms" + const mysqlConnectURL = "policy:password@tcp(boulder-mysql:3306)/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms" const expected = "policy:password@tcp(boulder-mysql:3306)/boulder_policy_integration?clientFoundRows=true&parseTime=true&readTimeout=800ms&writeTimeout=800ms&long_query_time=0.6400000000000001&max_statement_time=0.76&sql_mode=STRICT_ALL_TABLES" oldSQLOpen := sqlOpen defer func() { @@ -52,7 +52,7 @@ func TestNewDbMap(t *testing.T) { dbMap, err := NewDbMap(mysqlConnectURL, 0) if err != errExpected { - t.Errorf("got incorrect error: %v", err) + t.Errorf("got incorrect error. Got %v, expected %v", err, errExpected) } if dbMap != nil { t.Errorf("expected nil, got %v", dbMap) diff --git a/test/secrets/backfiller_dburl b/test/secrets/backfiller_dburl index 8ffb5cef7..68b6f3c3f 100644 --- a/test/secrets/backfiller_dburl +++ b/test/secrets/backfiller_dburl @@ -1 +1 @@ -mysql+tcp://sa@boulder-mysql:3306/boulder_sa_integration +sa@tcp(boulder-mysql:3306)/boulder_sa_integration diff --git a/test/secrets/cert_checker_dburl b/test/secrets/cert_checker_dburl index ed8f875a7..36259bb73 100644 --- a/test/secrets/cert_checker_dburl +++ b/test/secrets/cert_checker_dburl @@ -1 +1 @@ -mysql+tcp://cert_checker@boulder-mysql:3306/boulder_sa_integration +cert_checker@tcp(boulder-mysql:3306)/boulder_sa_integration diff --git a/test/secrets/expiration_mailer_dburl b/test/secrets/expiration_mailer_dburl index 30a7b4c79..ebc9ed019 100644 --- a/test/secrets/expiration_mailer_dburl +++ b/test/secrets/expiration_mailer_dburl @@ -1 +1 @@ -mysql+tcp://mailer@boulder-mysql:3306/boulder_sa_integration +mailer@tcp(boulder-mysql:3306)/boulder_sa_integration diff --git a/test/secrets/mailer_dburl b/test/secrets/mailer_dburl index 30a7b4c79..ebc9ed019 100644 --- a/test/secrets/mailer_dburl +++ b/test/secrets/mailer_dburl @@ -1 +1 @@ -mysql+tcp://mailer@boulder-mysql:3306/boulder_sa_integration +mailer@tcp(boulder-mysql:3306)/boulder_sa_integration diff --git a/test/secrets/ocsp_updater_dburl b/test/secrets/ocsp_updater_dburl index 22b0ecdb5..129766d3a 100644 --- a/test/secrets/ocsp_updater_dburl +++ b/test/secrets/ocsp_updater_dburl @@ -1 +1 @@ -mysql+tcp://ocsp_update@boulder-mysql:3306/boulder_sa_integration?readTimeout=800ms&writeTimeout=800ms&timeout=100ms +ocsp_update@tcp(boulder-mysql:3306)/boulder_sa_integration?readTimeout=800ms&writeTimeout=800ms&timeout=100ms diff --git a/test/secrets/purger_dburl b/test/secrets/purger_dburl index a54cbd04f..a720d3316 100644 --- a/test/secrets/purger_dburl +++ b/test/secrets/purger_dburl @@ -1 +1 @@ -mysql+tcp://purger@boulder-mysql:3306/boulder_sa_integration +purger@tcp(boulder-mysql:3306)/boulder_sa_integration diff --git a/test/secrets/revoker_dburl b/test/secrets/revoker_dburl index f2cea5be3..4f14dcc6f 100644 --- a/test/secrets/revoker_dburl +++ b/test/secrets/revoker_dburl @@ -1 +1 @@ -mysql+tcp://revoker@boulder-mysql:3306/boulder_sa_integration +revoker@tcp(boulder-mysql:3306)/boulder_sa_integration diff --git a/test/secrets/sa_dburl b/test/secrets/sa_dburl index 732835272..a438c0bf8 100644 --- a/test/secrets/sa_dburl +++ b/test/secrets/sa_dburl @@ -1 +1 @@ -mysql+tcp://sa@boulder-mysql:3306/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s +sa@tcp(boulder-mysql:3306)/boulder_sa_integration?readTimeout=14s&writeTimeout=14s&timeout=1s