From 94887208ed29c62616810f6e7f2c6a66315c3632 Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Thu, 24 Aug 2023 13:59:07 +0100 Subject: [PATCH 1/3] Revert change made to Limitador CRs Adds a new watch to check for changes in the Limitador. If there are changes we reconcile back to the expected configuration as stated in the rate limit polices (RPL). - Valid for RPL attached to httpRoute - Valid for RPL attached to gateway --- controllers/limitador_eventmapper.go | 42 +++++ controllers/ratelimitpolicy_controller.go | 20 ++- controllers/ratelimitpolicy_limits.go | 78 +++++++- pkg/rlptools/utils.go | 39 ++++ pkg/rlptools/utils_test.go | 209 ++++++++++++++++++++++ 5 files changed, 383 insertions(+), 5 deletions(-) create mode 100644 controllers/limitador_eventmapper.go diff --git a/controllers/limitador_eventmapper.go b/controllers/limitador_eventmapper.go new file mode 100644 index 000000000..3682bfef3 --- /dev/null +++ b/controllers/limitador_eventmapper.go @@ -0,0 +1,42 @@ +package controllers + +import ( + "encoding/json" + "github.com/go-logr/logr" + "github.com/kuadrant/kuadrant-operator/pkg/common" + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +type LimitadorEventMapper struct { + Logger logr.Logger +} + +func (m LimitadorEventMapper) MapToRateLimitPolicy(obj client.Object) []reconcile.Request { + limitador, ok := obj.(*limitadorv1alpha1.Limitador) + if !ok { + return []reconcile.Request{} + } + + objAnnotations := limitador.GetAnnotations() + val, ok := objAnnotations[common.RateLimitPoliciesBackRefAnnotation] + if !ok { + return []reconcile.Request{} + } + + var refs []client.ObjectKey + err := json.Unmarshal([]byte(val), &refs) + if err != nil { + return []reconcile.Request{} + } + + requests := make([]reconcile.Request, 0) + for _, ref := range refs { + m.Logger.V(1).Info("MapRateLimitPolicy", "ratelimitpolicy", ref) + requests = append(requests, reconcile.Request{ + NamespacedName: ref, + }) + } + return requests +} diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index 8c94a400a..ac3bdbeca 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -19,8 +19,8 @@ package controllers import ( "context" "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" @@ -186,7 +186,7 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp return err } - // set annotation of policies afftecting the gateway - should be the last step, only when all the reconciliation steps succeed + // set annotation of policies affecting the gateway - should be the last step, only when all the reconciliation steps succeed return r.ReconcileGatewayPolicyReferences(ctx, rlp, gatewayDiffObj) } @@ -216,7 +216,13 @@ func (r *RateLimitPolicyReconciler) deleteResources(ctx context.Context, rlp *ku } } - // update annotation of policies afftecting the gateway + // remove direct back ref from limitador CR + err = r.deleteLimitadorBackReference(ctx, rlp) + if err != nil { + return err + } + + // update annotation of policies affecting the gateway return r.ReconcileGatewayPolicyReferences(ctx, rlp, gatewayDiffObj) } @@ -241,6 +247,11 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Logger: r.Logger().WithName("gatewayRateLimitPolicyEventMapper"), Client: r.Client(), } + + limitadorEventMapper := &LimitadorEventMapper{ + Logger: r.Logger().WithName("limitadorEventMapper"), + } + return ctrl.NewControllerManagedBy(mgr). For(&kuadrantv1beta2.RateLimitPolicy{}). Watches( @@ -258,5 +269,8 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { &source.Kind{Type: &kuadrantv1beta2.RateLimitPolicy{}}, handler.EnqueueRequestsFromMapFunc(gatewayRateLimtPolicyEventMapper.MapRouteRateLimitPolicy), ). + Watches(&source.Kind{Type: &limitadorv1alpha1.Limitador{}}, + handler.EnqueueRequestsFromMapFunc(limitadorEventMapper.MapToRateLimitPolicy), + ). Complete(r) } diff --git a/controllers/ratelimitpolicy_limits.go b/controllers/ratelimitpolicy_limits.go index 1704c74ad..5de454432 100644 --- a/controllers/ratelimitpolicy_limits.go +++ b/controllers/ratelimitpolicy_limits.go @@ -2,6 +2,7 @@ package controllers import ( "context" + "encoding/json" "github.com/go-logr/logr" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" @@ -68,8 +69,12 @@ func (r *RateLimitPolicyReconciler) reconcileLimitador(ctx context.Context, rlp return err } - // return if limitador is up to date - if rlptools.Equal(rateLimitIndex.ToRateLimits(), limitador.Spec.Limits) { + updated, err := r.reconcileLimitadorBackRef(limitador, rlp) + if err != nil { + return err + } + // return if limitador is up-to-date + if rlptools.Equal(rateLimitIndex.ToRateLimits(), limitador.Spec.Limits) && !updated { logger.V(1).Info("limitador is up to date, skipping update") return nil } @@ -108,3 +113,72 @@ func (r *RateLimitPolicyReconciler) buildRateLimitIndex(ctx context.Context, rlp return rateLimitIndex, nil } + +func (r *RateLimitPolicyReconciler) reconcileLimitadorBackRef(limitador *limitadorv1alpha1.Limitador, policyKey client.Object) (updated bool, err error) { + policy := client.ObjectKeyFromObject(policyKey) + objAnnotations := common.ReadAnnotationsFromObject(limitador) + var refs []client.ObjectKey + + val, ok := objAnnotations[common.RateLimitPoliciesBackRefAnnotation] + if ok { + err := json.Unmarshal([]byte(val), &refs) + if err != nil { + return false, err + } + if common.ContainsObjectKey(refs, policy) { + r.Logger().V(1).Info("policy references in annotations", "policy", policy) + return false, nil + } + } + + refs = append(refs, policy) + serialized, err := json.Marshal(refs) + if err != nil { + return false, err + } + objAnnotations[common.RateLimitPoliciesBackRefAnnotation] = string(serialized) + limitador.SetAnnotations(objAnnotations) + return true, nil +} + +func (r *RateLimitPolicyReconciler) deleteLimitadorBackReference(ctx context.Context, policy client.Object) error { + policyKey := client.ObjectKeyFromObject(policy) + + limitadorList, err := r.listLimitadorByNamespace(ctx, policyKey.Namespace) + if err != nil { + return err + } + + updateList, err := rlptools.RemoveRLPLabelsFromLimitadorList(limitadorList, policyKey) + if err != nil { + return err + } + + err = r.updateLimitadorCRs(ctx, updateList) + if err != nil { + return err + } + + return nil +} + +func (r *RateLimitPolicyReconciler) updateLimitadorCRs(ctx context.Context, updateList limitadorv1alpha1.LimitadorList) error { + for index := range updateList.Items { + err := r.Client().Update(ctx, &updateList.Items[index]) + if err != nil { + return err + } + } + return nil +} + +func (r *RateLimitPolicyReconciler) listLimitadorByNamespace(ctx context.Context, namespace string) (limitadorv1alpha1.LimitadorList, error) { + limitadorList := limitadorv1alpha1.LimitadorList{} + listOptions := &client.ListOptions{Namespace: namespace} + + err := r.Client().List(ctx, &limitadorList, listOptions) + if err != nil { + return limitadorv1alpha1.LimitadorList{}, err + } + return limitadorList, nil +} diff --git a/pkg/rlptools/utils.go b/pkg/rlptools/utils.go index ae0163617..a840932b1 100644 --- a/pkg/rlptools/utils.go +++ b/pkg/rlptools/utils.go @@ -3,7 +3,9 @@ package rlptools import ( "crypto/sha256" "encoding/hex" + "encoding/json" "fmt" + "sigs.k8s.io/controller-runtime/pkg/client" "unicode" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" @@ -88,3 +90,40 @@ func rateToSeconds(rate kuadrantv1beta2.Rate) (maxValue int, seconds int) { return } + +func RemoveRLPLabelsFromLimitadorList(limitadorList limitadorv1alpha1.LimitadorList, policyKey client.ObjectKey) (limitadorv1alpha1.LimitadorList, error) { + var updateList limitadorv1alpha1.LimitadorList + for index := range limitadorList.Items { + limitador := limitadorList.Items[index] + objAnnotations := limitador.GetAnnotations() + val, ok := objAnnotations[common.RateLimitPoliciesBackRefAnnotation] + if !ok { + continue + } + + var refs []client.ObjectKey + err := json.Unmarshal([]byte(val), &refs) + if err != nil { + return updateList, err + } + refID := common.FindObjectKey(refs, policyKey) + if refID != len(refs) { + // remove index + refs = append(refs[:refID], refs[refID+1:]...) + + if len(refs) > 0 { + serialized, err := json.Marshal(refs) + if err != nil { + return updateList, err + } + objAnnotations[common.RateLimitPoliciesBackRefAnnotation] = string(serialized) + } else { + delete(objAnnotations, common.RateLimitPoliciesBackRefAnnotation) + } + + limitador.SetAnnotations(objAnnotations) + updateList.Items = append(updateList.Items, limitador) + } + } + return updateList, nil +} diff --git a/pkg/rlptools/utils_test.go b/pkg/rlptools/utils_test.go index 0b85796fa..f7a04b1ee 100644 --- a/pkg/rlptools/utils_test.go +++ b/pkg/rlptools/utils_test.go @@ -5,6 +5,7 @@ package rlptools import ( "reflect" "regexp" + "sigs.k8s.io/controller-runtime/pkg/client" "testing" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" @@ -363,3 +364,211 @@ func TestConvertRateIntoSeconds(t *testing.T) { }) } } + +func TestRemoveRLPLabelsFromLimitadorList(t *testing.T) { + policyKey := client.ObjectKey{Name: "test-RLP", Namespace: "test"} + + type args struct { + limitadorList limitadorv1alpha1.LimitadorList + policyKey client.ObjectKey + } + tests := []struct { + name string + args args + want limitadorv1alpha1.LimitadorList + wantErr bool + }{ + { + name: "LimitadorList is empty", + wantErr: false, + want: limitadorv1alpha1.LimitadorList{}, + args: args{ + limitadorList: limitadorv1alpha1.LimitadorList{}, + policyKey: policyKey, + }, + }, + { + name: "LimitadorList has one entry with no labels", + wantErr: false, + want: limitadorv1alpha1.LimitadorList{}, + args: args{ + limitadorList: limitadorv1alpha1.LimitadorList{ + Items: []limitadorv1alpha1.Limitador{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + Namespace: "test", + Annotations: map[string]string{"other": "label"}, + }, + }, + }, + }, + policyKey: policyKey, + }, + }, + { + name: "LimitadorList is has two entries, second entry with RLP labels", + wantErr: false, + want: limitadorv1alpha1.LimitadorList{ + Items: []limitadorv1alpha1.Limitador{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test2", + Namespace: "test", + Annotations: map[string]string{"other": "label"}, + }, + }, + }, + }, + args: args{ + limitadorList: limitadorv1alpha1.LimitadorList{ + Items: []limitadorv1alpha1.Limitador{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + Namespace: "test", + Annotations: map[string]string{"other": "label"}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test2", + Namespace: "test", + Annotations: map[string]string{ + "other": "label", + common.RateLimitPoliciesBackRefAnnotation: "[{\"Name\": \"test-RLP\", \"Namespace\": \"test\"}]", + }, + }, + }, + }, + }, + policyKey: policyKey, + }, + }, + { + name: "LimitadorList is has three entries, first and second with RLP labels", + wantErr: false, + want: limitadorv1alpha1.LimitadorList{ + Items: []limitadorv1alpha1.Limitador{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + Namespace: "test", + Annotations: map[string]string{"other": "label"}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test3", + Namespace: "test", + Annotations: map[string]string{"other": "label"}, + }, + }, + }, + }, + args: args{ + limitadorList: limitadorv1alpha1.LimitadorList{ + Items: []limitadorv1alpha1.Limitador{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + Namespace: "test", + Annotations: map[string]string{ + "other": "label", + common.RateLimitPoliciesBackRefAnnotation: "[{\"Name\": \"test-RLP\", \"Namespace\": \"test\"}]", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test2", + Namespace: "test", + Annotations: map[string]string{"other": "label"}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test3", + Namespace: "test", + Annotations: map[string]string{ + "other": "label", + common.RateLimitPoliciesBackRefAnnotation: "[{\"Name\": \"test-RLP\", \"Namespace\": \"test\"}]", + }, + }, + }, + }, + }, + policyKey: policyKey, + }, + }, + { + name: "LimitadorList, limitador CR had many RLP attached", + wantErr: false, + want: limitadorv1alpha1.LimitadorList{ + Items: []limitadorv1alpha1.Limitador{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + Namespace: "test", + Annotations: map[string]string{ + "other": "label", + common.RateLimitPoliciesBackRefAnnotation: "[{\"Namespace\":\"test\",\"Name\":\"other-RLP\"}]", + }, + }, + }, + }, + }, + args: args{ + limitadorList: limitadorv1alpha1.LimitadorList{ + Items: []limitadorv1alpha1.Limitador{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + Namespace: "test", + Annotations: map[string]string{ + "other": "label", + common.RateLimitPoliciesBackRefAnnotation: "[{\"Name\": \"other-RLP\", \"Namespace\": \"test\"}, {\"Name\": \"test-RLP\", \"Namespace\": \"test\"}]", + }, + }, + }, + }, + }, + policyKey: policyKey, + }, + }, + { + name: "LimitadorList, get unmarshal error", + wantErr: true, + want: limitadorv1alpha1.LimitadorList{}, + args: args{ + limitadorList: limitadorv1alpha1.LimitadorList{ + Items: []limitadorv1alpha1.Limitador{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test1", + Namespace: "test", + Annotations: map[string]string{ + "other": "label", + common.RateLimitPoliciesBackRefAnnotation: "[{Name: other-RLP, Namespace: test}]", + }, + }, + }, + }, + }, + policyKey: policyKey, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := RemoveRLPLabelsFromLimitadorList(tt.args.limitadorList, tt.args.policyKey) + if (err != nil) != tt.wantErr { + t.Errorf("RemoveRLPLabelsFromLimitadorList() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("RemoveRLPLabelsFromLimitadorList() got = %v, want %v", got, tt.want) + } + }) + } +} From 930dd943d46101b3593036ac2bf5281e6001f9df Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Wed, 23 Aug 2023 12:53:15 +0100 Subject: [PATCH 2/3] Revert change made to AuthConfig Adds a new watch to check for changes in the AuthConfig. If there are changes we reconcile back to the expected configuration as stated in the AuthPolicy. --- controllers/authconfig_eventmapper.go | 29 +++++++++++++++++++++++++++ controllers/authpolicy_auth_config.go | 5 +++++ controllers/authpolicy_controller.go | 7 +++++++ 3 files changed, 41 insertions(+) create mode 100644 controllers/authconfig_eventmapper.go diff --git a/controllers/authconfig_eventmapper.go b/controllers/authconfig_eventmapper.go new file mode 100644 index 000000000..f694b49af --- /dev/null +++ b/controllers/authconfig_eventmapper.go @@ -0,0 +1,29 @@ +package controllers + +import ( + "github.com/go-logr/logr" + "github.com/kuadrant/kuadrant-operator/pkg/common" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// AuthConfigEventMapper is an EventHandler that maps AuthConfig objects events to Policy events. +type AuthConfigEventMapper struct { + Logger logr.Logger +} + +func (m *AuthConfigEventMapper) MapToAuthPolicy(obj client.Object) []reconcile.Request { + return m.mapToPolicyRequest(obj, "authpolicy", common.AuthPoliciesBackRefAnnotation) +} + +func (m *AuthConfigEventMapper) mapToPolicyRequest(obj client.Object, policyKind string, policyBackRefAnnotationName string) []reconcile.Request { + policyRef, found := common.ReadAnnotationsFromObject(obj)[policyBackRefAnnotationName] + if !found { + return []reconcile.Request{} + } + + policyKey := common.NamespacedNameToObjectKey(policyRef, obj.GetNamespace()) + + m.Logger.V(1).Info("Processing object", "object", client.ObjectKeyFromObject(obj), policyKind, policyKey) + return []reconcile.Request{{NamespacedName: policyKey}} +} diff --git a/controllers/authpolicy_auth_config.go b/controllers/authpolicy_auth_config.go index 675769705..5022f2264 100644 --- a/controllers/authpolicy_auth_config.go +++ b/controllers/authpolicy_auth_config.go @@ -67,6 +67,8 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ap *api.AuthPolicy, targetNetwo return nil, err } + policyKey := client.ObjectKeyFromObject(ap) + return &authorinoapi.AuthConfig{ TypeMeta: metav1.TypeMeta{ Kind: "AuthConfig", @@ -75,6 +77,9 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ap *api.AuthPolicy, targetNetwo ObjectMeta: metav1.ObjectMeta{ Name: authConfigName(client.ObjectKeyFromObject(ap)), Namespace: ap.Namespace, + Annotations: map[string]string{ + common.AuthPoliciesBackRefAnnotation: policyKey.String(), + }, }, Spec: authorinoapi.AuthConfigSpec{ Hosts: hosts, diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index 8a23558ec..330db048a 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -3,6 +3,7 @@ package controllers import ( "context" "encoding/json" + authorinoapi "github.com/kuadrant/authorino/api/v1beta1" "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -199,6 +200,10 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { Logger: r.Logger().WithName("gatewayEventMapper"), } + authConfigEventMapper := &AuthConfigEventMapper{ + Logger: r.Logger().WithName("authConfigEventMapper"), + } + return ctrl.NewControllerManagedBy(mgr). For(&api.AuthPolicy{}). Watches( @@ -207,5 +212,7 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { ). Watches(&source.Kind{Type: &gatewayapiv1beta1.Gateway{}}, handler.EnqueueRequestsFromMapFunc(gatewayEventMapper.MapToAuthPolicy)). + Watches(&source.Kind{Type: &authorinoapi.AuthConfig{}}, + handler.EnqueueRequestsFromMapFunc(authConfigEventMapper.MapToAuthPolicy)). Complete(r) } From 1553face8156b3259934e7391376d9b2d36fd52b Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Wed, 30 Aug 2023 16:34:41 +0100 Subject: [PATCH 3/3] Refactor to have a `Existing` and `Desired` style of reconcile. --- controllers/authpolicy_controller.go | 1 + controllers/limitador_eventmapper.go | 2 + controllers/ratelimitpolicy_controller.go | 1 + controllers/ratelimitpolicy_limits.go | 49 +++++++---------------- pkg/common/annotations.go | 32 +++++++++++++++ pkg/common/back_reference.go | 26 ++++++++++++ pkg/rlptools/utils.go | 3 +- pkg/rlptools/utils_test.go | 3 +- 8 files changed, 81 insertions(+), 36 deletions(-) create mode 100644 pkg/common/annotations.go create mode 100644 pkg/common/back_reference.go diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index 330db048a..3390fdf30 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -3,6 +3,7 @@ package controllers import ( "context" "encoding/json" + authorinoapi "github.com/kuadrant/authorino/api/v1beta1" "github.com/go-logr/logr" diff --git a/controllers/limitador_eventmapper.go b/controllers/limitador_eventmapper.go index 3682bfef3..f6309414a 100644 --- a/controllers/limitador_eventmapper.go +++ b/controllers/limitador_eventmapper.go @@ -2,6 +2,7 @@ package controllers import ( "encoding/json" + "github.com/go-logr/logr" "github.com/kuadrant/kuadrant-operator/pkg/common" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" @@ -37,6 +38,7 @@ func (m LimitadorEventMapper) MapToRateLimitPolicy(obj client.Object) []reconcil requests = append(requests, reconcile.Request{ NamespacedName: ref, }) + break } return requests } diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index ac3bdbeca..83232e413 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -19,6 +19,7 @@ package controllers import ( "context" "encoding/json" + "github.com/go-logr/logr" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" apierrors "k8s.io/apimachinery/pkg/api/errors" diff --git a/controllers/ratelimitpolicy_limits.go b/controllers/ratelimitpolicy_limits.go index 5de454432..8680ed714 100644 --- a/controllers/ratelimitpolicy_limits.go +++ b/controllers/ratelimitpolicy_limits.go @@ -2,7 +2,6 @@ package controllers import ( "context" - "encoding/json" "github.com/go-logr/logr" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" @@ -43,6 +42,11 @@ func (r *RateLimitPolicyReconciler) reconcileLimitador(ctx context.Context, rlp return err } + rplsBackref, err := common.BuildBackRefs(rlpRefs) + if err != nil { + return err + } + // get the current limitador cr for the kuadrant instance so we can compare if it needs to be updated logger.V(1).Info("get kuadrant namespace") var kuadrantNamespace string @@ -69,17 +73,17 @@ func (r *RateLimitPolicyReconciler) reconcileLimitador(ctx context.Context, rlp return err } - updated, err := r.reconcileLimitadorBackRef(limitador, rlp) - if err != nil { - return err - } + update := false + update = update || !common.AnnotationEqual(common.RateLimitPoliciesBackRefAnnotation, rplsBackref, limitador.Annotations) + update = update || !rlptools.Equal(rateLimitIndex.ToRateLimits(), limitador.Spec.Limits) // return if limitador is up-to-date - if rlptools.Equal(rateLimitIndex.ToRateLimits(), limitador.Spec.Limits) && !updated { + if !update { logger.V(1).Info("limitador is up to date, skipping update") return nil } // update limitador + limitador.SetAnnotations(common.AddAnnotation(common.RateLimitPoliciesBackRefAnnotation, rplsBackref, limitador.Annotations)) limitador.Spec.Limits = rateLimitIndex.ToRateLimits() err = r.UpdateResource(ctx, limitador) logger.V(1).Info("update limitador", "limitador", limitadorKey, "err", err) @@ -114,41 +118,18 @@ func (r *RateLimitPolicyReconciler) buildRateLimitIndex(ctx context.Context, rlp return rateLimitIndex, nil } -func (r *RateLimitPolicyReconciler) reconcileLimitadorBackRef(limitador *limitadorv1alpha1.Limitador, policyKey client.Object) (updated bool, err error) { - policy := client.ObjectKeyFromObject(policyKey) - objAnnotations := common.ReadAnnotationsFromObject(limitador) - var refs []client.ObjectKey - - val, ok := objAnnotations[common.RateLimitPoliciesBackRefAnnotation] - if ok { - err := json.Unmarshal([]byte(val), &refs) - if err != nil { - return false, err - } - if common.ContainsObjectKey(refs, policy) { - r.Logger().V(1).Info("policy references in annotations", "policy", policy) - return false, nil - } - } - - refs = append(refs, policy) - serialized, err := json.Marshal(refs) +func (r *RateLimitPolicyReconciler) deleteLimitadorBackReference(ctx context.Context, policy client.Object) error { + kuadrantNamespace, err := common.GetKuadrantNamespace(policy) if err != nil { - return false, err + return err } - objAnnotations[common.RateLimitPoliciesBackRefAnnotation] = string(serialized) - limitador.SetAnnotations(objAnnotations) - return true, nil -} -func (r *RateLimitPolicyReconciler) deleteLimitadorBackReference(ctx context.Context, policy client.Object) error { - policyKey := client.ObjectKeyFromObject(policy) - - limitadorList, err := r.listLimitadorByNamespace(ctx, policyKey.Namespace) + limitadorList, err := r.listLimitadorByNamespace(ctx, kuadrantNamespace) if err != nil { return err } + policyKey := client.ObjectKeyFromObject(policy) updateList, err := rlptools.RemoveRLPLabelsFromLimitadorList(limitadorList, policyKey) if err != nil { return err diff --git a/pkg/common/annotations.go b/pkg/common/annotations.go new file mode 100644 index 000000000..8182eb339 --- /dev/null +++ b/pkg/common/annotations.go @@ -0,0 +1,32 @@ +package common + +func AddAnnotation(annotation string, value string, annotations map[string]string) map[string]string { + _, ok := annotations[annotation] + if !ok && len(value) == 0 { + return annotations + } + + if len(value) == 0 { + delete(annotations, annotation) + return annotations + } + + if annotations == nil { + annotations = map[string]string{} + } + annotations[annotation] = value + return annotations +} + +func AnnotationEqual(annotation string, value string, annotations map[string]string) bool { + val, ok := annotations[annotation] + if !ok && len(annotation) == 0 { + return true + } + + if val == value { + return true + } + + return false +} diff --git a/pkg/common/back_reference.go b/pkg/common/back_reference.go new file mode 100644 index 000000000..89b861069 --- /dev/null +++ b/pkg/common/back_reference.go @@ -0,0 +1,26 @@ +package common + +import ( + "encoding/json" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func BuildBackRefs(refs []client.ObjectKey) (string, error) { + if len(refs) == 0 { + return "", nil + } + + var uniqueKeys []client.ObjectKey + + for _, v := range refs { + if !Contains(uniqueKeys, v) { + uniqueKeys = append(uniqueKeys, v) + } + } + + serialized, err := json.Marshal(uniqueKeys) + if err != nil { + return "", err + } + return string(serialized), nil +} diff --git a/pkg/rlptools/utils.go b/pkg/rlptools/utils.go index a840932b1..13fea9d62 100644 --- a/pkg/rlptools/utils.go +++ b/pkg/rlptools/utils.go @@ -5,9 +5,10 @@ import ( "encoding/hex" "encoding/json" "fmt" - "sigs.k8s.io/controller-runtime/pkg/client" "unicode" + "sigs.k8s.io/controller-runtime/pkg/client" + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" diff --git a/pkg/rlptools/utils_test.go b/pkg/rlptools/utils_test.go index f7a04b1ee..013a32dc8 100644 --- a/pkg/rlptools/utils_test.go +++ b/pkg/rlptools/utils_test.go @@ -5,9 +5,10 @@ package rlptools import ( "reflect" "regexp" - "sigs.k8s.io/controller-runtime/pkg/client" "testing" + "sigs.k8s.io/controller-runtime/pkg/client" + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"