Add occ and occ-cron scripts

[PhrozenByte]

occ and occ-cron are simple wrapper scripts allowing server admins (or are they called "container admins" then? 😄) to easily run occ within the container.

The occ script makes things just way easier: Right now an admin has to remember to switch to the www-data user first and then use clumsy php -f /var/www/html/occ -- status calls to get the job done. Just calling occ status makes life significantly easier. The old way of doing things works as before, so it's fully backwards compatible.

[PhrozenByte]

Looks like CI is broken due to some runtime issues with the CI env right now. Will try again later...

[PhrozenByte]

CI is back up again and all tests are passing (even though not fully stable, still requiring re-runs... 😒). Ready for review and merge. 👍

[meonkeys]

I'm not clear on the purpose/intent. Are you exec'ing into the container often (I don't)? Or is this really just to shorten php /var/www/html/occ to occ inside entrypoint.sh and friends?

TBH, I don't see any end-user benefit, and I'm not sure there's a benefit to code maintenance. Do we want this change?

FWIW, I just use a script like this on the host:

#!/bin/bash

set -o errexit
set -o nounset
set -o pipefail

if [[ -t 1 ]]; then
    ttyargs='--interactive --tty'
else
    ttyargs=''
fi

sudo docker exec $ttyargs --user www-data nextcloud_app_1 php occ "$@"

Then as a user with sudo / sudo docker privileges, I can run, e.g. occ status.

[PhrozenByte]

I'm not clear on the purpose/intent. Are you exec'ing into the container often (I don't)?

Depends on what you consider "often". I do exec into Nextcloud containers from time to time to use occ for certain tasks, yes. occ exists for a reason. If there would be no use case for occ, it wouldn't exist after all. And I wouldn't have created these scripts and opened this PR.

Or is this really just to shorten php /var/www/html/occ to occ inside entrypoint.sh and friends?

It's a side-effect.

TBH, I don't see any end-user benefit, and I'm not sure there's a benefit to code maintenance. Do we want this change?

The end-user benefit is to make using occ easier.

To be honest, I'm a bit baffled that you claim that there's no end-user benefit. There certainly is one. You might argue that the benefit is tiny, because you assume that the majority of users (maybe even everyone except me) uses occ the way you do, but even then, there's a benefit. You might be right, who knows - the point is, we both don't know how people use it, it's all just wild guesses.

The question isn't whether there's a benefit, there is. The question is whether there are negative consequences that would counteract this benefit. A common negative consequence could be increased code maintenance. However, since having hard-coded paths in multiple places is an anti-pattern, I'd argue that it actually decreases code maintenance. Is it decreasing code maintenance by a lot? Certainly not. But still.

So, if you see any negative consequences, please go ahead. I'm very open for any discussion to find the best solution.

FWIW, I just use a script like this on the host:

So, you acknowledge that using occ is hard enough for end-users that it requires a solution, came up with a different one than me, but deny that mine has any benefit? 😕

I don't run a single Nextcloud container, but multiple. I don't even use Docker, but Podman and run all my containers as different unprivileged users. I even switch Nextcloud containers from one server to the other from time to time. Adding a host script would require me to replicate my container orchestration in a static script on the host. That's not practical. The whole idea of containerization is that it doesn't matter on which host the container runs. Expecting changes on the host is an anti-pattern.

[meonkeys]

occ exists for a reason. If there would be no use case for occ, it wouldn't exist after all.

No debate there... I was trying to better understand the intent of your patch, not occ itself.

we both don't know how people use it, it's all just wild guesses.

Agreed. Maybe we should poll users or continue to workshop this a bit? Or at least see what other maintainers/contributors/users think about it? Or just press on... I don't have a strong opinion. It sounds like you do though, and that's cool. Go for it.

With your last and most helpful paragraph I think I'm hearing a very different use case than mine. I manage only one Nextcloud server instance, I use Docker, and I rarely run occ manually.

So, if you see any negative consequences, please go ahead.

I don't see any negative consequences. Or positive ones (for me), honestly. Please understand I'm acting in good faith, driven by curiosity, and I'm just speaking to my own use case. It is entirely possible I'm just failing to get the point, so don't mind me, really. I'm not an official maintainer or anything.

Anyway, I'll try add some detail about my use case. Let's see if we can make some progress.

I rarely if ever exec into any containers. I'm just one user/admin speaking for myself, but I'd say it's best to stay out of containers (avoid manual interaction) as much as possible. Would you agree? occ is an exception since I sometimes must use it as a Nextcloud admin.

I get that you're making it possible to run occ instead of php occ, but the longer example is not something I've used, it only appears in scripts in the container.

This occ path change is basically a no-op for me because I always run sudo docker exec --user www-data -it nextcloud_app_1 php occ from outside the container, so with your change I'd run sudo docker exec --user www-data -it nextcloud_app_1 occ instead and save 4 characters. So I guess what I'm saying is this doesn't make occ any easier for me.

I also don't need to run php -f /var/www/html/occ -- status per your original description, just php occ status works fine. Must you use the longer version? Why?

And--perhaps I'm being super controversial here--I'm not sure shorter/relative paths are categorically better. Absolute paths are more specific, arguably more secure, and often higher maintenance. Yet, just about as easy to search and replace with a regex. Heck, maybe the path to php should be absolute.

I'm very open for any discussion to find the best solution.

Cool. Same here, I always appreciate collaboration! I hope this is helpful.

So, you acknowledge that using occ is hard enough for end-users that it requires a solution, came up with a different one than me, but deny that mine has any benefit? 😕

Hmm. Well, maybe I missed something, but it looks to me like your changes are entirely in-container, so my kludgy host-side workaround serves a different purpose: simplification outside the container. With your patch I'd still have to exec in per my examples above, and I still have to specify the user is www-data.

I use Docker (for now). Maybe with Podman your patch brings other benefits?

I don't run a single Nextcloud container, but multiple. I don't even use Docker, but Podman and run all my containers as different unprivileged users. I even switch Nextcloud containers from one server to the other from time to time. Adding a host script would require me to replicate my container orchestration in a static script on the host. That's not practical. The whole idea of containerization is that it doesn't matter on which host the container runs. Expecting changes on the host is an anti-pattern.

Agreed, I don't like it either. This is helpful info, thanks. Again, our use cases our likely different. How do you exec into your Nextcloud containers? Is the webserver in the container still running as www-data, or does that change as well? I'm not familiar with Podman and I don't use Docker namespaces (yet).

[PhrozenByte]

I rarely if ever exec into any containers. I'm just one user/admin speaking for myself, but I'd say it's best to stay out of containers (avoid manual interaction) as much as possible. Would you agree? occ is an exception since I sometimes must use it as a Nextcloud admin.

I got nothing to add here: occ must be used from time to time.

I get that you're making it possible to run occ instead of php occ, but the longer example is not something I've used, it only appears in scripts in the container.

This occ path change is basically a no-op for me because I always run sudo docker exec --user www-data -it nextcloud_app_1 php occ from outside the container, so with your change I'd run sudo docker exec --user www-data -it nextcloud_app_1 occ instead and save 4 characters. So I guess what I'm saying is this doesn't make occ any easier for me.

I also don't need to run php -f /var/www/html/occ -- status per your original description, just php occ status works fine. Must you use the longer version? Why?

• php occ status requires that your CWD always is /var/www/html
• php occ status requires that you remembered exec'ing into the container using --user www-data
• php occ status requires that you don't also need root permissions within the container (e.g. to copy files)
• php occ status requires php not to change their CLI option parser again (php occ --help would have printed PHP's help message in older PHP versions)

Many requirements that make using occ unnecessary hard. Not sure why we should make an admin's life harder than necessary... Plus it improves code maintenance.

Hmm. Well, maybe I missed something, but it looks to me like your changes are entirely in-container, so my kludgy host-side workaround serves a different purpose: simplification outside the container. With your patch I'd still have to exec in per my examples above, and I still have to specify the user is www-data.

Please check docker-occ.sh again, it doesn't require you to switch to www-data - it will do that for you.

Agreed, I don't like it either. This is helpful info, thanks. Again, our use cases our likely different. How do you exec into your Nextcloud containers?

Just a plain simple podman exec -it uXXX ash.

Is the webserver in the container still running as www-data, or does that change as well?

I'm not using the apache variant, i.e. the webserver is running in a different container. Not sure why this matters though.

[PhrozenByte]

Rebased and added some more code simplifications by removing the no longer needed run_as() function. I dropped 1733e21 in favour of #2115. CI again required many re-runs due to failing jobs caused by hickups and unavailable services...

[J0WI]

Thanks for your suggestion, however I think it's out-of-scope to add another wrapper here. For your own use-case, you can easily adopt the script and prefix it with docker exec -u ... or mount it info the container. Within the container ./occ status works already, in it's default configuration it's not necessary to specify the full path.
IMHO the wrapper adds another layer of complexity and may confuse users with special edge cases even more if something is not working for them.

[PhrozenByte]

For your own use-case, you can easily adopt the script and prefix it with docker exec -u ... or mount it info the container.

As explained earlier, requiring changes on the host defies the purpose of containerization.

Stating that one could mount the script into the container is a dead-end argument; it's the same as stating that one could also fork the project. It's true, but not how we want to discuss things in Nextcloud. Let's put the advantages and disadvantages on the table and weigh them. It's a trade-off, and if everyone else ends up concluding that the advantages don't outweigh the disadvantages, I'll happily agree. However, I'd like to understand where you see disadvantages and/or why you weigh the advantages differently.

Within the container ./occ status works already, in it's default configuration it's not necessary to specify the full path.

This is only true in a very limited scenario: Regarding the path only and for direct invocations only, i.e. docker exec --user www-data nextcloud ./occ status. This invocation by itself is a non-default call, as it requires one to exec with a non-default user (option --user www-data). Unfortunately we can't change the default user for exec. Furthermore, it works - in all practical manner - for direct invocations only. If one needs or wants to exec using an interactive shell, e.g. to investigate an app, it will stop working as soon as one changes the CWD. This burdens a lot of things to remember on the user's side. Simplifying this (i.e. interactive exec) is the primary advantage of this solution. Also simplifying direct invocations by making --user www-data obsolete is an additional advantage.

IMHO the wrapper adds another layer of complexity and may confuse users with special edge cases even more if something is not working for them.

@J0WI, can you please elaborate in which way it might confuse users and what edge cases you have in mind? I truly can't think of any right now.

Can you please also elaborate in which way it adds another layer of complexity? Since it's just an additional and optional (it doesn't affect any existing usage) script, I really just don't understand how it might add another layer of complexity? Or are you solely referring to "it's more LoC"?

[J0WI]

@J0WI, can you please elaborate in which way it might confuse users and what edge cases you have in mind?

Not really. This image is used in a bunch of heterogeneous environments. Some use it with docker, podman, k8s and probably more. Some use the image as-is, some install a bunch of additional extensions, scripts and other features on top of it. And some run it with limited user, capabilities, SELinux etc. We try to provide a generic base image with the option to customize it. We also try to keep it similar to other images from the official library.

We expect that people look at Nextcloud documentation if they need to change something within their instance. The Nextcloud documentation refers to Nextcloud's own occ and not to a third party wrapper script. So it might be confusion which occ is executed and how it differs from what's documented.

Can you please also elaborate in which way it adds another layer of complexity? Since it's just an additional and optional (it doesn't affect any existing usage) script, I really just don't understand how it might add another layer of complexity? Or are you solely referring to "it's more LoC"?

It's a tool that is not maintained by Nextcloud itself. So we have to maintain it, check compatibility with each version, write documentation for it and if anything goes wrong Nextcloud will forward people complaining about it to this repo. I hope you can follow these doubts.

[PhrozenByte]

@J0WI, can you please elaborate in which way it might confuse users and what edge cases you have in mind?

Not really. This image is used in a bunch of heterogeneous environments. Some use it with docker, podman, k8s and probably more. Some use the image as-is, some install a bunch of additional extensions, scripts and other features on top of it. And some run it with limited user, capabilities, SELinux etc.

So, none? 🤷‍♂️ None of these apply that wouldn't also apply for any pre-existing procedure, which won't be affected in any way by adding such script. That means, if one had created an individual image with whatever limitation imaginable, the solution one came up with to still use occ will work as before. If you have an use case in mind in which adding such script might be problematic, please state that and we can talk about it. If you don't, please don't act as if, I can't proof something not to exist.

We try to provide a generic base image with the option to customize it. We also try to keep it similar to other images from the official library.

Adding such scripts is standard procedure in the official library. Check e.g. the PHP image.

We expect that people look at Nextcloud documentation if they need to change something within their instance. The Nextcloud documentation refers to Nextcloud's own occ and not to a third party wrapper script. So it might be confusion which occ is executed and how it differs from what's documented.

First: In which way does it differ?

Second: php -f /var/www/html/occ -- will work as before. Even though the docs don't really help much anyway, because they don't explain docker exec -it --user www-data. So, I assume we're talking about README.md, right? Fair enough, totally forgot to update that, just did, see 026307a. Thanks for the reminder.

Can you please also elaborate in which way it adds another layer of complexity? Since it's just an additional and optional (it doesn't affect any existing usage) script, I really just don't understand how it might add another layer of complexity? Or are you solely referring to "it's more LoC"?

