libnetwork/cni: add support for arbitrary ipam plugins

When we parse CNI config files to convert them into a native Network struct we should not error unless there is something broken. The goal is to allow all cni configs to still function but podman inspect will not be able to show all informations such as subnets about this network. Because this is a valid use case we only log this at info level. Fixes containers/podman#12990 Fixes containers/podman#13124 Signed-off-by: Paul Holzinger <[email protected]>
Luap99 · Mar 17, 2022 · 2d5f753 · 2d5f753
1 parent 324a8d2
commit 2d5f753
Show file tree

Hide file tree

Showing 5 changed files with 158 additions and 6 deletions.
diff --git a/libnetwork/cni/cni_conversion.go b/libnetwork/cni/cni_conversion.go
@@ -133,7 +133,13 @@ func convertIPAMConfToNetwork(network *types.Network, ipam *ipamConfig, confPath
 	}
 
 	if ipam.PluginType != types.HostLocalIPAMDriver {
-		return errors.Errorf("unsupported ipam plugin %s in %s", ipam.PluginType, confPath)
+		// This is not an error. While we only support certain ipam drivers, we
+		// cannot make it fail for unsupported ones. CNI is still able to use them,
+		// just our translation logic cannot convert this into a Network.
+		// For the same reason this is not warning, it would just be annoying for
+		// everyone using a unknown ipam driver.
+		logrus.Infof("unsupported ipam plugin %q in %s", ipam.PluginType, confPath)
+		return nil
 	}
 
 	network.IPAMOptions[types.Driver] = types.HostLocalIPAMDriver

