Skip to content

Commit

Permalink
fixup! remove routeSelectors from v1beta3
Browse files Browse the repository at this point in the history
  • Loading branch information
eguzki committed Oct 4, 2024
1 parent 3e60d00 commit 0152aa5
Show file tree
Hide file tree
Showing 8 changed files with 13 additions and 337 deletions.
31 changes: 8 additions & 23 deletions doc/rate-limiting.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ The `RateLimitPolicy` spec includes, basically, two parts:
Each limit definition includes:
* A set of rate limits (`spec.limits.<limit-name>.rates[]`)
* (Optional) A set of dynamic counter qualifiers (`spec.limits.<limit-name>.counters[]`)
* (Optional) A set of route selectors, to further qualify the specific routing rules when to activate the limit (`spec.limits.<limit-name>.routeSelectors[]`)
* (Optional) A set of additional dynamic conditions to activate the limit (`spec.limits.<limit-name>.when[]`)

The limit definitions (`limits`) can be declared at the top-level level of the spec (with the semantics of _defaults_) or alternatively within explicit `defaults` or `overrides` blocks.
Expand Down Expand Up @@ -73,12 +72,6 @@ spec:
# Check out Kuadrant RFC 0002 (https://github.com/Kuadrant/architecture/blob/main/rfcs/0002-well-known-attributes.md) to learn more about the Well-known Attributes that can be used in this field.
counters: […]

# Further qualification of the scpecific HTTPRouteRules within the targeted HTTPRoute that should trigger the limit.
# Each element contains a HTTPRouteMatch object that will be used to select HTTPRouteRules that include at least one identical HTTPRouteMatch.
# The HTTPRouteMatch part does not have to be fully identical, but the what's stated in the selector must be identically stated in the HTTPRouteRule.
# Do not use it on RateLimitPolicies that target a Gateway.
routeSelectors: […]

# Additional dynamic conditions to trigger the limit.
# Use it for filtering attributes not supported by HTTPRouteRule or with RateLimitPolicies that target a Gateway.
# Check out Kuadrant RFC 0002 (https://github.com/Kuadrant/architecture/blob/main/rfcs/0002-well-known-attributes.md) to learn more about the Well-known Attributes that can be used in this field.
Expand All @@ -103,8 +96,6 @@ spec:
When a RateLimitPolicy targets a HTTPRoute, the policy is enforced to all traffic routed according to the rules and hostnames specified in the HTTPRoute, across all Gateways referenced in the `spec.parentRefs` field of the HTTPRoute.

The targeted HTTPRoute's rules and/or hostnames to which the policy must be enforced can be filtered to specific subsets, by specifying the [`routeSelectors`](reference/route-selectors.md#the-routeselectors-field) field of the limit definition.

Target a HTTPRoute by setting the `spec.targetRef` field of the RateLimitPolicy as follows:

```yaml
Expand Down Expand Up @@ -195,7 +186,6 @@ Expected behavior:
### Limit definition

A limit will be activated whenever a request comes in and the request matches:
- any of the route rules selected by the limit (via [`routeSelectors`](reference/route-selectors.md#the-routeselectors-field) or implicit "catch-all" selector), and
- all of the `when` conditions specified in the limit.

A limit can define:
Expand Down Expand Up @@ -225,19 +215,20 @@ spec:
unit: minute
counters:
- auth.identity.username
routeSelectors:
hostnames:
- api.toystore.com
when:
- selector: request.host
operator: eq
value: "api.toystore.com"
"toystore-admin-unverified-users":
rates:
- limit: 250
duration: 1
unit: second
routeSelectors:
hostnames:
- admin.toystore.com
when:
- selector: request.host
operator: eq
value: "admin.toystore.com"
- selector: auth.identity.email_verified
operator: eq
value: "false"
Expand All @@ -249,15 +240,9 @@ spec:
| `admin.toystore.com` | 250rps |
| `other.toystore.com` | 5000rps |

### Route selectors

Route selectors allow targeting sections of a HTTPRoute, by specifying sets of HTTPRouteMatches and/or hostnames that make the policy controller look up within the HTTPRoute spec for compatible declarations, and select the corresponding HTTPRouteRules and hostnames, to then build conditions that activate the policy or policy rule.

Check out [Route selectors](reference/route-selectors.md) for a full description, semantics and API reference.

#### `when` conditions

`when` conditions can be used to scope a limit (i.e. to filter the traffic to which a limit definition applies) without any coupling to the underlying network topology, i.e. without making direct references to HTTPRouteRules via [`routeSelectors`](reference/route-selectors.md#the-routeselectors-field).
`when` conditions can be used to scope a limit (i.e. to filter the traffic to which a limit definition applies) without any coupling to the underlying network topology, i.e. without making direct references to HTTPRouteRules.

Use `when` conditions to conditionally activate limits based on attributes that cannot be expressed in the HTTPRoutes' `spec.hostnames` and `spec.rules.matches` fields, or in general in RateLimitPolicies that target a Gateway.

Expand Down
1 change: 0 additions & 1 deletion doc/reference/ratelimitpolicy.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
|------------------|-----------------------------------------------------|:------------:|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `rates` | [][RateLimit](#ratelimit) | No | List of rate limits associated with the limit definition |
| `counters` | []String | No | List of rate limit counter qualifiers. Items must be a valid [Well-known attribute](https://github.com/Kuadrant/architecture/blob/main/rfcs/0002-well-known-attributes.md). Each distinct value resolved in the data plane starts a separate counter for each rate limit. |
| `routeSelectors` | [][RouteSelector](route-selectors.md#routeselector) | No | List of selectors of HTTPRouteRules whose matching rules activate the limit. At least one HTTPRouteRule must be selected to activate the limit. If omitted, all HTTPRouteRules of the targeted HTTPRoute activate the limit. Do not use it in policies targeting a Gateway. |
| `when` | [][WhenCondition](#whencondition) | No | List of additional dynamic conditions (expressions) to activate the limit. All expression must evaluate to true for the limit to be applied. Use it for filtering attributes that cannot be expressed in the targeted HTTPRoute's `spec.hostnames` and `spec.rules.matches` fields, or when targeting a Gateway. |

#### RateLimit
Expand Down
10 changes: 4 additions & 6 deletions doc/user-guides/simple-rl-for-app-developers.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,10 @@ spec:
- limit: 5
duration: 10
unit: second
routeSelectors:
- matches: # selects the 2nd HTTPRouteRule of the targeted route
- method: POST
path:
type: Exact
value: "/toys"
when:
- selector: request.method
operator: eq
value: "POST"
EOF
```
Expand Down
6 changes: 0 additions & 6 deletions examples/toystore/ratelimitpolicy_httproute.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@ spec:
- limit: 5
duration: 30
unit: second
routeSelectors:
- matches:
- path:
type: Exact
value: "/toy"
method: GET

