-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Corrects the behaviour of default-ssl-certificate #170
Conversation
Signed-off-by: Tang Le <at28997146@163.com>
@rikatz please do not make |
@aledbf Tks for the answer. Will see another way to implement this :) |
@aledbf This is WIP. Will continue this on thursday. I don't know if the last changes I've made are enough for this self generated certificate. I've noticed that Ingress Controller doesn't follow cert updates when you change the 'TLS' directive and so, don't know if this is a bug inserted by me with the modifications, or if it's something pre-existent. I'll try with the original Master branch the same tests I did here, to see if this still happens. |
OK, so the behaviour is the same without this PR. |
add some unit test cases for some packages under folder "core.pkg.ingress"
Fix rate limit issue when more than 2 servers enabled in ingress
Clarify usage of Ingress backend.servicePort
Update nginx to 1.11.9
Fix TLS does not get updated when changed
Refactoring sysctlFSFileMax helper
…to defaultsslcert
@aledbf Closing this PR. Did some mess here. Going to open another one, only with the right updates. |
This PR corrects the behaviour of default-ssl-certificate
WARNING - There is a breaking change here. Now the directive default-ssl-certificate is mandatory, and also it's existence. If there's no default-ssl-certificate the ingress controller deployment will fail.
When the Ingress Controller starts, it tries to fetch a default SSL Certificate to be used on the default backend service.
Also, when a user configures a ingress controller with the 'tls' directive, but doesn't specify the secret to be used, the default will be used instead.
This PR is related to issue #163 and also is a proposal.