From 5d96e63f20b41d57f88e3b02347b84152e383c19 Mon Sep 17 00:00:00 2001 From: Benjamin Leggett Date: Mon, 8 Apr 2024 12:21:12 -0400 Subject: [PATCH] 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 | 106 ++++++++++++++++++------------------ pkg/types/types.go | 18 +++--- pkg/version/version.go | 2 +- pkg/version/version_test.go | 8 +-- plugins/test/noop/main.go | 4 +- 9 files changed, 170 insertions(+), 122 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 3cc887dd..9ae02e1f 100644 --- a/libcni/api.go +++ b/libcni/api.go @@ -67,17 +67,22 @@ 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 - Plugins []*NetworkConfig - Bytes []byte + Name string + CNIVersion string + DisableCheck bool + LoadOnlyInlinedPlugins bool + Plugins []*PluginConfig + Bytes []byte } type NetworkAttachment struct { @@ -101,14 +106,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 @@ -146,7 +151,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{}{ @@ -182,7 +187,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{}) @@ -403,7 +408,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) } @@ -415,7 +420,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) } @@ -478,7 +483,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 { @@ -520,7 +525,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 { @@ -562,7 +567,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 { @@ -605,7 +610,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 "" } @@ -619,7 +624,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 @@ -633,7 +638,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 @@ -649,7 +654,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 @@ -709,7 +714,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 { @@ -827,7 +832,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 { @@ -862,7 +867,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 33254633..638e8b89 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 @@ -1625,7 +1625,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 ) @@ -1798,7 +1798,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 @@ -1920,14 +1920,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 6c5d99de..5e724530 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) } @@ -153,7 +152,7 @@ func ConfListFromBytes(bytes []byte) (*NetworkConfigList, error) { Name: name, DisableCheck: disableCheck, CNIVersion: cniVersion, - Bytes: bytes, + Bytes: confBytes, } var plugins []interface{} @@ -184,7 +183,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) @@ -192,6 +191,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) @@ -218,6 +243,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 { @@ -240,7 +266,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 @@ -248,7 +284,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 } @@ -257,7 +293,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 { @@ -273,7 +309,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 { @@ -297,12 +334,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 ea7104f2..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") + _, 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 193ac46e..ccdf2247 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -56,8 +56,12 @@ func (n *IPNet) UnmarshalJSON(data []byte) error { return nil } -// NetConf describes a network. -type NetConf 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"` @@ -83,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 *NetConf) 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)) //nolint:all if err != nil { @@ -117,9 +121,9 @@ func (i *IPAM) IsEmpty() bool { type NetConfList struct { CNIVersion string `json:"cniVersion,omitempty"` - Name string `json:"name,omitempty"` - DisableCheck bool `json:"disableCheck,omitempty"` - Plugins []*NetConf `json:"plugins,omitempty"` + Name string `json:"name,omitempty"` + DisableCheck bool `json:"disableCheck,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