[gentoo-dev] [PATCH 1/2] acct-user.eclass: Support ACCT_USER_ID override

[gentoo-dev] [PATCH 1/2] acct-user.eclass: Support ACCT_USER_ID override

[Michał Górny]

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

diff --git a/eclass/acct-user.eclass b/eclass/acct-user.eclass
index 5c55b8092c69..ca5a0b2e6b23 100644
--- a/eclass/acct-user.eclass
+++ b/eclass/acct-user.eclass
@@ -67,7 +67,8 @@ readonly ACCT_USER_NAME
 # @REQUIRED
 # @DESCRIPTION:
 # Preferred UID for the new user.  This variable is obligatory, and its
-# value must be unique across all user packages.
+# value must be unique across all user packages.  This can be overriden
+# in make.conf through ACCT_USER_<UPPERCASE_USERNAME>_ID variable.
 #
 # Overlays should set this to -1 to dynamically allocate UID.  Using -1
 # in ::gentoo is prohibited by policy.
@@ -296,25 +297,33 @@ acct-user_pkg_pretend() {
 
 	# verify ACCT_USER_ID
 	[[ -n ${ACCT_USER_ID} ]] || die "Ebuild error: ACCT_USER_ID must be set!"
-	[[ ${ACCT_USER_ID} -eq -1 ]] && return
-	[[ ${ACCT_USER_ID} -ge 0 ]] || die "Ebuild errors: ACCT_USER_ID=${ACCT_USER_ID} invalid!"
+	[[ ${ACCT_USER_ID} -ge -1 ]] || die "Ebuild error: ACCT_USER_ID=${ACCT_USER_ID} invalid!"
+	local user_id=${ACCT_USER_ID}
+
+	# check for the override
+	local override_name=${ACCT_USER_NAME^^}
+	local override_var=ACCT_USER_${override_name//-/_}_ID
+	if [[ -n ${!override_var} ]]; then
+		user_id=${!override_var}
+		[[ ${user_id} -ge -1 ]] || die "${override_var}=${user_id} invalid!"
+	fi
 
 	# check for ACCT_USER_ID collisions early
-	if [[ -n ${ACCT_USER_ENFORCE_ID} ]]; then
-		local user_by_id=$(egetusername "${ACCT_USER_ID}")
+	if [[ ${user_id} -ne -1 && -n ${ACCT_USER_ENFORCE_ID} ]]; then
+		local user_by_id=$(egetusername "${user_id}")
 		local user_by_name=$(egetent passwd "${ACCT_USER_NAME}")
 		if [[ -n ${user_by_id} ]]; then
 			if [[ ${user_by_id} != ${ACCT_USER_NAME} ]]; then
 				eerror "The required UID is already taken by another user."
-				eerror "  UID: ${ACCT_USER_ID}"
+				eerror "  UID: ${user_id}"
 				eerror "  needed for: ${ACCT_USER_NAME}"
 				eerror "  current user: ${user_by_id}"
-				die "UID ${ACCT_USER_ID} taken already"
+				die "UID ${user_id} taken already"
 			fi
 		elif [[ -n ${user_by_name} ]]; then
 			eerror "The requested user exists already with wrong UID."
 			eerror "  username: ${ACCT_USER_NAME}"
-			eerror "  requested UID: ${ACCT_USER_ID}"
+			eerror "  requested UID: ${user_id}"
 			eerror "  current entry: ${user_by_name}"
 			die "Username ${ACCT_USER_NAME} exists with wrong UID"
 		fi
@@ -335,7 +344,7 @@ acct-user_src_install() {
 	local override_name=${ACCT_USER_NAME^^}
 	override_name=${override_name//-/_}
 	local var
-	for var in ACCT_USER_{SHELL,HOME{,_OWNER,_PERMS},GROUPS}; do
+	for var in ACCT_USER_{ID,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."
@@ -363,7 +372,7 @@ acct-user_src_install() {
 	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/#-*/-}:${groups[0]}" \
+			"${_ACCT_USER_ID/#-*/-}:${groups[0]}" \
 			"${DESCRIPTION//[:,=]/;}" \
 			"${_ACCT_USER_HOME}" \
 			"${_ACCT_USER_SHELL/#-*/-}"
@@ -382,7 +391,7 @@ acct-user_pkg_preinst() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	enewuser ${ACCT_USER_ENFORCE_ID:+-F} -M "${ACCT_USER_NAME}" \
-		"${ACCT_USER_ID}" "${_ACCT_USER_SHELL}" "${_ACCT_USER_HOME}" \
+		"${_ACCT_USER_ID}" "${_ACCT_USER_SHELL}" "${_ACCT_USER_HOME}" \
 		"${_ACCT_USER_GROUPS// /,}"
 
 	if [[ ${_ACCT_USER_HOME} != /dev/null ]]; then
-- 
2.30.0

[gentoo-dev] [PATCH 2/2] acct-group.eclass: Support ACCT_GROUP_ID override

[Michał Górny]

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/acct-group.eclass | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/eclass/acct-group.eclass b/eclass/acct-group.eclass
index 1d5d14290dad..1ccc40218434 100644
--- a/eclass/acct-group.eclass
+++ b/eclass/acct-group.eclass
@@ -59,7 +59,8 @@ readonly ACCT_GROUP_NAME
 # @REQUIRED
 # @DESCRIPTION:
 # Preferred GID for the new group.  This variable is obligatory, and its
-# value must be unique across all group packages.
+# value must be unique across all group packages.  This can be overriden
+# in make.conf through ACCT_GROUP_<UPPERCASE_USERNAME>_ID variable.
 #
 # Overlays should set this to -1 to dynamically allocate GID.  Using -1
 # in ::gentoo is prohibited by policy.
@@ -91,25 +92,33 @@ acct-group_pkg_pretend() {
 
 	# verify ACCT_GROUP_ID
 	[[ -n ${ACCT_GROUP_ID} ]] || die "Ebuild error: ACCT_GROUP_ID must be set!"
-	[[ ${ACCT_GROUP_ID} -eq -1 ]] && return
-	[[ ${ACCT_GROUP_ID} -ge 0 ]] || die "Ebuild errors: ACCT_GROUP_ID=${ACCT_GROUP_ID} invalid!"
+	[[ ${ACCT_GROUP_ID} -ge -1 ]] || die "Ebuild error: ACCT_GROUP_ID=${ACCT_GROUP_ID} invalid!"
+	local group_id=${ACCT_GROUP_ID}
+
+	# check for the override
+	local override_name=${ACCT_GROUP_NAME^^}
+	local override_var=ACCT_GROUP_${override_name//-/_}_ID
+	if [[ -n ${!override_var} ]]; then
+		group_id=${!override_var}
+		[[ ${group_id} -ge -1 ]] || die "${override_var}=${group_id} invalid!"
+	fi
 
 	# check for ACCT_GROUP_ID collisions early
-	if [[ -n ${ACCT_GROUP_ENFORCE_ID} ]]; then
-		local group_by_id=$(egetgroupname "${ACCT_GROUP_ID}")
+	if [[ ${group_id} -ne -1 && -n ${ACCT_GROUP_ENFORCE_ID} ]]; then
+		local group_by_id=$(egetgroupname "${group_id}")
 		local group_by_name=$(egetent group "${ACCT_GROUP_NAME}")
 		if [[ -n ${group_by_id} ]]; then
 			if [[ ${group_by_id} != ${ACCT_GROUP_NAME} ]]; then
 				eerror "The required GID is already taken by another group."
-				eerror "  GID: ${ACCT_GROUP_ID}"
+				eerror "  GID: ${group_id}"
 				eerror "  needed for: ${ACCT_GROUP_NAME}"
 				eerror "  current group: ${group_by_id}"
-				die "GID ${ACCT_GROUP_ID} taken already"
+				die "GID ${group_id} taken already"
 			fi
 		elif [[ -n ${group_by_name} ]]; then
 			eerror "The requested group exists already with wrong GID."
 			eerror "  groupname: ${ACCT_GROUP_NAME}"
-			eerror "  requested GID: ${ACCT_GROUP_ID}"
+			eerror "  requested GID: ${group_id}"
 			eerror "  current entry: ${group_by_name}"
 			die "Group ${ACCT_GROUP_NAME} exists with wrong GID"
 		fi
@@ -122,11 +131,21 @@ acct-group_pkg_pretend() {
 acct-group_src_install() {
 	debug-print-function ${FUNCNAME} "${@}"
 
+	# check for the override
+	local override_name=${ACCT_GROUP_NAME^^}
+	local override_var=ACCT_GROUP_${override_name//-/_}_ID
+	if [[ -n ${!override_var} ]]; then
+		ewarn "${override_var}=${!override_var} override in effect, support will not be provided."
+		_ACCT_GROUP_ID=${!override_var}
+	else
+		_ACCT_GROUP_ID=${ACCT_GROUP_ID}
+	fi
+
 	insinto /usr/lib/sysusers.d
 	newins - ${CATEGORY}-${ACCT_GROUP_NAME}.conf < <(
 		printf "g\t%q\t%q\n" \
 			"${ACCT_GROUP_NAME}" \
-			"${ACCT_GROUP_ID/#-*/-}"
+			"${_ACCT_GROUP_ID/#-*/-}"
 	)
 }
 
@@ -137,7 +156,7 @@ acct-group_pkg_preinst() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	enewgroup ${ACCT_GROUP_ENFORCE_ID:+-F} "${ACCT_GROUP_NAME}" \
-		"${ACCT_GROUP_ID}"
+		"${_ACCT_GROUP_ID}"
 }
 
 fi
-- 
2.30.0

Re: [gentoo-dev] [PATCH 1/2] acct-user.eclass: Support ACCT_USER_ID override

[Ulrich Mueller]

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

>>>>> On Tue, 05 Jan 2021, Michał Górny wrote:

> +	# check for the override
> +	local override_name=${ACCT_USER_NAME^^}
> +	local override_var=ACCT_USER_${override_name//-/_}_ID

I wonder about this line. Both hyphen and underscore are valid
characters in user names.

So, ACCT_USER_FOO_BAR_ID would override the id for both foo_bar and
foo-bar users.

Ulrich

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

Re: [gentoo-dev] [PATCH 1/2] acct-user.eclass: Support ACCT_USER_ID override

[Thomas Deutschmann]

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

Hi,

is there a specific reason why we want to support dynamic variables 
(ACCT_USER_$foo) at all?

Isn't package.env support enough, i.e. use ACCT_USER_ID from environment 
if set (which we should detect and log, maybe this will require a 
different namespace for the variables at all to be able to differentiate 
between values set by acct-* ebuild and user override)?

Of course this won't allow something like `ACCT_USER_ID=42 emerge 
<package which will pull in multiple acct-user/*>` but I am not sure if 
this is an implementation goal.


-- 
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 1/2] acct-user.eclass: Support ACCT_USER_ID override

[Patrick McLean]

On Wed, 6 Jan 2021 15:02:12 +0100
Thomas Deutschmann <whissi@gentoo.org> wrote:

> Hi,
> 
> is there a specific reason why we want to support dynamic variables 
> (ACCT_USER_$foo) at all?
> 
> Isn't package.env support enough, i.e. use ACCT_USER_ID from environment 
> if set (which we should detect and log, maybe this will require a 
> different namespace for the variables at all to be able to differentiate 
> between values set by acct-* ebuild and user override)?
> 
> Of course this won't allow something like `ACCT_USER_ID=42 emerge 
> <package which will pull in multiple acct-user/*>` but I am not sure if 
> this is an implementation goal.

This is so ACCT_USER_$foo can be set in make.conf, and not have to
be specified as an environment variable whenever portage is run. This
helps when automated systems are building Gentoo images or systems.

Re: [gentoo-dev] [PATCH 1/2] acct-user.eclass: Support ACCT_USER_ID override

[Michał Górny]

On Wed, 2021-01-06 at 14:25 +0100, Ulrich Mueller wrote:
> > > > > > On Tue, 05 Jan 2021, Michał Górny wrote:
> 
> > +	# check for the override
> > +	local override_name=${ACCT_USER_NAME^^}
> > +	local override_var=ACCT_USER_${override_name//-/_}_ID
> 
> I wonder about this line. Both hyphen and underscore are valid
> characters in user names.
> 
> So, ACCT_USER_FOO_BAR_ID would override the id for both foo_bar and
> foo-bar users.

I don't think this is the problem we need to be worrying about.  I mean,
if someone actually created user identifiers that differ only be non-
alnum characters, I think that'd the problem to tackle.

-- 
Best regards,
Michał Górny

Re: [gentoo-dev] [PATCH 1/2] acct-user.eclass: Support ACCT_USER_ID override

[Alec Warner]

On Wed, Jan 6, 2021 at 11:05 AM Patrick McLean <chutzpah@gentoo.org> wrote:
>
> On Wed, 6 Jan 2021 15:02:12 +0100
> Thomas Deutschmann <whissi@gentoo.org> wrote:
>
> > Hi,
> >
> > is there a specific reason why we want to support dynamic variables
> > (ACCT_USER_$foo) at all?
> >
> > Isn't package.env support enough, i.e. use ACCT_USER_ID from environment
> > if set (which we should detect and log, maybe this will require a
> > different namespace for the variables at all to be able to differentiate
> > between values set by acct-* ebuild and user override)?
> >
> > Of course this won't allow something like `ACCT_USER_ID=42 emerge
> > <package which will pull in multiple acct-user/*>` but I am not sure if
> > this is an implementation goal.
>
> This is so ACCT_USER_$foo can be set in make.conf, and not have to
> be specified as an environment variable whenever portage is run. This
> helps when automated systems are building Gentoo images or systems.
>

Not sure I follow. Whether your automation sets a variable in
/etc/portage/make.conf or /etc/portage/package.env; it's basically the
same problem space; no?

-A

Re: [gentoo-dev] [PATCH 1/2] acct-user.eclass: Support ACCT_USER_ID override

[Thomas Deutschmann]

On 2021-01-06 20:12, Alec Warner wrote:
> Not sure I follow. Whether your automation sets a variable in
> /etc/portage/make.conf or /etc/portage/package.env; it's basically the
> same problem space; no?

No.

Assuming we will always stick to same variables,

ACCT_USER_ID
ACCT_USER_GROUPS
ACCT_USER_SHELL
ACCT_USER_HOME
ACCT_USER_NAME
...

You don't have to deal with variable names which could clash with other 
stuff. Instead you will only use values which are safe (no need to care 
about stuff like underscores)...

Also, because we are always using same variable names, this will add 
some kind of consistency and makes documentation easier. Like you can 
referrer to same example (template) and just need to adjust values (it's 
actually really hard to get people understand that the example for let's 
say mail-filter/opendkim requires more than just copying and adjusting 
*values*; for instance, we have packages named acct-user/foo but 
*username* is actually food -- do they actually need to override via 
ACCT_USER_<ACCT-USER-PKGNAME-THEY-WANT-TO-OVERRIDE>_ or 
ACCT_USER_<NAME-USED-IN-ACCT-USER-PKG-THEY-WANT-TO-OVERRIDE>_? Sticking 
to same variables names will avoid this confusion).

Like said we will probably need to introduce an own namespace to 
override via environment variable and be able to detect the override to 
have them logged.


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

Re: [gentoo-dev] [PATCH 1/2] acct-user.eclass: Support ACCT_USER_ID override

[Thomas Deutschmann]

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

Hi,

On 2021-01-06 20:05, Patrick McLean wrote:
> This is so ACCT_USER_$foo can be set in make.conf, and not have to
> be specified as an environment variable whenever portage is run. This
> helps when automated systems are building Gentoo images or systems.

Please see my reply to  Alec for more details.

An additional argument I would like to add based on your reply:

We already have package.env mechanism to override stuff. ACCT_USER_$foo 
would introduce an additional way. I wouldn't create an additional way 
for consistency.

But don't get me wrong here. I am just asking and I am always for KISS. 
ACCT_USER_$foo support would create some additional headaches which we 
could avoid from my POV. But I am probably not going to use the override 
feature like I prefer doing stuff like that in configuration management 
tool which would create these users for me exactly the way I want it. 
And it doesn't matter if I apply the role to a Gentoo, Debian, Ubuntu or 
RHEL box... ;)

So I am not blocking ACCT_USER_$foo if anyone really believe it would 
help them.


-- 
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 1/2] acct-user.eclass: Support ACCT_USER_ID override

[Ulrich Mueller]

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

>>>>> On Wed, 06 Jan 2021, Michał Górny wrote:

> On Wed, 2021-01-06 at 14:25 +0100, Ulrich Mueller wrote:
>> I wonder about this line. Both hyphen and underscore are valid
>> characters in user names.
>> 
>> So, ACCT_USER_FOO_BAR_ID would override the id for both foo_bar and
>> foo-bar users.

> I don't think this is the problem we need to be worrying about.  I mean,
> if someone actually created user identifiers that differ only be non-
> alnum characters, I think that'd the problem to tackle.

It is legal to do that, and we already have examples for both hyphen and
underscore in acct-user package names. So the syntax should be able to
cope with it.

A simple mapping from user names (which can contain a hyphen) to
variable names (which cannot) doesn't work and IMHO also violates the
principle of least surprise.

Ulrich

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