From 9f73d4da825f853a6be81e6f9b20ef8d7257de7f Mon Sep 17 00:00:00 2001 From: Benjamin Leggett Date: Mon, 8 Apr 2024 12:13:26 -0400 Subject: [PATCH 1/3] SPEC: #928 support non-inlined plugin loading Signed-off-by: Benjamin Leggett --- SPEC.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/SPEC.md b/SPEC.md index 71d35dd7..19e732e8 100644 --- a/SPEC.md +++ b/SPEC.md @@ -109,14 +109,16 @@ A network configuration consists of a JSON object with the following keys: - `cniVersion` (string): [Semantic Version 2.0](https://semver.org) of CNI specification to which this configuration list and all the individual configurations conform. Currently "1.1.0" - `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 (-). +- `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. - `disableGC` (boolean): Either `true` or `false`. If `disableGC` is `true`, runtimes must not call `GC` for this network configuration list. This allows an administrator to prevent `GC`ing when it is known that garbage collection may have undesired effects (e.g. shared configuration between multiple runtimes). -- `plugins` (list): A list of CNI plugins and their configuration, which is a list of plugin configuration objects. +- `loadOnlyInlinedPlugins` (boolean): Either `true` or `false`. If `false` (default), indicates [plugin configuration objects](#plugin-configuration-objects) can be aggregated from multiple sources. Any valid plugin configuration objects aggregated from other sources must be appended to the final list of `plugins` for that network name. If set to `true`, indicates that valid plugin configuration objects aggregated from sources other than the main network configuration 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: -Plugin configuration objects may contain additional fields than the ones defined here. -The runtime MUST pass through these fields, unchanged, to the plugin, as defined in section 3. +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 validated, and each plugin with a valid configuration object must be invoked. + +Plugin configuration objects may contain additional fields beyond the ones defined here. The runtime MUST pass through these fields, unchanged, to the invoked plugin, as defined in section 3. **Required keys:** - `type` (string): Matches the name of the CNI plugin binary on disk. Must not contain characters disallowed in file paths for the system (e.g. / or \\). @@ -147,6 +149,7 @@ Plugins that consume any of these configuration keys should respect their intend Plugins may define additional fields that they accept and may generate an error if called with unknown fields. Runtimes must preserve unknown fields in plugin configuration objects when transforming for execution. #### Example configuration +The following is an example JSON representation of a network configuration `dbnet` with three plugin configurations (`bridge`, `tuning`, and `portmap`). ```jsonc { "cniVersion": "1.1.0", @@ -158,7 +161,7 @@ Plugins may define additional fields that they accept and may generate an error // plugin specific parameters "bridge": "cni0", "keyA": ["some more", "plugin specific", "configuration"], - + "ipam": { "type": "host-local", // ipam specific @@ -375,8 +378,6 @@ Resources may, for example, include: A plugin SHOULD remove as many stale resources as possible. For example, a plugin should remove any IPAM reservations associated with attachments not in the provided list. The plugin MAY assume that the isolation domain (e.g. network namespace) has been deleted, and thus any resources (e.g. network interfaces) therein have been removed. -Garbage collection is a per-network operation. If a plugin manages resources shared across multiple networks, it must only remove stale resources known to belong to the network provided in the `GC `action. - Plugins should generally complete a `GC` action without error. If an error is encountered, a plugin should continue; removing as many resources as possible, and report the errors back to the runtime. Plugins MUST, additionally, forward any GC calls to delegated plugins they are configured to use (see section 4). From c0cc785dbcc28acb9685ac4c9e6902e46fd1bc7a Mon Sep 17 00:00:00 2001 From: Benjamin Leggett Date: Mon, 8 Apr 2024 12:21:12 -0400 Subject: [PATCH 2/3] Use type aliases to hint deprecation for old API types (#928) Signed-off-by: Benjamin Leggett --- cnitool/cnitool.go | 2 +- libcni/api.go | 59 +++++++++++--------- libcni/api_test.go | 14 ++--- libcni/conf.go | 79 +++++++++++++++++++------- libcni/conf_test.go | 108 ++++++++++++++++++------------------ pkg/types/types.go | 21 +++---- pkg/version/version.go | 2 +- pkg/version/version_test.go | 8 +-- plugins/test/noop/main.go | 4 +- 9 files changed, 171 insertions(+), 126 deletions(-) diff --git a/cnitool/cnitool.go b/cnitool/cnitool.go index fc515fd4..3ac2ba37 100644 --- a/cnitool/cnitool.go +++ b/cnitool/cnitool.go @@ -68,7 +68,7 @@ func main() { if netdir == "" { netdir = DefaultNetDir } - netconf, err := libcni.LoadConfList(netdir, os.Args[2]) + netconf, err := libcni.LoadNetworkConf(netdir, os.Args[2]) if err != nil { exit(err) } diff --git a/libcni/api.go b/libcni/api.go index de2dc622..3fedb761 100644 --- a/libcni/api.go +++ b/libcni/api.go @@ -67,18 +67,23 @@ type RuntimeConf struct { CacheDir string } -type NetworkConfig struct { - Network *types.NetConf +// Deprecated: Use PluginConfig instead of NetworkConfig, the NetworkConfig +// backwards-compat alias will be removed in a future release. +type NetworkConfig = PluginConfig + +type PluginConfig struct { + Network *types.PluginConf Bytes []byte } type NetworkConfigList struct { - Name string - CNIVersion string - DisableCheck bool + Name string + CNIVersion string + DisableCheck bool DisableGC bool - Plugins []*NetworkConfig - Bytes []byte + LoadOnlyInlinedPlugins bool + Plugins []*PluginConfig + Bytes []byte } type NetworkAttachment struct { @@ -102,14 +107,14 @@ type CNI interface { GetNetworkListCachedResult(net *NetworkConfigList, rt *RuntimeConf) (types.Result, error) GetNetworkListCachedConfig(net *NetworkConfigList, rt *RuntimeConf) ([]byte, *RuntimeConf, error) - AddNetwork(ctx context.Context, net *NetworkConfig, rt *RuntimeConf) (types.Result, error) - CheckNetwork(ctx context.Context, net *NetworkConfig, rt *RuntimeConf) error - DelNetwork(ctx context.Context, net *NetworkConfig, rt *RuntimeConf) error - GetNetworkCachedResult(net *NetworkConfig, rt *RuntimeConf) (types.Result, error) - GetNetworkCachedConfig(net *NetworkConfig, rt *RuntimeConf) ([]byte, *RuntimeConf, error) + AddNetwork(ctx context.Context, net *PluginConfig, rt *RuntimeConf) (types.Result, error) + CheckNetwork(ctx context.Context, net *PluginConfig, rt *RuntimeConf) error + DelNetwork(ctx context.Context, net *PluginConfig, rt *RuntimeConf) error + GetNetworkCachedResult(net *PluginConfig, rt *RuntimeConf) (types.Result, error) + GetNetworkCachedConfig(net *PluginConfig, rt *RuntimeConf) ([]byte, *RuntimeConf, error) ValidateNetworkList(ctx context.Context, net *NetworkConfigList) ([]string, error) - ValidateNetwork(ctx context.Context, net *NetworkConfig) ([]string, error) + ValidateNetwork(ctx context.Context, net *PluginConfig) ([]string, error) GCNetworkList(ctx context.Context, net *NetworkConfigList, args *GCArgs) error GetStatusNetworkList(ctx context.Context, net *NetworkConfigList) error @@ -147,7 +152,7 @@ func NewCNIConfigWithCacheDir(path []string, cacheDir string, exec invoke.Exec) } } -func buildOneConfig(name, cniVersion string, orig *NetworkConfig, prevResult types.Result, rt *RuntimeConf) (*NetworkConfig, error) { +func buildOneConfig(name, cniVersion string, orig *PluginConfig, prevResult types.Result, rt *RuntimeConf) (*PluginConfig, error) { var err error inject := map[string]interface{}{ @@ -183,7 +188,7 @@ func buildOneConfig(name, cniVersion string, orig *NetworkConfig, prevResult typ // capabilities include "portMappings", and the CapabilityArgs map includes a // "portMappings" key, that key and its value are added to the "runtimeConfig" // dictionary to be passed to the plugin's stdin. -func injectRuntimeConfig(orig *NetworkConfig, rt *RuntimeConf) (*NetworkConfig, error) { +func injectRuntimeConfig(orig *PluginConfig, rt *RuntimeConf) (*PluginConfig, error) { var err error rc := make(map[string]interface{}) @@ -404,7 +409,7 @@ func (c *CNIConfig) GetNetworkListCachedResult(list *NetworkConfigList, rt *Runt // GetNetworkCachedResult returns the cached Result of the previous // AddNetwork() operation for a network, or an error. -func (c *CNIConfig) GetNetworkCachedResult(net *NetworkConfig, rt *RuntimeConf) (types.Result, error) { +func (c *CNIConfig) GetNetworkCachedResult(net *PluginConfig, rt *RuntimeConf) (types.Result, error) { return c.getCachedResult(net.Network.Name, net.Network.CNIVersion, rt) } @@ -416,7 +421,7 @@ func (c *CNIConfig) GetNetworkListCachedConfig(list *NetworkConfigList, rt *Runt // GetNetworkCachedConfig copies the input RuntimeConf to output // RuntimeConf with fields updated with info from the cached Config. -func (c *CNIConfig) GetNetworkCachedConfig(net *NetworkConfig, rt *RuntimeConf) ([]byte, *RuntimeConf, error) { +func (c *CNIConfig) GetNetworkCachedConfig(net *PluginConfig, rt *RuntimeConf) ([]byte, *RuntimeConf, error) { return c.getCachedConfig(net.Network.Name, rt) } @@ -482,7 +487,7 @@ func (c *CNIConfig) GetCachedAttachments(containerID string) ([]*NetworkAttachme return attachments, nil } -func (c *CNIConfig) addNetwork(ctx context.Context, name, cniVersion string, net *NetworkConfig, prevResult types.Result, rt *RuntimeConf) (types.Result, error) { +func (c *CNIConfig) addNetwork(ctx context.Context, name, cniVersion string, net *PluginConfig, prevResult types.Result, rt *RuntimeConf) (types.Result, error) { c.ensureExec() pluginPath, err := c.exec.FindInPath(net.Network.Type, c.Path) if err != nil { @@ -524,7 +529,7 @@ func (c *CNIConfig) AddNetworkList(ctx context.Context, list *NetworkConfigList, return result, nil } -func (c *CNIConfig) checkNetwork(ctx context.Context, name, cniVersion string, net *NetworkConfig, prevResult types.Result, rt *RuntimeConf) error { +func (c *CNIConfig) checkNetwork(ctx context.Context, name, cniVersion string, net *PluginConfig, prevResult types.Result, rt *RuntimeConf) error { c.ensureExec() pluginPath, err := c.exec.FindInPath(net.Network.Type, c.Path) if err != nil { @@ -566,7 +571,7 @@ func (c *CNIConfig) CheckNetworkList(ctx context.Context, list *NetworkConfigLis return nil } -func (c *CNIConfig) delNetwork(ctx context.Context, name, cniVersion string, net *NetworkConfig, prevResult types.Result, rt *RuntimeConf) error { +func (c *CNIConfig) delNetwork(ctx context.Context, name, cniVersion string, net *PluginConfig, prevResult types.Result, rt *RuntimeConf) error { c.ensureExec() pluginPath, err := c.exec.FindInPath(net.Network.Type, c.Path) if err != nil { @@ -607,7 +612,7 @@ func (c *CNIConfig) DelNetworkList(ctx context.Context, list *NetworkConfigList, return nil } -func pluginDescription(net *types.NetConf) string { +func pluginDescription(net *types.PluginConf) string { if net == nil { return "" } @@ -621,7 +626,7 @@ func pluginDescription(net *types.NetConf) string { } // AddNetwork executes the plugin with the ADD command -func (c *CNIConfig) AddNetwork(ctx context.Context, net *NetworkConfig, rt *RuntimeConf) (types.Result, error) { +func (c *CNIConfig) AddNetwork(ctx context.Context, net *PluginConfig, rt *RuntimeConf) (types.Result, error) { result, err := c.addNetwork(ctx, net.Network.Name, net.Network.CNIVersion, net, nil, rt) if err != nil { return nil, err @@ -635,7 +640,7 @@ func (c *CNIConfig) AddNetwork(ctx context.Context, net *NetworkConfig, rt *Runt } // CheckNetwork executes the plugin with the CHECK command -func (c *CNIConfig) CheckNetwork(ctx context.Context, net *NetworkConfig, rt *RuntimeConf) error { +func (c *CNIConfig) CheckNetwork(ctx context.Context, net *PluginConfig, rt *RuntimeConf) error { // CHECK was added in CNI spec version 0.4.0 and higher if gtet, err := version.GreaterThanOrEqualTo(net.Network.CNIVersion, "0.4.0"); err != nil { return err @@ -651,7 +656,7 @@ func (c *CNIConfig) CheckNetwork(ctx context.Context, net *NetworkConfig, rt *Ru } // DelNetwork executes the plugin with the DEL command -func (c *CNIConfig) DelNetwork(ctx context.Context, net *NetworkConfig, rt *RuntimeConf) error { +func (c *CNIConfig) DelNetwork(ctx context.Context, net *PluginConfig, rt *RuntimeConf) error { var cachedResult types.Result // Cached result on DEL was added in CNI spec version 0.4.0 and higher @@ -711,7 +716,7 @@ func (c *CNIConfig) ValidateNetworkList(ctx context.Context, list *NetworkConfig // ValidateNetwork checks that a configuration is reasonably valid. // It uses the same logic as ValidateNetworkList) // Returns a list of capabilities -func (c *CNIConfig) ValidateNetwork(ctx context.Context, net *NetworkConfig) ([]string, error) { +func (c *CNIConfig) ValidateNetwork(ctx context.Context, net *PluginConfig) ([]string, error) { caps := []string{} for c, ok := range net.Network.Capabilities { if ok { @@ -834,7 +839,7 @@ func (c *CNIConfig) GCNetworkList(ctx context.Context, list *NetworkConfigList, return errors.Join(errs...) } -func (c *CNIConfig) gcNetwork(ctx context.Context, net *NetworkConfig) error { +func (c *CNIConfig) gcNetwork(ctx context.Context, net *PluginConfig) error { c.ensureExec() pluginPath, err := c.exec.FindInPath(net.Network.Type, c.Path) if err != nil { @@ -869,7 +874,7 @@ func (c *CNIConfig) GetStatusNetworkList(ctx context.Context, list *NetworkConfi return nil } -func (c *CNIConfig) getStatusNetwork(ctx context.Context, net *NetworkConfig) error { +func (c *CNIConfig) getStatusNetwork(ctx context.Context, net *PluginConfig) error { c.ensureExec() pluginPath, err := c.exec.FindInPath(net.Network.Type, c.Path) if err != nil { diff --git a/libcni/api_test.go b/libcni/api_test.go index 2b0ec555..76d20925 100644 --- a/libcni/api_test.go +++ b/libcni/api_test.go @@ -176,7 +176,7 @@ var _ = Describe("Invoking plugins", func() { pluginConfig []byte cniConfig *libcni.CNIConfig runtimeConfig *libcni.RuntimeConf - netConfig *libcni.NetworkConfig + netConfig *libcni.PluginConfig ctx context.Context ) @@ -295,7 +295,7 @@ var _ = Describe("Invoking plugins", func() { debug *noop_debug.Debug pluginConfig string cniConfig *libcni.CNIConfig - netConfig *libcni.NetworkConfig + netConfig *libcni.PluginConfig runtimeConfig *libcni.RuntimeConf ctx context.Context @@ -1636,7 +1636,7 @@ var _ = Describe("Invoking plugins", func() { cniBinPath string pluginConfig string cniConfig *libcni.CNIConfig - netConfig *libcni.NetworkConfig + netConfig *libcni.PluginConfig runtimeConfig *libcni.RuntimeConf netConfigList *libcni.NetworkConfigList ) @@ -1809,7 +1809,7 @@ var _ = Describe("Invoking plugins", func() { cniBinPath string pluginConfig string cniConfig *libcni.CNIConfig - netConfig *libcni.NetworkConfig + netConfig *libcni.PluginConfig runtimeConfig *libcni.RuntimeConf ctx context.Context @@ -1931,14 +1931,14 @@ var _ = Describe("Invoking plugins", func() { Context("when the RuntimeConf is incomplete", func() { var ( testRt *libcni.RuntimeConf - testNetConf *libcni.NetworkConfig + testNetConf *libcni.PluginConfig testNetConfList *libcni.NetworkConfigList ) BeforeEach(func() { testRt = &libcni.RuntimeConf{} - testNetConf = &libcni.NetworkConfig{ - Network: &types.NetConf{}, + testNetConf = &libcni.PluginConfig{ + Network: &types.PluginConf{}, } testNetConfList = &libcni.NetworkConfigList{} }) diff --git a/libcni/conf.go b/libcni/conf.go index daab839f..5db8cba1 100644 --- a/libcni/conf.go +++ b/libcni/conf.go @@ -46,9 +46,16 @@ func (e NoConfigsFoundError) Error() string { return fmt.Sprintf(`no net configurations found in %s`, e.Dir) } -func ConfFromBytes(bytes []byte) (*NetworkConfig, error) { - conf := &NetworkConfig{Bytes: bytes, Network: &types.NetConf{}} - if err := json.Unmarshal(bytes, conf.Network); err != nil { +// This will not validate that the plugins actually belong to the netconfig by ensuring +// that they are loaded from a directory named after the networkName, relative to the network config. +// +// Since here we are just accepting raw bytes, the caller is responsible for ensuring that the plugin +// config provided here actually "belongs" to the networkconfig in question. +func NetworkPluginConfFromBytes(pluginConfBytes []byte) (*PluginConfig, error) { + // TODO why are we creating a struct that holds both the byte representation and the deserialized + // representation, and returning that, instead of just returning the deserialized representation? + conf := &PluginConfig{Bytes: pluginConfBytes, Network: &types.PluginConf{}} + if err := json.Unmarshal(pluginConfBytes, conf.Network); err != nil { return nil, fmt.Errorf("error parsing configuration: %w", err) } if conf.Network.Type == "" { @@ -57,17 +64,9 @@ func ConfFromBytes(bytes []byte) (*NetworkConfig, error) { return conf, nil } -func ConfFromFile(filename string) (*NetworkConfig, error) { - bytes, err := os.ReadFile(filename) - if err != nil { - return nil, fmt.Errorf("error reading %s: %w", filename, err) - } - return ConfFromBytes(bytes) -} - -func ConfListFromBytes(bytes []byte) (*NetworkConfigList, error) { +func NetworkConfFromBytes(confBytes []byte) (*NetworkConfigList, error) { rawList := make(map[string]interface{}) - if err := json.Unmarshal(bytes, &rawList); err != nil { + if err := json.Unmarshal(confBytes, &rawList); err != nil { return nil, fmt.Errorf("error parsing configuration list: %w", err) } @@ -168,7 +167,7 @@ func ConfListFromBytes(bytes []byte) (*NetworkConfigList, error) { DisableCheck: disableCheck, DisableGC: disableGC, CNIVersion: cniVersion, - Bytes: bytes, + Bytes: confBytes, } var plugins []interface{} @@ -199,7 +198,7 @@ func ConfListFromBytes(bytes []byte) (*NetworkConfigList, error) { return list, nil } -func ConfListFromFile(filename string) (*NetworkConfigList, error) { +func NetworkConfFromFile(filename string) (*NetworkConfigList, error) { bytes, err := os.ReadFile(filename) if err != nil { return nil, fmt.Errorf("error reading %s: %w", filename, err) @@ -207,6 +206,32 @@ func ConfListFromFile(filename string) (*NetworkConfigList, error) { return ConfListFromBytes(bytes) } +// Deprecated: This file format is no longer supported, use NetworkConfXXX and NetworkPluginXXX functions +func ConfFromBytes(bytes []byte) (*NetworkConfig, error) { + return NetworkPluginConfFromBytes(bytes) +} + +// Deprecated: This file format is no longer supported, use NetworkConfXXX and NetworkPluginXXX functions +func ConfFromFile(filename string) (*NetworkConfig, error) { + bytes, err := os.ReadFile(filename) + if err != nil { + return nil, fmt.Errorf("error reading %s: %w", filename, err) + } + return ConfFromBytes(bytes) +} + +// Deprecated: Use NetworkConfXXX and NetworkPluginXXX functions +func ConfListFromBytes(bytes []byte) (*NetworkConfigList, error) { + return NetworkConfFromBytes(bytes) +} + +// Deprecated: Use NetworkConfXXX and NetworkPluginXXX functions +func ConfListFromFile(filename string) (*NetworkConfigList, error) { + return NetworkConfFromFile(filename) +} + +// ConfFiles simply returns a slice of all files in the provided directory +// with extensions matching the provided set. func ConfFiles(dir string, extensions []string) ([]string, error) { // In part, adapted from rkt/networking/podenv.go#listFiles files, err := os.ReadDir(dir) @@ -233,6 +258,7 @@ func ConfFiles(dir string, extensions []string) ([]string, error) { return confFiles, nil } +// Deprecated: This file format is no longer supported, use NetworkConfXXX and NetworkPluginXXX functions func LoadConf(dir, name string) (*NetworkConfig, error) { files, err := ConfFiles(dir, []string{".conf", ".json"}) switch { @@ -255,7 +281,17 @@ func LoadConf(dir, name string) (*NetworkConfig, error) { return nil, NotFoundError{dir, name} } +// Deprecated: Use NetworkConfXXX and NetworkPluginXXX functions func LoadConfList(dir, name string) (*NetworkConfigList, error) { + return LoadNetworkConf(dir, name) +} + +// LoadNetworkConf looks at all the network configs in a given dir, +// loads and parses them all, and returns the first one with an extension of `.conf` +// that matches the provided network name predicate. +func LoadNetworkConf(dir, name string) (*NetworkConfigList, error) { + // TODO this .conflist/.conf extension thing is confusing and inexact + // for implementors. We should pick one extension for everything and stick with it. files, err := ConfFiles(dir, []string{".conflist"}) if err != nil { return nil, err @@ -263,7 +299,7 @@ func LoadConfList(dir, name string) (*NetworkConfigList, error) { sort.Strings(files) for _, confFile := range files { - conf, err := ConfListFromFile(confFile) + conf, err := NetworkConfFromFile(confFile) if err != nil { return nil, err } @@ -272,7 +308,7 @@ func LoadConfList(dir, name string) (*NetworkConfigList, error) { } } - // Try and load a network configuration file (instead of list) + // Deprecated: Try and load a network configuration file (instead of list) // from the same name, then upconvert. singleConf, err := LoadConf(dir, name) if err != nil { @@ -288,7 +324,8 @@ func LoadConfList(dir, name string) (*NetworkConfigList, error) { return ConfListFromConf(singleConf) } -func InjectConf(original *NetworkConfig, newValues map[string]interface{}) (*NetworkConfig, error) { +// InjectConf takes a PluginConfig and inserts additional values into it, ensuring the result is serializable. +func InjectConf(original *PluginConfig, newValues map[string]interface{}) (*PluginConfig, error) { config := make(map[string]interface{}) err := json.Unmarshal(original.Bytes, &config) if err != nil { @@ -312,12 +349,14 @@ func InjectConf(original *NetworkConfig, newValues map[string]interface{}) (*Net return nil, err } - return ConfFromBytes(newBytes) + return NetworkPluginConfFromBytes(newBytes) } // ConfListFromConf "upconverts" a network config in to a NetworkConfigList, // with the single network as the only entry in the list. -func ConfListFromConf(original *NetworkConfig) (*NetworkConfigList, error) { +// +// Deprecated: Non-conflist file formats are unsupported, use NetworkConfXXX and NetworkPluginXXX functions +func ConfListFromConf(original *PluginConfig) (*NetworkConfigList, error) { // Re-deserialize the config's json, then make a raw map configlist. // This may seem a bit strange, but it's to make the Bytes fields // actually make sense. Otherwise, the generated json is littered with diff --git a/libcni/conf_test.go b/libcni/conf_test.go index 8aafeb9f..379e1152 100644 --- a/libcni/conf_test.go +++ b/libcni/conf_test.go @@ -50,8 +50,8 @@ var _ = Describe("Loading configuration from disk", func() { It("finds the network config file for the plugin of the given type", func() { netConfig, err := libcni.LoadConf(configDir, "some-plugin") Expect(err).NotTo(HaveOccurred()) - Expect(netConfig).To(Equal(&libcni.NetworkConfig{ - Network: &types.NetConf{ + Expect(netConfig).To(Equal(&libcni.PluginConfig{ + Network: &types.PluginConf{ Name: "some-plugin", Type: "foobar", }, @@ -79,8 +79,8 @@ var _ = Describe("Loading configuration from disk", func() { It("finds the network config file for the plugin of the given type", func() { netConfig, err := libcni.LoadConf(configDir, "some-plugin") Expect(err).NotTo(HaveOccurred()) - Expect(netConfig).To(Equal(&libcni.NetworkConfig{ - Network: &types.NetConf{ + Expect(netConfig).To(Equal(&libcni.PluginConfig{ + Network: &types.PluginConf{ Name: "some-plugin", Type: "foobar", }, @@ -181,7 +181,7 @@ var _ = Describe("Loading configuration from disk", func() { }) }) - Describe("ConfFromBytes", func() { + Describe("NetworkPluginConfFromBytes", func() { Context("when the config is missing 'type'", func() { It("returns a useful error", func() { _, err := libcni.ConfFromBytes([]byte(`{ "name": "some-plugin", "some-key": "some-value" }`)) @@ -190,7 +190,7 @@ var _ = Describe("Loading configuration from disk", func() { }) }) - Describe("LoadConfList", func() { + Describe("LoadNetworkConf", func() { var ( configDir string configList []byte @@ -202,7 +202,7 @@ var _ = Describe("Loading configuration from disk", func() { Expect(err).NotTo(HaveOccurred()) configList = []byte(`{ - "name": "some-list", + "name": "some-network", "cniVersion": "0.2.0", "disableCheck": true, "plugins": [ @@ -228,23 +228,23 @@ var _ = Describe("Loading configuration from disk", func() { }) It("finds the network config file for the plugin of the given type", func() { - netConfigList, err := libcni.LoadConfList(configDir, "some-list") + netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network") Expect(err).NotTo(HaveOccurred()) Expect(netConfigList).To(Equal(&libcni.NetworkConfigList{ - Name: "some-list", + Name: "some-network", CNIVersion: "0.2.0", DisableCheck: true, - Plugins: []*libcni.NetworkConfig{ + Plugins: []*libcni.PluginConfig{ { - Network: &types.NetConf{Type: "host-local"}, + Network: &types.PluginConf{Type: "host-local"}, Bytes: []byte(`{"subnet":"10.0.0.1/24","type":"host-local"}`), }, { - Network: &types.NetConf{Type: "bridge"}, + Network: &types.PluginConf{Type: "bridge"}, Bytes: []byte(`{"mtu":1400,"type":"bridge"}`), }, { - Network: &types.NetConf{Type: "port-forwarding"}, + Network: &types.PluginConf{Type: "port-forwarding"}, Bytes: []byte(`{"ports":{"20.0.0.1:8080":"80"},"type":"port-forwarding"}`), }, }, @@ -255,7 +255,7 @@ var _ = Describe("Loading configuration from disk", func() { Context("when there is a config file with the same name as the list", func() { BeforeEach(func() { configFile := []byte(`{ - "name": "some-list", + "name": "some-network", "cniVersion": "0.2.0", "type": "bridge" }`) @@ -263,7 +263,7 @@ var _ = Describe("Loading configuration from disk", func() { }) It("Loads the config list first", func() { - netConfigList, err := libcni.LoadConfList(configDir, "some-list") + netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network") Expect(err).NotTo(HaveOccurred()) Expect(netConfigList.Plugins).To(HaveLen(3)) }) @@ -271,7 +271,7 @@ var _ = Describe("Loading configuration from disk", func() { It("falls back to the config file", func() { Expect(os.Remove(filepath.Join(configDir, "50-whatever.conflist"))).To(Succeed()) - netConfigList, err := libcni.LoadConfList(configDir, "some-list") + netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network") Expect(err).NotTo(HaveOccurred()) Expect(netConfigList.Plugins).To(HaveLen(1)) Expect(netConfigList.Plugins[0].Network.Type).To(Equal("bridge")) @@ -284,15 +284,15 @@ var _ = Describe("Loading configuration from disk", func() { }) It("returns a useful error", func() { - _, err := libcni.LoadConfList(configDir, "some-plugin") + _, err := libcni.LoadNetworkConf(configDir, "some-network") Expect(err).To(MatchError(libcni.NoConfigsFoundError{Dir: configDir})) }) }) - Context("when there is no config for the desired plugin list", func() { + Context("when there is no config for the desired network name", func() { It("returns a useful error", func() { - _, err := libcni.LoadConfList(configDir, "some-other-plugin") - Expect(err).To(MatchError(libcni.NotFoundError{Dir: configDir, Name: "some-other-plugin"})) + _, err := libcni.LoadNetworkConf(configDir, "some-other-network") + Expect(err).To(MatchError(libcni.NotFoundError{Dir: configDir, Name: "some-other-network"})) }) }) @@ -302,7 +302,7 @@ var _ = Describe("Loading configuration from disk", func() { }) It("returns a useful error", func() { - _, err := libcni.LoadConfList(configDir, "some-plugin") + _, err := libcni.LoadNetworkConf(configDir, "some-plugin") Expect(err).To(MatchError(`error parsing configuration list: unexpected end of JSON input`)) }) }) @@ -326,7 +326,7 @@ var _ = Describe("Loading configuration from disk", func() { }) It("will not find the config", func() { - _, err := libcni.LoadConfList(configDir, "deep") + _, err := libcni.LoadNetworkConf(configDir, "deep") Expect(err).To(MatchError(HavePrefix("no net configuration with name"))) }) }) @@ -334,7 +334,7 @@ var _ = Describe("Loading configuration from disk", func() { Context("when disableCheck is a string not a boolean", func() { It("will read a 'true' value and convert to boolean", func() { configList = []byte(`{ - "name": "some-list", + "name": "some-network", "cniVersion": "0.4.0", "disableCheck": "true", "plugins": [ @@ -346,14 +346,14 @@ var _ = Describe("Loading configuration from disk", func() { }`) Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed()) - netConfigList, err := libcni.LoadConfList(configDir, "some-list") + netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network") Expect(err).NotTo(HaveOccurred()) Expect(netConfigList.DisableCheck).To(BeTrue()) }) It("will read a 'false' value and convert to boolean", func() { configList = []byte(`{ - "name": "some-list", + "name": "some-network", "cniVersion": "0.4.0", "disableCheck": "false", "plugins": [ @@ -365,7 +365,7 @@ var _ = Describe("Loading configuration from disk", func() { }`) Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed()) - netConfigList, err := libcni.LoadConfList(configDir, "some-list") + netConfigList, err := libcni.LoadNetworkConf(configDir, "some-network") Expect(err).NotTo(HaveOccurred()) Expect(netConfigList.DisableCheck).To(BeFalse()) }) @@ -373,7 +373,7 @@ var _ = Describe("Loading configuration from disk", func() { It("will return an error on an unrecognized value", func() { const badValue string = "adsfasdfasf" configList = []byte(fmt.Sprintf(`{ - "name": "some-list", + "name": "some-network", "cniVersion": "0.4.0", "disableCheck": "%s", "plugins": [ @@ -385,35 +385,35 @@ var _ = Describe("Loading configuration from disk", func() { }`, badValue)) Expect(os.WriteFile(filepath.Join(configDir, "50-whatever.conflist"), configList, 0o600)).To(Succeed()) - _, err := libcni.LoadConfList(configDir, "some-list") - Expect(err).To(MatchError(`error parsing configuration list: invalid value "adsfasdfasf" for disableCheck`)) + _, err := libcni.LoadNetworkConf(configDir, "some-network") + Expect(err).To(MatchError(fmt.Sprintf("error parsing configuration list: invalid disableCheck value \"%s\"", badValue))) }) }) }) - Describe("ConfListFromFile", func() { + Describe("NetworkConfFromFile", func() { Context("when the file cannot be opened", func() { It("returns a useful error", func() { - _, err := libcni.ConfListFromFile("/tmp/nope/not-here") + _, err := libcni.NetworkConfFromFile("/tmp/nope/not-here") Expect(err).To(MatchError(HavePrefix(`error reading /tmp/nope/not-here: open /tmp/nope/not-here`))) }) }) }) Describe("InjectConf", func() { - var testNetConfig *libcni.NetworkConfig + var testNetConfig *libcni.PluginConfig BeforeEach(func() { - testNetConfig = &libcni.NetworkConfig{ - Network: &types.NetConf{Name: "some-plugin", Type: "foobar"}, + testNetConfig = &libcni.PluginConfig{ + Network: &types.PluginConf{Name: "some-plugin", Type: "foobar"}, Bytes: []byte(`{ "name": "some-plugin", "type": "foobar" }`), } }) Context("when function parameters are incorrect", func() { It("returns unmarshal error", func() { - conf := &libcni.NetworkConfig{ - Network: &types.NetConf{Name: "some-plugin"}, + conf := &libcni.PluginConfig{ + Network: &types.PluginConf{Name: "some-plugin"}, Bytes: []byte(`{ cc cc cc}`), } @@ -438,8 +438,8 @@ var _ = Describe("Loading configuration from disk", func() { resultConfig, err := libcni.InjectConf(testNetConfig, map[string]interface{}{"test": "test"}) Expect(err).NotTo(HaveOccurred()) - Expect(resultConfig).To(Equal(&libcni.NetworkConfig{ - Network: &types.NetConf{ + Expect(resultConfig).To(Equal(&libcni.PluginConfig{ + Network: &types.PluginConf{ Name: "some-plugin", Type: "foobar", }, @@ -456,8 +456,8 @@ var _ = Describe("Loading configuration from disk", func() { resultConfig, err = libcni.InjectConf(resultConfig, map[string]interface{}{"test": "changedValue"}) Expect(err).NotTo(HaveOccurred()) - Expect(resultConfig).To(Equal(&libcni.NetworkConfig{ - Network: &types.NetConf{ + Expect(resultConfig).To(Equal(&libcni.PluginConfig{ + Network: &types.PluginConf{ Name: "some-plugin", Type: "foobar", }, @@ -474,8 +474,8 @@ var _ = Describe("Loading configuration from disk", func() { resultConfig, err = libcni.InjectConf(resultConfig, map[string]interface{}{"test": "test"}) Expect(err).NotTo(HaveOccurred()) - Expect(resultConfig).To(Equal(&libcni.NetworkConfig{ - Network: &types.NetConf{ + Expect(resultConfig).To(Equal(&libcni.PluginConfig{ + Network: &types.PluginConf{ Name: "some-plugin", Type: "foobar", }, @@ -496,8 +496,8 @@ var _ = Describe("Loading configuration from disk", func() { resultConfig, err = libcni.InjectConf(resultConfig, map[string]interface{}{"type": "bridge"}) Expect(err).NotTo(HaveOccurred()) - Expect(resultConfig).To(Equal(&libcni.NetworkConfig{ - Network: &types.NetConf{Name: "some-plugin", Type: "bridge", DNS: types.DNS{Nameservers: servers, Domain: "local"}}, + Expect(resultConfig).To(Equal(&libcni.PluginConfig{ + Network: &types.PluginConf{Name: "some-plugin", Type: "bridge", DNS: types.DNS{Nameservers: servers, Domain: "local"}}, Bytes: expectedPluginConfig, })) }) @@ -505,7 +505,7 @@ var _ = Describe("Loading configuration from disk", func() { }) }) -var _ = Describe("ConfListFromBytes", func() { +var _ = Describe("NetworkConfFromBytes", func() { Describe("Version selection", func() { makeConfig := func(versions ...string) []byte { // ugly fake json encoding, but whatever @@ -516,36 +516,36 @@ var _ = Describe("ConfListFromBytes", func() { return []byte(fmt.Sprintf(`{"name": "test", "cniVersions": [%s], "plugins": [{"type": "foo"}]}`, strings.Join(vs, ","))) } It("correctly selects the maximum version", func() { - conf, err := libcni.ConfListFromBytes(makeConfig("1.1.0", "0.4.0", "1.0.0")) + conf, err := libcni.NetworkConfFromBytes(makeConfig("1.1.0", "0.4.0", "1.0.0")) Expect(err).NotTo(HaveOccurred()) Expect(conf.CNIVersion).To(Equal("1.1.0")) }) It("selects the highest version supported by libcni", func() { - conf, err := libcni.ConfListFromBytes(makeConfig("99.0.0", "1.1.0", "0.4.0", "1.0.0")) + conf, err := libcni.NetworkConfFromBytes(makeConfig("99.0.0", "1.1.0", "0.4.0", "1.0.0")) Expect(err).NotTo(HaveOccurred()) Expect(conf.CNIVersion).To(Equal("1.1.0")) }) It("fails when invalid versions are specified", func() { - _, err := libcni.ConfListFromBytes(makeConfig("1.1.0", "0.4.0", "1.0.f")) + _, err := libcni.NetworkConfFromBytes(makeConfig("1.1.0", "0.4.0", "1.0.f")) Expect(err).To(HaveOccurred()) }) It("falls back to cniVersion", func() { - conf, err := libcni.ConfListFromBytes([]byte(`{"name": "test", "cniVersion": "1.2.3", "plugins": [{"type": "foo"}]}`)) + conf, err := libcni.NetworkConfFromBytes([]byte(`{"name": "test", "cniVersion": "1.2.3", "plugins": [{"type": "foo"}]}`)) Expect(err).NotTo(HaveOccurred()) Expect(conf.CNIVersion).To(Equal("1.2.3")) }) It("merges cniVersions and cniVersion", func() { - conf, err := libcni.ConfListFromBytes([]byte(`{"name": "test", "cniVersion": "1.0.0", "cniVersions": ["0.1.0", "0.4.0"], "plugins": [{"type": "foo"}]}`)) + conf, err := libcni.NetworkConfFromBytes([]byte(`{"name": "test", "cniVersion": "1.0.0", "cniVersions": ["0.1.0", "0.4.0"], "plugins": [{"type": "foo"}]}`)) Expect(err).NotTo(HaveOccurred()) Expect(conf.CNIVersion).To(Equal("1.0.0")) }) It("handles an empty cniVersions array", func() { - conf, err := libcni.ConfListFromBytes([]byte(`{"name": "test", "cniVersions": [], "plugins": [{"type": "foo"}]}`)) + conf, err := libcni.NetworkConfFromBytes([]byte(`{"name": "test", "cniVersions": [], "plugins": [{"type": "foo"}]}`)) Expect(err).NotTo(HaveOccurred()) Expect(conf.CNIVersion).To(Equal("")) }) @@ -553,7 +553,7 @@ var _ = Describe("ConfListFromBytes", func() { }) var _ = Describe("ConfListFromConf", func() { - var testNetConfig *libcni.NetworkConfig + var testNetConfig *libcni.PluginConfig BeforeEach(func() { pb := []byte(`{"name":"some-plugin","cniVersion":"0.3.1", "type":"foobar"}`) @@ -575,11 +575,11 @@ var _ = Describe("ConfListFromConf", func() { Expect(ncl).To(Equal(&libcni.NetworkConfigList{ Name: "some-plugin", CNIVersion: "0.3.1", - Plugins: []*libcni.NetworkConfig{testNetConfig}, + Plugins: []*libcni.PluginConfig{testNetConfig}, })) // Test that the json unmarshals to the same data - ncl2, err := libcni.ConfListFromBytes(bytes) + ncl2, err := libcni.NetworkConfFromBytes(bytes) Expect(err).NotTo(HaveOccurred()) ncl2.Bytes = nil ncl2.Plugins[0].Bytes = nil diff --git a/pkg/types/types.go b/pkg/types/types.go index 8453bb5d..f8afaae7 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -56,8 +56,12 @@ func (n *IPNet) UnmarshalJSON(data []byte) error { return nil } -// NetConfType describes a network. -type NetConfType struct { +// Deprecated: Use PluginConf instead of NetConf, the NetConf +// backwards-compat alias will be removed in a future release. +type NetConf = PluginConf + +// PluginConf describes a plugin configuration for a specific network. +type PluginConf struct { CNIVersion string `json:"cniVersion,omitempty"` Name string `json:"name,omitempty"` @@ -73,9 +77,6 @@ type NetConfType struct { ValidAttachments []GCAttachment `json:"cni.dev/valid-attachments,omitempty"` } -// NetConf is defined as different type as custom MarshalJSON() and issue #1096 -type NetConf NetConfType - // GCAttachment is the parameters to a GC call -- namely, // the container ID and ifname pair that represents a // still-valid attachment. @@ -86,9 +87,9 @@ 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 *NetConfType) MarshalJSON() ([]byte, error) { +func (n *PluginConf) MarshalJSON() ([]byte, error) { // use type alias to escape recursion for json.Marshal() to MarshalJSON() - type fixObjType = NetConf + type fixObjType = PluginConf bytes, err := json.Marshal(fixObjType(*n)) if err != nil { @@ -120,10 +121,10 @@ func (i *IPAM) IsEmpty() bool { type NetConfList struct { CNIVersion string `json:"cniVersion,omitempty"` - Name string `json:"name,omitempty"` - DisableCheck bool `json:"disableCheck,omitempty"` + Name string `json:"name,omitempty"` + DisableCheck bool `json:"disableCheck,omitempty"` DisableGC bool `json:"disableGC,omitempty"` - Plugins []*NetConf `json:"plugins,omitempty"` + Plugins []*PluginConf `json:"plugins,omitempty"` } // Result is an interface that provides the result of plugin execution diff --git a/pkg/version/version.go b/pkg/version/version.go index a4d442c8..cfb6a12f 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -63,7 +63,7 @@ func NewResult(version string, resultBytes []byte) (types.Result, error) { // ParsePrevResult parses a prevResult in a NetConf structure and sets // the NetConf's PrevResult member to the parsed Result object. -func ParsePrevResult(conf *types.NetConf) error { +func ParsePrevResult(conf *types.PluginConf) error { if conf.RawPrevResult == nil { return nil } diff --git a/pkg/version/version_test.go b/pkg/version/version_test.go index 2ddf5eb4..a730b62b 100644 --- a/pkg/version/version_test.go +++ b/pkg/version/version_test.go @@ -59,7 +59,7 @@ var _ = Describe("Version operations", func() { err := json.Unmarshal(rawBytes, &raw) Expect(err).NotTo(HaveOccurred()) - conf := &types.NetConf{ + conf := &types.PluginConf{ CNIVersion: "1.0.0", Name: "foobar", Type: "baz", @@ -94,7 +94,7 @@ var _ = Describe("Version operations", func() { }) It("fails if the prevResult version is unknown", func() { - conf := &types.NetConf{ + conf := &types.PluginConf{ CNIVersion: version.Current(), Name: "foobar", Type: "baz", @@ -108,7 +108,7 @@ var _ = Describe("Version operations", func() { }) It("fails if the prevResult version does not match the prevResult version", func() { - conf := &types.NetConf{ + conf := &types.PluginConf{ CNIVersion: version.Current(), Name: "foobar", Type: "baz", @@ -128,7 +128,7 @@ var _ = Describe("Version operations", func() { Context("when a prevResult is not available", func() { It("does not fail", func() { - conf := &types.NetConf{ + conf := &types.PluginConf{ CNIVersion: version.Current(), Name: "foobar", Type: "baz", diff --git a/plugins/test/noop/main.go b/plugins/test/noop/main.go index ab424339..77496481 100644 --- a/plugins/test/noop/main.go +++ b/plugins/test/noop/main.go @@ -36,7 +36,7 @@ import ( ) type NetConf struct { - types.NetConf + types.PluginConf DebugFile string `json:"debugFile"` CommandLog string `json:"commandLog"` } @@ -46,7 +46,7 @@ func loadConf(bytes []byte) (*NetConf, error) { if err := json.Unmarshal(bytes, n); err != nil { return nil, fmt.Errorf("failed to load netconf: %w %q", err, string(bytes)) } - if err := version.ParsePrevResult(&n.NetConf); err != nil { + if err := version.ParsePrevResult(&n.PluginConf); err != nil { return nil, err } return n, nil From 425425837ef952146a7becdccec088e0db68a372 Mon Sep 17 00:00:00 2001 From: Benjamin Leggett Date: Mon, 8 Apr 2024 12:25:36 -0400 Subject: [PATCH 3/3] 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"` }