Skip to content

Commit 322e5fc

Browse files
committed
refactor: enhance string handling and performance in LDAP and config components
- Replaced `fmt.Sprintf` with `strconv.Itoa` and `strconv.FormatInt` for integer to string conversions, improving performance and reducing memory allocations. - Utilized `strings.Builder` for efficient DN construction in multiple locations, optimizing string concatenation and memory usage. - Streamlined monitoring metrics by ensuring consistent use of string conversion methods, enhancing readability and performance.
1 parent 2ff7c0d commit 322e5fc

2 files changed

Lines changed: 71 additions & 32 deletions

File tree

v2/pkg/handler/config.go

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net"
77
"regexp"
88
"sort"
9+
"strconv"
910
"strings"
1011
"sync"
1112
"time"
@@ -305,7 +306,7 @@ func (h configHandler) FindPosixAccounts(ctx context.Context, hierarchy string)
305306
}
306307

307308
attrs = append(attrs, &ldap.EntryAttribute{Name: "ou", Values: []string{h.getGroupName(ctx, u.PrimaryGroup)}})
308-
attrs = append(attrs, &ldap.EntryAttribute{Name: "uidNumber", Values: []string{fmt.Sprintf("%d", u.UIDNumber)}})
309+
attrs = append(attrs, &ldap.EntryAttribute{Name: "uidNumber", Values: []string{strconv.Itoa(u.UIDNumber)}})
309310

310311
if u.Disabled {
311312
attrs = append(attrs, &ldap.EntryAttribute{Name: "accountStatus", Values: []string{"inactive"}})
@@ -334,7 +335,7 @@ func (h configHandler) FindPosixAccounts(ctx context.Context, hierarchy string)
334335

335336
attrs = append(attrs, &ldap.EntryAttribute{Name: "description", Values: []string{u.Name}})
336337
attrs = append(attrs, &ldap.EntryAttribute{Name: "gecos", Values: []string{u.Name}})
337-
attrs = append(attrs, &ldap.EntryAttribute{Name: "gidNumber", Values: []string{fmt.Sprintf("%d", u.PrimaryGroup)}})
338+
attrs = append(attrs, &ldap.EntryAttribute{Name: "gidNumber", Values: []string{strconv.Itoa(u.PrimaryGroup)}})
338339
attrs = append(attrs, &ldap.EntryAttribute{Name: "memberOf", Values: h.getGroupDNs(ctx, append(u.OtherGroups, u.PrimaryGroup))})
339340

340341
attrs = append(attrs, &ldap.EntryAttribute{Name: "shadowExpire", Values: []string{"-1"}})
@@ -361,7 +362,7 @@ func (h configHandler) FindPosixAccounts(ctx context.Context, hierarchy string)
361362
case string:
362363
values = append(values, MaybeDecode(typedvalue))
363364
default:
364-
values = append(values, MaybeDecode(fmt.Sprintf("%v", typedvalue)))
365+
values = append(values, MaybeDecode(strconv.FormatInt(int64(typedvalue.(int)), 10)))
365366
}
366367
}
367368

@@ -372,14 +373,24 @@ func (h configHandler) FindPosixAccounts(ctx context.Context, hierarchy string)
372373
}
373374
}
374375

375-
var dn string
376-
377-
if hierarchy == "" {
378-
dn = fmt.Sprintf("%s=%s,%s=%s,%s", h.backend.NameFormatAsArray[0], u.Name, h.backend.GroupFormatAsArray[0], h.getGroupName(ctx, u.PrimaryGroup), h.backend.BaseDN)
379-
} else {
380-
dn = fmt.Sprintf("%s=%s,%s=%s,%s,%s", h.backend.NameFormatAsArray[0], u.Name, h.backend.GroupFormatAsArray[0], h.getGroupName(ctx, u.PrimaryGroup), hierarchy, h.backend.BaseDN)
376+
// Use strings.Builder for efficient DN construction
377+
var dn strings.Builder
378+
dn.WriteString(h.backend.NameFormatAsArray[0])
379+
dn.WriteString("=")
380+
dn.WriteString(u.Name)
381+
dn.WriteString(",")
382+
dn.WriteString(h.backend.GroupFormatAsArray[0])
383+
dn.WriteString("=")
384+
dn.WriteString(h.getGroupName(ctx, u.PrimaryGroup))
385+
386+
if hierarchy != "" {
387+
dn.WriteString(",")
388+
dn.WriteString(hierarchy)
381389
}
382-
entries = append(entries, &ldap.Entry{DN: dn, Attributes: attrs})
390+
391+
dn.WriteString(",")
392+
dn.WriteString(h.backend.BaseDN)
393+
entries = append(entries, &ldap.Entry{DN: dn.String(), Attributes: attrs})
383394
}
384395

385396
return entries, nil
@@ -404,7 +415,7 @@ func (h configHandler) FindPosixGroups(ctx context.Context, hierarchy string) (e
404415
}
405416

