From 059134c87930903f8cdd2c70094ba59a0ae55728 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 | 7 +++-- ...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 +++- .../ratelimit-config/value-invert-match.yaml | 29 +++++++++++++++++++ .../ratelimit-config/value-invert-match.yaml | 20 +++++++++++++ 10 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 internal/xds/translator/testdata/in/ratelimit-config/value-invert-match.yaml create mode 100644 internal/xds/translator/testdata/out/ratelimit-config/value-invert-match.yaml diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index d71d49f32ca9..8a40bfe7a3f7 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,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 index e4f1fc10c644..f536d9a20bcf 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 2f7a2d5e8f91..07fa997e1096 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 8aefbd553ed7..d62e976bf032 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 14b624f22f3f..5ff9a8736efd 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 5afb29d12ce8..4400b555dd73 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 1503758dfb42..ba330e22034c 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 8e3e661f9d7e..660bc2a7dec0 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/ratelimit-config/value-invert-match.yaml b/internal/xds/translator/testdata/in/ratelimit-config/value-invert-match.yaml new file mode 100644 index 000000000000..23550d7d4326 --- /dev/null +++ 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 new file mode 100644 index 000000000000..fa71116ba75b --- /dev/null +++ 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