Merge pull request #10 from s1061123/fix/wrong-iptables

Add namespace check between pod and multi-networkpolicy

pkg/controllers/pod.go

@@ -191,13 +191,12 @@ type InterfaceInfo struct {
 // CheckPolicyNetwork checks whether given interface is target or not,
 // based on policyNetworks
 func (info *InterfaceInfo) CheckPolicyNetwork(policyNetworks []string) bool {
-	isExists := false
 	for _, policyNetworkName := range policyNetworks {
 		if policyNetworkName == info.NetattachName {
-			isExists = true
+			return true
 		}
 	}
-	return isExists
+	return false
 }
 
 // PodInfo contains information that defines a pod.
@@ -210,6 +209,19 @@ type PodInfo struct {
 	Interfaces    []InterfaceInfo
 }
 
+// CheckPolicyNetwork checks whether given pod is target or not,
+// based on policyNetworks
+func (info *PodInfo) CheckPolicyNetwork(policyNetworks []string) bool {
+	for _, intf := range info.Interfaces {
+		for _, policyNetworkName := range policyNetworks {
+			if policyNetworkName == intf.NetattachName {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // GetMultusNetIFs ...
 func (info *PodInfo) GetMultusNetIFs() []string {
 	results := []string{}

pkg/server/policyrules.go

@@ -198,14 +198,7 @@ func (ipt *iptableBuffer) renderIngressPorts(s *Server, podInfo *controllers.Pod
 	// Add jump from MULTI-INGRESS
 	writeLine(ipt.policyIndex, "-A", fmt.Sprintf("MULTI-%d-INGRESS", pIndex), "-j", chainName)
 
-	// Add skip rule if no ports
-	if len(ports) == 0 {
-		writeLine(ipt.ingressPorts, "-A", chainName,
-			"-m", "comment", "--comment", "\"no ingress ports, skipped\"",
-			"-j", "MARK", "--set-xmark", "0x10000/0x10000")
-		return
-	}
-
+	validPorts := 0
 	for _, port := range ports {
 		proto := strings.ToLower(string(*port.Protocol))
 		for _, podIntf := range podInfo.Interfaces {
@@ -216,8 +209,17 @@ func (ipt *iptableBuffer) renderIngressPorts(s *Server, podInfo *controllers.Pod
 				"-i", podIntf.InterfaceName,
 				"-m", proto, "-p", proto, "--dport", port.Port.String(),
 				"-j", "MARK", "--set-xmark", "0x10000/0x10000")
+			validPorts++
 		}
 	}
+
+	// Add skip rule if no ports
+	if len(ports) == 0 || validPorts == 0 {
+		writeLine(ipt.ingressPorts, "-A", chainName,
+			"-m", "comment", "--comment", "\"no ingress ports, skipped\"",
+			"-j", "MARK", "--set-xmark", "0x10000/0x10000")
+	}
+	return
 }
 
 func (ipt *iptableBuffer) renderIngressFrom(s *Server, podInfo *controllers.PodInfo, pIndex, iIndex int, from []multiv1beta1.MultiNetworkPolicyPeer, policyNetworks []string) {
@@ -227,15 +229,8 @@ func (ipt *iptableBuffer) renderIngressFrom(s *Server, podInfo *controllers.PodI
 	// Add jump from MULTI-INGRESS
 	writeLine(ipt.policyIndex, "-A", fmt.Sprintf("MULTI-%d-INGRESS", pIndex), "-j", chainName)
 
-	// Add skip rule if no froms
-	if len(from) == 0 {
-		writeLine(ipt.ingressFrom, "-A", chainName,
-			"-m", "comment", "--comment", "\"no ingress from, skipped\"",
-			"-j", "MARK", "--set-xmark", "0x20000/0x20000")
-		return
-	}
-
 	s.podMap.Update(s.podChanges)
+	validPeers := 0
 	for _, peer := range from {
 		if peer.PodSelector != nil {
 			podSelectorMap, err := metav1.LabelSelectorAsMap(peer.PodSelector)
@@ -288,6 +283,7 @@ func (ipt *iptableBuffer) renderIngressFrom(s *Server, podInfo *controllers.PodI
 							writeLine(ipt.ingressFrom, "-A", chainName,
 								"-i", podIntf.InterfaceName, "-s", ip,
 								"-j", "MARK", "--set-xmark", "0x20000/0x20000")
+							validPeers++
 						}
 					}
 				}
@@ -300,6 +296,7 @@ func (ipt *iptableBuffer) renderIngressFrom(s *Server, podInfo *controllers.PodI
 					}
 					writeLine(ipt.ingressFrom, "-A", chainName,
 						"-i", podIntf.InterfaceName, "-s", except, "-j", "DROP")
+					validPeers++
 				}
 			}
 			for _, podIntf := range podInfo.Interfaces {
@@ -309,11 +306,20 @@ func (ipt *iptableBuffer) renderIngressFrom(s *Server, podInfo *controllers.PodI
 				writeLine(ipt.ingressFrom, "-A", chainName,
 					"-i", podIntf.InterfaceName, "-s", peer.IPBlock.CIDR,
 					"-j", "MARK", "--set-xmark", "0x20000/0x20000")
+				validPeers++
 			}
 		} else {
 			klog.Errorf("unknown rule")
 		}
 	}
+
+	// Add skip rule if no froms
+	if len(from) == 0 || validPeers == 0 {
+		writeLine(ipt.ingressFrom, "-A", chainName,
+			"-m", "comment", "--comment", "\"no ingress from, skipped\"",
+			"-j", "MARK", "--set-xmark", "0x20000/0x20000")
+	}
+	return
 }
 
 func (ipt *iptableBuffer) renderEgress(s *Server, podInfo *controllers.PodInfo, idx int, policy *multiv1beta1.MultiNetworkPolicy, policyNetworks []string) {
@@ -345,14 +351,7 @@ func (ipt *iptableBuffer) renderEgressPorts(s *Server, podInfo *controllers.PodI
 	// Add jump from MULTI-EGRESS
 	writeLine(ipt.policyIndex, "-A", fmt.Sprintf("MULTI-%d-EGRESS", pIndex), "-j", chainName)
 
-	// Add skip rules if no ports
-	if len(ports) == 0 {
-		writeLine(ipt.egressPorts, "-A", chainName,
-			"-m", "comment", "--comment", "\"no egress ports, skipped\"",
-			"-j", "MARK", "--set-xmark", "0x10000/0x10000")
-		return
-	}
-
+	validPorts := 0
 	for _, port := range ports {
 		proto := strings.ToLower(string(*port.Protocol))
 		for _, podIntf := range podInfo.Interfaces {
@@ -363,8 +362,17 @@ func (ipt *iptableBuffer) renderEgressPorts(s *Server, podInfo *controllers.PodI
 				"-o", podIntf.InterfaceName,
 				"-m", proto, "-p", proto, "--dport", port.Port.String(),
 				"-j", "MARK", "--set-xmark", "0x10000/0x10000")
+			validPorts++
 		}
 	}
+
+	// Add skip rules if no ports
+	if len(ports) == 0 || validPorts == 0 {
+		writeLine(ipt.egressPorts, "-A", chainName,
+			"-m", "comment", "--comment", "\"no egress ports, skipped\"",
+			"-j", "MARK", "--set-xmark", "0x10000/0x10000")
+	}
+	return
 }
 
 func (ipt *iptableBuffer) renderEgressTo(s *Server, podInfo *controllers.PodInfo, pIndex, iIndex int, to []multiv1beta1.MultiNetworkPolicyPeer, policyNetworks []string) {
@@ -374,14 +382,8 @@ func (ipt *iptableBuffer) renderEgressTo(s *Server, podInfo *controllers.PodInfo
 	// Add jump from MULTI-EGRESS
 	writeLine(ipt.policyIndex, "-A", fmt.Sprintf("MULTI-%d-EGRESS", pIndex), "-j", chainName)
 
-	// Add skip rules if no to
-	if len(to) == 0 {
-		writeLine(ipt.egressTo, "-A", chainName,
-			"-m", "comment", "--comment", "\"no egress to, skipped\"",
-			"-j", "MARK", "--set-xmark", "0x20000/0x20000")
-		return
-	}
-
+	s.podMap.Update(s.podChanges)
+	validPeers := 0
 	for _, peer := range to {
 		if peer.PodSelector != nil {
 			podSelectorMap, err := metav1.LabelSelectorAsMap(peer.PodSelector)
@@ -435,6 +437,7 @@ func (ipt *iptableBuffer) renderEgressTo(s *Server, podInfo *controllers.PodInfo
 							writeLine(ipt.egressTo, "-A", chainName,
 								"-o", podIntf.InterfaceName, "-d", ip,
 								"-j", "MARK", "--set-xmark", "0x20000/0x20000")
+							validPeers++
 						}
 					}
 				}
@@ -447,6 +450,7 @@ func (ipt *iptableBuffer) renderEgressTo(s *Server, podInfo *controllers.PodInfo
 					}
 					writeLine(ipt.egressTo, "-A", chainName,
 						"-o", multi.InterfaceName, "-d", except, "-j", "DROP")
+					validPeers++
 				}
 			}
 			for _, podIntf := range podInfo.Interfaces {
@@ -456,11 +460,20 @@ func (ipt *iptableBuffer) renderEgressTo(s *Server, podInfo *controllers.PodInfo
 				writeLine(ipt.egressTo, "-A", chainName,
 					"-o", podIntf.InterfaceName, "-d", peer.IPBlock.CIDR,
 					"-j", "MARK", "--set-xmark", "0x20000/0x20000")
+				validPeers++
 			}
 		} else {
 			klog.Errorf("unknown rule")
 		}
 	}
+
+	// Add skip rules if no to
+	if len(to) == 0 || validPeers == 0 {
+		writeLine(ipt.egressTo, "-A", chainName,
+			"-m", "comment", "--comment", "\"no egress to, skipped\"",
+			"-j", "MARK", "--set-xmark", "0x20000/0x20000")
+	}
+	return
 }
 
 // Join all words with spaces, terminate with newline and write to buf.

pkg/server/policyrules_test.go

@@ -721,6 +721,8 @@ var _ = Describe("policyrules testing - invalid case", func() {
 -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 -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 -m comment --comment "no ingress from, skipped" -j MARK --set-xmark 0x20000/0x20000
 COMMIT
 `)
 		Expect(buf.filterRules.Bytes()).To(Equal(finalizedRules))
@@ -809,6 +811,8 @@ COMMIT
 -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 -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 -m comment --comment "no ingress from, skipped" -j MARK --set-xmark 0x20000/0x20000
 COMMIT
 `)
 		Expect(buf.filterRules.Bytes()).To(Equal(finalizedRules))
@@ -884,6 +888,8 @@ COMMIT
 -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 -j DROP
+-A MULTI-0-EGRESS-0-PORTS -m comment --comment "no egress ports, skipped" -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
 `)
 		Expect(buf.filterRules.Bytes()).To(Equal(finalizedRules))
@@ -972,6 +978,8 @@ COMMIT
 -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 -j DROP
+-A MULTI-0-EGRESS-0-PORTS -m comment --comment "no egress ports, skipped" -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
 `)
 		Expect(buf.filterRules.Bytes()).To(Equal(finalizedRules))

pkg/server/server.go

@@ -431,6 +431,8 @@ func (s *Server) OnNamespaceSynced() {
 
 func (s *Server) syncMultiPolicy() {
 	klog.V(4).Infof("syncMultiPolicy")
+	s.namespaceMap.Update(s.nsChanges)
+	s.podMap.Update(s.podChanges)
 	s.policyMap.Update(s.policyChanges)
 
 	pods, err := s.podLister.Pods(metav1.NamespaceAll).List(labels.Everything())
@@ -445,6 +447,7 @@ func (s *Server) syncMultiPolicy() {
 		}
 		klog.V(8).Infof("SYNC %s/%s", p.Namespace, p.Name)
 		if multiutils.CheckNodeNameIdentical(s.Hostname, p.Spec.NodeName) {
+			s.podMap.Update(s.podChanges)
 			podInfo, err := s.podMap.GetPodInfo(p)
 			if err != nil {
 				klog.Errorf("cannot get %s/%s podInfo: %v", p.Namespace, p.Name, err)
@@ -537,6 +540,9 @@ func (s *Server) generatePolicyRules(pod *v1.Pod, podInfo *controllers.PodInfo)
 	idx := 0
 	for _, p := range s.policyMap {
 		policy := p.Policy
+		if policy.GetNamespace() != pod.Namespace {
+			continue
+		}
 		if policy.Spec.PodSelector.Size() != 0 {
 			policyMap, err := metav1.LabelSelectorAsMap(&policy.Spec.PodSelector)
 			if err != nil {
@@ -570,20 +576,22 @@ func (s *Server) generatePolicyRules(pod *v1.Pod, podInfo *controllers.PodInfo)
 		}
 		policyNetworksAnnot = strings.ReplaceAll(policyNetworksAnnot, " ", "")
 		policyNetworks := strings.Split(policyNetworksAnnot, ",")
-		for idx, networkName := range policyNetworks {
+		for pidx, networkName := range policyNetworks {
 			// fill namespace
 			if strings.IndexAny(networkName, "/") == -1 {
-				policyNetworks[idx] = fmt.Sprintf("%s/%s", policy.GetNamespace(), networkName)
+				policyNetworks[pidx] = fmt.Sprintf("%s/%s", policy.GetNamespace(), networkName)
 			}
 		}
 
-		if ingressEnable {
-			iptableBuffer.renderIngress(s, podInfo, idx, policy, policyNetworks)
-		}
-		if egressEnable {
-			iptableBuffer.renderEgress(s, podInfo, idx, policy, policyNetworks)
+		if podInfo.CheckPolicyNetwork(policyNetworks) {
+			if ingressEnable {
+				iptableBuffer.renderIngress(s, podInfo, idx, policy, policyNetworks)
+			}
+			if egressEnable {
+				iptableBuffer.renderEgress(s, podInfo, idx, policy, policyNetworks)
+			}
+			idx++
 		}
-		idx++
 	}
 
 	if !iptableBuffer.IsUsed() {