Skip to content

Commit

Permalink
[rlp-v2] WasmPlugin reconciliation (Kuadrant#204)
Browse files Browse the repository at this point in the history
* reconciliation of route selectors for RLPs targeting HTTPRoutes

* Change `common.Contains(slice []string, target string)` to any comparable type (using generics)
* Define a `common.HTTPRouteRuleSelector` type that checks whether a `HTTPRouteMatch` selects a `HTTPRouteRule` or not
* Define a `rlptools.HTTPRouteRulesFromRouteSelector` function that returns all rules from a HTTPRoute that a route selector selects (using `common.HTTPRouteRuleSelector`)
* Modify `rlptools.conditionsFromLimit` to use `rlptools.HTTPRouteRulesFromRouteSelector` and generate wasm conditions for each combination of selected route rule match and hostname
* Ensure all generated route conditions include a pattern for the hostnames (route selectors’, HTTPRoute’s or Gateway’s)
* Ensure all generated route conditions include the user-defined `when` conditions

* add tests for generating a wasmplugin config out of a complex httproute and complex ratelimitpolicy

The httproute declares 2 rules and 2 hostnames.

The RLP declares 2 limits:
- one limit declares one counter, targets one of the httprouterules, for one hostname only, with an additional 'when' condition and counter qualifier;
- the other limit declares 2 counters, targets the other httprouterule, for all hostnames, and states no additional 'when' condition nor counter qualifier.

* generate wasm rules only for routeSelector-filtered hostnames that belong to the targeted route

* fix: pattern expression for hostnames with the right operator - introduces 'endswith' pattern operator

* fix: compute wasm rules only for the applicable hostnames

Passes a list of hostnames into the call to the function that generates the wasm rules for a given RLP, notoriously to compute the conditions when to apply each limit. The list of hostnames is the domain intersection between the hostnames specified in the HTTPRoute and its parent gateway for which the wasm config is being built. The route selection considers this as a boundary, so it doesn’t generate rules for the wrong gateway/hostname.

Fixes the computation of conditions when no HTTPRouteRule matches the route selectors.

* do not generate wasm condition patterns for the hostname when all apply

* do not repeat RLPs for each hostname within the wasm config

* `rlptools.HTTPRouteRulesFromRouteSelector` moved to `api/v1beta2/route_selectors.go` along with the `RouteSelector` type itself

+ no longer injecting a separate list of hostnames for avoiding building wasm rules for hostnames that do not apply, as this did not seem to belong to the route selector logic anyway.

* reconciliation of route selectors for RLPs targeting Gateways

Get all HTTPRoutes that are children of the Gateway and call the generation of the set of wasm rules (conditions and data for each limit) for each HTTPRoute

* match the gateway hostnames instead of the httproute ones when building the wasm rules for rlps that target a gateway

* refactor: make HTTPRouteRuleSelector.Selects return sooner if the HTTP method does not match

* refactor: ensure wasm rule conditions are generated out of selected HTTPRouteRules in the same order as the selectors select the rules

* Prevent the usage of routeSelectors in RLPs that target a Gateway

* do not generate conditions for gateway policy wasm rules from httproutes that have a rlp of its own and avoid building wasmplugins when there are no rules to apply

* avoid adding rlps without any rules to the wasm config
  • Loading branch information
guicassolato authored and KevFan committed Aug 10, 2023
1 parent 541307c commit c906414
Show file tree
Hide file tree
Showing 17 changed files with 2,152 additions and 325 deletions.
27 changes: 11 additions & 16 deletions api/v1beta2/ratelimitpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
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"
)

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
Expand All @@ -40,13 +39,14 @@ import (
// +kubebuilder:validation:MaxLength=253
type ContextSelector string

// +kubebuilder:validation:Enum:=eq;neq;startswith;incl;excl;matches
// +kubebuilder:validation:Enum:=eq;neq;startswith;endswith;incl;excl;matches
type WhenConditionOperator string

const (
EqualOperator WhenConditionOperator = "eq"
NotEqualOperator WhenConditionOperator = "neq"
StartsWithOperator WhenConditionOperator = "startswith"
EndsWithOperator WhenConditionOperator = "endswith"
IncludeOperator WhenConditionOperator = "incl"
ExcludeOperator WhenConditionOperator = "excl"
MatchesOperator WhenConditionOperator = "matches"
Expand Down Expand Up @@ -83,20 +83,6 @@ type WhenCondition struct {
Value string `json:"value"`
}

// RouteSelector defines semantics for matching an HTTP request based on conditions
// https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRouteSpec
type RouteSelector struct {
// Hostnames defines a set of hostname that should match against the HTTP Host header to select a HTTPRoute to process the request
// https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRouteSpec
// +optional
Hostnames []gatewayapiv1beta1.Hostname `json:"hostnames,omitempty"`

// Matches define conditions used for matching the rule against incoming HTTP requests.
// https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRouteSpec
// +optional
Matches []gatewayapiv1beta1.HTTPRouteMatch `json:"matches,omitempty"`
}

// Limit represents a complete rate limit configuration
type Limit struct {
// RouteSelectors defines semantics for matching an HTTP request based on conditions
Expand Down Expand Up @@ -197,6 +183,15 @@ func (r *RateLimitPolicy) Validate() error {
return fmt.Errorf("invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", *r.Spec.TargetRef.Namespace)
}

// prevents usage of routeSelectors in a gateway RLP
if r.Spec.TargetRef.Kind == gatewayapiv1alpha2.Kind("Gateway") {
for _, limit := range r.Spec.Limits {
if len(limit.RouteSelectors) > 0 {
return fmt.Errorf("route selectors not supported when targetting a Gateway")
}
}
}

return nil
}

Expand Down
52 changes: 52 additions & 0 deletions api/v1beta2/route_selectors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package v1beta2

import (
gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

orderedmap "github.com/elliotchance/orderedmap/v2"

"github.com/kuadrant/kuadrant-operator/pkg/common"
)

// RouteSelector defines semantics for matching an HTTP request based on conditions
// https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRouteSpec
type RouteSelector struct {
// Hostnames defines a set of hostname that should match against the HTTP Host header to select a HTTPRoute to process the request
// https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRouteSpec
// +optional
Hostnames []gatewayapiv1beta1.Hostname `json:"hostnames,omitempty"`

// Matches define conditions used for matching the rule against incoming HTTP requests.
// https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRouteSpec
// +optional
Matches []gatewayapiv1beta1.HTTPRouteMatch `json:"matches,omitempty"`
}

// SelectRules returns, from a HTTPRoute, all HTTPRouteRules that either specify no HTTRouteMatches or that contain at
// least one HTTRouteMatch whose statements expressly include (partially or totally) the statements of at least one of
// the matches of the selector. If the selector does not specify any matches, then all HTTPRouteRules are selected.
//
// Additionally, if the selector specifies a non-empty list of hostnames, a non-empty intersection between the literal
// hostnames of the selector and set of hostnames specified in the HTTPRoute must exist. Otherwise, the function
// returns nil.
func (s *RouteSelector) SelectRules(route *gatewayapiv1beta1.HTTPRoute) (rules []gatewayapiv1beta1.HTTPRouteRule) {
rulesIndices := orderedmap.NewOrderedMap[int, gatewayapiv1beta1.HTTPRouteRule]()
if len(s.Hostnames) > 0 && !common.Intersect(s.Hostnames, route.Spec.Hostnames) {
return nil
}
if len(s.Matches) == 0 {
return route.Spec.Rules
}
for _, routeSelectorMatch := range s.Matches {
for idx, rule := range route.Spec.Rules {
rs := common.HTTPRouteRuleSelector{HTTPRouteMatch: &routeSelectorMatch}
if rs.Selects(rule) {
rulesIndices.Set(idx, rule)
}
}
}
for el := rulesIndices.Front(); el != nil; el = el.Next() {
rules = append(rules, el.Value)
}
return
}
211 changes: 211 additions & 0 deletions api/v1beta2/route_selectors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
//go:build unit

package v1beta2

import (
"fmt"
"reflect"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

"github.com/kuadrant/kuadrant-operator/pkg/common"
)

func TestRouteSelectors(t *testing.T) {
gatewayHostnames := []gatewayapiv1beta1.Hostname{
"*.toystore.com",
}

gateway := &gatewayapiv1beta1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Name: "my-gateway",
},
}

for _, hostname := range gatewayHostnames {
gateway.Spec.Listeners = append(gateway.Spec.Listeners, gatewayapiv1beta1.Listener{Hostname: &hostname})
}

route := &gatewayapiv1beta1.HTTPRoute{
Spec: gatewayapiv1beta1.HTTPRouteSpec{
CommonRouteSpec: gatewayapiv1beta1.CommonRouteSpec{
ParentRefs: []gatewayapiv1beta1.ParentReference{
{
Name: gatewayapiv1beta1.ObjectName(gateway.Name),
},
},
},
Hostnames: []gatewayapiv1beta1.Hostname{"api.toystore.com"},
Rules: []gatewayapiv1beta1.HTTPRouteRule{
{
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
// get /toys*
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Type: &[]gatewayapiv1beta1.PathMatchType{gatewayapiv1beta1.PathMatchPathPrefix}[0],
Value: &[]string{"/toy"}[0],
},
Method: &[]gatewayapiv1beta1.HTTPMethod{gatewayapiv1beta1.HTTPMethod("GET")}[0],
},
// post /toys*
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Type: &[]gatewayapiv1beta1.PathMatchType{gatewayapiv1beta1.PathMatchPathPrefix}[0],
Value: &[]string{"/toy"}[0],
},
Method: &[]gatewayapiv1beta1.HTTPMethod{gatewayapiv1beta1.HTTPMethod("POST")}[0],
},
},
},
{
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
// /assets*
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Type: &[]gatewayapiv1beta1.PathMatchType{gatewayapiv1beta1.PathMatchPathPrefix}[0],
Value: &[]string{"/assets"}[0],
},
},
},
},
},
},
}

