Merge ~d-info-e/cloud-init:fix-user-group-doc into cloud-init:master

[do3meli]

correct documentation to match correct attribute name usage.

LP: #1420018

[Scott Moser (smoser)]

this one is harder than it should be.

_normalize_users ends up fixing the '-' to '_' in all those before
'distro.create_user' gets called. So either actually works.
Documentation and implementation in cloud-init have unfortunately been
inconsistent with '-' versus '_'.

We should absolutely strive to be consistent both in implementation
and in documentation.

So...
* I'm 98% certain that 'ssh-import-id' in the diff context and
  'ssh-authorized-keys' support either - or _. Lets just change everything to
  document '_' for consistency.
  Please fix those also.
* lets also update the in-module documentation in
  cloudinit/config/cc_users_groups.py to match (that is what ends up
  being documented at
  http://cloudinit.readthedocs.io/en/latest/topics/modules.html#users-and-groups

[do3meli (d-info-e)]

i haven't tested it but just looking at the code i assume you think that line 133 is the one that converts '-' into '_', right?

see https://git.launchpad.net/cloud-init/tree/cloudinit/distros/ug_util.py#n133

it definitely makes sense to be consistent in the docs as well as the actual implementation. so i will make sure that cloudinit/config/cc_users_groups.py is correct too.

on the other hand i am unsure if ssh-import-id and ssh-authorized-keys are identical. i will dig into that further later on.

c6493bb... by do3meli on 2018-03-29

adjust also inline code docs for cc_user_groups

[do3meli (d-info-e)]

hi scott,
i have addressed your point 2), but not yet 1) as i am still unsure if it will break something or not. looking at it a bit more closer i believe any '-' in the cc_user_group module should be replaced if it really works both ways. that also would include others like primary-group, selinux-user, ect. see cloud-config-user-groups.txt for more. what do you think about that?

[Scott Moser (smoser)]

I'm pretty sure it does work both ways.
so yeah, i suggest we change everything there to have _.

[Chad Smith (chad.smith)]

+1 on changing any hyphenat-delimited keys to underscore-delimited "_". If there are any inconsistencies there in the code honoring underscore-delmited keys we will fix the code. Consistency across all applicable user-provided fields pays dividends.

Dominic, I'll await your final pass on this and we'll land it right away. Thx again.

[Chad Smith (chad.smith)]

review: Approve

[Chad Smith (chad.smith)]

When you update docs, it's probably good to replace

primary-group: foobar with
with
primary_group: foobar
in our integration test @ tests/cloud_tests/testcases/modules/user_groups.yaml

[do3meli (d-info-e)]

i will grep through the whole repo when changing a key so i won't miss any place. will keep you updated on this once having time to fix them all.

[do3meli (d-info-e)]

just found this old bug report here: https://bugs.launchpad.net/cloud-init/+bug/1293254
matches perfectly but is even more general i guess.

4b892c2... by do3meli on 2018-04-01

change all - to _ in tests,modules,doc for user-group module

[do3meli (d-info-e)]

chad,

i have adjusted all other occurrence i found. the ssh-import-id was a bit tricky as we have a module called the same. you may have a closer look at this one specifically.

please run CI. thanks

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

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

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=b27f713a

diff --git a/cloudinit/config/cc_users_groups.py b/cloudinit/config/cc_users_groups.py
index f363000..b215e95 100644
--- a/cloudinit/config/cc_users_groups.py
+++ b/cloudinit/config/cc_users_groups.py
@@ -34,16 +34,16 @@ config keys for an entry in ``users`` are as follows:
     - ``homedir``: Optional. Home dir for user. Default is ``/home/<username>``
     - ``inactive``: Optional. Mark user inactive. Default: false
     - ``lock_passwd``: Optional. Disable password login. Default: true
-    - ``no-create-home``: Optional. Do not create home directory. Default:
+    - ``no_create_home``: Optional. Do not create home directory. Default:
       false
-    - ``no-log-init``: Optional. Do not initialize lastlog and faillog for
+    - ``no_log_init``: Optional. Do not initialize lastlog and faillog for
       user. Default: false
-    - ``no-user-group``: Optional. Do not create group named after user.
+    - ``no_user_group``: Optional. Do not create group named after user.
       Default: false
     - ``passwd``: Hash of user password
-    - ``primary-group``: Optional. Primary group for user. Default to new group
+    - ``primary_group``: Optional. Primary group for user. Default to new group
       named after user.
