Ensure user-gids mapper values are sorted for each user (#15)

OSC · Jul 28, 2021 · ed0ffc6 · ed0ffc6
1 parent d3f98ee
commit ed0ffc6
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## Unreleased
+
+* Ensure user-gids mapper values are sorted for each user
+
 ## v0.5.0 / 2021-07-28
 
 * Add user-gids mapper

diff --git a/cmd/k8-ldap-configmap/main_test.go b/cmd/k8-ldap-configmap/main_test.go
@@ -120,7 +120,7 @@ func TestRun(t *testing.T) {
 	}
 	if val, ok := userGIDMap.Data["testuser2"]; !ok {
 		t.Errorf("Configmap is missing testuser2")
-	} else if val != "1000" {
+	} else if val != "1001" {
 		t.Errorf("Configmap value for testuser2 is incorrect")
 	}
 

diff --git a/internal/mapper/mapper.go b/internal/mapper/mapper.go
@@ -15,12 +15,14 @@ package mapper
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
 
 	"github.com/OSC/k8-ldap-configmap/internal/config"
 	"github.com/OSC/k8-ldap-configmap/internal/metrics"
 	"github.com/OSC/k8-ldap-configmap/internal/utils"
 	"github.com/go-kit/kit/log"
+	"github.com/go-kit/kit/log/level"
 	ldap "github.com/go-ldap/ldap/v3"
 )
 
