fix: Mount the writable emptyDir file into the nextcloud container directly - #879
Open
edvinas31 wants to merge 2 commits into
Open
fix: Mount the writable emptyDir file into the nextcloud container directly#879edvinas31 wants to merge 2 commits into
edvinas31 wants to merge 2 commits into
Conversation
…rectly Signed-off-by: esulzickis <esulzickis@toogoodtogo.com>
…rectly Signed-off-by: esulzickis <esulzickis@toogoodtogo.com>
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
Mount the writable php-confd emptyDir over /usr/local/etc/php/conf.d/redis-session.ini in the main nextcloud container, guarded by .Values.redis.enabled.
When redis.enabled=true the chart already creates everything needed for this:
a php-confd emptyDir volume (deployment.yaml, volumes section), an init-redis-session-ini init container that mounts that volume at /usr/local/etc/php/conf.d and touches redis-session.ini into it, using the Nextcloud image so the file is owned by the right UID, and a subPath mount of that file into the nextcloud-cron sidecar.
The one container that actually needs it - the main nextcloud container, whose entrypoint writes the file - was never given the mount. This adds it, mirroring the existing block in the cron sidecar.
Benefits
Fixes /entrypoint.sh: cannot create /usr/local/etc/php/conf.d/redis-session.ini: Permission denied when running the main container as non-root (e.g. nextcloud.securityContext.runAsUser: 33) with Redis enabled. /usr/local/etc/php/conf.d inside the image is root-owned, so the entrypoint's "Configuring Redis as session handler" step fails for any non-root user; the emptyDir the chart already provisions makes the single file writable without shadowing the rest of conf.d.
Users currently have to work around this in their own values with:
which reaches into a chart-internal volume name. That workaround becomes unnecessary.
Possible drawbacks
Anyone using the extraVolumeMounts workaround above will get a duplicate mountPath on the nextcloud container, which the API server rejects, until they remove it from their values. Worth a release note.
The mount stays gated on .Values.redis.enabled, consistent with the volume and the init container. externalRedis.enabled=true sets REDIS_HOST too, so it hits the same entrypoint path and the same permission error, but the volume and init container don't exist in that case - fixing external Redis means widening all four conditions and is left out of this PR deliberately.
Applicable issues
relates to #187 - the original fix for this error; it covered the cron sidecar but not the main container
relates to #814 - same error reported on RKE2 with a non-root securityContext (closed without a chart-side fix)
Additional information
Verified with helm template on the local chart:
redis.enabled=true, cronjob.enabled=true, nextcloud.securityContext.runAsUser=33 → the mount renders exactly once in the nextcloud container and once in nextcloud-cron; no duplicate mountPath in any container
redis.enabled=false → no php-confd references anywhere in the rendered Deployment
helm lint passes
No new or changed values, so no README changes are needed.
Checklist
Chart.yamlaccording to semver.