Skip to content

fix: restore rbac with empty meta panic #39141

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

Merged
merged 1 commit into from
Jan 13, 2025
Merged
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
34 changes: 17 additions & 17 deletions internal/metastore/kv/rootcoord/kv_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -1489,7 +1489,7 @@
log.Ctx(ctx).Warn("failed to restore rbac, try to rollback", zap.Error(err))
// roll back role
for _, role := range needRollbackRole {
err = kc.DropRole(ctx, tenant, role.Name)
err = kc.DropRole(ctx, tenant, role.GetName())
if err != nil {
log.Ctx(ctx).Warn("failed to rollback roles after restore failed", zap.Error(err))
}
Expand All @@ -1505,15 +1505,15 @@

for _, user := range needRollbackUser {
// roll back user
err = kc.DropCredential(ctx, user.User)
err = kc.DropCredential(ctx, user.GetUser())
if err != nil {
log.Ctx(ctx).Warn("failed to rollback users after restore failed", zap.Error(err))
}
}

// roll back privilege group
for _, group := range needRollbackPrivilegeGroups {
err = kc.DropPrivilegeGroup(ctx, group.GroupName)
err = kc.DropPrivilegeGroup(ctx, group.GetGroupName())
if err != nil {
log.Ctx(ctx).Warn("failed to rollback privilege groups after restore failed", zap.Error(err))
}
Expand All @@ -1527,7 +1527,7 @@
return err
}
existRoleMap := lo.SliceToMap(existRoles, func(entity *milvuspb.RoleResult) (string, struct{}) { return entity.GetRole().GetName(), struct{}{} })
for _, role := range meta.Roles {
for _, role := range meta.GetRoles() {
if _, ok := existRoleMap[role.GetName()]; ok {
log.Ctx(ctx).Warn("failed to restore, role already exists", zap.String("role", role.GetName()))
err = errors.Newf("role [%s] already exists", role.GetName())
Expand All @@ -1545,11 +1545,11 @@
if err != nil {
return err
}
existPrivGroupMap := lo.SliceToMap(existPrivGroups, func(entity *milvuspb.PrivilegeGroupInfo) (string, struct{}) { return entity.GroupName, struct{}{} })
for _, group := range meta.PrivilegeGroups {
if _, ok := existPrivGroupMap[group.GroupName]; ok {
log.Ctx(ctx).Warn("failed to restore, privilege group already exists", zap.String("group", group.GroupName))
err = errors.Newf("privilege group [%s] already exists", group.GroupName)
existPrivGroupMap := lo.SliceToMap(existPrivGroups, func(entity *milvuspb.PrivilegeGroupInfo) (string, struct{}) { return entity.GetGroupName(), struct{}{} })
for _, group := range meta.GetPrivilegeGroups() {
if _, ok := existPrivGroupMap[group.GetGroupName()]; ok {
log.Ctx(ctx).Warn("failed to restore, privilege group already exists", zap.String("group", group.GetGroupName()))
err = errors.Newf("privilege group [%s] already exists", group.GetGroupName())

Check warning on line 1552 in internal/metastore/kv/rootcoord/kv_catalog.go

View check run for this annotation

Codecov / codecov/patch

internal/metastore/kv/rootcoord/kv_catalog.go#L1551-L1552

Added lines #L1551 - L1552 were not covered by tests
return err
}
err = kc.SavePrivilegeGroup(ctx, group)
Expand All @@ -1564,9 +1564,9 @@
if err != nil {
return err
}
existPrivGroupMap = lo.SliceToMap(existPrivGroups, func(entity *milvuspb.PrivilegeGroupInfo) (string, struct{}) { return entity.GroupName, struct{}{} })
for _, grant := range meta.Grants {
privName := grant.Grantor.Privilege.Name
existPrivGroupMap = lo.SliceToMap(existPrivGroups, func(entity *milvuspb.PrivilegeGroupInfo) (string, struct{}) { return entity.GetGroupName(), struct{}{} })
for _, grant := range meta.GetGrants() {
privName := grant.GetGrantor().GetPrivilege().GetName()
if util.IsPrivilegeNameDefined(privName) {
grant.Grantor.Privilege.Name = util.PrivilegeNameForMetastore(privName)
} else if _, ok := existPrivGroupMap[privName]; ok {
Expand All @@ -1589,16 +1589,16 @@
return err
}
existUserMap := lo.SliceToMap(existUser, func(entity *milvuspb.UserResult) (string, struct{}) { return entity.GetUser().GetName(), struct{}{} })
for _, user := range meta.Users {
for _, user := range meta.GetUsers() {
if _, ok := existUserMap[user.GetUser()]; ok {
log.Ctx(ctx).Info("failed to restore, user already exists", zap.String("user", user.GetUser()))
err = errors.Newf("user [%s] already exists", user.GetUser())
return err
}
// restore user
err = kc.CreateCredential(ctx, &model.Credential{
Username: user.User,
EncryptedPassword: user.Password,
Username: user.GetUser(),
EncryptedPassword: user.GetPassword(),
})
if err != nil {
return err
Expand All @@ -1607,9 +1607,9 @@

// restore user role mapping
entity := &milvuspb.UserEntity{
Name: user.User,
Name: user.GetUser(),
}
for _, role := range user.Roles {
for _, role := range user.GetRoles() {
err = kc.AlterUserRole(ctx, tenant, entity, role, milvuspb.OperateUserRoleType_AddUserToRole)
if err != nil {
return err
Expand Down
3 changes: 3 additions & 0 deletions internal/proxy/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -5656,6 +5656,9 @@ func (node *Proxy) RestoreRBAC(ctx context.Context, req *milvuspb.RestoreRBACMet
if err := merr.CheckHealthy(node.GetStateCode()); err != nil {
return merr.Status(err), nil
}
if req.RBACMeta == nil {
return merr.Success(), nil
}

result, err := node.rootCoord.RestoreRBAC(ctx, req)
if err != nil {
Expand Down
6 changes: 5 additions & 1 deletion tests/integration/rbac/rbac_backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,12 @@ func (s *RBACBackupTestSuite) TestBackup() {
s.Equal(groupName, backupRBACResp.GetRBACMeta().PrivilegeGroups[0].GroupName)
s.Equal(2, len(backupRBACResp.GetRBACMeta().PrivilegeGroups[0].Privileges))

restoreRBACResp, err := s.Cluster.Proxy.RestoreRBAC(ctx, &milvuspb.RestoreRBACMetaRequest{})
s.NoError(err)
s.True(merr.Ok(restoreRBACResp))

// test restore, expect to failed due to role/user already exist
restoreRBACResp, err := s.Cluster.Proxy.RestoreRBAC(ctx, &milvuspb.RestoreRBACMetaRequest{
restoreRBACResp, err = s.Cluster.Proxy.RestoreRBAC(ctx, &milvuspb.RestoreRBACMetaRequest{
RBACMeta: backupRBACResp.GetRBACMeta(),
})
s.NoError(err)
Expand Down
Loading