Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store UGID in group cache to handle group renames at provider's end #647

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions internal/users/cache/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const (
userByIDBucketName = "UserByID"
groupByNameBucketName = "GroupByName"
groupByIDBucketName = "GroupByID"
groupByUGIDBucketName = "GroupByUGID"
userToGroupsBucketName = "UserToGroups"
groupToUsersBucketName = "GroupToUsers"
userToBrokerBucketName = "UserToBroker"
Expand All @@ -32,8 +33,8 @@ var (
allBuckets = [][]byte{
[]byte(userByNameBucketName), []byte(userByIDBucketName),
[]byte(groupByNameBucketName), []byte(groupByIDBucketName),
[]byte(userToGroupsBucketName), []byte(groupToUsersBucketName),
[]byte(userToBrokerBucketName),
[]byte(groupByUGIDBucketName), []byte(userToGroupsBucketName),
[]byte(groupToUsersBucketName), []byte(userToBrokerBucketName),
}
)

Expand Down Expand Up @@ -65,6 +66,7 @@ type UserDB struct {
type GroupDB struct {
Name string
GID uint32
UGID string
Users []string
}

Expand Down
12 changes: 12 additions & 0 deletions internal/users/cache/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ func deleteUserFromGroup(buckets map[string]bucketWithName, uid, gid uint32) err
if err = buckets[groupByNameBucketName].Delete([]byte(group.Name)); err != nil {
panic(fmt.Sprintf("programming error: delete is not allowed in a RO transaction: %v", err))
}
if err = buckets[groupByUGIDBucketName].Delete([]byte(group.UGID)); err != nil {
panic(fmt.Sprintf("programming error: delete is not allowed in a RO transaction: %v", err))
}
return nil
}

Expand Down Expand Up @@ -171,3 +174,12 @@ func deleteOrphanedUser(buckets map[string]bucketWithName, uid uint32) (err erro

return nil
}

// deleteRenamedGroup removes a group record with the given Name from groupByName bucket
func deleteRenamedGroup(buckets map[string]bucketWithName, groupName string) (err error) {
// Delete calls fail if the transaction is read only, so we should panic if this function is called in that context.
if err = buckets[groupByNameBucketName].Delete([]byte(groupName)); err != nil {
panic(fmt.Sprintf("programming error: delete is not allowed in a RO transaction: %v", err))
}
return nil
}
17 changes: 13 additions & 4 deletions internal/users/cache/getgroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import (
type groupDB struct {
Name string
GID uint32
UGID string
}

// NewGroupDB creates a new GroupDB.
func NewGroupDB(name string, gid uint32, members []string) GroupDB {
func NewGroupDB(name string, gid uint32, ugid string, members []string) GroupDB {
return GroupDB{
Name: name,
GID: gid,
UGID: ugid,
Users: members,
}
}
Expand All @@ -31,6 +33,11 @@ func (c *Cache) GroupByName(name string) (GroupDB, error) {
return getGroup(c, groupByNameBucketName, name)
}

// GroupByUGID returns a group matching this ugid or an error if the database is corrupted or no entry was found.
func (c *Cache) GroupByUGID(ugid string) (GroupDB, error) {
return getGroup(c, groupByUGIDBucketName, ugid)
}

// AllGroups returns all groups or an error if the database is corrupted.
func (c *Cache) AllGroups() (all []GroupDB, err error) {
c.mu.RLock()
Expand All @@ -53,7 +60,7 @@ func (c *Cache) AllGroups() (all []GroupDB, err error) {
return err
}

all = append(all, NewGroupDB(g.Name, g.GID, users))
all = append(all, NewGroupDB(g.Name, g.GID, g.UGID, users))
return nil
})
})
Expand All @@ -70,6 +77,7 @@ func (c *Cache) AllGroups() (all []GroupDB, err error) {
func getGroup[K uint32 | string](c *Cache, bucketName string, key K) (GroupDB, error) {
var groupName string
var gid uint32
var ugid string
var users []string

c.mu.RLock()
Expand All @@ -80,14 +88,15 @@ func getGroup[K uint32 | string](c *Cache, bucketName string, key K) (GroupDB, e
return err
}

// Get id and name of the group.
// Get id, name and ugid of the group.
g, err := getFromBucket[groupDB](buckets[bucketName], key)
if err != nil {
return err
}

groupName = g.Name
gid = g.GID
ugid = g.UGID

// Get user names in the group.
users, err = getUsersInGroup(buckets, gid)
Expand All @@ -102,7 +111,7 @@ func getGroup[K uint32 | string](c *Cache, bucketName string, key K) (GroupDB, e
return GroupDB{}, err
}

return NewGroupDB(groupName, gid, users), nil
return NewGroupDB(groupName, gid, ugid, users), nil
}

// usersInGroup returns all user names in a given group. It returns an error if the database is corrupted.
Expand Down
19 changes: 14 additions & 5 deletions internal/users/cache/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func updateUser(buckets map[string]bucketWithName, userContent userDB) error {
return nil
}

// updateUser updates both group buckets with groupContent.
// updateUser updates all group buckets with groupContent.
func updateGroups(buckets map[string]bucketWithName, groupContents []GroupDB) error {
for _, groupContent := range groupContents {
existingGroup, err := getFromBucket[groupDB](buckets[groupByIDBucketName], groupContent.GID)
Expand All @@ -92,14 +92,23 @@ func updateGroups(buckets map[string]bucketWithName, groupContents []GroupDB) er
}

// If a group with the same GID exists, we need to ensure that it's the same group or fail the update otherwise.
if existingGroup.Name != "" && existingGroup.Name != groupContent.Name {
log.Errorf(context.TODO(), "GID %d for group %q already in use by group %q", groupContent.GID, groupContent.Name, existingGroup.Name)
if existingGroup.UGID != "" && existingGroup.UGID != groupContent.UGID {
log.Errorf(context.TODO(), "GID %d for group with UGID %q already in use by a group with UGID %q", groupContent.GID, groupContent.UGID, existingGroup.UGID)
return fmt.Errorf("GID for group %q already in use by a different group", groupContent.Name)
}

// Same UID and UGID but a different Name can happen due to group renaming at provider's end.
if existingGroup.Name != "" && existingGroup.Name != groupContent.Name {
// The record being pointed by the existing group name in the groupByName bucket should be deleted.
if err := deleteRenamedGroup(buckets, existingGroup.Name); err != nil {
return err
}
}

// Update group buckets
updateBucket(buckets[groupByIDBucketName], groupContent.GID, groupDB{Name: groupContent.Name, GID: groupContent.GID})
updateBucket(buckets[groupByNameBucketName], groupContent.Name, groupDB{Name: groupContent.Name, GID: groupContent.GID})
updateBucket(buckets[groupByIDBucketName], groupContent.GID, groupDB{Name: groupContent.Name, GID: groupContent.GID, UGID: groupContent.UGID})
updateBucket(buckets[groupByNameBucketName], groupContent.Name, groupDB{Name: groupContent.Name, GID: groupContent.GID, UGID: groupContent.UGID})
updateBucket(buckets[groupByUGIDBucketName], groupContent.UGID, groupDB{Name: groupContent.Name, GID: groupContent.GID, UGID: groupContent.UGID})
}

return nil
Expand Down
6 changes: 4 additions & 2 deletions internal/users/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ func (m *Manager) UpdateUser(u UserInfo) (err error) {
}

// Check if the group already exists in the database
oldGroup, err := m.cache.GroupByName(g.Name)
// We search by UGID because this is a non-local group
// and it should have a unique UGID
oldGroup, err := m.cache.GroupByUGID(g.UGID)
if err != nil && !errors.Is(err, cache.NoDataFoundError{}) {
return err
}
Expand All @@ -132,7 +134,7 @@ func (m *Manager) UpdateUser(u UserInfo) (err error) {
localGroups = append(localGroups, g.Name)
continue
}
groupContents = append(groupContents, cache.NewGroupDB(g.Name, *g.GID, nil))
groupContents = append(groupContents, cache.NewGroupDB(g.Name, *g.GID, g.UGID, nil))
}

// Update user information in the cache.
Expand Down
Loading