Merge pull request moby#45905 from akerouanton/endpoint-specific-mac-…

…address

api: Add a field MacAddress to EndpointSettings

api/server/router/container/container_routes.go

@@ -628,6 +628,13 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
 		}
 	}
 
+	var warnings []string
+	if warn, err := handleMACAddressBC(config, hostConfig, networkingConfig, version); err != nil {
+		return err
+	} else if warn != "" {
+		warnings = append(warnings, warn)
+	}
+
 	if hostConfig.PidsLimit != nil && *hostConfig.PidsLimit <= 0 {
 		// Don't set a limit if either no limit was specified, or "unlimited" was
 		// explicitly set.
@@ -647,10 +654,58 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
 	if err != nil {
 		return err
 	}
-
+	ccr.Warnings = append(ccr.Warnings, warnings...)
 	return httputils.WriteJSON(w, http.StatusCreated, ccr)
 }
 
+// handleMACAddressBC takes care of backward-compatibility for the container-wide MAC address by mutating the
+// networkingConfig to set the endpoint-specific MACAddress field introduced in API v1.44. It returns a warning message
+// or an error if the container-wide field was specified for API >= v1.44.
+func handleMACAddressBC(config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, version string) (string, error) {
+	if config.MacAddress == "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
+		return "", nil
+	}
+
+	deprecatedMacAddress := config.MacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
+
+	if versions.LessThan(version, "1.44") {
+		// The container-wide MacAddress parameter is deprecated and should now be specified in EndpointsConfig.
+		if hostConfig.NetworkMode.IsDefault() || hostConfig.NetworkMode.IsBridge() || hostConfig.NetworkMode.IsUserDefined() {
+			nwName := hostConfig.NetworkMode.NetworkName()
+			if _, ok := networkingConfig.EndpointsConfig[nwName]; !ok {
+				networkingConfig.EndpointsConfig[nwName] = &network.EndpointSettings{}
+			}
+			// Overwrite the config: either the endpoint's MacAddress was set by the user on API < v1.44, which
+			// must be ignored, or migrate the top-level MacAddress to the endpoint's config.
+			networkingConfig.EndpointsConfig[nwName].MacAddress = deprecatedMacAddress
+		}
+		if !hostConfig.NetworkMode.IsDefault() && !hostConfig.NetworkMode.IsBridge() && !hostConfig.NetworkMode.IsUserDefined() {
+			return "", runconfig.ErrConflictContainerNetworkAndMac
+		}
+
+		return "", nil
+	}
+
+	var warning string
+	if hostConfig.NetworkMode.IsDefault() || hostConfig.NetworkMode.IsBridge() || hostConfig.NetworkMode.IsUserDefined() {
+		nwName := hostConfig.NetworkMode.NetworkName()
+		if _, ok := networkingConfig.EndpointsConfig[nwName]; !ok {
+			networkingConfig.EndpointsConfig[nwName] = &network.EndpointSettings{}
+		}
+
+		ep := networkingConfig.EndpointsConfig[nwName]
+		if ep.MacAddress == "" {
+			ep.MacAddress = deprecatedMacAddress
+		} else if ep.MacAddress != deprecatedMacAddress {
+			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"))
+		}
+	}
+	warning = "The container-wide MacAddress field is now deprecated. It should be specified in EndpointsConfig instead."
+	config.MacAddress = "" //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
+
+	return warning, nil
+}
+
 func (s *containerRouter) deleteContainers(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	if err := httputils.ParseForm(r); err != nil {
 		return err

api/swagger.yaml

@@ -1313,7 +1313,10 @@ definitions:
         type: "boolean"
         x-nullable: true
       MacAddress:
-        description: "MAC address of the container."
+        description: |
+          MAC address of the container.
+
+          Deprecated: this field is deprecated in API v1.44 and up. Use EndpointSettings.MacAddress instead.
         type: "string"
         x-nullable: true
       OnBuild:
@@ -1381,6 +1384,7 @@ definitions:
             LinkLocalIPs:
               - "169.254.34.68"
               - "fe80::3468"
+          MacAddress: "02:42:ac:12:05:02"
           Links:
             - "container_1"
             - "container_2"
@@ -2455,6 +2459,11 @@ definitions:
         example:
           - "container_1"
           - "container_2"
+      MacAddress:
+        description: |
+          MAC address for the endpoint on this network. The network driver might ignore this parameter.
+        type: "string"
+        example: "02:42:ac:11:00:04"
       Aliases:
         type: "array"
         items:
@@ -2505,11 +2514,6 @@ definitions:
         type: "integer"
         format: "int64"
         example: 64
-      MacAddress:
-        description: |
-          MAC address for the endpoint on this network.
-        type: "string"
-        example: "02:42:ac:11:00:04"
       DriverOpts:
         description: |
           DriverOpts is a mapping of driver options and values. These options
@@ -10130,6 +10134,7 @@ paths:
                 IPAMConfig:
                   IPv4Address: "172.24.56.89"
                   IPv6Address: "2001:db8::5689"
+                MacAddress: "02:42:ac:12:05:02"
       tags: ["Network"]
 
   /networks/{id}/disconnect:

api/types/container/config.go

@@ -70,10 +70,13 @@ type Config struct {
 	WorkingDir      string              // Current directory (PWD) in the command will be launched
 	Entrypoint      strslice.StrSlice   // Entrypoint to run when starting the container
 	NetworkDisabled bool                `json:",omitempty"` // Is network disabled
-	MacAddress      string              `json:",omitempty"` // Mac Address of the container
-	OnBuild         []string            // ONBUILD metadata that were defined on the image Dockerfile
-	Labels          map[string]string   // List of labels set to this container
-	StopSignal      string              `json:",omitempty"` // Signal to stop a container
-	StopTimeout     *int                `json:",omitempty"` // Timeout (in seconds) to stop a container
-	Shell           strslice.StrSlice   `json:",omitempty"` // Shell for shell-form of RUN, CMD, ENTRYPOINT
+	// Mac Address of the container.
+	//
+	// Deprecated: this field is deprecated since API v1.44. Use EndpointSettings.MacAddress instead.
+	MacAddress  string            `json:",omitempty"`
+	OnBuild     []string          // ONBUILD metadata that were defined on the image Dockerfile
+	Labels      map[string]string // List of labels set to this container
+	StopSignal  string            `json:",omitempty"` // Signal to stop a container
+	StopTimeout *int              `json:",omitempty"` // Timeout (in seconds) to stop a container
+	Shell       strslice.StrSlice `json:",omitempty"` // Shell for shell-form of RUN, CMD, ENTRYPOINT
 }

api/types/network/endpoint.go

@@ -14,6 +14,7 @@ type EndpointSettings struct {
 	IPAMConfig *EndpointIPAMConfig
 	Links      []string
 	Aliases    []string
+	MacAddress string
 	// Operational data
 	NetworkID           string
 	EndpointID          string
@@ -23,7 +24,6 @@ type EndpointSettings struct {
 	IPv6Gateway         string
 	GlobalIPv6Address   string
 	GlobalIPv6PrefixLen int
-	MacAddress          string
 	DriverOpts          map[string]string
 }
 

client/container_create.go

@@ -39,6 +39,9 @@ func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config
 	if err := cli.NewVersionError(ctx, "1.44", "specify health-check start interval"); config != nil && config.Healthcheck != nil && config.Healthcheck.StartInterval != 0 && err != nil {
 		return response, err
 	}
+	if err := cli.NewVersionError(ctx, "1.44", "specify mac-address per network"); hasEndpointSpecificMacAddress(networkingConfig) && err != nil {
+		return response, err
+	}
 
 	if hostConfig != nil {
 		if versions.LessThan(cli.ClientVersion(), "1.25") {
@@ -55,6 +58,11 @@ func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config
 		}
 	}
 
+	// Since API 1.44, the container-wide MacAddress is deprecated and will trigger a WARNING if it's specified.
+	if versions.GreaterThanOrEqualTo(cli.ClientVersion(), "1.44") {
+		config.MacAddress = "" //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
+	}
+
 	query := url.Values{}
 	if p := formatPlatform(platform); p != "" {
 		query.Set("platform", p)
@@ -91,3 +99,16 @@ func formatPlatform(platform *ocispec.Platform) string {
 	}
 	return path.Join(platform.OS, platform.Architecture, platform.Variant)
 }
+
+// hasEndpointSpecificMacAddress checks whether one of the endpoint in networkingConfig has a MacAddress defined.
+func hasEndpointSpecificMacAddress(networkingConfig *network.NetworkingConfig) bool {
+	if networkingConfig == nil {
+		return false
+	}
+	for _, endpoint := range networkingConfig.EndpointsConfig {
+		if endpoint.MacAddress != "" {
+			return true
+		}
+	}
+	return false
+}

daemon/container_operations.go

@@ -612,6 +612,13 @@ func validateEndpointSettings(nw *libnetwork.Network, nwName string, epConfig *n
 		}
 	}
 
+	if epConfig.MacAddress != "" {
+		_, err := net.ParseMAC(epConfig.MacAddress)
+		if err != nil {
+			return fmt.Errorf("invalid MAC address %s", epConfig.MacAddress)
+		}
+	}
+
 	if err := multierror.Join(errs...); err != nil {
 		return fmt.Errorf("invalid endpoint settings:\n%w", err)
 	}
@@ -628,7 +635,6 @@ func cleanOperationalData(es *network.EndpointSettings) {
 	es.IPv6Gateway = ""
 	es.GlobalIPv6Address = ""
 	es.GlobalIPv6PrefixLen = 0
-	es.MacAddress = ""
 	if es.IPAMOperational {
 		es.IPAMConfig = nil
 	}

daemon/inspect.go

@@ -27,8 +27,9 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, size bo
 		return daemon.containerInspectPre120(ctx, name)
 	case versions.Equal(version, "1.20"):
 		return daemon.containerInspect120(name)
+	default:
+		return daemon.ContainerInspectCurrent(ctx, name, size)
 	}
-	return daemon.ContainerInspectCurrent(ctx, name, size)
 }
 
 // ContainerInspectCurrent returns low-level information about a
@@ -116,7 +117,7 @@ func (daemon *Daemon) containerInspect120(name string) (*v1p20.ContainerJSON, er
 		Mounts:            ctr.GetMountPoints(),
 		Config: &v1p20.ContainerConfig{
 			Config:          ctr.Config,
-			MacAddress:      ctr.Config.MacAddress,
+			MacAddress:      ctr.Config.MacAddress, //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
 			NetworkDisabled: ctr.Config.NetworkDisabled,
 			ExposedPorts:    ctr.Config.ExposedPorts,
 			VolumeDriver:    ctr.HostConfig.VolumeDriver,
@@ -138,6 +139,18 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, container *contai
 	// We merge the Ulimits from hostConfig with daemon default
 	daemon.mergeUlimits(&hostConfig, daemonCfg)
 
+	// Migrate the container's default network's MacAddress to the top-level
+	// Config.MacAddress field for older API versions (< 1.44). We set it here
+	// unconditionally, to keep backward compatibility with clients that use
+	// unversioned API endpoints.
+	if container.Config != nil && container.Config.MacAddress == "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
+		if nwm := hostConfig.NetworkMode; nwm.IsDefault() || nwm.IsBridge() || nwm.IsUserDefined() {
+			if epConf, ok := container.NetworkSettings.Networks[nwm.NetworkName()]; ok {
+				container.Config.MacAddress = epConf.MacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
+			}
+		}
+	}
+
 	var containerHealth *types.Health
 	if container.State.Health != nil {
 		containerHealth = &types.Health{

daemon/inspect_linux.go

@@ -47,7 +47,7 @@ func (daemon *Daemon) containerInspectPre120(ctx context.Context, name string) (
 		VolumesRW:         volumesRW,
 		Config: &v1p19.ContainerConfig{
 			Config:          ctr.Config,
-			MacAddress:      ctr.Config.MacAddress,
+			MacAddress:      ctr.Config.MacAddress, //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
 			NetworkDisabled: ctr.Config.NetworkDisabled,
 			ExposedPorts:    ctr.Config.ExposedPorts,
 			VolumeDriver:    ctr.HostConfig.VolumeDriver,

daemon/network.go

@@ -788,6 +788,7 @@ func (daemon *Daemon) clearAttachableNetworks() {
 // buildCreateEndpointOptions builds endpoint options from a given network.
 func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, epConfig *network.EndpointSettings, sb *libnetwork.Sandbox, daemonDNS []string) ([]libnetwork.EndpointOption, error) {
 	var createOptions []libnetwork.EndpointOption
+	var genericOptions = make(options.Generic)
 
 	nwName := n.Name()
 	defaultNetName := runconfig.DefaultDaemonNetworkMode().NetworkName()
@@ -825,6 +826,14 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e
 		for k, v := range epConfig.DriverOpts {
 			createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(options.Generic{k: v}))
 		}
+
+		if epConfig.MacAddress != "" {
+			mac, err := net.ParseMAC(epConfig.MacAddress)
+			if err != nil {
+				return nil, err
+			}
+			genericOptions[netlabel.MacAddress] = mac
+		}
 	}
 
 	if svcCfg := c.NetworkSettings.Service; svcCfg != nil {
@@ -852,23 +861,6 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e
 		createOptions = append(createOptions, libnetwork.CreateOptionDisableResolution())
 	}
 
-	// configs that are applicable only for the endpoint in the network
-	// to which container was connected to on docker run.
-	// Ideally all these network-specific endpoint configurations must be moved under
-	// container.NetworkSettings.Networks[n.Name()]
-	netMode := c.HostConfig.NetworkMode
-	if nwName == netMode.NetworkName() || n.ID() == netMode.NetworkName() || (nwName == defaultNetName && netMode.IsDefault()) {
-		if c.Config.MacAddress != "" {
-			mac, err := net.ParseMAC(c.Config.MacAddress)
-			if err != nil {
-				return nil, err
-			}
-			createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(options.Generic{
-				netlabel.MacAddress: mac,
-			}))
-		}
-	}
-
 	// Port-mapping rules belong to the container & applicable only to non-internal networks.
 	//
 	// TODO(thaJeztah): Look if we can provide a more minimal function for getPortMapInfo, as it does a lot, and we only need the "length".
@@ -940,7 +932,10 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e
 		createOptions = append(createOptions, libnetwork.CreateOptionDNS(daemonDNS))
 	}
 
-	createOptions = append(createOptions, libnetwork.CreateOptionPortMapping(publishedPorts), libnetwork.CreateOptionExposedPorts(exposedPorts))
+	createOptions = append(createOptions,
+		libnetwork.CreateOptionPortMapping(publishedPorts),
+		libnetwork.CreateOptionExposedPorts(exposedPorts),
+		libnetwork.EndpointOptionGeneric(genericOptions))
 
 	return createOptions, nil
 }

docs/api/version-history.md

@@ -55,6 +55,9 @@ keywords: "API, Docker, rcli, REST, documentation"
 * `POST /services/create` and `POST /services/{id}/update` now accept `Seccomp`
   and `AppArmor` fields in the `ContainerSpec.Privileges` object. This allows
   some configuration of Seccomp and AppArmor in Swarm services.
+* A new endpoint-specific `MacAddress` field has been added to `NetworkSettings.EndpointSettings`
+  on `POST /containers/create`, and to `EndpointConfig` on `POST /networks/{id}/connect`.
+  The container-wide `MacAddress` field in `Config`, on `POST /containers/create`, is now deprecated.
 
 ## v1.43 API changes
 

integration-cli/docker_cli_netmode_test.go

@@ -31,6 +31,7 @@ func (s *DockerCLINetmodeSuite) OnTimeout(c *testing.T) {
 // DockerCmdWithFail executes a docker command that is supposed to fail and returns
 // the output. If the command returns a Nil error, it will fail and stop the tests.
 func dockerCmdWithFail(c *testing.T, args ...string) string {
+	c.Helper()
 	out, _, err := dockerCmdWithError(args...)
 	assert.Assert(c, err != nil, "%v", out)
 	return out
@@ -88,8 +89,6 @@ func (s *DockerCLINetmodeSuite) TestConflictNetworkModeAndOptions(c *testing.T)
 	assert.Assert(c, strings.Contains(out, runconfig.ErrConflictNetworkAndDNS.Error()))
 	out = dockerCmdWithFail(c, "run", "--net=container:other", "--add-host=name:8.8.8.8", "busybox", "ps")
 	assert.Assert(c, strings.Contains(out, runconfig.ErrConflictNetworkHosts.Error()))
-	out = dockerCmdWithFail(c, "run", "--net=container:other", "--mac-address=92:d0:c6:0a:29:33", "busybox", "ps")
-	assert.Assert(c, strings.Contains(out, runconfig.ErrConflictContainerNetworkAndMac.Error()))
 	out = dockerCmdWithFail(c, "run", "--net=container:other", "-P", "busybox", "ps")
 	assert.Assert(c, strings.Contains(out, runconfig.ErrConflictNetworkPublishPorts.Error()))
 	out = dockerCmdWithFail(c, "run", "--net=container:other", "-p", "8080", "busybox", "ps")

integration-cli/docker_cli_run_test.go

@@ -3311,11 +3311,6 @@ func (s *DockerCLIRunSuite) TestRunContainerNetModeWithDNSMacHosts(c *testing.T)
 		c.Fatalf("run --net=container with --dns should error out")
 	}
 
-	out, _, err = dockerCmdWithError("run", "--mac-address", "92:d0:c6:0a:29:33", "--net=container:parent", "busybox")
-	if err == nil || !strings.Contains(out, runconfig.ErrConflictContainerNetworkAndMac.Error()) {
-		c.Fatalf("run --net=container with --mac-address should error out")
-	}
-
 	out, _, err = dockerCmdWithError("run", "--add-host", "test:192.168.2.109", "--net=container:parent", "busybox")
 	if err == nil || !strings.Contains(out, runconfig.ErrConflictNetworkHosts.Error()) {
 		c.Fatalf("run --net=container with --add-host should error out")