-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Conversation
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.
Some improvements to do but this is really good 👍🏻
Also https://github.com/elastic/helm-charts/tree/main/elasticsearch#how-to-deploy-clusters-with-security-authentication-and-tls-enabled should be updated to mention that certs are created by default but you can provide your own certs using the example in https://github.com/elastic/helm-charts/tree/main/elasticsearch/examples/security |
jenkins test this please |
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.
https://devops-ci.elastic.co/job/elastic+helm-charts+pull-request+integration-elasticsearch/1250/ES_SUITE=multi,KUBERNETES_VERSION=1.19,label=docker&&virtual/console is failing because the data and client Helm releases are creating their own secret cert instead of mounting the one created by the master release.
I think that data and client values for this test should be updated with:
createCert: false
secretMounts:
- name: elastic-certificates
secretName: multi-master-certs
path: /usr/share/elasticsearch/config/certs
checking |
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⛴
Elasticsearch tests are successful 🎉 As discussed, we'll merge this PR and fix the tests for the other charts in dedicated follow-up PRs. |
Fix #1443
This PR add ssl configuration by default in our helm chart. The SSL certs are generated by helm.