Merge branch 'main' into PMM-10684-clickhouse-v2

api/managementpb/role/role.proto

@@ -44,6 +44,8 @@ message DeleteRoleRequest {
       int_gt: 0
     }
   ];
+  // Role ID to be used as a replacement for the role. Additional logic applies.
+  uint32 replacement_role_id = 2;
 }
 
 message DeleteRoleResponse {}

managed/models/role_helpers.go

@@ -27,8 +27,6 @@ import (
 var (
 	// ErrRoleNotFound is returned when a role is not found.
 	ErrRoleNotFound = fmt.Errorf("RoleNotFound")
-	// ErrRoleIsAssigned is returned when a role is assigned to a user and cannot be removed.
-	ErrRoleIsAssigned = fmt.Errorf("RoleIsAssigned")
 	// ErrRoleIsDefaultRole is returned when trying to delete a default role.
 	ErrRoleIsDefaultRole = fmt.Errorf("RoleIsDefaultRole")
 )
@@ -81,14 +79,15 @@ func AssignDefaultRole(tx *reform.TX, userID int) error {
 }
 
 // DeleteRole deletes a role, if possible.
-func DeleteRole(tx *reform.TX, roleID int) error {
+func DeleteRole(tx *reform.TX, roleID, replacementRoleID int) error {
 	q := tx.Querier
 
 	var role Role
 	if err := FindAndLockRole(tx, roleID, &role); err != nil {
 		return err
 	}
 
+	// Check if it's the default role.
 	settings, err := GetSettings(tx)
 	if err != nil {
 		return err
@@ -98,15 +97,10 @@ func DeleteRole(tx *reform.TX, roleID int) error {
 		return ErrRoleIsDefaultRole
 	}
 
-	s, err := q.FindOneFrom(UserRolesView, "role_id", roleID)
-	if err != nil && !errors.As(err, &reform.ErrNoRows) {
+	if err := replaceRole(tx, roleID, replacementRoleID); err != nil {
 		return err
 	}
 
-	if s != nil {
-		return ErrRoleIsAssigned
-	}
-
 	if err := q.Delete(&role); err != nil {
 		if errors.As(err, &reform.ErrNoRows) {
 			return ErrRoleNotFound
@@ -118,6 +112,72 @@ func DeleteRole(tx *reform.TX, roleID int) error {
 	return nil
 }
 
+func replaceRole(tx *reform.TX, roleID, newRoleID int) error {
+	q := tx.Querier
+
+	// If no new role is assigned, we remove all role assignements.
+	if newRoleID == 0 {
+		_, err := q.DeleteFrom(UserRolesView, "WHERE role_id = $1", roleID)
+		return err
+	}
+
+	if err := FindAndLockRole(tx, newRoleID, &Role{}); err != nil {
+		return err
+	}
+
+	// Check if the role is the last role for a user and apply special logic if it is.
+	structs, err := q.FindAllFrom(UserRolesView, "role_id", roleID)
+	if err != nil {
+		return err
+	}
+
+	for _, s := range structs {
+		ur, ok := s.(*UserRoles)
+		if !ok {
+			return fmt.Errorf("invalid data structure in user roles for role ID %d. Found %+v", roleID, s)
+		}
+
+		roleStructs, err := q.SelectAllFrom(UserRolesView, "WHERE user_id = $1 FOR UPDATE", ur.UserID)
+		if err != nil {
+			return err
+		}
+
+		// If there are more than 1 roles, we remove the role without a replacement.
+		if len(roleStructs) > 1 {
+			_, err := q.DeleteFrom(UserRolesView, "WHERE user_id = $1 AND role_id = $2", ur.UserID, roleID)
+			if err != nil {
+				return err
+			}
+
+			continue
+		}
+
+		// The removed role is the last one. We replace it with a new role.
+		_, err = q.Exec(`
+			UPDATE user_roles
+			SET
+				role_id = $1
+			WHERE
+				user_id = $2 AND
+				role_id = $3 AND
+				NOT EXISTS(
+					SELECT 1
+					FROM user_roles ur
+					WHERE
+						ur.user_id = $2 AND
+						ur.role_id = $1
+				)`,
+			newRoleID,
+			ur.UserID,
+			roleID)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // FindAndLockRole retrieves a role by ID and locks it for update.
 func FindAndLockRole(tx *reform.TX, roleID int, role *Role) error {
 	err := tx.Querier.SelectOneTo(role, "WHERE id = $1 FOR UPDATE", roleID)

managed/models/role_helpers_test.go

@@ -27,6 +27,11 @@ import (
 	"github.com/percona/pmm/managed/utils/testdb"
 )
 
+const (
+	roleATitle = "Role A"
+	roleBTitle = "Role B"
+)
+
 //nolint:paralleltest
 func TestRoleHelpers(t *testing.T) {
 	sqlDB := testdb.Open(t, models.SkipFixtures, nil)
@@ -72,7 +77,7 @@ func TestRoleHelpers(t *testing.T) {
 	})
 
 	//nolint:paralleltest
-	t.Run("shall throw on role not found", func(t *testing.T) {
+	t.Run("shall throw on assigning non-existent role", func(t *testing.T) {
 		tx, teardown := setup(t)
 		defer teardown(t)
 
@@ -81,7 +86,7 @@ func TestRoleHelpers(t *testing.T) {
 	})
 
 	//nolint:paralleltest
-	t.Run("shall create new user", func(t *testing.T) {
+	t.Run("shall create a new user on role assign", func(t *testing.T) {
 		tx, teardown := setup(t)
 		defer teardown(t)
 
@@ -94,27 +99,109 @@ func TestRoleHelpers(t *testing.T) {
 	})
 
 	//nolint:paralleltest
-	t.Run("shall delete role", func(t *testing.T) {
-		tx, teardown := setup(t)
-		defer teardown(t)
-
-		var role models.Role
-		role.Title = "Role A"
-		require.NoError(t, models.CreateRole(tx.Querier, &role))
-		require.NoError(t, models.DeleteRole(tx, int(role.ID)))
+	t.Run("unassigned role", func(t *testing.T) {
+		//nolint:paralleltest
+		t.Run("shall delete role with no replacement", func(t *testing.T) {
+			tx, teardown := setup(t)
+			defer teardown(t)
+
+			var role models.Role
+			role.Title = roleATitle
+			require.NoError(t, models.CreateRole(tx.Querier, &role))
+			require.NoError(t, models.DeleteRole(tx, int(role.ID), 0))
+		})
+
+		//nolint:paralleltest
+		t.Run("shall delete role with replacement", func(t *testing.T) {
+			tx, teardown := setup(t)
+			defer teardown(t)
+
+			var roleA, roleB models.Role
+			roleA.Title = roleATitle
+			roleB.Title = roleBTitle
+			require.NoError(t, models.CreateRole(tx.Querier, &roleA))
+			require.NoError(t, models.CreateRole(tx.Querier, &roleB))
+			require.NoError(t, models.DeleteRole(tx, int(roleA.ID), int(roleB.ID)))
+		})
 	})
 
 	//nolint:paralleltest
-	t.Run("shall not delete if role is assigned", func(t *testing.T) {
-		tx, teardown := setup(t)
-		defer teardown(t)
-
-		var role models.Role
-		require.NoError(t, models.CreateRole(tx.Querier, &role))
-		require.NoError(t, models.AssignRoles(tx, 24, []int{int(role.ID)}))
+	t.Run("single role assigned", func(t *testing.T) {
+		//nolint:paralleltest
+		t.Run("shall delete role with no replacement", func(t *testing.T) {
+			tx, teardown := setup(t)
+			defer teardown(t)
+
+			var role models.Role
+			role.Title = roleATitle
+			require.NoError(t, models.CreateRole(tx.Querier, &role))
+			require.NoError(t, models.AssignRoles(tx, userID, []int{int(role.ID)}))
+			require.NoError(t, models.DeleteRole(tx, int(role.ID), 0))
+
+			roles, err := models.GetUserRoles(tx.Querier, userID)
+			require.NoError(t, err)
+			require.Equal(t, len(roles), 0)
+		})
+
+		//nolint:paralleltest
+		t.Run("shall delete role with replacement", func(t *testing.T) {
+			tx, teardown := setup(t)
+			defer teardown(t)
+
+			var roleA, roleB models.Role
+			roleA.Title = roleATitle
+			roleB.Title = roleBTitle
+			require.NoError(t, models.CreateRole(tx.Querier, &roleA))
+			require.NoError(t, models.CreateRole(tx.Querier, &roleB))
+			require.NoError(t, models.AssignRoles(tx, userID, []int{int(roleA.ID)}))
+			require.NoError(t, models.DeleteRole(tx, int(roleA.ID), int(roleB.ID)))
+
+			roles, err := models.GetUserRoles(tx.Querier, userID)
+			require.NoError(t, err)
+			require.Equal(t, len(roles), 1)
+			require.Equal(t, roles[0].ID, roleB.ID)
+		})
+	})
 
-		err := models.DeleteRole(tx, int(role.ID))
-		require.Equal(t, err, models.ErrRoleIsAssigned)
+	//nolint:paralleltest
+	t.Run("multiple roles assigned", func(t *testing.T) {
+		//nolint:paralleltest
+		t.Run("shall delete role with no replacement", func(t *testing.T) {
+			tx, teardown := setup(t)
+			defer teardown(t)
+
+			var roleA, roleB models.Role
+			roleA.Title = roleATitle
+			roleB.Title = roleBTitle
+			require.NoError(t, models.CreateRole(tx.Querier, &roleA))
+			require.NoError(t, models.CreateRole(tx.Querier, &roleB))
+			require.NoError(t, models.AssignRoles(tx, userID, []int{int(roleA.ID), int(roleB.ID)}))
+			require.NoError(t, models.DeleteRole(tx, int(roleA.ID), 0))
+
+			roles, err := models.GetUserRoles(tx.Querier, userID)
+			require.NoError(t, err)
+			require.Equal(t, len(roles), 1)
+			require.Equal(t, roles[0].ID, roleB.ID)
+		})
+
+		//nolint:paralleltest
+		t.Run("shall delete role with replacement", func(t *testing.T) {
+			tx, teardown := setup(t)
+			defer teardown(t)
+
+			var roleA, roleB models.Role
+			roleA.Title = roleATitle
+			roleB.Title = roleBTitle
+			require.NoError(t, models.CreateRole(tx.Querier, &roleA))
+			require.NoError(t, models.CreateRole(tx.Querier, &roleB))
+			require.NoError(t, models.AssignRoles(tx, userID, []int{int(roleA.ID), int(roleB.ID)}))
+			require.NoError(t, models.DeleteRole(tx, int(roleA.ID), int(roleB.ID)))
+
+			roles, err := models.GetUserRoles(tx.Querier, userID)
+			require.NoError(t, err)
+			require.Equal(t, len(roles), 1)
+			require.Equal(t, roles[0].ID, roleB.ID)
+		})
 	})
 
 	//nolint:paralleltest
@@ -127,7 +214,7 @@ func TestRoleHelpers(t *testing.T) {
 		require.NoError(t, models.AssignRoles(tx, 24, []int{int(role.ID)}))
 		require.NoError(t, models.ChangeDefaultRole(tx, int(role.ID)))
 
-		err := models.DeleteRole(tx, int(role.ID))
+		err := models.DeleteRole(tx, int(role.ID), 0)
 		require.Equal(t, err, models.ErrRoleIsDefaultRole)
 	})
 
@@ -137,8 +224,8 @@ func TestRoleHelpers(t *testing.T) {
 		defer teardown(t)
 
 		var roleA, roleB models.Role
-		roleA.Title = "Role A"
-		roleB.Title = "Role B"
+		roleA.Title = roleATitle
+		roleB.Title = roleBTitle
 
 		require.NoError(t, models.CreateRole(tx.Querier, &roleA))
 		require.NoError(t, models.CreateRole(tx.Querier, &roleB))

managed/services/management/role.go

@@ -91,7 +91,7 @@ func (r *RoleService) UpdateRole(_ context.Context, req *rolev1beta1.UpdateRoleR
 //nolint:unparam
 func (r *RoleService) DeleteRole(_ context.Context, req *rolev1beta1.DeleteRoleRequest) (*rolev1beta1.DeleteRoleResponse, error) {
 	errTx := r.db.InTransaction(func(tx *reform.TX) error {
-		if err := models.DeleteRole(tx, int(req.RoleId)); err != nil {
+		if err := models.DeleteRole(tx, int(req.RoleId), int(req.ReplacementRoleId)); err != nil {
 			if errors.Is(err, models.ErrRoleNotFound) {
 				return status.Errorf(codes.NotFound, "Role not found")
 			}

update/ansible/playbook/tasks/update.yml

@@ -14,6 +14,7 @@
       - dbaas-tools
       - pmm2-client
       - pmm-dump
+      - vmproxy
   pre_tasks:
     - name: detect /srv/pmm-distribution
       stat: