Browse code

api: Deprecate ContainerConfig.MacAddress

Having a sandbox/container-wide MacAddress field makes little sense
since a container can be connected to multiple networks at the same
time. This field is an artefact of old times where a container could be
connected to a single network only.

As we now have a way to specify per-endpoint mac address, this field is
now deprecated.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Albin Kerouanton authored on 2023/07/04 19:40:35
Showing 12 changed files
... ...
@@ -628,6 +628,13 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
628 628
 		}
629 629
 	}
630 630
 
631
+	var warnings []string
632
+	if warn, err := handleMACAddressBC(config, hostConfig, networkingConfig, version); err != nil {
633
+		return err
634
+	} else if warn != "" {
635
+		warnings = append(warnings, warn)
636
+	}
637
+
631 638
 	if hostConfig.PidsLimit != nil && *hostConfig.PidsLimit <= 0 {
632 639
 		// Don't set a limit if either no limit was specified, or "unlimited" was
633 640
 		// explicitly set.
... ...
@@ -647,10 +654,58 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
647 647
 	if err != nil {
648 648
 		return err
649 649
 	}
650
-
650
+	ccr.Warnings = append(ccr.Warnings, warnings...)
651 651
 	return httputils.WriteJSON(w, http.StatusCreated, ccr)
652 652
 }
653 653
 
