nixos/unbound: Coerce all single settings values to lists

[infinisil]

Motivation for this change

Makes composability much better, as it allows mixing single values together without having to declare them as lists. This allows the following to work:

{
  services.unbound = {
    enable = true;
    settings.server = lib.mkMerge [
      {
        local-zone = ''"local." static'';
        local-zone-tag = ''"local." "local_net"'';
        local-data = [
          "router.local. IN A 10.0.0.1"
          "server.local. IN A 10.0.0.10"
        ];
        local-data-ptr = [
          "10.0.0.1 router.local."
          "10.0.0.10 server.local."
        ];
      }
      {
        local-zone = ''"foo." static'';
        local-zone-tag = ''"foo." "local_net"'';
        local-data = [
         "router.foo. IN A 10.0.0.2"
          "server.foo. IN A 10.0.0.20"
        ];
        local-data-ptr = [
          "10.0.0.2 router.foo."
          "10.0.0.20 server.foo."
        ];
      }
    ];
  };
}

I was motivated to improve this after @pennae and @andir had some complaints on IRC :)

The settings approach was introduced in #89572, ping @rissson @lovesegfault

Things done

I have not tested this other than making sure that above configuration evaluates properly. I'd like to defer testing to @pennae and @andir as they are actually using this module.

[infinisil]

Note that the mkMerge there is just a way to specify multiple definitions for the same option in the same file. Alternatively this would have the same effect:

{
  imports = [ ./a.nix ./b.nix ];
  services.unbound.enable = true;
}

a.nix:

{
  services.unbound.settings.server = {
    local-zone = ''"local." static'';
    local-zone-tag = ''"local." "local_net"'';
    local-data = [
      "router.local. IN A 10.0.0.1"
      "server.local. IN A 10.0.0.10"
    ];
    local-data-ptr = [
      "10.0.0.1 router.local."
      "10.0.0.10 server.local."
    ];
  };
}

b.nix:

{
  services.unbound.settings.server = {
    local-zone = ''"foo." static'';
    local-zone-tag = ''"foo." "local_net"'';
    local-data = [
      "router.foo. IN A 10.0.0.2"
      "server.foo. IN A 10.0.0.20"
    ];
    local-data-ptr = [
      "10.0.0.2 router.foo."
      "10.0.0.20 server.foo."
    ];
  };
}

[andir]

@ofborg test unbound

In general I dislike adding more complexity to the modules. We are far far away from a string templating machine. In fact, for the first time in a long long time, the module produced invalid configuration that was only caught during runtime.

[infinisil]

Yeah that is the tradeoff for such settings options:

• You get all the flexibility you could ever need
• In turn, invalid configurations are only caught later (or not at all if the program is lenient)

We can however always add more checks to catch errors earlier, by way of adding more options for specific settings values.

[infinisil]

In this case for example, you could add options for local-zone and local-data which do the string -> list coercion for their values.

[pennae]

as said on irc, we don't think this is a good idea. some keys have list semantics and some have last-occurence semantics, but it would be prohibitive to encode that in the nix module. the best approach may be to extend the documentation somewhat with a larger example that includes the incantations necessary to merge multiple definitions somewhat safely.

[infinisil]

Closing for now, but for future reference, it would also be possible to allow only strings to be list-coerced with

diff --git a/nixos/modules/services/networking/unbound.nix b/nixos/modules/services/networking/unbound.nix
index 3b0096e2000..d754cb211ee 100644
--- a/nixos/modules/services/networking/unbound.nix
+++ b/nixos/modules/services/networking/unbound.nix
@@ -100,9 +100,8 @@ in {
         type = with types; submodule {
 
           freeformType = let
-            validSettingsPrimitiveTypes = oneOf [ int str bool float ];
             coercedList = t: coercedTo t singleton (listOf t);
-            validSettingsTypes = coercedList validSettingsPrimitiveTypes;
+            validSettingsTypes = oneOf [ int (coercedList str) bool float ];
             settingsType = coercedList (attrsOf validSettingsTypes);
           in attrsOf (oneOf [ string settingsType ])
               // { description = ''

The merging behavior can in general be modified greatly :)