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

DO NOT MERGE Net 8416 add gateway api request redirect filter #21089

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

missylbytes
Copy link
Contributor

Description

Testing & Reproduction steps

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@github-actions github-actions bot added theme/api Relating to the HTTP API interface theme/envoy/xds Related to Envoy support labels May 13, 2024
@missylbytes missylbytes added the theme/api-gw Track API gateway work label May 13, 2024
@@ -214,6 +214,8 @@ message HTTPRouteFilter {
// URLRewrite defines a schema for a filter that modifies a request during
// forwarding.
HTTPURLRewriteFilter url_rewrite = 5;
Copy link
Member

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
Copy link
Member

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"`
Copy link
Member

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 {
Copy link
Member

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 {
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/api Relating to the HTTP API interface theme/api-gw Track API gateway work theme/envoy/xds Related to Envoy support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants