Merge lp:~axwalk/juju-core/ssh-proxycommand into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

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://codereview.appspot.com/77340046/

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://codereview.appspot.com/77340046/

[Andrew Wilkins (axwalk)]

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-proxycommand/+merge/211662

(do not edit description out of merge proposal)

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

Affected files (+300, -89 lines):
   A [revision details]
   M cmd/juju/scp.go
   M cmd/juju/scp_test.go
   M cmd/juju/ssh.go
   M cmd/juju/ssh_test.go
   M juju/api.go
   M juju/apiconn_test.go
   M juju/export_test.go
   M state/api/apiclient.go
   M state/api/client.go
   M state/api/params/params.go
   M state/apiserver/client/client.go
   M state/apiserver/client/client_test.go
   M utils/ssh/ssh.go
   M utils/ssh/ssh_openssh.go

[Tim Penhey (thumper)]

LGTM - the code looks fine, and I'm assuming you have tested it.

https://codereview.appspot.com/77340046/

[Andrew Wilkins (axwalk)]

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.

https://codereview.appspot.com/77340046/

=== modified file 'cmd/juju/scp.go'
--- cmd/juju/scp.go	2014-02-24 16:21:43 +0000
+++ cmd/juju/scp.go	2014-03-19 03:21:48 +0000
@@ -106,5 +106,13 @@
 			targets = append(targets, arg)
 		}
 	}
-	return ssh.Copy(targets, extraArgs, nil)
+
+	var options *ssh.Options
+	if c.proxy {
+		options = new(ssh.Options)
+		if err := c.setProxyCommand(options); err != nil {
+			return err
+		}
+	}
+	return ssh.Copy(targets, extraArgs, options)
 }
 
=== modified file 'cmd/juju/scp_test.go'
--- cmd/juju/scp_test.go	2014-02-24 16:21:43 +0000
+++ cmd/juju/scp_test.go	2014-03-19 03:21:48 +0000
@@ -27,67 +27,64 @@
 	about  string
 	args   []string
 	result string
