feat(helm)!: separate worker serviceAccount.create from rbac.create (#…

…735) * feat(helm)!: separate worker serviceAccount.create from rbac.create Signed-off-by: Olivier Léobal <[email protected]> * improve polish Signed-off-by: Olivier Léobal <[email protected]> * Fix bugs in helm templates One in my PR, and one preexisting Signed-off-by: Olivier Léobal <[email protected]> * fix typo, update changelog Signed-off-by: Olivier Léobal <[email protected]> * chore: rebase charts changelog Signed-off-by: SdgJlbl <[email protected]> --------- Signed-off-by: Olivier Léobal <[email protected]> Signed-off-by: SdgJlbl <[email protected]> Co-authored-by: SdgJlbl <[email protected]>
Substra · Apr 4, 2024 · 4b10571 · 4b10571
1 parent f6798d4
commit 4b10571
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 14 deletions.
diff --git a/charts/substra-backend/CHANGELOG.md b/charts/substra-backend/CHANGELOG.md
@@ -2,8 +2,17 @@
 
 <!-- towncrier release notes start -->
 
-## [24.5.0] - 2024-03-27
+## [25.0.0] - 2024-04-03
+
+### Changed
 
+- BREAKING: Remove ServiceAccount creation from `worker.rbac.create`, putting it in a new `worker.serviceAccount.create` option
+- BREAKING: Some options that would result in service account names defaulting to "default" now raise an error at rendering time. Set the name to "default" by hand to reproduce the old behavior.
+- Add a new `worker.serviceAccount.name` option
+- Fix "api events" services using the "worker events" service account instead of the "api events" one
+
+
+## [24.5.0] - 2024-03-27
 
 ### Changed
 

diff --git a/charts/substra-backend/Chart.yaml b/charts/substra-backend/Chart.yaml
@@ -1,7 +1,7 @@
 apiVersion: v2
 name: substra-backend
 home: https://github.com/Substra
-version: 24.5.0
+version: 25.0.0
 appVersion: 0.45.0
 kubeVersion: ">= 1.19.0-0"
 description: Main package for Substra

