diff --git a/daemon/oci_windows.go b/daemon/oci_windows.go index b3dbfa1db1..b7304e5fb2 100644 --- a/daemon/oci_windows.go +++ b/daemon/oci_windows.go @@ -254,27 +254,13 @@ func (daemon *Daemon) createSpecWindowsFields(c *container.Container, s *specs.S return err } - // Do we have any assigned devices? - if len(c.HostConfig.Devices) > 0 { - if isHyperV { - return errors.New("device assignment is not supported for HyperV containers") - } - for _, deviceMapping := range c.HostConfig.Devices { - srcParts := strings.SplitN(deviceMapping.PathOnHost, "/", 2) - if len(srcParts) != 2 { - return errors.New("invalid device assignment path") - } - if srcParts[0] != "class" { - return errors.Errorf("invalid device assignment type: '%s' should be 'class'", srcParts[0]) - } - wd := specs.WindowsDevice{ - ID: srcParts[1], - IDType: srcParts[0], - } - s.Windows.Devices = append(s.Windows.Devices, wd) - } + devices, err := setupWindowsDevices(c.HostConfig.Devices, isHyperV) + if err != nil { + return err } + s.Windows.Devices = append(s.Windows.Devices, devices...) + return nil } @@ -465,3 +451,30 @@ func readCredentialSpecFile(id, root, location string) (string, error) { } return string(bcontents[:]), nil } + +func setupWindowsDevices(devices []containertypes.DeviceMapping, isHyperV bool) (specDevices []specs.WindowsDevice, err error) { + if len(devices) == 0 { + return + } + + if isHyperV { + return nil, errors.New("device assignment is not supported for HyperV containers") + } + + for _, deviceMapping := range devices { + srcParts := strings.SplitN(deviceMapping.PathOnHost, "/", 2) + if len(srcParts) != 2 { + return nil, errors.New("invalid device assignment path") + } + if srcParts[0] != "class" { + return nil, errors.Errorf("invalid device assignment type: '%s' should be 'class'", srcParts[0]) + } + wd := specs.WindowsDevice{ + ID: srcParts[1], + IDType: srcParts[0], + } + specDevices = append(specDevices, wd) + } + + return +} diff --git a/daemon/oci_windows_test.go b/daemon/oci_windows_test.go index cc041bf3ea..29ad7d9d89 100644 --- a/daemon/oci_windows_test.go +++ b/daemon/oci_windows_test.go @@ -311,3 +311,63 @@ func setRegistryOpenKeyFunc(t *testing.T, key *dummyRegistryKey, err ...error) f registryOpenKeyFunc = previousRegistryOpenKeyFunc } } + +func TestSetupWindowsDevices(t *testing.T) { + t.Run("it does nothing if there are no devices and HyperV is disabled", func(t *testing.T) { + devices, err := setupWindowsDevices(nil, false) + assert.NilError(t, err) + assert.Equal(t, len(devices), 0) + }) + + t.Run("it does nothing if there are no devices and HyperV is enabled", func(t *testing.T) { + devices, err := setupWindowsDevices(nil, true) + assert.NilError(t, err) + assert.Equal(t, len(devices), 0) + }) + + t.Run("it fails if there are devices and HyperV is enabled", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "anything"}}, true) + assert.ErrorContains(t, err, "device assignment is not supported for HyperV containers") + assert.Equal(t, len(devices), 0) + }) + + t.Run("it fails if any devices are blank and HyperV is disabled", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: ""}}, false) + assert.ErrorContains(t, err, "invalid device assignment path") + assert.Equal(t, len(devices), 0) + }) + + t.Run("it fails if all devices do not contain '/' and HyperV is disabled", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "anything"}, {PathOnHost: "goes"}}, false) + assert.ErrorContains(t, err, "invalid device assignment path") + assert.Equal(t, len(devices), 0) + }) + + t.Run("it fails if any devices do not contain '/' and HyperV is disabled", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "goes"}}, false) + assert.ErrorContains(t, err, "invalid device assignment path") + assert.Equal(t, len(devices), 0) + }) + + t.Run("it fails if all devices do not have IDType 'class' and HyperV is disabled", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "klass/anything"}, {PathOnHost: "klass/goes"}}, false) + assert.ErrorContains(t, err, "invalid device assignment type: 'klass' should be 'class'") + assert.Equal(t, len(devices), 0) + }) + + t.Run("it fails if any devices do not have IDType 'class' and HyperV is disabled", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "klass/goes"}}, false) + assert.ErrorContains(t, err, "invalid device assignment type: 'klass' should be 'class'") + assert.Equal(t, len(devices), 0) + }) + + t.Run("it creates devices if all devices have IDType 'class' and HyperV is disabled", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "class/goes"}}, false) + expectedDevices := []specs.WindowsDevice{{IDType: "class", ID: "anything"}, {IDType: "class", ID: "goes"}} + assert.NilError(t, err) + assert.Equal(t, len(devices), len(expectedDevices)) + for i := range expectedDevices { + assert.Equal(t, devices[i], expectedDevices[i]) + } + }) +} diff --git a/integration/container/devices_windows_test.go b/integration/container/devices_windows_test.go new file mode 100644 index 0000000000..7c9c113aaa --- /dev/null +++ b/integration/container/devices_windows_test.go @@ -0,0 +1,67 @@ +package container // import "github.com/docker/docker/integration/container" + +import ( + "context" + "strings" + "testing" + "time" + + containertypes "github.com/docker/docker/api/types/container" + "github.com/docker/docker/integration/internal/container" + "gotest.tools/v3/assert" + "gotest.tools/v3/poll" + "gotest.tools/v3/skip" +) + +// TestWindowsDevices that Windows Devices are correctly propagated +// via HostConfig.Devices through to the implementation in hcsshim. +func TestWindowsDevices(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "windows") + defer setupTest(t)() + client := testEnv.APIClient() + ctx := context.Background() + + testData := []struct { + doc string + devices []string + expectedExitCode int + expectedStdout string + expectedStderr string + }{ + { + doc: "no device mounted", + expectedExitCode: 1, + }, + { + doc: "class/5B45201D-F2F2-4F3B-85BB-30FF1F953599 mounted", + devices: []string{"class/5B45201D-F2F2-4F3B-85BB-30FF1F953599"}, + expectedStdout: "/Windows/System32/HostDriverStore/FileRepository", + }, + } + + for _, d := range testData { + d := d + t.Run(d.doc, func(t *testing.T) { + t.Parallel() + deviceOptions := []func(*container.TestContainerConfig){container.WithIsolation(containertypes.IsolationProcess)} + for _, deviceName := range d.devices { + deviceOptions = append(deviceOptions, container.WithWindowsDevice(deviceName)) + } + id := container.Run(ctx, t, client, deviceOptions...) + + poll.WaitOn(t, container.IsInState(ctx, client, id, "running"), poll.WithDelay(100*time.Millisecond)) + + // /Windows/System32/HostDriverStore is mounted from the host when class GUID 5B45201D-F2F2-4F3B-85BB-30FF1F953599 + // is mounted. See `C:\windows\System32\containers\devices.def` on a Windows host for (slightly more) details. + res, err := container.Exec(ctx, client, id, []string{"sh", "-c", + "ls -d /Windows/System32/HostDriverStore/* | grep /Windows/System32/HostDriverStore/FileRepository"}) + assert.NilError(t, err) + assert.Equal(t, d.expectedExitCode, res.ExitCode) + if d.expectedExitCode == 0 { + assert.Equal(t, d.expectedStdout, strings.TrimSpace(res.Stdout())) + assert.Equal(t, d.expectedStderr, strings.TrimSpace(res.Stderr())) + } + + }) + } +} diff --git a/integration/internal/container/ops.go b/integration/internal/container/ops.go index dae5a2a512..0a600361aa 100644 --- a/integration/internal/container/ops.go +++ b/integration/internal/container/ops.go @@ -213,3 +213,17 @@ func WithPlatform(p *specs.Platform) func(*TestContainerConfig) { c.Platform = p } } + +// WithWindowsDevice specifies a Windows Device, ala `--device` on the CLI +func WithWindowsDevice(device string) func(*TestContainerConfig) { + return func(c *TestContainerConfig) { + c.HostConfig.Devices = append(c.HostConfig.Devices, containertypes.DeviceMapping{PathOnHost: device}) + } +} + +// WithIsolation specifies the isolation technology to apply to the container +func WithIsolation(isolation containertypes.Isolation) func(*TestContainerConfig) { + return func(c *TestContainerConfig) { + c.HostConfig.Isolation = isolation + } +}