Allow Redirects to Drop Trailing Slash

[robscott]

What would you like to be added: A way to configure a redirect to drop a trailing slash.

Why this is needed: Today our redirect filter makes it seem like this should maybe be possible, for example this gets kinda close:

kind: HTTPRoute
metadata:
  name: http-filter-redirect
spec:
  hostnames:
  - example.com
  rules:
  - matches:
    - path:
        type: Exact
        value: /example/
    filters:
    - type: RequestRedirect
      requestRedirect:
        statusCode: 302
        path:
          type: ReplaceFullPath
          replacePrefixMatch: /example

Unfortunately an implementation of this config might result in a redirect loop depending on what we decide in https://github.com/kubernetes-sigs/gateway-api/issues/1953. Even if it did work, it's still not as easy as it should be to do that. At a minimum, we should clarify whether this works. We should also consider providing a better more scalable way to do this even if this way may technically work.

[tonygilkerson]

with this would I also be able to do the following?

...
  - matches:
    - path:
        type: PathPrefix
        value: /api/
    filters:
    - type: URLRewrite
      urlRewrite:
        path:
          type: ReplacePrefixMatch
          replacePrefixMatch: /
    backendRefs:
    - name: ace-hub-docs
      port: 80
...

So that a request for /api/foo and /api/bar become /foo and /bar? When I try this today I get //foo and //bar

[robscott]

@tonygilkerson As long as it's broadly implementable, I think that needs to be in scope for whatever is proposed here.

[shaneutt]

/assign @robscott

[JackPott]

I have observed the same behavior as @tonygilkerson, which feels like a bug to me. If one sets replacePrefixMatch: "" the rule disables complete, but with replacePrefixMatch: / its rewriting /foo/bar into //bar, which my upstream rejects as a bad request.

• /foo becomes / which seems correct
• foo/ becomes //
• foo/bar becomes //bar
• /foo/bar/ becomes //bar/

Workaround is have one rule for /foo -> / and another for /foo/bar -> /bar but this has obvious drawbacks.

This is using Istio gateway as the implementation, unclear if this is their interpretation of the API?

[roquie]

1.0.0 is already being released, but this bug has not been fixed. This is a very critical problem, why is no one paying attention to it?

[janeklb]

This is particularly confusing since the docs for HTTPPathModifier (https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io%2fv1.HTTPPathModifier) have a table of examples which are inconsistent with actual behaviour.

[shaneutt]

Where are we at with this one?

[Demonsthere]

Bumping thread as we experience the same issue:

  - filters:
    - requestRedirect:
        path:
          replacePrefixMatch: /
          type: ReplacePrefixMatch
        port: 8080
        scheme: https
        statusCode: 301
      type: RequestRedirect
    matches:
    - path:
        type: PathPrefix
        value: /api/foo/public/

In the given example we want to simply delete the whole Prefix, replace it with /. However what we see happening is a double slash:

GET https://REDACTED/api/foo/public/bar
      accept: application/json
      accept-encoding: gzip,deflate,br
    ← 301 Moved Permanently
      transfer-encoding: chunked
      connection: close
      location: https://REDACTED:8080//bar

We have tried fixing that with a clienttrafficpolicy:

apiVersion: gateway.envoyproxy.io/v1alpha1
kind: ClientTrafficPolicy
metadata:
  name: proxy
spec:
  path:
    disableMergeSlashes: false

But that didn't remove the double slash as expected

[zhaohuabing]

I also ran into the same issue. To solve this, should we also make trailing splash significant for prefix match? @robscott

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[janeklb]

/remove-lifecycle stale

[shaneutt]

Hi @janeklb!

I see you've bumped the lifecycle on this, out of curiosity is this an issue you might have some time to work on personally?

[janeklb]

Hi @janeklb!

I see you've bumped the lifecycle on this, out of curiosity is this an issue you might have some time to work on personally?

Hard to say just now, but I'd happily taken temporary ownership of this issue (maybe for a month?) to see if I can get it done within that timeframe. My go experience is very limited though, so i'll need some time to ramp up a bit.

I've looked through https://gateway-api.sigs.k8s.io/contributing/devguide/

Would you be able to point me at any files that you'd expect to be involved in solving this?

[robscott]

Hey @janeklb, thanks for following up on this one! I think this is more of a question of "how" we could even accomplish this. As a reminder, the root of the problem is how we've defined prefix matching:

https://github.com/kubernetes-sigs/gateway-api/blob/1e7e64f495f55ece9784910dae2e5e95e03d7bac/apis/v1/httproute_types.go#L381-L391

To solve this, we'd need a solution that allowed most common dataplanes (Envoy, NGINX, HAProxy, etc) to be able to both implement that matching logic and also honor a redirect to drop a trailing slash. The risk is that a naive implementation would result in a redirect loop:

1. Request to /foo/ is matched by /foo PathPrefix
2. We redirect to /foo
3. Request to /foo is matched by /foo PathPrefix
4. We redirect to /foo
5. Repeat 3-4 indefinitely

This discussion ends up being closely tied to https://github.com/kubernetes-sigs/gateway-api/issues/1371.

I don't think we can change how existing path matching logic works in a backwards compatible way, so that seems to leave us with the following options:

1. Introduce a new prefix path matching type that considers a trailing slash meaningful (this is an API change so would need to roll out over several releases following the GEP process
2. Find a safe and broadly implementable way to prevent redirect loops with some form of spec clarification (this clarification may be limited to how we match trailing slashes)

I haven't had the time to think through this enough to figure out if 2 is viable, but if so, it seems like it would be the shortest path to a solution. If not, I feel fairly confident that 1 would be welcome and fairly straightforward, just would take a fair amount of time to become available.

[janeklb]

Thanks @robscott! Seems like this requires significant background knowledge, and given that it's more open ended than I expected, I don't feel as eager to approach it let alone commit to delivering a solution.

That said, I'll try to learn more about how all this works... but I wouldn't be surprised if the triage bot marks this as stale before I provide an update.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten