From 176c7bd221c76c812d09007f9b7626f992c2fc1e Mon Sep 17 00:00:00 2001 From: Deng Yun Date: Sat, 5 Oct 2024 10:10:56 +0000 Subject: [PATCH] remove_np_sp_finalizer --- .../networkpolicy/networkpolicy_controller.go | 100 ++++++----- .../securitypolicy_controller.go | 106 ++++++++---- .../securitypolicy_controller_test.go | 60 +++++-- pkg/nsx/services/common/types.go | 163 +++++++++--------- pkg/nsx/services/securitypolicy/builder.go | 7 + .../services/securitypolicy/builder_test.go | 9 + pkg/nsx/services/securitypolicy/firewall.go | 14 +- .../services/securitypolicy/firewall_test.go | 109 +++++++----- pkg/nsx/services/securitypolicy/store.go | 18 ++ pkg/util/utils.go | 4 + 10 files changed, 378 insertions(+), 212 deletions(-) diff --git a/pkg/controllers/networkpolicy/networkpolicy_controller.go b/pkg/controllers/networkpolicy/networkpolicy_controller.go index 66097ee98..16f67a359 100644 --- a/pkg/controllers/networkpolicy/networkpolicy_controller.go +++ b/pkg/controllers/networkpolicy/networkpolicy_controller.go @@ -18,7 +18,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" "github.com/vmware-tanzu/nsx-operator/pkg/logger" @@ -100,21 +99,21 @@ func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reques metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerSyncTotal, MetricResType) if err := r.Client.Get(ctx, req.NamespacedName, networkPolicy); err != nil { - log.Error(err, "unable to fetch network policy", "req", req.NamespacedName) - return ResultNormal, client.IgnoreNotFound(err) + // IgnoreNotFound returns nil on NotFound errors. + if client.IgnoreNotFound(err) == nil { + if err := r.deleteNetworkPolicyByName(req.Name, req.Namespace); err != nil { + log.Error(err, "failed to delete NetworkPolicy", "networkpolicy", req.NamespacedName) + return ResultRequeue, err + } + return ResultNormal, nil + } + // In case that client is unable to check CR + log.Error(err, "client is unable to fetch NetworkPolicy CR", "req", req.NamespacedName) + return ResultRequeue, err } if networkPolicy.ObjectMeta.DeletionTimestamp.IsZero() { metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerUpdateTotal, MetricResType) - if !controllerutil.ContainsFinalizer(networkPolicy, servicecommon.NetworkPolicyFinalizerName) { - controllerutil.AddFinalizer(networkPolicy, servicecommon.NetworkPolicyFinalizerName) - if err := r.Client.Update(ctx, networkPolicy); err != nil { - log.Error(err, "add finalizer", "networkpolicy", req.NamespacedName) - updateFail(r, ctx, networkPolicy, &err) - return ResultRequeue, err - } - log.V(1).Info("added finalizer on networkpolicy", "networkpolicy", req.NamespacedName) - } if err := r.Service.CreateOrUpdateSecurityPolicy(networkPolicy); err != nil { if errors.As(err, &nsxutil.RestrictionError{}) { @@ -135,25 +134,14 @@ func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reques updateSuccess(r, ctx, networkPolicy) cleanNetworkPolicyErrorAnnotation(ctx, networkPolicy, r.Client) } else { - if controllerutil.ContainsFinalizer(networkPolicy, servicecommon.NetworkPolicyFinalizerName) { - metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType) - if err := r.Service.DeleteSecurityPolicy(networkPolicy, false, false, servicecommon.ResourceTypeNetworkPolicy); err != nil { - log.Error(err, "deletion failed, would retry exponentially", "networkpolicy", req.NamespacedName) - deleteFail(r, ctx, networkPolicy, &err) - return ResultRequeue, err - } - controllerutil.RemoveFinalizer(networkPolicy, servicecommon.NetworkPolicyFinalizerName) - if err := r.Client.Update(ctx, networkPolicy); err != nil { - log.Error(err, "deletion failed, would retry exponentially", "networkpolicy", req.NamespacedName) - deleteFail(r, ctx, networkPolicy, &err) - return ResultRequeue, err - } - log.V(1).Info("removed finalizer", "networkpolicy", req.NamespacedName) - deleteSuccess(r, ctx, networkPolicy) - } else { - // only print a message because it's not a normal case - log.Info("finalizers cannot be recognized", "networkpolicy", req.NamespacedName) + metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType) + + if err := r.Service.DeleteSecurityPolicy(networkPolicy, false, false, servicecommon.ResourceTypeNetworkPolicy); err != nil { + log.Error(err, "deletion failed, would retry exponentially", "networkpolicy", req.NamespacedName) + deleteFail(r, ctx, networkPolicy, &err) + return ResultRequeue, err } + deleteSuccess(r, ctx, networkPolicy) } return ResultNormal, nil @@ -186,19 +174,12 @@ func (r *NetworkPolicyReconciler) CollectGarbage(ctx context.Context) { if len(nsxPolicySet) == 0 { return } - policyList := &networkingv1.NetworkPolicyList{} - err := r.Client.List(ctx, policyList) + + CRPolicySet, err := r.listNetworkPolciyCRIDs() if err != nil { - log.Error(err, "failed to list NetworkPolicy") return } - CRPolicySet := sets.New[string]() - for _, policy := range policyList.Items { - CRPolicySet.Insert(r.Service.BuildNetworkPolicyAllowPolicyID(string(policy.UID))) - CRPolicySet.Insert(r.Service.BuildNetworkPolicyIsolationPolicyID(string(policy.UID))) - } - diffSet := nsxPolicySet.Difference(CRPolicySet) for elem := range diffSet { log.V(1).Info("GC collected NetworkPolicy", "ID", elem) @@ -212,6 +193,47 @@ func (r *NetworkPolicyReconciler) CollectGarbage(ctx context.Context) { } } +func (r *NetworkPolicyReconciler) deleteNetworkPolicyByName(name, ns string) error { + nsxSecurityPolicies := r.Service.ListNetworkPolicyByNamespacedName(name, ns) + + CRPolicySet, err := r.listNetworkPolciyCRIDs() + if err != nil { + return err + } + + for _, item := range nsxSecurityPolicies { + uid := nsxutil.FindTag(item.Tags, servicecommon.TagScopeNetworkPolicyUID) + if CRPolicySet.Has(uid) { + log.Info("skipping deletion, NetworkPolicy CR still exists in K8s", "networkPolicyUID", uid, "nsxSecurityPolicyId", *item.Id) + continue + } + + log.Info("deleting NetworkPolicy", "networkPolicyUID", uid, "nsxSecurityPolicyId", *item.Id) + if err := r.Service.DeleteSecurityPolicy(types.UID(uid), false, false, servicecommon.ResourceTypeNetworkPolicy); err != nil { + log.Error(err, "failed to delete NetworkPolicy", "networkPolicyUID", uid, "nsxSecurityPolicyId", *item.Id) + return err + } + log.Info("successfully deleted NetworkPolicy", "networkPolicyUID", uid, "nsxSecurityPolicyId", *item.Id) + } + return nil +} + +func (r *NetworkPolicyReconciler) listNetworkPolciyCRIDs() (sets.Set[string], error) { + networkPolicyList := &networkingv1.NetworkPolicyList{} + err := r.Client.List(context.Background(), networkPolicyList) + if err != nil { + log.Error(err, "failed to list NetworkPolicy CRs") + return nil, err + } + + CRPolicySet := sets.New[string]() + for _, policy := range networkPolicyList.Items { + CRPolicySet.Insert(r.Service.BuildNetworkPolicyAllowPolicyID(string(policy.UID))) + CRPolicySet.Insert(r.Service.BuildNetworkPolicyIsolationPolicyID(string(policy.UID))) + } + return CRPolicySet, nil +} + func StartNetworkPolicyController(mgr ctrl.Manager, commonService servicecommon.Service, vpcService servicecommon.VPCServiceProvider) { networkPolicyReconcile := NetworkPolicyReconciler{ Client: mgr.GetClient(), diff --git a/pkg/controllers/securitypolicy/securitypolicy_controller.go b/pkg/controllers/securitypolicy/securitypolicy_controller.go index 830dec59d..d96dafa5a 100644 --- a/pkg/controllers/securitypolicy/securitypolicy_controller.go +++ b/pkg/controllers/securitypolicy/securitypolicy_controller.go @@ -137,12 +137,21 @@ func (r *SecurityPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque obj = &v1alpha1.SecurityPolicy{} } - log.Info("reconciling securitypolicy CR", "securitypolicy", req.NamespacedName) + log.Info("reconciling SecurityPolicy CR", "securitypolicy", req.NamespacedName) metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerSyncTotal, MetricResType) if err := r.Client.Get(ctx, req.NamespacedName, obj); err != nil { - log.Error(err, "unable to fetch security policy CR", "req", req.NamespacedName) - return ResultNormal, client.IgnoreNotFound(err) + // IgnoreNotFound returns nil on NotFound errors. + if client.IgnoreNotFound(err) == nil { + if err := r.deleteSecurityPolicyByName(req.Name, req.Namespace); err != nil { + log.Error(err, "failed to delete SecurityPolicy", "securitypolicy", req.NamespacedName) + return ResultRequeue, err + } + return ResultNormal, nil + } + // In case that client is unable to check CR + log.Error(err, "client is unable to fetch SecurityPolicy CR", "req", req.NamespacedName) + return ResultRequeue, err } isZero := false @@ -152,7 +161,6 @@ func (r *SecurityPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque case *crdv1alpha1.SecurityPolicy: o := obj.(*crdv1alpha1.SecurityPolicy) isZero = o.ObjectMeta.DeletionTimestamp.IsZero() - finalizerName = servicecommon.SecurityPolicyFinalizerName realObj = securitypolicy.VPCToT1(o) case *v1alpha1.SecurityPolicy: realObj = obj.(*v1alpha1.SecurityPolicy) @@ -170,15 +178,6 @@ func (r *SecurityPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque if isZero { metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerUpdateTotal, MetricResType) - if !controllerutil.ContainsFinalizer(obj, finalizerName) { - controllerutil.AddFinalizer(obj, finalizerName) - if err := r.Client.Update(ctx, obj); err != nil { - log.Error(err, "add finalizer", "securitypolicy", req.NamespacedName) - updateFail(r, ctx, realObj, &err) - return ResultRequeue, err - } - log.V(1).Info("added finalizer on securitypolicy CR", "securitypolicy", req.NamespacedName) - } if isCRInSysNs, err := util.IsSystemNamespace(r.Client, req.Namespace, nil); err != nil { err = errors.New("fetch namespace associated with security policy CR failed") @@ -213,25 +212,24 @@ func (r *SecurityPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque cleanSecurityPolicyErrorAnnotation(ctx, realObj, securitypolicy.IsVPCEnabled(r.Service), r.Client) } else { log.Info("reconciling CR to delete securitypolicy", "securitypolicy", req.NamespacedName) + metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType) + + // For T1 upgrade, the upgraded CRs still has finalizer if controllerutil.ContainsFinalizer(obj, finalizerName) { - metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType) - if err := r.Service.DeleteSecurityPolicy(realObj.UID, false, false, servicecommon.ResourceTypeSecurityPolicy); err != nil { - log.Error(err, "deletion failed, would retry exponentially", "securitypolicy", req.NamespacedName) - deleteFail(r, ctx, realObj, &err) - return ResultRequeue, err - } controllerutil.RemoveFinalizer(obj, finalizerName) if err := r.Client.Update(ctx, obj); err != nil { - log.Error(err, "deletion failed, would retry exponentially", "securitypolicy", req.NamespacedName) + log.Error(err, "finalizer remove failed, would retry exponentially", "securitypolicy", req.NamespacedName) deleteFail(r, ctx, realObj, &err) return ResultRequeue, err } log.V(1).Info("removed finalizer", "securitypolicy", req.NamespacedName) - deleteSuccess(r, ctx, realObj) - } else { - // only print a message because it's not a normal case - log.Info("finalizers cannot be recognized", "securitypolicy", req.NamespacedName) } + if err := r.Service.DeleteSecurityPolicy(realObj.UID, false, false, servicecommon.ResourceTypeSecurityPolicy); err != nil { + log.Error(err, "deletion failed, would retry exponentially", "securitypolicy", req.NamespacedName) + deleteFail(r, ctx, realObj, &err) + return ResultRequeue, err + } + deleteSuccess(r, ctx, realObj) } return ResultNormal, nil @@ -361,16 +359,60 @@ func (r *SecurityPolicyReconciler) CollectGarbage(ctx context.Context) { return } + CRPolicySet, err := r.listSecurityPolciyCRIDs() + if err != nil { + return + } + + diffSet := nsxPolicySet.Difference(CRPolicySet) + for elem := range diffSet { + 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), true, false, servicecommon.ResourceTypeSecurityPolicy) + if err != nil { + metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResType) + } else { + metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteSuccessTotal, MetricResType) + } + } +} + +func (r *SecurityPolicyReconciler) deleteSecurityPolicyByName(name, ns string) error { + nsxSecurityPolicies := r.Service.ListSecurityPolicyByNamespacedName(name, ns) + + CRPolicySet, err := r.listSecurityPolciyCRIDs() + if err != nil { + return err + } + + for _, item := range nsxSecurityPolicies { + uid := nsxutil.FindTag(item.Tags, servicecommon.TagValueScopeSecurityPolicyUID) + if CRPolicySet.Has(uid) { + log.Info("skipping deletion, SecurityPolicy CR still exists in K8s", "securityPolicyUID", uid, "nsxSecurityPolicyId", *item.Id) + continue + } + + log.Info("deleting SecurityPolicy", "securityPolicyUID", uid, "nsxSecurityPolicyId", *item.Id) + if err := r.Service.DeleteSecurityPolicy(types.UID(uid), false, false, servicecommon.ResourceTypeSecurityPolicy); err != nil { + log.Error(err, "failed to delete SecurityPolicy", "securityPolicyUID", uid, "nsxSecurityPolicyId", *item.Id) + return err + } + log.Info("successfully deleted SecurityPolicy", "securityPolicyUID", uid, "nsxSecurityPolicyId", *item.Id) + } + return nil +} + +func (r *SecurityPolicyReconciler) listSecurityPolciyCRIDs() (sets.Set[string], error) { var objectList client.ObjectList if securitypolicy.IsVPCEnabled(r.Service) { objectList = &crdv1alpha1.SecurityPolicyList{} } else { objectList = &v1alpha1.SecurityPolicyList{} } - err := r.Client.List(ctx, objectList) + err := r.Client.List(context.Background(), objectList) if err != nil { log.Error(err, "failed to list SecurityPolicy CR") - return + return nil, err } CRPolicySet := sets.New[string]() @@ -387,17 +429,7 @@ func (r *SecurityPolicyReconciler) CollectGarbage(ctx context.Context) { } } - diffSet := nsxPolicySet.Difference(CRPolicySet) - for elem := range diffSet { - 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), true, false, servicecommon.ResourceTypeSecurityPolicy) - if err != nil { - metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResType) - } else { - metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteSuccessTotal, MetricResType) - } - } + return CRPolicySet, nil } // It is triggered by associated controller like pod, namespace, etc. diff --git a/pkg/controllers/securitypolicy/securitypolicy_controller_test.go b/pkg/controllers/securitypolicy/securitypolicy_controller_test.go index 49f70a42c..c245d01bc 100644 --- a/pkg/controllers/securitypolicy/securitypolicy_controller_test.go +++ b/pkg/controllers/securitypolicy/securitypolicy_controller_test.go @@ -36,6 +36,7 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/nsx" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/securitypolicy" + "github.com/vmware-tanzu/nsx-operator/pkg/util" ) func fakeService() *securitypolicy.SecurityPolicyService { @@ -193,8 +194,13 @@ func TestSecurityPolicyReconciler_Reconcile(t *testing.T) { // not found errNotFound := errors.New("not found") k8sClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(errNotFound) - _, err := r.Reconcile(ctx, req) + deleteSecurityPolicyByNamePatch := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "deleteSecurityPolicyByName", func(_ *SecurityPolicyReconciler, name, ns string) error { + return nil + }) + defer deleteSecurityPolicyByNamePatch.Reset() + result, err := r.Reconcile(ctx, req) assert.Equal(t, err, errNotFound) + assert.Equal(t, ResultRequeue, result) // NSX version check failed case sp := &v1alpha1.SecurityPolicy{} @@ -202,10 +208,10 @@ func TestSecurityPolicyReconciler_Reconcile(t *testing.T) { return false }) k8sClient.EXPECT().Get(ctx, gomock.Any(), sp).Return(nil) - patches := gomonkey.ApplyFunc(updateFail, + updateFailPatch := gomonkey.ApplyFunc(updateFail, func(r *SecurityPolicyReconciler, c context.Context, o *v1alpha1.SecurityPolicy, e *error) { }) - defer patches.Reset() + defer updateFailPatch.Reset() result, ret := r.Reconcile(ctx, req) resultRequeueAfter5mins := controllerruntime.Result{Requeue: true, RequeueAfter: 5 * time.Minute} assert.Equal(t, nil, ret) @@ -217,29 +223,60 @@ func TestSecurityPolicyReconciler_Reconcile(t *testing.T) { }) defer checkNsxVersionPatch.Reset() - // DeletionTimestamp.IsZero = ture, client update failed + // DeletionTimestamp.IsZero = ture, create security policy in SystemNamespace k8sClient.EXPECT().Get(ctx, gomock.Any(), sp).Return(nil) - err = errors.New("Update failed") - k8sClient.EXPECT().Update(ctx, gomock.Any(), gomock.Any()).Return(err) - _, ret = r.Reconcile(ctx, req) + err = errors.New("fetch namespace associated with security policy CR failed") + IsSystemNamespacePatch := gomonkey.ApplyFunc(util.IsSystemNamespace, func(client client.Client, ns string, obj *v1.Namespace, + ) (bool, error) { + return true, errors.New("fetch namespace associated with security policy CR failed") + }) + + result, ret = r.Reconcile(ctx, req) + IsSystemNamespacePatch.Reset() assert.Equal(t, err, ret) + assert.Equal(t, ResultRequeue, result) - // DeletionTimestamp.IsZero = false, Finalizers doesn't include util.SecurityPolicyFinalizerName + // DeletionTimestamp.IsZero = false, Finalizers include util.SecurityPolicyFinalizerName and update fails k8sClient.EXPECT().Get(ctx, gomock.Any(), sp).Return(nil).Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { v1sp := obj.(*v1alpha1.SecurityPolicy) time := metav1.Now() v1sp.ObjectMeta.DeletionTimestamp = &time + v1sp.Finalizers = []string{common.T1SecurityPolicyFinalizerName} return nil }) + patch := gomonkey.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isGc bool, isVPCCleanup bool) error { assert.FailNow(t, "should not be called") return nil }) - _, ret = r.Reconcile(ctx, req) + + err = errors.New("finalizer remove failed, would retry exponentially") + k8sClient.EXPECT().Update(ctx, gomock.Any()).Return(err) + deleteFailPatch := gomonkey.ApplyFunc(deleteFail, + func(r *SecurityPolicyReconciler, c context.Context, o *v1alpha1.SecurityPolicy, e *error) { + }) + defer deleteFailPatch.Reset() + result, ret = r.Reconcile(ctx, req) + assert.Equal(t, ret, err) + assert.Equal(t, ResultRequeue, result) + patch.Reset() + + // DeletionTimestamp.IsZero = false, Finalizers doesn't include util.SecurityPolicyFinalizerName + k8sClient.EXPECT().Get(ctx, gomock.Any(), sp).Return(nil).Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { + v1sp := obj.(*v1alpha1.SecurityPolicy) + time := metav1.Now() + v1sp.ObjectMeta.DeletionTimestamp = &time + return nil + }) + patch = gomonkey.ApplyMethod(reflect.TypeOf(service), "DeleteSecurityPolicy", func(_ *securitypolicy.SecurityPolicyService, UID interface{}, isGc bool, isVPCCleanup bool) error { + return nil + }) + result, ret = r.Reconcile(ctx, req) assert.Equal(t, ret, nil) + assert.Equal(t, ResultNormal, result) patch.Reset() - // DeletionTimestamp.IsZero = false, Finalizers include util.SecurityPolicyFinalizerName + // DeletionTimestamp.IsZero = false, Finalizers include util.SecurityPolicyFinalizerName k8sClient.EXPECT().Get(ctx, gomock.Any(), sp).Return(nil).Do(func(_ context.Context, _ client.ObjectKey, obj client.Object, option ...client.GetOption) error { v1sp := obj.(*v1alpha1.SecurityPolicy) time := metav1.Now() @@ -251,8 +288,9 @@ func TestSecurityPolicyReconciler_Reconcile(t *testing.T) { return nil }) k8sClient.EXPECT().Update(ctx, gomock.Any(), gomock.Any()).Return(nil) - _, ret = r.Reconcile(ctx, req) + result, ret = r.Reconcile(ctx, req) assert.Equal(t, ret, nil) + assert.Equal(t, ResultNormal, result) patch.Reset() } diff --git a/pkg/nsx/services/common/types.go b/pkg/nsx/services/common/types.go index 31ec87ff3..f5d7ee575 100644 --- a/pkg/nsx/services/common/types.go +++ b/pkg/nsx/services/common/types.go @@ -15,83 +15,85 @@ import ( ) const ( - HashLength int = 8 - MaxTagScopeLength int = 128 - MaxTagValueLength int = 256 - MaxIdLength int = 255 - MaxNameLength int = 255 - MaxSubnetNameLength int = 80 - PriorityNetworkPolicyAllowRule int = 2010 - PriorityNetworkPolicyIsolationRule int = 2090 - TagScopeNCPCluster string = "ncp/cluster" - TagScopeNCPProjectUID string = "ncp/project_uid" - TagScopeNCPVIFProjectUID string = "ncp/vif_project_uid" - TagScopeNCPPod string = "ncp/pod" - TagScopeNCPVNETInterface string = "ncp/vnet_interface" - TagScopeCreatedFor string = "nsx-op/created_for" - TagScopeVersion string = "nsx-op/version" - TagScopeCluster string = "nsx-op/cluster" - TagScopeNamespace string = "nsx-op/namespace" - TagScopeNamespaceUID string = "nsx-op/namespace_uid" - TagScopeSecurityPolicyCRName string = "nsx-op/security_policy_cr_name" - TagScopeSecurityPolicyCRUID string = "nsx-op/security_policy_cr_uid" - TagScopeSecurityPolicyName string = "nsx-op/security_policy_name" - TagScopeSecurityPolicyUID string = "nsx-op/security_policy_uid" - TagScopeNetworkPolicyName string = "nsx-op/network_policy_name" - TagScopeNetworkPolicyUID string = "nsx-op/network_policy_uid" - TagScopeStaticRouteCRName string = "nsx-op/static_route_name" - TagScopeStaticRouteCRUID string = "nsx-op/static_route_uid" - TagScopeRuleID string = "nsx-op/rule_id" - TagScopeGoupID string = "nsx-op/group_id" - TagScopeGroupType string = "nsx-op/group_type" - TagScopeSelectorHash string = "nsx-op/selector_hash" - TagScopeNSXServiceAccountCRName string = "nsx-op/nsx_service_account_name" - TagScopeNSXServiceAccountCRUID string = "nsx-op/nsx_service_account_uid" - TagScopeNSXProjectID string = "nsx-op/nsx_project_id" - 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" - TagScopeIPPoolCRUID string = "nsx-op/ippool_uid" - TagScopeIPPoolCRType string = "nsx-op/ippool_type" - TagScopeIPAddressAllocationCRName string = "nsx-op/ipaddressallocation_name" - TagScopeIPAddressAllocationCRUID string = "nsx-op/ipaddressallocation_uid" - TagScopeIPSubnetName string = "nsx-op/ipsubnet_name" - TagScopeVMNamespaceUID string = "nsx-op/vm_namespace_uid" - TagScopeVMNamespace string = "nsx-op/vm_namespace" - TagScopeVPCManagedBy string = "nsx/managed-by" - AutoCreatedVPCTagValue string = "nsx-op" - LabelDefaultSubnetSet string = "nsxoperator.vmware.com/default-subnetset-for" - LabelDefaultVMSubnetSet string = "VirtualMachine" - LabelDefaultPodSubnetSet string = "Pod" - LabelLbIngressIpMode string = "nsx.vmware.com/ingress-ip-mode" - LabelLbIngressIpModeVipValue string = "vip" - LabelLbIngressIpModeProxyValue string = "proxy" - DefaultPodSubnetSet string = "pod-default" - DefaultVMSubnetSet string = "vm-default" - SystemVPCNetworkConfigurationName string = "system" - TagScopeSubnetCRUID string = "nsx-op/subnet_uid" - TagScopeSubnetCRName string = "nsx-op/subnet_name" - TagScopeSubnetSetCRName string = "nsx-op/subnetset_name" - TagScopeSubnetSetCRUID string = "nsx-op/subnetset_uid" - 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" - AnnotationSharedVPCNamespace string = "nsx.vmware.com/shared_vpc_namespace" - AnnotationDefaultNetworkConfig string = "nsx.vmware.com/default" - AnnotationAttachmentRef string = "nsx.vmware.com/attachment_ref" - TagScopePodName string = "nsx-op/pod_name" - TagScopePodUID string = "nsx-op/pod_uid" - ValueMajorVersion string = "1" - ValueMinorVersion string = "0" - ValuePatchVersion string = "0" - ConnectorUnderline string = "_" + HashLength int = 8 + MaxTagScopeLength int = 128 + MaxTagValueLength int = 256 + MaxIdLength int = 255 + MaxNameLength int = 255 + MaxSubnetNameLength int = 80 + PriorityNetworkPolicyAllowRule int = 2010 + PriorityNetworkPolicyIsolationRule int = 2090 + TagScopeNCPCluster string = "ncp/cluster" + TagScopeNCPProjectUID string = "ncp/project_uid" + TagScopeNCPVIFProjectUID string = "ncp/vif_project_uid" + TagScopeNCPPod string = "ncp/pod" + TagScopeNCPVNETInterface string = "ncp/vnet_interface" + TagScopeCreatedFor string = "nsx-op/created_for" + TagScopeVersion string = "nsx-op/version" + TagScopeCluster string = "nsx-op/cluster" + TagScopeNamespace string = "nsx-op/namespace" + TagScopeNamespaceUID string = "nsx-op/namespace_uid" + TagScopeSecurityPolicyCRName string = "nsx-op/security_policy_cr_name" + TagScopeSecurityPolicyCRUID string = "nsx-op/security_policy_cr_uid" + TagScopeSecurityPolicyName string = "nsx-op/security_policy_name" + TagScopeSecurityPolicyUID string = "nsx-op/security_policy_uid" + TagScopeSecurityPolicyNamespacedName string = "nsx-op/security_policy_namespaced_name" + TagScopeNetworkPolicyName string = "nsx-op/network_policy_name" + TagScopeNetworkPolicyUID string = "nsx-op/network_policy_uid" + TagScopeNetworkPolicyNamespacedName string = "nsx-op/network_policy_namespaced_name" + TagScopeStaticRouteCRName string = "nsx-op/static_route_name" + TagScopeStaticRouteCRUID string = "nsx-op/static_route_uid" + TagScopeRuleID string = "nsx-op/rule_id" + TagScopeGoupID string = "nsx-op/group_id" + TagScopeGroupType string = "nsx-op/group_type" + TagScopeSelectorHash string = "nsx-op/selector_hash" + TagScopeNSXServiceAccountCRName string = "nsx-op/nsx_service_account_name" + TagScopeNSXServiceAccountCRUID string = "nsx-op/nsx_service_account_uid" + TagScopeNSXProjectID string = "nsx-op/nsx_project_id" + 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" + TagScopeIPPoolCRUID string = "nsx-op/ippool_uid" + TagScopeIPPoolCRType string = "nsx-op/ippool_type" + TagScopeIPAddressAllocationCRName string = "nsx-op/ipaddressallocation_name" + TagScopeIPAddressAllocationCRUID string = "nsx-op/ipaddressallocation_uid" + TagScopeIPSubnetName string = "nsx-op/ipsubnet_name" + TagScopeVMNamespaceUID string = "nsx-op/vm_namespace_uid" + TagScopeVMNamespace string = "nsx-op/vm_namespace" + TagScopeVPCManagedBy string = "nsx/managed-by" + AutoCreatedVPCTagValue string = "nsx-op" + LabelDefaultSubnetSet string = "nsxoperator.vmware.com/default-subnetset-for" + LabelDefaultVMSubnetSet string = "VirtualMachine" + LabelDefaultPodSubnetSet string = "Pod" + LabelLbIngressIpMode string = "nsx.vmware.com/ingress-ip-mode" + LabelLbIngressIpModeVipValue string = "vip" + LabelLbIngressIpModeProxyValue string = "proxy" + DefaultPodSubnetSet string = "pod-default" + DefaultVMSubnetSet string = "vm-default" + SystemVPCNetworkConfigurationName string = "system" + TagScopeSubnetCRUID string = "nsx-op/subnet_uid" + TagScopeSubnetCRName string = "nsx-op/subnet_name" + TagScopeSubnetSetCRName string = "nsx-op/subnetset_name" + TagScopeSubnetSetCRUID string = "nsx-op/subnetset_uid" + 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" + AnnotationSharedVPCNamespace string = "nsx.vmware.com/shared_vpc_namespace" + AnnotationDefaultNetworkConfig string = "nsx.vmware.com/default" + AnnotationAttachmentRef string = "nsx.vmware.com/attachment_ref" + TagScopePodName string = "nsx-op/pod_name" + TagScopePodUID string = "nsx-op/pod_uid" + ValueMajorVersion string = "1" + ValueMinorVersion string = "0" + ValuePatchVersion string = "0" + ConnectorUnderline string = "_" GCInterval = 60 * time.Second RealizeTimeout = 2 * time.Minute @@ -101,11 +103,8 @@ const ( IPPoolTypePublic = "Public" IPPoolTypePrivate = "Private" - NSXServiceAccountFinalizerName = "nsxserviceaccount.nsx.vmware.com/finalizer" - T1SecurityPolicyFinalizerName = "securitypolicy.nsx.vmware.com/finalizer" - - SecurityPolicyFinalizerName = "securitypolicy.crd.nsx.vmware.com/finalizer" - NetworkPolicyFinalizerName = "networkpolicy.crd.nsx.vmware.com/finalizer" + NSXServiceAccountFinalizerName = "nsxserviceaccount.nsx.vmware.com/finalizer" + T1SecurityPolicyFinalizerName = "securitypolicy.nsx.vmware.com/finalizer" StaticRouteFinalizerName = "staticroute.crd.nsx.vmware.com/finalizer" SubnetFinalizerName = "subnet.crd.nsx.vmware.com/finalizer" SubnetSetFinalizerName = "subnetset.crd.nsx.vmware.com/finalizer" diff --git a/pkg/nsx/services/securitypolicy/builder.go b/pkg/nsx/services/securitypolicy/builder.go index b1a31e45e..b92a6f74c 100644 --- a/pkg/nsx/services/securitypolicy/builder.go +++ b/pkg/nsx/services/securitypolicy/builder.go @@ -252,9 +252,11 @@ func (service *SecurityPolicyService) buildTargetTags(obj *v1alpha1.SecurityPoli func (service *SecurityPolicyService) buildBasicTags(obj *v1alpha1.SecurityPolicy, createdFor string) []model.Tag { scopeOwnerName := common.TagValueScopeSecurityPolicyName scopeOwnerUID := common.TagValueScopeSecurityPolicyUID + scopeNamespacedName := common.TagScopeSecurityPolicyNamespacedName if createdFor == common.ResourceTypeNetworkPolicy { scopeOwnerName = common.TagScopeNetworkPolicyName scopeOwnerUID = common.TagScopeNetworkPolicyUID + scopeNamespacedName = common.TagScopeNetworkPolicyNamespacedName } tags := util.BuildBasicTags(getCluster(service), obj, service.getNamespaceUID(obj.ObjectMeta.Namespace)) @@ -267,7 +269,12 @@ func (service *SecurityPolicyService) buildBasicTags(obj *v1alpha1.SecurityPolic Scope: String(scopeOwnerUID), Tag: String(string(obj.UID)), }, + { + Scope: String(scopeNamespacedName), + Tag: String(util.CombineNamespaceName(obj.ObjectMeta.Name, obj.ObjectMeta.Namespace)), + }, }...) + return tags } diff --git a/pkg/nsx/services/securitypolicy/builder_test.go b/pkg/nsx/services/securitypolicy/builder_test.go index 5fbca2def..a5c1a4d7c 100644 --- a/pkg/nsx/services/securitypolicy/builder_test.go +++ b/pkg/nsx/services/securitypolicy/builder_test.go @@ -17,6 +17,7 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/apis/legacy/v1alpha1" "github.com/vmware-tanzu/nsx-operator/pkg/config" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" + "github.com/vmware-tanzu/nsx-operator/pkg/util" ) func TestBuildSecurityPolicy(t *testing.T) { @@ -420,6 +421,10 @@ func TestBuildTargetTags(t *testing.T) { Scope: &tagScopeRuleID, Tag: &ruleTagID0, }, + { + Scope: &tagScopeSecurityPolicyamespacedName, + Tag: String(util.CombineNamespaceName(spWithPodSelector.Name, tagValueNS)), + }, }, }, } @@ -485,6 +490,10 @@ func TestBuildPeerTags(t *testing.T) { Scope: &tagScopeSecurityPolicyCRUID, Tag: &tagValuePolicyCRUID, }, + { + Scope: &tagScopeSecurityPolicyamespacedName, + Tag: String(util.CombineNamespaceName(spWithPodSelector.Name, tagValueNS)), + }, }, }, } diff --git a/pkg/nsx/services/securitypolicy/firewall.go b/pkg/nsx/services/securitypolicy/firewall.go index 3d06ad578..bf28653f9 100644 --- a/pkg/nsx/services/securitypolicy/firewall.go +++ b/pkg/nsx/services/securitypolicy/firewall.go @@ -142,8 +142,10 @@ func (s *SecurityPolicyService) setUpStore(indexScope string) { s.securityPolicyStore = &SecurityPolicyStore{ResourceStore: common.ResourceStore{ Indexer: cache.NewIndexer( keyFunc, cache.Indexers{ - indexScope: indexBySecurityPolicyUID, - common.TagScopeNetworkPolicyUID: indexByNetworkPolicyUID, + indexScope: indexBySecurityPolicyUID, + common.TagScopeNetworkPolicyUID: indexByNetworkPolicyUID, + common.TagScopeNetworkPolicyNamespacedName: indexByNetworkPolicyNamespacedName, + common.TagScopeSecurityPolicyNamespacedName: indexBySecurityPolicyNamespacedName, }), BindingType: model.SecurityPolicyBindingType(), }} @@ -1104,6 +1106,14 @@ func (service *SecurityPolicyService) ListNetworkPolicyID() sets.Set[string] { return service.getGCSecurityPolicyIDSet(indexScope) } +func (service *SecurityPolicyService) ListNetworkPolicyByNamespacedName(name, ns string) []*model.SecurityPolicy { + return service.securityPolicyStore.GetByIndex(common.TagScopeNetworkPolicyNamespacedName, util.CombineNamespaceName(name, ns)) +} + +func (service *SecurityPolicyService) ListSecurityPolicyByNamespacedName(name, ns string) []*model.SecurityPolicy { + return service.securityPolicyStore.GetByIndex(common.TagScopeSecurityPolicyNamespacedName, util.CombineNamespaceName(name, ns)) +} + func (service *SecurityPolicyService) Cleanup(ctx context.Context) error { // Delete all the security policies in store uids := service.ListSecurityPolicyID() diff --git a/pkg/nsx/services/securitypolicy/firewall_test.go b/pkg/nsx/services/securitypolicy/firewall_test.go index 85969bc3f..4c1d1f97f 100644 --- a/pkg/nsx/services/securitypolicy/firewall_test.go +++ b/pkg/nsx/services/securitypolicy/firewall_test.go @@ -23,6 +23,7 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/apis/legacy/v1alpha1" "github.com/vmware-tanzu/nsx-operator/pkg/config" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" + "github.com/vmware-tanzu/nsx-operator/pkg/util" ) var ( @@ -31,47 +32,49 @@ var ( directionIn = v1alpha1.RuleDirectionIn directionOut = v1alpha1.RuleDirectionOut - tagScopeVersion = common.TagScopeVersion - tagScopeGroupType = common.TagScopeGroupType - tagScopeCluster = common.TagScopeCluster - tagScopeNamespace = common.TagScopeNamespace - tagScopeNamespaceUID = common.TagScopeNamespaceUID - tagScopeSecurityPolicyCRName = common.TagValueScopeSecurityPolicyName - tagScopeSecurityPolicyCRUID = common.TagValueScopeSecurityPolicyUID - tagScopeSecurityPolicyName = common.TagScopeSecurityPolicyName - tagScopeSecurityPolicyUID = common.TagScopeSecurityPolicyUID - tagScopeRuleID = common.TagScopeRuleID - tagScopeSelectorHash = common.TagScopeSelectorHash - spName = "sp_ns1_spA" - spGroupName = "ns1_spA_scope" - spID = "sp_uidA" - spID2 = "sp_uidB" - spGroupID = "sp_uidA_scope" - seq0 = int64(0) - seq1 = int64(1) - seq2 = int64(2) - ruleNameWithPodSelector00 = "rule-with-pod-selector" - ruleNameWithNsSelector00 = "rule-with-ns-selector" - cidr = "192.168.1.1/24" - ruleID0 = "sp_uidA_0" - ruleID1 = "sp_uidA_1" - ruleIDPort000 = "sp_uidA_0_0_0" - ruleIDPort100 = "sp_uidA_1_0_0" - nsxRuleDirectionIn = "IN" - nsxRuleActionAllow = "ALLOW" - nsxRuleDirectionOut = "OUT" - nsxRuleActionDrop = "DROP" - clusterName = "k8scl-one" - tagValueVersion = strings.Join(common.TagValueVersion, ".") - tagValueGroupScope = common.TagValueGroupScope - tagValueGroupSource = common.TagValueGroupSource - tagValueNS = "ns1" - tagValueNSUID = "us1UID" - tagValuePolicyCRName = "spA" - tagValuePolicyCRUID = "uidA" - tagValuePodSelectorHash = "a42321575d78a6c340c6963c7a82c86c7217f847" - tagValueRuleSrcHash = "52ec44a8f417d08f05720333292c24acfb108dab" - timeStamp = int64(1641892699021) + tagScopeVersion = common.TagScopeVersion + tagScopeGroupType = common.TagScopeGroupType + tagScopeCluster = common.TagScopeCluster + tagScopeNamespace = common.TagScopeNamespace + tagScopeNamespaceUID = common.TagScopeNamespaceUID + tagScopeSecurityPolicyCRName = common.TagValueScopeSecurityPolicyName + tagScopeSecurityPolicyCRUID = common.TagValueScopeSecurityPolicyUID + tagScopeSecurityPolicyName = common.TagScopeSecurityPolicyName + tagScopeSecurityPolicyUID = common.TagScopeSecurityPolicyUID + tagScopeSecurityPolicyamespacedName = common.TagScopeSecurityPolicyNamespacedName + tagScopeNetworkPolicyamespacedName = common.TagScopeNetworkPolicyNamespacedName + tagScopeRuleID = common.TagScopeRuleID + tagScopeSelectorHash = common.TagScopeSelectorHash + spName = "sp_ns1_spA" + spGroupName = "ns1_spA_scope" + spID = "sp_uidA" + spID2 = "sp_uidB" + spGroupID = "sp_uidA_scope" + seq0 = int64(0) + seq1 = int64(1) + seq2 = int64(2) + ruleNameWithPodSelector00 = "rule-with-pod-selector" + ruleNameWithNsSelector00 = "rule-with-ns-selector" + cidr = "192.168.1.1/24" + ruleID0 = "sp_uidA_0" + ruleID1 = "sp_uidA_1" + ruleIDPort000 = "sp_uidA_0_0_0" + ruleIDPort100 = "sp_uidA_1_0_0" + nsxRuleDirectionIn = "IN" + nsxRuleActionAllow = "ALLOW" + nsxRuleDirectionOut = "OUT" + nsxRuleActionDrop = "DROP" + clusterName = "k8scl-one" + tagValueVersion = strings.Join(common.TagValueVersion, ".") + tagValueGroupScope = common.TagValueGroupScope + tagValueGroupSource = common.TagValueGroupSource + tagValueNS = "ns1" + tagValueNSUID = "us1UID" + tagValuePolicyCRName = "spA" + tagValuePolicyCRUID = "uidA" + tagValuePodSelectorHash = "a42321575d78a6c340c6963c7a82c86c7217f847" + tagValueRuleSrcHash = "52ec44a8f417d08f05720333292c24acfb108dab" + timeStamp = int64(1641892699021) podSelectorMatchExpression = []metav1.LabelSelectorRequirement{ { @@ -279,6 +282,10 @@ var ( Scope: &tagScopeSecurityPolicyCRUID, Tag: String(string(spWithVMSelector.UID)), }, + { + Scope: &tagScopeSecurityPolicyamespacedName, + Tag: String(util.CombineNamespaceName(spWithVMSelector.Name, tagValueNS)), + }, } basicTags = []model.Tag{ @@ -306,6 +313,10 @@ var ( Scope: &tagScopeSecurityPolicyCRUID, Tag: &tagValuePolicyCRUID, }, + { + Scope: &tagScopeSecurityPolicyamespacedName, + Tag: String(util.CombineNamespaceName(tagValuePolicyCRName, tagValueNS)), + }, } vpcBasicTags = []model.Tag{ @@ -333,6 +344,10 @@ var ( Scope: &tagScopeSecurityPolicyUID, Tag: &tagValuePolicyCRUID, }, + { + Scope: &tagScopeSecurityPolicyamespacedName, + Tag: String(util.CombineNamespaceName(tagValuePolicyCRName, tagValueNS)), + }, } vpcBasicTagsForSpWithVMSelector = []model.Tag{ @@ -360,6 +375,10 @@ var ( Scope: &tagScopeSecurityPolicyUID, Tag: String(string(spWithVMSelector.UID)), }, + { + Scope: &tagScopeSecurityPolicyamespacedName, + Tag: String(util.CombineNamespaceName(spWithVMSelector.Name, tagValueNS)), + }, } // Create the NetworkPolicy object @@ -438,6 +457,10 @@ var ( Scope: common.String(common.TagScopeNetworkPolicyUID), Tag: String(string(npWithNsSelecotr.UID + "_allow")), }, + { + Scope: &tagScopeNetworkPolicyamespacedName, + Tag: String(util.CombineNamespaceName(npWithNsSelecotr.Name, tagValueNS)), + }, } npIsolationBasicTags = []model.Tag{ @@ -465,6 +488,10 @@ var ( Scope: common.String(common.TagScopeNetworkPolicyUID), Tag: String(string(npWithNsSelecotr.UID + "_isolation")), }, + { + Scope: &tagScopeNetworkPolicyamespacedName, + Tag: String(util.CombineNamespaceName(npWithNsSelecotr.Name, tagValueNS)), + }, } ) diff --git a/pkg/nsx/services/securitypolicy/store.go b/pkg/nsx/services/securitypolicy/store.go index b7c60026d..884aee9f0 100644 --- a/pkg/nsx/services/securitypolicy/store.go +++ b/pkg/nsx/services/securitypolicy/store.go @@ -76,6 +76,24 @@ func indexGroupFunc(obj interface{}) ([]string, error) { } } +func indexByNetworkPolicyNamespacedName(obj interface{}) ([]string, error) { + switch o := obj.(type) { + case *model.SecurityPolicy: + return filterTag(o.Tags, common.TagScopeNetworkPolicyNamespacedName), nil + default: + return nil, errors.New("indexByNetworkPolicyNamespacedName doesn't support unknown type") + } +} + +func indexBySecurityPolicyNamespacedName(obj interface{}) ([]string, error) { + switch o := obj.(type) { + case *model.SecurityPolicy: + return filterTag(o.Tags, common.TagScopeSecurityPolicyNamespacedName), nil + default: + return nil, errors.New("indexBySecurityPolicyNamespacedName doesn't support unknown type") + } +} + var filterRuleTag = func(v []model.Tag) []string { res := make([]string, 0, 5) for _, tag := range v { diff --git a/pkg/util/utils.go b/pkg/util/utils.go index e3e663c72..74c22eda7 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -528,3 +528,7 @@ func GetRandomIndexString() string { func IsPowerOfTwo(n int) bool { return n > 0 && (n&(n-1)) == 0 } + +func CombineNamespaceName(name, namespace string) string { + return fmt.Sprintf("%s/%s", namespace, name) +}