Skip to content

Commit

Permalink
Add annotations for controlling request rate limiting (#4660)
Browse files Browse the repository at this point in the history
  • Loading branch information
dbaumgarten authored Jan 9, 2024
1 parent 0b0c658 commit cb42312
Show file tree
Hide file tree
Showing 11 changed files with 787 additions and 0 deletions.
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

0 comments on commit cb42312

Please sign in to comment.