seafile-docker icon indicating copy to clipboard operation
seafile-docker copied to clipboard

Allow specifying FILE_SERVER_ROOT

Open undergroundwires opened this issue 1 year ago • 6 comments

This is backward compatible feature that allow configuring FILE_SERVER_ROOT variable.

It allows Seafile Docker (without Lets Encrypt) + TLS proxy (such as reverse proxy, Web Application Firewall or Application Gateway) configuration which was not possible before.

Before, without specifying this variable, if Let's Encrypt is disabled (i.e., SEAFILE_SERVER_LETSENCRYPT=false) then FILE_SERVER_ROOT is always set to http:// protocol. However, if the user wishes to run the container on HTTP mode but have another proxy over HTTPs, it does not work due to the hardcoded http:// protocol.

After this commit, the user can override FILE_SERVER_ROOT and this way run the docker in HTTP mode meanwhile serving the docker container over a custom HTTPS proxy.

undergroundwires avatar Jan 23 '23 02:01 undergroundwires

Great idea! I was waiting for this a long time. But I think the well known acme challenge route in the nginx template must also be disabled if using a reverse proxy in front of the container handling the https challenge.

Thank you!

300481 avatar Jan 23 '23 06:01 300481

In my option, you can always modify FILE_SERVER_ROOT in the generated config file manually, so there is not need to support specify FILE_SERVER_ROOT during initial configuration for special cases.

freeplant avatar Jan 23 '23 07:01 freeplant

Great idea! I was waiting for this a long time. But I think the well known acme challenge route in the nginx template must also be disabled if using a reverse proxy in front of the container handling the https challenge.

Thank you for your nice feedback, I guess SEAFILE_SERVER_LETSENCRYPT=false should do the job so configuration would then require setting a) SEAFILE_SERVER_LETSENCRYPT=false and b) FILE_SERVER_ROOT in this case.

In my option, you can always modify FILE_SERVER_ROOT in the generated config file manually, so there is not need to support specify FILE_SERVER_ROOT during initial configuration for special cases.

That is right, but it is not desirable for a cloud-native immutable solution where we have disposable environments and stuff could be destroyed and redeployed easily using infrastructure as code. In my situation I deploy test environments on the fly once a PR is created which means that I have different copies of the environment. I would expect the container run smoothly in this case without manually going inside the container and mutating it on every fresh install, which tends to happen often.

And the goal of Docker images is to automate stuff for the most part anyway, isn't it? Going manually on each deployment and doing system ops, or writing glue scripts around the container would cause headache for many, meanwhile we could support this scenario out-of-the-box officially in the upstream without breaking anything.

Setting this variable would also align with official documentation for how to run Seafile behind a proxy/webserver.

undergroundwires avatar Jan 24 '23 01:01 undergroundwires

Hi @freeplant

In my option, you can always modify FILE_SERVER_ROOT in the generated config file manually, so there is not need to support specify FILE_SERVER_ROOT during initial configuration for special cases.

I see it the same way as @undergroundwires

We use containers to benefit from all the nice features it brings. That is, among other things, immutability, descriptive deployments and repeatability. Changing things in running deployments is mutability, this is the exact opposite of what we want to achieve.

It looks like a low hanging fruit.

Do you see any issue implementing this?

300481 avatar Jan 24 '23 08:01 300481

Hi @undergroundwires

sadly SEAFILE_SERVER_LETSENCRYPT=false will not deactivate the well-known/acme-challenge route.

I would assume that it needs a bit code like {% if https -%} around this section as you can find here: https://github.com/haiwen/seafile-docker/blob/master/image/seafile_9.0/templates/seafile.nginx.conf.template#L22

300481 avatar Jan 24 '23 08:01 300481

Hi @undergroundwires,

I've created a PR for the NGINX template: https://github.com/haiwen/seafile-docker/pull/325 This should combine perfectly with your PR.

300481 avatar Jan 24 '23 09:01 300481