Fix misisng protocol in service when processing ANP named ports (#5370)

When processing AdminNetworkPolicy and BaselineAdminNetworkPolicy named port rules, the ports section will not have protocol specified. Antrea agent should infer the protocol from the container spec so that rule can be enforced correctly in ovs. Signed-off-by: Dyanngg <[email protected]>
antrea-io · Aug 15, 2023 · a84aea9 · a84aea9
1 parent 160f25f
commit a84aea9
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 13 deletions.
diff --git a/pkg/agent/controller/networkpolicy/reconciler.go b/pkg/agent/controller/networkpolicy/reconciler.go
@@ -1292,7 +1292,12 @@ func resolveService(service *v1beta2.Service, member *v1beta2.GroupMember) *v1be
 		// as the port name is matched.
 		if port.Name == service.Port.StrVal && (service.Protocol == nil || port.Protocol == *service.Protocol) {
 			resolvedPort := intstr.FromInt(int(port.Port))
-			return &v1beta2.Service{Protocol: service.Protocol, Port: &resolvedPort}
+			resolvedProtocol := service.Protocol
+			if resolvedProtocol == nil {
+				// Derive named port protocol from the container spec
+				resolvedProtocol = &port.Protocol
+			}
+			return &v1beta2.Service{Protocol: resolvedProtocol, Port: &resolvedPort}
 		}
 	}
 	klog.InfoS("Cannot resolve Service port for endpoints", "port", service.Port.StrVal, "member", member)

diff --git a/pkg/agent/controller/networkpolicy/reconciler_test.go b/pkg/agent/controller/networkpolicy/reconciler_test.go
@@ -66,12 +66,13 @@ var (
 	portHTTP  = intstr.FromString("http")
 	portHTTPS = intstr.FromString("https")
 
-	serviceTCP80   = v1beta2.Service{Protocol: &protocolTCP, Port: &port80}
-	serviceTCP443  = v1beta2.Service{Protocol: &protocolTCP, Port: &port443}
-	serviceTCP8080 = v1beta2.Service{Protocol: &protocolTCP, Port: &port8080}
-	serviceTCP     = v1beta2.Service{Protocol: &protocolTCP}
-	serviceHTTP    = v1beta2.Service{Protocol: &protocolTCP, Port: &portHTTP}
-	serviceHTTPS   = v1beta2.Service{Protocol: &protocolTCP, Port: &portHTTPS}
+	serviceTCP80          = v1beta2.Service{Protocol: &protocolTCP, Port: &port80}
+	serviceTCP443         = v1beta2.Service{Protocol: &protocolTCP, Port: &port443}
+	serviceTCP8080        = v1beta2.Service{Protocol: &protocolTCP, Port: &port8080}
+	serviceTCP            = v1beta2.Service{Protocol: &protocolTCP}
+	serviceHTTPNoProtocol = v1beta2.Service{Port: &portHTTP}
+	serviceHTTP           = v1beta2.Service{Protocol: &protocolTCP, Port: &portHTTP}
+	serviceHTTPS          = v1beta2.Service{Protocol: &protocolTCP, Port: &portHTTPS}
 
 	services1    = []v1beta2.Service{serviceTCP80}
 	servicesKey1 = normalizeServices(services1)
@@ -92,6 +93,11 @@ var (
 		Name: "name1",
 		UID:  "uid1",
 	}
+	anp1 = v1beta2.NetworkPolicyReference{
+		Type: v1beta2.AdminNetworkPolicy,
+		Name: "anp1",
+		UID:  "uid2",
+	}
 
 	transientError = errors.New("Transient OVS error")
 )
@@ -395,6 +401,31 @@ func TestReconcilerReconcile(t *testing.T) {
 			},
 			false,
 		},
