All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 0/6] Improvements for useradd-staticids.bbclass
@ 2015-12-18 23:53 Peter Kjellerstedt
  2015-12-18 23:53 ` [PATCHv2 1/6] useradd-staticids.bbclass: Treat mutually exclusive options as such Peter Kjellerstedt
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Peter Kjellerstedt @ 2015-12-18 23:53 UTC (permalink / raw)
  To: openembedded-core

It is now 1.5 months since I originally supplied these patches. The
only comment I received was about a couple of unnecessary spaces that
I had removed. After explaining why removing them should not make any
change to the implementation (more than a month ago), I have not
received any response at all even after pushing for it on the list (a
month ago) and in private (two weeks ago).

Since I am now fed up with waithing for any response, I have taken the
time to split out the removal of those three spaces into a separate
patch in the vain hope that this will make the patches suitable to be
applied...

Now I will go on vacation till the beginning of the next year, hoping
that the patches have been applied when I get back. So until then,
Merry Christmas and a Happy New Year!

Original patch set description below.

This series of patches aims to improve useradd-staticids.bbclass.

We are currently using useradd-staticids.bbclass to make sure all
users and groups have well defined IDs. So far we have had the
definitions of the users both in the recipes and in the passwd file
used by useradd-staticids.bbclass. Since we have a huge number of
recipes that create users, having to duplicate the definitions all
over the place has turned out to be a burden we should be able to
avoid.

So the current plan for us is to have one passwd file per layer with
the definitions of all users that layer needs. These definitions do
not include the static IDs for the users. Instead the static IDs for
the users are specified in a distro specific passwd-static file. There
is also a distro specific group-static file for the group IDs. With
that in place it should be enough to define a user as:

USERADD_PARAM_${PN} = "--system foobar"

in a recipe and let useradd-staticids.bbclass handle the specifics for
how that user should be defined.

The above worked fine for all users that had a primary group with the
same name as the user. However, it turned out that for users that
wanted some other primary group, specifying it in the passwd file was
not enough. We still had to add --gid <some group> in the recipe where
<some group> had to match what was specified in the passwd file. This
was less than optimal, and somewhat defeated the setup.

It also turned out that for users with a primary group that does not
match the user name, useradd-staticids.bbclass would still add the
creation of a group with the same name as the user (when it parsed the
passwd-static file) and the add another creation of the correct group
(when it parsed the passwd file).

So after spending quite a lot of time on trying to decode how
rewrite_useradd() calculated the --gid option, I came up with this
series of changes that should correct the problems described above and
make the code easier to understand while (hopefully) maintaining
compatibility with the old code.

//Peter

The following changes since commit 5f406915b5e26761faa4ea5e0edd887ac5ae6e2f:

  bitbake: toaster: remove 2 confusing parameters (2015-12-18 13:51:54 +0000)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib pkj/useradd_improvements
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=pkj/useradd_improvements

Peter Kjellerstedt (6):
  useradd-staticids.bbclass: Treat mutually exclusive options as such
  useradd-staticids.bbclass: Make --no-user-group have effect
  useradd-staticids.bbclass: Simplify some logic
  useradd-staticids.bbclass: Simplify the logic for when to add groups
  useradd-staticids.bbclass: Read passwd/group files before parsing
  useradd-staticids.bbclass: Remove unnecessary spaces

 meta/classes/useradd-staticids.bbclass | 192 ++++++++++++++++++---------------
 1 file changed, 103 insertions(+), 89 deletions(-)

-- 
2.1.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCHv2 1/6] useradd-staticids.bbclass: Treat mutually exclusive options as such
  2015-12-18 23:53 [PATCHv2 0/6] Improvements for useradd-staticids.bbclass Peter Kjellerstedt
@ 2015-12-18 23:53 ` Peter Kjellerstedt
  2015-12-18 23:53 ` [PATCHv2 2/6] useradd-staticids.bbclass: Make --no-user-group have effect Peter Kjellerstedt
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Peter Kjellerstedt @ 2015-12-18 23:53 UTC (permalink / raw)
  To: openembedded-core

The useradd options --create-home/--no-create-home and
--user-group/--no-user-group are mutually exclusive and should be
treated as such.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/classes/useradd-staticids.bbclass | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/meta/classes/useradd-staticids.bbclass b/meta/classes/useradd-staticids.bbclass
index 924d6ea..9d59aca 100644
--- a/meta/classes/useradd-staticids.bbclass
+++ b/meta/classes/useradd-staticids.bbclass
@@ -37,21 +37,21 @@ def update_useradd_static_config(d):
         parser.add_argument("-k", "--skel", metavar="SKEL_DIR", help="use this alternative skeleton directory")
         parser.add_argument("-K", "--key", metavar="KEY=VALUE", help="override /etc/login.defs defaults")
         parser.add_argument("-l", "--no-log-init", help="do not add the user to the lastlog and faillog databases", action="store_true")
-        parser.add_argument("-m", "--create-home", help="create the user's home directory", action="store_true")
-        parser.add_argument("-M", "--no-create-home", help="do not create the user's home directory", action="store_true")
-        parser.add_argument("-N", "--no-user-group", help="do not create a group with the same name as the user", action="store_true")
+        parser.add_argument("-m", "--create-home", help="create the user's home directory", action="store_const", const=True)
+        parser.add_argument("-M", "--no-create-home", dest="create_home", help="do not create the user's home directory", action="store_const", const=False)
+        parser.add_argument("-N", "--no-user-group", dest="user_group", help="do not create a group with the same name as the user", action="store_const", const=False)
         parser.add_argument("-o", "--non-unique", help="allow to create users with duplicate (non-unique UID)", action="store_true")
         parser.add_argument("-p", "--password", metavar="PASSWORD", help="encrypted password of the new account")
         parser.add_argument("-R", "--root", metavar="CHROOT_DIR", help="directory to chroot into")
         parser.add_argument("-r", "--system", help="create a system account", action="store_true")
         parser.add_argument("-s", "--shell", metavar="SHELL", help="login shell of the new account")
         parser.add_argument("-u", "--uid", metavar="UID", help="user ID of the new account")
-        parser.add_argument("-U", "--user-group", help="create a group with the same name as the user", action="store_true")
+        parser.add_argument("-U", "--user-group", help="create a group with the same name as the user", action="store_const", const=True)
         parser.add_argument("LOGIN", help="Login name of the new user")
 
         # Return a list of configuration files based on either the default
         # files/passwd or the contents of USERADD_UID_TABLES
-        # paths are resulved via BBPATH
+        # paths are resolved via BBPATH
         def get_passwd_list(d):
             str = ""
             bbpath = d.getVar('BBPATH', True)
@@ -106,7 +106,8 @@ def update_useradd_static_config(d):
                             # So if the implicit username-group creation is on, then the implicit groupname (LOGIN)
                             # is used, and we disable the user_group option.
                             #
-                            uaargs.groupname = [uaargs.gid, uaargs.LOGIN][not uaargs.gid or uaargs.user_group]
+                            user_group = uaargs.user_group is None or uaargs.user_group is True
+                            uaargs.groupname = [uaargs.gid, uaargs.LOGIN][not uaargs.gid or user_group]
                             uaargs.groupid = [uaargs.gid, uaargs.groupname][not uaargs.gid]
                             uaargs.groupid = [field[3], uaargs.groupid][not field[3]]
 
@@ -120,7 +121,7 @@ def update_useradd_static_config(d):
                                     # We don't have a number, so we have to add a name
                                     bb.debug(1, "Adding group %s!" % (uaargs.groupname))
                                     uaargs.gid = uaargs.groupid
-                                    uaargs.user_group = False
+                                    uaargs.user_group = None
                                     groupadd = d.getVar("GROUPADD_PARAM_%s" % pkg, True)
                                     newgroup = "%s %s" % (['', ' --system'][uaargs.system], uaargs.groupname)
                                     if groupadd:
@@ -131,7 +132,7 @@ def update_useradd_static_config(d):
                                     # We have a group name and a group number to assign it to
                                     bb.debug(1, "Adding group %s  gid (%s)!" % (uaargs.groupname, uaargs.groupid))
                                     uaargs.gid = uaargs.groupid
-                                    uaargs.user_group = False
+                                    uaargs.user_group = None
                                     groupadd = d.getVar("GROUPADD_PARAM_%s" % pkg, True)
                                     newgroup = "-g %s %s" % (uaargs.gid, uaargs.groupname)
                                     if groupadd:
@@ -161,16 +162,16 @@ def update_useradd_static_config(d):
             newparam += ['', ' --skel %s' % uaargs.skel][uaargs.skel != None]
             newparam += ['', ' --key %s' % uaargs.key][uaargs.key != None]
             newparam += ['', ' --no-log-init'][uaargs.no_log_init]
-            newparam += ['', ' --create-home'][uaargs.create_home]
-            newparam += ['', ' --no-create-home'][uaargs.no_create_home]
-            newparam += ['', ' --no-user-group'][uaargs.no_user_group]
+            newparam += ['', ' --create-home'][uaargs.create_home is True]
+            newparam += ['', ' --no-create-home'][uaargs.create_home is False]
+            newparam += ['', ' --no-user-group'][uaargs.user_group is False]
             newparam += ['', ' --non-unique'][uaargs.non_unique]
             newparam += ['', ' --password %s' % uaargs.password][uaargs.password != None]
             newparam += ['', ' --root %s' % uaargs.root][uaargs.root != None]
             newparam += ['', ' --system'][uaargs.system]
             newparam += ['', ' --shell %s' % uaargs.shell][uaargs.shell != None]
             newparam += ['', ' --uid %s' % uaargs.uid][uaargs.uid != None]
-            newparam += ['', ' --user-group'][uaargs.user_group]
+            newparam += ['', ' --user-group'][uaargs.user_group is True]
             newparam += ' %s' % uaargs.LOGIN
 
             newparams.append(newparam)
@@ -192,7 +193,7 @@ def update_useradd_static_config(d):
 
         # Return a list of configuration files based on either the default
         # files/group or the contents of USERADD_GID_TABLES
-        # paths are resulved via BBPATH
+        # paths are resolved via BBPATH
         def get_group_list(d):
             str = ""
             bbpath = d.getVar('BBPATH', True)
-- 
2.1.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCHv2 2/6] useradd-staticids.bbclass: Make --no-user-group have effect
  2015-12-18 23:53 [PATCHv2 0/6] Improvements for useradd-staticids.bbclass Peter Kjellerstedt
  2015-12-18 23:53 ` [PATCHv2 1/6] useradd-staticids.bbclass: Treat mutually exclusive options as such Peter Kjellerstedt
