search

kubernetes / ingress-nginx

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

ssl-passthrough config lost for ingress on config reload when --disable-full-test is set #10386

Closed jfpucheu closed 1 week ago

jfpucheu commented 1 year ago

What happened: Ingress using ssl-passthrough stop working on some nginx config reload and finaly the request is sent to nginx and the fake certificate is display because no certificates are set on this ingress because it is ssl-passthrough

for example:

I0908 08:26:40.475440       6 tcp.go:74] "TLS Client Hello" host="auth.ccnp-mgt01.com.toto"
I0908 08:26:40.475550       6 tcp.go:84] "passing to" hostport="10.105.235.186:5556"   --> this is ok
#### config reload ######
I0908 08:26:50.674770       6 tcp.go:74] "TLS Client Hello" host="auth.ccnp-mgt01.com.toto"
I0908 08:26:50.674898       6 tcp.go:84] "passing to" hostport="127.0.0.1:442" ->  ssl-passthrough stop working , redirect to nginx

a full restart of nginx solve the issue during some times..

What you expected to happen:

Nginx-ingress-controller should continue to redirect request tcp to 10.105.235.186:5556 all time

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

-------------------------------------------------------------------------------
NGINX Ingress controller
  Release:       v1.8.1
  Build:         dc88dce9ea5e700f3301d16f971fa17c6cfe757d
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.21.6

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

Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.8", GitCommit:"0ce7342c984110dfc93657d64df5dc3b2c0d1fe9", GitTreeState:"clean", BuildDate:"2023-03-15T13:39:54Z", GoVersion:"go1.19.7", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v4.5.7
Server Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.8", GitCommit:"0ce7342c984110dfc93657d64df5dc3b2c0d1fe9", GitTreeState:"clean", BuildDate:"2023-03-15T13:33:02Z", GoVersion:"go1.19.7", Compiler:"gc", Platform:"linux/amd64"}

Environment:

How to reproduce this issue:

Create an ingress with ssl-passthrough:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    nginx.ingress.kubernetes.io/backend-protocol: HTTPS
    nginx.ingress.kubernetes.io/ssl-passthrough: "true"
    nginx.ingress.kubernetes.io/ssl-redirect: "true"
  name: dex
  namespace: dex
spec:
  ingressClassName: nginx
  rules:
  - host: auth.ccnp-mgt01.com.toto
    http:
      paths:
      - backend:
          service:
            name: dex
            port:
              number: 5556
        path: /
        pathType: ImplementationSpecific

add the parameter: --disable-full-test to ingress-controller

after some time and config reload the issue appear

Anything else we need to know:

the issue seems to be present only with --disable-full-test parameter set. A restart of nginx reload all the config and solve the issue temporarly

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

this message 

"passing to" hostport="127.0.0.1:442" ->

shows a loopback ipaddress and it sort of implies you want a TLS connection to the ingress-controller pod on its internal loopback interface. Does not make sense. Much more data is needed as to understand this.

Please try to add info here like ;

/remove-kind bug /kind support

jfpucheu commented 1 year ago

"shows a loopback ipaddress and it sort of implies you want a TLS connection to the ingress-controller pod on its internal loopback interface" ---> that is the subject .... it is not what we want. it is SSL Passtrough set up , we should never see the request going to 127.0.0.1:442. But sometimes on some reloads it's append when --disable-full-test is set

longwuyuan commented 1 year ago

There are 2 aspects. If the state was in transit to being reconciled and a new connection for tls-passthrough was attempted, then this would be expected.

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

rtikunov commented 9 months ago

Hit this bug also. In big clusters with big number of ingresses it takes ages to reload config without --disable-full-test. So this option used often. And really it breaks nginx.ingress.kubernetes.io/ssl-passthrough=true annotation.

longwuyuan commented 1 week ago

It is possible that large volume of change being done concurrently, will cause delay in reload and error during reload.

But the description of this issue is not enough to take any practical action on. We already know about the large sixe change reload performance. There is work in progress to mitigate that large size reload and security problem.

If this issue is only about large size reload breaking ssl-passthrough even then there is no action item here for the project because when the controller process is stuck, then many things will break and not only ssl-passthrough. Having more compute or memory at the time and on the node where the reload process is happening may be a temporary workaround. This is inherited from nginx as vanilla nginx also will reload delay if the number of changes is too bug and all concurrent.

Plain ssl-passthrough works without problems as even other software use it https://argo-cd.readthedocs.io/en/stable/operator-manual/ingress/#kubernetesingress-nginx .

Since there is no action-item tracking here in this issue I will close it because it is adding to the tally of open issues.

/close

k8s-ci-robot commented 1 week ago

@longwuyuan: Closing this issue.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/10386#issuecomment-2350884622): >It is possible that large volume of change being done concurrently, will cause delay in reload and error during reload. > >But the description of this issue is not enough to take any practical action on. We already know about the large sixe change reload performance. There is work in progress to mitigate that large size reload and security problem. > >If this issue is only about large size reload breaking ssl-passthrough even then there is no action item here for the project because when the controller process is stuck, then many things will break and not only ssl-passthrough. Having more compute or memory at the time and on the node where the reload process is happening may be a temporary workaround. This is inherited from nginx as vanilla nginx also will reload delay if the number of changes is too bug and all concurrent. > >Plain ssl-passthrough works without problems as even other software use it https://argo-cd.readthedocs.io/en/stable/operator-manual/ingress/#kubernetesingress-nginx . > >Since there is no action-item tracking here in this issue I will close it because it is adding to the tally of open issues. > >/close 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-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.