From b29431227a6c9f116aa4a3406bb621eb2d985a22 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 18 Oct 2019 16:48:20 -0700 Subject: [PATCH 1/4] fix tests for default home --- pkg/commands/run.go | 12 +++++++++-- pkg/commands/run_test.go | 45 ++++++++++++++++++++++++++++++++++++--- pkg/commands/user_test.go | 4 ++++ 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 0122f37d5..2e4aa3f36 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -39,6 +39,11 @@ type RunCommand struct { cmd *instructions.RunCommand } +// for testing +var ( + userLookup = user.Lookup +) + func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { var newCommand []string if r.cmd.PrependShell { @@ -134,9 +139,12 @@ func addDefaultHOME(u string, envs []string) []string { // If user is set to username, set value of HOME to /home/${user} // Otherwise the user is set to uid and HOME is / home := fmt.Sprintf("%s=/", constants.HOME) - userObj, err := user.Lookup(u) + userObj, err := userLookup(u) if err == nil { - u = userObj.Username + u = userObj.HomeDir + if u == "" { + u = userObj.Username + } home = fmt.Sprintf("%s=/home/%s", constants.HOME, u) } diff --git a/pkg/commands/run_test.go b/pkg/commands/run_test.go index 8609ac67e..fe228a41f 100644 --- a/pkg/commands/run_test.go +++ b/pkg/commands/run_test.go @@ -16,6 +16,7 @@ limitations under the License. package commands import ( + "os/user" "testing" "github.com/GoogleContainerTools/kaniko/testutil" @@ -25,6 +26,7 @@ func Test_addDefaultHOME(t *testing.T) { tests := []struct { name string user string + mockUser *user.User initial []string expected []string }{ @@ -52,19 +54,54 @@ func Test_addDefaultHOME(t *testing.T) { }, }, { - name: "HOME isn't set, user is set", - user: "newuser", + name: "HOME isn't set, user and homedir for the user set", + user: "www-add", + mockUser: &user.User{ + Username: "www-add", + HomeDir: "some-other", + }, initial: []string{ "PATH=/something/else", }, expected: []string{ "PATH=/something/else", - "HOME=/", + "HOME=/home/some-other", + }, + }, + { + name: "HOME isn't set, user set", + user: "www-add", + mockUser: &user.User{ + Username: "www-add", + }, + initial: []string{ + "PATH=/something/else", + }, + expected: []string{ + "PATH=/something/else", + "HOME=/home/www-add", + }, + }, + { + name: "HOME isn't set, user is set", + user: "newuser", + mockUser: &user.User{ + Username: "newuser", + }, + initial: []string{ + "PATH=/something/else", + }, + expected: []string{ + "PATH=/something/else", + "HOME=/home/newuser", }, }, { name: "HOME isn't set, user is set to root", user: "root", + mockUser: &user.User{ + Username: "root", + }, initial: []string{ "PATH=/something/else", }, @@ -76,6 +113,8 @@ func Test_addDefaultHOME(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { + userLookup = func(username string) ( *user.User, error) { return test.mockUser, nil} + defer func() {userLookup = user.Lookup}() actual := addDefaultHOME(test.user, test.initial) testutil.CheckErrorAndDeepEqual(t, false, nil, test.expected, actual) }) diff --git a/pkg/commands/user_test.go b/pkg/commands/user_test.go index 343a7bed4..12afcb02f 100644 --- a/pkg/commands/user_test.go +++ b/pkg/commands/user_test.go @@ -33,6 +33,10 @@ var userTests = []struct { user: "root", expectedUID: "root", }, + { + user: "root-add", + expectedUID: "root-add", + }, { user: "0", expectedUID: "0", From 334f0c70d6d4a9c3c7485f58f5eef6ce22d851f7 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Thu, 7 Nov 2019 13:35:28 -0800 Subject: [PATCH 2/4] fix golint --- pkg/commands/run_test.go | 6 +++--- pkg/util/fs_util.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/commands/run_test.go b/pkg/commands/run_test.go index fe228a41f..f65c0e9e3 100644 --- a/pkg/commands/run_test.go +++ b/pkg/commands/run_test.go @@ -58,7 +58,7 @@ func Test_addDefaultHOME(t *testing.T) { user: "www-add", mockUser: &user.User{ Username: "www-add", - HomeDir: "some-other", + HomeDir: "some-other", }, initial: []string{ "PATH=/something/else", @@ -113,8 +113,8 @@ func Test_addDefaultHOME(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - userLookup = func(username string) ( *user.User, error) { return test.mockUser, nil} - defer func() {userLookup = user.Lookup}() + userLookup = func(username string) (*user.User, error) { return test.mockUser, nil } + defer func() { userLookup = user.Lookup }() actual := addDefaultHOME(test.user, test.initial) testutil.CheckErrorAndDeepEqual(t, false, nil, test.expected, actual) }) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index ab327c860..00562e56f 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -84,7 +84,7 @@ func GetFSFromImage(root string, img v1.Image) ([]string, error) { for i, l := range layers { if mediaType, err := l.MediaType(); err == nil { - logrus.Tracef("Extracting layer %d of media type %s", mediaType) + logrus.Tracef("Extracting layer %d of media type %s", i, mediaType) } else { logrus.Tracef("Extracting layer %d", i) } From f36dd4013c9f3d895b99fa15def2e53dfd395176 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Thu, 7 Nov 2019 15:17:25 -0800 Subject: [PATCH 3/4] only add tests --- pkg/commands/run.go | 5 +---- pkg/commands/run_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 2e4aa3f36..fe8d4b27e 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -141,10 +141,7 @@ func addDefaultHOME(u string, envs []string) []string { home := fmt.Sprintf("%s=/", constants.HOME) userObj, err := userLookup(u) if err == nil { - u = userObj.HomeDir - if u == "" { - u = userObj.Username - } + u = userObj.Username home = fmt.Sprintf("%s=/home/%s", constants.HOME, u) } diff --git a/pkg/commands/run_test.go b/pkg/commands/run_test.go index f65c0e9e3..8640f57e4 100644 --- a/pkg/commands/run_test.go +++ b/pkg/commands/run_test.go @@ -43,7 +43,7 @@ func Test_addDefaultHOME(t *testing.T) { }, }, { - name: "HOME isn't set, user isn't set", + name: "HOME not set, user not set", user: "", initial: []string{ "PATH=/something/else", @@ -54,7 +54,7 @@ func Test_addDefaultHOME(t *testing.T) { }, }, { - name: "HOME isn't set, user and homedir for the user set", + name: "HOME not set, user and homedir for the user set", user: "www-add", mockUser: &user.User{ Username: "www-add", @@ -65,11 +65,11 @@ func Test_addDefaultHOME(t *testing.T) { }, expected: []string{ "PATH=/something/else", - "HOME=/home/some-other", + "HOME=/home/www-add", }, }, { - name: "HOME isn't set, user set", + name: "HOME not set, user set", user: "www-add", mockUser: &user.User{ Username: "www-add", @@ -83,7 +83,7 @@ func Test_addDefaultHOME(t *testing.T) { }, }, { - name: "HOME isn't set, user is set", + name: "HOME not set, user is set", user: "newuser", mockUser: &user.User{ Username: "newuser", @@ -97,7 +97,7 @@ func Test_addDefaultHOME(t *testing.T) { }, }, { - name: "HOME isn't set, user is set to root", + name: "HOME not set, user is set to root", user: "root", mockUser: &user.User{ Username: "root", From b04d15db82a470240e435dd7fd2567aa2a64e291 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Thu, 7 Nov 2019 15:29:56 -0800 Subject: [PATCH 4/4] fix the issue now --- pkg/commands/run.go | 11 +++++++---- pkg/commands/run_test.go | 14 +++++++------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index fe8d4b27e..4867fd546 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -138,14 +138,17 @@ func addDefaultHOME(u string, envs []string) []string { // If user is set to username, set value of HOME to /home/${user} // Otherwise the user is set to uid and HOME is / - home := fmt.Sprintf("%s=/", constants.HOME) + home := "/" userObj, err := userLookup(u) if err == nil { - u = userObj.Username - home = fmt.Sprintf("%s=/home/%s", constants.HOME, u) + if userObj.HomeDir != "" { + home = userObj.HomeDir + } else { + home = fmt.Sprintf("/home/%s", userObj.Username) + } } - return append(envs, home) + return append(envs, fmt.Sprintf("%s=%s", constants.HOME, home)) } // String returns some information about the command for the image config diff --git a/pkg/commands/run_test.go b/pkg/commands/run_test.go index 8640f57e4..0e7c51e8c 100644 --- a/pkg/commands/run_test.go +++ b/pkg/commands/run_test.go @@ -43,7 +43,7 @@ func Test_addDefaultHOME(t *testing.T) { }, }, { - name: "HOME not set, user not set", + name: "HOME not set and user not set", user: "", initial: []string{ "PATH=/something/else", @@ -54,22 +54,22 @@ func Test_addDefaultHOME(t *testing.T) { }, }, { - name: "HOME not set, user and homedir for the user set", + name: "HOME not set and user and homedir for the user set", user: "www-add", mockUser: &user.User{ Username: "www-add", - HomeDir: "some-other", + HomeDir: "/home/some-other", }, initial: []string{ "PATH=/something/else", }, expected: []string{ "PATH=/something/else", - "HOME=/home/www-add", + "HOME=/home/some-other", }, }, { - name: "HOME not set, user set", + name: "HOME not set and user set", user: "www-add", mockUser: &user.User{ Username: "www-add", @@ -83,7 +83,7 @@ func Test_addDefaultHOME(t *testing.T) { }, }, { - name: "HOME not set, user is set", + name: "HOME not set and user is set", user: "newuser", mockUser: &user.User{ Username: "newuser", @@ -97,7 +97,7 @@ func Test_addDefaultHOME(t *testing.T) { }, }, { - name: "HOME not set, user is set to root", + name: "HOME not set and user is set to root", user: "root", mockUser: &user.User{ Username: "root",