654
+// handleMACAddressBC takes care of backward-compatibility for the container-wide MAC address by mutating the
655
+// networkingConfig to set the endpoint-specific MACAddress field introduced in API v1.44. It returns a warning message
656
+// or an error if the container-wide field was specified for API >= v1.44.
657
+func handleMACAddressBC(config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, version string) (string, error) {
658
+	if config.MacAddress == "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
659
+		return "", nil
660
+	}
661
+
662
+	deprecatedMacAddress := config.MacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
663
+
664
+	if versions.LessThan(version, "1.44") {
665
+		// The container-wide MacAddress parameter is deprecated and should now be specified in EndpointsConfig.
666
+		if hostConfig.NetworkMode.IsDefault() || hostConfig.NetworkMode.IsBridge() || hostConfig.NetworkMode.IsUserDefined() {
667
+			nwName := hostConfig.NetworkMode.NetworkName()
668
+			if _, ok := networkingConfig.EndpointsConfig[nwName]; !ok {
669
+				networkingConfig.EndpointsConfig[nwName] = &network.EndpointSettings{}
670
+			}
671
+			// Overwrite the config: either the endpoint's MacAddress was set by the user on API < v1.44, which
672
+			// must be ignored, or migrate the top-level MacAddress to the endpoint's config.
673
+			networkingConfig.EndpointsConfig[nwName].MacAddress = deprecatedMacAddress
674
+		}
675
+		if !hostConfig.NetworkMode.IsDefault() && !hostConfig.NetworkMode.IsBridge() && !hostConfig.NetworkMode.IsUserDefined() {
676
+			return "", runconfig.ErrConflictContainerNetworkAndMac
677
+		}
678
+
679
+		return "", nil
680
+	}
681
+
682
+	var warning string
683
+	if hostConfig.NetworkMode.IsDefault() || hostConfig.NetworkMode.IsBridge() || hostConfig.NetworkMode.IsUserDefined() {
684
+		nwName := hostConfig.NetworkMode.NetworkName()
685
+		if _, ok := networkingConfig.EndpointsConfig[nwName]; !ok {
686
+			networkingConfig.EndpointsConfig[nwName] = &network.EndpointSettings{}
687
+		}
688
+
689
+		ep := networkingConfig.EndpointsConfig[nwName]
690
+		if ep.MacAddress == "" {
691
+			ep.MacAddress = deprecatedMacAddress
692
+		} else if ep.MacAddress != deprecatedMacAddress {
693
+			return "", errdefs.InvalidParameter(errors.New("the container-wide MAC address should match the endpoint-specific MAC address for the main network or should be left empty"))
694
+		}
695
+	}
696
+	warning = "The container-wide MacAddress field is now deprecated. It should be specified in EndpointsConfig instead."
697
+	config.MacAddress = "" //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
698
+
699
+	return warning, nil
700
+}
701
+
654 702
 func (s *containerRouter) deleteContainers(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
655 703
 	if err := httputils.ParseForm(r); err != nil {
656 704
 		return err
... ...
@@ -1313,7 +1313,10 @@ definitions:
1313 1313
         type: "boolean"
1314 1314
         x-nullable: true
1315 1315
       MacAddress:
1316
-        description: "MAC address of the container."
1316
+        description: |
1317
+          MAC address of the container.
1318
+
1319
+          Deprecated: this field is deprecated in API v1.44 and up. Use EndpointSettings.MacAddress instead.
1317 1320
         type: "string"
1318 1321
         x-nullable: true
1319 1322
       OnBuild:
... ...
@@ -70,10 +70,13 @@ type Config struct {
70 70
 	WorkingDir      string              // Current directory (PWD) in the command will be launched
71 71
 	Entrypoint      strslice.StrSlice   // Entrypoint to run when starting the container
72 72
 	NetworkDisabled bool                `json:",omitempty"` // Is network disabled
73
-	MacAddress      string              `json:",omitempty"` // Mac Address of the container
74
-	OnBuild         []string            // ONBUILD metadata that were defined on the image Dockerfile
75
-	Labels          map[string]string   // List of labels set to this container
76
-	StopSignal      string              `json:",omitempty"` // Signal to stop a container
77
-	StopTimeout     *int                `json:",omitempty"` // Timeout (in seconds) to stop a container
78
-	Shell           strslice.StrSlice   `json:",omitempty"` // Shell for shell-form of RUN, CMD, ENTRYPOINT
73
+	// Mac Address of the container.
74
+	//
75
+	// Deprecated: this field is deprecated since API v1.44. Use EndpointSettings.MacAddress instead.
76
+	MacAddress  string            `json:",omitempty"`
77
+	OnBuild     []string          // ONBUILD metadata that were defined on the image Dockerfile
78
+	Labels      map[string]string // List of labels set to this container
79
+	StopSignal  string            `json:",omitempty"` // Signal to stop a container
80
+	StopTimeout *int              `json:",omitempty"` // Timeout (in seconds) to stop a container
81
+	Shell       strslice.StrSlice `json:",omitempty"` // Shell for shell-form of RUN, CMD, ENTRYPOINT
79 82
 }
... ...
@@ -39,7 +39,7 @@ func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config
39 39
 	if err := cli.NewVersionError(ctx, "1.44", "specify health-check start interval"); config != nil && config.Healthcheck != nil && config.Healthcheck.StartInterval != 0 && err != nil {
40 40
 		return response, err
41 41
 	}
42
-	if err := cli.NewVersionError("1.44", "specify mac-address per network"); hasEndpointSpecificMacAddress(networkingConfig) && err != nil {
42
+	if err := cli.NewVersionError(ctx, "1.44", "specify mac-address per network"); hasEndpointSpecificMacAddress(networkingConfig) && err != nil {
43 43
 		return response, err
44 44
 	}
45 45
 
... ...
@@ -58,6 +58,11 @@ func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config
58 58
 		}
59 59
 	}
60 60
 
61
+	// Since API 1.44, the container-wide MacAddress is deprecated and will trigger a WARNING if it's specified.
62
+	if versions.GreaterThanOrEqualTo(cli.ClientVersion(), "1.44") {
63
+		config.MacAddress = "" //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
64
+	}
65
+
61 66
 	query := url.Values{}
62 67
 	if p := formatPlatform(platform); p != "" {
63 68
 		query.Set("platform", p)
... ...
@@ -27,8 +27,9 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, size bo
27 27
 		return daemon.containerInspectPre120(ctx, name)
28 28
 	case versions.Equal(version, "1.20"):
29 29
 		return daemon.containerInspect120(name)
30
+	default:
31
+		return daemon.ContainerInspectCurrent(ctx, name, size)
30 32
 	}
31
-	return daemon.ContainerInspectCurrent(ctx, name, size)
32 33
 }
