Merge pull request #26682 from Devashish08/fix-empty-device-mode-error

Fix: Improve error message for empty device modes

pkg/specgen/generate/config_common.go

@@ -15,8 +15,11 @@ func ParseDevice(device string) (string, string, string, error) {
 	arr := strings.Split(device, ":")
 	switch len(arr) {
 	case 3:
+		if arr[2] == "" {
+			return "", "", "", fmt.Errorf("empty device mode in device specification: %s", device)
+		}
 		if !IsValidDeviceMode(arr[2]) {
-			return "", "", "", fmt.Errorf("invalid device mode: %s", arr[2])
+			return "", "", "", fmt.Errorf("invalid device mode %q in device %q", arr[2], device)
 		}
 		permissions = arr[2]
 		fallthrough
@@ -25,7 +28,7 @@ func ParseDevice(device string) (string, string, string, error) {
 			permissions = arr[1]
 		} else {
 			if len(arr[1]) > 0 && arr[1][0] != '/' {
-				return "", "", "", fmt.Errorf("invalid device mode: %s", arr[1])
+				return "", "", "", fmt.Errorf("invalid device mode %q in device %q", arr[1], device)
 			}
 			dst = arr[1]
 		}

pkg/specgen/generate/config_common_test.go

@@ -20,6 +20,7 @@ func TestParseDevice(t *testing.T) {
 		{"/dev/foo:/dev/bar:rw", "/dev/foo", "/dev/bar", "rw"},
 		{"/dev/foo:rw", "/dev/foo", "/dev/foo", "rw"},
 		{"/dev/foo::rw", "/dev/foo", "/dev/foo", "rw"},
+		{"/dev/foo:", "/dev/foo", "/dev/foo", "rwm"},
 	}
 	for _, test := range tests {
 		src, dst, perm, err := ParseDevice(test.device)
@@ -29,3 +30,26 @@ func TestParseDevice(t *testing.T) {
 		assert.Equal(t, perm, test.perm)
 	}
 }
+
+func TestParseDeviceErrors(t *testing.T) {
+	errorTests := []struct {
+		device        string
+		expectedError string
+	}{
+		{"/dev/fuse::", "empty device mode in device specification: /dev/fuse::"},
+		{"/dev/fuse:invalid", `invalid device mode "invalid" in device "/dev/fuse:invalid"`},
+		{"/dev/fuse:/path:xyz", `invalid device mode "xyz" in device "/dev/fuse:/path:xyz"`},
+		{"/dev/fuse:/path:rw:extra", `invalid device specification: /dev/fuse:/path:rw:extra`},
+		{"/dev/fuse:/path:rw:extra:more", `invalid device specification: /dev/fuse:/path:rw:extra:more`},
+		{"/dev/fuse:notapath", `invalid device mode "notapath" in device "/dev/fuse:notapath"`},
+		{"/dev/fuse:x", `invalid device mode "x" in device "/dev/fuse:x"`},
+		{"/dev/fuse:rwx", `invalid device mode "rwx" in device "/dev/fuse:rwx"`},
+		{"/dev/fuse:rrw", `invalid device mode "rrw" in device "/dev/fuse:rrw"`},
+	}
+
+	for _, test := range errorTests {
+		_, _, _, err := ParseDevice(test.device)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), test.expectedError)
+	}
+}

test/e2e/run_device_test.go

@@ -82,7 +82,13 @@ var _ = Describe("Podman run device", func() {
 	It("podman run device rename and bad permission test", func() {
 		session := podmanTest.Podman([]string{"run", "-q", "--security-opt", "label=disable", "--device", "/dev/kmsg:/dev/kmsg1:rd", ALPINE, "true"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitWithError(125, "invalid device mode: rd"))
+		Expect(session).Should(ExitWithError(125, "invalid device mode \"rd\" in device \"/dev/kmsg:/dev/kmsg1:rd\""))
+	})
+
+	It("podman run device with empty mode test", func() {
+		session := podmanTest.Podman([]string{"run", "-q", "--device", "/dev/fuse::", ALPINE, "true"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitWithError(125, "empty device mode in device specification: /dev/fuse::"))
 	})
 
 	It("podman run device host device and container device parameter are directories", func() {