From 8c64b85fb90c9e518aee6da3e8adfd0bfeabf255 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Mon, 29 Jan 2024 18:38:05 +0000 Subject: [PATCH] No inspect 'Config.MacAddress' unless configured. Do not set 'Config.MacAddress' in inspect output unless the MAC address is configured. Also, make sure it is filled in for a configured address on the default network before the container is started (by translating the network name from 'default' to 'config' so that the address lookup works). Signed-off-by: Rob Murray --- daemon/inspect.go | 9 ++- integration/networking/mac_addr_test.go | 96 +++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/daemon/inspect.go b/daemon/inspect.go index 6d75fdd11ba55..9ec907a5ba902 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -70,7 +70,6 @@ func (daemon *Daemon) ContainerInspectCurrent(ctx context.Context, name string, if epConf.EndpointSettings != nil { // We must make a copy of this pointer object otherwise it can race with other operations apiNetworks[nwName] = epConf.EndpointSettings.Copy() - apiNetworks[nwName].DesiredMacAddress = "" } } @@ -163,8 +162,12 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, container *contai // 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. + name := nwm.NetworkName() + if nwm.IsDefault() { + name = daemon.netController.Config().DefaultNetwork + } + if epConf, ok := container.NetworkSettings.Networks[name]; ok { + container.Config.MacAddress = epConf.DesiredMacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. } } } diff --git a/integration/networking/mac_addr_test.go b/integration/networking/mac_addr_test.go index f79c20b118c1e..92c24d4db48c0 100644 --- a/integration/networking/mac_addr_test.go +++ b/integration/networking/mac_addr_test.go @@ -4,9 +4,11 @@ import ( "testing" containertypes "github.com/docker/docker/api/types/container" + "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/network" "github.com/docker/docker/libnetwork/drivers/bridge" + "github.com/docker/docker/testutil" "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" @@ -135,3 +137,97 @@ func TestCfgdMACAddrOnRestart(t *testing.T) { d.Restart(t) startAndCheck() } + +// Regression test for https://github.com/moby/moby/issues/47228 - check that a +// generated MAC address is not included in the Config section of 'inspect' +// output, but a configured address is. +func TestInspectCfgdMAC(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows") + + ctx := setupTest(t) + + d := daemon.New(t) + d.StartWithBusybox(ctx, t) + defer d.Stop(t) + + testcases := []struct { + name string + desiredMAC string + netName string + ctrWide bool + }{ + { + name: "generated address default bridge", + netName: "bridge", + }, + { + name: "configured address default bridge", + desiredMAC: "02:42:ac:11:00:42", + netName: "bridge", + }, + { + name: "generated address custom bridge", + netName: "testnet", + }, + { + name: "configured address custom bridge", + desiredMAC: "02:42:ac:11:00:42", + netName: "testnet", + }, + { + name: "ctr-wide address default bridge", + desiredMAC: "02:42:ac:11:00:42", + netName: "bridge", + ctrWide: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ctx := testutil.StartSpan(ctx, t) + + var copts []client.Opt + if tc.ctrWide { + copts = append(copts, client.WithVersion("1.43")) + } + c := d.NewClientT(t, copts...) + defer c.Close() + + if tc.netName != "bridge" { + const netName = "inspectcfgmac" + network.CreateNoError(ctx, t, c, netName, + network.WithDriver("bridge"), + network.WithOption(bridge.BridgeName, netName)) + defer network.RemoveNoError(ctx, t, c, netName) + } + + const ctrName = "ctr" + opts := []func(*container.TestContainerConfig){ + container.WithName(ctrName), + container.WithCmd("top"), + container.WithImage("busybox:latest"), + } + // Don't specify the network name for the bridge network, because that + // exercises a different code path (the network name isn't set until the + // container starts, until then it's "default"). + if tc.netName != "bridge" { + opts = append(opts, container.WithNetworkMode(tc.netName)) + } + if tc.desiredMAC != "" { + if tc.ctrWide { + opts = append(opts, container.WithContainerWideMacAddress(tc.desiredMAC)) + } else { + opts = append(opts, container.WithMacAddress(tc.netName, tc.desiredMAC)) + } + } + id := container.Create(ctx, t, c, opts...) + defer c.ContainerRemove(ctx, id, containertypes.RemoveOptions{ + Force: true, + }) + + inspect := container.Inspect(ctx, t, c, ctrName) + configMAC := inspect.Config.MacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. + assert.Check(t, is.DeepEqual(configMAC, tc.desiredMAC)) + }) + } +}