nixos/mailman: lots of big improvements #77444
Conversation
ofborg
bot
added
6.topic: nixos
|
@FRidh all done :) |
-1 to squashing any of it. Would make it much more difficult to see what had changed. I think it’s probably not worth backporting. There are security implications, but this is a big, breaking change, and it’s been like this for months… |
|
This is a big improvement, thanks @alyssais ! |
|
@jonringer done |
|
You might want to have a look at https://github.com/mayflower/nixexprs/blob/master/modules/mailman3.nix, which predates this module, to see if you want to steal something. I deemed it too hacky to try and push it upstream at the time but it might be interesting to compare if there's anything there missing here. When this PR is merged I'll probably just dump it anyway in favour of this. |
|
I’ve dropped e6f970d7a0957b0f2661db547e1eef6a236eddb8 because of https://gitlab.com/mailman/mailman/merge_requests/594. I could have applied the patch to the mailman package, but since it was an entirely cosmetic change I think it’s better to just wait for it to be fixed in upstream, and then we can unify the config file locations. |
| COMPRESS_OFFLINE = true; | ||
| STATIC_ROOT = "/var/lib/mailman-web/static"; | ||
| MEDIA_ROOT = "/var/lib/mailman-web/media"; | ||
| } // cfg.webSettings; |
There was a problem hiding this comment.
You can assign these defaults in the config section of the module with
{
config.services.mailman.webSettings = {
DEFAULT_FROM_EMAIL = mkDefault cfg.siteOwner;
SERVER_EMAIL = mkDefault cfg.siteOwner;
# ...
};
}Then you don't need to do this overriding here and makes the defaults inspectable.
There was a problem hiding this comment.
Not possible until #75584, and I don’t think this should have to wait until then.
Not everybody is using Apache.
A default of example.com is useful to nobody. The correct value of this depends on the system.
This replaces all Mailman secrets with ones that are generated the first time the service is run. This replaces the hyperkittyApiKey option, which would lead to a secret in the world-readable store. Even worse were the secrets hard-coded into mailman-web, which are not just world-readable, but identical for all users! services.mailman.hyperkittyApiKey has been removed, and so can no longer be used to determine whether to enable Hyperkitty. In its place, there is a new option, services.mailman.hyperkitty.enable. For consistency, services.mailman.hyperkittyBaseUrl has been renamed to services.mailman.hyperkitty.baseUrl.
alyssais
force-pushed
the
mailman
branch
2 times, most recently
from
59644fc to
d1f4e18
Compare
It's likely that a user might want to set multiple values for relay_domains, transport_maps, and local_recipient_maps, and the order is significant. This means that there's no good way to set these across multiple NixOS modules, and they should probably all be set together in the user's Postfix configuration. So, rather than setting these in the Mailman module, just make the Mailman module check that the values it needs to occur somewhere, and advise the user on what to set if not.
We already had python3Packages.mailman, but that's only really usable as a library. The only other option was to create a whole Python environment, which was undesirable to install as a system-wide package.
Previously, some files were copied into the Nixpkgs tree, which meant we wouldn't easily be able to update them, and was also just messy. The reason it was done that way before was so that a few NixOS options could be substituted in. Some problems with doing it this way were that the _package_ changed depending on the values of the settings, which is pretty strange, and also that it only allowed those few settings to be set. In the new model, mailman-web is a usable package without needing to override, and I've implemented the NixOS options in a much more flexible way. NixOS' mailman-web config file first reads the mailman-web settings to use as defaults, but then it loads another configuration file generated from the new services.mailman.webSettings option, so _any_ mailman-web Django setting can be customised by the user, rather than just the three that were supported before. I've kept the old options, but there might not really be any good reason to keep them.
This will allow users to provide other archiver plugins than the default mailman-hyperkitty.
|
I plan on merging this today so it gets into 20.03. The only significant change I’ve made recently is adding the release notes. If anybody is up for reviewing the release notes, that would be much appreciated. |
ofborg
bot
added
8.has: changelog
8.has: documentation
Motivation for this change
Our current Mailman module is quite lacking in a number of ways, including security issues:
SECRET_KEYwere hard coded in these config files, meaning that not only were they stored in the Nix store, they were the same for all NixOS users.mkForce.With these changes, all of these problems are resolved, and the Mailman module is much more useful. I’ve been using it pretty much like this for months, and it has worked perfectly.
Where necessary, I’ve explained the rationale for each change in more detail in the commit message. I strongly recommend reviewing commit by commit to understand why each change was made.
I’ve retained backwards compatibility as much as was practical, but some small changes will be required by users. The
hyperkittyApiKeysetting no longer makes sense, for example, and users will be required to set some Postfix configuration for reasons described above. All of this will be guided by error and warning messages.Fixes #77274.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)