Merge pull request moby#50864 from akerouanton/bridge-driver-config

libnet/d/bridge: Register: pass a Configuration struct

Author: akerouanton

daemon/daemon.go

@@ -1649,7 +1649,6 @@ func (daemon *Daemon) networkOptions(conf *config.Config, pg plugingetter.Plugin
 		nwconfig.OptionExecRoot(conf.GetExecRoot()),
 		nwconfig.OptionDefaultDriver(network.DefaultNetwork),
 		nwconfig.OptionDefaultNetwork(network.DefaultNetwork),
-		nwconfig.OptionLabels(conf.Labels),
 		nwconfig.OptionNetworkControlPlaneMTU(conf.NetworkControlPlaneMTU),
 		nwconfig.OptionFirewallBackend(conf.FirewallBackend),
 	}

daemon/daemon_unix.go

@@ -35,7 +35,6 @@ import (
 	"github.com/moby/moby/v2/daemon/libnetwork/drivers/bridge"
 	"github.com/moby/moby/v2/daemon/libnetwork/netlabel"
 	"github.com/moby/moby/v2/daemon/libnetwork/nlwrap"
-	"github.com/moby/moby/v2/daemon/libnetwork/options"
 	lntypes "github.com/moby/moby/v2/daemon/libnetwork/types"
 	"github.com/moby/moby/v2/daemon/pkg/opts"
 	volumemounts "github.com/moby/moby/v2/daemon/volume/mounts"
@@ -927,17 +926,15 @@ func networkPlatformOptions(conf *config.Config) []nwconfig.Option {
 	return []nwconfig.Option{
 		nwconfig.OptionRootless(conf.Rootless),
 		nwconfig.OptionUserlandProxy(conf.EnableUserlandProxy, conf.UserlandProxyPath),
-		nwconfig.OptionDriverConfig("bridge", options.Generic{
-			netlabel.GenericData: options.Generic{
-				"EnableIPForwarding":       conf.BridgeConfig.EnableIPForward,
-				"DisableFilterForwardDrop": conf.BridgeConfig.DisableFilterForwardDrop,
-				"EnableIPTables":           conf.BridgeConfig.EnableIPTables,
-				"EnableIP6Tables":          conf.BridgeConfig.EnableIP6Tables,
-				"EnableProxy":              conf.EnableUserlandProxy && conf.UserlandProxyPath != "",
-				"ProxyPath":                conf.UserlandProxyPath,
-				"AllowDirectRouting":       conf.BridgeConfig.AllowDirectRouting,
-				"AcceptFwMark":             conf.BridgeConfig.BridgeAcceptFwMark,
-			},
+		nwconfig.OptionBridgeConfig(bridge.Configuration{
+			EnableIPForwarding:       conf.BridgeConfig.EnableIPForward,
+			DisableFilterForwardDrop: conf.BridgeConfig.DisableFilterForwardDrop,
+			EnableIPTables:           conf.BridgeConfig.EnableIPTables,
+			EnableIP6Tables:          conf.BridgeConfig.EnableIP6Tables,
+			EnableProxy:              conf.EnableUserlandProxy && conf.UserlandProxyPath != "",
+			ProxyPath:                conf.UserlandProxyPath,
+			AllowDirectRouting:       conf.BridgeConfig.AllowDirectRouting,
+			AcceptFwMark:             conf.BridgeConfig.BridgeAcceptFwMark,
 		}),
 	}
 }

daemon/libnetwork/config/config.go

@@ -8,7 +8,6 @@ import (
 	"github.com/moby/moby/v2/daemon/libnetwork/cluster"
 	"github.com/moby/moby/v2/daemon/libnetwork/datastore"
 	"github.com/moby/moby/v2/daemon/libnetwork/ipamutils"
-	"github.com/moby/moby/v2/daemon/libnetwork/netlabel"
 	"github.com/moby/moby/v2/pkg/plugingetter"
 )
 
@@ -19,6 +18,8 @@ const (
 
 // Config encapsulates configurations of various Libnetwork components
 type Config struct {
+	PlatformConfig
+
 	DataDir string
 	// ExecRoot is the base-path for libnetwork external key listeners
 	// (created in "<ExecRoot>/libnetwork/<Controller-Short-ID>.sock"),
@@ -32,7 +33,6 @@ type Config struct {
 	DefaultNetwork         string
 	DefaultDriver          string
 	Labels                 []string
-	driverCfg              map[string]map[string]any
 	ClusterProvider        cluster.Provider
 	NetworkControlPlaneMTU int
 	DefaultAddressPool     []*ipamutils.NetworkToSplit
@@ -48,7 +48,6 @@ type Config struct {
 // New creates a new Config and initializes it with the given Options.
 func New(opts ...Option) *Config {
 	cfg := &Config{
-		driverCfg:       make(map[string]map[string]any),
 		DatastoreBucket: datastore.DefaultBucket,
 	}
 
@@ -61,10 +60,6 @@ func New(opts ...Option) *Config {
 	return cfg
 }
 
-func (c *Config) DriverConfig(name string) map[string]any {
-	return c.driverCfg[name]
-}
-
 // Option is an option setter function type used to pass various configurations
 // to the controller
 type Option func(c *Config)
@@ -92,24 +87,6 @@ func OptionDefaultAddressPoolConfig(addressPool []*ipamutils.NetworkToSplit) Opt
 	}
 }
 
-// OptionDriverConfig returns an option setter for driver configuration.
-func OptionDriverConfig(networkType string, config map[string]any) Option {
-	return func(c *Config) {
-		c.driverCfg[networkType] = config
-	}
-}
-
-// OptionLabels function returns an option setter for labels
-func OptionLabels(labels []string) Option {
-	return func(c *Config) {
-		for _, label := range labels {
-			if strings.HasPrefix(label, netlabel.Prefix) {
-				c.Labels = append(c.Labels, label)
-			}
-		}
-	}
-}
-
 // OptionDataDir function returns an option setter for data folder
 func OptionDataDir(dataDir string) Option {
 	return func(c *Config) {

daemon/libnetwork/config/config_freebsd.go

@@ -1,6 +1,21 @@
 package config
 
-import "github.com/moby/moby/v2/daemon/libnetwork/osl"
+import (
+	"github.com/moby/moby/v2/daemon/libnetwork/drivers/bridge"
+	"github.com/moby/moby/v2/daemon/libnetwork/osl"
+)
+
+// PlatformConfig defines platform-specific configuration.
+type PlatformConfig struct {
+	BridgeConfig bridge.Configuration
+}
+
+// OptionBridgeConfig returns an option setter for bridge driver config.
+func OptionBridgeConfig(config bridge.Configuration) Option {
+	return func(c *Config) {
+		c.BridgeConfig = config
+	}
+}
 
 // optionExecRoot on Linux sets both the controller's ExecRoot and osl.basePath.
 func optionExecRoot(execRoot string) Option {

daemon/libnetwork/config/config_linux.go

@@ -1,7 +1,8 @@
-//go:build !linux && !freebsd
-
 package config
 
+// PlatformConfig defines platform-specific configuration.
+type PlatformConfig struct{}
+
 // optionExecRoot is a no-op on non-unix platforms.
 func optionExecRoot(execRoot string) Option {
 	return func(*Config) {}

daemon/libnetwork/config/config_test.go

@@ -187,7 +187,7 @@ func New(ctx context.Context, cfgOptions ...config.Option) (_ *Controller, retEr
 		return nil, err
 	}
 
-	if err := registerNetworkDrivers(&c.drvRegistry, c.store, &c.pmRegistry, c.makeDriverConfig); err != nil {
+	if err := registerNetworkDrivers(&c.drvRegistry, c.cfg, c.store, &c.pmRegistry); err != nil {
 		return nil, err
 	}
 
@@ -383,29 +383,6 @@ func (c *Controller) agentStopComplete() {
 	c.mu.Unlock()
 }
 
-func (c *Controller) makeDriverConfig(ntype string) map[string]any {
-	if c.cfg == nil {
-		return nil
-	}
-
-	cfg := map[string]any{}
-	for _, label := range c.cfg.Labels {
-		key, val, _ := strings.Cut(label, "=")
-		if !strings.HasPrefix(key, netlabel.DriverPrefix+"."+ntype) {
-			continue
-		}
-
-		cfg[key] = val
-	}
-
-	// Merge in the existing config for this driver.
-	for k, v := range c.cfg.DriverConfig(ntype) {
-		cfg[k] = v
-	}
-
-	return cfg
-}
-
 // ID returns the controller's unique identity.
 func (c *Controller) ID() string {
 	return c.id

…/libnetwork/config/config_unsupported.go …emon/libnetwork/config/config_windows.godaemon/libnetwork/config/config_unsupported.go renamed to daemon/libnetwork/config/config_windows.go daemon/libnetwork/config/config_unsupported.go renamed to daemon/libnetwork/config/config_windows.go

@@ -10,8 +10,6 @@ import (
 	"github.com/moby/moby/api/types/system"
 	"github.com/moby/moby/v2/daemon/libnetwork/internal/nftables"
 	"github.com/moby/moby/v2/daemon/libnetwork/iptables"
-	"github.com/moby/moby/v2/daemon/libnetwork/netlabel"
-	"github.com/moby/moby/v2/daemon/libnetwork/options"
 	"github.com/moby/moby/v2/daemon/libnetwork/osl"
 )
 
@@ -34,24 +32,11 @@ func (c *Controller) FirewallBackend() *system.FirewallInfo {
 // enabledIptablesVersions returns the iptables versions that are enabled
 // for the controller.
 func (c *Controller) enabledIptablesVersions() []iptables.IPVersion {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-	if c.cfg == nil {
-		return nil
-	}
-	// parse map cfg["bridge"]["generic"]["EnableIPTable"]
-	cfgBridge := c.cfg.DriverConfig("bridge")
-	cfgGeneric, ok := cfgBridge[netlabel.GenericData].(options.Generic)
-	if !ok {
-		return nil
-	}
-
 	var versions []iptables.IPVersion
-	if enabled, ok := cfgGeneric["EnableIPTables"].(bool); enabled || !ok {
-		// iptables is enabled unless user explicitly disabled it
+	if c.cfg.BridgeConfig.EnableIPTables {
 		versions = append(versions, iptables.IPv4)
 	}
-	if enabled, _ := cfgGeneric["EnableIP6Tables"].(bool); enabled {
+	if c.cfg.BridgeConfig.EnableIP6Tables {
 		versions = append(versions, iptables.IPv6)
 	}
 	return versions

daemon/libnetwork/controller.go

@@ -63,8 +63,8 @@ const spanPrefix = "libnetwork.drivers.bridge"
 // FIXME(robmry) - it doesn't belong here.
 const DockerForwardChain = iptabler.DockerForwardChain
 
-// configuration info for the "bridge" driver.
-type configuration struct {
+// Configuration info for the "bridge" driver.
+type Configuration struct {
 	EnableIPForwarding       bool
 	DisableFilterForwardDrop bool
 	EnableIPTables           bool
@@ -156,7 +156,7 @@ type bridgeNetwork struct {
 }
 
 type driver struct {
-	config        configuration
+	config        Configuration
 	networks      map[string]*bridgeNetwork
 	store         *datastore.Store
 	nlh           nlwrap.Handle
@@ -178,19 +178,40 @@ const (
 )
 
 // New constructs a new bridge driver
-func newDriver(store *datastore.Store, pms *drvregistry.PortMappers) *driver {
-	return &driver{
+func newDriver(store *datastore.Store, config Configuration, pms *drvregistry.PortMappers) (*driver, error) {
+	fw, err := newFirewaller(context.Background(), firewaller.Config{
+		IPv4:               config.EnableIPTables,
+		IPv6:               config.EnableIP6Tables,
+		Hairpin:            !config.EnableProxy,
+		AllowDirectRouting: config.AllowDirectRouting,
+		WSL2Mirrored:       isRunningUnderWSL2MirroredMode(context.Background()),
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	d := &driver{
 		store:       store,
+		config:      config,
 		nlh:         ns.NlHandle(),
 		networks:    map[string]*bridgeNetwork{},
+		firewaller:  fw,
 		portmappers: pms,
 	}
+
+	if err := d.initStore(); err != nil {
+		return nil, err
+	}
+
+	iptables.OnReloaded(d.handleFirewalldReload)
+
+	return d, nil
 }
 
 // Register registers a new instance of bridge driver.
-func Register(r driverapi.Registerer, store *datastore.Store, pms *drvregistry.PortMappers, config map[string]any) error {
-	d := newDriver(store, pms)
-	if err := d.configure(config); err != nil {
+func Register(r driverapi.Registerer, store *datastore.Store, pms *drvregistry.PortMappers, config Configuration) error {
+	d, err := newDriver(store, config, pms)
+	if err != nil {
 		return err
 	}
 	return r.RegisterDriver(NetworkType, d, driverapi.Capability{
@@ -496,48 +517,6 @@ func (n *bridgeNetwork) getEndpoint(eid string) (*bridgeEndpoint, error) {
 	return nil, nil
 }
 
-func (d *driver) configure(option map[string]any) error {
-	var config configuration
-	switch opt := option[netlabel.GenericData].(type) {
-	case options.Generic:
-		opaqueConfig, err := options.GenerateFromModel(opt, &configuration{})
-		if err != nil {
-			return err
-		}
-		config = *opaqueConfig.(*configuration)
-	case *configuration:
-		config = *opt
-	case nil:
-		// No GenericData option set. Use defaults.
-	default:
-		return errdefs.InvalidParameter(fmt.Errorf("invalid configuration type (%T) passed", opt))
-	}
-
-	var err error
-	d.firewaller, err = newFirewaller(context.Background(), firewaller.Config{
-		IPv4:               config.EnableIPTables,
-		IPv6:               config.EnableIP6Tables,
-		Hairpin:            !config.EnableProxy,
-		AllowDirectRouting: config.AllowDirectRouting,
-		WSL2Mirrored:       isRunningUnderWSL2MirroredMode(context.Background()),
-	})
-	if err != nil {
-		return err
-	}
-
-	d.mu.Lock()
-	d.config = config
-	d.mu.Unlock()
-
-	// Register for an event when firewalld is reloaded, but take the config lock so
-	// that events won't be processed until the initial load from Store is complete.
-	d.configNetwork.Lock()
-	defer d.configNetwork.Unlock()
-	iptables.OnReloaded(d.handleFirewalldReload)
-
-	return d.initStore()
-}
-
 var newFirewaller = func(ctx context.Context, config firewaller.Config) (firewaller.Firewaller, error) {
 	if nftables.Enabled() {
 		fw, err := nftabler.NewNftabler(ctx, config)
@@ -1057,7 +1036,6 @@ func (d *driver) CreateEndpoint(ctx context.Context, nid, eid string, ifInfo dri
 	// Get the network handler and make sure it exists
 	d.mu.Lock()
 	n, ok := d.networks[nid]
-	dconfig := d.config
 	d.mu.Unlock()
 
 	if !ok {
@@ -1169,7 +1147,7 @@ func (d *driver) CreateEndpoint(ctx context.Context, nid, eid string, ifInfo dri
 		return fmt.Errorf("adding interface %s to bridge %s failed: %v", hostIfName, config.BridgeName, err)
 	}
 
-	if !dconfig.EnableProxy {
+	if !d.config.EnableProxy {
 		err = setHairpinMode(d.nlh, host, true)
 		if err != nil {
 			return err