daemon: Daemon.getInspectData: move migration code to router

There also appeared to be duplication between daemon.getInspectData,
and the containerRouter.postContainersCreate methods, as both were
back-filling the field.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

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, error)
+	ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *container.InspectResponse, desiredMACAddress string, _ 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/cluster/executor/container/adapter.go

@@ -375,7 +375,7 @@ func (c *containerAdapter) start(ctx context.Context) error {
 }
 
 func (c *containerAdapter) inspect(ctx context.Context) (containertypes.InspectResponse, error) {
-	cs, err := c.backend.ContainerInspect(ctx, c.container.name(), backend.ContainerInspectOptions{})
+	cs, _, err := c.backend.ContainerInspect(ctx, c.container.name(), backend.ContainerInspectOptions{})
 	if ctx.Err() != nil {
 		return containertypes.InspectResponse{}, ctx.Err()
 	}

daemon/inspect.go

@@ -19,18 +19,18 @@ 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, error) {
+func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *containertypes.InspectResponse, desiredMACAddress string, _ error) {
 	ctr, err := daemon.GetContainer(name)
 	if err != nil {
-		return nil, err
+		return nil, "", err
 	}
 
 	ctr.Lock()
 
-	base, err := daemon.getInspectData(&daemon.config().Config, ctr)
+	base, desiredMACAddress, err := daemon.getInspectData(&daemon.config().Config, ctr)
 	if err != nil {
 		ctr.Unlock()
-		return nil, err
+		return 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, "", err
 		}
 		base.SizeRw = &sizeRw
 		base.SizeRootFs = &sizeRootFs
@@ -80,10 +80,10 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options
 	base.NetworkSettings = networkSettings
 	base.ImageManifestDescriptor = imageManifest
 
-	return base, nil
+	return base, desiredMACAddress, nil
 }
 
-func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Container) (*containertypes.InspectResponse, error) {
+func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Container) (_ *containertypes.InspectResponse, desiredMACAddress string, _ error) {
 	// make a copy to play with
 	hostConfig := *ctr.HostConfig
 
@@ -101,10 +101,11 @@ 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.
-	if ctr.Config != nil && ctr.Config.MacAddress == "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
+	var macAddress string
+	if ctr.Config != nil {
 		if nwm := hostConfig.NetworkMode; nwm.IsBridge() || nwm.IsUserDefined() {
 			if epConf, ok := ctr.NetworkSettings.Networks[nwm.NetworkName()]; ok {
-				ctr.Config.MacAddress = epConf.DesiredMacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
+				macAddress = epConf.DesiredMacAddress
 			}
 		}
 	}
@@ -163,7 +164,7 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co
 		}
 
 		// Additional information only applies to graphDrivers, so we're done.
-		return inspectResponse, nil
+		return inspectResponse, macAddress, nil
 	}
 
 	inspectResponse.GraphDriver = &storage.DriverData{
@@ -171,9 +172,9 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co
 	}
 	if ctr.RWLayer == nil {
 		if ctr.State.Dead {
-			return inspectResponse, nil
+			return inspectResponse, macAddress, nil
 		}
-		return nil, errdefs.System(errors.New("RWLayer of container " + ctr.ID + " is unexpectedly nil"))
+		return nil, "", errdefs.System(errors.New("RWLayer of container " + ctr.ID + " is unexpectedly nil"))
 	}
 
 	graphDriverData, err := ctr.RWLayer.Metadata()
@@ -181,13 +182,13 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co
 		if ctr.State.Dead {
 			// container is marked as Dead, and its graphDriver metadata may
 			// have been removed; we can ignore errors.
-			return inspectResponse, nil
+			return inspectResponse, macAddress, nil
 		}
-		return nil, errdefs.System(err)
+		return nil, "", errdefs.System(err)
 	}
 
 	inspectResponse.GraphDriver.Data = graphDriverData
-	return inspectResponse, nil
+	return inspectResponse, macAddress, nil
 }
 
 // ContainerExecInspect returns low-level information about the exec

daemon/inspect_test.go

@@ -26,10 +26,10 @@ func TestGetInspectData(t *testing.T) {
 	cfg := &configStore{}
 	d.configStore.Store(cfg)
 
-	_, err := d.getInspectData(&cfg.Config, c)
+	_, _, err := d.getInspectData(&cfg.Config, c)
 	assert.Check(t, is.ErrorContains(err, "RWLayer of container inspect-me is unexpectedly nil"))
 
 	c.State.Dead = true
-	_, err = d.getInspectData(&cfg.Config, c)
+	_, _, err = d.getInspectData(&cfg.Config, c)
 	assert.Check(t, err)
 }

daemon/server/router/container/backend.go

@@ -48,7 +48,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, error)
+	ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *container.InspectResponse, desiredMACAddress string, _ 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/inspect.go

@@ -15,7 +15,7 @@ import (
 
 // getContainersByName inspects container's configuration and serializes it as json.
 func (c *containerRouter) getContainersByName(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
-	ctr, err := c.backend.ContainerInspect(ctx, vars["name"], backend.ContainerInspectOptions{
+	ctr, desiredMACAddress, err := c.backend.ContainerInspect(ctx, vars["name"], backend.ContainerInspectOptions{
 		Size: httputils.BoolValue(r, "size"),
 	})
 	if err != nil {
@@ -39,7 +39,7 @@ func (c *containerRouter) getContainersByName(ctx context.Context, w http.Respon
 	if versions.LessThan(version, "1.52") {
 		if bridgeNw := ctr.NetworkSettings.Networks["bridge"]; bridgeNw != nil {
 			// Old API versions showed the bridge's configuration as top-level
-			// fields in "NetworkConfig".
+			// fields in "NetworkSettings".
 			//
 			// This was deprecated in API v1.44, but kept in place until
 			// API v1.52, which removes this entirely.
@@ -56,6 +56,19 @@ func (c *containerRouter) getContainersByName(ctx context.Context, w http.Respon
 				},
 			}))
 		}
+
+		// Migrate the container's main / default network's MacAddress to
+		// the Config.MacAddress field for older API versions (< 1.44).
+		//
+		// This was deprecated in API v1.44, but kept in place until
+		// API v1.52, which removed this entirely.
+		if desiredMACAddress != "" {
+			wrapOpts = append(wrapOpts, compat.WithExtraFields(map[string]any{
+				"Config": map[string]any{
+					"MacAddress": desiredMACAddress,
+				},
+			}))
+		}
 	}
 
 	return httputils.WriteJSON(w, http.StatusOK, compat.Wrap(ctr, wrapOpts...))