Merge lp:~axwalk/juju-core/ssh-proxycommand into lp:~go-bot/juju-core/trunk
- ssh-proxycommand
- Merge into trunk
Proposed by
Andrew Wilkins
Status: | Merged |
---|---|
Approved by: | Andrew Wilkins |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2442 |
Proposed branch: | lp:~axwalk/juju-core/ssh-proxycommand |
Merge into: | lp:~go-bot/juju-core/trunk |
Diff against target: |
732 lines (+298/-89) 14 files modified
cmd/juju/scp.go (+9/-1) cmd/juju/scp_test.go (+56/-58) cmd/juju/ssh.go (+74/-11) cmd/juju/ssh_test.go (+10/-2) juju/api.go (+10/-3) juju/apiconn_test.go (+13/-13) juju/export_test.go (+1/-1) state/api/apiclient.go (+7/-0) state/api/client.go (+9/-0) state/api/params/params.go (+10/-0) state/apiserver/client/client.go (+28/-0) state/apiserver/client/client_test.go (+60/-0) utils/ssh/ssh.go (+8/-0) utils/ssh/ssh_openssh.go (+3/-0) |
To merge this branch: | bzr merge lp:~axwalk/juju-core/ssh-proxycommand |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Engineering | Pending | ||
Review via email: mp+211662@code.launchpad.net |
Commit message
Add --proxy option to "juju ssh"
juju ssh will now, by default, proxy
connections through the API server host.
I have not yet implemented support for the
ProxyCommand option for the go.crypto/ssh
client; this will be done as a follow up.
Description of the change
Add --proxy option to "juju ssh"
juju ssh will now, by default, proxy
connections through the API server host.
I have not yet implemented support for the
ProxyCommand option for the go.crypto/ssh
client; this will be done as a follow up.
To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote : | # |
Revision history for this message
Tim Penhey (thumper) wrote : | # |
LGTM - the code looks fine, and I'm assuming you have tested it.
Revision history for this message
Andrew Wilkins (axwalk) wrote : | # |
On 2014/03/19 03:41:35, thumper wrote:
> LGTM - the code looks fine, and I'm assuming you have tested it.
Tested with local, canonistack and azure. Verified that ssh, scp,
debug-log, and debug-hooks.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'cmd/juju/scp.go' |
2 | --- cmd/juju/scp.go 2014-02-24 16:21:43 +0000 |
3 | +++ cmd/juju/scp.go 2014-03-19 03:21:48 +0000 |
4 | @@ -106,5 +106,13 @@ |
5 | targets = append(targets, arg) |
6 | } |
7 | } |
8 | - return ssh.Copy(targets, extraArgs, nil) |
9 | + |
10 | + var options *ssh.Options |
11 | + if c.proxy { |
12 | + options = new(ssh.Options) |
13 | + if err := c.setProxyCommand(options); err != nil { |
14 | + return err |
15 | + } |
16 | + } |
17 | + return ssh.Copy(targets, extraArgs, options) |
18 | } |
19 | |
20 | === modified file 'cmd/juju/scp_test.go' |
21 | --- cmd/juju/scp_test.go 2014-02-24 16:21:43 +0000 |
22 | +++ cmd/juju/scp_test.go 2014-03-19 03:21:48 +0000 |
23 | @@ -27,67 +27,64 @@ |
24 | about string |
25 | args []string |
26 | result string |
27 | + proxy bool |
28 | error string |
29 | }{ |
30 | { |
31 | - "scp from machine 0 to current dir", |
32 | - []string{"0:foo", "."}, |
33 | - commonArgs + "ubuntu@dummyenv-0.dns:foo .\n", |
34 | - "", |
35 | - }, |
36 | - { |
37 | - "scp from machine 0 to current dir with extra args", |
38 | - []string{"0:foo", ".", "-rv -o SomeOption"}, |
39 | - commonArgs + "-rv -o SomeOption ubuntu@dummyenv-0.dns:foo .\n", |
40 | - "", |
41 | - }, |
42 | - { |
43 | - "scp from current dir to machine 0", |
44 | - []string{"foo", "0:"}, |
45 | - commonArgs + "foo ubuntu@dummyenv-0.dns:\n", |
46 | - "", |
47 | - }, |
48 | - { |
49 | - "scp from current dir to machine 0 with extra args", |
50 | - []string{"foo", "0:", "-r -v"}, |
51 | - commonArgs + "-r -v foo ubuntu@dummyenv-0.dns:\n", |
52 | - "", |
53 | - }, |
54 | - { |
55 | - "scp from machine 0 to unit mysql/0", |
56 | - []string{"0:foo", "mysql/0:/foo"}, |
57 | - commonArgs + "ubuntu@dummyenv-0.dns:foo ubuntu@dummyenv-0.dns:/foo\n", |
58 | - "", |
59 | - }, |
60 | - { |
61 | - "scp from machine 0 to unit mysql/0 and extra args", |
62 | - []string{"0:foo", "mysql/0:/foo", "-q"}, |
63 | - commonArgs + "-q ubuntu@dummyenv-0.dns:foo ubuntu@dummyenv-0.dns:/foo\n", |
64 | - "", |
65 | - }, |
66 | - { |
67 | - "scp from machine 0 to unit mysql/0 and extra args before", |
68 | - []string{"-q", "-r", "0:foo", "mysql/0:/foo"}, |
69 | - "", |
70 | - `unexpected argument "-q"; extra arguments must be last`, |
71 | - }, |
72 | - { |
73 | - "scp two local files to unit mysql/0", |
74 | - []string{"file1", "file2", "mysql/0:/foo/"}, |
75 | - commonArgs + "file1 file2 ubuntu@dummyenv-0.dns:/foo/\n", |
76 | - "", |
77 | - }, |
78 | - { |
79 | - "scp from unit mongodb/1 to unit mongodb/0 and multiple extra args", |
80 | - []string{"mongodb/1:foo", "mongodb/0:", "-r -v -q -l5"}, |
81 | - commonArgs + "-r -v -q -l5 ubuntu@dummyenv-2.dns:foo ubuntu@dummyenv-1.dns:\n", |
82 | - "", |
83 | - }, |
84 | - { |
85 | - "scp works with IPv6 addresses", |
86 | - []string{"ipv6-svc/0:foo", "bar"}, |
87 | - commonArgs + `ubuntu@\[2001:db8::\]:foo bar` + "\n", |
88 | - "", |
89 | + about: "scp from machine 0 to current dir", |
90 | + args: []string{"0:foo", "."}, |
91 | + result: commonArgsNoProxy + "ubuntu@dummyenv-0.dns:foo .\n", |
92 | + }, |
93 | + { |
94 | + about: "scp from machine 0 to current dir with extra args", |
95 | + args: []string{"0:foo", ".", "-rv -o SomeOption"}, |
96 | + result: commonArgsNoProxy + "-rv -o SomeOption ubuntu@dummyenv-0.dns:foo .\n", |
97 | + }, |
98 | + { |
99 | + about: "scp from current dir to machine 0", |
100 | + args: []string{"foo", "0:"}, |
101 | + result: commonArgsNoProxy + "foo ubuntu@dummyenv-0.dns:\n", |
102 | + }, |
103 | + { |
104 | + about: "scp from current dir to machine 0 with extra args", |
105 | + args: []string{"foo", "0:", "-r -v"}, |
106 | + result: commonArgsNoProxy + "-r -v foo ubuntu@dummyenv-0.dns:\n", |
107 | + }, |
108 | + { |
109 | + about: "scp from machine 0 to unit mysql/0", |
110 | + args: []string{"0:foo", "mysql/0:/foo"}, |
111 | + result: commonArgsNoProxy + "ubuntu@dummyenv-0.dns:foo ubuntu@dummyenv-0.dns:/foo\n", |
112 | + }, |
113 | + { |
114 | + about: "scp from machine 0 to unit mysql/0 and extra args", |
115 | + args: []string{"0:foo", "mysql/0:/foo", "-q"}, |
116 | + result: commonArgsNoProxy + "-q ubuntu@dummyenv-0.dns:foo ubuntu@dummyenv-0.dns:/foo\n", |
117 | + }, |
118 | + { |
119 | + about: "scp from machine 0 to unit mysql/0 and extra args before", |
120 | + args: []string{"-q", "-r", "0:foo", "mysql/0:/foo"}, |
121 | + error: `unexpected argument "-q"; extra arguments must be last`, |
122 | + }, |
123 | + { |
124 | + about: "scp two local files to unit mysql/0", |
125 | + args: []string{"file1", "file2", "mysql/0:/foo/"}, |
126 | + result: commonArgsNoProxy + "file1 file2 ubuntu@dummyenv-0.dns:/foo/\n", |
127 | + }, |
128 | + { |
129 | + about: "scp from unit mongodb/1 to unit mongodb/0 and multiple extra args", |
130 | + args: []string{"mongodb/1:foo", "mongodb/0:", "-r -v -q -l5"}, |
131 | + result: commonArgsNoProxy + "-r -v -q -l5 ubuntu@dummyenv-2.dns:foo ubuntu@dummyenv-1.dns:\n", |
132 | + }, |
133 | + { |
134 | + about: "scp works with IPv6 addresses", |
135 | + args: []string{"ipv6-svc/0:foo", "bar"}, |
136 | + result: commonArgsNoProxy + `ubuntu@\[2001:db8::\]:foo bar` + "\n", |
137 | + }, |
138 | + { |
139 | + about: "scp from machine 0 to unit mysql/0 with proxy", |
140 | + args: []string{"0:foo", "mysql/0:/foo"}, |
141 | + result: commonArgs + "ubuntu@dummyenv-0.dns:foo ubuntu@dummyenv-0.dns:/foo\n", |
142 | + proxy: true, |
143 | }, |
144 | } |
145 | |
146 | @@ -122,6 +119,7 @@ |
147 | c.Logf("test %d: %s -> %s\n", i, t.about, t.args) |
148 | ctx := coretesting.Context(c) |
149 | scpcmd := &SCPCommand{} |
150 | + scpcmd.proxy = t.proxy |
151 | |
152 | err := scpcmd.Init(t.args) |
153 | c.Check(err, gc.IsNil) |
154 | |
155 | === modified file 'cmd/juju/ssh.go' |
156 | --- cmd/juju/ssh.go 2014-02-22 13:45:10 +0000 |
157 | +++ cmd/juju/ssh.go 2014-03-19 03:21:48 +0000 |
158 | @@ -6,8 +6,13 @@ |
159 | import ( |
160 | "errors" |
161 | "fmt" |
162 | + "net" |
163 | + "os" |
164 | + "os/exec" |
165 | "time" |
166 | |
167 | + "launchpad.net/gnuflag" |
168 | + |
169 | "launchpad.net/juju-core/cmd" |
170 | "launchpad.net/juju-core/instance" |
171 | "launchpad.net/juju-core/juju" |
172 | @@ -26,13 +31,34 @@ |
173 | // SSHCommon provides common methods for SSHCommand, SCPCommand and DebugHooksCommand. |
174 | type SSHCommon struct { |
175 | cmd.EnvCommandBase |
176 | + proxy bool |
177 | Target string |
178 | Args []string |
179 | apiClient *api.Client |
180 | + apiAddr string |
181 | // Only used for compatibility with 1.16 |
182 | rawConn *juju.Conn |
183 | } |
184 | |
185 | +func (c *SSHCommon) SetFlags(f *gnuflag.FlagSet) { |
186 | + c.EnvCommandBase.SetFlags(f) |
187 | + f.BoolVar(&c.proxy, "proxy", true, "proxy through the API server") |
188 | +} |
189 | + |
190 | +// setProxyCommand sets the proxy command option. |
191 | +func (c *SSHCommon) setProxyCommand(options *ssh.Options) error { |
192 | + apiServerHost, _, err := net.SplitHostPort(c.apiAddr) |
193 | + if err != nil { |
194 | + return fmt.Errorf("failed to get proxy address: %v", err) |
195 | + } |
196 | + juju, err := getJujuExecutable() |
197 | + if err != nil { |
198 | + return fmt.Errorf("failed to get juju executable path: %v", err) |
199 | + } |
200 | + options.SetProxyCommand(juju, "ssh", "--proxy=false", apiServerHost, "-T", "nc -q0 %h %p") |
201 | + return nil |
202 | +} |
203 | + |
204 | const sshDoc = ` |
205 | Launch an ssh shell on the machine identified by the <target> parameter. |
206 | <target> can be either a machine id as listed by "juju status" in the |
207 | @@ -75,6 +101,12 @@ |
208 | return nil |
209 | } |
210 | |
211 | +// getJujuExecutable returns the path to the juju |
212 | +// executable, or an error if it could not be found. |
213 | +var getJujuExecutable = func() (string, error) { |
214 | + return exec.LookPath(os.Args[0]) |
215 | +} |
216 | + |
217 | // Run resolves c.Target to a machine, to the address of a i |
218 | // machine or unit forks ssh passing any arguments provided. |
219 | func (c *SSHCommand) Run(ctx *cmd.Context) error { |
220 | @@ -92,6 +124,11 @@ |
221 | } |
222 | var options ssh.Options |
223 | options.EnablePTY() |
224 | + if c.proxy { |
225 | + if err := c.setProxyCommand(&options); err != nil { |
226 | + return err |
227 | + } |
228 | + } |
229 | cmd := ssh.Command("ubuntu@"+host, c.Args, &options) |
230 | cmd.Stdin = ctx.Stdin |
231 | cmd.Stdout = ctx.Stdout |
232 | @@ -102,9 +139,13 @@ |
233 | // initAPIClient initialises the API connection. |
234 | // It is the caller's responsibility to close the connection. |
235 | func (c *SSHCommon) initAPIClient() (*api.Client, error) { |
236 | - var err error |
237 | - c.apiClient, err = juju.NewAPIClientFromName(c.EnvName) |
238 | - return c.apiClient, err |
239 | + st, err := juju.NewAPIFromName(c.EnvName) |
240 | + if err != nil { |
241 | + return nil, err |
242 | + } |
243 | + c.apiClient = st.Client() |
244 | + c.apiAddr = st.Addr() |
245 | + return c.apiClient, nil |
246 | } |
247 | |
248 | // attemptStarter is an interface corresponding to utils.AttemptStrategy |
249 | @@ -158,9 +199,15 @@ |
250 | if err != nil { |
251 | return "", err |
252 | } |
253 | - addr := instance.SelectPublicAddress(machine.Addresses()) |
254 | - if addr == "" { |
255 | - return "", fmt.Errorf("machine %q has no public address", machine) |
256 | + var addr string |
257 | + if c.proxy { |
258 | + if addr = instance.SelectInternalAddress(machine.Addresses(), false); addr == "" { |
259 | + return "", fmt.Errorf("machine %q has no internal address", machine) |
260 | + } |
261 | + } else { |
262 | + if addr = instance.SelectPublicAddress(machine.Addresses()); addr == "" { |
263 | + return "", fmt.Errorf("machine %q has no public address", machine) |
264 | + } |
265 | } |
266 | return addr, nil |
267 | } |
268 | @@ -171,9 +218,16 @@ |
269 | if err != nil { |
270 | return "", err |
271 | } |
272 | - addr, ok := unit.PublicAddress() |
273 | - if !ok { |
274 | - return "", fmt.Errorf("unit %q has no public address", unit) |
275 | + var addr string |
276 | + var ok bool |
277 | + if c.proxy { |
278 | + if addr, ok = unit.PrivateAddress(); !ok { |
279 | + return "", fmt.Errorf("unit %q has no internal address", unit) |
280 | + } |
281 | + } else { |
282 | + if addr, ok = unit.PublicAddress(); !ok { |
283 | + return "", fmt.Errorf("unit %q has no public address", unit) |
284 | + } |
285 | } |
286 | return addr, nil |
287 | } |
288 | @@ -181,6 +235,11 @@ |
289 | } |
290 | |
291 | func (c *SSHCommon) hostFromTarget(target string) (string, error) { |
292 | + // If the target is neither a machine nor a unit, |
293 | + // assume it's a hostname and try it directly. |
294 | + if !names.IsMachine(target) && !names.IsUnit(target) { |
295 | + return target, nil |
296 | + } |
297 | var addr string |
298 | var err error |
299 | var useStateConn bool |
300 | @@ -189,9 +248,13 @@ |
301 | // a loop. |
302 | for a := sshHostFromTargetAttemptStrategy.Start(); a.Next(); { |
303 | if !useStateConn { |
304 | - addr, err = c.apiClient.PublicAddress(target) |
305 | + if c.proxy { |
306 | + addr, err = c.apiClient.PrivateAddress(target) |
307 | + } else { |
308 | + addr, err = c.apiClient.PublicAddress(target) |
309 | + } |
310 | if params.IsCodeNotImplemented(err) { |
311 | - logger.Infof("API server does not support Client.PublicAddress falling back to 1.16 compatibility mode (direct DB access)") |
312 | + logger.Infof("API server does not support Client.PrivateAddress falling back to 1.16 compatibility mode (direct DB access)") |
313 | useStateConn = true |
314 | } |
315 | } |
316 | |
317 | === modified file 'cmd/juju/ssh_test.go' |
318 | --- cmd/juju/ssh_test.go 2014-02-22 13:45:10 +0000 |
319 | +++ cmd/juju/ssh_test.go 2014-03-19 03:21:48 +0000 |
320 | @@ -39,6 +39,7 @@ |
321 | |
322 | func (s *SSHCommonSuite) SetUpTest(c *gc.C) { |
323 | s.JujuConnSuite.SetUpTest(c) |
324 | + s.PatchValue(&getJujuExecutable, func() (string, error) { return "juju", nil }) |
325 | |
326 | s.bin = c.MkDir() |
327 | s.PatchEnvPathPrepend(s.bin) |
328 | @@ -53,8 +54,10 @@ |
329 | } |
330 | |
331 | const ( |
332 | - commonArgs = `-o StrictHostKeyChecking no -o PasswordAuthentication no ` |
333 | - sshArgs = commonArgs + `-t -t ` |
334 | + commonArgsNoProxy = `-o StrictHostKeyChecking no -o PasswordAuthentication no ` |
335 | + commonArgs = `-o StrictHostKeyChecking no -o ProxyCommand juju ssh --proxy=false 127.0.0.1 -T "nc -q0 %h %p" -o PasswordAuthentication no ` |
336 | + sshArgs = commonArgs + `-t -t ` |
337 | + sshArgsNoProxy = commonArgsNoProxy + `-t -t ` |
338 | ) |
339 | |
340 | var sshTests = []struct { |
341 | @@ -82,6 +85,11 @@ |
342 | []string{"ssh", "mongodb/1", "ls", "/"}, |
343 | sshArgs + "ubuntu@dummyenv-2.dns ls /\n", |
344 | }, |
345 | + { |
346 | + "connect to unit mysql/0 without proxy", |
347 | + []string{"ssh", "--proxy=false", "mysql/0"}, |
348 | + sshArgsNoProxy + "ubuntu@dummyenv-0.dns\n", |
349 | + }, |
350 | } |
351 | |
352 | func (s *SSHSuite) TestSSHCommand(c *gc.C) { |
353 | |
354 | === modified file 'juju/api.go' |
355 | --- juju/api.go 2014-03-12 10:59:17 +0000 |
356 | +++ juju/api.go 2014-03-19 03:21:48 +0000 |
357 | @@ -100,17 +100,24 @@ |
358 | return keymanager.NewClient(st), nil |
359 | } |
360 | |
361 | +// NewAPIFromName returns an api.State connected to the API Server for |
362 | +// the named environment. If envName is "", the default environment will |
363 | +// be used. |
364 | +func NewAPIFromName(envName string) (*api.State, error) { |
365 | + return newAPIClient(envName) |
366 | +} |
367 | + |
368 | func newAPIClient(envName string) (*api.State, error) { |
369 | store, err := configstore.NewDisk(osenv.JujuHome()) |
370 | if err != nil { |
371 | return nil, err |
372 | } |
373 | - return newAPIFromName(envName, store) |
374 | + return newAPIFromStore(envName, store) |
375 | } |
376 | |
377 | -// newAPIFromName implements the bulk of NewAPIClientFromName |
378 | +// newAPIFromStore implements the bulk of NewAPIClientFromName |
379 | // but is separate for testing purposes. |
380 | -func newAPIFromName(envName string, store configstore.Storage) (*api.State, error) { |
381 | +func newAPIFromStore(envName string, store configstore.Storage) (*api.State, error) { |
382 | // Try to read the default environment configuration file. |
383 | // If it doesn't exist, we carry on in case |
384 | // there's some environment info for that environment. |
385 | |
386 | === modified file 'juju/apiconn_test.go' |
387 | --- juju/apiconn_test.go 2014-03-13 07:54:56 +0000 |
388 | +++ juju/apiconn_test.go 2014-03-19 03:21:48 +0000 |
389 | @@ -139,11 +139,11 @@ |
390 | called++ |
391 | return expectState, nil |
392 | } |
393 | - // Give NewAPIFromName a store interface that can report when the |
394 | + // Give NewAPIFromStore a store interface that can report when the |
395 | // config was written to, to ensure the cache isn't updated. |
396 | defer testbase.PatchValue(juju.APIOpen, apiOpen).Restore() |
397 | mockStore := &storageWithWriteNotify{store: store} |
398 | - st, err := juju.NewAPIFromName("noconfig", mockStore) |
399 | + st, err := juju.NewAPIFromStore("noconfig", mockStore) |
400 | c.Assert(err, gc.IsNil) |
401 | c.Assert(st, gc.Equals, expectState) |
402 | c.Assert(called, gc.Equals, 1) |
403 | @@ -186,7 +186,7 @@ |
404 | return expectState, nil |
405 | } |
406 | defer testbase.PatchValue(juju.APIOpen, apiOpen).Restore() |
407 | - st, err := juju.NewAPIFromName("myenv", store) |
408 | + st, err := juju.NewAPIFromStore("myenv", store) |
409 | c.Assert(err, gc.IsNil) |
410 | c.Assert(st, gc.Equals, expectState) |
411 | c.Assert(called, gc.Equals, 1) |
412 | @@ -209,7 +209,7 @@ |
413 | expectErr := fmt.Errorf("an error") |
414 | store := newConfigStoreWithError(expectErr) |
415 | defer testbase.PatchValue(juju.APIOpen, panicAPIOpen).Restore() |
416 | - client, err := juju.NewAPIFromName("noconfig", store) |
417 | + client, err := juju.NewAPIFromStore("noconfig", store) |
418 | c.Assert(err, gc.Equals, expectErr) |
419 | c.Assert(client, gc.IsNil) |
420 | } |
421 | @@ -228,7 +228,7 @@ |
422 | }) |
423 | defer testbase.PatchValue(juju.APIOpen, panicAPIOpen).Restore() |
424 | |
425 | - st, err := juju.NewAPIFromName("noconfig", store) |
426 | + st, err := juju.NewAPIFromStore("noconfig", store) |
427 | c.Assert(err, gc.ErrorMatches, `environment "noconfig" not found`) |
428 | c.Assert(st, gc.IsNil) |
429 | } |
430 | @@ -246,7 +246,7 @@ |
431 | return nil, expectErr |
432 | } |
433 | defer testbase.PatchValue(juju.APIOpen, apiOpen).Restore() |
434 | - st, err := juju.NewAPIFromName("noconfig", store) |
435 | + st, err := juju.NewAPIFromStore("noconfig", store) |
436 | c.Assert(err, gc.Equals, expectErr) |
437 | c.Assert(st, gc.IsNil) |
438 | } |
439 | @@ -277,7 +277,7 @@ |
440 | defer restoreAPIClose.Restore() |
441 | |
442 | startTime := time.Now() |
443 | - st, err := juju.NewAPIFromName(coretesting.SampleEnvName, store) |
444 | + st, err := juju.NewAPIFromStore(coretesting.SampleEnvName, store) |
445 | c.Assert(err, gc.IsNil) |
446 | // The connection logic should wait for some time before opening |
447 | // the API from the configuration. |
448 | @@ -342,7 +342,7 @@ |
449 | |
450 | done := make(chan struct{}) |
451 | go func() { |
452 | - st, err := juju.NewAPIFromName(coretesting.SampleEnvName, store) |
453 | + st, err := juju.NewAPIFromStore(coretesting.SampleEnvName, store) |
454 | c.Check(err, gc.IsNil) |
455 | c.Check(st, gc.Equals, infoOpenedState) |
456 | close(done) |
457 | @@ -360,7 +360,7 @@ |
458 | c.Fatalf("api never opened via config") |
459 | } |
460 | // Let the info endpoint open go ahead and |
461 | - // check that the NewAPIFromName call returns. |
462 | + // check that the NewAPIFromStore call returns. |
463 | infoEndpointOpened <- struct{}{} |
464 | select { |
465 | case <-done: |
466 | @@ -393,7 +393,7 @@ |
467 | return nil, fmt.Errorf("config connect failed") |
468 | } |
469 | defer testbase.PatchValue(juju.APIOpen, apiOpen).Restore() |
470 | - st, err := juju.NewAPIFromName(coretesting.SampleEnvName, store) |
471 | + st, err := juju.NewAPIFromStore(coretesting.SampleEnvName, store) |
472 | c.Check(err, gc.ErrorMatches, "config connect failed") |
473 | c.Check(st, gc.IsNil) |
474 | } |
475 | @@ -426,7 +426,7 @@ |
476 | err = os.Remove(osenv.JujuHomePath("environments.yaml")) |
477 | c.Assert(err, gc.IsNil) |
478 | |
479 | - st, err := juju.NewAPIFromName(coretesting.SampleEnvName, store) |
480 | + st, err := juju.NewAPIFromStore(coretesting.SampleEnvName, store) |
481 | c.Check(err, gc.IsNil) |
482 | st.Close() |
483 | } |
484 | @@ -454,7 +454,7 @@ |
485 | // Now we have info for envName2 which will actually |
486 | // cause a connection to the originally bootstrapped |
487 | // state. |
488 | - st, err := juju.NewAPIFromName(envName2, store) |
489 | + st, err := juju.NewAPIFromStore(envName2, store) |
490 | c.Check(err, gc.IsNil) |
491 | st.Close() |
492 | |
493 | @@ -464,7 +464,7 @@ |
494 | // Disable for now until an upcoming branch fixes it. |
495 | // err = info2.Destroy() |
496 | // c.Assert(err, gc.IsNil) |
497 | - // st, err = juju.NewAPIFromName(envName2, store) |
498 | + // st, err = juju.NewAPIFromStore(envName2, store) |
499 | // if err == nil { |
500 | // st.Close() |
501 | // } |
502 | |
503 | === modified file 'juju/export_test.go' |
504 | --- juju/export_test.go 2013-12-04 10:18:57 +0000 |
505 | +++ juju/export_test.go 2014-03-19 03:21:48 +0000 |
506 | @@ -4,5 +4,5 @@ |
507 | APIOpen = &apiOpen |
508 | APIClose = &apiClose |
509 | ProviderConnectDelay = &providerConnectDelay |
510 | - NewAPIFromName = newAPIFromName |
511 | + NewAPIFromStore = newAPIFromStore |
512 | ) |
513 | |
514 | === modified file 'state/api/apiclient.go' |
515 | --- state/api/apiclient.go 2014-02-20 01:09:59 +0000 |
516 | +++ state/api/apiclient.go 2014-03-19 03:21:48 +0000 |
517 | @@ -25,6 +25,7 @@ |
518 | type State struct { |
519 | client *rpc.Conn |
520 | conn *websocket.Conn |
521 | + addr string |
522 | |
523 | // authTag holds the authenticated entity's tag after login. |
524 | authTag string |
525 | @@ -127,6 +128,7 @@ |
526 | st := &State{ |
527 | client: client, |
528 | conn: conn, |
529 | + addr: cfg.Location.Host, |
530 | serverRoot: "https://" + cfg.Location.Host, |
531 | tag: info.Tag, |
532 | password: info.Password, |
533 | @@ -186,3 +188,8 @@ |
534 | func (s *State) RPCClient() *rpc.Conn { |
535 | return s.client |
536 | } |
537 | + |
538 | +// Addr returns the address used to connect to the RPC server. |
539 | +func (s *State) Addr() string { |
540 | + return s.addr |
541 | +} |
542 | |
543 | === modified file 'state/api/client.go' |
544 | --- state/api/client.go 2014-03-13 22:47:05 +0000 |
545 | +++ state/api/client.go 2014-03-19 03:21:48 +0000 |
546 | @@ -143,6 +143,15 @@ |
547 | return results.PublicAddress, err |
548 | } |
549 | |
550 | +// PrivateAddress returns the private address of the specified |
551 | +// machine or unit. |
552 | +func (c *Client) PrivateAddress(target string) (string, error) { |
553 | + var results params.PrivateAddressResults |
554 | + p := params.PrivateAddress{Target: target} |
555 | + err := c.st.Call("Client", "", "PrivateAddress", p, &results) |
556 | + return results.PrivateAddress, err |
557 | +} |
558 | + |
559 | // ServiceSetYAML sets configuration options on a service |
560 | // given options in YAML format. |
561 | func (c *Client) ServiceSetYAML(service string, yaml string) error { |
562 | |
563 | === modified file 'state/api/params/params.go' |
564 | --- state/api/params/params.go 2014-03-13 13:42:50 +0000 |
565 | +++ state/api/params/params.go 2014-03-19 03:21:48 +0000 |
566 | @@ -236,6 +236,16 @@ |
567 | PublicAddress string |
568 | } |
569 | |
570 | +// PrivateAddress holds parameters for the PrivateAddress call. |
571 | +type PrivateAddress struct { |
572 | + Target string |
573 | +} |
574 | + |
575 | +// PrivateAddressResults holds results of the PrivateAddress call. |
576 | +type PrivateAddressResults struct { |
577 | + PrivateAddress string |
578 | +} |
579 | + |
580 | // Resolved holds parameters for the Resolved call. |
581 | type Resolved struct { |
582 | UnitName string |
583 | |
584 | === modified file 'state/apiserver/client/client.go' |
585 | --- state/apiserver/client/client.go 2014-03-18 02:36:58 +0000 |
586 | +++ state/apiserver/client/client.go 2014-03-19 03:21:48 +0000 |
587 | @@ -184,6 +184,34 @@ |
588 | return results, fmt.Errorf("unknown unit or machine %q", p.Target) |
589 | } |
590 | |
591 | +// PrivateAddress implements the server side of Client.PrivateAddress. |
592 | +func (c *Client) PrivateAddress(p params.PrivateAddress) (results params.PrivateAddressResults, err error) { |
593 | + switch { |
594 | + case names.IsMachine(p.Target): |
595 | + machine, err := c.api.state.Machine(p.Target) |
596 | + if err != nil { |
597 | + return results, err |
598 | + } |
599 | + addr := instance.SelectInternalAddress(machine.Addresses(), false) |
600 | + if addr == "" { |
601 | + return results, fmt.Errorf("machine %q has no internal address", machine) |
602 | + } |
603 | + return params.PrivateAddressResults{PrivateAddress: addr}, nil |
604 | + |
605 | + case names.IsUnit(p.Target): |
606 | + unit, err := c.api.state.Unit(p.Target) |
607 | + if err != nil { |
608 | + return results, err |
609 | + } |
610 | + addr, ok := unit.PrivateAddress() |
611 | + if !ok { |
612 | + return results, fmt.Errorf("unit %q has no internal address", unit) |
613 | + } |
614 | + return params.PrivateAddressResults{PrivateAddress: addr}, nil |
615 | + } |
616 | + return results, fmt.Errorf("unknown unit or machine %q", p.Target) |
617 | +} |
618 | + |
619 | // ServiceExpose changes the juju-managed firewall to expose any ports that |
620 | // were also explicitly marked by units as open. |
621 | func (c *Client) ServiceExpose(args params.ServiceExpose) error { |
622 | |
623 | === modified file 'state/apiserver/client/client_test.go' |
624 | --- state/apiserver/client/client_test.go 2014-03-18 02:36:58 +0000 |
625 | +++ state/apiserver/client/client_test.go 2014-03-19 03:21:48 +0000 |
626 | @@ -1413,6 +1413,66 @@ |
627 | c.Assert(addr, gc.Equals, "127.0.0.1") |
628 | } |
629 | |
630 | +func (s *clientSuite) TestClientPrivateAddressErrors(c *gc.C) { |
631 | + s.setUpScenario(c) |
632 | + _, err := s.APIState.Client().PrivateAddress("wordpress") |
633 | + c.Assert(err, gc.ErrorMatches, `unknown unit or machine "wordpress"`) |
634 | + _, err = s.APIState.Client().PrivateAddress("0") |
635 | + c.Assert(err, gc.ErrorMatches, `machine "0" has no internal address`) |
636 | + _, err = s.APIState.Client().PrivateAddress("wordpress/0") |
637 | + c.Assert(err, gc.ErrorMatches, `unit "wordpress/0" has no internal address`) |
638 | +} |
639 | + |
640 | +func (s *clientSuite) TestClientPrivateAddressMachine(c *gc.C) { |
641 | + s.setUpScenario(c) |
642 | + |
643 | + // Internally, instance.SelectInternalAddress is used; the public |
644 | + // address if no cloud-local one is available. |
645 | + m1, err := s.State.Machine("1") |
646 | + c.Assert(err, gc.IsNil) |
647 | + cloudLocalAddress := instance.NewAddress("cloudlocal") |
648 | + cloudLocalAddress.NetworkScope = instance.NetworkCloudLocal |
649 | + publicAddress := instance.NewAddress("public") |
650 | + publicAddress.NetworkScope = instance.NetworkCloudLocal |
651 | + err = m1.SetAddresses([]instance.Address{publicAddress}) |
652 | + c.Assert(err, gc.IsNil) |
653 | + addr, err := s.APIState.Client().PrivateAddress("1") |
654 | + c.Assert(err, gc.IsNil) |
655 | + c.Assert(addr, gc.Equals, "public") |
656 | + err = m1.SetAddresses([]instance.Address{cloudLocalAddress, publicAddress}) |
657 | + addr, err = s.APIState.Client().PrivateAddress("1") |
658 | + c.Assert(err, gc.IsNil) |
659 | + c.Assert(addr, gc.Equals, "cloudlocal") |
660 | +} |
661 | + |
662 | +func (s *clientSuite) TestClientPrivateAddressUnitWithMachine(c *gc.C) { |
663 | + s.setUpScenario(c) |
664 | + |
665 | + // Private address of unit is taken from its machine |
666 | + // (if its machine has addresses). |
667 | + m1, err := s.State.Machine("1") |
668 | + publicAddress := instance.NewAddress("public") |
669 | + publicAddress.NetworkScope = instance.NetworkCloudLocal |
670 | + err = m1.SetAddresses([]instance.Address{publicAddress}) |
671 | + c.Assert(err, gc.IsNil) |
672 | + addr, err := s.APIState.Client().PrivateAddress("wordpress/0") |
673 | + c.Assert(err, gc.IsNil) |
674 | + c.Assert(addr, gc.Equals, "public") |
675 | +} |
676 | + |
677 | +func (s *clientSuite) TestClientPrivateAddressUnitWithoutMachine(c *gc.C) { |
678 | + s.setUpScenario(c) |
679 | + // If the unit's machine has no addresses, the public address |
680 | + // comes from the unit's document. |
681 | + u, err := s.State.Unit("wordpress/1") |
682 | + c.Assert(err, gc.IsNil) |
683 | + err = u.SetPrivateAddress("127.0.0.1") |
684 | + c.Assert(err, gc.IsNil) |
685 | + addr, err := s.APIState.Client().PrivateAddress("wordpress/1") |
686 | + c.Assert(err, gc.IsNil) |
687 | + c.Assert(addr, gc.Equals, "127.0.0.1") |
688 | +} |
689 | + |
690 | func (s *clientSuite) TestClientEnvironmentGet(c *gc.C) { |
691 | envConfig, err := s.State.EnvironConfig() |
692 | c.Assert(err, gc.IsNil) |
693 | |
694 | === modified file 'utils/ssh/ssh.go' |
695 | --- utils/ssh/ssh.go 2014-03-10 20:22:44 +0000 |
696 | +++ utils/ssh/ssh.go 2014-03-19 03:21:48 +0000 |
697 | @@ -19,6 +19,9 @@ |
698 | |
699 | // Options is a client-implementation independent SSH options set. |
700 | type Options struct { |
701 | + // proxyCommand specifies the command to |
702 | + // execute to proxy SSH traffic through. |
703 | + proxyCommand []string |
704 | // ssh server port; zero means use the default (22) |
705 | port int |
706 | // no PTY forced by default |
707 | @@ -31,6 +34,11 @@ |
708 | identities []string |
709 | } |
710 | |
711 | +// SetProxyCommand sets a command to execute to proxy traffic through. |
712 | +func (o *Options) SetProxyCommand(command ...string) { |
713 | + o.proxyCommand = append([]string{}, command...) |
714 | +} |
715 | + |
716 | // SetPort sets the SSH server port to connect to. |
717 | func (o *Options) SetPort(port int) { |
718 | o.port = port |
719 | |
720 | === modified file 'utils/ssh/ssh_openssh.go' |
721 | --- utils/ssh/ssh_openssh.go 2014-02-22 13:45:10 +0000 |
722 | +++ utils/ssh/ssh_openssh.go 2014-03-19 03:21:48 +0000 |
723 | @@ -71,6 +71,9 @@ |
724 | if options == nil { |
725 | options = &Options{} |
726 | } |
727 | + if len(options.proxyCommand) > 0 { |
728 | + args["-o"] = append(args["-o"], "ProxyCommand "+utils.CommandString(options.proxyCommand...)) |
729 | + } |
730 | if !options.passwordAuthAllowed { |
731 | args["-o"] = append(args["-o"], "PasswordAuthentication no") |
732 | } |
Reviewers: mp+211662_ code.launchpad. net,
Message:
Please take a look.
Description:
Add --proxy option to "juju ssh"
juju ssh will now, by default, proxy
connections through the API server host.
I have not yet implemented support for the
ProxyCommand option for the go.crypto/ssh
client; this will be done as a follow up.
https:/ /code.launchpad .net/~axwalk/ juju-core/ ssh-proxycomman d/+merge/ 211662
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/77340046/
Affected files (+300, -89 lines): scp_test. go ssh_test. go test.go apiclient. go params/ params. go /client/ client. go /client/ client_ test.go ssh_openssh. go
A [revision details]
M cmd/juju/scp.go
M cmd/juju/
M cmd/juju/ssh.go
M cmd/juju/
M juju/api.go
M juju/apiconn_
M juju/export_test.go
M state/api/
M state/api/client.go
M state/api/
M state/apiserver
M state/apiserver
M utils/ssh/ssh.go
M utils/ssh/