@ 2015-12-18 23:53 ` Peter Kjellerstedt
  2015-12-18 23:53 ` [PATCHv2 3/6] useradd-staticids.bbclass: Simplify some logic Peter Kjellerstedt
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Peter Kjellerstedt @ 2015-12-18 23:53 UTC (permalink / raw)
  To: openembedded-core

If --no-user-group is specified in USERADD_PARAM_${PN} for a user and
no --gid is specified, then we should not assume that the group name
for the user is the user name.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/classes/useradd-staticids.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/useradd-staticids.bbclass b/meta/classes/useradd-staticids.bbclass
index 9d59aca..c2e6579 100644
--- a/meta/classes/useradd-staticids.bbclass
+++ b/meta/classes/useradd-staticids.bbclass
@@ -107,7 +107,7 @@ def update_useradd_static_config(d):
                             # is used, and we disable the user_group option.
                             #
                             user_group = uaargs.user_group is None or uaargs.user_group is True
-                            uaargs.groupname = [uaargs.gid, uaargs.LOGIN][not uaargs.gid or user_group]
+                            uaargs.groupname = [uaargs.LOGIN, uaargs.gid][not user_group]
                             uaargs.groupid = [uaargs.gid, uaargs.groupname][not uaargs.gid]
                             uaargs.groupid = [field[3], uaargs.groupid][not field[3]]
 
