Skip to content

Commit

Permalink
Merge the rules for a RLP that targets a GW into all RLPs that target…
Browse files Browse the repository at this point in the history 
… HTTPRoutes linked to that GW

Merging the rules of the gateway RLP is needed because the wasm-shim indexes all RLPs but will only enforce one RLP config per request. It looks up in the index for the RLP config whose associated hostnames contain a match, by checking the longest (most specific) hostnames first. (This is the same behaviour of how Authorino works.)

Without merging the gateway RLP rules in, while having RLPs targetting at both levels (HTTPRoute and Gateway), a request whose hostname matches a hostname of the HTTPRoute would trigger only the limits from the route RLP and not the ones from the gateway RLP.
  • Loading branch information
@guicassolato
guicassolato committed Jul 11, 2023
1 parent 61edeea commit 185f46f
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 184 deletions.
114 changes: 2 additions & 112 deletions controllers/ratelimitpolicy_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,11 +488,6 @@ var _ = Describe("RateLimitPolicy controller", 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{"*.example.com"})
err := k8sClient.Create(context.Background(), httpRoute)
Expect(err).ToNot(HaveOccurred())

// create ratelimitpolicy
rlp := &kuadrantv1beta2.RateLimitPolicy{
TypeMeta: metav1.TypeMeta{
Expand Down Expand Up @@ -520,7 +515,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
},
},
}
err = k8sClient.Create(context.Background(), rlp)
err := k8sClient.Create(context.Background(), rlp)
Expect(err).ToNot(HaveOccurred())

// Check RLP status is available
Expand Down Expand Up @@ -575,22 +570,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
Domain: fmt.Sprintf("%s/%s#%s", testNamespace, gwName, "*"),
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",
},
},
},
},
Conditions: nil,
Data: []wasm.DataItem{
{
Static: &wasm.StaticSpec{
Expand All @@ -616,96 +596,6 @@ var _ = Describe("RateLimitPolicy controller", func() {
Expect(err).ToNot(HaveOccurred())
Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(common.RateLimitPoliciesBackRefAnnotation, string(serialized)))
})

It("Creates all the resources for a basic Gateway and RateLimitPolicy when missing a HTTPRoute attached to the Gateway", 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: gatewayapiv1beta1.Group("gateway.networking.k8s.io"),
Kind: "Gateway",
Name: gatewayapiv1beta1.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 Gateway direct back reference
gwKey := client.ObjectKeyFromObject(gateway)
existingGateway := &gatewayapiv1beta1.Gateway{}
err = k8sClient.Get(context.Background(), gwKey, existingGateway)
// must exist
Expect(err).ToNot(HaveOccurred())
Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(
common.RateLimitPolicyBackRefAnnotation, client.ObjectKeyFromObject(rlp).String()))

// check limits
limitadorKey := client.ObjectKey{Name: common.LimitadorName, Namespace: testNamespace}
existingLimitador := &limitadorv1alpha1.Limitador{}
err = k8sClient.Get(context.Background(), limitadorKey, existingLimitador)
// must exist
Expect(err).ToNot(HaveOccurred())
Expect(existingLimitador.Spec.Limits).To(ContainElements(limitadorv1alpha1.RateLimit{
MaxValue: 1,
Seconds: 3 * 60,
Namespace: common.MarshallNamespace(client.ObjectKeyFromObject(gateway), "*"),
Conditions: []string{fmt.Sprintf("%s/%s/l1 == \"1\"", testNamespace, rlpName)},
Variables: []string{},
}))

// Check envoy filter
efName := fmt.Sprintf("kuadrant-ratelimiting-cluster-%s", gwName)
efKey := client.ObjectKey{Name: efName, Namespace: testNamespace}
existingEF := &istioclientnetworkingv1alpha3.EnvoyFilter{}
err = k8sClient.Get(context.Background(), efKey, existingEF)
// must exist
Expect(err).ToNot(HaveOccurred())

// Check wasm plugin
wpName := fmt.Sprintf("kuadrant-%s", gwName)
wasmPluginKey := client.ObjectKey{Name: wpName, Namespace: testNamespace}
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.WASMPlugin{
FailureMode: wasm.FailureModeDeny,
RateLimitPolicies: []wasm.RateLimitPolicy{},
}))

