Merge lp:~jameinel/juju-core/1.18-authorized-keys-1312537 into lp:~go-bot/juju-core/trunk

Proposed by John A Meinel
Status: Merged
Merged at revision: 2683
Proposed branch: lp:~jameinel/juju-core/1.18-authorized-keys-1312537
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 422 lines (+81/-43) (has conflicts)
13 files modified
cmd/juju/authorizedkeys.go (+7/-6)
cmd/juju/authorizedkeys_add.go (+1/-1)
cmd/juju/authorizedkeys_delete.go (+1/-1)
cmd/juju/authorizedkeys_import.go (+1/-1)
cmd/juju/authorizedkeys_list.go (+2/-2)
cmd/juju/authorizedkeys_test.go (+23/-23)
cmd/juju/help_topics.go (+5/-5)
cmd/juju/main.go (+2/-2)
cmd/juju/main_test.go (+2/-1)
cmd/supercommand.go (+6/-1)
cmd/supercommand_test.go (+23/-0)
scripts/win-installer/setup.iss (+4/-0)
version/version.go (+4/-0)
Text conflict in scripts/win-installer/setup.iss
Text conflict in version/version.go
To merge this branch: bzr merge lp:~jameinel/juju-core/1.18-authorized-keys-1312537
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+217174@code.launchpad.net

Description of the change

rename authorised-keys to authorized-keys

This addresses bug #1312537. We were very inconsistent about whether
then name was "authorized-keys" or "authorised-keys". I can understand
the cultural spelling biasies. I originally intended to just alias the
command to 'authorized-keys' but then I realized the environments.yaml
setting is 'authorized-keys' so I felt it would be best to be consistent
with something we can't change at this point.

The patch was written against 1.18, so it should be trivial to land it
there, but I'm proposing it just against trunk to start with. We could
also do a more-trivial patch that just adds 'authorized-keys' as the
alias for 1.18, but it means that 'juju help authorized-keys' will tell
you the command is called 'authorised-keys' which I feel should change
in 1.19+.

https://codereview.appspot.com/93800043/

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

Reviewers: mp+217174_code.launchpad.net,

Message:
Please take a look.

Description:
rename authorised-keys to authorized-keys

This addresses bug #1312537. We were very inconsistent about whether
then name was "authorized-keys" or "authorised-keys". I can understand
the cultural spelling biasies. I originally intended to just alias the
command to 'authorized-keys' but then I realized the environments.yaml
setting is 'authorized-keys' so I felt it would be best to be consistent
with something we can't change at this point.

The patch was written against 1.18, so it should be trivial to land it
there, but I'm proposing it just against trunk to start with. We could
also do a more-trivial patch that just adds 'authorized-keys' as the
alias for 1.18, but it means that 'juju help authorized-keys' will tell
you the command is called 'authorised-keys' which I feel should change
in 1.19+.

https://code.launchpad.net/~jameinel/juju-core/1.18-authorized-keys-1312537/+merge/217174

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/93800043/

Affected files (+77, -45 lines):
   A [revision details]
   M cmd/juju/authorizedkeys.go
   M cmd/juju/authorizedkeys_add.go
   M cmd/juju/authorizedkeys_delete.go
   M cmd/juju/authorizedkeys_import.go
   M cmd/juju/authorizedkeys_list.go
   M cmd/juju/authorizedkeys_test.go
   M cmd/juju/help_topics.go
   M cmd/juju/main.go
   M cmd/juju/main_test.go
   M cmd/supercommand.go
   M cmd/supercommand_test.go
   M scripts/win-installer/setup.iss
   M version/version.go

Revision history for this message
William Reade (fwereade) wrote :
Revision history for this message
John A Meinel (jameinel) wrote :

Do you think we should land this in 1.18 as well?
John
=:->

On Fri, Apr 25, 2014 at 11:36 AM, William Reade <<email address hidden>
> wrote:

