Invert flag, so directory-based plugin loading happens by default.

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

Author: bleggett

Diff for: SPEC.md

@@ -111,8 +111,8 @@ A network configuration consists of a JSON object with the following keys:
 - `cniVersions` (string list): List of all CNI versions which this configuration supports. See [version selection](#version-selection) below.
 - `name` (string): Network name. This should be unique across all network configurations on a host (or other administrative domain).  Must start with an alphanumeric character, optionally followed by any combination of one or more alphanumeric characters, underscore, dot (.) or hyphen (-). Must not contain characters disallowed in file paths. A path segment (such as a filesystem directory) with the same name as the network name, containing one or more plugin configuration JSON objects for that network, should exist at the same level as the network configuration object itself.
 - `disableCheck` (boolean): Either `true` or `false`.  If `disableCheck` is `true`, runtimes must not call `CHECK` for this network configuration list.  This allows an administrator to prevent `CHECK`ing where a combination of plugins is known to return spurious errors.
-- `loadPluginsFromFolder` (boolean): Either `true` or `false`. If `true`, indicates [plugin configuration objects](#plugin-configuration-objects) should be loaded from a sibling folder with the same name as the network `name` field. These sibling folders should exist at the same path as the network configuration itself. Any valid plugin configuration objects within the sibling folder will be appended to the final list of plugins for that network. If `plugins` is not present in the network configuration, `loadPluginsFromFolder` must be present, and set to true.
-- `plugins` (list): A list of inlined [plugin configuration objects](#plugin-configuration-objects). If this key is populated with inlined plugin objects, and `loadPluginsFromFolder` is true, the final set of plugins for a network must consist of all the plugin objects in this list, merged with all the plugins loaded from the sibling folder with the same name as the network.
+- `disableLoadPluginsFromPath` (boolean): Either `true` or `false`. If `false` (default), indicates [plugin configuration objects](#plugin-configuration-objects) should be loaded from a sibling directory with the same name as the network `name` field. These sibling directories should exist at the same path as the network configuration itself. Any valid plugin configuration objects within a named sibling directory will be appended to the final list of `plugins` for that network name. If set to `true`, plugin configuration objects in sibling directories will be ignored. If `plugins` is not present in the network configuration, `disableLoadPluginsFromPath` cannot be set to `true`.
+- `plugins` (list): A list of inlined [plugin configuration objects](#plugin-configuration-objects). If this key is populated with inlined plugin objects, and `disableLoadPluginsFromPath` is true, the final set of plugins for a network must consist of all the plugin objects in this list, merged with all the plugins loaded from the sibling folder with the same name as the network.
 
 #### Plugin configuration objects:
 All plugin configuration objects present in a directory with the same name as a valid network configuration must be parsed, and each plugin with a parsable configuration object must be invoked.
@@ -155,7 +155,7 @@ Network configuration with no inlined plugin confs, and two loaded plugin confs:
   "cniVersion": "1.1.0",
   "cniVersions": ["0.3.1", "0.4.0", "1.0.0", "1.1.0"],
   "name": "dbnet",
-  "loadPluginsFromFolder": true,
+  "disableLoadPluginsFromPath": false,
 }
 ```
 
@@ -202,7 +202,7 @@ Network configuration with one inlined plugin conf, and one loaded plugin conf:
   "cniVersion": "1.1.0",
   "cniVersions": ["0.3.1", "0.4.0", "1.0.0", "1.1.0"],
   "name": "dbnet",
-  "loadPluginsFromFolder": true,
+  "disableLoadPluginsFromPath": false,
   plugins: [
     {
       "type": "bridge",
@@ -247,6 +247,7 @@ Network configuration with one inlined plugin conf, and no loaded plugin conf:
   "cniVersion": "1.1.0",
   "cniVersions": ["0.3.1", "0.4.0", "1.0.0", "1.1.0"],
   "name": "dbnet",
+  "disableLoadPluginsFromPath": true,
   "plugins": [
     {
       "type": "bridge",

Diff for: libcni/api.go

@@ -77,12 +77,12 @@ type PluginConfig struct {
 }
 
 type NetworkConfigList struct {
-	Name                  string
-	CNIVersion            string
-	DisableCheck          bool
-	LoadPluginsFromFolder bool
-	Plugins               []*PluginConfig
-	Bytes                 []byte
+	Name                       string
+	CNIVersion                 string
+	DisableCheck               bool
+	DisableLoadPluginsFromPath bool
+	Plugins                    []*PluginConfig
+	Bytes                      []byte
 }
 
 type NetworkAttachment struct {

Diff for: libcni/conf.go

@@ -72,13 +72,8 @@ func NetworkPluginConfsFromFiles(networkConfPath, networkName string) ([]*Plugin
 	pluginConfPath := filepath.Join(networkConfPath, networkName)
 
 	pluginConfFiles, err := ConfFiles(pluginConfPath, []string{".conf"})
-	switch {
-	case err != nil:
+	if err != nil {
 		return nil, fmt.Errorf("failed to read plugin config files in %s: %w", pluginConfPath, err)
-	case len(pluginConfFiles) == 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 config found in %s", pluginConfPath)
 	}
 
 	for _, pluginConfFile := range pluginConfFiles {
@@ -179,31 +174,35 @@ func NetworkConfFromBytes(confBytes []byte) (*NetworkConfigList, error) {
 		}
 	}
 
-	loadPluginsFromFolder := false
-	if rawLoadCheck, ok := rawList["loadPluginsFromFolder"]; ok {
-		loadPluginsFromFolder, ok = rawLoadCheck.(bool)
+	disableLoadPluginsFromPath := false
+	if rawLoadCheck, ok := rawList["disableLoadPluginsFromPath"]; ok {
+		disableLoadPluginsFromPath, ok = rawLoadCheck.(bool)
 		if !ok {
-			return nil, fmt.Errorf("error parsing configuration list: invalid loadPluginsFromFolder type %T", rawLoadCheck)
+			return nil, fmt.Errorf("error parsing configuration list: invalid disableLoadPluginsFromPath type %T", rawLoadCheck)
 		}
 	}
 
 	list := &NetworkConfigList{
-		Name:                  name,
-		DisableCheck:          disableCheck,
-		LoadPluginsFromFolder: loadPluginsFromFolder,
-		CNIVersion:            cniVersion,
-		Bytes:                 confBytes,
+		Name:                       name,
+		DisableCheck:               disableCheck,
+		DisableLoadPluginsFromPath: disableLoadPluginsFromPath,
+		CNIVersion:                 cniVersion,
+		Bytes:                      confBytes,
 	}
 
 	var plugins []interface{}
 	plug, ok := rawList["plugins"]
 	// We can have a `plugins` list key in the main conf,
-	// We can also have `loadPluginsFromFolder == true`
-	// They can both be present in the same config.
-	// But if one of them is NOT present/false, the other *must* be there.
-	if !ok && !loadPluginsFromFolder {
-		return nil, fmt.Errorf("error parsing configuration list: `loadPluginsFromFolder` not true, and no 'plugins' key")
-	} else if !ok && loadPluginsFromFolder {
+	// We can also have `disableLoadPluginsFromPath == true`
+	//
+	// If `plugins` is there, then `disableLoadPluginsFromPath` can be true
+	//
+	// If plugins is NOT there, then `disableLoadPluginsFromPath` cannot be true
+	//
+	// We have to have at least some plugins.
+	if !ok && disableLoadPluginsFromPath {
+		return nil, fmt.Errorf("error parsing configuration list: `disableLoadPluginsFromPath` is true, and no 'plugins' key")
+	} else if !ok && !disableLoadPluginsFromPath {
 		return list, nil
 	}
 
@@ -240,14 +239,19 @@ func NetworkConfFromFile(filename string) (*NetworkConfigList, error) {
 		return nil, err
 	}
 
-	if conf.LoadPluginsFromFolder {
+	if !conf.DisableLoadPluginsFromPath {
 		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
 }
 
@@ -283,6 +287,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

Diff for: libcni/conf_test.go

@@ -390,12 +390,12 @@ var _ = Describe("Loading configuration from disk", func() {
 			})
 		})
 
-		Context("for loadPluginsFromFolder", func() {
+		Context("for disableLoadPluginsFromPath", func() {
 			It("the value will be parsed", func() {
 				configList = []byte(`{
 				  "name": "some-network",
 				  "cniVersion": "0.4.0",
-				  "loadPluginsFromFolder": true,
+				  "disableLoadPluginsFromPath": true,
 				  "plugins": [
 				    {
 				      "type": "host-local",
@@ -416,7 +416,7 @@ var _ = Describe("Loading configuration from disk", func() {
 
 				netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
 				Expect(err).NotTo(HaveOccurred())
-				Expect(netConfigList.LoadPluginsFromFolder).To(BeTrue())
+				Expect(netConfigList.DisableLoadPluginsFromPath).To(BeTrue())
 			})
 
 			It("the value will be false if not in config", func() {
@@ -434,15 +434,15 @@ var _ = Describe("Loading configuration from disk", func() {
 
 				netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
 				Expect(err).NotTo(HaveOccurred())
-				Expect(netConfigList.LoadPluginsFromFolder).To(BeFalse())
+				Expect(netConfigList.DisableLoadPluginsFromPath).To(BeFalse())
 			})
 
 			It("will return an error on an unrecognized value", func() {
-				const badValue string = "spagnum"
+				const badValue string = "sphagnum"
 				configList = []byte(fmt.Sprintf(`{
 				  "name": "some-network",
 				  "cniVersion": "0.4.0",
-				  "loadPluginsFromFolder": "%s",
+				  "disableLoadPluginsFromPath": "%s",
 				  "plugins": [
 				    {
 				      "type": "host-local",
@@ -453,25 +453,26 @@ var _ = Describe("Loading configuration from disk", func() {
 				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 loadPluginsFromFolder type string"))
+				Expect(err).To(MatchError("error parsing configuration list: invalid disableLoadPluginsFromPath type string"))
 			})
 
-			It("will return an error if `plugins` is missing and `loadPluginsFromFolder` is also missing", func() {
+			It("will return an error if `plugins` is missing and `disableLoadPluginsFromPath` is `true`", func() {
 				configList = []byte(`{
 				  "name": "some-network",
-				  "cniVersion": "0.4.0"
+				  "cniVersion": "0.4.0",
+				  "disableLoadPluginsFromPath": 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: `loadPluginsFromFolder` not true, and no 'plugins' key"))
+				Expect(err).To(MatchError("error parsing configuration list: `disableLoadPluginsFromPath` is true, and no 'plugins' key"))
 			})
 
-			It("will return no error if `plugins` is missing and `loadPluginsFromFolder` is true", func() {
+			It("will return no error if `plugins` is missing and `disableLoadPluginsFromPath` is false", func() {
 				configList = []byte(`{
 				  "name": "some-network",
 				  "cniVersion": "0.4.0",
-				  "loadPluginsFromFolder": true
+				  "disableLoadPluginsFromPath": false
 				}`)
 				Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())
 
@@ -486,43 +487,62 @@ var _ = Describe("Loading configuration from disk", func() {
 
 				netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
 				Expect(err).NotTo(HaveOccurred())
-				Expect(netConfigList.LoadPluginsFromFolder).To(BeTrue())
+				Expect(netConfigList.DisableLoadPluginsFromPath).To(BeFalse())
 				Expect(netConfigList.Plugins).To(HaveLen(1))
 			})
 
-			It("will return error if `loadPluginsFromFolder` is true but no plugins subfolder with network name exists", func() {
+			It("will return error if `disableLoadPluginsFromPath` 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 `disableLoadPluginsFromPath` 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",
-				  "loadPluginsFromFolder": true
+				  "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")
 				_, err := libcni.LoadNetworkConf(configDir, "some-network")
-				Expect(err).To(MatchError(fmt.Sprintf("no plugin config found in %s", subDir)))
+				Expect(err).NotTo(HaveOccurred())
 			})
 
-			It("will return  error if `loadPluginsFromFolder` is true and network name subfolder exists, but no plugin configs", func() {
+			It("will return NO error if `disableLoadPluginsFromPath` 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",
-				  "loadPluginsFromFolder": true
+				  "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).To(MatchError(fmt.Sprintf("no plugin config found in %s", subDir)))
+				Expect(err).NotTo(HaveOccurred())
 			})
 
-			It("will merge loaded and inlined plugin lists if both `plugins` is set and `loadPluginsFromFolder` is true", func() {
+			It("will merge loaded and inlined plugin lists if both `plugins` is set and `disableLoadPluginsFromPath` is false", func() {
 				configList = []byte(`{
 				  "name": "some-network",
 				  "cniVersion": "0.4.0",
-				  "loadPluginsFromFolder": true,
 				  "plugins": [
 				    {
 				      "type": "host-local",
@@ -544,14 +564,15 @@ var _ = Describe("Loading configuration from disk", func() {
 
 				netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
 				Expect(err).NotTo(HaveOccurred())
-				Expect(netConfigList.LoadPluginsFromFolder).To(BeTrue())
+				Expect(netConfigList.DisableLoadPluginsFromPath).To(BeFalse())
 				Expect(netConfigList.Plugins).To(HaveLen(2))
 			})
 
-			It("will ignore loaded plugins if `plugins` is set and `loadPluginsFromFolder` is not present", func() {
+			It("will ignore loaded plugins if `plugins` is set and `disableLoadPluginsFromPath` is true", func() {
 				configList = []byte(`{
 				  "name": "some-network",
 				  "cniVersion": "0.4.0",
+				  "disableLoadPluginsFromPath": true,
 				  "plugins": [
 				    {
 				      "type": "host-local",
@@ -572,7 +593,7 @@ var _ = Describe("Loading configuration from disk", func() {
 
 				netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
 				Expect(err).NotTo(HaveOccurred())
-				Expect(netConfigList.LoadPluginsFromFolder).To(BeFalse())
+				Expect(netConfigList.DisableLoadPluginsFromPath).To(BeTrue())
 				Expect(netConfigList.Plugins).To(HaveLen(1))
 				Expect(netConfigList.Plugins[0].Network.Type).To(Equal("host-local"))
 			})