Support default ports value

If an Ingress/Egress Ports field does not specify a `Port` value, but it has a valid `Protocol`, then the rule should match every possible port number. Add unit and end2end test to cover the feature Signed-off-by: Andrea Panattoni <[email protected]>
k8snetworkplumbingwg · Dec 12, 2024 · bf54fd5 · bf54fd5
1 parent 76cd193
commit bf54fd5
Show file tree

Hide file tree

Showing 6 changed files with 297 additions and 5 deletions.
diff --git a/e2e/setup_cluster.sh b/e2e/setup_cluster.sh
@@ -30,7 +30,7 @@ kind export kubeconfig
 sleep 1
 
 # install calico
-kubectl apply -f https://raw.githubusercontent.com/projectcalico/calico/master/manifests/calico.yaml
+kubectl apply -f https://raw.githubusercontent.com/projectcalico/calico/v3.28.1/manifests/calico.yaml
 kubectl -n kube-system set env daemonset/calico-node FELIX_IGNORELOOSERPF=true
 kubectl -n kube-system set env daemonset/calico-node FELIX_XDPENABLED=false
 

diff --git a/e2e/tests/common.bash b/e2e/tests/common.bash
@@ -4,7 +4,7 @@ kubewait_timeout=300s
 
 get_net1_ip() {
 	if [ "$#" == "2" ]; then
-		echo $(kubectl exec -n $1 -c macvlan-worker1 "$2" -- ip -j a show  | jq -r \
+		echo $(kubectl exec -n $1 "$2" -- ip -j a show  | jq -r \
 			 '.[]|select(.ifname =="net1")|.addr_info[]|select(.family=="inet").local')
 	else
 		echo "unknown ip $1"
@@ -13,7 +13,7 @@ get_net1_ip() {
 
 get_net1_ip6() {
 	if [ "$#" == "2" ]; then
-		echo $(kubectl exec -n $1 -c macvlan-worker1 "$2" -- ip -j a show  | jq -r \
+		echo $(kubectl exec -n $1 "$2" -- ip -j a show  | jq -r \
 			 '.[]|select(.ifname =="net1")|.addr_info[]|select(.family=="inet6" and .scope=="global").local')
 	else
 		echo "unknown ip $1"

diff --git a/e2e/tests/protocol-only-ports.bats b/e2e/tests/protocol-only-ports.bats
@@ -0,0 +1,56 @@
+#!/usr/bin/env bats
+
+# Note:
+# These test cases, simple, will create simple (one policy for ingress) and test the 
+# traffic policying by ncat (nc) command. In addition, these cases also verifies that
+# simple iptables generation check by iptables-save and pod-iptable in multi-networkpolicy pod.
+
+setup() {
+	cd $BATS_TEST_DIRNAME
+	load "common"
+	pod_a_net1=$(get_net1_ip "test-protocol-only-ports" "pod-a")
+	pod_b_net1=$(get_net1_ip "test-protocol-only-ports" "pod-b")
+}
+
+@test "setup environments" {
+	# create test manifests
+	kubectl create -f protocol-only-ports.yml
+
+	# verify all pods are available
+	run kubectl -n test-protocol-only-ports wait --for=condition=ready -l app=test-protocol-only-ports pod --timeout=${kubewait_timeout}
+	[ "$status" -eq  "0" ]
+
+	sleep 3
+}
+
+@test "test-protocol-only-ports check pod-a -> pod-b TCP" {
+	# nc should succeed from client-a to server by policy
+	run kubectl -n test-protocol-only-ports exec pod-a -- sh -c "echo x | nc -w 1 ${pod_b_net1} 5555"
+	[ "$status" -eq  "0" ]
+}
+
+@test "test-protocol-only-ports check pod-a -> pod-b UDP" {
+	# nc should succeed from client-a to server by policy
+	run kubectl -n test-protocol-only-ports exec pod-a -- sh -c "echo x | nc --udp -w 1 ${pod_b_net1} 6666"
+	[ "$status" -eq  "1" ]
+}
+
+@test "test-protocol-only-ports check pod-b -> pod-a TCP" {
+	# nc should succeed from client-a to server by policy
+	run kubectl -n test-protocol-only-ports exec pod-b -- sh -c "echo x | nc -w 1 ${pod_a_net1} 5555"
+	[ "$status" -eq  "1" ]
+}
+
+@test "test-protocol-only-ports check pod-b -> pod-a UDP" {
+	# nc should succeed from client-a to server by policy
+	run kubectl -n test-protocol-only-ports exec pod-b -- sh -c "echo x | nc --udp -w 1 ${pod_a_net1} 6666"
+	[ "$status" -eq  "0" ]
+}
+
+@test "cleanup environments" {
+	# remove test manifests
+	kubectl delete -f protocol-only-ports.yml
+	run kubectl -n test-protocol-only-ports wait --for=delete -l app=test-protocol-only-ports pod --timeout=${kubewait_timeout}
+	[ "$status" -eq  "0" ]
+}
+#2.2.6.18
diff --git a/e2e/tests/protocol-only-ports.yml b/e2e/tests/protocol-only-ports.yml
@@ -0,0 +1,97 @@
+---
+apiVersion: "k8s.cni.cncf.io/v1"
+kind: NetworkAttachmentDefinition
+metadata:
+  namespace: default
+  name: macvlan1-simple
+spec: 
+  config: '{
+            "cniVersion": "0.3.1",
+            "name": "macvlan1-simple",
+            "plugins": [
+                {
+                    "type": "macvlan",
+                    "mode": "bridge",
+                    "ipam":{
+                      "type":"host-local",
+                      "subnet":"2.2.6.0/24",
+                      "rangeStart":"2.2.6.8",
+                      "rangeEnd":"2.2.6.67"
+                    }
+                }]
+        }'
+---
+# namespace for MultiNetworkPolicy 
+apiVersion: v1
+kind: Namespace
+metadata:
+    name: test-protocol-only-ports
+---
+# Pods
+apiVersion: v1
+kind: Pod
+metadata:
+  name: pod-a
+  namespace: test-protocol-only-ports
+  annotations:
+    k8s.v1.cni.cncf.io/networks: default/macvlan1-simple
+  labels:
+    app: test-protocol-only-ports
+    name: pod-a
+spec:
+  containers:
+  - name: netcat-tcp
+    image: ghcr.io/k8snetworkplumbingwg/multi-networkpolicy-iptables:e2e-test
+    command: ["nc", "-klp", "5555"]
+    securityContext:
+      privileged: true
+  - name: netcat-udp
+    image: ghcr.io/k8snetworkplumbingwg/multi-networkpolicy-iptables:e2e-test
+    command: ["nc", "-vv", "--udp", "--keep-open", "--sh-exec", "/bin/cat >&2", "--listen", "6666"]
+    securityContext:
+      privileged: true
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  name: pod-b
+  namespace: test-protocol-only-ports
+  annotations:
+    k8s.v1.cni.cncf.io/networks: default/macvlan1-simple
+  labels:
+    app: test-protocol-only-ports
+    name: pod-b
+spec:
+  containers:
+  - name: netcat-tcp
+    image: ghcr.io/k8snetworkplumbingwg/multi-networkpolicy-iptables:e2e-test
+    command: ["nc", "-klp", "5555"]
+    securityContext:
+      privileged: true
+  - name: netcat-udp
+    image: ghcr.io/k8snetworkplumbingwg/multi-networkpolicy-iptables:e2e-test
+    command: ["nc", "-vv", "--udp", "--keep-open", "--sh-exec", "/bin/cat >&2", "--listen", "6666"]
+    securityContext:
+      privileged: true
+---
+# MultiNetworkPolicies
+apiVersion: k8s.cni.cncf.io/v1beta1
+kind: MultiNetworkPolicy
+metadata:
+  name: test-multinetwork-policy-simple-1
+  namespace: test-protocol-only-ports
+  annotations:
+    k8s.v1.cni.cncf.io/policy-for: default/macvlan1-simple
+spec:
+  podSelector:
+    matchLabels:
+      name: pod-a
+  policyTypes:
+  - Egress
+  - Ingress
+  egress:
+  - ports:
+    - protocol: TCP
+  ingress:
+  - ports:
+    - protocol: UDP
diff --git a/pkg/server/policyrules.go b/pkg/server/policyrules.go
@@ -255,9 +255,15 @@ func (ipt *iptableBuffer) renderIngressPorts(_ *Server, podInfo *controllers.Pod
 			if !podIntf.CheckPolicyNetwork(policyNetworks) {
 				continue
 			}
+
+			dport := ""
+			if port.Port != nil {
+				dport = "--dport " + port.Port.String()
+			}
+
 			writeLine(ipt.ingressPorts, "-A", chainName,
 				"-i", podIntf.InterfaceName,
-				"-m", proto, "-p", proto, "--dport", port.Port.String(),
+				"-m", proto, "-p", proto, dport,
 				"-j", "MARK", "--set-xmark", "0x10000/0x10000")
 			validPorts++
 		}
@@ -480,9 +486,15 @@ func (ipt *iptableBuffer) renderEgressPorts(_ *Server, podInfo *controllers.PodI
 			if !podIntf.CheckPolicyNetwork(policyNetworks) {
 				continue
 			}
+
+			dport := ""
+			if port.Port != nil {
+				dport = "--dport " + port.Port.String()
+			}
+
 			writeLine(ipt.egressPorts, "-A", chainName,
 				"-o", podIntf.InterfaceName,
-				"-m", proto, "-p", proto, "--dport", port.Port.String(),
+				"-m", proto, "-p", proto, dport,
 				"-j", "MARK", "--set-xmark", "0x10000/0x10000")
 			validPorts++
 		}

diff --git a/pkg/server/policyrules_test.go b/pkg/server/policyrules_test.go
@@ -1804,6 +1804,133 @@ COMMIT
 
 	})
 
+	It("match all ports when only the protocol is specified", func() {
+		// https://github.com/zeeke/multi-networkpolicy/blob/f76867e779b86b5ca6ba0002bfe716876e66e959/scheme.yml#L59
+
+		protoTCP := v1.ProtocolTCP
+		protoUDP := v1.ProtocolTCP
+		policy1 := &multiv1beta1.MultiNetworkPolicy{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "all-ports-policy",
+				Namespace: "testns1",
+				Annotations: map[string]string{
+					PolicyNetworkAnnotation: "net-attach1",
+				},
+			},
+			Spec: multiv1beta1.MultiNetworkPolicySpec{
+				PodSelector: metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"role": "targetpod",
+					},
+				},
+				Ingress: []multiv1beta1.MultiNetworkPolicyIngressRule{{
+					Ports: []multiv1beta1.MultiNetworkPolicyPort{{
+						Protocol: &protoTCP,
+					}},
+				}},
+				Egress: []multiv1beta1.MultiNetworkPolicyEgressRule{{
+					Ports: []multiv1beta1.MultiNetworkPolicyPort{{
+						Protocol: &protoUDP,
+					}},
+				}},
+			},
+		}
+
+		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)
+fmt.Println(result.Dump.String())
+		Expect(result.Dump.String()).To(Equal(`*nat
+:PREROUTING - [0:0]
+:INPUT - [0:0]
+:OUTPUT - [0:0]
+:POSTROUTING - [0:0]
+-A PREROUTING -i net1 -j RETURN
+COMMIT
+*filter
+:INPUT - [0:0]
+:FORWARD - [0:0]
+:OUTPUT - [0:0]
+:MULTI-INGRESS - [0:0]
+:MULTI-EGRESS - [0:0]
+:MULTI-INGRESS-COMMON - [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]
+:MULTI-0-EGRESS - [0:0]
+:MULTI-0-EGRESS-0-PORTS - [0:0]
+:MULTI-0-EGRESS-0-TO - [0:0]
+-A INPUT -i net1 -j MULTI-INGRESS
+-A OUTPUT -o net1 -j MULTI-EGRESS
+-A MULTI-INGRESS -j MULTI-INGRESS-COMMON
+-A MULTI-INGRESS -m comment --comment "policy:all-ports-policy net-attach-def:testns1/net-attach1" -i net1 -j MULTI-0-INGRESS
+-A MULTI-INGRESS -m mark --mark 0x30000/0x30000 -j RETURN
+-A MULTI-INGRESS -j DROP
+-A MULTI-EGRESS -j MULTI-EGRESS-COMMON
+-A MULTI-EGRESS -m comment --comment "policy:all-ports-policy net-attach-def:testns1/net-attach1" -o net1 -j MULTI-0-EGRESS
+-A MULTI-EGRESS -m mark --mark 0x30000/0x30000 -j RETURN
+-A MULTI-EGRESS -j DROP
+-A MULTI-INGRESS-COMMON -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
+-A MULTI-EGRESS-COMMON -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
+-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-0-INGRESS-0-PORTS -i net1 -m tcp -p tcp  -j MARK --set-xmark 0x10000/0x10000
+-A MULTI-0-INGRESS-0-FROM -m comment --comment "no ingress from, skipped" -j MARK --set-xmark 0x20000/0x20000
+-A MULTI-0-EGRESS -j MARK --set-xmark 0x0/0x30000
+-A MULTI-0-EGRESS -j MULTI-0-EGRESS-0-PORTS
+-A MULTI-0-EGRESS -j MULTI-0-EGRESS-0-TO
+-A MULTI-0-EGRESS -m mark --mark 0x30000/0x30000 -j RETURN
+-A MULTI-0-EGRESS-0-PORTS -o net1 -m tcp -p tcp  -j MARK --set-xmark 0x10000/0x10000
+-A MULTI-0-EGRESS-0-TO -m comment --comment "no egress to, skipped" -j MARK --set-xmark 0x20000/0x20000
+COMMIT
+*mangle
+COMMIT
+`))
+
+	})
+
 	Context("IPv6", func() {
 		It("shoud avoid using IPv4 addresses on ip6tables", func() {