kubernetes / ingress-nginx

Ingress NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.41k stars 8.24k forks source link

Merge slashes issues #10088

Closed iusergii closed 1 year ago

iusergii commented 1 year ago

What happened: I have use case when application makes request https://bar.com//foo/zhs/api and as result I have 500.

What you expected to happen:

I was under immersion that merge_slashes is enabled by default and should merge it.

NGINX Ingress controller version: 1.3.1 Kubernetes version: 1.21.14

k8s-ci-robot commented 1 year ago

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
longwuyuan commented 1 year ago

/remove-kind bug /kind feature

longwuyuan commented 1 year ago

/remove-kind bug /kind feature

k8s-ci-robot commented 1 year ago

@longwuyuan: Those labels are not set on the issue: kind/bug

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/10088#issuecomment-1593109632): >/remove-kind bug >/kind feature Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
z1cheng commented 1 year ago

Hi @iusergii Could you please provide more info, like your ingress spec? If the URL is not merged and not matched, the response should be 404 Not Found. 500 response code means Internal Server Error. And no errors when using https://bar.com/foo/zhs/api?

iusergii commented 1 year ago

I need to cut path prefix and I cannot use rewrite-target as it fails in case of special character there.

Applications built on top of Spring Boot and it requires X-Forwarded-Prefix headers. So my ingress rules looks like this:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    nginx.ingress.kubernetes.io/backend-protocol: HTTPS
    nginx.ingress.kubernetes.io/configuration-snippet: |
      more_clear_input_headers ;
      proxy_set_header X-Forwarded-Prefix /foo/zhs/api;

      if ($request_uri ~ "^/foo/zhs/api(.*)") {
        proxy_pass https://upstream_balancer$1;
        break;
      }
  name: foo
spec:
  ingressClassName: nginx
  rules:
  - host: '*.com'
    http:
      paths:
      - backend:
          service:
            name: foo
            port:
              name: https
        path: /foo/zhs/api
        pathType: Prefix

URI without double slash https://bar.com/foo/zhs/api works.

z1cheng commented 1 year ago

@iusergii I tested it with your spec on my local machine. $request_uri is not changed even after merging slashes. So your condition $request_uri ~ "^/foo/zhs/api(.*)" is not matched when using https://bar.com//foo/zhs/api, then your requests are routed to foo service, not https://upstream_balancer$1.

A feasible solution is to update your regex: $request_uri ~ "^(/+)foo(/+)zhs(/+)api(.*)" (but it needs your load balancer to be able to recognize and accept the requests with multiple slashes in a URI)

iusergii commented 1 year ago

@z1cheng thank you for clarification. This seems really an issue with workaround. Closing.