This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/nixos-19-03-feature-freeze/1950/24

Also, any chance for a nixos test?

Do you have a relevant example I could peek from?

Also, any chance for a nixos test?

Do you have a relevant example I could peek from?

Nothing specifically in mind, just always good to have a test in nixos/tests for every module.

Updated. I added the test in the second commit and, while I was at it, added support for subordinate servers in the third commit (properly extending the test fixture).

Updated. I added the test in the second commit and, while I was at it, added support for subordinate servers in the third commit (properly extending the test fixture).

Glad to see a test. Looks like a new issue or two to work through, though.

Okay, I think, I satisfied the build bot.

services.tinydns.records = [
  { data = ".example.com:1.2.3.4:a"; }
  { SRV = f: f "_some.service.example.com" 1 1 123 "server.example.com."; }
];

I'm not a fan of this representation, because

• What does it mean when an attribute set has a data and a SRV key?
• Having to pass a function as config is in general discouraged as these aren't serializable. See also lib/types: add the selectorFunction type #44601 as well.
• The function is really just used as a way to get a library function in scope without having to import the library, which is weird
• The arguments of the function are hard to know what they are. In NixOS I especially like how all options have a proper name, this would eliminate that benefit.

So instead I propose this:

{ lib, ... }: {
  services.tinydns.records = with lib.dns; [
    (raw ".example.com:1.2.3.4:a")
    (srv "_some.service.example.com" 1 1 123 "server.example.com.")
  ];
}

Where these functions are defined somewhat like this in lib/dns.nix:

{
  raw = data: {
    type = "raw";
    value = data;
  };
  
  srv = service: priority: weight: port: target: {
    type = "srv";
    inherit service priority weight port target;
  };
  
  # ...
}

Namely, these functions should just capture the arguments, leaving the interpretation and encoding to tinydns. Therefore these functions should be compatible and usable by other dns servers as well. Like this, the user is still free to use full attribute names (which will also be in the docs).

• What does it mean when an attribute set has a data and a SRV key?

It is an error, nix will tell you with a clear message that only one record type is allowed and yes, this is an oddity. I use it to have proper named NixOS options.

• Having to pass a function as config is in general discouraged as these aren't serializable. See also lib/types: add the selectorFunction type #44601 as well.

That is true in general and I am not proposing this format without proper thought. The reason I still chose functions was, that the data needs pre-computation (e.g. transformation of var-length strings to a list of fixed-length strings). This is a complexity level that cannot be easily hidden without hurting other properties badly.

• The function is really just used as a way to get a library function in scope without having to import the library, which is weird

I'm sorry, but this is not the reason for it. This notation was the only way I found to feed nix an un-evaluated lambda function. If you look closely, you will notice that you are allowed to do the following:

{
  services.tinydns.records = [
    { SRV = f: f "_service.example.com" 1 1 23 "server.example.com"; }
    { SRV = f: f "_service.example.com" 1 1 23 "replacement-server.example.com" "" "future-timestamp"; }
  ];
}

With this construct, you can supply a variable amount of arguments to a lambda notation. I have not found another way to do this with nix.

Why all the effort? As I wrote in my initial PR, on a medium-sized DNS Server you have to maintain thousands of records. It is not these one or two records that might look weird in the beginning but screenfuls of it. Interspersing data with named arguments or supplying fake argument due to the lack of support for variable amount of arguments did not seem to cut it. Reducing line noise was a top priority.

• The arguments of the function are hard to know what they are. In NixOS I especially like how all options have a proper name, this would eliminate that benefit.

I cannot follow your argument. When you look closely you see that the record type is distinguished by an attribute name on the left. This is, of course, implemented as a NixOS option and, thus, has proper documentation in the right place. Please have a look at the generated documentation to see if that fits together.

srv = service: priority: weight: port: target: {
type = "srv";
inherit service priority weight port target;
};

This would have the following drawbacks:

• Function documentation separate from NixOS module documentation
• No varargs to functions, i.e. a lot of fake args in the module.

Namely, these functions should just capture the arguments, leaving the interpretation and encoding to tinydns. Therefore these functions should be compatible and usable by other dns servers as well. Like this, the user is still free to use full attribute names (which will also be in the docs).

This can also be done in the current PR by using the data record type.

Having said all this: This is not my first PR to this project and I am very open to changes. Maintaining DNS records is a very special task that only hits very few users (but hits them hard). And I am proposing this non-obvious module with good reason and a lot of thought. So, if this does not find consent, I will gladly change it, but I beg to not ask for changes lightly or to just streamline it with the rest of the NixOS modules.

