776641 - Sort out user management, esp. on Darwin.

Status: RESOLVED FIXED

(Infrastructure & Operations :: RelOps: General, task)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

This means both
 - putting common variables in common places a la bug 764948 comment 29
 - setting up cltbld and root as much as possible on Darwin
 - automatic login on Darwin

I'm not entirely sure what the current state is, so I'll figure that out and put up some info here.

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

I'll do these as separate patches, since the dscl stuff is making me mad.

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug776641.patch

OK, this does the first part, sorting out the usernames.  I'll add corresponding docs to the users module while landing.

You can see my work in progress on the next part at
  https://github.com/djmitche/releng-puppet/commit/310ede6a2405920578532348db5970111766ac5f

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

docs at https://wiki.mozilla.org/ReleaseEngineering/PuppetAgain/Modules/users

Comment 4

[Justin Wood (:Callek)]

Comment on attachment 645089 [details] [diff] [review]
bug776641.patch

>+    # calculate the proper homedir
>+	$home = $operatingsystem ? {

Nit tabs.

>-        "$home_dir/.ssh/known_hosts":
>+        "$home/.ssh/known_hosts":

Are you bitrotting me, or am I bitrotting you, (I thought I already landed that ssh module)

>-        "$home_dir/.bashrc":
>+        "$home/.bashrc":
>             mode => 0644,
>-            owner => "$config::builder_username",
>-            group => "$config::builder_username",
>+            owner => $username,
>+            group => $group,
>             content => template("${module_name}/builder-bashrc.erb");
>-        "$home_dir/.hgrc":
>+        "$home/.hgrc":
>             mode => 0644,
>-            owner => "$config::builder_username",
>-            group => "$config::builder_username",
>+            owner => $username,
>+            group => $group,
>             source => "puppet:///modules/users/hgrc";
>-        "$home_dir/.vimrc":
>+        "$home/.vimrc":
>             mode => 0644,
>-            owner => "$config::builder_username",
>-            group => "$config::builder_username",
>+            owner => $username,
>+            group => $group,
>             source => "puppet:///modules/users/vimrc";
>-        "$home_dir/.screenrc":
>+        "$home/.screenrc":
>             mode => 0644,
>-            owner => "$config::builder_username",
>-            group => "$config::builder_username",
>+            owner => $username,
>+            group => $group,
>             source => "puppet:///modules/users/screenrc";
> 
>     }
> 
>     python::user_pip_conf {
>-        "$config::builder_username": ;
>+        $username: ;
>     }
> }
> 
>diff --git a/modules/users/manifests/root.pp b/modules/users/manifests/root.pp
>index 7cbaf3e..74e805b 100644
>--- a/modules/users/manifests/root.pp
>+++ b/modules/users/manifests/root.pp

Nit: I'd like all vars exposed by users::* to also be exposed here, even if they are not necessary as vars.

In this case uid.

Also please update the "root" hardcode below (used for password) and elsewhere, if we have the var, we might as well *actually* use it where we specify a root ownership/user/etc.

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

(In reply to Justin Wood (:Callek) from comment #4)
> Are you bitrotting me, or am I bitrotting you, (I thought I already landed
> that ssh module)

No SSH module in place..

> Nit: I'd like all vars exposed by users::* to also be exposed here, even if
> they are not necessary as vars.
>
> In this case uid.

I'm not sure what you mean, since you wrote that after the diff header for users::root..

> Also please update the "root" hardcode below (used for password) and
> elsewhere, if we have the var, we might as well *actually* use it where we
> specify a root ownership/user/etc.

I don't want to go overboard there -- the $users::root:username is just there for symmetry, really.  But sure, a few places could use a change.

Comment 6

[Kim Moir [:kmoir] ET]

Comment on attachment 645089 [details] [diff] [review]
bug776641.patch

It looks good, the ssh client files settings can be refactored once a ssh module is available in bug 774157.

Comment 7

[Kim Moir [:kmoir] ET]

Also the builder.pp should have 

python::user_pip_conf {
        "$username":
            homedir => $home,
            group => $group;

    }

instead of 

python::user_pip_conf {
        $username: ;
     }

so it works on Darwin.

Comment 8

[Kim Moir [:kmoir] ET]

Attachment: builder.pp patch

I used your previous patch for testing today along with the patches in bug 776568 and they seemed to work fine.  I'm attaching a patch for an updated builder.pp because I think changes from other bugs have gone in since you created it.  Also, I noticed that managehome => for CentOS went missing from a previous patch. The other changes in the bug776641.patch are fine, this is just to fix builder.pp.

Comment 9

[Dustin J. Mitchell [:dustin] (he/him)]

Hmm, the whitespaces changes make it *really* hard to tell what you changed there, vs what I had changed.  I'll try applying based on a visual inspection and your comments above, and re-post for review.

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug776641-names.patch

Changes since last patch:
* temporarily conditionalize the user { .. } (will be fixed in subsequent patches on this bug)
* parameters to python::user_pip_conf in users::builder (comment 7)
* tabs (comment 4)
* use $username in users::builder (comment 4)

Kim: managehome never disappeared, that I can see..

Callek: I understand your comment now.  $uid is *very* temporary, so it need not be replicated.

Comment 11

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 646122 [details] [diff] [review]
bug776641-names.patch

Landing now.  I'm going to try to get the user-creation to work, too, based on the patches available in the (now no longer NDA'd!) http://projects.puppetlabs.com/issues/12833

