diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index b5e8fe3ff..9793aacfe 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -21,6 +21,7 @@ import ( "encoding/json" "github.com/go-logr/logr" + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" apierrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -223,9 +224,13 @@ func (r *RateLimitPolicyReconciler) deleteNetworkResourceDirectBackReference(ctx func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper"))) gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper"))) + limitadorToRLPsEventMapper := mappers.NewLimitadorToRateLimitPoliciesEventMapper( + mappers.WithLogger(r.Logger().WithName("limitadorToRLPsEventMapper")), + mappers.WithClient(r.Client())) return ctrl.NewControllerManagedBy(mgr). For(&kuadrantv1beta2.RateLimitPolicy{}). + Watches(&limitadorv1alpha1.Limitador{}, handler.EnqueueRequestsFromMapFunc(limitadorToRLPsEventMapper.Map)). Watches( &gatewayapiv1.HTTPRoute{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index d83d48811..9ba9f4831 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/controllers/ratelimitpolicy_controller_test.go @@ -12,6 +12,8 @@ import ( limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" @@ -21,6 +23,7 @@ import ( kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/common" + "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/pkg/rlptools" ) @@ -394,6 +397,67 @@ var _ = Describe("RateLimitPolicy controller", func() { time.Minute, 5*time.Second).Should(BeTrue()) }) }) + + Context("RLP Enforced Reasons", func() { + assertAcceptedCondTrueAndEnforcedCond := func(policy *kuadrantv1beta2.RateLimitPolicy, conditionStatus metav1.ConditionStatus, reason, message string) func(g Gomega) { + return func(g Gomega) { + existingPolicy := &kuadrantv1beta2.RateLimitPolicy{} + g.Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(policy), existingPolicy)).To(Succeed()) + acceptedCond := meta.FindStatusCondition(existingPolicy.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)) + g.Expect(acceptedCond).ToNot(BeNil()) + + acceptedCondMatch := acceptedCond.Status == metav1.ConditionTrue && acceptedCond.Reason == string(gatewayapiv1alpha2.PolicyReasonAccepted) + + enforcedCond := meta.FindStatusCondition(existingPolicy.Status.Conditions, string(kuadrant.PolicyReasonEnforced)) + g.Expect(enforcedCond).ToNot(BeNil()) + enforcedCondMatch := enforcedCond.Status == conditionStatus && enforcedCond.Reason == reason && enforcedCond.Message == message + + g.Expect(acceptedCondMatch && enforcedCondMatch).To(BeTrue()) + } + } + + BeforeEach(func(ctx SpecContext) { + route := testBuildBasicHttpRoute(testHTTPRouteName, testGatewayName, testNamespace, []string{"*.toystore.com"}) + Expect(k8sClient.Create(ctx, route)).To(Succeed()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(route))).WithContext(ctx).Should(BeTrue()) + }, NodeTimeout(time.Minute)) + + It("Enforced Reason", func(ctx SpecContext) { + policy := policyFactory() + Expect(k8sClient.Create(ctx, policy)).To(Succeed()) + + Eventually(assertAcceptedCondTrueAndEnforcedCond(policy, metav1.ConditionTrue, string(kuadrant.PolicyReasonEnforced), + "RateLimitPolicy has been successfully enforced")).WithContext(ctx).Should(Succeed()) + + // Remove limitador deployment to simulate enforcement error + // RLP should transition to enforcement false in this case + Expect(k8sClient.Delete(ctx, &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "limitador-limitador", Namespace: testNamespace}})).To(Succeed()) + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(ctx, client.ObjectKey{Name: "limitador-limitador", Namespace: testNamespace}, &appsv1.Deployment{})) + }).WithContext(ctx).Should(BeTrue()) + + Eventually(assertAcceptedCondTrueAndEnforcedCond(policy, metav1.ConditionFalse, string(kuadrant.PolicyReasonUnknown), + "RateLimitPolicy has encountered some issues: limitador is not ready")).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(time.Minute)) + + It("Unknown Reason", func(ctx SpecContext) { + // Remove limitador deployment to simulate enforcement error + Expect(k8sClient.Delete(ctx, &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "limitador-limitador", Namespace: testNamespace}})).To(Succeed()) + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(ctx, client.ObjectKey{Name: "limitador-limitador", Namespace: testNamespace}, &appsv1.Deployment{})) + }).WithContext(ctx).Should(BeTrue()) + + // Enforced false as limitador is not ready + policy := policyFactory() + Expect(k8sClient.Create(ctx, policy)).To(Succeed()) + Eventually(assertAcceptedCondTrueAndEnforcedCond(policy, metav1.ConditionFalse, string(kuadrant.PolicyReasonUnknown), + "RateLimitPolicy has encountered some issues: limitador is not ready")).WithContext(ctx).Should(Succeed()) + + // Enforced true once limitador is ready + Eventually(assertAcceptedCondTrueAndEnforcedCond(policy, metav1.ConditionTrue, string(kuadrant.PolicyReasonEnforced), + "RateLimitPolicy has been successfully enforced")).WithContext(ctx).Should(Succeed()) + }, SpecTimeout(time.Minute)) + }) }) var _ = Describe("RateLimitPolicy CEL Validations", func() { diff --git a/controllers/ratelimitpolicy_limits.go b/controllers/ratelimitpolicy_limits.go index f8b421962..a77f67bb8 100644 --- a/controllers/ratelimitpolicy_limits.go +++ b/controllers/ratelimitpolicy_limits.go @@ -43,8 +43,31 @@ func (r *RateLimitPolicyReconciler) reconcileLimitador(ctx context.Context, rlp if err != nil { return err } - // get the current limitador cr for the kuadrant instance so we can compare if it needs to be updated + limitador, err := r.getLimitador(ctx, rlp) + if err != nil { + return err + } + // return if limitador is up to date + if rlptools.Equal(rateLimitIndex.ToRateLimits(), limitador.Spec.Limits) { + logger.V(1).Info("limitador is up to date, skipping update") + return nil + } + + // update limitador + limitador.Spec.Limits = rateLimitIndex.ToRateLimits() + err = r.UpdateResource(ctx, limitador) + logger.V(1).Info("update limitador", "limitador", client.ObjectKeyFromObject(limitador), "err", err) + if err != nil { + return err + } + + return nil +} + +func (r *RateLimitPolicyReconciler) getLimitador(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy) (*limitadorv1alpha1.Limitador, error) { + logger, _ := logr.FromContext(ctx) + logger.V(1).Info("get kuadrant namespace") var kuadrantNamespace string kuadrantNamespace, isSet := kuadrant.GetKuadrantNamespaceFromPolicy(rlp) @@ -53,38 +76,24 @@ func (r *RateLimitPolicyReconciler) reconcileLimitador(ctx context.Context, rlp kuadrantNamespace, err = kuadrant.GetKuadrantNamespaceFromPolicyTargetRef(ctx, r.Client(), rlp) if err != nil { logger.Error(err, "failed to get kuadrant namespace") - return err + return nil, err } kuadrant.AnnotateObject(rlp, kuadrantNamespace) err = r.UpdateResource(ctx, rlp) // @guicassolato: not sure if this belongs to here if err != nil { logger.Error(err, "failed to update policy, re-queuing") - return err + return nil, err } } limitadorKey := client.ObjectKey{Name: common.LimitadorName, Namespace: kuadrantNamespace} limitador := &limitadorv1alpha1.Limitador{} - err = r.Client().Get(ctx, limitadorKey, limitador) + err := r.Client().Get(ctx, limitadorKey, limitador) logger.V(1).Info("get limitador", "limitador", limitadorKey, "err", err) if err != nil { - return err + return nil, err } - // return if limitador is up to date - if rlptools.Equal(rateLimitIndex.ToRateLimits(), limitador.Spec.Limits) { - logger.V(1).Info("limitador is up to date, skipping update") - return nil - } - - // update limitador - limitador.Spec.Limits = rateLimitIndex.ToRateLimits() - err = r.UpdateResource(ctx, limitador) - logger.V(1).Info("update limitador", "limitador", limitadorKey, "err", err) - if err != nil { - return err - } - - return nil + return limitador, nil } func (r *RateLimitPolicyReconciler) buildRateLimitIndex(ctx context.Context, rlpRefs []client.ObjectKey) (*rlptools.RateLimitIndex, error) { diff --git a/controllers/ratelimitpolicy_status.go b/controllers/ratelimitpolicy_status.go index 0edffd78e..42000782d 100644 --- a/controllers/ratelimitpolicy_status.go +++ b/controllers/ratelimitpolicy_status.go @@ -2,14 +2,17 @@ package controllers import ( "context" + "errors" "fmt" "slices" "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/reconcile" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" @@ -51,7 +54,7 @@ func (r *RateLimitPolicyReconciler) reconcileStatus(ctx context.Context, rlp *ku return ctrl.Result{}, nil } -func (r *RateLimitPolicyReconciler) calculateStatus(_ context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, specErr error) *kuadrantv1beta2.RateLimitPolicyStatus { +func (r *RateLimitPolicyReconciler) calculateStatus(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, specErr error) *kuadrantv1beta2.RateLimitPolicyStatus { newStatus := &kuadrantv1beta2.RateLimitPolicyStatus{ // Copy initial conditions. Otherwise, status will always be updated Conditions: slices.Clone(rlp.Status.Conditions), @@ -62,5 +65,30 @@ func (r *RateLimitPolicyReconciler) calculateStatus(_ context.Context, rlp *kuad meta.SetStatusCondition(&newStatus.Conditions, *acceptedCond) + // Do not set enforced condition if Accepted condition is false + if meta.IsStatusConditionFalse(newStatus.Conditions, string(gatewayapiv1alpha2.PolicyReasonAccepted)) { + return newStatus + } + + enforcedCond := r.enforcedCondition(ctx, rlp) + meta.SetStatusCondition(&newStatus.Conditions, *enforcedCond) + return newStatus } + +func (r *RateLimitPolicyReconciler) enforcedCondition(ctx context.Context, policy *kuadrantv1beta2.RateLimitPolicy) *metav1.Condition { + logger, _ := logr.FromContext(ctx) + + limitador, err := r.getLimitador(ctx, policy) + if err != nil { + logger.V(1).Error(err, "failed to get limitador") + return kuadrant.EnforcedCondition(policy, kuadrant.NewErrUnknown(policy.Kind(), err), false) + } + if meta.IsStatusConditionFalse(limitador.Status.Conditions, "Ready") { + logger.V(1).Info("Limitador is not ready") + return kuadrant.EnforcedCondition(policy, kuadrant.NewErrUnknown(policy.Kind(), errors.New("limitador is not ready")), false) + } + + logger.V(1).Info("RateLimitPolicy is enforced") + return kuadrant.EnforcedCondition(policy, nil, true) +} diff --git a/go.mod b/go.mod index 3ec8bfd4f..243696b47 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,6 @@ require ( github.com/onsi/gomega v1.30.0 go.uber.org/zap v1.26.0 golang.org/x/net v0.19.0 - golang.org/x/sync v0.5.0 google.golang.org/protobuf v1.33.0 gotest.tools v2.2.0+incompatible istio.io/api v1.20.0 @@ -148,6 +147,7 @@ require ( golang.org/x/crypto v0.17.0 // indirect golang.org/x/exp v0.0.0-20231127185646-65229373498e // indirect golang.org/x/oauth2 v0.15.0 // indirect + golang.org/x/sync v0.5.0 // indirect golang.org/x/sys v0.15.0 // indirect golang.org/x/term v0.15.0 // indirect golang.org/x/text v0.14.0 // indirect diff --git a/pkg/library/mappers/limitador_to_rlps.go b/pkg/library/mappers/limitador_to_rlps.go new file mode 100644 index 000000000..33efd9d2e --- /dev/null +++ b/pkg/library/mappers/limitador_to_rlps.go @@ -0,0 +1,47 @@ +package mappers + +import ( + "context" + + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" + kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" +) + +type LimitadorToRateLimitPoliciesEventMapper struct { + opts MapperOptions +} + +func NewLimitadorToRateLimitPoliciesEventMapper(o ...MapperOption) *LimitadorToRateLimitPoliciesEventMapper { + return &LimitadorToRateLimitPoliciesEventMapper{opts: Apply(o...)} +} + +// Map limitador to RLP requests +func (m *LimitadorToRateLimitPoliciesEventMapper) Map(ctx context.Context, obj client.Object) []reconcile.Request { + kuadrantList := &kuadrantv1beta1.KuadrantList{} + if err := m.opts.Client.List(ctx, kuadrantList, &client.ListOptions{Namespace: obj.GetNamespace()}); err != nil { + m.opts.Logger.V(1).Error(err, "failed to list kuadrant in namespace", "namespace", obj.GetNamespace()) + return []reconcile.Request{} + } + + // No kuadrant in limitador namespace - skipping as it's not managed by kuadrant + if len(kuadrantList.Items) == 0 { + m.opts.Logger.V(1).Info("no kuadrant resources found in limitador namespace, skipping") + return []reconcile.Request{} + } + + // List all RLPs as there's been an event from Limitador which may affect RLP status + rlpList := &kuadrantv1beta2.RateLimitPolicyList{} + if err := m.opts.Client.List(ctx, rlpList); err != nil { + m.opts.Logger.V(1).Error(err, "failed to list RLPs") + return []reconcile.Request{} + } + + return utils.Map(rlpList.Items, func(policy kuadrantv1beta2.RateLimitPolicy) reconcile.Request { + return reconcile.Request{NamespacedName: types.NamespacedName{Namespace: policy.GetNamespace(), Name: policy.GetName()}} + }) +} diff --git a/pkg/library/mappers/limitador_to_rlps_test.go b/pkg/library/mappers/limitador_to_rlps_test.go new file mode 100644 index 000000000..77e273436 --- /dev/null +++ b/pkg/library/mappers/limitador_to_rlps_test.go @@ -0,0 +1,88 @@ +package mappers + +import ( + "context" + "reflect" + "testing" + + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" + kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" + "github.com/kuadrant/kuadrant-operator/pkg/log" +) + +func TestLimitadorToRateLimitPoliciesEventMapper_Map(t *testing.T) { + scheme := runtime.NewScheme() + if err := kuadrantv1beta1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + if err := kuadrantv1beta2.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + type fields struct { + opts MapperOptions + } + type args struct { + ctx context.Context + obj client.Object + } + tests := []struct { + name string + fields fields + args args + want []reconcile.Request + }{ + { + name: "no kuadrants in object ns", + fields: fields{ + opts: MapperOptions{Logger: log.NewLogger(), + Client: fake.NewClientBuilder().WithObjects( + &kuadrantv1beta1.Kuadrant{ObjectMeta: metav1.ObjectMeta{Namespace: "kuadrant"}}, + ).WithScheme(scheme).Build()}, + }, + args: args{ + ctx: context.Background(), + obj: &limitadorv1alpha1.Limitador{ObjectMeta: metav1.ObjectMeta{Namespace: "limitador"}}, + }, + want: []reconcile.Request{}, + }, + { + name: "kuadrant in object ns - map RLP to requests", + fields: fields{ + opts: MapperOptions{ + Logger: log.NewLogger(), + Client: fake.NewClientBuilder().WithObjects( + &kuadrantv1beta1.Kuadrant{ObjectMeta: metav1.ObjectMeta{Namespace: "kuadrant"}}, + &kuadrantv1beta2.RateLimitPolicy{ObjectMeta: metav1.ObjectMeta{Name: "rlp1", Namespace: "ns1"}}, + &kuadrantv1beta2.RateLimitPolicy{ObjectMeta: metav1.ObjectMeta{Name: "rlp2", Namespace: "ns2"}}, + ).WithScheme(scheme).Build()}, + }, + args: args{ + ctx: context.Background(), + obj: &limitadorv1alpha1.Limitador{ObjectMeta: metav1.ObjectMeta{Namespace: "kuadrant"}}, + }, + want: []reconcile.Request{ + {NamespacedName: types.NamespacedName{Name: "rlp1", Namespace: "ns1"}}, + {NamespacedName: types.NamespacedName{Name: "rlp2", Namespace: "ns2"}}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := &LimitadorToRateLimitPoliciesEventMapper{ + opts: tt.fields.opts, + } + if got := m.Map(tt.args.ctx, tt.args.obj); !reflect.DeepEqual(got, tt.want) { + t.Errorf("Map() = %v, want %v", got, tt.want) + } + }) + } +}