Merge pull request #335 from caseydavenport/fix-multi-rules

Fix bug with multiple rules in Kubernetes datastore driver

Author: caseydavenport

lib/backend/k8s/conversion.go

@@ -32,6 +32,7 @@ import (
 	kapiv1 "k8s.io/client-go/pkg/api/v1"
 	extensions "k8s.io/client-go/pkg/apis/extensions/v1beta1"
 	metav1 "k8s.io/client-go/pkg/apis/meta/v1"
+	"k8s.io/client-go/pkg/util/intstr"
 )
 
 var (
@@ -322,10 +323,26 @@ func (c converter) k8sIngressRuleToCalico(r extensions.NetworkPolicyIngressRule,
 
 	// Built up a list of the sources and a list of the destinations.
 	for _, f := range r.From {
-		peers = append(peers, &f)
+		// We need to add a copy of the peer so all the rules don't
+		// point to the same location.
+		peers = append(peers, &extensions.NetworkPolicyPeer{
+			NamespaceSelector: f.NamespaceSelector,
+			PodSelector:       f.PodSelector,
+		})
 	}
 	for _, p := range r.Ports {
-		ports = append(ports, &p)
+		// We need to add a copy of the port so all the rules don't
+		// point to the same location.
+		port := extensions.NetworkPolicyPort{}
+		if p.Port != nil {
+			portval := intstr.FromString(p.Port.String())
+			port.Port = &portval
+		}
+		if p.Protocol != nil {
+			protval := kapiv1.Protocol(fmt.Sprintf("%s", *p.Protocol))
+			port.Protocol = &protval
+		}
+		ports = append(ports, &port)
 	}
 
 	// If there no peers, or no ports, represent that as nil.
@@ -337,6 +354,8 @@ func (c converter) k8sIngressRuleToCalico(r extensions.NetworkPolicyIngressRule,
 	}
 
 	// Combine desintations with sources to generate rules.
+	// TODO: This currently creates a lot of rules by making every combination of from / ports
+	// into a rule.  We can combine these so that we don't need as many rules!
 	for _, port := range ports {
 		for _, peer := range peers {
 			// Build rule and append to list.

lib/backend/k8s/conversion_test.go

@@ -19,6 +19,8 @@ import (
 	. "github.com/onsi/gomega"
 
 	"github.com/projectcalico/libcalico-go/lib/backend/model"
+	"github.com/projectcalico/libcalico-go/lib/numorstring"
+
 	k8sapi "k8s.io/client-go/pkg/api/v1"
 	extensions "k8s.io/client-go/pkg/apis/extensions/v1beta1"
 	metav1 "k8s.io/client-go/pkg/apis/meta/v1"
@@ -267,6 +269,145 @@ var _ = Describe("Test NetworkPolicy conversion", func() {
 		Expect(len(pol.Value.(*model.Policy).OutboundRules)).To(Equal(0))
 	})
 
+	It("should parse a NetworkPolicy with multiple peers", func() {
+		np := extensions.NetworkPolicy{
+			ObjectMeta: k8sapi.ObjectMeta{
+				Name:      "testPolicy",
+				Namespace: "default",
+			},
+			Spec: extensions.NetworkPolicySpec{
+				PodSelector: metav1.LabelSelector{
+					MatchLabels: map[string]string{"label": "value"},
+				},
+				Ingress: []extensions.NetworkPolicyIngressRule{
+					extensions.NetworkPolicyIngressRule{
+						Ports: []extensions.NetworkPolicyPort{
+							extensions.NetworkPolicyPort{},
+						},
+						From: []extensions.NetworkPolicyPeer{
+							extensions.NetworkPolicyPeer{
+								PodSelector: &metav1.LabelSelector{
+									MatchLabels: map[string]string{
+										"k": "v",
+									},
+								},
+							},
+							extensions.NetworkPolicyPeer{
+								PodSelector: &metav1.LabelSelector{
+									MatchLabels: map[string]string{
+										"k2": "v2",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+
+		var pol *model.KVPair
+		var err error
+		By("parsing the policy", func() {
+			pol, err = c.networkPolicyToPolicy(&np)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(pol.Key.(model.PolicyKey).Name).To(Equal("np.projectcalico.org/default.testPolicy"))
+			Expect(int(*pol.Value.(*model.Policy).Order)).To(Equal(1000))
+		})
+
+		By("having the correct endpoint selector", func() {
+			Expect(pol.Value.(*model.Policy).Selector).To(Equal("calico/k8s_ns == 'default' && label == 'value'"))
+		})
+
+		By("having the correct peer selectors", func() {
+			Expect(len(pol.Value.(*model.Policy).InboundRules)).To(Equal(2))
+			Expect(len(pol.Value.(*model.Policy).OutboundRules)).To(Equal(0))
+			Expect(pol.Value.(*model.Policy).InboundRules[0].SrcSelector).To(Equal("calico/k8s_ns == 'default' && k == 'v'"))
+			Expect(pol.Value.(*model.Policy).InboundRules[1].SrcSelector).To(Equal("calico/k8s_ns == 'default' && k2 == 'v2'"))
+		})
+	})
+
+	It("should parse a NetworkPolicy with multiple peers and ports", func() {
+		tcp := k8sapi.ProtocolTCP
+		udp := k8sapi.ProtocolUDP
+		eighty := intstr.FromInt(80)
+		ninety := intstr.FromInt(90)
+
+		np := extensions.NetworkPolicy{
+			ObjectMeta: k8sapi.ObjectMeta{
+				Name:      "testPolicy",
+				Namespace: "default",
+			},
+			Spec: extensions.NetworkPolicySpec{
+				PodSelector: metav1.LabelSelector{
+					MatchLabels: map[string]string{"label": "value"},
+				},
+				Ingress: []extensions.NetworkPolicyIngressRule{
+					extensions.NetworkPolicyIngressRule{
+						Ports: []extensions.NetworkPolicyPort{
+							extensions.NetworkPolicyPort{
+								Port:     &ninety,
+								Protocol: &udp,
+							},
+							extensions.NetworkPolicyPort{
+								Port:     &eighty,
+								Protocol: &tcp,
+							},
+						},
+						From: []extensions.NetworkPolicyPeer{
+							extensions.NetworkPolicyPeer{
+								PodSelector: &metav1.LabelSelector{
+									MatchLabels: map[string]string{
+										"k": "v",
+									},
+								},
+							},
+							extensions.NetworkPolicyPeer{
+								PodSelector: &metav1.LabelSelector{
+									MatchLabels: map[string]string{
+										"k2": "v2",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+
+		var pol *model.KVPair
+		var err error
+		By("parsing the policy", func() {
+			pol, err = c.networkPolicyToPolicy(&np)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(pol.Key.(model.PolicyKey).Name).To(Equal("np.projectcalico.org/default.testPolicy"))
+			Expect(int(*pol.Value.(*model.Policy).Order)).To(Equal(1000))
+		})
+
+		By("having the correct endpoint selector", func() {
+			Expect(pol.Value.(*model.Policy).Selector).To(Equal("calico/k8s_ns == 'default' && label == 'value'"))
+		})
+
+		By("having the correct peer selectors", func() {
+			eighty, _ := numorstring.PortFromString("80")
+			ninety, _ := numorstring.PortFromString("90")
+			Expect(len(pol.Value.(*model.Policy).InboundRules)).To(Equal(4))
+			Expect(len(pol.Value.(*model.Policy).OutboundRules)).To(Equal(0))
+
+			Expect(pol.Value.(*model.Policy).InboundRules[0].SrcSelector).To(Equal("calico/k8s_ns == 'default' && k == 'v'"))
+			Expect(pol.Value.(*model.Policy).InboundRules[0].DstPorts).To(Equal([]numorstring.Port{ninety}))
+
+			Expect(pol.Value.(*model.Policy).InboundRules[1].SrcSelector).To(Equal("calico/k8s_ns == 'default' && k2 == 'v2'"))
+			Expect(pol.Value.(*model.Policy).InboundRules[1].DstPorts).To(Equal([]numorstring.Port{ninety}))
+
+			Expect(pol.Value.(*model.Policy).InboundRules[2].SrcSelector).To(Equal("calico/k8s_ns == 'default' && k == 'v'"))
+			Expect(pol.Value.(*model.Policy).InboundRules[2].DstPorts).To(Equal([]numorstring.Port{eighty}))
+
+			Expect(pol.Value.(*model.Policy).InboundRules[3].SrcSelector).To(Equal("calico/k8s_ns == 'default' && k2 == 'v2'"))
+			Expect(pol.Value.(*model.Policy).InboundRules[3].DstPorts).To(Equal([]numorstring.Port{eighty}))
+
+		})
+	})
+
 	It("should parse a NetworkPolicy with empty podSelector", func() {
 		np := extensions.NetworkPolicy{
 			ObjectMeta: k8sapi.ObjectMeta{