From 5747fd71c98238d42cf9cd307fdf4d59747a6e1d Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Tue, 26 Jan 2021 15:38:05 +0100
Subject: [PATCH 01/15] remote exec: write conmon error on hijacked connection

Make sure to write error from conmon on the hijacked http connection.
This fixes issues where errors were not reported on the client side,
for instance, when specified command was not found on the container.

To future generations: I am sorry.  The code is complex, and there are
many interdependencies among the concurrent goroutines.  I added more
complexity on top but I don't have a good idea of how to reduce
complexity in the available time.

Fixes: #8281
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 libpod/oci_conmon_exec_linux.go | 48 ++++++++++++++++++++++++++-------
 libpod/util.go                  | 17 +++++++-----
 test/system/075-exec.bats       |  2 --
 3 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go
index dc5dd03df4..faf86ea5b5 100644
--- a/libpod/oci_conmon_exec_linux.go
+++ b/libpod/oci_conmon_exec_linux.go
@@ -126,20 +126,25 @@ func (r *ConmonOCIRuntime) ExecContainerHTTP(ctr *Container, sessionID string, o
 	}()
 
 	attachChan := make(chan error)
+	conmonPipeDataChan := make(chan conmonPipeData)
 	go func() {
 		// attachToExec is responsible for closing pipes
-		attachChan <- attachExecHTTP(ctr, sessionID, req, w, streams, pipes, detachKeys, options.Terminal, cancel, hijackDone, holdConnOpen)
+		attachChan <- attachExecHTTP(ctr, sessionID, req, w, streams, pipes, detachKeys, options.Terminal, cancel, hijackDone, holdConnOpen, execCmd, conmonPipeDataChan, ociLog)
 		close(attachChan)
 	}()
 
-	// Wait for conmon to succeed, when return.
-	if err := execCmd.Wait(); err != nil {
-		return -1, nil, errors.Wrapf(err, "cannot run conmon")
-	}
+	// NOTE: the channel is needed to communicate conmon's data.  In case
+	// of an error, the error will be written on the hijacked http
+	// connection such that remote clients will receive the error.
+	pipeData := <-conmonPipeDataChan
 
-	pid, err := readConmonPipeData(pipes.syncPipe, ociLog)
+	return pipeData.pid, attachChan, pipeData.err
+}
 
-	return pid, attachChan, err
+// conmonPipeData contains the data when reading from conmon's pipe.
+type conmonPipeData struct {
+	pid int
+	err error
 }
 
 // ExecContainerDetached executes a command in a running container, but does
@@ -488,9 +493,16 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex
 }
 
 // Attach to a container over HTTP
