Repository: lima-vm/lima
Pull request #1946 (2023-10-25 09:15:48)

pkg/osutil: refactor LimaUser and LimaGroup functions
This PR refactors the function `osutil.LimaUser` and `osutil.LimaGroup`:

- compile three regexps only once;
- replace `.Match([]byte(...` with `.MatchString(...`;
- remove duplicated regexp for matching username;
- replace regexp for matching uid/gid with the new function `parseUidGid`;
- replace `strconv.ParseUint` with `parseUidGid`;
- add function for converting `uid/gid` to string.


Patch:
diff --git a/.golangci.yml b/.golangci.yml
index 1df721eb..afffb779 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -151,3 +151,8 @@ linters-settings:
     - typeUnparen
     - unnamedResult
     - unnecessaryBlock
+issues:
+  exclude-rules:
+  # Allow using Uid, Gid in pkg/osutil.
+  - path: "pkg/osutil/"
+    text: "uid"
diff --git a/pkg/osutil/osutil_linux.go b/pkg/osutil/osutil_linux.go
index 7e8e2294..817ef663 100644
--- a/pkg/osutil/osutil_linux.go
+++ b/pkg/osutil/osutil_linux.go
@@ -10,7 +10,7 @@ const UnixPathMax = 108
 
 // Stat is a selection of syscall.Stat_t
 type Stat struct {
-	Uid uint32 //nolint:revive
+	Uid uint32
 	Gid uint32
 }
 
diff --git a/pkg/osutil/osutil_others.go b/pkg/osutil/osutil_others.go
index f73a4610..d61e2d7c 100644
--- a/pkg/osutil/osutil_others.go
+++ b/pkg/osutil/osutil_others.go
@@ -13,7 +13,7 @@ const UnixPathMax = 104
 
 // Stat is a selection of syscall.Stat_t
 type Stat struct {
-	Uid uint32 //nolint:revive
+	Uid uint32
 	Gid uint32
 }
 
diff --git a/pkg/osutil/osutil_windows.go b/pkg/osutil/osutil_windows.go
index 768a2042..62bf32fa 100644
--- a/pkg/osutil/osutil_windows.go
+++ b/pkg/osutil/osutil_windows.go
@@ -13,7 +13,7 @@ const UnixPathMax = 108
 
 // Stat is a selection of syscall.Stat_t
 type Stat struct {
-	Uid uint32 //nolint:revive
+	Uid uint32
 	Gid uint32
 }
 
