Closed
Bug 774157
Opened 12 years ago
Closed 12 years ago
Add an ssh module and use it for extra known_hosts for linux-foopy
Categories
(Infrastructure & Operations Graveyard :: CIDuty, task)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Callek, Assigned: Callek)
References
Details
Attachments
(1 file, 1 obsolete file)
33.21 KB,
patch
|
dustin
:
review+
|
Details | Diff | Splinter Review |
So, I learned that stuff required for linux-foopy was failing out, and this was actually for hosts that most machines don't need to access. Instead of just inflating known_hosts, I decided to split the ssh stuff into its own module, and use the (new) concat plugin (which we added for motd) to do it. I'm going to give kim a shot at reviewing this, but feel free to divert to dustin. And dustin should feel free to make additional comments on it.
Attachment #642463 -
Flags: review?(kmoir)
Comment 1•12 years ago
|
||
This is a great idea, and well-executed! A few thoughts. First, put known_hosts entries in /etc/ssh/ssh_known_hosts, rather than the per-user known_hosts. This has the advantage of not being edited by manual runs of SSH (I'm seeing a lot of puppet runs where ~/.ssh/known_hosts is reverted), and also makes the hosts entries available to other users. The ssh module could be a nice utility module, if it didn't depend on users::builder. Perhaps the homedir information could be passed *to* it, so that it can in principle configure any number of users. For example: ssh::user_config { $::config::builder_user: strict_host_keys => true; } and, elsewhere (e.g., when configuring a crontab) ssh::user_authorized_keys { "${::config::builder_user}|dustins_key": key => "..."; } Both have the tricky bit of needing to translate the username into a home directory, though. Maybe it's best left for a second round, then, unless you see an easy solution. I did verify that things like file { "~root/.ssh/authorized_keys": .. } don't work.
Assignee | ||
Comment 2•12 years ago
|
||
Adjusted for bitrot, made into parameterized classes. (In reply to Dustin J. Mitchell [:dustin] from comment #1) > This is a great idea, and well-executed! > > A few thoughts. First, put known_hosts entries in /etc/ssh/ssh_known_hosts, > rather than the per-user known_hosts. This has the advantage of not being > edited by manual runs of SSH (I'm seeing a lot of puppet runs where > ~/.ssh/known_hosts is reverted), and also makes the hosts entries available > to other users. This is a future intention, but not in this patch. Right now we only have the requirement for the builder user, and this is a requirement for linux foopy. We also have .ssh/config Batchmode set, so we don't auto-add known_hosts anyway. > The ssh module could be a nice utility module, if it didn't depend on > users::builder. Perhaps the homedir information could be passed *to* it, so > that it can in principle configure any number of users. This is now (sorta) done, but we still currently hard-code to users::builder here. > ssh::user_authorized_keys { > "${::config::builder_user}|dustins_key": > key => "..."; > } Also future improvement ;-)
Attachment #642463 -
Attachment is obsolete: true
Attachment #642463 -
Flags: review?(kmoir)
Attachment #646274 -
Flags: review?(dustin)
Comment 3•12 years ago
|
||
Comment on attachment 646274 [details] [diff] [review] [puppet] v2 Ship it. Reassign this to me and I'll loop back later to make it look more like I suggested in comment 1.
Attachment #646274 -
Flags: review?(dustin) → review+
Assignee | ||
Comment 4•12 years ago
|
||
http://hg.mozilla.org/build/puppet/rev/4839fe79093f
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 5•12 years ago
|
||
oops, this is still open for docs.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 6•12 years ago
|
||
docs ping?
Assignee | ||
Comment 7•12 years ago
|
||
Resolving in favor of Docs Bug 816921
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
Updated•6 years ago
|
Product: Release Engineering → Infrastructure & Operations
Updated•4 years ago
|
Product: Infrastructure & Operations → Infrastructure & Operations Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•