[bug#61740] [PATCH v3] services: Add rspamd-service-type. (was [bug#61740] [PATCH] services: Add rspamd-service-type.)

[Saku Laesvuori via Guix-patches via <guix-patches@gnu.org>]

[-- Attachment #1.1: Type: text/plain, Size: 4867 bytes --]

On Wed, Dec 06, 2023 at 02:58:19PM +0000, Bruno Victal wrote:
> Hi Saku,
> 
> Some comments:
> 
> > +(define (directory-tree? xs)
> > +  (match xs
> > +    (((file-name file-like) ...)
> > +     (and (every string? file-name)
> > +          (every file-like? file-like)))
> > +    (_ #f)))
> 
> You can express this more compactly as:
> 
> --8<---------------cut here---------------start------------->8---
> (define directory-tree?
>   (match-lambda
>     ((((? string?) (? file-like?)) ...) #t)
>     (_ #f)))
> --8<---------------cut here---------------end--------------->8---

Done in v4.

> 
> > +  (user
> > +   (string "rspamd")
> > +   "The user to run rspamd as.")
> > +  (group
> > +   (string "rspamd")
> > +   "The group to run rspamd as.")
> 
> How about using user-account and user-group records instead? (see
> vnstat-service-type for an example)

Done in v4.

> 
> > +  (pid-file
> > +   (string "/var/run/rspamd/rspamd.pid")
> > +   "Where to store the PID file.")
> 
> Is it useful to expose this?

I don't know. It was there when I picked up this patch but I can't come
up with a case in which one would want to change it. Removed in v4.

> 
> 
> > +  (insecure?
> > +   (boolean #f)
> > +   "Ignore running workers as privileged users (insecure).")
> 
> To me it seems redundant to restate “(insecure)” in the description.

True. Removed in v4.

> 
> > +                     (make-forkexec-constructor
> > +                      (list #$rspamd "-c" #$config-file
> 
> I'd prefer the long-name --config over the shorter ones here.

Done in v4.

> > +                            "--var" (string-append "LOCAL_CONFDIR=" #$local-confdir)
> 
> Curiously I don't see this listed in the 'rspamd' manpage although
> it is on the 'rspamadm' one. Can you confirm whether this works
> and if so, report to upstream that their docs are missing this?

It does work; I've used it since before I submitted this patch. The
`--var` option is listed on `rspamd --help`. Unfortunately, Rspamd
tracks their issues on Github and I'd prefer not registering an account
there.

> > +     (service-extension profile-service-type
> > +                        (compose list rspamd-configuration-package))
> 
> What's the motivation for adding the rspamd package to the profile?

That was also there when I picked up this patch. I assume it is added to
the profile so that the `rspamadm` and `rspamc` programs are available
and compatible with the daemon. I don't have strong feelings about this
in either direction.

> > +(define %rspamd-os
> > +  (simple-operating-system
> > +   (service dhcp-client-service-type)
> > +   (service rspamd-service-type)))
> 
> Is 'dhcp-client-service-type' needed for this system test?
> I haven't tested it but it looks unnecessary to me.

It provides 'networking for the http test. Apparently the test wasn't
working yet anyway (I had no experience in Guix tests when I sent my
versions of the patch and just assumed that they were working in Thomas'
version). The tests are now fixed in v4.

> > +          ;; Check that we can access the web ui
> > +          (test-equal "http-get"
> > +            200
> > +            (begin
> > +              (let-values (((response text)
> > +                            (http-get "http://localhost:22668/"
> > +                                      #:decode-body? #t)))
> > +                (response-code response))))
> 
> IMO if you're only interested in the HTTP response code a http-head
> is the better option, unless the program handles those requests
> differently. Also, since 'text' isn't used you can simplify this to:
> 
> --8<---------------cut here---------------start------------->8---
> ;; Don't forget to remove the unused (srfi srfi-11) import.
> 
> (test-equal "Web UI is accessible"
>   200
>   (response-code (http-head "http://localhost:22668/")))
> --8<---------------cut here---------------end--------------->8---

Done in v4.

> > +          (test-assert "rspamd pid ready"
> > +            (marionette-eval
> > +             '(file-exists? "/var/run/rspamd/rspamd.pid")
> > +             marionette))
> 
> There's a procedure dedicated for this:
> 
> --8<---------------cut here---------------start------------->8---
> (test-assert "rspamd pid ready"
>   (wait-for-file #$(rspamd-configuration-pid-file (rspamd-configuration)) marionette)))
> --8<---------------cut here---------------end--------------->8---

Done in v4.

> > +(define %test-rspamd
> > +  (system-test
> > +   (name "rspamd")
> > +   (description "Send an email to a running rspamd server.")
> > +   (value (run-rspamd-test))))
> 
> I'd change the description to something like "Basic rspamd service test."
> as the current one is misleading.

Done in v4.

[-- Attachment #1.2: Type: text/plain, Size: 15667 bytes --]

From 1a2a4378304e77ee6ac4823734b916c8810b0834 Mon Sep 17 00:00:00 2001
Message-ID: <1a2a4378304e77ee6ac4823734b916c8810b0834.1702023246.git.saku@laesvuori.fi>
From: Thomas Ieong <th.ieong@free.fr>
Date: Thu, 23 Feb 2023 21:16:14 +0100
Subject: [PATCH v4] services: Add rspamd-service-type.

* gnu/services/mail.scm (rspamd-service-type): New variable.
* gnu/tests/mail.scm (%test-rspamd): New variable.
* doc/guix.texi: Document it.

Co-authored-by: Saku Laesvuori <saku@laesvuori.fi>
Change-Id: I7196643f087ffe9fc91aab231b69d5ed8dc9d198
---
 doc/guix.texi         |  62 +++++++++++++
 gnu/services/mail.scm | 206 +++++++++++++++++++++++++++++++++++++++++-
 gnu/tests/mail.scm    |  74 ++++++++++++++-
 3 files changed, 340 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f82bb99069..5875008ec3 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -119,6 +119,8 @@
 Copyright @copyright{} 2023 Zheng Junjie@*
 Copyright @copyright{} 2023 Brian Cully@*
 Copyright @copyright{} 2023 Felix Lechner@*
+Copyright @copyright{} 2023 Thomas Ieong@*
+Copyright @copyright{} 2023 Saku Laesvuori@*
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -27393,6 +27395,66 @@ Mail Services
 @end table
 @end deftp
 
+@subsubheading Rspamd Service
+@cindex email
+@cindex spam
+
+@defvar rspamd-service-type
+This is the type of the @uref{https://rspamd.com/, Rspamd} filtering
+system whose value should be a @code{rspamd-configuration}.
+@end defvar
+
+@c %start of fragment
+
+@deftp {Data Type} rspamd-configuration
+Available @code{rspamd-configuration} fields are:
+
+@table @asis
+@item @code{package} (default: @code{rspamd}) (type: file-like)
+The package that provides rspamd.
+
+@item @code{config-file} (default: @code{%default-rspamd-config-file}) (type: file-like)
+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{local.d-files} (default: @code{()}) (type: directory-tree)
+Configuration files in local.d, provided as a list of two element lists
+where the first element is the filename and the second one is a
+file-like object.  Settings in these files will be merged with the
+defaults.
+
+@item @code{override.d-files} (default: @code{()}) (type: directory-tree)
+Configuration files in override.d, provided as a list of two element
+lists where the first element is the filename and the second one is a
+file-like object.  Settings in these files will override the defaults.
+
+@item @code{user} (default: @code{%default-rspamd-account}) (type: user-account)
+The user to run rspamd as.
+
+@item @code{group} (default: @code{%default-rspamd-group}) (type: user-group)
+The group to run rspamd as.
+
+@item @code{debug?} (default: @code{#f}) (type: boolean)
+Force debug output.
+
+@item @code{insecure?} (default: @code{#f}) (type: boolean)
+Ignore running workers as privileged users.
+
+@item @code{skip-template?} (default: @code{#f}) (type: boolean)
+Do not apply Jinja templates.
+
+@item @code{shepherd-requirements} (default: @code{(loopback)}) (type: list-of-symbols)
+This is a list of symbols naming Shepherd services that this service
+will depend on.
+
+@end table
+
+@end deftp
+
+
+@c %end of fragment
+
 @node Messaging Services
 @subsection Messaging Services
 
diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm
index 12dcc8e71d..0ec0c43a4d 100644
--- a/gnu/services/mail.scm
+++ b/gnu/services/mail.scm
@@ -5,6 +5,8 @@
 ;;; Copyright © 2017, 2020 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2019 Kristofer Buffington <kristoferbuffington@gmail.com>
 ;;; Copyright © 2020 Jonathan Brielmaier <jonathan.brielmaier@web.de>
+;;; Copyright © 2023 Thomas Ieong <th.ieong@free.fr>
+;;; Copyright © 2023 Saku Laesvuori <saku@laesvuori.fi>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -80,7 +82,13 @@ (define-module (gnu services mail)
             radicale-configuration
             radicale-configuration?
             radicale-service-type
-            %default-radicale-config-file))
+            %default-radicale-config-file
+
+            rspamd-configuration
+            rspamd-service-type
+            %default-rspamd-account
+            %default-rspamd-config-file
+            %default-rspamd-group))
 
 ;;; Commentary:
 ;;;
@@ -1987,3 +1995,199 @@ (define radicale-service-type
           (service-extension account-service-type (const %radicale-accounts))
           (service-extension activation-service-type radicale-activation)))
    (default-value (radicale-configuration))))
+
+;;;
+;;; Rspamd.
+;;;
+
+(define (directory-tree? xs)
+  (match xs
+    ((((? string?) (? file-like?)) ...) #t)
+    (_ #f)))
+
+(define (list-of-symbols? x)
+  (and (list? x)
+       (every symbol? x)))
+
+(define-configuration/no-serialization rspamd-configuration
+  (package
+   (file-like rspamd)
+   "The package that provides rspamd.")
+  (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")
+  (local.d-files
+   (directory-tree '())
+   "Configuration files in local.d, provided as a list of two element lists where
+the first element is the filename and the second one is a file-like object.  Settings
+in these files will be merged with the defaults.")
+  (override.d-files
+   (directory-tree '())
+   "Configuration files in override.d, provided as a list of two element lists where
+the first element is the filename and the second one is a file-like object.  Settings
+in these files will override the defaults.")
+  (user
+   (user-account %default-rspamd-account)
+   "The user to run rspamd as.")
+  (group
+   (user-group %default-rspamd-group)
+   "The group to run rspamd as.")
+  (debug?
+   (boolean #f)
+   "Force debug output.")
+  (insecure?
+   (boolean #f)
+   "Ignore running workers as privileged users.")
+  (skip-template?
+   (boolean #f)
+   "Do not apply Jinja templates.")
+  (shepherd-requirements
+   (list-of-symbols '(loopback))
+   "This is a list of symbols naming Shepherd services that this service
+will depend on."))
+
+(define %default-rspamd-account
+  (user-account
+      (name "rspamd")
+      (group "rspamd")
+      (system? #t)
+      (comment "Rspamd daemon")
+      (home-directory "/var/empty")
+      (shell (file-append shadow "/sbin/nologin"))))
+
+(define %default-rspamd-group
+  (user-group
+    (name "rspamd")
+    (system? #t)))
+
+(define %default-rspamd-config-file
+  (plain-file "rspamd.conf" "
+.include \"$CONFDIR/common.conf\"
+
+options {
+    pidfile = \"$RUNDIR/rspamd.pid\";
+    .include \"$CONFDIR/options.inc\"
+    .include(try=true; priority=1,duplicate=merge) \"$LOCAL_CONFDIR/local.d/options.inc\"
+    .include(try=true; priority=10) \"$LOCAL_CONFDIR/override.d/options.inc\"
+}
+
+logging {
+    type = \"file\";
+    filename = \"$LOGDIR/rspamd.log\";
+    .include \"$CONFDIR/logging.inc\"
+    .include(try=true; priority=1,duplicate=merge) \"$LOCAL_CONFDIR/local.d/logging.inc\"
+    .include(try=true; priority=10) \"$LOCAL_CONFDIR/override.d/logging.inc\"
+}
+
+worker \"normal\" {
+    bind_socket = \"localhost:11333\";
+    .include \"$CONFDIR/worker-normal.inc\"
+    .include(try=true; priority=1,duplicate=merge) \"$LOCAL_CONFDIR/local.d/worker-normal.inc\"
+    .include(try=true; priority=10) \"$LOCAL_CONFDIR/override.d/worker-normal.inc\"
+}
+
+worker \"controller\" {
+    bind_socket = \"localhost:11334\";
+    .include \"$CONFDIR/worker-controller.inc\"
+    .include(try=true; priority=1,duplicate=merge) \"$LOCAL_CONFDIR/local.d/worker-controller.inc\"
+    .include(try=true; priority=10) \"$LOCAL_CONFDIR/override.d/worker-controller.inc\"
+}
+
+worker \"rspamd_proxy\" {
+    bind_socket = \"localhost:11332\";
+    .include \"$CONFDIR/worker-proxy.inc\"
+    .include(try=true; priority=1,duplicate=merge) \"$LOCAL_CONFDIR/local.d/worker-proxy.inc\"
+    .include(try=true; priority=10) \"$LOCAL_CONFDIR/override.d/worker-proxy.inc\"
+}
+
+# Local fuzzy storage is disabled by default
+
+worker \"fuzzy\" {
+    bind_socket = \"localhost:11335\";
+    count = -1; # Disable by default
+    .include \"$CONFDIR/worker-fuzzy.inc\"
+    .include(try=true; priority=1,duplicate=merge) \"$LOCAL_CONFDIR/local.d/worker-fuzzy.inc\"
+    .include(try=true; priority=10) \"$LOCAL_CONFDIR/override.d/worker-fuzzy.inc\"
+}
+"))
+
+(define (rspamd-accounts config)
+  (match-record config <rspamd-configuration>
+    (user group)
+    (list group user)))
+
+(define (rspamd-shepherd-service config)
+  (match-record config <rspamd-configuration>
+    (package config-file user group debug? insecure? skip-template?
+     local.d-files override.d-files shepherd-requirements)
+    (list
+     (shepherd-service
+      (provision '(rspamd))
+      (documentation "Run the rspamd daemon.")
+      (requirement shepherd-requirements)
+      (start (let ((rspamd (file-append package "/bin/rspamd"))
+                   (local-confdir
+                     (file-union
+                      "rspamd-local-confdir"
+                      `(("local.d" ,(file-union "local.d" local.d-files))
+                        ("override.d" ,(file-union "override.d" override.d-files))))))
+               (with-imported-modules (source-module-closure '((gnu build activation)))
+                 #~(begin
+                     (use-modules (gnu build activation)) ; for mkdir-p/perms
+                     (let ((user (getpwnam #$(user-account-name user))))
+                       (mkdir-p/perms "/var/run/rspamd" user #o755)
+                       (mkdir-p/perms "/var/log/rspamd" user #o755)
+                       (mkdir-p/perms "/var/lib/rspamd" user #o755))
+                     (make-forkexec-constructor
+                      (list #$rspamd "--config" #$config-file
+                            "--var" (string-append "LOCAL_CONFDIR=" #$local-confdir)
+                            "--no-fork"
+                            #$@(if debug?
+                                 '("--debug")
+                                 '())
+                            #$@(if insecure?
+                                 '("--insecure")
+                                 '())
+                            #$@(if skip-template?
+                                 '("--skip-template")
+                                 '()))
+                      #:user #$(user-account-name user)
+                      #:group #$(user-group-name group))))))
+      (stop #~(make-kill-destructor))
+      (actions
+       (list
+        (shepherd-configuration-action config-file)
+        (shepherd-action
+         (name 'reload)
+         (documentation "Reload rspamd.")
+         (procedure
+          #~(lambda (pid)
+              (if pid
+                (begin
+                  (kill pid SIGHUP)
+                  (display "Service rspamd has been reloaded"))
+                (format #t "Service rspamd is not running.")))))
+        (shepherd-action
+         (name 'reopenlog)
+         (documentation "Reopen log files.")
+         (procedure
+          #~(lambda (pid)
+              (if pid
+                (begin
+                  (kill pid SIGUSR1)
+                  (display "Reopening the logs for rspamd"))
+                (format #t "Service rspamd is not running.")))))))))))
+
+(define rspamd-service-type
+  (service-type
+   (name 'rspamd)
+   (description "Run the rapid spam filtering system.")
+   (extensions
+    (list
+     (service-extension shepherd-root-service-type rspamd-shepherd-service)
+     (service-extension account-service-type rspamd-accounts)
+     (service-extension profile-service-type
+                        (compose list rspamd-configuration-package))))
+   (default-value (rspamd-configuration))))
diff --git a/gnu/tests/mail.scm b/gnu/tests/mail.scm
index dcb8f08ea8..fc1c69047b 100644
--- a/gnu/tests/mail.scm
+++ b/gnu/tests/mail.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
 ;;; Copyright © 2019 Christopher Baines <mail@cbaines.net>
 ;;; Copyright © 2019, 2020 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2023 Thomas Ieong <th.ieong@free.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -40,7 +41,8 @@ (define-module (gnu tests mail)
   #:export (%test-opensmtpd
             %test-exim
             %test-dovecot
-            %test-getmail))
+            %test-getmail
+            %test-rspamd))
 
 (define %opensmtpd-os
   (simple-operating-system
@@ -579,3 +581,73 @@ (define %test-getmail
    (name "getmail")
    (description "Connect to a running Getmail server.")
    (value (run-getmail-test))))
+
+(define %rspamd-os
+  (simple-operating-system
+   (service dhcp-client-service-type)
+   (service rspamd-service-type
+            (rspamd-configuration
+              (shepherd-requirements '(networking))
+              (local.d-files `(("worker-controller.inc"
+                                ,(plain-file
+                                   "rspamd-public-web-controller.conf"
+                                   "bind_socket = \"0.0.0.0:11334\";"))))))))
+
+(define (run-rspamd-test)
+  "Return a test of an OS running Rspamd service."
+
+  (define rspamd-ports
+    '((22668 . 11334)))    ;; web controller
+
+  (define vm
+    (virtual-machine
+     (operating-system (marionette-operating-system
+                        %rspamd-os
+                        #:imported-modules '((gnu services herd))))
+     (port-forwardings rspamd-ports)))
+
+  (define test
+    (with-imported-modules '((gnu build marionette))
+      #~(begin
+          (use-modules (srfi srfi-64)
+                       (gnu build marionette)
+                       (web uri)
+                       (web client)
+                       (web response))
+
+          (define marionette
+            (make-marionette '(#$vm)))
+
+          (test-runner-current (system-test-runner #$output))
+          (test-begin "rspamd")
+
+          (test-assert "service is running"
+            (marionette-eval
+             '(begin
+                (use-modules (gnu services herd))
+                (start-service 'rspamd))
+             marionette))
+
+          (test-assert "rspamd socket ready"
+            (wait-for-unix-socket
+             "/var/lib/rspamd/rspamd.sock"
+             marionette))
+
+          (test-assert "rspamd log file"
+            (wait-for-file "/var/log/rspamd/rspamd.log" marionette))
+
+          ;; Check that we can access the web ui
+
+          (test-equal "http-get"
+            200
+            (response-code (http-get "http://localhost:22668/"))) ; HEAD is unsupported
+
+          (test-end))))
+
+  (gexp->derivation "rspamd-test" test))
+
+(define %test-rspamd
+  (system-test
+   (name "rspamd")
+   (description "Basic rspamd service test.")
+   (value (run-rspamd-test))))

base-commit: ea88bef3e0579264b20fa8edbf059c02d9cbe104
prerequisite-patch-id: 6b143a0f0a9c696e5214b42bb7928cf2abd7fc52
-- 
2.41.0


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