feat: support inverting header matches for rate limit

Signed-off-by: Rudrakh Panigrahi <[email protected]>
envoyproxy · Sep 29, 2024 · 059134c · 059134c
1 parent b91f296
commit 059134c
Show file tree

Hide file tree

Showing 10 changed files with 103 additions and 5 deletions.
diff --git 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,12 +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:
 				m := &ir.StringMatch{
 					Name:     header.Name,
 					Distinct: true,
+					Invert:   header.Invert,
 				}
 				irRule.HeaderMatches = append(irRule.HeaderMatches, m)
 			default:

diff --git 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
@@ -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
@@ -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
@@ -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
diff --git 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
@@ -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/ratelimit-config/value-invert-match.yaml b/internal/xds/translator/testdata/in/ratelimit-config/value-invert-match.yaml
@@ -0,0 +1,29 @@
+name: "first-listener"
+address: "0.0.0.0"
+port: 10080
+hostnames:
+- "*"
+path:
+  mergeSlashes: true
+  escapedSlashesAction: UnescapeAndRedirect
+routes:
+- name: "first-route"
+  traffic:
+    rateLimit:
+      global:
+        rules:
+        - headerMatches:
+          - name: "x-user-id"
+            exact: "one"
+            invert: true
+          limit:
+            requests: 5
+            unit: second
+  pathMatch:
+    exact: "foo/bar"
+  destination:
+    name: "first-route-dest"
+    settings:
+    - endpoints:
+      - host: "1.2.3.4"
+        port: 50000
diff --git a/internal/xds/translator/testdata/out/ratelimit-config/value-invert-match.yaml b/internal/xds/translator/testdata/out/ratelimit-config/value-invert-match.yaml
@@ -0,0 +1,20 @@
+name: first-listener
+domain: first-listener
+descriptors:
+  - key: first-route
+    value: first-route
+    rate_limit: null
+    descriptors:
+      - key: rule-0-match-0
+        value: rule-0-match-0
+        rate_limit:
+          requests_per_unit: 5
+          unit: SECOND
+          unlimited: false
+          name: ""
+          replaces: []
+        descriptors: []
+        shadow_mode: false
+        detailed_metric: false
+    shadow_mode: false
+    detailed_metric: false