It's a tool that is not maintained by Nextcloud itself. So we have to maintain it, check compatibility with each version, write documentation for it and if anything goes wrong Nextcloud will forward people complaining about it to this repo. I hope you can follow these doubts.

I stumbled across "third party wrapper script" earlier, but what is "maintained by Nextcloud itself" supposed to mean? That it's no part of the server repo? That's the entrypoint script neither. About who wrote the script? You're a Nextcloud member, I'm a Nextcloud member? As always, one adds stuff, one takes responsibility for it.

Besides, we're at the same point as before: Can you please elaborate on what exact problems (towards maintenance and compatibility), documentation changes and user complaints you expect from this addition specifically? Towards user complaints I rather expect the opposite, since the current solution requires users to consider multiple non-default things when using occ. Towards maintenance and compatibility, I truly can't think of any reason for the wrapper to break, they are based on features existing for decades now; but if they appear nevertheless I'll happily fix them. Towards documentation, see above, I'd like to learn where you see any differences, especially since the docs already specifically mention that one commonly needs to use wrappers.

[PhrozenByte]

Rebased and bumping this.

@J0WI Thank you for the concerns you've raised. I believe that I've addressed all of them, but unfortunately you didn't follow up. So, can I assume that your concerns have been eliminated? If not, please explain what issues remain and name specific examples to give me a chance to address them.

