Merge pull request #7176 from mheon/make_entrypoint

Ensure WORKDIR from images is created

5 files changed, 163 insertions, 25 deletions

diff --git a/libpod/container_config.go b/libpod/container_config.go
index 301b867fc..5f89395c1 100644
--- a/libpod/container_config.go
+++ b/libpod/container_config.go
@@ -13,25 +13,52 @@ import (
 
 // ContainerConfig contains all information that was used to create the
 // container. It may not be changed once created.
-// It is stored, read-only, on disk
+// It is stored, read-only, on disk in Libpod's State.
+// Any changes will not be written back to the database, and will cause
+// inconsistencies with other Libpod instances.
 type ContainerConfig struct {
+	// Spec is OCI runtime spec used to create the container. This is passed
+	// in when the container is created, but it is not the final spec used
+	// to run the container - it will be modified by Libpod to add things we
+	// manage (e.g. bind mounts for /etc/resolv.conf, named volumes, a
+	// network namespace prepared by CNI or slirp4netns) in the
+	// generateSpec() function.
 	Spec *spec.Spec `json:"spec"`
 
+	// ID is a hex-encoded 256-bit pseudorandom integer used as a unique
+	// identifier for the container. IDs are globally unique in Libpod -
+	// once an ID is in use, no other container or pod will be created with
+	// the same one until the holder of the ID has been removed.
+	// ID is generated by Libpod, and cannot be chosen or influenced by the
+	// user (except when restoring a checkpointed container).
+	// ID is guaranteed to be 64 characters long.
 	ID string `json:"id"`
 
+	// Name is a human-readable name for the container. All containers must
+	// have a non-empty name. Name may be provided when the container is
+	// created; if no name is chosen, a name will be auto-generated.
 	Name string `json:"name"`
 
-	// Full ID of the pood the container belongs to
+	// Pod is the full ID of the pod the container belongs to. If the
+	// container does not belong to a pod, this will be empty.
+	// If this is not empty, a pod with this ID is guaranteed to exist in
+	// the state for the duration of this container's existence.
 	Pod string `json:"pod,omitempty"`
 
-	// Namespace the container is in
+	// Namespace is the libpod Namespace the container is in.
+	// Namespaces are used to divide containers in the state.
 	Namespace string `json:"namespace,omitempty"`
 
-	// ID of this container's lock
+	// LockID is the ID of this container's lock. Each container, pod, and
+	// volume is assigned a unique Lock (from one of several backends) by
+	// the libpod Runtime. This lock will belong only to this container for
+	// the duration of the container's lifetime.
 	LockID uint32 `json:"lockID"`
 
-	// CreateCommand is the full command plus arguments of the process the
-	// container has been created with.
+	// CreateCommand is the full command plus arguments that were used to
+	// create the container. It is shown in the output of Inspect, and may
+	// be used to recreate an identical container for automatic updates or
+	// portable systemd unit files.
 	CreateCommand []string `json:"CreateCommand,omitempty"`
 
 	// RawImageName is the raw and unprocessed name of the image when creating
@@ -40,10 +67,13 @@ type ContainerConfig struct {
 	// name and not some normalized instance of it.
 	RawImageName string `json:"RawImageName,omitempty"`
 
-	// UID/GID mappings used by the storage
+	// IDMappings are UID/GID mappings used by the container's user
+	// namespace. They are used by the OCI runtime when creating the
+	// container, and by c/storage to ensure that the container's files have
+	// the appropriate owner.
 	IDMappings storage.IDMappingOptions `json:"idMappingsOptions,omitempty"`
 
-	// IDs of dependency containers.
+	// Dependencies are the IDs of dependency containers.
 	// These containers must be started before this container is started.
 	Dependencies []string
 
@@ -59,45 +89,92 @@ type ContainerConfig struct {
 // ContainerRootFSConfig is an embedded sub-config providing config info
 // about the container's root fs.
 type ContainerRootFSConfig struct {
-	RootfsImageID   string `json:"rootfsImageID,omitempty"`
+	// RootfsImageID is the ID of the image used to create the container.
+	// If the container was created from a Rootfs, this will be empty.
+	// If non-empty, Podman will create a root filesystem for the container
+	// based on an image with this ID.
+	// This conflicts with Rootfs.
+	RootfsImageID string `json:"rootfsImageID,omitempty"`
+	// RootfsImageName is the (normalized) name of the image used to create
+	// the container. If the container was created from a Rootfs, this will
+	// be empty.
 	RootfsImageName string `json:"rootfsImageName,omitempty"`
-	// Rootfs to use for the container, this conflicts with RootfsImageID
+	// Rootfs is a directory to use as the container's root filesystem.
+	// If RootfsImageID is set, this will be empty.
+	// If this is set, Podman will not create a root filesystem for the
+	// container based on an image, and will instead use the given directory
+	// as the container's root.
+	// Conflicts with RootfsImageID.
 	Rootfs string `json:"rootfs,omitempty"`
-	// Src path to be mounted on /dev/shm in container.
+	// ShmDir is the path to be mounted on /dev/shm in container.
+	// If not set manually at creation time, Libpod will create a tmpfs
+	// with the size specified in ShmSize and populate this with the path of
+	// said tmpfs.
 	ShmDir string `json:"ShmDir,omitempty"`
-	// Size of the container's SHM.
+	// ShmSize is the size of the container's SHM. Only used if ShmDir was
+	// not set manually at time of creation.
 	ShmSize int64 `json:"shmSize"`
 	// Static directory for container content that will persist across
 	// reboot.
+	// StaticDir is a persistent directory for Libpod files that will
+	// survive system reboot. It is not part of the container's rootfs and
+	// is not mounted into the container. It will be removed when the
+	// container is removed.
+	// Usually used to store container log files, files that will be bind
+	// mounted into the container (e.g. the resolv.conf we made for the
+	// container), and other per-container content.
 	StaticDir string `json:"staticDir"`
-	// Mounts list contains all additional mounts into the container rootfs.
-	// These include the SHM mount.
+	// Mounts contains all additional mounts into the container rootfs.
+	// It is presently only used for the container's SHM directory.
 	// These must be unmounted before the container's rootfs is unmounted.
 	Mounts []string `json:"mounts,omitempty"`
-	// NamedVolumes lists the named volumes to mount into the container.
+	// NamedVolumes lists the Libpod named volumes to mount into the
+	// container. Each named volume is guaranteed to exist so long as this
+	// container exists.
 	NamedVolumes []*ContainerNamedVolume `json:"namedVolumes,omitempty"`
 	// OverlayVolumes lists the overlay volumes to mount into the container.
 	OverlayVolumes []*ContainerOverlayVolume `json:"overlayVolumes,omitempty"`
+	// CreateWorkingDir indicates that Libpod should create the container's
+	// working directory if it does not exist. Some OCI runtimes do this by
+	// default, but others do not.
+	CreateWorkingDir bool `json:"createWorkingDir,omitempty"`
 }
 
 // ContainerSecurityConfig is an embedded sub-config providing security configuration
 // to the container.
 type ContainerSecurityConfig struct {
-	// Whether the container is privileged
+	// Pirivileged is whether the container is privileged. Privileged
+	// containers have lessened security and increased access to the system.
+	// Note that this does NOT directly correspond to Podman's --privileged
+	// flag - most of the work of that flag is done in creating the OCI spec
+	// given to Libpod. This only enables a small subset of the overall
+	// operation, mostly around mounting the container image with reduced
+	// security.
 	Privileged bool `json:"privileged"`
-	// SELinux process label for container
+	// ProcessLabel is the SELinux process label for the container.
 	ProcessLabel string `json:"ProcessLabel,omitempty"`
-	// SELinux mount label for root filesystem
+	// MountLabel is the SELinux mount label for the container's root
+	// filesystem. Only used if the container was created from an image.
+	// If not explicitly set, an unused random MLS label will be assigned by
+	// containers/storage (but only if SELinux is enabled).
 	MountLabel string `json:"MountLabel,omitempty"`
-	// LabelOpts are options passed in by the user to setup SELinux labels
+	// LabelOpts are options passed in by the user to setup SELinux labels.
+	// These are used by the containers/storage library.
 	LabelOpts []string `json:"labelopts,omitempty"`
-	// User and group to use in the container
-	// Can be specified by name or UID/GID
+	// User and group to use in the container. Can be specified as only user
+	// (in which case we will attempt to look up the user in the container
+	// to determine the appropriate group) or user and group separated by a
+	// colon.
+	// Can be specified by name or UID/GID.
+	// If unset, this will default to UID and GID 0 (root).
 	User string `json:"user,omitempty"`
-	// Additional groups to add
+	// Groups are additional groups to add the container's user to. These
+	// are resolved within the container using the container's /etc/passwd.
 	Groups []string `json:"groups,omitempty"`
-	// AddCurrentUserPasswdEntry indicates that the current user passwd entry
-	// should be added to the /etc/passwd within the container
+	// AddCurrentUserPasswdEntry indicates that Libpod should ensure that
+	// the container's /etc/passwd contains an entry for the user running
+	// Libpod - mostly used in rootless containers where the user running
+	// Libpod wants to retain their UID inside the container.
 	AddCurrentUserPasswdEntry bool `json:"addCurrentUserPasswdEntry,omitempty"`
 }
 
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 4cfe992ea..9fb9738dc 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -159,7 +159,32 @@ func (c *Container) prepare() error {
 	}
 
 	// Save changes to container state
-	return c.save()
+	if err := c.save(); err != nil {
+		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()
+
+		if err := os.MkdirAll(workdir, 0755); err != nil {
+			if os.IsExist(err) {
+				return nil
+			}
+			return errors.Wrapf(err, "error creating container %s working dir", c.ID())
+		}
+
+		if err := os.Chown(workdir, rootUID, rootGID); err != nil {
+			return errors.Wrapf(err, "error chowning container %s working directory to container root", c.ID())
+		}
+	}
+
+	return nil
 }
 
 // cleanupNetwork unmounts and cleans up the container's network
diff --git a/libpod/options.go b/libpod/options.go
index b98ef2221..16b05d9b6 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -1451,6 +1451,19 @@ func WithCreateCommand(cmd []string) CtrCreateOption {
 	}
 }
 
+// WithCreateWorkingDir tells Podman to create the container's working directory
+// if it does not exist.
+func WithCreateWorkingDir() CtrCreateOption {
+	return func(ctr *Container) error {
+		if ctr.valid {
+			return define.ErrCtrFinalized
+		}
+
+		ctr.config.CreateWorkingDir = true
+		return nil
+	}
+}
+
 // Volume Creation Options
 
 // WithVolumeName sets the name of the volume.
diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go
index 9dfb35be3..b61ac2c30 100644
--- a/pkg/specgen/generate/container_create.go
+++ b/pkg/specgen/generate/container_create.go
@@ -238,6 +238,17 @@ 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 s.WorkDir == "" && img != nil {
+		newWD, err := img.WorkingDir(ctx)
+		if err != nil {
+			return nil, err
+		}
+		if newWD != "" {
+			options = append(options, libpod.WithCreateWorkingDir())
+		}
+	}
 	if s.StopSignal != nil {
 		options = append(options, libpod.WithStopSignal(*s.StopSignal))
 	}
diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
index 574c5c3f2..9cb76d1f6 100644
--- a/test/e2e/run_test.go
+++ b/test/e2e/run_test.go
@@ -1142,4 +1142,16 @@ USER mail`
 		Expect(session.ExitCode()).To(Not(Equal(0)))
 		Expect(session.ErrorToString()).To(ContainSubstring("Invalid umask"))
 	})
+
+	It("podman run makes entrypoint from image", func() {
+		// BuildImage does not seem to work remote
+		SkipIfRemote()
+		dockerfile := `FROM busybox
+WORKDIR /madethis`
+		podmanTest.BuildImage(dockerfile, "test", "false")
+		session := podmanTest.Podman([]string{"run", "--rm", "test", "pwd"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(ContainSubstring("/madethis"))
+	})
 })