Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove global-rate-limit feature #11851

Merged
merged 1 commit into from
Aug 25, 2024
Merged

Remove global-rate-limit feature #11851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions charts/ingress-nginx/tests/controller-configmap_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,8 @@ tests:
- it: should create a ConfigMap with templated values if `controller.config` contains templates
set:
controller.config:
global-rate-limit-memcached-host: "memcached.{{ .Release.Namespace }}.svc.kubernetes.local"
global-rate-limit-memcached-port: 11211
use-gzip: true
asserts:
- equal:
path: data.global-rate-limit-memcached-host
value: memcached.NAMESPACE.svc.kubernetes.local
- equal:
path: data.global-rate-limit-memcached-port
value: "11211"
Comment on lines -19 to -28
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was meant for testing different value types, not the global-rate-limit feature itself. Gonna re-add other values.

- equal:
path: data.use-gzip
value: "true"
5 changes: 0 additions & 5 deletions docs/e2e-tests.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ Do not try to edit it manually.


### [[Admission] admission controller](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/admission/admission.go#L39)
- [reject ingress with global-rate-limit annotations when memcached is not configured](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/admission/admission.go#L47)
- [should not allow overlaps of host and paths without canary annotations](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/admission/admission.go#L74)
- [should allow overlaps of host and paths with canary annotation](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/admission/admission.go#L91)
- [should block ingress with invalid path](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/admission/admission.go#L112)
Expand Down Expand Up @@ -173,8 +172,6 @@ Do not try to edit it manually.
### [from-to-www-redirect](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/annotations/fromtowwwredirect.go#L31)
- [should redirect from www HTTP to HTTP](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/annotations/fromtowwwredirect.go#L38)
- [should redirect from www HTTPS to HTTPS](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/annotations/fromtowwwredirect.go#L64)
### [annotation-global-rate-limit](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/annotations/globalratelimit.go#L30)
- [generates correct configuration](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/annotations/globalratelimit.go#L38)
### [backend-protocol - GRPC](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/annotations/grpc.go#L45)
- [should use grpc_pass in the configuration file](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/annotations/grpc.go#L48)
- [should return OK for service with backend protocol GRPC](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/annotations/grpc.go#L71)
Expand Down Expand Up @@ -420,8 +417,6 @@ Do not try to edit it manually.
### [global-options](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/settings/global_options.go#L28)
- [should have worker_rlimit_nofile option](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/settings/global_options.go#L31)
- [should have worker_rlimit_nofile option and be independent on amount of worker processes](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/settings/global_options.go#L37)
### [settings-global-rate-limit](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/settings/globalratelimit.go#L30)
- [generates correct NGINX configuration](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/settings/globalratelimit.go#L38)
### [GRPC](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/settings/grpc.go#L39)
- [should set the correct GRPC Buffer Size](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/settings/grpc.go#L42)
### [gzip](https://github.com/kubernetes/ingress-nginx/tree/main//test/e2e/settings/gzip.go#L30)
Expand Down
4 changes: 0 additions & 4 deletions docs/user-guide/nginx-configuration/annotations-risk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,6 @@
| ExternalAuth | auth-url | High | location |
| FastCGI | fastcgi-index | Medium | location |
| FastCGI | fastcgi-params-configmap | Medium | location |
| GlobalRateLimit | global-rate-limit | Low | ingress |
| GlobalRateLimit | global-rate-limit-ignored-cidrs | Medium | ingress |
| GlobalRateLimit | global-rate-limit-key | High | ingress |
| GlobalRateLimit | global-rate-limit-window | Low | ingress |
| HTTP2PushPreload | http2-push-preload | Low | location |
| LoadBalancing | load-balance | Low | location |
| Logs | enable-access-log | Low | location |
Expand Down
44 changes: 0 additions & 44 deletions docs/user-guide/nginx-configuration/annotations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
|[nginx.ingress.kubernetes.io/http2-push-preload](#http2-push-preload)|"true" or "false"|
|[nginx.ingress.kubernetes.io/limit-connections](#rate-limiting)|number|
|[nginx.ingress.kubernetes.io/limit-rps](#rate-limiting)|number|
|[nginx.ingress.kubernetes.io/global-rate-limit](#global-rate-limiting)|number|
|[nginx.ingress.kubernetes.io/global-rate-limit-window](#global-rate-limiting)|duration|
|[nginx.ingress.kubernetes.io/global-rate-limit-key](#global-rate-limiting)|string|
|[nginx.ingress.kubernetes.io/global-rate-limit-ignored-cidrs](#global-rate-limiting)|string|
|[nginx.ingress.kubernetes.io/permanent-redirect](#permanent-redirect)|string|
|[nginx.ingress.kubernetes.io/permanent-redirect-code](#permanent-redirect-code)|number|
|[nginx.ingress.kubernetes.io/temporal-redirect](#temporal-redirect)|string|
Expand Down Expand Up @@ -560,46 +556,6 @@ To configure settings globally for all Ingress rules, the `limit-rate-after` and

The client IP address will be set based on the use of [PROXY protocol](./configmap.md#use-proxy-protocol) or from the `X-Forwarded-For` header value when [use-forwarded-headers](./configmap.md#use-forwarded-headers) is enabled.

### Global Rate Limiting

**Note:** Be careful when configuring both (Local) Rate Limiting and Global Rate Limiting at the same time.
They are two completely different rate limiting implementations. Whichever limit exceeds first will reject the
requests. It might be a good idea to configure both of them to ease load on Global Rate Limiting backend
in cases of spike in traffic.

The stock NGINX rate limiting does not share its counters among different NGINX instances.
Given that most ingress-nginx deployments are elastic and number of replicas can change any day
it is impossible to configure a proper rate limit using stock NGINX functionalities.
Global Rate Limiting overcome this by using [lua-resty-global-throttle](https://github.com/ElvinEfendi/lua-resty-global-throttle). `lua-resty-global-throttle` shares its counters via a central store such as `memcached`.
The obvious shortcoming of this is users have to deploy and operate a `memcached` instance
in order to benefit from this functionality. Configure the `memcached`
using [these configmap settings](./configmap.md#global-rate-limit).

**Here are a few remarks for ingress-nginx integration of `lua-resty-global-throttle`:**

1. We minimize `memcached` access by caching exceeding limit decisions. The expiry of
cache entry is the desired delay `lua-resty-global-throttle` calculates for us.
The Lua Shared Dictionary used for that is `global_throttle_cache`. Currently its size defaults to 10M.
Customize it as per your needs using [lua-shared-dicts](./configmap.md#lua-shared-dicts).
When we fail to cache the exceeding limit decision then we log an NGINX error. You can monitor
for that error to decide if you need to bump the cache size. Without cache the cost of processing a
request is two memcached commands: `GET`, and `INCR`. With the cache it is only `INCR`.
1. Log NGINX variable `$global_rate_limit_exceeding`'s value to have some visibility into
what portion of requests are rejected (value `y`), whether they are rejected using cached decision (value `c`),
or if they are not rejected (default value `n`). You can use [log-format-upstream](./configmap.md#log-format-upstream)
to include that in access logs.
1. In case of an error it will log the error message and **fail open**.
1. The annotations below creates Global Rate Limiting instance per ingress.
That means if there are multiple paths configured under the same ingress,
the Global Rate Limiting will count requests to all the paths under the same counter.
Extract a path out into its own ingress if you need to isolate a certain path.


* `nginx.ingress.kubernetes.io/global-rate-limit`: Configures maximum allowed number of requests per window. Required.
* `nginx.ingress.kubernetes.io/global-rate-limit-window`: Configures a time window (i.e `1m`) that the limit is applied. Required.
* `nginx.ingress.kubernetes.io/global-rate-limit-key`: Configures a key for counting the samples. Defaults to `$remote_addr`. You can also combine multiple NGINX variables here, like `${remote_addr}-${http_x_api_client}` which would mean the limit will be applied to requests coming from the same API client (indicated by `X-API-Client` HTTP request header) with the same source IP address.
* `nginx.ingress.kubernetes.io/global-rate-limit-ignored-cidrs`: comma separated list of IPs and CIDRs to match client IP against. When there's a match request is not considered for rate limiting.

### Permanent Redirect

This annotation allows to return a permanent redirect (Return Code 301) instead of sending data to the upstream. For example `nginx.ingress.kubernetes.io/permanent-redirect: https://www.google.com` would redirect everything to Google.
Expand Down
22 changes: 0 additions & 22 deletions docs/user-guide/nginx-configuration/configmap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,6 @@ The following table shows a configuration option's name, type, and the default v
| [block-referers](#block-referers) | []string | "" | |
| [proxy-ssl-location-only](#proxy-ssl-location-only) | bool | "false" | |
| [default-type](#default-type) | string | "text/html" | |
| [global-rate-limit-memcached-host](#global-rate-limit) | string | "" | |
| [global-rate-limit-memcached-port](#global-rate-limit) | int | 11211 | |
| [global-rate-limit-memcached-connect-timeout](#global-rate-limit) | int | 50 | |
| [global-rate-limit-memcached-max-idle-timeout](#global-rate-limit) | int | 10000 | |
| [global-rate-limit-memcached-pool-size](#global-rate-limit) | int | 50 | |
| [global-rate-limit-status-code](#global-rate-limit) | int | 429 | |
| [service-upstream](#service-upstream) | bool | "false" | |
| [ssl-reject-handshake](#ssl-reject-handshake) | bool | "false" | |
| [debug-connections](#debug-connections) | []string | "127.0.0.1,1.1.1.1/24" | |
Expand Down Expand Up @@ -1349,22 +1343,6 @@ _**default:**_ text/html
_References:_
[https://nginx.org/en/docs/http/ngx_http_core_module.html#default_type](https://nginx.org/en/docs/http/ngx_http_core_module.html#default_type)

## global-rate-limit

* `global-rate-limit-status-code`: configure HTTP status code to return when rejecting requests. Defaults to 429.

Configure `memcached` client for [Global Rate Limiting](https://github.com/kubernetes/ingress-nginx/blob/main/docs/user-guide/nginx-configuration/annotations.md#global-rate-limiting).

* `global-rate-limit-memcached-host`: IP/FQDN of memcached server to use. Required to enable Global Rate Limiting.
* `global-rate-limit-memcached-port`: port of memcached server to use. Defaults default memcached port of `11211`.
* `global-rate-limit-memcached-connect-timeout`: configure timeout for connect, send and receive operations. Unit is millisecond. Defaults to 50ms.
* `global-rate-limit-memcached-max-idle-timeout`: configure timeout for cleaning idle connections. Unit is millisecond. Defaults to 50ms.
* `global-rate-limit-memcached-pool-size`: configure number of max connections to keep alive. Make sure your `memcached` server can handle
`global-rate-limit-memcached-pool-size * worker-processes * <number of ingress-nginx replicas>` simultaneous connections.

These settings get used by [lua-resty-global-throttle](https://github.com/ElvinEfendi/lua-resty-global-throttle)
that ingress-nginx includes. Refer to the link to learn more about `lua-resty-global-throttle`.

## service-upstream

Set if the service's Cluster IP and port should be used instead of a list of all endpoints. This can be overwritten by an annotation on an Ingress rule.
Expand Down
9 changes: 0 additions & 9 deletions images/nginx-1.25/rootfs/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,6 @@ export LUA_RESTY_REDIS_VERSION=8641b9f1b6f75cca50c90cf8ca5c502ad8950aa8
# Check for recent changes: https://github.com/api7/lua-resty-ipmatcher/compare/v0.6.1...master
export LUA_RESTY_IPMATCHER_VERSION=3e93c53eb8c9884efe939ef070486a0e507cc5be

# Check for recent changes: https://github.com/ElvinEfendi/lua-resty-global-throttle/compare/v0.2.0...main
export LUA_RESTY_GLOBAL_THROTTLE_VERSION=v0.2.0

# Check for recent changes: https://github.com/microsoft/mimalloc/compare/v2.1.7...master
export MIMALOC_VERSION=v2.1.7

Expand Down Expand Up @@ -276,9 +273,6 @@ get_src c15aed1a01c88a3a6387d9af67a957dff670357f5fdb4ee182beb44635eef3f1 \
get_src efb767487ea3f6031577b9b224467ddbda2ad51a41c5867a47582d4ad85d609e \
"https://github.com/api7/lua-resty-ipmatcher/archive/$LUA_RESTY_IPMATCHER_VERSION.tar.gz" "lua-resty-ipmatcher"

get_src 0fb790e394510e73fdba1492e576aaec0b8ee9ef08e3e821ce253a07719cf7ea \
"https://github.com/ElvinEfendi/lua-resty-global-throttle/archive/$LUA_RESTY_GLOBAL_THROTTLE_VERSION.tar.gz" "lua-resty-global-throttle"

get_src d74f86ada2329016068bc5a243268f1f555edd620b6a7d6ce89295e7d6cf18da \
"https://github.com/microsoft/mimalloc/archive/${MIMALOC_VERSION}.tar.gz" "mimalloc"

Expand Down Expand Up @@ -591,9 +585,6 @@ make install
cd "$BUILD_PATH/lua-resty-ipmatcher"
INST_LUADIR=/usr/local/lib/lua make install

cd "$BUILD_PATH/lua-resty-global-throttle"
make install

cd "$BUILD_PATH/mimalloc"
mkdir -p out/release
cd out/release
Expand Down
Loading