-- 
2.1.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCHv2 3/6] useradd-staticids.bbclass: Simplify some logic
  2015-12-18 23:53 [PATCHv2 0/6] Improvements for useradd-staticids.bbclass Peter Kjellerstedt
  2015-12-18 23:53 ` [PATCHv2 1/6] useradd-staticids.bbclass: Treat mutually exclusive options as such Peter Kjellerstedt
  2015-12-18 23:53 ` [PATCHv2 2/6] useradd-staticids.bbclass: Make --no-user-group have effect Peter Kjellerstedt
@ 2015-12-18 23:53 ` Peter Kjellerstedt
  2015-12-18 23:53 ` [PATCHv2 4/6] useradd-staticids.bbclass: Simplify the logic for when to add groups Peter Kjellerstedt
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Peter Kjellerstedt @ 2015-12-18 23:53 UTC (permalink / raw)
  To: openembedded-core

The [<on_true>, <on_false>][not <condition>] construct may solve the
problem of implementing a conditional operator, but it is not very
readable. At least I find this:

    uaargs.groupid = field[3] or uaargs.gid or uaargs.groupname

a lot more readable than this:

    uaargs.groupid = [uaargs.gid, uaargs.groupname][not uaargs.gid]
    uaargs.groupid = [field[3], uaargs.groupid][not field[3]]

Also, the official conditional operator since Python 2.5 (<on_true> if
<condition> else <on_false>) does not evaluate both <on_false> and
<on_true> as [<on_true>, <on_false>][not <condition>] does.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/classes/useradd-staticids.bbclass | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/meta/classes/useradd-staticids.bbclass b/meta/classes/useradd-staticids.bbclass
index c2e6579..0e855e9 100644
--- a/meta/classes/useradd-staticids.bbclass
+++ b/meta/classes/useradd-staticids.bbclass
@@ -97,7 +97,7 @@ def update_useradd_static_config(d):
                         if field[0] == uaargs.LOGIN:
                             if uaargs.uid and field[2] and (uaargs.uid != field[2]):
                                 bb.warn("%s: Changing username %s's uid from (%s) to (%s), verify configuration files!" % (d.getVar('PN', True), uaargs.LOGIN, uaargs.uid, field[2]))
-                            uaargs.uid = [field[2], uaargs.uid][not field[2]]
+                            uaargs.uid = field[2] or uaargs.uid
 
                             # Determine the possible groupname
                             # Unless the group name (or gid) is specified, we assume that the LOGIN is the groupname
@@ -107,9 +107,8 @@ def update_useradd_static_config(d):
                             # is used, and we disable the user_group option.
                             #
                             user_group = uaargs.user_group is None or uaargs.user_group is True
-                            uaargs.groupname = [uaargs.LOGIN, uaargs.gid][not user_group]
-                            uaargs.groupid = [uaargs.gid, uaargs.groupname][not uaargs.gid]
-                            uaargs.groupid = [field[3], uaargs.groupid][not field[3]]
+                            uaargs.groupname = uaargs.LOGIN if user_group else uaargs.gid
+                            uaargs.groupid = field[3] or uaargs.gid or uaargs.groupname
 
                             if not uaargs.gid or uaargs.gid != uaargs.groupid:
                                 if (uaargs.groupid and uaargs.groupid.isdigit()) and (uaargs.groupname and uaargs.groupname.isdigit()) and (uaargs.groupid != uaargs.groupname):
@@ -123,7 +122,7 @@ def update_useradd_static_config(d):
                                     uaargs.gid = uaargs.groupid
                                     uaargs.user_group = None
                                     groupadd = d.getVar("GROUPADD_PARAM_%s" % pkg, True)
-                                    newgroup = "%s %s" % (['', ' --system'][uaargs.system], uaargs.groupname)
+                                    newgroup = "%s %s" % (' --system' if uaargs.system else '', uaargs.groupname)
                                     if groupadd:
                                         d.setVar("GROUPADD_PARAM_%s" % pkg, "%s ; %s" % (groupadd, newgroup))
                                     else:
@@ -140,9 +139,9 @@ def update_useradd_static_config(d):
                                     else:
                                         d.setVar("GROUPADD_PARAM_%s" % pkg, newgroup)
 
