Merge pull request #50357 from robmry/firewall_backend_option

Add daemon option --firewall-backend

daemon/command/config_unix.go

@@ -51,6 +51,7 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) {
 	flags.BoolVar(&conf.NoNewPrivileges, "no-new-privileges", false, "Set no-new-privileges by default for new containers")
 	flags.StringVar(&conf.IpcMode, "default-ipc-mode", conf.IpcMode, `Default mode for containers ipc ("shareable" | "private")`)
 	flags.Var(&conf.NetworkConfig.DefaultAddressPools, "default-address-pool", "Default address pools for node specific local networks")
+	flags.StringVar(&conf.NetworkConfig.FirewallBackend, "firewall-backend", "", "Firewall backend to use, iptables or nftables")
 	// rootless needs to be explicitly specified for running "rootful" dockerd in rootless dockerd (#38702)
 	// Note that conf.BridgeConfig.UserlandProxyPath and honorXDG are configured according to the value of rootless.RunningWithRootlessKit, not the value of --rootless.
 	flags.BoolVar(&conf.Rootless, "rootless", conf.Rootless, "Enable rootless mode; typically used with RootlessKit")

daemon/config/config.go

@@ -148,6 +148,10 @@ type NetworkConfig struct {
 	NetworkControlPlaneMTU int `json:"network-control-plane-mtu,omitempty"`
 	// Default options for newly created networks
 	DefaultNetworkOpts map[string]map[string]string `json:"default-network-opts,omitempty"`
+	// FirewallBackend overrides the daemon's default selection of firewall
+	// implementation. Currently only used on Linux, it is an error to
+	// supply a value for other platforms.
+	FirewallBackend string `json:"firewall-backend,omitempty"`
 }
 
 // TLSOptions defines TLS configuration for the daemon server.

daemon/config/config_linux.go

@@ -243,6 +243,10 @@ func validatePlatformConfig(conf *Config) error {
 		return errors.Wrap(err, "invalid fixed-cidr-v6")
 	}
 
+	if err := validateFirewallBackend(conf.FirewallBackend); err != nil {
+		return errors.Wrap(err, "invalid firewall-backend")
+	}
+
 	return verifyDefaultCgroupNsMode(conf.CgroupNamespaceMode)
 }
 
@@ -294,6 +298,14 @@ func verifyDefaultIpcMode(mode string) error {
 	return nil
 }
 