Well, personally I'm not convinced this representation of DNS RRs is a good approach. I believe the zonefile format is nice – to write by hand, easy to generate from whatever tools you like (including nix), and you can run a checker inside a derivation on it (e.g. load it into a server) – I see little motivation for such checks to be evaluation-time. Also it's been widely understood for decades.

EDIT: hmm, perhaps for some kinds of use cases, like overriding particular RRs 🤔

Still, I'm not tidnydns kind of person anyway, so I'll probably just leave the above comment here and not interfere anymore :-) I do DNS for job, though I'm on the resolver side.

This still seems fairly contentious; I'm not very confident these issues will be resolved within the next week, so I'd suggest postponing this for 19.09. Unless of course everyone's happy with this by branch-off :)

Well, personally I'm not convinced this representation of DNS RRs is a good approach.

True, but this is not my position, here. I am convinced by the performance of the software and try to provide a reasonable means of configuring it via NixOS. I just take the input data format as a necessary thing this NixOS module has to provide. So, I am not making the rules, just reacting on them.

This still seems fairly contentious; I'm not very confident these issues will be resolved within the next week, so I'd suggest postponing this for 19.09. Unless of course everyone's happy with this by branch-off :)

What would be the procedure for progress? I worry that this PR is just forgotten for good in a month or so. (Given that it was initially open for 3 months without any action.)

NixPkgs merges over a thousand PRs per month, so it's quite common that "less interesting" things just fall through the cracks, unless e.g. the package has maintainers that react when you ping them, etc.

I'd suggest (as an option which I can see leading to this getting merged before the 19.03 branch-off) reworking this to take out the controversial data generation and focus on the other improvements — enabling reload instead of restart and the high-availability mechanism. The data generation could be moved into its own module in NUR or similar.

Okay. I will rework this to just feature the pure (already encoded) data file and the rest of the improvements. I will add the data transformation functions to the library as @infinisil originally suggested.

@lheckemann, @infinisil I hope this is in acceptable form, now. Thanks for the review and hints!

Oh actually regarding the vararg thing, there is a slightly unorthodox way to get that:

