unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: ludo@gnu.org (Ludovic Courtès)
To: John Darrington <jmd@gnu.org>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] gnu: Add kerberos service.
Date: Fri, 18 Nov 2016 23:51:16 +0100	[thread overview]
Message-ID: <87a8cw5rmj.fsf@gnu.org> (raw)
In-Reply-To: <1478721522-312-1-git-send-email-jmd@gnu.org> (John Darrington's message of "Wed, 9 Nov 2016 20:58:42 +0100")

Hello!

John Darrington <jmd@gnu.org> skribis:

> * gnu/services/kerberos.scm (krb5-realm, krb5-configuration,
> krb5-service-type): New variables.

Could you add documentation in guix.texi, along with an example of how
to use it?

I very strongly encourage you to write a system test for this as well.
Essentially, it’s just about writing down in a file a test that you’ve
already run anyway.  I’m happy to help if needed.  The main ideas are
described in
<https://www.gnu.org/software/guix/news/guixsd-system-tests.html>.

(I think this will become a requirement for future patches.  :-))

> +(define-record-type* <krb5-realm>
> +  krb5-realm      make-krb5-realm
> +  krb5-realm?
> +  (name                krb5-realm-name)
> +
> +  (admin-server        krb5-realm-admin-server)
> +  (kdc                 krb5-realm-kdc)
> +  (auth-to-local       krb5-realm-auth-to-local (default '()))
> +  (auth-to-local-names krb5-realm-auth-to-local-names (default '()))
> +  (http-anchors        krb5-realm-http-anchors (default '()))
> +  (default-domain      krb5-realm-default-domain (default #f))
> +  (kpasswd-server      krb5-realm-kpasswd-server (default #f))
> +  (master-kdc          krb5-realm-master-kdc (default #f))
> +  (v4-instance-convert krb5-realm-v4-instance-convert (default '()))
> +  (v4-realm            krb5-realm-v4-realm (default #f)))

I find it helpful to add a one- or two-line comment above stating what
this is, and margin comments next to the fields to give an idea of what
their type is.

Could you try something along these lines?

> +(define-syntax  guile->krb-cfg
> +  (syntax-rules ()
> +    ((guile->krb-cfg accessor what)
> +     (string-map
> +      (lambda (c) (if (eq? c #\-) #\_ c))
> +      (string-drop (symbol->string accessor)
> +                   (string-length what))))))
> +
> +(define-syntax cfg-opt-string
> +  (syntax-rules ()
> +    ((cfg-opt-string accessor realm)
> +     (if (accessor realm)
> +         (format #f "\n\t~a = ~a"
> +                        (guile->krb-cfg 'accessor "krb5-realm-")
> +                        (accessor realm))
> +         ""))))
> +
> +
> +;; Generates one line of text per list item
> +(define-syntax cfg-opt-list
> +  (syntax-rules ()
> +    ((cfg-opt-list accessor realm)
> +     (if (not (null? (accessor realm)))
> +         (string-concatenate
> +          (map (lambda (item)
> +                 (format #f "\n\t~a = ~a"
> +                         (guile->krb-cfg 'accessor "krb5-realm-")
> +                         item))
> +              (accessor realm)))
> +     ""))))

Would Andy’s ‘define-configuration’ (in mail.scm and cups.scm) be usable
here, possibly with some adjustments?  It has the advantage that
configuration fields, their types, and their docstring all appear at the
same place.  I think we should consolidate it into a single API.

If not, please mind the naming convention (info "(guix) Formatting
Code"), and use ‘define-syntax-rule’ for macros with a single pattern.

Perhaps pass the whole file through M-x indent-region to fix
inconsistencies.

> +;; For explanation of these fields see man 5 krb5.conf
> +(define-record-type* <krb5-configuration>
> +  krb5-configuration    make-krb5-configuration
> +  krb5-configuration?
> +
> +  ;; [libdefaults]
> +  (allow-weak-crypto          krb5-configuration-allow-weak-crypto (default #f))
> +  (ap-req-checksum-type       krb5-configuration-ap-req-checksum-type (default #f))
> +  (canonicalize               krb5-configuration-canonicalize (default #f))
> +  (ccache-type                krb5-configuration-ccache-type (default #f))
> +  (clockskew                  krb5-configuration-clockskew (default #f))
> +  (default-ccache-name        krb5-configuration-default-ccache-name (default #f))
> +  (default-client-keytab-name krb5-configuration-default-client-keytab-name
> +                                                                     (default #f))
> +  (default-keytab-name        krb5-configuration-default-keytab-name (default #f))
> +  (default-realm              krb5-configuration-default-realm (default #f))
> +  (default-tgs-enctypes       krb5-configuration-default-tgs-enctypes (default #f))
> +  (default-tkt-enctypes       krb5-configuration-default-tkt-enctypes (default #f))
> +  (dns-canonicalize-hostname  krb5-configuration-dns-canonicalize-hostname
> +                              (default #t))
> +  (dns-lookup-kdc             krb5-configuration-dns-lookup-kdc
> +                              (default #f))
> +  (err-fmt                    krb5-configuration-err-fmt (default #f))
> +  (extra-addresses            krb5-configuration-extra-addresses
> +                              (default #f))
> +  (forwardable                krb5-configuration-forwardable (default #t))
> +  (ignore-acceptor-hostname   krb5-configuration-ignore-acceptor-hostname
> +                              (default #f))
> +  (k5login-authoritative      krb5-configuration-k5login-authoritative (default #t))
> +  (k5login-directory          krb5-configuration-k5login-directory (default #f))
> +  (kcm-mach-service           krb5-configuration-kcm-mach-service
> +                                (default "org.h5l.kcm"))
> +  (kcm-socket                 krb5-configuration-kcm-socket
> +                                (default "/var/run/.heim_org.h5l.kcm-socket"))
> +  (kdc-default-options        krb5-configuration-kdc-default-options
> +                                (default #f))
> +  (kdc-timesync               krb5-configuration-kdc-timesync (default #t))
> +  (kdc-req-checksum-type      krb5-configuration-kdc-req-checksum-type (default #f))
> +  (noaddresses                krb5-configuration-noaddresses
> +                               (default #f))
> +  (permitted-enctypes         krb5-configuration-permitted-enctypes
> +                              (default #f))
> +  (plugin-base-dir            krb5-configuration-plugin-base-dir
> +                                (default #f))
> +  (preferred-preauth-types    krb5-configuration-preferred-preauth-types
> +                              (default #f))
> +  (proxiable                  krb5-configuration-proxiable (default #f))
> +  (rdns                       krb5-configuration-rdns (default #t))
> +  (realm-try-domains          krb5-configuration-realm-try-domains
> +                               (default #f))
> +  (renew-lifetime             krb5-configuration-renew-lifetime
> +                              (default #f))
> +  (safe-checksum-type         krb5-configuration-safe-checksum-type
> +                              (default #f))
> +  (ticket-lifetime            krb5-configuration-ticket-lifetime
> +                              (default #f))
> +  (udp-preference-limit       krb5-configuration-udp-preference-limit
> +                              (default #f))
> +  (verify-ap-req-nofail       krb5-configuration-verify-ap-req-nofail
> +                              (default #f))
> +
> +  ;;[realms]
> +  (realms                     krb5-configuration-realms)
> +
> +  ;;[domain_realm]
> +  (domain-realm-map           krb5-configuration-domain-realm-map (default '())))

Woow!  :-)  Please use full separate words; use question marks for
Boolean fields.

> +(define (krb5-etc-service config)
> +  (list `("krb5.conf" ,(krb5-configuration-file config))))
> +
> +
> +(define krb5-service-type
> +  (service-type (name 'krb5)
> +                (extensions
> +                 (list (service-extension etc-service-type
> +                                          krb5-etc-service)))))

So this service doesn’t do anything by itself.  Perhaps it should also
create a Shepherd service for the Kerberos daemon, or something like
that?

Thank you!

Ludo’.

  parent reply	other threads:[~2016-11-18 22:51 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-09 19:58 [PATCH] gnu: Add kerberos service John Darrington
2016-11-18 15:23 ` John Darrington
2016-11-18 22:51 ` Ludovic Courtès [this message]
2016-11-19  6:57   ` John Darrington
2016-11-21  8:59     ` Ludovic Courtès
2016-11-22 17:52       ` [PATCH] gnu: Add Kerberos client service John Darrington
2016-11-23 22:01         ` Ludovic Courtès
2016-11-29 18:39           ` John Darrington
2016-11-29 18:39             ` John Darrington
2016-11-30 13:09               ` Ludovic Courtès
2016-11-30 13:44                 ` John Darrington
2016-11-30 13:52                 ` Andy Wingo
2016-12-03 12:27                   ` John Darrington
2016-12-03 15:13                     ` 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=87a8cw5rmj.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=guix-devel@gnu.org \
    --cc=jmd@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).