[bug#67555] [PATCH 2/2] services: kerberos/heimdal.scm: New file, add Heimdal Kerberos services.

[Bruno Victal]

Hi Felix,

On 2023-12-01 00:45, Felix Lechner wrote:
> +  (ports
> +   (list-of-strings '())
> +   "Ports to listen on.")

I'd prefer to use a list of exact-integers. (*)
Hint: you can use the procedures in (gnu services configuration)
to define this predicate with (list-of exact-integer?).

> +  (disable-des?
> +   (boolean #f)
> +   "Disable all DES encryption types."))

I'd avoid the double negative here, i.e. by naming this enable-des?.
Another note, how about defaulting to disabled DES support
to discourage its use?

> +     (start #~(make-forkexec-constructor
> +               (list #$(file-append heimdal "/libexec/kdc")
> +                     #$@(if (maybe-value-set? config-file)
> +                            `(,(string-append "--config-file=" (maybe-value 
> config-file)))
> +                            '())

Simply do:
`(,(string-append "--config-file=" config-file))

You don't need to use 'maybe-value' to extract the value if
you've already tested it with 'maybe-value-set?'.
> +               #:log-file "/var/log/kdc-shepherd"))

I'd make this configurable in <heimdal-kdc-configuration>.

> +  (ports
> +   (list-of-strings '())
> +   "Ports to listen on."))

See (*).

> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2017 Peter Mikkelsen <petermikkelsen10@gmail.com>
> +;;; Copyright © 2022 Bruno Victal <mirai@makinata.eu>

Copy-paste leftovers perhaps? 😅

> new file mode 100644
> index 0000000000..b6424ace9e
> --- /dev/null
> +++ b/gnu/tests/heimdal-kdc.scm

How about merging these tests under a single gnu/tests/krb-heimdal.scm
instead of splitting them as gnu/tests/heimdal-kadmind.scm and
gnu/tests/heimdal-kadmind.scm?

If you're up for it I'd love to see one more test (might
involve multiple VMs) that actually tests the kerberos integration.
(i.e. performs an actual kerberos test)
That way we could be at least sure that there's a working kerberos
setup that we can use as a reference point for documentation/cookbooks.

My 2¢!

-- 
Furthermore, I consider that nonfree software must be eradicated.

Cheers,
Bruno.

OpenPGP_signature.asc
Description: OpenPGP digital signature