Skip to content

Commit

Permalink
No inspect 'Config.MacAddress' unless configured.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
robmry committed Jan 31, 2024
1 parent d0202bb commit 4ace85d
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 12 deletions.
6 changes: 3 additions & 3 deletions api/types/network/endpoint.go
Expand Up @@ -14,9 +14,9 @@ type EndpointSettings struct {
IPAMConfig *EndpointIPAMConfig
Links []string
Aliases []string // Aliases holds the list of extra, user-specified DNS names for this endpoint.
// MacAddress may contain a configured value from the API. Its value is copied
// into DesiredMacAddress when the container is created, after which MacAddress
// becomes operational data (it may contain a generated address).
// 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
// Operational data
NetworkID string
Expand Down
7 changes: 3 additions & 4 deletions daemon/container_operations.go
Expand Up @@ -695,10 +695,9 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.
return nil
}
if endpointConfig == nil {
endpointConfig = &network.EndpointSettings{}
}
if endpointConfig.EndpointSettings == nil {
endpointConfig.EndpointSettings = &networktypes.EndpointSettings{}
endpointConfig = &network.EndpointSettings{
EndpointSettings: &networktypes.EndpointSettings{},
}
}

n, nwCfg, err := daemon.findAndAttachNetwork(container, idOrName, endpointConfig.EndpointSettings)
Expand Down
9 changes: 6 additions & 3 deletions daemon/inspect.go
Expand Up @@ -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 = ""
}
}

Expand Down Expand Up @@ -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.
}
}
}
Expand Down
101 changes: 99 additions & 2 deletions integration/networking/mac_addr_test.go
Expand Up @@ -4,8 +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"
Expand Down Expand Up @@ -35,7 +38,7 @@ func TestMACAddrOnRestart(t *testing.T) {
const netName = "testmacaddrs"
network.CreateNoError(ctx, t, c, netName,
network.WithDriver("bridge"),
network.WithOption("com.docker.network.bridge.name", netName))
network.WithOption(bridge.BridgeName, netName))
defer network.RemoveNoError(ctx, t, c, netName)

const ctr1Name = "ctr1"
Expand Down Expand Up @@ -95,7 +98,7 @@ func TestCfgdMACAddrOnRestart(t *testing.T) {
const netName = "testcfgmacaddr"
network.CreateNoError(ctx, t, c, netName,
network.WithDriver("bridge"),
network.WithOption("com.docker.network.bridge.name", netName))
network.WithOption(bridge.BridgeName, netName))
defer network.RemoveNoError(ctx, t, c, netName)

const wantMAC = "02:42:ac:11:00:42"
Expand Down Expand Up @@ -134,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))
})
}
}

0 comments on commit 4ace85d

Please sign in to comment.