-                            uaargs.comment = ["'%s'" % field[4], uaargs.comment][not field[4]]
-                            uaargs.home_dir = [field[5], uaargs.home_dir][not field[5]]
-                            uaargs.shell = [field[6], uaargs.shell][not field[6]]
+                            uaargs.comment = "'%s'" % field[4] if field[4] else uaargs.comment
+                            uaargs.home_dir = field[5] or uaargs.home_dir
+                            uaargs.shell = field[6] or uaargs.shell
                             break
 
             # Should be an error if a specific option is set...
-- 
2.1.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCHv2 4/6] useradd-staticids.bbclass: Simplify the logic for when to add groups
  2015-12-18 23:53 [PATCHv2 0/6] Improvements for useradd-staticids.bbclass Peter Kjellerstedt
                   ` (2 preceding siblings ...)
  2015-12-18 23:53 ` [PATCHv2 3/6] useradd-staticids.bbclass: Simplify some logic Peter Kjellerstedt
@ 2015-12-18 23:53 ` Peter Kjellerstedt
  2015-12-18 23:53 ` [PATCHv2 5/6] useradd-staticids.bbclass: Read passwd/group files before parsing Peter Kjellerstedt
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Peter Kjellerstedt @ 2015-12-18 23:53 UTC (permalink / raw)
  To: openembedded-core

The original code was near impossible to follow, and missed a couple
of cases. For example, if one added the following line to the passwd
file specified in USERADD_UID_TABLES:

foobar:x:12345:nogroup::/:/bin/sh

and then specified the user as:

USERADD_PARAM_${PN} = "--system foobar"

one would then assume that the foobar user would be created with the
primary group set to nogroup. However, it was not (the primary group
would be foobar), and the only way to get it correct was to explicitly
add --gid nogroup to the USERADD_PARAM_${PN}.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/classes/useradd-staticids.bbclass | 36 +++++++++++++++-------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/meta/classes/useradd-staticids.bbclass b/meta/classes/useradd-staticids.bbclass
index 0e855e9..df4902e 100644
--- a/meta/classes/useradd-staticids.bbclass
+++ b/meta/classes/useradd-staticids.bbclass
@@ -110,30 +110,26 @@ def update_useradd_static_config(d):
                             uaargs.groupname = uaargs.LOGIN if user_group else uaargs.gid
                             uaargs.groupid = field[3] or uaargs.gid or uaargs.groupname
 
-                            if not uaargs.gid or uaargs.gid != uaargs.groupid:
-                                if (uaargs.groupid and uaargs.groupid.isdigit()) and (uaargs.groupname and uaargs.groupname.isdigit()) and (uaargs.groupid != uaargs.groupname):
+                            if uaargs.groupid and uaargs.gid != uaargs.groupid:
+                                newgroup = None
+                                if not uaargs.groupid.isdigit():
+                                    # We don't have a group number, so we have to add a name
+                                    bb.debug(1, "Adding group %s!" % uaargs.groupid)
+                                    newgroup = "%s %s" % (' --system' if uaargs.system else '', uaargs.groupid)
+                                elif uaargs.groupname and not uaargs.groupname.isdigit():
+                                    # We have a group name and a group number to assign it to
+                                    bb.debug(1, "Adding group %s (gid %s)!" % (uaargs.groupname, uaargs.groupid))
+                                    newgroup = "-g %s %s" % (uaargs.groupid, uaargs.groupname)
+                                else:
                                     # We want to add a group, but we don't know it's name... so we can't add the group...
                                     # We have to assume the group has previously been added or we'll fail on the adduser...
                                     # Note: specifying the actual gid is very rare in OE, usually the group name is specified.
-                                    bb.warn("%s: Changing gid for login %s from (%s) to (%s), verify configuration files!" % (d.getVar('PN', True), uaargs.LOGIN, uaargs.groupname, uaargs.gid))
-                                elif (uaargs.groupid and not uaargs.groupid.isdigit()) and uaargs.groupid == uaargs.groupname:
-                                    # We don't have a number, so we have to add a name
-                                    bb.debug(1, "Adding group %s!" % (uaargs.groupname))
-                                    uaargs.gid = uaargs.groupid
-                                    uaargs.user_group = None
-                                    groupadd = d.getVar("GROUPADD_PARAM_%s" % pkg, True)
-                                    newgroup = "%s %s" % (' --system' if uaargs.system else '', uaargs.groupname)
-                                    if groupadd:
-                                        d.setVar("GROUPADD_PARAM_%s" % pkg, "%s ; %s" % (groupadd, newgroup))
-                                    else:
-                                        d.setVar("GROUPADD_PARAM_%s" % pkg, newgroup)
-                                elif uaargs.groupname and (uaargs.groupid and uaargs.groupid.isdigit()):
-                                    # We have a group name and a group number to assign it to
-                                    bb.debug(1, "Adding group %s  gid (%s)!" % (uaargs.groupname, uaargs.groupid))
-                                    uaargs.gid = uaargs.groupid
-                                    uaargs.user_group = None
+                                    bb.warn("%s: Changing gid for login %s to %s, verify configuration files!" % (d.getVar('PN', True), uaargs.LOGIN, uaargs.groupid))
+
+                                uaargs.gid = uaargs.groupid
+                                uaargs.user_group = None
+                                if newgroup:
                                     groupadd = d.getVar("GROUPADD_PARAM_%s" % pkg, True)
