diff --git a/pkg/controller/networkpolicy/clustergroup.go b/pkg/controller/networkpolicy/clustergroup.go index 735b365c241..000759ff70d 100644 --- a/pkg/controller/networkpolicy/clustergroup.go +++ b/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) } } diff --git a/pkg/controller/networkpolicy/clusternetworkpolicy.go b/pkg/controller/networkpolicy/clusternetworkpolicy.go index 14d24e1417d..f711232bfa0 100644 --- a/pkg/controller/networkpolicy/clusternetworkpolicy.go +++ b/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 diff --git a/pkg/controller/networkpolicy/clusternetworkpolicy_test.go b/pkg/controller/networkpolicy/clusternetworkpolicy_test.go index 1a0d19e3921..3562c861196 100644 --- a/pkg/controller/networkpolicy/clusternetworkpolicy_test.go +++ b/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", diff --git a/pkg/controller/networkpolicy/crd_utils.go b/pkg/controller/networkpolicy/crd_utils.go index 801470383e4..f58c82ed51f 100644 --- a/pkg/controller/networkpolicy/crd_utils.go +++ b/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. diff --git a/pkg/controller/networkpolicy/crd_utils_test.go b/pkg/controller/networkpolicy/crd_utils_test.go index a572e347c28..40963a94596 100644 --- a/pkg/controller/networkpolicy/crd_utils_test.go +++ b/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 { diff --git a/pkg/controller/networkpolicy/networkpolicy_controller.go b/pkg/controller/networkpolicy/networkpolicy_controller.go index 3e83e2017f4..af1edbcf773 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller.go +++ b/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) } } diff --git a/pkg/controller/networkpolicy/networkpolicy_controller_test.go b/pkg/controller/networkpolicy/networkpolicy_controller_test.go index 9459934d557..2ee35d67d12 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller_test.go +++ b/pkg/controller/networkpolicy/networkpolicy_controller_test.go @@ -41,6 +41,7 @@ import ( k8stesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" + "k8s.io/utils/pointer" fakepolicyversioned "sigs.k8s.io/network-policy-api/pkg/client/clientset/versioned/fake" policyv1a1informers "sigs.k8s.io/network-policy-api/pkg/client/informers/externalversions" @@ -2602,8 +2603,9 @@ func TestGetAppliedToWorkloads(t *testing.T) { { name: "atg-for-cg", inATG: &antreatypes.AppliedToGroup{ - Name: cgA.Name, - UID: cgA.UID, + Name: cgA.Name, + UID: cgA.UID, + SourceGroup: cgA.Name, }, expPods: []*corev1.Pod{podA}, expEEs: emptyEEs, @@ -2612,8 +2614,9 @@ func TestGetAppliedToWorkloads(t *testing.T) { { name: "atg-for-cg-no-pod-match", inATG: &antreatypes.AppliedToGroup{ - Name: cgB.Name, - UID: cgB.UID, + Name: cgB.Name, + UID: cgB.UID, + SourceGroup: cgB.Name, }, expPods: emptyPods, expEEs: emptyEEs, @@ -2622,8 +2625,9 @@ func TestGetAppliedToWorkloads(t *testing.T) { { name: "atg-for-nested-cg-one-child-empty", inATG: &antreatypes.AppliedToGroup{ - Name: nestedCG1.Name, - UID: nestedCG1.UID, + Name: nestedCG1.Name, + UID: nestedCG1.UID, + SourceGroup: nestedCG1.Name, }, expPods: []*corev1.Pod{podA}, expEEs: emptyEEs, @@ -2632,8 +2636,9 @@ func TestGetAppliedToWorkloads(t *testing.T) { { name: "atg-for-nested-cg-both-children-match-pod", inATG: &antreatypes.AppliedToGroup{ - Name: nestedCG2.Name, - UID: nestedCG2.UID, + Name: nestedCG2.Name, + UID: nestedCG2.UID, + SourceGroup: nestedCG2.Name, }, expPods: []*corev1.Pod{podA, podB}, expEEs: emptyEEs, @@ -2642,8 +2647,9 @@ func TestGetAppliedToWorkloads(t *testing.T) { { name: "atg-for-nested-cg-children-overlap-pod", inATG: &antreatypes.AppliedToGroup{ - Name: nestedCG3.Name, - UID: nestedCG3.UID, + Name: nestedCG3.Name, + UID: nestedCG3.UID, + SourceGroup: nestedCG3.Name, }, expPods: []*corev1.Pod{podA, podB}, expEEs: emptyEEs, @@ -2747,40 +2753,45 @@ func TestGetAddressGroupMemberSet(t *testing.T) { { name: "addrgrp-for-cg", inAddrGrp: &antreatypes.AddressGroup{ - Name: cgA.Name, - UID: cgA.UID, + Name: cgA.Name, + UID: cgA.UID, + SourceGroup: cgA.Name, }, expMemberSet: podAMemberSet, }, { name: "addrgrp-for-cg-no-pod-match", inAddrGrp: &antreatypes.AddressGroup{ - Name: cgB.Name, - UID: cgB.UID, + Name: cgB.Name, + UID: cgB.UID, + SourceGroup: cgB.Name, }, expMemberSet: controlplane.GroupMemberSet{}, }, { name: "addrgrp-for-nested-cg-one-child-empty", inAddrGrp: &antreatypes.AddressGroup{ - Name: nestedCG1.Name, - UID: nestedCG1.UID, + Name: nestedCG1.Name, + UID: nestedCG1.UID, + SourceGroup: nestedCG1.Name, }, expMemberSet: podAMemberSet, }, { name: "addrgrp-for-nested-cg-both-children-match-pod", inAddrGrp: &antreatypes.AddressGroup{ - Name: nestedCG2.Name, - UID: nestedCG2.UID, + Name: nestedCG2.Name, + UID: nestedCG2.UID, + SourceGroup: nestedCG2.Name, }, expMemberSet: podABMemberSet, }, { name: "addrgrp-for-nested-cg-children-overlap-pod", inAddrGrp: &antreatypes.AddressGroup{ - Name: nestedCG3.Name, - UID: nestedCG3.UID, + Name: nestedCG3.Name, + UID: nestedCG3.UID, + SourceGroup: nestedCG3.Name, }, expMemberSet: podABMemberSet, }, @@ -3037,7 +3048,7 @@ func TestMultipleNetworkPoliciesWithSameAppliedTo(t *testing.T) { SpanMeta: antreatypes.SpanMeta{NodeNames: sets.New[string]("nodeA", "nodeB")}, // according to policyA and policyB UID: types.UID(selectorBGroupUID), Name: selectorBGroupUID, - Selector: *selectorBGroup, + Selector: selectorBGroup, GroupMembers: controlplane.NewGroupMemberSet(&controlplane.GroupMember{Pod: &controlplane.PodReference{ Name: podC.Name, Namespace: podC.Namespace, @@ -3166,7 +3177,6 @@ func checkAddressGroupNotExist(t *testing.T, c *networkPolicyController, address func TestSyncInternalNetworkPolicy(t *testing.T) { p10 := float64(10) - allowAction := v1beta1.RuleActionAllow inputPolicy := &v1beta1.ClusterNetworkPolicy{ ObjectMeta: metav1.ObjectMeta{Name: "cnpA", UID: "uidA"}, Spec: v1beta1.ClusterNetworkPolicySpec{ @@ -3490,7 +3500,6 @@ func TestSyncInternalNetworkPolicyConcurrently(t *testing.T) { func TestSyncInternalNetworkPolicyWithGroups(t *testing.T) { p10 := float64(10) - allowAction := v1beta1.RuleActionAllow podA := getPod("podA", "nsA", "nodeA", "10.0.0.1", false) podA.Labels = selectorA.MatchLabels podB := getPod("podB", "nsB", "nodeB", "10.0.0.2", false) @@ -3888,6 +3897,151 @@ func TestSyncAppliedToGroupWithNode(t *testing.T) { assert.Equal(t, expectedAppliedToGroup, gotAppliedToGroup) } +func TestClusterNetworkPolicyWithClusterGroup(t *testing.T) { + ctx := context.TODO() + podA := getPod("podA", "nsA", "nodeA", "10.0.0.1", false) + podA.Labels = map[string]string{"fooA": "barA"} + podB := getPod("podB", "nsB", "nodeB", "10.0.0.2", false) + podB.Labels = map[string]string{"fooB": "barB"} + + cgA := &v1beta1.ClusterGroup{ + ObjectMeta: metav1.ObjectMeta{Name: "cgA", UID: "cgA-uid"}, + Spec: v1beta1.GroupSpec{PodSelector: &metav1.LabelSelector{MatchLabels: podA.Labels}}, + } + cgB := &v1beta1.ClusterGroup{ + ObjectMeta: metav1.ObjectMeta{Name: "cgB", UID: "cgB-uid"}, + Spec: v1beta1.GroupSpec{PodSelector: &metav1.LabelSelector{MatchLabels: podB.Labels}}, + } + acnp := &v1beta1.ClusterNetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "acnpA", UID: "acnpA-uid"}, + Spec: v1beta1.ClusterNetworkPolicySpec{ + AppliedTo: []v1beta1.AppliedTo{{Group: cgA.Name}}, + Priority: 10, + Ingress: []v1beta1.Rule{ + {From: []v1beta1.NetworkPolicyPeer{{Group: cgB.Name}}, Action: &allowAction}, + }, + }, + } + + _, npc := newController([]runtime.Object{podA, podB}, []runtime.Object{cgA, cgB, acnp}) + stopCh := make(chan struct{}) + defer close(stopCh) + npc.informerFactory.Start(stopCh) + npc.informerFactory.WaitForCacheSync(stopCh) + npc.crdInformerFactory.Start(stopCh) + npc.crdInformerFactory.WaitForCacheSync(stopCh) + go npc.Run(stopCh) + go npc.groupingController.Run(stopCh) + go npc.groupingInterface.Run(stopCh) + + expectedPolicy := &antreatypes.NetworkPolicy{ + SpanMeta: antreatypes.SpanMeta{NodeNames: sets.New(podA.Spec.NodeName)}, + UID: acnp.UID, + Name: string(acnp.UID), + SourceRef: &controlplane.NetworkPolicyReference{Type: controlplane.AntreaClusterNetworkPolicy, Name: acnp.Name, UID: acnp.UID}, + Priority: pointer.Float64(acnp.Spec.Priority), + Rules: []controlplane.NetworkPolicyRule{ + { + Direction: controlplane.DirectionIn, + From: controlplane.NetworkPolicyPeer{AddressGroups: []string{cgB.Name}}, + Action: &allowAction, + }, + }, + AppliedToGroups: []string{cgA.Name}, + TierPriority: &DefaultTierPriority, + } + expectedAppliedToGroup := &antreatypes.AppliedToGroup{ + SpanMeta: antreatypes.SpanMeta{NodeNames: sets.New(podA.Spec.NodeName)}, + UID: cgA.UID, + Name: cgA.Name, + SourceGroup: cgA.Name, + GroupMemberByNode: map[string]controlplane.GroupMemberSet{ + podA.Spec.NodeName: controlplane.NewGroupMemberSet(&controlplane.GroupMember{ + Pod: &controlplane.PodReference{Name: podA.Name, Namespace: podA.Namespace}, + }), + }, + } + expectedAddressGroup := &antreatypes.AddressGroup{ + SpanMeta: antreatypes.SpanMeta{NodeNames: sets.New(podA.Spec.NodeName)}, + UID: cgB.UID, + Name: cgB.Name, + SourceGroup: cgB.Name, + GroupMembers: controlplane.NewGroupMemberSet(&controlplane.GroupMember{ + Pod: &controlplane.PodReference{Name: podB.Name, Namespace: podB.Namespace}, + IPs: []controlplane.IPAddress{ipStrToIPAddress(podB.Status.PodIP)}, + }), + } + + checkResources := func(policy *antreatypes.NetworkPolicy, atg *antreatypes.AppliedToGroup, ag *antreatypes.AddressGroup) { + require.EventuallyWithT(t, func(c *assert.CollectT) { + policies := npc.internalNetworkPolicyStore.List() + if !assert.Len(c, policies, 1) { + return + } + assert.Equal(c, policy, policies[0].(*antreatypes.NetworkPolicy)) + + atgs := npc.appliedToGroupStore.List() + if atg != nil { + if !assert.Len(c, atgs, 1) { + return + } + assert.Equal(c, atg, atgs[0].(*antreatypes.AppliedToGroup)) + } else { + assert.Empty(c, atgs) + } + + ags := npc.addressGroupStore.List() + if ag != nil { + if !assert.Len(c, ags, 1) { + return + } + assert.Equal(c, ag, ags[0].(*antreatypes.AddressGroup)) + } else { + assert.Empty(c, ags) + } + }, 2*time.Second, 50*time.Millisecond) + } + checkResources(expectedPolicy, expectedAppliedToGroup, expectedAddressGroup) + + // Delete the ClusterGroup used by the AddressGroup, the AddressGroup should be deleted, the rule's peer should be empty. + npc.crdClient.CrdV1beta1().ClusterGroups().Delete(ctx, cgB.Name, metav1.DeleteOptions{}) + expectedPolicy2 := &antreatypes.NetworkPolicy{ + SpanMeta: antreatypes.SpanMeta{NodeNames: sets.New(podA.Spec.NodeName)}, + UID: acnp.UID, + Name: string(acnp.UID), + SourceRef: &controlplane.NetworkPolicyReference{Type: controlplane.AntreaClusterNetworkPolicy, Name: acnp.Name, UID: acnp.UID}, + Priority: pointer.Float64(acnp.Spec.Priority), + Rules: []controlplane.NetworkPolicyRule{ + {Direction: controlplane.DirectionIn, Action: &allowAction}, + }, + AppliedToGroups: []string{cgA.Name}, + TierPriority: &DefaultTierPriority, + } + checkResources(expectedPolicy2, expectedAppliedToGroup, nil) + + // Delete the ClusterGroup used by the AppliedToGroup, the AppliedToGroup should be deleted, the policy's span and + // appliedToGroup should be empty. + npc.crdClient.CrdV1beta1().ClusterGroups().Delete(ctx, cgA.Name, metav1.DeleteOptions{}) + expectedPolicy3 := &antreatypes.NetworkPolicy{ + SpanMeta: antreatypes.SpanMeta{NodeNames: sets.New[string]()}, + UID: acnp.UID, + Name: string(acnp.UID), + SourceRef: &controlplane.NetworkPolicyReference{Type: controlplane.AntreaClusterNetworkPolicy, Name: acnp.Name, UID: acnp.UID}, + Priority: pointer.Float64(acnp.Spec.Priority), + Rules: []controlplane.NetworkPolicyRule{ + {Direction: controlplane.DirectionIn, Action: &allowAction}, + }, + AppliedToGroups: []string{}, + TierPriority: &DefaultTierPriority, + } + checkResources(expectedPolicy3, nil, nil) + + // Recreate the ClusterGroups, everything should be restored. + npc.crdClient.CrdV1beta1().ClusterGroups().Create(ctx, cgA, metav1.CreateOptions{}) + npc.crdClient.CrdV1beta1().ClusterGroups().Create(ctx, cgB, metav1.CreateOptions{}) + checkResources(expectedPolicy, expectedAppliedToGroup, expectedAddressGroup) +} + func checkQueueItemExistence(t *testing.T, queue workqueue.RateLimitingInterface, items ...string) { require.Equal(t, len(items), queue.Len()) expectedItems := sets.New[string](items...) diff --git a/pkg/controller/networkpolicy/store/addressgroup.go b/pkg/controller/networkpolicy/store/addressgroup.go index aa0f8c4e2d4..cbb878f704f 100644 --- a/pkg/controller/networkpolicy/store/addressgroup.go +++ b/pkg/controller/networkpolicy/store/addressgroup.go @@ -157,7 +157,7 @@ func NewAddressGroupStore() storage.Interface { indexers := cache.Indexers{ cache.NamespaceIndex: func(obj interface{}) ([]string, error) { ag, ok := obj.(*types.AddressGroup) - if !ok { + if !ok || ag.Selector == nil { return []string{}, nil } // ag.Selector.Namespace == "" means it's a cluster scoped group, we index it as it is. @@ -165,11 +165,18 @@ func NewAddressGroupStore() storage.Interface { }, IsNodeAddressGroupIndex: func(obj interface{}) ([]string, error) { ag, ok := obj.(*types.AddressGroup) - if !ok || ag.Selector.NodeSelector == nil { + if !ok || ag.Selector == nil || ag.Selector.NodeSelector == nil { return []string{}, nil } return []string{"true"}, nil }, + SourceGroupIndex: func(obj interface{}) ([]string, error) { + atg, ok := obj.(*types.AddressGroup) + if !ok || atg.SourceGroup == "" { + return []string{}, nil + } + return []string{atg.SourceGroup}, nil + }, } return ram.NewStore(AddressGroupKeyFunc, indexers, genAddressGroupEvent, keyAndSpanSelectFunc, func() runtime.Object { return new(controlplane.AddressGroup) }) } diff --git a/pkg/controller/networkpolicy/store/appliedtogroup.go b/pkg/controller/networkpolicy/store/appliedtogroup.go index d4a9ae44f21..86c98861d42 100644 --- a/pkg/controller/networkpolicy/store/appliedtogroup.go +++ b/pkg/controller/networkpolicy/store/appliedtogroup.go @@ -28,7 +28,10 @@ import ( "antrea.io/antrea/pkg/controller/types" ) -const IsAppliedToServiceIndex = "isAppliedToService" +const ( + IsAppliedToServiceIndex = "isAppliedToService" + SourceGroupIndex = "sourceGroup" +) // appliedToGroupEvent implements storage.InternalEvent. type appliedToGroupEvent struct { @@ -185,6 +188,13 @@ func NewAppliedToGroupStore() storage.Interface { } return []string{"true"}, nil }, + SourceGroupIndex: func(obj interface{}) ([]string, error) { + atg, ok := obj.(*types.AppliedToGroup) + if !ok || atg.SourceGroup == "" { + return []string{}, nil + } + return []string{atg.SourceGroup}, nil + }, } return ram.NewStore(AppliedToGroupKeyFunc, indexers, genAppliedToGroupEvent, keyAndSpanSelectFunc, func() runtime.Object { return new(controlplane.AppliedToGroup) }) } diff --git a/pkg/controller/types/networkpolicy.go b/pkg/controller/types/networkpolicy.go index 534e099a7db..86ca232ae2a 100644 --- a/pkg/controller/types/networkpolicy.go +++ b/pkg/controller/types/networkpolicy.go @@ -41,18 +41,24 @@ func (meta *SpanMeta) Has(nodeName string) bool { type AppliedToGroup struct { SpanMeta // If the AppliedToGroup is created from GroupSelector, UID is generated from the hash value of GroupSelector.NormalizedName. - // If the AppliedToGroup is created for a ClusterGroup, the UID is that of the corresponding ClusterGroup. + // If the AppliedToGroup is created for a ClusterGroup/Group, the UID is that of the corresponding ClusterGroup/Group. // If the AppliedToGroup is created for a Service, the UID is generated from the hash value of NamespacedName of the Service. UID types.UID - // Name of this group, currently it's same as UID. + // In case the AddressGroup is created for a ClusterGroup, it's the Name of the corresponding ClusterGroup. + // In case the AddressGroup is created for a Group, it's the Namespace/Name of the corresponding Group. + // Otherwise, it's same as UID. Name string - // Selector describes how the group selects pods. - // Selector can't be used with Service. + + // Selector, Service, and SourceGroup are mutually exclusive ways of selecting GroupMembers for the AppliedToGroup. + // For any AppliedToGroup, only one must be set. + // Selector describes how the group selects pods using selector. Selector *GroupSelector // Service refers to the Service this group selects. Only a NodePort type Service // can be referred by this field. - // Service can't be used with Selector. Service *controlplane.ServiceReference + // SourceGroup refers to the ClusterGroup or Group the AppliedToGroup is derived from. + SourceGroup string + // GroupMemberByNode is a mapping from nodeName to a set of GroupMembers on the Node, // either GroupMembers or ExternalEntity on the external node. // It will be converted to a slice of GroupMember for transferring according @@ -65,14 +71,21 @@ type AppliedToGroup struct { // AddressGroup describes a set of addresses used as source or destination of Network Policy rules. type AddressGroup struct { SpanMeta - // UID is generated from the hash value of GroupSelector.NormalizedName. - // In case the AddressGroup is created for a ClusterGroup, the UID is - // that of the corresponding ClusterGroup. + // If the AddressGroup is created from GroupSelector, UID is generated from the hash value of GroupSelector.NormalizedName. + // If the AddressGroup is created for a ClusterGroup/Group, the UID is that of the corresponding ClusterGroup/Group. UID types.UID - // Name of this group, currently it's same as UID. + // In case the AddressGroup is created for a ClusterGroup, it's the Name of the corresponding ClusterGroup. + // In case the AddressGroup is created for a Group, it's the Namespace/Name of the corresponding ClusterGroup. + // Otherwise, it's same as UID. Name string + + // Selector and SourceGroup are mutually exclusive ways of selecting GroupMembers for the AddressGroup. + // For any AddressGroup, only one must be set. // Selector describes how the group selects pods to get their addresses. - Selector GroupSelector + Selector *GroupSelector + // SourceGroup refers to the ClusterGroup or Group the AddressGroup is derived from. + SourceGroup string + // GroupMembers is a set of GroupMembers selected by this group. // It will be converted to a slice of GroupMember for transferring according // to client's selection.