From d3a36d523144724c6ea818151ce7ae77efce0b13 Mon Sep 17 00:00:00 2001 From: Deng Yun Date: Wed, 11 Sep 2024 10:04:12 +0800 Subject: [PATCH] Support NSX default project for SecurityPolicy (#670) Starting from VPC 2.0, it's not allowed to created objects under /orgs/default/projects/default/infra path. So for NetworkPolicy/SecurityPolicy with namespaceSelector, in order to create Groups under Default Project, it's needed to create them under /infra/domains/default/groups/<>. As for Groups under non default Project, it's still allowed to create them under /orgs/default/projects//infra/domains/default/groups. This patch is to: 1. Support NetworkPolicy/SecurityPolicy creation under Default Project. 2. Refactor VPC SecurityPolicy HAPI call process for both creation and deletion. 3. Refactor VPC SecurityPolicy store apply process after creation and deletion. --- .../networkpolicy/networkpolicy_controller.go | 2 +- .../securitypolicy_controller.go | 16 +- .../securitypolicy_controller_test.go | 10 +- pkg/nsx/services/common/types.go | 8 +- pkg/nsx/services/securitypolicy/builder.go | 369 +++++----- .../services/securitypolicy/builder_test.go | 3 +- pkg/nsx/services/securitypolicy/expand.go | 13 +- .../services/securitypolicy/expand_test.go | 2 +- pkg/nsx/services/securitypolicy/firewall.go | 695 ++++++++++++------ .../services/securitypolicy/firewall_test.go | 117 +-- pkg/nsx/services/securitypolicy/parse.go | 16 +- pkg/nsx/services/securitypolicy/wrap.go | 44 +- 12 files changed, 785 insertions(+), 510 deletions(-) diff --git a/pkg/controllers/networkpolicy/networkpolicy_controller.go b/pkg/controllers/networkpolicy/networkpolicy_controller.go index f4d840e2b..7273fa703 100644 --- a/pkg/controllers/networkpolicy/networkpolicy_controller.go +++ b/pkg/controllers/networkpolicy/networkpolicy_controller.go @@ -167,7 +167,7 @@ func (r *NetworkPolicyReconciler) CollectGarbage(ctx context.Context) { for elem := range diffSet { log.V(1).Info("GC collected NetworkPolicy", "ID", elem) metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType) - err = r.Service.DeleteSecurityPolicy(types.UID(elem), false, servicecommon.ResourceTypeNetworkPolicy) + err = r.Service.DeleteSecurityPolicy(types.UID(elem), true, servicecommon.ResourceTypeNetworkPolicy) if err != nil { metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResType) } else { diff --git a/pkg/controllers/securitypolicy/securitypolicy_controller.go b/pkg/controllers/securitypolicy/securitypolicy_controller.go index 6195fc9a2..5f900c0c5 100644 --- a/pkg/controllers/securitypolicy/securitypolicy_controller.go +++ b/pkg/controllers/securitypolicy/securitypolicy_controller.go @@ -88,7 +88,7 @@ func deleteSuccess(r *SecurityPolicyReconciler, _ context.Context, o *v1alpha1.S func (r *SecurityPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { var obj client.Object - if r.Service.NSXConfig.EnableVPCNetwork { + if securitypolicy.IsVPCEnabled(r.Service) { obj = &crdv1alpha1.SecurityPolicy{} } else { obj = &v1alpha1.SecurityPolicy{} @@ -149,6 +149,7 @@ func (r *SecurityPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ResultNormal, nil } + log.Info("reconciling CR to create or update securitypolicy", "securitypolicy", req.NamespacedName) if err := r.Service.CreateOrUpdateSecurityPolicy(realObj); err != nil { if errors.As(err, &nsxutil.RestrictionError{}) { log.Error(err, err.Error(), "securitypolicy", req.NamespacedName) @@ -181,6 +182,7 @@ func (r *SecurityPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque } updateSuccess(r, ctx, realObj) } else { + log.Info("reconciling CR to delete securitypolicy", "securitypolicy", req.NamespacedName) if controllerutil.ContainsFinalizer(obj, finalizerName) { metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType) if err := r.Service.DeleteSecurityPolicy(realObj.UID, false, servicecommon.ResourceTypeSecurityPolicy); err != nil { @@ -242,7 +244,7 @@ func (r *SecurityPolicyReconciler) updateSecurityPolicyStatusConditions(ctx cont } } if conditionsUpdated { - if r.Service.NSXConfig.EnableVPCNetwork { + if securitypolicy.IsVPCEnabled(r.Service) { finalObj := securitypolicy.T1ToVPC(secPolicy) err := r.Client.Status().Update(ctx, finalObj) if err != nil { @@ -288,7 +290,7 @@ func getExistingConditionOfType(conditionType v1alpha1.ConditionType, existingCo func (r *SecurityPolicyReconciler) setupWithManager(mgr ctrl.Manager) error { var blr *builder.Builder - if r.Service.NSXConfig.EnableVPCNetwork { + if securitypolicy.IsVPCEnabled(r.Service) { blr = ctrl.NewControllerManagedBy(mgr).For(&crdv1alpha1.SecurityPolicy{}) } else { blr = ctrl.NewControllerManagedBy(mgr).For(&v1alpha1.SecurityPolicy{}) @@ -330,7 +332,7 @@ func (r *SecurityPolicyReconciler) CollectGarbage(ctx context.Context) { } var objectList client.ObjectList - if r.Service.NSXConfig.EnableVPCNetwork { + if securitypolicy.IsVPCEnabled(r.Service) { objectList = &crdv1alpha1.SecurityPolicyList{} } else { objectList = &v1alpha1.SecurityPolicyList{} @@ -357,9 +359,9 @@ func (r *SecurityPolicyReconciler) CollectGarbage(ctx context.Context) { diffSet := nsxPolicySet.Difference(CRPolicySet) for elem := range diffSet { - log.V(1).Info("GC collected SecurityPolicy CR", "UID", elem) + log.V(1).Info("GC collected SecurityPolicy CR", "securityPolicyUID", elem) metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType) - err = r.Service.DeleteSecurityPolicy(types.UID(elem), false, servicecommon.ResourceTypeSecurityPolicy) + err = r.Service.DeleteSecurityPolicy(types.UID(elem), true, servicecommon.ResourceTypeSecurityPolicy) if err != nil { metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResType) } else { @@ -373,7 +375,7 @@ func reconcileSecurityPolicy(r *SecurityPolicyReconciler, pkgclient client.Clien podPortNames := getAllPodPortNames(pods) log.V(1).Info("pod named port", "podPortNames", podPortNames) var spList client.ObjectList - if r.Service.NSXConfig.EnableVPCNetwork { + if securitypolicy.IsVPCEnabled(r.Service) { spList = &crdv1alpha1.SecurityPolicyList{} } else { spList = &v1alpha1.SecurityPolicyList{} diff --git a/pkg/controllers/securitypolicy/securitypolicy_controller_test.go b/pkg/controllers/securitypolicy/securitypolicy_controller_test.go index e6810e918..2da389dbb 100644 --- a/pkg/controllers/securitypolicy/securitypolicy_controller_test.go +++ b/pkg/controllers/securitypolicy/securitypolicy_controller_test.go @@ -231,7 +231,7 @@ func TestSecurityPolicyReconciler_Reconcile(t *testing.T) { v1sp.ObjectMeta.DeletionTimestamp = &time return nil }) - patch := gomonkey.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isVpcCleanup bool) error { + patch := gomonkey.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isVPCCleanupOrGC bool) error { assert.FailNow(t, "should not be called") return nil }) @@ -247,7 +247,7 @@ func TestSecurityPolicyReconciler_Reconcile(t *testing.T) { v1sp.Finalizers = []string{common.T1SecurityPolicyFinalizerName} return nil }) - patch = gomonkey.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isVpcCleanup bool) error { + patch = gomonkey.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isVPCCleanupOrGC bool) error { return nil }) k8sClient.EXPECT().Update(ctx, gomock.Any(), gomock.Any()).Return(nil) @@ -276,7 +276,7 @@ func TestSecurityPolicyReconciler_GarbageCollector(t *testing.T) { a.Insert("2345") return a }) - patch.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isVpcCleanup bool) error { + patch.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isVPCCleanupOrGC bool) error { return nil }) defer patch.Reset() @@ -306,7 +306,7 @@ func TestSecurityPolicyReconciler_GarbageCollector(t *testing.T) { a.Insert("1234") return a }) - patch.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isVpcCleanup bool) error { + patch.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isVPCCleanupOrGC bool) error { assert.FailNow(t, "should not be called") return nil }) @@ -325,7 +325,7 @@ func TestSecurityPolicyReconciler_GarbageCollector(t *testing.T) { a := sets.New[string]() return a }) - patch.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isVpcCleanup bool) error { + patch.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isVPCCleanupOrGC bool) error { assert.FailNow(t, "should not be called") return nil }) diff --git a/pkg/nsx/services/common/types.go b/pkg/nsx/services/common/types.go index 82e0b0bc3..f4b77e2a3 100644 --- a/pkg/nsx/services/common/types.go +++ b/pkg/nsx/services/common/types.go @@ -48,7 +48,7 @@ const ( TagScopeNSXServiceAccountCRName string = "nsx-op/nsx_service_account_name" TagScopeNSXServiceAccountCRUID string = "nsx-op/nsx_service_account_uid" TagScopeNSXProjectID string = "nsx-op/nsx_project_id" - TagScopeProjectGroupShared string = "nsx-op/is_nsx_project_shared" + TagScopeNSXShareCreatedFor string = "nsx-op/nsx_share_created_for" TagScopeSubnetPortCRName string = "nsx-op/subnetport_name" TagScopeSubnetPortCRUID string = "nsx-op/subnetport_uid" TagScopeIPPoolCRName string = "nsx-op/ippool_name" @@ -77,6 +77,9 @@ const ( TagValueGroupScope string = "scope" TagValueGroupSource string = "source" TagValueGroupDestination string = "destination" + TagValueShareCreatedForInfra string = "infra" + TagValueShareCreatedForProject string = "project" + TagValueShareNotCreated string = "notShared" TagValueGroupAvi string = "avi" TagValueSLB string = "SLB" AnnotationVPCNetworkConfig string = "nsx.vmware.com/vpc_network_config" @@ -122,13 +125,14 @@ const ( RuleSuffixEgressDrop = "egress-isolation" RuleSuffixIngressReject = "ingress-reject" RuleSuffixEgressReject = "egress-reject" + DefaultProject = "default" SecurityPolicyPrefix = "sp" NetworkPolicyPrefix = "np" TargetGroupSuffix = "scope" SrcGroupSuffix = "src" DstGroupSuffix = "dst" IpSetGroupSuffix = "ipset" - SharePrefix = "share" + ShareSuffix = "share" ) var ( diff --git a/pkg/nsx/services/securitypolicy/builder.go b/pkg/nsx/services/securitypolicy/builder.go index b7cef914d..c641ed3f7 100644 --- a/pkg/nsx/services/securitypolicy/builder.go +++ b/pkg/nsx/services/securitypolicy/builder.go @@ -39,10 +39,9 @@ var ( ) func (service *SecurityPolicyService) buildSecurityPolicyName(obj *v1alpha1.SecurityPolicy, createdFor string) string { - if isVpcEnabled(service) { + if IsVPCEnabled(service) { // For VPC scenario, we use obj.Name as the NSX resource display name for both SecurityPolicy and NetworkPolicy. return util.GenerateTruncName(common.MaxNameLength, obj.Name, "", "", "", "") - } prefix := common.SecurityPolicyPrefix if createdFor != common.ResourceTypeSecurityPolicy { @@ -54,7 +53,7 @@ func (service *SecurityPolicyService) buildSecurityPolicyName(obj *v1alpha1.Secu } func (service *SecurityPolicyService) buildSecurityPolicyID(obj *v1alpha1.SecurityPolicy, createdFor string) string { - if isVpcEnabled(service) { + if IsVPCEnabled(service) { return util.GenerateIDByObject(obj) } prefix := common.SecurityPolicyPrefix @@ -65,12 +64,12 @@ func (service *SecurityPolicyService) buildSecurityPolicyID(obj *v1alpha1.Securi return nsxSecurityPolicyID } -func (service *SecurityPolicyService) buildSecurityPolicy(obj *v1alpha1.SecurityPolicy, createdFor string) (*model.SecurityPolicy, *[]model.Group, *[]ProjectShare, error) { +func (service *SecurityPolicyService) buildSecurityPolicy(obj *v1alpha1.SecurityPolicy, createdFor string) (*model.SecurityPolicy, *[]model.Group, *[]GroupShare, error) { var nsxRules []model.Rule var nsxGroups []model.Group - var nsxProjectGroups []model.Group - var nsxProjectShares []model.Share - var projectShares []ProjectShare + var nsxShareGroups []model.Group + var nsxShares []model.Share + var nsxGroupShares []GroupShare log.V(1).Info("building the model SecurityPolicy from CR SecurityPolicy", "object", *obj) nsxSecurityPolicy := &model.SecurityPolicy{} @@ -94,7 +93,7 @@ func (service *SecurityPolicyService) buildSecurityPolicy(obj *v1alpha1.Security for ruleIdx, r := range obj.Spec.Rules { rule := r // A rule containing named port may expand to multiple rules if the name maps to multiple port numbers. - expandRules, buildGroups, buildProjectShares, err := service.buildRuleAndGroups(obj, &rule, ruleIdx, createdFor) + expandRules, buildGroups, buildGroupShares, err := service.buildRuleAndGroups(obj, &rule, ruleIdx, createdFor) if err != nil { log.Error(err, "failed to build rule and groups", "rule", rule, "ruleIndex", ruleIdx) return nil, nil, nil, err @@ -120,13 +119,13 @@ func (service *SecurityPolicyService) buildSecurityPolicy(obj *v1alpha1.Security } currentSet.Clear() - for _, projectShare := range buildProjectShares { - if projectShare != nil { - if !currentSet.Has(*projectShare.share.Id) { - currentSet.Insert(*projectShare.share.Id) - projectShares = append(projectShares, *projectShare) - nsxProjectGroups = append(nsxProjectGroups, *projectShare.shareGroup) - nsxProjectShares = append(nsxProjectShares, *projectShare.share) + for _, item := range buildGroupShares { + if item != nil { + if !currentSet.Has(*item.share.Id) { + currentSet.Insert(*item.share.Id) + nsxGroupShares = append(nsxGroupShares, *item) + nsxShareGroups = append(nsxShareGroups, *item.shareGroup) + nsxShares = append(nsxShares, *item.share) } } } @@ -135,9 +134,10 @@ func (service *SecurityPolicyService) buildSecurityPolicy(obj *v1alpha1.Security nsxSecurityPolicy.Rules = nsxRules nsxSecurityPolicy.Tags = service.buildBasicTags(obj, createdFor) // nsxRules info are included in nsxSecurityPolicy obj - log.Info("built nsxSecurityPolicy", "nsxSecurityPolicy", nsxSecurityPolicy, "nsxGroups", nsxGroups, "nsxProjectGroups", nsxProjectGroups, "nsxProjectShares", nsxProjectShares) + log.Info("built nsxSecurityPolicy", "nsxSecurityPolicy", nsxSecurityPolicy, "nsxGroups", nsxGroups, + "nsxShareGroups", nsxShareGroups, "nsxShares", nsxShares) - return nsxSecurityPolicy, &nsxGroups, &projectShares, nil + return nsxSecurityPolicy, &nsxGroups, &nsxGroupShares, nil } func (service *SecurityPolicyService) buildPolicyGroup(obj *v1alpha1.SecurityPolicy, createdFor string) (*model.Group, string, error) { @@ -224,14 +224,14 @@ func (service *SecurityPolicyService) buildTargetTags(obj *v1alpha1.SecurityPoli targetTags = append(targetTags, tag) } - // In non-VPC network, there is no need to add is_project_shared tag for target groups - if isVpcEnabled(service) { + // In non-VPC network, there is no need to add NSX share createdFor tag for target groups + if IsVPCEnabled(service) { // the target group for policy or rule is always not group shared // because target group doesn't have nameSpace selector targetTags = append(targetTags, model.Tag{ - Scope: String(common.TagScopeProjectGroupShared), - Tag: String("false"), + Scope: String(common.TagScopeNSXShareCreatedFor), + Tag: String(common.TagValueShareNotCreated), }, ) } @@ -375,7 +375,7 @@ func (service *SecurityPolicyService) buildExpressionsMatchExpression(matchExpre // build appliedTo group ID for both policy and rule levels. func (service *SecurityPolicyService) buildAppliedGroupID(obj *v1alpha1.SecurityPolicy, ruleIdx int, createdFor string) string { - if isVpcEnabled(service) { + if IsVPCEnabled(service) { suffix := common.TargetGroupSuffix if ruleIdx != -1 { suffix = fmt.Sprintf("%d_%s", ruleIdx, suffix) @@ -398,8 +398,8 @@ func (service *SecurityPolicyService) buildAppliedGroupID(obj *v1alpha1.Security func (service *SecurityPolicyService) buildAppliedGroupPath(obj *v1alpha1.SecurityPolicy, ruleIdx int, createdFor string) (string, error) { groupID := service.buildAppliedGroupID(obj, ruleIdx, createdFor) - if isVpcEnabled(service) { - vpcInfo, err := service.getVpcInfo(obj.ObjectMeta.Namespace) + if IsVPCEnabled(service) { + vpcInfo, err := service.getVPCInfo(obj.ObjectMeta.Namespace) if err != nil { return "", err } @@ -427,13 +427,15 @@ func (service *SecurityPolicyService) buildAppliedGroupName(obj *v1alpha1.Securi return util.GenerateTruncName(common.MaxNameLength, ruleName, "", common.TargetGroupSuffix, "", "") } -func (service *SecurityPolicyService) buildRuleAndGroups(obj *v1alpha1.SecurityPolicy, rule *v1alpha1.SecurityPolicyRule, ruleIdx int, createdFor string) ([]*model.Rule, []*model.Group, []*ProjectShare, error) { +func (service *SecurityPolicyService) buildRuleAndGroups(obj *v1alpha1.SecurityPolicy, rule *v1alpha1.SecurityPolicyRule, + ruleIdx int, createdFor string, +) ([]*model.Rule, []*model.Group, []*GroupShare, error) { var ruleGroups []*model.Group - var projectShares []*ProjectShare var nsxRuleAppliedGroup *model.Group var nsxRuleSrcGroup *model.Group var nsxRuleDstGroup *model.Group - var nsxProjectShare *ProjectShare + var nsxGroupShare *GroupShare + var nsxGroupShares []*GroupShare var nsxRuleAppliedGroupPath string var nsxRuleDstGroupPath string var nsxRuleSrcGroupPath string @@ -456,7 +458,7 @@ func (service *SecurityPolicyService) buildRuleAndGroups(obj *v1alpha1.SecurityP for _, nsxRule := range nsxRules { if ruleDirection == "IN" { - nsxRuleSrcGroup, nsxRuleSrcGroupPath, nsxRuleDstGroupPath, nsxProjectShare, err = service.buildRuleInGroup( + nsxRuleSrcGroup, nsxRuleSrcGroupPath, nsxRuleDstGroupPath, nsxGroupShare, err = service.buildRuleInGroup( obj, rule, nsxRule, ruleIdx, createdFor) if err != nil { return nil, nil, nil, err @@ -465,11 +467,8 @@ func (service *SecurityPolicyService) buildRuleAndGroups(obj *v1alpha1.SecurityP if nsxRuleSrcGroup != nil { ruleGroups = append(ruleGroups, nsxRuleSrcGroup) } - if nsxProjectShare != nil { - projectShares = append(projectShares, nsxProjectShare) - } } else if ruleDirection == "OUT" { - nsxRuleDstGroup, nsxRuleSrcGroupPath, nsxRuleDstGroupPath, nsxProjectShare, err = service.buildRuleOutGroup( + nsxRuleDstGroup, nsxRuleSrcGroupPath, nsxRuleDstGroupPath, nsxGroupShare, err = service.buildRuleOutGroup( obj, rule, nsxRule, ruleIdx, createdFor) if err != nil { return nil, nil, nil, err @@ -478,9 +477,9 @@ func (service *SecurityPolicyService) buildRuleAndGroups(obj *v1alpha1.SecurityP if nsxRuleDstGroup != nil { ruleGroups = append(ruleGroups, nsxRuleDstGroup) } - if nsxProjectShare != nil { - projectShares = append(projectShares, nsxProjectShare) - } + } + if nsxGroupShare != nil { + nsxGroupShares = append(nsxGroupShares, nsxGroupShare) } nsxRule.SourceGroups = []string{nsxRuleSrcGroupPath} @@ -494,7 +493,7 @@ func (service *SecurityPolicyService) buildRuleAndGroups(obj *v1alpha1.SecurityP ruleGroups = append(ruleGroups, nsxRuleAppliedGroup) nsxRule.Scope = []string{nsxRuleAppliedGroupPath} } - return nsxRules, ruleGroups, projectShares, nil + return nsxRules, ruleGroups, nsxGroupShares, nil } func (service *SecurityPolicyService) buildRuleServiceEntries(port v1alpha1.SecurityPolicyPort, portAddress nsxutil.PortAddress) *data.StructValue { @@ -549,14 +548,16 @@ func (service *SecurityPolicyService) buildRuleAppliedToGroup(obj *v1alpha1.Secu return nsxRuleAppliedGroup, nsxRuleAppliedGroupPath, nil } -func (service *SecurityPolicyService) buildRuleInGroup(obj *v1alpha1.SecurityPolicy, rule *v1alpha1.SecurityPolicyRule, nsxRule *model.Rule, ruleIdx int, createdFor string) (*model.Group, string, string, *ProjectShare, error) { +func (service *SecurityPolicyService) buildRuleInGroup(obj *v1alpha1.SecurityPolicy, rule *v1alpha1.SecurityPolicyRule, + nsxRule *model.Rule, ruleIdx int, createdFor string, +) (*model.Group, string, string, *GroupShare, error) { var nsxRuleSrcGroup *model.Group - var nsxProjectShare *ProjectShare + var nsxGroupShare *GroupShare var nsxRuleSrcGroupPath string var nsxRuleDstGroupPath string var err error if len(rule.Sources) > 0 { - nsxRuleSrcGroup, nsxRuleSrcGroupPath, nsxProjectShare, err = service.buildRulePeerGroup(obj, rule, ruleIdx, true, createdFor) + nsxRuleSrcGroup, nsxRuleSrcGroupPath, nsxGroupShare, err = service.buildRulePeerGroup(obj, rule, ruleIdx, true, createdFor) if err != nil { return nil, "", "", nil, err } @@ -569,12 +570,14 @@ func (service *SecurityPolicyService) buildRuleInGroup(obj *v1alpha1.SecurityPol } else { nsxRuleDstGroupPath = "ANY" } - return nsxRuleSrcGroup, nsxRuleSrcGroupPath, nsxRuleDstGroupPath, nsxProjectShare, nil + return nsxRuleSrcGroup, nsxRuleSrcGroupPath, nsxRuleDstGroupPath, nsxGroupShare, nil } -func (service *SecurityPolicyService) buildRuleOutGroup(obj *v1alpha1.SecurityPolicy, rule *v1alpha1.SecurityPolicyRule, nsxRule *model.Rule, ruleIdx int, createdFor string) (*model.Group, string, string, *ProjectShare, error) { +func (service *SecurityPolicyService) buildRuleOutGroup(obj *v1alpha1.SecurityPolicy, rule *v1alpha1.SecurityPolicyRule, + nsxRule *model.Rule, ruleIdx int, createdFor string, +) (*model.Group, string, string, *GroupShare, error) { var nsxRuleDstGroup *model.Group - var nsxProjectShare *ProjectShare + var nsxGroupShare *GroupShare var nsxRuleSrcGroupPath string var nsxRuleDstGroupPath string var err error @@ -582,7 +585,7 @@ func (service *SecurityPolicyService) buildRuleOutGroup(obj *v1alpha1.SecurityPo nsxRuleDstGroupPath = nsxRule.DestinationGroups[0] } else { if len(rule.Destinations) > 0 { - nsxRuleDstGroup, nsxRuleDstGroupPath, nsxProjectShare, err = service.buildRulePeerGroup(obj, rule, ruleIdx, false, createdFor) + nsxRuleDstGroup, nsxRuleDstGroupPath, nsxGroupShare, err = service.buildRulePeerGroup(obj, rule, ruleIdx, false, createdFor) if err != nil { return nil, "", "", nil, err } @@ -591,14 +594,14 @@ func (service *SecurityPolicyService) buildRuleOutGroup(obj *v1alpha1.SecurityPo } } nsxRuleSrcGroupPath = "ANY" - return nsxRuleDstGroup, nsxRuleSrcGroupPath, nsxRuleDstGroupPath, nsxProjectShare, nil + return nsxRuleDstGroup, nsxRuleSrcGroupPath, nsxRuleDstGroupPath, nsxGroupShare, nil } func (service *SecurityPolicyService) buildRuleID(obj *v1alpha1.SecurityPolicy, rule *v1alpha1.SecurityPolicyRule, ruleIdx int, createdFor string) string { serializedBytes, _ := json.Marshal(rule) ruleHash := fmt.Sprintf("%s", util.Sha1(string(serializedBytes))) ruleIdxStr := fmt.Sprintf("%d", ruleIdx) - if isVpcEnabled(service) { + if IsVPCEnabled(service) { suffix := fmt.Sprintf("%s_%s", ruleIdxStr, ruleHash) return util.GenerateIDByObjectWithSuffix(obj, suffix) } @@ -749,7 +752,7 @@ func (service *SecurityPolicyService) buildRulePeerGroupID(obj *v1alpha1.Securit if isSource == true { suffix = common.SrcGroupSuffix } - if isVpcEnabled(service) { + if IsVPCEnabled(service) { return util.GenerateIDByObjectWithSuffix(obj, fmt.Sprintf("%d_%s", ruleIdx, suffix)) } return util.GenerateID(string(obj.UID), common.SecurityPolicyPrefix, suffix, fmt.Sprintf("%d", ruleIdx)) @@ -768,30 +771,29 @@ func (service *SecurityPolicyService) buildRulePeerGroupName(obj *v1alpha1.Secur return util.GenerateTruncName(common.MaxNameLength, ruleName, "", suffix, "", "") } -func (service *SecurityPolicyService) buildRulePeerGroupPath(obj *v1alpha1.SecurityPolicy, ruleIdx int, isSource, groupShared bool) (string, error) { +func (service *SecurityPolicyService) buildRulePeerGroupPath(obj *v1alpha1.SecurityPolicy, ruleIdx int, isSource, infraGroupShared, projectGroupShared bool, vpcInfo *common.VPCResourceInfo) (string, error) { groupID := service.buildRulePeerGroupID(obj, ruleIdx, isSource) - if isVpcEnabled(service) { - vpcInfo, err := service.getVpcInfo(obj.ObjectMeta.Namespace) - if err != nil { - return "", err + if IsVPCEnabled(service) { + if infraGroupShared { + return fmt.Sprintf("/infra/domains/%s/groups/%s", getDefaultProjectDomain(), groupID), nil } - orgId := (*vpcInfo).OrgID - projectId := (*vpcInfo).ProjectID - vpcId := (*vpcInfo).VPCID - - if groupShared { - return fmt.Sprintf("/orgs/%s/projects/%s/infra/domains/%s/groups/%s", orgId, projectId, getVpcProjectDomain(), groupID), nil + if projectGroupShared { + return fmt.Sprintf("/orgs/%s/projects/%s/infra/domains/%s/groups/%s", vpcInfo.OrgID, vpcInfo.ProjectID, getVPCProjectDomain(), groupID), nil } - return fmt.Sprintf("/orgs/%s/projects/%s/vpcs/%s/groups/%s", orgId, projectId, vpcId, groupID), nil + return fmt.Sprintf("/orgs/%s/projects/%s/vpcs/%s/groups/%s", vpcInfo.OrgID, vpcInfo.ProjectID, vpcInfo.VPCID, groupID), nil } return fmt.Sprintf("/infra/domains/%s/groups/%s", getDomain(service), groupID), nil } -func (service *SecurityPolicyService) buildRulePeerGroup(obj *v1alpha1.SecurityPolicy, rule *v1alpha1.SecurityPolicyRule, ruleIdx int, isSource bool, createdFor string) (*model.Group, string, *ProjectShare, error) { +func (service *SecurityPolicyService) buildRulePeerGroup(obj *v1alpha1.SecurityPolicy, rule *v1alpha1.SecurityPolicyRule, + ruleIdx int, isSource bool, createdFor string, +) (*model.Group, string, *GroupShare, error) { var rulePeers []v1alpha1.SecurityPolicyPeer var ruleDirection string + var err error + var vpcInfo *common.VPCResourceInfo rulePeerGroupID := service.buildRulePeerGroupID(obj, ruleIdx, isSource) rulePeerGroupName := service.buildRulePeerGroupName(obj, ruleIdx, isSource) if isSource == true { @@ -803,6 +805,8 @@ func (service *SecurityPolicyService) buildRulePeerGroup(obj *v1alpha1.SecurityP } groupShared := false + infraGroupShared := false + projectGroupShared := false for _, peer := range rulePeers { if peer.NamespaceSelector != nil { groupShared = true @@ -810,12 +814,26 @@ func (service *SecurityPolicyService) buildRulePeerGroup(obj *v1alpha1.SecurityP } } - rulePeerGroupPath, err := service.buildRulePeerGroupPath(obj, ruleIdx, isSource, groupShared) + if IsVPCEnabled(service) { + vpcInfo, err = service.getVPCInfo(obj.ObjectMeta.Namespace) + if err != nil { + return nil, "", nil, err + } + if groupShared { + if vpcInfo.ProjectID == common.DefaultProject { + infraGroupShared = true + } else { + projectGroupShared = true + } + } + } + + rulePeerGroupPath, err := service.buildRulePeerGroupPath(obj, ruleIdx, isSource, infraGroupShared, projectGroupShared, vpcInfo) if err != nil { return nil, "", nil, err } - peerTags := service.buildPeerTags(obj, rule, ruleIdx, isSource, groupShared, createdFor) + peerTags := service.buildPeerTags(obj, rule, ruleIdx, isSource, infraGroupShared, projectGroupShared, createdFor) rulePeerGroup := model.Group{ Id: &rulePeerGroupID, DisplayName: &rulePeerGroupName, @@ -863,28 +881,37 @@ func (service *SecurityPolicyService) buildRulePeerGroup(obj *v1alpha1.SecurityP return nil, "", nil, err } - if isVpcEnabled(service) && (groupShared == true) { - var projectShare ProjectShare - projectShare.shareGroup = &rulePeerGroup + if IsVPCEnabled(service) && (groupShared == true) { + var projectGroupShare GroupShare + var infraGroupShare GroupShare - var sharedNamespace []string - // Share group with the VPC in which SecurityPolicy rule is put - sharedNamespace = append(sharedNamespace, obj.ObjectMeta.Namespace) + log.V(1).Info("building share in Namespace", "Namespace", obj.ObjectMeta.Namespace) + if infraGroupShared == true { + infraGroupShare.shareGroup = &rulePeerGroup + // Share group with the project in which SecurityPolicy rule is put + sharedWith := service.buildSharedWith(vpcInfo, true, false) + // Build a NSX share resource in infra level + nsxInfraShare, err := service.buildGroupShare(obj, &rulePeerGroup, []string{rulePeerGroupPath}, *sharedWith, vpcInfo, true, false, createdFor) + if err != nil { + log.Error(err, "failed to build NSX infra share", "ruleGroupName", rulePeerGroupName) + return nil, "", nil, err + } + infraGroupShare.share = nsxInfraShare + return nil, rulePeerGroupPath, &infraGroupShare, err + } else { + projectGroupShare.shareGroup = &rulePeerGroup + // Share group with the VPC in which SecurityPolicy rule is put + sharedWith := service.buildSharedWith(vpcInfo, false, true) + // Build a NSX share resource in project level + nsxProjectShare, err := service.buildGroupShare(obj, &rulePeerGroup, []string{rulePeerGroupPath}, *sharedWith, vpcInfo, false, true, createdFor) + if err != nil { + log.Error(err, "failed to build NSX project share", "ruleGroupName", rulePeerGroupName) + return nil, "", nil, err + } + projectGroupShare.share = nsxProjectShare - sharedWith, err := service.buildSharedWith(&sharedNamespace) - if err != nil { - log.Error(err, "failed to build SharedWith path", "ruleGroupName", rulePeerGroupName) - return nil, "", nil, err + return nil, rulePeerGroupPath, &projectGroupShare, err } - // Build a nsx share resource in project level - nsxShare, err := service.buildProjectShare(obj, &rulePeerGroup, []string{rulePeerGroupPath}, *sharedWith, createdFor) - if err != nil { - log.Error(err, "failed to build NSX project share", "ruleGroupName", rulePeerGroupName) - return nil, "", nil, err - } - - projectShare.share = nsxShare - return nil, rulePeerGroupPath, &projectShare, err } return &rulePeerGroup, rulePeerGroupPath, nil, err @@ -910,7 +937,7 @@ func (service *SecurityPolicyService) buildRuleBasicInfo(obj *v1alpha1.SecurityP } displayName, err := service.buildRuleDisplayName(rule, portIdx, portNumber, hasNamedport, createdFor) if err != nil { - log.Error(err, "failed to build rule's display name", "object.UID", obj.UID, "rule", rule, "createdFor", createdFor) + log.Error(err, "failed to build rule's display name", "securityPolicyUID", obj.UID, "rule", rule, "createdFor", createdFor) } nsxRule := model.Rule{ @@ -926,7 +953,7 @@ func (service *SecurityPolicyService) buildRuleBasicInfo(obj *v1alpha1.SecurityP return &nsxRule, nil } -func (service *SecurityPolicyService) buildPeerTags(obj *v1alpha1.SecurityPolicy, rule *v1alpha1.SecurityPolicyRule, ruleIdx int, isSource, groupShared bool, createdFor string) []model.Tag { +func (service *SecurityPolicyService) buildPeerTags(obj *v1alpha1.SecurityPolicy, rule *v1alpha1.SecurityPolicyRule, ruleIdx int, isSource, infraGroupShared, projectGroupShared bool, createdFor string) []model.Tag { basicTags := service.buildBasicTags(obj, createdFor) groupTypeTag := String(common.TagValueGroupDestination) peers := &rule.Destinations @@ -943,10 +970,6 @@ func (service *SecurityPolicyService) buildPeerTags(obj *v1alpha1.SecurityPolicy }) serializedBytes, _ := json.Marshal(*peers) - groupSharedTag := String("false") - if groupShared == true { - groupSharedTag = String("true") - } peerTags := []model.Tag{ { Scope: String(common.TagScopeGroupType), @@ -965,14 +988,30 @@ func (service *SecurityPolicyService) buildPeerTags(obj *v1alpha1.SecurityPolicy peerTags = append(peerTags, tag) } - // In non-VPC network, there is no need to add is_project_shared tag for rule peer groups - if isVpcEnabled(service) { - peerTags = append(peerTags, - model.Tag{ - Scope: String(common.TagScopeProjectGroupShared), - Tag: groupSharedTag, - }, - ) + // In non-VPC network, there is no need to add NSX share createdFor tag for rule peer groups + if IsVPCEnabled(service) { + if infraGroupShared { + peerTags = append(peerTags, + model.Tag{ + Scope: String(common.TagScopeNSXShareCreatedFor), + Tag: String(common.TagValueShareCreatedForInfra), + }, + ) + } else if projectGroupShared { + peerTags = append(peerTags, + model.Tag{ + Scope: String(common.TagScopeNSXShareCreatedFor), + Tag: String(common.TagValueShareCreatedForProject), + }, + ) + } else { + peerTags = append(peerTags, + model.Tag{ + Scope: String(common.TagScopeNSXShareCreatedFor), + Tag: String(common.TagValueShareNotCreated), + }, + ) + } } return peerTags } @@ -987,7 +1026,7 @@ func (service *SecurityPolicyService) updateTargetExpressions(obj *v1alpha1.Secu matchLabelsCount, matchExpressionsCount := 0, 0 memberType, clusterMemberType := "SegmentPort", "Segment" - if isVpcEnabled(service) { + if IsVPCEnabled(service) { memberType, clusterMemberType = "VpcSubnetPort", "VpcSubnetPort" } @@ -1060,7 +1099,7 @@ func (service *SecurityPolicyService) updateTargetExpressions(obj *v1alpha1.Secu } } - if isVpcEnabled(service) { + if IsVPCEnabled(service) { // In VPC level, it doesn't support mixed expression criteria totalCriteriaCount, totalExprCount, err = service.validateSelectorExpressions( matchLabelsCount, @@ -1416,7 +1455,7 @@ func (service *SecurityPolicyService) updatePeerExpressions(obj *v1alpha1.Securi opInValueCount, totalCriteriaCount, totalExprCount := 0, 0, 0 matchLabelsCount, matchExpressionsCount := 0, 0 mixedNsSelector := false - isVpcEnable := isVpcEnabled(service) + isVpcEnable := IsVPCEnabled(service) if len(peer.IPBlocks) > 0 { addresses := data.NewListValue() @@ -1670,52 +1709,41 @@ func (service *SecurityPolicyService) updatePeerExpressions(obj *v1alpha1.Securi return totalCriteriaCount, totalExprCount, nil } -func (service *SecurityPolicyService) buildShareName(nsxProjectName, groupName string) string { - nsxProjectShareName := util.GenerateTruncName(common.MaxNameLength, nsxProjectName, common.SharePrefix, fmt.Sprintf("group-%s", groupName), "", "") - return nsxProjectShareName +func (service *SecurityPolicyService) buildShareName(nsxResourceID, groupName string) string { + nsxShareName := util.GenerateTruncName(common.MaxNameLength, fmt.Sprintf("%s-group-%s", nsxResourceID, groupName), "", common.ShareSuffix, "", "") + return nsxShareName } -func (service *SecurityPolicyService) buildShareID(nsxProjectName, groupID string) string { - nsxProjectShareId := util.GenerateID(nsxProjectName, common.SharePrefix, fmt.Sprintf("group_%s", groupID), "") - return nsxProjectShareId +func (service *SecurityPolicyService) buildShareID(nsxResourceID, groupID string) string { + nsxShareId := util.GenerateID(fmt.Sprintf("%s_group_%s", nsxResourceID, groupID), "", common.ShareSuffix, "") + return nsxShareId } -func (service *SecurityPolicyService) buildShareTags(obj *v1alpha1.SecurityPolicy, projectId string, group *model.Group, createdFor string) []model.Tag { - var scopeOwnerName, scopeOwnerUID string - if createdFor == common.ResourceTypeSecurityPolicy { - scopeOwnerName = common.TagValueScopeSecurityPolicyName - scopeOwnerUID = common.TagValueScopeSecurityPolicyUID - } else { - scopeOwnerName = common.TagScopeNetworkPolicyName - scopeOwnerUID = common.TagScopeNetworkPolicyUID +func (service *SecurityPolicyService) buildShareTags(obj *v1alpha1.SecurityPolicy, infraGroupShared, projectGroupShared bool, createdFor string) []model.Tag { + basicTags := service.buildBasicTags(obj, createdFor) + shareTags := []model.Tag{} + + for _, tag := range basicTags { + shareTags = append(shareTags, tag) } - tags := []model.Tag{ - { - Scope: String(common.TagScopeVersion), - Tag: String(strings.Join(common.TagValueVersion, ".")), - }, - { - Scope: String(common.TagScopeCluster), - Tag: String(getCluster(service)), - }, - { - Scope: String(common.TagScopeNSXProjectID), - Tag: String(projectId), - }, - { - Scope: String(scopeOwnerName), - Tag: String(obj.ObjectMeta.Name), - }, - { - Scope: String(scopeOwnerUID), - Tag: String(string(obj.UID)), - }, - { - Scope: String(common.TagScopeGoupID), - Tag: String(*group.Id), - }, + + if infraGroupShared { + shareTags = append(shareTags, + model.Tag{ + Scope: String(common.TagScopeNSXShareCreatedFor), + Tag: String(common.TagValueShareCreatedForInfra), + }, + ) + } else if projectGroupShared { + shareTags = append(shareTags, + model.Tag{ + Scope: String(common.TagScopeNSXShareCreatedFor), + Tag: String(common.TagValueShareCreatedForProject), + }, + ) } - return tags + + return shareTags } func (service *SecurityPolicyService) buildSharedResource(shareId string, sharedGroupPath []string) (*model.SharedResource, error) { @@ -1758,55 +1786,54 @@ func (service *SecurityPolicyService) buildChildSharedResource(shareId string, s return []*data.StructValue{dataValue.(*data.StructValue)}, nil } -func (service *SecurityPolicyService) buildProjectShare(obj *v1alpha1.SecurityPolicy, group *model.Group, - sharedGroupPath, sharedWith []string, createdFor string, +func (service *SecurityPolicyService) buildGroupShare(obj *v1alpha1.SecurityPolicy, group *model.Group, + sharedGroupPath, sharedWith []string, vpcInfo *common.VPCResourceInfo, infraGroupShared, projectGroupShared bool, createdFor string, ) (*model.Share, error) { - resourceType := common.ResourceTypeShare - vpcInfo, err := service.getVpcInfo(obj.ObjectMeta.Namespace) - if err != nil { - return nil, err - } - projectId := (*vpcInfo).ProjectID + var shareId string + var shareTags []model.Tag + var groupShare model.Share + var shareName string - projectShareId := service.buildShareID(projectId, *group.Id) - projectShareTags := service.buildShareTags(obj, projectId, group, createdFor) - projectShareName := service.buildShareName(projectId, *group.DisplayName) - - childSharedResource, err := service.buildChildSharedResource(projectShareId, sharedGroupPath) + resourceType := common.ResourceTypeShare + projectId := vpcInfo.ProjectID + shareId = service.buildShareID(projectId, *group.Id) + shareName = service.buildShareName(projectId, *group.DisplayName) + shareTags = service.buildShareTags(obj, infraGroupShared, projectGroupShared, createdFor) + childSharedResource, err := service.buildChildSharedResource(shareId, sharedGroupPath) if err != nil { return nil, err } - projectShare := model.Share{ - Id: &projectShareId, - DisplayName: &projectShareName, - Tags: projectShareTags, + groupShare = model.Share{ + Id: &shareId, + DisplayName: &shareName, + Tags: shareTags, ResourceType: &resourceType, SharedWith: sharedWith, - Children: childSharedResource, + // Sharing Strategy is default value: NONE_DESCENDANTS if sharing with non-default projects. + Children: childSharedResource, + } + if infraGroupShared { + // Sharing Strategy must be ALL_DESCENDANTS if sharing with default project. + groupShare.SharingStrategy = String(model.Share_SHARING_STRATEGY_ALL_DESCENDANTS) } - return &projectShare, nil + return &groupShare, nil } -func (service *SecurityPolicyService) buildSharedWith(sharedNamespace *[]string) (*[]string, error) { +func (service *SecurityPolicyService) buildSharedWith(vpcInfo *common.VPCResourceInfo, infraGroupShared, projectGroupShared bool) *[]string { var sharedWith []string - for _, ns := range *sharedNamespace { - log.V(1).Info("building shared with in Namespace", "sharedNamespace", ns) - - vpcInfo, err := service.getVpcInfo(ns) - if err != nil { - return nil, err - } - orgId := (*vpcInfo).OrgID - projectId := (*vpcInfo).ProjectID - vpcId := (*vpcInfo).VPCID - - sharedWithPath := fmt.Sprintf("/orgs/%s/projects/%s/vpcs/%s", orgId, projectId, vpcId) + if infraGroupShared { + sharedWithPath := fmt.Sprintf("/orgs/%s/projects/%s", vpcInfo.OrgID, vpcInfo.ProjectID) sharedWith = append(sharedWith, sharedWithPath) + return &sharedWith } - - return &sharedWith, nil + if projectGroupShared { + sharedWithPath := fmt.Sprintf("/orgs/%s/projects/%s/vpcs/%s", vpcInfo.OrgID, vpcInfo.ProjectID, vpcInfo.VPCID) + sharedWith = append(sharedWith, sharedWithPath) + return &sharedWith + } + return nil } func (service *SecurityPolicyService) getNamespaceUID(ns string) (nsUid types.UID) { diff --git a/pkg/nsx/services/securitypolicy/builder_test.go b/pkg/nsx/services/securitypolicy/builder_test.go index 005e3dead..7c9ef1b5f 100644 --- a/pkg/nsx/services/securitypolicy/builder_test.go +++ b/pkg/nsx/services/securitypolicy/builder_test.go @@ -332,7 +332,7 @@ func TestBuildPeerTags(t *testing.T) { defer patches.Reset() for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.ElementsMatch(t, tt.expectedTags, service.buildPeerTags(tt.inputPolicy, &tt.inputPolicy.Spec.Rules[0], tt.inputIndex, true, false, common.ResourceTypeSecurityPolicy)) + assert.ElementsMatch(t, tt.expectedTags, service.buildPeerTags(tt.inputPolicy, &tt.inputPolicy.Spec.Rules[0], tt.inputIndex, true, false, false, common.ResourceTypeSecurityPolicy)) }) } } @@ -985,7 +985,6 @@ func TestBuildGroupName(t *testing.T) { assert.Equal(t, tc.expId, groupID) }) } - }) t.Run("build applied group name", func(t *testing.T) { diff --git a/pkg/nsx/services/securitypolicy/expand.go b/pkg/nsx/services/securitypolicy/expand.go index 02270528f..b2ae52685 100644 --- a/pkg/nsx/services/securitypolicy/expand.go +++ b/pkg/nsx/services/securitypolicy/expand.go @@ -50,7 +50,6 @@ func (service *SecurityPolicyService) expandRule(obj *v1alpha1.SecurityPolicy, r ruleServiceEntries = append(ruleServiceEntries, serviceEntry) } nsxRule.ServiceEntries = ruleServiceEntries - nsxRules = append(nsxRules, nsxRule) } @@ -87,13 +86,13 @@ func (service *SecurityPolicyService) expandRuleByPort(obj *v1alpha1.SecurityPol } startPort, err = service.resolveNamedPort(obj, rule, port) if err != nil { - // In case there is no more valid ip set selected, so clear the stale ip set group in nsx if stale ips exist + // In case there is no more valid ip set selected, so clear the stale ip set group in NSX if stale ips exist if errors.As(err, &nsxutil.NoEffectiveOption{}) { groups := service.groupStore.GetByIndex(common.TagScopeRuleID, service.buildRuleID(obj, rule, ruleIdx, createdFor)) var ipSetGroup *model.Group for _, group := range groups { ipSetGroup = group - // Clear ip set group in nsx + // Clear ip set group in NSX ipSetGroup.Expression = nil log.V(1).Info("clear ruleIPSetGroup", "ruleIPSetGroup", ipSetGroup) err3 := service.createOrUpdateGroups(obj, []*model.Group{ipSetGroup}) @@ -220,8 +219,8 @@ func (service *SecurityPolicyService) buildRuleIPSetGroupName(ruleModel *model.R func (service *SecurityPolicyService) buildRuleIPSetGroupPath(obj *v1alpha1.SecurityPolicy, ruleModel *model.Rule, groupShared bool) (string, error) { ipSetGroupID := service.buildRuleIPSetGroupID(ruleModel) - if isVpcEnabled(service) { - vpcInfo, err := service.getVpcInfo(obj.ObjectMeta.Namespace) + if IsVPCEnabled(service) { + vpcInfo, err := service.getVPCInfo(obj.ObjectMeta.Namespace) if err != nil { return "", err } @@ -230,7 +229,7 @@ func (service *SecurityPolicyService) buildRuleIPSetGroupPath(obj *v1alpha1.Secu vpcId := (*vpcInfo).VPCID if groupShared { - return fmt.Sprintf("/orgs/%s/projects/%s/infra/domains/%s/groups/%s", orgId, projectId, getVpcProjectDomain(), ipSetGroupID), nil + return fmt.Sprintf("/orgs/%s/projects/%s/infra/domains/%s/groups/%s", orgId, projectId, getVPCProjectDomain(), ipSetGroupID), nil } return fmt.Sprintf("/orgs/%s/projects/%s/vpcs/%s/groups/%s", orgId, projectId, vpcId, ipSetGroupID), nil } @@ -250,7 +249,7 @@ func (service *SecurityPolicyService) buildRuleIPSetGroup(obj *v1alpha1.Security ipSetGroup.DisplayName = &ipSetGroupName // IPSetGroup is always destination group for named port - peerTags := service.buildPeerTags(obj, rule, ruleIdx, false, false, createdFor) + peerTags := service.buildPeerTags(obj, rule, ruleIdx, false, false, false, createdFor) ipSetGroup.Tags = peerTags addresses := data.NewListValue() diff --git a/pkg/nsx/services/securitypolicy/expand_test.go b/pkg/nsx/services/securitypolicy/expand_test.go index 782a4a636..d225ac42b 100644 --- a/pkg/nsx/services/securitypolicy/expand_test.go +++ b/pkg/nsx/services/securitypolicy/expand_test.go @@ -66,7 +66,7 @@ func TestSecurityPolicyService_buildRuleIPGroup(t *testing.T) { DisplayName: &policyGroupName, Expression: []*data.StructValue{blockExpression}, // build ipset group tags from input securitypolicy and securitypolicy rule - Tags: service.buildPeerTags(sp, &rule, 0, false, false, common.ResourceTypeSecurityPolicy), + Tags: service.buildPeerTags(sp, &rule, 0, false, false, false, common.ResourceTypeSecurityPolicy), } type args struct { diff --git a/pkg/nsx/services/securitypolicy/firewall.go b/pkg/nsx/services/securitypolicy/firewall.go index 6819ea993..1df1a1064 100644 --- a/pkg/nsx/services/securitypolicy/firewall.go +++ b/pkg/nsx/services/securitypolicy/firewall.go @@ -38,12 +38,14 @@ type SecurityPolicyService struct { securityPolicyStore *SecurityPolicyStore ruleStore *RuleStore groupStore *GroupStore + infraGroupStore *GroupStore + infraShareStore *ShareStore projectGroupStore *GroupStore - shareStore *ShareStore + projectShareStore *ShareStore vpcService common.VPCServiceProvider } -type ProjectShare struct { +type GroupShare struct { shareGroup *model.Group share *model.Share } @@ -53,7 +55,7 @@ var ( lock = &sync.Mutex{} ) -// GetSecurityService get singleton SecurityPolicyService instance, networkpolicy/securitypolicy controller share the same instance. +// GetSecurityService get singleton SecurityPolicy Service instance, NetworkPolicy/SecurityPolicy controller share the same instance. func GetSecurityService(service common.Service, vpcService common.VPCServiceProvider) *SecurityPolicyService { if securityService == nil { lock.Lock() @@ -75,39 +77,47 @@ func InitializeSecurityPolicy(service common.Service, vpcService common.VPCServi wgDone := make(chan bool) fatalErrors := make(chan error) - wg.Add(5) + wg.Add(7) securityPolicyService := &SecurityPolicyService{Service: service} - if isVpcEnabled(securityPolicyService) { + if IsVPCEnabled(securityPolicyService) { common.TagValueScopeSecurityPolicyName = common.TagScopeSecurityPolicyName common.TagValueScopeSecurityPolicyUID = common.TagScopeSecurityPolicyUID } indexScope := common.TagValueScopeSecurityPolicyUID - securityPolicyService.setUpStore(indexScope) securityPolicyService.vpcService = vpcService - projectGroupShareTag := []model.Tag{ + infraShareTag := []model.Tag{ { - Scope: String(common.TagScopeProjectGroupShared), - Tag: String("true"), + Scope: String(common.TagScopeNSXShareCreatedFor), + Tag: String(common.TagValueShareCreatedForInfra), }, } - projectGroupNotShareTag := []model.Tag{ + projectShareTag := []model.Tag{ { - Scope: String(common.TagScopeProjectGroupShared), - Tag: String("false"), + Scope: String(common.TagScopeNSXShareCreatedFor), + Tag: String(common.TagValueShareCreatedForProject), }, } - if isVpcEnabled(securityPolicyService) { - go securityPolicyService.InitializeResourceStore(&wg, fatalErrors, ResourceTypeGroup, projectGroupNotShareTag, securityPolicyService.groupStore) + notShareTag := []model.Tag{ + { + Scope: String(common.TagScopeNSXShareCreatedFor), + Tag: String(common.TagValueShareNotCreated), + }, + } + + go securityPolicyService.InitializeResourceStore(&wg, fatalErrors, ResourceTypeGroup, infraShareTag, securityPolicyService.infraGroupStore) + go securityPolicyService.InitializeResourceStore(&wg, fatalErrors, ResourceTypeShare, infraShareTag, securityPolicyService.infraShareStore) + go securityPolicyService.InitializeResourceStore(&wg, fatalErrors, ResourceTypeGroup, projectShareTag, securityPolicyService.projectGroupStore) + go securityPolicyService.InitializeResourceStore(&wg, fatalErrors, ResourceTypeShare, projectShareTag, securityPolicyService.projectShareStore) + + if IsVPCEnabled(securityPolicyService) { + go securityPolicyService.InitializeResourceStore(&wg, fatalErrors, ResourceTypeGroup, notShareTag, securityPolicyService.groupStore) } else { go securityPolicyService.InitializeResourceStore(&wg, fatalErrors, ResourceTypeGroup, nil, securityPolicyService.groupStore) } - - go securityPolicyService.InitializeResourceStore(&wg, fatalErrors, ResourceTypeGroup, projectGroupShareTag, securityPolicyService.projectGroupStore) - go securityPolicyService.InitializeResourceStore(&wg, fatalErrors, ResourceTypeShare, nil, securityPolicyService.shareStore) go securityPolicyService.InitializeResourceStore(&wg, fatalErrors, ResourceTypeSecurityPolicy, nil, securityPolicyService.securityPolicyStore) go securityPolicyService.InitializeResourceStore(&wg, fatalErrors, ResourceTypeRule, nil, securityPolicyService.ruleStore) @@ -151,7 +161,20 @@ func (s *SecurityPolicyService) setUpStore(indexScope string) { }), BindingType: model.RuleBindingType(), }} - + s.infraGroupStore = &GroupStore{ResourceStore: common.ResourceStore{ + Indexer: cache.NewIndexer(keyFunc, cache.Indexers{ + indexScope: indexBySecurityPolicyUID, + common.TagScopeNetworkPolicyUID: indexByNetworkPolicyUID, + }), + BindingType: model.GroupBindingType(), + }} + s.infraShareStore = &ShareStore{ResourceStore: common.ResourceStore{ + Indexer: cache.NewIndexer(keyFunc, cache.Indexers{ + indexScope: indexBySecurityPolicyUID, + common.TagScopeNetworkPolicyUID: indexByNetworkPolicyUID, + }), + BindingType: model.ShareBindingType(), + }} s.projectGroupStore = &GroupStore{ResourceStore: common.ResourceStore{ Indexer: cache.NewIndexer(keyFunc, cache.Indexers{ indexScope: indexBySecurityPolicyUID, @@ -159,7 +182,7 @@ func (s *SecurityPolicyService) setUpStore(indexScope string) { }), BindingType: model.GroupBindingType(), }} - s.shareStore = &ShareStore{ResourceStore: common.ResourceStore{ + s.projectShareStore = &ShareStore{ResourceStore: common.ResourceStore{ Indexer: cache.NewIndexer(keyFunc, cache.Indexers{ indexScope: indexBySecurityPolicyUID, common.TagScopeNetworkPolicyUID: indexByNetworkPolicyUID, @@ -187,7 +210,7 @@ func (service *SecurityPolicyService) CreateOrUpdateSecurityPolicy(obj interface } } case *v1alpha1.SecurityPolicy: - if isVpcEnabled(service) { + if IsVPCEnabled(service) { err = service.createOrUpdateVPCSecurityPolicy(obj.(*v1alpha1.SecurityPolicy), common.ResourceTypeSecurityPolicy) } else { // For T1 network SecurityPolicy create/update @@ -385,21 +408,55 @@ func (service *SecurityPolicyService) convertNetworkPolicyPortToSecurityPolicyPo return spPort, nil } -func (service *SecurityPolicyService) getStores() (*SecurityPolicyStore, *RuleStore, *GroupStore, *GroupStore, *ShareStore) { - return service.securityPolicyStore, service.ruleStore, service.groupStore, service.projectGroupStore, service.shareStore +func (service *SecurityPolicyService) getSecurityPolicyResourceStores() (*SecurityPolicyStore, *RuleStore, *GroupStore) { + return service.securityPolicyStore, service.ruleStore, service.groupStore } -func (service *SecurityPolicyService) getFinalSecurityPolicyResource(obj *v1alpha1.SecurityPolicy, createdFor string) (*model.SecurityPolicy, []model.Group, *[]ProjectShare, bool, error) { - securityPolicyStore, ruleStore, groupStore, _, _ := service.getStores() +func (service *SecurityPolicyService) getVPCShareResourceStores() (*GroupStore, *ShareStore, *GroupStore, *ShareStore) { + return service.infraGroupStore, service.infraShareStore, service.projectGroupStore, service.projectShareStore +} + +func (service *SecurityPolicyService) getFinalVPCShareResources(obj *v1alpha1.SecurityPolicy, indexScope string, nsxGroupShares *[]GroupShare, isDefaultProject bool) ([]model.Share, []model.Group) { + var finalShares []model.Share + var finalShareGroups []model.Group + nsxShares := make([]model.Share, 0) + nsxShareGroups := make([]model.Group, 0) + + infraGroupStore, infraShareStore, projectGroupStore, projectShareStore := service.getVPCShareResourceStores() + + // Create/Update NSX shares and NSX share groups + for i := len(*nsxGroupShares) - 1; i >= 0; i-- { + nsxShareGroups = append(nsxShareGroups, *((*nsxGroupShares)[i].shareGroup)) + nsxShares = append(nsxShares, *((*nsxGroupShares)[i].share)) + } + if isDefaultProject { + existingNsxShareGroups := infraGroupStore.GetByIndex(indexScope, string(obj.UID)) + finalShareGroups = service.getUpdateGroups(existingNsxShareGroups, nsxShareGroups) + + existingNsxShares := infraShareStore.GetByIndex(indexScope, string(obj.UID)) + finalShares = service.getUpdateShares(existingNsxShares, nsxShares) + } else { + existingNsxShareGroups := projectGroupStore.GetByIndex(indexScope, string(obj.UID)) + finalShareGroups = service.getUpdateGroups(existingNsxShareGroups, nsxShareGroups) + + existingNsxShares := projectShareStore.GetByIndex(indexScope, string(obj.UID)) + finalShares = service.getUpdateShares(existingNsxShares, nsxShares) + } + + return finalShares, finalShareGroups +} + +func (service *SecurityPolicyService) getFinalSecurityPolicyResource(obj *v1alpha1.SecurityPolicy, createdFor string, isDefaultProject bool) (*model.SecurityPolicy, []model.Group, []model.Share, []model.Group, bool, error) { + securityPolicyStore, ruleStore, groupStore := service.getSecurityPolicyResourceStores() - nsxSecurityPolicy, nsxGroups, projectShares, err := service.buildSecurityPolicy(obj, createdFor) + nsxSecurityPolicy, nsxGroups, nsxGroupShares, err := service.buildSecurityPolicy(obj, createdFor) if err != nil { - log.Error(err, "failed to build SecurityPolicy from CR", "ID", obj.UID) - return nil, nil, nil, false, err + log.Error(err, "failed to build SecurityPolicy from CR", "securityPolicyUID", obj.UID) + return nil, nil, nil, nil, false, err } if len(nsxSecurityPolicy.Scope) == 0 { - log.Info("SecurityPolicy has empty policy-level appliedTo") + log.Info("securityPolicy has empty policy-level appliedTo") } indexScope := common.TagValueScopeSecurityPolicyUID if createdFor == common.ResourceTypeNetworkPolicy { @@ -419,23 +476,24 @@ func (service *SecurityPolicyService) getFinalSecurityPolicyResource(obj *v1alph } existingRules := ruleStore.GetByIndex(indexScope, string(obj.UID)) - finalRules := service.updateRules(existingRules, nsxSecurityPolicy.Rules) + finalRules := service.getUpdateRules(existingRules, nsxSecurityPolicy.Rules) finalSecurityPolicy.Rules = finalRules existingGroups := groupStore.GetByIndex(indexScope, string(obj.UID)) - finalGroups := service.updateGroups(existingGroups, *nsxGroups) + finalGroups := service.getUpdateGroups(existingGroups, *nsxGroups) - if isVpcEnabled(service) { - return finalSecurityPolicy, finalGroups, projectShares, isChanged, nil + if IsVPCEnabled(service) { + finalShares, finalShareGroups := service.getFinalVPCShareResources(obj, indexScope, nsxGroupShares, isDefaultProject) + return finalSecurityPolicy, finalGroups, finalShares, finalShareGroups, isChanged, nil } else { - return finalSecurityPolicy, finalGroups, nil, isChanged, nil + return finalSecurityPolicy, finalGroups, nil, nil, isChanged, nil } } func (service *SecurityPolicyService) createOrUpdateSecurityPolicy(obj *v1alpha1.SecurityPolicy, createdFor string) error { - finalSecurityPolicy, finalGroups, _, isChanged, err := service.getFinalSecurityPolicyResource(obj, createdFor) + finalSecurityPolicy, finalGroups, _, _, isChanged, err := service.getFinalSecurityPolicyResource(obj, createdFor, false) if err != nil { - log.Error(err, "failed to get FinalSecurityPolicy from CR", "ID", obj.UID) + log.Error(err, "failed to get SecurityPolicy resources from CR", "securityPolicyUID", obj.UID) return err } @@ -467,7 +525,7 @@ func (service *SecurityPolicyService) createOrUpdateSecurityPolicy(obj *v1alpha1 return err } - securityPolicyStore, ruleStore, groupStore, _, _ := service.getStores() + securityPolicyStore, ruleStore, groupStore := service.getSecurityPolicyResourceStores() // The steps below know how to deal with NSX resources, if there is MarkedForDelete, then delete it from store, // otherwise add or update it to store. if isChanged { @@ -492,109 +550,46 @@ func (service *SecurityPolicyService) createOrUpdateSecurityPolicy(obj *v1alpha1 } func (service *SecurityPolicyService) createOrUpdateVPCSecurityPolicy(obj *v1alpha1.SecurityPolicy, createdFor string) error { - vpcInfo, err := service.getVpcInfo(obj.ObjectMeta.Namespace) + var err error + var finalGetNSXSecurityPolicy model.SecurityPolicy + isDefaultProject := false + vpcInfo, err := service.getVPCInfo(obj.ObjectMeta.Namespace) if err != nil { return err } + if vpcInfo.ProjectID == common.DefaultProject { + isDefaultProject = true + } - finalSecurityPolicy, finalGroups, projectShares, isChanged, err := service.getFinalSecurityPolicyResource(obj, createdFor) + finalSecurityPolicy, finalGroups, finalShares, finalShareGroups, isChanged, err := service.getFinalSecurityPolicyResource(obj, createdFor, isDefaultProject) if err != nil { - log.Error(err, "failed to get FinalSecurityPolicy from CR", "ID", obj.UID) + log.Error(err, "failed to get SecurityPolicy resources from CR", "securityPolicyUID", obj.UID) return err } - securityPolicyStore, ruleStore, groupStore, projectGroupStore, shareStore := service.getStores() - indexScope := common.TagValueScopeSecurityPolicyUID - if createdFor == common.ResourceTypeNetworkPolicy { - indexScope = common.TagScopeNetworkPolicyUID - } - // WrapHierarchyVpcSecurityPolicy will modify the input security policy rules and move the rules to Children fields for HAPI wrap, // so we need to make a copy for the rules store update. finalRules := finalSecurityPolicy.Rules - nsxProjectGroups := make([]model.Group, 0) - nsxProjectShares := make([]model.Share, 0) - for i := len(*projectShares) - 1; i >= 0; i-- { - nsxProjectGroups = append(nsxProjectGroups, *((*projectShares)[i].shareGroup)) - nsxProjectShares = append(nsxProjectShares, *((*projectShares)[i].share)) - } - - // Create/Update nsx project shares and nsx project level groups - existingNsxProjectGroups := projectGroupStore.GetByIndex(indexScope, string(obj.UID)) - finalProjectGroups := service.updateGroups(existingNsxProjectGroups, nsxProjectGroups) - - existingNsxProjectShares := shareStore.GetByIndex(indexScope, string(obj.UID)) - finalProjectShares := service.updateShares(existingNsxProjectShares, nsxProjectShares) - - if !isChanged && len(finalSecurityPolicy.Rules) == 0 && len(finalGroups) == 0 && len(finalProjectGroups) == 0 && len(finalProjectShares) == 0 { + if !isChanged && len(finalSecurityPolicy.Rules) == 0 && len(finalGroups) == 0 && len(finalShares) == 0 { log.Info("securityPolicy, rules, groups and shares are not changed, skip updating them", "nsxSecurityPolicyId", finalSecurityPolicy.Id) return nil } - - // TODO: Simplify resource wrap and patch for both create/delete. - // 1.Wrap project groups and shares into project child infra. - var projectInfra []*data.StructValue - if len(finalProjectGroups) != 0 || len(finalProjectShares) != 0 { - projectInfra, err = service.wrapHierarchyProjectResources(finalProjectShares, finalProjectGroups) - if err != nil { - log.Error(err, "failed to wrap project groups and shares", "nsxSecurityPolicyId", finalSecurityPolicy.Id) - return err - } + if !isDefaultProject { + finalGetNSXSecurityPolicy, err = service.manipulateSecurityPolicy(finalSecurityPolicy, finalGroups, finalShares, finalShareGroups, false, vpcInfo) + } else { + finalGetNSXSecurityPolicy, err = service.manipulateSecurityPolicyForDefaultProject(finalSecurityPolicy, finalGroups, finalShares, finalShareGroups, false, vpcInfo) } - - // 2.Wrap SecurityPolicy, groups, rules under VPC level together with project groups and shares into one hierarchy resource tree. - orgRoot, err := service.WrapHierarchyVpcSecurityPolicy(finalSecurityPolicy, finalGroups, projectInfra, vpcInfo) if err != nil { - log.Error(err, "failed to wrap SecurityPolicy in VPC", "nsxSecurityPolicyId", finalSecurityPolicy.Id) return err } - // 3.Create/update SecurityPolicy together with groups, rules under VPC level and project groups, shares. - err = service.NSXClient.OrgRootClient.Patch(*orgRoot, &EnforceRevisionCheckParam) - err = nsxutil.NSXApiError(err) + err = service.applySecurityPolicyStore(finalGetNSXSecurityPolicy, finalRules, finalGroups, isChanged) if err != nil { - log.Error(err, "failed to create or update SecurityPolicy in VPC", "nsxSecurityPolicyId", finalSecurityPolicy.Id) return err } - - // Get SecurityPolicy from NSX after HAPI call as NSX renders several fields like `path`/`parent_path`. - finalGetNSXSecurityPolicy, err := service.NSXClient.VPCSecurityClient.Get(vpcInfo.OrgID, vpcInfo.ProjectID, vpcInfo.VPCID, *finalSecurityPolicy.Id) - err = nsxutil.NSXApiError(err) + err = service.applyVPCShareResourceStore(finalShares, finalShareGroups, isDefaultProject) if err != nil { - log.Error(err, "failed to get SecurityPolicy in VPC", "nsxSecurityPolicyId", finalSecurityPolicy.Id) - return err - } - - // TODO: Simplify resource store update for both create/delete. - // The steps below know how to deal with NSX resources, if there is MarkedForDelete, then delete it from store, - // otherwise add or update it to store. - if isChanged { - err = securityPolicyStore.Apply(&finalGetNSXSecurityPolicy) - if err != nil { - log.Error(err, "failed to apply store", "securityPolicy", finalGetNSXSecurityPolicy) - return err - } - } - - err = ruleStore.Apply(&finalRules) - if err != nil { - log.Error(err, "failed to apply store", "nsxRules", finalRules) - return err - } - err = groupStore.Apply(&finalGroups) - if err != nil { - log.Error(err, "failed to apply store", "nsxGroups", finalGroups) - return err - } - err = projectGroupStore.Apply(&finalProjectGroups) - if err != nil { - log.Error(err, "failed to apply store", "nsxProjectGroups", finalProjectGroups) - return err - } - err = shareStore.Apply(&finalProjectShares) - if err != nil { - log.Error(err, "failed to apply store", "nsxProjectShares", finalProjectShares) return err } @@ -602,7 +597,7 @@ func (service *SecurityPolicyService) createOrUpdateVPCSecurityPolicy(obj *v1alp return nil } -func (service *SecurityPolicyService) DeleteSecurityPolicy(obj interface{}, isVpcCleanup bool, createdFor string) error { +func (service *SecurityPolicyService) DeleteSecurityPolicy(obj interface{}, isVPCCleanupOrGC bool, createdFor string) error { var err error switch sp := obj.(type) { case *networkingv1.NetworkPolicy: @@ -610,11 +605,11 @@ func (service *SecurityPolicyService) DeleteSecurityPolicy(obj interface{}, isVp CRPolicySet.Insert(service.BuildNetworkPolicyAllowPolicyID(string(sp.UID))) CRPolicySet.Insert(service.BuildNetworkPolicyIsolationPolicyID(string(sp.UID))) for elem := range CRPolicySet { - err = service.deleteVPCSecurityPolicy(types.UID(elem), createdFor) + err = service.deleteVPCSecurityPolicy(types.UID(elem), isVPCCleanupOrGC, createdFor) } case types.UID: - if isVpcEnabled(service) || isVpcCleanup { - err = service.deleteVPCSecurityPolicy(sp, createdFor) + if IsVPCEnabled(service) || isVPCCleanupOrGC { + err = service.deleteVPCSecurityPolicy(sp, isVPCCleanupOrGC, createdFor) } else { // For T1 network SecurityPolicy deletion err = service.deleteSecurityPolicy(sp) @@ -630,16 +625,16 @@ func (service *SecurityPolicyService) deleteSecurityPolicy(sp types.UID) error { nsxGroups := &g r := make([]model.Rule, 0) nsxRules := &r - securityPolicyStore, ruleStore, groupStore, _, _ := service.getStores() + securityPolicyStore, ruleStore, groupStore := service.getSecurityPolicyResourceStores() - // For normal SecurityPolicy deletion process, which means that SecurityPolicy has corresponding nsx SecurityPolicy object + // For normal SecurityPolicy deletion process, which means that SecurityPolicy has corresponding NSX SecurityPolicy object // And for SecurityPolicy GC or cleanup process, which means that SecurityPolicy doesn't exist in K8s any more - // but still has corresponding nsx SecurityPolicy object. + // but still has corresponding NSX SecurityPolicy object. // We use SecurityPolicy's UID from store to get NSX SecurityPolicy object indexScope := common.TagValueScopeSecurityPolicyUID existingSecurityPolices := securityPolicyStore.GetByIndex(indexScope, string(sp)) if len(existingSecurityPolices) == 0 { - log.Info("NSX security policy is not found in store, skip deleting it", "nsxSecurityPolicyUID", sp) + log.Info("NSX SecurityPolicy is not found in store, skip deleting it", "nsxSecurityPolicyUID", sp) return nil } nsxSecurityPolicy = existingSecurityPolices[0] @@ -651,7 +646,7 @@ func (service *SecurityPolicyService) deleteSecurityPolicy(sp types.UID) error { nsxSecurityPolicy.MarkedForDelete = &MarkedForDelete - // There is no nsx groups/rules in the security policy retrieved from securityPolicy store. + // There is no NSX groups/rules in the security policy retrieved from securityPolicy store. // The groups/rules associated the deleting security policy can only be gotten from group/rule store. existingGroups := groupStore.GetByIndex(indexScope, string(sp)) service.markDeleteGroups(existingGroups, nsxGroups, sp) @@ -696,7 +691,7 @@ func (service *SecurityPolicyService) deleteSecurityPolicy(sp types.UID) error { return nil } -func (service *SecurityPolicyService) deleteVPCSecurityPolicy(sp types.UID, createdFor string) error { +func (service *SecurityPolicyService) deleteVPCSecurityPolicy(sp types.UID, isVPCCleanupOrGC bool, createdFor string) error { var nsxSecurityPolicy *model.SecurityPolicy var err error g := make([]model.Group, 0) @@ -705,21 +700,33 @@ func (service *SecurityPolicyService) deleteVPCSecurityPolicy(sp types.UID, crea nsxRules := &r g1 := make([]model.Group, 0) s := make([]model.Share, 0) - nsxProjectGroups := &g1 - nsxProjectShares := &s - securityPolicyStore, ruleStore, groupStore, projectGroupStore, shareStore := service.getStores() + nsxShares := &s + nsxShareGroups := &g1 + + securityPolicyStore, ruleStore, groupStore := service.getSecurityPolicyResourceStores() + infraGroupStore, infraShareStore, projectGroupStore, projectShareStore := service.getVPCShareResourceStores() - // For normal SecurityPolicy deletion process, which means that SecurityPolicy has corresponding nsx SecurityPolicy object + // For normal SecurityPolicy deletion process, which means that SecurityPolicy has corresponding NSX SecurityPolicy object // And for SecurityPolicy GC or cleanup process, which means that SecurityPolicy doesn't exist in K8s any more - // but still has corresponding nsx SecurityPolicy object. + // but still has corresponding NSX SecurityPolicy object. // We use SecurityPolicy's UID from store to get NSX SecurityPolicy object indexScope := common.TagValueScopeSecurityPolicyUID if createdFor == common.ResourceTypeNetworkPolicy { indexScope = common.TagScopeNetworkPolicyUID } existingSecurityPolices := securityPolicyStore.GetByIndex(indexScope, string(sp)) + existingNsxInfraShares := infraShareStore.GetByIndex(indexScope, string(sp)) + existingNsxInfraShareGroups := infraGroupStore.GetByIndex(indexScope, string(sp)) + if isVPCCleanupOrGC && len(existingSecurityPolices) == 0 && (len(existingNsxInfraShares) != 0 || len(existingNsxInfraShareGroups) != 0) { + // There is a specific case that needs to be handle in GC process, that is, + // When the NSX security policy, rules, and groups at the VPC level are deleted, + // The following infra API call to delete infra share resources fail or NSX Operator restarts suddenly. + // So, there are no more NSX security policy but the related NSX infra share resources became stale. + log.Info("NSX SecurityPolicy is not found in store, but there are stale NSX infra share resource to be GC", "nsxSecurityPolicyUID", sp, "createdFor", createdFor) + return service.gcInfraSharesGroups(sp, indexScope) + } if len(existingSecurityPolices) == 0 { - log.Info("NSX security policy is not found in store, skip deleting it", "nsxSecurityPolicyUID", sp, "createdFor", createdFor) + log.Info("NSX SecurityPolicy is not found in store, skip deleting it", "nsxSecurityPolicyUID", sp, "createdFor", createdFor) return nil } nsxSecurityPolicy = existingSecurityPolices[0] @@ -732,10 +739,14 @@ func (service *SecurityPolicyService) deleteVPCSecurityPolicy(sp types.UID, crea return err } vpcInfo, _ := common.ParseVPCResourcePath(*(nsxSecurityPolicy.Path)) + isDefaultProject := false + if vpcInfo.ProjectID == common.DefaultProject { + isDefaultProject = true + } nsxSecurityPolicy.MarkedForDelete = &MarkedForDelete - // There is no nsx groups/rules in the security policy retrieved from securityPolicy store. + // There is no NSX groups/rules in the security policy retrieved from securityPolicy store. // The groups/rules associated the deleting security policy can only be gotten from group/rule store. existingGroups := groupStore.GetByIndex(indexScope, string(sp)) service.markDeleteGroups(existingGroups, nsxGroups, sp) @@ -744,64 +755,36 @@ func (service *SecurityPolicyService) deleteVPCSecurityPolicy(sp types.UID, crea service.markDeleteRules(existingRules, nsxRules, sp) nsxSecurityPolicy.Rules = *nsxRules - existingNsxProjectGroups := projectGroupStore.GetByIndex(indexScope, string(sp)) - service.markDeleteGroups(existingNsxProjectGroups, nsxProjectGroups, sp) - - existingNsxProjectShares := shareStore.GetByIndex(indexScope, string(sp)) - service.markDeleteShares(existingNsxProjectShares, nsxProjectShares, sp) + var existingNsxShares []*model.Share + var existingNsxGroups []*model.Group + if isDefaultProject { + existingNsxShares = infraShareStore.GetByIndex(indexScope, string(sp)) + existingNsxGroups = infraGroupStore.GetByIndex(indexScope, string(sp)) + } else { + existingNsxShares = projectShareStore.GetByIndex(indexScope, string(sp)) + existingNsxGroups = projectGroupStore.GetByIndex(indexScope, string(sp)) + } + service.markDeleteShares(existingNsxShares, nsxShares, sp) + service.markDeleteGroups(existingNsxGroups, nsxShareGroups, sp) - // WrapHierarchyVpcSecurityPolicy will modify the input security policy, so we need to make a copy for the following store update. + // wrapHierarchyVpcSecurityPolicy will modify the input security policy, so we need to make a copy for the following store update. finalSecurityPolicyCopy := *nsxSecurityPolicy finalSecurityPolicyCopy.Rules = nsxSecurityPolicy.Rules - // 1.Wrap project groups and shares into project child infra. - var projectInfra []*data.StructValue - if len(*nsxProjectShares) != 0 || len(*nsxProjectGroups) != 0 { - projectInfra, err = service.wrapHierarchyProjectResources(*nsxProjectShares, *nsxProjectGroups) - if err != nil { - log.Error(err, "failed to wrap project groups and shares", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) - return err - } - } - - // 2.Wrap SecurityPolicy, groups, rules under VPC level together with project groups and shares into one hierarchy resource tree. - orgRoot, err := service.WrapHierarchyVpcSecurityPolicy(nsxSecurityPolicy, *nsxGroups, projectInfra, &vpcInfo) - if err != nil { - log.Error(err, "failed to wrap SecurityPolicy in VPC", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) - return err - } - - // 3.Create/update SecurityPolicy together with groups, rules under VPC level and project groups, shares. - err = service.NSXClient.OrgRootClient.Patch(*orgRoot, &EnforceRevisionCheckParam) - err = nsxutil.NSXApiError(err) - if err != nil { - log.Error(err, "failed to delete SecurityPolicy in VPC", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) - return err - } - - err = securityPolicyStore.Apply(&finalSecurityPolicyCopy) - if err != nil { - log.Error(err, "failed to apply store", "securityPolicy", finalSecurityPolicyCopy) - return err - } - err = ruleStore.Apply(&finalSecurityPolicyCopy.Rules) - if err != nil { - log.Error(err, "failed to apply store", "nsxRules", finalSecurityPolicyCopy.Rules) - return err + if !isDefaultProject { + _, err = service.manipulateSecurityPolicy(nsxSecurityPolicy, *nsxGroups, *nsxShares, *nsxShareGroups, true, &vpcInfo) + } else { + _, err = service.manipulateSecurityPolicyForDefaultProject(nsxSecurityPolicy, *nsxGroups, *nsxShares, *nsxShareGroups, true, &vpcInfo) } - err = groupStore.Apply(nsxGroups) if err != nil { - log.Error(err, "failed to apply store", "nsxGroups", nsxGroups) return err } - err = projectGroupStore.Apply(nsxProjectGroups) + err = service.applySecurityPolicyStore(finalSecurityPolicyCopy, finalSecurityPolicyCopy.Rules, *nsxGroups, true) if err != nil { - log.Error(err, "failed to apply store", "nsxProjectGroups", nsxProjectGroups) return err } - err = shareStore.Apply(nsxProjectShares) + err = service.applyVPCShareResourceStore(*nsxShares, *nsxShareGroups, isDefaultProject) if err != nil { - log.Error(err, "failed to apply store", "nsxProjectShares", nsxProjectShares) return err } @@ -816,8 +799,8 @@ func (service *SecurityPolicyService) createOrUpdateGroups(obj *v1alpha1.Securit for _, group := range nsxGroups { group.MarkedForDelete = nil finalGroups = append(finalGroups, *group) - if isVpcEnabled(service) { - vpcInfo, err = service.getVpcInfo(obj.ObjectMeta.Namespace) + if IsVPCEnabled(service) { + vpcInfo, err = service.getVPCInfo(obj.ObjectMeta.Namespace) if err != nil { return err } @@ -844,74 +827,19 @@ func (service *SecurityPolicyService) createOrUpdateGroups(obj *v1alpha1.Securit return nil } -func (service *SecurityPolicyService) ListSecurityPolicyID() sets.Set[string] { - indexScope := common.TagValueScopeSecurityPolicyUID - - // List SecurityPolicyID to which groups resources are associated in group store - groupSet := service.groupStore.ListIndexFuncValues(indexScope) - // List SecurityPolicyID to which share resources are associated in share store - shareSet := service.shareStore.ListIndexFuncValues(indexScope) - policySet := service.securityPolicyStore.ListIndexFuncValues(indexScope) - - return groupSet.Union(policySet).Union(shareSet) -} - -func (service *SecurityPolicyService) ListNetworkPolicyID() sets.Set[string] { - // List ListNetworkPolicyID to which groups resources are associated in group store - groupSet := service.groupStore.ListIndexFuncValues(common.TagScopeNetworkPolicyUID) - // List service to which share resources are associated in share store - shareSet := service.shareStore.ListIndexFuncValues(common.TagScopeNetworkPolicyUID) - policySet := service.securityPolicyStore.ListIndexFuncValues(common.TagScopeNetworkPolicyUID) - - return groupSet.Union(policySet).Union(shareSet) -} - -func (service *SecurityPolicyService) Cleanup(ctx context.Context) error { - // Delete all the security policies in store - uids := service.ListSecurityPolicyID() - log.Info("cleaning up security policies created for CR", "count", len(uids)) - for uid := range uids { - select { - case <-ctx.Done(): - return errors.Join(nsxutil.TimeoutFailed, ctx.Err()) - default: - err := service.DeleteSecurityPolicy(types.UID(uid), true, common.ResourceTypeSecurityPolicy) - if err != nil { - return err - } - } - } - - // Delete all the security policies created for network policy in store - uids = service.ListNetworkPolicyID() - log.Info("cleaning up security policies created for network policy", "count", len(uids)) - for uid := range uids { - select { - case <-ctx.Done(): - return errors.Join(nsxutil.TimeoutFailed, ctx.Err()) - default: - err := service.DeleteSecurityPolicy(types.UID(uid), true, common.ResourceTypeNetworkPolicy) - if err != nil { - return err - } - } - } - return nil -} - -func (service *SecurityPolicyService) updateRules(existingRules []*model.Rule, expectedRules []model.Rule) []model.Rule { +func (service *SecurityPolicyService) getUpdateRules(existingRules []*model.Rule, expectedRules []model.Rule) []model.Rule { changed, stale := common.CompareResources(RulesPtrToComparable(existingRules), RulesToComparable(expectedRules)) changedRules, staleRules := ComparableToRules(changed), ComparableToRules(stale) finalRules := make([]model.Rule, 0) for i := len(staleRules) - 1; i >= 0; i-- { // Don't use range, it would copy the element - staleRules[i].MarkedForDelete = &MarkedForDelete // nsx clients need this field to delete the rules + staleRules[i].MarkedForDelete = &MarkedForDelete // NSX clients need this field to delete the rules } finalRules = append(finalRules, staleRules...) finalRules = append(finalRules, changedRules...) return finalRules } -func (service *SecurityPolicyService) updateGroups(existingGroups []*model.Group, expectedGroups []model.Group) []model.Group { +func (service *SecurityPolicyService) getUpdateGroups(existingGroups []*model.Group, expectedGroups []model.Group) []model.Group { changed, stale := common.CompareResources(GroupsPtrToComparable(existingGroups), GroupsToComparable(expectedGroups)) changedGroups, staleGroups := ComparableToGroups(changed), ComparableToGroups(stale) finalGroups := make([]model.Group, 0) @@ -923,7 +851,7 @@ func (service *SecurityPolicyService) updateGroups(existingGroups []*model.Group return finalGroups } -func (service *SecurityPolicyService) updateShares(existingShares []*model.Share, expectedShares []model.Share) []model.Share { +func (service *SecurityPolicyService) getUpdateShares(existingShares []*model.Share, expectedShares []model.Share) []model.Share { changed, stale := common.CompareResources(SharesPtrToComparable(existingShares), SharesToComparable(expectedShares)) changedShares, staleShares := ComparableToShares(changed), ComparableToShares(stale) for i := len(staleShares) - 1; i >= 0; i-- { @@ -974,12 +902,293 @@ func (service *SecurityPolicyService) markDeleteShares(existingShares []*model.S } } -func (service *SecurityPolicyService) getVpcInfo(spNameSpace string) (*common.VPCResourceInfo, error) { - VPCInfo := service.vpcService.ListVPCInfo(spNameSpace) - if len(VPCInfo) == 0 { +// Using hierarchy API call to manipulate SecurityPolicy CRUD on the whole resource tree for non-Default Project. +func (service *SecurityPolicyService) manipulateSecurityPolicy(nsxSecurityPolicy *model.SecurityPolicy, nsxGroups []model.Group, + nsxShares []model.Share, nsxShareGroups []model.Group, isDelete bool, vpcInfo *common.VPCResourceInfo, +) (model.SecurityPolicy, error) { + var err error + var projectInfraResource []*data.StructValue + nsxGetSecurityPolicy := model.SecurityPolicy{} + + if len(nsxShares) != 0 { + // Wrap project groups and shares into project child infra. + projectInfraResource, err = service.wrapHierarchyProjectResources(nsxShares, nsxShareGroups) + if err != nil { + log.Error(err, "failed to wrap NSX project groups and shares", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) + return nsxGetSecurityPolicy, err + } + } + + // Wrap SecurityPolicy, groups, rules under VPC level together with project groups and shares into one hierarchy resource tree. + orgRoot, err := service.wrapHierarchyVpcSecurityPolicy(nsxSecurityPolicy, nsxGroups, projectInfraResource, vpcInfo) + if err != nil { + log.Error(err, "failed to wrap SecurityPolicy in VPC", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) + return nsxGetSecurityPolicy, err + } + + // Create/update or delete SecurityPolicy together with groups, rules under VPC level and project groups, shares. + err = service.NSXClient.OrgRootClient.Patch(*orgRoot, &EnforceRevisionCheckParam) + err = nsxutil.NSXApiError(err) + if err != nil { + log.Error(err, "failed to create/update or delete SecurityPolicy in VPC", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) + return nsxGetSecurityPolicy, err + } + + if !isDelete { + // Get SecurityPolicy from NSX after HAPI call as NSX renders several fields like `path`/`parent_path`. + nsxGetSecurityPolicy, err = service.NSXClient.VPCSecurityClient.Get(vpcInfo.OrgID, vpcInfo.ProjectID, vpcInfo.VPCID, *nsxSecurityPolicy.Id) + err = nsxutil.NSXApiError(err) + if err != nil { + log.Error(err, "failed to get SecurityPolicy in VPC", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) + return nsxGetSecurityPolicy, err + } + } + + return nsxGetSecurityPolicy, nil +} + +// Using hierarchy API call to manipulate SecurityPolicy CRUD on the whole resource tree for Default Project +func (service *SecurityPolicyService) manipulateSecurityPolicyForDefaultProject(nsxSecurityPolicy *model.SecurityPolicy, nsxGroups []model.Group, + nsxShares []model.Share, nsxShareGroups []model.Group, isDelete bool, vpcInfo *common.VPCResourceInfo, +) (model.SecurityPolicy, error) { + var err error + var infraResource *model.Infra + var projectInfraResource []*data.StructValue + nsxGetSecurityPolicy := model.SecurityPolicy{} + + finalStaleShares := make([]model.Share, 0) + finalChangedShares := make([]model.Share, 0) + finalStaleShareGroups := make([]model.Group, 0) + finalChangedShareGroups := make([]model.Group, 0) + + for i := len(nsxShares) - 1; i >= 0; i-- { + if nsxShares[i].MarkedForDelete != nil && (*nsxShares[i].MarkedForDelete == MarkedForDelete) { + finalStaleShares = append(finalStaleShares, nsxShares[i]) + } else { + finalChangedShares = append(finalChangedShares, nsxShares[i]) + } + } + for i := len(nsxShareGroups) - 1; i >= 0; i-- { + if nsxShareGroups[i].MarkedForDelete != nil && (*nsxShareGroups[i].MarkedForDelete == MarkedForDelete) { + finalStaleShareGroups = append(finalStaleShareGroups, nsxShareGroups[i]) + } else { + finalChangedShareGroups = append(finalChangedShareGroups, nsxShareGroups[i]) + } + } + + // For create/update case, + // It's needed to create/update the infra resources before these resources are referred by VPC resources. + if !isDelete && (len(finalChangedShares) != 0 || len(finalChangedShareGroups) != 0) { + // Wrap infra groups and shares into infra child infra. + infraResource, err = service.wrapHierarchyInfraResources(finalChangedShares, finalChangedShareGroups) + if err != nil { + log.Error(err, "failed to wrap NSX infra changed groups and shares", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) + return nsxGetSecurityPolicy, err + } + + err = service.NSXClient.InfraClient.Patch(*infraResource, &EnforceRevisionCheckParam) + err = nsxutil.NSXApiError(err) + if err != nil { + log.Error(err, "failed to create or update NSX infra Resource", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) + return nsxGetSecurityPolicy, err + } + + } + + // Wrap SecurityPolicy, groups, rules under VPC level into one hierarchy resource tree. + orgRoot, err := service.wrapHierarchyVpcSecurityPolicy(nsxSecurityPolicy, nsxGroups, projectInfraResource, vpcInfo) + if err != nil { + log.Error(err, "failed to wrap SecurityPolicy in VPC", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) + return nsxGetSecurityPolicy, err + } + + // Create/update or delete SecurityPolicy together with groups, rules under VPC level. + err = service.NSXClient.OrgRootClient.Patch(*orgRoot, &EnforceRevisionCheckParam) + err = nsxutil.NSXApiError(err) + if err != nil { + log.Error(err, "failed to create/update or delete SecurityPolicy in VPC", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) + return nsxGetSecurityPolicy, err + } + + // For delete/update case, + // The infra share resources can be deleted only after the rules under VPC level which are referring the share resources have been deleted. + if len(finalStaleShares) != 0 || len(finalStaleShareGroups) != 0 { + // Wrap infra groups and shares into infra child infra. + infraResource, err = service.wrapHierarchyInfraResources(finalStaleShares, finalStaleShareGroups) + if err != nil { + log.Error(err, "failed to wrap NSX infra stale groups and shares", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) + return nsxGetSecurityPolicy, err + } + err = service.NSXClient.InfraClient.Patch(*infraResource, &EnforceRevisionCheckParam) + err = nsxutil.NSXApiError(err) + if err != nil { + log.Error(err, "failed to delete NSX infra Resource", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) + return nsxGetSecurityPolicy, err + } + } + + if !isDelete { + // Get SecurityPolicy from NSX after HAPI call as NSX renders several fields like `path`/`parent_path`. + nsxGetSecurityPolicy, err = service.NSXClient.VPCSecurityClient.Get(vpcInfo.OrgID, vpcInfo.ProjectID, vpcInfo.VPCID, *nsxSecurityPolicy.Id) + err = nsxutil.NSXApiError(err) + if err != nil { + log.Error(err, "failed to get SecurityPolicy in VPC", "nsxSecurityPolicyId", nsxSecurityPolicy.Id) + return nsxGetSecurityPolicy, err + } + } + + return nsxGetSecurityPolicy, nil +} + +func (service *SecurityPolicyService) applySecurityPolicyStore(nsxSecurityPolicy model.SecurityPolicy, nsxRules []model.Rule, nsxGroups []model.Group, isChanged bool) error { + var err error + securityPolicyStore, ruleStore, groupStore := service.getSecurityPolicyResourceStores() + if isChanged { + err = securityPolicyStore.Apply(&nsxSecurityPolicy) + if err != nil { + log.Error(err, "failed to apply store", "securityPolicy", nsxSecurityPolicy) + return err + } + } + + err = ruleStore.Apply(&nsxRules) + if err != nil { + log.Error(err, "failed to apply store", "nsxRules", nsxRules) + return err + } + err = groupStore.Apply(&nsxGroups) + if err != nil { + log.Error(err, "failed to apply store", "nsxGroups", nsxGroups) + return err + } + return nil +} + +func (service *SecurityPolicyService) applyVPCShareResourceStore(nsxShares []model.Share, nsxShareGroups []model.Group, isDefaultProject bool) error { + var err error + infraGroupStore, infraShareStore, projectGroupStore, projectShareStore := service.getVPCShareResourceStores() + if isDefaultProject { + err = infraGroupStore.Apply(&nsxShareGroups) + if err != nil { + log.Error(err, "failed to apply store", "nsxInfraGroups", nsxShareGroups) + return err + } + err = infraShareStore.Apply(&nsxShares) + if err != nil { + log.Error(err, "failed to apply store", "nsxInfraShares", nsxShares) + return err + } + } else { + err = projectGroupStore.Apply(&nsxShareGroups) + if err != nil { + log.Error(err, "failed to apply store", "nsxProjectGroups", nsxShareGroups) + return err + } + err = projectShareStore.Apply(&nsxShares) + if err != nil { + log.Error(err, "failed to apply store", "nsxProjectShares", nsxShares) + return err + } + } + return nil +} + +func (service *SecurityPolicyService) ListSecurityPolicyID() sets.Set[string] { + indexScope := common.TagValueScopeSecurityPolicyUID + return service.getGCSecurityPolicyIDSet(indexScope) +} + +func (service *SecurityPolicyService) ListNetworkPolicyID() sets.Set[string] { + indexScope := common.TagScopeNetworkPolicyUID + return service.getGCSecurityPolicyIDSet(indexScope) +} + +func (service *SecurityPolicyService) Cleanup(ctx context.Context) error { + // Delete all the security policies in store + uids := service.ListSecurityPolicyID() + log.Info("cleaning up security policies created for CR", "count", len(uids)) + for uid := range uids { + select { + case <-ctx.Done(): + return errors.Join(nsxutil.TimeoutFailed, ctx.Err()) + default: + err := service.DeleteSecurityPolicy(types.UID(uid), true, common.ResourceTypeSecurityPolicy) + if err != nil { + return err + } + } + } + + // Delete all the security policies created for network policy in store + uids = service.ListNetworkPolicyID() + log.Info("cleaning up security policies created for network policy", "count", len(uids)) + for uid := range uids { + select { + case <-ctx.Done(): + return errors.Join(nsxutil.TimeoutFailed, ctx.Err()) + default: + err := service.DeleteSecurityPolicy(types.UID(uid), true, common.ResourceTypeNetworkPolicy) + if err != nil { + return err + } + } + } + return nil +} + +func (service *SecurityPolicyService) gcInfraSharesGroups(sp types.UID, indexScope string) error { + var err error + var infraResource *model.Infra + var existingNsxShares []*model.Share + var existingNsxGroups []*model.Group + g1 := make([]model.Group, 0) + s := make([]model.Share, 0) + nsxShares := &s + nsxShareGroups := &g1 + + infraGroupStore, infraShareStore, _, _ := service.getVPCShareResourceStores() + existingNsxShares = infraShareStore.GetByIndex(indexScope, string(sp)) + existingNsxGroups = infraGroupStore.GetByIndex(indexScope, string(sp)) + + service.markDeleteShares(existingNsxShares, nsxShares, sp) + service.markDeleteGroups(existingNsxGroups, nsxShareGroups, sp) + + infraResource, err = service.wrapHierarchyInfraResources(*nsxShares, *nsxShareGroups) + if err != nil { + log.Error(err, "failed to wrap NSX infra groups and shares", "securityPolicyUID", sp) + return err + } + err = service.NSXClient.InfraClient.Patch(*infraResource, &EnforceRevisionCheckParam) + err = nsxutil.NSXApiError(err) + if err != nil { + log.Error(err, "failed to delete NSX infra Resource in GC", "securityPolicyUID", sp) + return err + } + return nil +} + +func (service *SecurityPolicyService) getGCSecurityPolicyIDSet(indexScope string) sets.Set[string] { + // List SecurityPolicyID to which groups resources are associated in group store + groupSet := service.groupStore.ListIndexFuncValues(indexScope) + + policySet := service.securityPolicyStore.ListIndexFuncValues(indexScope) + + // List SecurityPolicyID to which share resources are associated in project share/group store + projectShareSet := service.projectShareStore.ListIndexFuncValues(indexScope) + projectGroupSet := service.projectGroupStore.ListIndexFuncValues(indexScope) + // List SecurityPolicyID to which share resources are associated in infra share/group store + infraShareSet := service.infraShareStore.ListIndexFuncValues(indexScope) + infraGroupSet := service.infraGroupStore.ListIndexFuncValues(indexScope) + + return groupSet.Union(policySet).Union(projectShareSet).Union(projectGroupSet).Union(infraShareSet).Union(infraGroupSet) +} + +func (service *SecurityPolicyService) getVPCInfo(spNameSpace string) (*common.VPCResourceInfo, error) { + vpcInfo := service.vpcService.ListVPCInfo(spNameSpace) + if len(vpcInfo) == 0 { errorMsg := fmt.Sprintf("there is no VPC info found for namespace %s", spNameSpace) err := errors.New(errorMsg) return nil, err } - return &VPCInfo[0], nil + return &vpcInfo[0], nil } diff --git a/pkg/nsx/services/securitypolicy/firewall_test.go b/pkg/nsx/services/securitypolicy/firewall_test.go index 160c5d76b..64ca7f129 100644 --- a/pkg/nsx/services/securitypolicy/firewall_test.go +++ b/pkg/nsx/services/securitypolicy/firewall_test.go @@ -17,7 +17,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/client-go/tools/cache" "github.com/vmware-tanzu/nsx-operator/pkg/apis/legacy/v1alpha1" "github.com/vmware-tanzu/nsx-operator/pkg/config" @@ -284,22 +283,7 @@ func TestListSecurityPolicyID(t *testing.T) { service := &SecurityPolicyService{ Service: common.Service{NSXClient: nil}, } - service.securityPolicyStore = &SecurityPolicyStore{ResourceStore: common.ResourceStore{ - Indexer: cache.NewIndexer(keyFunc, cache.Indexers{common.TagValueScopeSecurityPolicyUID: indexBySecurityPolicyUID}), - BindingType: model.SecurityPolicyBindingType(), - }} - service.groupStore = &GroupStore{ResourceStore: common.ResourceStore{ - Indexer: cache.NewIndexer(keyFunc, cache.Indexers{common.TagValueScopeSecurityPolicyUID: indexBySecurityPolicyUID}), - BindingType: model.GroupBindingType(), - }} - service.ruleStore = &RuleStore{ResourceStore: common.ResourceStore{ - Indexer: cache.NewIndexer(keyFunc, cache.Indexers{common.TagValueScopeSecurityPolicyUID: indexBySecurityPolicyUID}), - BindingType: model.RuleBindingType(), - }} - service.shareStore = &ShareStore{ResourceStore: common.ResourceStore{ - Indexer: cache.NewIndexer(keyFunc, cache.Indexers{common.TagValueScopeSecurityPolicyUID: indexBySecurityPolicyUID}), - BindingType: model.ShareBindingType(), - }} + service.setUpStore(common.TagValueScopeSecurityPolicyUID) group := model.Group{} scope := "nsx-op/security_policy_cr_uid" @@ -335,13 +319,24 @@ func TestListSecurityPolicyID(t *testing.T) { t.Fatalf("Failed to add policy to store: %v", err) } - id3 := "shareId" - uuid3 := "shareIdUID" + id3 := "projectShareId" + uuid3 := "projectShareIdUID" share := model.Share{} - share.Id = &id1 + share.Id = &id3 share.UniqueId = &uuid3 share.Tags = []model.Tag{{Scope: &scope, Tag: &id3}} - err = service.shareStore.Add(&share) + err = service.projectShareStore.Add(&share) + if err != nil { + t.Fatalf("Failed to add share to store: %v", err) + } + + id4 := "infraShareId" + uuid4 := "infraShareIdUID" + share1 := model.Share{} + share1.Id = &id4 + share1.UniqueId = &uuid4 + share1.Tags = []model.Tag{{Scope: &scope, Tag: &id4}} + err = service.infraShareStore.Add(&share1) if err != nil { t.Fatalf("Failed to add share to store: %v", err) } @@ -362,6 +357,7 @@ func TestListSecurityPolicyID(t *testing.T) { tests[0].want.Insert(id1) tests[0].want.Insert(id2) tests[0].want.Insert(id3) + tests[0].want.Insert(id4) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := service.ListSecurityPolicyID() @@ -372,7 +368,7 @@ func TestListSecurityPolicyID(t *testing.T) { } } -func TestListUpdateRules(t *testing.T) { +func TestGetUpdateRules(t *testing.T) { r1 := model.Rule{ DisplayName: String("nsxrule1"), Id: String("nsxrule_1"), @@ -435,13 +431,13 @@ func TestListUpdateRules(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - finalRules := service.updateRules(tt.existingRules, tt.expectedRules) + finalRules := service.getUpdateRules(tt.existingRules, tt.expectedRules) assert.Equal(t, tt.finalRulesLen, len(finalRules)) }) } } -func TestListUpdateGroups(t *testing.T) { +func TestGetUpdateGroups(t *testing.T) { mId, mTag, mTag2, mScope := "11111", "11111", "22222", "nsx-op/security_policy_cr_uid" markDelete := true @@ -503,13 +499,13 @@ func TestListUpdateGroups(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - finalGroups := service.updateGroups(tt.existingGroups, tt.expectedGroups) + finalGroups := service.getUpdateGroups(tt.existingGroups, tt.expectedGroups) assert.Equal(t, tt.finalGroupsLen, len(finalGroups)) }) } } -func TestListUpdateShares(t *testing.T) { +func TestGetUpdateShares(t *testing.T) { mId, mTag, mScope := "11111", "11111", "nsx-op/security_policy_cr_uid" s1 := model.Share{ @@ -570,7 +566,7 @@ func TestListUpdateShares(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - finalShares := service.updateShares(tt.existingShares, tt.expectedShares) + finalShares := service.getUpdateShares(tt.existingShares, tt.expectedShares) assert.Equal(t, tt.finalSharesLen, len(finalShares)) }) } @@ -746,7 +742,7 @@ func TestDleleteVPCSecurityPolicy(t *testing.T) { SharedWith: []string{"/org/default/project/projectQuality/vpcs/vpc1"}, } *s1 = append(*s1, projectShare) - assert.NoError(t, s.shareStore.Apply(s1)) + assert.NoError(t, s.projectShareStore.Apply(s1)) patches := gomonkey.ApplyMethodSeq(s.NSXClient.OrgRootClient, "Patch", []gomonkey.OutputCell{{ Values: gomonkey.Params{nil}, @@ -865,14 +861,14 @@ func TestDleleteVPCSecurityPolicy(t *testing.T) { patches := tt.prepareFunc(t, fakeService) defer patches.Reset() - if err := fakeService.deleteVPCSecurityPolicy(tt.args.uid, tt.args.createdFor); (err != nil) != tt.wantErr { + if err := fakeService.deleteVPCSecurityPolicy(tt.args.uid, false, tt.args.createdFor); (err != nil) != tt.wantErr { t.Errorf("deleteVPCSecurityPolicy error = %v, wantErr %v", err, tt.wantErr) } assert.Equal(t, tt.wantSecurityPolicyStoreCount, len(fakeService.securityPolicyStore.ListKeys())) assert.Equal(t, tt.wantRuleStoreCount, len(fakeService.ruleStore.ListKeys())) assert.Equal(t, tt.wantGroupStoreCount, len(fakeService.groupStore.ListKeys())) assert.Equal(t, tt.wantProjectGroupStoreCount, len(fakeService.projectGroupStore.ListKeys())) - assert.Equal(t, tt.wantProjectShareStoreCount, len(fakeService.shareStore.ListKeys())) + assert.Equal(t, tt.wantProjectShareStoreCount, len(fakeService.projectShareStore.ListKeys())) }) } } @@ -913,7 +909,7 @@ func TestCreateOrUpdateSecurityPolicy(t *testing.T) { { name: "successCreateUpdateVPCSecurityPolicy", prepareFunc: func(t *testing.T, s *SecurityPolicyService) *gomonkey.Patches { - patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(s), "getVpcInfo", + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(s), "getVPCInfo", func(s *SecurityPolicyService, spNameSpace string) (*common.VPCResourceInfo, error) { return &VPCInfo[0], nil }) @@ -977,7 +973,7 @@ func TestCreateOrUpdateSecurityPolicy(t *testing.T) { { name: "errorCreateUpdateVPCSecurityPolicy", prepareFunc: func(t *testing.T, s *SecurityPolicyService) *gomonkey.Patches { - patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(s), "getVpcInfo", + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(s), "getVPCInfo", func(s *SecurityPolicyService, spNameSpace string) (*common.VPCResourceInfo, error) { return &VPCInfo[0], nil }) @@ -1027,7 +1023,7 @@ func TestCreateOrUpdateSecurityPolicy(t *testing.T) { assert.Equal(t, tt.wantRuleStoreCount, len(fakeService.ruleStore.ListKeys())) assert.Equal(t, tt.wantGroupStoreCount, len(fakeService.groupStore.ListKeys())) assert.Equal(t, tt.wantProjectGroupStoreCount, len(fakeService.projectGroupStore.ListKeys())) - assert.Equal(t, tt.wantProjectShareStoreCount, len(fakeService.shareStore.ListKeys())) + assert.Equal(t, tt.wantProjectShareStoreCount, len(fakeService.projectShareStore.ListKeys())) }) } } @@ -1047,22 +1043,23 @@ func TestGetFinalSecurityPolicyResouce(t *testing.T) { createdFor string } tests := []struct { - name string - prepareFunc func(*testing.T, *SecurityPolicyService) *gomonkey.Patches - args args - expectedPolicy *model.SecurityPolicy - wantErr bool - wantSecurityPolicyChanged bool - wantRuleStoreCount int - wantGroupStoreCount int - wantProjectGroupStoreCount int + name string + prepareFunc func(*testing.T, *SecurityPolicyService) *gomonkey.Patches + args args + expectedPolicy *model.SecurityPolicy + wantErr bool + wantSecurityPolicyChanged bool + wantRuleStoreCount int + wantGroupStoreCount int + wantShareStoreCount int + wantShareGroupStoreCount int }{ { name: "getFinalSecurityPolicyResouceForVPCMode", prepareFunc: func(t *testing.T, s *SecurityPolicyService) *gomonkey.Patches { s.NSXConfig.EnableVPCNetwork = true - patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(s), "getVpcInfo", + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(s), "getVPCInfo", func(s *SecurityPolicyService, spNameSpace string) (*common.VPCResourceInfo, error) { return &VPCInfo[0], nil }) @@ -1086,18 +1083,19 @@ func TestGetFinalSecurityPolicyResouce(t *testing.T) { Rules: []model.Rule{}, Tags: basicTags, }, - wantErr: false, - wantSecurityPolicyChanged: true, - wantRuleStoreCount: 2, - wantGroupStoreCount: 2, - wantProjectGroupStoreCount: 2, + wantErr: false, + wantSecurityPolicyChanged: true, + wantRuleStoreCount: 2, + wantGroupStoreCount: 2, + wantShareStoreCount: 2, + wantShareGroupStoreCount: 2, }, { name: "getFinalSecurityPolicyResouceForT1", prepareFunc: func(t *testing.T, s *SecurityPolicyService) *gomonkey.Patches { s.NSXConfig.EnableVPCNetwork = false - patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(s), "getVpcInfo", + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(s), "getVPCInfo", func(s *SecurityPolicyService, spNameSpace string) (*common.VPCResourceInfo, error) { return &VPCInfo[0], nil }) @@ -1121,11 +1119,12 @@ func TestGetFinalSecurityPolicyResouce(t *testing.T) { Rules: []model.Rule{}, Tags: basicTags, }, - wantErr: false, - wantSecurityPolicyChanged: true, - wantRuleStoreCount: 2, - wantGroupStoreCount: 4, - wantProjectGroupStoreCount: 0, + wantErr: false, + wantSecurityPolicyChanged: true, + wantRuleStoreCount: 2, + wantGroupStoreCount: 4, + wantShareStoreCount: 0, + wantShareGroupStoreCount: 0, }, } @@ -1138,11 +1137,12 @@ func TestGetFinalSecurityPolicyResouce(t *testing.T) { var finalSecurityPolicy *model.SecurityPolicy var finalGroups []model.Group - var projectShares *[]ProjectShare + var finalShares []model.Share + var finalShareGroups []model.Group var isChanged bool var err error - if finalSecurityPolicy, finalGroups, projectShares, isChanged, err = fakeService.getFinalSecurityPolicyResource(tt.args.spObj, tt.args.createdFor); (err != nil) != tt.wantErr { + if finalSecurityPolicy, finalGroups, finalShares, finalShareGroups, isChanged, err = fakeService.getFinalSecurityPolicyResource(tt.args.spObj, tt.args.createdFor, false); (err != nil) != tt.wantErr { t.Errorf("getFinalSecurityPolicyResouce error = %v, wantErr %v", err, tt.wantErr) } @@ -1153,9 +1153,10 @@ func TestGetFinalSecurityPolicyResouce(t *testing.T) { assert.Equal(t, tt.wantRuleStoreCount, len(finalSecurityPolicy.Rules)) if fakeService.NSXConfig.EnableVPCNetwork { - assert.Equal(t, tt.wantProjectGroupStoreCount, len(*projectShares)) + assert.Equal(t, tt.wantShareStoreCount, len(finalShares)) + assert.Equal(t, tt.wantShareGroupStoreCount, len(finalShareGroups)) } else { - assert.Equal(t, (*[]ProjectShare)(nil), projectShares) + assert.Equal(t, ([]model.Share)(nil), finalShares) } }) } diff --git a/pkg/nsx/services/securitypolicy/parse.go b/pkg/nsx/services/securitypolicy/parse.go index 1ba794296..cb4ff42e7 100644 --- a/pkg/nsx/services/securitypolicy/parse.go +++ b/pkg/nsx/services/securitypolicy/parse.go @@ -49,16 +49,20 @@ func getDomain(service *SecurityPolicyService) string { return getCluster(service) } -func getVpcProjectDomain() string { +func getVPCProjectDomain() string { return "default" } -func isVpcEnabled(service *SecurityPolicyService) bool { +func getDefaultProjectDomain() string { + return "default" +} + +func IsVPCEnabled(service *SecurityPolicyService) bool { return service.NSXConfig.EnableVPCNetwork } func getScopeCluserTag(service *SecurityPolicyService) string { - if isVpcEnabled(service) { + if IsVPCEnabled(service) { return common.TagScopeCluster } else { return common.TagScopeNCPCluster @@ -66,7 +70,7 @@ func getScopeCluserTag(service *SecurityPolicyService) string { } func getScopePodTag(service *SecurityPolicyService) string { - if isVpcEnabled(service) { + if IsVPCEnabled(service) { return common.TagScopePodUID } else { return common.TagScopeNCPPod @@ -74,7 +78,7 @@ func getScopePodTag(service *SecurityPolicyService) string { } func getScopeVMInterfaceTag(service *SecurityPolicyService) string { - if isVpcEnabled(service) { + if IsVPCEnabled(service) { return common.TagScopeSubnetPortCRUID } else { return common.TagScopeNCPVNETInterface @@ -82,7 +86,7 @@ func getScopeVMInterfaceTag(service *SecurityPolicyService) string { } func getScopeNamespaceUIDTag(service *SecurityPolicyService, isVMNameSpace bool) string { - if isVpcEnabled(service) { + if IsVPCEnabled(service) { if isVMNameSpace { return common.TagScopeVMNamespaceUID } else { diff --git a/pkg/nsx/services/securitypolicy/wrap.go b/pkg/nsx/services/securitypolicy/wrap.go index f70934dd4..19768b0d7 100644 --- a/pkg/nsx/services/securitypolicy/wrap.go +++ b/pkg/nsx/services/securitypolicy/wrap.go @@ -139,9 +139,9 @@ func (service *SecurityPolicyService) wrapSecurityPolicy(sp *model.SecurityPolic return securityPolicyChildren, nil } -// WrapHierarchyVpcSecurityPolicy wrap the security policy with groups and rules in VPC level and associated project infra children including project shares and groups +// wrapHierarchyVpcSecurityPolicy wrap the security policy with groups and rules in VPC level and associated project infra children including project shares and groups // into one hierarchy resource tree for OrgRootClient to patch. -func (service *SecurityPolicyService) WrapHierarchyVpcSecurityPolicy(sp *model.SecurityPolicy, gs []model.Group, projectInfraChildren []*data.StructValue, +func (service *SecurityPolicyService) wrapHierarchyVpcSecurityPolicy(sp *model.SecurityPolicy, gs []model.Group, projectInfraChildren []*data.StructValue, vpcInfo *common.VPCResourceInfo, ) (*model.OrgRoot, error) { orgID := (*vpcInfo).OrgID @@ -287,6 +287,8 @@ func (service *SecurityPolicyService) wrapChildTargetInfra(children []*data.Stru targetType := common.ResourceTypeInfra resourceType := common.ResourceTypeChildResourceReference + // This is the outermost layer of the hierarchy project child infra in VPC mode. + // It doesn't need ID field. childInfra := model.ChildResourceReference{ ResourceType: resourceType, TargetType: &targetType, @@ -316,18 +318,46 @@ func (service *SecurityPolicyService) wrapHierarchyProjectResources(shares []mod return nil, err } domainReferenceChildren = append(domainReferenceChildren, groupsChildren...) - domainId := getVpcProjectDomain() + domainId := getVPCProjectDomain() domainTargetChildren, err := service.wrapDomainResource(domainReferenceChildren, domainId) if err != nil { return nil, err } infraChildren = append(infraChildren, domainTargetChildren...) - // This is the outermost layer of the hierarchy project child infra in VPC mode. - // It doesn't need ID field. - projectInfraChildren, err := service.wrapChildTargetInfra(infraChildren) + wrapProjInfraChildren, err := service.wrapChildTargetInfra(infraChildren) + if err != nil { + return nil, err + } + return wrapProjInfraChildren, nil +} + +// wrapHierarchyInfraResources wrap the infra shares and groups into a infra children in VPC mode. +func (service *SecurityPolicyService) wrapHierarchyInfraResources(shares []model.Share, groups []model.Group) (*model.Infra, error) { + var domainReferenceChildren []*data.StructValue + var infraChildren []*data.StructValue + + shareChildren, err := service.wrapShares(shares) + if err != nil { + return nil, err + } + infraChildren = append(infraChildren, shareChildren...) + + groupsChildren, err := service.wrapGroups(groups) + if err != nil { + return nil, err + } + domainReferenceChildren = append(domainReferenceChildren, groupsChildren...) + domainId := getDefaultProjectDomain() + domainTargetChildren, err := service.wrapDomainResource(domainReferenceChildren, domainId) + if err != nil { + return nil, err + } + infraChildren = append(infraChildren, domainTargetChildren...) + + wrapInfra, err := service.wrapInfra(infraChildren) if err != nil { return nil, err } - return projectInfraChildren, nil + return wrapInfra, nil }