SteveLTN / https-portal

A fully automated HTTPS server powered by Nginx, Let's Encrypt and Docker.
MIT License
4.46k stars 295 forks source link

Allow on-the-fly reconfiguration #267

Closed MarcelWaldvogel closed 3 years ago

MarcelWaldvogel commented 3 years ago

Reducing down-time even further using live reconfiguration. reconfig now essentially does the same thing as setup, but without stopping/starting nginx, just reloading it.

Also, domains can now be removed and will also disappear (the certificates will remain as a cache).

Files placed in /var/lib/https-portal/dynamic-env override the environment variables on the fly using justc-envdir. Changing these files will cause an immediate reconfiguration, as if the container had been restarted, just without downtime.

SteveLTN commented 3 years ago

Should we add a script to cont-init.d that loads all files in /var/lib/https-portal/dynamic-env to env on start? In case you have some ENVs configured in dynamic-env, then you need to restart the container for some reason.

SteveLTN commented 3 years ago

Actually, how many environment variable would someone change except for DOMAINS?

Maybe we should just allow people to edit a folder /var/lib/https-portal/dynamic-domains, and put one domain as a file and have its content being the upstream descriptor? We can still watch it, but if any files change, we trigger a reconfig. And in NAConfig, we can blend these domains from the dynamic domains folder into NAConfig.domains.

Or, we can allow people to edit a single file, dynamic-domains. It will just have one domain per line, and if it changes, we trigger a reconfig.

What do you think?

MarcelWaldvogel commented 3 years ago

Should we add a script to cont-init.d that loads all files in /var/lib/https-portal/dynamic-env to env on start?

My mental model is that only environment variables treated by the CertManager will be changed (am I wrong here?).

All calls previous to this change should preload the container environment using with-contenv, which is essentially justc-envdir / var/run/s6/container_environment. Now, all calls to CertManager additionally load justc-envdir /var/lib/https-portal/dynamic-env on top of the container environment (some which are run at startup, potentially before the directory exists, with an additional -I to not care about the potentially missing directory). So copying the files is IMHO not necessary.

Actually, how many environment variable would someone change except for DOMAINS?

I think this will be the main change. However, I could also imagine changing ACCESS_LOG on demand for debugging purposes (even though I did not get it to affect Nginx, even though /etc/nginx/nginx.conf reflected the change (?)) as well as CUSTOM_NGINX_SERVER_CONFIG_BLOCK and friends. So I think putting everything into a single file, with ± the same functionality as dynamic-env/DOMAINS (except that it is used in addition to it) would unnecessarily reduce the functionality. I also like that – in the current PR – deleting domains is also possible.

However, if we think outside the current model of environment variables, another model sounds attractive to me: (That was what I thought you wanted to describe with dynamic-domains, until I noticed that I misread it.)

I think this would significantly extend the possibilities and uses of https-portal and provide modularity for the configuration. It would require some more work (not much) than the current minimally-invasive approach, but could be a reason for switching to version 2. Would this be a way in which you would like to go?

SteveLTN commented 3 years ago

your version of dynamic domains

Let's first talk about dynamic-domains. I don't intent to change the way HTTS-PORTAL works. The project was intended to be used by personal website builder, ease of use and minimal configuration was the first priority. More complex setup options were added later to accommodate some use-cases that are more sophisticated, like yours. However I don't want to complicate the codebase of this project to a degree where it's too hard for me to maintain, therefore I will be reluctant on adding dynamic domains as you described.

my version of dynamic domains

Going back to my version of dynamic domains. It was just an early idea. I felt like in comparison to keep changing a single DOMAIN file, one might prefer having one file per domain, and add/remove/edit them dynamically.

For instance, dynamic-domains folder can then be shared with other upstream containers, and each upstream container can maintain it's own domain file in dynamic-domains dynamically, knowing HTTPS-PORTAL will pick up the change.

I wasn't sure which way people would like better, so I asked for you opinion since you are the one who actually uses this feature. Now that I see you prefer the former way, I have no problem against the current solution.

preloading dynamic envs

On this, actually I misread the code. All I meant was we should do this, without noticing this was already done.

MarcelWaldvogel commented 3 years ago

I think your version of dynamic domains only requires a handful lines to implement. It would be a good complement to the bulky, but feature-rich environment variables and the docker-gen autoconfiguration. I'll give it a try.

SteveLTN commented 3 years ago

No, don't. I don't think it's necessary to have both dynamic-env and dynamic-domains.

MarcelWaldvogel commented 3 years ago

Sorry, didn't read your comment in time. The change is minimal, if we discount for the variable renaming in the watcher script. But feel free to not merge it.

SteveLTN commented 3 years ago

I'll merge it and then revert the last commit you made :-)