-    - ``selinux-user``: Optional. SELinux user for user's login. Default to
+    - ``selinux_user``: Optional. SELinux user for user's login. Default to
       default SELinux user.
     - ``shell``: Optional. The user's login shell. The default is to set no
       shell, which results in a system-specific default being used.
@@ -51,9 +51,9 @@ config keys for an entry in ``users`` are as follows:
       a Snappy user through ``snap create-user``. If an Ubuntu SSO account is
       associated with the address, username and SSH keys will be requested from
       there. Default: none
-    - ``ssh-authorized-keys``: Optional. List of ssh keys to add to user's
+    - ``ssh_authorized_keys``: Optional. List of ssh keys to add to user's
       authkeys file. Default: none
-    - ``ssh-import-id``: Optional. SSH id to import for user. Default: none
+    - ``ssh_import_id``: Optional. SSH id to import for user. Default: none
     - ``sudo``: Optional. Sudo rule to use, or list of sudo rules to use.
       Default: none.
     - ``system``: Optional. Create user as system user with no home directory.
@@ -89,18 +89,18 @@ config keys for an entry in ``users`` are as follows:
           homedir: <home directory>
           inactive: <true/false>
           lock_passwd: <true/false>
-          no-create-home: <true/false>
-          no-log-init: <true/false>
-          no-user-group: <true/false>
+          no_create_home: <true/false>
+          no_log_init: <true/false>
+          no_user_group: <true/false>
           passwd: <password>
-          primary-group: <primary group>
-          selinux-user: <selinux username>
+          primary_group: <primary group>
+          selinux_user: <selinux username>
           shell: <shell path>
           snapuser: <email>
-          ssh-authorized-keys:
+          ssh_authorized_keys:
               - <key>
               - <key>
-          ssh-import-id: <id>
+          ssh_import_id: <id>
           sudo: <sudo config>
           system: <true/false>
           uid: <user id>
diff --git a/doc/examples/cloud-config-user-groups.txt b/doc/examples/cloud-config-user-groups.txt
index 0554d1f..7bca24a 100644
--- a/doc/examples/cloud-config-user-groups.txt
+++ b/doc/examples/cloud-config-user-groups.txt
@@ -10,20 +10,20 @@ users:
   - default
   - name: foobar
     gecos: Foo B. Bar
-    primary-group: foobar
+    primary_group: foobar
     groups: users
-    selinux-user: staff_u
+    selinux_user: staff_u
     expiredate: 2012-09-01
-    ssh-import-id: foobar
+    ssh_import_id: foobar
     lock_passwd: false
     passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/
   - name: barfoo
     gecos: Bar B. Foo
     sudo: ALL=(ALL) NOPASSWD:ALL
     groups: users, admin
-    ssh-import-id: None
+    ssh_import_id: None
     lock_passwd: true
-    ssh-authorized-keys:
+    ssh_authorized_keys:
       - <ssh pub key 1>
       - <ssh pub key 2>
   - name: cloudy
@@ -37,10 +37,10 @@ users:
 #   gecos: The user name's real name, i.e. "Bob B. Smith"
 #   homedir: Optional. Set to the local path you want to use. Defaults to
 #           /home/<username>
-#   primary-group: define the primary group. Defaults to a new group created
+#   primary_group: define the primary group. Defaults to a new group created
 #           named after the user.
 #   groups:  Optional. Additional groups to add the user to. Defaults to none
-#   selinux-user:  Optional. The SELinux user for the user's login, such as
+#   selinux_user:  Optional. The SELinux user for the user's login, such as
 #           "staff_u". When this is omitted the system will select the default
 #           SELinux user.
 #   lock_passwd: Defaults to true. Lock the password to disable password login
@@ -66,11 +66,11 @@ users:
 #               should use SSH authentication only.
 #
 #               You have thus been warned.
-#   no-create-home: When set to true, do not create home directory.
-#   no-user-group: When set to true, do not create a group named after the user.
-#   no-log-init: When set to true, do not initialize lastlog and faillog database.
-#   ssh-import-id: Optional. Import SSH ids
-#   ssh-authorized-keys: Optional. [list] Add keys to user's authorized keys file
+#   no_create_home: When set to true, do not create home directory.
+#   no_user_group: When set to true, do not create a group named after the user.
+#   no_log_init: When set to true, do not initialize lastlog and faillog database.
+#   ssh_import_id: Optional. Import SSH ids
+#   ssh_authorized_keys: Optional. [list] Add keys to user's authorized keys file
 #   sudo: Defaults to none. Set to the sudo string you want to use, i.e.
 #           ALL=(ALL) NOPASSWD:ALL. To add multiple rules, use the following
 #           format.
