From 29d22b9aad7edd48c2a315e2775e33a4d76899e4 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 21 Dec 2015 17:43:59 -0500 Subject: [PATCH 1/3] Move ParseExec to the client where it is used. Signed-off-by: Daniel Nephin Upstream-commit: 4c0d586bd3a1b81cfba78af89af02be56041bc6b Component: engine --- components/engine/api/client/exec.go | 47 +++++++++++++++++- .../{runconfig => api/client}/exec_test.go | 2 +- .../engine/runconfig/{ => client}/parse.go | 0 .../runconfig/{ => client}/parse_test.go | 0 components/engine/runconfig/exec.go | 49 ------------------- 5 files changed, 46 insertions(+), 52 deletions(-) rename components/engine/{runconfig => api/client}/exec_test.go (99%) rename components/engine/runconfig/{ => client}/parse.go (100%) rename components/engine/runconfig/{ => client}/parse_test.go (100%) delete mode 100644 components/engine/runconfig/exec.go diff --git a/components/engine/api/client/exec.go b/components/engine/api/client/exec.go index ca30ee5a11..ac2e65868d 100644 --- a/components/engine/api/client/exec.go +++ b/components/engine/api/client/exec.go @@ -7,8 +7,8 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/api/types" Cli "github.com/docker/docker/cli" + flag "github.com/docker/docker/pkg/mflag" "github.com/docker/docker/pkg/promise" - "github.com/docker/docker/runconfig" ) // CmdExec runs a command in a running container. @@ -18,7 +18,7 @@ func (cli *DockerCli) CmdExec(args ...string) error { cmd := Cli.Subcmd("exec", []string{"CONTAINER COMMAND [ARG...]"}, Cli.DockerCommands["exec"].Description, true) detachKeys := cmd.String([]string{"-detach-keys"}, "", "Override the key sequence for detaching a container") - execConfig, err := runconfig.ParseExec(cmd, args) + execConfig, err := ParseExec(cmd, args) // just in case the ParseExec does not exit if execConfig.Container == "" || err != nil { return Cli.StatusError{StatusCode: 1} @@ -113,3 +113,46 @@ func (cli *DockerCli) CmdExec(args ...string) error { return nil } + +// ParseExec parses the specified args for the specified command and generates +// an ExecConfig from it. +// If the minimal number of specified args is not right or if specified args are +// not valid, it will return an error. +func ParseExec(cmd *flag.FlagSet, args []string) (*types.ExecConfig, error) { + var ( + flStdin = cmd.Bool([]string{"i", "-interactive"}, false, "Keep STDIN open even if not attached") + flTty = cmd.Bool([]string{"t", "-tty"}, false, "Allocate a pseudo-TTY") + flDetach = cmd.Bool([]string{"d", "-detach"}, false, "Detached mode: run command in the background") + flUser = cmd.String([]string{"u", "-user"}, "", "Username or UID (format: [:])") + flPrivileged = cmd.Bool([]string{"-privileged"}, false, "Give extended privileges to the command") + execCmd []string + container string + ) + cmd.Require(flag.Min, 2) + if err := cmd.ParseFlags(args, true); err != nil { + return nil, err + } + container = cmd.Arg(0) + parsedArgs := cmd.Args() + execCmd = parsedArgs[1:] + + execConfig := &types.ExecConfig{ + User: *flUser, + Privileged: *flPrivileged, + Tty: *flTty, + Cmd: execCmd, + Container: container, + Detach: *flDetach, + } + + // If -d is not set, attach to everything by default + if !*flDetach { + execConfig.AttachStdout = true + execConfig.AttachStderr = true + if *flStdin { + execConfig.AttachStdin = true + } + } + + return execConfig, nil +} diff --git a/components/engine/runconfig/exec_test.go b/components/engine/api/client/exec_test.go similarity index 99% rename from components/engine/runconfig/exec_test.go rename to components/engine/api/client/exec_test.go index 1acae6eff6..7fd4f7aded 100644 --- a/components/engine/runconfig/exec_test.go +++ b/components/engine/api/client/exec_test.go @@ -1,4 +1,4 @@ -package runconfig +package client import ( "fmt" diff --git a/components/engine/runconfig/parse.go b/components/engine/runconfig/client/parse.go similarity index 100% rename from components/engine/runconfig/parse.go rename to components/engine/runconfig/client/parse.go diff --git a/components/engine/runconfig/parse_test.go b/components/engine/runconfig/client/parse_test.go similarity index 100% rename from components/engine/runconfig/parse_test.go rename to components/engine/runconfig/client/parse_test.go diff --git a/components/engine/runconfig/exec.go b/components/engine/runconfig/exec.go deleted file mode 100644 index 0ef8926602..0000000000 --- a/components/engine/runconfig/exec.go +++ /dev/null @@ -1,49 +0,0 @@ -package runconfig - -import ( - "github.com/docker/docker/api/types" - flag "github.com/docker/docker/pkg/mflag" -) - -// ParseExec parses the specified args for the specified command and generates -// an ExecConfig from it. -// If the minimal number of specified args is not right or if specified args are -// not valid, it will return an error. -func ParseExec(cmd *flag.FlagSet, args []string) (*types.ExecConfig, error) { - var ( - flStdin = cmd.Bool([]string{"i", "-interactive"}, false, "Keep STDIN open even if not attached") - flTty = cmd.Bool([]string{"t", "-tty"}, false, "Allocate a pseudo-TTY") - flDetach = cmd.Bool([]string{"d", "-detach"}, false, "Detached mode: run command in the background") - flUser = cmd.String([]string{"u", "-user"}, "", "Username or UID (format: [:])") - flPrivileged = cmd.Bool([]string{"-privileged"}, false, "Give extended privileges to the command") - execCmd []string - container string - ) - cmd.Require(flag.Min, 2) - if err := cmd.ParseFlags(args, true); err != nil { - return nil, err - } - container = cmd.Arg(0) - parsedArgs := cmd.Args() - execCmd = parsedArgs[1:] - - execConfig := &types.ExecConfig{ - User: *flUser, - Privileged: *flPrivileged, - Tty: *flTty, - Cmd: execCmd, - Container: container, - Detach: *flDetach, - } - - // If -d is not set, attach to everything by default - if !*flDetach { - execConfig.AttachStdout = true - execConfig.AttachStderr = true - if *flStdin { - execConfig.AttachStdin = true - } - } - - return execConfig, nil -} From c43ba74b616a329862125e740dcdaa038f4d3d31 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 21 Dec 2015 20:05:55 -0500 Subject: [PATCH 2/3] Move the runconfig.Parse() function into the runconfig/opts package. The parse.go file is used almost exclusively in the client. The few small functions that are used outside of the client could easily be copied out when the client is extracted, allowing this runconfig/opts package to move to the client. Signed-off-by: Daniel Nephin Upstream-commit: 2b7ad47bd2649c3f164e8b57b31fae313045c8f4 Component: engine --- components/engine/api/client/create.go | 4 +- components/engine/api/client/run.go | 4 +- .../engine/builder/dockerfile/dispatchers.go | 3 +- components/engine/daemon/daemon_unix.go | 3 +- components/engine/runconfig/errors.go | 32 ++++++++++++++ .../runconfig/{ => opts}/fixtures/valid.env | 0 .../runconfig/{ => opts}/fixtures/valid.label | 0 .../runconfig/{client => opts}/parse.go | 42 ++++--------------- .../runconfig/{client => opts}/parse_test.go | 7 ++-- 9 files changed, 51 insertions(+), 44 deletions(-) create mode 100644 components/engine/runconfig/errors.go rename components/engine/runconfig/{ => opts}/fixtures/valid.env (100%) rename components/engine/runconfig/{ => opts}/fixtures/valid.label (100%) rename components/engine/runconfig/{client => opts}/parse.go (87%) rename components/engine/runconfig/{client => opts}/parse_test.go (99%) diff --git a/components/engine/api/client/create.go b/components/engine/api/client/create.go index 7a4070b0b9..2569ae755a 100644 --- a/components/engine/api/client/create.go +++ b/components/engine/api/client/create.go @@ -12,7 +12,7 @@ import ( "github.com/docker/docker/pkg/jsonmessage" "github.com/docker/docker/reference" "github.com/docker/docker/registry" - "github.com/docker/docker/runconfig" + runconfigopts "github.com/docker/docker/runconfig/opts" ) func (cli *DockerCli) pullImage(image string) error { @@ -156,7 +156,7 @@ func (cli *DockerCli) CmdCreate(args ...string) error { flName = cmd.String([]string{"-name"}, "", "Assign a name to the container") ) - config, hostConfig, cmd, err := runconfig.Parse(cmd, args) + config, hostConfig, cmd, err := runconfigopts.Parse(cmd, args) if err != nil { cmd.ReportError(err.Error(), true) os.Exit(1) diff --git a/components/engine/api/client/run.go b/components/engine/api/client/run.go index ec35530a47..0a7f5265ca 100644 --- a/components/engine/api/client/run.go +++ b/components/engine/api/client/run.go @@ -14,7 +14,7 @@ import ( "github.com/docker/docker/opts" "github.com/docker/docker/pkg/promise" "github.com/docker/docker/pkg/signal" - "github.com/docker/docker/runconfig" + runconfigopts "github.com/docker/docker/runconfig/opts" "github.com/docker/libnetwork/resolvconf/dns" ) @@ -82,7 +82,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { ErrConflictDetachAutoRemove = fmt.Errorf("Conflicting options: --rm and -d") ) - config, hostConfig, cmd, err := runconfig.Parse(cmd, args) + config, hostConfig, cmd, err := runconfigopts.Parse(cmd, args) // just in case the Parse does not exit if err != nil { cmd.ReportError(err.Error(), true) diff --git a/components/engine/builder/dockerfile/dispatchers.go b/components/engine/builder/dockerfile/dispatchers.go index c036cf74fb..02d4413f1f 100644 --- a/components/engine/builder/dockerfile/dispatchers.go +++ b/components/engine/builder/dockerfile/dispatchers.go @@ -25,6 +25,7 @@ import ( "github.com/docker/docker/pkg/signal" "github.com/docker/docker/pkg/system" "github.com/docker/docker/runconfig" + runconfigopts "github.com/docker/docker/runconfig/opts" "github.com/docker/go-connections/nat" ) @@ -337,7 +338,7 @@ func run(b *Builder, args []string, attributes map[string]bool, original string) // of RUN, without leaking it to the final image. It also aids cache // lookup for same image built with same build time environment. cmdBuildEnv := []string{} - configEnv := runconfig.ConvertKVStringsToMap(b.runConfig.Env) + configEnv := runconfigopts.ConvertKVStringsToMap(b.runConfig.Env) for key, val := range b.BuildArgs { if !b.isBuildArgAllowed(key) { // skip build-args that are not in allowed list, meaning they have diff --git a/components/engine/daemon/daemon_unix.go b/components/engine/daemon/daemon_unix.go index e6d7a08080..68e04ea882 100644 --- a/components/engine/daemon/daemon_unix.go +++ b/components/engine/daemon/daemon_unix.go @@ -23,6 +23,7 @@ import ( "github.com/docker/docker/pkg/sysinfo" "github.com/docker/docker/reference" "github.com/docker/docker/runconfig" + runconfigopts "github.com/docker/docker/runconfig/opts" "github.com/docker/libnetwork" nwconfig "github.com/docker/libnetwork/config" "github.com/docker/libnetwork/drivers/bridge" @@ -681,7 +682,7 @@ func (daemon *Daemon) registerLinks(container *container.Container, hostConfig * } for _, l := range hostConfig.Links { - name, alias, err := runconfig.ParseLink(l) + name, alias, err := runconfigopts.ParseLink(l) if err != nil { return err } diff --git a/components/engine/runconfig/errors.go b/components/engine/runconfig/errors.go new file mode 100644 index 0000000000..7dbdb9e1ce --- /dev/null +++ b/components/engine/runconfig/errors.go @@ -0,0 +1,32 @@ +package runconfig + +import ( + "fmt" +) + +var ( + // ErrConflictContainerNetworkAndLinks conflict between --net=container and links + ErrConflictContainerNetworkAndLinks = fmt.Errorf("Conflicting options: container type network can't be used with links. This would result in undefined behavior") + // ErrConflictUserDefinedNetworkAndLinks conflict between --net= and links + ErrConflictUserDefinedNetworkAndLinks = fmt.Errorf("Conflicting options: networking can't be used with links. This would result in undefined behavior") + // ErrConflictSharedNetwork conflict between private and other networks + ErrConflictSharedNetwork = fmt.Errorf("Container sharing network namespace with another container or host cannot be connected to any other network") + // ErrConflictHostNetwork conflict from being disconnected from host network or connected to host network. + ErrConflictHostNetwork = fmt.Errorf("Container cannot be disconnected from host network or connected to host network") + // ErrConflictNoNetwork conflict between private and other networks + ErrConflictNoNetwork = fmt.Errorf("Container cannot be connected to multiple networks with one of the networks in private (none) mode") + // ErrConflictNetworkAndDNS conflict between --dns and the network mode + ErrConflictNetworkAndDNS = fmt.Errorf("Conflicting options: dns and the network mode") + // ErrConflictNetworkHostname conflict between the hostname and the network mode + ErrConflictNetworkHostname = fmt.Errorf("Conflicting options: hostname and the network mode") + // ErrConflictHostNetworkAndLinks conflict between --net=host and links + ErrConflictHostNetworkAndLinks = fmt.Errorf("Conflicting options: host type networking can't be used with links. This would result in undefined behavior") + // ErrConflictContainerNetworkAndMac conflict between the mac address and the network mode + ErrConflictContainerNetworkAndMac = fmt.Errorf("Conflicting options: mac-address and the network mode") + // ErrConflictNetworkHosts conflict between add-host and the network mode + ErrConflictNetworkHosts = fmt.Errorf("Conflicting options: custom host-to-IP mapping and the network mode") + // ErrConflictNetworkPublishPorts conflict between the publish options and the network mode + ErrConflictNetworkPublishPorts = fmt.Errorf("Conflicting options: port publishing and the container type network mode") + // ErrConflictNetworkExposePorts conflict between the expose option and the network mode + ErrConflictNetworkExposePorts = fmt.Errorf("Conflicting options: port exposing and the container type network mode") +) diff --git a/components/engine/runconfig/fixtures/valid.env b/components/engine/runconfig/opts/fixtures/valid.env similarity index 100% rename from components/engine/runconfig/fixtures/valid.env rename to components/engine/runconfig/opts/fixtures/valid.env diff --git a/components/engine/runconfig/fixtures/valid.label b/components/engine/runconfig/opts/fixtures/valid.label similarity index 100% rename from components/engine/runconfig/fixtures/valid.label rename to components/engine/runconfig/opts/fixtures/valid.label diff --git a/components/engine/runconfig/client/parse.go b/components/engine/runconfig/opts/parse.go similarity index 87% rename from components/engine/runconfig/client/parse.go rename to components/engine/runconfig/opts/parse.go index b2c0f35651..c391c4a8d0 100644 --- a/components/engine/runconfig/client/parse.go +++ b/components/engine/runconfig/opts/parse.go @@ -1,4 +1,4 @@ -package runconfig +package opts import ( "fmt" @@ -12,39 +12,11 @@ import ( flag "github.com/docker/docker/pkg/mflag" "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/signal" - runconfigopts "github.com/docker/docker/runconfig/opts" "github.com/docker/docker/volume" "github.com/docker/go-connections/nat" "github.com/docker/go-units" ) -var ( - // ErrConflictContainerNetworkAndLinks conflict between --net=container and links - ErrConflictContainerNetworkAndLinks = fmt.Errorf("Conflicting options: container type network can't be used with links. This would result in undefined behavior") - // ErrConflictUserDefinedNetworkAndLinks conflict between --net= and links - ErrConflictUserDefinedNetworkAndLinks = fmt.Errorf("Conflicting options: networking can't be used with links. This would result in undefined behavior") - // ErrConflictSharedNetwork conflict between private and other networks - ErrConflictSharedNetwork = fmt.Errorf("Container sharing network namespace with another container or host cannot be connected to any other network") - // ErrConflictHostNetwork conflict from being disconnected from host network or connected to host network. - ErrConflictHostNetwork = fmt.Errorf("Container cannot be disconnected from host network or connected to host network") - // ErrConflictNoNetwork conflict between private and other networks - ErrConflictNoNetwork = fmt.Errorf("Container cannot be connected to multiple networks with one of the networks in private (none) mode") - // ErrConflictNetworkAndDNS conflict between --dns and the network mode - ErrConflictNetworkAndDNS = fmt.Errorf("Conflicting options: dns and the network mode") - // ErrConflictNetworkHostname conflict between the hostname and the network mode - ErrConflictNetworkHostname = fmt.Errorf("Conflicting options: hostname and the network mode") - // ErrConflictHostNetworkAndLinks conflict between --net=host and links - ErrConflictHostNetworkAndLinks = fmt.Errorf("Conflicting options: host type networking can't be used with links. This would result in undefined behavior") - // ErrConflictContainerNetworkAndMac conflict between the mac address and the network mode - ErrConflictContainerNetworkAndMac = fmt.Errorf("Conflicting options: mac-address and the network mode") - // ErrConflictNetworkHosts conflict between add-host and the network mode - ErrConflictNetworkHosts = fmt.Errorf("Conflicting options: custom host-to-IP mapping and the network mode") - // ErrConflictNetworkPublishPorts conflict between the publish options and the network mode - ErrConflictNetworkPublishPorts = fmt.Errorf("Conflicting options: port publishing and the container type network mode") - // ErrConflictNetworkExposePorts conflict between the expose option and the network mode - ErrConflictNetworkExposePorts = fmt.Errorf("Conflicting options: port exposing and the container type network mode") -) - // Parse parses the specified args for the specified command and generates a Config, // a HostConfig and returns them with the specified command. // If the specified args are not valid, it will return an error. @@ -54,17 +26,17 @@ func Parse(cmd *flag.FlagSet, args []string) (*container.Config, *container.Host flAttach = opts.NewListOpts(opts.ValidateAttach) flVolumes = opts.NewListOpts(nil) flTmpfs = opts.NewListOpts(nil) - flBlkioWeightDevice = runconfigopts.NewWeightdeviceOpt(runconfigopts.ValidateWeightDevice) - flDeviceReadBps = runconfigopts.NewThrottledeviceOpt(runconfigopts.ValidateThrottleBpsDevice) - flDeviceWriteBps = runconfigopts.NewThrottledeviceOpt(runconfigopts.ValidateThrottleBpsDevice) + flBlkioWeightDevice = NewWeightdeviceOpt(ValidateWeightDevice) + flDeviceReadBps = NewThrottledeviceOpt(ValidateThrottleBpsDevice) + flDeviceWriteBps = NewThrottledeviceOpt(ValidateThrottleBpsDevice) flLinks = opts.NewListOpts(ValidateLink) - flDeviceReadIOps = runconfigopts.NewThrottledeviceOpt(runconfigopts.ValidateThrottleIOpsDevice) - flDeviceWriteIOps = runconfigopts.NewThrottledeviceOpt(runconfigopts.ValidateThrottleIOpsDevice) + flDeviceReadIOps = NewThrottledeviceOpt(ValidateThrottleIOpsDevice) + flDeviceWriteIOps = NewThrottledeviceOpt(ValidateThrottleIOpsDevice) flEnv = opts.NewListOpts(opts.ValidateEnv) flLabels = opts.NewListOpts(opts.ValidateEnv) flDevices = opts.NewListOpts(ValidateDevice) - flUlimits = runconfigopts.NewUlimitOpt(nil) + flUlimits = NewUlimitOpt(nil) flPublish = opts.NewListOpts(nil) flExpose = opts.NewListOpts(nil) diff --git a/components/engine/runconfig/client/parse_test.go b/components/engine/runconfig/opts/parse_test.go similarity index 99% rename from components/engine/runconfig/client/parse_test.go rename to components/engine/runconfig/opts/parse_test.go index 7c48abe603..4ae73fa4e9 100644 --- a/components/engine/runconfig/client/parse_test.go +++ b/components/engine/runconfig/opts/parse_test.go @@ -1,4 +1,4 @@ -package runconfig +package opts import ( "bytes" @@ -12,6 +12,7 @@ import ( "github.com/docker/docker/api/types/container" flag "github.com/docker/docker/pkg/mflag" + "github.com/docker/docker/runconfig" "github.com/docker/go-connections/nat" ) @@ -288,7 +289,7 @@ func callDecodeContainerConfig(volumes []string, binds []string) (*container.Con c *container.Config h *container.HostConfig ) - w := ContainerConfigWrapper{ + w := runconfig.ContainerConfigWrapper{ Config: &container.Config{ Volumes: map[string]struct{}{}, }, @@ -303,7 +304,7 @@ func callDecodeContainerConfig(volumes []string, binds []string) (*container.Con if b, err = json.Marshal(w); err != nil { return nil, nil, fmt.Errorf("Error on marshal %s", err.Error()) } - c, h, err = DecodeContainerConfig(bytes.NewReader(b)) + c, h, err = runconfig.DecodeContainerConfig(bytes.NewReader(b)) if err != nil { return nil, nil, fmt.Errorf("Error parsing %s: %v", string(b), err) } From 7bea2a5a61415948a5482fa29bcf3fd2efaec18c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 22 Dec 2015 19:52:27 -0500 Subject: [PATCH 3/3] Move volume.SplitN() to the one place it is used in runconfig. Signed-off-by: Daniel Nephin Upstream-commit: c5a2fdb697e403af228a68d08c68d17d347f6cf3 Component: engine --- components/engine/runconfig/opts/parse.go | 59 ++++++++++++++++++- .../engine/runconfig/opts/parse_test.go | 49 +++++++++++++++ components/engine/volume/volume.go | 56 ------------------ components/engine/volume/volume_test.go | 49 --------------- 4 files changed, 106 insertions(+), 107 deletions(-) diff --git a/components/engine/runconfig/opts/parse.go b/components/engine/runconfig/opts/parse.go index c391c4a8d0..bef65c8779 100644 --- a/components/engine/runconfig/opts/parse.go +++ b/components/engine/runconfig/opts/parse.go @@ -12,7 +12,6 @@ import ( flag "github.com/docker/docker/pkg/mflag" "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/signal" - "github.com/docker/docker/volume" "github.com/docker/go-connections/nat" "github.com/docker/go-units" ) @@ -199,7 +198,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*container.Config, *container.Host var binds []string // add any bind targets to the list of container volumes for bind := range flVolumes.GetMap() { - if arr := volume.SplitN(bind, 2); len(arr) > 1 { + if arr := volumeSplitN(bind, 2); len(arr) > 1 { // after creating the bind mount we want to delete it from the flVolumes values because // we do not want bind mounts being committed to image configs binds = append(binds, bind) @@ -621,3 +620,59 @@ func validatePath(val string, validator func(string) bool) (string, error) { } return val, nil } + +// SplitN splits raw into a maximum of n parts, separated by a separator colon. +// A separator colon is the last `:` character in the regex `[/:\\]?[a-zA-Z]:` (note `\\` is `\` escaped). +// This allows to correctly split strings such as `C:\foo:D:\:rw`. +func volumeSplitN(raw string, n int) []string { + var array []string + if len(raw) == 0 || raw[0] == ':' { + // invalid + return nil + } + // numberOfParts counts the number of parts separated by a separator colon + numberOfParts := 0 + // left represents the left-most cursor in raw, updated at every `:` character considered as a separator. + left := 0 + // right represents the right-most cursor in raw incremented with the loop. Note this + // starts at index 1 as index 0 is already handle above as a special case. + for right := 1; right < len(raw); right++ { + // stop parsing if reached maximum number of parts + if n >= 0 && numberOfParts >= n { + break + } + if raw[right] != ':' { + continue + } + potentialDriveLetter := raw[right-1] + if (potentialDriveLetter >= 'A' && potentialDriveLetter <= 'Z') || (potentialDriveLetter >= 'a' && potentialDriveLetter <= 'z') { + if right > 1 { + beforePotentialDriveLetter := raw[right-2] + if beforePotentialDriveLetter != ':' && beforePotentialDriveLetter != '/' && beforePotentialDriveLetter != '\\' { + // e.g. `C:` is not preceded by any delimiter, therefore it was not a drive letter but a path ending with `C:`. + array = append(array, raw[left:right]) + left = right + 1 + numberOfParts++ + } + // else, `C:` is considered as a drive letter and not as a delimiter, so we continue parsing. + } + // if right == 1, then `C:` is the beginning of the raw string, therefore `:` is again not considered a delimiter and we continue parsing. + } else { + // if `:` is not preceded by a potential drive letter, then consider it as a delimiter. + array = append(array, raw[left:right]) + left = right + 1 + numberOfParts++ + } + } + // need to take care of the last part + if left < len(raw) { + if n >= 0 && numberOfParts >= n { + // if the maximum number of parts is reached, just append the rest to the last part + // left-1 is at the last `:` that needs to be included since not considered a separator. + array[n-1] += raw[left-1:] + } else { + array = append(array, raw[left:]) + } + } + return array +} diff --git a/components/engine/runconfig/opts/parse_test.go b/components/engine/runconfig/opts/parse_test.go index 4ae73fa4e9..90703ea878 100644 --- a/components/engine/runconfig/opts/parse_test.go +++ b/components/engine/runconfig/opts/parse_test.go @@ -763,3 +763,52 @@ func TestValidateDevice(t *testing.T) { } } } + +func TestVolumeSplitN(t *testing.T) { + for _, x := range []struct { + input string + n int + expected []string + }{ + {`C:\foo:d:`, -1, []string{`C:\foo`, `d:`}}, + {`:C:\foo:d:`, -1, nil}, + {`/foo:/bar:ro`, 3, []string{`/foo`, `/bar`, `ro`}}, + {`/foo:/bar:ro`, 2, []string{`/foo`, `/bar:ro`}}, + {`C:\foo\:/foo`, -1, []string{`C:\foo\`, `/foo`}}, + + {`d:\`, -1, []string{`d:\`}}, + {`d:`, -1, []string{`d:`}}, + {`d:\path`, -1, []string{`d:\path`}}, + {`d:\path with space`, -1, []string{`d:\path with space`}}, + {`d:\pathandmode:rw`, -1, []string{`d:\pathandmode`, `rw`}}, + {`c:\:d:\`, -1, []string{`c:\`, `d:\`}}, + {`c:\windows\:d:`, -1, []string{`c:\windows\`, `d:`}}, + {`c:\windows:d:\s p a c e`, -1, []string{`c:\windows`, `d:\s p a c e`}}, + {`c:\windows:d:\s p a c e:RW`, -1, []string{`c:\windows`, `d:\s p a c e`, `RW`}}, + {`c:\program files:d:\s p a c e i n h o s t d i r`, -1, []string{`c:\program files`, `d:\s p a c e i n h o s t d i r`}}, + {`0123456789name:d:`, -1, []string{`0123456789name`, `d:`}}, + {`MiXeDcAsEnAmE:d:`, -1, []string{`MiXeDcAsEnAmE`, `d:`}}, + {`name:D:`, -1, []string{`name`, `D:`}}, + {`name:D::rW`, -1, []string{`name`, `D:`, `rW`}}, + {`name:D::RW`, -1, []string{`name`, `D:`, `RW`}}, + {`c:/:d:/forward/slashes/are/good/too`, -1, []string{`c:/`, `d:/forward/slashes/are/good/too`}}, + {`c:\Windows`, -1, []string{`c:\Windows`}}, + {`c:\Program Files (x86)`, -1, []string{`c:\Program Files (x86)`}}, + + {``, -1, nil}, + {`.`, -1, []string{`.`}}, + {`..\`, -1, []string{`..\`}}, + {`c:\:..\`, -1, []string{`c:\`, `..\`}}, + {`c:\:d:\:xyzzy`, -1, []string{`c:\`, `d:\`, `xyzzy`}}, + } { + res := volumeSplitN(x.input, x.n) + if len(res) < len(x.expected) { + t.Fatalf("input: %v, expected: %v, got: %v", x.input, x.expected, res) + } + for i, e := range res { + if e != x.expected[i] { + t.Fatalf("input: %v, expected: %v, got: %v", x.input, x.expected, res) + } + } + } +} diff --git a/components/engine/volume/volume.go b/components/engine/volume/volume.go index a0adf31ba1..edd8160fae 100644 --- a/components/engine/volume/volume.go +++ b/components/engine/volume/volume.go @@ -113,59 +113,3 @@ func ParseVolumesFrom(spec string) (string, string, error) { } return id, mode, nil } - -// SplitN splits raw into a maximum of n parts, separated by a separator colon. -// A separator colon is the last `:` character in the regex `[/:\\]?[a-zA-Z]:` (note `\\` is `\` escaped). -// This allows to correctly split strings such as `C:\foo:D:\:rw`. -func SplitN(raw string, n int) []string { - var array []string - if len(raw) == 0 || raw[0] == ':' { - // invalid - return nil - } - // numberOfParts counts the number of parts separated by a separator colon - numberOfParts := 0 - // left represents the left-most cursor in raw, updated at every `:` character considered as a separator. - left := 0 - // right represents the right-most cursor in raw incremented with the loop. Note this - // starts at index 1 as index 0 is already handle above as a special case. - for right := 1; right < len(raw); right++ { - // stop parsing if reached maximum number of parts - if n >= 0 && numberOfParts >= n { - break - } - if raw[right] != ':' { - continue - } - potentialDriveLetter := raw[right-1] - if (potentialDriveLetter >= 'A' && potentialDriveLetter <= 'Z') || (potentialDriveLetter >= 'a' && potentialDriveLetter <= 'z') { - if right > 1 { - beforePotentialDriveLetter := raw[right-2] - if beforePotentialDriveLetter != ':' && beforePotentialDriveLetter != '/' && beforePotentialDriveLetter != '\\' { - // e.g. `C:` is not preceded by any delimiter, therefore it was not a drive letter but a path ending with `C:`. - array = append(array, raw[left:right]) - left = right + 1 - numberOfParts++ - } - // else, `C:` is considered as a drive letter and not as a delimiter, so we continue parsing. - } - // if right == 1, then `C:` is the beginning of the raw string, therefore `:` is again not considered a delimiter and we continue parsing. - } else { - // if `:` is not preceded by a potential drive letter, then consider it as a delimiter. - array = append(array, raw[left:right]) - left = right + 1 - numberOfParts++ - } - } - // need to take care of the last part - if left < len(raw) { - if n >= 0 && numberOfParts >= n { - // if the maximum number of parts is reached, just append the rest to the last part - // left-1 is at the last `:` that needs to be included since not considered a separator. - array[n-1] += raw[left-1:] - } else { - array = append(array, raw[left:]) - } - } - return array -} diff --git a/components/engine/volume/volume_test.go b/components/engine/volume/volume_test.go index 2ee62e6354..d3e7acc779 100644 --- a/components/engine/volume/volume_test.go +++ b/components/engine/volume/volume_test.go @@ -133,55 +133,6 @@ func TestParseMountSpec(t *testing.T) { } } -func TestSplitN(t *testing.T) { - for _, x := range []struct { - input string - n int - expected []string - }{ - {`C:\foo:d:`, -1, []string{`C:\foo`, `d:`}}, - {`:C:\foo:d:`, -1, nil}, - {`/foo:/bar:ro`, 3, []string{`/foo`, `/bar`, `ro`}}, - {`/foo:/bar:ro`, 2, []string{`/foo`, `/bar:ro`}}, - {`C:\foo\:/foo`, -1, []string{`C:\foo\`, `/foo`}}, - - {`d:\`, -1, []string{`d:\`}}, - {`d:`, -1, []string{`d:`}}, - {`d:\path`, -1, []string{`d:\path`}}, - {`d:\path with space`, -1, []string{`d:\path with space`}}, - {`d:\pathandmode:rw`, -1, []string{`d:\pathandmode`, `rw`}}, - {`c:\:d:\`, -1, []string{`c:\`, `d:\`}}, - {`c:\windows\:d:`, -1, []string{`c:\windows\`, `d:`}}, - {`c:\windows:d:\s p a c e`, -1, []string{`c:\windows`, `d:\s p a c e`}}, - {`c:\windows:d:\s p a c e:RW`, -1, []string{`c:\windows`, `d:\s p a c e`, `RW`}}, - {`c:\program files:d:\s p a c e i n h o s t d i r`, -1, []string{`c:\program files`, `d:\s p a c e i n h o s t d i r`}}, - {`0123456789name:d:`, -1, []string{`0123456789name`, `d:`}}, - {`MiXeDcAsEnAmE:d:`, -1, []string{`MiXeDcAsEnAmE`, `d:`}}, - {`name:D:`, -1, []string{`name`, `D:`}}, - {`name:D::rW`, -1, []string{`name`, `D:`, `rW`}}, - {`name:D::RW`, -1, []string{`name`, `D:`, `RW`}}, - {`c:/:d:/forward/slashes/are/good/too`, -1, []string{`c:/`, `d:/forward/slashes/are/good/too`}}, - {`c:\Windows`, -1, []string{`c:\Windows`}}, - {`c:\Program Files (x86)`, -1, []string{`c:\Program Files (x86)`}}, - - {``, -1, nil}, - {`.`, -1, []string{`.`}}, - {`..\`, -1, []string{`..\`}}, - {`c:\:..\`, -1, []string{`c:\`, `..\`}}, - {`c:\:d:\:xyzzy`, -1, []string{`c:\`, `d:\`, `xyzzy`}}, - } { - res := SplitN(x.input, x.n) - if len(res) < len(x.expected) { - t.Fatalf("input: %v, expected: %v, got: %v", x.input, x.expected, res) - } - for i, e := range res { - if e != x.expected[i] { - t.Fatalf("input: %v, expected: %v, got: %v", x.input, x.expected, res) - } - } - } -} - // testParseMountSpec is a structure used by TestParseMountSpecSplit for // specifying test cases for the ParseMountSpec() function. type testParseMountSpec struct {