Merge release10x (#396)

* update CI pipeline for gitflow branch names * PTEUDO-2142 remove validation of DSNName (#370) Previously, customizing the dsnname would throw a validation error and prevent any processing of the databaseclaim. This allows the customization but ensures the normalized fields are always populated. * update start-pgbouncer to exit after 60seconds (#379) * DEVOPS-30046 DEVOPS-30239 dsnexec conversion injects dbproxy (#381) A bug in the conversion webhook added dbproxy mutating labels when only dsnexec was requested. This would cause issues if the pod could not handle dbproxy listening on port 5432. * DEVOPS-30046 optional sidecar probes (#384) Make the probes optional as a fallback that the liveness probe failure issues are not resolved. * DEVOPS-30870: fix password rotation logic (#393) * Fix conflicts * Fix conflicts --------- Co-authored-by: Drew Wells <[email protected]>
infobloxopen · Jan 22, 2025 · d869227 · d869227
1 parent 84f1a71
commit d869227
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 23 deletions.
diff --git a/internal/controller/databaseclaim_controller_test.go b/internal/controller/databaseclaim_controller_test.go
@@ -327,5 +327,35 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
 			Expect(err).To(HaveOccurred())
 		})
+
+		It("Reconcile rotates the username", func() {
+			By("Updating CR with a DB Version")
+
+			resource := &persistancev1.DatabaseClaim{}
+			Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).NotTo(HaveOccurred())
+			Expect(resource.Spec.DBVersion).To(Equal(""))
+
+			By("Rotating to UserSuffixA")
+			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).NotTo(HaveOccurred())
+			Expect(resource.Status.Error).To(Equal(""))
+			Expect(resource.Status.ActiveDB.ConnectionInfo.Username).To(Equal("postgres_a"))
+
+			By("Rotating to UserSuffixB")
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).NotTo(HaveOccurred())
+			Expect(resource.Status.Error).To(Equal(""))
+			Expect(resource.Status.ActiveDB.ConnectionInfo.Username).To(Equal("postgres_b"))
+
+			By("Rotating to UserSuffixA")
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).NotTo(HaveOccurred())
+			Expect(resource.Status.Error).To(Equal(""))
+			Expect(resource.Status.ActiveDB.ConnectionInfo.Username).To(Equal("postgres_a"))
+
+		})
 	})
 })
