-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
DO NOT MERGE Net 8416 add gateway api request redirect filter #21089
base: main
Are you sure you want to change the base?
DO NOT MERGE Net 8416 add gateway api request redirect filter #21089
Conversation
@@ -214,6 +214,8 @@ message HTTPRouteFilter { | |||
// URLRewrite defines a schema for a filter that modifies a request during | |||
// forwarding. | |||
HTTPURLRewriteFilter url_rewrite = 5; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to touch this as the v2 code is scheduled for deletion (unless you need to for the xdsv2 compat shim tests to continue to pass in the interim).
case structs.HTTPPathModifierTypeReplacePrefixMatch: | ||
return HTTPPathModifierType_HTTPPathModifierTypeReplacePrefixMatch | ||
default: | ||
return HTTPPathModifierType_HTTPPathModifierTypeReplaceFullPath // TODO: Melisa Griffin what should this be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A: usually the zero value, unless you want to panic instead
@@ -471,6 +471,9 @@ type ServiceRouteDestination struct { | |||
// Allow HTTP header manipulation to be configured. | |||
RequestHeaders *HTTPHeaderModifiers `json:",omitempty" alias:"request_headers"` | |||
ResponseHeaders *HTTPHeaderModifiers `json:",omitempty" alias:"response_headers"` | |||
|
|||
// RequestRedirect TODO: Melisa update this comment | |||
RequestRedirect *RequestRedirect `json:",omitempty" alias:"request_redirect"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validation-wise you're going to want it to be mutually exclusive with: namespace, partition, service, serviceSubset
@@ -631,6 +631,10 @@ func (c *compiler) assembleChain() error { | |||
route := router.Routes[i] | |||
|
|||
compiledRoute := &structs.DiscoveryRoute{Definition: &route} | |||
if route.Destination.RequestRedirect != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if you move this past the append()
on the subsequent line, you can add in a continue
inside the conditional and thus skip all of the rest which is about picking the next node.
@@ -197,11 +197,12 @@ type HTTPQueryMatch struct { | |||
// HTTPFilters specifies a list of filters used to modify a request | |||
// before it is routed to an upstream. | |||
type HTTPFilters struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(for a different file)
You'll also need to update the api
file representing ServiceRouter
to pick up the new change, as well as the one for the v1/discovery-chain
endpoint response object.
Description
Testing & Reproduction steps
Links
PR Checklist