kubernetes / ingress-nginx

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

Adding configuration-snippet causes the configuration of cors-allow-headers to become invalid. #10862

Open hmthoo opened 10 months ago

hmthoo commented 10 months ago

NGINX Ingress controller: v1.9.4 k8s Verson: v1.27.3


My operation is as follows

test success

curl test success when i not adding the nginx.ingress.kubernetes.io/configuration-snippet . response headeraccess-control-allow-headers is ingress nginx.ingress.kubernetes.io/cors-allow-headers

HTTP/2 400
date: Tue, 16 Jan 2024 08:52:46 GMT
content-type: application/json
access-control-allow-origin: *
access-control-allow-methods: GET, PUT, POST, DELETE, PATCH, OPTIONS
access-control-max-age: 1728000
access-control-allow-headers: digi-host,digi-service,Accept,Authorization,Cache-Control,Content-Type,DNT,If-Modified-Since,Keep-Alive,Origin,User-Agent,X-Mx-ReqToken,X-Requested-With,digi-middleware-auth-app,digi-middleware-auth-access,digi-middleware-auth-user,digi-middleware-auth-use-info,digi-middleware-drive-access,digi-middleware-drive-access-info,digi-middleware-drive-arg,token,apptoken,accesstoken,Program-Code,Module-Name,locale,routerKey
access-control-allow-credentials: true

ingress config

    nginx.ingress.kubernetes.io/cors-allow-headers: digi-host,digi-service,Accept,Authorization,Cache-Control,Content-Type,DNT,If-Modified-Since,Keep-Alive,Origin,User-Agent,X-Mx-ReqToken,X-Requested-With,digi-middleware-auth-app,digi-middleware-auth-access,digi-middleware-auth-user,digi-middleware-auth-use-info,digi-middleware-drive-access,digi-middleware-drive-access-info,digi-middleware-drive-arg,token,apptoken,accesstoken,Program-Code,Module-Name,locale,routerKey
    nginx.ingress.kubernetes.io/enable-cors: "true"

test failed

curl test failed , response headeraccess-control-allow-headers is not ingress nginx.ingress.kubernetes.io/cors-allow-headers

HTTP/2 400
date: Tue, 16 Jan 2024 08:13:03 GMT
content-type: application/json
access-control-allow-origin: *
access-control-allow-methods: POST,PUT,GET,DELETE
access-control-max-age: 86400
access-control-allow-headers: Content-Type,token,accesstoken,digi-middleware-auth-app,Program-Code,Module-Name,origin-iam-url-key,locale,Accept-Language,routerKey

ingress config add nginx.ingress.kubernetes.io/configuration-snippet:

    nginx.ingress.kubernetes.io/configuration-snippet: |
      if ($uid  = test){
          set $var pre;
      }
      if ($uri ~* "^/test") {
          set $backend. xxx.test.svc.cluster.local:8080;
      }
      if ($var = pre) {
          proxy_pass http://$backend;
          break;
      }
    nginx.ingress.kubernetes.io/cors-allow-headers: digi-host,digi-service,Accept,Authorization,Cache-Control,Content-Type,DNT,If-Modified-Since,Keep-Alive,Origin,User-Agent,X-Mx-ReqToken,X-Requested-With,digi-middleware-auth-app,digi-middleware-auth-access,digi-middleware-auth-user,digi-middleware-auth-use-info,digi-middleware-drive-access,digi-middleware-drive-access-info,digi-middleware-drive-arg,token,apptoken,accesstoken,Program-Code,Module-Name,locale,routerKey
    nginx.ingress.kubernetes.io/enable-cors: "true"

This problem was not encountered in my previous ingress version. In this version, in addition to using more_set_headers to set the cors header in each nginx.ingress.kubernetes.io/configuration-snippet.

nginx.ingress.kubernetes.io/cors-allow-headers is set, it doesn’t feel like a good idea to set it again for each nginx.ingress.kubernetes.io/configuration-snippet.

Are there any other good ways?

k8s-ci-robot commented 10 months 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 10 months ago

/remove-kind bug

hmthoo commented 10 months ago

/remove-kind bug

  • I have not tested this and I don't fully understand the configuration snippet
  • But we did harden CORS behavior and tried to harden usage of snippets https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/annotations/#configuration-snippet
  • Generally, can you elaborate in complete detail, why you need to use those snippets ?
  • What are you trying to achieve ?
  • Have you checked the list of annotations and are you sure you can not achieve what you want, with annotations ?
  • You are trying to set a variable and then use that variable to set a backend. Why ?
longwuyuan commented 10 months ago
hmthoo commented 10 months ago
  • The closest I have come to Cookie based routing is while configuring affinity https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/annotations/#session-affinity
  • I was under the impression that routing with ingress was only possible with either the host or the path. Do you already know for sure that you can configure cookie based routing or are you experimenting
  • I have zero/nada/zilch thoughts that loss of headers after using snippet. Vague guess is that someone was talking about sub-locations in nginx.conf causing loss of headers in the use-case of auth related annotations

Don't focus on cookies!

Yes, I'm just curious why setting the snippet causes headers to be lost, whereas it was normal in previous versions of ingress.

longwuyuan commented 10 months ago

Can you add proof that exactly same configuration snippet did not cause loss of headers in a previous version of the controller ? With as much details as possible ? That will make it easier for a developer to review the PRs between the 2 versions of the controller

strongjz commented 10 months ago

Did you set the allow-snippet-annotations to true? We changed the default to false in 1.9.X

https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#allow-snippet-annotations

hmthoo commented 10 months ago

Did you set the allow-snippet-annotations to true? We changed the default to false in 1.9.X

https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#allow-snippet-annotations

I have set allow-snippet-annotations: "true"

github-actions[bot] commented 9 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.