Created attachment 247165 [details] Set the login class when starting postgresql I have default class restricted as to number of open files and processes, so this is quite visible to me. /usr/local/etc/rc.d/postgresql initdb already sets the class: postgresql_initdb() { ${su_cmd} -l -c ${postgresql_login_class} ${postgresql_user} -c "exec /usr/local/bin/initdb ${postgresql_initdb_flags} -D ${postgresql_data} -U ${postgresql_user}" } but /usr/local/etc/rc.d/postgresql start is missing it: postgresql_command() { ${su_cmd} -l ${postgresql_user} -c "exec ${command} ${command_args} ${rc_arg}" }
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=16f2f4c4df2ddeefb9612870166e6f15237cc4d4 commit 16f2f4c4df2ddeefb9612870166e6f15237cc4d4 Author: Palle Girgensohn <girgen@FreeBSD.org> AuthorDate: 2023-12-28 10:20:24 +0000 Commit: Palle Girgensohn <girgen@FreeBSD.org> CommitDate: 2023-12-28 10:20:24 +0000 databases/postgresql??-server: Properly set login class at start PR: 275851 (submitted by takeda at takeda.tk) databases/postgresql11-server/Makefile | 2 +- databases/postgresql11-server/files/postgresql.in | 2 +- databases/postgresql12-server/Makefile | 2 +- databases/postgresql12-server/files/postgresql.in | 2 +- databases/postgresql13-server/Makefile | 2 +- databases/postgresql13-server/files/postgresql.in | 2 +- databases/postgresql14-server/Makefile | 2 +- databases/postgresql14-server/files/postgresql.in | 2 +- databases/postgresql15-server/Makefile | 2 +- databases/postgresql15-server/files/postgresql.in | 2 +- databases/postgresql16-server/Makefile | 2 +- databases/postgresql16-server/files/postgresql.in | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-)
Committed. Thanks!
AFAIU, after this patch instead of using the loginclass from /etc/passwd it uses "default" as the default, doesn't it?
It uses the login class from the postgresql_login_class rc parameter, which defaults to "default". Since there are profiles, and there is no way to set either the login_class nor the user specifically for a certain profile, I guess this is not the correct fix. Rather, we should either 1. remove this change and the postgresql_login_class parameter completely, and rely on the login class set in /etc/passwd 2. or we should implement the user parameter for the profiles and remove the login_class parameter 3. or we should implement the login_class parameter for the profiles. I guess my preference at the momnent falls on option 2. Any objections? Thoughts?
(In reply to Palle Girgensohn from comment #4) Please remove the second occurance of -c, it's not needed. The synopsis of su(1) goes like this: su [-] [-c class] [-flms] [login [args]]
(In reply to Palle Girgensohn from comment #4) --- databases/postgresql*-server/files/postgresql.in.orig +++ databases/postgresql*-server/files/postgresql.in @@ -33,7 +33,7 @@ postgresql_flags=${postgresql_flags:-"-w -s -m fast"} postgresql_user=${postgresql_user:-"%%PG_USER%%"} eval postgresql_data=${postgresql_data:-"~${postgresql_user}/data%%PG_VERSION%%"} -postgresql_login_class=${postgresql_login_class:-"default"} +postgresql_login_class=${postgresql_login_class:-"$(/usr/sbin/pw user show ${postgresql_user} | /usr/bin/awk -F: '{print $5}')"} postgresql_initdb_flags=${postgresql_initdb_flags:-"--encoding=utf-8 --lc-collate=C"} name=postgresql @@ -63,6 +63,7 @@ eval postgresql_data="\${postgresql_${profile}_data:-${postgresql_data}}" eval postgresql_flags="\${postgresql_${profile}_flags:-${postgresql_flags}}" eval postgresql_user=\${postgresql_${profile}_user:-${postgresql_user}} + eval postgresql_login_class=\${postgresql_${profile}_login_class:-"$(/usr/sbin/pw user show ${postgresql_user} | /usr/bin/awk -F: '{print $5}')"} eval postgresql_initdb_flags="\${postgresql_${profile}_initdb_flags:-${postgresql_initdb_flags}}" fi else
Regarding removing the extra -c, please verify if it works. I think I originally was removing it and it broke the script. I believe the second -c is passed to /bin/sh
(In reply to takeda from comment #7) If that's the case, assuming su(1) uses getopt(3), the second -c should be preceeded by a double hyphen to terminate getopt(3)'s parsing of the argument vector. If not, I would put the double quotation mark before the second -c to fully visualize its purpose as seen from su(1)'s perspective.
(In reply to Trond Endrestøl from comment #8) I tried using double hyphen and double quotation marks. The latter doesn't work as I had hoped for. I still believe using a double hyphen will better display our intent. $ su -l root -c date Password: Tue Jan 2 08:56:33 CET 2024 $ su -l -- root -c date Password: Tue Jan 2 08:56:42 CET 2024 $ su -l root "-c date" Password: $
(In reply to Vladimir Druzenko from comment #6) a simpler solution would be to just remove the whole "-c ${postgresq_class}" if it is not set.
Created attachment 247530 [details] simple fix for default class How about the attached patch?
(In reply to Palle Girgensohn from comment #10) Indeed.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=4e2b811c04ce0643e2f526f65a0cef442c14d4a0 commit 4e2b811c04ce0643e2f526f65a0cef442c14d4a0 Author: Palle Girgensohn <girgen@FreeBSD.org> AuthorDate: 2024-01-11 21:50:31 +0000 Commit: Palle Girgensohn <girgen@FreeBSD.org> CommitDate: 2024-01-11 21:50:31 +0000 databases/postgresql??-server: honour login class set in /etc/passwd If postgresql_login_class is not set, honour the setting in /etc/passwd. The previous commit ignored the passwd setting and set the login class to "default" if it was left unset. PR: 275851 databases/postgresql12-server/Makefile | 2 +- .../files/pkg-message-server.in | 22 ++++++++++--------- databases/postgresql12-server/files/postgresql.in | 25 ++++++++++++++-------- databases/postgresql13-server/Makefile | 2 +- .../files/pkg-message-server.in | 22 ++++++++++--------- databases/postgresql13-server/files/postgresql.in | 25 ++++++++++++++-------- databases/postgresql14-server/Makefile | 2 +- .../files/pkg-message-server.in | 22 ++++++++++--------- databases/postgresql14-server/files/postgresql.in | 25 ++++++++++++++-------- databases/postgresql15-server/Makefile | 2 +- .../files/pkg-message-server.in | 22 ++++++++++--------- databases/postgresql15-server/files/postgresql.in | 25 ++++++++++++++-------- databases/postgresql16-server/Makefile | 2 +- .../files/pkg-message-server.in | 22 ++++++++++--------- databases/postgresql16-server/files/postgresql.in | 25 ++++++++++++++-------- 15 files changed, 145 insertions(+), 100 deletions(-)