fix(kv): Ensure UpdateUser cleans up the index when updating names.

kv/user.go

@@ -354,7 +354,7 @@ func (s *Service) updateUser(ctx context.Context, tx Tx, id influxdb.ID, upd inf
 	}
 
 	if upd.Name != nil {
-		if err := s.removeUserFromIndex(ctx, tx, id, *upd.Name); err != nil {
+		if err := s.removeUserFromIndex(ctx, tx, id, u.Name); err != nil {
 			return nil, err
 		}
 

tenant/error_user.go

@@ -8,6 +8,8 @@ import (
 
 var (
 	// ErrUserNotFound is used when the user is not found.
+	// TODO(brett): Transition to using this erroronce the tenant service is
+	// universally available.
 	ErrUserNotFound = &influxdb.Error{
 		Msg:  "user not found",
 		Code: influxdb.ENotFound,

tenant/service_user.go

@@ -31,7 +31,7 @@ func (s *Service) FindUserByID(ctx context.Context, id influxdb.ID) (*influxdb.U
 func (s *Service) FindUser(ctx context.Context, filter influxdb.UserFilter) (*influxdb.User, error) {
 	// if im given no filters its not a valid find user request. (leaving it unchecked seems dangerous)
 	if filter.ID == nil && filter.Name == nil {
-		return nil, ErrUserNotFound
+		return nil, kv.ErrUserNotFound
 	}
 
 	if filter.ID != nil {

tenant/storage_user.go

@@ -68,7 +68,7 @@ func (s *Store) GetUser(ctx context.Context, tx kv.Tx, id influxdb.ID) (*influxd
 
 	v, err := b.Get(encodedID)
 	if kv.IsNotFound(err) {
-		return nil, ErrUserNotFound
+		return nil, kv.ErrUserNotFound
 	}
 
 	if err != nil {
@@ -86,7 +86,7 @@ func (s *Store) GetUserByName(ctx context.Context, tx kv.Tx, n string) (*influxd
 
 	uid, err := b.Get([]byte(n))
 	if err == kv.ErrKeyNotFound {
-		return nil, ErrUserNotFound
+		return nil, kv.ErrUserNotFound
 	}
 
 	if err != nil {

tenant/storage_user_test.go

@@ -90,11 +90,11 @@ func TestUser(t *testing.T) {
 					t.Fatalf("expected identical user: \n%+v\n%+v", user, expected)
 				}
 
-				if _, err := store.GetUser(context.Background(), tx, 500); err != tenant.ErrUserNotFound {
+				if _, err := store.GetUser(context.Background(), tx, 500); err != kv.ErrUserNotFound {
 					t.Fatal("failed to get correct error when looking for invalid user by id")
 				}
 
-				if _, err := store.GetUserByName(context.Background(), tx, "notauser"); err != tenant.ErrUserNotFound {
+				if _, err := store.GetUserByName(context.Background(), tx, "notauser"); err != kv.ErrUserNotFound {
 					t.Fatal("failed to get correct error when looking for invalid user by name")
 				}
 
@@ -207,7 +207,7 @@ func TestUser(t *testing.T) {
 				}
 
 				err = store.DeleteUser(context.Background(), tx, 1)
-				if err != tenant.ErrUserNotFound {
+				if err != kv.ErrUserNotFound {
 					t.Fatal("invalid error when deleting user that has already been deleted", err)
 				}
 

testing/user_service.go

@@ -3,12 +3,14 @@ package testing
 import (
 	"bytes"
 	"context"
+	"errors"
 	"sort"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/influxdata/influxdb/v2"
 	platform "github.com/influxdata/influxdb/v2"
+	"github.com/influxdata/influxdb/v2/kv"
 	"github.com/influxdata/influxdb/v2/mock"
 )
 
@@ -79,6 +81,10 @@ func UserService(
 			name: "UpdateUser",
 			fn:   UpdateUser,
 		},
+		{
+			name: "UpdateUser_IndexHygiene",
+			fn:   UpdateUser_IndexHygiene,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -977,3 +983,51 @@ func UpdateUser(
 		})
 	}
 }
+
+func UpdateUser_IndexHygiene(
+	init func(UserFields, *testing.T) (platform.UserService, string, func()),
+	t *testing.T,
+) {
+
+	oldUserName := "user1"
+	users := UserFields{
+		Users: []*platform.User{
+			{
+				ID:     MustIDBase16(userOneID),
+				Name:   oldUserName,
+				Status: "active",
+			},
+		},
+	}
+	s, _, done := init(users, t)
+	defer done()
+
+	newUserName := "user1Updated"
+	upd := platform.UserUpdate{
+		Name: &newUserName,
+	}
+
+	ctx := context.Background()
+	_, err := s.UpdateUser(ctx, MustIDBase16(userOneID), upd)
+	if err != nil {
+		t.Error(err)
+	}
+
+	// Ensure we can find the user with the new name.
+	_, nerr := s.FindUser(ctx, platform.UserFilter{
+		Name: &newUserName,
+	})
+	if nerr != nil {
+		t.Error("unexpected error when finding user by name", nerr)
+	}
+
+	// Ensure we cannot find a user with the old name. The index used when
+	// searching by name should have been cleared out by the UpdateUser
+	// operation.
+	_, oerr := s.FindUser(ctx, platform.UserFilter{
+		Name: &oldUserName,
+	})
+	if !errors.Is(oerr, kv.ErrUserNotFound) {
+		t.Errorf("expected %q error, but got %v", kv.ErrUserNotFound, oerr)
+	}
+}