I'm absolutely fine with closing this PR if there are real life issues. I'd follow-up with another PR just incorporating the code cleanups in docker-entrypoint.sh to replace the run_as eval calls by a cleaner (then internal) occ function. Otherwise, if there are no more concerns, we could merge this.

Also requesting a review from Josh (@joshtrichards) now since he seems to be more active since this PR has been opened. It's quite hard to find other people active in this repo to get more opinions… So, please feel free to ping others.

[joshtrichards]

Sounds like this is to to address the following situations:

• executing occ from anywhere in the filesystem
• executing occ regardless of whether execed in as root or www-data

And, to a lesser extent, to do the same for cron.php.

Is that about right?

Couple quick thoughts:

• Is prefixing with php / php -f really necessary in any of this image's use cases or is use of that syntax more out of habit at this point? [EDIT: Outside of for cron.php I mean]
• What if we added /var/www/html to the system default $PATH (e.g. via /etc/profile) to address the first item in a relatively simple way?

For what it's worth, I'm routinely exec'd into my app containers and using occ / etc. My workflow tends to either be:

docker compose exec -u33 bash

Then running ./occ and/or /var/www/html/occ as needed.

Or, for a single occ execution:

docker compose exec -u33 ./occ

Or, when doing something as root (i.e. not occ):

