Add support to make delay, size, burst configurable in limit_req directive

docs/user-guide/nginx-configuration/annotations.md

@@ -534,9 +534,13 @@ These annotations define limits on connections and transmission rates.  These ca
 * `nginx.ingress.kubernetes.io/limit-rps`: number of requests accepted from a given IP each second. The burst limit is set to this limit multiplied by the burst multiplier, the default multiplier is 5. When clients exceed this limit,  [limit-req-status-code](https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#limit-req-status-code) ***default:*** 503 is returned.
 * `nginx.ingress.kubernetes.io/limit-rpm`: number of requests accepted from a given IP each minute. The burst limit is set to this limit multiplied by the burst multiplier, the default multiplier is 5. When clients exceed this limit,  [limit-req-status-code](https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#limit-req-status-code) ***default:*** 503 is returned.
 * `nginx.ingress.kubernetes.io/limit-burst-multiplier`: multiplier of the limit rate for burst size. The default burst multiplier is 5, this annotation override the default multiplier. When clients exceed this limit,  [limit-req-status-code](https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#limit-req-status-code) ***default:*** 503 is returned.
+* `nginx.ingress.kubernetes.io/limit-no-burst`: When set to true, burst size is not set. The default value is false.
+* `nginx.ingress.kubernetes.io/limit-delay`: The delay parameter defines the point at which, within the burst size, excessive requests are throttled (delayed) to comply with the defined rate limit. When set to non negative number, it means nodelay. Default value is -1.
 * `nginx.ingress.kubernetes.io/limit-rate-after`: initial number of kilobytes after which the further transmission of a response to a given connection will be rate limited. This feature must be used with [proxy-buffering](#proxy-buffering) enabled.
 * `nginx.ingress.kubernetes.io/limit-rate`: number of kilobytes per second allowed to send to a given connection.  The zero value disables rate limiting. This feature must be used with [proxy-buffering](#proxy-buffering) enabled.
 * `nginx.ingress.kubernetes.io/limit-whitelist`: client IP source ranges to be excluded from rate-limiting. The value is a comma separated list of CIDRs.
+* `nginx.ingress.kubernetes.io/limit-shared-size`: Set size of shared memory zone. The default value is 5.
+
 
 If you specify multiple annotations in a single Ingress rule, limits are applied in the order `limit-connections`, `limit-rpm`, `limit-rps`.
 

internal/ingress/annotations/ratelimit/main.go

@@ -104,6 +104,7 @@ type Zone struct {
 	Name  string `json:"name"`
 	Limit int    `json:"limit"`
 	Burst int    `json:"burst"`
+	Delay int    `json:"delay"`
 	// SharedSize amount of shared memory for the zone
 	SharedSize int `json:"sharedSize"`
 }
@@ -125,6 +126,9 @@ func (z1 *Zone) Equal(z2 *Zone) bool {
 	if z1.Burst != z2.Burst {
 		return false
 	}
+	if z1.Delay != z2.Delay {
+		return false
+	}
 	if z1.SharedSize != z2.SharedSize {
 		return false
 	}
@@ -141,6 +145,9 @@ const (
 	limitRateBurstMultiplierAnnotation = "limit-burst-multiplier"
 	limitWhitelistAnnotation           = "limit-whitelist" // This annotation is an alias for limit-allowlist
 	limitAllowlistAnnotation           = "limit-allowlist"
+	limitRateNoBurstAnnotation         = "limit-no-burst"
+	limitRateDelayAnnotation           = "limit-delay"
+	limitRateSharedSizeAnnotation      = "limit-shared-size"
 )
 
 var rateLimitAnnotations = parser.Annotation{
@@ -191,6 +198,24 @@ var rateLimitAnnotations = parser.Annotation{
 			Documentation:     `List of CIDR/IP addresses that will not be rate-limited.`,
 			AnnotationAliases: []string{limitWhitelistAnnotation},
 		},
+		limitRateNoBurstAnnotation: {
+			Validator:     parser.ValidateBool,
+			Scope:         parser.AnnotationScopeLocation,
+			Risk:          parser.AnnotationRiskLow, // Low, as it allows just a set of options
+			Documentation: `To enable/disable burst in ratelimiting`,
+		},
+		limitRateDelayAnnotation: {
+			Validator:     parser.ValidateInt,
+			Scope:         parser.AnnotationScopeLocation,
+			Risk:          parser.AnnotationRiskLow, // Low, as it allows just a set of options
+			Documentation: `Adds delay in nginx rate limiting`,
+		},
+		limitRateSharedSizeAnnotation: {
+			Validator:     parser.ValidateInt,
+			Scope:         parser.AnnotationScopeLocation,
+			Risk:          parser.AnnotationRiskLow, // Low, as it allows just a set of options
+			Documentation: `Used to configure the sharedSize in ratelimit zone`,
+		},
 	},
 }
 
@@ -237,6 +262,22 @@ func (a ratelimit) Parse(ing *networking.Ingress) (interface{}, error) {
 		burstMultiplier = defBurst
 	}
 
+	//nolint:errcheck // No need to check err here
+	noburst, _ := parser.GetBoolAnnotation(limitRateNoBurstAnnotation, ing, a.annotationConfig.Annotations)
+	if noburst {
+		burstMultiplier = 0
+	}
+
+	delay, err := parser.GetIntAnnotation(limitRateDelayAnnotation, ing, a.annotationConfig.Annotations)
+	if err != nil {
+		delay = -1
+	}
+
+	sharedSize, err := parser.GetIntAnnotation(limitRateSharedSizeAnnotation, ing, a.annotationConfig.Annotations)
+	if err != nil {
+		sharedSize = defSharedSize
+	}
+
 	val, err := parser.GetStringAnnotation(limitAllowlistAnnotation, ing, a.annotationConfig.Annotations)
 	if err != nil && errors.IsValidationError(err) {
 		return nil, err
@@ -264,19 +305,22 @@ func (a ratelimit) Parse(ing *networking.Ingress) (interface{}, error) {
 			Name:       fmt.Sprintf("%v_conn", zoneName),
 			Limit:      conn,
 			Burst:      conn * burstMultiplier,
-			SharedSize: defSharedSize,
+			Delay:      delay,
+			SharedSize: sharedSize,
 		},
 		RPS: Zone{
 			Name:       fmt.Sprintf("%v_rps", zoneName),
 			Limit:      rps,
 			Burst:      rps * burstMultiplier,
-			SharedSize: defSharedSize,
+			Delay:      delay,
+			SharedSize: sharedSize,
 		},
 		RPM: Zone{
 			Name:       fmt.Sprintf("%v_rpm", zoneName),
 			Limit:      rpm,
 			Burst:      rpm * burstMultiplier,
-			SharedSize: defSharedSize,
+			Delay:      delay,
+			SharedSize: sharedSize,
 		},
 		LimitRate:      lr,
 		LimitRateAfter: lra,

internal/ingress/annotations/ratelimit/main_test.go

@@ -154,6 +154,7 @@ func TestRateLimiting(t *testing.T) {
 	data[parser.GetAnnotationWithPrefix(limitRateAfterAnnotation)] = "100"
 	data[parser.GetAnnotationWithPrefix(limitRateAnnotation)] = "10"
 	data[parser.GetAnnotationWithPrefix(limitRateBurstMultiplierAnnotation)] = "3"
+	data[parser.GetAnnotationWithPrefix(limitRateDelayAnnotation)] = "5"
 
 	ing.SetAnnotations(data)
 
@@ -171,24 +172,92 @@ func TestRateLimiting(t *testing.T) {
 	if rateLimit.Connections.Burst != 5*3 {
 		t.Errorf("expected %d in burst limit by ip but %v was returned", 5*3, rateLimit.Connections)
 	}
+	if rateLimit.Connections.SharedSize != 5 {
+		t.Errorf("expected %d in sharedSize limit by ip but %v was returned", 5, rateLimit.Connections)
+	}
 	if rateLimit.RPS.Limit != 100 {
 		t.Errorf("expected 100 in limit by rps but %v was returned", rateLimit.RPS)
 	}
 	if rateLimit.RPS.Burst != 100*3 {
 		t.Errorf("expected %d in burst limit by rps but %v was returned", 100*3, rateLimit.RPS)
 	}
+	if rateLimit.RPS.Delay != 5 {
+		t.Errorf("expected %d in delay limit by rps but %v was returned", 5, rateLimit.RPS)
+	}
+	if rateLimit.RPS.SharedSize != 5 {
+		t.Errorf("expected %d in sharedSize limit by rps but %v was returned", 5, rateLimit.RPS)
+	}
 	if rateLimit.RPM.Limit != 10 {
 		t.Errorf("expected 10 in limit by rpm but %v was returned", rateLimit.RPM)
 	}
 	if rateLimit.RPM.Burst != 10*3 {
 		t.Errorf("expected %d in burst limit by rpm but %v was returned", 10*3, rateLimit.RPM)
 	}
+	if rateLimit.RPM.Delay != 5 {
+		t.Errorf("expected %d in delay limit by rpm but %v was returned", 5, rateLimit.RPM)
+	}
 	if rateLimit.LimitRateAfter != 100 {
 		t.Errorf("expected 100 in limit by limitrateafter but %v was returned", rateLimit.LimitRateAfter)
 	}
 	if rateLimit.LimitRate != 10 {
 		t.Errorf("expected 10 in limit by limitrate but %v was returned", rateLimit.LimitRate)
 	}
+
+	data = map[string]string{}
+	data[parser.GetAnnotationWithPrefix(limitRateConnectionsAnnotation)] = "5"
+	data[parser.GetAnnotationWithPrefix(limitRateRPSAnnotation)] = "100"
+	data[parser.GetAnnotationWithPrefix(limitRateRPMAnnotation)] = "10"
+	data[parser.GetAnnotationWithPrefix(limitRateAnnotation)] = "10"
+	data[parser.GetAnnotationWithPrefix(limitRateBurstMultiplierAnnotation)] = "3"
+	data[parser.GetAnnotationWithPrefix(limitRateNoBurstAnnotation)] = "true"
+	data[parser.GetAnnotationWithPrefix(limitRateSharedSizeAnnotation)] = "1"
+
+	ing.SetAnnotations(data)
+
+	i, err = NewParser(mockBackend{}).Parse(ing)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	rateLimit, ok = i.(*Config)
+	if !ok {
+		t.Errorf("expected a RateLimit type")
+	}
+	if rateLimit.Connections.Limit != 5 {
+		t.Errorf("expected 5 in limit by ip but %v was returned", rateLimit.Connections)
+	}
+	if rateLimit.Connections.Burst != 0 {
+		t.Errorf("expected %d in burst limit by ip but %v was returned", 0, rateLimit.Connections)
+	}
+	if rateLimit.Connections.SharedSize != 1 {
+		t.Errorf("expected %d in sharedSize limit by ip but %v was returned", 1, rateLimit.Connections)
+	}
+	if rateLimit.RPS.Limit != 100 {
+		t.Errorf("expected 100 in limit by rps but %v was returned", rateLimit.RPS)
+	}
+	if rateLimit.RPS.Burst != 0 {
+		t.Errorf("expected %d in burst limit by rps but %v was returned", 0, rateLimit.RPS)
+	}
+	if rateLimit.RPS.Delay != -1 {
+		t.Errorf("expected %d in delay limit by rps but %v was returned", -1, rateLimit.RPS)
+	}
+	if rateLimit.RPS.SharedSize != 1 {
+		t.Errorf("expected %d in sharedSize limit by rps but %v was returned", 1, rateLimit.RPS)
+	}
+	if rateLimit.RPM.Limit != 10 {
+		t.Errorf("expected 10 in limit by rpm but %v was returned", rateLimit.RPM)
+	}
+	if rateLimit.RPM.Burst != 0 {
+		t.Errorf("expected %d in burst limit by rpm but %v was returned", 0, rateLimit.RPM)
+	}
+	if rateLimit.RPM.Delay != -1 {
+		t.Errorf("expected %d in delay limit by rpm but %v was returned", -1, rateLimit.RPM)
+	}
+	if rateLimit.RPM.SharedSize != 1 {
+		t.Errorf("expected %d in sharedSize limit by rps but %v was returned", 1, rateLimit.RPM)
+	}
+	if rateLimit.LimitRate != 10 {
+		t.Errorf("expected 10 in limit by limitrate but %v was returned", rateLimit.LimitRate)
+	}
 }
 
 func TestAnnotationCIDR(t *testing.T) {

internal/ingress/controller/template/template.go

@@ -877,14 +877,34 @@ func buildRateLimit(input interface{}) []string {
 	}
 
 	if loc.RateLimit.RPS.Limit > 0 {
-		limit := fmt.Sprintf("limit_req zone=%v burst=%v nodelay;",
-			loc.RateLimit.RPS.Name, loc.RateLimit.RPS.Burst)
+		limit := fmt.Sprintf("limit_req zone=%v", loc.RateLimit.RPS.Name)
+
+		if loc.RateLimit.RPS.Burst > 0 {
+			limit = fmt.Sprintf("%v burst=%v", limit, loc.RateLimit.RPS.Burst)
+		}
+
+		if loc.RateLimit.RPS.Delay < 0 {
+			limit = fmt.Sprintf("%v nodelay;", limit)
+		} else {
+			limit = fmt.Sprintf("%v delay=%v;", limit, loc.RateLimit.RPS.Delay)
+		}
+
 		limits = append(limits, limit)
 	}
 
 	if loc.RateLimit.RPM.Limit > 0 {
-		limit := fmt.Sprintf("limit_req zone=%v burst=%v nodelay;",
-			loc.RateLimit.RPM.Name, loc.RateLimit.RPM.Burst)
+		limit := fmt.Sprintf("limit_req zone=%v", loc.RateLimit.RPM.Name)
+
+		if loc.RateLimit.RPM.Burst > 0 {
+			limit = fmt.Sprintf("%v burst=%v", limit, loc.RateLimit.RPM.Burst)
+		}
+
+		if loc.RateLimit.RPM.Delay < 0 {
+			limit = fmt.Sprintf("%v nodelay;", limit)
+		} else {
+			limit = fmt.Sprintf("%v delay=%v;", limit, loc.RateLimit.RPM.Delay)
+		}
+
 		limits = append(limits, limit)
 	}
 

internal/ingress/controller/template/template_test.go

@@ -959,10 +959,12 @@ func TestBuildRateLimit(t *testing.T) {
 	loc.RateLimit.RPS.Name = "rps"
 	loc.RateLimit.RPS.Limit = 1
 	loc.RateLimit.RPS.Burst = 1
+	loc.RateLimit.RPS.Delay = -1
 
 	loc.RateLimit.RPM.Name = "rpm"
 	loc.RateLimit.RPM.Limit = 2
 	loc.RateLimit.RPM.Burst = 2
+	loc.RateLimit.RPM.Delay = -1
 
 	loc.RateLimit.LimitRateAfter = 1
 	loc.RateLimit.LimitRate = 1
@@ -983,6 +985,56 @@ func TestBuildRateLimit(t *testing.T) {
 		}
 	}
 
+	loc = &ingress.Location{}
+
+	loc.RateLimit.RPS.Name = "rps"
+	loc.RateLimit.RPS.Limit = 1
+	loc.RateLimit.RPS.Burst = 5
+	loc.RateLimit.RPS.Delay = 2
+
+	loc.RateLimit.RPM.Name = "rpm"
+	loc.RateLimit.RPM.Limit = 2
+	loc.RateLimit.RPM.Burst = 5
+	loc.RateLimit.RPM.Delay = 3
+
+	validLimits = []string{
+		"limit_req zone=rps burst=5 delay=2;",
+		"limit_req zone=rpm burst=5 delay=3;",
+	}
+
+	limits = buildRateLimit(loc)
+
+	for i, limit := range limits {
+		if limit != validLimits[i] {
+			t.Errorf("Expected '%v' but returned '%v'", validLimits, limits)
+		}
+	}
+
+	loc = &ingress.Location{}
+
+	loc.RateLimit.RPS.Name = "rps_nodelay"
+	loc.RateLimit.RPS.Limit = 1
+	loc.RateLimit.RPS.Burst = 0
+	loc.RateLimit.RPS.Delay = -1
+
+	loc.RateLimit.RPM.Name = "rpm_nodelay"
+	loc.RateLimit.RPM.Limit = 2
+	loc.RateLimit.RPM.Burst = 0
+	loc.RateLimit.RPM.Delay = -1
+
+	validLimits = []string{
+		"limit_req zone=rps_nodelay nodelay;",
+		"limit_req zone=rpm_nodelay nodelay;",
+	}
+
+	limits = buildRateLimit(loc)
+
+	for i, limit := range limits {
+		if limit != validLimits[i] {
+			t.Errorf("Expected '%v' but returned '%v'", validLimits, limits)
+		}
+	}
+
 	// Invalid limit
 	limits = buildRateLimit(&ingress.Ingress{})
 	if !reflect.DeepEqual(expected, limits) {