nextcloud
Set `nextcloud.podSecurityContext.fsGroup` to `33` by default and allow users to configure it if needed.
Pull Request
Description of the change
Changes default fsGroup to be 33. In all apache and regular fpm, the www-data user is 33.
The www-data user is 82 in all alpine images (including nginx), so I added a note in the README about it.
Previously we relied on if the user was using nginx.enabled which is alpine by default, but using nginx:alpine doesn't mean you're using nextcloud:fpm-alpine.
Benefits
If someone uses image.flavor of apache or fpm, the fsGroup should be 33 by default.
Possible drawbacks
If you used an alpine image, you will now have to manually set nextcloud.podSecurityContext.fsGroup to be 82 in your values.yaml.
Applicable issues
- partially fixes #335
Additional information
Checklist
- [x] DCO has been signed off on the commit.
- [x] Chart version bumped in
Chart.yamlaccording to semver. - [x] (optional) Variables are documented in the README.md
just need to rebase to fix the commits, but this seems to be okay now I think
The name of the commit/PR is a bit misleading because fsGroup was 33 by default if using apache, so nothing changes for those cases.
Fixed the PR and can fix the commits. does this work?
Always podSecurityContext.fsGroup to 33, even if nginx is enabled
Happy to change it to whatever makes most sense. Also, thank you again for all your patience through this. It's been a doozy 😅
Perhaps it just doesn't like the git push --force-with-lease I did earlier? 🤔 It failed on listing changing, but passed on previous checks.
Getting this error still in the linting step in the github workflow:
Run ct lint --config ct.yaml
ct lint --config ct.yaml
shell: /usr/bin/bash -e {0}
env:
CT_CONFIG_DIR: /opt/hostedtoolcache/ct/v3.8.0/x86_64/etc
VIRTUAL_ENV: /opt/hostedtoolcache/ct/v3.8.0/x86_64/venv
Linting charts...
Error: failed linting charts: failed identifying charts to process: failed running process: exit status 128
------------------------------------------------------------------------------------------------------------------------
No chart changes detected.
------------------------------------------------------------------------------------------------------------------------
failed linting charts: failed identifying charts to process: failed running process: exit status 128
Error: Process completed with exit code 1.
maybe it has something to do with the fact that we changed the default branch from master to main, and when I opened this PR, it was against "master" 🤔 I could open a new PR and see if it persists? 🤷
seems to be happening on another PR as well, checking something, going to do a test commit here
ok, fixed the additional warnings in https://github.com/nextcloud/helm/pull/387 after testing here. Will undo the commit here where #387 is merged, then rebase again, and rerequest review after the GHA check is passing.
Yeah, Nextcloud uses a lot of GH actions and there is a max limit per org. I think some people are experimenting with adding our own runners to speed this up.
Oh, thank you for letting me know! Then I will not try to cancel and re-run them. Sorry about doing that earlier. I only tried to cancel and rerun one of them after it'd been over 15 minutes, because I thought there was something wrong with my own rate limiting, because I do a lot of stuff on github. 😅
I haven't merged this because I switched to using the image.flavor: fpm-alpine exclusively, which doesn't have the issues with at least mounting PostgreSQL SSL certs, and then I started using s3 for the primary object store, so I stopped having to deal with backups running as root for nextcloud's persistent volumes, because I'm not using them anymore, which means I can't do a final test to make sure this is solid. If this is breaking anyone else's flow and they really need it, we can still resolve the conflicts and merge it, but I can't easily test this at this moment. I'll close this for now, but if others like, they can ask that I either reopen this, or submit their own PR. Apologies for all the back and forth Kate :blue_heart:
ok, I have to begrudgingly reopen this because I want to solve https://github.com/nextcloud/helm/issues/367 but if this is not merged, then I cannot test using the fpm container with no nginx container.