feat: add support for com.docker.network.bridge.enable_icc network option

Signed-off-by: Swagat Bora <[email protected]>

Author: swagatbora90

cmd/nerdctl/network/network_create_linux_test.go

@@ -26,6 +26,7 @@ import (
 	"gotest.tools/v3/assert"
 
 	"github.com/containerd/nerdctl/mod/tigron/expect"
+	"github.com/containerd/nerdctl/mod/tigron/require"
 	"github.com/containerd/nerdctl/mod/tigron/test"
 	"github.com/containerd/nerdctl/mod/tigron/tig"
 
@@ -159,3 +160,100 @@ func TestNetworkCreate(t *testing.T) {
 
 	testCase.Run(t)
 }
+
+func TestNetworkCreateICC(t *testing.T) {
+	testCase := nerdtest.Setup()
+
+	testCase.Require = require.All(
+		require.Linux,
+	)
+
+	testCase.SubTests = []*test.Case{
+		{
+			Description: "with enable_icc=false",
+			Require:     nerdtest.CNIFirewallVersion("1.7.1"),
+			NoParallel:  true,
+			Setup: func(data test.Data, helpers test.Helpers) {
+				// Create a network with ICC disabled
+				helpers.Ensure("network", "create", data.Identifier(), "--driver", "bridge",
+					"--opt", "com.docker.network.bridge.enable_icc=false")
+
+				// Run a container in that network
+				data.Labels().Set("container1", helpers.Capture("run", "-d", "--net", data.Identifier(),
+					"--name", data.Identifier("c1"), testutil.CommonImage, "sleep", "infinity"))
+
+				// Wait for container to be running
+				nerdtest.EnsureContainerStarted(helpers, data.Identifier("c1"))
+			},
+			Cleanup: func(data test.Data, helpers test.Helpers) {
+				helpers.Anyhow("container", "rm", "-f", data.Identifier("c1"))
+				helpers.Anyhow("network", "rm", data.Identifier())
+			},
+			Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
+				// DEBUG: Check br_netfilter module status
+				helpers.Custom("sh", "-ec", "lsmod | grep br_netfilter || echo 'br_netfilter not loaded'").Run(&test.Expected{})
+				helpers.Custom("sh", "-ec", "cat /proc/sys/net/bridge/bridge-nf-call-iptables 2>/dev/null || echo 'bridge-nf-call-iptables not available'").Run(&test.Expected{})
+				helpers.Custom("sh", "-ec", "ls /proc/sys/net/bridge/ 2>/dev/null || echo 'bridge sysctl not available'").Run(&test.Expected{})
+				// Try to ping the other container in the same network
+				// This should fail when ICC is disabled
+				return helpers.Command("run", "--rm", "--net", data.Identifier(),
+					testutil.CommonImage, "ping", "-c", "1", "-W", "1", data.Identifier("c1"))
+			},
+			Expected: test.Expects(expect.ExitCodeGenericFail, nil, nil), // Expect ping to fail with exit code 1
+		},
+		{
+			Description: "with enable_icc=true",
+			Require:     nerdtest.CNIFirewallVersion("1.7.1"),
+			NoParallel:  true,
+			Setup: func(data test.Data, helpers test.Helpers) {
+				// Create a network with ICC enabled (default)
+				helpers.Ensure("network", "create", data.Identifier(), "--driver", "bridge",
+					"--opt", "com.docker.network.bridge.enable_icc=true")
+
+				// Run a container in that network
+				data.Labels().Set("container1", helpers.Capture("run", "-d", "--net", data.Identifier(),
+					"--name", data.Identifier("c1"), testutil.CommonImage, "sleep", "infinity"))
+				// Wait for container to be running
+				nerdtest.EnsureContainerStarted(helpers, data.Identifier("c1"))
+			},
+			Cleanup: func(data test.Data, helpers test.Helpers) {
+				helpers.Anyhow("container", "rm", "-f", data.Identifier("c1"))
+				helpers.Anyhow("network", "rm", data.Identifier())
+			},
+			Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
+				// Try to ping the other container in the same network
+				// This should succeed when ICC is enabled
+				return helpers.Command("run", "--rm", "--net", data.Identifier(),
+					testutil.CommonImage, "ping", "-c", "1", "-W", "1", data.Identifier("c1"))
+			},
+			Expected: test.Expects(0, nil, nil), // Expect ping to succeed with exit code 0
+		},
+		{
+			Description: "with no enable_icc option set",
+			NoParallel:  true,
+			Setup: func(data test.Data, helpers test.Helpers) {
+				// Create a network with ICC enabled (default)
+				helpers.Ensure("network", "create", data.Identifier(), "--driver", "bridge")
+
+				// Run a container in that network
+				data.Labels().Set("container1", helpers.Capture("run", "-d", "--net", data.Identifier(),
+					"--name", data.Identifier("c1"), testutil.CommonImage, "sleep", "infinity"))
+				// Wait for container to be running
+				nerdtest.EnsureContainerStarted(helpers, data.Identifier("c1"))
+			},
+			Cleanup: func(data test.Data, helpers test.Helpers) {
+				helpers.Anyhow("container", "rm", "-f", data.Identifier("c1"))
+				helpers.Anyhow("network", "rm", data.Identifier())
+			},
+			Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
+				// Try to ping the other container in the same network
+				// This should succeed when no ICC is set
+				return helpers.Command("run", "--rm", "--net", data.Identifier(),
+					testutil.CommonImage, "ping", "-c", "1", "-W", "1", data.Identifier("c1"))
+			},
+			Expected: test.Expects(0, nil, nil), // Expect ping to succeed with exit code 0
+		},
+	}
+
+	testCase.Run(t)
+}

