Expose reverse proxy related configuration as environment variables

[andrasmaroy]

As #819 seems abandoned to me, I implemented @kesselb's suggestion of putting these variables into a separate config file.

Should fix #792

[J0WI]

I think we go for #1010 (comment)

[kesselb]

I think we go for #1010 (comment)

Yeah. But we should do both ;) We need to merge the configuration after #1010 is merged and make sure to set the proper defaults.

[andrasmaroy]

I've set the defaults for these to empty strings (empty array obviously for trusted proxies), I figure that should remove these values when merging conflicts.

Please let me know if this should be done differently.

[kesselb]

Please change the default value to null.

It's necessary because Nextcloud merges the configuration files together. That's only a problem if you previously set a value and want to unset it again. To unset it again we need a value that evaluates to false on isset. Otherwise Nextcloud will not use the internal defaults leading to unexpected behaviour.

https://github.com/nextcloud/server/blob/19ca921676240350abd694d8ca2d7b8c19fbd95a/lib/private/Config.php#L96-L100 is the code in question.

[andrasmaroy]

Done, thanks for pointing me in the right direction.

[kesselb]

@J0WI mind to merge (or ping the other maintainers) #1010 first? It's easier to avoid conflicts then.

[kesselb]

@andrasmaroy the other PR is merged now. Mind to rebase your PR? We picked a similar but different name for the configuration file. reverse_proxy.config.php vs reverse-proxy.config.php. I don't have a strong opinion on that. Pick the one you like the most ;)

[andrasmaroy]

@andrasmaroy the other PR is merged now. Mind to rebase your PR? We picked a similar but different name for the configuration file. reverse_proxy.config.php vs reverse-proxy.config.php. I don't have a strong opinion on that. Pick the one you like the most ;)

Will do, I prefer merging instead of rebasing though if that's not against the conventions here.

[kesselb]

Did a quick test:

1. Patch master with https://patch-diff.githubusercontent.com/raw/nextcloud/docker/pull/1048.patch
2. Pick a version (for example 18.0-apache)
3. Build the image locally
4. Run the image docker run -p 8080:80 --env OVERWRITEHOST=cloud.test --env OVERWRITEPROTOCOL=http -it $imageId
5. Run the setup routine
6. Ask occ for the configuration docker exec -u www-data $containerName /var/www/html/occ config:list system --private

        "overwritehost": "cloud.test",
        "overwriteprotocol": "http",

[kesselb]

Build fails. Mind to update your branch with the latest changes from master one last time?

[kesselb]

Nextcloud 19-beta has been added as image. You might run update.sh again to apply the changes to that image too.

[kesselb]

Thanks 👍

Don't know what's wrong with the build.

[kesselb]

Hold on :) We have some reports that #1010 is going to break existing setups because null is used as default if the env is not set.

[kesselb]

Would you mind to update your pr another time and remove all the else null blocks?

[andrasmaroy]

Would you mind to update your pr another time and remove all the else null blocks?

Sure although in this case maybe there should be a notice somewhere that omitting these environment variables won't remove the configuration values?

[kesselb]

Sure although in this case maybe there should be a notice somewhere that omitting these environment variables won't remove the configuration values?

That make sense yes.

[crackers8199]

is this fixed in the 17.0.5 image? i can't get a clean install of 17.0.5 to work as far as the trusted proxies option goes...17.0.4 works, and i can then upgrade to 17.0.5 once it's set up. if i do a straight install of 17.0.5, trusted proxies never works.

[kesselb]

This pull request is not merged so the answer is no. Just use the new trusted proxies environment variable with 17.0.5.

[crackers8199]

This pull request is not merged so the answer is no. Just use the new trusted proxies environment variable with 17.0.5.

i'm good now because i just started with 17.0.4 and then upgraded, and that worked...just was curious if this would eventually fix the issue with 17.0.5 clean install not working with reverse proxy. thanks!

[kesselb]

It will fix the issue once merged.