-func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, pipes *execPipes, detachKeys []byte, isTerminal bool, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool) (deferredErr error) {
+func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, pipes *execPipes, detachKeys []byte, isTerminal bool, cancel <-chan bool, hijackDone chan<- bool, holdConnOpen <-chan bool, execCmd *exec.Cmd, conmonPipeDataChan chan<- conmonPipeData, ociLog string) (deferredErr error) {
+	// NOTE: As you may notice, the attach code is quite complex.
+	// Many things happen concurrently and yet are interdependent.
+	// If you ever change this function, make sure to write to the
+	// conmonPipeDataChan in case of an error.
+
 	if pipes == nil || pipes.startPipe == nil || pipes.attachPipe == nil {
-		return errors.Wrapf(define.ErrInvalidArg, "must provide a start and attach pipe to finish an exec attach")
+		err := errors.Wrapf(define.ErrInvalidArg, "must provide a start and attach pipe to finish an exec attach")
+		conmonPipeDataChan <- conmonPipeData{-1, err}
+		return err
 	}
 
 	defer func() {
@@ -509,17 +521,20 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp
 	// set up the socket path, such that it is the correct length and location for exec
 	sockPath, err := c.execAttachSocketPath(sessionID)
 	if err != nil {
+		conmonPipeDataChan <- conmonPipeData{-1, err}
 		return err
 	}
 
 	// 2: read from attachFd that the parent process has set up the console socket
 	if _, err := readConmonPipeData(pipes.attachPipe, ""); err != nil {
+		conmonPipeDataChan <- conmonPipeData{-1, err}
 		return err
 	}
 
 	// 2: then attach
 	conn, err := openUnixSocket(sockPath)
 	if err != nil {
+		conmonPipeDataChan <- conmonPipeData{-1, err}
 		return errors.Wrapf(err, "failed to connect to container's attach socket: %v", sockPath)
 	}
 	defer func() {
@@ -540,11 +555,13 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp
 	// Perform hijack
 	hijacker, ok := w.(http.Hijacker)
 	if !ok {
+		conmonPipeDataChan <- conmonPipeData{-1, err}
 		return errors.Errorf("unable to hijack connection")
 	}
 
 	httpCon, httpBuf, err := hijacker.Hijack()
 	if err != nil {
+		conmonPipeDataChan <- conmonPipeData{-1, err}
 		return errors.Wrapf(err, "error hijacking connection")
 	}
 
@@ -555,10 +572,23 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp
 
 	// Force a flush after the header is written.
 	if err := httpBuf.Flush(); err != nil {
+		conmonPipeDataChan <- conmonPipeData{-1, err}
 		return errors.Wrapf(err, "error flushing HTTP hijack header")
 	}
 
 	go func() {
+		// Wait for conmon to succeed, when return.
+		if err := execCmd.Wait(); err != nil {
+			conmonPipeDataChan <- conmonPipeData{-1, err}
+		} else {
+			pid, err := readConmonPipeData(pipes.syncPipe, ociLog)
+			if err != nil {
+				hijackWriteError(err, c.ID(), isTerminal, httpBuf)
+				conmonPipeDataChan <- conmonPipeData{pid, err}
+			} else {
+				conmonPipeDataChan <- conmonPipeData{pid, err}
+			}
+		}
 		// We need to hold the connection open until the complete exec
 		// function has finished. This channel will be closed in a defer
 		// in that function, so we can wait for it here.
diff --git a/libpod/util.go b/libpod/util.go
index bf9bf25427..391208fb90 100644
--- a/libpod/util.go
+++ b/libpod/util.go
@@ -235,20 +235,16 @@ func checkDependencyContainer(depCtr, ctr *Container) error {
 	return nil
 }
 
-// hijackWriteErrorAndClose writes an error to a hijacked HTTP session and
-// closes it. Intended to HTTPAttach function.
-// If error is nil, it will not be written; we'll only close the connection.
-func hijackWriteErrorAndClose(toWrite error, cid string, terminal bool, httpCon io.Closer, httpBuf *bufio.ReadWriter) {
+// hijackWriteError writes an error to a hijacked HTTP session.
+func hijackWriteError(toWrite error, cid string, terminal bool, httpBuf *bufio.ReadWriter) {
 	if toWrite != nil {
-		errString := []byte(fmt.Sprintf("%v\n", toWrite))
+		errString := []byte(fmt.Sprintf("Error: %v\n", toWrite))
 		if !terminal {
 			// We need a header.
 			header := makeHTTPAttachHeader(2, uint32(len(errString)))
 			if _, err := httpBuf.Write(header); err != nil {
 				logrus.Errorf("Error writing header for container %s attach connection error: %v", cid, err)
 			}
-			// TODO: May want to return immediately here to avoid
-			// writing garbage to the socket?
 		}
 		if _, err := httpBuf.Write(errString); err != nil {
 			logrus.Errorf("Error writing error to container %s HTTP attach connection: %v", cid, err)
@@ -257,6 +253,13 @@ func hijackWriteErrorAndClose(toWrite error, cid string, terminal bool, httpCon
 			logrus.Errorf("Error flushing HTTP buffer for container %s HTTP attach connection: %v", cid, err)
 		}
 	}
+}
+
+// hijackWriteErrorAndClose writes an error to a hijacked HTTP session and
+// closes it. Intended to HTTPAttach function.
+// If error is nil, it will not be written; we'll only close the connection.
+func hijackWriteErrorAndClose(toWrite error, cid string, terminal bool, httpCon io.Closer, httpBuf *bufio.ReadWriter) {
+	hijackWriteError(toWrite, cid, terminal, httpBuf)
 
 	if err := httpCon.Close(); err != nil {
 		logrus.Errorf("Error closing container %s HTTP attach connection: %v", cid, err)
diff --git a/test/system/075-exec.bats b/test/system/075-exec.bats
index c028e16c98..badf44c490 100644
--- a/test/system/075-exec.bats
+++ b/test/system/075-exec.bats
@@ -6,8 +6,6 @@
 load helpers
 
 @test "podman exec - basic test" {
-    skip_if_remote "FIXME: pending #7241"
-
     rand_filename=$(random_string 20)
     rand_content=$(random_string 50)
 

From bb88db783c1756291e8047a1a7ffd214477f7f9b Mon Sep 17 00:00:00 2001
From: Matthew Heon <mheon@redhat.com>
Date: Tue, 19 Jan 2021 13:41:50 -0500
Subject: [PATCH 02/15] Ensure the Volumes field in Compat Create is honored

Docker has, for unclear reasons, three separate fields in their
Create Container struct in which volumes can be placed. Right now
we support two of those - Binds and Mounts, which (roughly)
correspond to `-v` and `--mount` respectively. Unfortunately, we
did not support the third, `Volumes`, which is used for anonymous
named volumes created by `-v` (e.g. `-v /test`). It seems that
volumes listed here are *not* included in the remaining two from
my investigation, so it should be safe to just append them into
our handling of the `Binds` (`-v`) field.

Fixes #8649

Signed-off-by: Matthew Heon <mheon@redhat.com>
---
 cmd/podman/common/create_opts.go | 26 ++++++++++++++++++++++++--
 test/apiv2/20-containers.at      |  9 +++++++++
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go
index f252618ceb..f3918d233c 100644
--- a/cmd/podman/common/create_opts.go
+++ b/cmd/podman/common/create_opts.go
@@ -3,6 +3,7 @@ package common
 import (
 	"fmt"
 	"net"
+	"path/filepath"
 	"strconv"
 	"strings"
 
@@ -383,8 +384,29 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, cgroup
 	}
 
 	// volumes
-	if volumes := cc.HostConfig.Binds; len(volumes) > 0 {
-		cliOpts.Volume = volumes
+	volDestinations := make(map[string]bool)
+	for _, vol := range cc.HostConfig.Binds {
+		cliOpts.Volume = append(cliOpts.Volume, vol)
+		// Extract the destination so we don't add duplicate mounts in
+		// the volumes phase.
+		splitVol := strings.SplitN(vol, ":", 3)
+		switch len(splitVol) {
+		case 1:
+			volDestinations[vol] = true
+		default:
+			volDestinations[splitVol[1]] = true
+		}
+	}
+	// Anonymous volumes are added differently from other volumes, in their
+	// own special field, for reasons known only to Docker. Still use the
+	// format of `-v` so we can just append them in there.
+	// Unfortunately, these may be duplicates of existing mounts in Binds.
+	// So... We need to catch that.
+	for vol := range cc.Volumes {
+		if _, ok := volDestinations[filepath.Clean(vol)]; ok {
+			continue
+		}
+		cliOpts.Volume = append(cliOpts.Volume, vol)
 	}
 	if len(cc.HostConfig.BlkioWeightDevice) > 0 {
 		devices := make([]string, 0, len(cc.HostConfig.BlkioWeightDevice))
diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at
index decdc4754d..0da196e46f 100644
--- a/test/apiv2/20-containers.at
+++ b/test/apiv2/20-containers.at
@@ -237,3 +237,12 @@ t GET containers/$cid/json 200 \
 t DELETE containers/$cid 204
 t DELETE images/${MultiTagName}?force=true 200
 # vim: filetype=sh
+
+# Test Volumes field adds an anonymous volume
+t POST containers/create '"Image":"'$IMAGE'","Volumes":{"/test":{}}' 201 \
+  .Id~[0-9a-f]\\{64\\}
+cid=$(jq -r '.Id' <<<"$output")
+t GET containers/$cid/json 200 \
+  .Mounts[0].Destination="/test"
+
+t DELETE containers/$cid?v=true 204

From b4aa54c7561f7093f795093a72c75248018f3517 Mon Sep 17 00:00:00 2001
From: Paul Holzinger <paul.holzinger@web.de>
Date: Wed, 27 Jan 2021 10:42:22 +0100
Subject: [PATCH 03/15] Fix podman history --no-trunc for the CREATED BY field

Fixes #9120

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
---
 cmd/podman/images/history.go |  2 +-
 test/e2e/history_test.go     | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/cmd/podman/images/history.go b/cmd/podman/images/history.go
index 964c7a9752..af40dd73a7 100644
--- a/cmd/podman/images/history.go
+++ b/cmd/podman/images/history.go
@@ -162,7 +162,7 @@ func (h historyReporter) Size() string {
 }
 
 func (h historyReporter) CreatedBy() string {
-	if len(h.ImageHistoryLayer.CreatedBy) > 45 {
+	if !opts.noTrunc && len(h.ImageHistoryLayer.CreatedBy) > 45 {
 		return h.ImageHistoryLayer.CreatedBy[:45-3] + "..."
 	}
 	return h.ImageHistoryLayer.CreatedBy
diff --git a/test/e2e/history_test.go b/test/e2e/history_test.go
index fea3f4d43f..1c57c60de8 100644
--- a/test/e2e/history_test.go
+++ b/test/e2e/history_test.go
@@ -65,6 +65,23 @@ var _ = Describe("Podman history", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		Expect(len(session.OutputToStringArray())).To(BeNumerically(">", 0))
+
+		session = podmanTest.Podman([]string{"history", "--no-trunc", "--format", "{{.ID}}", ALPINE})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		lines := session.OutputToStringArray()
+		Expect(len(lines)).To(BeNumerically(">", 0))
+		// the image id must be 64 chars long
+		Expect(len(lines[0])).To(BeNumerically("==", 64))
+
+		session = podmanTest.Podman([]string{"history", "--no-trunc", "--format", "{{.CreatedBy}}", ALPINE})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		lines = session.OutputToStringArray()
+		Expect(len(lines)).To(BeNumerically(">", 0))
+		Expect(session.OutputToString()).ToNot(ContainSubstring("..."))
+		// the second line in the alpine history contains a command longer than 45 chars
+		Expect(len(lines[1])).To(BeNumerically(">", 45))
 	})
 
 	It("podman history with json flag", func() {

From 2393dd304de3453fabfb7eac99ef6b2fb308bd29 Mon Sep 17 00:00:00 2001
From: baude <bbaude@redhat.com>
Date: Mon, 25 Jan 2021 13:58:04 -0600
Subject: [PATCH 04/15] Add default net info in container inspect

when inspecting a container that is only connected to the default
network, we should populate the default network in the container inspect
information.

Fixes: #6618

Signed-off-by: baude <bbaude@redhat.com>

MH: Small fixes, added another test

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
---
 libpod/networking_linux.go |  8 +++++---
 test/e2e/inspect_test.go   | 23 +++++++++++++++++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index addf1814c4..94b8563feb 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -955,7 +955,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e
 	if c.state.NetNS == nil {
 		// We still want to make dummy configurations for each CNI net
 		// the container joined.
-		if len(networks) > 0 && !isDefault {
+		if len(networks) > 0 {
 			settings.Networks = make(map[string]*define.InspectAdditionalNetwork, len(networks))
 			for _, net := range networks {
 				cniNet := new(define.InspectAdditionalNetwork)
@@ -976,7 +976,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e
 	}
 
 	// If we have CNI networks - handle that here
-	if len(networks) > 0 && !isDefault {
+	if len(networks) > 0 {
 		if len(networks) != len(c.state.NetworkStatus) {
 			return nil, errors.Wrapf(define.ErrInternal, "network inspection mismatch: asked to join %d CNI network(s) %v, but have information on %d network(s)", len(networks), networks, len(c.state.NetworkStatus))
 		}
@@ -1006,7 +1006,9 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e
 			settings.Networks[name] = addedNet
 		}
 
-		return settings, nil
+		if !isDefault {
+			return settings, nil
+		}
 	}
 
 	// If not joining networks, we should have at most 1 result
diff --git a/test/e2e/inspect_test.go b/test/e2e/inspect_test.go
index 97f77414eb..8fc9721f9a 100644
--- a/test/e2e/inspect_test.go
+++ b/test/e2e/inspect_test.go
@@ -443,4 +443,27 @@ var _ = Describe("Podman inspect", func() {
 		Expect(inspect.OutputToString()).To(Equal(`"{"80/tcp":[{"HostIp":"","HostPort":"8080"}]}"`))
 	})
 
+	It("Verify container inspect has default network", func() {
+		SkipIfRootless("Requires root CNI networking")
+		ctrName := "testctr"
+		session := podmanTest.Podman([]string{"run", "-d", "--name", ctrName, ALPINE, "top"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+
+		inspect := podmanTest.InspectContainer(ctrName)
+		Expect(len(inspect)).To(Equal(1))
+		Expect(len(inspect[0].NetworkSettings.Networks)).To(Equal(1))
+	})
+
+	It("Verify stopped container still has default network in inspect", func() {
+		SkipIfRootless("Requires root CNI networking")
+		ctrName := "testctr"
+		session := podmanTest.Podman([]string{"create", "--name", ctrName, ALPINE, "top"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(BeZero())
+
+		inspect := podmanTest.InspectContainer(ctrName)
+		Expect(len(inspect)).To(Equal(1))
+		Expect(len(inspect[0].NetworkSettings.Networks)).To(Equal(1))
+	})
 })

From 0b500515d8b8900851763a17b61609556f58d99c Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Fri, 22 Jan 2021 15:38:51 +0100
Subject: [PATCH 05/15] libpod: add (*Container).ResolvePath()

Add an API to libpod to resolve a path on the container.  We can
refactor the code that was originally written for copy.  Other
functions are requiring a proper path resolution, so libpod seems
like a reasonable home for sharing that code.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 libpod/container_api.go                 |  29 +++++
 libpod/container_path_resolution.go     | 127 +++++++++++++++++++++
 pkg/domain/infra/abi/archive.go         |  14 ++-
 pkg/domain/infra/abi/containers_stat.go | 140 ++----------------------
 4 files changed, 176 insertions(+), 134 deletions(-)
 create mode 100644 libpod/container_path_resolution.go

diff --git a/libpod/container_api.go b/libpod/container_api.go
index 0d62a2dd76..951227a4f1 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -776,3 +776,32 @@ func (c *Container) ShouldRestart(ctx context.Context) bool {
 	}
 	return c.shouldRestart()
 }
+
+// ResolvePath resolves the specified path on the root for the container.  The
+// root must either be the mounted image of the container or the already
+// mounted container storage.
+//
+// It returns the resolved root and the resolved path.  Note that the path may
+// resolve to the container's mount point or to a volume or bind mount.
+func (c *Container) ResolvePath(ctx context.Context, root string, path string) (string, string, error) {
+	logrus.Debugf("Resolving path %q (root %q) on container %s", path, root, c.ID())
+
+	// Minimal sanity checks.
+	if len(root)*len(path) == 0 {
+		return "", "", errors.Wrapf(define.ErrInternal, "ResolvePath: root (%q) and path (%q) must be non empty", root, path)
+	}
+	if _, err := os.Stat(root); err != nil {
+		return "", "", errors.Wrapf(err, "cannot locate root to resolve path on container %s", c.ID())
+	}
+
+	if !c.batched {
+		c.lock.Lock()
+		defer c.lock.Unlock()
+
+		if err := c.syncContainer(); err != nil {
+			return "", "", err
+		}
+	}
+
+	return c.resolvePath(root, path)
+}
diff --git a/libpod/container_path_resolution.go b/libpod/container_path_resolution.go
new file mode 100644
index 0000000000..68f174278d
--- /dev/null
+++ b/libpod/container_path_resolution.go
@@ -0,0 +1,127 @@
+package libpod
+
+import (
+	"path/filepath"
+	"strings"
+
+	securejoin "github.com/cyphar/filepath-securejoin"
+	"github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+)
+
+// 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.
+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
+	if !filepath.IsAbs(containerPath) {
+		// If the containerPath is not absolute, it's relative to the
+		// container's working dir.  To be extra careful, let's first
+		// join the working dir with "/", and the add the containerPath
+		// to it.
+		pathRelativeToContainerMountPoint = filepath.Join(filepath.Join("/", container.WorkingDir()), containerPath)
+	}
+	resolvedPathOnTheContainerMountPoint := filepath.Join(mountPoint, pathRelativeToContainerMountPoint)
+	pathRelativeToContainerMountPoint = strings.TrimPrefix(pathRelativeToContainerMountPoint, mountPoint)
+	pathRelativeToContainerMountPoint = filepath.Join("/", pathRelativeToContainerMountPoint)
+
+	// Now we have an "absolute container Path" but not yet resolved on the
+	// host (e.g., "/foo/bar/file.txt").  As mentioned above, we need to
+	// check if "/foo/bar/file.txt" is on a volume or bind mount.  To do
+	// that, we need to walk *down* the paths to the root.  Assuming
+	// volume-1 is mounted to "/foo" and volume-2 is mounted to "/foo/bar",
+	// we must select "/foo/bar".  Once selected, we need to rebase the
+	// remainder (i.e, "/file.txt") on the volume's mount point on the
+	// host.  Same applies to bind mounts.
+
+	searchPath := pathRelativeToContainerMountPoint
+	for {
+		volume, err := findVolume(container, searchPath)
+		if err != nil {
+			return "", "", err
+		}
+		if volume != nil {
+			logrus.Debugf("Container path %q resolved to volume %q on path %q", containerPath, volume.Name(), searchPath)
+
+			// TODO: We really need to force the volume to mount
+			// before doing this, but that API is not exposed
+			// externally right now and doing so is beyond the scope
+			// of this commit.
+			mountPoint, err := volume.MountPoint()
+			if err != nil {
+				return "", "", err
+			}
+			if mountPoint == "" {
+				return "", "", errors.Errorf("volume %s is not mounted, cannot copy into it", volume.Name())
+			}
+
+			// We found a matching volume for searchPath.  We now
+			// need to first find the relative path of our input
+			// path to the searchPath, and then join it with the
+			// volume's mount point.
+			pathRelativeToVolume := strings.TrimPrefix(pathRelativeToContainerMountPoint, searchPath)
+			absolutePathOnTheVolumeMount, err := securejoin.SecureJoin(mountPoint, pathRelativeToVolume)
+			if err != nil {
+				return "", "", err
+			}
+			return mountPoint, absolutePathOnTheVolumeMount, nil
+		}
+
+		if mount := findBindMount(container, searchPath); mount != nil {
+			logrus.Debugf("Container path %q resolved to bind mount %q:%q on path %q", containerPath, mount.Source, mount.Destination, searchPath)
+			// We found a matching bind mount for searchPath.  We
+			// now need to first find the relative path of our
+			// input path to the searchPath, and then join it with
+			// the source of the bind mount.
+			pathRelativeToBindMount := strings.TrimPrefix(pathRelativeToContainerMountPoint, searchPath)
+			absolutePathOnTheBindMount, err := securejoin.SecureJoin(mount.Source, pathRelativeToBindMount)
+			if err != nil {
+				return "", "", err
+			}
+			return mount.Source, absolutePathOnTheBindMount, nil
+
+		}
+
+		if searchPath == "/" {
+			// Cannot go beyond "/", so we're done.
+			break
+		}
+
+		// Walk *down* the path (e.g., "/foo/bar/x" -> "/foo/bar").
+		searchPath = filepath.Dir(searchPath)
+	}
+
+	// No volume, no bind mount but just a normal path on the container.
+	return mountPoint, resolvedPathOnTheContainerMountPoint, nil
+}
+
+// findVolume checks if the specified container path matches a volume inside
+// the container.  It returns a matching volume or nil.
+func findVolume(c *Container, containerPath string) (*Volume, error) {
+	runtime := c.Runtime()
+	cleanedContainerPath := filepath.Clean(containerPath)
+	for _, vol := range c.Config().NamedVolumes {
+		if cleanedContainerPath == filepath.Clean(vol.Dest) {
+			return runtime.GetVolume(vol.Name)
+		}
+	}
+	return nil, nil
+}
+
+// findBindMount checks if the specified container path matches a bind mount
+// inside the container.  It 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 {
+		if m.Type != "bind" {
+			continue
+		}
+		if cleanedPath == filepath.Clean(m.Destination) {
+			mount := m
+			return &mount
+		}
+	}
+	return nil
+}
diff --git a/pkg/domain/infra/abi/archive.go b/pkg/domain/infra/abi/archive.go
index 809813756f..c64dfb02a0 100644
--- a/pkg/domain/infra/abi/archive.go
+++ b/pkg/domain/infra/abi/archive.go
@@ -26,13 +26,18 @@ func (ic *ContainerEngine) ContainerCopyFromArchive(ctx context.Context, nameOrI
 		return nil, err
 	}
 
+	containerMountPoint, err := container.Mount()
+	if err != nil {
+		return nil, err
+	}
+
 	unmount := func() {
 		if err := container.Unmount(false); err != nil {
 			logrus.Errorf("Error unmounting container: %v", err)
 		}
 	}
 
-	_, resolvedRoot, resolvedContainerPath, err := ic.containerStat(container, containerPath)
+	_, resolvedRoot, resolvedContainerPath, err := ic.containerStat(container, containerMountPoint, containerPath)
 	if err != nil {
 		unmount()
 		return nil, err
@@ -71,6 +76,11 @@ func (ic *ContainerEngine) ContainerCopyToArchive(ctx context.Context, nameOrID
 		return nil, err
 	}
 
+	containerMountPoint, err := container.Mount()
+	if err != nil {
+		return nil, err
+	}
+
 	unmount := func() {
 		if err := container.Unmount(false); err != nil {
 			logrus.Errorf("Error unmounting container: %v", err)
@@ -83,7 +93,7 @@ func (ic *ContainerEngine) ContainerCopyToArchive(ctx context.Context, nameOrID
 		containerPath = "/."
 	}
 
-	_, resolvedRoot, resolvedContainerPath, err := ic.containerStat(container, containerPath)
+	_, resolvedRoot, resolvedContainerPath, err := ic.containerStat(container, containerMountPoint, containerPath)
 	if err != nil {
 		unmount()
 		return nil, err
diff --git a/pkg/domain/infra/abi/containers_stat.go b/pkg/domain/infra/abi/containers_stat.go
index 931e77026f..f3d0799a04 100644
--- a/pkg/domain/infra/abi/containers_stat.go
+++ b/pkg/domain/infra/abi/containers_stat.go
@@ -10,18 +10,11 @@ import (
 	"github.com/containers/podman/v2/libpod"
 	"github.com/containers/podman/v2/pkg/copy"
 	"github.com/containers/podman/v2/pkg/domain/entities"
-	securejoin "github.com/cyphar/filepath-securejoin"
-	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
-func (ic *ContainerEngine) containerStat(container *libpod.Container, containerPath string) (*entities.ContainerStatReport, string, string, error) {
-	containerMountPoint, err := container.Mount()
-	if err != nil {
-		return nil, "", "", err
-	}
-
+func (ic *ContainerEngine) containerStat(container *libpod.Container, containerMountPoint string, containerPath string) (*entities.ContainerStatReport, string, string, error) {
 	// Make sure that "/" copies the *contents* of the mount point and not
 	// the directory.
 	if containerPath == "/" {
@@ -30,7 +23,7 @@ func (ic *ContainerEngine) containerStat(container *libpod.Container, containerP
 
 	// Now resolve the container's path.  It may hit a volume, it may hit a
 	// bind mount, it may be relative.
-	resolvedRoot, resolvedContainerPath, err := resolveContainerPaths(container, containerMountPoint, containerPath)
+	resolvedRoot, resolvedContainerPath, err := container.ResolvePath(context.Background(), containerMountPoint, containerPath)
 	if err != nil {
 		return nil, "", "", err
 	}
@@ -94,138 +87,21 @@ func (ic *ContainerEngine) ContainerStat(ctx context.Context, nameOrID string, c
 		return nil, err
 	}
 
+	containerMountPoint, err := container.Mount()
+	if err != nil {
+		return nil, err
+	}
+
 	defer func() {
 		if err := container.Unmount(false); err != nil {
 			logrus.Errorf("Error unmounting container: %v", err)
 		}
 	}()
 
-	statReport, _, _, err := ic.containerStat(container, containerPath)
+	statReport, _, _, err := ic.containerStat(container, containerMountPoint, containerPath)
 	return statReport, err
 }
 
-// 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.
-//
-// NOTE: We must take volumes and bind mounts into account as, regrettably, we
-// can copy to/from stopped containers.  In that case, the volumes and bind
-// mounts are not present.  For running containers, the runtime (e.g., runc or
-// crun) takes care of these mounts.  For stopped ones, we need to do quite
-// some dance, as done below.
-func resolveContainerPaths(container *libpod.Container, mountPoint string, containerPath string) (string, string, error) {
-	// Let's first make sure we have a path relative to the mount point.
-	pathRelativeToContainerMountPoint := containerPath
-	if !filepath.IsAbs(containerPath) {
-		// If the containerPath is not absolute, it's relative to the
-		// container's working dir.  To be extra careful, let's first
-		// join the working dir with "/", and the add the containerPath
-		// to it.
-		pathRelativeToContainerMountPoint = filepath.Join(filepath.Join("/", container.WorkingDir()), containerPath)
-	}
-	resolvedPathOnTheContainerMountPoint := filepath.Join(mountPoint, pathRelativeToContainerMountPoint)
-	pathRelativeToContainerMountPoint = strings.TrimPrefix(pathRelativeToContainerMountPoint, mountPoint)
-	pathRelativeToContainerMountPoint = filepath.Join("/", pathRelativeToContainerMountPoint)
-
-	// Now we have an "absolute container Path" but not yet resolved on the
-	// host (e.g., "/foo/bar/file.txt").  As mentioned above, we need to
-	// check if "/foo/bar/file.txt" is on a volume or bind mount.  To do
-	// that, we need to walk *down* the paths to the root.  Assuming
-	// volume-1 is mounted to "/foo" and volume-2 is mounted to "/foo/bar",
-	// we must select "/foo/bar".  Once selected, we need to rebase the
-	// remainder (i.e, "/file.txt") on the volume's mount point on the
-	// host.  Same applies to bind mounts.
-
-	searchPath := pathRelativeToContainerMountPoint
-	for {
-		volume, err := findVolume(container, searchPath)
-		if err != nil {
-			return "", "", err
-		}
-		if volume != nil {
-			logrus.Debugf("Container path %q resolved to volume %q on path %q", containerPath, volume.Name(), searchPath)
-
-			// TODO: We really need to force the volume to mount
-			// before doing this, but that API is not exposed
-			// externally right now and doing so is beyond the scope
-			// of this commit.
-			mountPoint, err := volume.MountPoint()
-			if err != nil {
-				return "", "", err
-			}
-			if mountPoint == "" {
-				return "", "", errors.Errorf("volume %s is not mounted, cannot copy into it", volume.Name())
-			}
-
-			// We found a matching volume for searchPath.  We now
-			// need to first find the relative path of our input
-			// path to the searchPath, and then join it with the
-			// volume's mount point.
-			pathRelativeToVolume := strings.TrimPrefix(pathRelativeToContainerMountPoint, searchPath)
-			absolutePathOnTheVolumeMount, err := securejoin.SecureJoin(mountPoint, pathRelativeToVolume)
-			if err != nil {
-				return "", "", err
-			}
-			return mountPoint, absolutePathOnTheVolumeMount, nil
-		}
-
-		if mount := findBindMount(container, searchPath); mount != nil {
-			logrus.Debugf("Container path %q resolved to bind mount %q:%q on path %q", containerPath, mount.Source, mount.Destination, searchPath)
-			// We found a matching bind mount for searchPath.  We
-			// now need to first find the relative path of our
-			// input path to the searchPath, and then join it with
-			// the source of the bind mount.
-			pathRelativeToBindMount := strings.TrimPrefix(pathRelativeToContainerMountPoint, searchPath)
-			absolutePathOnTheBindMount, err := securejoin.SecureJoin(mount.Source, pathRelativeToBindMount)
-			if err != nil {
-				return "", "", err
-			}
-			return mount.Source, absolutePathOnTheBindMount, nil
-
-		}
-
-		if searchPath == "/" {
-			// Cannot go beyond "/", so we're done.
-			break
-		}
-
-		// Walk *down* the path (e.g., "/foo/bar/x" -> "/foo/bar").
-		searchPath = filepath.Dir(searchPath)
-	}
-
-	// No volume, no bind mount but just a normal path on the container.
-	return mountPoint, resolvedPathOnTheContainerMountPoint, nil
-}
-
-// findVolume checks if the specified container path matches a volume inside
-// the container.  It returns a matching volume or nil.
-func findVolume(c *libpod.Container, containerPath string) (*libpod.Volume, error) {
-	runtime := c.Runtime()
-	cleanedContainerPath := filepath.Clean(containerPath)
-	for _, vol := range c.Config().NamedVolumes {
-		if cleanedContainerPath == filepath.Clean(vol.Dest) {
-			return runtime.GetVolume(vol.Name)
-		}
-	}
-	return nil, nil
-}
-
-// findBindMount checks if the specified container path matches a bind mount
-// inside the container.  It returns a matching mount or nil.
-func findBindMount(c *libpod.Container, containerPath string) *specs.Mount {
-	cleanedPath := filepath.Clean(containerPath)
-	for _, m := range c.Config().Spec.Mounts {
-		if m.Type != "bind" {
-			continue
-		}
-		if cleanedPath == filepath.Clean(m.Destination) {
-			mount := m
-			return &mount
-		}
-	}
-	return nil
-}
-
 // secureStat extracts file info for path in a chroot'ed environment in root.
 func secureStat(root string, path string) (*buildahCopiah.StatForItem, error) {
 	var glob string

From c48753bfa0d56b855f71dc6ad62e8eff62cf98e1 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Thu, 21 Jan 2021 15:41:14 +0100
Subject: [PATCH 06/15] workdir presence checks

A container's workdir can be specified via the CLI via `--workdir` and
via an image config with the CLI having precedence.

Since images have a tendency to specify workdirs without necessarily
shipping the paths with the root FS, make sure that Podman creates the
workdir.  When specified via the CLI, do not create the path, but check
for its existence and return a human-friendly error.

NOTE: `crun` is performing a similar check that would yield exit code
127.  With this change, however, Podman performs the check and yields
exit code 126.  Since this is specific to `crun`, I do not consider it
to be a breaking change of Podman.

Fixes: #9040
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 libpod/container_internal_linux.go       | 63 ++++++++++++++++++------
 libpod/container_path_resolution.go      | 47 ++++++++++++++++--
 pkg/specgen/generate/container.go        | 14 ------
 pkg/specgen/generate/container_create.go | 12 ++++-
 test/e2e/run_working_dir_test.go         |  6 +--
 test/system/030-run.bats                 | 21 ++++++++
 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 0553cc59c7..9c5989d23c 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 68f174278d..805b3b9478 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 cc3f7928ce..31d317bf83 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 4f36744cab..1bc050b003 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 7d8db361c3..59538448e1 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 29dc95dc30..dcf1da3709 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 0e83a184b7..bf9fa789cf 100644
--- a/test/system/070-build.bats
+++ b/test/system/070-build.bats
@@ -145,10 +145,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

From 07f2df472cdbb4c1e533dbb6e17a6c04bd2f76fe Mon Sep 17 00:00:00 2001
From: Matthew Heon <mheon@redhat.com>
Date: Mon, 25 Jan 2021 15:29:00 -0500
Subject: [PATCH 07/15] Ensure shutdown handler access is syncronized

There was a potential race where two handlers could be added at
the same time. Go Maps are not thread-safe, so that could do
unpleasant things. Add a mutex to keep things safe.

Also, swap the order or Register and Start for the handlers in
Libpod runtime created. As written, there was a small gap between
Start and Register where SIGTERM/SIGINT would be completely
ignored, instead of stopping Podman. Swapping the two closes this
gap.

Signed-off-by: Matthew Heon <mheon@redhat.com>
---
 libpod/runtime.go          | 14 +++++++-------
 libpod/shutdown/handler.go | 10 ++++++++++
 pkg/api/server/server.go   |  8 ++++----
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/libpod/runtime.go b/libpod/runtime.go
index 34c737a67e..0dc220b52a 100644
--- a/libpod/runtime.go
+++ b/libpod/runtime.go
@@ -180,6 +180,13 @@ func newRuntimeFromConfig(ctx context.Context, conf *config.Config, options ...R
 		}
 	}
 
+	if err := shutdown.Register("libpod", func(sig os.Signal) error {
+		os.Exit(1)
+		return nil
+	}); err != nil && errors.Cause(err) != shutdown.ErrHandlerExists {
+		logrus.Errorf("Error registering shutdown handler for libpod: %v", err)
+	}
+
 	if err := shutdown.Start(); err != nil {
 		return nil, errors.Wrapf(err, "error starting shutdown signal handler")
 	}
@@ -188,13 +195,6 @@ func newRuntimeFromConfig(ctx context.Context, conf *config.Config, options ...R
 		return nil, err
 	}
 
-	if err := shutdown.Register("libpod", func(sig os.Signal) error {
-		os.Exit(1)
-		return nil
-	}); err != nil && errors.Cause(err) != shutdown.ErrHandlerExists {
-		logrus.Errorf("Error registering shutdown handler for libpod: %v", err)
-	}
-
 	return runtime, nil
 }
 
diff --git a/libpod/shutdown/handler.go b/libpod/shutdown/handler.go
index f0f228b19e..ac1d339101 100644
--- a/libpod/shutdown/handler.go
+++ b/libpod/shutdown/handler.go
@@ -18,6 +18,8 @@ var (
 	stopped    bool
 	sigChan    chan os.Signal
 	cancelChan chan bool
+	// Syncronize accesses to the map
+	handlerLock sync.Mutex
 	// Definitions of all on-shutdown handlers
 	handlers map[string]func(os.Signal) error
 	// Ordering that on-shutdown handlers will be invoked.
@@ -50,6 +52,7 @@ func Start() error {
 		case sig := <-sigChan:
 			logrus.Infof("Received shutdown signal %v, terminating!", sig)
 			shutdownInhibit.Lock()
+			handlerLock.Lock()
 			for _, name := range handlerOrder {
 				handler, ok := handlers[name]
 				if !ok {
@@ -61,6 +64,7 @@ func Start() error {
 					logrus.Errorf("Error running shutdown handler %s: %v", name, err)
 				}
 			}
+			handlerLock.Unlock()
 			shutdownInhibit.Unlock()
 			return
 		}
@@ -97,6 +101,9 @@ func Uninhibit() {
 // by a signal. Handlers are invoked LIFO - the last handler registered is the
 // first run.
 func Register(name string, handler func(os.Signal) error) error {
+	handlerLock.Lock()
+	defer handlerLock.Unlock()
+
 	if handlers == nil {
 		handlers = make(map[string]func(os.Signal) error)
 	}
@@ -113,6 +120,9 @@ func Register(name string, handler func(os.Signal) error) error {
 
 // Unregister un-registers a given shutdown handler.
 func Unregister(name string) error {
+	handlerLock.Lock()
+	defer handlerLock.Unlock()
+
 	if handlers == nil {
 		return nil
 	}
diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go
index 046f6561c1..d612041f6d 100644
--- a/pkg/api/server/server.go
+++ b/pkg/api/server/server.go
@@ -179,15 +179,15 @@ func setupSystemd() {
 func (s *APIServer) Serve() error {
 	setupSystemd()
 
-	// Start the shutdown signal handler.
-	if err := shutdown.Start(); err != nil {
-		return err
-	}
 	if err := shutdown.Register("server", func(sig os.Signal) error {
 		return s.Shutdown()
 	}); err != nil {
 		return err
 	}
+	// Start the shutdown signal handler.
+	if err := shutdown.Start(); err != nil {
+		return err
+	}
 
 	errChan := make(chan error, 1)
 

From b111370306bca917d77d3b84352791f157e09983 Mon Sep 17 00:00:00 2001
From: Paul Holzinger <paul.holzinger@web.de>
Date: Wed, 20 Jan 2021 14:30:42 +0100
Subject: [PATCH 08/15] Make generate systemd --new robust against double curly
 braces

If the container create command contains an argument with double
curly braces the golang template parsing can fail since it tries
to interpret the value as variable. To fix this change the default
delimiter for the internal template to `{{{{`.

Fixes #9034

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
---
 pkg/systemd/generate/common.go          | 12 +++---
 pkg/systemd/generate/containers.go      | 48 +++++++++++------------
 pkg/systemd/generate/containers_test.go | 39 +++++++++++++++++++
 pkg/systemd/generate/pods.go            | 52 ++++++++++++-------------
 pkg/systemd/generate/pods_test.go       | 43 ++++++++++++++++++++
 test/e2e/generate_systemd_test.go       | 21 ++++++++++
 6 files changed, 159 insertions(+), 56 deletions(-)

diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go
index 8901298db0..de6751a179 100644
--- a/pkg/systemd/generate/common.go
+++ b/pkg/systemd/generate/common.go
@@ -30,14 +30,14 @@ func validateRestartPolicy(restart string) error {
 	return errors.Errorf("%s is not a valid restart policy", restart)
 }
 
-const headerTemplate = `# {{.ServiceName}}.service
-# autogenerated by Podman {{.PodmanVersion}}
-{{- if .TimeStamp}}
-# {{.TimeStamp}}
-{{- end}}
+const headerTemplate = `# {{{{.ServiceName}}}}.service
+# autogenerated by Podman {{{{.PodmanVersion}}}}
+{{{{- if .TimeStamp}}}}
+# {{{{.TimeStamp}}}}
+{{{{- end}}}}
 
 [Unit]
-Description=Podman {{.ServiceName}}.service
+Description=Podman {{{{.ServiceName}}}}.service
 Documentation=man:podman-generate-systemd(1)
 Wants=network.target
 After=network-online.target
diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go
index b64b2593ca..5f52b0a777 100644
--- a/pkg/systemd/generate/containers.go
+++ b/pkg/systemd/generate/containers.go
@@ -72,22 +72,22 @@ type containerInfo struct {
 }
 
 const containerTemplate = headerTemplate + `
-{{- if .BoundToServices}}
-BindsTo={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}}
-After={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}}
-{{- end}}
+{{{{- if .BoundToServices}}}}
+BindsTo={{{{- range $index, $value := .BoundToServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
+After={{{{- range $index, $value := .BoundToServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
+{{{{- end}}}}
 
 [Service]
-Environment={{.EnvVariable}}=%n
-Restart={{.RestartPolicy}}
-TimeoutStopSec={{.TimeoutStopSec}}
-{{- if .ExecStartPre}}
-ExecStartPre={{.ExecStartPre}}
-{{- end}}
-ExecStart={{.ExecStart}}
-ExecStop={{.ExecStop}}
-ExecStopPost={{.ExecStopPost}}
-PIDFile={{.PIDFile}}
+Environment={{{{.EnvVariable}}}}=%n
+Restart={{{{.RestartPolicy}}}}
+TimeoutStopSec={{{{.TimeoutStopSec}}}}
+{{{{- if .ExecStartPre}}}}
+ExecStartPre={{{{.ExecStartPre}}}}
+{{{{- end}}}}
+ExecStart={{{{.ExecStart}}}}
+ExecStop={{{{.ExecStop}}}}
+ExecStopPost={{{{.ExecStopPost}}}}
+PIDFile={{{{.PIDFile}}}}
 Type=forking
 
 [Install]
@@ -173,9 +173,9 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 	}
 
 	info.EnvVariable = EnvVariable
-	info.ExecStart = "{{.Executable}} start {{.ContainerNameOrID}}"
-	info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}}"
-	info.ExecStopPost = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}}"
+	info.ExecStart = "{{{{.Executable}}}} start {{{{.ContainerNameOrID}}}}"
+	info.ExecStop = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.ContainerNameOrID}}}}"
+	info.ExecStopPost = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.ContainerNameOrID}}}}"
 
 	// Assemble the ExecStart command when creating a new container.
 	//
@@ -209,8 +209,8 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 		}
 		startCommand = append(startCommand,
 			"run",
-			"--conmon-pidfile", "{{.PIDFile}}",
-			"--cidfile", "{{.ContainerIDFile}}",
+			"--conmon-pidfile", "{{{{.PIDFile}}}}",
+			"--cidfile", "{{{{.ContainerIDFile}}}}",
 			"--cgroups=no-conmon",
 		)
 		// If the container is in a pod, make sure that the
@@ -281,10 +281,10 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 		startCommand = append(startCommand, remainingCmd...)
 		startCommand = quoteArguments(startCommand)
 
-		info.ExecStartPre = "/bin/rm -f {{.PIDFile}} {{.ContainerIDFile}}"
+		info.ExecStartPre = "/bin/rm -f {{{{.PIDFile}}}} {{{{.ContainerIDFile}}}}"
 		info.ExecStart = strings.Join(startCommand, " ")
-		info.ExecStop = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}stop --ignore --cidfile {{.ContainerIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}"
-		info.ExecStopPost = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}rm --ignore -f --cidfile {{.ContainerIDFile}}"
+		info.ExecStop = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}stop --ignore --cidfile {{{{.ContainerIDFile}}}} {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}}"
+		info.ExecStopPost = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}rm --ignore -f --cidfile {{{{.ContainerIDFile}}}}"
 	}
 
 	info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout
@@ -307,7 +307,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 	// generation.  That's especially needed for embedding the PID and ID
 	// files in other fields which will eventually get replaced in the 2nd
 	// template execution.
-	templ, err := template.New("container_template").Parse(containerTemplate)
+	templ, err := template.New("container_template").Delims("{{{{", "}}}}").Parse(containerTemplate)
 	if err != nil {
 		return "", errors.Wrap(err, "error parsing systemd service template")
 	}
@@ -318,7 +318,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
 	}
 
 	// Now parse the generated template (i.e., buf) and execute it.
-	templ, err = template.New("container_template").Parse(buf.String())
+	templ, err = template.New("container_template").Delims("{{{{", "}}}}").Parse(buf.String())
 	if err != nil {
 		return "", errors.Wrap(err, "error parsing systemd service template")
 	}
diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go
index c8e65bfe3b..96d95644b8 100644
--- a/pkg/systemd/generate/containers_test.go
+++ b/pkg/systemd/generate/containers_test.go
@@ -329,6 +329,29 @@ Type=forking
 WantedBy=multi-user.target default.target
 `
 
+	goodNewWithJournaldTag := `# jadda-jadda.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman jadda-jadda.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network.target
+After=network-online.target
+
+[Service]
+Environment=PODMAN_SYSTEMD_UNIT=%n
+Restart=always
+TimeoutStopSec=70
+ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id
+ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d --replace --name test --log-driver=journald --log-opt=tag={{.Name}} awesome-image:latest
+ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10
+ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id
+PIDFile=%t/jadda-jadda.pid
+Type=forking
+
+[Install]
+WantedBy=multi-user.target default.target
+`
 	tests := []struct {
 		name    string
 		info    containerInfo
@@ -608,6 +631,22 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 		},
+		{"good with journald log tag (see #9034)",
+			containerInfo{
+				Executable:        "/usr/bin/podman",
+				ServiceName:       "jadda-jadda",
+				ContainerNameOrID: "jadda-jadda",
+				RestartPolicy:     "always",
+				PIDFile:           "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:       10,
+				PodmanVersion:     "CI",
+				CreateCommand:     []string{"I'll get stripped", "create", "--name", "test", "--log-driver=journald", "--log-opt=tag={{.Name}}", "awesome-image:latest"},
+				EnvVariable:       EnvVariable,
+			},
+			goodNewWithJournaldTag,
+			true,
+			false,
+		},
 	}
 	for _, tt := range tests {
 		test := tt
diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go
index 7678a240ff..c7e3aa955f 100644
--- a/pkg/systemd/generate/pods.go
+++ b/pkg/systemd/generate/pods.go
@@ -72,23 +72,23 @@ type podInfo struct {
 	ExecStopPost string
 }
 
-const podTemplate = headerTemplate + `Requires={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}}
-Before={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}}
+const podTemplate = headerTemplate + `Requires={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
+Before={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
 
 [Service]
-Environment={{.EnvVariable}}=%n
-Restart={{.RestartPolicy}}
-TimeoutStopSec={{.TimeoutStopSec}}
-{{- if .ExecStartPre1}}
-ExecStartPre={{.ExecStartPre1}}
-{{- end}}
-{{- if .ExecStartPre2}}
-ExecStartPre={{.ExecStartPre2}}
-{{- end}}
-ExecStart={{.ExecStart}}
-ExecStop={{.ExecStop}}
-ExecStopPost={{.ExecStopPost}}
-PIDFile={{.PIDFile}}
+Environment={{{{.EnvVariable}}}}=%n
+Restart={{{{.RestartPolicy}}}}
+TimeoutStopSec={{{{.TimeoutStopSec}}}}
+{{{{- if .ExecStartPre1}}}}
+ExecStartPre={{{{.ExecStartPre1}}}}
+{{{{- end}}}}
+{{{{- if .ExecStartPre2}}}}
+ExecStartPre={{{{.ExecStartPre2}}}}
+{{{{- end}}}}
+ExecStart={{{{.ExecStart}}}}
+ExecStop={{{{.ExecStop}}}}
+ExecStopPost={{{{.ExecStopPost}}}}
+PIDFile={{{{.PIDFile}}}}
 Type=forking
 
 [Install]
@@ -236,9 +236,9 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 	}
 
 	info.EnvVariable = EnvVariable
-	info.ExecStart = "{{.Executable}} start {{.InfraNameOrID}}"
-	info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.InfraNameOrID}}"
-	info.ExecStopPost = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.InfraNameOrID}}"
+	info.ExecStart = "{{{{.Executable}}}} start {{{{.InfraNameOrID}}}}"
+	info.ExecStop = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.InfraNameOrID}}}}"
+	info.ExecStopPost = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.InfraNameOrID}}}}"
 
 	// Assemble the ExecStart command when creating a new pod.
 	//
@@ -278,8 +278,8 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 		startCommand = append(startCommand, podRootArgs...)
 		startCommand = append(startCommand,
 			[]string{"pod", "create",
-				"--infra-conmon-pidfile", "{{.PIDFile}}",
-				"--pod-id-file", "{{.PodIDFile}}"}...)
+				"--infra-conmon-pidfile", "{{{{.PIDFile}}}}",
+				"--pod-id-file", "{{{{.PodIDFile}}}}"}...)
 
 		// Presence check for certain flags/options.
 		fs := pflag.NewFlagSet("args", pflag.ContinueOnError)
@@ -308,11 +308,11 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 		startCommand = append(startCommand, podCreateArgs...)
 		startCommand = quoteArguments(startCommand)
 
-		info.ExecStartPre1 = "/bin/rm -f {{.PIDFile}} {{.PodIDFile}}"
+		info.ExecStartPre1 = "/bin/rm -f {{{{.PIDFile}}}} {{{{.PodIDFile}}}}"
 		info.ExecStartPre2 = strings.Join(startCommand, " ")
-		info.ExecStart = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod start --pod-id-file {{.PodIDFile}}"
-		info.ExecStop = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod stop --ignore --pod-id-file {{.PodIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}"
-		info.ExecStopPost = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod rm --ignore -f --pod-id-file {{.PodIDFile}}"
+		info.ExecStart = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod start --pod-id-file {{{{.PodIDFile}}}}"
+		info.ExecStop = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod stop --ignore --pod-id-file {{{{.PodIDFile}}}} {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}}"
+		info.ExecStopPost = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod rm --ignore -f --pod-id-file {{{{.PodIDFile}}}}"
 	}
 	info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout
 
@@ -334,7 +334,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 	// generation.  That's especially needed for embedding the PID and ID
 	// files in other fields which will eventually get replaced in the 2nd
 	// template execution.
-	templ, err := template.New("pod_template").Parse(podTemplate)
+	templ, err := template.New("pod_template").Delims("{{{{", "}}}}").Parse(podTemplate)
 	if err != nil {
 		return "", errors.Wrap(err, "error parsing systemd service template")
 	}
@@ -345,7 +345,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
 	}
 
 	// Now parse the generated template (i.e., buf) and execute it.
-	templ, err = template.New("pod_template").Parse(buf.String())
+	templ, err = template.New("pod_template").Delims("{{{{", "}}}}").Parse(buf.String())
 	if err != nil {
 		return "", errors.Wrap(err, "error parsing systemd service template")
 	}
diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go
index 1c63301609..2b430226b7 100644
--- a/pkg/systemd/generate/pods_test.go
+++ b/pkg/systemd/generate/pods_test.go
@@ -139,6 +139,33 @@ ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-
 PIDFile=%t/pod-123abc.pid
 Type=forking
 
+[Install]
+WantedBy=multi-user.target default.target
+`
+
+	podNewLabelWithCurlyBraces := `# pod-123abc.service
+# autogenerated by Podman CI
+
+[Unit]
+Description=Podman pod-123abc.service
+Documentation=man:podman-generate-systemd(1)
+Wants=network.target
+After=network-online.target
+Requires=container-1.service container-2.service
+Before=container-1.service container-2.service
+
+[Service]
+Environment=PODMAN_SYSTEMD_UNIT=%n
+Restart=on-failure
+TimeoutStopSec=70
+ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id
+ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --name foo --label key={{someval}} --replace
+ExecStart=/usr/bin/podman pod start --pod-id-file %t/pod-123abc.pod-id
+ExecStop=/usr/bin/podman pod stop --ignore --pod-id-file %t/pod-123abc.pod-id -t 10
+ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-id
+PIDFile=%t/pod-123abc.pid
+Type=forking
+
 [Install]
 WantedBy=multi-user.target default.target
 `
@@ -230,6 +257,22 @@ WantedBy=multi-user.target default.target
 			true,
 			false,
 		},
+		{"pod --new with double curly braces",
+			podInfo{
+				Executable:       "/usr/bin/podman",
+				ServiceName:      "pod-123abc",
+				InfraNameOrID:    "jadda-jadda-infra",
+				RestartPolicy:    "on-failure",
+				PIDFile:          "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
+				StopTimeout:      10,
+				PodmanVersion:    "CI",
+				RequiredServices: []string{"container-1", "container-2"},
+				CreateCommand:    []string{"podman", "pod", "create", "--name", "foo", "--label", "key={{someval}}"},
+			},
+			podNewLabelWithCurlyBraces,
+			true,
+			false,
+		},
 	}
 
 	for _, tt := range tests {
diff --git a/test/e2e/generate_systemd_test.go b/test/e2e/generate_systemd_test.go
index be97275914..606d756b04 100644
--- a/test/e2e/generate_systemd_test.go
+++ b/test/e2e/generate_systemd_test.go
@@ -355,4 +355,25 @@ var _ = Describe("Podman generate systemd", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 		Expect(session.IsJSONOutputValid()).To(BeTrue())
 	})
+
+	It("podman generate systemd --new create command with double curly braces", func() {
+		// Regression test for #9034
+		session := podmanTest.Podman([]string{"create", "--name", "foo", "--log-driver=journald", "--log-opt=tag={{.Name}}", ALPINE})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"generate", "systemd", "--new", "foo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(ContainSubstring(" --log-opt=tag={{.Name}} "))
+
+		session = podmanTest.Podman([]string{"pod", "create", "--name", "pod", "--label", "key={{someval}}"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"generate", "systemd", "--new", "pod"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(ContainSubstring(" --label key={{someval}}"))
+	})
 })

From 367c64919e6cfd07101e4f0d40902ad33d9fdcbd Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Sun, 17 Jan 2021 16:30:56 +0100
Subject: [PATCH 09/15] make: generate bindings: use vendor

Set `-mod=vendor` when generating the bindings.  We expect all
dependencies to be vendored already.  This should slightly speed
up the bindings generation and prevent redundant network accesses.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index cc7d9f0ffb..9014b81f78 100644
--- a/Makefile
+++ b/Makefile
@@ -468,7 +468,7 @@ ifneq ($(shell uname -s), Darwin)
 		pushd $${dirname}>/dev/null;  \
 		echo $${dirname}; \
 		echo $(GO) generate; \
-		$(GO) generate; \
+		$(GO) generate -mod=vendor; \
 		popd > /dev/null; \
 	done;
 endif

From 9fa6aa885b64057f974b866506e0909d1cd30a4a Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Sun, 17 Jan 2021 16:37:55 +0100
Subject: [PATCH 10/15] simplify bindings generation

Run `go generate ./pkg/bindings/...` once for all bindings instead of
generating them separately.  This should speed up bindings generation
as a given package is visited only once, and it fixes #8989 by dropping
the use of pushd and popd.

Fixes: #8989
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 Makefile | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 9014b81f78..7d307793cb 100644
--- a/Makefile
+++ b/Makefile
@@ -462,15 +462,7 @@ podman-remote-%-release:
 BINDINGS_SOURCE = $(wildcard pkg/bindings/**/types.go)
 .generate-bindings: $(BINDINGS_SOURCE)
 ifneq ($(shell uname -s), Darwin)
-	for i in $(BINDINGS_SOURCE); do \
-		dirname=$$(dirname $${i}); \
-		shortname=$$(basename $${dirname}); \
-		pushd $${dirname}>/dev/null;  \
-		echo $${dirname}; \
-		echo $(GO) generate; \
-		$(GO) generate -mod=vendor; \
-		popd > /dev/null; \
-	done;
+	$(GO) generate -mod=vendor ./pkg/bindings/... ;
 endif
 	touch .generate-bindings
 

From 960baf1a0966794779ffe88b1e3f77f02993d764 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Tue, 19 Jan 2021 10:27:01 +0100
Subject: [PATCH 11/15] make bindings generation more robuts

The Go gods did not shine upon us trying to understand what's going on
in #9000.  The symptom is that `go generate` did not add required
imports to a generated file, ultimately breaking subsequent compilation.

While it still remains unclear *why* Go is behaving like that, the
symptom disappears when `go generate` runs in module mode; that is
without `-mod=vendor` and without `GO111MODULE=off`.  This was
reproducible on two separate machines (Ubuntu and Fedora).

Also, when facing an unset GOPATH, set it to Go's default (i.e.,
$HOME/go) and make sure that GOBIN is in PATH since `goimports`
is required by `go generate`.

Fixes: #9000
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 7d307793cb..bebb360d4c 100644
--- a/Makefile
+++ b/Makefile
@@ -86,7 +86,7 @@ PODMAN_SERVER_LOG ?=
 
 # If GOPATH not specified, use one in the local directory
 ifeq ($(GOPATH),)
-export GOPATH := $(CURDIR)/_output
+export GOPATH := $(HOME)/go
 unexport GOBIN
 endif
 FIRST_GOPATH := $(firstword $(subst :, ,$(GOPATH)))
@@ -98,6 +98,8 @@ ifeq ($(GOBIN),)
 GOBIN := $(FIRST_GOPATH)/bin
 endif
 
+export PATH := $(PATH):$(GOBIN)
+
 GOMD2MAN ?= $(shell command -v go-md2man || echo '$(GOBIN)/go-md2man')
 
 CROSS_BUILD_TARGETS := \
@@ -462,7 +464,7 @@ podman-remote-%-release:
 BINDINGS_SOURCE = $(wildcard pkg/bindings/**/types.go)
 .generate-bindings: $(BINDINGS_SOURCE)
 ifneq ($(shell uname -s), Darwin)
-	$(GO) generate -mod=vendor ./pkg/bindings/... ;
+	GO111MODULE=off $(GO) generate ./pkg/bindings/... ;
 endif
 	touch .generate-bindings
 

From 6bf4d6195a02fcd39b6d1f7220b4e8ab4f3c2cb5 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Tue, 19 Jan 2021 16:13:02 +0100
Subject: [PATCH 12/15] make bindings generation explicit

Instead of implicitly generating the bindings, make it explicit, similar
to `make vendor`.  This should prevent redundant and possibly error
prone generations.  A following commit will shield CI.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 Makefile | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index bebb360d4c..d56ba72f26 100644
--- a/Makefile
+++ b/Makefile
@@ -208,7 +208,7 @@ endif
 podman: bin/podman
 
 .PHONY: bin/podman-remote
-bin/podman-remote: .gopathok .generate-bindings $(SOURCES) go.mod go.sum ## Build with podman on remote environment
+bin/podman-remote: .gopathok $(SOURCES) go.mod go.sum ## Build with podman on remote environment
 	$(GO) build $(BUILDFLAGS) -gcflags '$(GCFLAGS)' -asmflags '$(ASMFLAGS)' -ldflags '$(LDFLAGS_PODMAN)' -tags "${REMOTETAGS}" -o $@ ./cmd/podman
 
 .PHONY: bin/podman-remote-static
@@ -279,7 +279,6 @@ clean: ## Clean artifacts
 		libpod/container_easyjson.go \
 		libpod/pod_easyjson.go \
 		.install.goimports \
-		.generate-bindings \
 		docs/build
 	make -C docs clean
 
@@ -461,12 +460,10 @@ podman-remote-%-release:
 	rm -f release.txt
 	$(MAKE) podman-remote-release-$*.zip
 
-BINDINGS_SOURCE = $(wildcard pkg/bindings/**/types.go)
-.generate-bindings: $(BINDINGS_SOURCE)
+generate-bindings:
 ifneq ($(shell uname -s), Darwin)
 	GO111MODULE=off $(GO) generate ./pkg/bindings/... ;
 endif
-	touch .generate-bindings
 
 .PHONY: docker-docs
 docker-docs: docs

From edd9a06dc7ea3661f8fd581ecc2086a6c0111ab3 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Tue, 19 Jan 2021 17:19:46 +0100
Subject: [PATCH 13/15] Cirrus: add bindings checks

Make sure that bindings are in sync with the code.  The check is similar
to what's already being done with `make vendor`, so integrate the two.

[NO TESTS NEEDED]

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 .cirrus.yml                         | 14 ++++++++------
 Makefile                            |  1 +
 contrib/cirrus/runner.sh            |  6 ++++--
 contrib/cirrus/setup_environment.sh |  2 +-
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 0abb711467..7c797cdf28 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -274,17 +274,19 @@ swagger_task:
 
 
 # Check that all included go modules from other sources match
-# what is expected in `vendor/modules.txt` vs `go.mod`.
-vendor_task:
-    name: "Test Vendoring"
-    alias: vendor
+# what is expected in `vendor/modules.txt` vs `go.mod`.  Also
+# make sure that the generated bindings in pkg/bindings/...
+# are in sync with the code.
+consistency_task:
+    name: "Test Code Consistency"
+    alias: consistency
     skip: *tags
     depends_on:
         - build
     container: *smallcontainer
     env:
         <<: *stdenvars
-        TEST_FLAVOR: vendor
+        TEST_FLAVOR: consistency
         TEST_ENVIRON: container
         CTR_FQIN: ${FEDORA_CONTAINER_FQIN}
     clone_script: *full_clone  # build-cache not available to container tasks
@@ -642,7 +644,7 @@ success_task:
         - validate
         - bindings
         - swagger
-        - vendor
+        - consistency
         - alt_build
         - static_alt_build
         - osx_alt_build
diff --git a/Makefile b/Makefile
index d56ba72f26..11a46f5ced 100644
--- a/Makefile
+++ b/Makefile
@@ -460,6 +460,7 @@ podman-remote-%-release:
 	rm -f release.txt
 	$(MAKE) podman-remote-release-$*.zip
 
+.PHONY: generate-bindings
 generate-bindings:
 ifneq ($(shell uname -s), Darwin)
 	GO111MODULE=off $(GO) generate ./pkg/bindings/... ;
diff --git a/contrib/cirrus/runner.sh b/contrib/cirrus/runner.sh
index 572f0b44ae..6e6747f288 100755
--- a/contrib/cirrus/runner.sh
+++ b/contrib/cirrus/runner.sh
@@ -142,9 +142,11 @@ function _run_swagger() {
     cp -v $GOSRC/pkg/api/swagger.yaml $GOSRC/
 }
 
-function _run_vendor() {
+function _run_consistency() {
     make vendor
-    ./hack/tree_status.sh
+    SUGGESTION="run 'make vendor' and commit all changes" ./hack/tree_status.sh
+    make generate-bindings
+    SUGGESTION="run 'make generate-bindings' and commit all changes" ./hack/tree_status.sh
 }
 
 function _run_build() {
diff --git a/contrib/cirrus/setup_environment.sh b/contrib/cirrus/setup_environment.sh
index 5c6f05ac0b..7b49caba02 100755
--- a/contrib/cirrus/setup_environment.sh
+++ b/contrib/cirrus/setup_environment.sh
@@ -214,7 +214,7 @@ case "$TEST_FLAVOR" in
 
         install_test_configs
         ;;
-    vendor) make clean ;;
+    consistency) make clean ;;
     release) ;;
     *) die_unknown TEST_FLAVOR
 esac

From 510a983a752a136a5df0bdbff9d14db0490956b2 Mon Sep 17 00:00:00 2001
From: Matthew Heon <matthew.heon@pm.me>
Date: Fri, 29 Jan 2021 15:37:18 -0500
Subject: [PATCH 14/15] Bump to v3.0.0-RC2

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
---
 version/version.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/version/version.go b/version/version.go
index 855ac2cb05..f6c578afa0 100644
--- a/version/version.go
+++ b/version/version.go
@@ -8,7 +8,7 @@ import (
 // NOTE: remember to bump the version at the top
 // of the top-level README.md file when this is
 // bumped.
-var Version = semver.MustParse("3.0.0-rc1")
+var Version = semver.MustParse("3.0.0-rc2")
 
 // APIVersion is the version for the remote
 // client API.  It is used to determine compatibility

From c1f05be4d7ac31c741a9ea542e284d731c6544a1 Mon Sep 17 00:00:00 2001
From: Matthew Heon <matthew.heon@pm.me>
Date: Fri, 29 Jan 2021 15:37:40 -0500
Subject: [PATCH 15/15] Bump to v3.0.0-dev

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
---
 version/version.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/version/version.go b/version/version.go
index f6c578afa0..0bba0147b2 100644
--- a/version/version.go
+++ b/version/version.go
@@ -8,7 +8,7 @@ import (
 // NOTE: remember to bump the version at the top
 // of the top-level README.md file when this is
 // bumped.
-var Version = semver.MustParse("3.0.0-rc2")
+var Version = semver.MustParse("3.0.0-dev")
 
 // APIVersion is the version for the remote
 // client API.  It is used to determine compatibility