docs/command-reference.md

@@ -1193,6 +1193,8 @@ Flags:
 - :whale: `-o, --opt`: Set driver specific options
   - :whale: `--opt=com.docker.network.driver.mtu=<MTU>`: Set the containers network MTU
   - :nerd_face: `--opt=mtu=<MTU>`: Alias of `--opt=com.docker.network.driver.mtu=<MTU>`
+  - :whale: `--opt=com.docker.network.bridge.enable_icc=<true/false>`: Enable or Disable inter-container connectivity
+  - :nerd_face: `--opt=icc=<true/false>`: Alias of `--opt=com.docker.network.bridge.enable_icc`
   - :whale: `--opt=macvlan_mode=(bridge)>`: Set macvlan network mode (default: bridge)
   - :whale: `--opt=ipvlan_mode=(l2|l3)`: Set IPvlan network mode (default: l2)
   - :nerd_face: `--opt=mode=(bridge|l2|l3)`: Alias of `--opt=macvlan_mode=(bridge)` and `--opt=ipvlan_mode=(l2|l3)`

pkg/netutil/cni_plugin_unix.go

@@ -95,13 +95,18 @@ type firewallConfig struct {
 
 	// IngressPolicy is supported since firewall plugin v1.1.0.
 	// "same-bridge" mode replaces the deprecated "isolation" plugin.
+	// "isolated" mode has been added since firewall plugin v1.7.1
 	IngressPolicy string `json:"ingressPolicy,omitempty"`
 }
 
