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: containers#9387 Signed-off-by: Daniel J Walsh <[email protected]>
rhatdan · Feb 16, 2021 · 3d50393 · 3d50393
1 parent df8ba7f
commit 3d50393
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 10 deletions.
diff --git 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"
@@ -203,10 +204,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())
 			}
@@ -216,19 +224,19 @@ 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
 		}
 		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
@@ -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))