Fix handling of devices

Devices are supposed to be able to be passed in via the form of

--device /dev/foo
--device /dev/foo:/dev/bar
--device /dev/foo:rwm
--device /dev/foo:/dev/bar:rwm

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

Closes: #1299
Approved by: umohnani8

4 files changed, 89 insertions, 85 deletions

diff --git a/cmd/podman/parse.go b/cmd/podman/parse.go
index 22197ba20..158a006fb 100644
--- a/cmd/podman/parse.go
+++ b/cmd/podman/parse.go
@@ -38,19 +38,6 @@ type PortMapping struct {
 	HostIp string `protobuf:"bytes,4,opt,name=host_ip,json=hostIp,proto3" json:"host_ip,omitempty"`
 }
 
-// Device specifies a host device to mount into a container.
-type HostDevice struct {
-	// Path of the device within the container.
-	ContainerPath string `protobuf:"bytes,1,opt,name=container_path,json=containerPath,proto3" json:"container_path,omitempty"`
-	// Path of the device on the host.
-	HostPath string `protobuf:"bytes,2,opt,name=host_path,json=hostPath,proto3" json:"host_path,omitempty"`
-	// Cgroups permissions of the device, candidates are one or more of
-	// * r - allows container to read from the specified device.
-	// * w - allows container to write to the specified device.
-	// * m - allows container to create device files that do not yet exist.
-	Permissions string `protobuf:"bytes,3,opt,name=permissions,proto3" json:"permissions,omitempty"`
-}
-
 // Note: for flags that are in the form <number><unit>, use the RAMInBytes function
 // from the units package in docker/go-units/size.go
 
@@ -106,75 +93,6 @@ func validateBlkioWeight(val int64) (int64, error) { //nolint
 	return -1, errors.Errorf("invalid blkio weight %q, should be between 10 and 1000", val)
 }
 
