Move netmode validation to server

Signed-off-by: John Howard <[email protected]>

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

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
 	}

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()

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:<name|id>", 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)
+}

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
 }

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

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

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

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

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

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