diff --git a/cmd/nerdctl/network/network_create_linux_test.go b/cmd/nerdctl/network/network_create_linux_test.go index 843ec56c44b..f9c35c845aa 100644 --- a/cmd/nerdctl/network/network_create_linux_test.go +++ b/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) +} diff --git a/docs/command-reference.md b/docs/command-reference.md index c5d6863de1a..a3a8979f8de 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -1193,6 +1193,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 f71dc100742..046c173d122 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) } @@ -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). diff --git a/pkg/netutil/netutil_windows.go b/pkg/netutil/netutil_windows.go index bd03e6ec4aa..484b03c9b77 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) + }, + } +}