NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.37k stars 13.6k forks source link

Guidelines / Recommendations for when to configure nginx in a service module? #277723

Open Ma27 opened 8 months ago

Ma27 commented 8 months ago

Context & Motivation

A lot of NixOS modules configuring web-services also provide configuring a reverse-proxy (usually nginx). Most common examples are:

There are in fact a few good reason why this is done:

However, I recently started getting some doubts whether adding nginx configurations to service-modules is generally a good idea.

Issues with nginx configurations

Since there's no standardization (e.g. configureNginx vs nginx.enable, does the module also configure TLS&ACME?), you have to check for every single module how (and if) there's nginx configuration already provided because there's a high chance to encounter subtle differences. If there's something you need to change, that can become pretty hard because of that since nginx uses extraConfig rather than RFC42-style settings for a lot of stuff[2], an example is whether or not to add HSTS headers:

The stance I have on that is actually quite inconsistent: for Nextcloud, I came to the conclusion that it's the best if we try to stay as close to the upstream suggestions as possible. But on other modules I don't think it's good to inconsistently configure additional things (especially if there are no guidelines recommending that). My fear is that people might learn by accident that e.g. forceSSL implies HSTS, but that's only the case because of some magic a random list of modules is doing[3].

In fact, we have this problem already. You'll always have to look what to do when configuring nginx with TLS and ACME for $serviceModule.

Next up, all of that machinery doesn't help at all in cases where the vhost isn't on the same machine as the service and that's something I'd consider rather common these days.

Moving forward

A bunch of ideas I have:

And finally, what do we do with existing modules?

CCing a few people who may have an opinion here: @lheckemann @RaitoBezarius @mweinelt @Izorkin @SuperSandro2000

[1] an extreme example is probably ocmProviderIsNotAStaticDirAnymore in nextcloud ;-) [2] I know that it will be very hard to implement that for nginx. [3] I'm well-aware that both of my positions contradict here, that's part of the reason why I'm opening this issue :)


Add a :+1: reaction to issues you find important.

Izorkin commented 8 months ago

A sample version of recommendations:

SuperSandro2000 commented 8 months ago

prohibit modules to provide nginx configurations for services that have their own server builtin (e.g. nodejs, uwsgi, Go/Rust binaries).

I would probably add to that if the config is just proxyPass. Those could still require plentiful of locations. Also configuring TLS for each of those services is a pain and just using nginx for everything is easier.

  • we may want to consider doing that in its own module (something like services.nginx-vhosts.matrix-synapse?

I don't think we gain much by that move other than that it might be forgotten to be updated.

That way it might also be possible to point the proxyPass to a different machine).

That should probably be some kind of variable or nginx should be of the freeform type. Also I don't want to support more remote types of deployment because we don't have full control over that in code.

either provide the standard configuration from upstream or try to do as few things as possible.

* does that mean we still enable TLS by default?

Upstreams default vary from pretty good to outright dangerous and bad. Just copying upstream will usually not do it especially if we run postgres and phpfpm instead of dusted apache.

Also nudging people to do TLS is probably a good idea.


I think most problems we have boil down to nginx not being of a freeform type and having way to many specific settings for specific things that in the end just concats strings.

Many of nginx' settings can only be set once per layer and otherwise crash nginx outright. It should be pretty easy to convert those to a freeform type. Some settings like add_header can be defined multiple times. If we had a type we could replace all the extraConfig and common options with, then extending the nginx config would be easy and providing good defaults, too.

Then we could easily implement options like HSTS by default by creating a new option which writes into every block where headers are added.

Right now the recommended* options already have that problem. If you need to change any option in there, you need to turn of the config, cop it's content and change the one setting you wanted to change.

(Sidenote: I also recently noticed that headers are not propagated down layers and if a new add_header is written, then all before are ignored.)


If we rewrite the nginx module to freeform, we probably need some more sophisticated types which know at which level they are (http,server,location,etc), if the option is valid for there (we could only do that for the most common ones), if it can be written once or multiple times and then we could move all the specifics like ACME into smaller submodules which use those types.


Or maybe we just write down what modules are expected to do with nginx configs and that except location blocks and the necessary configuration nothing should be added.


I've just recently discovered that you can easily expand the virtualHost/locations options in your own modules https://github.com/SuperSandro2000/nixos-modules/blob/master/modules/nginx.nix#L77-L79 .

Maybe doing this as an addon to the nginx module would also be an idea?

I really don't know whats the best option, but we should kick out some of the feature creep we have accumulated over the years.