diff --git a/charts/substra-backend/README.md b/charts/substra-backend/README.md
@@ -123,7 +123,9 @@ See [UPGRADE.md](https://github.com/Substra/substra-backend/blob/main/charts/sub
 | `worker.nodeSelector`                          | Node labels for pod assignment                                                                                                                     | `{}`                      |
 | `worker.tolerations`                           | Toleration labels for pod assignment                                                                                                               | `[]`                      |
 | `worker.affinity`                              | Affinity settings for pod assignment, ignored if `DataSampleStorageInServerMedia` is `true`                                                        | `{}`                      |
-| `worker.rbac.create`                           | Create a role and service account for the worker                                                                                                   | `true`                    |
+| `worker.rbac.create`                           | Create a role for the worker                                                                                                                       | `true`                    |
+| `worker.serviceAccount.create`                 | Create a service account for the worker                                                                                                            | `true`                    |
+| `worker.serviceAccount.name`                   | The name of the ServiceAccount to use. If not set and create is true, a name is generated using the substra.fullname template                      | `""`                      |
 | `worker.persistence.storageClass`              | Specify the _StorageClass_ used to provision the volume. Or the default _StorageClass_ will be used. Set it to `-` to disable dynamic provisioning | `""`                      |
 | `worker.persistence.size`                      | The size of the volume. The size of this volume should be sufficient to store many assets.                                                         | `10Gi`                    |
 | `worker.computePod.maxStartupWaitSeconds`      | Set the maximum amount of time we will wait for the compute pod to be ready                                                                        | `300`                     |

diff --git a/charts/substra-backend/UPGRADE.md b/charts/substra-backend/UPGRADE.md
@@ -1,5 +1,11 @@
 ## unreleased
 
+## 25.0.0
+
+If you have set `worker.rbac.create=false`, you now need to also set `worker.serviceAccount.create=false` to preserve the same behavior.
+
+If you have set any of `worker.serviceAccount.create`, `worker.events.serviceAccount.create`, or `api.events.serviceAccount.create` to `false` without explicitely telling Substra how the corresponding externally-created ServiceAccount is named (this is very unlikely), it defaulted to `default`, which you now need to set by hand through `{...}.serviceAccount.name`.
+
 ## 23.0.0
 
 Database should be dumped and restored as the `postgresql` sub-chart version incremented. You can follow [Upgrading a PostgreSQL Cluster](https://www.postgresql.org/docs/16/upgrading.html) documentation for further detail

diff --git a/charts/substra-backend/templates/_helpers.tpl b/charts/substra-backend/templates/_helpers.tpl
@@ -140,25 +140,49 @@ Return the user list
 {{- end }}
 {{- end -}}
 
+
+{{/*
+    Create the name for the service account to use for the worker
+*/}}
+{{- define "substra.worker.serviceAccountName" -}}
+{{- if .Values.worker.serviceAccount.create -}}
+    {{ default (printf "%s-worker" ( include "substra.fullname" .)) .Values.worker.serviceAccount.name }}
+{{- else -}}
+    {{- if .Values.worker.serviceAccount.name -}}
+        {{ .Values.worker.serviceAccount.name }}
+    {{-  else -}}
+        {{ fail "if worker.serviceAccount.create is false, worker.serviceAccount.name must be given" }}
+    {{- end -}}
+{{- end -}}
+{{- end -}}
+
 {{/*
-    Create the name fo the service account to use for the worker event app
+    Create the name for the service account to use for the worker event app
 */}}
 {{- define "substra.worker.events.serviceAccountName" -}}
 {{- if .Values.worker.events.serviceAccount.create -}}
     {{ default (printf "%s-event" ( include "substra.fullname" .)) .Values.worker.events.serviceAccount.name }}
 {{- else -}}
-    {{ default "default" .Values.worker.events.serviceAccount.name }}
+    {{- if .Values.worker.events.serviceAccount.name -}}
+        {{ .Values.worker.events.serviceAccount.name }}
+    {{-  else -}}
+        {{ fail "if worker.events.serviceAccount.create is false, worker.events.serviceAccount.name must be given" }}
+    {{- end -}}
 {{- end -}}
 {{- end -}}
 
 {{/*
-    Create the name fo the service account to use for the api event app
+    Create the name for the service account to use for the api event app
 */}}
 {{- define "substra.api.events.serviceAccountName" -}}
 {{- if .Values.api.events.serviceAccount.create -}}
-    {{ default (printf "%s-event" ( include "substra.fullname" .)) .Values.api.events.serviceAccount.name }}
+    {{ default (printf "%s-api-event" ( include "substra.fullname" .)) .Values.api.events.serviceAccount.name }}
 {{- else -}}
-    {{ default "default" .Values.api.events.serviceAccount.name }}
+    {{- if .Values.api.events.serviceAccount.name -}}
+        {{ .Values.api.events.serviceAccount.name }}
+    {{-  else -}}
+        {{ fail "if api.events.serviceAccount.create is false, api.events.serviceAccount.name must be given" }}
+    {{- end -}}
 {{- end -}}
 {{- end -}}
 

diff --git a/charts/substra-backend/templates/rbac.yaml b/charts/substra-backend/templates/rbac.yaml
@@ -1,12 +1,15 @@
-{{- if .Values.worker.rbac.create }}
+{{- if .Values.worker.serviceAccount.create }}
 ---
 apiVersion: v1
 kind: ServiceAccount
 metadata:
-  name: {{ template "substra.fullname" . }}-worker
+  name: {{ include "substra.worker.serviceAccountName" . }}
   labels:
     {{ include "substra.labels" . | nindent 4 }}
     app.kubernetes.io/name: {{ template "substra.name" . }}
+{{- end }}
+
+{{- if .Values.worker.rbac.create }}
 ---
 kind: Role
 apiVersion: rbac.authorization.k8s.io/v1
@@ -38,7 +41,7 @@ metadata:
     app.kubernetes.io/name: {{ template "substra.name" . }}
 subjects:
   - kind: ServiceAccount
-    name: {{ template "substra.fullname" . }}-worker
+    name: {{ include "substra.worker.serviceAccountName" . }}
 roleRef:
   kind: Role
   name: {{ template "substra.fullname" . }}-worker
@@ -50,11 +53,12 @@ roleRef:
 apiVersion: v1
 kind: ServiceAccount
 metadata:
-  name: {{ template "substra.fullname" . }}-event
+  name: {{ include "substra.worker.events.serviceAccountName" . }}
   labels:
     {{ include "substra.labels" . | nindent 4 }}
     app.kubernetes.io/name: {{ template "substra.name" . }}
 {{- end -}}
+
 {{- if .Values.worker.events.rbac.create }}
 ---
 kind: Role
@@ -84,6 +88,7 @@ roleRef:
   name: {{ template "substra.fullname" . }}-worker-event
   apiGroup: rbac.authorization.k8s.io
 {{- end }}
+
 {{- if .Values.api.events.serviceAccount.create }}
 ---
 apiVersion: v1
@@ -94,12 +99,13 @@ metadata:
     {{ include "substra.labels" . | nindent 4 }}
     app.kubernetes.io/name: {{ template "substra.name" . }}
 {{- end -}}
+
 {{- if .Values.api.events.rbac.create }}
 ---
 kind: Role
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
-  name: {{ template "substra.fullname" . }}-api-event
+  name: {{ include "substra.api.events.serviceAccountName" . }}
   labels:
     {{ include "substra.labels" . | nindent 4 }}
     app.kubernetes.io/name: {{ template "substra.name" . }}

diff --git a/charts/substra-backend/values.yaml b/charts/substra-backend/values.yaml
@@ -315,10 +315,17 @@ worker:
   ## @param worker.affinity Affinity settings for pod assignment, ignored if `DataSampleStorageInServerMedia` is `true`
   ##
   affinity: {}
-  ## @param worker.rbac.create Create a role and service account for the worker
+  ## @param worker.rbac.create Create a role for the worker
   ##
   rbac:
     create: true
+  serviceAccount:
+    ## @param worker.serviceAccount.create Create a service account for the worker
+    ##
+    create: true
+    ## @param worker.serviceAccount.name The name of the ServiceAccount to use. If not set and create is true, a name is generated using the substra.fullname template
+    ##
+    name: ""
   ## @param worker.persistence.storageClass Specify the _StorageClass_ used to provision the volume. Or the default _StorageClass_ will be used. Set it to `-` to disable dynamic provisioning
   ## @param worker.persistence.size The size of the volume. The size of this volume should be sufficient to store many assets.
   ##