Issue 56680043: testing: use valid authorized keys

Can't Edit
Can't Publish+Mail
Start Review

Created:
10 years, 3 months ago by rog

Modified:
10 years, 3 months ago

Reviewers:
mp+203137, dimitern, natefinch

Visibility:
Public.

Description

testing: use valid authorized keys This was causing cmd/jujud to fail sometimes, and produce vast quantities of log messages regardless. Also we fix a problem where if the authorized_keys entry in the config didn't end in a newline, the system key was appended directly without adding one. And we fix a test in provider/ec2 which depended on the fact that the keys were invalid and therefore untouched by EnsureJujuComment. https://code.launchpad.net/~rogpeppe/juju-core/491-fix-fake-authkey/+merge/203137 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : testing: use valid authorized keys #

Total comments: 6

Patch Set 3 : testing: use valid authorized keys #

Created: 10 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+62 lines, -24 lines)			Patch
A	[revision details]	View	1 2	1 chunk	+2 lines, -0 lines	0 comments	Download
M	environs/config/config_test.go	View		15 chunks	+15 lines, -15 lines	0 comments	Download
M	provider/common/bootstrap.go	View	1 2	2 chunks	+18 lines, -1 line	0 comments	Download
M	provider/dummy/environs.go	View		1 chunk	+1 line, -1 line	0 comments	Download
M	provider/ec2/local_test.go	View	1 2	3 chunks	+17 lines, -3 lines	0 comments	Download
M	state/api/provisioner/provisioner_test.go	View	1 2	1 chunk	+1 line, -1 line	0 comments	Download
M	state/apiserver/provisioner/provisioner_test.go	View	1 2	1 chunk	+1 line, -1 line	0 comments	Download
M	testing/environ.go	View	1 2	1 chunk	+6 lines, -1 line	0 comments	Download
M	worker/provisioner/lxc-broker_test.go	View	1 2	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 5

Expand All Messages | Collapse All Messages

dimitern

LGTM with a few suggestions. https://codereview.appspot.com/56680043/diff/20001/provider/common/bootstrap.go File provider/common/bootstrap.go (right): https://codereview.appspot.com/56680043/diff/20001/provider/common/bootstrap.go#newcode116 provider/common/bootstrap.go:116: func concatAuthKeys(a, b string) ...

10 years, 3 months ago (2014-01-24 18:09:34 UTC) #3

rog

10 years, 3 months ago (2014-01-25 11:26:18 UTC) #5

Please take a look.

https://codereview.appspot.com/56680043/diff/20001/provider/common/bootstrap.go
File provider/common/bootstrap.go (right):

https://codereview.appspot.com/56680043/diff/20001/provider/common/bootstrap....
provider/common/bootstrap.go:116: func concatAuthKeys(a, b string) string {
On 2014/01/24 18:09:35, dimitern wrote:
> Doc comment please, explaining why not using just +

Done.

https://codereview.appspot.com/56680043/diff/20001/provider/ec2/local_test.go
File provider/ec2/local_test.go (right):

https://codereview.appspot.com/56680043/diff/20001/provider/ec2/local_test.go...
provider/ec2/local_test.go:283: func splitAuthKeys(keys string) []interface{} {
On 2014/01/24 18:09:35, dimitern wrote:
> Doc comment here as well please.

Done.

https://codereview.appspot.com/56680043/diff/20001/testing/environ.go
File testing/environ.go (right):

https://codereview.appspot.com/56680043/diff/20001/testing/environ.go#newcode18
testing/environ.go:18: const FakeAuthKeys = `ssh-rsa BBBB jo@bloggs.com`
On 2014/01/24 18:09:35, dimitern wrote:
> Doc comment here explaining why should we use this and where would be nice.

Done.

Expand All Messages | Collapse All Messages