Skip to content

Commit f6ed590

Browse files
author
John Howard
committed
Move netmode validation to server
Signed-off-by: John Howard <[email protected]>
1 parent 650feb2 commit f6ed590

15 files changed

Lines changed: 300 additions & 187 deletions

‎Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ RUN set -x \
148148
&& rm -rf "$GOPATH"
149149

150150
# Get the "docker-py" source so we can run their integration tests
151-
ENV DOCKER_PY_COMMIT 8a87001d09852058f08a807ab6e8491d57ca1e88
151+
ENV DOCKER_PY_COMMIT 139850f3f3b17357bab5ba3edfb745fb14043764
152152
RUN git clone https://github.com/docker/docker-py.git /docker-py \
153153
&& cd /docker-py \
154154
&& git checkout -q $DOCKER_PY_COMMIT

‎daemon/container.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,10 @@ func (container *Container) Start() (err error) {
264264
return err
265265
}
266266

267+
// Make sure NetworkMode has an acceptable value. We do this to ensure
268+
// backwards API compatibility.
269+
container.hostConfig = runconfig.SetDefaultNetModeIfBlank(container.hostConfig)
270+
267271
if err := container.initializeNetworking(); err != nil {
268272
return err
269273
}

‎daemon/container_unix.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -913,11 +913,6 @@ func (container *Container) configureNetwork(networkName, service, networkDriver
913913
func (container *Container) initializeNetworking() error {
914914
var err error
915915

916-
// Make sure NetworkMode has an acceptable value before
917-
// initializing networking.
918-
if container.hostConfig.NetworkMode == runconfig.NetworkMode("") {
919-
container.hostConfig.NetworkMode = runconfig.NetworkMode("default")
920-
}
921916
if container.hostConfig.NetworkMode.IsContainer() {
922917
// we need to get the hosts files from the container to join
923918
nc, err := container.getNetworkedContainer()

‎integration-cli/docker_cli_netmode_test.go‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
package main
2+
3+
import (
4+
"os/exec"
5+
"strings"
6+
7+
"github.com/docker/docker/runconfig"
8+
"github.com/go-check/check"
9+
)
10+
11+
// GH14530. Validates combinations of --net= with other options
12+
13+
// stringCheckPS is how the output of PS starts in order to validate that
14+
// the command executed in a container did really run PS correctly.
15+
const stringCheckPS = "PID USER"
16+
17+
// checkContains is a helper function that validates a command output did
18+
// contain what was expected.
19+
func checkContains(expected string, out string, c *check.C) {
20+
if !strings.Contains(out, expected) {
21+
c.Fatalf("Expected '%s', got '%s'", expected, out)
22+
}
23+
}
24+
25+
func (s *DockerSuite) TestNetHostname(c *check.C) {
26+
27+
var (
28+
out string
29+
err error
30+
runCmd *exec.Cmd
31+
)
32+
33+
runCmd = exec.Command(dockerBinary, "run", "-h=name", "busybox", "ps")
34+
if out, _, err = runCommandWithOutput(runCmd); err != nil {
35+
c.Fatalf(out, err)
36+
}
37+
checkContains(stringCheckPS, out, c)
38+
39+
runCmd = exec.Command(dockerBinary, "run", "--net=host", "busybox", "ps")
40+
if out, _, err = runCommandWithOutput(runCmd); err != nil {
41+
c.Fatalf(out, err)
42+
}
43+
checkContains(stringCheckPS, out, c)
44+
45+
runCmd = exec.Command(dockerBinary, "run", "-h=name", "--net=bridge", "busybox", "ps")
46+
if out, _, err = runCommandWithOutput(runCmd); err != nil {
47+
c.Fatalf(out, err)
48+
}
49+
checkContains(stringCheckPS, out, c)
50+
51+
runCmd = exec.Command(dockerBinary, "run", "-h=name", "--net=none", "busybox", "ps")
52+
if out, _, err = runCommandWithOutput(runCmd); err != nil {
53+
c.Fatalf(out, err)
54+
}
55+
checkContains(stringCheckPS, out, c)
56+
57+
runCmd = exec.Command(dockerBinary, "run", "-h=name", "--net=host", "busybox", "ps")
58+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
59+
c.Fatalf(out, err)
60+
}
61+
checkContains(runconfig.ErrConflictNetworkHostname.Error(), out, c)
62+
63+
runCmd = exec.Command(dockerBinary, "run", "-h=name", "--net=container:other", "busybox", "ps")
64+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
65+
c.Fatalf(out, err, c)
66+
}
67+
checkContains(runconfig.ErrConflictNetworkHostname.Error(), out, c)
68+
69+
runCmd = exec.Command(dockerBinary, "run", "--net=container", "busybox", "ps")
70+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
71+
c.Fatalf(out, err, c)
72+
}
73+
checkContains("--net: invalid net mode: invalid container format container:<name|id>", out, c)
74+
75+
runCmd = exec.Command(dockerBinary, "run", "--net=weird", "busybox", "ps")
76+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
77+
c.Fatalf(out, err)
78+
}
79+
checkContains("invalid --net: weird", out, c)
80+
}
81+
82+
func (s *DockerSuite) TestConflictContainerNetworkAndLinks(c *check.C) {
83+
var (
84+
out string
85+
err error
86+
runCmd *exec.Cmd
87+
)
88+
89+
runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "--link=zip:zap", "busybox", "ps")
90+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
91+
c.Fatalf(out, err)
92+
}
93+
checkContains(runconfig.ErrConflictContainerNetworkAndLinks.Error(), out, c)
94+
95+
runCmd = exec.Command(dockerBinary, "run", "--net=host", "--link=zip:zap", "busybox", "ps")
96+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
97+
c.Fatalf(out, err)
98+
}
99+
checkContains(runconfig.ErrConflictHostNetworkAndLinks.Error(), out, c)
100+
}
101+
102+
func (s *DockerSuite) TestConflictNetworkModeAndOptions(c *check.C) {
103+
var (
104+
out string
105+
err error
106+
runCmd *exec.Cmd
107+
)
108+
109+
runCmd = exec.Command(dockerBinary, "run", "--net=host", "--dns=8.8.8.8", "busybox", "ps")
110+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
111+
c.Fatalf(out, err)
112+
}
113+
checkContains(runconfig.ErrConflictNetworkAndDNS.Error(), out, c)
114+
115+
runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "--dns=8.8.8.8", "busybox", "ps")
116+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
117+
c.Fatalf(out, err)
118+
}
119+
checkContains(runconfig.ErrConflictNetworkAndDNS.Error(), out, c)
120+
121+
runCmd = exec.Command(dockerBinary, "run", "--net=host", "--add-host=name:8.8.8.8", "busybox", "ps")
122+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
123+
c.Fatalf(out, err)
124+
}
125+
checkContains(runconfig.ErrConflictNetworkHosts.Error(), out, c)
126+
127+
runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "--add-host=name:8.8.8.8", "busybox", "ps")
128+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
129+
c.Fatalf(out, err)
130+
}
131+
checkContains(runconfig.ErrConflictNetworkHosts.Error(), out, c)
132+
133+
runCmd = exec.Command(dockerBinary, "run", "--net=host", "--mac-address=92:d0:c6:0a:29:33", "busybox", "ps")
134+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
135+
c.Fatalf(out, err)
136+
}
137+
checkContains(runconfig.ErrConflictContainerNetworkAndMac.Error(), out, c)
138+
139+
runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "--mac-address=92:d0:c6:0a:29:33", "busybox", "ps")
140+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
141+
c.Fatalf(out, err)
142+
}
143+
checkContains(runconfig.ErrConflictContainerNetworkAndMac.Error(), out, c)
144+
145+
runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "-P", "busybox", "ps")
146+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
147+
c.Fatalf(out, err)
148+
}
149+
checkContains(runconfig.ErrConflictNetworkPublishPorts.Error(), out, c)
150+
151+
runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "-p", "8080", "busybox", "ps")
152+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
153+
c.Fatalf(out, err)
154+
}
155+
checkContains(runconfig.ErrConflictNetworkPublishPorts.Error(), out, c)
156+
157+
runCmd = exec.Command(dockerBinary, "run", "--net=container:other", "--expose", "8000-9000", "busybox", "ps")
158+
if out, _, err = runCommandWithOutput(runCmd); err == nil {
159+
c.Fatalf(out, err)
160+
}
161+
checkContains(runconfig.ErrConflictNetworkExposePorts.Error(), out, c)
162+
}

