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

Global RateLimit Xds translation #726

Merged
merged 10 commits into from
Jan 9, 2023
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
58 changes: 56 additions & 2 deletions internal/ir/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ var (
ErrHTTPRouteMatchEmpty = errors.New("either PathMatch, HeaderMatches or QueryParamMatches fields must be specified")
ErrRouteDestinationHostInvalid = errors.New("field Address must be a valid IP address")
ErrRouteDestinationPortInvalid = errors.New("field Port specified is invalid")
ErrStringMatchConditionInvalid = errors.New("only one of the Exact, Prefix or SafeRegex fields must be specified")
ErrStringMatchConditionInvalid = errors.New("only one of the Exact, Prefix, SafeRegex or Distinct fields must 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")
ErrRedirectUnsupportedScheme = errors.New("only http and https are supported for the scheme in redirect filters")
Expand Down Expand Up @@ -211,6 +212,9 @@ type HTTPRoute struct {
Destinations []*RouteDestination
// Rewrite to be changed for this route.
URLRewrite *URLRewrite
// RateLimit defines the more specific match conditions as well as limits for ratelimiting
// the requests on this route.
RateLimit *RateLimit
Copy link
Contributor

@danehans danehans Jan 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arkodg I don't understand why RateLimit doesn't use any of the RateLimitFilter fields. For example, #813 uses JwtAuthenticationFilterProvider instead of recreating the type in the IR. Since we own extension APIs, e.g. RateLimitFilter, we should leverage these types as much as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

creating a decoupling from Front End as well as Back End API, that was the main goal of the IR, which is why we created custom structures even for known structures such as StringMatch

Copy link
Contributor Author

@arkodg arkodg Jan 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as you mentioned, since the project owns the structure, should be safe to reuse it, but the structure fields are also arranged differently to facilitate better translation (more machine friendly than user friendly)

}

// Validate the fields within the HTTPRoute structure
Expand Down Expand Up @@ -460,7 +464,7 @@ func (r HTTPPathModifier) Validate() error {
}

// StringMatch holds the various match conditions.
// Only one of Exact, Prefix or SafeRegex can be set.
// Only one of Exact, Prefix, SafeRegex or Distinct can be set.
// +k8s:deepcopy-gen=true
type StringMatch struct {
// Name of the field to match on.
Expand All @@ -473,6 +477,9 @@ type StringMatch struct {
Suffix *string
// SafeRegex match condition.
SafeRegex *string
// Distinct match condition.
// Used to match any and all possible unique values encountered within the Name field.
Distinct bool
arkodg marked this conversation as resolved.
Show resolved Hide resolved
}

// Validate the fields within the StringMatch structure
Expand All @@ -491,6 +498,12 @@ func (s StringMatch) Validate() error {
if s.SafeRegex != nil {
matchCount++
}
if s.Distinct {
zirain marked this conversation as resolved.
Show resolved Hide resolved
if s.Name == "" {
errs = multierror.Append(errs, ErrStringMatchNameIsEmpty)
}
matchCount++
}

if matchCount != 1 {
errs = multierror.Append(errs, ErrStringMatchConditionInvalid)
Expand Down Expand Up @@ -591,3 +604,44 @@ func (h UDPListener) Validate() error {
}
return errs
}

// RateLimit holds the rate limiting configuration.
// +k8s:deepcopy-gen=true
type RateLimit struct {
// Global rate limit settings.
Global *GlobalRateLimit
}

// GlobalRateLimit holds the global rate limiting configuration.
// +k8s:deepcopy-gen=true
type GlobalRateLimit struct {
// Rules for rate limiting.
Rules []*RateLimitRule
}

// RateLimitRule holds the match and limit configuration for ratelimiting.
// +k8s:deepcopy-gen=true
type RateLimitRule struct {
// HeaderMatches define the match conditions on the request headers for this route.
HeaderMatches []*StringMatch
// Limit holds the rate limit values.
Limit *RateLimitValue
}

type RateLimitUnit string

const (
Second RateLimitUnit = "second"
Minute RateLimitUnit = "minute"
Hour RateLimitUnit = "hour"
Day RateLimitUnit = "day"
)

// RateLimitValue holds the
// +k8s:deepcopy-gen=true
type RateLimitValue struct {
// Requests are the number of requests that need to be rate limited.
Requests uint32
// Unit of rate limiting.
Unit RateLimitUnit
}
97 changes: 97 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.

5 changes: 5 additions & 0 deletions internal/xds/translator/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ func addXdsHTTPFilterChain(xdsListener *listener.Listener, irListener *ir.HTTPLi
}},
}

// TODO: Make this a generic interface for all API Gateway features.
if err := patchHCMWithRateLimit(mgr, irListener); err != nil {
return err
}

mgrAny, err := anypb.New(mgr)
if err != nil {
return err
Expand Down
Loading