From 5c645de2c35d718dff4e60f4b889a9fed001f242 Mon Sep 17 00:00:00 2001 From: Rudrakh Panigrahi Date: Fri, 20 Sep 2024 01:01:15 +0530 Subject: [PATCH] feat: support inverting header matches for rate limit Signed-off-by: Rudrakh Panigrahi --- internal/gatewayapi/backendtrafficpolicy.go | 10 +- ...-ratelimit-invalid-distinct-invert.in.yaml | 53 ++++++ ...ratelimit-invalid-distinct-invert.out.yaml | 164 ++++++++++++++++++ ...ackendtrafficpolicy-with-ratelimit.in.yaml | 3 + ...ckendtrafficpolicy-with-ratelimit.out.yaml | 7 + internal/ir/xds.go | 6 + internal/ir/xds_test.go | 19 +- internal/ir/zz_generated.deepcopy.go | 5 + internal/xds/translator/local_ratelimit.go | 6 +- internal/xds/translator/ratelimit.go | 6 +- .../testdata/in/xds-ir/ratelimit.yaml | 21 +++ .../out/xds-ir/ratelimit.clusters.yaml | 17 ++ .../out/xds-ir/ratelimit.endpoints.yaml | 12 ++ .../testdata/out/xds-ir/ratelimit.routes.yaml | 20 +++ 14 files changed, 344 insertions(+), 5 deletions(-) create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.in.yaml create mode 100644 internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.out.yaml diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index d71d49f32ca..12453ea1826 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -710,8 +710,9 @@ func buildRateLimitRule(rule egv1a1.RateLimitRule) (*ir.RateLimitRule, error) { fallthrough case *header.Type == egv1a1.HeaderMatchExact && header.Value != nil: m := &ir.StringMatch{ - Name: header.Name, - Exact: header.Value, + Name: header.Name, + Exact: header.Value, + Invert: header.Invert, } irRule.HeaderMatches = append(irRule.HeaderMatches, m) case *header.Type == egv1a1.HeaderMatchRegularExpression && header.Value != nil: @@ -721,9 +722,14 @@ func buildRateLimitRule(rule egv1a1.RateLimitRule) (*ir.RateLimitRule, error) { m := &ir.StringMatch{ Name: header.Name, SafeRegex: header.Value, + Invert: header.Invert, } irRule.HeaderMatches = append(irRule.HeaderMatches, m) case *header.Type == egv1a1.HeaderMatchDistinct && header.Value == nil: + if header.Invert != nil && *header.Invert { + return nil, fmt.Errorf("unable to translate rateLimit." + + "Invert is not applicable for distinct header match type") + } m := &ir.StringMatch{ Name: header.Name, Distinct: true, diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.in.yaml new file mode 100644 index 00000000000..a1ed0f512cc --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.in.yaml @@ -0,0 +1,53 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway + 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 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway + sectionName: http + rules: + - backendRefs: + - name: service + 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 + rateLimit: + type: Global + global: + rules: + - clientSelectors: + - headers: + - name: x-org-id + type: Distinct + invert: true + limit: + requests: 10 + unit: Hour diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.out.yaml new file mode 100644 index 00000000000..4ea1623c867 --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-distinct-invert.out.yaml @@ -0,0 +1,164 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway + namespace: envoy-gateway + spec: + rateLimit: + global: + rules: + - clientSelectors: + - headers: + - invert: true + name: x-org-id + type: Distinct + limit: + requests: 10 + unit: Hour + type: Global + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: 'RateLimit: unable to translate rateLimit.Invert is not applicable + for distinct header match type.' + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway + 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 + namespace: default + spec: + parentRefs: + - name: gateway + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service + port: 8080 + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Service default/service not found + reason: BackendNotFound + status: "False" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway +xdsIR: + envoy-gateway/gateway: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: true + metadata: + kind: Gateway + name: gateway + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - destination: + name: grpcroute/default/grpcroute/rule/0 + settings: + - weight: 1 + directResponse: + statusCode: 500 + hostname: '*' + isHTTP2: true + metadata: + kind: GRPCRoute + name: grpcroute + namespace: default + name: grpcroute/default/grpcroute/rule/0/match/-1/* diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.in.yaml index e4f1fc10c64..f536d9a20bc 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.in.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.in.yaml @@ -83,6 +83,9 @@ backendTrafficPolicies: value: one - name: x-org-id type: Distinct + - name: x-org-id + value: admin + invert: true limit: requests: 10 unit: Hour diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.out.yaml index 2f7a2d5e8f9..07fa997e109 100644 --- a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.out.yaml +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.out.yaml @@ -52,6 +52,9 @@ backendTrafficPolicies: value: one - name: x-org-id type: Distinct + - invert: true + name: x-org-id + value: admin limit: requests: 10 unit: Hour @@ -306,6 +309,10 @@ xdsIR: name: x-user-id - distinct: true name: x-org-id + - distinct: false + exact: admin + invert: true + name: x-org-id limit: requests: 10 unit: Hour diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 9750680f387..40e7da917f3 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -47,6 +47,7 @@ var ( ErrDestEndpointUDSPortInvalid = errors.New("field Port must not be specified for Unix Domain Socket address") ErrDestEndpointUDSHostInvalid = errors.New("field Host must not be specified for Unix Domain Socket address") ErrStringMatchConditionInvalid = errors.New("only one of the Exact, Prefix, SafeRegex or Distinct fields must be set") + ErrStringMatchInvertDistinctInvalid = errors.New("only one of the Invert or Distinct fields can be set") ErrStringMatchNameIsEmpty = errors.New("field Name must be specified") ErrDirectResponseStatusInvalid = errors.New("only HTTP status codes 100 - 599 are supported for DirectResponse") ErrRedirectUnsupportedStatus = errors.New("only HTTP status codes 301 and 302 are supported for redirect filters") @@ -1443,6 +1444,8 @@ type StringMatch struct { // Distinct match condition. // Used to match any and all possible unique values encountered within the Name field. Distinct bool `json:"distinct" yaml:"distinct"` + // Invert inverts the final match decision + Invert *bool `json:"invert,omitempty" yaml:"invert,omitempty"` } // Validate the fields within the StringMatch structure @@ -1465,6 +1468,9 @@ func (s StringMatch) Validate() error { if s.Name == "" { errs = errors.Join(errs, ErrStringMatchNameIsEmpty) } + if s.Invert != nil && *s.Invert { + errs = errors.Join(errs, ErrStringMatchInvertDistinctInvalid) + } matchCount++ } diff --git a/internal/ir/xds_test.go b/internal/ir/xds_test.go index 14b624f22f3..5ff9a8736ef 100644 --- a/internal/ir/xds_test.go +++ b/internal/ir/xds_test.go @@ -1182,12 +1182,20 @@ func TestValidateStringMatch(t *testing.T) { want error }{ { - name: "happy", + name: "happy exact", input: StringMatch{ Exact: ptr.To("example"), }, want: nil, }, + { + name: "happy distinct", + input: StringMatch{ + Distinct: true, + Name: "example", + }, + want: nil, + }, { name: "no fields set", input: StringMatch{}, @@ -1202,6 +1210,15 @@ func TestValidateStringMatch(t *testing.T) { }, want: ErrStringMatchConditionInvalid, }, + { + name: "both invert and distinct fields are set", + input: StringMatch{ + Distinct: true, + Name: "example", + Invert: ptr.To(true), + }, + want: ErrStringMatchInvertDistinctInvalid, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 5afb29d12ce..4400b555dd7 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -2573,6 +2573,11 @@ func (in *StringMatch) DeepCopyInto(out *StringMatch) { *out = new(string) **out = **in } + if in.Invert != nil { + in, out := &in.Invert, &out.Invert + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StringMatch. diff --git a/internal/xds/translator/local_ratelimit.go b/internal/xds/translator/local_ratelimit.go index 1503758dfb4..ba330e22034 100644 --- a/internal/xds/translator/local_ratelimit.go +++ b/internal/xds/translator/local_ratelimit.go @@ -218,13 +218,17 @@ func buildRouteLocalRateLimits(local *ir.LocalRateLimit) ( StringMatch: buildXdsStringMatcher(match), }, } + expectMatch := true + if match.Invert != nil && *match.Invert { + expectMatch = false + } action := &routev3.RateLimit_Action{ ActionSpecifier: &routev3.RateLimit_Action_HeaderValueMatch_{ HeaderValueMatch: &routev3.RateLimit_Action_HeaderValueMatch{ DescriptorKey: descriptorKey, DescriptorValue: descriptorVal, ExpectMatch: &wrapperspb.BoolValue{ - Value: true, + Value: expectMatch, }, Headers: []*routev3.HeaderMatcher{headerMatcher}, }, diff --git a/internal/xds/translator/ratelimit.go b/internal/xds/translator/ratelimit.go index 8e3e661f9d7..660bc2a7dec 100644 --- a/internal/xds/translator/ratelimit.go +++ b/internal/xds/translator/ratelimit.go @@ -180,13 +180,17 @@ func buildRouteRateLimits(descriptorPrefix string, global *ir.GlobalRateLimit) [ StringMatch: buildXdsStringMatcher(match), }, } + expectMatch := true + if match.Invert != nil && *match.Invert { + expectMatch = false + } action := &routev3.RateLimit_Action{ ActionSpecifier: &routev3.RateLimit_Action_HeaderValueMatch_{ HeaderValueMatch: &routev3.RateLimit_Action_HeaderValueMatch{ DescriptorKey: descriptorKey, DescriptorValue: descriptorVal, ExpectMatch: &wrapperspb.BoolValue{ - Value: true, + Value: expectMatch, }, Headers: []*routev3.HeaderMatcher{headerMatcher}, }, diff --git a/internal/xds/translator/testdata/in/xds-ir/ratelimit.yaml b/internal/xds/translator/testdata/in/xds-ir/ratelimit.yaml index 271d39cfdcb..2279315caed 100644 --- a/internal/xds/translator/testdata/in/xds-ir/ratelimit.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/ratelimit.yaml @@ -65,3 +65,24 @@ http: - endpoints: - host: "1.2.3.4" port: 50000 + - name: "fourth-route" + hostname: "*" + traffic: + rateLimit: + global: + rules: + - headerMatches: + - name: "x-org-id" + exact: "admin" + invert: true + limit: + requests: 5 + unit: second + pathMatch: + exact: "foo/bar/login" + destination: + name: "fourth-route-dest" + settings: + - endpoints: + - host: "1.2.3.4" + port: 50000 diff --git a/internal/xds/translator/testdata/out/xds-ir/ratelimit.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/ratelimit.clusters.yaml index 0ba1749076a..427f6d15340 100644 --- a/internal/xds/translator/testdata/out/xds-ir/ratelimit.clusters.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/ratelimit.clusters.yaml @@ -49,6 +49,23 @@ outlierDetection: {} perConnectionBufferLimitBytes: 32768 type: EDS +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_ONLY + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: fourth-route-dest + lbPolicy: LEAST_REQUEST + name: fourth-route-dest + outlierDetection: {} + perConnectionBufferLimitBytes: 32768 + type: EDS - circuitBreakers: thresholds: - maxRetries: 1024 diff --git a/internal/xds/translator/testdata/out/xds-ir/ratelimit.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/ratelimit.endpoints.yaml index 475b89a087c..f185af17da7 100644 --- a/internal/xds/translator/testdata/out/xds-ir/ratelimit.endpoints.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/ratelimit.endpoints.yaml @@ -34,3 +34,15 @@ loadBalancingWeight: 1 locality: region: third-route-dest/backend/0 +- clusterName: fourth-route-dest + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: fourth-route-dest/backend/0 diff --git a/internal/xds/translator/testdata/out/xds-ir/ratelimit.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/ratelimit.routes.yaml index 479c2cd143c..e6e83bc2bfb 100644 --- a/internal/xds/translator/testdata/out/xds-ir/ratelimit.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/ratelimit.routes.yaml @@ -55,3 +55,23 @@ descriptorValue: rule-0-match--1 upgradeConfigs: - upgradeType: websocket + - match: + path: foo/bar/login + name: fourth-route + route: + cluster: fourth-route-dest + rateLimits: + - actions: + - genericKey: + descriptorKey: fourth-route + descriptorValue: fourth-route + - headerValueMatch: + descriptorKey: rule-0-match-0 + descriptorValue: rule-0-match-0 + expectMatch: false + headers: + - name: x-org-id + stringMatch: + exact: admin + upgradeConfigs: + - upgradeType: websocket