diff --git a/rfcs/0000-rlp-cleanup.md b/rfcs/0000-rlp-cleanup.md new file mode 100644 index 00000000..ad377484 --- /dev/null +++ b/rfcs/0000-rlp-cleanup.md @@ -0,0 +1,331 @@ +# RFC RateLimitPolicy cleanup + +- Feature Name: `rlp-v2` +- Start Date: (fill me in with today's date, YYYY-MM-DD) +- RFC PR: [Kuadrant/architecture#0000](https://github.com/Kuadrant/architecture/pull/0000) +- Issue tracking: [Kuadrant/architecture#0000](https://github.com/Kuadrant/architecture/issues/0000) + +## Summary +[summary]: #summary + +Proposal of clean-up of Kuadrant's `RateLimitPolicy` API (RLP) for improved UX. + +## Motivation +[motivation]: #motivation + +The [`RateLimitPolicy`](https://pkg.go.dev/github.com/kuadrant/kuadrant-operator@v0.2.1/api/v1beta1#RateLimitPolicy) API (v1beta1), particularly its [`RateLimit`](https://pkg.go.dev/github.com/kuadrant/kuadrant-operator@v0.2.1/api/v1beta1#RateLimit) type used in `ratelimitpolicy.spec.rateLimits`, designed in part to fit the underlying implementation based on the Envoy [Rate limit](https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/rate_limit_filter) filter, has been proven to be (i) _unnecessarily complex_, as well as (ii) possibly _limiting for the extension of the API_ either beyond this specific implementation and/or for supporting use cases of rate limiting not contemplated in the original design, such as comprising the concept of _weights_. + +Users of the `RateLimitPolicy` will immediatelly recognize elements of Envoy's Rate limit API in the definitions of the `RateLimit` type, with almost 1:1 correspondence between the [`Configuration`](https://pkg.go.dev/github.com/kuadrant/kuadrant-operator@v0.2.1/api/v1beta1#Configuration) and [`Rule`](https://pkg.go.dev/github.com/kuadrant/kuadrant-operator@v0.2.1/api/v1beta1#Rule) types and their counterparts in the Envoy configuration. Although compatibility between those continue to be desired, the leakage of such implementation details to the level of the API could otherwise be avoided to provide a better abstraction for the context in vogue, where activators ("matchers") and payload ("descriptors") are defined by users in a same document instead of in different configurations of different components. + +Furthermore, the also related [`Limit`](https://pkg.go.dev/github.com/kuadrant/kuadrant-operator@v0.2.1/api/v1beta1#Limit) type – used as well in the `RateLimitPolicy`'s `RateLimit` – implies presently a logical relationship between its inner concepts – i.e. conditions and variables on one side, and limits themselves on the other – that otherwise could be shaped in a different manner, to provide (i) clearer understanding of the meaning of these concepts by the user as well as (ii) to avoid repetition. + +### Goals of cleaning up the RLP + +1. Decouple the API from the underlying implementation - i.e. provide a more generic abstraction +2. Simplify the API - i.e. DRY, straight to the point (for defining rate limits) +3. Consistency with Kuadrant's [AuthPolicy](https://pkg.go.dev/github.com/kuadrant/kuadrant-operator@v0.2.1/api/v1beta1#AuthPolicy) API - i.e. same language, similar UX + +### Current WIP to consider while discussing the cleaning up of the RLP + +1. Inheritance of the network matching rules from the upstream network object (https://github.com/Kuadrant/architecture/pull/4) +2. Improvements to RateLimitPolicy - aka "minimal working example" without `configurations`, `limits.conditions` and `limits.variables` (https://github.com/Kuadrant/kuadrant-operator/issues/133) +3. Implement `skip_if_absent` for the RequestHeaders action (https://github.com/Kuadrant/wasm-shim/issues/29) +4. Rate Limiting across clusters ([doc](https://docs.google.com/document/d/1pqCODRAkNUTLB6lJfRWcv3d2Hlj9WqsGySmjrP707Vk/edit#heading=h.nzpgr3pef6uy)) + +## Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +Given the following HTTPRoute: + +```yaml +apiVersion: gateway.networking.k8s.io/v1alpha2 +kind: HTTPRoute +metadata: + name: toystore + labels: + app: toystore +spec: + parentRefs: + - name: istio-ingressgateway + namespace: istio-system + hostnames: ["*.toystore.com"] + rules: + - matches: + - path: + type: PathPrefix + value: "/toys" + method: GET + - path: + type: PathPrefix + value: "/toys" + method: POST + backendRefs: + - name: toystore + port: 80 +``` + +These are examples of RateLimitPolicies targeting the route: + +**Example 1.** Minimal example - unconditional, unqualified rate limiting + +```yaml +apiVersion: kuadrant.io/v2beta1 +kind: RateLimitPolicy +metadata: + name: toystore +spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: toystore + limits: + - rates: + - limit: 5 + unit: minute +``` + +**Example 2.** All you can get - conditions, counter qualifiers, multiple rates, and weights + +```yaml +apiVersion: kuadrant.io/v2beta1 +kind: RateLimitPolicy +metadata: + name: toystore +spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: toystore + limits: + - name: writers + selectors: + # this is a condition + - selector: request.http.method + operator: eq + value: POST + # this is counter qualifier + - selector: metadata.auth.identity.username + rates: + - limit: 5 + duration: 1 + unit: minute + - limit: 100 + duration: 12 + unit: hour + weight: 1 + + - name: readers + selectors: + - selector: request.http.method + operator: eq + value: GET + - selector: metadata.auth.identity.username + rates: + - limit: 50 + duration: 1 + unit: minute + weight: 1 + + - name: read-expensive + selectors: + - selector: request.http.method + operator: eq + value: GET + - selector: request.http.path + operator: eq + value: /toys/expensive + rates: + - limit: 10 + duration: 1 + unit: minute + weight: 1 +``` + +## Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +### Differences and similarities compared to current RateLimitPolicy + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
CurrentNewReason
1:1 relation between Limit (object) and the actual Rate limit (value)Rate becomes a detail of Limit where each limit may define one or more rates (1:N) +
    +
  • It allows to reuse selectors (previously "conditions" and "variables") for groups of rate limits
    • +
