fix ratelimit namespace and handle deletion of rl policy (Kuadrant#94)

* fix ratelimit namespace and handle deletion of rl policy

* ratelimit cr name template rlp-%s-%s-%d

Author: eguzki

Makefile

@@ -260,7 +260,7 @@ istio-manifest-update-test: generate-istio-manifests
 
 .PHONY: local-setup
 local-setup: local-cleanup local-setup-kind manifests kustomize generate
-	./utils/local-deployment/local-setup.sh
+	export PATH=$(PROJECT_PATH)/bin:$$PATH;	./utils/local-deployment/local-setup.sh
 
 # Deploys all services and manifests required by kuadrant to run
 # kuadrant is not deployed

controllers/apim/ratelimitpolicy_controller.go

@@ -19,7 +19,6 @@ package apim
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 
 	"github.com/go-logr/logr"
 	"github.com/gogo/protobuf/types"
@@ -82,21 +81,25 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl
 	}
 
 	if rlp.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(&rlp, patchesFinalizer) {
-		logger.Info("Removing finalizers")
+		logger.V(1).Info("Handling removal of ratelimitpolicy object")
 		if err := r.finalizeEnvoyFilters(ctx, &rlp); err != nil {
 			logger.Error(err, "failed to remove ownerRlp entry from filters patch")
 			return ctrl.Result{}, err
 		}
+		if err := r.deleteRateLimits(ctx, &rlp); err != nil {
+			logger.Error(err, "failed to delete RateLimt objects")
+			return ctrl.Result{}, err
+		}
 		controllerutil.RemoveFinalizer(&rlp, patchesFinalizer)
-		if err := r.BaseReconciler.UpdateResource(ctx, &rlp); client.IgnoreNotFound(err) != nil {
+		if err := r.UpdateResource(ctx, &rlp); client.IgnoreNotFound(err) != nil {
 			return ctrl.Result{}, err
 		}
 		return ctrl.Result{}, nil
 	}
 
 	if !controllerutil.ContainsFinalizer(&rlp, patchesFinalizer) {
 		controllerutil.AddFinalizer(&rlp, patchesFinalizer)
-		if err := r.BaseReconciler.UpdateResource(ctx, &rlp); client.IgnoreNotFound(err) != nil {
+		if err := r.UpdateResource(ctx, &rlp); client.IgnoreNotFound(err) != nil {
 			return ctrl.Result{Requeue: true}, err
 		}
 	}
@@ -530,14 +533,17 @@ func virtualHostRateLimitsPatch(vHostName string, rateLimits []*apimv1alpha1.Rat
 }
 
 func (r *RateLimitPolicyReconciler) reconcileLimits(ctx context.Context, rlp *apimv1alpha1.RateLimitPolicy) error {
-	logger := r.Logger()
+	logger := logr.FromContext(ctx)
+	rlpKey := client.ObjectKeyFromObject(rlp)
 
 	// create the RateLimit resource
 	for i, rlSpec := range rlp.Spec.Limits {
 		ratelimitfactory := common.RateLimitFactory{
 			Key: client.ObjectKey{
-				Name:      fmt.Sprintf("%s-limit-%d", rlp.Name, i+1),
-				Namespace: rlp.Namespace,
+				Name: limitadorRatelimitsName(rlpKey, i+1),
+				// Currently, Limitador Operator (v0.2.0) will configure limitador services with
+				// RateLimit CRs created in the same namespace.
+				Namespace: common.KuadrantNamespace,
 			},
 			Conditions: rlSpec.Conditions,
 			MaxValue:   rlSpec.MaxValue,
@@ -547,11 +553,7 @@ func (r *RateLimitPolicyReconciler) reconcileLimits(ctx context.Context, rlp *ap
 		}
 
 		ratelimit := ratelimitfactory.RateLimit()
-		if err := controllerutil.SetOwnerReference(rlp, ratelimit, r.Client().Scheme()); err != nil {
-			logger.Error(err, "failed to add owner ref to RateLimit resource")
-			return err
-		}
-		err := r.BaseReconciler.ReconcileResource(ctx, &v1alpha1.RateLimit{}, ratelimit, alwaysUpdateRateLimit)
+		err := r.ReconcileResource(ctx, &v1alpha1.RateLimit{}, ratelimit, alwaysUpdateRateLimit)
 		if err != nil && !apierrors.IsAlreadyExists(err) {
 			logger.Error(err, "ReconcileResource failed to create/update RateLimit resource")
 			return err

controllers/apim/ratelimitpolicy_finalizers.go

@@ -156,3 +156,28 @@ func (r *RateLimitPolicyReconciler) addParentRefEntry(ctx context.Context, patch
 	logger.Info("Successfully added parentRef entry to the EnvoyFilter")
 	return nil
 }
+
+func (r *RateLimitPolicyReconciler) deleteRateLimits(ctx context.Context, rlp *apimv1alpha1.RateLimitPolicy) error {
+	logger := logr.FromContext(ctx)
+	rlpKey := client.ObjectKeyFromObject(rlp)
+	for i := range rlp.Spec.Limits {
+		ratelimitfactory := common.RateLimitFactory{
+			Key: client.ObjectKey{
+				Name: limitadorRatelimitsName(rlpKey, i+1),
+				// Currently, Limitador Operator (v0.2.0) will configure limitador services with
+				// RateLimit CRs created in the same namespace.
+				Namespace: common.KuadrantNamespace,
+			},
+			// rest of the parameters empty
+		}
+
+		rateLimit := ratelimitfactory.RateLimit()
+		err := r.DeleteResource(ctx, rateLimit)
+		logger.V(1).Info("Removing rate limit", "ratelimit", client.ObjectKeyFromObject(rateLimit), "error", err)
+		if err != nil && !apierrors.IsNotFound(err) {
+			return err
+		}
+	}
+
+	return nil
+}

controllers/apim/utils.go

@@ -31,6 +31,11 @@ func ratelimitsPatchName(gwName string, networkKey client.ObjectKey) string {
 	return fmt.Sprintf("ratelimits-on-%s-using-%s-%s", gwName, networkKey.Namespace, networkKey.Name)
 }
 
+// limitadorRatelimitsName returns the name of Limitador RateLimit CR.
+func limitadorRatelimitsName(objKey client.ObjectKey, idx int) string {
+	return fmt.Sprintf("rlp-%s-%s-%d", objKey.Namespace, objKey.Name, idx)
+}
+
 // getAuthPolicyName generates the name of an AuthorizationPolicy using VirtualService info.
 func getAuthPolicyName(gwName, vsName string) string {
 	return fmt.Sprintf("on-%s-using-%s", gwName, vsName)