-                                    newgroup = "-g %s %s" % (uaargs.gid, uaargs.groupname)
                                     if groupadd:
                                         d.setVar("GROUPADD_PARAM_%s" % pkg, "%s ; %s" % (groupadd, newgroup))
                                     else:
-- 
2.1.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCHv2 5/6] useradd-staticids.bbclass: Read passwd/group files before parsing
  2015-12-18 23:53 [PATCHv2 0/6] Improvements for useradd-staticids.bbclass Peter Kjellerstedt
                   ` (3 preceding siblings ...)
  2015-12-18 23:53 ` [PATCHv2 4/6] useradd-staticids.bbclass: Simplify the logic for when to add groups Peter Kjellerstedt
@ 2015-12-18 23:53 ` Peter Kjellerstedt
  2015-12-18 23:53 ` [PATCHv2 6/6] useradd-staticids.bbclass: Remove unnecessary spaces Peter Kjellerstedt
  2016-01-13 17:36 ` [PATCHv2 0/6] Improvements for useradd-staticids.bbclass Peter Kjellerstedt
  6 siblings, 0 replies; 8+ messages in thread
From: Peter Kjellerstedt @ 2015-12-18 23:53 UTC (permalink / raw)
  To: openembedded-core

Read and merge the passwd/group files before parsing the user and
group definitions. This means they will only be read once per
recipe. This solves a problem where if a user was definied in multiple
files, it could generate group definitions for groups that should not
be created. E.g., if the first passwd file read defines a user as:

foobar::1234::::

and the second passwd file defines it as:

foobar:::nogroup:The foobar user:/:/bin/sh

then a foobar group would be created even if the user will use the
nogroup as its primary group.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/classes/useradd-staticids.bbclass | 164 ++++++++++++++++++---------------
 1 file changed, 91 insertions(+), 73 deletions(-)

diff --git a/meta/classes/useradd-staticids.bbclass b/meta/classes/useradd-staticids.bbclass
index df4902e..4e0ab1b 100644
--- a/meta/classes/useradd-staticids.bbclass
+++ b/meta/classes/useradd-staticids.bbclass
@@ -22,6 +22,30 @@ def update_useradd_static_config(d):
         and return it as a list"""
         return list(itertools.islice(itertools.chain(iterable, itertools.repeat(obj)), length))
 
+    def merge_files(file_list, exp_fields):
+        """Read each passwd/group file in file_list, split each line and create
+        a dictionary with the user/group names as keys and the split lines as
+        values. If the user/group name already exists in the dictionary, then
+        update any fields in the list with the values from the new list (if they
+        are set)."""
+        id_table = dict()
+        for conf in file_list.split():
+            if os.path.exists(conf):
+                f = open(conf, "r")
+                for line in f:
+                    if line.startswith('#'):
+                        continue
+                    # Make sure there always are at least exp_fields elements in
+                    # the field list. This allows for leaving out trailing
+                    # colons in the files.
+                    fields = list_extend(line.rstrip().split(":"), exp_fields)
+                    if fields[0] not in id_table:
+                        id_table[fields[0]] = fields
+                    else:
+                        id_table[fields[0]] = list(itertools.imap(lambda x, y: x or y, fields, id_table[fields[0]]))
+
+        return id_table
+
     # We parse and rewrite the useradd components
     def rewrite_useradd(params):
         # The following comes from --help on useradd from shadow
@@ -63,6 +87,7 @@ def update_useradd_static_config(d):
             return str
 
         newparams = []
+        users = None
         for param in re.split('''[ \t]*;[ \t]*(?=(?:[^'"]|'[^']*'|"[^"]*")*$)''', params):
             param = param.strip()
             if not param:
@@ -72,10 +97,9 @@ def update_useradd_static_config(d):
             except:
                 raise bb.build.FuncFailed("%s: Unable to parse arguments for USERADD_PARAM_%s: '%s'" % (d.getVar('PN', True), pkg, param))
 
-            # files/passwd or the contents of USERADD_UID_TABLES
+            # Read all passwd files specified in USERADD_UID_TABLES or files/passwd
             # Use the standard passwd layout:
             #  username:password:user_id:group_id:comment:home_directory:login_shell
-            # (we want to process in reverse order, as 'last found' in the list wins)
             #
             # If a field is left blank, the original value will be used.  The 'username'
             # field is required.
@@ -84,61 +108,57 @@ def update_useradd_static_config(d):
             # in the useradd command may introduce a security hole.  It's assumed that
             # all new users get the default ('*' which prevents login) until the user is
             # specifically configured by the system admin.
-            for conf in get_passwd_list(d).split()[::-1]:
-                if os.path.exists(conf):
-                    f = open(conf, "r")
-                    for line in f:
-                        if line.startswith('#'):
-                            continue
-                        # Make sure there always are at least seven elements in
-                        # the field list. This allows for leaving out trailing
-                        # colons in the passwd file.
-                        field = list_extend(line.rstrip().split(":"), 7)
-                        if field[0] == uaargs.LOGIN:
-                            if uaargs.uid and field[2] and (uaargs.uid != field[2]):
-                                bb.warn("%s: Changing username %s's uid from (%s) to (%s), verify configuration files!" % (d.getVar('PN', True), uaargs.LOGIN, uaargs.uid, field[2]))
-                            uaargs.uid = field[2] or uaargs.uid
-
-                            # Determine the possible groupname
-                            # Unless the group name (or gid) is specified, we assume that the LOGIN is the groupname
-                            #
-                            # By default the system has creation of the matching groups enabled
-                            # So if the implicit username-group creation is on, then the implicit groupname (LOGIN)
-                            # is used, and we disable the user_group option.
-                            #
-                            user_group = uaargs.user_group is None or uaargs.user_group is True
-                            uaargs.groupname = uaargs.LOGIN if user_group else uaargs.gid
-                            uaargs.groupid = field[3] or uaargs.gid or uaargs.groupname
-
-                            if uaargs.groupid and uaargs.gid != uaargs.groupid:
-                                newgroup = None
-                                if not uaargs.groupid.isdigit():
-                                    # We don't have a group number, so we have to add a name
-                                    bb.debug(1, "Adding group %s!" % uaargs.groupid)
-                                    newgroup = "%s %s" % (' --system' if uaargs.system else '', uaargs.groupid)
-                                elif uaargs.groupname and not uaargs.groupname.isdigit():
-                                    # We have a group name and a group number to assign it to
-                                    bb.debug(1, "Adding group %s (gid %s)!" % (uaargs.groupname, uaargs.groupid))
-                                    newgroup = "-g %s %s" % (uaargs.groupid, uaargs.groupname)
-                                else:
-                                    # We want to add a group, but we don't know it's name... so we can't add the group...
-                                    # We have to assume the group has previously been added or we'll fail on the adduser...
-                                    # Note: specifying the actual gid is very rare in OE, usually the group name is specified.
-                                    bb.warn("%s: Changing gid for login %s to %s, verify configuration files!" % (d.getVar('PN', True), uaargs.LOGIN, uaargs.groupid))
-
-                                uaargs.gid = uaargs.groupid
-                                uaargs.user_group = None
-                                if newgroup:
-                                    groupadd = d.getVar("GROUPADD_PARAM_%s" % pkg, True)
-                                    if groupadd:
-                                        d.setVar("GROUPADD_PARAM_%s" % pkg, "%s ; %s" % (groupadd, newgroup))
-                                    else:
-                                        d.setVar("GROUPADD_PARAM_%s" % pkg, newgroup)
-
-                            uaargs.comment = "'%s'" % field[4] if field[4] else uaargs.comment
-                            uaargs.home_dir = field[5] or uaargs.home_dir
-                            uaargs.shell = field[6] or uaargs.shell
-                            break
+            if not users:
+                users = merge_files(get_passwd_list(d), 7)
+
+            if uaargs.LOGIN not in users:
+                continue
+
+            field = users[uaargs.LOGIN]
+
+            if uaargs.uid and field[2] and (uaargs.uid != field[2]):
+                bb.warn("%s: Changing username %s's uid from (%s) to (%s), verify configuration files!" % (d.getVar('PN', True), uaargs.LOGIN, uaargs.uid, field[2]))
+            uaargs.uid = field[2] or uaargs.uid
+
+            # Determine the possible groupname
+            # Unless the group name (or gid) is specified, we assume that the LOGIN is the groupname
+            #
+            # By default the system has creation of the matching groups enabled
+            # So if the implicit username-group creation is on, then the implicit groupname (LOGIN)
+            # is used, and we disable the user_group option.
+            #
+            user_group = uaargs.user_group is None or uaargs.user_group is True
+            uaargs.groupname = uaargs.LOGIN if user_group else uaargs.gid
+            uaargs.groupid = field[3] or uaargs.gid or uaargs.groupname
+
+            if uaargs.groupid and uaargs.gid != uaargs.groupid:
+                newgroup = None
+                if not uaargs.groupid.isdigit():
+                    # We don't have a group number, so we have to add a name
+                    bb.debug(1, "Adding group %s!" % uaargs.groupid)
+                    newgroup = "%s %s" % (' --system' if uaargs.system else '', uaargs.groupid)
+                elif uaargs.groupname and not uaargs.groupname.isdigit():
+                    # We have a group name and a group number to assign it to
+                    bb.debug(1, "Adding group %s (gid %s)!" % (uaargs.groupname, uaargs.groupid))
+                    newgroup = "-g %s %s" % (uaargs.groupid, uaargs.groupname)
+                else:
+                    # We want to add a group, but we don't know it's name... so we can't add the group...
+                    # We have to assume the group has previously been added or we'll fail on the adduser...
+                    # Note: specifying the actual gid is very rare in OE, usually the group name is specified.
+                    bb.warn("%s: Changing gid for login %s to %s, verify configuration files!" % (d.getVar('PN', True), uaargs.LOGIN, uaargs.groupid))
+
+                uaargs.gid = uaargs.groupid
+                uaargs.user_group = None
+                if newgroup:
+                    groupadd = d.getVar("GROUPADD_PARAM_%s" % pkg, True)
+                    if groupadd:
+                        d.setVar("GROUPADD_PARAM_%s" % pkg, "%s ; %s" % (groupadd, newgroup))
+                    else:
+                        d.setVar("GROUPADD_PARAM_%s" % pkg, newgroup)
+
+            uaargs.comment = "'%s'" % field[4] if field[4] else uaargs.comment
+            uaargs.home_dir = field[5] or uaargs.home_dir
+            uaargs.shell = field[6] or uaargs.shell
 
             # Should be an error if a specific option is set...
             if d.getVar('USERADD_ERROR_DYNAMIC', True) == '1' and not ((uaargs.uid and uaargs.uid.isdigit()) and uaargs.gid):
@@ -200,6 +220,7 @@ def update_useradd_static_config(d):
             return str
 
         newparams = []
+        groups = None
         for param in re.split('''[ \t]*;[ \t]*(?=(?:[^'"]|'[^']*'|"[^"]*")*$)''', params):
             param = param.strip()
             if not param:
@@ -210,7 +231,7 @@ def update_useradd_static_config(d):
             except:
                 raise bb.build.FuncFailed("%s: Unable to parse arguments for GROUPADD_PARAM_%s: '%s'" % (d.getVar('PN', True), pkg, param))
 
-            # Need to iterate over layers and open the right file(s)
+            # Read all group files specified in USERADD_GID_TABLES or files/group
             # Use the standard group layout:
             #  groupname:password:group_id:group_members
             #
@@ -219,21 +240,18 @@ def update_useradd_static_config(d):
             #
             # Note: similar to the passwd file, the 'password' filed is ignored
             # Note: group_members is ignored, group members must be configured with the GROUPMEMS_PARAM
-            for conf in get_group_list(d).split()[::-1]:
-                if os.path.exists(conf):
-                    f = open(conf, "r")
-                    for line in f:
-                        if line.startswith('#'):
-                            continue
-                        # Make sure there always are at least four elements in
-                        # the field list. This allows for leaving out trailing
-                        # colons in the group file.
-                        field = list_extend(line.rstrip().split(":"), 4)
-                        if field[0] == gaargs.GROUP and field[2]:
-                            if gaargs.gid and (gaargs.gid != field[2]):
-                                bb.warn("%s: Changing groupname %s's gid from (%s) to (%s), verify configuration files!" % (d.getVar('PN', True), gaargs.GROUP, gaargs.gid, field[2]))
-                            gaargs.gid = field[2]
-                            break
+            if not groups:
+                groups = merge_files(get_group_list(d), 4)
+
+            if gaargs.GROUP not in groups:
+                continue
+
+            field = groups[gaargs.GROUP]
+
+            if field[2]:
+                if gaargs.gid and (gaargs.gid != field[2]):
+                    bb.warn("%s: Changing groupname %s's gid from (%s) to (%s), verify configuration files!" % (d.getVar('PN', True), gaargs.GROUP, gaargs.gid, field[2]))
+                gaargs.gid = field[2]
 
             if d.getVar('USERADD_ERROR_DYNAMIC', True) == '1' and not (gaargs.gid and gaargs.gid.isdigit()):
                 #bb.error("Skipping recipe %s, package %s which adds groupname %s does not have a static gid defined." % (d.getVar('PN', True),  pkg, gaargs.GROUP))
-- 
2.1.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCHv2 6/6] useradd-staticids.bbclass: Remove unnecessary spaces
  2015-12-18 23:53 [PATCHv2 0/6] Improvements for useradd-staticids.bbclass Peter Kjellerstedt
                   ` (4 preceding siblings ...)
  2015-12-18 23:53 ` [PATCHv2 5/6] useradd-staticids.bbclass: Read passwd/group files before parsing Peter Kjellerstedt
@ 2015-12-18 23:53 ` Peter Kjellerstedt
  2016-01-13 17:36 ` [PATCHv2 0/6] Improvements for useradd-staticids.bbclass Peter Kjellerstedt
  6 siblings, 0 replies; 8+ messages in thread
From: Peter Kjellerstedt @ 2015-12-18 23:53 UTC (permalink / raw)
  To: openembedded-core

This removes unnecessary spaces inserted before semicolons in the
modified USERADD_PARAM_${PN} and GROUPADD_PARAM_${PN} variables. This
should not affect the handling of the variables as the only one that
actually sees the semicolons is the code in useradd.bbclass that uses
cut to split the variables at them, and any whitespace preceeding or
following the semicolons will be properly ignored.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/classes/useradd-staticids.bbclass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/meta/classes/useradd-staticids.bbclass b/meta/classes/useradd-staticids.bbclass
index 4e0ab1b..a9b506d 100644
--- a/meta/classes/useradd-staticids.bbclass
+++ b/meta/classes/useradd-staticids.bbclass
@@ -152,7 +152,7 @@ def update_useradd_static_config(d):
                 if newgroup:
                     groupadd = d.getVar("GROUPADD_PARAM_%s" % pkg, True)
                     if groupadd:
-                        d.setVar("GROUPADD_PARAM_%s" % pkg, "%s ; %s" % (groupadd, newgroup))
+                        d.setVar("GROUPADD_PARAM_%s" % pkg, "%s; %s" % (groupadd, newgroup))
                     else:
                         d.setVar("GROUPADD_PARAM_%s" % pkg, newgroup)
 
@@ -191,7 +191,7 @@ def update_useradd_static_config(d):
 
             newparams.append(newparam)
 
-        return " ;".join(newparams).strip()
+        return ";".join(newparams).strip()
 
     # We parse and rewrite the groupadd components
     def rewrite_groupadd(params):
@@ -269,7 +269,7 @@ def update_useradd_static_config(d):
 
             newparams.append(newparam)
 
-        return " ;".join(newparams).strip()
+        return ";".join(newparams).strip()
 
     # Load and process the users and groups, rewriting the adduser/addgroup params
     useradd_packages = d.getVar('USERADD_PACKAGES', True)
-- 
2.1.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCHv2 0/6] Improvements for useradd-staticids.bbclass
  2015-12-18 23:53 [PATCHv2 0/6] Improvements for useradd-staticids.bbclass Peter Kjellerstedt
                   ` (5 preceding siblings ...)
  2015-12-18 23:53 ` [PATCHv2 6/6] useradd-staticids.bbclass: Remove unnecessary spaces Peter Kjellerstedt
@ 2016-01-13 17:36 ` Peter Kjellerstedt
  6 siblings, 0 replies; 8+ messages in thread
