Skip to content

Commit

Permalink
container: workdir resolution must consider symlink if explicitly con…
Browse files Browse the repository at this point in the history
…figured

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 <[email protected]>
  • Loading branch information
flouthoc authored and mheon committed Mar 30, 2022
1 parent 0a839a5 commit 69543d6
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 0 deletions.
56 changes: 56 additions & 0 deletions libpod/container_internal_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/build/workdir-symlink/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
FROM alpine
RUN mkdir /tmp/destination
RUN ln -s /tmp/destination /tmp/link
WORKDIR /tmp/link
CMD ["echo", "hello"]
13 changes: 13 additions & 0 deletions test/e2e/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
35 changes: 35 additions & 0 deletions test/e2e/play_kube_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down

0 comments on commit 69543d6

Please sign in to comment.