From 19f4c27d81c2268d29f7f309b0b5aded9aeae2bf Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Thu, 30 Oct 2025 15:57:47 -0400 Subject: [PATCH] api/t/network: represent MAC addrs as byte slices Make invalid states unrepresentable by moving away from stringly-typed MAC address values in API structs. As go.dev/issue/29678 has not yet been implemented, provide our own HardwareAddr byte-slice type which implements TextMarshaler and TextUnmarshaler to retain compatibility with the API wire format. When stdlib's net.HardwareAddr type implements TextMarshaler and TextUnmarshaler and GODEBUG=netmarshal becomes the default, we should be able to make the type a straight alias for stdlib net.HardwareAddr as a non-breaking change. Signed-off-by: Cory Snider --- api/swagger.yaml | 4 + api/types/network/endpoint.go | 2 +- api/types/network/endpoint_resource.go | 2 +- api/types/network/hwaddr.go | 37 ++++++++ api/types/network/hwaddr_test.go | 87 +++++++++++++++++++ daemon/cluster/executor/backend.go | 2 +- daemon/container_operations.go | 10 +-- daemon/inspect.go | 16 ++-- daemon/network.go | 14 ++- daemon/network/settings.go | 2 +- daemon/server/router/container/backend.go | 3 +- .../router/container/container_routes.go | 15 ++-- .../router/container/container_routes_test.go | 47 +++++----- daemon/server/router/container/inspect.go | 2 +- integration/container/run_linux_test.go | 3 +- integration/internal/container/ops.go | 7 +- integration/networking/bridge_linux_test.go | 7 +- integration/networking/mac_addr_test.go | 9 +- .../moby/moby/api/types/network/endpoint.go | 2 +- .../api/types/network/endpoint_resource.go | 2 +- .../moby/moby/api/types/network/hwaddr.go | 37 ++++++++ 21 files changed, 235 insertions(+), 75 deletions(-) create mode 100644 api/types/network/hwaddr.go create mode 100644 api/types/network/hwaddr_test.go create mode 100644 vendor/github.com/moby/moby/api/types/network/hwaddr.go diff --git a/api/swagger.yaml b/api/swagger.yaml index 7c0d9a0b7d..1e8644a5c6 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -2628,6 +2628,8 @@ definitions: type: "string" x-omitempty: false example: "02:42:ac:13:00:02" + x-go-type: + type: HardwareAddr IPv4Address: type: "string" x-omitempty: false @@ -2914,6 +2916,8 @@ definitions: MAC address for the endpoint on this network. The network driver might ignore this parameter. type: "string" example: "02:42:ac:11:00:04" + x-go-type: + type: HardwareAddr Aliases: type: "array" items: diff --git a/api/types/network/endpoint.go b/api/types/network/endpoint.go index ee98fab01f..cd39c579f0 100644 --- a/api/types/network/endpoint.go +++ b/api/types/network/endpoint.go @@ -30,7 +30,7 @@ type EndpointSettings struct { // MacAddress may be used to specify a MAC address when the container is created. // Once the container is running, it becomes operational data (it may contain a // generated address). - MacAddress string + MacAddress HardwareAddr IPPrefixLen int IPv6Gateway netip.Addr GlobalIPv6Address netip.Addr diff --git a/api/types/network/endpoint_resource.go b/api/types/network/endpoint_resource.go index 6ff25b1bb6..bf493ad5dd 100644 --- a/api/types/network/endpoint_resource.go +++ b/api/types/network/endpoint_resource.go @@ -24,7 +24,7 @@ type EndpointResource struct { // mac address // Example: 02:42:ac:13:00:02 - MacAddress string `json:"MacAddress"` + MacAddress HardwareAddr `json:"MacAddress"` // IPv4 address // Example: 172.19.0.2/16 diff --git a/api/types/network/hwaddr.go b/api/types/network/hwaddr.go new file mode 100644 index 0000000000..9d0c2b4b62 --- /dev/null +++ b/api/types/network/hwaddr.go @@ -0,0 +1,37 @@ +package network + +import ( + "encoding" + "fmt" + "net" +) + +// A HardwareAddr represents a physical hardware address. +// It implements [encoding.TextMarshaler] and [encoding.TextUnmarshaler] +// in the absence of go.dev/issue/29678. +type HardwareAddr net.HardwareAddr + +var _ encoding.TextMarshaler = (HardwareAddr)(nil) +var _ encoding.TextUnmarshaler = (*HardwareAddr)(nil) +var _ fmt.Stringer = (HardwareAddr)(nil) + +func (m *HardwareAddr) UnmarshalText(text []byte) error { + if len(text) == 0 { + *m = nil + return nil + } + hw, err := net.ParseMAC(string(text)) + if err != nil { + return err + } + *m = HardwareAddr(hw) + return nil +} + +func (m HardwareAddr) MarshalText() ([]byte, error) { + return []byte(net.HardwareAddr(m).String()), nil +} + +func (m HardwareAddr) String() string { + return net.HardwareAddr(m).String() +} diff --git a/api/types/network/hwaddr_test.go b/api/types/network/hwaddr_test.go new file mode 100644 index 0000000000..303804d10c --- /dev/null +++ b/api/types/network/hwaddr_test.go @@ -0,0 +1,87 @@ +package network_test + +import ( + "encoding/json" + "testing" + + "github.com/moby/moby/api/types/network" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +func TestHardwareAddr_UnmarshalText(t *testing.T) { + cases := []struct { + in string + out network.HardwareAddr + err string + }{ + {"", nil, ""}, + {"00:11:22:33:44:55", network.HardwareAddr{0x00, 0x11, 0x22, 0x33, 0x44, 0x55}, ""}, + {"xx:xx:xx:xx:xx:xx", network.HardwareAddr{0xde, 0xad, 0xbe, 0xef}, "invalid MAC address"}, + } + for _, c := range cases { + a := network.HardwareAddr{0xde, 0xad, 0xbe, 0xef} + err := a.UnmarshalText([]byte(c.in)) + if (c.err == "") != (err == nil) { + t.Errorf("UnmarshalText(%q) error = %v, want %v", c.in, err, c.err) + } + assert.Check(t, is.DeepEqual(a, c.out), "UnmarshalText(%q)", c.in) + } +} + +func TestHardwareAddr_MarshalText(t *testing.T) { + cases := []struct { + in network.HardwareAddr + out string + }{ + {nil, ""}, + {network.HardwareAddr{}, ""}, + {network.HardwareAddr{0x00, 0x11, 0x22, 0x33, 0x44, 0x55}, "00:11:22:33:44:55"}, + } + for _, c := range cases { + out, err := c.in.MarshalText() + assert.Check(t, err, "MarshalText(%v)", c.in) + assert.Check(t, is.Equal(string(out), c.out), "MarshalText(%v)", c.in) + } +} + +func TestHardwareAddr_MarshalJSON(t *testing.T) { + cases := []struct { + in network.HardwareAddr + out string + }{ + {nil, `{"mac":""}`}, + {network.HardwareAddr{}, `{"mac":""}`}, + {network.HardwareAddr{0x00, 0x11, 0x22, 0x33, 0x44, 0x55}, `{"mac":"00:11:22:33:44:55"}`}, + } + for _, c := range cases { + s := struct { + Mac network.HardwareAddr `json:"mac"` + }{c.in} + got, err := json.Marshal(s) + assert.Check(t, err, "json.Marshal(network.HardwareAddr(%v))", c.in) + assert.Check(t, is.Equal(string(got), c.out), "json.Marshal(network.HardwareAddr(%v))", c.in) + } +} + +func TestHardwareAddr_UnmarshalJSON(t *testing.T) { + cases := []struct { + in string + out network.HardwareAddr + err string + }{ + {`{"mac":""}`, nil, ""}, + {`{"mac":"00:11:22:33:44:55"}`, network.HardwareAddr{0x00, 0x11, 0x22, 0x33, 0x44, 0x55}, ""}, + {`{"mac":"xx:xx:xx:xx:xx:xx"}`, network.HardwareAddr{0xde, 0xad, 0xbe, 0xef}, "invalid MAC address"}, + } + for _, c := range cases { + s := struct { + Mac network.HardwareAddr `json:"mac"` + }{network.HardwareAddr{0xde, 0xad, 0xbe, 0xef}} + err := json.Unmarshal([]byte(c.in), &s) + if (c.err == "") != (err == nil) { + t.Errorf("json.Unmarshal(%q) error = %v, want %v", c.in, err, c.err) + } + assert.Check(t, is.DeepEqual(s.Mac, c.out), "json.Unmarshal(%q)", c.in) + } +} diff --git a/daemon/cluster/executor/backend.go b/daemon/cluster/executor/backend.go index 428eb7f568..91b3da31cc 100644 --- a/daemon/cluster/executor/backend.go +++ b/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 diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 30b862ef30..df52d3b4f5 100644 --- a/daemon/container_operations.go +++ b/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 } diff --git a/daemon/inspect.go b/daemon/inspect.go index 835f22f28f..41dd00cdc6 100644 --- a/daemon/inspect.go +++ b/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 diff --git a/daemon/network.go b/daemon/network.go index 531b41a2e3..7851bb3b68 100644 --- a/daemon/network.go +++ b/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 { diff --git a/daemon/network/settings.go b/daemon/network/settings.go index 577a53c083..c8be7baab6 100644 --- a/daemon/network/settings.go +++ b/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. diff --git a/daemon/server/router/container/backend.go b/daemon/server/router/container/backend.go index ffba42989c..521da9c1a3 100644 --- a/daemon/server/router/container/backend.go +++ b/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) diff --git a/daemon/server/router/container/container_routes.go b/daemon/server/router/container/container_routes.go index 38bfc157f1..015d3814c6 100644 --- a/daemon/server/router/container/container_routes.go +++ b/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")) } } diff --git a/daemon/server/router/container/container_routes_test.go b/daemon/server/router/container/container_routes_test.go index d95be74777..20ee31768a 100644 --- a/daemon/server/router/container/container_routes_test.go +++ b/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())) } }) } diff --git a/daemon/server/router/container/inspect.go b/daemon/server/router/container/inspect.go index a22e017364..15ee82b5db 100644 --- a/daemon/server/router/container/inspect.go +++ b/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, diff --git a/integration/container/run_linux_test.go b/integration/container/run_linux_test.go index 065792c477..90891f32ae 100644 --- a/integration/container/run_linux_test.go +++ b/integration/container/run_linux_test.go @@ -16,6 +16,7 @@ import ( "github.com/docker/go-units" "github.com/moby/moby/api/pkg/stdcopy" containertypes "github.com/moby/moby/api/types/container" + networktypes "github.com/moby/moby/api/types/network" "github.com/moby/moby/client" "github.com/moby/moby/v2/integration/internal/container" net "github.com/moby/moby/v2/integration/internal/network" @@ -295,7 +296,7 @@ func TestMacAddressIsAppliedToMainNetworkWithShortID(t *testing.T) { c := container.Inspect(ctx, t, apiClient, cid) assert.Assert(t, c.NetworkSettings.Networks["testnet"] != nil) - assert.Equal(t, c.NetworkSettings.Networks["testnet"].MacAddress, "02:42:08:26:a9:55") + assert.DeepEqual(t, c.NetworkSettings.Networks["testnet"].MacAddress, networktypes.HardwareAddr{0x02, 0x42, 0x08, 0x26, 0xa9, 0x55}) } func TestStaticIPOutsideSubpool(t *testing.T) { diff --git a/integration/internal/container/ops.go b/integration/internal/container/ops.go index a607fae93b..7cad36de19 100644 --- a/integration/internal/container/ops.go +++ b/integration/internal/container/ops.go @@ -2,6 +2,7 @@ package container import ( "maps" + "net" "net/netip" "slices" "strings" @@ -151,6 +152,10 @@ func WithTmpfs(targetAndOpts string) func(config *TestContainerConfig) { } func WithMacAddress(networkName, mac string) func(config *TestContainerConfig) { + maddr, err := net.ParseMAC(mac) + if err != nil { + panic(err) + } return func(c *TestContainerConfig) { if c.NetworkingConfig.EndpointsConfig == nil { c.NetworkingConfig.EndpointsConfig = map[string]*network.EndpointSettings{} @@ -158,7 +163,7 @@ func WithMacAddress(networkName, mac string) func(config *TestContainerConfig) { if v, ok := c.NetworkingConfig.EndpointsConfig[networkName]; !ok || v == nil { c.NetworkingConfig.EndpointsConfig[networkName] = &network.EndpointSettings{} } - c.NetworkingConfig.EndpointsConfig[networkName].MacAddress = mac + c.NetworkingConfig.EndpointsConfig[networkName].MacAddress = network.HardwareAddr(maddr) } } diff --git a/integration/networking/bridge_linux_test.go b/integration/networking/bridge_linux_test.go index ab0f699fe1..a4e291a993 100644 --- a/integration/networking/bridge_linux_test.go +++ b/integration/networking/bridge_linux_test.go @@ -1816,7 +1816,7 @@ func TestAdvertiseAddresses(t *testing.T) { // The original defer will stop ctr2Id. ctr2NewMAC := container.Inspect(ctx, t, c, ctr2Id).NetworkSettings.Networks[netName].MacAddress - assert.Check(t, ctr2OrigMAC != ctr2NewMAC, "expected restarted ctr2 to have a different MAC address") + assert.Check(t, !slices.Equal(ctr2OrigMAC, ctr2NewMAC), "expected restarted ctr2 to have a different MAC address") ctr1Neighs = container.ExecT(ctx, t, c, ctr1Id, []string{"ip", "neigh", "show"}) assert.Assert(t, is.Equal(ctr1Neighs.ExitCode, 0)) @@ -1843,9 +1843,6 @@ func TestAdvertiseAddresses(t *testing.T) { // Check ARP/NA messages received for ctr2's new address (all unsolicited). - ctr2NewHwAddr, err := net.ParseMAC(ctr2NewMAC) - assert.NilError(t, err) - checkPkts := func(pktDesc string, pkts []network.TimestampedPkt, matchIP netip.Addr, unpack func(pkt network.TimestampedPkt) (sh net.HardwareAddr, sp netip.Addr, err error)) { t.Helper() var count int @@ -1860,7 +1857,7 @@ func TestAdvertiseAddresses(t *testing.T) { continue } t.Logf("%s %d: %s '%s' is at '%s'", pktDesc, i+1, p.ReceivedAt.Format("15:04:05.000"), pa, ha) - if pa != matchIP || slices.Compare(ha, ctr2NewHwAddr) != 0 { + if pa != matchIP || slices.Compare(ha, net.HardwareAddr(ctr2NewMAC)) != 0 { continue } count++ diff --git a/integration/networking/mac_addr_test.go b/integration/networking/mac_addr_test.go index c68370a6bb..5f592b2fe6 100644 --- a/integration/networking/mac_addr_test.go +++ b/integration/networking/mac_addr_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "net/http" "net/netip" + "slices" "testing" containertypes "github.com/moby/moby/api/types/container" @@ -82,7 +83,7 @@ func TestMACAddrOnRestart(t *testing.T) { ctr2Inspect := container.Inspect(ctx, t, c, ctr2Name) ctr2MAC := ctr2Inspect.NetworkSettings.Networks[netName].MacAddress - assert.Check(t, ctr1MAC != ctr2MAC, + assert.Check(t, !slices.Equal(ctr1MAC, ctr2MAC), "expected containers to have different MAC addresses; got %q for both", ctr1MAC) } @@ -120,7 +121,7 @@ func TestCfgdMACAddrOnRestart(t *testing.T) { inspect := container.Inspect(ctx, t, c, ctr1Name) gotMAC := inspect.NetworkSettings.Networks[netName].MacAddress - assert.Check(t, is.Equal(wantMAC, gotMAC)) + assert.Check(t, is.Equal(wantMAC, gotMAC.String())) startAndCheck := func() { t.Helper() @@ -128,7 +129,7 @@ func TestCfgdMACAddrOnRestart(t *testing.T) { assert.Assert(t, is.Nil(err)) inspect = container.Inspect(ctx, t, c, ctr1Name) gotMAC = inspect.NetworkSettings.Networks[netName].MacAddress - assert.Check(t, is.Equal(wantMAC, gotMAC)) + assert.Check(t, is.Equal(wantMAC, gotMAC.String())) } // Restart the container, check that the MAC address is restored. @@ -301,7 +302,7 @@ func TestWatchtowerCreate(t *testing.T) { inspect := container.Inspect(ctx, t, c, ctrName) netSettings := inspect.NetworkSettings.Networks[netName] assert.Check(t, is.Equal(netSettings.IPAddress, netip.MustParseAddr(ctrIP))) - assert.Check(t, is.Equal(netSettings.MacAddress, ctrMAC)) + assert.Check(t, is.Equal(netSettings.MacAddress.String(), ctrMAC)) } type legacyCreateRequest struct { diff --git a/vendor/github.com/moby/moby/api/types/network/endpoint.go b/vendor/github.com/moby/moby/api/types/network/endpoint.go index ee98fab01f..cd39c579f0 100644 --- a/vendor/github.com/moby/moby/api/types/network/endpoint.go +++ b/vendor/github.com/moby/moby/api/types/network/endpoint.go @@ -30,7 +30,7 @@ type EndpointSettings struct { // MacAddress may be used to specify a MAC address when the container is created. // Once the container is running, it becomes operational data (it may contain a // generated address). - MacAddress string + MacAddress HardwareAddr IPPrefixLen int IPv6Gateway netip.Addr GlobalIPv6Address netip.Addr diff --git a/vendor/github.com/moby/moby/api/types/network/endpoint_resource.go b/vendor/github.com/moby/moby/api/types/network/endpoint_resource.go index 6ff25b1bb6..bf493ad5dd 100644 --- a/vendor/github.com/moby/moby/api/types/network/endpoint_resource.go +++ b/vendor/github.com/moby/moby/api/types/network/endpoint_resource.go @@ -24,7 +24,7 @@ type EndpointResource struct { // mac address // Example: 02:42:ac:13:00:02 - MacAddress string `json:"MacAddress"` + MacAddress HardwareAddr `json:"MacAddress"` // IPv4 address // Example: 172.19.0.2/16 diff --git a/vendor/github.com/moby/moby/api/types/network/hwaddr.go b/vendor/github.com/moby/moby/api/types/network/hwaddr.go new file mode 100644 index 0000000000..9d0c2b4b62 --- /dev/null +++ b/vendor/github.com/moby/moby/api/types/network/hwaddr.go @@ -0,0 +1,37 @@ +package network + +import ( + "encoding" + "fmt" + "net" +) + +// A HardwareAddr represents a physical hardware address. +// It implements [encoding.TextMarshaler] and [encoding.TextUnmarshaler] +// in the absence of go.dev/issue/29678. +type HardwareAddr net.HardwareAddr + +var _ encoding.TextMarshaler = (HardwareAddr)(nil) +var _ encoding.TextUnmarshaler = (*HardwareAddr)(nil) +var _ fmt.Stringer = (HardwareAddr)(nil) + +func (m *HardwareAddr) UnmarshalText(text []byte) error { + if len(text) == 0 { + *m = nil + return nil + } + hw, err := net.ParseMAC(string(text)) + if err != nil { + return err + } + *m = HardwareAddr(hw) + return nil +} + +func (m HardwareAddr) MarshalText() ([]byte, error) { + return []byte(net.HardwareAddr(m).String()), nil +} + +func (m HardwareAddr) String() string { + return net.HardwareAddr(m).String() +}