feat(argo-rollouts): configurable automountServiceAccountToken #3032
Conversation
k-cogswell
force-pushed
the
feat/configurable-automountServiceAccountToken
branch
from
a0706de to
92d7cd0
Compare
k-cogswell
changed the title
charts/argo-rollouts/values.yaml
Outdated
| # -- Specifies wether a service account should be automounted | ||
| automount: true |
There was a problem hiding this comment.
Regarding the comment and default value, please align to #2625 .
Reason of the default value false is also mentioned in the PR.
There was a problem hiding this comment.
@yu-croco The comments and default values have been updated. Thank you for the feedback.
There was a problem hiding this comment.
@yu-croco looking at the referenced PR again, the default value is set to true.
You can also see in this in main
argo-helm/charts/argo-cd/values.yaml
Line 834 in 9361018
If you're okay with this I can revert my values file back to the original value of true,
There was a problem hiding this comment.
Yes, lets use true as the default. Otherwise it would be a breaking change.
There was a problem hiding this comment.
My bad... 🫠 Please revert as true .
There was a problem hiding this comment.
Updated! Thanks again for your feedback @yu-croco @mkilchhofer
k-cogswell
force-pushed
the
feat/configurable-automountServiceAccountToken
branch
2 times, most recently
from
285f4a2 to
401f991
Compare
Signed-off-by: Kyle Cogswell <kcogswell26@gmail.com>
k-cogswell
force-pushed
the
feat/configurable-automountServiceAccountToken
branch
from
1ea9b3d to
e7da93b
Compare
| @@ -44,6 +44,7 @@ spec: | |||
| {{- toYaml . | nindent 8 }} | |||
| {{- end }} | |||
| serviceAccountName: {{ include "argo-rollouts.serviceAccountName" . }} | |||
| automountServiceAccountToken: {{ .Values.serviceAccount.automount }} | |||
There was a problem hiding this comment.
Since we are setting the field on the kind: Deployment rather than on the kind: ServiceAccount can we use {{ .Values.controller.automountServiceAccountToken }} instead?
Background: I'd like to be in line with every chart we provide and this is already implemented in the argo-cd Helm chart.
| @@ -45,6 +45,7 @@ spec: | |||
| {{- toYaml . | nindent 8 }} | |||
| {{- end }} | |||
| serviceAccountName: {{ include "argo-rollouts.serviceAccountName" . }}-dashboard | |||
| automountServiceAccountToken: {{ .Values.dashboard.serviceAccount.automount }} | |||
There was a problem hiding this comment.
Since we are setting the field on the kind: Deployment rather than on the kind: ServiceAccount can we use {{ .Values.dashboard.automountServiceAccountToken }} instead?
Inspired by #2625
Adding a configurable option for
automountServiceAccountTokenin argo-rollouts chart. This is my first contribution so please let me know if I am missing anything.Checklist: