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.

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

Add annotations for controlling request rate limiting #4660

Merged
merged 5 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,22 @@ The table below summarizes the available annotations.
|``nginx.org/use-cluster-ip`` | N/A | Enables using the Cluster IP and port of the service instead of the default behavior of using the IP and port of the pods. When this field is enabled, the fields that configure NGINX behavior related to multiple upstream servers (like ``lb-method`` and ``next-upstream``) will have no effect, as NGINX Ingress Controller will configure NGINX with only one upstream server that will match the service Cluster IP. | ``False`` | |
{{% /table %}}

### Rate limiting

{{% table %}}
|Annotation | ConfigMap Key | Description | Default | Example |
| ---| ---| ---| ---| --- |
|``nginx.org/limit-req-rate`` | N/A | Enables request-rate-limiting for this ingress by creating a [limit_req_zone](https://nginx.org/en/docs/http/ngx_http_limit_req_module.html#limit_req_zone) and matching [limit_req](https://nginx.org/en/docs/http/ngx_http_limit_req_module.html#limit_req) for each location. All servers/locations of one ingress share the same zone. Must have unit r/s or r/m. | N/A | 200r/s |
|``nginx.org/limit-req-key`` | N/A | The key to which the rate limit is applied. Can contain text, variables, or a combination of them. Variables must be surrounded by ${}. | ${binary_remote_addr} | ${binary_remote_addr} |
|``nginx.org/limit-req-zone-size`` | N/A | Configures the size of the created [limit_req_zone](https://nginx.org/en/docs/http/ngx_http_limit_req_module.html#limit_req_zone). | 10m | 20m |
|``nginx.org/limit-req-delay`` | N/A | Configures the delay-parameter of the [limit_req](https://nginx.org/en/docs/http/ngx_http_limit_req_module.html#limit_req) directive. | 0 | 100 |
|``nginx.org/limit-req-no-delay`` | N/A | Configures the nodelay-parameter of the [limit_req](https://nginx.org/en/docs/http/ngx_http_limit_req_module.html#limit_req) directive. | false | true |
|``nginx.org/limit-req-burst`` | N/A | Configures the burst-parameter of the [limit_req](https://nginx.org/en/docs/http/ngx_http_limit_req_module.html#limit_req) directive. | N/A | 100 |
|``nginx.org/limit-req-dry-run`` | N/A | Enables the dry run mode. In this mode, the rate limit is not actually applied, but the number of excessive requests is accounted as usual in the shared memory zone. | false | true |
|``nginx.org/limit-req-log-level`` | N/A | Sets the desired logging level for cases when the server refuses to process requests due to rate exceeding, or delays request processing. Allowed values are info, notice, warn or error. | error | info |
|``nginx.org/limit-req-reject-code`` | N/A | Sets the status code to return in response to rejected requests. Must fall into the range 400..599. | 429 | 503 |
{{% /table %}}

### Snippets and Custom Templates

{{% table %}}
Expand Down
84 changes: 84 additions & 0 deletions internal/configs/annotations.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package configs

import (
"fmt"
"slices"

"github.com/golang/glog"
)

Expand Down Expand Up @@ -78,6 +81,15 @@ var minionInheritanceList = map[string]bool{
"nginx.org/max-fails": true,
"nginx.org/max-conns": true,
"nginx.org/fail-timeout": true,
"nginx.org/limit-req-rate": true,
"nginx.org/limit-req-key": true,
"nginx.org/limit-req-zone-size": true,
"nginx.org/limit-req-delay": true,
"nginx.org/limit-req-no-delay": true,
"nginx.org/limit-req-burst": true,
"nginx.org/limit-req-dry-run": true,
"nginx.org/limit-req-log-level": true,
"nginx.org/limit-req-reject-code": true,
}

var validPathRegex = map[string]bool{
Expand Down Expand Up @@ -413,9 +425,81 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool
cfgParams.UseClusterIP = useClusterIP
}
}

for _, err := range parseRateLimitAnnotations(ingEx.Ingress.Annotations, &cfgParams, ingEx.Ingress) {
glog.Error(err)
}

return cfgParams
}

// parseRateLimitAnnotations parses rate-limiting-related annotations and places them into cfgParams. Occurring errors are collected and returned, but do not abort parsing.
//
//gocyclo:ignore
func parseRateLimitAnnotations(annotations map[string]string, cfgParams *ConfigParams, context apiObject) []error {
errors := make([]error, 0)
if requestRateLimit, exists := annotations["nginx.org/limit-req-rate"]; exists {
if rate, err := ParseRequestRate(requestRateLimit); err != nil {
errors = append(errors, fmt.Errorf("Ingress %s/%s: Invalid value for nginx.org/limit-req-rate: got %s: %w", context.GetNamespace(), context.GetName(), requestRateLimit, err))
} else {
cfgParams.LimitReqRate = rate
}
}
if requestRateKey, exists := annotations["nginx.org/limit-req-key"]; exists {
cfgParams.LimitReqKey = requestRateKey
}
if requestRateZoneSize, exists := annotations["nginx.org/limit-req-zone-size"]; exists {
if size, err := ParseSize(requestRateZoneSize); err != nil {
errors = append(errors, fmt.Errorf("Ingress %s/%s: Invalid value for nginx.org/limit-req-zone-size: got %s: %w", context.GetNamespace(), context.GetName(), requestRateZoneSize, err))
} else {
cfgParams.LimitReqZoneSize = size
}
}
if requestRateDelay, exists, err := GetMapKeyAsInt(annotations, "nginx.org/limit-req-delay", context); exists {
if err != nil {
errors = append(errors, err)
} else {
cfgParams.LimitReqDelay = requestRateDelay
}
}
if requestRateNoDelay, exists, err := GetMapKeyAsBool(annotations, "nginx.org/limit-req-no-delay", context); exists {
if err != nil {
errors = append(errors, err)
} else {
cfgParams.LimitReqNoDelay = requestRateNoDelay
}
}
if requestRateBurst, exists, err := GetMapKeyAsInt(annotations, "nginx.org/limit-req-burst", context); exists {
if err != nil {
errors = append(errors, err)
} else {
cfgParams.LimitReqBurst = requestRateBurst
}
}
if requestRateDryRun, exists, err := GetMapKeyAsBool(annotations, "nginx.org/limit-req-dry-run", context); exists {
if err != nil {
errors = append(errors, err)
} else {
cfgParams.LimitReqDryRun = requestRateDryRun
}
}
if requestRateLogLevel, exists := annotations["nginx.org/limit-req-log-level"]; exists {
if !slices.Contains([]string{"info", "notice", "warn", "error"}, requestRateLogLevel) {
errors = append(errors, fmt.Errorf("Ingress %s/%s: Invalid value for nginx.org/limit-req-log-level: got %s", context.GetNamespace(), context.GetName(), requestRateLogLevel))
} else {
cfgParams.LimitReqLogLevel = requestRateLogLevel
}
}
if requestRateRejectCode, exists, err := GetMapKeyAsInt(annotations, "nginx.org/limit-req-reject-code", context); exists {
if err != nil {
errors = append(errors, err)
} else {
cfgParams.LimitReqRejectCode = requestRateRejectCode
}
}
return errors
}

func getWebsocketServices(ingEx *IngressEx) map[string]bool {
if value, exists := ingEx.Ingress.Annotations["nginx.org/websocket-services"]; exists {
return ParseServiceList(value)
Expand Down
56 changes: 56 additions & 0 deletions internal/configs/annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import (
"reflect"
"sort"
"testing"

networking "k8s.io/api/networking/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestParseRewrites(t *testing.T) {
Expand Down Expand Up @@ -159,3 +162,56 @@ func TestMergeMasterAnnotationsIntoMinion(t *testing.T) {
t.Errorf("mergeMasterAnnotationsIntoMinion returned %v, but expected %v", minionAnnotations, expectedMergedAnnotations)
}
}

func TestParseRateLimitAnnotations(t *testing.T) {
context := &networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "context",
},
}

if errors := parseRateLimitAnnotations(map[string]string{
"nginx.org/limit-req-rate": "200r/s",
"nginx.org/limit-req-key": "${request_uri}",
"nginx.org/limit-req-burst": "100",
"nginx.org/limit-req-delay": "80",
"nginx.org/limit-req-no-delay": "true",
"nginx.org/limit-req-reject-code": "429",
"nginx.org/limit-req-zone-size": "11m",
"nginx.org/limit-req-dry-run": "true",
"nginx.org/limit-req-log-level": "info",
}, NewDefaultConfigParams(false), context); len(errors) > 0 {
t.Error("Errors when parsing valid limit-req annotations")
}

if errors := parseRateLimitAnnotations(map[string]string{
"nginx.org/limit-req-rate": "200",
}, NewDefaultConfigParams(false), context); len(errors) == 0 {
t.Error("No Errors when parsing invalid request rate")
}

if errors := parseRateLimitAnnotations(map[string]string{
"nginx.org/limit-req-rate": "200r/h",
}, NewDefaultConfigParams(false), context); len(errors) == 0 {
t.Error("No Errors when parsing invalid request rate")
}

if errors := parseRateLimitAnnotations(map[string]string{
"nginx.org/limit-req-rate": "0r/s",
}, NewDefaultConfigParams(false), context); len(errors) == 0 {
t.Error("No Errors when parsing invalid request rate")
}

if errors := parseRateLimitAnnotations(map[string]string{
"nginx.org/limit-req-zone-size": "10abc",
}, NewDefaultConfigParams(false), context); len(errors) == 0 {
t.Error("No Errors when parsing invalid zone size")
}

if errors := parseRateLimitAnnotations(map[string]string{
"nginx.org/limit-req-log-level": "foobar",
}, NewDefaultConfigParams(false), context); len(errors) == 0 {
t.Error("No Errors when parsing invalid log level")
}
}
14 changes: 14 additions & 0 deletions internal/configs/config_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,16 @@ type ConfigParams struct {
SSLPorts []int

SpiffeServerCerts bool

LimitReqRate string
LimitReqKey string
LimitReqZoneSize string
LimitReqDelay int
LimitReqNoDelay bool
LimitReqBurst int
LimitReqDryRun bool
LimitReqLogLevel string
LimitReqRejectCode int
}

// StaticConfigParams holds immutable NGINX configuration parameters that affect the main NGINX config.
Expand Down Expand Up @@ -191,6 +201,10 @@ func NewDefaultConfigParams(isPlus bool) *ConfigParams {
MainKeepaliveRequests: 100,
VariablesHashBucketSize: 256,
VariablesHashMaxSize: 1024,
LimitReqKey: "${binary_remote_addr}",
LimitReqZoneSize: "10m",
LimitReqLogLevel: "error",
LimitReqRejectCode: 429,
}
}

Expand Down
35 changes: 35 additions & 0 deletions internal/configs/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings)
allWarnings := newWarnings()

var servers []version1.Server
var limitReqZones []version1.LimitReqZone

for _, rule := range p.ingEx.Ingress.Spec.Rules {
// skipping invalid hosts
Expand Down Expand Up @@ -265,6 +266,27 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings)
allWarnings.Add(warnings)
}