406417
attrs = append(attrs, &ldap.EntryAttribute{Name: "description", Values: []string{g.Name}})
407-
attrs = append(attrs, &ldap.EntryAttribute{Name: "gidNumber", Values: []string{fmt.Sprintf("%d", g.GIDNumber)}})
418+
attrs = append(attrs, &ldap.EntryAttribute{Name: "gidNumber", Values: []string{strconv.Itoa(g.GIDNumber)}})
408419
attrs = append(attrs, &ldap.EntryAttribute{Name: "uniqueMember", Values: h.getGroupMemberDNs(ctx, g.GIDNumber)})
409420

410421
if asGroupOfUniqueNames {
@@ -414,8 +425,16 @@ func (h configHandler) FindPosixGroups(ctx context.Context, hierarchy string) (e
414425
attrs = append(attrs, &ldap.EntryAttribute{Name: "objectClass", Values: []string{"posixGroup", "top"}})
415426
}
416427

417-
dn := fmt.Sprintf("%s=%s,%s,%s", h.backend.GroupFormatAsArray[0], g.Name, hierarchy, h.backend.BaseDN)
418-
entries = append(entries, &ldap.Entry{DN: dn, Attributes: attrs})
428+
// Use strings.Builder for efficient DN construction
429+
var dn strings.Builder
430+
dn.WriteString(h.backend.GroupFormatAsArray[0])
431+
dn.WriteString("=")
432+
dn.WriteString(g.Name)
433+
dn.WriteString(",")
434+
dn.WriteString(hierarchy)
435+
dn.WriteString(",")
436+
dn.WriteString(h.backend.BaseDN)
437+
entries = append(entries, &ldap.Entry{DN: dn.String(), Attributes: attrs})
419438
}
420439