From: Peter Kjellerstedt @ 2016-01-13 17:36 UTC (permalink / raw)
  To: Peter Kjellerstedt, openembedded-core

*ping*

//Peter

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org
> [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
> Peter Kjellerstedt
> Sent: den 19 december 2015 00:54
> To: openembedded-core@lists.openembedded.org
> Subject: [OE-core] [PATCHv2 0/6] Improvements for useradd-
> staticids.bbclass
> 
> It is now 1.5 months since I originally supplied these patches. The
> only comment I received was about a couple of unnecessary spaces that
> I had removed. After explaining why removing them should not make any
> change to the implementation (more than a month ago), I have not
> received any response at all even after pushing for it on the list (a
> month ago) and in private (two weeks ago).
> 
> Since I am now fed up with waithing for any response, I have taken the
> time to split out the removal of those three spaces into a separate
> patch in the vain hope that this will make the patches suitable to be
> applied...
> 
> Now I will go on vacation till the beginning of the next year, hoping
> that the patches have been applied when I get back. So until then,
> Merry Christmas and a Happy New Year!
> 
> Original patch set description below.
> 
> This series of patches aims to improve useradd-staticids.bbclass.
> 
> We are currently using useradd-staticids.bbclass to make sure all
> users and groups have well defined IDs. So far we have had the
> definitions of the users both in the recipes and in the passwd file
> used by useradd-staticids.bbclass. Since we have a huge number of
> recipes that create users, having to duplicate the definitions all
> over the place has turned out to be a burden we should be able to
> avoid.
> 
> So the current plan for us is to have one passwd file per layer with
> the definitions of all users that layer needs. These definitions do
> not include the static IDs for the users. Instead the static IDs for
> the users are specified in a distro specific passwd-static file. There
> is also a distro specific group-static file for the group IDs. With
> that in place it should be enough to define a user as:
> 
> USERADD_PARAM_${PN} = "--system foobar"
> 
> in a recipe and let useradd-staticids.bbclass handle the specifics for
> how that user should be defined.
> 
> The above worked fine for all users that had a primary group with the
> same name as the user. However, it turned out that for users that
> wanted some other primary group, specifying it in the passwd file was
> not enough. We still had to add --gid <some group> in the recipe where
> <some group> had to match what was specified in the passwd file. This
> was less than optimal, and somewhat defeated the setup.
> 
> It also turned out that for users with a primary group that does not
> match the user name, useradd-staticids.bbclass would still add the
> creation of a group with the same name as the user (when it parsed the
> passwd-static file) and the add another creation of the correct group
> (when it parsed the passwd file).
> 
> So after spending quite a lot of time on trying to decode how
> rewrite_useradd() calculated the --gid option, I came up with this
> series of changes that should correct the problems described above and
> make the code easier to understand while (hopefully) maintaining
> compatibility with the old code.
> 
> //Peter
> 
> The following changes since commit
> 5f406915b5e26761faa4ea5e0edd887ac5ae6e2f:
> 
>   bitbake: toaster: remove 2 confusing parameters (2015-12-18 13:51:54
> +0000)
> 
> are available in the git repository at:
> 
>   git://git.yoctoproject.org/poky-contrib pkj/useradd_improvements
>   http://git.yoctoproject.org/cgit.cgi/poky-
> contrib/log/?h=pkj/useradd_improvements
> 
> Peter Kjellerstedt (6):
>   useradd-staticids.bbclass: Treat mutually exclusive options as such
>   useradd-staticids.bbclass: Make --no-user-group have effect
>   useradd-staticids.bbclass: Simplify some logic
>   useradd-staticids.bbclass: Simplify the logic for when to add groups
>   useradd-staticids.bbclass: Read passwd/group files before parsing
>   useradd-staticids.bbclass: Remove unnecessary spaces
> 
>  meta/classes/useradd-staticids.bbclass | 192 ++++++++++++++++++-------
> --------
>  1 file changed, 103 insertions(+), 89 deletions(-)
> 
> --
> 2.1.0
> 
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-01-13 17:36 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-18 23:53 [PATCHv2 0/6] Improvements for useradd-staticids.bbclass Peter Kjellerstedt
2015-12-18 23:53 ` [PATCHv2 1/6] useradd-staticids.bbclass: Treat mutually exclusive options as such Peter Kjellerstedt
2015-12-18 23:53 ` [PATCHv2 2/6] useradd-staticids.bbclass: Make --no-user-group have effect Peter Kjellerstedt
2015-12-18 23:53 ` [PATCHv2 3/6] useradd-staticids.bbclass: Simplify some logic Peter Kjellerstedt
2015-12-18 23:53 ` [PATCHv2 4/6] useradd-staticids.bbclass: Simplify the logic for when to add groups Peter Kjellerstedt
2015-12-18 23:53 ` [PATCHv2 5/6] useradd-staticids.bbclass: Read passwd/group files before parsing Peter Kjellerstedt
2015-12-18 23:53 ` [PATCHv2 6/6] useradd-staticids.bbclass: Remove unnecessary spaces Peter Kjellerstedt
2016-01-13 17:36 ` [PATCHv2 0/6] Improvements for useradd-staticids.bbclass Peter Kjellerstedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.