Move ParseLink and validators into runconfig.parse where they are used.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

daemon/daemon_unix.go

@@ -19,7 +19,6 @@ import (
 	"github.com/docker/docker/layer"
 	pblkiodev "github.com/docker/docker/pkg/blkiodev"
 	"github.com/docker/docker/pkg/idtools"
-	"github.com/docker/docker/pkg/parsers"
 	"github.com/docker/docker/pkg/parsers/kernel"
 	"github.com/docker/docker/pkg/sysinfo"
 	"github.com/docker/docker/runconfig"
@@ -648,7 +647,7 @@ func (daemon *Daemon) registerLinks(container *container.Container, hostConfig *
 	}
 
 	for _, l := range hostConfig.Links {
-		name, alias, err := parsers.ParseLink(l)
+		name, alias, err := runconfig.ParseLink(l)
 		if err != nil {
 			return err
 		}

opts/opts.go

@@ -4,13 +4,11 @@ import (
 	"fmt"
 	"net"
 	"os"
-	"path"
 	"regexp"
 	"strconv"
 	"strings"
 
 	"github.com/docker/docker/pkg/blkiodev"
-	"github.com/docker/docker/pkg/parsers"
 	"github.com/docker/go-units"
 )
 
@@ -221,82 +219,6 @@ func ValidateThrottleBpsDevice(val string) (*blkiodev.ThrottleDevice, error) {
 	}, nil
 }
 
-// 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
-	}
-	return val, nil
-}
-
-// ValidDeviceMode checks if the mode for device is valid or not.
-// Valid mode is a composition of r (read), w (write), and m (mknod).
-func ValidDeviceMode(mode string) bool {
-	var legalDeviceMode = map[rune]bool{
-		'r': true,
-		'w': true,
-		'm': true,
-	}
-	if mode == "" {
-		return false
-	}
-	for _, c := range mode {
-		if !legalDeviceMode[c] {
-			return false
-		}
-		legalDeviceMode[c] = false
-	}
-	return true
-}
-
-// ValidateDevice validates a path for devices
-// It will make sure 'val' is in the form:
-//    [host-dir:]container-path[:mode]
-// It also validates the device mode.
-func ValidateDevice(val string) (string, error) {
-	return validatePath(val, ValidDeviceMode)
-}
-
-func validatePath(val string, validator func(string) bool) (string, error) {
-	var containerPath string
-	var mode string
-
-	if strings.Count(val, ":") > 2 {
-		return val, fmt.Errorf("bad format for path: %s", val)
-	}
-
-	split := strings.SplitN(val, ":", 3)
-	if split[0] == "" {
-		return val, fmt.Errorf("bad format for path: %s", val)
-	}
-	switch len(split) {
-	case 1:
-		containerPath = split[0]
-		val = path.Clean(containerPath)
-	case 2:
-		if isValid := validator(split[1]); isValid {
-			containerPath = split[0]
-			mode = split[1]
-			val = fmt.Sprintf("%s:%s", path.Clean(containerPath), mode)
-		} else {
-			containerPath = split[1]
-			val = fmt.Sprintf("%s:%s", split[0], path.Clean(containerPath))
-		}
-	case 3:
-		containerPath = split[1]
-		mode = split[2]
-		if isValid := validator(split[2]); !isValid {
-			return val, fmt.Errorf("bad mode specified: %s", mode)
-		}
-		val = fmt.Sprintf("%s:%s:%s", split[0], containerPath, mode)
-	}
-
-	if !path.IsAbs(containerPath) {
-		return val, fmt.Errorf("%s is not an absolute path", containerPath)
-	}
-	return val, nil
-}
-
 // ValidateEnv validates an environment variable and returns it.
 // If no value is specified, it returns the current value using os.Getenv.
 //

opts/opts_test.go

@@ -244,86 +244,6 @@ func TestValidateAttach(t *testing.T) {
 	}
 }
 
