Resolve more comments

pkg/agent/controller/networkpolicy/reconciler.go

@@ -748,7 +748,7 @@ func groupPodsByServices(services []v1beta1.Service, pods v1beta1.GroupMemberPod
 	resolvedServices := make([]v1beta1.Service, len(services))
 	for podKey, pod := range pods {
 		for i := range services {
-			resolvedServices[i] = *resolveService(&services[i], *pod.ToGroupMember())
+			resolvedServices[i] = *resolveService(&services[i], pod.ToGroupMember())
 		}
 		svcKey := normalizeServices(resolvedServices)
 		if _, exists := podsByServicesMap[svcKey]; !exists {
@@ -788,7 +788,7 @@ func groupMembersByServices(services []v1beta1.Service, memberSet v1beta1.GroupM
 	resolvedServices := make([]v1beta1.Service, len(services))
 	for memberKey, member := range memberSet {
 		for i := range services {
-			resolvedServices[i] = *resolveService(&services[i], *member)
+			resolvedServices[i] = *resolveService(&services[i], member)
 		}
 		svcKey := normalizeServices(resolvedServices)
 		if _, exists := membersByServicesMap[svcKey]; !exists {
@@ -886,7 +886,7 @@ func filterUnresolvablePort(in []v1beta1.Service) []v1beta1.Service {
 
 // resolveService resolves the port name of the provided service to a port number
 // for the provided groupMember.
-func resolveService(service *v1beta1.Service, member v1beta1.GroupMember) *v1beta1.Service {
+func resolveService(service *v1beta1.Service, member *v1beta1.GroupMember) *v1beta1.Service {
 	// If port is not specified or is already a number, return it as is.
 	if service.Port == nil || service.Port.Type == intstr.Int {
 		return service

pkg/apis/controlplane/helper.go

@@ -0,0 +1,33 @@
+// Copyright 2020 Antrea Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package controlplane
+
+// Conversion functions between GroupMember and GroupMemberPod
+func (g *GroupMember) ToGroupMemberPod() *GroupMemberPod {
+	return &GroupMemberPod{
+		Pod:   g.Pod,
+		IP:    g.Endpoints[0].IP,
+		Ports: g.Endpoints[0].Ports,
+	}
+}
+
+func (p *GroupMemberPod) ToGroupMember() *GroupMember {
+	return &GroupMember{
+		Pod: p.Pod,
+		Endpoints: []Endpoint{
+			{IP: p.IP, Ports: p.Ports},
+		},
+	}
+}

pkg/apis/controlplane/sets.go

@@ -213,21 +213,3 @@ func (s GroupMemberSet) Items() []*GroupMember {
 	}
 	return res
 }
-
-// Conversion functions
-func (g *GroupMember) ToGroupMemberPod() *GroupMemberPod {
-	return &GroupMemberPod{
-		Pod:   g.Pod,
-		IP:    g.Endpoints[0].IP,
-		Ports: g.Endpoints[0].Ports,
-	}
-}
-
-func (p *GroupMemberPod) ToGroupMember() *GroupMember {
-	return &GroupMember{
-		Pod: p.Pod,
-		Endpoints: []Endpoint{
-			{IP: p.IP, Ports: p.Ports},
-		},
-	}
-}

pkg/apis/controlplane/v1beta1/helper.go

@@ -0,0 +1,33 @@
+// Copyright 2020 Antrea Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package v1beta1
+
+// Conversion functions between GroupMember and GroupMemberPod
+func (g *GroupMember) ToGroupMemberPod() *GroupMemberPod {
+	return &GroupMemberPod{
+		Pod:   g.Pod,
+		IP:    g.Endpoints[0].IP,
+		Ports: g.Endpoints[0].Ports,
+	}
+}
+
+func (p *GroupMemberPod) ToGroupMember() *GroupMember {
+	return &GroupMember{
+		Pod: p.Pod,
+		Endpoints: []Endpoint{
+			{IP: p.IP, Ports: p.Ports},
+		},
+	}
+}

pkg/apis/controlplane/v1beta1/sets.go

@@ -224,21 +224,3 @@ func (s GroupMemberSet) Items() []*GroupMember {
 	}
 	return res
 }
-
-// Conversion functions
-func (g *GroupMember) ToGroupMemberPod() *GroupMemberPod {
-	return &GroupMemberPod{
-		Pod:   g.Pod,
-		IP:    g.Endpoints[0].IP,
-		Ports: g.Endpoints[0].Ports,
-	}
-}
-
-func (p *GroupMemberPod) ToGroupMember() *GroupMember {
-	return &GroupMember{
-		Pod: p.Pod,
-		Endpoints: []Endpoint{
-			{IP: p.IP, Ports: p.Ports},
-		},
-	}
-}

pkg/controller/networkpolicy/antreanetworkpolicy_test.go

@@ -383,3 +383,40 @@ func getANP() *secv1alpha1.NetworkPolicy {
 	}
 	return npObj
 }
+
+func getEETestANP(selectorAppliedTo, selectorIn, selectorOut metav1.LabelSelector) *secv1alpha1.NetworkPolicy {
+	allowAction := secv1alpha1.RuleActionAllow
+	return &secv1alpha1.NetworkPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "anpA",
+			Namespace: "nsA",
+		},
+		Spec: secv1alpha1.NetworkPolicySpec{
+			AppliedTo: []secv1alpha1.NetworkPolicyPeer{
+				{
+					PodSelector: &selectorAppliedTo,
+				},
+			},
+			Ingress: []secv1alpha1.Rule{
+				{
+					From: []secv1alpha1.NetworkPolicyPeer{
+						{
+							ExternalEntitySelector: &selectorIn,
+						},
+					},
+					Action: &allowAction,
+				},
+			},
+			Egress: []secv1alpha1.Rule{
+				{
+					To: []secv1alpha1.NetworkPolicyPeer{
+						{
+							ExternalEntitySelector: &selectorOut,
+						},
+					},
+					Action: &allowAction,
+				},
+			},
+		},
+	}
+}

pkg/controller/networkpolicy/networkpolicy_controller.go

@@ -148,7 +148,6 @@ type NetworkPolicyController struct {
 	// anpLister is able to list/get AntreaNetworkPolicies and is populated by the shared informer passed to
 	// NewNetworkPolicyController.
 	anpLister seclisters.NetworkPolicyLister
-
 	// anpListerSynced is a function which returns true if the AntreaNetworkPolicies shared informer has been synced at least once.
 	anpListerSynced cache.InformerSynced
 
@@ -915,10 +914,10 @@ func (n *NetworkPolicyController) updateExternalEntity(oldObj, curObj interface{
 		return
 	}
 	// Find groups matching the old ExternalEntity's labels.
+	oldAppliedToGroupKeySet := n.filterAppliedToGroupsForPodOrExternalEntity(oldEE)
 	oldAddressGroupKeySet := n.filterAddressGroupsForPodOrExternalEntity(oldEE)
-	oldAppliedToGroupKeySet := n.filterAddressGroupsForPodOrExternalEntity(oldEE)
 	// Find groups matching the new ExternalEntity's labels.
-	curAppliedToGroupKeySet := n.filterAddressGroupsForPodOrExternalEntity(curEE)
+	curAppliedToGroupKeySet := n.filterAppliedToGroupsForPodOrExternalEntity(curEE)
 	curAddressGroupKeySet := n.filterAddressGroupsForPodOrExternalEntity(curEE)
 	// Create set to hold the group keys to enqueue.
 	var appliedToGroupKeys sets.String
@@ -1351,17 +1350,17 @@ func podToMemberPod(pod *v1.Pod, includeIP, includePodRef bool) *controlplane.Gr
 func externalEntityToGroupMember(ee *v1alpha1.ExternalEntity) *controlplane.GroupMember {
 	memberEntity := &controlplane.GroupMember{}
 	for _, endpoint := range ee.Spec.Endpoints {
-		var networkingPorts []controlplane.NamedPort
+		var namedPorts []controlplane.NamedPort
 		for _, port := range endpoint.Ports {
-			networkingPorts = append(networkingPorts, controlplane.NamedPort{
+			namedPorts = append(namedPorts, controlplane.NamedPort{
 				Port:     port.Port,
 				Name:     port.Name,
 				Protocol: controlplane.Protocol(port.Protocol),
 			})
 		}
 		ep := controlplane.Endpoint{
 			IP:    ipStrToIPAddress(endpoint.IP),
-			Ports: networkingPorts,
+			Ports: namedPorts,
 		}
 		memberEntity.Endpoints = append(memberEntity.Endpoints, ep)
 	}
@@ -1396,13 +1395,11 @@ func (n *NetworkPolicyController) processSelector(groupSelector antreatypes.Grou
 			}
 		}
 	} else if groupSelector.NamespaceSelector != nil {
-		// All the Pods and EEs from Namespaces matching the nsSelector must be selected.
+		// All the Pods from Namespaces matching the nsSelector must be selected.
 		namespaces, _ := n.namespaceLister.List(groupSelector.NamespaceSelector)
 		for _, ns := range namespaces {
 			nsPods, _ := n.podLister.Pods(ns.Name).List(labels.Everything())
 			pods = append(pods, nsPods...)
-			nsExtEntities, _ := n.externalEntityLister.ExternalEntities(ns.Name).List(labels.Everything())
-			externalEntities = append(externalEntities, nsExtEntities...)
 		}
 	} else if groupSelector.PodSelector != nil {
 		// Lack of Namespace and NamespaceSelector indicates Pods must be selected