// Check gateway back references
err = k8sClient.Get(context.Background(), gwKey, existingGateway)
// must exist
Expect(err).ToNot(HaveOccurred())
refs := []client.ObjectKey{rlpKey}
serialized, err := json.Marshal(refs)
Expect(err).ToNot(HaveOccurred())
Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(common.RateLimitPoliciesBackRefAnnotation, string(serialized)))
})
})
})

Expand Down
85 changes: 42 additions & 43 deletions controllers/ratelimitpolicy_wasm_plugins.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,11 @@ func (r *RateLimitPolicyReconciler) wasmPluginConfig(ctx context.Context, gw com
gwHostnames = []gatewayapiv1beta1.Hostname{"*"}
}

type objects struct {
rlp kuadrantv1beta2.RateLimitPolicy
httpRoute gatewayapiv1beta1.HTTPRoute
}
rlps := make(map[string]*objects, 0)
rlps := make(map[string]kuadrantv1beta2.RateLimitPolicy, 0)
var gwRLPFound bool
gwRLPWasmRules := make([]wasm.Rule, 0)

// fetch and store all RLPs affecting this gateway, and prebuilds the wasm rules for the gateway RLP if there is one
for _, rlpKey := range rlpRefs {
rlp := &kuadrantv1beta2.RateLimitPolicy{}
err := r.Client().Get(ctx, rlpKey, rlp)
Expand All @@ -143,63 +142,63 @@ func (r *RateLimitPolicyReconciler) wasmPluginConfig(ctx context.Context, gw com
return nil, err
}

rlps[rlpKey.String()] = *rlp

// target ref is a HTTPRoute
if common.IsTargetRefHTTPRoute(rlp.Spec.TargetRef) {
httpRoute, err := r.FetchValidHTTPRoute(ctx, rlp.TargetKey())
if err != nil {
return nil, err
}
rlps[rlpKey.String()] = &objects{rlp: *rlp, httpRoute: *httpRoute}
continue
}

// target ref is a Gateway
if rlps[rlpKey.String()] != nil {
// prevents multiple RLPs targeting the gateway
if gwRLPFound {
return nil, fmt.Errorf("wasmPluginConfig: multiple gateway RLP found and only one expected. rlp keys: %v", rlpRefs)
}
// build a single httproute with all rules from all httproutes attached to the gateway and the hostnames of the gateway.
// the hostnames of the gateway are used because, otherwise, the rlp that targets the gateway would have to specify
// hostnames for route selection exactly as they are stated in the httproutes, not as they stated in the gateway,
// and this would be counterintuitive for the user, because, unlike other types of route selection (e.g. by path),
// the hostnames are part of the gateway spec.
rules := make([]gatewayapiv1beta1.HTTPRouteRule, 0)
for _, route := range r.FetchAcceptedGatewayHTTPRoutes(ctx, rlp.TargetKey()) {
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 rlp", "ratelimitpolicy", rlpKey)
continue
}
rlps[rlpKey.String()] = &objects{
rlp: *rlp,
httpRoute: gatewayapiv1beta1.HTTPRoute{
Spec: gatewayapiv1beta1.HTTPRouteSpec{
Hostnames: gwHostnames,
Rules: rules,
},
gwRLPFound = true

// prebuilds wasm rules for the limits in the gateway RLP
gwRLPWasmRules = append(gwRLPWasmRules, rlptools.WasmRules(rlp, &gatewayapiv1beta1.HTTPRoute{
Spec: gatewayapiv1beta1.HTTPRouteSpec{
Hostnames: gwHostnames,
Rules: []gatewayapiv1beta1.HTTPRouteRule{{}},
},
}
})...)
}

wasmPlugin := &wasm.WASMPlugin{
FailureMode: wasm.FailureModeDeny,
RateLimitPolicies: make([]wasm.RateLimitPolicy, 0),
}

for rlpKey, objs := range rlps {
// modifies (in memory) the list of hostnames specified in the route so we don't generate rules that only apply to other gateways
// this is a no-op for the rlp that targets the gateway
httpRoute := objs.httpRoute
hostnames := common.FilterValidSubdomains(gwHostnames, httpRoute.Spec.Hostnames)
if len(hostnames) == 0 { // it should only happen when the route specifies no hostnames
hostnames = gwHostnames
// builds the wasm plugin rate limit policy configs for each RLP affecting this gateway
// keeps same the order as the policies are referenced in the gateway
for _, rlpKey := range rlpRefs {
rlp := rlps[rlpKey.String()]

hostnames := gwHostnames
wasmRules := gwRLPWasmRules

// target ref is a HTTPRoute
if common.IsTargetRefHTTPRoute(rlp.Spec.TargetRef) {
httpRoute, err := r.FetchValidHTTPRoute(ctx, rlp.TargetKey())
if err != nil {
return nil, err
}

// narrows the list of hostnames specified in the route so we don't generate rules that only apply to other gateways
validRouteHostnames := common.FilterValidSubdomains(gwHostnames, httpRoute.Spec.Hostnames)
if len(validRouteHostnames) > 0 {
hostnames = validRouteHostnames
}
httpRoute.Spec.Hostnames = hostnames

// merge the wasm rules built from the gateway RLP with the rules from the HTTPRoute one
wasmRules = append(wasmRules, rlptools.WasmRules(&rlp, httpRoute)...)
}
httpRoute.Spec.Hostnames = hostnames

wasmPlugin.RateLimitPolicies = append(wasmPlugin.RateLimitPolicies, wasm.RateLimitPolicy{
Name: rlpKey,
Name: rlpKey.String(),
Domain: common.MarshallNamespace(gw.Key(), string(hostnames[0])), // TODO(guicassolato): https://github.com/Kuadrant/kuadrant-operator/issues/201. Meanwhile, we are using the first hostname so it matches at least one set of limit definitions in the Limitador CR
Rules: rlptools.WasmRules(&objs.rlp, &httpRoute),
Rules: wasmRules,
Hostnames: common.HostnamesToStrings(hostnames), // we might be listing more hostnames than needed due to route selectors hostnames possibly being more restrictive
Service: common.KuadrantRateLimitClusterName,
})
Expand Down
29 changes: 0 additions & 29 deletions pkg/reconcilers/targetref_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,35 +94,6 @@ func (r *TargetRefReconciler) FetchValidTargetRef(ctx context.Context, targetRef
return nil, fmt.Errorf("FetchValidTargetRef: targetRef (%v) to unknown network resource", targetRef)
}

// FetchAcceptedGatewayHTTPRoutes returns the list of HTTPRoutes that have been accepted as children of a gateway.
func (r *TargetRefReconciler) FetchAcceptedGatewayHTTPRoutes(ctx context.Context, gwKey client.ObjectKey) (routeKeys []gatewayapiv1beta1.HTTPRoute) {
logger, _ := logr.FromContext(ctx)
logger = logger.WithName("FetchAcceptedGatewayHTTPRoutes").WithValues("gateway", gwKey)

routeList := &gatewayapiv1beta1.HTTPRouteList{}
err := r.Client().List(ctx, routeList)
if err != nil {
logger.V(1).Info("failed to list httproutes", "err", err)
return
}

for _, route := range routeList.Items {
routeParentStatus, found := common.Find(route.Status.RouteStatus.Parents, func(p gatewayapiv1beta1.RouteParentStatus) bool {
return *p.ParentRef.Kind == ("Gateway") &&
((p.ParentRef.Namespace == nil && route.GetNamespace() == gwKey.Namespace) || string(*p.ParentRef.Namespace) == gwKey.Namespace) &&
string(p.ParentRef.Name) == gwKey.Name
})
if found && meta.IsStatusConditionTrue(routeParentStatus.Conditions, "Accepted") {
logger.V(1).Info("found route attached to gateway", "httproute", client.ObjectKeyFromObject(&route))
routeKeys = append(routeKeys, route)
continue
}
logger.V(1).Info("skipping route, not attached to gateway", "httproute", client.ObjectKeyFromObject(&route))
}

return
}

// TargetedGatewayKeys returns the list of gateways that are being referenced from the target.
func (r *TargetRefReconciler) TargetedGatewayKeys(ctx context.Context, targetNetworkObject client.Object) []client.ObjectKey {
switch obj := targetNetworkObject.(type) {
Expand Down

0 comments on commit 185f46f

Please sign in to comment.