Merge pull request #215 from chef/SHACK-272/respsect-ssh-config-values

[SHACK-272] Prefer .ssh/config default values over train's defaults.

components/chef-run/i18n/en.yml

@@ -153,7 +153,7 @@ cli:
     description: Show the current version of Chef Run.
     show: "chef-run: %1"
   status:
-    connecting: "Connecting..."
+    connecting: "Connecting as %1..."
     connected: "Connected."
     connection_failed: "Connection failed: %1"
 

components/chef-run/lib/chef-run/cli.rb

@@ -134,12 +134,13 @@ def perform_run
     # Accepts a target_host and establishes the connection to that host
     # while providing visual feedback via the Terminal API.
     def connect_target(target_host, reporter = nil)
+      connect_message = T.status.connecting(target_host.user)
       if reporter.nil?
-        UI::Terminal.render_job(T.status.connecting, prefix: "[#{target_host.config[:host]}]") do |rep|
+        UI::Terminal.render_job(connect_message, prefix: "[#{target_host.config[:host]}]") do |rep|
           do_connect(target_host, rep, :success)
         end
       else
-        reporter.update(T.status.connecting)
+        reporter.update(connect_message)
         do_connect(target_host, reporter, :update)
       end
       target_host

components/chef-run/lib/chef-run/target_host.rb

@@ -20,7 +20,11 @@
 require "train"
 module ChefRun
   class TargetHost
-    attr_reader :config, :reporter, :backend, :transport_type, :opts
+    attr_reader :config, :reporter, :backend, :transport_type
+    # These values may exist in .ssh/config but will be ignored by train
+    # in favor of its defaults unless we specify them explicitly.
+    # See #apply_ssh_config
+    SSH_CONFIG_OVERRIDE_KEYS = [:user, :port, :proxy]
 
     def self.instance_for_url(target, opts = {})
       opts = { target: @url }
@@ -30,39 +34,61 @@ def self.instance_for_url(target, opts = {})
     end
 
     def initialize(host_url, opts = {}, logger = nil)
-      @opts = opts.dup
-      @opts = { target: host_url,
-                sudo: opts[:sudo] === false ? false : true,
-                www_form_encoded_password: true,
-                key_files: opts[:identity_file],
-                logger: ChefRun::Log }
-
-      if opts.has_key? :ssl
-        @opts[:ssl] = opts[:ssl]
-        @opts[:self_signed] = (opts[:ssl_verify] === false ? true : false)
+      @config = connection_config(host_url, opts, logger)
+      @transport_type = Train.validate_backend(@config)
+      apply_ssh_config(@config, opts) if @transport_type == "ssh"
+      @train_connection = Train.create(@transport_type, config)
+    end
+
+    def connection_config(host_url, opts_in, logger)
+      connection_opts = { target: host_url,
+                          sudo: opts_in[:sudo] === false ? false : true,
+                          www_form_encoded_password: true,
+                          key_files: opts_in[:identity_file],
+                          logger: ChefRun::Log }
+      if opts_in.has_key? :ssl
+        connection_opts[:ssl] = opts_in[:ssl]
+        connection_opts[:self_signed] = (opts_in[:ssl_verify] === false ? true : false)
       end
+
       [:sudo_password, :sudo, :sudo_command, :password, :user].each do |key|
-        @opts[key] = opts[key] if opts.has_key? key
+        connection_opts[key] = opts_in[key] if opts_in.has_key? key
+      end
+      Train.target_config(connection_opts)
+    end
+
+    def apply_ssh_config(config, opts_in)
+      # If we don't provide certain options, they will be defaulted
+      # within train - in the case of ssh, this will prevent the .ssh/config
+      # values from being picked up.
+      # Here we'll modify the returned @config to specify
+      # values that we get out of .ssh/config, when they haven't
+      # been explicitly given.
+      host_cfg = ssh_config_for_host(config[:host])
+      SSH_CONFIG_OVERRIDE_KEYS.each do |key|
+        if host_cfg.has_key?(key) && opts_in[key].nil?
+          config[key] = host_cfg[key]
+        end
       end
-      @config = Train.target_config(@opts)
-      @transport_type = Train.validate_backend(@config)
-      @train_connection = Train.create(@transport_type, config)
     end
 
     def connect!
       return unless @backend.nil?
       @backend = train_connection.connection
       @backend.wait_until_ready
     rescue Train::UserError => e
-
-      # TODO now we have some overlap with the connection error logic in error_printer...
-      raise ConnectionFailure.new(e, opts)
+      raise ConnectionFailure.new(e, config)
     rescue Train::Error => e
       # These are typically wrapper errors for other problems,
       # so we'll prefer to use e.cause over e if available.
       raise ConnectionFailure.new(e.cause || e, opts)
     end
 
+    # Returns the user being used to connect
+    def user
+      config[:user]
+    end
+
     def hostname
       config[:host]
     end
@@ -149,6 +175,11 @@ def train_connection
       @train_connection
     end
 
+    def ssh_config_for_host(host)
+      require "net/ssh"
+      Net::SSH::Config.for(host)
+    end
+
     class RemoteExecutionFailed < ChefRun::ErrorNoLogs
       attr_reader :stdout, :stderr
       def initialize(host, command, result)

components/chef-run/spec/unit/cli_spec.rb

@@ -243,7 +243,7 @@
   end
 
   describe "#connect_target" do
-    let(:host) { double("TargetHost", config: {} ) }
+    let(:host) { double("TargetHost", config: {}, user: "root" ) }
     context "when simulating the multi-host path" do
       let(:reporter) { double("reporter", update: :ok, success: :ok) }
       it "invokes do_connect with correct options" do

components/chef-run/spec/unit/target_host_spec.rb

@@ -176,4 +176,30 @@
       end
     end
   end
+
+  context "#apply_ssh_config" do
+    let(:ssh_host_config) { { user: "testuser", port: 1000, proxy: double("Net:SSH::Proxy::Command") } }
+    let(:connection_config) { { user: "user1", port: 8022, proxy: nil } }
+    before do
+      allow(subject).to receive(:ssh_config_for_host).and_return ssh_host_config
+    end
+
+    ChefRun::TargetHost::SSH_CONFIG_OVERRIDE_KEYS.each do |key|
+      context "when a value is not explicitly provided in options" do
+        it "replaces config config[:#{key}] with the ssh config value" do
+          subject.apply_ssh_config(connection_config, key => nil)
+          expect(connection_config[key]).to eq(ssh_host_config[key])
+        end
+      end
+
+      context "when a value is explicitly provided in options" do
+        it "the connection configuration isnot updated with a value from ssh config" do
+          original_config = connection_config.clone
+          subject.apply_ssh_config(connection_config, { key => "testvalue" } )
+          expect(connection_config[key]).to eq original_config[key]
+        end
+      end
+    end
+  end
+
 end