-// parseDevice parses a device mapping string to a container.DeviceMapping struct
-// for device flag
-func parseDevice(device string) (*HostDevice, error) { //nolint
-	_, err := validateDevice(device)
-	if err != nil {
-		return nil, errors.Wrapf(err, "device string not valid %q", device)
-	}
-
-	src := ""
-	dst := ""
-	permissions := "rwm"
-	arr := strings.Split(device, ":")
-	switch len(arr) {
-	case 3:
-		permissions = arr[2]
-		fallthrough
-	case 2:
-		if validDeviceMode(arr[1]) {
-			permissions = arr[1]
-		} else {
-			dst = arr[1]
-		}
-		fallthrough
-	case 1:
-		src = arr[0]
-	default:
-		return nil, fmt.Errorf("invalid device specification: %s", device)
-	}
-
-	if dst == "" {
-		dst = src
-	}
-
-	deviceMapping := &HostDevice{
-		ContainerPath: dst,
-		HostPath:      src,
-		Permissions:   permissions,
-	}
-	return deviceMapping, nil
-}
-
-// validDeviceMode checks if the mode for device is valid or not.
-// Valid mode is a composition of r (read), w (write), and m (mknod).
-func validDeviceMode(mode string) bool {
-	var legalDeviceMode = map[rune]bool{
-		'r': true,
-		'w': true,
-		'm': true,
-	}
-	if mode == "" {
-		return false
-	}
-	for _, c := range mode {
-		if !legalDeviceMode[c] {
-			return false
-		}
-		legalDeviceMode[c] = false
-	}
-	return true
-}
-
-// validateDevice validates a path for devices
-// It will make sure 'val' is in the form:
-//    [host-dir:]container-path[:mode]
-// It also validates the device mode.
-func validateDevice(val string) (string, error) {
-	return validatePath(val, validDeviceMode)
-}
-
 func validatePath(val string, validator func(string) bool) (string, error) {
 	var containerPath string
 	var mode string
diff --git a/pkg/spec/config_linux.go b/pkg/spec/config_linux.go
index cd633bc69..ea04b95bd 100644
--- a/pkg/spec/config_linux.go
+++ b/pkg/spec/config_linux.go
@@ -28,10 +28,15 @@ func Device(d *configs.Device) spec.LinuxDevice {
 }
 
 func addDevice(g *generate.Generator, device string) error {
-	dev, err := devices.DeviceFromPath(device, "rwm")
+	src, dst, permissions, err := parseDevice(device)
 	if err != nil {
-		return errors.Wrapf(err, "%s is not a valid device", device)
+		return err
+	}
+	dev, err := devices.DeviceFromPath(src, permissions)
+	if err != nil {
+		return errors.Wrapf(err, "%s is not a valid device", src)
 	}
+	dev.Path = dst
 	linuxdev := spec.LinuxDevice{
 		Path:     dev.Path,
 		Type:     string(dev.Type),
diff --git a/pkg/spec/parse.go b/pkg/spec/parse.go
index d4a655e4f..d34e10760 100644
--- a/pkg/spec/parse.go
+++ b/pkg/spec/parse.go
@@ -126,3 +126,58 @@ func getLoggingPath(opts []string) string {
 	}
 	return ""
 }
+
+// parseDevice parses device mapping string to a src, dest & permissions string
+func parseDevice(device string) (string, string, string, error) { //nolint
+	src := ""
+	dst := ""
+	permissions := "rwm"
+	arr := strings.Split(device, ":")
+	switch len(arr) {
+	case 3:
+		if !validDeviceMode(arr[2]) {
+			return "", "", "", fmt.Errorf("invalid device mode: %s", arr[2])
+		}
+		permissions = arr[2]
+		fallthrough
+	case 2:
+		if validDeviceMode(arr[1]) {
+			permissions = arr[1]
+		} else {
+			if arr[1][0] != '/' {
+				return "", "", "", fmt.Errorf("invalid device mode: %s", arr[2])
+			}
+			dst = arr[1]
+		}
+		fallthrough
+	case 1:
+		src = arr[0]
+	default:
+		return "", "", "", fmt.Errorf("invalid device specification: %s", device)
+	}
+
+	if dst == "" {
+		dst = src
+	}
+	return src, dst, permissions, nil
+}
+
+// validDeviceMode checks if the mode for device is valid or not.
+// Valid mode is a composition of r (read), w (write), and m (mknod).
+func validDeviceMode(mode string) bool {
+	var legalDeviceMode = map[rune]bool{
+		'r': true,
+		'w': true,
+		'm': true,
+	}
+	if mode == "" {
+		return false
+	}
+	for _, c := range mode {
+		if !legalDeviceMode[c] {
+			return false
+		}
+		legalDeviceMode[c] = false
+	}
+	return true
+}
diff --git a/test/e2e/run_device_test.go b/test/e2e/run_device_test.go
index eb1b423dd..fedd696d1 100644
--- a/test/e2e/run_device_test.go
+++ b/test/e2e/run_device_test.go
@@ -8,7 +8,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-var _ = Describe("Podman kill", func() {
+var _ = Describe("Podman run device", func() {
 	var (
 		tempdir    string
 		err        error
@@ -43,4 +43,30 @@ var _ = Describe("Podman kill", func() {
 		Expect(session.ExitCode()).To(Equal(0))
 		Expect(session.OutputToString()).To(Equal("/dev/kmsg"))
 	})
+
+	It("podman run device rename test", func() {
+		session := podmanTest.Podman([]string{"run", "-q", "--device", "/dev/kmsg:/dev/kmsg1", ALPINE, "ls", "--color=never", "/dev/kmsg1"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal("/dev/kmsg1"))
+	})
+
+	It("podman run device permission test", func() {
+		session := podmanTest.Podman([]string{"run", "-q", "--device", "/dev/kmsg:r", ALPINE, "ls", "--color=never", "/dev/kmsg"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal("/dev/kmsg"))
+	})
+
+	It("podman run device rename and permission test", func() {
+		session := podmanTest.Podman([]string{"run", "-q", "--device", "/dev/kmsg:/dev/kmsg1:r", ALPINE, "ls", "--color=never", "/dev/kmsg1"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal("/dev/kmsg1"))
+	})
+	It("podman run device rename and bad permission test", func() {
+		session := podmanTest.Podman([]string{"run", "-q", "--device", "/dev/kmsg:/dev/kmsg1:rd", ALPINE, "ls", "--color=never", "/dev/kmsg1"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Not(Equal(0)))
+	})
 })