GitList

Browse code

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 <rob.murray@docker.com>

Rob Murray authored on 2024/01/30 03:38:05
Showing 2 changed files

daemon/inspect.go index 6d75fdd..9ec907a 100644
integration/networking/mac_addr_test.go index f79c20b..92c24d4 100644

daemon/inspect.go

History View file @ 8c64b85

@@ -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.
 			}
 		}
 	}

integration/networking/mac_addr_test.go

History View file @ 8c64b85

@@ -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))
                     +		})
                     +	}
                     +}

...	...	@@ -70,7 +70,6 @@ func (daemon *Daemon) ContainerInspectCurrent(ctx context.Context, name string,
70	70	if epConf.EndpointSettings != nil {
71	71	// We must make a copy of this pointer object otherwise it can race with other operations
72	72	apiNetworks[nwName] = epConf.EndpointSettings.Copy()
73		- apiNetworks[nwName].DesiredMacAddress = ""
74	73	}
75	74	}
76	75
...	...	@@ -163,8 +162,12 @@ func (daemon Daemon) getInspectData(daemonCfg config.Config, container *contai
163	163	// unversioned API endpoints.
164	164	if container.Config != nil && container.Config.MacAddress == "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
165	165	if nwm := hostConfig.NetworkMode; nwm.IsDefault() \|\| nwm.IsBridge() \|\| nwm.IsUserDefined() {
166		- if epConf, ok := container.NetworkSettings.Networks[nwm.NetworkName()]; ok {
167		- container.Config.MacAddress = epConf.MacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
	166	+ name := nwm.NetworkName()
	167	+ if nwm.IsDefault() {
	168	+ name = daemon.netController.Config().DefaultNetwork
	169	+ }
	170	+ if epConf, ok := container.NetworkSettings.Networks[name]; ok {
	171	+ container.Config.MacAddress = epConf.DesiredMacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
168	172	}
169	173	}
170	174	}