Comment 12

[Dustin J. Mitchell [:dustin] (he/him)]

I'd like to try to work around this with a custom provider, based on Gary's code in the bug above.

Comment 13

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: 35e2367e355c052d796ea083c8591d5161484932.patch

This is a custom type (darwinuser) and provider (directoryservice) based on Gary's code.

Unfortunately, this needs to be checked into the production environment for the puppetmaster to use it, meaning I can't very effectively test the manifests that use it until the provider is landed.

I'll probably make some bugfixes to the provider as Gary updates his patch.

Comment 14

[Kim Moir [:kmoir] ET]

Comment on attachment 646360 [details] [diff] [review]
35e2367e355c052d796ea083c8591d5161484932.patch

It looks good but to be honest I'm not that familiar with custom types and providers in order to make any comments.  Like you said it will need some testing on the server.

Comment 15

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: mtnlion-user-info.rb

A script to get the relevant user info on a mountain lion system.

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 646360 [details] [diff] [review]
35e2367e355c052d796ea083c8591d5161484932.patch

I landed an updated version of this patch:

http://hg.mozilla.org/build/puppet/rev/e6bf77ccdd12

Comment 17

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug776641.patch

Manage root and builder on 10.8

This splits account creation into child classes, both for clarity and 
to allow them to be require'd by class name rather than by User[..].

This has some additional tweaks to the darwinuser provider to allow it
to "upgrade" the root account, which was created on 10.7.  It also adds
the secrets and whatnot required to supply the passwords.

It deletes the 'administrator' account, which is an artifact of how we
set up these systems, but it can't hurt to ensure => absent.

This does *not* add the builder user to the Administrators group.  By  
default, only Administrators are allowed to SSH to a machine, so the 
builder user will not be permitted.  The sshd module in bug 764948 will
take care of this.

Comment 18

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 646684 [details] [diff] [review]
bug776641.patch

Hm, this seems not to get the root password right on the first run.  This just locked me out of talos-r5-mtnlion-086.

Comment 19

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug776641-autologin.patch

This can't land until I re-draft attachment 646684 [details] [diff] [review], but it's simple and, I think, works.

Comment 20

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug776641-usermgmt.patch

This version worked with a copy of the root credentials from talos-r5-mntlion-081, so I think it fixes the bug that locked me out.  The difference between attachment 646684 [details] [diff] [review] and this one is to revert the (not so smart) hacks I had made in directoryservice.rb, and upgrade to the latest from glarizza.

With this r+'d, I'll land attachment 646801 [details] [diff] [review], too, and this bug should be finished.

Comment 21

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug776641-usermgmt-v2.patch

"v2" may be overstating the case here, but anyway..

since the previous patch:
* remove screen resolution validation - doesn't work as root
* apply darwinuser provider updates from me and glarizza
* don't delete Administrator, for now, so we aren't locked out of hosts
* hack around race condition in creating new users

In particular, to #1 - we'll need to do this as cltbld, not root, since screenresolution won't work before a user is logged in (and most likely still won't work if you're not in the console context).

To #3, I'll add code to delete Administrator in a week or two, after I'm sure we're not getting locked out.

To #4, I'll track updates in https://github.com/puppetlabs/puppet/pull/981 and hopefully we'll get a better solution in place, but this will do for now.

Comment 22

[Dustin J. Mitchell [:dustin] (he/him)]

And I forgot to mention, I've tested this pretty extensively!

Comment 23

[Kim Moir [:kmoir] ET]

Comment on attachment 647288 [details] [diff] [review]
bug776641-usermgmt-v2.patch

I was going to ask how to we would set the screenresolution as non-root but then I see you opened bug 778942 :-)