docker compose exec -u0 bash

That's it I think.

First: In which way does it differ?

occ: Interpolation of environment variables comes to mind. It's already bad enough when people try to run things through sudo/su/docker exec. I'll admit I'm fearful of introducing further complications or inadvertent differences in behavior in that chain of things (even "better" ones, since they just mean more to support that is different than upstream).

occ-cron: Non-standard. I'd rather at least stick with cron.php

P.S. Incidentally, this PR makes me wonder if we should switch to runuser over su in entrypoint, but that's unrelated.

[PhrozenByte]

Sounds like this is to to address the following situations:

• executing occ from anywhere in the filesystem
• executing occ regardless of whether execed in as root or www-data

And, to a lesser extent, to do the same for cron.php.

Is that about right?

Yes, that's about right: Making these things easier and less prone to errors.

The other advantage (improved code maintenance due to the removal of run_as in /entrypoint.sh) doesn't really depend on a standalone occ script.

• Is prefixing with php / php -f really necessary in any of this image's use cases or is use of that syntax more out of habit at this point? [EDIT: Outside of for cron.php I mean]
• What if we added /var/www/html to the system default $PATH (e.g. via /etc/profile) to address the first item in a relatively simple way?

Prefixing occ with php/php -f of course isn't strictly necessary thanks to the shebang, however, without also adding /var/www/html to $PATH one can't execute occ as command, but needs a path instead - either the absolute path /var/www/html/occ, or some relative path depending on the current $PWD (i.e. there's no generally true path then), with ./occ being the minimal if currently inside /var/www/html.

The other disadvantages of directly invoking /var/www/html/occ don't really apply tough. So, for example, the most common reason to not document the direct invocation of PHP scripts is because php might not be the right binary, but rather something like php82. We naturally don't have this issue here.

However, I'd strongly vote against adding /var/www/html to $PATH. One shouldn't add a directory to $PATH that also contains non-executable files. A simple file permissions mixup might have unexpected side effects then. That's the reason why there's usually some separate bin/ directory that can be added to $PATH. Unfortunately there's no such directory upstream.

In both cases we introduce another way of doing things that's different from upstream tough - with the exception that using wrappers isn't new and already mentioned in the upstream docs.

For what it's worth, I'm routinely exec'd into my app containers and using occ / etc. My workflow tends to either be:
[…]
Or, when doing something as root (i.e. not occ):

docker compose exec -u0 bash

What do you do if you need to call occ in this scenario?

First: In which way does it differ?

occ: Interpolation of environment variables comes to mind. It's already bad enough when people try to run things through sudo/su/docker exec. I'll admit I'm fearful of introducing further complications or inadvertent differences in behavior in that chain of things (even "better" ones, since they just mean more to support that is different than upstream).

Excellent point, thanks 👍 That's solved by using exec (and su -p if the current user isn't already www-data): It ensures that the full environment is kept. So, whatever you pass to the occ script will be passed through /var/www/html/occ exactly the same way as if you'd have executed /var/www/html/occ directly.

occ-cron: Non-standard. I'd rather at least stick with cron.php

Yeah, I'd be fine with that, running cron.php manually really isn't that common…

P.S. Incidentally, this PR makes me wonder if we should switch to runuser over su in entrypoint, but that's unrelated.

runuser basically is "su without PAM". We don't have PAM in the container, do we? Btw, runuser isn't pre-installed on Alpine, however, installing it surely wouldn't be a big deal…