+		{
+			"ingress-rule-with-namedport-no-protocol",
+			&CompletedRule{
+				rule: &rule{
+					ID:             "ingress-rule",
+					Direction:      v1beta2.DirectionIn,
+					Services:       []v1beta2.Service{serviceHTTPNoProtocol},
+					SourceRef:      &anp1,
+					TierPriority:   &tierPriority,
+					PolicyPriority: &policyPriority,
+					Priority:       1,
+				},
+				TargetMembers: appliedToGroupWithSameContainerPort,
+			},
+			[]*types.PolicyRule{
+				{
+					Direction: v1beta2.DirectionIn,
+					From:      []types.Address{},
+					To:        ofPortsToOFAddresses(sets.New[int32](1, 3)),
+					Service:   []v1beta2.Service{serviceTCP80},
+					PolicyRef: &anp1,
+				},
+			},
+			false,
+		},
 		{
 			"ingress-rule-with-diff-namedport",
 			&CompletedRule{

diff --git a/pkg/controller/networkpolicy/adminnetworkpolicy.go b/pkg/controller/networkpolicy/adminnetworkpolicy.go
@@ -64,7 +64,7 @@ func getBANPReference(banp *v1alpha1.BaselineAdminNetworkPolicy) *controlplane.N
 func (n *NetworkPolicyController) addAdminNP(obj interface{}) {
 	defer n.heartbeat("addAdminNP")
 	anp := obj.(*v1alpha1.AdminNetworkPolicy)
-	klog.V(2).InfoS("Processing AdminNetworkPolicy ADD event", "anp", anp.Name)
+	klog.InfoS("Processing AdminNetworkPolicy ADD event", "anp", anp.Name)
 	n.enqueueInternalNetworkPolicy(getAdminNPReference(anp))
 }
 
@@ -73,7 +73,7 @@ func (n *NetworkPolicyController) addAdminNP(obj interface{}) {
 func (n *NetworkPolicyController) updateAdminNP(_, cur interface{}) {
 	defer n.heartbeat("updateAdminNP")
 	curANP := cur.(*v1alpha1.AdminNetworkPolicy)
-	klog.V(2).InfoS("Processing AdminNetworkPolicy UPDATE event", "anp", curANP.Name)
+	klog.InfoS("Processing AdminNetworkPolicy UPDATE event", "anp", curANP.Name)
 	n.enqueueInternalNetworkPolicy(getAdminNPReference(curANP))
 }
 
@@ -94,7 +94,7 @@ func (n *NetworkPolicyController) deleteAdminNP(old interface{}) {
 		}
 	}
 	defer n.heartbeat("deleteAdminNP")
-	klog.V(2).InfoS("Processing AdminNetworkPolicy DELETE event", "anp", anp.Name)
+	klog.InfoS("Processing AdminNetworkPolicy DELETE event", "anp", anp.Name)
 	n.enqueueInternalNetworkPolicy(getAdminNPReference(anp))
 }
 
@@ -103,7 +103,7 @@ func (n *NetworkPolicyController) deleteAdminNP(old interface{}) {
 func (n *NetworkPolicyController) addBANP(obj interface{}) {
 	defer n.heartbeat("addBANP")
 	banp := obj.(*v1alpha1.BaselineAdminNetworkPolicy)
-	klog.V(2).InfoS("Processing BaselineAdminNetworkPolicy ADD event", "banp", banp.Name)
+	klog.InfoS("Processing BaselineAdminNetworkPolicy ADD event", "banp", banp.Name)
 	n.enqueueInternalNetworkPolicy(getBANPReference(banp))
 }
 
@@ -112,7 +112,7 @@ func (n *NetworkPolicyController) addBANP(obj interface{}) {
 func (n *NetworkPolicyController) updateBANP(_, cur interface{}) {
 	defer n.heartbeat("updateBANP")
 	curBANP := cur.(*v1alpha1.BaselineAdminNetworkPolicy)
-	klog.V(2).InfoS("Processing BaselineAdminNetworkPolicy UPDATE event", "banp", curBANP.Name)
+	klog.InfoS("Processing BaselineAdminNetworkPolicy UPDATE event", "banp", curBANP.Name)
 	n.enqueueInternalNetworkPolicy(getBANPReference(curBANP))
 }
 
@@ -133,7 +133,7 @@ func (n *NetworkPolicyController) deleteBANP(old interface{}) {
 		}
 	}
 	defer n.heartbeat("deleteBANP")
-	klog.V(2).InfoS("Processing BaselineAdminNetworkPolicy DELETE event", "banp", banp.Name)
+	klog.InfoS("Processing BaselineAdminNetworkPolicy DELETE event", "banp", banp.Name)
 	n.enqueueInternalNetworkPolicy(getBANPReference(banp))
 }