nix-repl> f = { args = []; __functor = self: arg: self // { args = self.args ++ [ arg ]; }; }

nix-repl> (f 10 20 30).args
[ 10 20 30 ]

__functor is documented in the Nix manual so I think it's fine to use it, and this is a very good use case imo.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-in-distress/3604/18

please remove the merge commit, and use git pull -r origin master when updating a PR, this way the git history remains clean

Fixed.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-dns-a-nix-dsl-for-dns-zone-files/2466/4

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-dns-a-nix-dsl-for-dns-zone-files/2466/5

Since I wrote one of the other implementations of Nix DSLs mentioned on Discourse for DNS I'm biased, so I tried to not commented a whole lot on lib/tinydns.nix (plus others before me already did that). That said however, I don't think it's generic enough for this to be residing in lib/.

I would be gladly moving this out of lib/, but when I first made this PR there were no other entry points to make the functions available to the module user.

Additionally the way you've implemented zone transfers is also highly subjective and opinionated, so I'd also leave that up to the user on deciding how to do zone transfers (if they're even needed at all, eg. if you're not deploying your DNS servers synchronously). Maybe even just allow to specify custom commands - but then again - this is already possible using systemd.services.foo.preStart &c.

I do use synchronously deployed DNS servers. I implemented zone transfers as recommended by the author of tinydns. AXFR zone transfers have many known security issues, so while they are supported by tinydns, I didn't implement them. In fact, the only reason that the axfrdns is also running is to serve DNS records larger than 512kB and to accommodate certain clients that refuse to do UDP DNS queries.

IMHO this is waaay too complex and specialized to have in the nixpkgs repository. I think it would be better to have this kind of module in a separate repository (e.g. as a flake).

Also, reminder that Nix is not a general-purpose language and it's definitely not an appropriate language in which to write parsers. That sort of thing is likely to be very slow and memory-hungry. (BTW, lib is supposed to be generic. There should never be a file named lib/tinydns.nix.)

IMHO this is waaay too complex and specialized to have in the nixpkgs repository. I think it would be better to have this kind of module in a separate repository (e.g. as a flake).

I'll reduce this to take a list of entries, then, and put the lib/ code into a separate repository. Nix flakes wasn't an option back then and the only reason to have it in lib/ was that I did not find another way to make the functionality available to the user.

EDIT: The consequence of removing this functionality is, though, that the record database is practically forced to be monolithic. Every other nixos module would have to pull in the record-building functions from the outside, which would clearly not be compatible with upstream. No other nixos module could then supply DNS records. This is maybe acceptable, but I wanted to make that point known.

Also, reminder that Nix is not a general-purpose language and it's definitely not an appropriate language in which to write parsers. That sort of thing is likely to be very slow and memory-hungry. (BTW, lib is supposed to be generic. There should never be a file named lib/tinydns.nix.)

Just to be clear, I did not write a parser anywhere. It is just a generator that takes nix attributes and outputs the data formatted for tinydns. The tinydns database is never parsed or interpreted.

IMHO this is waaay too complex and specialized to have in the nixpkgs repository. I think it would be better to have this kind of module in a separate repository (e.g. as a flake).

I'll reduce this to take a list of entries, then, and put the lib/ code into a separate repository. Nix flakes wasn't an option back then and the only reason to have it in lib/ was that I did not find another way to make the functionality available to the user.

Referring to other flakes from inside nixpkgs probably isn't an option for quite some time.

Mainline nix doesn't support flakes yet, and minver.nix is still on 2.2 - so whatever code is added, it'd need to be compatible with Nix 2.2.

Adding config format generators to nixpkgs, not parsers, is probably fine, and has been done for many formats (see lib/generators.nix).

I agree the tinydns config format isn't generic enough to live in lib/generators.nix - but I wouldn't be opposed to having it in a nixos/modules/services/networking/tinydns/lib.nix, for example.

I could greatly simplify the approach without sacrificing functionality. Internally, the tinydns."xxx".data option is now a list of attribute sets.

The verbatim data record type, which contains opaque server-specific data:

{ type = "DATA"; data = "arbitrary verbatim output data"; }

Any other record type:

{ type = "record type"; ttl = "..."; ts = timestamp; location = split-horizon; <other record-specific attributes> };

e.g.

{ type = "TXT"; service = "_foo.service.domain"; txt = "text record content"; ttl = ""; ts = ""; location = ""; }

Records can be specified as strings (become DATA), lists with the record type as the first element (terse, by-order-of-arguments specification), or as attribute sets:

services.tinydns."0.0.0.0".data = [
''
  =a.b.c.d:10.0.0.1
  +e.b.c.d:10.0.0.1''
[ "TXT" "_kerberos.b.c.d" "B.C.D" ]
{ type = "SRV"; service = "_printers.b.c.d."; priority = 10; weight = 10; port = 631; target = "e.b.c.d."; }

Things, I'd like to point out:

• The terse by-order-of-arguments format is still somewhat specific to tinydns. The last three elements of the list always denote ttl, ts and location. But this is not a global definition, other DNS servers could specify a different order of arguments.
• The internal value of data is exclusively a list of attribute sets. So this should be easily convertible to arbitrary zone formats. (Except for DATA records, that obviously are server-specific).
• The internal structure of the record attribute set should be agreed upon but is easily extensible, as each DNS server only draws from the attributes it knows about. Maybe this could be a good basis for a generic DNS data format.
• Conversion of the internal data value to the output format is now done completely inside the module. There is no need for a library anymore.

@spacefrogg: Okay, this is going into a similar direction of Nix-dns now, except that you're using "list of X" versus "attribute set of X" and a few other semantic differences.

However, I'm not even sure if anyone would even want Nix-dns or even my implementation for that matter, so maybe I should have made it more clear earlier:

Please stop wasting your time going forward with this until at least one of the reviewers here thinks this is heading into the right direction.

I personally had lots of pull request going into the trash bin as well, even ones that were less controversial than this one, and if I parse the comments here correctly I think this is leading nowhere.

For reference, for opinionated packages/modules that are not general enough for nixpkgs/NixOS, we have the NUR, the Nix User Repository, to make those a bit easier to use.

So maybe @jerith666 (who did the initial implementation of the NixOS module and package) ... can chime in here as well in order to have some more (possibly unbiased) input on this.

Thanks for tagging me, I hadn't seen this yet.

I'm generally in favor of having a nix DSL for specifying DNS information. I think it'd be best if that DSL were server-agnostic, and let each server (nsd, tinydns, etc.) implement a translation layer to output its particular config file format.

I don't think I'm qualified to comment on much else here. My use case for tinydns is super simple, I just use it for SOA, NS, and A records for machines on my local network. I'm confident I could adapt to whatever others agree on! :)

Closing this as too opinionated and narrow scoped.