From f9271f0689006adaaf0fbc295a9f620dc886b202 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Mon, 2 Oct 2023 18:06:33 +0200 Subject: [PATCH 01/21] wasmplugin controller --- .../rate_limiting_wasmplugin_controller.go | 296 ++++++++++++ controllers/ratelimitpolicy_controller.go | 9 - .../ratelimitpolicy_istio_wasmplugin.go | 237 ---------- main.go | 23 + pkg/common/common.go | 8 + pkg/common/gatewayapi_event_mappers.go | 49 ++ pkg/common/gatewayapi_utils.go | 87 +++- pkg/common/istio_utils.go | 9 + pkg/common/kuadrant_policy_test.go | 56 +++ .../kuadrant_policy_to_gateway_eventmapper.go | 77 ++++ pkg/common/kuadrant_topology.go | 385 ++++++++++++++++ pkg/common/kuadrant_topology_test.go | 432 ++++++++++++++++++ 12 files changed, 1415 insertions(+), 253 deletions(-) create mode 100644 controllers/rate_limiting_wasmplugin_controller.go delete mode 100644 controllers/ratelimitpolicy_istio_wasmplugin.go create mode 100644 pkg/common/gatewayapi_event_mappers.go create mode 100644 pkg/common/kuadrant_policy_test.go create mode 100644 pkg/common/kuadrant_policy_to_gateway_eventmapper.go create mode 100644 pkg/common/kuadrant_topology.go create mode 100644 pkg/common/kuadrant_topology_test.go diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go new file mode 100644 index 000000000..c163e7128 --- /dev/null +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -0,0 +1,296 @@ +/* +Copyright 2021 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "encoding/json" + "sort" + + "github.com/go-logr/logr" + istioextensionsv1alpha1 "istio.io/api/extensions/v1alpha1" + istiov1beta1 "istio.io/api/type/v1beta1" + istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + + kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" + "github.com/kuadrant/kuadrant-operator/pkg/common" + "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" + "github.com/kuadrant/kuadrant-operator/pkg/rlptools" + "github.com/kuadrant/kuadrant-operator/pkg/rlptools/wasm" +) + +// RateLimitingWASMPluginReconciler reconciles a WASMPlugin object for rate limiting +type RateLimitingWASMPluginReconciler struct { + reconcilers.TargetRefReconciler +} + +//+kubebuilder:rbac:groups=extensions.istio.io,resources=wasmplugins,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways,verbs=get;list;watch;update;patch +//+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=httproutes,verbs=get;list;watch;update;patch +//+kubebuilder:rbac:groups=kuadrant.io,resources=ratelimitpolicies,verbs=get;list;watch;update;patch + +// 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.Info("Reconciling rate limiting WASMPlugin") + ctx := logr.NewContext(eventCtx, logger) + + gw := &gatewayapiv1.Gateway{} + if err := r.Client().Get(ctx, req.NamespacedName, gw); err != nil { + if apierrors.IsNotFound(err) { + logger.Info("no gateway found") + return ctrl.Result{}, nil + } + logger.Error(err, "failed to get gateway") + return ctrl.Result{}, err + } + + if logger.V(1).Enabled() { + jsonData, err := json.MarshalIndent(gw, "", " ") + if err != nil { + return ctrl.Result{}, err + } + logger.V(1).Info(string(jsonData)) + } + + desired, err := r.desiredRateLimitingWASMPlugin(ctx, gw) + if err != nil { + return ctrl.Result{}, err + } + + err = r.ReconcileResource(ctx, &istioclientgoextensionv1alpha1.WasmPlugin{}, desired, rlptools.WASMPluginMutator) + if err != nil { + return ctrl.Result{}, err + } + + logger.Info("Rate limiting WASMPlugin reconciled successfully") + return ctrl.Result{}, nil +} + +func (r *RateLimitingWASMPluginReconciler) desiredRateLimitingWASMPlugin(ctx context.Context, gw *gatewayapiv1.Gateway) (*istioclientgoextensionv1alpha1.WasmPlugin, error) { + wasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{ + TypeMeta: metav1.TypeMeta{ + Kind: "WasmPlugin", + APIVersion: "extensions.istio.io/v1alpha1", + }, + ObjectMeta: common.RateLimitingWASMPluginName(gw), + Spec: istioextensionsv1alpha1.WasmPlugin{ + TargetRef: &istiov1beta1.PolicyTargetReference{ + Group: "gateway.networking.k8s.io", + Kind: "Gateway", + Name: gw.Name, + }, + Url: rlptools.WASMFilterImageURL, + PluginConfig: nil, + // Insert plugin before Istio stats filters and after Istio authorization filters. + Phase: istioextensionsv1alpha1.PluginPhase_STATS, + }, + } + + pluginConfig, err := r.wasmPluginConfig(ctx, gw) + if err != nil { + return nil, err + } + + if pluginConfig == nil || len(pluginConfig.RateLimitPolicies) == 0 { + common.TagObjectToDelete(wasmPlugin) + return wasmPlugin, nil + } + + pluginConfigStruct, err := pluginConfig.ToStruct() + if err != nil { + return nil, err + } + + wasmPlugin.Spec.PluginConfig = pluginConfigStruct + + // controller reference + if err := r.SetOwnerReference(gw, wasmPlugin); err != nil { + return nil, err + } + + return wasmPlugin, nil +} + +func (r *RateLimitingWASMPluginReconciler) wasmPluginConfig(ctx context.Context, gw *gatewayapiv1.Gateway) (*wasm.Plugin, error) { + logger, err := logr.FromContext(ctx) + if err != nil { + return nil, err + } + + wasmPlugin := &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: make([]wasm.RateLimitPolicy, 0), + } + + gatewayAPITopology, err := r.gatewayAPITopologyFromGateway(ctx, gw) + if err != nil { + return nil, err + } + + rateLimitPolicies := gatewayAPITopology.PoliciesFromGateway(gw) + + logger.V(1).Info("wasmPluginConfig", "#RLPS", len(rateLimitPolicies)) + + // Shallow copy by assignment + rateLimitPoliciesSorted := rateLimitPolicies + + // Sort RLPs for consistent comparison with existing objects + sort.Sort(common.PolicyByKey(rateLimitPoliciesSorted)) + + for _, policy := range rateLimitPolicies { + rlp := policy.(*kuadrantv1beta2.RateLimitPolicy) + wasmRLP := r.WASMRateLimitPolicy(gatewayAPITopology, rlp, gw) + if wasmRLP == nil { + // skip this RLP + continue + } + + wasmPlugin.RateLimitPolicies = append(wasmPlugin.RateLimitPolicies, *wasmRLP) + } + + return wasmPlugin, nil +} + +func (r *RateLimitingWASMPluginReconciler) gatewayAPITopologyFromGateway(ctx context.Context, gw *gatewayapiv1.Gateway) (*common.KuadrantTopology, error) { + logger, err := logr.FromContext(ctx) + if err != nil { + return nil, err + } + + routeList := &gatewayapiv1.HTTPRouteList{} + // Get all the routes having the gateway as parent + err = r.Client().List(ctx, routeList, client.MatchingFields{common.HTTPRouteParents: client.ObjectKeyFromObject(gw).String()}) + logger.V(1).Info("gatewayAPITopologyFromGateway: list httproutes from gateway", "err", err) + if err != nil { + return nil, err + } + + rlpList := &kuadrantv1beta2.RateLimitPolicyList{} + // Get all the rate limit policies + // TODO(eastizle): Add index field?? + err = r.Client().List(ctx, rlpList) + logger.V(1).Info("gatewayAPITopologyFromGateway: list rate limit policies", "err", err) + if err != nil { + return nil, err + } + + return common.NewKuadrantTopology( + []*gatewayapiv1.Gateway{gw}, + common.Map(routeList.Items, func(r gatewayapiv1.HTTPRoute) *gatewayapiv1.HTTPRoute { return &r }), + common.Map(rlpList.Items, func(p kuadrantv1beta2.RateLimitPolicy) common.KuadrantPolicy { return &p }), + ), nil +} + +func (r *RateLimitingWASMPluginReconciler) WASMRateLimitPolicy(t *common.KuadrantTopology, rlp *kuadrantv1beta2.RateLimitPolicy, gw *gatewayapiv1.Gateway) *wasm.RateLimitPolicy { + gwHostnamesTmp := common.TargetHostnames(gw) + gwHostnames := common.Map(gwHostnamesTmp, func(str string) gatewayapiv1.Hostname { return gatewayapiv1.Hostname(str) }) + + route := r.RouteFromRLP(t, rlp, gw) + + rules := rlptools.WasmRules(rlp, route) + 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 + return nil + } + + // narrow the list of hostnames specified in the route so we don't generate wasm rules that only apply to other gateways + // this is a no-op for the gateway rlp + hostnames := common.FilterValidSubdomains(gwHostnames, route.Spec.Hostnames) + if len(hostnames) == 0 { // it should only happen when the route specifies no hostnames + hostnames = gwHostnames + } + + return &wasm.RateLimitPolicy{ + Name: client.ObjectKeyFromObject(rlp).String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlp), + Hostnames: common.HostnamesToStrings(hostnames), // we might be listing more hostnames than needed due to route selectors hostnames possibly being more restrictive + Service: common.KuadrantRateLimitClusterName, + Rules: rules, + } +} + +func (r *RateLimitingWASMPluginReconciler) RouteFromRLP(t *common.KuadrantTopology, rlp *kuadrantv1beta2.RateLimitPolicy, gw *gatewayapiv1.Gateway) *gatewayapiv1.HTTPRoute { + route := t.GetPolicyHTTPRoute(rlp) + + if route == nil { + // The policy is targeting a gateway + // This gateway policy will be enforced into all HTTPRoutes that do not have a policy attached to it + + // Build imaginary route with all the routes not having a RLP targeting it + freeRoutes := t.GetFreeRoutes(gw) + + // For policies targeting a gateway, when no httproutes is attached to the gateway, skip wasm config + // test wasm config when no http routes attached to the gateway + //logger.V(1).Info("no httproutes attached to the targeted gateway, skipping wasm config for the gateway rlp", "ratelimitpolicy", gwRLPKey) + freeRules := make([]gatewayapiv1.HTTPRouteRule, 0) + for idx := range freeRoutes { + freeroute := freeRoutes[idx] + freeRules = append(freeRules, freeroute.Spec.Rules...) + } + + gwHostnamesTmp := common.TargetHostnames(gw) + gwHostnames := common.Map(gwHostnamesTmp, func(str string) gatewayapiv1.Hostname { return gatewayapiv1.Hostname(str) }) + route = &gatewayapiv1.HTTPRoute{ + Spec: gatewayapiv1.HTTPRouteSpec{ + Hostnames: gwHostnames, + Rules: freeRules, + }, + } + } + + return route +} + +// SetupWithManager sets up the controller with the Manager. +func (r *RateLimitingWASMPluginReconciler) SetupWithManager(mgr ctrl.Manager) error { + httpRouteToParentGatewaysEventMapper := &common.HTTPRouteToParentGatewaysEventMapper{ + Logger: r.Logger().WithName("httpRouteToParentGatewaysEventMapper"), + } + + rlpToParentGatewaysEventMapper := &common.KuadrantPolicyToParentGatewaysEventMapper{ + Logger: r.Logger().WithName("ratelimitpolicyToParentGatewaysEventMapper"), + Client: r.Client(), + } + + return ctrl.NewControllerManagedBy(mgr). + // Rate limiting WASMPlugin controller only cares about + // Gateway API Gateway + // Gateway API HTTPRoutes + // Kuadrant RateLimitPolicies + + // The type of object being *reconciled* is the Gateway. + // TODO(eguzki): consider having the WasmPlugin as the type of object being *reconciled* + For(&gatewayapiv1.Gateway{}). + Owns(&istioclientgoextensionv1alpha1.WasmPlugin{}). + Watches( + &gatewayapiv1.HTTPRoute{}, + handler.EnqueueRequestsFromMapFunc(httpRouteToParentGatewaysEventMapper.Map), + ). + Watches( + &kuadrantv1beta2.RateLimitPolicy{}, + handler.EnqueueRequestsFromMapFunc(rlpToParentGatewaysEventMapper.Map), + ). + Complete(r) +} diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index 88ac46e30..365b840ca 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -44,7 +44,6 @@ type RateLimitPolicyReconciler struct { //+kubebuilder:rbac:groups=kuadrant.io,resources=ratelimitpolicies/status,verbs=get;update;patch //+kubebuilder:rbac:groups=kuadrant.io,resources=ratelimitpolicies/finalizers,verbs=update //+kubebuilder:rbac:groups=limitador.kuadrant.io,resources=limitadors,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=extensions.istio.io,resources=wasmplugins,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=httproutes,verbs=get;list;watch;update;patch //+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gateways,verbs=get;list;watch;update;patch @@ -177,10 +176,6 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp return err } - if err := r.reconcileWASMPluginConf(ctx, rlp, gatewayDiffObj); err != nil { - return err - } - // set direct back ref - i.e. claim the target network object as taken asap if err := r.reconcileNetworkResourceDirectBackReference(ctx, rlp, targetNetworkObject); err != nil { return err @@ -197,10 +192,6 @@ func (r *RateLimitPolicyReconciler) deleteResources(ctx context.Context, rlp *ku return err } - if err := r.reconcileWASMPluginConf(ctx, rlp, gatewayDiffObj); err != nil { - return err - } - if err := r.deleteLimits(ctx, rlp); err != nil && !apierrors.IsNotFound(err) { return err } diff --git a/controllers/ratelimitpolicy_istio_wasmplugin.go b/controllers/ratelimitpolicy_istio_wasmplugin.go deleted file mode 100644 index 5802b8d3f..000000000 --- a/controllers/ratelimitpolicy_istio_wasmplugin.go +++ /dev/null @@ -1,237 +0,0 @@ -package controllers - -import ( - "context" - "fmt" - "slices" - - "github.com/go-logr/logr" - istioextensionsv1alpha1 "istio.io/api/extensions/v1alpha1" - istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - - kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" - "github.com/kuadrant/kuadrant-operator/pkg/common" - "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" - "github.com/kuadrant/kuadrant-operator/pkg/rlptools" - "github.com/kuadrant/kuadrant-operator/pkg/rlptools/wasm" -) - -func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, gwDiffObj *reconcilers.GatewayDiff) error { - logger, _ := logr.FromContext(ctx) - - for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef { - logger.V(1).Info("reconcileWASMPluginConf: gateway with invalid policy ref", "gw key", gw.Key()) - rlpRefs := gw.PolicyRefs() - rlpKey := client.ObjectKeyFromObject(rlp) - // Remove the RLP key from the reference list. Only if it exists (it should) - if refID := common.FindObjectKey(rlpRefs, rlpKey); refID != len(rlpRefs) { - // remove index - rlpRefs = append(rlpRefs[:refID], rlpRefs[refID+1:]...) - } - wp, err := r.gatewayWASMPlugin(ctx, gw, rlpRefs) - if err != nil { - return err - } - err = r.ReconcileResource(ctx, &istioclientgoextensionv1alpha1.WasmPlugin{}, wp, rlptools.WASMPluginMutator) - if err != nil { - return err - } - } - - for _, gw := range gwDiffObj.GatewaysWithValidPolicyRef { - logger.V(1).Info("reconcileWASMPluginConf: gateway with valid policy ref", "gw key", gw.Key()) - wp, err := r.gatewayWASMPlugin(ctx, gw, gw.PolicyRefs()) - if err != nil { - return err - } - err = r.ReconcileResource(ctx, &istioclientgoextensionv1alpha1.WasmPlugin{}, wp, rlptools.WASMPluginMutator) - if err != nil { - return err - } - } - - for _, gw := range gwDiffObj.GatewaysMissingPolicyRef { - logger.V(1).Info("reconcileWASMPluginConf: gateway missing policy ref", "gw key", gw.Key()) - rlpRefs := gw.PolicyRefs() - rlpKey := client.ObjectKeyFromObject(rlp) - // Add the RLP key to the reference list. Only if it does not exist (it should not) - if !slices.Contains(rlpRefs, rlpKey) { - rlpRefs = append(gw.PolicyRefs(), rlpKey) - } - wp, err := r.gatewayWASMPlugin(ctx, gw, rlpRefs) - if err != nil { - return err - } - err = r.ReconcileResource(ctx, &istioclientgoextensionv1alpha1.WasmPlugin{}, wp, rlptools.WASMPluginMutator) - if err != nil { - return err - } - } - return nil -} - -func (r *RateLimitPolicyReconciler) gatewayWASMPlugin(ctx context.Context, gw common.GatewayWrapper, rlpRefs []client.ObjectKey) (*istioclientgoextensionv1alpha1.WasmPlugin, error) { - logger, _ := logr.FromContext(ctx) - logger.V(1).Info("gatewayWASMPlugin", "gwKey", gw.Key(), "rlpRefs", rlpRefs) - - wasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{ - TypeMeta: metav1.TypeMeta{ - Kind: "WasmPlugin", - APIVersion: "extensions.istio.io/v1alpha1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: rlptools.WASMPluginName(gw.Gateway), - Namespace: gw.Namespace, - }, - Spec: istioextensionsv1alpha1.WasmPlugin{ - Selector: common.IstioWorkloadSelectorFromGateway(ctx, r.Client(), gw.Gateway), - Url: rlptools.WASMFilterImageURL, - PluginConfig: nil, - // Insert plugin before Istio stats filters and after Istio authorization filters. - Phase: istioextensionsv1alpha1.PluginPhase_STATS, - }, - } - - if len(rlpRefs) < 1 { - common.TagObjectToDelete(wasmPlugin) - return wasmPlugin, nil - } - - pluginConfig, err := r.wasmPluginConfig(ctx, gw, rlpRefs) - if err != nil { - return nil, err - } - - if pluginConfig == nil { - common.TagObjectToDelete(wasmPlugin) - return wasmPlugin, nil - } - - pluginConfigStruct, err := pluginConfig.ToStruct() - if err != nil { - return nil, err - } - - wasmPlugin.Spec.PluginConfig = pluginConfigStruct - - return wasmPlugin, nil -} - -// returns nil when there is no rate limit policy to apply -func (r *RateLimitPolicyReconciler) wasmPluginConfig(ctx context.Context, gw common.GatewayWrapper, rlpRefs []client.ObjectKey) (*wasm.Plugin, error) { - logger, _ := logr.FromContext(ctx) - logger = logger.WithName("wasmPluginConfig").WithValues("gateway", gw.Key()) - - type store struct { - rlp kuadrantv1beta2.RateLimitPolicy - route gatewayapiv1.HTTPRoute - skip bool - } - rlps := make(map[string]*store, len(rlpRefs)) - routeKeys := make(map[string]struct{}, 0) - var gwRLPKey string - - // store all rlps and find the one that targets the gateway (if there is one) - for _, rlpKey := range rlpRefs { - 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 - } - - // target ref is a HTTPRoute - if common.IsTargetRefHTTPRoute(rlp.Spec.TargetRef) { - route, err := r.FetchValidHTTPRoute(ctx, rlp.TargetKey()) - if err != nil { - return nil, err - } - rlps[rlpKey.String()] = &store{rlp: *rlp, route: *route} - routeKeys[client.ObjectKeyFromObject(route).String()] = struct{}{} - continue - } - - // target ref is a Gateway - if rlps[rlpKey.String()] != nil { - return nil, fmt.Errorf("wasmPluginConfig: multiple gateway RLP found and only one expected. rlp keys: %v", rlpRefs) - } - gwRLPKey = rlpKey.String() - rlps[gwRLPKey] = &store{rlp: *rlp} - } - - gwHostnames := gw.Hostnames() - if len(gwHostnames) == 0 { - gwHostnames = []gatewayapiv1.Hostname{"*"} - } - - // if there is a gateway rlp, fake a single httproute with all rules from all httproutes accepted by the gateway, - // that do not have a rlp of its own, so we can generate wasm rules for those cases - if gwRLPKey != "" { - rules := make([]gatewayapiv1.HTTPRouteRule, 0) - routes := r.FetchAcceptedGatewayHTTPRoutes(ctx, rlps[gwRLPKey].rlp.TargetKey()) - for idx := range routes { - route := routes[idx] - // skip routes that have a rlp of its own - if _, found := routeKeys[client.ObjectKeyFromObject(&route).String()]; found { - continue - } - rules = append(rules, route.Spec.Rules...) - } - if len(rules) == 0 { - logger.V(1).Info("no httproutes attached to the targeted gateway, skipping wasm config for the gateway rlp", "ratelimitpolicy", gwRLPKey) - rlps[gwRLPKey].skip = true - } else { - rlps[gwRLPKey].route = gatewayapiv1.HTTPRoute{ - Spec: gatewayapiv1.HTTPRouteSpec{ - Hostnames: gwHostnames, - Rules: rules, - }, - } - } - } - - wasmPlugin := &wasm.Plugin{ - FailureMode: wasm.FailureModeDeny, - RateLimitPolicies: make([]wasm.RateLimitPolicy, 0), - } - - for _, rlpKey := range rlpRefs { - s := rlps[rlpKey.String()] - if s.skip { - continue - } - rlp := s.rlp - route := s.route - - // narrow the list of hostnames specified in the route so we don't generate wasm rules that only apply to other gateways - // this is a no-op for the gateway rlp - hostnames := common.FilterValidSubdomains(gwHostnames, route.Spec.Hostnames) - if len(hostnames) == 0 { // it should only happen when the route specifies no hostnames - hostnames = gwHostnames - } - route.Spec.Hostnames = hostnames - - rules := rlptools.WasmRules(&rlp, &route) - if len(rules) == 0 { - continue // 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 - } - - wasmPlugin.RateLimitPolicies = append(wasmPlugin.RateLimitPolicies, wasm.RateLimitPolicy{ - Name: rlpKey.String(), - Domain: rlptools.LimitsNamespaceFromRLP(&rlp), - Rules: rules, - Hostnames: common.HostnamesToStrings(hostnames), // we might be listing more hostnames than needed due to route selectors hostnames possibly being more restrictive - Service: common.KuadrantRateLimitClusterName, - }) - } - - // avoid building a wasm plugin config if there are no rules to apply - if len(wasmPlugin.RateLimitPolicies) == 0 { - return nil, nil - } - - return wasmPlugin, nil -} diff --git a/main.go b/main.go index eecc0f875..da4fcdd83 100644 --- a/main.go +++ b/main.go @@ -58,6 +58,7 @@ import ( kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/controllers" + "github.com/kuadrant/kuadrant-operator/pkg/common" "github.com/kuadrant/kuadrant-operator/pkg/log" "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" //+kubebuilder:scaffold:imports @@ -139,6 +140,12 @@ func main() { os.Exit(1) } + err = common.AddHTTPRouteByGatewayIndexer(mgr, log.Log.WithName("routeByGatewayIndexer")) + if err != nil { + setupLog.Error(err, "unable to add indexer to the manager") + os.Exit(1) + } + kuadrantBaseReconciler := reconcilers.NewBaseReconciler( mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(), log.Log.WithName("kuadrant"), @@ -240,6 +247,22 @@ func main() { os.Exit(1) } + rateLimitingWASMPluginBaseReconciler := reconcilers.NewBaseReconciler( + mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(), + log.Log.WithName("ratelimitpolicy").WithName("wasmplugin"), + mgr.GetEventRecorderFor("GatewayKuadrant"), + ) + + if err = (&controllers.RateLimitingWASMPluginReconciler{ + TargetRefReconciler: reconcilers.TargetRefReconciler{ + // TODO: TargetRefReconciler needed? + BaseReconciler: rateLimitingWASMPluginBaseReconciler, + }, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "RateLimitingWASMPlugin") + os.Exit(1) + } + //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { diff --git a/pkg/common/common.go b/pkg/common/common.go index 3667968a0..4e0a79ed1 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -54,6 +54,14 @@ type KuadrantPolicyList interface { GetItems() []KuadrantPolicy } +type PolicyByKey []KuadrantPolicy + +func (a PolicyByKey) Len() int { return len(a) } +func (a PolicyByKey) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a PolicyByKey) Less(i, j int) bool { + return client.ObjectKeyFromObject(a[i]).String() < client.ObjectKeyFromObject(a[j]).String() +} + // GetEmptySliceIfNil returns a provided slice, or an empty slice of the same type if the input slice is nil. func GetEmptySliceIfNil[T any](val []T) []T { if val == nil { diff --git a/pkg/common/gatewayapi_event_mappers.go b/pkg/common/gatewayapi_event_mappers.go new file mode 100644 index 000000000..4ef9ad9a7 --- /dev/null +++ b/pkg/common/gatewayapi_event_mappers.go @@ -0,0 +1,49 @@ +package common + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +// HTTPRouteToParentGatewaysEventMapper is an EventHandler that maps HTTPRoute events to gateway events, +// by going through the parentRefs of the route +type HTTPRouteToParentGatewaysEventMapper struct { + Logger logr.Logger +} + +func (m *HTTPRouteToParentGatewaysEventMapper) Map(_ context.Context, obj client.Object) []reconcile.Request { + logger := m.Logger.WithValues("object", client.ObjectKeyFromObject(obj)) + + route, ok := obj.(*gatewayapiv1.HTTPRoute) + if !ok { + logger.Error(fmt.Errorf("%T is not a *gatewayapiv1.HTTPRoute", obj), "cannot map") + return []reconcile.Request{} + } + + requests := make([]reconcile.Request, 0) + + for _, parentRef := range route.Spec.ParentRefs { + // skips if parentRef is not a Gateway + if !IsParentGateway(parentRef) { + continue + } + + ns := route.Namespace + if parentRef.Namespace != nil { + ns = string(*parentRef.Namespace) + } + + nn := types.NamespacedName{Name: string(parentRef.Name), Namespace: ns} + logger.V(1).Info("map", " gateway", nn) + + requests = append(requests, reconcile.Request{NamespacedName: nn}) + } + + return requests +} diff --git a/pkg/common/gatewayapi_utils.go b/pkg/common/gatewayapi_utils.go index dc4a0feda..86a66bf50 100644 --- a/pkg/common/gatewayapi_utils.go +++ b/pkg/common/gatewayapi_utils.go @@ -8,16 +8,21 @@ import ( "slices" "strings" + "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) -const GatewayProgrammedConditionType = "Programmed" +const ( + GatewayProgrammedConditionType = "Programmed" + HTTPRouteParents = ".metadata.parent" +) type HTTPRouteRule struct { Paths []string @@ -33,6 +38,10 @@ func IsTargetRefGateway(targetRef gatewayapiv1alpha2.PolicyTargetReference) bool return targetRef.Group == ("gateway.networking.k8s.io") && targetRef.Kind == ("Gateway") } +func IsParentGateway(ref gatewayapiv1.ParentReference) bool { + return (ref.Kind == nil || *ref.Kind == "Gateway") && (ref.Group == nil || *ref.Group == "gateway.networking.k8s.io") +} + func RouteHTTPMethodToRuleMethod(httpMethod *gatewayapiv1.HTTPMethod) []string { if httpMethod == nil { return nil @@ -530,7 +539,7 @@ func (g GatewayWrapperList) Swap(i, j int) { } // TargetHostnames returns an array of hostnames coming from the network object (HTTPRoute, Gateway) -func TargetHostnames(targetNetworkObject client.Object) ([]string, error) { +func TargetHostnames(targetNetworkObject client.Object) []string { hosts := make([]string, 0) switch obj := targetNetworkObject.(type) { case *gatewayapiv1.HTTPRoute: @@ -549,7 +558,7 @@ func TargetHostnames(targetNetworkObject client.Object) ([]string, error) { hosts = append(hosts, "*") } - return hosts, nil + return hosts } // HostnamesFromHTTPRoute returns an array of all hostnames specified in a HTTPRoute or inherited from its parent Gateways @@ -581,10 +590,7 @@ func HostnamesFromHTTPRoute(ctx context.Context, route *gatewayapiv1.HTTPRoute, // ValidateHierarchicalRules returns error if the policy rules hostnames fail to match the target network hosts func ValidateHierarchicalRules(policy KuadrantPolicy, targetNetworkObject client.Object) error { - targetHostnames, err := TargetHostnames(targetNetworkObject) - if err != nil { - return err - } + targetHostnames := TargetHostnames(targetNetworkObject) if valid, invalidHost := ValidSubdomains(targetHostnames, policy.GetRulesHostnames()); !valid { return fmt.Errorf( @@ -650,3 +656,70 @@ func IsHTTPRouteAccepted(httpRoute *gatewayapiv1.HTTPRoute) bool { return true } + +// AddHTTPRouteByGatewayIndexer declares an index key that we can later use with the client as a pseudo-field name, +// allowing to query all the routes parenting a given gateway +func AddHTTPRouteByGatewayIndexer(mgr ctrl.Manager, logger logr.Logger) error { + if err := mgr.GetFieldIndexer().IndexField(context.Background(), &gatewayapiv1.HTTPRoute{}, HTTPRouteParents, func(rawObj client.Object) []string { + // grab the route object, extract the parents + route, assertionOk := rawObj.(*gatewayapiv1.HTTPRoute) + logger.Info("assertionOK", "ok", assertionOk) + if !assertionOk { + return nil + } + + logger.Info("route", "name", client.ObjectKeyFromObject(route).String()) + + keys := make([]string, 0) + + for _, parentRef := range route.Spec.CommonRouteSpec.ParentRefs { + if !IsParentGateway(parentRef) { + logger.Info("parent not gateway", "ParentRefs", parentRef) + continue + } + + key := client.ObjectKey{ + Name: string(parentRef.Name), + Namespace: string(ptr.Deref(parentRef.Namespace, gatewayapiv1.Namespace(route.Namespace))), + } + + logger.Info("new key", "key", key.String()) + + keys = append(keys, key.String()) + } + + // ...and if so, return it + return keys + }); err != nil { + return err + } + + return nil +} + +func GetRouteAcceptedGatewayParentKeys(route *gatewayapiv1.HTTPRoute) []client.ObjectKey { + if route == nil { + return nil + } + + acceptedRouteParentStatus := Filter(route.Status.RouteStatus.Parents, func(p gatewayapiv1.RouteParentStatus) bool { + // Only gateway parents + if !IsParentGateway(p.ParentRef) { + return false + } + + // Only gateways that accepted this route + if meta.IsStatusConditionFalse(p.Conditions, "Accepted") { + return false + } + + return true + }) + + return Map(acceptedRouteParentStatus, func(p gatewayapiv1.RouteParentStatus) client.ObjectKey { + return client.ObjectKey{ + Name: string(p.ParentRef.Name), + Namespace: string(ptr.Deref(p.ParentRef.Namespace, gatewayapiv1.Namespace(route.Namespace))), + } + }) +} diff --git a/pkg/common/istio_utils.go b/pkg/common/istio_utils.go index d263f401d..a0a41b661 100644 --- a/pkg/common/istio_utils.go +++ b/pkg/common/istio_utils.go @@ -2,9 +2,11 @@ package common import ( "context" + "fmt" "github.com/go-logr/logr" istiocommon "istio.io/api/type/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" ) @@ -20,3 +22,10 @@ func IstioWorkloadSelectorFromGateway(ctx context.Context, k8sClient client.Clie MatchLabels: gatewayWorkloadSelector, } } + +func RateLimitingWASMPluginName(gw *gatewayapiv1.Gateway) metav1.ObjectMeta { + return metav1.ObjectMeta{ + Name: fmt.Sprintf("kuadrant-%s", gw.Name), + Namespace: gw.Namespace, + } +} diff --git a/pkg/common/kuadrant_policy_test.go b/pkg/common/kuadrant_policy_test.go new file mode 100644 index 000000000..fe07f31d1 --- /dev/null +++ b/pkg/common/kuadrant_policy_test.go @@ -0,0 +1,56 @@ +//go:build unit + +package common + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" +) + +var ( + _ KuadrantPolicy = &TestPolicy{} +) + +type TestPolicy struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + TargetRef gatewayapiv1alpha2.PolicyTargetReference `json:"targetRef"` +} + +func (p *TestPolicy) GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference { + return p.TargetRef +} + +func (p *TestPolicy) GetWrappedNamespace() gatewayapiv1beta1.Namespace { + return gatewayapiv1beta1.Namespace(p.Namespace) +} + +func (p *TestPolicy) GetRulesHostnames() []string { + return nil +} + +func (p *TestPolicy) DeepCopyObject() runtime.Object { + if c := p.DeepCopy(); c != nil { + return c + } + return nil +} + +func (p *TestPolicy) DeepCopy() *TestPolicy { + if p == nil { + return nil + } + out := new(TestPolicy) + p.DeepCopyInto(out) + return out +} + +func (p *TestPolicy) DeepCopyInto(out *TestPolicy) { + *out = *p + out.TypeMeta = p.TypeMeta + p.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + p.TargetRef.DeepCopyInto(&out.TargetRef) +} diff --git a/pkg/common/kuadrant_policy_to_gateway_eventmapper.go b/pkg/common/kuadrant_policy_to_gateway_eventmapper.go new file mode 100644 index 000000000..67f5b322f --- /dev/null +++ b/pkg/common/kuadrant_policy_to_gateway_eventmapper.go @@ -0,0 +1,77 @@ +package common + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +// KuadrantPolicyToParentGatewaysEventMapper is an EventHandler that maps Kuadrant policies to gateway events, +// by going through the policies targetRefs and parentRefs of the route +type KuadrantPolicyToParentGatewaysEventMapper struct { + Logger logr.Logger + Client client.Client +} + +func (k *KuadrantPolicyToParentGatewaysEventMapper) Map(ctx context.Context, obj client.Object) []reconcile.Request { + logger := k.Logger.WithValues("object", client.ObjectKeyFromObject(obj)) + + kuadrantPolicy, ok := obj.(KuadrantPolicy) + if !ok { + logger.Error(fmt.Errorf("%T is not a KuadrantPolicy", obj), "cannot map") + return []reconcile.Request{} + } + + if IsTargetRefGateway(kuadrantPolicy.GetTargetRef()) { + namespace := string(ptr.Deref(kuadrantPolicy.GetTargetRef().Namespace, kuadrantPolicy.GetWrappedNamespace())) + + nn := types.NamespacedName{Name: string(kuadrantPolicy.GetTargetRef().Name), Namespace: namespace} + logger.V(1).Info("map", " gateway", nn) + + return []reconcile.Request{{NamespacedName: nn}} + } + + if IsTargetRefHTTPRoute(kuadrantPolicy.GetTargetRef()) { + namespace := string(ptr.Deref(kuadrantPolicy.GetTargetRef().Namespace, kuadrantPolicy.GetWrappedNamespace())) + routeKey := client.ObjectKey{Name: string(kuadrantPolicy.GetTargetRef().Name), Namespace: namespace} + route := &gatewayapiv1.HTTPRoute{} + if err := k.Client.Get(ctx, routeKey, route); err != nil { + if apierrors.IsNotFound(err) { + logger.V(1).Info("no route found", "route", routeKey) + return []reconcile.Request{} + } + logger.Error(err, "failed to get target", "route", routeKey) + return []reconcile.Request{} + } + + requests := make([]reconcile.Request, 0) + + for _, parentRef := range route.Spec.ParentRefs { + // skips if parentRef is not a Gateway + if !IsParentGateway(parentRef) { + continue + } + + ns := route.Namespace + if parentRef.Namespace != nil { + ns = string(*parentRef.Namespace) + } + + nn := types.NamespacedName{Name: string(parentRef.Name), Namespace: ns} + logger.V(1).Info("map", " gateway", nn) + + requests = append(requests, reconcile.Request{NamespacedName: nn}) + } + + return requests + } + + return []reconcile.Request{} +} diff --git a/pkg/common/kuadrant_topology.go b/pkg/common/kuadrant_topology.go new file mode 100644 index 000000000..0698f389c --- /dev/null +++ b/pkg/common/kuadrant_topology.go @@ -0,0 +1,385 @@ +package common + +import ( + "encoding/json" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +type KuadrantTopology struct { + // gatewayPolicies is an index of gateways mapping to Kuadrant Policies which + // directly or indirectly are targeting the indexed gateway. + // When a kuadrant policy directly or indirectly targets a gateway, the policy's configuration + // needs to be added to that gateway. + // Type: Gateway -> []Policy + gatewayPolicies map[client.ObjectKey][]KuadrantPolicy + + // policyRoute is an index of policies mapping to HTTPRoutes + // The index only includes policies targeting only existing and accepted (by parent gateways) HTTPRoutes + // Type: Policy -> HTTPRoute + policyRoute map[client.ObjectKey]*gatewayapiv1.HTTPRoute + + // freeRoutes is an index of gateways mapping to HTTPRoutes not targeted by a kuadrant policy + // Gateway -> []HTTPRoute + freeRoutes map[client.ObjectKey][]*gatewayapiv1.HTTPRoute + + // Raw topology with gateways, routes and policies + // Currently only used for logging + internalTopology *gatewayAPITopology +} + +func NewKuadrantTopology(gateways []*gatewayapiv1.Gateway, routes []*gatewayapiv1.HTTPRoute, policies []KuadrantPolicy) *KuadrantTopology { + t := newGatewayAPITopology(gateways, routes, policies) + + return &KuadrantTopology{ + gatewayPolicies: buildGatewayPoliciesIndex(t), + policyRoute: buildPolicyRouteIndex(t), + freeRoutes: buildFreeRoutesIndex(t), + internalTopology: t, + } +} + +func (k *KuadrantTopology) PoliciesFromGateway(gateway *gatewayapiv1.Gateway) []KuadrantPolicy { + return k.gatewayPolicies[client.ObjectKeyFromObject(gateway)] +} + +func (k *KuadrantTopology) GetPolicyHTTPRoute(policy KuadrantPolicy) *gatewayapiv1.HTTPRoute { + return k.policyRoute[client.ObjectKeyFromObject(policy)] +} + +func (k *KuadrantTopology) GetFreeRoutes(gateway *gatewayapiv1.Gateway) []*gatewayapiv1.HTTPRoute { + return k.freeRoutes[client.ObjectKeyFromObject(gateway)] +} + +// String representation of the topology +// This is not designed to be a serialization format that could be deserialized +func (k *KuadrantTopology) String() string { + type Gateway struct { + Key string `json:"id"` + Routes []string `json:"routes"` + Policy *string `json:"policy"` + } + + type Route struct { + Key string `json:"id"` + Parents []string `json:"parents"` + Policy *string `json:"policy"` + } + + type Policy struct { + Key string `json:"id"` + Gateway *string `json:"gateway"` + Route *string `json:"route"` + } + + gateways := func() []Gateway { + var gList []Gateway + for _, gwNode := range k.internalTopology.GatewaysIndex { + gw := Gateway{Key: gwNode.ObjectKey().String()} + + var rList []string + for _, route := range gwNode.Routes { + rList = append(rList, route.ObjectKey().String()) + } + gw.Routes = rList + + if gwNode.DirectPolicy != nil { + gw.Policy = &[]string{gwNode.DirectPolicy.ObjectKey().String()}[0] + } + + gList = append(gList, gw) + } + return gList + }() + + routes := func() []Route { + var rList []Route + for _, routeNode := range k.internalTopology.RoutesIndex { + route := Route{Key: routeNode.ObjectKey().String()} + + var pList []string + for _, parent := range routeNode.Parents { + pList = append(pList, parent.ObjectKey().String()) + } + route.Parents = pList + + if routeNode.DirectPolicy != nil { + route.Policy = &[]string{routeNode.DirectPolicy.ObjectKey().String()}[0] + } + + rList = append(rList, route) + } + return rList + }() + + policies := func() []Policy { + var pList []Policy + for _, policyNode := range k.internalTopology.PoliciesIndex { + policy := Policy{Key: policyNode.ObjectKey().String()} + if policyNode.TargetedGateway != nil { + policy.Gateway = &[]string{policyNode.TargetedGateway.ObjectKey().String()}[0] + } + if policyNode.TargetedRoute != nil { + policy.Route = &[]string{policyNode.TargetedRoute.ObjectKey().String()}[0] + } + pList = append(pList, policy) + } + return pList + }() + + policiesPerGateway := func() map[string][]string { + index := make(map[string][]string, 0) + for gatewayKey, policyList := range k.gatewayPolicies { + index[gatewayKey.String()] = Map(policyList, func(p KuadrantPolicy) string { + return client.ObjectKeyFromObject(p).String() + }) + } + if len(index) == 0 { + return nil + } + return index + }() + + policiesTargetingRoutes := func() map[string]string { + index := make(map[string]string, 0) + for policyKey, route := range k.policyRoute { + index[policyKey.String()] = client.ObjectKeyFromObject(route).String() + } + if len(index) == 0 { + return nil + } + return index + }() + + freeRoutesPerGateway := func() map[string][]string { + index := make(map[string][]string, 0) + for gatewayKey, routeList := range k.freeRoutes { + index[gatewayKey.String()] = Map(routeList, func(route *gatewayapiv1.HTTPRoute) string { + return client.ObjectKeyFromObject(route).String() + }) + } + if len(index) == 0 { + return nil + } + return index + }() + + topologyRepr := struct { + Gateways []Gateway `json:"gateways"` + Routes []Route `json:"routes"` + Policies []Policy `json:"policies"` + GatewayPolicies map[string][]string `json:"policiesPerGateway"` + PolicyRoute map[string]string `json:"policiesTargetingRoutes"` + FreeRoutes map[string][]string `json:"freeRoutesPerGateway"` + }{ + gateways, + routes, + policies, + policiesPerGateway, + policiesTargetingRoutes, + freeRoutesPerGateway, + } + + jsonData, err := json.MarshalIndent(topologyRepr, "", " ") + if err != nil { + panic(err) + } + return string(jsonData) +} + +func buildGatewayPoliciesIndex(t *gatewayAPITopology) map[client.ObjectKey][]KuadrantPolicy { + // Build Gateway -> []Policy index with all the policies affecting the indexed gateway + index := make(map[client.ObjectKey][]KuadrantPolicy, 0) + for _, gatewayNode := range t.GatewaysIndex { + // Consisting of: + // - Policy targeting directly the gateway + // - Policies targeting the descendant routes of the gateway + policies := make([]KuadrantPolicy, 0) + + if gatewayNode.DirectPolicy != nil { + policies = append(policies, gatewayNode.DirectPolicy.Policy) + } + + for _, routeNode := range gatewayNode.Routes { + if routeNode.DirectPolicy != nil { + policies = append(policies, routeNode.DirectPolicy.Policy) + } + } + + index[gatewayNode.ObjectKey()] = policies + } + + return index +} + +func buildPolicyRouteIndex(t *gatewayAPITopology) map[client.ObjectKey]*gatewayapiv1.HTTPRoute { + // Build Policy -> HTTPRoute index with the route targeted by the indexed policy + index := make(map[client.ObjectKey]*gatewayapiv1.HTTPRoute, 0) + for _, policyNode := range t.PoliciesIndex { + if policyNode.TargetedRoute != nil { + index[policyNode.ObjectKey()] = policyNode.TargetedRoute.Route + } + } + + return index +} + +func buildFreeRoutesIndex(t *gatewayAPITopology) map[client.ObjectKey][]*gatewayapiv1.HTTPRoute { + // Build Gateway -> []HTTPRoute index with all the routes not targeted by a policy + index := make(map[client.ObjectKey][]*gatewayapiv1.HTTPRoute, 0) + + for _, gatewayNode := range t.GatewaysIndex { + routes := make([]*gatewayapiv1.HTTPRoute, 0) + + for _, routeNode := range gatewayNode.Routes { + if routeNode.DirectPolicy == nil { + routes = append(routes, routeNode.Route) + } + } + + index[gatewayNode.ObjectKey()] = routes + } + + return index +} + +type gatewayNode struct { + DirectPolicy *policyNode + Routes map[client.ObjectKey]*routeNode + Gateway *gatewayapiv1.Gateway +} + +func (g *gatewayNode) ObjectKey() client.ObjectKey { + if g.Gateway == nil { + return client.ObjectKey{} + } + + return client.ObjectKeyFromObject(g.Gateway) +} + +type routeNode struct { + Parents map[client.ObjectKey]*gatewayNode + DirectPolicy *policyNode + Route *gatewayapiv1.HTTPRoute +} + +func (r *routeNode) ObjectKey() client.ObjectKey { + if r.Route == nil { + return client.ObjectKey{} + } + + return client.ObjectKeyFromObject(r.Route) +} + +type policyNode struct { + Policy KuadrantPolicy + TargetedGateway *gatewayNode + TargetedRoute *routeNode +} + +func (p *policyNode) ObjectKey() client.ObjectKey { + if p.Policy == nil { + return client.ObjectKey{} + } + + return client.ObjectKeyFromObject(p.Policy) +} + +type gatewayAPITopology struct { + GatewaysIndex map[client.ObjectKey]*gatewayNode + RoutesIndex map[client.ObjectKey]*routeNode + PoliciesIndex map[client.ObjectKey]*policyNode +} + +func newGatewayAPITopology(gateways []*gatewayapiv1.Gateway, routes []*gatewayapiv1.HTTPRoute, policies []KuadrantPolicy) *gatewayAPITopology { + gatewaysIndex := initializeGateways(gateways) + routesIndex := initializeRoutes(routes) + policiesIndex := initializePolicies(policies) + + // Build botton -> up. Start from policies (leaves) up to gateways + for _, policyNode := range policiesIndex { + if IsTargetRefGateway(policyNode.Policy.GetTargetRef()) { + namespace := string(ptr.Deref(policyNode.Policy.GetTargetRef().Namespace, policyNode.Policy.GetWrappedNamespace())) + + gwKey := client.ObjectKey{Name: string(policyNode.Policy.GetTargetRef().Name), Namespace: namespace} + + gatewayNode := gatewaysIndex[gwKey] + + // the targeted gateway may not be in the available list of gateways + if gatewayNode != nil { + policyNode.TargetedGateway = gatewayNode + gatewayNode.DirectPolicy = policyNode + } + } else if IsTargetRefHTTPRoute(policyNode.Policy.GetTargetRef()) { + namespace := string(ptr.Deref(policyNode.Policy.GetTargetRef().Namespace, policyNode.Policy.GetWrappedNamespace())) + + routeKey := client.ObjectKey{Name: string(policyNode.Policy.GetTargetRef().Name), Namespace: namespace} + + routeNode := routesIndex[routeKey] + + // the targeted route may not be in the available list of routes + if routeNode != nil { + policyNode.TargetedRoute = routeNode + routeNode.DirectPolicy = policyNode + } + } + + // skipping the policy as it does not target neither a valid route nor a valid gateway + } + + for _, routeNode := range routesIndex { + for _, parentKey := range GetRouteAcceptedGatewayParentKeys(routeNode.Route) { + gatewayNode := gatewaysIndex[parentKey] + // the parent gateway may not be in the available list of gateways + // or the gateway may not be valid + if gatewayNode != nil { + gatewayNode.Routes[routeNode.ObjectKey()] = routeNode + routeNode.Parents[gatewayNode.ObjectKey()] = gatewayNode + } + } + } + + return &gatewayAPITopology{ + GatewaysIndex: gatewaysIndex, + RoutesIndex: routesIndex, + PoliciesIndex: policiesIndex, + } +} + +func initializeGateways(gateways []*gatewayapiv1.Gateway) map[client.ObjectKey]*gatewayNode { + gatewaysIndex := make(map[client.ObjectKey]*gatewayNode, 0) + + validGateways := Filter(gateways, func(g *gatewayapiv1.Gateway) bool { + return meta.IsStatusConditionTrue(g.Status.Conditions, GatewayProgrammedConditionType) + }) + + for _, gateway := range validGateways { + gatewaysIndex[client.ObjectKeyFromObject(gateway)] = &gatewayNode{ + Routes: make(map[client.ObjectKey]*routeNode, 0), + Gateway: gateway, + } + } + return gatewaysIndex +} + +func initializeRoutes(routes []*gatewayapiv1.HTTPRoute) map[client.ObjectKey]*routeNode { + routesIndex := make(map[client.ObjectKey]*routeNode, 0) + for _, route := range routes { + routesIndex[client.ObjectKeyFromObject(route)] = &routeNode{ + Parents: make(map[client.ObjectKey]*gatewayNode, 0), + Route: route, + } + } + return routesIndex +} + +func initializePolicies(policies []KuadrantPolicy) map[client.ObjectKey]*policyNode { + policiesIndex := make(map[client.ObjectKey]*policyNode, 0) + for _, policy := range policies { + policiesIndex[client.ObjectKeyFromObject(policy)] = &policyNode{Policy: policy} + } + return policiesIndex +} diff --git a/pkg/common/kuadrant_topology_test.go b/pkg/common/kuadrant_topology_test.go new file mode 100644 index 000000000..64bb0d042 --- /dev/null +++ b/pkg/common/kuadrant_topology_test.go @@ -0,0 +1,432 @@ +//go:build unit + +package common + +import ( + "strings" + "testing" + + "gotest.tools/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" +) + +const ( + NS = "nsA" +) + +func TestPoliciesFromGateway(t *testing.T) { + t.Run("empty topology", func(subT *testing.T) { + topology := NewKuadrantTopology(nil, nil, nil) + + policies := topology.PoliciesFromGateway(testBasicGateway("gw1")) + assert.Equal(subT, len(policies), 0) + }) + + t.Run("unknown gateway", func(subT *testing.T) { + gateways := []*gatewayapiv1beta1.Gateway{ + testBasicGateway("gw1"), + testBasicGateway("gw2"), + } + + topology := NewKuadrantTopology(gateways, nil, nil) + + policies := topology.PoliciesFromGateway(testBasicGateway("unknown")) + assert.Equal(subT, len(policies), 0) + }) + + t.Run("invalid gateway is skipped", func(subT *testing.T) { + // route1 -> gw1 + // policy1 -> gw1 + // policy2 -> route1 + + invalidGateway := testInvalidGateway("gw1") + gateways := []*gatewayapiv1beta1.Gateway{invalidGateway} + + route1 := testBasicRoute("route1", invalidGateway) + routes := []*gatewayapiv1beta1.HTTPRoute{route1} + + gwPolicy := testBasicGatewayPolicy("policy1", invalidGateway) + routePolicy := testBasicRoutePolicy("policy2", route1) + policies := []KuadrantPolicy{gwPolicy, routePolicy} + + topology := NewKuadrantTopology(gateways, routes, policies) + + policiesFromGateway := topology.PoliciesFromGateway(invalidGateway) + assert.Equal(subT, len(policiesFromGateway), 0) + }) + + t.Run("gateway with direct policy", func(subT *testing.T) { + // route1 -> gw1 + // policy1 -> gw1 + + gw1 := testBasicGateway("gw1") + gateways := []*gatewayapiv1beta1.Gateway{gw1} + + route1 := testBasicRoute("route1", gw1) + routes := []*gatewayapiv1beta1.HTTPRoute{route1} + + gwPolicy := testBasicGatewayPolicy("policy1", gw1) + policies := []KuadrantPolicy{gwPolicy} + + topology := NewKuadrantTopology(gateways, routes, policies) + + policiesFromGateway := topology.PoliciesFromGateway(gw1) + assert.Equal(subT, len(policiesFromGateway), 1) + assert.Equal(subT, + client.ObjectKeyFromObject(policiesFromGateway[0]), + client.ObjectKeyFromObject(gwPolicy), + ) + }) + + t.Run("gateway with policies targeting routes", func(subT *testing.T) { + // route1 -> gw1 + // policy1 -> route1 + + gw1 := testBasicGateway("gw1") + gateways := []*gatewayapiv1beta1.Gateway{gw1} + + route1 := testBasicRoute("route1", gw1) + routes := []*gatewayapiv1beta1.HTTPRoute{route1} + + routePolicy := testBasicRoutePolicy("policy1", route1) + policies := []KuadrantPolicy{routePolicy} + + topology := NewKuadrantTopology(gateways, routes, policies) + + policiesFromGateway := topology.PoliciesFromGateway(gw1) + assert.Equal(subT, len(policiesFromGateway), 1) + assert.Equal(subT, + client.ObjectKeyFromObject(policiesFromGateway[0]), + client.ObjectKeyFromObject(routePolicy), + ) + }) + + t.Run("single policy targeting route with multiple parent gateways", func(subT *testing.T) { + // route1 -> gw1 + // route1 -> gw2 + // policy1 -> route1 + + gw1 := testBasicGateway("gw1") + gw2 := testBasicGateway("gw2") + gateways := []*gatewayapiv1beta1.Gateway{gw1, gw2} + + route1 := testBasicRoute("route1", gw1, gw2) + routes := []*gatewayapiv1beta1.HTTPRoute{route1} + + routePolicy := testBasicRoutePolicy("policy1", route1) + policies := []KuadrantPolicy{routePolicy} + + topology := NewKuadrantTopology(gateways, routes, policies) + + policiesGw1 := topology.PoliciesFromGateway(gw1) + assert.Equal(subT, len(policiesGw1), 1) + assert.Equal(subT, + client.ObjectKeyFromObject(policiesGw1[0]), + client.ObjectKeyFromObject(routePolicy), + ) + + policiesGw2 := topology.PoliciesFromGateway(gw2) + assert.Equal(subT, len(policiesGw2), 1) + assert.Equal(subT, + client.ObjectKeyFromObject(policiesGw2[0]), + client.ObjectKeyFromObject(routePolicy), + ) + }) +} + +func TestGetPolicyHTTPRoute(t *testing.T) { + t.Run("empty topology", func(subT *testing.T) { + // policy1 -> route1 + + route1 := testBasicRoute("route1", nil...) + policy := testBasicRoutePolicy("policy1", route1) + + topology := NewKuadrantTopology(nil, nil, nil) + + route := topology.GetPolicyHTTPRoute(policy) + assert.Assert(subT, route == nil) + }) + + t.Run("gateway with direct policy", func(subT *testing.T) { + // policy1 -> gw1 + + gw1 := testBasicGateway("gw1") + gateways := []*gatewayapiv1beta1.Gateway{gw1} + + gwPolicy := testBasicGatewayPolicy("policy1", gw1) + policies := []KuadrantPolicy{gwPolicy} + + topology := NewKuadrantTopology(gateways, nil, policies) + + route := topology.GetPolicyHTTPRoute(gwPolicy) + assert.Assert(subT, route == nil) + }) + + t.Run("route with direct policy", func(subT *testing.T) { + // route1 -> gw1 + // policy1 -> route1 + + gw1 := testBasicGateway("gw1") + gateways := []*gatewayapiv1beta1.Gateway{gw1} + + route1 := testBasicRoute("route1", gw1) + routes := []*gatewayapiv1beta1.HTTPRoute{route1} + + routePolicy := testBasicRoutePolicy("policy1", route1) + policies := []KuadrantPolicy{routePolicy} + + topology := NewKuadrantTopology(gateways, routes, policies) + + route := topology.GetPolicyHTTPRoute(routePolicy) + assert.Assert(subT, route != nil) + assert.Equal(subT, + client.ObjectKeyFromObject(route), + client.ObjectKeyFromObject(route1), + ) + }) +} + +func TestGetFreeRoutes(t *testing.T) { + t.Run("gateway without routes", func(subT *testing.T) { + // gw1 + // policy1 -> gw1 + gw1 := testBasicGateway("gw1") + gateways := []*gatewayapiv1beta1.Gateway{gw1} + + gatewayPolicy := testBasicGatewayPolicy("policy1", gw1) + policies := []KuadrantPolicy{gatewayPolicy} + + topology := NewKuadrantTopology(gateways, nil, policies) + + freeRoutes := topology.GetFreeRoutes(gw1) + assert.Equal(subT, len(freeRoutes), 0) + }) + + t.Run("all routes have policies", func(subT *testing.T) { + // gw1 + // route 1 -> gw1 + // route 2 -> gw1 + // policy1 -> route1 + // policy2 -> route1 + gw1 := testBasicGateway("gw1") + gateways := []*gatewayapiv1beta1.Gateway{gw1} + + route1 := testBasicRoute("route1", gw1) + route2 := testBasicRoute("route2", gw1) + routes := []*gatewayapiv1beta1.HTTPRoute{route1, route2} + + routePolicy1 := testBasicRoutePolicy("policy1", route1) + routePolicy2 := testBasicRoutePolicy("policy2", route2) + policies := []KuadrantPolicy{routePolicy1, routePolicy2} + + topology := NewKuadrantTopology(gateways, routes, policies) + + freeRoutes := topology.GetFreeRoutes(gw1) + assert.Equal(subT, len(freeRoutes), 0) + }) + + t.Run("only one route is free", func(subT *testing.T) { + // gw1 + // route 1 -> gw1 + // route 2 -> gw1 + // policy1 -> route1 + gw1 := testBasicGateway("gw1") + gateways := []*gatewayapiv1beta1.Gateway{gw1} + + route1 := testBasicRoute("route1", gw1) + route2 := testBasicRoute("route2", gw1) + routes := []*gatewayapiv1beta1.HTTPRoute{route1, route2} + + routePolicy1 := testBasicRoutePolicy("policy1", route1) + policies := []KuadrantPolicy{routePolicy1} + + topology := NewKuadrantTopology(gateways, routes, policies) + + freeRoutes := topology.GetFreeRoutes(gw1) + assert.Equal(subT, len(freeRoutes), 1) + assert.Equal(subT, + client.ObjectKeyFromObject(freeRoutes[0]), + client.ObjectKeyFromObject(route2), + ) + }) + + t.Run("all routes are free", func(subT *testing.T) { + // gw1 + // route 1 -> gw1 + // route 2 -> gw1 + gw1 := testBasicGateway("gw1") + gateways := []*gatewayapiv1beta1.Gateway{gw1} + + route1 := testBasicRoute("route1", gw1) + route2 := testBasicRoute("route2", gw1) + routes := []*gatewayapiv1beta1.HTTPRoute{route1, route2} + + topology := NewKuadrantTopology(gateways, routes, nil) + + freeRoutes := topology.GetFreeRoutes(gw1) + assert.Equal(subT, len(freeRoutes), 2) + }) +} + +func TestKuadrantTopologyString(t *testing.T) { + t.Run("empty topology", func(subT *testing.T) { + topology := NewKuadrantTopology(nil, nil, nil) + + topologyStr := topology.String() + assert.Assert(subT, strings.Contains(topologyStr, `"gateways": null`)) + assert.Assert(subT, strings.Contains(topologyStr, `"routes": null`)) + assert.Assert(subT, strings.Contains(topologyStr, `"policies": null`)) + assert.Assert(subT, strings.Contains(topologyStr, `"policiesPerGateway": null`)) + assert.Assert(subT, strings.Contains(topologyStr, `"policiesTargetingRoutes": null`)) + assert.Assert(subT, strings.Contains(topologyStr, `"freeRoutesPerGateway": null`)) + }) + + t.Run("1 gateway 1 route 1 policy for route", func(subT *testing.T) { + // route1 -> gw1 + // policy1 -> route1 + + gw1 := testBasicGateway("gw1") + gateways := []*gatewayapiv1beta1.Gateway{gw1} + + route1 := testBasicRoute("route1", gw1) + routes := []*gatewayapiv1beta1.HTTPRoute{route1} + + routePolicy := testBasicRoutePolicy("policy1", route1) + policies := []KuadrantPolicy{routePolicy} + + topology := NewKuadrantTopology(gateways, routes, policies) + + topologyStr := topology.String() + assert.Assert(subT, strings.Contains(topologyStr, `"gateways": [ + { + "id": "nsA/gw1", + "routes": [ + "nsA/route1" + ], + "policy": null + } + ]`)) + assert.Assert(subT, strings.Contains(topologyStr, `"routes": [ + { + "id": "nsA/route1", + "parents": [ + "nsA/gw1" + ], + "policy": "nsA/policy1" + } + ]`)) + assert.Assert(subT, strings.Contains(topologyStr, `"policies": [ + { + "id": "nsA/policy1", + "gateway": null, + "route": "nsA/route1" + } + ]`)) + assert.Assert(subT, strings.Contains(topologyStr, `"policiesPerGateway": { + "nsA/gw1": [ + "nsA/policy1" + ] + }`)) + assert.Assert(subT, strings.Contains(topologyStr, `"policiesTargetingRoutes": { + "nsA/policy1": "nsA/route1" + }`)) + assert.Assert(subT, strings.Contains(topologyStr, `"freeRoutesPerGateway": { + "nsA/gw1": [] + }`)) + }) +} + +func testBasicGateway(name string) *gatewayapiv1beta1.Gateway { + // Valid gateway + return &gatewayapiv1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: NS, + Name: name, + }, + Status: gatewayapiv1beta1.GatewayStatus{ + Conditions: []metav1.Condition{ + { + Type: GatewayProgrammedConditionType, + Status: metav1.ConditionTrue, + }, + }, + }, + } +} + +func testInvalidGateway(name string) *gatewayapiv1beta1.Gateway { + gw := testBasicGateway(name) + // remove conditions to make it invalid + gw.Status = gatewayapiv1beta1.GatewayStatus{} + + return gw +} + +func testBasicRoute(name string, parents ...*gatewayapiv1beta1.Gateway) *gatewayapiv1beta1.HTTPRoute { + parentRefs := make([]gatewayapiv1beta1.ParentReference, 0) + for _, val := range parents { + parentRefs = append(parentRefs, gatewayapiv1beta1.ParentReference{ + Group: &[]gatewayapiv1beta1.Group{"gateway.networking.k8s.io"}[0], + Kind: &[]gatewayapiv1beta1.Kind{"Gateway"}[0], + Namespace: &[]gatewayapiv1beta1.Namespace{gatewayapiv1beta1.Namespace(val.Namespace)}[0], + Name: gatewayapiv1beta1.ObjectName(val.Name), + }) + } + + parentStatusRefs := Map(parentRefs, func(p gatewayapiv1beta1.ParentReference) gatewayapiv1beta1.RouteParentStatus { + return gatewayapiv1beta1.RouteParentStatus{ + ParentRef: p, + Conditions: []metav1.Condition{{Type: "Accepted", Status: metav1.ConditionTrue}}, + } + }) + + return &gatewayapiv1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: NS, + Name: name, + }, + Spec: gatewayapiv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ + ParentRefs: parentRefs, + }, + }, + Status: gatewayapiv1beta1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1beta1.RouteStatus{ + Parents: parentStatusRefs, + }, + }, + } +} + +func testBasicGatewayPolicy(name string, gateway *gatewayapiv1beta1.Gateway) KuadrantPolicy { + return &TestPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: NS, + Name: name, + }, + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1beta1.Group("gateway.networking.k8s.io"), + Kind: gatewayapiv1beta1.Kind("Gateway"), + Namespace: &[]gatewayapiv1beta1.Namespace{gatewayapiv1beta1.Namespace(gateway.Namespace)}[0], + Name: gatewayapiv1beta1.ObjectName(gateway.Name), + }, + } +} + +func testBasicRoutePolicy(name string, route *gatewayapiv1beta1.HTTPRoute) KuadrantPolicy { + return &TestPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: NS, + Name: name, + }, + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1beta1.Group("gateway.networking.k8s.io"), + Kind: gatewayapiv1beta1.Kind("HTTPRoute"), + Namespace: &[]gatewayapiv1beta1.Namespace{gatewayapiv1beta1.Namespace(route.Namespace)}[0], + Name: gatewayapiv1beta1.ObjectName(route.Name), + }, + } +} From 0056fcc68d643d3ddceab929097d187aea7e6c08 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 23 Nov 2023 23:25:57 +0100 Subject: [PATCH 02/21] wasmplugin label selector is back --- controllers/rate_limiting_wasmplugin_controller.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go index c163e7128..a60a3485b 100644 --- a/controllers/rate_limiting_wasmplugin_controller.go +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -23,7 +23,6 @@ import ( "github.com/go-logr/logr" istioextensionsv1alpha1 "istio.io/api/extensions/v1alpha1" - istiov1beta1 "istio.io/api/type/v1beta1" istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -96,11 +95,7 @@ func (r *RateLimitingWASMPluginReconciler) desiredRateLimitingWASMPlugin(ctx con }, ObjectMeta: common.RateLimitingWASMPluginName(gw), Spec: istioextensionsv1alpha1.WasmPlugin{ - TargetRef: &istiov1beta1.PolicyTargetReference{ - Group: "gateway.networking.k8s.io", - Kind: "Gateway", - Name: gw.Name, - }, + Selector: common.IstioWorkloadSelectorFromGateway(ctx, r.Client(), gw), Url: rlptools.WASMFilterImageURL, PluginConfig: nil, // Insert plugin before Istio stats filters and after Istio authorization filters. From 90cd9e327d5d62b928a4c2a43c82493bf1c5c92c Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 24 Nov 2023 11:46:54 +0100 Subject: [PATCH 03/21] wasmplugin name from rlptools --- controllers/rate_limiting_wasmplugin_controller.go | 5 ++++- pkg/common/istio_utils.go | 9 --------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go index a60a3485b..49c39ecd4 100644 --- a/controllers/rate_limiting_wasmplugin_controller.go +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -93,7 +93,10 @@ func (r *RateLimitingWASMPluginReconciler) desiredRateLimitingWASMPlugin(ctx con Kind: "WasmPlugin", APIVersion: "extensions.istio.io/v1alpha1", }, - ObjectMeta: common.RateLimitingWASMPluginName(gw), + ObjectMeta: metav1.ObjectMeta{ + Name: rlptools.WASMPluginName(gw), + Namespace: gw.Namespace, + }, Spec: istioextensionsv1alpha1.WasmPlugin{ Selector: common.IstioWorkloadSelectorFromGateway(ctx, r.Client(), gw), Url: rlptools.WASMFilterImageURL, diff --git a/pkg/common/istio_utils.go b/pkg/common/istio_utils.go index a0a41b661..d263f401d 100644 --- a/pkg/common/istio_utils.go +++ b/pkg/common/istio_utils.go @@ -2,11 +2,9 @@ package common import ( "context" - "fmt" "github.com/go-logr/logr" istiocommon "istio.io/api/type/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" ) @@ -22,10 +20,3 @@ func IstioWorkloadSelectorFromGateway(ctx context.Context, k8sClient client.Clie MatchLabels: gatewayWorkloadSelector, } } - -func RateLimitingWASMPluginName(gw *gatewayapiv1.Gateway) metav1.ObjectMeta { - return metav1.ObjectMeta{ - Name: fmt.Sprintf("kuadrant-%s", gw.Name), - Namespace: gw.Namespace, - } -} From aeafadac0e51815638a54349c5f91d6de584b123 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 24 Nov 2023 11:51:19 +0100 Subject: [PATCH 04/21] fix unittests --- pkg/common/kuadrant_topology_test.go | 102 +++++++++++++-------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/pkg/common/kuadrant_topology_test.go b/pkg/common/kuadrant_topology_test.go index 64bb0d042..f9194bb40 100644 --- a/pkg/common/kuadrant_topology_test.go +++ b/pkg/common/kuadrant_topology_test.go @@ -9,8 +9,8 @@ import ( "gotest.tools/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" ) const ( @@ -26,7 +26,7 @@ func TestPoliciesFromGateway(t *testing.T) { }) t.Run("unknown gateway", func(subT *testing.T) { - gateways := []*gatewayapiv1beta1.Gateway{ + gateways := []*gatewayapiv1.Gateway{ testBasicGateway("gw1"), testBasicGateway("gw2"), } @@ -43,10 +43,10 @@ func TestPoliciesFromGateway(t *testing.T) { // policy2 -> route1 invalidGateway := testInvalidGateway("gw1") - gateways := []*gatewayapiv1beta1.Gateway{invalidGateway} + gateways := []*gatewayapiv1.Gateway{invalidGateway} route1 := testBasicRoute("route1", invalidGateway) - routes := []*gatewayapiv1beta1.HTTPRoute{route1} + routes := []*gatewayapiv1.HTTPRoute{route1} gwPolicy := testBasicGatewayPolicy("policy1", invalidGateway) routePolicy := testBasicRoutePolicy("policy2", route1) @@ -63,10 +63,10 @@ func TestPoliciesFromGateway(t *testing.T) { // policy1 -> gw1 gw1 := testBasicGateway("gw1") - gateways := []*gatewayapiv1beta1.Gateway{gw1} + gateways := []*gatewayapiv1.Gateway{gw1} route1 := testBasicRoute("route1", gw1) - routes := []*gatewayapiv1beta1.HTTPRoute{route1} + routes := []*gatewayapiv1.HTTPRoute{route1} gwPolicy := testBasicGatewayPolicy("policy1", gw1) policies := []KuadrantPolicy{gwPolicy} @@ -86,10 +86,10 @@ func TestPoliciesFromGateway(t *testing.T) { // policy1 -> route1 gw1 := testBasicGateway("gw1") - gateways := []*gatewayapiv1beta1.Gateway{gw1} + gateways := []*gatewayapiv1.Gateway{gw1} route1 := testBasicRoute("route1", gw1) - routes := []*gatewayapiv1beta1.HTTPRoute{route1} + routes := []*gatewayapiv1.HTTPRoute{route1} routePolicy := testBasicRoutePolicy("policy1", route1) policies := []KuadrantPolicy{routePolicy} @@ -111,10 +111,10 @@ func TestPoliciesFromGateway(t *testing.T) { gw1 := testBasicGateway("gw1") gw2 := testBasicGateway("gw2") - gateways := []*gatewayapiv1beta1.Gateway{gw1, gw2} + gateways := []*gatewayapiv1.Gateway{gw1, gw2} route1 := testBasicRoute("route1", gw1, gw2) - routes := []*gatewayapiv1beta1.HTTPRoute{route1} + routes := []*gatewayapiv1.HTTPRoute{route1} routePolicy := testBasicRoutePolicy("policy1", route1) policies := []KuadrantPolicy{routePolicy} @@ -154,7 +154,7 @@ func TestGetPolicyHTTPRoute(t *testing.T) { // policy1 -> gw1 gw1 := testBasicGateway("gw1") - gateways := []*gatewayapiv1beta1.Gateway{gw1} + gateways := []*gatewayapiv1.Gateway{gw1} gwPolicy := testBasicGatewayPolicy("policy1", gw1) policies := []KuadrantPolicy{gwPolicy} @@ -170,10 +170,10 @@ func TestGetPolicyHTTPRoute(t *testing.T) { // policy1 -> route1 gw1 := testBasicGateway("gw1") - gateways := []*gatewayapiv1beta1.Gateway{gw1} + gateways := []*gatewayapiv1.Gateway{gw1} route1 := testBasicRoute("route1", gw1) - routes := []*gatewayapiv1beta1.HTTPRoute{route1} + routes := []*gatewayapiv1.HTTPRoute{route1} routePolicy := testBasicRoutePolicy("policy1", route1) policies := []KuadrantPolicy{routePolicy} @@ -194,7 +194,7 @@ func TestGetFreeRoutes(t *testing.T) { // gw1 // policy1 -> gw1 gw1 := testBasicGateway("gw1") - gateways := []*gatewayapiv1beta1.Gateway{gw1} + gateways := []*gatewayapiv1.Gateway{gw1} gatewayPolicy := testBasicGatewayPolicy("policy1", gw1) policies := []KuadrantPolicy{gatewayPolicy} @@ -212,11 +212,11 @@ func TestGetFreeRoutes(t *testing.T) { // policy1 -> route1 // policy2 -> route1 gw1 := testBasicGateway("gw1") - gateways := []*gatewayapiv1beta1.Gateway{gw1} + gateways := []*gatewayapiv1.Gateway{gw1} route1 := testBasicRoute("route1", gw1) route2 := testBasicRoute("route2", gw1) - routes := []*gatewayapiv1beta1.HTTPRoute{route1, route2} + routes := []*gatewayapiv1.HTTPRoute{route1, route2} routePolicy1 := testBasicRoutePolicy("policy1", route1) routePolicy2 := testBasicRoutePolicy("policy2", route2) @@ -234,11 +234,11 @@ func TestGetFreeRoutes(t *testing.T) { // route 2 -> gw1 // policy1 -> route1 gw1 := testBasicGateway("gw1") - gateways := []*gatewayapiv1beta1.Gateway{gw1} + gateways := []*gatewayapiv1.Gateway{gw1} route1 := testBasicRoute("route1", gw1) route2 := testBasicRoute("route2", gw1) - routes := []*gatewayapiv1beta1.HTTPRoute{route1, route2} + routes := []*gatewayapiv1.HTTPRoute{route1, route2} routePolicy1 := testBasicRoutePolicy("policy1", route1) policies := []KuadrantPolicy{routePolicy1} @@ -258,11 +258,11 @@ func TestGetFreeRoutes(t *testing.T) { // route 1 -> gw1 // route 2 -> gw1 gw1 := testBasicGateway("gw1") - gateways := []*gatewayapiv1beta1.Gateway{gw1} + gateways := []*gatewayapiv1.Gateway{gw1} route1 := testBasicRoute("route1", gw1) route2 := testBasicRoute("route2", gw1) - routes := []*gatewayapiv1beta1.HTTPRoute{route1, route2} + routes := []*gatewayapiv1.HTTPRoute{route1, route2} topology := NewKuadrantTopology(gateways, routes, nil) @@ -289,10 +289,10 @@ func TestKuadrantTopologyString(t *testing.T) { // policy1 -> route1 gw1 := testBasicGateway("gw1") - gateways := []*gatewayapiv1beta1.Gateway{gw1} + gateways := []*gatewayapiv1.Gateway{gw1} route1 := testBasicRoute("route1", gw1) - routes := []*gatewayapiv1beta1.HTTPRoute{route1} + routes := []*gatewayapiv1.HTTPRoute{route1} routePolicy := testBasicRoutePolicy("policy1", route1) policies := []KuadrantPolicy{routePolicy} @@ -339,14 +339,14 @@ func TestKuadrantTopologyString(t *testing.T) { }) } -func testBasicGateway(name string) *gatewayapiv1beta1.Gateway { +func testBasicGateway(name string) *gatewayapiv1.Gateway { // Valid gateway - return &gatewayapiv1beta1.Gateway{ + return &gatewayapiv1.Gateway{ ObjectMeta: metav1.ObjectMeta{ Namespace: NS, Name: name, }, - Status: gatewayapiv1beta1.GatewayStatus{ + Status: gatewayapiv1.GatewayStatus{ Conditions: []metav1.Condition{ { Type: GatewayProgrammedConditionType, @@ -357,76 +357,76 @@ func testBasicGateway(name string) *gatewayapiv1beta1.Gateway { } } -func testInvalidGateway(name string) *gatewayapiv1beta1.Gateway { +func testInvalidGateway(name string) *gatewayapiv1.Gateway { gw := testBasicGateway(name) // remove conditions to make it invalid - gw.Status = gatewayapiv1beta1.GatewayStatus{} + gw.Status = gatewayapiv1.GatewayStatus{} return gw } -func testBasicRoute(name string, parents ...*gatewayapiv1beta1.Gateway) *gatewayapiv1beta1.HTTPRoute { - parentRefs := make([]gatewayapiv1beta1.ParentReference, 0) +func testBasicRoute(name string, parents ...*gatewayapiv1.Gateway) *gatewayapiv1.HTTPRoute { + parentRefs := make([]gatewayapiv1.ParentReference, 0) for _, val := range parents { - parentRefs = append(parentRefs, gatewayapiv1beta1.ParentReference{ - Group: &[]gatewayapiv1beta1.Group{"gateway.networking.k8s.io"}[0], - Kind: &[]gatewayapiv1beta1.Kind{"Gateway"}[0], - Namespace: &[]gatewayapiv1beta1.Namespace{gatewayapiv1beta1.Namespace(val.Namespace)}[0], - Name: gatewayapiv1beta1.ObjectName(val.Name), + parentRefs = append(parentRefs, gatewayapiv1.ParentReference{ + Group: &[]gatewayapiv1.Group{"gateway.networking.k8s.io"}[0], + Kind: &[]gatewayapiv1.Kind{"Gateway"}[0], + Namespace: &[]gatewayapiv1.Namespace{gatewayapiv1.Namespace(val.Namespace)}[0], + Name: gatewayapiv1.ObjectName(val.Name), }) } - parentStatusRefs := Map(parentRefs, func(p gatewayapiv1beta1.ParentReference) gatewayapiv1beta1.RouteParentStatus { - return gatewayapiv1beta1.RouteParentStatus{ + parentStatusRefs := Map(parentRefs, func(p gatewayapiv1.ParentReference) gatewayapiv1.RouteParentStatus { + return gatewayapiv1.RouteParentStatus{ ParentRef: p, Conditions: []metav1.Condition{{Type: "Accepted", Status: metav1.ConditionTrue}}, } }) - return &gatewayapiv1beta1.HTTPRoute{ + return &gatewayapiv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Namespace: NS, Name: name, }, - Spec: gatewayapiv1beta1.HTTPRouteSpec{ - CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{ + Spec: gatewayapiv1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapiv1.CommonRouteSpec{ ParentRefs: parentRefs, }, }, - Status: gatewayapiv1beta1.HTTPRouteStatus{ - RouteStatus: gatewayapiv1beta1.RouteStatus{ + Status: gatewayapiv1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1.RouteStatus{ Parents: parentStatusRefs, }, }, } } -func testBasicGatewayPolicy(name string, gateway *gatewayapiv1beta1.Gateway) KuadrantPolicy { +func testBasicGatewayPolicy(name string, gateway *gatewayapiv1.Gateway) KuadrantPolicy { return &TestPolicy{ ObjectMeta: metav1.ObjectMeta{ Namespace: NS, Name: name, }, TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1beta1.Group("gateway.networking.k8s.io"), - Kind: gatewayapiv1beta1.Kind("Gateway"), - Namespace: &[]gatewayapiv1beta1.Namespace{gatewayapiv1beta1.Namespace(gateway.Namespace)}[0], - Name: gatewayapiv1beta1.ObjectName(gateway.Name), + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: gatewayapiv1.Kind("Gateway"), + Namespace: &[]gatewayapiv1.Namespace{gatewayapiv1.Namespace(gateway.Namespace)}[0], + Name: gatewayapiv1.ObjectName(gateway.Name), }, } } -func testBasicRoutePolicy(name string, route *gatewayapiv1beta1.HTTPRoute) KuadrantPolicy { +func testBasicRoutePolicy(name string, route *gatewayapiv1.HTTPRoute) KuadrantPolicy { return &TestPolicy{ ObjectMeta: metav1.ObjectMeta{ Namespace: NS, Name: name, }, TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1beta1.Group("gateway.networking.k8s.io"), - Kind: gatewayapiv1beta1.Kind("HTTPRoute"), - Namespace: &[]gatewayapiv1beta1.Namespace{gatewayapiv1beta1.Namespace(route.Namespace)}[0], - Name: gatewayapiv1beta1.ObjectName(route.Name), + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: gatewayapiv1.Kind("HTTPRoute"), + Namespace: &[]gatewayapiv1.Namespace{gatewayapiv1.Namespace(route.Namespace)}[0], + Name: gatewayapiv1.ObjectName(route.Name), }, } } From 78e177331c78ce323b5cf2ae5cb553a07ae4effa Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 24 Nov 2023 12:40:03 +0100 Subject: [PATCH 05/21] wasmplugin controller: custom field indexer called from the wasmplugin controller --- .../rate_limiting_wasmplugin_controller.go | 55 ++++++++++++++++++- controllers/suite_test.go | 14 +++++ main.go | 9 +-- pkg/common/gatewayapi_utils.go | 43 --------------- 4 files changed, 69 insertions(+), 52 deletions(-) diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go index 49c39ecd4..22221b0d4 100644 --- a/controllers/rate_limiting_wasmplugin_controller.go +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -26,6 +26,7 @@ import ( istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -38,6 +39,10 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/rlptools/wasm" ) +const ( + HTTPRouteParents = ".metadata.parent" +) + // RateLimitingWASMPluginReconciler reconciles a WASMPlugin object for rate limiting type RateLimitingWASMPluginReconciler struct { reconcilers.TargetRefReconciler @@ -179,7 +184,7 @@ func (r *RateLimitingWASMPluginReconciler) gatewayAPITopologyFromGateway(ctx con routeList := &gatewayapiv1.HTTPRouteList{} // Get all the routes having the gateway as parent - err = r.Client().List(ctx, routeList, client.MatchingFields{common.HTTPRouteParents: client.ObjectKeyFromObject(gw).String()}) + err = r.Client().List(ctx, routeList, client.MatchingFields{HTTPRouteParents: client.ObjectKeyFromObject(gw).String()}) logger.V(1).Info("gatewayAPITopologyFromGateway: list httproutes from gateway", "err", err) if err != nil { return nil, err @@ -261,8 +266,56 @@ func (r *RateLimitingWASMPluginReconciler) RouteFromRLP(t *common.KuadrantTopolo return route } +// addHTTPRouteByGatewayIndexer declares an index key that we can later use with the client as a pseudo-field name, +// allowing to query all the routes parenting a given gateway +// to prevent creating the same index field multiple times, the function is declared private to be +// called ontly by this controller +func addHTTPRouteByGatewayIndexer(mgr ctrl.Manager, logger logr.Logger) error { + if err := mgr.GetFieldIndexer().IndexField(context.Background(), &gatewayapiv1.HTTPRoute{}, HTTPRouteParents, func(rawObj client.Object) []string { + // grab the route object, extract the parents + route, assertionOk := rawObj.(*gatewayapiv1.HTTPRoute) + logger.Info("assertionOK", "ok", assertionOk) + if !assertionOk { + return nil + } + + logger.Info("route", "name", client.ObjectKeyFromObject(route).String()) + + keys := make([]string, 0) + + for _, parentRef := range route.Spec.CommonRouteSpec.ParentRefs { + if !common.IsParentGateway(parentRef) { + logger.Info("parent not gateway", "ParentRefs", parentRef) + continue + } + + key := client.ObjectKey{ + Name: string(parentRef.Name), + Namespace: string(ptr.Deref(parentRef.Namespace, gatewayapiv1.Namespace(route.Namespace))), + } + + logger.Info("new key", "key", key.String()) + + keys = append(keys, key.String()) + } + + // ...and if so, return it + return keys + }); err != nil { + return err + } + + return nil +} + // SetupWithManager sets up the controller with the Manager. func (r *RateLimitingWASMPluginReconciler) SetupWithManager(mgr ctrl.Manager) error { + // Add custom indexer + err := addHTTPRouteByGatewayIndexer(mgr, r.Logger().WithName("routeByGatewayIndexer")) + if err != nil { + return err + } + httpRouteToParentGatewaysEventMapper := &common.HTTPRouteToParentGatewaysEventMapper{ Logger: r.Logger().WithName("httpRouteToParentGatewaysEventMapper"), } diff --git a/controllers/suite_test.go b/controllers/suite_test.go index c6b7bbf7f..d593d1aa5 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -231,6 +231,20 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) + rateLimitingWASMPluginBaseReconciler := reconcilers.NewBaseReconciler( + mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(), + log.Log.WithName("ratelimitpolicy").WithName("wasmplugin"), + mgr.GetEventRecorderFor("RateLimitingWASMPlugin"), + ) + + err = (&RateLimitingWASMPluginReconciler{ + TargetRefReconciler: reconcilers.TargetRefReconciler{ + BaseReconciler: rateLimitingWASMPluginBaseReconciler, + }, + }).SetupWithManager(mgr) + + Expect(err).NotTo(HaveOccurred()) + go func() { defer GinkgoRecover() err = mgr.Start(ctrl.SetupSignalHandler()) diff --git a/main.go b/main.go index da4fcdd83..394d3636b 100644 --- a/main.go +++ b/main.go @@ -58,7 +58,6 @@ import ( kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/controllers" - "github.com/kuadrant/kuadrant-operator/pkg/common" "github.com/kuadrant/kuadrant-operator/pkg/log" "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" //+kubebuilder:scaffold:imports @@ -140,12 +139,6 @@ func main() { os.Exit(1) } - err = common.AddHTTPRouteByGatewayIndexer(mgr, log.Log.WithName("routeByGatewayIndexer")) - if err != nil { - setupLog.Error(err, "unable to add indexer to the manager") - os.Exit(1) - } - kuadrantBaseReconciler := reconcilers.NewBaseReconciler( mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(), log.Log.WithName("kuadrant"), @@ -250,7 +243,7 @@ func main() { rateLimitingWASMPluginBaseReconciler := reconcilers.NewBaseReconciler( mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(), log.Log.WithName("ratelimitpolicy").WithName("wasmplugin"), - mgr.GetEventRecorderFor("GatewayKuadrant"), + mgr.GetEventRecorderFor("RateLimitingWASMPlugin"), ) if err = (&controllers.RateLimitingWASMPluginReconciler{ diff --git a/pkg/common/gatewayapi_utils.go b/pkg/common/gatewayapi_utils.go index 86a66bf50..1c20a44d2 100644 --- a/pkg/common/gatewayapi_utils.go +++ b/pkg/common/gatewayapi_utils.go @@ -8,12 +8,10 @@ import ( "slices" "strings" - "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -21,7 +19,6 @@ import ( const ( GatewayProgrammedConditionType = "Programmed" - HTTPRouteParents = ".metadata.parent" ) type HTTPRouteRule struct { @@ -657,46 +654,6 @@ func IsHTTPRouteAccepted(httpRoute *gatewayapiv1.HTTPRoute) bool { return true } -// AddHTTPRouteByGatewayIndexer declares an index key that we can later use with the client as a pseudo-field name, -// allowing to query all the routes parenting a given gateway -func AddHTTPRouteByGatewayIndexer(mgr ctrl.Manager, logger logr.Logger) error { - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &gatewayapiv1.HTTPRoute{}, HTTPRouteParents, func(rawObj client.Object) []string { - // grab the route object, extract the parents - route, assertionOk := rawObj.(*gatewayapiv1.HTTPRoute) - logger.Info("assertionOK", "ok", assertionOk) - if !assertionOk { - return nil - } - - logger.Info("route", "name", client.ObjectKeyFromObject(route).String()) - - keys := make([]string, 0) - - for _, parentRef := range route.Spec.CommonRouteSpec.ParentRefs { - if !IsParentGateway(parentRef) { - logger.Info("parent not gateway", "ParentRefs", parentRef) - continue - } - - key := client.ObjectKey{ - Name: string(parentRef.Name), - Namespace: string(ptr.Deref(parentRef.Namespace, gatewayapiv1.Namespace(route.Namespace))), - } - - logger.Info("new key", "key", key.String()) - - keys = append(keys, key.String()) - } - - // ...and if so, return it - return keys - }); err != nil { - return err - } - - return nil -} - func GetRouteAcceptedGatewayParentKeys(route *gatewayapiv1.HTTPRoute) []client.ObjectKey { if route == nil { return nil From 1699bf11893e16bb95f48eb5b7ffe6b0d301ff36 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 24 Nov 2023 14:26:52 +0100 Subject: [PATCH 06/21] wasmplugin controller: fix use case of rlp targeting gateway with no free routes --- .../rate_limiting_wasmplugin_controller.go | 42 ++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go index 22221b0d4..46dbfc8b7 100644 --- a/controllers/rate_limiting_wasmplugin_controller.go +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -164,7 +164,11 @@ func (r *RateLimitingWASMPluginReconciler) wasmPluginConfig(ctx context.Context, for _, policy := range rateLimitPolicies { rlp := policy.(*kuadrantv1beta2.RateLimitPolicy) - wasmRLP := r.WASMRateLimitPolicy(gatewayAPITopology, rlp, gw) + wasmRLP, err := r.WASMRateLimitPolicy(ctx, gatewayAPITopology, rlp, gw) + if err != nil { + return nil, err + } + if wasmRLP == nil { // skip this RLP continue @@ -206,16 +210,25 @@ func (r *RateLimitingWASMPluginReconciler) gatewayAPITopologyFromGateway(ctx con ), nil } -func (r *RateLimitingWASMPluginReconciler) WASMRateLimitPolicy(t *common.KuadrantTopology, rlp *kuadrantv1beta2.RateLimitPolicy, gw *gatewayapiv1.Gateway) *wasm.RateLimitPolicy { +func (r *RateLimitingWASMPluginReconciler) WASMRateLimitPolicy(ctx context.Context, t *common.KuadrantTopology, rlp *kuadrantv1beta2.RateLimitPolicy, gw *gatewayapiv1.Gateway) (*wasm.RateLimitPolicy, error) { gwHostnamesTmp := common.TargetHostnames(gw) gwHostnames := common.Map(gwHostnamesTmp, func(str string) gatewayapiv1.Hostname { return gatewayapiv1.Hostname(str) }) - route := r.RouteFromRLP(t, rlp, gw) + route, err := r.RouteFromRLP(ctx, t, rlp, gw) + if err != nil { + return nil, err + } + if route == nil { + // no need to add the policy if there are no routes; + // a rlp can return no rules if all its limits fail to match any route rule + // or targeting a gateway with no "free" routes. "free" meaning no route with policies targeting it + return nil, nil + } rules := rlptools.WasmRules(rlp, route) 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 - return nil + return nil, nil } // narrow the list of hostnames specified in the route so we don't generate wasm rules that only apply to other gateways @@ -231,10 +244,15 @@ func (r *RateLimitingWASMPluginReconciler) WASMRateLimitPolicy(t *common.Kuadran Hostnames: common.HostnamesToStrings(hostnames), // we might be listing more hostnames than needed due to route selectors hostnames possibly being more restrictive Service: common.KuadrantRateLimitClusterName, Rules: rules, - } + }, nil } -func (r *RateLimitingWASMPluginReconciler) RouteFromRLP(t *common.KuadrantTopology, rlp *kuadrantv1beta2.RateLimitPolicy, gw *gatewayapiv1.Gateway) *gatewayapiv1.HTTPRoute { +func (r *RateLimitingWASMPluginReconciler) RouteFromRLP(ctx context.Context, t *common.KuadrantTopology, rlp *kuadrantv1beta2.RateLimitPolicy, gw *gatewayapiv1.Gateway) (*gatewayapiv1.HTTPRoute, error) { + logger, err := logr.FromContext(ctx) + if err != nil { + return nil, err + } + route := t.GetPolicyHTTPRoute(rlp) if route == nil { @@ -244,9 +262,13 @@ func (r *RateLimitingWASMPluginReconciler) RouteFromRLP(t *common.KuadrantTopolo // Build imaginary route with all the routes not having a RLP targeting it freeRoutes := t.GetFreeRoutes(gw) - // For policies targeting a gateway, when no httproutes is attached to the gateway, skip wasm config - // test wasm config when no http routes attached to the gateway - //logger.V(1).Info("no httproutes attached to the targeted gateway, skipping wasm config for the gateway rlp", "ratelimitpolicy", gwRLPKey) + if len(freeRoutes) == 0 { + // For policies targeting a gateway, when no httproutes is attached to the gateway, skip wasm config + // test wasm config when no http routes attached to the gateway + logger.V(1).Info("no free httproutes attached to the targeted gateway, skipping wasm config for the gateway rlp", "ratelimitpolicy", client.ObjectKeyFromObject(rlp)) + return nil, nil + } + freeRules := make([]gatewayapiv1.HTTPRouteRule, 0) for idx := range freeRoutes { freeroute := freeRoutes[idx] @@ -263,7 +285,7 @@ func (r *RateLimitingWASMPluginReconciler) RouteFromRLP(t *common.KuadrantTopolo } } - return route + return route, nil } // addHTTPRouteByGatewayIndexer declares an index key that we can later use with the client as a pseudo-field name, From 65e6427a286921f58189f37bdef0a070cfd5831a Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Wed, 29 Nov 2023 17:33:05 +0100 Subject: [PATCH 07/21] wasmplugin controller: integration tests --- ...ate_limiting_wasmplugin_controller_test.go | 501 ++++++++++++++++++ .../ratelimitpolicy_controller_test.go | 430 +-------------- 2 files changed, 526 insertions(+), 405 deletions(-) create mode 100644 controllers/rate_limiting_wasmplugin_controller_test.go diff --git a/controllers/rate_limiting_wasmplugin_controller_test.go b/controllers/rate_limiting_wasmplugin_controller_test.go new file mode 100644 index 000000000..3f8b02b1b --- /dev/null +++ b/controllers/rate_limiting_wasmplugin_controller_test.go @@ -0,0 +1,501 @@ +//go:build integration + +package controllers + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" + "github.com/kuadrant/kuadrant-operator/pkg/common" + "github.com/kuadrant/kuadrant-operator/pkg/rlptools" + "github.com/kuadrant/kuadrant-operator/pkg/rlptools/wasm" +) + +var _ = Describe("Rate Limiting WasmPlugin controller", func() { + var ( + testNamespace string + gwName = "toystore-gw" + gateway *gatewayapiv1.Gateway + ) + + beforeEachCallback := func() { + CreateNamespace(&testNamespace) + gateway = testBuildBasicGateway(gwName, testNamespace) + err := k8sClient.Create(context.Background(), gateway) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) + ApplyKuadrantCR(testNamespace) + } + + BeforeEach(beforeEachCallback) + AfterEach(DeleteNamespaceCallback(&testNamespace)) + + Context("Basic tests", func() { + var ( + routeName = "toystore-route" + rlpName = "toystore-rlp" + ) + + It("Simple RLP targeting HTTPRoute creates wasmplugin", func() { + // create httproute + httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) + err := k8sClient.Create(context.Background(), httpRoute) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) + + // create ratelimitpolicy + rlp := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "HTTPRoute", + Name: gatewayapiv1.ObjectName(routeName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "l1": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlp) + Expect(err).ToNot(HaveOccurred()) + + // Check RLP status is available + rlpKey := client.ObjectKeyFromObject(rlp) + Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} + Eventually(testWasmPluginIsAvailable(wasmPluginKey), time.Minute, 5*time.Second).Should(BeTrue()) + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err = k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + // must exist + Expect(err).ToNot(HaveOccurred()) + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + Expect(err).ToNot(HaveOccurred()) + Expect(existingWASMConfig).To(Equal(&wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlpKey.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlp), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/toy", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.l1__2804bad6`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*.example.com"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + })) + }) + + It("Full featured RLP targeting HTTPRoute creates wasmplugin", func() { + // create httproute + httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.toystore.acme.com", "api.toystore.io"}) + httpRoute.Spec.Rules = []gatewayapiv1.HTTPRouteRule{ + { + Matches: []gatewayapiv1.HTTPRouteMatch{ + { // get /toys* + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/toys"), + }, + Method: ptr.To(gatewayapiv1.HTTPMethod("GET")), + }, + { // post /toys* + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/toys"), + }, + Method: ptr.To(gatewayapiv1.HTTPMethod("POST")), + }, + }, + }, + { + Matches: []gatewayapiv1.HTTPRouteMatch{ + { // /assets* + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/assets"), + }, + }, + }, + }, + } + err := k8sClient.Create(context.Background(), httpRoute) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) + + // create ratelimitpolicy + rlp := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", + APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: rlpName, + Namespace: testNamespace, + }, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "HTTPRoute", + Name: gatewayapiv1.ObjectName(routeName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "toys": { + Rates: []kuadrantv1beta2.Rate{ + {Limit: 50, Duration: 1, Unit: kuadrantv1beta2.TimeUnit("minute")}, + }, + Counters: []kuadrantv1beta2.ContextSelector{"auth.identity.username"}, + RouteSelectors: []kuadrantv1beta2.RouteSelector{ + { // selects the 1st HTTPRouteRule (i.e. get|post /toys*) for one of the hostnames + Matches: []gatewayapiv1.HTTPRouteMatch{ + { + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/toys"), + }, + }, + }, + Hostnames: []gatewayapiv1.Hostname{"*.toystore.acme.com"}, + }, + }, + When: []kuadrantv1beta2.WhenCondition{ + { + Selector: "auth.identity.group", + Operator: kuadrantv1beta2.WhenConditionOperator("neq"), + Value: "admin", + }, + }, + }, + "assets": { + Rates: []kuadrantv1beta2.Rate{ + {Limit: 5, Duration: 1, Unit: kuadrantv1beta2.TimeUnit("minute")}, + {Limit: 100, Duration: 12, Unit: kuadrantv1beta2.TimeUnit("hour")}, + }, + RouteSelectors: []kuadrantv1beta2.RouteSelector{ + { // selects the 2nd HTTPRouteRule (i.e. /assets*) for all hostnames + Matches: []gatewayapiv1.HTTPRouteMatch{ + { + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/assets"), + }, + }, + }, + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlp) + Expect(err).ToNot(HaveOccurred()) + + // Check RLP status is available + rlpKey := client.ObjectKeyFromObject(rlp) + Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} + Eventually(testWasmPluginIsAvailable(wasmPluginKey), time.Minute, 5*time.Second).Should(BeTrue()) + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err = k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + // must exist + Expect(err).ToNot(HaveOccurred()) + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + Expect(err).ToNot(HaveOccurred()) + Expect(existingWASMConfig.FailureMode).To(Equal(wasm.FailureModeDeny)) + Expect(existingWASMConfig.RateLimitPolicies).To(HaveLen(1)) + wasmRLP := existingWASMConfig.RateLimitPolicies[0] + Expect(wasmRLP.Name).To(Equal(rlpKey.String())) + Expect(wasmRLP.Domain).To(Equal(rlptools.LimitsNamespaceFromRLP(rlp))) + Expect(wasmRLP.Rules).To(ContainElement(wasm.Rule{ // rule to activate the 'toys' limit definition + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/toys", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + { + Selector: "request.host", + Operator: wasm.PatternOperator(kuadrantv1beta2.EndsWithOperator), + Value: ".toystore.acme.com", + }, + { + Selector: "auth.identity.group", + Operator: wasm.PatternOperator(kuadrantv1beta2.NotEqualOperator), + Value: "admin", + }, + }, + }, + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/toys", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "POST", + }, + { + Selector: "request.host", + Operator: wasm.PatternOperator(kuadrantv1beta2.EndsWithOperator), + Value: ".toystore.acme.com", + }, + { + Selector: "auth.identity.group", + Operator: wasm.PatternOperator(kuadrantv1beta2.NotEqualOperator), + Value: "admin", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: "limit.toys__3bfcbeee", + Value: "1", + }, + }, + { + Selector: &wasm.SelectorSpec{ + Selector: kuadrantv1beta2.ContextSelector("auth.identity.username"), + }, + }, + }, + })) + Expect(wasmRLP.Rules).To(ContainElement(wasm.Rule{ // rule to activate the 'assets' limit definition + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/assets", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: "limit.assets__8bf729ff", + Value: "1", + }, + }, + }, + })) + Expect(wasmRLP.Hostnames).To(Equal([]string{"*.toystore.acme.com", "api.toystore.io"})) + Expect(wasmRLP.Service).To(Equal(common.KuadrantRateLimitClusterName)) + }) + + It("Simple RLP targeting Gateway parenting one HTTPRoute creates wasmplugin", func() { + // create httproute + httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) + err := k8sClient.Create(context.Background(), httpRoute) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) + + // create ratelimitpolicy + rlp := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "Gateway", + Name: gatewayapiv1.ObjectName(gwName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "l1": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlp) + Expect(err).ToNot(HaveOccurred()) + + // Check RLP status is available + rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} + Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} + Eventually(testWasmPluginIsAvailable(wasmPluginKey), time.Minute, 5*time.Second).Should(BeTrue()) + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err = k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + // must exist + Expect(err).ToNot(HaveOccurred()) + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + Expect(err).ToNot(HaveOccurred()) + Expect(existingWASMConfig).To(Equal(&wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlpKey.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlp), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/toy", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.l1__2804bad6`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + })) + }) + }) + + Context("RLP targeting HTTPRoute-less Gateway", func() { + var ( + rlpName = "toystore-rlp" + ) + + It("Wasmplugin must not be created", func() { + // create ratelimitpolicy + rlp := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "Gateway", + Name: gatewayapiv1.ObjectName(gwName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "l1": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err := k8sClient.Create(context.Background(), rlp) + Expect(err).ToNot(HaveOccurred()) + + // Check RLP status is available + rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} + Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} + // Wait a bit to catch cases where wasmplugin is created and takes a bit to be created + Eventually(testWasmPluginIsAvailable(wasmPluginKey), 20*time.Second, 5*time.Second).Should(BeFalse()) + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + // must not exist + err = k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }) + }) +}) + +func testWasmPluginIsAvailable(key client.ObjectKey) func() bool { + return func() bool { + wp := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), key, wp) + if err != nil { + return false + } + + // Unfortunately, WasmPlugin does not have status yet + // Leaving this here for future use + //if !meta.IsStatusConditionTrue(wp.Status.Conditions, "Available") { + // return false + //} + + return true + } +} diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index a8b04c8d4..52fd7a8a1 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/controllers/ratelimitpolicy_controller_test.go @@ -75,37 +75,8 @@ var _ = Describe("RateLimitPolicy controller", func() { gateway = testBuildBasicGateway(gwName, testNamespace) err := k8sClient.Create(context.Background(), gateway) Expect(err).ToNot(HaveOccurred()) - - Eventually(func() bool { - existingGateway := &gatewayapiv1.Gateway{} - err := k8sClient.Get(context.Background(), client.ObjectKeyFromObject(gateway), existingGateway) - if err != nil { - logf.Log.V(1).Info("[WARN] Creating gateway failed", "error", err) - return false - } - - if meta.IsStatusConditionFalse(existingGateway.Status.Conditions, common.GatewayProgrammedConditionType) { - logf.Log.V(1).Info("[WARN] Gateway not ready") - return false - } - - return true - }, 15*time.Second, 5*time.Second).Should(BeTrue()) - + Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) ApplyKuadrantCR(testNamespace) - - // Check Limitador Status is Ready - Eventually(func() bool { - limitador := &limitadorv1alpha1.Limitador{} - err := k8sClient.Get(context.Background(), client.ObjectKey{Name: common.LimitadorName, Namespace: testNamespace}, limitador) - if err != nil { - return false - } - if !meta.IsStatusConditionTrue(limitador.Status.Conditions, "Ready") { - return false - } - return true - }, time.Minute, 5*time.Second).Should(BeTrue()) } BeforeEach(beforeEachCallback) @@ -152,55 +123,6 @@ var _ = Describe("RateLimitPolicy controller", func() { Name: rlptools.LimitsNameFromRLP(rlp), })) - // Check wasm plugin - wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} - Eventually(testWasmPluginIsAvailable(wasmPluginKey), time.Minute, 5*time.Second).Should(BeTrue()) - existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} - err = k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) - // must exist - Expect(err).ToNot(HaveOccurred()) - existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) - Expect(err).ToNot(HaveOccurred()) - Expect(existingWASMConfig).To(Equal(&wasm.Plugin{ - FailureMode: wasm.FailureModeDeny, - RateLimitPolicies: []wasm.RateLimitPolicy{ - { - Name: rlpKey.String(), - Domain: rlptools.LimitsNamespaceFromRLP(rlp), - Rules: []wasm.Rule{ - { - Conditions: []wasm.Condition{ - { - AllOf: []wasm.PatternExpression{ - { - Selector: "request.url_path", - Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), - Value: "/toy", - }, - { - Selector: "request.method", - Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), - Value: "GET", - }, - }, - }, - }, - Data: []wasm.DataItem{ - { - Static: &wasm.StaticSpec{ - Key: `limit.l1__2804bad6`, - Value: "1", - }, - }, - }, - }, - }, - Hostnames: []string{"*.example.com"}, - Service: common.KuadrantRateLimitClusterName, - }, - }, - })) - // Check gateway back references gwKey := client.ObjectKey{Name: gwName, Namespace: testNamespace} existingGateway := &gatewayapiv1.Gateway{} @@ -213,218 +135,43 @@ var _ = Describe("RateLimitPolicy controller", func() { Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue( common.RateLimitPoliciesBackRefAnnotation, string(serialized))) }) + }) - It("Creates the correct WasmPlugin for a complex HTTPRoute and a RateLimitPolicy", func() { + Context("RLP targeting Gateway", func() { + It("Creates all the resources for a basic Gateway and RateLimitPolicy", func() { // create httproute - httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.toystore.acme.com", "api.toystore.io"}) - httpRoute.Spec.Rules = []gatewayapiv1.HTTPRouteRule{ - { - Matches: []gatewayapiv1.HTTPRouteMatch{ - { // get /toys* - Path: &gatewayapiv1.HTTPPathMatch{ - Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), - Value: ptr.To("/toys"), - }, - Method: ptr.To(gatewayapiv1.HTTPMethod("GET")), - }, - { // post /toys* - Path: &gatewayapiv1.HTTPPathMatch{ - Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), - Value: ptr.To("/toys"), - }, - Method: ptr.To(gatewayapiv1.HTTPMethod("POST")), - }, - }, - }, - { - Matches: []gatewayapiv1.HTTPRouteMatch{ - { // /assets* - Path: &gatewayapiv1.HTTPPathMatch{ - Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), - Value: ptr.To("/assets"), - }, - }, - }, - }, - } + httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) err := k8sClient.Create(context.Background(), httpRoute) Expect(err).ToNot(HaveOccurred()) Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) // create ratelimitpolicy - rlp := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { - policy.Spec.Limits = map[string]kuadrantv1beta2.Limit{ - "toys": { - Rates: []kuadrantv1beta2.Rate{ - {Limit: 50, Duration: 1, Unit: kuadrantv1beta2.TimeUnit("minute")}, - }, - Counters: []kuadrantv1beta2.ContextSelector{"auth.identity.username"}, - RouteSelectors: []kuadrantv1beta2.RouteSelector{ - { // selects the 1st HTTPRouteRule (i.e. get|post /toys*) for one of the hostnames - Matches: []gatewayapiv1.HTTPRouteMatch{ - { - Path: &gatewayapiv1.HTTPPathMatch{ - Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), - Value: ptr.To("/toys"), - }, - }, - }, - Hostnames: []gatewayapiv1.Hostname{"*.toystore.acme.com"}, - }, - }, - When: []kuadrantv1beta2.WhenCondition{ - { - Selector: "auth.identity.group", - Operator: kuadrantv1beta2.WhenConditionOperator("neq"), - Value: "admin", - }, - }, - }, - "assets": { - Rates: []kuadrantv1beta2.Rate{ - {Limit: 5, Duration: 1, Unit: kuadrantv1beta2.TimeUnit("minute")}, - {Limit: 100, Duration: 12, Unit: kuadrantv1beta2.TimeUnit("hour")}, - }, - RouteSelectors: []kuadrantv1beta2.RouteSelector{ - { // selects the 2nd HTTPRouteRule (i.e. /assets*) for all hostnames - Matches: []gatewayapiv1.HTTPRouteMatch{ - { - Path: &gatewayapiv1.HTTPPathMatch{ - Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), - Value: ptr.To("/assets"), - }, - }, - }, - }, - }, - }, - } - }) - err = k8sClient.Create(context.Background(), rlp) - Expect(err).ToNot(HaveOccurred()) - - // Check RLP status is available - rlpKey := client.ObjectKeyFromObject(rlp) - Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) - - // Check wasm plugin - wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} - Eventually(testWasmPluginIsAvailable(wasmPluginKey), time.Minute, 5*time.Second).Should(BeTrue()) - existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} - err = k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) - // must exist - Expect(err).ToNot(HaveOccurred()) - existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) - Expect(err).ToNot(HaveOccurred()) - Expect(existingWASMConfig.FailureMode).To(Equal(wasm.FailureModeDeny)) - Expect(existingWASMConfig.RateLimitPolicies).To(HaveLen(1)) - wasmRLP := existingWASMConfig.RateLimitPolicies[0] - Expect(wasmRLP.Name).To(Equal(rlpKey.String())) - Expect(wasmRLP.Domain).To(Equal(rlptools.LimitsNamespaceFromRLP(rlp))) - Expect(wasmRLP.Rules).To(ContainElement(wasm.Rule{ // rule to activate the 'toys' limit definition - Conditions: []wasm.Condition{ - { - AllOf: []wasm.PatternExpression{ - { - Selector: "request.url_path", - Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), - Value: "/toys", - }, - { - Selector: "request.method", - Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), - Value: "GET", - }, - { - Selector: "request.host", - Operator: wasm.PatternOperator(kuadrantv1beta2.EndsWithOperator), - Value: ".toystore.acme.com", - }, - { - Selector: "auth.identity.group", - Operator: wasm.PatternOperator(kuadrantv1beta2.NotEqualOperator), - Value: "admin", - }, - }, - }, - { - AllOf: []wasm.PatternExpression{ - { - Selector: "request.url_path", - Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), - Value: "/toys", - }, - { - Selector: "request.method", - Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), - Value: "POST", - }, - { - Selector: "request.host", - Operator: wasm.PatternOperator(kuadrantv1beta2.EndsWithOperator), - Value: ".toystore.acme.com", - }, - { - Selector: "auth.identity.group", - Operator: wasm.PatternOperator(kuadrantv1beta2.NotEqualOperator), - Value: "admin", - }, - }, - }, + rlp := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", + APIVersion: kuadrantv1beta2.GroupVersion.String(), }, - Data: []wasm.DataItem{ - { - Static: &wasm.StaticSpec{ - Key: "limit.toys__3bfcbeee", - Value: "1", - }, - }, - { - Selector: &wasm.SelectorSpec{ - Selector: kuadrantv1beta2.ContextSelector("auth.identity.username"), - }, - }, + ObjectMeta: metav1.ObjectMeta{ + Name: rlpName, + Namespace: testNamespace, }, - })) - Expect(wasmRLP.Rules).To(ContainElement(wasm.Rule{ // rule to activate the 'assets' limit definition - Conditions: []wasm.Condition{ - { - AllOf: []wasm.PatternExpression{ - { - Selector: "request.url_path", - Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), - Value: "/assets", - }, - }, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "Gateway", + Name: gatewayapiv1.ObjectName(gwName), }, - }, - Data: []wasm.DataItem{ - { - Static: &wasm.StaticSpec{ - Key: "limit.assets__8bf729ff", - Value: "1", + Limits: map[string]kuadrantv1beta2.Limit{ + "l1": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, }, }, }, - })) - Expect(wasmRLP.Hostnames).To(Equal([]string{"*.toystore.acme.com", "api.toystore.io"})) - Expect(wasmRLP.Service).To(Equal(common.KuadrantRateLimitClusterName)) - }) - }) - - Context("RLP targeting Gateway", func() { - It("Creates all the resources for a basic Gateway and RateLimitPolicy", func() { - // create httproute - httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) - err := k8sClient.Create(context.Background(), httpRoute) - Expect(err).ToNot(HaveOccurred()) - Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) - - // create ratelimitpolicy - rlp := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { - policy.Spec.TargetRef.Kind = "Gateway" - policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gwName) - }) + } err = k8sClient.Create(context.Background(), rlp) Expect(err).ToNot(HaveOccurred()) @@ -456,55 +203,6 @@ var _ = Describe("RateLimitPolicy controller", func() { Name: rlptools.LimitsNameFromRLP(rlp), })) - // Check wasm plugin - wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} - Eventually(testWasmPluginIsAvailable(wasmPluginKey), time.Minute, 5*time.Second).Should(BeTrue()) - existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} - err = k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) - // must exist - Expect(err).ToNot(HaveOccurred()) - existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) - Expect(err).ToNot(HaveOccurred()) - Expect(existingWASMConfig).To(Equal(&wasm.Plugin{ - FailureMode: wasm.FailureModeDeny, - RateLimitPolicies: []wasm.RateLimitPolicy{ - { - Name: rlpKey.String(), - Domain: rlptools.LimitsNamespaceFromRLP(rlp), - Rules: []wasm.Rule{ - { - Conditions: []wasm.Condition{ - { - AllOf: []wasm.PatternExpression{ - { - Selector: "request.url_path", - Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), - Value: "/toy", - }, - { - Selector: "request.method", - Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), - Value: "GET", - }, - }, - }, - }, - Data: []wasm.DataItem{ - { - Static: &wasm.StaticSpec{ - Key: `limit.l1__2804bad6`, - Value: "1", - }, - }, - }, - }, - }, - Hostnames: []string{"*"}, - Service: common.KuadrantRateLimitClusterName, - }, - }, - })) - // Check gateway back references err = k8sClient.Get(context.Background(), gwKey, existingGateway) // must exist @@ -552,15 +250,6 @@ var _ = Describe("RateLimitPolicy controller", func() { Name: rlptools.LimitsNameFromRLP(rlp), })) - // Check wasm plugin - wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} - // Wait a bit to catch cases where wasmplugin is created and takes a bit to be created - Eventually(testWasmPluginIsAvailable(wasmPluginKey), 20*time.Second, 5*time.Second).Should(BeFalse()) - existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} - // must not exist - err = k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) - Expect(apierrors.IsNotFound(err)).To(BeTrue()) - // Check gateway back references err = k8sClient.Get(context.Background(), gwKey, existingGateway) // must exist @@ -571,75 +260,6 @@ var _ = Describe("RateLimitPolicy controller", func() { Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(common.RateLimitPoliciesBackRefAnnotation, string(serialized))) }) }) - - Context("RLP accepted condition reasons", func() { - assertAcceptedConditionFalse := func(rlp *kuadrantv1beta2.RateLimitPolicy, reason, message string) func() bool { - return func() bool { - rlpKey := client.ObjectKeyFromObject(rlp) - existingRLP := &kuadrantv1beta2.RateLimitPolicy{} - err := k8sClient.Get(context.Background(), rlpKey, existingRLP) - if err != nil { - return false - } - - cond := meta.FindStatusCondition(existingRLP.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)) - if cond == nil { - return false - } - - return cond.Status == metav1.ConditionFalse && cond.Reason == reason && cond.Message == message - } - } - - // Accepted reason is already tested generally by the existing tests - - It("Target not found reason", func() { - rlp := policyFactory() - err := k8sClient.Create(context.Background(), rlp) - Expect(err).ToNot(HaveOccurred()) - - Eventually(assertAcceptedConditionFalse(rlp, string(gatewayapiv1alpha2.PolicyReasonTargetNotFound), - fmt.Sprintf("RateLimitPolicy target %s was not found", routeName)), - time.Minute, 5*time.Second).Should(BeTrue()) - }) - - It("Conflict reason", func() { - httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) - err := k8sClient.Create(context.Background(), httpRoute) - Expect(err).ToNot(HaveOccurred()) - Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) - - rlp := policyFactory() - err = k8sClient.Create(context.Background(), rlp) - Expect(err).ToNot(HaveOccurred()) - - rlp2 := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { - policy.Name = "conflicting-rlp" - }) - err = k8sClient.Create(context.Background(), rlp2) - Expect(err).ToNot(HaveOccurred()) - - Eventually(assertAcceptedConditionFalse(rlp2, string(gatewayapiv1alpha2.PolicyReasonConflicted), - fmt.Sprintf("RateLimitPolicy is conflicted by %[1]v/toystore-rlp: the gateway.networking.k8s.io/v1, Kind=HTTPRoute target %[1]v/toystore-route is already referenced by policy %[1]v/toystore-rlp", testNamespace)), - time.Minute, 5*time.Second).Should(BeTrue()) - }) - - It("Validation reason", func() { - const targetRefName, targetRefNamespace = "istio-ingressgateway", "istio-system" - - rlp := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) { - policy.Spec.TargetRef.Kind = "Gateway" - policy.Spec.TargetRef.Name = targetRefName - policy.Spec.TargetRef.Namespace = ptr.To(gatewayapiv1.Namespace(targetRefNamespace)) - }) - err := k8sClient.Create(context.Background(), rlp) - Expect(err).ToNot(HaveOccurred()) - - Eventually(assertAcceptedConditionFalse(rlp, string(gatewayapiv1alpha2.PolicyReasonInvalid), - fmt.Sprintf("RateLimitPolicy target is invalid: invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", targetRefNamespace)), - time.Minute, 5*time.Second).Should(BeTrue()) - }) - }) }) var _ = Describe("RateLimitPolicy CEL Validations", func() { From 4546226e2efef8008ad783f04bc010de13786d31 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Tue, 12 Dec 2023 16:57:45 +0100 Subject: [PATCH 08/21] wasmplugin controller: integration tests --- ...ate_limiting_wasmplugin_controller_test.go | 243 ++++++++++++++++++ 1 file changed, 243 insertions(+) diff --git a/controllers/rate_limiting_wasmplugin_controller_test.go b/controllers/rate_limiting_wasmplugin_controller_test.go index 3f8b02b1b..79e2115d8 100644 --- a/controllers/rate_limiting_wasmplugin_controller_test.go +++ b/controllers/rate_limiting_wasmplugin_controller_test.go @@ -4,6 +4,7 @@ package controllers import ( "context" + "reflect" "time" . "github.com/onsi/ginkgo/v2" @@ -480,6 +481,248 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { Expect(apierrors.IsNotFound(err)).To(BeTrue()) }) }) + + Context("RLP targeting HTTPRoute when route selection match is empty", func() { + var ( + routeName = "toystore-route" + rlpName = "toystore-rlp" + ) + + It("When the gateway does not have more policies, the wasmplugin resource is not created", func() { + // create httproute + httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) + err := k8sClient.Create(context.Background(), httpRoute) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) + + // create ratelimitpolicy with no matching routes + rlp := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "HTTPRoute", + Name: gatewayapiv1.ObjectName(routeName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "l1": { + RouteSelectors: []kuadrantv1beta2.RouteSelector{ + { // does no select any HTTPRouteRule (i.e. GET /toys*) + Matches: []gatewayapiv1.HTTPRouteMatch{ + { + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/other"), + }, + }, + }, + }, + }, + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlp) + Expect(err).ToNot(HaveOccurred()) + + // Check RLP status is available + rlpKey := client.ObjectKeyFromObject(rlp) + Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} + // Wait a bit to catch cases where wasmplugin is created and takes a bit to be created + Eventually(testWasmPluginIsAvailable(wasmPluginKey), 20*time.Second, 5*time.Second).Should(BeFalse()) + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + // must not exist + err = k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }) + + It("When the gateway has more policies, the wasmplugin resource does not have any configuration regarding the current RLP", func() { + // Gw A + // Route B -> Gw A + // RLP A -> Gw A + // Route C -> GW A + // RLP B -> Route C (however, no matching routes) + + var ( + routeBName = "toystore-b" + routeCName = "toystore-c" + rlpAName = "toystore-a" + rlpBName = "toystore-b" + ) + + // create httproute B + httpRouteB := testBuildBasicHttpRoute(routeBName, gwName, testNamespace, []string{"*.b.example.com"}) + err := k8sClient.Create(context.Background(), httpRouteB) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteB)), time.Minute, 5*time.Second).Should(BeTrue()) + + // create RLP A -> Gw A + rlpA := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlpAName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "Gateway", + Name: gatewayapiv1.ObjectName(gwName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "l1": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlpA) + Expect(err).ToNot(HaveOccurred()) + + // Check RLP status is available + rlpAKey := client.ObjectKey{Name: rlpAName, Namespace: testNamespace} + Eventually(testRLPIsAvailable(rlpAKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // create httproute C + httpRouteC := testBuildBasicHttpRoute(routeCName, gwName, testNamespace, []string{"*.c.example.com"}) + httpRouteC.Spec.Rules = []gatewayapiv1.HTTPRouteRule{ + { + Matches: []gatewayapiv1.HTTPRouteMatch{ + { + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/otherPathRouteC"), + }, + Method: ptr.To(gatewayapiv1.HTTPMethod("GET")), + }, + }, + }, + } + + err = k8sClient.Create(context.Background(), httpRouteC) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteC)), time.Minute, 5*time.Second).Should(BeTrue()) + + // create RLP B -> Route C (however, no matching routes) + rlpB := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlpBName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "HTTPRoute", + Name: gatewayapiv1.ObjectName(routeCName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "l1": { + RouteSelectors: []kuadrantv1beta2.RouteSelector{ + { // does no select any HTTPRouteRule (i.e. GET /otherPathRouteC*) + Matches: []gatewayapiv1.HTTPRouteMatch{ + { + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/notmatchingpath"), + }, + }, + }, + }, + }, + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlpB) + Expect(err).ToNot(HaveOccurred()) + + // Check RLP status is available + rlpBKey := client.ObjectKey{Name: rlpBName, Namespace: testNamespace} + Eventually(testRLPIsAvailable(rlpBKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin only has configuration ONLY from the RLP targeting the gateway + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{ + Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace, + } + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err != nil { + return false + } + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + if err != nil { + return false + } + + expectedPlugin := &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlpAKey.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlpA), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/toy", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.l1__2804bad6`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + } + + if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + return false + } + + return true + }, time.Minute, 5*time.Second).Should(BeTrue()) + }) + }) }) func testWasmPluginIsAvailable(key client.ObjectKey) func() bool { From 6a1e30e45f298d023552be09386dc6502f406ae5 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Wed, 13 Dec 2023 17:50:06 +0100 Subject: [PATCH 09/21] controllers/ratelimitpolicy_controller_test.go: fix imports --- controllers/ratelimitpolicy_controller_test.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index 52fd7a8a1..9f1b07151 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/controllers/ratelimitpolicy_controller_test.go @@ -9,22 +9,19 @@ import ( "strings" "time" - kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" - "github.com/kuadrant/kuadrant-operator/pkg/common" - "github.com/kuadrant/kuadrant-operator/pkg/rlptools" - "github.com/kuadrant/kuadrant-operator/pkg/rlptools/wasm" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" - 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" "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" + "github.com/kuadrant/kuadrant-operator/pkg/common" + "github.com/kuadrant/kuadrant-operator/pkg/rlptools" ) var _ = Describe("RateLimitPolicy controller", func() { From 990f2b6aaf983f7316e62a8d5173327d946c3964 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Wed, 13 Dec 2023 22:32:00 +0100 Subject: [PATCH 10/21] wasmplugin controller: integration tests --- ...ate_limiting_wasmplugin_controller_test.go | 182 ++++++++++++++++++ 1 file changed, 182 insertions(+) diff --git a/controllers/rate_limiting_wasmplugin_controller_test.go b/controllers/rate_limiting_wasmplugin_controller_test.go index 79e2115d8..11bb79a3f 100644 --- a/controllers/rate_limiting_wasmplugin_controller_test.go +++ b/controllers/rate_limiting_wasmplugin_controller_test.go @@ -723,6 +723,188 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { }, time.Minute, 5*time.Second).Should(BeTrue()) }) }) + + Context("HTTPRoute switches parentship from one gateway to another", func() { + var ( + routeName = "route-a" + rlpName = "rlp-a" + gwBName = "gw-b" + ) + + It("RLP targeting a gateway, GwA should not have wasmplugin and GwB should have wasmplugin", func() { + // Initial state + // Gw A + // Gw B + // RLP A -> Gw A + // Route A -> Gw A + // + // Switch parentship + // Gw A + // Gw B + // RLP A -> Gw A + // Route A -> Gw B + + // Gw A will be the pre-existing $gateway with name $gwName + + // create RLP A -> Gw A + rlpA := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "Gateway", + Name: gatewayapiv1.ObjectName(gwName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "l1": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err := k8sClient.Create(context.Background(), rlpA) + Expect(err).ToNot(HaveOccurred()) + // Check RLP status is available + rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} + Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // create Route A -> Gw A + httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) + err = k8sClient.Create(context.Background(), httpRoute) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) + + // create Gateway B + gwB := testBuildBasicGateway(gwBName, testNamespace) + err = k8sClient.Create(context.Background(), gwB) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gwB), 30*time.Second, 5*time.Second).Should(BeTrue()) + + // Initial state set. + // Check wasm plugin for gateway A has configuration from the route + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{ + Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace, + } + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err != nil { + return false + } + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + if err != nil { + return false + } + + expectedPlugin := &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlpKey.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlpA), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/toy", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.l1__2804bad6`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + } + + if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + return false + } + + return true + }, time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin for gateway B does not exist + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + // Check wasm plugin + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gwB), Namespace: testNamespace} + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + return apierrors.IsNotFound(err) + }) + + // Proceed with the update: + // From Route A -> Gw A + // To Route A -> Gw B + httpRouteUpdated := &gatewayapiv1.HTTPRoute{} + err = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(httpRoute), httpRouteUpdated) + Expect(err).ToNot(HaveOccurred()) + httpRouteUpdated.Spec.CommonRouteSpec.ParentRefs[0].Name = gatewayapiv1.ObjectName(gwBName) + err = k8sClient.Update(context.Background(), httpRouteUpdated) + Expect(err).ToNot(HaveOccurred()) + + // Check wasm plugin for gateway A no longer exists + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + return apierrors.IsNotFound(err) + }) + + // Check wasm plugin for gateway B does not exist + // There is not RLP targeting Gateway B or any route parenting Gateway B + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gwB), Namespace: testNamespace} + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + return apierrors.IsNotFound(err) + }) + }) + + It("RLP targeting a route, GwA should not have wasmplugin and GwB should have wasmplugin", func() { + // Initial state + // Gw A + // Gw B + // Route A -> Gw A + // RLP A -> Route A + // + // Switch parentship + // Gw A + // Gw B + // Route A -> Gw B + // RLP A -> Route A + }) + }) }) func testWasmPluginIsAvailable(key client.ObjectKey) func() bool { From aab39f2aaaa43309d05cc459f92f646788fb551f Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 14 Dec 2023 10:36:09 +0100 Subject: [PATCH 11/21] wasmplugin controller: integration tests --- controllers/helper_test.go | 43 ++- ...ate_limiting_wasmplugin_controller_test.go | 271 +++++++++++++++++- .../ratelimitpolicy_controller_test.go | 1 + 3 files changed, 299 insertions(+), 16 deletions(-) diff --git a/controllers/helper_test.go b/controllers/helper_test.go index fe7c2500d..0c9095e48 100644 --- a/controllers/helper_test.go +++ b/controllers/helper_test.go @@ -16,7 +16,6 @@ import ( certmanmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "github.com/google/uuid" . "github.com/onsi/gomega" - istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" @@ -35,7 +34,6 @@ import ( gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" - kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/pkg/common" @@ -308,7 +306,18 @@ func testRouteIsAccepted(routeKey client.ObjectKey) func() bool { return func() bool { route := &gatewayapiv1.HTTPRoute{} err := k8sClient.Get(context.Background(), routeKey, route) - return err == nil && common.IsHTTPRouteAccepted(route) + + if err != nil { + logf.Log.V(1).Info("httpRoute not read", "route", routeKey, "error", err) + return false + } + + if !common.IsHTTPRouteAccepted(route) { + logf.Log.V(1).Info("httpRoute not accepted", "route", routeKey) + return false + } + + return true } } @@ -316,18 +325,13 @@ func testGatewayIsReady(gateway *gatewayapiv1.Gateway) func() bool { return func() bool { existingGateway := &gatewayapiv1.Gateway{} err := k8sClient.Get(context.Background(), client.ObjectKeyFromObject(gateway), existingGateway) - return err == nil && meta.IsStatusConditionTrue(existingGateway.Status.Conditions, common.GatewayProgrammedConditionType) - } -} - -func testRLPIsAccepted(rlpKey client.ObjectKey) func() bool { - return func() bool { - existingRLP := &kuadrantv1beta2.RateLimitPolicy{} - err := k8sClient.Get(context.Background(), rlpKey, existingRLP) if err != nil { + logf.Log.V(1).Info("gateway not read", "gateway", client.ObjectKeyFromObject(gateway), "error", err) return false } - if !meta.IsStatusConditionTrue(existingRLP.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)) { + + if !meta.IsStatusConditionTrue(existingGateway.Status.Conditions, common.GatewayProgrammedConditionType) { + logf.Log.V(1).Info("gateway not programmed", "gateway", client.ObjectKeyFromObject(gateway)) return false } @@ -353,6 +357,21 @@ func testWasmPluginIsAvailable(key client.ObjectKey) func() bool { } } +func testRLPIsAccepted(rlpKey client.ObjectKey) func() bool { + return func() bool { + existingRLP := &kuadrantv1beta2.RateLimitPolicy{} + err := k8sClient.Get(context.Background(), rlpKey, existingRLP) + if err != nil { + return false + } + if !meta.IsStatusConditionTrue(existingRLP.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)) { + return false + } + + return true + } +} + // DNS func testBuildManagedZone(name, ns, domainName string) *kuadrantdnsv1alpha1.ManagedZone { diff --git a/controllers/rate_limiting_wasmplugin_controller_test.go b/controllers/rate_limiting_wasmplugin_controller_test.go index 11bb79a3f..4f77043ef 100644 --- a/controllers/rate_limiting_wasmplugin_controller_test.go +++ b/controllers/rate_limiting_wasmplugin_controller_test.go @@ -7,6 +7,7 @@ import ( "reflect" "time" + "github.com/google/go-cmp/cmp" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" @@ -14,6 +15,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -668,10 +670,12 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) return false } existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) if err != nil { + logf.Log.V(1).Info("wasmplugin could not be deserialized", "key", wasmPluginKey, "error", err) return false } @@ -716,6 +720,8 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { } if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + diff := cmp.Diff(existingWASMConfig, expectedPlugin) + logf.Log.V(1).Info("wasmplugin does not match", "key", wasmPluginKey, "diff", diff) return false } @@ -731,7 +737,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { gwBName = "gw-b" ) - It("RLP targeting a gateway, GwA should not have wasmplugin and GwB should have wasmplugin", func() { + It("RLP targeting a gateway, GwA should not have wasmplugin and GwB should not have wasmplugin", func() { // Initial state // Gw A // Gw B @@ -797,10 +803,12 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) return false } existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) if err != nil { + logf.Log.V(1).Info("wasmplugin could not be deserialized", "key", wasmPluginKey, "error", err) return false } @@ -845,6 +853,8 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { } if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + diff := cmp.Diff(existingWASMConfig, expectedPlugin) + logf.Log.V(1).Info("wasmplugin does not match", "key", wasmPluginKey, "diff", diff) return false } @@ -858,7 +868,16 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gwB), Namespace: testNamespace} existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) - return apierrors.IsNotFound(err) + if err == nil { + logf.Log.V(1).Info("wasmplugin found unexpectedly", "key", wasmPluginKey) + return false + } + if !apierrors.IsNotFound(err) { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + // not found + return true }) // Proceed with the update: @@ -877,7 +896,16 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) - return apierrors.IsNotFound(err) + if err == nil { + logf.Log.V(1).Info("wasmplugin found unexpectedly", "key", wasmPluginKey) + return false + } + if !apierrors.IsNotFound(err) { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + // not found + return true }) // Check wasm plugin for gateway B does not exist @@ -887,7 +915,16 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gwB), Namespace: testNamespace} existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) - return apierrors.IsNotFound(err) + if err == nil { + logf.Log.V(1).Info("wasmplugin found unexpectedly", "key", wasmPluginKey) + return false + } + if !apierrors.IsNotFound(err) { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + // not found + return true }) }) @@ -903,6 +940,231 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Gw B // Route A -> Gw B // RLP A -> Route A + + // Gw A will be the pre-existing $gateway with name $gwName + + // create Gateway B + gwB := testBuildBasicGateway(gwBName, testNamespace) + err := k8sClient.Create(context.Background(), gwB) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gwB), 30*time.Second, 5*time.Second).Should(BeTrue()) + + // create Route A -> Gw A + httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) + err = k8sClient.Create(context.Background(), httpRoute) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) + + // create RLP A -> Route A + rlpA := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "HTTPRoute", + Name: gatewayapiv1.ObjectName(routeName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "l1": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlpA) + Expect(err).ToNot(HaveOccurred()) + // Check RLP status is available + rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} + Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // Initial state set. + // Check wasm plugin for gateway A has configuration from the route + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{ + Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace, + } + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + if err != nil { + logf.Log.V(1).Info("wasmplugin could not be deserialized", "key", wasmPluginKey, "error", err) + return false + } + + expectedPlugin := &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlpKey.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlpA), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/toy", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.l1__2804bad6`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*.example.com"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + } + + if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + diff := cmp.Diff(existingWASMConfig, expectedPlugin) + logf.Log.V(1).Info("wasmplugin does not match", "key", wasmPluginKey, "diff", diff) + return false + } + + return true + }, time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin for gateway B does not exist + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + // Check wasm plugin + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gwB), Namespace: testNamespace} + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err == nil { + logf.Log.V(1).Info("wasmplugin found unexpectedly", "key", wasmPluginKey) + return false + } + if !apierrors.IsNotFound(err) { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + // not found + return true + }) + + // Proceed with the update: + // From Route A -> Gw A + // To Route A -> Gw B + httpRouteUpdated := &gatewayapiv1.HTTPRoute{} + err = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(httpRoute), httpRouteUpdated) + Expect(err).ToNot(HaveOccurred()) + httpRouteUpdated.Spec.CommonRouteSpec.ParentRefs[0].Name = gatewayapiv1.ObjectName(gwBName) + err = k8sClient.Update(context.Background(), httpRouteUpdated) + Expect(err).ToNot(HaveOccurred()) + + // Check wasm plugin for gateway A no longer exists + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err == nil { + logf.Log.V(1).Info("wasmplugin found unexpectedly", "key", wasmPluginKey) + return false + } + if !apierrors.IsNotFound(err) { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + // not found + return true + }) + + // Check wasm plugin for gateway B has configuration from the route + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{ + Name: rlptools.WASMPluginName(gwB), Namespace: testNamespace, + } + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + if err != nil { + logf.Log.V(1).Info("wasmplugin could not be deserialized", "key", wasmPluginKey, "error", err) + return false + } + + expectedPlugin := &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlpKey.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlpA), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/toy", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.l1__2804bad6`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*.example.com"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + } + + if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + diff := cmp.Diff(existingWASMConfig, expectedPlugin) + logf.Log.V(1).Info("wasmplugin does not match", "key", wasmPluginKey, "diff", diff) + return false + } + + return true + }, time.Minute, 5*time.Second).Should(BeTrue()) }) }) }) @@ -912,6 +1174,7 @@ func testWasmPluginIsAvailable(key client.ObjectKey) func() bool { wp := &istioclientgoextensionv1alpha1.WasmPlugin{} err := k8sClient.Get(context.Background(), key, wp) if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", key, "error", err) return false } diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index 9f1b07151..e978c66ce 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/controllers/ratelimitpolicy_controller_test.go @@ -16,6 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" From 7616512f57f4cc0b0dac594222a8be11040f0f51 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 14 Dec 2023 16:09:16 +0100 Subject: [PATCH 12/21] wasmplugin controller: integration tests --- ...ate_limiting_wasmplugin_controller_test.go | 242 ++++++++++++++++++ 1 file changed, 242 insertions(+) diff --git a/controllers/rate_limiting_wasmplugin_controller_test.go b/controllers/rate_limiting_wasmplugin_controller_test.go index 4f77043ef..c96adb492 100644 --- a/controllers/rate_limiting_wasmplugin_controller_test.go +++ b/controllers/rate_limiting_wasmplugin_controller_test.go @@ -1167,6 +1167,248 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { }, time.Minute, 5*time.Second).Should(BeTrue()) }) }) + + Context("RLP switches targetRef from one route A to another route B", func() { + It("wasmplugin config should update config", func() { + // Initial state + // Gw A + // Route A -> Gw A + // Route B -> Gw A + // RLP R -> Route A + // + // Switch targetRef + // Gw A + // Route A -> Gw A + // Route B -> Gw A + // RLP R -> Route B + + var ( + routeAName = "route-a" + routeBName = "route-b" + rlpName = "rlp-r" + ) + + // + // create Route A -> Gw A on *.a.example.com + // + httpRouteA := testBuildBasicHttpRoute(routeAName, gwName, testNamespace, []string{"*.a.example.com"}) + // GET /routeA + httpRouteA.Spec.Rules = []gatewayapiv1.HTTPRouteRule{ + { + Matches: []gatewayapiv1.HTTPRouteMatch{ + { + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/routeA"), + }, + Method: ptr.To(gatewayapiv1.HTTPMethod("GET")), + }, + }, + }, + } + err := k8sClient.Create(context.Background(), httpRouteA) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteA)), time.Minute, 5*time.Second).Should(BeTrue()) + + // + // create Route B -> Gw A on *.b.example.com + // + httpRouteB := testBuildBasicHttpRoute(routeBName, gwName, testNamespace, []string{"*.b.example.com"}) + // GET /routeB + httpRouteB.Spec.Rules = []gatewayapiv1.HTTPRouteRule{ + { + Matches: []gatewayapiv1.HTTPRouteMatch{ + { + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/routeB"), + }, + Method: ptr.To(gatewayapiv1.HTTPMethod("GET")), + }, + }, + }, + } + err = k8sClient.Create(context.Background(), httpRouteB) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteB)), time.Minute, 5*time.Second).Should(BeTrue()) + + // + // create RLP R -> Route A + // + rlpR := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "HTTPRoute", + Name: gatewayapiv1.ObjectName(routeAName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "l1": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlpR) + Expect(err).ToNot(HaveOccurred()) + // Check RLP status is available + rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} + Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // Initial state set. + // Check wasm plugin has configuration from the route A + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{ + Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace, + } + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + if err != nil { + logf.Log.V(1).Info("wasmplugin could not be deserialized", "key", wasmPluginKey, "error", err) + return false + } + + expectedPlugin := &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlpKey.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlpR), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/routeA", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.l1__2804bad6`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*.a.example.com"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + } + + if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + diff := cmp.Diff(existingWASMConfig, expectedPlugin) + logf.Log.V(1).Info("wasmplugin does not match", "key", wasmPluginKey, "diff", diff) + return false + } + + return true + }, time.Minute, 5*time.Second).Should(BeTrue()) + + // Proceed with the update: + // From RLP R -> Route A + // To RLP R -> Route B + rlpUpdated := &kuadrantv1beta2.RateLimitPolicy{} + err = k8sClient.Get(context.Background(), client.ObjectKeyFromObject(rlpR), rlpUpdated) + Expect(err).ToNot(HaveOccurred()) + rlpUpdated.Spec.TargetRef.Name = gatewayapiv1.ObjectName(routeBName) + err = k8sClient.Update(context.Background(), rlpUpdated) + Expect(err).ToNot(HaveOccurred()) + + // Check wasm plugin has configuration from the route B + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{ + Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace, + } + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + if err != nil { + logf.Log.V(1).Info("wasmplugin could not be deserialized", "key", wasmPluginKey, "error", err) + return false + } + + expectedPlugin := &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlpKey.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlpR), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/routeB", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.l1__2804bad6`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*.b.example.com"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + } + + if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + diff := cmp.Diff(existingWASMConfig, expectedPlugin) + logf.Log.V(1).Info("wasmplugin does not match", "key", wasmPluginKey, "diff", diff) + return false + } + + return true + }, time.Minute, 5*time.Second).Should(BeTrue()) + }) + }) }) func testWasmPluginIsAvailable(key client.ObjectKey) func() bool { From aa593413e748e5a23acd08add0d78b94fc3f54f4 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Mon, 18 Dec 2023 10:55:37 +0100 Subject: [PATCH 13/21] wasmplugin controller: integration tests: Free Route gets dedicated RLP --- ...ate_limiting_wasmplugin_controller_test.go | 240 ++++++++++++++++++ 1 file changed, 240 insertions(+) diff --git a/controllers/rate_limiting_wasmplugin_controller_test.go b/controllers/rate_limiting_wasmplugin_controller_test.go index c96adb492..52f96d27e 100644 --- a/controllers/rate_limiting_wasmplugin_controller_test.go +++ b/controllers/rate_limiting_wasmplugin_controller_test.go @@ -1409,6 +1409,246 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { }, time.Minute, 5*time.Second).Should(BeTrue()) }) }) + + Context("Free Route gets dedicated RLP", func() { + It("wasmplugin config should update config", func() { + // Initial state + // Gw A + // Route A -> Gw A (free route, i.e. no rlp targeting it) + // RLP 1 -> Gw A + // + // Add new RLP 2 + // Gw A + // Route A -> Gw A + // RLP 1 -> Gw A + // RLP 2 -> Route A + + var ( + routeAName = "route-a" + rlp1Name = "rlp-1" + rlp2Name = "rlp-2" + ) + + // + // create Route A -> Gw A on *.a.example.com + // + httpRouteA := testBuildBasicHttpRoute(routeAName, gwName, testNamespace, []string{"*.a.example.com"}) + // GET /routeA + httpRouteA.Spec.Rules = []gatewayapiv1.HTTPRouteRule{ + { + Matches: []gatewayapiv1.HTTPRouteMatch{ + { + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/routeA"), + }, + Method: ptr.To(gatewayapiv1.HTTPMethod("GET")), + }, + }, + }, + } + err := k8sClient.Create(context.Background(), httpRouteA) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteA)), time.Minute, 5*time.Second).Should(BeTrue()) + + // create RLP 1 -> Gw A + rlp1 := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlp1Name, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "Gateway", + Name: gatewayapiv1.ObjectName(gwName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "gatewaylimit": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlp1) + Expect(err).ToNot(HaveOccurred()) + // Check RLP status is available + rlp1Key := client.ObjectKey{Name: rlp1Name, Namespace: testNamespace} + Eventually(testRLPIsAvailable(rlp1Key), time.Minute, 5*time.Second).Should(BeTrue()) + + // Initial state set. + // Check wasm plugin for gateway A has configuration from the route 1 + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{ + Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace, + } + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + if err != nil { + logf.Log.V(1).Info("wasmplugin could not be deserialized", "key", wasmPluginKey, "error", err) + return false + } + + expectedPlugin := &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlp1Key.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlp1), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/routeA", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.gatewaylimit__b95fa83b`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + } + + if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + diff := cmp.Diff(existingWASMConfig, expectedPlugin) + logf.Log.V(1).Info("wasmplugin does not match", "key", wasmPluginKey, "diff", diff) + return false + } + + return true + }, time.Minute, 5*time.Second).Should(BeTrue()) + + // Proceed with the update: + // New RLP 2 -> Route A + + // create RLP 2 -> Route A + rlp2 := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlp2Name, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "HTTPRoute", + Name: gatewayapiv1.ObjectName(routeAName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "routelimit": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 4, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlp2) + Expect(err).ToNot(HaveOccurred()) + // Check RLP status is available + rlp2Key := client.ObjectKey{Name: rlp2Name, Namespace: testNamespace} + Eventually(testRLPIsAvailable(rlp2Key), time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin has configuration from the route A and RLP 2. + // RLP 1 should not add any config to the wasm plugin + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{ + Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace, + } + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + if err != nil { + logf.Log.V(1).Info("wasmplugin could not be deserialized", "key", wasmPluginKey, "error", err) + return false + } + + expectedPlugin := &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlp2Key.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlp2), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/routeA", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.routelimit__efc5113c`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*.a.example.com"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + } + + if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + diff := cmp.Diff(existingWASMConfig, expectedPlugin) + logf.Log.V(1).Info("wasmplugin does not match", "key", wasmPluginKey, "diff", diff) + return false + } + + return true + }, time.Minute, 5*time.Second).Should(BeTrue()) + }) + }) }) func testWasmPluginIsAvailable(key client.ObjectKey) func() bool { From 42b7fb25f6cf3fb3b79577ac75eb5260c13286c1 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Mon, 18 Dec 2023 11:25:31 +0100 Subject: [PATCH 14/21] wasmplugin controller: integration tests: New free route on a Gateway with RLP --- ...ate_limiting_wasmplugin_controller_test.go | 305 +++++++++++++++++- 1 file changed, 304 insertions(+), 1 deletion(-) diff --git a/controllers/rate_limiting_wasmplugin_controller_test.go b/controllers/rate_limiting_wasmplugin_controller_test.go index 52f96d27e..f38a9aeb3 100644 --- a/controllers/rate_limiting_wasmplugin_controller_test.go +++ b/controllers/rate_limiting_wasmplugin_controller_test.go @@ -1411,7 +1411,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { }) Context("Free Route gets dedicated RLP", func() { - It("wasmplugin config should update config", func() { + It("wasmplugin should update config", func() { // Initial state // Gw A // Route A -> Gw A (free route, i.e. no rlp targeting it) @@ -1551,7 +1551,9 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Proceed with the update: // New RLP 2 -> Route A + // // create RLP 2 -> Route A + // rlp2 := &kuadrantv1beta2.RateLimitPolicy{ TypeMeta: metav1.TypeMeta{ Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), @@ -1649,6 +1651,307 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { }, time.Minute, 5*time.Second).Should(BeTrue()) }) }) + + Context("New free route on a Gateway with RLP", func() { + It("wasmplugin should update config", func() { + // Initial state + // Gw A + // Route A -> Gw A + // RLP 1 -> Gw A + // RLP 2 -> Route A + // + // Add new Route B (free route, i.e. no rlp targeting it) + // Gw A + // Route A -> Gw A + // Route B -> Gw A + // RLP 1 -> Gw A + // RLP 2 -> Route A + + var ( + routeAName = "route-a" + routeBName = "route-b" + rlp1Name = "rlp-1" + rlp2Name = "rlp-2" + ) + + // + // create Route A -> Gw A on *.a.example.com + // + httpRouteA := testBuildBasicHttpRoute(routeAName, gwName, testNamespace, []string{"*.a.example.com"}) + // GET /routeA + httpRouteA.Spec.Rules = []gatewayapiv1.HTTPRouteRule{ + { + Matches: []gatewayapiv1.HTTPRouteMatch{ + { + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/routeA"), + }, + Method: ptr.To(gatewayapiv1.HTTPMethod("GET")), + }, + }, + }, + } + err := k8sClient.Create(context.Background(), httpRouteA) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteA)), time.Minute, 5*time.Second).Should(BeTrue()) + + // create RLP 1 -> Gw A + rlp1 := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlp1Name, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "Gateway", + Name: gatewayapiv1.ObjectName(gwName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "gatewaylimit": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlp1) + Expect(err).ToNot(HaveOccurred()) + // Check RLP status is available + rlp1Key := client.ObjectKey{Name: rlp1Name, Namespace: testNamespace} + Eventually(testRLPIsAvailable(rlp1Key), time.Minute, 5*time.Second).Should(BeTrue()) + + // create RLP 2 -> Route A + rlp2 := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlp2Name, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Kind: "HTTPRoute", + Name: gatewayapiv1.ObjectName(routeAName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "routelimit": { + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 4, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlp2) + Expect(err).ToNot(HaveOccurred()) + // Check RLP status is available + rlp2Key := client.ObjectKey{Name: rlp2Name, Namespace: testNamespace} + Eventually(testRLPIsAvailable(rlp2Key), time.Minute, 5*time.Second).Should(BeTrue()) + + // Initial state set. + // Check wasm plugin for gateway A has configuration from the route A only affected by RLP 2 + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{ + Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace, + } + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + if err != nil { + logf.Log.V(1).Info("wasmplugin could not be deserialized", "key", wasmPluginKey, "error", err) + return false + } + + expectedPlugin := &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlp2Key.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlp2), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/routeA", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.routelimit__efc5113c`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*.a.example.com"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + } + + if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + diff := cmp.Diff(existingWASMConfig, expectedPlugin) + logf.Log.V(1).Info("wasmplugin does not match", "key", wasmPluginKey, "diff", diff) + return false + } + + return true + }, time.Minute, 5*time.Second).Should(BeTrue()) + + // Proceed with the update: + // New Route B -> Gw A (free route, i.e. no rlp targeting it) + + // + // create Route B -> Gw A on *.b.example.com + // + httpRouteB := testBuildBasicHttpRoute(routeBName, gwName, testNamespace, []string{"*.b.example.com"}) + // GET /routeB + httpRouteB.Spec.Rules = []gatewayapiv1.HTTPRouteRule{ + { + Matches: []gatewayapiv1.HTTPRouteMatch{ + { + Path: &gatewayapiv1.HTTPPathMatch{ + Type: ptr.To(gatewayapiv1.PathMatchPathPrefix), + Value: ptr.To("/routeB"), + }, + Method: ptr.To(gatewayapiv1.HTTPMethod("GET")), + }, + }, + }, + } + err = k8sClient.Create(context.Background(), httpRouteB) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteB)), time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin has configuration from: + // - the route A with route level RLP 2 + // - the route B with gateway level RLP 1 + // it may take some reconciliation loops to get to that, so checking it with eventually + Eventually(func() bool { + wasmPluginKey := client.ObjectKey{ + Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace, + } + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err := k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", wasmPluginKey, "error", err) + return false + } + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + if err != nil { + logf.Log.V(1).Info("wasmplugin could not be deserialized", "key", wasmPluginKey, "error", err) + return false + } + + expectedPlugin := &wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { // First RLP 1 as the controller will sort based on RLP name + Name: rlp1Key.String(), // Route B affected by RLP 1 -> Gateway + Domain: rlptools.LimitsNamespaceFromRLP(rlp1), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/routeB", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.gatewaylimit__b95fa83b`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*"}, + Service: common.KuadrantRateLimitClusterName, + }, + { + Name: rlp2Key.String(), // Route A affected by RLP 1 -> Route A + Domain: rlptools.LimitsNamespaceFromRLP(rlp2), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/routeA", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.routelimit__efc5113c`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{"*.a.example.com"}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + } + + if !reflect.DeepEqual(existingWASMConfig, expectedPlugin) { + diff := cmp.Diff(existingWASMConfig, expectedPlugin) + logf.Log.V(1).Info("wasmplugin does not match", "key", wasmPluginKey, "diff", diff) + return false + } + + return true + }, time.Minute, 5*time.Second).Should(BeTrue()) + + }) + }) }) func testWasmPluginIsAvailable(key client.ObjectKey) func() bool { From a733363dfdfc83bd29f41ea16377bf0b7c32174a Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Wed, 21 Feb 2024 20:43:29 +0100 Subject: [PATCH 15/21] picking changes when rebasing --- controllers/helper_test.go | 3 ++ ...ate_limiting_wasmplugin_controller_test.go | 47 ++++++------------- .../ratelimitpolicy_controller_test.go | 3 -- pkg/common/kuadrant_policy_test.go | 4 ++ 4 files changed, 21 insertions(+), 36 deletions(-) diff --git a/controllers/helper_test.go b/controllers/helper_test.go index 0c9095e48..822e04b10 100644 --- a/controllers/helper_test.go +++ b/controllers/helper_test.go @@ -344,6 +344,7 @@ func testWasmPluginIsAvailable(key client.ObjectKey) func() bool { wp := &istioclientgoextensionv1alpha1.WasmPlugin{} err := k8sClient.Get(context.Background(), key, wp) if err != nil { + logf.Log.V(1).Info("wasmplugin not read", "key", key, "error", err) return false } @@ -362,9 +363,11 @@ func testRLPIsAccepted(rlpKey client.ObjectKey) func() bool { existingRLP := &kuadrantv1beta2.RateLimitPolicy{} err := k8sClient.Get(context.Background(), rlpKey, existingRLP) if err != nil { + logf.Log.V(1).Info("ratelimitpolicy not read", "rlp", rlpKey, "error", err) return false } if !meta.IsStatusConditionTrue(existingRLP.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)) { + logf.Log.V(1).Info("ratelimitpolicy not available", "rlp", rlpKey) return false } diff --git a/controllers/rate_limiting_wasmplugin_controller_test.go b/controllers/rate_limiting_wasmplugin_controller_test.go index f38a9aeb3..e010e1e97 100644 --- a/controllers/rate_limiting_wasmplugin_controller_test.go +++ b/controllers/rate_limiting_wasmplugin_controller_test.go @@ -85,7 +85,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKeyFromObject(rlp) - Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} @@ -243,7 +243,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKeyFromObject(rlp) - Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} @@ -384,7 +384,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} - Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} @@ -471,7 +471,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} - Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} @@ -537,7 +537,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpKey := client.ObjectKeyFromObject(rlp) - Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} @@ -597,7 +597,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpAKey := client.ObjectKey{Name: rlpAName, Namespace: testNamespace} - Eventually(testRLPIsAvailable(rlpAKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlpAKey), time.Minute, 5*time.Second).Should(BeTrue()) // create httproute C httpRouteC := testBuildBasicHttpRoute(routeCName, gwName, testNamespace, []string{"*.c.example.com"}) @@ -659,7 +659,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { // Check RLP status is available rlpBKey := client.ObjectKey{Name: rlpBName, Namespace: testNamespace} - Eventually(testRLPIsAvailable(rlpBKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlpBKey), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin only has configuration ONLY from the RLP targeting the gateway // it may take some reconciliation loops to get to that, so checking it with eventually @@ -779,7 +779,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { Expect(err).ToNot(HaveOccurred()) // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} - Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // create Route A -> Gw A httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) @@ -982,7 +982,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { Expect(err).ToNot(HaveOccurred()) // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} - Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Initial state set. // Check wasm plugin for gateway A has configuration from the route @@ -1261,7 +1261,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { Expect(err).ToNot(HaveOccurred()) // Check RLP status is available rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace} - Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) // Initial state set. // Check wasm plugin has configuration from the route A @@ -1478,7 +1478,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { Expect(err).ToNot(HaveOccurred()) // Check RLP status is available rlp1Key := client.ObjectKey{Name: rlp1Name, Namespace: testNamespace} - Eventually(testRLPIsAvailable(rlp1Key), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlp1Key), time.Minute, 5*time.Second).Should(BeTrue()) // Initial state set. // Check wasm plugin for gateway A has configuration from the route 1 @@ -1580,7 +1580,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { Expect(err).ToNot(HaveOccurred()) // Check RLP status is available rlp2Key := client.ObjectKey{Name: rlp2Name, Namespace: testNamespace} - Eventually(testRLPIsAvailable(rlp2Key), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlp2Key), time.Minute, 5*time.Second).Should(BeTrue()) // Check wasm plugin has configuration from the route A and RLP 2. // RLP 1 should not add any config to the wasm plugin @@ -1723,7 +1723,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { Expect(err).ToNot(HaveOccurred()) // Check RLP status is available rlp1Key := client.ObjectKey{Name: rlp1Name, Namespace: testNamespace} - Eventually(testRLPIsAvailable(rlp1Key), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlp1Key), time.Minute, 5*time.Second).Should(BeTrue()) // create RLP 2 -> Route A rlp2 := &kuadrantv1beta2.RateLimitPolicy{ @@ -1752,7 +1752,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { Expect(err).ToNot(HaveOccurred()) // Check RLP status is available rlp2Key := client.ObjectKey{Name: rlp2Name, Namespace: testNamespace} - Eventually(testRLPIsAvailable(rlp2Key), time.Minute, 5*time.Second).Should(BeTrue()) + Eventually(testRLPIsAccepted(rlp2Key), time.Minute, 5*time.Second).Should(BeTrue()) // Initial state set. // Check wasm plugin for gateway A has configuration from the route A only affected by RLP 2 @@ -1953,22 +1953,3 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { }) }) }) - -func testWasmPluginIsAvailable(key client.ObjectKey) func() bool { - return func() bool { - wp := &istioclientgoextensionv1alpha1.WasmPlugin{} - err := k8sClient.Get(context.Background(), key, wp) - if err != nil { - logf.Log.V(1).Info("wasmplugin not read", "key", key, "error", err) - return false - } - - // Unfortunately, WasmPlugin does not have status yet - // Leaving this here for future use - //if !meta.IsStatusConditionTrue(wp.Status.Conditions, "Available") { - // return false - //} - - return true - } -} diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index e978c66ce..4ddf5d691 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/controllers/ratelimitpolicy_controller_test.go @@ -5,18 +5,15 @@ package controllers import ( "context" "encoding/json" - "fmt" "strings" "time" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" diff --git a/pkg/common/kuadrant_policy_test.go b/pkg/common/kuadrant_policy_test.go index fe07f31d1..91c679950 100644 --- a/pkg/common/kuadrant_policy_test.go +++ b/pkg/common/kuadrant_policy_test.go @@ -32,6 +32,10 @@ func (p *TestPolicy) GetRulesHostnames() []string { return nil } +func (p *TestPolicy) Kind() string { + return p.TypeMeta.Kind +} + func (p *TestPolicy) DeepCopyObject() runtime.Object { if c := p.DeepCopy(); c != nil { return c From 7be17ecbd17ab46e69a9d2960f3fd3d3123b213f Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 22 Feb 2024 19:52:54 +0100 Subject: [PATCH 16/21] wasmplugin controller: enhance logging --- .../rate_limiting_wasmplugin_controller.go | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go index 46dbfc8b7..96b4cac14 100644 --- a/controllers/rate_limiting_wasmplugin_controller.go +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -40,7 +40,7 @@ import ( ) const ( - HTTPRouteParents = ".metadata.parent" + HTTPRouteParents = ".metadata.route.parent" ) // RateLimitingWASMPluginReconciler reconciles a WASMPlugin object for rate limiting @@ -93,6 +93,11 @@ func (r *RateLimitingWASMPluginReconciler) Reconcile(eventCtx context.Context, r } func (r *RateLimitingWASMPluginReconciler) desiredRateLimitingWASMPlugin(ctx context.Context, gw *gatewayapiv1.Gateway) (*istioclientgoextensionv1alpha1.WasmPlugin, error) { + baseLogger, err := logr.FromContext(ctx) + if err != nil { + return nil, err + } + wasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{ TypeMeta: metav1.TypeMeta{ Kind: "WasmPlugin", @@ -111,12 +116,15 @@ func (r *RateLimitingWASMPluginReconciler) desiredRateLimitingWASMPlugin(ctx con }, } + logger := baseLogger.WithValues("wasmplugin", client.ObjectKeyFromObject(wasmPlugin)) + pluginConfig, err := r.wasmPluginConfig(ctx, gw) if err != nil { return nil, err } if pluginConfig == nil || len(pluginConfig.RateLimitPolicies) == 0 { + logger.V(1).Info("pluginConfig is empty. Wasmplugin will be deleted if it exists") common.TagObjectToDelete(wasmPlugin) return wasmPlugin, nil } @@ -291,23 +299,22 @@ func (r *RateLimitingWASMPluginReconciler) RouteFromRLP(ctx context.Context, t * // addHTTPRouteByGatewayIndexer declares an index key that we can later use with the client as a pseudo-field name, // allowing to query all the routes parenting a given gateway // to prevent creating the same index field multiple times, the function is declared private to be -// called ontly by this controller -func addHTTPRouteByGatewayIndexer(mgr ctrl.Manager, logger logr.Logger) error { +// called only by this controller +func addHTTPRouteByGatewayIndexer(mgr ctrl.Manager, baseLogger logr.Logger) error { if err := mgr.GetFieldIndexer().IndexField(context.Background(), &gatewayapiv1.HTTPRoute{}, HTTPRouteParents, func(rawObj client.Object) []string { // grab the route object, extract the parents route, assertionOk := rawObj.(*gatewayapiv1.HTTPRoute) - logger.Info("assertionOK", "ok", assertionOk) if !assertionOk { return nil } - logger.Info("route", "name", client.ObjectKeyFromObject(route).String()) + logger := baseLogger.WithValues("route", client.ObjectKeyFromObject(route).String()) keys := make([]string, 0) for _, parentRef := range route.Spec.CommonRouteSpec.ParentRefs { if !common.IsParentGateway(parentRef) { - logger.Info("parent not gateway", "ParentRefs", parentRef) + logger.V(1).Info("parent is not gateway", "ParentRefs", parentRef) continue } @@ -316,12 +323,11 @@ func addHTTPRouteByGatewayIndexer(mgr ctrl.Manager, logger logr.Logger) error { Namespace: string(ptr.Deref(parentRef.Namespace, gatewayapiv1.Namespace(route.Namespace))), } - logger.Info("new key", "key", key.String()) + logger.V(1).Info("new gateway added", "key", key.String()) keys = append(keys, key.String()) } - // ...and if so, return it return keys }); err != nil { return err From 7d010d853b0a1ca473691f920e776124ff08b096 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 22 Feb 2024 19:53:31 +0100 Subject: [PATCH 17/21] wasmplugin controller: integration tests for hostnameless route --- ...ate_limiting_wasmplugin_controller_test.go | 236 ++++++++++++++++-- 1 file changed, 216 insertions(+), 20 deletions(-) diff --git a/controllers/rate_limiting_wasmplugin_controller_test.go b/controllers/rate_limiting_wasmplugin_controller_test.go index e010e1e97..1ceeffd5e 100644 --- a/controllers/rate_limiting_wasmplugin_controller_test.go +++ b/controllers/rate_limiting_wasmplugin_controller_test.go @@ -28,16 +28,10 @@ import ( var _ = Describe("Rate Limiting WasmPlugin controller", func() { var ( testNamespace string - gwName = "toystore-gw" - gateway *gatewayapiv1.Gateway ) beforeEachCallback := func() { CreateNamespace(&testNamespace) - gateway = testBuildBasicGateway(gwName, testNamespace) - err := k8sClient.Create(context.Background(), gateway) - Expect(err).ToNot(HaveOccurred()) - Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) ApplyKuadrantCR(testNamespace) } @@ -48,8 +42,19 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { var ( routeName = "toystore-route" rlpName = "toystore-rlp" + gwName = "toystore-gw" + gateway *gatewayapiv1.Gateway ) + beforeEachCallback := func() { + gateway = testBuildBasicGateway(gwName, testNamespace) + err := k8sClient.Create(context.Background(), gateway) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) + } + + BeforeEach(beforeEachCallback) + It("Simple RLP targeting HTTPRoute creates wasmplugin", func() { // create httproute httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) @@ -65,7 +70,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "HTTPRoute", Name: gatewayapiv1.ObjectName(routeName), }, @@ -186,7 +191,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { }, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "HTTPRoute", Name: gatewayapiv1.ObjectName(routeName), }, @@ -364,7 +369,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "Gateway", Name: gatewayapiv1.ObjectName(gwName), }, @@ -440,8 +445,19 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { Context("RLP targeting HTTPRoute-less Gateway", func() { var ( rlpName = "toystore-rlp" + gwName = "toystore-gw" + gateway *gatewayapiv1.Gateway ) + beforeEachCallback := func() { + gateway = testBuildBasicGateway(gwName, testNamespace) + err := k8sClient.Create(context.Background(), gateway) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) + } + + BeforeEach(beforeEachCallback) + It("Wasmplugin must not be created", func() { // create ratelimitpolicy rlp := &kuadrantv1beta2.RateLimitPolicy{ @@ -451,7 +467,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "Gateway", Name: gatewayapiv1.ObjectName(gwName), }, @@ -488,8 +504,19 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { var ( routeName = "toystore-route" rlpName = "toystore-rlp" + gwName = "toystore-gw" + gateway *gatewayapiv1.Gateway ) + beforeEachCallback := func() { + gateway = testBuildBasicGateway(gwName, testNamespace) + err := k8sClient.Create(context.Background(), gateway) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) + } + + BeforeEach(beforeEachCallback) + It("When the gateway does not have more policies, the wasmplugin resource is not created", func() { // create httproute httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"}) @@ -505,7 +532,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "HTTPRoute", Name: gatewayapiv1.ObjectName(routeName), }, @@ -577,7 +604,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlpAName, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "Gateway", Name: gatewayapiv1.ObjectName(gwName), }, @@ -627,7 +654,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlpBName, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "HTTPRoute", Name: gatewayapiv1.ObjectName(routeCName), }, @@ -734,9 +761,20 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { var ( routeName = "route-a" rlpName = "rlp-a" + gwName = "toystore-gw" + gateway *gatewayapiv1.Gateway gwBName = "gw-b" ) + beforeEachCallback := func() { + gateway = testBuildBasicGateway(gwName, testNamespace) + err := k8sClient.Create(context.Background(), gateway) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) + } + + BeforeEach(beforeEachCallback) + It("RLP targeting a gateway, GwA should not have wasmplugin and GwB should not have wasmplugin", func() { // Initial state // Gw A @@ -760,7 +798,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "Gateway", Name: gatewayapiv1.ObjectName(gwName), }, @@ -963,7 +1001,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "HTTPRoute", Name: gatewayapiv1.ObjectName(routeName), }, @@ -1169,6 +1207,20 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { }) Context("RLP switches targetRef from one route A to another route B", func() { + var ( + gwName = "toystore-gw" + gateway *gatewayapiv1.Gateway + ) + + beforeEachCallback := func() { + gateway = testBuildBasicGateway(gwName, testNamespace) + err := k8sClient.Create(context.Background(), gateway) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) + } + + BeforeEach(beforeEachCallback) + It("wasmplugin config should update config", func() { // Initial state // Gw A @@ -1242,7 +1294,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "HTTPRoute", Name: gatewayapiv1.ObjectName(routeAName), }, @@ -1411,6 +1463,20 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { }) Context("Free Route gets dedicated RLP", func() { + var ( + gwName = "toystore-gw" + gateway *gatewayapiv1.Gateway + ) + + beforeEachCallback := func() { + gateway = testBuildBasicGateway(gwName, testNamespace) + err := k8sClient.Create(context.Background(), gateway) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) + } + + BeforeEach(beforeEachCallback) + It("wasmplugin should update config", func() { // Initial state // Gw A @@ -1459,7 +1525,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlp1Name, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "Gateway", Name: gatewayapiv1.ObjectName(gwName), }, @@ -1561,7 +1627,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlp2Name, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "HTTPRoute", Name: gatewayapiv1.ObjectName(routeAName), }, @@ -1653,6 +1719,20 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { }) Context("New free route on a Gateway with RLP", func() { + var ( + gwName = "toystore-gw" + gateway *gatewayapiv1.Gateway + ) + + beforeEachCallback := func() { + gateway = testBuildBasicGateway(gwName, testNamespace) + err := k8sClient.Create(context.Background(), gateway) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) + } + + BeforeEach(beforeEachCallback) + It("wasmplugin should update config", func() { // Initial state // Gw A @@ -1704,7 +1784,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlp1Name, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "Gateway", Name: gatewayapiv1.ObjectName(gwName), }, @@ -1733,7 +1813,7 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { ObjectMeta: metav1.ObjectMeta{Name: rlp2Name, Namespace: testNamespace}, Spec: kuadrantv1beta2.RateLimitPolicySpec{ TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ - Group: gatewayapiv1.Group("gateway.networking.k8s.io"), + Group: gatewayapiv1.GroupName, Kind: "HTTPRoute", Name: gatewayapiv1.ObjectName(routeAName), }, @@ -1952,4 +2032,120 @@ var _ = Describe("Rate Limiting WasmPlugin controller", func() { }) }) + + Context("Gateway with hostname in listener", func() { + var ( + gwName = "toystore-gw" + routeName = "toystore-route" + rlpName = "rlp-a" + gateway *gatewayapiv1.Gateway + gwHostname = "*.gw.example.com" + ) + + beforeEachCallback := func() { + gateway = testBuildBasicGateway(gwName, testNamespace) + gateway.Spec.Listeners[0].Hostname = ptr.To(gatewayapiv1.Hostname(gwHostname)) + err := k8sClient.Create(context.Background(), gateway) + Expect(err).ToNot(HaveOccurred()) + Eventually(testGatewayIsReady(gateway), 30*time.Second, 5*time.Second).Should(BeTrue()) + } + + BeforeEach(beforeEachCallback) + + It("RLP with hostnames in route selector targeting hostname less HTTPRoute creates wasmplugin", func() { + // create httproute + var emptyRouteHostnames []string + httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, emptyRouteHostnames) + err := k8sClient.Create(context.Background(), httpRoute) + Expect(err).ToNot(HaveOccurred()) + Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute)), time.Minute, 5*time.Second).Should(BeTrue()) + + // create ratelimitpolicy + rlp := &kuadrantv1beta2.RateLimitPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: "RateLimitPolicy", APIVersion: kuadrantv1beta2.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{Name: rlpName, Namespace: testNamespace}, + Spec: kuadrantv1beta2.RateLimitPolicySpec{ + TargetRef: gatewayapiv1alpha2.PolicyTargetReference{ + Group: gatewayapiv1.GroupName, + Kind: "HTTPRoute", + Name: gatewayapiv1.ObjectName(routeName), + }, + Limits: map[string]kuadrantv1beta2.Limit{ + "l1": { + RouteSelectors: []kuadrantv1beta2.RouteSelector{ + { + // Route does not specify any hostname + // gateway's listener specifies *.gw.example.com + Hostnames: []gatewayapiv1.Hostname{"*.gw.example.com"}, + }, + }, + + Rates: []kuadrantv1beta2.Rate{ + { + Limit: 1, Duration: 3, Unit: kuadrantv1beta2.TimeUnit("minute"), + }, + }, + }, + }, + }, + } + err = k8sClient.Create(context.Background(), rlp) + Expect(err).ToNot(HaveOccurred()) + + // Check RLP status is available + rlpKey := client.ObjectKeyFromObject(rlp) + Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue()) + + // Check wasm plugin + wasmPluginKey := client.ObjectKey{Name: rlptools.WASMPluginName(gateway), Namespace: testNamespace} + Eventually(testWasmPluginIsAvailable(wasmPluginKey), time.Minute, 5*time.Second).Should(BeTrue()) + existingWasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{} + err = k8sClient.Get(context.Background(), wasmPluginKey, existingWasmPlugin) + // must exist + Expect(err).ToNot(HaveOccurred()) + existingWASMConfig, err := rlptools.WASMPluginFromStruct(existingWasmPlugin.Spec.PluginConfig) + Expect(err).ToNot(HaveOccurred()) + Expect(existingWASMConfig).To(Equal(&wasm.Plugin{ + FailureMode: wasm.FailureModeDeny, + RateLimitPolicies: []wasm.RateLimitPolicy{ + { + Name: rlpKey.String(), + Domain: rlptools.LimitsNamespaceFromRLP(rlp), + Rules: []wasm.Rule{ + { + Conditions: []wasm.Condition{ + { + AllOf: []wasm.PatternExpression{ + { + Selector: "request.url_path", + Operator: wasm.PatternOperator(kuadrantv1beta2.StartsWithOperator), + Value: "/toy", + }, + { + Selector: "request.method", + Operator: wasm.PatternOperator(kuadrantv1beta2.EqualOperator), + Value: "GET", + }, + }, + }, + }, + Data: []wasm.DataItem{ + { + Static: &wasm.StaticSpec{ + Key: `limit.l1__2804bad6`, + Value: "1", + }, + }, + }, + }, + }, + Hostnames: []string{gwHostname}, + Service: common.KuadrantRateLimitClusterName, + }, + }, + })) + }) + }) }) From c05b0f42231b58a5c9de2f64dfb255f6f60af528 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 23 Feb 2024 00:58:57 +0100 Subject: [PATCH 18/21] wasmplugin controller: wasm rules from existing or inherited hostnames --- .../rate_limiting_wasmplugin_controller.go | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go index 96b4cac14..8b3773adf 100644 --- a/controllers/rate_limiting_wasmplugin_controller.go +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -219,9 +219,6 @@ func (r *RateLimitingWASMPluginReconciler) gatewayAPITopologyFromGateway(ctx con } func (r *RateLimitingWASMPluginReconciler) WASMRateLimitPolicy(ctx context.Context, t *common.KuadrantTopology, rlp *kuadrantv1beta2.RateLimitPolicy, gw *gatewayapiv1.Gateway) (*wasm.RateLimitPolicy, error) { - gwHostnamesTmp := common.TargetHostnames(gw) - gwHostnames := common.Map(gwHostnamesTmp, func(str string) gatewayapiv1.Hostname { return gatewayapiv1.Hostname(str) }) - route, err := r.RouteFromRLP(ctx, t, rlp, gw) if err != nil { return nil, err @@ -233,19 +230,33 @@ func (r *RateLimitingWASMPluginReconciler) WASMRateLimitPolicy(ctx context.Conte return nil, nil } - rules := rlptools.WasmRules(rlp, route) - 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 - return nil, nil - } - // narrow the list of hostnames specified in the route so we don't generate wasm rules that only apply to other gateways // this is a no-op for the gateway rlp + gwHostnames := common.GatewayWrapper{Gateway: gw}.Hostnames() + if len(gwHostnames) == 0 { + gwHostnames = []gatewayapiv1.Hostname{"*"} + } hostnames := common.FilterValidSubdomains(gwHostnames, route.Spec.Hostnames) if len(hostnames) == 0 { // it should only happen when the route specifies no hostnames hostnames = gwHostnames } + // + // The route selectors logic rely on the "hostnames" field of the route object. + // However, routes effective hostname can be inherited from parent gateway, + // hence it depends on the context as multiple gateways can be targeted by a route + // The route selectors logic needs to be refactored + // or just deleted as soon as the HTTPRoute has name in the route object + // + routeWithEffectiveHostnames := route.DeepCopy() + routeWithEffectiveHostnames.Spec.Hostnames = hostnames + + 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 + return nil, nil + } + return &wasm.RateLimitPolicy{ Name: client.ObjectKeyFromObject(rlp).String(), Domain: rlptools.LimitsNamespaceFromRLP(rlp), From 321cf904c8eddb18ac6f61a59b45342a1f383bc9 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Mon, 26 Feb 2024 20:00:47 +0100 Subject: [PATCH 19/21] wasmplugin controller: sort policies by creation timestamp --- .../rate_limiting_wasmplugin_controller.go | 2 +- pkg/common/common.go | 16 +++-- pkg/common/common_test.go | 67 +++++++++++++++++++ 3 files changed, 80 insertions(+), 5 deletions(-) diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go index 8b3773adf..2b84ee86b 100644 --- a/controllers/rate_limiting_wasmplugin_controller.go +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -168,7 +168,7 @@ func (r *RateLimitingWASMPluginReconciler) wasmPluginConfig(ctx context.Context, rateLimitPoliciesSorted := rateLimitPolicies // Sort RLPs for consistent comparison with existing objects - sort.Sort(common.PolicyByKey(rateLimitPoliciesSorted)) + sort.Sort(common.PolicyByCreationTimestamp(rateLimitPoliciesSorted)) for _, policy := range rateLimitPolicies { rlp := policy.(*kuadrantv1beta2.RateLimitPolicy) diff --git a/pkg/common/common.go b/pkg/common/common.go index 4e0a79ed1..c6b19a94c 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -21,6 +21,7 @@ import ( "slices" "strings" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -54,11 +55,18 @@ type KuadrantPolicyList interface { GetItems() []KuadrantPolicy } -type PolicyByKey []KuadrantPolicy +type PolicyByCreationTimestamp []KuadrantPolicy -func (a PolicyByKey) Len() int { return len(a) } -func (a PolicyByKey) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a PolicyByKey) Less(i, j int) bool { +func (a PolicyByCreationTimestamp) Len() int { return len(a) } +func (a PolicyByCreationTimestamp) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a PolicyByCreationTimestamp) Less(i, j int) bool { + p1Time := ptr.To(a[i].GetCreationTimestamp()) + p2Time := ptr.To(a[j].GetCreationTimestamp()) + if !p1Time.Equal(p2Time) { + return ptr.To(a[i].GetCreationTimestamp()).Before(ptr.To(a[j].GetCreationTimestamp())) + } + + // The policy appearing first in alphabetical order by "{namespace}/{name}". return client.ObjectKeyFromObject(a[i]).String() < client.ObjectKeyFromObject(a[j]).String() } diff --git a/pkg/common/common_test.go b/pkg/common/common_test.go index d62df1c07..6fc1066c9 100644 --- a/pkg/common/common_test.go +++ b/pkg/common/common_test.go @@ -5,8 +5,11 @@ package common import ( "fmt" "reflect" + "sort" "testing" + "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" ) @@ -693,3 +696,67 @@ func TestFilterValidSubdomains(t *testing.T) { }) } } + +func createTestPolicy(name string, creationTime time.Time) *TestPolicy { + return &TestPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "testnamespace", + Name: name, + CreationTimestamp: metav1.Time{creationTime}, + }, + } +} + +func TestPolicyByCreationTimestamp(t *testing.T) { + testCases := []struct { + name string + policies []KuadrantPolicy + sortedPolicies []KuadrantPolicy + }{ + { + name: "nil input", + policies: nil, + sortedPolicies: nil, + }, + { + name: "empty slices", + policies: make([]KuadrantPolicy, 0), + sortedPolicies: make([]KuadrantPolicy, 0), + }, + { + name: "by creation date", + policies: []KuadrantPolicy{ + createTestPolicy("ccc", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC)), + createTestPolicy("bbb", time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC)), + createTestPolicy("aaa", time.Date(2000, time.November, 10, 23, 0, 0, 0, time.UTC)), + }, + sortedPolicies: []KuadrantPolicy{ + createTestPolicy("aaa", time.Date(2000, time.November, 10, 23, 0, 0, 0, time.UTC)), + createTestPolicy("bbb", time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC)), + createTestPolicy("ccc", time.Date(2020, time.November, 10, 23, 0, 0, 0, time.UTC)), + }, + }, + { + name: "by name when creation date are equal", + policies: []KuadrantPolicy{ + createTestPolicy("ccc", time.Date(2000, time.November, 10, 23, 0, 0, 0, time.UTC)), + createTestPolicy("bbb", time.Date(2000, time.November, 10, 23, 0, 0, 0, time.UTC)), + createTestPolicy("aaa", time.Date(2000, time.November, 10, 23, 0, 0, 0, time.UTC)), + }, + sortedPolicies: []KuadrantPolicy{ + createTestPolicy("aaa", time.Date(2000, time.November, 10, 23, 0, 0, 0, time.UTC)), + createTestPolicy("bbb", time.Date(2000, time.November, 10, 23, 0, 0, 0, time.UTC)), + createTestPolicy("ccc", time.Date(2000, time.November, 10, 23, 0, 0, 0, time.UTC)), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(subT *testing.T) { + sort.Sort(PolicyByCreationTimestamp(tc.policies)) + if !reflect.DeepEqual(tc.policies, tc.sortedPolicies) { + subT.Errorf("expected=%v; got=%v", tc.sortedPolicies, tc.policies) + } + }) + } +} From 88af9f23866a86799f2796f3572f15628555b309 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Mon, 26 Feb 2024 23:22:29 +0100 Subject: [PATCH 20/21] wasmplugin controller: reuse GetRouteAcceptedGatewayParentKeys --- .../rate_limiting_wasmplugin_controller.go | 24 ++++-------------- pkg/common/gatewayapi_event_mappers.go | 25 +++---------------- .../kuadrant_policy_to_gateway_eventmapper.go | 25 ++++--------------- 3 files changed, 14 insertions(+), 60 deletions(-) diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go index 2b84ee86b..33ed94616 100644 --- a/controllers/rate_limiting_wasmplugin_controller.go +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -19,6 +19,7 @@ package controllers import ( "context" "encoding/json" + "fmt" "sort" "github.com/go-logr/logr" @@ -26,7 +27,6 @@ import ( istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -316,30 +316,16 @@ func addHTTPRouteByGatewayIndexer(mgr ctrl.Manager, baseLogger logr.Logger) erro // grab the route object, extract the parents route, assertionOk := rawObj.(*gatewayapiv1.HTTPRoute) if !assertionOk { + baseLogger.V(1).Error(fmt.Errorf("%T is not a *gatewayapiv1.HTTPRoute", rawObj), "cannot map") return nil } logger := baseLogger.WithValues("route", client.ObjectKeyFromObject(route).String()) - keys := make([]string, 0) - - for _, parentRef := range route.Spec.CommonRouteSpec.ParentRefs { - if !common.IsParentGateway(parentRef) { - logger.V(1).Info("parent is not gateway", "ParentRefs", parentRef) - continue - } - - key := client.ObjectKey{ - Name: string(parentRef.Name), - Namespace: string(ptr.Deref(parentRef.Namespace, gatewayapiv1.Namespace(route.Namespace))), - } - + return common.Map(common.GetRouteAcceptedGatewayParentKeys(route), func(key client.ObjectKey) string { logger.V(1).Info("new gateway added", "key", key.String()) - - keys = append(keys, key.String()) - } - - return keys + return key.String() + }) }); err != nil { return err } diff --git a/pkg/common/gatewayapi_event_mappers.go b/pkg/common/gatewayapi_event_mappers.go index 4ef9ad9a7..1e97a7d3f 100644 --- a/pkg/common/gatewayapi_event_mappers.go +++ b/pkg/common/gatewayapi_event_mappers.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -26,24 +25,8 @@ func (m *HTTPRouteToParentGatewaysEventMapper) Map(_ context.Context, obj client return []reconcile.Request{} } - requests := make([]reconcile.Request, 0) - - for _, parentRef := range route.Spec.ParentRefs { - // skips if parentRef is not a Gateway - if !IsParentGateway(parentRef) { - continue - } - - ns := route.Namespace - if parentRef.Namespace != nil { - ns = string(*parentRef.Namespace) - } - - nn := types.NamespacedName{Name: string(parentRef.Name), Namespace: ns} - logger.V(1).Info("map", " gateway", nn) - - requests = append(requests, reconcile.Request{NamespacedName: nn}) - } - - return requests + return Map(GetRouteAcceptedGatewayParentKeys(route), func(key client.ObjectKey) reconcile.Request { + logger.V(1).Info("new gateway event", "key", key.String()) + return reconcile.Request{NamespacedName: key} + }) } diff --git a/pkg/common/kuadrant_policy_to_gateway_eventmapper.go b/pkg/common/kuadrant_policy_to_gateway_eventmapper.go index 67f5b322f..4c970daa9 100644 --- a/pkg/common/kuadrant_policy_to_gateway_eventmapper.go +++ b/pkg/common/kuadrant_policy_to_gateway_eventmapper.go @@ -51,27 +51,12 @@ func (k *KuadrantPolicyToParentGatewaysEventMapper) Map(ctx context.Context, obj return []reconcile.Request{} } - requests := make([]reconcile.Request, 0) - - for _, parentRef := range route.Spec.ParentRefs { - // skips if parentRef is not a Gateway - if !IsParentGateway(parentRef) { - continue - } - - ns := route.Namespace - if parentRef.Namespace != nil { - ns = string(*parentRef.Namespace) - } - - nn := types.NamespacedName{Name: string(parentRef.Name), Namespace: ns} - logger.V(1).Info("map", " gateway", nn) - - requests = append(requests, reconcile.Request{NamespacedName: nn}) - } - - return requests + return Map(GetRouteAcceptedGatewayParentKeys(route), func(key client.ObjectKey) reconcile.Request { + logger.V(1).Info("new gateway event", "key", key.String()) + return reconcile.Request{NamespacedName: key} + }) } + logger.V(1).Info("policy targeting unexpected resource, skipping it", "key", client.ObjectKeyFromObject(kuadrantPolicy)) return []reconcile.Request{} } From c58450a85a91fa1d8dba20192260e029ba2bb68d Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Mon, 26 Feb 2024 23:35:43 +0100 Subject: [PATCH 21/21] wasmplugin controller: refactor freeRoute -> untargetedRoute --- .../rate_limiting_wasmplugin_controller.go | 15 ++++--- pkg/common/kuadrant_topology.go | 39 ++++++++++++------- pkg/common/kuadrant_topology_test.go | 28 ++++++------- 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/controllers/rate_limiting_wasmplugin_controller.go b/controllers/rate_limiting_wasmplugin_controller.go index 33ed94616..f80c1e4cc 100644 --- a/controllers/rate_limiting_wasmplugin_controller.go +++ b/controllers/rate_limiting_wasmplugin_controller.go @@ -279,19 +279,18 @@ func (r *RateLimitingWASMPluginReconciler) RouteFromRLP(ctx context.Context, t * // This gateway policy will be enforced into all HTTPRoutes that do not have a policy attached to it // Build imaginary route with all the routes not having a RLP targeting it - freeRoutes := t.GetFreeRoutes(gw) + untargetedRoutes := t.GetUntargetedRoutes(gw) - if len(freeRoutes) == 0 { + if len(untargetedRoutes) == 0 { // For policies targeting a gateway, when no httproutes is attached to the gateway, skip wasm config // test wasm config when no http routes attached to the gateway - logger.V(1).Info("no free httproutes attached to the targeted gateway, skipping wasm config for the gateway rlp", "ratelimitpolicy", client.ObjectKeyFromObject(rlp)) + logger.V(1).Info("no untargeted httproutes attached to the targeted gateway, skipping wasm config for the gateway rlp", "ratelimitpolicy", client.ObjectKeyFromObject(rlp)) return nil, nil } - freeRules := make([]gatewayapiv1.HTTPRouteRule, 0) - for idx := range freeRoutes { - freeroute := freeRoutes[idx] - freeRules = append(freeRules, freeroute.Spec.Rules...) + untargetedRules := make([]gatewayapiv1.HTTPRouteRule, 0) + for idx := range untargetedRoutes { + untargetedRules = append(untargetedRules, untargetedRoutes[idx].Spec.Rules...) } gwHostnamesTmp := common.TargetHostnames(gw) @@ -299,7 +298,7 @@ func (r *RateLimitingWASMPluginReconciler) RouteFromRLP(ctx context.Context, t * route = &gatewayapiv1.HTTPRoute{ Spec: gatewayapiv1.HTTPRouteSpec{ Hostnames: gwHostnames, - Rules: freeRules, + Rules: untargetedRules, }, } } diff --git a/pkg/common/kuadrant_topology.go b/pkg/common/kuadrant_topology.go index 0698f389c..4a2e0332d 100644 --- a/pkg/common/kuadrant_topology.go +++ b/pkg/common/kuadrant_topology.go @@ -22,9 +22,9 @@ type KuadrantTopology struct { // Type: Policy -> HTTPRoute policyRoute map[client.ObjectKey]*gatewayapiv1.HTTPRoute - // freeRoutes is an index of gateways mapping to HTTPRoutes not targeted by a kuadrant policy + // untargetedRoutes is an index of gateways mapping to HTTPRoutes not targeted by a kuadrant policy // Gateway -> []HTTPRoute - freeRoutes map[client.ObjectKey][]*gatewayapiv1.HTTPRoute + untargetedRoutes map[client.ObjectKey][]*gatewayapiv1.HTTPRoute // Raw topology with gateways, routes and policies // Currently only used for logging @@ -37,21 +37,30 @@ func NewKuadrantTopology(gateways []*gatewayapiv1.Gateway, routes []*gatewayapiv return &KuadrantTopology{ gatewayPolicies: buildGatewayPoliciesIndex(t), policyRoute: buildPolicyRouteIndex(t), - freeRoutes: buildFreeRoutesIndex(t), + untargetedRoutes: buildUntargetedRoutesIndex(t), internalTopology: t, } } +// PoliciesFromGateway returns Kuadrant Policies which +// directly or indirectly are targeting the gateway given as input. +// Type: Gateway -> []Policy func (k *KuadrantTopology) PoliciesFromGateway(gateway *gatewayapiv1.Gateway) []KuadrantPolicy { return k.gatewayPolicies[client.ObjectKeyFromObject(gateway)] } +// GetPolicyHTTPRoute returns the HTTPRoute being targetd by the policy. +// The method only returns existing and accepted (by parent gateways) HTTPRoutes +// Type: Policy -> HTTPRoute func (k *KuadrantTopology) GetPolicyHTTPRoute(policy KuadrantPolicy) *gatewayapiv1.HTTPRoute { return k.policyRoute[client.ObjectKeyFromObject(policy)] } -func (k *KuadrantTopology) GetFreeRoutes(gateway *gatewayapiv1.Gateway) []*gatewayapiv1.HTTPRoute { - return k.freeRoutes[client.ObjectKeyFromObject(gateway)] +// GetUntargetedRoutes returns the HTTPRoutes not targeted by any kuadrant policy +// having the gateway given as input as parent. +// Gateway -> []HTTPRoute +func (k *KuadrantTopology) GetUntargetedRoutes(gateway *gatewayapiv1.Gateway) []*gatewayapiv1.HTTPRoute { + return k.untargetedRoutes[client.ObjectKeyFromObject(gateway)] } // String representation of the topology @@ -154,9 +163,9 @@ func (k *KuadrantTopology) String() string { return index }() - freeRoutesPerGateway := func() map[string][]string { + untargetedRoutesPerGateway := func() map[string][]string { index := make(map[string][]string, 0) - for gatewayKey, routeList := range k.freeRoutes { + for gatewayKey, routeList := range k.untargetedRoutes { index[gatewayKey.String()] = Map(routeList, func(route *gatewayapiv1.HTTPRoute) string { return client.ObjectKeyFromObject(route).String() }) @@ -168,19 +177,19 @@ func (k *KuadrantTopology) String() string { }() topologyRepr := struct { - Gateways []Gateway `json:"gateways"` - Routes []Route `json:"routes"` - Policies []Policy `json:"policies"` - GatewayPolicies map[string][]string `json:"policiesPerGateway"` - PolicyRoute map[string]string `json:"policiesTargetingRoutes"` - FreeRoutes map[string][]string `json:"freeRoutesPerGateway"` + Gateways []Gateway `json:"gateways"` + Routes []Route `json:"routes"` + Policies []Policy `json:"policies"` + GatewayPolicies map[string][]string `json:"policiesPerGateway"` + PolicyRoute map[string]string `json:"policiesTargetingRoutes"` + UntargetedRoutes map[string][]string `json:"untargetedRoutesPerGateway"` }{ gateways, routes, policies, policiesPerGateway, policiesTargetingRoutes, - freeRoutesPerGateway, + untargetedRoutesPerGateway, } jsonData, err := json.MarshalIndent(topologyRepr, "", " ") @@ -227,7 +236,7 @@ func buildPolicyRouteIndex(t *gatewayAPITopology) map[client.ObjectKey]*gatewaya return index } -func buildFreeRoutesIndex(t *gatewayAPITopology) map[client.ObjectKey][]*gatewayapiv1.HTTPRoute { +func buildUntargetedRoutesIndex(t *gatewayAPITopology) map[client.ObjectKey][]*gatewayapiv1.HTTPRoute { // Build Gateway -> []HTTPRoute index with all the routes not targeted by a policy index := make(map[client.ObjectKey][]*gatewayapiv1.HTTPRoute, 0) diff --git a/pkg/common/kuadrant_topology_test.go b/pkg/common/kuadrant_topology_test.go index f9194bb40..7e19e524a 100644 --- a/pkg/common/kuadrant_topology_test.go +++ b/pkg/common/kuadrant_topology_test.go @@ -189,7 +189,7 @@ func TestGetPolicyHTTPRoute(t *testing.T) { }) } -func TestGetFreeRoutes(t *testing.T) { +func TestGetUntargetedRoutes(t *testing.T) { t.Run("gateway without routes", func(subT *testing.T) { // gw1 // policy1 -> gw1 @@ -201,8 +201,8 @@ func TestGetFreeRoutes(t *testing.T) { topology := NewKuadrantTopology(gateways, nil, policies) - freeRoutes := topology.GetFreeRoutes(gw1) - assert.Equal(subT, len(freeRoutes), 0) + untargetedRoutes := topology.GetUntargetedRoutes(gw1) + assert.Equal(subT, len(untargetedRoutes), 0) }) t.Run("all routes have policies", func(subT *testing.T) { @@ -224,11 +224,11 @@ func TestGetFreeRoutes(t *testing.T) { topology := NewKuadrantTopology(gateways, routes, policies) - freeRoutes := topology.GetFreeRoutes(gw1) - assert.Equal(subT, len(freeRoutes), 0) + untargetedRoutes := topology.GetUntargetedRoutes(gw1) + assert.Equal(subT, len(untargetedRoutes), 0) }) - t.Run("only one route is free", func(subT *testing.T) { + t.Run("only one route is untargeted", func(subT *testing.T) { // gw1 // route 1 -> gw1 // route 2 -> gw1 @@ -245,15 +245,15 @@ func TestGetFreeRoutes(t *testing.T) { topology := NewKuadrantTopology(gateways, routes, policies) - freeRoutes := topology.GetFreeRoutes(gw1) - assert.Equal(subT, len(freeRoutes), 1) + untargetedRoutes := topology.GetUntargetedRoutes(gw1) + assert.Equal(subT, len(untargetedRoutes), 1) assert.Equal(subT, - client.ObjectKeyFromObject(freeRoutes[0]), + client.ObjectKeyFromObject(untargetedRoutes[0]), client.ObjectKeyFromObject(route2), ) }) - t.Run("all routes are free", func(subT *testing.T) { + t.Run("all routes are untargeted", func(subT *testing.T) { // gw1 // route 1 -> gw1 // route 2 -> gw1 @@ -266,8 +266,8 @@ func TestGetFreeRoutes(t *testing.T) { topology := NewKuadrantTopology(gateways, routes, nil) - freeRoutes := topology.GetFreeRoutes(gw1) - assert.Equal(subT, len(freeRoutes), 2) + untargetedRoutes := topology.GetUntargetedRoutes(gw1) + assert.Equal(subT, len(untargetedRoutes), 2) }) } @@ -281,7 +281,7 @@ func TestKuadrantTopologyString(t *testing.T) { assert.Assert(subT, strings.Contains(topologyStr, `"policies": null`)) assert.Assert(subT, strings.Contains(topologyStr, `"policiesPerGateway": null`)) assert.Assert(subT, strings.Contains(topologyStr, `"policiesTargetingRoutes": null`)) - assert.Assert(subT, strings.Contains(topologyStr, `"freeRoutesPerGateway": null`)) + assert.Assert(subT, strings.Contains(topologyStr, `"untargetedRoutesPerGateway": null`)) }) t.Run("1 gateway 1 route 1 policy for route", func(subT *testing.T) { @@ -333,7 +333,7 @@ func TestKuadrantTopologyString(t *testing.T) { assert.Assert(subT, strings.Contains(topologyStr, `"policiesTargetingRoutes": { "nsA/policy1": "nsA/route1" }`)) - assert.Assert(subT, strings.Contains(topologyStr, `"freeRoutesPerGateway": { + assert.Assert(subT, strings.Contains(topologyStr, `"untargetedRoutesPerGateway": { "nsA/gw1": [] }`)) })