Merge branch 'master' into mysql8-rename-table

doc/command-line-flags.md

@@ -111,6 +111,14 @@ While the ongoing estimated number of rows is still heuristic, it's almost exact
 
 Without this parameter, migration is a _noop_: testing table creation and validity of migration, but not touching data.
 
+### force-named-cut-over
+
+If given, a `cut-over` command must name the migrated table, or else ignored.
+
+### force-named-panic
+
+If given, a `panic` command must name the migrated table, or else ignored.
+
 ### force-table-names
 
 Table name prefix to be used on the temporary tables.
@@ -173,6 +181,10 @@ See also: [`concurrent-migrations`](cheatsheet.md#concurrent-migrations) on the
 
 By default `gh-ost` verifies no foreign keys exist on the migrated table. On servers with large number of tables this check can take a long time. If you're absolutely certain no foreign keys exist (table does not reference other table nor is referenced by other tables) and wish to save the check time, provide with `--skip-foreign-key-checks`.
 
+### skip-strict-mode
+
+By default `gh-ost` enforces STRICT_ALL_TABLES sql_mode as a safety measure. In some cases this changes the behaviour of other modes (namely ERROR_FOR_DIVISION_BY_ZERO, NO_ZERO_DATE, and NO_ZERO_IN_DATE) which may lead to errors during migration. Use `--skip-strict-mode` to explicitly tell `gh-ost` not to enforce this. **Danger** This may have some unexpected disastrous side effects.
+
 ### skip-renamed-columns
 
 See [`approve-renamed-columns`](#approve-renamed-columns)
@@ -189,6 +201,14 @@ Allows `gh-ost` to connect to the MySQL servers using encrypted connections, but
 
 `--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`.
 
+### ssl-cert
+
+`--ssl-cert=/path/to/ssl-cert.crt`: SSL public key certificate file (in PEM format).
+
+### ssl-key
+
+`--ssl-key=/path/to/ssl-key.key`: SSL private key file (in PEM format).
+
 ### 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)

doc/requirements-and-limitations.md

@@ -22,7 +22,7 @@ The `SUPER` privilege is required for `STOP SLAVE`, `START SLAVE` operations. Th
 
 ### Limitations
 
-- Foreign keys not supported. They may be supported in the future, to some extent.
+- Foreign key constraints are not supported. They may be supported in the future, to some extent.
 
 - Triggers are not supported. They may be supported in the future.
 
@@ -38,7 +38,7 @@ The `SUPER` privilege is required for `STOP SLAVE`, `START SLAVE` operations. Th
 - It is not allowed to migrate a table where another table exists with same name and different upper/lower case.
   - For example, you may not migrate `MyTable` if another table called `MYtable` exists in the same schema.
 
-- Amazon RDS works, but has it's own [limitations](rds.md).
+- Amazon RDS works, but has its own [limitations](rds.md).
 - Google Cloud SQL works, `--gcp` flag required.
 - Aliyun RDS works, `--aliyun-rds` flag required.
 

go/base/context.go

@@ -86,6 +86,7 @@ type MigrationContext struct {
 	SwitchToRowBinlogFormat  bool
 	AssumeRBR                bool
 	SkipForeignKeyChecks     bool
+	SkipStrictMode           bool
 	NullableUniqueKeyAllowed bool
 	ApproveRenamedColumns    bool
 	SkipRenamedColumns       bool
@@ -102,6 +103,8 @@ type MigrationContext struct {
 	UseTLS            bool
 	TLSAllowInsecure  bool
 	TLSCACertificate  string
+	TLSCertificate    string
+	TLSKey            string
 	CliMasterUser     string
 	CliMasterPassword string
 
@@ -126,6 +129,7 @@ type MigrationContext struct {
 	CutOverExponentialBackoff           bool
 	ExponentialBackoffMaxInterval       int64
 	ForceNamedCutOverCommand            bool
+	ForceNamedPanicCommand              bool
 	PanicFlagFile                       string
 	HooksPath                           string
 	HooksHintMessage                    string
@@ -702,7 +706,7 @@ func (this *MigrationContext) ApplyCredentials() {
 
 func (this *MigrationContext) SetupTLS() error {
 	if this.UseTLS {
-		return this.InspectorConnectionConfig.UseTLS(this.TLSCACertificate, this.TLSAllowInsecure)
+		return this.InspectorConnectionConfig.UseTLS(this.TLSCACertificate, this.TLSCertificate, this.TLSKey, this.TLSAllowInsecure)
 	}
 	return nil
 }

go/cmd/gh-ost/main.go

@@ -57,6 +57,8 @@ 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.TLSCertificate, "ssl-cert", "", "Certificate in PEM format for TLS connections to MySQL hosts. Requires --ssl")
+	flag.StringVar(&migrationContext.TLSKey, "ssl-key", "", "Key in PEM format for TLS connections to MySQL hosts. 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)")
@@ -72,6 +74,7 @@ func main() {
 	flag.BoolVar(&migrationContext.IsTungsten, "tungsten", false, "explicitly let gh-ost know that you are running on a tungsten-replication based topology (you are likely to also provide --assume-master-host)")
 	flag.BoolVar(&migrationContext.DiscardForeignKeys, "discard-foreign-keys", false, "DANGER! This flag will migrate a table that has foreign keys and will NOT create foreign keys on the ghost table, thus your altered table will have NO foreign keys. This is useful for intentional dropping of foreign keys")
 	flag.BoolVar(&migrationContext.SkipForeignKeyChecks, "skip-foreign-key-checks", false, "set to 'true' when you know for certain there are no foreign keys on your table, and wish to skip the time it takes for gh-ost to verify that")
+	flag.BoolVar(&migrationContext.SkipStrictMode, "skip-strict-mode", false, "explicitly tell gh-ost binlog applier not to enforce strict sql mode")
 	flag.BoolVar(&migrationContext.AliyunRDS, "aliyun-rds", false, "set to 'true' when you execute on Aliyun RDS.")
 	flag.BoolVar(&migrationContext.GoogleCloudPlatform, "gcp", false, "set to 'true' when you execute on a 1st generation Google Cloud Platform (GCP).")
 
@@ -86,6 +89,7 @@ func main() {
 	flag.BoolVar(&migrationContext.TimestampOldTable, "timestamp-old-table", false, "Use a timestamp in old table name. This makes old table names unique and non conflicting cross migrations")
 	cutOver := flag.String("cut-over", "atomic", "choose cut-over type (default|atomic, two-step)")
 	flag.BoolVar(&migrationContext.ForceNamedCutOverCommand, "force-named-cut-over", false, "When true, the 'unpostpone|cut-over' interactive command must name the migrated table")
+	flag.BoolVar(&migrationContext.ForceNamedPanicCommand, "force-named-panic", false, "When true, the 'panic' interactive command must name the migrated table")
 
 	flag.BoolVar(&migrationContext.SwitchToRowBinlogFormat, "switch-to-rbr", false, "let this tool automatically switch binary log format to 'ROW' on the replica, if needed. The format will NOT be switched back. I'm too scared to do that, and wish to protect you if you happen to execute another migration while this one is running")
 	flag.BoolVar(&migrationContext.AssumeRBR, "assume-rbr", false, "set to 'true' when you know for certain your server uses 'ROW' binlog_format. gh-ost is unable to tell, event after reading binlog_format, whether the replication process does indeed use 'ROW', and restarts replication to be certain RBR setting is applied. Such operation requires SUPER privileges which you might not have. Setting this flag avoids restarting replication and you can proceed to use gh-ost without SUPER privileges")
@@ -204,6 +208,12 @@ func main() {
 	if migrationContext.TLSCACertificate != "" && !migrationContext.UseTLS {
 		log.Fatalf("--ssl-ca requires --ssl")
 	}
+	if migrationContext.TLSCertificate != "" && !migrationContext.UseTLS {
+		log.Fatalf("--ssl-cert requires --ssl")
+	}
+	if migrationContext.TLSKey != "" && !migrationContext.UseTLS {
+		log.Fatalf("--ssl-key requires --ssl")
+	}
 	if migrationContext.TLSAllowInsecure && !migrationContext.UseTLS {
 		log.Fatalf("--ssl-allow-insecure requires --ssl")
 	}

go/logic/applier.go

@@ -482,10 +482,10 @@ func (this *Applier) ApplyIterationInsertQuery() (chunkSize int64, rowsAffected
 			return nil, err
 		}
 		defer tx.Rollback()
-		sessionQuery := fmt.Sprintf(`SET
+		sessionQuery := fmt.Sprintf(`SET SESSION time_zone = '%s'`, this.migrationContext.ApplierTimeZone)
-			SESSION time_zone = '%s',
+		if !this.migrationContext.SkipStrictMode {
-			sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES')
+			sessionQuery += ", sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES')"
-			`, this.migrationContext.ApplierTimeZone)
+		}
 		if _, err := tx.Exec(sessionQuery); err != nil {
 			return nil, err
 		}
@@ -1000,6 +1000,7 @@ func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) (result
 	return append(results, newDmlBuildResultError(fmt.Errorf("Unknown dml event type: %+v", dmlEvent.DML)))
 }
 
+
 // ApplyDMLEventQueries applies multiple DML queries onto the _ghost_ table
 func (this *Applier) ApplyDMLEventQueries(dmlEvents [](*binlog.BinlogDMLEvent)) error {
 
@@ -1016,10 +1017,10 @@ func (this *Applier) ApplyDMLEventQueries(dmlEvents [](*binlog.BinlogDMLEvent))
 			return err
 		}
 
-		sessionQuery := `SET
+		sessionQuery := "SET SESSION time_zone = '+00:00'"
-			SESSION time_zone = '+00:00',
+		if !this.migrationContext.SkipStrictMode {
-			sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES')
+			sessionQuery += ", sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES')"
-			`
+		}
 		if _, err := tx.Exec(sessionQuery); err != nil {
 			return rollback(err)
 		}

go/logic/server.go

@@ -292,12 +292,22 @@ help                                 # This message
 		}
 	case "throttle", "pause", "suspend":
 		{
+			if arg != "" && arg != this.migrationContext.OriginalTableName {
+				// User explicitly provided table name. This is a courtesy protection mechanism
+				err := fmt.Errorf("User commanded 'throttle' on %s, but migrated table is %s; ignoring request.", arg, this.migrationContext.OriginalTableName)
+				return NoPrintStatusRule, err
+			}
 			atomic.StoreInt64(&this.migrationContext.ThrottleCommandedByUser, 1)
 			fmt.Fprintf(writer, throttleHint)
 			return ForcePrintStatusAndHintRule, nil
 		}
 	case "no-throttle", "unthrottle", "resume", "continue":
 		{
+			if arg != "" && arg != this.migrationContext.OriginalTableName {
+				// User explicitly provided table name. This is a courtesy protection mechanism
+				err := fmt.Errorf("User commanded 'no-throttle' on %s, but migrated table is %s; ignoring request.", arg, this.migrationContext.OriginalTableName)
+				return NoPrintStatusRule, err
+			}
 			atomic.StoreInt64(&this.migrationContext.ThrottleCommandedByUser, 0)
 			return ForcePrintStatusAndHintRule, nil
 		}
@@ -322,6 +332,15 @@ help                                 # This message
 		}
 	case "panic":
 		{
+			if arg == "" && this.migrationContext.ForceNamedPanicCommand {
+				err := fmt.Errorf("User commanded 'panic' without specifying table name, but --force-named-panic is set")
+				return NoPrintStatusRule, err
+			}
+			if arg != "" && arg != this.migrationContext.OriginalTableName {
+				// User explicitly provided table name. This is a courtesy protection mechanism
+				err := fmt.Errorf("User commanded 'panic' on %s, but migrated table is %s; ignoring request.", arg, this.migrationContext.OriginalTableName)
+				return NoPrintStatusRule, err
+			}
 			err := fmt.Errorf("User commanded 'panic'. I will now panic, without cleanup. PANIC!")
 			this.migrationContext.PanicAbort <- err
 			return NoPrintStatusRule, err

go/mysql/connection.go

@@ -16,6 +16,10 @@ import (
 	"github.com/go-sql-driver/mysql"
 )
 
+const (
+	TLS_CONFIG_KEY = "ghost"
+)
+
 // ConnectionConfig is the minimal configuration required to connect to a MySQL server
 type ConnectionConfig struct {
 	Key        InstanceKey
@@ -57,11 +61,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, allowInsecure bool) error {
+func (this *ConnectionConfig) UseTLS(caCertificatePath, clientCertificate, clientKey string, allowInsecure bool) error {
 	var rootCertPool *x509.CertPool
+	var certs []tls.Certificate
 	var err error
 
-	if !allowInsecure {
 	if caCertificatePath == "" {
 		rootCertPool, err = x509.SystemCertPool()
 		if err != nil {
@@ -77,14 +81,21 @@ func (this *ConnectionConfig) UseTLS(caCertificatePath string, allowInsecure boo
 			return errors.New("could not add ca certificate to cert pool")
 		}
 	}
+	if clientCertificate != "" || clientKey != "" {
+		cert, err := tls.LoadX509KeyPair(clientCertificate, clientKey)
+		if err != nil {
+			return err
+		}
+		certs = []tls.Certificate{cert}
 	}
 
 	this.tlsConfig = &tls.Config{
+		Certificates:       certs,
 		RootCAs:            rootCertPool,
 		InsecureSkipVerify: allowInsecure,
 	}
 
-	return mysql.RegisterTLSConfig(this.Key.StringCode(), this.tlsConfig)
+	return mysql.RegisterTLSConfig(TLS_CONFIG_KEY, this.tlsConfig)
 }
 
 func (this *ConnectionConfig) TLSConfig() *tls.Config {
@@ -103,7 +114,7 @@ func (this *ConnectionConfig) GetDBUri(databaseName string) string {
 	// simplify construction of the DSN below.
 	tlsOption := "false"
 	if this.tlsConfig != nil {
-		tlsOption = this.Key.StringCode()
+		tlsOption = TLS_CONFIG_KEY
 	}
 	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)
 }

go/mysql/connection_test.go

@@ -80,5 +80,5 @@ func TestGetDBUriWithTLSSetup(t *testing.T) {
 	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")
+	test.S(t).ExpectEquals(uri, "gromit:penguin@tcp(myhost:3306)/test?interpolateParams=true&autocommit=true&charset=utf8mb4,utf8,latin1&tls=ghost")
 }