Skip to content

Commit

Permalink
Fixup as per comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
  • Loading branch information
@bleggett
bleggett committed Feb 1, 2024
1 parent 455c465 commit a33b3b3
Show file tree
Hide file tree
Showing 5 changed files with 370 additions and 89 deletions.
83 changes: 76 additions & 7 deletions SPEC.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ A network configuration consists of a JSON object with the following keys:
- `cniVersions` (string list): List of all CNI versions which this configuration supports. See [version selection](#version-selection) below.
- `name` (string): Network name. This should be unique across all network configurations on a host (or other administrative domain). Must start with an alphanumeric character, optionally followed by any combination of one or more alphanumeric characters, underscore, dot (.) or hyphen (-). Must not contain characters disallowed in file paths. A path segment (such as a filesystem directory) with the same name as the network name, containing one or more plugin configuration JSON objects for that network, should exist at the same level as the network configuration object itself.
- `disableCheck` (boolean): Either `true` or `false`. If `disableCheck` is `true`, runtimes must not call `CHECK` for this network configuration list. This allows an administrator to prevent `CHECK`ing where a combination of plugins is known to return spurious errors.
<!-- - `plugins` (list): A list of CNI plugins and their configuration, which is a list of plugin configuration objects. -->
- `loadPluginsFromFolder` (boolean): Either `true` or `false`. If `true`, indicates [plugin configuration objects](#plugin-configuration-objects) should be loaded from a sibling folder with the same name as the network `name` field. These sibling folders should exist at the same path as the network configuration itself. Any valid plugin configuration objects within the sibling folder will be appended to the final list of plugins for that network. If `plugins` is not present in the network configuration, `loadPluginsFromFolder` must be present, and set to true.
- `plugins` (list): A list of inlined [plugin configuration objects](#plugin-configuration-objects). If this key is populated with inlined plugin objects, and `loadPluginsFromFolder` is true, the final set of plugins for a network must consist of all the plugin objects in this list and the all the plugins loaded from the sibling folder with the same name as the network.

#### Plugin configuration objects:
All plugin configuration objects present in a directory with the same name as a valid network configuration must be parsed, and each plugin with a parsable configuration object must be invoked.
Expand Down Expand Up @@ -147,13 +148,14 @@ 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

Network configuration with no inlined plugin confs, and two loaded plugin confs:
`/etc/cni/net.d/10-dbnet.conf`:
```jsonc
{
"cniVersion": "1.1.0",
"cniVersions": ["0.3.1", "0.4.0", "1.0.0", "1.1.0"],
"name": "dbnet",
"loadPluginsFromFolder": true,
}
```

Expand All @@ -177,7 +179,7 @@ Plugins may define additional fields that they accept and may generate an error
"dns": {
"nameservers": [ "10.1.0.1" ]
}
},
}
```

`/etc/cni/net.d/dbnet/10-tuning.conf`:
Expand All @@ -190,16 +192,83 @@ Plugins may define additional fields that they accept and may generate an error
"sysctl": {
"net.core.somaxconn": "500"
}
},
}
```

`/etc/cni/net.d/dbnet/15-portmap.conf`:
Network configuration with one inlined plugin conf, and one loaded plugin conf:
`/etc/cni/net.d/10-dbnet.conf`:
```jsonc
{
"type": "portmap",
"capabilities": {"portMappings": true}
"cniVersion": "1.1.0",
"cniVersions": ["0.3.1", "0.4.0", "1.0.0", "1.1.0"],
"name": "dbnet",
"loadPluginsFromFolder": true,
plugins: [
{
"type": "bridge",
// plugin specific parameters
"bridge": "cni0",
"keyA": ["some more", "plugin specific", "configuration"],

"ipam": {
"type": "host-local",
// ipam specific
"subnet": "10.1.0.0/16",
"gateway": "10.1.0.1",
"routes": [
{"dst": "0.0.0.0/0"}
]
},
"dns": {
"nameservers": [ "10.1.0.1" ]
}
}
]
}
```

`/etc/cni/net.d/dbnet/10-tuning.conf`:
```jsonc
{
"type": "tuning",
"capabilities": {
"mac": true
},
"sysctl": {
"net.core.somaxconn": "500"
}
}
```

Network configuration with one inlined plugin conf, and no loaded plugin conf:
`/etc/cni/net.d/10-dbnet.conf`:
```jsonc
{
"cniVersion": "1.1.0",
"cniVersions": ["0.3.1", "0.4.0", "1.0.0", "1.1.0"],
"name": "dbnet",
"plugins": [
{
"type": "bridge",
// plugin specific parameters
"bridge": "cni0",
"keyA": ["some more", "plugin specific", "configuration"],

"ipam": {
"type": "host-local",
// ipam specific
"subnet": "10.1.0.0/16",
"gateway": "10.1.0.1",
"routes": [
{"dst": "0.0.0.0/0"}
]
},
"dns": {
"nameservers": [ "10.1.0.1" ]
}
}
]
}
```

### Version considerations
Expand Down
2 changes: 1 addition & 1 deletion cnitool/cnitool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,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)
}
Expand Down
11 changes: 6 additions & 5 deletions libcni/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,12 @@ type PluginConfig struct {
}

type NetworkConfigList struct {
Name string
CNIVersion string
DisableCheck bool
Plugins []*PluginConfig
Bytes []byte
Name string
CNIVersion string
DisableCheck bool
LoadPluginsFromFolder bool
Plugins []*PluginConfig
Bytes []byte
}

