Skip to content

Commit

Permalink
Change disableLoadPluginsFromPath -> loadOnlyInlinedPlugins
Browse files Browse the repository at this point in the history
Signed-off-by: Benjamin Leggett <[email protected]>
  • Loading branch information
bleggett committed Apr 8, 2024
1 parent 900eab3 commit 23ffb6f
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 38 deletions.
10 changes: 5 additions & 5 deletions SPEC.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
- `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.
- `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.
- `loadOnlyInlinedPlugins` (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, `loadOnlyInlinedPlugins` 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 `loadOnlyInlinedPlugins` 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:
Runtimes may aggregate plugin configuration objects from multiple sources, and must unambiguously associate each loaded plugin configuration object with a single, valid network configuration. All aggregated plugin configuration objects must be parsed, and each plugin with a parsable configuration object must be invoked.
Expand Down Expand Up @@ -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",
"disableLoadPluginsFromPath": false,
"loadOnlyInlinedPlugins": false,
}
```

Expand Down Expand Up @@ -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",
"disableLoadPluginsFromPath": false,
"loadOnlyInlinedPlugins": false,
plugins: [
{
"type": "bridge",
Expand Down Expand Up @@ -247,7 +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,
"loadOnlyInlinedPlugins": true,
"plugins": [
{
"type": "bridge",
Expand Down
2 changes: 1 addition & 1 deletion libcni/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ type NetworkConfigList struct {
Name string
CNIVersion string
DisableCheck bool
DisableLoadPluginsFromPath bool
LoadOnlyInlinedPlugins bool
Plugins []*PluginConfig
Bytes []byte
}
Expand Down
24 changes: 12 additions & 12 deletions libcni/conf.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,35 +174,35 @@ func NetworkConfFromBytes(confBytes []byte) (*NetworkConfigList, error) {
}
}

disableLoadPluginsFromPath := false
if rawLoadCheck, ok := rawList["disableLoadPluginsFromPath"]; ok {
disableLoadPluginsFromPath, ok = rawLoadCheck.(bool)
loadOnlyInlinedPlugins := false
if rawLoadCheck, ok := rawList["loadOnlyInlinedPlugins"]; ok {
loadOnlyInlinedPlugins, ok = rawLoadCheck.(bool)
if !ok {
return nil, fmt.Errorf("error parsing configuration list: invalid disableLoadPluginsFromPath type %T", rawLoadCheck)
return nil, fmt.Errorf("error parsing configuration list: invalid loadOnlyInlinedPlugins type %T", rawLoadCheck)
}
}

list := &NetworkConfigList{
Name: name,
DisableCheck: disableCheck,
DisableLoadPluginsFromPath: disableLoadPluginsFromPath,
LoadOnlyInlinedPlugins: loadOnlyInlinedPlugins,
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 `disableLoadPluginsFromPath == true`
// We can also have `loadOnlyInlinedPlugins == true`
//
// If `plugins` is there, then `disableLoadPluginsFromPath` can be true
// If `plugins` is there, then `loadOnlyInlinedPlugins` can be true
//
// If plugins is NOT there, then `disableLoadPluginsFromPath` cannot be true
// If plugins is NOT there, then `loadOnlyInlinedPlugins` 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 {
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
}

Expand Down Expand Up @@ -239,7 +239,7 @@ func NetworkConfFromFile(filename string) (*NetworkConfigList, error) {
return nil, err
}

if !conf.DisableLoadPluginsFromPath {
if !conf.LoadOnlyInlinedPlugins {
plugins, err := NetworkPluginConfsFromFiles(filepath.Dir(filename), conf.Name)
if err != nil {
return nil, err
Expand Down
40 changes: 20 additions & 20 deletions libcni/conf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,12 +390,12 @@ var _ = Describe("Loading configuration from disk", func() {
})
})

Context("for disableLoadPluginsFromPath", func() {
Context("for loadOnlyInlinedPlugins", func() {
It("the value will be parsed", func() {
configList = []byte(`{
"name": "some-network",
"cniVersion": "0.4.0",
"disableLoadPluginsFromPath": true,
"loadOnlyInlinedPlugins": true,
"plugins": [
{
"type": "host-local",
Expand All @@ -416,7 +416,7 @@ var _ = Describe("Loading configuration from disk", func() {

netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
Expect(err).NotTo(HaveOccurred())
Expect(netConfigList.DisableLoadPluginsFromPath).To(BeTrue())
Expect(netConfigList.LoadOnlyInlinedPlugins).To(BeTrue())
})

It("the value will be false if not in config", func() {
Expand All @@ -434,15 +434,15 @@ var _ = Describe("Loading configuration from disk", func() {

netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
Expect(err).NotTo(HaveOccurred())
Expect(netConfigList.DisableLoadPluginsFromPath).To(BeFalse())
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",
"disableLoadPluginsFromPath": "%s",
"loadOnlyInlinedPlugins": "%s",
"plugins": [
{
"type": "host-local",
Expand All @@ -453,26 +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 disableLoadPluginsFromPath type string"))
Expect(err).To(MatchError("error parsing configuration list: invalid loadOnlyInlinedPlugins type string"))
})

It("will return an error if `plugins` is missing and `disableLoadPluginsFromPath` is `true`", func() {
It("will return an error if `plugins` is missing and `loadOnlyInlinedPlugins` is `true`", func() {
configList = []byte(`{
"name": "some-network",
"cniVersion": "0.4.0",
"disableLoadPluginsFromPath": true
"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: `disableLoadPluginsFromPath` is true, and no 'plugins' key"))
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 `disableLoadPluginsFromPath` is false", func() {
It("will return no error if `plugins` is missing and `loadOnlyInlinedPlugins` is false", func() {
configList = []byte(`{
"name": "some-network",
"cniVersion": "0.4.0",
"disableLoadPluginsFromPath": false
"loadOnlyInlinedPlugins": false
}`)
Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed())

Expand All @@ -487,11 +487,11 @@ var _ = Describe("Loading configuration from disk", func() {

netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
Expect(err).NotTo(HaveOccurred())
Expect(netConfigList.DisableLoadPluginsFromPath).To(BeFalse())
Expect(netConfigList.LoadOnlyInlinedPlugins).To(BeFalse())
Expect(netConfigList.Plugins).To(HaveLen(1))
})

It("will return error if `disableLoadPluginsFromPath` is implicitly false + no conf plugin is defined, but no plugins subfolder with network name exists", func() {
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"
Expand All @@ -502,7 +502,7 @@ var _ = Describe("Loading configuration from disk", func() {
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() {
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",
Expand All @@ -519,7 +519,7 @@ var _ = Describe("Loading configuration from disk", func() {
Expect(err).NotTo(HaveOccurred())
})

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() {
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",
Expand All @@ -539,7 +539,7 @@ var _ = Describe("Loading configuration from disk", func() {
Expect(err).NotTo(HaveOccurred())
})

It("will merge loaded and inlined plugin lists if both `plugins` is set and `disableLoadPluginsFromPath` is false", func() {
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",
Expand All @@ -564,15 +564,15 @@ var _ = Describe("Loading configuration from disk", func() {

netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
Expect(err).NotTo(HaveOccurred())
Expect(netConfigList.DisableLoadPluginsFromPath).To(BeFalse())
Expect(netConfigList.LoadOnlyInlinedPlugins).To(BeFalse())
Expect(netConfigList.Plugins).To(HaveLen(2))
})

It("will ignore loaded plugins if `plugins` is set and `disableLoadPluginsFromPath` is true", func() {
It("will ignore loaded plugins if `plugins` is set and `loadOnlyInlinedPlugins` is true", func() {
configList = []byte(`{
"name": "some-network",
"cniVersion": "0.4.0",
"disableLoadPluginsFromPath": true,
"loadOnlyInlinedPlugins": true,
"plugins": [
{
"type": "host-local",
Expand All @@ -593,7 +593,7 @@ var _ = Describe("Loading configuration from disk", func() {

netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network")
Expect(err).NotTo(HaveOccurred())
Expect(netConfigList.DisableLoadPluginsFromPath).To(BeTrue())
Expect(netConfigList.LoadOnlyInlinedPlugins).To(BeTrue())
Expect(netConfigList.Plugins).To(HaveLen(1))
Expect(netConfigList.Plugins[0].Network.Type).To(Equal("host-local"))
})
Expand Down

0 comments on commit 23ffb6f

Please sign in to comment.