Implement :// separator for arbitrary Windows Device IDTypes

Arbitrary here does not include '', best to catch that one early as it's
almost certainly a mistake (possibly an attempt to pass a POSIX path
through this API)

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>

daemon/oci_windows.go

@@ -458,12 +458,17 @@ func setupWindowsDevices(devices []containertypes.DeviceMapping) (specDevices []
 	}
 
 	for _, deviceMapping := range devices {
-		srcParts := strings.SplitN(deviceMapping.PathOnHost, "/", 2)
+		devicePath := deviceMapping.PathOnHost
+		if strings.HasPrefix(devicePath, "class/") {
+			devicePath = strings.Replace(devicePath, "class/", "class://", 1)
+		}
+
+		srcParts := strings.SplitN(devicePath, "://", 2)
 		if len(srcParts) != 2 {
-			return nil, errors.New("invalid device assignment path")
+			return nil, errors.Errorf("invalid device assignment path: '%s', must be 'class/ID' or 'IDType://ID'", deviceMapping.PathOnHost)
 		}
-		if srcParts[0] != "class" {
-			return nil, errors.Errorf("invalid device assignment type: '%s' should be 'class'", srcParts[0])
+		if srcParts[0] == "" {
+			return nil, errors.Errorf("invalid device assignment path: '%s', IDType cannot be empty", deviceMapping.PathOnHost)
 		}
 		wd := specs.WindowsDevice{
 			ID:     srcParts[1],

daemon/oci_windows_test.go

@@ -322,34 +322,46 @@ func TestSetupWindowsDevices(t *testing.T) {
 	t.Run("it fails if any devices are blank", func(t *testing.T) {
 		devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: ""}})
 		assert.ErrorContains(t, err, "invalid device assignment path")
+		assert.ErrorContains(t, err, "''")
 		assert.Equal(t, len(devices), 0)
 	})
 
-	t.Run("it fails if all devices do not contain '/'", func(t *testing.T) {
+	t.Run("it fails if all devices do not contain '/' or '://'", func(t *testing.T) {
 		devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "anything"}, {PathOnHost: "goes"}})
 		assert.ErrorContains(t, err, "invalid device assignment path")
+		assert.ErrorContains(t, err, "'anything'")
 		assert.Equal(t, len(devices), 0)
 	})
 
-	t.Run("it fails if any devices do not contain '/'", func(t *testing.T) {
+	t.Run("it fails if any devices do not contain '/' or '://'", func(t *testing.T) {
 		devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "goes"}})
 		assert.ErrorContains(t, err, "invalid device assignment path")
+		assert.ErrorContains(t, err, "'goes'")
 		assert.Equal(t, len(devices), 0)
 	})
 
-	t.Run("it fails if all devices do not have IDType 'class'", func(t *testing.T) {
+	t.Run("it fails if all '/'-separated devices do not have IDType 'class'", func(t *testing.T) {
 		devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "klass/anything"}, {PathOnHost: "klass/goes"}})
-		assert.ErrorContains(t, err, "invalid device assignment type: 'klass' should be 'class'")
+		assert.ErrorContains(t, err, "invalid device assignment path")
+		assert.ErrorContains(t, err, "'klass/anything'")
 		assert.Equal(t, len(devices), 0)
 	})
 
-	t.Run("it fails if any devices do not have IDType 'class'", func(t *testing.T) {
+	t.Run("it fails if any '/'-separated devices do not have IDType 'class'", func(t *testing.T) {
 		devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "klass/goes"}})
-		assert.ErrorContains(t, err, "invalid device assignment type: 'klass' should be 'class'")
+		assert.ErrorContains(t, err, "invalid device assignment path")
+		assert.ErrorContains(t, err, "'klass/goes'")
+		assert.Equal(t, len(devices), 0)
+	})
+
+	t.Run("it fails if any '://'-separated devices have IDType ''", func(t *testing.T) {
+		devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "://goes"}})
+		assert.ErrorContains(t, err, "invalid device assignment path")
+		assert.ErrorContains(t, err, "'://goes'")
 		assert.Equal(t, len(devices), 0)
 	})
 
-	t.Run("it creates devices if all devices have IDType 'class'", func(t *testing.T) {
+	t.Run("it creates devices if all '/'-separated devices have IDType 'class'", func(t *testing.T) {
 		devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "class/goes"}})
 		expectedDevices := []specs.WindowsDevice{{IDType: "class", ID: "anything"}, {IDType: "class", ID: "goes"}}
 		assert.NilError(t, err)
@@ -358,4 +370,24 @@ func TestSetupWindowsDevices(t *testing.T) {
 			assert.Equal(t, devices[i], expectedDevices[i])
 		}
 	})