diff --git a/pkg/databaseclaim/databaseclaim.go b/pkg/databaseclaim/databaseclaim.go
@@ -473,7 +473,7 @@ func (r *DatabaseClaimReconciler) reconcileUseExistingDB(ctx context.Context, re
 	dbName := existingDBConnInfo.DatabaseName
 	r.statusManager.UpdateDBStatus(&dbClaim.Status.NewDB, dbName)
 
-	err = r.manageUserAndExtensions(ctx, reqInfo, logr, dbClient, dbClaim, operationalMode)
+	err = r.manageUserAndExtensions(ctx, reqInfo, logr, dbClient, &dbClaim.Status.NewDB, &dbClaim.Status.ActiveDB, dbName, dbClaim.Spec.Username, operationalMode)
 	if err != nil {
 		logr.Error(err, "unable to update users, user credentials not persisted to status object")
 		return err
@@ -578,7 +578,7 @@ func (r *DatabaseClaimReconciler) reconcileNewDB(ctx context.Context, reqInfo *r
 	}
 
 	// Updates the connection info username
-	err = r.manageUserAndExtensions(ctx, reqInfo, logr, dbClient, dbClaim, operationalMode)
+	err = r.manageUserAndExtensions(ctx, reqInfo, logr, dbClient, &dbClaim.Status.NewDB, &dbClaim.Status.ActiveDB, dbClaim.Spec.DatabaseName, dbClaim.Spec.Username, operationalMode)
 	if err != nil {
 		logr.Error(err, "unable to update users, user credentials not persisted to status object")
 		return r.statusManager.SetError(ctx, dbClaim, err)
@@ -1126,11 +1126,15 @@ func (r *DatabaseClaimReconciler) createDatabaseAndExtensions(ctx context.Contex
 	return nil
 }
 
-func (r *DatabaseClaimReconciler) manageUserAndExtensions(ctx context.Context, reqInfo *requestInfo, logger logr.Logger, dbClient dbclient.Clienter, dbClaim *v1.DatabaseClaim, operationalMode ModeEnum) error {
+func (r *DatabaseClaimReconciler) manageUserAndExtensions(ctx context.Context, reqInfo *requestInfo, logger logr.Logger, dbClient dbclient.Clienter, statusNewDB *v1.Status, statusActiveDB *v1.Status, dbName string, baseUsername string, operationalMode ModeEnum) error {
 
-	status := &dbClaim.Status.NewDB
-	dbName := dbClaim.Spec.DatabaseName
-	baseUsername := dbClaim.Spec.Username
+	if statusNewDB == nil {
+		return fmt.Errorf("status newDB is nil")
+	}
+
+	if statusNewDB.ConnectionInfo == nil {
+		return fmt.Errorf("newDB connection info is nil")
+	}
 
 	dbu := dbuser.NewDBUser(baseUsername)
 	rotationTime := r.getPasswordRotationTime()
@@ -1150,14 +1154,14 @@ func (r *DatabaseClaimReconciler) manageUserAndExtensions(ctx context.Context, r
 		}
 	}
 
-	if status.ConnectionInfo == nil {
-		return fmt.Errorf("connection info is nil")
+	currentUser := statusNewDB.ConnectionInfo.Username
+	if currentUser == "" && statusActiveDB.ConnectionInfo != nil && statusActiveDB.ConnectionInfo.Uri() != "" && operationalMode == M_UseNewDB {
+		logger.Info("using active db user", "active_db", statusActiveDB)
+		currentUser = statusActiveDB.ConnectionInfo.Username
 	}
 
-	userName := status.ConnectionInfo.Username
-
-	if dbu.IsUserChanged(userName) {
-		oldUsername := dbuser.TrimUserSuffix(userName)
+	if dbu.IsUserChanged(currentUser) {
+		oldUsername := dbuser.TrimUserSuffix(currentUser)
 		if err := dbClient.RenameUser(oldUsername, baseUsername); err != nil {
 			return err
 		}
@@ -1169,7 +1173,7 @@ func (r *DatabaseClaimReconciler) manageUserAndExtensions(ctx context.Context, r
 		if err := dbClient.UpdateUser(oldUsername+dbuser.SuffixA, dbu.GetUserA(), baseUsername, userPassword); err != nil {
 			return err
 		}
-		r.statusManager.UpdateUserStatus(status, reqInfo, dbu.GetUserA(), userPassword)
+		r.statusManager.UpdateUserStatus(statusNewDB, reqInfo, dbu.GetUserA(), userPassword)
 		// updating user b
 		userPassword, err = r.generatePassword()
 		if err != nil {
@@ -1180,27 +1184,32 @@ func (r *DatabaseClaimReconciler) manageUserAndExtensions(ctx context.Context, r
 		}
 	}
 
-	if status.UserUpdatedAt == nil || time.Since(status.UserUpdatedAt.Time) > rotationTime {
-		logger.V(1).Info("rotating_users", "rotationTime", rotationTime)
+	if statusNewDB.UserUpdatedAt == nil || time.Since(statusActiveDB.UserUpdatedAt.Time) >= rotationTime {
+		logger.V(1).Info("rotating_user_password",
+			"currentUser", currentUser,
+			"dbu", dbu,
+			"statusNewDB", statusNewDB,
+			"statusActiveDB", statusActiveDB)
 
 		userPassword, err := r.generatePassword()
 		if err != nil {
 			return err
 		}
 
-		nextUser := dbu.NextUser(status.ConnectionInfo.Username)
-		created, err := dbClient.CreateUser(nextUser, baseUsername, userPassword)
+		nextUser := dbu.NextUser(currentUser)
+		logger.V(1).Info("setting next user", "nextUser", nextUser)
+
+		_, err = dbClient.CreateUser(nextUser, baseUsername, userPassword)
 		if err != nil {
 			metrics.PasswordRotatedErrors.WithLabelValues("create error").Inc()
 			return err
 		}
-		_ = created
 
 		if err := dbClient.UpdatePassword(nextUser, userPassword); err != nil {
 			return err
 		}
 
-		r.statusManager.UpdateUserStatus(status, reqInfo, nextUser, userPassword)
+		r.statusManager.UpdateUserStatus(statusNewDB, reqInfo, nextUser, userPassword)
 	}
 
 	// baseUsername = myuser
@@ -1217,17 +1226,17 @@ func (r *DatabaseClaimReconciler) manageUserAndExtensions(ctx context.Context, r
 		return fmt.Errorf("managecreaterole on role %s: %w", baseUsername, err)
 	}
 	// TODO: change this to modify the the role ie. baseUsername
-	err = dbClient.ManageReplicationRole(status.ConnectionInfo.Username, reqInfo.EnableReplicationRole)
+	err = dbClient.ManageReplicationRole(currentUser, reqInfo.EnableReplicationRole)
 	// Ignore errors when revoking replication role from a userrole. This may
 	// fail on non-cloud databases
 	if err != nil && reqInfo.EnableReplicationRole {
-		return fmt.Errorf("managereplicationrole on role %s: %w", status.ConnectionInfo.Username, err)
+		return fmt.Errorf("managereplicationrole on role %s: %w", currentUser, err)
 	}
-	err = dbClient.ManageReplicationRole(dbu.NextUser(status.ConnectionInfo.Username), reqInfo.EnableReplicationRole)
+	err = dbClient.ManageReplicationRole(dbu.NextUser(currentUser), reqInfo.EnableReplicationRole)
 	// Ignore errors when revoking replication role from a userrole. This may
 	// fail on non-cloud databases
 	if err != nil && reqInfo.EnableReplicationRole {
-		return fmt.Errorf("managereplicationrole on role %s: %w", dbu.NextUser(status.ConnectionInfo.Username), err)
+		return fmt.Errorf("managereplicationrole on role %s: %w", dbu.NextUser(currentUser), err)
 	}
 
 	return nil

diff --git a/pkg/dbuser/dbuser.go b/pkg/dbuser/dbuser.go
@@ -9,12 +9,14 @@ const (
 	SuffixB = "_b"
 )
 
+// DBUser is a struct that holds the usernames for a given role.
 type DBUser struct {
 	rolename string
 	userA    string
 	userB    string
 }
 
+// NewDBUser returns a new DBUser instance.
 func NewDBUser(baseName string) DBUser {
 	return DBUser{
 		rolename: baseName,
@@ -23,6 +25,8 @@ func NewDBUser(baseName string) DBUser {
 	}
 }
 
+// IsUserChanged checks if the given currentUserName has changed compared to the
+// rolename of the DBUser instance.
 func (dbu DBUser) IsUserChanged(currentUserName string) bool {
 
 	if currentUserName == "" {
@@ -32,14 +36,17 @@ func (dbu DBUser) IsUserChanged(currentUserName string) bool {
 	return TrimUserSuffix(currentUserName) != dbu.rolename
 }
 
+// TrimUserSuffix removes the suffixes from the given string.
 func TrimUserSuffix(in string) string {
 	return strings.TrimSuffix(strings.TrimSuffix(in, SuffixA), SuffixB)
 }
 
+// GetUserA returns the username for UserA.
 func (dbu DBUser) GetUserA() string {
 	return dbu.userA
 }
 
+// GetUserB returns the username for UserB.
 func (dbu DBUser) GetUserB() string {
 	return dbu.userB
 }