nixos/mpd: allow to explicitly close firewall without a warning

[doronbehar]

Things done

• Fixes nixos/mpd: cannot configure VPN based auth without emitting warnings #484912
• Addresses the comments here: nixos/mpd: apply RFC 42 #456989 (comment)
• Built on platform:
  • x86_64-linux
  • aarch64-linux
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

[a-kenji]

I think this is still a little odd, for lack of a better term, because this is very different from how most any other service operates in nixpkgs.

That being said, I see this as an improvement and I see no issue with this PR from that standpoint.

[doronbehar]

That being said, I see this as an improvement and I see no issue with this PR from that standpoint.

Yes I agree. Adding such a warning could be a nice feature to any module that has both openFirewall option and a "bind address" option such as mpd's bind_to_address. I personally don't maintain enough services to push that, but I think it should help users avoid bashing their head when their services are running but are not accessible from outside.

I think this is still a little odd, for lack of a better term, because this is very different from how most any other service operates in nixpkgs.

Thanks for the approval. Could you please test this with your config, I really want to make sure it works for you with those non-trivial network interfaces.

[SigmaSquadron]

Our warnings/assertion infrastructure as a whole could do with a lot of improvement. I think @emilazy was considering something of the sort a while back, but I am not entirely sure what came out of that work.

[SigmaSquadron]

It might be more ergonomic if we set the type to be either Boolean or some special string that disabled the warning. null is meaningless, especially since its behaviour is not defined in the option description.

[doronbehar]

It might be more ergonomic if we set the type to be either Boolean or some special string that disabled the warning. null is meaningless, especially since its behaviour is not defined in the option description.

I agree. Either with null or a specific string the description will have to be modified. If I'll modify the description to explain the meaning of null would you be satisfied?

[SigmaSquadron]

Whatever is easiest for you seems good to me.

[doronbehar]

Whatever is easiest for you seems good to me.

Done.

[a-kenji]

Could you please test this with your config, I really want to make sure it works for you with those non-trivial network interfaces.

I tested it with an ipv6 bind_to_address.

[doronbehar]

Could you please test this with your config, I really want to make sure it works for you with those non-trivial network interfaces.

I tested it with an ipv6 bind_to_address.

Just out of curiosity, with bind_to_address = "::"?

[a-kenji]

Yes:

settings.bind_to_address = "[::]";
openFirewall = false;