Skip to content

Commit

Permalink
Merge pull request projectcalico#1135 from monzo/convert-ports
Browse files Browse the repository at this point in the history 
Combine together ports of the same protocol
  • Loading branch information
@caseydavenport
caseydavenport committed Oct 4, 2019
2 parents 368282f + 3912f3d commit 6ccf338
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 47 deletions.
36 changes: 33 additions & 3 deletions lib/backend/k8s/conversion/conversion.go
View file
Expand Up @@ -535,15 +535,45 @@ func (c Converter) k8sRuleToCalico(rPeers []networkingv1.NetworkPolicyPeer, rPor
ports = []*networkingv1.NetworkPolicyPort{nil}
}

// Combine destinations 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!
protocolPorts := map[string][]numorstring.Port{}

for _, port := range ports {
protocol, calicoPorts, err := c.k8sPortToCalicoFields(port)
if err != nil {
return nil, fmt.Errorf("failed to parse k8s port: %s", err)
}

// These are either both present or both nil
if protocol == nil && calicoPorts == nil {
// If nil, no ports were specified, or an empty port struct was provided, which we translate to allowing all.
// We want to use a nil protocol and a nil list of ports, which will allow any destination (for ingress).
// Given we're gonna allow all, we may as well break here and keep only this rule
protocolPorts = map[string][]numorstring.Port{"": nil}
break
}

pStr := protocol.String()
protocolPorts[pStr] = append(protocolPorts[pStr], calicoPorts...)
}

protocols := make([]string, 0, len(protocolPorts))
for k := range protocolPorts {
protocols = append(protocols, k)
}
// Ensure deterministic output
sort.Strings(protocols)

// Combine destinations with sources to generate rules. We generate one rule per protocol,
// with each rule containing all the allowed ports.
for _, protocolStr := range protocols {
calicoPorts := protocolPorts[protocolStr]

var protocol *numorstring.Protocol
if protocolStr != "" {
p := numorstring.ProtocolFromString(protocolStr)
protocol = &p
}

for _, peer := range peers {
selector, nsSelector, nets, notNets := c.k8sPeerToCalicoFields(peer, ns)
if ingress {
Expand Down
154 changes: 110 additions & 44 deletions lib/backend/k8s/conversion/conversion_test.go
View file
Expand Up @@ -682,27 +682,113 @@ var _ = Describe("Test NetworkPolicy conversion", func() {
Selector: "projectcalico.org/orchestrator == 'k8s' && k == 'v' && k2 == 'v2'",
},
Destination: apiv3.EntityRule{
Ports: []numorstring.Port{numorstring.SinglePort(80)},
Ports: []numorstring.Port{numorstring.SinglePort(80), {MinPort: 0, MaxPort: 0, PortName: "foo"}},
},
},
))

// Check that Types field exists and has only 'ingress'
Expect(len(pol.Value.(*apiv3.NetworkPolicy).Spec.Types)).To(Equal(1))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Types[0]).To(Equal(apiv3.PolicyTypeIngress))

// There should be no Egress rules
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress).To(HaveLen(0))
})

It("should parse a k8s NetworkPolicy with no ports", func() {
np := networkingv1.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "test.policy",
Namespace: "default",
},
Spec: networkingv1.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
"label": "value",
"label2": "value2",
},
},
Ingress: []networkingv1.NetworkPolicyIngressRule{
{
From: []networkingv1.NetworkPolicyPeer{
{
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"k": "v",
"k2": "v2",
},
},
},
},
},
},
PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress},
},
}

// Parse the policy.
pol, err := c.K8sNetworkPolicyToCalico(&np)
Expect(err).NotTo(HaveOccurred())

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress).To(ConsistOf(
apiv3.Rule{
Action: "Allow",
Protocol: &protoTCP, // Defaulted to TCP.
Protocol: nil, // We only default to TCP when ports exist
Source: apiv3.EntityRule{
Selector: "projectcalico.org/orchestrator == 'k8s' && k == 'v' && k2 == 'v2'",
},
Destination: apiv3.EntityRule{
Ports: []numorstring.Port{numorstring.NamedPort("foo")},
},
Destination: apiv3.EntityRule{},
},
))
})