type NetworkAttachment struct {
Expand Down
115 changes: 69 additions & 46 deletions libcni/conf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,20 @@ func NetworkPluginConfFromBytes(pluginConfBytes []byte) (*PluginConfig, error) {
// 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) {
pluginConfFiles, err := ConfFiles(filepath.Join(networkConfPath, networkName), []string{".conf"})
var pConfs []*PluginConfig

pluginConfPath := filepath.Join(networkConfPath, networkName)

pluginConfFiles, err := ConfFiles(pluginConfPath, []string{".conf"})
switch {
case err != nil:
return nil, fmt.Errorf("failed to read plugin config file: %w", err)
return nil, fmt.Errorf("failed to read plugin config files in %s: %w", pluginConfPath, err)
case len(pluginConfFiles) == 0:
// Having 0 plugins for a given network is not necessarily a problem,
// but return as error for caller to decide.
return nil, fmt.Errorf("no plugin config found in %s: %w", networkConfPath, err)
// but return as error for caller to decide, since they tried to load
return nil, fmt.Errorf("no plugin config found in %s", pluginConfPath)
}

var pConfs []*PluginConfig
for _, pluginConfFile := range pluginConfFiles {
pluginConfBytes, err := os.ReadFile(pluginConfFile)
if err != nil {
Expand Down Expand Up @@ -176,15 +179,52 @@ func NetworkConfFromBytes(confBytes []byte) (*NetworkConfigList, error) {
}
}

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

list := &NetworkConfigList{
Name: name,
DisableCheck: disableCheck,
CNIVersion: cniVersion,
Bytes: confBytes,
Name: name,
DisableCheck: disableCheck,
LoadPluginsFromFolder: loadPluginsFromFolder,
CNIVersion: cniVersion,
Bytes: confBytes,
}

var plugins []interface{}
plug, ok := rawList["plugins"]
// We can have a `plugins` list key in the main conf,
// We can also have `loadPluginsFromFolder == true`
// They can both be present in the same config.
// But if one of them is NOT present/false, the other *must* be there.
if !ok && !loadPluginsFromFolder {
return nil, fmt.Errorf("error parsing configuration list: `loadPluginsFromFolder` not true, and no 'plugins' key")
} else if !ok && loadPluginsFromFolder {
return list, nil
}

if _, ok := rawList["plugins"]; ok {
return nil, fmt.Errorf("error parsing configuration list: no 'plugins' key allowed, cniVersion %s must load plugins from subdirectories", version.Current())
plugins, ok = plug.([]interface{})
if !ok {
return nil, fmt.Errorf("error parsing configuration list: invalid 'plugins' type %T", plug)
}
if len(plugins) == 0 {
return nil, fmt.Errorf("error parsing configuration list: no plugins in list")
}

for i, conf := range plugins {
newBytes, err := json.Marshal(conf)
if err != nil {
return nil, fmt.Errorf("failed to marshal plugin config %d: %w", i, err)
}
netConf, err := ConfFromBytes(newBytes)
if err != nil {
return nil, fmt.Errorf("failed to parse plugin config %d: %w", i, err)
}
list.Plugins = append(list.Plugins, netConf)
}
return list, nil
}
Expand All @@ -195,31 +235,27 @@ func NetworkConfFromFile(filename string) (*NetworkConfigList, error) {
return nil, fmt.Errorf("error reading %s: %w", filename, err)
}

// TODO
conf, err := NetworkConfFromBytes(bytes)
if err != nil {
return nil, err
}

plugins, err := NetworkPluginConfsFromFiles(filepath.Dir(filename), conf.Name)
if err != nil {
return nil, err
if conf.LoadPluginsFromFolder {
plugins, err := NetworkPluginConfsFromFiles(filepath.Dir(filename), conf.Name)
if err != nil {
return nil, err
}
conf.Plugins = append(conf.Plugins, plugins...)
}

conf.Plugins = plugins

return conf, nil
}

// Deprecated: This file format is no longer supported, use NetworkConfXXX and NetworkPluginXXX functions
func ConfFromBytes(bytes []byte) (*PluginConfig, error) {
func ConfFromBytes(bytes []byte) (*NetworkConfig, error) {
return NetworkPluginConfFromBytes(bytes)
}

// TODO Are we ok, at this point, cutting a new major version of libCNI and dropping the non-list CNI config?
//
// The non-list format was dropped in 1.0 and so I feel we should.
//
// Deprecated: This file format is no longer supported, use NetworkConfXXX and NetworkPluginXXX functions
func ConfFromFile(filename string) (*NetworkConfig, error) {
bytes, err := os.ReadFile(filename)
Expand Down Expand Up @@ -292,14 +328,23 @@ func LoadConf(dir, name string) (*NetworkConfig, error) {

// 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
}
sort.Strings(files)

for _, confFile := range files {
conf, err := ConfListFromFile(confFile)
conf, err := NetworkConfFromFile(confFile)
if err != nil {
return nil, err
}
Expand All @@ -308,7 +353,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 {
Expand All @@ -324,28 +369,6 @@ func LoadConfList(dir, name string) (*NetworkConfigList, error) {
return ConfListFromConf(singleConf)
}

// LoadNetworkConf looks at all the network configs in a given dir,
// loads and parses them all, and returns the first one that matches the provided
// network name predicate.
func LoadNetworkConf(dir, name string) (*NetworkConfigList, error) {
files, err := ConfFiles(dir, []string{".conf"})
if err != nil {
return nil, err
}
sort.Strings(files)

for _, confFile := range files {
conf, err := NetworkConfFromFile(confFile)
if err != nil {
return nil, err
}
if conf.Name == name {
return conf, nil
}
}
return nil, fmt.Errorf("no netconfig found in %q with name %s", dir, name)
}

// 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{})
Expand Down
Loading

0 comments on commit a33b3b3

Please sign in to comment.