Skip to content

Commit e9ecf64

Browse files
committed
Add review feedback
1 parent d2a5045 commit e9ecf64

File tree

1 file changed

+5
-4
lines changed

1 file changed

+5
-4
lines changed

docs/proposals/rate-limiting.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ server {
5151
- Act as a circuit-breaker for heavy endpoints.
5252
- Enable Canary / blue-green safety.
5353
- Add additional security to developer namespaces.
54-
- Fit my specific needs.
5554

5655
## API
5756

@@ -147,9 +146,11 @@ type RateLimitRule struct {
147146
// ZoneSize is the size of the shared memory zone.
148147
//
149148
// Directive: https://nginx.org/en/docs/http/ngx_http_limit_req_module.html#limit_req_zone
150-
ZoneSize Size `json:"zoneSize"`
149+
//
150+
// +optional
151+
ZoneSize *Size `json:"zoneSize"`
151152

152-
// Delay specifies a limit at which excessive requests become delayed. Default value is zero, i.e. all excessive requests are delayed.
153+
// Delay specifies a limit at which excessive requests become delayed. Default value is zero, which means all excessive requests are delayed.
153154
//
154155
// Default: 0
155156
// Directive: https://nginx.org/en/docs/http/ngx_http_limit_req_module.html#limit_req
@@ -440,7 +441,7 @@ Key validation rules:
440441
## Alternatives
441442

442443
- **Direct Policy**: If there's no strong use case for the Cluster Operator setting defaults for these settings on a Gateway, we could use a Direct Policy. However, since Rate Limit rules should be able to be defined on both Gateway and Routes, an Inherited Policy is the only Policy type for our solution.
443-
- **ExtensionRef approach**: We could use Gateway API's extensionRef mechanism instead of a Policy. However, Policy attachment is more appropriate for this use case as it follows the established pattern in NGINX Gateway Fabric, provides better status reporting, and allows for Rate Limit rules to be set by the Cluster Operator on a Gateway.
444+
- **ExtensionRef approach**: We could use Gateway API's extensionRef, aka Filter option, mechanism instead of a Policy. However, Policy attachment is more appropriate for this use case as it follows the established pattern in NGINX Gateway Fabric, provides better status reporting, and allows for Rate Limit rules to be set by the Cluster Operator on a Gateway.
444445
- Allow `RateLimitPolicies` attached at the Route level to overwrite rules set at the Gateway level. Currently if a Route `location` inherits a rate limit rule from a Gateway, there is no way to disable it or override it. The workaround around this problem is to either remove the Route from the Gateway, or remove the `RateLimitPolicy` from attaching at the Gateway level, and instead attach to the Routes on the Gateway. However, this is inconvinient and may be a common scenario warranting supporting through either a field in the `RateLimitRule` or changing how `RateLimitPolicies` interact with each other.
445446

446447
## Future Work

0 commit comments

Comments
 (0)