Merge pull request #1017 from tejal29/correct_user_grp_str

fix group string being always set to uid in case a user has a gid set

pkg/commands/user.go

@@ -17,15 +17,22 @@ limitations under the License.
 package commands
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/GoogleContainerTools/kaniko/pkg/dockerfile"
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
+// for testing
+var (
+	Lookup = util.Lookup
+)
+
 type UserCommand struct {
 	BaseCommand
 	cmd *instructions.UserCommand
@@ -38,13 +45,13 @@ func (r *UserCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
 	userStr, err := util.ResolveEnvironmentReplacement(userAndGroup[0], replacementEnvs, false)
 	if err != nil {
-		return err
+		return errors.Wrap(err, fmt.Sprintf("resolving user %s", userAndGroup[0]))
 	}
-	groupStr := userStr
+	var groupStr = setGroupDefault(userStr)
 	if len(userAndGroup) > 1 {
 		groupStr, err = util.ResolveEnvironmentReplacement(userAndGroup[1], replacementEnvs, false)
 		if err != nil {
-			return err
+			return errors.Wrap(err, fmt.Sprintf("resolving group %s", userAndGroup[1]))
 		}
 	}
 
@@ -57,3 +64,12 @@ func (r *UserCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 func (r *UserCommand) String() string {
 	return r.cmd.String()
 }
+
+func setGroupDefault(userStr string) string {
+	userObj, err := Lookup(userStr)
+	if err != nil {
+		logrus.Debugf("could not lookup user %s. Setting group empty", userStr)
+		return ""
+	}
+	return userObj.Gid
+}

pkg/commands/user_test.go

@@ -16,9 +16,12 @@ limitations under the License.
 package commands
 
 import (
+	"fmt"
+	"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,54 +30,70 @@ 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",
 	},
+	{
+		user:        "some",
+		expectedUID: "some:",
+	},
 }
 
 func TestUpdateUser(t *testing.T) {
@@ -90,6 +109,13 @@ func TestUpdateUser(t *testing.T) {
 				User: test.user,
 			},
 		}
+		Lookup = func(_ string) (*user.User, error) {
+			if test.userObj != nil {
+				return test.userObj, nil
+			}
+			return nil, fmt.Errorf("error while looking up user")
+		}
+		defer func() { Lookup = util.Lookup }()
 		buildArgs := dockerfile.NewBuildArgs([]string{})
 		err := cmd.ExecuteCommand(cfg, buildArgs)
 		testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedUID, cfg.User)

pkg/util/command_util.go

@@ -328,16 +328,9 @@ Loop:
 
 func GetUserFromUsername(userStr string, groupStr string) (string, string, error) {
 	// Lookup by username
-	userObj, err := user.Lookup(userStr)
+	userObj, err := Lookup(userStr)
 	if err != nil {
-		if _, ok := err.(user.UnknownUserError); !ok {
-			return "", "", err
-		}
-		// Lookup by id
-		userObj, err = user.LookupId(userStr)
-		if err != nil {
-			return "", "", err
-		}
+		return "", "", err
 	}
 
 	// Same dance with groups
@@ -363,3 +356,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
+}