diff --git a/tests/cloud_tests/testcases/examples/including_user_groups.yaml b/tests/cloud_tests/testcases/examples/including_user_groups.yaml
index 469d03c..77528d9 100644
--- a/tests/cloud_tests/testcases/examples/including_user_groups.yaml
+++ b/tests/cloud_tests/testcases/examples/including_user_groups.yaml
@@ -16,7 +16,7 @@ cloud_config: |
     - default
     - name: foobar
       gecos: Foo B. Bar
-      primary-group: foobar
+      primary_group: foobar
       groups: users
       expiredate: 2038-01-19
       lock_passwd: false
diff --git a/tests/cloud_tests/testcases/modules/user_groups.yaml b/tests/cloud_tests/testcases/modules/user_groups.yaml
index 22b5d70..675dfb8 100644
--- a/tests/cloud_tests/testcases/modules/user_groups.yaml
+++ b/tests/cloud_tests/testcases/modules/user_groups.yaml
@@ -15,7 +15,7 @@ cloud_config: |
     - default
     - name: foobar
       gecos: Foo B. Bar
-      primary-group: foobar
+      primary_group: foobar
       groups: users
       expiredate: 2038-01-19
       lock_passwd: false
diff --git a/tests/data/merge_sources/expected7.yaml b/tests/data/merge_sources/expected7.yaml
index 25284f0..d32988e 100644
--- a/tests/data/merge_sources/expected7.yaml
+++ b/tests/data/merge_sources/expected7.yaml
@@ -4,20 +4,20 @@ users:
   - default
   - name: foobar
     gecos: Foo B. Bar
-    primary-group: foobar
+    primary_group: foobar
     groups: users
-    selinux-user: staff_u
+    selinux_user: staff_u
     expiredate: 2012-09-01
-    ssh-import-id: foobar
+    ssh_import_id: foobar
     lock-passwd: false
     passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/
   - name: barfoo
     gecos: Bar B. Foo
     sudo: ALL=(ALL) NOPASSWD:ALL
     groups: users, admin
-    ssh-import-id: None
+    ssh_import_id: None
     lock-passwd: true
-    ssh-authorized-keys:
+    ssh_authorized_keys:
       - <ssh pub key 1>
       - <ssh pub key 2>
   - name: cloudy
@@ -29,10 +29,10 @@ users:
   - sue
   - name: foobar_jr
     gecos: Foo B. Bar Jr
-    primary-group: foobar
+    primary_group: foobar
     groups: users
-    selinux-user: staff_u
+    selinux_user: staff_u
     expiredate: 2012-09-01
-    ssh-import-id: foobar
+    ssh_import_id: foobar
     lock-passwd: false
     passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/
diff --git a/tests/data/merge_sources/source7-1.yaml b/tests/data/merge_sources/source7-1.yaml
index 8fb9b32..6405fc9 100644
--- a/tests/data/merge_sources/source7-1.yaml
+++ b/tests/data/merge_sources/source7-1.yaml
@@ -4,20 +4,20 @@ users:
   - default
   - name: foobar
     gecos: Foo B. Bar
-    primary-group: foobar
+    primary_group: foobar
     groups: users
-    selinux-user: staff_u
+    selinux_user: staff_u
     expiredate: 2012-09-01
-    ssh-import-id: foobar
+    ssh_import_id: foobar
     lock-passwd: false
     passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/
   - name: barfoo
     gecos: Bar B. Foo
     sudo: ALL=(ALL) NOPASSWD:ALL
     groups: users, admin
-    ssh-import-id: None
+    ssh_import_id: None
     lock-passwd: true
-    ssh-authorized-keys:
+    ssh_authorized_keys:
       - <ssh pub key 1>
       - <ssh pub key 2>
   - name: cloudy
diff --git a/tests/data/merge_sources/source7-2.yaml b/tests/data/merge_sources/source7-2.yaml
index 1e26201..0cd2897 100644
--- a/tests/data/merge_sources/source7-2.yaml
+++ b/tests/data/merge_sources/source7-2.yaml
@@ -6,11 +6,11 @@ users:
   - sue
   - name: foobar_jr
     gecos: Foo B. Bar Jr
-    primary-group: foobar
+    primary_group: foobar
     groups: users
-    selinux-user: staff_u
+    selinux_user: staff_u
     expiredate: 2012-09-01
-    ssh-import-id: foobar
+    ssh_import_id: foobar
     lock-passwd: false
     passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/