+
+	t.Run("it creates devices if all '://'-separated devices have non-blank IDType", func(t *testing.T) {
+		devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class://anything"}, {PathOnHost: "klass://goes"}})
+		expectedDevices := []specs.WindowsDevice{{IDType: "class", ID: "anything"}, {IDType: "klass", ID: "goes"}}
+		assert.NilError(t, err)
+		assert.Equal(t, len(devices), len(expectedDevices))
+		for i := range expectedDevices {
+			assert.Equal(t, devices[i], expectedDevices[i])
+		}
+	})
+
+	t.Run("it creates devices when given a mix of '/'-separated and '://'-separated devices", func(t *testing.T) {
+		devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "klass://goes"}})
+		expectedDevices := []specs.WindowsDevice{{IDType: "class", ID: "anything"}, {IDType: "klass", ID: "goes"}}
+		assert.NilError(t, err)
+		assert.Equal(t, len(devices), len(expectedDevices))
+		for i := range expectedDevices {
+			assert.Equal(t, devices[i], expectedDevices[i])
+		}
+	})
 }

docs/api/version-history.md

@@ -65,6 +65,11 @@ keywords: "API, Docker, rcli, REST, documentation"
     - "locked"
     - "active/worker"
     - "active/manager"
+* `POST /containers/create` for Windows containers now accepts a new syntax in
+  `HostConfig.Resources.Devices.PathOnHost`. As well as the existing `class/<GUID>`
+  syntax, `<IDType>://<ID>` is now recognised. Support for specific `<IDType>` values
+  depends on the underlying implementation and Windows version. This change is not
+  versioned, and affects all API versions if the daemon has this patch.
 
 ## v1.41 API changes
 

integration/container/devices_windows_test.go

@@ -43,6 +43,20 @@ func TestWindowsDevices(t *testing.T) {
 			isolation:      containertypes.IsolationProcess,
 			expectedStdout: "/Windows/System32/HostDriverStore/FileRepository",
 		},
+		{
+			doc:            "process/class://5B45201D-F2F2-4F3B-85BB-30FF1F953599 mounted",
+			devices:        []string{"class://5B45201D-F2F2-4F3B-85BB-30FF1F953599"},
+			isolation:      containertypes.IsolationProcess,
+			expectedStdout: "/Windows/System32/HostDriverStore/FileRepository",
+		},
+		{
+			doc:                         "process/vpci-class-guid://5B45201D-F2F2-4F3B-85BB-30FF1F953599 mounted",
+			devices:                     []string{"vpci-class-guid://5B45201D-F2F2-4F3B-85BB-30FF1F953599"},
+			isolation:                   containertypes.IsolationProcess,
+			expectedStartFailure:        !testEnv.RuntimeIsWindowsContainerd(),
+			expectedStartFailureMessage: "device assignment of type 'vpci-class-guid' is not supported",
+			expectedStdout:              "/Windows/System32/HostDriverStore/FileRepository",
+		},
 		{
 			doc:              "hyperv/no device mounted",
 			isolation:        containertypes.IsolationHyperV,
@@ -56,6 +70,22 @@ func TestWindowsDevices(t *testing.T) {
 			expectedStartFailureMessage: "device assignment is not supported for HyperV containers",
 			expectedStdout:              "/Windows/System32/HostDriverStore/FileRepository",
 		},
+		{
+			doc:                         "hyperv/class://5B45201D-F2F2-4F3B-85BB-30FF1F953599 mounted",
+			devices:                     []string{"class://5B45201D-F2F2-4F3B-85BB-30FF1F953599"},
+			isolation:                   containertypes.IsolationHyperV,
+			expectedStartFailure:        !testEnv.RuntimeIsWindowsContainerd(),
+			expectedStartFailureMessage: "device assignment is not supported for HyperV containers",
+			expectedStdout:              "/Windows/System32/HostDriverStore/FileRepository",
+		},
+		{
+			doc:                         "hyperv/vpci-class-guid://5B45201D-F2F2-4F3B-85BB-30FF1F953599 mounted",
+			devices:                     []string{"vpci-class-guid://5B45201D-F2F2-4F3B-85BB-30FF1F953599"},
+			isolation:                   containertypes.IsolationHyperV,
+			expectedStartFailure:        !testEnv.RuntimeIsWindowsContainerd(),
+			expectedStartFailureMessage: "device assignment is not supported for HyperV containers",
+			expectedStdout:              "/Windows/System32/HostDriverStore/FileRepository",
+		},
 	}
 
 	for _, d := range testData {

libcontainerd/local/local_windows.go

@@ -312,6 +312,11 @@ func (c *client) createWindows(id string, spec *specs.Spec, runtimeOptions inter
 			return errors.New("device assignment is not supported for HyperV containers")
 		}
 		for _, d := range spec.Windows.Devices {
+			// Per https://github.com/microsoft/hcsshim/blob/v0.9.2/internal/uvm/virtual_device.go#L17-L18,
+			// this represents an Interface Class GUID.
+			if d.IDType != "class" {
+				return errors.Errorf("device assignment of type '%s' is not supported", d.IDType)
+			}
 			configuration.AssignedDevices = append(configuration.AssignedDevices, hcsshim.AssignedDevice{InterfaceClassGUID: d.ID})
 		}
 	}