diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index 36ad2f2..2f835da 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -9,7 +9,7 @@ on: jobs: build: - runs-on: ubuntu-latest + runs-on: self-hosted steps: - name: Checkout uses: actions/checkout@v4 @@ -46,6 +46,5 @@ jobs: context: . file: ./Dockerfile push: true -# tags: ${{ secrets.DOCKERHUB_USERNAME }}/sgroups.k8s.netguard:${{ github.head_ref || github.ref_name }}-${{ steps.short-sha.outputs.sha }} tags: ${{ secrets.DOCKERHUB_USERNAME }}/sgroups.k8s.netguard:${{ steps.sanitize.outputs.branch }}-${{ steps.short-sha.outputs.sha }} platforms: linux/amd64,linux/arm64 diff --git a/api/v1alpha1/rules2s_types.go b/api/v1alpha1/rules2s_types.go index 23511e9..ab125a6 100644 --- a/api/v1alpha1/rules2s_types.go +++ b/api/v1alpha1/rules2s_types.go @@ -37,6 +37,9 @@ type RuleS2SSpec struct { // ServiceRef is a reference to the target service // +kubebuilder:validation:Required ServiceRef NamespacedObjectReference `json:"serviceRef"` + + // +optional + Description string `json:"description,omitempty"` } // RuleS2SStatus defines the observed state of RuleS2S. diff --git a/config/crd/bases/netguard.sgroups.io_rules2ses.yaml b/config/crd/bases/netguard.sgroups.io_rules2ses.yaml index 75714de..a4875c4 100644 --- a/config/crd/bases/netguard.sgroups.io_rules2ses.yaml +++ b/config/crd/bases/netguard.sgroups.io_rules2ses.yaml @@ -39,6 +39,8 @@ spec: spec: description: RuleS2SSpec defines the desired state of RuleS2S. properties: + description: + type: string serviceLocalRef: description: ServiceLocalRef is a reference to the local service properties: diff --git a/internal/controller/addressgroupbinding_controller.go b/internal/controller/addressgroupbinding_controller.go index e7ea97b..8750caf 100644 --- a/internal/controller/addressgroupbinding_controller.go +++ b/internal/controller/addressgroupbinding_controller.go @@ -24,7 +24,6 @@ import ( "time" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -87,23 +86,6 @@ func (r *AddressGroupBindingReconciler) Reconcile(ctx context.Context, req ctrl. "generation", binding.Generation, "resourceVersion", binding.ResourceVersion) - // TEMPORARY-DEBUG-CODE: Detailed logging for problematic resources - if binding.Name == "dynamic-2rx8z" || binding.Name == "dynamic-7dls7" || - binding.Name == "dynamic-fb5qw" || binding.Name == "dynamic-g6jfj" || - binding.Name == "dynamic-jd2b7" || binding.Name == "dynamic-lsjlt" { - - logger.Info("TEMPORARY-DEBUG-CODE: Detailed state of problematic binding", - "name", binding.Name, - "namespace", binding.Namespace, - "generation", binding.Generation, - "resourceVersion", binding.ResourceVersion, - "finalizers", binding.Finalizers, - "ownerReferences", formatOwnerReferences(binding.OwnerReferences), - "serviceRef", formatObjectReference(binding.Spec.ServiceRef), - "addressGroupRef", formatNamespacedObjectReference(binding.Spec.AddressGroupRef), - "conditions", formatConditions(binding.Status.Conditions)) - } - // Add finalizer if it doesn't exist const finalizer = "addressgroupbinding.netguard.sgroups.io/finalizer" if !controllerutil.ContainsFinalizer(binding, finalizer) { @@ -182,65 +164,24 @@ func (r *AddressGroupBindingReconciler) reconcileNormal(ctx context.Context, bin } if err := r.Get(ctx, addressGroupKey, addressGroup); err != nil { if apierrors.IsNotFound(err) { - // Check if we already have a condition for AddressGroupNotFound with the same generation - var existingCondition *metav1.Condition - for i := range binding.Status.Conditions { - if binding.Status.Conditions[i].Type == netguardv1alpha1.ConditionReady && - binding.Status.Conditions[i].Reason == "AddressGroupNotFound" && - binding.Status.Conditions[i].ObservedGeneration == binding.Generation { - existingCondition = &binding.Status.Conditions[i] - break - } - } - - // If condition already exists with the same generation, update with detailed message and don't requeue - if existingCondition != nil { - logger.Info("AddressGroup not found, not requeuing until resource changes", - "addressGroupName", addressGroupRef.GetName(), - "addressGroupNamespace", addressGroupNamespace) - - // Update the message with more detailed information - meta.SetStatusCondition(&binding.Status.Conditions, metav1.Condition{ - Type: netguardv1alpha1.ConditionReady, - Status: metav1.ConditionFalse, - Reason: "AddressGroupNotFound", - Message: fmt.Sprintf("AddressGroup %s not found in namespace %s. This binding will not be reconciled until the AddressGroup is created or the resource is modified.", - addressGroupRef.GetName(), addressGroupNamespace), - ObservedGeneration: binding.Generation, - LastTransitionTime: existingCondition.LastTransitionTime, - }) - - if err := UpdateStatusWithRetry(ctx, r.Client, binding, DefaultMaxRetries); err != nil { - logger.Error(err, "Failed to update AddressGroupBinding status") - return ctrl.Result{}, err - } - - // Don't requeue - return ctrl.Result{}, nil - } - - // First time seeing this issue or generation changed, set condition and requeue once - logger.Info("AddressGroup not found, will requeue once to update status", + logger.Info("AddressGroup not found, deleting AddressGroupBinding to maintain consistency", "addressGroupName", addressGroupRef.GetName(), - "addressGroupNamespace", addressGroupNamespace) - - meta.SetStatusCondition(&binding.Status.Conditions, metav1.Condition{ - Type: netguardv1alpha1.ConditionReady, - Status: metav1.ConditionFalse, - Reason: "AddressGroupNotFound", - Message: fmt.Sprintf("AddressGroup %s not found in namespace %s. This binding will be reconciled once more to update status.", - addressGroupRef.GetName(), addressGroupNamespace), - ObservedGeneration: binding.Generation, - LastTransitionTime: metav1.Now(), - }) + "addressGroupNamespace", addressGroupNamespace, + "binding", binding.GetName()) - if err := UpdateStatusWithRetry(ctx, r.Client, binding, DefaultMaxRetries); err != nil { - logger.Error(err, "Failed to update AddressGroupBinding status") + // Delete the binding since its referenced AddressGroup no longer exists + if err := r.Delete(ctx, binding); err != nil { + logger.Error(err, "Failed to delete AddressGroupBinding after AddressGroup deletion", + "addressGroup", addressGroupRef.GetName(), + "binding", binding.GetName()) return ctrl.Result{}, err } - // Requeue after a short time to update the status with the final message - return ctrl.Result{RequeueAfter: time.Second * 5}, nil + logger.Info("Successfully initiated deletion of AddressGroupBinding", + "addressGroup", addressGroupRef.GetName(), + "binding", binding.GetName()) + + return ctrl.Result{}, nil } logger.Error(err, "Failed to get AddressGroup") return ctrl.Result{}, err @@ -329,22 +270,40 @@ func (r *AddressGroupBindingReconciler) reconcileNormal(ctx context.Context, bin ownerRefsUpdated = true } - // Add OwnerReference to AddressGroup - agOwnerRef := metav1.OwnerReference{ - APIVersion: addressGroup.APIVersion, - Kind: addressGroup.Kind, - Name: addressGroup.Name, - UID: addressGroup.UID, - BlockOwnerDeletion: pointer.Bool(false), - Controller: pointer.Bool(false), - } - if !containsOwnerReference(binding.GetOwnerReferences(), agOwnerRef) { - logger.Info("Adding AddressGroup owner reference", + // Add OwnerReference to AddressGroup only if same namespace + if addressGroupNamespace == binding.GetNamespace() { + agOwnerRef := metav1.OwnerReference{ + APIVersion: addressGroup.APIVersion, + Kind: addressGroup.Kind, + Name: addressGroup.Name, + UID: addressGroup.UID, + BlockOwnerDeletion: pointer.Bool(false), + Controller: pointer.Bool(false), + } + if !containsOwnerReference(binding.GetOwnerReferences(), agOwnerRef) { + logger.Info("Adding AddressGroup owner reference (same namespace)", + "addressGroup", fmt.Sprintf("%s/%s", addressGroup.Kind, addressGroup.Name), + "addressGroupUID", addressGroup.UID) + + // Remove existing owner references for the same AddressGroup (by Kind+Name+APIVersion) + var updatedOwnerRefs []metav1.OwnerReference + for _, ref := range binding.GetOwnerReferences() { + if !(ref.Kind == agOwnerRef.Kind && + ref.Name == agOwnerRef.Name && + ref.APIVersion == agOwnerRef.APIVersion) { + updatedOwnerRefs = append(updatedOwnerRefs, ref) + } + } + // Add the new owner reference + updatedOwnerRefs = append(updatedOwnerRefs, agOwnerRef) + binding.OwnerReferences = updatedOwnerRefs + ownerRefsUpdated = true + } + } else { + logger.Info("Skipping AddressGroup owner reference (cross-namespace not supported)", "addressGroup", fmt.Sprintf("%s/%s", addressGroup.Kind, addressGroup.Name), - "addressGroupUID", addressGroup.UID) - - binding.OwnerReferences = append(binding.OwnerReferences, agOwnerRef) - ownerRefsUpdated = true + "addressGroupNamespace", addressGroupNamespace, + "bindingNamespace", binding.GetNamespace()) } // If owner references were updated, update the binding @@ -424,18 +383,14 @@ func (r *AddressGroupBindingReconciler) reconcileNormal(ctx context.Context, bin logger.Info("Ports have changed, updating ServicePortsRef", "service", fmt.Sprintf("%s/%s", sp.GetNamespace(), sp.GetName())) - // Create a copy for patching original := portMapping.DeepCopy() - - // Update the ports portMapping.AccessPorts.Items[i].Ports = servicePortsRef.Ports - - // Apply patch with retry patch := client.MergeFrom(original) if err := PatchWithRetry(ctx, r.Client, portMapping, patch, DefaultMaxRetries); err != nil { logger.Error(err, "Failed to update AddressGroupPortMapping.AccessPorts") return ctrl.Result{}, err } + logger.Info("Successfully updated Service ports in AddressGroupPortMapping", "service", service.GetName(), "addressGroup", addressGroupRef.GetName()) @@ -454,11 +409,7 @@ func (r *AddressGroupBindingReconciler) reconcileNormal(ctx context.Context, bin // Create a copy for patching original := portMapping.DeepCopy() - - // Add the service to the list portMapping.AccessPorts.Items = append(portMapping.AccessPorts.Items, servicePortsRef) - - // Apply patch with retry patch := client.MergeFrom(original) if err := PatchWithRetry(ctx, r.Client, portMapping, patch, DefaultMaxRetries); err != nil { logger.Error(err, "Failed to add Service to AddressGroupPortMapping.AccessPorts") @@ -490,22 +441,6 @@ func (r *AddressGroupBindingReconciler) reconcileNormal(ctx context.Context, bin "namespace", binding.Namespace, "generation", binding.Generation, "resourceVersion", binding.ResourceVersion) - - // TEMPORARY-DEBUG-CODE: Final state logging for problematic resources - if binding.Name == "dynamic-2rx8z" || binding.Name == "dynamic-7dls7" || - binding.Name == "dynamic-fb5qw" || binding.Name == "dynamic-g6jfj" || - binding.Name == "dynamic-jd2b7" || binding.Name == "dynamic-lsjlt" { - - logger.Info("TEMPORARY-DEBUG-CODE: Final state of problematic binding after successful reconciliation", - "name", binding.Name, - "namespace", binding.Namespace, - "generation", binding.Generation, - "resourceVersion", binding.ResourceVersion, - "finalizers", binding.Finalizers, - "ownerReferences", formatOwnerReferences(binding.OwnerReferences), - "conditions", formatConditions(binding.Status.Conditions)) - } - return ctrl.Result{}, nil } @@ -518,23 +453,6 @@ func (r *AddressGroupBindingReconciler) reconcileDelete(ctx context.Context, bin "finalizers", binding.Finalizers, "conditions", formatConditions(binding.Status.Conditions)) - // TEMPORARY-DEBUG-CODE: Detailed logging for problematic resources being deleted - if binding.Name == "dynamic-2rx8z" || binding.Name == "dynamic-7dls7" || - binding.Name == "dynamic-fb5qw" || binding.Name == "dynamic-g6jfj" || - binding.Name == "dynamic-jd2b7" || binding.Name == "dynamic-lsjlt" { - - logger.Info("TEMPORARY-DEBUG-CODE: Detailed state of problematic binding being deleted", - "name", binding.Name, - "namespace", binding.Namespace, - "generation", binding.Generation, - "resourceVersion", binding.ResourceVersion, - "finalizers", binding.Finalizers, - "ownerReferences", formatOwnerReferences(binding.OwnerReferences), - "serviceRef", formatObjectReference(binding.Spec.ServiceRef), - "addressGroupRef", formatNamespacedObjectReference(binding.Spec.AddressGroupRef), - "conditions", formatConditions(binding.Status.Conditions)) - } - // 1. Remove AddressGroup from Service.AddressGroups serviceRef := binding.Spec.ServiceRef service := &netguardv1alpha1.Service{} @@ -683,19 +601,6 @@ func (r *AddressGroupBindingReconciler) reconcileDelete(ctx context.Context, bin "name", binding.GetName(), "namespace", binding.GetNamespace()) - // TEMPORARY-DEBUG-CODE: Final state logging for problematic resources being deleted - if binding.Name == "dynamic-2rx8z" || binding.Name == "dynamic-7dls7" || - binding.Name == "dynamic-fb5qw" || binding.Name == "dynamic-g6jfj" || - binding.Name == "dynamic-jd2b7" || binding.Name == "dynamic-lsjlt" { - - logger.Info("TEMPORARY-DEBUG-CODE: Final state of problematic binding after successful deletion", - "name", binding.Name, - "namespace", binding.Namespace, - "generation", binding.Generation, - "resourceVersion", binding.ResourceVersion, - "finalizers", binding.Finalizers) - } - return ctrl.Result{}, nil } @@ -721,7 +626,6 @@ func setCondition(binding *netguardv1alpha1.AddressGroupBinding, conditionType s } } - // Condition not found, append it binding.Status.Conditions = append(binding.Status.Conditions, condition) } @@ -729,12 +633,27 @@ func setCondition(binding *netguardv1alpha1.AddressGroupBinding, conditionType s func containsOwnerReference(refs []metav1.OwnerReference, ref metav1.OwnerReference) bool { for _, r := range refs { if r.UID == ref.UID { + // Если BlockOwnerDeletion отличается, считаем что нужно обновить + if !equalBoolPtr(r.BlockOwnerDeletion, ref.BlockOwnerDeletion) { + return false + } return true } } return false } +// equalBoolPtr сравнивает два указателя на bool +func equalBoolPtr(a, b *bool) bool { + if a == nil && b == nil { + return true + } + if a == nil || b == nil { + return false + } + return *a == *b +} + // formatConditions formats a slice of conditions into a readable string func formatConditions(conditions []metav1.Condition) string { var result []string @@ -832,9 +751,15 @@ func (r *AddressGroupBindingReconciler) findBindingsForAddressGroup(ctx context. return nil } + logger := log.FromContext(ctx) + logger.Info("Finding bindings for AddressGroup", + "addressGroup", addressGroup.GetName(), + "namespace", addressGroup.GetNamespace()) + // Get all AddressGroupBinding bindingList := &netguardv1alpha1.AddressGroupBindingList{} if err := r.List(ctx, bindingList); err != nil { + logger.Error(err, "Failed to list AddressGroupBindings") return nil } @@ -842,9 +767,18 @@ func (r *AddressGroupBindingReconciler) findBindingsForAddressGroup(ctx context. // Filter bindings that reference this address group for _, binding := range bindingList.Items { + // Resolve the namespace for the AddressGroupRef + resolvedNamespace := v1alpha1.ResolveNamespace(binding.Spec.AddressGroupRef.GetNamespace(), binding.GetNamespace()) + if binding.Spec.AddressGroupRef.GetName() == addressGroup.GetName() && - (binding.Spec.AddressGroupRef.GetNamespace() == addressGroup.GetNamespace() || - binding.Spec.AddressGroupRef.GetNamespace() == "") { + resolvedNamespace == addressGroup.GetNamespace() { + + logger.Info("Found binding that references this AddressGroup", + "binding", binding.GetName(), + "bindingNamespace", binding.GetNamespace(), + "addressGroupRef", binding.Spec.AddressGroupRef.GetName(), + "resolvedNamespace", resolvedNamespace) + requests = append(requests, reconcile.Request{ NamespacedName: types.NamespacedName{ Name: binding.GetName(), @@ -854,6 +788,10 @@ func (r *AddressGroupBindingReconciler) findBindingsForAddressGroup(ctx context. } } + logger.Info("Found bindings for AddressGroup", + "addressGroup", addressGroup.GetName(), + "bindingsCount", len(requests)) + return requests } diff --git a/internal/controller/addressgroupbindingpolicy_controller.go b/internal/controller/addressgroupbindingpolicy_controller.go index 11ca430..c37e182 100644 --- a/internal/controller/addressgroupbindingpolicy_controller.go +++ b/internal/controller/addressgroupbindingpolicy_controller.go @@ -18,15 +18,16 @@ package controller import ( "context" - "fmt" - "time" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" netguardv1alpha1 "sgroups.io/netguard/api/v1alpha1" providerv1alpha1 "sgroups.io/netguard/deps/apis/sgroups-k8s-provider/v1alpha1" @@ -85,33 +86,44 @@ func (r *AddressGroupBindingPolicyReconciler) Reconcile(ctx context.Context, req } if err := r.Get(ctx, addressGroupKey, addressGroup); err != nil { - // Set condition to indicate that the AddressGroup was not found - setAddressGroupBindingPolicyCondition(policy, "AddressGroupFound", metav1.ConditionFalse, "AddressGroupNotFound", - fmt.Sprintf("AddressGroup %s not found in namespace %s", addressGroupRef.GetName(), addressGroupNamespace)) - if err := UpdateStatusWithRetry(ctx, r.Client, policy, DefaultMaxRetries); err != nil { - logger.Error(err, "Failed to update AddressGroupBindingPolicy status") + if apierrors.IsNotFound(err) { + logger.Info("AddressGroup not found, deleting policy", + "addressGroup", addressGroupKey.String()) + // AddressGroup deleted, delete the policy with retry + if err := DeleteWithRetry(ctx, r.Client, policy, DefaultMaxRetries); err != nil { + logger.Error(err, "Failed to delete policy after AddressGroup deletion") + return ctrl.Result{}, err + } + logger.Info("Policy deleted successfully after AddressGroup deletion") + return ctrl.Result{}, nil } - return ctrl.Result{RequeueAfter: time.Minute}, nil + logger.Error(err, "Failed to get AddressGroup") + return ctrl.Result{}, err } - // 2. Verify Service exists + // Check if the Service exists serviceRef := policy.Spec.ServiceRef serviceNamespace := v1alpha1.ResolveNamespace(serviceRef.GetNamespace(), policy.GetNamespace()) - - service := &netguardv1alpha1.Service{} serviceKey := client.ObjectKey{ Name: serviceRef.GetName(), Namespace: serviceNamespace, } + service := &netguardv1alpha1.Service{} if err := r.Get(ctx, serviceKey, service); err != nil { - // Set condition to indicate that the Service was not found - setAddressGroupBindingPolicyCondition(policy, "ServiceFound", metav1.ConditionFalse, "ServiceNotFound", - fmt.Sprintf("Service %s not found in namespace %s", serviceRef.GetName(), serviceNamespace)) - if err := UpdateStatusWithRetry(ctx, r.Client, policy, DefaultMaxRetries); err != nil { - logger.Error(err, "Failed to update AddressGroupBindingPolicy status") + if apierrors.IsNotFound(err) { + logger.Info("Service not found, deleting policy", + "service", serviceKey.String()) + // Service deleted, delete the policy with retry + if err := DeleteWithRetry(ctx, r.Client, policy, DefaultMaxRetries); err != nil { + logger.Error(err, "Failed to delete policy after Service deletion") + return ctrl.Result{}, err + } + logger.Info("Policy deleted successfully after Service deletion") + return ctrl.Result{}, nil } - return ctrl.Result{RequeueAfter: time.Minute}, nil + logger.Error(err, "Failed to get Service") + return ctrl.Result{}, err } // All resources exist, set Ready condition to true @@ -152,10 +164,137 @@ func setAddressGroupBindingPolicyCondition(policy *netguardv1alpha1.AddressGroup policy.Status.Conditions = append(policy.Status.Conditions, condition) } +// findPoliciesForService finds policies that reference a specific service +func (r *AddressGroupBindingPolicyReconciler) findPoliciesForService(ctx context.Context, obj client.Object) []reconcile.Request { + service, ok := obj.(*netguardv1alpha1.Service) + if !ok { + return nil + } + + logger := log.FromContext(ctx) + logger.Info("Finding policies for Service", + "service", service.GetName(), + "namespace", service.GetNamespace()) + + // Use index for faster lookup + policyList := &netguardv1alpha1.AddressGroupBindingPolicyList{} + if err := r.List(ctx, policyList, client.MatchingFields{"spec.serviceRef.name": service.GetName()}); err != nil { + logger.Error(err, "Failed to list AddressGroupBindingPolicies by service index") + return nil + } + + var requests []reconcile.Request + + // Filter policies that reference this service (additional namespace check) + for _, policy := range policyList.Items { + // Resolve the namespace for the ServiceRef + resolvedNamespace := v1alpha1.ResolveNamespace(policy.Spec.ServiceRef.GetNamespace(), policy.GetNamespace()) + + if resolvedNamespace == service.GetNamespace() { + logger.Info("Found policy that references this Service", + "policy", policy.GetName(), + "policyNamespace", policy.GetNamespace(), + "serviceRef", policy.Spec.ServiceRef.GetName(), + "resolvedNamespace", resolvedNamespace) + + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: policy.GetName(), + Namespace: policy.GetNamespace(), + }, + }) + } + } + + logger.Info("Found policies for Service", + "service", service.GetName(), + "policiesCount", len(requests)) + + return requests +} + +// findPoliciesForAddressGroup finds policies that reference a specific address group +func (r *AddressGroupBindingPolicyReconciler) findPoliciesForAddressGroup(ctx context.Context, obj client.Object) []reconcile.Request { + addressGroup, ok := obj.(*providerv1alpha1.AddressGroup) + if !ok { + return nil + } + + logger := log.FromContext(ctx) + logger.Info("Finding policies for AddressGroup", + "addressGroup", addressGroup.GetName(), + "namespace", addressGroup.GetNamespace()) + + // Use index for faster lookup + policyList := &netguardv1alpha1.AddressGroupBindingPolicyList{} + if err := r.List(ctx, policyList, client.MatchingFields{"spec.addressGroupRef.name": addressGroup.GetName()}); err != nil { + logger.Error(err, "Failed to list AddressGroupBindingPolicies by addressGroup index") + return nil + } + + var requests []reconcile.Request + + // Filter policies that reference this address group (additional namespace check) + for _, policy := range policyList.Items { + // Resolve the namespace for the AddressGroupRef + resolvedNamespace := v1alpha1.ResolveNamespace(policy.Spec.AddressGroupRef.GetNamespace(), policy.GetNamespace()) + + if resolvedNamespace == addressGroup.GetNamespace() { + logger.Info("Found policy that references this AddressGroup", + "policy", policy.GetName(), + "policyNamespace", policy.GetNamespace(), + "addressGroupRef", policy.Spec.AddressGroupRef.GetName(), + "resolvedNamespace", resolvedNamespace) + + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: policy.GetName(), + Namespace: policy.GetNamespace(), + }, + }) + } + } + + logger.Info("Found policies for AddressGroup", + "addressGroup", addressGroup.GetName(), + "policiesCount", len(requests)) + + return requests +} + // SetupWithManager sets up the controller with the Manager. func (r *AddressGroupBindingPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { + // Add indexes for faster lookups + if err := mgr.GetFieldIndexer().IndexField(context.Background(), + &netguardv1alpha1.AddressGroupBindingPolicy{}, "spec.serviceRef.name", + func(obj client.Object) []string { + policy := obj.(*netguardv1alpha1.AddressGroupBindingPolicy) + return []string{policy.Spec.ServiceRef.Name} + }); err != nil { + return err + } + + if err := mgr.GetFieldIndexer().IndexField(context.Background(), + &netguardv1alpha1.AddressGroupBindingPolicy{}, "spec.addressGroupRef.name", + func(obj client.Object) []string { + policy := obj.(*netguardv1alpha1.AddressGroupBindingPolicy) + return []string{policy.Spec.AddressGroupRef.Name} + }); err != nil { + return err + } + return ctrl.NewControllerManagedBy(mgr). For(&netguardv1alpha1.AddressGroupBindingPolicy{}). + // Watch for changes to Service + Watches( + &netguardv1alpha1.Service{}, + handler.EnqueueRequestsFromMapFunc(r.findPoliciesForService), + ). + // Watch for changes to AddressGroup + Watches( + &providerv1alpha1.AddressGroup{}, + handler.EnqueueRequestsFromMapFunc(r.findPoliciesForAddressGroup), + ). Named("addressgroupbindingpolicy"). Complete(r) } diff --git a/internal/controller/rules2s_controller.go b/internal/controller/rules2s_controller.go index 38b6670..b6bf960 100644 --- a/internal/controller/rules2s_controller.go +++ b/internal/controller/rules2s_controller.go @@ -20,15 +20,18 @@ import ( "context" "crypto/sha256" "fmt" + "sort" "strings" + "sync" "time" + "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -46,6 +49,52 @@ type RuleS2SReconciler struct { Scheme *runtime.Scheme } +// EnablePortAggregation controls whether port aggregation is enabled for multiple RuleS2S +const EnablePortAggregation = true + +// aggregationMutexes contains mutexes for synchronizing aggregation operations +var aggregationMutexes = sync.Map{} + +// getAggregationMutex returns a mutex for a specific aggregation key +func getAggregationMutex(key RuleAggregationKey) *sync.Mutex { + mutexKey := fmt.Sprintf("%s-%s-%s-%s", + key.Traffic, key.LocalAGName, key.TargetAGName, key.Protocol) + + mutex, _ := aggregationMutexes.LoadOrStore(mutexKey, &sync.Mutex{}) + return mutex.(*sync.Mutex) +} + +// logAggregationOperation logs aggregation operations for debugging +func (r *RuleS2SReconciler) logAggregationOperation(logger logr.Logger, operation string, key RuleAggregationKey, details map[string]interface{}) { + baseFields := []interface{}{ + "operation", operation, + "traffic", key.Traffic, + "localAG", key.LocalAGName, + "targetAG", key.TargetAGName, + "protocol", key.Protocol, + } + + for k, v := range details { + baseFields = append(baseFields, k, v) + } + + logger.Info("AGGREGATION_OPERATION", baseFields...) +} + +// ContributingRule represents a RuleS2S that contributes to an IEAgAgRule +type ContributingRule struct { + RuleS2S *netguardv1alpha1.RuleS2S + Ports []string +} + +// RuleAggregationKey uniquely identifies an aggregated rule +type RuleAggregationKey struct { + Traffic string + LocalAGName string + TargetAGName string + Protocol string +} + // +kubebuilder:rbac:groups=netguard.sgroups.io,resources=rules2s,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=netguard.sgroups.io,resources=rules2s/status,verbs=get;update;patch // +kubebuilder:rbac:groups=netguard.sgroups.io,resources=rules2s/finalizers,verbs=update @@ -148,10 +197,7 @@ func (r *RuleS2SReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct logger.Error(err, "Failed to update RuleS2S status") } - // Логируем информацию logger.Info(errorMsg, "name", ruleS2S.Spec.ServiceLocalRef.Name) - - // Возвращаем пустой Result без RequeueAfter return ctrl.Result{}, nil } @@ -164,7 +210,6 @@ func (r *RuleS2SReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct errorMsg := fmt.Sprintf("Target service alias '%s' not found in namespace '%s': %v", ruleS2S.Spec.ServiceRef.Name, targetNamespace, err) - // Update status with error condition meta.SetStatusCondition(&ruleS2S.Status.Conditions, metav1.Condition{ Type: netguardv1alpha1.ConditionReady, Status: metav1.ConditionFalse, @@ -175,10 +220,7 @@ func (r *RuleS2SReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct logger.Error(err, "Failed to update RuleS2S status") } - // Логируем информацию logger.Info(errorMsg, "name", ruleS2S.Spec.ServiceRef.Name, "namespace", targetNamespace) - - // Возвращаем пустой Result без RequeueAfter return ctrl.Result{}, nil } @@ -203,10 +245,7 @@ func (r *RuleS2SReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct logger.Error(err, "Failed to update RuleS2S status") } - // Логируем информацию logger.Info(errorMsg, "name", localServiceAlias.Spec.ServiceRef.Name, "namespace", localServiceNamespace) - - // Возвращаем пустой Result без RequeueAfter return ctrl.Result{}, nil } @@ -230,16 +269,12 @@ func (r *RuleS2SReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct logger.Error(err, "Failed to update RuleS2S status") } - // Логируем информацию logger.Info(errorMsg, "name", targetServiceAlias.Spec.ServiceRef.Name, "namespace", targetServiceNamespace) - - // Возвращаем пустой Result без RequeueAfter return ctrl.Result{}, nil } // Update RuleS2SDstOwnRef for cross-namespace references if ruleS2S.Namespace != targetServiceNamespace { - // Add this rule to the target service's RuleS2SDstOwnRef found := false for _, ref := range targetService.RuleS2SDstOwnRef.Items { if ref.Name == ruleS2S.Name && ref.Namespace == ruleS2S.Namespace { @@ -263,7 +298,6 @@ func (r *RuleS2SReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct errorMsg := fmt.Sprintf("Failed to update target service '%s' RuleS2SDstOwnRef: %v", targetService.Name, err) logger.Error(err, errorMsg) - // Проверяем, нужна ли периодическая проверка для этого правила if val, ok := ruleS2S.Annotations["netguard.sgroups.io/periodic-reconcile"]; ok && val == "true" { return ctrl.Result{RequeueAfter: time.Minute}, err } @@ -272,7 +306,6 @@ func (r *RuleS2SReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } } } else { - // For rules in the same namespace, use owner references if err := controllerutil.SetControllerReference(targetService, ruleS2S, r.Scheme); err != nil { errorMsg := fmt.Sprintf("Failed to set owner reference from target service '%s' to RuleS2S '%s': %v", targetService.Name, ruleS2S.Name, err) @@ -357,19 +390,15 @@ func (r *RuleS2SReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct logger.Error(err, "Failed to update RuleS2S status") } - // Логируем информацию logger.Info("Rule is valid but inactive, deleting related IEAgAgRules", "conditions", strings.Join(inactiveConditions, "; "), "localService", localService.Name, "targetService", targetService.Name) - // Удаляем связанные IEAgAgRules if err := r.deleteRelatedIEAgAgRules(ctx, ruleS2S); err != nil { - // Если не удалось удалить некоторые правила, логируем ошибку, но продолжаем logger.Error(err, "Failed to delete some related IEAgAgRules") } - // Возвращаем пустой Result без RequeueAfter return ctrl.Result{}, nil } @@ -557,7 +586,6 @@ func (r *RuleS2SReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct logger.Error(err, "Failed to update RuleS2S status") } - // Логируем информацию logger.Info(errorMsg, "localService", localService.Name, "targetService", targetService.Name, @@ -565,13 +593,220 @@ func (r *RuleS2SReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct "targetAddressGroups", len(targetAddressGroups), "ports", len(ports)) - // Возвращаем пустой Result без RequeueAfter return ctrl.Result{}, nil } return ctrl.Result{}, nil } +// findContributingRuleS2S finds all RuleS2S that should contribute to one IEAgAgRule +func (r *RuleS2SReconciler) findContributingRuleS2S( + ctx context.Context, + currentRule *netguardv1alpha1.RuleS2S, + localService *netguardv1alpha1.Service, + targetService *netguardv1alpha1.Service, +) ([]ContributingRule, error) { + logger := log.FromContext(ctx) + + logger.Info("Finding contributing RuleS2S", + "currentRule", currentRule.Name, + "localService", localService.Name, + "targetService", targetService.Name) + + allRules := &netguardv1alpha1.RuleS2SList{} + if err := r.List(ctx, allRules); err != nil { + logger.Error(err, "Failed to list RuleS2S") + return nil, err + } + + var contributingRules []ContributingRule + + for _, rule := range allRules.Items { + if !rule.DeletionTimestamp.IsZero() { + logger.V(2).Info("Skipping rule being deleted", "rule", rule.Name) + continue + } + + contributes, ports, err := r.checkIfRuleContributes(ctx, &rule, currentRule, localService, targetService) + if err != nil { + logger.Error(err, "Error checking rule contribution", "rule", rule.Name) + continue + } + + if contributes && len(ports) > 0 { + logger.Info("Found contributing rule", + "rule", rule.Name, + "namespace", rule.Namespace, + "ports", strings.Join(ports, ",")) + + contributingRules = append(contributingRules, ContributingRule{ + RuleS2S: &rule, + Ports: ports, + }) + } + } + + logger.Info("Found contributing rules", + "count", len(contributingRules), + "currentRule", currentRule.Name) + + return contributingRules, nil +} + +// checkIfRuleContributes checks if a rule should contribute to aggregation +func (r *RuleS2SReconciler) checkIfRuleContributes( + ctx context.Context, + candidateRule *netguardv1alpha1.RuleS2S, + currentRule *netguardv1alpha1.RuleS2S, + localService *netguardv1alpha1.Service, + targetService *netguardv1alpha1.Service, +) (bool, []string, error) { + logger := log.FromContext(ctx) + + if candidateRule.Spec.Traffic != currentRule.Spec.Traffic { + return false, nil, nil + } + + candidateLocalService, candidateTargetService, err := r.getServicesForRule(ctx, candidateRule) + if err != nil { + return false, nil, err + } + + if !r.servicesHaveSameAddressGroups(localService, candidateLocalService) || + !r.servicesHaveSameAddressGroups(targetService, candidateTargetService) { + return false, nil, nil + } + + var ports []string + if strings.ToLower(candidateRule.Spec.Traffic) == "ingress" { + ports = r.extractPortsFromService(candidateLocalService) + } else { + ports = r.extractPortsFromService(candidateTargetService) + } + + logger.V(2).Info("Rule contribution check", + "candidateRule", candidateRule.Name, + "contributes", true, + "ports", strings.Join(ports, ",")) + + return true, ports, nil +} + +// getServicesForRule gets services for RuleS2S (extracting existing logic) +func (r *RuleS2SReconciler) getServicesForRule( + ctx context.Context, + rule *netguardv1alpha1.RuleS2S, +) (*netguardv1alpha1.Service, *netguardv1alpha1.Service, error) { + logger := log.FromContext(ctx) + + localServiceAlias := &netguardv1alpha1.ServiceAlias{} + if err := r.Get(ctx, types.NamespacedName{ + Namespace: rule.Namespace, + Name: rule.Spec.ServiceLocalRef.Name, + }, localServiceAlias); err != nil { + logger.Error(err, "Local service alias not found", + "name", rule.Spec.ServiceLocalRef.Name, + "namespace", rule.Namespace) + return nil, nil, err + } + + targetServiceAlias := &netguardv1alpha1.ServiceAlias{} + targetNamespace := rule.Spec.ServiceRef.ResolveNamespace(rule.Namespace) + if err := r.Get(ctx, types.NamespacedName{ + Namespace: targetNamespace, + Name: rule.Spec.ServiceRef.Name, + }, targetServiceAlias); err != nil { + logger.Error(err, "Target service alias not found", + "name", rule.Spec.ServiceRef.Name, + "namespace", targetNamespace) + return nil, nil, err + } + + localService := &netguardv1alpha1.Service{} + localServiceNamespace := localServiceAlias.Spec.ServiceRef.ResolveNamespace(localServiceAlias.Namespace) + if err := r.Get(ctx, types.NamespacedName{ + Namespace: localServiceNamespace, + Name: localServiceAlias.Spec.ServiceRef.Name, + }, localService); err != nil { + logger.Error(err, "Local service not found", + "name", localServiceAlias.Spec.ServiceRef.Name, + "namespace", localServiceNamespace) + return nil, nil, err + } + + targetService := &netguardv1alpha1.Service{} + targetServiceNamespace := targetServiceAlias.Spec.ServiceRef.ResolveNamespace(targetServiceAlias.Namespace) + if err := r.Get(ctx, types.NamespacedName{ + Namespace: targetServiceNamespace, + Name: targetServiceAlias.Spec.ServiceRef.Name, + }, targetService); err != nil { + logger.Error(err, "Target service not found", + "name", targetServiceAlias.Spec.ServiceRef.Name, + "namespace", targetServiceNamespace) + return nil, nil, err + } + + return localService, targetService, nil +} + +// servicesHaveSameAddressGroups checks if services have the same address groups +func (r *RuleS2SReconciler) servicesHaveSameAddressGroups( + service1 *netguardv1alpha1.Service, + service2 *netguardv1alpha1.Service, +) bool { + if len(service1.AddressGroups.Items) != len(service2.AddressGroups.Items) { + return false + } + + agMap := make(map[string]bool) + for _, ag := range service1.AddressGroups.Items { + key := fmt.Sprintf("%s/%s", ag.GetNamespace(), ag.Name) + agMap[key] = true + } + + for _, ag := range service2.AddressGroups.Items { + key := fmt.Sprintf("%s/%s", ag.GetNamespace(), ag.Name) + if !agMap[key] { + return false + } + } + + return true +} + +// extractPortsFromService extracts ports from the service +func (r *RuleS2SReconciler) extractPortsFromService( + service *netguardv1alpha1.Service, +) []string { + var ports []string + for _, port := range service.Spec.IngressPorts { + ports = append(ports, port.Port) + } + return ports +} + +// aggregatePortsWithProtocol aggregates ports for a specific protocol +func (r *RuleS2SReconciler) aggregatePortsWithProtocol( + contributingRules []ContributingRule, + protocol netguardv1alpha1.TransportProtocol, +) []string { + portSet := make(map[string]bool) + + for _, rule := range contributingRules { + for _, port := range rule.Ports { + portSet[port] = true + } + } + + var aggregatedPorts []string + for port := range portSet { + aggregatedPorts = append(aggregatedPorts, port) + } + + sort.Strings(aggregatedPorts) + return aggregatedPorts +} + // createOrUpdateIEAgAgRule creates or updates an IEAgAgRule func (r *RuleS2SReconciler) createOrUpdateIEAgAgRule( ctx context.Context, @@ -583,8 +818,182 @@ func (r *RuleS2SReconciler) createOrUpdateIEAgAgRule( ) (string, error) { logger := log.FromContext(ctx) - // Логирование входных параметров - logger.Info("Starting createOrUpdateIEAgAgRule", + if !EnablePortAggregation { + return r.createOrUpdateIEAgAgRuleLegacy(ctx, ruleS2S, localAG, targetAG, protocol, portsStr) + } + + aggregationKey := RuleAggregationKey{ + Traffic: ruleS2S.Spec.Traffic, + LocalAGName: localAG.Name, + TargetAGName: targetAG.Name, + Protocol: string(protocol), + } + + mutex := getAggregationMutex(aggregationKey) + mutex.Lock() + defer mutex.Unlock() + + logger.Info("Starting createOrUpdateIEAgAgRule with aggregation (SYNCHRONIZED)", + "ruleS2S", ruleS2S.Name, + "ruleS2S.UID", ruleS2S.GetUID(), + "localAG", localAG.Name, + "localAG.Namespace", localAG.GetNamespace(), + "targetAG", targetAG.Name, + "targetAG.Namespace", targetAG.GetNamespace(), + "protocol", protocol, + "ports", portsStr, + "aggregationKey", fmt.Sprintf("%s-%s-%s-%s", + aggregationKey.Traffic, aggregationKey.LocalAGName, + aggregationKey.TargetAGName, aggregationKey.Protocol)) + + r.logAggregationOperation(logger, "START_AGGREGATION", aggregationKey, map[string]interface{}{ + "ruleS2S": ruleS2S.Name, + "ruleUID": ruleS2S.GetUID(), + "ports": portsStr, + }) + + var ruleNamespace string + if ruleS2S.Spec.Traffic == "ingress" { + ruleNamespace = localAG.ResolveNamespace(ruleS2S.GetNamespace()) + } else { + ruleNamespace = targetAG.ResolveNamespace(ruleS2S.GetNamespace()) + } + + if ruleNamespace == "" { + logger.Error(fmt.Errorf("empty namespace"), "Cannot create rule with empty namespace") + return "", fmt.Errorf("cannot create rule with empty namespace") + } + + ruleName := r.generateRuleName( + ruleS2S.Spec.Traffic, + localAG.Name, + targetAG.Name, + string(protocol)) + + localService, targetService, err := r.getServicesForRule(ctx, ruleS2S) + if err != nil { + logger.Error(err, "Failed to get services for rule, falling back to legacy") + return r.createOrUpdateIEAgAgRuleLegacy(ctx, ruleS2S, localAG, targetAG, protocol, portsStr) + } + + contributingRules, err := r.findContributingRuleS2S(ctx, ruleS2S, localService, targetService) + if err != nil { + logger.Error(err, "Failed to find contributing rules, falling back to legacy") + return r.createOrUpdateIEAgAgRuleLegacy(ctx, ruleS2S, localAG, targetAG, protocol, portsStr) + } + + aggregatedPorts := r.aggregatePortsWithProtocol(contributingRules, protocol) + logger.Info("Aggregated ports for rule", + "ruleName", ruleName, + "protocol", protocol, + "aggregatedPorts", strings.Join(aggregatedPorts, ","), + "contributingRulesCount", len(contributingRules)) + + // Check if aggregation resulted in empty ports - delete existing rule if so + if len(aggregatedPorts) == 0 { + logger.Info("No ports after aggregation, checking if rule exists to delete", + "ruleName", ruleName, + "protocol", protocol) + + // If rule exists - delete it + existingRule := &providerv1alpha1.IEAgAgRule{} + err = r.Get(ctx, types.NamespacedName{ + Namespace: ruleNamespace, + Name: ruleName, + }, existingRule) + + if err == nil { + // Rule exists, delete it + logger.Info("Deleting IEAgAgRule with no ports after aggregation", + "namespace", ruleNamespace, + "name", ruleName) + + if err := r.Delete(ctx, existingRule); err != nil && !errors.IsNotFound(err) { + logger.Error(err, "Failed to delete rule with no ports") + r.logAggregationOperation(logger, "END_AGGREGATION", aggregationKey, map[string]interface{}{ + "result": "DELETE_ERROR", + "error": err, + }) + return "", err + } + + r.logAggregationOperation(logger, "END_AGGREGATION", aggregationKey, map[string]interface{}{ + "result": "DELETED_NO_PORTS", + "ruleName": ruleName, + }) + + return "", nil + } else if !errors.IsNotFound(err) { + // Error getting the rule + logger.Error(err, "Error checking if rule exists for deletion") + r.logAggregationOperation(logger, "END_AGGREGATION", aggregationKey, map[string]interface{}{ + "result": "GET_ERROR", + "error": err, + }) + return "", err + } + + // Rule doesn't exist, nothing to delete + logger.Info("No rule exists and no ports to create, skipping", + "ruleName", ruleName) + + r.logAggregationOperation(logger, "END_AGGREGATION", aggregationKey, map[string]interface{}{ + "result": "SKIP_NO_PORTS", + "ruleName": ruleName, + }) + + return "", nil + } + + existingRule := &providerv1alpha1.IEAgAgRule{} + err = r.Get(ctx, types.NamespacedName{ + Namespace: ruleNamespace, + Name: ruleName, + }, existingRule) + + if err != nil && errors.IsNotFound(err) { + result, err := r.createNewIEAgAgRuleWithAggregation(ctx, ruleNamespace, ruleName, + ruleS2S, localAG, targetAG, protocol, aggregatedPorts, contributingRules) + + r.logAggregationOperation(logger, "END_AGGREGATION", aggregationKey, map[string]interface{}{ + "result": "CREATE", + "ruleName": result, + "error": err, + }) + + return result, err + } else if err != nil { + r.logAggregationOperation(logger, "END_AGGREGATION", aggregationKey, map[string]interface{}{ + "result": "ERROR", + "error": err, + }) + return "", err + } + + result, err := r.updateExistingIEAgAgRuleWithAggregation(ctx, existingRule, + aggregatedPorts, contributingRules) + + r.logAggregationOperation(logger, "END_AGGREGATION", aggregationKey, map[string]interface{}{ + "result": "UPDATE", + "ruleName": result, + "error": err, + }) + + return result, err +} + +// createOrUpdateIEAgAgRuleLegacy creates or updates an IEAgAgRule (legacy logic) +func (r *RuleS2SReconciler) createOrUpdateIEAgAgRuleLegacy( + ctx context.Context, + ruleS2S *netguardv1alpha1.RuleS2S, + localAG netguardv1alpha1.NamespacedObjectReference, + targetAG netguardv1alpha1.NamespacedObjectReference, + protocol netguardv1alpha1.TransportProtocol, + portsStr string, +) (string, error) { + logger := log.FromContext(ctx) + + logger.Info("Starting createOrUpdateIEAgAgRuleLegacy", "ruleS2S", ruleS2S.Name, "ruleS2S.UID", ruleS2S.GetUID(), "localAG", localAG.Name, @@ -594,17 +1003,14 @@ func (r *RuleS2SReconciler) createOrUpdateIEAgAgRule( "protocol", protocol, "ports", portsStr) - // Determine namespace for the rule based on traffic direction var ruleNamespace string if ruleS2S.Spec.Traffic == "ingress" { - // For ingress, rule goes in the local AG namespace (receiver) ruleNamespace = localAG.ResolveNamespace(ruleS2S.GetNamespace()) logger.Info("Using ingress namespace logic", "ruleNamespace", ruleNamespace, "localAG.Namespace", localAG.GetNamespace(), "ruleS2S.Namespace", ruleS2S.GetNamespace()) } else { - // For egress, rule goes in the target AG namespace (receiver) ruleNamespace = targetAG.ResolveNamespace(ruleS2S.GetNamespace()) logger.Info("Using egress namespace logic", "ruleNamespace", ruleNamespace, @@ -612,13 +1018,11 @@ func (r *RuleS2SReconciler) createOrUpdateIEAgAgRule( "ruleS2S.Namespace", ruleS2S.GetNamespace()) } - // Ensure namespace is not empty if ruleNamespace == "" { logger.Error(fmt.Errorf("empty namespace"), "Cannot create rule with empty namespace") return "", fmt.Errorf("cannot create rule with empty namespace") } - // Generate rule name using the helper function ruleName := r.generateRuleName( ruleS2S.Spec.Traffic, localAG.Name, @@ -633,7 +1037,6 @@ func (r *RuleS2SReconciler) createOrUpdateIEAgAgRule( targetAG.Name, strings.ToLower(string(protocol)))) - // Define the rule spec ruleSpec := providerv1alpha1.IEAgAgRuleSpec{ Transport: providerv1alpha1.TransportProtocol(string(protocol)), Traffic: providerv1alpha1.TrafficDirection(strings.ToUpper(ruleS2S.Spec.Traffic)), @@ -665,7 +1068,6 @@ func (r *RuleS2SReconciler) createOrUpdateIEAgAgRule( }, } - // Check if the rule already exists existingRule := &providerv1alpha1.IEAgAgRule{} err := r.Get(ctx, types.NamespacedName{ Namespace: ruleNamespace, @@ -678,10 +1080,8 @@ func (r *RuleS2SReconciler) createOrUpdateIEAgAgRule( "name", ruleName, "error", err.Error()) - // Rule doesn't exist, create it with retry logger.Info("Creating new IEAgAgRule", "namespace", ruleNamespace, "name", ruleName) - // Create the rule newRule := &providerv1alpha1.IEAgAgRule{ ObjectMeta: metav1.ObjectMeta{ Name: ruleName, @@ -692,8 +1092,8 @@ func (r *RuleS2SReconciler) createOrUpdateIEAgAgRule( Kind: "RuleS2S", Name: ruleS2S.GetName(), UID: ruleS2S.GetUID(), - Controller: pointer.Bool(false), - BlockOwnerDeletion: pointer.Bool(true), + Controller: ptr.To(false), + BlockOwnerDeletion: ptr.To(true), }, }, }, @@ -779,7 +1179,6 @@ func (r *RuleS2SReconciler) createOrUpdateIEAgAgRule( // Error getting the rule return "", err } else { - // Правило существует logger.Info("Rule exists, will update", "namespace", ruleNamespace, "name", ruleName, @@ -814,7 +1213,6 @@ func (r *RuleS2SReconciler) createOrUpdateIEAgAgRule( // Update the spec latestRule.Spec = ruleSpec - // Логирование перед патчем logger.Info("Applying patch to rule", "namespace", ruleNamespace, "name", ruleName, @@ -838,6 +1236,223 @@ func (r *RuleS2SReconciler) createOrUpdateIEAgAgRule( return ruleName, nil } +// createNewIEAgAgRuleWithAggregation создает новое правило с учетом всех contributing rules +func (r *RuleS2SReconciler) createNewIEAgAgRuleWithAggregation( + ctx context.Context, + ruleNamespace string, + ruleName string, + currentRuleS2S *netguardv1alpha1.RuleS2S, + localAG netguardv1alpha1.NamespacedObjectReference, + targetAG netguardv1alpha1.NamespacedObjectReference, + protocol netguardv1alpha1.TransportProtocol, + aggregatedPorts []string, + contributingRules []ContributingRule, +) (string, error) { + logger := log.FromContext(ctx) + + logger.Info("Creating new IEAgAgRule with aggregation", + "namespace", ruleNamespace, + "name", ruleName, + "aggregatedPorts", strings.Join(aggregatedPorts, ","), + "contributingRules", len(contributingRules)) + + // Don't create rule with empty ports + if len(aggregatedPorts) == 0 { + logger.Info("Not creating rule with empty aggregated ports", + "namespace", ruleNamespace, + "name", ruleName) + return "", nil + } + + ownerRefs := make([]metav1.OwnerReference, 0, len(contributingRules)) + processedUIDs := make(map[types.UID]bool) + + for _, contrib := range contributingRules { + if !processedUIDs[contrib.RuleS2S.UID] { + ownerRefs = append(ownerRefs, metav1.OwnerReference{ + APIVersion: netguardv1alpha1.GroupVersion.String(), + Kind: "RuleS2S", + Name: contrib.RuleS2S.Name, + UID: contrib.RuleS2S.UID, + Controller: ptr.To(false), + BlockOwnerDeletion: ptr.To(true), + }) + processedUIDs[contrib.RuleS2S.UID] = true + + logger.Info("Adding owner reference", + "ruleS2S", contrib.RuleS2S.Name, + "uid", contrib.RuleS2S.UID) + } + } + + ruleSpec := providerv1alpha1.IEAgAgRuleSpec{ + Transport: providerv1alpha1.TransportProtocol(string(protocol)), + Traffic: providerv1alpha1.TrafficDirection(strings.ToUpper(currentRuleS2S.Spec.Traffic)), + AddressGroupLocal: providerv1alpha1.NamespacedObjectReference{ + ObjectReference: providerv1alpha1.ObjectReference{ + APIVersion: localAG.APIVersion, + Kind: localAG.Kind, + Name: localAG.Name, + }, + Namespace: localAG.ResolveNamespace(localAG.GetNamespace()), + }, + AddressGroup: providerv1alpha1.NamespacedObjectReference{ + ObjectReference: providerv1alpha1.ObjectReference{ + APIVersion: targetAG.APIVersion, + Kind: targetAG.Kind, + Name: targetAG.Name, + }, + Namespace: targetAG.ResolveNamespace(targetAG.GetNamespace()), + }, + Ports: []providerv1alpha1.AccPorts{ + { + D: strings.Join(aggregatedPorts, ","), + }, + }, + Action: providerv1alpha1.ActionAccept, + Logs: true, + Priority: &providerv1alpha1.RulePrioritySpec{ + Value: 100, + }, + } + + newRule := &providerv1alpha1.IEAgAgRule{ + ObjectMeta: metav1.ObjectMeta{ + Name: ruleName, + Namespace: ruleNamespace, + OwnerReferences: ownerRefs, + }, + Spec: ruleSpec, + } + + for i := 0; i < DefaultMaxRetries; i++ { + if err := r.Create(ctx, newRule); err != nil { + if errors.IsAlreadyExists(err) { + logger.Info("Rule already exists, switching to update", + "namespace", ruleNamespace, + "name", ruleName) + + existingRule := &providerv1alpha1.IEAgAgRule{} + if err := r.Get(ctx, types.NamespacedName{ + Namespace: ruleNamespace, + Name: ruleName, + }, existingRule); err == nil { + return r.updateExistingIEAgAgRuleWithAggregation(ctx, existingRule, + aggregatedPorts, contributingRules) + } + } else if errors.IsConflict(err) { + logger.Info("Conflict detected, retrying", + "attempt", i+1, + "maxRetries", DefaultMaxRetries) + time.Sleep(DefaultRetryInterval) + continue + } else { + logger.Error(err, "Failed to create rule") + return "", err + } + } else { + logger.Info("Successfully created aggregated rule", + "namespace", ruleNamespace, + "name", ruleName) + return ruleName, nil + } + } + + return "", fmt.Errorf("failed to create rule after %d retries", DefaultMaxRetries) +} + +// updateExistingIEAgAgRuleWithAggregation обновляет существующее правило с агрегацией +func (r *RuleS2SReconciler) updateExistingIEAgAgRuleWithAggregation( + ctx context.Context, + existingRule *providerv1alpha1.IEAgAgRule, + aggregatedPorts []string, + contributingRules []ContributingRule, +) (string, error) { + logger := log.FromContext(ctx) + + logger.Info("Updating existing IEAgAgRule with aggregation", + "namespace", existingRule.Namespace, + "name", existingRule.Name, + "currentPorts", existingRule.Spec.Ports, + "newAggregatedPorts", strings.Join(aggregatedPorts, ",")) + + // If no ports after aggregation, delete the rule + if len(aggregatedPorts) == 0 { + logger.Info("Deleting existing IEAgAgRule with no ports after aggregation", + "namespace", existingRule.Namespace, + "name", existingRule.Name) + + if err := r.Delete(ctx, existingRule); err != nil && !errors.IsNotFound(err) { + logger.Error(err, "Failed to delete rule with no ports") + return "", err + } + + logger.Info("Successfully deleted rule with no aggregated ports", + "namespace", existingRule.Namespace, + "name", existingRule.Name) + + return "", nil + } + + latestRule := &providerv1alpha1.IEAgAgRule{} + if err := r.Get(ctx, types.NamespacedName{ + Namespace: existingRule.Namespace, + Name: existingRule.Name, + }, latestRule); err != nil { + logger.Error(err, "Failed to get latest version of rule") + return "", err + } + + original := latestRule.DeepCopy() + + latestRule.Spec.Ports = []providerv1alpha1.AccPorts{ + { + D: strings.Join(aggregatedPorts, ","), + }, + } + + existingUIDs := make(map[types.UID]bool) + for _, ref := range latestRule.OwnerReferences { + existingUIDs[ref.UID] = true + } + + addedCount := 0 + for _, contrib := range contributingRules { + if !existingUIDs[contrib.RuleS2S.UID] { + latestRule.OwnerReferences = append(latestRule.OwnerReferences, metav1.OwnerReference{ + APIVersion: netguardv1alpha1.GroupVersion.String(), + Kind: "RuleS2S", + Name: contrib.RuleS2S.Name, + UID: contrib.RuleS2S.UID, + Controller: ptr.To(false), + BlockOwnerDeletion: ptr.To(true), + }) + addedCount++ + + logger.Info("Adding new owner reference", + "ruleS2S", contrib.RuleS2S.Name, + "uid", contrib.RuleS2S.UID) + } + } + + logger.Info("Owner references update summary", + "existing", len(original.OwnerReferences), + "added", addedCount, + "total", len(latestRule.OwnerReferences)) + + patch := client.MergeFrom(original) + if err := PatchWithRetry(ctx, r.Client, latestRule, patch, DefaultMaxRetries); err != nil { + logger.Error(err, "Failed to patch rule") + return "", err + } + + logger.Info("Successfully updated aggregated rule", + "namespace", latestRule.Namespace, + "name", latestRule.Name) + + return latestRule.Name, nil +} + // generateRuleName creates a deterministic rule name based on input parameters func (r *RuleS2SReconciler) generateRuleName( trafficDirection string, @@ -852,8 +1467,6 @@ func (r *RuleS2SReconciler) generateRuleName( targetAGName, strings.ToLower(protocol)) - // Нет доступа к логгеру здесь, но мы логируем входные параметры и результат в вызывающей функции - h := sha256.New() h.Write([]byte(input)) hash := h.Sum(nil) @@ -914,74 +1527,394 @@ func (e *FailedToDeleteRulesError) Error() string { return fmt.Sprintf("failed to delete the following IEAgAgRules: %s", strings.Join(e.FailedRules, ", ")) } -// deleteRelatedIEAgAgRules deletes all IEAgAgRules that have an OwnerReference to the given RuleS2S -// Returns a FailedToDeleteRulesError if any rules could not be deleted +// deleteRelatedIEAgAgRules deletes or updates IEAgAgRules when RuleS2S is deleted func (r *RuleS2SReconciler) deleteRelatedIEAgAgRules(ctx context.Context, ruleS2S *netguardv1alpha1.RuleS2S) error { logger := log.FromContext(ctx) - // Get all IEAgAgRules across all namespaces + logger.Info("Processing deletion of related IEAgAgRules with aggregation support", + "ruleS2S", ruleS2S.Name, + "uid", ruleS2S.UID) + ieAgAgRuleList := &providerv1alpha1.IEAgAgRuleList{} if err := r.List(ctx, ieAgAgRuleList); err != nil { return err } - logger.Info("Deleting IEAgAgRules") - logger.Info("Deleting rule", "ruleName", ruleS2S.GetName(), "uuid", ruleS2S.GetUID()) - logger.Info("ieAgAgRuleList", "listNumber", len(ieAgAgRuleList.Items)) + // Get services for this RuleS2S to generate legacy rule names + localService, targetService, err := r.getServicesForRule(ctx, ruleS2S) + var expectedLegacyRuleNames map[string]bool + if err == nil { + expectedLegacyRuleNames = r.generateExpectedLegacyRuleNames(ruleS2S, localService, targetService) + logger.Info("Generated expected legacy rule names for search", + "ruleS2S", ruleS2S.Name, + "expectedLegacyRules", len(expectedLegacyRuleNames)) + } else { + logger.Info("Could not get services for legacy rule name generation, will only search by owner reference", + "ruleS2S", ruleS2S.Name, + "error", err) + expectedLegacyRuleNames = make(map[string]bool) + } var failedRules []string + processedRules := 0 + updatedRules := 0 + deletedRules := 0 - // Check each rule for an OwnerReference to this RuleS2S for _, rule := range ieAgAgRuleList.Items { + hasOwnerRef := false + isLegacyRule := false + + // Check if rule has owner reference to this RuleS2S (new aggregation rules) for _, ownerRef := range rule.GetOwnerReferences() { - logger.Info("Checking rule", "Kind", ownerRef.Kind, "uuid", ownerRef.UID) if ownerRef.UID == ruleS2S.GetUID() && ownerRef.Kind == "RuleS2S" && ownerRef.APIVersion == netguardv1alpha1.GroupVersion.String() { + hasOwnerRef = true + break + } + } - // Found a rule that references this RuleS2S - logger.Info("Deleting related IEAgAgRule", "name", rule.Name, "namespace", rule.Namespace) - - // First, remove the finalizer if it exists - ruleCopy := rule.DeepCopy() - if controllerutil.ContainsFinalizer(ruleCopy, "provider.sgroups.io/finalizer") { - logger.Info("Removing finalizer from IEAgAgRule", "name", ruleCopy.Name, "namespace", ruleCopy.Namespace) - controllerutil.RemoveFinalizer(ruleCopy, "provider.sgroups.io/finalizer") - - // Update the rule to remove the finalizer with retry - if err := UpdateWithRetry(ctx, r.Client, ruleCopy, DefaultMaxRetries); err != nil { - if !errors.IsNotFound(err) { - logger.Error(err, "Failed to remove finalizer from IEAgAgRule", - "name", ruleCopy.Name, "namespace", ruleCopy.Namespace) - // Continue with deletion attempt even if finalizer removal fails - } + // Check if rule name matches expected legacy rule names (old rules) + if !hasOwnerRef && len(expectedLegacyRuleNames) > 0 { + ruleKey := fmt.Sprintf("%s/%s", rule.Namespace, rule.Name) + if expectedLegacyRuleNames[ruleKey] { + isLegacyRule = true + logger.Info("Found legacy rule by name match", + "rule", rule.Name, + "namespace", rule.Namespace, + "ruleS2S", ruleS2S.Name) + } + } + + // Additional search for orphaned legacy rules when services have no address groups + if !hasOwnerRef && !isLegacyRule && len(expectedLegacyRuleNames) == 0 { + // Check if this could be a legacy rule created by this RuleS2S + // by examining traffic direction and rule name pattern + if r.couldBeLegacyRuleForRuleS2S(&rule, ruleS2S) { + isLegacyRule = true + logger.Info("Found orphaned legacy rule by pattern match", + "rule", rule.Name, + "namespace", rule.Namespace, + "ruleS2S", ruleS2S.Name, + "traffic", rule.Spec.Traffic) + } + } + + if !hasOwnerRef && !isLegacyRule { + continue + } + + processedRules++ + + // For legacy rules (no owner ref), always delete them directly + if isLegacyRule && !hasOwnerRef { + logger.Info("Deleting legacy IEAgAgRule (no owner reference)", + "rule", rule.Name, + "namespace", rule.Namespace) + + ruleCopy := rule.DeepCopy() + if controllerutil.ContainsFinalizer(ruleCopy, "provider.sgroups.io/finalizer") { + controllerutil.RemoveFinalizer(ruleCopy, "provider.sgroups.io/finalizer") + if err := UpdateWithRetry(ctx, r.Client, ruleCopy, DefaultMaxRetries); err != nil { + if !errors.IsNotFound(err) { + logger.Error(err, "Failed to remove finalizer from legacy rule") } } + } - // Then delete the rule - if err := r.Delete(ctx, ruleCopy); err != nil { + if err := r.Delete(ctx, ruleCopy); err != nil { + if !errors.IsNotFound(err) { + logger.Error(err, "Failed to delete legacy rule") + failedRules = append(failedRules, fmt.Sprintf("%s/%s", ruleCopy.Namespace, ruleCopy.Name)) + } else { + deletedRules++ + } + } else { + deletedRules++ + } + continue + } + + // For rules with owner references (new aggregation rules) + if len(rule.GetOwnerReferences()) > 1 { + logger.Info("Updating IEAgAgRule - removing owner reference and recalculating ports", + "rule", rule.Name, + "namespace", rule.Namespace, + "remainingOwners", len(rule.GetOwnerReferences())-1) + + if err := r.updateRuleRemoveOwnerAndRecalculatePorts(ctx, &rule, ruleS2S); err != nil { + logger.Error(err, "Failed to update rule after owner removal", + "rule", rule.Name, + "namespace", rule.Namespace) + failedRules = append(failedRules, fmt.Sprintf("%s/%s", rule.Namespace, rule.Name)) + } else { + updatedRules++ + } + } else { + logger.Info("Deleting IEAgAgRule - last owner", + "rule", rule.Name, + "namespace", rule.Namespace) + + ruleCopy := rule.DeepCopy() + if controllerutil.ContainsFinalizer(ruleCopy, "provider.sgroups.io/finalizer") { + controllerutil.RemoveFinalizer(ruleCopy, "provider.sgroups.io/finalizer") + if err := UpdateWithRetry(ctx, r.Client, ruleCopy, DefaultMaxRetries); err != nil { if !errors.IsNotFound(err) { - logger.Error(err, "Failed to delete related IEAgAgRule", - "name", ruleCopy.Name, "namespace", ruleCopy.Namespace) - // Add to failed rules list instead of returning immediately - failedRules = append(failedRules, fmt.Sprintf("%s/%s", ruleCopy.Namespace, ruleCopy.Name)) + logger.Error(err, "Failed to remove finalizer") } } } + + if err := r.Delete(ctx, ruleCopy); err != nil { + if !errors.IsNotFound(err) { + logger.Error(err, "Failed to delete rule") + failedRules = append(failedRules, fmt.Sprintf("%s/%s", ruleCopy.Namespace, ruleCopy.Name)) + } else { + deletedRules++ + } + } else { + deletedRules++ + } } } - // If any rules failed to delete, return a custom error + logger.Info("Deletion processing summary", + "processedRules", processedRules, + "updatedRules", updatedRules, + "deletedRules", deletedRules, + "failedRules", len(failedRules)) + if len(failedRules) > 0 { - logger.Error(fmt.Errorf("failed to delete some IEAgAgRules"), - "Some IEAgAgRules could not be deleted", - "failedRules", strings.Join(failedRules, ", ")) return &FailedToDeleteRulesError{FailedRules: failedRules} } return nil } +// generateExpectedLegacyRuleNames generates expected rule names for legacy rules +func (r *RuleS2SReconciler) generateExpectedLegacyRuleNames( + ruleS2S *netguardv1alpha1.RuleS2S, + localService *netguardv1alpha1.Service, + targetService *netguardv1alpha1.Service, +) map[string]bool { + expectedRules := make(map[string]bool) + + localAddressGroups := localService.AddressGroups.Items + targetAddressGroups := targetService.AddressGroups.Items + + // Generate rule names for all combinations of address groups and protocols + for _, localAG := range localAddressGroups { + for _, targetAG := range targetAddressGroups { + // Generate TCP rule name + tcpRuleName := r.generateRuleName( + ruleS2S.Spec.Traffic, + localAG.Name, + targetAG.Name, + "tcp") + + // Generate UDP rule name + udpRuleName := r.generateRuleName( + ruleS2S.Spec.Traffic, + localAG.Name, + targetAG.Name, + "udp") + + // Determine namespace for the rule based on traffic direction + var ruleNamespace string + if ruleS2S.Spec.Traffic == "ingress" { + ruleNamespace = localAG.ResolveNamespace(ruleS2S.GetNamespace()) + } else { + ruleNamespace = targetAG.ResolveNamespace(ruleS2S.GetNamespace()) + } + + // Add to expected rules map + tcpRuleKey := fmt.Sprintf("%s/%s", ruleNamespace, tcpRuleName) + udpRuleKey := fmt.Sprintf("%s/%s", ruleNamespace, udpRuleName) + + expectedRules[tcpRuleKey] = true + expectedRules[udpRuleKey] = true + } + } + + return expectedRules +} + +// couldBeLegacyRuleForRuleS2S checks if an IEAgAgRule could be a legacy rule created by the given RuleS2S +func (r *RuleS2SReconciler) couldBeLegacyRuleForRuleS2S( + rule *providerv1alpha1.IEAgAgRule, + ruleS2S *netguardv1alpha1.RuleS2S, +) bool { + // Check traffic direction matches + if strings.ToLower(string(rule.Spec.Traffic)) != strings.ToLower(ruleS2S.Spec.Traffic) { + return false + } + + // Check rule name pattern - legacy rules have prefixes based on traffic direction + var expectedPrefix string + if strings.ToLower(ruleS2S.Spec.Traffic) == "ingress" { + expectedPrefix = "ing-" + } else { + expectedPrefix = "egr-" + } + + if !strings.HasPrefix(rule.Name, expectedPrefix) { + return false + } + + // Check if rule has empty ports + if len(rule.Spec.Ports) > 0 && rule.Spec.Ports[0].D == "" { + // This is likely an orphaned rule with empty ports that should be deleted + return true + } + + return false +} + +// updateRuleRemoveOwnerAndRecalculatePorts updates a rule after removing one of its owners +func (r *RuleS2SReconciler) updateRuleRemoveOwnerAndRecalculatePorts( + ctx context.Context, + rule *providerv1alpha1.IEAgAgRule, + deletedRuleS2S *netguardv1alpha1.RuleS2S, +) error { + logger := log.FromContext(ctx) + + latestRule := &providerv1alpha1.IEAgAgRule{} + if err := r.Get(ctx, types.NamespacedName{ + Namespace: rule.Namespace, + Name: rule.Name, + }, latestRule); err != nil { + return err + } + + original := latestRule.DeepCopy() + + newOwnerRefs := []metav1.OwnerReference{} + remainingRuleS2SUIDs := []types.UID{} + + for _, ref := range latestRule.OwnerReferences { + if ref.UID != deletedRuleS2S.UID { + newOwnerRefs = append(newOwnerRefs, ref) + if ref.Kind == "RuleS2S" { + remainingRuleS2SUIDs = append(remainingRuleS2SUIDs, ref.UID) + } + } + } + + latestRule.OwnerReferences = newOwnerRefs + + logger.Info("Recalculating ports from remaining owners", + "remainingOwners", len(remainingRuleS2SUIDs)) + + aggregatedPorts, err := r.recalculatePortsFromRemainingOwners(ctx, remainingRuleS2SUIDs, rule) + if err != nil { + logger.Error(err, "Failed to recalculate ports") + } else { + // If no ports remain after recalculation, delete the rule instead of updating + if len(aggregatedPorts) == 0 { + logger.Info("No ports remaining after recalculation, deleting rule", + "rule", latestRule.Name, + "namespace", latestRule.Namespace) + + if err := r.Delete(ctx, latestRule); err != nil && !errors.IsNotFound(err) { + logger.Error(err, "Failed to delete rule with no remaining ports") + return err + } + + logger.Info("Successfully deleted rule with no remaining ports", + "rule", latestRule.Name, + "namespace", latestRule.Namespace) + + return nil + } + + latestRule.Spec.Ports = []providerv1alpha1.AccPorts{ + { + D: strings.Join(aggregatedPorts, ","), + }, + } + + logger.Info("Updated ports after recalculation", + "oldPorts", original.Spec.Ports, + "newPorts", strings.Join(aggregatedPorts, ",")) + } + + patch := client.MergeFrom(original) + return PatchWithRetry(ctx, r.Client, latestRule, patch, DefaultMaxRetries) +} + +// recalculatePortsFromRemainingOwners recalculates ports from remaining RuleS2S owners +func (r *RuleS2SReconciler) recalculatePortsFromRemainingOwners( + ctx context.Context, + remainingRuleS2SUIDs []types.UID, + rule *providerv1alpha1.IEAgAgRule, +) ([]string, error) { + logger := log.FromContext(ctx) + + if len(remainingRuleS2SUIDs) == 0 { + return []string{}, nil + } + + allRules := &netguardv1alpha1.RuleS2SList{} + if err := r.List(ctx, allRules); err != nil { + return nil, err + } + + uidToRuleS2S := make(map[types.UID]*netguardv1alpha1.RuleS2S) + for i := range allRules.Items { + uidToRuleS2S[allRules.Items[i].UID] = &allRules.Items[i] + } + + var contributingRules []ContributingRule + + for _, uid := range remainingRuleS2SUIDs { + ruleS2S, exists := uidToRuleS2S[uid] + if !exists { + logger.Info("RuleS2S not found by UID, skipping", "uid", uid) + continue + } + + localService, targetService, err := r.getServicesForRule(ctx, ruleS2S) + if err != nil { + logger.Error(err, "Failed to get services for RuleS2S", "name", ruleS2S.Name) + continue + } + + var ports []string + if strings.ToLower(ruleS2S.Spec.Traffic) == "ingress" { + ports = r.extractPortsFromService(localService) + } else { + ports = r.extractPortsFromService(targetService) + } + + if len(ports) > 0 { + contributingRules = append(contributingRules, ContributingRule{ + RuleS2S: ruleS2S, + Ports: ports, + }) + } + } + + portSet := make(map[string]bool) + for _, contrib := range contributingRules { + for _, port := range contrib.Ports { + portSet[port] = true + } + } + + var aggregatedPorts []string + for port := range portSet { + aggregatedPorts = append(aggregatedPorts, port) + } + + sort.Strings(aggregatedPorts) + + logger.Info("Recalculated aggregated ports", + "contributingRules", len(contributingRules), + "aggregatedPorts", strings.Join(aggregatedPorts, ",")) + + return aggregatedPorts, nil +} + // findRuleS2SForService finds all RuleS2S resources that reference a Service through ServiceAlias func (r *RuleS2SReconciler) findRuleS2SForService(ctx context.Context, obj client.Object) []reconcile.Request { service, ok := obj.(*netguardv1alpha1.Service) @@ -1122,12 +2055,10 @@ func (r *RuleS2SReconciler) SetupWithManager(mgr ctrl.Manager) error { return err } - // Добавляем составной индекс для быстрого поиска дубликатов if err := mgr.GetFieldIndexer().IndexField(context.Background(), &netguardv1alpha1.RuleS2S{}, "spec.composite", func(obj client.Object) []string { rule := obj.(*netguardv1alpha1.RuleS2S) - // Создаем уникальный ключ на основе полей спецификации composite := fmt.Sprintf("%s-%s-%s-%s", rule.Spec.Traffic, rule.Spec.ServiceLocalRef.Name, diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 113683d..ba973d7 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -285,3 +285,63 @@ func SafeDeleteAndWait(ctx context.Context, c client.Client, obj client.Object, } } } + +// DeleteWithRetry deletes a resource with retries on specific errors +func DeleteWithRetry(ctx context.Context, c client.Client, obj client.Object, maxRetries int) error { + logger := log.FromContext(ctx) + name := obj.GetName() + namespace := obj.GetNamespace() + + logger.Info("Starting DeleteWithRetry", + "resource", fmt.Sprintf("%s/%s", namespace, name), + "resourceType", fmt.Sprintf("%T", obj), + "maxRetries", maxRetries) + + for i := 0; i < maxRetries; i++ { + err := c.Delete(ctx, obj) + if err == nil { + logger.Info("Delete successful", + "resource", fmt.Sprintf("%s/%s", namespace, name), + "attempt", i+1) + return nil + } + + if apierrors.IsNotFound(err) { + // Resource already deleted + logger.Info("Resource already deleted", + "resource", fmt.Sprintf("%s/%s", namespace, name), + "attempt", i+1) + return nil + } + + // Check if this is a validation error that might be temporary + // (e.g., webhook blocking deletion due to active bindings) + if apierrors.IsInvalid(err) || apierrors.IsForbidden(err) { + logger.Info("Validation/Forbidden error detected, retrying", + "resource", fmt.Sprintf("%s/%s", namespace, name), + "attempt", i+1, + "maxRetries", maxRetries, + "error", err.Error()) + + // Wait before retrying with exponential backoff + backoff := DefaultRetryInterval * time.Duration(1<