Skip to content

Commit

Permalink
Improve the rbac log when milvus is starting (milvus-io#23884)
Browse files Browse the repository at this point in the history
Signed-off-by: SimFG <[email protected]>
  • Loading branch information
SimFG authored May 8, 2023
1 parent ed81eaa commit 4cb65c5
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 12 deletions.
17 changes: 14 additions & 3 deletions internal/metastore/kv/rootcoord/kv_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,11 @@ func (kc *Catalog) GetCredential(ctx context.Context, username string) (*model.C
k := fmt.Sprintf("%s/%s", CredentialPrefix, username)
v, err := kc.Txn.Load(k)
if err != nil {
log.Warn("get credential meta fail", zap.String("key", k), zap.Error(err))
if common.IsKeyNotExistError(err) {
log.Debug("not found the user", zap.String("key", k))
} else {
log.Warn("get credential meta fail", zap.String("key", k), zap.Error(err))
}
return nil, err
}

Expand Down Expand Up @@ -609,6 +613,7 @@ func (kc *Catalog) save(k string) error {
return err
}
if err == nil {
log.Debug("the key has existed", zap.String("key", k))
return common.NewIgnorableError(fmt.Errorf("the key[%s] is existed", k))
}
return kc.Txn.Save(k, "")
Expand All @@ -628,7 +633,7 @@ func (kc *Catalog) remove(k string) error {
func (kc *Catalog) CreateRole(ctx context.Context, tenant string, entity *milvuspb.RoleEntity) error {
k := funcutil.HandleTenantForEtcdKey(RolePrefix, tenant, entity.Name)
err := kc.save(k)
if err != nil {
if err != nil && !common.IsIgnorableError(err) {
log.Warn("fail to save the role", zap.String("key", k), zap.Error(err))
}
return err
Expand Down Expand Up @@ -818,14 +823,18 @@ func (kc *Catalog) AlterGrant(ctx context.Context, tenant string, entity *milvus

v, err = kc.Txn.Load(k)
if err != nil {
log.Warn("fail to load grant privilege entity", zap.String("key", k), zap.Any("type", operateType), zap.Error(err))
if common.IsKeyNotExistError(err) {
log.Debug("not found the privilege entity", zap.String("key", k), zap.Any("type", operateType))
}
if funcutil.IsRevoke(operateType) {
if common.IsKeyNotExistError(err) {
return common.NewIgnorableError(fmt.Errorf("the grant[%s] isn't existed", k))
}
log.Warn("fail to load grant privilege entity", zap.String("key", k), zap.Any("type", operateType), zap.Error(err))
return err
}
if !common.IsKeyNotExistError(err) {
log.Warn("fail to load grant privilege entity", zap.String("key", k), zap.Any("type", operateType), zap.Error(err))
return err
}

Expand All @@ -843,8 +852,10 @@ func (kc *Catalog) AlterGrant(ctx context.Context, tenant string, entity *milvus
if err != nil {
log.Warn("fail to load the grantee id", zap.String("key", k), zap.Error(err))
if !common.IsKeyNotExistError(err) {
log.Warn("fail to load the grantee id", zap.String("key", k), zap.Error(err))
return err
}
log.Debug("not found the grantee id", zap.String("key", k))
if funcutil.IsRevoke(operateType) {
return common.NewIgnorableError(fmt.Errorf("the grantee-id[%s] isn't existed", k))
}
Expand Down
12 changes: 3 additions & 9 deletions internal/metastore/kv/rootcoord/kv_catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1477,11 +1477,10 @@ func TestRBAC_Role(t *testing.T) {
name string

expectedError error
ignorable bool
}{
{"key not exists", true, notExistName, nil, false},
{"other error", false, errorName, otherError, false},
{"ignorable error", false, "key1", &common.IgnorableError{}, true},
{"key not exists", true, notExistName, nil},
{"other error", false, errorName, otherError},
{"ignorable error", false, "key1", &common.IgnorableError{}},
}

for _, test := range tests {
Expand All @@ -1494,11 +1493,6 @@ func TestRBAC_Role(t *testing.T) {
} else {
assert.Error(t, err)
}

if test.ignorable {
_, ok := err.(*common.IgnorableError)
assert.True(t, ok)
}
})
}
})
Expand Down

0 comments on commit 4cb65c5

Please sign in to comment.