cloud-init

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

  1. Git
  2. lp:~d-info-e/cloud-init
  3. fix-user-group-doc
  4. Merge into master
Proposed by do3meli
Status: Merged
Approved by: Chad Smith
Approved revision: 4b892c2d1fa97bb35935f351d65c872bde513ebf
Merge reported by: Chad Smith
Merged at revision: b27f713ae5b4c5b38eda63758dbaeab92be13b9d
Proposed branch: ~d-info-e/cloud-init:fix-user-group-doc
Merge into: cloud-init:master
Diff against target: 242 lines (+44/-44)
7 files modified
cloudinit/config/cc_users_groups.py (+14/-14)
doc/examples/cloud-config-user-groups.txt (+12/-12)
tests/cloud_tests/testcases/examples/including_user_groups.yaml (+1/-1)
tests/cloud_tests/testcases/modules/user_groups.yaml (+1/-1)
tests/data/merge_sources/expected7.yaml (+8/-8)
tests/data/merge_sources/source7-1.yaml (+5/-5)
tests/data/merge_sources/source7-2.yaml (+3/-3)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Chad Smith Approve
Review via email: mp+342251@code.launchpad.net

Commit message

correct documentation to match correct attribute name usage.

LP: #1420018

To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote :

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

Revision history for this message
do3meli (d-info-e) wrote :

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.

~d-info-e/cloud-init:fix-user-group-doc updated
c6493bb... by do3meli

adjust also inline code docs for cc_user_groups

Revision history for this message
do3meli (d-info-e) wrote :

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?

Revision history for this message
Scott Moser (smoser) wrote :

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

Revision history for this message
Chad Smith (chad.smith) wrote :

+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.

Revision history for this message
Chad Smith (chad.smith) :
review: Approve
Revision history for this message
Chad Smith (chad.smith) wrote :

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

Revision history for this message
do3meli (d-info-e) wrote :

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.

Revision history for this message
do3meli (d-info-e) wrote :

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

~d-info-e/cloud-init:fix-user-group-doc updated
4b892c2... by do3meli

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

Revision history for this message
do3meli (d-info-e) wrote :

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

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

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)
Revision history for this message
Chad Smith (chad.smith) wrote :