+func validateFirewallBackend(val string) error {
+	switch val {
+	case "", "iptables", "nftables":
+		return nil
+	}
+	return errors.New(`allowed values are "iptables" and "nftables"`)
+}
+
 func verifyDefaultCgroupNsMode(mode string) error {
 	cm := container.CgroupnsMode(mode)
 	if !cm.Valid() {

daemon/config/config_windows.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -88,6 +89,9 @@ func validatePlatformConfig(conf *Config) error {
 	if conf.MTU != 0 && conf.MTU != DefaultNetworkMtu {
 		log.G(context.TODO()).Warn(`WARNING: MTU for the default network is not configurable on Windows, and this option will be ignored.`)
 	}
+	if conf.FirewallBackend != "" {
+		return errors.New("firewall-backend can only be configured on Linux")
+	}
 	return nil
 }
 

daemon/daemon.go

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

daemon/libnetwork/config/config.go

@@ -42,6 +42,7 @@ type Config struct {
 	DatastoreBucket        string
 	ActiveSandboxes        map[string]any
 	PluginGetter           plugingetter.PluginGetter
+	FirewallBackend        string
 }
 
 // New creates a new Config and initializes it with the given Options.
@@ -154,3 +155,10 @@ func OptionActiveSandboxes(sandboxes map[string]any) Option {
 		c.ActiveSandboxes = sandboxes
 	}
 }
+
+// OptionFirewallBackend returns an option setter for selection of the firewall backend.
+func OptionFirewallBackend(val string) Option {
+	return func(c *Config) {
+		c.FirewallBackend = val
+	}
+}

daemon/libnetwork/controller.go

@@ -168,7 +168,9 @@ func New(ctx context.Context, cfgOptions ...config.Option) (_ *Controller, retEr
 		diagnosticServer: diagnostic.New(),
 	}
 
-	c.selectFirewallBackend()
+	if err := c.selectFirewallBackend(); err != nil {
+		return nil, err
+	}
 	c.drvRegistry.Notify = c
 
 	// External plugins don't need config passed through daemon. They can

daemon/libnetwork/firewall_linux.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"os"
 
 	"github.com/containerd/log"
 	"github.com/docker/docker/daemon/libnetwork/internal/nftables"
@@ -13,16 +12,23 @@ import (
 
 const userChain = "DOCKER-USER"
 
-func (c *Controller) selectFirewallBackend() {
-	// Don't try to enable nftables if firewalld is running.
-	if iptables.UsingFirewalld() {
-		return
+func (c *Controller) selectFirewallBackend() error {
+	// If explicitly configured to use iptables, don't consider nftables.
+	if c.cfg.FirewallBackend == "iptables" {
+		return nil
 	}
-	// Only try to use nftables if explicitly enabled by env-var.
-	// TODO(robmry) - command line options?
-	if os.Getenv("DOCKER_FIREWALL_BACKEND") == "nftables" {
-		_ = nftables.Enable()
+	// If configured to use nftables, but it can't be initialised, return an error.
+	if c.cfg.FirewallBackend == "nftables" {
+		// Don't try to enable nftables if firewalld is running.
+		if iptables.UsingFirewalld() {
+			return errors.New("firewall-backend is set to nftables, but firewalld is running")
+		}
+		if err := nftables.Enable(); err != nil {
+			return fmt.Errorf("firewall-backend is set to nftables: %v", err)
+		}
+		return nil
 	}
+	return nil
 }
 
 // Sets up the DOCKER-USER chain for each iptables version (IPv4, IPv6) that's

daemon/libnetwork/firewall_others.go

@@ -2,6 +2,8 @@
 
 package libnetwork
 
-func (c *Controller) selectFirewallBackend() {}
+func (c *Controller) selectFirewallBackend() error {
+	return nil
+}
 
 func (c *Controller) setupUserChains() {}

daemon/libnetwork/internal/nftables/nftables_linux.go

@@ -62,6 +62,8 @@ var (
 	// nftPath is the path of the "nft" tool, set by [Enable] and left empty if the tool
 	// is not present - in which case, nftables is disabled.
 	nftPath string
+	// Error returned by Enable if nftables could not be initialised.
+	nftEnableError error
 	// incrementalUpdateTempl is a parsed text/template, used to apply incremental updates.
 	incrementalUpdateTempl *template.Template
 	// reloadTempl is a parsed text/template, used to apply a whole table.
@@ -134,21 +136,23 @@ const (
 	nftTypeIfname      nftType = "ifname"
 )
 
-// Enable checks whether the "nft" tool is available, and returns true if it is.
-// Subsequent calls to [Enabled] will return the same result.
-func Enable() bool {
+// Enable tries once to initialise nftables.
+func Enable() error {
 	enableOnce.Do(func() {
 		path, err := exec.LookPath("nft")
 		if err != nil {
 			log.G(context.Background()).WithError(err).Warnf("Failed to find nft tool")
+			nftEnableError = fmt.Errorf("failed to find nft tool: %w", err)
+			return
 		}
 		if err := parseTemplate(); err != nil {
 			log.G(context.Background()).WithError(err).Error("Internal error while initialising nftables")
+			nftEnableError = fmt.Errorf("internal error while initialising nftables: %w", err)
 			return
 		}
 		nftPath = path
 	})
-	return nftPath != ""
+	return nftEnableError
 }
 
 // Enabled returns true if the "nft" tool is available and [Enable] has been called.

daemon/libnetwork/internal/nftables/nftables_linux_test.go

@@ -14,7 +14,7 @@ import (
 
 func testSetup(t *testing.T) func() {
 	t.Helper()
-	if !Enable() {
+	if err := Enable(); err != nil {
 		// Make sure it didn't fail because of a bug in the text/template.
 		assert.NilError(t, parseTemplate())
 		// If this is not CI, skip.
@@ -22,7 +22,7 @@ func testSetup(t *testing.T) func() {
 			t.Skip("Cannot enable nftables, no 'nft' command in $PATH ?")
 		}
 		// In CI, nft should always be installed, fail the test.
-		t.Fatal("Failed to enable nftables")
+		t.Fatalf("Failed to enable nftables: %s", err)
 	}
 	cleanupContext := netnsutils.SetupTestOSContext(t)
 	return func() {

hack/make/.integration-daemon-start

@@ -118,6 +118,7 @@ if [ -z "$DOCKER_TEST_HOST" ]; then
 			--storage-driver "$DOCKER_GRAPHDRIVER" \
 			--pidfile "$DEST/docker.pid" \
 			--userland-proxy="$DOCKER_USERLANDPROXY" \
+			--firewall-backend="$DOCKER_FIREWALL_BACKEND" \
 			${storage_params} \
 			${extra_params} \
 			&> "$DEST/docker.log"

hack/make/run

@@ -58,6 +58,7 @@ args=(
 	--host="unix://${socket}"
 	--storage-driver="${DOCKER_GRAPHDRIVER}"
 	--userland-proxy="${DOCKER_USERLANDPROXY}"
+	--firewall-backend="${DOCKER_FIREWALL_BACKEND}"
 	--tls=false
 	$storage_params
 	$extra_params

hack/test/e2e-run.sh

@@ -47,6 +47,7 @@ test_env() {
 			DOCKER_CERT_PATH="$DOCKER_TEST_CERT_PATH" \
 			DOCKER_GRAPHDRIVER="$DOCKER_GRAPHDRIVER" \
 			DOCKER_USERLANDPROXY="$DOCKER_USERLANDPROXY" \
+			DOCKER_FIREWALL_BACKEND="$DOCKER_FIREWALL_BACKEND" \
 			DOCKER_HOST="$DOCKER_HOST" \
 			DOCKER_REMAP_ROOT="$DOCKER_REMAP_ROOT" \
 			DOCKER_REMOTE_DAEMON="$DOCKER_REMOTE_DAEMON" \

integration/network/bridge/bridge_linux_test.go

@@ -856,9 +856,8 @@ func TestFirewallBackendSwitch(t *testing.T) {
 
 	networkCreated := false
 	runDaemon := func(backend string) {
-		d.SetEnvVar("DOCKER_FIREWALL_BACKEND", backend)
 		host.Do(t, func() {
-			d.StartWithBusybox(ctx, t)
+			d.StartWithBusybox(ctx, t, "--firewall-backend="+backend)
 			defer d.Stop(t)
 
 			// Create a network (and its firewall rules) first time through.

testutil/daemon/daemon.go

@@ -234,10 +234,6 @@ func New(t testing.TB, ops ...Option) *Daemon {
 	}
 	ops = append(ops, WithOOMScoreAdjust(-500))
 
-	if val, ok := os.LookupEnv("DOCKER_FIREWALL_BACKEND"); ok {
-		ops = append(ops, WithEnvVars("DOCKER_FIREWALL_BACKEND="+val))
-	}
-
 	d, err := NewDaemon(dest, ops...)
 	assert.NilError(t, err, "could not create daemon at %q", dest)
 	if d.rootlessUser != nil && d.dockerdBinary != defaultDockerdBinary {
@@ -534,6 +530,15 @@ func (d *Daemon) StartWithLogFile(out *os.File, providedArgs ...string) error {
 		d.args = append(d.args, "--storage-driver", d.storageDriver)
 	}
 
+	hasFwBackendArg := !slices.ContainsFunc(providedArgs, func(s string) bool {
+		return strings.HasPrefix(s, "--firewall-backend")
+	})
+	if hasFwBackendArg {
+		if fw := os.Getenv("DOCKER_FIREWALL_BACKEND"); fw != "" {
+			d.args = append(d.args, "--firewall-backend="+fw)
+		}
+	}
+
 	d.args = append(d.args, providedArgs...)
 	cmd := exec.Command(dockerdBinary, d.args...)
 	cmd.Env = append(os.Environ(), "DOCKER_SERVICE_PREFER_OFFLINE_IMAGE=1")