Browse Source

Fix bridge driver panic in CreateNetwork

Bridge driver panics in `CreateNetwork` if called without
a prior `Config` call. This causes issues in dnet which
tries to create network using default driver configuration.
It should be valid to call `CreateNetwork` without a prior
`Config` call in which case we need to assume default driver
config.

Fixed this by properly initializing the driver config pointer.
Also introduced a `configured` bool to make sure that still
`Config` is called exactly once for the instance of the bridge
driver.

Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>
Jana Radhakrishnan 10 years ago
parent
commit
005d8f1f52
2 changed files with 49 additions and 22 deletions
  1. 36 22
      libnetwork/drivers/bridge/bridge.go
  2. 13 0
      libnetwork/drivers/bridge/bridge_test.go

+ 36 - 22
libnetwork/drivers/bridge/bridge.go

@@ -98,6 +98,7 @@ type bridgeNetwork struct {
 
 
 type driver struct {
 type driver struct {
 	config      *configuration
 	config      *configuration
+	configured  bool
 	network     *bridgeNetwork
 	network     *bridgeNetwork
 	natChain    *iptables.ChainInfo
 	natChain    *iptables.ChainInfo
 	filterChain *iptables.ChainInfo
 	filterChain *iptables.ChainInfo
@@ -108,7 +109,7 @@ type driver struct {
 // New constructs a new bridge driver
 // New constructs a new bridge driver
 func newDriver() driverapi.Driver {
 func newDriver() driverapi.Driver {
 	ipAllocator = ipallocator.New()
 	ipAllocator = ipallocator.New()
-	return &driver{networks: map[string]*bridgeNetwork{}}
+	return &driver{networks: map[string]*bridgeNetwork{}, config: &configuration{}}
 }
 }
 
 
 // Init registers a new instance of bridge driver
 // Init registers a new instance of bridge driver
@@ -433,29 +434,26 @@ func (d *driver) Config(option map[string]interface{}) error {
 	d.Lock()
 	d.Lock()
 	defer d.Unlock()
 	defer d.Unlock()
 
 
-	if d.config != nil {
+	if d.configured {
 		return &ErrConfigExists{}
 		return &ErrConfigExists{}
 	}
 	}
 
 
 	genericData, ok := option[netlabel.GenericData]
 	genericData, ok := option[netlabel.GenericData]
-	if ok && genericData != nil {
-		switch opt := genericData.(type) {
-		case options.Generic:
-			opaqueConfig, err := options.GenerateFromModel(opt, &configuration{})
-			if err != nil {
-				return err
-			}
-			config = opaqueConfig.(*configuration)
-		case *configuration:
-			config = opt
-		default:
-			return &ErrInvalidDriverConfig{}
-		}
+	if !ok || genericData == nil {
+		return nil
+	}
 
 
-		d.config = config
-	} else {
-		config = &configuration{}
-		d.config = config
+	switch opt := genericData.(type) {
+	case options.Generic:
+		opaqueConfig, err := options.GenerateFromModel(opt, &configuration{})
+		if err != nil {
+			return err
+		}
+		config = opaqueConfig.(*configuration)
+	case *configuration:
+		config = opt
+	default:
+		return &ErrInvalidDriverConfig{}
 	}
 	}
 
 
 	if config.EnableIPForwarding {
 	if config.EnableIPForwarding {
@@ -467,9 +465,13 @@ func (d *driver) Config(option map[string]interface{}) error {
 
 
 	if config.EnableIPTables {
 	if config.EnableIPTables {
 		d.natChain, d.filterChain, err = setupIPChains(config)
 		d.natChain, d.filterChain, err = setupIPChains(config)
-		return err
+		if err != nil {
+			return err
+		}
 	}
 	}
 
 
+	d.configured = true
+	d.config = config
 	return nil
 	return nil
 }
 }
 
 
@@ -566,12 +568,20 @@ func (d *driver) getNetworks() []*bridgeNetwork {
 
 
 // Create a new network using bridge plugin
 // Create a new network using bridge plugin
 func (d *driver) CreateNetwork(id string, option map[string]interface{}) error {
 func (d *driver) CreateNetwork(id string, option map[string]interface{}) error {
-	var err error
+	var (
+		err          error
+		configLocked bool
+	)
 
 
 	defer osl.InitOSContext()()
 	defer osl.InitOSContext()()
 
 
 	// Sanity checks
 	// Sanity checks
 	d.Lock()
 	d.Lock()
+	if !d.configured {
+		configLocked = true
+		d.configured = true
+	}
+
 	if _, ok := d.networks[id]; ok {
 	if _, ok := d.networks[id]; ok {
 		d.Unlock()
 		d.Unlock()
 		return types.ForbiddenErrorf("network %s exists", id)
 		return types.ForbiddenErrorf("network %s exists", id)
@@ -610,6 +620,10 @@ func (d *driver) CreateNetwork(id string, option map[string]interface{}) error {
 	defer func() {
 	defer func() {
 		if err != nil {
 		if err != nil {
 			d.Lock()
 			d.Lock()
+			if configLocked {
+				d.configured = false
+			}
+
 			delete(d.networks, id)
 			delete(d.networks, id)
 			d.Unlock()
 			d.Unlock()
 		}
 		}
@@ -651,7 +665,7 @@ func (d *driver) CreateNetwork(id string, option map[string]interface{}) error {
 	bridgeSetup.queueStep(setupBridgeIPv4)
 	bridgeSetup.queueStep(setupBridgeIPv4)
 
 
 	enableIPv6Forwarding := false
 	enableIPv6Forwarding := false
-	if d.config != nil && d.config.EnableIPForwarding && config.FixedCIDRv6 != nil {
+	if d.config.EnableIPForwarding && config.FixedCIDRv6 != nil {
 		enableIPv6Forwarding = true
 		enableIPv6Forwarding = true
 	}
 	}
 
 

+ 13 - 0
libnetwork/drivers/bridge/bridge_test.go

@@ -68,6 +68,19 @@ func TestCreateFullOptions(t *testing.T) {
 	}
 	}
 }
 }
 
 
+func TestCreateNoConfig(t *testing.T) {
+	defer osl.SetupTestOSContext(t)()
+	d := newDriver()
+
+	netconfig := &networkConfiguration{BridgeName: DefaultBridgeName}
+	genericOption := make(map[string]interface{})
+	genericOption[netlabel.GenericData] = netconfig
+
+	if err := d.CreateNetwork("dummy", genericOption); err != nil {
+		t.Fatalf("Failed to create bridge: %v", err)
+	}
+}
+
 func TestCreate(t *testing.T) {
 func TestCreate(t *testing.T) {
 	defer osl.SetupTestOSContext(t)()
 	defer osl.SetupTestOSContext(t)()
 	d := newDriver()
 	d := newDriver()