From 956495784e8b14dea3a80b0d028770f5027d38b0 Mon Sep 17 00:00:00 2001
From: Tejal Desai <tejaldesai@google.com>
Date: Fri, 31 Jan 2020 15:19:57 -0800
Subject: [PATCH] fix group string being always set to uid in case a user has a
 gid set

---
 pkg/commands/user.go      | 11 ++++++++++-
 pkg/commands/user_test.go | 20 +++++++++++++++++++-
 pkg/util/command_util.go  | 15 +++++++++++++++
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/pkg/commands/user.go b/pkg/commands/user.go
index f79e1bd60..35568f74d 100644
--- a/pkg/commands/user.go
+++ b/pkg/commands/user.go
@@ -26,6 +26,11 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// for testing
+var (
+	Lookup = util.Lookup
+)
+
 type UserCommand struct {
 	BaseCommand
 	cmd *instructions.UserCommand
@@ -40,7 +45,11 @@ func (r *UserCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 	if err != nil {
 		return err
 	}
-	groupStr := userStr
+	userObj, err := Lookup(userStr)
+	if err != nil {
+		return err
+	}
+	groupStr := userObj.Gid
 	if len(userAndGroup) > 1 {
 		groupStr, err = util.ResolveEnvironmentReplacement(userAndGroup[1], replacementEnvs, false)
 		if err != nil {
diff --git a/pkg/commands/user_test.go b/pkg/commands/user_test.go
index 916f98c0f..a0db5decb 100644
--- a/pkg/commands/user_test.go
+++ b/pkg/commands/user_test.go
@@ -16,9 +16,11 @@ limitations under the License.
 package commands
 
 import (
+	"os/user"
 	"testing"
 
 	"github.com/GoogleContainerTools/kaniko/pkg/dockerfile"
+	"github.com/GoogleContainerTools/kaniko/pkg/util"
 
 	"github.com/GoogleContainerTools/kaniko/testutil"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
@@ -27,52 +29,64 @@ import (
 
 var userTests = []struct {
 	user        string
+	userObj     user.User
 	expectedUID string
 	expectedGID string
 }{
 	{
 		user:        "root",
+		userObj:     user.User{Uid: "root", Gid: "root"},
 		expectedUID: "root:root",
 	},
 	{
 		user:        "root-add",
-		expectedUID: "root-add:root-add",
+		userObj:     user.User{Uid: "root-add", Gid: "root"},
+		expectedUID: "root-add:root",
 	},
 	{
 		user:        "0",
+		userObj:     user.User{Uid: "0", Gid: "0"},
 		expectedUID: "0:0",
 	},
 	{
 		user:        "fakeUser",
+		userObj:     user.User{Uid: "fakeUser", Gid: "fakeUser"},
 		expectedUID: "fakeUser:fakeUser",
 	},
 	{
 		user:        "root:root",
+		userObj:     user.User{Uid: "root", Gid: "some"},
 		expectedUID: "root:root",
 	},
 	{
 		user:        "0:root",
+		userObj:     user.User{Uid: "0"},
 		expectedUID: "0:root",
 	},
 	{
 		user:        "root:0",
+		userObj:     user.User{Uid: "root"},
 		expectedUID: "root:0",
 		expectedGID: "f0",
 	},
 	{
 		user:        "0:0",
+		userObj:     user.User{Uid: "0"},
 		expectedUID: "0:0",
 	},
 	{
 		user:        "$envuser",
+		userObj:     user.User{Uid: "root", Gid: "root"},
 		expectedUID: "root:root",
 	},
 	{
 		user:        "root:$envgroup",
+		userObj:     user.User{Uid: "root"},
 		expectedUID: "root:grp",
 	},
 	{
 		user:        "some:grp",
+		userObj:     user.User{Uid: "some"},
 		expectedUID: "some:grp",
 	},
 }
@@ -90,6 +104,10 @@ func TestUpdateUser(t *testing.T) {
 				User: test.user,
 			},
 		}
+		Lookup = func(_ string) (*user.User, error) {
+			return &test.userObj, nil
+		}
+		defer func() { Lookup = util.Lookup }()
 		buildArgs := dockerfile.NewBuildArgs([]string{})
 		err := cmd.ExecuteCommand(cfg, buildArgs)
 		testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedUID, cfg.User)
diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go
index 6f391e133..3fdcf7d19 100644
--- a/pkg/util/command_util.go
+++ b/pkg/util/command_util.go
@@ -363,3 +363,18 @@ func GetUserFromUsername(userStr string, groupStr string) (string, string, error
 
 	return uid, gid, nil
 }
+
+func Lookup(userStr string) (*user.User, error) {
+	userObj, err := user.Lookup(userStr)
+	if err != nil {
+		if _, ok := err.(user.UnknownUserError); !ok {
+			return nil, err
+		}
+		// Lookup by id
+		userObj, err = user.LookupId(userStr)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return userObj, nil
+}