code refactor (#1786)

* implement LDAP mock client

Signed-off-by: hongming <talonwan@yunify.com>

* update

Signed-off-by: hongming <talonwan@yunify.com>

* update

Signed-off-by: hongming <talonwan@yunify.com>

* resolve conflict

Signed-off-by: hongming <talonwan@yunify.com>

pkg/simple/client/factory.go

@@ -38,11 +38,11 @@ func NewClientSetOptions() *ClientSetOptions {
 		mySQLOptions:        mysql.NewMySQLOptions(),
 		redisOptions:        cache.NewRedisOptions(),
 		kubernetesOptions:   k8s.NewKubernetesOptions(),
-		ldapOptions:         ldap.NewLdapOptions(),
+		ldapOptions:         ldap.NewOptions(),
 		devopsOptions:       jenkins.NewDevopsOptions(),
 		sonarqubeOptions:    sonarqube.NewSonarQubeOptions(),
 		s3Options:           s3.NewS3Options(),
-		openPitrixOptions:   openpitrix.NewOpenPitrixOptions(),
+		openPitrixOptions:   openpitrix.NewOptions(),
 		prometheusOptions:   prometheus.NewPrometheusOptions(),
 		kubesphereOptions:   kubesphere.NewKubeSphereOptions(),
 		elasticSearhOptions: esclient.NewElasticSearchOptions(),
@@ -114,7 +114,7 @@ type ClientSet struct {
 	mySQLClient *mysql.Client
 
 	k8sClient           k8s.Client
-	ldapClient          *ldap.Client
+	ldapClient          ldap.Client
 	devopsClient        *jenkins.Client
 	sonarQubeClient     *sonarqube.Client
 	redisClient         cache.Interface
@@ -242,7 +242,7 @@ func (cs *ClientSet) SonarQube() (*sonarqube.Client, error) {
 	}
 }
 
-func (cs *ClientSet) Ldap() (*ldap.Client, error) {
+func (cs *ClientSet) Ldap() (ldap.Client, error) {
 	var err error
 
 	if cs.csoptions.ldapOptions == nil || cs.csoptions.ldapOptions.Host == "" {
@@ -256,7 +256,7 @@ func (cs *ClientSet) Ldap() (*ldap.Client, error) {
 		defer mutex.Unlock()
 
 		if cs.ldapClient == nil {
-			cs.ldapClient, err = ldap.NewLdapClient(cs.csoptions.ldapOptions, cs.stopCh)
+			cs.ldapClient, err = ldap.NewClient(cs.csoptions.ldapOptions, cs.stopCh)
 			if err != nil {
 				return nil, err
 			}
@@ -311,7 +311,7 @@ func (cs *ClientSet) OpenPitrix() (openpitrix.Client, error) {
 	if cs.openpitrixClient != nil {
 		return cs.openpitrixClient, nil
 	} else {
-		cs.openpitrixClient, err = openpitrix.NewOpenPitrixClient(cs.csoptions.openPitrixOptions)
+		cs.openpitrixClient, err = openpitrix.NewClient(cs.csoptions.openPitrixOptions)
 		if err != nil {
 			return nil, err
 		}

pkg/simple/client/ldap/channel.go

@@ -25,7 +25,7 @@ type channelPool struct {
 // PoolFactory is a function to create new connections.
 type PoolFactory func(string) (ldap.Client, error)
 
-// NewChannelPool returns a new pool based on buffered channels with an initial
+// newChannelPool returns a new pool based on buffered channels with an initial
 // capacity and maximum capacity. Factory is used when initial capacity is
 // greater than zero to fill the pool. A zero initialCap doesn't fill the Pool
 // until a new Get() is called. During a Get(), If there is no new connection
@@ -36,7 +36,7 @@ type PoolFactory func(string) (ldap.Client, error)
 // of the call is one of those passed, most likely you want to set this to something
 // like
 //   []uint8{ldap.LDAPResultTimeLimitExceeded, ldap.ErrorNetwork}
-func NewChannelPool(initialCap, maxCap int, name string, factory PoolFactory, closeAt []uint16) (Pool, error) {
+func newChannelPool(initialCap, maxCap int, name string, factory PoolFactory, closeAt []uint16) (Pool, error) {
 	if initialCap < 0 || maxCap <= 0 || initialCap > maxCap {
 		return nil, errors.New("invalid capacity settings")
 	}
@@ -85,7 +85,7 @@ func (c *channelPool) Get() (*PoolConn, error) {
 		return nil, ErrClosed
 	}
 
-	// wrap our connections with our ldap.PoolClient implementation (wrapConn
+	// wrap our connections with our ldap.Client implementation (wrapConn
 	// method) that puts the connection back to the pool if it's closed.
 	select {
 	case conn := <-conns:

pkg/simple/client/ldap/conn.go

@@ -8,7 +8,7 @@ import (
 	"github.com/go-ldap/ldap"
 )
 
-// PoolConn implements PoolClient to override the Close() method
+// PoolConn implements Client to override the Close() method
 type PoolConn struct {
 	Conn     ldap.Client
 	c        *channelPool

pkg/simple/client/ldap/ldap.go

@@ -20,11 +20,13 @@ package ldap
 import (
 	"fmt"
 	"github.com/go-ldap/ldap"
+	"github.com/golang/mock/gomock"
 	"k8s.io/klog"
 )
 
 type Client interface {
 	NewConn() (ldap.Client, error)
+	Close()
 	GroupSearchBase() string
 	UserSearchBase() string
 }
@@ -35,8 +37,8 @@ type poolClient struct {
 }
 
 // panic if cannot connect to ldap service
-func NewLdapClient(options *Options, stopCh <-chan struct{}) (Client, error) {
-	pool, err := NewChannelPool(8, 64, "kubesphere", func(s string) (ldap.Client, error) {
+func NewClient(options *Options, stopCh <-chan struct{}) (Client, error) {
+	pool, err := newChannelPool(options.InitialCap, options.MaxCap, options.PoolName, func(s string) (ldap.Client, error) {
 		conn, err := ldap.Dial("tcp", options.Host)
 		if err != nil {
 			return nil, err
@@ -46,7 +48,6 @@ func NewLdapClient(options *Options, stopCh <-chan struct{}) (Client, error) {
 
 	if err != nil {
 		klog.Error(err)
-		pool.Close()
 		return nil, err
 	}
 
@@ -57,13 +58,16 @@ func NewLdapClient(options *Options, stopCh <-chan struct{}) (Client, error) {
 
 	go func() {
 		<-stopCh
-		if client.pool != nil {
-			client.pool.Close()
-		}
+		client.Close()
 	}()
 
 	return client, nil
 }
+func (l *poolClient) Close() {
+	if l.pool != nil {
+		l.pool.Close()
+	}
+}
 
 func (l *poolClient) NewConn() (ldap.Client, error) {
 	if l.pool == nil {
@@ -81,7 +85,7 @@ func (l *poolClient) NewConn() (ldap.Client, error) {
 	err = conn.Bind(l.options.ManagerDN, l.options.ManagerPassword)
 	if err != nil {
 		conn.Close()
-		klog.Error(err)
+		klog.Errorln(err)
 		return nil, err
 	}
 	return conn, nil
@@ -94,3 +98,25 @@ func (l *poolClient) GroupSearchBase() string {
 func (l *poolClient) UserSearchBase() string {
 	return l.options.UserSearchBase
 }
+
+func NewMockClient(options *Options, ctrl *gomock.Controller, setup func(client *MockClient)) (Client, error) {
+	pool, err := newChannelPool(options.InitialCap, options.MaxCap, options.PoolName, func(s string) (ldap.Client, error) {
+		conn := newMockClient(ctrl)
+		conn.EXPECT().Bind(gomock.Any(), gomock.Any()).AnyTimes()
+		conn.EXPECT().Close().AnyTimes()
+		setup(conn)
+		return conn, nil
+	}, []uint16{ldap.LDAPResultAdminLimitExceeded, ldap.ErrorNetwork})
+
+	if err != nil {
+		klog.Error(err)
+		return nil, err
+	}
+
+	client := &poolClient{
+		pool:    pool,
+		options: options,
+	}
+
+	return client, nil
+}

pkg/simple/client/ldap/mock.go

@@ -0,0 +1,231 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: github.com/go-ldap/ldap (interfaces: Client)
+
+// Package ldap is a generated GoMock package.
+package ldap
+
+import (
+	tls "crypto/tls"
+	ldap "github.com/go-ldap/ldap"
+	gomock "github.com/golang/mock/gomock"
+	reflect "reflect"
+	time "time"
+)
+
+// MockClient is a mock of Client interface
+type MockClient struct {
+	ctrl     *gomock.Controller
+	recorder *MockClientMockRecorder
+}
+
+// MockClientMockRecorder is the mock recorder for MockClient
+type MockClientMockRecorder struct {
+	mock *MockClient
+}
+
+// newMockClient creates a new mock instance
+func newMockClient(ctrl *gomock.Controller) *MockClient {
+	mock := &MockClient{ctrl: ctrl}
+	mock.recorder = &MockClientMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use
+func (m *MockClient) EXPECT() *MockClientMockRecorder {
+	return m.recorder
+}
+
+// Add mocks base method
+func (m *MockClient) Add(arg0 *ldap.AddRequest) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Add", arg0)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// Add indicates an expected call of Add
+func (mr *MockClientMockRecorder) Add(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*MockClient)(nil).Add), arg0)
+}
+
+// Bind mocks base method
+func (m *MockClient) Bind(arg0, arg1 string) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Bind", arg0, arg1)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// Bind indicates an expected call of Bind
+func (mr *MockClientMockRecorder) Bind(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Bind", reflect.TypeOf((*MockClient)(nil).Bind), arg0, arg1)
+}
+
+// Close mocks base method
+func (m *MockClient) Close() {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "Close")
+}
+
+// Close indicates an expected call of Close
+func (mr *MockClientMockRecorder) Close() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockClient)(nil).Close))
+}
+
+// Compare mocks base method
+func (m *MockClient) Compare(arg0, arg1, arg2 string) (bool, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Compare", arg0, arg1, arg2)
+	ret0, _ := ret[0].(bool)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// Compare indicates an expected call of Compare
+func (mr *MockClientMockRecorder) Compare(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Compare", reflect.TypeOf((*MockClient)(nil).Compare), arg0, arg1, arg2)
+}
+
+// Del mocks base method
+func (m *MockClient) Del(arg0 *ldap.DelRequest) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Del", arg0)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// Del indicates an expected call of Del
+func (mr *MockClientMockRecorder) Del(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Del", reflect.TypeOf((*MockClient)(nil).Del), arg0)
+}
+
+// Modify mocks base method
+func (m *MockClient) Modify(arg0 *ldap.ModifyRequest) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Modify", arg0)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// Modify indicates an expected call of Modify
+func (mr *MockClientMockRecorder) Modify(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Modify", reflect.TypeOf((*MockClient)(nil).Modify), arg0)
+}
+
+// ModifyDN mocks base method
+func (m *MockClient) ModifyDN(arg0 *ldap.ModifyDNRequest) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "ModifyDN", arg0)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// ModifyDN indicates an expected call of ModifyDN
+func (mr *MockClientMockRecorder) ModifyDN(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ModifyDN", reflect.TypeOf((*MockClient)(nil).ModifyDN), arg0)
+}
+
+// PasswordModify mocks base method
+func (m *MockClient) PasswordModify(arg0 *ldap.PasswordModifyRequest) (*ldap.PasswordModifyResult, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "PasswordModify", arg0)
+	ret0, _ := ret[0].(*ldap.PasswordModifyResult)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// PasswordModify indicates an expected call of PasswordModify
+func (mr *MockClientMockRecorder) PasswordModify(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PasswordModify", reflect.TypeOf((*MockClient)(nil).PasswordModify), arg0)
+}
+
+// Search mocks base method
+func (m *MockClient) Search(arg0 *ldap.SearchRequest) (*ldap.SearchResult, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "Search", arg0)
+	ret0, _ := ret[0].(*ldap.SearchResult)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// Search indicates an expected call of Search
+func (mr *MockClientMockRecorder) Search(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Search", reflect.TypeOf((*MockClient)(nil).Search), arg0)
+}
+
+// SearchWithPaging mocks base method
+func (m *MockClient) SearchWithPaging(arg0 *ldap.SearchRequest, arg1 uint32) (*ldap.SearchResult, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "SearchWithPaging", arg0, arg1)
+	ret0, _ := ret[0].(*ldap.SearchResult)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// SearchWithPaging indicates an expected call of SearchWithPaging
+func (mr *MockClientMockRecorder) SearchWithPaging(arg0, arg1 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SearchWithPaging", reflect.TypeOf((*MockClient)(nil).SearchWithPaging), arg0, arg1)
+}
+
+// SetTimeout mocks base method
+func (m *MockClient) SetTimeout(arg0 time.Duration) {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "SetTimeout", arg0)
+}
+
+// SetTimeout indicates an expected call of SetTimeout
+func (mr *MockClientMockRecorder) SetTimeout(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTimeout", reflect.TypeOf((*MockClient)(nil).SetTimeout), arg0)
+}
+
+// SimpleBind mocks base method
+func (m *MockClient) SimpleBind(arg0 *ldap.SimpleBindRequest) (*ldap.SimpleBindResult, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "SimpleBind", arg0)
+	ret0, _ := ret[0].(*ldap.SimpleBindResult)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// SimpleBind indicates an expected call of SimpleBind
+func (mr *MockClientMockRecorder) SimpleBind(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SimpleBind", reflect.TypeOf((*MockClient)(nil).SimpleBind), arg0)
+}
+
+// Start mocks base method
+func (m *MockClient) Start() {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "Start")
+}
+
+// Start indicates an expected call of Start
+func (mr *MockClientMockRecorder) Start() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Start", reflect.TypeOf((*MockClient)(nil).Start))
+}
+
+// StartTLS mocks base method
+func (m *MockClient) StartTLS(arg0 *tls.Config) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "StartTLS", arg0)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// StartTLS indicates an expected call of StartTLS
+func (mr *MockClientMockRecorder) StartTLS(arg0 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StartTLS", reflect.TypeOf((*MockClient)(nil).StartTLS), arg0)
+}

pkg/simple/client/ldap/options.go

@@ -11,11 +11,14 @@ type Options struct {
 	ManagerPassword string `json:"managerPassword,omitempty" yaml:"managerPassword"`
 	UserSearchBase  string `json:"userSearchBase,omitempty" yaml:"userSearchBase"`
 	GroupSearchBase string `json:"groupSearchBase,omitempty" yaml:"groupSearchBase"`
+	InitialCap      int    `json:"initialCap,omitempty" yaml:"initialCap"`
+	MaxCap          int    `json:"maxCap,omitempty" yaml:"maxCap"`
+	PoolName        string `json:"poolName,omitempty" yaml:"poolName"`
 }
 
-// NewLdapOptions return a default option
+// NewOptions return a default option
 // which host field point to nowhere.
-func NewLdapOptions() *Options {
+func NewOptions() *Options {
 	return &Options{
 		Host:            "",
 		ManagerDN:       "cn=admin,dc=example,dc=org",
@@ -25,7 +28,7 @@ func NewLdapOptions() *Options {
 }
 
 func (l *Options) Validate() []error {
-	errors := []error{}
+	var errors []error
 
 	return errors
 }

pkg/simple/client/openpitrix/openpitrix.go

@@ -134,7 +134,7 @@ func newAppManagerClient(endpoint string) (pb.AppManagerClient, error) {
 	return pb.NewAppManagerClient(conn), nil
 }
 
-func NewOpenPitrixClient(options *Options) (Client, error) {
+func NewClient(options *Options) (Client, error) {
 
 	runtimeMangerClient, err := newRuntimeManagerClient(options.RuntimeManagerEndpoint)
 

pkg/simple/client/openpitrix/options.go

@@ -16,7 +16,7 @@ type Options struct {
 	RepoIndexerEndpoint       string `json:"repoIndexerEndpoint,omitempty" yaml:"repoIndexerEndpoint,omitempty"`
 }
 
-func NewOpenPitrixOptions() *Options {
+func NewOptions() *Options {
 	return &Options{}
 }