diff --git a/Makefile b/Makefile index ff4ef67eb8..00f9c7f16c 100644 --- a/Makefile +++ b/Makefile @@ -66,6 +66,10 @@ minikube-setup: test: out/executor @ ./scripts/test.sh +test-with-coverage: test + go tool cover -html=out/coverage.out + + .PHONY: integration-test integration-test: @ ./scripts/integration-test.sh diff --git a/integration/dockerfiles/Dockerfile_test_copy_chown_nonexisting_user b/integration/dockerfiles/Dockerfile_test_copy_chown_nonexisting_user new file mode 100644 index 0000000000..eea97ae289 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_copy_chown_nonexisting_user @@ -0,0 +1,22 @@ +# Copyright 2018 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. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +FROM debian:9.11 +RUN echo "hey" > /tmp/foo + +FROM debian:9.11 +RUN groupadd --gid 5000 testgroup +COPY --from=0 --chown=1001:1001 /tmp/foo /tmp/baz +# with existing group +COPY --from=0 --chown=1001:testgroup /tmp/foo /tmp/baz diff --git a/integration/dockerfiles/Dockerfile_test_user_nonexisting b/integration/dockerfiles/Dockerfile_test_user_nonexisting new file mode 100644 index 0000000000..8b65af54ac --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_user_nonexisting @@ -0,0 +1,25 @@ +# Copyright 2018 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. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +FROM debian:9.11 +USER 1001:1001 +RUN echo "hey2" > /tmp/foo +USER 1001 +RUN echo "hello" > /tmp/foobar + +# With existing group +USER root +RUN groupadd testgroup +USER 1001:testgroup +RUN echo "hello" > /tmp/foobar diff --git a/integration/k8s_test.go b/integration/k8s_test.go index 935d5f9ef8..0765931620 100644 --- a/integration/k8s_test.go +++ b/integration/k8s_test.go @@ -89,7 +89,7 @@ func TestK8s(t *testing.T) { t.Logf("Waiting for K8s kaniko build job to finish: %s\n", "job/kaniko-test-"+job.Name) - kubeWaitCmd := exec.Command("kubectl", "wait", "--for=condition=complete", "--timeout=1m", + kubeWaitCmd := exec.Command("kubectl", "wait", "--for=condition=complete", "--timeout=2m", "job/kaniko-test-"+job.Name) if out, errR := RunCommandWithoutTest(kubeWaitCmd); errR != nil { t.Log(kubeWaitCmd.Args) diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 2e572794a0..c85b0f89e8 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -53,6 +53,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu replacementEnvs := buildArgs.ReplacementEnvs(config.Env) uid, gid, err := getUserGroup(c.cmd.Chown, replacementEnvs) + logrus.Debugf("found uid %v and gid %v for chown string %v", uid, gid, c.cmd.Chown) if err != nil { return errors.Wrap(err, "getting user group from chown") } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 36a87dc1eb..3290ca0192 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -20,7 +20,6 @@ import ( "fmt" "os" "os/exec" - "os/user" "strings" "syscall" @@ -41,8 +40,7 @@ type RunCommand struct { // for testing var ( - userLookup = user.Lookup - userLookupID = user.LookupId + userLookup = util.LookupUser ) func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { @@ -151,11 +149,7 @@ func addDefaultHOME(u string, envs []string) ([]string, error) { // Otherwise the user is set to uid and HOME is / userObj, err := userLookup(u) if err != nil { - if uo, e := userLookupID(u); e == nil { - userObj = uo - } else { - return nil, err - } + return nil, fmt.Errorf("lookup user %v: %w", u, err) } return append(envs, fmt.Sprintf("%s=%s", constants.HOME, userObj.HomeDir)), nil @@ -256,6 +250,7 @@ func (cr *CachingRunCommand) MetadataOnly() bool { return false } +// todo: this should create the workdir if it doesn't exist, atleast this is what docker does func setWorkDirIfExists(workdir string) string { if _, err := os.Lstat(workdir); err == nil { return workdir diff --git a/pkg/commands/run_test.go b/pkg/commands/run_test.go index 7891329159..0d19d94b26 100644 --- a/pkg/commands/run_test.go +++ b/pkg/commands/run_test.go @@ -18,7 +18,6 @@ package commands import ( "archive/tar" "bytes" - "errors" "io" "io/ioutil" "log" @@ -38,7 +37,6 @@ func Test_addDefaultHOME(t *testing.T) { user string mockUser *user.User lookupError error - mockUserID *user.User initial []string expected []string }{ @@ -81,19 +79,18 @@ func Test_addDefaultHOME(t *testing.T) { }, }, { - name: "USER is set using the UID", - user: "newuser", - lookupError: errors.New("User not found"), - mockUserID: &user.User{ - Username: "user", - HomeDir: "/home/user", + name: "USER is set using the UID", + user: "1000", + mockUser: &user.User{ + Username: "1000", + HomeDir: "/", }, initial: []string{ "PATH=/something/else", }, expected: []string{ "PATH=/something/else", - "HOME=/home/user", + "HOME=/", }, }, { @@ -113,11 +110,10 @@ func Test_addDefaultHOME(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { + original := userLookup 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 + userLookup = original }() actual, err := addDefaultHOME(test.user, test.initial) testutil.CheckErrorAndDeepEqual(t, false, err, test.expected, actual) diff --git a/pkg/commands/user.go b/pkg/commands/user.go index ed31c7b560..937a16a691 100644 --- a/pkg/commands/user.go +++ b/pkg/commands/user.go @@ -28,11 +28,6 @@ import ( "github.com/sirupsen/logrus" ) -// for testing -var ( - Lookup = util.Lookup -) - type UserCommand struct { BaseCommand cmd *instructions.UserCommand diff --git a/pkg/commands/user_test.go b/pkg/commands/user_test.go index 99775b714c..d88d11b856 100644 --- a/pkg/commands/user_test.go +++ b/pkg/commands/user_test.go @@ -16,12 +16,10 @@ 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" @@ -109,13 +107,6 @@ 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) diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index 615ef99fa4..728f89dc4f 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -23,7 +23,6 @@ import ( "os" "os/user" "path/filepath" - reflect "reflect" "strconv" "strings" @@ -39,7 +38,7 @@ import ( // for testing var ( - getUIDAndGID = GetUIDAndGIDFromString + getUIDAndGIDFunc = getUIDAndGID ) const ( @@ -353,7 +352,7 @@ func GetUserGroup(chownStr string, env []string) (int64, int64, error) { return -1, -1, err } - uid32, gid32, err := getUIDAndGID(chown, true) + uid32, gid32, err := getUIDAndGIDFromString(chown, true) if err != nil { return -1, -1, err } @@ -364,86 +363,112 @@ func GetUserGroup(chownStr string, env []string) (int64, int64, error) { // Extract user and group id from a string formatted 'user:group'. // If fallbackToUID is set, the gid is equal to uid if the group is not specified // otherwise gid is set to zero. -func GetUIDAndGIDFromString(userGroupString string, fallbackToUID bool) (uint32, uint32, error) { +// UserID and GroupID don't need to be present on the system. +func getUIDAndGIDFromString(userGroupString string, fallbackToUID bool) (uint32, uint32, error) { userAndGroup := strings.Split(userGroupString, ":") userStr := userAndGroup[0] var groupStr string if len(userAndGroup) > 1 { groupStr = userAndGroup[1] } + return getUIDAndGIDFunc(userStr, groupStr, fallbackToUID) +} - if reflect.TypeOf(userStr).String() == "int" { - return 0, 0, nil - } - - uidStr, gidStr, err := GetUserFromUsername(userStr, groupStr, fallbackToUID) +func getUIDAndGID(userStr string, groupStr string, fallbackToUID bool) (uint32, uint32, error) { + user, err := LookupUser(userStr) if err != nil { return 0, 0, err } - - // uid and gid need to be fit into uint32 - uid64, err := strconv.ParseUint(uidStr, 10, 32) + uid32, err := getUID(user.Uid) if err != nil { return 0, 0, err } - gid64, err := strconv.ParseUint(gidStr, 10, 32) + gid, err := getGIDFromName(groupStr, fallbackToUID) if err != nil { + if errors.Is(err, fallbackToUIDError) { + return uid32, uid32, nil + } return 0, 0, err } - - return uint32(uid64), uint32(gid64), nil + return uid32, gid, nil } -func GetUserFromUsername(userStr string, groupStr string, fallbackToUID bool) (string, string, error) { - // Lookup by username - userObj, err := Lookup(userStr) +// getGID tries to parse the gid or falls back to getGroupFromName if it's not an id +func getGID(groupStr string, fallbackToUID bool) (uint32, error) { + gid, err := strconv.ParseUint(groupStr, 10, 32) if err != nil { - return "", "", err + return 0, fallbackToUIDOrError(err, fallbackToUID) } + return uint32(gid), nil +} - // Same dance with groups - var group *user.Group - if groupStr != "" { - group, err = user.LookupGroup(groupStr) +// getGIDFromName tries to parse the groupStr into an existing group. +// if the group doesn't exist, fallback to getGID to parse non-existing valid GIDs. +func getGIDFromName(groupStr string, fallbackToUID bool) (uint32, error) { + group, err := user.LookupGroup(groupStr) + if err != nil { + // unknown group error could relate to a non existing group + var groupErr *user.UnknownGroupError + if errors.Is(err, groupErr) { + return getGID(groupStr, fallbackToUID) + } + group, err = user.LookupGroupId(groupStr) if err != nil { - if _, ok := err.(user.UnknownGroupError); !ok { - return "", "", err - } - group, err = user.LookupGroupId(groupStr) - if err != nil { - return "", "", err - } + return getGID(groupStr, fallbackToUID) } } + return getGID(group.Gid, fallbackToUID) +} + +var fallbackToUIDError = new(fallbackToUIDErrorType) + +type fallbackToUIDErrorType struct{} - uid := userObj.Uid - gid := "0" +func (e fallbackToUIDErrorType) Error() string { + return "fallback to uid" +} + +func fallbackToUIDOrError(err error, fallbackToUID bool) error { if fallbackToUID { - gid = userObj.Gid + return fallbackToUIDError } - if group != nil { - gid = group.Gid - } - - return uid, gid, nil + return err } -func Lookup(userStr string) (*user.User, error) { +// LookupUser will try to lookup the userStr inside the passwd file. +// If the user does not exists, the function will fallback to parsing the userStr as an uid. +func LookupUser(userStr string) (*user.User, error) { userObj, err := user.Lookup(userStr) if err != nil { - if _, ok := err.(user.UnknownUserError); !ok { + unknownUserErr := new(user.UnknownUserError) + // only return if it's not an unknown user error + if !errors.As(err, unknownUserErr) { return nil, err } // Lookup by id - u, e := user.LookupId(userStr) - if e != nil { - return nil, err + userObj, err = user.LookupId(userStr) + if err != nil { + uid, err := getUID(userStr) + if err != nil { + // at this point, the user does not exist and the userStr is not a valid number. + return nil, fmt.Errorf("user %v is not a uid and does not exist on the system", userStr) + } + userObj = &user.User{ + Uid: fmt.Sprint(uid), + HomeDir: "/", + } } - - userObj = u } - return userObj, nil } + +func getUID(userStr string) (uint32, error) { + // checkif userStr is a valid id + uid, err := strconv.ParseUint(userStr, 10, 32) + if err != nil { + return 0, err + } + return uint32(uid), nil +} diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index 632ef2eb42..483fbfff84 100644 --- a/pkg/util/command_util_test.go +++ b/pkg/util/command_util_test.go @@ -555,28 +555,32 @@ func Test_RemoteUrls(t *testing.T) { func TestGetUserGroup(t *testing.T) { tests := []struct { - description string - chown string - env []string - mock func(string, bool) (uint32, uint32, error) - expectedU int64 - expectedG int64 - shdErr bool + description string + chown string + env []string + mockIDGetter func(userStr string, groupStr string, fallbackToUID bool) (uint32, uint32, error) + // needed, in case uid is a valid number, but group is a name + mockGroupIDGetter func(groupStr string) (*user.Group, error) + expectedU int64 + expectedG int64 + shdErr bool }{ { description: "non empty chown", chown: "some:some", env: []string{}, - mock: func(string, bool) (uint32, uint32, error) { return 100, 1000, nil }, - expectedU: 100, - expectedG: 1000, + mockIDGetter: func(string, string, bool) (uint32, uint32, error) { + return 100, 1000, nil + }, + expectedU: 100, + expectedG: 1000, }, { description: "non empty chown with env replacement", chown: "some:$foo", env: []string{"foo=key"}, - mock: func(c string, t bool) (uint32, uint32, error) { - if c == "some:key" { + mockIDGetter: func(userStr string, groupStr string, fallbackToUID bool) (uint32, uint32, error) { + if userStr == "some" && groupStr == "key" { return 10, 100, nil } return 0, 0, fmt.Errorf("did not resolve environment variable") @@ -586,7 +590,7 @@ func TestGetUserGroup(t *testing.T) { }, { description: "empty chown string", - mock: func(c string, t bool) (uint32, uint32, error) { + mockIDGetter: func(string, string, bool) (uint32, uint32, error) { return 0, 0, fmt.Errorf("should not be called") }, expectedU: -1, @@ -595,9 +599,11 @@ func TestGetUserGroup(t *testing.T) { } for _, tc := range tests { t.Run(tc.description, func(t *testing.T) { - original := getUIDAndGID - defer func() { getUIDAndGID = original }() - getUIDAndGID = tc.mock + originalIDGetter := getUIDAndGIDFunc + defer func() { + getUIDAndGIDFunc = originalIDGetter + }() + getUIDAndGIDFunc = tc.mockIDGetter uid, gid, err := GetUserGroup(tc.chown, tc.env) testutil.CheckErrorAndDeepEqual(t, tc.shdErr, err, uid, tc.expectedU) testutil.CheckErrorAndDeepEqual(t, tc.shdErr, err, gid, tc.expectedG) @@ -661,33 +667,191 @@ func TestResolveEnvironmentReplacementList(t *testing.T) { } func Test_GetUIDAndGIDFromString(t *testing.T) { - currentUser, err := user.Current() - if err != nil { - t.Fatalf("Cannot get current user: %s", err) + currentUser := testutil.GetCurrentUser(t) + + type args struct { + userGroupStr string + fallbackToUID bool } - groups, err := currentUser.GroupIds() - if err != nil || len(groups) == 0 { - t.Fatalf("Cannot get groups for current user: %s", err) + + type expected struct { + userID uint32 + groupID uint32 } - primaryGroupObj, err := user.LookupGroupId(groups[0]) - if err != nil { - t.Fatalf("Could not lookup name of group %s: %s", groups[0], err) + + currentUserUID, _ := strconv.ParseUint(currentUser.Uid, 10, 32) + currentUserGID, _ := strconv.ParseUint(currentUser.Gid, 10, 32) + expectedCurrentUser := expected{ + userID: uint32(currentUserUID), + groupID: uint32(currentUserGID), } - primaryGroup := primaryGroupObj.Name - testCases := []string{ - fmt.Sprintf("%s:%s", currentUser.Uid, currentUser.Gid), - fmt.Sprintf("%s:%s", currentUser.Username, currentUser.Gid), - fmt.Sprintf("%s:%s", currentUser.Uid, primaryGroup), - fmt.Sprintf("%s:%s", currentUser.Username, primaryGroup), + testCases := []struct { + testname string + args args + expected expected + wantErr bool + }{ + { + testname: "current user uid and gid", + args: args{ + userGroupStr: fmt.Sprintf("%d:%d", currentUserUID, currentUserGID), + }, + expected: expectedCurrentUser, + }, + { + testname: "current user username and gid", + args: args{ + userGroupStr: fmt.Sprintf("%s:%d", currentUser.Username, currentUserGID), + }, + expected: expectedCurrentUser, + }, + { + testname: "current user username and primary group", + args: args{ + userGroupStr: fmt.Sprintf("%s:%s", currentUser.Username, currentUser.PrimaryGroup), + }, + expected: expectedCurrentUser, + }, + { + testname: "current user uid and primary group", + args: args{ + userGroupStr: fmt.Sprintf("%d:%s", currentUserUID, currentUser.PrimaryGroup), + }, + expected: expectedCurrentUser, + }, + { + testname: "non-existing valid uid and gid", + args: args{ + userGroupStr: fmt.Sprintf("%d:%d", 1001, 50000), + }, + expected: expected{ + userID: 1001, + groupID: 50000, + }, + }, + { + testname: "uid and existing group", + args: args{ + userGroupStr: fmt.Sprintf("%d:%s", 1001, currentUser.PrimaryGroup), + }, + expected: expected{ + userID: 1001, + groupID: uint32(currentUserGID), + }, + }, + { + 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, + }, + { + testname: "name and non existing gid", + args: args{ + userGroupStr: fmt.Sprintf("%s:%d", currentUser.Username, 50000), + }, + expected: expected{ + userID: expectedCurrentUser.userID, + groupID: 50000, + }, + }, + { + testname: "only uid and fallback is false", + args: args{ + userGroupStr: fmt.Sprintf("%d", currentUserUID), + fallbackToUID: false, + }, + wantErr: true, + }, + { + testname: "only uid and fallback is true", + args: args{ + userGroupStr: fmt.Sprintf("%d", currentUserUID), + fallbackToUID: true, + }, + expected: expected{ + userID: expectedCurrentUser.userID, + groupID: expectedCurrentUser.userID, + }, + }, + { + testname: "non-existing user without group", + args: args{ + userGroupStr: "helloworlduser", + }, + wantErr: true, + }, } - expectedU, _ := strconv.ParseUint(currentUser.Uid, 10, 32) - expectedG, _ := strconv.ParseUint(currentUser.Gid, 10, 32) for _, tt := range testCases { - uid, gid, err := GetUIDAndGIDFromString(tt, false) - if uid != uint32(expectedU) || gid != uint32(expectedG) || err != nil { - t.Errorf("Could not correctly decode %s to uid/gid %d:%d. Result: %d:%d", tt, expectedU, expectedG, + uid, gid, err := getUIDAndGIDFromString(tt.args.userGroupStr, tt.args.fallbackToUID) + testutil.CheckError(t, tt.wantErr, err) + if uid != tt.expected.userID || gid != tt.expected.groupID { + t.Errorf("%v failed. Could not correctly decode %s to uid/gid %d:%d. Result: %d:%d", + tt.testname, + tt.args.userGroupStr, + tt.expected.userID, tt.expected.groupID, uid, gid) } } } + +func TestLookupUser(t *testing.T) { + currentUser := testutil.GetCurrentUser(t) + + type args struct { + userStr string + } + tests := []struct { + testname string + args args + expected *user.User + wantErr bool + }{ + { + testname: "non-existing user", + args: args{ + userStr: "foobazbar", + }, + wantErr: true, + }, + { + testname: "uid", + args: args{ + userStr: "30000", + }, + expected: &user.User{ + Uid: "30000", + HomeDir: "/", + }, + wantErr: false, + }, + { + testname: "current user", + args: args{ + userStr: currentUser.Username, + }, + expected: currentUser.User, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.testname, func(t *testing.T) { + got, err := LookupUser(tt.args.userStr) + testutil.CheckErrorAndDeepEqual(t, tt.wantErr, err, tt.expected, got) + }) + } + +} diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index b21653da2a..d29fb43b46 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -43,8 +43,10 @@ import ( "github.com/sirupsen/logrus" ) -const DoNotChangeUID = -1 -const DoNotChangeGID = -1 +const ( + DoNotChangeUID = -1 + DoNotChangeGID = -1 +) const ( snapshotTimeout = "SNAPSHOT_TIMEOUT_DURATION" @@ -539,6 +541,26 @@ func FilepathExists(path string) bool { return !os.IsNotExist(err) } +// resetFileOwnershipIfNotMatching function changes ownership of the file at path to newUID and newGID. +// If the ownership already matches, chown is not executed. +func resetFileOwnershipIfNotMatching(path string, newUID, newGID uint32) error { + fsInfo, err := os.Lstat(path) + if err != nil { + return errors.Wrap(err, "getting stat of present file") + } + stat, ok := fsInfo.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("can't convert fs.FileInfo of %v to linux syscall.Stat_t", path) + } + if stat.Uid != newUID && stat.Gid != newGID { + err = os.Chown(path, int(newUID), int(newGID)) + if err != nil { + return errors.Wrap(err, "reseting file ownership to root") + } + } + return nil +} + // CreateFile creates a file at path and copies over contents from the reader func CreateFile(path string, reader io.Reader, perm os.FileMode, uid uint32, gid uint32) error { // Create directory path if it doesn't exist @@ -546,6 +568,15 @@ func CreateFile(path string, reader io.Reader, perm os.FileMode, uid uint32, gid return errors.Wrap(err, "creating parent dir") } + // if the file is already created with ownership other than root, reset the ownership + if FilepathExists(path) { + logrus.Debugf("file at %v already exists, resetting file ownership to root", path) + err := resetFileOwnershipIfNotMatching(path, 0, 0) + if err != nil { + return errors.Wrap(err, "reseting file ownership") + } + } + dest, err := os.Create(path) if err != nil { return errors.Wrap(err, "creating file") @@ -793,7 +824,12 @@ func mkdirAllWithPermissions(path string, mode os.FileMode, uid, gid int64) erro } if uid > math.MaxUint32 || gid > math.MaxUint32 { // due to https://github.com/golang/go/issues/8537 - return errors.New(fmt.Sprintf("Numeric User-ID or Group-ID greater than %v are not properly supported.", uint64(math.MaxUint32))) + return errors.New( + fmt.Sprintf( + "Numeric User-ID or Group-ID greater than %v are not properly supported.", + uint64(math.MaxUint32), + ), + ) } if err := os.Chown(path, int(uid), int(gid)); err != nil { return err @@ -851,7 +887,6 @@ func CreateTargetTarfile(tarpath string) (*os.File, error) { } } return os.Create(tarpath) - } // Returns true if a file is a symlink @@ -1009,8 +1044,8 @@ type walkFSResult struct { func WalkFS( dir string, existingPaths map[string]struct{}, - changeFunc func(string) (bool, error)) ([]string, map[string]struct{}) { - + changeFunc func(string) (bool, error), +) ([]string, map[string]struct{}) { timeOutStr := os.Getenv(snapshotTimeout) if timeOutStr == "" { logrus.Tracef("Environment '%s' not set. Using default snapshot timeout '%s'", snapshotTimeout, defaultTimeout) @@ -1102,7 +1137,6 @@ func GetFSInfoMap(dir string, existing map[string]os.FileInfo) (map[string]os.Fi fileMap[path] = fi foundPaths = append(foundPaths, path) } - } return nil }, diff --git a/pkg/util/groupids_cgo.go b/pkg/util/groupids_cgo.go index 376762e079..73865a638e 100644 --- a/pkg/util/groupids_cgo.go +++ b/pkg/util/groupids_cgo.go @@ -26,5 +26,9 @@ import ( // groupIDs returns all of the group ID's a user is a member of func groupIDs(u *user.User) ([]string, error) { + // user can have no gid if it's a non existing user + if u.Gid == "" { + return []string{}, nil + } return u.GroupIds() } diff --git a/pkg/util/groupids_fallback.go b/pkg/util/groupids_fallback.go index 62d0760244..e336d72e9f 100644 --- a/pkg/util/groupids_fallback.go +++ b/pkg/util/groupids_fallback.go @@ -45,6 +45,11 @@ type group struct { func groupIDs(u *user.User) ([]string, error) { logrus.Infof("Performing slow lookup of group ids for %s", u.Username) + // user can have no gid if it's a non existing user + if u.Gid == "" { + return []string{}, nil + } + f, err := os.Open(groupFile) if err != nil { return nil, errors.Wrap(err, "open") diff --git a/pkg/util/syscall_credentials.go b/pkg/util/syscall_credentials.go index 8aa9b11aa6..a316ea004d 100644 --- a/pkg/util/syscall_credentials.go +++ b/pkg/util/syscall_credentials.go @@ -17,6 +17,7 @@ limitations under the License. package util import ( + "fmt" "strconv" "syscall" @@ -25,18 +26,19 @@ import ( ) func SyscallCredentials(userStr string) (*syscall.Credential, error) { - uid, gid, err := GetUIDAndGIDFromString(userStr, true) + uid, gid, err := getUIDAndGIDFromString(userStr, true) if err != nil { return nil, errors.Wrap(err, "get uid/gid") } - u, err := Lookup(userStr) + u, err := LookupUser(fmt.Sprint(uid)) if err != nil { return nil, errors.Wrap(err, "lookup") } logrus.Infof("Util.Lookup returned: %+v", u) - var groups []uint32 + // initiliaze empty + groups := []uint32{} gidStr, err := groupIDs(u) if err != nil { diff --git a/pkg/util/syscall_credentials_test.go b/pkg/util/syscall_credentials_test.go new file mode 100644 index 0000000000..e987dcd773 --- /dev/null +++ b/pkg/util/syscall_credentials_test.go @@ -0,0 +1,99 @@ +/* +Copyright 2020 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "fmt" + "strconv" + "syscall" + "testing" + + "github.com/GoogleContainerTools/kaniko/testutil" +) + +func TestSyscallCredentials(t *testing.T) { + currentUser := testutil.GetCurrentUser(t) + uid, _ := strconv.ParseUint(currentUser.Uid, 10, 32) + currentUserUID32 := uint32(uid) + gid, _ := strconv.ParseUint(currentUser.Gid, 10, 32) + currentUserGID32 := uint32(gid) + + currentUserGroupIDsU32 := []uint32{} + currentUserGroupIDs, _ := currentUser.GroupIds() + for _, id := range currentUserGroupIDs { + id32, _ := strconv.ParseUint(id, 10, 32) + currentUserGroupIDsU32 = append(currentUserGroupIDsU32, uint32(id32)) + } + + type args struct { + userStr string + } + tests := []struct { + name string + args args + want *syscall.Credential + wantErr bool + }{ + { + name: "non-existing user without group", + args: args{ + userStr: "helloworld-user", + }, + wantErr: true, + }, + { + name: "non-existing uid without group", + args: args{ + userStr: "50000", + }, + want: &syscall.Credential{ + Uid: 50000, + // because fallback is enabled + Gid: 50000, + Groups: []uint32{}, + }, + }, + { + name: "non-existing uid with existing gid", + args: args{ + userStr: fmt.Sprintf("50000:%d", currentUserGID32), + }, + want: &syscall.Credential{ + Uid: 50000, + Gid: currentUserGID32, + Groups: []uint32{}, + }, + }, + { + name: "existing username with non-existing gid", + args: args{ + userStr: fmt.Sprintf("%s:50000", currentUser.Username), + }, + want: &syscall.Credential{ + Uid: currentUserUID32, + Gid: 50000, + Groups: currentUserGroupIDsU32, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := SyscallCredentials(tt.args.userStr) + testutil.CheckErrorAndDeepEqual(t, tt.wantErr, err, tt.want, got) + }) + } +} diff --git a/scripts/test.sh b/scripts/test.sh index d9439a7218..c097453c34 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -14,14 +14,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -set -e +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" + +#set -e RED='\033[0;31m' GREEN='\033[0;32m' RESET='\033[0m' echo "Running go tests..." -go test -cover -v -timeout 60s `go list ./... | grep -v vendor | grep -v integration` | sed ''/PASS/s//$(printf "${GREEN}PASS${RESET}")/'' | sed ''/FAIL/s//$(printf "${RED}FAIL${RESET}")/'' +go test -cover -coverprofile=out/coverage.out -v -timeout 60s `go list ./... | grep -v vendor | grep -v integration` | sed ''/PASS/s//$(printf "${GREEN}PASS${RESET}")/'' | sed ''/FAIL/s//$(printf "${RED}FAIL${RESET}")/'' GO_TEST_EXIT_CODE=${PIPESTATUS[0]} if [[ $GO_TEST_EXIT_CODE -ne 0 ]]; then exit $GO_TEST_EXIT_CODE @@ -29,15 +31,15 @@ fi echo "Running validation scripts..." scripts=( - "hack/boilerplate.sh" - "hack/gofmt.sh" - "hack/linter.sh" + "$DIR/../hack/boilerplate.sh" + "$DIR/../hack/gofmt.sh" + "$DIR/../hack/linter.sh" ) fail=0 for s in "${scripts[@]}" do echo "RUN ${s}" - if "./${s}"; then + if "${s}"; then echo -e "${GREEN}PASSED${RESET} ${s}" else echo -e "${RED}FAILED${RESET} ${s}" diff --git a/testutil/util.go b/testutil/util.go index 9a10ea6daf..e3d0b04c19 100644 --- a/testutil/util.go +++ b/testutil/util.go @@ -20,6 +20,7 @@ import ( "fmt" "io/ioutil" "os" + "os/user" "path/filepath" "reflect" "testing" @@ -41,6 +42,33 @@ func SetupFiles(path string, files map[string]string) error { return nil } +type CurrentUser struct { + *user.User + + PrimaryGroup string +} + +func GetCurrentUser(t *testing.T) CurrentUser { + currentUser, err := user.Current() + if err != nil { + t.Fatalf("Cannot get current user: %s", err) + } + groups, err := currentUser.GroupIds() + if err != nil || len(groups) == 0 { + t.Fatalf("Cannot get groups for current user: %s", err) + } + primaryGroupObj, err := user.LookupGroupId(groups[0]) + if err != nil { + t.Fatalf("Could not lookup name of group %s: %s", groups[0], err) + } + primaryGroup := primaryGroupObj.Name + + return CurrentUser{ + User: currentUser, + PrimaryGroup: primaryGroup, + } +} + func CheckDeepEqual(t *testing.T, expected, actual interface{}) { t.Helper() if diff := cmp.Diff(actual, expected); diff != "" {