Use native kubernetes ways of configuration

[nesl247]

I'm currently using the helm chart in the incubator (https://github.com/helm/charts/tree/master/incubator/vault), and trying to migrate over to this official chart.

Unfortunately a lot of what is done in this helm chart is so much harder that the one from the incubator. Annotations for the pod for example require you to use a string instead of helm parsing it, allowing for the same configuration one would do for kubernetes itself. Same goes for volume and mounts.

[jasonodonnell]

Hi @nesl247!

We're always interested in making this easier to use. I'm trying to understand where we're falling short here. Can you provide me an example of the UX you'd prefer or point to code that could be changed?

Thanks!

[nesl247]

Hi @jasonodonnell,

See how pod annotations are done here: https://github.com/helm/charts/blob/8180c3ffcf90dd5c2bc40e7006715100d7d6386d/incubator/vault/templates/deployment.yaml#L41 as an example. This is a lot easier to work with rather than having to do yaml strings.

[jasonodonnell]

@nesl247,

Appreciate the tip, I think this is a good improvement and makes sense to add. Keeping this open to track the work!

[davidmogar]

I didn't see this issue when preparing my change, but it makes a lot of sense. Specially when you have to write sensitive information as passwords. I'm trying to cover the config part, more or less, with this change #213.

Another thing that puzzles me but I didn't want to introduce in my change is the fact that deployment mode is split and each one define it's config. A single variable could include the mode and another one the config (with any validation done by Helm).

But for sure, my biggest problem right now is the config specification. Having the possibility to do it in a more dynamic way opens the doors to GitOps and secret management.

[nesl247]

Any word on if this would be accepted if I were to submit a PR? Working with this helm chart is a bit painful as it does a lot of custom stuff instead of adopting community standards, and using native features of yaml.

[tvoran]

@nesl247 What all do you have in mind? Just changing multiline strings to yaml maps, or something more?

You mentioned Pulumi in another issue, is that the motivation for the changes? Can you elaborate on the issues you’re encountering?

[nesl247]

I would change any type of value that can be represented in a kubernetes native way to such way. For example, yaml maps for annotations and labels, volumes and volume mounts for volumes, affinity, nodeSelector, rename several values such as disruptionBudget to podDisruptionBudget (more commonly used), allow the config to be stored in an already existing ConfigMap or Secret, refactor the liveness and readiness probes, etc.

It took me a lot longer to figure out all of the custom ways that this helm chart worked rather than using the kubernetes native values, as well as coming from other helm charts that follow this format. This also makes the chart a lot more flexible as it doesn't depend upon as much custom logic which needs to support wider use cases.

On top of which, it solves the discrepancy between values such as resources and volumes, the former using the kubernetes native values, and the latter using a custom solution.

[tvoran]

Hi @nesl247, I'd say we’re interested in supporting both YAML and YAML-formatted strings for chart values, as done in #272. For the other changes you mentioned I’d suggest separate PRs for each one to make it easier to review.

[nesl247]

I will do so when I have some time if someone doesn't beat me to it.

Thanks.