libcni: Support subdirectory-based plugin loading (#928)

Signed-off-by: Benjamin Leggett <[email protected]>

libcni/api.go

@@ -77,13 +77,13 @@ type PluginConfig struct {
 }
 
 type NetworkConfigList struct {
-	Name                       string
-	CNIVersion                 string
-	DisableCheck               bool
-	DisableGC    bool
+	Name                   string
+	CNIVersion             string
+	DisableCheck           bool
+	DisableGC              bool
 	LoadOnlyInlinedPlugins bool
-	Plugins                    []*PluginConfig
-	Bytes                      []byte
+	Plugins                []*PluginConfig
+	Bytes                  []byte
 }
 
 type NetworkAttachment struct {

libcni/conf.go

@@ -64,6 +64,32 @@ func NetworkPluginConfFromBytes(pluginConfBytes []byte) (*PluginConfig, error) {
 	return conf, nil
 }
 
+// Given a path to a directory containing a network configuration, and the name of a network,
+// loads all plugin definitions found at path `networkConfPath/networkName/*.conf`
+func NetworkPluginConfsFromFiles(networkConfPath, networkName string) ([]*PluginConfig, error) {
+	var pConfs []*PluginConfig
+
+	pluginConfPath := filepath.Join(networkConfPath, networkName)
+
+	pluginConfFiles, err := ConfFiles(pluginConfPath, []string{".conf"})
+	if err != nil {
+		return nil, fmt.Errorf("failed to read plugin config files in %s: %w", pluginConfPath, err)
+	}
+
+	for _, pluginConfFile := range pluginConfFiles {
+		pluginConfBytes, err := os.ReadFile(pluginConfFile)
+		if err != nil {
+			return nil, fmt.Errorf("error reading %s: %w", pluginConfFile, err)
+		}
+		pluginConf, err := NetworkPluginConfFromBytes(pluginConfBytes)
+		if err != nil {
+			return nil, err
+		}
+		pConfs = append(pConfs, pluginConf)
+	}
+	return pConfs, nil
+}
+
 func NetworkConfFromBytes(confBytes []byte) (*NetworkConfigList, error) {
 	rawList := make(map[string]interface{})
 	if err := json.Unmarshal(confBytes, &rawList); err != nil {
@@ -162,19 +188,36 @@ func NetworkConfFromBytes(confBytes []byte) (*NetworkConfigList, error) {
 		return nil, err
 	}
 
+	loadOnlyInlinedPlugins, err := readBool("loadOnlyInlinedPlugins")
+	if err != nil {
+		return nil, err
+	}
+
 	list := &NetworkConfigList{
-		Name:         name,
-		DisableCheck: disableCheck,
-		DisableGC:    disableGC,
-		CNIVersion:   cniVersion,
-		Bytes:        confBytes,
+		Name:                   name,
+		DisableCheck:           disableCheck,
+		DisableGC:              disableGC,
+		LoadOnlyInlinedPlugins: loadOnlyInlinedPlugins,
+		CNIVersion:             cniVersion,
+		Bytes:                  confBytes,
 	}
 
 	var plugins []interface{}
 	plug, ok := rawList["plugins"]
-	if !ok {
-		return nil, fmt.Errorf("error parsing configuration list: no 'plugins' key")
+	// We can have a `plugins` list key in the main conf,
+	// We can also have `loadOnlyInlinedPlugins == true`
+	//
+	// If `plugins` is there, then `loadOnlyInlinedPlugins` can be true
+	//
+	// If plugins is NOT there, then `loadOnlyInlinedPlugins` cannot be true
+	//
+	// We have to have at least some plugins.
+	if !ok && loadOnlyInlinedPlugins {
+		return nil, fmt.Errorf("error parsing configuration list: `loadOnlyInlinedPlugins` is true, and no 'plugins' key")
+	} else if !ok && !loadOnlyInlinedPlugins {
+		return list, nil
 	}
+
 	plugins, ok = plug.([]interface{})
 	if !ok {
 		return nil, fmt.Errorf("error parsing configuration list: invalid 'plugins' type %T", plug)
@@ -194,7 +237,6 @@ func NetworkConfFromBytes(confBytes []byte) (*NetworkConfigList, error) {
 		}
 		list.Plugins = append(list.Plugins, netConf)
 	}
-
 	return list, nil
 }
 
@@ -203,7 +245,26 @@ func NetworkConfFromFile(filename string) (*NetworkConfigList, error) {
 	if err != nil {
 		return nil, fmt.Errorf("error reading %s: %w", filename, err)
 	}
-	return ConfListFromBytes(bytes)
+
+	conf, err := NetworkConfFromBytes(bytes)
+	if err != nil {
+		return nil, err
+	}
+
+	if !conf.LoadOnlyInlinedPlugins {
+		plugins, err := NetworkPluginConfsFromFiles(filepath.Dir(filename), conf.Name)
+		if err != nil {
+			return nil, err
+		}
+		conf.Plugins = append(conf.Plugins, plugins...)
+	}
+
+	if len(conf.Plugins) == 0 {
+		// Having 0 plugins for a given network is not necessarily a problem,
+		// but return as error for caller to decide, since they tried to load
+		return nil, fmt.Errorf("no plugin configs found")
+	}
+	return conf, nil
 }
 
 // Deprecated: This file format is no longer supported, use NetworkConfXXX and NetworkPluginXXX functions
@@ -238,6 +299,8 @@ func ConfFiles(dir string, extensions []string) ([]string, error) {
 	switch {
 	case err == nil: // break
 	case os.IsNotExist(err):
+		// If folder not there, return no error - only return an
+		// error if we cannot read contents or there are no contents.
 		return nil, nil
 	default:
 		return nil, err

libcni/conf_test.go

@@ -389,6 +389,215 @@ var _ = Describe("Loading configuration from disk", func() {
 				Expect(err).To(MatchError(fmt.Sprintf("error parsing configuration list: invalid disableCheck value \"%s\"", badValue)))
 			})
 		})
+
+		Context("for loadOnlyInlinedPlugins", func() {
+			It("the value will be parsed", func() {
+				configList = []byte(`{
+				  "name": "some-network",
+				  "cniVersion": "0.4.0",
+				  "loadOnlyInlinedPlugins": true,
+				  "plugins": [
+				    {
+				      "type": "host-local",
+				      "subnet": "10.0.0.1/24"
+				    }
+				  ]
+				}`)
+				Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())
+
+				dirPluginConf := []byte(`{
+				      "type": "bro-check-out-my-plugin",
+				      "subnet": "10.0.0.1/24"
+				}`)
+
+				subDir := filepath.Join(configDir, "some-network")
+				Expect(os.MkdirAll(subDir, 0o700)).To(Succeed())
+				Expect(os.WriteFile(filepath.Join(subDir, "funky-second-plugin.conf"), dirPluginConf, 0o600)).To(Succeed())
+
+				netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
+				Expect(err).NotTo(HaveOccurred())
+				Expect(netConfigList.LoadOnlyInlinedPlugins).To(BeTrue())
+			})
+
+			It("the value will be false if not in config", func() {
+				configList = []byte(`{
+				  "name": "some-network",
+				  "cniVersion": "0.4.0",
+				  "plugins": [
+				    {
+				      "type": "host-local",
+				      "subnet": "10.0.0.1/24"
+				    }
+				  ]
+				}`)
+				Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())
+
+				netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
+				Expect(err).NotTo(HaveOccurred())
+				Expect(netConfigList.LoadOnlyInlinedPlugins).To(BeFalse())
+			})
+
+			It("will return an error on an unrecognized value", func() {
+				const badValue string = "sphagnum"
+				configList = []byte(fmt.Sprintf(`{
+				  "name": "some-network",
+				  "cniVersion": "0.4.0",
+				  "loadOnlyInlinedPlugins": "%s",
+				  "plugins": [
+				    {
+				      "type": "host-local",
+				      "subnet": "10.0.0.1/24"
+				    }
+				  ]
+				}`, badValue))
+				Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())
+
+				_, err := libcni.LoadNetworkConf(configDir, "some-network")
+				Expect(err).To(MatchError("error parsing configuration list: invalid loadOnlyInlinedPlugins type string"))
+			})
+
+			It("will return an error if `plugins` is missing and `loadOnlyInlinedPlugins` is `true`", func() {
+				configList = []byte(`{
+				  "name": "some-network",
+				  "cniVersion": "0.4.0",
+				  "loadOnlyInlinedPlugins": true
+				}`)
+				Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())
+
+				_, err := libcni.LoadNetworkConf(configDir, "some-network")
+				Expect(err).To(MatchError("error parsing configuration list: `loadOnlyInlinedPlugins` is true, and no 'plugins' key"))
+			})
+
+			It("will return no error if `plugins` is missing and `loadOnlyInlinedPlugins` is false", func() {
+				configList = []byte(`{
+				  "name": "some-network",
+				  "cniVersion": "0.4.0",
+				  "loadOnlyInlinedPlugins": false
+				}`)
+				Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())
+
+				dirPluginConf := []byte(`{
+				      "type": "bro-check-out-my-plugin",
+				      "subnet": "10.0.0.1/24"
+				}`)
+
+				subDir := filepath.Join(configDir, "some-network")
+				Expect(os.MkdirAll(subDir, 0o700)).To(Succeed())
+				Expect(os.WriteFile(filepath.Join(subDir, "funky-second-plugin.conf"), dirPluginConf, 0o600)).To(Succeed())
+
+				netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
+				Expect(err).NotTo(HaveOccurred())
+				Expect(netConfigList.LoadOnlyInlinedPlugins).To(BeFalse())
+				Expect(netConfigList.Plugins).To(HaveLen(1))
+			})
+
+			It("will return error if `loadOnlyInlinedPlugins` is implicitly false + no conf plugin is defined, but no plugins subfolder with network name exists", func() {
+				configList = []byte(`{
+				  "name": "some-network",
+				  "cniVersion": "0.4.0"
+				}`)
+				Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())
+
+				_, err := libcni.LoadNetworkConf(configDir, "some-network")
+				Expect(err).To(MatchError("no plugin configs found"))
+			})
+
+			It("will return NO error if `loadOnlyInlinedPlugins` is implicitly false + at least 1 conf plugin is defined, but no plugins subfolder with network name exists", func() {
+				configList = []byte(`{
+				  "name": "some-network",
+				  "cniVersion": "0.4.0",
+				  "plugins": [
+				    {
+				      "type": "host-local",
+				      "subnet": "10.0.0.1/24"
+				    }
+				  ]
+				}`)
+				Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())
+
+				_, err := libcni.LoadNetworkConf(configDir, "some-network")
+				Expect(err).NotTo(HaveOccurred())
+			})
+
+			It("will return NO error if `loadOnlyInlinedPlugins` is implicitly false + at least 1 conf plugin is defined and network name subfolder exists, but is empty/unreadable", func() {
+				configList = []byte(`{
+				  "name": "some-network",
+				  "cniVersion": "0.4.0",
+				  "plugins": [
+				    {
+				      "type": "host-local",
+				      "subnet": "10.0.0.1/24"
+				    }
+				  ]
+				}`)
+				Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())
+
+				subDir := filepath.Join(configDir, "some-network")
+				Expect(os.MkdirAll(subDir, 0o700)).To(Succeed())
+
+				_, err := libcni.LoadNetworkConf(configDir, "some-network")
+				Expect(err).NotTo(HaveOccurred())
+			})
+
+			It("will merge loaded and inlined plugin lists if both `plugins` is set and `loadOnlyInlinedPlugins` is false", func() {
+				configList = []byte(`{
+				  "name": "some-network",
+				  "cniVersion": "0.4.0",
+				  "plugins": [
+				    {
+				      "type": "host-local",
+				      "subnet": "10.0.0.1/24"
+				    }
+	          	  ]
+				}`)
+
+				dirPluginConf := []byte(`{
+				      "type": "bro-check-out-my-plugin",
+				      "subnet": "10.0.0.1/24"
+				}`)
+
+				Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())
+
+				subDir := filepath.Join(configDir, "some-network")
+				Expect(os.MkdirAll(subDir, 0o700)).To(Succeed())
+				Expect(os.WriteFile(filepath.Join(subDir, "funky-second-plugin.conf"), dirPluginConf, 0o600)).To(Succeed())
+
+				netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
+				Expect(err).NotTo(HaveOccurred())
+				Expect(netConfigList.LoadOnlyInlinedPlugins).To(BeFalse())
+				Expect(netConfigList.Plugins).To(HaveLen(2))
+			})
+
+			It("will ignore loaded plugins if `plugins` is set and `loadOnlyInlinedPlugins` is true", func() {
+				configList = []byte(`{
+				  "name": "some-network",
+				  "cniVersion": "0.4.0",
+				  "loadOnlyInlinedPlugins": true,
+				  "plugins": [
+				    {
+				      "type": "host-local",
+				      "subnet": "10.0.0.1/24"
+				    }
+	          	  ]
+				}`)
+
+				dirPluginConf := []byte(`{
+				      "type": "bro-check-out-my-plugin",
+				      "subnet": "10.0.0.1/24"
+				}`)
+
+				Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())
+				subDir := filepath.Join(configDir, "some-network")
+				Expect(os.MkdirAll(subDir, 0o700)).To(Succeed())
+				Expect(os.WriteFile(filepath.Join(subDir, "funky-second-plugin.conf"), dirPluginConf, 0o600)).To(Succeed())
+
+				netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
+				Expect(err).NotTo(HaveOccurred())
+				Expect(netConfigList.LoadOnlyInlinedPlugins).To(BeTrue())
+				Expect(netConfigList.Plugins).To(HaveLen(1))
+				Expect(netConfigList.Plugins[0].Network.Type).To(Equal("host-local"))
+			})
+		})
 	})
 
 	Describe("NetworkConfFromFile", func() {

pkg/types/types.go

@@ -88,10 +88,7 @@ type GCAttachment struct {
 // Note: DNS should be omit if DNS is empty but default Marshal function
 // will output empty structure hence need to write a Marshal function
 func (n *PluginConf) MarshalJSON() ([]byte, error) {
-	// use type alias to escape recursion for json.Marshal() to MarshalJSON()
-	type fixObjType = PluginConf
-
-	bytes, err := json.Marshal(fixObjType(*n))
+	bytes, err := json.Marshal(*n)
 	if err != nil {
 		return nil, err
 	}
@@ -123,7 +120,7 @@ type NetConfList struct {
 
 	Name         string        `json:"name,omitempty"`
 	DisableCheck bool          `json:"disableCheck,omitempty"`
-	DisableGC    bool       `json:"disableGC,omitempty"`
+	DisableGC    bool          `json:"disableGC,omitempty"`
 	Plugins      []*PluginConf `json:"plugins,omitempty"`
 }