fix: getUIDandGID is able to resolve non-existing users and groups (#2106)

* fix: getUIDandGID is able to resolve non-existing users and groups A common pattern in dockerfiles is to provide a plain uid and gid number, which doesn't neccesarily exist inside the os. Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * test: add chown dockerfile Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * chore: format Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * chore: add comment Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * tests: fix chown dockerfile Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * refactor: split up getIdsFromUsernameAndGroup func Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * fix: implement raw uid logic for LookupUser Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * test: add dockerfiles for integration test * fix: lookup user error message * test: add dockerfiles for non-existing user testcase * fix: forgot error check * tests: fix syscall credentials test * chore: add debug output for copy command * tests: set specific gid for integration dockerfile * tests: fix syscall credentials test github runner had the exact uid that i was testing on, so the groups were not empty Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * tests: fix test script Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * chore: apply golangci lint checks Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * fix: reset file ownership in createFile if not root owned Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * chore: logrus.Debugf missed format variable Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * chore(test-script): remove go html coverage Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de> * test(k8s): increase wait timeout Signed-off-by: Höhl, Lukas <lukas.hoehl@accso.de>
2026-10-21 17:17:15 +02:00 · 2022-07-12 10:21:37 -04:00
parent 8710ce3311
commit aad03dc285
18 changed files with 542 additions and 150 deletions
@@ -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,7 +590,7 @@ 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,
@@ -595,9 +599,11 @@ func TestGetUserGroup(t *testing.T) {
 	}
 	for _, tc := range tests {
 		t.Run(tc.description, func(t *testing.T) {
-			original := getUIDAndGID
-			defer func() { getUIDAndGID = original }()
-			getUIDAndGID = tc.mock
+			originalIDGetter := getUIDAndGIDFunc
+			defer func() {
+				getUIDAndGIDFunc = originalIDGetter
+			}()
+			getUIDAndGIDFunc = tc.mockIDGetter
 			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)
@@ -661,33 +667,191 @@ func TestResolveEnvironmentReplacementList(t *testing.T) {
 }

 func Test_GetUIDAndGIDFromString(t *testing.T) {
-	currentUser, err := user.Current()
-	if err != nil {
-		t.Fatalf("Cannot get current user: %s", err)
-	}
-	groups, err := currentUser.GroupIds()
-	if err != nil || len(groups) == 0 {
-		t.Fatalf("Cannot get groups for current user: %s", err)
-	}
-	primaryGroupObj, err := user.LookupGroupId(groups[0])
-	if err != nil {
-		t.Fatalf("Could not lookup name of group %s: %s", groups[0], err)
-	}
-	primaryGroup := primaryGroupObj.Name
+	currentUser := testutil.GetCurrentUser(t)

-	testCases := []string{
-		fmt.Sprintf("%s:%s", currentUser.Uid, currentUser.Gid),
-		fmt.Sprintf("%s:%s", currentUser.Username, currentUser.Gid),
-		fmt.Sprintf("%s:%s", currentUser.Uid, primaryGroup),
-		fmt.Sprintf("%s:%s", currentUser.Username, primaryGroup),
+	type args struct {
+		userGroupStr  string
+		fallbackToUID bool
+	}
+
+	type expected struct {
+		userID  uint32
+		groupID uint32
+	}
+
+	currentUserUID, _ := strconv.ParseUint(currentUser.Uid, 10, 32)
+	currentUserGID, _ := strconv.ParseUint(currentUser.Gid, 10, 32)
+	expectedCurrentUser := expected{
+		userID:  uint32(currentUserUID),
+		groupID: uint32(currentUserGID),
+	}
+
+	testCases := []struct {
+		testname string
+		args     args
+		expected expected
+		wantErr  bool
+	}{
+		{
+			testname: "current user uid and gid",
+			args: args{
+				userGroupStr: fmt.Sprintf("%d:%d", currentUserUID, currentUserGID),
+			},
+			expected: expectedCurrentUser,
+		},
+		{
+			testname: "current user username and gid",
+			args: args{
+				userGroupStr: fmt.Sprintf("%s:%d", currentUser.Username, currentUserGID),
+			},
+			expected: expectedCurrentUser,
+		},
+		{
+			testname: "current user username and primary group",
+			args: args{
+				userGroupStr: fmt.Sprintf("%s:%s", currentUser.Username, currentUser.PrimaryGroup),
+			},
+			expected: expectedCurrentUser,
+		},
+		{
+			testname: "current user uid and primary group",
+			args: args{
+				userGroupStr: fmt.Sprintf("%d:%s", currentUserUID, currentUser.PrimaryGroup),
+			},
+			expected: expectedCurrentUser,
+		},
+		{
+			testname: "non-existing valid uid and gid",
+			args: args{
+				userGroupStr: fmt.Sprintf("%d:%d", 1001, 50000),
+			},
+			expected: expected{
+				userID:  1001,
+				groupID: 50000,
+			},
+		},
+		{
+			testname: "uid and existing group",
+			args: args{
+				userGroupStr: fmt.Sprintf("%d:%s", 1001, currentUser.PrimaryGroup),
+			},
+			expected: expected{
+				userID:  1001,
+				groupID: uint32(currentUserGID),
+			},
+		},
+		{
+			testname: "uid and non existing group-name with fallbackToUID",
+			args: args{
+				userGroupStr:  fmt.Sprintf("%d:%s", 1001, "hello-world-group"),
+				fallbackToUID: true,
+			},
+			expected: expected{
+				userID:  1001,
+				groupID: 1001,
+			},
+		},
+		{
+			testname: "uid and non existing group-name",
+			args: args{
+				userGroupStr: fmt.Sprintf("%d:%s", 1001, "hello-world-group"),
+			},
+			wantErr: true,
+		},
+		{
+			testname: "name and non existing gid",
+			args: args{
+				userGroupStr: fmt.Sprintf("%s:%d", currentUser.Username, 50000),
+			},
+			expected: expected{
+				userID:  expectedCurrentUser.userID,
+				groupID: 50000,
+			},
+		},
+		{
+			testname: "only uid and fallback is false",
+			args: args{
+				userGroupStr:  fmt.Sprintf("%d", currentUserUID),
+				fallbackToUID: false,
+			},
+			wantErr: true,
+		},
+		{
+			testname: "only uid and fallback is true",
+			args: args{
+				userGroupStr:  fmt.Sprintf("%d", currentUserUID),
+				fallbackToUID: true,
+			},
+			expected: expected{
+				userID:  expectedCurrentUser.userID,
+				groupID: expectedCurrentUser.userID,
+			},
+		},
+		{
+			testname: "non-existing user without group",
+			args: args{
+				userGroupStr: "helloworlduser",
+			},
+			wantErr: true,
+		},
 	}
-	expectedU, _ := strconv.ParseUint(currentUser.Uid, 10, 32)
-	expectedG, _ := strconv.ParseUint(currentUser.Gid, 10, 32)
 	for _, tt := range testCases {
-		uid, gid, err := GetUIDAndGIDFromString(tt, false)
-		if uid != uint32(expectedU) || gid != uint32(expectedG) || err != nil {
-			t.Errorf("Could not correctly decode %s to uid/gid %d:%d. Result: %d:%d", tt, expectedU, expectedG,
+		uid, gid, err := getUIDAndGIDFromString(tt.args.userGroupStr, tt.args.fallbackToUID)
+		testutil.CheckError(t, tt.wantErr, err)
+		if uid != tt.expected.userID || gid != tt.expected.groupID {
+			t.Errorf("%v failed. Could not correctly decode %s to uid/gid %d:%d. Result: %d:%d",
+				tt.testname,
+				tt.args.userGroupStr,
+				tt.expected.userID, tt.expected.groupID,
 				uid, gid)
 		}
 	}
 }
+
+func TestLookupUser(t *testing.T) {
+	currentUser := testutil.GetCurrentUser(t)
+
+	type args struct {
+		userStr string
+	}
+	tests := []struct {
+		testname string
+		args     args
+		expected *user.User
+		wantErr  bool
+	}{
+		{
+			testname: "non-existing user",
+			args: args{
+				userStr: "foobazbar",
+			},
+			wantErr: true,
+		},
+		{
+			testname: "uid",
+			args: args{
+				userStr: "30000",
+			},
+			expected: &user.User{
+				Uid:     "30000",
+				HomeDir: "/",
+			},
+			wantErr: false,
+		},
+		{
+			testname: "current user",
+			args: args{
+				userStr: currentUser.Username,
+			},
+			expected: currentUser.User,
+			wantErr:  false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.testname, func(t *testing.T) {
+			got, err := LookupUser(tt.args.userStr)
+			testutil.CheckErrorAndDeepEqual(t, tt.wantErr, err, tt.expected, got)
+		})
+	}
+
+}