From 98323ac114703f61d09a9e46c17201f5d1419e9a Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Fri, 15 Sep 2023 17:51:39 +0200 Subject: [PATCH 1/5] integration/container: target v1.43 in a soon-to-break test Signed-off-by: Albin Kerouanton --- integration/container/run_linux_test.go | 6 ++++-- integration/internal/container/ops.go | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/integration/container/run_linux_test.go b/integration/container/run_linux_test.go index aedd8ef284904..079fbdf86ca01 100644 --- a/integration/container/run_linux_test.go +++ b/integration/container/run_linux_test.go @@ -12,6 +12,7 @@ import ( containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/versions" + "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" net "github.com/docker/docker/integration/internal/network" "github.com/docker/docker/pkg/stdcopy" @@ -286,7 +287,8 @@ func TestMacAddressIsAppliedToMainNetworkWithShortID(t *testing.T) { d.StartWithBusybox(ctx, t) defer d.Stop(t) - apiClient := d.NewClientT(t) + apiClient, err := client.NewClientWithOpts(client.FromEnv, client.WithVersion("1.43")) + assert.NilError(t, err) n := net.CreateNoError(ctx, t, apiClient, "testnet", net.WithIPAM("192.168.101.0/24", "192.168.101.1")) @@ -295,7 +297,7 @@ func TestMacAddressIsAppliedToMainNetworkWithShortID(t *testing.T) { container.WithCmd("/bin/sleep", "infinity"), container.WithStopSignal("SIGKILL"), container.WithNetworkMode(n[:10]), - container.WithMacAddress("02:42:08:26:a9:55")) + container.WithContainerWideMacAddress("02:42:08:26:a9:55")) defer container.Remove(ctx, t, apiClient, cid, containertypes.RemoveOptions{Force: true}) c := container.Inspect(ctx, t, apiClient, cid) diff --git a/integration/internal/container/ops.go b/integration/internal/container/ops.go index 89f48768c0a44..03a7365c6a28d 100644 --- a/integration/internal/container/ops.go +++ b/integration/internal/container/ops.go @@ -298,7 +298,7 @@ func WithStopSignal(stopSignal string) func(c *TestContainerConfig) { } } -func WithMacAddress(address string) func(c *TestContainerConfig) { +func WithContainerWideMacAddress(address string) func(c *TestContainerConfig) { return func(c *TestContainerConfig) { c.Config.MacAddress = address } From 052562ffd5b2a5b4e6805e735243135886053e3c Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Tue, 4 Jul 2023 23:48:58 +0200 Subject: [PATCH 2/5] api: Add a field MacAddress to EndpointSettings Prior to this commit, only container.Config had a MacAddress field and it's used only for the first network the container connects to. It's a relic of old times where custom networks were not supported. Signed-off-by: Albin Kerouanton --- api/swagger.yaml | 12 ++++---- api/types/network/endpoint.go | 2 +- client/container_create.go | 16 ++++++++++ daemon/container_operations.go | 1 - daemon/network.go | 19 +++++++++--- docs/api/version-history.md | 2 ++ integration/container/create_test.go | 42 +++++++++++++++++++++++++++ integration/internal/container/ops.go | 12 ++++++++ 8 files changed, 95 insertions(+), 11 deletions(-) diff --git a/api/swagger.yaml b/api/swagger.yaml index 3c3d83fb9abce..c5725d73c4188 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -1381,6 +1381,7 @@ definitions: LinkLocalIPs: - "169.254.34.68" - "fe80::3468" + MacAddress: "02:42:ac:12:05:02" Links: - "container_1" - "container_2" @@ -2455,6 +2456,11 @@ definitions: example: - "container_1" - "container_2" + MacAddress: + description: | + MAC address for the endpoint on this network. The network driver might ignore this parameter. + type: "string" + example: "02:42:ac:11:00:04" Aliases: type: "array" items: @@ -2505,11 +2511,6 @@ definitions: type: "integer" format: "int64" example: 64 - MacAddress: - description: | - MAC address for the endpoint on this network. - type: "string" - example: "02:42:ac:11:00:04" DriverOpts: description: | DriverOpts is a mapping of driver options and values. These options @@ -10130,6 +10131,7 @@ paths: IPAMConfig: IPv4Address: "172.24.56.89" IPv6Address: "2001:db8::5689" + MacAddress: "02:42:ac:12:05:02" tags: ["Network"] /networks/{id}/disconnect: diff --git a/api/types/network/endpoint.go b/api/types/network/endpoint.go index a119dca909754..d7d835f6d593f 100644 --- a/api/types/network/endpoint.go +++ b/api/types/network/endpoint.go @@ -14,6 +14,7 @@ type EndpointSettings struct { IPAMConfig *EndpointIPAMConfig Links []string Aliases []string + MacAddress string // Operational data NetworkID string EndpointID string @@ -23,7 +24,6 @@ type EndpointSettings struct { IPv6Gateway string GlobalIPv6Address string GlobalIPv6PrefixLen int - MacAddress string DriverOpts map[string]string } diff --git a/client/container_create.go b/client/container_create.go index 7a95461c88a45..ce9a7a404d013 100644 --- a/client/container_create.go +++ b/client/container_create.go @@ -39,6 +39,9 @@ func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config if err := cli.NewVersionError(ctx, "1.44", "specify health-check start interval"); config != nil && config.Healthcheck != nil && config.Healthcheck.StartInterval != 0 && err != nil { return response, err } + if err := cli.NewVersionError("1.44", "specify mac-address per network"); hasEndpointSpecificMacAddress(networkingConfig) && err != nil { + return response, err + } if hostConfig != nil { if versions.LessThan(cli.ClientVersion(), "1.25") { @@ -91,3 +94,16 @@ func formatPlatform(platform *ocispec.Platform) string { } return path.Join(platform.OS, platform.Architecture, platform.Variant) } + +// hasEndpointSpecificMacAddress checks whether one of the endpoint in networkingConfig has a MacAddress defined. +func hasEndpointSpecificMacAddress(networkingConfig *network.NetworkingConfig) bool { + if networkingConfig == nil { + return false + } + for _, endpoint := range networkingConfig.EndpointsConfig { + if endpoint.MacAddress != "" { + return true + } + } + return false +} diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 652f347689a1a..2c5e04888ec2d 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -628,7 +628,6 @@ func cleanOperationalData(es *network.EndpointSettings) { es.IPv6Gateway = "" es.GlobalIPv6Address = "" es.GlobalIPv6PrefixLen = 0 - es.MacAddress = "" if es.IPAMOperational { es.IPAMConfig = nil } diff --git a/daemon/network.go b/daemon/network.go index 5e467d7ed4ce6..a49ded0f8b2d2 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -788,6 +788,7 @@ func (daemon *Daemon) clearAttachableNetworks() { // buildCreateEndpointOptions builds endpoint options from a given network. func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, epConfig *network.EndpointSettings, sb *libnetwork.Sandbox, daemonDNS []string) ([]libnetwork.EndpointOption, error) { var createOptions []libnetwork.EndpointOption + var genericOptions = make(options.Generic) nwName := n.Name() defaultNetName := runconfig.DefaultDaemonNetworkMode().NetworkName() @@ -825,6 +826,14 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e for k, v := range epConfig.DriverOpts { createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(options.Generic{k: v})) } + + if epConfig.MacAddress != "" { + mac, err := net.ParseMAC(epConfig.MacAddress) + if err != nil { + return nil, err + } + genericOptions[netlabel.MacAddress] = mac + } } if svcCfg := c.NetworkSettings.Service; svcCfg != nil { @@ -863,9 +872,8 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e if err != nil { return nil, err } - createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(options.Generic{ - netlabel.MacAddress: mac, - })) + + genericOptions[netlabel.MacAddress] = mac } } @@ -940,7 +948,10 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e createOptions = append(createOptions, libnetwork.CreateOptionDNS(daemonDNS)) } - createOptions = append(createOptions, libnetwork.CreateOptionPortMapping(publishedPorts), libnetwork.CreateOptionExposedPorts(exposedPorts)) + createOptions = append(createOptions, + libnetwork.CreateOptionPortMapping(publishedPorts), + libnetwork.CreateOptionExposedPorts(exposedPorts), + libnetwork.EndpointOptionGeneric(genericOptions)) return createOptions, nil } diff --git a/docs/api/version-history.md b/docs/api/version-history.md index 21d91b7568c21..6d60ba46d326e 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -55,6 +55,8 @@ keywords: "API, Docker, rcli, REST, documentation" * `POST /services/create` and `POST /services/{id}/update` now accept `Seccomp` and `AppArmor` fields in the `ContainerSpec.Privileges` object. This allows some configuration of Seccomp and AppArmor in Swarm services. +* A new endpoint-specific `MacAddress` field has been added to `NetworkSettings.EndpointSettings` + on `POST /containers/create`, and to `EndpointConfig` on `POST /networks/{id}/connect`. ## v1.43 API changes diff --git a/integration/container/create_test.go b/integration/container/create_test.go index f30f9cdf073cf..9473ac611fcf7 100644 --- a/integration/container/create_test.go +++ b/integration/container/create_test.go @@ -1,10 +1,12 @@ package container // import "github.com/docker/docker/integration/container" import ( + "bufio" "context" "encoding/json" "fmt" "strconv" + "strings" "testing" "time" @@ -14,6 +16,7 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/errdefs" ctr "github.com/docker/docker/integration/internal/container" + net "github.com/docker/docker/integration/internal/network" "github.com/docker/docker/oci" "github.com/docker/docker/testutil" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -631,3 +634,42 @@ func TestCreateWithMultipleEndpointSettings(t *testing.T) { }) } } + +func TestCreateWithCustomMACs(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows") + skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.44"), "requires API v1.44") + + ctx := setupTest(t) + apiClient := testEnv.APIClient() + + net.CreateNoError(ctx, t, apiClient, "testnet") + + attachCtx, cancel := context.WithTimeout(ctx, 1*time.Second) + defer cancel() + res := ctr.RunAttach(attachCtx, t, apiClient, + ctr.WithCmd("ip", "-o", "link", "show"), + ctr.WithNetworkMode("bridge"), + ctr.WithMacAddress("bridge", "02:32:1c:23:00:04")) + + assert.Equal(t, res.ExitCode, 0) + assert.Equal(t, res.Stderr.String(), "") + + scanner := bufio.NewScanner(res.Stdout) + for scanner.Scan() { + fields := strings.Fields(scanner.Text()) + // The expected output is: + // 1: lo: mtu 65536 qdisc noqueue qlen 1000\ link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 + // 134: eth0@if135: mtu 1400 qdisc noqueue \ link/ether 02:42:ac:11:00:04 brd ff:ff:ff:ff:ff:ff + if len(fields) < 11 { + continue + } + + ifaceName := fields[1] + if ifaceName[:3] != "eth" { + continue + } + + mac := fields[len(fields)-3] + assert.Equal(t, mac, "02:32:1c:23:00:04") + } +} diff --git a/integration/internal/container/ops.go b/integration/internal/container/ops.go index 03a7365c6a28d..2e29d4562a411 100644 --- a/integration/internal/container/ops.go +++ b/integration/internal/container/ops.go @@ -114,6 +114,18 @@ func WithTmpfs(targetAndOpts string) func(config *TestContainerConfig) { } } +func WithMacAddress(networkName, mac string) func(config *TestContainerConfig) { + return func(c *TestContainerConfig) { + if c.NetworkingConfig.EndpointsConfig == nil { + c.NetworkingConfig.EndpointsConfig = map[string]*network.EndpointSettings{} + } + if v, ok := c.NetworkingConfig.EndpointsConfig[networkName]; !ok || v == nil { + c.NetworkingConfig.EndpointsConfig[networkName] = &network.EndpointSettings{} + } + c.NetworkingConfig.EndpointsConfig[networkName].MacAddress = mac + } +} + // WithIPv4 sets the specified ip for the specified network of the container func WithIPv4(networkName, ip string) func(*TestContainerConfig) { return func(c *TestContainerConfig) { From ee9f0ed895a15131ee9e3e85670d25f8a5abe2e1 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Tue, 4 Jul 2023 12:40:35 +0200 Subject: [PATCH 3/5] api: Deprecate ContainerConfig.MacAddress Having a sandbox/container-wide MacAddress field makes little sense since a container can be connected to multiple networks at the same time. This field is an artefact of old times where a container could be connected to a single network only. As we now have a way to specify per-endpoint mac address, this field is now deprecated. Signed-off-by: Albin Kerouanton Signed-off-by: Sebastiaan van Stijn --- .../router/container/container_routes.go | 57 ++++++++++++++++++- api/swagger.yaml | 5 +- api/types/container/config.go | 15 +++-- client/container_create.go | 7 ++- daemon/inspect.go | 17 +++++- daemon/inspect_linux.go | 2 +- daemon/network.go | 16 ------ docs/api/version-history.md | 1 + integration-cli/docker_cli_netmode_test.go | 2 - integration-cli/docker_cli_run_test.go | 5 -- integration/internal/container/ops.go | 2 +- runconfig/hostconfig.go | 4 -- 12 files changed, 93 insertions(+), 40 deletions(-) diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 2251fb140089f..027885975171b 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -628,6 +628,13 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo } } + var warnings []string + if warn, err := handleMACAddressBC(config, hostConfig, networkingConfig, version); err != nil { + return err + } else if warn != "" { + warnings = append(warnings, warn) + } + if hostConfig.PidsLimit != nil && *hostConfig.PidsLimit <= 0 { // Don't set a limit if either no limit was specified, or "unlimited" was // explicitly set. @@ -647,10 +654,58 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo if err != nil { return err } - + ccr.Warnings = append(ccr.Warnings, warnings...) return httputils.WriteJSON(w, http.StatusCreated, ccr) } +// 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(config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, version string) (string, error) { + if config.MacAddress == "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. + return "", nil + } + + deprecatedMacAddress := config.MacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. + + if versions.LessThan(version, "1.44") { + // The container-wide MacAddress parameter is deprecated and should now be specified in EndpointsConfig. + if hostConfig.NetworkMode.IsDefault() || hostConfig.NetworkMode.IsBridge() || hostConfig.NetworkMode.IsUserDefined() { + nwName := hostConfig.NetworkMode.NetworkName() + if _, ok := networkingConfig.EndpointsConfig[nwName]; !ok { + networkingConfig.EndpointsConfig[nwName] = &network.EndpointSettings{} + } + // Overwrite the config: either the endpoint's MacAddress was set by the user on API < v1.44, which + // must be ignored, or migrate the top-level MacAddress to the endpoint's config. + networkingConfig.EndpointsConfig[nwName].MacAddress = deprecatedMacAddress + } + if !hostConfig.NetworkMode.IsDefault() && !hostConfig.NetworkMode.IsBridge() && !hostConfig.NetworkMode.IsUserDefined() { + return "", runconfig.ErrConflictContainerNetworkAndMac + } + + return "", nil + } + + var warning string + if hostConfig.NetworkMode.IsDefault() || hostConfig.NetworkMode.IsBridge() || hostConfig.NetworkMode.IsUserDefined() { + nwName := hostConfig.NetworkMode.NetworkName() + if _, ok := networkingConfig.EndpointsConfig[nwName]; !ok { + networkingConfig.EndpointsConfig[nwName] = &network.EndpointSettings{} + } + + ep := networkingConfig.EndpointsConfig[nwName] + if ep.MacAddress == "" { + ep.MacAddress = deprecatedMacAddress + } else if ep.MacAddress != deprecatedMacAddress { + return "", errdefs.InvalidParameter(errors.New("the container-wide MAC address should match the endpoint-specific MAC address for the main network or should be left empty")) + } + } + warning = "The container-wide MacAddress field is now deprecated. It should be specified in EndpointsConfig instead." + config.MacAddress = "" //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. + + return warning, nil +} + func (s *containerRouter) deleteContainers(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { if err := httputils.ParseForm(r); err != nil { return err diff --git a/api/swagger.yaml b/api/swagger.yaml index c5725d73c4188..3ee7e50b817c8 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -1313,7 +1313,10 @@ definitions: type: "boolean" x-nullable: true MacAddress: - description: "MAC address of the container." + description: | + MAC address of the container. + + Deprecated: this field is deprecated in API v1.44 and up. Use EndpointSettings.MacAddress instead. type: "string" x-nullable: true OnBuild: diff --git a/api/types/container/config.go b/api/types/container/config.go index d8cfe443f8b97..be41d6315e5d7 100644 --- a/api/types/container/config.go +++ b/api/types/container/config.go @@ -70,10 +70,13 @@ type Config struct { WorkingDir string // Current directory (PWD) in the command will be launched Entrypoint strslice.StrSlice // Entrypoint to run when starting the container NetworkDisabled bool `json:",omitempty"` // Is network disabled - MacAddress string `json:",omitempty"` // Mac Address of the container - OnBuild []string // ONBUILD metadata that were defined on the image Dockerfile - Labels map[string]string // List of labels set to this container - StopSignal string `json:",omitempty"` // Signal to stop a container - StopTimeout *int `json:",omitempty"` // Timeout (in seconds) to stop a container - Shell strslice.StrSlice `json:",omitempty"` // Shell for shell-form of RUN, CMD, ENTRYPOINT + // Mac Address of the container. + // + // Deprecated: this field is deprecated since API v1.44. Use EndpointSettings.MacAddress instead. + MacAddress string `json:",omitempty"` + OnBuild []string // ONBUILD metadata that were defined on the image Dockerfile + Labels map[string]string // List of labels set to this container + StopSignal string `json:",omitempty"` // Signal to stop a container + StopTimeout *int `json:",omitempty"` // Timeout (in seconds) to stop a container + Shell strslice.StrSlice `json:",omitempty"` // Shell for shell-form of RUN, CMD, ENTRYPOINT } diff --git a/client/container_create.go b/client/container_create.go index ce9a7a404d013..409f5b492a6ed 100644 --- a/client/container_create.go +++ b/client/container_create.go @@ -39,7 +39,7 @@ func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config if err := cli.NewVersionError(ctx, "1.44", "specify health-check start interval"); config != nil && config.Healthcheck != nil && config.Healthcheck.StartInterval != 0 && err != nil { return response, err } - if err := cli.NewVersionError("1.44", "specify mac-address per network"); hasEndpointSpecificMacAddress(networkingConfig) && err != nil { + if err := cli.NewVersionError(ctx, "1.44", "specify mac-address per network"); hasEndpointSpecificMacAddress(networkingConfig) && err != nil { return response, err } @@ -58,6 +58,11 @@ func (cli *Client) ContainerCreate(ctx context.Context, config *container.Config } } + // Since API 1.44, the container-wide MacAddress is deprecated and will trigger a WARNING if it's specified. + if versions.GreaterThanOrEqualTo(cli.ClientVersion(), "1.44") { + config.MacAddress = "" //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. + } + query := url.Values{} if p := formatPlatform(platform); p != "" { query.Set("platform", p) diff --git a/daemon/inspect.go b/daemon/inspect.go index a60b61d67b9e0..a6fc35b10a91b 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -27,8 +27,9 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, size bo return daemon.containerInspectPre120(ctx, name) case versions.Equal(version, "1.20"): return daemon.containerInspect120(name) + default: + return daemon.ContainerInspectCurrent(ctx, name, size) } - return daemon.ContainerInspectCurrent(ctx, name, size) } // ContainerInspectCurrent returns low-level information about a @@ -116,7 +117,7 @@ func (daemon *Daemon) containerInspect120(name string) (*v1p20.ContainerJSON, er Mounts: ctr.GetMountPoints(), Config: &v1p20.ContainerConfig{ Config: ctr.Config, - MacAddress: ctr.Config.MacAddress, + MacAddress: ctr.Config.MacAddress, //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. NetworkDisabled: ctr.Config.NetworkDisabled, ExposedPorts: ctr.Config.ExposedPorts, VolumeDriver: ctr.HostConfig.VolumeDriver, @@ -138,6 +139,18 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, container *contai // We merge the Ulimits from hostConfig with daemon default daemon.mergeUlimits(&hostConfig, daemonCfg) + // Migrate the container's default network's MacAddress to the top-level + // 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 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. + } + } + } + var containerHealth *types.Health if container.State.Health != nil { containerHealth = &types.Health{ diff --git a/daemon/inspect_linux.go b/daemon/inspect_linux.go index 77cb0086778e2..bcfaf105ed264 100644 --- a/daemon/inspect_linux.go +++ b/daemon/inspect_linux.go @@ -47,7 +47,7 @@ func (daemon *Daemon) containerInspectPre120(ctx context.Context, name string) ( VolumesRW: volumesRW, Config: &v1p19.ContainerConfig{ Config: ctr.Config, - MacAddress: ctr.Config.MacAddress, + MacAddress: ctr.Config.MacAddress, //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. NetworkDisabled: ctr.Config.NetworkDisabled, ExposedPorts: ctr.Config.ExposedPorts, VolumeDriver: ctr.HostConfig.VolumeDriver, diff --git a/daemon/network.go b/daemon/network.go index a49ded0f8b2d2..35624e5acbb84 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -861,22 +861,6 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e createOptions = append(createOptions, libnetwork.CreateOptionDisableResolution()) } - // configs that are applicable only for the endpoint in the network - // to which container was connected to on docker run. - // Ideally all these network-specific endpoint configurations must be moved under - // container.NetworkSettings.Networks[n.Name()] - netMode := c.HostConfig.NetworkMode - if nwName == netMode.NetworkName() || n.ID() == netMode.NetworkName() || (nwName == defaultNetName && netMode.IsDefault()) { - if c.Config.MacAddress != "" { - mac, err := net.ParseMAC(c.Config.MacAddress) - if err != nil { - return nil, err - } - - genericOptions[netlabel.MacAddress] = mac - } - } - // Port-mapping rules belong to the container & applicable only to non-internal networks. // // TODO(thaJeztah): Look if we can provide a more minimal function for getPortMapInfo, as it does a lot, and we only need the "length". diff --git a/docs/api/version-history.md b/docs/api/version-history.md index 6d60ba46d326e..ddef65e1553da 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -57,6 +57,7 @@ keywords: "API, Docker, rcli, REST, documentation" some configuration of Seccomp and AppArmor in Swarm services. * A new endpoint-specific `MacAddress` field has been added to `NetworkSettings.EndpointSettings` on `POST /containers/create`, and to `EndpointConfig` on `POST /networks/{id}/connect`. + The container-wide `MacAddress` field in `Config`, on `POST /containers/create`, is now deprecated. ## v1.43 API changes diff --git a/integration-cli/docker_cli_netmode_test.go b/integration-cli/docker_cli_netmode_test.go index 80ba1374f144b..5756a06360764 100644 --- a/integration-cli/docker_cli_netmode_test.go +++ b/integration-cli/docker_cli_netmode_test.go @@ -88,8 +88,6 @@ func (s *DockerCLINetmodeSuite) TestConflictNetworkModeAndOptions(c *testing.T) assert.Assert(c, strings.Contains(out, runconfig.ErrConflictNetworkAndDNS.Error())) out = dockerCmdWithFail(c, "run", "--net=container:other", "--add-host=name:8.8.8.8", "busybox", "ps") assert.Assert(c, strings.Contains(out, runconfig.ErrConflictNetworkHosts.Error())) - out = dockerCmdWithFail(c, "run", "--net=container:other", "--mac-address=92:d0:c6:0a:29:33", "busybox", "ps") - assert.Assert(c, strings.Contains(out, runconfig.ErrConflictContainerNetworkAndMac.Error())) out = dockerCmdWithFail(c, "run", "--net=container:other", "-P", "busybox", "ps") assert.Assert(c, strings.Contains(out, runconfig.ErrConflictNetworkPublishPorts.Error())) out = dockerCmdWithFail(c, "run", "--net=container:other", "-p", "8080", "busybox", "ps") diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index c96a0f6a9b67b..585a3239bb0f6 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -3311,11 +3311,6 @@ func (s *DockerCLIRunSuite) TestRunContainerNetModeWithDNSMacHosts(c *testing.T) c.Fatalf("run --net=container with --dns should error out") } - out, _, err = dockerCmdWithError("run", "--mac-address", "92:d0:c6:0a:29:33", "--net=container:parent", "busybox") - if err == nil || !strings.Contains(out, runconfig.ErrConflictContainerNetworkAndMac.Error()) { - c.Fatalf("run --net=container with --mac-address should error out") - } - out, _, err = dockerCmdWithError("run", "--add-host", "test:192.168.2.109", "--net=container:parent", "busybox") if err == nil || !strings.Contains(out, runconfig.ErrConflictNetworkHosts.Error()) { c.Fatalf("run --net=container with --add-host should error out") diff --git a/integration/internal/container/ops.go b/integration/internal/container/ops.go index 2e29d4562a411..43247c8c1714e 100644 --- a/integration/internal/container/ops.go +++ b/integration/internal/container/ops.go @@ -312,6 +312,6 @@ func WithStopSignal(stopSignal string) func(c *TestContainerConfig) { func WithContainerWideMacAddress(address string) func(c *TestContainerConfig) { return func(c *TestContainerConfig) { - c.Config.MacAddress = address + c.Config.MacAddress = address //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. } } diff --git a/runconfig/hostconfig.go b/runconfig/hostconfig.go index 9603a94ecade5..29d3ac812e102 100644 --- a/runconfig/hostconfig.go +++ b/runconfig/hostconfig.go @@ -53,10 +53,6 @@ func validateNetContainerMode(c *container.Config, hc *container.HostConfig) err return ErrConflictNetworkHosts } - if (hc.NetworkMode.IsContainer() || hc.NetworkMode.IsHost()) && c.MacAddress != "" { - return ErrConflictContainerNetworkAndMac - } - if hc.NetworkMode.IsContainer() && (len(hc.PortBindings) > 0 || hc.PublishAllPorts) { return ErrConflictNetworkPublishPorts } From 61c78d9438186dc82e6cad67bc1363849f1889b5 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Fri, 4 Aug 2023 16:47:34 +0200 Subject: [PATCH 4/5] daemon: Validate MacAddress on container create This was previously checked only on ContainerStart. Signed-off-by: Albin Kerouanton --- daemon/container_operations.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 2c5e04888ec2d..74d1c3ccdfa13 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -612,6 +612,13 @@ func validateEndpointSettings(nw *libnetwork.Network, nwName string, epConfig *n } } + if epConfig.MacAddress != "" { + _, err := net.ParseMAC(epConfig.MacAddress) + if err != nil { + return fmt.Errorf("invalid MAC address %s", epConfig.MacAddress) + } + } + if err := multierror.Join(errs...); err != nil { return fmt.Errorf("invalid endpoint settings:\n%w", err) } From 59c5f6e35b5b230f24fd887000ec5c8d35cc0565 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Wed, 25 Oct 2023 10:25:00 +0200 Subject: [PATCH 5/5] integration-cli: mark dockerCmdWithFail as an helper function Signed-off-by: Albin Kerouanton --- integration-cli/docker_cli_netmode_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/integration-cli/docker_cli_netmode_test.go b/integration-cli/docker_cli_netmode_test.go index 5756a06360764..61c43569390b8 100644 --- a/integration-cli/docker_cli_netmode_test.go +++ b/integration-cli/docker_cli_netmode_test.go @@ -31,6 +31,7 @@ func (s *DockerCLINetmodeSuite) OnTimeout(c *testing.T) { // DockerCmdWithFail executes a docker command that is supposed to fail and returns // the output. If the command returns a Nil error, it will fail and stop the tests. func dockerCmdWithFail(c *testing.T, args ...string) string { + c.Helper() out, _, err := dockerCmdWithError(args...) assert.Assert(c, err != nil, "%v", out) return out