CNI: switch away from isolation plugin to firewall plugin with `i…

…ngressPolicy` The `isolation` plugin is now deprecated, in favor of the `firewall` plugin v1.1.0 with `{"ingressPolicy": "same-bridge"}`. containernetworking/plugins@22dd6c5 WIP: `func firewallPluginGEQ110(firewallPath string) (bool, error)` Signed-off-by: Akihiro Suda <[email protected]>
containerd · Mar 3, 2022 · c140d82 · c140d82
1 parent 20233c2
commit c140d82
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 43 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -22,8 +22,6 @@ ARG CONTAINERD_VERSION=v1.6.1
 ARG RUNC_VERSION=v1.1.0
 ARG CNI_PLUGINS_VERSION=v1.1.0
 
-# Extra deps: CNI isolation
-ARG CNI_ISOLATION_VERSION=v0.0.4
 # Extra deps: Build
 ARG BUILDKIT_VERSION=v0.9.3
 # Extra deps: Lazy-pulling
@@ -129,13 +127,6 @@ RUN fname="cni-plugins-${TARGETOS:-linux}-${TARGETARCH:-amd64}-${CNI_PLUGINS_VER
   tar xzf "${fname}" -C /out/libexec/cni && \
   rm -f "${fname}" && \
   echo "- CNI plugins: ${CNI_PLUGINS_VERSION}" >> /out/share/doc/nerdctl-full/README.md
-ARG CNI_ISOLATION_VERSION
-RUN fname="cni-isolation-${TARGETARCH:-amd64}.tgz" && \
-  curl -o "${fname}" -fSL "https://github.com/AkihiroSuda/cni-isolation/releases/download/${CNI_ISOLATION_VERSION}/${fname}" && \
-  grep "${fname}" "/SHA256SUMS.d/cni-isolation-${CNI_ISOLATION_VERSION}" | sha256sum -c && \
-  tar xzf "${fname}" -C /out/libexec/cni && \
-  rm -f "${fname}" && \
-  echo "- CNI isolation plugin: ${CNI_ISOLATION_VERSION}" >> /out/share/doc/nerdctl-full/README.md
 ARG BUILDKIT_VERSION
 RUN fname="buildkit-${BUILDKIT_VERSION}.${TARGETOS:-linux}-${TARGETARCH:-amd64}.tar.gz" && \
   curl -o "${fname}" -fSL "https://github.com/moby/buildkit/releases/download/${BUILDKIT_VERSION}/${fname}" && \

diff --git a/Dockerfile.d/SHA256SUMS.d/cni-isolation-v0.0.4 b/Dockerfile.d/SHA256SUMS.d/cni-isolation-v0.0.4
diff --git a/README.md b/README.md
@@ -95,11 +95,11 @@ See [`./docs/rootless.md`](./docs/rootless.md).
 ## Install
 Binaries are available here: https://github.com/containerd/nerdctl/releases
 
