Browse code

Separate IPv4 IPAM conf from the rest of default bridge conf

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

Rob Murray authored on 2024/08/11 02:38:15
Showing 3 changed files
... ...
@@ -5,7 +5,6 @@ import (
5 5
 	"context"
6 6
 	"fmt"
7 7
 	"io"
8
-	"net"
9 8
 	"os"
10 9
 	"regexp"
11 10
 	"strings"
... ...
@@ -148,42 +147,20 @@ func setupResolvConf(config *config.Config) {
148 148
 	config.ResolvConf = resolvconf.Path()
149 149
 }
150 150
 
151
-// ifaceAddrs returns the IPv4 and IPv6 addresses assigned to the network
151
+// ifaceAddrs returns the addresses from family assigned to the network
152 152
 // interface with name linkName.
153 153
 //
154 154
 // No error is returned if the named interface does not exist.
155
-func ifaceAddrs(linkName string) (v4, v6 []*net.IPNet, err error) {
155
+func ifaceAddrs(linkName string, family int) ([]netlink.Addr, error) {
156 156
 	nl := ns.NlHandle()
157 157
 	link, err := nl.LinkByName(linkName)
158 158
 	if err != nil {
159 159
 		if !errors.As(err, new(netlink.LinkNotFoundError)) {
160
-			return nil, nil, err
161
-		}
162
-		return nil, nil, nil
163
-	}
164
-
165
-	get := func(family int) ([]*net.IPNet, error) {
166
-		addrs, err := nl.AddrList(link, family)
167
-		if err != nil {
168 160
 			return nil, err
169 161
 		}
170
-
171
-		ipnets := make([]*net.IPNet, len(addrs))
172
-		for i := range addrs {
173
-			ipnets[i] = addrs[i].IPNet
174
-		}
175
-		return ipnets, nil
176
-	}
177
-
178
-	v4, err = get(netlink.FAMILY_V4)
179
-	if err != nil {
180
-		return nil, nil, err
181
-	}
182
-	v6, err = get(netlink.FAMILY_V6)
183
-	if err != nil {
184
-		return nil, nil, err
162
+		return nil, nil
185 163
 	}
186
-	return v4, v6, nil
164
+	return nl.AddrList(link, family)
187 165
 }
188 166
 
189 167
 var (
... ...
@@ -363,12 +363,20 @@ func TestIfaceAddrs(t *testing.T) {
363 363
 
364 364
 			createBridge(t, "test", tt.nws...)
365 365
 
366
-			ipv4Nw, ipv6Nw, err := ifaceAddrs("test")
366
+			ipv4Nw, err := ifaceAddrs("test", netlink.FAMILY_V4)
367
+			if err != nil {
368
+				t.Fatal(err)
369
+			}
370
+			ipv6Nw, err := ifaceAddrs("test", netlink.FAMILY_V6)
367 371
 			if err != nil {
368 372
 				t.Fatal(err)
369 373
 			}
370 374
 
371
-			assert.Check(t, is.DeepEqual(tt.nws, ipv4Nw,
375
+			ipnets := make([]*net.IPNet, len(ipv4Nw))
376
+			for i := range ipv4Nw {
377
+				ipnets[i] = ipv4Nw[i].IPNet
378
+			}
379
+			assert.Check(t, is.DeepEqual(ipnets, tt.nws,
372 380
 				cmpopts.SortSlices(func(a, b *net.IPNet) bool { return a.String() < b.String() })))
373 381
 			// IPv6 link-local address
374 382
 			assert.Check(t, is.Len(ipv6Nw, 1))
... ...
@@ -940,84 +940,9 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
940 940
 		netOption[bridge.DefaultBindingIP] = cfg.DefaultIP.String()
941 941
 	}
942 942
 
943
-	nwList, nw6List, err := ifaceAddrs(bridgeName)
943
+	ipamV4Conf, err := getDefaultBridgeIPAMConf(bridgeName, userManagedBridge, cfg, netlink.FAMILY_V4)
944 944
 	if err != nil {
945
-		return errors.Wrap(err, "list bridge addresses failed")
946
-	}
947
-
948
-	var (
949
-		fCidrIP, bIP       net.IP
950
-		fCidrIPNet, bIPNet *net.IPNet
951
-	)
952
-
953
-	if cfg.FixedCIDR != "" {
954
-		if fCidrIP, fCidrIPNet, err = net.ParseCIDR(cfg.FixedCIDR); err != nil {
955
-			return errors.Wrap(err, "parse fixed-cidr failed")
956
-		}
957
-	}
958
-
959
-	if cfg.IP != "" {
960
-		if bIP, bIPNet, err = net.ParseCIDR(cfg.IP); err != nil {
961
-			return err
962
-		}
963
-	} else {
964
-		bIP, bIPNet = selectBIP(nwList, fCidrIP, fCidrIPNet)
965
-		if !userManagedBridge && fCidrIP != nil && bIPNet != nil {
966
-			if !bIPNet.Contains(fCidrIP) {
967
-				// The bridge is docker-managed (docker0) and fixed-cidr is not
968
-				// inside a subnet belonging to any existing bridge IP. (fixed-cidr
969
-				// has changed.) So, ignore the existing bridge IP.
970
-				bIP = nil
971
-				bIPNet = nil
972
-			} else {
973
-				fCidrOnes, _ := fCidrIPNet.Mask.Size()
974
-				bIPOnes, _ := bIPNet.Mask.Size()
975
-				if fCidrOnes < bIPOnes {
976
-					// The bridge is docker-managed (docker0) and fixed-cidr (the
977
-					// allocatable address range) is bigger than the subnet implied
978
-					// by the bridge's current address. (fixed-cidr has changed.)
979
-					// The bridge's address is ok, but its subnet needs to be updated.
980
-					bIPNet.IP = bIPNet.IP.Mask(fCidrIPNet.Mask)
981
-					bIPNet.Mask = fCidrIPNet.Mask
982
-				}
983
-			}
984
-		}
985
-	}
986
-
987
-	ipamV4Conf := &libnetwork.IpamConf{AuxAddresses: make(map[string]string)}
988
-	if bIP != nil {
989
-		ipamV4Conf.PreferredPool = bIPNet.String()
990
-		ipamV4Conf.Gateway = bIP.String()
991
-	} else if !userManagedBridge && ipamV4Conf.PreferredPool != "" {
992
-		log.G(context.TODO()).Infof("Default bridge (%s) is assigned with an IP address %s. Daemon option --bip can be used to set a preferred IP address", bridgeName, ipamV4Conf.PreferredPool)
993
-	}
994
-
995
-	if fCidrIP != nil {
996
-		ipamV4Conf.SubPool = fCidrIPNet.String()
997
-		if ipamV4Conf.PreferredPool == "" {
998
-			ipamV4Conf.PreferredPool = fCidrIPNet.String()
999
-		} else if userManagedBridge && bIPNet != nil {
1000
-			fCidrOnes, _ := fCidrIPNet.Mask.Size()
1001
-			bIPOnes, _ := bIPNet.Mask.Size()
1002
-			if !bIPNet.Contains(fCidrIP) || (fCidrOnes < bIPOnes) {
1003
-				// Don't allow SubPool (the range of allocatable addresses) to be outside, or
1004
-				// bigger than, the network itself. This is a configuration error, either the
1005
-				// user-managed bridge is missing an address to match fixed-cidr, or fixed-cidr
1006
-				// is wrong. But, just log rather than raise an error that would cause daemon
1007
-				// startup to fail, because this has been allowed by earlier versions. Remove
1008
-				// the SubPool, so that addresses are allocated from the whole of PreferredPool.
1009
-				log.G(context.TODO()).WithFields(log.Fields{
1010
-					"bridge":         bridgeName,
1011
-					"fixed-cidr":     cfg.FixedCIDR,
1012
-					"bridge-network": bIPNet.String(),
1013
-				}).Warn("fixed-cidr is outside the subnet implied by addresses on the user-managed default bridge, this may be treated as an error in a future release")
1014
-				ipamV4Conf.SubPool = ""
1015
-			}
1016
-		}
1017
-	}
1018
-
1019
-	if cfg.DefaultGatewayIPv4 != nil {
1020
-		ipamV4Conf.AuxAddresses["DefaultGatewayIPv4"] = cfg.DefaultGatewayIPv4.String()
945
+		return err
1021 946
 	}
1022 947
 
1023 948
 	var (
... ...
@@ -1050,6 +975,10 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
1050 1050
 		// In case the --fixed-cidr-v6 is specified and the current docker0 bridge IPv6
1051 1051
 		// address belongs to the same network, we need to inform libnetwork about it, so
1052 1052
 		// that it can be reserved with IPAM and it will not be given away to somebody else
1053
+		nw6List, err := ifaceAddrs(bridgeName, netlink.FAMILY_V6)
1054
+		if err != nil {
1055
+			return errors.Wrap(err, "list bridge IPv6 addresses failed")
1056
+		}
1053 1057
 		for _, nw6 := range nw6List {
1054 1058
 			if fCIDRv6.Contains(nw6.IP) {
1055 1059
 				ipamV6Conf.Gateway = nw6.IP.String()
... ...
@@ -1065,7 +994,6 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
1065 1065
 		ipamV6Conf.AuxAddresses["DefaultGatewayIPv6"] = cfg.DefaultGatewayIPv6.String()
1066 1066
 	}
1067 1067
 
1068
-	v4Conf := []*libnetwork.IpamConf{ipamV4Conf}
1069 1068
 	v6Conf := []*libnetwork.IpamConf{}
1070 1069
 	if ipamV6Conf != nil {
1071 1070
 		v6Conf = append(v6Conf, ipamV6Conf)
... ...
@@ -1075,7 +1003,7 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
1075 1075
 		libnetwork.NetworkOptionEnableIPv4(true),
1076 1076
 		libnetwork.NetworkOptionEnableIPv6(cfg.EnableIPv6),
1077 1077
 		libnetwork.NetworkOptionDriverOpts(netOption),
1078
-		libnetwork.NetworkOptionIpam("default", "", v4Conf, v6Conf, nil),
1078
+		libnetwork.NetworkOptionIpam("default", "", ipamV4Conf, v6Conf, nil),
1079 1079
 		libnetwork.NetworkOptionDeferIPv6Alloc(deferIPv6Alloc))
1080 1080
 	if err != nil {
1081 1081
 		return fmt.Errorf(`error creating default %q network: %v`, network.NetworkBridge, err)
... ...
@@ -1105,25 +1033,152 @@ func getDefaultBridgeName(cfg config.BridgeConfig) (bridgeName string, userManag
1105 1105
 	return bridge.DefaultBridgeName, false
1106 1106
 }
1107 1107
 
1108
-// selectBIP searches bridgeNws for:
1108
+// getDefaultBridgeIPAMConf works out IPAM configuration for the
1109
+// default bridge, for the given address family (netlink.FAMILY_V4 or
1110
+// netlink.FAMILY_V6).
1111
+//
1112
+// Inputs are:
1113
+// - bip
1114
+//   - CIDR, not a plain IP address.
1115
+//   - docker-managed bridge only (docker0), not allowed with a user-managed
1116
+//     bridge (--bridge) where the user is responsible for creating the bridge
1117
+//     and configuring its addresses.
1118
+//   - determines the network subnet (ipamConf.PreferredPool) and becomes the
1119
+//     bridge/gateway address (ipamConf.Gateway).
1120
+//
1121
+// - existing bridge addresses
1122
+//   - for a user-managed bridge
1123
+//   - an address is selected from the bridge to perform the same role as
1124
+//     bip for a daemon-managed bridge.
1125
+//   - for docker0
1126
+//   - if there's an address on the bridge that's compatible with the other
1127
+//     options, it's used as the gateway address and - because it's always
1128
+//     worked this way, to determine the subnet if it's bigger than the
1129
+//     sub-pool configured through fixed-cidr[-v6]. For example, if the
1130
+//     bridge has address 10.11.12.13/16 and fixed-cidr=10.11.22.0/24, the
1131
+//     default bridge network's subnet is 10.11.0.0/16, sub-pool for
1132
+//     automatic address allocation 10.11.22.0/24, gateway 10.11.12.13.
1133
+//
1134
+// - fixed-cidr/fixed-cidr-v6
1135
+//   - ipamConf.SubPool, the pool for automatic address allocation (somewhat
1136
+//     equivalent to --ip-range for a user-defined network), must be contained
1137
+//     within the subnet. Used as ipamConf.PreferredPool if it's not given a
1138
+//     value by other rules.
1139
+//
1140
+// So, for example, with this config (taken from docs):
1141
+//
1142
+//	"bip": "192.168.1.1/24",
1143
+//	"fixed-cidr": "192.168.1.0/25",
1144
+//
1145
+// - the bridge's address is "192.168.1.1/24"
1146
+// - the subnet is "192.168.1.0/24"
1147
+// - the bridge driver can allocate addresses from "192.168.1.0/25"
1148
+//
1149
+// The result is the same if "bip" is unset (including for a user-managed
1150
+// bridge), when the bridge already has address "192.168.1.1/24".
1151
+//
1152
+// Note that this function logs-then-ignores invalid configuration, because it
1153
+// has to tolerate existing configuration - raising an error prevents daemon
1154
+// startup. Earlier versions of the daemon didn't spot bad config, but generally
1155
+// did something unsurprising with it.
1156
+func getDefaultBridgeIPAMConf(
1157
+	bridgeName string,
1158
+	userManagedBridge bool,
1159
+	cfg config.BridgeConfig,
1160
+	family int,
1161
+) ([]*libnetwork.IpamConf, error) {
1162
+	var (
1163
+		fCidrIP, bIP       net.IP
1164
+		fCidrIPNet, bIPNet *net.IPNet
1165
+		err                error
1166
+	)
1167
+
1168
+	if cfg.FixedCIDR != "" {
1169
+		if fCidrIP, fCidrIPNet, err = net.ParseCIDR(cfg.FixedCIDR); err != nil {
1170
+			return nil, errors.Wrap(err, "parse fixed-cidr failed")
1171
+		}
1172
+	}
1173
+
1174
+	if cfg.IP != "" {
1175
+		if bIP, bIPNet, err = net.ParseCIDR(cfg.IP); err != nil {
1176
+			return nil, err
1177
+		}
1178
+	} else {
1179
+		if bIP, bIPNet, err = selectBIP(userManagedBridge, bridgeName, family, fCidrIP, fCidrIPNet); err != nil {
1180
+			return nil, err
1181
+		}
1182
+	}
1183
+
1184
+	ipamConf := &libnetwork.IpamConf{AuxAddresses: make(map[string]string)}
1185
+	if bIP != nil {
1186
+		ipamConf.PreferredPool = bIPNet.String()
1187
+		ipamConf.Gateway = bIP.String()
1188
+	} else if !userManagedBridge && ipamConf.PreferredPool != "" {
1189
+		log.G(context.TODO()).Infof("Default bridge (%s) is assigned with an IP address %s. Daemon option --bip can be used to set a preferred IP address", bridgeName, ipamConf.PreferredPool)
1190
+	}
1191
+
1192
+	if fCidrIP != nil && fCidrIPNet != nil {
1193
+		ipamConf.SubPool = fCidrIPNet.String()
1194
+		if ipamConf.PreferredPool == "" {
1195
+			ipamConf.PreferredPool = fCidrIPNet.String()
1196
+		} else if userManagedBridge && bIPNet != nil {
1197
+			fCidrOnes, _ := fCidrIPNet.Mask.Size()
1198
+			bIPOnes, _ := bIPNet.Mask.Size()
1199
+			if !bIPNet.Contains(fCidrIP) || (fCidrOnes < bIPOnes) {
1200
+				// Don't allow SubPool (the range of allocatable addresses) to be outside, or
1201
+				// bigger than, the network itself. This is a configuration error, either the
1202
+				// user-managed bridge is missing an address to match fixed-cidr, or fixed-cidr
1203
+				// is wrong. But, just log rather than raise an error that would cause daemon
1204
+				// startup to fail, because this has been allowed by earlier versions. Remove
1205
+				// the SubPool, so that addresses are allocated from the whole of PreferredPool.
1206
+				log.G(context.TODO()).WithFields(log.Fields{
1207
+					"bridge":         bridgeName,
1208
+					"fixed-cidr":     cfg.FixedCIDR,
1209
+					"bridge-network": bIPNet.String(),
1210
+				}).Warn("fixed-cidr is outside the subnet implied by addresses on the user-managed default bridge, this may be treated as an error in a future release")
1211
+				ipamConf.SubPool = ""
1212
+			}
1213
+		}
1214
+	}
1215
+
1216
+	if cfg.DefaultGatewayIPv4 != nil {
1217
+		ipamConf.AuxAddresses["DefaultGatewayIPv4"] = cfg.DefaultGatewayIPv4.String()
1218
+	}
1219
+
1220
+	return []*libnetwork.IpamConf{ipamConf}, nil
1221
+}
1222
+
1223
+// selectBIP searches the addresses from family on bridge bridgeName for:
1109 1224
 // - An address that encompasses fCidrNet if there is one.
1110 1225
 // - Else, an address that is within fCidrNet if there is one.
1111 1226
 // - Else, any address, if there is one.
1112
-// If an address is found, it's returned as bIP, with its subnet in canonical
1227
+//
1228
+// If an address is found, the bridge is docker managed (docker0), and the
1229
+// bridge address is not compatible with current fixed-cidr/bip configuration,
1230
+// the address is ignored or modified accordingly, so that the current config
1231
+// can take effect.
1232
+//
1233
+// If there is an address, it's returned as bIP with its subnet in canonical
1113 1234
 // form in bIPNet.
1114 1235
 func selectBIP(
1115
-	bridgeNws []*net.IPNet,
1236
+	userManagedBridge bool,
1237
+	bridgeName string,
1238
+	family int,
1116 1239
 	fCidrIP net.IP,
1117 1240
 	fCidrNet *net.IPNet,
1118
-) (bIP net.IP, bIPNet *net.IPNet) {
1241
+) (bIP net.IP, bIPNet *net.IPNet, err error) {
1242
+	bridgeNws, err := ifaceAddrs(bridgeName, family)
1243
+	if err != nil {
1244
+		return nil, nil, errors.Wrap(err, "list bridge addresses failed")
1245
+	}
1119 1246
 	if len(bridgeNws) > 0 {
1120 1247
 		// Pick any address from the bridge as a starting point.
1121
-		nw := bridgeNws[0]
1248
+		nw := bridgeNws[0].IPNet
1122 1249
 		if len(bridgeNws) > 1 && fCidrNet != nil {
1123 1250
 			// If there's an address with a subnet that contains fixed-cidr, use it.
1124 1251
 			for _, entry := range bridgeNws {
1125 1252
 				if entry.Contains(fCidrIP) {
1126
-					nw = entry
1253
+					nw = entry.IPNet
1127 1254
 					break
1128 1255
 				}
1129 1256
 				// For backwards compatibility - prefer the first bridge address within
... ...
@@ -1131,7 +1186,7 @@ func selectBIP(
1131 1131
 				// make sense - the allocatable range (fixed-cidr) will be bigger than the subnet
1132 1132
 				// (entry.IPNet).
1133 1133
 				if fCidrNet.Contains(entry.IP) && !fCidrNet.Contains(nw.IP) {
1134
-					nw = entry
1134
+					nw = entry.IPNet
1135 1135
 				}
1136 1136
 			}
1137 1137
 		}
... ...
@@ -1139,7 +1194,29 @@ func selectBIP(
1139 1139
 		bIP = nw.IP
1140 1140
 		bIPNet = lntypes.GetIPNetCanonical(nw)
1141 1141
 	}
1142
-	return bIP, bIPNet
1142
+
1143
+	if !userManagedBridge && fCidrIP != nil && bIPNet != nil {
1144
+		if !bIPNet.Contains(fCidrIP) {
1145
+			// The bridge is docker-managed (docker0) and fixed-cidr is not
1146
+			// inside a subnet belonging to any existing bridge IP. (fixed-cidr
1147
+			// has changed.) So, ignore the existing bridge IP.
1148
+			bIP = nil
1149
+			bIPNet = nil
1150
+		} else {
1151
+			fCidrOnes, _ := fCidrNet.Mask.Size()
1152
+			bIPOnes, _ := bIPNet.Mask.Size()
1153
+			if fCidrOnes < bIPOnes {
1154
+				// The bridge is docker-managed (docker0) and fixed-cidr (the
1155
+				// allocatable address range) is bigger than the subnet implied
1156
+				// by the bridge's current address. (fixed-cidr has changed.)
1157
+				// The bridge's address is ok, but its subnet needs to be updated.
1158
+				bIPNet.IP = bIPNet.IP.Mask(fCidrNet.Mask)
1159
+				bIPNet.Mask = fCidrNet.Mask
1160
+			}
1161
+		}
1162
+	}
1163
+
1164
+	return bIP, bIPNet, nil
1143 1165
 }
1144 1166
 
1145 1167
 // Remove default bridge interface if present (--bridge=none use case)