421440
return entries, nil
@@ -570,8 +589,14 @@ func (h configHandler) getGroupDNs(ctx context.Context, gids []int) []string {
570589
for _, gid := range gids {
571590
for _, g := range h.cfg.Groups {
572591
if g.GIDNumber == gid {
573-
dn := fmt.Sprintf("%s=%s,ou=groups,%s", h.backend.GroupFormatAsArray[0], g.Name, h.backend.BaseDN)
574-
groups[dn] = true
592+
// Use strings.Builder for efficient DN construction
593+
var dn strings.Builder
594+
dn.WriteString(h.backend.GroupFormatAsArray[0])
595+
dn.WriteString("=")
596+
dn.WriteString(g.Name)
597+
dn.WriteString(",ou=groups,")
598+
dn.WriteString(h.backend.BaseDN)
599+
groups[dn.String()] = true
575600
}
576601

577602
for _, includegroupid := range g.IncludeGroups {

v2/pkg/handler/ldap.go

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,11 @@ func NewLdapHandler(opts ...Option) Handler {
106106
handler.servers = append(handler.servers, l)
107107

108108
// Initialize connection pool for this server
109-
serverKey := fmt.Sprintf("%s:%d", l.Hostname, l.Port)
110-
handler.connPool.connections[serverKey] = make(chan *ldap.Conn, handler.connPool.maxPoolSize)
109+
var serverKey strings.Builder
110+
serverKey.WriteString(l.Hostname)
111+
serverKey.WriteString(":")
112+
serverKey.WriteString(strconv.Itoa(l.Port))
113+
handler.connPool.connections[serverKey.String()] = make(chan *ldap.Conn, handler.connPool.maxPoolSize)
111114
}
112115

113116
// test server connectivity before listening, then keep it updated
@@ -123,7 +126,7 @@ func (h *ldapHandler) Bind(bindDN, bindSimplePw string, conn net.Conn) (result l
123126
start := time.Now()
124127
defer func() {
125128
h.monitor.SetResponseTimeMetric(
126-
map[string]string{"operation": "bind", "status": fmt.Sprintf("%v", result)},
129+
map[string]string{"operation": "bind", "status": strconv.FormatInt(int64(result), 10)},
127130
time.Since(start).Seconds(),
128131
)
129132
}()
@@ -198,8 +201,10 @@ func (h *ldapHandler) Search(boundDN string, searchReq ldap.SearchRequest, conn
198201

199202
start := time.Now()
200203
defer func() {
204+
status := strconv.FormatInt(int64(result.ResultCode), 10)
205+
201206
h.monitor.SetResponseTimeMetric(
202-
map[string]string{"operation": "search", "status": fmt.Sprintf("%v", result.ResultCode)},
207+
map[string]string{"operation": "search", "status": status},
203208
time.Since(start).Seconds(),
204209
)
205210
}()
@@ -387,7 +392,7 @@ func (h *ldapHandler) Add(boundDN string, req ldap.AddRequest, conn net.Conn) (r
387392
start := time.Now()
388393
defer func() {
389394
h.monitor.SetResponseTimeMetric(
390-
map[string]string{"operation": "add", "status": fmt.Sprintf("%v", result)},
395+
map[string]string{"operation": "add", "status": strconv.FormatInt(int64(result), 10)},
391396
time.Since(start).Seconds(),
392397
)
393398
}()
@@ -403,7 +408,7 @@ func (h *ldapHandler) Modify(boundDN string, req ldap.ModifyRequest, conn net.Co
403408

404409
defer func() {
405410
h.monitor.SetResponseTimeMetric(
406-
map[string]string{"operation": "modify", "status": fmt.Sprintf("%v", result)},
411+
map[string]string{"operation": "modify", "status": strconv.FormatInt(int64(result), 10)},
407412
time.Since(start).Seconds(),
408413
)
409414
}()
@@ -420,7 +425,7 @@ func (h *ldapHandler) Delete(boundDN string, deleteDN string, conn net.Conn) (re
420425

421426
defer func() {
422427
h.monitor.SetResponseTimeMetric(
423-
map[string]string{"operation": "delete", "status": fmt.Sprintf("%v", result)},
428+
map[string]string{"operation": "delete", "status": strconv.FormatInt(int64(result), 10)},
424429
time.Since(start).Seconds(),
425430
)
426431
}()
@@ -599,33 +604,39 @@ func (h *ldapHandler) getBestServer() (ldapBackend, error) {
599604
// helper functions
600605
func connID(conn net.Conn) string {
601606
key := conn.LocalAddr().String() + conn.RemoteAddr().String()
602-
return fmt.Sprintf("%x", xxhash.Sum64String(key))
607+
return strconv.FormatUint(xxhash.Sum64String(key), 16)
603608
}
604609

605610
// createConnection creates a new LDAP connection to the specified server
606611
func (h *ldapHandler) createConnection(server ldapBackend) (*ldap.Conn, error) {
607-
dest := fmt.Sprintf("%s:%d", server.Hostname, server.Port)
612+
var dest strings.Builder
613+
dest.WriteString(server.Hostname)
614+
dest.WriteString(":")
615+
dest.WriteString(strconv.Itoa(server.Port))
608616

609617
switch server.Scheme {
610618
case "ldaps":
611619
tlsCfg := &tls.Config{}
612620
if h.backend.Insecure {
613621
tlsCfg.InsecureSkipVerify = true
614622
}
615-
return ldap.DialTLS("tcp", dest, tlsCfg)
623+
return ldap.DialTLS("tcp", dest.String(), tlsCfg)
616624
case "ldap":
617-
return ldap.Dial("tcp", dest)
625+
return ldap.Dial("tcp", dest.String())
618626
default:
619627
return nil, fmt.Errorf("unsupported LDAP scheme: %s", server.Scheme)
620628
}
621629
}
622630

623631
// getConnectionFromPool tries to get a connection from the pool, creates a new one if pool is empty
624632
func (h *ldapHandler) getConnectionFromPool(server ldapBackend) (*ldap.Conn, error) {
625-
serverKey := fmt.Sprintf("%s:%d", server.Hostname, server.Port)
633+
var serverKey strings.Builder
634+
serverKey.WriteString(server.Hostname)
635+
serverKey.WriteString(":")
636+
serverKey.WriteString(strconv.Itoa(server.Port))
626637

627638
h.connPool.mu.RLock()
628-
pool, exists := h.connPool.connections[serverKey]
639+
pool, exists := h.connPool.connections[serverKey.String()]
629640
h.connPool.mu.RUnlock()
630641

631642
if !exists {
@@ -657,10 +668,13 @@ func (h *ldapHandler) returnConnectionToPool(server ldapBackend, conn *ldap.Conn
657668
return
658669
}
659670

660-
serverKey := fmt.Sprintf("%s:%d", server.Hostname, server.Port)
671+
var serverKey strings.Builder
672+
serverKey.WriteString(server.Hostname)
673+
serverKey.WriteString(":")
674+
serverKey.WriteString(strconv.Itoa(server.Port))
661675

662676
h.connPool.mu.RLock()
663-
pool, exists := h.connPool.connections[serverKey]
677+
pool, exists := h.connPool.connections[serverKey.String()]
664678
h.connPool.mu.RUnlock()
665679

666680
if !exists {
@@ -673,11 +687,11 @@ func (h *ldapHandler) returnConnectionToPool(server ldapBackend, conn *ldap.Conn
673687
select {
674688
case pool <- conn:
675689
// Successfully returned to pool
676-
h.log.Debug().Str("server", serverKey).Msg("Connection returned to pool")
690+
h.log.Debug().Str("server", serverKey.String()).Msg("Connection returned to pool")
677691
default:
678692
// Pool is full, close connection
679693
conn.Close()
680-
h.log.Debug().Str("server", serverKey).Msg("Pool full, connection closed")
694+
h.log.Debug().Str("server", serverKey.String()).Msg("Pool full, connection closed")
681695
}
682696
}
683697

0 commit comments

Comments
 (0)