-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 TestValidateDevice(t *testing.T) {
-	valid := []string{
-		"/home",
-		"/home:/home",
-		"/home:/something/else",
-		"/with space",
-		"/home:/with space",
-		"relative:/absolute-path",
-		"hostPath:/containerPath:r",
-		"/hostPath:/containerPath:rw",
-		"/hostPath:/containerPath:mrw",
-	}
-	invalid := map[string]string{
-		"":        "bad format for path: ",
-		"./":      "./ 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 path: :",
-		"/tmp:":   " is not an absolute path",
-		":test":   "bad format for path: :test",
-		":/test":  "bad format for path: :/test",
-		"tmp:":    " is not an absolute path",
-		":test:":  "bad format for path: :test:",
-		"::":      "bad format for path: ::",
-		":::":     "bad format for path: :::",
-		"/tmp:::": "bad format for path: /tmp:::",
-		":/tmp::": "bad format for path: :/tmp::",
-		"path:ro": "ro is not an absolute path",
-		"path:rr": "rr is not an absolute path",
-		"a:/b:ro": "bad mode specified: ro",
-		"a:/b:rr": "bad mode specified: rr",
-	}
-
-	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) {
 	valids := map[string]string{
 		"a":                   "a",

pkg/parsers/parsers.go

@@ -5,7 +5,6 @@ package parsers
 
 import (
 	"fmt"
-	"path"
 	"strconv"
 	"strings"
 )
@@ -68,28 +67,6 @@ func ParsePortRange(ports string) (uint64, uint64, error) {
 	return start, end, nil
 }
 
-// ParseLink parses and validates the specified string as a link format (name:alias)
-func ParseLink(val string) (string, string, error) {
-	if val == "" {
-		return "", "", fmt.Errorf("empty string specified for links")
-	}
-	arr := strings.Split(val, ":")
-	if len(arr) > 2 {
-		return "", "", fmt.Errorf("bad format for links: %s", val)
-	}
-	if len(arr) == 1 {
-		return val, val, nil
-	}
-	// This is kept because we can actually get an HostConfig with links
-	// from an already created container and the format is not `foo:bar`
-	// but `/foo:/c1/bar`
-	if strings.HasPrefix(arr[0], "/") {
-		_, alias := path.Split(arr[1])
-		return arr[0][1:], alias, nil
-	}
-	return arr[0], arr[1], nil
-}
-
 // ParseUintList parses and validates the specified string as the value
 // found in some cgroup file (e.g. `cpuset.cpus`, `cpuset.mems`), which could be
 // one of the formats below. Note that duplicates are actually allowed in the

pkg/parsers/parsers_test.go

@@ -81,38 +81,6 @@ func TestParsePortRangeIncorrectStartRange(t *testing.T) {
 	}
 }
 
-func TestParseLink(t *testing.T) {
-	name, alias, err := ParseLink("name:alias")
-	if err != nil {
-		t.Fatalf("Expected not to error out on a valid name:alias format but got: %v", err)
-	}
-	if name != "name" {
-		t.Fatalf("Link name should have been name, got %s instead", name)
-	}
-	if alias != "alias" {
-		t.Fatalf("Link alias should have been alias, got %s instead", alias)
-	}
-	// short format definition
-	name, alias, err = ParseLink("name")
-	if err != nil {
-		t.Fatalf("Expected not to error out on a valid name only format but got: %v", err)
-	}
-	if name != "name" {
-		t.Fatalf("Link name should have been name, got %s instead", name)
-	}
-	if alias != "name" {
-		t.Fatalf("Link alias should have been name, got %s instead", alias)
-	}
-	// empty string link definition is not allowed
-	if _, _, err := ParseLink(""); err == nil || !strings.Contains(err.Error(), "empty string specified for links") {
-		t.Fatalf("Expected error 'empty string specified for links' but got: %v", err)
-	}
-	// more than two colons are not allowed
-	if _, _, err := ParseLink("link:alias:wrong"); err == nil || !strings.Contains(err.Error(), "bad format for links: link:alias:wrong") {
-		t.Fatalf("Expected error 'bad format for links: link:alias:wrong' but got: %v", err)
-	}
-}
-
 func TestParseUintList(t *testing.T) {
 	valids := map[string]map[int]bool{
 		"":             {},

runconfig/parse.go

@@ -2,6 +2,7 @@ package runconfig
 
 import (
 	"fmt"
+	"path"
 	"strconv"
 	"strings"
 
@@ -55,10 +56,10 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe
 		flBlkioWeightDevice = opts.NewWeightdeviceOpt(opts.ValidateWeightDevice)
 		flDeviceReadBps     = opts.NewThrottledeviceOpt(opts.ValidateThrottleBpsDevice)
 		flDeviceWriteBps    = opts.NewThrottledeviceOpt(opts.ValidateThrottleBpsDevice)
-		flLinks             = opts.NewListOpts(opts.ValidateLink)
+		flLinks             = opts.NewListOpts(ValidateLink)
 		flEnv               = opts.NewListOpts(opts.ValidateEnv)
 		flLabels            = opts.NewListOpts(opts.ValidateEnv)
-		flDevices           = opts.NewListOpts(opts.ValidateDevice)
+		flDevices           = opts.NewListOpts(ValidateDevice)
 
 		flUlimits = opts.NewUlimitOpt(nil)
 
@@ -532,7 +533,7 @@ func ParseDevice(device string) (DeviceMapping, error) {
 		permissions = arr[2]
 		fallthrough
 	case 2:
-		if opts.ValidDeviceMode(arr[1]) {
+		if ValidDeviceMode(arr[1]) {
 			permissions = arr[1]
 		} else {
 			dst = arr[1]
@@ -555,3 +556,101 @@ func ParseDevice(device string) (DeviceMapping, error) {
 	}
 	return deviceMapping, nil
 }
+
+// ParseLink parses and validates the specified string as a link format (name:alias)
+func ParseLink(val string) (string, string, error) {
+	if val == "" {
+		return "", "", fmt.Errorf("empty string specified for links")
+	}
+	arr := strings.Split(val, ":")
+	if len(arr) > 2 {
+		return "", "", fmt.Errorf("bad format for links: %s", val)
+	}
+	if len(arr) == 1 {
+		return val, val, nil
+	}
+	// This is kept because we can actually get an HostConfig with links
+	// from an already created container and the format is not `foo:bar`
+	// but `/foo:/c1/bar`
+	if strings.HasPrefix(arr[0], "/") {
+		_, alias := path.Split(arr[1])
+		return arr[0][1:], alias, nil
+	}
+	return arr[0], arr[1], nil
+}
+
+// ValidateLink validates that the specified string has a valid link format (containerName:alias).
+func ValidateLink(val string) (string, error) {
+	if _, _, err := ParseLink(val); err != nil {
+		return val, err
+	}
+	return val, nil
+}
+
+// ValidDeviceMode checks if the mode for device is valid or not.
+// Valid mode is a composition of r (read), w (write), and m (mknod).
+func ValidDeviceMode(mode string) bool {
+	var legalDeviceMode = map[rune]bool{
+		'r': true,
+		'w': true,
+		'm': true,
+	}
+	if mode == "" {
+		return false
+	}
+	for _, c := range mode {
+		if !legalDeviceMode[c] {
+			return false
+		}
+		legalDeviceMode[c] = false
+	}
+	return true
+}
+
+// ValidateDevice validates a path for devices
+// It will make sure 'val' is in the form:
+//    [host-dir:]container-path[:mode]
+// It also validates the device mode.
+func ValidateDevice(val string) (string, error) {
+	return validatePath(val, ValidDeviceMode)
+}
+
+func validatePath(val string, validator func(string) bool) (string, error) {
+	var containerPath string
+	var mode string
+
+	if strings.Count(val, ":") > 2 {
+		return val, fmt.Errorf("bad format for path: %s", val)
+	}
+
+	split := strings.SplitN(val, ":", 3)
+	if split[0] == "" {
+		return val, fmt.Errorf("bad format for path: %s", val)
+	}
+	switch len(split) {
+	case 1:
+		containerPath = split[0]
+		val = path.Clean(containerPath)
+	case 2:
+		if isValid := validator(split[1]); isValid {
+			containerPath = split[0]
+			mode = split[1]
+			val = fmt.Sprintf("%s:%s", path.Clean(containerPath), mode)
+		} else {
+			containerPath = split[1]
+			val = fmt.Sprintf("%s:%s", split[0], path.Clean(containerPath))
+		}
+	case 3:
+		containerPath = split[1]
+		mode = split[2]
+		if isValid := validator(split[2]); !isValid {
+			return val, fmt.Errorf("bad mode specified: %s", mode)
+		}
+		val = fmt.Sprintf("%s:%s:%s", split[0], containerPath, mode)
+	}
+
+	if !path.IsAbs(containerPath) {
+		return val, fmt.Errorf("%s is not an absolute path", containerPath)
+	}
+	return val, nil
+}

runconfig/parse_test.go

@@ -649,3 +649,115 @@ func TestParseEntryPoint(t *testing.T) {
 		t.Fatalf("Expected entrypoint 'anything', got %v", config.Entrypoint)
 	}
 }
+
+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 TestParseLink(t *testing.T) {
+	name, alias, err := ParseLink("name:alias")
+	if err != nil {
+		t.Fatalf("Expected not to error out on a valid name:alias format but got: %v", err)
+	}
+	if name != "name" {
+		t.Fatalf("Link name should have been name, got %s instead", name)
+	}
+	if alias != "alias" {
+		t.Fatalf("Link alias should have been alias, got %s instead", alias)
+	}
+	// short format definition
+	name, alias, err = ParseLink("name")
+	if err != nil {
+		t.Fatalf("Expected not to error out on a valid name only format but got: %v", err)
+	}
+	if name != "name" {
+		t.Fatalf("Link name should have been name, got %s instead", name)
+	}
+	if alias != "name" {
+		t.Fatalf("Link alias should have been name, got %s instead", alias)
+	}
+	// empty string link definition is not allowed
+	if _, _, err := ParseLink(""); err == nil || !strings.Contains(err.Error(), "empty string specified for links") {
+		t.Fatalf("Expected error 'empty string specified for links' but got: %v", err)
+	}
+	// more than two colons are not allowed
+	if _, _, err := ParseLink("link:alias:wrong"); err == nil || !strings.Contains(err.Error(), "bad format for links: link:alias:wrong") {
+		t.Fatalf("Expected error 'bad format for links: link:alias:wrong' but got: %v", err)
+	}
+}
+
+func TestValidateDevice(t *testing.T) {
+	valid := []string{
+		"/home",
+		"/home:/home",
+		"/home:/something/else",
+		"/with space",
+		"/home:/with space",
+		"relative:/absolute-path",
+		"hostPath:/containerPath:r",
+		"/hostPath:/containerPath:rw",
+		"/hostPath:/containerPath:mrw",
+	}
+	invalid := map[string]string{
+		"":        "bad format for path: ",
+		"./":      "./ 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 path: :",
+		"/tmp:":   " is not an absolute path",
+		":test":   "bad format for path: :test",
+		":/test":  "bad format for path: :/test",
+		"tmp:":    " is not an absolute path",
+		":test:":  "bad format for path: :test:",
+		"::":      "bad format for path: ::",
+		":::":     "bad format for path: :::",
+		"/tmp:::": "bad format for path: /tmp:::",
+		":/tmp::": "bad format for path: :/tmp::",
+		"path:ro": "ro is not an absolute path",
+		"path:rr": "rr is not an absolute path",
+		"a:/b:ro": "bad mode specified: ro",
+		"a:/b:rr": "bad mode specified: rr",
+	}
+
+	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())
+			}
+		}
+	}
+}