Code review comment for lp:~thumper/juju-core/system-ssh-key

Revision history for this message
Roger Peppe (rogpeppe) wrote :

Looks reasonable if we actually want to do this, but I have my
reservations. I'd like more information on why we're doing this.

https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt
File doc/system-ssh-key.txt (right):

https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt#newcode2
doc/system-ssh-key.txt:2: potential use-cases.
I'd like to see a few of those use cases outlined here.

Also, we already have a system private key. Can't we just use that?

https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt#newcode5
doc/system-ssh-key.txt:5: as part of the clout-init machine creation
process. The public key part is
s/clout/cloud/

https://codereview.appspot.com/43730044/diff/1/utils/ssh/systemidentity.go
File utils/ssh/systemidentity.go (right):

https://codereview.appspot.com/43730044/diff/1/utils/ssh/systemidentity.go#newcode15
utils/ssh/systemidentity.go:15: func WriteSystemIdentity(dataDir string,
privateKey string) error {
On 2013/12/19 01:50:05, axw wrote:
> This seems a bit too high-level for the utils/ssh package. It probably
ought to
> go in environs/ssh, as a "system identity" is a Juju-level concept
rather than
> an SSH-level one.

+1
This definitely feels like the wrong place to me. Doesn't
environs/cloudinit encapsulate most of this kind of knowledge?

https://codereview.appspot.com/43730044/

« Back to merge proposal