-In addition to containerd, the following components should be installed (optional):
+In addition to containerd, the following components should be installed:
 - [CNI plugins](https://github.com/containernetworking/plugins): for using `nerdctl run`.
-- [CNI isolation plugin](https://github.com/AkihiroSuda/cni-isolation): for isolating bridge networks (`nerdctl network create`)
-- [BuildKit](https://github.com/moby/buildkit): for using `nerdctl build`. BuildKit daemon (`buildkitd`) needs to be running.
-- [RootlessKit](https://github.com/rootless-containers/rootlesskit) and [slirp4netns](https://github.com/rootless-containers/slirp4netns): for [Rootless mode](./docs/rootless.md)
+   - v1.1.0 or later is highly recommended. Older versions require extra [CNI isolation plugin](https://github.com/AkihiroSuda/cni-isolation) for isolating bridge networks (`nerdctl network create`).
+- [BuildKit](https://github.com/moby/buildkit) (OPTIONAL): for using `nerdctl build`. BuildKit daemon (`buildkitd`) needs to be running.
+- [RootlessKit](https://github.com/rootless-containers/rootlesskit) and [slirp4netns](https://github.com/rootless-containers/slirp4netns) (OPTIONAL): for [Rootless mode](./docs/rootless.md)
    - RootlessKit needs to be v0.10.0 or later. v0.14.1 or later is recommended.
    - slirp4netns needs to be v0.4.0 or later. v1.1.7 or later is recommended.
 

diff --git a/docs/cni.md b/docs/cni.md
@@ -46,19 +46,26 @@ Configuration of the default network `bridge` of Linux:
       }
     },
     {
-      "type": "firewall"
+      "type": "firewall",
+      "ingressPolicy": "same-bridge"
     },
     {
       "type": "tuning"
-    },
-    {
-      "type": "isolation"
     }
   ]
 }
 ```
 
-When CNI plugin `isolation` be installed, will inject isolation configuration `{"type":"isolation"}` automatically.
+## Bridge isolation
+
+nerdctl >= 0.18 sets the `ingressPolicy` to `same-bridge` when `firewall` plugin >= 1.1.0 is installed.
+This `ingressPolicy` replaces the CNI `isolation` plugin used in nerdctl <= 0.17.
+
+When the `isolation` plugin is found, nerdctl uses the `isolation` plugin instead of `ingressPolicy`.
+The `isolation` plugin has been deprecated, and a future version of `nerdctl` will solely support `ingressPolicy`.
+
+When neither of `firewall` plugin >= 1.1.0 or `isolation` plugin is found, nerdctl does not enable the bridge isolation.
+This means a container in `--net=foo` can connect to a container in `--net=bar`.
 
 ## Custom networks
 
@@ -102,7 +109,3 @@ this network to create a container:
     inet6 fe80::5c5b:3fff:fe0c:3656/64 scope link tentative
        valid_lft forever preferred_lft forever
 ```
-
-## Bridge Isolation Plugin
-
-If you have the [CNI isolation plugin](https://github.com/AkihiroSuda/cni-isolation) installed, the `isolation` plugin will be used automatically.
diff --git a/pkg/netutil/cni_plugin_unix.go b/pkg/netutil/cni_plugin_unix.go
@@ -87,11 +87,16 @@ func (*portMapConfig) GetPluginType() string {
 type firewallConfig struct {
 	PluginType string `json:"type"`
 	Backend    string `json:"backend,omitempty"`
+
+	// IngressPolicy is supported since firewall plugin v1.1.0.
+	// "same-bridge" mode replaces the deprecated "isolation" plugin.
+	IngressPolicy string `json:"ingressPolicy,omitempty"`
 }
 
 func newFirewallPlugin() *firewallConfig {
 	return &firewallConfig{
-		PluginType: "firewall",
+		PluginType:    "firewall",
+		IngressPolicy: "same-bridge",
 	}
 }
 

diff --git a/pkg/netutil/netutil.go b/pkg/netutil/netutil.go
@@ -30,8 +30,6 @@ import (
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/nerdctl/pkg/strutil"
 	"github.com/containernetworking/cni/libcni"
-
-	"github.com/sirupsen/logrus"
 )
 
 type NetworkConfigList struct {
@@ -74,19 +72,8 @@ func GenerateConfigList(e *CNIEnv, labels []string, id int, name string, plugins
 			return nil, fmt.Errorf("needs CNI plugin %q to be installed in CNI_PATH (%q), see https://github.com/containernetworking/plugins/releases: %w", f, e.Path, err)
 		}
 	}
-	var extraPlugin CNIPlugin
-	if _, err := exec.LookPath(filepath.Join(e.Path, "isolation")); err == nil {
-		logrus.Debug("found CNI isolation plugin")
-		extraPlugin = newIsolationPlugin()
-	} else if name != DefaultNetworkName {
-		// the warning is suppressed for DefaultNetworkName
-		logrus.Warnf("To isolate bridge networks, CNI plugin \"isolation\" needs to be installed in CNI_PATH (%q), see https://github.com/AkihiroSuda/cni-isolation",
-			e.Path)
-	}
+	plugins = fixUpIsolation(e, name, plugins)
 
-	if extraPlugin != nil {
-		plugins = append(plugins, extraPlugin)
-	}
 	labelsMap := strutil.ConvertKVStringsToMap(labels)
 
 	conf := &cniNetworkConfig{

diff --git a/pkg/netutil/netutil_unix.go b/pkg/netutil/netutil_unix.go
@@ -21,6 +21,7 @@ package netutil
 
 import (
 	"fmt"
+	"os/exec"
 	"path/filepath"
 
 	"github.com/containerd/nerdctl/pkg/defaults"
@@ -137,3 +138,41 @@ func GenerateIPAM(driver string, subnetStr, gatewayStr, ipRangeStr string, opts
 	}
 	return ipam, nil
 }
+
+func fixUpIsolation(e *CNIEnv, name string, plugins []CNIPlugin) []CNIPlugin {
+	isolationPath := filepath.Join(e.Path, "isolation")
+	if _, err := exec.LookPath(isolationPath); err == nil {
+		// the warning is suppressed for DefaultNetworkName (because multi-bridge networking is not involved)
+		if name != DefaultNetworkName {
+			logrus.Warnf(`network %q: Using the deprecated CNI "isolation" plugin instead of CNI "firewall" plugin (>= 1.1.0) ingressPolicy.
+To dismiss this warning, uninstall %q and install CNI "firewall" plugin (>= 1.1.0) from https://github.com/containernetworking/plugins`,
+				name, isolationPath)
+		}
+		plugins = append(plugins, newIsolationPlugin())
+		for _, f := range plugins {
+			if x, ok := f.(*firewallConfig); ok {
+				if name != DefaultNetworkName {
+					logrus.Warnf("network %q: Unsetting firewall ingressPolicy %q (because using the deprecated \"isolation\" plugin)", name, x.IngressPolicy)
+				}
+				x.IngressPolicy = ""
+			}
+		}
+	} else if name != DefaultNetworkName {
+		firewallPath := filepath.Join(e.Path, "firewall")
+		ok, err := firewallPluginGEQ110(firewallPath)
+		if err != nil {
+			logrus.WithError(err).Warnf("Failed to detect whether %q is newer than v1.1.0", firewallPath)
+		}
+		if !ok {
+			logrus.Warnf("To isolate bridge networks, CNI plugin \"firewall\" (>= 1.1.0), or CNI plugin \"isolation\" (deprecated) needs to be installed in CNI_PATH (%q), see https://github.com/containernetworking/plugins",
+				e.Path)
+		}
+	}
+
+	return plugins
+}
+
+func firewallPluginGEQ110(firewallPath string) (bool, error) {
+	// FIXME
+	return true, nil
+}
diff --git a/pkg/netutil/netutil_windows.go b/pkg/netutil/netutil_windows.go
@@ -62,3 +62,7 @@ func GenerateIPAM(driver string, subnetStr, gatewayStr, ipRangeStr string, opts
 	}
 	return ipam, nil
 }
+
+func fixUpIsolation(e *CNIEnv, name string, plugins []CNIPlugin) []CNIPlugin {
+	return plugins
+}