1. docker
  2. Commit b656cffe4cdfd9dd80b7003b2f386f0017fb5355
Browse code

Disallow "network generic data" with type options.Generic

Field 'generic' in 'libnetwork.Network' is used to store driver options,
it has type 'options.Generic', which is 'map[string]any'.

In that map, there may be a key 'netlabel.GenericData' holding options
known as "network generic options", used for options like:
-o com.docker.network.bridge.name=br-foo

The value type for key 'netlabel.GenericData' is always 'map[string]string'
when created via an API request. But, some unit tests use type
'options.Generic'.

That works because the bridge, ipvlan and macvlan drivers look for type
'options.Generic' as well as 'map[string]string'. If they find
'options.Generic', Go reflection is used to map keys to fields of the
config struct with the expectation that the value has the same type as
that field. But, that's only used in unit tests (so the tests aren't
testing the same code path as the API would use). The 'options.Generic'
form of the bridge name option is:
"BridgeName": "br-foo"
(Because "BridgeName" is the name of the field in the bridge driver's
network config struct.)

The libnetwork code expects "network generic options" to have type
'map[string]string', and makes no provision for 'options.Generic'. So,
for example, function Network.DriverOptions will panic if called when
'Network.generic[netlabel.GenericData]' has type 'options.Generic'.

The type of 'Network.generic[netlabel.GenericData]' can't be statically
checked, because it's just a field in a 'map[string]any'.

So - remove the driver code that converts "network generic options"
from type 'options.Generic', as it's only used in tests and just makes
things more confusing.

This should reduce the chances of things appearing to work when the
type is wrong, and converting unit tests to use 'map[string]string'
means they're testing the right thing.

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

Rob Murray authored on 2024/11/20 05:00:25
Showing 5 changed files
libnetwork/drivers/bridge/bridge_linux.go
History View file @ b656cff
... ... 
@@ -646,11 +646,6 @@ func parseNetworkGenericOptions(data interface{}) (*networkConfiguration, error)
646 646 
 			EnableIPMasquerade: true,
647 647 
 		}
648 648 
 		err = config.fromLabels(opt)
649 
-	case options.Generic:
650 
-		var opaqueConfig interface{}
651 
-		if opaqueConfig, err = options.GenerateFromModel(opt, config); err == nil {
652 
-			config = opaqueConfig.(*networkConfiguration)
653 
-		}
654 649 
 	default:
655 650 
 		err = types.InvalidParameterErrorf("do not recognize network configuration format: %T", opt)
656 651 
 	}
libnetwork/drivers/ipvlan/ipvlan_network.go
History View file @ b656cff
... ... 
@@ -227,13 +227,6 @@ func parseNetworkGenericOptions(data interface{}) (*configuration, error) {
227 227 
 		return opt, nil
228 228 
 	case map[string]string:
229 229 
 		return newConfigFromLabels(opt), nil
230 
-	case options.Generic:
231 
-		var config *configuration
232 
-		opaqueConfig, err := options.GenerateFromModel(opt, config)
233 
-		if err != nil {
234 
-			return nil, err
235 
-		}
236 
-		return opaqueConfig.(*configuration), nil
237 230 
 	default:
238 231 
 		return nil, types.InvalidParameterErrorf("unrecognized network configuration format: %v", opt)
239 232 
 	}
libnetwork/drivers/macvlan/macvlan_network.go
History View file @ b656cff
... ... 
@@ -236,13 +236,6 @@ func parseNetworkGenericOptions(data interface{}) (*configuration, error) {
236 236 
 		return opt, nil
237 237 
 	case map[string]string:
238 238 
 		return newConfigFromLabels(opt), nil
239 
-	case options.Generic:
240 
-		var config *configuration
241 
-		opaqueConfig, err := options.GenerateFromModel(opt, config)
242 
-		if err != nil {
243 
-			return nil, err
244 
-		}
245 
-		return opaqueConfig.(*configuration), nil
246 239 
 	default:
247 240 
 		return nil, types.InvalidParameterErrorf("unrecognized network configuration format: %v", opt)
248 241 
 	}
