unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Chris Marusich <cmmarusich@gmail.com>
To: Oleg Pykhalov <go.wigust@gmail.com>
Cc: guix-devel@gnu.org
Subject: Re: isc-bind service draft
Date: Tue, 14 Nov 2017 20:48:07 -0800	[thread overview]
Message-ID: <87po8kno54.fsf@gmail.com> (raw)
In-Reply-To: <87vaijkyam.fsf@gmail.com> (Oleg Pykhalov's message of "Thu, 09 Nov 2017 23:11:13 +0300")

[-- Attachment #1: Type: text/plain, Size: 10461 bytes --]

Hi Oleg,

Oleg Pykhalov <go.wigust@gmail.com> writes:

> I work on isc-bind service.  Currently generation of named.conf is done.
> Ideas and suggestions are welcome!  :-)

Awesome!  Thank you for working on this.  I'm not familiar with BIND
configuration, so I can't really comment much on the particular fields
you've chosen to include in the various configuration objects you've
created.  It'd be nice if someone more familiar with BIND could give it
a look.

> (define-record-type* <bind-options-configuration>

Are these options intended to be used when invoking bind?  If so, maybe
a name like "bind-options" is probably good enough.

>   bind-options-configuration make-bind-options-configuration
>   bind-options-configuration?
>   (user             bind-options-configuration-user          ; string
>                     (default "bind"))
>   (group            bind-options-configuration-group         ; string
>                     (default "bind"))
>   (run-directory    bind-options-configuration-run-directory ; string
>                     (default "/var/run/bind"))
>   (pid-file         bind-options-configuration-pid-file      ; string
>                     (default "/var/run/bind/named.pid"))

For what it's worth, nowadays some distros use /run as the "run
directory" [1].  I don't know if GuixSD has adopted any particular
policy about whether to use /var/run or /run for the default "run
directory".  I don't currently know of any reason why it matters much,
so I think it's fine to use /var/run here.

>   (listen-v4        bind-options-configuration-listen-v4     ; string
>                     (default "0.0.0.0"))
>   (listen-v6        bind-options-configuration-listen-v6     ; string
>                     (default "::"))
>   (listen-port      bind-options-configuration-listen-port   ; integer
>                     (default 53))
>   (allow-recursion? bind-configuration-allow-recursion?      ; list
>                     (default (list "127.0.0.1")))
>   (allow-transfer?  bind-configuration-allow-transfer?       ; list
>                     (default (list "none")))
>   (allow-update?    bind-configuration-allow-update?         ; list
>                     (default (list "none")))
>   (version          bind-configuration-version               ; string
>                     (default "none"))
>   (hostname         bind-configuration-hostname              ; string
>                     (default "none"))

Why not use the system's host name by default?  For example:

   (hostname         bind-configuration-hostname              ; string
                     (default (gethostname)))

>   
>   (server-id bind-configuration-server-id ; string (default "none")))
>
> (define (bind-configuration-statement-string statements)
>   (string-join (list "{" (string-join statements ";\n") "}")))
>

You could also write it like this:

 (define (bind-configuration-statement-string statements)
   (string-append "{" (string-join statements ";\n") "}"))

>
> (define-record-type* <bind-zone-configuration>
>   bind-zone-configuration make-bind-zone-configuration
>   bind-zone-configuration?
>   (network bind-zone-configuration-network  ; string
>            (default '()))
>   (class   bind-zone-configuration-class    ; string
>     (default '()))
>   (type    bind-zone-configuration-type     ; string
>            (default '()))
>   (file    bind-zone-configuration-filename ; string
>            (default '())))
>
> (define-record-type* <bind-configuration-file>
>   bind-configuration-file make-bind-configuration-file
>   bind-configuration-file?
>
>   ;; <bind-options-configuration>
>   (config-options bind-configuration-file-config-options
>                   (default (bind-options-configuration)))
>
>   ;; list of <bind-zone-configuration>
>   (config-zones   bind-configuration-file-config-zones
>                   (default (list (bind-zone-configuration
>                                   (network "localhost")
>                                   (class   "IN")
>                                   (type    "master")
>                                   (file    "localhost.zone"))
>                                  (bind-zone-configuration
>                                   (network "0.0.127.in-addr.arpa")
>                                   (class   "IN")
>                                   (type    "master")
>                                   (file    "127.0.0.zone"))
>                                  (bind-zone-configuration
>                                   (network (string-append "1.0.0.0.0.0.0.0.0.0."
>                                                           "0.0.0.0.0.0.0.0.0.0."
>                                                           "0.0.0.0.0.0.0.0.0.0."
>                                                           "0.0.ip6.arpa"))
>                                   (class   "IN")
>                                   (type    "master")
>                                   (file    "localhost.ip6.zone"))
>                                  (bind-zone-configuration
>                                   (network "255.in-addr.arpa")
>                                   (class   "IN")
>                                   (type    "master")
>                                   (file    "empty.zone"))
>                                  (bind-zone-configuration
>                                   (network "0.in-addr.arpa")
>                                   (class   "IN")
>                                   (type    "master")
>                                   (file    "empty.zone"))
>                                  (bind-zone-configuration
>                                   (network ".")
>                                   (class   "IN")
>                                   (type    "master")
>                                   (file    "root.hint"))))))

What is the intended behavior of these defaults?  In what situations
will they work, and in what situations will they not?  It might be good
to put a comment in that explains the intended default behavior and why
it is reasonable.

> (define-record-type* <bind-configuration>
>   bind-configuration make-bind-configuration
>   bind-configuration?
>   (config-file bind-configuration-config-file
>                (default (bind-configuration-file)))
>   (package     bind-configuration-package ; <package>
>                (default bind)))
>
> (define-syntax option
>   (syntax-rules ()
>     ((_ key value) (if value
>                        (list "    " (string-join (list key value)) ";" "\n")
>                        '()))))

Does this need to be a macro?  By the way, you could use string-append
here, too, to make it simpler.

> (define-syntax key/value
>   (syntax-rules ()
>     ((_ (key value) rest ...)
>      (append (option key value)
>              (key/value rest ...)))
>     ((_) '())))

Does this need to be a macro?

> (define (emit-bind-zones-config zone)
>   (match zone
>     (($ <bind-zone-configuration> network class type file)
>      (list (string-join `(,(string-join (list "zone"
>                                               (string-append "\""
>                                                              network
>                                                              "\"")
>                                               class "{\n"))
>                           ,@(key/value ("type" type)
>                                        ("file" file))
>                           "};\n")
>                         "")))))
>
> (define (emit-bind-options-config options)
>   (match options
>     (($ <bind-options-configuration> user _ run-directory pid-file
>                                      listen-v4 listen-v6 listen-port
>                                      allow-recursion? allow-transfer?
>                                      allow-update?
>                                      version hostname server-id)

Some of these slots (e.g., listen-v4) appear to be un-used.  Instead of
listing positional slots by name, maybe it would be better to bind the
entire <bind-options-configuration> to a variable, and then use the
accessor procedures (e.g., bind-options-configuration-listen-v4) to get
just the attributes you need.  This has the benefit of being more
resilient to refactorings which change the order of fields in the
record, also.  I realize that a lot of the code in Guix relies on
positional matching of slots like this, so I don't mind if you keep it
as-is, but consider my suggestion as food for thought.

>      `("options {\n"
>        ,@(key/value ("directory" run-directory)
>                     ("pid-file" pid-file)
>                     ("allow-recursion"
>                      (bind-configuration-statement-string allow-recursion?))
>                     ("allow-transfer"
>                      (bind-configuration-statement-string allow-transfer?))
>                     ("allow-update"
>                      (bind-configuration-statement-string allow-update?))
>                     ("version" version)
>                     ("hostname" hostname)
>                     ("server-id" server-id))
>        "};\n"))))
>
> (define-gexp-compiler (bind-configuration-compiler
>                        (file <bind-configuration>) system target)
>   (match file
>     (($ <bind-configuration> config-file)
>      (match config-file
>        (($ <bind-configuration-file> config-options config-zones)
>         (apply text-file* "named.conf"
>                (append (fold append '() (map emit-bind-zones-config config-zones))
>                        (emit-bind-options-config config-options))))))))
>

Is it necessary to define a gexp compiler here?  I would have thought we
could just invoke plain-file or text-file instead (see (guix)
G-Expressions in the Guix manual).  Why can't we?  Other services do
this; for example, see the service definitions in gnu/services/mail.scm.

Also, is it possible for a user to pass in an existing configuration
file to be used verbatim, or included somewhere in the config?  Having
an "escape hatch" like that seems useful for most services; perhaps it
could be useful here, too.

Footnotes: 
[1]  https://unix.stackexchange.com/questions/13972/what-is-this-new-run-filesystem

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  reply	other threads:[~2017-11-15  4:48 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-09 20:11 isc-bind service draft Oleg Pykhalov
2017-11-15  4:48 ` Chris Marusich [this message]
2017-11-16  3:21   ` Chris Marusich
2017-11-16 16:18     ` Ludovic Courtès
2017-11-24  8:31       ` Oleg Pykhalov
2017-11-30 17:03         ` Ludovic Courtès
2017-11-16 16:19 ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87po8kno54.fsf@gmail.com \
    --to=cmmarusich@gmail.com \
    --cc=go.wigust@gmail.com \
    --cc=guix-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).