From 40563a1c702d9e613ed282cad84046943d008c99 Mon Sep 17 00:00:00 2001 From: Andy Gilbert Date: Mon, 19 Nov 2018 10:32:16 -0800 Subject: [PATCH] Add support for migrating views with the migration plugin - Add a warning that events, procedures, and triggers are NOT migrated [#157373092] Signed-off-by: Andrew Garner --- Gopkg.lock | 1 + mysql-tools/plugin/plugin.go | 1 + mysql-tools/plugin/plugin_test.go | 12 +++ specs/acceptance/migrate_test.go | 95 ++++++++++++++++++- tasks/migrate/backup_restore.go | 35 ++++++- tasks/migrate/backup_restore_test.go | 84 ++++++++++++---- tasks/migrate/discovery.go | 8 +- tasks/migrate/main.go | 3 +- .../poll_cf/commandstarter/command_starter.go | 2 +- 9 files changed, 205 insertions(+), 36 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 0608ca05..a11b7888 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -293,6 +293,7 @@ "github.com/onsi/ginkgo", "github.com/onsi/ginkgo/config", "github.com/onsi/gomega", + "github.com/onsi/gomega/format", "github.com/onsi/gomega/gbytes", "github.com/onsi/gomega/gexec", "github.com/pborman/uuid", diff --git a/mysql-tools/plugin/plugin.go b/mysql-tools/plugin/plugin.go index 45c4e9d8..6485f2a5 100644 --- a/mysql-tools/plugin/plugin.go +++ b/mysql-tools/plugin/plugin.go @@ -137,6 +137,7 @@ func Migrate(migrator Migrator, args []string) error { return err } + log.Printf("Warning: The mysql-tools migrate command will not migrate any triggers, routines or events.") productName := os.Getenv("RECIPIENT_PRODUCT_NAME") if productName == "" { productName = "p.mysql" diff --git a/mysql-tools/plugin/plugin_test.go b/mysql-tools/plugin/plugin_test.go index fb849e09..c0c14a21 100644 --- a/mysql-tools/plugin/plugin_test.go +++ b/mysql-tools/plugin/plugin_test.go @@ -13,7 +13,10 @@ package plugin_test import ( + "bytes" "errors" + "io" + "log" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -24,12 +27,17 @@ import ( var _ = Describe("Plugin Commands", func() { var ( fakeMigrator *pluginfakes.FakeMigrator + logOutput *bytes.Buffer ) const usage = `Usage: cf mysql-tools migrate [-h] [--no-cleanup] ` BeforeEach(func() { fakeMigrator = new(pluginfakes.FakeMigrator) + logOutput = &bytes.Buffer{} + + w := io.MultiWriter(GinkgoWriter, logOutput) + log.SetOutput(w) }) Context("Migrate", func() { @@ -39,6 +47,10 @@ var _ = Describe("Plugin Commands", func() { } Expect(plugin.Migrate(fakeMigrator, args)).To(Succeed()) + By("log a message that we don't migrate triggers, routines and events", func() { + Expect(logOutput.String()).To(ContainSubstring(`Warning: The mysql-tools migrate command will not migrate any triggers, routines or events`)) + }) + By("checking that donor exists", func() { Expect(fakeMigrator.CheckServiceExistsCallCount()). To(Equal(1)) diff --git a/specs/acceptance/migrate_test.go b/specs/acceptance/migrate_test.go index 4221db10..7ff1274d 100644 --- a/specs/acceptance/migrate_test.go +++ b/specs/acceptance/migrate_test.go @@ -81,6 +81,10 @@ var _ = Describe("Migrate Integration Tests", func() { }) Context("When a valid donor service instance exists", func() { + BeforeEach(func() { + setupStoredCodeFixtures(sourceInstance) + }) + AfterEach(func() { if springAppName != "" { test_helpers.DeleteApp(springAppName) @@ -166,6 +170,10 @@ var _ = Describe("Migrate Integration Tests", func() { NotTo(BeEmpty(), "Expected recipient service instance to be TLS enabled, but it was not") }) + + By("Verifying the views get migrated with invoker security type and procedures are not migrated", func() { + validateMigratedStoredCode(sourceInstance) + }) }) Context("when the --no-cleanup flag is specified", func() { @@ -246,12 +254,48 @@ var _ = Describe("Migrate Integration Tests", func() { }) }) -func createInvalidMigrationState(sourceInstance string) { +func setupStoredCodeFixtures(instanceName string) { + appName := generator.PrefixedRandomName("MYSQL", "INVALID_MIGRATION") + serviceKey := generator.PrefixedRandomName("MYSQL", "SERVICE_KEY") + + test_helpers.PushApp(appName, "../assets/spring-music") + test_helpers.BindAppToService(appName, instanceName) + defer func() { + test_helpers.DeleteApp(appName) + test_helpers.AssertAppIsDeleted(appName) + }() + + test_helpers.StartApp(appName) + + serviceKeyCreds := test_helpers.GetServiceKey(instanceName, serviceKey) + defer test_helpers.DeleteServiceKey(instanceName, serviceKey) + + closeTunnel := test_helpers.OpenDatabaseTunnelToApp(63308, appName, serviceKeyCreds) + defer closeTunnel() + + dsn := fmt.Sprintf("%s:%s@tcp(127.0.0.1:63308)/%s", + serviceKeyCreds.Username, + serviceKeyCreds.Password, + serviceKeyCreds.Name, + ) + + db, err := sql.Open("mysql", dsn) + defer db.Close() + Expect(err).NotTo(HaveOccurred()) + + _, err = db.Exec("CREATE VIEW migrate_view AS SELECT 1") + Expect(err).NotTo(HaveOccurred()) + + _, err = db.Exec("CREATE PROCEDURE migrate_procedure() BEGIN END") + Expect(err).NotTo(HaveOccurred()) +} + +func validateMigratedStoredCode(instanceName string) { appName := generator.PrefixedRandomName("MYSQL", "INVALID_MIGRATION") - sourceServiceKey := generator.PrefixedRandomName("MYSQL", "SERVICE_KEY") + serviceKey := generator.PrefixedRandomName("MYSQL", "SERVICE_KEY") test_helpers.PushApp(appName, "../assets/spring-music") - test_helpers.BindAppToService(appName, sourceInstance) + test_helpers.BindAppToService(appName, instanceName) defer func() { test_helpers.DeleteApp(appName) test_helpers.AssertAppIsDeleted(appName) @@ -259,8 +303,8 @@ func createInvalidMigrationState(sourceInstance string) { test_helpers.StartApp(appName) - serviceKeyCreds := test_helpers.GetServiceKey(sourceInstance, sourceServiceKey) - defer test_helpers.DeleteServiceKey(sourceInstance, sourceServiceKey) + serviceKeyCreds := test_helpers.GetServiceKey(instanceName, serviceKey) + defer test_helpers.DeleteServiceKey(instanceName, serviceKey) closeTunnel := test_helpers.OpenDatabaseTunnelToApp(63308, appName, serviceKeyCreds) defer closeTunnel() @@ -270,6 +314,47 @@ func createInvalidMigrationState(sourceInstance string) { serviceKeyCreds.Password, serviceKeyCreds.Name, ) + + db, err := sql.Open("mysql", dsn) + defer db.Close() + Expect(err).NotTo(HaveOccurred()) + + var routineCount int + routineCountSQL := `SELECT COUNT(*) FROM information_schema.routines WHERE ROUTINE_SCHEMA = 'service_instance_db'` + Expect(db.QueryRow(routineCountSQL).Scan(&routineCount)).To(Succeed()) + Expect(routineCount).To(BeZero()) + + var viewSecurityType string + checkViewSQL := `SELECT SECURITY_TYPE FROM information_schema.views WHERE table_schema = 'service_instance_db' and table_name = 'migrate_view'` + Expect(db.QueryRow(checkViewSQL).Scan(&viewSecurityType)).To(Succeed()) + Expect(viewSecurityType).To(Equal("INVOKER")) +} + +func createInvalidMigrationState(instanceName string) { + appName := generator.PrefixedRandomName("MYSQL", "INVALID_MIGRATION") + serviceKey := generator.PrefixedRandomName("MYSQL", "SERVICE_KEY") + + test_helpers.PushApp(appName, "../assets/spring-music") + test_helpers.BindAppToService(appName, instanceName) + defer func() { + test_helpers.DeleteApp(appName) + test_helpers.AssertAppIsDeleted(appName) + }() + + test_helpers.StartApp(appName) + + serviceKeyCreds := test_helpers.GetServiceKey(instanceName, serviceKey) + defer test_helpers.DeleteServiceKey(instanceName, serviceKey) + + closeTunnel := test_helpers.OpenDatabaseTunnelToApp(63308, appName, serviceKeyCreds) + defer closeTunnel() + + dsn := fmt.Sprintf("%s:%s@tcp(127.0.0.1:63308)/%s", + serviceKeyCreds.Username, + serviceKeyCreds.Password, + serviceKeyCreds.Name, + ) + db, err := sql.Open("mysql", dsn) defer db.Close() Expect(err).NotTo(HaveOccurred()) diff --git a/tasks/migrate/backup_restore.go b/tasks/migrate/backup_restore.go index 310d739e..846c8d09 100644 --- a/tasks/migrate/backup_restore.go +++ b/tasks/migrate/backup_restore.go @@ -48,9 +48,10 @@ func MySQLDumpCmd(credentials Credentials, schemas ...string) *exec.Cmd { cmd.Args = append(cmd.Args, "--max-allowed-packet=1G", "--single-transaction", - "--routines", - "--events", + "--skip-routines", + "--skip-events", "--set-gtid-purged=off", + "--skip-triggers", ) if len(schemas) > 1 { @@ -70,18 +71,40 @@ func MySQLCmd(credentials Credentials) *exec.Cmd { return cmd } -func CopyData(mysqldump, mysql *exec.Cmd) error { +func ReplaceDefinerCmd() *exec.Cmd { + args := []string{ + "-e", + "s/DEFINER=`.*`@`%` SQL SECURITY DEFINER/SQL SECURITY INVOKER/", + } + + cmd := exec.Command("sed", args...) + cmd.Stderr = os.Stderr + return cmd +} + +func CopyData(mysqldump, replaceDefinerCmd, mysql *exec.Cmd) error { dumpOut, err := mysqldump.StdoutPipe() if err != nil { return errors.Wrap(err, "couldn't pipe the output of mysqldump") } - mysql.Stdin = dumpOut + replaceDefinerCmd.Stdin = dumpOut + + replaceOut, err := replaceDefinerCmd.StdoutPipe() + if err != nil { + return errors.Wrap(err, "couldn't pipe the output of sed") + } + + mysql.Stdin = replaceOut if err := mysqldump.Start(); err != nil { return errors.Wrap(err, "couldn't start mysqldump") } + if err := replaceDefinerCmd.Start(); err != nil { + return errors.Wrap(err, "couldn't start sed") + } + if err := mysql.Start(); err != nil { return errors.Wrap(err, "couldn't start mysql") } @@ -90,6 +113,10 @@ func CopyData(mysqldump, mysql *exec.Cmd) error { return errors.Wrap(err, "mysql command failed") } + if err := replaceDefinerCmd.Wait(); err != nil { + return errors.Wrap(err, "sed command failed") + } + if err := mysqldump.Wait(); err != nil { return errors.Wrap(err, "mysqldump command failed") } diff --git a/tasks/migrate/backup_restore_test.go b/tasks/migrate/backup_restore_test.go index 3f96ddbb..8b39d8a3 100644 --- a/tasks/migrate/backup_restore_test.go +++ b/tasks/migrate/backup_restore_test.go @@ -64,9 +64,10 @@ var _ = Describe("MySQLDumpCmd", func() { "--port=3307", "--max-allowed-packet=1G", "--single-transaction", - "--routines", - "--events", + "--skip-routines", + "--skip-events", "--set-gtid-purged=off", + "--skip-triggers", "--databases", "foo", "bar", @@ -98,9 +99,10 @@ var _ = Describe("MySQLDumpCmd", func() { "--ssl-capath=/etc/ssl/certs", "--max-allowed-packet=1G", "--single-transaction", - "--routines", - "--events", + "--skip-routines", + "--skip-events", "--set-gtid-purged=off", + "--skip-triggers", "--databases", "foo", "bar", @@ -126,9 +128,10 @@ var _ = Describe("MySQLDumpCmd", func() { "--port=3307", "--max-allowed-packet=1G", "--single-transaction", - "--routines", - "--events", + "--skip-routines", + "--skip-events", "--set-gtid-purged=off", + "--skip-triggers", "one-database", })) Expect(mysqldump.Env).To(ContainElement("MYSQL_PWD=some-password")) @@ -157,9 +160,10 @@ var _ = Describe("MySQLDumpCmd", func() { "--ssl-capath=/etc/ssl/certs", "--max-allowed-packet=1G", "--single-transaction", - "--routines", - "--events", + "--skip-routines", + "--skip-events", "--set-gtid-purged=off", + "--skip-triggers", "one-database", })) Expect(mysqldump.Env).To(ContainElement("MYSQL_PWD=some-password")) @@ -235,12 +239,41 @@ var _ = Describe("MySQLCmd", func() { }) }) +var _ = Describe("ReplaceDefinerCmd", func() { + var ( + credentials Credentials + ) + + BeforeEach(func() { + credentials = Credentials{ + Username: "some-user-name", + Name: "some-db-name", + Password: "some-password", + Hostname: "some-hostname", + Port: 3307, + } + }) + + It("builds the sed command", func() { + replace := ReplaceDefinerCmd() + Expect(replace).ToNot(BeNil()) + Expect(replace.Args).To(Equal([]string{ + "sed", + "-e", + "s/DEFINER=`.*`@`%` SQL SECURITY DEFINER/SQL SECURITY INVOKER/", + })) + }) + +}) + var _ = Describe("CopyData", func() { var ( - mySQLDumpMock *binmock.Mock - mySQLDumpCmd *exec.Cmd - mySQLMock *binmock.Mock - mySQLCmd *exec.Cmd + mySQLDumpMock *binmock.Mock + mySQLDumpCmd *exec.Cmd + mySQLMock *binmock.Mock + mySQLCmd *exec.Cmd + replaceDefinerMock *binmock.Mock + replaceDefinerCmd *exec.Cmd ) BeforeEach(func() { @@ -251,20 +284,29 @@ var _ = Describe("CopyData", func() { WillExitWith(0) mySQLDumpCmd = exec.Command(mySQLDumpMock.Path) + replaceDefinerMock = binmock.NewBinMock(Fail) + replaceDefinerMock. + WhenCalled(). + WillPrintToStdOut(`replaced`). + WillExitWith(0) + replaceDefinerCmd = exec.Command(replaceDefinerMock.Path) + mySQLMock = binmock.NewBinMock(Fail) mySQLMock.WhenCalled().WillExitWith(0) mySQLCmd = exec.Command(mySQLMock.Path) }) It("Pipes the output of the mySQLDumpCmd command into the mySQLCmd command", func() { - Expect(CopyData(mySQLDumpCmd, mySQLCmd)).To(Succeed()) + Expect(CopyData(mySQLDumpCmd, replaceDefinerCmd, mySQLCmd)).To(Succeed()) Expect(mySQLDumpMock.Invocations()).To(HaveLen(1)) - + Expect(replaceDefinerMock.Invocations()).To(HaveLen(1)) Expect(mySQLMock.Invocations()).To(HaveLen(1)) - Expect(mySQLMock.Invocations()[0].Stdin()). + Expect(replaceDefinerMock.Invocations()[0].Stdin()). To(ConsistOf(`something`)) + Expect(mySQLMock.Invocations()[0].Stdin()). + To(ConsistOf(`replaced`)) }) When("piping the output of mysqldump fails", func() { @@ -281,7 +323,7 @@ var _ = Describe("CopyData", func() { }) It("returns an error", func() { - Expect(CopyData(mySQLDumpCmd, mySQLCmd)). + Expect(CopyData(mySQLDumpCmd, replaceDefinerCmd, mySQLCmd)). To(MatchError(`mysqldump command failed: exit status 1`)) }) }) @@ -292,7 +334,7 @@ var _ = Describe("CopyData", func() { }) It("returns an error", func() { - Expect(CopyData(mySQLDumpCmd, mySQLCmd)). + Expect(CopyData(mySQLDumpCmd, replaceDefinerCmd, mySQLCmd)). To(MatchError(`couldn't start mysqldump: fork/exec /invalid/path/to/mysqldump: no such file or directory`)) }) }) @@ -303,7 +345,7 @@ var _ = Describe("CopyData", func() { }) It("returns an error", func() { - Expect(CopyData(mySQLDumpCmd, mySQLCmd)). + Expect(CopyData(mySQLDumpCmd, replaceDefinerCmd, mySQLCmd)). To(MatchError(`couldn't start mysql: fork/exec /invalid/path/to/mysql: no such file or directory`)) }) }) @@ -315,7 +357,7 @@ var _ = Describe("CopyData", func() { }) It("returns an error", func() { - Expect(CopyData(mySQLDumpCmd, mySQLCmd)). + Expect(CopyData(mySQLDumpCmd, replaceDefinerCmd, mySQLCmd)). To(MatchError("mysqldump command failed: exit status 1")) }) }) @@ -327,7 +369,7 @@ var _ = Describe("CopyData", func() { }) It("returns an error", func() { - Expect(CopyData(mySQLDumpCmd, mySQLCmd)). + Expect(CopyData(mySQLDumpCmd, replaceDefinerCmd, mySQLCmd)). To(MatchError("mysql command failed: exit status 1")) }) }) @@ -338,7 +380,7 @@ var _ = Describe("CopyData", func() { }) It("returns an error", func() { - Expect(CopyData(mySQLDumpCmd, mySQLCmd)). + Expect(CopyData(mySQLDumpCmd, replaceDefinerCmd, mySQLCmd)). To(MatchError("couldn't pipe the output of mysqldump: exec: Stdout already set")) }) }) diff --git a/tasks/migrate/discovery.go b/tasks/migrate/discovery.go index 2dea0680..2f998384 100644 --- a/tasks/migrate/discovery.go +++ b/tasks/migrate/discovery.go @@ -28,17 +28,17 @@ func DiscoverDatabases(db *sql.DB) ([]string, error) { var dbs []string filterSchemas := map[string]struct{}{ - "cf_metadata": {}, + "cf_metadata": {}, "information_schema": {}, - "mysql": {}, + "mysql": {}, "performance_schema": {}, - "sys": {}, + "sys": {}, } for rows.Next() { var dbName string if err := rows.Scan(&dbName); err != nil { - return nil, errors.Wrap(err,"failed to scan the list of databases") + return nil, errors.Wrap(err, "failed to scan the list of databases") } if _, ok := filterSchemas[dbName]; ok { diff --git a/tasks/migrate/main.go b/tasks/migrate/main.go index 955e2e5b..0c9383dc 100644 --- a/tasks/migrate/main.go +++ b/tasks/migrate/main.go @@ -75,8 +75,9 @@ func main() { mySQLDumpCmd := MySQLDumpCmd(sourceCredentials, sourceSchemas...) mySQLCmd := MySQLCmd(destCredentials) + replaceCmd := ReplaceDefinerCmd() - if err := CopyData(mySQLDumpCmd, mySQLCmd); err != nil { + if err := CopyData(mySQLDumpCmd, replaceCmd, mySQLCmd); err != nil { log.Fatalf("Failed to copy data: %v", err) } } diff --git a/test_helpers/poll_cf/commandstarter/command_starter.go b/test_helpers/poll_cf/commandstarter/command_starter.go index ad6f7916..a512c0fc 100644 --- a/test_helpers/poll_cf/commandstarter/command_starter.go +++ b/test_helpers/poll_cf/commandstarter/command_starter.go @@ -13,10 +13,10 @@ package commandstarter import ( - "os/exec" "github.com/onsi/ginkgo" "github.com/onsi/gomega/gexec" "github.com/pivotal-cf/mysql-cli-plugin/test_helpers/poll_cf/internal" + "os/exec" ) type CommandStarter struct {