Package: guix-patches;
Reported by: Giacomo Leidi <goodoldpaul <at> autistici.org>
Date: Thu, 12 Sep 2024 11:26:01 UTC
Severity: normal
Tags: patch
Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 73196 in the body.
You can then email your comments to 73196 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
pelzflorian <at> pelzflorian.de, ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Thu, 12 Sep 2024 11:26:01 GMT) Full text and rfc822 format available.Giacomo Leidi <goodoldpaul <at> autistici.org>
:pelzflorian <at> pelzflorian.de, ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org
.
(Thu, 12 Sep 2024 11:26:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Giacomo Leidi <goodoldpaul <at> autistici.org> To: guix-patches <at> gnu.org Cc: Giacomo Leidi <goodoldpaul <at> autistici.org> Subject: [PATCH] services: postgresql-role: Add support for password files. Date: Thu, 12 Sep 2024 13:24:23 +0200
This commit adds a password-file to the postgresql-role field. It allows users to provision Postgres roles with a set password. * gnu/services/databases.scm (postgresql-role): Add password-file field; (postgresql-role-configuration): add requirement field; (postgresql-create-roles): add support for setting passwords from a file without leaking passwords to the command line; (postgresql-role-shepherd-service): add support for customizable requirements. * gnu/tests/databases.scm: Test it. * doc/guix.texi: Document it. Change-Id: I3aabaa10b0c5e826c5aa874e5649e25a3508a585 --- doc/guix.texi | 15 +++++++++--- gnu/services/databases.scm | 47 +++++++++++++++++++++++++++++++++----- gnu/tests/databases.scm | 38 +++++++++++++++++++++++++++--- 3 files changed, 88 insertions(+), 12 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index 981ffb8c58..8e6f1b8b2a 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -26294,9 +26294,10 @@ Database Services @lisp (service-extension postgresql-role-service-type - (const (postgresql-role - (name "alice") - (create-database? #t)))) + (const (list + (postgresql-role + (name "alice") + (create-database? #t))))) @end lisp @end defvar @@ -26319,6 +26320,10 @@ Database Services @item @code{create-database?} (default: @code{#f}) whether to create a database with the same name as the role. +@item @code{password-file} (default: @code{#f}) +A string representing the path of a file that contains the password to be set +for the role. + @item @code{encoding} (default: @code{"UTF8"}) The character set to use for storing text in the database. @@ -26347,6 +26352,10 @@ Database Services @item @code{log} (default: @code{"/var/log/postgresql_roles.log"}) File name of the log file. +@item @code{requirement} (default: @code{'()}) (type: list-of-symbols) +Set additional Shepherd services dependencies to the provisioned +Shepherd service. + @item @code{roles} (default: @code{'()}) The initial PostgreSQL roles to create. @end table diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm index fa332d7978..d23dba60e3 100644 --- a/gnu/services/databases.scm +++ b/gnu/services/databases.scm @@ -9,6 +9,7 @@ ;;; Copyright © 2020, 2022 Marius Bakke <marius <at> gnu.org> ;;; Copyright © 2021 David Larsson <david.larsson <at> selfhosted.xyz> ;;; Copyright © 2021 Aljosha Papsch <ep <at> stern-data.com> +;;; Copyright © 2024 Giacomo Leidi <goodoldpaul <at> autistici.org> ;;; ;;; This file is part of GNU Guix. ;;; @@ -31,6 +32,7 @@ (define-module (gnu services databases) #:use-module (gnu system shadow) #:use-module (gnu packages admin) #:use-module (gnu packages base) + #:use-module (gnu packages bash) #:use-module (gnu packages databases) #:use-module (guix build-system trivial) #:use-module (guix build union) @@ -65,11 +67,13 @@ (define-module (gnu services databases) postgresql-role postgresql-role? postgresql-role-name + postgresql-role-password-file postgresql-role-permissions postgresql-role-create-database? postgresql-role-configuration postgresql-role-configuration? postgresql-role-configuration-host + postgresql-role-configuration-requirement postgresql-role-configuration-roles postgresql-role-service-type @@ -372,6 +376,8 @@ (define-record-type* <postgresql-role> postgresql-role make-postgresql-role postgresql-role? (name postgresql-role-name) ;string + (password-file postgresql-role-password-file ;string + (default #f)) (permissions postgresql-role-permissions (default '(createdb login))) ;list (create-database? postgresql-role-create-database? ;boolean @@ -390,6 +396,8 @@ (define-record-type* <postgresql-role-configuration> postgresql-role-configuration? (host postgresql-role-configuration-host ;string (default "/var/run/postgresql")) + (requirement postgresql-role-configuration-requirement ;list-of-symbols + (default '())) (log postgresql-role-configuration-log ;string (default "/var/log/postgresql_roles.log")) (roles postgresql-role-configuration-roles @@ -407,19 +415,36 @@ (define (postgresql-create-roles config) permissions) " "))) + (define (password-value role) + (string-append "password_" (postgresql-role-name role))) + + (define (role->password-variable role) + (define file-name + (postgresql-role-password-file role)) + (if (string? file-name) + ;; This way passwords do not leak to the command line + (string-append "-v \"" (password-value role) + "=$(cat " file-name ")\"") + "")) + (define (roles->queries roles) (apply mixed-text-file "queries" (append-map (lambda (role) (match-record role <postgresql-role> (name permissions create-database? encoding collation ctype - template) + template password-file) `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ rolname = '" ,name "')) as not_exists;\n" "\\gset\n" "\\if :not_exists\n" "CREATE ROLE \"" ,name "\"" " WITH " ,(format-permissions permissions) +,(if (and (string? password-file) + (not (string-null? password-file))) + (string-append + "\nPASSWORD :'" (password-value role) "'") + "") ";\n" ,@(if create-database? `("CREATE DATABASE \"" ,name "\"" @@ -434,20 +459,30 @@ (define (postgresql-create-roles config) (let ((host (postgresql-role-configuration-host config)) (roles (postgresql-role-configuration-roles config))) - #~(let ((psql #$(file-append postgresql "/bin/psql"))) - (list psql "-a" "-h" #$host "-f" #$(roles->queries roles))))) + (program-file "run-queries" + #~(let ((bash #$(file-append bash-minimal "/bin/bash")) + (psql #$(file-append postgresql "/bin/psql"))) + (define command + (string-append + "set -e; exec " psql " -c -a -h " #$host " -f " + #$(roles->queries roles) " " + (string-join + (list + #$@(map role->password-variable roles)) + " "))) + (execlp bash bash "-c" command))))) (define (postgresql-role-shepherd-service config) (match-record config <postgresql-role-configuration> - (log) + (log requirement) (list (shepherd-service - (requirement '(postgres)) + (requirement `(postgres ,@requirement)) (provision '(postgres-roles)) (one-shot? #t) (start #~(lambda args (let ((pid (fork+exec-command - #$(postgresql-create-roles config) + (list #$(postgresql-create-roles config)) #:user "postgres" #:group "postgres" #:log-file #$log))) diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm index 7c8b87942f..81484b2954 100644 --- a/gnu/tests/databases.scm +++ b/gnu/tests/databases.scm @@ -142,6 +142,8 @@ (define %role-log-file (define %postgresql-os (simple-operating-system + (extra-special-file "/password" + (plain-file "password" "hello")) (service postgresql-service-type (postgresql-configuration (postgresql postgresql) @@ -158,6 +160,10 @@ (define %postgresql-os (roles (list (postgresql-role (name "root") + (create-database? #t)) + (postgresql-role + (name "alice") + (password-file "/password") (create-database? #t)))))))) (define (run-postgresql-test) @@ -230,14 +236,40 @@ (define (run-postgresql-test) (marionette-eval '(begin (use-modules (gnu services herd) + (srfi srfi-1) (ice-9 popen)) (current-output-port (open-file "/dev/console" "w0")) + (every + (lambda (role) + (let* ((port (open-pipe* + OPEN_READ + #$(file-append postgresql "/bin/psql") + "-tA" "-c" + (string-append + "SELECT 1 FROM pg_database WHERE" + " datname='" role "'"))) + (output (get-string-all port))) + (close-pipe port) + (string-contains output "1"))) + '("root" "alice"))) + marionette)) + + (test-assert "database passwords are set" + (marionette-eval + '(begin + (use-modules (gnu services herd) + (ice-9 match) + (ice-9 popen)) + (current-output-port + (open-file "/dev/console" "w0")) + (setgid (passwd:gid (getpwnam "alice"))) + (setuid (passwd:uid (getpw "alice"))) + (setenv "PGPASSWORD" "hello") (let* ((port (open-pipe* OPEN_READ - #$(file-append postgresql "/bin/psql") - "-tA" "-c" "SELECT 1 FROM pg_database WHERE - datname='root'")) + #$(file-append postgresql "/bin/psql") "-tA" "-c" + "SELECT 1 FROM pg_database WHERE datname='alice'")) (output (get-string-all port))) (close-pipe port) (string-contains output "1"))) base-commit: 590904cca15922e6474fbd3a71af9b3a45b268af -- 2.46.0
guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Thu, 12 Sep 2024 12:19:01 GMT) Full text and rfc822 format available.Message #8 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: paul <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Subject: Re: [PATCH] services: postgresql-role: Add support for password files. Date: Thu, 12 Sep 2024 14:18:16 +0200
Hi Guix, I'm sending a v2 with a small fix in the Guix system service extension logic. Thank you for all your work, giacomo
pelzflorian <at> pelzflorian.de, ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Thu, 12 Sep 2024 12:19:02 GMT) Full text and rfc822 format available.Message #11 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: Giacomo Leidi <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Cc: Giacomo Leidi <goodoldpaul <at> autistici.org> Subject: [PATCH v2] services: postgresql-role: Add support for password files. Date: Thu, 12 Sep 2024 14:18:28 +0200
This commit adds a password-file to the postgresql-role field. It allows users to provision Postgres roles with a set password. * gnu/services/databases.scm (postgresql-role): Add password-file field; (postgresql-role-configuration): add requirement field; (postgresql-create-roles): add support for setting passwords from a file without leaking passwords to the command line; (postgresql-role-shepherd-service): add support for customizable requirements. * gnu/tests/databases.scm: Test it. * doc/guix.texi: Document it. Change-Id: I3aabaa10b0c5e826c5aa874e5649e25a3508a585 --- doc/guix.texi | 15 ++++++++--- gnu/services/databases.scm | 51 ++++++++++++++++++++++++++++++++------ gnu/tests/databases.scm | 38 +++++++++++++++++++++++++--- 3 files changed, 90 insertions(+), 14 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index 981ffb8c58..8e6f1b8b2a 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -26294,9 +26294,10 @@ Database Services @lisp (service-extension postgresql-role-service-type - (const (postgresql-role - (name "alice") - (create-database? #t)))) + (const (list + (postgresql-role + (name "alice") + (create-database? #t))))) @end lisp @end defvar @@ -26319,6 +26320,10 @@ Database Services @item @code{create-database?} (default: @code{#f}) whether to create a database with the same name as the role. +@item @code{password-file} (default: @code{#f}) +A string representing the path of a file that contains the password to be set +for the role. + @item @code{encoding} (default: @code{"UTF8"}) The character set to use for storing text in the database. @@ -26347,6 +26352,10 @@ Database Services @item @code{log} (default: @code{"/var/log/postgresql_roles.log"}) File name of the log file. +@item @code{requirement} (default: @code{'()}) (type: list-of-symbols) +Set additional Shepherd services dependencies to the provisioned +Shepherd service. + @item @code{roles} (default: @code{'()}) The initial PostgreSQL roles to create. @end table diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm index fa332d7978..d77988d8c5 100644 --- a/gnu/services/databases.scm +++ b/gnu/services/databases.scm @@ -9,6 +9,7 @@ ;;; Copyright © 2020, 2022 Marius Bakke <marius <at> gnu.org> ;;; Copyright © 2021 David Larsson <david.larsson <at> selfhosted.xyz> ;;; Copyright © 2021 Aljosha Papsch <ep <at> stern-data.com> +;;; Copyright © 2024 Giacomo Leidi <goodoldpaul <at> autistici.org> ;;; ;;; This file is part of GNU Guix. ;;; @@ -31,6 +32,7 @@ (define-module (gnu services databases) #:use-module (gnu system shadow) #:use-module (gnu packages admin) #:use-module (gnu packages base) + #:use-module (gnu packages bash) #:use-module (gnu packages databases) #:use-module (guix build-system trivial) #:use-module (guix build union) @@ -65,11 +67,13 @@ (define-module (gnu services databases) postgresql-role postgresql-role? postgresql-role-name + postgresql-role-password-file postgresql-role-permissions postgresql-role-create-database? postgresql-role-configuration postgresql-role-configuration? postgresql-role-configuration-host + postgresql-role-configuration-requirement postgresql-role-configuration-roles postgresql-role-service-type @@ -372,6 +376,8 @@ (define-record-type* <postgresql-role> postgresql-role make-postgresql-role postgresql-role? (name postgresql-role-name) ;string + (password-file postgresql-role-password-file ;string + (default #f)) (permissions postgresql-role-permissions (default '(createdb login))) ;list (create-database? postgresql-role-create-database? ;boolean @@ -390,6 +396,8 @@ (define-record-type* <postgresql-role-configuration> postgresql-role-configuration? (host postgresql-role-configuration-host ;string (default "/var/run/postgresql")) + (requirement postgresql-role-configuration-requirement ;list-of-symbols + (default '())) (log postgresql-role-configuration-log ;string (default "/var/log/postgresql_roles.log")) (roles postgresql-role-configuration-roles @@ -407,19 +415,36 @@ (define (postgresql-create-roles config) permissions) " "))) + (define (password-value role) + (string-append "password_" (postgresql-role-name role))) + + (define (role->password-variable role) + (define file-name + (postgresql-role-password-file role)) + (if (string? file-name) + ;; This way passwords do not leak to the command line + (string-append "-v \"" (password-value role) + "=$(cat " file-name ")\"") + "")) + (define (roles->queries roles) (apply mixed-text-file "queries" (append-map (lambda (role) (match-record role <postgresql-role> (name permissions create-database? encoding collation ctype - template) + template password-file) `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ rolname = '" ,name "')) as not_exists;\n" "\\gset\n" "\\if :not_exists\n" "CREATE ROLE \"" ,name "\"" " WITH " ,(format-permissions permissions) +,(if (and (string? password-file) + (not (string-null? password-file))) + (string-append + "\nPASSWORD :'" (password-value role) "'") + "") ";\n" ,@(if create-database? `("CREATE DATABASE \"" ,name "\"" @@ -434,20 +459,30 @@ (define (postgresql-create-roles config) (let ((host (postgresql-role-configuration-host config)) (roles (postgresql-role-configuration-roles config))) - #~(let ((psql #$(file-append postgresql "/bin/psql"))) - (list psql "-a" "-h" #$host "-f" #$(roles->queries roles))))) + (program-file "run-queries" + #~(let ((bash #$(file-append bash-minimal "/bin/bash")) + (psql #$(file-append postgresql "/bin/psql"))) + (define command + (string-append + "set -e; exec " psql " -c -a -h " #$host " -f " + #$(roles->queries roles) " " + (string-join + (list + #$@(map role->password-variable roles)) + " "))) + (execlp bash bash "-c" command))))) (define (postgresql-role-shepherd-service config) (match-record config <postgresql-role-configuration> - (log) + (log requirement) (list (shepherd-service - (requirement '(postgres)) + (requirement `(postgres ,@requirement)) (provision '(postgres-roles)) (one-shot? #t) (start #~(lambda args (let ((pid (fork+exec-command - #$(postgresql-create-roles config) + (list #$(postgresql-create-roles config)) #:user "postgres" #:group "postgres" #:log-file #$log))) @@ -462,9 +497,9 @@ (define postgresql-role-service-type (compose concatenate) (extend (lambda (config extended-roles) (match-record config <postgresql-role-configuration> - (host roles) + (roles) (postgresql-role-configuration - (host host) + (inherit config) (roles (append roles extended-roles)))))) (default-value (postgresql-role-configuration)) (description "Ensure the specified PostgreSQL roles are diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm index 7c8b87942f..81484b2954 100644 --- a/gnu/tests/databases.scm +++ b/gnu/tests/databases.scm @@ -142,6 +142,8 @@ (define %role-log-file (define %postgresql-os (simple-operating-system + (extra-special-file "/password" + (plain-file "password" "hello")) (service postgresql-service-type (postgresql-configuration (postgresql postgresql) @@ -158,6 +160,10 @@ (define %postgresql-os (roles (list (postgresql-role (name "root") + (create-database? #t)) + (postgresql-role + (name "alice") + (password-file "/password") (create-database? #t)))))))) (define (run-postgresql-test) @@ -230,14 +236,40 @@ (define (run-postgresql-test) (marionette-eval '(begin (use-modules (gnu services herd) + (srfi srfi-1) (ice-9 popen)) (current-output-port (open-file "/dev/console" "w0")) + (every + (lambda (role) + (let* ((port (open-pipe* + OPEN_READ + #$(file-append postgresql "/bin/psql") + "-tA" "-c" + (string-append + "SELECT 1 FROM pg_database WHERE" + " datname='" role "'"))) + (output (get-string-all port))) + (close-pipe port) + (string-contains output "1"))) + '("root" "alice"))) + marionette)) + + (test-assert "database passwords are set" + (marionette-eval + '(begin + (use-modules (gnu services herd) + (ice-9 match) + (ice-9 popen)) + (current-output-port + (open-file "/dev/console" "w0")) + (setgid (passwd:gid (getpwnam "alice"))) + (setuid (passwd:uid (getpw "alice"))) + (setenv "PGPASSWORD" "hello") (let* ((port (open-pipe* OPEN_READ - #$(file-append postgresql "/bin/psql") - "-tA" "-c" "SELECT 1 FROM pg_database WHERE - datname='root'")) + #$(file-append postgresql "/bin/psql") "-tA" "-c" + "SELECT 1 FROM pg_database WHERE datname='alice'")) (output (get-string-all port))) (close-pipe port) (string-contains output "1"))) base-commit: 590904cca15922e6474fbd3a71af9b3a45b268af -- 2.46.0
guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Sun, 20 Oct 2024 23:01:01 GMT) Full text and rfc822 format available.Message #14 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: paul <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Subject: Re: [PATCH] services: postgresql-role: Add support for password files. Date: Mon, 21 Oct 2024 00:59:44 +0200
Hi Guix , this is a friendly ping. I'm sending a patchset rebased on current master. Thank you for your work, giacomo
ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Sun, 20 Oct 2024 23:01:02 GMT) Full text and rfc822 format available.Message #17 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: Giacomo Leidi <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Cc: Giacomo Leidi <goodoldpaul <at> autistici.org> Subject: [PATCH v2] services: postgresql-role: Add support for password files. Date: Mon, 21 Oct 2024 00:59:47 +0200
This commit adds a password-file to the postgresql-role field. It allows users to provision Postgres roles with a set password. * gnu/services/databases.scm (postgresql-role): Add password-file field; (postgresql-role-configuration): add requirement field; (postgresql-create-roles): add support for setting passwords from a file without leaking passwords to the command line; (postgresql-role-shepherd-service): add support for customizable requirements. * gnu/tests/databases.scm: Test it. * doc/guix.texi: Document it. Change-Id: I3aabaa10b0c5e826c5aa874e5649e25a3508a585 --- doc/guix.texi | 15 ++++++++--- gnu/services/databases.scm | 51 ++++++++++++++++++++++++++++++++------ gnu/tests/databases.scm | 38 +++++++++++++++++++++++++--- 3 files changed, 90 insertions(+), 14 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index ac3a7adef0..71c717e161 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -26379,9 +26379,10 @@ Database Services @lisp (service-extension postgresql-role-service-type - (const (postgresql-role - (name "alice") - (create-database? #t)))) + (const (list + (postgresql-role + (name "alice") + (create-database? #t))))) @end lisp @end defvar @@ -26404,6 +26405,10 @@ Database Services @item @code{create-database?} (default: @code{#f}) whether to create a database with the same name as the role. +@item @code{password-file} (default: @code{#f}) +A string representing the path of a file that contains the password to be set +for the role. + @item @code{encoding} (default: @code{"UTF8"}) The character set to use for storing text in the database. @@ -26432,6 +26437,10 @@ Database Services @item @code{log} (default: @code{"/var/log/postgresql_roles.log"}) File name of the log file. +@item @code{requirement} (default: @code{'()}) (type: list-of-symbols) +Set additional Shepherd services dependencies to the provisioned +Shepherd service. + @item @code{roles} (default: @code{'()}) The initial PostgreSQL roles to create. @end table diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm index fa332d7978..d77988d8c5 100644 --- a/gnu/services/databases.scm +++ b/gnu/services/databases.scm @@ -9,6 +9,7 @@ ;;; Copyright © 2020, 2022 Marius Bakke <marius <at> gnu.org> ;;; Copyright © 2021 David Larsson <david.larsson <at> selfhosted.xyz> ;;; Copyright © 2021 Aljosha Papsch <ep <at> stern-data.com> +;;; Copyright © 2024 Giacomo Leidi <goodoldpaul <at> autistici.org> ;;; ;;; This file is part of GNU Guix. ;;; @@ -31,6 +32,7 @@ (define-module (gnu services databases) #:use-module (gnu system shadow) #:use-module (gnu packages admin) #:use-module (gnu packages base) + #:use-module (gnu packages bash) #:use-module (gnu packages databases) #:use-module (guix build-system trivial) #:use-module (guix build union) @@ -65,11 +67,13 @@ (define-module (gnu services databases) postgresql-role postgresql-role? postgresql-role-name + postgresql-role-password-file postgresql-role-permissions postgresql-role-create-database? postgresql-role-configuration postgresql-role-configuration? postgresql-role-configuration-host + postgresql-role-configuration-requirement postgresql-role-configuration-roles postgresql-role-service-type @@ -372,6 +376,8 @@ (define-record-type* <postgresql-role> postgresql-role make-postgresql-role postgresql-role? (name postgresql-role-name) ;string + (password-file postgresql-role-password-file ;string + (default #f)) (permissions postgresql-role-permissions (default '(createdb login))) ;list (create-database? postgresql-role-create-database? ;boolean @@ -390,6 +396,8 @@ (define-record-type* <postgresql-role-configuration> postgresql-role-configuration? (host postgresql-role-configuration-host ;string (default "/var/run/postgresql")) + (requirement postgresql-role-configuration-requirement ;list-of-symbols + (default '())) (log postgresql-role-configuration-log ;string (default "/var/log/postgresql_roles.log")) (roles postgresql-role-configuration-roles @@ -407,19 +415,36 @@ (define (postgresql-create-roles config) permissions) " "))) + (define (password-value role) + (string-append "password_" (postgresql-role-name role))) + + (define (role->password-variable role) + (define file-name + (postgresql-role-password-file role)) + (if (string? file-name) + ;; This way passwords do not leak to the command line + (string-append "-v \"" (password-value role) + "=$(cat " file-name ")\"") + "")) + (define (roles->queries roles) (apply mixed-text-file "queries" (append-map (lambda (role) (match-record role <postgresql-role> (name permissions create-database? encoding collation ctype - template) + template password-file) `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ rolname = '" ,name "')) as not_exists;\n" "\\gset\n" "\\if :not_exists\n" "CREATE ROLE \"" ,name "\"" " WITH " ,(format-permissions permissions) +,(if (and (string? password-file) + (not (string-null? password-file))) + (string-append + "\nPASSWORD :'" (password-value role) "'") + "") ";\n" ,@(if create-database? `("CREATE DATABASE \"" ,name "\"" @@ -434,20 +459,30 @@ (define (postgresql-create-roles config) (let ((host (postgresql-role-configuration-host config)) (roles (postgresql-role-configuration-roles config))) - #~(let ((psql #$(file-append postgresql "/bin/psql"))) - (list psql "-a" "-h" #$host "-f" #$(roles->queries roles))))) + (program-file "run-queries" + #~(let ((bash #$(file-append bash-minimal "/bin/bash")) + (psql #$(file-append postgresql "/bin/psql"))) + (define command + (string-append + "set -e; exec " psql " -c -a -h " #$host " -f " + #$(roles->queries roles) " " + (string-join + (list + #$@(map role->password-variable roles)) + " "))) + (execlp bash bash "-c" command))))) (define (postgresql-role-shepherd-service config) (match-record config <postgresql-role-configuration> - (log) + (log requirement) (list (shepherd-service - (requirement '(postgres)) + (requirement `(postgres ,@requirement)) (provision '(postgres-roles)) (one-shot? #t) (start #~(lambda args (let ((pid (fork+exec-command - #$(postgresql-create-roles config) + (list #$(postgresql-create-roles config)) #:user "postgres" #:group "postgres" #:log-file #$log))) @@ -462,9 +497,9 @@ (define postgresql-role-service-type (compose concatenate) (extend (lambda (config extended-roles) (match-record config <postgresql-role-configuration> - (host roles) + (roles) (postgresql-role-configuration - (host host) + (inherit config) (roles (append roles extended-roles)))))) (default-value (postgresql-role-configuration)) (description "Ensure the specified PostgreSQL roles are diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm index 7c8b87942f..81484b2954 100644 --- a/gnu/tests/databases.scm +++ b/gnu/tests/databases.scm @@ -142,6 +142,8 @@ (define %role-log-file (define %postgresql-os (simple-operating-system + (extra-special-file "/password" + (plain-file "password" "hello")) (service postgresql-service-type (postgresql-configuration (postgresql postgresql) @@ -158,6 +160,10 @@ (define %postgresql-os (roles (list (postgresql-role (name "root") + (create-database? #t)) + (postgresql-role + (name "alice") + (password-file "/password") (create-database? #t)))))))) (define (run-postgresql-test) @@ -230,14 +236,40 @@ (define (run-postgresql-test) (marionette-eval '(begin (use-modules (gnu services herd) + (srfi srfi-1) (ice-9 popen)) (current-output-port (open-file "/dev/console" "w0")) + (every + (lambda (role) + (let* ((port (open-pipe* + OPEN_READ + #$(file-append postgresql "/bin/psql") + "-tA" "-c" + (string-append + "SELECT 1 FROM pg_database WHERE" + " datname='" role "'"))) + (output (get-string-all port))) + (close-pipe port) + (string-contains output "1"))) + '("root" "alice"))) + marionette)) + + (test-assert "database passwords are set" + (marionette-eval + '(begin + (use-modules (gnu services herd) + (ice-9 match) + (ice-9 popen)) + (current-output-port + (open-file "/dev/console" "w0")) + (setgid (passwd:gid (getpwnam "alice"))) + (setuid (passwd:uid (getpw "alice"))) + (setenv "PGPASSWORD" "hello") (let* ((port (open-pipe* OPEN_READ - #$(file-append postgresql "/bin/psql") - "-tA" "-c" "SELECT 1 FROM pg_database WHERE - datname='root'")) + #$(file-append postgresql "/bin/psql") "-tA" "-c" + "SELECT 1 FROM pg_database WHERE datname='alice'")) (output (get-string-all port))) (close-pipe port) (string-contains output "1"))) base-commit: 5ab3c4c1e43ebb637551223791db0ea3519986e1 -- 2.46.0
guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Thu, 03 Apr 2025 18:38:02 GMT) Full text and rfc822 format available.Message #20 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: paul <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Subject: Re: [PATCH] services: postgresql-role: Add support for password files. Date: Thu, 3 Apr 2025 20:37:18 +0200
Hi I'm sending a an updated patch rebased on current master. cheers, giacomo
guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Thu, 03 Apr 2025 19:28:02 GMT) Full text and rfc822 format available.Message #23 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: Giacomo Leidi <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Cc: Giacomo Leidi <goodoldpaul <at> autistici.org> Subject: [PATCH v3 2/2] as Date: Thu, 3 Apr 2025 21:26:52 +0200
Change-Id: Ia7ea0c1bc75189beb20e3d41f0d861cbd7c22b62 --- gnu/services/databases.scm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm index 8c677a1ef66..b845327f89e 100644 --- a/gnu/services/databases.scm +++ b/gnu/services/databases.scm @@ -501,9 +501,10 @@ (define postgresql-role-service-type (compose concatenate) (extend (lambda (config extended-roles) (match-record config <postgresql-role-configuration> - (roles) + (host roles) (postgresql-role-configuration (inherit config) + (host host) (roles (append roles extended-roles)))))) (default-value (postgresql-role-configuration)) (description "Ensure the specified PostgreSQL roles are -- 2.49.0
ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Thu, 03 Apr 2025 19:28:02 GMT) Full text and rfc822 format available.Message #26 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: Giacomo Leidi <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Cc: Giacomo Leidi <goodoldpaul <at> autistici.org> Subject: [PATCH v3 1/2] services: postgresql-role: Add support for password files. Date: Thu, 3 Apr 2025 21:26:51 +0200
This commit adds a password-file to the postgresql-role field. It allows users to provision Postgres roles with a set password. * gnu/services/databases.scm (postgresql-role): Add password-file field; (postgresql-role-configuration): add requirement field; (postgresql-create-roles): add support for setting passwords from a file without leaking passwords to the command line; (postgresql-role-shepherd-service): add support for customizable requirements. * gnu/tests/databases.scm: Test it. * doc/guix.texi: Document it. Change-Id: I3aabaa10b0c5e826c5aa874e5649e25a3508a585 --- doc/guix.texi | 15 ++++++++--- gnu/services/databases.scm | 51 ++++++++++++++++++++++++++++++++------ gnu/tests/databases.scm | 38 +++++++++++++++++++++++++--- 3 files changed, 90 insertions(+), 14 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index 3d6080dbaa5..eb0b94409c2 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -27586,9 +27586,10 @@ Database Services @lisp (service-extension postgresql-role-service-type - (const (postgresql-role - (name "alice") - (create-database? #t)))) + (const (list + (postgresql-role + (name "alice") + (create-database? #t))))) @end lisp @end defvar @@ -27611,6 +27612,10 @@ Database Services @item @code{create-database?} (default: @code{#f}) whether to create a database with the same name as the role. +@item @code{password-file} (default: @code{#f}) +A string representing the path of a file that contains the password to be set +for the role. + @item @code{encoding} (default: @code{"UTF8"}) The character set to use for storing text in the database. @@ -27639,6 +27644,10 @@ Database Services @item @code{log} (default: @code{"/var/log/postgresql_roles.log"}) File name of the log file. +@item @code{requirement} (default: @code{'()}) (type: list-of-symbols) +Set additional Shepherd services dependencies to the provisioned +Shepherd service. + @item @code{roles} (default: @code{'()}) The initial PostgreSQL roles to create. @end table diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm index 6d80376d90d..8c677a1ef66 100644 --- a/gnu/services/databases.scm +++ b/gnu/services/databases.scm @@ -9,6 +9,7 @@ ;;; Copyright © 2020, 2022 Marius Bakke <marius <at> gnu.org> ;;; Copyright © 2021 David Larsson <david.larsson <at> selfhosted.xyz> ;;; Copyright © 2021 Aljosha Papsch <ep <at> stern-data.com> +;;; Copyright © 2025 Giacomo Leidi <goodoldpaul <at> autistici.org> ;;; ;;; This file is part of GNU Guix. ;;; @@ -31,6 +32,7 @@ (define-module (gnu services databases) #:use-module (gnu system shadow) #:use-module (gnu packages admin) #:use-module (gnu packages base) + #:use-module (gnu packages bash) #:use-module (gnu packages databases) #:use-module (guix build-system trivial) #:use-module (guix build union) @@ -65,11 +67,13 @@ (define-module (gnu services databases) postgresql-role postgresql-role? postgresql-role-name + postgresql-role-password-file postgresql-role-permissions postgresql-role-create-database? postgresql-role-configuration postgresql-role-configuration? postgresql-role-configuration-host + postgresql-role-configuration-requirement postgresql-role-configuration-roles postgresql-role-service-type @@ -374,6 +378,8 @@ (define-record-type* <postgresql-role> postgresql-role make-postgresql-role postgresql-role? (name postgresql-role-name) ;string + (password-file postgresql-role-password-file ;string + (default #f)) (permissions postgresql-role-permissions (default '(createdb login))) ;list (create-database? postgresql-role-create-database? ;boolean @@ -392,6 +398,8 @@ (define-record-type* <postgresql-role-configuration> postgresql-role-configuration? (host postgresql-role-configuration-host ;string (default "/var/run/postgresql")) + (requirement postgresql-role-configuration-requirement ;list-of-symbols + (default '())) (log postgresql-role-configuration-log ;string (default "/var/log/postgresql_roles.log")) (roles postgresql-role-configuration-roles @@ -409,19 +417,36 @@ (define (postgresql-create-roles config) permissions) " "))) + (define (password-value role) + (string-append "password_" (postgresql-role-name role))) + + (define (role->password-variable role) + (define file-name + (postgresql-role-password-file role)) + (if (string? file-name) + ;; This way passwords do not leak to the command line + (string-append "-v \"" (password-value role) + "=$(cat " file-name ")\"") + "")) + (define (roles->queries roles) (apply mixed-text-file "queries" (append-map (lambda (role) (match-record role <postgresql-role> (name permissions create-database? encoding collation ctype - template) + template password-file) `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ rolname = '" ,name "')) as not_exists;\n" "\\gset\n" "\\if :not_exists\n" "CREATE ROLE \"" ,name "\"" " WITH " ,(format-permissions permissions) +,(if (and (string? password-file) + (not (string-null? password-file))) + (string-append + "\nPASSWORD :'" (password-value role) "'") + "") ";\n" ,@(if create-database? `("CREATE DATABASE \"" ,name "\"" @@ -436,20 +461,30 @@ (define (postgresql-create-roles config) (let ((host (postgresql-role-configuration-host config)) (roles (postgresql-role-configuration-roles config))) - #~(let ((psql #$(file-append postgresql "/bin/psql"))) - (list psql "-a" "-h" #$host "-f" #$(roles->queries roles))))) + (program-file "run-queries" + #~(let ((bash #$(file-append bash-minimal "/bin/bash")) + (psql #$(file-append postgresql "/bin/psql"))) + (define command + (string-append + "set -e; exec " psql " -c -a -h " #$host " -f " + #$(roles->queries roles) " " + (string-join + (list + #$@(map role->password-variable roles)) + " "))) + (execlp bash bash "-c" command))))) (define (postgresql-role-shepherd-service config) (match-record config <postgresql-role-configuration> - (log) + (log requirement) (list (shepherd-service - (requirement '(user-processes postgres)) + (requirement `(user-processes postgres ,@requirement)) (provision '(postgres-roles)) (one-shot? #t) (start #~(lambda args (zero? (spawn-command - #$(postgresql-create-roles config) + (list #$(postgresql-create-roles config)) #:user "postgres" #:group "postgres" ;; XXX: As of Shepherd 1.0.2, #:log-file is not @@ -466,9 +501,9 @@ (define postgresql-role-service-type (compose concatenate) (extend (lambda (config extended-roles) (match-record config <postgresql-role-configuration> - (host roles) + (roles) (postgresql-role-configuration - (host host) + (inherit config) (roles (append roles extended-roles)))))) (default-value (postgresql-role-configuration)) (description "Ensure the specified PostgreSQL roles are diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm index fd5041344b6..c5da603565d 100644 --- a/gnu/tests/databases.scm +++ b/gnu/tests/databases.scm @@ -142,6 +142,8 @@ (define %role-log-file (define %postgresql-os (simple-operating-system + (extra-special-file "/password" + (plain-file "password" "hello")) (service postgresql-service-type (postgresql-configuration (postgresql postgresql) @@ -158,6 +160,10 @@ (define %postgresql-os (roles (list (postgresql-role (name "root") + (create-database? #t)) + (postgresql-role + (name "alice") + (password-file "/password") (create-database? #t)))))))) (define (run-postgresql-test) @@ -230,14 +236,40 @@ (define (run-postgresql-test) (marionette-eval '(begin (use-modules (gnu services herd) + (srfi srfi-1) (ice-9 popen)) (current-output-port (open-file "/dev/console" "w0")) + (every + (lambda (role) + (let* ((port (open-pipe* + OPEN_READ + #$(file-append postgresql "/bin/psql") + "-tA" "-c" + (string-append + "SELECT 1 FROM pg_database WHERE" + " datname='" role "'"))) + (output (get-string-all port))) + (close-pipe port) + (string-contains output "1"))) + '("root" "alice"))) + marionette)) + + (test-assert "database passwords are set" + (marionette-eval + '(begin + (use-modules (gnu services herd) + (ice-9 match) + (ice-9 popen)) + (current-output-port + (open-file "/dev/console" "w0")) + (setgid (passwd:gid (getpwnam "alice"))) + (setuid (passwd:uid (getpw "alice"))) + (setenv "PGPASSWORD" "hello") (let* ((port (open-pipe* OPEN_READ - #$(file-append postgresql "/bin/psql") - "-tA" "-c" "SELECT 1 FROM pg_database WHERE - datname='root'")) + #$(file-append postgresql "/bin/psql") "-tA" "-c" + "SELECT 1 FROM pg_database WHERE datname='alice'")) (output (get-string-all port))) (close-pipe port) (string-contains output "1"))) base-commit: 6a2a78fde19683f07c8b10f492cda67447bc99eb -- 2.49.0
ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Thu, 03 Apr 2025 19:29:02 GMT) Full text and rfc822 format available.Message #29 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: Giacomo Leidi <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Cc: Giacomo Leidi <goodoldpaul <at> autistici.org> Subject: [PATCH v4] services: postgresql-role: Add support for password files. Date: Thu, 3 Apr 2025 21:28:01 +0200
This commit adds a password-file to the postgresql-role field. It allows users to provision Postgres roles with a set password. * gnu/services/databases.scm (postgresql-role): Add password-file field; (postgresql-role-configuration): add requirement field; (postgresql-create-roles): add support for setting passwords from a file without leaking passwords to the command line; (postgresql-role-shepherd-service): add support for customizable requirements. * gnu/tests/databases.scm: Test it. * doc/guix.texi: Document it. Change-Id: I3aabaa10b0c5e826c5aa874e5649e25a3508a585 --- doc/guix.texi | 15 +++++++++--- gnu/services/databases.scm | 48 +++++++++++++++++++++++++++++++++----- gnu/tests/databases.scm | 38 +++++++++++++++++++++++++++--- 3 files changed, 89 insertions(+), 12 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index 3d6080dbaa5..eb0b94409c2 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -27586,9 +27586,10 @@ Database Services @lisp (service-extension postgresql-role-service-type - (const (postgresql-role - (name "alice") - (create-database? #t)))) + (const (list + (postgresql-role + (name "alice") + (create-database? #t))))) @end lisp @end defvar @@ -27611,6 +27612,10 @@ Database Services @item @code{create-database?} (default: @code{#f}) whether to create a database with the same name as the role. +@item @code{password-file} (default: @code{#f}) +A string representing the path of a file that contains the password to be set +for the role. + @item @code{encoding} (default: @code{"UTF8"}) The character set to use for storing text in the database. @@ -27639,6 +27644,10 @@ Database Services @item @code{log} (default: @code{"/var/log/postgresql_roles.log"}) File name of the log file. +@item @code{requirement} (default: @code{'()}) (type: list-of-symbols) +Set additional Shepherd services dependencies to the provisioned +Shepherd service. + @item @code{roles} (default: @code{'()}) The initial PostgreSQL roles to create. @end table diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm index 6d80376d90d..b845327f89e 100644 --- a/gnu/services/databases.scm +++ b/gnu/services/databases.scm @@ -9,6 +9,7 @@ ;;; Copyright © 2020, 2022 Marius Bakke <marius <at> gnu.org> ;;; Copyright © 2021 David Larsson <david.larsson <at> selfhosted.xyz> ;;; Copyright © 2021 Aljosha Papsch <ep <at> stern-data.com> +;;; Copyright © 2025 Giacomo Leidi <goodoldpaul <at> autistici.org> ;;; ;;; This file is part of GNU Guix. ;;; @@ -31,6 +32,7 @@ (define-module (gnu services databases) #:use-module (gnu system shadow) #:use-module (gnu packages admin) #:use-module (gnu packages base) + #:use-module (gnu packages bash) #:use-module (gnu packages databases) #:use-module (guix build-system trivial) #:use-module (guix build union) @@ -65,11 +67,13 @@ (define-module (gnu services databases) postgresql-role postgresql-role? postgresql-role-name + postgresql-role-password-file postgresql-role-permissions postgresql-role-create-database? postgresql-role-configuration postgresql-role-configuration? postgresql-role-configuration-host + postgresql-role-configuration-requirement postgresql-role-configuration-roles postgresql-role-service-type @@ -374,6 +378,8 @@ (define-record-type* <postgresql-role> postgresql-role make-postgresql-role postgresql-role? (name postgresql-role-name) ;string + (password-file postgresql-role-password-file ;string + (default #f)) (permissions postgresql-role-permissions (default '(createdb login))) ;list (create-database? postgresql-role-create-database? ;boolean @@ -392,6 +398,8 @@ (define-record-type* <postgresql-role-configuration> postgresql-role-configuration? (host postgresql-role-configuration-host ;string (default "/var/run/postgresql")) + (requirement postgresql-role-configuration-requirement ;list-of-symbols + (default '())) (log postgresql-role-configuration-log ;string (default "/var/log/postgresql_roles.log")) (roles postgresql-role-configuration-roles @@ -409,19 +417,36 @@ (define (postgresql-create-roles config) permissions) " "))) + (define (password-value role) + (string-append "password_" (postgresql-role-name role))) + + (define (role->password-variable role) + (define file-name + (postgresql-role-password-file role)) + (if (string? file-name) + ;; This way passwords do not leak to the command line + (string-append "-v \"" (password-value role) + "=$(cat " file-name ")\"") + "")) + (define (roles->queries roles) (apply mixed-text-file "queries" (append-map (lambda (role) (match-record role <postgresql-role> (name permissions create-database? encoding collation ctype - template) + template password-file) `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ rolname = '" ,name "')) as not_exists;\n" "\\gset\n" "\\if :not_exists\n" "CREATE ROLE \"" ,name "\"" " WITH " ,(format-permissions permissions) +,(if (and (string? password-file) + (not (string-null? password-file))) + (string-append + "\nPASSWORD :'" (password-value role) "'") + "") ";\n" ,@(if create-database? `("CREATE DATABASE \"" ,name "\"" @@ -436,20 +461,30 @@ (define (postgresql-create-roles config) (let ((host (postgresql-role-configuration-host config)) (roles (postgresql-role-configuration-roles config))) - #~(let ((psql #$(file-append postgresql "/bin/psql"))) - (list psql "-a" "-h" #$host "-f" #$(roles->queries roles))))) + (program-file "run-queries" + #~(let ((bash #$(file-append bash-minimal "/bin/bash")) + (psql #$(file-append postgresql "/bin/psql"))) + (define command + (string-append + "set -e; exec " psql " -c -a -h " #$host " -f " + #$(roles->queries roles) " " + (string-join + (list + #$@(map role->password-variable roles)) + " "))) + (execlp bash bash "-c" command))))) (define (postgresql-role-shepherd-service config) (match-record config <postgresql-role-configuration> - (log) + (log requirement) (list (shepherd-service - (requirement '(user-processes postgres)) + (requirement `(user-processes postgres ,@requirement)) (provision '(postgres-roles)) (one-shot? #t) (start #~(lambda args (zero? (spawn-command - #$(postgresql-create-roles config) + (list #$(postgresql-create-roles config)) #:user "postgres" #:group "postgres" ;; XXX: As of Shepherd 1.0.2, #:log-file is not @@ -468,6 +503,7 @@ (define postgresql-role-service-type (match-record config <postgresql-role-configuration> (host roles) (postgresql-role-configuration + (inherit config) (host host) (roles (append roles extended-roles)))))) (default-value (postgresql-role-configuration)) diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm index fd5041344b6..c5da603565d 100644 --- a/gnu/tests/databases.scm +++ b/gnu/tests/databases.scm @@ -142,6 +142,8 @@ (define %role-log-file (define %postgresql-os (simple-operating-system + (extra-special-file "/password" + (plain-file "password" "hello")) (service postgresql-service-type (postgresql-configuration (postgresql postgresql) @@ -158,6 +160,10 @@ (define %postgresql-os (roles (list (postgresql-role (name "root") + (create-database? #t)) + (postgresql-role + (name "alice") + (password-file "/password") (create-database? #t)))))))) (define (run-postgresql-test) @@ -230,14 +236,40 @@ (define (run-postgresql-test) (marionette-eval '(begin (use-modules (gnu services herd) + (srfi srfi-1) (ice-9 popen)) (current-output-port (open-file "/dev/console" "w0")) + (every + (lambda (role) + (let* ((port (open-pipe* + OPEN_READ + #$(file-append postgresql "/bin/psql") + "-tA" "-c" + (string-append + "SELECT 1 FROM pg_database WHERE" + " datname='" role "'"))) + (output (get-string-all port))) + (close-pipe port) + (string-contains output "1"))) + '("root" "alice"))) + marionette)) + + (test-assert "database passwords are set" + (marionette-eval + '(begin + (use-modules (gnu services herd) + (ice-9 match) + (ice-9 popen)) + (current-output-port + (open-file "/dev/console" "w0")) + (setgid (passwd:gid (getpwnam "alice"))) + (setuid (passwd:uid (getpw "alice"))) + (setenv "PGPASSWORD" "hello") (let* ((port (open-pipe* OPEN_READ - #$(file-append postgresql "/bin/psql") - "-tA" "-c" "SELECT 1 FROM pg_database WHERE - datname='root'")) + #$(file-append postgresql "/bin/psql") "-tA" "-c" + "SELECT 1 FROM pg_database WHERE datname='alice'")) (output (get-string-all port))) (close-pipe port) (string-contains output "1"))) base-commit: 6a2a78fde19683f07c8b10f492cda67447bc99eb -- 2.49.0
guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Tue, 08 Apr 2025 20:39:01 GMT) Full text and rfc822 format available.Message #32 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: paul <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Subject: Re: [PATCH] services: postgresql-role: Add support for password files. Date: Tue, 8 Apr 2025 22:38:09 +0200
Hi I'm sending a v5 rebased on current master that uses a gexp to reference cat. cheers, giacomo
ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Tue, 08 Apr 2025 20:40:02 GMT) Full text and rfc822 format available.Message #35 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: Giacomo Leidi <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Cc: Giacomo Leidi <goodoldpaul <at> autistici.org> Subject: [PATCH v5] services: postgresql-role: Add support for password files. Date: Tue, 8 Apr 2025 22:38:56 +0200
This commit adds a password-file to the postgresql-role field. It allows users to provision Postgres roles with a set password. * gnu/services/databases.scm (postgresql-role): Add password-file field; (postgresql-role-configuration): add requirement field; (postgresql-create-roles): add support for setting passwords from a file without leaking passwords to the command line; (postgresql-role-shepherd-service): add support for customizable requirements. * gnu/tests/databases.scm: Test it. * doc/guix.texi: Document it. Change-Id: I3aabaa10b0c5e826c5aa874e5649e25a3508a585 --- doc/guix.texi | 15 +++++++++--- gnu/services/databases.scm | 48 +++++++++++++++++++++++++++++++++----- gnu/tests/databases.scm | 38 +++++++++++++++++++++++++++--- 3 files changed, 89 insertions(+), 12 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index bee80cd4e2b..8b32557f76a 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -27588,9 +27588,10 @@ Database Services @lisp (service-extension postgresql-role-service-type - (const (postgresql-role - (name "alice") - (create-database? #t)))) + (const (list + (postgresql-role + (name "alice") + (create-database? #t))))) @end lisp @end defvar @@ -27613,6 +27614,10 @@ Database Services @item @code{create-database?} (default: @code{#f}) whether to create a database with the same name as the role. +@item @code{password-file} (default: @code{#f}) +A string representing the path of a file that contains the password to be set +for the role. + @item @code{encoding} (default: @code{"UTF8"}) The character set to use for storing text in the database. @@ -27641,6 +27646,10 @@ Database Services @item @code{log} (default: @code{"/var/log/postgresql_roles.log"}) File name of the log file. +@item @code{requirement} (default: @code{'()}) (type: list-of-symbols) +Set additional Shepherd services dependencies to the provisioned +Shepherd service. + @item @code{roles} (default: @code{'()}) The initial PostgreSQL roles to create. @end table diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm index 6d80376d90d..9242fa62642 100644 --- a/gnu/services/databases.scm +++ b/gnu/services/databases.scm @@ -9,6 +9,7 @@ ;;; Copyright © 2020, 2022 Marius Bakke <marius <at> gnu.org> ;;; Copyright © 2021 David Larsson <david.larsson <at> selfhosted.xyz> ;;; Copyright © 2021 Aljosha Papsch <ep <at> stern-data.com> +;;; Copyright © 2025 Giacomo Leidi <goodoldpaul <at> autistici.org> ;;; ;;; This file is part of GNU Guix. ;;; @@ -31,6 +32,7 @@ (define-module (gnu services databases) #:use-module (gnu system shadow) #:use-module (gnu packages admin) #:use-module (gnu packages base) + #:use-module (gnu packages bash) #:use-module (gnu packages databases) #:use-module (guix build-system trivial) #:use-module (guix build union) @@ -65,11 +67,13 @@ (define-module (gnu services databases) postgresql-role postgresql-role? postgresql-role-name + postgresql-role-password-file postgresql-role-permissions postgresql-role-create-database? postgresql-role-configuration postgresql-role-configuration? postgresql-role-configuration-host + postgresql-role-configuration-requirement postgresql-role-configuration-roles postgresql-role-service-type @@ -374,6 +378,8 @@ (define-record-type* <postgresql-role> postgresql-role make-postgresql-role postgresql-role? (name postgresql-role-name) ;string + (password-file postgresql-role-password-file ;string + (default #f)) (permissions postgresql-role-permissions (default '(createdb login))) ;list (create-database? postgresql-role-create-database? ;boolean @@ -392,6 +398,8 @@ (define-record-type* <postgresql-role-configuration> postgresql-role-configuration? (host postgresql-role-configuration-host ;string (default "/var/run/postgresql")) + (requirement postgresql-role-configuration-requirement ;list-of-symbols + (default '())) (log postgresql-role-configuration-log ;string (default "/var/log/postgresql_roles.log")) (roles postgresql-role-configuration-roles @@ -409,19 +417,36 @@ (define (postgresql-create-roles config) permissions) " "))) + (define (password-value role) + (string-append "password_" (postgresql-role-name role))) + + (define (role->password-variable role) + (define file-name + (postgresql-role-password-file role)) + (if (string? file-name) + ;; This way passwords do not leak to the command line + #~(string-append "-v \"" #$(password-value role) + "=$(" #+coreutils "/bin/cat " #$file-name ")\"") + "")) + (define (roles->queries roles) (apply mixed-text-file "queries" (append-map (lambda (role) (match-record role <postgresql-role> (name permissions create-database? encoding collation ctype - template) + template password-file) `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ rolname = '" ,name "')) as not_exists;\n" "\\gset\n" "\\if :not_exists\n" "CREATE ROLE \"" ,name "\"" " WITH " ,(format-permissions permissions) +,(if (and (string? password-file) + (not (string-null? password-file))) + (string-append + "\nPASSWORD :'" (password-value role) "'") + "") ";\n" ,@(if create-database? `("CREATE DATABASE \"" ,name "\"" @@ -436,20 +461,30 @@ (define (postgresql-create-roles config) (let ((host (postgresql-role-configuration-host config)) (roles (postgresql-role-configuration-roles config))) - #~(let ((psql #$(file-append postgresql "/bin/psql"))) - (list psql "-a" "-h" #$host "-f" #$(roles->queries roles))))) + (program-file "run-queries" + #~(let ((bash #$(file-append bash-minimal "/bin/bash")) + (psql #$(file-append postgresql "/bin/psql"))) + (define command + (string-append + "set -e; exec " psql " -c -a -h " #$host " -f " + #$(roles->queries roles) " " + (string-join + (list + #$@(map role->password-variable roles)) + " "))) + (execlp bash bash "-c" command))))) (define (postgresql-role-shepherd-service config) (match-record config <postgresql-role-configuration> - (log) + (log requirement) (list (shepherd-service - (requirement '(user-processes postgres)) + (requirement `(user-processes postgres ,@requirement)) (provision '(postgres-roles)) (one-shot? #t) (start #~(lambda args (zero? (spawn-command - #$(postgresql-create-roles config) + (list #$(postgresql-create-roles config)) #:user "postgres" #:group "postgres" ;; XXX: As of Shepherd 1.0.2, #:log-file is not @@ -468,6 +503,7 @@ (define postgresql-role-service-type (match-record config <postgresql-role-configuration> (host roles) (postgresql-role-configuration + (inherit config) (host host) (roles (append roles extended-roles)))))) (default-value (postgresql-role-configuration)) diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm index fd5041344b6..c5da603565d 100644 --- a/gnu/tests/databases.scm +++ b/gnu/tests/databases.scm @@ -142,6 +142,8 @@ (define %role-log-file (define %postgresql-os (simple-operating-system + (extra-special-file "/password" + (plain-file "password" "hello")) (service postgresql-service-type (postgresql-configuration (postgresql postgresql) @@ -158,6 +160,10 @@ (define %postgresql-os (roles (list (postgresql-role (name "root") + (create-database? #t)) + (postgresql-role + (name "alice") + (password-file "/password") (create-database? #t)))))))) (define (run-postgresql-test) @@ -230,14 +236,40 @@ (define (run-postgresql-test) (marionette-eval '(begin (use-modules (gnu services herd) + (srfi srfi-1) (ice-9 popen)) (current-output-port (open-file "/dev/console" "w0")) + (every + (lambda (role) + (let* ((port (open-pipe* + OPEN_READ + #$(file-append postgresql "/bin/psql") + "-tA" "-c" + (string-append + "SELECT 1 FROM pg_database WHERE" + " datname='" role "'"))) + (output (get-string-all port))) + (close-pipe port) + (string-contains output "1"))) + '("root" "alice"))) + marionette)) + + (test-assert "database passwords are set" + (marionette-eval + '(begin + (use-modules (gnu services herd) + (ice-9 match) + (ice-9 popen)) + (current-output-port + (open-file "/dev/console" "w0")) + (setgid (passwd:gid (getpwnam "alice"))) + (setuid (passwd:uid (getpw "alice"))) + (setenv "PGPASSWORD" "hello") (let* ((port (open-pipe* OPEN_READ - #$(file-append postgresql "/bin/psql") - "-tA" "-c" "SELECT 1 FROM pg_database WHERE - datname='root'")) + #$(file-append postgresql "/bin/psql") "-tA" "-c" + "SELECT 1 FROM pg_database WHERE datname='alice'")) (output (get-string-all port))) (close-pipe port) (string-contains output "1"))) base-commit: b413d1ea6ae5198becaaeed73495d5c0341e38f7 -- 2.49.0
guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Tue, 08 Apr 2025 20:58:01 GMT) Full text and rfc822 format available.Message #38 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: paul <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Subject: Re: [PATCH] services: postgresql-role: Add support for password files. Date: Tue, 8 Apr 2025 22:57:12 +0200
I'm sending a v6 fixing psql command. cheers, giacomo
ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Tue, 08 Apr 2025 20:59:02 GMT) Full text and rfc822 format available.Message #41 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: Giacomo Leidi <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Cc: Giacomo Leidi <goodoldpaul <at> autistici.org> Subject: [PATCH v6] services: postgresql-role: Add support for password files. Date: Tue, 8 Apr 2025 22:57:26 +0200
This commit adds a password-file to the postgresql-role field. It allows users to provision Postgres roles with a set password. * gnu/services/databases.scm (postgresql-role): Add password-file field; (postgresql-role-configuration): add requirement field; (postgresql-create-roles): add support for setting passwords from a file without leaking passwords to the command line; (postgresql-role-shepherd-service): add support for customizable requirements. * gnu/tests/databases.scm: Test it. * doc/guix.texi: Document it. Change-Id: I3aabaa10b0c5e826c5aa874e5649e25a3508a585 --- doc/guix.texi | 15 +++++++++--- gnu/services/databases.scm | 48 +++++++++++++++++++++++++++++++++----- gnu/tests/databases.scm | 38 +++++++++++++++++++++++++++--- 3 files changed, 89 insertions(+), 12 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index bee80cd4e2b..8b32557f76a 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -27588,9 +27588,10 @@ Database Services @lisp (service-extension postgresql-role-service-type - (const (postgresql-role - (name "alice") - (create-database? #t)))) + (const (list + (postgresql-role + (name "alice") + (create-database? #t))))) @end lisp @end defvar @@ -27613,6 +27614,10 @@ Database Services @item @code{create-database?} (default: @code{#f}) whether to create a database with the same name as the role. +@item @code{password-file} (default: @code{#f}) +A string representing the path of a file that contains the password to be set +for the role. + @item @code{encoding} (default: @code{"UTF8"}) The character set to use for storing text in the database. @@ -27641,6 +27646,10 @@ Database Services @item @code{log} (default: @code{"/var/log/postgresql_roles.log"}) File name of the log file. +@item @code{requirement} (default: @code{'()}) (type: list-of-symbols) +Set additional Shepherd services dependencies to the provisioned +Shepherd service. + @item @code{roles} (default: @code{'()}) The initial PostgreSQL roles to create. @end table diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm index 6d80376d90d..28b69bca3cb 100644 --- a/gnu/services/databases.scm +++ b/gnu/services/databases.scm @@ -9,6 +9,7 @@ ;;; Copyright © 2020, 2022 Marius Bakke <marius <at> gnu.org> ;;; Copyright © 2021 David Larsson <david.larsson <at> selfhosted.xyz> ;;; Copyright © 2021 Aljosha Papsch <ep <at> stern-data.com> +;;; Copyright © 2025 Giacomo Leidi <goodoldpaul <at> autistici.org> ;;; ;;; This file is part of GNU Guix. ;;; @@ -31,6 +32,7 @@ (define-module (gnu services databases) #:use-module (gnu system shadow) #:use-module (gnu packages admin) #:use-module (gnu packages base) + #:use-module (gnu packages bash) #:use-module (gnu packages databases) #:use-module (guix build-system trivial) #:use-module (guix build union) @@ -65,11 +67,13 @@ (define-module (gnu services databases) postgresql-role postgresql-role? postgresql-role-name + postgresql-role-password-file postgresql-role-permissions postgresql-role-create-database? postgresql-role-configuration postgresql-role-configuration? postgresql-role-configuration-host + postgresql-role-configuration-requirement postgresql-role-configuration-roles postgresql-role-service-type @@ -374,6 +378,8 @@ (define-record-type* <postgresql-role> postgresql-role make-postgresql-role postgresql-role? (name postgresql-role-name) ;string + (password-file postgresql-role-password-file ;string + (default #f)) (permissions postgresql-role-permissions (default '(createdb login))) ;list (create-database? postgresql-role-create-database? ;boolean @@ -392,6 +398,8 @@ (define-record-type* <postgresql-role-configuration> postgresql-role-configuration? (host postgresql-role-configuration-host ;string (default "/var/run/postgresql")) + (requirement postgresql-role-configuration-requirement ;list-of-symbols + (default '())) (log postgresql-role-configuration-log ;string (default "/var/log/postgresql_roles.log")) (roles postgresql-role-configuration-roles @@ -409,19 +417,36 @@ (define (postgresql-create-roles config) permissions) " "))) + (define (password-value role) + (string-append "password_" (postgresql-role-name role))) + + (define (role->password-variable role) + (define file-name + (postgresql-role-password-file role)) + (if (string? file-name) + ;; This way passwords do not leak to the command line + #~(string-append "-v \"" #$(password-value role) + "=$(" #+coreutils "/bin/cat " #$file-name ")\"") + "")) + (define (roles->queries roles) (apply mixed-text-file "queries" (append-map (lambda (role) (match-record role <postgresql-role> (name permissions create-database? encoding collation ctype - template) + template password-file) `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ rolname = '" ,name "')) as not_exists;\n" "\\gset\n" "\\if :not_exists\n" "CREATE ROLE \"" ,name "\"" " WITH " ,(format-permissions permissions) +,(if (and (string? password-file) + (not (string-null? password-file))) + (string-append + "\nPASSWORD :'" (password-value role) "'") + "") ";\n" ,@(if create-database? `("CREATE DATABASE \"" ,name "\"" @@ -436,20 +461,30 @@ (define (postgresql-create-roles config) (let ((host (postgresql-role-configuration-host config)) (roles (postgresql-role-configuration-roles config))) - #~(let ((psql #$(file-append postgresql "/bin/psql"))) - (list psql "-a" "-h" #$host "-f" #$(roles->queries roles))))) + (program-file "run-queries" + #~(let ((bash #$(file-append bash-minimal "/bin/bash")) + (psql #$(file-append postgresql "/bin/psql"))) + (define command + (string-append + "set -e; exec " psql " -a -h " #$host " -f " + #$(roles->queries roles) " " + (string-join + (list + #$@(map role->password-variable roles)) + " "))) + (execlp bash bash "-c" command))))) (define (postgresql-role-shepherd-service config) (match-record config <postgresql-role-configuration> - (log) + (log requirement) (list (shepherd-service - (requirement '(user-processes postgres)) + (requirement `(user-processes postgres ,@requirement)) (provision '(postgres-roles)) (one-shot? #t) (start #~(lambda args (zero? (spawn-command - #$(postgresql-create-roles config) + (list #$(postgresql-create-roles config)) #:user "postgres" #:group "postgres" ;; XXX: As of Shepherd 1.0.2, #:log-file is not @@ -468,6 +503,7 @@ (define postgresql-role-service-type (match-record config <postgresql-role-configuration> (host roles) (postgresql-role-configuration + (inherit config) (host host) (roles (append roles extended-roles)))))) (default-value (postgresql-role-configuration)) diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm index fd5041344b6..c5da603565d 100644 --- a/gnu/tests/databases.scm +++ b/gnu/tests/databases.scm @@ -142,6 +142,8 @@ (define %role-log-file (define %postgresql-os (simple-operating-system + (extra-special-file "/password" + (plain-file "password" "hello")) (service postgresql-service-type (postgresql-configuration (postgresql postgresql) @@ -158,6 +160,10 @@ (define %postgresql-os (roles (list (postgresql-role (name "root") + (create-database? #t)) + (postgresql-role + (name "alice") + (password-file "/password") (create-database? #t)))))))) (define (run-postgresql-test) @@ -230,14 +236,40 @@ (define (run-postgresql-test) (marionette-eval '(begin (use-modules (gnu services herd) + (srfi srfi-1) (ice-9 popen)) (current-output-port (open-file "/dev/console" "w0")) + (every + (lambda (role) + (let* ((port (open-pipe* + OPEN_READ + #$(file-append postgresql "/bin/psql") + "-tA" "-c" + (string-append + "SELECT 1 FROM pg_database WHERE" + " datname='" role "'"))) + (output (get-string-all port))) + (close-pipe port) + (string-contains output "1"))) + '("root" "alice"))) + marionette)) + + (test-assert "database passwords are set" + (marionette-eval + '(begin + (use-modules (gnu services herd) + (ice-9 match) + (ice-9 popen)) + (current-output-port + (open-file "/dev/console" "w0")) + (setgid (passwd:gid (getpwnam "alice"))) + (setuid (passwd:uid (getpw "alice"))) + (setenv "PGPASSWORD" "hello") (let* ((port (open-pipe* OPEN_READ - #$(file-append postgresql "/bin/psql") - "-tA" "-c" "SELECT 1 FROM pg_database WHERE - datname='root'")) + #$(file-append postgresql "/bin/psql") "-tA" "-c" + "SELECT 1 FROM pg_database WHERE datname='alice'")) (output (get-string-all port))) (close-pipe port) (string-contains output "1"))) base-commit: b413d1ea6ae5198becaaeed73495d5c0341e38f7 -- 2.49.0
guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Fri, 25 Apr 2025 13:28:02 GMT) Full text and rfc822 format available.Message #44 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: Giacomo Leidi <goodoldpaul <at> autistici.org> Cc: 73196 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org> Subject: Re: bug#73196: [PATCH] services: postgresql-role: Add support for password files. Date: Fri, 25 Apr 2025 22:27:27 +0900
Hi, Giacomo Leidi <goodoldpaul <at> autistici.org> writes: > This commit adds a password-file to the postgresql-role field. It > allows users to provision Postgres roles with a set password. > > * gnu/services/databases.scm (postgresql-role): Add password-file field; > (postgresql-role-configuration): add requirement field; > (postgresql-create-roles): add support for setting passwords from a > file without leaking passwords to the command line; > (postgresql-role-shepherd-service): add support for customizable > requirements. > * gnu/tests/databases.scm: Test it. > * doc/guix.texi: Document it. Looks useful! Nitpick: Each change can be a sentence, starting with a capitalized letter and ending with a period. > Change-Id: I3aabaa10b0c5e826c5aa874e5649e25a3508a585 > --- > doc/guix.texi | 15 +++++++++--- > gnu/services/databases.scm | 48 +++++++++++++++++++++++++++++++++----- > gnu/tests/databases.scm | 38 +++++++++++++++++++++++++++--- > 3 files changed, 89 insertions(+), 12 deletions(-) > > diff --git a/doc/guix.texi b/doc/guix.texi > index bee80cd4e2b..8b32557f76a 100644 > --- a/doc/guix.texi > +++ b/doc/guix.texi > @@ -27588,9 +27588,10 @@ Database Services > > @lisp > (service-extension postgresql-role-service-type > - (const (postgresql-role > - (name "alice") > - (create-database? #t)))) > + (const (list > + (postgresql-role > + (name "alice") > + (create-database? #t))))) Are you sure this is correct? If it's a bug fix, it's not listed in the changelog. > @end lisp > @end defvar > > @@ -27613,6 +27614,10 @@ Database Services > @item @code{create-database?} (default: @code{#f}) > whether to create a database with the same name as the role. > > +@item @code{password-file} (default: @code{#f}) > +A string representing the path of a file that contains the password to be set > +for the role. > + > @item @code{encoding} (default: @code{"UTF8"}) > The character set to use for storing text in the database. > > @@ -27641,6 +27646,10 @@ Database Services > @item @code{log} (default: @code{"/var/log/postgresql_roles.log"}) > File name of the log file. > > +@item @code{requirement} (default: @code{'()}) (type: list-of-symbols) > +Set additional Shepherd services dependencies to the provisioned > +Shepherd service. It seems the canonical/most conventional configuration field name to use for this is 'shepherd-requirement', not 'requirement'. [...] > postgresql-role? > postgresql-role-name > + postgresql-role-password-file > postgresql-role-permissions > postgresql-role-create-database? > postgresql-role-configuration > postgresql-role-configuration? > postgresql-role-configuration-host > + postgresql-role-configuration-requirement > postgresql-role-configuration-roles > > postgresql-role-service-type > @@ -374,6 +378,8 @@ (define-record-type* <postgresql-role> > postgresql-role make-postgresql-role > postgresql-role? > (name postgresql-role-name) ;string > + (password-file postgresql-role-password-file ;string > + (default #f)) > (permissions postgresql-role-permissions > (default '(createdb login))) ;list > (create-database? postgresql-role-create-database? ;boolean > @@ -392,6 +398,8 @@ (define-record-type* <postgresql-role-configuration> > postgresql-role-configuration? > (host postgresql-role-configuration-host ;string > (default "/var/run/postgresql")) > + (requirement postgresql-role-configuration-requirement ;list-of-symbols > + (default '())) as mentioned above, this should be 'shepherd-requirement'. > (log postgresql-role-configuration-log ;string > (default "/var/log/postgresql_roles.log")) > (roles postgresql-role-configuration-roles > @@ -409,19 +417,36 @@ (define (postgresql-create-roles config) > permissions) > " "))) > > + (define (password-value role) > + (string-append "password_" (postgresql-role-name role))) > + > + (define (role->password-variable role) > + (define file-name > + (postgresql-role-password-file role)) I'd maybe use a let here, just for stylistic preferences. > + (if (string? file-name) > + ;; This way passwords do not leak to the command line Please use complete sentences, including the trailing period, for stand-alone comments. > + #~(string-append "-v \"" #$(password-value role) > + "=$(" #+coreutils "/bin/cat " #$file-name ")\"") Are you sure this should use ungexp-native (#+) on coreutils? If this runs in a snippet, it should use the regular ungexp (#$). It could also use file-append. Only if it runs as part of a derivation build should it use #+. > + "")) > + > (define (roles->queries roles) > (apply mixed-text-file "queries" > (append-map > (lambda (role) > (match-record role <postgresql-role> > (name permissions create-database? encoding collation ctype > - template) > + template password-file) > `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ > rolname = '" ,name "')) as not_exists;\n" > "\\gset\n" > "\\if :not_exists\n" > "CREATE ROLE \"" ,name "\"" > " WITH " ,(format-permissions permissions) > +,(if (and (string? password-file) > + (not (string-null? password-file))) Since you already check for the string-null? case, perhaps the default password value should be "" instead of #f? And then you could do away with the string? check (though ideally there could be a sanitizer to verify that a string is passed -- perhaps best leaving it for the day this gets ported to define-configuration, which auto-generates sanitizers based on the type specified for the field in its form. > + (string-append > + "\nPASSWORD :'" (password-value role) "'") > + "") > ";\n" > ,@(if create-database? > `("CREATE DATABASE \"" ,name "\"" > @@ -436,20 +461,30 @@ (define (postgresql-create-roles config) > > (let ((host (postgresql-role-configuration-host config)) > (roles (postgresql-role-configuration-roles config))) > - #~(let ((psql #$(file-append postgresql "/bin/psql"))) > - (list psql "-a" "-h" #$host "-f" #$(roles->queries roles))))) > + (program-file "run-queries" > + #~(let ((bash #$(file-append bash-minimal "/bin/bash")) > + (psql #$(file-append postgresql "/bin/psql"))) > + (define command > + (string-append > + "set -e; exec " psql " -a -h " #$host " -f " > + #$(roles->queries roles) " " > + (string-join > + (list > + #$@(map role->password-variable roles)) > + " "))) > + (execlp bash bash "-c" command))))) > > (define (postgresql-role-shepherd-service config) > (match-record config <postgresql-role-configuration> > - (log) > + (log requirement) > (list (shepherd-service > - (requirement '(user-processes postgres)) > + (requirement `(user-processes postgres ,@requirement)) I see two styles being used in Guix services: 'shepherd-requirement' extend the requirements, or overrides it, with the 'user-processes at least normally found in the default values. I think at this point in time the later fits better in Guix (think of arguments such as #:modules and #:imported-modules, etc. the user has full control on them, for the better and worst). > (provision '(postgres-roles)) > (one-shot? #t) > (start > #~(lambda args > (zero? (spawn-command > - #$(postgresql-create-roles config) > + (list #$(postgresql-create-roles config)) Why is this change now necessary? I didn't follow. > #:user "postgres" > #:group "postgres" > ;; XXX: As of Shepherd 1.0.2, #:log-file is not > @@ -468,6 +503,7 @@ (define postgresql-role-service-type > (match-record config <postgresql-role-configuration> > (host roles) > (postgresql-role-configuration > + (inherit config) and this one? Was it a preexisting bug? > (host host) > (roles (append roles extended-roles)))))) > (default-value (postgresql-role-configuration)) > diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm > index fd5041344b6..c5da603565d 100644 > --- a/gnu/tests/databases.scm > +++ b/gnu/tests/databases.scm > @@ -142,6 +142,8 @@ (define %role-log-file > > (define %postgresql-os > (simple-operating-system > + (extra-special-file "/password" > + (plain-file "password" "hello")) > (service postgresql-service-type > (postgresql-configuration > (postgresql postgresql) > @@ -158,6 +160,10 @@ (define %postgresql-os > (roles > (list (postgresql-role > (name "root") > + (create-database? #t)) > + (postgresql-role > + (name "alice") > + (password-file "/password") > (create-database? #t)))))))) > > (define (run-postgresql-test) > @@ -230,14 +236,40 @@ (define (run-postgresql-test) > (marionette-eval > '(begin > (use-modules (gnu services herd) > + (srfi srfi-1) > (ice-9 popen)) > (current-output-port > (open-file "/dev/console" "w0")) > + (every > + (lambda (role) > + (let* ((port (open-pipe* > + OPEN_READ > + #$(file-append postgresql "/bin/psql") > + "-tA" "-c" > + (string-append > + "SELECT 1 FROM pg_database WHERE" > + " datname='" role "'"))) > + (output (get-string-all port))) > + (close-pipe port) > + (string-contains output "1"))) > + '("root" "alice"))) > + marionette)) > + > + (test-assert "database passwords are set" > + (marionette-eval > + '(begin > + (use-modules (gnu services herd) > + (ice-9 match) > + (ice-9 popen)) > + (current-output-port > + (open-file "/dev/console" "w0")) > + (setgid (passwd:gid (getpwnam "alice"))) > + (setuid (passwd:uid (getpw "alice"))) > + (setenv "PGPASSWORD" "hello") > (let* ((port (open-pipe* > OPEN_READ > - #$(file-append postgresql "/bin/psql") > - "-tA" "-c" "SELECT 1 FROM pg_database WHERE > - datname='root'")) > + #$(file-append postgresql "/bin/psql") "-tA" "-c" > + "SELECT 1 FROM pg_database WHERE datname='alice'")) Nice to have tests! Could you please send a v7 taking care of my comments? Then I guess it'd be good to go. -- Thanks, Maxim
guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Sun, 27 Apr 2025 15:08:02 GMT) Full text and rfc822 format available.Message #47 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: paul <goodoldpaul <at> autistici.org> To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> Cc: 73196 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org> Subject: Re: bug#73196: [PATCH] services: postgresql-role: Add support for password files. Date: Sun, 27 Apr 2025 17:07:01 +0200
[Message part 1 (text/plain, inline)]
Hi Maxim, On 4/25/25 3:27 PM, Maxim Cournoyer wrote: > Hi, > > Giacomo Leidi<goodoldpaul <at> autistici.org> writes: > >> This commit adds a password-file to the postgresql-role field. It >> allows users to provision Postgres roles with a set password. >> >> * gnu/services/databases.scm (postgresql-role): Add password-file field; >> (postgresql-role-configuration): add requirement field; >> (postgresql-create-roles): add support for setting passwords from a >> file without leaking passwords to the command line; >> (postgresql-role-shepherd-service): add support for customizable >> requirements. >> * gnu/tests/databases.scm: Test it. >> * doc/guix.texi: Document it. > Looks useful! > > Nitpick: Each change can be a sentence, starting with a capitalized > letter and ending with a period. Should be fixed now. > >> Change-Id: I3aabaa10b0c5e826c5aa874e5649e25a3508a585 >> --- >> doc/guix.texi | 15 +++++++++--- >> gnu/services/databases.scm | 48 +++++++++++++++++++++++++++++++++----- >> gnu/tests/databases.scm | 38 +++++++++++++++++++++++++++--- >> 3 files changed, 89 insertions(+), 12 deletions(-) >> >> diff --git a/doc/guix.texi b/doc/guix.texi >> index bee80cd4e2b..8b32557f76a 100644 >> --- a/doc/guix.texi >> +++ b/doc/guix.texi >> @@ -27588,9 +27588,10 @@ Database Services >> >> @lisp >> (service-extension postgresql-role-service-type >> - (const (postgresql-role >> - (name "alice") >> - (create-database? #t)))) >> + (const (list >> + (postgresql-role >> + (name "alice") >> + (create-database? #t))))) > Are you sure this is correct? If it's a bug fix, it's not listed in the > changelog. Yes I am sure, the service type has (compose concatenate) this is now mentioned in the changelog. >> postgresql-role? >> postgresql-role-name >> + postgresql-role-password-file >> postgresql-role-permissions >> postgresql-role-create-database? >> postgresql-role-configuration >> postgresql-role-configuration? >> postgresql-role-configuration-host >> + postgresql-role-configuration-requirement >> postgresql-role-configuration-roles >> >> postgresql-role-service-type >> @@ -374,6 +378,8 @@ (define-record-type* <postgresql-role> >> postgresql-role make-postgresql-role >> postgresql-role? >> (name postgresql-role-name) ;string >> + (password-file postgresql-role-password-file ;string >> + (default #f)) >> (permissions postgresql-role-permissions >> (default '(createdb login))) ;list >> (create-database? postgresql-role-create-database? ;boolean >> @@ -392,6 +398,8 @@ (define-record-type* <postgresql-role-configuration> >> postgresql-role-configuration? >> (host postgresql-role-configuration-host ;string >> (default "/var/run/postgresql")) >> + (requirement postgresql-role-configuration-requirement ;list-of-symbols >> + (default '())) > as mentioned above, this should be 'shepherd-requirement'. I know at least two examples that are already in master which use 'requirement' (I may be biased as I authored both of them): the restic-backup-service-type and the oci-container-service-type. If you don't find this to be a blocker I would defer this change to a specific commit setting a convention for the whole codebase after an analysis of which services use simply 'requirement' and which use 'shepherd-requirement'. Is it ok for you? >> (log postgresql-role-configuration-log ;string >> (default "/var/log/postgresql_roles.log")) >> (roles postgresql-role-configuration-roles >> @@ -409,19 +417,36 @@ (define (postgresql-create-roles config) >> permissions) >> " "))) >> >> + (define (password-value role) >> + (string-append "password_" (postgresql-role-name role))) >> + >> + (define (role->password-variable role) >> + (define file-name >> + (postgresql-role-password-file role)) > I'd maybe use a let here, just for stylistic preferences. I changed the define to let > >> + (if (string? file-name) >> + ;; This way passwords do not leak to the command line > Please use complete sentences, including the trailing period, for > stand-alone comments. I added the period > >> + #~(string-append "-v \"" #$(password-value role) >> + "=$(" #+coreutils "/bin/cat " #$file-name ")\"") > Are you sure this should use ungexp-native (#+) on coreutils? If this > runs in a snippet, it should use the regular ungexp (#$). It could also > use file-append. Only if it runs as part of a derivation build should > it use #+. this runs at system activation so in my understanding it should be #$ . Thank you for explaining the difference, I was not aware :) > >> + "")) >> + >> (define (roles->queries roles) >> (apply mixed-text-file "queries" >> (append-map >> (lambda (role) >> (match-record role <postgresql-role> >> (name permissions create-database? encoding collation ctype >> - template) >> + template password-file) >> `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ >> rolname = '" ,name "')) as not_exists;\n" >> "\\gset\n" >> "\\if :not_exists\n" >> "CREATE ROLE \"" ,name "\"" >> " WITH " ,(format-permissions permissions) >> +,(if (and (string? password-file) >> + (not (string-null? password-file))) > Since you already check for the string-null? case, perhaps the default > password value should be "" instead of #f? And then you could do away > with the string? check (though ideally there could be a sanitizer to > verify that a string is passed -- perhaps best leaving it for the day > this gets ported to define-configuration, which auto-generates sanitizers > based on the type specified for the field in its form. in its ideal form (using define-configuration) this would be a 'maybe-string' type, as it represents an optional path. if you don't find this a blocker I would leave this for a later change where we could use 'maybe-value-set?' and leave the type check to a sanitizer. What do you think? >> + (string-append >> + "\nPASSWORD :'" (password-value role) "'") >> + "") >> ";\n" >> ,@(if create-database? >> `("CREATE DATABASE \"" ,name "\"" >> @@ -436,20 +461,30 @@ (define (postgresql-create-roles config) >> >> (let ((host (postgresql-role-configuration-host config)) >> (roles (postgresql-role-configuration-roles config))) >> - #~(let ((psql #$(file-append postgresql "/bin/psql"))) >> - (list psql "-a" "-h" #$host "-f" #$(roles->queries roles))))) >> + (program-file "run-queries" >> + #~(let ((bash #$(file-append bash-minimal "/bin/bash")) >> + (psql #$(file-append postgresql "/bin/psql"))) >> + (define command >> + (string-append >> + "set -e; exec " psql " -a -h " #$host " -f " >> + #$(roles->queries roles) " " >> + (string-join >> + (list >> + #$@(map role->password-variable roles)) >> + " "))) >> + (execlp bash bash "-c" command))))) >> >> (define (postgresql-role-shepherd-service config) >> (match-record config <postgresql-role-configuration> >> - (log) >> + (log requirement) >> (list (shepherd-service >> - (requirement '(user-processes postgres)) >> + (requirement `(user-processes postgres ,@requirement)) > I see two styles being used in Guix services: 'shepherd-requirement' > extend the requirements, or overrides it, with the 'user-processes at > least normally found in the default values. I think at this point in > time the later fits better in Guix (think of arguments such as #:modules > and #:imported-modules, etc. the user has full control on them, for the > better and worst). Thank you for pointing this out, I created a %postgresql-role-shepherd-requirement value and exported it to allow users which want to override the Guix defaults, while retaining them can simply append to this list. > >> (provision '(postgres-roles)) >> (one-shot? #t) >> (start >> #~(lambda args >> (zero? (spawn-command >> - #$(postgresql-create-roles config) >> + (list #$(postgresql-create-roles config)) > Why is this change now necessary? I didn't follow. the return value of postgresql-create-roles changed, before this change it returned a list containing a psql command line, after this change it returns a gexp which will evaluate to the entrypoint responsible for reading password files and then calling psql. > >> #:user "postgres" >> #:group "postgres" >> ;; XXX: As of Shepherd 1.0.2, #:log-file is not >> @@ -468,6 +503,7 @@ (define postgresql-role-service-type >> (match-record config <postgresql-role-configuration> >> (host roles) >> (postgresql-role-configuration >> + (inherit config) > and this one? Was it a preexisting bug? Yes, for example without this change we would lose the 'log' field's value upon extension. > >> (host host) >> (roles (append roles extended-roles)))))) >> (default-value (postgresql-role-configuration)) >> diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm >> index fd5041344b6..c5da603565d 100644 >> --- a/gnu/tests/databases.scm >> +++ b/gnu/tests/databases.scm >> @@ -142,6 +142,8 @@ (define %role-log-file >> >> (define %postgresql-os >> (simple-operating-system >> + (extra-special-file "/password" >> + (plain-file "password" "hello")) >> (service postgresql-service-type >> (postgresql-configuration >> (postgresql postgresql) >> @@ -158,6 +160,10 @@ (define %postgresql-os >> (roles >> (list (postgresql-role >> (name "root") >> + (create-database? #t)) >> + (postgresql-role >> + (name "alice") >> + (password-file "/password") >> (create-database? #t)))))))) >> >> (define (run-postgresql-test) >> @@ -230,14 +236,40 @@ (define (run-postgresql-test) >> (marionette-eval >> '(begin >> (use-modules (gnu services herd) >> + (srfi srfi-1) >> (ice-9 popen)) >> (current-output-port >> (open-file "/dev/console" "w0")) >> + (every >> + (lambda (role) >> + (let* ((port (open-pipe* >> + OPEN_READ >> + #$(file-append postgresql "/bin/psql") >> + "-tA" "-c" >> + (string-append >> + "SELECT 1 FROM pg_database WHERE" >> + " datname='" role "'"))) >> + (output (get-string-all port))) >> + (close-pipe port) >> + (string-contains output "1"))) >> + '("root" "alice"))) >> + marionette)) >> + >> + (test-assert "database passwords are set" >> + (marionette-eval >> + '(begin >> + (use-modules (gnu services herd) >> + (ice-9 match) >> + (ice-9 popen)) >> + (current-output-port >> + (open-file "/dev/console" "w0")) >> + (setgid (passwd:gid (getpwnam "alice"))) >> + (setuid (passwd:uid (getpw "alice"))) >> + (setenv "PGPASSWORD" "hello") >> (let* ((port (open-pipe* >> OPEN_READ >> - #$(file-append postgresql "/bin/psql") >> - "-tA" "-c" "SELECT 1 FROM pg_database WHERE >> - datname='root'")) >> + #$(file-append postgresql "/bin/psql") "-tA" "-c" >> + "SELECT 1 FROM pg_database WHERE datname='alice'")) > Nice to have tests! Could you please send a v7 taking care of my > comments? Then I guess it'd be good to go. I should have addressed all your comments, I'm about to send a v7. Thank you for your help! cheers giacomo
[Message part 2 (text/html, inline)]
ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Sun, 27 Apr 2025 15:14:02 GMT) Full text and rfc822 format available.Message #50 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: Giacomo Leidi <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Cc: Giacomo Leidi <goodoldpaul <at> autistici.org> Subject: [PATCH v7] services: postgresql-role: Add support for password files. Date: Sun, 27 Apr 2025 17:12:54 +0200
This commit adds a password-file to the postgresql-role field. It allows users to provision Postgres roles with a set password. * gnu/services/databases.scm (postgresql-role): Add password-file field. (postgresql-role-configuration): Add requirement field. (postgresql-create-roles): Add support for setting passwords from a file without leaking passwords to the command line. (postgresql-role-shepherd-service): Add support for customizable requirements. * gnu/tests/databases.scm: Test it. * doc/guix.texi: Document the new field and fix the extension point example. Change-Id: I3aabaa10b0c5e826c5aa874e5649e25a3508a585 --- doc/guix.texi | 15 ++++++++--- gnu/services/databases.scm | 52 +++++++++++++++++++++++++++++++++----- gnu/tests/databases.scm | 38 +++++++++++++++++++++++++--- 3 files changed, 93 insertions(+), 12 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index 7b418a4089..ca690c8ace 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -27741,9 +27741,10 @@ Database Services @lisp (service-extension postgresql-role-service-type - (const (postgresql-role - (name "alice") - (create-database? #t)))) + (const (list + (postgresql-role + (name "alice") + (create-database? #t))))) @end lisp @end defvar @@ -27766,6 +27767,10 @@ Database Services @item @code{create-database?} (default: @code{#f}) whether to create a database with the same name as the role. +@item @code{password-file} (default: @code{#f}) +A string representing the path of a file that contains the password to be set +for the role. + @item @code{encoding} (default: @code{"UTF8"}) The character set to use for storing text in the database. @@ -27794,6 +27799,10 @@ Database Services @item @code{log} (default: @code{"/var/log/postgresql_roles.log"}) File name of the log file. +@item @code{requirement} (default: @code{'(user-processes postgres)}) (type: list-of-symbols) +Set additional Shepherd services dependencies to the provisioned +Shepherd service. + @item @code{roles} (default: @code{'()}) The initial PostgreSQL roles to create. @end table diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm index edc3198ad5..86d4ecb739 100644 --- a/gnu/services/databases.scm +++ b/gnu/services/databases.scm @@ -9,6 +9,7 @@ ;;; Copyright © 2020, 2022 Marius Bakke <marius <at> gnu.org> ;;; Copyright © 2021 David Larsson <david.larsson <at> selfhosted.xyz> ;;; Copyright © 2021 Aljosha Papsch <ep <at> stern-data.com> +;;; Copyright © 2025 Giacomo Leidi <goodoldpaul <at> autistici.org> ;;; ;;; This file is part of GNU Guix. ;;; @@ -32,6 +33,7 @@ (define-module (gnu services databases) #:autoload (gnu system accounts) (default-shell) #:use-module (gnu packages admin) #:use-module (gnu packages base) + #:use-module (gnu packages bash) #:use-module (gnu packages databases) #:use-module (guix build-system trivial) #:use-module (guix build union) @@ -68,14 +70,18 @@ (define-module (gnu services databases) postgresql-service postgresql-service-type + %postgresql-role-shepherd-requirement + postgresql-role postgresql-role? postgresql-role-name + postgresql-role-password-file postgresql-role-permissions postgresql-role-create-database? postgresql-role-configuration postgresql-role-configuration? postgresql-role-configuration-host + postgresql-role-configuration-requirement postgresql-role-configuration-roles postgresql-role-service-type @@ -390,6 +396,8 @@ (define-record-type* <postgresql-role> postgresql-role make-postgresql-role postgresql-role? (name postgresql-role-name) ;string + (password-file postgresql-role-password-file ;string + (default #f)) (permissions postgresql-role-permissions (default '(createdb login))) ;list (create-database? postgresql-role-create-database? ;boolean @@ -403,11 +411,16 @@ (define-record-type* <postgresql-role> (template postgresql-role-template ;string (default "template1"))) +(define %postgresql-role-shepherd-requirement + '(user-processes postgres)) + (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")) + (requirement postgresql-role-configuration-requirement ;list-of-symbols + (default %postgresql-role-shepherd-requirement)) (log postgresql-role-configuration-log ;string (default "/var/log/postgresql_roles.log")) (roles postgresql-role-configuration-roles @@ -425,19 +438,35 @@ (define (postgresql-create-roles config) permissions) " "))) + (define (password-value role) + (string-append "password_" (postgresql-role-name role))) + + (define (role->password-variable role) + (let ((file-name (postgresql-role-password-file role))) + (if (string? file-name) + ;; This way passwords do not leak to the command line. + #~(string-append "-v \"" #$(password-value role) + "=$(" #$coreutils "/bin/cat " #$file-name ")\"") + ""))) + (define (roles->queries roles) (apply mixed-text-file "queries" (append-map (lambda (role) (match-record role <postgresql-role> (name permissions create-database? encoding collation ctype - template) + template password-file) `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ rolname = '" ,name "')) as not_exists;\n" "\\gset\n" "\\if :not_exists\n" "CREATE ROLE \"" ,name "\"" " WITH " ,(format-permissions permissions) +,(if (and (string? password-file) + (not (string-null? password-file))) + (string-append + "\nPASSWORD :'" (password-value role) "'") + "") ";\n" ,@(if create-database? `("CREATE DATABASE \"" ,name "\"" @@ -452,20 +481,30 @@ (define (postgresql-create-roles config) (let ((host (postgresql-role-configuration-host config)) (roles (postgresql-role-configuration-roles config))) - #~(let ((psql #$(file-append postgresql "/bin/psql"))) - (list psql "-a" "-h" #$host "-f" #$(roles->queries roles))))) + (program-file "run-queries" + #~(let ((bash #$(file-append bash-minimal "/bin/bash")) + (psql #$(file-append postgresql "/bin/psql"))) + (define command + (string-append + "set -e; exec " psql " -a -h " #$host " -f " + #$(roles->queries roles) " " + (string-join + (list + #$@(map role->password-variable roles)) + " "))) + (execlp bash bash "-c" command))))) (define (postgresql-role-shepherd-service config) (match-record config <postgresql-role-configuration> - (log) + (log requirement) (list (shepherd-service - (requirement '(user-processes postgres)) + (requirement requirement) (provision '(postgres-roles)) (one-shot? #t) (start #~(lambda args (zero? (spawn-command - #$(postgresql-create-roles config) + (list #$(postgresql-create-roles config)) #:user "postgres" #:group "postgres" ;; XXX: As of Shepherd 1.0.2, #:log-file is not @@ -484,6 +523,7 @@ (define postgresql-role-service-type (match-record config <postgresql-role-configuration> (host roles) (postgresql-role-configuration + (inherit config) (host host) (roles (append roles extended-roles)))))) (default-value (postgresql-role-configuration)) diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm index fd5041344b..c5da603565 100644 --- a/gnu/tests/databases.scm +++ b/gnu/tests/databases.scm @@ -142,6 +142,8 @@ (define %role-log-file (define %postgresql-os (simple-operating-system + (extra-special-file "/password" + (plain-file "password" "hello")) (service postgresql-service-type (postgresql-configuration (postgresql postgresql) @@ -158,6 +160,10 @@ (define %postgresql-os (roles (list (postgresql-role (name "root") + (create-database? #t)) + (postgresql-role + (name "alice") + (password-file "/password") (create-database? #t)))))))) (define (run-postgresql-test) @@ -230,14 +236,40 @@ (define (run-postgresql-test) (marionette-eval '(begin (use-modules (gnu services herd) + (srfi srfi-1) (ice-9 popen)) (current-output-port (open-file "/dev/console" "w0")) + (every + (lambda (role) + (let* ((port (open-pipe* + OPEN_READ + #$(file-append postgresql "/bin/psql") + "-tA" "-c" + (string-append + "SELECT 1 FROM pg_database WHERE" + " datname='" role "'"))) + (output (get-string-all port))) + (close-pipe port) + (string-contains output "1"))) + '("root" "alice"))) + marionette)) + + (test-assert "database passwords are set" + (marionette-eval + '(begin + (use-modules (gnu services herd) + (ice-9 match) + (ice-9 popen)) + (current-output-port + (open-file "/dev/console" "w0")) + (setgid (passwd:gid (getpwnam "alice"))) + (setuid (passwd:uid (getpw "alice"))) + (setenv "PGPASSWORD" "hello") (let* ((port (open-pipe* OPEN_READ - #$(file-append postgresql "/bin/psql") - "-tA" "-c" "SELECT 1 FROM pg_database WHERE - datname='root'")) + #$(file-append postgresql "/bin/psql") "-tA" "-c" + "SELECT 1 FROM pg_database WHERE datname='alice'")) (output (get-string-all port))) (close-pipe port) (string-contains output "1"))) base-commit: 97ea59b846c5267098a019f36c84dcaa55fb123e -- 2.49.0
guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Mon, 28 Apr 2025 04:10:02 GMT) Full text and rfc822 format available.Message #53 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: paul <goodoldpaul <at> autistici.org> Cc: 73196 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org> Subject: Re: bug#73196: [PATCH] services: postgresql-role: Add support for password files. Date: Mon, 28 Apr 2025 13:09:11 +0900
Hi, paul <goodoldpaul <at> autistici.org> writes: Thanks for addressing most of my comments. I have a few more ones, in light of your last replies. [...] >>> + (requirement postgresql-role-configuration-requirement ;list-of-symbols >>> + (default '())) >> as mentioned above, this should be 'shepherd-requirement'. > I know at least two examples that are already in master which use > 'requirement' (I may be biased as I authored both of them): the > restic-backup-service-type and the oci-container-service-type. If you > don't find this to be a blocker I would defer this change to a > specific commit setting a convention for the whole codebase after an > analysis of which services use simply 'requirement' and which use > 'shepherd-requirement'. Is it ok for you? OK! I think we'd want to normalize to shepherd-requirement because 'requirement' could well be an option key in a config file (name clash); 'shepherd-requirement', makes this much less likely, while making it more obvious that this is a shepherd thing exposed through the configuration object. For this reason I'd err on using 'shepherd-requirement', because it seems more likely we settle for this variant and deprecating fields is a bit annoying. [...] >> >>> + "")) >>> + >>> (define (roles->queries roles) >>> (apply mixed-text-file "queries" >>> (append-map >>> (lambda (role) >>> (match-record role <postgresql-role> >>> (name permissions create-database? encoding collation ctype >>> - template) >>> + template password-file) >>> `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ >>> rolname = '" ,name "')) as not_exists;\n" >>> "\\gset\n" >>> "\\if :not_exists\n" >>> "CREATE ROLE \"" ,name "\"" >>> " WITH " ,(format-permissions permissions) >>> +,(if (and (string? password-file) >>> + (not (string-null? password-file))) >> Since you already check for the string-null? case, perhaps the default >> password value should be "" instead of #f? And then you could do away >> with the string? check (though ideally there could be a sanitizer to >> verify that a string is passed -- perhaps best leaving it for the day >> this gets ported to define-configuration, which auto-generates sanitizers >> based on the type specified for the field in its form. > in its ideal form (using define-configuration) this would be a > 'maybe-string' type, as it represents an optional path. if you don't > find this a blocker I would leave this for a later change where we > could use 'maybe-value-set?' and leave the type check to a > sanitizer. What do you think? That's OK! [...] >>> - (log) >>> + (log requirement) >>> (list (shepherd-service >>> - (requirement '(user-processes postgres)) >>> + (requirement `(user-processes postgres ,@requirement)) >> I see two styles being used in Guix services: 'shepherd-requirement' >> extend the requirements, or overrides it, with the 'user-processes at >> least normally found in the default values. I think at this point in >> time the later fits better in Guix (think of arguments such as #:modules >> and #:imported-modules, etc. the user has full control on them, for the >> better and worst). > Thank you for pointing this out, I created a > %postgresql-role-shepherd-requirement value and exported it to allow > users which want to override the Guix defaults, while retaining them > can simply append to this list. OK! In my experience it's (somewhat?) conventional to use %default as a prefix for default values, so perhaps %default-postgresql-role-shepherd-requirement could be nicer (it's long but in Scheme the usual style is to not shy away from being extra descriptive in general). >> >>> (provision '(postgres-roles)) >>> (one-shot? #t) >>> (start >>> #~(lambda args >>> (zero? (spawn-command >>> - #$(postgresql-create-roles config) >>> + (list #$(postgresql-create-roles config)) >> Why is this change now necessary? I didn't follow. > > the return value of postgresql-create-roles changed, before this > change it returned a list containing a psql command line, after this > change it returns a gexp which will evaluate to the entrypoint > responsible for reading password files and then calling psql. Thanks for explaining. >> >>> #:user "postgres" >>> #:group "postgres" >>> ;; XXX: As of Shepherd 1.0.2, #:log-file is not >>> @@ -468,6 +503,7 @@ (define postgresql-role-service-type >>> (match-record config <postgresql-role-configuration> >>> (host roles) >>> (postgresql-role-configuration >>> + (inherit config) >> and this one? Was it a preexisting bug? > Yes, for example without this change we would lose the 'log' field's > value upon extension. OK, make sure this is mentioned in the changelog (sorry, I didn't verify if it's already there). I'm looking forward to the v7 :-) -- Thanks, Maxim
guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Tue, 29 Apr 2025 15:51:05 GMT) Full text and rfc822 format available.Message #56 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: paul <goodoldpaul <at> autistici.org> To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> Cc: 73196 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org> Subject: Re: bug#73196: [PATCH] services: postgresql-role: Add support for password files. Date: Tue, 29 Apr 2025 17:50:16 +0200
Hi Maxim, I should have addressed all your remaining comments. I'm about to send a v8, hopefully this will be the correct one :) thank you a lot for your help cheers, giacomo
ludo <at> gnu.org, maxim.cournoyer <at> gmail.com, guix-patches <at> gnu.org
:bug#73196
; Package guix-patches
.
(Tue, 29 Apr 2025 15:52:02 GMT) Full text and rfc822 format available.Message #59 received at 73196 <at> debbugs.gnu.org (full text, mbox):
From: Giacomo Leidi <goodoldpaul <at> autistici.org> To: 73196 <at> debbugs.gnu.org Cc: Giacomo Leidi <goodoldpaul <at> autistici.org> Subject: [PATCH v8] services: postgresql-role: Add support for password files. Date: Tue, 29 Apr 2025 17:51:10 +0200
This commit adds a password-file to the postgresql-role field. It allows users to provision Postgres roles with a set password. * gnu/services/databases.scm (postgresql-role): Add password-file field. (postgresql-role-configuration): Add requirement field. (postgresql-create-roles): Add support for setting passwords from a file without leaking passwords to the command line. (postgresql-role-shepherd-service): Add support for customizable requirements. (postgresql-role-service-type): Pass on postgresql-role-configuration fields values by default, this way user configured fields are not lost. * gnu/tests/databases.scm: Test it. * doc/guix.texi: Document the new field and fix the extension point example. Change-Id: I3aabaa10b0c5e826c5aa874e5649e25a3508a585 --- doc/guix.texi | 15 +++++++-- gnu/services/databases.scm | 64 +++++++++++++++++++++++++++++++------- gnu/tests/databases.scm | 51 ++++++++++++++++++++++++++++-- 3 files changed, 112 insertions(+), 18 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index 7b418a40892..42073e52edb 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -27741,9 +27741,10 @@ Database Services @lisp (service-extension postgresql-role-service-type - (const (postgresql-role - (name "alice") - (create-database? #t)))) + (const (list + (postgresql-role + (name "alice") + (create-database? #t))))) @end lisp @end defvar @@ -27766,6 +27767,10 @@ Database Services @item @code{create-database?} (default: @code{#f}) whether to create a database with the same name as the role. +@item @code{password-file} (default: @code{#f}) +A string representing the path of a file that contains the password to be set +for the role. + @item @code{encoding} (default: @code{"UTF8"}) The character set to use for storing text in the database. @@ -27794,6 +27799,10 @@ Database Services @item @code{log} (default: @code{"/var/log/postgresql_roles.log"}) File name of the log file. +@item @code{shepherd-requirement} (default: @code{'(user-processes postgres)}) (type: list-of-symbols) +Set additional Shepherd services dependencies to the provisioned +Shepherd service. + @item @code{roles} (default: @code{'()}) The initial PostgreSQL roles to create. @end table diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm index edc3198ad50..882543ce5f4 100644 --- a/gnu/services/databases.scm +++ b/gnu/services/databases.scm @@ -9,6 +9,7 @@ ;;; Copyright © 2020, 2022 Marius Bakke <marius <at> gnu.org> ;;; Copyright © 2021 David Larsson <david.larsson <at> selfhosted.xyz> ;;; Copyright © 2021 Aljosha Papsch <ep <at> stern-data.com> +;;; Copyright © 2025 Giacomo Leidi <goodoldpaul <at> autistici.org> ;;; ;;; This file is part of GNU Guix. ;;; @@ -32,6 +33,7 @@ (define-module (gnu services databases) #:autoload (gnu system accounts) (default-shell) #:use-module (gnu packages admin) #:use-module (gnu packages base) + #:use-module (gnu packages bash) #:use-module (gnu packages databases) #:use-module (guix build-system trivial) #:use-module (guix build union) @@ -68,14 +70,18 @@ (define-module (gnu services databases) postgresql-service postgresql-service-type + %default-postgresql-role-shepherd-requirement + postgresql-role postgresql-role? postgresql-role-name + postgresql-role-password-file postgresql-role-permissions postgresql-role-create-database? postgresql-role-configuration postgresql-role-configuration? postgresql-role-configuration-host + postgresql-role-configuration-shepherd-requirement postgresql-role-configuration-roles postgresql-role-service-type @@ -390,6 +396,8 @@ (define-record-type* <postgresql-role> postgresql-role make-postgresql-role postgresql-role? (name postgresql-role-name) ;string + (password-file postgresql-role-password-file ;string + (default #f)) (permissions postgresql-role-permissions (default '(createdb login))) ;list (create-database? postgresql-role-create-database? ;boolean @@ -403,15 +411,20 @@ (define-record-type* <postgresql-role> (template postgresql-role-template ;string (default "template1"))) +(define %default-postgresql-role-shepherd-requirement + '(user-processes postgres)) + (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 + (host postgresql-role-configuration-host ;string + (default "/var/run/postgresql")) + (shepherd-requirement postgresql-role-configuration-shepherd-requirement ;list-of-symbols + (default %default-postgresql-role-shepherd-requirement)) + (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 @@ -425,19 +438,35 @@ (define (postgresql-create-roles config) permissions) " "))) + (define (password-value role) + (string-append "password_" (postgresql-role-name role))) + + (define (role->password-variable role) + (let ((file-name (postgresql-role-password-file role))) + (if (string? file-name) + ;; This way passwords do not leak to the command line. + #~(string-append "-v \"" #$(password-value role) + "=$(" #$coreutils "/bin/cat " #$file-name ")\"") + ""))) + (define (roles->queries roles) (apply mixed-text-file "queries" (append-map (lambda (role) (match-record role <postgresql-role> (name permissions create-database? encoding collation ctype - template) + template password-file) `("SELECT NOT(EXISTS(SELECT 1 FROM pg_catalog.pg_roles WHERE \ rolname = '" ,name "')) as not_exists;\n" "\\gset\n" "\\if :not_exists\n" "CREATE ROLE \"" ,name "\"" " WITH " ,(format-permissions permissions) +,(if (and (string? password-file) + (not (string-null? password-file))) + (string-append + "\nPASSWORD :'" (password-value role) "'") + "") ";\n" ,@(if create-database? `("CREATE DATABASE \"" ,name "\"" @@ -452,20 +481,30 @@ (define (postgresql-create-roles config) (let ((host (postgresql-role-configuration-host config)) (roles (postgresql-role-configuration-roles config))) - #~(let ((psql #$(file-append postgresql "/bin/psql"))) - (list psql "-a" "-h" #$host "-f" #$(roles->queries roles))))) + (program-file "run-queries" + #~(let ((bash #$(file-append bash-minimal "/bin/bash")) + (psql #$(file-append postgresql "/bin/psql"))) + (define command + (string-append + "set -e; exec " psql " -a -h " #$host " -f " + #$(roles->queries roles) " " + (string-join + (list + #$@(map role->password-variable roles)) + " "))) + (execlp bash bash "-c" command))))) (define (postgresql-role-shepherd-service config) (match-record config <postgresql-role-configuration> - (log) + (log shepherd-requirement) (list (shepherd-service - (requirement '(user-processes postgres)) + (requirement shepherd-requirement) (provision '(postgres-roles)) (one-shot? #t) (start #~(lambda args (zero? (spawn-command - #$(postgresql-create-roles config) + (list #$(postgresql-create-roles config)) #:user "postgres" #:group "postgres" ;; XXX: As of Shepherd 1.0.2, #:log-file is not @@ -484,6 +523,7 @@ (define postgresql-role-service-type (match-record config <postgresql-role-configuration> (host roles) (postgresql-role-configuration + (inherit config) (host host) (roles (append roles extended-roles)))))) (default-value (postgresql-role-configuration)) diff --git a/gnu/tests/databases.scm b/gnu/tests/databases.scm index fd5041344b6..84ec2987d68 100644 --- a/gnu/tests/databases.scm +++ b/gnu/tests/databases.scm @@ -1,6 +1,7 @@ ;;; GNU Guix --- Functional package management for GNU ;;; Copyright © 2017 Christopher Baines <mail <at> cbaines.net> ;;; Copyright © 2020, 2022 Marius Bakke <marius <at> gnu.org> +;;; Copyright © 2025 Giacomo Leidi <goodoldpaul <at> autistici.org> ;;; ;;; This file is part of GNU Guix. ;;; @@ -142,6 +143,8 @@ (define %role-log-file (define %postgresql-os (simple-operating-system + (extra-special-file "/password" + (plain-file "password" "hello")) (service postgresql-service-type (postgresql-configuration (postgresql postgresql) @@ -158,6 +161,10 @@ (define %postgresql-os (roles (list (postgresql-role (name "root") + (create-database? #t)) + (postgresql-role + (name "a_database") + (password-file "/password") (create-database? #t)))))))) (define (run-postgresql-test) @@ -230,17 +237,55 @@ (define (run-postgresql-test) (marionette-eval '(begin (use-modules (gnu services herd) + (srfi srfi-1) (ice-9 popen)) (current-output-port (open-file "/dev/console" "w0")) + (every + (lambda (role) + (let* ((port (open-pipe* + OPEN_READ + #$(file-append postgresql "/bin/psql") + "-tA" "-c" + (string-append + "SELECT 1 FROM pg_database WHERE" + " datname='" role "'"))) + (output (get-string-all port))) + (close-pipe port) + (string-contains output "1"))) + '("root" "a_database"))) + marionette)) + + (test-assert "database use fails without a password" + (marionette-eval + '(begin + (use-modules (gnu services herd) + (ice-9 popen)) + (setgid (passwd:gid (getpwnam "alice"))) + (setuid (passwd:uid (getpw "alice"))) + (let ((output + (system* #$(file-append postgresql "/bin/psql") "-tA" "-h" "localhost" "-U" "a_database" "-c" + "SELECT 1 FROM pg_database WHERE datname='a_database'"))) + (not (= output 0)))) + marionette)) + + (test-assert "database passwords are set" + (marionette-eval + '(begin + (use-modules (gnu services herd) + (ice-9 popen)) + (setgid (passwd:gid (getpwnam "alice"))) + (setuid (passwd:uid (getpw "alice"))) + (setenv "PGPASSWORD" + (call-with-input-file "/password" get-string-all)) (let* ((port (open-pipe* OPEN_READ #$(file-append postgresql "/bin/psql") - "-tA" "-c" "SELECT 1 FROM pg_database WHERE - datname='root'")) + "-U" "a_database" "-tA" "-h" "localhost" "-c" + "SELECT 1 FROM pg_database WHERE datname='a_database'")) (output (get-string-all port))) (close-pipe port) - (string-contains output "1"))) + (string=? output "1\n"))) marionette)) (test-end)))) base-commit: 1710c0941db517453ac2b88c0e854e8348172603 -- 2.49.0
Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
:Giacomo Leidi <goodoldpaul <at> autistici.org>
:Message #64 received at 73196-done <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: paul <goodoldpaul <at> autistici.org> Cc: Ludovic Courtès <ludo <at> gnu.org>, 73196-done <at> debbugs.gnu.org Subject: Re: bug#73196: [PATCH] services: postgresql-role: Add support for password files. Date: Fri, 02 May 2025 15:33:40 +0900
Hi Paul, paul <goodoldpaul <at> autistici.org> writes: > Hi Maxim, > > I should have addressed all your remaining comments. I'm about to send > a v8, hopefully this will be the correct one :) thank you a lot for > your help Thanks! I've made a few cosmetic changes, e.g.: --8<---------------cut here---------------start------------->8--- 3 files changed, 18 insertions(+), 18 deletions(-) doc/guix.texi | 5 +++-- gnu/services/databases.scm | 17 +++++++++-------- gnu/tests/databases.scm | 14 ++++++-------- modified doc/guix.texi @@ -27805,8 +27805,9 @@ Database Services @item @code{shepherd-requirement} (default: @code{'(user-processes postgres)}) -Set additional Shepherd services dependencies to the provisioned -Shepherd service. +The Shepherd services dependencies to use. Add extra dependencies to +@code{%default-postgresql-role-shepherd-requirement} to extend its +value. @item @code{roles} (default: @code{'()}) The initial PostgreSQL roles to create. modified gnu/services/databases.scm @@ -417,14 +417,15 @@ (define %default-postgresql-role-shepherd-requirement (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")) - (shepherd-requirement postgresql-role-configuration-shepherd-requirement ;list-of-symbols - (default %default-postgresql-role-shepherd-requirement)) - (log postgresql-role-configuration-log ;string - (default "/var/log/postgresql_roles.log")) - (roles postgresql-role-configuration-roles - (default '()))) ;list + (shepherd-requirement + postgresql-role-configuration-shepherd-requirement ;list-of-symbols + (default %default-postgresql-role-shepherd-requirement)) + (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 modified gnu/tests/databases.scm @@ -259,21 +259,19 @@ (define (run-postgresql-test) (test-assert "database use fails without a password" (marionette-eval '(begin - (use-modules (gnu services herd) - (ice-9 popen)) (setgid (passwd:gid (getpwnam "alice"))) (setuid (passwd:uid (getpw "alice"))) - (let ((output - (system* #$(file-append postgresql "/bin/psql") "-tA" "-h" "localhost" "-U" "a_database" "-c" - "SELECT 1 FROM pg_database WHERE datname='a_database'"))) - (not (= output 0)))) + (not (zero? + (system* #$(file-append postgresql "/bin/psql") + "-tA" "-h" "localhost" "-U" "a_database" "-c" + (string-append "SELECT 1 FROM pg_database " + "WHERE datname='a_database'"))))) marionette)) (test-assert "database passwords are set" (marionette-eval '(begin - (use-modules (gnu services herd) - (ice-9 popen)) + (use-modules (ice-9 popen)) (setgid (passwd:gid (getpwnam "alice"))) (setuid (passwd:uid (getpw "alice"))) (setenv "PGPASSWORD" --8<---------------cut here---------------end--------------->8--- ensured 'make check-system TESTS=postgresql' was still happy, and pushed as commit 9d216d2ae9f. Thank you! -- Thanks, Maxim
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org
.
(Fri, 30 May 2025 11:24:13 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.