diff mbox series

[bug#67175,7/9] services: jami: Use ‘least-authority-wrapper’.

Message ID d11e633a8b8d905e6a3c6c00599db03dec973cfb.1699970930.git.ludo@gnu.org
State New
Headers show
Series Removing 'make-forkexec-constructor/container' | expand

Commit Message

Ludovic Courtès Nov. 14, 2023, 2:09 p.m. UTC
* gnu/services/telephony.scm (jami-configuration->command-line-arguments)
[wrapper]: New procedure.
Use it.
(jami-shepherd-services): In ‘start’ method of ‘jami’ service, use
‘fork+exec-command’ instead of ‘make-forkexec-constructor/container’.
Remove use of (gnu build shepherd).

Change-Id: Ic71c0c88477d92bf137d9d0a5832bae8721cc210
---
 gnu/services/telephony.scm | 66 +++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 29 deletions(-)

Comments

Maxim Cournoyer Dec. 4, 2023, 1:38 a.m. UTC | #1
Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> * gnu/services/telephony.scm (jami-configuration->command-line-arguments)
> [wrapper]: New procedure.

nitpick: Should be <wrapper>, according to 'info (standards) Indicating
the Part Changed'

> Use it.
> (jami-shepherd-services): In ‘start’ method of ‘jami’ service, use
> ‘fork+exec-command’ instead of ‘make-forkexec-constructor/container’.
> Remove use of (gnu build shepherd).
>
> Change-Id: Ic71c0c88477d92bf137d9d0a5832bae8721cc210
> ---
>  gnu/services/telephony.scm | 66 +++++++++++++++++++++-----------------
>  1 file changed, 37 insertions(+), 29 deletions(-)
>
> diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
> index 832470527d..16d109b8b1 100644
> --- a/gnu/services/telephony.scm
> +++ b/gnu/services/telephony.scm
> @@ -261,9 +261,37 @@ (define %jami-accounts
>  (define (jami-configuration->command-line-arguments config)
>    "Derive the command line arguments to used to launch the Jami daemon from
>  CONFIG, a <jami-configuration> object."
> +  (define (wrapper libjami)
> +    (least-authority-wrapper
> +     ;; XXX: 'gexp-input' is needed as the outer layer so that
> +     ;; 'references-file' picks the right output of LIBJAMI.

It seems clearer to me to stick to the current #~(string-append
#$libjami:bin "/libexec/jamid") until file-append can handle non-default
outputs more elegantly (did we have a bug for that? -- I couldn't find
one).

The rest LGTM, if both jami system tests pass.
Ludovic Courtès Dec. 21, 2023, 10:16 p.m. UTC | #2
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> * gnu/services/telephony.scm (jami-configuration->command-line-arguments)
>> [wrapper]: New procedure.
>
> nitpick: Should be <wrapper>, according to 'info (standards) Indicating
> the Part Changed'

OK.

>> +  (define (wrapper libjami)
>> +    (least-authority-wrapper
>> +     ;; XXX: 'gexp-input' is needed as the outer layer so that
>> +     ;; 'references-file' picks the right output of LIBJAMI.
>
> It seems clearer to me to stick to the current #~(string-append
> #$libjami:bin "/libexec/jamid") until file-append can handle non-default
> outputs more elegantly (did we have a bug for that? -- I couldn't find
> one).

We cannot write #~(string-append …) here because
‘least-authority-wrapper’ expects a file-like object (because it passes
it to ‘references-file’).

> The rest LGTM, if both jami system tests pass.

Alright!

Ludo’.
Ludovic Courtès Dec. 21, 2023, 11:42 p.m. UTC | #3
Pushed as commit ca813173894360edef35a5d98878a3135e99e62a after
double-checking with:

  make check-system TESTS="jami jami-provisioning jami-provisioning-partial"

I inserted a new commit, 2cc881ac13522566a27d996afd1fb88df363f75e, to
increase timeouts in the tests: on my laptop the three tests would
occasionally fail when using the initial 20s timeouts (it’s not “really”
20s as the time measured in the host, not in the guest).

Thanks,
Ludo’.
diff mbox series

Patch

diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
index 832470527d..16d109b8b1 100644
--- a/gnu/services/telephony.scm
+++ b/gnu/services/telephony.scm
@@ -261,9 +261,37 @@  (define %jami-accounts
 (define (jami-configuration->command-line-arguments config)
   "Derive the command line arguments to used to launch the Jami daemon from
 CONFIG, a <jami-configuration> object."
+  (define (wrapper libjami)
+    (least-authority-wrapper
+     ;; XXX: 'gexp-input' is needed as the outer layer so that
+     ;; 'references-file' picks the right output of LIBJAMI.
+     (gexp-input (file-append (gexp-input libjami "bin") "/libexec/jamid")
+                 "bin")
+     #:mappings
+     (list (file-system-mapping
+            (source "/dev/log") ;for syslog
+            (target source))
+           (file-system-mapping
+            (source "/var/lib/jami")
+            (target source)
+            (writable? #t))
+           (file-system-mapping
+            (source "/var/run/jami")
+            (target source)
+            (writable? #t))
+           ;; Expose TLS certificates for GnuTLS.
+           (file-system-mapping
+            (source (file-append nss-certs "/etc/ssl/certs"))
+            (target "/etc/ssl/certs")))
+     #:preserved-environment-variables
+     '("DBUS_SESSION_BUS_ADDRESS" "SSL_CERT_DIR")
+     #:user "jami"
+     #:group "jami"
+     #:namespaces (fold delq %namespaces '(net user))))
+
   (match-record config <jami-configuration>
     (libjami dbus enable-logging? debug? auto-answer?)
-    `(,#~(string-append #$libjami:bin "/libexec/jamid")
+    `(,(wrapper libjami)
       "--persistent"                    ;stay alive after client quits
       ,@(if enable-logging?
             '()                         ;logs go to syslog by default
@@ -334,7 +362,6 @@  (define (jami-shepherd-services config)
       (with-imported-modules (source-module-closure
                               '((gnu build dbus-service)
                                 (gnu build jami-service)
-                                (gnu build shepherd)
                                 (gnu system file-systems)))
 
         (define list-accounts-action
@@ -562,7 +589,6 @@  (define (jami-shepherd-services config)
                           (srfi srfi-26)
                           (gnu build dbus-service)
                           (gnu build jami-service)
-                          (gnu build shepherd)
                           (gnu system file-systems)
                           ,@%default-modules))
                (start
@@ -608,32 +634,14 @@  (define (jami-shepherd-services config)
 
                     ;; Start the daemon.
                     (define daemon-pid
-                      ((make-forkexec-constructor/container
-                        (list #$@(jami-configuration->command-line-arguments
-                                  config))
-                        #:mappings
-                        (list (file-system-mapping
-                               (source "/dev/log") ;for syslog
-                               (target source))
-                              (file-system-mapping
-                               (source "/var/lib/jami")
-                               (target source)
-                               (writable? #t))
-                              (file-system-mapping
-                               (source "/var/run/jami")
-                               (target source)
-                               (writable? #t))
-                              ;; Expose TLS certificates for GnuTLS.
-                              (file-system-mapping
-                               (source #$(file-append nss-certs "/etc/ssl/certs"))
-                               (target "/etc/ssl/certs")))
-                        #:user "jami"
-                        #:group "jami"
-                        #:environment-variables
-                        (list (string-append "DBUS_SESSION_BUS_ADDRESS="
-                                             "unix:path=/var/run/jami/bus")
-                              ;; Expose TLS certificates for OpenSSL.
-                              "SSL_CERT_DIR=/etc/ssl/certs"))))
+                      (fork+exec-command
+                       (list #$@(jami-configuration->command-line-arguments
+                                 config))
+                       #:environment-variables
+                       (list (string-append "DBUS_SESSION_BUS_ADDRESS="
+                                            "unix:path=/var/run/jami/bus")
+                             ;; Expose TLS certificates for OpenSSL.
+                             "SSL_CERT_DIR=/etc/ssl/certs")))
 
                     (setenv "DBUS_SESSION_BUS_ADDRESS"
                             "unix:path=/var/run/jami/bus")