Merge pull request #1072 from greut/integration-test-homedir

integration: test case for HOME

integration/dockerfiles/Dockerfile_test_user_without_grp → integration/dockerfiles/Dockerfile_test_user_home

@@ -1,4 +1,4 @@
-# Copyright 2018 Google, Inc. All rights reserved.
+# Copyright 2020 Google, Inc. All rights reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -13,7 +13,23 @@
 # limitations under the License.
 
 FROM debian:9.11
+
+# default values for the root user.
+RUN touch $HOME/hello
+
+# testuser1 with the default home created by useradd.
+
 RUN groupadd testgroup && \
-    useradd --create-home --gid testgroup alice
+    useradd --create-home --gid testgroup alice && \
+    useradd --create-home --uid 1111 --home-dir /home/john --gid testgroup bob
+
+USER alice:testgroup
+RUN touch $HOME/hello
+
+USER bob
+RUN touch $HOME/hello
+
+USER root
 
-USER alice
+USER 1111
+RUN touch $HOME/world

pkg/commands/run.go

@@ -40,7 +40,8 @@ type RunCommand struct {
 
 // for testing
 var (
-	userLookup = user.Lookup
+	userLookup   = user.Lookup
+	userLookupID = user.LookupId
 )
 
 func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
@@ -68,16 +69,29 @@ func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 	cmd.Stderr = os.Stderr
 	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
 	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
-	var userStr string
+
+	u := config.User
+	userAndGroup := strings.Split(u, ":")
+	userStr, err := util.ResolveEnvironmentReplacement(userAndGroup[0], replacementEnvs, false)
+	if err != nil {
+		return errors.Wrapf(err, "resolving user %s", userAndGroup[0])
+	}
+
 	// If specified, run the command as a specific user
-	if config.User != "" {
-		uid, gid, err := util.GetUIDAndGIDFromString(config.User, true)
+	if userStr != "" {
+		uid, gid, err := util.GetUIDAndGIDFromString(userStr, true)
 		if err != nil {
 			return err
 		}
 		cmd.SysProcAttr.Credential = &syscall.Credential{Uid: uid, Gid: gid}
 	}
-	cmd.Env = addDefaultHOME(userStr, replacementEnvs)
+
+	env, err := addDefaultHOME(userStr, replacementEnvs)
+	if err != nil {
+		return errors.Wrap(err, "adding default HOME variable")
+	}
+
+	cmd.Env = env
 
 	if err := cmd.Start(); err != nil {
 		return errors.Wrap(err, "starting command")
@@ -100,32 +114,31 @@ func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 }
 
 // addDefaultHOME adds the default value for HOME if it isn't already set
-func addDefaultHOME(u string, envs []string) []string {
+func addDefaultHOME(u string, envs []string) ([]string, error) {
 	for _, env := range envs {
 		split := strings.SplitN(env, "=", 2)
 		if split[0] == constants.HOME {
-			return envs
+			return envs, nil
 		}
 	}
 
 	// If user isn't set, set default value of HOME
 	if u == "" || u == constants.RootUser {
-		return append(envs, fmt.Sprintf("%s=%s", constants.HOME, constants.DefaultHOMEValue))
+		return append(envs, fmt.Sprintf("%s=%s", constants.HOME, constants.DefaultHOMEValue)), nil
 	}
 
 	// If user is set to username, set value of HOME to /home/${user}
 	// Otherwise the user is set to uid and HOME is /
-	home := "/"
 	userObj, err := userLookup(u)
-	if err == nil {
-		if userObj.HomeDir != "" {
-			home = userObj.HomeDir
+	if err != nil {
+		if uo, e := userLookupID(u); e == nil {
+			userObj = uo
 		} else {
-			home = fmt.Sprintf("/home/%s", userObj.Username)
+			return nil, err
 		}
 	}
 
-	return append(envs, fmt.Sprintf("%s=%s", constants.HOME, home))
+	return append(envs, fmt.Sprintf("%s=%s", constants.HOME, userObj.HomeDir)), nil
 }
 
 // String returns some information about the command for the image config

pkg/commands/run_test.go

@@ -18,6 +18,7 @@ package commands
 import (
 	"archive/tar"
 	"bytes"
+	"errors"
 	"io"
 	"io/ioutil"
 	"log"
@@ -33,11 +34,13 @@ import (
 
 func Test_addDefaultHOME(t *testing.T) {
 	tests := []struct {
-		name     string
-		user     string
-		mockUser *user.User
-		initial  []string
-		expected []string
+		name        string
+		user        string
+		mockUser    *user.User
+		lookupError error
+		mockUserID  *user.User
+		initial     []string
+		expected    []string
 	}{
 		{
 			name: "HOME already set",
@@ -78,31 +81,19 @@ func Test_addDefaultHOME(t *testing.T) {
 			},
 		},
 		{
-			name: "HOME not set and 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 not set and user is set",
-			user: "newuser",
-			mockUser: &user.User{
-				Username: "newuser",
+			name:        "USER is set using the UID",
+			user:        "newuser",
+			lookupError: errors.New("User not found"),
+			mockUserID: &user.User{
+				Username: "user",
+				HomeDir:  "/home/user",
 			},
 			initial: []string{
 				"PATH=/something/else",
 			},
 			expected: []string{
 				"PATH=/something/else",
-				"HOME=/home/newuser",
+				"HOME=/home/user",
 			},
 		},
 		{
@@ -122,10 +113,14 @@ 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)
+			userLookup = func(username string) (*user.User, error) { return test.mockUser, test.lookupError }
+			userLookupID = func(username string) (*user.User, error) { return test.mockUserID, nil }
+			defer func() {
+				userLookup = user.Lookup
+				userLookupID = user.LookupId
+			}()
+			actual, err := addDefaultHOME(test.user, test.initial)
+			testutil.CheckErrorAndDeepEqual(t, false, err, test.expected, actual)
 		})
 	}
 }

pkg/commands/user_test.go

@@ -55,40 +55,40 @@ var userTests = []struct {
 		expectedUID: "fakeUser",
 	},
 	{
-		user:        "root:root",
+		user:        "root",
 		userObj:     &user.User{Uid: "root", Gid: "some"},
-		expectedUID: "root:root",
+		expectedUID: "root",
 	},
 	{
-		user:        "0:root",
+		user:        "0",
 		userObj:     &user.User{Uid: "0"},
-		expectedUID: "0:root",
+		expectedUID: "0",
 	},
 	{
-		user:        "root:0",
+		user:        "root",
 		userObj:     &user.User{Uid: "root"},
-		expectedUID: "root:0",
+		expectedUID: "root",
 		expectedGID: "f0",
 	},
 	{
-		user:        "0:0",
+		user:        "0",
 		userObj:     &user.User{Uid: "0"},
-		expectedUID: "0:0",
+		expectedUID: "0",
 	},
 	{
 		user:        "$envuser",
 		userObj:     &user.User{Uid: "root", Gid: "root"},
 		expectedUID: "root",
 	},
 	{
-		user:        "root:$envgroup",
+		user:        "root",
 		userObj:     &user.User{Uid: "root"},
-		expectedUID: "root:grp",
+		expectedUID: "root",
 	},
 	{
-		user:        "some:grp",
+		user:        "some",
 		userObj:     &user.User{Uid: "some"},
-		expectedUID: "some:grp",
+		expectedUID: "some",
 	},
 	{
 		user:        "some",