diff --git a/libnetwork/cni/config_test.go b/libnetwork/cni/config_test.go
@@ -34,6 +34,7 @@ var _ = Describe("Config", func() {
 		}
 		logBuffer = bytes.Buffer{}
 		logrus.SetOutput(&logBuffer)
+		logrus.SetLevel(logrus.InfoLevel)
 	})
 
 	JustBeforeEach(func() {
@@ -45,6 +46,7 @@ var _ = Describe("Config", func() {
 	})
 
 	AfterEach(func() {
+		logrus.SetLevel(logrus.InfoLevel)
 		os.RemoveAll(cniConfDir)
 	})
 
@@ -1071,6 +1073,8 @@ var _ = Describe("Config", func() {
 
 	Context("network load valid existing ones", func() {
 
+		numberOfConfiFiles := 0
+
 		BeforeEach(func() {
 			dir := "testfiles/valid"
 			files, err := ioutil.ReadDir(dir)
@@ -1088,26 +1092,41 @@ var _ = Describe("Config", func() {
 					Fail("Failed to copy test files")
 				}
 			}
+			numberOfConfiFiles = len(files)
 		})
 
 		It("load networks from disk", func() {
+			logrus.SetLevel(logrus.WarnLevel)
 			nets, err := libpodNet.NetworkList()
 			Expect(err).To(BeNil())
-			Expect(nets).To(HaveLen(9))
+			Expect(nets).To(HaveLen(numberOfConfiFiles))
 			// test the we do not show logrus warnings/errors
 			logString := logBuffer.String()
 			Expect(logString).To(BeEmpty())
 		})
 
+		It("load networks from disk with log level debug", func() {
+			logrus.SetLevel(logrus.DebugLevel)
+			nets, err := libpodNet.NetworkList()
+			Expect(err).To(BeNil())
+			Expect(nets).To(HaveLen(numberOfConfiFiles))
+			// check for the unsupported ipam plugin message
+			logString := logBuffer.String()
+			Expect(logString).ToNot(BeEmpty())
+			Expect(logString).To(ContainSubstring("unsupported ipam plugin \\\"\\\" in %s", cniConfDir+"/ipam-none.conflist"))
+			Expect(logString).To(ContainSubstring("unsupported ipam plugin \\\"\\\" in %s", cniConfDir+"/ipam-empty.conflist"))
+			Expect(logString).To(ContainSubstring("unsupported ipam plugin \\\"static\\\" in %s", cniConfDir+"/ipam-static.conflist"))
+		})
+
 		It("change network struct fields should not affect network struct in the backend", func() {
 			nets, err := libpodNet.NetworkList()
 			Expect(err).To(BeNil())
-			Expect(nets).To(HaveLen(9))
+			Expect(nets).To(HaveLen(numberOfConfiFiles))
 
 			nets[0].Name = "myname"
 			nets, err = libpodNet.NetworkList()
 			Expect(err).To(BeNil())
-			Expect(nets).To(HaveLen(9))
+			Expect(nets).To(HaveLen(numberOfConfiFiles))
 			Expect(nets).ToNot(ContainElement(HaveNetworkName("myname")))
 
 			network, err := libpodNet.NetworkInspect("bridge")
@@ -1230,6 +1249,15 @@ var _ = Describe("Config", func() {
 			))
 		})
 
+		It("ipam static network", func() {
+			network, err := libpodNet.NetworkInspect("ipam-static")
+			Expect(err).To(BeNil())
+			Expect(network.Name).To(Equal("ipam-static"))
+			Expect(network.ID).To(HaveLen(64))
+			Expect(network.Driver).To(Equal("bridge"))
+			Expect(network.Subnets).To(HaveLen(0))
+		})
+
 		It("network list with filters (name)", func() {
 			filters := map[string][]string{
 				"name": {"internal", "bridge"},
@@ -1304,10 +1332,12 @@ var _ = Describe("Config", func() {
 
 			networks, err := libpodNet.NetworkList(filterFuncs...)
 			Expect(err).To(BeNil())
-			Expect(networks).To(HaveLen(9))
+			Expect(networks).To(HaveLen(numberOfConfiFiles))
 			Expect(networks).To(ConsistOf(HaveNetworkName("internal"), HaveNetworkName("bridge"),
 				HaveNetworkName("mtu"), HaveNetworkName("vlan"), HaveNetworkName("podman"),
-				HaveNetworkName("label"), HaveNetworkName("macvlan"), HaveNetworkName("macvlan_mtu"), HaveNetworkName("dualstack")))
+				HaveNetworkName("label"), HaveNetworkName("macvlan"), HaveNetworkName("macvlan_mtu"),
+				HaveNetworkName("dualstack"), HaveNetworkName("ipam-none"), HaveNetworkName("ipam-empty"),
+				HaveNetworkName("ipam-static")))
 		})
 
 		It("network list with filters (label)", func() {

diff --git a/libnetwork/cni/testfiles/valid/ipam-empty.conflist b/libnetwork/cni/testfiles/valid/ipam-empty.conflist
@@ -0,0 +1,34 @@
+{
+   "cniVersion": "0.4.0",
+   "name": "ipam-empty",
+   "plugins": [
+      {
+         "type": "bridge",
+         "bridge": "cni-podman124",
+         "isGateway": true,
+         "ipMasq": true,
+         "hairpinMode": true,
+         "ipam": {}
+      },
+      {
+         "type": "portmap",
+         "capabilities": {
+            "portMappings": true
+         }
+      },
+      {
+         "type": "firewall",
+         "backend": ""
+      },
+      {
+         "type": "tuning"
+      },
+      {
+         "type": "dnsname",
+         "domainName": "dns.podman",
+         "capabilities": {
+            "aliases": true
+         }
+      }
+   ]
+}
diff --git a/libnetwork/cni/testfiles/valid/ipam-none.conflist b/libnetwork/cni/testfiles/valid/ipam-none.conflist
@@ -0,0 +1,33 @@
+{
+   "cniVersion": "0.4.0",
+   "name": "ipam-none",
+   "plugins": [
+      {
+         "type": "bridge",
+         "bridge": "cni-podman123",
+         "isGateway": true,
+         "ipMasq": true,
+         "hairpinMode": true
+      },
+      {
+         "type": "portmap",
+         "capabilities": {
+            "portMappings": true
+         }
+      },
+      {
+         "type": "firewall",
+         "backend": ""
+      },
+      {
+         "type": "tuning"
+      },
+      {
+         "type": "dnsname",
+         "domainName": "dns.podman",
+         "capabilities": {
+            "aliases": true
+         }
+      }
+   ]
+}
diff --git a/libnetwork/cni/testfiles/valid/ipam-static.conflist b/libnetwork/cni/testfiles/valid/ipam-static.conflist
@@ -0,0 +1,49 @@
+{
+   "cniVersion": "0.4.0",
+   "name": "ipam-static",
+   "plugins": [
+      {
+         "type": "bridge",
+         "bridge": "cni-podman125",
+         "isGateway": true,
+         "ipMasq": true,
+         "hairpinMode": true,
+         "ipam": {
+            "type": "static",
+            "routes": [
+               {
+                  "dst": "0.0.0.0/0"
+               }
+            ],
+            "addresses": [
+               [
+                  {
+                     "subnet": "10.89.0.89/16",
+                     "gateway": "10.89.0.1"
+                  }
+               ]
+            ]
+         }
+      },
+      {
+         "type": "portmap",
+         "capabilities": {
+            "portMappings": true
+         }
+      },
+      {
+         "type": "firewall",
+         "backend": ""
+      },
+      {
+         "type": "tuning"
+      },
+      {
+         "type": "dnsname",
+         "domainName": "dns.podman",
+         "capabilities": {
+            "aliases": true
+         }
+      }
+   ]
+}