diff --git a/cli/command/container/opts.go b/cli/command/container/opts.go index 327c91e3adac..401492b1bef7 100644 --- a/cli/command/container/opts.go +++ b/cli/command/container/opts.go @@ -17,6 +17,7 @@ import ( networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/strslice" "github.com/docker/docker/api/types/versions" + "github.com/docker/docker/errdefs" "github.com/docker/docker/pkg/signal" "github.com/docker/go-connections/nat" "github.com/pkg/errors" @@ -654,67 +655,116 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con EndpointsConfig: make(map[string]*networktypes.EndpointSettings), } - if copts.ipv4Address != "" || copts.ipv6Address != "" || copts.linkLocalIPs.Len() > 0 { - epConfig := &networktypes.EndpointSettings{} - networkingConfig.EndpointsConfig[string(hostConfig.NetworkMode)] = epConfig - - epConfig.IPAMConfig = &networktypes.EndpointIPAMConfig{ - IPv4Address: copts.ipv4Address, - IPv6Address: copts.ipv6Address, - } - - if copts.linkLocalIPs.Len() > 0 { - epConfig.IPAMConfig.LinkLocalIPs = make([]string, copts.linkLocalIPs.Len()) - copy(epConfig.IPAMConfig.LinkLocalIPs, copts.linkLocalIPs.GetAll()) - } + networkingConfig.EndpointsConfig, err = parseNetworkOpts(copts) + if err != nil { + return nil, err } - if hostConfig.NetworkMode.IsUserDefined() && len(hostConfig.Links) > 0 { - epConfig := networkingConfig.EndpointsConfig[string(hostConfig.NetworkMode)] - if epConfig == nil { - epConfig = &networktypes.EndpointSettings{} - } - epConfig.Links = make([]string, len(hostConfig.Links)) - copy(epConfig.Links, hostConfig.Links) - networkingConfig.EndpointsConfig[string(hostConfig.NetworkMode)] = epConfig - } - value := copts.netMode.Value() + return &containerConfig{ + Config: config, + HostConfig: hostConfig, + NetworkingConfig: networkingConfig, + }, nil +} - if value != nil && hostConfig.NetworkMode.IsUserDefined() { - epConfig := networkingConfig.EndpointsConfig[string(hostConfig.NetworkMode)] - if epConfig == nil { - epConfig = &networktypes.EndpointSettings{} - } +// parseNetworkOpts converts --network advanced options to endpoint-specs, and combines +// them with the old --network-alias and --links. If returns an error if conflicting options +// are found. +// +// this function may return _multiple_ endpoints, which is not currently supported +// by the daemon, but may be in future; it's up to the daemon to produce an error +// in case that is not supported. +func parseNetworkOpts(copts *containerOptions) (map[string]*networktypes.EndpointSettings, error) { + var ( + endpoints = make(map[string]*networktypes.EndpointSettings, len(copts.netMode.Value())) + hasUserDefined, hasNonUserDefined bool + ) - if len(value[0].Aliases) > 0 { + for i, n := range copts.netMode.Value() { + if container.NetworkMode(n.Target).IsUserDefined() { + hasUserDefined = true + } else { + hasNonUserDefined = true + } + if i == 0 { + // The first network corresponds with what was previously the "only" + // network, and what would be used when using the non-advanced syntax + // `--network-alias`, `--link`, `--ip`, `--ip6`, and `--link-local-ip` + // are set on this network, to preserve backward compatibility with + // the non-advanced notation + // TODO should copts.MacAddress actually be set on the first network? (currently it's not) + + // TODO should we error if _any_ advanced option is used? (i.e. forbid to combine advanced notation with the "old" flags (`--network-alias`, `--link`, `--ip`, `--ip6`)? + if len(n.Aliases) > 0 && copts.aliases.Len() > 0 { + return nil, errdefs.InvalidParameter(errors.New("conflicting options: cannot specify both --network-alias and per-network alias")) + } + if len(n.Links) > 0 && copts.links.Len() > 0 { + return nil, errdefs.InvalidParameter(errors.New("conflicting options: cannot specify both --link and per-network links")) + } if copts.aliases.Len() > 0 { - return nil, fmt.Errorf("ambiguity in alias options provided") + n.Aliases = make([]string, copts.aliases.Len()) + copy(n.Aliases, copts.aliases.GetAll()) } - epConfig.Aliases = append(epConfig.Aliases, value[0].Aliases...) - } + if copts.links.Len() > 0 { + n.Links = make([]string, copts.links.Len()) + copy(n.Links, copts.links.GetAll()) + } + + // TODO add IPv4/IPv6 options to the csv notation for --network, and error-out in case of conflicting options + n.IPv4Address = copts.ipv4Address + n.IPv6Address = copts.ipv6Address - if len(value[0].DriverOpts) > 0 { - epConfig.DriverOpts = make(map[string]string) - epConfig.DriverOpts = value[0].DriverOpts + // TODO should linkLocalIPs be added to the _first_ network only, or to _all_ networks? (should this be a per-network option as well?) + if copts.linkLocalIPs.Len() > 0 { + n.LinkLocalIPs = make([]string, copts.linkLocalIPs.Len()) + copy(n.LinkLocalIPs, copts.linkLocalIPs.GetAll()) + } + } + ep, err := parseNetworkAttachmentOpt(n) + if err != nil { + return nil, err } - networkingConfig.EndpointsConfig[string(hostConfig.NetworkMode)] = epConfig + if _, ok := endpoints[n.Target]; ok { + return nil, errdefs.InvalidParameter(errors.Errorf("network %q is specified multiple times", n.Target)) + } + endpoints[n.Target] = ep + } + if hasUserDefined && hasNonUserDefined { + return nil, errdefs.InvalidParameter(errors.New("conflicting options: cannot attach both user-defined and non-user-defined network-modes")) } + return endpoints, nil +} - if copts.aliases.Len() > 0 { - epConfig := networkingConfig.EndpointsConfig[string(hostConfig.NetworkMode)] - if epConfig == nil { - epConfig = &networktypes.EndpointSettings{} +func parseNetworkAttachmentOpt(ep opts.NetworkAttachmentOpts) (*networktypes.EndpointSettings, error) { + if strings.TrimSpace(ep.Target) == "" { + return nil, errors.New("no name set for network") + } + if !container.NetworkMode(ep.Target).IsUserDefined() { + if len(ep.Aliases) > 0 { + return nil, errors.New("network-scoped aliases are only supported for user-defined networks") + } + if len(ep.Links) > 0 { + return nil, errors.New("links are only supported for user-defined networks") } - epConfig.Aliases = make([]string, copts.aliases.Len()) - copy(epConfig.Aliases, copts.aliases.GetAll()) - networkingConfig.EndpointsConfig[string(hostConfig.NetworkMode)] = epConfig } - return &containerConfig{ - Config: config, - HostConfig: hostConfig, - NetworkingConfig: networkingConfig, - }, nil + epConfig := &networktypes.EndpointSettings{} + epConfig.Aliases = append(epConfig.Aliases, ep.Aliases...) + if len(ep.DriverOpts) > 0 { + epConfig.DriverOpts = make(map[string]string) + epConfig.DriverOpts = ep.DriverOpts + } + if len(ep.Links) > 0 { + epConfig.Links = ep.Links + } + if ep.IPv4Address != "" || ep.IPv6Address != "" || len(ep.LinkLocalIPs) > 0 { + epConfig.IPAMConfig = &networktypes.EndpointIPAMConfig{ + IPv4Address: ep.IPv4Address, + IPv6Address: ep.IPv6Address, + LinkLocalIPs: ep.LinkLocalIPs, + } + } + return epConfig, nil } func parsePortOpts(publishOpts []string) ([]string, error) { diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go index 3a6aa5857054..bf6387162cb7 100644 --- a/cli/command/container/opts_test.go +++ b/cli/command/container/opts_test.go @@ -390,6 +390,139 @@ func TestParseDevice(t *testing.T) { } +func TestParseNetworkConfig(t *testing.T) { + tests := []struct { + name string + flags []string + expected map[string]*networktypes.EndpointSettings + expectedCfg container.HostConfig + expectedErr string + }{ + { + name: "single-network-legacy", + flags: []string{"--network", "net1"}, + expected: map[string]*networktypes.EndpointSettings{"net1": {}}, + expectedCfg: container.HostConfig{NetworkMode: "net1"}, + }, + { + name: "single-network-advanced", + flags: []string{"--network", "name=net1"}, + expected: map[string]*networktypes.EndpointSettings{"net1": {}}, + expectedCfg: container.HostConfig{NetworkMode: "net1"}, + }, + { + name: "single-network-legacy-with-options", + flags: []string{ + "--ip", "172.20.88.22", + "--ip6", "2001:db8::8822", + "--link", "foo:bar", + "--link", "bar:baz", + "--link-local-ip", "169.254.2.2", + "--link-local-ip", "fe80::169:254:2:2", + "--network", "name=net1", + "--network-alias", "web1", + "--network-alias", "web2", + }, + expected: map[string]*networktypes.EndpointSettings{ + "net1": { + IPAMConfig: &networktypes.EndpointIPAMConfig{ + IPv4Address: "172.20.88.22", + IPv6Address: "2001:db8::8822", + LinkLocalIPs: []string{"169.254.2.2", "fe80::169:254:2:2"}, + }, + Links: []string{"foo:bar", "bar:baz"}, + Aliases: []string{"web1", "web2"}, + }, + }, + expectedCfg: container.HostConfig{NetworkMode: "net1"}, + }, + { + name: "multiple-network-advanced-mixed", + flags: []string{ + "--ip", "172.20.88.22", + "--ip6", "2001:db8::8822", + "--link", "foo:bar", + "--link", "bar:baz", + "--link-local-ip", "169.254.2.2", + "--link-local-ip", "fe80::169:254:2:2", + "--network", "name=net1,driver-opt=field1=value1", + "--network-alias", "web1", + "--network-alias", "web2", + "--network", "net2", + "--network", "name=net3,alias=web3,driver-opt=field3=value3", + }, + expected: map[string]*networktypes.EndpointSettings{ + "net1": { + DriverOpts: map[string]string{"field1": "value1"}, + IPAMConfig: &networktypes.EndpointIPAMConfig{ + IPv4Address: "172.20.88.22", + IPv6Address: "2001:db8::8822", + LinkLocalIPs: []string{"169.254.2.2", "fe80::169:254:2:2"}, + }, + Links: []string{"foo:bar", "bar:baz"}, + Aliases: []string{"web1", "web2"}, + }, + "net2": {}, + "net3": { + DriverOpts: map[string]string{"field3": "value3"}, + Aliases: []string{"web3"}, + }, + }, + expectedCfg: container.HostConfig{NetworkMode: "net1"}, + }, + { + name: "single-network-advanced-with-options", + flags: []string{"--network", "name=net1,alias=web1,alias=web2,driver-opt=field1=value1,driver-opt=field2=value2"}, + expected: map[string]*networktypes.EndpointSettings{ + "net1": { + DriverOpts: map[string]string{ + "field1": "value1", + "field2": "value2", + }, + Aliases: []string{"web1", "web2"}, + }, + }, + expectedCfg: container.HostConfig{NetworkMode: "net1"}, + }, + { + name: "multiple-networks", + flags: []string{"--network", "net1", "--network", "name=net2"}, + expected: map[string]*networktypes.EndpointSettings{"net1": {}, "net2": {}}, + expectedCfg: container.HostConfig{NetworkMode: "net1"}, + }, + { + name: "conflict-network", + flags: []string{"--network", "duplicate", "--network", "name=duplicate"}, + expectedErr: `network "duplicate" is specified multiple times`, + }, + { + name: "conflict-options", + flags: []string{"--network", "name=net1,alias=web1", "--network-alias", "web1"}, + expectedErr: `conflicting options: cannot specify both --network-alias and per-network alias`, + }, + { + name: "invalid-mixed-network-types", + flags: []string{"--network", "name=host", "--network", "net1"}, + expectedErr: `conflicting options: cannot attach both user-defined and non-user-defined network-modes`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, hConfig, nwConfig, err := parseRun(tc.flags) + + if tc.expectedErr != "" { + assert.Error(t, err, tc.expectedErr) + return + } + + assert.NilError(t, err) + assert.DeepEqual(t, hConfig.NetworkMode, tc.expectedCfg.NetworkMode) + assert.DeepEqual(t, nwConfig.EndpointsConfig, tc.expected) + }) + } +} + func TestParseModes(t *testing.T) { // pid ko flags, copts := setupRunFlags() diff --git a/cli/command/network/connect.go b/cli/command/network/connect.go index a5a4e12408c0..04108e20fadb 100644 --- a/cli/command/network/connect.go +++ b/cli/command/network/connect.go @@ -2,7 +2,6 @@ package network import ( "context" - "fmt" "strings" diff --git a/opts/network.go b/opts/network.go index 3026a7efee67..4f5b53b1b60d 100644 --- a/opts/network.go +++ b/opts/network.go @@ -15,9 +15,13 @@ const ( // NetworkAttachmentOpts represents the network options for endpoint creation type NetworkAttachmentOpts struct { - Target string - Aliases []string - DriverOpts map[string]string + Target string + Aliases []string + DriverOpts map[string]string + Links []string // TODO add support for links in the csv notation of `--network` + IPv4Address string // TODO add support for IPv4-address in the csv notation of `--network` + IPv6Address string // TODO add support for IPv6-address in the csv notation of `--network` + LinkLocalIPs []string // TODO add support for LinkLocalIPs in the csv notation of `--network` ? } // NetworkOpt represents a network config in swarm mode.