diff --git a/.changelog/23629.txt b/.changelog/23629.txt new file mode 100644 index 00000000000..8d17f4cdc68 --- /dev/null +++ b/.changelog/23629.txt @@ -0,0 +1,3 @@ +```release-note:bug +cni: .conf and .json config files are now parsed properly +``` diff --git a/client/allocrunner/networking_bridge_linux.go b/client/allocrunner/networking_bridge_linux.go index 0fca4c097fe..13e78920c37 100644 --- a/client/allocrunner/networking_bridge_linux.go +++ b/client/allocrunner/networking_bridge_linux.go @@ -72,7 +72,11 @@ func newBridgeNetworkConfigurator(log hclog.Logger, alloc *structs.Allocation, b netCfg = buildNomadBridgeNetConfig(*b, false) } - c, err := newCNINetworkConfiguratorWithConf(log, cniPath, bridgeNetworkAllocIfPrefix, ignorePortMappingHostIP, netCfg, node) + parser := &cniConfParser{ + listBytes: netCfg, + } + + c, err := newCNINetworkConfiguratorWithConf(log, cniPath, bridgeNetworkAllocIfPrefix, ignorePortMappingHostIP, parser, node) if err != nil { return nil, err } diff --git a/client/allocrunner/networking_cni.go b/client/allocrunner/networking_cni.go index f8f932b7bd0..87a5575097e 100644 --- a/client/allocrunner/networking_cni.go +++ b/client/allocrunner/networking_cni.go @@ -11,6 +11,7 @@ package allocrunner import ( "context" "encoding/json" + "errors" "fmt" "math/rand" "os" @@ -51,7 +52,7 @@ const ( type cniNetworkConfigurator struct { cni cni.CNI - cniConf []byte + confParser *cniConfParser ignorePortMappingHostIP bool nodeAttrs map[string]string nodeMeta map[string]string @@ -61,17 +62,17 @@ type cniNetworkConfigurator struct { } func newCNINetworkConfigurator(logger log.Logger, cniPath, cniInterfacePrefix, cniConfDir, networkName string, ignorePortMappingHostIP bool, node *structs.Node) (*cniNetworkConfigurator, error) { - cniConf, err := loadCNIConf(cniConfDir, networkName) + parser, err := loadCNIConf(cniConfDir, networkName) if err != nil { return nil, fmt.Errorf("failed to load CNI config: %v", err) } - return newCNINetworkConfiguratorWithConf(logger, cniPath, cniInterfacePrefix, ignorePortMappingHostIP, cniConf, node) + return newCNINetworkConfiguratorWithConf(logger, cniPath, cniInterfacePrefix, ignorePortMappingHostIP, parser, node) } -func newCNINetworkConfiguratorWithConf(logger log.Logger, cniPath, cniInterfacePrefix string, ignorePortMappingHostIP bool, cniConf []byte, node *structs.Node) (*cniNetworkConfigurator, error) { +func newCNINetworkConfiguratorWithConf(logger log.Logger, cniPath, cniInterfacePrefix string, ignorePortMappingHostIP bool, parser *cniConfParser, node *structs.Node) (*cniNetworkConfigurator, error) { conf := &cniNetworkConfigurator{ - cniConf: cniConf, + confParser: parser, rand: rand.New(rand.NewSource(time.Now().Unix())), logger: logger, ignorePortMappingHostIP: ignorePortMappingHostIP, @@ -118,7 +119,7 @@ func addCustomCNIArgs(networks []*structs.NetworkResource, cniArgs map[string]st // Setup calls the CNI plugins with the add action func (c *cniNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Allocation, spec *drivers.NetworkIsolationSpec) (*structs.AllocNetworkStatus, error) { if err := c.ensureCNIInitialized(); err != nil { - return nil, err + return nil, fmt.Errorf("cni not initialized: %w", err) } cniArgs := map[string]string{ // CNI plugins are called one after the other with the same set of @@ -444,7 +445,26 @@ func (c *cniNetworkConfigurator) cniToAllocNet(res *cni.Result) (*structs.AllocN return netStatus, nil } -func loadCNIConf(confDir, name string) ([]byte, error) { +// cniConfParser parses different config formats as appropriate +type cniConfParser struct { + listBytes []byte + confBytes []byte +} + +// getOpt produces a cni.Opt to load with cni.CNI.Load() +func (c *cniConfParser) getOpt() (cni.Opt, error) { + if len(c.listBytes) > 0 { + return cni.WithConfListBytes(c.listBytes), nil + } + if len(c.confBytes) > 0 { + return cni.WithConf(c.confBytes), nil + } + // theoretically should never be reached + return nil, errors.New("no CNI network config found") +} + +// loadCNIConf looks in confDir for a CNI config with the specified name +func loadCNIConf(confDir, name string) (*cniConfParser, error) { files, err := cnilibrary.ConfFiles(confDir, []string{".conf", ".conflist", ".json"}) switch { case err != nil: @@ -463,7 +483,9 @@ func loadCNIConf(confDir, name string) ([]byte, error) { return nil, fmt.Errorf("failed to load CNI config list file %s: %v", confFile, err) } if confList.Name == name { - return confList.Bytes, nil + return &cniConfParser{ + listBytes: confList.Bytes, + }, nil } } else { conf, err := cnilibrary.ConfFromFile(confFile) @@ -471,7 +493,9 @@ func loadCNIConf(confDir, name string) ([]byte, error) { return nil, fmt.Errorf("failed to load CNI config file %s: %v", confFile, err) } if conf.Network.Name == name { - return conf.Bytes, nil + return &cniConfParser{ + confBytes: conf.Bytes, + }, nil } } } @@ -585,11 +609,14 @@ func (c *cniNetworkConfigurator) forceCleanup(ipt IPTables, allocID string) erro } func (c *cniNetworkConfigurator) ensureCNIInitialized() error { - if err := c.cni.Status(); cni.IsCNINotInitialized(err) { - return c.cni.Load(cni.WithConfListBytes(c.cniConf)) - } else { + if err := c.cni.Status(); !cni.IsCNINotInitialized(err) { + return err + } + opt, err := c.confParser.getOpt() + if err != nil { return err } + return c.cni.Load(opt) } // nsOpts keeps track of NamespaceOpts usage, mainly for test assertions. diff --git a/client/allocrunner/networking_cni_test.go b/client/allocrunner/networking_cni_test.go index dda54c2cde1..16c83c0de97 100644 --- a/client/allocrunner/networking_cni_test.go +++ b/client/allocrunner/networking_cni_test.go @@ -10,6 +10,8 @@ import ( "errors" "math/rand" "net" + "os" + "path/filepath" "testing" "github.com/containerd/go-cni" @@ -25,6 +27,98 @@ import ( "github.com/stretchr/testify/require" ) +func TestLoadCNIConf_confParser(t *testing.T) { + confDir := t.TempDir() + + writeFile := func(t *testing.T, filename, content string) { + t.Helper() + path := filepath.Join(confDir, filename) + must.NoError(t, os.WriteFile(path, []byte(content), 0644)) + t.Cleanup(func() { + test.NoError(t, os.Remove(path)) + }) + } + + cases := []struct { + name, file, content string + expectErr string + }{ + { + name: "good-conflist", + file: "good.conflist", content: ` +{ + "cniVersion": "1.0.0", + "name": "good-conflist", + "plugins": [{ + "type": "cool-plugin" + }] +}`, + }, + { + name: "good-conf", + file: "good.conf", content: ` +{ + "cniVersion": "1.0.0", + "name": "good-conf", + "type": "cool-plugin" +}`, + }, + { + name: "good-json", + file: "good.json", content: ` +{ + "cniVersion": "1.0.0", + "name": "good-json", + "type": "cool-plugin" +}`, + }, + { + name: "no-config", + expectErr: "no CNI network config found in", + }, + { + name: "invalid-conflist", + file: "invalid.conflist", content: "{invalid}", + expectErr: "error parsing configuration list:", + }, + { + name: "invalid-conf", + file: "invalid.conf", content: "{invalid}", + expectErr: "error parsing configuration:", + }, + { + name: "invalid-json", + file: "invalid.json", content: "{invalid}", + expectErr: "error parsing configuration:", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if tc.file != "" && tc.content != "" { + writeFile(t, tc.file, tc.content) + } + + parser, err := loadCNIConf(confDir, tc.name) + if tc.expectErr != "" { + must.ErrorContains(t, err, tc.expectErr) + return + } + must.NoError(t, err) + opt, err := parser.getOpt() + must.NoError(t, err) + c, err := cni.New(opt) + must.NoError(t, err) + + config := c.GetConfig() + must.Len(t, 1, config.Networks, must.Sprint("expect 1 network in config")) + plugins := config.Networks[0].Config.Plugins + must.Len(t, 1, plugins, must.Sprint("expect 1 plugin in network")) + must.Eq(t, "cool-plugin", plugins[0].Network.Type) + }) + } +} + func TestSetup(t *testing.T) { ci.Parallel(t)