From c2edd95521421aed3d0201e97a9d0da799ee61cf Mon Sep 17 00:00:00 2001 From: Robert Kubica <168091212+kubicar@users.noreply.github.com> Date: Wed, 15 Jan 2025 08:52:36 +0100 Subject: [PATCH] feat: extend APIGateway blocking deletion with RateLimit resource (#1602) * feat: extend APIGateway blocking deletion with RateLimit resource ( #1552) * feat: hide blocking deletion with RateLimit resource behind feature gate (#1552) --- .../gateway/ratelimit/ratelimit_disabled.go | 2 + .../gateway/ratelimit/ratelimit_enabled.go | 6 +- controllers/operator/apigateway_controller.go | 32 ++++++- .../apigateway_controller_integration_test.go | 90 ++++++++++++++++++- controllers/operator/suite_test.go | 2 + 5 files changed, 129 insertions(+), 3 deletions(-) diff --git a/controllers/gateway/ratelimit/ratelimit_disabled.go b/controllers/gateway/ratelimit/ratelimit_disabled.go index 22d6b9929..1a203c2a1 100644 --- a/controllers/gateway/ratelimit/ratelimit_disabled.go +++ b/controllers/gateway/ratelimit/ratelimit_disabled.go @@ -7,6 +7,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/manager" ) +const RateLimiterEnabled = false + func Setup(_ manager.Manager, _ *runtime.Scheme) error { return nil } diff --git a/controllers/gateway/ratelimit/ratelimit_enabled.go b/controllers/gateway/ratelimit/ratelimit_enabled.go index 7146faf01..2de6f9e75 100644 --- a/controllers/gateway/ratelimit/ratelimit_enabled.go +++ b/controllers/gateway/ratelimit/ratelimit_enabled.go @@ -11,7 +11,11 @@ import ( "time" ) -const defaultReconciliationPeriod = 3 * time.Minute +const ( + RateLimiterEnabled = true + + defaultReconciliationPeriod = 3 * time.Minute +) func Setup(mgr manager.Manager, scheme *runtime.Scheme) error { utilruntime.Must(ratelimitv1alpha1.AddToScheme(scheme)) diff --git a/controllers/operator/apigateway_controller.go b/controllers/operator/apigateway_controller.go index c82be2603..dedffdc4c 100644 --- a/controllers/operator/apigateway_controller.go +++ b/controllers/operator/apigateway_controller.go @@ -28,10 +28,12 @@ import ( "errors" + ratelimitv1alpha1 "github.com/kyma-project/api-gateway/apis/gateway/ratelimit/v1alpha1" "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" "github.com/kyma-project/api-gateway/apis/operator/v1alpha1" operatorv1alpha1 "github.com/kyma-project/api-gateway/apis/operator/v1alpha1" "github.com/kyma-project/api-gateway/controllers" + "github.com/kyma-project/api-gateway/controllers/gateway/ratelimit" "github.com/kyma-project/api-gateway/internal/dependencies" "github.com/kyma-project/api-gateway/internal/reconciliations/gateway" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -223,7 +225,17 @@ func (r *APIGatewayReconciler) reconcileFinalizer(ctx context.Context, apiGatewa "There are ORY Oathkeeper Rule(s) that block the deletion of API-Gateway CR. Please take a look at kyma-system/api-gateway-controller-manager logs to see more information about the warning", conditions.DeletionBlockedExistingResources.AdditionalMessage(": "+strings.Join(oryRulesFound, ", ")).Condition()) } - + if ratelimit.RateLimiterEnabled { + rateLimiterRules, err := rateLimitsExists(ctx, r.Client) + if err != nil { + return controllers.ErrorStatus(err, "Error during listing existing Rate Limit", conditions.ReconcileFailed.Condition()) + } + if len(rateLimiterRules) > 0 { + return controllers.WarningStatus(errors.New("could not delete API-Gateway CR since there are RateLimit(s) that block its deletion"), + "There are RateLimit(s) that block the deletion of API-Gateway CR. Please take a look at kyma-system/api-gateway-controller-manager logs to see more information about the warning", + conditions.DeletionBlockedExistingResources.AdditionalMessage(": "+strings.Join(rateLimiterRules, ", ")).Condition()) + } + } if err := removeFinalizer(ctx, r.Client, apiGatewayCR); err != nil { ctrl.Log.Error(err, "Error happened during API-Gateway CR finalizer removal") return controllers.ErrorStatus(err, "Could not remove finalizer", conditions.ReconcileFailed.Condition()) @@ -233,6 +245,24 @@ func (r *APIGatewayReconciler) reconcileFinalizer(ctx context.Context, apiGatewa return controllers.ReadyStatus(conditions.ReconcileSucceeded.Condition()) } +func rateLimitsExists(ctx context.Context, k8sClient client.Client) ([]string, error) { + rateLimits := ratelimitv1alpha1.RateLimitList{} + err := k8sClient.List(ctx, &rateLimits) + if meta.IsNoMatchError(err) || apierrors.IsNotFound(err) { + // RateLimits does not exist, there is not blocking rate limits + return nil, nil + } + + ctrl.Log.Info(fmt.Sprintf("There are %d RateLimit(s) found on cluster", len(rateLimits.Items))) + var blockingRateLimits []string + for _, rateLimit := range rateLimits.Items { + rateLimitNamespacedName := rateLimit.GetNamespace() + "/" + rateLimit.GetName() + ctrl.Log.Info("RateLimit rule blocking deletion", "rule", rateLimitNamespacedName) + blockingRateLimits = append(blockingRateLimits, rateLimitNamespacedName) + } + return blockingRateLimits, nil +} + func apiRulesExist(ctx context.Context, k8sClient client.Client) ([]string, error) { apiRuleList := v1beta1.APIRuleList{} err := k8sClient.List(ctx, &apiRuleList) diff --git a/controllers/operator/apigateway_controller_integration_test.go b/controllers/operator/apigateway_controller_integration_test.go index 9208dac21..afb2ea9ad 100644 --- a/controllers/operator/apigateway_controller_integration_test.go +++ b/controllers/operator/apigateway_controller_integration_test.go @@ -3,6 +3,7 @@ package operator import ( "context" "fmt" + "github.com/kyma-project/api-gateway/controllers/gateway/ratelimit" "math/rand" "time" @@ -13,6 +14,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/utils/ptr" + ratelimitv1alpha1 "github.com/kyma-project/api-gateway/apis/gateway/ratelimit/v1alpha1" gatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" "github.com/kyma-project/api-gateway/apis/operator/v1alpha1" . "github.com/onsi/ginkgo/v2" @@ -28,6 +30,7 @@ var _ = Describe("API Gateway Controller", Serial, func() { AfterEach(func() { deleteApiRules() deleteVirtualServices() + deleteRateLimitRules() deleteApiGateways() }) @@ -154,7 +157,40 @@ var _ = Describe("API Gateway Controller", Serial, func() { g.Expect(apiGateway.Status.Description).To(Equal("There are APIRule(s) that block the deletion of API-Gateway CR. Please take a look at kyma-system/api-gateway-controller-manager logs to see more information about the warning")) }, eventuallyTimeout).Should(Succeed()) }) - + if ratelimit.RateLimiterEnabled { + It("Should set APIGateway CR in Warning state on deletion when RateLimit(s) exist", func() { + // given + apiGateway := v1alpha1.APIGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: generateName(), + }, + } + rateLimit := getRateLimit() + + By("Creating RateLimit") + Expect(k8sClient.Create(context.Background(), &rateLimit)).Should(Succeed()) + By("Creating APIGateway") + Expect(k8sClient.Create(context.Background(), &apiGateway)).Should(Succeed()) + + By("Verifying that APIGateway CR reconciliation was successful") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: apiGateway.Name}, &apiGateway)).Should(Succeed()) + g.Expect(apiGateway.Status.State).To(Equal(v1alpha1.Ready)) + }, eventuallyTimeout).Should(Succeed()) + + // when + By("Deleting APIGateway") + Expect(k8sClient.Delete(context.Background(), &apiGateway)).Should(Succeed()) + + // then + By("Verifying that APIGateway CR has Warning state") + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(context.Background(), client.ObjectKey{Name: apiGateway.Name}, &apiGateway)).Should(Succeed()) + g.Expect(apiGateway.Status.State).To(Equal(v1alpha1.Warning)) + g.Expect(apiGateway.Status.Description).To(Equal("There are RateLimit(s) that block the deletion of API-Gateway CR. Please take a look at kyma-system/api-gateway-controller-manager logs to see more information about the warning")) + }, eventuallyTimeout).Should(Succeed()) + }) + } It("should update lastTransitionTime of Ready condition when only reason or message changed", func() { // given blockingApiRule := getApiRule() @@ -561,3 +597,55 @@ func deleteVirtualServices() { } }, eventuallyTimeout).Should(Succeed()) } + +func getRateLimit() ratelimitv1alpha1.RateLimit { + return ratelimitv1alpha1.RateLimit{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Spec: ratelimitv1alpha1.RateLimitSpec{ + SelectorLabels: map[string]string{"app": "test"}, + Local: ratelimitv1alpha1.LocalConfig{ + DefaultBucket: ratelimitv1alpha1.BucketSpec{ + MaxTokens: 1, + TokensPerFill: 1, + FillInterval: &metav1.Duration{ + Duration: 60 * time.Second, + }, + }, + }, + EnableResponseHeaders: false, + Enforce: false, + }, + } +} + +func deleteRateLimitRules() { + Eventually(func(g Gomega) { + By("Checking if RateLimit exists as part of teardown") + list := ratelimitv1alpha1.RateLimitList{} + Expect(k8sClient.List(context.Background(), &list)).Should(Succeed()) + + for _, rateLimit := range list.Items { + rateLimitTeardown(&rateLimit) + } + }, eventuallyTimeout).Should(Succeed()) +} + +func rateLimitTeardown(rateLimit *ratelimitv1alpha1.RateLimit) { + By(fmt.Sprintf("Deleting RateLimit %s as part of teardown", rateLimit.GetName())) + Eventually(func(g Gomega) { + err := k8sClient.Delete(context.Background(), rateLimit) + + if err != nil { + Expect(errors.IsNotFound(err)).To(BeTrue()) + } + + r := ratelimitv1alpha1.RateLimit{} + + err = k8sClient.Get(context.Background(), client.ObjectKey{Name: rateLimit.Name, Namespace: rateLimit.Namespace}, &r) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + + }, eventuallyTimeout).Should(Succeed()) +} diff --git a/controllers/operator/suite_test.go b/controllers/operator/suite_test.go index a0a1d6776..090d4f14d 100644 --- a/controllers/operator/suite_test.go +++ b/controllers/operator/suite_test.go @@ -25,6 +25,7 @@ import ( "github.com/kyma-project/api-gateway/internal/reconciliations/oathkeeper" + ratelimitv1alpha1 "github.com/kyma-project/api-gateway/apis/gateway/ratelimit/v1alpha1" "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" gatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1" operatorv1alpha1 "github.com/kyma-project/api-gateway/apis/operator/v1alpha1" @@ -250,6 +251,7 @@ func getTestScheme() *runtime.Scheme { utilruntime.Must(networkingv1alpha3.AddToScheme(s)) utilruntime.Must(networkingv1beta1.AddToScheme(s)) utilruntime.Must(oryv1alpha1.AddToScheme(s)) + utilruntime.Must(ratelimitv1alpha1.AddToScheme(s)) return s }