Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(argo-rollouts): configurable automountServiceAccountToken #3032

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions charts/argo-rollouts/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v2
appVersion: v1.7.2
description: A Helm chart for Argo Rollouts
name: argo-rollouts
version: 2.37.8
version: 2.37.9
home: https://github.com/argoproj/argo-helm
icon: https://argoproj.github.io/argo-rollouts/assets/logo.png
keywords:
Expand All @@ -18,5 +18,5 @@ annotations:
fingerprint: 2B8F22F57260EFA67BE1C5824B11F800CD9D2252
url: https://argoproj.github.io/argo-helm/pgp_keys.asc
artifacthub.io/changes: |
- kind: fixed
description: Fixed rendering of plugins in the ConfigMap
- kind: changed
description: Add configurable automountServiceAccountToken
2 changes: 2 additions & 0 deletions charts/argo-rollouts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ For full list of changes please check ArtifactHub [changelog].
| podLabels | object | `{}` | Labels to be added to the Rollout pods |
| podSecurityContext | object | `{"runAsNonRoot":true}` | Security Context to set on pod level |
| serviceAccount.annotations | object | `{}` | Annotations to add to the service account |
| serviceAccount.automount | bool | `true` | Automount API credentials for the Service Account into the pod. |
| serviceAccount.create | bool | `true` | Specifies whether a service account should be created |
| serviceAccount.name | string | `""` | The name of the service account to use. If not set and create is true, a name is generated using the fullname template |
| serviceAnnotations | object | `{}` | Annotations to be added to the Rollout service |
Expand Down Expand Up @@ -191,6 +192,7 @@ For full list of changes please check ArtifactHub [changelog].
| dashboard.service.targetPort | int | `3100` | Service target port |
| dashboard.service.type | string | `"ClusterIP"` | Sets the type of the Service |
| dashboard.serviceAccount.annotations | object | `{}` | Annotations to add to the dashboard service account |
| dashboard.serviceAccount.automount | bool | `true` | Automount API credentials for the Service Account into the pod. |
| dashboard.serviceAccount.create | bool | `true` | Specifies whether a dashboard service account should be created |
| dashboard.serviceAccount.name | string | `""` | The name of the service account to use. If not set and create is true, a name is generated using the fullname template |
| dashboard.tolerations | list | `[]` | [Tolerations] for use with node taints |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ spec:
{{- toYaml . | nindent 8 }}
{{- end }}
serviceAccountName: {{ include "argo-rollouts.serviceAccountName" . }}
automountServiceAccountToken: {{ .Values.serviceAccount.automount }}
Copy link
Member

@mkilchhofer mkilchhofer Nov 20, 2024

Choose a reason for hiding this comment

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

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.

containers:
- image: "{{ .Values.controller.image.registry }}/{{ .Values.controller.image.repository }}:{{ default .Chart.AppVersion .Values.controller.image.tag }}"
args:
Expand Down
1 change: 1 addition & 0 deletions charts/argo-rollouts/templates/dashboard/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ spec:
{{- toYaml . | nindent 8 }}
{{- end }}
serviceAccountName: {{ include "argo-rollouts.serviceAccountName" . }}-dashboard
automountServiceAccountToken: {{ .Values.dashboard.serviceAccount.automount }}
Copy link
Member

Choose a reason for hiding this comment

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

Since we are setting the field on the kind: Deployment rather than on the kind: ServiceAccount can we use {{ .Values.dashboard.automountServiceAccountToken }} instead?

containers:
- image: "{{ .Values.dashboard.image.registry }}/{{ .Values.dashboard.image.repository }}:{{ default .Chart.AppVersion .Values.dashboard.image.tag }}"
imagePullPolicy: {{ .Values.dashboard.image.pullPolicy }}
Expand Down
4 changes: 4 additions & 0 deletions charts/argo-rollouts/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ serviceAccount:
# -- The name of the service account to use.
# If not set and create is true, a name is generated using the fullname template
name: ""
# -- Automount API credentials for the Service Account into the pod.
automount: true

# -- Annotations to be added to all CRDs
crdAnnotations: {}
Expand Down Expand Up @@ -384,6 +386,8 @@ dashboard:
# -- The name of the service account to use.
# If not set and create is true, a name is generated using the fullname template
name: ""
# -- Automount API credentials for the Service Account into the pod.
automount: true

## Configure Pod Disruption Budget for the dashboard
pdb:
Expand Down