k8sclient: use Go struct to parse annotations and add unit tests

k8sclient/k8sclient.go

@@ -15,7 +15,6 @@
 package k8sclient
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
 	"regexp"
@@ -120,44 +119,44 @@ func parsePodNetworkObjectName(podnetwork string) (string, string, string, error
 	return netNsName, networkName, netIfName, nil
 }
 
-func parsePodNetworkObject(podnetwork string) ([]map[string]interface{}, error) {
-	var podNet []map[string]interface{}
+func parsePodNetworkAnnotation(podNetworks, defaultNamespace string) ([]*types.NetworkSelectionElement, error) {
+	var networks []*types.NetworkSelectionElement
 
-	if podnetwork == "" {
-		return nil, fmt.Errorf("parsePodNetworkObject: pod annotation not having \"network\" as key, refer Multus README.md for the usage guide")
+	if podNetworks == "" {
+		return nil, fmt.Errorf("parsePodNetworkAnnotation: pod annotation not having \"network\" as key, refer Multus README.md for the usage guide")
 	}
 
-	// Parse the podnetwork string, and assume it is JSON.
-	if err := json.Unmarshal([]byte(podnetwork), &podNet); err != nil {
-
-		// If JSON doesn't parse, assume comma-delimited.
-		commaItems := strings.Split(podnetwork, ",")
-
-		// Build a map from the comma delimited items.
-		for i := range commaItems {
-
+	if strings.IndexAny(podNetworks, "[{\"") >= 0 {
+		if err := json.Unmarshal([]byte(podNetworks), &networks); err != nil {
+			return nil, fmt.Errorf("parsePodNetworkAnnotation: failed to parse pod Network Attachment Selection Annotation JSON format: %v", err)
+		}
+	} else {
+		// Comma-delimited list of network attachment object names
+		for _, item := range strings.Split(podNetworks, ",") {
 			// Remove leading and trailing whitespace.
-			commaItems[i] = strings.TrimSpace(commaItems[i])
+			item = strings.TrimSpace(item)
 
 			// Parse network name (i.e. <namespace>/<network name>@<ifname>)
-			netNsName, networkName, netIfName, err := parsePodNetworkObjectName(commaItems[i])
+			netNsName, networkName, netIfName, err := parsePodNetworkObjectName(item)
 			if err != nil {
-				return nil, fmt.Errorf("parsePodNetworkObject: %v", err)
-			}
-			m := make(map[string]interface{})
-			m["name"] = networkName
-			if netNsName != "" {
-				m["namespace"] = netNsName
-			}
-			if netIfName != "" {
-				m["interfaceRequest"] = netIfName
+				return nil, fmt.Errorf("parsePodNetworkAnnotation: %v", err)
 			}
 
-			podNet = append(podNet, m)
+			networks = append(networks, &types.NetworkSelectionElement{
+				Name:             networkName,
+				Namespace:        netNsName,
+				InterfaceRequest: netIfName,
+			})
 		}
 	}
 
-	return podNet, nil
+	for _, net := range networks {
+		if net.Namespace == "" {
+			net.Namespace = defaultNamespace
+		}
+	}
+
+	return networks, nil
 }
 
 func getCNIConfig(name string, ifname string, confdir string) (string, error) {
@@ -173,9 +172,9 @@ func getCNIConfig(name string, ifname string, confdir string) (string, error) {
 	files, err := libcni.ConfFiles(confdir, []string{".conf", ".json"})
 	switch {
 	case err != nil:
-		fmt.Errorf("No networks found in %s", confdir)
+		return "", fmt.Errorf("No networks found in %s", confdir)
 	case len(files) == 0:
-		fmt.Errorf("No networks found in %s", confdir)
+		return "", fmt.Errorf("No networks found in %s", confdir)
 	}
 
 	for _, confFile := range files {
@@ -260,101 +259,53 @@ func getNetSpec(ns types.NetworkSpec, name string, ifname string) (string, error
 
 }
 
-func getNetObject(net types.Network, ifname string, confdir string) (string, error) {
+func cniConfigFromNetworkResource(customResource *types.Network, net *types.NetworkSelectionElement, confdir string) (string, error) {
 	var config string
 	var err error
 
-	if (types.NetworkSpec{}) == net.Spec {
-		config, err = getCNIConfig(net.Metadata.Name, ifname, confdir)
+	if (types.NetworkSpec{}) == customResource.Spec {
+		// 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)
 		if err != nil {
-			return "", fmt.Errorf("getNetObject: err in getCNIConfig: %v", err)
+			return "", fmt.Errorf("cniConfigFromNetworkResource: err in getCNIConfig: %v", err)
 		}
 	} else {
-		config, err = getNetSpec(net.Spec, net.Metadata.Name, ifname)
+		// Generate delegate from CNI configuration embedded in the
+		// custom resource
+		config, err = getNetSpec(customResource.Spec, customResource.Metadata.Name, net.InterfaceRequest)
 		if err != nil {
-			return "", fmt.Errorf("getNetObject: err in getNetSpec: %v", err)
+			return "", fmt.Errorf("cniConfigFromNetworkResource: err in getNetSpec: %v", err)
 		}
 	}
 
 	return config, nil
 }
 
-func getnetplugin(client KubeClient, networkinfo map[string]interface{}, confdir, defaultNamespace string) (string, error) {
-	networkname := networkinfo["name"].(string)
-	if networkname == "" {
-		return "", fmt.Errorf("getnetplugin: network name can't be empty")
-	}
-
-	netNsName := defaultNamespace
-	if networkinfo["namespace"] != nil {
-		netNsName = networkinfo["namespace"].(string)
-	}
-
-	tprclient := fmt.Sprintf("/apis/kubernetes.cni.cncf.io/v1/namespaces/%s/networks/%s", netNsName, networkname)
-
-	netobjdata, err := client.GetRawWithPath(tprclient)
+func getKubernetesDelegate(client KubeClient, net *types.NetworkSelectionElement, confdir string) (*types.DelegateNetConf, error) {
+	rawPath := fmt.Sprintf("/apis/kubernetes.cni.cncf.io/v1/namespaces/%s/networks/%s", net.Namespace, net.Name)
+	netData, err := client.GetRawWithPath(rawPath)
 	if err != nil {
-		return "", fmt.Errorf("getnetplugin: failed to get CRD (result: %s), refer Multus README.md for the usage guide: %v", netobjdata, err)
+		return nil, fmt.Errorf("getKubernetesDelegate: failed to get network resource, refer Multus README.md for the usage guide: %v", err)
 	}
 
-	netobj := types.Network{}
-	if err := json.Unmarshal(netobjdata, &netobj); err != nil {
-		return "", fmt.Errorf("getnetplugin: failed to get the netplugin data: %v", err)
+	customResource := &types.Network{}
+	if err := json.Unmarshal(netData, customResource); err != nil {
+		return nil, fmt.Errorf("getKubernetesDelegate: failed to get the netplugin data: %v", err)
 	}
 
-	ifnameRequest := ""
-	if networkinfo["interfaceRequest"] != nil {
-		ifnameRequest = networkinfo["interfaceRequest"].(string)
-	}
-
-	netargs, err := getNetObject(netobj, ifnameRequest, confdir)
+	cniConfig, err := cniConfigFromNetworkResource(customResource, net, confdir)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
-	return netargs, nil
-}
-
-func getPodNetworkObj(client KubeClient, netObjs []map[string]interface{}, confdir, defaultNamespace string) (string, error) {
-
-	var np string
-	var err error
-
-	var str bytes.Buffer
-	str.WriteString("[")
-
-	for index, net := range netObjs {
-		np, err = getnetplugin(client, net, confdir, defaultNamespace)
+	delegate, err := types.LoadDelegateNetConf([]byte(cniConfig))
 	if err != nil {
-			return "", fmt.Errorf("getPodNetworkObj: failed in getting the netplugin: %v", err)
+		return nil, err
 	}
 
-		str.WriteString(np)
-		if index != (len(netObjs) - 1) {
-			str.WriteString(",")
-		}
-	}
-
-	str.WriteString("]")
-	netconf := str.String()
-	return netconf, nil
-}
-
-func getMultusDelegates(delegate string) ([]*types.DelegateNetConf, error) {
-	if delegate == "" {
-		return nil, fmt.Errorf("getMultusDelegates: TPR network obj data can't be empty")
-	}
-
-	n, err := types.LoadNetConf([]byte("{\"delegates\": " + delegate + "}"))
-	if err != nil {
-		return nil, fmt.Errorf("getMultusDelegates: failed to load netconf for delegate %v: %v", delegate, err)
-	}
-
-	if len(n.Delegates) == 0 {
-		return nil, fmt.Errorf(`getMultusDelegates: "delegates" is must, refer Multus README.md for the usage guide`)
-	}
-
-	return n.Delegates, nil
+	return delegate, nil
 }
 
 type KubeClient interface {
@@ -386,15 +337,20 @@ func GetK8sNetwork(args *skel.CmdArgs, kubeconfig string, k8sclient KubeClient,
 		return nil, &NoK8sNetworkError{"no kubernetes network found"}
 	}
 
-	netObjs, err := parsePodNetworkObject(netAnnot)
+	networks, err := parsePodNetworkAnnotation(netAnnot, defaultNamespace)
 	if err != nil {
 		return nil, err
 	}
 
-	multusDelegates, err := getPodNetworkObj(k8sclient, netObjs, confdir, defaultNamespace)
+	// Read all network objects referenced by 'networks'
+	var delegates []*types.DelegateNetConf
+	for _, net := range networks {
+		delegate, err := getKubernetesDelegate(k8sclient, net, confdir)
 		if err != nil {
-		return nil, err
+			return nil, fmt.Errorf("GetK8sNetwork: failed getting the delegate: %v", err)
+		}
+		delegates = append(delegates, delegate)
 	}
 
-	return getMultusDelegates(multusDelegates)
+	return delegates, nil
 }

k8sclient/k8sclient_test.go

@@ -0,0 +1,231 @@
+// Copyright (c) 2017 Intel Corporation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+package k8sclient
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	testutils "github.com/intel/multus-cni/testing"
+
+	"github.com/containernetworking/cni/pkg/skel"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestK8sClient(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "k8sclient")
+}
+
+var _ = Describe("k8sclient operations", func() {
+	var tmpDir string
+	var err error
+
+	BeforeEach(func() {
+		tmpDir, err = ioutil.TempDir("", "multus_tmp")
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	AfterEach(func() {
+		err := os.RemoveAll(tmpDir)
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("retrieves delegates from kubernetes using simple format annotation", func() {
+		fakePod := testutils.NewFakePod("testpod", "net1,net2")
+		net1 := `{
+	"name": "net1",
+	"type": "mynet",
+	"cniVersion": "0.2.0"
+}`
+		net2 := `{
+	"name": "net2",
+	"type": "mynet2",
+	"cniVersion": "0.2.0"
+}`
+		net3 := `{
+	"name": "net3",
+	"type": "mynet3",
+	"cniVersion": "0.2.0"
+}`
+		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", net1)
+		fKubeClient.AddNetConfig(fakePod.ObjectMeta.Namespace, "net2", net2)
+		// net3 is not used; make sure it's not accessed
+		fKubeClient.AddNetConfig(fakePod.ObjectMeta.Namespace, "net3", net3)
+
+		delegates, err := GetK8sNetwork(args, "", fKubeClient, tmpDir)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(fKubeClient.PodCount).To(Equal(1))
+		Expect(fKubeClient.NetCount).To(Equal(2))
+
+		Expect(len(delegates)).To(Equal(2))
+		Expect(delegates[0].Name).To(Equal("net1"))
+		Expect(delegates[0].Type).To(Equal("mynet"))
+		Expect(delegates[0].MasterPlugin).To(BeFalse())
+		Expect(delegates[1].Name).To(Equal("net2"))
+		Expect(delegates[1].Type).To(Equal("mynet2"))
+		Expect(delegates[1].MasterPlugin).To(BeFalse())
+	})
+
+	It("fails when the network does not exist", func() {
+		fakePod := testutils.NewFakePod("testpod", "net1,net2")
+		net3 := `{
+	"name": "net3",
+	"type": "mynet3",
+	"cniVersion": "0.2.0"
+}`
+		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, "net3", net3)
+
+		delegates, err := GetK8sNetwork(args, "", fKubeClient, tmpDir)
+		Expect(len(delegates)).To(Equal(0))
+		Expect(err).To(MatchError("GetK8sNetwork: failed getting the delegate: getKubernetesDelegate: failed to get network resource, refer Multus README.md for the usage guide: resource not found"))
+	})
+
+	It("retrieves delegates from kubernetes using JSON format annotation", func() {
+		fakePod := testutils.NewFakePod("testpod", `[
+{"name":"net1"},
+{
+  "name":"net2",
+  "ipRequest": "1.2.3.4",
+  "macRequest": "aa:bb:cc:dd:ee:ff"
+},
+{
+  "name":"net3",
+  "namespace":"other-ns"
+}
+]`)
+		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", `{
+	"name": "net1",
+	"type": "mynet",
+	"cniVersion": "0.2.0"
+}`)
+		fKubeClient.AddNetConfig(fakePod.ObjectMeta.Namespace, "net2", `{
+	"name": "net2",
+	"type": "mynet2",
+	"cniVersion": "0.2.0"
+}`)
+		fKubeClient.AddNetConfig("other-ns", "net3", `{
+	"name": "net3",
+	"type": "mynet3",
+	"cniVersion": "0.2.0"
+}`)
+
+		delegates, err := GetK8sNetwork(args, "", fKubeClient, tmpDir)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(fKubeClient.PodCount).To(Equal(1))
+		Expect(fKubeClient.NetCount).To(Equal(3))
+
+		Expect(len(delegates)).To(Equal(3))
+		Expect(delegates[0].Name).To(Equal("net1"))
+		Expect(delegates[0].Type).To(Equal("mynet"))
+		Expect(delegates[1].Name).To(Equal("net2"))
+		Expect(delegates[1].Type).To(Equal("mynet2"))
+		Expect(delegates[2].Name).To(Equal("net3"))
+		Expect(delegates[2].Type).To(Equal("mynet3"))
+	})
+
+	It("fails when the JSON format annotation is invalid", func() {
+		fakePod := testutils.NewFakePod("testpod", "[adsfasdfasdfasf]")
+		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)
+
+		delegates, err := GetK8sNetwork(args, "", fKubeClient, tmpDir)
+		Expect(len(delegates)).To(Equal(0))
+		Expect(err).To(MatchError("parsePodNetworkAnnotation: failed to parse pod Network Attachment Selection Annotation JSON format: invalid character 'a' looking for beginning of value"))
+	})
+
+	It("retrieves delegates from kubernetes using on-disk config files", func() {
+		fakePod := testutils.NewFakePod("testpod", "net1,net2")
+		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)
+		net1Name := filepath.Join(tmpDir, "10-net1.conf")
+		fKubeClient.AddNetFile(fakePod.ObjectMeta.Namespace, "net1", net1Name, `{
+	"name": "net1",
+	"type": "mynet",
+	"cniVersion": "0.2.0"
+}`)
+		net2Name := filepath.Join(tmpDir, "20-net2.conf")
+		fKubeClient.AddNetFile(fakePod.ObjectMeta.Namespace, "net2", net2Name, `{
+	"name": "net2",
+	"type": "mynet2",
+	"cniVersion": "0.2.0"
+}`)
+
+		delegates, err := GetK8sNetwork(args, "", fKubeClient, tmpDir)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(fKubeClient.PodCount).To(Equal(1))
+		Expect(fKubeClient.NetCount).To(Equal(2))
+
+		Expect(len(delegates)).To(Equal(2))
+		Expect(delegates[0].Name).To(Equal("net1"))
+		Expect(delegates[0].Type).To(Equal("mynet"))
+		Expect(delegates[1].Name).To(Equal("net2"))
+		Expect(delegates[1].Type).To(Equal("mynet2"))
+	})
+
+	It("fails when on-disk config file is not valid", func() {
+		fakePod := testutils.NewFakePod("testpod", "net1,net2")
+		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)
+		net1Name := filepath.Join(tmpDir, "10-net1.conf")
+		fKubeClient.AddNetFile(fakePod.ObjectMeta.Namespace, "net1", net1Name, `{
+	"name": "net1",
+	"type": "mynet",
+	"cniVersion": "0.2.0"
+}`)
+		net2Name := filepath.Join(tmpDir, "20-net2.conf")
+		fKubeClient.AddNetFile(fakePod.ObjectMeta.Namespace, "net2", net2Name, "asdfasdfasfdasfd")
+
+		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)))
+	})
+})

types/conf.go

@@ -28,17 +28,12 @@ const (
 	defaultConfDir = "/etc/cni/multus/net.d"
 )
 
-// Convert a raw delegate config map into a DelegateNetConf structure
-func loadDelegateNetConf(rawConf map[string]interface{}) (*DelegateNetConf, error) {
-	bytes, err := json.Marshal(rawConf)
-	if err != nil {
-		return nil, fmt.Errorf("error marshalling delegate config: %v", err)
-	}
+// Convert raw CNI JSON into a DelegateNetConf structure
+func LoadDelegateNetConf(bytes []byte) (*DelegateNetConf, error) {
 	delegateConf := &DelegateNetConf{}
-	if err = json.Unmarshal(bytes, delegateConf); err != nil {
+	if err := json.Unmarshal(bytes, delegateConf); err != nil {
 		return nil, fmt.Errorf("error unmarshalling delegate config: %v", err)
 	}
-	delegateConf.RawConfig = rawConf
 	delegateConf.Bytes = bytes
 
 	// Do some minimal validation
@@ -90,7 +85,11 @@ func LoadNetConf(bytes []byte) (*NetConf, error) {
 	}
 
 	for idx, rawConf := range netconf.RawDelegates {
-		delegateConf, err := loadDelegateNetConf(rawConf)
+		bytes, err := json.Marshal(rawConf)
+		if err != nil {
+			return nil, fmt.Errorf("error marshalling delegate %d config: %v", idx, err)
+		}
+		delegateConf, err := LoadDelegateNetConf(bytes)
 		if err != nil {
 			return nil, fmt.Errorf("failed to load delegate %d config: %v", idx, err)
 		}
@@ -104,21 +103,6 @@ func LoadNetConf(bytes []byte) (*NetConf, error) {
 	return netconf, nil
 }
 
-func (d *DelegateNetConf) updateRawConfig() error {
-	if d.IfnameRequest != "" {
-		d.RawConfig["ifnameRequest"] = d.IfnameRequest
-	} else {
-		delete(d.RawConfig, "ifnameRequest")
-	}
-
-	bytes, err := json.Marshal(d.RawConfig)
-	if err != nil {
-		return err
-	}
-	d.Bytes = bytes
-	return nil
-}
-
 // AddDelegates appends the new delegates to the delegates list
 func (n *NetConf) AddDelegates(newDelegates []*DelegateNetConf) error {
 	n.Delegates = append(n.Delegates, newDelegates...)

types/types.go

@@ -46,9 +46,7 @@ type DelegateNetConf struct {
 	// MasterPlugin is only used internal housekeeping
 	MasterPlugin bool `json:"-"`
 
-	// Raw unmarshalled JSON
-	RawConfig map[string]interface{}
-	// Raw bytes
+	// Raw JSON
 	Bytes []byte
 }
 
@@ -88,6 +86,26 @@ type NetworkSpec struct {
 	Plugin string `json:"plugin"`
 }
 
+// NetworkSelectionElement represents one element of the JSON format
+// Network Attachment Selection Annotation as described in section 4.1.2
+// of the CRD specification.
+type NetworkSelectionElement struct {
+	// Name contains the name of the Network object this element selects
+	Name string `json:"name"`
+	// Namespace contains the optional namespace that the network referenced
+	// by Name exists in
+	Namespace string `json:"namespace,omitempty"`
+	// IPRequest contains an optional requested IP address for this network
+	// attachment
+	IPRequest string `json:"ipRequest,omitempty"`
+	// MacRequest contains an optional requested MAC address for this
+	// network attachment
+	MacRequest string `json:"macRequest,omitempty"`
+	// InterfaceRequest contains an optional requested name for the
+	// network interface this attachment will create in the container
+	InterfaceRequest string `json:"interfaceRequest,omitempty"`
+}
+
 // K8sArgs is the valid CNI_ARGS used for Kubernetes
 type K8sArgs struct {
 	types.CommonArgs