RFC: Automatically handle nginx domain and make letsencrypt setup simpler

[ilkka-ollakka]

Description

This PR is build on top of #3540 on splitting up nginx config. It contains new bw-dev command and separated docker-compose -file just to bootstrap letsencrypt certificate. It allows documentation to be simplied so docker-compose or nginx-config files don't need to be modified manually when setting up bookwyrm with https.

One key point in certbot is that I added --keep-until-expiring compared to production command, this makes it run in mode that it gets certificate if not present, otherwise it checks if current one needs to be renewed or not. So should be pretty safe to run always. It can also replace renew command to my understanding.

Docker-compose file is also modified so it takes the PORT configuration into account from .env and .env file now contains NGINX_SETUP variable to tell if nginx should be in development, production (https) or reverse-proxy mode. Use only needs to modify .env file to change between those, no need to modify nginx-files or copy those over or modify docker-compose.

Certbot container is added and it has entrypoint to run renew and sleep a day if USE_HTTPS is configured to be true in .env. This allows to run the stack without cron-script setup.

Nginx deployment has small entrypoint script added that will periodically tell nginx to reload config, this takes renewed certificates into use automatically.

• Related Issue #
• Closes Figure out a certbot flow that doesn't involve commenting things out in docker-compose.yml #1786

What type of Pull Request is this?

• Bug Fix
• Enhancement
• Plumbing / Internals / Dependencies
• Refactor

Does this PR change settings or dependencies, or break something?

• This PR changes or adds default settings, configuration, or .env values
• This PR changes or adds dependencies
• This PR introduces other breaking changes

Details of breaking or configuration changes (if any of above checked)

Documentation

• New or amended documentation will be required if this PR is merged
• I have created a matching pull request in the Documentation repository
• I intend to create a matching pull request in the Documentation repository after this PR is merged

Tests

• My changes do not need new tests
• All tests I have added are passing
• I have written tests but need help to make them pass
• I have not written tests and need help to write them

[ilkka-ollakka]

I'll split the server name to separate file, just to make simpler to document if it needs to be changed.

[ilkka-ollakka]

I think I can split this up to smaller change-sets, so it is not so much changing in one PR?

[ilkka-ollakka]

With this PR I think there is no differences on production docker-compose and main-branch docker-compose. Hopefully making the update/testing simpler also from that point of view.

[hughrun]

I think this is a positive move forward - we have a lot of Issues and comments about making nginx setup simpler and for me the ideal scenario is someone can create an .env and just spin up an instance without touching docker or nginx files.

This seems to work fine in development, but with a few caveats that at the very least will require a breaking changes note and further updated documentation.

localhost works fine out of the box (as long as PORT has not been set), but anyone using ngrok will need to adjust their settings.

Up to this change, I have been using ngrok for development, running through port 1333:

ngrok http 1333

But this PR changes this behaviour in potentially confusing ways. It reverses the existing behaviour of developing on localhost vs ngrok because we move from publishing on port 1333 to 80.

Up until this change, devs would set PORT to 1333 if using localhost, and not set a value for PORT if using ngrok or something similar.

Now if PORT is not set, then instead of running through 1333 we are using 80, even when USE_HTTPS is True. But if we set PORT to 1333, BookWyrm will load initially, but we end up with a CSRF error on any form submission. That is, we have to not set PORT, and run ngrok with ngrok http 80.

[ilkka-ollakka]

Thanks for checking this, you found really good points

This seems to work fine in development, but with a few caveats that at the very least will require a breaking changes note and further updated documentation.

localhost works fine out of the box (as long as PORT has not been set), but anyone using ngrok will need to adjust their settings.

I think this has come from mismatch between .env.example comments on PORT and actual nginx-config in main branch. .env file mentions that PORT should be set if it is wanted to be something else than default, I followed that approach by the idea that .env file modifications should be enough for most of the cases.

But you are right, it breaks the config that is currently in place and needs some more documentation or highlighting.

Now if PORT is not set, then instead of running through 1333 we are using 80, even when USE_HTTPS is True. But if we set PORT to 1333, BookWyrm will load initially, but we end up with a CSRF error on any form submission. That is, we have to not set PORT, and run ngrok with ngrok http 80.

One option that I was thingking was to introduce the PORT_HTTP and PORT_HTTPS variables, as currently we have just PORT and USE_HTTPS and it might be tricky to anticipate that should the port be https or http port and what the other should be.

But I'll check if I can introduce the USE_HTTPS to PORT handling in nginx part and if it helps.

On domain part, I agree. I'll remove the www-subdomain.

[hughrun]

One option that I was thingking was to introduce the PORT_HTTP and PORT_HTTPS variables, as currently we have just PORT and USE_HTTPS and it might be tricky to anticipate that should the port be https or http port and what the other should be.

If we can avoid further complicating .env I think that would be best. This really is only a problem in development, and your overall direction is the right one, I think. I really just wanted to highlight that this will break the current expectations in the development environment so it needs to be made clear in release notes and docs.

But if setting PORT causes CSRF errors regardless then it will not be very useful, which is why I highlighted that. Not sure if this is a problem in production?

[ilkka-ollakka]

If we can avoid further complicating .env I think that would be best. This really is only a problem in development, and your overall direction is the right one, I think. I really just wanted to highlight that this will break the current expectations in the development environment so it needs to be made clear in release notes and docs.

But if setting PORT causes CSRF errors regardless then it will not be very useful, which is why I highlighted that. Not sure if this is a problem in production?

I think it is problem only with PORT and USE_HTTPS=true combination, in your case, are you terminating the ssl in nginx or in ngrok?

I was thingking that more feasible flow for that could be to set NGINX_SETUP=development in .env, as currently it defaults to `production? I'm not sure if there is common use-case to run http/https combination in custom ports? This would need to be documented, but might be feasible flow for development changes.

Now that I look at those, I'm not sure if reverse-proxy is that usefull compared to development and those could be same and we could just have NGINX_SETUP=https|reverse-proxy and just document that if someone uses something in front of nginx, set the setup to reverse-proxy ?

I can add the commits on the changes that I'm mentioning here, so it might be easier to follow what I mean?

[hughrun]

I think it is problem only with PORT and USE_HTTPS=true combination, in your case, are you terminating the ssl in nginx or in ngrok?

Yes I think you're right. And now that you ask, I haven't done anything special with nginx so I'm not really sure - ngrok serves both http and https but if I tried ngrok http 443 it didn't work so I guess nginx is only serving it on 80? I am a little out of my depth to be honest.

I can add the commits on the changes that I'm mentioning here, so it might be easier to follow what I mean?

Yes please!

[ilkka-ollakka]

I ended up with flow that USE_HTTPS indicates if connections should come via https, either something in front of nginx terminating them or nginx directly. NGINX_SETUP=https to tell that nginx should handle certificates and ssl.

settings changes were added so that if we have reverse_proxy, we still most likely have something in default port to listen connections and that default should be used. I'm not sure how good assumption that is, but it should address the case that you have ngrok listening https/http and bookwyrm is in reverse_proxy mode.

I also changed the certbot entrypoint to check NGINX_SETUP mode, that indicates more if we assume nginx/letsencrypt to handle certificates than USE_HTTPS.

[hughrun]

Ok this looks good! Just a question about the ports interpolation in docker-compose.

all changes resolved.