An upstream commit landed for this bug.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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:
34 - ``homedir``: Optional. Home dir for user. Default is ``/home/<username>``34 - ``homedir``: Optional. Home dir for user. Default is ``/home/<username>``
35 - ``inactive``: Optional. Mark user inactive. Default: false35 - ``inactive``: Optional. Mark user inactive. Default: false
36 - ``lock_passwd``: Optional. Disable password login. Default: true36 - ``lock_passwd``: Optional. Disable password login. Default: true
37 - ``no-create-home``: Optional. Do not create home directory. Default:37 - ``no_create_home``: Optional. Do not create home directory. Default:
38 false38 false
39 - ``no-log-init``: Optional. Do not initialize lastlog and faillog for39 - ``no_log_init``: Optional. Do not initialize lastlog and faillog for
40 user. Default: false40 user. Default: false
41 - ``no-user-group``: Optional. Do not create group named after user.41 - ``no_user_group``: Optional. Do not create group named after user.
42 Default: false42 Default: false
43 - ``passwd``: Hash of user password43 - ``passwd``: Hash of user password
44 - ``primary-group``: Optional. Primary group for user. Default to new group44 - ``primary_group``: Optional. Primary group for user. Default to new group
45 named after user.45 named after user.
46 - ``selinux-user``: Optional. SELinux user for user's login. Default to46 - ``selinux_user``: Optional. SELinux user for user's login. Default to
47 default SELinux user.47 default SELinux user.
48 - ``shell``: Optional. The user's login shell. The default is to set no48 - ``shell``: Optional. The user's login shell. The default is to set no
49 shell, which results in a system-specific default being used.49 shell, which results in a system-specific default being used.
@@ -51,9 +51,9 @@ config keys for an entry in ``users`` are as follows:
51 a Snappy user through ``snap create-user``. If an Ubuntu SSO account is51 a Snappy user through ``snap create-user``. If an Ubuntu SSO account is
52 associated with the address, username and SSH keys will be requested from52 associated with the address, username and SSH keys will be requested from
53 there. Default: none53 there. Default: none
54 - ``ssh-authorized-keys``: Optional. List of ssh keys to add to user's54 - ``ssh_authorized_keys``: Optional. List of ssh keys to add to user's
55 authkeys file. Default: none55 authkeys file. Default: none
56 - ``ssh-import-id``: Optional. SSH id to import for user. Default: none56 - ``ssh_import_id``: Optional. SSH id to import for user. Default: none
57 - ``sudo``: Optional. Sudo rule to use, or list of sudo rules to use.57 - ``sudo``: Optional. Sudo rule to use, or list of sudo rules to use.
58 Default: none.58 Default: none.
59 - ``system``: Optional. Create user as system user with no home directory.59 - ``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:
89 homedir: <home directory>89 homedir: <home directory>
90 inactive: <true/false>90 inactive: <true/false>
91 lock_passwd: <true/false>91 lock_passwd: <true/false>
92 no-create-home: <true/false>92 no_create_home: <true/false>
93 no-log-init: <true/false>93 no_log_init: <true/false>
94 no-user-group: <true/false>94 no_user_group: <true/false>
95 passwd: <password>95 passwd: <password>
96 primary-group: <primary group>96 primary_group: <primary group>
97 selinux-user: <selinux username>97 selinux_user: <selinux username>
98 shell: <shell path>98 shell: <shell path>
99 snapuser: <email>99 snapuser: <email>
100 ssh-authorized-keys:100 ssh_authorized_keys:
101 - <key>101 - <key>
102 - <key>102 - <key>
103 ssh-import-id: <id>103 ssh_import_id: <id>
104 sudo: <sudo config>104 sudo: <sudo config>
105 system: <true/false>105 system: <true/false>
106 uid: <user id>106 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:
10 - default10 - default
11 - name: foobar11 - name: foobar
12 gecos: Foo B. Bar12 gecos: Foo B. Bar
13 primary-group: foobar13 primary_group: foobar
14 groups: users14 groups: users
15 selinux-user: staff_u15 selinux_user: staff_u
16 expiredate: 2012-09-0116 expiredate: 2012-09-01
17 ssh-import-id: foobar17 ssh_import_id: foobar
18 lock_passwd: false18 lock_passwd: false
19 passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/19 passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/
20 - name: barfoo20 - name: barfoo
21 gecos: Bar B. Foo21 gecos: Bar B. Foo
22 sudo: ALL=(ALL) NOPASSWD:ALL22 sudo: ALL=(ALL) NOPASSWD:ALL
23 groups: users, admin23 groups: users, admin
24 ssh-import-id: None24 ssh_import_id: None
25 lock_passwd: true25 lock_passwd: true
26 ssh-authorized-keys:26 ssh_authorized_keys:
27 - <ssh pub key 1>27 - <ssh pub key 1>
28 - <ssh pub key 2>28 - <ssh pub key 2>
29 - name: cloudy29 - name: cloudy
@@ -37,10 +37,10 @@ users:
37# gecos: The user name's real name, i.e. "Bob B. Smith"37# gecos: The user name's real name, i.e. "Bob B. Smith"
38# homedir: Optional. Set to the local path you want to use. Defaults to38# homedir: Optional. Set to the local path you want to use. Defaults to
39# /home/<username>39# /home/<username>
40# primary-group: define the primary group. Defaults to a new group created40# primary_group: define the primary group. Defaults to a new group created
41# named after the user.41# named after the user.
42# groups: Optional. Additional groups to add the user to. Defaults to none42# groups: Optional. Additional groups to add the user to. Defaults to none
43# selinux-user: Optional. The SELinux user for the user's login, such as43# selinux_user: Optional. The SELinux user for the user's login, such as
44# "staff_u". When this is omitted the system will select the default44# "staff_u". When this is omitted the system will select the default
45# SELinux user.45# SELinux user.
46# lock_passwd: Defaults to true. Lock the password to disable password login46# lock_passwd: Defaults to true. Lock the password to disable password login
@@ -66,11 +66,11 @@ users:
66# should use SSH authentication only.66# should use SSH authentication only.
67#67#
68# You have thus been warned.68# You have thus been warned.
69# no-create-home: When set to true, do not create home directory.69# no_create_home: When set to true, do not create home directory.
70# no-user-group: When set to true, do not create a group named after the user.70# no_user_group: When set to true, do not create a group named after the user.
71# no-log-init: When set to true, do not initialize lastlog and faillog database.71# no_log_init: When set to true, do not initialize lastlog and faillog database.
72# ssh-import-id: Optional. Import SSH ids72# ssh_import_id: Optional. Import SSH ids
73# ssh-authorized-keys: Optional. [list] Add keys to user's authorized keys file73# ssh_authorized_keys: Optional. [list] Add keys to user's authorized keys file
74# sudo: Defaults to none. Set to the sudo string you want to use, i.e.74# sudo: Defaults to none. Set to the sudo string you want to use, i.e.
75# ALL=(ALL) NOPASSWD:ALL. To add multiple rules, use the following75# ALL=(ALL) NOPASSWD:ALL. To add multiple rules, use the following
76# format.76# 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: |
16 - default16 - default
17 - name: foobar17 - name: foobar
18 gecos: Foo B. Bar18 gecos: Foo B. Bar
19 primary-group: foobar19 primary_group: foobar
20 groups: users20 groups: users
21 expiredate: 2038-01-1921 expiredate: 2038-01-19
22 lock_passwd: false22 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: |
15 - default15 - default
16 - name: foobar16 - name: foobar
17 gecos: Foo B. Bar17 gecos: Foo B. Bar
18 primary-group: foobar18 primary_group: foobar
19 groups: users19 groups: users
20 expiredate: 2038-01-1920 expiredate: 2038-01-19
21 lock_passwd: false21 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:
4 - default4 - default
5 - name: foobar5 - name: foobar
6 gecos: Foo B. Bar6 gecos: Foo B. Bar
7 primary-group: foobar7 primary_group: foobar
8 groups: users8 groups: users
9 selinux-user: staff_u9 selinux_user: staff_u
10 expiredate: 2012-09-0110 expiredate: 2012-09-01
11 ssh-import-id: foobar11 ssh_import_id: foobar
12 lock-passwd: false12 lock-passwd: false
13 passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/13 passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/
14 - name: barfoo14 - name: barfoo
15 gecos: Bar B. Foo15 gecos: Bar B. Foo
16 sudo: ALL=(ALL) NOPASSWD:ALL16 sudo: ALL=(ALL) NOPASSWD:ALL
17 groups: users, admin17 groups: users, admin
18 ssh-import-id: None18 ssh_import_id: None
19 lock-passwd: true19 lock-passwd: true
20 ssh-authorized-keys:20 ssh_authorized_keys:
21 - <ssh pub key 1>21 - <ssh pub key 1>
22 - <ssh pub key 2>22 - <ssh pub key 2>
23 - name: cloudy23 - name: cloudy
@@ -29,10 +29,10 @@ users:
29 - sue29 - sue
30 - name: foobar_jr30 - name: foobar_jr
31 gecos: Foo B. Bar Jr31 gecos: Foo B. Bar Jr
32 primary-group: foobar32 primary_group: foobar
33 groups: users33 groups: users
34 selinux-user: staff_u34 selinux_user: staff_u
35 expiredate: 2012-09-0135 expiredate: 2012-09-01
36 ssh-import-id: foobar36 ssh_import_id: foobar
37 lock-passwd: false37 lock-passwd: false
38 passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/38 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:
4 - default4 - default
5 - name: foobar5 - name: foobar
6 gecos: Foo B. Bar6 gecos: Foo B. Bar
7 primary-group: foobar7 primary_group: foobar
8 groups: users8 groups: users
9 selinux-user: staff_u9 selinux_user: staff_u
10 expiredate: 2012-09-0110 expiredate: 2012-09-01
11 ssh-import-id: foobar11 ssh_import_id: foobar
12 lock-passwd: false12 lock-passwd: false
13 passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/13 passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/
14 - name: barfoo14 - name: barfoo
15 gecos: Bar B. Foo15 gecos: Bar B. Foo
16 sudo: ALL=(ALL) NOPASSWD:ALL16 sudo: ALL=(ALL) NOPASSWD:ALL
17 groups: users, admin17 groups: users, admin
18 ssh-import-id: None18 ssh_import_id: None
19 lock-passwd: true19 lock-passwd: true
20 ssh-authorized-keys:20 ssh_authorized_keys:
21 - <ssh pub key 1>21 - <ssh pub key 1>
22 - <ssh pub key 2>22 - <ssh pub key 2>
23 - name: cloudy23 - 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:
6 - sue6 - sue
7 - name: foobar_jr7 - name: foobar_jr
8 gecos: Foo B. Bar Jr8 gecos: Foo B. Bar Jr
9 primary-group: foobar9 primary_group: foobar
10 groups: users10 groups: users
11 selinux-user: staff_u11 selinux_user: staff_u
12 expiredate: 2012-09-0112 expiredate: 2012-09-01
13 ssh-import-id: foobar13 ssh_import_id: foobar
14 lock-passwd: false14 lock-passwd: false
15 passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/15 passwd: $6$j212wezy$7H/1LT4f9/N3wpgNunhsIqtMj62OKiS3nyNwuizouQc3u7MbYCarYeAHWYPYb2FT.lbioDm2RrkJPb9BZMN1O/
1616

Subscribers

People subscribed via source and target branches