+	proxy  bool
 	error  string
 }{
 	{
-		"scp from machine 0 to current dir",
-		[]string{"0:foo", "."},
-		commonArgs + "[e-mail]:foo .\n",
-		"",
-	},
-	{
-		"scp from machine 0 to current dir with extra args",
-		[]string{"0:foo", ".", "-rv -o SomeOption"},
-		commonArgs + "-rv -o SomeOption [e-mail]:foo .\n",
-		"",
-	},
-	{
-		"scp from current dir to machine 0",
-		[]string{"foo", "0:"},
-		commonArgs + "foo [e-mail]:\n",
-		"",
-	},
-	{
-		"scp from current dir to machine 0 with extra args",
-		[]string{"foo", "0:", "-r -v"},
-		commonArgs + "-r -v foo [e-mail]:\n",
-		"",
-	},
-	{
-		"scp from machine 0 to unit mysql/0",
-		[]string{"0:foo", "mysql/0:/foo"},
-		commonArgs + "[e-mail]:foo [e-mail]:/foo\n",
-		"",
-	},
-	{
-		"scp from machine 0 to unit mysql/0 and extra args",
-		[]string{"0:foo", "mysql/0:/foo", "-q"},
-		commonArgs + "-q [e-mail]:foo [e-mail]:/foo\n",
-		"",
-	},
-	{
-		"scp from machine 0 to unit mysql/0 and extra args before",
-		[]string{"-q", "-r", "0:foo", "mysql/0:/foo"},
-		"",
-		`unexpected argument "-q"; extra arguments must be last`,
-	},
-	{
-		"scp two local files to unit mysql/0",
-		[]string{"file1", "file2", "mysql/0:/foo/"},
-		commonArgs + "file1 file2 [e-mail]:/foo/\n",
-		"",
-	},
-	{
-		"scp from unit mongodb/1 to unit mongodb/0 and multiple extra args",
-		[]string{"mongodb/1:foo", "mongodb/0:", "-r -v -q -l5"},
-		commonArgs + "-r -v -q -l5 [e-mail]:foo [e-mail]:\n",
-		"",
-	},
-	{
-		"scp works with IPv6 addresses",
-		[]string{"ipv6-svc/0:foo", "bar"},
-		commonArgs + `ubuntu@\[2001:db8::\]:foo bar` + "\n",
-		"",
+		about:  "scp from machine 0 to current dir",
+		args:   []string{"0:foo", "."},
+		result: commonArgsNoProxy + "[e-mail]:foo .\n",
+	},
+	{
+		about:  "scp from machine 0 to current dir with extra args",
+		args:   []string{"0:foo", ".", "-rv -o SomeOption"},
+		result: commonArgsNoProxy + "-rv -o SomeOption [e-mail]:foo .\n",
+	},
+	{
+		about:  "scp from current dir to machine 0",
+		args:   []string{"foo", "0:"},
+		result: commonArgsNoProxy + "foo [e-mail]:\n",
+	},
+	{
+		about:  "scp from current dir to machine 0 with extra args",
+		args:   []string{"foo", "0:", "-r -v"},
+		result: commonArgsNoProxy + "-r -v foo [e-mail]:\n",
+	},
+	{
+		about:  "scp from machine 0 to unit mysql/0",
+		args:   []string{"0:foo", "mysql/0:/foo"},
+		result: commonArgsNoProxy + "[e-mail]:foo [e-mail]:/foo\n",
+	},
+	{
+		about:  "scp from machine 0 to unit mysql/0 and extra args",
+		args:   []string{"0:foo", "mysql/0:/foo", "-q"},
+		result: commonArgsNoProxy + "-q [e-mail]:foo [e-mail]:/foo\n",
+	},
+	{
+		about: "scp from machine 0 to unit mysql/0 and extra args before",
+		args:  []string{"-q", "-r", "0:foo", "mysql/0:/foo"},
+		error: `unexpected argument "-q"; extra arguments must be last`,
+	},
+	{
+		about:  "scp two local files to unit mysql/0",
+		args:   []string{"file1", "file2", "mysql/0:/foo/"},
+		result: commonArgsNoProxy + "file1 file2 [e-mail]:/foo/\n",
+	},
+	{
+		about:  "scp from unit mongodb/1 to unit mongodb/0 and multiple extra args",
+		args:   []string{"mongodb/1:foo", "mongodb/0:", "-r -v -q -l5"},
+		result: commonArgsNoProxy + "-r -v -q -l5 [e-mail]:foo [e-mail]:\n",
+	},
+	{
+		about:  "scp works with IPv6 addresses",
+		args:   []string{"ipv6-svc/0:foo", "bar"},
+		result: commonArgsNoProxy + `ubuntu@\[2001:db8::\]:foo bar` + "\n",
+	},
+	{
+		about:  "scp from machine 0 to unit mysql/0 with proxy",
+		args:   []string{"0:foo", "mysql/0:/foo"},
+		result: commonArgs + "[e-mail]:foo [e-mail]:/foo\n",
+		proxy:  true,
 	},
 }
 
@@ -122,6 +119,7 @@
 		c.Logf("test %d: %s -> %s\n", i, t.about, t.args)
 		ctx := coretesting.Context(c)
 		scpcmd := &SCPCommand{}
+		scpcmd.proxy = t.proxy
 
 		err := scpcmd.Init(t.args)
 		c.Check(err, gc.IsNil)
 
