diff --git a/libnetwork/drivers/bridge/bridge_linux.go b/libnetwork/drivers/bridge/bridge_linux.go index 0f9d58d167..700a0e7fde 100644 --- a/libnetwork/drivers/bridge/bridge_linux.go +++ b/libnetwork/drivers/bridge/bridge_linux.go @@ -226,10 +226,6 @@ func (c *networkConfiguration) Validate() error { return ErrInvalidMtu(c.Mtu) } - if err := validateIPv6Subnet(c.AddressIPv6); err != nil { - return err - } - // If bridge v4 subnet is specified if c.AddressIPv4 != nil { // If default gw is specified, it must be part of bridge subnet @@ -240,12 +236,21 @@ func (c *networkConfiguration) Validate() error { } } - // If default v6 gw is specified, AddressIPv6 must be specified and gw must belong to AddressIPv6 subnet - if c.EnableIPv6 && c.DefaultGatewayIPv6 != nil { - if c.AddressIPv6 == nil || !c.AddressIPv6.Contains(c.DefaultGatewayIPv6) { + if c.EnableIPv6 { + // If IPv6 is enabled, AddressIPv6 must have been configured. + if c.AddressIPv6 == nil { + return errdefs.System(errors.New("no IPv6 address was allocated for the bridge")) + } + // AddressIPv6 must be IPv6, and not overlap with the LL subnet prefix. + if err := validateIPv6Subnet(c.AddressIPv6); err != nil { + return err + } + // If a default gw is specified, it must belong to AddressIPv6's subnet + if c.DefaultGatewayIPv6 != nil && !c.AddressIPv6.Contains(c.DefaultGatewayIPv6) { return &ErrInvalidGateway{} } } + return nil } @@ -791,7 +796,7 @@ func (d *driver) createNetwork(config *networkConfiguration) (err error) { // Even if a bridge exists try to setup IPv4. bridgeSetup.queueStep(setupBridgeIPv4) - enableIPv6Forwarding := d.config.EnableIPForwarding && config.AddressIPv6 != nil + enableIPv6Forwarding := config.EnableIPv6 && d.config.EnableIPForwarding // Conditionally queue setup steps depending on configuration values. for _, step := range []struct { diff --git a/libnetwork/drivers/bridge/bridge_linux_test.go b/libnetwork/drivers/bridge/bridge_linux_test.go index 1f9bd51231..65fddd3a6d 100644 --- a/libnetwork/drivers/bridge/bridge_linux_test.go +++ b/libnetwork/drivers/bridge/bridge_linux_test.go @@ -218,6 +218,15 @@ func getIPv4Data(t *testing.T) []driverapi.IPAMData { return []driverapi.IPAMData{ipd} } +func getIPv6Data(t *testing.T) []driverapi.IPAMData { + ipd := driverapi.IPAMData{AddressSpace: "full"} + // There's no default IPv6 address pool, so use an arbitrary unique-local prefix. + addr, nw, _ := net.ParseCIDR("fdcd:d1b1:99d2:abcd::1/64") + ipd.Pool = nw + ipd.Gateway = &net.IPNet{IP: addr, Mask: nw.Mask} + return []driverapi.IPAMData{ipd} +} + func TestCreateFullOptions(t *testing.T) { defer netnsutils.SetupTestOSContext(t)() d := newDriver() @@ -254,7 +263,7 @@ func TestCreateFullOptions(t *testing.T) { AuxAddresses: map[string]*net.IPNet{DefaultGatewayV4AuxKey: defgw}, }, } - err := d.CreateNetwork("dummy", netOption, nil, ipdList, nil) + err := d.CreateNetwork("dummy", netOption, nil, ipdList, getIPv6Data(t)) if err != nil { t.Fatalf("Failed to create bridge: %v", err) } diff --git a/libnetwork/drivers/bridge/errors.go b/libnetwork/drivers/bridge/errors.go index eda1bcb432..8360b3226f 100644 --- a/libnetwork/drivers/bridge/errors.go +++ b/libnetwork/drivers/bridge/errors.go @@ -84,16 +84,6 @@ func (eig *ErrInvalidGateway) Error() string { // InvalidParameter denotes the type of this error func (eig *ErrInvalidGateway) InvalidParameter() {} -// ErrInvalidContainerSubnet is returned when the container subnet (FixedCIDR) is not valid. -type ErrInvalidContainerSubnet struct{} - -func (eis *ErrInvalidContainerSubnet) Error() string { - return "container subnet must be a subset of bridge network" -} - -// InvalidParameter denotes the type of this error -func (eis *ErrInvalidContainerSubnet) InvalidParameter() {} - // ErrInvalidMtu is returned when the user provided MTU is not valid. type ErrInvalidMtu int diff --git a/libnetwork/drivers/bridge/interface_linux.go b/libnetwork/drivers/bridge/interface_linux.go index add9c231b5..2a9c375522 100644 --- a/libnetwork/drivers/bridge/interface_linux.go +++ b/libnetwork/drivers/bridge/interface_linux.go @@ -70,39 +70,28 @@ func (i *bridgeInterface) addresses(family int) ([]netlink.Addr, error) { return addrs, nil } -func getRequiredIPv6Addrs(config *networkConfiguration) (requiredAddrs map[netip.Addr]netip.Prefix, addr *net.IPNet, gateway net.IP, err error) { +func getRequiredIPv6Addrs(config *networkConfiguration) (requiredAddrs map[netip.Addr]netip.Prefix, err error) { requiredAddrs = make(map[netip.Addr]netip.Prefix) - // TODO(robmry) - is config.AddressIPv6 always set at this point? - // The logic here is preserved from the original setupBridgeIPv6(), but - // can probably be simplified. - // Always give the bridge 'fe80::1' - every interface is required to have an // address in 'fe80::/64'. Linux may assign an address, but we'll replace it with // 'fe80::1'. Then, if the configured prefix is 'fe80::/64', the IPAM pool // assigned address will not be a second address in the LL subnet. - addr = bridgeIPv6 - gateway = bridgeIPv6.IP ra, ok := netiputil.ToPrefix(bridgeIPv6) if !ok { err = fmt.Errorf("Failed to convert Link-Local IPv6 address to netip.Prefix") - return nil, nil, net.IP{}, err + return nil, err } requiredAddrs[ra.Addr()] = ra - // Set up the user-specified bridge address and gateway. - if config.AddressIPv6 != nil { - addr = config.AddressIPv6 - gateway = config.AddressIPv6.IP - ra, ok := netiputil.ToPrefix(config.AddressIPv6) - if !ok { - err = fmt.Errorf("failed to convert bridge IPv6 address '%s' to netip.Prefix", config.AddressIPv6.String()) - return nil, nil, net.IP{}, err - } - requiredAddrs[ra.Addr()] = ra + ra, ok = netiputil.ToPrefix(config.AddressIPv6) + if !ok { + err = fmt.Errorf("failed to convert bridge IPv6 address '%s' to netip.Prefix", config.AddressIPv6.String()) + return nil, err } + requiredAddrs[ra.Addr()] = ra - return requiredAddrs, addr, gateway, nil + return requiredAddrs, nil } func (i *bridgeInterface) programIPv6Addresses(config *networkConfiguration) error { @@ -114,10 +103,12 @@ func (i *bridgeInterface) programIPv6Addresses(config *networkConfiguration) err // Get the list of required IPv6 addresses for this bridge. var requiredAddrs map[netip.Addr]netip.Prefix - requiredAddrs, i.bridgeIPv6, i.gatewayIPv6, err = getRequiredIPv6Addrs(config) + requiredAddrs, err = getRequiredIPv6Addrs(config) if err != nil { return errdefs.System(err) } + i.bridgeIPv6 = config.AddressIPv6 + i.gatewayIPv6 = config.AddressIPv6.IP // Remove addresses that aren't required. for _, existingAddr := range existingAddrs { diff --git a/libnetwork/drivers/bridge/interface_linux_test.go b/libnetwork/drivers/bridge/interface_linux_test.go index 734c69b4dd..557185eca8 100644 --- a/libnetwork/drivers/bridge/interface_linux_test.go +++ b/libnetwork/drivers/bridge/interface_linux_test.go @@ -130,10 +130,8 @@ func TestGetRequiredIPv6Addrs(t *testing.T) { expResult[netip.MustParseAddr(strings.Split(addr, "/")[0])] = netip.MustParsePrefix(addr) } - reqd, addr, gw, err := getRequiredIPv6Addrs(config) + reqd, err := getRequiredIPv6Addrs(config) assert.Check(t, is.Nil(err)) - assert.Check(t, is.DeepEqual(addr, config.AddressIPv6)) - assert.Check(t, is.DeepEqual(gw, config.AddressIPv6.IP)) assert.Check(t, is.DeepEqual(reqd, expResult, cmp.Comparer(func(a, b netip.Prefix) bool { return a == b }))) }) @@ -143,7 +141,7 @@ func TestGetRequiredIPv6Addrs(t *testing.T) { func TestProgramIPv6Addresses(t *testing.T) { defer netnsutils.SetupTestOSContext(t)() - checkAddrs := func(i *bridgeInterface, expAddrs []string) { + checkAddrs := func(i *bridgeInterface, nc *networkConfiguration, expAddrs []string) { t.Helper() exp := []netlink.Addr{} for _, a := range expAddrs { @@ -153,6 +151,8 @@ func TestProgramIPv6Addresses(t *testing.T) { actual, err := i.addresses(netlink.FAMILY_V6) assert.NilError(t, err) assert.DeepEqual(t, exp, actual) + assert.Check(t, is.DeepEqual(i.bridgeIPv6, nc.AddressIPv6)) + assert.Check(t, is.DeepEqual(i.gatewayIPv6, nc.AddressIPv6.IP)) } nc := &networkConfiguration{} @@ -163,7 +163,7 @@ func TestProgramIPv6Addresses(t *testing.T) { nc.AddressIPv6 = cidrToIPNet(t, "2000:3000::1/64") err := i.programIPv6Addresses(nc) assert.NilError(t, err) - checkAddrs(i, []string{"2000:3000::1/64", "fe80::1/64"}) + checkAddrs(i, nc, []string{"2000:3000::1/64", "fe80::1/64"}) // Shrink the subnet of that regular address, the prefix length of the address // will not be modified - but it's informational-only, the address itself has @@ -171,18 +171,18 @@ func TestProgramIPv6Addresses(t *testing.T) { nc.AddressIPv6 = cidrToIPNet(t, "2000:3000::1/80") err = i.programIPv6Addresses(nc) assert.NilError(t, err) - checkAddrs(i, []string{"2000:3000::1/64", "fe80::1/64"}) + checkAddrs(i, nc, []string{"2000:3000::1/64", "fe80::1/64"}) // Ask for link-local only, by specifying an address with the Link Local prefix. // The regular address should be removed. nc.AddressIPv6 = cidrToIPNet(t, "fe80::1/64") err = i.programIPv6Addresses(nc) assert.NilError(t, err) - checkAddrs(i, []string{"fe80::1/64"}) + checkAddrs(i, nc, []string{"fe80::1/64"}) // Swap the standard link local address for a nonstandard one. nc.AddressIPv6 = cidrToIPNet(t, "fe80:5555::1/55") err = i.programIPv6Addresses(nc) assert.NilError(t, err) - checkAddrs(i, []string{"fe80:5555::1/55", "fe80::1/64"}) + checkAddrs(i, nc, []string{"fe80:5555::1/55", "fe80::1/64"}) } diff --git a/libnetwork/drivers/bridge/network_linux_test.go b/libnetwork/drivers/bridge/network_linux_test.go index d057e86ba0..fc7bd21b32 100644 --- a/libnetwork/drivers/bridge/network_linux_test.go +++ b/libnetwork/drivers/bridge/network_linux_test.go @@ -27,7 +27,7 @@ func TestLinkCreate(t *testing.T) { genericOption[netlabel.GenericData] = config ipdList := getIPv4Data(t) - err := d.CreateNetwork("dummy", genericOption, nil, ipdList, nil) + err := d.CreateNetwork("dummy", genericOption, nil, ipdList, getIPv6Data(t)) if err != nil { t.Fatalf("Failed to create bridge: %v", err) } @@ -120,7 +120,7 @@ func TestLinkCreateTwo(t *testing.T) { genericOption[netlabel.GenericData] = config ipdList := getIPv4Data(t) - err := d.CreateNetwork("dummy", genericOption, nil, ipdList, nil) + err := d.CreateNetwork("dummy", genericOption, nil, ipdList, getIPv6Data(t)) if err != nil { t.Fatalf("Failed to create bridge: %v", err) } @@ -157,7 +157,7 @@ func TestLinkCreateNoEnableIPv6(t *testing.T) { genericOption[netlabel.GenericData] = config ipdList := getIPv4Data(t) - err := d.CreateNetwork("dummy", genericOption, nil, ipdList, nil) + err := d.CreateNetwork("dummy", genericOption, nil, ipdList, getIPv6Data(t)) if err != nil { t.Fatalf("Failed to create bridge: %v", err) } @@ -193,7 +193,7 @@ func TestLinkDelete(t *testing.T) { genericOption[netlabel.GenericData] = config ipdList := getIPv4Data(t) - err := d.CreateNetwork("dummy", genericOption, nil, ipdList, nil) + err := d.CreateNetwork("dummy", genericOption, nil, ipdList, getIPv6Data(t)) if err != nil { t.Fatalf("Failed to create bridge: %v", err) } diff --git a/libnetwork/drivers/bridge/port_mapping_linux_test.go b/libnetwork/drivers/bridge/port_mapping_linux_test.go index 6724cbbcc0..58d5c3ec63 100644 --- a/libnetwork/drivers/bridge/port_mapping_linux_test.go +++ b/libnetwork/drivers/bridge/port_mapping_linux_test.go @@ -37,13 +37,13 @@ func TestPortMappingConfig(t *testing.T) { netOptions := make(map[string]interface{}) netOptions[netlabel.GenericData] = netConfig - ipdList := getIPv4Data(t) - err := d.CreateNetwork("dummy", netOptions, nil, ipdList, nil) + ipdList4 := getIPv4Data(t) + err := d.CreateNetwork("dummy", netOptions, nil, ipdList4, getIPv6Data(t)) if err != nil { t.Fatalf("Failed to create bridge: %v", err) } - te := newTestEndpoint(ipdList[0].Pool, 11) + te := newTestEndpoint(ipdList4[0].Pool, 11) err = d.CreateEndpoint("dummy", "ep1", te.Interface(), nil) if err != nil { t.Fatalf("Failed to create the endpoint: %s", err.Error()) @@ -122,13 +122,13 @@ func TestPortMappingV6Config(t *testing.T) { netOptions := make(map[string]interface{}) netOptions[netlabel.GenericData] = netConfig - ipdList := getIPv4Data(t) - err := d.CreateNetwork("dummy", netOptions, nil, ipdList, nil) + ipdList4 := getIPv4Data(t) + err := d.CreateNetwork("dummy", netOptions, nil, ipdList4, getIPv6Data(t)) if err != nil { t.Fatalf("Failed to create bridge: %v", err) } - te := newTestEndpoint(ipdList[0].Pool, 11) + te := newTestEndpoint(ipdList4[0].Pool, 11) err = d.CreateEndpoint("dummy", "ep1", te.Interface(), nil) if err != nil { t.Fatalf("Failed to create the endpoint: %s", err.Error()) diff --git a/libnetwork/drivers/bridge/setup_ipv6_linux.go b/libnetwork/drivers/bridge/setup_ipv6_linux.go index 08ce0f4397..779306f35b 100644 --- a/libnetwork/drivers/bridge/setup_ipv6_linux.go +++ b/libnetwork/drivers/bridge/setup_ipv6_linux.go @@ -53,9 +53,6 @@ func setupBridgeIPv6(config *networkConfiguration, i *bridgeInterface) error { } func setupGatewayIPv6(config *networkConfiguration, i *bridgeInterface) error { - if config.AddressIPv6 == nil { - return &ErrInvalidContainerSubnet{} - } if !config.AddressIPv6.Contains(config.DefaultGatewayIPv6) { return &ErrInvalidGateway{} } diff --git a/libnetwork/drivers/bridge/setup_ipv6_linux_test.go b/libnetwork/drivers/bridge/setup_ipv6_linux_test.go index 5ad9e13820..278da7745c 100644 --- a/libnetwork/drivers/bridge/setup_ipv6_linux_test.go +++ b/libnetwork/drivers/bridge/setup_ipv6_linux_test.go @@ -21,6 +21,8 @@ func TestSetupIPv6(t *testing.T) { defer nh.Close() config, br := setupTestInterface(t, nh) + addr, nw, _ := net.ParseCIDR("fdcc:949:6399:1234::1/64") + config.AddressIPv6 = &net.IPNet{IP: addr, Mask: nw.Mask} if err := setupBridgeIPv6(config, br); err != nil { t.Fatalf("Failed to setup bridge IPv6: %v", err) }