// There should be no Egress rules
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress).To(HaveLen(0))
It("should parse a k8s NetworkPolicy with blank ports", func() {
port80 := intstr.FromInt(80)
np := networkingv1.NetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "test.policy",
Namespace: "default",
},
Spec: networkingv1.NetworkPolicySpec{
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
"label": "value",
"label2": "value2",
},
},
Ingress: []networkingv1.NetworkPolicyIngressRule{
{
Ports: []networkingv1.NetworkPolicyPort{{Port: &port80}, {}},
From: []networkingv1.NetworkPolicyPeer{
{
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"k": "v",
"k2": "v2",
},
},
},
},
},
},
PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress},
},
}

// Check that Types field exists and has only 'ingress'
Expect(len(pol.Value.(*apiv3.NetworkPolicy).Spec.Types)).To(Equal(1))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Types[0]).To(Equal(apiv3.PolicyTypeIngress))
// Parse the policy.
pol, err := c.K8sNetworkPolicyToCalico(&np)
Expect(err).NotTo(HaveOccurred())

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress).To(ConsistOf(
apiv3.Rule{
Action: "Allow",
Protocol: nil, // We only default to TCP when ports exist
Source: apiv3.EntityRule{
Selector: "projectcalico.org/orchestrator == 'k8s' && k == 'v' && k2 == 'v2'",
},
Destination: apiv3.EntityRule{},
},
))
})

