diff --git a/cli/command/container/create.go b/cli/command/container/create.go index 49a3d4356c..868e4fb4ba 100644 --- a/cli/command/container/create.go +++ b/cli/command/container/create.go @@ -72,7 +72,7 @@ func runCreate(dockerCli command.Cli, flags *pflag.FlagSet, options *createOptio } } copts.env = *opts.NewListOptsRef(&newEnv, nil) - containerConfig, err := parse(flags, copts) + containerConfig, err := parse(flags, copts, dockerCli.ServerInfo().OSType) if err != nil { reportError(dockerCli.Err(), "create", err.Error(), true) return cli.StatusError{StatusCode: 125} diff --git a/cli/command/container/opts.go b/cli/command/container/opts.go index bce5c231e0..4546839629 100644 --- a/cli/command/container/opts.go +++ b/cli/command/container/opts.go @@ -141,7 +141,7 @@ func addFlags(flags *pflag.FlagSet) *containerOptions { deviceReadIOps: opts.NewThrottledeviceOpt(opts.ValidateThrottleIOpsDevice), deviceWriteBps: opts.NewThrottledeviceOpt(opts.ValidateThrottleBpsDevice), deviceWriteIOps: opts.NewThrottledeviceOpt(opts.ValidateThrottleIOpsDevice), - devices: opts.NewListOpts(validateDevice), + devices: opts.NewListOpts(nil), // devices can only be validated after we know the server OS env: opts.NewListOpts(opts.ValidateEnv), envFile: opts.NewListOpts(nil), expose: opts.NewListOpts(nil), @@ -299,7 +299,7 @@ type containerConfig struct { // a HostConfig and returns them with the specified command. // If the specified args are not valid, it will return an error. // nolint: gocyclo -func parse(flags *pflag.FlagSet, copts *containerOptions) (*containerConfig, error) { +func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*containerConfig, error) { var ( attachStdin = copts.attach.Get("stdin") attachStdout = copts.attach.Get("stdout") @@ -417,10 +417,22 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*containerConfig, err } } - // parse device mappings + // validate and parse device mappings. Note we do late validation of the + // device path (as opposed to during flag parsing), as at the time we are + // parsing flags, we haven't yet sent a _ping to the daemon to determine + // what operating system it is. deviceMappings := []container.DeviceMapping{} for _, device := range copts.devices.GetAll() { - deviceMapping, err := parseDevice(device) + var ( + validated string + deviceMapping container.DeviceMapping + err error + ) + validated, err = validateDevice(device, serverOS) + if err != nil { + return nil, err + } + deviceMapping, err = parseDevice(validated, serverOS) if err != nil { return nil, err } @@ -747,7 +759,19 @@ func parseStorageOpts(storageOpts []string) (map[string]string, error) { } // parseDevice parses a device mapping string to a container.DeviceMapping struct -func parseDevice(device string) (container.DeviceMapping, error) { +func parseDevice(device, serverOS string) (container.DeviceMapping, error) { + switch serverOS { + case "linux": + return parseLinuxDevice(device) + case "windows": + return parseWindowsDevice(device) + } + return container.DeviceMapping{}, errors.Errorf("unknown server OS: %s", serverOS) +} + +// parseLinuxDevice parses a device mapping string to a container.DeviceMapping struct +// knowing that the target is a Linux daemon +func parseLinuxDevice(device string) (container.DeviceMapping, error) { src := "" dst := "" permissions := "rwm" @@ -781,6 +805,12 @@ func parseDevice(device string) (container.DeviceMapping, error) { return deviceMapping, nil } +// parseWindowsDevice parses a device mapping string to a container.DeviceMapping struct +// knowing that the target is a Windows daemon +func parseWindowsDevice(device string) (container.DeviceMapping, error) { + return container.DeviceMapping{PathOnHost: device}, nil +} + // validateDeviceCgroupRule validates a device cgroup rule string format // It will make sure 'val' is in the form: // 'type major:minor mode' @@ -813,14 +843,23 @@ func validDeviceMode(mode string) bool { } // validateDevice validates a path for devices +func validateDevice(val string, serverOS string) (string, error) { + switch serverOS { + case "linux": + return validateLinuxPath(val, validDeviceMode) + case "windows": + // Windows does validation entirely server-side + return val, nil + } + return "", errors.Errorf("unknown server OS: %s", serverOS) +} + +// validateLinuxPath is the implementation of validateDevice knowing that the +// target server operating system is a Linux daemon. // 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) { +func validateLinuxPath(val string, validator func(string) bool) (string, error) { var containerPath string var mode string diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go index 70bedc6617..3a6aa58570 100644 --- a/cli/command/container/opts_test.go +++ b/cli/command/container/opts_test.go @@ -16,6 +16,7 @@ import ( "github.com/spf13/pflag" "gotest.tools/assert" is "gotest.tools/assert/cmp" + "gotest.tools/skip" ) func TestValidateAttach(t *testing.T) { @@ -48,7 +49,7 @@ func parseRun(args []string) (*container.Config, *container.HostConfig, *network return nil, nil, nil, err } // TODO: fix tests to accept ContainerConfig - containerConfig, err := parse(flags, copts) + containerConfig, err := parse(flags, copts, runtime.GOOS) if err != nil { return nil, nil, nil, err } @@ -351,6 +352,7 @@ func TestParseWithExpose(t *testing.T) { } func TestParseDevice(t *testing.T) { + skip.If(t, runtime.GOOS == "windows") // Windows validates server-side valids := map[string]container.DeviceMapping{ "/dev/snd": { PathOnHost: "/dev/snd", @@ -393,7 +395,7 @@ func TestParseModes(t *testing.T) { flags, copts := setupRunFlags() args := []string{"--pid=container:", "img", "cmd"} assert.NilError(t, flags.Parse(args)) - _, err := parse(flags, copts) + _, err := parse(flags, copts, runtime.GOOS) assert.ErrorContains(t, err, "--pid: invalid PID mode") // pid ok @@ -615,6 +617,7 @@ func TestParseEntryPoint(t *testing.T) { } func TestValidateDevice(t *testing.T) { + skip.If(t, runtime.GOOS == "windows") // Windows validates server-side valid := []string{ "/home", "/home:/home", @@ -649,13 +652,13 @@ func TestValidateDevice(t *testing.T) { } for _, path := range valid { - if _, err := validateDevice(path); err != nil { + if _, err := validateDevice(path, runtime.GOOS); err != nil { t.Fatalf("ValidateDevice(`%q`) should succeed: error %q", path, err) } } for path, expectedError := range invalid { - if _, err := validateDevice(path); err == nil { + if _, err := validateDevice(path, runtime.GOOS); err == nil { t.Fatalf("ValidateDevice(`%q`) should have failed validation", path) } else { if err.Error() != expectedError { diff --git a/cli/command/container/run.go b/cli/command/container/run.go index e42e8115e1..2ce3f3679c 100644 --- a/cli/command/container/run.go +++ b/cli/command/container/run.go @@ -78,7 +78,7 @@ func runRun(dockerCli command.Cli, flags *pflag.FlagSet, ropts *runOptions, copt } } copts.env = *opts.NewListOptsRef(&newEnv, nil) - containerConfig, err := parse(flags, copts) + containerConfig, err := parse(flags, copts, dockerCli.ServerInfo().OSType) // just in case the parse does not exit if err != nil { reportError(dockerCli.Err(), "run", err.Error(), true)