fix: cascade delete

Author: gozdan-lordz

internal/controller/addressgroupbinding_controller.go

@@ -86,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) {
@@ -287,33 +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",
-			"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 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
 		}
-		// 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),
+			"addressGroupNamespace", addressGroupNamespace,
+			"bindingNamespace", binding.GetNamespace())
 	}
 
 	// If owner references were updated, update the binding

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)
 }

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<<uint(i))
+			logger.Info("Waiting before retry",
+				"resource", fmt.Sprintf("%s/%s", namespace, name),
+				"backoffMs", backoff.Milliseconds())
+			time.Sleep(backoff)
+			continue
+		}
+
+		// For other errors, return immediately
+		logger.Error(err, "Non-retryable error when deleting",
+			"resource", fmt.Sprintf("%s/%s", namespace, name),
+			"errorType", fmt.Sprintf("%T", err))
+		return err
+	}
+
+	logger.Error(fmt.Errorf("max retries exceeded"), "Failed to delete resource after max retries",
+		"resource", fmt.Sprintf("%s/%s", namespace, name),
+		"maxRetries", maxRetries)
+
+	return fmt.Errorf("failed to delete resource after %d retries", maxRetries)
+}