fix: getUIDandGID is able to resolve non-existing users and groups

pkg/commands/user.go

@@ -30,7 +30,7 @@ import (
 
 // for testing
 var (
-	Lookup = util.Lookup
+	Lookup = util.LookupUser
 )
 
 type UserCommand struct {

pkg/commands/user_test.go

@@ -115,7 +115,7 @@ func TestUpdateUser(t *testing.T) {
 			}
 			return nil, fmt.Errorf("error while looking up user")
 		}
-		defer func() { Lookup = util.Lookup }()
+		defer func() { Lookup = util.LookupUser }()
 		buildArgs := dockerfile.NewBuildArgs([]string{})
 		err := cmd.ExecuteCommand(cfg, buildArgs)
 		testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedUID, cfg.User)

pkg/util/command_util.go

@@ -23,7 +23,6 @@ import (
 	"os"
 	"os/user"
 	"path/filepath"
-	reflect "reflect"
 	"strconv"
 	"strings"
 
@@ -39,7 +38,8 @@ import (
 
 // for testing
 var (
-	getUIDAndGID = GetUIDAndGIDFromString
+	getIdsFromUsernameAndGroup = getIdsFromUsernameAndGroupImpl
+	getGroupFromStr            = getGroup
 )
 
 const (
@@ -353,7 +353,7 @@ func GetUserGroup(chownStr string, env []string) (int64, int64, error) {
 		return -1, -1, err
 	}
 
-	uid32, gid32, err := getUIDAndGID(chown, true)
+	uid32, gid32, err := GetUIDAndGIDFromString(chown, true)
 	if err != nil {
 		return -1, -1, err
 	}
@@ -372,48 +372,45 @@ func GetUIDAndGIDFromString(userGroupString string, fallbackToUID bool) (uint32,
 		groupStr = userAndGroup[1]
 	}
 
-	if reflect.TypeOf(userStr).String() == "int" {
-		return 0, 0, nil
-	}
-
-	uidStr, gidStr, err := GetUserFromUsername(userStr, groupStr, fallbackToUID)
-	if err != nil {
-		return 0, 0, err
-	}
-
-	// uid and gid need to be fit into uint32
-	uid64, err := strconv.ParseUint(uidStr, 10, 32)
+	// checkif userStr is a valid id
+	uid, err := strconv.ParseUint(userStr, 10, 32)
 	if err != nil {
-		return 0, 0, err
+		return getIdsFromUsernameAndGroup(userStr, groupStr, fallbackToUID)
 	}
 
-	gid64, err := strconv.ParseUint(gidStr, 10, 32)
+	gid, err := strconv.ParseUint(groupStr, 10, 32)
 	if err != nil {
-		return 0, 0, err
+		// check if the group is specified as a string
+		group, err := getGroupFromStr(groupStr)
+		if err != nil && !fallbackToUID {
+			return 0, 0, err
+		}
+		gid, err := strconv.ParseUint(group.Gid, 10, 32)
+		if err != nil && !fallbackToUID {
+			// no fallback and group is not a valid uid (this should never be the case, because then the whole group is malformed)
+			return 0, 0, err
+		} else if err != nil && fallbackToUID {
+			return uint32(uid), uint32(uid), nil
+		}
+		return uint32(uid), uint32(gid), nil
 	}
 
-	return uint32(uid64), uint32(gid64), nil
+	return uint32(uid), uint32(gid), nil
 }
 
-func GetUserFromUsername(userStr string, groupStr string, fallbackToUID bool) (string, string, error) {
+func getIdsFromUsernameAndGroupImpl(userStr string, groupStr string, fallbackToUID bool) (uint32, uint32, error) {
 	// Lookup by username
-	userObj, err := Lookup(userStr)
+	userObj, err := LookupUser(userStr)
 	if err != nil {
-		return "", "", err
+		return 0, 0, err
 	}
 
 	// Same dance with groups
 	var group *user.Group
 	if groupStr != "" {
-		group, err = user.LookupGroup(groupStr)
+		group, err = getGroupFromStr(groupStr)
 		if err != nil {
-			if _, ok := err.(user.UnknownGroupError); !ok {
-				return "", "", err
-			}
-			group, err = user.LookupGroupId(groupStr)
-			if err != nil {
-				return "", "", err
-			}
+			return 0, 0, err
 		}
 	}
 
@@ -426,10 +423,32 @@ func GetUserFromUsername(userStr string, groupStr string, fallbackToUID bool) (s
 		gid = group.Gid
 	}
 
-	return uid, gid, nil
+	uid64, err := strconv.ParseUint(uid, 10, 32)
+	if err != nil {
+		return 0, 0, err
+	}
+	gid64, err := strconv.ParseUint(gid, 10, 32)
+	if err != nil {
+		return 0, 0, err
+	}
+	return uint32(uid64), uint32(gid64), nil
+}
+
+func getGroup(groupStr string) (*user.Group, error) {
+	group, err := user.LookupGroup(groupStr)
+	if err != nil {
+		if _, ok := err.(user.UnknownGroupError); !ok {
+			return nil, err
+		}
+		group, err = user.LookupGroupId(groupStr)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return group, nil
 }
 
-func Lookup(userStr string) (*user.User, error) {
+func LookupUser(userStr string) (*user.User, error) {
 	userObj, err := user.Lookup(userStr)
 	if err != nil {
 		if _, ok := err.(user.UnknownUserError); !ok {

pkg/util/command_util_test.go

@@ -555,28 +555,32 @@ func Test_RemoteUrls(t *testing.T) {
 
 func TestGetUserGroup(t *testing.T) {
 	tests := []struct {
-		description string
-		chown       string
-		env         []string
-		mock        func(string, bool) (uint32, uint32, error)
-		expectedU   int64
-		expectedG   int64
-		shdErr      bool
+		description  string
+		chown        string
+		env          []string
+		mockIDGetter func(userStr string, groupStr string, fallbackToUID bool) (uint32, uint32, error)
+		// needed, in case uid is a valid number, but group is a name
+		mockGroupIDGetter func(groupStr string) (*user.Group, error)
+		expectedU         int64
+		expectedG         int64
+		shdErr            bool
 	}{
 		{
 			description: "non empty chown",
 			chown:       "some:some",
 			env:         []string{},
-			mock:        func(string, bool) (uint32, uint32, error) { return 100, 1000, nil },
-			expectedU:   100,
-			expectedG:   1000,
+			mockIDGetter: func(string, string, bool) (uint32, uint32, error) {
+				return 100, 1000, nil
+			},
+			expectedU: 100,
+			expectedG: 1000,
 		},
 		{
 			description: "non empty chown with env replacement",
 			chown:       "some:$foo",
 			env:         []string{"foo=key"},
-			mock: func(c string, t bool) (uint32, uint32, error) {
-				if c == "some:key" {
+			mockIDGetter: func(userStr string, groupStr string, fallbackToUID bool) (uint32, uint32, error) {
+				if userStr == "some" && groupStr == "key" {
 					return 10, 100, nil
 				}
 				return 0, 0, fmt.Errorf("did not resolve environment variable")
@@ -586,18 +590,48 @@ func TestGetUserGroup(t *testing.T) {
 		},
 		{
 			description: "empty chown string",
-			mock: func(c string, t bool) (uint32, uint32, error) {
+			mockIDGetter: func(string, string, bool) (uint32, uint32, error) {
 				return 0, 0, fmt.Errorf("should not be called")
 			},
 			expectedU: -1,
 			expectedG: -1,
 		},
+		{
+			description: "with uid instead of username",
+			chown:       "1001:1001",
+			env:         []string{},
+			mockIDGetter: func(string, string, bool) (uint32, uint32, error) {
+				return 0, 0, fmt.Errorf("should not be called")
+			},
+			expectedU: 1001,
+			expectedG: 1001,
+		},
+		{
+			description: "uid but group is a name",
+			chown:       "1001:mygroup",
+			env:         []string{},
+			mockIDGetter: func(string, string, bool) (uint32, uint32, error) {
+				return 0, 0, fmt.Errorf("should not be called")
+			},
+			mockGroupIDGetter: func(groupStr string) (*user.Group, error) {
+				// random uid for mygroup since it's a test
+				return &user.Group{Gid: "2000"}, nil
+			},
+			expectedU: 1001,
+			expectedG: 2000,
+			shdErr: false,
+		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.description, func(t *testing.T) {
-			original := getUIDAndGID
-			defer func() { getUIDAndGID = original }()
-			getUIDAndGID = tc.mock
+			originalIDGetter := getIdsFromUsernameAndGroup
+			originalGroupIDGetter := getGroupFromStr
+			defer func() {
+				getIdsFromUsernameAndGroup = originalIDGetter
+				getGroupFromStr = originalGroupIDGetter
+			}()
+			getIdsFromUsernameAndGroup = tc.mockIDGetter
+			getGroupFromStr = tc.mockGroupIDGetter
 			uid, gid, err := GetUserGroup(tc.chown, tc.env)
 			testutil.CheckErrorAndDeepEqual(t, tc.shdErr, err, uid, tc.expectedU)
 			testutil.CheckErrorAndDeepEqual(t, tc.shdErr, err, gid, tc.expectedG)
@@ -686,6 +720,7 @@ func Test_GetUIDAndGIDFromString(t *testing.T) {
 	for _, tt := range testCases {
 		uid, gid, err := GetUIDAndGIDFromString(tt, false)
 		if uid != uint32(expectedU) || gid != uint32(expectedG) || err != nil {
+			t.Logf("Error: %v", err)
 			t.Errorf("Could not correctly decode %s to uid/gid %d:%d. Result: %d:%d", tt, expectedU, expectedG,
 				uid, gid)
 		}

pkg/util/syscall_credentials.go

@@ -30,7 +30,7 @@ func SyscallCredentials(userStr string) (*syscall.Credential, error) {
 		return nil, errors.Wrap(err, "get uid/gid")
 	}
 
-	u, err := Lookup(userStr)
+	u, err := LookupUser(userStr)
 	if err != nil {
 		return nil, errors.Wrap(err, "lookup")
 	}