libnetwork/libnetwork_linux_test.go
History View file @ b656cff
... ... 
@@ -22,6 +22,7 @@ import (
22 22 
 	"github.com/docker/docker/libnetwork"
23 23 
 	"github.com/docker/docker/libnetwork/config"
24 24 
 	"github.com/docker/docker/libnetwork/driverapi"
25 
+	"github.com/docker/docker/libnetwork/drivers/bridge"
25 26 
 	"github.com/docker/docker/libnetwork/ipams/defaultipam"
26 27 
 	"github.com/docker/docker/libnetwork/ipams/null"
27 28 
 	"github.com/docker/docker/libnetwork/ipamutils"
... ... 
@@ -170,8 +171,8 @@ func TestNetworkName(t *testing.T) {
170 170
171 171 
 	netOption := options.Generic{
172 172 
 		netlabel.EnableIPv4: true,
173 
-		netlabel.GenericData: options.Generic{
174 
-			"BridgeName": "testnetwork",
173 
+		netlabel.GenericData: map[string]string{
174 
+			bridge.BridgeName: "testnetwork",
175 175 
 		},
176 176 
 	}
177 177
... ... 
@@ -206,8 +207,8 @@ func TestNetworkType(t *testing.T) {
206 206
207 207 
 	netOption := options.Generic{
208 208 
 		netlabel.EnableIPv4: true,
209 
-		netlabel.GenericData: options.Generic{
210 
-			"BridgeName": "testnetwork",
209 
+		netlabel.GenericData: map[string]string{
210 
+			bridge.BridgeName: "testnetwork",
211 211 
 		},
212 212 
 	}
213 213
... ... 
@@ -232,8 +233,8 @@ func TestNetworkID(t *testing.T) {
232 232
233 233 
 	netOption := options.Generic{
234 234 
 		netlabel.EnableIPv4: true,
235 
-		netlabel.GenericData: options.Generic{
236 
-			"BridgeName": "testnetwork",
235 
+		netlabel.GenericData: map[string]string{
236 
+			bridge.BridgeName: "testnetwork",
237 237 
 		},
238 238 
 	}
239 239
... ... 
@@ -256,12 +257,11 @@ func TestDeleteNetworkWithActiveEndpoints(t *testing.T) {
256 256 
 	defer netnsutils.SetupTestOSContext(t)()
257 257 
 	controller := newController(t)
258 258
259 
-	netOption := options.Generic{
260 
-		"BridgeName": "testnetwork",
261 
-	}
262 259 
 	option := options.Generic{
263 
-		netlabel.EnableIPv4:  true,
264 
-		netlabel.GenericData: netOption,
260 
+		netlabel.EnableIPv4: true,
261 
+		netlabel.GenericData: map[string]string{
262 
+			bridge.BridgeName: "testnetwork",
263 
+		},
265 264 
 	}
266 265
267 266 
 	network, err := createTestNetwork(controller, bridgeNetType, "testnetwork", option, nil, nil)
... ... 
@@ -311,11 +311,10 @@ func TestNetworkConfig(t *testing.T) {
311 311 
 	}
312 312
313 313 
 	// Create supported config network
314 
-	netOption := options.Generic{
315 
-		"EnableICC": false,
316 
-	}
317 314 
 	option := options.Generic{
318 
-		netlabel.GenericData: netOption,
315 
+		netlabel.GenericData: map[string]string{
316 
+			bridge.EnableICC: "false",
317 
+		},
319 318 
 	}
320 319 
 	ipamV4ConfList := []*libnetwork.IpamConf{{PreferredPool: "192.168.100.0/24", SubPool: "192.168.100.128/25", Gateway: "192.168.100.1"}}
321 320 
 	ipamV6ConfList := []*libnetwork.IpamConf{{PreferredPool: "2001:db8:abcd::/64", SubPool: "2001:db8:abcd::ef99/80", Gateway: "2001:db8:abcd::22"}}
... ... 
@@ -401,12 +400,11 @@ func TestUnknownNetwork(t *testing.T) {
401 401 
 	defer netnsutils.SetupTestOSContext(t)()
402 402 
 	controller := newController(t)
403 403
404 
-	netOption := options.Generic{
405 
-		"BridgeName": "testnetwork",
406 
-	}
407 404 
 	option := options.Generic{
408 
-		netlabel.EnableIPv4:  true,
409 
-		netlabel.GenericData: netOption,
405 
+		netlabel.EnableIPv4: true,
406 
+		netlabel.GenericData: map[string]string{
407 
+			bridge.BridgeName: "testnetwork",
408 
+		},
410 409 
 	}
411 410
412 411 
 	network, err := createTestNetwork(controller, bridgeNetType, "testnetwork", option, nil, nil)
... ... 
@@ -433,12 +431,11 @@ func TestUnknownEndpoint(t *testing.T) {
433 433 
 	defer netnsutils.SetupTestOSContext(t)()
434 434 
 	controller := newController(t)
435 435
436 
-	netOption := options.Generic{
437 
-		"BridgeName": "testnetwork",
438 
-	}
439 436 
 	option := options.Generic{
440 
-		netlabel.EnableIPv4:  true,
441 
-		netlabel.GenericData: netOption,
437 
+		netlabel.EnableIPv4: true,
438 
+		netlabel.GenericData: map[string]string{
439 
+			bridge.BridgeName: "testnetwork",
440 
+		},
442 441 
 	}
443 442 
 	ipamV4ConfList := []*libnetwork.IpamConf{{PreferredPool: "192.168.100.0/24"}}
444 443
... ... 
@@ -478,8 +475,8 @@ func TestNetworkEndpointsWalkers(t *testing.T) {
478 478 
 	// Create network 1 and add 2 endpoint: ep11, ep12
479 479 
 	netOption := options.Generic{
480 480 
 		netlabel.EnableIPv4: true,
481 
-		netlabel.GenericData: options.Generic{
482 
-			"BridgeName": "network1",
481 
+		netlabel.GenericData: map[string]string{
482 
+			bridge.BridgeName: "network1",
483 483 
 		},
484 484 
 	}
485 485
... ... 
@@ -552,8 +549,8 @@ func TestNetworkEndpointsWalkers(t *testing.T) {
552 552 
 	// Create network 2
553 553 
 	netOption = options.Generic{
554 554 
 		netlabel.EnableIPv4: true,
555 
-		netlabel.GenericData: options.Generic{
556 
-			"BridgeName": "network2",
555 
+		netlabel.GenericData: map[string]string{
556 
+			bridge.BridgeName: "network2",
557 557 
 		},
558 558 
 	}
559 559
... ... 
@@ -609,8 +606,8 @@ func TestDuplicateEndpoint(t *testing.T) {
609 609
610 610 
 	netOption := options.Generic{
611 611 
 		netlabel.EnableIPv4: true,
612 
-		netlabel.GenericData: options.Generic{
613 
-			"BridgeName": "testnetwork",
612 
+		netlabel.GenericData: map[string]string{
613 
+			bridge.BridgeName: "testnetwork",
614 614 
 		},
615 615 
 	}
616 616 
 	n, err := createTestNetwork(controller, bridgeNetType, "testnetwork", netOption, nil, nil)
... ... 
@@ -659,8 +656,8 @@ func TestControllerQuery(t *testing.T) {
659 659 
 	// Create network 1
660 660 
 	netOption := options.Generic{
661 661 
 		netlabel.EnableIPv4: true,
662 
-		netlabel.GenericData: options.Generic{
663 
-			"BridgeName": "network1",
662 
+		netlabel.GenericData: map[string]string{
663 
+			bridge.BridgeName: "network1",
664 664 
 		},
665 665 
 	}
666 666 
 	net1, err := createTestNetwork(controller, bridgeNetType, "network1", netOption, nil, nil)
... ... 
@@ -676,8 +673,8 @@ func TestControllerQuery(t *testing.T) {
676 676 
 	// Create network 2
677 677 
 	netOption = options.Generic{
678 678 
 		netlabel.EnableIPv4: true,
679 
-		netlabel.GenericData: options.Generic{
680 
-			"BridgeName": "network2",
679 
+		netlabel.GenericData: map[string]string{
680 
+			bridge.BridgeName: "network2",
681 681 
 		},
682 682 
 	}
683 683 
 	net2, err := createTestNetwork(controller, bridgeNetType, "network2", netOption, nil, nil)
... ... 
@@ -762,8 +759,8 @@ func TestNetworkQuery(t *testing.T) {
762 762 
 	// Create network 1 and add 2 endpoint: ep11, ep12
763 763 
 	netOption := options.Generic{
764 764 
 		netlabel.EnableIPv4: true,
765 
-		netlabel.GenericData: options.Generic{
766 
-			"BridgeName": "network1",
765 
+		netlabel.GenericData: map[string]string{
766 
+			bridge.BridgeName: "network1",
767 767 
 		},
768 768 
 	}
769 769 
 	net1, err := createTestNetwork(controller, bridgeNetType, "network1", netOption, nil, nil)
... ... 
@@ -848,8 +845,8 @@ func TestEndpointDeleteWithActiveContainer(t *testing.T) {
848 848
849 849 
 	n, err := createTestNetwork(controller, bridgeNetType, "testnetwork", options.Generic{
850 850 
 		netlabel.EnableIPv4: true,
851 
-		netlabel.GenericData: options.Generic{
852 
-			"BridgeName": "testnetwork",
851 
+		netlabel.GenericData: map[string]string{
852 
+			bridge.BridgeName: "testnetwork",
853 853 
 		},
854 854 
 	}, nil, nil)
855 855 
 	if err != nil {
... ... 
@@ -863,8 +860,8 @@ func TestEndpointDeleteWithActiveContainer(t *testing.T) {
863 863
864 864 
 	n2, err := createTestNetwork(controller, bridgeNetType, "testnetwork2", options.Generic{
865 865 
 		netlabel.EnableIPv4: true,
866 
-		netlabel.GenericData: options.Generic{
867 
-			"BridgeName": "testnetwork2",
866 
+		netlabel.GenericData: map[string]string{
867 
+			bridge.BridgeName: "testnetwork2",
868 868 
 		},
869 869 
 	}, nil, nil)
870 870 
 	if err != nil {
... ... 
@@ -924,8 +921,8 @@ func TestEndpointMultipleJoins(t *testing.T) {
924 924
925 925 
 	n, err := createTestNetwork(controller, bridgeNetType, "testmultiple", options.Generic{
926 926 
 		netlabel.EnableIPv4: true,
927 
-		netlabel.GenericData: options.Generic{
928 
-			"BridgeName": "testmultiple",
927 
+		netlabel.GenericData: map[string]string{
928 
+			bridge.BridgeName: "testmultiple",
929 929 
 		},
930 930 
 	}, nil, nil)
931 931 
 	if err != nil {
... ... 
@@ -998,8 +995,8 @@ func TestLeaveAll(t *testing.T) {
998 998
999 999 
 	n, err := createTestNetwork(controller, bridgeNetType, "testnetwork", options.Generic{
1000 1000 
 		netlabel.EnableIPv4: true,
1001 
-		netlabel.GenericData: options.Generic{
1002 
-			"BridgeName": "testnetwork",
1001 
+		netlabel.GenericData: map[string]string{
1002 
+			bridge.BridgeName: "testnetwork",
1003 1003 
 		},
1004 1004 
 	}, nil, nil)
1005 1005 
 	if err != nil {
... ... 
@@ -1014,8 +1011,8 @@ func TestLeaveAll(t *testing.T) {
1014 1014
1015 1015 
 	n2, err := createTestNetwork(controller, bridgeNetType, "testnetwork2", options.Generic{
1016 1016 
 		netlabel.EnableIPv4: true,
1017 
-		netlabel.GenericData: options.Generic{
1018 
-			"BridgeName": "testnetwork2",
1017 
+		netlabel.GenericData: map[string]string{
1018 
+			bridge.BridgeName: "testnetwork2",
1019 1019 
 		},
1020 1020 
 	}, nil, nil)
1021 1021 
 	if err != nil {
... ... 
@@ -1064,8 +1061,8 @@ func TestContainerInvalidLeave(t *testing.T) {
1064 1064
1065 1065 
 	n, err := createTestNetwork(controller, bridgeNetType, "testnetwork", options.Generic{
1066 1066 
 		netlabel.EnableIPv4: true,
1067 
-		netlabel.GenericData: options.Generic{
1068 
-			"BridgeName": "testnetwork",
1067 
+		netlabel.GenericData: map[string]string{
1068 
+			bridge.BridgeName: "testnetwork",
1069 1069 
 		},
1070 1070 
 	}, nil, nil)
1071 1071 
 	if err != nil {
... ... 
@@ -1130,8 +1127,8 @@ func TestEndpointUpdateParent(t *testing.T) {
1130 1130
1131 1131 
 	n, err := createTestNetwork(controller, bridgeNetType, "testnetwork", options.Generic{
1132 1132 
 		netlabel.EnableIPv4: true,
1133 
-		netlabel.GenericData: options.Generic{
1134 
-			"BridgeName": "testnetwork",
1133 
+		netlabel.GenericData: map[string]string{
1134 
+			bridge.BridgeName: "testnetwork",
1135 1135 
 		},
1136 1136 
 	}, nil, nil)
1137 1137 
 	if err != nil {
... ... 
@@ -1303,8 +1300,8 @@ func makeTestIPv6Network(t *testing.T, c *libnetwork.Controller) *libnetwork.Net
1303 1303 
 	netOptions := options.Generic{
1304 1304 
 		netlabel.EnableIPv4: true,
1305 1305 
 		netlabel.EnableIPv6: true,
1306 
-		netlabel.GenericData: options.Generic{
1307 
-			"BridgeName": "testnetwork",
1306 
+		netlabel.GenericData: map[string]string{
1307 
+			bridge.BridgeName: "testnetwork",
1308 1308 
 		},
1309 1309 
 	}
1310 1310 
 	ipamV6ConfList := []*libnetwork.IpamConf{
... ... 
@@ -1427,10 +1424,10 @@ func TestBridgeIpv6FromMac(t *testing.T) {
1427 1427 
 	controller := newController(t)
1428 1428
1429 1429 
 	netOption := options.Generic{
1430 
-		netlabel.GenericData: options.Generic{
1431 
-			"BridgeName":         "testipv6mac",
1432 
-			"EnableICC":          true,
1433 
-			"EnableIPMasquerade": true,
1430 
+		netlabel.GenericData: map[string]string{
1431 
+			bridge.BridgeName:         "testipv6mac",
1432 
+			bridge.EnableICC:          "true",
1433 
+			bridge.EnableIPMasquerade: "true",
1434 1434 
 		},
1435 1435 
 	}
1436 1436 
 	ipamV4ConfList := []*libnetwork.IpamConf{{PreferredPool: "192.168.100.0/24", Gateway: "192.168.100.1"}}
... ... 
@@ -1504,10 +1501,10 @@ func TestEndpointJoin(t *testing.T) {
1504 1504
1505 1505 
 	// Create network 1 and add 2 endpoint: ep11, ep12
1506 1506 
 	netOption := options.Generic{
1507 
-		netlabel.GenericData: options.Generic{
1508 
-			"BridgeName":         "testnetwork1",
1509 
-			"EnableICC":          true,
1510 
-			"EnableIPMasquerade": true,
1507 
+		netlabel.GenericData: map[string]string{
1508 
+			bridge.BridgeName:         "testnetwork1",
1509 
+			bridge.EnableICC:          "true",
1510 
+			bridge.EnableIPMasquerade: "true",
1511 1511 
 		},
1512 1512 
 	}
1513 1513 
 	ipamV6ConfList := []*libnetwork.IpamConf{{PreferredPool: "fe90::/64", Gateway: "fe90::22"}}
... ... 
@@ -1630,8 +1627,8 @@ func TestEndpointJoin(t *testing.T) {
1630 1630 
 	n2, err := createTestNetwork(controller, bridgeNetType, "testnetwork2",
1631 1631 
 		options.Generic{
1632 1632 
 			netlabel.EnableIPv4: true,
1633 
-			netlabel.GenericData: options.Generic{
1634 
-				"BridgeName": "testnetwork2",
1633 
+			netlabel.GenericData: map[string]string{
1634 
+				bridge.BridgeName: "testnetwork2",
1635 1635 
 			},
1636 1636 
 		}, nil, nil)
1637 1637 
 	if err != nil {
... ... 
@@ -1681,8 +1678,8 @@ func externalKeyTest(t *testing.T, reexec bool) {
1681 1681
1682 1682 
 	n, err := createTestNetwork(controller, bridgeNetType, "testnetwork", options.Generic{
1683 1683 
 		netlabel.EnableIPv4: true,
1684 
-		netlabel.GenericData: options.Generic{
1685 
-			"BridgeName": "testnetwork",
1684 
+		netlabel.GenericData: map[string]string{
1685 
+			bridge.BridgeName: "testnetwork",
1686 1686 
 		},
1687 1687 
 	}, nil, nil)
1688 1688 
 	if err != nil {
... ... 
@@ -1696,8 +1693,8 @@ func externalKeyTest(t *testing.T, reexec bool) {
1696 1696
1697 1697 
 	n2, err := createTestNetwork(controller, bridgeNetType, "testnetwork2", options.Generic{
1698 1698 
 		netlabel.EnableIPv4: true,
1699 
-		netlabel.GenericData: options.Generic{
1700 
-			"BridgeName": "testnetwork2",
1699 
+		netlabel.GenericData: map[string]string{
1700 
+			bridge.BridgeName: "testnetwork2",
1701 1701 
 		},
1702 1702 
 	}, nil, nil)
1703 1703 
 	if err != nil {
... ... 
@@ -1994,8 +1991,8 @@ func TestParallel(t *testing.T) {
1994 1994
1995 1995 
 	netOption := options.Generic{
1996 1996 
 		netlabel.EnableIPv4: true,
1997 
-		netlabel.GenericData: options.Generic{
1998 
-			"BridgeName": "network",
1997 
+		netlabel.GenericData: map[string]string{
1998 
+			bridge.BridgeName: "network",
1999 1999 
 		},
2000 2000 
 	}
2001 2001
... ... 
@@ -2056,10 +2053,10 @@ func TestBridge(t *testing.T) {
2056 2056 
 	netOption := options.Generic{
2057 2057 
 		netlabel.EnableIPv4: true,
2058 2058 
 		netlabel.EnableIPv6: true,
2059 
-		netlabel.GenericData: options.Generic{
2060 
-			"BridgeName":         "testnetwork",
2061 
-			"EnableICC":          true,
2062 
-			"EnableIPMasquerade": true,
2059 
+		netlabel.GenericData: map[string]string{
2060 
+			bridge.BridgeName:         "testnetwork",
2061 
+			bridge.EnableICC:          "true",
2062 
+			bridge.EnableIPMasquerade: "true",
2063 2063 
 		},
2064 2064 
 	}
2065 2065 
 	ipamV4ConfList := []*libnetwork.IpamConf{{PreferredPool: "192.168.100.0/24", Gateway: "192.168.100.1"}}
libnetwork/sandbox_unix_test.go
History View file @ b656cff
... ... 
@@ -10,6 +10,7 @@ import (
10 10 
 	"github.com/docker/docker/errdefs"
11 11 
 	"github.com/docker/docker/internal/testutils/netnsutils"
12 12 
 	"github.com/docker/docker/libnetwork/config"
13 
+	"github.com/docker/docker/libnetwork/drivers/bridge"
13 14 
 	"github.com/docker/docker/libnetwork/ipams/defaultipam"
14 15 
 	"github.com/docker/docker/libnetwork/ipamutils"
15 16 
 	"github.com/docker/docker/libnetwork/netlabel"
... ... 
@@ -42,7 +43,9 @@ func getTestEnv(t *testing.T, opts ...[]NetworkOption) (*Controller, []*Network)
42 42 
 		name := "test_nw_" + strconv.Itoa(i)
43 43 
 		newOptions := []NetworkOption{
44 44 
 			NetworkOptionGeneric(options.Generic{
45 
-				netlabel.GenericData: options.Generic{"BridgeName": name},
45 
+				netlabel.GenericData: map[string]string{
46 
+					bridge.BridgeName: name,
47 
+				},
46 48 
 			}),
47 49 
 		}
48 50 
 		newOptions = append(newOptions, opt...)