From 0007f5a85935b2edcb08eb2d7e736e4db59157a9 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 29 Dec 2015 11:28:21 -0500 Subject: [PATCH] Move some validators from opts to runconfig/opts. These validators are only used by runconfig.Parse() or some other part of the client, so move them into the client-side package. Signed-off-by: Daniel Nephin --- api/client/build.go | 2 +- opts/opts.go | 62 ------------- opts/opts_test.go | 101 --------------------- {opts => runconfig/opts}/envfile.go | 0 {opts => runconfig/opts}/envfile_test.go | 0 runconfig/opts/opts.go | 70 +++++++++++++++ runconfig/opts/opts_test.go | 108 +++++++++++++++++++++++ runconfig/opts/parse.go | 12 +-- 8 files changed, 185 insertions(+), 170 deletions(-) rename {opts => runconfig/opts}/envfile.go (100%) rename {opts => runconfig/opts}/envfile_test.go (100%) create mode 100644 runconfig/opts/opts.go create mode 100644 runconfig/opts/opts_test.go diff --git a/api/client/build.go b/api/client/build.go index 38ade49ef7..e741c5e0a3 100644 --- a/api/client/build.go +++ b/api/client/build.go @@ -57,7 +57,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error { flCPUSetCpus := cmd.String([]string{"-cpuset-cpus"}, "", "CPUs in which to allow execution (0-3, 0,1)") flCPUSetMems := cmd.String([]string{"-cpuset-mems"}, "", "MEMs in which to allow execution (0-3, 0,1)") flCgroupParent := cmd.String([]string{"-cgroup-parent"}, "", "Optional parent cgroup for the container") - flBuildArg := opts.NewListOpts(opts.ValidateEnv) + flBuildArg := opts.NewListOpts(runconfigopts.ValidateEnv) cmd.Var(&flBuildArg, []string{"-build-arg"}, "Set build-time variables") isolation := cmd.String([]string{"-isolation"}, "", "Container isolation level") diff --git a/opts/opts.go b/opts/opts.go index b244f5a3a9..abc9ab8a18 100644 --- a/opts/opts.go +++ b/opts/opts.go @@ -3,7 +3,6 @@ package opts import ( "fmt" "net" - "os" "regexp" "strings" ) @@ -152,34 +151,6 @@ type ValidatorFctType func(val string) (string, error) // ValidatorFctListType defines a validator function that returns a validated 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"} { - if s == str { - return s, nil - } - } - return val, fmt.Errorf("valid streams are STDIN, STDOUT and STDERR") -} - -// ValidateEnv validates an environment variable and returns it. -// If no value is specified, it returns the current value using os.Getenv. -// -// As on ParseEnvFile and related to #16585, environment variable names -// are not validate what so ever, it's up to application inside docker -// to validate them or not. -func ValidateEnv(val string) (string, error) { - arr := strings.Split(val, "=") - if len(arr) > 1 { - return val, nil - } - 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)) @@ -189,15 +160,6 @@ 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 { - return "", err - } - return val, nil -} - // ValidateDNSSearch validates domain for resolvconf search configuration. // A zero length domain is represented by a dot (.). func ValidateDNSSearch(val string) (string, error) { @@ -218,20 +180,6 @@ func validateDomain(val string) (string, error) { return "", fmt.Errorf("%s is not a valid domain", val) } -// ValidateExtraHost validates that the specified 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) - if len(arr) != 2 || len(arr[0]) == 0 { - return "", fmt.Errorf("bad format for add-host: %q", val) - } - if _, err := ValidateIPAddress(arr[1]); err != nil { - return "", fmt.Errorf("invalid IP address in add-host: %q", arr[1]) - } - return val, nil -} - // ValidateLabel validates that the specified string is a valid label, and returns it. // Labels are in the form on key=value. func ValidateLabel(val string) (string, error) { @@ -240,13 +188,3 @@ func ValidateLabel(val string) (string, error) { } return val, nil } - -func doesEnvExist(name string) bool { - for _, entry := range os.Environ() { - parts := strings.SplitN(entry, "=", 2) - if parts[0] == name { - return true - } - } - return false -} diff --git a/opts/opts_test.go b/opts/opts_test.go index e2af1c11d0..da86b21fa3 100644 --- a/opts/opts_test.go +++ b/opts/opts_test.go @@ -2,7 +2,6 @@ package opts import ( "fmt" - "os" "strings" "testing" ) @@ -55,20 +54,6 @@ func TestMapOpts(t *testing.T) { } } -func TestValidateMACAddress(t *testing.T) { - if _, err := ValidateMACAddress(`92:d0:c6:0a:29:33`); err != nil { - t.Fatalf("ValidateMACAddress(`92:d0:c6:0a:29:33`) got %s", err) - } - - if _, err := ValidateMACAddress(`92:d0:c6:0a:33`); err == nil { - t.Fatalf("ValidateMACAddress(`92:d0:c6:0a:33`) succeeded; expected failure on invalid MAC") - } - - if _, err := ValidateMACAddress(`random invalid string`); err == nil { - t.Fatalf("ValidateMACAddress(`random invalid string`) succeeded; expected failure on invalid MAC") - } -} - func TestListOptsWithoutValidator(t *testing.T) { o := NewListOpts(nil) o.Set("foo") @@ -188,92 +173,6 @@ func TestValidateDNSSearch(t *testing.T) { } } -func TestValidateExtraHosts(t *testing.T) { - valid := []string{ - `myhost:192.168.0.1`, - `thathost:10.0.2.1`, - `anipv6host:2003:ab34:e::1`, - `ipv6local:::1`, - } - - invalid := map[string]string{ - `myhost:192.notanipaddress.1`: `invalid IP`, - `thathost-nosemicolon10.0.0.1`: `bad format`, - `anipv6host:::::1`: `invalid IP`, - `ipv6local:::0::`: `invalid IP`, - } - - for _, extrahost := range valid { - if _, err := ValidateExtraHost(extrahost); err != nil { - t.Fatalf("ValidateExtraHost(`"+extrahost+"`) should succeed: error %v", err) - } - } - - for extraHost, expectedError := range invalid { - if _, err := ValidateExtraHost(extraHost); err == nil { - t.Fatalf("ValidateExtraHost(`%q`) should have failed validation", extraHost) - } else { - if !strings.Contains(err.Error(), expectedError) { - t.Fatalf("ValidateExtraHost(`%q`) error should contain %q", extraHost, expectedError) - } - } - } -} - -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 _, 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 TestValidateEnv(t *testing.T) { - 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", - "asd!qwe": "asd!qwe", - "1asd": "1asd", - "123": "123", - "some space": "some space", - " some space before": " some space before", - "some space after ": "some space after ", - } - 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) diff --git a/opts/envfile.go b/runconfig/opts/envfile.go similarity index 100% rename from opts/envfile.go rename to runconfig/opts/envfile.go diff --git a/opts/envfile_test.go b/runconfig/opts/envfile_test.go similarity index 100% rename from opts/envfile_test.go rename to runconfig/opts/envfile_test.go diff --git a/runconfig/opts/opts.go b/runconfig/opts/opts.go new file mode 100644 index 0000000000..f919218d6f --- /dev/null +++ b/runconfig/opts/opts.go @@ -0,0 +1,70 @@ +package opts + +import ( + "fmt" + fopts "github.com/docker/docker/opts" + "net" + "os" + "strings" +) + +// 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"} { + if s == str { + return s, nil + } + } + return val, fmt.Errorf("valid streams are STDIN, STDOUT and STDERR") +} + +// ValidateEnv validates an environment variable and returns it. +// If no value is specified, it returns the current value using os.Getenv. +// +// As on ParseEnvFile and related to #16585, environment variable names +// are not validate what so ever, it's up to application inside docker +// to validate them or not. +func ValidateEnv(val string) (string, error) { + arr := strings.Split(val, "=") + if len(arr) > 1 { + return val, nil + } + if !doesEnvExist(val) { + return val, nil + } + return fmt.Sprintf("%s=%s", val, os.Getenv(val)), nil +} + +func doesEnvExist(name string) bool { + for _, entry := range os.Environ() { + parts := strings.SplitN(entry, "=", 2) + if parts[0] == name { + return true + } + } + return false +} + +// ValidateExtraHost validates that the specified 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) + if len(arr) != 2 || len(arr[0]) == 0 { + return "", fmt.Errorf("bad format for add-host: %q", val) + } + if _, err := fopts.ValidateIPAddress(arr[1]); err != nil { + return "", fmt.Errorf("invalid IP address in add-host: %q", arr[1]) + } + return val, nil +} + +// ValidateMACAddress validates a MAC address. +func ValidateMACAddress(val string) (string, error) { + _, err := net.ParseMAC(strings.TrimSpace(val)) + if err != nil { + return "", err + } + return val, nil +} diff --git a/runconfig/opts/opts_test.go b/runconfig/opts/opts_test.go new file mode 100644 index 0000000000..69eac88adc --- /dev/null +++ b/runconfig/opts/opts_test.go @@ -0,0 +1,108 @@ +package opts + +import ( + "fmt" + "os" + "strings" + "testing" +) + +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 _, 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 TestValidateEnv(t *testing.T) { + 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", + "asd!qwe": "asd!qwe", + "1asd": "1asd", + "123": "123", + "some space": "some space", + " some space before": " some space before", + "some space after ": "some space after ", + } + 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 TestValidateExtraHosts(t *testing.T) { + valid := []string{ + `myhost:192.168.0.1`, + `thathost:10.0.2.1`, + `anipv6host:2003:ab34:e::1`, + `ipv6local:::1`, + } + + invalid := map[string]string{ + `myhost:192.notanipaddress.1`: `invalid IP`, + `thathost-nosemicolon10.0.0.1`: `bad format`, + `anipv6host:::::1`: `invalid IP`, + `ipv6local:::0::`: `invalid IP`, + } + + for _, extrahost := range valid { + if _, err := ValidateExtraHost(extrahost); err != nil { + t.Fatalf("ValidateExtraHost(`"+extrahost+"`) should succeed: error %v", err) + } + } + + for extraHost, expectedError := range invalid { + if _, err := ValidateExtraHost(extraHost); err == nil { + t.Fatalf("ValidateExtraHost(`%q`) should have failed validation", extraHost) + } else { + if !strings.Contains(err.Error(), expectedError) { + t.Fatalf("ValidateExtraHost(`%q`) error should contain %q", extraHost, expectedError) + } + } + } +} + +func TestValidateMACAddress(t *testing.T) { + if _, err := ValidateMACAddress(`92:d0:c6:0a:29:33`); err != nil { + t.Fatalf("ValidateMACAddress(`92:d0:c6:0a:29:33`) got %s", err) + } + + if _, err := ValidateMACAddress(`92:d0:c6:0a:33`); err == nil { + t.Fatalf("ValidateMACAddress(`92:d0:c6:0a:33`) succeeded; expected failure on invalid MAC") + } + + if _, err := ValidateMACAddress(`random invalid string`); err == nil { + t.Fatalf("ValidateMACAddress(`random invalid string`) succeeded; expected failure on invalid MAC") + } +} diff --git a/runconfig/opts/parse.go b/runconfig/opts/parse.go index bef65c8779..200ae5c47d 100644 --- a/runconfig/opts/parse.go +++ b/runconfig/opts/parse.go @@ -22,7 +22,7 @@ import ( func Parse(cmd *flag.FlagSet, args []string) (*container.Config, *container.HostConfig, *flag.FlagSet, error) { var ( // FIXME: use utils.ListOpts for attach and volumes? - flAttach = opts.NewListOpts(opts.ValidateAttach) + flAttach = opts.NewListOpts(ValidateAttach) flVolumes = opts.NewListOpts(nil) flTmpfs = opts.NewListOpts(nil) flBlkioWeightDevice = NewWeightdeviceOpt(ValidateWeightDevice) @@ -31,8 +31,8 @@ func Parse(cmd *flag.FlagSet, args []string) (*container.Config, *container.Host flLinks = opts.NewListOpts(ValidateLink) flDeviceReadIOps = NewThrottledeviceOpt(ValidateThrottleIOpsDevice) flDeviceWriteIOps = NewThrottledeviceOpt(ValidateThrottleIOpsDevice) - flEnv = opts.NewListOpts(opts.ValidateEnv) - flLabels = opts.NewListOpts(opts.ValidateEnv) + flEnv = opts.NewListOpts(ValidateEnv) + flLabels = opts.NewListOpts(ValidateEnv) flDevices = opts.NewListOpts(ValidateDevice) flUlimits = NewUlimitOpt(nil) @@ -42,7 +42,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*container.Config, *container.Host flDNS = opts.NewListOpts(opts.ValidateIPAddress) flDNSSearch = opts.NewListOpts(opts.ValidateDNSSearch) flDNSOptions = opts.NewListOpts(nil) - flExtraHosts = opts.NewListOpts(opts.ValidateExtraHost) + flExtraHosts = opts.NewListOpts(ValidateExtraHost) flVolumesFrom = opts.NewListOpts(nil) flEnvFile = opts.NewListOpts(nil) flCapAdd = opts.NewListOpts(nil) @@ -130,7 +130,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*container.Config, *container.Host // Validate the input mac address if *flMacAddress != "" { - if _, err := opts.ValidateMACAddress(*flMacAddress); err != nil { + if _, err := ValidateMACAddress(*flMacAddress); err != nil { return nil, nil, cmd, fmt.Errorf("%s is not a valid mac address", *flMacAddress) } } @@ -413,7 +413,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*container.Config, *container.Host func readKVStrings(files []string, override []string) ([]string, error) { envVariables := []string{} for _, ef := range files { - parsedVars, err := opts.ParseEnvFile(ef) + parsedVars, err := ParseEnvFile(ef) if err != nil { return nil, err }