Merge ~oddbloke/cloud-init/+git/cloud-init:useradd into cloud-init:master

[Dan Watkins]

distros: fix confusing variable names

Building the subp arguments for a `useradd` call in a variable named
`adduser_cmd` is extremely confusing; let's not do that.

(This also changes the snap and freebsd variables to something more
apropos.)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:65f501f39cc3646a761028733a4bfeb370cd9cf5
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1036/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1036//rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

The method is call add_user and the variables match the method.

So now we have

def add_user()
   useradd_cmd =

Isn't this the same confusion?

The command to add a user could be something else. In cloudinit/distros/freebsd.py for example, the adduser_cmd , is 'pw useradd'.

[Dan Watkins (oddbloke)]

On Mon, Aug 12, 2019 at 02:31:18PM -0000, Ryan Harper wrote:
> The method is call add_user and the variables match the method.

If the variable matched the method, I'd expect it to be add_user_cmd.
I'd be fine with that, if that's what you would prefer?

> So now we have
>
> def add_user()
> useradd_cmd =
>
> Isn't this the same confusion?

I don't think so; the name of the command we do actually use is
hardcoded in a string literal later on that line:

    useradd_cmd = ['useradd', ...]

> The command to add a user could be something else.

Not without code changes, which I would expect to also change the
variable name.

> In cloudinit/distros/freebsd.py for example, the adduser_cmd , is 'pw
> useradd'.

Right, I didn't touch the variable name there.

[Chad Smith (chad.smith)]

So, can we say freebsd.py should be pw_cmd or pw_useradd_cmd? Since this is not a functional change I'm fine with whatever you gents prefer but maybe since it is a cosmetic/style thing maybe we can fix both implementations to adhere to the same guideline. Then we don't have to look at this specific case in the future.

review: Approve

a0cc273... by Dan Watkins on 2019-08-14

distros/freebsd: clean up variable name for consistency

[Chad Smith (chad.smith)]

LGTM!

review: Approve

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:a0cc273d4147c4ccabd88d7bc443d70f9a126cde
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1044/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1044//rebuild

review: Approve (continuous-integration)

diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index 20c994d..00bdee3 100644
--- a/cloudinit/distros/__init__.py
+++ b/cloudinit/distros/__init__.py
@@ -396,16 +396,16 @@ class Distro(object):
         else:
             create_groups = True
 
-        adduser_cmd = ['useradd', name]
-        log_adduser_cmd = ['useradd', name]
+        useradd_cmd = ['useradd', name]
+        log_useradd_cmd = ['useradd', name]
         if util.system_is_snappy():
-            adduser_cmd.append('--extrausers')
-            log_adduser_cmd.append('--extrausers')
+            useradd_cmd.append('--extrausers')
+            log_useradd_cmd.append('--extrausers')
 
         # Since we are creating users, we want to carefully validate the
         # inputs. If something goes wrong, we can end up with a system
         # that nobody can login to.