"admin-post-or-delete-toy-per-user":
rates:
Expand Down
3 changes: 1 addition & 2 deletions pkg/rlptools/wasm/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"

kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2"
kuadrantv1beta3 "github.com/kuadrant/kuadrant-operator/api/v1beta3"
"github.com/kuadrant/kuadrant-operator/pkg/common"
kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi"
Expand Down Expand Up @@ -117,7 +116,7 @@ func conditionsFromLimit(limit *kuadrantv1beta3.Limit, route *gatewayapiv1.HTTPR
routeConditions := make([]Condition, 0)

// build conditions from all rules
hostnamesForConditions := (&kuadrantv1beta2.RouteSelector{}).HostnamesForConditions(route)
hostnamesForConditions := route.Spec.Hostnames
for _, rule := range route.Spec.Rules {
routeConditions = append(routeConditions, conditionsFromRule(rule, hostnamesForConditions)...)
}
Expand Down
192 changes: 0 additions & 192 deletions pkg/rlptools/wasm/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,198 +118,6 @@ func TestRules(t *testing.T) {
},
},
},
{
name: "RLP with route selector based on hostname",
rlp: rlp("my-rlp", map[string]kuadrantv1beta3.Limit{
"50rps-for-selected-hostnames": {
Rates: []kuadrantv1beta3.Rate{counter50rps},
RouteSelectors: []kuadrantv1beta3.RouteSelector{
{
Hostnames: []gatewayapiv1.Hostname{
"*.example.com",
"myapp.apps.example.com", // ignored
},
},
},
},
}),
route: httpRoute,
expectedRules: []Rule{
{
Conditions: []Condition{
{
AllOf: []PatternExpression{
{
Selector: "request.url_path",
Operator: PatternOperator(kuadrantv1beta3.StartsWithOperator),
Value: "/toy",
},
{
Selector: "request.method",
Operator: PatternOperator(kuadrantv1beta3.EqualOperator),
Value: "GET",
},
{
Selector: "request.host",
Operator: PatternOperator(kuadrantv1beta3.EndsWithOperator),
Value: ".example.com",
},
},
},
},
Actions: []Action{
{
Scope: "my-app/my-rlp",
ExtensionName: RateLimitPolicyExtensionName,
Data: []DataType{
{
Value: &Static{
Static: StaticSpec{
Key: "limit.50rps_for_selected_hostnames__ac4044ab",
Value: "1",
},
},
},
},
},
},
},
},
},
{
name: "RLP with route selector based on http route matches (full match)",
rlp: rlp("my-rlp", map[string]kuadrantv1beta3.Limit{
"50rps-for-selected-route": {
Rates: []kuadrantv1beta3.Rate{counter50rps},
RouteSelectors: []kuadrantv1beta3.RouteSelector{
{
Matches: []gatewayapiv1.HTTPRouteMatch{
{
Path: &gatewayapiv1.HTTPPathMatch{
Type: &[]gatewayapiv1.PathMatchType{gatewayapiv1.PathMatchPathPrefix}[0],
Value: &[]string{"/toy"}[0],
},
Method: &[]gatewayapiv1.HTTPMethod{"GET"}[0],
},
},
},
},
},
}),
route: httpRoute,
expectedRules: []Rule{
{
Conditions: []Condition{
{
AllOf: []PatternExpression{
{
Selector: "request.url_path",
Operator: PatternOperator(kuadrantv1beta3.StartsWithOperator),
Value: "/toy",
},
{
Selector: "request.method",
Operator: PatternOperator(kuadrantv1beta3.EqualOperator),
Value: "GET",
},
},
},
},
Actions: []Action{
{
Scope: "my-app/my-rlp",
ExtensionName: RateLimitPolicyExtensionName,
Data: []DataType{
{
Value: &Static{
Static: StaticSpec{
Key: "limit.50rps_for_selected_route__db289136",
Value: "1",
},
},
},
},
},
},
},
},
},
{
name: "RLP with route selector based on http route matches (partial match)",
rlp: rlp("my-rlp", map[string]kuadrantv1beta3.Limit{
"50rps-for-selected-path": {
Rates: []kuadrantv1beta3.Rate{counter50rps},
RouteSelectors: []kuadrantv1beta3.RouteSelector{
{
Matches: []gatewayapiv1.HTTPRouteMatch{
{
Path: &gatewayapiv1.HTTPPathMatch{
Type: &[]gatewayapiv1.PathMatchType{gatewayapiv1.PathMatchPathPrefix}[0],
Value: &[]string{"/toy"}[0],
},
},
},
},
},
},
}),
route: httpRoute,
expectedRules: []Rule{
{
Conditions: []Condition{
{
AllOf: []PatternExpression{
{
Selector: "request.url_path",
Operator: PatternOperator(kuadrantv1beta3.StartsWithOperator),
Value: "/toy",
},
{
Selector: "request.method",
Operator: PatternOperator(kuadrantv1beta3.EqualOperator),
Value: "GET",
},
},
},
},
Actions: []Action{
{
Scope: "my-app/my-rlp",
ExtensionName: RateLimitPolicyExtensionName,
Data: []DataType{
{
Value: &Static{
Static: StaticSpec{
Key: "limit.50rps_for_selected_path__38eb97a4",
Value: "1",
},
},
},
},
},
},
},
},
},
{
name: "RLP with mismatching route selectors",
rlp: rlp("my-rlp", map[string]kuadrantv1beta3.Limit{
"50rps-for-non-existent-route": {
Rates: []kuadrantv1beta3.Rate{counter50rps},
RouteSelectors: []kuadrantv1beta3.RouteSelector{
{
Matches: []gatewayapiv1.HTTPRouteMatch{
{
Method: &[]gatewayapiv1.HTTPMethod{"POST"}[0],
},
},
},
},
},
}),
route: httpRoute,
expectedRules: []Rule{},
},
{
name: "HTTPRouteRules without rule matches",
rlp: rlp("my-rlp", map[string]kuadrantv1beta3.Limit{
Expand Down
40 changes: 0 additions & 40 deletions tests/common/ratelimitpolicy/ratelimitpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1594,44 +1594,4 @@ var _ = Describe("RateLimitPolicy CEL Validations", func() {
Expect(k8sClient.Create(ctx, policy)).To(Succeed())
}, testTimeOut)
})

Context("Route Selector Validation", func() {
const (
gateWayRouteSelectorErrorMessage = "route selectors not supported when targeting a Gateway"
)

It("invalid usage of limit route selectors with a gateway targetRef", func(ctx SpecContext) {
policy := policyFactory(func(policy *kuadrantv1beta3.RateLimitPolicy) {
policy.Spec.TargetRef.Kind = "Gateway"
policy.Spec.TargetRef.Name = "my-gw"
policy.Spec.RateLimitPolicyCommonSpec = kuadrantv1beta3.RateLimitPolicyCommonSpec{
Limits: map[string]kuadrantv1beta3.Limit{
"l1": {
Rates: []kuadrantv1beta3.Rate{
{
Limit: 1, Duration: 3, Unit: "minute",
},
},
RouteSelectors: []kuadrantv1beta3.RouteSelector{
{
Hostnames: []gatewayapiv1.Hostname{"*.foo.io"},
Matches: []gatewayapiv1.HTTPRouteMatch{
{
Path: &gatewayapiv1.HTTPPathMatch{
Value: ptr.To("/foo"),
},
},
},
},
},
},
},
}
})

err := k8sClient.Create(ctx, policy)
Expect(err).To(Not(BeNil()))
Expect(strings.Contains(err.Error(), gateWayRouteSelectorErrorMessage)).To(BeTrue())
}, testTimeOut)
})
})
Loading

0 comments on commit 0152aa5

Please sign in to comment.