‎runconfig/config.go‎

Lines changed: 9 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -158,44 +158,6 @@ type Config struct {
158158
Labels map[string]string // List of labels set to this container
159159
}
160160

161-
// ContainerConfigWrapper is a Config wrapper that hold the container Config (portable)
162-
// and the corresponding HostConfig (non-portable).
163-
type ContainerConfigWrapper struct {
164-
*Config
165-
InnerHostConfig *HostConfig `json:"HostConfig,omitempty"`
166-
Cpuset string `json:",omitempty"` // Deprecated. Exported for backwards compatibility.
167-
*HostConfig // Deprecated. Exported to read attrubutes from json that are not in the inner host config structure.
168-
169-
}
170-
171-
// GetHostConfig gets the HostConfig of the Config.
172-
// It's mostly there to handle Deprecated fields of the ContainerConfigWrapper
173-
func (w *ContainerConfigWrapper) GetHostConfig() *HostConfig {
174-
hc := w.HostConfig
175-
176-
if hc == nil && w.InnerHostConfig != nil {
177-
hc = w.InnerHostConfig
178-
} else if w.InnerHostConfig != nil {
179-
if hc.Memory != 0 && w.InnerHostConfig.Memory == 0 {
180-
w.InnerHostConfig.Memory = hc.Memory
181-
}
182-
if hc.MemorySwap != 0 && w.InnerHostConfig.MemorySwap == 0 {
183-
w.InnerHostConfig.MemorySwap = hc.MemorySwap
184-
}
185-
if hc.CPUShares != 0 && w.InnerHostConfig.CPUShares == 0 {
186-
w.InnerHostConfig.CPUShares = hc.CPUShares
187-
}
188-
189-
hc = w.InnerHostConfig
190-
}
191-
192-
if hc != nil && w.Cpuset != "" && hc.CpusetCpus == "" {
193-
hc.CpusetCpus = w.Cpuset
194-
}
195-
196-
return hc
197-
}
198-
199161
// DecodeContainerConfig decodes a json encoded config into a ContainerConfigWrapper
200162
// struct and returns both a Config and an HostConfig struct
201163
// 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) {
208170
return nil, nil, err
209171
}
210172

