diff options
| -rw-r--r-- | libpod/container_internal_linux.go | 63 | ||||
| -rw-r--r-- | libpod/container_path_resolution.go | 47 | ||||
| -rw-r--r-- | pkg/specgen/generate/container.go | 14 | ||||
| -rw-r--r-- | pkg/specgen/generate/container_create.go | 12 | ||||
| -rw-r--r-- | test/e2e/run_working_dir_test.go | 6 | ||||
| -rw-r--r-- | test/system/030-run.bats | 21 | ||||
| -rw-r--r-- | test/system/070-build.bats | 4 |
7 files changed, 127 insertions, 40 deletions
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index b41a3fa38..6c9489a08 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -174,25 +174,60 @@ func (c *Container) prepare() error { return err } - // Ensure container entrypoint is created (if required) - if c.config.CreateWorkingDir { - workdir, err := securejoin.SecureJoin(c.state.Mountpoint, c.WorkingDir()) - if err != nil { - return errors.Wrapf(err, "error creating path to container %s working dir", c.ID()) - } - rootUID := c.RootUID() - rootGID := c.RootGID() + // Make sure the workdir exists + if err := c.resolveWorkDir(); err != nil { + return err + } - if err := os.MkdirAll(workdir, 0755); err != nil { - if os.IsExist(err) { - return nil + return nil +} + +// resolveWorkDir resolves the container's workdir and, depending on the +// configuration, will create it, or error out if it does not exist. +// Note that the container must be mounted before. +func (c *Container) resolveWorkDir() error { + workdir := c.WorkingDir() + + // If the specified workdir is a subdir of a volume or mount, + // we don't need to do anything. The runtime is taking care of + // that. + if isPathOnVolume(c, workdir) || isPathOnBindMount(c, workdir) { + logrus.Debugf("Workdir %q resolved to a volume or mount", workdir) + return nil + } + + _, resolvedWorkdir, err := c.resolvePath(c.state.Mountpoint, workdir) + if err != nil { + return err + } + 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. + if !c.config.CreateWorkingDir { + if _, err := os.Stat(resolvedWorkdir); err != nil { + if os.IsNotExist(err) { + return errors.Errorf("workdir %q does not exist on container %s", workdir, c.ID()) } - return errors.Wrapf(err, "error creating container %s working dir", c.ID()) + // This might be a serious error (e.g., permission), so + // we need to return the full error. + return errors.Wrapf(err, "error detecting workdir %q on container %s", workdir, c.ID()) } + } + + // Ensure container entrypoint is created (if required). + rootUID := c.RootUID() + rootGID := c.RootGID() - if err := os.Chown(workdir, rootUID, rootGID); err != nil { - return errors.Wrapf(err, "error chowning container %s working directory to container root", c.ID()) + 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 { + return errors.Wrapf(err, "error chowning container %s workdir to container root", c.ID()) } return nil diff --git a/libpod/container_path_resolution.go b/libpod/container_path_resolution.go index 68f174278..805b3b947 100644 --- a/libpod/container_path_resolution.go +++ b/libpod/container_path_resolution.go @@ -13,6 +13,11 @@ import ( // resolveContainerPaths resolves the container's mount point and the container // path as specified by the user. Both may resolve to paths outside of the // container's mount point when the container path hits a volume or bind mount. +// +// It returns a bool, indicating whether containerPath resolves outside of +// mountPoint (e.g., via a mount or volume), the resolved root (e.g., container +// mount, bind mount or volume) and the resolved path on the root (absolute to +// the host). func (container *Container) resolvePath(mountPoint string, containerPath string) (string, string, error) { // Let's first make sure we have a path relative to the mount point. pathRelativeToContainerMountPoint := containerPath @@ -97,8 +102,8 @@ func (container *Container) resolvePath(mountPoint string, containerPath string) return mountPoint, resolvedPathOnTheContainerMountPoint, nil } -// findVolume checks if the specified container path matches a volume inside -// the container. It returns a matching volume or nil. +// findVolume checks if the specified containerPath matches the destination +// path of a Volume. Returns a matching Volume or nil. func findVolume(c *Container, containerPath string) (*Volume, error) { runtime := c.Runtime() cleanedContainerPath := filepath.Clean(containerPath) @@ -110,8 +115,25 @@ func findVolume(c *Container, containerPath string) (*Volume, error) { return nil, nil } -// findBindMount checks if the specified container path matches a bind mount -// inside the container. It returns a matching mount or nil. +// isPathOnVolume returns true if the specified containerPath is a subdir of any +// Volume's destination. +func isPathOnVolume(c *Container, containerPath string) bool { + cleanedContainerPath := filepath.Clean(containerPath) + for _, vol := range c.Config().NamedVolumes { + if cleanedContainerPath == filepath.Clean(vol.Dest) { + return true + } + for dest := vol.Dest; dest != "/"; dest = filepath.Dir(dest) { + if cleanedContainerPath == dest { + return true + } + } + } + return false +} + +// findBindMounts checks if the specified containerPath matches the destination +// path of a Mount. Returns a matching Mount or nil. func findBindMount(c *Container, containerPath string) *specs.Mount { cleanedPath := filepath.Clean(containerPath) for _, m := range c.Config().Spec.Mounts { @@ -125,3 +147,20 @@ func findBindMount(c *Container, containerPath string) *specs.Mount { } return nil } + +/// isPathOnBindMount returns true if the specified containerPath is a subdir of any +// Mount's destination. +func isPathOnBindMount(c *Container, containerPath string) bool { + cleanedContainerPath := filepath.Clean(containerPath) + for _, m := range c.Config().Spec.Mounts { + if cleanedContainerPath == filepath.Clean(m.Destination) { + return true + } + for dest := m.Destination; dest != "/"; dest = filepath.Dir(dest) { + if cleanedContainerPath == dest { + return true + } + } + } + return false +} diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go index cc3f7928c..31d317bf8 100644 --- a/pkg/specgen/generate/container.go +++ b/pkg/specgen/generate/container.go @@ -203,20 +203,6 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat } s.Annotations = annotations - // workdir - if s.WorkDir == "" { - if newImage != nil { - workingDir, err := newImage.WorkingDir(ctx) - if err != nil { - return nil, err - } - s.WorkDir = workingDir - } - } - if s.WorkDir == "" { - s.WorkDir = "/" - } - if len(s.SeccompProfilePath) < 1 { p, err := libpod.DefaultSeccompPath() if err != nil { diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index 4f36744ca..1bc050b00 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -272,10 +272,18 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. if s.Entrypoint != nil { options = append(options, libpod.WithEntrypoint(s.Entrypoint)) } - // If the user did not set an workdir but the image did, ensure it is - // created. + // If the user did not specify a workdir on the CLI, let's extract it + // from the image. if s.WorkDir == "" && img != nil { options = append(options, libpod.WithCreateWorkingDir()) + wd, err := img.WorkingDir(ctx) + if err != nil { + return nil, err + } + s.WorkDir = wd + } + if s.WorkDir == "" { + s.WorkDir = "/" } if s.StopSignal != nil { options = append(options, libpod.WithStopSignal(*s.StopSignal)) diff --git a/test/e2e/run_working_dir_test.go b/test/e2e/run_working_dir_test.go index 7d8db361c..59538448e 100644 --- a/test/e2e/run_working_dir_test.go +++ b/test/e2e/run_working_dir_test.go @@ -2,7 +2,6 @@ package integration import ( "os" - "strings" . "github.com/containers/podman/v2/test/utils" . "github.com/onsi/ginkgo" @@ -41,12 +40,9 @@ var _ = Describe("Podman run", func() { }) It("podman run a container using non existing --workdir", func() { - if !strings.Contains(podmanTest.OCIRuntime, "crun") { - Skip("Test only works on crun") - } session := podmanTest.Podman([]string{"run", "--workdir", "/home/foobar", ALPINE, "pwd"}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(Equal(127)) + Expect(session.ExitCode()).To(Equal(126)) }) It("podman run a container on an image with a workdir", func() { diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 29dc95dc3..dcf1da370 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -589,4 +589,25 @@ json-file | f is "${lines[1]}" "$rand" "Container runs successfully despite warning" } +@test "podman run - check workdir" { + # Workdirs specified via the CLI are not created on the root FS. + run_podman 126 run --rm --workdir /i/do/not/exist $IMAGE pwd + # Note: remote error prepends an attach error. + is "$output" "Error: .*workdir \"/i/do/not/exist\" does not exist on container.*" + + testdir=$PODMAN_TMPDIR/volume + mkdir -p $testdir + randomcontent=$(random_string 10) + echo "$randomcontent" > $testdir/content + + # Workdir does not exist on the image but is volume mounted. + run_podman run --rm --workdir /IamNotOnTheImage -v $testdir:/IamNotOnTheImage $IMAGE cat content + is "$output" "$randomcontent" "cat random content" + + # Workdir does not exist on the image but is created by the runtime as it's + # a subdir of a volume. + run_podman run --rm --workdir /IamNotOntheImage -v $testdir/content:/IamNotOntheImage/foo $IMAGE cat foo + is "$output" "$randomcontent" "cat random content" +} + # vim: filetype=sh diff --git a/test/system/070-build.bats b/test/system/070-build.bats index 9e5e3ee1d..cee28658f 100644 --- a/test/system/070-build.bats +++ b/test/system/070-build.bats @@ -163,10 +163,12 @@ EOF https_proxy=https-proxy-in-env-file EOF + # NOTE: it's important to not create the workdir. + # Podman will make sure to create a missing workdir + # if needed. See #9040. cat >$tmpdir/Containerfile <<EOF FROM $IMAGE LABEL $label_name=$label_value -RUN mkdir $workdir WORKDIR $workdir # Test for #7094 - chowning of invalid symlinks |