https://codereview.appspot.com/43730044/diff/1/environs/cloudinit/cloudinit.go#newcode262
environs/cloudinit/cloudinit.go:262: identityFile :=
ssh.SystemIdentityFilename(cfg.DataDir)
See comment on ssh.SystemIdentityFilename: the path here should be
obtained with path.Join, not filepath.Join. If you expose
ssh.SystemIdentity, you can just use cfg.dataFile(ssh.SystemIdentity)
here.
https://codereview.appspot.com/43730044/diff/1/utils/ssh/generate.go#newcode19
utils/ssh/generate.go:19: func GenerateKey(comment string) (private,
public string, err error) {
Doc comment please. What are the properties of the keys? It's RSA,
2048-bits, unencrypted (i.e. no passphrase), with the private key
encoded as PEM and the public key encoded in authorized_keys format. I
think it'd be good to spell all of that out.
https://codereview.appspot.com/43730044/diff/1/utils/ssh/systemidentity.go#newcode15
utils/ssh/systemidentity.go:15: func WriteSystemIdentity(dataDir string,
privateKey string) error {
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.
I'm probably being a bit too anal though, so I won't insist.
https://codereview.appspot.com/43730044/diff/1/utils/ssh/systemidentity.go#newcode28
utils/ssh/systemidentity.go:28: return filepath.Join(dataDir,
systemIdentity)
Using filepath.Join isn't correct in the case of bootstrapping from
Windows. I'd say just expose the systemIdentity constant, and use the
appropriate path/filepath.Join depending on the context.
https:/ /codereview. appspot. com/43730044/ diff/1/ environs/ cloudinit/ cloudinit. go cloudinit/ cloudinit. go (right):
File environs/
https:/ /codereview. appspot. com/43730044/ diff/1/ environs/ cloudinit/ cloudinit. go#newcode262 cloudinit/ cloudinit. go:262: identityFile := ityFilename( cfg.DataDir) ityFilename: the path here should be ssh.SystemIdent ity)
environs/
ssh.SystemIdent
See comment on ssh.SystemIdent
obtained with path.Join, not filepath.Join. If you expose
ssh.SystemIdentity, you can just use cfg.dataFile(
here.
https:/ /codereview. appspot. com/43730044/ diff/1/ provider/ common/ bootstrap. go common/ bootstrap. go (right):
File provider/
https:/ /codereview. appspot. com/43730044/ diff/1/ provider/ common/ bootstrap. go#newcode90 common/ bootstrap. go:90: func GenerateSystemS SHKey(env
provider/
environs.Environ) (privateKey string, err error) {
Doc comment please. Please mention that the function records the public
key in the environment config.
https:/ /codereview. appspot. com/43730044/ diff/1/ provider/ common/ bootstrap. go#newcode90 common/ bootstrap. go:90: func GenerateSystemS SHKey(env
provider/
environs.Environ) (privateKey string, err error) {
Can you please add a test for this function in bootstrap_test.go, to
make sure the public key is added to the environment's config?
https:/ /codereview. appspot. com/43730044/ diff/1/ provider/ common/ bootstrap. go#newcode95 common/ bootstrap. go:95: return "", fmt.Errorf("failed to create
provider/
system key: %v", err)
I'm told the errors should be worded like "creating system key", so it
ends up as
error: creating system key: <thing>
(I've traditionally done it the same way as you have here, mind.)
https:/ /codereview. appspot. com/43730044/ diff/1/ utils/ssh/ generate. go generate. go (right):
File utils/ssh/
https:/ /codereview. appspot. com/43730044/ diff/1/ utils/ssh/ generate. go#newcode19 generate. go:19: func GenerateKey(comment string) (private,
utils/ssh/
public string, err error) {
Doc comment please. What are the properties of the keys? It's RSA,
2048-bits, unencrypted (i.e. no passphrase), with the private key
encoded as PEM and the public key encoded in authorized_keys format. I
think it'd be good to spell all of that out.
https:/ /codereview. appspot. com/43730044/ diff/1/ utils/ssh/ systemidentity. go systemidentity. go (right):
File utils/ssh/
https:/ /codereview. appspot. com/43730044/ diff/1/ utils/ssh/ systemidentity. go#newcode15 systemidentity. go:15: func WriteSystemIden tity(dataDir string,
utils/ssh/
privateKey string) error {
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.
I'm probably being a bit too anal though, so I won't insist.
https:/ /codereview. appspot. com/43730044/ diff/1/ utils/ssh/ systemidentity. go#newcode28 systemidentity. go:28: return filepath. Join(dataDir,
utils/ssh/
systemIdentity)
Using filepath.Join isn't correct in the case of bootstrapping from
Windows. I'd say just expose the systemIdentity constant, and use the
appropriate path/filepath.Join depending on the context.
https:/ /codereview. appspot. com/43730044/