nixos/postfix: fix default postfix config #34060
Conversation
|
With your PR I think it is impossible to remove default configs. For example the default postfix config for recipient_canonical_maps is If I want to set it to which would now merge to |
|
@uwap you are right about the merging of the values, which is the intended behaviour, but you should still be able to override the default value with a 'lower' priority e.g. |
|
I think the changed behavior should be documented in the release changelog then? cc @fpletz |
| @@ -24,7 +24,7 @@ let | |||
| then [ (concatStringsSep ", " (map (s: "reject_rbl_client " + s) cfg.dnsBlacklists)) ] | |||
| else []; | |||
|
|
|||
| clientRestrictions = concatStringsSep ", " (clientAccess ++ dnsBl); | |||
| clientRestrictions = clientAccess ++ dnsBl; | |||
There was a problem hiding this comment.
I don't really get why you do this change. Could you explain it?
There was a problem hiding this comment.
I think I made a mistake there, as I can't think of why I did that. Thank you 👍
There was a problem hiding this comment.
The module adds commas inconsistently. Commas seem to be optional in postfix's main.cf so we should either add them for all options that are lists or not.
`services.postfix.config` is now correctly merged with the default attrset specified in the module. Some options that are lists in postfix also have to be lists in nix to be merged correctly. Other default options are now set with `mkDefault` so they can be overridden via the module system.
WilliButz
force-pushed
the
fix-postfix-module
branch
from
8eb5488
to
9bd7798
Compare
|
@uwap I consider the previous behaviour a bug because the options would not merge nicely as other options do in the module system. We will document the change in the release notes. |
|
@uwap Regarding your concern with merging default options: We made sure that we only set list options if another option in If the old behaviour is used and the user sets |
|
I've added a mention in the release notes and a commit to add commas between list option elements. This makes the output of |
|
@fpletz Great! Thank you! |
services.postfix.configis now correctly merged with the default attrsetspecified in the module. Some options that are lists in postfix also
have to be lists in nix to be merged correctly. Other default options are
now set with
mkDefaultso they can be overridden via the module system.Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)/cc @qknight @globin @fpletz