From f6ed590596cbf73764b40ff4f32f90b1cdb8b213 Mon Sep 17 00:00:00 2001 From: John Howard Date: Thu, 9 Jul 2015 15:12:36 -0700 Subject: [PATCH] Move netmode validation to server Signed-off-by: John Howard --- Dockerfile | 2 +- daemon/container.go | 4 + daemon/container_unix.go | 5 - integration-cli/docker_cli_netmode_test.go | 162 +++++++++++++++++++++ runconfig/config.go | 48 ++---- runconfig/config_unix.go | 47 ++++++ runconfig/config_windows.go | 13 ++ runconfig/hostconfig.go | 26 ++-- runconfig/hostconfig_test.go | 3 + runconfig/hostconfig_unix.go | 10 ++ runconfig/hostconfig_windows.go | 9 ++ runconfig/parse.go | 37 +---- runconfig/parse_test.go | 71 --------- runconfig/parse_unix.go | 34 +++-- runconfig/parse_windows.go | 16 +- 15 files changed, 300 insertions(+), 187 deletions(-) create mode 100644 integration-cli/docker_cli_netmode_test.go create mode 100644 runconfig/config_unix.go create mode 100644 runconfig/config_windows.go diff --git a/Dockerfile b/Dockerfile index e47908c421..0e58df1065 100644 --- a/Dockerfile +++ b/Dockerfile @@ -148,7 +148,7 @@ RUN set -x \ && rm -rf "$GOPATH" # Get the "docker-py" source so we can run their integration tests -ENV DOCKER_PY_COMMIT 8a87001d09852058f08a807ab6e8491d57ca1e88 +ENV DOCKER_PY_COMMIT 139850f3f3b17357bab5ba3edfb745fb14043764 RUN git clone https://github.com/docker/docker-py.git /docker-py \ && cd /docker-py \ && git checkout -q $DOCKER_PY_COMMIT diff --git a/daemon/container.go b/daemon/container.go index becf5f3d27..f56b8cbe99 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -264,6 +264,10 @@ func (container *Container) Start() (err error) { return err } + // Make sure NetworkMode has an acceptable value. We do this to ensure + // backwards API compatibility. + container.hostConfig = runconfig.SetDefaultNetModeIfBlank(container.hostConfig) + if err := container.initializeNetworking(); err != nil { return err } diff --git a/daemon/container_unix.go b/daemon/container_unix.go index b84f0e7287..b52f14c5de 100644 --- a/daemon/container_unix.go +++ b/daemon/container_unix.go @@ -913,11 +913,6 @@ func (container *Container) configureNetwork(networkName, service, networkDriver func (container *Container) initializeNetworking() error { var err error - // Make sure NetworkMode has an acceptable value before - // initializing networking. - if container.hostConfig.NetworkMode == runconfig.NetworkMode("") { - container.hostConfig.NetworkMode = runconfig.NetworkMode("default") - } if container.hostConfig.NetworkMode.IsContainer() { // we need to get the hosts files from the container to join nc, err := container.getNetworkedContainer() diff --git a/integration-cli/docker_cli_netmode_test.go b/integration-cli/docker_cli_netmode_test.go new file mode 100644 index 0000000000..12520941a6 --- /dev/null +++ b/integration-cli/docker_cli_netmode_test.go @@ -0,0 +1,162 @@ +package main + +import ( + "os/exec" + "strings" + + "github.com/docker/docker/runconfig" + "github.com/go-check/check" +) + +// GH14530. Validates combinations of --net= with other options + +// stringCheckPS is how the output of PS starts in order to validate that +// the command executed in a container did really run PS correctly. +const stringCheckPS = "PID USER" + +// checkContains is a helper function that validates a command output did +// contain what was expected. +func checkContains(expected string, out string, c *check.C) { + if !strings.Contains(out, expected) { + c.Fatalf("Expected '%s', got '%s'", expected, out) + } +} + +func (s *DockerSuite) TestNetHostname(c *check.C) { + + var ( + out string + err error + runCmd *exec.Cmd + ) + + runCmd = exec.Command(dockerBinary, "run", "-h=name", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err != nil { + c.Fatalf(out, err) + } + checkContains(stringCheckPS, out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=host", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err != nil { + c.Fatalf(out, err) + } + checkContains(stringCheckPS, out, c) + + runCmd = exec.Command(dockerBinary, "run", "-h=name", "--net=bridge", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err != nil { + c.Fatalf(out, err) + } + checkContains(stringCheckPS, out, c) + + runCmd = exec.Command(dockerBinary, "run", "-h=name", "--net=none", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err != nil { + c.Fatalf(out, err) + } + checkContains(stringCheckPS, out, c) + + runCmd = exec.Command(dockerBinary, "run", "-h=name", "--net=host", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictNetworkHostname.Error(), out, c) + + runCmd = exec.Command(dockerBinary, "run", "-h=name", "--net=container:other", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err, c) + } + checkContains(runconfig.ErrConflictNetworkHostname.Error(), out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=container", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err, c) + } + checkContains("--net: invalid net mode: invalid container format container:", out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=weird", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains("invalid --net: weird", out, c) +} + +func (s *DockerSuite) TestConflictContainerNetworkAndLinks(c *check.C) { + var ( + out string + err error + runCmd *exec.Cmd + ) + + runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "--link=zip:zap", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictContainerNetworkAndLinks.Error(), out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=host", "--link=zip:zap", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictHostNetworkAndLinks.Error(), out, c) +} + +func (s *DockerSuite) TestConflictNetworkModeAndOptions(c *check.C) { + var ( + out string + err error + runCmd *exec.Cmd + ) + + runCmd = exec.Command(dockerBinary, "run", "--net=host", "--dns=8.8.8.8", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictNetworkAndDNS.Error(), out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "--dns=8.8.8.8", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictNetworkAndDNS.Error(), out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=host", "--add-host=name:8.8.8.8", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictNetworkHosts.Error(), out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "--add-host=name:8.8.8.8", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictNetworkHosts.Error(), out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=host", "--mac-address=92:d0:c6:0a:29:33", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictContainerNetworkAndMac.Error(), out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "--mac-address=92:d0:c6:0a:29:33", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictContainerNetworkAndMac.Error(), out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "-P", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictNetworkPublishPorts.Error(), out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "-p", "8080", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictNetworkPublishPorts.Error(), out, c) + + runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "--expose", "8000-9000", "busybox", "ps") + if out, _, err = runCommandWithOutput(runCmd); err == nil { + c.Fatalf(out, err) + } + checkContains(runconfig.ErrConflictNetworkExposePorts.Error(), out, c) +} diff --git a/runconfig/config.go b/runconfig/config.go index 0312cc4c88..9984f50a7b 100644 --- a/runconfig/config.go +++ b/runconfig/config.go @@ -158,44 +158,6 @@ type Config struct { Labels map[string]string // List of labels set to this container } -// ContainerConfigWrapper is a Config wrapper that hold the container Config (portable) -// and the corresponding HostConfig (non-portable). -type ContainerConfigWrapper struct { - *Config - InnerHostConfig *HostConfig `json:"HostConfig,omitempty"` - Cpuset string `json:",omitempty"` // Deprecated. Exported for backwards compatibility. - *HostConfig // Deprecated. Exported to read attrubutes from json that are not in the inner host config structure. - -} - -// GetHostConfig gets the HostConfig of the Config. -// It's mostly there to handle Deprecated fields of the ContainerConfigWrapper -func (w *ContainerConfigWrapper) GetHostConfig() *HostConfig { - hc := w.HostConfig - - if hc == nil && w.InnerHostConfig != nil { - hc = w.InnerHostConfig - } else if w.InnerHostConfig != nil { - if hc.Memory != 0 && w.InnerHostConfig.Memory == 0 { - w.InnerHostConfig.Memory = hc.Memory - } - if hc.MemorySwap != 0 && w.InnerHostConfig.MemorySwap == 0 { - w.InnerHostConfig.MemorySwap = hc.MemorySwap - } - if hc.CPUShares != 0 && w.InnerHostConfig.CPUShares == 0 { - w.InnerHostConfig.CPUShares = hc.CPUShares - } - - hc = w.InnerHostConfig - } - - if hc != nil && w.Cpuset != "" && hc.CpusetCpus == "" { - hc.CpusetCpus = w.Cpuset - } - - return hc -} - // DecodeContainerConfig decodes a json encoded config into a ContainerConfigWrapper // struct and returns both a Config and an HostConfig struct // Be aware this function is not checking whether the resulted structs are nil, @@ -208,5 +170,13 @@ func DecodeContainerConfig(src io.Reader) (*Config, *HostConfig, error) { return nil, nil, err } - return w.Config, w.GetHostConfig(), nil + hc := w.getHostConfig() + + // Certain parameters need daemon-side validation that cannot be done + // on the client, as only the daemon knows what is valid for the platform. + if err := ValidateNetMode(w.Config, hc); err != nil { + return nil, nil, err + } + + return w.Config, hc, nil } diff --git a/runconfig/config_unix.go b/runconfig/config_unix.go new file mode 100644 index 0000000000..8d655eedda --- /dev/null +++ b/runconfig/config_unix.go @@ -0,0 +1,47 @@ +// +build !windows + +package runconfig + +// ContainerConfigWrapper is a Config wrapper that hold the container Config (portable) +// and the corresponding HostConfig (non-portable). +type ContainerConfigWrapper struct { + *Config + InnerHostConfig *HostConfig `json:"HostConfig,omitempty"` + Cpuset string `json:",omitempty"` // Deprecated. Exported for backwards compatibility. + *HostConfig // Deprecated. Exported to read attributes from json that are not in the inner host config structure. +} + +// getHostConfig gets the HostConfig of the Config. +// It's mostly there to handle Deprecated fields of the ContainerConfigWrapper +func (w *ContainerConfigWrapper) getHostConfig() *HostConfig { + hc := w.HostConfig + + if hc == nil && w.InnerHostConfig != nil { + hc = w.InnerHostConfig + } else if w.InnerHostConfig != nil { + if hc.Memory != 0 && w.InnerHostConfig.Memory == 0 { + w.InnerHostConfig.Memory = hc.Memory + } + if hc.MemorySwap != 0 && w.InnerHostConfig.MemorySwap == 0 { + w.InnerHostConfig.MemorySwap = hc.MemorySwap + } + if hc.CPUShares != 0 && w.InnerHostConfig.CPUShares == 0 { + w.InnerHostConfig.CPUShares = hc.CPUShares + } + if hc.CpusetCpus != "" && w.InnerHostConfig.CpusetCpus == "" { + w.InnerHostConfig.CpusetCpus = hc.CpusetCpus + } + + hc = w.InnerHostConfig + } + + if hc != nil && w.Cpuset != "" && hc.CpusetCpus == "" { + hc.CpusetCpus = w.Cpuset + } + + // Make sure NetworkMode has an acceptable value. We do this to ensure + // backwards compatible API behaviour. + hc = SetDefaultNetModeIfBlank(hc) + + return hc +} diff --git a/runconfig/config_windows.go b/runconfig/config_windows.go new file mode 100644 index 0000000000..2ab8e19a27 --- /dev/null +++ b/runconfig/config_windows.go @@ -0,0 +1,13 @@ +package runconfig + +// ContainerConfigWrapper is a Config wrapper that hold the container Config (portable) +// and the corresponding HostConfig (non-portable). +type ContainerConfigWrapper struct { + *Config + HostConfig *HostConfig `json:"HostConfig,omitempty"` +} + +// getHostConfig gets the HostConfig of the Config. +func (w *ContainerConfigWrapper) getHostConfig() *HostConfig { + return w.HostConfig +} diff --git a/runconfig/hostconfig.go b/runconfig/hostconfig.go index f6a89a314a..af5f3ecea2 100644 --- a/runconfig/hostconfig.go +++ b/runconfig/hostconfig.go @@ -298,16 +298,6 @@ type HostConfig struct { ConsoleSize [2]int // Initial console size on Windows } -// MergeConfigs merges the specified container Config and HostConfig. -// It creates a ContainerConfigWrapper. -func MergeConfigs(config *Config, hostConfig *HostConfig) *ContainerConfigWrapper { - return &ContainerConfigWrapper{ - config, - hostConfig, - "", nil, - } -} - // DecodeHostConfig creates a HostConfig based on the specified Reader. // It assumes the content of the reader will be JSON, and decodes it. func DecodeHostConfig(src io.Reader) (*HostConfig, error) { @@ -318,7 +308,19 @@ func DecodeHostConfig(src io.Reader) (*HostConfig, error) { return nil, err } - hc := w.GetHostConfig() - + hc := w.getHostConfig() return hc, nil } + +// SetDefaultNetModeIfBlank changes the NetworkMode in a HostConfig structure +// to default if it is not populated. This ensures backwards compatibility after +// the validation of the network mode was moved from the docker CLI to the +// docker daemon. +func SetDefaultNetModeIfBlank(hc *HostConfig) *HostConfig { + if hc != nil { + if hc.NetworkMode == NetworkMode("") { + hc.NetworkMode = NetworkMode("default") + } + } + return hc +} diff --git a/runconfig/hostconfig_test.go b/runconfig/hostconfig_test.go index 7c0befc7e7..2c5b06505a 100644 --- a/runconfig/hostconfig_test.go +++ b/runconfig/hostconfig_test.go @@ -1,3 +1,5 @@ +// +build !windows + package runconfig import ( @@ -8,6 +10,7 @@ import ( "testing" ) +// TODO Windows: This will need addressing for a Windows daemon. func TestNetworkModeTest(t *testing.T) { networkModes := map[NetworkMode][]bool{ // private, bridge, host, container, none, default diff --git a/runconfig/hostconfig_unix.go b/runconfig/hostconfig_unix.go index 5239cb7d90..0ba9e32973 100644 --- a/runconfig/hostconfig_unix.go +++ b/runconfig/hostconfig_unix.go @@ -58,3 +58,13 @@ func (n NetworkMode) IsContainer() bool { func (n NetworkMode) IsNone() bool { return n == "none" } + +// MergeConfigs merges the specified container Config and HostConfig. +// It creates a ContainerConfigWrapper. +func MergeConfigs(config *Config, hostConfig *HostConfig) *ContainerConfigWrapper { + return &ContainerConfigWrapper{ + config, + hostConfig, + "", nil, + } +} diff --git a/runconfig/hostconfig_windows.go b/runconfig/hostconfig_windows.go index a4c0297bb3..19c90bf72a 100644 --- a/runconfig/hostconfig_windows.go +++ b/runconfig/hostconfig_windows.go @@ -18,3 +18,12 @@ func (n NetworkMode) NetworkName() string { } return "" } + +// MergeConfigs merges the specified container Config and HostConfig. +// It creates a ContainerConfigWrapper. +func MergeConfigs(config *Config, hostConfig *HostConfig) *ContainerConfigWrapper { + return &ContainerConfigWrapper{ + config, + hostConfig, + } +} diff --git a/runconfig/parse.go b/runconfig/parse.go index 6488817cfd..e9ab6c69dd 100644 --- a/runconfig/parse.go +++ b/runconfig/parse.go @@ -31,20 +31,6 @@ var ( ErrConflictNetworkExposePorts = fmt.Errorf("Conflicting options: --expose and the network mode (--expose)") ) -// validateNM is the set of fields passed to validateNetMode() -type validateNM struct { - netMode NetworkMode - flHostname *string - flLinks opts.ListOpts - flDNS opts.ListOpts - flExtraHosts opts.ListOpts - flMacAddress *string - flPublish opts.ListOpts - flPublishAll *bool - flExpose opts.ListOpts - flVolumeDriver string -} - // Parse parses the specified args for the specified command and generates a Config, // a HostConfig and returns them with the specified command. // If the specified args are not valid, it will return an error. @@ -143,27 +129,6 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe attachStderr = flAttach.Get("stderr") ) - netMode, err := parseNetMode(*flNetMode) - if err != nil { - return nil, nil, cmd, fmt.Errorf("--net: invalid net mode: %v", err) - } - - vals := validateNM{ - netMode: netMode, - flHostname: flHostname, - flLinks: flLinks, - flDNS: flDNS, - flExtraHosts: flExtraHosts, - flMacAddress: flMacAddress, - flPublish: flPublish, - flPublishAll: flPublishAll, - flExpose: flExpose, - } - - if err := validateNetMode(&vals); err != nil { - return nil, nil, cmd, err - } - // Validate the input mac address if *flMacAddress != "" { if _, err := opts.ValidateMACAddress(*flMacAddress); err != nil { @@ -371,7 +336,7 @@ func Parse(cmd *flag.FlagSet, args []string) (*Config, *HostConfig, *flag.FlagSe DNSSearch: flDNSSearch.GetAll(), ExtraHosts: flExtraHosts.GetAll(), VolumesFrom: flVolumesFrom.GetAll(), - NetworkMode: netMode, + NetworkMode: NetworkMode(*flNetMode), IpcMode: ipcMode, PidMode: pidMode, UTSMode: utsMode, diff --git a/runconfig/parse_test.go b/runconfig/parse_test.go index 8916e7d4df..2acefbfb3d 100644 --- a/runconfig/parse_test.go +++ b/runconfig/parse_test.go @@ -204,77 +204,6 @@ func TestParseLxcConfOpt(t *testing.T) { } -func TestNetHostname(t *testing.T) { - if _, _, _, err := parseRun([]string{"-h=name", "img", "cmd"}); err != nil { - t.Fatalf("Unexpected error: %s", err) - } - - if _, _, _, err := parseRun([]string{"--net=host", "img", "cmd"}); err != nil { - t.Fatalf("Unexpected error: %s", err) - } - - if _, _, _, err := parseRun([]string{"-h=name", "--net=bridge", "img", "cmd"}); err != nil { - t.Fatalf("Unexpected error: %s", err) - } - - if _, _, _, err := parseRun([]string{"-h=name", "--net=none", "img", "cmd"}); err != nil { - t.Fatalf("Unexpected error: %s", err) - } - - if _, _, _, err := parseRun([]string{"-h=name", "--net=host", "img", "cmd"}); err != ErrConflictNetworkHostname { - t.Fatalf("Expected error ErrConflictNetworkHostname, got: %s", err) - } - - if _, _, _, err := parseRun([]string{"-h=name", "--net=container:other", "img", "cmd"}); err != ErrConflictNetworkHostname { - t.Fatalf("Expected error ErrConflictNetworkHostname, got: %s", err) - } - if _, _, _, err := parseRun([]string{"--net=container", "img", "cmd"}); err == nil || err.Error() != "--net: invalid net mode: invalid container format container:" { - t.Fatalf("Expected error with --net=container, got : %v", err) - } - if _, _, _, err := parseRun([]string{"--net=weird", "img", "cmd"}); err == nil || err.Error() != "--net: invalid net mode: invalid --net: weird" { - t.Fatalf("Expected error with --net=weird, got: %s", err) - } -} - -func TestConflictContainerNetworkAndLinks(t *testing.T) { - if _, _, _, err := parseRun([]string{"--net=container:other", "--link=zip:zap", "img", "cmd"}); err != ErrConflictContainerNetworkAndLinks { - t.Fatalf("Expected error ErrConflictContainerNetworkAndLinks, got: %s", err) - } - if _, _, _, err := parseRun([]string{"--net=host", "--link=zip:zap", "img", "cmd"}); err != ErrConflictHostNetworkAndLinks { - t.Fatalf("Expected error ErrConflictHostNetworkAndLinks, got: %s", err) - } -} - -func TestConflictNetworkModeAndOptions(t *testing.T) { - if _, _, _, err := parseRun([]string{"--net=host", "--dns=8.8.8.8", "img", "cmd"}); err != ErrConflictNetworkAndDNS { - t.Fatalf("Expected error ErrConflictNetworkAndDns, got %s", err) - } - if _, _, _, err := parseRun([]string{"--net=container:other", "--dns=8.8.8.8", "img", "cmd"}); err != ErrConflictNetworkAndDNS { - t.Fatalf("Expected error ErrConflictNetworkAndDns, got %s", err) - } - if _, _, _, err := parseRun([]string{"--net=host", "--add-host=name:8.8.8.8", "img", "cmd"}); err != ErrConflictNetworkHosts { - t.Fatalf("Expected error ErrConflictNetworkAndDns, got %s", err) - } - if _, _, _, err := parseRun([]string{"--net=container:other", "--add-host=name:8.8.8.8", "img", "cmd"}); err != ErrConflictNetworkHosts { - t.Fatalf("Expected error ErrConflictNetworkAndDns, got %s", err) - } - if _, _, _, err := parseRun([]string{"--net=host", "--mac-address=92:d0:c6:0a:29:33", "img", "cmd"}); err != ErrConflictContainerNetworkAndMac { - t.Fatalf("Expected error ErrConflictContainerNetworkAndMac, got %s", err) - } - if _, _, _, err := parseRun([]string{"--net=container:other", "--mac-address=92:d0:c6:0a:29:33", "img", "cmd"}); err != ErrConflictContainerNetworkAndMac { - t.Fatalf("Expected error ErrConflictContainerNetworkAndMac, got %s", err) - } - if _, _, _, err := parseRun([]string{"--net=container:other", "-P", "img", "cmd"}); err != ErrConflictNetworkPublishPorts { - t.Fatalf("Expected error ErrConflictNetworkPublishPorts, got %s", err) - } - if _, _, _, err := parseRun([]string{"--net=container:other", "-p", "8080", "img", "cmd"}); err != ErrConflictNetworkPublishPorts { - t.Fatalf("Expected error ErrConflictNetworkPublishPorts, got %s", err) - } - if _, _, _, err := parseRun([]string{"--net=container:other", "--expose", "8000-9000", "img", "cmd"}); err != ErrConflictNetworkExposePorts { - t.Fatalf("Expected error ErrConflictNetworkExposePorts, got %s", err) - } -} - // Simple parse with MacAddress validatation func TestParseWithMacAddress(t *testing.T) { invalidMacAddress := "--mac-address=invalidMacAddress" diff --git a/runconfig/parse_unix.go b/runconfig/parse_unix.go index 7086b1ad08..1381685d85 100644 --- a/runconfig/parse_unix.go +++ b/runconfig/parse_unix.go @@ -7,51 +7,53 @@ import ( "strings" ) -func parseNetMode(netMode string) (NetworkMode, error) { - parts := strings.Split(netMode, ":") +// ValidateNetMode ensures that the various combinations of requested +// network settings are valid. +func ValidateNetMode(c *Config, hc *HostConfig) error { + // We may not be passed a host config, such as in the case of docker commit + if hc == nil { + return nil + } + parts := strings.Split(string(hc.NetworkMode), ":") switch mode := parts[0]; mode { case "default", "bridge", "none", "host": case "container": if len(parts) < 2 || parts[1] == "" { - return "", fmt.Errorf("invalid container format container:") + return fmt.Errorf("--net: invalid net mode: invalid container format container:") } default: - return "", fmt.Errorf("invalid --net: %s", netMode) + return fmt.Errorf("invalid --net: %s", hc.NetworkMode) } - return NetworkMode(netMode), nil -} -func validateNetMode(vals *validateNM) error { - - if (vals.netMode.IsHost() || vals.netMode.IsContainer()) && *vals.flHostname != "" { + if (hc.NetworkMode.IsHost() || hc.NetworkMode.IsContainer()) && c.Hostname != "" { return ErrConflictNetworkHostname } - if vals.netMode.IsHost() && vals.flLinks.Len() > 0 { + if hc.NetworkMode.IsHost() && len(hc.Links) > 0 { return ErrConflictHostNetworkAndLinks } - if vals.netMode.IsContainer() && vals.flLinks.Len() > 0 { + if hc.NetworkMode.IsContainer() && len(hc.Links) > 0 { return ErrConflictContainerNetworkAndLinks } - if (vals.netMode.IsHost() || vals.netMode.IsContainer()) && vals.flDNS.Len() > 0 { + if (hc.NetworkMode.IsHost() || hc.NetworkMode.IsContainer()) && len(hc.DNS) > 0 { return ErrConflictNetworkAndDNS } - if (vals.netMode.IsContainer() || vals.netMode.IsHost()) && vals.flExtraHosts.Len() > 0 { + if (hc.NetworkMode.IsContainer() || hc.NetworkMode.IsHost()) && len(hc.ExtraHosts) > 0 { return ErrConflictNetworkHosts } - if (vals.netMode.IsContainer() || vals.netMode.IsHost()) && *vals.flMacAddress != "" { + if (hc.NetworkMode.IsContainer() || hc.NetworkMode.IsHost()) && c.MacAddress != "" { return ErrConflictContainerNetworkAndMac } - if vals.netMode.IsContainer() && (vals.flPublish.Len() > 0 || *vals.flPublishAll == true) { + if hc.NetworkMode.IsContainer() && (len(hc.PortBindings) > 0 || hc.PublishAllPorts == true) { return ErrConflictNetworkPublishPorts } - if vals.netMode.IsContainer() && vals.flExpose.Len() > 0 { + if hc.NetworkMode.IsContainer() && len(c.ExposedPorts) > 0 { return ErrConflictNetworkExposePorts } return nil diff --git a/runconfig/parse_windows.go b/runconfig/parse_windows.go index ca0a2e6d3a..01e39c7f85 100644 --- a/runconfig/parse_windows.go +++ b/runconfig/parse_windows.go @@ -5,16 +5,18 @@ import ( "strings" ) -func parseNetMode(netMode string) (NetworkMode, error) { - parts := strings.Split(netMode, ":") +// ValidateNetMode ensures that the various combinations of requested +// network settings are valid. +func ValidateNetMode(c *Config, hc *HostConfig) error { + // We may not be passed a host config, such as in the case of docker commit + if hc == nil { + return nil + } + parts := strings.Split(string(hc.NetworkMode), ":") switch mode := parts[0]; mode { case "default", "none": default: - return "", fmt.Errorf("invalid --net: %s", netMode) + return fmt.Errorf("invalid --net: %s", hc.NetworkMode) } - return NetworkMode(netMode), nil -} - -func validateNetMode(vals *validateNM) error { return nil }