Fix nil pointer dereference when ClusterGroup/Group is used

pkg/controller/networkpolicy/clustergroup.go

@@ -281,17 +281,17 @@ func (c *NetworkPolicyController) triggerParentGroupUpdates(grp string) {
 
 // triggerDerivedGroupUpdates triggers processing of AppliedToGroup and AddressGroup derived from the provided group.
 func (c *NetworkPolicyController) triggerDerivedGroupUpdates(grp string) {
-	_, exists, _ := c.appliedToGroupStore.Get(grp)
-	if exists {
+	groups, _ := c.appliedToGroupStore.GetByIndex(store.SourceGroupIndex, grp)
+	for _, group := range groups {
 		// It's fine if the group is deleted after checking its existence as syncAppliedToGroup will do nothing when it
 		// doesn't find the group.
-		c.enqueueAppliedToGroup(grp)
+		c.enqueueAppliedToGroup(group.(*antreatypes.AppliedToGroup).Name)
 	}
-	_, exists, _ = c.addressGroupStore.Get(grp)
-	if exists {
+	groups, _ = c.addressGroupStore.GetByIndex(store.SourceGroupIndex, grp)
+	for _, group := range groups {
 		// It's fine if the group is deleted after checking its existence as syncAddressGroup will do nothing when it
 		// doesn't find the group.
-		c.enqueueAddressGroup(grp)
+		c.enqueueAddressGroup(group.(*antreatypes.AddressGroup).Name)
 	}
 }
 

pkg/controller/networkpolicy/clusternetworkpolicy.go

@@ -615,7 +615,7 @@ func (n *NetworkPolicyController) processRefGroupOrClusterGroup(g, namespace str
 	// up if the Group/ClusterGroup becomes ipBlocks-only.
 	createAddrGroup, ipb := n.processInternalGroupForRule(intGrp)
 	if createAddrGroup {
-		ag := &antreatypes.AddressGroup{UID: intGrp.UID, Name: key}
+		ag := &antreatypes.AddressGroup{UID: intGrp.UID, Name: key, SourceGroup: key}
 		return ag, ipb
 	}
 	return nil, ipb

pkg/controller/networkpolicy/clusternetworkpolicy_test.go

@@ -1961,8 +1961,12 @@ func TestProcessRefGroupOrClusterGroup(t *testing.T) {
 		{
 			name:           "cg-with-selector",
 			inputGroupName: cgA.Name,
-			expectedAG:     &antreatypes.AddressGroup{UID: cgA.UID, Name: cgA.Name},
-			expectedIPB:    nil,
+			expectedAG: &antreatypes.AddressGroup{
+				UID:         cgA.UID,
+				Name:        cgA.Name,
+				SourceGroup: cgA.Name,
+			},
+			expectedIPB: nil,
 		},
 		{
 			name:           "cg-with-selector-not-found",
@@ -1995,7 +1999,11 @@ func TestProcessRefGroupOrClusterGroup(t *testing.T) {
 		{
 			name:           "nested-cg-with-ipblock-and-selector",
 			inputGroupName: cgNested2.Name,
-			expectedAG:     &antreatypes.AddressGroup{UID: cgNested2.UID, Name: cgNested2.Name},
+			expectedAG: &antreatypes.AddressGroup{
+				UID:         cgNested2.UID,
+				Name:        cgNested2.Name,
+				SourceGroup: cgNested2.Name,
+			},
 			expectedIPB: []controlplane.IPBlock{
 				{
 					CIDR:   *cidrIPNet,
@@ -2014,8 +2022,12 @@ func TestProcessRefGroupOrClusterGroup(t *testing.T) {
 			name:           "g-with-selector",
 			inputNamespace: gA.Namespace,
 			inputGroupName: gA.Name,
-			expectedAG:     &antreatypes.AddressGroup{UID: gA.UID, Name: fmt.Sprintf("%s/%s", gA.Namespace, gA.Name)},
-			expectedIPB:    nil,
+			expectedAG: &antreatypes.AddressGroup{
+				UID:         gA.UID,
+				Name:        fmt.Sprintf("%s/%s", gA.Namespace, gA.Name),
+				SourceGroup: fmt.Sprintf("%s/%s", gA.Namespace, gA.Name),
+			},
+			expectedIPB: nil,
 		},
 		{
 			name:           "non-existing-group",

pkg/controller/networkpolicy/crd_utils.go

@@ -322,7 +322,7 @@ func (n *NetworkPolicyController) createAppliedToGroupForGroup(namespace, group
 		klog.V(2).InfoS("Group with IPBlocks can not be used as AppliedTo", "Group", key)
 		return nil
 	}
-	return &antreatypes.AppliedToGroup{UID: intGrp.UID, Name: key}
+	return &antreatypes.AppliedToGroup{UID: intGrp.UID, Name: key, SourceGroup: key}
 }
 
 // getTierPriority retrieves the priority associated with the input Tier name.

pkg/controller/networkpolicy/crd_utils_test.go

@@ -550,9 +550,13 @@ func TestCreateAppliedToGroupsForGroup(t *testing.T) {
 			expectedATG: nil,
 		},
 		{
-			name:        "cluster group with selectors",
-			inputGroup:  clusterGroupWithSelector.Name,
-			expectedATG: &antreatypes.AppliedToGroup{UID: clusterGroupWithSelector.UID, Name: clusterGroupWithSelector.Name},
+			name:       "cluster group with selectors",
+			inputGroup: clusterGroupWithSelector.Name,
+			expectedATG: &antreatypes.AppliedToGroup{
+				UID:         clusterGroupWithSelector.UID,
+				Name:        clusterGroupWithSelector.Name,
+				SourceGroup: clusterGroupWithSelector.Name,
+			},
 		},
 		{
 			name:           "empty group name",
@@ -576,7 +580,11 @@ func TestCreateAppliedToGroupsForGroup(t *testing.T) {
 			name:           "group with selectors",
 			inputNamespace: groupWithSelector.Namespace,
 			inputGroup:     groupWithSelector.Name,
-			expectedATG:    &antreatypes.AppliedToGroup{UID: groupWithSelector.UID, Name: fmt.Sprintf("%s/%s", groupWithSelector.Namespace, groupWithSelector.Name)},
+			expectedATG: &antreatypes.AppliedToGroup{
+				UID:         groupWithSelector.UID,
+				Name:        fmt.Sprintf("%s/%s", groupWithSelector.Namespace, groupWithSelector.Name),
+				SourceGroup: fmt.Sprintf("%s/%s", groupWithSelector.Namespace, groupWithSelector.Name),
+			},
 		},
 	}
 	for _, tt := range tests {

pkg/controller/networkpolicy/networkpolicy_controller.go

@@ -623,7 +623,7 @@ func (n *NetworkPolicyController) createAddressGroup(namespace string, podSelect
 	addressGroup := &antreatypes.AddressGroup{
 		UID:      types.UID(normalizedUID),
 		Name:     normalizedUID,
-		Selector: *groupSelector,
+		Selector: groupSelector,
 	}
 	return addressGroup
 }
@@ -1104,6 +1104,7 @@ func (n *NetworkPolicyController) syncAddressGroup(key string) error {
 		Name:         addressGroup.Name,
 		UID:          addressGroup.UID,
 		Selector:     addressGroup.Selector,
+		SourceGroup:  addressGroup.SourceGroup,
 		GroupMembers: memberSet,
 		SpanMeta:     antreatypes.SpanMeta{NodeNames: addrGroupNodeNames},
 	}
@@ -1123,17 +1124,23 @@ func (c *NetworkPolicyController) getNodeMemberSet(selector labels.Selector) con
 // getAddressGroupMemberSet knows how to construct a GroupMemberSet that contains
 // all the entities selected by an AddressGroup.
 func (n *NetworkPolicyController) getAddressGroupMemberSet(g *antreatypes.AddressGroup) controlplane.GroupMemberSet {
-	// Check if an internal Group object exists corresponding to this AddressGroup.
-	groupObj, found, _ := n.internalGroupStore.Get(g.Name)
-	if found {
-		// This AddressGroup is derived from a ClusterGroup.
-		// In case the ClusterGroup is defined by a mix of childGroup with selectors and
-		// childGroup with ipBlocks, this function only returns the aggregated GroupMemberSet
-		// computed from childGroup with selectors, as ipBlocks will be processed differently.
-		group := groupObj.(*antreatypes.Group)
-		members, _ := n.getInternalGroupMembers(group)
-		return members
+	// This AddressGroup is derived from a ClusterGroup/Group.
+	if g.SourceGroup != "" {
+		// Check if an internal Group object exists corresponding to this AddressGroup.
+		groupObj, found, _ := n.internalGroupStore.Get(g.SourceGroup)
+		if found {
+			// In case the ClusterGroup/Group is defined by a mix of childGroup with selectors and
+			// childGroup with ipBlocks, this function only returns the aggregated GroupMemberSet
+			// computed from childGroup with selectors, as ipBlocks will be processed differently.
+			group := groupObj.(*antreatypes.Group)
+			members, _ := n.getInternalGroupMembers(group)
+			return members
+		}
+		// The internal Group doesn't exist yet or has been deleted. The AddressGroup selects nothing at the moment.
+		// Once the internalGroup is created, the AddressGroup will be resynced.
+		return nil
 	}
+	// Selector can't be nil when it reaches here.
 	if g.Selector.NodeSelector != nil {
 		return n.getNodeMemberSet(g.Selector.NodeSelector)
 	}
@@ -1306,10 +1313,11 @@ func (n *NetworkPolicyController) syncAppliedToGroup(key string) error {
 		if err != nil {
 			klog.ErrorS(err, "Error when getting AppliedTo workloads for AppliedToGroup", "AppliedToGroup", appliedToGroup.Name)
 			updatedAppliedToGroup = &antreatypes.AppliedToGroup{
-				UID:       appliedToGroup.UID,
-				Name:      appliedToGroup.Name,
-				Selector:  appliedToGroup.Selector,
-				SyncError: err,
+				UID:         appliedToGroup.UID,
+				Name:        appliedToGroup.Name,
+				Selector:    appliedToGroup.Selector,
+				SourceGroup: appliedToGroup.SourceGroup,
+				SyncError:   err,
 			}
 		} else {
 			scheduledPodNum, scheduledExtEntityNum := 0, 0
@@ -1357,6 +1365,7 @@ func (n *NetworkPolicyController) syncAppliedToGroup(key string) error {
 				UID:               appliedToGroup.UID,
 				Name:              appliedToGroup.Name,
 				Selector:          appliedToGroup.Selector,
+				SourceGroup:       appliedToGroup.SourceGroup,
 				GroupMemberByNode: memberSetByNode,
 				SpanMeta:          antreatypes.SpanMeta{NodeNames: appGroupNodeNames},
 			}
@@ -1373,14 +1382,20 @@ func (n *NetworkPolicyController) syncAppliedToGroup(key string) error {
 // getAppliedToWorkloads returns a list of workloads (Pods, ExternalEntities or Nodes) selected by an AppliedToGroup
 // for standalone selectors or Pods and ExternalEntities corresponding to a ClusterGroup.
 func (n *NetworkPolicyController) getAppliedToWorkloads(g *antreatypes.AppliedToGroup) ([]*v1.Pod, []*v1alpha2.ExternalEntity, []*v1.Node, error) {
-	// Check if an internal Group object exists corresponding to this AppliedToGroup
-	group, found, _ := n.internalGroupStore.Get(g.Name)
-	if found {
-		// This AppliedToGroup is derived from a ClusterGroup.
-		grp := group.(*antreatypes.Group)
-		pods, ees, err := n.getInternalGroupWorkloads(grp)
-		return pods, ees, nil, err
+	// This AppliedToGroup is derived from a ClusterGroup/Group.
+	if g.SourceGroup != "" {
+		// Check if an internal Group object exists corresponding to this AppliedToGroup
+		group, found, _ := n.internalGroupStore.Get(g.SourceGroup)
+		if found {
+			grp := group.(*antreatypes.Group)
+			pods, ees, err := n.getInternalGroupWorkloads(grp)
+			return pods, ees, nil, err
+		}
+		// The internal Group doesn't exist yet or has been deleted. The AppliedToGroup selects nothing at the moment.
+		// Once the internalGroup is created, the AppliedToGroup will be resynced.
+		return nil, nil, nil, nil
 	}
+	// Selector can't be nil when it reaches here.
 	if g.Selector.NodeSelector != nil {
 		nodes, err := n.nodeLister.List(g.Selector.NodeSelector)
 		return nil, nil, nodes, err
@@ -1595,8 +1610,8 @@ func (n *NetworkPolicyController) syncInternalNetworkPolicy(key *controlplane.Ne
 			n.addressGroupStore.Create(addressGroup)
 			// For an AddressGroup that selects Nodes via nodeSelector, we calculate its members via NodeLister
 			// directly, instead of groupingInterface which handles Pod and ExternalEntity currently.
-			if addressGroup.Selector.NodeSelector == nil {
-				n.groupingInterface.AddGroup(addressGroupType, addressGroup.Name, &addressGroup.Selector)
+			if addressGroup.Selector != nil && addressGroup.Selector.NodeSelector == nil {
+				n.groupingInterface.AddGroup(addressGroupType, addressGroup.Name, addressGroup.Selector)
 			}
 		}