From a35716f5b93d738e32edfc218645cab7cb9be4ec Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Tue, 2 Apr 2024 15:10:33 +0100 Subject: [PATCH 1/3] Factor out selection of endpoint for config migration Signed-off-by: Rob Murray --- .../router/container/container_routes.go | 107 +++++++++++------- .../router/container/container_routes_test.go | 76 ++++++++++++- 2 files changed, 139 insertions(+), 44 deletions(-) diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 3872028652..7191d291c8 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -662,23 +662,11 @@ func handleMACAddressBC(config *container.Config, hostConfig *container.HostConf return "", runconfig.ErrConflictContainerNetworkAndMac } - // There cannot be more than one entry in EndpointsConfig with API < 1.44. - - // If there's no EndpointsConfig, create a place to store the configured address. It is - // safe to use NetworkMode as the network name, whether it's a name or id/short-id, as - // it will be normalised later and there is no other EndpointSettings object that might - // refer to this network/endpoint. - if len(networkingConfig.EndpointsConfig) == 0 { - nwName := hostConfig.NetworkMode.NetworkName() - networkingConfig.EndpointsConfig[nwName] = &network.EndpointSettings{} - } - // There's exactly one network in EndpointsConfig, either from the API or just-created. - // Migrate the container-wide setting to it. - // No need to check for a match between NetworkMode and the names/ids in EndpointsConfig, - // the old version of the API would have applied the address to this network anyway. - for _, ep := range networkingConfig.EndpointsConfig { - ep.MacAddress = deprecatedMacAddress + epConfig, err := epConfigForNetMode(version, hostConfig.NetworkMode, networkingConfig) + if err != nil { + return "", err } + epConfig.MacAddress = deprecatedMacAddress return "", nil } @@ -688,31 +676,16 @@ func handleMACAddressBC(config *container.Config, hostConfig *container.HostConf } var warning string if hostConfig.NetworkMode.IsBridge() || hostConfig.NetworkMode.IsUserDefined() { - nwName := hostConfig.NetworkMode.NetworkName() - // If there's no endpoint config, create a place to store the configured address. - if len(networkingConfig.EndpointsConfig) == 0 { - networkingConfig.EndpointsConfig[nwName] = &network.EndpointSettings{ - MacAddress: deprecatedMacAddress, - } - } else { - // There is existing endpoint config - if it's not indexed by NetworkMode.Name(), we - // can't tell which network the container-wide settings was intended for. NetworkMode, - // the keys in EndpointsConfig and the NetworkID in EndpointsConfig may mix network - // name/id/short-id. It's not safe to create EndpointsConfig under the NetworkMode - // name to store the container-wide MAC address, because that may result in two sets - // of EndpointsConfig for the same network and one set will be discarded later. So, - // reject the request ... - ep, ok := networkingConfig.EndpointsConfig[nwName] - if !ok { - return "", errdefs.InvalidParameter(errors.New("if a container-wide MAC address is supplied, HostConfig.NetworkMode must match the identity of a network in NetworkSettings.Networks")) - } - // ep is the endpoint that needs the container-wide MAC address; migrate the address - // to it, or bail out if there's a mismatch. - if ep.MacAddress == "" { - ep.MacAddress = deprecatedMacAddress - } else if ep.MacAddress != deprecatedMacAddress { - return "", errdefs.InvalidParameter(errors.New("the container-wide MAC address must match the endpoint-specific MAC address for the main network, or be left empty")) - } + ep, err := epConfigForNetMode(version, hostConfig.NetworkMode, networkingConfig) + if err != nil { + return "", errors.Wrap(err, "unable to migrate container-wide MAC address to a specific network") + } + // ep is the endpoint that needs the container-wide MAC address; migrate the address + // to it, or bail out if there's a mismatch. + if ep.MacAddress == "" { + ep.MacAddress = deprecatedMacAddress + } else if ep.MacAddress != deprecatedMacAddress { + return "", errdefs.InvalidParameter(errors.New("the container-wide MAC address must match the endpoint-specific MAC address for the main network, or be left empty")) } } warning = "The container-wide MacAddress field is now deprecated. It should be specified in EndpointsConfig instead." @@ -721,6 +694,58 @@ func handleMACAddressBC(config *container.Config, hostConfig *container.HostConf return warning, nil } +// epConfigForNetMode finds, or creates, an entry in netConfig.EndpointsConfig +// corresponding to nwMode. +// +// nwMode.NetworkName() may be the network's name, its id, or its short-id. +// +// The corresponding endpoint in netConfig.EndpointsConfig may be keyed on a +// different one of name/id/short-id. If there's any ambiguity (there are +// endpoints but the names don't match), return an error and do not create a new +// endpoint, because it might be a duplicate. +func epConfigForNetMode( + version string, + nwMode container.NetworkMode, + netConfig *network.NetworkingConfig, +) (*network.EndpointSettings, error) { + nwName := nwMode.NetworkName() + + // It's always safe to create an EndpointsConfig entry under nwName if there are + // no entries already (because there can't be an entry for this network nwName + // refers to under any other name/short-id/id). + if len(netConfig.EndpointsConfig) == 0 { + es := &network.EndpointSettings{} + netConfig.EndpointsConfig = map[string]*network.EndpointSettings{ + nwName: es, + } + return es, nil + } + + // There cannot be more than one entry in EndpointsConfig with API < 1.44. + if versions.LessThan(version, "1.44") { + // No need to check for a match between NetworkMode and the names/ids in EndpointsConfig, + // the old version of the API would pick this network anyway. + for _, ep := range netConfig.EndpointsConfig { + return ep, nil + } + } + + // There is existing endpoint config - if it's not indexed by NetworkMode.Name(), we + // can't tell which network the container-wide settings are intended for. NetworkMode, + // the keys in EndpointsConfig and the NetworkID in EndpointsConfig may mix network + // name/id/short-id. It's not safe to create EndpointsConfig under the NetworkMode + // name to store the container-wide setting, because that may result in two sets + // of EndpointsConfig for the same network and one set will be discarded later. So, + // reject the request ... + ep, ok := netConfig.EndpointsConfig[nwName] + if !ok { + return nil, errdefs.InvalidParameter( + errors.New("HostConfig.NetworkMode must match the identity of a network in NetworkSettings.Networks")) + } + + return ep, 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/server/router/container/container_routes_test.go b/api/server/router/container/container_routes_test.go index 2a96ad75d5..7c70e1c01f 100644 --- a/api/server/router/container/container_routes_test.go +++ b/api/server/router/container/container_routes_test.go @@ -102,7 +102,7 @@ func TestHandleMACAddressBC(t *testing.T) { ctrWideMAC: "11:22:33:44:55:66", networkMode: "aNetId", epConfig: map[string]*network.EndpointSettings{"aNetName": {}}, - expError: "if a container-wide MAC address is supplied, HostConfig.NetworkMode must match the identity of a network in NetworkSettings.Networks", + expError: "unable to migrate container-wide MAC address to a specific network: HostConfig.NetworkMode must match the identity of a network in NetworkSettings.Networks", expCtrWideMAC: "11:22:33:44:55:66", }, { @@ -126,8 +126,8 @@ func TestHandleMACAddressBC(t *testing.T) { } epConfig := make(map[string]*network.EndpointSettings, len(tc.epConfig)) for k, v := range tc.epConfig { - v := v - epConfig[k] = v + v := *v + epConfig[k] = &v } netCfg := &network.NetworkingConfig{ EndpointsConfig: epConfig, @@ -158,3 +158,73 @@ func TestHandleMACAddressBC(t *testing.T) { }) } } + +func TestEpConfigForNetMode(t *testing.T) { + testcases := []struct { + name string + apiVersion string + networkMode string + epConfig map[string]*network.EndpointSettings + expEpId string + expNumEps int + expError bool + }{ + { + name: "old api no eps", + apiVersion: "1.43", + networkMode: "mynet", + expNumEps: 1, + }, + { + name: "new api no eps", + apiVersion: "1.44", + networkMode: "mynet", + expNumEps: 1, + }, + { + name: "old api with ep", + apiVersion: "1.43", + networkMode: "mynet", + epConfig: map[string]*network.EndpointSettings{ + "anything": {EndpointID: "epone"}, + }, + expEpId: "epone", + expNumEps: 1, + }, + { + name: "new api with matching ep", + apiVersion: "1.44", + networkMode: "mynet", + epConfig: map[string]*network.EndpointSettings{ + "mynet": {EndpointID: "epone"}, + }, + expEpId: "epone", + expNumEps: 1, + }, + { + name: "new api with mismatched ep", + apiVersion: "1.44", + networkMode: "mynet", + epConfig: map[string]*network.EndpointSettings{ + "shortid": {EndpointID: "epone"}, + }, + expError: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + netConfig := &network.NetworkingConfig{ + EndpointsConfig: tc.epConfig, + } + ep, err := epConfigForNetMode(tc.apiVersion, container.NetworkMode(tc.networkMode), netConfig) + if tc.expError { + assert.Check(t, is.ErrorContains(err, "HostConfig.NetworkMode must match the identity of a network in NetworkSettings.Networks")) + } else { + assert.Assert(t, err) + assert.Check(t, is.Equal(ep.EndpointID, tc.expEpId)) + assert.Check(t, is.Len(netConfig.EndpointsConfig, tc.expNumEps)) + } + }) + } +} From 1e29f9b12fd00900a51f2b50ca058d62d2ac6599 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Wed, 8 May 2024 10:27:20 +0100 Subject: [PATCH 2/3] Move EndpointSettings.DriverOpts from op-state to config Signed-off-by: Rob Murray --- api/swagger.yaml | 22 +++++++++++----------- api/types/network/endpoint.go | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/api/swagger.yaml b/api/swagger.yaml index 2aaae02447..c0b6fcba31 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -2495,6 +2495,17 @@ definitions: example: - "server_x" - "server_y" + DriverOpts: + description: | + DriverOpts is a mapping of driver options and values. These options + are passed directly to the driver and are driver specific. + type: "object" + x-nullable: true + additionalProperties: + type: "string" + example: + com.example.some-label: "some-value" + com.example.some-other-label: "some-other-value" # Operational data NetworkID: @@ -2538,17 +2549,6 @@ definitions: type: "integer" format: "int64" example: 64 - DriverOpts: - description: | - DriverOpts is a mapping of driver options and values. These options - are passed directly to the driver and are driver specific. - type: "object" - x-nullable: true - additionalProperties: - type: "string" - example: - com.example.some-label: "some-value" - com.example.some-other-label: "some-other-value" DNSNames: description: | List of all DNS names an endpoint has on a specific network. This diff --git a/api/types/network/endpoint.go b/api/types/network/endpoint.go index 9edd1c38d9..0fbb40b351 100644 --- a/api/types/network/endpoint.go +++ b/api/types/network/endpoint.go @@ -18,6 +18,7 @@ type EndpointSettings struct { // Once the container is running, it becomes operational data (it may contain a // generated address). MacAddress string + DriverOpts map[string]string // Operational data NetworkID string EndpointID string @@ -27,7 +28,6 @@ type EndpointSettings struct { IPv6Gateway string GlobalIPv6Address string GlobalIPv6PrefixLen int - DriverOpts map[string]string // DNSNames holds all the (non fully qualified) DNS names associated to this endpoint. First entry is used to // generate PTR records. DNSNames []string From 0071832226b42dbb12bc01bf1c668dca891b2d33 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Tue, 2 Apr 2024 15:11:02 +0100 Subject: [PATCH 3/3] Add per-endpoint sysctls to DriverOpts Until now it's been possible to set per-interface sysctls using, for example, '--sysctl net.ipv6.conf.eth0.accept_ra=2'. But, the index in the interface name is allocated serially, and the numbering in a container with more than one interface may change when a container is restarted. The change to make it possible to connect a container to more than one network when it's created increased the ambiguity. This change adds label "com.docker.network.endpoint.sysctls" to the DriverOpts in EndpointSettings. This option is explicitly associated with the interface. Settings in "--sysctl" for "eth0" are migrated to DriverOpts. Because using "--sysctl" with any interface apart from "eth0" would have unpredictable results, it is now an error to use any other interface name in the top level "--sysctl" option. The error message includes a hint at how to use the new per-interface setting. The per-endpoint sysctl name has the interface name replaced by "IFNAME". For example: net.ipv6.conf.eth0.accept_ra=2 becomes: net.ipv6.conf.IFNAME.accept_ra=2 The value of DriverOpts["com.docker.network.endpoint.sysctls"] is a comma separated list. Settings from '--sysctl' are applied by the runtime lib during task creation. So, task creation fails if the endpoint does not exist. Applying per-endpoint settings during interface configuration means the endpoint can be created later, which paves the way for removal of the SetKey OCI prestart hook. Unlike other DriverOpts, the sysctl label itself is not driver-specific, but each driver has a chance to check settings/values and raise an error if a setting would cause it a problem - no such checks have been added in this initial version. As a future extension, if required, it would be possible for the driver to echo back valid/extended/modified settings to libnetwork for it to apply to the interface. (At that point, the syntax for the options could become driver specific to allow, for example, a driver to create more than one interface). Signed-off-by: Rob Murray --- .../router/container/container_routes.go | 95 ++++++++++++++ .../router/container/container_routes_test.go | 120 ++++++++++++++++++ daemon/container_operations.go | 15 +++ docs/api/version-history.md | 12 ++ integration/networking/bridge_test.go | 37 ++++++ libnetwork/endpoint.go | 13 ++ libnetwork/netlabel/labels.go | 4 + libnetwork/osl/interface_linux.go | 50 +++++++- libnetwork/osl/options_linux.go | 8 ++ libnetwork/sandbox_linux.go | 3 + 10 files changed, 355 insertions(+), 2 deletions(-) diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 7191d291c8..97ccda3734 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -23,6 +23,7 @@ import ( "github.com/docker/docker/api/types/versions" containerpkg "github.com/docker/docker/container" "github.com/docker/docker/errdefs" + "github.com/docker/docker/libnetwork/netlabel" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/runconfig" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -619,6 +620,12 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo warnings = append(warnings, warn) } + if warn, err := handleSysctlBC(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. @@ -694,6 +701,94 @@ func handleMACAddressBC(config *container.Config, hostConfig *container.HostConf return warning, nil } +// handleSysctlBC migrates top level network endpoint-specific '--sysctl' +// settings to an DriverOpts for an endpoint. This is necessary because sysctls +// are applied during container task creation, but sysctls that name an interface +// (for example 'net.ipv6.conf.eth0.forwarding') cannot be applied until the +// interface has been created. So, these settings are removed from hostConfig.Sysctls +// and added to DriverOpts[netlabel.EndpointSysctls]. +// +// Because interface names ('ethN') are allocated sequentially, and the order of +// network connections is not deterministic on container restart, only 'eth0' +// would work reliably in a top-level '--sysctl' option, and then only when +// there's a single initial network connection. So, settings for 'eth0' are +// migrated to the primary interface, identified by 'hostConfig.NetworkMode'. +// Settings for other interfaces are treated as errors. +// +// In the DriverOpts, because the interface name cannot be determined in advance, the +// interface name is replaced by "IFNAME". For example, 'net.ipv6.conf.eth0.forwarding' +// becomes 'net.ipv6.conf.IFNAME.forwarding'. The value in DriverOpts is a +// comma-separated list. +// +// A warning is generated when settings are migrated. +func handleSysctlBC( + hostConfig *container.HostConfig, + netConfig *network.NetworkingConfig, + version string, +) (string, error) { + if !hostConfig.NetworkMode.IsPrivate() { + return "", nil + } + + var ep *network.EndpointSettings + var toDelete []string + var netIfSysctls []string + for k, v := range hostConfig.Sysctls { + // If the sysctl name matches "net.*.*.eth0.*" ... + if spl := strings.SplitN(k, ".", 5); len(spl) == 5 && spl[0] == "net" && strings.HasPrefix(spl[3], "eth") { + netIfSysctl := fmt.Sprintf("net.%s.%s.IFNAME.%s=%s", spl[1], spl[2], spl[4], v) + // Find the EndpointConfig to migrate settings to, if not already found. + if ep == nil { + // Per-endpoint sysctls were introduced in API version 1.46. Migration is + // needed, but refuse to do it automatically for newer versions of the API. + if versions.GreaterThan(version, "1.46") { + return "", fmt.Errorf("interface specific sysctl setting %q must be supplied using driver option '%s'", + k, netlabel.EndpointSysctls) + } + var err error + ep, err = epConfigForNetMode(version, hostConfig.NetworkMode, netConfig) + if err != nil { + return "", fmt.Errorf("unable to find a network for sysctl %s: %w", k, err) + } + } + // Only try to migrate settings for "eth0", anything else would always + // have behaved unpredictably. + if spl[3] != "eth0" { + return "", fmt.Errorf(`unable to determine network endpoint for sysctl %s, use driver option '%s' to set per-interface sysctls`, + k, netlabel.EndpointSysctls) + } + // Prepare the migration. + toDelete = append(toDelete, k) + netIfSysctls = append(netIfSysctls, netIfSysctl) + } + } + if ep == nil { + return "", nil + } + + newDriverOpt := strings.Join(netIfSysctls, ",") + warning := fmt.Sprintf(`Migrated sysctl %q to DriverOpts{%q:%q}.`, + strings.Join(toDelete, ","), + netlabel.EndpointSysctls, newDriverOpt) + + // Append existing per-endpoint sysctls to the migrated sysctls (give priority + // to per-endpoint settings). + if ep.DriverOpts == nil { + ep.DriverOpts = map[string]string{} + } + if oldDriverOpt, ok := ep.DriverOpts[netlabel.EndpointSysctls]; ok { + newDriverOpt += "," + oldDriverOpt + } + ep.DriverOpts[netlabel.EndpointSysctls] = newDriverOpt + + // Delete migrated settings from the top-level sysctls. + for _, k := range toDelete { + delete(hostConfig.Sysctls, k) + } + + return warning, nil +} + // epConfigForNetMode finds, or creates, an entry in netConfig.EndpointsConfig // corresponding to nwMode. // diff --git a/api/server/router/container/container_routes_test.go b/api/server/router/container/container_routes_test.go index 7c70e1c01f..dd31ca2ab2 100644 --- a/api/server/router/container/container_routes_test.go +++ b/api/server/router/container/container_routes_test.go @@ -1,10 +1,12 @@ package container import ( + "strings" "testing" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" + "github.com/docker/docker/libnetwork/netlabel" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -228,3 +230,121 @@ func TestEpConfigForNetMode(t *testing.T) { }) } } + +func TestHandleSysctlBC(t *testing.T) { + testcases := []struct { + name string + apiVersion string + networkMode string + sysctls map[string]string + epConfig map[string]*network.EndpointSettings + expEpSysctls []string + expSysctls map[string]string + expWarningContains []string + expError string + }{ + { + name: "migrate to new ep", + apiVersion: "1.46", + networkMode: "mynet", + sysctls: map[string]string{ + "net.ipv6.conf.all.disable_ipv6": "0", + "net.ipv6.conf.eth0.accept_ra": "2", + "net.ipv6.conf.eth0.forwarding": "1", + }, + expSysctls: map[string]string{ + "net.ipv6.conf.all.disable_ipv6": "0", + }, + expEpSysctls: []string{"net.ipv6.conf.IFNAME.forwarding=1", "net.ipv6.conf.IFNAME.accept_ra=2"}, + expWarningContains: []string{ + "Migrated", + "net.ipv6.conf.eth0.accept_ra", "net.ipv6.conf.IFNAME.accept_ra=2", + "net.ipv6.conf.eth0.forwarding", "net.ipv6.conf.IFNAME.forwarding=1", + }, + }, + { + name: "migrate nothing", + apiVersion: "1.46", + networkMode: "mynet", + sysctls: map[string]string{ + "net.ipv6.conf.all.disable_ipv6": "0", + }, + expSysctls: map[string]string{ + "net.ipv6.conf.all.disable_ipv6": "0", + }, + }, + { + name: "migration disabled for newer api", + apiVersion: "1.47", + networkMode: "mynet", + sysctls: map[string]string{ + "net.ipv6.conf.eth0.accept_ra": "2", + }, + expError: "must be supplied using driver option 'com.docker.network.endpoint.sysctls'", + }, + { + name: "only migrate eth0", + apiVersion: "1.46", + networkMode: "mynet", + sysctls: map[string]string{ + "net.ipv6.conf.eth1.accept_ra": "2", + }, + expError: "unable to determine network endpoint", + }, + { + name: "net name mismatch", + apiVersion: "1.46", + networkMode: "mynet", + epConfig: map[string]*network.EndpointSettings{ + "shortid": {EndpointID: "epone"}, + }, + sysctls: map[string]string{ + "net.ipv6.conf.eth1.accept_ra": "2", + }, + expError: "unable to find a network for sysctl", + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + hostCfg := &container.HostConfig{ + NetworkMode: container.NetworkMode(tc.networkMode), + Sysctls: map[string]string{}, + } + for k, v := range tc.sysctls { + hostCfg.Sysctls[k] = v + } + netCfg := &network.NetworkingConfig{ + EndpointsConfig: tc.epConfig, + } + + warnings, err := handleSysctlBC(hostCfg, netCfg, tc.apiVersion) + + for _, s := range tc.expWarningContains { + assert.Check(t, is.Contains(warnings, s)) + } + + if tc.expError != "" { + assert.Check(t, is.ErrorContains(err, tc.expError)) + } else { + assert.Check(t, err) + + assert.Check(t, is.DeepEqual(hostCfg.Sysctls, tc.expSysctls)) + + ep := netCfg.EndpointsConfig[tc.networkMode] + if ep == nil { + assert.Check(t, is.Nil(tc.expEpSysctls)) + } else { + got, ok := ep.DriverOpts[netlabel.EndpointSysctls] + assert.Check(t, ok) + // Check for expected ep-sysctls. + for _, want := range tc.expEpSysctls { + assert.Check(t, is.Contains(got, want)) + } + // Check for unexpected ep-sysctls. + assert.Check(t, is.Len(got, len(strings.Join(tc.expEpSysctls, ",")))) + } + } + }) + } +} diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 86da02f172..f6e4ff55f2 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -615,6 +615,21 @@ func validateEndpointSettings(nw *libnetwork.Network, nwName string, epConfig *n } } + if sysctls, ok := epConfig.DriverOpts[netlabel.EndpointSysctls]; ok { + for _, sysctl := range strings.Split(sysctls, ",") { + scname := strings.SplitN(sysctl, ".", 5) + // Allow "ifname" as well as "IFNAME", because the CLI converts to lower case. + if len(scname) != 5 || + (scname[1] != "ipv4" && scname[1] != "ipv6" && scname[1] != "mpls") || + (scname[3] != "IFNAME" && scname[3] != "ifname") { + errs = append(errs, + fmt.Errorf( + "unrecognised network interface sysctl '%s'; represent 'net.X.Y.ethN.Z=V' as 'net.X.Y.IFNAME.Z=V', 'X' must be 'ipv4', 'ipv6' or 'mpls'", + sysctl)) + } + } + } + if err := multierror.Join(errs...); err != nil { return fmt.Errorf("invalid endpoint settings:\n%w", err) } diff --git a/docs/api/version-history.md b/docs/api/version-history.md index 2a1fc4fd93..de05d33d63 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -15,6 +15,18 @@ keywords: "API, Docker, rcli, REST, documentation" ## v1.46 API changes +[Docker Engine API v1.46](https://docs.docker.com/engine/api/v1.46/) documentation + +* `POST /containers/create` field `NetworkingConfig.EndpointsConfig.DriverOpts`, + and `POST /networks/{id}/connect` field `EndpointsConfig.DriverOpts`, now + support label `com.docker.network.endpoint.sysctls` for setting per-interface + sysctls. The value is a comma separated list of sysctl assignments, the + interface name must be "IFNAME". For example, to set + `net.ipv4.config.eth0.log_martians=1`, use + `net.ipv4.config.IFNAME.log_martians=1`. In API versions up-to 1.46, top level + `--sysctl` settings for `eth0` will be migrated to `DriverOpts` when possible. + This automatic migration will be removed for API versions 1.47 and greater. + ## v1.45 API changes [Docker Engine API v1.45](https://docs.docker.com/engine/api/v1.45/) documentation diff --git a/integration/networking/bridge_test.go b/integration/networking/bridge_test.go index cb2abfaf66..908e21b29e 100644 --- a/integration/networking/bridge_test.go +++ b/integration/networking/bridge_test.go @@ -10,8 +10,10 @@ import ( "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" + apinetwork "github.com/docker/docker/api/types/network" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/network" + "github.com/docker/docker/libnetwork/netlabel" "github.com/docker/docker/testutil" "github.com/docker/docker/testutil/daemon" "github.com/google/go-cmp/cmp/cmpopts" @@ -828,3 +830,38 @@ func TestReadOnlySlashProc(t *testing.T) { }) } } + +// Test that it's possible to set a sysctl on an interface in the container +// using DriverOpts. +func TestSetEndpointSysctl(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows", "no sysctl on Windows") + + ctx := setupTest(t) + d := daemon.New(t) + d.StartWithBusybox(ctx, t) + defer d.Stop(t) + + c := d.NewClientT(t) + defer c.Close() + + const scName = "net.ipv4.conf.eth0.forwarding" + for _, ifname := range []string{"IFNAME", "ifname"} { + for _, val := range []string{"0", "1"} { + t.Run("ifname="+ifname+"/val="+val, func(t *testing.T) { + ctx := testutil.StartSpan(ctx, t) + runRes := container.RunAttach(ctx, t, c, + container.WithCmd("sysctl", "-qn", scName), + container.WithEndpointSettings(apinetwork.NetworkBridge, &apinetwork.EndpointSettings{ + DriverOpts: map[string]string{ + netlabel.EndpointSysctls: "net.ipv4.conf." + ifname + ".forwarding=" + val, + }, + }), + ) + defer c.ContainerRemove(ctx, runRes.ContainerID, containertypes.RemoveOptions{Force: true}) + + stdout := runRes.Stdout.String() + assert.Check(t, is.Equal(strings.TrimSpace(stdout), val)) + }) + } + } +} diff --git a/libnetwork/endpoint.go b/libnetwork/endpoint.go index 836313ccd3..1ee235f1dc 100644 --- a/libnetwork/endpoint.go +++ b/libnetwork/endpoint.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" "net" + "strings" "sync" "github.com/containerd/log" @@ -401,6 +402,18 @@ func (ep *Endpoint) Value() []byte { return b } +func (ep *Endpoint) getSysctls() []string { + ep.mu.Lock() + defer ep.mu.Unlock() + + if s, ok := ep.generic[netlabel.EndpointSysctls]; ok { + if ss, ok := s.(string); ok { + return strings.Split(ss, ",") + } + } + return nil +} + func (ep *Endpoint) SetValue(value []byte) error { return json.Unmarshal(value, ep) } diff --git a/libnetwork/netlabel/labels.go b/libnetwork/netlabel/labels.go index 91232af6aa..5e243b8520 100644 --- a/libnetwork/netlabel/labels.go +++ b/libnetwork/netlabel/labels.go @@ -26,6 +26,10 @@ const ( // DNSServers A list of DNS servers associated with the endpoint DNSServers = Prefix + ".endpoint.dnsservers" + // EndpointSysctls is a comma separated list interface-specific sysctls + // where the interface name is represented by the string "IFNAME". + EndpointSysctls = Prefix + ".endpoint.sysctls" + // EnableIPv6 constant represents enabling IPV6 at network level EnableIPv6 = Prefix + ".enable_ipv6" diff --git a/libnetwork/osl/interface_linux.go b/libnetwork/osl/interface_linux.go index 3491aac70c..1dec44dfd4 100644 --- a/libnetwork/osl/interface_linux.go +++ b/libnetwork/osl/interface_linux.go @@ -4,12 +4,16 @@ import ( "context" "fmt" "net" + "os" + "path/filepath" + "strings" "syscall" "time" "github.com/containerd/log" "github.com/docker/docker/libnetwork/ns" "github.com/docker/docker/libnetwork/types" + "github.com/pkg/errors" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" ) @@ -55,6 +59,7 @@ type Interface struct { llAddrs []*net.IPNet routes []*net.IPNet bridge bool + sysctls []string ns *Namespace } @@ -223,7 +228,7 @@ func (n *Namespace) AddInterface(srcName, dstPrefix string, options ...IfaceOpti } // Configure the interface now this is moved in the proper namespace. - if err := configureInterface(nlh, iface, i); err != nil { + if err := n.configureInterface(nlh, iface, i); err != nil { // If configuring the device fails move it back to the host namespace // and change the name back to the source name. This allows the caller // to properly cleanup the interface. Its important especially for @@ -312,7 +317,7 @@ func (n *Namespace) RemoveInterface(i *Interface) error { return nil } -func configureInterface(nlh *netlink.Handle, iface netlink.Link, i *Interface) error { +func (n *Namespace) configureInterface(nlh *netlink.Handle, iface netlink.Link, i *Interface) error { ifaceName := iface.Attrs().Name ifaceConfigurators := []struct { Fn func(*netlink.Handle, netlink.Link, *Interface) error @@ -331,6 +336,11 @@ func configureInterface(nlh *netlink.Handle, iface netlink.Link, i *Interface) e return fmt.Errorf("%s: %v", config.ErrMessage, err) } } + + if err := n.setSysctls(i.dstName, i.sysctls); err != nil { + return err + } + return nil } @@ -393,6 +403,42 @@ func setInterfaceLinkLocalIPs(nlh *netlink.Handle, iface netlink.Link, i *Interf return nil } +func (n *Namespace) setSysctls(ifName string, sysctls []string) error { + for _, sc := range sysctls { + k, v, found := strings.Cut(sc, "=") + if !found { + return fmt.Errorf("expected sysctl '%s' to have format name=value", sc) + } + sk := strings.Split(k, ".") + if len(sk) != 5 { + return fmt.Errorf("expected sysctl '%s' to have format net.X.Y.IFNAME.Z", sc) + } + + sysPath := filepath.Join(append([]string{"/proc/sys", sk[0], sk[1], sk[2], ifName}, sk[4:]...)...) + var errF error + f := func() { + if fi, err := os.Stat(sysPath); err != nil || !fi.Mode().IsRegular() { + errF = fmt.Errorf("%s is not a sysctl file", sysPath) + } else if curVal, err := os.ReadFile(sysPath); err != nil { + errF = errors.Wrapf(err, "unable to read '%s'", sysPath) + } else if strings.TrimSpace(string(curVal)) == v { + // The value is already correct, don't try to write the file in case + // "/proc/sys/net" is a read-only filesystem. + } else if err := os.WriteFile(sysPath, []byte(v), 0o644); err != nil { + errF = errors.Wrapf(err, "unable to write to '%s'", sysPath) + } + } + + if err := n.InvokeFunc(f); err != nil { + return errors.Wrapf(err, "failed to run sysctl setter in network namespace") + } + if errF != nil { + return errF + } + } + return nil +} + func setInterfaceName(nlh *netlink.Handle, iface netlink.Link, i *Interface) error { return nlh.LinkSetName(iface, i.DstName()) } diff --git a/libnetwork/osl/options_linux.go b/libnetwork/osl/options_linux.go index bc617c7b0a..f2e668f242 100644 --- a/libnetwork/osl/options_linux.go +++ b/libnetwork/osl/options_linux.go @@ -81,3 +81,11 @@ func WithRoutes(routes []*net.IPNet) IfaceOption { return nil } } + +// WithSysctls sets the interface sysctls. +func WithSysctls(sysctls []string) IfaceOption { + return func(i *Interface) error { + i.sysctls = sysctls + return nil + } +} diff --git a/libnetwork/sandbox_linux.go b/libnetwork/sandbox_linux.go index c34e96cce6..3cae9b9904 100644 --- a/libnetwork/sandbox_linux.go +++ b/libnetwork/sandbox_linux.go @@ -315,6 +315,9 @@ func (sb *Sandbox) populateNetworkResources(ep *Endpoint) error { if i.mac != nil { ifaceOptions = append(ifaceOptions, osl.WithMACAddress(i.mac)) } + if sysctls := ep.getSysctls(); len(sysctls) > 0 { + ifaceOptions = append(ifaceOptions, osl.WithSysctls(sysctls)) + } if err := sb.osSbox.AddInterface(i.srcName, i.dstPrefix, ifaceOptions...); err != nil { return fmt.Errorf("failed to add interface %s to sandbox: %v", i.srcName, err)