Skip to content

fix(nextcloud): mount defaultConfigs without requiring nextcloud.configs - #888

Open
JanWelker wants to merge 1 commit into
nextcloud:mainfrom
JanWelker:fix/mount-default-configs
Open

JanWelker wants to merge 1 commit into
nextcloud:mainfrom
JanWelker:fix/mount-default-configs

Conversation

@JanWelker

Copy link
Copy Markdown

Description of the change

templates/config.yaml, the nextcloud-config volume in deployment.yaml and cronjob.yaml, and the defaultConfigs loop in nextcloud.volumeMounts were all gated on nextcloud.configs being non-empty. On a default install every enabled nextcloud.defaultConfigs entry was therefore ignored.

This PR adds one helper, nextcloud.configs.enabled, that is true when nextcloud.configs holds a file or any defaultConfigs entry is enabled, and uses it in all four places. The ConfigMap is still skipped entirely when both are empty.

Also: Chart.yaml bumped to 9.3.1, the outdated "Will be used only if you put extra configs" comment in values.yaml replaced, and README rows added for the two defaultConfigs entries that were undocumented (helm-metrics.config.php, imaginary.config.php).

Benefits

  • nextcloud.openmetrics.allowedClients works on its own. Its only consumer is helm-metrics.config.php, which never reached the pod unless an unrelated nextcloud.configs entry was set, so Nextcloud kept its built-in 127.0.0.1 and refused Prometheus with 403. The chart's own metrics.enabled + prometheus.serviceMonitor.enabled defaults produced a permanently down target for the same reason.
  • The documented Imaginary setup (nextcloud.defaultConfigs.imaginary.config.php: true) works without the extra nextcloud.configs workaround (Cannot enable nextcloud.defaultConfigs without also setting nextcloud.configs #761).
  • defaultConfigs now means what the values file says it means.

Possible drawbacks

  • On upgrade, every install now gets the ConfigMap and a mount per enabled default. That is the same set of files that already mounted for anyone with nextcloud.configs set, but it is new for default installs, and the existing nextcloud-config-hash annotation restarts the pod once.
  • Anyone who edited one of the image's config files in place on the PVC will see it shadowed by the chart copy. Setting that defaultConfigs.<file>: false restores the PVC file.
  • The chart copies now shadow the image copies for everyone, so they need to track nextcloud/docker. Today they are identical except s3.config.php, which lacks the image's SSE-KMS block (OBJECTSTORE_S3_SSE_KMS_ENABLED / OBJECTSTORE_S3_SSE_KMS_KEY_ID). Happy to send that sync as a separate PR.

The all-enabled behaviour is intentional rather than mounting only the two chart-only files: the image entrypoint seeds /var/www/html/config from /usr/src/nextcloud/config only when the directory is empty (docker-entrypoint.sh), so mounting any single file into it on a fresh install would suppress the image defaults. Mounting all of them is the existing design for that case; this PR just applies it consistently.

Applicable issues

Additional information

Verified with helm lint and helm template:

  • default values: ConfigMap renders with the 12 enabled defaults, 12 mounts in the app container, helm-metrics.config.php among them
  • all defaultConfigs false and configs: {}: no ConfigMap, no volume, no mounts
  • defaultConfigs all false with one configs entry: ConfigMap and mount for that entry only
  • every file in test-values/ renders; imaginary.yaml mounts imaginary.config.php
  • cron-cronjob.yaml: the CronJob pod gets the volume too

Checklist

The config ConfigMap and its volume were only rendered when
nextcloud.configs held at least one file. Every enabled
nextcloud.defaultConfigs entry was therefore ignored on a default
install. That silently broke nextcloud.openmetrics.allowedClients,
whose only consumer is helm-metrics.config.php, and the documented
imaginary.config.php setup. Neither file exists in the nextcloud
image, so there is no fallback copy.

Render and mount the ConfigMap whenever nextcloud.configs is non-empty
or any defaultConfigs entry is enabled. Because the image entrypoint
only seeds /var/www/html/config when it is empty, every enabled default
mounts together, the same set that already mounted whenever
nextcloud.configs was set.

Fixes nextcloud#887
Fixes nextcloud#761

Signed-off-by: Jan Welker <jan@wlkr.ch>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant