Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support inverting header matches for rate limit #4286

Merged
merged 1 commit into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions internal/gatewayapi/backendtrafficpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,8 +710,9 @@
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:
Expand All @@ -721,9 +722,14 @@
m := &ir.StringMatch{
Name: header.Name,
SafeRegex: header.Value,
Invert: header.Invert,

Check warning on line 725 in internal/gatewayapi/backendtrafficpolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/backendtrafficpolicy.go#L725

Added line #L725 was not covered by tests
}
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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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/*
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions internal/ir/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand All @@ -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++
}

Expand Down
19 changes: 18 additions & 1 deletion internal/ir/xds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand All @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions internal/ir/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion internal/xds/translator/local_ratelimit.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,13 +218,17 @@
StringMatch: buildXdsStringMatcher(match),
},
}
expectMatch := true
if match.Invert != nil && *match.Invert {
expectMatch = false

Check warning on line 223 in internal/xds/translator/local_ratelimit.go

View check run for this annotation

Codecov / codecov/patch

internal/xds/translator/local_ratelimit.go#L223

Added line #L223 was not covered by tests
}
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},
},
Expand Down
6 changes: 5 additions & 1 deletion internal/xds/translator/ratelimit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
},
Expand Down
Loading
Loading