Avoid returning the UID when resolving the GIDs.

[gadiego92]

Fixes #2327

Description

Avoid returning the UID when resolving the GIDs.

It looks like Kaniko @ head by default returns the UID as a GID.
When it tries to resolve the GIDs it cannot find the UID in the group list because indeed it could be it is not a GID.

Reviewer Notes

• The code flow looks good.
• Unit tests and or integration tests added.

[aaron-prindle]

@gadiego92 thanks for the PR here! Currently it seems the unit tests are failing w/ this PR:

full logs:
https://github.com/GoogleContainerTools/kaniko/actions/runs/5913491105/job/16037939866?pr=2689#step:4:58

log snippet:

=== RUN   Test_GetUIDAndGIDFromString
    command_util_test.go:772: only uid and fallback is false failed. Could not correctly decode 1001 to uid/gid 1001:0. Result: 1001:123
    command_util_test.go:772: only uid and fallback is true failed. Could not correctly decode 1001 to uid/gid 1001:1001. Result: 1001:123
--- FAIL: Test_GetUIDAndGIDFromString (0.00s)

[aaron-prindle]

Thank you for your investigation here @gadiego92, adding some of the information you sent me inline into the comment for visibility/context:

We have these two tests under: kaniko/pkg/util/command_util_test.go

{
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,
},

The point is in the first test we expect 1001:1001 but in the second one we want an error.
Is that ok ?
Because in kaniko/pkg/util/command_util.go we have this comment:

// If fallbackToUID is set, the gid is equal to uid if the group is not specified otherwise gid is set to zero.

So under my understanding one of the test is wrong. I see two options:

1. we return 1001:1001 (fallback true) and 1001:0 (fallback false)
2. we return error on both tests (because the fallback is used when the group is not set as the comment states).

In doing my own investigation into docker build behaviour for these cases, this is what I have found:

logs for docker build . failure:

aprindle@aprindle-ssd ~/kaniko-ctx-dir  [main]docker build .
Sending build context to Docker daemon  67.58kB
Step 1/3 : FROM ubuntu
 ---> 01f29b872827
Step 2/3 : USER 1000:invalidgroup
 ---> Using cache
 ---> f6e2cff1e352
Step 3/3 : RUN id
 ---> Running in 47d721f4e3cb
unable to find group invalidgroup: no matching entries in group file

Additionally, it seems the fallbackToUID bool option is not actually toggled in kaniko's code path's, it is always true. It is only in tests where it is actually false:

getUIDAndGIDFromString only called with fallbackToUID true in actual kaniko code paths (excluding tests)

aprindle@aprindle-ssd ~/kaniko  [fix-2327]ack getUIDAndGIDFromString
pkg/util/syscall_credentials.go
29:	uid, gid, err := getUIDAndGIDFromString(userStr, true) # <--- true

pkg/util/command_util_test.go
770:		uid, gid, err := getUIDAndGIDFromString(tt.args.userGroupStr, tt.args.fallbackToUID) # <--- false only in tests

pkg/util/command_util.go
358:	uid32, gid32, err := getUIDAndGIDFromString(chown, true) # <--- true
370:func getUIDAndGIDFromString(userGroupString string, fallbackToUID bool) (uint32, uint32, error) {

From this, I believe that the option 2 you presented is likely best to match docker build behaviour:

2. we return error on both tests (because the fallback is used when the group is not set as the comment states).

[gadiego92]

@aaron-prindle the main issue (#2327) is fixed. I mean when you use the following Dockerfile:

Kaniko @ head as you already mentioned fails to run the groups command. The groups command returns the groups that the user belongs to and also the UID of the user. This is wrong. Sometimes the UID is not a group and that is what I have fixed.

[aaron-prindle]

@gadiego92 thanks for your investigation and PR here! LGTM!