@@ -38,7 +40,7 @@ type Mapper interface {
 
 type Group struct {
 	name string
-	gid  string
+	gid  int
 }
 
 func registerMapper(name string, requiredUser []string, requiredGroup []string, factory func(config *config.Config, logger log.Logger) Mapper) {
@@ -98,7 +100,7 @@ func ParseDN(dn string) string {
 	return name[1]
 }
 
-func GetUserGroups(users *ldap.SearchResult, groups *ldap.SearchResult, config *config.Config) (map[string][]Group, error) {
+func GetUserGroups(users *ldap.SearchResult, groups *ldap.SearchResult, config *config.Config, logger log.Logger) (map[string][]Group, error) {
 	userDNs := make(map[string]string)
 	groupDNs := make(map[string]string)
 	groupToGid := make(map[string]string)
@@ -155,7 +157,12 @@ func GetUserGroups(users *ldap.SearchResult, groups *ldap.SearchResult, config *
 		for _, groupName := range groupNames {
 			group := Group{name: groupName}
 			if gid, ok := groupToGid[groupName]; ok {
-				group.gid = gid
+				gidInt, err := strconv.Atoi(gid)
+				if err != nil {
+					level.Error(logger).Log("msg", "Unable to parse GID to int", "err", err, "group", groupName, "gid", gid)
+					return nil, err
+				}
+				group.gid = gidInt
 			}
 			groups = append(groups, group)
 		}

diff --git a/internal/mapper/user-gids.go b/internal/mapper/user-gids.go
@@ -15,6 +15,8 @@ package mapper
 
 import (
 	"encoding/json"
+	"sort"
+	"strconv"
 
 	"github.com/OSC/k8-ldap-configmap/internal/config"
 	"github.com/go-kit/kit/log"
@@ -48,17 +50,22 @@ func (m UserGIDs) ConfigMapName() string {
 
 func (m UserGIDs) GetData(users *ldap.SearchResult, groups *ldap.SearchResult) (map[string]string, error) {
 	level.Debug(m.logger).Log("msg", "Mapper running")
-	data, err := GetUserGroups(users, groups, m.config)
+	data, err := GetUserGroups(users, groups, m.config, m.logger)
 	if err != nil {
 		return nil, err
 	}
 	userGIDs := make(map[string]string)
 	for user, groups := range data {
-		groupGIDs := []string{}
+		groupGIDs := []int{}
 		for _, group := range groups {
 			groupGIDs = append(groupGIDs, group.gid)
 		}
-		groupGIDsJSON, _ := json.Marshal(groupGIDs)
+		sort.Ints(groupGIDs)
+		groupGIDsStr := make([]string, len(groupGIDs))
+		for i, gid := range groupGIDs {
+			groupGIDsStr[i] = strconv.Itoa(gid)
+		}
+		groupGIDsJSON, _ := json.Marshal(groupGIDsStr)
 		userGIDs[user] = string(groupGIDsJSON)
 	}
 	level.Debug(m.logger).Log("msg", "Mapper complete", "user-gids", len(userGIDs))

diff --git a/internal/mapper/user-gids_test.go b/internal/mapper/user-gids_test.go
@@ -73,7 +73,7 @@ func TestGetUserGIDsDataMember(t *testing.T) {
 	}
 	if val, ok := data["testuser1"]; !ok {
 		t.Errorf("testuser1 not found in data")
-	} else if val != "[\"1000\"]" {
+	} else if val != "[\"1001\"]" {
 		t.Errorf("Unexpected value for testuser1, got:%s", val)
 	}
 	if val, ok := data["testuser2"]; !ok {
@@ -107,7 +107,7 @@ func TestGetUserGIDsDataMemberUID(t *testing.T) {
 	}
 	if val, ok := data["testuser1"]; !ok {
 		t.Errorf("testuser1 not found in data")
-	} else if val != "[\"1000\"]" {
+	} else if val != "[\"1001\"]" {
 		t.Errorf("Unexpected value for testuser1, got:%s", val)
 	}
 	if val, ok := data["testuser2"]; !ok {

diff --git a/internal/mapper/user-groups.go b/internal/mapper/user-groups.go
@@ -48,7 +48,7 @@ func (m UserGroups) ConfigMapName() string {
 
 func (m UserGroups) GetData(users *ldap.SearchResult, groups *ldap.SearchResult) (map[string]string, error) {
 	level.Debug(m.logger).Log("msg", "Mapper running")
-	data, err := GetUserGroups(users, groups, m.config)
+	data, err := GetUserGroups(users, groups, m.config, m.logger)
 	if err != nil {
 		return nil, err
 	}

diff --git a/internal/test/ldapserver.go b/internal/test/ldapserver.go
@@ -16,8 +16,8 @@ package test
 import (
 	"crypto/tls"
 	"fmt"
+	"io"
 	"log"
-	"os"
 
 	"github.com/lor00x/goldap/message"
 	ldap "github.com/vjeantet/ldapserver"
@@ -93,7 +93,8 @@ mfc9r598/by35iVqsCWBf2o3/Q==
 `)
 
 func LdapServer() *ldap.Server {
-	ldap.Logger = log.New(os.Stdout, "[server] ", log.LstdFlags)
+	//ldap.Logger = log.New(os.Stdout, "[server] ", log.LstdFlags)
+	ldap.Logger = log.New(io.Discard, "[server] ", log.LstdFlags)
 	server := ldap.NewServer()
 	routes := ldap.NewRouteMux()
 	//routes.NotFound(handleNotFound)
@@ -144,7 +145,7 @@ func handleSearchGroup(w ldap.ResponseWriter, m *ldap.Message) {
 	data := map[string]map[string][]string{
 		"testgroup1": {
 			"objectClass": []string{"posixGroup"},
-			"gidNumber":   []string{"1000"},
+			"gidNumber":   []string{"1001"},
 			"memberUid":   []string{"testuser1", "testuser3"},
 			"member": []string{
 				fmt.Sprintf("cn=testuser1,%s", UserBaseDN),
@@ -153,7 +154,7 @@ func handleSearchGroup(w ldap.ResponseWriter, m *ldap.Message) {
 		},
 		"testgroup2": {
 			"objectClass": []string{"posixGroup"},
-			"gidNumber":   []string{"1001"},
+			"gidNumber":   []string{"1000"},
 			"memberUid":   []string{"testuser2", "testuser4"},
 			"member": []string{
 				fmt.Sprintf("cn=testuser2,%s", UserBaseDN),
@@ -184,7 +185,7 @@ func handleSearchUser(w ldap.ResponseWriter, m *ldap.Message) {
 		"testuser1": {
 			"objectClass": []string{"posixAccount"},
 			"uidNumber":   []string{"1000"},
-			"gidNumber":   []string{"1000"},
+			"gidNumber":   []string{"1001"},
 			"memberOf": []string{
 				fmt.Sprintf("cn=Testgroup1,%s", GroupBaseDN),
 				fmt.Sprintf("cn=Testgroup2,%s", GroupBaseDN),
@@ -194,23 +195,23 @@ func handleSearchUser(w ldap.ResponseWriter, m *ldap.Message) {
 		"testuser2": {
 			"objectClass": []string{"posixAccount"},
 			"uidNumber":   []string{"1001"},
-			"gidNumber":   []string{"1000"},
+			"gidNumber":   []string{"1001"},
 			"memberOf": []string{
 				fmt.Sprintf("cn=Testgroup2,%s", GroupBaseDN),
 			},
 		},
 		"testuser3": {
 			"objectClass": []string{"posixAccount"},
 			"uidNumber":   []string{"1002"},
-			"gidNumber":   []string{"1001"},
+			"gidNumber":   []string{"1000"},
 			"memberOf": []string{
 				fmt.Sprintf("cn=Testgroup2,%s", GroupBaseDN),
 			},
 		},
 		"testuser4": {
 			"objectClass": []string{"posixAccount"},
 			"uidNumber":   []string{"1003"},
-			"gidNumber":   []string{"1001"},
+			"gidNumber":   []string{"1000"},
 		},
 	}
 	for cn, attrs := range data {