Check, then assume an IPv6 bridge has a subnet.

If IPv6 is enabled for a bridge network, by the time configuration
is applied, the bridge will always have an address. Assert that, by
raising an error when the configuration is validated.

Use that to simplify the logic used to calculate which addresses
should be assigned to a bridge. Also remove a redundant check in
setupGatewayIPv6() and the error associated with it.

Fix unit tests that enabled IPv6, but didn't supply an IPv6 IPAM
address/pool. Before this change, these tests passed but silently
left the bridge without an IPv6 address.

(The daemon already ensured there was an IPv6 address, this change
does not add a new restriction on config at that level.)

Signed-off-by: Rob Murray <rob.murray@docker.com>

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 {

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)
 	}

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
 

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 {

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"})
 }

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)
 	}

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())

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{}
 	}

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)
 	}