diff --git a/cmd/antrea-controller/controller.go b/cmd/antrea-controller/controller.go index c28f19e02ac..1c800284e5a 100644 --- a/cmd/antrea-controller/controller.go +++ b/cmd/antrea-controller/controller.go @@ -119,6 +119,7 @@ func run(o *Options) error { tfInformer := crdInformerFactory.Crd().V1alpha1().Traceflows() cgv1a2Informer := crdInformerFactory.Crd().V1alpha2().ClusterGroups() cgInformer := crdInformerFactory.Crd().V1alpha3().ClusterGroups() + gInformer := crdInformerFactory.Crd().V1alpha3().Groups() egressInformer := crdInformerFactory.Crd().V1alpha2().Egresses() externalIPPoolInformer := crdInformerFactory.Crd().V1alpha2().ExternalIPPools() @@ -160,6 +161,7 @@ func run(o *Options) error { anpInformer, tierInformer, cgInformer, + gInformer, addressGroupStore, appliedToGroupStore, networkPolicyStore, diff --git a/pkg/apis/controlplane/helper.go b/pkg/apis/controlplane/helper.go index 7f73cbd0580..d1cf739927f 100644 --- a/pkg/apis/controlplane/helper.go +++ b/pkg/apis/controlplane/helper.go @@ -23,3 +23,19 @@ func (r *NetworkPolicyReference) ToString() string { return fmt.Sprintf("%s:%s/%s", r.Type, r.Namespace, r.Name) } } + +func (r *GroupReference) ToString() string { + if r.Namespace == "" { + return r.Name + } + return fmt.Sprintf("%s/%s", r.Namespace, r.Name) +} + +// ToTypedString returns the Group or ClusterGroup namespaced name as a string along with its type. +// Typical usage of typed string will be to use in log messages. +func (r *GroupReference) ToTypedString() string { + if r.Namespace == "" { + return fmt.Sprintf("ClusterGroup: %s", r.Name) + } + return fmt.Sprintf("Group: %s/%s", r.Namespace, r.Name) +} diff --git a/pkg/apis/controlplane/helper_test.go b/pkg/apis/controlplane/helper_test.go new file mode 100644 index 00000000000..a521e653e40 --- /dev/null +++ b/pkg/apis/controlplane/helper_test.go @@ -0,0 +1,115 @@ +// Copyright 2021 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 + +import ( + "github.com/stretchr/testify/assert" + "testing" +) + +func TestGroupReferenceToString(t *testing.T) { + tests := []struct { + name string + inGroupRef *GroupReference + out string + }{ + { + name: "cg-ref", + inGroupRef: &GroupReference{ + Namespace: "", + Name: "cgA", + }, + out: "cgA", + }, + { + name: "g-ref", + inGroupRef: &GroupReference{ + Namespace: "nsA", + Name: "gA", + }, + out: "nsA/gA", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actualOut := tt.inGroupRef.ToString() + assert.Equal(t, tt.out, actualOut) + }) + } +} + +func TestGroupReferenceToTypedString(t *testing.T) { + tests := []struct { + name string + inGroupRef *GroupReference + out string + }{ + { + name: "cg-ref", + inGroupRef: &GroupReference{ + Namespace: "", + Name: "cgA", + }, + out: "ClusterGroup: cgA", + }, + { + name: "g-ref", + inGroupRef: &GroupReference{ + Namespace: "nsA", + Name: "gA", + }, + out: "Group: nsA/gA", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actualOut := tt.inGroupRef.ToTypedString() + assert.Equal(t, tt.out, actualOut) + }) + } +} + +func TestNetworkPolicyReferenceToString(t *testing.T) { + tests := []struct { + name string + inNPRef *NetworkPolicyReference + out string + }{ + { + name: "acnp-ref", + inNPRef: &NetworkPolicyReference{ + Type: AntreaClusterNetworkPolicy, + Namespace: "", + Name: "acnpA", + }, + out: "AntreaClusterNetworkPolicy:acnpA", + }, + { + name: "anp-ref", + inNPRef: &NetworkPolicyReference{ + Type: AntreaNetworkPolicy, + Namespace: "nsA", + Name: "anpA", + }, + out: "AntreaNetworkPolicy:nsA/anpA", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actualOut := tt.inNPRef.ToString() + assert.Equal(t, tt.out, actualOut) + }) + } +} diff --git a/pkg/apis/crd/v1alpha3/register.go b/pkg/apis/crd/v1alpha3/register.go index f5fddd90c4e..a163d967408 100644 --- a/pkg/apis/crd/v1alpha3/register.go +++ b/pkg/apis/crd/v1alpha3/register.go @@ -46,6 +46,8 @@ func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, &ClusterGroup{}, &ClusterGroupList{}, + &Group{}, + &GroupList{}, ) metav1.AddToGroupVersion(scheme, SchemeGroupVersion) diff --git a/pkg/apis/crd/v1alpha3/types.go b/pkg/apis/crd/v1alpha3/types.go index 235abe44b92..f897857dd66 100644 --- a/pkg/apis/crd/v1alpha3/types.go +++ b/pkg/apis/crd/v1alpha3/types.go @@ -98,9 +98,6 @@ type ServiceReference struct { // ClusterGroupReference represent reference to a ClusterGroup. type ClusterGroupReference string -// GroupReference represent reference to a Group. -type GroupReference string - // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object type ClusterGroupList struct { diff --git a/pkg/apiserver/registry/networkpolicy/groupassociation/rest.go b/pkg/apiserver/registry/networkpolicy/groupassociation/rest.go index 307e285be4b..f028834ec31 100644 --- a/pkg/apiserver/registry/networkpolicy/groupassociation/rest.go +++ b/pkg/apiserver/registry/networkpolicy/groupassociation/rest.go @@ -63,7 +63,7 @@ func (r *REST) Get(ctx context.Context, name string, options *metav1.GetOptions) items := make([]controlplane.GroupReference, 0, len(groups)) for i := range groups { item := controlplane.GroupReference{ - Name: groups[i].Name, + Name: groups[i].SourceReference.Name, UID: groups[i].UID, } items = append(items, item) diff --git a/pkg/apiserver/registry/networkpolicy/groupassociation/rest_test.go b/pkg/apiserver/registry/networkpolicy/groupassociation/rest_test.go index 57879223c50..2a77300dffb 100644 --- a/pkg/apiserver/registry/networkpolicy/groupassociation/rest_test.go +++ b/pkg/apiserver/registry/networkpolicy/groupassociation/rest_test.go @@ -44,18 +44,27 @@ func TestRESTGet(t *testing.T) { groups := map[string][]antreatypes.Group{ "default/podA": { { - UID: "groupUID1", - Name: "cg1", + UID: "groupUID1", + SourceReference: &controlplane.GroupReference{ + Name: "cg1", + UID: "groupUID1", + }, }, }, "default/podB": { { - UID: "groupUID2", - Name: "cg2", + UID: "groupUID2", + SourceReference: &controlplane.GroupReference{ + Name: "cg2", + UID: "groupUID2", + }, }, { - UID: "groupUID3", - Name: "cg3", + UID: "groupUID3", + SourceReference: &controlplane.GroupReference{ + Name: "cg3", + UID: "groupUID3", + }, }, }, } diff --git a/pkg/controller/networkpolicy/antreanetworkpolicy.go b/pkg/controller/networkpolicy/antreanetworkpolicy.go index 4b36190564d..bdb88d6241e 100644 --- a/pkg/controller/networkpolicy/antreanetworkpolicy.go +++ b/pkg/controller/networkpolicy/antreanetworkpolicy.go @@ -130,10 +130,8 @@ func (n *NetworkPolicyController) processAntreaNetworkPolicy(np *crdv1alpha1.Net // The span calculation and stale appliedToGroup cleanup logic would work seamlessly for both cases. appliedToGroupNamesSet := sets.String{} // Create AppliedToGroup for each AppliedTo present in AntreaNetworkPolicy spec. - for _, at := range np.Spec.AppliedTo { - appliedToGroupNamesSet.Insert(n.createAppliedToGroup( - np.Namespace, at.PodSelector, at.NamespaceSelector, at.ExternalEntitySelector)) - } + + n.processAppliedTo(np.Namespace, np.Spec.AppliedTo, appliedToGroupNamesSet) rules := make([]controlplane.NetworkPolicyRule, 0, len(np.Spec.Ingress)+len(np.Spec.Egress)) // Compute NetworkPolicyRule for Ingress Rule. for idx, ingressRule := range np.Spec.Ingress { @@ -141,11 +139,8 @@ func (n *NetworkPolicyController) processAntreaNetworkPolicy(np *crdv1alpha1.Net services, namedPortExists := toAntreaServicesForCRD(ingressRule.Ports) var appliedToGroupNamesForRule []string // Create AppliedToGroup for each AppliedTo present in the ingress rule. - for _, at := range ingressRule.AppliedTo { - atGroup := n.createAppliedToGroup(np.Namespace, at.PodSelector, at.NamespaceSelector, at.ExternalEntitySelector) - appliedToGroupNamesForRule = append(appliedToGroupNamesForRule, atGroup) - appliedToGroupNamesSet.Insert(atGroup) - } + atGroups := n.processAppliedTo(np.Namespace, ingressRule.AppliedTo, appliedToGroupNamesSet) + appliedToGroupNamesForRule = append(appliedToGroupNamesForRule, atGroups...) rules = append(rules, controlplane.NetworkPolicyRule{ Direction: controlplane.DirectionIn, From: *n.toAntreaPeerForCRD(ingressRule.From, np, controlplane.DirectionIn, namedPortExists), @@ -162,12 +157,9 @@ func (n *NetworkPolicyController) processAntreaNetworkPolicy(np *crdv1alpha1.Net // Set default action to ALLOW to allow traffic. services, namedPortExists := toAntreaServicesForCRD(egressRule.Ports) var appliedToGroupNamesForRule []string - // Create AppliedToGroup for each AppliedTo present in the ingress rule. - for _, at := range egressRule.AppliedTo { - atGroup := n.createAppliedToGroup(np.Namespace, at.PodSelector, at.NamespaceSelector, at.ExternalEntitySelector) - appliedToGroupNamesForRule = append(appliedToGroupNamesForRule, atGroup) - appliedToGroupNamesSet.Insert(atGroup) - } + // Create AppliedToGroup for each AppliedTo present in the egress rule. + atGroups := n.processAppliedTo(np.Namespace, egressRule.AppliedTo, appliedToGroupNamesSet) + appliedToGroupNamesForRule = append(appliedToGroupNamesForRule, atGroups...) rules = append(rules, controlplane.NetworkPolicyRule{ Direction: controlplane.DirectionOut, To: *n.toAntreaPeerForCRD(egressRule.To, np, controlplane.DirectionOut, namedPortExists), @@ -198,3 +190,46 @@ func (n *NetworkPolicyController) processAntreaNetworkPolicy(np *crdv1alpha1.Net } return internalNetworkPolicy } + +func (n *NetworkPolicyController) processAppliedTo(namespace string, appliedTo []crdv1alpha1.NetworkPolicyPeer, appliedToGroupNamesSet sets.String) []string { + var appliedToGroupNames []string + for _, at := range appliedTo { + var atg string + if at.Group != "" { + atg = n.processAppliedToGroupForG(namespace, at.Group) + } else { + atg = n.createAppliedToGroup(namespace, at.PodSelector, at.NamespaceSelector, at.ExternalEntitySelector) + } + if atg != "" { + appliedToGroupNames = append(appliedToGroupNames, atg) + appliedToGroupNamesSet.Insert(atg) + } + } + return appliedToGroupNames +} + +func (n *NetworkPolicyController) processAppliedToGroupForG(namespace, groupName string) string { + // Retrieve Group for corresponding entry in the AppliedToGroup. + g, err := n.gLister.Groups(namespace).Get(groupName) + if err != nil { + // This error should not occur as we validate that a Group must exist before + // referencing it in an ANP. + klog.Errorf("Group %s not found: %v", g, err) + return "" + } + key := internalGroupKeyFunc(g) + // Find the internal Group corresponding to this Group + ig, found, _ := n.internalGroupStore.Get(key) + if !found { + // Internal Group was not found. Once the internal Group is created, the sync + // worker for internal group will re-enqueue the ClusterNetworkPolicy processing + // which will trigger the creation of AddressGroup. + return "" + } + intGrp := ig.(*antreatypes.Group) + if len(intGrp.IPBlocks) > 0 || intGrp.Selector.NamespaceSelector != nil { + klog.V(2).Infof("Group %s with IPBlocks or NamespaceSelector will not be processed as AppliedTo", g) + return "" + } + return n.createAppliedToGroupForInternalGroup(intGrp) +} diff --git a/pkg/controller/networkpolicy/antreanetworkpolicy_test.go b/pkg/controller/networkpolicy/antreanetworkpolicy_test.go index d15a6a9fe0c..0d8a6c3f5c2 100644 --- a/pkg/controller/networkpolicy/antreanetworkpolicy_test.go +++ b/pkg/controller/networkpolicy/antreanetworkpolicy_test.go @@ -15,6 +15,8 @@ package networkpolicy import ( + crdv1alpha3 "antrea.io/antrea/pkg/apis/crd/v1alpha3" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -560,6 +562,78 @@ func TestDeleteANP(t *testing.T) { assert.False(t, found, "expected internal NetworkPolicy to be deleted") } +func TestProcessAppliedToGroupsForG(t *testing.T) { + selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}} + cidr := "10.0.0.0/24" + // gA with selector present in cache + gA := crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + PodSelector: &selectorA, + }, + } + // gB with IPBlock present in cache + gB := crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsB", Name: "gB", UID: "uidB"}, + Spec: crdv1alpha3.GroupSpec{ + IPBlocks: []crdv1alpha1.IPBlock{ + { + CIDR: cidr, + }, + }, + }, + } + // gC not found in cache + gC := crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsC", Name: "gC", UID: "uidC"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorA, + }, + } + _, npc := newController() + npc.addGroup(&gA) + npc.addGroup(&gB) + npc.gStore.Add(&gA) + npc.gStore.Add(&gB) + tests := []struct { + name string + namespace string + inputG string + expectedAG string + }{ + { + name: "empty-grp-no-result", + namespace: "nsA", + inputG: "", + expectedAG: "", + }, + { + name: "ipblock-grp-no-result", + namespace: "nsB", + inputG: gB.Name, + expectedAG: "", + }, + { + name: "selector-grp-missing-no-result", + namespace: "nsC", + inputG: gC.Name, + expectedAG: "", + }, + { + name: "selector-grp", + namespace: "nsA", + inputG: gA.Name, + expectedAG: fmt.Sprintf("%s/%s", gA.Namespace, gA.Name), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actualAG := npc.processAppliedToGroupForG(tt.namespace, tt.inputG) + assert.Equal(t, tt.expectedAG, actualAG, "appliedToGroup list does not match") + }) + } +} + // util functions for testing. func getANP() *crdv1alpha1.NetworkPolicy { p10 := float64(10) diff --git a/pkg/controller/networkpolicy/clustergroup.go b/pkg/controller/networkpolicy/clustergroup.go index 1f532ce2341..8c1c33c5484 100644 --- a/pkg/controller/networkpolicy/clustergroup.go +++ b/pkg/controller/networkpolicy/clustergroup.go @@ -118,8 +118,8 @@ func (n *NetworkPolicyController) deleteClusterGroup(oldObj interface{}) { func (n *NetworkPolicyController) processClusterGroup(cg *crdv1alpha3.ClusterGroup) *antreatypes.Group { internalGroup := antreatypes.Group{ - Name: cg.Name, - UID: cg.UID, + SourceReference: getClusterGroupSourceRef(cg), + UID: cg.UID, } if len(cg.Spec.ChildGroups) > 0 { for _, childCGName := range cg.Spec.ChildGroups { @@ -206,23 +206,27 @@ func (n *NetworkPolicyController) syncInternalGroup(key string) error { return nil } grp := grpObj.(*antreatypes.Group) + if grp.SourceReference.Namespace != "" { + // Sync the Group as a Namespaced Group. + return n.syncNamespacedInternalGroup(grp) + } // Retrieve the ClusterGroup corresponding to this key. - cg, err := n.cgLister.Get(grp.Name) + cg, err := n.cgLister.Get(grp.SourceReference.ToString()) if err != nil { - klog.Infof("Didn't find the ClusterGroup %s, skip processing of internal group", grp.Name) + klog.Infof("Didn't find the %s, skip processing of internal group", grp.SourceReference.ToTypedString()) return nil } selectorUpdated := n.processServiceReference(grp) if grp.Selector != nil { - n.groupingInterface.AddGroup(clusterGroupType, grp.Name, grp.Selector) + n.groupingInterface.AddGroup(clusterGroupType, grp.SourceReference.ToString(), grp.Selector) } else { - n.groupingInterface.DeleteGroup(clusterGroupType, grp.Name) + n.groupingInterface.DeleteGroup(clusterGroupType, grp.SourceReference.ToString()) } if selectorUpdated { // Update the internal Group object in the store with the new selector. updatedGrp := &antreatypes.Group{ UID: grp.UID, - Name: grp.Name, + SourceReference: grp.SourceReference, Selector: grp.Selector, ServiceReference: grp.ServiceReference, ChildGroups: grp.ChildGroups, @@ -232,7 +236,7 @@ func (n *NetworkPolicyController) syncInternalGroup(key string) error { } // Update the ClusterGroup status to Realized as Antrea has recognized the Group and // processed its group members. - err = n.updateGroupStatus(cg, v1.ConditionTrue) + err = n.updateClusterGroupStatus(cg, v1.ConditionTrue) if err != nil { klog.Errorf("Failed to update ClusterGroup %s GroupMembersComputed condition to %s: %v", cg.Name, v1.ConditionTrue, err) return err @@ -241,17 +245,25 @@ func (n *NetworkPolicyController) syncInternalGroup(key string) error { return n.triggerCNPUpdates(cg) } +func getClusterGroupSourceRef(cg *crdv1alpha3.ClusterGroup) *controlplane.GroupReference { + return &controlplane.GroupReference{ + Name: cg.GetName(), + Namespace: cg.GetNamespace(), + UID: cg.GetUID(), + } +} + func (n *NetworkPolicyController) triggerParentGroupSync(grp *antreatypes.Group) { // TODO: if the max supported group nesting level increases, a Group having children // will no longer be a valid indication that it cannot have parents. if len(grp.ChildGroups) == 0 { - parentGroupObjs, err := n.internalGroupStore.GetByIndex(store.ChildGroupIndex, grp.Name) + parentGroupObjs, err := n.internalGroupStore.GetByIndex(store.ChildGroupIndex, grp.SourceReference.ToString()) if err != nil { - klog.Errorf("Error retrieving parents of ClusterGroup %s: %v", grp.Name, err) + klog.Errorf("Error retrieving parents of %s: %v", grp.SourceReference.ToTypedString(), err) } for _, p := range parentGroupObjs { parentGrp := p.(*antreatypes.Group) - n.enqueueInternalGroup(parentGrp.Name) + n.enqueueInternalGroup(parentGrp.SourceReference.ToString()) } } } @@ -305,13 +317,13 @@ func (n *NetworkPolicyController) triggerCNPUpdates(cg *crdv1alpha3.ClusterGroup return nil } -// updateGroupStatus updates the Status subresource for a ClusterGroup. -func (n *NetworkPolicyController) updateGroupStatus(cg *crdv1alpha3.ClusterGroup, cStatus v1.ConditionStatus) error { +// updateClusterGroupStatus updates the Status subresource for a ClusterGroup. +func (n *NetworkPolicyController) updateClusterGroupStatus(cg *crdv1alpha3.ClusterGroup, cStatus v1.ConditionStatus) error { condStatus := crdv1alpha3.GroupCondition{ Status: cStatus, Type: crdv1alpha3.GroupMembersComputed, } - if groupMembersComputedConditionEqual(cg.Status.Conditions, condStatus) { + if clusterGroupMembersComputedConditionEqual(cg.Status.Conditions, condStatus) { // There is no change in conditions. return nil } @@ -326,9 +338,9 @@ func (n *NetworkPolicyController) updateGroupStatus(cg *crdv1alpha3.ClusterGroup return err } -// groupMembersComputedConditionEqual checks whether the condition status for GroupMembersComputed condition +// clusterGroupMembersComputedConditionEqual checks whether the condition status for GroupMembersComputed condition // is same. Returns true if equal, otherwise returns false. It disregards the lastTransitionTime field. -func groupMembersComputedConditionEqual(conds []crdv1alpha3.GroupCondition, condition crdv1alpha3.GroupCondition) bool { +func clusterGroupMembersComputedConditionEqual(conds []crdv1alpha3.GroupCondition, condition crdv1alpha3.GroupCondition) bool { for _, c := range conds { if c.Type == crdv1alpha3.GroupMembersComputed { if c.Status == condition.Status { @@ -350,7 +362,7 @@ func (n *NetworkPolicyController) processServiceReference(group *antreatypes.Gro originalSelectorName := getNormalizedNameForSelector(group.Selector) svc, err := n.serviceLister.Services(svcRef.Namespace).Get(svcRef.Name) if err != nil { - klog.V(2).Infof("Error getting Service object %s/%s: %v, setting empty selector for Group %s", svcRef.Namespace, svcRef.Name, err, group.Name) + klog.V(2).Infof("Error getting Service object %s/%s: %v, setting empty selector for %s", svcRef.Namespace, svcRef.Name, err, group.SourceReference.ToTypedString()) group.Selector = nil return originalSelectorName == getNormalizedNameForSelector(nil) } @@ -396,8 +408,8 @@ func (n *NetworkPolicyController) GetAssociatedGroups(name, namespace string) ([ // Remove duplicates in the groupObj slice. groupKeys, j := make(map[string]bool), 0 for _, g := range groupObjs { - if _, exists := groupKeys[g.Name]; !exists { - groupKeys[g.Name] = true + if _, exists := groupKeys[g.SourceReference.ToString()]; !exists { + groupKeys[g.SourceReference.ToString()] = true groupObjs[j] = g j++ } @@ -415,9 +427,9 @@ func (n *NetworkPolicyController) getAssociatedGroupsByName(grpName string) []an } grp := groupObj.(*antreatypes.Group) groups = append(groups, *grp) - parentGroupObjs, err := n.internalGroupStore.GetByIndex(store.ChildGroupIndex, grp.Name) + parentGroupObjs, err := n.internalGroupStore.GetByIndex(store.ChildGroupIndex, grp.SourceReference.ToString()) if err != nil { - klog.Errorf("Error retrieving parents of ClusterGroup %s: %v", grp.Name, err) + klog.Errorf("Error retrieving parents of %s: %v", grp.SourceReference.ToTypedString(), err) } for _, p := range parentGroupObjs { parentGrp := p.(*antreatypes.Group) diff --git a/pkg/controller/networkpolicy/clustergroup_test.go b/pkg/controller/networkpolicy/clustergroup_test.go index db55e3b9ea5..90c837a5b12 100644 --- a/pkg/controller/networkpolicy/clustergroup_test.go +++ b/pkg/controller/networkpolicy/clustergroup_test.go @@ -50,8 +50,11 @@ func TestProcessClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidA", - Name: "cgA", + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "cgA", + UID: "uidA", + }, Selector: toGroupSelector("", nil, &selectorA, nil), }, }, @@ -64,8 +67,11 @@ func TestProcessClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidB", - Name: "cgB", + UID: "uidB", + SourceReference: &controlplane.GroupReference{ + Name: "cgB", + UID: "uidB", + }, Selector: toGroupSelector("", &selectorB, nil, nil), }, }, @@ -79,8 +85,11 @@ func TestProcessClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidC", - Name: "cgC", + UID: "uidC", + SourceReference: &controlplane.GroupReference{ + Name: "cgC", + UID: "uidC", + }, Selector: toGroupSelector("", &selectorC, &selectorD, nil), }, }, @@ -97,8 +106,11 @@ func TestProcessClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidD", - Name: "cgD", + UID: "uidD", + SourceReference: &controlplane.GroupReference{ + Name: "cgD", + UID: "uidD", + }, IPBlocks: []controlplane.IPBlock{ { CIDR: *cidrIPNet, @@ -119,8 +131,11 @@ func TestProcessClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidE", - Name: "cgE", + UID: "uidE", + SourceReference: &controlplane.GroupReference{ + Name: "cgE", + UID: "uidE", + }, ServiceReference: &controlplane.ServiceReference{ Name: "test-svc", Namespace: "test-ns", @@ -136,8 +151,11 @@ func TestProcessClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidF", - Name: "cgF", + UID: "uidF", + SourceReference: &controlplane.GroupReference{ + Name: "cgF", + UID: "uidF", + }, ChildGroups: []string{"cgA", "cgB"}, }, }, @@ -172,8 +190,11 @@ func TestAddClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidA", - Name: "cgA", + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "cgA", + UID: "uidA", + }, Selector: toGroupSelector("", nil, &selectorA, nil), }, }, @@ -186,8 +207,11 @@ func TestAddClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidB", - Name: "cgB", + UID: "uidB", + SourceReference: &controlplane.GroupReference{ + Name: "cgB", + UID: "uidB", + }, Selector: toGroupSelector("", &selectorB, nil, nil), }, }, @@ -201,8 +225,11 @@ func TestAddClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidC", - Name: "cgC", + UID: "uidC", + SourceReference: &controlplane.GroupReference{ + Name: "cgC", + UID: "uidC", + }, Selector: toGroupSelector("", &selectorC, &selectorD, nil), }, }, @@ -219,8 +246,11 @@ func TestAddClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidD", - Name: "cgD", + UID: "uidD", + SourceReference: &controlplane.GroupReference{ + Name: "cgD", + UID: "uidD", + }, IPBlocks: []controlplane.IPBlock{ { CIDR: *cidrIPNet, @@ -269,8 +299,11 @@ func TestUpdateClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidA", - Name: "cgA", + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "cgA", + UID: "uidA", + }, Selector: toGroupSelector("", nil, &selectorB, nil), }, }, @@ -283,8 +316,11 @@ func TestUpdateClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidA", - Name: "cgA", + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "cgA", + UID: "uidA", + }, Selector: toGroupSelector("", &selectorC, nil, nil), }, }, @@ -298,8 +334,11 @@ func TestUpdateClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidA", - Name: "cgA", + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "cgA", + UID: "uidA", + }, Selector: toGroupSelector("", &selectorC, &selectorD, nil), }, }, @@ -316,8 +355,11 @@ func TestUpdateClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidA", - Name: "cgA", + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "cgA", + UID: "uidA", + }, IPBlocks: []controlplane.IPBlock{ { CIDR: *cidrIPNet, @@ -338,8 +380,11 @@ func TestUpdateClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidA", - Name: "cgA", + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "cgA", + UID: "uidA", + }, ServiceReference: &controlplane.ServiceReference{ Name: "test-svc", Namespace: "test-ns", @@ -355,8 +400,11 @@ func TestUpdateClusterGroup(t *testing.T) { }, }, expectedGroup: &antreatypes.Group{ - UID: "uidA", - Name: "cgA", + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "cgA", + UID: "uidA", + }, ChildGroups: []string{"cgB", "cgC"}, }, }, @@ -390,7 +438,7 @@ func TestDeleteCG(t *testing.T) { assert.False(t, found, "expected internal Group to be deleted") } -func TestGroupMembersComputedConditionEqual(t *testing.T) { +func TestClusterClusterGroupMembersComputedConditionEqual(t *testing.T) { tests := []struct { name string existingConds []crdv1alpha3.GroupCondition @@ -436,7 +484,7 @@ func TestGroupMembersComputedConditionEqual(t *testing.T) { Type: crdv1alpha3.GroupMembersComputed, Status: tt.checkStatus, } - actualValue := groupMembersComputedConditionEqual(tt.existingConds, inCond) + actualValue := clusterGroupMembersComputedConditionEqual(tt.existingConds, inCond) assert.Equal(t, tt.expValue, actualValue) }) } @@ -465,16 +513,22 @@ func TestFilterInternalGroupsForService(t *testing.T) { }, } grp1 := &antreatypes.Group{ - UID: "uid1", - Name: "cgA", + UID: "uid1", + SourceReference: &controlplane.GroupReference{ + Name: "cgA", + UID: "uid1", + }, ServiceReference: &controlplane.ServiceReference{ Name: "svc1", Namespace: metav1.NamespaceDefault, }, } grp2 := &antreatypes.Group{ - UID: "uid2", - Name: "cgB", + UID: "uid2", + SourceReference: &controlplane.GroupReference{ + Name: "cgB", + UID: "uid1", + }, ServiceReference: &controlplane.ServiceReference{ Name: "svc1", Namespace: metav1.NamespaceDefault, @@ -482,8 +536,11 @@ func TestFilterInternalGroupsForService(t *testing.T) { Selector: toGroupSelector(metav1.NamespaceDefault, &selectorSpec, nil, nil), } grp3 := &antreatypes.Group{ - UID: "uid3", - Name: "cgC", + UID: "uid3", + SourceReference: &controlplane.GroupReference{ + Name: "cgC", + UID: "uid3", + }, ServiceReference: &controlplane.ServiceReference{ Name: "svc2", Namespace: "test", @@ -492,8 +549,11 @@ func TestFilterInternalGroupsForService(t *testing.T) { Selector: toGroupSelector("test", nil, nil, nil), } grp4 := &antreatypes.Group{ - UID: "uid4", - Name: "cgD", + UID: "uid4", + SourceReference: &controlplane.GroupReference{ + Name: "cgD", + UID: "uid4", + }, ServiceReference: &controlplane.ServiceReference{ Name: "svc3", }, @@ -562,24 +622,33 @@ func TestServiceToGroupSelector(t *testing.T) { } grp1 := &antreatypes.Group{ - UID: "uid1", - Name: "cgA", + UID: "uid1", + SourceReference: &controlplane.GroupReference{ + Name: "cgA", + UID: "uid1", + }, ServiceReference: &controlplane.ServiceReference{ Name: "svc1", Namespace: metav1.NamespaceDefault, }, } grp2 := &antreatypes.Group{ - UID: "uid2", - Name: "cgB", + UID: "uid2", + SourceReference: &controlplane.GroupReference{ + Name: "cg2", + UID: "uidB", + }, ServiceReference: &controlplane.ServiceReference{ Name: "svc2", Namespace: "test", }, } grp3 := &antreatypes.Group{ - UID: "uid3", - Name: "cgC", + UID: "uid3", + SourceReference: &controlplane.GroupReference{ + Name: "cgC", + UID: "uid3", + }, ServiceReference: &controlplane.ServiceReference{ Name: "svc3", Namespace: "test", @@ -701,33 +770,51 @@ var externalEntities = []*crdv1alpha2.ExternalEntity{ var groups = []antreatypes.Group{ { - UID: "groupUID0", - Name: "group0", + UID: "groupUID0", + SourceReference: &controlplane.GroupReference{ + Name: "group0", + UID: "groupUID0", + }, Selector: antreatypes.NewGroupSelector("test-ns", &metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}, nil, nil), }, { - UID: "groupUID1", - Name: "group1", + UID: "groupUID1", + SourceReference: &controlplane.GroupReference{ + Name: "group1", + UID: "groupUID1", + }, Selector: antreatypes.NewGroupSelector("test-ns", nil, nil, nil), }, { - UID: "groupUID2", - Name: "group2", + UID: "groupUID2", + SourceReference: &controlplane.GroupReference{ + Name: "group2", + UID: "groupUID2", + }, Selector: antreatypes.NewGroupSelector("test-ns", &metav1.LabelSelector{MatchLabels: map[string]string{"app": "other"}}, nil, nil), }, { - UID: "groupUID3", - Name: "group3", + UID: "groupUID3", + SourceReference: &controlplane.GroupReference{ + Name: "group3", + UID: "groupUID3", + }, ChildGroups: []string{"group0", "group1"}, }, { - UID: "groupUID4", - Name: "group4", + UID: "groupUID4", + SourceReference: &controlplane.GroupReference{ + Name: "group4", + UID: "groupUID4", + }, ChildGroups: []string{"group0", "group2"}, }, { - UID: "groupUID5", - Name: "group5", + UID: "groupUID5", + SourceReference: &controlplane.GroupReference{ + Name: "group5", + UID: "groupUID5", + }, ChildGroups: []string{"group1", "group2"}, }, } @@ -774,7 +861,7 @@ func TestGetAssociatedGroups(t *testing.T) { for i, g := range tt.existingGroups { npc.internalGroupStore.Create(&tt.existingGroups[i]) if g.Selector != nil { - npc.groupingInterface.AddGroup(clusterGroupType, g.Name, g.Selector) + npc.groupingInterface.AddGroup(clusterGroupType, g.SourceReference.Name, g.Selector) } } groups, err := npc.GetAssociatedGroups(tt.queryName, tt.queryNamespace) @@ -821,10 +908,36 @@ func TestGetGroupMembers(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { npc.internalGroupStore.Create(&tt.group) - npc.groupingInterface.AddGroup(clusterGroupType, tt.group.Name, tt.group.Selector) - members, err := npc.GetGroupMembers(tt.group.Name) + npc.groupingInterface.AddGroup(clusterGroupType, tt.group.SourceReference.Name, tt.group.Selector) + members, err := npc.GetGroupMembers(tt.group.SourceReference.Name) assert.Equal(t, nil, err) assert.Equal(t, tt.expectedMembers, members) }) } } + +func TestGetClusterGroupSourceRef(t *testing.T) { + tests := []struct { + name string + group *crdv1alpha3.ClusterGroup + expectedRef *controlplane.GroupReference + }{ + { + name: "cg-ref", + group: &crdv1alpha3.ClusterGroup{ + ObjectMeta: metav1.ObjectMeta{Name: "cgA", UID: "uidA"}, + }, + expectedRef: &controlplane.GroupReference{ + Name: "cgA", + Namespace: "", + UID: "uidA", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actualRef := getClusterGroupSourceRef(tt.group) + assert.Equal(t, tt.expectedRef, actualRef) + }) + } +} diff --git a/pkg/controller/networkpolicy/clusternetworkpolicy.go b/pkg/controller/networkpolicy/clusternetworkpolicy.go index d97a01afce6..ec17afc9cd1 100644 --- a/pkg/controller/networkpolicy/clusternetworkpolicy.go +++ b/pkg/controller/networkpolicy/clusternetworkpolicy.go @@ -482,35 +482,6 @@ func getUniqueNSSelectors(selectors []labels.Selector) []labels.Selector { return selectors[:i] } -// processRefCG processes the ClusterGroup reference present in the rule and returns the -// NetworkPolicyPeer with the corresponding AddressGroup or IPBlock. -func (n *NetworkPolicyController) processRefCG(g string) (string, []controlplane.IPBlock) { - // Retrieve ClusterGroup for corresponding entry in the rule. - cg, err := n.cgLister.Get(g) - if err != nil { - // This error should not occur as we validate that a CG must exist before - // referencing it in an ACNP. - klog.Errorf("ClusterGroup %s not found: %v", g, err) - return "", nil - } - key := internalGroupKeyFunc(cg) - // Find the internal Group corresponding to this ClusterGroup - ig, found, _ := n.internalGroupStore.Get(key) - if !found { - // Internal Group was not found. Once the internal Group is created, the sync - // worker for internal group will re-enqueue the ClusterNetworkPolicy processing - // which will trigger the creation of AddressGroup. - return "", nil - } - intGrp := ig.(*antreatypes.Group) - if len(intGrp.IPBlocks) > 0 { - return "", intGrp.IPBlocks - } - agKey := n.createAddressGroupForClusterGroupCRD(intGrp) - // Return if addressGroup was created or found. - return agKey, nil -} - func (n *NetworkPolicyController) processAppliedToGroupForCG(g string) string { // Retrieve ClusterGroup for corresponding entry in the AppliedToGroup. cg, err := n.cgLister.Get(g) @@ -534,5 +505,5 @@ func (n *NetworkPolicyController) processAppliedToGroupForCG(g string) string { klog.V(2).Infof("ClusterGroup %s with IPBlocks will not be processed as AppliedTo", g) return "" } - return n.createAppliedToGroupForClusterGroupCRD(intGrp) + return n.createAppliedToGroupForInternalGroup(intGrp) } diff --git a/pkg/controller/networkpolicy/clusternetworkpolicy_test.go b/pkg/controller/networkpolicy/clusternetworkpolicy_test.go index 9647f40a0ed..e7ff386a1c5 100644 --- a/pkg/controller/networkpolicy/clusternetworkpolicy_test.go +++ b/pkg/controller/networkpolicy/clusternetworkpolicy_test.go @@ -1486,85 +1486,6 @@ func TestGetTierPriority(t *testing.T) { } } -func TestProcessRefCG(t *testing.T) { - selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}} - cidr := "10.0.0.0/24" - cidrIPNet, _ := cidrStrToIPNet(cidr) - // cgA with selector present in cache - cgA := crdv1alpha3.ClusterGroup{ - ObjectMeta: metav1.ObjectMeta{Name: "cgA", UID: "uidA"}, - Spec: crdv1alpha3.GroupSpec{ - NamespaceSelector: &selectorA, - }, - } - // cgB with IPBlock present in cache - cgB := crdv1alpha3.ClusterGroup{ - ObjectMeta: metav1.ObjectMeta{Name: "cgB", UID: "uidB"}, - Spec: crdv1alpha3.GroupSpec{ - IPBlocks: []crdv1alpha1.IPBlock{ - { - CIDR: cidr, - }, - }, - }, - } - // cgC not found in cache - cgC := crdv1alpha3.ClusterGroup{ - ObjectMeta: metav1.ObjectMeta{Name: "cgC", UID: "uidC"}, - Spec: crdv1alpha3.GroupSpec{ - NamespaceSelector: &selectorA, - }, - } - _, npc := newController() - npc.addClusterGroup(&cgA) - npc.addClusterGroup(&cgB) - npc.cgStore.Add(&cgA) - npc.cgStore.Add(&cgB) - tests := []struct { - name string - inputCG string - expectedAG string - expectedIPB []controlplane.IPBlock - }{ - { - name: "empty-cg-no-result", - inputCG: "", - expectedAG: "", - expectedIPB: nil, - }, - { - name: "cg-with-selector", - inputCG: cgA.Name, - expectedAG: cgA.Name, - expectedIPB: nil, - }, - { - name: "cg-with-selector-not-found", - inputCG: cgC.Name, - expectedAG: "", - expectedIPB: nil, - }, - { - name: "cg-with-ipblock", - inputCG: cgB.Name, - expectedAG: "", - expectedIPB: []controlplane.IPBlock{ - { - CIDR: *cidrIPNet, - Except: []controlplane.IPNet{}, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - actualAG, actualIPB := npc.processRefCG(tt.inputCG) - assert.Equal(t, tt.expectedIPB, actualIPB, "IPBlock does not match") - assert.Equal(t, tt.expectedAG, actualAG, "addressGroup does not match") - }) - } -} - func TestProcessAppliedToGroupsForCGs(t *testing.T) { selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}} cidr := "10.0.0.0/24" diff --git a/pkg/controller/networkpolicy/crd_utils.go b/pkg/controller/networkpolicy/crd_utils.go index 770d2411de7..2e37d07af0b 100644 --- a/pkg/controller/networkpolicy/crd_utils.go +++ b/pkg/controller/networkpolicy/crd_utils.go @@ -105,7 +105,7 @@ func (n *NetworkPolicyController) toAntreaPeerForCRD(peers []v1alpha1.NetworkPol } ipBlocks = append(ipBlocks, *ipBlock) } else if peer.Group != "" { - normalizedUID, groupIPBlocks := n.processRefCG(peer.Group) + normalizedUID, groupIPBlocks := n.processRefGroupOrClusterGroup(peer.Group, np.GetNamespace()) if normalizedUID != "" { addressGroups = append(addressGroups, normalizedUID) } else if len(groupIPBlocks) > 0 { @@ -119,6 +119,43 @@ func (n *NetworkPolicyController) toAntreaPeerForCRD(peers []v1alpha1.NetworkPol return &controlplane.NetworkPolicyPeer{AddressGroups: addressGroups, IPBlocks: ipBlocks} } +// processRefGroupOrClusterGroup processes the Group/ClusterGroup reference present in the rule and returns the +// NetworkPolicyPeer with the corresponding AddressGroup or IPBlock. +func (n *NetworkPolicyController) processRefGroupOrClusterGroup(g, namespace string) (string, []controlplane.IPBlock) { + var key string + if namespace != "" { + grp, err := n.gLister.Groups(namespace).Get(g) + if err != nil { + klog.Errorf("Group %s/%s not found: %v", namespace, g, err) + return "", nil + } + key = internalGroupKeyFunc(grp) + } else { + // Retrieve ClusterGroup for corresponding entry in the rule. + cg, err := n.cgLister.Get(g) + if err != nil { + klog.Errorf("ClusterGroup %s not found: %v", g, err) + return "", nil + } + key = internalGroupKeyFunc(cg) + } + // Find the internal Group corresponding to this ClusterGroup + ig, found, _ := n.internalGroupStore.Get(key) + if !found { + // Internal Group was not found. Once the internal Group is created, the sync + // worker for internal group will re-enqueue the ClusterNetworkPolicy processing + // which will trigger the creation of AddressGroup. + return "", nil + } + intGrp := ig.(*antreatypes.Group) + if len(intGrp.IPBlocks) > 0 { + return "", intGrp.IPBlocks + } + agKey := n.createAddressGroupForInternalGroup(intGrp) + // Return if addressGroup was created or found. + return agKey, nil +} + // toNamespacedPeerForCRD creates an Antrea controlplane NetworkPolicyPeer for crdv1alpha1 NetworkPolicyPeer // for a particular Namespace. It is used when a single crdv1alpha1 NetworkPolicyPeer maps to multiple // controlplane NetworkPolicyPeers because the appliedTo workloads reside in different Namespaces. @@ -131,11 +168,11 @@ func (n *NetworkPolicyController) toNamespacedPeerForCRD(peers []v1alpha1.Networ return &controlplane.NetworkPolicyPeer{AddressGroups: addressGroups} } -// createAppliedToGroupForClusterGroupCRD creates an AppliedToGroup object corresponding to a +// createAppliedToGroupForInternalGroup creates an AppliedToGroup object corresponding to a // internal Group. If the AppliedToGroup already exists, it returns the key // otherwise it copies the internal Group contents to an AppliedToGroup resource and returns // its key. -func (n *NetworkPolicyController) createAppliedToGroupForClusterGroupCRD(intGrp *antreatypes.Group) string { +func (n *NetworkPolicyController) createAppliedToGroupForInternalGroup(intGrp *antreatypes.Group) string { key, err := store.GroupKeyFunc(intGrp) if err != nil { return "" @@ -150,17 +187,17 @@ func (n *NetworkPolicyController) createAppliedToGroupForClusterGroupCRD(intGrp UID: intGrp.UID, Name: key, } - klog.V(2).Infof("Creating new AppliedToGroup %v corresponding to ClusterGroup CRD %s", appliedToGroup.UID, intGrp.Name) + klog.V(2).Infof("Creating new AppliedToGroup %v corresponding to %s", appliedToGroup.UID, intGrp.SourceReference.ToTypedString()) n.appliedToGroupStore.Create(appliedToGroup) n.enqueueAppliedToGroup(key) return key } -// createAddressGroupForClusterGroupCRD creates an AddressGroup object corresponding to a -// ClusterGroup spec. If the AddressGroup already exists, it returns the key +// createAddressGroupForInternalGroup creates an AddressGroup object corresponding to an +// internal Group spec. If the AddressGroup already exists, it returns the key // otherwise it copies the ClusterGroup CRD contents to an AddressGroup resource and returns // its key. If the corresponding internal Group is not found return empty. -func (n *NetworkPolicyController) createAddressGroupForClusterGroupCRD(intGrp *antreatypes.Group) string { +func (n *NetworkPolicyController) createAddressGroupForInternalGroup(intGrp *antreatypes.Group) string { key, err := store.GroupKeyFunc(intGrp) if err != nil { return "" @@ -176,7 +213,7 @@ func (n *NetworkPolicyController) createAddressGroupForClusterGroupCRD(intGrp *a Name: key, } n.addressGroupStore.Create(addressGroup) - klog.V(2).Infof("Created new AddressGroup %v corresponding to ClusterGroup CRD %s", addressGroup.UID, intGrp.Name) + klog.V(2).Infof("Created new AddressGroup %v corresponding to %s", addressGroup.UID, intGrp.SourceReference.ToTypedString()) return key } diff --git a/pkg/controller/networkpolicy/crd_utils_test.go b/pkg/controller/networkpolicy/crd_utils_test.go index 1e0af6079f2..5702741b1fa 100644 --- a/pkg/controller/networkpolicy/crd_utils_test.go +++ b/pkg/controller/networkpolicy/crd_utils_test.go @@ -300,3 +300,150 @@ func TestToAntreaPeerForCRD(t *testing.T) { }) } } + +func TestProcessRefGroupOrClusterGroup(t *testing.T) { + selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}} + cidr := "10.0.0.0/24" + cidrIPNet, _ := cidrStrToIPNet(cidr) + // cgA with selector present in cache + cgA := crdv1alpha3.ClusterGroup{ + ObjectMeta: metav1.ObjectMeta{Name: "cgA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorA, + }, + } + // cgB with IPBlock present in cache + cgB := crdv1alpha3.ClusterGroup{ + ObjectMeta: metav1.ObjectMeta{Name: "cgB", UID: "uidB"}, + Spec: crdv1alpha3.GroupSpec{ + IPBlocks: []crdv1alpha1.IPBlock{ + { + CIDR: cidr, + }, + }, + }, + } + // cgC not found in cache + cgC := crdv1alpha3.ClusterGroup{ + ObjectMeta: metav1.ObjectMeta{Name: "cgC", UID: "uidC"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorA, + }, + } + + // gA with selector present in cache + gA := crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidGA"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorA, + }, + } + // gB with IPBlock present in cache + gB := crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsB", Name: "gB", UID: "uidGB"}, + Spec: crdv1alpha3.GroupSpec{ + IPBlocks: []crdv1alpha1.IPBlock{ + { + CIDR: cidr, + }, + }, + }, + } + // gC not found in cache + gC := crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsC", Name: "gC", UID: "uidGC"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorA, + }, + } + _, npc := newController() + npc.addClusterGroup(&cgA) + npc.addClusterGroup(&cgB) + npc.cgStore.Add(&cgA) + npc.cgStore.Add(&cgB) + npc.addGroup(&gA) + npc.addGroup(&gB) + npc.gStore.Add(&gA) + npc.gStore.Add(&gB) + tests := []struct { + name string + inputGroupOrCG string + inputNamespace string + expectedAG string + expectedIPB []controlplane.IPBlock + }{ + { + name: "empty-cg-no-result", + inputGroupOrCG: "", + inputNamespace: "", + expectedAG: "", + expectedIPB: nil, + }, + { + name: "cg-with-selector", + inputGroupOrCG: cgA.Name, + inputNamespace: "", + expectedAG: cgA.Name, + expectedIPB: nil, + }, + { + name: "cg-with-selector-not-found", + inputGroupOrCG: cgC.Name, + inputNamespace: "", + expectedAG: "", + expectedIPB: nil, + }, + { + name: "cg-with-ipblock", + inputGroupOrCG: cgB.Name, + inputNamespace: "", + expectedAG: "", + expectedIPB: []controlplane.IPBlock{ + { + CIDR: *cidrIPNet, + Except: []controlplane.IPNet{}, + }, + }, + }, + { + name: "empty-g-no-result", + inputGroupOrCG: "", + inputNamespace: "", + expectedAG: "", + expectedIPB: nil, + }, + { + name: "g-with-selector", + inputGroupOrCG: gA.Name, + inputNamespace: gA.Namespace, + expectedAG: fmt.Sprintf("%s/%s", gA.Namespace, gA.Name), + expectedIPB: nil, + }, + { + name: "g-with-selector-not-found", + inputGroupOrCG: gC.Name, + inputNamespace: gC.Namespace, + expectedAG: "", + expectedIPB: nil, + }, + { + name: "g-with-ipblock", + inputGroupOrCG: gB.Name, + inputNamespace: gB.Namespace, + expectedAG: "", + expectedIPB: []controlplane.IPBlock{ + { + CIDR: *cidrIPNet, + Except: []controlplane.IPNet{}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actualAG, actualIPB := npc.processRefGroupOrClusterGroup(tt.inputGroupOrCG, tt.inputNamespace) + assert.Equal(t, tt.expectedIPB, actualIPB, "IPBlock does not match") + assert.Equal(t, tt.expectedAG, actualAG, "addressGroup does not match") + }) + } +} diff --git a/pkg/controller/networkpolicy/group.go b/pkg/controller/networkpolicy/group.go new file mode 100644 index 00000000000..80726160e27 --- /dev/null +++ b/pkg/controller/networkpolicy/group.go @@ -0,0 +1,275 @@ +// Copyright 2021 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 networkpolicy + +import ( + "context" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/tools/cache" + "k8s.io/klog/v2" + + "antrea.io/antrea/pkg/apis/controlplane" + crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1" + crdv1alpha3 "antrea.io/antrea/pkg/apis/crd/v1alpha3" + antreatypes "antrea.io/antrea/pkg/controller/types" +) + +// addGroup is responsible for processing the ADD event of a Group resource. +func (n *NetworkPolicyController) addGroup(curObj interface{}) { + g := curObj.(*crdv1alpha3.Group) + key := internalGroupKeyFunc(g) + klog.V(2).Infof("Processing ADD event for Group %s/%s", g.GetNamespace(), g.GetName()) + newGroup := n.processGroup(g) + klog.V(2).Infof("Creating new internal Group %s", newGroup.UID) + n.internalGroupStore.Create(newGroup) + n.enqueueInternalGroup(key) +} + +// updateGroup is responsible for processing the UPDATE event of a Group resource. +func (n *NetworkPolicyController) updateGroup(oldObj, curObj interface{}) { + cg := curObj.(*crdv1alpha3.Group) + og := oldObj.(*crdv1alpha3.Group) + key := internalGroupKeyFunc(cg) + klog.V(2).Infof("Processing UPDATE event for Group %s/%s", cg.Namespace, cg.Name) + newGroup := n.processGroup(cg) + oldGroup := n.processGroup(og) + + selectorUpdated := func() bool { + return getNormalizedNameForSelector(newGroup.Selector) != getNormalizedNameForSelector(oldGroup.Selector) + } + svcRefUpdated := func() bool { + oldSvc, newSvc := oldGroup.ServiceReference, newGroup.ServiceReference + if oldSvc != nil && newSvc != nil && oldSvc.Name == newSvc.Name && oldSvc.Namespace == newSvc.Namespace { + return false + } else if oldSvc == nil && newSvc == nil { + return false + } + return true + } + ipBlocksUpdated := func() bool { + oldIPBs, newIPBs := sets.String{}, sets.String{} + for _, ipb := range oldGroup.IPBlocks { + oldIPBs.Insert(ipNetToCIDRStr(ipb.CIDR)) + } + for _, ipb := range newGroup.IPBlocks { + newIPBs.Insert(ipNetToCIDRStr(ipb.CIDR)) + } + return oldIPBs.Equal(newIPBs) + } + childGroupsUpdated := func() bool { + oldChildGroups, newChildGroups := sets.String{}, sets.String{} + for _, c := range oldGroup.ChildGroups { + oldChildGroups.Insert(c) + } + for _, c := range newGroup.ChildGroups { + newChildGroups.Insert(c) + } + return !oldChildGroups.Equal(newChildGroups) + } + if !ipBlocksUpdated() && !svcRefUpdated() && !selectorUpdated() && !childGroupsUpdated() { + // No change in the contents of the Group. No need to enqueue for further sync. + return + } + n.internalGroupStore.Update(newGroup) + n.enqueueInternalGroup(key) +} + +// deleteGroup is responsible for processing the DELETE event of a Group resource. +func (n *NetworkPolicyController) deleteGroup(oldObj interface{}) { + og, ok := oldObj.(*crdv1alpha3.Group) + klog.V(2).Infof("Processing DELETE event for Group %s/%s", og.GetNamespace(), og.GetName()) + if !ok { + tombstone, ok := oldObj.(cache.DeletedFinalStateUnknown) + if !ok { + klog.Errorf("Error decoding object when deleting Group, invalid type: %v", oldObj) + return + } + og, ok = tombstone.Obj.(*crdv1alpha3.Group) + if !ok { + klog.Errorf("Error decoding object tombstone when deleting Group, invalid type: %v", tombstone.Obj) + return + } + } + key := internalGroupKeyFunc(og) + klog.V(2).Infof("Deleting internal Group %s", key) + err := n.internalGroupStore.Delete(key) + if err != nil { + klog.Errorf("Unable to delete internal Group %s from store: %v", key, err) + } + n.enqueueInternalGroup(key) +} + +func (n *NetworkPolicyController) processGroup(g *crdv1alpha3.Group) *antreatypes.Group { + internalGroup := antreatypes.Group{ + SourceReference: getGroupSourceRef(g), + UID: g.UID, + } + if len(g.Spec.ChildGroups) > 0 { + for _, childGName := range g.Spec.ChildGroups { + internalGroup.ChildGroups = append(internalGroup.ChildGroups, string(childGName)) + } + return &internalGroup + } + if len(g.Spec.IPBlocks) > 0 { + for i := range g.Spec.IPBlocks { + ipb, _ := toAntreaIPBlockForCRD(&g.Spec.IPBlocks[i]) + internalGroup.IPBlocks = append(internalGroup.IPBlocks, *ipb) + } + return &internalGroup + } + svcSelector := g.Spec.ServiceReference + if svcSelector != nil { + // ServiceReference will be converted to groupSelector once the internalGroup is synced. + internalGroup.ServiceReference = &controlplane.ServiceReference{ + Namespace: svcSelector.Namespace, + Name: svcSelector.Name, + } + } else { + groupSelector := toGroupSelector(g.Namespace, g.Spec.PodSelector, g.Spec.NamespaceSelector, g.Spec.ExternalEntitySelector) + internalGroup.Selector = groupSelector + } + return &internalGroup +} + +func getGroupSourceRef(g *crdv1alpha3.Group) *controlplane.GroupReference { + return &controlplane.GroupReference{ + Name: g.GetName(), + Namespace: g.GetNamespace(), + UID: g.GetUID(), + } +} + +func (n *NetworkPolicyController) syncNamespacedInternalGroup(grp *antreatypes.Group) error { + // Retrieve the Group corresponding to this key. + g, err := n.gLister.Groups(grp.SourceReference.Namespace).Get(grp.SourceReference.Name) + if err != nil { + klog.Infof("Didn't find the %s, skip processing of internal group", grp.SourceReference.ToTypedString()) + return nil + } + key := internalGroupKeyFunc(g) + selectorUpdated := n.processServiceReference(grp) + if grp.Selector != nil { + n.groupingInterface.AddGroup(clusterGroupType, key, grp.Selector) + } else { + n.groupingInterface.DeleteGroup(clusterGroupType, key) + } + if selectorUpdated { + // Update the internal Group object in the store with the new selector. + updatedGrp := &antreatypes.Group{ + UID: grp.UID, + SourceReference: grp.SourceReference, + Selector: grp.Selector, + ServiceReference: grp.ServiceReference, + ChildGroups: grp.ChildGroups, + } + klog.V(2).Infof("Updating existing internal Group %s", key) + n.internalGroupStore.Update(updatedGrp) + } + // Update the Group status to Realized as Antrea has recognized the Group and + // processed its group members. + err = n.updateGroupStatus(g, v1.ConditionTrue) + if err != nil { + klog.Errorf("Failed to update Group %s/%s GroupMembersComputed condition to %s: %v", g.Namespace, g.Name, v1.ConditionTrue, err) + return err + } + n.triggerParentGroupSync(grp) + return n.triggerANPUpdates(g) +} + +// triggerANPUpdates triggers processing of Antrea NetworkPolicies associated with the input Group. +func (n *NetworkPolicyController) triggerANPUpdates(g *crdv1alpha3.Group) error { + // If a Group is added/updated, it might have a reference in Antrea NetworkPolicy. + anps, err := n.anpInformer.Informer().GetIndexer().ByIndex(GroupIndex, g.Name) + if err != nil { + klog.Errorf("Error retrieving Antrea NetworkPolicies corresponding to Group %s/%s", g.Namespace, g.Name) + return err + } + for _, obj := range anps { + anp := obj.(*crdv1alpha1.NetworkPolicy) + // Re-process Antrea NetworkPolicies which may be affected due to updates to Group. + curInternalNP := n.processAntreaNetworkPolicy(anp) + klog.V(2).Infof("Updating existing internal NetworkPolicy %s for %s", curInternalNP.Name, curInternalNP.SourceRef.ToString()) + key := internalNetworkPolicyKeyFunc(anp) + // Lock access to internal NetworkPolicy store such that concurrent access + // to an internal NetworkPolicy is not allowed. This will avoid the + // case in which an Update to an internal NetworkPolicy object may + // cause the SpanMeta member to be overridden with stale SpanMeta members + // from an older internal NetworkPolicy. + n.internalNetworkPolicyMutex.Lock() + oldInternalNPObj, _, _ := n.internalNetworkPolicyStore.Get(key) + oldInternalNP := oldInternalNPObj.(*antreatypes.NetworkPolicy) + // Must preserve old internal NetworkPolicy Span. + curInternalNP.SpanMeta = oldInternalNP.SpanMeta + n.internalNetworkPolicyStore.Update(curInternalNP) + // Unlock the internal NetworkPolicy store. + n.internalNetworkPolicyMutex.Unlock() + // Enqueue addressGroup keys to update their group members. + // TODO: optimize this to avoid enqueueing address groups when not updated. + for _, atg := range curInternalNP.AppliedToGroups { + n.enqueueAppliedToGroup(atg) + } + for _, rule := range curInternalNP.Rules { + for _, addrGroupName := range rule.From.AddressGroups { + n.enqueueAddressGroup(addrGroupName) + } + for _, addrGroupName := range rule.To.AddressGroups { + n.enqueueAddressGroup(addrGroupName) + } + } + n.enqueueInternalNetworkPolicy(key) + n.deleteDereferencedAddressGroups(oldInternalNP) + for _, atg := range oldInternalNP.AppliedToGroups { + n.deleteDereferencedAppliedToGroup(atg) + } + } + return nil +} + +// updateGroupStatus updates the Status subresource for a Group. +func (n *NetworkPolicyController) updateGroupStatus(g *crdv1alpha3.Group, cStatus v1.ConditionStatus) error { + condStatus := crdv1alpha3.GroupCondition{ + Status: cStatus, + Type: crdv1alpha3.GroupMembersComputed, + } + if groupMembersComputedConditionEqual(g.Status.Conditions, condStatus) { + // There is no change in conditions. + return nil + } + condStatus.LastTransitionTime = metav1.Now() + status := crdv1alpha3.GroupStatus{ + Conditions: []crdv1alpha3.GroupCondition{condStatus}, + } + klog.V(4).Infof("Updating Group %s/%s status to %#v", g.Namespace, g.Name, condStatus) + toUpdate := g.DeepCopy() + toUpdate.Status = status + _, err := n.crdClient.CrdV1alpha3().Groups(g.GetNamespace()).UpdateStatus(context.TODO(), toUpdate, metav1.UpdateOptions{}) + return err +} + +// groupMembersComputedConditionEqual checks whether the condition status for GroupMembersComputed condition +// is same. Returns true if equal, otherwise returns false. It disregards the lastTransitionTime field. +func groupMembersComputedConditionEqual(conds []crdv1alpha3.GroupCondition, condition crdv1alpha3.GroupCondition) bool { + for _, c := range conds { + if c.Type == crdv1alpha3.GroupMembersComputed { + if c.Status == condition.Status { + return true + } + } + } + return false +} diff --git a/pkg/controller/networkpolicy/group_test.go b/pkg/controller/networkpolicy/group_test.go new file mode 100644 index 00000000000..0856faa5df4 --- /dev/null +++ b/pkg/controller/networkpolicy/group_test.go @@ -0,0 +1,452 @@ +// Copyright 2021 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 networkpolicy + +import ( + "fmt" + "testing" + + "antrea.io/antrea/pkg/apis/controlplane" + crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1" + crdv1alpha3 "antrea.io/antrea/pkg/apis/crd/v1alpha3" + antreatypes "antrea.io/antrea/pkg/controller/types" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestProcessGroup(t *testing.T) { + selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}} + selectorB := metav1.LabelSelector{MatchLabels: map[string]string{"foo2": "bar2"}} + selectorC := metav1.LabelSelector{MatchLabels: map[string]string{"foo3": "bar3"}} + selectorD := metav1.LabelSelector{MatchLabels: map[string]string{"foo4": "bar4"}} + cidr := "10.0.0.0/24" + cidrIPNet, _ := cidrStrToIPNet(cidr) + tests := []struct { + name string + inputGroup *crdv1alpha3.Group + expectedGroup *antreatypes.Group + }{ + { + name: "g-with-ns-selector", + inputGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorA, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "gA", + Namespace: "nsA", + UID: "uidA", + }, + Selector: toGroupSelector("nsA", nil, &selectorA, nil), + }, + }, + { + name: "g-with-pod-selector", + inputGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsB", Name: "gB", UID: "uidB"}, + Spec: crdv1alpha3.GroupSpec{ + PodSelector: &selectorB, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidB", + SourceReference: &controlplane.GroupReference{ + Name: "gB", + Namespace: "nsB", + UID: "uidB", + }, + Selector: toGroupSelector("nsB", &selectorB, nil, nil), + }, + }, + { + name: "g-with-pod-ns-selector", + inputGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsC", Name: "gC", UID: "uidC"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorD, + PodSelector: &selectorC, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidC", + SourceReference: &controlplane.GroupReference{ + Name: "gC", + Namespace: "nsC", + UID: "uidC", + }, + Selector: toGroupSelector("nsC", &selectorC, &selectorD, nil), + }, + }, + { + name: "g-with-ip-block", + inputGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsD", Name: "gD", UID: "uidD"}, + Spec: crdv1alpha3.GroupSpec{ + IPBlocks: []crdv1alpha1.IPBlock{ + { + CIDR: cidr, + }, + }, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidD", + SourceReference: &controlplane.GroupReference{ + Name: "gD", + Namespace: "nsD", + UID: "uidD", + }, + IPBlocks: []controlplane.IPBlock{ + { + CIDR: *cidrIPNet, + Except: []controlplane.IPNet{}, + }, + }, + }, + }, + { + name: "g-with-svc-reference", + inputGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsE", Name: "gE", UID: "uidE"}, + Spec: crdv1alpha3.GroupSpec{ + ServiceReference: &crdv1alpha3.ServiceReference{ + Name: "test-svc", + Namespace: "nsE", + }, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidE", + SourceReference: &controlplane.GroupReference{ + Name: "gE", + Namespace: "nsE", + UID: "uidE", + }, + ServiceReference: &controlplane.ServiceReference{ + Name: "test-svc", + Namespace: "nsE", + }, + }, + }, + { + name: "g-with-child-groups", + inputGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsF", Name: "gF", UID: "uidF"}, + Spec: crdv1alpha3.GroupSpec{ + ChildGroups: []crdv1alpha3.ClusterGroupReference{"gA", "gB"}, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidF", + SourceReference: &controlplane.GroupReference{ + Name: "gF", + Namespace: "nsF", + UID: "uidF", + }, + ChildGroups: []string{"gA", "gB"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, c := newController() + actualGroup := c.processGroup(tt.inputGroup) + assert.Equal(t, tt.expectedGroup, actualGroup) + }) + } +} + +func TestAddGroup(t *testing.T) { + selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}} + selectorB := metav1.LabelSelector{MatchLabels: map[string]string{"foo2": "bar2"}} + selectorC := metav1.LabelSelector{MatchLabels: map[string]string{"foo3": "bar3"}} + selectorD := metav1.LabelSelector{MatchLabels: map[string]string{"foo4": "bar4"}} + cidr := "10.0.0.0/24" + cidrIPNet, _ := cidrStrToIPNet(cidr) + tests := []struct { + name string + inputGroup *crdv1alpha3.Group + expectedGroup *antreatypes.Group + }{ + { + name: "g-with-ns-selector", + inputGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorA, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "gA", + Namespace: "nsA", + UID: "uidA", + }, + Selector: toGroupSelector("nsA", nil, &selectorA, nil), + }, + }, + { + name: "g-with-pod-selector", + inputGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsB", Name: "gB", UID: "uidB"}, + Spec: crdv1alpha3.GroupSpec{ + PodSelector: &selectorB, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidB", + SourceReference: &controlplane.GroupReference{ + Name: "gB", + Namespace: "nsB", + UID: "uidB", + }, + Selector: toGroupSelector("nsB", &selectorB, nil, nil), + }, + }, + { + name: "g-with-pod-ns-selector", + inputGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsC", Name: "gC", UID: "uidC"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorD, + PodSelector: &selectorC, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidC", + SourceReference: &controlplane.GroupReference{ + Name: "gC", + Namespace: "nsC", + UID: "uidC", + }, + Selector: toGroupSelector("nsC", &selectorC, &selectorD, nil), + }, + }, + { + name: "g-with-ip-block", + inputGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsD", Name: "gD", UID: "uidD"}, + Spec: crdv1alpha3.GroupSpec{ + IPBlocks: []crdv1alpha1.IPBlock{ + { + CIDR: cidr, + }, + }, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidD", + SourceReference: &controlplane.GroupReference{ + Name: "gD", + Namespace: "nsD", + UID: "uidD", + }, + IPBlocks: []controlplane.IPBlock{ + { + CIDR: *cidrIPNet, + Except: []controlplane.IPNet{}, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, npc := newController() + npc.addGroup(tt.inputGroup) + key := fmt.Sprintf("%s/%s", tt.inputGroup.Namespace, tt.inputGroup.Name) + actualGroupObj, _, _ := npc.internalGroupStore.Get(key) + actualGroup := actualGroupObj.(*antreatypes.Group) + assert.Equal(t, tt.expectedGroup, actualGroup) + }) + } +} + +func TestUpdateGroup(t *testing.T) { + selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}} + selectorB := metav1.LabelSelector{MatchLabels: map[string]string{"foo2": "bar2"}} + selectorC := metav1.LabelSelector{MatchLabels: map[string]string{"foo3": "bar3"}} + selectorD := metav1.LabelSelector{MatchLabels: map[string]string{"foo4": "bar4"}} + testG := crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorA, + }, + } + cidr := "10.0.0.0/24" + cidrIPNet, _ := cidrStrToIPNet(cidr) + tests := []struct { + name string + updatedGroup *crdv1alpha3.Group + expectedGroup *antreatypes.Group + }{ + { + name: "g-update-ns-selector", + updatedGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorB, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "gA", + Namespace: "nsA", + UID: "uidA", + }, + Selector: toGroupSelector("nsA", nil, &selectorB, nil), + }, + }, + { + name: "g-update-pod-selector", + updatedGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + PodSelector: &selectorC, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "gA", + Namespace: "nsA", + UID: "uidA", + }, + Selector: toGroupSelector("nsA", &selectorC, nil, nil), + }, + }, + { + name: "g-update-pod-ns-selector", + updatedGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorD, + PodSelector: &selectorC, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "gA", + Namespace: "nsA", + UID: "uidA", + }, + Selector: toGroupSelector("nsA", &selectorC, &selectorD, nil), + }, + }, + { + name: "g-update-ip-block", + updatedGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + IPBlocks: []crdv1alpha1.IPBlock{ + { + CIDR: cidr, + }, + }, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "gA", + Namespace: "nsA", + UID: "uidA", + }, + IPBlocks: []controlplane.IPBlock{ + { + CIDR: *cidrIPNet, + Except: []controlplane.IPNet{}, + }, + }, + }, + }, + { + name: "g-update-svc-reference", + updatedGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + ServiceReference: &crdv1alpha3.ServiceReference{ + Name: "test-svc", + Namespace: "nsA", + }, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "gA", + Namespace: "nsA", + UID: "uidA", + }, + ServiceReference: &controlplane.ServiceReference{ + Name: "test-svc", + Namespace: "nsA", + }, + }, + }, + { + name: "g-update-child-groups", + updatedGroup: &crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + ChildGroups: []crdv1alpha3.ClusterGroupReference{"gB", "gC"}, + }, + }, + expectedGroup: &antreatypes.Group{ + UID: "uidA", + SourceReference: &controlplane.GroupReference{ + Name: "gA", + Namespace: "nsA", + UID: "uidA", + }, + ChildGroups: []string{"gB", "gC"}, + }, + }, + } + _, npc := newController() + npc.addGroup(&testG) + key := fmt.Sprintf("%s/%s", testG.Namespace, testG.Name) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + npc.updateGroup(&testG, tt.updatedGroup) + actualGroupObj, _, _ := npc.internalGroupStore.Get(key) + actualGroup := actualGroupObj.(*antreatypes.Group) + assert.Equal(t, tt.expectedGroup, actualGroup) + }) + } +} + +func TestDeleteG(t *testing.T) { + selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}} + testG := crdv1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uidA"}, + Spec: crdv1alpha3.GroupSpec{ + NamespaceSelector: &selectorA, + }, + } + key := fmt.Sprintf("%s/%s", testG.Namespace, testG.Name) + _, npc := newController() + npc.addGroup(&testG) + npc.deleteGroup(&testG) + _, found, _ := npc.internalGroupStore.Get(key) + assert.False(t, found, "expected internal Group to be deleted") +} diff --git a/pkg/controller/networkpolicy/networkpolicy_controller.go b/pkg/controller/networkpolicy/networkpolicy_controller.go index e270876596b..eca57cbe021 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller.go +++ b/pkg/controller/networkpolicy/networkpolicy_controller.go @@ -83,6 +83,8 @@ const ( PriorityIndex = "priority" // ClusterGroupIndex is used to index ClusterNetworkPolicies by ClusterGroup names. ClusterGroupIndex = "clustergroup" + // GroupIndex is used to index Antrea NetworkPolicies by Group names. + GroupIndex = "group" appliedToGroupType grouping.GroupType = "appliedToGroup" addressGroupType grouping.GroupType = "addressGroup" @@ -176,6 +178,14 @@ type NetworkPolicyController struct { // once. cgListerSynced cache.InformerSynced + gInformer crdv1a3informers.GroupInformer + // gLister is able to list/get Groups and is populated by the shared informer passed to + // NewGroupController. + gLister crdv1a3listers.GroupLister + // gListerSynced is a function which returns true if the Group shared informer has been synced at least + // once. + gListerSynced cache.InformerSynced + // addressGroupStore is the storage where the populated Address Groups are stored. addressGroupStore storage.Interface // appliedToGroupStore is the storage where the populated AppliedTo Groups are stored. @@ -227,6 +237,7 @@ func NewNetworkPolicyController(kubeClient clientset.Interface, anpInformer secinformers.NetworkPolicyInformer, tierInformer secinformers.TierInformer, cgInformer crdv1a3informers.ClusterGroupInformer, + gInformer crdv1a3informers.GroupInformer, addressGroupStore storage.Interface, appliedToGroupStore storage.Interface, internalNetworkPolicyStore storage.Interface, @@ -280,6 +291,9 @@ func NewNetworkPolicyController(kubeClient clientset.Interface, n.cgInformer = cgInformer n.cgLister = cgInformer.Lister() n.cgListerSynced = cgInformer.Informer().HasSynced + n.gInformer = gInformer + n.gLister = gInformer.Lister() + n.gListerSynced = gInformer.Informer().HasSynced // Add handlers for Namespace events. n.namespaceInformer.Informer().AddEventHandlerWithResyncPeriod( cache.ResourceEventHandlerFuncs{ @@ -375,6 +389,45 @@ func NewNetworkPolicyController(kubeClient clientset.Interface, } return []string{anp.Spec.Tier}, nil }, + GroupIndex: func(obj interface{}) ([]string, error) { + anp, ok := obj.(*secv1alpha1.NetworkPolicy) + if !ok { + return []string{}, nil + } + groupNames := sets.String{} + for _, appTo := range anp.Spec.AppliedTo { + if appTo.Group != "" { + groupNames.Insert(appTo.Group) + } + } + if len(anp.Spec.Ingress) == 0 && len(anp.Spec.Egress) == 0 { + return groupNames.List(), nil + } + appendGroups := func(rule secv1alpha1.Rule) { + for _, peer := range rule.To { + if peer.Group != "" { + groupNames.Insert(peer.Group) + } + } + for _, peer := range rule.From { + if peer.Group != "" { + groupNames.Insert(peer.Group) + } + } + for _, appTo := range rule.AppliedTo { + if appTo.Group != "" { + groupNames.Insert(appTo.Group) + } + } + } + for _, rule := range anp.Spec.Egress { + appendGroups(rule) + } + for _, rule := range anp.Spec.Ingress { + appendGroups(rule) + } + return groupNames.List(), nil + }, }, ) anpInformer.Informer().AddEventHandlerWithResyncPeriod( @@ -394,6 +447,15 @@ func NewNetworkPolicyController(kubeClient clientset.Interface, }, resyncPeriod, ) + // Add event handlers for Group notification. + gInformer.Informer().AddEventHandlerWithResyncPeriod( + cache.ResourceEventHandlerFuncs{ + AddFunc: n.addGroup, + UpdateFunc: n.updateGroup, + DeleteFunc: n.deleteGroup, + }, + resyncPeriod, + ) } return n } @@ -1122,14 +1184,14 @@ func (n *NetworkPolicyController) getAddressGroupMemberSet(g *antreatypes.Addres // the members are computed as the union of all its childGroup's members. func (n *NetworkPolicyController) getClusterGroupMemberSet(group *antreatypes.Group) controlplane.GroupMemberSet { if len(group.ChildGroups) == 0 { - return n.getMemberSetForGroupType(clusterGroupType, group.Name) + return n.getMemberSetForGroupType(clusterGroupType, group.SourceReference.ToString()) } groupMemberSet := controlplane.GroupMemberSet{} for _, childName := range group.ChildGroups { childGroup, found, _ := n.internalGroupStore.Get(childName) if found { child := childGroup.(*antreatypes.Group) - groupMemberSet.Merge(n.getMemberSetForGroupType(clusterGroupType, child.Name)) + groupMemberSet.Merge(n.getMemberSetForGroupType(clusterGroupType, child.SourceReference.ToString())) } } return groupMemberSet @@ -1305,13 +1367,15 @@ func (n *NetworkPolicyController) getAppliedToWorkloads(g *antreatypes.AppliedTo // For ClusterGroup that has childGroups, the workloads are computed as the union of all its childGroup's workloads. func (n *NetworkPolicyController) getClusterGroupWorkloads(group *antreatypes.Group) ([]*v1.Pod, []*v1alpha2.ExternalEntity) { if len(group.ChildGroups) == 0 { - return n.groupingInterface.GetEntities(clusterGroupType, group.Name) + return n.groupingInterface.GetEntities(clusterGroupType, group.SourceReference.ToString()) } podNameSet, eeNameSet := sets.String{}, sets.String{} var pods []*v1.Pod var ees []*v1alpha2.ExternalEntity for _, childName := range group.ChildGroups { - childPods, childEEs := n.groupingInterface.GetEntities(clusterGroupType, childName) + // childNameString will either be name of the child ClusterGroup or Namespaced name of the child Group. + childNameString := k8s.NamespacedName(group.SourceReference.Namespace, childName) + childPods, childEEs := n.groupingInterface.GetEntities(clusterGroupType, childNameString) for _, pod := range childPods { podString := k8s.NamespacedName(pod.Namespace, pod.Name) if !podNameSet.Has(podString) { @@ -1440,7 +1504,11 @@ func internalNetworkPolicyKeyFunc(obj metav1.Object) string { } // internalGroupKeyFunc knows how to generate the key for an internal Group based on the object metadata -// of the corresponding ClusterGroup resource. Currently the Name of the ClusterGroup is used to ensure uniqueness. +// of the corresponding Group and ClusterGroup resource. Currently the Name of the ClusterGroup is used to ensure +// uniqueness. Similarly, the Namespaced Name of the Group is used to ensure uniqueness for the Group resource. func internalGroupKeyFunc(obj metav1.Object) string { + if len(obj.GetNamespace()) > 0 { + return obj.GetNamespace() + "/" + obj.GetName() + } return obj.GetName() } diff --git a/pkg/controller/networkpolicy/networkpolicy_controller_test.go b/pkg/controller/networkpolicy/networkpolicy_controller_test.go index 9f3e4a8a2fd..8395812078b 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller_test.go +++ b/pkg/controller/networkpolicy/networkpolicy_controller_test.go @@ -75,6 +75,7 @@ type networkPolicyController struct { cnpStore cache.Store tierStore cache.Store cgStore cache.Store + gStore cache.Store appliedToGroupStore storage.Interface addressGroupStore storage.Interface internalNetworkPolicyStore storage.Interface @@ -94,6 +95,7 @@ func newController(objects ...runtime.Object) (*fake.Clientset, *networkPolicyCo internalNetworkPolicyStore := store.NewNetworkPolicyStore() internalGroupStore := store.NewGroupStore() cgInformer := crdInformerFactory.Crd().V1alpha3().ClusterGroups() + gInformer := crdInformerFactory.Crd().V1alpha3().Groups() groupEntityIndex := grouping.NewGroupEntityIndex() groupingController := grouping.NewGroupEntityController(groupEntityIndex, informerFactory.Core().V1().Pods(), @@ -109,6 +111,7 @@ func newController(objects ...runtime.Object) (*fake.Clientset, *networkPolicyCo crdInformerFactory.Crd().V1alpha1().NetworkPolicies(), crdInformerFactory.Crd().V1alpha1().Tiers(), cgInformer, + gInformer, addressGroupStore, appliedToGroupStore, internalNetworkPolicyStore, @@ -132,6 +135,7 @@ func newController(objects ...runtime.Object) (*fake.Clientset, *networkPolicyCo crdInformerFactory.Crd().V1alpha1().ClusterNetworkPolicies().Informer().GetStore(), crdInformerFactory.Crd().V1alpha1().Tiers().Informer().GetStore(), crdInformerFactory.Crd().V1alpha3().ClusterGroups().Informer().GetStore(), + crdInformerFactory.Crd().V1alpha3().Groups().Informer().GetStore(), appliedToGroupStore, addressGroupStore, internalNetworkPolicyStore, @@ -154,6 +158,7 @@ func newControllerWithoutEventHandler(objects ...runtime.Object) (*fake.Clientse internalGroupStore := store.NewGroupStore() networkPolicyInformer := informerFactory.Networking().V1().NetworkPolicies() cgStore := crdInformerFactory.Crd().V1alpha2().ClusterGroups().Informer().GetStore() + gStore := crdInformerFactory.Crd().V1alpha3().Groups().Informer().GetStore() groupEntityIndex := grouping.NewGroupEntityIndex() npController := &NetworkPolicyController{ kubeClient: client, @@ -179,6 +184,7 @@ func newControllerWithoutEventHandler(objects ...runtime.Object) (*fake.Clientse crdInformerFactory.Crd().V1alpha1().ClusterNetworkPolicies().Informer().GetStore(), crdInformerFactory.Crd().V1alpha1().Tiers().Informer().GetStore(), cgStore, + gStore, appliedToGroupStore, addressGroupStore, internalNetworkPolicyStore, @@ -3006,6 +3012,16 @@ func TestInternalGroupKeyFunc(t *testing.T) { } actualValue := internalGroupKeyFunc(&cg) assert.Equal(t, expValue, actualValue) + + expValue = "nsA/gA" + g := v1alpha3.Group{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "gA", UID: "uid-a"}, + Spec: v1alpha3.GroupSpec{ + NamespaceSelector: &selectorA, + }, + } + actualValue = internalGroupKeyFunc(&g) + assert.Equal(t, expValue, actualValue) } func TestGetAppliedToWorkloads(t *testing.T) { diff --git a/pkg/controller/networkpolicy/store/group.go b/pkg/controller/networkpolicy/store/group.go index d8316592660..cad8bc998de 100644 --- a/pkg/controller/networkpolicy/store/group.go +++ b/pkg/controller/networkpolicy/store/group.go @@ -31,14 +31,13 @@ const ( ChildGroupIndex = "childGroup" ) -// GroupKeyFunc knows how to get the key of an Group. +// GroupKeyFunc knows how to get the key of a Group. func GroupKeyFunc(obj interface{}) (string, error) { group, ok := obj.(*antreatypes.Group) if !ok { return "", fmt.Errorf("object is not *types.Group: %v", obj) } - // Replace empty Namespace with Group.Namespace once Namespaced Groups are introduced. - return k8s.NamespacedName("", group.Name), nil + return k8s.NamespacedName(group.SourceReference.Namespace, group.SourceReference.Name), nil } // NewGroupStore creates a store of Group. diff --git a/pkg/controller/networkpolicy/validate.go b/pkg/controller/networkpolicy/validate.go index 5cda9881a3f..823fc21fdb4 100644 --- a/pkg/controller/networkpolicy/validate.go +++ b/pkg/controller/networkpolicy/validate.go @@ -15,7 +15,6 @@ package networkpolicy import ( - crdv1alpha3 "antrea.io/antrea/pkg/apis/crd/v1alpha3" "encoding/json" "fmt" "strconv" @@ -30,7 +29,9 @@ import ( crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1" crdv1alpha2 "antrea.io/antrea/pkg/apis/crd/v1alpha2" + crdv1alpha3 "antrea.io/antrea/pkg/apis/crd/v1alpha3" "antrea.io/antrea/pkg/controller/networkpolicy/store" + "antrea.io/antrea/pkg/controller/types" "antrea.io/antrea/pkg/util/env" ) @@ -375,14 +376,6 @@ func (v *antreaPolicyValidator) tierExists(name string) bool { return true } -func (v *antreaPolicyValidator) clusterGroupExists(name string) bool { - _, err := v.networkPolicyController.cgLister.Get(name) - if err != nil { - return false - } - return true -} - // GetAdmissionResponseForErr returns an object of type AdmissionResponse with // the submitted error message. func GetAdmissionResponseForErr(err error) *admv1.AdmissionResponse { @@ -474,34 +467,6 @@ func (a *antreaPolicyValidator) validateAppliedTo(ingress, egress []crdv1alpha1. if numAppliedToInRules > 0 && (numAppliedToInRules != len(ingress)+len(egress)) { return "appliedTo field should either be set in all rules or in none of them", false } - // Ensure CG exists - checkAppTo := func(appTos []crdv1alpha1.NetworkPolicyPeer) bool { - for _, appTo := range specAppliedTo { - if appTo.Group != "" { - // Ensure that group exists - if !a.clusterGroupExists(appTo.Group) { - return false - } - } - } - return true - } - if appliedToInSpec { - if !checkAppTo(specAppliedTo) { - return fmt.Sprintf("cluster group referenced in appliedTo does not exist"), false - } - } else { - for _, rule := range ingress { - if !checkAppTo(rule.AppliedTo) { - return fmt.Sprintf("cluster group referenced in appliedTo does not exist"), false - } - } - for _, rule := range egress { - if !checkAppTo(rule.AppliedTo) { - return fmt.Sprintf("cluster group referenced in appliedTo does not exist"), false - } - } - } return "", true } @@ -519,10 +484,6 @@ func (a *antreaPolicyValidator) validatePeers(ingress, egress []crdv1alpha1.Rule if peer.PodSelector != nil || peer.IPBlock != nil || peer.NamespaceSelector != nil { return "group cannot be set with other peers in rules", false } - // Ensure that group exists - if !a.clusterGroupExists(peer.Group) { - return fmt.Sprintf("cluster group %s referenced in rules does not exist", peer.Group), false - } } return "", true } @@ -651,9 +612,9 @@ func (t *tierValidator) deleteValidate(oldObj interface{}, userInfo authenticati return "", true } -// validateAntreaGroupSpec ensures that an IPBlock is not set along with namespaceSelector and/or a +// validateAntreaClusterGroupSpec ensures that an IPBlock is not set along with namespaceSelector and/or a // podSelector. Similarly, ExternalEntitySelector cannot be set with PodSelector. -func validateAntreaGroupSpec(s crdv1alpha2.GroupSpec) (string, bool) { +func validateAntreaClusterGroupSpec(s crdv1alpha2.GroupSpec) (string, bool) { errMsg := "At most one of podSelector, externalEntitySelector, serviceReference, ipBlock, ipBlocks or childGroups can be set for a ClusterGroup" if s.PodSelector != nil && s.ExternalEntitySelector != nil { return errMsg, false @@ -680,7 +641,31 @@ func validateAntreaGroupSpec(s crdv1alpha2.GroupSpec) (string, bool) { return "", true } -func (g *groupValidator) validateChildGroup(s *crdv1alpha2.ClusterGroup) (string, bool) { +func validateAntreaGroupSpec(s crdv1alpha3.GroupSpec) (string, bool) { + errMsg := "At most one of podSelector, externalEntitySelector, serviceReference, ipBlocks or childGroups can be set for a Group" + if s.PodSelector != nil && s.ExternalEntitySelector != nil { + return errMsg, false + } + selector, serviceRef, ipBlocks, childGroups := 0, 0, 0, 0 + if s.NamespaceSelector != nil || s.ExternalEntitySelector != nil || s.PodSelector != nil { + selector = 1 + } + if len(s.IPBlocks) > 0 { + ipBlocks = 1 + } + if s.ServiceReference != nil { + serviceRef = 1 + } + if len(s.ChildGroups) > 0 { + childGroups = 1 + } + if selector+serviceRef+ipBlocks+childGroups > 1 { + return errMsg, false + } + return "", true +} + +func (g *groupValidator) validateChildClusterGroup(s *crdv1alpha2.ClusterGroup) (string, bool) { if len(s.Spec.ChildGroups) > 0 { parentGrps, err := g.networkPolicyController.internalGroupStore.GetByIndex(store.ChildGroupIndex, s.Name) if err != nil { @@ -705,41 +690,100 @@ func (g *groupValidator) validateChildGroup(s *crdv1alpha2.ClusterGroup) (string return "", true } +func (g *groupValidator) validateChildGroup(s *crdv1alpha3.Group) (string, bool) { + if len(s.Spec.ChildGroups) > 0 { + parentGrps, err := g.networkPolicyController.internalGroupStore.GetByIndex(store.ChildGroupIndex, s.Name) + if err != nil { + return fmt.Sprintf("error retrieving parents of Group %s/%s: %v", s.Namespace, s.Name, err), false + } + // TODO: relax this constraint when max group nesting level increases. + if len(parentGrps) > 0 { + return fmt.Sprintf("cannot set childGroups for Group %s/%s, who has %d parents", s.Namespace, s.Name, len(parentGrps)), false + } + for _, groupname := range s.Spec.ChildGroups { + childGrp, err := g.networkPolicyController.gLister.Groups(s.Namespace).Get(string(groupname)) + if err != nil { + // the childGroup has not been created yet. + continue + } + // TODO: relax this constraint when max group nesting level increases. + if len(childGrp.Spec.ChildGroups) > 0 { + return fmt.Sprintf("cannot set Group %s/%s as childGroup, who has %d childGroups itself", s.Namespace, string(groupname), len(childGrp.Spec.ChildGroups)), false + } + } + } + return "", true +} + +func (g *groupValidator) validateCG(cg *crdv1alpha2.ClusterGroup) (string, bool) { + reason, allowed := validateAntreaClusterGroupSpec(cg.Spec) + if !allowed { + return reason, allowed + } + // TODO: validate child groups for Group + return g.validateChildClusterGroup(cg) +} + +func (g *groupValidator) validateG(grp *crdv1alpha3.Group) (string, bool) { + reason, allowed := validateAntreaGroupSpec(grp.Spec) + if !allowed { + return reason, allowed + } + // TODO: validate child groups for Group + return g.validateChildGroup(grp) +} + // createValidate validates the CREATE events of Group, ClusterGroup resources. func (g *groupValidator) createValidate(curObj interface{}, userInfo authenticationv1.UserInfo) (string, bool) { var curCG *crdv1alpha2.ClusterGroup + var curG *crdv1alpha3.Group + var reason string + var allowed bool switch curObj.(type) { case *crdv1alpha2.ClusterGroup: curCG = curObj.(*crdv1alpha2.ClusterGroup) + reason, allowed = g.validateCG(curCG) + + case *crdv1alpha3.Group: + curG = curObj.(*crdv1alpha3.Group) + reason, allowed = g.validateG(curG) } - reason, allowed := validateAntreaGroupSpec(curCG.Spec) - if !allowed { - return reason, allowed - } - return g.validateChildGroup(curCG) + return reason, allowed } -// updateValidate validates the UPDATE events of ClusterGroup resources. +// updateValidate validates the UPDATE events of Group, ClusterGroup resources. func (g *groupValidator) updateValidate(curObj, oldObj interface{}, userInfo authenticationv1.UserInfo) (string, bool) { var curCG *crdv1alpha2.ClusterGroup + var curG *crdv1alpha3.Group + var reason string + var allowed bool switch curObj.(type) { case *crdv1alpha2.ClusterGroup: curCG = curObj.(*crdv1alpha2.ClusterGroup) + reason, allowed = g.validateCG(curCG) + case *crdv1alpha3.Group: + curG = curObj.(*crdv1alpha3.Group) + reason, allowed = g.validateG(curG) } - reason, allowed := validateAntreaGroupSpec(curCG.Spec) - if !allowed { - return reason, allowed - } - return g.validateChildGroup(curCG) + return reason, allowed } -// deleteValidate validates the DELETE events of ClusterGroup resources. +// deleteValidate validates the DELETE events of Group, ClusterGroup resources. func (g *groupValidator) deleteValidate(oldObj interface{}, userInfo authenticationv1.UserInfo) (string, bool) { var oldCG *crdv1alpha2.ClusterGroup + var oldG *crdv1alpha3.Group switch oldObj.(type) { case *crdv1alpha2.ClusterGroup: oldCG = oldObj.(*crdv1alpha2.ClusterGroup) + return g.deleteValidateCG(oldCG) + case *crdv1alpha3.Group: + oldG = oldObj.(*crdv1alpha3.Group) + return g.deleteValidateGroup(oldG) } + return "", true +} + +func (g *groupValidator) deleteValidateCG(oldCG *crdv1alpha2.ClusterGroup) (string, bool) { // ClusterGroup with existing ACNP references cannot be deleted. cnps, err := g.networkPolicyController.cnpInformer.Informer().GetIndexer().ByIndex(ClusterGroupIndex, oldCG.Name) if err != nil { @@ -755,3 +799,33 @@ func (g *groupValidator) deleteValidate(oldObj interface{}, userInfo authenticat } return "", true } + +func (g *groupValidator) deleteValidateGroup(oldG *crdv1alpha3.Group) (string, bool) { + // Group with existing ANP references cannot be deleted. + anps, err := g.networkPolicyController.anpInformer.Informer().GetIndexer().ByIndex(GroupIndex, oldG.Name) + if err != nil { + return fmt.Sprintf("error occurred when retrieving Antrea NetworkPolicies that refer to Group %s: %v", oldG.Name, err), false + } + if len(anps) > 0 { + anpNameList := make([]string, len(anps)) + for i := range anps { + anpObj := anps[i].(*crdv1alpha1.NetworkPolicy) + anpNameList[i] = anpObj.Name + } + return fmt.Sprintf("Group %s is referenced by %d Antrea NetworkPolicies: %v", oldG.Name, len(anps), anpNameList), false + } + // Group referenced by other Groups as childGroup cannot be deleted. + parentGrps, err := g.networkPolicyController.internalGroupStore.GetByIndex(store.ChildGroupIndex, oldG.Name) + if err != nil { + return fmt.Sprintf("error retrieving parents of Group %s: %v", oldG.Name, err), false + } + if len(parentGrps) > 0 { + parentGrpNameList := make([]string, len(parentGrps)) + for i := range parentGrps { + grpObject := parentGrps[i].(*types.Group) + parentGrpNameList[i] = grpObject.SourceReference.Name + } + return fmt.Sprintf("Group %s is referenced by %d Groups as childGroup: %v", oldG.Name, len(parentGrps), parentGrpNameList), false + } + return "", true +} diff --git a/pkg/controller/types/group.go b/pkg/controller/types/group.go index e0708879be7..8f52bbe1e91 100644 --- a/pkg/controller/types/group.go +++ b/pkg/controller/types/group.go @@ -102,8 +102,8 @@ type Group struct { // UID is a unique identifier of this internal Group. It is same as that of the ClusterGroup // resource UID. UID types.UID - // Name of the ClusterGroup for which this internal Group is created. - Name string + // Reference of the ClusterGroup/Group for which this internal Group is created. + SourceReference *controlplane.GroupReference // Selector describes how the internal group selects Pods to get their addresses. // Selector is nil if Group is defined with ipBlock, or if it has ServiceReference // and has not been processed by the controller yet / Service cannot be found.