toolshed/docker-cli
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Use compose volume spec parser for container volume flag

Browse Source 
Restore testcases for Volume spec parsing.
And correctly interpret the parsed volume.

Signed-off-by: Daniel Nephin <dnephin@docker.com>
This commit is contained in:
Daniel Nephin
2017-05-09 19:21:17 -04:00
parent db6ff357a7
commit 732261f774
5 changed files with 92 additions and 156 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
cli/command/container/opts.go
View File

@ -12,6 +12,7 @@ import (
"time"
"github.com/Sirupsen/logrus"
"github.com/docker/cli/cli/compose/loader"
"github.com/docker/cli/opts"
"github.com/docker/docker/api/types/container"
networktypes "github.com/docker/docker/api/types/network"
@ -332,7 +333,8 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*containerConfig, err
volumes := copts.volumes.GetMap()
// add any bind targets to the list of container volumes
for bind := range copts.volumes.GetMap() {
if arr := volumeSplitN(bind, 2); len(arr) > 1 {
parsed, _ := loader.ParseVolume(bind)
if parsed.Source != "" {
// after creating the bind mount we want to delete it from the copts.volumes values because
// we do not want bind mounts being committed to image configs
binds = append(binds, bind)
@ -827,67 +829,6 @@ func validatePath(val string, validator func(string) bool) (string, error) {
return val, nil
}
// volumeSplitN 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).
// In Windows driver letter appears in two situations:
// a. `^[a-zA-Z]:` (A colon followed by `^[a-zA-Z]:` is OK as colon is the separator in volume option)
// b. A string in the format like `\\?\C:\Windows\...` (UNC).
// Therefore, a driver letter can only follow either a `:` or `\\`
// This allows to correctly split strings such as `C:\foo:D:\:rw` or `/tmp/q:/foo`.
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]
// Only `:` or `\\` are checked (`/` could fall into the case of `/tmp/q:/foo`)
if 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
}
// validateAttach validates that the specified string is a valid attach option.
func validateAttach(val string) (string, error) {
s := strings.ToLower(val)

96
cli/command/container/opts_test.go
View File

@ -19,6 +19,7 @@ import (
"github.com/pkg/errors"
"github.com/spf13/pflag"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestValidateAttach(t *testing.T) {
@ -366,51 +367,46 @@ func TestParseDevice(t *testing.T) {
func TestParseModes(t *testing.T) {
// ipc ko
if _, _, _, err := parseRun([]string{"--ipc=container:", "img", "cmd"}); err == nil || err.Error() != "--ipc: invalid IPC mode" {
t.Fatalf("Expected an error with message '--ipc: invalid IPC mode', got %v", err)
}
_, _, _, err := parseRun([]string{"--ipc=container:", "img", "cmd"})
testutil.ErrorContains(t, err, "--ipc: invalid IPC mode")
// ipc ok
_, hostconfig, _, err := parseRun([]string{"--ipc=host", "img", "cmd"})
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
if !hostconfig.IpcMode.Valid() {
t.Fatalf("Expected a valid IpcMode, got %v", hostconfig.IpcMode)
}
// pid ko
if _, _, _, err := parseRun([]string{"--pid=container:", "img", "cmd"}); err == nil || err.Error() != "--pid: invalid PID mode" {
t.Fatalf("Expected an error with message '--pid: invalid PID mode', got %v", err)
}
_, _, _, err = parseRun([]string{"--pid=container:", "img", "cmd"})
testutil.ErrorContains(t, err, "--pid: invalid PID mode")
// pid ok
_, hostconfig, _, err = parseRun([]string{"--pid=host", "img", "cmd"})
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
if !hostconfig.PidMode.Valid() {
t.Fatalf("Expected a valid PidMode, got %v", hostconfig.PidMode)
}
// uts ko
if _, _, _, err := parseRun([]string{"--uts=container:", "img", "cmd"}); err == nil || err.Error() != "--uts: invalid UTS mode" {
t.Fatalf("Expected an error with message '--uts: invalid UTS mode', got %v", err)
}
_, _, _, err = parseRun([]string{"--uts=container:", "img", "cmd"})
testutil.ErrorContains(t, err, "--uts: invalid UTS mode")
// uts ok
_, hostconfig, _, err = parseRun([]string{"--uts=host", "img", "cmd"})
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
if !hostconfig.UTSMode.Valid() {
t.Fatalf("Expected a valid UTSMode, got %v", hostconfig.UTSMode)
}
// shm-size ko
expectedErr := `invalid argument "a128m" for --shm-size=a128m: invalid size: 'a128m'`
if _, _, _, err = parseRun([]string{"--shm-size=a128m", "img", "cmd"}); err == nil || err.Error() != expectedErr {
t.Fatalf("Expected an error with message '%v', got %v", expectedErr, err)
}
_, _, _, err = parseRun([]string{"--shm-size=a128m", "img", "cmd"})
testutil.ErrorContains(t, err, expectedErr)
// shm-size ok
_, hostconfig, _, err = parseRun([]string{"--shm-size=128m", "img", "cmd"})
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
if hostconfig.ShmSize != 134217728 {
t.Fatalf("Expected a valid ShmSize, got %d", hostconfig.ShmSize)
}
@ -754,58 +750,6 @@ func callDecodeContainerConfig(volumes []string, binds []string) (*container.Con
return c, h, err
}
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`}},
// Cover directories with one-character name
{`/tmp/x/y:/foo/x/y`, -1, []string{`/tmp/x/y`, `/foo/x/y`}},
} {
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)
}
}
}
}
func TestValidateDevice(t *testing.T) {
valid := []string{
"/home",