In Helm Chart, change the default serviceAccount name from blank (auto detect) to tf-runner
chanwit opened this issue · 4 comments
Looking at the chart helpers, it looks like the ultimate default is derived from the chart name:
tf-controller.serviceAccountName
{{- define "tf-controller.serviceAccountName" -}}
...
{{- default (include "tf-controller.fullname" .) .Values.serviceAccount.name }}
...{{- define "tf-controller.fullname" -}}
{{- if .Values.fullnameOverride }}
{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- $name := default .Chart.Name .Values.nameOverride }}
...It looks to me like, when the rename of the chart happens it should take care of this SA name.
However the runner SA doesn't follow this, instead having a hard coded default value of tf-runner:
{{- define "tf-controller.runner.serviceAccountName" -}}
...
{{- default "tf-runner" .Values.runner.serviceAccount.name }}
...So, from what I can see, when/if the chart is renamed from tf-controller to tofu-controller, it should propagate though to the controller's SA.
The runner SA, current default tf-runner, won't be affected by the chart rename.
For consitency, should it be tofu-runner, as opposed to tf-runner?
@chanwit I'm not sure I follow this issue either, as currently it defaults to tf-runner already if it's left blank.
Is this issue mainly for the testing you did of the coderabbit ai reviewer? Or is it to change the default values.yaml to explicitly state the service account name?
It's just for testing. Please leave it as is.
The results from Code Rabbit are not very impressive. We won't adopt this for now.