211-
return w.Config, w.GetHostConfig(), nil
173+
hc := w.getHostConfig()
174+
175+
// Certain parameters need daemon-side validation that cannot be done
176+
// on the client, as only the daemon knows what is valid for the platform.
177+
if err := ValidateNetMode(w.Config, hc); err != nil {
178+
return nil, nil, err
179+
}
180+
181+
return w.Config, hc, nil
212182
}

‎runconfig/config_unix.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// +build !windows
2+
3+
package runconfig
4+
5+
// ContainerConfigWrapper is a Config wrapper that hold the container Config (portable)
6+
// and the corresponding HostConfig (non-portable).
7+
type ContainerConfigWrapper struct {
8+
*Config
9+
InnerHostConfig *HostConfig `json:"HostConfig,omitempty"`
10+
Cpuset string `json:",omitempty"` // Deprecated. Exported for backwards compatibility.
11+
*HostConfig // Deprecated. Exported to read attributes from json that are not in the inner host config structure.
12+
}
13+
14+
// getHostConfig gets the HostConfig of the Config.
15+
// It's mostly there to handle Deprecated fields of the ContainerConfigWrapper
16+
func (w *ContainerConfigWrapper) getHostConfig() *HostConfig {
17+
hc := w.HostConfig
18+
19+
if hc == nil && w.InnerHostConfig != nil {
20+
hc = w.InnerHostConfig
21+
} else if w.InnerHostConfig != nil {
22+
if hc.Memory != 0 && w.InnerHostConfig.Memory == 0 {
23+
w.InnerHostConfig.Memory = hc.Memory
24+
}
25+
if hc.MemorySwap != 0 && w.InnerHostConfig.MemorySwap == 0 {
26+
w.InnerHostConfig.MemorySwap = hc.MemorySwap
27+
}
28+
if hc.CPUShares != 0 && w.InnerHostConfig.CPUShares == 0 {
29+
w.InnerHostConfig.CPUShares = hc.CPUShares
30+
}
31+
if hc.CpusetCpus != "" && w.InnerHostConfig.CpusetCpus == "" {
32+
w.InnerHostConfig.CpusetCpus = hc.CpusetCpus
33+
}
34+
35+
hc = w.InnerHostConfig
36+
}
37+
38+
if hc != nil && w.Cpuset != "" && hc.CpusetCpus == "" {
39+
hc.CpusetCpus = w.Cpuset
40+
}
41+
42+
// Make sure NetworkMode has an acceptable value. We do this to ensure
43+
// backwards compatible API behaviour.
44+
hc = SetDefaultNetModeIfBlank(hc)
45+
46+
return hc
47+
}

