Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying FILE_SERVER_ROOT #324

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

undergroundwires
Copy link

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.

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.
@300481
Copy link
Contributor

300481 commented Jan 23, 2023

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!

@freeplant
Copy link
Member

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.

@undergroundwires
Copy link
Author

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.

@300481
Copy link
Contributor

300481 commented Jan 24, 2023

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
Copy link
Contributor

300481 commented Jan 24, 2023

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
Copy link
Contributor

300481 commented Jan 24, 2023

Hi @undergroundwires,

I've created a PR for the NGINX template: #325
This should combine perfectly with your PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants