From ccf55963acb1db8110b4fed6bf2b08d44ad1b934 Mon Sep 17 00:00:00 2001
From: Qiu Jian <qiujian@yunionyun.com>
Date: Wed, 26 Aug 2020 00:35:35 +0800
Subject: [PATCH] fix: add keystone sync user debug info

---
 pkg/keystone/driver/ldap/info.go         | 35 +++++++++++++++++++-----
 pkg/keystone/driver/ldap/sync.go         | 13 +++++++--
 pkg/keystone/models/identity_provider.go |  1 +
 3 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/pkg/keystone/driver/ldap/info.go b/pkg/keystone/driver/ldap/info.go
index cf22ffd6dc..a8c2f31e1a 100644
--- a/pkg/keystone/driver/ldap/info.go
+++ b/pkg/keystone/driver/ldap/info.go
@@ -14,6 +14,17 @@
 
 package ldap
 
+import (
+	"yunion.io/x/pkg/errors"
+)
+
+const (
+	ErrEmptyDN      = errors.Error("empty DN")
+	ErrEmptyId      = errors.Error("empty id")
+	ErrEmptyName    = errors.Error("empty name")
+	ErrDisabledUser = errors.Error("disabled user")
+)
+
 type SDomainInfo struct {
 	DN   string
 	Id   string
@@ -31,17 +42,27 @@ type SGroupInfo struct {
 	Members []string
 }
 
-func (info SDomainInfo) isValid() bool {
-	return len(info.DN) > 0 && len(info.Id) > 0 && len(info.Name) > 0
+func (info SDomainInfo) isValid() error {
+	if len(info.DN) == 0 {
+		return ErrEmptyDN
+	}
+	if len(info.Id) == 0 {
+		return ErrEmptyId
+	}
+	if len(info.Name) == 0 {
+		return ErrEmptyName
+	}
+	return nil
 }
 
-func (info SUserInfo) isValid() bool {
-	if !info.SDomainInfo.isValid() {
-		return false
+func (info SUserInfo) isValid() error {
+	err := info.SDomainInfo.isValid()
+	if err != nil {
+		return err
 	}
 	// regarding disabled LDAP user as invalid
 	if !info.Enabled {
-		return false
+		return ErrDisabledUser
 	}
-	return true
+	return nil
 }
diff --git a/pkg/keystone/driver/ldap/sync.go b/pkg/keystone/driver/ldap/sync.go
index 4f71925902..1ec073f995 100644
--- a/pkg/keystone/driver/ldap/sync.go
+++ b/pkg/keystone/driver/ldap/sync.go
@@ -105,7 +105,9 @@ func (self *SLDAPDriver) syncDomains(ctx context.Context, cli *ldaputils.SLDAPCl
 	domainIds := make([]string, 0)
 	for i := range entries {
 		domainInfo := self.entry2Domain(entries[i])
-		if !domainInfo.isValid() {
+		err := domainInfo.isValid()
+		if err != nil {
+			log.Errorf("invalid domainInfo: %s, skip", err)
 			continue
 		}
 		domain, err := self.syncDomainInfo(ctx, domainInfo)
@@ -179,11 +181,14 @@ func (self *SLDAPDriver) syncUsers(ctx context.Context, cli *ldaputils.SLDAPClie
 	if err != nil {
 		return nil, errors.Wrap(err, "searchLDAP")
 	}
+	log.Debugf("syncUsers: ldapSearch entries: %s", entries)
 	userIds := make([]string, 0)
 	userIdMap := make(map[string]string)
 	for i := range entries {
 		userInfo := self.entry2User(entries[i])
-		if !userInfo.isValid() {
+		err := userInfo.isValid()
+		if err != nil {
+			log.Debugf("userInfo is invalid: %s, skip", err)
 			continue
 		}
 		userId, err := self.syncUserDB(ctx, userInfo, domainId)
@@ -267,7 +272,9 @@ func (self *SLDAPDriver) syncGroups(ctx context.Context, cli *ldaputils.SLDAPCli
 	groupIds := make([]string, 0)
 	for i := range entries {
 		groupInfo := self.entry2Group(entries[i])
-		if !groupInfo.isValid() {
+		err := groupInfo.isValid()
+		if err != nil {
+			log.Errorf("invalid group info: %s, skip", err)
 			continue
 		}
 		groupId, err := self.syncGroupDB(ctx, groupInfo, domainId, userIdMap)
diff --git a/pkg/keystone/models/identity_provider.go b/pkg/keystone/models/identity_provider.go
index 57f93a2caf..2a381005ae 100644
--- a/pkg/keystone/models/identity_provider.go
+++ b/pkg/keystone/models/identity_provider.go
@@ -1042,6 +1042,7 @@ func (self *SIdentityProvider) SyncOrCreateDomain(ctx context.Context, extId str
 }
 
 func (self *SIdentityProvider) SyncOrCreateUser(ctx context.Context, extId string, extName string, domainId string, enableDefault bool, syncUserInfo func(*SUser)) (*SUser, error) {
+	log.Debugf("SyncOrCreateUser extId: %s extName: %s", extId, extName)
 	userId, err := IdmappingManager.RegisterIdMap(ctx, self.Id, extId, api.IdMappingEntityUser)
 	if err != nil {
 		return nil, errors.Wrap(err, "IdmappingManager.RegisterIdMap")