feat!: harmonize service account binding #major

[aslafy-z]

Closes #320
Closes #361

• BREAKING: Rename rbac.serviceAccount.enabled field to rbac.serviceAccount.create.

• Fix inconsistencies in serviceAccount binding across CronJob, Job, and Deployment templates by introducing a new application.serviceAccountName template:

  rbac.serviceAccount.create	rbac.serviceAccount.name	result
  true	""	(include "application.name" .)
  true	"foo"	"foo"
  false	""	"default"
  false	"bar"	"bar"

[d3adb5]

Left a few comments. I like the idea of adding a semantic value like this, but I feel the way it currently is introduces more confusion than clarity. I expanded upon this in one of the threads I opened below.

[d3adb5]

I fear that this may just cause confusion when reading release values later down the line. I'd much rather have .Values.rbac.enabled be changed to .Values.rbac.create instead, but I realize that would be a breaking change. The way things are in this PR, I find it a bit confusing for a release using an existing service account:

• .Values.rbac.serviceAccount.name exists, but is completely empty, meaning default or disabled. The comments hint at the default value being {{ include "application.name" $ }}.
• .Values.rbac.serviceAccount.enabled exists and is set to false. What that means is unclear unless we peek into the templates or make an assumption based on widely available charts.
• .Values.rbac.existingServiceAccount exists is finally what we were expecting. Its mere presence introduces branching in the line of code this thread highlights.

If instead we had .Values.rbac.serviceAccount.create as a boolean defaulting to false, .Values.rbac.serviceAccount.name would always have the name of the mounted (when applicable) service account.

Do you believe this legibility tradeoff is enough to justify a breaking change? I find myself unable to make that judgment.

[aslafy-z]

I renamed enabled to create for it to be aligned with what is done in the community. I personally don't care about breaking changes if they are clearly indicated as such and major is bumped. I prefer to consistent and easily understandable APIs, over over-engineering for compatible solutions, even if it come with the cost of a breaking change. This is my personal take, I think it works for this chart. WDYT?

[d3adb5]

Edge case, may or may not be something to take into consideration: service accounts named null. Also, we're assuming the result of this partial template to be interpreted as YAML rather than a raw string, that may create confusion.

Suggested change

	{{- default "null" .Values.rbac.existingServiceAccountName }}
	{{- $saName := .Values.rbac.existingServiceAccountName }}
	{{- empty $saName | ternary (quote $saName) "null" }}

[aslafy-z]

the partial now only returns strings, do you think i should still quote the output?