-        adduser_opts = {
+        useradd_opts = {
             "gecos": '--comment',
             "homedir": '--home',
             "primary_group": '--gid',
@@ -418,7 +418,7 @@ class Distro(object):
             "selinux_user": '--selinux-user',
         }
 
-        adduser_flags = {
+        useradd_flags = {
             "no_user_group": '--no-user-group',
             "system": '--system',
             "no_log_init": '--no-log-init',
@@ -453,32 +453,32 @@ class Distro(object):
         # Check the values and create the command
         for key, val in sorted(kwargs.items()):
 
-            if key in adduser_opts and val and isinstance(val, str):
-                adduser_cmd.extend([adduser_opts[key], val])
+            if key in useradd_opts and val and isinstance(val, str):
+                useradd_cmd.extend([useradd_opts[key], val])
 
                 # Redact certain fields from the logs
                 if key in redact_opts:
-                    log_adduser_cmd.extend([adduser_opts[key], 'REDACTED'])
+                    log_useradd_cmd.extend([useradd_opts[key], 'REDACTED'])
                 else:
-                    log_adduser_cmd.extend([adduser_opts[key], val])
+                    log_useradd_cmd.extend([useradd_opts[key], val])
 
-            elif key in adduser_flags and val:
-                adduser_cmd.append(adduser_flags[key])
-                log_adduser_cmd.append(adduser_flags[key])
+            elif key in useradd_flags and val:
+                useradd_cmd.append(useradd_flags[key])
+                log_useradd_cmd.append(useradd_flags[key])
 
         # Don't create the home directory if directed so or if the user is a
         # system user
         if kwargs.get('no_create_home') or kwargs.get('system'):
-            adduser_cmd.append('-M')
-            log_adduser_cmd.append('-M')
+            useradd_cmd.append('-M')
+            log_useradd_cmd.append('-M')
         else:
-            adduser_cmd.append('-m')
-            log_adduser_cmd.append('-m')
+            useradd_cmd.append('-m')
+            log_useradd_cmd.append('-m')
 
         # Run the command
         LOG.debug("Adding user %s", name)
         try:
-            util.subp(adduser_cmd, logstring=log_adduser_cmd)
+            util.subp(useradd_cmd, logstring=log_useradd_cmd)
         except Exception as e:
             util.logexc(LOG, "Failed to create user %s", name)
             raise e
@@ -490,15 +490,15 @@ class Distro(object):
 
         snapuser = kwargs.get('snapuser')
         known = kwargs.get('known', False)
-        adduser_cmd = ["snap", "create-user", "--sudoer", "--json"]
+        create_user_cmd = ["snap", "create-user", "--sudoer", "--json"]
         if known:
-            adduser_cmd.append("--known")
-        adduser_cmd.append(snapuser)
+            create_user_cmd.append("--known")
+        create_user_cmd.append(snapuser)
 
         # Run the command
         LOG.debug("Adding snap user %s", name)
         try:
-            (out, err) = util.subp(adduser_cmd, logstring=adduser_cmd,
+            (out, err) = util.subp(create_user_cmd, logstring=create_user_cmd,
                                    capture=True)
             LOG.debug("snap create-user returned: %s:%s", out, err)
             jobj = util.load_json(out)
diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py
index ff22d56..f7825fd 100644
--- a/cloudinit/distros/freebsd.py
+++ b/cloudinit/distros/freebsd.py
@@ -185,10 +185,10 @@ class Distro(distros.Distro):
             LOG.info("User %s already exists, skipping.", name)
             return False
 
-        adduser_cmd = ['pw', 'useradd', '-n', name]
-        log_adduser_cmd = ['pw', 'useradd', '-n', name]
+        pw_useradd_cmd = ['pw', 'useradd', '-n', name]
+        log_pw_useradd_cmd = ['pw', 'useradd', '-n', name]
 
-        adduser_opts = {
+        pw_useradd_opts = {
             "homedir": '-d',
             "gecos": '-c',
             "primary_group": '-g',
@@ -196,34 +196,34 @@ class Distro(distros.Distro):
             "shell": '-s',
             "inactive": '-E',
         }
-        adduser_flags = {
+        pw_useradd_flags = {
             "no_user_group": '--no-user-group',
             "system": '--system',
             "no_log_init": '--no-log-init',
         }
 
         for key, val in kwargs.items():
-            if (key in adduser_opts and val and
+            if (key in pw_useradd_opts and val and
                isinstance(val, six.string_types)):
-                adduser_cmd.extend([adduser_opts[key], val])
+                pw_useradd_cmd.extend([pw_useradd_opts[key], val])
 
-            elif key in adduser_flags and val:
-                adduser_cmd.append(adduser_flags[key])
-                log_adduser_cmd.append(adduser_flags[key])
+            elif key in pw_useradd_flags and val:
+                pw_useradd_cmd.append(pw_useradd_flags[key])
+                log_pw_useradd_cmd.append(pw_useradd_flags[key])
 
         if 'no_create_home' in kwargs or 'system' in kwargs:
-            adduser_cmd.append('-d/nonexistent')
-            log_adduser_cmd.append('-d/nonexistent')
+            pw_useradd_cmd.append('-d/nonexistent')
+            log_pw_useradd_cmd.append('-d/nonexistent')
         else:
-            adduser_cmd.append('-d/usr/home/%s' % name)
-            adduser_cmd.append('-m')
-            log_adduser_cmd.append('-d/usr/home/%s' % name)
-            log_adduser_cmd.append('-m')
+            pw_useradd_cmd.append('-d/usr/home/%s' % name)
+            pw_useradd_cmd.append('-m')
+            log_pw_useradd_cmd.append('-d/usr/home/%s' % name)
+            log_pw_useradd_cmd.append('-m')
 
         # Run the command
         LOG.info("Adding user %s", name)
         try:
-            util.subp(adduser_cmd, logstring=log_adduser_cmd)
+            util.subp(pw_useradd_cmd, logstring=log_pw_useradd_cmd)
         except Exception as e:
             util.logexc(LOG, "Failed to create user %s", name)
             raise e