testCases := []struct {
name string
routeSelector RouteSelector
route *gatewayapiv1beta1.HTTPRoute
expected []gatewayapiv1beta1.HTTPRouteRule
}{
{
name: "empty route selector selects all HTTPRouteRules",
routeSelector: RouteSelector{},
route: route,
expected: route.Spec.Rules,
},
{
name: "route selector selects the HTTPRouteRules whose set of HTTPRouteMatch is a perfect match",
routeSelector: RouteSelector{
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Type: &[]gatewayapiv1beta1.PathMatchType{gatewayapiv1beta1.PathMatchPathPrefix}[0],
Value: &[]string{"/assets"}[0],
},
},
},
},
route: route,
expected: []gatewayapiv1beta1.HTTPRouteRule{route.Spec.Rules[1]},
},
{
name: "route selector selects the HTTPRouteRules whose set of HTTPRouteMatch contains at least one match",
routeSelector: RouteSelector{
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Type: &[]gatewayapiv1beta1.PathMatchType{gatewayapiv1beta1.PathMatchPathPrefix}[0],
Value: &[]string{"/toy"}[0],
},
Method: &[]gatewayapiv1beta1.HTTPMethod{gatewayapiv1beta1.HTTPMethod("POST")}[0],
},
},
},
route: route,
expected: []gatewayapiv1beta1.HTTPRouteRule{route.Spec.Rules[0]},
},
{
name: "route selector with missing part of a HTTPRouteMatch still selects the HTTPRouteRules that match",
routeSelector: RouteSelector{
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Type: &[]gatewayapiv1beta1.PathMatchType{gatewayapiv1beta1.PathMatchPathPrefix}[0],
Value: &[]string{"/toy"}[0],
},
},
},
},
route: route,
expected: []gatewayapiv1beta1.HTTPRouteRule{route.Spec.Rules[0]},
},
{
name: "route selector selects no HTTPRouteRule when no criterion matches",
routeSelector: RouteSelector{
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Type: &[]gatewayapiv1beta1.PathMatchType{gatewayapiv1beta1.PathMatchExact}[0],
Value: &[]string{"/toy"}[0],
},
},
},
},
route: route,
expected: nil,
},
{
name: "route selector selects the HTTPRouteRules whose HTTPRoute's hostnames match the selector",
routeSelector: RouteSelector{
Hostnames: []gatewayapiv1beta1.Hostname{"api.toystore.com"},
},
route: route,
expected: route.Spec.Rules,
},
{
name: "route selector selects the HTTPRouteRules whose HTTPRoute's hostnames match the selector additionally to other criteria",
routeSelector: RouteSelector{
Hostnames: []gatewayapiv1beta1.Hostname{"api.toystore.com"},
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Type: &[]gatewayapiv1beta1.PathMatchType{gatewayapiv1beta1.PathMatchPathPrefix}[0],
Value: &[]string{"/toy"}[0],
},
},
},
},
route: route,
expected: []gatewayapiv1beta1.HTTPRouteRule{route.Spec.Rules[0]},
},
{
name: "route selector does not select HTTPRouteRules whose HTTPRoute's hostnames do not match the selector",
routeSelector: RouteSelector{
Hostnames: []gatewayapiv1beta1.Hostname{"www.toystore.com"},
},
route: route,
expected: nil,
},
{
name: "route selector does not select HTTPRouteRules whose HTTPRoute's hostnames do not match the selector even when other criteria match",
routeSelector: RouteSelector{
Hostnames: []gatewayapiv1beta1.Hostname{"www.toystore.com"},
Matches: []gatewayapiv1beta1.HTTPRouteMatch{
{
Path: &gatewayapiv1beta1.HTTPPathMatch{
Type: &[]gatewayapiv1beta1.PathMatchType{gatewayapiv1beta1.PathMatchPathPrefix}[0],
Value: &[]string{"/toy"}[0],
},
},
},
},
route: route,
expected: nil,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
rules := tc.routeSelector.SelectRules(tc.route)
rulesToStringSlice := func(rules []gatewayapiv1beta1.HTTPRouteRule) []string {
return common.Map(common.Map(rules, common.HTTPRouteRuleToString), func(r string) string { return fmt.Sprintf("{%s}", r) })
}
if !reflect.DeepEqual(rules, tc.expected) {
t.Errorf("expected %v, got %v", rulesToStringSlice(tc.expected), rulesToStringSlice(rules))
}
})
}
}
1 change: 1 addition & 0 deletions bundle/manifests/kuadrant.io_ratelimitpolicies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ spec:
- eq
- neq
- startswith
- endswith
- incl
- excl
- matches
Expand Down
1 change: 1 addition & 0 deletions config/crd/bases/kuadrant.io_ratelimitpolicies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ spec:
- eq
- neq
- startswith
- endswith
- incl
- excl
- matches
Expand Down
Loading

0 comments on commit c906414

Please sign in to comment.