Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of the change
templates/config.yaml, thenextcloud-configvolume indeployment.yamlandcronjob.yaml, and thedefaultConfigsloop innextcloud.volumeMountswere all gated onnextcloud.configsbeing non-empty. On a default install every enablednextcloud.defaultConfigsentry was therefore ignored.This PR adds one helper,
nextcloud.configs.enabled, that is true whennextcloud.configsholds a file or anydefaultConfigsentry is enabled, and uses it in all four places. The ConfigMap is still skipped entirely when both are empty.Also:
Chart.yamlbumped to 9.3.1, the outdated "Will be used only if you put extra configs" comment invalues.yamlreplaced, and README rows added for the twodefaultConfigsentries that were undocumented (helm-metrics.config.php,imaginary.config.php).Benefits
nextcloud.openmetrics.allowedClientsworks on its own. Its only consumer ishelm-metrics.config.php, which never reached the pod unless an unrelatednextcloud.configsentry was set, so Nextcloud kept its built-in127.0.0.1and refused Prometheus with 403. The chart's ownmetrics.enabled+prometheus.serviceMonitor.enableddefaults produced a permanently down target for the same reason.nextcloud.defaultConfigs.imaginary.config.php: true) works without the extranextcloud.configsworkaround (Cannot enablenextcloud.defaultConfigswithout also settingnextcloud.configs#761).defaultConfigsnow means what the values file says it means.Possible drawbacks
nextcloud.configsset, but it is new for default installs, and the existingnextcloud-config-hashannotation restarts the pod once.defaultConfigs.<file>: falserestores the PVC file.nextcloud/docker. Today they are identical excepts3.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/configfrom/usr/src/nextcloud/configonly 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
nextcloud.defaultConfigswithout also settingnextcloud.configs#761Additional information
Verified with
helm lintandhelm template:helm-metrics.config.phpamong themdefaultConfigsfalse andconfigs: {}: no ConfigMap, no volume, no mountsdefaultConfigsall false with oneconfigsentry: ConfigMap and mount for that entry onlytest-values/renders;imaginary.yamlmountsimaginary.config.phpcron-cronjob.yaml: the CronJob pod gets the volume tooChecklist
Chart.yamlaccording to semver.