[bug#61744] [PATCH v2 2/2] services: pam-limits-service-type: Deprecate file-like object support in favour for lists as service value.

[Bruno Victal]

* doc/guix.texi (Base Services): Document it.
* gnu/local.mk: Register test.
* gnu/services/base.scm (pam-limits-service-type): Accept both lists and
file-like objects. Deprecate file-like object support.
* gnu/tests/pam.scm: New file.
---
 doc/guix.texi         | 27 +++++-------
 gnu/local.mk          |  2 +
 gnu/services/base.scm | 36 +++++++++++-----
 gnu/tests/pam.scm     | 97 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 135 insertions(+), 27 deletions(-)
 create mode 100644 gnu/tests/pam.scm

diff --git a/doc/guix.texi b/doc/guix.texi
index 3aa9c0cdf4..5c9a9333b9 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -18950,23 +18950,18 @@ Base Services
 Type of the service that installs a configuration file for the
 @uref{http://linux-pam.org/Linux-PAM-html/sag-pam_limits.html,
 @code{pam_limits} module}.  The value for this service type is
-a file-like object containing a list of @code{pam-limits-entry} values
-which can be used to specify @code{ulimit} limits and @code{nice}
-priority limits to user sessions.
+a list of @code{pam-limits-entry} values, which can be used to specify
+@code{ulimit} limits and @code{nice} priority limits to user sessions.
+By default, the value is the empty list.
 
 The following limits definition sets two hard and soft limits for all
 login sessions of users in the @code{realtime} group:
 
 @lisp
-(service
-  pam-limits-service-type
-  (plain-file
-    "limits.conf"
-    (string-join
-      (map pam-limits-entry->string
-        (list (pam-limits-entry "@@realtime" 'both 'rtprio 99)
-              (pam-limits-entry "@@realtime" 'both 'memlock 'unlimited)))
-      "\n")))
+(service pam-limits-service-type
+         (list
+          (pam-limits-entry "@@realtime" 'both 'rtprio 99)
+          (pam-limits-entry "@@realtime" 'both 'memlock 'unlimited)))
 @end lisp
 
 The first entry increases the maximum realtime priority for
@@ -18978,11 +18973,9 @@ Base Services
 descriptors that can be used:
 
 @lisp
-(service
-  pam-limits-service-type
-  (plain-file
-    "limits.conf"
-    (pam-limits-entry->string (pam-limits-entry "*" 'both 'nofile 100000))))
+(service pam-limits-service-type
+         (list
+          (pam-limits-entry "*" 'both 'nofile 100000)))
 @end lisp
 
 In the above example, the asterisk means the limit should apply to any
diff --git a/gnu/local.mk b/gnu/local.mk
index 415955bd3f..6291d8a558 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -56,6 +56,7 @@
 # Copyright © 2022 Alex Griffin <a@ajgrf.com>
 # Copyright © 2022 ( <paren@disroot.org>
 # Copyright © 2022 jgart <jgart@dismail.de>
+# Copyright © 2023 Bruno Victal <mirai@makinata.eu>
 #
 # This file is part of GNU Guix.
 #
@@ -778,6 +779,7 @@ GNU_SYSTEM_MODULES =                                \
   %D%/tests/messaging.scm                      \
   %D%/tests/networking.scm                     \
   %D%/tests/package-management.scm             \
+  %D%/tests/pam.scm                            \
   %D%/tests/reconfigure.scm                    \
   %D%/tests/rsync.scm                          \
   %D%/tests/samba.scm                          \
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index e5023b8175..80f9607d44 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -40,7 +40,7 @@
 (define-module (gnu services base)
   #:use-module (guix store)
   #:use-module (guix deprecation)
-  #:autoload   (guix diagnostics) (warning &fix-hint)
+  #:autoload   (guix diagnostics) (warning formatted-message &fix-hint)
   #:autoload   (guix i18n) (G_)
   #:use-module (guix combinators)
   #:use-module (gnu services)
@@ -1584,17 +1584,13 @@ (define-deprecated (syslog-service #:optional (config 
(syslog-configuration)))
 
 
 (define pam-limits-service-type
-  (let ((security-limits
-         ;; Create /etc/security containing the provided "limits.conf" file.
-         (lambda (limits-file)
-           `(("security/limits.conf"
-              ,limits-file))))
-        (pam-extension
+  (let ((pam-extension
          (lambda (pam)
            (let ((pam-limits (pam-entry
                               (control "required")
                               (module "pam_limits.so")
-                              (arguments 
'("conf=/etc/security/limits.conf")))))
+                              (arguments
+                               '("conf=/etc/security/limits.conf")))))
              (if (member (pam-service-name pam)
                          '("login" "greetd" "su" "slim" "gdm-password" "sddm"
                            "sudo" "sshd"))
@@ -1602,7 +1598,27 @@ (define pam-limits-service-type
                   (inherit pam)
                   (session (cons pam-limits
                                  (pam-service-session pam))))
-                 pam)))))
+                 pam))))
+
+        ;; XXX: Using file-like objects is deprecated, use lists instead.
+        ;;      This is to be reduced into the list? case when the deprecated
+        ;;      code gets removed.
+        ;; Create /etc/security containing the provided "limits.conf" file.
+        (security-limits
+         (match-lambda
+           ((? file-like? obj)
+            (warning (G_ "Using file-like value for \
+'pam-limits-service-type' is deprecated~%"))
+            `(("security/limits.conf" ,obj)))
+           ((? list? lst)
+            `(("security/limits.conf"
+               ,(plain-file "limits.conf"
+                            (string-join (map pam-limits-entry->string lst)
+                                         "\n" 'suffix)))))
+           (_ (raise
+               (formatted-message
+                (G_ "invalid input for 'pam-limits-service-type'~%")))))))
+
     (service-type
      (name 'limits)
      (extensions
@@ -1613,7 +1629,7 @@ (define pam-limits-service-type
       "Install the specified resource usage limits by populating
 @file{/etc/security/limits.conf} and using the @code{pam_limits}
 authentication module.")
-     (default-value (plain-file "limits.conf" "")))))
+     (default-value '()))))
 
 (define-deprecated (pam-limits-service #:optional (limits '()))
   pam-limits-service-type
diff --git a/gnu/tests/pam.scm b/gnu/tests/pam.scm
new file mode 100644
index 0000000000..5cf13d97d7
--- /dev/null
+++ b/gnu/tests/pam.scm
@@ -0,0 +1,97 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu tests pam)
+  #:use-module (gnu tests)
+  #:use-module (gnu services)
+  #:use-module (gnu services base)
+  #:use-module (gnu system)
+  #:use-module (gnu system pam)
+  #:use-module (gnu system vm)
+  #:use-module (guix gexp)
+  #:use-module (ice-9 format)
+  #:export (%test-pam-limits
+            %test-pam-limits-deprecated))
+
+
+;;;
+;;; pam-limits-service-type
+;;;
+
+(define pam-limit-entries
+  (list
+   (pam-limits-entry "@realtime" 'both 'rtprio 99)
+   (pam-limits-entry "@realtime" 'both 'memlock 'unlimited)))
+
+(define (run-test-pam-limits config)
+  "Run tests in a os with pam-limits-service-type configured."
+  (define os
+    (marionette-operating-system
+     (simple-operating-system
+      (service pam-limits-service-type config))))
+
+  (define vm
+    (virtual-machine os))
+
+  (define name (format #f "pam-limit-service~:[~;-deprecated~]"
+                       (file-like? config)))
+
+  (define test
+    (with-imported-modules '((gnu build marionette))
+      #~(begin
+          (use-modules (gnu build marionette)
+                       (srfi srfi-64))
+
+          (let ((marionette (make-marionette (list #$vm))))
+
+            (test-runner-current (system-test-runner #$output))
+
+            (test-begin #$name)
+
+            (test-assert "/etc/security/limits.conf ready"
+              (wait-for-file "/etc/security/limits.conf" marionette))
+
+            (test-equal "/etc/security/limits.conf content matches"
+              #$(string-join (map pam-limits-entry->string pam-limit-entries)
+                             "\n" 'suffix)
+              (marionette-eval
+               '(call-with-input-file "/etc/security/limits.conf"
+                  get-string-all)
+               marionette))
+
+            (test-end)))))
+
+  (gexp->derivation (string-append name "-test") test))
+
+(define %test-pam-limits
+  (system-test
+   (name "pam-limits-service")
+   (description "Test that pam-limits-service can serialize its config
+(as a list) to @file{limits.conf}.")
+   (value (run-test-pam-limits pam-limit-entries))))
+
+(define %test-pam-limits-deprecated
+  (system-test
+   (name "pam-limits-service-deprecated")
+   (description "Test that pam-limits-service can serialize its config
+(as a file-like object) to @file{limits.conf}.")
+   (value (run-test-pam-limits
+           (plain-file "limits.conf"
+                       (string-join (map pam-limits-entry->string
+                                         pam-limit-entries)
+                                    "\n" 'suffix))))))
-- 
2.39.1