From a84aea9b3d9b01078eb84f0ffe6bac32f091dec1 Mon Sep 17 00:00:00 2001 From: Dyanngg Date: Tue, 15 Aug 2023 02:46:59 -0700 Subject: [PATCH] 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 --- .../controller/networkpolicy/reconciler.go | 7 ++- .../networkpolicy/reconciler_test.go | 43 ++++++++++++++++--- .../networkpolicy/adminnetworkpolicy.go | 12 +++--- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/pkg/agent/controller/networkpolicy/reconciler.go b/pkg/agent/controller/networkpolicy/reconciler.go index 5f7c7279abf..135fcb109ad 100644 --- 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 index 0619e60263a..d8fd3e12179 100644 --- 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 index 75d3c772dab..acf4d374ee3 100644 --- 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)) }