Closed
Bug 776641
Opened 12 years ago
Closed 12 years ago
Sort out user management, esp. on Darwin.
Categories
(Infrastructure & Operations :: RelOps: General, task)
Infrastructure & Operations
RelOps: General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: dustin, Assigned: dustin)
References
Details
Attachments
(4 files, 5 obsolete files)
(deleted),
patch
|
kmoir
:
review+
dustin
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
kmoir
:
review+
dustin
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
kmoir
:
review+
dustin
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
kmoir
:
review+
dustin
:
checked-in+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•12 years ago
|
||
I'll do these as separate patches, since the dscl stuff is making me mad.
Assignee | ||
Comment 2•12 years ago
|
||
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
Attachment #645089 -
Flags: review?(kmoir)
Assignee | ||
Comment 3•12 years ago
|
||
Comment 4•12 years ago
|
||
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.
Attachment #645089 -
Flags: feedback+
Assignee | ||
Comment 5•12 years ago
|
||
(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•12 years ago
|
||
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.
Attachment #645089 -
Flags: review?(kmoir) → review+
Comment 7•12 years ago
|
||
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•12 years ago
|
||
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.
Assignee | ||
Comment 9•12 years ago
|
||
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.
Assignee | ||
Comment 10•12 years ago
|
||
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.
Attachment #645089 -
Attachment is obsolete: true
Attachment #645988 -
Attachment is obsolete: true
Attachment #646122 -
Flags: review?(kmoir)
Updated•12 years ago
|
Attachment #646122 -
Flags: review?(kmoir) → review+
Assignee | ||
Comment 11•12 years ago
|
||
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
Attachment #646122 -
Flags: checked-in+
Assignee | ||
Comment 12•12 years ago
|
||
I'd like to try to work around this with a custom provider, based on Gary's code in the bug above.
Assignee | ||
Comment 13•12 years ago
|
||
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.
Attachment #646360 -
Flags: review?(kmoir)
Comment 14•12 years ago
|
||
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.
Attachment #646360 -
Flags: review?(kmoir) → review+
Assignee | ||
Comment 15•12 years ago
|
||
A script to get the relevant user info on a mountain lion system.
Assignee | ||
Comment 16•12 years ago
|
||
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
Attachment #646360 -
Flags: checked-in+
Assignee | ||
Comment 17•12 years ago
|
||
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.
Attachment #646550 -
Attachment is obsolete: true
Attachment #646684 -
Flags: review?
Assignee | ||
Comment 18•12 years ago
|
||
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.
Attachment #646684 -
Flags: review?
Assignee | ||
Comment 19•12 years ago
|
||
This can't land until I re-draft attachment 646684 [details] [diff] [review], but it's simple and, I think, works.
Attachment #646801 -
Flags: review?(kmoir)
Updated•12 years ago
|
Attachment #646801 -
Flags: review?(kmoir) → review+
Assignee | ||
Comment 20•12 years ago
|
||
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.
Attachment #646684 -
Attachment is obsolete: true
Attachment #646861 -
Flags: review?
Assignee | ||
Comment 21•12 years ago
|
||
"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.
Attachment #646861 -
Attachment is obsolete: true
Attachment #646861 -
Flags: review?
Attachment #647288 -
Flags: review?(kmoir)
Assignee | ||
Comment 22•12 years ago
|
||
And I forgot to mention, I've tested this pretty extensively!
Comment 23•12 years ago
|
||
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 :-)
Attachment #647288 -
Flags: review?(kmoir) → review+
Assignee | ||
Updated•12 years ago
|
Attachment #647288 -
Flags: checked-in+
Assignee | ||
Updated•12 years ago
|
Attachment #646801 -
Flags: checked-in+
Assignee | ||
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Component: Server Operations: RelEng → RelOps
Product: mozilla.org → Infrastructure & Operations
You need to log in
before you can comment on or make changes to this bug.
Description
•