diff options
author | Aditya R <arajan@redhat.com> | 2022-03-02 15:07:06 +0530 |
---|---|---|
committer | Matthew Heon <mheon@redhat.com> | 2022-03-30 13:30:47 -0400 |
commit | 69543d6c7aff9b31d8ad3f151f05267aa34e8e6c (patch) | |
tree | d3e8f79f407907d8549322c03e91a55e8fcbf5c7 | |
parent | 0a839a59ec43ae03c522a1b82224605219256ab1 (diff) | |
download | podman-69543d6c7aff9b31d8ad3f151f05267aa34e8e6c.tar.gz podman-69543d6c7aff9b31d8ad3f151f05267aa34e8e6c.tar.bz2 podman-69543d6c7aff9b31d8ad3f151f05267aa34e8e6c.zip |
container: workdir resolution must consider symlink if explicitly configured
While resolving `workdir` we mostly create a `workdir` when `stat`
fails with `ENOENT` or `ErrNotExist` however following cases are not
true when user explicitly specifies a `workdir` while `running` using
`--workdir` which tells `podman` to only use workdir if its exists on
the container. Following configuration is implicity set with other
`run` mechanism like `podman play kube`
Problem with explicit `--workdir` or similar implicit config in `podman play
kube` is that currently podman ignores the fact that workdir can also be
a `symlink` and actual `link` could be valid.
Hence following commit ensures that in such scenarios when a `workdir`
is not found and we cannot create a `workdir` podman must perform a
check to ensure that if `workdir` is a `symlink` and `link` is resolved
successfully and resolved link is present on the container then we
return as it is.
Docker performs a similar behviour.
Signed-off-by: Aditya R <arajan@redhat.com>
-rw-r--r-- | libpod/container_internal_linux.go | 56 | ||||
-rw-r--r-- | test/e2e/build/workdir-symlink/Dockerfile | 5 | ||||
-rw-r--r-- | test/e2e/build_test.go | 13 | ||||
-rw-r--r-- | test/e2e/play_kube_test.go | 35 |
4 files changed, 109 insertions, 0 deletions
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index afa351c17..f7c8c190f 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -173,6 +173,57 @@ func (c *Container) prepare() error { return nil } +// isWorkDirSymlink returns true if resolved workdir is symlink or a chain of symlinks, +// and final resolved target is present either on volume, mount or inside of container +// otherwise it returns false. Following function is meant for internal use only and +// can change at any point of time. +func (c *Container) isWorkDirSymlink(resolvedPath string) bool { + // We cannot create workdir since explicit --workdir is + // set in config but workdir could also be a symlink. + // If its a symlink lets check if resolved link is present + // on the container or not. + + // If we can resolve symlink and resolved link is present on the container + // then return nil cause its a valid use-case. + + maxSymLinks := 0 + for { + // Linux only supports a chain of 40 links. + // Reference: https://github.com/torvalds/linux/blob/master/include/linux/namei.h#L13 + if maxSymLinks > 40 { + break + } + resolvedSymlink, err := os.Readlink(resolvedPath) + if err != nil { + // End sym-link resolution loop. + break + } + if resolvedSymlink != "" { + _, resolvedSymlinkWorkdir, err := c.resolvePath(c.state.Mountpoint, resolvedSymlink) + if isPathOnVolume(c, resolvedSymlinkWorkdir) || isPathOnBindMount(c, resolvedSymlinkWorkdir) { + // Resolved symlink exists on external volume or mount + return true + } + if err != nil { + // Could not resolve path so end sym-link resolution loop. + break + } + if resolvedSymlinkWorkdir != "" { + resolvedPath = resolvedSymlinkWorkdir + _, err := os.Stat(resolvedSymlinkWorkdir) + if err == nil { + // Symlink resolved successfully and resolved path exists on container, + // this is a valid use-case so return nil. + logrus.Debugf("Workdir is a symlink with target to %q and resolved symlink exists on container", resolvedSymlink) + return true + } + } + } + maxSymLinks++ + } + return false +} + // 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. @@ -205,6 +256,11 @@ func (c *Container) resolveWorkDir() error { // the path exists on the container. if err != nil { if os.IsNotExist(err) { + // If resolved Workdir path gets marked as a valid symlink, + // return nil cause this is valid use-case. + if c.isWorkDirSymlink(resolvedWorkdir) { + return nil + } return errors.Errorf("workdir %q does not exist on container %s", workdir, c.ID()) } // This might be a serious error (e.g., permission), so diff --git a/test/e2e/build/workdir-symlink/Dockerfile b/test/e2e/build/workdir-symlink/Dockerfile new file mode 100644 index 000000000..abc9b47ee --- /dev/null +++ b/test/e2e/build/workdir-symlink/Dockerfile @@ -0,0 +1,5 @@ +FROM alpine +RUN mkdir /tmp/destination +RUN ln -s /tmp/destination /tmp/link +WORKDIR /tmp/link +CMD ["echo", "hello"] diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index a1c2f5e54..14fa12fa2 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -259,6 +259,19 @@ var _ = Describe("Podman build", func() { Expect(session.OutputToString()).NotTo(ContainSubstring("io.podman.annotations.seccomp")) }) + It("podman build where workdir is a symlink and run without creating new workdir", func() { + session := podmanTest.Podman([]string{ + "build", "-f", "build/workdir-symlink/Dockerfile", "-t", "test-symlink", + }) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + session = podmanTest.Podman([]string{"run", "--workdir", "/tmp/link", "test-symlink"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(session.OutputToString()).To(ContainSubstring("hello")) + }) + It("podman build --http_proxy flag", func() { os.Setenv("http_proxy", "1.2.3.4") if IsRemote() { diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 48bc2b9da..f0f0c6dc4 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -38,6 +38,21 @@ spec: hostname: unknown ` +var workdirSymlinkPodYaml = ` +apiVersion: v1 +kind: Pod +metadata: + labels: + app: test-symlink + name: test-symlink +spec: + containers: + - image: test-symlink + name: test-symlink + resources: {} + restartPolicy: Never +` + var podnameEqualsContainerNameYaml = ` apiVersion: v1 kind: Pod @@ -1332,6 +1347,26 @@ var _ = Describe("Podman play kube", func() { Expect(sharednamespaces).To(ContainSubstring("pid")) }) + It("podman play kube should be able to run image where workdir is a symlink", func() { + session := podmanTest.Podman([]string{ + "build", "-f", "build/workdir-symlink/Dockerfile", "-t", "test-symlink", + }) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + err := writeYaml(workdirSymlinkPodYaml, kubeYaml) + Expect(err).To(BeNil()) + + kube := podmanTest.Podman([]string{"play", "kube", kubeYaml}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + logs := podmanTest.Podman([]string{"pod", "logs", "-c", "test-symlink-test-symlink", "test-symlink"}) + logs.WaitWithDefaultTimeout() + Expect(logs).Should(Exit(0)) + Expect(logs.OutputToString()).To(ContainSubstring("hello")) + }) + It("podman play kube should not rename pod if container in pod has same name", func() { err := writeYaml(podnameEqualsContainerNameYaml, kubeYaml) Expect(err).To(BeNil()) |