Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

[kibana] Make service account configurable, create one by default, modify help… #208

Closed
wants to merge 5 commits into from

Conversation

naseemkullah
Copy link
Contributor

…ers.tpl

Signed-off-by: Naseem naseemkullah@gmail.com

  • Chart version not bumped (the versions are all bumped and released at the same time)
  • README.md updated with any new values or changes
  • Updated template tests in ${CHART}/tests/*.py
  • Updated integration tests in ${CHART}/examples/*/test/goss.yaml

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

…ers.tpl

Signed-off-by: Naseem <naseemkullah@gmail.com>
kibana/README.md Outdated Show resolved Hide resolved
@naseemkullah naseemkullah force-pushed the kibana-sa branch 2 times, most recently from 2e56ee1 to 9e36c71 Compare August 7, 2019 15:00
Signed-off-by: Naseem <naseemkullah@gmail.com>
kibana/values.yaml Outdated Show resolved Hide resolved
kibana/templates/deployment.yaml Outdated Show resolved Hide resolved
kibana/values.yaml Outdated Show resolved Hide resolved
@@ -54,6 +54,9 @@ helm install --name kibana elastic/kibana --set imageTag=7.3.0
| `serverHost` | The [`server.host`](https://www.elastic.co/guide/en/kibana/current/settings.html) Kibana setting. This is set explicitly so that the default always matches what comes with the docker image. | `0.0.0.0` |
| `healthCheckPath` | The path used for the readinessProbe to check that Kibana is ready. If you are setting `server.basePath` you will also need to update this to `/${basePath}/app/kibana` | `/app/kibana` |
| `kibanaConfig` | Allows you to add any config files in `/usr/share/kibana/config/` such as `kibana.yml`. See [values.yaml](./values.yaml) for an example of the formatting. | `{}` |
| `managedServiceAccount.create` | Create service account | `true` |
| `managedServiceAccount.name` | The name of the ServiceAccount to use. If not set and create is true, a name is generated using the fullname template | `` |
| `serviceAccount` | Allows you to overwrite the "default" [serviceAccount](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/) for the pod | `[]` |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a duplicate line since it is already in the readme

Co-Authored-By: Michael Russell <Crazybus@users.noreply.github.com>
@jmlrt
Copy link
Member

jmlrt commented Sep 20, 2019

jenkins test this please

@jmlrt
Copy link
Member

jmlrt commented Sep 23, 2019

Hi @naseemkullah,
Following #207 (comment), service account creation has already been implemented in Elasticsearch Chart by #265 and for consistency reasons, we expect it to be implemented in the same way in every Elastic Helm Charts.
Thank you for your great work on this PR.
I'll keep it open until we have a PR merged with an implementation similar to what's done in #265 for Elasticsearch. Don't hesitate reopen another PR for the new labels creation or if you want to provide #265 implementation for Kibana and the Beats Charts.

@naseemkullah
Copy link
Contributor Author

Hi @jmlrt just a small note with regards to #265, if I understand correctly the rbac key also creates a service account, according to helm documentation it is better to have these created under separate keys: https://helm.sh/docs/chart_best_practices/#yaml-configuration

@jmlrt jmlrt added the enhancement New feature or request label Oct 17, 2019
@jmlrt
Copy link
Member

jmlrt commented Nov 18, 2019

Hi @naseemkullah, sorry for the very late answer.

Thanks for your link.

Also following Helm Best Practices is important, our main priority with these Helm charts are "ensuring that we don't introduce any breaking changes so upgrading from a version of the chart to a newer shouldn't break running Helm releases" (we'll have to add some one day, but we prefer not currently) and "ensuring that we provide consistency between all our charts (Elasticsearch, Kibana, Logstash, Filebeat and Metricbeat) so that a user shouldn't have different ways to do something in one chart or another".

As #265 was merged before for Elasticsearch, we prefer not change our way to manage RBAC and PSP unless we find some way to do it which won't break for existing users during helm upgrade and we prefer not introduce a different way to manage service accounts in other charts to avoid inconsistencies.

@naseemkullah
Copy link
Contributor Author

Very well @jmlrt, thanks for the explanation! Closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants