-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add TLS for Nginx Helm Chart #2115
Conversation
Nice feature. Could you drop a few words in the kubernetes.md to explain the difference between this (end to end encryption to the nginx pod) and the the pre-existing way to configure TLS at ingress level (which was fine too I guess provided the cluster itself is sufficiently hardened and has things like mutual tls between nodes)? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as commented, could you add a bit of documentation?
thanks!
@ptrovatelli I added some documentation for the new part. But there is still room for improvements. Maybe in some later PRs :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Will merge now.
@ptrovatelli it semms I cant merge the PR in github until you approve that your requested change is fulfilled. Please take a look and merge if possible. |
OK OK go... please let all remember that self-signed certificate by itself is only a first step because it's only good against network sniffing. Protection against man in the middle requires proper CA validation. |
Alright.. merging... |
On behalf of DB Systel GmbH