33 34
 
34 35
 // ContainerInspectCurrent returns low-level information about a
... ...
@@ -116,7 +117,7 @@ func (daemon *Daemon) containerInspect120(name string) (*v1p20.ContainerJSON, er
116 116
 		Mounts:            ctr.GetMountPoints(),
117 117
 		Config: &v1p20.ContainerConfig{
118 118
 			Config:          ctr.Config,
119
-			MacAddress:      ctr.Config.MacAddress,
119
+			MacAddress:      ctr.Config.MacAddress, //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
120 120
 			NetworkDisabled: ctr.Config.NetworkDisabled,
121 121
 			ExposedPorts:    ctr.Config.ExposedPorts,
122 122
 			VolumeDriver:    ctr.HostConfig.VolumeDriver,
... ...
@@ -138,6 +139,18 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, container *contai
138 138
 	// We merge the Ulimits from hostConfig with daemon default
139 139
 	daemon.mergeUlimits(&hostConfig, daemonCfg)
140 140
 
141
+	// Migrate the container's default network's MacAddress to the top-level
142
+	// Config.MacAddress field for older API versions (< 1.44). We set it here
143
+	// unconditionally, to keep backward compatibility with clients that use
144
+	// unversioned API endpoints.
145
+	if container.Config != nil && container.Config.MacAddress == "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
146
+		if nwm := hostConfig.NetworkMode; nwm.IsDefault() || nwm.IsBridge() || nwm.IsUserDefined() {
147
+			if epConf, ok := container.NetworkSettings.Networks[nwm.NetworkName()]; ok {
148
+				container.Config.MacAddress = epConf.MacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
149
+			}
150
+		}
151
+	}
152
+
141 153
 	var containerHealth *types.Health
142 154
 	if container.State.Health != nil {
143 155
 		containerHealth = &types.Health{
... ...
@@ -47,7 +47,7 @@ func (daemon *Daemon) containerInspectPre120(ctx context.Context, name string) (
47 47
 		VolumesRW:         volumesRW,
48 48
 		Config: &v1p19.ContainerConfig{
49 49
 			Config:          ctr.Config,
50
-			MacAddress:      ctr.Config.MacAddress,
50
+			MacAddress:      ctr.Config.MacAddress, //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
51 51
 			NetworkDisabled: ctr.Config.NetworkDisabled,
52 52
 			ExposedPorts:    ctr.Config.ExposedPorts,
53 53
 			VolumeDriver:    ctr.HostConfig.VolumeDriver,
... ...
@@ -861,22 +861,6 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e
861 861
 		createOptions = append(createOptions, libnetwork.CreateOptionDisableResolution())
862 862
 	}
863 863
 
864
-	// configs that are applicable only for the endpoint in the network
865
-	// to which container was connected to on docker run.
866
-	// Ideally all these network-specific endpoint configurations must be moved under
867
-	// container.NetworkSettings.Networks[n.Name()]
868
-	netMode := c.HostConfig.NetworkMode
869
-	if nwName == netMode.NetworkName() || n.ID() == netMode.NetworkName() || (nwName == defaultNetName && netMode.IsDefault()) {
870
-		if c.Config.MacAddress != "" {
871
-			mac, err := net.ParseMAC(c.Config.MacAddress)
872
-			if err != nil {
873
-				return nil, err
874
-			}
875
-
876
-			genericOptions[netlabel.MacAddress] = mac
877
-		}
878
-	}
879
-
880 864
 	// Port-mapping rules belong to the container & applicable only to non-internal networks.
881 865
 	//
882 866
 	// TODO(thaJeztah): Look if we can provide a more minimal function for getPortMapInfo, as it does a lot, and we only need the "length".
... ...
@@ -57,6 +57,7 @@ keywords: "API, Docker, rcli, REST, documentation"
57 57
   some configuration of Seccomp and AppArmor in Swarm services.
58 58
 * A new endpoint-specific `MacAddress` field has been added to `NetworkSettings.EndpointSettings`
59 59
   on `POST /containers/create`, and to `EndpointConfig` on `POST /networks/{id}/connect`.
60
+  The container-wide `MacAddress` field in `Config`, on `POST /containers/create`, is now deprecated.
60 61
 
61 62
 ## v1.43 API changes
62 63
 
... ...
@@ -88,8 +88,6 @@ func (s *DockerCLINetmodeSuite) TestConflictNetworkModeAndOptions(c *testing.T)
88 88
 	assert.Assert(c, strings.Contains(out, runconfig.ErrConflictNetworkAndDNS.Error()))
89 89
 	out = dockerCmdWithFail(c, "run", "--net=container:other", "--add-host=name:8.8.8.8", "busybox", "ps")
90 90
 	assert.Assert(c, strings.Contains(out, runconfig.ErrConflictNetworkHosts.Error()))
91
-	out = dockerCmdWithFail(c, "run", "--net=container:other", "--mac-address=92:d0:c6:0a:29:33", "busybox", "ps")
92
-	assert.Assert(c, strings.Contains(out, runconfig.ErrConflictContainerNetworkAndMac.Error()))
93 91
 	out = dockerCmdWithFail(c, "run", "--net=container:other", "-P", "busybox", "ps")
94 92
 	assert.Assert(c, strings.Contains(out, runconfig.ErrConflictNetworkPublishPorts.Error()))
95 93
 	out = dockerCmdWithFail(c, "run", "--net=container:other", "-p", "8080", "busybox", "ps")
... ...
@@ -3311,11 +3311,6 @@ func (s *DockerCLIRunSuite) TestRunContainerNetModeWithDNSMacHosts(c *testing.T)
3311 3311
 		c.Fatalf("run --net=container with --dns should error out")
3312 3312
 	}
3313 3313
 
3314
-	out, _, err = dockerCmdWithError("run", "--mac-address", "92:d0:c6:0a:29:33", "--net=container:parent", "busybox")
3315
-	if err == nil || !strings.Contains(out, runconfig.ErrConflictContainerNetworkAndMac.Error()) {
3316
-		c.Fatalf("run --net=container with --mac-address should error out")
3317
-	}
3318
-
3319 3314
 	out, _, err = dockerCmdWithError("run", "--add-host", "test:192.168.2.109", "--net=container:parent", "busybox")
3320 3315
 	if err == nil || !strings.Contains(out, runconfig.ErrConflictNetworkHosts.Error()) {
3321 3316
 		c.Fatalf("run --net=container with --add-host should error out")
... ...
@@ -312,6 +312,6 @@ func WithStopSignal(stopSignal string) func(c *TestContainerConfig) {
312 312
 
313 313
 func WithContainerWideMacAddress(address string) func(c *TestContainerConfig) {
314 314
 	return func(c *TestContainerConfig) {
315
-		c.Config.MacAddress = address
315
+		c.Config.MacAddress = address //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
316 316
 	}
317 317
 }
... ...
@@ -53,10 +53,6 @@ func validateNetContainerMode(c *container.Config, hc *container.HostConfig) err
53 53
 		return ErrConflictNetworkHosts
54 54
 	}
55 55
 
56
-	if (hc.NetworkMode.IsContainer() || hc.NetworkMode.IsHost()) && c.MacAddress != "" {
57
-		return ErrConflictContainerNetworkAndMac
58
-	}
59
-
60 56
 	if hc.NetworkMode.IsContainer() && (len(hc.PortBindings) > 0 || hc.PublishAllPorts) {
61 57
 		return ErrConflictNetworkPublishPorts
62 58
 	}