-
Notifications
You must be signed in to change notification settings - Fork 712
Setting disabled = true on a route should disable the virtual host global rate limit policy
#5657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e9f6234
0ff5a4b
27f8cd9
dcd74b8
890a899
6d1a7a1
8667603
bd9af46
171d135
26a35df
dece9ef
af28f65
9cdff28
4ee2c11
e005233
d77620f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| ## Specific routes can now opt out of the virtual host's global rate limit policy | ||
|
|
||
| Setting `rateLimitPolicy.global.disabled` flag to true on a specific route now disables the global rate limit policy inherited from the virtual host for that route. | ||
|
|
||
| ### Sample Configurations | ||
| In the example below, `/foo` route is opted out from the global rate limit policy defined by the virtualhost. | ||
| #### httpproxy.yaml | ||
| ```yaml | ||
| apiVersion: projectcontour.io/v1 | ||
| kind: HTTPProxy | ||
| metadata: | ||
| name: echo | ||
| spec: | ||
| virtualhost: | ||
| fqdn: local.projectcontour.io | ||
| rateLimitPolicy: | ||
| global: | ||
| descriptors: | ||
| - entries: | ||
| - remoteAddress: {} | ||
| - genericKey: | ||
| key: vhost | ||
| value: local.projectcontour.io | ||
| routes: | ||
| - conditions: | ||
| - prefix: / | ||
| services: | ||
| - name: ingress-conformance-echo | ||
| port: 80 | ||
| - conditions: | ||
| - prefix: /foo | ||
| rateLimitPolicy: | ||
| global: | ||
| disabled: true | ||
| services: | ||
| - name: ingress-conformance-echo | ||
| port: 80 | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -335,6 +335,9 @@ type Route struct { | |
| // RateLimitPolicy defines if/how requests for the route are rate limited. | ||
| RateLimitPolicy *RateLimitPolicy | ||
|
|
||
| // RateLimitPerRoute defines how the route should handle rate limits defined by the virtual host. | ||
| RateLimitPerRoute *RateLimitPerRoute | ||
|
|
||
| // RequestHashPolicies is a list of policies for configuring hashes on | ||
| // request attributes. | ||
| RequestHashPolicies []RequestHashPolicy | ||
|
|
@@ -571,6 +574,24 @@ type HeaderValueMatchDescriptorEntry struct { | |
| Value string | ||
| } | ||
|
|
||
| type VhRateLimitsType int | ||
|
|
||
| const ( | ||
| // VhRateLimitsOverride (Default) will use the virtual host rate limits unless the route has a rate limit policy. | ||
| VhRateLimitsOverride VhRateLimitsType = iota | ||
|
|
||
| // VhRateLimitsInclude will use the virtual host rate limits even if the route has a rate limit policy. | ||
| VhRateLimitsInclude | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have a strong opinion here. |
||
|
|
||
| // VhRateLimitsIgnore will ignore the virtual host rate limits even if the route does not have a rate limit policy. | ||
| VhRateLimitsIgnore | ||
| ) | ||
|
|
||
| // RateLimitPerRoute configures how the route should handle the rate limits defined by the virtual host. | ||
| type RateLimitPerRoute struct { | ||
| VhRateLimits VhRateLimitsType | ||
| } | ||
|
|
||
| // RemoteAddressDescriptorEntry configures a descriptor entry | ||
| // that contains the remote address (i.e. client IP). | ||
| type RemoteAddressDescriptorEntry struct{} | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be able to reuse
RateLimitPolicy.disabled. which will allow us to not introduce new fields.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So
dag. RateLimitPolicydoesn't really havedisabledon it, this noob is only on the API.I don't believe introducing this switch to DAG is a good idea as DAG is closer to Envoy than to the HTTPProxy API and also
RateLimitPerRouteis native to Envoy's struct for doing exactly this.