> LGTM
>
> https://codereview.appspot.com/93800043/
>
> --
>
> https://code.launchpad.net/~jameinel/juju-core/1.18-authorized-keys-1312537/+merge/217174
> You are the owner of lp:~jameinel/juju-core/1.18-authorized-keys-1312537.
>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== renamed file 'cmd/juju/authorisedkeys.go' => 'cmd/juju/authorizedkeys.go'
2--- cmd/juju/authorisedkeys.go 2013-12-13 06:53:01 +0000
3+++ cmd/juju/authorizedkeys.go 2014-04-25 06:45:44 +0000
4@@ -15,17 +15,18 @@
5
6 `
7
8-type AuthorisedKeysCommand struct {
9+type AuthorizedKeysCommand struct {
10 *cmd.SuperCommand
11 }
12
13-func NewAuthorisedKeysCommand() cmd.Command {
14- sshkeyscmd := &AuthorisedKeysCommand{
15+func NewAuthorizedKeysCommand() cmd.Command {
16+ sshkeyscmd := &AuthorizedKeysCommand{
17 SuperCommand: cmd.NewSuperCommand(cmd.SuperCommandParams{
18- Name: "authorised-keys",
19+ Name: "authorized-keys",
20 Doc: authKeysDoc,
21 UsagePrefix: "juju",
22- Purpose: "manage authorised ssh keys",
23+ Purpose: "manage authorized ssh keys",
24+ Aliases: []string{"authorised-keys"},
25 }),
26 }
27 sshkeyscmd.Register(&AddKeysCommand{})
28@@ -35,6 +36,6 @@
29 return sshkeyscmd
30 }
31
32-func (c *AuthorisedKeysCommand) SetFlags(f *gnuflag.FlagSet) {
33+func (c *AuthorizedKeysCommand) SetFlags(f *gnuflag.FlagSet) {
34 c.SetCommonFlags(f)
35 }
36
37=== renamed file 'cmd/juju/authorisedkeys_add.go' => 'cmd/juju/authorizedkeys_add.go'
38--- cmd/juju/authorisedkeys_add.go 2014-04-01 04:53:43 +0000
39+++ cmd/juju/authorizedkeys_add.go 2014-04-25 06:45:44 +0000
40@@ -15,7 +15,7 @@
41 )
42
43 var addKeysDoc = `
44-Add new authorised ssh keys to allow the holder of those keys to log on to Juju nodes or machines.
45+Add new authorized ssh keys to allow the holder of those keys to log on to Juju nodes or machines.
46 `
47
48 // AddKeysCommand is used to add a new authorized ssh key for a user.
49
50=== renamed file 'cmd/juju/authorisedkeys_delete.go' => 'cmd/juju/authorizedkeys_delete.go'
51--- cmd/juju/authorisedkeys_delete.go 2014-04-01 04:53:43 +0000
52+++ cmd/juju/authorizedkeys_delete.go 2014-04-25 06:45:44 +0000
53@@ -15,7 +15,7 @@
54 )
55
56 var deleteKeysDoc = `
57-Delete existing authorised ssh keys to remove ssh access for the holder of those keys.
58+Delete existing authorized ssh keys to remove ssh access for the holder of those keys.
59 The keys to delete are found by specifying either the "comment" portion of the ssh key,
60 typically something like "user@host", or the key fingerprint found by using ssh-keygen.
61 `
62
63=== renamed file 'cmd/juju/authorisedkeys_import.go' => 'cmd/juju/authorizedkeys_import.go'
64--- cmd/juju/authorisedkeys_import.go 2014-04-01 04:53:43 +0000
65+++ cmd/juju/authorizedkeys_import.go 2014-04-25 06:45:44 +0000
66@@ -15,7 +15,7 @@
67 )
68
69 var importKeysDoc = `
70-Import new authorised ssh keys to allow the holder of those keys to log on to Juju nodes or machines.
71+Import new authorized ssh keys to allow the holder of those keys to log on to Juju nodes or machines.
72 The keys are imported using ssh-import-id.
73 `
74
75
76=== renamed file 'cmd/juju/authorisedkeys_list.go' => 'cmd/juju/authorizedkeys_list.go'
77--- cmd/juju/authorisedkeys_list.go 2014-04-01 04:53:43 +0000
78+++ cmd/juju/authorizedkeys_list.go 2014-04-25 06:45:44 +0000
79@@ -16,7 +16,7 @@
80 )
81
82 var listKeysDoc = `
83-List a user's authorised ssh keys, allowing the holders of those keys to log on to Juju nodes.
84+List a user's authorized ssh keys, allowing the holders of those keys to log on to Juju nodes.
85 By default, just the key fingerprint is printed. Use --full to display the entire key.
86
87 `
88@@ -32,7 +32,7 @@
89 return &cmd.Info{
90 Name: "list",
91 Doc: listKeysDoc,
92- Purpose: "list authorised ssh keys for a specified user",
93+ Purpose: "list authorized ssh keys for a specified user",
94 }
95 }
96
97
98=== renamed file 'cmd/juju/authorisedkeys_test.go' => 'cmd/juju/authorizedkeys_test.go'
99--- cmd/juju/authorisedkeys_test.go 2014-03-12 22:33:09 +0000
100+++ cmd/juju/authorizedkeys_test.go 2014-04-25 06:45:44 +0000
101@@ -18,12 +18,12 @@
102 sshtesting "launchpad.net/juju-core/utils/ssh/testing"
103 )
104
105-type AuthorisedKeysSuite struct {
106+type AuthorizedKeysSuite struct {
107 testbase.LoggingSuite
108 jujuHome *coretesting.FakeHome
109 }
110
111-var _ = gc.Suite(&AuthorisedKeysSuite{})
112+var _ = gc.Suite(&AuthorizedKeysSuite{})
113
114 var authKeysCommandNames = []string{
115 "add",
116@@ -33,20 +33,20 @@
117 "list",
118 }
119
120-func (s *AuthorisedKeysSuite) SetUpTest(c *gc.C) {
121+func (s *AuthorizedKeysSuite) SetUpTest(c *gc.C) {
122 s.LoggingSuite.SetUpTest(c)
123 s.jujuHome = coretesting.MakeEmptyFakeHome(c)
124 }
125
126-func (s *AuthorisedKeysSuite) TearDownTest(c *gc.C) {
127+func (s *AuthorizedKeysSuite) TearDownTest(c *gc.C) {
128 s.jujuHome.Restore()
129 s.LoggingSuite.TearDownTest(c)
130 }
131
132-func (s *AuthorisedKeysSuite) TestHelpCommands(c *gc.C) {
133+func (s *AuthorizedKeysSuite) TestHelpCommands(c *gc.C) {
134 // Check that we have correctly registered all the sub commands
135 // by checking the help output.
136- out := badrun(c, 0, "authorised-keys", "--help")
137+ out := badrun(c, 0, "authorized-keys", "--help")
138 lines := strings.Split(out, "\n")
139 var names []string
140 subcommandsFound := false
141@@ -65,29 +65,29 @@
142 c.Assert(names, gc.DeepEquals, authKeysCommandNames)
143 }
144
145-func (s *AuthorisedKeysSuite) assertHelpOutput(c *gc.C, cmd, args string) {
146+func (s *AuthorizedKeysSuite) assertHelpOutput(c *gc.C, cmd, args string) {
147 if args != "" {
148 args = " " + args
149 }
150- expected := fmt.Sprintf("usage: juju authorised-keys %s [options]%s", cmd, args)
151- out := badrun(c, 0, "authorised-keys", cmd, "--help")
152+ expected := fmt.Sprintf("usage: juju authorized-keys %s [options]%s", cmd, args)
153+ out := badrun(c, 0, "authorized-keys", cmd, "--help")
154 lines := strings.Split(out, "\n")
155 c.Assert(lines[0], gc.Equals, expected)
156 }
157
158-func (s *AuthorisedKeysSuite) TestHelpList(c *gc.C) {
159+func (s *AuthorizedKeysSuite) TestHelpList(c *gc.C) {
160 s.assertHelpOutput(c, "list", "")
161 }
162
163-func (s *AuthorisedKeysSuite) TestHelpAdd(c *gc.C) {
164+func (s *AuthorizedKeysSuite) TestHelpAdd(c *gc.C) {
165 s.assertHelpOutput(c, "add", "<ssh key> [...]")
166 }
167
168-func (s *AuthorisedKeysSuite) TestHelpDelete(c *gc.C) {
169+func (s *AuthorizedKeysSuite) TestHelpDelete(c *gc.C) {
170 s.assertHelpOutput(c, "delete", "<ssh key id> [...]")
171 }
172
173-func (s *AuthorisedKeysSuite) TestHelpImport(c *gc.C) {
174+func (s *AuthorizedKeysSuite) TestHelpImport(c *gc.C) {
175 s.assertHelpOutput(c, "import", "<ssh key id> [...]")
176 }
177
178@@ -100,7 +100,7 @@
179 s.PatchEnvironment(osenv.JujuEnvEnvKey, "dummyenv")
180 }
181
182-func (s *keySuiteBase) setAuthorisedKeys(c *gc.C, keys ...string) {
183+func (s *keySuiteBase) setAuthorizedKeys(c *gc.C, keys ...string) {
184 keyString := strings.Join(keys, "\n")
185 err := s.State.UpdateEnvironConfig(map[string]interface{}{"authorized-keys": keyString}, nil, nil)
186 c.Assert(err, gc.IsNil)
187@@ -125,7 +125,7 @@
188 func (s *ListKeysSuite) TestListKeys(c *gc.C) {
189 key1 := sshtesting.ValidKeyOne.Key + " user@host"
190 key2 := sshtesting.ValidKeyTwo.Key + " another@host"
191- s.setAuthorisedKeys(c, key1, key2)
192+ s.setAuthorizedKeys(c, key1, key2)
193
194 context, err := coretesting.RunCommand(c, &ListKeysCommand{}, []string{})
195 c.Assert(err, gc.IsNil)
196@@ -137,7 +137,7 @@
197 func (s *ListKeysSuite) TestListFullKeys(c *gc.C) {
198 key1 := sshtesting.ValidKeyOne.Key + " user@host"
199 key2 := sshtesting.ValidKeyTwo.Key + " another@host"
200- s.setAuthorisedKeys(c, key1, key2)
201+ s.setAuthorizedKeys(c, key1, key2)
202
203 context, err := coretesting.RunCommand(c, &ListKeysCommand{}, []string{"--full"})
204 c.Assert(err, gc.IsNil)
205@@ -149,7 +149,7 @@
206 func (s *ListKeysSuite) TestListKeysNonDefaultUser(c *gc.C) {
207 key1 := sshtesting.ValidKeyOne.Key + " user@host"
208 key2 := sshtesting.ValidKeyTwo.Key + " another@host"
209- s.setAuthorisedKeys(c, key1, key2)
210+ s.setAuthorizedKeys(c, key1, key2)
211 _, err := s.State.AddUser("fred", "password")
212 c.Assert(err, gc.IsNil)
213
214@@ -173,7 +173,7 @@
215
216 func (s *AddKeySuite) TestAddKey(c *gc.C) {
217 key1 := sshtesting.ValidKeyOne.Key + " user@host"
218- s.setAuthorisedKeys(c, key1)
219+ s.setAuthorizedKeys(c, key1)
220
221 key2 := sshtesting.ValidKeyTwo.Key + " another@host"
222 context, err := coretesting.RunCommand(c, &AddKeysCommand{}, []string{key2, "invalid-key"})
223@@ -184,7 +184,7 @@
224
225 func (s *AddKeySuite) TestAddKeyNonDefaultUser(c *gc.C) {
226 key1 := sshtesting.ValidKeyOne.Key + " user@host"
227- s.setAuthorisedKeys(c, key1)
228+ s.setAuthorizedKeys(c, key1)
229 _, err := s.State.AddUser("fred", "password")
230 c.Assert(err, gc.IsNil)
231
232@@ -204,7 +204,7 @@
233 func (s *DeleteKeySuite) TestDeleteKeys(c *gc.C) {
234 key1 := sshtesting.ValidKeyOne.Key + " user@host"
235 key2 := sshtesting.ValidKeyTwo.Key + " another@host"
236- s.setAuthorisedKeys(c, key1, key2)
237+ s.setAuthorizedKeys(c, key1, key2)
238
239 context, err := coretesting.RunCommand(
240 c, &DeleteKeysCommand{}, []string{sshtesting.ValidKeyTwo.Fingerprint, "invalid-key"})
241@@ -216,7 +216,7 @@
242 func (s *DeleteKeySuite) TestDeleteKeyNonDefaultUser(c *gc.C) {
243 key1 := sshtesting.ValidKeyOne.Key + " user@host"
244 key2 := sshtesting.ValidKeyTwo.Key + " another@host"
245- s.setAuthorisedKeys(c, key1, key2)
246+ s.setAuthorizedKeys(c, key1, key2)
247 _, err := s.State.AddUser("fred", "password")
248 c.Assert(err, gc.IsNil)
249
250@@ -240,7 +240,7 @@
251
252 func (s *ImportKeySuite) TestImportKeys(c *gc.C) {
253 key1 := sshtesting.ValidKeyOne.Key + " user@host"
254- s.setAuthorisedKeys(c, key1)
255+ s.setAuthorizedKeys(c, key1)
256
257 context, err := coretesting.RunCommand(c, &ImportKeysCommand{}, []string{"lp:validuser", "invalid-key"})
258 c.Assert(err, gc.IsNil)
259@@ -250,7 +250,7 @@
260
261 func (s *ImportKeySuite) TestImportKeyNonDefaultUser(c *gc.C) {
262 key1 := sshtesting.ValidKeyOne.Key + " user@host"
263- s.setAuthorisedKeys(c, key1)
264+ s.setAuthorizedKeys(c, key1)
265 _, err := s.State.AddUser("fred", "password")
266 c.Assert(err, gc.IsNil)
267
268
269=== modified file 'cmd/juju/help_topics.go'
270--- cmd/juju/help_topics.go 2014-03-17 20:03:47 +0000
271+++ cmd/juju/help_topics.go 2014-04-25 06:45:44 +0000
272@@ -50,14 +50,14 @@
273 const helpLocalProvider = `
274 The local provider is a Linux-only Juju environment that uses LXC containers as
275 a virtual cloud on the local machine. Because of this, lxc and mongodb are
276-required for the local provider to work. If you don't already have lxc and
277-mongodb installed, run the following commands:
278+required for the local provider to work. All of these dependencies are tracked
279+in the 'juju-local' package. You can install that with:
280
281 sudo apt-get update
282- sudo apt-get install lxc mongodb-server
283+ sudo apt-get install juju-local
284
285-After that you might get error for SSH authorized/public key not found. ERROR
286-SSH authorized/public key not found.
287+After that you might get error for SSH authorised/public key not found. ERROR
288+SSH authorised/public key not found.
289
290 ssh-keygen -t rsa
291
292
293=== modified file 'cmd/juju/main.go'
294--- cmd/juju/main.go 2014-04-11 11:51:08 +0000
295+++ cmd/juju/main.go 2014-04-25 06:45:44 +0000
296@@ -122,8 +122,8 @@
297 // Charm tool commands.
298 jujucmd.Register(wrap(&HelpToolCommand{}))
299
300- // Manage authorised ssh keys.
301- jujucmd.Register(wrap(NewAuthorisedKeysCommand()))
302+ // Manage authorized ssh keys.
303+ jujucmd.Register(wrap(NewAuthorizedKeysCommand()))
304
305 // Manage state server availability.
306 // TODO: enable once the backend is ready for it.
307
308=== modified file 'cmd/juju/main_test.go'
309--- cmd/juju/main_test.go 2014-04-14 19:15:35 +0000
310+++ cmd/juju/main_test.go 2014-04-25 06:45:44 +0000
311@@ -214,7 +214,8 @@
312 "add-relation",
313 "add-unit",
314 "api-endpoints",
315- "authorised-keys",
316+ "authorised-keys", // alias for authorized-keys
317+ "authorized-keys",
318 "bootstrap",
319 "debug-hooks",
320 "debug-log",
321
322=== modified file 'cmd/supercommand.go'
323--- cmd/supercommand.go 2014-04-01 07:47:11 +0000
324+++ cmd/supercommand.go 2014-04-25 06:45:44 +0000
325@@ -47,6 +47,7 @@
326 Doc string
327 Log *Log
328 MissingCallback MissingCallback
329+ Aliases []string
330 }
331
332 // NewSuperCommand creates and initializes a new `SuperCommand`, and returns
333@@ -58,7 +59,9 @@
334 Doc: params.Doc,
335 Log: params.Log,
336 usagePrefix: params.UsagePrefix,
337- missingCallback: params.MissingCallback}
338+ missingCallback: params.MissingCallback,
339+ Aliases: params.Aliases,
340+ }
341 command.init()
342 return command
343 }
344@@ -73,6 +76,7 @@
345 Purpose string
346 Doc string
347 Log *Log
348+ Aliases []string
349 usagePrefix string
350 subcmds map[string]Command
351 commonflags *gnuflag.FlagSet
352@@ -185,6 +189,7 @@
353 Args: "<command> ...",
354 Purpose: c.Purpose,
355 Doc: strings.Join(docParts, "\n\n"),
356+ Aliases: c.Aliases,
357 }
358 }
359
360
361=== modified file 'cmd/supercommand_test.go'
362--- cmd/supercommand_test.go 2014-03-20 09:12:53 +0000
363+++ cmd/supercommand_test.go 2014-04-25 06:45:44 +0000
364@@ -264,3 +264,26 @@
365 c.Assert(testing.Stdout(ctx), gc.Equals, "this is std out")
366 c.Assert(testing.Stderr(ctx), gc.Equals, "this is std err")
367 }
368+
369+func (s *SuperCommandSuite) TestSupercommandAliases(c *gc.C) {
370+ jc := cmd.NewSuperCommand(cmd.SuperCommandParams{
371+ Name: "jujutest",
372+ UsagePrefix: "juju",
373+ })
374+ sub := cmd.NewSuperCommand(cmd.SuperCommandParams{
375+ Name: "jubar",
376+ UsagePrefix: "juju jujutest",
377+ Aliases: []string{"jubaz", "jubing"},
378+ })
379+ info := sub.Info()
380+ c.Check(info.Aliases, gc.DeepEquals, []string{"jubaz", "jubing"})
381+ jc.Register(sub)
382+ for _, name := range []string{"jubar", "jubaz", "jubing"} {
383+ c.Logf("testing command name %q", name)
384+ ctx := testing.Context(c)
385+ code := cmd.Main(jc, ctx, []string{name, "--help"})
386+ c.Assert(code, gc.Equals, 0)
387+ stripped := strings.Replace(bufferString(ctx.Stdout), "\n", "", -1)
388+ c.Assert(stripped, gc.Matches, ".*usage: juju jujutest jubar.*aliases: jubaz, jubing")
389+ }
390+}
391
392=== modified file 'scripts/win-installer/setup.iss'
393--- scripts/win-installer/setup.iss 2014-04-15 17:15:47 +0000
394+++ scripts/win-installer/setup.iss 2014-04-25 06:45:44 +0000
395@@ -2,7 +2,11 @@
396 ; SEE THE DOCUMENTATION FOR DETAILS ON CREATING INNO SETUP SCRIPT FILES!
397
398 #define MyAppName "Juju"
399+<<<<<<< TREE
400 #define MyAppVersion "1.19.1"
401+=======
402+#define MyAppVersion "1.18.2"
403+>>>>>>> MERGE-SOURCE
404 #define MyAppPublisher "Canonical, Ltd"
405 #define MyAppURL "http://juju.ubuntu.com/"
406 #define MyAppExeName "juju.exe"
407
408=== modified file 'version/version.go'
409--- version/version.go 2014-04-15 17:15:47 +0000
410+++ version/version.go 2014-04-25 06:45:44 +0000
411@@ -23,7 +23,11 @@
412 // The presence and format of this constant is very important.
413 // The debian/rules build recipe uses this value for the version
414 // number of the release package.
415+<<<<<<< TREE
416 const version = "1.19.1"
417+=======
418+const version = "1.18.2"
419+>>>>>>> MERGE-SOURCE
420
421 // lsbReleaseFile is the name of the file that is read in order to determine
422 // the release version of ubuntu.

Subscribers

People subscribed via source and target branches

to status/vote changes: