From 5f587c3262259c4d6ca5a17e955fc96f8f03e0d5 Mon Sep 17 00:00:00 2001
From: Shlomi Noach <shlomi-noach@github.com>
Date: Mon, 14 Jan 2019 13:27:44 +0200
Subject: [PATCH 1/7] Adding --force-named-panic option

---
 go/base/context.go    | 1 +
 go/cmd/gh-ost/main.go | 1 +
 go/logic/server.go    | 9 +++++++++
 3 files changed, 11 insertions(+)

diff --git a/go/base/context.go b/go/base/context.go
index 0043d07..318fdf9 100644
--- a/go/base/context.go
+++ b/go/base/context.go
@@ -123,6 +123,7 @@ type MigrationContext struct {
 	CutOverExponentialBackoff           bool
 	ExponentialBackoffMaxInterval       int64
 	ForceNamedCutOverCommand            bool
+	ForceNamedPanicCommand              bool
 	PanicFlagFile                       string
 	HooksPath                           string
 	HooksHintMessage                    string
diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go
index 3024702..a745c21 100644
--- a/go/cmd/gh-ost/main.go
+++ b/go/cmd/gh-ost/main.go
@@ -81,6 +81,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")
diff --git a/go/logic/server.go b/go/logic/server.go
index 919f178..831fdfa 100644
--- a/go/logic/server.go
+++ b/go/logic/server.go
@@ -322,6 +322,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

From 6c5805d844935776a0ebdb4260b91f598f878c81 Mon Sep 17 00:00:00 2001
From: Shlomi Noach <shlomi-noach@github.com>
Date: Mon, 14 Jan 2019 13:32:43 +0200
Subject: [PATCH 2/7] documenting new flag

---
 doc/command-line-flags.md | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/doc/command-line-flags.md b/doc/command-line-flags.md
index 804ba58..3d96ce4 100644
--- a/doc/command-line-flags.md
+++ b/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.

From 7d5749b84a30b1e41ec99bbb55795db1f0352425 Mon Sep 17 00:00:00 2001
From: babinomec <babinomec@users.noreply.github.com>
Date: Thu, 14 Feb 2019 15:58:49 +0100
Subject: [PATCH 3/7] add --skip-strict-mode option

---
 doc/command-line-flags.md |  4 ++++
 go/base/context.go        |  1 +
 go/cmd/gh-ost/main.go     |  1 +
 go/logic/applier.go       | 24 ++++++++++++------------
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/doc/command-line-flags.md b/doc/command-line-flags.md
index c20a5a8..02e7c64 100644
--- a/doc/command-line-flags.md
+++ b/doc/command-line-flags.md
@@ -173,6 +173,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)
diff --git a/go/base/context.go b/go/base/context.go
index b2a6c15..09bc9c0 100644
--- a/go/base/context.go
+++ b/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
diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go
index a467f07..55e65d8 100644
--- a/go/cmd/gh-ost/main.go
+++ b/go/cmd/gh-ost/main.go
@@ -72,6 +72,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).")
 
