Skip to content

fix(nextcloud): run the pods as the chart's own service account by default #896

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.

Sign up for GitHub

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

Open
martin-mat wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(nextcloud): run the pods as the chart's own service account by default #896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion charts/nextcloud/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apiVersion: v2
name: nextcloud
version: 9.4.1
version: 9.5.0
# renovate: image=docker.io/library/nextcloud
appVersion: 35.0.2
description: A file sharing server that puts the control and security of your own data back into your hands.
Expand Down
6 changes: 3 additions & 3 deletions charts/nextcloud/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,9 @@ The following table lists the configurable parameters of the nextcloud chart and
| `phpClientHttpsFix.protocol` | Sets OVERWRITEPROTOCOL for https ingress redirect | `https` |
| `resources` | CPU/Memory resource requests/limits | `{}` |
| `priorityClassName` | Priority Class for pods | `nil` |
| `rbac.enabled` | Enable Role and rolebinding for priveledged PSP | `false` |
| `rbac.serviceaccount.create` | Wether to create a serviceaccount or use an existing one (requires rbac) | `true` |
| `rbac.serviceaccount.name` | The name of the sevice account that the deployment will use (requires rbac) | `nextcloud-serviceaccount` |
| `rbac.enabled` | Enable Role and rolebinding for priveledged PSP (removed in Kubernetes 1.25) | `false` |
| `rbac.serviceaccount.create` | Whether to create the serviceaccount the Nextcloud pods run as, or use an existing one | `true` |
| `rbac.serviceaccount.name` | Name of the serviceaccount; defaults to the release's full name (`nextcloud-serviceaccount` with `rbac.enabled`, as before), or to `default` when `create` is false | `""` |
| `rbac.serviceaccount.annotations` | Serviceaccount annotations | `{}` |
| `hpa.enabled` | Boolean to create a HorizontalPodAutoscaler. If set to `true`, ignores `replicaCount`. | `false` |
| `hpa.cputhreshold` | CPU threshold percent for the HorizontalPodAutoscale | `60` |
Expand Down
19 changes: 19 additions & 0 deletions charts/nextcloud/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,3 +481,22 @@ app.kubernetes.io/managed-by: {{ .rootContext.Release.Service }}
app.kubernetes.io/version: {{ quote . }}
{{- end }}
{{- end -}}

{{/*
The service account the Nextcloud pods run as: rbac.serviceaccount.name when
set; else nextcloud-serviceaccount when rbac.enabled is set, the previous
default, so installs that enabled rbac keep their account; else the release's
full name when the chart creates the account; else the namespace's default
account.
*/}}
{{- define "nextcloud.serviceAccountName" -}}
{{- if .Values.rbac.serviceaccount.name -}}

@wrenix wrenix Oct 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{{- if .Values.rbac.serviceaccount.name -}}
{{- with .Values.rbac.serviceaccount.name -}}
{{ . }}

@martin-mat martin-mat Oct 23, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks. One thing before I change it: the rest of the helper is an else if chain, and in Go templates with can only be followed by else or else with, not else if, so the suggestion as written doesn't parse (parse error at (nextcloud/templates/_helpers.tpl:495): unexpected <if> in input, helm v4.3.1). Using with would mean nesting the rest of the chain under an else, one level deeper:

{{- with .Values.rbac.serviceaccount.name -}}
{{ . }}
{{- else -}}
{{- if .Values.rbac.enabled -}}
nextcloud-serviceaccount
{{- else if .Values.rbac.serviceaccount.create -}}
{{- include "nextcloud.fullname" . -}}
{{- else -}}
default
{{- end -}}
{{- end -}}

Both forms render identically for all eight combinations of rbac.enabled × create × name (run). I find the flat if chain easier to read; would you like to keep it, or should I switch to the nested with?

{{- .Values.rbac.serviceaccount.name -}}
{{- else if .Values.rbac.enabled -}}
nextcloud-serviceaccount

@wrenix wrenix Oct 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

where this value come from?

@martin-mat martin-mat Oct 23, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's the default rbac.serviceaccount.name had in values.yaml until this change (name: nextcloud-serviceaccount, line 1104 on main). This PR sets that default to "" so that a new install gets its own <release>-nextcloud account, but an install that set rbac.enabled: true and kept the default name has always run as nextcloud-serviceaccount; without the literal here, upgrading it would rename its service account and roll the pods. So it's there for those installs only.

Two ways to go from here:

  1. Keep it, with a line in the helper's comment saying where the name comes from.
  2. Keep nextcloud-serviceaccount as the default in values.yaml and use it for every install: no name in the helper, but every release in a namespace shares one account name, so two releases there would collide.

Which do you prefer?

{{- else if .Values.rbac.serviceaccount.create -}}
{{- include "nextcloud.fullname" . -}}
{{- else -}}
default
{{- end -}}
{{- end -}}
4 changes: 1 addition & 3 deletions charts/nextcloud/templates/cronjob.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,7 @@ spec:
fsGroup: 33
{{- end }}
{{- end }}{{/* end-with podSecurityContext */}}
{{- if .Values.rbac.enabled }}
serviceAccountName: {{ .Values.rbac.serviceaccount.name }}
{{- end }}
serviceAccountName: {{ include "nextcloud.serviceAccountName" $ }}
{{- with .Values.dnsConfig }}
dnsConfig:
{{- toYaml . | nindent 12 }}
Expand Down
4 changes: 1 addition & 3 deletions charts/nextcloud/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,9 +425,7 @@ spec:
fsGroup: 33
{{- end }}
{{- end }}{{/* end-with podSecurityContext */}}
{{- if .Values.rbac.enabled }}
serviceAccountName: {{ .Values.rbac.serviceaccount.name }}
{{- end }}
serviceAccountName: {{ include "nextcloud.serviceAccountName" . }}
{{- with .Values.dnsConfig }}
dnsConfig:
{{- toYaml . | nindent 8 }}
Expand Down
2 changes: 1 addition & 1 deletion charts/nextcloud/templates/rbac.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ roleRef:
name: {{ template "nextcloud.fullname" . }}-privileged
subjects:
- kind: ServiceAccount
name: {{ .Values.rbac.serviceaccount.name }}
name: {{ include "nextcloud.serviceAccountName" . }}
namespace: {{ .Release.Namespace }}
{{- end }}
4 changes: 2 additions & 2 deletions charts/nextcloud/templates/serviceaccount.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{{- if and .Values.rbac.enabled .Values.rbac.serviceaccount.create }}
{{- if .Values.rbac.serviceaccount.create }}
---
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ .Values.rbac.serviceaccount.name }}
name: {{ include "nextcloud.serviceAccountName" . }}
{{- with .Values.rbac.serviceaccount.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
Expand Down
10 changes: 9 additions & 1 deletion charts/nextcloud/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1098,10 +1098,18 @@ prometheus:
additionalRules: []

rbac:
# Role and RoleBinding for the privileged PodSecurityPolicy (removed in Kubernetes 1.25)
enabled: false
# The service account the Nextcloud pods run as, created by the chart by
# default so they don't share the namespace's default account
serviceaccount:
# Create the service account; when false, the pods run as `name`, or as
# the namespace's default account if it is empty
create: true
name: nextcloud-serviceaccount
# Defaults to the release's full name. Installs with rbac.enabled keep the
# previous default, nextcloud-serviceaccount, so their account does not
# change on upgrade
name: ""
annotations: {}

## @param securityContext for nextcloud pod @deprecated Use `nextcloud.podSecurityContext` instead
Expand Down