all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / Atom feed
* [bug#45860] Improve PostgreSQL service.
@ 2021-01-14 13:36 Mathieu Othacehe
  2021-01-14 21:56 ` Christopher Baines
  2021-01-18 10:16 ` [bug#45860] [PATCH v2 0/5] services: postgresql: Improve service Mathieu Othacehe
  0 siblings, 2 replies; 10+ messages in thread
From: Mathieu Othacehe @ 2021-01-14 13:36 UTC (permalink / raw)
  To: 45860

[-- Attachment #1: Type: text/plain, Size: 201 bytes --]


Hello,

Here's a patch to improve PostgreSQL service. It merges
<postgresql-configuration> and <postgresql-config-file> records. It also
sanitises parameters conversion and logging.

Thanks,

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Improve-PostgreSQL-service.patch --]
[-- Type: text/x-diff, Size: 25911 bytes --]

From 87703b749631acd8ddc2b9eeb36a5be7189a019b Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Thu, 14 Jan 2021 14:13:30 +0100
Subject: [PATCH] Improve PostgreSQL service.

Merge <postgresql-configuration> and <postgresql-config-file> records,
sanitize parameters convertion and logging.

* gnu/services/databases.scm (postgresql-config-file,
postgresql-config-file?, postgresql-config-file-log-destination,
postgresql-config-file-hba-file, postgresql-config-file-ident-file,
postgresql-config-file-extra-config, postgresql-configuration): Remove them.
(postgresql-configuration-log-destination,
postgresql-configuration-hba-file,
postgresql-configuration-ident-file,
postgresql-configuration-socket-directory,
postgresql-configuration-extra-config,
postgresql-configuration-extension-packages): New exported procedures.
(<postgresql-config-file>): Merge it with ...
(<postgresql-configuration>): ... this record, and add a "socket-directory"
field.
(postgresql-config-file-compiler): Replace it with ...
(postgresql-config-file): ... this procedure.
(postgresql-activation): Use "match-record" instead of "match". Create the
"socket-directory" if needed.
(postgresql-shepherd-service): Use "match-record" intead of "match". Pass the
"log-destination" argument to "pg_ctl" if needed.
(postgresql-service): Remove it.
* gnu/tests/databases.scm (%postgresql-log-directory): New variable.
(%postgresql-os): Pass "log-destination" and "extra-config" fields.
(log-file): New test case.
* gnu/tests/guix.scm (%guix-data-service-os): Adapt accordingly.
* doc/guix.texi (Database Services): Ditto.
---
 doc/guix.texi              |  89 +++++-----
 gnu/services/databases.scm | 332 +++++++++++++++++++------------------
 gnu/tests/databases.scm    |  30 +++-
 gnu/tests/guix.scm         |  10 +-
 4 files changed, 245 insertions(+), 216 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f38e018dff..7fb7652166 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19302,14 +19302,41 @@ Port on which PostgreSQL should listen.
 @item @code{locale} (default: @code{"en_US.utf8"})
 Locale to use as the default when creating the database cluster.
 
-@item @code{config-file} (default: @code{(postgresql-config-file)})
-The configuration file to use when running PostgreSQL.  The default
-behaviour uses the postgresql-config-file record with the default values
-for the fields.
-
 @item @code{data-directory} (default: @code{"/var/lib/postgresql/data"})
 Directory in which to store the data.
 
+@item @code{log-destination} (default: @code{'syslog})
+The logging method to use for PostgreSQL.  It can be set to a directory,
+such as @code{"/var/log/postgresql"}.  In that case, PostgreSQL will
+write log files to that directory.  The @command{pg_ctl} output will
+also be written to a file named @code{"pg_ctl.log"} in that very
+directory.  This file can be useful to debug PostgreSQL configuration
+errors for instance.
+
+@item @code{hba-file} (default: @code{%default-postgres-hba})
+Filename or G-expression for the host-based authentication
+configuration.
+
+@item @code{ident-file} (default: @code{%default-postgres-ident})
+Filename or G-expression for the user name mapping configuration.
+
+@item @code{socket-directory} (default: @code{"/var/lib/postgresql"})
+Specifies the directory of the Unix-domain socket(s) on which PostgreSQL
+is to listen for connections from client applications.  If set to
+@code{#false} PostgreSQL does not listen on any Unix-domain sockets, in
+which case only TCP/IP sockets can be used to connect to the server.
+
+@item @code{extra-config} (default: @code{'()})
+List of additional keys and values to include in the PostgreSQL config
+file.  Each entry in the list should be a list where the first element
+is the key, and the remaining elements are the values.
+
+The values can be numbers, booleans or strings and will be mapped to
+PostgreSQL parameters types @code{Boolean}, @code{String},
+@code{Numeric}, @code{Numeric with Unit} and @code{Enumerated} described
+@uref{https://www.postgresql.org/docs/current/config-setting.html,
+here}.
+
 @item @code{extension-packages} (default: @code{'()})
 @cindex postgresql extension-packages
 Additional extensions are loaded from packages listed in
@@ -19351,54 +19378,28 @@ dblink as they are already loadable by postgresql.  This field is only
 required to add extensions provided by other packages.
 
 @end table
-@end deftp
 
-@deftp {Data Type} postgresql-config-file
-Data type representing the PostgreSQL configuration file.  As shown in
-the following example, this can be used to customize the configuration
-of PostgreSQL.  Note that you can use any G-expression or filename in
-place of this record, if you already have a configuration file you'd
-like to use for example.
+Here is an example of PostgreSQL configuration, with the log destination
+set to @code{"/var/log/postgresql"} directory.  A few random extra
+config parameters types are passed.
 
 @lisp
 (service postgresql-service-type
          (postgresql-configuration
-          (config-file
-           (postgresql-config-file
-            (log-destination "stderr")
-            (hba-file
-             (plain-file "pg_hba.conf"
-                         "
+          (log-destination "/var/log/postgresql")
+          (hba-file
+           (plain-file "pg_hba.conf"
+                       "
 local	all	all			trust
 host	all	all	127.0.0.1/32 	md5
 host	all	all	::1/128 	md5"))
-            (extra-config
-             '(("session_preload_libraries"     "'auto_explain'")
-               ("random_page_cost"              "2")
-               ("auto_explain.log_min_duration" "'100ms'")
-               ("work_mem"                      "'500MB'")
-               ("logging_collector"             "on")
-               ("log_directory"                 "'/var/log/postgresql'")))))))
+          (extra-config
+           '(("session_preload_libraries"     "auto_explain")
+             ("random_page_cost"              2)
+             ("auto_explain.log_min_duration" "100 ms")
+             ("work_mem"                      "500 MB")
+             ("debug_print_plan"              #t)))))
 @end lisp
-
-@table @asis
-@item @code{log-destination} (default: @code{"syslog"})
-The logging method to use for PostgreSQL.  Multiple values are accepted,
-separated by commas.
-
-@item @code{hba-file} (default: @code{%default-postgres-hba})
-Filename or G-expression for the host-based authentication
-configuration.
-
-@item @code{ident-file} (default: @code{%default-postgres-ident})
-Filename or G-expression for the user name mapping configuration.
-
-@item @code{extra-config} (default: @code{'()})
-List of additional keys and values to include in the PostgreSQL config
-file.  Each entry in the list should be a list where the first element
-is the key, and the remaining elements are the values.
-
-@end table
 @end deftp
 
 @subsubheading MariaDB/MySQL
diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm
index d2dc5f0da8..013ca97227 100644
--- a/gnu/services/databases.scm
+++ b/gnu/services/databases.scm
@@ -38,22 +38,19 @@
   #:use-module (guix gexp)
   #:use-module (srfi srfi-1)
   #:use-module (ice-9 match)
-  #:export (postgresql-config-file
-            postgresql-config-file?
-            postgresql-config-file-log-destination
-            postgresql-config-file-hba-file
-            postgresql-config-file-ident-file
-            postgresql-config-file-extra-config
-
-            postgresql-configuration
+  #:export (postgresql-configuration
             postgresql-configuration?
             postgresql-configuration-postgresql
             postgresql-configuration-port
             postgresql-configuration-locale
-            postgresql-configuration-file
             postgresql-configuration-data-directory
+            postgresql-configuration-log-destination
+            postgresql-configuration-hba-file
+            postgresql-configuration-ident-file
+            postgresql-configuration-socket-directory
+            postgresql-configuration-extra-config
+            postgresql-configuration-extension-packages
 
-            postgresql-service
             postgresql-service-type
 
             memcached-service-type
@@ -98,49 +95,6 @@ host	all	all	::1/128 	md5"))
   (plain-file "pg_ident.conf"
               "# MAPNAME       SYSTEM-USERNAME         PG-USERNAME"))
 
-(define-record-type* <postgresql-config-file>
-  postgresql-config-file make-postgresql-config-file
-  postgresql-config-file?
-  (log-destination postgresql-config-file-log-destination
-                   (default "syslog"))
-  (hba-file        postgresql-config-file-hba-file
-                   (default %default-postgres-hba))
-  (ident-file      postgresql-config-file-ident-file
-                   (default %default-postgres-ident))
-  (extra-config    postgresql-config-file-extra-config
-                   (default '())))
-
-(define-gexp-compiler (postgresql-config-file-compiler
-                       (file <postgresql-config-file>) system target)
-  (match file
-    (($ <postgresql-config-file> log-destination hba-file
-                                 ident-file extra-config)
-     (define (single-quote string)
-       (if string
-           (list "'" string "'")
-           '()))
-
-     (define contents
-       (append-map
-        (match-lambda
-          ((key) '())
-          ((key . #f) '())
-          ((key values ...) `(,key " = " ,@values "\n")))
-
-        `(("log_destination" ,@(single-quote log-destination))
-          ("hba_file" ,@(single-quote hba-file))
-          ("ident_file" ,@(single-quote ident-file))
-          ,@extra-config)))
-
-     (gexp->derivation
-      "postgresql.conf"
-      #~(call-with-output-file (ungexp output "out")
-          (lambda (port)
-            (display
-             (string-append #$@contents)
-             port)))
-      #:local-build? #t))))
-
 (define-record-type* <postgresql-configuration>
   postgresql-configuration make-postgresql-configuration
   postgresql-configuration?
@@ -149,13 +103,59 @@ host	all	all	::1/128 	md5"))
                       (default 5432))
   (locale             postgresql-configuration-locale
                       (default "en_US.utf8"))
-  (config-file        postgresql-configuration-file
-                      (default (postgresql-config-file)))
   (data-directory     postgresql-configuration-data-directory
                       (default "/var/lib/postgresql/data"))
+  (log-destination    postgresql-configuration-log-destination
+                      (default 'syslog))
+  (hba-file           postgresql-configuration-hba-file
+                      (default %default-postgres-hba))
+  (ident-file         postgresql-configuration-ident-file
+                      (default %default-postgres-ident))
+  (socket-directory   postgresql-configuration-socket-directory
+                      (default "/var/run/postgresql"))
+  (extra-config       postgresql-configuration-extra-config
+                      (default '()))
   (extension-packages postgresql-configuration-extension-packages
                       (default '())))
 
+(define (postgresql-config-file config)
+  (match-record config <postgresql-configuration>
+    (log-destination hba-file ident-file socket-directory extra-config)
+    ;; See: https://www.postgresql.org/docs/current/config-setting.html.
+    (define (format-value value)
+      (cond
+       ((boolean? value)
+        (list (if value "on" "off")))
+       ((number? value)
+        (list (number->string value)))
+       (else
+        (list "'" value "'"))))
+
+    (define contents
+      (append-map
+       (match-lambda
+         ((key) '())
+         ((key . #f) '())
+         ((key values ...)
+          `(,key " = " ,@(append-map format-value values) "\n")))
+
+       `(,@(cond
+            ((eq? log-destination 'syslog)
+             '(("log_destination" "syslog")))
+            ((string? log-destination)
+             `(("log_destination" "stderr")
+               ("logging_collector" #t)
+               ("log_directory" ,log-destination)))
+            (else '()))
+         ("hba_file" ,hba-file)
+         ("ident_file" ,ident-file)
+         ,@(if socket-directory
+               `(("unix_socket_directories" ,socket-directory))
+               '())
+         ,@extra-config)))
+
+    (apply mixed-text-file "postgresql.conf" contents)))
+
 (define %postgresql-accounts
   (list (user-group (name "postgres") (system? #t))
         (user-account
@@ -178,124 +178,126 @@ host	all	all	::1/128 	md5"))
          #:builder
          (begin
            (use-modules (guix build utils) (guix build union) (srfi srfi-26))
-           (union-build (assoc-ref %outputs "out") (map (lambda (input) (cdr input)) %build-inputs))
+           (union-build (assoc-ref %outputs "out")
+                        (map (lambda (input) (cdr input)) %build-inputs))
            #t)))
       (inputs
        `(("postgresql" ,postgresql)
          ,@(map (lambda (extension) (list "extension" extension))
                 extension-packages))))))
 
-(define postgresql-activation
-  (match-lambda
-    (($ <postgresql-configuration> postgresql port locale config-file data-directory
-        extension-packages)
-     #~(begin
-         (use-modules (guix build utils)
-                      (ice-9 match))
-
-         (let ((user (getpwnam "postgres"))
-               (initdb (string-append #$(final-postgresql postgresql extension-packages)
-                                      "/bin/initdb"))
-               (initdb-args
-                (append
-                 (if #$locale
-                     (list (string-append "--locale=" #$locale))
-                     '()))))
-           ;; Create db state directory.
-           (mkdir-p #$data-directory)
-           (chown #$data-directory (passwd:uid user) (passwd:gid user))
-
-           ;; Drop privileges and init state directory in a new
-           ;; process.  Wait for it to finish before proceeding.
-           (match (primitive-fork)
-             (0
-              ;; Exit with a non-zero status code if an exception is thrown.
-              (dynamic-wind
-                (const #t)
-                (lambda ()
-                  (setgid (passwd:gid user))
-                  (setuid (passwd:uid user))
-                  (primitive-exit
-                   (apply system*
-                          initdb
-                          "-D"
-                          #$data-directory
-                          initdb-args)))
-                (lambda ()
-                  (primitive-exit 1))))
-             (pid (waitpid pid))))))))
-
-(define postgresql-shepherd-service
-  (match-lambda
-    (($ <postgresql-configuration> postgresql port locale config-file data-directory
-        extension-packages)
-     (let* ((pg_ctl-wrapper
-             ;; Wrapper script that switches to the 'postgres' user before
-             ;; launching daemon.
-             (program-file
-              "pg_ctl-wrapper"
-              #~(begin
-                  (use-modules (ice-9 match)
-                               (ice-9 format))
-                  (match (command-line)
-                    ((_ mode)
-                     (let ((user (getpwnam "postgres"))
-                           (pg_ctl #$(file-append (final-postgresql postgresql extension-packages)
-                                                  "/bin/pg_ctl"))
-                           (options (format #f "--config-file=~a -p ~d"
-                                            #$config-file #$port)))
-                       (setgid (passwd:gid user))
-                       (setuid (passwd:uid user))
-                       (execl pg_ctl pg_ctl "-D" #$data-directory "-o" options
-                              mode)))))))
-            (pid-file (in-vicinity data-directory "postmaster.pid"))
-            (action (lambda args
-                      #~(lambda _
-                          (invoke #$pg_ctl-wrapper #$@args)
-                          (match '#$args
-                            (("start")
-                             (call-with-input-file #$pid-file read))
-                            (_ #t))))))
-       (list (shepherd-service
-              (provision '(postgres))
-              (documentation "Run the PostgreSQL daemon.")
-              (requirement '(user-processes loopback syslogd))
-              (modules `((ice-9 match)
-                         ,@%default-modules))
-              (start (action "start"))
-              (stop (action "stop"))))))))
+(define (postgresql-activation config)
+  (match-record config <postgresql-configuration>
+    (postgresql port locale data-directory log-destination socket-directory
+                extension-packages)
+    #~(begin
+        (use-modules (guix build utils)
+                     (ice-9 match))
+
+        (let ((user (getpwnam "postgres"))
+              (initdb (string-append
+                       #$(final-postgresql postgresql extension-packages)
+                       "/bin/initdb"))
+              (initdb-args
+               (append
+                (if #$locale
+                    (list (string-append "--locale=" #$locale))
+                    '()))))
+          ;; Create db state directory.
+          (mkdir-p #$data-directory)
+          (chown #$data-directory (passwd:uid user) (passwd:gid user))
+
+          (when (string? #$socket-directory)
+            (mkdir-p #$socket-directory)
+            (chown #$socket-directory (passwd:uid user) (passwd:gid user)))
+
+          (when (string? #$log-destination)
+            (mkdir-p #$log-destination)
+            (chown #$log-destination (passwd:uid user) (passwd:gid user)))
+
+          ;; Drop privileges and init state directory in a new
+          ;; process.  Wait for it to finish before proceeding.
+          (match (primitive-fork)
+            (0
+             ;; Exit with a non-zero status code if an exception is thrown.
+             (dynamic-wind
+               (const #t)
+               (lambda ()
+                 (setgid (passwd:gid user))
+                 (setuid (passwd:uid user))
+                 (primitive-exit
+                  (apply system*
+                         initdb
+                         "-D"
+                         #$data-directory
+                         initdb-args)))
+               (lambda ()
+                 (primitive-exit 1))))
+            (pid (waitpid pid)))))))
+
+(define (postgresql-shepherd-service config)
+  (match-record config <postgresql-configuration>
+    (postgresql port locale data-directory log-destination extension-packages)
+    (let* ((config-file (postgresql-config-file config))
+           (pg_ctl-wrapper
+            ;; Wrapper script that switches to the 'postgres' user before
+            ;; launching daemon.
+            (program-file
+             "pg_ctl-wrapper"
+             #~(begin
+                 (use-modules (ice-9 match)
+                              (ice-9 format))
+                 (match (command-line)
+                   ((_ mode)
+                    (let ((user (getpwnam "postgres"))
+                          (pg_ctl #$(file-append
+                                     (final-postgresql postgresql
+                                                       extension-packages)
+                                     "/bin/pg_ctl"))
+                          (options
+                           (format #f "--config-file=~a -p ~d"
+                                   #$config-file
+                                   #$port)))
+                      (setgid (passwd:gid user))
+                      (setuid (passwd:uid user))
+                      (execl pg_ctl pg_ctl "-D" #$data-directory
+                             #$@(if (string? log-destination)
+                                    (list "-l"
+                                          (string-append log-destination
+                                                         "/pg_ctl.log"))
+                                    '())
+                             "-o" options
+                             mode)))))))
+           (pid-file (in-vicinity data-directory "postmaster.pid"))
+           (action (lambda args
+                     #~(lambda _
+                         (invoke #$pg_ctl-wrapper #$@args)
+                         (match '#$args
+                           (("start")
+                            (call-with-input-file #$pid-file read))
+                           (_ #t))))))
+      (list (shepherd-service
+             (provision '(postgres))
+             (documentation "Run the PostgreSQL daemon.")
+             (requirement '(user-processes loopback syslogd))
+             (modules `((ice-9 match)
+                        ,@%default-modules))
+             (start (action "start"))
+             (stop (action "stop")))))))
 
 (define postgresql-service-type
-  (service-type (name 'postgresql)
-                (extensions
-                 (list (service-extension shepherd-root-service-type
-                                          postgresql-shepherd-service)
-                       (service-extension activation-service-type
-                                          postgresql-activation)
-                       (service-extension account-service-type
-                                          (const %postgresql-accounts))
-                       (service-extension profile-service-type
-                                          (compose list postgresql-configuration-postgresql))))))
-
-(define-deprecated (postgresql-service #:key (postgresql postgresql)
-                                       (port 5432)
-                                       (locale "en_US.utf8")
-                                       (config-file (postgresql-config-file))
-                                       (data-directory "/var/lib/postgresql/data")
-                                       (extension-packages '()))
-  postgresql-service-type
-  "Return a service that runs @var{postgresql}, the PostgreSQL database server.
-
-The PostgreSQL daemon loads its runtime configuration from @var{config-file}
-and stores the database cluster in @var{data-directory}."
-  (service postgresql-service-type
-           (postgresql-configuration
-            (postgresql postgresql)
-            (port port)
-            (locale locale)
-            (config-file config-file)
-            (data-directory data-directory)
-            (extension-packages extension-packages))))
+  (service-type
+   (name 'postgresql)
+   (extensions
+    (list (service-extension shepherd-root-service-type
+                             postgresql-shepherd-service)
+          (service-extension activation-service-type
+                             postgresql-activation)
+          (service-extension account-service-type
+                             (const %postgresql-accounts))
+          (service-extension
+           profile-service-type
+           (compose list postgresql-configuration-postgresql))))))
 
 \f
 ;;;
diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm
index 31d5ae4c6a..499ab8c9d1 100644
--- a/gnu/tests/databases.scm
+++ b/gnu/tests/databases.scm
@@ -24,6 +24,7 @@
   #:use-module (gnu system shadow)
   #:use-module (gnu system vm)
   #:use-module (gnu services)
+  #:use-module (gnu services base)
   #:use-module (gnu services databases)
   #:use-module (gnu services networking)
   #:use-module (gnu packages databases)
@@ -214,11 +215,21 @@
 ;;; The PostgreSQL service.
 ;;;
 
+(define %postgresql-log-directory
+  "/var/log/postgresql")
+
 (define %postgresql-os
   (simple-operating-system
    (service postgresql-service-type
             (postgresql-configuration
-             (postgresql postgresql-10)))))
+             (postgresql postgresql-10)
+             (log-destination %postgresql-log-directory)
+             (extra-config
+              '(("session_preload_libraries" "auto_explain")
+                ("random_page_cost" 2)
+                ("auto_explain.log_min_duration" "100 ms")
+                ("work_mem" "500 MB")
+                ("debug_print_plan" #t)))))))
 
 (define (run-postgresql-test)
   "Run tests in %POSTGRESQL-OS."
@@ -254,6 +265,23 @@
                 (start-service 'postgres))
              marionette))
 
+          (test-assert "log-file"
+            (marionette-eval
+             '(begin
+                (use-modules (ice-9 ftw)
+                             (ice-9 match))
+                (current-output-port
+                 (open-file "/dev/console" "w0"))
+                (let ((server-log-file
+                       (string-append #$%postgresql-log-directory
+                                      "/pg_ctl.log")))
+                  (and (file-exists? server-log-file)
+                       (display
+                        (call-with-input-file server-log-file
+                          get-string-all)))
+                  #t))
+             marionette))
+
           (test-end)
           (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
 
diff --git a/gnu/tests/guix.scm b/gnu/tests/guix.scm
index af7d8f0b21..4446c4e36b 100644
--- a/gnu/tests/guix.scm
+++ b/gnu/tests/guix.scm
@@ -157,14 +157,12 @@
    (service postgresql-service-type
             (postgresql-configuration
              (postgresql postgresql-10)
-             (config-file
-              (postgresql-config-file
-               (hba-file
-                (plain-file "pg_hba.conf"
-                            "
+             (hba-file
+              (plain-file "pg_hba.conf"
+                          "
 local	all	all			trust
 host	all	all	127.0.0.1/32 	trust
-host	all	all	::1/128 	trust"))))))
+host	all	all	::1/128 	trust"))))
    (service guix-data-service-type
             (guix-data-service-configuration
              (host "0.0.0.0")))
-- 
2.29.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#45860] Improve PostgreSQL service.
  2021-01-14 13:36 [bug#45860] Improve PostgreSQL service Mathieu Othacehe
@ 2021-01-14 21:56 ` Christopher Baines
  2021-01-15  8:56   ` Mathieu Othacehe
  2021-01-18 10:16 ` [bug#45860] [PATCH v2 0/5] services: postgresql: Improve service Mathieu Othacehe
  1 sibling, 1 reply; 10+ messages in thread
From: Christopher Baines @ 2021-01-14 21:56 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 45860

[-- Attachment #1: Type: text/plain, Size: 2305 bytes --]


Mathieu Othacehe <othacehe@gnu.org> writes:

> Hello,
>
> Here's a patch to improve PostgreSQL service. It merges
> <postgresql-configuration> and <postgresql-config-file> records. It also
> sanitises parameters conversion and logging.
>
> Thanks,
>
> Mathieu
> From 87703b749631acd8ddc2b9eeb36a5be7189a019b Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <othacehe@gnu.org>
> Date: Thu, 14 Jan 2021 14:13:30 +0100
> Subject: [PATCH] Improve PostgreSQL service.
>
> Merge <postgresql-configuration> and <postgresql-config-file> records,
> sanitize parameters convertion and logging.
>
> * gnu/services/databases.scm (postgresql-config-file,
> postgresql-config-file?, postgresql-config-file-log-destination,
> postgresql-config-file-hba-file, postgresql-config-file-ident-file,
> postgresql-config-file-extra-config, postgresql-configuration): Remove them.
> (postgresql-configuration-log-destination,
> postgresql-configuration-hba-file,
> postgresql-configuration-ident-file,
> postgresql-configuration-socket-directory,
> postgresql-configuration-extra-config,
> postgresql-configuration-extension-packages): New exported procedures.
> (<postgresql-config-file>): Merge it with ...
> (<postgresql-configuration>): ... this record, and add a "socket-directory"
> field.
> (postgresql-config-file-compiler): Replace it with ...
> (postgresql-config-file): ... this procedure.
> (postgresql-activation): Use "match-record" instead of "match". Create the
> "socket-directory" if needed.
> (postgresql-shepherd-service): Use "match-record" intead of "match". Pass the
> "log-destination" argument to "pg_ctl" if needed.
> (postgresql-service): Remove it.
> * gnu/tests/databases.scm (%postgresql-log-directory): New variable.
> (%postgresql-os): Pass "log-destination" and "extra-config" fields.
> (log-file): New test case.
> * gnu/tests/guix.scm (%guix-data-service-os): Adapt accordingly.
> * doc/guix.texi (Database Services): Ditto.

I haven't read through these changes in detail, but the mixing of the
record describing the config file, and the record for configuring the
service introduces the limitation that you can no longer specify any
lowerable object (like a file) or something like a string to use a
config file outside of the store. Did you have a reason for mixing the
records together?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#45860] Improve PostgreSQL service.
  2021-01-14 21:56 ` Christopher Baines
@ 2021-01-15  8:56   ` Mathieu Othacehe
  2021-01-16 11:44     ` Christopher Baines
  0 siblings, 1 reply; 10+ messages in thread
From: Mathieu Othacehe @ 2021-01-15  8:56 UTC (permalink / raw)
  To: Christopher Baines; +Cc: 45860


Hello Chris,

> I haven't read through these changes in detail, but the mixing of the
> record describing the config file, and the record for configuring the
> service introduces the limitation that you can no longer specify any
> lowerable object (like a file) or something like a string to use a
> config file outside of the store. Did you have a reason for mixing the
> records together?

I must admit I overlooked that possibility. The reason for merging the
records is that the "log-destination" is now needed both to enable
"pg_ctl" logging in "postgresql-shepherd-service" and in
"postgresql-config-file" to be written in PostgreSQL configuration.

Plus having a record called <postgresql-configuration> that does not
contain some of the configuration field feels weird.

Is passing a lowerable config file a use case of yours? In that case I
could still add a "raw-config" field to override the configuration file
creation.

Thanks,

Mathieu




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#45860] Improve PostgreSQL service.
  2021-01-15  8:56   ` Mathieu Othacehe
@ 2021-01-16 11:44     ` Christopher Baines
  0 siblings, 0 replies; 10+ messages in thread
From: Christopher Baines @ 2021-01-16 11:44 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 45860

[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]


Mathieu Othacehe <othacehe@gnu.org> writes:

> Hello Chris,
>
>> I haven't read through these changes in detail, but the mixing of the
>> record describing the config file, and the record for configuring the
>> service introduces the limitation that you can no longer specify any
>> lowerable object (like a file) or something like a string to use a
>> config file outside of the store. Did you have a reason for mixing the
>> records together?
>
> I must admit I overlooked that possibility. The reason for merging the
> records is that the "log-destination" is now needed both to enable
> "pg_ctl" logging in "postgresql-shepherd-service" and in
> "postgresql-config-file" to be written in PostgreSQL configuration.
>
> Plus having a record called <postgresql-configuration> that does not
> contain some of the configuration field feels weird.
>
> Is passing a lowerable config file a use case of yours? In that case I
> could still add a "raw-config" field to override the configuration file
> creation.

It's not, however given I'm able to make changes to the service
definition, that's what I generally do when I have a problem with it,
rather than sidestepping the Guix configuration layer. It's hard to tell
if anyone is doing that or not.

This pattern of using a record with a gexp-compiler is used for quite a
few services now, but mostly because I've implemented quite a few
services (I think there's one case where someone else did similarly).

I get that there's some value in trying to help users by creating the
relevant directory for logs, but I'm not sure it requires all these
changes.

I'm also unsure about using the same names for configuration parameters,
but picking different semantics. log-destination [1] can be a list
(comma separated string), which I reasonably could be "stderr,syslog" or
'(stderr syslog) in the Guix configuration (just as an example), and
with the service changes proposed here the string value would mean that
log_directory gets set to "stderr,syslog" which seems wrong.

1: https://www.postgresql.org/docs/13/runtime-config-logging.html#GUC-LOG-DESTINATION

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#45860] [PATCH v2 0/5] services: postgresql: Improve service.
  2021-01-14 13:36 [bug#45860] Improve PostgreSQL service Mathieu Othacehe
  2021-01-14 21:56 ` Christopher Baines
@ 2021-01-18 10:16 ` Mathieu Othacehe
  2021-01-18 10:16   ` [bug#45860] [PATCH v2 1/5] services: postgresql: Use Guile datatypes Mathieu Othacehe
                     ` (4 more replies)
  1 sibling, 5 replies; 10+ messages in thread
From: Mathieu Othacehe @ 2021-01-18 10:16 UTC (permalink / raw)
  To: 45860; +Cc: Mathieu Othacehe

Hello,

Here's a v2 of the patchset. Following Chris advises, I did not merge the two
configuration records. I also break the patch into four smaller patches.

I also added a 'postgresql-role-service-type' that allows to create database
roles in a declarative fashion.

Thanks,

Mathieu

Mathieu Othacehe (5):
  services: postgresql: Use Guile datatypes.
  services: postgresql: Add socket directory support.
  services: postgresql: Add log directory support.
  services: postgresql: Wrap long lines.
  services: postgresql: Add postgresql-role-service-type.

 doc/guix.texi              |  90 +++++++++++++-
 gnu/services/databases.scm | 233 +++++++++++++++++++++++++++++--------
 gnu/tests/databases.scm    |  72 +++++++++++-
 3 files changed, 342 insertions(+), 53 deletions(-)

-- 
2.29.2





^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#45860] [PATCH v2 1/5] services: postgresql: Use Guile datatypes.
  2021-01-18 10:16 ` [bug#45860] [PATCH v2 0/5] services: postgresql: Improve service Mathieu Othacehe
@ 2021-01-18 10:16   ` Mathieu Othacehe
  2021-01-18 10:16   ` [bug#45860] [PATCH v2 2/5] services: postgresql: Add socket directory support Mathieu Othacehe
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Mathieu Othacehe @ 2021-01-18 10:16 UTC (permalink / raw)
  To: 45860; +Cc: Mathieu Othacehe

* gnu/services/databases.scm (postgresql-config-file-compiler): Support Guile
datatypes in the "extra-config" field.
* gnu/tests/databases.scm (%postgresql-os): Test it.
* doc/guix.texi (Database Services): Document it.
---
 doc/guix.texi              | 18 ++++++++++++------
 gnu/services/databases.scm | 38 ++++++++++++++++++++++----------------
 gnu/tests/databases.scm    | 10 +++++++++-
 3 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index dc41fe9aea..3ec5e3be15 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19382,12 +19382,12 @@ local	all	all			trust
 host	all	all	127.0.0.1/32 	md5
 host	all	all	::1/128 	md5"))
             (extra-config
-             '(("session_preload_libraries"     "'auto_explain'")
-               ("random_page_cost"              "2")
-               ("auto_explain.log_min_duration" "'100ms'")
-               ("work_mem"                      "'500MB'")
-               ("logging_collector"             "on")
-               ("log_directory"                 "'/var/log/postgresql'")))))))
+             '(("session_preload_libraries"     "auto_explain")
+               ("random_page_cost"              2)
+               ("auto_explain.log_min_duration" "100 ms")
+               ("work_mem"                      "500 MB")
+               ("logging_collector"             #t)
+               ("log_directory"                 "/var/log/postgresql")))))))
 @end lisp
 
 @table @asis
@@ -19407,6 +19407,12 @@ List of additional keys and values to include in the PostgreSQL config
 file.  Each entry in the list should be a list where the first element
 is the key, and the remaining elements are the values.
 
+The values can be numbers, booleans or strings and will be mapped to
+PostgreSQL parameters types @code{Boolean}, @code{String},
+@code{Numeric}, @code{Numeric with Unit} and @code{Enumerated} described
+@uref{https://www.postgresql.org/docs/current/config-setting.html,
+here}.
+
 @end table
 @end deftp
 
diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm
index d2dc5f0da8..bb0e40632e 100644
--- a/gnu/services/databases.scm
+++ b/gnu/services/databases.scm
@@ -115,22 +115,28 @@ host	all	all	::1/128 	md5"))
   (match file
     (($ <postgresql-config-file> log-destination hba-file
                                  ident-file extra-config)
-     (define (single-quote string)
-       (if string
-           (list "'" string "'")
-           '()))
-
-     (define contents
-       (append-map
-        (match-lambda
-          ((key) '())
-          ((key . #f) '())
-          ((key values ...) `(,key " = " ,@values "\n")))
-
-        `(("log_destination" ,@(single-quote log-destination))
-          ("hba_file" ,@(single-quote hba-file))
-          ("ident_file" ,@(single-quote ident-file))
-          ,@extra-config)))
+     ;; See: https://www.postgresql.org/docs/current/config-setting.html.
+    (define (format-value value)
+      (cond
+       ((boolean? value)
+        (list (if value "on" "off")))
+       ((number? value)
+        (list (number->string value)))
+       (else
+        (list "'" value "'"))))
+
+    (define contents
+      (append-map
+       (match-lambda
+         ((key) '())
+         ((key . #f) '())
+         ((key values ...)
+          `(,key " = " ,@(append-map format-value values) "\n")))
+
+       `(("log_destination" ,log-destination)
+         ("hba_file" ,hba-file)
+         ("ident_file" ,ident-file)
+         ,@extra-config)))
 
      (gexp->derivation
       "postgresql.conf"
diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm
index 31d5ae4c6a..7338007919 100644
--- a/gnu/tests/databases.scm
+++ b/gnu/tests/databases.scm
@@ -218,7 +218,15 @@
   (simple-operating-system
    (service postgresql-service-type
             (postgresql-configuration
-             (postgresql postgresql-10)))))
+             (postgresql postgresql-10)
+             (config-file
+              (postgresql-config-file
+               (extra-config
+                '(("session_preload_libraries" "auto_explain")
+                  ("random_page_cost" 2)
+                  ("auto_explain.log_min_duration" "100 ms")
+                  ("work_mem" "500 MB")
+                  ("debug_print_plan" #t)))))))))
 
 (define (run-postgresql-test)
   "Run tests in %POSTGRESQL-OS."
-- 
2.29.2





^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#45860] [PATCH v2 2/5] services: postgresql: Add socket directory support.
  2021-01-18 10:16 ` [bug#45860] [PATCH v2 0/5] services: postgresql: Improve service Mathieu Othacehe
  2021-01-18 10:16   ` [bug#45860] [PATCH v2 1/5] services: postgresql: Use Guile datatypes Mathieu Othacehe
@ 2021-01-18 10:16   ` Mathieu Othacehe
  2021-01-18 10:16   ` [bug#45860] [PATCH v2 3/5] services: postgresql: Add log " Mathieu Othacehe
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Mathieu Othacehe @ 2021-01-18 10:16 UTC (permalink / raw)
  To: 45860; +Cc: Mathieu Othacehe

* gnu/services/databases.scm (postgresql-config-file-socket-directory): New
procedure.
(<postgresql-config-file>)[socket-directory]: New field.
(postgresql-config-file-compiler): Honor it.
(postgresql-activation): Create the socket directory if needed.
* doc/guix.texi (Database Services): Document it.
---
 doc/guix.texi              |  6 ++++++
 gnu/services/databases.scm | 32 +++++++++++++++++++++++---------
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 3ec5e3be15..46039d26d0 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19402,6 +19402,12 @@ configuration.
 @item @code{ident-file} (default: @code{%default-postgres-ident})
 Filename or G-expression for the user name mapping configuration.
 
+@item @code{socket-directory} (default: @code{"/var/lib/postgresql"})
+Specifies the directory of the Unix-domain socket(s) on which PostgreSQL
+is to listen for connections from client applications.  If set to
+@code{#false} PostgreSQL does not listen on any Unix-domain sockets, in
+which case only TCP/IP sockets can be used to connect to the server.
+
 @item @code{extra-config} (default: @code{'()})
 List of additional keys and values to include in the PostgreSQL config
 file.  Each entry in the list should be a list where the first element
diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm
index bb0e40632e..83dee52cf3 100644
--- a/gnu/services/databases.scm
+++ b/gnu/services/databases.scm
@@ -43,6 +43,7 @@
             postgresql-config-file-log-destination
             postgresql-config-file-hba-file
             postgresql-config-file-ident-file
+            postgresql-config-file-socket-directory
             postgresql-config-file-extra-config
 
             postgresql-configuration
@@ -101,20 +102,23 @@ host	all	all	::1/128 	md5"))
 (define-record-type* <postgresql-config-file>
   postgresql-config-file make-postgresql-config-file
   postgresql-config-file?
-  (log-destination postgresql-config-file-log-destination
-                   (default "syslog"))
-  (hba-file        postgresql-config-file-hba-file
-                   (default %default-postgres-hba))
-  (ident-file      postgresql-config-file-ident-file
-                   (default %default-postgres-ident))
-  (extra-config    postgresql-config-file-extra-config
-                   (default '())))
+  (log-destination   postgresql-config-file-log-destination
+                     (default "syslog"))
+  (hba-file          postgresql-config-file-hba-file
+                     (default %default-postgres-hba))
+  (ident-file        postgresql-config-file-ident-file
+                     (default %default-postgres-ident))
+  (socket-directory  postgresql-config-file-socket-directory
+                     (default "/var/run/postgresql"))
+  (extra-config      postgresql-config-file-extra-config
+                     (default '())))
 
 (define-gexp-compiler (postgresql-config-file-compiler
                        (file <postgresql-config-file>) system target)
   (match file
     (($ <postgresql-config-file> log-destination hba-file
-                                 ident-file extra-config)
+                                 ident-file socket-directory
+                                 extra-config)
      ;; See: https://www.postgresql.org/docs/current/config-setting.html.
     (define (format-value value)
       (cond
@@ -136,6 +140,9 @@ host	all	all	::1/128 	md5"))
        `(("log_destination" ,log-destination)
          ("hba_file" ,hba-file)
          ("ident_file" ,ident-file)
+         ,@(if socket-directory
+               `(("unix_socket_directories" ,socket-directory))
+               '())
          ,@extra-config)))
 
      (gexp->derivation
@@ -211,6 +218,13 @@ host	all	all	::1/128 	md5"))
            (mkdir-p #$data-directory)
            (chown #$data-directory (passwd:uid user) (passwd:gid user))
 
+           ;; Create the socket directory.
+           (let ((socket-directory
+                  #$(postgresql-config-file-socket-directory config-file)))
+             (when (string? socket-directory)
+               (mkdir-p socket-directory)
+               (chown socket-directory (passwd:uid user) (passwd:gid user))))
+
            ;; Drop privileges and init state directory in a new
            ;; process.  Wait for it to finish before proceeding.
            (match (primitive-fork)
-- 
2.29.2





^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#45860] [PATCH v2 3/5] services: postgresql: Add log directory support.
  2021-01-18 10:16 ` [bug#45860] [PATCH v2 0/5] services: postgresql: Improve service Mathieu Othacehe
  2021-01-18 10:16   ` [bug#45860] [PATCH v2 1/5] services: postgresql: Use Guile datatypes Mathieu Othacehe
  2021-01-18 10:16   ` [bug#45860] [PATCH v2 2/5] services: postgresql: Add socket directory support Mathieu Othacehe
@ 2021-01-18 10:16   ` Mathieu Othacehe
  2021-01-18 10:16   ` [bug#45860] [PATCH v2 4/5] services: postgresql: Wrap long lines Mathieu Othacehe
  2021-01-18 10:16   ` [bug#45860] [PATCH v2 5/5] services: postgresql: Add postgresql-role-service-type Mathieu Othacehe
  4 siblings, 0 replies; 10+ messages in thread
From: Mathieu Othacehe @ 2021-01-18 10:16 UTC (permalink / raw)
  To: 45860; +Cc: Mathieu Othacehe

* gnu/services/databases.scm (postgresql-configuration-log-directory): New
procedure.
(<postgresql-configuration>)[log-directory]: New field.
(postgresql-activation): Create the log directory.
(postgresql-shepherd-service): Honor it.
* gnu/tests/databases.scm (%postgresql-log-directory): New variable.
(log-file): New test case.
* doc/guix.texi (Database Services): Document it.
---
 doc/guix.texi              |  5 +++++
 gnu/services/databases.scm | 36 ++++++++++++++++++++++++++++--------
 gnu/tests/databases.scm    | 20 ++++++++++++++++++++
 3 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 46039d26d0..22674e2804 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19316,6 +19316,11 @@ The configuration file to use when running PostgreSQL.  The default
 behaviour uses the postgresql-config-file record with the default values
 for the fields.
 
+@item @code{log-directory} (default: @code{"/var/log/postgresql"})
+The directory where @command{pg_ctl} output will be written in a file
+named @code{"pg_ctl.log"}.  This file can be useful to debug PostgreSQL
+configuration errors for instance.
+
 @item @code{data-directory} (default: @code{"/var/lib/postgresql/data"})
 Directory in which to store the data.
 
diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm
index 83dee52cf3..c387a7da6c 100644
--- a/gnu/services/databases.scm
+++ b/gnu/services/databases.scm
@@ -52,6 +52,7 @@
             postgresql-configuration-port
             postgresql-configuration-locale
             postgresql-configuration-file
+            postgresql-configuration-log-directory
             postgresql-configuration-data-directory
 
             postgresql-service
@@ -164,6 +165,8 @@ host	all	all	::1/128 	md5"))
                       (default "en_US.utf8"))
   (config-file        postgresql-configuration-file
                       (default (postgresql-config-file)))
+  (log-directory      postgresql-configuration-log-directory
+                      (default "/var/log/postgresql"))
   (data-directory     postgresql-configuration-data-directory
                       (default "/var/lib/postgresql/data"))
   (extension-packages postgresql-configuration-extension-packages
@@ -200,15 +203,18 @@ host	all	all	::1/128 	md5"))
 
 (define postgresql-activation
   (match-lambda
-    (($ <postgresql-configuration> postgresql port locale config-file data-directory
-        extension-packages)
+    (($ <postgresql-configuration> postgresql port locale config-file
+                                   log-directory data-directory
+                                   extension-packages)
      #~(begin
          (use-modules (guix build utils)
                       (ice-9 match))
 
          (let ((user (getpwnam "postgres"))
-               (initdb (string-append #$(final-postgresql postgresql extension-packages)
-                                      "/bin/initdb"))
+               (initdb (string-append
+                        #$(final-postgresql postgresql
+                                            extension-packages)
+                        "/bin/initdb"))
                (initdb-args
                 (append
                  (if #$locale
@@ -225,6 +231,11 @@ host	all	all	::1/128 	md5"))
                (mkdir-p socket-directory)
                (chown socket-directory (passwd:uid user) (passwd:gid user))))
 
+           ;; Create the log directory.
+           (when (string? #$log-directory)
+             (mkdir-p #$log-directory)
+             (chown #$log-directory (passwd:uid user) (passwd:gid user)))
+
            ;; Drop privileges and init state directory in a new
            ;; process.  Wait for it to finish before proceeding.
            (match (primitive-fork)
@@ -247,8 +258,9 @@ host	all	all	::1/128 	md5"))
 
 (define postgresql-shepherd-service
   (match-lambda
-    (($ <postgresql-configuration> postgresql port locale config-file data-directory
-        extension-packages)
+    (($ <postgresql-configuration> postgresql port locale config-file
+                                   log-directory data-directory
+                                   extension-packages)
      (let* ((pg_ctl-wrapper
              ;; Wrapper script that switches to the 'postgres' user before
              ;; launching daemon.
@@ -260,13 +272,21 @@ host	all	all	::1/128 	md5"))
                   (match (command-line)
                     ((_ mode)
                      (let ((user (getpwnam "postgres"))
-                           (pg_ctl #$(file-append (final-postgresql postgresql extension-packages)
+                           (pg_ctl #$(file-append
+                                      (final-postgresql postgresql
+                                                        extension-packages)
                                                   "/bin/pg_ctl"))
                            (options (format #f "--config-file=~a -p ~d"
                                             #$config-file #$port)))
                        (setgid (passwd:gid user))
                        (setuid (passwd:uid user))
-                       (execl pg_ctl pg_ctl "-D" #$data-directory "-o" options
+                       (execl pg_ctl pg_ctl "-D" #$data-directory
+                              #$@(if (string? log-directory)
+                                     (list "-l"
+                                           (string-append log-directory
+                                                          "/pg_ctl.log"))
+                                     '())
+                              "-o" options
                               mode)))))))
             (pid-file (in-vicinity data-directory "postmaster.pid"))
             (action (lambda args
diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm
index 7338007919..d881a8c3ee 100644
--- a/gnu/tests/databases.scm
+++ b/gnu/tests/databases.scm
@@ -214,6 +214,9 @@
 ;;; The PostgreSQL service.
 ;;;
 
+(define %postgresql-log-directory
+  "/var/log/postgresql")
+
 (define %postgresql-os
   (simple-operating-system
    (service postgresql-service-type
@@ -262,6 +265,23 @@
                 (start-service 'postgres))
              marionette))
 
+          (test-assert "log-file"
+            (marionette-eval
+             '(begin
+                (use-modules (ice-9 ftw)
+                             (ice-9 match))
+                (current-output-port
+                 (open-file "/dev/console" "w0"))
+                (let ((server-log-file
+                       (string-append #$%postgresql-log-directory
+                                      "/pg_ctl.log")))
+                  (and (file-exists? server-log-file)
+                       (display
+                        (call-with-input-file server-log-file
+                          get-string-all)))
+                  #t))
+             marionette))
+
           (test-end)
           (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
 
-- 
2.29.2





^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#45860] [PATCH v2 4/5] services: postgresql: Wrap long lines.
  2021-01-18 10:16 ` [bug#45860] [PATCH v2 0/5] services: postgresql: Improve service Mathieu Othacehe
                     ` (2 preceding siblings ...)
  2021-01-18 10:16   ` [bug#45860] [PATCH v2 3/5] services: postgresql: Add log " Mathieu Othacehe
@ 2021-01-18 10:16   ` Mathieu Othacehe
  2021-01-18 10:16   ` [bug#45860] [PATCH v2 5/5] services: postgresql: Add postgresql-role-service-type Mathieu Othacehe
  4 siblings, 0 replies; 10+ messages in thread
From: Mathieu Othacehe @ 2021-01-18 10:16 UTC (permalink / raw)
  To: 45860; +Cc: Mathieu Othacehe

* gnu/services/databases.scm: Wrap long lines, no functional change.
---
 gnu/services/databases.scm | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm
index c387a7da6c..0d60616156 100644
--- a/gnu/services/databases.scm
+++ b/gnu/services/databases.scm
@@ -194,7 +194,9 @@ host	all	all	::1/128 	md5"))
          #:builder
          (begin
            (use-modules (guix build utils) (guix build union) (srfi srfi-26))
-           (union-build (assoc-ref %outputs "out") (map (lambda (input) (cdr input)) %build-inputs))
+           (union-build (assoc-ref %outputs "out")
+                        (map (lambda (input) (cdr input))
+                             %build-inputs))
            #t)))
       (inputs
        `(("postgresql" ,postgresql)
@@ -306,25 +308,29 @@ host	all	all	::1/128 	md5"))
               (stop (action "stop"))))))))
 
 (define postgresql-service-type
-  (service-type (name 'postgresql)
-                (extensions
-                 (list (service-extension shepherd-root-service-type
-                                          postgresql-shepherd-service)
-                       (service-extension activation-service-type
-                                          postgresql-activation)
-                       (service-extension account-service-type
-                                          (const %postgresql-accounts))
-                       (service-extension profile-service-type
-                                          (compose list postgresql-configuration-postgresql))))))
+  (service-type
+   (name 'postgresql)
+   (extensions
+    (list (service-extension shepherd-root-service-type
+                             postgresql-shepherd-service)
+          (service-extension activation-service-type
+                             postgresql-activation)
+          (service-extension account-service-type
+                             (const %postgresql-accounts))
+          (service-extension
+           profile-service-type
+           (compose list postgresql-configuration-postgresql))))))
 
 (define-deprecated (postgresql-service #:key (postgresql postgresql)
                                        (port 5432)
                                        (locale "en_US.utf8")
                                        (config-file (postgresql-config-file))
-                                       (data-directory "/var/lib/postgresql/data")
+                                       (data-directory
+                                        "/var/lib/postgresql/data")
                                        (extension-packages '()))
   postgresql-service-type
-  "Return a service that runs @var{postgresql}, the PostgreSQL database server.
+  "Return a service that runs @var{postgresql}, the PostgreSQL database
+server.
 
 The PostgreSQL daemon loads its runtime configuration from @var{config-file}
 and stores the database cluster in @var{data-directory}."
-- 
2.29.2





^ permalink raw reply	[flat|nested] 10+ messages in thread

* [bug#45860] [PATCH v2 5/5] services: postgresql: Add postgresql-role-service-type.
  2021-01-18 10:16 ` [bug#45860] [PATCH v2 0/5] services: postgresql: Improve service Mathieu Othacehe
                     ` (3 preceding siblings ...)
  2021-01-18 10:16   ` [bug#45860] [PATCH v2 4/5] services: postgresql: Wrap long lines Mathieu Othacehe
@ 2021-01-18 10:16   ` Mathieu Othacehe
  4 siblings, 0 replies; 10+ messages in thread
From: Mathieu Othacehe @ 2021-01-18 10:16 UTC (permalink / raw)
  To: 45860; +Cc: Mathieu Othacehe

* gnu/services/databases.scm (postgresql-role,
postgresql-role?, postgresql-role-name,
postgresql-role-permissions, postgresql-role-create-database?,
postgresql-role-configuration, postgresql-role-configuration?,
postgresql-role-configuration-host, postgresql-role-configuration-roles,
postgresql-role-service-type): New procedures.
* gnu/tests/databases.scm: Test it.
* doc/guix.texi: Document it.
---
 doc/guix.texi              | 61 ++++++++++++++++++++++++
 gnu/services/databases.scm | 95 ++++++++++++++++++++++++++++++++++++++
 gnu/tests/databases.scm    | 44 +++++++++++++++++-
 3 files changed, 199 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 22674e2804..13d95b36d1 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19427,6 +19427,67 @@ here}.
 @end table
 @end deftp
 
+@deffn {Scheme Variable} postgresql-role-service-type
+This service allows to create PostgreSQL roles and databases after
+PostgreSQL service start.  Here is an example of its use.
+
+@lisp
+(service postgresql-role-service-type
+            (postgresql-role-configuration
+             (roles
+              (list (postgresql-role
+                     (name "test")
+                     (create-database? #t))))))
+@end lisp
+
+This service can be extended with extra roles, as in this
+example:
+
+@lisp
+(service-extension postgresql-role-service-type
+                   (const (postgresql-role
+                           (name "alice")
+                           (create-database? #t))))
+@end lisp
+@end deffn
+
+@deftp {Data Type} postgresql-role
+PostgreSQL manages database access permissions using the concept of
+roles.  A role can be thought of as either a database user, or a group
+of database users, depending on how the role is set up.  Roles can own
+database objects (for example, tables) and can assign privileges on
+those objects to other roles to control who has access to which objects.
+
+@table @asis
+@item @code{name}
+The role name.
+
+@item @code{permissions} (default: @code{'(createdb login)})
+The role permissions list.  Supported permissions are @code{createdb}
+and @code{login}.
+
+@item @code{create-database?} (default: @code{#f})
+Whether to create a database with the same name as the role.
+
+@end table
+@end deftp
+
+@deftp {Data Type} postgresql-role-configuration
+Data type representing the configuration of
+@var{postgresql-role-service-type}.
+
+@table @asis
+@item @code{host} (default: @code{"/var/run/postgresql"})
+The PostgreSQL host to connect to.
+
+@item @code{log} (default: @code{"/var/log/postgresql_roles.log"})
+File name of the log file.
+
+@item @code{roles} (default: @code{'()})
+The initial PostgreSQL roles to create.
+@end table
+@end deftp
+
 @subsubheading MariaDB/MySQL
 
 @defvr {Scheme Variable} mysql-service-type
diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm
index 0d60616156..88e4b1813a 100644
--- a/gnu/services/databases.scm
+++ b/gnu/services/databases.scm
@@ -58,6 +58,18 @@
             postgresql-service
             postgresql-service-type
 
+            postgresql-role
+            postgresql-role?
+            postgresql-role-name
+            postgresql-role-permissions
+            postgresql-role-create-database?
+            postgresql-role-configuration
+            postgresql-role-configuration?
+            postgresql-role-configuration-host
+            postgresql-role-configuration-roles
+
+            postgresql-role-service-type
+
             memcached-service-type
             memcached-configuration
             memcached-configuration?
@@ -343,6 +355,89 @@ and stores the database cluster in @var{data-directory}."
             (data-directory data-directory)
             (extension-packages extension-packages))))
 
+(define-record-type* <postgresql-role>
+  postgresql-role make-postgresql-role
+  postgresql-role?
+  (name             postgresql-role-name) ;string
+  (permissions      postgresql-role-permissions
+                    (default '(createdb login))) ;list
+  (create-database? postgresql-role-create-database?  ;boolean
+                    (default #f)))
+
+(define-record-type* <postgresql-role-configuration>
+  postgresql-role-configuration make-postgresql-role-configuration
+  postgresql-role-configuration?
+  (host             postgresql-role-configuration-host ;string
+                    (default "/var/run/postgresql"))
+  (log              postgresql-role-configuration-log ;string
+                    (default "/var/log/postgresql_roles.log"))
+  (roles            postgresql-role-configuration-roles
+                    (default '()))) ;list
+
+(define (postgresql-create-roles config)
+  ;; See: https://www.postgresql.org/docs/current/sql-createrole.html for the
+  ;; complete permissions list.
+  (define (format-permissions permissions)
+    (let ((dict '((createdb . "CREATEDB")
+                  (login    . "LOGIN"))))
+      (string-join (map (lambda (permission)
+                          (assq-ref dict permission))
+                        permissions)
+                   " ")))
+
+  (define (roles->queries roles)
+    (apply mixed-text-file "queries"
+           (append-map (lambda (role)
+                         (match-record role <postgresql-role>
+                           (name permissions create-database?)
+                           `("CREATE ROLE " ,name
+                             " WITH " ,(format-permissions permissions)
+                             ";\n"
+                             ,@(if create-database?
+                                   `("CREATE DATABASE " ,name
+                                     " OWNER " ,name ";\n")
+                                   '()))))
+                       roles)))
+
+  (let ((host (postgresql-role-configuration-host config))
+        (roles (postgresql-role-configuration-roles config)))
+    (program-file
+     "postgresql-create-roles"
+     #~(begin
+         (let ((psql #$(file-append postgresql "/bin/psql")))
+           (execl psql psql "-a"
+                  "-h" #$host
+                  "-f" #$(roles->queries roles)))))))
+
+(define (postgresql-role-shepherd-service config)
+  (match-record config <postgresql-role-configuration>
+    (log)
+    (list (shepherd-service
+           (requirement '(postgres))
+           (provision '(postgres-roles))
+           (one-shot? #t)
+           (start #~(make-forkexec-constructor
+                     (list #$(postgresql-create-roles config))
+                     #:user "postgres" #:group "postgres"
+                     #:log-file #$log))
+           (documentation "Create PostgreSQL roles.")))))
+
+(define postgresql-role-service-type
+  (service-type (name 'postgresql-role)
+                (extensions
+                 (list (service-extension shepherd-root-service-type
+                                          postgresql-role-shepherd-service)))
+                (compose concatenate)
+                (extend (lambda (config extended-roles)
+                          (match-record config <postgresql-role-configuration>
+                            (host roles)
+                            (postgresql-role-configuration
+                             (host host)
+                             (roles (append roles extended-roles))))))
+                (default-value (postgresql-role-configuration))
+                (description "Ensure the specified PostgreSQL roles are
+created after the PostgreSQL database is started.")))
+
 \f
 ;;;
 ;;; Memcached
diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm
index d881a8c3ee..e831d69f5a 100644
--- a/gnu/tests/databases.scm
+++ b/gnu/tests/databases.scm
@@ -217,6 +217,9 @@
 (define %postgresql-log-directory
   "/var/log/postgresql")
 
+(define %role-log-file
+  "/var/log/postgresql_roles.log")
+
 (define %postgresql-os
   (simple-operating-system
    (service postgresql-service-type
@@ -229,7 +232,13 @@
                   ("random_page_cost" 2)
                   ("auto_explain.log_min_duration" "100 ms")
                   ("work_mem" "500 MB")
-                  ("debug_print_plan" #t)))))))))
+                  ("debug_print_plan" #t)))))))
+   (service postgresql-role-service-type
+            (postgresql-role-configuration
+             (roles
+              (list (postgresql-role
+                     (name "root")
+                     (create-database? #t))))))))
 
 (define (run-postgresql-test)
   "Run tests in %POSTGRESQL-OS."
@@ -282,6 +291,39 @@
                   #t))
              marionette))
 
+          (test-assert "database ready"
+            (begin
+              (marionette-eval
+               '(begin
+                  (let loop ((i 10))
+                    (unless (or (zero? i)
+                                (and (file-exists? #$%role-log-file)
+                                     (string-contains
+                                      (call-with-input-file #$%role-log-file
+                                        get-string-all)
+                                      ";\nCREATE DATABASE")))
+                      (sleep 1)
+                      (loop (- i 1)))))
+               marionette)))
+
+          (test-assert "database creation"
+            (marionette-eval
+             '(begin
+                (use-modules (gnu services herd)
+                             (ice-9 popen))
+                (current-output-port
+                 (open-file "/dev/console" "w0"))
+                (let* ((port (open-pipe*
+                              OPEN_READ
+                              #$(file-append postgresql "/bin/psql")
+                              "-tAh" "/var/run/postgresql"
+                              "-c" "SELECT 1 FROM pg_database WHERE
+ datname='root'"))
+                       (output (get-string-all port)))
+                  (close-pipe port)
+                  (string-contains output "1")))
+             marionette))
+
           (test-end)
           (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
 
-- 
2.29.2





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-01-18 10:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-14 13:36 [bug#45860] Improve PostgreSQL service Mathieu Othacehe
2021-01-14 21:56 ` Christopher Baines
2021-01-15  8:56   ` Mathieu Othacehe
2021-01-16 11:44     ` Christopher Baines
2021-01-18 10:16 ` [bug#45860] [PATCH v2 0/5] services: postgresql: Improve service Mathieu Othacehe
2021-01-18 10:16   ` [bug#45860] [PATCH v2 1/5] services: postgresql: Use Guile datatypes Mathieu Othacehe
2021-01-18 10:16   ` [bug#45860] [PATCH v2 2/5] services: postgresql: Add socket directory support Mathieu Othacehe
2021-01-18 10:16   ` [bug#45860] [PATCH v2 3/5] services: postgresql: Add log " Mathieu Othacehe
2021-01-18 10:16   ` [bug#45860] [PATCH v2 4/5] services: postgresql: Wrap long lines Mathieu Othacehe
2021-01-18 10:16   ` [bug#45860] [PATCH v2 5/5] services: postgresql: Add postgresql-role-service-type Mathieu Othacehe

all messages for Guix-related lists mirrored at yhetil.org

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/guix

Example config snippet for mirrors.


AGPL code for this site: git clone http://ou63pmih66umazou.onion/public-inbox.git