From a922a61e408e1cc0886a32b6eab10c5023de71d2 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Thu, 30 Nov 2023 21:10:16 +0800 Subject: [PATCH 01/15] local rate limit Signed-off-by: huabing zhao --- api/v1alpha1/ratelimit_types.go | 39 +- api/v1alpha1/zz_generated.deepcopy.go | 27 ++ ....envoyproxy.io_backendtrafficpolicies.yaml | 123 +++++++ internal/gatewayapi/backendtrafficpolicy.go | 221 ++++++++---- ...trafficpolicy-with-local-ratelimit.in.yaml | 122 +++++++ ...rafficpolicy-with-local-ratelimit.out.yaml | 340 ++++++++++++++++++ internal/ir/xds.go | 18 +- internal/ir/zz_generated.deepcopy.go | 37 +- internal/xds/translator/httpfilters.go | 2 + internal/xds/translator/local_ratelimit.go | 298 +++++++++++++++ .../testdata/in/xds-ir/local-ratelimit.yaml | 82 +++++ .../out/xds-ir/local-ratelimit.clusters.yaml | 42 +++ .../out/xds-ir/local-ratelimit.endpoints.yaml | 36 ++ .../out/xds-ir/local-ratelimit.listeners.yaml | 37 ++ .../out/xds-ir/local-ratelimit.routes.yaml | 149 ++++++++ internal/xds/translator/translator_test.go | 3 + site/content/en/latest/api/extension_types.md | 16 + 17 files changed, 1513 insertions(+), 79 deletions(-) create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml create mode 100755 internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml create mode 100644 internal/xds/translator/local_ratelimit.go create mode 100644 internal/xds/translator/testdata/in/xds-ir/local-ratelimit.yaml create mode 100755 internal/xds/translator/testdata/out/xds-ir/local-ratelimit.clusters.yaml create mode 100755 internal/xds/translator/testdata/out/xds-ir/local-ratelimit.endpoints.yaml create mode 100755 internal/xds/translator/testdata/out/xds-ir/local-ratelimit.listeners.yaml create mode 100755 internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml diff --git a/api/v1alpha1/ratelimit_types.go b/api/v1alpha1/ratelimit_types.go index e013579efd..bf8488f46b 100644 --- a/api/v1alpha1/ratelimit_types.go +++ b/api/v1alpha1/ratelimit_types.go @@ -17,6 +17,11 @@ type RateLimitSpec struct { // // +optional Global *GlobalRateLimit `json:"global,omitempty"` + + // Local defines local rate limit configuration. + // + // +optional + Local *LocalRateLimit `json:"local,omitempty"` } // RateLimitType specifies the types of RateLimiting. @@ -24,8 +29,13 @@ type RateLimitSpec struct { type RateLimitType string const ( - // GlobalRateLimitType allows the rate limits to be applied across all Envoy proxy instances. + // GlobalRateLimitType allows the rate limits to be applied across all Envoy + // proxy instances. GlobalRateLimitType RateLimitType = "Global" + + // LocalRateLimitType allows the rate limits to be applied on a per Envoy + // proxy instance basis. + LocalRateLimitType RateLimitType = "Local" ) // GlobalRateLimit defines global rate limit configuration. @@ -42,6 +52,16 @@ type GlobalRateLimit struct { Rules []RateLimitRule `json:"rules"` } +// LocalRateLimit defines local rate limit configuration. +type LocalRateLimit struct { + // Rules are a list of RateLimit selectors and limits. + // Orders matters here as the rules are processed sequentially. + // The first rule that matches the request is applied. + // + // +kubebuilder:validation:MaxItems=16 + Rules []RateLimitRule `json:"rules"` +} + // RateLimitRule defines the semantics for matching attributes // from the incoming requests, and setting limits for them. type RateLimitRule struct { @@ -91,6 +111,7 @@ const ( SourceMatchExact SourceMatchType = "Exact" // SourceMatchDistinct Each IP Address within the specified Source IP CIDR is treated as a distinct client selector // and uses a separate rate limit bucket/counter. + // Note: This is only supported for Global Rate Limits. SourceMatchDistinct SourceMatchType = "Distinct" ) @@ -148,6 +169,7 @@ const ( // HeaderMatchDistinct matches any and all possible unique values encountered in the // specified HTTP Header. Note that each unique value will receive its own rate limit // bucket. + // Note: This is only supported for Global Rate Limits. HeaderMatchDistinct HeaderMatchType = "Distinct" ) @@ -162,3 +184,18 @@ type RateLimitValue struct { // // +kubebuilder:validation:Enum=Second;Minute;Hour;Day type RateLimitUnit string + +// RateLimitUnit constants. +const ( + // RateLimitUnitSecond specifies the rate limit interval to be 1 second. + RateLimitUnitSecond RateLimitUnit = "Second" + + // RateLimitUnitMinute specifies the rate limit interval to be 1 minute. + RateLimitUnitMinute RateLimitUnit = "Minute" + + // RateLimitUnitHour specifies the rate limit interval to be 1 hour. + RateLimitUnitHour RateLimitUnit = "Hour" + + // RateLimitUnitDay specifies the rate limit interval to be 1 day. + RateLimitUnitDay RateLimitUnit = "Day" +) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2654e070a2..5ed670e449 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1631,6 +1631,28 @@ func (in *LoadBalancer) DeepCopy() *LoadBalancer { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LocalRateLimit) DeepCopyInto(out *LocalRateLimit) { + *out = *in + if in.Rules != nil { + in, out := &in.Rules, &out.Rules + *out = make([]RateLimitRule, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LocalRateLimit. +func (in *LocalRateLimit) DeepCopy() *LocalRateLimit { + if in == nil { + return nil + } + out := new(LocalRateLimit) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OIDC) DeepCopyInto(out *OIDC) { *out = *in @@ -2115,6 +2137,11 @@ func (in *RateLimitSpec) DeepCopyInto(out *RateLimitSpec) { *out = new(GlobalRateLimit) (*in).DeepCopyInto(*out) } + if in.Local != nil { + in, out := &in.Local, &out.Local + *out = new(LocalRateLimit) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RateLimitSpec. diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml index 6606d2dd85..d585a9aa69 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml @@ -245,6 +245,129 @@ spec: required: - rules type: object + local: + description: Local defines local rate limit configuration. + properties: + rules: + description: Rules are a list of RateLimit selectors and limits. + Orders matters here as the rules are processed sequentially. + The first rule that matches the request is applied. + items: + description: RateLimitRule defines the semantics for matching + attributes from the incoming requests, and setting limits + for them. + properties: + clientSelectors: + description: ClientSelectors holds the list of select + conditions to select 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. + items: + description: RateLimitSelectCondition specifies the + attributes within the traffic flow that can be used + to select a subset of clients to be ratelimited. + All the individual conditions must hold True for + the overall condition to hold True. + properties: + headers: + description: Headers is a list of request headers + to match. Multiple header values are ANDed together, + meaning, a request MUST match all the specified + headers. + items: + description: HeaderMatch defines the match attributes + within the HTTP Headers of the request. + properties: + name: + description: Name of the HTTP header. + maxLength: 256 + minLength: 1 + type: string + type: + default: Exact + description: Type specifies how to match + against the value of the header. + enum: + - Exact + - RegularExpression + - Distinct + type: string + value: + description: Value within the HTTP header. + Due to the case-insensitivity of header + names, "foo" and "Foo" are considered + equivalent. Do not set this field when + Type="Distinct", implying matching on + any/all unique values within the header. + maxLength: 1024 + type: string + required: + - name + type: object + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + sourceCIDR: + description: SourceCIDR is the client IP Address + range to match on. + properties: + type: + default: Exact + type: string + value: + description: Value is the IP CIDR that represents + the range of Source IP Addresses of the + client. These could also be the intermediate + addresses through which the request has + flown through and is part of the `X-Forwarded-For` + header. For example, `192.168.0.1/32`, `192.168.0.0/24`, + `001:db8::/64`. + maxLength: 256 + minLength: 1 + type: string + required: + - value + type: object + type: object + maxItems: 8 + type: array + limit: + description: Limit holds the rate limit values. This + limit is applied for traffic flows when the selectors + compute to True, causing the request to be counted + towards the limit. The limit is enforced and the request + is ratelimited, i.e. a response with 429 HTTP status + code is sent back to the client when the selected + requests have reached the limit. + properties: + requests: + type: integer + unit: + description: RateLimitUnit specifies the intervals + for setting rate limits. Valid RateLimitUnit values + are "Second", "Minute", "Hour", and "Day". + enum: + - Second + - Minute + - Hour + - Day + type: string + required: + - requests + - unit + type: object + required: + - limit + type: object + maxItems: 16 + type: array + required: + - rules + type: object type: description: Type decides the scope for the RateLimits. Valid RateLimitType values are "Global". diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index fc21e970ba..029397c64e 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -6,6 +6,7 @@ package gatewayapi import ( + "errors" "fmt" "net" "sort" @@ -313,8 +314,83 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back } func (t *Translator) buildRateLimit(policy *egv1a1.BackendTrafficPolicy) *ir.RateLimit { + switch policy.Spec.RateLimit.Type { + case egv1a1.GlobalRateLimitType: + return t.buildGlobalRateLimit(policy) + case egv1a1.LocalRateLimitType: + return t.buildLocalRateLimit(policy) + } + + status.SetBackendTrafficPolicyCondition(policy, + gwv1a2.PolicyConditionAccepted, + metav1.ConditionFalse, + gwv1a2.PolicyReasonInvalid, + "Invalid rateLimit type", + ) + return nil +} + +func (t *Translator) buildLocalRateLimit(policy *egv1a1.BackendTrafficPolicy) *ir.RateLimit { + if policy.Spec.RateLimit.Local == nil { + message := "Local configuration empty for rateLimit." + 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)), + }, + } + + irRules := rateLimit.Local.Rules + var err error + for i, rule := range policy.Spec.RateLimit.Local.Rules { + irRules[i], err = buildRateLimitRule(rule) + if err != nil { + status.SetBackendTrafficPolicyCondition(policy, + gwv1a2.PolicyConditionAccepted, + metav1.ConditionFalse, + gwv1a2.PolicyReasonInvalid, + status.Error2ConditionMsg(err), + ) + return nil + } + if irRules[i].CIDRMatch != nil && irRules[i].CIDRMatch.Distinct { + message := "Local rateLimit does not support distinct CIDRMatch." + status.SetBackendTrafficPolicyCondition(policy, + gwv1a2.PolicyConditionAccepted, + metav1.ConditionFalse, + gwv1a2.PolicyReasonInvalid, + message, + ) + return nil + } + for _, match := range irRules[i].HeaderMatches { + if match.Distinct { + message := "Local rateLimit does not support distinct HeaderMatch." + status.SetBackendTrafficPolicyCondition(policy, + gwv1a2.PolicyConditionAccepted, + metav1.ConditionFalse, + gwv1a2.PolicyReasonInvalid, + message, + ) + return nil + } + } + } + + return rateLimit +} + +func (t *Translator) buildGlobalRateLimit(policy *egv1a1.BackendTrafficPolicy) *ir.RateLimit { if policy.Spec.RateLimit.Global == nil { - message := "Global configuration empty for rateLimit" + message := "Global configuration empty for rateLimit." status.SetBackendTrafficPolicyCondition(policy, gwv1a2.PolicyConditionAccepted, metav1.ConditionFalse, @@ -323,8 +399,9 @@ func (t *Translator) buildRateLimit(policy *egv1a1.BackendTrafficPolicy) *ir.Rat ) return nil } + if !t.GlobalRateLimitEnabled { - message := "Enable Ratelimit in the EnvoyGateway config to configure global rateLimit" + message := "Enable Ratelimit in the EnvoyGateway config to configure global rateLimit." status.SetBackendTrafficPolicyCondition(policy, gwv1a2.PolicyConditionAccepted, metav1.ConditionFalse, @@ -333,92 +410,92 @@ func (t *Translator) buildRateLimit(policy *egv1a1.BackendTrafficPolicy) *ir.Rat ) return nil } + rateLimit := &ir.RateLimit{ Global: &ir.GlobalRateLimit{ Rules: make([]*ir.RateLimitRule, len(policy.Spec.RateLimit.Global.Rules)), }, } - rules := rateLimit.Global.Rules + irRules := rateLimit.Global.Rules + var err error for i, rule := range policy.Spec.RateLimit.Global.Rules { - rules[i] = &ir.RateLimitRule{ - Limit: &ir.RateLimitValue{ - Requests: rule.Limit.Requests, - Unit: ir.RateLimitUnit(rule.Limit.Unit), - }, - HeaderMatches: make([]*ir.StringMatch, 0), + irRules[i], err = buildRateLimitRule(rule) + if err != nil { + status.SetBackendTrafficPolicyCondition(policy, + gwv1a2.PolicyConditionAccepted, + metav1.ConditionFalse, + gwv1a2.PolicyReasonInvalid, + status.Error2ConditionMsg(err), + ) + return nil } - for _, match := range rule.ClientSelectors { - for _, header := range match.Headers { - switch { - case header.Type == nil && header.Value != nil: - fallthrough - case *header.Type == egv1a1.HeaderMatchExact && header.Value != nil: - m := &ir.StringMatch{ - Name: header.Name, - Exact: header.Value, - } - rules[i].HeaderMatches = append(rules[i].HeaderMatches, m) - case *header.Type == egv1a1.HeaderMatchRegularExpression && header.Value != nil: - m := &ir.StringMatch{ - Name: header.Name, - SafeRegex: header.Value, - } - rules[i].HeaderMatches = append(rules[i].HeaderMatches, m) - case *header.Type == egv1a1.HeaderMatchDistinct && header.Value == nil: - m := &ir.StringMatch{ - Name: header.Name, - Distinct: true, - } - rules[i].HeaderMatches = append(rules[i].HeaderMatches, m) - default: - // set negative status condition. - message := "Unable to translate rateLimit. Either the header.Type is not valid or the header is missing a value" - status.SetBackendTrafficPolicyCondition(policy, - gwv1a2.PolicyConditionAccepted, - metav1.ConditionFalse, - gwv1a2.PolicyReasonInvalid, - message, - ) - - return nil + } + + return rateLimit +} + +func buildRateLimitRule(rule egv1a1.RateLimitRule) (*ir.RateLimitRule, error) { + irRule := &ir.RateLimitRule{ + Limit: ir.RateLimitValue{ + Requests: rule.Limit.Requests, + Unit: ir.RateLimitUnit(rule.Limit.Unit), + }, + HeaderMatches: make([]*ir.StringMatch, 0), + } + for _, match := range rule.ClientSelectors { + for _, header := range match.Headers { + switch { + case header.Type == nil && header.Value != nil: + fallthrough + case *header.Type == egv1a1.HeaderMatchExact && header.Value != nil: + m := &ir.StringMatch{ + Name: header.Name, + Exact: header.Value, + } + irRule.HeaderMatches = append(irRule.HeaderMatches, m) + case *header.Type == egv1a1.HeaderMatchRegularExpression && header.Value != nil: + m := &ir.StringMatch{ + Name: header.Name, + SafeRegex: header.Value, + } + irRule.HeaderMatches = append(irRule.HeaderMatches, m) + case *header.Type == egv1a1.HeaderMatchDistinct && header.Value == nil: + m := &ir.StringMatch{ + Name: header.Name, + Distinct: true, } + 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") } + } - if match.SourceCIDR != nil { - // distinct means that each IP Address within the specified Source IP CIDR is treated as a - // distinct client selector and uses a separate rate limit bucket/counter. - distinct := false - sourceCIDR := match.SourceCIDR.Value - if match.SourceCIDR.Type != nil && *match.SourceCIDR.Type == egv1a1.SourceMatchDistinct { - distinct = true - } + if match.SourceCIDR != nil { + // distinct means that each IP Address within the specified Source IP CIDR is treated as a + // distinct client selector and uses a separate rate limit bucket/counter. + distinct := false + sourceCIDR := match.SourceCIDR.Value + if match.SourceCIDR.Type != nil && *match.SourceCIDR.Type == egv1a1.SourceMatchDistinct { + distinct = true + } - ip, ipn, err := net.ParseCIDR(sourceCIDR) - if err != nil { - message := "Unable to translate rateLimit" - status.SetBackendTrafficPolicyCondition(policy, - gwv1a2.PolicyConditionAccepted, - metav1.ConditionFalse, - gwv1a2.PolicyReasonInvalid, - message, - ) - - return nil - } + ip, ipn, err := net.ParseCIDR(sourceCIDR) + if err != nil { + return nil, errors.New("unable to translate rateLimit") + } - mask, _ := ipn.Mask.Size() - rules[i].CIDRMatch = &ir.CIDRMatch{ - CIDR: ipn.String(), - IPv6: ip.To4() == nil, - MaskLen: mask, - Distinct: distinct, - } + mask, _ := ipn.Mask.Size() + irRule.CIDRMatch = &ir.CIDRMatch{ + CIDR: ipn.String(), + IPv6: ip.To4() == nil, + MaskLen: mask, + Distinct: distinct, } } } - - return rateLimit + return irRule, nil } func (t *Translator) buildLoadBalancer(policy *egv1a1.BackendTrafficPolicy) *ir.LoadBalancer { diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml new file mode 100644 index 0000000000..1c4903692b --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml @@ -0,0 +1,122 @@ +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 +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-2 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +grpcRoutes: +- apiVersion: gateway.networking.k8s.io/v1alpha2 + kind: GRPCRoute + metadata: + namespace: default + name: grpcroute-1 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 +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-2 + 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: + 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 +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + namespace: default + name: policy-for-route + spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-1 + namespace: default + rateLimit: + type: Local + local: + rules: + - clientSelectors: + - sourceCIDR: + type: "Distinct" + value: 192.168.0.0/16 + limit: + requests: 20 + unit: Hour diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml new file mode 100755 index 0000000000..ff31805d4c --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml @@ -0,0 +1,340 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + creationTimestamp: null + name: policy-for-route + namespace: default + spec: + rateLimit: + local: + rules: + - clientSelectors: + - sourceCIDR: + type: Distinct + value: 192.168.0.0/16 + limit: + requests: 20 + unit: Hour + type: Local + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-1 + namespace: default + status: + conditions: + - lastTransitionTime: null + message: Local rateLimit does not support distinct CIDRMatch. + reason: Invalid + status: "False" + type: Accepted +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway + namespace: envoy-gateway + spec: + rateLimit: + local: + 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 + type: Local + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + status: + conditions: + - lastTransitionTime: null + message: BackendTrafficPolicy has been accepted. + reason: Accepted + status: "True" + type: Accepted +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-2 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +grpcRoutes: +- apiVersion: gateway.networking.k8s.io/v1alpha2 + kind: GRPCRoute + metadata: + creationTimestamp: null + name: grpcroute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-2 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-2 + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-1 + envoy-gateway/gateway-2: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-2 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-2 +xdsIR: + envoy-gateway/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: true + name: envoy-gateway/gateway-1/http + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: grpcroute/default/grpcroute-1/rule/0 + settings: + - endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: GRPC + weight: 1 + hostname: '*' + name: grpcroute/default/grpcroute-1/rule/0/match/-1/* + rateLimit: + local: + rules: + - headerMatches: + - distinct: false + exact: one + name: x-user-id + - distinct: false + exact: foo + name: x-org-id + limit: + requests: 10 + unit: Hour + - cidrMatch: + cidr: 192.168.0.0/16 + distinct: false + ipv6: false + maskLen: 16 + headerMatches: + - distinct: false + exact: two + name: x-user-id + - distinct: false + exact: bar + name: x-org-id + limit: + requests: 10 + unit: Minute + envoy-gateway/gateway-2: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-2/http + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-1/rule/0 + settings: + - endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: gateway.envoyproxy.io + name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io + pathMatch: + distinct: false + name: "" + prefix: / diff --git a/internal/ir/xds.go b/internal/ir/xds.go index de252ca614..00a9a262ac 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -890,6 +890,9 @@ func (h UDPListener) Validate() error { type RateLimit struct { // Global rate limit settings. Global *GlobalRateLimit `json:"global,omitempty" yaml:"global,omitempty"` + + // Local rate limit settings. + Local *LocalRateLimit `json:"local,omitempty" yaml:"local,omitempty"` } // GlobalRateLimit holds the global rate limiting configuration. @@ -899,6 +902,13 @@ type GlobalRateLimit struct { Rules []*RateLimitRule `json:"rules,omitempty" yaml:"rules,omitempty"` } +// LocalRateLimit holds the local rate limiting configuration. +// +k8s:deepcopy-gen=true +type LocalRateLimit struct { + // Rules for rate limiting. + Rules []*RateLimitRule `json:"rules,omitempty" yaml:"rules,omitempty"` +} + // RateLimitRule holds the match and limit configuration for ratelimiting. // +k8s:deepcopy-gen=true type RateLimitRule struct { @@ -907,7 +917,7 @@ type RateLimitRule struct { // CIDRMatch define the match conditions on the source IP's CIDR for this route. CIDRMatch *CIDRMatch `json:"cidrMatch,omitempty" yaml:"cidrMatch,omitempty"` // Limit holds the rate limit values. - Limit *RateLimitValue `json:"limit,omitempty" yaml:"limit,omitempty"` + Limit RateLimitValue `json:"limit,omitempty" yaml:"limit,omitempty"` } type CIDRMatch struct { @@ -919,10 +929,16 @@ type CIDRMatch struct { Distinct bool `json:"distinct" yaml:"distinct"` } +// TODO zhaohuabing: remove this function func (r *RateLimitRule) IsMatchSet() bool { return len(r.HeaderMatches) != 0 || r.CIDRMatch != nil } +// MatchAll returns true if the rule matches all requests on a route. +func (r *RateLimitRule) MatchAll() bool { + return len(r.HeaderMatches) == 0 || r.CIDRMatch == nil +} + type RateLimitUnit egv1a1.RateLimitUnit // RateLimitValue holds the diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 3eb0f6ac92..39d1a47fed 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -712,6 +712,32 @@ func (in *LoadBalancer) DeepCopy() *LoadBalancer { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LocalRateLimit) DeepCopyInto(out *LocalRateLimit) { + *out = *in + if in.Rules != nil { + in, out := &in.Rules, &out.Rules + *out = make([]*RateLimitRule, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(RateLimitRule) + (*in).DeepCopyInto(*out) + } + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LocalRateLimit. +func (in *LocalRateLimit) DeepCopy() *LocalRateLimit { + if in == nil { + return nil + } + out := new(LocalRateLimit) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Metrics) DeepCopyInto(out *Metrics) { *out = *in @@ -882,6 +908,11 @@ func (in *RateLimit) DeepCopyInto(out *RateLimit) { *out = new(GlobalRateLimit) (*in).DeepCopyInto(*out) } + if in.Local != nil { + in, out := &in.Local, &out.Local + *out = new(LocalRateLimit) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RateLimit. @@ -913,11 +944,7 @@ func (in *RateLimitRule) DeepCopyInto(out *RateLimitRule) { *out = new(CIDRMatch) **out = **in } - if in.Limit != nil { - in, out := &in.Limit, &out.Limit - *out = new(RateLimitValue) - **out = **in - } + out.Limit = in.Limit } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RateLimitRule. diff --git a/internal/xds/translator/httpfilters.go b/internal/xds/translator/httpfilters.go index f16bb5c09a..8bea1b0e99 100644 --- a/internal/xds/translator/httpfilters.go +++ b/internal/xds/translator/httpfilters.go @@ -99,6 +99,8 @@ func newOrderedHTTPFilter(filter *hcmv3.HttpFilter) *OrderedHTTPFilter { order = 4 case filter.Name == wellknown.HTTPRateLimit: order = 5 + case filter.Name == localRateLimitFilter: + order = 6 case filter.Name == wellknown.Router: order = 100 } diff --git a/internal/xds/translator/local_ratelimit.go b/internal/xds/translator/local_ratelimit.go new file mode 100644 index 0000000000..27f588fd18 --- /dev/null +++ b/internal/xds/translator/local_ratelimit.go @@ -0,0 +1,298 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package translator + +import ( + "errors" + "fmt" + + routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + rlv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/common/ratelimit/v3" + localrlv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/local_ratelimit/v3" + hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" + typev3 "github.com/envoyproxy/go-control-plane/envoy/type/v3" + "github.com/golang/protobuf/ptypes/duration" + "google.golang.org/protobuf/types/known/anypb" + "google.golang.org/protobuf/types/known/wrapperspb" + + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/xds/types" +) + +const ( + localRateLimitFilter = "envoy.filters.http.local_ratelimit" + localRateLimitFilterStatPrefix = "http_local_rate_limiter" + descriptorMaskedRemoteAddress = "masked_remote_address" +) + +func init() { + registerHTTPFilter(&localRateLimit{}) +} + +type localRateLimit struct { +} + +var _ httpFilter = &localRateLimit{} + +// patchHCM builds and appends the local rate limit filter to the HTTP Connection Manager +// if applicable, and it does not already exist. +func (*localRateLimit) patchHCM(mgr *hcmv3.HttpConnectionManager, irListener *ir.HTTPListener) error { + if mgr == nil { + return errors.New("hcm is nil") + } + if irListener == nil { + return errors.New("ir listener is nil") + } + if !listenerContainsLocalRateLimit(irListener) { + return nil + } + + // Return early if filter already exists. + for _, httpFilter := range mgr.HttpFilters { + if httpFilter.Name == localRateLimitFilter { + return nil + } + } + + localRl := &localrlv3.LocalRateLimit{ + StatPrefix: localRateLimitFilterStatPrefix, + } + + localRlAny, err := anypb.New(localRl) + if err != nil { + return err + } + + // The local rate limit filter at the HTTP connection manager level is an + // empty filter. The real configuration is done at the route level. + // See https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/local_rate_limit_filter + filter := &hcmv3.HttpFilter{ + Name: localRateLimitFilter, + ConfigType: &hcmv3.HttpFilter_TypedConfig{ + TypedConfig: localRlAny, + }, + } + + mgr.HttpFilters = append(mgr.HttpFilters, filter) + return nil +} + +func listenerContainsLocalRateLimit(irListener *ir.HTTPListener) bool { + if irListener == nil { + return false + } + + for _, route := range irListener.Routes { + if routeContainsLocalRateLimit(route) { + return true + } + } + + return false +} + +func routeContainsLocalRateLimit(irRoute *ir.HTTPRoute) bool { + if irRoute == nil || irRoute.RateLimit == nil || irRoute.RateLimit.Local == nil { + return false + } + + return true +} + +func (*localRateLimit) patchResources(*types.ResourceVersionTable, + []*ir.HTTPRoute) error { + return nil +} + +func (*localRateLimit) patchRouteConfig(*routev3.RouteConfiguration, *ir.HTTPListener) error { + return nil +} + +func (*localRateLimit) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error { + routeAction := route.GetRoute() + + // Return early if no rate limit config exists. + if irRoute.RateLimit == nil || irRoute.RateLimit.Local == nil || routeAction == nil { + return nil + } + + if routeAction.RateLimits != nil { + // This should not happen since this is the only place where the rate limit + // config is added in a route. + return fmt.Errorf( + "route already contains rate limit config: %s", + route.Name) + } + + rateLimits, descriptors, err := buildRouteLocalRateLimits(irRoute.Name, irRoute.RateLimit.Local) + if err != nil { + return err + } + routeAction.RateLimits = rateLimits + + filterCfg := route.GetTypedPerFilterConfig() + if _, ok := filterCfg[localRateLimitFilter]; ok { + // This should not happen since this is the only place where the filter + // config is added in a route. + return fmt.Errorf( + "route already contains local rate limit filter config: %s", + route.Name) + } + + localRl := &localrlv3.LocalRateLimit{ + StatPrefix: localRateLimitFilterStatPrefix, + Descriptors: descriptors, + } + + localRlAny, err := anypb.New(localRl) + if err != nil { + return err + } + + if filterCfg == nil { + route.TypedPerFilterConfig = make(map[string]*anypb.Any) + } + + route.TypedPerFilterConfig[localRateLimitFilter] = localRlAny + return nil +} + +func buildRouteLocalRateLimits(descriptorPrefix string, local *ir.LocalRateLimit) ( + []*routev3.RateLimit, []*rlv3.LocalRateLimitDescriptor, error) { + var rateLimits []*routev3.RateLimit + var descriptors []*rlv3.LocalRateLimitDescriptor + + // Rules are ORed + for rIdx, rule := range local.Rules { + var rlActions []*routev3.RateLimit_Action + var descriptorEntries []*rlv3.RateLimitDescriptor_Entry + + // HeaderMatches + for mIdx, match := range rule.HeaderMatches { + if match.Distinct { + // This is a sanity check. This should never happen because Gateway + // API translator should have already validated this. + if rule.CIDRMatch.Distinct { + return nil, nil, errors.New("local rateLimit does not support distinct HeaderMatch") + } + } + + // Setup HeaderValueMatch actions + descriptorKey := getRateLimitDescriptorKey(descriptorPrefix, rIdx, mIdx) + descriptorVal := getRateLimitDescriptorValue(descriptorPrefix, rIdx, mIdx) + headerMatcher := &routev3.HeaderMatcher{ + Name: match.Name, + HeaderMatchSpecifier: &routev3.HeaderMatcher_StringMatch{ + StringMatch: buildXdsStringMatcher(match), + }, + } + action := &routev3.RateLimit_Action{ + ActionSpecifier: &routev3.RateLimit_Action_HeaderValueMatch_{ + HeaderValueMatch: &routev3.RateLimit_Action_HeaderValueMatch{ + DescriptorKey: descriptorKey, + DescriptorValue: descriptorVal, + ExpectMatch: &wrapperspb.BoolValue{ + Value: true, + }, + Headers: []*routev3.HeaderMatcher{headerMatcher}, + }, + }, + } + entry := &rlv3.RateLimitDescriptor_Entry{ + Key: descriptorKey, + Value: descriptorVal, + } + rlActions = append(rlActions, action) + descriptorEntries = append(descriptorEntries, entry) + } + + // Source IP CIDRMatch + if rule.CIDRMatch != nil { + // This is a sanity check. This should never happen because Gateway + // API translator should have already validated this. + if rule.CIDRMatch.Distinct { + return nil, nil, errors.New("local rateLimit does not support distinct CIDRMatch") + } + + // Setup MaskedRemoteAddress action + mra := &routev3.RateLimit_Action_MaskedRemoteAddress{} + maskLen := &wrapperspb.UInt32Value{Value: uint32(rule.CIDRMatch.MaskLen)} + if rule.CIDRMatch.IPv6 { + mra.V6PrefixMaskLen = maskLen + } else { + mra.V4PrefixMaskLen = maskLen + } + action := &routev3.RateLimit_Action{ + ActionSpecifier: &routev3.RateLimit_Action_MaskedRemoteAddress_{ + MaskedRemoteAddress: mra, + }, + } + entry := &rlv3.RateLimitDescriptor_Entry{ + Key: descriptorMaskedRemoteAddress, + Value: rule.CIDRMatch.CIDR, + } + descriptorEntries = append(descriptorEntries, entry) + rlActions = append(rlActions, action) + } + + // Match all traffic if no HeaderMatches or CIDRMatch + if len(rule.HeaderMatches) == 0 && rule.CIDRMatch == nil { + // Setup GenericKey action + key := getRateLimitDescriptorKey(descriptorPrefix, rIdx, -1) + value := getRateLimitDescriptorValue(descriptorPrefix, rIdx, -1) + action := &routev3.RateLimit_Action{ + ActionSpecifier: &routev3.RateLimit_Action_GenericKey_{ + GenericKey: &routev3.RateLimit_Action_GenericKey{ + DescriptorKey: key, + DescriptorValue: value, + }, + }, + } + entry := &rlv3.RateLimitDescriptor_Entry{ + Key: key, + Value: value, + } + descriptorEntries = append(descriptorEntries, entry) + rlActions = append(rlActions, action) + } + + rateLimit := &routev3.RateLimit{Actions: rlActions} + rateLimits = append(rateLimits, rateLimit) + + descriptor := &rlv3.LocalRateLimitDescriptor{ + Entries: descriptorEntries, + TokenBucket: &typev3.TokenBucket{ + MaxTokens: uint32(rule.Limit.Requests), + TokensPerFill: &wrapperspb.UInt32Value{ + Value: uint32(rule.Limit.Requests), + }, + FillInterval: ratelimitUnitToDuration(rule.Limit.Unit), + }, + } + descriptors = append(descriptors, descriptor) + } + + return rateLimits, descriptors, nil +} + +func ratelimitUnitToDuration(unit ir.RateLimitUnit) *duration.Duration { + var seconds int64 + + switch egv1a1.RateLimitUnit(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 &duration.Duration{ + Seconds: seconds, + } +} diff --git a/internal/xds/translator/testdata/in/xds-ir/local-ratelimit.yaml b/internal/xds/translator/testdata/in/xds-ir/local-ratelimit.yaml new file mode 100644 index 0000000000..94787199b8 --- /dev/null +++ b/internal/xds/translator/testdata/in/xds-ir/local-ratelimit.yaml @@ -0,0 +1,82 @@ +http: +- name: "first-listener" + address: "0.0.0.0" + port: 10080 + hostnames: + - "*" + routes: + - name: "first-route-ratelimit-single-rule" + hostname: "*" + rateLimit: + local: + rules: + - headerMatches: + - name: x-user-id + exact: one + - name: x-org-id + exact: foo + limit: + requests: 10 + unit: Hour + pathMatch: + exact: "foo/bar" + destination: + name: "first-route-dest" + settings: + - endpoints: + - host: "1.2.3.4" + port: 50000 + - name: "second-route-ratelimit-multiple-rules" + hostname: "*" + rateLimit: + local: + rules: + - headerMatches: + - name: x-user-id + exact: one + - name: x-org-id + exact: foo + limit: + requests: 10 + unit: Hour + - cidrMatch: + cidr: 192.168.0.0/16 + maskLen: 16 + headerMatches: + - name: x-user-id + exact: two + - name: x-org-id + exact: bar + limit: + requests: 10 + unit: Minute + pathMatch: + exact: "example" + destination: + name: "second-route-dest" + settings: + - endpoints: + - host: "1.2.3.4" + port: 50000 + - name: "third-route-ratelimit-no-match" + hostname: "*" + rateLimit: + local: + rules: + - headerMatches: + - name: x-user-id + exact: one + limit: + requests: 10 + unit: Hour + - limit: + requests: 5 + unit: second + pathMatch: + exact: "test" + destination: + name: "third-route-dest" + settings: + - endpoints: + - host: "1.2.3.4" + port: 50000 diff --git a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.clusters.yaml new file mode 100755 index 0000000000..869321c650 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.clusters.yaml @@ -0,0 +1,42 @@ +- commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_ONLY + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: first-route-dest + lbPolicy: LEAST_REQUEST + name: first-route-dest + outlierDetection: {} + perConnectionBufferLimitBytes: 32768 + type: EDS +- commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_ONLY + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: second-route-dest + lbPolicy: LEAST_REQUEST + name: second-route-dest + outlierDetection: {} + perConnectionBufferLimitBytes: 32768 + type: EDS +- commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_ONLY + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: third-route-dest + lbPolicy: LEAST_REQUEST + name: third-route-dest + outlierDetection: {} + perConnectionBufferLimitBytes: 32768 + type: EDS diff --git a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.endpoints.yaml new file mode 100755 index 0000000000..475b89a087 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.endpoints.yaml @@ -0,0 +1,36 @@ +- clusterName: first-route-dest + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: first-route-dest/backend/0 +- clusterName: second-route-dest + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: second-route-dest/backend/0 +- clusterName: third-route-dest + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: third-route-dest/backend/0 diff --git a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.listeners.yaml new file mode 100755 index 0000000000..d9d4a248f2 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.listeners.yaml @@ -0,0 +1,37 @@ +- address: + socketAddress: + address: 0.0.0.0 + portValue: 10080 + defaultFilterChain: + filters: + - name: envoy.filters.network.http_connection_manager + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + commonHttpProtocolOptions: + headersWithUnderscoresAction: REJECT_REQUEST + http2ProtocolOptions: + initialConnectionWindowSize: 1048576 + initialStreamWindowSize: 65536 + maxConcurrentStreams: 100 + httpFilters: + - name: envoy.filters.http.local_ratelimit + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.local_ratelimit.v3.LocalRateLimit + statPrefix: http_local_rate_limiter + - name: envoy.filters.http.router + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + mergeSlashes: true + normalizePath: true + pathWithEscapedSlashesAction: UNESCAPE_AND_REDIRECT + rds: + configSource: + ads: {} + resourceApiVersion: V3 + routeConfigName: first-listener + statPrefix: http + upgradeConfigs: + - upgradeType: websocket + useRemoteAddress: true + name: first-listener + perConnectionBufferLimitBytes: 32768 diff --git a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml new file mode 100755 index 0000000000..da69ca7d60 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml @@ -0,0 +1,149 @@ +- ignorePortInHostMatching: true + name: first-listener + virtualHosts: + - domains: + - '*' + name: first-listener/* + routes: + - match: + path: foo/bar + name: first-route-ratelimit-single-rule + route: + cluster: first-route-dest + rateLimits: + - actions: + - headerValueMatch: + descriptorKey: first-route-ratelimit-single-rule-key-rule-0-match-0 + descriptorValue: first-route-ratelimit-single-rule-value-rule-0-match-0 + expectMatch: true + headers: + - name: x-user-id + stringMatch: + exact: one + - headerValueMatch: + descriptorKey: first-route-ratelimit-single-rule-key-rule-0-match-1 + descriptorValue: first-route-ratelimit-single-rule-value-rule-0-match-1 + expectMatch: true + headers: + - name: x-org-id + stringMatch: + exact: foo + typedPerFilterConfig: + envoy.filters.http.local_ratelimit: + '@type': type.googleapis.com/envoy.extensions.filters.http.local_ratelimit.v3.LocalRateLimit + descriptors: + - entries: + - key: first-route-ratelimit-single-rule-key-rule-0-match-0 + value: first-route-ratelimit-single-rule-value-rule-0-match-0 + - key: first-route-ratelimit-single-rule-key-rule-0-match-1 + value: first-route-ratelimit-single-rule-value-rule-0-match-1 + tokenBucket: + fillInterval: 3600s + maxTokens: 10 + tokensPerFill: 10 + statPrefix: http_local_rate_limiter + - match: + path: example + name: second-route-ratelimit-multiple-rules + route: + cluster: second-route-dest + rateLimits: + - actions: + - headerValueMatch: + descriptorKey: second-route-ratelimit-multiple-rules-key-rule-0-match-0 + descriptorValue: second-route-ratelimit-multiple-rules-value-rule-0-match-0 + expectMatch: true + headers: + - name: x-user-id + stringMatch: + exact: one + - headerValueMatch: + descriptorKey: second-route-ratelimit-multiple-rules-key-rule-0-match-1 + descriptorValue: second-route-ratelimit-multiple-rules-value-rule-0-match-1 + expectMatch: true + headers: + - name: x-org-id + stringMatch: + exact: foo + - actions: + - headerValueMatch: + descriptorKey: second-route-ratelimit-multiple-rules-key-rule-1-match-0 + descriptorValue: second-route-ratelimit-multiple-rules-value-rule-1-match-0 + expectMatch: true + headers: + - name: x-user-id + stringMatch: + exact: two + - headerValueMatch: + descriptorKey: second-route-ratelimit-multiple-rules-key-rule-1-match-1 + descriptorValue: second-route-ratelimit-multiple-rules-value-rule-1-match-1 + expectMatch: true + headers: + - name: x-org-id + stringMatch: + exact: bar + - maskedRemoteAddress: + v4PrefixMaskLen: 16 + typedPerFilterConfig: + envoy.filters.http.local_ratelimit: + '@type': type.googleapis.com/envoy.extensions.filters.http.local_ratelimit.v3.LocalRateLimit + descriptors: + - entries: + - key: second-route-ratelimit-multiple-rules-key-rule-0-match-0 + value: second-route-ratelimit-multiple-rules-value-rule-0-match-0 + - key: second-route-ratelimit-multiple-rules-key-rule-0-match-1 + value: second-route-ratelimit-multiple-rules-value-rule-0-match-1 + tokenBucket: + fillInterval: 3600s + maxTokens: 10 + tokensPerFill: 10 + - entries: + - key: second-route-ratelimit-multiple-rules-key-rule-1-match-0 + value: second-route-ratelimit-multiple-rules-value-rule-1-match-0 + - key: second-route-ratelimit-multiple-rules-key-rule-1-match-1 + value: second-route-ratelimit-multiple-rules-value-rule-1-match-1 + - key: masked_remote_address + value: 192.168.0.0/16 + tokenBucket: + fillInterval: 60s + maxTokens: 10 + tokensPerFill: 10 + statPrefix: http_local_rate_limiter + - match: + path: test + name: third-route-ratelimit-no-match + route: + cluster: third-route-dest + rateLimits: + - actions: + - headerValueMatch: + descriptorKey: third-route-ratelimit-no-match-key-rule-0-match-0 + descriptorValue: third-route-ratelimit-no-match-value-rule-0-match-0 + expectMatch: true + headers: + - name: x-user-id + stringMatch: + exact: one + - actions: + - genericKey: + descriptorKey: third-route-ratelimit-no-match-key-rule-1-match--1 + descriptorValue: third-route-ratelimit-no-match-value-rule-1-match--1 + typedPerFilterConfig: + envoy.filters.http.local_ratelimit: + '@type': type.googleapis.com/envoy.extensions.filters.http.local_ratelimit.v3.LocalRateLimit + descriptors: + - entries: + - key: third-route-ratelimit-no-match-key-rule-0-match-0 + value: third-route-ratelimit-no-match-value-rule-0-match-0 + tokenBucket: + fillInterval: 3600s + maxTokens: 10 + tokensPerFill: 10 + - entries: + - key: third-route-ratelimit-no-match-key-rule-1-match--1 + value: third-route-ratelimit-no-match-value-rule-1-match--1 + tokenBucket: + fillInterval: 0s + maxTokens: 5 + tokensPerFill: 5 + statPrefix: http_local_rate_limiter diff --git a/internal/xds/translator/translator_test.go b/internal/xds/translator/translator_test.go index a501964fab..d21ef55479 100644 --- a/internal/xds/translator/translator_test.go +++ b/internal/xds/translator/translator_test.go @@ -211,6 +211,9 @@ func TestTranslateXds(t *testing.T) { { name: "basic-auth", }, + { + name: "local-ratelimit", + }, } for _, tc := range testCases { diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 650863ef0f..fe65da9a6e 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1099,6 +1099,20 @@ _Appears in:_ +#### LocalRateLimit + + + +LocalRateLimit defines local rate limit configuration. + +_Appears in:_ +- [RateLimitSpec](#ratelimitspec) + +| Field | Description | +| --- | --- | +| `rules` _[RateLimitRule](#ratelimitrule) array_ | Rules are a list of RateLimit selectors and limits. Orders matters here as the rules are processed sequentially. The first rule that matches the request is applied. | + + #### LogLevel _Underlying type:_ `string` @@ -1497,6 +1511,7 @@ RateLimitRule defines the semantics for matching attributes from the incoming re _Appears in:_ - [GlobalRateLimit](#globalratelimit) +- [LocalRateLimit](#localratelimit) | Field | Description | | --- | --- | @@ -1532,6 +1547,7 @@ _Appears in:_ | --- | --- | | `type` _[RateLimitType](#ratelimittype)_ | Type decides the scope for the RateLimits. Valid RateLimitType values are "Global". | | `global` _[GlobalRateLimit](#globalratelimit)_ | Global defines global rate limit configuration. | +| `local` _[LocalRateLimit](#localratelimit)_ | Local defines local rate limit configuration. | #### RateLimitType From 49dd74182b461c90f1d84989f81bcdd452e1b440 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Fri, 1 Dec 2023 17:01:05 +0800 Subject: [PATCH 02/15] e2e test Signed-off-by: huabing zhao --- api/v1alpha1/ratelimit_types.go | 14 +- ....envoyproxy.io_backendtrafficpolicies.yaml | 3 +- site/content/en/latest/api/extension_types.md | 2 +- test/e2e/testdata/local-ratelimit.yaml | 92 +++++++ test/e2e/tests/basic-auth.go | 2 +- test/e2e/tests/local-ratelimit.go | 226 ++++++++++++++++++ test/e2e/tests/ratelimit.go | 14 +- 7 files changed, 341 insertions(+), 12 deletions(-) create mode 100644 test/e2e/testdata/local-ratelimit.yaml create mode 100644 test/e2e/tests/local-ratelimit.go diff --git a/api/v1alpha1/ratelimit_types.go b/api/v1alpha1/ratelimit_types.go index bf8488f46b..d9e55758f9 100644 --- a/api/v1alpha1/ratelimit_types.go +++ b/api/v1alpha1/ratelimit_types.go @@ -9,7 +9,7 @@ package v1alpha1 // +union type RateLimitSpec struct { // Type decides the scope for the RateLimits. - // Valid RateLimitType values are "Global". + // Valid RateLimitType values are "Global" or "Local". // // +unionDiscriminator Type RateLimitType `json:"type"` @@ -25,7 +25,7 @@ type RateLimitSpec struct { } // RateLimitType specifies the types of RateLimiting. -// +kubebuilder:validation:Enum=Global +// +kubebuilder:validation:Enum=Global;Local type RateLimitType string const ( @@ -54,6 +54,16 @@ type GlobalRateLimit struct { // LocalRateLimit defines local rate limit configuration. 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 + // 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. + Default RateLimitValue `json:"default"` + // Rules are a list of RateLimit selectors and limits. // Orders matters here as the rules are processed sequentially. // The first rule that matches the request is applied. diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml index d585a9aa69..145af99d54 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml @@ -370,9 +370,10 @@ spec: type: object type: description: Type decides the scope for the RateLimits. Valid - RateLimitType values are "Global". + RateLimitType values are "Global" or "Local". enum: - Global + - Local type: string required: - type diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index fe65da9a6e..b918406232 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1545,7 +1545,7 @@ _Appears in:_ | Field | Description | | --- | --- | -| `type` _[RateLimitType](#ratelimittype)_ | Type decides the scope for the RateLimits. Valid RateLimitType values are "Global". | +| `type` _[RateLimitType](#ratelimittype)_ | Type decides the scope for the RateLimits. Valid RateLimitType values are "Global" or "Local". | | `global` _[GlobalRateLimit](#globalratelimit)_ | Global defines global rate limit configuration. | | `local` _[LocalRateLimit](#localratelimit)_ | Local defines local rate limit configuration. | diff --git a/test/e2e/testdata/local-ratelimit.yaml b/test/e2e/testdata/local-ratelimit.yaml new file mode 100644 index 0000000000..e2139359fa --- /dev/null +++ b/test/e2e/testdata/local-ratelimit.yaml @@ -0,0 +1,92 @@ +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: BackendTrafficPolicy +metadata: + name: ratelimit-specific-user + namespace: gateway-conformance-infra +spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: http-ratelimit-specific-user + namespace: gateway-conformance-infra + rateLimit: + type: Local + local: + rules: + - clientSelectors: + - headers: + - name: x-user-id + value: john + limit: + requests: 3 + unit: Hour +--- +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: BackendTrafficPolicy +metadata: + name: ratelimit-all-traffic + namespace: gateway-conformance-infra +spec: + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: http-ratelimit-all-traffic + namespace: gateway-conformance-infra + rateLimit: + type: Local + local: + rules: + - limit: + requests: 3 + unit: Hour +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: http-ratelimit-specific-user + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: same-namespace + rules: + - backendRefs: + - name: infra-backend-v1 + port: 8080 + matches: + - path: + type: Exact + value: /ratelimit-specific-user +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: http-ratelimit-all-traffic + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: same-namespace + rules: + - backendRefs: + - name: infra-backend-v1 + port: 8080 + matches: + - path: + type: Exact + value: /ratelimit-all-traffic +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: http-no-ratelimit + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: same-namespace + rules: + - backendRefs: + - name: infra-backend-v1 + port: 8080 + matches: + - path: + type: Exact + value: /no-ratelimit diff --git a/test/e2e/tests/basic-auth.go b/test/e2e/tests/basic-auth.go index a5a584d226..cecac93021 100644 --- a/test/e2e/tests/basic-auth.go +++ b/test/e2e/tests/basic-auth.go @@ -181,5 +181,5 @@ func SecurityPolicyMustBeAccepted( t.Logf("SecurityPolicy not yet accepted: %v", securityPolicy) return false, nil }) - require.NoErrorf(t, waitErr, "error waiting for HTTPRoute to have parents matching expectations") + require.NoErrorf(t, waitErr, "error waiting for SecurityPolicy to be accepted") } diff --git a/test/e2e/tests/local-ratelimit.go b/test/e2e/tests/local-ratelimit.go new file mode 100644 index 0000000000..7cdf5768ef --- /dev/null +++ b/test/e2e/tests/local-ratelimit.go @@ -0,0 +1,226 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +//go:build e2e +// +build e2e + +package tests + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" + "sigs.k8s.io/controller-runtime/pkg/client" + gwv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + "sigs.k8s.io/gateway-api/conformance/utils/http" + "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" + "sigs.k8s.io/gateway-api/conformance/utils/suite" + + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" +) + +func init() { + ConformanceTests = append(ConformanceTests, LocalRateLimitSpecificUserTest) + ConformanceTests = append(ConformanceTests, LocalRateLimitAllTrafficTest) + ConformanceTests = append(ConformanceTests, LocalRateLimitNoLimitRouteTest) +} + +var LocalRateLimitSpecificUserTest = suite.ConformanceTest{ + ShortName: "LocalRateLimitSpecificUser", + Description: "Limit a specific user", + Manifests: []string{"testdata/local-ratelimit.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + t.Run("limit a specific user", func(t *testing.T) { + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "http-ratelimit-specific-user", Namespace: ns} + gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + backendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-specific-user", Namespace: ns}) + + expectOkResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-specific-user", + Headers: map[string]string{ + "x-user-id": "john", + }, + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + + expectOkReq := http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") + + expectLimitResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-specific-user", + Headers: map[string]string{ + "x-user-id": "john", + }, + }, + Response: http.Response{ + StatusCode: 429, + }, + Namespace: ns, + } + expectLimitReq := http.MakeRequest(t, &expectLimitResp, gwAddr, "HTTP", "http") + + // should just send exactly 4 requests, and expect 429 + + // keep sending requests till get 200 first, that will cost one 200 + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) + + // fire the rest request + if err := GotExactExpectedResponse(t, 2, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { + t.Errorf("fail to get expected response at first three request: %v", err) + } + + // this request should be limited because the user is john and the limit is 3 + if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectLimitResp); err != nil { + t.Errorf("fail to get expected response at last fourth request: %v", err) + } + + // test another user + expectOkResp = http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-specific-user", + Headers: map[string]string{ + "x-user-id": "mike", + }, + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + expectOkReq = http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") + // the requests should not be limited because the user is mike + if err := GotExactExpectedResponse(t, 4, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { + t.Errorf("fail to get expected response at first three request: %v", err) + } + }) + }, +} + +var LocalRateLimitAllTrafficTest = suite.ConformanceTest{ + ShortName: "LocalRateLimitAllTraffic", + Description: "Limit all traffic on a route", + Manifests: []string{"testdata/local-ratelimit.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + t.Run("limit all traffic on a route", func(t *testing.T) { + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "http-ratelimit-all-traffic", Namespace: ns} + gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + backendTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "ratelimit-all-traffic", Namespace: ns}) + + expectOkResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-all-traffic", + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + + expectOkReq := http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") + + expectLimitResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/ratelimit-all-traffic", + }, + Response: http.Response{ + StatusCode: 429, + }, + Namespace: ns, + } + expectLimitReq := http.MakeRequest(t, &expectLimitResp, gwAddr, "HTTP", "http") + + // should just send exactly 4 requests, and expect 429 + + // keep sending requests till get 200 first, that will cost one 200 + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) + + // fire the rest request + if err := GotExactExpectedResponse(t, 2, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { + t.Errorf("fail to get expected response at first three request: %v", err) + } + + // this request should be limited because the user is john and the limit is 3 + if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectLimitResp); err != nil { + t.Errorf("fail to get expected response at last fourth request: %v", err) + } + }) + }, +} + +var LocalRateLimitNoLimitRouteTest = suite.ConformanceTest{ + ShortName: "LocalRateLimitNoLimitRoute", + Description: "No rate limit on this route", + Manifests: []string{"testdata/local-ratelimit.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + t.Run("no rate limit on this route", func(t *testing.T) { + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "SecurityPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: \"ratelimit-specific-user\", Namespace: ns})", Namespace: ns} + gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + + expectOkResp := http.ExpectedResponse{ + Request: http.Request{ + Path: "/no-ratelimit", + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + + expectOkReq := http.MakeRequest(t, &expectOkResp, gwAddr, "HTTP", "http") + + // should just send exactly 4 requests, and expect 429 + + // keep sending requests till get 200 first, that will cost one 200 + http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) + + // the requests should not be limited because there is no rate limit on this route + if err := GotExactExpectedResponse(t, 3, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { + t.Errorf("fail to get expected response at last fourth request: %v", err) + } + }) + }, +} + +// backendTrafficPolicyMustBeAccepted waits for the specified BackendTrafficPolicy to be accepted. +func backendTrafficPolicyMustBeAccepted( + t *testing.T, + client client.Client, + securityPolicyName types.NamespacedName) { + t.Helper() + + waitErr := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 60*time.Second, true, func(ctx context.Context) (bool, error) { + policy := &egv1a1.BackendTrafficPolicy{} + err := client.Get(ctx, securityPolicyName, policy) + if err != nil { + return false, fmt.Errorf("error fetching SecurityPolicy: %w", err) + } + + for _, condition := range policy.Status.Conditions { + if condition.Type == string(gwv1a2.PolicyConditionAccepted) && condition.Status == metav1.ConditionTrue { + return true, nil + } + } + t.Logf("BackendTrafficPolicy not yet accepted: %v", policy) + return false, nil + }) + require.NoErrorf(t, waitErr, "error waiting for BackendTrafficPolicy to be accepted") +} diff --git a/test/e2e/tests/ratelimit.go b/test/e2e/tests/ratelimit.go index 6293731551..eb48f955f3 100644 --- a/test/e2e/tests/ratelimit.go +++ b/test/e2e/tests/ratelimit.go @@ -64,10 +64,10 @@ var RateLimitTest = suite.ConformanceTest{ http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectOkResp) // fire the rest request - if err := GotExactNExpectedResponse(t, 2, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { + if err := GotExactExpectedResponse(t, 2, suite.RoundTripper, expectOkReq, expectOkResp); err != nil { t.Errorf("fail to get expected response at first three request: %v", err) } - if err := GotExactNExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectLimitResp); err != nil { + if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectLimitResp); err != nil { t.Errorf("fail to get expected response at last fourth request: %v", err) } }) @@ -165,15 +165,15 @@ var RateLimitBasedJwtClaimsTest = suite.ConformanceTest{ http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, JwtOkResp) // fire the rest request - if err := GotExactNExpectedResponse(t, 2, suite.RoundTripper, JwtReq, JwtOkResp); err != nil { + if err := GotExactExpectedResponse(t, 2, suite.RoundTripper, JwtReq, JwtOkResp); err != nil { t.Errorf("failed to get expected response at third request: %v", err) } - if err := GotExactNExpectedResponse(t, 1, suite.RoundTripper, JwtReq, expectLimitResp); err != nil { + if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, JwtReq, expectLimitResp); err != nil { t.Errorf("failed to get expected response at the fourth request: %v", err) } // Carrying different jwt claims will not be limited - if err := GotExactNExpectedResponse(t, 4, suite.RoundTripper, difJwtReq, expectOkResp); err != nil { + if err := GotExactExpectedResponse(t, 4, suite.RoundTripper, difJwtReq, expectOkResp); err != nil { t.Errorf("failed to get expected response for the request with a different jwt: %v", err) } @@ -188,7 +188,7 @@ var RateLimitBasedJwtClaimsTest = suite.ConformanceTest{ Namespace: ns, } noTokenReq := http.MakeRequest(t, &noTokenResp, gwAddr, "HTTP", "http") - if err := GotExactNExpectedResponse(t, 1, suite.RoundTripper, noTokenReq, noTokenResp); err != nil { + if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, noTokenReq, noTokenResp); err != nil { t.Errorf("failed to get expected response: %v", err) } @@ -196,7 +196,7 @@ var RateLimitBasedJwtClaimsTest = suite.ConformanceTest{ }, } -func GotExactNExpectedResponse(t *testing.T, n int, r roundtripper.RoundTripper, req roundtripper.Request, resp http.ExpectedResponse) error { +func GotExactExpectedResponse(t *testing.T, n int, r roundtripper.RoundTripper, req roundtripper.Request, resp http.ExpectedResponse) error { for i := 0; i < n; i++ { cReq, cRes, err := r.CaptureRoundTrip(req) if err != nil { From cebb9990ea0532b0320dc85a2ecd969bd8460818 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Sat, 2 Dec 2023 13:02:43 +0800 Subject: [PATCH 03/15] add default bucket for local rate limit Signed-off-by: huabing zhao --- api/v1alpha1/ratelimit_types.go | 28 ++- api/v1alpha1/zz_generated.deepcopy.go | 1 + ....envoyproxy.io_backendtrafficpolicies.yaml | 55 ++++-- internal/gatewayapi/backendtrafficpolicy.go | 59 +++++- ...th-local-ratelimit-invalid-default.in.yaml | 74 ++++++++ ...h-local-ratelimit-invalid-default.out.yaml | 174 ++++++++++++++++++ ...local-ratelimit-invalid-match-type.in.yaml | 74 ++++++++ ...ocal-ratelimit-invalid-match-type.out.yaml | 173 +++++++++++++++++ ...h-local-ratelimit-invalid-no-match.in.yaml | 57 ++++++ ...-local-ratelimit-invalid-no-match.out.yaml | 157 ++++++++++++++++ ...trafficpolicy-with-local-ratelimit.in.yaml | 56 +----- ...rafficpolicy-with-local-ratelimit.out.yaml | 174 ++---------------- internal/ir/xds.go | 11 +- internal/ir/zz_generated.deepcopy.go | 1 + internal/xds/translator/local_ratelimit.go | 47 ++--- .../testdata/in/xds-ir/local-ratelimit.yaml | 21 +-- .../out/xds-ir/local-ratelimit.routes.yaml | 61 +++--- site/content/en/latest/api/extension_types.md | 12 +- test/e2e/testdata/local-ratelimit.yaml | 10 +- test/e2e/tests/local-ratelimit.go | 8 +- 20 files changed, 940 insertions(+), 313 deletions(-) create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.in.yaml create mode 100755 internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.out.yaml create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.in.yaml create mode 100755 internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.out.yaml create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.in.yaml create mode 100755 internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.out.yaml diff --git a/api/v1alpha1/ratelimit_types.go b/api/v1alpha1/ratelimit_types.go index d9e55758f9..a3459a97b1 100644 --- a/api/v1alpha1/ratelimit_types.go +++ b/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,28 @@ 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. - Default RateLimitValue `json:"default"` - - // Rules are a list of RateLimit selectors and limits. + // Limit is the default rate limit for a route if no rules match. + // If a request does not match any of the rules, it is counted towards this limit. + // + // Note: Limit 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 limit is 100r/s, then + // each route has its own 100r/s rate limit bucket. + Limit RateLimitValue `json:"default"` + + // Rules are a list of RateLimit selectors and limits. They're used to define + // fine-grained rate limits that can be applied to specific clients using + // attributes from the traffic flow. + // // 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 +91,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 +110,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 +119,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"` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 5ed670e449..c9c1cd5e38 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1634,6 +1634,7 @@ func (in *LoadBalancer) DeepCopy() *LoadBalancer { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LocalRateLimit) DeepCopyInto(out *LocalRateLimit) { *out = *in + out.Limit = in.Limit if in.Rules != nil { in, out := &in.Rules, &out.Rules *out = make([]RateLimitRule, len(*in)) diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml index 145af99d54..78dbca77f2 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml @@ -139,8 +139,6 @@ spec: conditions to select 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. items: description: RateLimitSelectCondition specifies the attributes within the traffic flow that can be used @@ -152,7 +150,8 @@ spec: description: Headers is a list of request headers to match. Multiple header values are ANDed together, meaning, a request MUST match all the specified - headers. + headers. At least one of headers or sourceCIDR + condition must be specified. items: description: HeaderMatch defines the match attributes within the HTTP Headers of the request. @@ -190,7 +189,8 @@ spec: x-kubernetes-list-type: map sourceCIDR: description: SourceCIDR is the client IP Address - range to match on. + range to match on. At least one of headers or + sourceCIDR condition must be specified. properties: type: default: Exact @@ -211,6 +211,7 @@ spec: type: object type: object maxItems: 8 + minItems: 1 type: array limit: description: Limit holds the rate limit values. This @@ -248,10 +249,39 @@ spec: local: description: Local defines local rate limit configuration. properties: + default: + description: "Limit is the default rate limit for a route + if no rules match. If a request does not match any of the + rules, it is counted towards this limit. \n Note: Limit + 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 + limit is 100r/s, then each route has its own 100r/s rate + limit bucket." + properties: + requests: + type: integer + unit: + description: RateLimitUnit specifies the intervals for + setting rate limits. Valid RateLimitUnit values are + "Second", "Minute", "Hour", and "Day". + enum: + - Second + - Minute + - Hour + - Day + type: string + required: + - requests + - unit + type: object rules: - description: Rules are a list of RateLimit selectors and limits. - Orders matters here as the rules are processed sequentially. - The first rule that matches the request is applied. + description: "Rules are a list of RateLimit selectors and + limits. They're used to define fine-grained rate limits + that can be applied to specific clients using attributes + from the traffic flow. \n Orders matters here as the rules + are processed sequentially. The first rule that matches + the request is applied." items: description: RateLimitRule defines the semantics for matching attributes from the incoming requests, and setting limits @@ -262,8 +292,6 @@ spec: conditions to select 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. items: description: RateLimitSelectCondition specifies the attributes within the traffic flow that can be used @@ -275,7 +303,8 @@ spec: description: Headers is a list of request headers to match. Multiple header values are ANDed together, meaning, a request MUST match all the specified - headers. + headers. At least one of headers or sourceCIDR + condition must be specified. items: description: HeaderMatch defines the match attributes within the HTTP Headers of the request. @@ -313,7 +342,8 @@ spec: x-kubernetes-list-type: map sourceCIDR: description: SourceCIDR is the client IP Address - range to match on. + range to match on. At least one of headers or + sourceCIDR condition must be specified. properties: type: default: Exact @@ -334,6 +364,7 @@ spec: type: object type: object maxItems: 8 + minItems: 1 type: array limit: description: Limit holds the rate limit values. This @@ -366,7 +397,7 @@ spec: maxItems: 16 type: array required: - - rules + - default type: object type: description: Type decides the scope for the RateLimits. Valid diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index 029397c64e..42e2804c32 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/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.Limit.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.Limit.Requests, + Unit: ir.RateLimitUnit(local.Limit.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 +} diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.in.yaml new file mode 100644 index 0000000000..b2ee456765 --- /dev/null +++ b/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 diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.out.yaml new file mode 100755 index 0000000000..dccc484bf9 --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.out.yaml @@ -0,0 +1,174 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway + namespace: envoy-gateway + spec: + rateLimit: + 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 + type: Local + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + status: + conditions: + - lastTransitionTime: null + message: Local rateLimit rule limit unit must be a multiple of the default limit + unit. + reason: Invalid + status: "False" + type: Accepted +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-1 +xdsIR: + envoy-gateway/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: gateway.envoyproxy.io + name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io + pathMatch: + distinct: false + name: "" + prefix: / diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.in.yaml new file mode 100644 index 0000000000..783dbb557b --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.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: 100 + unit: Second + rules: + - clientSelectors: + - headers: + - name: x-user-id + value: one + - name: x-org-id + type: Distinct + 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 diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.out.yaml new file mode 100755 index 0000000000..3f6b9030c9 --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.out.yaml @@ -0,0 +1,173 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway + namespace: envoy-gateway + spec: + rateLimit: + local: + default: + requests: 100 + unit: Second + rules: + - clientSelectors: + - headers: + - name: x-user-id + value: one + - name: x-org-id + type: Distinct + 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 + type: Local + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + status: + conditions: + - lastTransitionTime: null + message: Local rateLimit does not support distinct HeaderMatch. + reason: Invalid + status: "False" + type: Accepted +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-1 +xdsIR: + envoy-gateway/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: gateway.envoyproxy.io + name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io + pathMatch: + distinct: false + name: "" + prefix: / diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.in.yaml new file mode 100644 index 0000000000..28b78177c8 --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.in.yaml @@ -0,0 +1,57 @@ +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: 100 + unit: Second + rules: + - limit: + requests: 10 + unit: Minute diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.out.yaml new file mode 100755 index 0000000000..69be65457a --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.out.yaml @@ -0,0 +1,157 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway + namespace: envoy-gateway + spec: + rateLimit: + local: + default: + requests: 100 + unit: Second + rules: + - limit: + requests: 10 + unit: Minute + type: Local + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + status: + conditions: + - lastTransitionTime: null + message: Unable to translate rateLimit. At least one clientSelector must be + specified. + reason: Invalid + status: "False" + type: Accepted +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-1 +xdsIR: + envoy-gateway/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: gateway.envoyproxy.io + name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io + pathMatch: + distinct: false + name: "" + prefix: / diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml index 1c4903692b..d906972101 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml @@ -13,35 +13,6 @@ gateways: allowedRoutes: namespaces: from: All -- apiVersion: gateway.networking.k8s.io/v1 - kind: Gateway - metadata: - namespace: envoy-gateway - name: gateway-2 - spec: - gatewayClassName: envoy-gateway-class - listeners: - - name: http - protocol: HTTP - port: 80 - allowedRoutes: - namespaces: - from: All -grpcRoutes: -- apiVersion: gateway.networking.k8s.io/v1alpha2 - kind: GRPCRoute - metadata: - namespace: default - name: grpcroute-1 - spec: - parentRefs: - - namespace: envoy-gateway - name: gateway-1 - sectionName: http - rules: - - backendRefs: - - name: service-1 - port: 8080 httpRoutes: - apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute @@ -53,7 +24,7 @@ httpRoutes: - gateway.envoyproxy.io parentRefs: - namespace: envoy-gateway - name: gateway-2 + name: gateway-1 sectionName: http rules: - matches: @@ -77,6 +48,9 @@ backendTrafficPolicies: rateLimit: type: Local local: + default: + requests: 10 + unit: Minute rules: - clientSelectors: - headers: @@ -98,25 +72,3 @@ backendTrafficPolicies: limit: requests: 10 unit: Minute -- apiVersion: gateway.envoyproxy.io/v1alpha1 - kind: BackendTrafficPolicy - metadata: - namespace: default - name: policy-for-route - spec: - targetRef: - group: gateway.networking.k8s.io - kind: HTTPRoute - name: httproute-1 - namespace: default - rateLimit: - type: Local - local: - rules: - - clientSelectors: - - sourceCIDR: - type: "Distinct" - value: 192.168.0.0/16 - limit: - requests: 20 - unit: Hour diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml index ff31805d4c..82339b6e8e 100755 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml @@ -1,34 +1,4 @@ backendTrafficPolicies: -- apiVersion: gateway.envoyproxy.io/v1alpha1 - kind: BackendTrafficPolicy - metadata: - creationTimestamp: null - name: policy-for-route - namespace: default - spec: - rateLimit: - local: - rules: - - clientSelectors: - - sourceCIDR: - type: Distinct - value: 192.168.0.0/16 - limit: - requests: 20 - unit: Hour - type: Local - targetRef: - group: gateway.networking.k8s.io - kind: HTTPRoute - name: httproute-1 - namespace: default - status: - conditions: - - lastTransitionTime: null - message: Local rateLimit does not support distinct CIDRMatch. - reason: Invalid - status: "False" - type: Accepted - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: BackendTrafficPolicy metadata: @@ -38,6 +8,9 @@ backendTrafficPolicies: spec: rateLimit: local: + default: + requests: 10 + unit: Minute rules: - clientSelectors: - headers: @@ -113,80 +86,6 @@ gateways: kind: HTTPRoute - group: gateway.networking.k8s.io kind: GRPCRoute -- apiVersion: gateway.networking.k8s.io/v1 - kind: Gateway - metadata: - creationTimestamp: null - name: gateway-2 - namespace: envoy-gateway - spec: - gatewayClassName: envoy-gateway-class - listeners: - - allowedRoutes: - namespaces: - from: All - name: http - port: 80 - protocol: HTTP - status: - listeners: - - attachedRoutes: 1 - conditions: - - lastTransitionTime: null - message: Sending translated listener configuration to the data plane - reason: Programmed - status: "True" - type: Programmed - - lastTransitionTime: null - message: Listener has been successfully translated - reason: Accepted - status: "True" - type: Accepted - - lastTransitionTime: null - message: Listener references have been resolved - reason: ResolvedRefs - status: "True" - type: ResolvedRefs - name: http - supportedKinds: - - group: gateway.networking.k8s.io - kind: HTTPRoute - - group: gateway.networking.k8s.io - kind: GRPCRoute -grpcRoutes: -- apiVersion: gateway.networking.k8s.io/v1alpha2 - kind: GRPCRoute - metadata: - creationTimestamp: null - name: grpcroute-1 - namespace: default - spec: - parentRefs: - - name: gateway-1 - namespace: envoy-gateway - sectionName: http - rules: - - backendRefs: - - name: service-1 - port: 8080 - status: - parents: - - conditions: - - lastTransitionTime: null - message: Route is accepted - reason: Accepted - status: "True" - type: Accepted - - lastTransitionTime: null - message: Resolved all the Object references for the Route - reason: ResolvedRefs - status: "True" - type: ResolvedRefs - controllerName: gateway.envoyproxy.io/gatewayclass-controller - parentRef: - name: gateway-1 - namespace: envoy-gateway - sectionName: http httpRoutes: - apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute @@ -198,7 +97,7 @@ httpRoutes: hostnames: - gateway.envoyproxy.io parentRefs: - - name: gateway-2 + - name: gateway-1 namespace: envoy-gateway sectionName: http rules: @@ -223,7 +122,7 @@ httpRoutes: type: ResolvedRefs controllerName: gateway.envoyproxy.io/gatewayclass-controller parentRef: - name: gateway-2 + name: gateway-1 namespace: envoy-gateway sectionName: http infraIR: @@ -241,20 +140,6 @@ infraIR: gateway.envoyproxy.io/owning-gateway-name: gateway-1 gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway name: envoy-gateway/gateway-1 - envoy-gateway/gateway-2: - proxy: - listeners: - - address: "" - ports: - - containerPort: 10080 - name: http - protocol: HTTP - servicePort: 80 - metadata: - labels: - gateway.envoyproxy.io/owning-gateway-name: gateway-2 - gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway - name: envoy-gateway/gateway-2 xdsIR: envoy-gateway/gateway-1: accessLog: @@ -264,7 +149,7 @@ xdsIR: - address: 0.0.0.0 hostnames: - '*' - isHTTP2: true + isHTTP2: false name: envoy-gateway/gateway-1/http port: 10080 routes: @@ -272,17 +157,25 @@ xdsIR: invalid: 0 valid: 0 destination: - name: grpcroute/default/grpcroute-1/rule/0 + name: httproute/default/httproute-1/rule/0 settings: - - endpoints: + - addressType: IP + endpoints: - host: 7.7.7.7 port: 8080 - protocol: GRPC + protocol: HTTP weight: 1 - hostname: '*' - name: grpcroute/default/grpcroute-1/rule/0/match/-1/* + hostname: gateway.envoyproxy.io + name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io + pathMatch: + distinct: false + name: "" + prefix: / rateLimit: local: + default: + requests: 10 + unit: Minute rules: - headerMatches: - distinct: false @@ -309,32 +202,3 @@ xdsIR: limit: requests: 10 unit: Minute - envoy-gateway/gateway-2: - accessLog: - text: - - path: /dev/stdout - http: - - address: 0.0.0.0 - hostnames: - - '*' - isHTTP2: false - name: envoy-gateway/gateway-2/http - port: 10080 - routes: - - backendWeights: - invalid: 0 - valid: 0 - destination: - name: httproute/default/httproute-1/rule/0 - settings: - - endpoints: - - host: 7.7.7.7 - port: 8080 - protocol: HTTP - weight: 1 - hostname: gateway.envoyproxy.io - name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io - pathMatch: - distinct: false - name: "" - prefix: / diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 00a9a262ac..5ef9cfc49a 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -898,6 +898,8 @@ type RateLimit struct { // GlobalRateLimit holds the global rate limiting configuration. // +k8s:deepcopy-gen=true type GlobalRateLimit struct { + // TODO zhaohuabing: add default values for Global rate limiting. + // Rules for rate limiting. Rules []*RateLimitRule `json:"rules,omitempty" yaml:"rules,omitempty"` } @@ -905,6 +907,10 @@ type GlobalRateLimit struct { // LocalRateLimit holds the local rate limiting configuration. // +k8s:deepcopy-gen=true type LocalRateLimit struct { + // Default rate limiting values. + // If a request does not match any of the rules, the default values are used. + Default RateLimitValue `json:"default,omitempty" yaml:"default,omitempty"` + // Rules for rate limiting. Rules []*RateLimitRule `json:"rules,omitempty" yaml:"rules,omitempty"` } @@ -934,11 +940,6 @@ func (r *RateLimitRule) IsMatchSet() bool { return len(r.HeaderMatches) != 0 || r.CIDRMatch != nil } -// MatchAll returns true if the rule matches all requests on a route. -func (r *RateLimitRule) MatchAll() bool { - return len(r.HeaderMatches) == 0 || r.CIDRMatch == nil -} - type RateLimitUnit egv1a1.RateLimitUnit // RateLimitValue holds the diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 39d1a47fed..42fee40c04 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -715,6 +715,7 @@ func (in *LoadBalancer) DeepCopy() *LoadBalancer { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LocalRateLimit) DeepCopyInto(out *LocalRateLimit) { *out = *in + out.Default = in.Default if in.Rules != nil { in, out := &in.Rules, &out.Rules *out = make([]*RateLimitRule, len(*in)) diff --git a/internal/xds/translator/local_ratelimit.go b/internal/xds/translator/local_ratelimit.go index 27f588fd18..e8d8db449e 100644 --- a/internal/xds/translator/local_ratelimit.go +++ b/internal/xds/translator/local_ratelimit.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" + configv3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" rlv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/common/ratelimit/v3" localrlv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/local_ratelimit/v3" @@ -128,7 +129,9 @@ func (*localRateLimit) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) e route.Name) } - rateLimits, descriptors, err := buildRouteLocalRateLimits(irRoute.Name, irRoute.RateLimit.Local) + local := irRoute.RateLimit.Local + + rateLimits, descriptors, err := buildRouteLocalRateLimits(irRoute.Name, local) if err != nil { return err } @@ -144,7 +147,26 @@ func (*localRateLimit) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) e } localRl := &localrlv3.LocalRateLimit{ - StatPrefix: localRateLimitFilterStatPrefix, + StatPrefix: localRateLimitFilterStatPrefix, + TokenBucket: &typev3.TokenBucket{ + MaxTokens: uint32(local.Default.Requests), + TokensPerFill: &wrapperspb.UInt32Value{ + Value: uint32(local.Default.Requests), + }, + FillInterval: ratelimitUnitToDuration(local.Default.Unit), + }, + FilterEnabled: &configv3.RuntimeFractionalPercent{ + DefaultValue: &typev3.FractionalPercent{ + Numerator: 100, + Denominator: typev3.FractionalPercent_HUNDRED, + }, + }, + FilterEnforced: &configv3.RuntimeFractionalPercent{ + DefaultValue: &typev3.FractionalPercent{ + Numerator: 100, + Denominator: typev3.FractionalPercent_HUNDRED, + }, + }, Descriptors: descriptors, } @@ -239,27 +261,6 @@ func buildRouteLocalRateLimits(descriptorPrefix string, local *ir.LocalRateLimit rlActions = append(rlActions, action) } - // Match all traffic if no HeaderMatches or CIDRMatch - if len(rule.HeaderMatches) == 0 && rule.CIDRMatch == nil { - // Setup GenericKey action - key := getRateLimitDescriptorKey(descriptorPrefix, rIdx, -1) - value := getRateLimitDescriptorValue(descriptorPrefix, rIdx, -1) - action := &routev3.RateLimit_Action{ - ActionSpecifier: &routev3.RateLimit_Action_GenericKey_{ - GenericKey: &routev3.RateLimit_Action_GenericKey{ - DescriptorKey: key, - DescriptorValue: value, - }, - }, - } - entry := &rlv3.RateLimitDescriptor_Entry{ - Key: key, - Value: value, - } - descriptorEntries = append(descriptorEntries, entry) - rlActions = append(rlActions, action) - } - rateLimit := &routev3.RateLimit{Actions: rlActions} rateLimits = append(rateLimits, rateLimit) diff --git a/internal/xds/translator/testdata/in/xds-ir/local-ratelimit.yaml b/internal/xds/translator/testdata/in/xds-ir/local-ratelimit.yaml index 94787199b8..ee136965b9 100644 --- a/internal/xds/translator/testdata/in/xds-ir/local-ratelimit.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/local-ratelimit.yaml @@ -9,6 +9,9 @@ http: hostname: "*" rateLimit: local: + default: + requests: 10 + unit: Minute rules: - headerMatches: - name: x-user-id @@ -30,6 +33,9 @@ http: hostname: "*" rateLimit: local: + default: + requests: 10 + unit: Minute rules: - headerMatches: - name: x-user-id @@ -58,20 +64,13 @@ http: - endpoints: - host: "1.2.3.4" port: 50000 - - name: "third-route-ratelimit-no-match" + - name: "third-route-ratelimit-no-rule" hostname: "*" rateLimit: local: - rules: - - headerMatches: - - name: x-user-id - exact: one - limit: - requests: 10 - unit: Hour - - limit: - requests: 5 - unit: second + default: + requests: 10 + unit: Minute pathMatch: exact: "test" destination: diff --git a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml index da69ca7d60..beb161f612 100755 --- a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml @@ -41,7 +41,17 @@ fillInterval: 3600s maxTokens: 10 tokensPerFill: 10 + filterEnabled: + defaultValue: + numerator: 100 + filterEnforced: + defaultValue: + numerator: 100 statPrefix: http_local_rate_limiter + tokenBucket: + fillInterval: 60s + maxTokens: 10 + tokensPerFill: 10 - match: path: example name: second-route-ratelimit-multiple-rules @@ -108,42 +118,33 @@ fillInterval: 60s maxTokens: 10 tokensPerFill: 10 + filterEnabled: + defaultValue: + numerator: 100 + filterEnforced: + defaultValue: + numerator: 100 statPrefix: http_local_rate_limiter + tokenBucket: + fillInterval: 60s + maxTokens: 10 + tokensPerFill: 10 - match: path: test - name: third-route-ratelimit-no-match + name: third-route-ratelimit-no-rule route: cluster: third-route-dest - rateLimits: - - actions: - - headerValueMatch: - descriptorKey: third-route-ratelimit-no-match-key-rule-0-match-0 - descriptorValue: third-route-ratelimit-no-match-value-rule-0-match-0 - expectMatch: true - headers: - - name: x-user-id - stringMatch: - exact: one - - actions: - - genericKey: - descriptorKey: third-route-ratelimit-no-match-key-rule-1-match--1 - descriptorValue: third-route-ratelimit-no-match-value-rule-1-match--1 typedPerFilterConfig: envoy.filters.http.local_ratelimit: '@type': type.googleapis.com/envoy.extensions.filters.http.local_ratelimit.v3.LocalRateLimit - descriptors: - - entries: - - key: third-route-ratelimit-no-match-key-rule-0-match-0 - value: third-route-ratelimit-no-match-value-rule-0-match-0 - tokenBucket: - fillInterval: 3600s - maxTokens: 10 - tokensPerFill: 10 - - entries: - - key: third-route-ratelimit-no-match-key-rule-1-match--1 - value: third-route-ratelimit-no-match-value-rule-1-match--1 - tokenBucket: - fillInterval: 0s - maxTokens: 5 - tokensPerFill: 5 + filterEnabled: + defaultValue: + numerator: 100 + filterEnforced: + defaultValue: + numerator: 100 statPrefix: http_local_rate_limiter + tokenBucket: + fillInterval: 60s + maxTokens: 10 + tokensPerFill: 10 diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index b918406232..d15571b662 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1110,7 +1110,10 @@ _Appears in:_ | Field | Description | | --- | --- | -| `rules` _[RateLimitRule](#ratelimitrule) array_ | Rules are a list of RateLimit selectors and limits. Orders matters here as the rules are processed sequentially. The first rule that matches the request is applied. | +| `default` _[RateLimitValue](#ratelimitvalue)_ | Limit is the default rate limit for a route if no rules match. If a request does not match any of the rules, it is counted towards this limit. + Note: Limit 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 limit is 100r/s, then each route has its own 100r/s rate limit bucket. | +| `rules` _[RateLimitRule](#ratelimitrule) array_ | Rules are a list of RateLimit selectors and limits. They're used to define fine-grained rate limits that can be applied to specific clients using attributes from the traffic flow. + Orders matters here as the rules are processed sequentially. The first rule that matches the request is applied. | #### LogLevel @@ -1515,7 +1518,7 @@ _Appears in:_ | Field | Description | | --- | --- | -| `clientSelectors` _[RateLimitSelectCondition](#ratelimitselectcondition) array_ | ClientSelectors holds the list of select conditions to select 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. | +| `clientSelectors` _[RateLimitSelectCondition](#ratelimitselectcondition) array_ | ClientSelectors holds the list of select conditions to select specific clients using attributes from the traffic flow. All individual select conditions must hold True for this rule and its limit to be applied. | | `limit` _[RateLimitValue](#ratelimitvalue)_ | Limit holds the rate limit values. This limit is applied for traffic flows when the selectors compute to True, causing the request to be counted towards the limit. The limit is enforced and the request is ratelimited, i.e. a response with 429 HTTP status code is sent back to the client when the selected requests have reached the limit. | @@ -1530,8 +1533,8 @@ _Appears in:_ | Field | Description | | --- | --- | -| `headers` _[HeaderMatch](#headermatch) array_ | Headers is a list of request headers to match. Multiple header values are ANDed together, meaning, a request MUST match all the specified headers. | -| `sourceCIDR` _[SourceMatch](#sourcematch)_ | SourceCIDR is the client IP Address range to match on. | +| `headers` _[HeaderMatch](#headermatch) array_ | 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. | +| `sourceCIDR` _[SourceMatch](#sourcematch)_ | SourceCIDR is the client IP Address range to match on. At least one of headers or sourceCIDR condition must be specified. | #### RateLimitSpec @@ -1579,6 +1582,7 @@ _Appears in:_ RateLimitValue defines the limits for rate limiting. _Appears in:_ +- [LocalRateLimit](#localratelimit) - [RateLimitRule](#ratelimitrule) | Field | Description | diff --git a/test/e2e/testdata/local-ratelimit.yaml b/test/e2e/testdata/local-ratelimit.yaml index e2139359fa..71268732f8 100644 --- a/test/e2e/testdata/local-ratelimit.yaml +++ b/test/e2e/testdata/local-ratelimit.yaml @@ -12,6 +12,9 @@ spec: rateLimit: type: Local local: + default: + requests: 10 + unit: Hour rules: - clientSelectors: - headers: @@ -35,10 +38,9 @@ spec: rateLimit: type: Local local: - rules: - - limit: - requests: 3 - unit: Hour + default: + requests: 3 + unit: Hour --- apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute diff --git a/test/e2e/tests/local-ratelimit.go b/test/e2e/tests/local-ratelimit.go index 7cdf5768ef..f89cfb1d04 100644 --- a/test/e2e/tests/local-ratelimit.go +++ b/test/e2e/tests/local-ratelimit.go @@ -171,7 +171,7 @@ var LocalRateLimitNoLimitRouteTest = suite.ConformanceTest{ Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { t.Run("no rate limit on this route", func(t *testing.T) { ns := "gateway-conformance-infra" - routeNN := types.NamespacedName{Name: "SecurityPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: \"ratelimit-specific-user\", Namespace: ns})", Namespace: ns} + routeNN := types.NamespacedName{Name: "http-no-ratelimit", Namespace: ns} gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) @@ -204,14 +204,14 @@ var LocalRateLimitNoLimitRouteTest = suite.ConformanceTest{ func backendTrafficPolicyMustBeAccepted( t *testing.T, client client.Client, - securityPolicyName types.NamespacedName) { + policyName types.NamespacedName) { t.Helper() waitErr := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 60*time.Second, true, func(ctx context.Context) (bool, error) { policy := &egv1a1.BackendTrafficPolicy{} - err := client.Get(ctx, securityPolicyName, policy) + err := client.Get(ctx, policyName, policy) if err != nil { - return false, fmt.Errorf("error fetching SecurityPolicy: %w", err) + return false, fmt.Errorf("error fetching BackendTrafficPolicy: %w", err) } for _, condition := range policy.Status.Conditions { From c564c8d1867835defb9a26646c1c2888c5e85a1d Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Tue, 12 Dec 2023 09:47:32 +0800 Subject: [PATCH 04/15] set always_consume_default_token_bucket false Signed-off-by: huabing zhao --- internal/xds/translator/local_ratelimit.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/xds/translator/local_ratelimit.go b/internal/xds/translator/local_ratelimit.go index e8d8db449e..e287d96c9e 100644 --- a/internal/xds/translator/local_ratelimit.go +++ b/internal/xds/translator/local_ratelimit.go @@ -16,6 +16,7 @@ import ( hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" typev3 "github.com/envoyproxy/go-control-plane/envoy/type/v3" "github.com/golang/protobuf/ptypes/duration" + "github.com/golang/protobuf/ptypes/wrappers" "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/wrapperspb" @@ -168,6 +169,13 @@ func (*localRateLimit) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) e }, }, Descriptors: descriptors, + // By setting AlwaysConsumeDefaultTokenBucket to false, the descriptors + // won't consume the default token bucket. This means that a request only + // counts towards the default token bucket if it does not match any of the + // descriptors. + AlwaysConsumeDefaultTokenBucket: &wrappers.BoolValue{ + Value: false, + }, } localRlAny, err := anypb.New(localRl) From a0385c3eb9bdac74ace3297fd3caf481d071eae6 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Tue, 12 Dec 2023 10:07:11 +0800 Subject: [PATCH 05/15] set always_consume_default_token_bucket false Signed-off-by: huabing zhao --- .../translator/testdata/out/xds-ir/local-ratelimit.routes.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml index beb161f612..5625479d34 100755 --- a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml @@ -31,6 +31,7 @@ typedPerFilterConfig: envoy.filters.http.local_ratelimit: '@type': type.googleapis.com/envoy.extensions.filters.http.local_ratelimit.v3.LocalRateLimit + alwaysConsumeDefaultTokenBucket: false descriptors: - entries: - key: first-route-ratelimit-single-rule-key-rule-0-match-0 @@ -97,6 +98,7 @@ typedPerFilterConfig: envoy.filters.http.local_ratelimit: '@type': type.googleapis.com/envoy.extensions.filters.http.local_ratelimit.v3.LocalRateLimit + alwaysConsumeDefaultTokenBucket: false descriptors: - entries: - key: second-route-ratelimit-multiple-rules-key-rule-0-match-0 @@ -137,6 +139,7 @@ typedPerFilterConfig: envoy.filters.http.local_ratelimit: '@type': type.googleapis.com/envoy.extensions.filters.http.local_ratelimit.v3.LocalRateLimit + alwaysConsumeDefaultTokenBucket: false filterEnabled: defaultValue: numerator: 100 From 0bb9b8e93d8f3fbb899762708fd32140a88e405c Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Wed, 20 Dec 2023 12:16:54 +0800 Subject: [PATCH 06/15] address comments Signed-off-by: huabing zhao --- api/v1alpha1/ratelimit_types.go | 28 +-- api/v1alpha1/zz_generated.deepcopy.go | 1 - ....envoyproxy.io_backendtrafficpolicies.yaml | 50 ++--- internal/gatewayapi/backendtrafficpolicy.go | 79 +++++-- ...telimit-default-route-level-limit.in.yaml} | 5 +- ...telimit-default-route-level-limit.out.yaml | 201 ++++++++++++++++++ ...ocal-ratelimit-invalid-limit-unit.in.yaml} | 25 ++- ...cal-ratelimit-invalid-limit-unit.out.yaml} | 6 +- ...local-ratelimit-invalid-match-type.in.yaml | 3 - ...ocal-ratelimit-invalid-match-type.out.yaml | 3 - ...nvalid-multiple-route-level-limits.in.yaml | 77 +++++++ ...alid-multiple-route-level-limits.out.yaml} | 29 ++- ...trafficpolicy-with-local-ratelimit.in.yaml | 6 +- ...rafficpolicy-with-local-ratelimit.out.yaml | 6 +- internal/xds/translator/local_ratelimit.go | 8 +- .../out/xds-ir/local-ratelimit.routes.yaml | 48 ++--- site/content/en/latest/api/extension_types.md | 7 +- 17 files changed, 448 insertions(+), 134 deletions(-) rename internal/gatewayapi/testdata/{backendtrafficpolicy-with-local-ratelimit-invalid-default.in.yaml => backendtrafficpolicy-with-local-ratelimit-default-route-level-limit.in.yaml} (94%) create mode 100755 internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-default-route-level-limit.out.yaml rename internal/gatewayapi/testdata/{backendtrafficpolicy-with-local-ratelimit-invalid-no-match.in.yaml => backendtrafficpolicy-with-local-ratelimit-invalid-limit-unit.in.yaml} (64%) rename internal/gatewayapi/testdata/{backendtrafficpolicy-with-local-ratelimit-invalid-default.out.yaml => backendtrafficpolicy-with-local-ratelimit-invalid-limit-unit.out.yaml} (98%) create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-multiple-route-level-limits.in.yaml rename internal/gatewayapi/testdata/{backendtrafficpolicy-with-local-ratelimit-invalid-no-match.out.yaml => backendtrafficpolicy-with-local-ratelimit-invalid-multiple-route-level-limits.out.yaml} (85%) diff --git a/api/v1alpha1/ratelimit_types.go b/api/v1alpha1/ratelimit_types.go index a3459a97b1..c42ab3dccd 100644 --- a/api/v1alpha1/ratelimit_types.go +++ b/api/v1alpha1/ratelimit_types.go @@ -40,8 +40,6 @@ 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,22 +54,6 @@ type GlobalRateLimit struct { // LocalRateLimit defines local rate limit configuration. type LocalRateLimit struct { - - // Envoy requires a default rate limit to be set for each route. - // 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 - - // Limit is the default rate limit for a route if no rules match. - // If a request does not match any of the rules, it is counted towards this limit. - // - // Note: Limit 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 limit is 100r/s, then - // each route has its own 100r/s rate limit bucket. - Limit RateLimitValue `json:"default"` - // Rules are a list of RateLimit selectors and limits. They're used to define // fine-grained rate limits that can be applied to specific clients using // attributes from the traffic flow. @@ -92,7 +74,15 @@ type RateLimitRule struct { // All individual select conditions must hold True for this rule // and its limit to be applied. // - // +kubebuilder:validation:MinItems=1 + // If no client selectors are specified, the rule applies to all traffic of + // the targeted Route. + // + // If the policy targets a Gateway, the rule applies to each Route of the Gateway. + // Please note that each Route has its own rate limit counters. For example, + // if a Gateway has two Routes, and the policy has a rule with limit 10rps, + // the Gateway will have 20rps limit in total. + // + // +optional // +kubebuilder:validation:MaxItems=8 ClientSelectors []RateLimitSelectCondition `json:"clientSelectors,omitempty"` // Limit holds the rate limit values. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 3a51d8a7f2..0a197a8c70 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1674,7 +1674,6 @@ func (in *LoadBalancer) DeepCopy() *LoadBalancer { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LocalRateLimit) DeepCopyInto(out *LocalRateLimit) { *out = *in - out.Limit = in.Limit if in.Rules != nil { in, out := &in.Rules, &out.Rules *out = make([]RateLimitRule, len(*in)) diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml index 64fbf26934..af5965f68e 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml @@ -168,10 +168,18 @@ spec: for them. properties: clientSelectors: - description: ClientSelectors holds the list of select + description: "ClientSelectors holds the list of select conditions to select specific clients using attributes from the traffic flow. All individual select conditions must hold True for this rule and its limit to be applied. + \n If no client selectors are specified, the rule + applies to all traffic of the targeted Route. \n If + the policy targets a Gateway, the rule applies to + each Route of the Gateway. Please note that each Route + has its own rate limit counters. For example, if a + Gateway has two Routes, and the policy has a rule + with limit 10rps, the Gateway will have 20rps limit + in total." items: description: RateLimitSelectCondition specifies the attributes within the traffic flow that can be used @@ -244,7 +252,6 @@ spec: type: object type: object maxItems: 8 - minItems: 1 type: array limit: description: Limit holds the rate limit values. This @@ -282,32 +289,6 @@ spec: local: description: Local defines local rate limit configuration. properties: - default: - description: "Limit is the default rate limit for a route - if no rules match. If a request does not match any of the - rules, it is counted towards this limit. \n Note: Limit - 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 - limit is 100r/s, then each route has its own 100r/s rate - limit bucket." - properties: - requests: - type: integer - unit: - description: RateLimitUnit specifies the intervals for - setting rate limits. Valid RateLimitUnit values are - "Second", "Minute", "Hour", and "Day". - enum: - - Second - - Minute - - Hour - - Day - type: string - required: - - requests - - unit - type: object rules: description: "Rules are a list of RateLimit selectors and limits. They're used to define fine-grained rate limits @@ -321,10 +302,18 @@ spec: for them. properties: clientSelectors: - description: ClientSelectors holds the list of select + description: "ClientSelectors holds the list of select conditions to select specific clients using attributes from the traffic flow. All individual select conditions must hold True for this rule and its limit to be applied. + \n If no client selectors are specified, the rule + applies to all traffic of the targeted Route. \n If + the policy targets a Gateway, the rule applies to + each Route of the Gateway. Please note that each Route + has its own rate limit counters. For example, if a + Gateway has two Routes, and the policy has a rule + with limit 10rps, the Gateway will have 20rps limit + in total." items: description: RateLimitSelectCondition specifies the attributes within the traffic flow that can be used @@ -397,7 +386,6 @@ spec: type: object type: object maxItems: 8 - minItems: 1 type: array limit: description: Limit holds the rate limit values. This @@ -429,8 +417,6 @@ spec: type: object maxItems: 16 type: array - required: - - default type: object type: description: Type decides the scope for the RateLimits. Valid diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index 823183d4ce..0b2a964ce7 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -8,6 +8,7 @@ package gatewayapi import ( "errors" "fmt" + "math" "net" "sort" "strings" @@ -343,13 +344,46 @@ func (t *Translator) buildLocalRateLimit(policy *egv1a1.BackendTrafficPolicy) *i } local := policy.Spec.RateLimit.Local - defaultLimitUnit := ratelimitUnitToDuration(local.Limit.Unit) + + // Envoy local rateLimit requires a default limit to be set for a route. + // EG uses the first rule without clientSelectors as the default route-level + // limit. If no such rule is found, EG uses a default limit of uint32 max. + var defaultLimit *ir.RateLimitValue + for _, rule := range local.Rules { + if rule.ClientSelectors == nil || len(rule.ClientSelectors) == 0 { + if defaultLimit != nil { + message := "Local rateLimit can not have more than one rule without clientSelectors." + status.SetBackendTrafficPolicyCondition(policy, + gwv1a2.PolicyConditionAccepted, + metav1.ConditionFalse, + gwv1a2.PolicyReasonInvalid, + message, + ) + return nil + } + defaultLimit = &ir.RateLimitValue{ + Requests: rule.Limit.Requests, + Unit: ir.RateLimitUnit(rule.Limit.Unit), + } + } + } + // If no rule without clientSelectors is found, use uint32 max as the default + // limit, which effectively make the default limit unlimited. + if defaultLimit == nil { + defaultLimit = &ir.RateLimitValue{ + Requests: math.MaxUint32, + Unit: ir.RateLimitUnit(egv1a1.RateLimitUnitSecond), + } + } + + // Validate that the rule limit unit is 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 + defaultLimitUnit := ratelimitUnitToDuration(egv1a1.RateLimitUnit(defaultLimit.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, @@ -360,20 +394,17 @@ func (t *Translator) buildLocalRateLimit(policy *egv1a1.BackendTrafficPolicy) *i } } - rateLimit := &ir.RateLimit{ - Local: &ir.LocalRateLimit{ - Default: ir.RateLimitValue{ - Requests: local.Limit.Requests, - Unit: ir.RateLimitUnit(local.Limit.Unit), - }, - Rules: make([]*ir.RateLimitRule, len(local.Rules)), - }, - } - - irRules := rateLimit.Local.Rules var err error - for i, rule := range local.Rules { - irRules[i], err = buildRateLimitRule(rule) + var irRule *ir.RateLimitRule + var irRules = make([]*ir.RateLimitRule, 0) + for _, rule := range local.Rules { + // We don't process the rule without clientSelectors here because it's + // previously used as the default route-level limit. + if len(rule.ClientSelectors) == 0 { + continue + } + + irRule, err = buildRateLimitRule(rule) if err != nil { status.SetBackendTrafficPolicyCondition(policy, gwv1a2.PolicyConditionAccepted, @@ -383,7 +414,7 @@ func (t *Translator) buildLocalRateLimit(policy *egv1a1.BackendTrafficPolicy) *i ) return nil } - if irRules[i].CIDRMatch != nil && irRules[i].CIDRMatch.Distinct { + if irRule.CIDRMatch != nil && irRule.CIDRMatch.Distinct { message := "Local rateLimit does not support distinct CIDRMatch." status.SetBackendTrafficPolicyCondition(policy, gwv1a2.PolicyConditionAccepted, @@ -393,7 +424,7 @@ func (t *Translator) buildLocalRateLimit(policy *egv1a1.BackendTrafficPolicy) *i ) return nil } - for _, match := range irRules[i].HeaderMatches { + for _, match := range irRule.HeaderMatches { if match.Distinct { message := "Local rateLimit does not support distinct HeaderMatch." status.SetBackendTrafficPolicyCondition(policy, @@ -405,8 +436,15 @@ func (t *Translator) buildLocalRateLimit(policy *egv1a1.BackendTrafficPolicy) *i return nil } } + irRules = append(irRules, irRule) } + rateLimit := &ir.RateLimit{ + Local: &ir.LocalRateLimit{ + Default: *defaultLimit, + Rules: irRules, + }, + } return rateLimit } @@ -466,10 +504,7 @@ 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( diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-default-route-level-limit.in.yaml similarity index 94% rename from internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.in.yaml rename to internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-default-route-level-limit.in.yaml index b2ee456765..c1136403c3 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.in.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-default-route-level-limit.in.yaml @@ -48,10 +48,7 @@ backendTrafficPolicies: rateLimit: type: Local local: - default: - requests: 1000 - unit: Hour - rules: + rules: # No Rule without selector is specified, so int32 max is used as default. - clientSelectors: - headers: - name: x-user-id diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-default-route-level-limit.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-default-route-level-limit.out.yaml new file mode 100755 index 0000000000..d536e7ca2b --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-default-route-level-limit.out.yaml @@ -0,0 +1,201 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway + namespace: envoy-gateway + spec: + rateLimit: + local: + 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 + type: Local + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + status: + conditions: + - lastTransitionTime: null + message: BackendTrafficPolicy has been accepted. + reason: Accepted + status: "True" + type: Accepted +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: "" + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-1 +xdsIR: + envoy-gateway/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: gateway.envoyproxy.io + name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io + pathMatch: + distinct: false + name: "" + prefix: / + rateLimit: + local: + default: + requests: 4294967295 + unit: Second + rules: + - headerMatches: + - distinct: false + exact: one + name: x-user-id + - distinct: false + exact: foo + name: x-org-id + limit: + requests: 10 + unit: Hour + - cidrMatch: + cidr: 192.168.0.0/16 + distinct: false + ipv6: false + maskLen: 16 + headerMatches: + - distinct: false + exact: two + name: x-user-id + - distinct: false + exact: bar + name: x-org-id + limit: + requests: 10 + unit: Minute diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-limit-unit.in.yaml similarity index 64% rename from internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.in.yaml rename to internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-limit-unit.in.yaml index 28b78177c8..c1d34e7048 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.in.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-limit-unit.in.yaml @@ -48,10 +48,27 @@ backendTrafficPolicies: rateLimit: type: Local local: - default: - requests: 100 - unit: Second rules: - limit: + requests: 1000 + unit: Hour + - clientSelectors: + - headers: + - name: x-user-id + value: one + - name: x-org-id + value: foo + limit: requests: 10 - unit: Minute + 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 # Local rateLimit rule limit unit is not a multiple of the default limit unit. diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-limit-unit.out.yaml similarity index 98% rename from internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.out.yaml rename to internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-limit-unit.out.yaml index dccc484bf9..dd822c40e7 100755 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-default.out.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-limit-unit.out.yaml @@ -8,10 +8,10 @@ backendTrafficPolicies: spec: rateLimit: local: - default: - requests: 1000 - unit: Hour rules: + - limit: + requests: 1000 + unit: Hour - clientSelectors: - headers: - name: x-user-id diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.in.yaml index 783dbb557b..e6a595b157 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.in.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.in.yaml @@ -48,9 +48,6 @@ backendTrafficPolicies: rateLimit: type: Local local: - default: - requests: 100 - unit: Second rules: - clientSelectors: - headers: diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.out.yaml index 3f6b9030c9..e18999b02d 100755 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.out.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.out.yaml @@ -8,9 +8,6 @@ backendTrafficPolicies: spec: rateLimit: local: - default: - requests: 100 - unit: Second rules: - clientSelectors: - headers: diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-multiple-route-level-limits.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-multiple-route-level-limits.in.yaml new file mode 100644 index 0000000000..9a2f3b9424 --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-multiple-route-level-limits.in.yaml @@ -0,0 +1,77 @@ +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: + rules: + - limit: # There are two Rule without selector, so this is invalid. + requests: 10 + unit: Minute + - limit: + requests: 20 + unit: Minute + - 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 diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-multiple-route-level-limits.out.yaml similarity index 85% rename from internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.out.yaml rename to internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-multiple-route-level-limits.out.yaml index 69be65457a..561f7fb2af 100755 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-no-match.out.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-multiple-route-level-limits.out.yaml @@ -8,13 +8,33 @@ backendTrafficPolicies: spec: rateLimit: local: - default: - requests: 100 - unit: Second rules: - limit: requests: 10 unit: Minute + - limit: + requests: 20 + unit: Minute + - 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 type: Local targetRef: group: gateway.networking.k8s.io @@ -24,8 +44,7 @@ backendTrafficPolicies: status: conditions: - lastTransitionTime: null - message: Unable to translate rateLimit. At least one clientSelector must be - specified. + message: Local rateLimit can not have more than one rule without clientSelectors. reason: Invalid status: "False" type: Accepted diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml index d906972101..7f9d36ab48 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.in.yaml @@ -48,10 +48,10 @@ backendTrafficPolicies: rateLimit: type: Local local: - default: - requests: 10 - unit: Minute rules: + - limit: # The Rule without selector is used as default + requests: 10 + unit: Minute - clientSelectors: - headers: - name: x-user-id diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml index 82339b6e8e..ad7814b6ef 100755 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml @@ -8,10 +8,10 @@ backendTrafficPolicies: spec: rateLimit: local: - default: - requests: 10 - unit: Minute rules: + - limit: + requests: 10 + unit: Minute - clientSelectors: - headers: - name: x-user-id diff --git a/internal/xds/translator/local_ratelimit.go b/internal/xds/translator/local_ratelimit.go index e287d96c9e..f54071d353 100644 --- a/internal/xds/translator/local_ratelimit.go +++ b/internal/xds/translator/local_ratelimit.go @@ -132,7 +132,7 @@ func (*localRateLimit) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) e local := irRoute.RateLimit.Local - rateLimits, descriptors, err := buildRouteLocalRateLimits(irRoute.Name, local) + rateLimits, descriptors, err := buildRouteLocalRateLimits(local) if err != nil { return err } @@ -191,7 +191,7 @@ func (*localRateLimit) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) e return nil } -func buildRouteLocalRateLimits(descriptorPrefix string, local *ir.LocalRateLimit) ( +func buildRouteLocalRateLimits(local *ir.LocalRateLimit) ( []*routev3.RateLimit, []*rlv3.LocalRateLimitDescriptor, error) { var rateLimits []*routev3.RateLimit var descriptors []*rlv3.LocalRateLimitDescriptor @@ -212,8 +212,8 @@ func buildRouteLocalRateLimits(descriptorPrefix string, local *ir.LocalRateLimit } // Setup HeaderValueMatch actions - descriptorKey := getRateLimitDescriptorKey(descriptorPrefix, rIdx, mIdx) - descriptorVal := getRateLimitDescriptorValue(descriptorPrefix, rIdx, mIdx) + descriptorKey := getRouteRuleDescriptor(rIdx, mIdx) + descriptorVal := getRouteRuleDescriptor(rIdx, mIdx) headerMatcher := &routev3.HeaderMatcher{ Name: match.Name, HeaderMatchSpecifier: &routev3.HeaderMatcher_StringMatch{ diff --git a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml index 5625479d34..3f28acd5a1 100755 --- a/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/local-ratelimit.routes.yaml @@ -13,16 +13,16 @@ rateLimits: - actions: - headerValueMatch: - descriptorKey: first-route-ratelimit-single-rule-key-rule-0-match-0 - descriptorValue: first-route-ratelimit-single-rule-value-rule-0-match-0 + descriptorKey: rule-0-match-0 + descriptorValue: rule-0-match-0 expectMatch: true headers: - name: x-user-id stringMatch: exact: one - headerValueMatch: - descriptorKey: first-route-ratelimit-single-rule-key-rule-0-match-1 - descriptorValue: first-route-ratelimit-single-rule-value-rule-0-match-1 + descriptorKey: rule-0-match-1 + descriptorValue: rule-0-match-1 expectMatch: true headers: - name: x-org-id @@ -34,10 +34,10 @@ alwaysConsumeDefaultTokenBucket: false descriptors: - entries: - - key: first-route-ratelimit-single-rule-key-rule-0-match-0 - value: first-route-ratelimit-single-rule-value-rule-0-match-0 - - key: first-route-ratelimit-single-rule-key-rule-0-match-1 - value: first-route-ratelimit-single-rule-value-rule-0-match-1 + - key: rule-0-match-0 + value: rule-0-match-0 + - key: rule-0-match-1 + value: rule-0-match-1 tokenBucket: fillInterval: 3600s maxTokens: 10 @@ -61,16 +61,16 @@ rateLimits: - actions: - headerValueMatch: - descriptorKey: second-route-ratelimit-multiple-rules-key-rule-0-match-0 - descriptorValue: second-route-ratelimit-multiple-rules-value-rule-0-match-0 + descriptorKey: rule-0-match-0 + descriptorValue: rule-0-match-0 expectMatch: true headers: - name: x-user-id stringMatch: exact: one - headerValueMatch: - descriptorKey: second-route-ratelimit-multiple-rules-key-rule-0-match-1 - descriptorValue: second-route-ratelimit-multiple-rules-value-rule-0-match-1 + descriptorKey: rule-0-match-1 + descriptorValue: rule-0-match-1 expectMatch: true headers: - name: x-org-id @@ -78,16 +78,16 @@ exact: foo - actions: - headerValueMatch: - descriptorKey: second-route-ratelimit-multiple-rules-key-rule-1-match-0 - descriptorValue: second-route-ratelimit-multiple-rules-value-rule-1-match-0 + descriptorKey: rule-1-match-0 + descriptorValue: rule-1-match-0 expectMatch: true headers: - name: x-user-id stringMatch: exact: two - headerValueMatch: - descriptorKey: second-route-ratelimit-multiple-rules-key-rule-1-match-1 - descriptorValue: second-route-ratelimit-multiple-rules-value-rule-1-match-1 + descriptorKey: rule-1-match-1 + descriptorValue: rule-1-match-1 expectMatch: true headers: - name: x-org-id @@ -101,19 +101,19 @@ alwaysConsumeDefaultTokenBucket: false descriptors: - entries: - - key: second-route-ratelimit-multiple-rules-key-rule-0-match-0 - value: second-route-ratelimit-multiple-rules-value-rule-0-match-0 - - key: second-route-ratelimit-multiple-rules-key-rule-0-match-1 - value: second-route-ratelimit-multiple-rules-value-rule-0-match-1 + - key: rule-0-match-0 + value: rule-0-match-0 + - key: rule-0-match-1 + value: rule-0-match-1 tokenBucket: fillInterval: 3600s maxTokens: 10 tokensPerFill: 10 - entries: - - key: second-route-ratelimit-multiple-rules-key-rule-1-match-0 - value: second-route-ratelimit-multiple-rules-value-rule-1-match-0 - - key: second-route-ratelimit-multiple-rules-key-rule-1-match-1 - value: second-route-ratelimit-multiple-rules-value-rule-1-match-1 + - key: rule-1-match-0 + value: rule-1-match-0 + - key: rule-1-match-1 + value: rule-1-match-1 - key: masked_remote_address value: 192.168.0.0/16 tokenBucket: diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index e5de6da8ce..2d3183131e 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1129,8 +1129,6 @@ _Appears in:_ | Field | Description | | --- | --- | -| `default` _[RateLimitValue](#ratelimitvalue)_ | Limit is the default rate limit for a route if no rules match. If a request does not match any of the rules, it is counted towards this limit. - Note: Limit 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 limit is 100r/s, then each route has its own 100r/s rate limit bucket. | | `rules` _[RateLimitRule](#ratelimitrule) array_ | Rules are a list of RateLimit selectors and limits. They're used to define fine-grained rate limits that can be applied to specific clients using attributes from the traffic flow. Orders matters here as the rules are processed sequentially. The first rule that matches the request is applied. | @@ -1537,7 +1535,9 @@ _Appears in:_ | Field | Description | | --- | --- | -| `clientSelectors` _[RateLimitSelectCondition](#ratelimitselectcondition) array_ | ClientSelectors holds the list of select conditions to select specific clients using attributes from the traffic flow. All individual select conditions must hold True for this rule and its limit to be applied. | +| `clientSelectors` _[RateLimitSelectCondition](#ratelimitselectcondition) array_ | ClientSelectors holds the list of select conditions to select 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 no client selectors are specified, the rule applies to all traffic of the targeted Route. + If the policy targets a Gateway, the rule applies to each Route of the Gateway. Please note that each Route has its own rate limit counters. For example, if a Gateway has two Routes, and the policy has a rule with limit 10rps, the Gateway will have 20rps limit in total. | | `limit` _[RateLimitValue](#ratelimitvalue)_ | Limit holds the rate limit values. This limit is applied for traffic flows when the selectors compute to True, causing the request to be counted towards the limit. The limit is enforced and the request is ratelimited, i.e. a response with 429 HTTP status code is sent back to the client when the selected requests have reached the limit. | @@ -1601,7 +1601,6 @@ _Appears in:_ RateLimitValue defines the limits for rate limiting. _Appears in:_ -- [LocalRateLimit](#localratelimit) - [RateLimitRule](#ratelimitrule) | Field | Description | From b5327112164d5bcb8c9b29482af97cbef2de738b Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Wed, 20 Dec 2023 14:17:57 +0800 Subject: [PATCH 07/15] fix e2e test Signed-off-by: huabing zhao --- test/e2e/testdata/local-ratelimit.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/e2e/testdata/local-ratelimit.yaml b/test/e2e/testdata/local-ratelimit.yaml index 71268732f8..fa2b05342a 100644 --- a/test/e2e/testdata/local-ratelimit.yaml +++ b/test/e2e/testdata/local-ratelimit.yaml @@ -12,10 +12,10 @@ spec: rateLimit: type: Local local: - default: - requests: 10 - unit: Hour rules: + - limit: + requests: 10 + unit: Hour - clientSelectors: - headers: - name: x-user-id @@ -38,9 +38,9 @@ spec: rateLimit: type: Local local: - default: - requests: 3 - unit: Hour + - limit: + requests: 10 + unit: Hour --- apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute From 37ce393a2084e51d0b10a622ba3aa38b32e41022 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Wed, 20 Dec 2023 15:27:01 +0800 Subject: [PATCH 08/15] fix lint Signed-off-by: huabing zhao --- test/e2e/testdata/local-ratelimit.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/testdata/local-ratelimit.yaml b/test/e2e/testdata/local-ratelimit.yaml index fa2b05342a..22d84580c4 100644 --- a/test/e2e/testdata/local-ratelimit.yaml +++ b/test/e2e/testdata/local-ratelimit.yaml @@ -38,9 +38,9 @@ spec: rateLimit: type: Local local: - - limit: - requests: 10 - unit: Hour + - limit: + requests: 10 + unit: Hour --- apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute From a8da2c7ac3f5ddce3b2348d8fcc6de54de25777c Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Wed, 20 Dec 2023 16:20:38 +0800 Subject: [PATCH 09/15] fix e2e test Signed-off-by: huabing zhao --- test/e2e/testdata/local-ratelimit.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/e2e/testdata/local-ratelimit.yaml b/test/e2e/testdata/local-ratelimit.yaml index 22d84580c4..335a4a133d 100644 --- a/test/e2e/testdata/local-ratelimit.yaml +++ b/test/e2e/testdata/local-ratelimit.yaml @@ -38,9 +38,10 @@ spec: rateLimit: type: Local local: - - limit: - requests: 10 - unit: Hour + rules: + - limit: + requests: 10 + unit: Hour --- apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute From 7809964b7c0a4950b3930f6c7e63cb34f64eeec1 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Wed, 20 Dec 2023 16:37:08 +0800 Subject: [PATCH 10/15] fix e2e test Signed-off-by: huabing zhao --- test/e2e/testdata/local-ratelimit.yaml | 2 +- test/e2e/tests/local-ratelimit.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/testdata/local-ratelimit.yaml b/test/e2e/testdata/local-ratelimit.yaml index 335a4a133d..e0e5513a03 100644 --- a/test/e2e/testdata/local-ratelimit.yaml +++ b/test/e2e/testdata/local-ratelimit.yaml @@ -40,7 +40,7 @@ spec: local: rules: - limit: - requests: 10 + requests: 3 unit: Hour --- apiVersion: gateway.networking.k8s.io/v1 diff --git a/test/e2e/tests/local-ratelimit.go b/test/e2e/tests/local-ratelimit.go index f89cfb1d04..28e3caa703 100644 --- a/test/e2e/tests/local-ratelimit.go +++ b/test/e2e/tests/local-ratelimit.go @@ -156,7 +156,7 @@ var LocalRateLimitAllTrafficTest = suite.ConformanceTest{ t.Errorf("fail to get expected response at first three request: %v", err) } - // this request should be limited because the user is john and the limit is 3 + // this request should be limited because the limit is 3 if err := GotExactExpectedResponse(t, 1, suite.RoundTripper, expectLimitReq, expectLimitResp); err != nil { t.Errorf("fail to get expected response at last fourth request: %v", err) } From e24f056f16f76822b116a455af563185c993296a Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Wed, 20 Dec 2023 18:57:11 +0800 Subject: [PATCH 11/15] add comments to explain global rate limit rules Signed-off-by: huabing zhao --- api/v1alpha1/ratelimit_types.go | 23 ++++++++----------- ....envoyproxy.io_backendtrafficpolicies.yaml | 19 ++++++++------- site/content/en/latest/api/extension_types.md | 2 +- 3 files changed, 19 insertions(+), 25 deletions(-) diff --git a/api/v1alpha1/ratelimit_types.go b/api/v1alpha1/ratelimit_types.go index c42ab3dccd..17c8ffc4f5 100644 --- a/api/v1alpha1/ratelimit_types.go +++ b/api/v1alpha1/ratelimit_types.go @@ -40,26 +40,21 @@ const ( // GlobalRateLimit defines global rate limit configuration. type GlobalRateLimit struct { - // 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 - // rules get selected, each of their associated - // limits get applied, so a single traffic request - // might increase the rate limit counters for multiple - // rules if selected. - // + // Rules are a list of RateLimit selectors and limits. Each rule and its + // associated limit is applied in a mutually exclusive way. If a request + // matches multiple rules, each of their associated limits get applied, so a + // single request might increase the rate limit counters for multiple rules + // if selected. The rate limit service will return a logical OR of the individual + // rate limit decisions of all matching rules. For example, if a request + // matches two rules, one rate limited and one not, the final decision will be + // to rate limit the request. // +kubebuilder:validation:MaxItems=16 Rules []RateLimitRule `json:"rules"` } // LocalRateLimit defines local rate limit configuration. type LocalRateLimit struct { - // Rules are a list of RateLimit selectors and limits. They're used to define - // fine-grained rate limits that can be applied to specific clients using - // attributes from the traffic flow. - // - // Orders matters here as the rules are processed sequentially. - // The first rule that matches the request is applied. + // Rules are a list of RateLimit selectors and limits. // // +optional // +kubebuilder:validation:MaxItems=16 diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml index af5965f68e..7713b0995a 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml @@ -158,10 +158,14 @@ spec: rules: description: 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 rules get selected, each - of their associated limits get applied, so a single traffic - request might increase the rate limit counters for multiple - rules if selected. + exclusive way. If a request matches multiple rules, each + of their associated limits get applied, so a single request + might increase the rate limit counters for multiple rules + if selected. The rate limit service will return a logical + OR of the individual rate limit decisions of all matching + rules. For example, if a request matches two rules, one + rate limited and one not, the final decision will be to + rate limit the request. items: description: RateLimitRule defines the semantics for matching attributes from the incoming requests, and setting limits @@ -290,12 +294,7 @@ spec: description: Local defines local rate limit configuration. properties: rules: - description: "Rules are a list of RateLimit selectors and - limits. They're used to define fine-grained rate limits - that can be applied to specific clients using attributes - from the traffic flow. \n Orders matters here as the rules - are processed sequentially. The first rule that matches - the request is applied." + description: Rules are a list of RateLimit selectors and limits. items: description: RateLimitRule defines the semantics for matching attributes from the incoming requests, and setting limits diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 2d3183131e..840b2fe9a0 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1129,7 +1129,7 @@ _Appears in:_ | Field | Description | | --- | --- | -| `rules` _[RateLimitRule](#ratelimitrule) array_ | Rules are a list of RateLimit selectors and limits. They're used to define fine-grained rate limits that can be applied to specific clients using attributes from the traffic flow. +| `rules` _[RateLimitRule](#ratelimitrule) array_ | Rules are a list of RateLimit selectors and limits. Orders matters here as the rules are processed sequentially. The first rule that matches the request is applied. | From bac22c923238a2ea99bf9b529766f5202e1ee812 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Wed, 20 Dec 2023 19:01:10 +0800 Subject: [PATCH 12/15] fix gen Signed-off-by: huabing zhao --- site/content/en/latest/api/extension_types.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 840b2fe9a0..47ee80c485 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -832,7 +832,7 @@ _Appears in:_ | Field | Description | | --- | --- | -| `rules` _[RateLimitRule](#ratelimitrule) array_ | 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 rules get selected, each of their associated limits get applied, so a single traffic request might increase the rate limit counters for multiple rules if selected. | +| `rules` _[RateLimitRule](#ratelimitrule) array_ | Rules are a list of RateLimit selectors and limits. Each rule and its associated limit is applied in a mutually exclusive way. If a request matches multiple rules, each of their associated limits get applied, so a single request might increase the rate limit counters for multiple rules if selected. The rate limit service will return a logical OR of the individual rate limit decisions of all matching rules. For example, if a request matches two rules, one rate limited and one not, the final decision will be to rate limit the request. | #### GroupVersionKind @@ -1129,8 +1129,7 @@ _Appears in:_ | Field | Description | | --- | --- | -| `rules` _[RateLimitRule](#ratelimitrule) array_ | Rules are a list of RateLimit selectors and limits. - Orders matters here as the rules are processed sequentially. The first rule that matches the request is applied. | +| `rules` _[RateLimitRule](#ratelimitrule) array_ | Rules are a list of RateLimit selectors and limits. | #### LogLevel From 07c8397ef8f106ee06129ea2efb5bfc6bd104a1b Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Wed, 20 Dec 2023 19:09:15 +0800 Subject: [PATCH 13/15] add comments to explain local rate limit rules Signed-off-by: huabing zhao --- api/v1alpha1/ratelimit_types.go | 6 +++++- .../gateway.envoyproxy.io_backendtrafficpolicies.yaml | 4 ++++ site/content/en/latest/api/extension_types.md | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/ratelimit_types.go b/api/v1alpha1/ratelimit_types.go index 17c8ffc4f5..fe29da450b 100644 --- a/api/v1alpha1/ratelimit_types.go +++ b/api/v1alpha1/ratelimit_types.go @@ -48,13 +48,17 @@ type GlobalRateLimit struct { // rate limit decisions of all matching rules. For example, if a request // matches two rules, one rate limited and one not, the final decision will be // to rate limit the request. + // // +kubebuilder:validation:MaxItems=16 Rules []RateLimitRule `json:"rules"` } // LocalRateLimit defines local rate limit configuration. type LocalRateLimit struct { - // Rules are a list of RateLimit selectors and limits. + // Rules are a list of RateLimit selectors and limits. If a request matches + // multiple rules, the strictest limit is applied. For example, if a request + // matches two rules, one with 10rps and one with 20rps, the final limit will + // be based on the rule with 10rps. // // +optional // +kubebuilder:validation:MaxItems=16 diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml index 7713b0995a..7c088ffe07 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml @@ -295,6 +295,10 @@ spec: properties: rules: description: Rules are a list of RateLimit selectors and limits. + If a request matches multiple rules, the strictest limit + is applied. For example, if a request matches two rules, + one with 10rps and one with 20rps, the final limit will + be based on the rule with 10rps. items: description: RateLimitRule defines the semantics for matching attributes from the incoming requests, and setting limits diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 47ee80c485..43ce633ecb 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1129,7 +1129,7 @@ _Appears in:_ | Field | Description | | --- | --- | -| `rules` _[RateLimitRule](#ratelimitrule) array_ | Rules are a list of RateLimit selectors and limits. | +| `rules` _[RateLimitRule](#ratelimitrule) array_ | Rules are a list of RateLimit selectors and limits. If a request matches multiple rules, the strictest limit is applied. For example, if a request matches two rules, one with 10rps and one with 20rps, the final limit will be based on the rule with 10rps. | #### LogLevel From 4e229ba8cbe0520bfc46ffb51421c007d16a5f59 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Thu, 21 Dec 2023 08:40:45 +0800 Subject: [PATCH 14/15] address comments Signed-off-by: huabing zhao --- api/v1alpha1/ratelimit_types.go | 2 +- .../gateway.envoyproxy.io_backendtrafficpolicies.yaml | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/v1alpha1/ratelimit_types.go b/api/v1alpha1/ratelimit_types.go index fe29da450b..6e183b7ccd 100644 --- a/api/v1alpha1/ratelimit_types.go +++ b/api/v1alpha1/ratelimit_types.go @@ -79,7 +79,7 @@ type RateLimitRule struct { // If the policy targets a Gateway, the rule applies to each Route of the Gateway. // Please note that each Route has its own rate limit counters. For example, // if a Gateway has two Routes, and the policy has a rule with limit 10rps, - // the Gateway will have 20rps limit in total. + // each Route will have its own 10rps limit. // // +optional // +kubebuilder:validation:MaxItems=8 diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml index 7c088ffe07..c91768b63b 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml @@ -182,8 +182,8 @@ spec: each Route of the Gateway. Please note that each Route has its own rate limit counters. For example, if a Gateway has two Routes, and the policy has a rule - with limit 10rps, the Gateway will have 20rps limit - in total." + with limit 10rps, each Route will have its own 10rps + limit." items: description: RateLimitSelectCondition specifies the attributes within the traffic flow that can be used @@ -315,8 +315,8 @@ spec: each Route of the Gateway. Please note that each Route has its own rate limit counters. For example, if a Gateway has two Routes, and the policy has a rule - with limit 10rps, the Gateway will have 20rps limit - in total." + with limit 10rps, each Route will have its own 10rps + limit." items: description: RateLimitSelectCondition specifies the attributes within the traffic flow that can be used From b1cdeb1118dcffecf6b3c41fa67f928cf4331433 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Thu, 21 Dec 2023 15:45:55 +0800 Subject: [PATCH 15/15] fix gen Signed-off-by: huabing zhao --- site/content/en/latest/api/extension_types.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index a233c910b0..8a4f001ce6 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1595,7 +1595,7 @@ _Appears in:_ | --- | --- | | `clientSelectors` _[RateLimitSelectCondition](#ratelimitselectcondition) array_ | ClientSelectors holds the list of select conditions to select 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 no client selectors are specified, the rule applies to all traffic of the targeted Route. - If the policy targets a Gateway, the rule applies to each Route of the Gateway. Please note that each Route has its own rate limit counters. For example, if a Gateway has two Routes, and the policy has a rule with limit 10rps, the Gateway will have 20rps limit in total. | + If the policy targets a Gateway, the rule applies to each Route of the Gateway. Please note that each Route has its own rate limit counters. For example, if a Gateway has two Routes, and the policy has a rule with limit 10rps, each Route will have its own 10rps limit. | | `limit` _[RateLimitValue](#ratelimitvalue)_ | Limit holds the rate limit values. This limit is applied for traffic flows when the selectors compute to True, causing the request to be counted towards the limit. The limit is enforced and the request is ratelimited, i.e. a response with 429 HTTP status code is sent back to the client when the selected requests have reached the limit. |