diff --git a/pkg/osutil/user.go b/pkg/osutil/user.go
index 9711d749..1b760d91 100644
--- a/pkg/osutil/user.go
+++ b/pkg/osutil/user.go
@@ -16,7 +16,7 @@ import (
 
 type User struct {
 	User  string
-	Uid   uint32 //nolint:revive
+	Uid   uint32
 	Group string
 	Gid   uint32
 	Home  string
@@ -30,6 +30,14 @@ type Group struct {
 var users map[string]User
 var groups map[string]Group
 
+// regexUsername matches user and group names to be valid for `useradd`.
+// `useradd` allows names with a trailing '$', but it feels prudent to map those
+// names to the fallback user as well, so the regex does not allow them.
+var regexUsername = regexp.MustCompile("^[a-z_][a-z0-9_-]*$")
+
+// regexPath detects valid Linux path.
+var regexPath = regexp.MustCompile("^[/a-zA-Z0-9_-]+$")
+
 func LookupUser(name string) (User, error) {
 	if users == nil {
 		users = make(map[string]User)
@@ -43,15 +51,15 @@ func LookupUser(name string) (User, error) {
 		if err != nil {
 			return User{}, err
 		}
-		uid, err := strconv.ParseUint(u.Uid, 10, 32)
+		uid, err := parseUidGid(u.Uid)
 		if err != nil {
 			return User{}, err
 		}
-		gid, err := strconv.ParseUint(u.Gid, 10, 32)
+		gid, err := parseUidGid(u.Gid)
 		if err != nil {
 			return User{}, err
 		}
-		users[name] = User{User: u.Username, Uid: uint32(uid), Group: g.Name, Gid: uint32(gid), Home: u.HomeDir}
+		users[name] = User{User: u.Username, Uid: uid, Group: g.Name, Gid: gid, Home: u.HomeDir}
 	}
 	return users[name], nil
 }
@@ -65,18 +73,18 @@ func LookupGroup(name string) (Group, error) {
 		if err != nil {
 			return Group{}, err
 		}
-		gid, err := strconv.ParseUint(g.Gid, 10, 32)
+		gid, err := parseUidGid(g.Gid)
 		if err != nil {
 			return Group{}, err
 		}
-		groups[name] = Group{Name: g.Name, Gid: uint32(gid)}
+		groups[name] = Group{Name: g.Name, Gid: gid}
 	}
 	return groups[name], nil
 }
 
 const (
 	fallbackUser = "lima"
-	fallbackUid  = 1000 //nolint:revive
+	fallbackUid  = 1000
 	fallbackGid  = 1000
 )
 
@@ -101,12 +109,9 @@ func LimaUser(warn bool) (*user.User, error) {
 	cache.Do(func() {
 		cache.u, cache.err = user.Current()
 		if cache.err == nil {
-			// `useradd` only allows user and group names matching the following pattern:
-			// (it allows a trailing '$', but it feels prudent to map those to the fallback user as well)
-			validName := "^[a-z_][a-z0-9_-]*$"
-			if !regexp.MustCompile(validName).Match([]byte(cache.u.Username)) {
+			if !regexUsername.MatchString(cache.u.Username) {
 				warning := fmt.Sprintf("local user %q is not a valid Linux username (must match %q); using %q username instead",
-					cache.u.Username, validName, fallbackUser)
+					cache.u.Username, regexUsername.String(), fallbackUser)
 				cache.warnings = append(cache.warnings, warning)
 				cache.u.Username = fallbackUser
 			}
@@ -115,29 +120,29 @@ func LimaUser(warn bool) (*user.User, error) {
 				if err != nil {
 					logrus.Debug(err)
 				}
-				uid, err := strconv.ParseUint(idu, 10, 32)
+				uid, err := parseUidGid(idu)
 				if err != nil {
 					uid = fallbackUid
 				}
-				if !regexp.MustCompile("^[0-9]+$").Match([]byte(cache.u.Uid)) {
+				if _, err := parseUidGid(cache.u.Uid); err != nil {
 					warning := fmt.Sprintf("local uid %q is not a valid Linux uid (must be integer); using %d uid instead",
 						cache.u.Uid, uid)
 					cache.warnings = append(cache.warnings, warning)
-					cache.u.Uid = fmt.Sprintf("%d", uid)
+					cache.u.Uid = formatUidGid(uid)
 				}
 				idg, err := call([]string{"id", "-g"})
 				if err != nil {
 					logrus.Debug(err)
 				}
-				gid, err := strconv.ParseUint(idg, 10, 32)
+				gid, err := parseUidGid(idg)
 				if err != nil {
 					gid = fallbackGid
 				}
-				if !regexp.MustCompile("^[0-9]+$").Match([]byte(cache.u.Gid)) {
+				if _, err := parseUidGid(cache.u.Gid); err != nil {
 					warning := fmt.Sprintf("local gid %q is not a valid Linux gid (must be integer); using %d gid instead",
 						cache.u.Gid, gid)
 					cache.warnings = append(cache.warnings, warning)
-					cache.u.Gid = fmt.Sprintf("%d", gid)
+					cache.u.Gid = formatUidGid(gid)
 				}
 				home, err := call([]string{"cygpath", cache.u.HomeDir})
 				if err != nil {
@@ -150,10 +155,9 @@ func LimaUser(warn bool) (*user.User, error) {
 					prefix := strings.ToLower(fmt.Sprintf("/%c", drive[0]))
 					home = strings.Replace(home, drive, prefix, 1)
 				}
-				validPath := "^[/a-zA-Z0-9_-]+$"
-				if !regexp.MustCompile(validPath).Match([]byte(cache.u.HomeDir)) {
+				if !regexPath.MatchString(cache.u.HomeDir) {
 					warning := fmt.Sprintf("local home %q is not a valid Linux path (must match %q); using %q home instead",
-						cache.u.HomeDir, validPath, home)
+						cache.u.HomeDir, regexPath.String(), home)
 					cache.warnings = append(cache.warnings, warning)
 					cache.u.HomeDir = home
 				}
@@ -167,3 +171,17 @@ func LimaUser(warn bool) (*user.User, error) {
 	}
 	return cache.u, cache.err
 }
+
+// parseUidGid converts string value to Linux uid or gid.
+func parseUidGid(uidOrGid string) (uint32, error) {
+	res, err := strconv.ParseUint(uidOrGid, 10, 32)
+	if err != nil {
+		return 0, err
+	}
+	return uint32(res), nil
+}
+
+// formatUidGid converts uid or gid to string value.
+func formatUidGid(uidOrGid uint32) string {
+	return strconv.FormatUint(uint64(uidOrGid), 10)
+}


Test patch:
diff --git a/pkg/osutil/user_test.go b/pkg/osutil/user_test.go
index 342d5a7c..860f0a71 100644
--- a/pkg/osutil/user_test.go
+++ b/pkg/osutil/user_test.go
@@ -2,7 +2,6 @@ package osutil
 
 import (
 	"path"
-	"regexp"
 	"strconv"
 	"testing"
 
@@ -15,8 +14,7 @@ func TestLimaUserWarn(t *testing.T) {
 }
 
 func validUsername(username string) bool {
-	validName := "^[a-z_][a-z0-9_-]*$"
-	return regexp.MustCompile(validName).Match([]byte(username))
+	return regexUsername.MatchString(username)
 }
 
 func TestLimaUsername(t *testing.T) {