little-cui commented on code in PR #1355:
URL: 
https://github.com/apache/servicecomb-service-center/pull/1355#discussion_r1017400787


##########
datasource/etcd/role.go:
##########
@@ -156,3 +156,27 @@ func (rm *RbacDAO) UpdateRole(ctx context.Context, name 
string, role *crbac.Role
        opts = append(opts, syncOpts...)
        return etcdadpt.Txn(ctx, opts)
 }
+func (rm *RbacDAO) IsMigrated(ctx context.Context) (bool, error) {
+       exist, err := etcdadpt.Exist(ctx, datasource.IsMigrated)
+       if err != nil {
+               return false, err
+       }
+       if !exist {

Review Comment:
   if取反,提前退出



##########
datasource/common.go:
##########
@@ -26,6 +26,7 @@ const (
        RegistryDomainProject = "default/default"
        RegistryAppID         = "default"
        Provider              = "p"
+       IsMigrated            = "IsMigrated"

Review Comment:
   flag的路径要写明,etcd保存格式为:KEY=/cse-sr/role-migrated,VALUE=bool



##########
server/service/rbac/permission.go:
##########
@@ -59,3 +59,12 @@ func DevPerms() []*rbac.Permission {
        }
        return perm
 }
+
+// ConfigPerms allocate all config resource
+func ConfigPerms() *rbac.Permission {

Review Comment:
   放到role service里私有即可,没有复用的场景



##########
datasource/rbac/role.go:
##########
@@ -38,4 +38,5 @@ type RoleManager interface {
        ListRole(ctx context.Context) ([]*rbac.Role, int64, error)
        DeleteRole(ctx context.Context, name string) (bool, error)
        UpdateRole(ctx context.Context, name string, role *rbac.Role) error
+       IsMigrated(ctx context.Context) (bool, error)

Review Comment:
   直接叫Migrate接口,里面包含了判断是否执行过和roles叠加config资源权限



##########
server/service/rbac/init.go:
##########
@@ -81,3 +81,12 @@ func createBuildInRole(r *rbac.Role) {
        }
        log.Fatal(fmt.Sprintf("create role [%s] failed", r.Name), err)
 }
+
+func migrateOldRole() {
+       _, err := MigrateRole(context.Background())

Review Comment:
   没考虑多个service-center实例并发的问题



##########
datasource/etcd/role.go:
##########
@@ -156,3 +156,27 @@ func (rm *RbacDAO) UpdateRole(ctx context.Context, name 
string, role *crbac.Role
        opts = append(opts, syncOpts...)
        return etcdadpt.Txn(ctx, opts)
 }
+func (rm *RbacDAO) IsMigrated(ctx context.Context) (bool, error) {
+       exist, err := etcdadpt.Exist(ctx, datasource.IsMigrated)
+       if err != nil {
+               return false, err
+       }
+       if !exist {
+               opts := []etcdadpt.OpOptions{
+                       
etcdadpt.OpPut(etcdadpt.WithStrKey(datasource.IsMigrated), 
etcdadpt.WithValue([]byte("true"))),
+               }
+               syncOpts, err := esync.GenCreateOpts(ctx, 
datasource.ResourceRole, datasource.IsMigrated)
+               if err != nil {
+                       log.Error("fail to create sync opts", err)
+                       return false, err
+               }
+               opts = append(opts, syncOpts...)
+               err = etcdadpt.Txn(ctx, opts)

Review Comment:
   用etcdadpt.Put,这里不用考虑sync场景



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to