if cfgParams.LimitReqRate != "" {
zoneName := p.ingEx.Ingress.Namespace + "/" + p.ingEx.Ingress.Name
loc.LimitReq = &version1.LimitReq{
Zone: zoneName,
Burst: cfgParams.LimitReqBurst,
Delay: cfgParams.LimitReqDelay,
NoDelay: cfgParams.LimitReqNoDelay,
DryRun: cfgParams.LimitReqDryRun,
LogLevel: cfgParams.LimitReqLogLevel,
RejectCode: cfgParams.LimitReqRejectCode,
}
if !limitReqZoneExists(limitReqZones, zoneName) {
limitReqZones = append(limitReqZones, version1.LimitReqZone{
Name: zoneName,
Key: cfgParams.LimitReqKey,
Size: cfgParams.LimitReqZoneSize,
Rate: cfgParams.LimitReqRate,
})
}
}

locations = append(locations, loc)

if loc.Path == "/" {
Expand Down Expand Up @@ -317,6 +339,7 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings)
SpiffeClientCerts: p.staticParams.NginxServiceMesh && !cfgParams.SpiffeServerCerts,
DynamicSSLReloadEnabled: p.staticParams.DynamicSSLReload,
StaticSSLPath: p.staticParams.StaticSSLPath,
LimitReqZones: limitReqZones,
}, allWarnings
}

