From 538ced1258b6fcf698e9fee18f98bd4f534c674d Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Mon, 15 Feb 2021 16:22:44 -0500 Subject: [PATCH] Don't chown workdir if it already exists Currently podman is always chowning the WORKDIR to root:root This PR will return if the WORKDIR already exists. Fixes: https://github.com/containers/podman/issues/9387 Signed-off-by: Daniel J Walsh --- libpod/container_internal_linux.go | 26 +++++++++++++++++--------- test/e2e/run_working_dir_test.go | 6 +++++- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 8a55f01b15..a45eae45fc 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -21,6 +21,7 @@ import ( cnitypes "github.com/containernetworking/cni/pkg/types/current" "github.com/containernetworking/plugins/pkg/ns" + "github.com/containers/buildah/pkg/chrootuser" "github.com/containers/buildah/pkg/overlay" "github.com/containers/common/pkg/apparmor" "github.com/containers/common/pkg/config" @@ -202,10 +203,17 @@ func (c *Container) resolveWorkDir() error { } logrus.Debugf("Workdir %q resolved to host path %q", workdir, resolvedWorkdir) - // No need to create it (e.g., `--workdir=/foo`), so let's make sure - // the path exists on the container. + st, err := os.Stat(resolvedWorkdir) + if err == nil { + if !st.IsDir() { + return errors.Errorf("workdir %q exists on container %s, but is not a directory", workdir, c.ID()) + } + return nil + } if !c.config.CreateWorkingDir { - if _, err := os.Stat(resolvedWorkdir); err != nil { + // No need to create it (e.g., `--workdir=/foo`), so let's make sure + // the path exists on the container. + if err != nil { if os.IsNotExist(err) { return errors.Errorf("workdir %q does not exist on container %s", workdir, c.ID()) } @@ -215,11 +223,6 @@ func (c *Container) resolveWorkDir() error { } return nil } - - // Ensure container entrypoint is created (if required). - rootUID := c.RootUID() - rootGID := c.RootGID() - if err := os.MkdirAll(resolvedWorkdir, 0755); err != nil { if os.IsExist(err) { return nil @@ -227,7 +230,12 @@ func (c *Container) resolveWorkDir() error { return errors.Wrapf(err, "error creating container %s workdir", c.ID()) } - if err := os.Chown(resolvedWorkdir, rootUID, rootGID); err != nil { + // Ensure container entrypoint is created (if required). + uid, gid, _, err := chrootuser.GetUser(c.state.Mountpoint, c.User()) + if err != nil { + return errors.Wrapf(err, "error looking up %s inside of the container %s", c.User(), c.ID()) + } + if err := os.Chown(resolvedWorkdir, int(uid), int(gid)); err != nil { return errors.Wrapf(err, "error chowning container %s workdir to container root", c.ID()) } diff --git a/test/e2e/run_working_dir_test.go b/test/e2e/run_working_dir_test.go index 59538448e1..948ed05e7e 100644 --- a/test/e2e/run_working_dir_test.go +++ b/test/e2e/run_working_dir_test.go @@ -47,7 +47,7 @@ var _ = Describe("Podman run", func() { It("podman run a container on an image with a workdir", func() { dockerfile := `FROM alpine -RUN mkdir -p /home/foobar +RUN mkdir -p /home/foobar /etc/foobar; chown bin:bin /etc/foobar WORKDIR /etc/foobar` podmanTest.BuildImage(dockerfile, "test", "false") @@ -56,6 +56,10 @@ WORKDIR /etc/foobar` Expect(session.ExitCode()).To(Equal(0)) Expect(session.OutputToString()).To(Equal("/etc/foobar")) + session = podmanTest.Podman([]string{"run", "test", "ls", "-ld", "."}) + session.WaitWithDefaultTimeout() + Expect(session.LineInOutputContains("bin")).To(BeTrue()) + session = podmanTest.Podman([]string{"run", "--workdir", "/home/foobar", "test", "pwd"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0))