[martin-mat]

Description of the change

The chart's service account is now created whenever rbac.serviceaccount.create is true (the default), independent of rbac.enabled, and the Nextcloud Deployment and the cron CronJob always run as it:

• templates/serviceaccount.yaml: rendered on rbac.serviceaccount.create alone.
• templates/_helpers.tpl: a nextcloud.serviceAccountName helper. The name is rbac.serviceaccount.name when set; otherwise the release's full name when the chart creates the account, as its other objects are named; with create: false and no name, the namespace's default account, as before.
• templates/deployment.yaml, templates/cronjob.yaml: serviceAccountName always set from the helper; templates/rbac.yaml: the RoleBinding subject from the same helper.
• values.yaml: rbac.serviceaccount.name defaults to "", with comments; README rows corrected; chart 9.4.1 → 9.5.0.

The PSP Role behind rbac.enabled is unchanged.

Benefits

The pods no longer run as the namespace's default service account, so:

• they don't share its permissions with every other pod in the namespace;
• rbac.serviceaccount.annotations works without rbac.enabled, for cloud identity (IRSA, Workload Identity) to S3 primary storage;
• audit entries name Nextcloud;
• CIS 5.1.5 and the CNTi Test Suite's dedicated_service_account test are satisfied.

Two releases in one namespace no longer collide on a fixed account name.

Possible drawbacks

• Installs that set rbac.enabled without a name used nextcloud-serviceaccount. They now get the release's full name as well (as asked in review, so two releases in one namespace don't collide): the chart creates that account, moves its RoleBinding to it, and the pods roll out once. To keep the old account, set rbac.serviceaccount.name: nextcloud-serviceaccount; with create: false that is required, as for any existing account.
• A rbac.serviceaccount.name set without rbac.enabled is now used, where the chart ignored it before; with create: false that account has to exist.

Applicable issues

• fixes Nextcloud pods run as the namespace's default service account: the chart's account is only created with rbac.enabled, which binds the removed PodSecurityPolicy #895

Additional information

Rebased onto main at fc36d83 (7 Nov, Nextcloud 35.0.2). Verified on a fork:

• All eight combinations of rbac.enabled × serviceaccount.create × serviceaccount.name rendered against main: a named account is used as given; otherwise the release's own account when the chart creates it, or default with create: false; the pods and the rbac.enabled RoleBinding always name the same account. helm lint on the defaults and all nine test-values sets.
• This repository's Lint and Test Charts workflow on GitHub-hosted runners: ct lint and all eight install cases pass (run 42389401742).
• Before the review change of 13 Nov, which renames the rbac.enabled installs' account: helm upgrade from main on kind, with the live Deployment pod template diffed before and after (run 42389401502): defaults get <release>-nextcloud and roll once (the diff is serviceAccountName alone); rbac.enabled, with and without create: false on a pre-created account, keep nextcloud-serviceaccount with an identical pod template and no rollout. Nextcloud answers on /status.php in all three.
• CNTi Test Suite (same run): dedicated_service_account fails on main and passes with this change; the essential tests are unchanged at 15 of 19.

Checklist

• I have read the CONTRIBUTING.md doc.
• DCO has been signed off on the commit.
• Chart version bumped in Chart.yaml according to semver.
• (optional) Parameters are documented in the README.md

🤖 Generated with Claude Code

[wrenix]

there ia no fallback to namespace default service-account. you drop rbac.enabled

[martin-mat]

Thanks, you're right on both counts, and the first version also failed the install with create: false and no name. Both are fixed in the rebased commit:

• With rbac.serviceaccount.create: false and no name, the pods run as the namespace's default account again, as before.
• Installs with rbac.enabled keep nextcloud-serviceaccount whether the chart creates it or an existing one is used. I had defaulted the name to the release's full name everywhere; that would have renamed the account of every install that set rbac.enabled, which is the only way to get a dedicated account today. Now the release's full name is the default only where there was no account before.

So rbac.enabled still controls the Role and RoleBinding for the PodSecurityPolicy, and installs that set it are unchanged: I rendered all eight combinations of rbac.enabled × create × name against main, and every rbac.enabled one is identical to main down to the full manifest. On kind, upgrading an rbac.enabled install (also with create: false on a pre-created account) from main leaves the pod template identical and rolls nothing; a default install gets <release>-nextcloud and rolls once. Details and run links in the description.

[wrenix]

Suggested change

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

[martin-mat]

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?

[martin-mat]

Done, in the nested form: with for the name, and the rest of the chain under its else. All eight combinations of rbac.enabled × serviceaccount.create × serviceaccount.name render the same as before.

[wrenix]

where this value come from?

[martin-mat]

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?

[martin-mat]

I kept it (option 1). The helper's comment now says where it comes from: it's the default rbac.serviceaccount.name had up to chart 9.4.1, kept so installs with rbac.enabled keep their account on upgrade. If you'd rather have option 2, a shared default in values.yaml, I'll switch.

[wrenix]

to avoid conflicts we should use {{ template "nextcloud.fullname" . }} like on every manifest.

[martin-mat]

Done: no more literal. The account is now rbac.serviceaccount.name when set, otherwise {{ template "nextcloud.fullname" . }} when the chart creates it, otherwise default, so it no longer depends on rbac.enabled.

That changes upgrades for installs with rbac.enabled and no name: their account moves from nextcloud-serviceaccount to the release's full name, the RoleBinding follows, and the pods roll out once. Setting rbac.serviceaccount.name: nextcloud-serviceaccount keeps the old one; with create: false a name is now needed. The commit message and the description say so. Rebased onto 9.4.3, chart 9.5.0.