From dfc6c04fa3f7dcb0e78e9dd5e8e4dd285b98546d Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Sun, 12 Jul 2015 10:33:30 +0200 Subject: [PATCH] Add test coverage to opts and refactor - Refactor opts.ValidatePath and add an opts.ValidateDevice ValidePath will now accept : containerPath:mode, hostPath:containerPath:mode and hostPath:containerPath. ValidateDevice will have the same behavior as current. - Refactor opts.ValidateEnv, opts.ParseEnvFile Environment variables will now be validated with the following definition : > Environment variables set by the user must have a name consisting > solely of alphabetics, numerics, and underscores - the first of > which must not be numeric. Signed-off-by: Vincent Demeester --- daemon/config.go | 2 +- daemon/volumes.go | 31 +--- opts/envfile.go | 28 ++-- opts/envfile_test.go | 75 ++++++---- opts/ip.go | 1 + opts/ip_test.go | 54 +++++++ opts/opts.go | 116 ++++++++++++--- opts/opts_test.go | 309 +++++++++++++++++++++++++++++++++++++--- opts/ulimit_test.go | 42 ++++++ runconfig/parse.go | 4 +- runconfig/parse_test.go | 11 +- volume/volume.go | 26 ++++ 12 files changed, 589 insertions(+), 110 deletions(-) create mode 100644 opts/ip_test.go create mode 100644 opts/ulimit_test.go diff --git a/daemon/config.go b/daemon/config.go index fbb05c3ac4..39846a1737 100644 --- a/daemon/config.go +++ b/daemon/config.go @@ -54,7 +54,7 @@ func (config *Config) InstallCommonFlags() { flag.StringVar(&config.CorsHeaders, []string{"-api-cors-header"}, "", "Set CORS headers in the remote API") // FIXME: why the inconsistency between "hosts" and "sockets"? opts.IPListVar(&config.Dns, []string{"#dns", "-dns"}, "DNS server to use") - opts.DnsSearchListVar(&config.DnsSearch, []string{"-dns-search"}, "DNS search domains to use") + opts.DNSSearchListVar(&config.DnsSearch, []string{"-dns-search"}, "DNS search domains to use") opts.LabelListVar(&config.Labels, []string{"-label"}, "Set key=value labels to the daemon") flag.StringVar(&config.LogConfig.Type, []string{"-log-driver"}, "json-file", "Default driver for container logs") opts.LogOptsVar(config.LogConfig.Config, []string{"-log-opt"}, "Set log driver options") diff --git a/daemon/volumes.go b/daemon/volumes.go index bd1fc9c3b6..383fa421da 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -73,10 +73,11 @@ func parseBindMount(spec string, mountLabel string, config *runconfig.Config) (* case 3: bind.Destination = arr[1] mode := arr[2] - if !validMountMode(mode) { + isValid, isRw := volume.ValidateMountMode(mode) + if !isValid { return nil, fmt.Errorf("invalid mode for volumes-from: %s", mode) } - bind.RW = rwModes[mode] + bind.RW = isRw // Relabel will apply a SELinux label, if necessary bind.Relabel = mode default: @@ -113,37 +114,13 @@ func parseVolumesFrom(spec string) (string, string, error) { if len(specParts) == 2 { mode = specParts[1] - if !validMountMode(mode) { + if isValid, _ := volume.ValidateMountMode(mode); !isValid { return "", "", fmt.Errorf("invalid mode for volumes-from: %s", mode) } } return id, mode, nil } -// read-write modes -var rwModes = map[string]bool{ - "rw": true, - "rw,Z": true, - "rw,z": true, - "z,rw": true, - "Z,rw": true, - "Z": true, - "z": true, -} - -// read-only modes -var roModes = map[string]bool{ - "ro": true, - "ro,Z": true, - "ro,z": true, - "z,ro": true, - "Z,ro": true, -} - -func validMountMode(mode string) bool { - return roModes[mode] || rwModes[mode] -} - func copyExistingContents(source, destination string) error { volList, err := ioutil.ReadDir(source) if err != nil { diff --git a/opts/envfile.go b/opts/envfile.go index 74d7f96f34..b854227e86 100644 --- a/opts/envfile.go +++ b/opts/envfile.go @@ -4,12 +4,18 @@ import ( "bufio" "fmt" "os" + "regexp" "strings" ) -/* -Read in a line delimited file with environment variables enumerated -*/ +var ( + // EnvironmentVariableRegexp A regexp to validate correct environment variables + // Environment variables set by the user must have a name consisting solely of + // alphabetics, numerics, and underscores - the first of which must not be numeric. + EnvironmentVariableRegexp = regexp.MustCompile("^[[:alpha:]_][[:alpha:][:digit:]_]*$") +) + +// ParseEnvFile Read in a line delimited file with environment variables enumerated func ParseEnvFile(filename string) ([]string, error) { fh, err := os.Open(filename) if err != nil { @@ -23,14 +29,15 @@ func ParseEnvFile(filename string) ([]string, error) { line := scanner.Text() // line is not empty, and not starting with '#' if len(line) > 0 && !strings.HasPrefix(line, "#") { - if strings.Contains(line, "=") { - data := strings.SplitN(line, "=", 2) + data := strings.SplitN(line, "=", 2) - // trim the front of a variable, but nothing else - variable := strings.TrimLeft(data[0], whiteSpaces) - if strings.ContainsAny(variable, whiteSpaces) { - return []string{}, ErrBadEnvVariable{fmt.Sprintf("variable '%s' has white spaces", variable)} - } + // trim the front of a variable, but nothing else + variable := strings.TrimLeft(data[0], whiteSpaces) + + if !EnvironmentVariableRegexp.MatchString(variable) { + return []string{}, ErrBadEnvVariable{fmt.Sprintf("variable '%s' is not a valid environment variable", variable)} + } + if len(data) > 1 { // pass the value through, no trimming lines = append(lines, fmt.Sprintf("%s=%s", variable, data[1])) @@ -45,6 +52,7 @@ func ParseEnvFile(filename string) ([]string, error) { var whiteSpaces = " \t" +// ErrBadEnvVariable typed error for bad environment variable type ErrBadEnvVariable struct { msg string } diff --git a/opts/envfile_test.go b/opts/envfile_test.go index 3748481d1b..cd0ca8f325 100644 --- a/opts/envfile_test.go +++ b/opts/envfile_test.go @@ -10,15 +10,15 @@ import ( "testing" ) -func tmpFileWithContent(content string) (string, error) { +func tmpFileWithContent(content string, t *testing.T) string { tmpFile, err := ioutil.TempFile("", "envfile-test") if err != nil { - return "", err + t.Fatal(err) } defer tmpFile.Close() tmpFile.WriteString(content) - return tmpFile.Name(), nil + return tmpFile.Name() } // Test ParseEnvFile for a file with a few well formatted lines @@ -27,42 +27,36 @@ func TestParseEnvFileGoodFile(t *testing.T) { baz=quux # comment -foobar=foobaz +_foobar=foobaz ` - tmpFile, err := tmpFileWithContent(content) - if err != nil { - t.Fatal("failed to create test data file") - } + tmpFile := tmpFileWithContent(content, t) defer os.Remove(tmpFile) lines, err := ParseEnvFile(tmpFile) if err != nil { - t.Fatal("ParseEnvFile failed; expected success") + t.Fatal(err) } - expected_lines := []string{ + expectedLines := []string{ "foo=bar", "baz=quux", - "foobar=foobaz", + "_foobar=foobaz", } - if !reflect.DeepEqual(lines, expected_lines) { + if !reflect.DeepEqual(lines, expectedLines) { t.Fatal("lines not equal to expected_lines") } } // Test ParseEnvFile for an empty file func TestParseEnvFileEmptyFile(t *testing.T) { - tmpFile, err := tmpFileWithContent("") - if err != nil { - t.Fatal("failed to create test data file") - } + tmpFile := tmpFileWithContent("", t) defer os.Remove(tmpFile) lines, err := ParseEnvFile(tmpFile) if err != nil { - t.Fatal("ParseEnvFile failed; expected success") + t.Fatal(err) } if len(lines) != 0 { @@ -76,6 +70,9 @@ func TestParseEnvFileNonExistentFile(t *testing.T) { if err == nil { t.Fatal("ParseEnvFile succeeded; expected failure") } + if _, ok := err.(*os.PathError); !ok { + t.Fatalf("Expected a PathError, got [%v]", err) + } } // Test ParseEnvFile for a badly formatted file @@ -84,15 +81,19 @@ func TestParseEnvFileBadlyFormattedFile(t *testing.T) { f =quux ` - tmpFile, err := tmpFileWithContent(content) - if err != nil { - t.Fatal("failed to create test data file") - } + tmpFile := tmpFileWithContent(content, t) defer os.Remove(tmpFile) - _, err = ParseEnvFile(tmpFile) + _, err := ParseEnvFile(tmpFile) if err == nil { - t.Fatal("ParseEnvFile succeeded; expected failure") + t.Fatalf("Expected a ErrBadEnvVariable, got nothing") + } + if _, ok := err.(ErrBadEnvVariable); !ok { + t.Fatalf("Expected a ErrBadEnvVariable, got [%v]", err) + } + expectedMessage := "poorly formatted environment: variable 'f ' is not a valid environment variable" + if err.Error() != expectedMessage { + t.Fatalf("Expected [%v], got [%v]", expectedMessage, err.Error()) } } @@ -101,14 +102,32 @@ func TestParseEnvFileLineTooLongFile(t *testing.T) { content := strings.Repeat("a", bufio.MaxScanTokenSize+42) content = fmt.Sprint("foo=", content) - tmpFile, err := tmpFileWithContent(content) - if err != nil { - t.Fatal("failed to create test data file") - } + tmpFile := tmpFileWithContent(content, t) defer os.Remove(tmpFile) - _, err = ParseEnvFile(tmpFile) + _, err := ParseEnvFile(tmpFile) if err == nil { t.Fatal("ParseEnvFile succeeded; expected failure") } } + +// ParseEnvFile with a random file, pass through +func TestParseEnvFileRandomFile(t *testing.T) { + content := `first line +another invalid line` + tmpFile := tmpFileWithContent(content, t) + defer os.Remove(tmpFile) + + _, err := ParseEnvFile(tmpFile) + + if err == nil { + t.Fatalf("Expected a ErrBadEnvVariable, got nothing") + } + if _, ok := err.(ErrBadEnvVariable); !ok { + t.Fatalf("Expected a ErrBadEnvvariable, got [%v]", err) + } + expectedMessage := "poorly formatted environment: variable 'first line' is not a valid environment variable" + if err.Error() != expectedMessage { + t.Fatalf("Expected [%v], got [%v]", expectedMessage, err.Error()) + } +} diff --git a/opts/ip.go b/opts/ip.go index d960dcd935..b1f9587555 100644 --- a/opts/ip.go +++ b/opts/ip.go @@ -5,6 +5,7 @@ import ( "net" ) +// IpOpt type that hold an IP type IpOpt struct { *net.IP } diff --git a/opts/ip_test.go b/opts/ip_test.go new file mode 100644 index 0000000000..b6b526a578 --- /dev/null +++ b/opts/ip_test.go @@ -0,0 +1,54 @@ +package opts + +import ( + "net" + "testing" +) + +func TestIpOptString(t *testing.T) { + addresses := []string{"", "0.0.0.0"} + var ip net.IP + + for _, address := range addresses { + stringAddress := NewIpOpt(&ip, address).String() + if stringAddress != address { + t.Fatalf("IpOpt string should be `%s`, not `%s`", address, stringAddress) + } + } +} + +func TestNewIpOptInvalidDefaultVal(t *testing.T) { + ip := net.IPv4(127, 0, 0, 1) + defaultVal := "Not an ip" + + ipOpt := NewIpOpt(&ip, defaultVal) + + expected := "127.0.0.1" + if ipOpt.String() != expected { + t.Fatalf("Expected [%v], got [%v]", expected, ipOpt.String()) + } +} + +func TestNewIpOptValidDefaultVal(t *testing.T) { + ip := net.IPv4(127, 0, 0, 1) + defaultVal := "192.168.1.1" + + ipOpt := NewIpOpt(&ip, defaultVal) + + expected := "192.168.1.1" + if ipOpt.String() != expected { + t.Fatalf("Expected [%v], got [%v]", expected, ipOpt.String()) + } +} + +func TestIpOptSetInvalidVal(t *testing.T) { + ip := net.IPv4(127, 0, 0, 1) + ipOpt := &IpOpt{IP: &ip} + + invalidIp := "invalid ip" + expectedError := "invalid ip is not an ip address" + err := ipOpt.Set(invalidIp) + if err == nil || err.Error() != expectedError { + t.Fatalf("Expected an Error with [%v], got [%v]", expectedError, err.Error()) + } +} diff --git a/opts/opts.go b/opts/opts.go index e40c1a334a..b739012791 100644 --- a/opts/opts.go +++ b/opts/opts.go @@ -11,61 +11,85 @@ import ( flag "github.com/docker/docker/pkg/mflag" "github.com/docker/docker/pkg/parsers" "github.com/docker/docker/pkg/ulimit" + "github.com/docker/docker/volume" ) var ( - alphaRegexp = regexp.MustCompile(`[a-zA-Z]`) - domainRegexp = regexp.MustCompile(`^(:?(:?[a-zA-Z0-9]|(:?[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9]))(:?\.(:?[a-zA-Z0-9]|(:?[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])))*)\.?\s*$`) - DefaultHTTPHost = "127.0.0.1" // Default HTTP Host used if only port is provided to -H flag e.g. docker -d -H tcp://:8080 + alphaRegexp = regexp.MustCompile(`[a-zA-Z]`) + domainRegexp = regexp.MustCompile(`^(:?(:?[a-zA-Z0-9]|(:?[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9]))(:?\.(:?[a-zA-Z0-9]|(:?[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])))*)\.?\s*$`) + // DefaultHTTPHost Default HTTP Host used if only port is provided to -H flag e.g. docker -d -H tcp://:8080 + DefaultHTTPHost = "127.0.0.1" + // DefaultHTTPPort Default HTTP Port used if only the protocol is provided to -H flag e.g. docker -d -H tcp:// // TODO Windows. DefaultHTTPPort is only used on Windows if a -H parameter // is not supplied. A better longer term solution would be to use a named // pipe as the default on the Windows daemon. - DefaultHTTPPort = 2375 // Default HTTP Port - DefaultUnixSocket = "/var/run/docker.sock" // Docker daemon by default always listens on the default unix socket + DefaultHTTPPort = 2375 // Default HTTP Port + // DefaultUnixSocket Path for the unix socket. + // Docker daemon by default always listens on the default unix socket + DefaultUnixSocket = "/var/run/docker.sock" ) +// ListVar Defines a flag with the specified names and usage, and put the value +// list into ListOpts that will hold the values. func ListVar(values *[]string, names []string, usage string) { flag.Var(newListOptsRef(values, nil), names, usage) } +// MapVar Defines a flag with the specified names and usage, and put the value +// map into MapOpt that will hold the values (key,value). func MapVar(values map[string]string, names []string, usage string) { flag.Var(newMapOpt(values, nil), names, usage) } +// LogOptsVar Defines a flag with the specified names and usage for --log-opts, +// and put the value map into MapOpt that will hold the values (key,value). func LogOptsVar(values map[string]string, names []string, usage string) { flag.Var(newMapOpt(values, nil), names, usage) } +// HostListVar Defines a flag with the specified names and usage and put the +// value into a ListOpts that will hold the values, validating the Host format. func HostListVar(values *[]string, names []string, usage string) { flag.Var(newListOptsRef(values, ValidateHost), names, usage) } +// IPListVar Defines a flag with the specified names and usage and put the +// value into a ListOpts that will hold the values, validating the IP format. func IPListVar(values *[]string, names []string, usage string) { flag.Var(newListOptsRef(values, ValidateIPAddress), names, usage) } -func DnsSearchListVar(values *[]string, names []string, usage string) { - flag.Var(newListOptsRef(values, ValidateDnsSearch), names, usage) +// DNSSearchListVar Defines a flag with the specified names and usage and put the +// value into a ListOpts that will hold the values, validating the DNS search format. +func DNSSearchListVar(values *[]string, names []string, usage string) { + flag.Var(newListOptsRef(values, ValidateDNSSearch), names, usage) } +// IPVar Defines a flag with the specified names and usage for IP and will use +// the specified defaultValue if the specified value is not valid. func IPVar(value *net.IP, names []string, defaultValue, usage string) { flag.Var(NewIpOpt(value, defaultValue), names, usage) } +// LabelListVar Defines a flag with the specified names and usage and put the +// value into a ListOpts that will hold the values, validating the label format. func LabelListVar(values *[]string, names []string, usage string) { flag.Var(newListOptsRef(values, ValidateLabel), names, usage) } +// UlimitMapVar Defines a flag with the specified names and usage for --ulimit, +// and put the value map into a UlimitOpt that will hold the values. func UlimitMapVar(values map[string]*ulimit.Ulimit, names []string, usage string) { flag.Var(NewUlimitOpt(values), names, usage) } -// ListOpts type +// ListOpts type that hold a list of values and a validation function. type ListOpts struct { values *[]string validator ValidatorFctType } +// NewListOpts Create a new ListOpts with the specified validator. func NewListOpts(validator ValidatorFctType) ListOpts { var values []string return *newListOptsRef(&values, validator) @@ -138,12 +162,14 @@ func (opts *ListOpts) Len() int { return len((*opts.values)) } -//MapOpts type +//MapOpts type that holds a map of values and a validation function. type MapOpts struct { values map[string]string validator ValidatorFctType } +// Set validates if needed the input value and add it to the +// internal map, by splitting on '='. func (opts *MapOpts) Set(value string) error { if opts.validator != nil { v, err := opts.validator(value) @@ -172,10 +198,13 @@ func newMapOpt(values map[string]string, validator ValidatorFctType) *MapOpts { } } -// Validators +// ValidatorFctType validator that return a validate string and/or an error type ValidatorFctType func(val string) (string, error) + +// ValidatorFctListType validator that return a validate list of string and/or an error type ValidatorFctListType func(val string) ([]string, error) +// ValidateAttach Validates that the specified string is a valid attach option. func ValidateAttach(val string) (string, error) { s := strings.ToLower(val) for _, str := range []string{"stdin", "stdout", "stderr"} { @@ -183,9 +212,10 @@ func ValidateAttach(val string) (string, error) { return s, nil } } - return val, fmt.Errorf("valid streams are STDIN, STDOUT and STDERR.") + return val, fmt.Errorf("valid streams are STDIN, STDOUT and STDERR") } +// ValidateLink Validates that the specified string has a valid link format (containerName:alias). func ValidateLink(val string) (string, error) { if _, _, err := parsers.ParseLink(val); err != nil { return val, err @@ -193,22 +223,53 @@ func ValidateLink(val string) (string, error) { return val, nil } -// ValidatePath will make sure 'val' is in the form: -// [host-dir:]container-path[:rw|ro] - but doesn't validate the mode part +// ValidateDevice Validate a path for devices +// It will make sure 'val' is in the form: +// [host-dir:]container-path[:mode] +func ValidateDevice(val string) (string, error) { + return validatePath(val, false) +} + +// ValidatePath Validate a path for volumes +// It will make sure 'val' is in the form: +// [host-dir:]container-path[:rw|ro] +// It will also validate the mount mode. func ValidatePath(val string) (string, error) { + return validatePath(val, true) +} + +func validatePath(val string, validateMountMode bool) (string, error) { var containerPath string + var mode string if strings.Count(val, ":") > 2 { return val, fmt.Errorf("bad format for volumes: %s", val) } - splited := strings.SplitN(val, ":", 2) - if len(splited) == 1 { + splited := strings.SplitN(val, ":", 3) + if splited[0] == "" { + return val, fmt.Errorf("bad format for volumes: %s", val) + } + switch len(splited) { + case 1: containerPath = splited[0] - val = path.Clean(splited[0]) - } else { + val = path.Clean(containerPath) + case 2: + if isValid, _ := volume.ValidateMountMode(splited[1]); validateMountMode && isValid { + containerPath = splited[0] + mode = splited[1] + val = fmt.Sprintf("%s:%s", path.Clean(containerPath), mode) + } else { + containerPath = splited[1] + val = fmt.Sprintf("%s:%s", splited[0], path.Clean(containerPath)) + } + case 3: containerPath = splited[1] - val = fmt.Sprintf("%s:%s", splited[0], path.Clean(splited[1])) + mode = splited[2] + if isValid, _ := volume.ValidateMountMode(splited[2]); validateMountMode && !isValid { + return val, fmt.Errorf("bad mount mode specified : %s", mode) + } + val = fmt.Sprintf("%s:%s:%s", splited[0], containerPath, mode) } if !path.IsAbs(containerPath) { @@ -217,17 +278,24 @@ func ValidatePath(val string) (string, error) { return val, nil } +// ValidateEnv Validate an environment variable and returns it +// It will use EnvironmentVariableRegexp to ensure the name of the environment variable is valid. +// If no value is specified, it returns the current value using os.Getenv. func ValidateEnv(val string) (string, error) { arr := strings.Split(val, "=") if len(arr) > 1 { return val, nil } + if !EnvironmentVariableRegexp.MatchString(arr[0]) { + return val, ErrBadEnvVariable{fmt.Sprintf("variable '%s' is not a valid environment variable", val)} + } if !doesEnvExist(val) { return val, nil } return fmt.Sprintf("%s=%s", val, os.Getenv(val)), nil } +// ValidateIPAddress Validates an Ip address func ValidateIPAddress(val string) (string, error) { var ip = net.ParseIP(strings.TrimSpace(val)) if ip != nil { @@ -236,6 +304,7 @@ func ValidateIPAddress(val string) (string, error) { return "", fmt.Errorf("%s is not an ip address", val) } +// ValidateMACAddress Validates a MAC address func ValidateMACAddress(val string) (string, error) { _, err := net.ParseMAC(strings.TrimSpace(val)) if err != nil { @@ -244,9 +313,9 @@ func ValidateMACAddress(val string) (string, error) { return val, nil } -// Validates domain for resolvconf search configuration. +// ValidateDNSSearch Validates domain for resolvconf search configuration. // A zero length domain is represented by . -func ValidateDnsSearch(val string) (string, error) { +func ValidateDNSSearch(val string) (string, error) { if val = strings.Trim(val, " "); val == "." { return val, nil } @@ -264,6 +333,8 @@ func validateDomain(val string) (string, error) { return "", fmt.Errorf("%s is not a valid domain", val) } +// ValidateExtraHost Validate that the given string is a valid extrahost and returns it +// ExtraHost are in the form of name:ip where the ip has to be a valid ip (ipv4 or ipv6) func ValidateExtraHost(val string) (string, error) { // allow for IPv6 addresses in extra hosts by only splitting on first ":" arr := strings.SplitN(val, ":", 2) @@ -276,13 +347,16 @@ func ValidateExtraHost(val string) (string, error) { return val, nil } +// ValidateLabel Validate that the given string is a valid label, and returns it +// Labels are in the form on key=value func ValidateLabel(val string) (string, error) { - if strings.Count(val, "=") != 1 { + if strings.Count(val, "=") < 1 { return "", fmt.Errorf("bad attribute format: %s", val) } return val, nil } +// ValidateHost Validate that the given string is a valid host and returns it func ValidateHost(val string) (string, error) { host, err := parsers.ParseHost(DefaultHTTPHost, DefaultUnixSocket, val) if err != nil { diff --git a/opts/opts_test.go b/opts/opts_test.go index 921009c6b3..3e639c1fa0 100644 --- a/opts/opts_test.go +++ b/opts/opts_test.go @@ -2,7 +2,7 @@ package opts import ( "fmt" - "net" + "os" "strings" "testing" ) @@ -69,7 +69,7 @@ func TestValidateMACAddress(t *testing.T) { } } -func TestListOpts(t *testing.T) { +func TestListOptsWithoutValidator(t *testing.T) { o := NewListOpts(nil) o.Set("foo") if o.String() != "[foo]" { @@ -79,6 +79,10 @@ func TestListOpts(t *testing.T) { if o.Len() != 2 { t.Errorf("%d != 2", o.Len()) } + o.Set("bar") + if o.Len() != 3 { + t.Errorf("%d != 3", o.Len()) + } if !o.Get("bar") { t.Error("o.Get(\"bar\") == false") } @@ -86,12 +90,48 @@ func TestListOpts(t *testing.T) { t.Error("o.Get(\"baz\") == true") } o.Delete("foo") - if o.String() != "[bar]" { - t.Errorf("%s != [bar]", o.String()) + if o.String() != "[bar bar]" { + t.Errorf("%s != [bar bar]", o.String()) + } + listOpts := o.GetAll() + if len(listOpts) != 2 || listOpts[0] != "bar" || listOpts[1] != "bar" { + t.Errorf("Expected [[bar bar]], got [%v]", listOpts) + } + mapListOpts := o.GetMap() + if len(mapListOpts) != 1 { + t.Errorf("Expected [map[bar:{}]], got [%v]", mapListOpts) + } + +} + +func TestListOptsWithValidator(t *testing.T) { + // Re-using logOptsvalidator (used by MapOpts) + o := NewListOpts(logOptsValidator) + o.Set("foo") + if o.String() != "[]" { + t.Errorf("%s != []", o.String()) + } + o.Set("foo=bar") + if o.String() != "[]" { + t.Errorf("%s != []", o.String()) + } + o.Set("max-file=2") + if o.Len() != 1 { + t.Errorf("%d != 1", o.Len()) + } + if !o.Get("max-file=2") { + t.Error("o.Get(\"max-file=2\") == false") + } + if o.Get("baz") { + t.Error("o.Get(\"baz\") == true") + } + o.Delete("max-file=2") + if o.String() != "[]" { + t.Errorf("%s != []", o.String()) } } -func TestValidateDnsSearch(t *testing.T) { +func TestValidateDNSSearch(t *testing.T) { valid := []string{ `.`, `a`, @@ -136,14 +176,14 @@ func TestValidateDnsSearch(t *testing.T) { } for _, domain := range valid { - if ret, err := ValidateDnsSearch(domain); err != nil || ret == "" { - t.Fatalf("ValidateDnsSearch(`"+domain+"`) got %s %s", ret, err) + if ret, err := ValidateDNSSearch(domain); err != nil || ret == "" { + t.Fatalf("ValidateDNSSearch(`"+domain+"`) got %s %s", ret, err) } } for _, domain := range invalid { - if ret, err := ValidateDnsSearch(domain); err == nil || ret != "" { - t.Fatalf("ValidateDnsSearch(`"+domain+"`) got %s %s", ret, err) + if ret, err := ValidateDNSSearch(domain); err == nil || ret != "" { + t.Fatalf("ValidateDNSSearch(`"+domain+"`) got %s %s", ret, err) } } } @@ -180,14 +220,251 @@ func TestValidateExtraHosts(t *testing.T) { } } -func TestIpOptString(t *testing.T) { - addresses := []string{"", "0.0.0.0"} - var ip net.IP +func TestValidateAttach(t *testing.T) { + valid := []string{ + "stdin", + "stdout", + "stderr", + "STDIN", + "STDOUT", + "STDERR", + } + if _, err := ValidateAttach("invalid"); err == nil { + t.Fatalf("Expected error with [valid streams are STDIN, STDOUT and STDERR], got nothing") + } - for _, address := range addresses { - stringAddress := NewIpOpt(&ip, address).String() - if stringAddress != address { - t.Fatalf("IpOpt string should be `%s`, not `%s`", address, stringAddress) + for _, attach := range valid { + value, err := ValidateAttach(attach) + if err != nil { + t.Fatal(err) + } + if value != strings.ToLower(attach) { + t.Fatalf("Expected [%v], got [%v]", attach, value) + } + } +} + +func TestValidateLink(t *testing.T) { + valid := []string{ + "name", + "dcdfbe62ecd0:alias", + "7a67485460b7642516a4ad82ecefe7f57d0c4916f530561b71a50a3f9c4e33da", + "angry_torvalds:linus", + } + invalid := map[string]string{ + "": "empty string specified for links", + "too:much:of:it": "bad format for links: too:much:of:it", + } + + for _, link := range valid { + if _, err := ValidateLink(link); err != nil { + t.Fatalf("ValidateLink(`%q`) should succeed: error %q", link, err) + } + } + + for link, expectedError := range invalid { + if _, err := ValidateLink(link); err == nil { + t.Fatalf("ValidateLink(`%q`) should have failed validation", link) + } else { + if !strings.Contains(err.Error(), expectedError) { + t.Fatalf("ValidateLink(`%q`) error should contain %q", link, expectedError) + } + } + } +} + +func TestValidatePath(t *testing.T) { + valid := []string{ + "/home", + "/home:/home", + "/home:/something/else", + "/with space", + "/home:/with space", + "relative:/absolute-path", + "hostPath:/containerPath:ro", + "/hostPath:/containerPath:rw", + "/rw:/ro", + "/path:rw", + "/path:ro", + "/rw:rw", + } + invalid := map[string]string{ + "": "bad format for volumes: ", + "./": "./ is not an absolute path", + "../": "../ is not an absolute path", + "/:../": "../ is not an absolute path", + "/:path": "path is not an absolute path", + ":": "bad format for volumes: :", + "/tmp:": " is not an absolute path", + ":test": "bad format for volumes: :test", + ":/test": "bad format for volumes: :/test", + "tmp:": " is not an absolute path", + ":test:": "bad format for volumes: :test:", + "::": "bad format for volumes: ::", + ":::": "bad format for volumes: :::", + "/tmp:::": "bad format for volumes: /tmp:::", + ":/tmp::": "bad format for volumes: :/tmp::", + "path:ro": "path is not an absolute path", + "/path:/path:sw": "bad mount mode specified : sw", + "/path:/path:rwz": "bad mount mode specified : rwz", + } + + for _, path := range valid { + if _, err := ValidatePath(path); err != nil { + t.Fatalf("ValidatePath(`%q`) should succeed: error %q", path, err) + } + } + + for path, expectedError := range invalid { + if _, err := ValidatePath(path); err == nil { + t.Fatalf("ValidatePath(`%q`) should have failed validation", path) + } else { + if err.Error() != expectedError { + t.Fatalf("ValidatePath(`%q`) error should contain %q, got %q", path, expectedError, err.Error()) + } + } + } +} +func TestValidateDevice(t *testing.T) { + valid := []string{ + "/home", + "/home:/home", + "/home:/something/else", + "/with space", + "/home:/with space", + "relative:/absolute-path", + "hostPath:/containerPath:ro", + "/hostPath:/containerPath:rw", + "/hostPath:/containerPath:mrw", + } + invalid := map[string]string{ + "": "bad format for volumes: ", + "./": "./ is not an absolute path", + "../": "../ is not an absolute path", + "/:../": "../ is not an absolute path", + "/:path": "path is not an absolute path", + ":": "bad format for volumes: :", + "/tmp:": " is not an absolute path", + ":test": "bad format for volumes: :test", + ":/test": "bad format for volumes: :/test", + "tmp:": " is not an absolute path", + ":test:": "bad format for volumes: :test:", + "::": "bad format for volumes: ::", + ":::": "bad format for volumes: :::", + "/tmp:::": "bad format for volumes: /tmp:::", + ":/tmp::": "bad format for volumes: :/tmp::", + "path:ro": "ro is not an absolute path", + } + + for _, path := range valid { + if _, err := ValidateDevice(path); err != nil { + t.Fatalf("ValidateDevice(`%q`) should succeed: error %q", path, err) + } + } + + for path, expectedError := range invalid { + if _, err := ValidateDevice(path); err == nil { + t.Fatalf("ValidateDevice(`%q`) should have failed validation", path) + } else { + if err.Error() != expectedError { + t.Fatalf("ValidateDevice(`%q`) error should contain %q, got %q", path, expectedError, err.Error()) + } + } + } +} + +func TestValidateEnv(t *testing.T) { + invalids := map[string]string{ + "some spaces": "poorly formatted environment: variable 'some spaces' is not a valid environment variable", + "asd!qwe": "poorly formatted environment: variable 'asd!qwe' is not a valid environment variable", + "1asd": "poorly formatted environment: variable '1asd' is not a valid environment variable", + "123": "poorly formatted environment: variable '123' is not a valid environment variable", + } + valids := map[string]string{ + "a": "a", + "something": "something", + "_=a": "_=a", + "env1=value1": "env1=value1", + "_env1=value1": "_env1=value1", + "env2=value2=value3": "env2=value2=value3", + "env3=abc!qwe": "env3=abc!qwe", + "env_4=value 4": "env_4=value 4", + "PATH": fmt.Sprintf("PATH=%v", os.Getenv("PATH")), + "PATH=something": "PATH=something", + } + for value, expectedError := range invalids { + _, err := ValidateEnv(value) + if err == nil { + t.Fatalf("Expected ErrBadEnvVariable, got nothing") + } + if _, ok := err.(ErrBadEnvVariable); !ok { + t.Fatalf("Expected ErrBadEnvVariable, got [%s]", err) + } + if err.Error() != expectedError { + t.Fatalf("Expected ErrBadEnvVariable with message [%s], got [%s]", expectedError, err.Error()) + } + } + for value, expected := range valids { + actual, err := ValidateEnv(value) + if err != nil { + t.Fatal(err) + } + if actual != expected { + t.Fatalf("Expected [%v], got [%v]", expected, actual) + } + } +} + +func TestValidateLabel(t *testing.T) { + if _, err := ValidateLabel("label"); err == nil || err.Error() != "bad attribute format: label" { + t.Fatalf("Expected an error [bad attribute format: label], go %v", err) + } + if actual, err := ValidateLabel("key1=value1"); err != nil || actual != "key1=value1" { + t.Fatalf("Expected [key1=value1], got [%v,%v]", actual, err) + } + // Validate it's working with more than one = + if actual, err := ValidateLabel("key1=value1=value2"); err != nil { + t.Fatalf("Expected [key1=value1=value2], got [%v,%v]", actual, err) + } + // Validate it's working with one more + if actual, err := ValidateLabel("key1=value1=value2=value3"); err != nil { + t.Fatalf("Expected [key1=value1=value2=value2], got [%v,%v]", actual, err) + } +} + +func TestValidateHost(t *testing.T) { + invalid := map[string]string{ + "anything": "Invalid bind address format: anything", + "something with spaces": "Invalid bind address format: something with spaces", + "://": "Invalid bind address format: ://", + "unknown://": "Invalid bind address format: unknown://", + "tcp://": "Invalid proto, expected tcp: ", + "tcp://:port": "Invalid bind address format: :port", + "tcp://invalid": "Invalid bind address format: invalid", + "tcp://invalid:port": "Invalid bind address format: invalid:port", + } + valid := map[string]string{ + "fd://": "fd://", + "fd://something": "fd://something", + "tcp://:2375": "tcp://127.0.0.1:2375", // default ip address + "tcp://:2376": "tcp://127.0.0.1:2376", // default ip address + "tcp://0.0.0.0:8080": "tcp://0.0.0.0:8080", + "tcp://192.168.0.0:12000": "tcp://192.168.0.0:12000", + "tcp://192.168:8080": "tcp://192.168:8080", + "tcp://0.0.0.0:1234567890": "tcp://0.0.0.0:1234567890", // yeah it's valid :P + "tcp://docker.com:2375": "tcp://docker.com:2375", + "unix://": "unix:///var/run/docker.sock", // default unix:// value + "unix://path/to/socket": "unix://path/to/socket", + } + + for value, errorMessage := range invalid { + if _, err := ValidateHost(value); err == nil || err.Error() != errorMessage { + t.Fatalf("Expected an error for %v with [%v], got [%v]", value, errorMessage, err) + } + } + for value, expected := range valid { + if actual, err := ValidateHost(value); err != nil || actual != expected { + t.Fatalf("Expected for %v [%v], got [%v, %v]", value, expected, actual, err) } } } diff --git a/opts/ulimit_test.go b/opts/ulimit_test.go new file mode 100644 index 0000000000..e72be7a813 --- /dev/null +++ b/opts/ulimit_test.go @@ -0,0 +1,42 @@ +package opts + +import ( + "testing" + + "github.com/docker/docker/pkg/ulimit" +) + +func TestUlimitOpt(t *testing.T) { + ulimitMap := map[string]*ulimit.Ulimit{ + "nofile": {"nofile", 1024, 512}, + } + + ulimitOpt := NewUlimitOpt(ulimitMap) + + expected := "[nofile=512:1024]" + if ulimitOpt.String() != expected { + t.Fatalf("Expected %v, got %v", expected, ulimitOpt) + } + + // Valid ulimit append to opts + if err := ulimitOpt.Set("core=1024:1024"); err != nil { + t.Fatal(err) + } + + // Invalid ulimit type returns an error and do not append to opts + if err := ulimitOpt.Set("notavalidtype=1024:1024"); err == nil { + t.Fatalf("Expected error on invalid ulimit type") + } + expected = "[nofile=512:1024 core=1024:1024]" + expected2 := "[core=1024:1024 nofile=512:1024]" + result := ulimitOpt.String() + if result != expected && result != expected2 { + t.Fatalf("Expected %v or %v, got %v", expected, expected2, ulimitOpt) + } + + // And test GetList + ulimits := ulimitOpt.GetList() + if len(ulimits) != 2 { + t.Fatalf("Expected a ulimit list of 2, got %v", ulimits) + } +} diff --git a/runconfig/parse.go b/runconfig/parse.go index e50018bbe6..24b1545f66 100644 --- a/runconfig/parse.go +++ b/runconfig/parse.go @@ -45,7 +45,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe flLinks = opts.NewListOpts(opts.ValidateLink) flEnv = opts.NewListOpts(opts.ValidateEnv) flLabels = opts.NewListOpts(opts.ValidateEnv) - flDevices = opts.NewListOpts(opts.ValidatePath) + flDevices = opts.NewListOpts(opts.ValidateDevice) ulimits = make(map[string]*ulimit.Ulimit) flUlimits = opts.NewUlimitOpt(ulimits) @@ -53,7 +53,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe flPublish = opts.NewListOpts(nil) flExpose = opts.NewListOpts(nil) flDns = opts.NewListOpts(opts.ValidateIPAddress) - flDnsSearch = opts.NewListOpts(opts.ValidateDnsSearch) + flDnsSearch = opts.NewListOpts(opts.ValidateDNSSearch) flExtraHosts = opts.NewListOpts(opts.ValidateExtraHost) flVolumesFrom = opts.NewListOpts(nil) flLxcOpts = opts.NewListOpts(nil) diff --git a/runconfig/parse_test.go b/runconfig/parse_test.go index d1f3c69eb0..5e91876955 100644 --- a/runconfig/parse_test.go +++ b/runconfig/parse_test.go @@ -124,8 +124,12 @@ func TestParseRunVolumes(t *testing.T) { t.Fatalf("Error parsing volume flags, `-v /hostTmp:/containerTmp:ro -v /hostVar:/containerVar:rw` should mount-bind /hostTmp into /containeTmp and /hostVar into /hostContainer. Received %v", hostConfig.Binds) } - if _, hostConfig := mustParse(t, "-v /hostTmp:/containerTmp:roZ -v /hostVar:/containerVar:rwZ"); hostConfig.Binds == nil || compareRandomizedStrings(hostConfig.Binds[0], hostConfig.Binds[1], "/hostTmp:/containerTmp:roZ", "/hostVar:/containerVar:rwZ") != nil { - t.Fatalf("Error parsing volume flags, `-v /hostTmp:/containerTmp:roZ -v /hostVar:/containerVar:rwZ` should mount-bind /hostTmp into /containeTmp and /hostVar into /hostContainer. Received %v", hostConfig.Binds) + if _, hostConfig := mustParse(t, "-v /containerTmp:ro -v /containerVar:rw"); hostConfig.Binds == nil || compareRandomizedStrings(hostConfig.Binds[0], hostConfig.Binds[1], "/containerTmp:ro", "/containerVar:rw") != nil { + t.Fatalf("Error parsing volume flags, `-v /hostTmp:/containerTmp:ro -v /hostVar:/containerVar:rw` should mount-bind /hostTmp into /containeTmp and /hostVar into /hostContainer. Received %v", hostConfig.Binds) + } + + if _, hostConfig := mustParse(t, "-v /hostTmp:/containerTmp:ro,Z -v /hostVar:/containerVar:rw,Z"); hostConfig.Binds == nil || compareRandomizedStrings(hostConfig.Binds[0], hostConfig.Binds[1], "/hostTmp:/containerTmp:ro,Z", "/hostVar:/containerVar:rw,Z") != nil { + t.Fatalf("Error parsing volume flags, `-v /hostTmp:/containerTmp:ro,Z -v /hostVar:/containerVar:rw,Z` should mount-bind /hostTmp into /containeTmp and /hostVar into /hostContainer. Received %v", hostConfig.Binds) } if _, hostConfig := mustParse(t, "-v /hostTmp:/containerTmp:Z -v /hostVar:/containerVar:z"); hostConfig.Binds == nil || compareRandomizedStrings(hostConfig.Binds[0], hostConfig.Binds[1], "/hostTmp:/containerTmp:Z", "/hostVar:/containerVar:z") != nil { @@ -157,9 +161,6 @@ func TestParseRunVolumes(t *testing.T) { if _, _, err := parse(t, "-v /tmp:"); err == nil { t.Fatalf("Error parsing volume flags, `-v /tmp:` should fail but didn't") } - if _, _, err := parse(t, "-v /tmp:ro"); err == nil { - t.Fatalf("Error parsing volume flags, `-v /tmp:ro` should fail but didn't") - } if _, _, err := parse(t, "-v /tmp::"); err == nil { t.Fatalf("Error parsing volume flags, `-v /tmp::` should fail but didn't") } diff --git a/volume/volume.go b/volume/volume.go index 6edcae3c21..e97381d88e 100644 --- a/volume/volume.go +++ b/volume/volume.go @@ -24,3 +24,29 @@ type Volume interface { // Unmount unmounts the volume when it is no longer in use. Unmount() error } + +// read-write modes +var rwModes = map[string]bool{ + "rw": true, + "rw,Z": true, + "rw,z": true, + "z,rw": true, + "Z,rw": true, + "Z": true, + "z": true, +} + +// read-only modes +var roModes = map[string]bool{ + "ro": true, + "ro,Z": true, + "ro,z": true, + "z,ro": true, + "Z,ro": true, +} + +// ValidateMountMode will make sure the mount mode is valid. +// returns if it's a valid mount mode and if it's read-write or not. +func ValidateMountMode(mode string) (bool, bool) { + return roModes[mode] || rwModes[mode], rwModes[mode] +}