diff --git a/internal/metastore/kv/rootcoord/kv_catalog.go b/internal/metastore/kv/rootcoord/kv_catalog.go index 5c02a239f0..33877b0f09 100644 --- a/internal/metastore/kv/rootcoord/kv_catalog.go +++ b/internal/metastore/kv/rootcoord/kv_catalog.go @@ -541,9 +541,24 @@ func (kc *Catalog) DropPartition(ctx context.Context, dbID int64, collectionID t func (kc *Catalog) DropCredential(ctx context.Context, username string) error { k := fmt.Sprintf("%s/%s", CredentialPrefix, username) - err := kc.Txn.Remove(k) + userResults, err := kc.ListUser(ctx, util.DefaultTenant, &milvuspb.UserEntity{Name: username}, true) + if err != nil && !common.IsKeyNotExistError(err) { + log.Warn("fail to list user", zap.String("key", k), zap.Error(err)) + return err + } + deleteKeys := make([]string, 0, len(userResults)+1) + deleteKeys = append(deleteKeys, k) + for _, userResult := range userResults { + if userResult.User.Name == username { + for _, role := range userResult.Roles { + userRoleKey := funcutil.HandleTenantForEtcdKey(RoleMappingPrefix, util.DefaultTenant, fmt.Sprintf("%s/%s", username, role.Name)) + deleteKeys = append(deleteKeys, userRoleKey) + } + } + } + err = kc.Txn.MultiRemove(deleteKeys) if err != nil { - log.Error("drop credential update meta fail", zap.String("key", k), zap.Error(err)) + log.Warn("fail to drop credential", zap.String("key", k), zap.Error(err)) return err } @@ -736,9 +751,26 @@ func (kc *Catalog) CreateRole(ctx context.Context, tenant string, entity *milvus func (kc *Catalog) DropRole(ctx context.Context, tenant string, roleName string) error { k := funcutil.HandleTenantForEtcdKey(RolePrefix, tenant, roleName) - err := kc.Txn.Remove(k) + roleResults, err := kc.ListRole(ctx, tenant, &milvuspb.RoleEntity{Name: roleName}, true) + if err != nil && !common.IsKeyNotExistError(err) { + log.Warn("fail to list role", zap.String("key", k), zap.Error(err)) + return err + } + + deleteKeys := make([]string, 0, len(roleResults)+1) + deleteKeys = append(deleteKeys, k) + for _, roleResult := range roleResults { + if roleResult.Role.Name == roleName { + for _, userInfo := range roleResult.Users { + userRoleKey := funcutil.HandleTenantForEtcdKey(RoleMappingPrefix, tenant, fmt.Sprintf("%s/%s", userInfo.Name, roleName)) + deleteKeys = append(deleteKeys, userRoleKey) + } + } + } + + err = kc.Txn.MultiRemove(deleteKeys) if err != nil { - log.Error("fail to drop role", zap.String("key", k), zap.Error(err)) + log.Warn("fail to drop role", zap.String("key", k), zap.Error(err)) return err } return nil diff --git a/internal/metastore/kv/rootcoord/kv_catalog_test.go b/internal/metastore/kv/rootcoord/kv_catalog_test.go index 985727d861..3a3fe0ee9b 100644 --- a/internal/metastore/kv/rootcoord/kv_catalog_test.go +++ b/internal/metastore/kv/rootcoord/kv_catalog_test.go @@ -10,10 +10,12 @@ import ( "testing" "github.com/golang/protobuf/proto" + "github.com/milvus-io/milvus/internal/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "go.uber.org/atomic" + "go.uber.org/zap" "golang.org/x/exp/maps" "github.com/milvus-io/milvus-proto/go-api/v2/commonpb" @@ -1236,6 +1238,12 @@ func TestCatalog_DropCollection(t *testing.T) { }) } +func getUserInfoMetaString(username string) string { + validInfo := internalpb.CredentialInfo{Username: username, EncryptedPassword: "pwd" + username} + validBytes, _ := json.Marshal(validInfo) + return string(validBytes) +} + func TestRBAC_Credential(t *testing.T) { ctx := context.TODO() @@ -1345,12 +1353,34 @@ func TestRBAC_Credential(t *testing.T) { kvmock = mocks.NewTxnKV(t) c = &Catalog{Txn: kvmock} - dropFailName = "drop-fail" - dropFailKey = fmt.Sprintf("%s/%s", CredentialPrefix, dropFailName) + validName = "user1" + validUserRoleKeyPrefix = funcutil.HandleTenantForEtcdKey(RoleMappingPrefix, util.DefaultTenant, validName) + + dropFailName = "drop-fail" + dropUserRoleKeyPrefix = funcutil.HandleTenantForEtcdKey(RoleMappingPrefix, util.DefaultTenant, dropFailName) + getFailName = "get-fail" ) - kvmock.EXPECT().Remove(dropFailKey).Return(errors.New("Mock invalid remove")) - kvmock.EXPECT().Remove(mock.Anything).Return(nil) + kvmock.EXPECT().MultiRemove([]string{fmt.Sprintf("%s/%s", CredentialPrefix, dropFailName)}).Return(errors.New("Mock drop fail")) + kvmock.EXPECT().MultiRemove( + []string{ + fmt.Sprintf("%s/%s", CredentialPrefix, validName), + validUserRoleKeyPrefix + "/role1", + validUserRoleKeyPrefix + "/role2", + }, + ).Return(nil) + kvmock.EXPECT().MultiRemove(mock.Anything).Return(errors.New("Mock invalid multi remove")) + + kvmock.EXPECT().Load(fmt.Sprintf("%s/%s", CredentialPrefix, getFailName)).Return("", errors.New("Mock invalid load")) + kvmock.EXPECT().Load(fmt.Sprintf("%s/%s", CredentialPrefix, validName)).Return(getUserInfoMetaString(validName), nil) + kvmock.EXPECT().Load(fmt.Sprintf("%s/%s", CredentialPrefix, dropFailName)).Return(getUserInfoMetaString(dropFailName), nil) + + kvmock.EXPECT().LoadWithPrefix(validUserRoleKeyPrefix).Return( + []string{validUserRoleKeyPrefix + "/role1", validUserRoleKeyPrefix + "/role2"}, + []string{"", ""}, + nil, + ) + kvmock.EXPECT().LoadWithPrefix(dropUserRoleKeyPrefix).Return([]string{}, []string{}, nil) tests := []struct { description string @@ -1358,8 +1388,8 @@ func TestRBAC_Credential(t *testing.T) { user string }{ - {"valid user1", true, "user1"}, - {"valid user2", true, "user2"}, + {"valid user1", true, validName}, + {"invalid user get-fail", false, getFailName}, {"invalid user drop-fail", false, dropFailName}, } @@ -1582,20 +1612,42 @@ func TestRBAC_Role(t *testing.T) { kvmock = mocks.NewTxnKV(t) c = &Catalog{Txn: kvmock} - errorName = "error" - errorPath = funcutil.HandleTenantForEtcdKey(RolePrefix, tenant, errorName) + validName = "role1" + errorName = "error" + getFailName = "get-fail" ) - kvmock.EXPECT().Remove(errorPath).Return(errors.New("mock remove error")).Once() - kvmock.EXPECT().Remove(mock.Anything).Return(nil).Once() + kvmock.EXPECT().MultiRemove([]string{funcutil.HandleTenantForEtcdKey(RolePrefix, tenant, errorName)}).Return(errors.New("remove error")) + kvmock.EXPECT().MultiRemove([]string{ + funcutil.HandleTenantForEtcdKey(RolePrefix, tenant, validName), + funcutil.HandleTenantForEtcdKey(RoleMappingPrefix, tenant, fmt.Sprintf("%s/%s", "user1", validName)), + funcutil.HandleTenantForEtcdKey(RoleMappingPrefix, tenant, fmt.Sprintf("%s/%s", "user2", validName)), + }).Return(nil) + kvmock.EXPECT().MultiRemove(mock.Anything).Run(func(keys []string) { + log.Info("keys", zap.Any("keys", keys)) + }).Return(errors.New("mock multi remove error")) + + getRoleMappingKey := func(username, rolename string) string { + return funcutil.HandleTenantForEtcdKey(RoleMappingPrefix, tenant, fmt.Sprintf("%s/%s", username, rolename)) + } + + kvmock.EXPECT().LoadWithPrefix(funcutil.HandleTenantForEtcdKey(RoleMappingPrefix, tenant, "")).Return( + []string{getRoleMappingKey("user1", validName), getRoleMappingKey("user2", validName), getRoleMappingKey("user3", "role3")}, + []string{}, + nil, + ) + + kvmock.EXPECT().Load(funcutil.HandleTenantForEtcdKey(RolePrefix, tenant, getFailName)).Return("", errors.New("mock load error")) + kvmock.EXPECT().Load(funcutil.HandleTenantForEtcdKey(RolePrefix, tenant, validName)).Return("", nil) + kvmock.EXPECT().Load(funcutil.HandleTenantForEtcdKey(RolePrefix, tenant, errorName)).Return("", nil) tests := []struct { description string isValid bool - - role string + role string }{ - {"valid role role1", true, "role1"}, + {"valid role role1", true, validName}, + {"fail to get role info", false, getFailName}, {"invalid role error", false, errorName}, } diff --git a/internal/proxy/impl.go b/internal/proxy/impl.go index 7b1e1e3e06..4c73b46386 100644 --- a/internal/proxy/impl.go +++ b/internal/proxy/impl.go @@ -4942,9 +4942,10 @@ func (node *Proxy) OperatePrivilege(ctx context.Context, req *milvuspb.OperatePr } curUser, err := GetCurUserFromContext(ctx) if err != nil { + log.Warn("fail to get current user", zap.Error(err)) return &commonpb.Status{ ErrorCode: commonpb.ErrorCode_UnexpectedError, - Reason: err.Error(), + Reason: "fail to get current user, please make sure the authorizationEnabled setting in the milvus.yaml is true", }, nil } req.Entity.Grantor.User = &milvuspb.UserEntity{Name: curUser} diff --git a/internal/proxy/meta_cache.go b/internal/proxy/meta_cache.go index 90647611d5..7e8c6dd0d0 100644 --- a/internal/proxy/meta_cache.go +++ b/internal/proxy/meta_cache.go @@ -882,7 +882,10 @@ func (m *MetaCache) expireShardLeaderCache(ctx context.Context) { func (m *MetaCache) InitPolicyInfo(info []string, userRoles []string) { m.mu.Lock() defer m.mu.Unlock() + m.unsafeInitPolicyInfo(info, userRoles) +} +func (m *MetaCache) unsafeInitPolicyInfo(info []string, userRoles []string) { m.privilegeInfos = util.StringSet(info) for _, userRole := range userRoles { user, role, err := funcutil.DecodeUserRoleCache(userRole) @@ -940,6 +943,21 @@ func (m *MetaCache) RefreshPolicyInfo(op typeutil.CacheOp) error { if m.userToRoles[user] != nil { delete(m.userToRoles[user], role) } + case typeutil.CacheDeleteUser: + delete(m.userToRoles, op.OpKey) + case typeutil.CacheDropRole: + for user := range m.userToRoles { + delete(m.userToRoles[user], op.OpKey) + } + case typeutil.CacheRefresh: + resp, err := m.rootCoord.ListPolicy(context.Background(), &internalpb.ListPolicyRequest{}) + if err != nil { + log.Error("fail to init meta cache", zap.Error(err)) + return err + } + m.userToRoles = make(map[string]map[string]struct{}) + m.privilegeInfos = make(map[string]struct{}) + m.unsafeInitPolicyInfo(resp.PolicyInfos, resp.UserRoles) default: return fmt.Errorf("invalid opType, op_type: %d, op_key: %s", int(op.OpType), op.OpKey) } diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index 86d5fbceae..9ed1a4db26 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "math/rand" "net" "os" "strconv" @@ -30,13 +31,6 @@ import ( "github.com/golang/protobuf/proto" grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" ot "github.com/grpc-ecosystem/go-grpc-middleware/tracing/opentracing" - "github.com/prometheus/client_golang/prometheus" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "go.uber.org/zap" - "google.golang.org/grpc" - "google.golang.org/grpc/keepalive" - "github.com/milvus-io/milvus-proto/go-api/v2/commonpb" "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" "github.com/milvus-io/milvus-proto/go-api/v2/schemapb" @@ -78,6 +72,12 @@ import ( "github.com/milvus-io/milvus/internal/util/sessionutil" "github.com/milvus-io/milvus/internal/util/trace" "github.com/milvus-io/milvus/internal/util/typeutil" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "go.uber.org/zap" + "google.golang.org/grpc" + "google.golang.org/grpc/keepalive" ) const ( @@ -2196,7 +2196,6 @@ func TestProxy(t *testing.T) { updateCredentialReq.NewPassword = crypto.Base64Encode(newPassword) updateResp, err = proxy.UpdateCredential(rootCtx, updateCredentialReq) assert.NoError(t, err) - fmt.Println("simfg fubang:", updateResp) assert.Equal(t, commonpb.ErrorCode_Success, updateResp.ErrorCode) }) @@ -2220,7 +2219,8 @@ func TestProxy(t *testing.T) { getCredentialReq.Username = "(" getResp, err = rootCoordClient.GetCredential(ctx, getCredentialReq) - assert.Error(t, err) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, getResp.Status.ErrorCode) }) wg.Add(1) @@ -3368,6 +3368,73 @@ func testProxyRole(ctx context.Context, t *testing.T, proxy *Proxy) { assert.Equal(t, commonpb.ErrorCode_Success, opResp.ErrorCode) }) + wg.Add(1) + t.Run("User Role mapping info", func(t *testing.T) { + defer wg.Done() + + ctx := context.Background() + username := fmt.Sprintf("user%d", rand.Int31()) + roleName := fmt.Sprintf("role%d", rand.Int31()) + { + createCredentialResp, err := proxy.CreateCredential(ctx, &milvuspb.CreateCredentialRequest{Username: username, Password: crypto.Base64Encode("userpwd")}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_Success, createCredentialResp.ErrorCode) + createRoleResp, err := proxy.CreateRole(ctx, &milvuspb.CreateRoleRequest{Entity: &milvuspb.RoleEntity{Name: roleName}}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_Success, createRoleResp.ErrorCode) + } + { + resp, err := proxy.OperateUserRole(ctx, &milvuspb.OperateUserRoleRequest{Username: username, RoleName: roleName}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_Success, resp.ErrorCode) + + } + { + resp, err := proxy.OperateUserRole(ctx, &milvuspb.OperateUserRoleRequest{Username: username, RoleName: "admin"}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_Success, resp.ErrorCode) + } + { + selectUserResp, err := proxy.SelectUser(ctx, &milvuspb.SelectUserRequest{User: &milvuspb.UserEntity{Name: username}, IncludeRoleInfo: true}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_Success, selectUserResp.Status.ErrorCode) + assert.Equal(t, 2, len(selectUserResp.Results[0].Roles)) + + selectRoleResp, err := proxy.SelectRole(ctx, &milvuspb.SelectRoleRequest{Role: &milvuspb.RoleEntity{Name: roleName}, IncludeUserInfo: true}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_Success, selectRoleResp.Status.ErrorCode) + assert.Equal(t, 1, len(selectRoleResp.Results[0].Users)) + } + { + resp, err := proxy.DropRole(ctx, &milvuspb.DropRoleRequest{RoleName: roleName}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_Success, resp.ErrorCode) + } + { + selectUserResp, err := proxy.SelectUser(ctx, &milvuspb.SelectUserRequest{User: &milvuspb.UserEntity{Name: username}, IncludeRoleInfo: true}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_Success, selectUserResp.Status.ErrorCode) + assert.Equal(t, 1, len(selectUserResp.Results[0].Roles)) + + selectRoleResp, err := proxy.SelectRole(ctx, &milvuspb.SelectRoleRequest{Role: &milvuspb.RoleEntity{Name: roleName}, IncludeUserInfo: true}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_Success, selectRoleResp.Status.ErrorCode) + assert.Equal(t, 0, len(selectRoleResp.Results)) + } + { + resp, err := proxy.DeleteCredential(ctx, &milvuspb.DeleteCredentialRequest{Username: username}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_Success, resp.ErrorCode) + time.Sleep(time.Second) + } + { + selectUserResp, err := proxy.SelectUser(ctx, &milvuspb.SelectUserRequest{User: &milvuspb.UserEntity{Name: username}, IncludeRoleInfo: true}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_Success, selectUserResp.Status.ErrorCode) + assert.Equal(t, 0, len(selectUserResp.Results)) + } + }) + wg.Wait() } diff --git a/internal/rootcoord/mock_test.go b/internal/rootcoord/mock_test.go index 7e3fe15854..096854ee36 100644 --- a/internal/rootcoord/mock_test.go +++ b/internal/rootcoord/mock_test.go @@ -22,6 +22,8 @@ import ( "math/rand" "os" + "github.com/milvus-io/milvus/internal/proto/internalpb" + "go.uber.org/zap" "github.com/milvus-io/milvus-proto/go-api/v2/commonpb" @@ -72,6 +74,21 @@ type mockMetaTable struct { GetCollectionVirtualChannelsFunc func(colID int64) []string AlterCollectionFunc func(ctx context.Context, oldColl *model.Collection, newColl *model.Collection, ts Timestamp) error RenameCollectionFunc func(ctx context.Context, oldName string, newName string, ts Timestamp) error + AddCredentialFunc func(credInfo *internalpb.CredentialInfo) error + GetCredentialFunc func(username string) (*internalpb.CredentialInfo, error) + DeleteCredentialFunc func(username string) error + AlterCredentialFunc func(credInfo *internalpb.CredentialInfo) error + ListCredentialUsernamesFunc func() (*milvuspb.ListCredUsersResponse, error) + CreateRoleFunc func(tenant string, entity *milvuspb.RoleEntity) error + DropRoleFunc func(tenant string, roleName string) error + OperateUserRoleFunc func(tenant string, userEntity *milvuspb.UserEntity, roleEntity *milvuspb.RoleEntity, operateType milvuspb.OperateUserRoleType) error + SelectRoleFunc func(tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) + SelectUserFunc func(tenant string, entity *milvuspb.UserEntity, includeRoleInfo bool) ([]*milvuspb.UserResult, error) + OperatePrivilegeFunc func(tenant string, entity *milvuspb.GrantEntity, operateType milvuspb.OperatePrivilegeType) error + SelectGrantFunc func(tenant string, entity *milvuspb.GrantEntity) ([]*milvuspb.GrantEntity, error) + DropGrantFunc func(tenant string, role *milvuspb.RoleEntity) error + ListPolicyFunc func(tenant string) ([]string, error) + ListUserRoleFunc func(tenant string) ([]string, error) } func (m mockMetaTable) ListDatabases(ctx context.Context, ts typeutil.Timestamp) ([]*model.Database, error) { @@ -154,6 +171,66 @@ func (m mockMetaTable) GetCollectionVirtualChannels(colID int64) []string { return m.GetCollectionVirtualChannelsFunc(colID) } +func (m mockMetaTable) AddCredential(credInfo *internalpb.CredentialInfo) error { + return m.AddCredentialFunc(credInfo) +} + +func (m mockMetaTable) GetCredential(username string) (*internalpb.CredentialInfo, error) { + return m.GetCredentialFunc(username) +} + +func (m mockMetaTable) DeleteCredential(username string) error { + return m.DeleteCredentialFunc(username) +} + +func (m mockMetaTable) AlterCredential(credInfo *internalpb.CredentialInfo) error { + return m.AlterCredentialFunc(credInfo) +} + +func (m mockMetaTable) ListCredentialUsernames() (*milvuspb.ListCredUsersResponse, error) { + return m.ListCredentialUsernamesFunc() +} + +func (m mockMetaTable) CreateRole(tenant string, entity *milvuspb.RoleEntity) error { + return m.CreateRoleFunc(tenant, entity) +} + +func (m mockMetaTable) DropRole(tenant string, roleName string) error { + return m.DropRoleFunc(tenant, roleName) +} + +func (m mockMetaTable) OperateUserRole(tenant string, userEntity *milvuspb.UserEntity, roleEntity *milvuspb.RoleEntity, operateType milvuspb.OperateUserRoleType) error { + return m.OperateUserRoleFunc(tenant, userEntity, roleEntity, operateType) +} + +func (m mockMetaTable) SelectRole(tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) { + return m.SelectRoleFunc(tenant, entity, includeUserInfo) +} + +func (m mockMetaTable) SelectUser(tenant string, entity *milvuspb.UserEntity, includeRoleInfo bool) ([]*milvuspb.UserResult, error) { + return m.SelectUserFunc(tenant, entity, includeRoleInfo) +} + +func (m mockMetaTable) OperatePrivilege(tenant string, entity *milvuspb.GrantEntity, operateType milvuspb.OperatePrivilegeType) error { + return m.OperatePrivilegeFunc(tenant, entity, operateType) +} + +func (m mockMetaTable) SelectGrant(tenant string, entity *milvuspb.GrantEntity) ([]*milvuspb.GrantEntity, error) { + return m.SelectGrantFunc(tenant, entity) +} + +func (m mockMetaTable) DropGrant(tenant string, role *milvuspb.RoleEntity) error { + return m.DropGrantFunc(tenant, role) +} + +func (m mockMetaTable) ListPolicy(tenant string) ([]string, error) { + return m.ListPolicyFunc(tenant) +} + +func (m mockMetaTable) ListUserRole(tenant string) ([]string, error) { + return m.ListUserRoleFunc(tenant) +} + func newMockMetaTable() *mockMetaTable { return &mockMetaTable{} } @@ -409,6 +486,51 @@ func withInvalidMeta() Opt { meta.DropAliasFunc = func(ctx context.Context, dbName string, alias string, ts Timestamp) error { return errors.New("error mock DropAlias") } + meta.AddCredentialFunc = func(credInfo *internalpb.CredentialInfo) error { + return errors.New("error mock AddCredential") + } + meta.GetCredentialFunc = func(username string) (*internalpb.CredentialInfo, error) { + return nil, errors.New("error mock GetCredential") + } + meta.DeleteCredentialFunc = func(username string) error { + return errors.New("error mock DeleteCredential") + } + meta.AlterCredentialFunc = func(credInfo *internalpb.CredentialInfo) error { + return errors.New("error mock AlterCredential") + } + meta.ListCredentialUsernamesFunc = func() (*milvuspb.ListCredUsersResponse, error) { + return nil, errors.New("error mock ListCredentialUsernames") + } + meta.CreateRoleFunc = func(tenant string, entity *milvuspb.RoleEntity) error { + return errors.New("error mock CreateRole") + } + meta.DropRoleFunc = func(tenant string, roleName string) error { + return errors.New("error mock DropRole") + } + meta.OperateUserRoleFunc = func(tenant string, userEntity *milvuspb.UserEntity, roleEntity *milvuspb.RoleEntity, operateType milvuspb.OperateUserRoleType) error { + return errors.New("error mock OperateUserRole") + } + meta.SelectUserFunc = func(tenant string, entity *milvuspb.UserEntity, includeRoleInfo bool) ([]*milvuspb.UserResult, error) { + return nil, errors.New("error mock SelectUser") + } + meta.SelectRoleFunc = func(tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) { + return nil, errors.New("error mock SelectRole") + } + meta.OperatePrivilegeFunc = func(tenant string, entity *milvuspb.GrantEntity, operateType milvuspb.OperatePrivilegeType) error { + return errors.New("error mock OperatePrivilege") + } + meta.SelectGrantFunc = func(tenant string, entity *milvuspb.GrantEntity) ([]*milvuspb.GrantEntity, error) { + return nil, errors.New("error mock SelectGrant") + } + meta.DropGrantFunc = func(tenant string, role *milvuspb.RoleEntity) error { + return errors.New("error mock DropGrant") + } + meta.ListPolicyFunc = func(tenant string) ([]string, error) { + return nil, errors.New("error mock ListPolicy") + } + meta.ListUserRoleFunc = func(tenant string) ([]string, error) { + return nil, errors.New("error mock ListUserRole") + } return withMeta(meta) } diff --git a/internal/rootcoord/root_coord.go b/internal/rootcoord/root_coord.go index 5a23c16986..d34ce45297 100644 --- a/internal/rootcoord/root_coord.go +++ b/internal/rootcoord/root_coord.go @@ -669,6 +669,18 @@ func (c *Core) startInternal() error { c.startServerLoop() c.UpdateStateCode(commonpb.StateCode_Healthy) + // refresh rbac cache + if err := retry.Do(c.ctx, func() error { + if err := c.proxyClientManager.RefreshPolicyInfoCache(c.ctx, &proxypb.RefreshPolicyInfoCacheRequest{ + OpType: int32(typeutil.CacheRefresh), + }); err != nil { + log.Warn("fail to refresh policy info cache", zap.Error(err)) + return err + } + return nil + }, retry.Attempts(100), retry.Sleep(time.Second)); err != nil { + log.Panic("fail to refresh policy info cache", zap.Error(err)) + } logutil.Logger(c.ctx).Info("rootcoord startup successfully") return nil @@ -2113,7 +2125,7 @@ func (c *Core) GetCredential(ctx context.Context, in *rootcoordpb.GetCredentialR metrics.RootCoordDDLReqCounter.WithLabelValues(method, metrics.FailLabel).Inc() return &rootcoordpb.GetCredentialResponse{ Status: failStatus(commonpb.ErrorCode_GetCredentialFailure, "GetCredential failed: "+err.Error()), - }, err + }, nil } log.Info("GetCredential success", zap.String("role", typeutil.RootCoordRole), zap.String("username", in.Username)) @@ -2171,35 +2183,61 @@ func (c *Core) DeleteCredential(ctx context.Context, in *milvuspb.DeleteCredenti metrics.RootCoordDDLReqCounter.WithLabelValues(method, metrics.TotalLabel).Inc() tr := timerecord.NewTimeRecorder(method) + var status *commonpb.Status + defer func() { + if status.ErrorCode != commonpb.ErrorCode_Success { + metrics.RootCoordDDLReqCounter.WithLabelValues(method, metrics.FailLabel).Inc() + } + }() + ctxLog := log.Ctx(ctx).With(zap.String("role", typeutil.RootCoordRole), zap.String("username", in.Username)) + if code, ok := c.checkHealthy(); !ok { - ret := &commonpb.Status{} - setNotServingStatus(ret, code) - return ret, nil + status = &commonpb.Status{} + setNotServingStatus(status, code) + return status, nil } - // delete data on storage - err := c.meta.DeleteCredential(in.Username) + redoTask := newBaseRedoTask(c.stepExecutor) + redoTask.AddSyncStep(NewSimpleStep("delete credential meta data", func(ctx context.Context) ([]nestedStep, error) { + err := c.meta.DeleteCredential(in.Username) + if err != nil { + ctxLog.Warn("delete credential meta data failed", zap.Error(err)) + } + return nil, err + })) + redoTask.AddAsyncStep(NewSimpleStep("delete credential cache", func(ctx context.Context) ([]nestedStep, error) { + err := c.ExpireCredCache(ctx, in.Username) + if err != nil { + ctxLog.Warn("delete credential cache failed", zap.Error(err)) + } + return nil, err + })) + redoTask.AddAsyncStep(NewSimpleStep("delete user role cache for the user", func(ctx context.Context) ([]nestedStep, error) { + err := c.proxyClientManager.RefreshPolicyInfoCache(ctx, &proxypb.RefreshPolicyInfoCacheRequest{ + OpType: int32(typeutil.CacheDeleteUser), + OpKey: in.Username, + }) + if err != nil { + ctxLog.Warn("delete user role cache failed for the user", zap.Error(err)) + } + return nil, err + })) + + err := redoTask.Execute(ctx) if err != nil { - log.Error("DeleteCredential remove credential failed", zap.String("role", typeutil.RootCoordRole), - zap.String("username", in.Username), zap.Error(err)) - metrics.RootCoordDDLReqCounter.WithLabelValues(method, metrics.FailLabel).Inc() - return failStatus(commonpb.ErrorCode_DeleteCredentialFailure, "DeleteCredential failed: "+err.Error()), err + errMsg := "fail to execute task when deleting the user" + ctxLog.Warn(errMsg, zap.Error(err)) + status = failStatus(commonpb.ErrorCode_DeleteCredentialFailure, errMsg) + return status, nil } - // invalidate proxy's local cache - err = c.ExpireCredCache(ctx, in.Username) - if err != nil { - log.Error("DeleteCredential expire credential cache failed", zap.String("role", typeutil.RootCoordRole), - zap.String("username", in.Username), zap.Error(err)) - metrics.RootCoordDDLReqCounter.WithLabelValues(method, metrics.FailLabel).Inc() - return failStatus(commonpb.ErrorCode_DeleteCredentialFailure, "DeleteCredential failed: "+err.Error()), nil - } - log.Info("DeleteCredential success", zap.String("role", typeutil.RootCoordRole), - zap.String("username", in.Username)) + + ctxLog.Info("DeleteCredential success") metrics.RootCoordDDLReqCounter.WithLabelValues(method, metrics.SuccessLabel).Inc() metrics.RootCoordDDLReqLatency.WithLabelValues(method).Observe(float64(tr.ElapseSpan().Milliseconds())) metrics.RootCoordNumOfCredentials.Dec() - return succStatus(), nil + status = succStatus() + return status, nil } // ListCredUsers list all usernames @@ -2217,11 +2255,11 @@ func (c *Core) ListCredUsers(ctx context.Context, in *milvuspb.ListCredUsersRequ credInfo, err := c.meta.ListCredentialUsernames() if err != nil { log.Error("ListCredUsers query usernames failed", zap.String("role", typeutil.RootCoordRole), - zap.Int64("msgID", in.Base.MsgID), zap.Error(err)) + zap.Any("in", in), zap.Error(err)) metrics.RootCoordDDLReqCounter.WithLabelValues(method, metrics.FailLabel).Inc() return &milvuspb.ListCredUsersResponse{ Status: failStatus(commonpb.ErrorCode_ListCredUsersFailure, "ListCredUsers failed: "+err.Error()), - }, err + }, nil } log.Info("ListCredUsers success", zap.String("role", typeutil.RootCoordRole)) @@ -2277,7 +2315,8 @@ func (c *Core) DropRole(ctx context.Context, in *milvuspb.DropRoleRequest) (*com method := "DropRole" metrics.RootCoordDDLReqCounter.WithLabelValues(method, metrics.TotalLabel).Inc() tr := timerecord.NewTimeRecorder(method) - logger.Debug(method, zap.Any("in", in)) + ctxLog := log.Ctx(ctx).With(zap.String("role", typeutil.RootCoordRole), zap.String("role_name", in.RoleName)) + ctxLog.Debug(method) if code, ok := c.checkHealthy(); !ok { ret := &commonpb.Status{} @@ -2286,7 +2325,7 @@ func (c *Core) DropRole(ctx context.Context, in *milvuspb.DropRoleRequest) (*com } if _, err := c.meta.SelectRole(util.DefaultTenant, &milvuspb.RoleEntity{Name: in.RoleName}, false); err != nil { errMsg := "not found the role, maybe the role isn't existed or internal system error" - log.Error(errMsg, zap.Any("in", in), zap.Error(err)) + ctxLog.Warn(errMsg, zap.Error(err)) return failStatus(commonpb.ErrorCode_DropRoleFailure, errMsg), nil } @@ -2295,42 +2334,36 @@ func (c *Core) DropRole(ctx context.Context, in *milvuspb.DropRoleRequest) (*com }) if len(grantEntities) != 0 { errMsg := "fail to drop the role that it has privileges. Use REVOKE API to revoke privileges" - log.Error(errMsg, zap.Any("in", in), zap.Error(err)) - return failStatus(commonpb.ErrorCode_DropRoleFailure, errMsg), nil - } - roleResults, err := c.meta.SelectRole(util.DefaultTenant, &milvuspb.RoleEntity{Name: in.RoleName}, true) - if err != nil { - errMsg := "fail to find the role by role name, maybe the role isn't existed or internal system error" - log.Error(errMsg, zap.Any("in", in), zap.Error(err)) - return failStatus(commonpb.ErrorCode_DropRoleFailure, errMsg), nil - } - logger.Debug("role to user info", zap.Int("counter", len(roleResults))) - for _, roleResult := range roleResults { - for index, userEntity := range roleResult.Users { - if err = c.meta.OperateUserRole(util.DefaultTenant, - &milvuspb.UserEntity{Name: userEntity.Name}, - &milvuspb.RoleEntity{Name: roleResult.Role.Name}, milvuspb.OperateUserRoleType_RemoveUserFromRole); err != nil { - if common.IsIgnorableError(err) { - continue - } - errMsg := "fail to remove user from role" - log.Error(errMsg, zap.Any("in", in), zap.String("role_name", roleResult.Role.Name), zap.String("username", userEntity.Name), zap.Int("current_index", index), zap.Error(err)) - return failStatus(commonpb.ErrorCode_OperateUserRoleFailure, errMsg), nil - } - } - } - if err = c.meta.DropGrant(util.DefaultTenant, &milvuspb.RoleEntity{Name: in.RoleName}); err != nil { - errMsg := "fail to drop the grant" - log.Error(errMsg, zap.Any("in", in), zap.Error(err)) - return failStatus(commonpb.ErrorCode_DropRoleFailure, errMsg), nil - } - if err = c.meta.DropRole(util.DefaultTenant, in.RoleName); err != nil { - errMsg := "fail to drop the role" - log.Error(errMsg, zap.Any("in", in), zap.Error(err)) + ctxLog.Warn(errMsg, zap.Error(err)) return failStatus(commonpb.ErrorCode_DropRoleFailure, errMsg), nil } - logger.Debug(method+" success", zap.String("role_name", in.RoleName)) + redoTask := newBaseRedoTask(c.stepExecutor) + redoTask.AddSyncStep(NewSimpleStep("drop role meta data", func(ctx context.Context) ([]nestedStep, error) { + err := c.meta.DropRole(util.DefaultTenant, in.RoleName) + if err != nil { + ctxLog.Warn("drop role mata data failed", zap.Error(err)) + } + return nil, err + })) + redoTask.AddAsyncStep(NewSimpleStep("drop role cache", func(ctx context.Context) ([]nestedStep, error) { + err := c.proxyClientManager.RefreshPolicyInfoCache(ctx, &proxypb.RefreshPolicyInfoCacheRequest{ + OpType: int32(typeutil.CacheDropRole), + OpKey: in.RoleName, + }) + if err != nil { + ctxLog.Warn("delete user role cache failed for the role", zap.Error(err)) + } + return nil, err + })) + err = redoTask.Execute(ctx) + if err != nil { + errMsg := "fail to execute task when dropping the role" + ctxLog.Warn(errMsg, zap.Error(err)) + return failStatus(commonpb.ErrorCode_DropRoleFailure, errMsg), nil + } + + ctxLog.Debug(method + " success") metrics.RootCoordDDLReqCounter.WithLabelValues(method, metrics.SuccessLabel).Inc() metrics.RootCoordDDLReqLatency.WithLabelValues(method).Observe(float64(tr.ElapseSpan().Milliseconds())) metrics.RootCoordNumOfRoles.Dec() @@ -2368,17 +2401,16 @@ func (c *Core) OperateUserRole(ctx context.Context, in *milvuspb.OperateUserRole } } - updateCache := true - if err := c.meta.OperateUserRole(util.DefaultTenant, &milvuspb.UserEntity{Name: in.Username}, &milvuspb.RoleEntity{Name: in.RoleName}, in.Type); err != nil { - if !common.IsIgnorableError(err) { - errMsg := "fail to operate user to role" - log.Error(errMsg, zap.Any("in", in), zap.Error(err)) - return failStatus(commonpb.ErrorCode_OperateUserRoleFailure, errMsg), nil + redoTask := newBaseRedoTask(c.stepExecutor) + redoTask.AddSyncStep(NewSimpleStep("operate user role meta data", func(ctx context.Context) ([]nestedStep, error) { + err := c.meta.OperateUserRole(util.DefaultTenant, &milvuspb.UserEntity{Name: in.Username}, &milvuspb.RoleEntity{Name: in.RoleName}, in.Type) + if err != nil && !common.IsIgnorableError(err) { + log.Warn("operate user role mata data failed", zap.Error(err)) + return nil, err } - updateCache = false - } - - if updateCache { + return nil, nil + })) + redoTask.AddAsyncStep(NewSimpleStep("operate user role cache", func(ctx context.Context) ([]nestedStep, error) { var opType int32 switch in.Type { case milvuspb.OperateUserRoleType_AddUserToRole: @@ -2387,17 +2419,23 @@ func (c *Core) OperateUserRole(ctx context.Context, in *milvuspb.OperateUserRole opType = int32(typeutil.CacheRemoveUserFromRole) default: errMsg := "invalid operate type for the OperateUserRole api" - log.Error(errMsg, zap.Any("in", in)) - return failStatus(commonpb.ErrorCode_OperateUserRoleFailure, errMsg), nil + log.Warn(errMsg, zap.Any("in", in)) + return nil, nil } if err := c.proxyClientManager.RefreshPolicyInfoCache(ctx, &proxypb.RefreshPolicyInfoCacheRequest{ OpType: opType, OpKey: funcutil.EncodeUserRoleCache(in.Username, in.RoleName), }); err != nil { - errMsg := "fail to refresh policy info cache" - log.Error(errMsg, zap.Any("in", in), zap.Error(err)) - return failStatus(commonpb.ErrorCode_OperateUserRoleFailure, errMsg), nil + log.Warn("fail to refresh policy info cache", zap.Any("in", in), zap.Error(err)) + return nil, err } + return nil, nil + })) + err := redoTask.Execute(ctx) + if err != nil { + errMsg := "fail to execute task when operate the user and role" + log.Warn(errMsg, zap.Error(err)) + return failStatus(commonpb.ErrorCode_OperateUserRoleFailure, errMsg), nil } logger.Debug(method + " success") @@ -2610,17 +2648,17 @@ func (c *Core) OperatePrivilege(ctx context.Context, in *milvuspb.OperatePrivile if in.Entity.Object.Name == commonpb.ObjectType_Global.String() { in.Entity.ObjectName = util.AnyWord } - updateCache := true - if err := c.meta.OperatePrivilege(util.DefaultTenant, in.Entity, in.Type); err != nil { - if !common.IsIgnorableError(err) { - errMsg := "fail to operate the privilege" - log.Error(errMsg, zap.Any("in", in), zap.Error(err)) - return failStatus(commonpb.ErrorCode_OperatePrivilegeFailure, errMsg), nil - } - updateCache = false - } - if updateCache { + redoTask := newBaseRedoTask(c.stepExecutor) + redoTask.AddSyncStep(NewSimpleStep("operate privilege meta data", func(ctx context.Context) ([]nestedStep, error) { + err := c.meta.OperatePrivilege(util.DefaultTenant, in.Entity, in.Type) + if err != nil && !common.IsIgnorableError(err) { + log.Warn("fail to operate the privilege", zap.Any("in", in), zap.Error(err)) + return nil, err + } + return nil, nil + })) + redoTask.AddAsyncStep(NewSimpleStep("operate privilege cache", func(ctx context.Context) ([]nestedStep, error) { var opType int32 switch in.Type { case milvuspb.OperatePrivilegeType_Grant: @@ -2628,18 +2666,24 @@ func (c *Core) OperatePrivilege(ctx context.Context, in *milvuspb.OperatePrivile case milvuspb.OperatePrivilegeType_Revoke: opType = int32(typeutil.CacheRevokePrivilege) default: - errMsg := "invalid operate type for the OperatePrivilege api" - log.Error(errMsg, zap.Any("in", in)) - return failStatus(commonpb.ErrorCode_OperatePrivilegeFailure, errMsg), nil + log.Warn("invalid operate type for the OperatePrivilege api", zap.Any("in", in)) + return nil, nil } if err := c.proxyClientManager.RefreshPolicyInfoCache(ctx, &proxypb.RefreshPolicyInfoCacheRequest{ OpType: opType, OpKey: funcutil.PolicyForPrivilege(in.Entity.Role.Name, in.Entity.Object.Name, in.Entity.ObjectName, in.Entity.Grantor.Privilege.Name, in.Entity.DbName), }); err != nil { - errMsg := "fail to refresh policy info cache" - log.Error(errMsg, zap.Any("in", in), zap.Error(err)) - return failStatus(commonpb.ErrorCode_OperatePrivilegeFailure, errMsg), nil + log.Warn("fail to refresh policy info cache", zap.Any("in", in), zap.Error(err)) + return nil, err } + return nil, nil + })) + + err := redoTask.Execute(ctx) + if err != nil { + errMsg := "fail to execute task when operating the privilege" + log.Warn(errMsg, zap.Error(err)) + return failStatus(commonpb.ErrorCode_OperatePrivilegeFailure, errMsg), nil } logger.Debug(method + " success") diff --git a/internal/rootcoord/root_coord_test.go b/internal/rootcoord/root_coord_test.go index 327f89afde..d2c6c1c0db 100644 --- a/internal/rootcoord/root_coord_test.go +++ b/internal/rootcoord/root_coord_test.go @@ -1446,6 +1446,36 @@ func TestCore_Rbac(t *testing.T) { // not healthy. c.stateCode.Store(commonpb.StateCode_Abnormal) + { + resp, err := c.CreateCredential(ctx, &internalpb.CredentialInfo{}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_NotReadyServe, resp.ErrorCode) + } + + { + resp, err := c.DeleteCredential(ctx, &milvuspb.DeleteCredentialRequest{}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_NotReadyServe, resp.ErrorCode) + } + + { + resp, err := c.UpdateCredential(ctx, &internalpb.CredentialInfo{}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_NotReadyServe, resp.ErrorCode) + } + + { + resp, err := c.GetCredential(ctx, &rootcoordpb.GetCredentialRequest{}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_NotReadyServe, resp.Status.ErrorCode) + } + + { + resp, err := c.ListCredUsers(ctx, &milvuspb.ListCredUsersRequest{}) + assert.NoError(t, err) + assert.Equal(t, commonpb.ErrorCode_NotReadyServe, resp.Status.ErrorCode) + } + { resp, err := c.CreateRole(ctx, &milvuspb.CreateRoleRequest{}) assert.NoError(t, err) @@ -1727,6 +1757,226 @@ func TestRootCoord_CheckHealth(t *testing.T) { }) } +func TestRootCoord_RBACError(t *testing.T) { + ctx := context.Background() + c := newTestCore(withHealthyCode(), withInvalidMeta()) + t.Run("create credential failed", func(t *testing.T) { + resp, err := c.CreateCredential(ctx, &internalpb.CredentialInfo{Username: "foo", EncryptedPassword: "bar"}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + }) + t.Run("get credential failed", func(t *testing.T) { + resp, err := c.GetCredential(ctx, &rootcoordpb.GetCredentialRequest{Username: "foo"}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + }) + t.Run("update credential failed", func(t *testing.T) { + resp, err := c.UpdateCredential(ctx, &internalpb.CredentialInfo{}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + }) + t.Run("delete credential failed", func(t *testing.T) { + resp, err := c.DeleteCredential(ctx, &milvuspb.DeleteCredentialRequest{Username: "foo"}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + }) + t.Run("list credential failed", func(t *testing.T) { + resp, err := c.ListCredUsers(ctx, &milvuspb.ListCredUsersRequest{}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + }) + t.Run("create role failed", func(t *testing.T) { + resp, err := c.CreateRole(ctx, &milvuspb.CreateRoleRequest{Entity: &milvuspb.RoleEntity{Name: "foo"}}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + }) + t.Run("drop role failed", func(t *testing.T) { + resp, err := c.DropRole(ctx, &milvuspb.DropRoleRequest{RoleName: "foo"}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + + mockMeta := c.meta.(*mockMetaTable) + mockMeta.SelectRoleFunc = func(tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) { + return nil, nil + } + defer func() { + mockMeta.SelectRoleFunc = func(tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) { + return nil, errors.New("mock error") + } + }() + + { + resp, err := c.DropRole(ctx, &milvuspb.DropRoleRequest{RoleName: "foo"}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + } + + mockMeta.SelectGrantFunc = func(tenant string, entity *milvuspb.GrantEntity) ([]*milvuspb.GrantEntity, error) { + return []*milvuspb.GrantEntity{{}}, nil + } + defer func() { + mockMeta.SelectGrantFunc = func(tenant string, entity *milvuspb.GrantEntity) ([]*milvuspb.GrantEntity, error) { + return nil, errors.New("mock error") + } + }() + { + resp, err := c.DropRole(ctx, &milvuspb.DropRoleRequest{RoleName: "foo"}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + } + }) + t.Run("operate user role failed", func(t *testing.T) { + mockMeta := c.meta.(*mockMetaTable) + mockMeta.SelectRoleFunc = func(tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) { + return nil, nil + } + mockMeta.SelectUserFunc = func(tenant string, entity *milvuspb.UserEntity, includeRoleInfo bool) ([]*milvuspb.UserResult, error) { + return nil, nil + } + resp, err := c.OperateUserRole(ctx, &milvuspb.OperateUserRoleRequest{RoleName: "foo", Username: "bar", Type: milvuspb.OperateUserRoleType_AddUserToRole}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + mockMeta.SelectRoleFunc = func(tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) { + return nil, errors.New("mock error") + } + mockMeta.SelectUserFunc = func(tenant string, entity *milvuspb.UserEntity, includeRoleInfo bool) ([]*milvuspb.UserResult, error) { + return nil, errors.New("mock error") + } + }) + t.Run("select role failed", func(t *testing.T) { + { + resp, err := c.SelectRole(ctx, &milvuspb.SelectRoleRequest{Role: &milvuspb.RoleEntity{Name: "foo"}}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + } + { + resp, err := c.SelectRole(ctx, &milvuspb.SelectRoleRequest{}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + } + }) + t.Run("select user failed", func(t *testing.T) { + { + resp, err := c.SelectUser(ctx, &milvuspb.SelectUserRequest{User: &milvuspb.UserEntity{Name: "foo"}}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + } + { + resp, err := c.SelectUser(ctx, &milvuspb.SelectUserRequest{}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + } + }) + t.Run("operate privilege failed", func(t *testing.T) { + { + resp, err := c.OperatePrivilege(ctx, &milvuspb.OperatePrivilegeRequest{Type: milvuspb.OperatePrivilegeType(100)}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + } + { + resp, err := c.OperatePrivilege(ctx, &milvuspb.OperatePrivilegeRequest{Type: milvuspb.OperatePrivilegeType_Grant}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + } + { + resp, err := c.OperatePrivilege(ctx, &milvuspb.OperatePrivilegeRequest{Entity: &milvuspb.GrantEntity{Object: &milvuspb.ObjectEntity{Name: "CollectionErr"}}}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + } + { + resp, err := c.OperatePrivilege(ctx, &milvuspb.OperatePrivilegeRequest{Entity: &milvuspb.GrantEntity{Object: &milvuspb.ObjectEntity{Name: "Collection"}, Role: &milvuspb.RoleEntity{Name: "foo"}}}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + } + + mockMeta := c.meta.(*mockMetaTable) + mockMeta.SelectRoleFunc = func(tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) { + return nil, nil + } + { + resp, err := c.OperatePrivilege(ctx, &milvuspb.OperatePrivilegeRequest{Entity: &milvuspb.GrantEntity{ + Role: &milvuspb.RoleEntity{Name: "foo"}, + Object: &milvuspb.ObjectEntity{Name: "Collection"}, + ObjectName: "col1", + Grantor: &milvuspb.GrantorEntity{ + User: &milvuspb.UserEntity{Name: "root"}, + Privilege: &milvuspb.PrivilegeEntity{Name: "Insert"}, + }, + }, Type: milvuspb.OperatePrivilegeType_Grant}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + } + + mockMeta.SelectUserFunc = func(tenant string, entity *milvuspb.UserEntity, includeRoleInfo bool) ([]*milvuspb.UserResult, error) { + return nil, nil + } + resp, err := c.OperatePrivilege(ctx, &milvuspb.OperatePrivilegeRequest{Entity: &milvuspb.GrantEntity{ + Role: &milvuspb.RoleEntity{Name: "foo"}, + Object: &milvuspb.ObjectEntity{Name: "Collection"}, + ObjectName: "col1", + Grantor: &milvuspb.GrantorEntity{ + User: &milvuspb.UserEntity{Name: "root"}, + Privilege: &milvuspb.PrivilegeEntity{Name: "Insert"}, + }, + }, Type: milvuspb.OperatePrivilegeType_Grant}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.ErrorCode) + mockMeta.SelectRoleFunc = func(tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) { + return nil, errors.New("mock error") + } + mockMeta.SelectUserFunc = func(tenant string, entity *milvuspb.UserEntity, includeRoleInfo bool) ([]*milvuspb.UserResult, error) { + return nil, errors.New("mock error") + } + }) + + t.Run("select grant failed", func(t *testing.T) { + { + resp, err := c.SelectGrant(ctx, &milvuspb.SelectGrantRequest{}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + } + { + resp, err := c.SelectGrant(ctx, &milvuspb.SelectGrantRequest{Entity: &milvuspb.GrantEntity{Role: &milvuspb.RoleEntity{Name: "foo"}}}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + } + mockMeta := c.meta.(*mockMetaTable) + mockMeta.SelectRoleFunc = func(tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) { + return nil, nil + } + { + resp, err := c.SelectGrant(ctx, &milvuspb.SelectGrantRequest{Entity: &milvuspb.GrantEntity{Role: &milvuspb.RoleEntity{Name: "foo"}, Object: &milvuspb.ObjectEntity{Name: "CollectionFoo"}}}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + } + { + resp, err := c.SelectGrant(ctx, &milvuspb.SelectGrantRequest{Entity: &milvuspb.GrantEntity{Role: &milvuspb.RoleEntity{Name: "foo"}, Object: &milvuspb.ObjectEntity{Name: "Collection"}}}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + } + mockMeta.SelectRoleFunc = func(tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) { + return nil, errors.New("mock error") + } + }) + + t.Run("list policy failed", func(t *testing.T) { + resp, err := c.ListPolicy(ctx, &internalpb.ListPolicyRequest{}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + + mockMeta := c.meta.(*mockMetaTable) + mockMeta.ListPolicyFunc = func(tenant string) ([]string, error) { + return []string{}, nil + } + resp, err = c.ListPolicy(ctx, &internalpb.ListPolicyRequest{}) + assert.NoError(t, err) + assert.NotEqual(t, commonpb.ErrorCode_Success, resp.Status.ErrorCode) + mockMeta.ListPolicyFunc = func(tenant string) ([]string, error) { + return []string{}, errors.New("mock error") + } + }) +} + func TestCore_Stop(t *testing.T) { t.Run("abnormal stop before component is ready", func(t *testing.T) { c := &Core{} diff --git a/internal/rootcoord/step.go b/internal/rootcoord/step.go index de64745374..47eb838317 100644 --- a/internal/rootcoord/step.go +++ b/internal/rootcoord/step.go @@ -438,3 +438,29 @@ func (b *confirmGCStep) Desc() string { func (b *confirmGCStep) Weight() stepPriority { return stepPriorityLow } + +type simpleStep struct { + desc string + weight stepPriority + executeFunc func(ctx context.Context) ([]nestedStep, error) +} + +func NewSimpleStep(desc string, executeFunc func(ctx context.Context) ([]nestedStep, error)) nestedStep { + return &simpleStep{ + desc: desc, + weight: stepPriorityNormal, + executeFunc: executeFunc, + } +} + +func (s *simpleStep) Execute(ctx context.Context) ([]nestedStep, error) { + return s.executeFunc(ctx) +} + +func (s *simpleStep) Desc() string { + return s.desc +} + +func (s *simpleStep) Weight() stepPriority { + return s.weight +} diff --git a/internal/util/typeutil/cache.go b/internal/util/typeutil/cache.go index 9ea4c12b2e..0d82addaa4 100644 --- a/internal/util/typeutil/cache.go +++ b/internal/util/typeutil/cache.go @@ -7,6 +7,9 @@ const ( CacheRemoveUserFromRole CacheGrantPrivilege CacheRevokePrivilege + CacheDeleteUser + CacheDropRole + CacheRefresh ) type CacheOp struct {