Subject: [bug#77204] [PATCH v3 2/3] services: dnsmasq: Add pid-file, conf-file and conf-dir configuration fields.

From: Maxim Cournoyer <[email-masked]>
Date: Tue, 22 Apr 2025 14:35:30 +0900

Hi,

Alexey Abramov <[email-masked]> writes:

> * gnu/services/dns.scm (<dnsmasq-configuration>)[pid-file]: New field to
>   specify alternate path for dnsmasq PID.
> * gnu/services/dns.scm (<dnsmasq-configuration>)[conf-file]: New field to
>   specify one or more configuration files.
> * gnu/services/dns.scm (<dnsmasq-configuration>)[conf-dir]: New field to
>   read configuration files from a directory.
> * gnu/services/dns.scm (<dnsmasq-configuration>)[extra-options]: Move
>   to the end of the definition as a last resort option.
> * gnu/services/dns.scm (dnsmasq-shepherd-service): Use the new fields
>   instead of hardcoded values.
> * gnu/services/dns.scm: Export all record accessors.
> * doc/guix.texi: Document the new configuration options.

As mentioned earlier, file names should only appear once.

> ---
>  doc/guix.texi        |  14 ++++
>  gnu/services/dns.scm | 157 ++++++++++++++++++++++++++++---------------
>  2 files changed, 115 insertions(+), 56 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 90fa6779657..1bbd1824a87 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -35076,6 +35076,9 @@ DNS Services
>  Likewise, @code{shepherd-requirement} is a list of Shepherd service names
>  (symbols) that this service will depend on.
>  
> +@item @code{pid-file} (default: @code{"/run/dnsmasq.pid"})
> +Specify an alternate path for dnsmasq to record its process-id in.
> +
>  @item @code{no-hosts?} (default: @code{#f})
>  When true, don't read the hostnames in /etc/hosts.
>  
> @@ -35196,6 +35199,17 @@ DNS Services
>  resolving MAC addresses is only possible if the client is in the local
>  network or obtained a DHCP lease from dnsmasq.
>  
> +@item @code{conf-file} (default: @code{'()})
> +Specify a configuration file or multiple.  The given value should be a list of
> +string paths to the configuration files. File-like objects are also supported.
> +
> +@item @code{conf-dir} (default: @code{#f})
> +Read all the files in the given directory as configuration
> +files. @command{dnsmasq} also supports extensions for the field, but
> +here it is not implemented. It is more convenient to make
> +@code{computed-file} directory in the store and use that. Files are
> +loaded in alphabetical order of filename.
> +
>  @item @code{extra-options} (default: @code{'()})
>  This option provides an ``escape hatch'' for the user to provide arbitrary
>  command-line arguments to @command{dnsmasq} as a list of strings.
> diff --git a/gnu/services/dns.scm b/gnu/services/dns.scm
> index f617f26891d..210fef4ece2 100644
> --- a/gnu/services/dns.scm
> +++ b/gnu/services/dns.scm
> @@ -55,6 +55,38 @@ (define-module (gnu services dns)
>  
>              dnsmasq-service-type
>              dnsmasq-configuration
> +            dnsmasq-configuration-package
> +            dnsmasq-configuration-shepherd-provision
> +            dnsmasq-configuration-shepherd-requirement
> +            dnsmasq-configuration-pid-file
> +            dnsmasq-configuration-no-hosts?
> +            dnsmasq-configuration-port
> +            dnsmasq-configuration-local-service?
> +            dnsmasq-configuration-listen-address
> +            dnsmasq-configuration-resolv-file
> +            dnsmasq-configuration-no-resolv?
> +            dnsmasq-configuration-forward-private-reverse-lookup?
> +            dnsmasq-configuration-query-servers-in-order?
> +            dnsmasq-configuration-servers
> +            dnsmasq-configuration-servers-file
> +            dnsmasq-configuration-addresses
> +            dnsmasq-configuration-cache-size
> +            dnsmasq-configuration-negative-cache?
> +            dnsmasq-configuration-cpe-id
> +            dnsmasq-configuration-tftp-enable?
> +            dnsmasq-configuration-tftp-no-fail?
> +            dnsmasq-configuration-tftp-single-port?
> +            dnsmasq-tftp-secure?
> +            dnsmasq-tftp-max
> +            dnsmasq-tftp-mtu
> +            dnsmasq-tftp-no-blocksize?
> +            dnsmasq-tftp-lowercase?
> +            dnsmasq-tftp-port-range
> +            dnsmasq-tftp-root
> +            dnsmasq-tftp-unique-root
> +            dnsmasq-configuration-conf-file
> +            dnsmasq-configuration-conf-dir
> +            dnsmasq-configuration-extra-options
>  
>              unbound-service-type
>              unbound-configuration
> @@ -750,60 +782,65 @@ (define-record-type* <dnsmasq-configuration>
>                                  (default '(dnsmasq)))
>    (shepherd-requirement         dnsmasq-configuration-shepherd-requirement
>                                  (default '(user-processes networking)))
> -  (no-hosts?        dnsmasq-configuration-no-hosts?
> -                    (default #f))       ;boolean
> -  (port             dnsmasq-configuration-port
> -                    (default 53))       ;integer
> -  (local-service?   dnsmasq-configuration-local-service?
> -                    (default #t))       ;boolean
> -  (listen-addresses dnsmasq-configuration-listen-address
> -                    (default '()))      ;list of string
> -  (extra-options    dnsmasq-configuration-extra-options
> -                    (default '()))      ;list of string
> -  (resolv-file      dnsmasq-configuration-resolv-file
> -                    (default "/etc/resolv.conf")) ;string
> -  (no-resolv?       dnsmasq-configuration-no-resolv?
> -                    (default #f))       ;boolean
> +  (pid-file                     dnsmasq-configuration-pid-file
> +                                (default "/run/dnsmasq.pid")) ;string
> +  (no-hosts?                    dnsmasq-configuration-no-hosts?
> +                                (default #f))       ;boolean
> +  (port                         dnsmasq-configuration-port
> +                                (default 53))       ;integer
> +  (local-service?               dnsmasq-configuration-local-service?
> +                                (default #t))       ;boolean
> +  (listen-addresses             dnsmasq-configuration-listen-address
> +                                (default '()))      ;list of string
> +  (resolv-file                  dnsmasq-configuration-resolv-file
> +                                (default "/etc/resolv.conf")) ;string
> +  (no-resolv?                   dnsmasq-configuration-no-resolv?
> +                                (default #f))       ;boolean
>    (forward-private-reverse-lookup?
> -                    dnsmasq-configuration-forward-private-reverse-lookup?
> -                    (default #t))       ;boolean
> -  (query-servers-in-order?
> -                    dnsmasq-configuration-query-servers-in-order?
> -                    (default #f))       ;boolean
> -  (servers          dnsmasq-configuration-servers
> -                    (default '()))      ;list of string
> -  (servers-file     dnsmasq-configuration-servers-file
> -                    (default #f))       ;string|file-like
> -  (addresses        dnsmasq-configuration-addresses
> -                    (default '()))      ;list of string
> -  (cache-size       dnsmasq-configuration-cache-size
> -                    (default 150))      ;integer
> -  (negative-cache?  dnsmasq-configuration-negative-cache?
> -                    (default #t))       ;boolean
> -  (cpe-id           dnsmasq-configuration-cpe-id
> -                    (default #f))       ;string
> -  (tftp-enable?     dnsmasq-configuration-tftp-enable?
> -                    (default #f))       ;boolean
> -  (tftp-no-fail?    dnsmasq-configuration-tftp-no-fail?
> -                    (default #f))       ;boolean
> -  (tftp-single-port? dnsmasq-configuration-tftp-single-port?
> -                    (default #f))       ;boolean
> -  (tftp-secure?     dnsmasq-tftp-secure?
> -                    (default #f))       ;boolean
> -  (tftp-max         dnsmasq-tftp-max
> -                    (default #f))       ;integer
> -  (tftp-mtu         dnsmasq-tftp-mtu
> -                    (default #f))       ;integer
> -  (tftp-no-blocksize? dnsmasq-tftp-no-blocksize?
> -                      (default #f))     ;boolean
> -  (tftp-lowercase?  dnsmasq-tftp-lowercase?
> -                    (default #f))       ;boolean
> -  (tftp-port-range  dnsmasq-tftp-port-range
> -                    (default #f))       ;string
> -  (tftp-root        dnsmasq-tftp-root
> -                    (default "/var/empty,lo")) ;string
> -  (tftp-unique-root dnsmasq-tftp-unique-root
> -                    (default #f)))      ;"" or "ip" or "mac"
> +                                dnsmasq-configuration-forward-private-reverse-lookup?
> +                                (default #t))       ;boolean
> +  (query-servers-in-order?      dnsmasq-configuration-query-servers-in-order?
> +                                (default #f))       ;boolean
> +  (servers                      dnsmasq-configuration-servers
> +                                (default '()))      ;list of string
> +  (servers-file                 dnsmasq-configuration-servers-file
> +                                (default #f))       ;string|file-like
> +  (addresses                    dnsmasq-configuration-addresses
> +                                (default '()))      ;list of string
> +  (cache-size                   dnsmasq-configuration-cache-size
> +                                (default 150))      ;integer
> +  (negative-cache?              dnsmasq-configuration-negative-cache?
> +                                (default #t))       ;boolean
> +  (cpe-id                       dnsmasq-configuration-cpe-id
> +                                (default #f))       ;string
> +  (tftp-enable?                 dnsmasq-configuration-tftp-enable?
> +                                (default #f))       ;boolean
> +  (tftp-no-fail?                dnsmasq-configuration-tftp-no-fail?
> +                                (default #f))       ;boolean
> +  (tftp-single-port?            dnsmasq-configuration-tftp-single-port?
> +                                (default #f))       ;boolean
> +  (tftp-secure?                 dnsmasq-tftp-secure?
> +                                (default #f))       ;boolean
> +  (tftp-max                     dnsmasq-tftp-max
> +                                (default #f))       ;integer
> +  (tftp-mtu                     dnsmasq-tftp-mtu
> +                                (default #f))       ;integer
> +  (tftp-no-blocksize?           dnsmasq-tftp-no-blocksize?
> +                                (default #f))       ;boolean
> +  (tftp-lowercase?              dnsmasq-tftp-lowercase?
> +                                (default #f))       ;boolean
> +  (tftp-port-range              dnsmasq-tftp-port-range
> +                                (default #f))       ;string
> +  (tftp-root                    dnsmasq-tftp-root
> +                                (default "/var/empty,lo")) ;string
> +  (tftp-unique-root             dnsmasq-tftp-unique-root
> +                                (default #f))       ;"" or "ip" or "mac"
> +  (conf-file                    dnsmasq-configuration-conf-file
> +                                (default '()))      ;list of string|file-like
> +  (conf-dir                     dnsmasq-configuration-conf-dir
> +                                (default #f))       ;string|file-like
> +  (extra-options                dnsmasq-configuration-extra-options
> +                                (default '())))

Please refrain from adjusting the indentation like this; it makes
reviewing difficult and here also breaks our max width of 80 columns.

>  (define (warn-deprecated-dnsmasq-configuration-provision value)
>    (when (pair? value)
> @@ -818,6 +855,7 @@ (define (dnsmasq-shepherd-service config)
>       provision
>       shepherd-provision
>       shepherd-requirement
> +     pid-file
>       no-hosts?
>       port local-service? listen-addresses
>       resolv-file no-resolv?
> @@ -829,7 +867,8 @@ (define (dnsmasq-shepherd-service config)
>       tftp-single-port? tftp-secure?
>       tftp-max tftp-mtu tftp-no-blocksize?
>       tftp-lowercase? tftp-port-range
> -     tftp-root tftp-unique-root extra-options)
> +     tftp-root tftp-unique-root
> +     conf-file conf-dir extra-options)
>      (shepherd-service
>       (provision (or provision shepherd-provision))
>       (requirement shepherd-requirement)
> @@ -838,7 +877,7 @@ (define (dnsmasq-shepherd-service config)
>                 (list
>                  #$(file-append package "/sbin/dnsmasq")
>                  "--keep-in-foreground"
> -                "--pid-file=/run/dnsmasq.pid"
> +                (string-append "--pid-file=" #$pid-file)
>                  #$@(if no-hosts?
>                         '("--no-hosts")
>                          '())
> @@ -909,8 +948,14 @@ (define (dnsmasq-shepherd-service config)
>                              (format #f "--tftp-unique-root=~a" tftp-unique-root)
>                              (format #f "--tftp-unique-root")))
>                         '())
> +                #$@(map (lambda (conf-file)
> +                          #~(string-append "--conf-file=" #$conf-file))
> +                        conf-file)
> +                #$@(if conf-dir
> +                       (list #~(string-append "--conf-dir=" #$conf-dir))
> +                       '())
>                  #$@extra-options)
> -               #:pid-file "/run/dnsmasq.pid"))
> +               #:pid-file #$pid-file))
>       (stop #~(make-kill-destructor)))))

Other than that, it looks like a good change!  Thank you.

-- 
Thanks,
Maxim