Skip to content

Commit

Permalink
rfc: RateLimitPolicy cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
guicassolato committed Feb 2, 2023
1 parent 953f620 commit d04d63a
Showing 1 changed file with 104 additions and 0 deletions.
104 changes: 104 additions & 0 deletions rfcs/0000-rlp-cleanup.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
# 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))

---

**_Stop reading now. Below this is all TODO._**

---

## Guide-level explanation
[guide-level-explanation]: #guide-level-explanation

Explain the proposal as if it was implemented and you were teaching it to Kuadrant user. That generally means:

- Introducing new named concepts.
- Explaining the feature largely in terms of examples.
- Explaining how a user should *think* about the feature, and how it would impact the way they already use Kuadrant. It should explain the impact as concretely as possible.
- If applicable, provide sample error messages, deprecation warnings, or migration guidance.
- If applicable, describe the differences between teaching this to existing and new Kuadrant users.

## Reference-level explanation
[reference-level-explanation]: #reference-level-explanation

This is the technical portion of the RFC. Explain the design in sufficient detail that:

- Its interaction with other features is clear.
- It is reasonably clear how the feature would be implemented.
- How error would be reported to the users.
- Corner cases are dissected by example.

The section should return to the examples given in the previous section, and explain more fully how the detailed proposal makes those examples work.

## Drawbacks
[drawbacks]: #drawbacks

Why should we *not* do this?

## Rationale and alternatives
[rationale-and-alternatives]: #rationale-and-alternatives

- Why is this design the best in the space of possible designs?
- What other designs have been considered and what is the rationale for not choosing them?
- What is the impact of not doing this?

## 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.

0 comments on commit d04d63a

Please sign in to comment.