test migrated db tables are owned by role from dbc spec (#339)

* test migrated db tables are owned by role from dbc spec

* unnest if statements

internal/controller/databasecontroller_migrate_test.go

@@ -73,6 +73,9 @@ var _ = Describe("claim migrate", func() {
 		// master creds to target db
 		// const targetSecretName = "postgres-target"
 
+		// user for new migrated database
+		var migratedowner = "migrate"
+
 		typeNamespacedName := types.NamespacedName{
 			Name:      resourceName,
 			Namespace: "default",
@@ -100,7 +103,8 @@ var _ = Describe("claim migrate", func() {
 			Expect(k8sClient.Get(kctx, typeNamespacedName, claim)).To(HaveOccurred())
 
 			By("creating the custom resource for the Kind DatabaseClaim")
-			parsedDSN, err := url.Parse(testDSN)
+			_, err := url.Parse(testDSN)
+
 			Expect(err).NotTo(HaveOccurred())
 			Expect(client.IgnoreNotFound(err)).To(Succeed())
 
@@ -130,7 +134,8 @@ var _ = Describe("claim migrate", func() {
 							},
 						},
 					},
-					Username: parsedDSN.User.Username(),
+					//Username: parsedDSN.User.Username(),
+					Username: migratedowner,
 				},
 			}
 			Expect(k8sClient.Create(kctx, resource)).To(Succeed())
@@ -201,18 +206,21 @@ var _ = Describe("claim migrate", func() {
 			aConn := claim.Status.ActiveDB.ConnectionInfo
 			Expect(aConn.Host).To(Equal(u.Hostname()))
 			Expect(aConn.Port).To(Equal(u.Port()))
-			Expect(aConn.Username).To(Equal(u.User.Username() + "_a"))
+			Expect(aConn.Username).To(Equal(migratedowner + "_a"))
 			Expect(aConn.DatabaseName).To(Equal(strings.TrimPrefix(u.Path, "/")))
 			Expect(aConn.SSLMode).To(Equal(u.Query().Get("sslmode")))
 
+			activeURI, err := url.Parse(aConn.Uri())
+			Expect(err).NotTo(HaveOccurred())
+
 			By("Checking the DSN in the secret")
-			redacted := u.Redacted()
+			redacted := activeURI.Redacted()
 			var creds corev1.Secret
 			Expect(k8sClient.Get(ctxLogger, typeNamespacedClaimSecretName, &creds)).NotTo(HaveOccurred())
 			Expect(creds.Data[persistancev1.DSNURIKey]).ToNot(BeNil())
 			dsn, err := url.Parse(string(creds.Data[persistancev1.DSNURIKey]))
 			Expect(err).NotTo(HaveOccurred())
-			Expect(strings.Replace(dsn.Redacted(), "_a", "", 1)).To(Equal(redacted))
+			Expect(dsn.Redacted()).To(Equal(redacted))
 		})
 
 		It("Migrate", func() {
@@ -233,9 +241,8 @@ var _ = Describe("claim migrate", func() {
 			fakeCPSecretName := fmt.Sprintf("%s-%s-%s", env, resourceName, hostParams.Hash())
 
 			By(fmt.Sprintf("Mocking a RDS pod to look like crossplane set it up: %s", fakeCPSecretName))
-			fakeDSN, fakeCancel := dockerdb.MockRDS(GinkgoT(), ctxLogger, k8sClient, fakeCPSecretName, "migrate", dbc.Spec.DatabaseName)
-			_ = fakeCancel
-			// DeferCleanup(fakeCancel)
+			fakeCli, fakeDSN, fakeCancel := dockerdb.MockRDS(GinkgoT(), ctxLogger, k8sClient, fakeCPSecretName, "migrate", dbc.Spec.DatabaseName)
+			DeferCleanup(fakeCancel)
 
 			fakeCPSecret := corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
@@ -262,11 +269,12 @@ var _ = Describe("claim migrate", func() {
 			By("Check source DSN looks roughly correct")
 			activeDB := dbc.Status.ActiveDB
 			Expect(activeDB.ConnectionInfo).NotTo(BeNil())
-			compareDSN := strings.Replace(testDSN, "//postgres:postgres", "//postgres_b:", 1)
+			compareDSN := strings.Replace(testDSN, "//postgres:postgres", fmt.Sprintf("//%s_b:", migratedowner), 1)
 			Expect(activeDB.ConnectionInfo.Uri()).To(Equal(compareDSN))
+
 			By("Check target DSN looks roughly correct")
 			newDB := dbc.Status.NewDB
-			compareDSN = strings.Replace(fakeDSN, "//migrate:postgres", "//postgres_a:", 1)
+			compareDSN = strings.Replace(fakeDSN, "//migrate:postgres", fmt.Sprintf("//%s_a:", migratedowner), 1)
 			Expect(newDB.ConnectionInfo).NotTo(BeNil())
 			Expect(newDB.ConnectionInfo.Uri()).To(Equal(compareDSN))
 			Expect(dbc.Status.MigrationState).To(Equal(pgctl.S_MigrationInProgress.String()))
@@ -288,7 +296,7 @@ var _ = Describe("claim migrate", func() {
 			By("Waiting to disable source, reconcile manually again")
 			res, err = controllerReconciler.Reconcile(ctxLogger, reconcile.Request{NamespacedName: typeNamespacedName})
 			Expect(err).To(BeNil())
-			Expect(res.RequeueAfter).To(Equal(time.Duration(60)))
+			Expect(res.RequeueAfter).To(Equal(time.Duration(60 * time.Second)))
 			By("Verify migration is complete on this reconcile")
 			res, err = controllerReconciler.Reconcile(ctxLogger, reconcile.Request{NamespacedName: typeNamespacedName})
 			Expect(err).To(BeNil())
@@ -299,6 +307,13 @@ var _ = Describe("claim migrate", func() {
 			activeDB = dbc.Status.ActiveDB
 			Expect(activeDB.ConnectionInfo).NotTo(BeNil())
 			Expect(activeDB.ConnectionInfo.Uri()).To(Equal(compareDSN))
+
+			By(fmt.Sprintf("Verify owner of migrated DB is %s", migratedowner))
+			row := fakeCli.QueryRowContext(ctxLogger, "select tableowner from pg_tables where tablename = 'users' AND schemaname = 'public';")
+			var owner string
+			Expect(row.Scan(&owner)).NotTo(HaveOccurred())
+			Expect(owner).To(Equal(migratedowner))
+
 		})
 
 	})

internal/controller/suite_test.go

@@ -136,6 +136,15 @@ var _ = BeforeSuite(func() {
 	})
 	logger.Info("postgres_setup_took", "duration", time.Since(now))
 
+	// Mock table for testing migrations
+	_, err = testdb.Exec(`CREATE TABLE IF NOT EXISTS users (
+		id SERIAL PRIMARY KEY,
+		name TEXT NOT NULL,
+		email TEXT NOT NULL,
+		created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
+	)`)
+	Expect(err).NotTo(HaveOccurred())
+
 	// Setup controller
 	By("setting up the database controller")
 	configPath, err := filepath.Abs(filepath.Join("..", "..", "cmd", "config", "config.yaml"))

internal/dockerdb/mockdb.go

@@ -2,6 +2,7 @@ package dockerdb
 
 import (
 	"context"
+	"database/sql"
 	"net/url"
 	"strings"
 
@@ -12,10 +13,10 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func MockRDS(t GinkgoTInterface, ctx context.Context, cli client.Client, secretName, userName, databaseName string) (string, func()) {
+func MockRDS(t GinkgoTInterface, ctx context.Context, cli client.Client, secretName, userName, databaseName string) (*sql.DB, string, func()) {
 	t.Helper()
 
-	_, fakeDSN, clean := Run(Config{
+	dbCli, fakeDSN, clean := Run(Config{
 		Database:  databaseName,
 		Username:  userName,
 		Password:  "postgres",
@@ -46,9 +47,9 @@ func MockRDS(t GinkgoTInterface, ctx context.Context, cli client.Client, secretN
 		t.Fatalf("failed to create secret: %v", err)
 	}
 
-	return fakeDSN, func() {
+	return dbCli, fakeDSN, func() {
 		if err := cli.Delete(ctx, secret); err != nil {
-			t.Fatalf("failed to delete secret: %v", err)
+			t.Logf("failed to delete secret: %v", err)
 		}
 		clean()
 	}

pkg/databaseclaim/databaseclaim.go

@@ -1139,7 +1139,9 @@ func (r *DatabaseClaimReconciler) manageUserAndExtensions(ctx context.Context, r
 	if err != nil {
 		return err
 	}
-	if roleCreated && operationalMode != M_MigrateExistingToNewDB && operationalMode != M_MigrationInProgress {
+
+	if roleCreated && operationalMode == M_UseNewDB {
+		// This only runs on new databases, and perhaps should not even be run there
 		// take care of special extensions related to the user
 		err = dbClient.CreateSpecialExtensions(dbName, baseUsername)
 		if err != nil {