From a42e5f06632f39d48e5878ec302ea13eb96d9c38 Mon Sep 17 00:00:00 2001 From: Madhu Venugopal Date: Wed, 23 Sep 2015 18:01:04 -0700 Subject: [PATCH] Flip the default for the flag AllowNonDefaultBridge in bridge driver Replaced it with DisableBridgeCreation and it can be used ONLY in a special case for docker0 bridge from docker, instead of calling it from all other case. Signed-off-by: Madhu Venugopal --- libnetwork/api/api.go | 3 - libnetwork/api/api_test.go | 34 +++---- libnetwork/cmd/dnet/dnet.go | 3 +- libnetwork/default_gateway.go | 7 +- libnetwork/drivers/bridge/bridge.go | 10 +- libnetwork/drivers/bridge/bridge_test.go | 10 +- libnetwork/drivers/bridge/errors.go | 11 +++ libnetwork/drivers/bridge/setup_device.go | 2 +- .../drivers/bridge/setup_device_test.go | 2 +- libnetwork/drivers/bridge/setup_ipv4.go | 4 +- libnetwork/libnetwork_test.go | 91 +++++++------------ libnetwork/sandbox_test.go | 6 +- 12 files changed, 80 insertions(+), 103 deletions(-) diff --git a/libnetwork/api/api.go b/libnetwork/api/api.go index 5f48531998..8bd88d9a58 100644 --- a/libnetwork/api/api.go +++ b/libnetwork/api/api.go @@ -291,9 +291,6 @@ func processCreateDefaults(c libnetwork.NetworkController, nc *networkCreate) { if _, ok := gData["BridgeName"]; !ok { gData["BridgeName"] = nc.Name } - if _, ok := gData["AllowNonDefaultBridge"]; !ok { - gData["AllowNonDefaultBridge"] = "true" - } nc.Options[netlabel.GenericData] = genericData } } diff --git a/libnetwork/api/api_test.go b/libnetwork/api/api_test.go index 6477a8d590..23fbdc6195 100644 --- a/libnetwork/api/api_test.go +++ b/libnetwork/api/api_test.go @@ -95,8 +95,7 @@ func createTestNetwork(t *testing.T, network string) (libnetwork.NetworkControll netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": network, - "AllowNonDefaultBridge": true, + "BridgeName": network, }, } netGeneric := libnetwork.NetworkOptionGeneric(netOption) @@ -210,10 +209,9 @@ func TestCreateDeleteNetwork(t *testing.T) { ops := options.Generic{ netlabel.EnableIPv6: true, netlabel.GenericData: map[string]string{ - "BridgeName": "abc", - "AllowNonDefaultBridge": "true", - "FixedCIDRv6": "fe80::1/64", - "AddressIP": "172.28.30.254/24", + "BridgeName": "abc", + "FixedCIDRv6": "fe80::1/64", + "AddressIP": "172.28.30.254/24", }, } nc := networkCreate{Name: "network_1", NetworkType: bridgeNetType, Options: ops} @@ -256,8 +254,7 @@ func TestGetNetworksAndEndpoints(t *testing.T) { ops := options.Generic{ netlabel.GenericData: map[string]string{ - "BridgeName": "api_test_nw", - "AllowNonDefaultBridge": "true", + "BridgeName": "api_test_nw", }, } @@ -527,8 +524,7 @@ func TestProcGetServices(t *testing.T) { netName1 := "production" netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": netName1, - "AllowNonDefaultBridge": true, + "BridgeName": netName1, }, } nw1, err := c.NewNetwork(bridgeNetType, netName1, libnetwork.NetworkOptionGeneric(netOption)) @@ -539,8 +535,7 @@ func TestProcGetServices(t *testing.T) { netName2 := "work-dev" netOption = options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": netName2, - "AllowNonDefaultBridge": true, + "BridgeName": netName2, }, } nw2, err := c.NewNetwork(bridgeNetType, netName2, libnetwork.NetworkOptionGeneric(netOption)) @@ -1771,14 +1766,13 @@ func TestEndToEnd(t *testing.T) { ops := options.Generic{ netlabel.EnableIPv6: true, netlabel.GenericData: map[string]string{ - "BridgeName": "cdef", - "FixedCIDRv6": "fe80:2000::1/64", - "EnableIPv6": "true", - "Mtu": "1460", - "EnableIPTables": "true", - "AddressIP": "172.28.30.254/16", - "EnableUserlandProxy": "true", - "AllowNonDefaultBridge": "true", + "BridgeName": "cdef", + "FixedCIDRv6": "fe80:2000::1/64", + "EnableIPv6": "true", + "Mtu": "1460", + "EnableIPTables": "true", + "AddressIP": "172.28.30.254/16", + "EnableUserlandProxy": "true", }, } diff --git a/libnetwork/cmd/dnet/dnet.go b/libnetwork/cmd/dnet/dnet.go index ee114d80f6..2d41364dfd 100644 --- a/libnetwork/cmd/dnet/dnet.go +++ b/libnetwork/cmd/dnet/dnet.go @@ -115,8 +115,7 @@ func createDefaultNetwork(c libnetwork.NetworkController) { // Bridge driver is special due to legacy reasons if d == "bridge" { genericOption[netlabel.GenericData] = map[string]interface{}{ - "BridgeName": nw, - "AllowNonDefaultBridge": "true", + "BridgeName": nw, } networkOption := libnetwork.NetworkOptionGeneric(genericOption) createOptions = append(createOptions, networkOption) diff --git a/libnetwork/default_gateway.go b/libnetwork/default_gateway.go index 31ebed1bdc..33996e1565 100644 --- a/libnetwork/default_gateway.go +++ b/libnetwork/default_gateway.go @@ -93,10 +93,9 @@ func (sb *sandbox) clearDefaultGW() error { func (c *controller) createGWNetwork() (Network, error) { netOption := options.Generic{ - "BridgeName": libnGWNetwork, - "EnableICC": false, - "AllowNonDefaultBridge": true, - "EnableIPMasquerade": true, + "BridgeName": libnGWNetwork, + "EnableICC": false, + "EnableIPMasquerade": true, } n, err := c.NewNetwork("bridge", libnGWNetwork, diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go index 21e1f2b35d..31c6469abf 100644 --- a/libnetwork/drivers/bridge/bridge.go +++ b/libnetwork/drivers/bridge/bridge.go @@ -59,7 +59,7 @@ type networkConfiguration struct { DefaultGatewayIPv4 net.IP DefaultGatewayIPv6 net.IP DefaultBindingIP net.IP - AllowNonDefaultBridge bool + DisableBridgeCreation bool } // endpointConfiguration represents the user specified configuration for the sandbox endpoint @@ -249,13 +249,13 @@ func (c *networkConfiguration) fromMap(data map[string]interface{}) error { } } - if i, ok := data["AllowNonDefaultBridge"]; ok && i != nil { + if i, ok := data["DisableBridgeCreation"]; ok && i != nil { if s, ok := i.(string); ok { - if c.AllowNonDefaultBridge, err = strconv.ParseBool(s); err != nil { - return types.BadRequestErrorf("failed to parse AllowNonDefaultBridge value: %s", err.Error()) + if c.DisableBridgeCreation, err = strconv.ParseBool(s); err != nil { + return types.BadRequestErrorf("failed to parse DisableBridgeCreation value: %s", err.Error()) } } else { - return types.BadRequestErrorf("invalid type for AllowNonDefaultBridge value") + return types.BadRequestErrorf("invalid type for DisableBridgeCreation value") } } diff --git a/libnetwork/drivers/bridge/bridge_test.go b/libnetwork/drivers/bridge/bridge_test.go index f2604fbc53..442440ad7e 100644 --- a/libnetwork/drivers/bridge/bridge_test.go +++ b/libnetwork/drivers/bridge/bridge_test.go @@ -123,7 +123,7 @@ func TestCreateFail(t *testing.T) { t.Fatalf("Failed to setup driver config: %v", err) } - netconfig := &networkConfiguration{BridgeName: "dummy0"} + netconfig := &networkConfiguration{BridgeName: "dummy0", DisableBridgeCreation: true} genericOption := make(map[string]interface{}) genericOption[netlabel.GenericData] = netconfig @@ -146,20 +146,20 @@ func TestCreateMultipleNetworks(t *testing.T) { t.Fatalf("Failed to setup driver config: %v", err) } - config1 := &networkConfiguration{BridgeName: "net_test_1", AllowNonDefaultBridge: true} + config1 := &networkConfiguration{BridgeName: "net_test_1"} genericOption = make(map[string]interface{}) genericOption[netlabel.GenericData] = config1 if err := d.CreateNetwork("1", genericOption); err != nil { t.Fatalf("Failed to create bridge: %v", err) } - config2 := &networkConfiguration{BridgeName: "net_test_2", AllowNonDefaultBridge: true} + config2 := &networkConfiguration{BridgeName: "net_test_2"} genericOption[netlabel.GenericData] = config2 if err := d.CreateNetwork("2", genericOption); err != nil { t.Fatalf("Failed to create bridge: %v", err) } - config3 := &networkConfiguration{BridgeName: "net_test_3", AllowNonDefaultBridge: true} + config3 := &networkConfiguration{BridgeName: "net_test_3"} genericOption[netlabel.GenericData] = config3 if err := d.CreateNetwork("3", genericOption); err != nil { t.Fatalf("Failed to create bridge: %v", err) @@ -168,7 +168,7 @@ func TestCreateMultipleNetworks(t *testing.T) { // Verify the network isolation rules are installed, each network subnet should appear 4 times verifyV4INCEntries(d.networks, 4, t) - config4 := &networkConfiguration{BridgeName: "net_test_4", AllowNonDefaultBridge: true} + config4 := &networkConfiguration{BridgeName: "net_test_4"} genericOption[netlabel.GenericData] = config4 if err := d.CreateNetwork("4", genericOption); 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 dd17ddf902..0e0d67aa69 100644 --- a/libnetwork/drivers/bridge/errors.go +++ b/libnetwork/drivers/bridge/errors.go @@ -211,6 +211,17 @@ func (ndbee NonDefaultBridgeExistError) Error() string { // Forbidden denotes the type of this error func (ndbee NonDefaultBridgeExistError) Forbidden() {} +// NonDefaultBridgeNeedsIPError is returned when a non-default +// bridge config is passed but it has no ip configured +type NonDefaultBridgeNeedsIPError string + +func (ndbee NonDefaultBridgeNeedsIPError) Error() string { + return fmt.Sprintf("bridge device with non default name %s must have a valid IP address", string(ndbee)) +} + +// Forbidden denotes the type of this error +func (ndbee NonDefaultBridgeNeedsIPError) Forbidden() {} + // FixedCIDRv4Error is returned when fixed-cidrv4 configuration // failed. type FixedCIDRv4Error struct { diff --git a/libnetwork/drivers/bridge/setup_device.go b/libnetwork/drivers/bridge/setup_device.go index 22bf64b2f7..50f01b8270 100644 --- a/libnetwork/drivers/bridge/setup_device.go +++ b/libnetwork/drivers/bridge/setup_device.go @@ -15,7 +15,7 @@ func setupDevice(config *networkConfiguration, i *bridgeInterface) error { // We only attempt to create the bridge when the requested device name is // the default one. - if config.BridgeName != DefaultBridgeName && !config.AllowNonDefaultBridge { + if config.BridgeName != DefaultBridgeName && config.DisableBridgeCreation { return NonDefaultBridgeExistError(config.BridgeName) } diff --git a/libnetwork/drivers/bridge/setup_device_test.go b/libnetwork/drivers/bridge/setup_device_test.go index ebefa1f6d5..235260c559 100644 --- a/libnetwork/drivers/bridge/setup_device_test.go +++ b/libnetwork/drivers/bridge/setup_device_test.go @@ -33,7 +33,7 @@ func TestSetupNewBridge(t *testing.T) { func TestSetupNewNonDefaultBridge(t *testing.T) { defer testutils.SetupTestOSContext(t)() - config := &networkConfiguration{BridgeName: "test0"} + config := &networkConfiguration{BridgeName: "test0", DisableBridgeCreation: true} br := &bridgeInterface{} err := setupDevice(config, br) diff --git a/libnetwork/drivers/bridge/setup_ipv4.go b/libnetwork/drivers/bridge/setup_ipv4.go index 85073932c4..59ae06511f 100644 --- a/libnetwork/drivers/bridge/setup_ipv4.go +++ b/libnetwork/drivers/bridge/setup_ipv4.go @@ -53,8 +53,8 @@ func setupBridgeIPv4(config *networkConfiguration, i *bridgeInterface) error { // Do not try to configure IPv4 on a non-default bridge unless you are // specifically asked to do so. - if config.BridgeName != DefaultBridgeName && !config.AllowNonDefaultBridge { - return NonDefaultBridgeExistError(config.BridgeName) + if config.BridgeName != DefaultBridgeName && config.DisableBridgeCreation { + return NonDefaultBridgeNeedsIPError(config.BridgeName) } bridgeIPv4, err := electBridgeIPv4(config) diff --git a/libnetwork/libnetwork_test.go b/libnetwork/libnetwork_test.go index 82e3dd642a..5ed2d1da3e 100644 --- a/libnetwork/libnetwork_test.go +++ b/libnetwork/libnetwork_test.go @@ -288,14 +288,13 @@ func TestBridge(t *testing.T) { netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "AddressIPv4": subnet, - "FixedCIDR": cidr, - "FixedCIDRv6": cidrv6, - "EnableIPv6": true, - "EnableICC": true, - "AllowNonDefaultBridge": true, - "EnableIPMasquerade": true, + "BridgeName": "testnetwork", + "AddressIPv4": subnet, + "FixedCIDR": cidr, + "FixedCIDRv6": cidrv6, + "EnableIPv6": true, + "EnableICC": true, + "EnableIPMasquerade": true, }, } @@ -389,8 +388,7 @@ func TestNetworkName(t *testing.T) { netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork", }, } @@ -426,8 +424,7 @@ func TestNetworkType(t *testing.T) { netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork", }, } @@ -453,8 +450,7 @@ func TestNetworkID(t *testing.T) { netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork", }, } @@ -479,8 +475,8 @@ func TestDeleteNetworkWithActiveEndpoints(t *testing.T) { } netOption := options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true} + "BridgeName": "testnetwork", + } option := options.Generic{ netlabel.GenericData: netOption, } @@ -520,8 +516,8 @@ func TestUnknownNetwork(t *testing.T) { } netOption := options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true} + "BridgeName": "testnetwork", + } option := options.Generic{ netlabel.GenericData: netOption, } @@ -558,9 +554,9 @@ func TestUnknownEndpoint(t *testing.T) { subnet.IP = ip netOption := options.Generic{ - "BridgeName": "testnetwork", - "AddressIPv4": subnet, - "AllowNonDefaultBridge": true} + "BridgeName": "testnetwork", + "AddressIPv4": subnet, + } option := options.Generic{ netlabel.GenericData: netOption, } @@ -602,8 +598,7 @@ func TestNetworkEndpointsWalkers(t *testing.T) { // Create network 1 and add 2 endpoint: ep11, ep12 netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "network1", - "AllowNonDefaultBridge": true, + "BridgeName": "network1", }, } @@ -675,8 +670,7 @@ func TestNetworkEndpointsWalkers(t *testing.T) { // Create network 2 netOption = options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "network2", - "AllowNonDefaultBridge": true, + "BridgeName": "network2", }, } @@ -733,8 +727,7 @@ func TestDuplicateEndpoint(t *testing.T) { netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork", }, } n, err := createTestNetwork(bridgeNetType, "testnetwork", netOption) @@ -784,8 +777,7 @@ func TestControllerQuery(t *testing.T) { // Create network 1 netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "network1", - "AllowNonDefaultBridge": true, + "BridgeName": "network1", }, } net1, err := createTestNetwork(bridgeNetType, "network1", netOption) @@ -801,8 +793,7 @@ func TestControllerQuery(t *testing.T) { // Create network 2 netOption = options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "network2", - "AllowNonDefaultBridge": true, + "BridgeName": "network2", }, } net2, err := createTestNetwork(bridgeNetType, "network2", netOption) @@ -888,8 +879,7 @@ func TestNetworkQuery(t *testing.T) { // Create network 1 and add 2 endpoint: ep11, ep12 netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "network1", - "AllowNonDefaultBridge": true, + "BridgeName": "network1", }, } net1, err := createTestNetwork(bridgeNetType, "network1", netOption) @@ -1010,8 +1000,7 @@ func TestEndpointJoin(t *testing.T) { // Create network 1 and add 2 endpoint: ep11, ep12 n1, err := createTestNetwork(bridgeNetType, "testnetwork1", options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork1", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork1", }, }) if err != nil { @@ -1121,8 +1110,7 @@ func TestEndpointJoin(t *testing.T) { n2, err := createTestNetwork(bridgeNetType, "testnetwork2", options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork2", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork2", }, }) if err != nil { @@ -1213,8 +1201,7 @@ func externalKeyTest(t *testing.T, reexec bool) { n, err := createTestNetwork(bridgeNetType, "testnetwork", options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork", }, }) if err != nil { @@ -1363,8 +1350,7 @@ func TestEndpointDeleteWithActiveContainer(t *testing.T) { n, err := createTestNetwork(bridgeNetType, "testnetwork", options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork", }, }) if err != nil { @@ -1427,8 +1413,7 @@ func TestEndpointMultipleJoins(t *testing.T) { n, err := createTestNetwork(bridgeNetType, "testmultiple", options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testmultiple", - "AllowNonDefaultBridge": true, + "BridgeName": "testmultiple", }, }) if err != nil { @@ -1499,8 +1484,7 @@ func TestLeaveAll(t *testing.T) { n, err := createTestNetwork(bridgeNetType, "testnetwork", options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork", }, }) if err != nil { @@ -1563,8 +1547,7 @@ func TestontainerInvalidLeave(t *testing.T) { n, err := createTestNetwork(bridgeNetType, "testnetwork", options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork", }, }) if err != nil { @@ -1630,8 +1613,7 @@ func TestEndpointUpdateParent(t *testing.T) { n, err := createTestNetwork("bridge", "testnetwork", options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork", }, }) if err != nil { @@ -1736,9 +1718,8 @@ func TestEnableIPv6(t *testing.T) { netOption := options.Generic{ netlabel.EnableIPv6: true, netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "FixedCIDRv6": cidrv6, - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork", + "FixedCIDRv6": cidrv6, }, } @@ -1909,8 +1890,7 @@ func TestResolvConf(t *testing.T) { netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "testnetwork", - "AllowNonDefaultBridge": true, + "BridgeName": "testnetwork", }, } n, err := createTestNetwork("bridge", "testnetwork", netOption) @@ -2178,8 +2158,7 @@ func createGlobalInstance(t *testing.T) { netOption := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": "network", - "AllowNonDefaultBridge": true, + "BridgeName": "network", }, } diff --git a/libnetwork/sandbox_test.go b/libnetwork/sandbox_test.go index 60fbf174e7..0f47248eca 100644 --- a/libnetwork/sandbox_test.go +++ b/libnetwork/sandbox_test.go @@ -35,8 +35,7 @@ func getTestEnv(t *testing.T) (NetworkController, Network, Network) { name1 := "test_nw_1" netOption1 := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": name1, - "AllowNonDefaultBridge": true, + "BridgeName": name1, }, } n1, err := c.NewNetwork(netType, name1, NetworkOptionGeneric(netOption1)) @@ -47,8 +46,7 @@ func getTestEnv(t *testing.T) (NetworkController, Network, Network) { name2 := "test_nw_2" netOption2 := options.Generic{ netlabel.GenericData: options.Generic{ - "BridgeName": name2, - "AllowNonDefaultBridge": true, + "BridgeName": name2, }, } n2, err := c.NewNetwork(netType, name2, NetworkOptionGeneric(netOption2))