Address comments

Signed-off-by: abhiraut <rauta@vmware.com>
antrea-io · Aug 20, 2021 · 656a7a2 · 656a7a2
1 parent 1ab6bbd
commit 656a7a2
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 12 deletions.
diff --git a/pkg/apis/controlplane/types.go b/pkg/apis/controlplane/types.go
@@ -15,8 +15,8 @@
 package controlplane
 
 import (
+	"fmt"
 	"net"
-	"strconv"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -135,7 +135,7 @@ type IPNet struct {
 }
 
 func (ipn IPNet) String() string {
-	return ipn.IP.String() + "/" + strconv.Itoa(int(ipn.PrefixLength))
+	return fmt.Sprintf("%s/%d", ipn.IP.String(), ipn.PrefixLength)
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

diff --git a/pkg/apiserver/registry/networkpolicy/clustergroupmember/rest.go b/pkg/apiserver/registry/networkpolicy/clustergroupmember/rest.go
@@ -57,7 +57,8 @@ func (r *REST) Get(ctx context.Context, name string, options *metav1.GetOptions)
 	if len(ipBlocks) > 0 {
 		effectiveIPBlocks := make([]controlplane.IPNet, 0, len(ipBlocks))
 		for _, ipb := range ipBlocks {
-			// ClusterGroup ipBlock cannot have except slices
+			// ClusterGroup ipBlock does not support Except slices, so no need to generate an effective
+			// list of IPs by removing Except slices from allowed CIDR.
 			effectiveIPBlocks = append(effectiveIPBlocks, ipb.CIDR)
 		}
 		memberList.EffectiveIPBlocks = effectiveIPBlocks

diff --git a/pkg/apiserver/registry/networkpolicy/clustergroupmember/rest_test.go b/pkg/apiserver/registry/networkpolicy/clustergroupmember/rest_test.go
@@ -28,18 +28,16 @@ import (
 )
 
 type fakeQuerier struct {
-	members map[string]controlplane.GroupMemberSet
+	members   map[string]controlplane.GroupMemberSet
+	ipMembers map[string][]controlplane.IPBlock
 }
 
 func (q fakeQuerier) GetGroupMembers(uid string) (controlplane.GroupMemberSet, []controlplane.IPBlock, error) {
+	if ipMemberList, ok := q.ipMembers[uid]; ok {
+		return nil, ipMemberList, nil
+	}
 	if memberList, ok := q.members[uid]; ok {
 		return memberList, nil, nil
-	} else if uid == "cgIPBlock" {
-		testCIDR := controlplane.IPNet{
-			IP:           controlplane.IPAddress(net.ParseIP("10.0.0.1")),
-			PrefixLength: int32(24),
-		}
-		return nil, []controlplane.IPBlock{{CIDR: testCIDR}}, nil
 	}
 	return nil, nil, nil
 }
@@ -69,6 +67,14 @@ func TestRESTGet(t *testing.T) {
 			},
 		},
 	}
+	testCIDR := controlplane.IPNet{
+		IP:           controlplane.IPAddress(net.ParseIP("10.0.0.1")),
+		PrefixLength: int32(24),
+	}
+	ipb := []controlplane.IPBlock{{CIDR: testCIDR}}
+	ipMembers := map[string][]controlplane.IPBlock{
+		"cgIPBlock": ipb,
+	}
 	tests := []struct {
 		name        string
 		groupName   string
@@ -145,7 +151,7 @@ func TestRESTGet(t *testing.T) {
 			expectedErr: false,
 		},
 	}
-	rest := NewREST(fakeQuerier{members: members})
+	rest := NewREST(fakeQuerier{members: members, ipMembers: ipMembers})
 	for _, tt := range tests {
 		actualGroupList, err := rest.Get(request.NewDefaultContext(), tt.groupName, &metav1.GetOptions{})
 		if tt.expectedErr {

diff --git a/pkg/controller/networkpolicy/clustergroup.go b/pkg/controller/networkpolicy/clustergroup.go
@@ -427,7 +427,7 @@ func (n *NetworkPolicyController) getAssociatedGroupsByName(grpName string) []an
 }
 
 // GetGroupMembers returns the current members of a ClusterGroup.
-// If the ClusterGroup is defined by IPBlocks, the returned members will be []controlplane.IPBlock.
+// If the ClusterGroup is defined with IPBlocks, the returned members will be []controlplane.IPBlock.
 // Otherwise, the returned members will be of type controlplane.GroupMemberSet.
 func (n *NetworkPolicyController) GetGroupMembers(cgName string) (controlplane.GroupMemberSet, []controlplane.IPBlock, error) {
 	groupObj, found, _ := n.internalGroupStore.Get(cgName)