Don't allow passing EnableIPv6 as a driver option (a label)

aidanhs · aidanhs · commit ee0f81b142e3 · 2016-01-30T16:01:29.000Z
Signed-off-by: Aidan Hobson Sayers &lt;aidanhs@cantab.net&gt;
diff --git a/api/api.go b/api/api.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"strconv"
 	"strings"
 
 	"github.com/docker/libnetwork"
@@ -276,22 +277,33 @@ func procCreateNetwork(c libnetwork.NetworkController, vars map[string]string, b
 
 	err := json.Unmarshal(body, &create)
 	if err != nil {
-		return "", &responseStatus{Status: "Invalid body: " + err.Error(), StatusCode: http.StatusBadRequest}
+		return nil, &responseStatus{Status: "Invalid body: " + err.Error(), StatusCode: http.StatusBadRequest}
 	}
 	processCreateDefaults(c, &create)
 
 	options := []libnetwork.NetworkOption{}
-	if len(create.NetworkOpts) > 0 {
-		if _, ok := create.NetworkOpts[netlabel.Internal]; ok {
+	if val, ok := create.NetworkOpts[netlabel.Internal]; ok {
+		internal, err := strconv.ParseBool(val)
+		if err != nil {
+			return nil, &responseStatus{Status: err.Error(), StatusCode: http.StatusBadRequest}
+		}
+		if internal {
 			options = append(options, libnetwork.NetworkOptionInternalNetwork())
 		}
 	}
+	if val, ok := create.NetworkOpts[netlabel.EnableIPv6]; ok {
+		enableIPv6, err := strconv.ParseBool(val)
+		if err != nil {
+			return nil, &responseStatus{Status: err.Error(), StatusCode: http.StatusBadRequest}
+		}
+		options = append(options, libnetwork.NetworkOptionEnableIPv6(enableIPv6))
+	}
 	if len(create.DriverOpts) > 0 {
 		options = append(options, libnetwork.NetworkOptionDriverOpts(create.DriverOpts))
 	}
 	nw, err := c.NewNetwork(create.NetworkType, create.Name, options...)
 	if err != nil {
-		return "", convertNetworkError(err)
+		return nil, convertNetworkError(err)
 	}
 
 	return nw.ID(), &createdResponse
diff --git a/api/api_test.go b/api/api_test.go
@@ -225,11 +225,13 @@ func TestCreateDeleteNetwork(t *testing.T) {
 		t.Fatalf("Expected StatusBadRequest status code, got: %v", errRsp)
 	}
 
-	ops := map[string]string{
-		bridge.BridgeName:   "abc",
+	dops := map[string]string{
+		bridge.BridgeName: "abc",
+	}
+	nops := map[string]string{
 		netlabel.EnableIPv6: "true",
 	}
-	nc := networkCreate{Name: "network_1", NetworkType: bridgeNetType, DriverOpts: ops}
+	nc := networkCreate{Name: "network_1", NetworkType: bridgeNetType, DriverOpts: dops, NetworkOpts: nops}
 	goodBody, err := json.Marshal(nc)
 	if err != nil {
 		t.Fatal(err)
@@ -257,29 +259,6 @@ func TestCreateDeleteNetwork(t *testing.T) {
 	if errRsp != &successResponse {
 		t.Fatalf("Unexepected failure: %v", errRsp)
 	}
-
-	// Create with labels
-	labels := map[string]string{
-		netlabel.EnableIPv6: "true",
-		bridge.BridgeName:   "abc",
-	}
-	nc = networkCreate{Name: "network_2", NetworkType: bridgeNetType, DriverOpts: labels}
-	goodBody, err = json.Marshal(nc)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_, errRsp = procCreateNetwork(c, vars, goodBody)
-	if errRsp != &createdResponse {
-		t.Fatalf("Unexepected failure: %v", errRsp)
-	}
-
-	vars[urlNwName] = "network_2"
-	_, errRsp = procDeleteNetwork(c, vars, nil)
-	if errRsp != &successResponse {
-		t.Fatalf("Unexepected failure: %v", errRsp)
-	}
-
 }
 
 func TestGetNetworksAndEndpoints(t *testing.T) {
@@ -1830,14 +1809,16 @@ func TestEndToEnd(t *testing.T) {
 
 	handleRequest := NewHTTPHandler(c)
 
-	ops := map[string]string{
-		bridge.BridgeName:   "cdef",
+	dops := map[string]string{
+		bridge.BridgeName:  "cdef",
+		netlabel.DriverMTU: "1460",
+	}
+	nops := map[string]string{
 		netlabel.EnableIPv6: "true",
-		netlabel.DriverMTU:  "1460",
 	}
 
 	// Create network
-	nc := networkCreate{Name: "network-fiftyfive", NetworkType: bridgeNetType, DriverOpts: ops}
+	nc := networkCreate{Name: "network-fiftyfive", NetworkType: bridgeNetType, DriverOpts: dops, NetworkOpts: nops}
 	body, err := json.Marshal(nc)
 	if err != nil {
 		t.Fatal(err)
diff --git a/client/network.go b/client/network.go
@@ -43,6 +43,7 @@ func (cli *NetworkCli) CmdNetworkCreate(chain string, args ...string) error {
 	cmd := cli.Subcmd(chain, "create", "NETWORK-NAME", "Creates a new network with a name specified by the user", false)
 	flDriver := cmd.String([]string{"d", "-driver"}, "", "Driver to manage the Network")
 	flInternal := cmd.Bool([]string{"-internal"}, false, "Config the network to be internal")
+	flIPv6 := cmd.Bool([]string{"-ipv6"}, false, "Enable IPv6 on the network")
 	cmd.Require(flag.Exact, 1)
 	err := cmd.ParseFlags(args, true)
 	if err != nil {
@@ -52,6 +53,9 @@ func (cli *NetworkCli) CmdNetworkCreate(chain string, args ...string) error {
 	if *flInternal {
 		networkOpts[netlabel.Internal] = "true"
 	}
+	if *flIPv6 {
+		networkOpts[netlabel.EnableIPv6] = "true"
+	}
 	// Construct network create request body
 	var driverOpts []string
 	nc := networkCreate{Name: cmd.Arg(0), NetworkType: *flDriver, DriverOpts: driverOpts, NetworkOpts: networkOpts}
diff --git a/default_gateway_linux.go b/default_gateway_linux.go
@@ -5,8 +5,6 @@ import (
 	"strconv"
 
 	"github.com/docker/libnetwork/drivers/bridge"
-	"github.com/docker/libnetwork/netlabel"
-	"github.com/docker/libnetwork/options"
 )
 
 func (c *controller) createGWNetwork() (Network, error) {
@@ -17,10 +15,9 @@ func (c *controller) createGWNetwork() (Network, error) {
 	}
 
 	n, err := c.NewNetwork("bridge", libnGWNetwork,
-		NetworkOptionGeneric(options.Generic{
-			netlabel.GenericData: netOption,
-			netlabel.EnableIPv6:  false,
-		}))
+		NetworkOptionDriverOpts(netOption),
+		NetworkOptionEnableIPv6(false),
+	)
 
 	if err != nil {
 		return nil, fmt.Errorf("error creating external connectivity network: %v", err)
diff --git a/drivers/bridge/bridge_test.go b/drivers/bridge/bridge_test.go
@@ -45,10 +45,6 @@ func TestCreateFullOptions(t *testing.T) {
 	br, _ := types.ParseCIDR("172.16.0.1/16")
 	defgw, _ := types.ParseCIDR("172.16.0.100/16")
 
-	netConfig := &networkConfiguration{
-		BridgeName: DefaultBridgeName,
-		EnableIPv6: true,
-	}
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = config
 
@@ -57,7 +53,10 @@ func TestCreateFullOptions(t *testing.T) {
 	}
 
 	netOption := make(map[string]interface{})
-	netOption[netlabel.GenericData] = netConfig
+	netOption[netlabel.EnableIPv6] = true
+	netOption[netlabel.GenericData] = &networkConfiguration{
+		BridgeName: DefaultBridgeName,
+	}
 
 	ipdList := []driverapi.IPAMData{
 		driverapi.IPAMData{
@@ -118,15 +117,15 @@ func TestCreateFullOptionsLabels(t *testing.T) {
 	gwV6, _ := types.ParseCIDR(gwV6s)
 
 	labels := map[string]string{
-		BridgeName:          DefaultBridgeName,
-		DefaultBridge:       "true",
-		netlabel.EnableIPv6: "true",
-		EnableICC:           "true",
-		EnableIPMasquerade:  "true",
-		DefaultBindingIP:    bndIPs,
+		BridgeName:         DefaultBridgeName,
+		DefaultBridge:      "true",
+		EnableICC:          "true",
+		EnableIPMasquerade: "true",
+		DefaultBindingIP:   bndIPs,
 	}
 
 	netOption := make(map[string]interface{})
+	netOption[netlabel.EnableIPv6] = true
 	netOption[netlabel.GenericData] = labels
 
 	ipdList := getIPv4Data(t)
@@ -783,13 +782,13 @@ func TestSetDefaultGw(t *testing.T) {
 
 	config := &networkConfiguration{
 		BridgeName:         DefaultBridgeName,
-		EnableIPv6:         true,
 		AddressIPv6:        subnetv6,
 		DefaultGatewayIPv4: gw4,
 		DefaultGatewayIPv6: gw6,
 	}
 
 	genericOption := make(map[string]interface{})
+	genericOption[netlabel.EnableIPv6] = true
 	genericOption[netlabel.GenericData] = config
 
 	err := d.CreateNetwork("dummy", genericOption, ipdList, nil)
diff --git a/libnetwork_test.go b/libnetwork_test.go
@@ -269,9 +269,9 @@ func TestBridge(t *testing.T) {
 	}
 
 	netOption := options.Generic{
+		netlabel.EnableIPv6: true,
 		netlabel.GenericData: options.Generic{
 			"BridgeName":         "testnetwork",
-			"EnableIPv6":         true,
 			"EnableICC":          true,
 			"EnableIPMasquerade": true,
 		},
@@ -323,7 +323,6 @@ func TestBridgeIpv6FromMac(t *testing.T) {
 	netOption := options.Generic{
 		netlabel.GenericData: options.Generic{
 			"BridgeName":         "testipv6mac",
-			"EnableIPv6":         true,
 			"EnableICC":          true,
 			"EnableIPMasquerade": true,
 		},
@@ -333,6 +332,7 @@ func TestBridgeIpv6FromMac(t *testing.T) {
 
 	network, err := controller.NewNetwork(bridgeNetType, "testipv6mac",
 		libnetwork.NetworkOptionGeneric(netOption),
+		libnetwork.NetworkOptionEnableIPv6(true),
 		libnetwork.NetworkOptionIpam(ipamapi.DefaultIPAM, "", ipamV4ConfList, ipamV6ConfList, nil),
 		libnetwork.NetworkOptionDeferIPv6Alloc(true))
 	if err != nil {
@@ -1008,14 +1008,14 @@ func TestEndpointJoin(t *testing.T) {
 	netOption := options.Generic{
 		netlabel.GenericData: options.Generic{
 			"BridgeName":         "testnetwork1",
-			"EnableIPv6":         true,
 			"EnableICC":          true,
 			"EnableIPMasquerade": true,
 		},
 	}
 	ipamV6ConfList := []*libnetwork.IpamConf{&libnetwork.IpamConf{PreferredPool: "fe90::/64", Gateway: "fe90::22"}}
 	n1, err := controller.NewNetwork(bridgeNetType, "testnetwork1",
 		libnetwork.NetworkOptionGeneric(netOption),
+		libnetwork.NetworkOptionEnableIPv6(true),
 		libnetwork.NetworkOptionIpam(ipamapi.DefaultIPAM, "", nil, ipamV6ConfList, nil),
 		libnetwork.NetworkOptionDeferIPv6Alloc(true))
 	if err != nil {
diff --git a/network.go b/network.go
@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"net"
-	"strconv"
 	"strings"
 	"sync"
 
@@ -476,9 +475,17 @@ type NetworkOption func(n *network)
 // in a Dictionary of Key-Value pair
 func NetworkOptionGeneric(generic map[string]interface{}) NetworkOption {
 	return func(n *network) {
-		n.generic = generic
-		if _, ok := generic[netlabel.EnableIPv6]; ok {
-			n.enableIPv6 = generic[netlabel.EnableIPv6].(bool)
+		if n.generic == nil {
+			n.generic = make(map[string]interface{})
+		}
+		if val, ok := generic[netlabel.EnableIPv6]; ok {
+			n.enableIPv6 = val.(bool)
+		}
+		if val, ok := generic[netlabel.Internal]; ok {
+			n.internal = val.(bool)
+		}
+		for k, v := range generic {
+			n.generic[k] = v
 		}
 	}
 }
@@ -490,14 +497,25 @@ func NetworkOptionPersist(persist bool) NetworkOption {
 	}
 }
 
+// NetworkOptionEnableIPv6 returns an option setter to explicitly configure IPv6
+func NetworkOptionEnableIPv6(enableIPv6 bool) NetworkOption {
+	return func(n *network) {
+		if n.generic == nil {
+			n.generic = make(map[string]interface{})
+		}
+		n.enableIPv6 = enableIPv6
+		n.generic[netlabel.EnableIPv6] = enableIPv6
+	}
+}
+
 // NetworkOptionInternalNetwork returns an option setter to config the network
 // to be internal which disables default gateway service
 func NetworkOptionInternalNetwork() NetworkOption {
 	return func(n *network) {
-		n.internal = true
 		if n.generic == nil {
 			n.generic = make(map[string]interface{})
 		}
+		n.internal = true
 		n.generic[netlabel.Internal] = true
 	}
 }
@@ -526,13 +544,6 @@ func NetworkOptionDriverOpts(opts map[string]string) NetworkOption {
 		}
 		// Store the options
 		n.generic[netlabel.GenericData] = opts
-		// Decode and store the endpoint options of libnetwork interest
-		if val, ok := opts[netlabel.EnableIPv6]; ok {
-			var err error
-			if n.enableIPv6, err = strconv.ParseBool(val); err != nil {
-				log.Warnf("Failed to parse %s' value: %s (%s)", netlabel.EnableIPv6, val, err.Error())
-			}
-		}
 	}
 }
 
