GitList

Browse code

Fix selection of subnet from user-managed default bridge

When a user-managed bridge is used instead of docker0 (--bridge), with
a fixed-cidr - the bridge should have an IP address/subnet that
encompasses fixed-cidr ... the bridge address's subnet then defines
the network's subnet, and fixed-cidr defines the allocatable range
within that.

But, selection of the correct subnet/address from the bridge depended
on the address being within fixed-cidr (within the allocatable range).

This change removes that assumption. So, a bridge address with a
subnet that includes fixed-cidr is selected.

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

Rob Murray authored on 2024/08/10 22:45:55
Showing 2 changed files

daemon/daemon_unix.go index 2fe35c5..426672a 100644
integration/daemon/daemon_linux_test.go index 6406cfc..9c87456 100644

daemon/daemon_unix.go

History View file @ 90baa2b

@@ -953,18 +953,26 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
+                     	}
                      	if len(nwList) > 0 {
                     +		// Pick any address from the bridge as a starting point.
                      		nw := nwList[0]
                      		if len(nwList) > 1 && cfg.FixedCIDR != "" {
                     -			_, fCIDR, err := net.ParseCIDR(cfg.FixedCIDR)
                     +			fCidrIP, fCidrNet, err := net.ParseCIDR(cfg.FixedCIDR)
                      			if err != nil {
                     -				return errors.Wrap(err, "parse CIDR failed")
                     +				return errors.Wrap(err, "parse fixed-cidr failed")
+                     			}
                     -			// Iterate through in case there are multiple addresses for the bridge
                     +			// If there's an address with a subnet that contains fixed-cidr, use it.
                      			for _, entry := range nwList {
                     -				if fCIDR.Contains(entry.IP) {
                     +				if entry.Contains(fCidrIP) {
                      					nw = entry
                      					break
+                     				}
                     +				// For backwards compatibility - prefer the first bridge address within
                     +				// fixed-cidr. If fixed-cidr has a bigger subnet than nw.IP, this doesn't really
                     +				// make sense - the allocatable range (fixed-cidr) will be bigger than the subnet
                     +				// (entry.IPNet).
                     +				if fCidrNet.Contains(entry.IP) && !fCidrNet.Contains(nw.IP) {
                     +					nw = entry
                     +				}
+                     			}
+                     		}

integration/daemon/daemon_linux_test.go

History View file @ 90baa2b

@@ -198,13 +198,12 @@ func TestDaemonDefaultBridgeIPAM_UserBr(t *testing.T) {
                      			name:               "fcidr in bridge subnet and bridge ip not in fcidr",
                      			initialBridgeAddrs: []string{"192.168.160.88/20", "192.168.176.88/20", "192.168.192.88/20"},
                      			daemonArgs:         []string{"--fixed-cidr", "192.168.177.0/24"},
                     +			// Selected bridge subnet should be the one that encompasses fixed-cidr.
                      			expIPAMConfig: []network.IPAMConfig{
+                     				{
                     -					// FIXME(robmry) - selected subnet should be the one that encompasses
                     -					//  fixed-cidr, allocatable range is outside the subnet.
                     -					Subnet:  "192.168.160.0/20",
                     +					Subnet:  "192.168.176.0/20",
                      					IPRange: "192.168.177.0/24",
                     -					Gateway: "192.168.160.88",
                     +					Gateway: "192.168.176.88",
                      				},
                      			},
                      		},