Infer PolicyTypes if missing (#50)

* Infer PolicyTypes if missing

In cases where Spec.PolicyTypes is not specified, it should
default to the existence of Ingress or Egress rules.

Updating end2end tests to cover also this scenario.

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

* e2e: Wait for policy sync during setup

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

---------

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

e2e/tests/simple-v4-egress-list.bats

@@ -21,6 +21,9 @@ setup() {
 	# verify all pods are available
 	run kubectl -n test-simple-v4-egress-list wait --for=condition=ready -l app=test-simple-v4-egress-list pod --timeout=${kubewait_timeout}
 	[ "$status" -eq  "0" ]
+
+	# wait for sync
+	sleep 3
 }
 
 @test "test-simple-v4-egress-list check client-a -> server" {

e2e/tests/simple-v4-egress.yml

@@ -95,8 +95,6 @@ spec:
   podSelector:
     matchLabels:
       name: pod-server
-  policyTypes:
-  - Egress
   egress:
   - to:
     - podSelector:

e2e/tests/simple-v4-ingress-list.bats

@@ -21,6 +21,9 @@ setup() {
 	# verify all pods are available
 	run kubectl -n test-simple-v4-ingress-list wait --for=condition=ready -l app=test-simple-v4-ingress-list pod --timeout=${kubewait_timeout}
 	[ "$status" -eq  "0" ]
+
+	# wait for sync
+	sleep 3
 }
 
 @test "test-simple-v4-ingress-list check client-a -> server" {

e2e/tests/simple-v4-ingress.yml

@@ -95,8 +95,6 @@ spec:
   podSelector:
     matchLabels:
       name: pod-server
-  policyTypes:
-  - Ingress
   ingress:
   - from:
     - podSelector:

e2e/tests/simple-v6-ingress-list.bats

@@ -22,6 +22,9 @@ setup() {
 	# verify all pods are available
 	run kubectl -n test-simple-v6-ingress-list wait --for=condition=ready -l app=test-simple-v6-ingress-list pod --timeout=${kubewait_timeout}
 	[ "$status" -eq  "0" ]
+
+	# wait for sync
+	sleep 3
 }
 
 @test "test-simple-v6-ingress-list check client-a -> server" {

pkg/server/policyrules_test.go

@@ -1597,6 +1597,102 @@ COMMIT
 		Expect(buf.egressPorts.String()).To(Equal(portRules))
 	})
 
+	It("policyType should be implicitly inferred", func() {
+
+		policy1 := &multiv1beta1.MultiNetworkPolicy{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "ingressPolicies1",
+				Namespace: "testns1",
+				Annotations: map[string]string{
+					PolicyNetworkAnnotation: "net-attach1",
+				},
+			},
+			Spec: multiv1beta1.MultiNetworkPolicySpec{
+				PodSelector: metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"role": "targetpod",
+					},
+				},
+				Ingress: []multiv1beta1.MultiNetworkPolicyIngressRule{{
+					From: []multiv1beta1.MultiNetworkPolicyPeer{{
+						PodSelector: &metav1.LabelSelector{
+							MatchLabels: map[string]string{
+								"foobar": "enabled",
+							},
+						},
+					}},
+				}},
+			},
+		}
+
+		s := NewFakeServer("samplehost")
+		Expect(s).NotTo(BeNil())
+
+		AddNamespace(s, "testns1")
+
+		Expect(
+			s.netdefChanges.Update(nil, NewNetDef("testns1", "net-attach1", NewCNIConfig("testCNI", "multi"))),
+		).To(BeTrue())
+
+		pod1 := NewFakePodWithNetAnnotation(
+			"testns1",
+			"testpod1",
+			"net-attach1",
+			NewFakeNetworkStatus("testns1", "net-attach1", "192.168.1.1", "10.1.1.1"),
+			map[string]string{
+				"role": "targetpod",
+			})
+		pod1.Spec.NodeName = "samplehost"
+
+		AddPod(s, pod1)
+		podInfo1, err := s.podMap.GetPodInfo(pod1)
+		Expect(err).NotTo(HaveOccurred())
+
+		pod2 := NewFakePodWithNetAnnotation(
+			"testns1",
+			"testpod2",
+			"net-attach1",
+			NewFakeNetworkStatus("testns1", "net-attach1", "192.168.1.2", "10.1.1.2"),
+			map[string]string{
+				"foobar": "enabled",
+			})
+		AddPod(s, pod2)
+
+		Expect(
+			s.policyChanges.Update(nil, policy1),
+		).To(BeTrue())
+		s.policyMap.Update(s.policyChanges)
+
+		result := fakeiptables.NewFake()
+		s.ip4Tables = result
+
+		s.generatePolicyRulesForPod(pod1, podInfo1)
+
+		Expect(string(result.Lines)).To(Equal(`*filter
+:MULTI-INGRESS - [0:0]
+:MULTI-INGRESS-COMMON - [0:0]
+:MULTI-EGRESS - [0:0]
+:MULTI-EGRESS-COMMON - [0:0]
+:MULTI-0-INGRESS - [0:0]
+:MULTI-0-INGRESS-0-PORTS - [0:0]
+:MULTI-0-INGRESS-0-FROM - [0:0]
+-A MULTI-INGRESS-COMMON -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
+-A MULTI-INGRESS -j MULTI-INGRESS-COMMON
+-A MULTI-INGRESS -m comment --comment "policy:ingressPolicies1 net-attach-def:testns1/net-attach1" -i net1 -j MULTI-0-INGRESS
+-A MULTI-INGRESS -m mark --mark 0x30000/0x30000 -j RETURN
+-A MULTI-0-INGRESS -j MARK --set-xmark 0x0/0x30000
+-A MULTI-0-INGRESS -j MULTI-0-INGRESS-0-PORTS
+-A MULTI-0-INGRESS -j MULTI-0-INGRESS-0-FROM
+-A MULTI-0-INGRESS -m mark --mark 0x30000/0x30000 -j RETURN
+-A MULTI-INGRESS -j DROP
+-A MULTI-0-INGRESS-0-PORTS -m comment --comment "no ingress ports, skipped" -j MARK --set-xmark 0x10000/0x10000
+-A MULTI-0-INGRESS-0-FROM -i net1 -s 10.1.1.2 -j MARK --set-xmark 0x20000/0x20000
+-A MULTI-0-INGRESS-0-FROM -i net1 -s 10.1.1.1 -j MARK --set-xmark 0x20000/0x20000
+COMMIT
+`))
+
+	})
+
 	Context("IPv6", func() {
 		It("shoud avoid using IPv4 addresses on ip6tables", func() {
 

pkg/server/server.go

@@ -477,7 +477,7 @@ func (s *Server) syncMultiPolicy() {
 
 			netns, err := ns.GetNS(netnsPath)
 			if err != nil {
-				klog.Errorf("cannot get pod (%s/%s:%s) netns: %v", p.Namespace, p.Name, p.Status.Phase, err)
+				klog.Errorf("cannot get pod (%s/%s:%s) netns (%s): %v", p.Namespace, p.Name, p.Status.Phase, netnsPath, err)
 				continue
 			}
 
@@ -603,19 +603,7 @@ func (s *Server) generatePolicyRulesForPodAndFamily(pod *v1.Pod, podInfo *contro
 			}
 		}
 
-		var ingressEnable, egressEnable bool
-		if len(policy.Spec.PolicyTypes) == 0 {
-			ingressEnable = true
-			egressEnable = true
-		} else {
-			for _, v := range policy.Spec.PolicyTypes {
-				if strings.EqualFold(string(v), string(multiv1beta1.PolicyTypeIngress)) {
-					ingressEnable = true
-				} else if strings.EqualFold(string(v), string(multiv1beta1.PolicyTypeEgress)) {
-					egressEnable = true
-				}
-			}
-		}
+		ingressEnable, egressEnable := getEnabledPolicyTypes(policy)
 		klog.V(8).Infof("ingress/egress = %v/%v\n", ingressEnable, egressEnable)
 
 		policyNetworksAnnot, ok := policy.GetAnnotations()[PolicyNetworkAnnotation]
@@ -706,3 +694,19 @@ func nadNamespacedName(o *netdefv1.NetworkAttachmentDefinition) string {
 	}
 	return o.GetNamespace() + "/" + o.GetName()
 }
+
+func getEnabledPolicyTypes(policy *multiv1beta1.MultiNetworkPolicy) (bool, bool) {
+	var ingressEnable, egressEnable bool
+	if len(policy.Spec.PolicyTypes) > 0 {
+		for _, v := range policy.Spec.PolicyTypes {
+			if strings.EqualFold(string(v), string(multiv1beta1.PolicyTypeIngress)) {
+				ingressEnable = true
+			} else if strings.EqualFold(string(v), string(multiv1beta1.PolicyTypeEgress)) {
+				egressEnable = true
+			}
+		}
+		return ingressEnable, egressEnable
+	}
+
+	return len(policy.Spec.Ingress) > 0, len(policy.Spec.Egress) > 0
+}