Merge pull request #19058 from dnephin/move_opts_to_runconfig_opts

Move some opts validators to runconfig/opts where they are used

api/client/build.go

@@ -58,7 +58,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")
 

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
-}

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)

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
+}

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")
+	}
+}

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)
 		}
 	}
@@ -412,7 +412,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
 		}