[bug#61740] [PATCH] services: Add rspamd-service-type.

[Bruno Victal]

Hi,

On 2023-02-23 20:16, Thomas Ieong wrote:
> * gnu/services/mail.scm (rspamd-service-type): New variable.
> * gnu/tests/mail.scm (%test-rspamd): New variable.
> * doc/guix.texi: Document it.
> ---
> 
> Hey Guix!
> 
> First time contributor here, this patch
> introduces some basic support for rspamd.
> 
> I do need guidance on some points.
> 
> How to handle the extra configs that a user can
> provide to rspamd?
> 
> On your average linux distro rspamd does expects
> you to not touch the rspamd.conf and instead put
> your changes in the /etc/rspamd/{local.d,override.d} directories
> (local is enough to redefine most settings, but if there are changes made via 
> the web ui, the web ui changes takes precedence, you need to use override.d 
> if you want to freeze a setting.)
> 
> For example to set the password of the web ui
> you're supposed to create /etc/rspamd/local.d/worker-controller.inc
> and then set password = "some_hash";
> 
> Then this will get merged with the config
> as something like:
> 
> worker {
>    type = "controller";
>    password = "some_hash";
> }
> 
> The point is we could ignore local.d/override.d
> and write these blocks directly to rspamd.conf.

For most services, the configuration is expected to be read-only (and generated 
& managed by guix)
though it is possible to have a mix of non guix-managed config files (but 
discouraged).

If you simply want to store the configuration in separate files, 
pulseaudio-service-type and mympd-service-type is an example that can do this.

> 
> Of course it needs some additionals configuration records for the workers and 
> the common options
> between them.
> 
> And finally for the test I do plan to add integration test with opensmtpd 
> when I get the time.
> 
> Are there examples of such integration test?

Specific examples no but gnu/tests/ contains many tests of varying complexity 
that could serve as inspiration.
See the NFS or web server tests.

> +
> +@deftp {Data Type} rspamd-configuration
> +Data type representing the configuration of @command{rspamd}.
> +
> +@table @asis
> +@item @code{package} (default: @code{rspamd})
> +The package that provides @command{rspamd}.
> +
> +@item @code{config-file} (default: @code{%default-rspamd-config-file})
> +File-like object of the configuration file to use. By default
> +all workers are enabled except fuzzy and they are binded
> +to their usual ports, e.g localhost:11334, localhost:11333 and so on.
> +
> +@item @code{user} (default: @code{"rspamd"})
> +The user to run rspamd as.
> +
> +@item @code{group} (default: @code{"rspamd"})
> +The user to run rspamd as.
> +
> +@item @code{pid-file} (default: @code{"/var/run/rspamd/rspamd.pid"})
> +Where to store the PID file.
> +
> +@item @code{debug?} (default: @code{#f})
> +Force debug output.
> +
> +@item @code{insecure?} (default: @code{#f})
> +Ignore running workers as privileged users (insecure).
> +
> +@item @code{skip-template?} (default: @code{#f})
> +Do not apply Jinja templates.
> +
> +@end table
> +@end deftp
> +

Was this manually typed? (It seems to be the case since it's missing the field 
type information)
You can generate the documentation automatically with 
configuration->documentation since you're using define-configuration.

> +;;;
> +;;; Rspamd.
> +;;;
> +
> +(define-maybe boolean)
> +
> +(define-configuration rspamd-configuration
> +  (package
> +    (file-like rspamd)
> +    "The package that provides rspamd."
> +    empty-serializer)
> +  (config-file
> +   (file-like %default-rspamd-config-file)
> +   "File-like object of the configuration file to use. By default
> +all workers are enabled except fuzzy and they are binded
> +to their usual ports, e.g localhost:11334, localhost:11333 and so on")
> +  (user
> +   (string "rspamd")
> +   "The user to run rspamd as."
> +   empty-serializer)
> +  (group
> +   (string "rspamd")
> +   "The group to run rspamd as."
> +   empty-serializer)
> +  (pid-file
> +   (string "/var/run/rspamd/rspamd.pid")
> +   "Where to store the PID file."
> +   empty-serializer)
> +  (debug?
> +   maybe-boolean
> +   "Force debug output."
> +   empty-serializer)
> +  (insecure?
> +   maybe-boolean
> +   "Ignore running workers as privileged users (insecure)."
> +   empty-serializer)
> +  (skip-template?
> +   maybe-boolean
> +   "Do not apply Jinja templates."
> +   empty-serializer))

If you're not going to use any serializer, you can use 
define-configuration/no-serialization instead.

> +
> +(define (rspamd-activation config)
> +  (match-record config <rspamd-configuration>
> +    (package config-file user)
> +    #~(begin
> +     (use-modules (guix build utils)
> +                  (ice-9 match))
> +     (let ((user (getpwnam #$user)))
> +       (mkdir-p/perms "/etc/rspamd" user #o755)
> +       (mkdir-p/perms "/etc/rspamd/local.d" user #o755)
> +       (mkdir-p/perms "/etc/rspamd/override.d" user #o755)
> +       (mkdir-p/perms "/var/run/rspamd" user #o755)
> +       (mkdir-p/perms "/var/log/rspamd" user #o755)
> +       (mkdir-p/perms "/var/lib/rspamd" user #o755))
> +     ;; Check configuration file syntax.
> +     (system* (string-append #$package "/bin/rspamadm")
> +              "configtest"
> +              "-c" #$config-file))))

This should be moved into the service constructor. See how mpd-service-type 
does this.

To expand a bit here, activation-service-type service-extensions are often 
abused for "pre-service launch tasks"
but this is incorrect usage (see #60657 which covers the pitfalls on doing so).

> +
> +(define rspamd-profile
> +  (compose list rspamd-configuration-package))

How about: 
(service-extension profile-service-type
                   (compose list rspamd-configuration-package))


> diff --git a/gnu/tests/mail.scm b/gnu/tests/mail.scm
> index f13751b72f..f532d30805 100644

Do not forget to register this file in gnu/local.mk.


Cheers,
Bruno