add default bucket for local rate limit

Signed-off-by: huabing zhao <[email protected]>

api/v1alpha1/ratelimit_types.go

@@ -40,6 +40,8 @@ const (
 
 // GlobalRateLimit defines global rate limit configuration.
 type GlobalRateLimit struct {
+	// TODO zhaohuabing add default rate limit here.
+
 	// Rules are a list of RateLimit selectors and limits.
 	// Each rule and its associated limit is applied
 	// in a mutually exclusive way i.e. if multiple
@@ -56,18 +58,27 @@ type GlobalRateLimit struct {
 type LocalRateLimit struct {
 
 	// Envoy requires a default rate limit to be set for each route.
-	// The other option is to set the default rate limit to unit32 max and set the
+	// The other possible option is to set the default rate limit to unit32 max and set the
 	// fill interval to a short period, like 1 second. This will effectively make
 	// the default rate limit "unlimited".
 	// See https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/local_rate_limit_filter#using-rate-limit-descriptors-for-local-rate-limiting
 
 	// Default is the default rate limit if no rules match.
+	// If a request does not match any of the rules, it is counted towards the default limit.
+	//
+	// Note: Default is applied per route. Even if a policy targets a gateway,
+	// each route in that gateway still has a separate rate limit bucket.
+	// For example, if a gateway has 2 routes, and the default rate limit is 100r/s,
+	// then each route has its own 100r/s rate limit bucket.
 	Default RateLimitValue `json:"default"`
 
-	// Rules are a list of RateLimit selectors and limits.
+	// Rules are a list of RateLimit selectors and limits. Fine-grained rate limits
+	// can be applied to specific clients using attributes from the request.
+	//
 	// Orders matters here as the rules are processed sequentially.
 	// The first rule that matches the request is applied.
 	//
+	// +optional
 	// +kubebuilder:validation:MaxItems=16
 	Rules []RateLimitRule `json:"rules"`
 }
@@ -79,10 +90,8 @@ type RateLimitRule struct {
 	// specific clients using attributes from the traffic flow.
 	// All individual select conditions must hold True for this rule
 	// and its limit to be applied.
-	// If this field is empty, it is equivalent to True, and
-	// the limit is applied.
 	//
-	// +optional
+	// +kubebuilder:validation:MinItems=1
 	// +kubebuilder:validation:MaxItems=8
 	ClientSelectors []RateLimitSelectCondition `json:"clientSelectors,omitempty"`
 	// Limit holds the rate limit values.
@@ -100,6 +109,7 @@ type RateLimitRule struct {
 type RateLimitSelectCondition struct {
 	// Headers is a list of request headers to match. Multiple header values are ANDed together,
 	// meaning, a request MUST match all the specified headers.
+	// At least one of headers or sourceCIDR condition must be specified.
 	//
 	// +listType=map
 	// +listMapKey=name
@@ -108,6 +118,7 @@ type RateLimitSelectCondition struct {
 	Headers []HeaderMatch `json:"headers,omitempty"`
 
 	// SourceCIDR is the client IP Address range to match on.
+	// At least one of headers or sourceCIDR condition must be specified.
 	//
 	// +optional
 	SourceCIDR *SourceMatch `json:"sourceCIDR,omitempty"`

internal/gatewayapi/backendtrafficpolicy.go

@@ -342,15 +342,37 @@ func (t *Translator) buildLocalRateLimit(policy *egv1a1.BackendTrafficPolicy) *i
 		return nil
 	}
 
+	local := policy.Spec.RateLimit.Local
+	defaultLimitUnit := ratelimitUnitToDuration(local.Default.Unit)
+	for _, rule := range local.Rules {
+		ruleLimitUint := ratelimitUnitToDuration(rule.Limit.Unit)
+		if defaultLimitUnit == 0 || ruleLimitUint%defaultLimitUnit != 0 {
+			message := "Local rateLimit rule limit unit must be a multiple of the default limit unit."
+			// This is required by Envoy local rateLimit implementation.
+			// see https://github.com/envoyproxy/envoy/blob/6d9a6e995f472526de2b75233abca69aa00021ed/source/extensions/filters/common/local_ratelimit/local_ratelimit_impl.cc#L49
+			status.SetBackendTrafficPolicyCondition(policy,
+				gwv1a2.PolicyConditionAccepted,
+				metav1.ConditionFalse,
+				gwv1a2.PolicyReasonInvalid,
+				message,
+			)
+			return nil
+		}
+	}
+
 	rateLimit := &ir.RateLimit{
 		Local: &ir.LocalRateLimit{
-			Rules: make([]*ir.RateLimitRule, len(policy.Spec.RateLimit.Local.Rules)),
+			Default: ir.RateLimitValue{
+				Requests: local.Default.Requests,
+				Unit:     ir.RateLimitUnit(local.Default.Unit),
+			},
+			Rules: make([]*ir.RateLimitRule, len(local.Rules)),
 		},
 	}
 
 	irRules := rateLimit.Local.Rules
 	var err error
-	for i, rule := range policy.Spec.RateLimit.Local.Rules {
+	for i, rule := range local.Rules {
 		irRules[i], err = buildRateLimitRule(rule)
 		if err != nil {
 			status.SetBackendTrafficPolicyCondition(policy,
@@ -411,15 +433,16 @@ func (t *Translator) buildGlobalRateLimit(policy *egv1a1.BackendTrafficPolicy) *
 		return nil
 	}
 
+	global := policy.Spec.RateLimit.Global
 	rateLimit := &ir.RateLimit{
 		Global: &ir.GlobalRateLimit{
-			Rules: make([]*ir.RateLimitRule, len(policy.Spec.RateLimit.Global.Rules)),
+			Rules: make([]*ir.RateLimitRule, len(global.Rules)),
 		},
 	}
 
 	irRules := rateLimit.Global.Rules
 	var err error
-	for i, rule := range policy.Spec.RateLimit.Global.Rules {
+	for i, rule := range global.Rules {
 		irRules[i], err = buildRateLimitRule(rule)
 		if err != nil {
 			status.SetBackendTrafficPolicyCondition(policy,
@@ -443,7 +466,16 @@ func buildRateLimitRule(rule egv1a1.RateLimitRule) (*ir.RateLimitRule, error) {
 		},
 		HeaderMatches: make([]*ir.StringMatch, 0),
 	}
+	if len(rule.ClientSelectors) == 0 {
+		return nil, errors.New(
+			"unable to translate rateLimit. At least one clientSelector must be specified")
+	}
 	for _, match := range rule.ClientSelectors {
+		if len(match.Headers) == 0 && match.SourceCIDR == nil {
+			return nil, errors.New(
+				"unable to translate rateLimit. At least one of the" +
+					" header or sourceCIDR must be specified")
+		}
 		for _, header := range match.Headers {
 			switch {
 			case header.Type == nil && header.Value != nil:
@@ -468,7 +500,8 @@ func buildRateLimitRule(rule egv1a1.RateLimitRule) (*ir.RateLimitRule, error) {
 				irRule.HeaderMatches = append(irRule.HeaderMatches, m)
 			default:
 				return nil, errors.New(
-					"unable to translate rateLimit. Either the header.Type is not valid or the header is missing a value")
+					"unable to translate rateLimit. Either the header." +
+						"Type is not valid or the header is missing a value")
 			}
 		}
 
@@ -559,3 +592,19 @@ func (t *Translator) buildProxyProtocol(policy *egv1a1.BackendTrafficPolicy) *ir
 
 	return pp
 }
+
+func ratelimitUnitToDuration(unit egv1a1.RateLimitUnit) int64 {
+	var seconds int64
+
+	switch unit {
+	case egv1a1.RateLimitUnitSecond:
+		seconds = 1
+	case egv1a1.RateLimitUnitMinute:
+		seconds = 60
+	case egv1a1.RateLimitUnitHour:
+		seconds = 60 * 60
+	case egv1a1.RateLimitUnitDay:
+		seconds = 60 * 60 * 24
+	}
+	return seconds
+}

internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.in.yaml

@@ -0,0 +1,74 @@
+gateways:
+- apiVersion: gateway.networking.k8s.io/v1
+  kind: Gateway
+  metadata:
+    namespace: envoy-gateway
+    name: gateway-1
+  spec:
+    gatewayClassName: envoy-gateway-class
+    listeners:
+    - name: http
+      protocol: HTTP
+      port: 80
+      allowedRoutes:
+        namespaces:
+          from: All
+httpRoutes:
+- apiVersion: gateway.networking.k8s.io/v1
+  kind: HTTPRoute
+  metadata:
+    namespace: default
+    name: httproute-1
+  spec:
+    hostnames:
+    - gateway.envoyproxy.io
+    parentRefs:
+    - namespace: envoy-gateway
+      name: gateway-1
+      sectionName: http
+    rules:
+    - matches:
+      - path:
+          value: "/"
+      backendRefs:
+      - name: service-1
+        port: 8080
+backendTrafficPolicies:
+- apiVersion: gateway.envoyproxy.io/v1alpha1
+  kind: BackendTrafficPolicy
+  metadata:
+    namespace: envoy-gateway
+    name: policy-for-gateway
+  spec:
+    targetRef:
+      group: gateway.networking.k8s.io
+      kind: Gateway
+      name: gateway-1
+      namespace: envoy-gateway
+    rateLimit:
+      type: Local
+      local:
+        default:
+          requests: 1000
+          unit: Hour
+        rules:
+        - clientSelectors:
+          - headers:
+            - name: x-user-id
+              value: one
+            - name: x-org-id
+              value: foo
+          limit:
+            requests: 10
+            unit: Hour
+        - clientSelectors:
+          - headers:
+            - name: x-user-id
+              value: two
+            - name: x-org-id
+              value: bar
+            sourceCIDR:
+              value: 192.168.0.0/16
+          limit:
+            requests: 10
+            unit: Minute