From cfb3ce460bf57efea0fc89d2fd1b36da4c5a532b Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Sat, 30 Jan 2016 00:54:57 +0000 Subject: [PATCH] Don't allow passing EnableIPv6 as a driver option (a label) Signed-off-by: Aidan Hobson Sayers --- libnetwork/api/api.go | 20 +++++++++--- libnetwork/api/api_test.go | 41 +++++++----------------- libnetwork/client/network.go | 4 +++ libnetwork/default_gateway_linux.go | 9 ++---- libnetwork/drivers/bridge/bridge_test.go | 23 +++++++------ libnetwork/libnetwork_test.go | 6 ++-- libnetwork/network.go | 35 +++++++++++++------- 7 files changed, 71 insertions(+), 67 deletions(-) diff --git a/libnetwork/api/api.go b/libnetwork/api/api.go index 0318ceb205..f860ab1f5f 100644 --- a/libnetwork/api/api.go +++ b/libnetwork/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/libnetwork/api/api_test.go b/libnetwork/api/api_test.go index bf539b05d1..41f46f5887 100644 --- a/libnetwork/api/api_test.go +++ b/libnetwork/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/libnetwork/client/network.go b/libnetwork/client/network.go index e539f2e85c..08e69ed033 100644 --- a/libnetwork/client/network.go +++ b/libnetwork/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/libnetwork/default_gateway_linux.go b/libnetwork/default_gateway_linux.go index c7f812778f..9376922a21 100644 --- a/libnetwork/default_gateway_linux.go +++ b/libnetwork/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/libnetwork/drivers/bridge/bridge_test.go b/libnetwork/drivers/bridge/bridge_test.go index 812b72b55a..8f47fd5a41 100644 --- a/libnetwork/drivers/bridge/bridge_test.go +++ b/libnetwork/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/libnetwork_test.go b/libnetwork/libnetwork_test.go index 9408dcbdfa..002f6d511e 100644 --- a/libnetwork/libnetwork_test.go +++ b/libnetwork/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,7 +1008,6 @@ func TestEndpointJoin(t *testing.T) { netOption := options.Generic{ netlabel.GenericData: options.Generic{ "BridgeName": "testnetwork1", - "EnableIPv6": true, "EnableICC": true, "EnableIPMasquerade": true, }, @@ -1016,6 +1015,7 @@ func TestEndpointJoin(t *testing.T) { 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/libnetwork/network.go b/libnetwork/network.go index ef94e17473..d995072f9c 100644 --- a/libnetwork/network.go +++ b/libnetwork/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()) - } - } } }