diff --git a/go/logic/applier.go b/go/logic/applier.go
index b3de0e1..1544359 100644
--- a/go/logic/applier.go
+++ b/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
-			SESSION time_zone = '%s',
-			sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES')
-			`, this.migrationContext.ApplierTimeZone)
+		sessionQuery := fmt.Sprintf(`SET SESSION time_zone = '%s'`, this.migrationContext.ApplierTimeZone)
+		if !this.migrationContext.SkipStrictMode {
+			sessionQuery += ", sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES')"
+		}
 		if _, err := tx.Exec(sessionQuery); err != nil {
 			return nil, err
 		}
@@ -1005,10 +1005,10 @@ func (this *Applier) ApplyDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) error {
 				tx.Rollback()
 				return err
 			}
-			sessionQuery := `SET
-			SESSION time_zone = '+00:00',
-			sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES')
-			`
+			sessionQuery := fmt.Sprintf("SET SESSION time_zone = '+00:00'")
+			if !this.migrationContext.SkipStrictMode {
+				sessionQuery += ", sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES')"
+			}
 			if _, err := tx.Exec(sessionQuery); err != nil {
 				return rollback(err)
 			}
@@ -1050,10 +1050,10 @@ func (this *Applier) ApplyDMLEventQueries(dmlEvents [](*binlog.BinlogDMLEvent))
 			return err
 		}
 
-		sessionQuery := `SET
-			SESSION time_zone = '+00:00',
-			sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES')
-			`
+		sessionQuery := "SET SESSION time_zone = '+00:00'"
+		if !this.migrationContext.SkipStrictMode {
+			sessionQuery += ", sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES')"
+		}
 		if _, err := tx.Exec(sessionQuery); err != nil {
 			return rollback(err)
 		}

From b4da7cfbfb0a08a94e13df82403789713da4ed71 Mon Sep 17 00:00:00 2001
From: Rafe Kettler <rafe@squareup.com>
Date: Fri, 22 Feb 2019 10:33:19 -0800
Subject: [PATCH 4/7] Improve SSL support

Related issue: https://github.com/github/gh-ost/issues/521

 - Add --ssl-cert and --ssl-key options to specify SSL public/private
   key files
 - Allow combining --ssl-allow-insecure with other --ssl* flags.
   `mysql.RegisterTLSConfig` allows combining the corresponding
   parameters in the `tls.Config` it receives, so gh-ost should
   allow this. I found being able to pass --ssl-allow-insecure along
   with --ssl-ca, --ssl-cert, and --ssl-key useful in testing.
 - Use the same TLS config everywhere. Since the CLI only supports
   a single set of --ssl* configuration parameters, this should be
   fine -- `mysql.RegisterTLSConfig` documentation indicates the
   TLS config given will not be modified, so it can safely be used
   in many goroutines provided we also do not modify it. The previous
   implementation did not work when the TLS config was duplicated,
   which happens when gh-ost walks up the replication chain trying
   to find the master. This is because, when the config is duplicated,
   we must call `RegisterTLSConfig` again with the new config. This
   config is exactly the same, so it's easiest to side-step the issue
   by registering the TLS config once and using it everywhere.
---
 doc/command-line-flags.md   |  8 +++++++
 go/base/context.go          |  4 +++-
 go/cmd/gh-ost/main.go       |  8 +++++++
 go/mysql/connection.go      | 47 +++++++++++++++++++++++--------------
 go/mysql/connection_test.go |  2 +-
 5 files changed, 49 insertions(+), 20 deletions(-)

diff --git a/doc/command-line-flags.md b/doc/command-line-flags.md
index c20a5a8..2d9bfd1 100644
--- a/doc/command-line-flags.md
+++ b/doc/command-line-flags.md
@@ -189,6 +189,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)
diff --git a/go/base/context.go b/go/base/context.go
index b2a6c15..8a8f721 100644
--- a/go/base/context.go
+++ b/go/base/context.go
@@ -102,6 +102,8 @@ type MigrationContext struct {
 	UseTLS            bool
 	TLSAllowInsecure  bool
 	TLSCACertificate  string
+	TLSCertificate    string
+	TLSKey            string
 	CliMasterUser     string
 	CliMasterPassword string
 
@@ -702,7 +704,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
 }
diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go
index a467f07..5c30ffd 100644
--- a/go/cmd/gh-ost/main.go
+++ b/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)")
@@ -204,6 +206,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")
 	}
diff --git a/go/mysql/connection.go b/go/mysql/connection.go
index d6c7215..654998c 100644
--- a/go/mysql/connection.go
+++ b/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,34 +61,41 @@ 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 {
-				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")
-			}
+	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")
+		}
+	}
+	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)
 }
diff --git a/go/mysql/connection_test.go b/go/mysql/connection_test.go
index 3a6cf29..6ce85a7 100644
--- a/go/mysql/connection_test.go
+++ b/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")
 }

From 7ead4c4a56dda163fbf50fce6bb22ac6d96b690f Mon Sep 17 00:00:00 2001
From: Shlomi Noach <shlomi-noach@github.com>
Date: Mon, 25 Feb 2019 14:02:57 +0200
Subject: [PATCH 5/7] named throttle, no-throttle

---
 go/logic/server.go | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/go/logic/server.go b/go/logic/server.go
index 831fdfa..774c4ab 100644
--- a/go/logic/server.go
+++ b/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
 		}

From 773cddc88acaa47b52f99157a3faf746b3708f78 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Fri, 29 Mar 2019 05:05:40 -0700
Subject: [PATCH 6/7] Clarify it's foreign key /constraints/ that don't work

---
 doc/requirements-and-limitations.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/requirements-and-limitations.md b/doc/requirements-and-limitations.md
index 628451d..8376e18 100644
--- a/doc/requirements-and-limitations.md
+++ b/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.
 

From 256025853c3cb386453ed223f865c9affc249b93 Mon Sep 17 00:00:00 2001
From: Jon Chambers <jon.chambers@gmail.com>
Date: Fri, 12 Apr 2019 12:07:00 -0400
Subject: [PATCH 7/7] Fix a trivial it's/its typo.

---
 doc/requirements-and-limitations.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/requirements-and-limitations.md b/doc/requirements-and-limitations.md
index 8376e18..096e2c9 100644
--- a/doc/requirements-and-limitations.md
+++ b/doc/requirements-and-limitations.md
@@ -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.