From 4f778f696e6fd3ea676a0f50e46b14fb655094a4 Mon Sep 17 00:00:00 2001
From: Trent Mick <trentm@gmail.com>
Date: Fri, 11 Mar 2016 14:58:27 -0800
Subject: [PATCH] joyent/node-triton#52 Fix interactive 'triton ssh <inst>'
 sessions; add workaround for #52

---
 CHANGES.md                | 15 +++++++++
 lib/do_instance/do_ssh.js | 71 +++++++++++++++++++++++++++++----------
 lib/do_ssh.js             |  1 +
 3 files changed, 70 insertions(+), 17 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 5506504..4528e99 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,13 +1,25 @@
 # node-triton changelog
 
+Known issues:
+
+- `triton ssh ...` disables ssh ControlMaster to avoid issue #52.
+
+
 ## 4.7.1 (not yet released)
 
+- #52 Workaround for `triton ssh ...`. In version 4.6.0, `triton ssh ...`
+  interactive sessions were broken. This version reverts that change and adds
+  a workaround for #52 (by disabling ControlMaster when spawning `ssh`).
+  See <https://github.com/joyent/node-triton/issues/52> for details.
 - #97 `triton profile set -` to set the *last* profile as current.
 - PUBAPI-1266 Added `instance enable-firewall` and `instance disable-firewall`
 
 
 ## 4.7.0
 
+**Known issue: `triton ssh` interactive sessions are broken.
+Upgrade to v4.7.1.**
+
 - #101 Bash completion for server-side data: instances, images, etc.
   Bash completion on TAB should now work for things like the following:
   `triton create <TAB to complete images> <TAB to complete packages`,
@@ -20,6 +32,9 @@
 
 ## 4.6.0
 
+**Known issue: `triton ssh` interactive sessions are broken.
+Upgrade to v4.7.1.**
+
 - #98 `triton inst get ID` for a deleted instance will now emit the instance
   object and error less obtusely. This adds a new `InstanceDeleted` error code
   from `TritonApi`.
diff --git a/lib/do_instance/do_ssh.js b/lib/do_instance/do_ssh.js
index 291bc06..d0c4ea8 100644
--- a/lib/do_instance/do_ssh.js
+++ b/lib/do_instance/do_ssh.js
@@ -10,6 +10,7 @@
  * `triton instance ssh ...`
  */
 
+var path = require('path');
 var spawn = require('child_process').spawn;
 
 var common = require('../common');
@@ -26,6 +27,7 @@ function do_ssh(subcmd, opts, args, callback) {
         return;
     }
 
+    var cli = this.top;
     var id = args.shift();
 
     var user = 'root';
@@ -35,7 +37,7 @@ function do_ssh(subcmd, opts, args, callback) {
         id = id.substr(i + 1);
     }
 
-    this.top.tritonapi.getInstance(id, function (err, inst) {
+    cli.tritonapi.getInstance(id, function (err, inst) {
         if (err) {
             callback(err);
             return;
@@ -47,16 +49,33 @@ function do_ssh(subcmd, opts, args, callback) {
             return;
         }
 
-        args = ['-l', user].concat(ip).concat(args);
+        args = ['-l', user, ip].concat(args);
+
+        /*
+         * By default we disable ControlMaster (aka mux, aka SSH connection
+         * multiplexing) because of
+         * https://github.com/joyent/node-triton/issues/52
+         *
+         */
+        if (!opts.no_disable_mux) {
+            /*
+             * A simple `-o ControlMaster=no` doesn't work. With just that
+             * option, a `ControlPath` option (from ~/.ssh/config) will still
+             * be used if it exists. Our hack is to set a ControlPath we
+             * know should not exist. Using '/dev/null' wasn't a good
+             * alternative because `ssh` tries "$ControlPath.$somerandomnum"
+             * and also because Windows.
+             */
+            var nullSshControlPath = path.resolve(
+                cli.tritonapi.config._configDir, 'tmp', 'nullSshControlPath');
+            args = [
+                '-o', 'ControlMaster=no',
+                '-o', 'ControlPath='+nullSshControlPath
+            ].concat(args);
+        }
 
         self.top.log.info({args: args}, 'forking ssh');
-        var child = spawn('ssh', args);
-        child.stdout.on('data', function (chunk) {
-            process.stdout.write(chunk);
-        });
-        child.stderr.on('data', function (chunk) {
-            process.stderr.write(chunk);
-        });
+        var child = spawn('ssh', args, {stdio: 'inherit'});
         child.on('close', function (code) {
             /*
              * Once node 0.10 support is dropped we could instead:
@@ -73,16 +92,34 @@ do_ssh.options = [
         names: ['help', 'h'],
         type: 'bool',
         help: 'Show this help.'
+    },
+    {
+        names: ['no-disable-mux', 'M'],
+        type: 'bool',
+        help: 'Do *not* disable usage of SSH connection multiplexing. Using '
+            + 'this option might result in hitting a known issue where '
+            + 'command output is lost. See below.'
     }
 ];
-do_ssh.help = (
-    'SSH to the primary IP of an instance\n'
-    + '\n'
-    + 'Usage:\n'
-    + '     {{name}} ssh <alias|id> [arguments]\n'
-    + '\n'
-    + '{{options}}'
-);
+do_ssh.help = [
+    /* BEGIN JSSTYLED */
+    'SSH to the primary IP of an instance',
+    '',
+    'Usage:',
+    '     {{name}} ssh [-h] [-M] <inst> [<ssh-arguments>]',
+    '',
+    '{{options}}',
+    'Where <inst> is the name, short ID or ID of a given instance. Note that',
+    'the <inst> argument must come before any `ssh` options or arguments.',
+    '',
+    'There is a known issue with SSH connection multiplexing (a.k.a. ',
+    'ControlMaster, mux) where stdout/stderr is lost. As a workaround, `ssh`',
+    'is spawned with options disabling ControlMaster. You may use the `-M`',
+    'option to not disable ControlMaster but, with node >=0.12, you will lose',
+    'stdout/stderr output. See <https://github.com/joyent/node-triton/issues/52>',
+    'for details.'
+    /* END JSSTYLED */
+].join('\n');
 
 do_ssh.interspersedOptions = false;
 
diff --git a/lib/do_ssh.js b/lib/do_ssh.js
index 14df1ca..8cf725d 100644
--- a/lib/do_ssh.js
+++ b/lib/do_ssh.js
@@ -21,6 +21,7 @@ function do_ssh(subcmd, opts, args, callback) {
 }
 
 do_ssh.help = 'A shortcut for "triton instance ssh".';
+do_ssh.interspersedOptions = targ.interspersedOptions;
 do_ssh.options = targ.options;
 do_ssh.completionArgtypes = targ.completionArgtypes;