search

linuxserver / reverse-proxy-confs

These confs are pulled into our SWAG image: https://github.com/linuxserver/docker-swag
GNU General Public License v3.0
1.29k stars 296 forks source link

Update nextcloud's instructions #684

Open quietsy opened 1 week ago

quietsy commented 1 week ago

We're getting a lot of support questions regarding trusted_proxies and gethostbyname, changing the instructions to be more fool-proof

thespad commented 1 week ago

Need to be careful as you're making assumptions about 172.16.0.0/12. Docker can also allocate /20s from 192.168.0.0/16 and, in the case of overlay networks, /24s from 10.0.0.0/8.

Technically it can allocate any sized subnet from those ranges, but those are the defaults and you'd hope if people have the wherewithal to change them they also know how to configure this without handholding.

quietsy commented 1 week ago

Need to be careful as you're making assumptions about 172.16.0.0/12. Docker can also allocate /20s from 192.168.0.0/16 and, in the case of overlay networks, /24s from 10.0.0.0/8.

Technically it can allocate any sized subnet from those ranges, but those are the defaults and you'd hope if people have the wherewithal to change them they also know how to configure this without handholding.

It will work in most cases and specifically in the common compose created network, and avoid the issues with gethostbyname. (which probably doesn't work anymore, definitely not for me)

nemchik commented 1 week ago

I searched through discord briefly and the only real issues I found with gethostbyname were that at some point NC (or a plugin) replaced it in the config.php with an actual IP address for SWAG at the time (which could later change). I can confirm this actually happened to me. The rest of what I saw regarding trusted_domains was mostly users who didn't have gethostbyname involved, or didn't have their NC and their proxy container (swag or otherwise) on the same docker network.

I had put gethostbyname in my config a few months ago and found today it was replaced with 172.16.0.8 at some point. I think it makes the most sense to try our best to explain what's actually going on in the comments in the file so users can decide what value they want to put in their config.php, potentially from multiple known working options. I would consider gethostbyname a known working option with a caveat (that it will likely get replaced by a static IP and not work long term). Aside from that, using the CIDR for the docker network if the user has their containers on the same docker network, or using the CIDR for their LAN as a last resort.

quietsy commented 1 week ago

I don't see a need to over-complicate it beyond: gethostbyname is known to break, using the CIDR doesn't break.

if the user has their containers on the same docker network

It shouldn't be an if, that is our recommended configuration with swag and what we officially support.

nemchik commented 1 week ago

I don't see a need to over-complicate it beyond: gethostbyname is known to break, using the CIDR doesn't break.

if the user has their containers on the same docker network

It shouldn't be an if, that is our recommended configuration with swag and what we officially support.

I can agree with both of those statements. Would it be worth writing a PHP script that detects the user's CIDR from within the container and displays it as a recommended value in the init log? Potentially even as an occ command the user can just copy/paste to set the correct value?

P.s. I don't think setting it automatically is the right idea, but displaying a recommendation for settings we support based on detected values in the user environment makes sense.

Also could be done in bash, but I figure if PHP can see the value it's more likely what nextcloud would actually see (depending on their implementation I guess).