It("should drop rules with invalid ports in a k8s NetworkPolicy", func() {
Expand Down Expand Up @@ -765,8 +851,8 @@ var _ = Describe("Test NetworkPolicy conversion", func() {

protoTCP := numorstring.ProtocolFromString("TCP")

// Only the two valid rules should exist. The third should have been dropped.
Expect(len(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress)).To(Equal(2))
// Only the two valid ports should exist. The third should have been dropped.
Expect(len(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress)).To(Equal(1))

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress).To(ConsistOf(
apiv3.Rule{
Expand All @@ -776,17 +862,7 @@ var _ = Describe("Test NetworkPolicy conversion", func() {
Selector: "projectcalico.org/orchestrator == 'k8s' && k == 'v' && k2 == 'v2'",
},
Destination: apiv3.EntityRule{
Ports: []numorstring.Port{numorstring.SinglePort(80)},
},
},
apiv3.Rule{
Action: "Allow",
Protocol: &protoTCP, // Defaulted to TCP.
Source: apiv3.EntityRule{
Selector: "projectcalico.org/orchestrator == 'k8s' && k == 'v' && k2 == 'v2'",
},
Destination: apiv3.EntityRule{
Ports: []numorstring.Port{numorstring.NamedPort("foo")},
Ports: []numorstring.Port{numorstring.SinglePort(80), numorstring.NamedPort("foo")},
},
},
))
Expand Down Expand Up @@ -999,17 +1075,7 @@ var _ = Describe("Test NetworkPolicy conversion", func() {
Selector: "projectcalico.org/orchestrator == 'k8s' && ! has(toast)",
},
Destination: apiv3.EntityRule{
Ports: []numorstring.Port{numorstring.SinglePort(80)},
},
},
apiv3.Rule{
Action: "Allow",
Protocol: &protoTCP, // Defaulted to TCP.
Source: apiv3.EntityRule{
Selector: "projectcalico.org/orchestrator == 'k8s' && ! has(toast)",
},
Destination: apiv3.EntityRule{
Ports: []numorstring.Port{numorstring.NamedPort("foo")},
Ports: []numorstring.Port{numorstring.SinglePort(80), numorstring.NamedPort("foo")},
},
},
))
Expand Down Expand Up @@ -1100,16 +1166,16 @@ var _ = Describe("Test NetworkPolicy conversion", func() {
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Types[0]).To(Equal(apiv3.PolicyTypeIngress))

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[0].Source.Selector).To(Equal("projectcalico.org/orchestrator == 'k8s' && k == 'v'"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[0].Destination.Ports).To(Equal([]numorstring.Port{ninety}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[0].Destination.Ports).To(Equal([]numorstring.Port{eighty}))

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[1].Source.Selector).To(Equal("projectcalico.org/orchestrator == 'k8s' && k2 == 'v2'"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[1].Destination.Ports).To(Equal([]numorstring.Port{ninety}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[1].Destination.Ports).To(Equal([]numorstring.Port{eighty}))

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[2].Source.Selector).To(Equal("projectcalico.org/orchestrator == 'k8s' && k == 'v'"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[2].Destination.Ports).To(Equal([]numorstring.Port{eighty}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[2].Destination.Ports).To(Equal([]numorstring.Port{ninety}))

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[3].Source.Selector).To(Equal("projectcalico.org/orchestrator == 'k8s' && k2 == 'v2'"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[3].Destination.Ports).To(Equal([]numorstring.Port{eighty}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[3].Destination.Ports).To(Equal([]numorstring.Port{ninety}))
})
})

Expand Down Expand Up @@ -1626,13 +1692,13 @@ var _ = Describe("Test NetworkPolicy conversion", func() {
Expect(int(*pol.Value.(*apiv3.NetworkPolicy).Spec.Order)).To(Equal(1000))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Selector).To(Equal("projectcalico.org/orchestrator == 'k8s'"))
Expect(len(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress)).To(Equal(2))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress[0].Destination.Ports).To(Equal([]numorstring.Port{ninetyName}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress[0].Destination.Ports).To(Equal([]numorstring.Port{eightyName}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress[0].Destination.Nets[0]).To(Equal("192.168.0.0/16"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress[0].Destination.NotNets[0]).To(Equal("192.168.3.0/24"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress[0].Destination.NotNets[1]).To(Equal("192.168.4.0/24"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress[0].Destination.NotNets[2]).To(Equal("192.168.5.0/24"))

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress[1].Destination.Ports).To(Equal([]numorstring.Port{eightyName}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress[1].Destination.Ports).To(Equal([]numorstring.Port{ninetyName}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress[1].Destination.Nets[0]).To(Equal("192.168.0.0/16"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress[1].Destination.NotNets[0]).To(Equal("192.168.3.0/24"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Egress[1].Destination.NotNets[1]).To(Equal("192.168.4.0/24"))
Expand Down Expand Up @@ -2026,16 +2092,16 @@ var _ = Describe("Test NetworkPolicy conversion (k8s <= 1.7, no policyTypes)", f
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Types[0]).To(Equal(apiv3.PolicyTypeIngress))

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[0].Source.Selector).To(Equal("projectcalico.org/orchestrator == 'k8s' && k == 'v'"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[0].Destination.Ports).To(Equal([]numorstring.Port{ninety}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[0].Destination.Ports).To(Equal([]numorstring.Port{eighty}))

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[1].Source.Selector).To(Equal("projectcalico.org/orchestrator == 'k8s' && k2 == 'v2'"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[1].Destination.Ports).To(Equal([]numorstring.Port{ninety}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[1].Destination.Ports).To(Equal([]numorstring.Port{eighty}))

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[2].Source.Selector).To(Equal("projectcalico.org/orchestrator == 'k8s' && k == 'v'"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[2].Destination.Ports).To(Equal([]numorstring.Port{eighty}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[2].Destination.Ports).To(Equal([]numorstring.Port{ninety}))

Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[3].Source.Selector).To(Equal("projectcalico.org/orchestrator == 'k8s' && k2 == 'v2'"))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[3].Destination.Ports).To(Equal([]numorstring.Port{eighty}))
Expect(pol.Value.(*apiv3.NetworkPolicy).Spec.Ingress[3].Destination.Ports).To(Equal([]numorstring.Port{ninety}))
})
})

Expand Down

0 comments on commit 6ccf338

Please sign in to comment.