Add support for matching NetworkPolicy rule without port number (#882)

Match the protocol type as one conjunctive match flow if the port number in the
NetworkPolicy rule is not specified.

pkg/agent/openflow/network_policy.go

@@ -549,7 +549,11 @@ func getServiceMatchType(protocol *v1beta1.Protocol) int {
 
 func (c *clause) generateServicePortConjMatch(port v1beta1.Service) *conjunctiveMatch {
 	matchKey := getServiceMatchType(port.Protocol)
-	matchValue := uint16(port.Port.IntVal)
+	// Match all ports with the given protocol type if the matchValue is not specified (value is 0).
+	matchValue := uint16(0)
+	if port.Port != nil {
+		matchValue = uint16(port.Port.IntVal)
+	}
 	match := &conjunctiveMatch{
 		tableID:    c.ruleTable.GetID(),
 		matchKey:   matchKey,

pkg/agent/openflow/pipeline.go

@@ -659,11 +659,23 @@ func (c *client) addFlowMatch(fb binding.FlowBuilder, matchType int, matchValue
 	case MatchSrcOFPort:
 		fb = fb.MatchProtocol(binding.ProtocolIP).MatchInPort(uint32(matchValue.(int32)))
 	case MatchTCPDstPort:
-		fb = fb.MatchProtocol(binding.ProtocolTCP).MatchTCPDstPort(matchValue.(uint16))
+		fb = fb.MatchProtocol(binding.ProtocolTCP)
+		portValue := matchValue.(uint16)
+		if portValue > 0 {
+			fb = fb.MatchTCPDstPort(portValue)
+		}
 	case MatchUDPDstPort:
-		fb = fb.MatchProtocol(binding.ProtocolUDP).MatchUDPDstPort(matchValue.(uint16))
+		fb = fb.MatchProtocol(binding.ProtocolUDP)
+		portValue := matchValue.(uint16)
+		if portValue > 0 {
+			fb = fb.MatchUDPDstPort(portValue)
+		}
 	case MatchSCTPDstPort:
-		fb = fb.MatchProtocol(binding.ProtocolSCTP).MatchSCTPDstPort(matchValue.(uint16))
+		fb = fb.MatchProtocol(binding.ProtocolSCTP)
+		portValue := matchValue.(uint16)
+		if portValue > 0 {
+			fb = fb.MatchSCTPDstPort(portValue)
+		}
 	}
 	return fb
 }

test/e2e/networkpolicy_test.go

@@ -248,6 +248,71 @@ func TestNetworkPolicyResyncAfterRestart(t *testing.T) {
 	}
 }
 
+func TestIngressPolicyWithoutPortNumber(t *testing.T) {
+	data, err := setupTest(t)
+	if err != nil {
+		t.Fatalf("Error when setting up test: %v", err)
+	}
+	defer teardownTest(t, data)
+
+	serverPort := 80
+	_, serverIP, cleanupFunc := createAndWaitForPod(t, data, data.createNginxPodOnNode, "test-server-", "")
+	defer cleanupFunc()
+
+	client0Name, _, cleanupFunc := createAndWaitForPod(t, data, data.createBusyboxPodOnNode, "test-client-", "")
+	defer cleanupFunc()
+
+	client1Name, _, cleanupFunc := createAndWaitForPod(t, data, data.createBusyboxPodOnNode, "test-client-", "")
+	defer cleanupFunc()
+
+	// Both clients can connect to server.
+	for _, clientName := range []string{client0Name, client1Name} {
+		if err = data.runNetcatCommandFromTestPod(clientName, serverIP, serverPort); err != nil {
+			t.Fatalf("Pod %s should be able to connect %s:%d, but was not able to connect", clientName, serverIP, serverPort)
+		}
+	}
+
+	protocol := corev1.ProtocolTCP
+	spec := &networkingv1.NetworkPolicySpec{
+		PodSelector: metav1.LabelSelector{},
+		PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress},
+		Ingress: []networkingv1.NetworkPolicyIngressRule{
+			{
+				Ports: []networkingv1.NetworkPolicyPort{
+					{
+						Protocol: &protocol,
+					},
+				},
+				From: []networkingv1.NetworkPolicyPeer{{
+					PodSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"antrea-e2e": client0Name,
+						},
+					}},
+				},
+			},
+		},
+	}
+	np, err := data.createNetworkPolicy("test-networkpolicy-ingress-no-portnumber", spec)
+	if err != nil {
+		t.Fatalf("Error when creating network policy: %v", err)
+	}
+	defer func() {
+		if err = data.deleteNetworkpolicy(np); err != nil {
+			t.Fatalf("Error when deleting network policy: %v", err)
+		}
+	}()
+
+	// Client0 can access server.
+	if err = data.runNetcatCommandFromTestPod(client0Name, serverIP, serverPort); err != nil {
+		t.Fatalf("Pod %s should be able to connect %s:%d, but was not able to connect", client0Name, serverIP, serverPort)
+	}
+	// Client1 can't access server.
+	if err = data.runNetcatCommandFromTestPod(client1Name, serverIP, serverPort); err == nil {
+		t.Fatalf("Pod %s should not be able to connect %s:%d, but was able to connect", client1Name, serverIP, serverPort)
+	}
+}
+
 func createAndWaitForPod(t *testing.T, data *TestData, createFunc func(name string, nodeName string) error, namePrefix string, nodeName string) (string, string, func()) {
 	name := randName(namePrefix)
 	if err := createFunc(name, nodeName); err != nil {

test/integration/agent/openflow_test.go

@@ -336,14 +336,16 @@ func TestNetworkPolicyFlows(t *testing.T) {
 	conjMatch := fmt.Sprintf("priority=%d,ip,reg1=0x%x", priorityNormal, ofport)
 	flow := &ofTestUtils.ExpectFlow{MatchStr: conjMatch, ActStr: fmt.Sprintf("conjunction(%d,2/3)", ruleID)}
 	assert.True(t, ofTestUtils.OfctlFlowMatch(flowList, ingressRuleTable, flow), "Failed to install conjunctive match flow")
+	serviceConjMatch := fmt.Sprintf("priority=%d,tcp,tp_dst=8080", priorityNormal)
+	flow = &ofTestUtils.ExpectFlow{MatchStr: serviceConjMatch, ActStr: fmt.Sprintf("conjunction(%d,3/3)", ruleID)}
+	assert.True(t, ofTestUtils.OfctlFlowMatch(flowList, ingressRuleTable, flow), "Failed to install service flow")
 
 	// Verify multiple conjunctions share the same match conditions.
 	ruleID2 := uint32(101)
 	toList2 := []string{"192.168.3.4"}
 	toIPList2 := prepareIPAddresses(toList2)
-	port3 := intstr.FromInt(206)
 	udpProtocol := v1beta1.ProtocolUDP
-	npPort2 := v1beta1.Service{Protocol: &udpProtocol, Port: &port3}
+	npPort2 := v1beta1.Service{Protocol: &udpProtocol}
 	rule2 := &types.PolicyRule{
 		Direction: v1beta1.DirectionIn,
 		To:        toIPList2,
@@ -358,9 +360,12 @@ func TestNetworkPolicyFlows(t *testing.T) {
 	conjMatch = fmt.Sprintf("priority=%d,ip,nw_dst=192.168.3.4", priorityNormal)
 	flow1 := &ofTestUtils.ExpectFlow{MatchStr: conjMatch, ActStr: fmt.Sprintf("conjunction(%d,2/3),conjunction(%d,1/2)", ruleID, ruleID2)}
 	flow2 := &ofTestUtils.ExpectFlow{MatchStr: conjMatch, ActStr: fmt.Sprintf("conjunction(%d,1/2),conjunction(%d,2/3)", ruleID2, ruleID)}
+	serviceConjMatch = fmt.Sprintf("priority=%d,udp", priorityNormal)
+	flow3 := &ofTestUtils.ExpectFlow{MatchStr: serviceConjMatch, ActStr: fmt.Sprintf("conjunction(%d,2/2)", ruleID2)}
 	if !ofTestUtils.OfctlFlowMatch(flowList, ingressRuleTable, flow1) && !ofTestUtils.OfctlFlowMatch(flowList, ingressRuleTable, flow2) {
 		t.Errorf("Failed to install conjunctive match flow")
 	}
+	require.True(t, ofTestUtils.OfctlFlowMatch(flowList, ingressRuleTable, flow3), "Failed to install service flow")
 	err = c.UninstallPolicyRuleFlows(ruleID2)
 	require.Nil(t, err, "Failed to InstallPolicyRuleFlows")
 	checkDefaultDropFlows(t, ingressDefaultTable, priorityNormal, types.DstAddress, toIPList2, true)