[gentoo-dev] [PATCH] acct-user.eclass: Support var overrides for user properties

[gentoo-dev] [PATCH] acct-user.eclass: Support var overrides for user properties

[Michał Górny]

Introduce a few variables to allow easy overrides of common user account
proprerties, that is:

- ACCT_USER_<username>_SHELL
- ACCT_USER_<username>_HOME
- ACCT_USER_<username>_HOME_OWNER
- ACCT_USER_<username>_HOME_PERMS
- ACCT_USER_<username>_GROUPS
- ACCT_USER_<username>_GROUPS_ADD

The first five variables override the respective ACCT_USER_* variables,
with ACCT_USER_*_GROUPS being a space-separated list.  The *_GROUPS_ADD
variable appends to groups present in the ebuild, as this seems a common
necessity.

We do realize that the original requirement of overriding ebuilds
in a local repository was inconvenient.  This new logic should permit
easy updates via make.conf.  Additionally, it has the advantage
of clearly reporting the changes made in the build logs.

This does not preclude other solutions to the problem.  However, this
is probably the best one and it should become the current
recommendation.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/acct-user.eclass | 84 +++++++++++++++++++++++++++++------------
 1 file changed, 60 insertions(+), 24 deletions(-)

diff --git a/eclass/acct-user.eclass b/eclass/acct-user.eclass
index 22b0038fbff7..5c55b8092c69 100644
--- a/eclass/acct-user.eclass
+++ b/eclass/acct-user.eclass
@@ -82,7 +82,8 @@ readonly ACCT_USER_NAME
 # @ECLASS-VARIABLE: ACCT_USER_SHELL
 # @DESCRIPTION:
 # The shell to use for the user.  If not specified, a 'nologin' variant
-# for the system is used.
+# for the system is used.  This can be overriden in make.conf through
+# ACCT_USER_<UPPERCASE_USERNAME>_SHELL variable.
 : ${ACCT_USER_SHELL:=-1}
 
 # @ECLASS-VARIABLE: ACCT_USER_HOME
@@ -90,6 +91,8 @@ readonly ACCT_USER_NAME
 # The home directory for the user.  If not specified, /dev/null is used.
 # The directory will be created with appropriate permissions if it does
 # not exist.  When updating, existing home directory will not be moved.
+# This can be overriden in make.conf through
+# ACCT_USER_<UPPERCASE_USERNAME>_HOME variable.
 : ${ACCT_USER_HOME:=/dev/null}
 
 # @ECLASS-VARIABLE: ACCT_USER_HOME_OWNER
@@ -97,11 +100,14 @@ readonly ACCT_USER_NAME
 # @DESCRIPTION:
 # The ownership to use for the home directory, in chown ([user][:group])
 # syntax.  Defaults to the newly created user, and its primary group.
+# This can be overriden in make.conf through
+# ACCT_USER_<UPPERCASE_USERNAME>_HOME_OWNER variable.
 
 # @ECLASS-VARIABLE: ACCT_USER_HOME_PERMS
 # @DESCRIPTION:
 # The permissions to use for the home directory, in chmod (octal
-# or verbose) form.
+# or verbose) form.  This can be overriden in make.conf through
+# ACCT_USER_<UPPERCASE_USERNAME>_HOME_PERMS variable.
 : ${ACCT_USER_HOME_PERMS:=0755}
 
 # @ECLASS-VARIABLE: ACCT_USER_GROUPS
@@ -110,6 +116,12 @@ readonly ACCT_USER_NAME
 # List of groups the user should belong to.  This must be a bash
 # array.  The first group specified is the user's primary group, while
 # the remaining groups (if any) become supplementary groups.
+#
+# This can be overriden in make.conf through
+# ACCT_USER_<UPPERCASE_USERNAME>_GROUPS variable, or appended to
+# via ACCT_USER_<UPPERCASE_USERNAME>_GROUPS_ADD.  Please note that
+# due to technical limitations, the override variables are not arrays
+# but space-separated lists.
 
 
 # << Boilerplate ebuild variables >>
