Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

proxy-redirect is ignored #12917

Open
gfrankliu opened this issue Mar 1, 2025 · 6 comments
Open

proxy-redirect is ignored #12917

gfrankliu opened this issue Mar 1, 2025 · 6 comments
Labels
kind/support Categorizes issue or PR as a support question. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@gfrankliu
Copy link

What happened:

I upgraded from ingress-nginx 1.8.0 to 1.12.0, and now the ingress annonation for proxy-redirect is ignored:

sample config:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: test-ingress
  annotations:
    nginx.ingress.kubernetes.io/use-regex: "true"
    nginx.ingress.kubernetes.io/ssl-redirect: "false"
    nginx.ingress.kubernetes.io/rewrite-target: /$4
    nginx.ingress.kubernetes.io/proxy-redirect-from: /
    nginx.ingress.kubernetes.io/proxy-redirect-to: /$1/$2/$3/
spec:
  ingressClassName: nginx
  rules:
  - http:
      paths:
      - path: /(prod)/([-a-z0-9]+)/(webrtc)/?(.*)
        pathType: ImplementationSpecific
        backend:
          service:
            name: streaming
            port:
              number: 8889

What you expected to happen:

The response "Location" header should have the proper uri prefix matching the request.

I checked the generated nginx.conf from the old ingress 1.8.0 and find:

proxy_redirect / /$1/$2/$3/;

But the nginx.conf from the new ingress 1.12.0 has:

proxy_redirect off;

It seems the new ingress ignored the annotation in the ingress config.

@gfrankliu gfrankliu added the kind/bug Categorizes issue or PR as related to a bug. label Mar 1, 2025
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Mar 1, 2025
@k8s-ci-robot
Copy link
Contributor

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. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@gfrankliu
Copy link
Author

Here is my configmap, in case it matters:

$ kubectl -n ingress-nginx get cm/ingress-nginx-controller -o yaml
apiVersion: v1
data:
  allow-snippet-annotations: "true"
  annotations-risk-level: Critical
  http2-max-requests: "7500000"
  keep-alive-requests: "7500000"
  upstream-keepalive-requests: "7500000"
kind: ConfigMap
...

@longwuyuan
Copy link
Contributor

Please look at relese notes for changes. The slash may not be passing validations tests for annotation.

/remove-kind bug
/kind support

@k8s-ci-robot k8s-ci-robot added kind/support Categorizes issue or PR as a support question. and removed kind/bug Categorizes issue or PR as related to a bug. labels Mar 2, 2025
@gfrankliu
Copy link
Author

Thanks @longwuyuan for the hint!
I patched the ingress-nginx-controller deployment and added the args --enable-annotation-validation=false. Now my annotation works:

kubectl -n ingress-nginx patch deployment ingress-nginx-controller \
  --type json \
  --patch '[{"op":"add","path":"/spec/template/spec/containers/0/args/-","value":"--enable-annotation-validation=false"}]'

Instead of patching the deployment, is there a way to use configmap to disable annotation validation?

Looking at the generated nginx.conf, I am not sure why we want to block slash from passing validation in this special case, given that it is wrapped within the proper Location block. Can the validation be improved?

                location ~* "^/(prod)/([-a-z0-9]+)/(webrtc)/?(.*)" {
...
                        rewrite "(?i)/(prod)/([-a-z0-9]+)/(webrtc)/?(.*)" /$4 break;
                        proxy_pass http://upstream_balancer;

                        proxy_redirect                          / /$1/$2/$3/;

                }

@longwuyuan
Copy link
Contributor

If yo uuse helm, then you can disable validations but also make sure to take care of risk-level confg.

I think slash is notorious for participating in undesired strings. The consequences can be really bad. Please discuss in the dev channel on slack for allowing slash in that annotation. Or join the monthly community meetings.

@gargamile
Copy link

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
kind/support Categorizes issue or PR as a support question. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

No branches or pull requests

4 participants