‎runconfig/config_windows.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package runconfig
2+
3+
// ContainerConfigWrapper is a Config wrapper that hold the container Config (portable)
4+
// and the corresponding HostConfig (non-portable).
5+
type ContainerConfigWrapper struct {
6+
*Config
7+
HostConfig *HostConfig `json:"HostConfig,omitempty"`
8+
}
9+
10+
// getHostConfig gets the HostConfig of the Config.
11+
func (w *ContainerConfigWrapper) getHostConfig() *HostConfig {
12+
return w.HostConfig
13+
}

‎runconfig/hostconfig.go‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -298,16 +298,6 @@ type HostConfig struct {
298298
ConsoleSize [2]int // Initial console size on Windows
299299
}
300300

301-
// MergeConfigs merges the specified container Config and HostConfig.
302-
// It creates a ContainerConfigWrapper.
303-
func MergeConfigs(config *Config, hostConfig *HostConfig) *ContainerConfigWrapper {
304-
return &ContainerConfigWrapper{
305-
config,
306-
hostConfig,
307-
"", nil,
308-
}
309-
}
310-
311301
// DecodeHostConfig creates a HostConfig based on the specified Reader.
312302
// It assumes the content of the reader will be JSON, and decodes it.
313303
func DecodeHostConfig(src io.Reader) (*HostConfig, error) {
@@ -318,7 +308,19 @@ func DecodeHostConfig(src io.Reader) (*HostConfig, error) {
318308
return nil, err
319309
}
320310

321-
hc := w.GetHostConfig()
322-
311+
hc := w.getHostConfig()
323312
return hc, nil
324313
}
314+
315+
// SetDefaultNetModeIfBlank changes the NetworkMode in a HostConfig structure
316+
// to default if it is not populated. This ensures backwards compatibility after
317+
// the validation of the network mode was moved from the docker CLI to the
318+
// docker daemon.
319+
func SetDefaultNetModeIfBlank(hc *HostConfig) *HostConfig {
320+
if hc != nil {
321+
if hc.NetworkMode == NetworkMode("") {
322+
hc.NetworkMode = NetworkMode("default")
323+
}
324+
}
325+
return hc
326+
}

‎runconfig/hostconfig_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// +build !windows
2+
13
package runconfig
24

35
import (
@@ -8,6 +10,7 @@ import (
810
"testing"
911
)
1012

13+
// TODO Windows: This will need addressing for a Windows daemon.
1114
func TestNetworkModeTest(t *testing.T) {
1215
networkModes := map[NetworkMode][]bool{
1316
// private, bridge, host, container, none, default

‎runconfig/hostconfig_unix.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,13 @@ func (n NetworkMode) IsContainer() bool {
5858
func (n NetworkMode) IsNone() bool {
5959
return n == "none"
6060
}
61+
62+
// MergeConfigs merges the specified container Config and HostConfig.
63+
// It creates a ContainerConfigWrapper.
64+
func MergeConfigs(config *Config, hostConfig *HostConfig) *ContainerConfigWrapper {
65+
return &ContainerConfigWrapper{
66+
config,
67+
hostConfig,
68+
"", nil,
69+
}
70+
}

0 commit comments

Comments
 (0)