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.
View this message in rfc822 format
From: help-debbugs <at> gnu.org (GNU bug Tracking System) To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> Cc: tracker <at> debbugs.gnu.org Subject: bug#73196: closed ([PATCH] services: postgresql-role: Add support for password files.) Date: Fri, 02 May 2025 06:34:01 +0000
[Message part 1 (text/plain, inline)]
Your message dated Fri, 02 May 2025 15:33:40 +0900 with message-id <87ikmj5xob.fsf <at> gmail.com> and subject line Re: bug#73196: [PATCH] services: postgresql-role: Add support for password files. has caused the debbugs.gnu.org bug report #73196, regarding [PATCH] services: postgresql-role: Add support for password files. to be marked as done. (If you believe you have received this mail in error, please contact help-debbugs <at> gnu.org.) -- 73196: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=73196 GNU Bug Tracking System Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
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 +0200This 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
[Message part 3 (message/rfc822, inline)]
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 +0900Hi 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.