-func newFirewallPlugin() *firewallConfig {
+func newFirewallPlugin(ingressPolicy string) *firewallConfig {
+	if ingressPolicy != "same-bridge" && ingressPolicy != "isolated" {
+		ingressPolicy = "same-bridge" // Default to "same-bridge" if invalid value provided
+	}
+
 	c := &firewallConfig{
 		PluginType:    "firewall",
-		IngressPolicy: "same-bridge",
+		IngressPolicy: ingressPolicy,
 	}
 	if rootlessutil.IsRootless() {
 		// https://github.com/containerd/nerdctl/issues/2818

pkg/netutil/netutil_unix.go

@@ -99,6 +99,7 @@ func (e *CNIEnv) generateCNIPlugins(driver string, name string, ipam map[string]
 	case "bridge":
 		mtu := 0
 		iPMasq := true
+		icc := true
 		for opt, v := range opts {
 			switch opt {
 			case "mtu", "com.docker.network.driver.mtu":
@@ -111,6 +112,11 @@ func (e *CNIEnv) generateCNIPlugins(driver string, name string, ipam map[string]
 				if err != nil {
 					return nil, err
 				}
+			case "icc", "com.docker.network.bridge.enable_icc":
+				icc, err = strconv.ParseBool(v)
+				if err != nil {
+					return nil, err
+				}
 			default:
 				return nil, fmt.Errorf("unsupported %q network option %q", driver, opt)
 			}
@@ -133,14 +139,29 @@ func (e *CNIEnv) generateCNIPlugins(driver string, name string, ipam map[string]
 		if ipv6 {
 			bridge.Capabilities["ips"] = true
 		}
+
+		// Determine the appropriate firewall ingress policy based on icc setting
+		ingressPolicy := "same-bridge" // Default policy
+		firewallPath := filepath.Join(e.Path, "firewall")
+		if !icc {
+			// Check if firewall plugin supports the "isolated" policy (v1.7.1+)
+			ok, err := FirewallPluginGEQVersion(firewallPath, "v1.7.1")
+			if err != nil {
+				log.L.WithError(err).Warnf("Failed to detect whether %q is newer than v1.7.1", firewallPath)
+			} else if ok {
+				ingressPolicy = "isolated"
+			} else {
+				log.L.Warnf("To use 'isolated' ingress policy, CNI plugin \"firewall\" (>= 1.7.1) needs to be installed in CNI_PATH (%q), see https://www.cni.dev/plugins/current/meta/firewall/", e.Path)
+			}
+		}
+
 		if internal {
-			plugins = []CNIPlugin{bridge, newFirewallPlugin(), newTuningPlugin()}
+			plugins = []CNIPlugin{bridge, newFirewallPlugin(ingressPolicy), newTuningPlugin()}
 		} else {
-			plugins = []CNIPlugin{bridge, newPortMapPlugin(), newFirewallPlugin(), newTuningPlugin()}
+			plugins = []CNIPlugin{bridge, newPortMapPlugin(), newFirewallPlugin(ingressPolicy), newTuningPlugin()}
 		}
 		if name != DefaultNetworkName {
-			firewallPath := filepath.Join(e.Path, "firewall")
-			ok, err := firewallPluginGEQ110(firewallPath)
+			ok, err := FirewallPluginGEQVersion(firewallPath, "v1.1.0")
 			if err != nil {
 				log.L.WithError(err).Warnf("Failed to detect whether %q is newer than v1.1.0", firewallPath)
 			}
@@ -291,7 +312,8 @@ func (e *CNIEnv) parseIPAMRanges(subnets []string, gateway, ipRange string, ipv6
 	return ranges, findIPv4, nil
 }
 
-func firewallPluginGEQ110(firewallPath string) (bool, error) {
+// FirewallPluginGEQVersion checks if the firewall plugin is greater than or equal to the specified version
+func FirewallPluginGEQVersion(firewallPath string, versionStr string) (bool, error) {
 	// TODO: guess true by default in 2023
 	guessed := false
 
@@ -320,8 +342,8 @@ func firewallPluginGEQ110(firewallPath string) (bool, error) {
 	if err != nil {
 		return guessed, fmt.Errorf("failed to guess the version of %q: %w", firewallPath, err)
 	}
-	ver110 := semver.MustParse("v1.1.0")
-	return ver.GreaterThan(ver110) || ver.Equal(ver110), nil
+	targetVer := semver.MustParse(versionStr)
+	return ver.GreaterThan(targetVer) || ver.Equal(targetVer), nil
 }
 
 // guessFirewallPluginVersion guess the version of the CNI firewall plugin (not the version of the implemented CNI spec).

pkg/netutil/netutil_windows.go

@@ -18,6 +18,7 @@ package netutil
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net"
 
@@ -95,3 +96,7 @@ func (e *CNIEnv) generateIPAM(driver string, subnets []string, gatewayStr, ipRan
 	}
 	return ipam, nil
 }
+
+func FirewallPluginGEQVersion(firewallPath string, versionStr string) (bool, error) {
+	return false, errors.New("unsupported in windows")
+}

pkg/testutil/nerdtest/requirements.go

@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os/exec"
+	"path/filepath"
 	"strings"
 
 	"github.com/Masterminds/semver/v3"
@@ -32,8 +33,10 @@ import (
 
 	"github.com/containerd/nerdctl/v2/pkg/buildkitutil"
 	"github.com/containerd/nerdctl/v2/pkg/clientutil"
+	ncdefaults "github.com/containerd/nerdctl/v2/pkg/defaults"
 	"github.com/containerd/nerdctl/v2/pkg/infoutil"
 	"github.com/containerd/nerdctl/v2/pkg/inspecttypes/dockercompat"
+	"github.com/containerd/nerdctl/v2/pkg/netutil"
 	"github.com/containerd/nerdctl/v2/pkg/rootlessutil"
 	"github.com/containerd/nerdctl/v2/pkg/snapshotterutil"
 	"github.com/containerd/nerdctl/v2/pkg/testutil"
@@ -447,3 +450,23 @@ func ContainerdVersion(v string) *test.Requirement {
 		},
 	}
 }
+
+// CNIFirewallVersion checks if the CNI firewall plugin version is greater than or equal to the specified version
+func CNIFirewallVersion(requiredVersion string) *test.Requirement {
+	return &test.Requirement{
+		Check: func(data test.Data, helpers test.Helpers) (bool, string) {
+			cniPath := ncdefaults.CNIPath()
+			firewallPath := filepath.Join(cniPath, "firewall")
+			ok, err := netutil.FirewallPluginGEQVersion(firewallPath, requiredVersion)
+			if err != nil {
+				return false, fmt.Sprintf("Failed to check CNI firewall version: %v", err)
+			}
+
+			if !ok {
+				return false, fmt.Sprintf("CNI firewall plugin version is less than required version %s", requiredVersion)
+			}
+
+			return true, fmt.Sprintf("CNI firewall plugin version is greater than or equal to required version %s", requiredVersion)
+		},
+	}
+}