@@ -316,23 +328,48 @@ acct-user_pkg_pretend() {
 acct-user_src_install() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	if [[ ${ACCT_USER_HOME} != /dev/null ]]; then
+	# serialize for override support
+	local ACCT_USER_GROUPS=${ACCT_USER_GROUPS[*]}
+
+	# support make.conf overrides
+	local override_name=${ACCT_USER_NAME^^}
+	override_name=${override_name//-/_}
+	local var
+	for var in ACCT_USER_{SHELL,HOME{,_OWNER,_PERMS},GROUPS}; do
+		local var_name=ACCT_USER_${override_name}_${var#ACCT_USER_}
+		if [[ -n ${!var_name} ]]; then
+			ewarn "${var_name}=${!var_name} override in effect, support will not be provided."
+		else
+			var_name=${var}
+		fi
+		declare -g "_${var}=${!var_name}"
+	done
+	var_name=ACCT_USER_${override_name}_GROUPS_ADD
+	if [[ -n ${!var_name} ]]; then
+		ewarn "${var_name}=${!var_name} override in effect, support will not be provided."
+		_ACCT_USER_GROUPS+=" ${!var_name}"
+	fi
+
+	# deserialize into an array
+	local groups=( ${_ACCT_USER_GROUPS} )
+
+	if [[ ${_ACCT_USER_HOME} != /dev/null ]]; then
 		# note: we can't set permissions here since the user isn't
 		# created yet
-		keepdir "${ACCT_USER_HOME}"
+		keepdir "${_ACCT_USER_HOME}"
 	fi
 
 	insinto /usr/lib/sysusers.d
 	newins - ${CATEGORY}-${ACCT_USER_NAME}.conf < <(
 		printf "u\t%q\t%q\t%q\t%q\t%q\n" \
 			"${ACCT_USER_NAME}" \
-			"${ACCT_USER_ID/#-*/-}:${ACCT_USER_GROUPS[0]}" \
+			"${ACCT_USER_ID/#-*/-}:${groups[0]}" \
 			"${DESCRIPTION//[:,=]/;}" \
-			"${ACCT_USER_HOME}" \
-			"${ACCT_USER_SHELL/#-*/-}"
-		if [[ ${#ACCT_USER_GROUPS[@]} -gt 1 ]]; then
+			"${_ACCT_USER_HOME}" \
+			"${_ACCT_USER_SHELL/#-*/-}"
+		if [[ ${#groups[@]} -gt 1 ]]; then
 			printf "m\t${ACCT_USER_NAME}\t%q\n" \
-				"${ACCT_USER_GROUPS[@]:1}"
+				"${groups[@]:1}"
 		fi
 	)
 }
@@ -344,26 +381,26 @@ acct-user_src_install() {
 acct-user_pkg_preinst() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	local groups=${ACCT_USER_GROUPS[*]}
 	enewuser ${ACCT_USER_ENFORCE_ID:+-F} -M "${ACCT_USER_NAME}" \
-		"${ACCT_USER_ID}" "${ACCT_USER_SHELL}" "${ACCT_USER_HOME}" \
-		"${groups// /,}"
+		"${ACCT_USER_ID}" "${_ACCT_USER_SHELL}" "${_ACCT_USER_HOME}" \
+		"${_ACCT_USER_GROUPS// /,}"
 
-	if [[ ${ACCT_USER_HOME} != /dev/null ]]; then
+	if [[ ${_ACCT_USER_HOME} != /dev/null ]]; then
 		# default ownership to user:group
-		if [[ -z ${ACCT_USER_HOME_OWNER} ]]; then
-			ACCT_USER_HOME_OWNER=${ACCT_USER_NAME}:${ACCT_USER_GROUPS[0]}
+		if [[ -z ${_ACCT_USER_HOME_OWNER} ]]; then
+			local group_array=( ${_ACCT_USER_GROUPS} )
+			_ACCT_USER_HOME_OWNER=${ACCT_USER_NAME}:${group_array[0]}
 		fi
 		# Path might be missing due to INSTALL_MASK, etc.
 		# https://bugs.gentoo.org/691478
-		if [[ ! -e "${ED}/${ACCT_USER_HOME#/}" ]]; then
+		if [[ ! -e "${ED}/${_ACCT_USER_HOME#/}" ]]; then
 			eerror "Home directory is missing from the installation image:"
-			eerror "  ${ACCT_USER_HOME}"
+			eerror "  ${_ACCT_USER_HOME}"
 			eerror "Check INSTALL_MASK for entries that would cause this."
-			die "${ACCT_USER_HOME} does not exist"
+			die "${_ACCT_USER_HOME} does not exist"
 		fi
-		fowners "${ACCT_USER_HOME_OWNER}" "${ACCT_USER_HOME}"
-		fperms "${ACCT_USER_HOME_PERMS}" "${ACCT_USER_HOME}"
+		fowners "${_ACCT_USER_HOME_OWNER}" "${_ACCT_USER_HOME}"
+		fperms "${_ACCT_USER_HOME_PERMS}" "${_ACCT_USER_HOME}"
 	fi
 }
 
@@ -380,10 +417,9 @@ acct-user_pkg_postinst() {
 	fi
 
 	# NB: eset* functions check current value
-	esethome "${ACCT_USER_NAME}" "${ACCT_USER_HOME}"
-	esetshell "${ACCT_USER_NAME}" "${ACCT_USER_SHELL}"
-	local groups=${ACCT_USER_GROUPS[*]}
-	esetgroups "${ACCT_USER_NAME}" "${groups// /,}"
+	esethome "${ACCT_USER_NAME}" "${_ACCT_USER_HOME}"
+	esetshell "${ACCT_USER_NAME}" "${_ACCT_USER_SHELL}"
+	esetgroups "${ACCT_USER_NAME}" "${_ACCT_USER_GROUPS// /,}"
 	# comment field can not contain colons
 	esetcomment "${ACCT_USER_NAME}" "${DESCRIPTION//[:,=]/;}"
 	eunlockuser "${ACCT_USER_NAME}"
-- 
2.30.0

Re: [gentoo-dev] [PATCH] acct-user.eclass: Support var overrides for user properties

[Thomas Deutschmann]

[-- Attachment #1.1: Type: text/plain, Size: 1389 bytes --]

On 2021-01-04 18:08, Michał Górny wrote:
> Introduce a few variables to allow easy overrides of common user account
> proprerties, that is:
> 
> - ACCT_USER_<username>_SHELL
> - ACCT_USER_<username>_HOME
> - ACCT_USER_<username>_HOME_OWNER
> - ACCT_USER_<username>_HOME_PERMS
> - ACCT_USER_<username>_GROUPS
> - ACCT_USER_<username>_GROUPS_ADD
> 
> The first five variables override the respective ACCT_USER_* variables,
> with ACCT_USER_*_GROUPS being a space-separated list.  The *_GROUPS_ADD
> variable appends to groups present in the ebuild, as this seems a common
> necessity.
> 
> We do realize that the original requirement of overriding ebuilds
> in a local repository was inconvenient.  This new logic should permit
> easy updates via make.conf.  Additionally, it has the advantage
> of clearly reporting the changes made in the build logs.
> 
> This does not preclude other solutions to the problem.  However, this
> is probably the best one and it should become the current
> recommendation.

This will improve the overlay situation and can be seen as overall 
improvement but it doesn't address any shared concerns nor is it a 
replacement for the proposed 'acct-user.eclass: don't modify existing 
user by default' patch.


-- 
Regards,
Thomas Deutschmann / Gentoo Linux Developer
fpr: C4DD 695F A713 8F24 2AA1 5638 5849 7EE5 1D5D 74A5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [gentoo-dev] [PATCH] acct-user.eclass: Support var overrides for user properties

[Alec Warner]

On Mon, Jan 4, 2021 at 9:15 AM Thomas Deutschmann <whissi@gentoo.org> wrote:
>
> On 2021-01-04 18:08, Michał Górny wrote:
> > Introduce a few variables to allow easy overrides of common user account
> > proprerties, that is:
> >
> > - ACCT_USER_<username>_SHELL
> > - ACCT_USER_<username>_HOME
> > - ACCT_USER_<username>_HOME_OWNER
> > - ACCT_USER_<username>_HOME_PERMS
> > - ACCT_USER_<username>_GROUPS
> > - ACCT_USER_<username>_GROUPS_ADD
> >
> > The first five variables override the respective ACCT_USER_* variables,
> > with ACCT_USER_*_GROUPS being a space-separated list.  The *_GROUPS_ADD
> > variable appends to groups present in the ebuild, as this seems a common
> > necessity.
> >
> > We do realize that the original requirement of overriding ebuilds
> > in a local repository was inconvenient.  This new logic should permit
> > easy updates via make.conf.  Additionally, it has the advantage
> > of clearly reporting the changes made in the build logs.
> >
> > This does not preclude other solutions to the problem.  However, this
> > is probably the best one and it should become the current
> > recommendation.
>
> This will improve the overlay situation and can be seen as overall
> improvement but it doesn't address any shared concerns nor is it a
> replacement for the proposed 'acct-user.eclass: don't modify existing
> user by default' patch.
>

Same response from me, merge it but please also merge the other patch.

-A

>
> --
> Regards,
> Thomas Deutschmann / Gentoo Linux Developer
> fpr: C4DD 695F A713 8F24 2AA1 5638 5849 7EE5 1D5D 74A5
>

Re: [gentoo-dev] [PATCH] acct-user.eclass: Support var overrides for user properties

[Michał Górny]

On Mon, 2021-01-04 at 18:14 +0100, Thomas Deutschmann wrote:
> On 2021-01-04 18:08, Michał Górny wrote:
> > Introduce a few variables to allow easy overrides of common user account
> > proprerties, that is:
> > 
> > - ACCT_USER_<username>_SHELL
> > - ACCT_USER_<username>_HOME
> > - ACCT_USER_<username>_HOME_OWNER
> > - ACCT_USER_<username>_HOME_PERMS
> > - ACCT_USER_<username>_GROUPS
> > - ACCT_USER_<username>_GROUPS_ADD
> > 
> > The first five variables override the respective ACCT_USER_* variables,
> > with ACCT_USER_*_GROUPS being a space-separated list.  The *_GROUPS_ADD
> > variable appends to groups present in the ebuild, as this seems a common
> > necessity.
> > 
> > We do realize that the original requirement of overriding ebuilds
> > in a local repository was inconvenient.  This new logic should permit
> > easy updates via make.conf.  Additionally, it has the advantage
> > of clearly reporting the changes made in the build logs.
> > 
> > This does not preclude other solutions to the problem.  However, this
> > is probably the best one and it should become the current
> > recommendation.
> 
> This will improve the overlay situation and can be seen as overall 
> improvement but it doesn't address any shared concerns nor is it a 
> replacement for the proposed 'acct-user.eclass: don't modify existing 
> user by default' patch.

If you read the commit message you'd realize I said that it's not
a replacement.

-- 
Best regards,
Michał Górny

Re: [gentoo-dev] [PATCH] acct-user.eclass: Support var overrides for user properties

[Patrick McLean]

On Mon,  4 Jan 2021 18:08:02 +0100
Michał Górny <mgorny@gentoo.org> wrote:

> Introduce a few variables to allow easy overrides of common user account
> proprerties, that is:
> 
> - ACCT_USER_<username>_SHELL
> - ACCT_USER_<username>_HOME
> - ACCT_USER_<username>_HOME_OWNER
> - ACCT_USER_<username>_HOME_PERMS
> - ACCT_USER_<username>_GROUPS
> - ACCT_USER_<username>_GROUPS_ADD

Please also add a way to override the UID/GID for the user/group.

> The first five variables override the respective ACCT_USER_* variables,
> with ACCT_USER_*_GROUPS being a space-separated list.  The *_GROUPS_ADD
> variable appends to groups present in the ebuild, as this seems a common
> necessity.

Re: [gentoo-dev] [PATCH] acct-user.eclass: Support var overrides for user properties

[Michał Górny]

On Mon, 2021-01-04 at 14:58 -0800, Patrick McLean wrote:
> On Mon,  4 Jan 2021 18:08:02 +0100
> Michał Górny <mgorny@gentoo.org> wrote:
> 
> > Introduce a few variables to allow easy overrides of common user account
> > proprerties, that is:
> > 
> > - ACCT_USER_<username>_SHELL
> > - ACCT_USER_<username>_HOME
> > - ACCT_USER_<username>_HOME_OWNER
> > - ACCT_USER_<username>_HOME_PERMS
> > - ACCT_USER_<username>_GROUPS
> > - ACCT_USER_<username>_GROUPS_ADD
> 
> Please also add a way to override the UID/GID for the user/group.

Damn it, and I thought I'd avoid that ;-).  But do we really need it? 
The eclass doesn't enforce UID/GID by default if the user exists
already, so it's a bit tangential to the original problem.

-- 
Best regards,
Michał Górny

Re: [gentoo-dev] [PATCH] acct-user.eclass: Support var overrides for user properties

[Patrick McLean]

On Tue, 05 Jan 2021 00:16:49 +0100
Michał Górny <mgorny@gentoo.org> wrote:

> On Mon, 2021-01-04 at 14:58 -0800, Patrick McLean wrote:
> > On Mon,  4 Jan 2021 18:08:02 +0100
> > Michał Górny <mgorny@gentoo.org> wrote:
> >   
> > > Introduce a few variables to allow easy overrides of common user account
> > > proprerties, that is:
> > > 
> > > - ACCT_USER_<username>_SHELL
> > > - ACCT_USER_<username>_HOME
> > > - ACCT_USER_<username>_HOME_OWNER
> > > - ACCT_USER_<username>_HOME_PERMS
> > > - ACCT_USER_<username>_GROUPS
> > > - ACCT_USER_<username>_GROUPS_ADD  
> > 
> > Please also add a way to override the UID/GID for the user/group.  
> 
> Damn it, and I thought I'd avoid that ;-).  But do we really need it? 
> The eclass doesn't enforce UID/GID by default if the user exists
> already, so it's a bit tangential to the original problem.
> 

The user needs to already exist for that to be helpful. When one using
automation to build/deploy large numbers of Gentoo systems, it's quite
useful to have control over that sort of things. At the moment, the
only way is to fork the ebuilds, which of course means they need to be
kept in sync.

Re: [gentoo-dev] [PATCH] acct-user.eclass: Support var overrides for user properties

[Michał Górny]

On Mon, 2021-01-04 at 15:50 -0800, Patrick McLean wrote:
> On Tue, 05 Jan 2021 00:16:49 +0100
> Michał Górny <mgorny@gentoo.org> wrote:
> 
> > On Mon, 2021-01-04 at 14:58 -0800, Patrick McLean wrote:
> > > On Mon,  4 Jan 2021 18:08:02 +0100
> > > Michał Górny <mgorny@gentoo.org> wrote:
> > >   
> > > 
> > > 
> > > 
> > > > Introduce a few variables to allow easy overrides of common user account
> > > > proprerties, that is:
> > > > 
> > > > - ACCT_USER_<username>_SHELL
> > > > - ACCT_USER_<username>_HOME
> > > > - ACCT_USER_<username>_HOME_OWNER
> > > > - ACCT_USER_<username>_HOME_PERMS
> > > > - ACCT_USER_<username>_GROUPS
> > > > - ACCT_USER_<username>_GROUPS_ADD  
> > > 
> > > Please also add a way to override the UID/GID for the user/group.  
> > 
> > Damn it, and I thought I'd avoid that ;-).  But do we really need it? 
> > The eclass doesn't enforce UID/GID by default if the user exists
> > already, so it's a bit tangential to the original problem.
> > 
> 
> The user needs to already exist for that to be helpful. When one using
> automation to build/deploy large numbers of Gentoo systems, it's quite
> useful to have control over that sort of things. At the moment, the
> only way is to fork the ebuilds, which of course means they need to be
> kept in sync.

Ok, I'll keep that mind.  However, I suppose you won't mind me
addressing that separately?  Unlike the patch sent, ID-related logic
needs to be done twice (due to pkg_pretend).  Ideally, could you report
a feature request on Bugzilla?


-- 
Best regards,
Michał Górny

Re: [gentoo-dev] [PATCH] acct-user.eclass: Support var overrides for user properties

[Patrick McLean]

On Tue, 05 Jan 2021 00:54:58 +0100
Michał Górny <mgorny@gentoo.org> wrote:

> On Mon, 2021-01-04 at 15:50 -0800, Patrick McLean wrote:
> > On Tue, 05 Jan 2021 00:16:49 +0100
> > Michał Górny <mgorny@gentoo.org> wrote:
> > > On Mon, 2021-01-04 at 14:58 -0800, Patrick McLean wrote:  
> > > > On Mon,  4 Jan 2021 18:08:02 +0100
> > > > Michał Górny <mgorny@gentoo.org> wrote:
> > > > > Introduce a few variables to allow easy overrides of common user account
> > > > > proprerties, that is:
> > > > > 
> > > > > - ACCT_USER_<username>_SHELL
> > > > > - ACCT_USER_<username>_HOME
> > > > > - ACCT_USER_<username>_HOME_OWNER
> > > > > - ACCT_USER_<username>_HOME_PERMS
> > > > > - ACCT_USER_<username>_GROUPS
> > > > > - ACCT_USER_<username>_GROUPS_ADD    
> > > > 
> > > > Please also add a way to override the UID/GID for the user/group.    
> > > 
> > > Damn it, and I thought I'd avoid that ;-).  But do we really need it? 
> > > The eclass doesn't enforce UID/GID by default if the user exists
> > > already, so it's a bit tangential to the original problem.
> > >   
> > 
> > The user needs to already exist for that to be helpful. When one using
> > automation to build/deploy large numbers of Gentoo systems, it's quite
> > useful to have control over that sort of things. At the moment, the
> > only way is to fork the ebuilds, which of course means they need to be
> > kept in sync.  
> 
> Ok, I'll keep that mind.  However, I suppose you won't mind me
> addressing that separately?  Unlike the patch sent, ID-related logic
> needs to be done twice (due to pkg_pretend).  Ideally, could you report
> a feature request on Bugzilla?

Sure, I don't mind it being addressed separately. I created a feature
request on Bugzilla: https://bugs.gentoo.org/763615