+
Conditions and variables in separate fields of the limitSelectors – with provided relational value ("conditions") or without provided relational value ("counter qualifiers") +
    +
  • Simpler structure for defining the "scope" of a group of rate limits
    • +
+
Parsed conditionsStructured condition fields composed of selector, operator and value + +
Configurations as a list of variables assignments ("actions" for building "descriptors")Descriptors computed from selectors effectively used to specify conditions and counter qualifiers +
    +
  • Abstract Envoy-specific concepts of "actions" and "descriptors"
    • +
  • No room for useless invariant descriptors (e.g. "limited = 1")
    • +
  • Source value of the selectors defined from an implicit "context" data structure
    • +
+
Key-value descriptorsStructured descriptors from a "context" data structure + +
maxValuelimit
secondsduration and unit +
    +
  • Support for more units beyond seconds
    • +
  • duration: 1 by default
    • +
+
No weights (or implicit weight: 1)weight: VALUE +
    +
  • Placeholder for (more complex) weighting systems in the future – e.g. %, 𝑓(), etc
    • +
  • Migration path from other RL solutions, e.g. 3scale (?)
    • +
+
+ +## Drawbacks +[drawbacks]: #drawbacks + +Why should we *not* do this? + +## Rationale and alternatives +[rationale-and-alternatives]: #rationale-and-alternatives + +### Possible variations for the selectors (conditions and counter qualifiers) + +The main drivers behind the proposed "selectors" designed based on structured condition expressions and single-field for conditions and counter qualifiers (variation "A" below) were: +1. Consistency with the Authorino API +2. Simplicity + +Nonetheless here are a few alternative variations to consider: + + + + + + + + + + + + + + + + + + + + + +
Structured condition expressionsParsed condition expressions
Single field + A + 
+selectors:
+  - selector: request.http.method
+    operator: eq
+    value: GET
+  - selector: metadata.auth.identity.username
+ 		+ B + 
+selectors:
+  - request.http.method == "GET"
+  - metadata.auth.identity.username
+
Distinct fields + C + 
+conditions:
+  - selector: request.http.method
+    operator: eq
+    value: GET
+counters:
+  - metadata.auth.identity.username
+ 		+ D + 
+conditions:
+  - request.http.method == "GET"
+counters:
+  - metadata.auth.identity.username
+
+ +## Prior art +[prior-art]: #prior-art + +Discuss prior art, both the good and the bad, in relation to this proposal. +A few examples of what this can include are: + +- Does another project have a similar feature? +- What can be learned from it? What's good? What's less optimal? +- Papers: Are there any published papers or great posts that discuss this? If you have some relevant papers to refer to, this can serve as a more detailed theoretical background. + +This section is intended to encourage you as an author to think about the lessons from other tentatives - successful or not, provide readers of your RFC with a fuller picture. + +Note that while precedent set by other projects is some motivation, it does not on its own motivate an RFC. + +## Unresolved questions +[unresolved-questions]: #unresolved-questions + +- What parts of the design do you expect to resolve through the RFC process before this gets merged? +- What parts of the design do you expect to resolve through the implementation of this feature before stabilization? +- What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC? + +## Future possibilities +[future-possibilities]: #future-possibilities + +Think about what the natural extension and evolution of your proposal would be and how it would affect the platform and project as a whole. Try to use this section as a tool to further consider all possible interactions with the project and its components in your proposal. Also consider how this all fits into the roadmap for the project and of the relevant sub-team. + +This is also a good place to "dump ideas", if they are out of scope for the RFC you are writing but otherwise related. + +Note that having something written down in the future-possibilities section is not a reason to accept the current or a future RFC; such notes should be in the section on motivation or rationale in this or subsequent RFCs. The section merely provides additional information.