Merge pull request #51355 from corhere/api-macaddr-marshaltext

api/t/network: represent MAC addrs as byte slices

daemon/cluster/executor/backend.go

@@ -43,7 +43,7 @@ type Backend interface {
 	ActivateContainerServiceBinding(containerName string) error
 	DeactivateContainerServiceBinding(containerName string) error
 	UpdateContainerServiceConfig(containerName string, serviceConfig *clustertypes.ServiceConfig) error
-	ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *container.InspectResponse, desiredMACAddress string, _ error)
+	ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *container.InspectResponse, desiredMACAddress network.HardwareAddr, _ error)
 	ContainerWait(ctx context.Context, name string, condition container.WaitCondition) (<-chan containerpkg.StateStatus, error)
 	ContainerRm(name string, config *backend.ContainerRmConfig) error
 	ContainerKill(name string, sig string) error

daemon/container_operations.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"net"
 	"net/netip"
 	"os"
 	"runtime"
@@ -545,13 +544,6 @@ func validateEndpointSettings(nw *libnetwork.Network, nwName string, epConfig *n
 		errs = validateIPAMConfigIsInRange(errs, ipamConfig, v4Configs, v6Configs)
 	}
 
-	if epConfig.MacAddress != "" {
-		_, err := net.ParseMAC(epConfig.MacAddress)
-		if err != nil {
-			return fmt.Errorf("invalid MAC address %s", epConfig.MacAddress)
-		}
-	}
-
 	if sysctls, ok := epConfig.DriverOpts[netlabel.EndpointSysctls]; ok {
 		for _, sysctl := range strings.Split(sysctls, ",") {
 			scname := strings.SplitN(sysctl, ".", 5)
@@ -647,7 +639,7 @@ func cleanOperationalData(es *network.EndpointSettings) {
 	es.IPv6Gateway = netip.Addr{}
 	es.GlobalIPv6Address = netip.Addr{}
 	es.GlobalIPv6PrefixLen = 0
-	es.MacAddress = ""
+	es.MacAddress = nil
 	if es.IPAMOperational {
 		es.IPAMConfig = nil
 	}

daemon/inspect.go

@@ -19,10 +19,10 @@ import (
 // ContainerInspect returns low-level information about a
 // container. Returns an error if the container cannot be found, or if
 // there is an error getting the data.
-func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *containertypes.InspectResponse, desiredMACAddress string, _ error) {
+func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *containertypes.InspectResponse, desiredMACAddress networktypes.HardwareAddr, _ error) {
 	ctr, err := daemon.GetContainer(name)
 	if err != nil {
-		return nil, "", err
+		return nil, nil, err
 	}
 
 	ctr.Lock()
@@ -30,7 +30,7 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options
 	base, desiredMACAddress, err := daemon.getInspectData(&daemon.config().Config, ctr)
 	if err != nil {
 		ctr.Unlock()
-		return nil, "", err
+		return nil, nil, err
 	}
 
 	// TODO(thaJeztah): do we need a deep copy here? Otherwise we could use maps.Clone (see https://github.com/moby/moby/commit/7917a36cc787ada58987320e67cc6d96858f3b55)
@@ -61,7 +61,7 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options
 	if options.Size {
 		sizeRw, sizeRootFs, err := daemon.imageService.GetContainerLayerSize(ctx, base.ID)
 		if err != nil {
-			return nil, "", err
+			return nil, nil, err
 		}
 		base.SizeRw = &sizeRw
 		base.SizeRootFs = &sizeRootFs
@@ -83,7 +83,7 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options
 	return base, desiredMACAddress, nil
 }
 
-func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Container) (_ *containertypes.InspectResponse, desiredMACAddress string, _ error) {
+func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Container) (_ *containertypes.InspectResponse, desiredMACAddress networktypes.HardwareAddr, _ error) {
 	// make a copy to play with
 	hostConfig := *ctr.HostConfig
 
@@ -101,7 +101,7 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co
 	// 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.
-	var macAddress string
+	var macAddress networktypes.HardwareAddr
 	if ctr.Config != nil {
 		if nwm := hostConfig.NetworkMode; nwm.IsBridge() || nwm.IsUserDefined() {
 			if epConf, ok := ctr.NetworkSettings.Networks[nwm.NetworkName()]; ok {
@@ -174,7 +174,7 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co
 		if ctr.State.Dead {
 			return inspectResponse, macAddress, nil
 		}
-		return nil, "", errdefs.System(errors.New("RWLayer of container " + ctr.ID + " is unexpectedly nil"))
+		return nil, nil, errdefs.System(errors.New("RWLayer of container " + ctr.ID + " is unexpectedly nil"))
 	}
 
 	graphDriverData, err := ctr.RWLayer.Metadata()
@@ -184,7 +184,7 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co
 			// have been removed; we can ignore errors.
 			return inspectResponse, macAddress, nil
 		}
-		return nil, "", errdefs.System(err)
+		return nil, nil, errdefs.System(err)
 	}
 
 	inspectResponse.GraphDriver.Data = graphDriverData

daemon/network.go

@@ -886,7 +886,7 @@ func buildEndpointResource(ep *libnetwork.Endpoint, info libnetwork.EndpointInfo
 		Name:       ep.Name(),
 	}
 	if iface := info.Iface(); iface != nil {
-		er.MacAddress = iface.MacAddress().String()
+		er.MacAddress = networktypes.HardwareAddr(iface.MacAddress())
 		er.IPv4Address = netiputil.Unmap(iface.Addr())
 		er.IPv6Address = iface.AddrIPv6()
 	}
@@ -955,12 +955,8 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e
 			createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(options.Generic{k: v}))
 		}
 
-		if epConfig.DesiredMacAddress != "" {
-			mac, err := net.ParseMAC(epConfig.DesiredMacAddress)
-			if err != nil {
-				return nil, err
-			}
-			genericOptions[netlabel.MacAddress] = mac
+		if len(epConfig.DesiredMacAddress) != 0 {
+			genericOptions[netlabel.MacAddress] = net.HardwareAddr(epConfig.DesiredMacAddress)
 		}
 	}
 
@@ -1174,8 +1170,8 @@ func buildEndpointInfo(networkSettings *network.Settings, n *libnetwork.Network,
 		return nil
 	}
 
-	if iface.MacAddress() != nil {
-		networkSettings.Networks[nwName].MacAddress = iface.MacAddress().String()
+	if mac := iface.MacAddress(); mac != nil {
+		networkSettings.Networks[nwName].MacAddress = networktypes.HardwareAddr(mac)
 	}
 
 	if iface.Address() != nil {

daemon/network/settings.go

@@ -28,7 +28,7 @@ type EndpointSettings struct {
 	IPAMOperational bool
 	// DesiredMacAddress is the configured value, it's copied from MacAddress (the
 	// API param field) when the container is created.
-	DesiredMacAddress string
+	DesiredMacAddress networktypes.HardwareAddr
 }
 
 // AttachmentStore stores the load balancer IP address for a network id.

daemon/server/router/container/backend.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/moby/go-archive"
 	"github.com/moby/moby/api/types/container"
+	"github.com/moby/moby/api/types/network"
 	containerpkg "github.com/moby/moby/v2/daemon/container"
 	"github.com/moby/moby/v2/daemon/internal/filters"
 	"github.com/moby/moby/v2/daemon/server/backend"
@@ -48,7 +49,7 @@ type stateBackend interface {
 // monitorBackend includes functions to implement to provide containers monitoring functionality.
 type monitorBackend interface {
 	ContainerChanges(ctx context.Context, name string) ([]archive.Change, error)
-	ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *container.InspectResponse, desiredMACAddress string, _ error)
+	ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *container.InspectResponse, desiredMACAddress network.HardwareAddr, _ error)
 	ContainerLogs(ctx context.Context, name string, config *backend.ContainerLogsOptions) (msgs <-chan *backend.LogMessage, tty bool, err error)
 	ContainerStats(ctx context.Context, name string, config *backend.ContainerStatsConfig) error
 	ContainerTop(name string, psArgs string) (*container.TopResponse, error)

daemon/server/router/container/container_routes.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"net/http"
 	"runtime"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -672,7 +673,7 @@ func (c *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
 			// Mac Address of the container.
 			//
 			// MacAddress field is deprecated since API v1.44. Use EndpointSettings.MacAddress instead.
-			MacAddress string `json:",omitempty"`
+			MacAddress network.HardwareAddr `json:",omitempty"`
 		}
 		_ = json.Unmarshal(requestBody.Bytes(), &legacyConfig)
 		if warn, err := handleMACAddressBC(hostConfig, networkingConfig, version, legacyConfig.MacAddress); err != nil {
@@ -745,14 +746,14 @@ func handleVolumeDriverBC(version string, hostConfig *container.HostConfig) (war
 // 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(hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, version string, deprecatedMacAddress string) (string, error) {
+func handleMACAddressBC(hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, version string, deprecatedMacAddress network.HardwareAddr) (string, error) {
 	// For older versions of the API, migrate the container-wide MAC address to EndpointsConfig.
 	if versions.LessThan(version, "1.44") {
-		if deprecatedMacAddress == "" {
+		if len(deprecatedMacAddress) == 0 {
 			// If a MAC address is supplied in EndpointsConfig, discard it because the old API
 			// would have ignored it.
 			for _, ep := range networkingConfig.EndpointsConfig {
-				ep.MacAddress = ""
+				ep.MacAddress = nil
 			}
 			return "", nil
 		}
@@ -769,7 +770,7 @@ func handleMACAddressBC(hostConfig *container.HostConfig, networkingConfig *netw
 	}
 
 	// The container-wide MacAddress parameter is deprecated and should now be specified in EndpointsConfig.
-	if deprecatedMacAddress == "" {
+	if len(deprecatedMacAddress) == 0 {
 		return "", nil
 	}
 
@@ -785,9 +786,9 @@ func handleMACAddressBC(hostConfig *container.HostConfig, networkingConfig *netw
 		}
 		// ep is the endpoint that needs the container-wide MAC address; migrate the address
 		// to it, or bail out if there's a mismatch.
-		if ep.MacAddress == "" {
+		if len(ep.MacAddress) == 0 {
 			ep.MacAddress = deprecatedMacAddress
-		} else if ep.MacAddress != deprecatedMacAddress {
+		} else if !slices.Equal(ep.MacAddress, deprecatedMacAddress) {
 			return "", errdefs.InvalidParameter(errors.New("the container-wide MAC address must match the endpoint-specific MAC address for the main network, or be left empty"))
 		}
 	}

daemon/server/router/container/container_routes_test.go

@@ -4,6 +4,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/moby/moby/api/types/container"
 	"github.com/moby/moby/api/types/network"
 	"github.com/moby/moby/v2/daemon/libnetwork/netlabel"
@@ -15,56 +16,56 @@ func TestHandleMACAddressBC(t *testing.T) {
 	testcases := []struct {
 		name                string
 		apiVersion          string
-		ctrWideMAC          string
+		ctrWideMAC          network.HardwareAddr
 		networkMode         container.NetworkMode
 		epConfig            map[string]*network.EndpointSettings
 		expEpWithCtrWideMAC string
 		expEpWithNoMAC      string
-		expCtrWideMAC       string
+		expCtrWideMAC       network.HardwareAddr
 		expWarning          string
 		expError            string
 	}{
 		{
 			name:                "old api ctr-wide mac mix id and name",
 			apiVersion:          "1.43",
-			ctrWideMAC:          "11:22:33:44:55:66",
+			ctrWideMAC:          network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 			networkMode:         "aNetId",
 			epConfig:            map[string]*network.EndpointSettings{"aNetName": {}},
 			expEpWithCtrWideMAC: "aNetName",
-			expCtrWideMAC:       "11:22:33:44:55:66",
+			expCtrWideMAC:       network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 		},
 		{
 			name:           "old api clear ep mac",
 			apiVersion:     "1.43",
 			networkMode:    "aNetId",
-			epConfig:       map[string]*network.EndpointSettings{"aNetName": {MacAddress: "11:22:33:44:55:66"}},
+			epConfig:       map[string]*network.EndpointSettings{"aNetName": {MacAddress: network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66}}},
 			expEpWithNoMAC: "aNetName",
 		},
 		{
 			name:          "old api no-network ctr-wide mac",
 			apiVersion:    "1.43",
 			networkMode:   "none",
-			ctrWideMAC:    "11:22:33:44:55:66",
+			ctrWideMAC:    network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 			expError:      "conflicting options: mac-address and the network mode",
-			expCtrWideMAC: "11:22:33:44:55:66",
+			expCtrWideMAC: network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 		},
 		{
 			name:                "old api create ep",
 			apiVersion:          "1.43",
 			networkMode:         "aNetId",
-			ctrWideMAC:          "11:22:33:44:55:66",
+			ctrWideMAC:          network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 			epConfig:            map[string]*network.EndpointSettings{},
 			expEpWithCtrWideMAC: "aNetId",
-			expCtrWideMAC:       "11:22:33:44:55:66",
+			expCtrWideMAC:       network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 		},
 		{
 			name:                "old api migrate ctr-wide mac",
 			apiVersion:          "1.43",
-			ctrWideMAC:          "11:22:33:44:55:66",
+			ctrWideMAC:          network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 			networkMode:         "aNetName",
 			epConfig:            map[string]*network.EndpointSettings{"aNetName": {}},
 			expEpWithCtrWideMAC: "aNetName",
-			expCtrWideMAC:       "11:22:33:44:55:66",
+			expCtrWideMAC:       network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 		},
 		{
 			name:        "new api no macs",
@@ -76,45 +77,45 @@ func TestHandleMACAddressBC(t *testing.T) {
 			name:        "new api ep specific mac",
 			apiVersion:  "1.44",
 			networkMode: "aNetName",
-			epConfig:    map[string]*network.EndpointSettings{"aNetName": {MacAddress: "11:22:33:44:55:66"}},
+			epConfig:    map[string]*network.EndpointSettings{"aNetName": {MacAddress: network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66}}},
 		},
 		{
 			name:                "new api migrate ctr-wide mac to new ep",
 			apiVersion:          "1.44",
-			ctrWideMAC:          "11:22:33:44:55:66",
+			ctrWideMAC:          network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 			networkMode:         "aNetName",
 			epConfig:            map[string]*network.EndpointSettings{},
 			expEpWithCtrWideMAC: "aNetName",
 			expWarning:          "The container-wide MacAddress field is now deprecated",
-			expCtrWideMAC:       "",
+			expCtrWideMAC:       nil,
 		},
 		{
 			name:                "new api migrate ctr-wide mac to existing ep",
 			apiVersion:          "1.44",
-			ctrWideMAC:          "11:22:33:44:55:66",
+			ctrWideMAC:          network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 			networkMode:         "aNetName",
 			epConfig:            map[string]*network.EndpointSettings{"aNetName": {}},
 			expEpWithCtrWideMAC: "aNetName",
 			expWarning:          "The container-wide MacAddress field is now deprecated",
-			expCtrWideMAC:       "",
+			expCtrWideMAC:       nil,
 		},
 		{
 			name:          "new api mode vs name mismatch",
 			apiVersion:    "1.44",
-			ctrWideMAC:    "11:22:33:44:55:66",
+			ctrWideMAC:    network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 			networkMode:   "aNetId",
 			epConfig:      map[string]*network.EndpointSettings{"aNetName": {}},
 			expError:      "unable to migrate container-wide MAC address to a specific network: HostConfig.NetworkMode must match the identity of a network in NetworkSettings.Networks",
-			expCtrWideMAC: "11:22:33:44:55:66",
+			expCtrWideMAC: network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 		},
 		{
 			name:          "new api mac mismatch",
 			apiVersion:    "1.44",
-			ctrWideMAC:    "11:22:33:44:55:66",
+			ctrWideMAC:    network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 			networkMode:   "aNetName",
-			epConfig:      map[string]*network.EndpointSettings{"aNetName": {MacAddress: "00:11:22:33:44:55"}},
+			epConfig:      map[string]*network.EndpointSettings{"aNetName": {MacAddress: network.HardwareAddr{0x00, 0x11, 0x22, 0x33, 0x44, 0x55}}},
 			expError:      "the container-wide MAC address must match the endpoint-specific MAC address",
-			expCtrWideMAC: "11:22:33:44:55:66",
+			expCtrWideMAC: network.HardwareAddr{0x11, 0x22, 0x33, 0x44, 0x55, 0x66},
 		},
 	}
 
@@ -146,11 +147,11 @@ func TestHandleMACAddressBC(t *testing.T) {
 			}
 			if tc.expEpWithCtrWideMAC != "" {
 				got := netCfg.EndpointsConfig[tc.expEpWithCtrWideMAC].MacAddress
-				assert.Check(t, is.Equal(got, tc.ctrWideMAC))
+				assert.Check(t, is.DeepEqual(got, tc.ctrWideMAC, cmpopts.EquateEmpty()))
 			}
 			if tc.expEpWithNoMAC != "" {
 				got := netCfg.EndpointsConfig[tc.expEpWithNoMAC].MacAddress
-				assert.Check(t, is.Equal(got, ""))
+				assert.Check(t, is.DeepEqual(got, network.HardwareAddr{}, cmpopts.EquateEmpty()))
 			}
 		})
 	}

daemon/server/router/container/inspect.go

@@ -63,7 +63,7 @@ func (c *containerRouter) getContainersByName(ctx context.Context, w http.Respon
 		//
 		// This was deprecated in API v1.44, but kept in place until
 		// API v1.52, which removed this entirely.
-		if desiredMACAddress != "" {
+		if len(desiredMACAddress) != 0 {
 			wrapOpts = append(wrapOpts, compat.WithExtraFields(map[string]any{
 				"Config": map[string]any{
 					"MacAddress": desiredMACAddress,