=== modified file 'cmd/juju/ssh.go'
--- cmd/juju/ssh.go	2014-02-22 13:45:10 +0000
+++ cmd/juju/ssh.go	2014-03-19 03:21:48 +0000
@@ -6,8 +6,13 @@
 import (
 	"errors"
 	"fmt"
+	"net"
+	"os"
+	"os/exec"
 	"time"
 
+	"launchpad.net/gnuflag"
+
 	"launchpad.net/juju-core/cmd"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju"
@@ -26,13 +31,34 @@
 // SSHCommon provides common methods for SSHCommand, SCPCommand and DebugHooksCommand.
 type SSHCommon struct {
 	cmd.EnvCommandBase
+	proxy     bool
 	Target    string
 	Args      []string
 	apiClient *api.Client
+	apiAddr   string
 	// Only used for compatibility with 1.16
 	rawConn *juju.Conn
 }
 
+func (c *SSHCommon) SetFlags(f *gnuflag.FlagSet) {
+	c.EnvCommandBase.SetFlags(f)
+	f.BoolVar(&c.proxy, "proxy", true, "proxy through the API server")
+}
+
+// setProxyCommand sets the proxy command option.
+func (c *SSHCommon) setProxyCommand(options *ssh.Options) error {
+	apiServerHost, _, err := net.SplitHostPort(c.apiAddr)
+	if err != nil {
+		return fmt.Errorf("failed to get proxy address: %v", err)
+	}
+	juju, err := getJujuExecutable()
+	if err != nil {
+		return fmt.Errorf("failed to get juju executable path: %v", err)
+	}
+	options.SetProxyCommand(juju, "ssh", "--proxy=false", apiServerHost, "-T", "nc -q0 %h %p")
+	return nil
+}
+
 const sshDoc = `
 Launch an ssh shell on the machine identified by the <target> parameter.
 <target> can be either a machine id  as listed by "juju status" in the
@@ -75,6 +101,12 @@
 	return nil
 }
 
+// getJujuExecutable returns the path to the juju
+// executable, or an error if it could not be found.
+var getJujuExecutable = func() (string, error) {
+	return exec.LookPath(os.Args[0])
+}
+
 // Run resolves c.Target to a machine, to the address of a i
 // machine or unit forks ssh passing any arguments provided.
 func (c *SSHCommand) Run(ctx *cmd.Context) error {
@@ -92,6 +124,11 @@
 	}
 	var options ssh.Options
 	options.EnablePTY()
+	if c.proxy {
+		if err := c.setProxyCommand(&options); err != nil {
+			return err
+		}
+	}
 	cmd := ssh.Command("ubuntu@"+host, c.Args, &options)
 	cmd.Stdin = ctx.Stdin
 	cmd.Stdout = ctx.Stdout
@@ -102,9 +139,13 @@
 // initAPIClient initialises the API connection.
 // It is the caller's responsibility to close the connection.
 func (c *SSHCommon) initAPIClient() (*api.Client, error) {
-	var err error
-	c.apiClient, err = juju.NewAPIClientFromName(c.EnvName)
-	return c.apiClient, err
+	st, err := juju.NewAPIFromName(c.EnvName)
+	if err != nil {
+		return nil, err
+	}
+	c.apiClient = st.Client()
+	c.apiAddr = st.Addr()
+	return c.apiClient, nil
 }
 
 // attemptStarter is an interface corresponding to utils.AttemptStrategy
@@ -158,9 +199,15 @@
 		if err != nil {
 			return "", err
 		}
-		addr := instance.SelectPublicAddress(machine.Addresses())
-		if addr == "" {
-			return "", fmt.Errorf("machine %q has no public address", machine)
+		var addr string
+		if c.proxy {
+			if addr = instance.SelectInternalAddress(machine.Addresses(), false); addr == "" {
+				return "", fmt.Errorf("machine %q has no internal address", machine)
+			}
+		} else {
+			if addr = instance.SelectPublicAddress(machine.Addresses()); addr == "" {
+				return "", fmt.Errorf("machine %q has no public address", machine)
+			}
 		}
 		return addr, nil
 	}
@@ -171,9 +218,16 @@
 		if err != nil {
 			return "", err
 		}
-		addr, ok := unit.PublicAddress()
-		if !ok {
-			return "", fmt.Errorf("unit %q has no public address", unit)
+		var addr string
+		var ok bool
+		if c.proxy {
+			if addr, ok = unit.PrivateAddress(); !ok {
+				return "", fmt.Errorf("unit %q has no internal address", unit)
+			}
+		} else {
+			if addr, ok = unit.PublicAddress(); !ok {
+				return "", fmt.Errorf("unit %q has no public address", unit)
+			}
 		}
 		return addr, nil
 	}
@@ -181,6 +235,11 @@
 }
 
 func (c *SSHCommon) hostFromTarget(target string) (string, error) {
+	// If the target is neither a machine nor a unit,
+	// assume it's a hostname and try it directly.
+	if !names.IsMachine(target) && !names.IsUnit(target) {
+		return target, nil
+	}
 	var addr string
 	var err error
 	var useStateConn bool
@@ -189,9 +248,13 @@
 	// a loop.
 	for a := sshHostFromTargetAttemptStrategy.Start(); a.Next(); {
 		if !useStateConn {
-			addr, err = c.apiClient.PublicAddress(target)
+			if c.proxy {
+				addr, err = c.apiClient.PrivateAddress(target)
+			} else {
+				addr, err = c.apiClient.PublicAddress(target)
+			}
 			if params.IsCodeNotImplemented(err) {
-				logger.Infof("API server does not support Client.PublicAddress falling back to 1.16 compatibility mode (direct DB access)")
+				logger.Infof("API server does not support Client.PrivateAddress falling back to 1.16 compatibility mode (direct DB access)")
 				useStateConn = true
 			}
 		}
 
=== modified file 'cmd/juju/ssh_test.go'
--- cmd/juju/ssh_test.go	2014-02-22 13:45:10 +0000
+++ cmd/juju/ssh_test.go	2014-03-19 03:21:48 +0000
@@ -39,6 +39,7 @@
 
 func (s *SSHCommonSuite) SetUpTest(c *gc.C) {
 	s.JujuConnSuite.SetUpTest(c)
+	s.PatchValue(&getJujuExecutable, func() (string, error) { return "juju", nil })
 
 	s.bin = c.MkDir()
 	s.PatchEnvPathPrepend(s.bin)
@@ -53,8 +54,10 @@
 }
 
 const (
-	commonArgs = `-o StrictHostKeyChecking no -o PasswordAuthentication no `
-	sshArgs    = commonArgs + `-t -t `
+	commonArgsNoProxy = `-o StrictHostKeyChecking no -o PasswordAuthentication no `
+	commonArgs        = `-o StrictHostKeyChecking no -o ProxyCommand juju ssh --proxy=false 127.0.0.1 -T "nc -q0 %h %p" -o PasswordAuthentication no `
+	sshArgs           = commonArgs + `-t -t `
+	sshArgsNoProxy    = commonArgsNoProxy + `-t -t `
 )
 
 var sshTests = []struct {
@@ -82,6 +85,11 @@
 		[]string{"ssh", "mongodb/1", "ls", "/"},
 		sshArgs + "[e-mail] ls /\n",
 	},
+	{
+		"connect to unit mysql/0 without proxy",
+		[]string{"ssh", "--proxy=false", "mysql/0"},
+		sshArgsNoProxy + "[e-mail]\n",
+	},
 }
 
 func (s *SSHSuite) TestSSHCommand(c *gc.C) {
 
=== modified file 'juju/api.go'
--- juju/api.go	2014-03-12 10:59:17 +0000
+++ juju/api.go	2014-03-19 03:21:48 +0000
@@ -100,17 +100,24 @@
 	return keymanager.NewClient(st), nil
 }
 
+// NewAPIFromName returns an api.State connected to the API Server for
+// the named environment. If envName is "", the default environment will
+// be used.
+func NewAPIFromName(envName string) (*api.State, error) {
+	return newAPIClient(envName)
+}
+
 func newAPIClient(envName string) (*api.State, error) {
 	store, err := configstore.NewDisk(osenv.JujuHome())
 	if err != nil {
 		return nil, err
 	}
-	return newAPIFromName(envName, store)
+	return newAPIFromStore(envName, store)
 }
 
-// newAPIFromName implements the bulk of NewAPIClientFromName
+// newAPIFromStore implements the bulk of NewAPIClientFromName
 // but is separate for testing purposes.
-func newAPIFromName(envName string, store configstore.Storage) (*api.State, error) {
+func newAPIFromStore(envName string, store configstore.Storage) (*api.State, error) {
 	// Try to read the default environment configuration file.
 	// If it doesn't exist, we carry on in case
 	// there's some environment info for that environment.
 
=== modified file 'juju/apiconn_test.go'
--- juju/apiconn_test.go	2014-03-13 07:54:56 +0000
+++ juju/apiconn_test.go	2014-03-19 03:21:48 +0000
@@ -139,11 +139,11 @@
 		called++
 		return expectState, nil
 	}
-	// Give NewAPIFromName a store interface that can report when the
+	// Give NewAPIFromStore a store interface that can report when the
 	// config was written to, to ensure the cache isn't updated.
 	defer testbase.PatchValue(juju.APIOpen, apiOpen).Restore()
 	mockStore := &storageWithWriteNotify{store: store}
-	st, err := juju.NewAPIFromName("noconfig", mockStore)
+	st, err := juju.NewAPIFromStore("noconfig", mockStore)
 	c.Assert(err, gc.IsNil)
 	c.Assert(st, gc.Equals, expectState)
 	c.Assert(called, gc.Equals, 1)
@@ -186,7 +186,7 @@
 		return expectState, nil
 	}
 	defer testbase.PatchValue(juju.APIOpen, apiOpen).Restore()
-	st, err := juju.NewAPIFromName("myenv", store)
+	st, err := juju.NewAPIFromStore("myenv", store)
 	c.Assert(err, gc.IsNil)
 	c.Assert(st, gc.Equals, expectState)
 	c.Assert(called, gc.Equals, 1)
@@ -209,7 +209,7 @@
 	expectErr := fmt.Errorf("an error")
 	store := newConfigStoreWithError(expectErr)
 	defer testbase.PatchValue(juju.APIOpen, panicAPIOpen).Restore()
-	client, err := juju.NewAPIFromName("noconfig", store)
+	client, err := juju.NewAPIFromStore("noconfig", store)
 	c.Assert(err, gc.Equals, expectErr)
 	c.Assert(client, gc.IsNil)
 }
@@ -228,7 +228,7 @@
 	})
 	defer testbase.PatchValue(juju.APIOpen, panicAPIOpen).Restore()
 
-	st, err := juju.NewAPIFromName("noconfig", store)
+	st, err := juju.NewAPIFromStore("noconfig", store)
 	c.Assert(err, gc.ErrorMatches, `environment "noconfig" not found`)
 	c.Assert(st, gc.IsNil)
 }
@@ -246,7 +246,7 @@
 		return nil, expectErr
 	}
 	defer testbase.PatchValue(juju.APIOpen, apiOpen).Restore()
-	st, err := juju.NewAPIFromName("noconfig", store)
+	st, err := juju.NewAPIFromStore("noconfig", store)
 	c.Assert(err, gc.Equals, expectErr)
 	c.Assert(st, gc.IsNil)
 }
@@ -277,7 +277,7 @@
 	defer restoreAPIClose.Restore()
 
 	startTime := time.Now()
-	st, err := juju.NewAPIFromName(coretesting.SampleEnvName, store)
+	st, err := juju.NewAPIFromStore(coretesting.SampleEnvName, store)
 	c.Assert(err, gc.IsNil)
 	// The connection logic should wait for some time before opening
 	// the API from the configuration.
@@ -342,7 +342,7 @@
 
 	done := make(chan struct{})
 	go func() {
-		st, err := juju.NewAPIFromName(coretesting.SampleEnvName, store)
+		st, err := juju.NewAPIFromStore(coretesting.SampleEnvName, store)
 		c.Check(err, gc.IsNil)
 		c.Check(st, gc.Equals, infoOpenedState)
 		close(done)
@@ -360,7 +360,7 @@
 		c.Fatalf("api never opened via config")
 	}
 	// Let the info endpoint open go ahead and
-	// check that the NewAPIFromName call returns.
+	// check that the NewAPIFromStore call returns.
 	infoEndpointOpened <- struct{}{}
 	select {
 	case <-done:
@@ -393,7 +393,7 @@
 		return nil, fmt.Errorf("config connect failed")
 	}
 	defer testbase.PatchValue(juju.APIOpen, apiOpen).Restore()
-	st, err := juju.NewAPIFromName(coretesting.SampleEnvName, store)
+	st, err := juju.NewAPIFromStore(coretesting.SampleEnvName, store)
 	c.Check(err, gc.ErrorMatches, "config connect failed")
 	c.Check(st, gc.IsNil)
 }
@@ -426,7 +426,7 @@
 	err = os.Remove(osenv.JujuHomePath("environments.yaml"))
 	c.Assert(err, gc.IsNil)
 
-	st, err := juju.NewAPIFromName(coretesting.SampleEnvName, store)
+	st, err := juju.NewAPIFromStore(coretesting.SampleEnvName, store)
 	c.Check(err, gc.IsNil)
 	st.Close()
 }
@@ -454,7 +454,7 @@
 	// Now we have info for envName2 which will actually
 	// cause a connection to the originally bootstrapped
 	// state.
-	st, err := juju.NewAPIFromName(envName2, store)
+	st, err := juju.NewAPIFromStore(envName2, store)
 	c.Check(err, gc.IsNil)
 	st.Close()
 
@@ -464,7 +464,7 @@
 	// Disable for now until an upcoming branch fixes it.
 	//	err = info2.Destroy()
 	//	c.Assert(err, gc.IsNil)
-	//	st, err = juju.NewAPIFromName(envName2, store)
+	//	st, err = juju.NewAPIFromStore(envName2, store)
 	//	if err == nil {
 	//		st.Close()
 	//	}
 
=== modified file 'juju/export_test.go'
--- juju/export_test.go	2013-12-04 10:18:57 +0000
+++ juju/export_test.go	2014-03-19 03:21:48 +0000
@@ -4,5 +4,5 @@
 	APIOpen              = &apiOpen
 	APIClose             = &apiClose
 	ProviderConnectDelay = &providerConnectDelay
-	NewAPIFromName       = newAPIFromName
+	NewAPIFromStore      = newAPIFromStore
 )
 
=== modified file 'state/api/apiclient.go'
--- state/api/apiclient.go	2014-02-20 01:09:59 +0000
+++ state/api/apiclient.go	2014-03-19 03:21:48 +0000
@@ -25,6 +25,7 @@
 type State struct {
 	client *rpc.Conn
 	conn   *websocket.Conn
+	addr   string
 
 	// authTag holds the authenticated entity's tag after login.
 	authTag string
@@ -127,6 +128,7 @@
 	st := &State{
 		client:     client,
 		conn:       conn,
+		addr:       cfg.Location.Host,
 		serverRoot: "https://" + cfg.Location.Host,
 		tag:        info.Tag,
 		password:   info.Password,
@@ -186,3 +188,8 @@
 func (s *State) RPCClient() *rpc.Conn {
 	return s.client
 }
+
+// Addr returns the address used to connect to the RPC server.
+func (s *State) Addr() string {
+	return s.addr
+}
 
=== modified file 'state/api/client.go'
--- state/api/client.go	2014-03-13 22:47:05 +0000
+++ state/api/client.go	2014-03-19 03:21:48 +0000
@@ -143,6 +143,15 @@
 	return results.PublicAddress, err
 }
 
+// PrivateAddress returns the private address of the specified
+// machine or unit.
+func (c *Client) PrivateAddress(target string) (string, error) {
+	var results params.PrivateAddressResults
+	p := params.PrivateAddress{Target: target}
+	err := c.st.Call("Client", "", "PrivateAddress", p, &results)
+	return results.PrivateAddress, err
+}
+
 // ServiceSetYAML sets configuration options on a service
 // given options in YAML format.
 func (c *Client) ServiceSetYAML(service string, yaml string) error {
 
=== modified file 'state/api/params/params.go'
--- state/api/params/params.go	2014-03-13 13:42:50 +0000
+++ state/api/params/params.go	2014-03-19 03:21:48 +0000
@@ -236,6 +236,16 @@
 	PublicAddress string
 }
 
+// PrivateAddress holds parameters for the PrivateAddress call.
+type PrivateAddress struct {
+	Target string
+}
+
+// PrivateAddressResults holds results of the PrivateAddress call.
+type PrivateAddressResults struct {
+	PrivateAddress string
+}
+
 // Resolved holds parameters for the Resolved call.
 type Resolved struct {
 	UnitName string
 
=== modified file 'state/apiserver/client/client.go'
--- state/apiserver/client/client.go	2014-03-18 02:36:58 +0000
+++ state/apiserver/client/client.go	2014-03-19 03:21:48 +0000
@@ -184,6 +184,34 @@
 	return results, fmt.Errorf("unknown unit or machine %q", p.Target)
 }
 
+// PrivateAddress implements the server side of Client.PrivateAddress.
+func (c *Client) PrivateAddress(p params.PrivateAddress) (results params.PrivateAddressResults, err error) {
+	switch {
+	case names.IsMachine(p.Target):
+		machine, err := c.api.state.Machine(p.Target)
+		if err != nil {
+			return results, err
+		}
+		addr := instance.SelectInternalAddress(machine.Addresses(), false)
+		if addr == "" {
+			return results, fmt.Errorf("machine %q has no internal address", machine)
+		}
+		return params.PrivateAddressResults{PrivateAddress: addr}, nil
+
+	case names.IsUnit(p.Target):
+		unit, err := c.api.state.Unit(p.Target)
+		if err != nil {
+			return results, err
+		}
+		addr, ok := unit.PrivateAddress()
+		if !ok {
+			return results, fmt.Errorf("unit %q has no internal address", unit)
+		}
+		return params.PrivateAddressResults{PrivateAddress: addr}, nil
+	}
+	return results, fmt.Errorf("unknown unit or machine %q", p.Target)
+}
+
 // ServiceExpose changes the juju-managed firewall to expose any ports that
 // were also explicitly marked by units as open.
 func (c *Client) ServiceExpose(args params.ServiceExpose) error {
 
=== modified file 'state/apiserver/client/client_test.go'
--- state/apiserver/client/client_test.go	2014-03-18 02:36:58 +0000
+++ state/apiserver/client/client_test.go	2014-03-19 03:21:48 +0000
@@ -1413,6 +1413,66 @@
 	c.Assert(addr, gc.Equals, "127.0.0.1")
 }
 
+func (s *clientSuite) TestClientPrivateAddressErrors(c *gc.C) {
+	s.setUpScenario(c)
+	_, err := s.APIState.Client().PrivateAddress("wordpress")
+	c.Assert(err, gc.ErrorMatches, `unknown unit or machine "wordpress"`)
+	_, err = s.APIState.Client().PrivateAddress("0")
+	c.Assert(err, gc.ErrorMatches, `machine "0" has no internal address`)
+	_, err = s.APIState.Client().PrivateAddress("wordpress/0")
+	c.Assert(err, gc.ErrorMatches, `unit "wordpress/0" has no internal address`)
+}
+
+func (s *clientSuite) TestClientPrivateAddressMachine(c *gc.C) {
+	s.setUpScenario(c)
+
+	// Internally, instance.SelectInternalAddress is used; the public
+	// address if no cloud-local one is available.
+	m1, err := s.State.Machine("1")
+	c.Assert(err, gc.IsNil)
+	cloudLocalAddress := instance.NewAddress("cloudlocal")
+	cloudLocalAddress.NetworkScope = instance.NetworkCloudLocal
+	publicAddress := instance.NewAddress("public")
+	publicAddress.NetworkScope = instance.NetworkCloudLocal
+	err = m1.SetAddresses([]instance.Address{publicAddress})
+	c.Assert(err, gc.IsNil)
+	addr, err := s.APIState.Client().PrivateAddress("1")
+	c.Assert(err, gc.IsNil)
+	c.Assert(addr, gc.Equals, "public")
+	err = m1.SetAddresses([]instance.Address{cloudLocalAddress, publicAddress})
+	addr, err = s.APIState.Client().PrivateAddress("1")
+	c.Assert(err, gc.IsNil)
+	c.Assert(addr, gc.Equals, "cloudlocal")
+}
+
+func (s *clientSuite) TestClientPrivateAddressUnitWithMachine(c *gc.C) {
+	s.setUpScenario(c)
+
+	// Private address of unit is taken from its machine
+	// (if its machine has addresses).
+	m1, err := s.State.Machine("1")
+	publicAddress := instance.NewAddress("public")
+	publicAddress.NetworkScope = instance.NetworkCloudLocal
+	err = m1.SetAddresses([]instance.Address{publicAddress})
+	c.Assert(err, gc.IsNil)
+	addr, err := s.APIState.Client().PrivateAddress("wordpress/0")
+	c.Assert(err, gc.IsNil)
+	c.Assert(addr, gc.Equals, "public")
+}
+
+func (s *clientSuite) TestClientPrivateAddressUnitWithoutMachine(c *gc.C) {
+	s.setUpScenario(c)
+	// If the unit's machine has no addresses, the public address
+	// comes from the unit's document.
+	u, err := s.State.Unit("wordpress/1")
+	c.Assert(err, gc.IsNil)
+	err = u.SetPrivateAddress("127.0.0.1")
+	c.Assert(err, gc.IsNil)
+	addr, err := s.APIState.Client().PrivateAddress("wordpress/1")
+	c.Assert(err, gc.IsNil)
+	c.Assert(addr, gc.Equals, "127.0.0.1")
+}
+
 func (s *clientSuite) TestClientEnvironmentGet(c *gc.C) {
 	envConfig, err := s.State.EnvironConfig()
 	c.Assert(err, gc.IsNil)
 
=== modified file 'utils/ssh/ssh.go'
--- utils/ssh/ssh.go	2014-03-10 20:22:44 +0000
+++ utils/ssh/ssh.go	2014-03-19 03:21:48 +0000
@@ -19,6 +19,9 @@
 
 // Options is a client-implementation independent SSH options set.
 type Options struct {
+	// proxyCommand specifies the command to
+	// execute to proxy SSH traffic through.
+	proxyCommand []string
 	// ssh server port; zero means use the default (22)
 	port int
 	// no PTY forced by default
@@ -31,6 +34,11 @@
 	identities []string
 }
 
+// SetProxyCommand sets a command to execute to proxy traffic through.
+func (o *Options) SetProxyCommand(command ...string) {
+	o.proxyCommand = append([]string{}, command...)
+}
+
 // SetPort sets the SSH server port to connect to.
 func (o *Options) SetPort(port int) {
 	o.port = port
 
=== modified file 'utils/ssh/ssh_openssh.go'
--- utils/ssh/ssh_openssh.go	2014-02-22 13:45:10 +0000
+++ utils/ssh/ssh_openssh.go	2014-03-19 03:21:48 +0000
@@ -71,6 +71,9 @@
 	if options == nil {
 		options = &Options{}
 	}
+	if len(options.proxyCommand) > 0 {
+		args["-o"] = append(args["-o"], "ProxyCommand "+utils.CommandString(options.proxyCommand...))
+	}
 	if !options.passwordAuthAllowed {
 		args["-o"] = append(args["-o"], "PasswordAuthentication no")
 	}