kubernetes / ingress-nginx

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

Unable to remove or update the NGINX added X-Forwarded-Proto header using `charts/ingress-nginx` #11040

Open ashageetha opened 9 months ago

ashageetha commented 9 months ago

What happened:

Cannot remove or override the nginx ingress controller added X-Forwarded-Proto header value. Even though able to pass the customized proxy header using below values.yaml to helm install, but the URL still gets added with X-Forwarded-Proto: http header value.

controller:
  proxySetHeaders:
    X-Forwarded-Proto: ""

The nginx.conf inside the ingress controller pod shows the customized value is being added but the URLs still gets added with the first value and not the latest X-Forwarded-Proto value.

# kubectl  exec -it nginx-ingress-ingress-nginx-controller-799b9ccd64-kvxxf -n myns sh
/etc/nginx $ cat nginx.conf|grep X-Forwarded-Proto
                        proxy_set_header X-Forwarded-Proto      $pass_access_scheme;
                        proxy_set_header X-Forwarded-Proto                    "";
.
.
.
.

What you expected to happen: Expect to have the support for removing the nginx controller added X-Forwarded-* headers from the URLs.

If we manually exec into the nginx controller pod and change the "$pass_access_scheme" to "" for X-Forwarded-Proto, works as expected.

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):

/etc/nginx $ /nginx-ingress-controller --version
-------------------------------------------------------------------------------
NGINX Ingress controller
  Release:       v1.9.6
  Build:         6a73aa3b05040a97ef8213675a16142a9c95952a
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.21.6

-------------------------------------------------------------------------------

/etc/nginx $

Kubernetes version (use kubectl version):

# kubectl version
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.6", GitCommit:"741c8db18a52787d734cbe4795f0b4ad860906d6", GitTreeState:"clean", BuildDate:"2023-09-13T09:21:34Z", GoVersion:"go1.20.8", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v5.0.1
Server Version: version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.8", GitCommit:"66fee42707cd7f5a89f1987f7cb81b02dd19161c", GitTreeState:"clean", BuildDate:"2023-11-15T16:50:09Z", GoVersion:"go1.20.11", Compiler:"gc", Platform:"linux/amd64"}

Environment:

How to reproduce this issue:

Anything else we need to know:

longwuyuan commented 9 months ago

/remove-kind bug

ashageetha commented 9 months ago
# kubectl  -n myns get cm nginx-ingress-ingress-nginx-controller -o yaml
apiVersion: v1
data:
  allow-snippet-annotations: "false"
  proxy-set-headers: myns/nginx-ingress-ingress-nginx-custom-proxy-headers
kind: ConfigMap
metadata:
  annotations:
    meta.helm.sh/release-name: nginx-ingress
    meta.helm.sh/release-namespace: myns
  creationTimestamp: "2024-02-29T06:00:51Z"
  labels:
    app.kubernetes.io/component: controller
    app.kubernetes.io/instance: nginx-ingress
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: ingress-nginx
    app.kubernetes.io/part-of: ingress-nginx
    app.kubernetes.io/version: 1.9.6
    helm.sh/chart: ingress-nginx-4.9.1
  name: nginx-ingress-ingress-nginx-controller
  namespace: myns
  resourceVersion: "11272346"
  uid: bae7c8c0-ac7f-474f-9dff-dce93af6661a

# kubectl -n myns get cm nginx-ingress-ingress-nginx-custom-proxy-headers -o yaml
apiVersion: v1
data:
  X-Forwarded-Proto: ""
kind: ConfigMap
metadata:
  annotations:
    meta.helm.sh/release-name: nginx-ingress
    meta.helm.sh/release-namespace: myns
  creationTimestamp: "2024-02-29T06:00:51Z"
  labels:
    app.kubernetes.io/component: controller
    app.kubernetes.io/instance: nginx-ingress
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: ingress-nginx
    app.kubernetes.io/part-of: ingress-nginx
    app.kubernetes.io/version: 1.9.6
    helm.sh/chart: ingress-nginx-4.9.1
  name: nginx-ingress-ingress-nginx-custom-proxy-headers
  namespace: myns
  resourceVersion: "11272345"
  uid: 1fe1c0e2-d431-4898-9055-e88575f50f78
longwuyuan commented 9 months ago

What I tested and found out is that the configMap is for adding new and customized headers but not for changing the default headers.

longwuyuan commented 9 months ago

I have very little know on this topic but I found that this header is actually a factual info that is sent to the upstream (backend pod) . https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/#using-the-forwarded-header

So in that context, why would you want to set this manually ? Its does not seem plausible that you make a HTTPS request to the cluster but you want to tell the backend pod that its GRPC or anything other than HTTPS !

To the layman reader here, please explain what is the real problem you want to solve by setting "X-Forwarded-Proto" header manually . Thanks

longwuyuan commented 9 months ago

/triage needs-information

ashageetha commented 9 months ago

One of our product URL is behaving different and causing too many redirects (recursively redirecting) if X-Forwarded-Proto is available in the request URL. Hence, we tried to remove this header and not able to achieve with NGINX. But we could modify and set accordingly with Traefik based ingress controller using customRequestHeaders.

longwuyuan commented 9 months ago
longwuyuan commented 9 months ago

After reading this https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto I am convinced you can not change it, in the context of saying your browser connects with HTTPS scheme but you change it manually to HTTP. If traefik changes it, I wonder how it impacts upstream getting false info. But like I said I am not a expert so wait for someone to comment

longwuyuan commented 9 months ago

/remove-triage needs-information /help

k8s-ci-robot commented 9 months ago

@longwuyuan: This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/11040): >/remove-triage needs-information >/help 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 9 months ago

/kind support /triage accepted

longwuyuan commented 9 months ago

/assign

github-actions[bot] commented 7 months ago

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

Mahoney commented 1 month ago

My problem is that I am using TLS termination at the NLB using https://github.com/kubernetes/ingress-nginx/blob/main/deploy/static/provider/aws/nlb-with-tls-termination/deploy.yaml .

Because an NLB is a layer 4 device it cannot add the X-Forwarded-Proto: https header when it proxies the unencrypted request to the nginx pod over HTTP (unlike a layer 7 device like an ALB, which can alter headers).

So when the request reaches nginx it is over HTTP and without any X-Forwarded-Proto header.

Consequently nginx proxies it to the actual application service without that header, so the actual application does not know that the original request was made over HTTPS.

It seems very odd to me that a manifest called nlb-with-tls-termination would not solve this problem - perhaps I am missing something?

longwuyuan commented 1 month ago

@Mahoney