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

[Bruno Victal <mirai@makinata.eu>]

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