From 61312a95ae1a305f5154f8d88dc58ee1e3259ae8 Mon Sep 17 00:00:00 2001 From: Aris Buzachis Date: Thu, 29 Sep 2022 04:18:40 +0300 Subject: [PATCH] fix(WORKDIR): use the config.User for the new dir permissions (#2269) WORKDIR ignores the currently set USER and creates the new directories with the root user ownership. This changes that, by executing a chown after the mkdir if needed, and also handle the case where the provided USER is an uid and the passwd file is not available to resolve to the username. Fixes #2259 Signed-off-by: Aris Buzachis Signed-off-by: Aris Buzachis --- .../Dockerfile_test_workdir_with_user | 41 +++++++++++++++++++ pkg/commands/workdir.go | 19 +++++++-- pkg/commands/workdir_test.go | 8 ++-- pkg/util/command_util.go | 4 +- pkg/util/fs_util.go | 6 +-- 5 files changed, 66 insertions(+), 12 deletions(-) create mode 100644 integration/dockerfiles/Dockerfile_test_workdir_with_user diff --git a/integration/dockerfiles/Dockerfile_test_workdir_with_user b/integration/dockerfiles/Dockerfile_test_workdir_with_user new file mode 100644 index 000000000..f82b306e1 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_workdir_with_user @@ -0,0 +1,41 @@ +FROM debian:9.11 +RUN groupadd -g 10000 bar +RUN useradd -c "Foo user" -u 10000 -g 10000 -m foo + + +# no passwd file +FROM scratch + +# without a USER Set +WORKDIR /workdir/wo/user + +USER 9999 +WORKDIR /workdir/w/uid + +USER 9999:9999 +WORKDIR /workdir/w/uid_gid + +USER 0 +WORKDIR /workdir/w/root_uid + +USER root +WORKDIR /workdir/w/root_username + +# with passwd file +COPY --from=0 /etc/passwd /etc/passwd +COPY --from=0 /etc/group /etc/group + +USER foo +WORKDIR /workdir/w/foo_username + +USER foo:10000 +WORKDIR /workdir/w/foo_username_gid + +USER 10000 +WORKDIR /workdir/w/foo_uid + +USER 10000:10000 +WORKDIR /workdir/w/foo_uid_gid + +USER 10000:bar +WORKDIR /workdir/w/foo_uid_groupname diff --git a/pkg/commands/workdir.go b/pkg/commands/workdir.go index 4a95424ec..eefa978b8 100644 --- a/pkg/commands/workdir.go +++ b/pkg/commands/workdir.go @@ -21,6 +21,7 @@ import ( "path/filepath" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + "github.com/pkg/errors" "github.com/GoogleContainerTools/kaniko/pkg/util" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -35,7 +36,7 @@ type WorkdirCommand struct { } // For testing -var mkdir = os.MkdirAll +var mkdirAllWithPermissions = util.MkdirAllWithPermissions func (w *WorkdirCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { logrus.Info("Cmd: workdir") @@ -59,9 +60,21 @@ func (w *WorkdirCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile // Only create and snapshot the dir if it didn't exist already w.snapshotFiles = []string{} if _, err := os.Stat(config.WorkingDir); os.IsNotExist(err) { - logrus.Infof("Creating directory %s", config.WorkingDir) + uid, gid := int64(-1), int64(-1) + + if config.User != "" { + logrus.Debugf("Fetching uid and gid for USER '%s'", config.User) + uid, gid, err = util.GetUserGroup(config.User, replacementEnvs) + if err != nil { + return errors.Wrapf(err, "identifying uid and gid for user %s", config.User) + } + } + + logrus.Infof("Creating directory %s with uid %d and gid %d", config.WorkingDir, uid, gid) w.snapshotFiles = append(w.snapshotFiles, config.WorkingDir) - return mkdir(config.WorkingDir, 0755) + if err := mkdirAllWithPermissions(config.WorkingDir, 0755, uid, gid); err != nil { + return errors.Wrapf(err, "creating workdir %s", config.WorkingDir) + } } return nil } diff --git a/pkg/commands/workdir_test.go b/pkg/commands/workdir_test.go index c347c7062..f82f107a7 100644 --- a/pkg/commands/workdir_test.go +++ b/pkg/commands/workdir_test.go @@ -83,17 +83,17 @@ var workdirTests = []struct { } // For testing -func mockDir(p string, fi os.FileMode) error { +func mockDir(path string, mode os.FileMode, uid, gid int64) error { return nil } func TestWorkdirCommand(t *testing.T) { // Mock out mkdir for testing. - oldMkdir := mkdir - mkdir = mockDir + oldMkdir := mkdirAllWithPermissions + mkdirAllWithPermissions = mockDir defer func() { - mkdir = oldMkdir + mkdirAllWithPermissions = oldMkdir }() cfg := &v1.Config{ diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index 728f89dc4..2f09fda65 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -442,8 +442,8 @@ func LookupUser(userStr string) (*user.User, error) { userObj, err := user.Lookup(userStr) if err != nil { unknownUserErr := new(user.UnknownUserError) - // only return if it's not an unknown user error - if !errors.As(err, unknownUserErr) { + // only return if it's not an unknown user error or the passwd file does not exist + if !errors.As(err, unknownUserErr) && !os.IsNotExist(err) { return nil, err } diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index d29fb43b4..b13882eb5 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -348,7 +348,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, int64(uid), int64(gid)); err != nil { + if err := MkdirAllWithPermissions(path, mode, int64(uid), int64(gid)); err != nil { return err } @@ -663,7 +663,7 @@ func CopyDir(src, dest string, context FileContext, uid, gid int64) ([]string, e mode := fi.Mode() uid, gid := DetermineTargetFileOwnership(fi, uid, gid) - if err := mkdirAllWithPermissions(destPath, mode, uid, gid); err != nil { + if err := MkdirAllWithPermissions(destPath, mode, uid, gid); err != nil { return nil, err } } else if IsSymlink(fi) { @@ -806,7 +806,7 @@ func Volumes() []string { return volumes } -func mkdirAllWithPermissions(path string, mode os.FileMode, uid, gid int64) error { +func MkdirAllWithPermissions(path string, mode os.FileMode, uid, gid int64) error { // Check if a file already exists on the path, if yes then delete it info, err := os.Stat(path) if err == nil && !info.IsDir() {