diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go index a49ad1ef2..7ef4ddab6 100644 --- a/controllers/rate_limiting_wasmplugin_controller.go +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -23,6 +23,7 @@ import ( "sort" "github.com/go-logr/logr" + "github.com/google/uuid" istioextensionsv1alpha1 "istio.io/api/extensions/v1alpha1" istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -61,7 +62,7 @@ type RateLimitingWASMPluginReconciler struct { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.10.0/pkg/reconcile func (r *RateLimitingWASMPluginReconciler) Reconcile(eventCtx context.Context, req ctrl.Request) (ctrl.Result, error) { - logger := r.Logger().WithValues("Gateway", req.NamespacedName) + logger := r.Logger().WithValues("Gateway", req.NamespacedName, "request id", uuid.NewString()) logger.Info("Reconciling rate limiting WASMPlugin") ctx := logr.NewContext(eventCtx, logger) @@ -174,7 +175,7 @@ func (r *RateLimitingWASMPluginReconciler) wasmPluginConfig(ctx context.Context, for _, policy := range rateLimitPolicies { rlp := policy.(*kuadrantv1beta2.RateLimitPolicy) - wasmRLP, err := r.wasmRateLimitPolicy(ctx, t, rlp, gw, rateLimitPolicies) + wasmRLP, err := r.wasmRateLimitPolicy(ctx, t, rlp, gw) if err != nil { return nil, err } @@ -219,7 +220,7 @@ func (r *RateLimitingWASMPluginReconciler) topologyIndexesFromGateway(ctx contex policies := utils.Map(rlpList.Items, func(p kuadrantv1beta2.RateLimitPolicy) kuadrantgatewayapi.Policy { return &p }) - t, err := kuadrantgatewayapi.NewTopology( + topology, err := kuadrantgatewayapi.NewTopology( kuadrantgatewayapi.WithGateways([]*gatewayapiv1.Gateway{gw}), kuadrantgatewayapi.WithRoutes(utils.Map(routeList.Items, ptr.To[gatewayapiv1.HTTPRoute])), kuadrantgatewayapi.WithPolicies(policies), @@ -229,12 +230,15 @@ func (r *RateLimitingWASMPluginReconciler) topologyIndexesFromGateway(ctx contex return nil, err } - return kuadrantgatewayapi.NewTopologyIndexes(t), nil -} + overriddenTopology, err := rlptools.ApplyOverrides(topology, gw) + if err != nil { + return nil, err + } -func (r *RateLimitingWASMPluginReconciler) wasmRateLimitPolicy(ctx context.Context, t *kuadrantgatewayapi.TopologyIndexes, rlp *kuadrantv1beta2.RateLimitPolicy, gw *gatewayapiv1.Gateway, affectedPolices []kuadrantgatewayapi.Policy) (*wasm.RateLimitPolicy, error) { - logger, _ := logr.FromContext(ctx) + return kuadrantgatewayapi.NewTopologyIndexes(overriddenTopology), nil +} +func (r *RateLimitingWASMPluginReconciler) wasmRateLimitPolicy(ctx context.Context, t *kuadrantgatewayapi.TopologyIndexes, rlp *kuadrantv1beta2.RateLimitPolicy, gw *gatewayapiv1.Gateway) (*wasm.RateLimitPolicy, error) { route, err := r.routeFromRLP(ctx, t, rlp, gw) if err != nil { return nil, err @@ -267,22 +271,6 @@ func (r *RateLimitingWASMPluginReconciler) wasmRateLimitPolicy(ctx context.Conte routeWithEffectiveHostnames := route.DeepCopy() routeWithEffectiveHostnames.Spec.Hostnames = hostnames - // Policy limits may be overridden by a gateway policy for route policies - if kuadrantgatewayapi.IsTargetRefHTTPRoute(rlp.GetTargetRef()) { - filteredPolicies := utils.Filter(affectedPolices, func(p kuadrantgatewayapi.Policy) bool { - return kuadrantgatewayapi.IsTargetRefGateway(p.GetTargetRef()) && p.GetUID() != rlp.GetUID() - }) - - for _, policy := range filteredPolicies { - p := policy.(*kuadrantv1beta2.RateLimitPolicy) - if p.Spec.Overrides != nil { - rlp.Spec.CommonSpec().Limits = p.Spec.Overrides.Limits - logger.V(1).Info("applying overrides from parent policy", "parentPolicy", client.ObjectKeyFromObject(p)) - break - } - } - } - rules := rlptools.WasmRules(rlp, routeWithEffectiveHostnames) if len(rules) == 0 { // no need to add the policy if there are no rules; a rlp can return no rules if all its limits fail to match any route rule @@ -324,7 +312,6 @@ func (r *RateLimitingWASMPluginReconciler) routeFromRLP(ctx context.Context, t * for idx := range untargetedRoutes { untargetedRules = append(untargetedRules, untargetedRoutes[idx].Spec.Rules...) } - gwHostnamesTmp := kuadrantgatewayapi.TargetHostnames(gw) gwHostnames := utils.Map(gwHostnamesTmp, func(str string) gatewayapiv1.Hostname { return gatewayapiv1.Hostname(str) }) route = &gatewayapiv1.HTTPRoute{ diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index ed9848a01..4e0cfc2e0 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -22,6 +22,7 @@ import ( "fmt" "github.com/go-logr/logr" + "github.com/google/uuid" apierrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -60,7 +61,7 @@ type RateLimitPolicyReconciler struct { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.10.0/pkg/reconcile func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Request) (ctrl.Result, error) { - logger := r.Logger().WithValues("RateLimitPolicy", req.NamespacedName) + logger := r.Logger().WithValues("RateLimitPolicy", req.NamespacedName, "request id", uuid.NewString()) logger.Info("Reconciling RateLimitPolicy") ctx := logr.NewContext(eventCtx, logger) diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index 7810a25ec..5b8cd1796 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/controllers/ratelimitpolicy_controller_test.go @@ -88,12 +88,15 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { } } - limitadorContainsLimit := func(ctx context.Context, limit limitadorv1alpha1.RateLimit) func(g Gomega) { + limitadorContainsLimit := func(ctx context.Context, limits ...limitadorv1alpha1.RateLimit) func(g Gomega) { return func(g Gomega) { limitadorKey := client.ObjectKey{Name: common.LimitadorName, Namespace: kuadrantInstallationNS} existingLimitador := &limitadorv1alpha1.Limitador{} g.Expect(k8sClient.Get(ctx, limitadorKey, existingLimitador)).To(Succeed()) - g.Expect(existingLimitador.Spec.Limits).To(ContainElements(limit)) + for i := range limits { + limit := limits[i] + g.Expect(existingLimitador.Spec.Limits).To(ContainElements(limit)) + } } } @@ -157,7 +160,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { MaxValue: 1, Seconds: 3 * 60, Namespace: rlptools.LimitsNamespaceFromRLP(rlp), - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(rlpKey, "l1"))}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(rlp), })) @@ -243,7 +246,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { MaxValue: 1, Seconds: 3 * 60, Namespace: rlptools.LimitsNamespaceFromRLP(rlp), - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(rlpKey, "l1"))}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(rlp), })) @@ -293,7 +296,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { MaxValue: 1, Seconds: 3 * 60, Namespace: rlptools.LimitsNamespaceFromRLP(rlp), - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(rlpKey, "l1"))}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(rlp), })) @@ -368,7 +371,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { MaxValue: 10, Seconds: 5, Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(routeRLPKey, "l1"))}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(routeRLP), })).WithContext(ctx).Should(Succeed()) @@ -483,7 +486,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { MaxValue: 1, Seconds: 180, Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(routeRLPKey, "l1"))}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(routeRLP), })).WithContext(ctx).Should(Succeed()) @@ -505,7 +508,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { MaxValue: 10, Seconds: 5, Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), - Conditions: []string{`limit.route__8a84e406 == "1"`}, + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(routeRLPKey, "route"))}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(routeRLP), })).WithContext(ctx).Should(Succeed()) @@ -540,7 +543,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { MaxValue: 1, Seconds: 180, Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(routeRLPKey, "l1"))}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(routeRLP), })).WithContext(ctx).Should(Succeed()) @@ -579,7 +582,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { MaxValue: 10, Seconds: 5, Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), - Conditions: []string{`limit.route__8a84e406 == "1"`}, + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(routeRLPKey, "route"))}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(routeRLP), })).WithContext(ctx).Should(Succeed()) @@ -602,7 +605,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { MaxValue: 1, Seconds: 180, Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(routeRLPKey, "l1"))}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(routeRLP), })).WithContext(ctx).Should(Succeed()) @@ -628,7 +631,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { MaxValue: 1, Seconds: 180, Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(routeRLPKey, "l1"))}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(routeRLP), })).WithContext(ctx).Should(Succeed()) @@ -651,7 +654,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { MaxValue: 10, Seconds: 5, Namespace: rlptools.LimitsNamespaceFromRLP(routeRLP), - Conditions: []string{`limit.route__8a84e406 == "1"`}, + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(routeRLPKey, "route"))}, Variables: []string{}, Name: rlptools.LimitsNameFromRLP(routeRLP), })).WithContext(ctx).Should(Succeed()) @@ -1208,6 +1211,148 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() { Eventually(assertPolicyIsAcceptedAndEnforced(ctx, rlpAKey)).WithContext(ctx).Should(BeTrue()) }, testTimeOut) }) + + Context("HTTPRoute with multiple gateway parents", func() { + var ( + gatewayAName = "gateway-a" + gatewayBName = "gateway-b" + targetedRouteName = "targeted-route" + untargetedRouteName = "untargeted-route" + + gatewayA *gatewayapiv1.Gateway + gatewayB *gatewayapiv1.Gateway + targetedRoute *gatewayapiv1.HTTPRoute + untargetedRoute *gatewayapiv1.HTTPRoute + ) + + BeforeEach(func(ctx SpecContext) { + gatewayA = tests.BuildBasicGateway(gatewayAName, testNamespace, func(g *gatewayapiv1.Gateway) { + g.Spec.Listeners[0].Hostname = ptr.To(gatewayapiv1.Hostname("*.a.example.com")) + }) + err := k8sClient.Create(ctx, gatewayA) + Expect(err).ToNot(HaveOccurred()) + Eventually(tests.GatewayIsReady(ctx, testClient(), gatewayA)).WithContext(ctx).Should(BeTrue()) + + gatewayB = tests.BuildBasicGateway(gatewayBName, testNamespace, func(g *gatewayapiv1.Gateway) { + g.Spec.Listeners[0].Hostname = ptr.To(gatewayapiv1.Hostname("*.b.example.com")) + }) + err = k8sClient.Create(ctx, gatewayB) + Expect(err).ToNot(HaveOccurred()) + Eventually(tests.GatewayIsReady(ctx, testClient(), gatewayB)).WithContext(ctx).Should(BeTrue()) + + gatewayParentsFunc := func(r *gatewayapiv1.HTTPRoute) { + r.Spec.ParentRefs = []gatewayapiv1.ParentReference{ + {Name: gatewayapiv1.ObjectName(gatewayAName)}, + {Name: gatewayapiv1.ObjectName(gatewayBName)}, + } + } + + targetedRoute = tests.BuildBasicHttpRoute(targetedRouteName, gatewayAName, testNamespace, []string{"targeted.a.example.com", "targeted.b.example.com"}, gatewayParentsFunc) + err = k8sClient.Create(ctx, targetedRoute) + Expect(err).ToNot(HaveOccurred()) + Eventually(tests.RouteIsAccepted(ctx, testClient(), client.ObjectKeyFromObject(targetedRoute))).WithContext(ctx).Should(BeTrue()) + + untargetedRoute = tests.BuildBasicHttpRoute(untargetedRouteName, gatewayAName, testNamespace, []string{"untargeted.a.example.com", "untargeted.b.example.com"}, gatewayParentsFunc) + err = k8sClient.Create(ctx, untargetedRoute) + Expect(err).ToNot(HaveOccurred()) + Eventually(tests.RouteIsAccepted(ctx, testClient(), client.ObjectKeyFromObject(untargetedRoute))).WithContext(ctx).Should(BeTrue()) + }) + + It("It defines route policy limits with gateway policy overrides", func(ctx SpecContext) { + rlpGatewayA := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { + policy.ObjectMeta.Name = gatewayAName + policy.Spec.TargetRef.Kind = "Gateway" + policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gatewayAName) + policy.Spec.Defaults = nil + policy.Spec.Overrides = &kuadrantv1beta2.RateLimitPolicyCommonSpec{ + Limits: map[string]kuadrantv1beta2.Limit{ + "gw-a-1000rps": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1000, Duration: 1, Unit: "second", + }, + }, + }, + }, + } + }) + err := k8sClient.Create(ctx, rlpGatewayA) + Expect(err).ToNot(HaveOccurred()) + + rlpGatewayB := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { + policy.ObjectMeta.Name = gatewayBName + policy.Spec.TargetRef.Kind = "Gateway" + policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gatewayBName) + policy.Spec.Defaults = nil + policy.Spec.Overrides = &kuadrantv1beta2.RateLimitPolicyCommonSpec{ + Limits: map[string]kuadrantv1beta2.Limit{ + "gw-b-100rps": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 100, Duration: 1, Unit: "second", + }, + }, + }, + }, + } + }) + err = k8sClient.Create(ctx, rlpGatewayB) + Expect(err).ToNot(HaveOccurred()) + + rlpTargetedRoute := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { + policy.ObjectMeta.Name = targetedRouteName + policy.Spec.TargetRef.Kind = "HTTPRoute" + policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(targetedRouteName) + policy.Spec.CommonSpec().Limits = map[string]kuadrantv1beta2.Limit{ + "route-10rps": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 10, Duration: 1, Unit: "second", + }, + }, + }, + } + }) + err = k8sClient.Create(ctx, rlpTargetedRoute) + Expect(err).ToNot(HaveOccurred()) + + Eventually(limitadorContainsLimit( + ctx, + limitadorv1alpha1.RateLimit{ + MaxValue: 1000, + Seconds: 1, + Namespace: rlptools.LimitsNamespaceFromRLP(rlpTargetedRoute), + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(client.ObjectKeyFromObject(rlpTargetedRoute), "gw-a-1000rps"))}, + Variables: []string{}, + Name: rlptools.LimitsNameFromRLP(rlpTargetedRoute), + }, + limitadorv1alpha1.RateLimit{ + MaxValue: 100, + Seconds: 1, + Namespace: rlptools.LimitsNamespaceFromRLP(rlpTargetedRoute), + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(client.ObjectKeyFromObject(rlpTargetedRoute), "gw-b-100rps"))}, + Variables: []string{}, + Name: rlptools.LimitsNameFromRLP(rlpTargetedRoute), + }, + limitadorv1alpha1.RateLimit{ // FIXME(@guicassolato): we need to create one limit definition per gateway × route combination, not one per gateway × policy combination + MaxValue: 1000, + Seconds: 1, + Namespace: rlptools.LimitsNamespaceFromRLP(rlpGatewayA), + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(client.ObjectKeyFromObject(rlpGatewayA), "gw-a-1000rps"))}, + Variables: []string{}, + Name: rlptools.LimitsNameFromRLP(rlpGatewayA), + }, + limitadorv1alpha1.RateLimit{ + MaxValue: 100, + Seconds: 1, + Namespace: rlptools.LimitsNamespaceFromRLP(rlpGatewayB), + Conditions: []string{fmt.Sprintf(`%s == "1"`, rlptools.LimitNameToLimitadorIdentifier(client.ObjectKeyFromObject(rlpGatewayB), "gw-b-100rps"))}, + Variables: []string{}, + Name: rlptools.LimitsNameFromRLP(rlpGatewayB), + }, + )).WithContext(ctx).Should(Succeed()) + }) + }) }) var _ = Describe("RateLimitPolicy CEL Validations", func() { diff --git a/controllers/ratelimitpolicy_enforced_status_controller.go b/controllers/ratelimitpolicy_enforced_status_controller.go index bf5ee458e..a8552b81b 100644 --- a/controllers/ratelimitpolicy_enforced_status_controller.go +++ b/controllers/ratelimitpolicy_enforced_status_controller.go @@ -9,9 +9,11 @@ import ( "github.com/go-logr/logr" "github.com/google/uuid" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + "github.com/samber/lo" 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" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -48,50 +50,113 @@ func (r *RateLimitPolicyEnforcedStatusReconciler) Reconcile(eventCtx context.Con return ctrl.Result{}, err } - t, err := BuildTopology(ctx, r.Client(), gw, (&kuadrantv1beta2.RateLimitPolicy{}).Kind(), &kuadrantv1beta2.RateLimitPolicyList{}) + topology, err := r.buildTopology(ctx, gw) if err != nil { return ctrl.Result{}, err } - policies := t.PoliciesFromGateway(gw) - unTargetedRoutes := len(t.GetUntargetedRoutes(gw)) + indexes := kuadrantgatewayapi.NewTopologyIndexes(topology) + policies := indexes.PoliciesFromGateway(gw) + numRoutes := len(topology.Routes()) + numUntargetedRoutes := len(indexes.GetUntargetedRoutes(gw)) sort.Sort(kuadrantgatewayapi.PolicyByTargetRefKindAndCreationTimeStamp(policies)) + // for each policy: + // if the policy is a gateway policy: + // and no route exists (numRoutes == 0) → set the Enforced condition of the gateway policy to 'false' (unknown) + // and the gateway contains routes (numRoutes > 0) + // and the gateway policy contains overrides → set the Enforced condition of the gateway policy to 'true' + // and the gateway policy contains defaults: + // and no routes have an attached policy (numUntargetedRoutes == numRoutes) → set the Enforced condition of the gateway policy to 'true' + // and some routes have attached policy (numUntargetedRoutes < numRoutes && numUntargetedRoutes > 1) → set the Enforced condition of the gateway policy to 'true' (partially enforced) + // and all routes have attached policy (numUntargetedRoutes == 0) → set the Enforced condition of the gateway policy to 'false' (overridden) + // if the policy is a route policy: + // and the route has no gateway parent (numGatewayParents == 0) → set the Enforced condition of the route policy to 'false' (unknown) + // and the route has gateway parents (numGatewayParents > 0) + // and all gateway parents of the route have gateway policies with overrides (numGatewayParentsWithOverrides == numGatewayParents) → set the Enforced condition of the route policy to 'false' (overridden) + // and some gateway parents of the route have gateway policies with overrides (numGatewayParentsWithOverrides < numGatewayParents && numGatewayParentsWithOverrides > 1) → set the Enforced condition of the route policy to 'true' (partially enforced) + // and no gateway parent of the route has gateway policies with overrides (numGatewayParentsWithOverrides == 0) → set the Enforced condition of the route policy to 'true' + for i := range policies { policy := policies[i] - p := policy.(*kuadrantv1beta2.RateLimitPolicy) - conditions := p.GetStatus().GetConditions() + rlpKey := client.ObjectKeyFromObject(policy) + rlp := policy.(*kuadrantv1beta2.RateLimitPolicy) + conditions := rlp.GetStatus().GetConditions() - // Skip policies if accepted condition is false + // skip policy if accepted condition is false if meta.IsStatusConditionFalse(policy.GetStatus().GetConditions(), string(gatewayapiv1alpha2.PolicyConditionAccepted)) { continue } - // Policy has been accepted - // Ensure no error on underlying subresource (i.e. Limitador) - if cond := r.hasErrCondOnSubResource(ctx, logger, p); cond != nil { - if err := r.setCondition(ctx, logger, p, &conditions, *cond); err != nil { + // ensure no error on underlying subresource (i.e. Limitador) + if condition := r.hasErrCondOnSubResource(ctx, rlp); condition != nil { + if err := r.setCondition(ctx, rlp, &conditions, *condition); err != nil { return ctrl.Result{}, err } continue } - if kuadrantgatewayapi.IsTargetRefGateway(p.GetTargetRef()) { - if p.Spec.Overrides != nil { - if err := r.setConditionForGWPolicyWithOverrides(ctx, logger, p, conditions, policies, unTargetedRoutes); err != nil { - return ctrl.Result{}, err + var condition *metav1.Condition + + if kuadrantgatewayapi.IsTargetRefGateway(rlp.GetTargetRef()) { // gateway policy + if numRoutes == 0 { + condition = kuadrant.EnforcedCondition(rlp, kuadrant.NewErrUnknown(rlp.Kind(), errors.New("no free routes to enforce policy")), true) // unknown + } else { + if rlp.Spec.Overrides != nil { + condition = kuadrant.EnforcedCondition(rlp, nil, true) // fully enforced + } else { + if numUntargetedRoutes == numRoutes { + condition = kuadrant.EnforcedCondition(rlp, nil, true) // fully enforced + } else if numUntargetedRoutes > 0 { + condition = kuadrant.EnforcedCondition(rlp, nil, false) // partially enforced + } else { + otherPolicies := lo.FilterMap(policies, func(p kuadrantgatewayapi.Policy, _ int) (client.ObjectKey, bool) { + key := client.ObjectKeyFromObject(p) + return key, key != rlpKey + }) + condition = kuadrant.EnforcedCondition(rlp, kuadrant.NewErrOverridden(rlp.Kind(), otherPolicies), true) // overridden + } } - break } - if err := r.setConditionForGWPolicyWithDefaults(ctx, logger, p, conditions, policies, unTargetedRoutes); err != nil { - return ctrl.Result{}, err + } else { // route policy + route := indexes.GetPolicyHTTPRoute(rlp) + gatewayParents := lo.FilterMap(kuadrantgatewayapi.GetRouteAcceptedGatewayParentKeys(route), func(parentKey client.ObjectKey, _ int) (*gatewayapiv1.Gateway, bool) { + g, found := utils.Find(topology.Gateways(), func(g kuadrantgatewayapi.GatewayNode) bool { return client.ObjectKeyFromObject(g.Gateway) == parentKey }) + if !found { + return nil, false + } + return g.Gateway, true + }) + numGatewayParents := len(gatewayParents) + if numGatewayParents == 0 { + condition = kuadrant.EnforcedCondition(rlp, kuadrant.NewErrUnknown(rlp.Kind(), errors.New("the targeted route has not been accepted by any gateway parent")), true) // unknown + } else { + var gatewayParentOverridePolicies []kuadrantgatewayapi.Policy + gatewayParentsWithOverrides := utils.Filter(gatewayParents, func(gatewayParent *gatewayapiv1.Gateway) bool { + _, found := utils.Find(indexes.PoliciesFromGateway(gatewayParent), func(p kuadrantgatewayapi.Policy) bool { + rlp := p.(*kuadrantv1beta2.RateLimitPolicy) + if kuadrantgatewayapi.IsTargetRefGateway(p.GetTargetRef()) && rlp != nil && rlp.Spec.Overrides != nil { + gatewayParentOverridePolicies = append(gatewayParentOverridePolicies, p) + return true + } + return false + }) + return found + }) + numGatewayParentsWithOverrides := len(gatewayParentsWithOverrides) + if numGatewayParentsWithOverrides == numGatewayParents { + sort.Sort(kuadrantgatewayapi.PolicyByTargetRefKindAndCreationTimeStamp(gatewayParentOverridePolicies)) + condition = kuadrant.EnforcedCondition(rlp, kuadrant.NewErrOverridden(rlp.Kind(), utils.Map(gatewayParentOverridePolicies, func(p kuadrantgatewayapi.Policy) client.ObjectKey { return client.ObjectKeyFromObject(p) })), true) // overridden + } else if numGatewayParentsWithOverrides > 0 { + condition = kuadrant.EnforcedCondition(rlp, nil, false) // partially enforced + } else { + condition = kuadrant.EnforcedCondition(rlp, nil, true) // fully enforced + } } - continue } - // Route Policy - if err := r.setCondition(ctx, logger, p, &conditions, *kuadrant.EnforcedCondition(p, nil, true)); err != nil { + if err := r.setCondition(ctx, rlp, &conditions, *condition); err != nil { return ctrl.Result{}, err } } @@ -100,67 +165,49 @@ func (r *RateLimitPolicyEnforcedStatusReconciler) Reconcile(eventCtx context.Con return ctrl.Result{}, nil } -func (r *RateLimitPolicyEnforcedStatusReconciler) setConditionForGWPolicyWithOverrides(ctx context.Context, logger logr.Logger, p *kuadrantv1beta2.RateLimitPolicy, conditions []metav1.Condition, policies []kuadrantgatewayapi.Policy, unTargetedRoutes int) error { - // Only have this policy and no free routes - if len(policies) == 1 && unTargetedRoutes == 0 { - if err := r.setCondition(ctx, logger, p, &conditions, *kuadrant.EnforcedCondition(p, kuadrant.NewErrUnknown(p.Kind(), errors.New("no free routes to enforce policy")), true)); err != nil { - return err - } +func (r *RateLimitPolicyEnforcedStatusReconciler) buildTopology(ctx context.Context, gw *gatewayapiv1.Gateway) (*kuadrantgatewayapi.Topology, error) { + logger, err := logr.FromContext(ctx) + if err != nil { + return nil, err } - // Has free routes or is overriding policies - // Set Enforced true condition for this GW policy - if err := r.setCondition(ctx, logger, p, &conditions, *kuadrant.EnforcedCondition(p, nil, true)); err != nil { - return err + gatewayList := &gatewayapiv1.GatewayList{} + err = r.Client().List(ctx, gatewayList) + logger.V(1).Info("list gateways", "#gateways", len(gatewayList.Items), "err", err) + if err != nil { + return nil, err } - // Update the rest of the policies as overridden - affectedPolices := utils.Filter(policies, func(ap kuadrantgatewayapi.Policy) bool { - return p != ap && ap.GetDeletionTimestamp() == nil - }) - - for i := range affectedPolices { - af := affectedPolices[i] - afp := af.(*kuadrantv1beta2.RateLimitPolicy) - afConditions := afp.GetStatus().GetConditions() + routeList := &gatewayapiv1.HTTPRouteList{} + // Get all the routes having the gateway as parent + err = r.Client().List(ctx, routeList, client.MatchingFields{HTTPRouteGatewayParentField: client.ObjectKeyFromObject(gw).String()}) + logger.V(1).Info("list routes by gateway", "#routes", len(routeList.Items), "err", err) + if err != nil { + return nil, err + } - if err := r.setCondition(ctx, logger, afp, &afConditions, *kuadrant.EnforcedCondition(afp, kuadrant.NewErrOverridden(afp.Kind(), []client.ObjectKey{client.ObjectKeyFromObject(p)}), true)); err != nil { - return err - } + policyList := &kuadrantv1beta2.RateLimitPolicyList{} + err = r.Client().List(ctx, policyList) + logger.V(1).Info("list rate limit policies", "#policies", len(policyList.Items), "err", err) + if err != nil { + return nil, err } - return nil + return kuadrantgatewayapi.NewTopology( + kuadrantgatewayapi.WithGateways(utils.Map(gatewayList.Items, ptr.To[gatewayapiv1.Gateway])), + kuadrantgatewayapi.WithRoutes(utils.Map(routeList.Items, ptr.To[gatewayapiv1.HTTPRoute])), + kuadrantgatewayapi.WithPolicies(utils.Map(policyList.Items, func(p kuadrantv1beta2.RateLimitPolicy) kuadrantgatewayapi.Policy { return &p })), + kuadrantgatewayapi.WithLogger(logger), + ) } -func (r *RateLimitPolicyEnforcedStatusReconciler) setConditionForGWPolicyWithDefaults(ctx context.Context, logger logr.Logger, p *kuadrantv1beta2.RateLimitPolicy, conditions []metav1.Condition, policies []kuadrantgatewayapi.Policy, unTargetedRoutes int) error { - // GW Policy defaults is defined - // Only have this policy or no free routes -> nothing to enforce policy - if len(policies) == 1 && unTargetedRoutes == 0 { - if err := r.setCondition(ctx, logger, p, &conditions, *kuadrant.EnforcedCondition(p, kuadrant.NewErrUnknown(p.Kind(), errors.New("no free routes to enforce policy")), true)); err != nil { - return err - } - } else if len(policies) > 1 && unTargetedRoutes == 0 { - // GW policy defaults are overridden by child policies - affectedPolices := utils.Filter(policies, func(ap kuadrantgatewayapi.Policy) bool { - return p != ap && ap.GetDeletionTimestamp() == nil - }) - - if err := r.setCondition(ctx, logger, p, &conditions, *kuadrant.EnforcedCondition(p, kuadrant.NewErrOverridden(p.Kind(), utils.Map(affectedPolices, func(ap kuadrantgatewayapi.Policy) client.ObjectKey { - return client.ObjectKeyFromObject(ap) - })), true)); err != nil { - return err - } - } else { - // Is enforcing default policy on a free route - if err := r.setCondition(ctx, logger, p, &conditions, *kuadrant.EnforcedCondition(p, nil, true)); err != nil { - return err - } +func (r *RateLimitPolicyEnforcedStatusReconciler) hasErrCondOnSubResource(ctx context.Context, p *kuadrantv1beta2.RateLimitPolicy) *metav1.Condition { + logger, err := logr.FromContext(ctx) + logger.WithName("hasErrCondOnSubResource") + if err != nil { + logger = r.Logger() } - return nil -} - -func (r *RateLimitPolicyEnforcedStatusReconciler) hasErrCondOnSubResource(ctx context.Context, logger logr.Logger, p *kuadrantv1beta2.RateLimitPolicy) *metav1.Condition { limitador, err := GetLimitador(ctx, r.Client(), p) if err != nil { logger.V(1).Error(err, "failed to get limitador") @@ -175,7 +222,13 @@ func (r *RateLimitPolicyEnforcedStatusReconciler) hasErrCondOnSubResource(ctx co return nil } -func (r *RateLimitPolicyEnforcedStatusReconciler) setCondition(ctx context.Context, logger logr.Logger, p *kuadrantv1beta2.RateLimitPolicy, conditions *[]metav1.Condition, cond metav1.Condition) error { +func (r *RateLimitPolicyEnforcedStatusReconciler) setCondition(ctx context.Context, p *kuadrantv1beta2.RateLimitPolicy, conditions *[]metav1.Condition, cond metav1.Condition) error { + logger, err := logr.FromContext(ctx) + logger.WithName("setCondition") + if err != nil { + logger = r.Logger() + } + idx := utils.Index(*conditions, func(c metav1.Condition) bool { return c.Type == cond.Type && c.Status == cond.Status && c.Reason == cond.Reason && c.Message == cond.Message }) diff --git a/controllers/ratelimitpolicy_limits.go b/controllers/ratelimitpolicy_limits.go index ce1f9eae1..64dd1078e 100644 --- a/controllers/ratelimitpolicy_limits.go +++ b/controllers/ratelimitpolicy_limits.go @@ -2,11 +2,13 @@ package controllers import ( "context" + "fmt" "slices" "sort" "github.com/go-logr/logr" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + "github.com/samber/lo" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -20,34 +22,38 @@ import ( ) func (r *RateLimitPolicyReconciler) reconcileLimits(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy) error { - rlpRefs, err := r.TargetRefReconciler.GetAllGatewayPolicyRefs(ctx, rlp) + policies, err := r.getPolicies(ctx) if err != nil { return err } - return r.reconcileLimitador(ctx, rlp, append(rlpRefs, client.ObjectKeyFromObject(rlp))) + topology, err := r.buildTopology(ctx, policies) + if err != nil { + return err + } + return r.reconcileLimitador(ctx, rlp, topology) } func (r *RateLimitPolicyReconciler) deleteLimits(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy) error { - rlpRefs, err := r.TargetRefReconciler.GetAllGatewayPolicyRefs(ctx, rlp) + policies, err := r.getPolicies(ctx) if err != nil { return err } - - rlpRefsWithoutRLP := utils.Filter(rlpRefs, func(rlpRef client.ObjectKey) bool { - return rlpRef.Name != rlp.Name || rlpRef.Namespace != rlp.Namespace + policiesWithoutRLP := utils.Filter(policies, func(policy kuadrantgatewayapi.Policy) bool { + return client.ObjectKeyFromObject(policy) != client.ObjectKeyFromObject(rlp) }) - - return r.reconcileLimitador(ctx, rlp, rlpRefsWithoutRLP) + topology, err := r.buildTopology(ctx, policiesWithoutRLP) + if err != nil { + return err + } + return r.reconcileLimitador(ctx, rlp, topology) } -func (r *RateLimitPolicyReconciler) reconcileLimitador(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, rlpRefs []client.ObjectKey) error { +func (r *RateLimitPolicyReconciler) reconcileLimitador(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, topology *kuadrantgatewayapi.Topology) error { logger, _ := logr.FromContext(ctx) - logger = logger.WithName("reconcileLimitador").WithValues("rlp refs", utils.Map(rlpRefs, func(ref client.ObjectKey) string { return ref.String() })) + logger = logger.WithName("reconcileLimitador") + + rateLimitIndex := r.buildRateLimitIndex(ctx, topology) - rateLimitIndex, err := r.buildRateLimitIndex(ctx, 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 limitador, err := GetLimitador(ctx, r.Client(), rlp) if err != nil { @@ -101,83 +107,22 @@ func GetLimitador(ctx context.Context, k8sclient client.Client, rlp *kuadrantv1b return limitador, nil } -func (r *RateLimitPolicyReconciler) buildRateLimitIndex(ctx context.Context, rlpRefs []client.ObjectKey) (*rlptools.RateLimitIndex, error) { +func (r *RateLimitPolicyReconciler) getPolicies(ctx context.Context) ([]kuadrantgatewayapi.Policy, error) { logger, _ := logr.FromContext(ctx) - logger = logger.WithName("buildRateLimitIndex").WithValues("ratelimitpolicies", rlpRefs) - t, err := r.generateTopology(ctx) + rlpList := &kuadrantv1beta2.RateLimitPolicyList{} + err := r.Client().List(ctx, rlpList) + logger.V(1).Info("topology: list rate limit policies", "#RLPS", len(rlpList.Items), "err", err) if err != nil { return nil, err } - rateLimitIndex := rlptools.NewRateLimitIndex() - - for _, rlpKey := range rlpRefs { - if _, ok := rateLimitIndex.Get(rlpKey); ok { - continue - } - - rlp := &kuadrantv1beta2.RateLimitPolicy{} - err := r.Client().Get(ctx, rlpKey, rlp) - logger.V(1).Info("get rlp", "ratelimitpolicy", rlpKey, "err", err) - if err != nil { - return nil, err - } - - // If rlp is targeting a route, limits may be overridden by other policies - if kuadrantgatewayapi.IsTargetRefHTTPRoute(rlp.GetTargetRef()) { - r.applyOverrides(ctx, rlp, t) - } - - rateLimitIndex.Set(rlpKey, rlptools.LimitadorRateLimitsFromRLP(rlp)) - } - - return rateLimitIndex, nil -} - -func (r *RateLimitPolicyReconciler) applyOverrides(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, t *kuadrantgatewayapi.Topology) { - logger, _ := logr.FromContext(ctx) - logger = logger.WithName("applyOverrides") - - // Retrieve affected policies - affectedPolicies := r.getAffectedPolicies(rlp, t) - - // Filter out current policy from affected policies - // Only gateway RLPs can affect Route RLP and these must not be marked for deletion - affectedPolicies = utils.Filter(affectedPolicies, func(policy kuadrantgatewayapi.Policy) bool { - return kuadrantgatewayapi.IsTargetRefGateway(policy.GetTargetRef()) && policy.GetUID() != rlp.GetUID() && policy.GetDeletionTimestamp() == nil - }) - - sort.Sort(kuadrantgatewayapi.PolicyByTargetRefKindAndCreationTimeStamp(affectedPolicies)) - - for _, policy := range affectedPolicies { - p := policy.(*kuadrantv1beta2.RateLimitPolicy) - if p.Spec.Overrides != nil { - rlp.Spec.CommonSpec().Limits = p.Spec.Overrides.Limits - logger.V(1).Info("applying overrides from parent policy", "parentPolicy", client.ObjectKeyFromObject(p)) - break - } - } -} - -func (r *RateLimitPolicyReconciler) getAffectedPolicies(rlp *kuadrantv1beta2.RateLimitPolicy, t *kuadrantgatewayapi.Topology) []kuadrantgatewayapi.Policy { - topologyIndexes := kuadrantgatewayapi.NewTopologyIndexes(t) - var affectedPolicies []kuadrantgatewayapi.Policy - - for _, gw := range t.Gateways() { - policyList := topologyIndexes.PoliciesFromGateway(gw.Gateway) - - if slices.Contains(utils.Map(policyList, func(p kuadrantgatewayapi.Policy) client.ObjectKey { - return client.ObjectKeyFromObject(p) - }), client.ObjectKeyFromObject(rlp)) { - affectedPolicies = append(affectedPolicies, policyList...) - } - } + policies := utils.Map(rlpList.Items, func(p kuadrantv1beta2.RateLimitPolicy) kuadrantgatewayapi.Policy { return &p }) - return affectedPolicies + return policies, nil } -func (r *RateLimitPolicyReconciler) generateTopology(ctx context.Context) (*kuadrantgatewayapi.Topology, error) { +func (r *RateLimitPolicyReconciler) buildTopology(ctx context.Context, policies []kuadrantgatewayapi.Policy) (*kuadrantgatewayapi.Topology, error) { logger, _ := logr.FromContext(ctx) gwList := &gatewayapiv1.GatewayList{} @@ -194,15 +139,6 @@ func (r *RateLimitPolicyReconciler) generateTopology(ctx context.Context) (*kuad return nil, err } - rlpList := &kuadrantv1beta2.RateLimitPolicyList{} - err = r.Client().List(ctx, rlpList) - logger.V(1).Info("topology: list rate limit policies", "#RLPS", len(rlpList.Items), "err", err) - if err != nil { - return nil, err - } - - policies := utils.Map(rlpList.Items, func(p kuadrantv1beta2.RateLimitPolicy) kuadrantgatewayapi.Policy { return &p }) - return kuadrantgatewayapi.NewTopology( kuadrantgatewayapi.WithGateways(utils.Map(gwList.Items, ptr.To[gatewayapiv1.Gateway])), kuadrantgatewayapi.WithRoutes(utils.Map(routeList.Items, ptr.To[gatewayapiv1.HTTPRoute])), @@ -210,3 +146,51 @@ func (r *RateLimitPolicyReconciler) generateTopology(ctx context.Context) (*kuad kuadrantgatewayapi.WithLogger(logger), ) } + +func (r *RateLimitPolicyReconciler) buildRateLimitIndex(ctx context.Context, topology *kuadrantgatewayapi.Topology) *rlptools.RateLimitIndex { + logger, _ := logr.FromContext(ctx) + logger = logger.WithName("buildRateLimitIndex") + + gateways := lo.KeyBy(topology.Gateways(), func(gateway kuadrantgatewayapi.GatewayNode) string { + return client.ObjectKeyFromObject(gateway.Gateway).String() + }) + + // sort the gateways for deterministic output and consistent comparison against existing objects + gatewayNames := lo.Keys(gateways) + slices.Sort(gatewayNames) + + rateLimitIndex := rlptools.NewRateLimitIndex() + + for _, gatewayName := range gatewayNames { + gateway := gateways[gatewayName].Gateway + topologyWithOverrides, err := rlptools.ApplyOverrides(topology, gateway) + if err != nil { + logger.Error(err, "failed to apply overrides") + return nil + } + + // sort the policies for deterministic output and consistent comparison against existing objects + indexes := kuadrantgatewayapi.NewTopologyIndexes(topologyWithOverrides) + policies := indexes.PoliciesFromGateway(gateway) + sort.Sort(kuadrantgatewayapi.PolicyByTargetRefKindAndCreationTimeStamp(policies)) + + logger.V(1).Info("new rate limit index", "gateway", client.ObjectKeyFromObject(gateway), "policies", lo.Map(policies, func(p kuadrantgatewayapi.Policy, _ int) string { return client.ObjectKeyFromObject(p).String() })) + + for _, policy := range policies { + rlpKey := client.ObjectKeyFromObject(policy) + gatewayKey := client.ObjectKeyFromObject(gateway) + key := rlptools.RateLimitIndexKey{ + RateLimitPolicyKey: rlpKey, + GatewayKey: gatewayKey, + } + if _, ok := rateLimitIndex.Get(key); ok { // should never happen + logger.Error(fmt.Errorf("unexpected duplicate rate limit policy key found"), "failed do add rate limit policy to index", "RateLimitPolicy", rlpKey.String(), "Gateway", gatewayKey) + continue + } + rlp := policy.(*kuadrantv1beta2.RateLimitPolicy) + rateLimitIndex.Set(key, rlptools.LimitadorRateLimitsFromRLP(rlp)) + } + } + + return rateLimitIndex +} diff --git a/controllers/target_status_controller.go b/controllers/target_status_controller.go index 58625f77b..0265040f1 100644 --- a/controllers/target_status_controller.go +++ b/controllers/target_status_controller.go @@ -105,7 +105,7 @@ func (r *TargetStatusReconciler) reconcileResourcesForPolicyKind(parentCtx conte policyKind := policy.GetObjectKind().GroupVersionKind().Kind ctx := logr.NewContext(parentCtx, logger.WithValues("kind", policyKind)) - topology, err := BuildTopology(ctx, r.Client(), gw, policyKind, listPolicyKind) + topology, err := r.buildTopology(ctx, gw, policyKind, listPolicyKind) if err != nil { return err } @@ -195,7 +195,7 @@ func (r *TargetStatusReconciler) reconcileResourcesForPolicyKind(parentCtx conte return errs } -func BuildTopology(ctx context.Context, ks8sClient client.Client, gw *gatewayapiv1.Gateway, policyKind string, listPolicyKind client.ObjectList) (*kuadrantgatewayapi.TopologyIndexes, error) { +func (r *TargetStatusReconciler) buildTopology(ctx context.Context, gw *gatewayapiv1.Gateway, policyKind string, listPolicyKind client.ObjectList) (*kuadrantgatewayapi.TopologyIndexes, error) { logger, err := logr.FromContext(ctx) if err != nil { return nil, err @@ -203,13 +203,13 @@ func BuildTopology(ctx context.Context, ks8sClient client.Client, gw *gatewayapi routeList := &gatewayapiv1.HTTPRouteList{} // Get all the routes having the gateway as parent - err = ks8sClient.List(ctx, routeList, client.MatchingFields{HTTPRouteGatewayParentField: client.ObjectKeyFromObject(gw).String()}) + err = r.Client().List(ctx, routeList, client.MatchingFields{HTTPRouteGatewayParentField: client.ObjectKeyFromObject(gw).String()}) logger.V(1).Info("list routes by gateway", "#routes", len(routeList.Items), "err", err) if err != nil { return nil, err } - policies, err := GetPoliciesByKind(ctx, ks8sClient, policyKind, listPolicyKind) + policies, err := r.getPoliciesByKind(ctx, policyKind, listPolicyKind) if err != nil { return nil, err } @@ -227,12 +227,12 @@ func BuildTopology(ctx context.Context, ks8sClient client.Client, gw *gatewayapi return kuadrantgatewayapi.NewTopologyIndexes(t), nil } -func GetPoliciesByKind(ctx context.Context, ks8sClient client.Client, policyKind string, listKind client.ObjectList) ([]kuadrantgatewayapi.Policy, error) { +func (r *TargetStatusReconciler) getPoliciesByKind(ctx context.Context, policyKind string, listKind client.ObjectList) ([]kuadrantgatewayapi.Policy, error) { logger, _ := logr.FromContext(ctx) logger = logger.WithValues("kind", policyKind) // Get all policies of the given kind - err := ks8sClient.List(ctx, listKind) + err := r.Client().List(ctx, listKind) policyList, ok := listKind.(kuadrant.PolicyList) if !ok { return nil, fmt.Errorf("%T is not a kuadrant.PolicyList", listKind) diff --git a/go.mod b/go.mod index a97cedd74..75c9d5232 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/martinlindhe/base36 v1.1.1 github.com/onsi/ginkgo/v2 v2.13.2 github.com/onsi/gomega v1.30.0 + github.com/samber/lo v1.39.0 go.uber.org/zap v1.26.0 golang.org/x/net v0.23.0 google.golang.org/protobuf v1.33.0 diff --git a/go.sum b/go.sum index 99ee103c8..c0dd7cc0a 100644 --- a/go.sum +++ b/go.sum @@ -392,6 +392,8 @@ github.com/rubenv/sql-migrate v1.5.2 h1:bMDqOnrJVV/6JQgQ/MxOpU+AdO8uzYYA/TxFUBzF github.com/rubenv/sql-migrate v1.5.2/go.mod h1:H38GW8Vqf8F0Su5XignRyaRcbXbJunSWxs+kmzlg0Is= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/samber/lo v1.39.0 h1:4gTz1wUhNYLhFSKl6O+8peW0v2F4BCY034GRpU9WnuA= +github.com/samber/lo v1.39.0/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA= github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ= github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= github.com/shopspring/decimal v1.2.0/go.mod h1:DKyhrW/HYNuLGql+MJL6WCR6knT2jwCFRcu2hWCYk4o= diff --git a/pkg/library/kuadrant/apimachinery_status_conditions.go b/pkg/library/kuadrant/apimachinery_status_conditions.go index fb0822ac5..eb4a42da4 100644 --- a/pkg/library/kuadrant/apimachinery_status_conditions.go +++ b/pkg/library/kuadrant/apimachinery_status_conditions.go @@ -104,10 +104,10 @@ func AcceptedCondition(p Policy, err error) *metav1.Condition { } // EnforcedCondition returns an enforced conditions with common reasons for a kuadrant policy -func EnforcedCondition(policy Policy, err PolicyError, allSubresourcesReady bool) *metav1.Condition { +func EnforcedCondition(policy Policy, err PolicyError, fully bool) *metav1.Condition { // Enforced message := fmt.Sprintf("%s has been successfully enforced", policy.Kind()) - if !allSubresourcesReady { + if !fully { message = fmt.Sprintf("%s has been partially enforced", policy.Kind()) } cond := &metav1.Condition{ diff --git a/pkg/library/reconcilers/target_ref_reconciler.go b/pkg/library/reconcilers/target_ref_reconciler.go index b8ec58aca..3e35dee99 100644 --- a/pkg/library/reconcilers/target_ref_reconciler.go +++ b/pkg/library/reconcilers/target_ref_reconciler.go @@ -19,10 +19,8 @@ package reconcilers import ( "context" "fmt" - "sort" "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/api/meta" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -184,44 +182,6 @@ func (r *TargetRefReconciler) DeleteTargetBackReference(ctx context.Context, tar return nil } -// GetAllGatewayPolicyRefs returns the policy refs of a given policy kind from all gateways managed by kuadrant. -// The gateway objects are handled in order of creation to mitigate the risk of non-idenpotent reconciliations based on -// this list of policy refs; nevertheless, the actual order of returned policy refs depends on the order the policy refs -// appear in the annotations of the gateways. -// Only gateways with status programmed are considered. -func (r *TargetRefReconciler) GetAllGatewayPolicyRefs(ctx context.Context, policyRefsConfig kuadrant.Referrer) ([]client.ObjectKey, error) { - var uniquePolicyRefs map[string]struct{} - var policyRefs []client.ObjectKey - - gwList := &gatewayapiv1.GatewayList{} - if err := r.Client.List(ctx, gwList); err != nil { - return nil, err - } - - // sort the gateways by creation timestamp to mitigate the risk of non-idenpotent reconciliations - var gateways kuadrant.GatewayWrapperList - for i := range gwList.Items { - gateway := gwList.Items[i] - // skip gateways that are not managed by kuadrant or that are not ready - if !kuadrant.IsKuadrantManaged(&gateway) || meta.IsStatusConditionFalse(gateway.Status.Conditions, string(gatewayapiv1.GatewayConditionProgrammed)) { - continue - } - gateways = append(gateways, kuadrant.GatewayWrapper{Gateway: &gateway, Referrer: policyRefsConfig}) - } - sort.Sort(gateways) - - for _, gw := range gateways { - for _, policyRef := range gw.PolicyRefs() { - if _, ok := uniquePolicyRefs[policyRef.String()]; ok { - continue - } - policyRefs = append(policyRefs, policyRef) - } - } - - return policyRefs, nil -} - // ReconcileGatewayPolicyReferences updates the annotations in the Gateway resources that list to all the policies // that directly or indirectly target the gateway, based upon a pre-computed gateway diff object func (r *TargetRefReconciler) ReconcileGatewayPolicyReferences(ctx context.Context, policy client.Object, gwDiffObj *GatewayDiffs) error { diff --git a/pkg/rlptools/overrides.go b/pkg/rlptools/overrides.go new file mode 100644 index 000000000..ef0eeeffd --- /dev/null +++ b/pkg/rlptools/overrides.go @@ -0,0 +1,57 @@ +package rlptools + +import ( + "slices" + + "github.com/samber/lo" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + + kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" + kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" +) + +// ApplyOverrides applies the overrides defined in the RateLimitPolicies attached to the gateway policies for a given +// gateway, and returns a new topology with all policies overridden as applicable. +func ApplyOverrides(topology *kuadrantgatewayapi.Topology, gateway *gatewayapiv1.Gateway) (*kuadrantgatewayapi.Topology, error) { + gatewayNode, ok := lo.Find(topology.Gateways(), func(g kuadrantgatewayapi.GatewayNode) bool { + return g.ObjectKey() == client.ObjectKeyFromObject(gateway) + }) + if !ok || len(gatewayNode.AttachedPolicies()) == 0 { + return topology, nil + } + + overridePolicies := lo.FilterMap(gatewayNode.AttachedPolicies(), func(policy kuadrantgatewayapi.Policy, _ int) (*kuadrantv1beta2.RateLimitPolicy, bool) { + rlp, ok := policy.(*kuadrantv1beta2.RateLimitPolicy) + if !ok || rlp.Spec.Overrides == nil { + return nil, false + } + return rlp, true + }) + + if len(overridePolicies) == 0 { + return topology, nil + } + + overriddenPolicies := lo.Map(overridePolicies, func(p *kuadrantv1beta2.RateLimitPolicy, _ int) kuadrantgatewayapi.Policy { return p }) + + for _, route := range topology.Routes() { + if !slices.Contains(kuadrantgatewayapi.GetRouteAcceptedGatewayParentKeys(route.HTTPRoute), client.ObjectKeyFromObject(gateway)) { + overriddenPolicies = append(overriddenPolicies, route.AttachedPolicies()...) + continue + } + + for _, policy := range route.AttachedPolicies() { + overriddenPolicy := policy.DeepCopyObject().(*kuadrantv1beta2.RateLimitPolicy) + overriddenPolicy.Spec.CommonSpec().Limits = overridePolicies[0].Spec.Overrides.Limits + overriddenPolicies = append(overriddenPolicies, overriddenPolicy) + } + } + + return kuadrantgatewayapi.NewTopology( + kuadrantgatewayapi.WithGateways(lo.Map(topology.Gateways(), func(g kuadrantgatewayapi.GatewayNode, _ int) *gatewayapiv1.Gateway { return g.Gateway })), + kuadrantgatewayapi.WithRoutes(lo.Map(topology.Routes(), func(r kuadrantgatewayapi.RouteNode, _ int) *gatewayapiv1.HTTPRoute { return r.HTTPRoute })), + kuadrantgatewayapi.WithPolicies(overriddenPolicies), + kuadrantgatewayapi.WithLogger(topology.Logger), + ) +} diff --git a/pkg/rlptools/rate_limit_index.go b/pkg/rlptools/rate_limit_index.go index 1cb84a4c2..c391981c2 100644 --- a/pkg/rlptools/rate_limit_index.go +++ b/pkg/rlptools/rate_limit_index.go @@ -7,12 +7,15 @@ import ( "github.com/elliotchance/orderedmap/v2" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" - "sigs.k8s.io/controller-runtime/pkg/client" + "k8s.io/apimachinery/pkg/types" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) -type RateLimitIndexKey = client.ObjectKey +type RateLimitIndexKey struct { + RateLimitPolicyKey types.NamespacedName + GatewayKey types.NamespacedName +} // NewRateLimitIndex builds an index to manage sets of rate limits, organized by key func NewRateLimitIndex() *RateLimitIndex { diff --git a/pkg/rlptools/rate_limit_index_test.go b/pkg/rlptools/rate_limit_index_test.go index e418f03fd..a5a8d0519 100644 --- a/pkg/rlptools/rate_limit_index_test.go +++ b/pkg/rlptools/rate_limit_index_test.go @@ -14,7 +14,8 @@ func TestRateLimitIndexSet(t *testing.T) { t.Run("index rate limits to a key", func(subT *testing.T) { index := NewRateLimitIndex() - index.Set(client.ObjectKey{Name: "rlp-1", Namespace: "ns"}, []limitadorv1alpha1.RateLimit{ + key := RateLimitIndexKey{RateLimitPolicyKey: client.ObjectKey{Name: "rlp-1", Namespace: "ns"}} + index.Set(key, []limitadorv1alpha1.RateLimit{ {Namespace: "ns/rlp-1", MaxValue: 10, Seconds: 1}, {Namespace: "ns/rlp-1", MaxValue: 100, Seconds: 60}, {Namespace: "ns/rlp-1", MaxValue: 1000, Seconds: 1}, @@ -30,17 +31,17 @@ func TestRateLimitIndexSet(t *testing.T) { t.Run("index rate limits to different keys", func(subT *testing.T) { index := NewRateLimitIndex() - index.Set(client.ObjectKey{Name: "rlp-1", Namespace: "ns"}, []limitadorv1alpha1.RateLimit{ + index.Set(RateLimitIndexKey{RateLimitPolicyKey: client.ObjectKey{Name: "rlp-1", Namespace: "ns"}}, []limitadorv1alpha1.RateLimit{ {Namespace: "ns/rlp-1", MaxValue: 10, Seconds: 1}, {Namespace: "ns/rlp-1", MaxValue: 100, Seconds: 60}, {Namespace: "ns/rlp-1", MaxValue: 1000, Seconds: 1}, }) - index.Set(client.ObjectKey{Name: "rlp-2", Namespace: "ns"}, []limitadorv1alpha1.RateLimit{ + index.Set(RateLimitIndexKey{RateLimitPolicyKey: client.ObjectKey{Name: "rlp-2", Namespace: "ns"}}, []limitadorv1alpha1.RateLimit{ {Namespace: "ns/rlp-2", MaxValue: 50, Seconds: 1}, }) - key := client.ObjectKey{Name: "rlp-1", Namespace: "ns"} + key := RateLimitIndexKey{RateLimitPolicyKey: client.ObjectKey{Name: "rlp-1", Namespace: "ns"}} rateLimits, found := index.Get(key) if !found { subT.Fatal("expected rate limits to be indexed to key but none found:", key) @@ -50,7 +51,7 @@ func TestRateLimitIndexSet(t *testing.T) { subT.Fatal("expected:", expectedCount, "rate limits for key", key, ", returned:", len(rateLimits)) } - key = client.ObjectKey{Name: "rlp-2", Namespace: "ns"} + key = RateLimitIndexKey{RateLimitPolicyKey: client.ObjectKey{Name: "rlp-2", Namespace: "ns"}} rateLimits, found = index.Get(key) if !found { subT.Fatal("expected rate limits to be indexed to key but none found:", key) @@ -70,13 +71,13 @@ func TestRateLimitIndexSet(t *testing.T) { t.Run("reset rate limits for an existing key", func(subT *testing.T) { index := NewRateLimitIndex() - index.Set(client.ObjectKey{Name: "rlp-1", Namespace: "ns"}, []limitadorv1alpha1.RateLimit{ + index.Set(RateLimitIndexKey{RateLimitPolicyKey: client.ObjectKey{Name: "rlp-1", Namespace: "ns"}}, []limitadorv1alpha1.RateLimit{ {Namespace: "ns/rlp-1", MaxValue: 10, Seconds: 1}, {Namespace: "ns/rlp-1", MaxValue: 100, Seconds: 60}, {Namespace: "ns/rlp-1", MaxValue: 1000, Seconds: 1}, }) - index.Set(client.ObjectKey{Name: "rlp-1", Namespace: "ns"}, []limitadorv1alpha1.RateLimit{ + index.Set(RateLimitIndexKey{RateLimitPolicyKey: client.ObjectKey{Name: "rlp-1", Namespace: "ns"}}, []limitadorv1alpha1.RateLimit{ {Namespace: "ns/rlp-1", MaxValue: 500, Seconds: 3600}, }) @@ -93,7 +94,7 @@ func TestRateLimitIndexSet(t *testing.T) { t.Run("add an empty list of limits if a noop", func(subT *testing.T) { idx := NewRateLimitIndex() - idx.Set(client.ObjectKey{Name: "gwA", Namespace: "nsA"}, []limitadorv1alpha1.RateLimit{}) + idx.Set(RateLimitIndexKey{GatewayKey: client.ObjectKey{Name: "gwA", Namespace: "nsA"}}, []limitadorv1alpha1.RateLimit{}) aggregatedRateLimits := idx.ToRateLimits() if len(aggregatedRateLimits) != 0 { @@ -104,7 +105,7 @@ func TestRateLimitIndexSet(t *testing.T) { t.Run("add nil list of limits if a noop", func(subT *testing.T) { idx := NewRateLimitIndex() - idx.Set(client.ObjectKey{Name: "gwA", Namespace: "nsA"}, []limitadorv1alpha1.RateLimit{}) + idx.Set(RateLimitIndexKey{GatewayKey: client.ObjectKey{Name: "gwA", Namespace: "nsA"}}, []limitadorv1alpha1.RateLimit{}) aggregatedRateLimits := idx.ToRateLimits() if len(aggregatedRateLimits) != 0 { diff --git a/pkg/rlptools/utils.go b/pkg/rlptools/utils.go index d6033c0db..e743010ac 100644 --- a/pkg/rlptools/utils.go +++ b/pkg/rlptools/utils.go @@ -7,6 +7,8 @@ import ( "unicode" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" @@ -16,7 +18,7 @@ const ( LimitadorRateLimitIdentifierPrefix = "limit." ) -func LimitNameToLimitadorIdentifier(uniqueLimitName string) string { +func LimitNameToLimitadorIdentifier(rlpKey types.NamespacedName, uniqueLimitName string) string { identifier := LimitadorRateLimitIdentifierPrefix // sanitize chars that are not allowed in limitador identifiers @@ -29,7 +31,7 @@ func LimitNameToLimitadorIdentifier(uniqueLimitName string) string { } // to avoid breaking the uniqueness of the limit name after sanitization, we add a hash of the original name - hash := sha256.Sum256([]byte(uniqueLimitName)) + hash := sha256.Sum256([]byte(fmt.Sprintf("%s/%s", rlpKey.String(), uniqueLimitName))) identifier += "__" + hex.EncodeToString(hash[:4]) return identifier @@ -39,10 +41,11 @@ func LimitNameToLimitadorIdentifier(uniqueLimitName string) string { // objects func LimitadorRateLimitsFromRLP(rlp *kuadrantv1beta2.RateLimitPolicy) []limitadorv1alpha1.RateLimit { limitsNamespace := LimitsNamespaceFromRLP(rlp) + rlpKey := client.ObjectKeyFromObject(rlp) rateLimits := make([]limitadorv1alpha1.RateLimit, 0) for limitKey, limit := range rlp.Spec.CommonSpec().Limits { - limitIdentifier := LimitNameToLimitadorIdentifier(limitKey) + limitIdentifier := LimitNameToLimitadorIdentifier(rlpKey, limitKey) for _, rate := range limit.Rates { maxValue, seconds := rateToSeconds(rate) rateLimits = append(rateLimits, limitadorv1alpha1.RateLimit{ diff --git a/pkg/rlptools/utils_test.go b/pkg/rlptools/utils_test.go index 8b93e1e49..cc0635e64 100644 --- a/pkg/rlptools/utils_test.go +++ b/pkg/rlptools/utils_test.go @@ -9,6 +9,7 @@ import ( limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/library/utils" @@ -145,39 +146,51 @@ func testRLP_1Limit_1Rate_1Counter(ns, name string) *kuadrantv1beta2.RateLimitPo func TestLimitNameToLimitadorIdentifier(t *testing.T) { testCases := []struct { - name string - input string - expected *regexp.Regexp + name string + rlpKey types.NamespacedName + uniqueLimitName string + expected *regexp.Regexp }{ { - name: "prepends the limitador limit identifier prefix", - input: "foo", - expected: regexp.MustCompile(`^limit\.foo.+`), + name: "prepends the limitador limit identifier prefix", + rlpKey: types.NamespacedName{Namespace: "testNS", Name: "rlpA"}, + uniqueLimitName: "foo", + expected: regexp.MustCompile(`^limit\.foo.+`), + }, + { + name: "sanitizes invalid chars", + rlpKey: types.NamespacedName{Namespace: "testNS", Name: "rlpA"}, + uniqueLimitName: "my/limit-0", + expected: regexp.MustCompile(`^limit\.my_limit_0.+$`), }, { - name: "sanitizes invalid chars", - input: "my/limit-0", - expected: regexp.MustCompile(`^limit\.my_limit_0.+$`), + name: "sanitizes the dot char (.) even though it is a valid char in limitador identifiers", + rlpKey: types.NamespacedName{Namespace: "testNS", Name: "rlpA"}, + uniqueLimitName: "my.limit", + expected: regexp.MustCompile(`^limit\.my_limit.+$`), }, { - name: "sanitizes the dot char (.) even though it is a valid char in limitador identifiers", - input: "my.limit", - expected: regexp.MustCompile(`^limit\.my_limit.+$`), + name: "appends a hash of the original name to avoid breaking uniqueness", + rlpKey: types.NamespacedName{Namespace: "testNS", Name: "rlpA"}, + uniqueLimitName: "foo", + expected: regexp.MustCompile(`^.+__1da6e70a$`), }, { - name: "appends a hash of the original name to avoid breaking uniqueness", - input: "foo", - expected: regexp.MustCompile(`^.+__2c26b46b$`), + name: "different rlp keys result in different identifiers", + rlpKey: types.NamespacedName{Namespace: "testNS", Name: "rlpB"}, + uniqueLimitName: "foo", + expected: regexp.MustCompile(`^.+__2c1520b6$`), }, { - name: "empty string", - input: "", - expected: regexp.MustCompile(`^limit.__e3b0c442$`), + name: "empty string", + rlpKey: types.NamespacedName{Namespace: "testNS", Name: "rlpA"}, + uniqueLimitName: "", + expected: regexp.MustCompile(`^limit.__6d5e49dc$`), }, } for _, tc := range testCases { t.Run(tc.name, func(subT *testing.T) { - identifier := LimitNameToLimitadorIdentifier(tc.input) + identifier := LimitNameToLimitadorIdentifier(tc.rlpKey, tc.uniqueLimitName) if !tc.expected.MatchString(identifier) { subT.Errorf("identifier does not match, expected(%s), got (%s)", tc.expected, identifier) } @@ -199,7 +212,7 @@ func TestLimitadorRateLimitsFromRLP(t *testing.T) { Namespace: "testNS/rlpA", MaxValue: 5, Seconds: 10, - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{`limit.l1__65f19ee8 == "1"`}, Variables: []string{}, Name: "testNS/rlpA", }, @@ -213,7 +226,7 @@ func TestLimitadorRateLimitsFromRLP(t *testing.T) { Namespace: "testNS/rlpA", MaxValue: 5, Seconds: 10, - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{`limit.l1__65f19ee8 == "1"`}, Variables: []string{}, Name: "testNS/rlpA", }, @@ -221,7 +234,7 @@ func TestLimitadorRateLimitsFromRLP(t *testing.T) { Namespace: "testNS/rlpA", MaxValue: 3, Seconds: 3600, - Conditions: []string{`limit.l2__8a1cee43 == "1"`}, + Conditions: []string{`limit.l2__3e871d60 == "1"`}, Variables: []string{}, Name: "testNS/rlpA", }, @@ -235,7 +248,7 @@ func TestLimitadorRateLimitsFromRLP(t *testing.T) { Namespace: "testNS/rlpA", MaxValue: 5, Seconds: 10, - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{`limit.l1__65f19ee8 == "1"`}, Variables: []string{}, Name: "testNS/rlpA", }, @@ -243,7 +256,7 @@ func TestLimitadorRateLimitsFromRLP(t *testing.T) { Namespace: "testNS/rlpA", MaxValue: 3, Seconds: 60, - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{`limit.l1__65f19ee8 == "1"`}, Variables: []string{}, Name: "testNS/rlpA", }, @@ -257,7 +270,7 @@ func TestLimitadorRateLimitsFromRLP(t *testing.T) { Namespace: "testNS/rlpA", MaxValue: 5, Seconds: 10, - Conditions: []string{`limit.l1__2804bad6 == "1"`}, + Conditions: []string{`limit.l1__65f19ee8 == "1"`}, Variables: []string{"request.path"}, Name: "testNS/rlpA", }, diff --git a/pkg/rlptools/wasm_utils.go b/pkg/rlptools/wasm_utils.go index f502095e2..8d4b71490 100644 --- a/pkg/rlptools/wasm_utils.go +++ b/pkg/rlptools/wasm_utils.go @@ -8,6 +8,7 @@ import ( "slices" "strings" + "github.com/samber/lo" _struct "google.golang.org/protobuf/types/known/structpb" istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" "k8s.io/utils/env" @@ -31,20 +32,17 @@ func WasmRules(rlp *kuadrantv1beta2.RateLimitPolicy, route *gatewayapiv1.HTTPRou return rules } - // Sort RLP limits for consistent comparison with existing wasmplugin objects + rlpKey := client.ObjectKeyFromObject(rlp) limits := rlp.Spec.CommonSpec().Limits - limitNames := make([]string, 0, len(limits)) - for name := range limits { - limitNames = append(limitNames, name) - } - // sort the slice by limit name + // Sort RLP limits for consistent comparison with existing wasmplugin objects + limitNames := lo.Keys(limits) slices.Sort(limitNames) for _, limitName := range limitNames { // 1 RLP limit <---> 1 WASM rule limit := limits[limitName] - limitIdentifier := LimitNameToLimitadorIdentifier(limitName) + limitIdentifier := LimitNameToLimitadorIdentifier(rlpKey, limitName) rule, err := ruleFromLimit(limitIdentifier, &limit, route) if err == nil { rules = append(rules, rule) diff --git a/pkg/rlptools/wasm_utils_test.go b/pkg/rlptools/wasm_utils_test.go index 31eb841b0..be2a6f15c 100644 --- a/pkg/rlptools/wasm_utils_test.go +++ b/pkg/rlptools/wasm_utils_test.go @@ -100,7 +100,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "limit.50rps__770adfd9", + Key: "limit.50rps__36e9aa4c", Value: "1", }, }, @@ -150,7 +150,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "limit.50rps_for_selected_hostnames__5af2c820", + Key: "limit.50rps_for_selected_hostnames__ac4044ab", Value: "1", }, }, @@ -200,7 +200,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "limit.50rps_for_selected_route__b6640119", + Key: "limit.50rps_for_selected_route__db289136", Value: "1", }, }, @@ -249,7 +249,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "limit.50rps_for_selected_path__4088dcf9", + Key: "limit.50rps_for_selected_path__38eb97a4", Value: "1", }, }, @@ -290,7 +290,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "limit.50rps__770adfd9", + Key: "limit.50rps__783b9343", Value: "1", }, }, @@ -313,7 +313,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "limit.50rps_per_username__f5bebfb8", + Key: "limit.50rps_per_username__d681f6c3", Value: "1", }, }, diff --git a/tests/commons.go b/tests/commons.go index b9abe0531..84114a790 100644 --- a/tests/commons.go +++ b/tests/commons.go @@ -128,8 +128,8 @@ func GatewayIsReady(ctx context.Context, cl client.Client, gateway *gatewayapiv1 } } -func BuildBasicHttpRoute(routeName, gwName, ns string, hostnames []string) *gatewayapiv1.HTTPRoute { - return &gatewayapiv1.HTTPRoute{ +func BuildBasicHttpRoute(routeName, gwName, ns string, hostnames []string, mutateFns ...func(*gatewayapiv1.HTTPRoute)) *gatewayapiv1.HTTPRoute { + route := &gatewayapiv1.HTTPRoute{ TypeMeta: metav1.TypeMeta{ Kind: "HTTPRoute", APIVersion: gatewayapiv1.GroupVersion.String(), @@ -164,6 +164,10 @@ func BuildBasicHttpRoute(routeName, gwName, ns string, hostnames []string) *gate }, }, } + for _, mutateFn := range mutateFns { + mutateFn(route) + } + return route } func RouteIsAccepted(ctx context.Context, cl client.Client, routeKey client.ObjectKey) func() bool { diff --git a/tests/istio/rate_limiting_wasmplugin_controller_test.go b/tests/istio/rate_limiting_wasmplugin_controller_test.go index 8a29bb00b..b77e6e2bb 100644 --- a/tests/istio/rate_limiting_wasmplugin_controller_test.go +++ b/tests/istio/rate_limiting_wasmplugin_controller_test.go @@ -162,7 +162,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.l1__2804bad6`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlpKey, "l1"), Value: "1", }, }, @@ -354,7 +354,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "limit.toys__3bfcbeee", + Key: rlptools.LimitNameToLimitadorIdentifier(rlpKey, "toys"), Value: "1", }, }, @@ -380,7 +380,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "limit.assets__8bf729ff", + Key: rlptools.LimitNameToLimitadorIdentifier(rlpKey, "assets"), Value: "1", }, }, @@ -465,7 +465,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.l1__2804bad6`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlpKey, "l1"), Value: "1", }, }, @@ -780,7 +780,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.l1__2804bad6`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlpAKey, "l1"), Value: "1", }, }, @@ -928,7 +928,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.l1__2804bad6`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlpKey, "l1"), Value: "1", }, }, @@ -1123,7 +1123,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.l1__2804bad6`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlpKey, "l1"), Value: "1", }, }, @@ -1239,7 +1239,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.l1__2804bad6`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlpKey, "l1"), Value: "1", }, }, @@ -1420,7 +1420,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.l1__2804bad6`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlpKey, "l1"), Value: "1", }, }, @@ -1499,7 +1499,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.l1__2804bad6`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlpKey, "l1"), Value: "1", }, }, @@ -1655,7 +1655,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.gatewaylimit__b95fa83b`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlp1Key, "gatewaylimit"), Value: "1", }, }, @@ -1759,7 +1759,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.routelimit__efc5113c`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlp2Key, "routelimit"), Value: "1", }, }, @@ -1949,7 +1949,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.routelimit__efc5113c`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlp2Key, "routelimit"), Value: "1", }, }, @@ -2043,7 +2043,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.gatewaylimit__b95fa83b`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlp1Key, "gatewaylimit"), Value: "1", }, }, @@ -2077,7 +2077,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.routelimit__efc5113c`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlp2Key, "routelimit"), Value: "1", }, }, @@ -2205,7 +2205,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: `limit.l1__2804bad6`, + Key: rlptools.LimitNameToLimitadorIdentifier(rlpKey, "l1"), Value: "1", }, }, @@ -2325,7 +2325,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { Expect(testClient().Get(ctx, wasmPluginKey, existingWasmPlugin)).To(Succeed()) existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) Expect(err).ToNot(HaveOccurred()) - Expect(existingWASMConfig).To(Equal(expectedWasmPluginConfig(gwRLPKey, gwRLP, "limit.gateway__4ea5ee68", "*"))) + Expect(existingWASMConfig).To(Equal(expectedWasmPluginConfig(gwRLPKey, gwRLP, rlptools.LimitNameToLimitadorIdentifier(gwRLPKey, "gateway"), "*"))) // Create Route RLP routeRLP := &kuadrantv1beta2.RateLimitPolicy{ @@ -2361,7 +2361,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { g.Expect(testClient().Get(ctx, wasmPluginKey, existingWasmPlugin)).To(Succeed()) existingWASMConfig, err = rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(existingWASMConfig).To(Equal(expectedWasmPluginConfig(routeRLPKey, routeRLP, "limit.route__8a84e406", "*.example.com"))) + g.Expect(existingWASMConfig).To(Equal(expectedWasmPluginConfig(routeRLPKey, routeRLP, rlptools.LimitNameToLimitadorIdentifier(routeRLPKey, "route"), "*.example.com"))) }).WithContext(ctx).Should(Succeed()) // Update GW RLP to overrides @@ -2378,7 +2378,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", Ordered, func() { g.Expect(testClient().Get(ctx, wasmPluginKey, existingWasmPlugin)).To(Succeed()) existingWASMConfig, err = rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(existingWASMConfig).To(Equal(expectedWasmPluginConfig(routeRLPKey, routeRLP, "limit.gateway__4ea5ee68", "*.example.com"))) + g.Expect(existingWASMConfig).To(Equal(expectedWasmPluginConfig(routeRLPKey, routeRLP, rlptools.LimitNameToLimitadorIdentifier(routeRLPKey, "gateway"), "*.example.com"))) }).WithContext(ctx).Should(Succeed()) }, testTimeOut)