From e3b5a7b85daa2209e697c82f44d41bff3966de00 Mon Sep 17 00:00:00 2001 From: xanonid Date: Fri, 10 Jan 2020 18:06:32 +0100 Subject: [PATCH 1/5] Support COPY --chown flag (Closes: #9) --- cmd/executor/cmd/root.go | 2 +- pkg/commands/copy.go | 20 ++++++++++++++++++-- pkg/commands/run.go | 16 ++-------------- pkg/util/command_util.go | 19 ++++++++++++++++++- pkg/util/fs_util.go | 37 ++++++++++++++++++++++++------------- pkg/util/fs_util_test.go | 2 +- 6 files changed, 64 insertions(+), 32 deletions(-) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 1d3717298..937e7f4a4 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -215,7 +215,7 @@ func resolveEnvironmentBuildArgs(arguments []string, resolver func(string) strin // copy Dockerfile to /kaniko/Dockerfile so that if it's specified in the .dockerignore // it won't be copied into the image func copyDockerfile() error { - if _, err := util.CopyFile(opts.DockerfilePath, constants.DockerfilePath, ""); err != nil { + if _, err := util.CopyFile(opts.DockerfilePath, constants.DockerfilePath, "", -1, -1); err != nil { return errors.Wrap(err, "copying dockerfile") } opts.DockerfilePath = constants.DockerfilePath diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index d94f3e10f..7af0b34f1 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -44,9 +44,25 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu if c.cmd.From != "" { c.buildcontext = filepath.Join(constants.KanikoDir, c.cmd.From) } + var uid, gid int64 + uid = -1 + gid = -1 replacementEnvs := buildArgs.ReplacementEnvs(config.Env) + if c.cmd.Chown != "" { + chown, err := util.ResolveEnvironmentReplacement(c.cmd.Chown, replacementEnvs, false) + if err != nil { + return err + } + uid32, gid32, err := util.GetUIDAndGIDFromString(chown, true) + uid = int64(uid32) + gid = int64(gid32) + if err != nil { + return err + } + } + srcs, dest, err := util.ResolveEnvAndWildcards(c.cmd.SourcesAndDest, c.buildcontext, replacementEnvs) if err != nil { return err @@ -80,7 +96,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu } if fi.IsDir() { - copiedFiles, err := util.CopyDir(fullPath, destPath, c.buildcontext) + copiedFiles, err := util.CopyDir(fullPath, destPath, c.buildcontext, uid, gid) if err != nil { return err } @@ -97,7 +113,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu c.snapshotFiles = append(c.snapshotFiles, destPath) } else { // ... Else, we want to copy over a file - exclude, err := util.CopyFile(fullPath, destPath, c.buildcontext) + exclude, err := util.CopyFile(fullPath, destPath, c.buildcontext, uid, gid) if err != nil { return err } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 108cdc5f6..73ea4ca94 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -21,7 +21,6 @@ import ( "os" "os/exec" "os/user" - "strconv" "strings" "syscall" @@ -80,24 +79,13 @@ func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui } uidStr, gidStr, err := util.GetUserFromUsername(userStr, groupStr) - if err != nil { + if err !=nil{ return err } - - // uid and gid need to be uint32 - uid64, err := strconv.ParseUint(uidStr, 10, 32) + uid, gid, err := util.GetUIDAndGIDFromString(uidStr, gidStr) if err != nil { return err } - uid := uint32(uid64) - var gid uint32 - if gidStr != "" { - gid64, err := strconv.ParseUint(gidStr, 10, 32) - if err != nil { - return err - } - gid = uint32(gid64) - } cmd.SysProcAttr.Credential = &syscall.Credential{Uid: uid, Gid: gid} } cmd.Env = addDefaultHOME(userStr, replacementEnvs) diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index cadf6e564..2121f4055 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -23,6 +23,7 @@ import ( "os" "os/user" "path/filepath" + "strconv" "strings" "github.com/GoogleContainerTools/kaniko/pkg/constants" @@ -326,6 +327,22 @@ Loop: return nil } +// Extract user and group id from a string formatted 'user:group'. +// If gidEqualToUIDIfNotGiven is set, the gid is equal to uid if the group is not specified +// otherwise gid is set to zero. +func GetUIDAndGIDFromString(uidStr string, gidStr string) (uint32, uint32, error) { + // uid and gid need to be fit into uint32 + uid64, err := strconv.ParseUint(uidStr, 10, 32) + if err != nil { + return 0, 0, err + } + gid64, err := strconv.ParseUint(gidStr, 10, 32) + if err != nil { + return 0, 0, err + } + return uint32(uid64), uint32(gid64), nil +} + func GetUserFromUsername(userStr string, groupStr string) (string, string, error) { // Lookup by username userObj, err := Lookup(userStr) @@ -349,7 +366,7 @@ func GetUserFromUsername(userStr string, groupStr string) (string, string, error } uid := userObj.Uid - gid := "" + gid := userObj.Gid if group != nil { gid = group.Gid } diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 010a14273..aff1f325c 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -23,6 +23,7 @@ import ( "fmt" "io" "io/ioutil" + "math" "net/http" "os" "path/filepath" @@ -303,7 +304,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error { currFile.Close() case tar.TypeDir: logrus.Tracef("creating dir %s", path) - if err := mkdirAllWithPermissions(path, mode, uid, gid); err != nil { + if err := mkdirAllWithPermissions(path, mode, int64(uid), int64(gid)); err != nil { return err } @@ -540,7 +541,7 @@ func DownloadFileToDest(rawurl, dest string) error { // CopyDir copies the file or directory at src to dest // It returns a list of files it copied over -func CopyDir(src, dest, buildcontext string) ([]string, error) { +func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { files, err := RelativeFiles("", src) if err != nil { return nil, err @@ -562,9 +563,12 @@ func CopyDir(src, dest, buildcontext string) ([]string, error) { logrus.Tracef("Creating directory %s", destPath) mode := fi.Mode() - uid := int(fi.Sys().(*syscall.Stat_t).Uid) - gid := int(fi.Sys().(*syscall.Stat_t).Gid) - + if uid < 0 { + uid = int64(int(fi.Sys().(*syscall.Stat_t).Uid)) + } + if gid < 0 { + gid = int64(int(fi.Sys().(*syscall.Stat_t).Gid)) + } if err := mkdirAllWithPermissions(destPath, mode, uid, gid); err != nil { return nil, err } @@ -575,7 +579,7 @@ func CopyDir(src, dest, buildcontext string) ([]string, error) { } } else { // ... Else, we want to copy over a file - if _, err := CopyFile(fullPath, destPath, buildcontext); err != nil { + if _, err := CopyFile(fullPath, destPath, buildcontext, uid, gid); err != nil { return nil, err } } @@ -606,7 +610,7 @@ func CopySymlink(src, dest, buildcontext string) (bool, error) { } // CopyFile copies the file at src to dest -func CopyFile(src, dest, buildcontext string) (bool, error) { +func CopyFile(src, dest, buildcontext string, uid, gid int64) (bool, error) { if ExcludeFile(src, buildcontext) { logrus.Debugf("%s found in .dockerignore, ignoring", src) return true, nil @@ -627,9 +631,13 @@ func CopyFile(src, dest, buildcontext string) (bool, error) { return false, err } defer srcFile.Close() - uid := fi.Sys().(*syscall.Stat_t).Uid - gid := fi.Sys().(*syscall.Stat_t).Gid - return false, CreateFile(dest, srcFile, fi.Mode(), uid, gid) + if uid < 0 { + uid = int64(fi.Sys().(*syscall.Stat_t).Uid) + } + if gid < 0 { + gid = int64(fi.Sys().(*syscall.Stat_t).Gid) + } + return false, CreateFile(dest, srcFile, fi.Mode(), uint32(uid), uint32(gid)) } // GetExcludedFiles gets a list of files to exclude from the .dockerignore @@ -699,12 +707,15 @@ func Volumes() []string { return volumes } -func mkdirAllWithPermissions(path string, mode os.FileMode, uid, gid int) error { +func mkdirAllWithPermissions(path string, mode os.FileMode, uid, gid int64) error { if err := os.MkdirAll(path, mode); err != nil { return err } - - if err := os.Chown(path, uid, gid); err != nil { + 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.", math.MaxUint32)) + } + if err := os.Chown(path, int(uid), int(gid)); err != nil { return err } // In some cases, MkdirAll doesn't change the permissions, so run Chmod diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index e2529ed97..109f2337a 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -963,7 +963,7 @@ func Test_CopyFile_skips_self(t *testing.T) { t.Fatal(err) } - ignored, err := CopyFile(tempFile, tempFile, "") + ignored, err := CopyFile(tempFile, tempFile, "", -1, -1) if err != nil { t.Fatal(err) } From 517cb50278b6eb6222feca9dd80bc1b8c737d4df Mon Sep 17 00:00:00 2001 From: xanonid Date: Fri, 24 Jan 2020 16:16:22 +0100 Subject: [PATCH 2/5] Add Unit test for GetUIDAndGIDFromString --- pkg/util/command_util_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index 8e85b3e25..b5895e9a6 100644 --- a/pkg/util/command_util_test.go +++ b/pkg/util/command_util_test.go @@ -17,8 +17,11 @@ limitations under the License. package util import ( + "fmt" + "os/user" "reflect" "sort" + "strconv" "testing" "github.com/GoogleContainerTools/kaniko/testutil" @@ -526,3 +529,35 @@ 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) + } + 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 + + 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), + } + expectedUid, _ := strconv.ParseUint(currentUser.Uid, 10, 32) + expectedGid, _ := strconv.ParseUint(currentUser.Gid, 10, 32) + for _, tt := range testCases { + uid, gid, err := GetUIDAndGIDFromString(tt, false) + if uid != uint32(expectedUid) || gid != uint32(expectedGid) || err != nil { + t.Errorf("Could not correctly decode %s to uid/gid %d:%d. Result: %d:%d", tt, expectedUid, expectedGid, + uid, gid) + } + } +} From ac4c80d2416d7486c9a0bc9b95ef2cab31601370 Mon Sep 17 00:00:00 2001 From: xanonid Date: Fri, 24 Jan 2020 16:49:48 +0100 Subject: [PATCH 3/5] Refactor code, introduce constants for uid/gid = -1 --- cmd/executor/cmd/root.go | 2 +- pkg/commands/copy.go | 4 ++-- pkg/util/fs_util.go | 29 +++++++++++++++++------------ pkg/util/fs_util_test.go | 2 +- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 937e7f4a4..f39e011ed 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -215,7 +215,7 @@ func resolveEnvironmentBuildArgs(arguments []string, resolver func(string) strin // copy Dockerfile to /kaniko/Dockerfile so that if it's specified in the .dockerignore // it won't be copied into the image func copyDockerfile() error { - if _, err := util.CopyFile(opts.DockerfilePath, constants.DockerfilePath, "", -1, -1); err != nil { + if _, err := util.CopyFile(opts.DockerfilePath, constants.DockerfilePath, "", util.DoNotChangeUID, util.DoNotChangeGID); err != nil { return errors.Wrap(err, "copying dockerfile") } opts.DockerfilePath = constants.DockerfilePath diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 7af0b34f1..25a78cdb9 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -45,8 +45,8 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu c.buildcontext = filepath.Join(constants.KanikoDir, c.cmd.From) } var uid, gid int64 - uid = -1 - gid = -1 + uid = util.DoNotChangeUID + gid = util.DoNotChangeGID replacementEnvs := buildArgs.ReplacementEnvs(config.Env) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index aff1f325c..dca42b90d 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -41,6 +41,9 @@ import ( "github.com/sirupsen/logrus" ) +const DoNotChangeUID = -1 +const DoNotChangeGID = -1 + type WhitelistEntry struct { Path string PrefixMatchOnly bool @@ -539,6 +542,18 @@ func DownloadFileToDest(rawurl, dest string) error { return os.Chtimes(dest, mTime, mTime) } +// DetermineTargetFileOwnership returns the user provided uid/gid combination. +// If they are set to -1, the uid/gid from the original file is used. +func DetermineTargetFileOwnership(fi os.FileInfo, uid, gid int64) (int64, int64) { + if uid <= DoNotChangeUID { + uid = int64(fi.Sys().(*syscall.Stat_t).Uid) + } + if gid <= DoNotChangeGID { + gid = int64(fi.Sys().(*syscall.Stat_t).Gid) + } + return uid, gid +} + // CopyDir copies the file or directory at src to dest // It returns a list of files it copied over func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { @@ -563,12 +578,7 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { logrus.Tracef("Creating directory %s", destPath) mode := fi.Mode() - if uid < 0 { - uid = int64(int(fi.Sys().(*syscall.Stat_t).Uid)) - } - if gid < 0 { - gid = int64(int(fi.Sys().(*syscall.Stat_t).Gid)) - } + uid, gid = DetermineTargetFileOwnership(fi, uid, gid) if err := mkdirAllWithPermissions(destPath, mode, uid, gid); err != nil { return nil, err } @@ -631,12 +641,7 @@ func CopyFile(src, dest, buildcontext string, uid, gid int64) (bool, error) { return false, err } defer srcFile.Close() - if uid < 0 { - uid = int64(fi.Sys().(*syscall.Stat_t).Uid) - } - if gid < 0 { - gid = int64(fi.Sys().(*syscall.Stat_t).Gid) - } + uid, gid = DetermineTargetFileOwnership(fi, uid, gid) return false, CreateFile(dest, srcFile, fi.Mode(), uint32(uid), uint32(gid)) } diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 109f2337a..ad0d935a2 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -963,7 +963,7 @@ func Test_CopyFile_skips_self(t *testing.T) { t.Fatal(err) } - ignored, err := CopyFile(tempFile, tempFile, "", -1, -1) + ignored, err := CopyFile(tempFile, tempFile, "", DoNotChangeUID, DoNotChangeGID) if err != nil { t.Fatal(err) } From 56125ff464219077beb512736670474288331e35 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 31 Jan 2020 13:48:14 -0800 Subject: [PATCH 4/5] Rebase from master with some changes --- pkg/commands/run.go | 13 +------------ pkg/util/command_util.go | 22 ++++++++++++++++++---- pkg/util/fs_util_test.go | 6 ++++++ 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 73ea4ca94..409060443 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -71,18 +71,7 @@ func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui var userStr string // If specified, run the command as a specific user if config.User != "" { - userAndGroup := strings.Split(config.User, ":") - userStr = userAndGroup[0] - var groupStr string - if len(userAndGroup) > 1 { - groupStr = userAndGroup[1] - } - - uidStr, gidStr, err := util.GetUserFromUsername(userStr, groupStr) - if err !=nil{ - return err - } - uid, gid, err := util.GetUIDAndGIDFromString(uidStr, gidStr) + uid, gid, err := util.GetUIDAndGIDFromString(config.User, false) if err != nil { return err } diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index 2121f4055..c9ba9f4cf 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -328,9 +328,20 @@ Loop: } // Extract user and group id from a string formatted 'user:group'. -// If gidEqualToUIDIfNotGiven is set, the gid is equal to uid if the group is not specified +// If fallbackToUID is set, the gid is equal to uid if the group is not specified // otherwise gid is set to zero. -func GetUIDAndGIDFromString(uidStr string, gidStr string) (uint32, uint32, error) { +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] + } + + uidStr, gidStr, err := GetUserFromUsername(userStr, groupStr, fallbackToUID) + if err != nil { + return 0, 0, err + } // uid and gid need to be fit into uint32 uid64, err := strconv.ParseUint(uidStr, 10, 32) if err != nil { @@ -343,7 +354,7 @@ func GetUIDAndGIDFromString(uidStr string, gidStr string) (uint32, uint32, error return uint32(uid64), uint32(gid64), nil } -func GetUserFromUsername(userStr string, groupStr string) (string, string, error) { +func GetUserFromUsername(userStr string, groupStr string, fallbackToUID bool) (string, string, error) { // Lookup by username userObj, err := Lookup(userStr) if err != nil { @@ -366,7 +377,10 @@ func GetUserFromUsername(userStr string, groupStr string) (string, string, error } uid := userObj.Uid - gid := userObj.Gid + gid := "" + if fallbackToUID { + gid = userObj.Gid + } if group != nil { gid = group.Gid } diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index ad0d935a2..361915350 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -1307,6 +1307,12 @@ func TestUpdateWhitelist(t *testing.T) { t.Run(tt.name, func(t *testing.T) { whitelist = initialWhitelist defer func() { whitelist = initialWhitelist }() + sort.Slice(tt.expected, func(i, j int) bool { + return tt.expected[i].Path < tt.expected[j].Path + }) + sort.Slice(whitelist, func(i, j int) bool { + return whitelist[i].Path < whitelist[j].Path + }) UpdateWhitelist(tt.whitelistVarRun) sort.Slice(tt.expected, func(i, j int) bool { return tt.expected[i].Path < tt.expected[j].Path From 31f626cb22507580b3728a031b0f0587c608a8ac Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Mon, 3 Feb 2020 15:23:16 -0800 Subject: [PATCH 5/5] initilize group to 0 --- pkg/util/command_util.go | 2 +- pkg/util/command_util_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index c9ba9f4cf..23f035ec4 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -377,7 +377,7 @@ func GetUserFromUsername(userStr string, groupStr string, fallbackToUID bool) (s } uid := userObj.Uid - gid := "" + gid := "0" if fallbackToUID { gid = userObj.Gid } diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index b5895e9a6..472646388 100644 --- a/pkg/util/command_util_test.go +++ b/pkg/util/command_util_test.go @@ -551,12 +551,12 @@ func Test_GetUIDAndGIDFromString(t *testing.T) { fmt.Sprintf("%s:%s", currentUser.Uid, primaryGroup), fmt.Sprintf("%s:%s", currentUser.Username, primaryGroup), } - expectedUid, _ := strconv.ParseUint(currentUser.Uid, 10, 32) - expectedGid, _ := strconv.ParseUint(currentUser.Gid, 10, 32) + 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(expectedUid) || gid != uint32(expectedGid) || err != nil { - t.Errorf("Could not correctly decode %s to uid/gid %d:%d. Result: %d:%d", tt, expectedUid, expectedGid, + 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) } }