diff --git a/k8sclient/k8sclient.go b/k8sclient/k8sclient.go index 345aa6a6e..bcbf3e9f0 100644 --- a/k8sclient/k8sclient.go +++ b/k8sclient/k8sclient.go @@ -159,7 +159,7 @@ func parsePodNetworkAnnotation(podNetworks, defaultNamespace string) ([]*types.N return networks, nil } -func getCNIConfig(name string, ifname string, confdir string) (string, error) { +func getCNIConfigFromFile(name string, confdir string) ([]byte, error) { // In the absence of valid keys in a Spec, the runtime (or // meta-plugin) should load and execute a CNI .configlist @@ -172,111 +172,74 @@ func getCNIConfig(name string, ifname string, confdir string) (string, error) { files, err := libcni.ConfFiles(confdir, []string{".conf", ".json"}) switch { case err != nil: - return "", fmt.Errorf("No networks found in %s", confdir) + return nil, fmt.Errorf("No networks found in %s", confdir) case len(files) == 0: - return "", fmt.Errorf("No networks found in %s", confdir) + return nil, fmt.Errorf("No networks found in %s", confdir) } for _, confFile := range files { conf, err := libcni.ConfFromFile(confFile) if err != nil { - return "", fmt.Errorf("Error loading CNI config file %s: %v", confFile, err) + return nil, fmt.Errorf("Error loading CNI config file %s: %v", confFile, err) } if conf.Network.Name == name { // Ensure the config has a "type" so we know what plugin to run. // Also catches the case where somebody put a conflist into a conf file. if conf.Network.Type == "" { - return "", fmt.Errorf("Error loading CNI config file %s: no 'type'; perhaps this is a .conflist?", confFile) + return nil, fmt.Errorf("Error loading CNI config file %s: no 'type'; perhaps this is a .conflist?", confFile) } - - return getConfig(string(conf.Bytes[:]), ifname), nil - + return conf.Bytes, nil } } - return "", fmt.Errorf("no network available in the name %s in cni dir %s", name, confdir) + return nil, fmt.Errorf("no network available in the name %s in cni dir %s", name, confdir) } -func getPlugin(plugin string, name string, ifname string) string { - tmpconfig := []string{} - - tmpconfig = append(tmpconfig, fmt.Sprintf(`{"cniVersion": "0.3.1" , "name": "%s", "type": "%s"`, name, plugin)) - - if ifname != "" { - tmpconfig = append(tmpconfig, fmt.Sprintf(`, "ifnameRequest": "%s"`, ifname)) - } - - tmpconfig = append(tmpconfig, "}") - - return strings.Join(tmpconfig, "") - -} - -func getConfig(config string, ifname string) string { - tmpconfig := []string{} - - config = strings.TrimSpace(config) - tmpconfig = append(tmpconfig, config[:1]) - - if ifname != "" { - tmpconfig = append(tmpconfig, fmt.Sprintf(` "ifnameRequest": "%s",`, ifname)) - } - - tmpconfig = append(tmpconfig, config[1:]) - - return strings.Join(tmpconfig, "") - -} - -func getNetSpec(ns types.NetworkAttachmentDefinitionSpec, name string, ifname string) (string, error) { - - if ns.Plugin == "" && ns.Config == "" { - return "", fmt.Errorf("Network Object spec plugin and config can't be empty") - } - - if ns.Plugin != "" && ns.Config != "" { - return "", fmt.Errorf("Network Object spec can't have both plugin and config") - } - - if ns.Plugin != "" { - // Plugin contains the name of a CNI plugin on-disk in a - // runtime-defined path (eg /opt/cni/bin and/or other paths. - // This plugin should be executed with a basic CNI JSON - // configuration on stdin containing the Network object - // name and the plugin: - // { “cniVersion”: “0.3.1”, “type”: , “name”: } - // and any additional “runtimeConfig” field per the - // CNI specification and conventions. - return getPlugin(ns.Plugin, name, ifname), nil - } - - // Config contains a standard JSON-encoded CNI configuration - // or configuration list which defines the plugin chain to - // execute. If present, this key takes precedence over - // ‘Plugin’. - return getConfig(ns.Config, ifname), nil - -} - -func cniConfigFromNetworkResource(customResource *types.NetworkAttachmentDefinition, net *types.NetworkSelectionElement, confdir string) (string, error) { - var config string +// getCNIConfigFromSpec reads a CNI JSON configuration from the NetworkAttachmentDefinition +// object's Spec.Config field and fills in any missing details like the network name +func getCNIConfigFromSpec(configData, netName string) ([]byte, error) { + var rawConfig map[string]interface{} var err error - if (types.NetworkAttachmentDefinitionSpec{}) == customResource.Spec { + configBytes := []byte(configData) + err = json.Unmarshal(configBytes, &rawConfig) + if err != nil { + return nil, fmt.Errorf("getCNIConfigFromSpec: failed to unmarshal Spec.Config: %v", err) + } + + // Inject network name if missing from Config for the thick plugin case + if n, ok := rawConfig["name"]; !ok || n == "" { + rawConfig["name"] = netName + configBytes, err = json.Marshal(rawConfig) + if err != nil { + return nil, fmt.Errorf("getCNIConfigFromSpec: failed to re-marshal Spec.Config: %v", err) + } + } + + return configBytes, nil +} + +func cniConfigFromNetworkResource(customResource *types.NetworkAttachmentDefinition, confdir string) ([]byte, error) { + var config []byte + var err error + + emptySpec := types.NetworkAttachmentDefinitionSpec{} + if (customResource.Spec == emptySpec) { // Network Spec empty; generate delegate from CNI JSON config // from the configuration directory that has the same network // name as the custom resource - config, err = getCNIConfig(customResource.Metadata.Name, net.InterfaceRequest, confdir) + config, err = getCNIConfigFromFile(customResource.Metadata.Name, confdir) if err != nil { - return "", fmt.Errorf("cniConfigFromNetworkResource: err in getCNIConfig: %v", err) + return nil, fmt.Errorf("cniConfigFromNetworkResource: err in getCNIConfigFromFile: %v", err) } } else { - // Generate delegate from CNI configuration embedded in the - // custom resource - config, err = getNetSpec(customResource.Spec, customResource.Metadata.Name, net.InterfaceRequest) + // Config contains a standard JSON-encoded CNI configuration + // or configuration list which defines the plugin chain to + // execute. + config, err = getCNIConfigFromSpec(customResource.Spec.Config, customResource.Metadata.Name) if err != nil { - return "", fmt.Errorf("cniConfigFromNetworkResource: err in getNetSpec: %v", err) + return nil, fmt.Errorf("cniConfigFromNetworkResource: err in getCNIConfigFromSpec: %v", err) } } @@ -295,12 +258,12 @@ func getKubernetesDelegate(client KubeClient, net *types.NetworkSelectionElement return nil, fmt.Errorf("getKubernetesDelegate: failed to get the netplugin data: %v", err) } - cniConfig, err := cniConfigFromNetworkResource(customResource, net, confdir) + configBytes, err := cniConfigFromNetworkResource(customResource, confdir) if err != nil { return nil, err } - delegate, err := types.LoadDelegateNetConf([]byte(cniConfig)) + delegate, err := types.LoadDelegateNetConf(configBytes, net.InterfaceRequest) if err != nil { return nil, err } diff --git a/k8sclient/k8sclient_test.go b/k8sclient/k8sclient_test.go index 00438ab55..d7a390710 100644 --- a/k8sclient/k8sclient_test.go +++ b/k8sclient/k8sclient_test.go @@ -207,6 +207,26 @@ var _ = Describe("k8sclient operations", func() { Expect(delegates[1].Type).To(Equal("mynet2")) }) + It("injects network name into minimal thick plugin CNI config", func() { + fakePod := testutils.NewFakePod("testpod", "net1") + args := &skel.CmdArgs{ + Args: fmt.Sprintf("K8S_POD_NAME=%s;K8S_POD_NAMESPACE=%s", fakePod.ObjectMeta.Name, fakePod.ObjectMeta.Namespace), + } + + fKubeClient := testutils.NewFakeKubeClient() + fKubeClient.AddPod(fakePod) + fKubeClient.AddNetConfig(fakePod.ObjectMeta.Namespace, "net1", "{\"type\": \"mynet\"}") + + delegates, err := GetK8sNetwork(args, "", fKubeClient, tmpDir) + Expect(err).NotTo(HaveOccurred()) + Expect(fKubeClient.PodCount).To(Equal(1)) + Expect(fKubeClient.NetCount).To(Equal(1)) + + Expect(len(delegates)).To(Equal(1)) + Expect(delegates[0].Name).To(Equal("net1")) + Expect(delegates[0].Type).To(Equal("mynet")) + }) + It("fails when on-disk config file is not valid", func() { fakePod := testutils.NewFakePod("testpod", "net1,net2") args := &skel.CmdArgs{ @@ -226,6 +246,6 @@ var _ = Describe("k8sclient operations", func() { delegates, err := GetK8sNetwork(args, "", fKubeClient, tmpDir) Expect(len(delegates)).To(Equal(0)) - Expect(err).To(MatchError(fmt.Sprintf("GetK8sNetwork: failed getting the delegate: cniConfigFromNetworkResource: err in getCNIConfig: Error loading CNI config file %s: error parsing configuration: invalid character 'a' looking for beginning of value", net2Name))) + Expect(err).To(MatchError(fmt.Sprintf("GetK8sNetwork: failed getting the delegate: cniConfigFromNetworkResource: err in getCNIConfigFromFile: Error loading CNI config file %s: error parsing configuration: invalid character 'a' looking for beginning of value", net2Name))) }) }) diff --git a/types/conf.go b/types/conf.go index 92861f168..6f869a125 100644 --- a/types/conf.go +++ b/types/conf.go @@ -30,7 +30,7 @@ const ( ) // Convert raw CNI JSON into a DelegateNetConf structure -func LoadDelegateNetConf(bytes []byte) (*DelegateNetConf, error) { +func LoadDelegateNetConf(bytes []byte, ifnameRequest string) (*DelegateNetConf, error) { delegateConf := &DelegateNetConf{} if err := json.Unmarshal(bytes, delegateConf); err != nil { return nil, fmt.Errorf("error unmarshalling delegate config: %v", err) @@ -42,6 +42,10 @@ func LoadDelegateNetConf(bytes []byte) (*DelegateNetConf, error) { return nil, fmt.Errorf("delegate must have the 'type' field") } + if ifnameRequest != "" { + delegateConf.IfnameRequest = ifnameRequest + } + return delegateConf, nil } @@ -98,7 +102,7 @@ func LoadNetConf(bytes []byte) (*NetConf, error) { if err != nil { return nil, fmt.Errorf("error marshalling delegate %d config: %v", idx, err) } - delegateConf, err := LoadDelegateNetConf(bytes) + delegateConf, err := LoadDelegateNetConf(bytes, "") if err != nil { return nil, fmt.Errorf("failed to load delegate %d config: %v", idx, err) } diff --git a/types/types.go b/types/types.go index 2e65e95d7..9af07c8c0 100644 --- a/types/types.go +++ b/types/types.go @@ -75,17 +75,6 @@ type NetworkAttachmentDefinitionSpec struct { // ‘Plugin’. // +optional Config string `json:"config"` - - // Plugin contains the name of a CNI plugin on-disk in a - // runtime-defined path (eg /opt/cni/bin and/or other paths. - // This plugin should be executed with a basic CNI JSON - // configuration on stdin containing the Network object - // name and the plugin: - // { “cniVersion”: “0.3.1”, “type”: , “name”: } - // and any additional “runtimeConfig” field per the - // CNI specification and conventions. - // +optional - Plugin string `json:"plugin"` } // NetworkSelectionElement represents one element of the JSON format