Expand Down Expand Up @@ -609,6 +632,7 @@ func generateNginxCfgForMergeableIngresses(p NginxCfgParams) (version1.IngressNg
var locations []version1.Location
var upstreams []version1.Upstream
healthChecks := make(map[string]version1.HealthCheck)
var limitReqZones []version1.LimitReqZone
var keepalive string

// replace master with a deepcopy because we will modify it
Expand Down Expand Up @@ -704,6 +728,7 @@ func generateNginxCfgForMergeableIngresses(p NginxCfgParams) (version1.IngressNg
}

upstreams = append(upstreams, nginxCfg.Upstreams...)
limitReqZones = append(limitReqZones, nginxCfg.LimitReqZones...)
}

masterServer.HealthChecks = healthChecks
Expand All @@ -717,9 +742,19 @@ func generateNginxCfgForMergeableIngresses(p NginxCfgParams) (version1.IngressNg
SpiffeClientCerts: p.staticParams.NginxServiceMesh && !p.baseCfgParams.SpiffeServerCerts,
DynamicSSLReloadEnabled: p.staticParams.DynamicSSLReload,
StaticSSLPath: p.staticParams.StaticSSLPath,
LimitReqZones: limitReqZones,
}, warnings
}

func limitReqZoneExists(zones []version1.LimitReqZone, zoneName string) bool {
for _, zone := range zones {
if zone.Name == zoneName {
return true
}
}
return false
}

func isSSLEnabled(isSSLService bool, cfgParams ConfigParams, staticCfgParams *StaticConfigParams) bool {
return isSSLService || staticCfgParams.NginxServiceMesh && !cfgParams.SpiffeServerCerts
}
Expand Down
Loading
Loading