From 0fd4603ae6ef33f3ffe39cc21bda31bcf6f3dfd6 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Wed, 1 Nov 2017 09:51:45 +0200 Subject: [PATCH 01/17] minor updates to test --- localtests/datetime-submillis/create.sql | 2 +- localtests/test.sh | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/localtests/datetime-submillis/create.sql b/localtests/datetime-submillis/create.sql index b4e0b0b..6c04adb 100644 --- a/localtests/datetime-submillis/create.sql +++ b/localtests/datetime-submillis/create.sql @@ -17,7 +17,7 @@ create event gh_ost_test starts current_timestamp ends current_timestamp + interval 60 second on completion not preserve - enable + disable on slave do begin insert into gh_ost_test values (null, 11, now(), now(), now(), 0); diff --git a/localtests/test.sh b/localtests/test.sh index b901ce3..10cf796 100755 --- a/localtests/test.sh +++ b/localtests/test.sh @@ -164,6 +164,7 @@ test_single() { diff $orig_content_output_file $ghost_content_output_file echo "diff $orig_content_output_file $ghost_content_output_file" + return 1 fi } From 21d455013e9d8ce3b4892e44e3750aaf4fb58d51 Mon Sep 17 00:00:00 2001 From: MOON_CLJ Date: Fri, 27 Apr 2018 14:58:07 +0800 Subject: [PATCH 02/17] minor changes and typo correction --- go/binlog/binlog_entry.go | 4 ++-- go/binlog/gomysql_reader.go | 2 +- go/logic/applier.go | 10 +++++++--- go/logic/inspect.go | 3 +++ go/logic/migrator.go | 2 +- go/mysql/utils.go | 4 ---- go/sql/builder.go | 3 +-- 7 files changed, 15 insertions(+), 13 deletions(-) diff --git a/go/binlog/binlog_entry.go b/go/binlog/binlog_entry.go index bb70bc5..5650acc 100644 --- a/go/binlog/binlog_entry.go +++ b/go/binlog/binlog_entry.go @@ -26,7 +26,7 @@ func NewBinlogEntry(logFile string, logPos uint64) *BinlogEntry { return binlogEntry } -// NewBinlogEntry creates an empty, ready to go BinlogEntry object +// NewBinlogEntryAt creates an empty, ready to go BinlogEntry object func NewBinlogEntryAt(coordinates mysql.BinlogCoordinates) *BinlogEntry { binlogEntry := &BinlogEntry{ Coordinates: coordinates, @@ -41,7 +41,7 @@ func (this *BinlogEntry) Duplicate() *BinlogEntry { return binlogEntry } -// Duplicate creates and returns a new binlog entry, with some of the attributes pre-assigned +// String() returns a string representation of this binlog entry func (this *BinlogEntry) String() string { return fmt.Sprintf("[BinlogEntry at %+v; dml:%+v]", this.Coordinates, this.DmlEvent) } diff --git a/go/binlog/gomysql_reader.go b/go/binlog/gomysql_reader.go index a479c99..b86bd72 100644 --- a/go/binlog/gomysql_reader.go +++ b/go/binlog/gomysql_reader.go @@ -111,7 +111,7 @@ func (this *GoMySQLReader) handleRowsEvent(ev *replication.BinlogEvent, rowsEven binlogEntry.DmlEvent.WhereColumnValues = sql.ToColumnValues(row) } } - // The channel will do the throttling. Whoever is reding from the channel + // The channel will do the throttling. Whoever is reading from the channel // decides whether action is taken synchronously (meaning we wait before // next iteration) or asynchronously (we keep pushing more events) // In reality, reads will be synchronous diff --git a/go/logic/applier.go b/go/logic/applier.go index afdc45f..0de5da4 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -126,7 +126,6 @@ func (this *Applier) readTableColumns() (err error) { // showTableStatus returns the output of `show table status like '...'` command func (this *Applier) showTableStatus(tableName string) (rowMap sqlutils.RowMap) { - rowMap = nil query := fmt.Sprintf(`show /* gh-ost */ table status from %s like '%s'`, sql.EscapeName(this.migrationContext.DatabaseName), tableName) sqlutils.QueryRowsMap(this.db, query, func(m sqlutils.RowMap) error { rowMap = m @@ -482,6 +481,7 @@ func (this *Applier) ApplyIterationInsertQuery() (chunkSize int64, rowsAffected if err != nil { return nil, err } + defer tx.Rollback() sessionQuery := fmt.Sprintf(`SET SESSION time_zone = '%s', sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES') @@ -1001,15 +1001,19 @@ func (this *Applier) ApplyDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) error { if err != nil { return err } + rollback := func(err error) error { + tx.Rollback() + return err + } sessionQuery := `SET SESSION time_zone = '+00:00', sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES') ` if _, err := tx.Exec(sessionQuery); err != nil { - return err + return rollback(err) } if _, err := tx.Exec(buildResult.query, buildResult.args...); err != nil { - return err + return rollback(err) } if err := tx.Commit(); err != nil { return err diff --git a/go/logic/inspect.go b/go/logic/inspect.go index 2568afe..4d25ab1 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -699,14 +699,17 @@ func (this *Inspector) getSharedColumns(originalColumns, ghostColumns *sql.Colum for _, ghostColumn := range ghostColumns.Names() { if strings.EqualFold(originalColumn, ghostColumn) { isSharedColumn = true + break } if strings.EqualFold(columnRenameMap[originalColumn], ghostColumn) { isSharedColumn = true + break } } for droppedColumn := range this.migrationContext.DroppedColumnsMap { if strings.EqualFold(originalColumn, droppedColumn) { isSharedColumn = false + break } } if isSharedColumn { diff --git a/go/logic/migrator.go b/go/logic/migrator.go index f52e2b8..f536de8 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -78,7 +78,7 @@ type Migrator struct { rowCopyCompleteFlag int64 // copyRowsQueue should not be buffered; if buffered some non-damaging but - // excessive work happens at the end of the iteration as new copy-jobs arrive befroe realizing the copy is complete + // excessive work happens at the end of the iteration as new copy-jobs arrive before realizing the copy is complete copyRowsQueue chan tableWriteFunc applyEventsQueue chan *applyEventStruct diff --git a/go/mysql/utils.go b/go/mysql/utils.go index 532cbb4..2ca8cc2 100644 --- a/go/mysql/utils.go +++ b/go/mysql/utils.go @@ -83,9 +83,6 @@ func GetMasterKeyFromSlaveStatus(connectionConfig *ConnectionConfig) (masterKey } defer db.Close() - if err != nil { - return nil, err - } err = sqlutils.QueryRowsMap(db, `show slave status`, func(rowMap sqlutils.RowMap) error { // We wish to recognize the case where the topology's master actually has replication configuration. // This can happen when a DBA issues a `RESET SLAVE` instead of `RESET SLAVE ALL`. @@ -98,7 +95,6 @@ func GetMasterKeyFromSlaveStatus(connectionConfig *ConnectionConfig) (masterKey slaveIORunning := rowMap.GetString("Slave_IO_Running") slaveSQLRunning := rowMap.GetString("Slave_SQL_Running") - // if slaveIORunning != "Yes" || slaveSQLRunning != "Yes" { return fmt.Errorf("Replication on %+v is broken: Slave_IO_Running: %s, Slave_SQL_Running: %s. Please make sure replication runs before using gh-ost.", connectionConfig.Key, diff --git a/go/sql/builder.go b/go/sql/builder.go index c3a6229..2c5a7ae 100644 --- a/go/sql/builder.go +++ b/go/sql/builder.go @@ -140,13 +140,12 @@ func BuildRangeComparison(columns []string, values []string, args []interface{}, comparisons := []string{} for i, column := range columns { - // value := values[i] rangeComparison, err := BuildValueComparison(column, value, comparisonSign) if err != nil { return "", explodedArgs, err } - if len(columns[0:i]) > 0 { + if i > 0 { equalitiesComparison, err := BuildEqualsComparison(columns[0:i], values[0:i]) if err != nil { return "", explodedArgs, err From 14eda7efe06f5eed297c0f243bd870aa6b6de8b4 Mon Sep 17 00:00:00 2001 From: MOON_CLJ Date: Sat, 28 Apr 2018 12:20:28 +0800 Subject: [PATCH 03/17] fix GetReplicationLag not used args --- go/logic/inspect.go | 3 +-- go/logic/throttler.go | 4 ++-- go/mysql/utils.go | 5 ++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/go/logic/inspect.go b/go/logic/inspect.go index 4d25ab1..db6254d 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -758,9 +758,8 @@ func (this *Inspector) getMasterConnectionConfig() (applierConfig *mysql.Connect } func (this *Inspector) getReplicationLag() (replicationLag time.Duration, err error) { - replicationLag, err = mysql.GetReplicationLag( + replicationLag, err = mysql.GetReplicationLagFromSlaveStatus( this.informationSchemaDb, - this.migrationContext.InspectorConnectionConfig, ) return replicationLag, err } diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 624956a..6f9f4bb 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -140,8 +140,8 @@ func (this *Throttler) collectReplicationLag(firstThrottlingCollected chan<- boo if this.migrationContext.TestOnReplica || this.migrationContext.MigrateOnReplica { // when running on replica, the heartbeat injection is also done on the replica. // This means we will always get a good heartbeat value. - // When runnign on replica, we should instead check the `SHOW SLAVE STATUS` output. - if lag, err := mysql.GetReplicationLag(this.inspector.informationSchemaDb, this.inspector.connectionConfig); err != nil { + // When running on replica, we should instead check the `SHOW SLAVE STATUS` output. + if lag, err := mysql.GetReplicationLagFromSlaveStatus(this.inspector.informationSchemaDb); err != nil { return log.Errore(err) } else { atomic.StoreInt64(&this.migrationContext.CurrentLag, int64(lag)) diff --git a/go/mysql/utils.go b/go/mysql/utils.go index 2ca8cc2..c96aebd 100644 --- a/go/mysql/utils.go +++ b/go/mysql/utils.go @@ -57,9 +57,8 @@ func GetDB(migrationUuid string, mysql_uri string) (*gosql.DB, bool, error) { return knownDBs[cacheKey], exists, nil } -// GetReplicationLag returns replication lag for a given connection config; either by explicit query -// or via SHOW SLAVE STATUS -func GetReplicationLag(informationSchemaDb *gosql.DB, connectionConfig *ConnectionConfig) (replicationLag time.Duration, err error) { +// GetReplicationLagFromSlaveStatus returns replication lag for a given db; via SHOW SLAVE STATUS +func GetReplicationLagFromSlaveStatus(informationSchemaDb *gosql.DB) (replicationLag time.Duration, err error) { err = sqlutils.QueryRowsMap(informationSchemaDb, `show slave status`, func(m sqlutils.RowMap) error { slaveIORunning := m.GetString("Slave_IO_Running") slaveSQLRunning := m.GetString("Slave_SQL_Running") From 5eec74132750f4119b1b761e7607aeee83928a67 Mon Sep 17 00:00:00 2001 From: Will Currie Date: Wed, 16 Jan 2019 06:28:05 +1100 Subject: [PATCH 04/17] Add Why Is "Connect to Replica" mode preferred? --- doc/questions.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/questions.md b/doc/questions.md index be6eab0..7585bc7 100644 --- a/doc/questions.md +++ b/doc/questions.md @@ -28,3 +28,9 @@ It is therefore unlikely that `gh-ost` will support this behavior. Yes. TL;DR if running all on same replica/master, make sure to provide `--replica-server-id`. [Read more](cheatsheet.md#concurrent-migrations) # Why + +### Why Is the "Connect to Replica" mode preferred? + +To avoid placing extra load on the master. `gh-ost` connects as a replication client. Each additional replica adds some load to the master. + +To monitor replication lag from a replica. This makes the replication lag throttle, `--max-lag-millis`, more representative of the lag experienced by other replicas following the master (perhaps N levels deep in a tree of replicas). From afa108f8fa49ec66a3b873c094f1bd09e4aaa169 Mon Sep 17 00:00:00 2001 From: twotwotwo Date: Tue, 22 Jan 2019 12:26:55 -0800 Subject: [PATCH 05/17] inspect: remove redundant join conditions The TABLE_SCHEMA and TABLE_NAME are already guaranteed to have the same value in COLUMNS and UNIQUES because of the WHEREs in each query. Dropping it from the ON clause makes it complete much faster. On (at least) MySQL 5.6 installs with thousands of tables, this query completes in a few seconds without the additional join conditions, but takes more than a minute with it. --- go/logic/inspect.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/go/logic/inspect.go b/go/logic/inspect.go index 5f875a0..5643c58 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -622,8 +622,6 @@ func (this *Inspector) getCandidateUniqueKeys(tableName string) (uniqueKeys [](* GROUP BY TABLE_SCHEMA, TABLE_NAME, INDEX_NAME ) AS UNIQUES ON ( - COLUMNS.TABLE_SCHEMA = UNIQUES.TABLE_SCHEMA AND - COLUMNS.TABLE_NAME = UNIQUES.TABLE_NAME AND COLUMNS.COLUMN_NAME = UNIQUES.FIRST_COLUMN_NAME ) WHERE From 3161cd5823ccb3e213f8783e972c082f72ad6571 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Thu, 24 Jan 2019 13:06:20 +0200 Subject: [PATCH 06/17] added: --hooks-hint-owner, --hooks-hint-token --- doc/hooks.md | 2 ++ go/base/context.go | 2 ++ go/cmd/gh-ost/main.go | 2 ++ go/logic/hooks.go | 2 ++ 4 files changed, 8 insertions(+) diff --git a/doc/hooks.md b/doc/hooks.md index 1b1958f..93f500f 100644 --- a/doc/hooks.md +++ b/doc/hooks.md @@ -69,6 +69,8 @@ The following variables are available on all hooks: - `GH_OST_INSPECTED_HOST` - `GH_OST_EXECUTING_HOST` - `GH_OST_HOOKS_HINT` - copy of `--hooks-hint` value +- `GH_OST_HOOKS_HINT_OWNER` - copy of `--hooks-hint-owner` value +- `GH_OST_HOOKS_HINT_TOKEN` - copy of `--hooks-hint-token` value - `GH_OST_DRY_RUN` - whether or not the `gh-ost` run is a dry run The following variable are available on particular hooks: diff --git a/go/base/context.go b/go/base/context.go index 0043d07..22a1fd0 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -126,6 +126,8 @@ type MigrationContext struct { PanicFlagFile string HooksPath string HooksHintMessage string + HooksHintOwner string + HooksHintToken string DropServeSocket bool ServeSocketFile string diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 3024702..9a4407b 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -109,6 +109,8 @@ func main() { flag.StringVar(&migrationContext.HooksPath, "hooks-path", "", "directory where hook files are found (default: empty, ie. hooks disabled). Hook files found on this path, and conforming to hook naming conventions will be executed") flag.StringVar(&migrationContext.HooksHintMessage, "hooks-hint", "", "arbitrary message to be injected to hooks via GH_OST_HOOKS_HINT, for your convenience") + flag.StringVar(&migrationContext.HooksHintOwner, "hooks-hint-owner", "", "arbitrary name of owner to be injected to hooks via GH_OST_HOOKS_HINT_OWNER, for your convenience") + flag.StringVar(&migrationContext.HooksHintToken, "hooks-hint-token", "", "arbitrary token to be injected to hooks via GH_OST_HOOKS_HINT_TOKEN, for your convenience") flag.UintVar(&migrationContext.ReplicaServerId, "replica-server-id", 99999, "server id used by gh-ost process. Default: 99999") diff --git a/go/logic/hooks.go b/go/logic/hooks.go index 6515d7f..be11130 100644 --- a/go/logic/hooks.go +++ b/go/logic/hooks.go @@ -64,6 +64,8 @@ func (this *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) [ env = append(env, fmt.Sprintf("GH_OST_INSPECTED_HOST=%s", this.migrationContext.GetInspectorHostname())) env = append(env, fmt.Sprintf("GH_OST_EXECUTING_HOST=%s", this.migrationContext.Hostname)) env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT=%s", this.migrationContext.HooksHintMessage)) + env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT_OWNER=%s", this.migrationContext.HooksHintOwner)) + env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT_TOKEN=%s", this.migrationContext.HooksHintToken)) env = append(env, fmt.Sprintf("GH_OST_DRY_RUN=%t", this.migrationContext.Noop)) for _, variable := range extraVariables { From 23617f287fbf0903e8979ec4ba5086b032e0bc23 Mon Sep 17 00:00:00 2001 From: Brandon Bodnar <33429657+brandonbodnar-wk@users.noreply.github.com> Date: Thu, 31 Jan 2019 10:03:48 -0600 Subject: [PATCH 07/17] Add initial support for ssl encryption connections to database servers. - Adding a command line option for users to enforce tls/ssl connections for the applier, inspector, and binlog reader. - The user can optionally request server certificate verification through a command line option to specify the ca cert via a file path. - Fixes an existing bug appending the timeout option to the singleton applier connection. --- go/base/context.go | 9 +++++++ go/binlog/gomysql_reader.go | 1 + go/cmd/gh-ost/main.go | 8 ++++++ go/logic/applier.go | 2 +- go/mysql/connection.go | 51 +++++++++++++++++++++++++++++++++---- go/mysql/connection_test.go | 19 +++++++++++++- 6 files changed, 83 insertions(+), 7 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 0043d07..856a29c 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -99,6 +99,8 @@ type MigrationContext struct { ConfigFile string CliUser string CliPassword string + UseTLS bool + TlsCACertificate string CliMasterUser string CliMasterPassword string @@ -695,6 +697,13 @@ func (this *MigrationContext) ApplyCredentials() { } } +func (this *MigrationContext) SetupTLS() error { + if this.UseTLS { + return this.InspectorConnectionConfig.UseTLS(this.TlsCACertificate) + } + return nil +} + // ReadConfigFile attempts to read the config file, if it exists func (this *MigrationContext) ReadConfigFile() error { this.configMutex.Lock() diff --git a/go/binlog/gomysql_reader.go b/go/binlog/gomysql_reader.go index 794d545..41d1ead 100644 --- a/go/binlog/gomysql_reader.go +++ b/go/binlog/gomysql_reader.go @@ -46,6 +46,7 @@ func NewGoMySQLReader(migrationContext *base.MigrationContext) (binlogReader *Go Port: uint16(binlogReader.connectionConfig.Key.Port), User: binlogReader.connectionConfig.User, Password: binlogReader.connectionConfig.Password, + TLSConfig: binlogReader.connectionConfig.TLSConfig(), UseDecimal: true, } binlogReader.binlogSyncer = replication.NewBinlogSyncer(binlogSyncerConfig) diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 3024702..8bd8c78 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -14,6 +14,7 @@ import ( "github.com/github/gh-ost/go/base" "github.com/github/gh-ost/go/logic" + _ "github.com/go-sql-driver/mysql" "github.com/outbrain/golib/log" "golang.org/x/crypto/ssh/terminal" @@ -54,6 +55,9 @@ func main() { flag.StringVar(&migrationContext.ConfigFile, "conf", "", "Config file") askPass := flag.Bool("ask-pass", false, "prompt for MySQL password") + flag.BoolVar(&migrationContext.UseTLS, "ssl", false, "Enable SSL encrypted connections to MySQL") + flag.StringVar(&migrationContext.TlsCACertificate, "ssl-ca", "", "CA certificate in PEM format for TLS connections. Requires --ssl") + flag.StringVar(&migrationContext.DatabaseName, "database", "", "database name (mandatory)") flag.StringVar(&migrationContext.OriginalTableName, "table", "", "table name (mandatory)") flag.StringVar(&migrationContext.AlterStatement, "alter", "", "alter statement (mandatory)") @@ -194,6 +198,9 @@ func main() { if migrationContext.CliMasterPassword != "" && migrationContext.AssumeMasterHostname == "" { log.Fatalf("--master-password requires --assume-master-host") } + if migrationContext.TlsCACertificate != "" && !migrationContext.UseTLS { + log.Fatalf("--ssl-ca requires --ssl") + } if *replicationLagQuery != "" { log.Warningf("--replication-lag-query is deprecated") } @@ -238,6 +245,7 @@ func main() { migrationContext.SetThrottleHTTP(*throttleHTTP) migrationContext.SetDefaultNumRetries(*defaultRetries) migrationContext.ApplyCredentials() + migrationContext.SetupTLS() if err := migrationContext.SetCutOverLockTimeoutSeconds(*cutOverLockTimeoutSeconds); err != nil { log.Errore(err) } diff --git a/go/logic/applier.go b/go/logic/applier.go index 140b1a4..b3de0e1 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -73,7 +73,7 @@ func (this *Applier) InitDBConnections() (err error) { if this.db, _, err = mysql.GetDB(this.migrationContext.Uuid, applierUri); err != nil { return err } - singletonApplierUri := fmt.Sprintf("%s?timeout=0", applierUri) + singletonApplierUri := fmt.Sprintf("%s&timeout=0", applierUri) if this.singletonDB, _, err = mysql.GetDB(this.migrationContext.Uuid, singletonApplierUri); err != nil { return err } diff --git a/go/mysql/connection.go b/go/mysql/connection.go index c9c75f2..bfb445a 100644 --- a/go/mysql/connection.go +++ b/go/mysql/connection.go @@ -6,8 +6,14 @@ package mysql import ( + "crypto/tls" + "crypto/x509" + "errors" "fmt" + "io/ioutil" "net" + + "github.com/go-sql-driver/mysql" ) // ConnectionConfig is the minimal configuration required to connect to a MySQL server @@ -16,6 +22,7 @@ type ConnectionConfig struct { User string Password string ImpliedKey *InstanceKey + tlsConfig *tls.Config } func NewConnectionConfig() *ConnectionConfig { @@ -29,9 +36,10 @@ func NewConnectionConfig() *ConnectionConfig { // DuplicateCredentials creates a new connection config with given key and with same credentials as this config func (this *ConnectionConfig) DuplicateCredentials(key InstanceKey) *ConnectionConfig { config := &ConnectionConfig{ - Key: key, - User: this.User, - Password: this.Password, + Key: key, + User: this.User, + Password: this.Password, + tlsConfig: this.tlsConfig, } config.ImpliedKey = &config.Key return config @@ -42,13 +50,42 @@ func (this *ConnectionConfig) Duplicate() *ConnectionConfig { } func (this *ConnectionConfig) String() string { - return fmt.Sprintf("%s, user=%s", this.Key.DisplayString(), this.User) + return fmt.Sprintf("%s, user=%s, usingTLS=%t", this.Key.DisplayString(), this.User, this.tlsConfig != nil) } func (this *ConnectionConfig) Equals(other *ConnectionConfig) bool { return this.Key.Equals(&other.Key) || this.ImpliedKey.Equals(other.ImpliedKey) } +func (this *ConnectionConfig) UseTLS(caCertificatePath string) error { + skipVerify := caCertificatePath == "" + var rootCertPool *x509.CertPool + if !skipVerify { + rootCertPool = x509.NewCertPool() + pem, err := ioutil.ReadFile(caCertificatePath) + if err != nil { + return err + } + if ok := rootCertPool.AppendCertsFromPEM(pem); !ok { + return errors.New("could not add ca certificate to cert pool") + } + } + + this.tlsConfig = &tls.Config{ + RootCAs: rootCertPool, + InsecureSkipVerify: skipVerify, + } + + if err := mysql.RegisterTLSConfig(this.Key.StringCode(), this.tlsConfig); err != nil { + return err + } + return nil +} + +func (this *ConnectionConfig) TLSConfig() *tls.Config { + return this.tlsConfig +} + func (this *ConnectionConfig) GetDBUri(databaseName string) string { hostname := this.Key.Hostname var ip = net.ParseIP(hostname) @@ -57,5 +94,9 @@ func (this *ConnectionConfig) GetDBUri(databaseName string) string { hostname = fmt.Sprintf("[%s]", hostname) } interpolateParams := true - return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=%t&autocommit=true&charset=utf8mb4,utf8,latin1", this.User, this.Password, hostname, this.Key.Port, databaseName, interpolateParams) + tlsOption := "false" + if this.tlsConfig != nil { + tlsOption = this.Key.StringCode() + } + return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=%t&autocommit=true&charset=utf8mb4,utf8,latin1&tls=%s", this.User, this.Password, hostname, this.Key.Port, databaseName, interpolateParams, tlsOption) } diff --git a/go/mysql/connection_test.go b/go/mysql/connection_test.go index 19badf3..3a6cf29 100644 --- a/go/mysql/connection_test.go +++ b/go/mysql/connection_test.go @@ -6,6 +6,7 @@ package mysql import ( + "crypto/tls" "testing" "github.com/outbrain/golib/log" @@ -31,6 +32,10 @@ func TestDuplicateCredentials(t *testing.T) { c.Key = InstanceKey{Hostname: "myhost", Port: 3306} c.User = "gromit" c.Password = "penguin" + c.tlsConfig = &tls.Config{ + InsecureSkipVerify: true, + ServerName: "feathers", + } dup := c.DuplicateCredentials(InstanceKey{Hostname: "otherhost", Port: 3310}) test.S(t).ExpectEquals(dup.Key.Hostname, "otherhost") @@ -39,6 +44,7 @@ func TestDuplicateCredentials(t *testing.T) { test.S(t).ExpectEquals(dup.ImpliedKey.Port, 3310) test.S(t).ExpectEquals(dup.User, "gromit") test.S(t).ExpectEquals(dup.Password, "penguin") + test.S(t).ExpectEquals(dup.tlsConfig, c.tlsConfig) } func TestDuplicate(t *testing.T) { @@ -63,5 +69,16 @@ func TestGetDBUri(t *testing.T) { c.Password = "penguin" uri := c.GetDBUri("test") - test.S(t).ExpectEquals(uri, "gromit:penguin@tcp(myhost:3306)/test?interpolateParams=true&autocommit=true&charset=utf8mb4,utf8,latin1") + test.S(t).ExpectEquals(uri, "gromit:penguin@tcp(myhost:3306)/test?interpolateParams=true&autocommit=true&charset=utf8mb4,utf8,latin1&tls=false") +} + +func TestGetDBUriWithTLSSetup(t *testing.T) { + c := NewConnectionConfig() + c.Key = InstanceKey{Hostname: "myhost", Port: 3306} + c.User = "gromit" + c.Password = "penguin" + c.tlsConfig = &tls.Config{} + + uri := c.GetDBUri("test") + test.S(t).ExpectEquals(uri, "gromit:penguin@tcp(myhost:3306)/test?interpolateParams=true&autocommit=true&charset=utf8mb4,utf8,latin1&tls=myhost:3306") } From 4f9367e6909dc93d824cda9f536921e449cf689e Mon Sep 17 00:00:00 2001 From: Brandon Bodnar <33429657+brandonbodnar-wk@users.noreply.github.com> Date: Thu, 31 Jan 2019 16:59:42 -0600 Subject: [PATCH 08/17] Fix casing for initialism --- go/base/context.go | 4 ++-- go/cmd/gh-ost/main.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 856a29c..90581e2 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -100,7 +100,7 @@ type MigrationContext struct { CliUser string CliPassword string UseTLS bool - TlsCACertificate string + TLSCACertificate string CliMasterUser string CliMasterPassword string @@ -699,7 +699,7 @@ func (this *MigrationContext) ApplyCredentials() { func (this *MigrationContext) SetupTLS() error { if this.UseTLS { - return this.InspectorConnectionConfig.UseTLS(this.TlsCACertificate) + return this.InspectorConnectionConfig.UseTLS(this.TLSCACertificate) } return nil } diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 8bd8c78..db044f3 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -56,7 +56,7 @@ func main() { askPass := flag.Bool("ask-pass", false, "prompt for MySQL password") flag.BoolVar(&migrationContext.UseTLS, "ssl", false, "Enable SSL encrypted connections to MySQL") - flag.StringVar(&migrationContext.TlsCACertificate, "ssl-ca", "", "CA certificate in PEM format for TLS connections. Requires --ssl") + flag.StringVar(&migrationContext.TLSCACertificate, "ssl-ca", "", "CA certificate in PEM format for TLS connections. Requires --ssl") flag.StringVar(&migrationContext.DatabaseName, "database", "", "database name (mandatory)") flag.StringVar(&migrationContext.OriginalTableName, "table", "", "table name (mandatory)") @@ -198,7 +198,7 @@ func main() { if migrationContext.CliMasterPassword != "" && migrationContext.AssumeMasterHostname == "" { log.Fatalf("--master-password requires --assume-master-host") } - if migrationContext.TlsCACertificate != "" && !migrationContext.UseTLS { + if migrationContext.TLSCACertificate != "" && !migrationContext.UseTLS { log.Fatalf("--ssl-ca requires --ssl") } if *replicationLagQuery != "" { From dc599bb0368f0ccd15a605348a6615cfe8f03faf Mon Sep 17 00:00:00 2001 From: Brandon Bodnar <33429657+brandonbodnar-wk@users.noreply.github.com> Date: Thu, 31 Jan 2019 17:20:11 -0600 Subject: [PATCH 09/17] Remove unnecessary branching --- go/mysql/connection.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/go/mysql/connection.go b/go/mysql/connection.go index bfb445a..7fa8268 100644 --- a/go/mysql/connection.go +++ b/go/mysql/connection.go @@ -76,10 +76,7 @@ func (this *ConnectionConfig) UseTLS(caCertificatePath string) error { InsecureSkipVerify: skipVerify, } - if err := mysql.RegisterTLSConfig(this.Key.StringCode(), this.tlsConfig); err != nil { - return err - } - return nil + return mysql.RegisterTLSConfig(this.Key.StringCode(), this.tlsConfig) } func (this *ConnectionConfig) TLSConfig() *tls.Config { From c440112d440112e819d17f8d6997f3b41882edac Mon Sep 17 00:00:00 2001 From: Brandon Bodnar <33429657+brandonbodnar-wk@users.noreply.github.com> Date: Thu, 31 Jan 2019 17:23:19 -0600 Subject: [PATCH 10/17] Explain default setting for TLS param in DSN --- go/mysql/connection.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go/mysql/connection.go b/go/mysql/connection.go index 7fa8268..742ca7e 100644 --- a/go/mysql/connection.go +++ b/go/mysql/connection.go @@ -91,6 +91,8 @@ func (this *ConnectionConfig) GetDBUri(databaseName string) string { hostname = fmt.Sprintf("[%s]", hostname) } interpolateParams := true + // go-mysql-driver defaults to false if tls param is not provided; explicitly setting here to + // simplify construction of the DSN below. tlsOption := "false" if this.tlsConfig != nil { tlsOption = this.Key.StringCode() From f67ec15f2b972fd6b6b5b1892fe9838a3f0c9e34 Mon Sep 17 00:00:00 2001 From: Brandon Bodnar <33429657+brandonbodnar-wk@users.noreply.github.com> Date: Fri, 1 Feb 2019 13:16:54 -0600 Subject: [PATCH 11/17] Handle returned error --- go/cmd/gh-ost/main.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index db044f3..9d7d60f 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -245,7 +245,9 @@ func main() { migrationContext.SetThrottleHTTP(*throttleHTTP) migrationContext.SetDefaultNumRetries(*defaultRetries) migrationContext.ApplyCredentials() - migrationContext.SetupTLS() + if err := migrationContext.SetupTLS(); err != nil { + log.Fatale(err) + } if err := migrationContext.SetCutOverLockTimeoutSeconds(*cutOverLockTimeoutSeconds); err != nil { log.Errore(err) } From 5319157789e317d296e298c0eb86240d3ef3cd56 Mon Sep 17 00:00:00 2001 From: Brandon Bodnar <33429657+brandonbodnar-wk@users.noreply.github.com> Date: Fri, 1 Feb 2019 13:20:17 -0600 Subject: [PATCH 12/17] Expand usage statement to indicate setting applies to multiple hosts --- go/cmd/gh-ost/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 9d7d60f..d8add4f 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -55,8 +55,8 @@ func main() { flag.StringVar(&migrationContext.ConfigFile, "conf", "", "Config file") askPass := flag.Bool("ask-pass", false, "prompt for MySQL password") - flag.BoolVar(&migrationContext.UseTLS, "ssl", false, "Enable SSL encrypted connections to MySQL") - flag.StringVar(&migrationContext.TLSCACertificate, "ssl-ca", "", "CA certificate in PEM format for TLS connections. Requires --ssl") + flag.BoolVar(&migrationContext.UseTLS, "ssl", false, "Enable SSL encrypted connections to MySQL hosts") + flag.StringVar(&migrationContext.TLSCACertificate, "ssl-ca", "", "CA certificate in PEM format for TLS connections to MySQL hosts. Requires --ssl") flag.StringVar(&migrationContext.DatabaseName, "database", "", "database name (mandatory)") flag.StringVar(&migrationContext.OriginalTableName, "table", "", "table name (mandatory)") From 79df0d1c5de9eec208da73f2657b6ff666af6239 Mon Sep 17 00:00:00 2001 From: Matt Belisle Date: Mon, 4 Feb 2019 14:46:08 -0600 Subject: [PATCH 13/17] Adding --ssl-insecure flag --- go/base/context.go | 19 ++++++++++--------- go/cmd/gh-ost/main.go | 4 ++++ go/mysql/connection.go | 28 ++++++++++++++++++---------- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 90581e2..9f9de9b 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -94,15 +94,16 @@ type MigrationContext struct { AliyunRDS bool GoogleCloudPlatform bool - config ContextConfig - configMutex *sync.Mutex - ConfigFile string - CliUser string - CliPassword string - UseTLS bool - TLSCACertificate string - CliMasterUser string - CliMasterPassword string + config ContextConfig + configMutex *sync.Mutex + ConfigFile string + CliUser string + CliPassword string + UseTLS bool + TLSInsecureSkipVerify bool + TLSCACertificate string + CliMasterUser string + CliMasterPassword string HeartbeatIntervalMilliseconds int64 defaultNumRetries int64 diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index d8add4f..96bd2dc 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -57,6 +57,7 @@ func main() { flag.BoolVar(&migrationContext.UseTLS, "ssl", false, "Enable SSL encrypted connections to MySQL hosts") flag.StringVar(&migrationContext.TLSCACertificate, "ssl-ca", "", "CA certificate in PEM format for TLS connections to MySQL hosts. Requires --ssl") + flag.StringVar(&migrationContext.TLSInsecureSkipVerify, "ssl-insecure", false, "Do not verify that the TLS connection is secure. Requires --ssl") flag.StringVar(&migrationContext.DatabaseName, "database", "", "database name (mandatory)") flag.StringVar(&migrationContext.OriginalTableName, "table", "", "table name (mandatory)") @@ -201,6 +202,9 @@ func main() { if migrationContext.TLSCACertificate != "" && !migrationContext.UseTLS { log.Fatalf("--ssl-ca requires --ssl") } + if migrationContext.TLSInsecureSkipVerify && !migrationContext.UseTLS { + log.Fatalf("--ssl-insecure requires --ssl") + } if *replicationLagQuery != "" { log.Warningf("--replication-lag-query is deprecated") } diff --git a/go/mysql/connection.go b/go/mysql/connection.go index 742ca7e..f0f73ce 100644 --- a/go/mysql/connection.go +++ b/go/mysql/connection.go @@ -58,22 +58,30 @@ func (this *ConnectionConfig) Equals(other *ConnectionConfig) bool { } func (this *ConnectionConfig) UseTLS(caCertificatePath string) error { - skipVerify := caCertificatePath == "" var rootCertPool *x509.CertPool - if !skipVerify { - rootCertPool = x509.NewCertPool() - pem, err := ioutil.ReadFile(caCertificatePath) - if err != nil { - return err - } - if ok := rootCertPool.AppendCertsFromPEM(pem); !ok { - return errors.New("could not add ca certificate to cert pool") + var err error + + if !this.TLSInsecureSkipVerify { + if caCertificatePath == "" { + rootCertPool, err = x509.SystemCertPool() + if err != nil { + return err + } + } else { + rootCertPool = x509.NewCertPool() + pem, err := ioutil.ReadFile(caCertificatePath) + if err != nil { + return err + } + if ok := rootCertPool.AppendCertsFromPEM(pem); !ok { + return errors.New("could not add ca certificate to cert pool") + } } } this.tlsConfig = &tls.Config{ RootCAs: rootCertPool, - InsecureSkipVerify: skipVerify, + InsecureSkipVerify: this.TLSInsecureSkipVerify, } return mysql.RegisterTLSConfig(this.Key.StringCode(), this.tlsConfig) From 5b0dfb009c5e0dfb815d125da2faec1f132a4c75 Mon Sep 17 00:00:00 2001 From: Brandon Bodnar <33429657+brandonbodnar-wk@users.noreply.github.com> Date: Mon, 4 Feb 2019 16:21:25 -0600 Subject: [PATCH 14/17] Wireup allowing insecure ssl --- go/base/context.go | 22 +++++++++++----------- go/cmd/gh-ost/main.go | 6 +++--- go/mysql/connection.go | 6 +++--- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 9f9de9b..e9991d8 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -94,16 +94,16 @@ type MigrationContext struct { AliyunRDS bool GoogleCloudPlatform bool - config ContextConfig - configMutex *sync.Mutex - ConfigFile string - CliUser string - CliPassword string - UseTLS bool - TLSInsecureSkipVerify bool - TLSCACertificate string - CliMasterUser string - CliMasterPassword string + config ContextConfig + configMutex *sync.Mutex + ConfigFile string + CliUser string + CliPassword string + UseTLS bool + TLSAllowInsecure bool + TLSCACertificate string + CliMasterUser string + CliMasterPassword string HeartbeatIntervalMilliseconds int64 defaultNumRetries int64 @@ -700,7 +700,7 @@ func (this *MigrationContext) ApplyCredentials() { func (this *MigrationContext) SetupTLS() error { if this.UseTLS { - return this.InspectorConnectionConfig.UseTLS(this.TLSCACertificate) + return this.InspectorConnectionConfig.UseTLS(this.TLSCACertificate, this.TLSAllowInsecure) } return nil } diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 96bd2dc..b02d6b7 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -57,7 +57,7 @@ func main() { flag.BoolVar(&migrationContext.UseTLS, "ssl", false, "Enable SSL encrypted connections to MySQL hosts") flag.StringVar(&migrationContext.TLSCACertificate, "ssl-ca", "", "CA certificate in PEM format for TLS connections to MySQL hosts. Requires --ssl") - flag.StringVar(&migrationContext.TLSInsecureSkipVerify, "ssl-insecure", false, "Do not verify that the TLS connection is secure. Requires --ssl") + flag.BoolVar(&migrationContext.TLSAllowInsecure, "ssl-allow-insecure", false, "Skips verification of MySQL hosts' certificate chain and host name. Requires --ssl") flag.StringVar(&migrationContext.DatabaseName, "database", "", "database name (mandatory)") flag.StringVar(&migrationContext.OriginalTableName, "table", "", "table name (mandatory)") @@ -202,8 +202,8 @@ func main() { if migrationContext.TLSCACertificate != "" && !migrationContext.UseTLS { log.Fatalf("--ssl-ca requires --ssl") } - if migrationContext.TLSInsecureSkipVerify && !migrationContext.UseTLS { - log.Fatalf("--ssl-insecure requires --ssl") + if migrationContext.TLSAllowInsecure && !migrationContext.UseTLS { + log.Fatalf("--ssl-allow-insecure requires --ssl") } if *replicationLagQuery != "" { log.Warningf("--replication-lag-query is deprecated") diff --git a/go/mysql/connection.go b/go/mysql/connection.go index f0f73ce..d6c7215 100644 --- a/go/mysql/connection.go +++ b/go/mysql/connection.go @@ -57,11 +57,11 @@ func (this *ConnectionConfig) Equals(other *ConnectionConfig) bool { return this.Key.Equals(&other.Key) || this.ImpliedKey.Equals(other.ImpliedKey) } -func (this *ConnectionConfig) UseTLS(caCertificatePath string) error { +func (this *ConnectionConfig) UseTLS(caCertificatePath string, allowInsecure bool) error { var rootCertPool *x509.CertPool var err error - if !this.TLSInsecureSkipVerify { + if !allowInsecure { if caCertificatePath == "" { rootCertPool, err = x509.SystemCertPool() if err != nil { @@ -81,7 +81,7 @@ func (this *ConnectionConfig) UseTLS(caCertificatePath string) error { this.tlsConfig = &tls.Config{ RootCAs: rootCertPool, - InsecureSkipVerify: this.TLSInsecureSkipVerify, + InsecureSkipVerify: allowInsecure, } return mysql.RegisterTLSConfig(this.Key.StringCode(), this.tlsConfig) From 1543098891296e496e16f63b671d6d0ea80342e8 Mon Sep 17 00:00:00 2001 From: Brandon Bodnar <33429657+brandonbodnar-wk@users.noreply.github.com> Date: Tue, 5 Feb 2019 08:02:47 -0600 Subject: [PATCH 15/17] Document ssl-related command line flags. --- doc/command-line-flags.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/doc/command-line-flags.md b/doc/command-line-flags.md index 804ba58..c20a5a8 100644 --- a/doc/command-line-flags.md +++ b/doc/command-line-flags.md @@ -177,6 +177,18 @@ By default `gh-ost` verifies no foreign keys exist on the migrated table. On ser See [`approve-renamed-columns`](#approve-renamed-columns) +### ssl + +By default `gh-ost` does not use ssl/tls connections to the database servers when performing migrations. This flag instructs `gh-ost` to use encrypted connections. If enabled, `gh-ost` will use the system's ca certificate pool for server certificate verification. If a different certificate is needed for server verification, see `--ssl-ca`. If you wish to skip server verification, but still use encrypted connections, use with `--ssl-allow-insecure`. + +### ssl-allow-insecure + +Allows `gh-ost` to connect to the MySQL servers using encrypted connections, but without verifying the validity of the certificate provided by the server during the connection. Requires `--ssl`. + +### ssl-ca + +`--ssl-ca=/path/to/ca-cert.pem`: ca certificate file (in PEM format) to use for server certificate verification. If specified, the default system ca cert pool will not be used for verification, only the ca cert provided here. Requires `--ssl`. + ### test-on-replica Issue the migration on a replica; do not modify data on master. Useful for validating, testing and benchmarking. See [`testing-on-replica`](testing-on-replica.md) From b82fc452212e6757b1ab97fb1762d142424c1fd4 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Tue, 12 Feb 2019 08:01:47 +0200 Subject: [PATCH 16/17] Adding maintainer email in DEB package --- build.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.sh b/build.sh index 264620b..df9ac7d 100755 --- a/build.sh +++ b/build.sh @@ -42,8 +42,8 @@ function build { builddir=$(setuptree) cp $buildpath/$target $builddir/gh-ost/usr/bin cd $buildpath - fpm -v "${RELEASE_VERSION}" --epoch 1 -f -s dir -n gh-ost -m shlomi-noach --description "GitHub's Online Schema Migrations for MySQL " --url "https://github.com/github/gh-ost" --vendor "GitHub" --license "Apache 2.0" -C $builddir/gh-ost --prefix=/ -t rpm . - fpm -v "${RELEASE_VERSION}" --epoch 1 -f -s dir -n gh-ost -m shlomi-noach --description "GitHub's Online Schema Migrations for MySQL " --url "https://github.com/github/gh-ost" --vendor "GitHub" --license "Apache 2.0" -C $builddir/gh-ost --prefix=/ -t deb --deb-no-default-config-files . + fpm -v "${RELEASE_VERSION}" --epoch 1 -f -s dir -n gh-ost -m 'shlomi-noach ' --description "GitHub's Online Schema Migrations for MySQL " --url "https://github.com/github/gh-ost" --vendor "GitHub" --license "Apache 2.0" -C $builddir/gh-ost --prefix=/ -t rpm . + fpm -v "${RELEASE_VERSION}" --epoch 1 -f -s dir -n gh-ost -m 'shlomi-noach ' --description "GitHub's Online Schema Migrations for MySQL " --url "https://github.com/github/gh-ost" --vendor "GitHub" --license "Apache 2.0" -C $builddir/gh-ost --prefix=/ -t deb --deb-no-default-config-files . fi } From a8fae9818daa42681c7806f843067e2d5d360273 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Thu, 14 Feb 2019 12:07:25 +0200 Subject: [PATCH 17/17] v1.0.48 --- RELEASE_VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE_VERSION b/RELEASE_VERSION index bde91a2..56d0dad 100644 --- a/RELEASE_VERSION +++ b/RELEASE_VERSION @@ -1 +1 @@ -1.0.47 +1.0.48