From a51896303571bef2f39c0368dcf2d49e776a88df Mon Sep 17 00:00:00 2001 From: Swagat Bora Date: Tue, 10 Jun 2025 23:18:59 +0000 Subject: [PATCH 1/2] feat: add support for com.docker.network.bridge.enable_icc network option Signed-off-by: Swagat Bora --- .../network/network_create_linux_test.go | 98 +++++++++++++++++++ docs/command-reference.md | 2 + pkg/netutil/cni_plugin_unix.go | 9 +- pkg/netutil/netutil_unix.go | 34 +++++-- pkg/netutil/netutil_windows.go | 5 + pkg/testutil/nerdtest/requirements.go | 23 +++++ 6 files changed, 163 insertions(+), 8 deletions(-) diff --git a/cmd/nerdctl/network/network_create_linux_test.go b/cmd/nerdctl/network/network_create_linux_test.go index 6d42809f2ff..47c5e3ffda8 100644 --- a/cmd/nerdctl/network/network_create_linux_test.go +++ b/cmd/nerdctl/network/network_create_linux_test.go @@ -25,6 +25,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" @@ -111,3 +112,100 @@ func TestNetworkCreate(t *testing.T) { testCase.Run(t) } + +func TestNetworkCreateICC(t *testing.T) { + testCase := nerdtest.Setup() + + testCase.Require = require.All( + require.Linux, + nerdtest.Rootful, + ) + + 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 + iptablesSave := "iptables-save | grep CNI-ISOLATION || true" + helpers.Custom("sh", "-ec", iptablesSave).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) +} diff --git a/docs/command-reference.md b/docs/command-reference.md index d2e82e8a0ea..9ea6bcdae69 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -1075,6 +1075,8 @@ Flags: - :whale: `-o, --opt`: Set driver specific options - :whale: `--opt=com.docker.network.driver.mtu=`: Set the containers network MTU - :nerd_face: `--opt=mtu=`: Alias of `--opt=com.docker.network.driver.mtu=` + - :whale: `--opt=com.docker.network.bridge.enable_icc=`: Enable or Disable inter-container connectivity + - :nerd_face: `--opt=icc=`: 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)` diff --git a/pkg/netutil/cni_plugin_unix.go b/pkg/netutil/cni_plugin_unix.go index 8d863d3be93..2851c7b5a3a 100644 --- a/pkg/netutil/cni_plugin_unix.go +++ b/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 diff --git a/pkg/netutil/netutil_unix.go b/pkg/netutil/netutil_unix.go index ffb1d8503a8..24d84a6d669 100644 --- a/pkg/netutil/netutil_unix.go +++ b/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) } @@ -129,10 +135,25 @@ func (e *CNIEnv) generateCNIPlugins(driver string, name string, ipam map[string] if ipv6 { bridge.Capabilities["ips"] = true } - plugins = []CNIPlugin{bridge, newPortMapPlugin(), newFirewallPlugin(), newTuningPlugin()} + + // 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) + } + } + + 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) } @@ -281,7 +302,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 @@ -310,8 +332,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). diff --git a/pkg/netutil/netutil_windows.go b/pkg/netutil/netutil_windows.go index 8e0e67a01ed..496f17512cc 100644 --- a/pkg/netutil/netutil_windows.go +++ b/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") +} diff --git a/pkg/testutil/nerdtest/requirements.go b/pkg/testutil/nerdtest/requirements.go index 46bbeee675d..40e3e08e955 100644 --- a/pkg/testutil/nerdtest/requirements.go +++ b/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) + }, + } +} From 9ce862521243d3e1ed0670a84f08d0a7644dca2c Mon Sep 17 00:00:00 2001 From: Swagat Bora Date: Thu, 24 Jul 2025 22:50:53 +0000 Subject: [PATCH 2/2] Add debug logs in test Signed-off-by: Swagat Bora --- .../network/network_create_linux_test.go | 56 ++++++++++++++++++- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/cmd/nerdctl/network/network_create_linux_test.go b/cmd/nerdctl/network/network_create_linux_test.go index 47c5e3ffda8..35cace94b8f 100644 --- a/cmd/nerdctl/network/network_create_linux_test.go +++ b/cmd/nerdctl/network/network_create_linux_test.go @@ -19,6 +19,7 @@ package network import ( "fmt" "net" + "path/filepath" "strings" "testing" @@ -29,6 +30,7 @@ import ( "github.com/containerd/nerdctl/mod/tigron/test" "github.com/containerd/nerdctl/mod/tigron/tig" + "github.com/containerd/nerdctl/v2/pkg/defaults" "github.com/containerd/nerdctl/v2/pkg/testutil" "github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest" ) @@ -122,6 +124,47 @@ func TestNetworkCreateICC(t *testing.T) { ) testCase.SubTests = []*test.Case{ + { + Description: "debug ICC feature", + 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: Show firewall plugin version + firewallCniPath := filepath.Join(defaults.CNIPath(), "firewall") + helpers.Custom("sh", "-ec", fmt.Sprintf("%s --version || echo 'firewall plugin not found'", firewallCniPath)).Run(&test.Expected{}) + helpers.Ensure("network", "ls") + helpers.Ensure("network", "inspect", data.Identifier()) + helpers.Custom("sh", "-ec", "ls /etc/cni/net.d").Run(&test.Expected{}) + helpers.Custom("iptables-save").Run(&test.Expected{}) + containerIP := helpers.Capture("container", "inspect", data.Identifier("c1"), "--format", "{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}") + helpers.Custom("echo", fmt.Sprintf("Container IP: %s", containerIP)).Run(&test.Expected{}) + helpers.Custom("sh", "-ec", "ip link show | grep br- || true").Run(&test.Expected{}) + helpers.Custom("sh", "-ec", "brctl show || true").Run(&test.Expected{}) + helpers.Custom("sleep", "3").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=false", Require: nerdtest.CNIFirewallVersion("1.7.1"), @@ -143,9 +186,16 @@ func TestNetworkCreateICC(t *testing.T) { helpers.Anyhow("network", "rm", data.Identifier()) }, Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { - // DEBUG - iptablesSave := "iptables-save | grep CNI-ISOLATION || true" - helpers.Custom("sh", "-ec", iptablesSave).Run(&test.Expected{}) + firewallCniPath := filepath.Join(defaults.CNIPath(), "firewall") + helpers.Custom("sh", "-ec", fmt.Sprintf("%s --version || echo 'firewall plugin not found'", firewallCniPath)).Run(&test.Expected{}) + helpers.Ensure("network", "inspect", data.Identifier()) + helpers.Custom("sh", "-ec", fmt.Sprintf("find /etc/cni/net.d/ -name '*%s*' -exec cat {} \\; || true", data.Identifier())).Run(&test.Expected{}) + helpers.Custom("iptables-save").Run(&test.Expected{}) + containerIP := helpers.Capture("container", "inspect", data.Identifier("c1"), "--format", "{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}") + helpers.Custom("echo", fmt.Sprintf("Container IP: %s", containerIP)).Run(&test.Expected{}) + helpers.Custom("sh", "-ec", "ip link show | grep br- || true").Run(&test.Expected{}) + helpers.Custom("sh", "-ec", "brctl show || true").Run(&test.Expected{}) + helpers.Custom("sleep", "3").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(),