From 425425837ef952146a7becdccec088e0db68a372 Mon Sep 17 00:00:00 2001 From: Benjamin Leggett Date: Mon, 8 Apr 2024 12:25:36 -0400 Subject: [PATCH] libcni: Support subdirectory-based plugin loading (#928) Signed-off-by: Benjamin Leggett --- libcni/api.go | 12 +-- libcni/conf.go | 81 +++++++++++++++-- libcni/conf_test.go | 211 +++++++++++++++++++++++++++++++++++++++++++- pkg/types/types.go | 7 +- 4 files changed, 290 insertions(+), 21 deletions(-) diff --git a/libcni/api.go b/libcni/api.go index 3fedb761..99cd03c9 100644 --- a/libcni/api.go +++ b/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 { diff --git a/libcni/conf.go b/libcni/conf.go index 5db8cba1..0385640a 100644 --- a/libcni/conf.go +++ b/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 diff --git a/libcni/conf_test.go b/libcni/conf_test.go index 379e1152..fb71b828 100644 --- a/libcni/conf_test.go +++ b/libcni/conf_test.go @@ -386,7 +386,216 @@ 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(fmt.Sprintf("error parsing configuration list: invalid disableCheck value \"%s\"", badValue))) + Expect(err).To(MatchError(fmt.Sprintf("error parsing configuration list: invalid value \"%s\" for disableCheck", 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(fmt.Sprintf(`error parsing configuration list: invalid value "%s" for loadOnlyInlinedPlugins`, badValue))) + }) + + 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")) }) }) }) diff --git a/pkg/types/types.go b/pkg/types/types.go index f8afaae7..630c11ed 100644 --- a/pkg/types/types.go +++ b/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"` }