GNU bug report logs - #68757
[PATCH] services: dns: Add unbound service

Previous Next

Full log

Message #8 received at 68757 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: soeren <at> soeren-tempel.net
Cc: 68757 <at> debbugs.gnu.org
Subject: Re: [bug#68757] [PATCH] services: dns: Add unbound service
Date: Sun, 18 Feb 2024 16:18:17 +0100

Hi Sören,

soeren <at> soeren-tempel.net skribis:

> From: Sören Tempel <soeren <at> soeren-tempel.net>
>
> This allows using Unbound as a local DNSSEC-enabled resolver. This
> commit also allows configuration of the Unbound DNS resolver via a
> Scheme API. Conceptually, the Unbound configuration consists of several
> "sections" that contain key-value pairs (see unbound.conf(5)). The
> configuration sections are modeled in Scheme using record-type fields,
> where each field expects a list of pairs.
>
> A sample configuration, which uses a DoT forwarder, looks as follows:
>
> 	(service unbound-service-type
> 	  (unbound-configuration
> 	    (forward-zone
> 	      '((name . ".")
> 	        (forward-addr . "149.112.112.112#dns.quad9.net")
> 	        (forward-addr . "2620:fe::9#dns.quad9.net")
> 	        (forward-tls-upstream . yes)))))
>
> * gnu/service/dns.scm (serialize-list): New procedure.
> * gnu/service/dns.scm (unbound-configuration): New record.
> * gnu/service/dns.scm (unbound-config-file): New procedure.
> * gnu/service/dns.scm (unbound-shepherd-service): New procedure.
> * gnu/service/dns.scm (unbound-account-service): New constant.
> * gnu/service/dns.scm (unbound-service-type): New services.
>
> Signed-off-by: Sören Tempel <soeren <at> soeren-tempel.net>

Nice!

Some comments:

  • Please document the service in doc/guix.texi.  Make sure to include
    an example like the one above in the introduction, with
    explanations (you take remove the example from the commit log
    though).

  • Unless it’s too hard, please provide a system test (the service for
    knot lacks one for some reason, so there’s a precedent, but the
    general rule is that system services should always have associated
    tests.)

> +(define-configuration unbound-configuration

I recommend adding an “escape hatch” by which users may provide raw
strings (or a file-like object) that gets inserted into the config file.

> +  (server
> +    (maybe-list '((interface . "127.0.0.1")
> +                  (interface . "::1")
> +
> +                  ;; TLS certificate bundle for DNS over TLS.
> +                  (tls-cert-bundle . "/etc/ssl/certs/ca-certificates.crt")
> +
> +                  (hide-identity . yes)
> +                  (hide-version . yes)))

Please use Scheme booleans #t and #f instead of 'yes and 'no.

> +    "The server section of the configuration.")
> +  (remote-control
> +    (maybe-list '((control-enable . yes)
> +                  (control-interface . "/run/unbound.sock")))
> +    "Configuration of the remote control facility.")

For ‘remote-control’ and ‘server’, it’s not clear to me why we resort to
alists instead of records (or fields within this record type); it looks
inconsistent.

Could you consider turning them into records or fields?

> +            (documentation "Unbound daemon.")

“Run the Unbound DNS resolver” maybe?

> +            (provision '(unbound dns))
> +            (requirement '(networking))

Add 'user-processes.  However, does it really need ‘networking’?  (See
<https://issues.guix.gnu.org/66306>.)

> +         (shell "/run/current-system/profile/sbin/nologin"))))

Rather (file-append …) as is done in other services.

> +(define unbound-service-type
> +  (service-type (name 'unbound)
> +                (description "Run the unbound DNS resolver.")

s/unbound/Unbound/

TIA,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.