Make Inspect's mounts struct accurate to Docker

We were formerly dumping spec.Mount structs, with no care as to
whether it was user-generated or not - a relic of the very early
days when we didn't know whether a user made a mount or not.

Now that we do, match our output to Docker's dedicated mount
struct.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

libpod/container_inspect.go

@@ -6,7 +6,7 @@ import (
 
 	"github.com/containers/libpod/libpod/driver"
 	"github.com/cri-o/ocicni/pkg/ocicni"
-	specs "github.com/opencontainers/runtime-spec/specs-go"
+	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -44,7 +44,7 @@ type InspectContainerData struct {
 	GraphDriver     *driver.Data            `json:"GraphDriver"`
 	SizeRw          int64                   `json:"SizeRw,omitempty"`
 	SizeRootFs      int64                   `json:"SizeRootFs,omitempty"`
-	Mounts          []specs.Mount           `json:"Mounts"`
+	Mounts          []*InspectMount         `json:"Mounts"`
 	Dependencies    []string                `json:"Dependencies"`
 	NetworkSettings *InspectNetworkSettings `json:"NetworkSettings"` //TODO
 	ExitCommand     []string                `json:"ExitCommand"`
@@ -52,6 +52,31 @@ type InspectContainerData struct {
 	IsInfra         bool                    `json:"IsInfra"`
 }
 
+// InspectMount provides a record of a single mount in a container. It contains
+// fields for both named and normal volumes. Only user-specified volumes will be
+// included, and tmpfs volumes are not included even if the user specified them.
+type InspectMount struct {
+	// Whether the mount is a volume or bind mount. Allowed values are
+	// "volume" and "bind".
+	Type string `json:"Type"`
+	// The name of the volume. Empty for bind mounts.
+	Name string `json:"Name,omptempty"`
+	// The source directory for the volume.
+	Src string `json:"Source"`
+	// The destination directory for the volume. Specified as a path within
+	// the container, as it would be passed into the OCI runtime.
+	Dst string `json:"Destination"`
+	// The driver used for the named volume. Empty for bind mounts.
+	Driver string `json:"Driver"`
+	// Mount options for the driver, excluding RO/RW and mount propagation.
+	Mode string `json:"Mode"`
+	// Whether the volume is read-write
+	RW bool `json:"RW"`
+	// Mount propagation for the mount. Can be empty if not specified, but
+	// is always printed - no omitempty.
+	Propagation string `json:"Propagation"`
+}
+
 // InspectContainerState provides a detailed record of a container's current
 // state. It is returned as part of InspectContainerData.
 // As with InspectContainerData, many portions of this struct are matched to
@@ -149,34 +174,24 @@ func (c *Container) getContainerInspectData(size bool, driverData *driver.Data)
 		execIDs = append(execIDs, id)
 	}
 
-	if c.state.BindMounts == nil {
-		c.state.BindMounts = make(map[string]string)
-	}
-
 	resolvPath := ""
+	hostsPath := ""
+	hostnamePath := ""
+	if c.state.BindMounts != nil {
 		if getPath, ok := c.state.BindMounts["/etc/resolv.conf"]; ok {
 			resolvPath = getPath
 		}
-
-	hostsPath := ""
 		if getPath, ok := c.state.BindMounts["/etc/hosts"]; ok {
 			hostsPath = getPath
 		}
-
-	hostnamePath := ""
 		if getPath, ok := c.state.BindMounts["/etc/hostname"]; ok {
 			hostnamePath = getPath
 		}
-
-	var mounts []specs.Mount
-	for i, mnt := range spec.Mounts {
-		mounts = append(mounts, mnt)
-		// We only want to show the name of the named volume in the inspect
-		// output, so split the path and get the name out of it.
-		if strings.Contains(mnt.Source, c.runtime.config.VolumePath) {
-			split := strings.Split(mnt.Source[len(c.runtime.config.VolumePath)+1:], "/")
-			mounts[i].Source = split[0]
 	}
+
+	mounts, err := c.getInspectMounts()
+	if err != nil {
+		return nil, err
 	}
 
 	data := &InspectContainerData{
@@ -280,3 +295,102 @@ func (c *Container) getContainerInspectData(size bool, driverData *driver.Data)
 	}
 	return data, nil
 }
+
+// Get inspect-formatted mounts list.
+// Only includes user-specified mounts. Only includes bind mounts and named
+// volumes, not tmpfs volumes.
+func (c *Container) getInspectMounts() ([]*InspectMount, error) {
+	inspectMounts := []*InspectMount{}
+
+	// No mounts, return early
+	if len(c.config.UserVolumes) == 0 {
+		return inspectMounts, nil
+	}
+
+	// We need to parse all named volumes and mounts into maps, so we don't
+	// end up with repeated lookups for each user volume.
+	// Map destination to struct, as destination is what is stored in
+	// UserVolumes.
+	namedVolumes := make(map[string]*ContainerNamedVolume)
+	mounts := make(map[string]spec.Mount)
+	for _, namedVol := range c.config.NamedVolumes {
+		namedVolumes[namedVol.Dest] = namedVol
+	}
+	for _, mount := range c.config.Spec.Mounts {
+		mounts[mount.Destination] = mount
+	}
+
+	for _, vol := range c.config.UserVolumes {
+		// We need to look up the volumes.
+		// First: is it a named volume?
+		if volume, ok := namedVolumes[vol]; ok {
+			mountStruct := new(InspectMount)
+			mountStruct.Type = "volume"
+			mountStruct.Dst = volume.Dest
+			mountStruct.Name = volume.Name
+
+			// For src and driver, we need to look up the named
+			// volume.
+			volFromDB, err := c.runtime.state.Volume(volume.Name)
+			if err != nil {
+				return nil, errors.Wrapf(err, "error looking up volume %s in container %s config", volume.Name, c.ID())
+			}
+			mountStruct.Driver = volFromDB.Driver()
+			mountStruct.Src = volFromDB.MountPoint()
+
+			parseMountOptionsForInspect(volume.Options, mountStruct)
+
+			inspectMounts = append(inspectMounts, mountStruct)
+		} else if mount, ok := mounts[vol]; ok {
+			// It's a mount.
+			// Is it a tmpfs? If so, discard.
+			if mount.Type == "tmpfs" {
+				continue
+			}
+
+			mountStruct := new(InspectMount)
+			mountStruct.Type = "bind"
+			mountStruct.Src = mount.Source
+			mountStruct.Dst = mount.Destination
+
+			parseMountOptionsForInspect(mount.Options, mountStruct)
+
+			inspectMounts = append(inspectMounts, mountStruct)
+		}
+		// We couldn't find a mount. Log a warning.
+		logrus.Warnf("Could not find mount at destination %q when building inspect output for container %s", vol, c.ID())
+	}
+
+	return inspectMounts, nil
+}
+
+// Parse mount options so we can populate them in the mount structure.
+// The mount passed in will be modified.
+func parseMountOptionsForInspect(options []string, mount *InspectMount) {
+	isRW := true
+	mountProp := ""
+	otherOpts := []string{}
+
+	// Some of these may be overwritten if the user passes us garbage opts
+	// (for example, [ro,rw])
+	// We catch these on the Podman side, so not a problem there, but other
+	// users of libpod who do not properly validate mount options may see
+	// this.
+	// Not really worth dealing with on our end - garbage in, garbage out.
+	for _, opt := range options {
+		switch opt {
+		case "ro":
+			isRW = false
+		case "rw":
+			// Do nothing, silently discard
+		case "shared", "slave", "private", "rshared", "rslave", "rprivate":
+			mountProp = opt
+		default:
+			otherOpts = append(otherOpts, opt)
+		}
+	}
+
+	mount.RW = isRW
+	mount.Propagation = mountProp
+	mount.Mode = strings.Join(otherOpts, ",")
+}

libpod/options.go

@@ -1127,6 +1127,8 @@ func WithGroups(groups []string) CtrCreateOption {
 // These are not added to the container's spec, but will instead be used during
 // commit to populate the volumes of the new image, and to trigger some OCI
 // hooks that are only added if volume mounts are present.
+// Furthermore, they are used in the output of inspect, to filter volumes -
+// only volumes included in this list will be included in the output.
 // Unless explicitly set, committed images will have no volumes.
 // The given volumes slice must not be nil.
 func WithUserVolumes(volumes []string) CtrCreateOption {