Do not run bootstrap script on the ssh command line

TMake bootstrap use train via chef_core/TargetHost

This commit implements usage of TargetHost instead of
knife_ssh.  TargetHost is a platform-independent representation
of a Train connection. It abstracts common operations
(such as file upload/download, permissions, temp directories,
command executation, etc) and connection error handling.

Moving to TargetHost and train gives us the ability to
execute commands on the bootstrap target; instead of
running sh -c 'long-command-string-containing-secrets', we'll
now upload the bootstrap script to a temporary directory on the
bootstrap target and execute it there.

Incorporating WinRM support directly (allowing core support
for Windows bootstraps, without the knife-windows plugin)
will follow.

Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>

1 files changed, 146 insertions, 62 deletions

diff --git a/lib/chef/knife/bootstrap.rb b/lib/chef/knife/bootstrap.rb
index b9e09a15ba..572cb7c46e 100644
--- a/lib/chef/knife/bootstrap.rb
+++ b/lib/chef/knife/bootstrap.rb
@@ -30,54 +30,70 @@ class Chef
 
       attr_accessor :client_builder
       attr_accessor :chef_vault_handler
+      attr_reader   :target_host
 
       deps do
         require "chef/knife/core/bootstrap_context"
         require "chef/json_compat"
         require "tempfile"
-        require "highline"
-        require "net/ssh"
-        require "net/ssh/multi"
-        require "chef/knife/ssh"
-        Chef::Knife::Ssh.load_deps
+        require "chef_core/text" # i18n and standardized error structures
+        require "chef_core/target_host"
+        # Because nothing else is using i18n out of Chef::Text yet, we're treating it
+        # as a dependency to avoid loading localization files before we need them.
+        ChefCore::Text.add_gem_localization("chef")
       end
 
       banner "knife bootstrap [SSH_USER@]FQDN (options)"
 
+
+
+      # SSH - :host
       option :ssh_user,
         short: "-x USERNAME",
         long: "--ssh-user USERNAME",
         description: "The ssh username",
         default: "root"
 
+      # SSH - :password
       option :ssh_password,
         short: "-P PASSWORD",
         long: "--ssh-password PASSWORD",
         description: "The ssh password"
 
+      # SSH :port
       option :ssh_port,
         short: "-p PORT",
         long: "--ssh-port PORT",
         description: "The ssh port",
         proc: Proc.new { |key| Chef::Config[:knife][:ssh_port] = key }
 
+      # TODO SSH  train gives bastion_host which seeems to map to getway/gateway_identity -
+      # though not exactly.
       option :ssh_gateway,
         short: "-G GATEWAY",
         long: "--ssh-gateway GATEWAY",
         description: "The ssh gateway",
         proc: Proc.new { |key| Chef::Config[:knife][:ssh_gateway] = key }
 
+      # TODO - missing in train: ssh_gateway_identity. But could just append to
+      # keyfiles - train accepts multiple?
+      # TODO - train supports bastion_user and bastion_port
+      # SSH  - this just maps to key_files  - under knife-ssh we would use either this,
+      # _or_ ssh_identity_file
+      #        either this or 'ssh_identity_file' but not both.
       option :ssh_gateway_identity,
         long: "--ssh-gateway-identity SSH_GATEWAY_IDENTITY",
         description: "The SSH identity file used for gateway authentication",
         proc: Proc.new { |key| Chef::Config[:knife][:ssh_gateway_identity] = key }
 
+      # SSH train ssh: options[:forward_agent]
       option :forward_agent,
         short: "-A",
         long: "--forward-agent",
         description: "Enable SSH agent forwarding",
         boolean: true
 
+      # SSH train: options[key_files]
       option :ssh_identity_file,
         short: "-i IDENTITY_FILE",
         long: "--ssh-identity-file IDENTITY_FILE",
@@ -92,49 +108,78 @@ class Chef
         long: "--prerelease",
         description: "Install the pre-release chef gems"
 
+      # client.rb
       option :bootstrap_version,
         long: "--bootstrap-version VERSION",
         description: "The version of Chef to install",
         proc: lambda { |v| Chef::Config[:knife][:bootstrap_version] = v }
 
+      # client.rb
       option :bootstrap_proxy,
         long: "--bootstrap-proxy PROXY_URL",
         description: "The proxy server for the node being bootstrapped",
         proc: Proc.new { |p| Chef::Config[:knife][:bootstrap_proxy] = p }
 
+      # client.rb
       option :bootstrap_proxy_user,
         long: "--bootstrap-proxy-user PROXY_USER",
         description: "The proxy authentication username for the node being bootstrapped"
 
+      # client.rb
       option :bootstrap_proxy_pass,
         long: "--bootstrap-proxy-pass PROXY_PASS",
         description: "The proxy authentication password for the node being bootstrapped"
 
+      # client.rb
       option :bootstrap_no_proxy,
         long: "--bootstrap-no-proxy [NO_PROXY_URL|NO_PROXY_IP]",
         description: "Do not proxy locations for the node being bootstrapped; this option is used internally by Opscode",
         proc: Proc.new { |np| Chef::Config[:knife][:bootstrap_no_proxy] = np }
 
+      # client.rb
       option :bootstrap_template,
         short: "-t TEMPLATE",
         long: "--bootstrap-template TEMPLATE",
         description: "Bootstrap Chef using a built-in or custom template. Set to the full path of an erb template or use one of the built-in templates."
 
+
+      # bootstrap_context - client.rb
+      option :node_ssl_verify_mode,
+        long: "--node-ssl-verify-mode [peer|none]",
+        description: "Whether or not to verify the SSL cert for all HTTPS requests.",
+        proc: Proc.new { |v|
+          valid_values = %w{none peer}
+          unless valid_values.include?(v)
+            raise "Invalid value '#{v}' for --node-ssl-verify-mode. Valid values are: #{valid_values.join(", ")}"
+          end
+          v
+        }
+
+      # bootstrap_context - client.rb
+      option :node_verify_api_cert,
+        long: "--[no-]node-verify-api-cert",
+        description: "Verify the SSL cert for HTTPS requests to the Chef server API.",
+        boolean: true
+
+      # runtime, prefixes to ssh command.  train: [:sudo] - auto prefixes everything
       option :use_sudo,
         long: "--sudo",
         description: "Execute the bootstrap via sudo",
         boolean: true
 
+      # runtime - prefixes to ssh command string
       option :preserve_home,
         long: "--sudo-preserve-home",
         description: "Preserve non-root user HOME environment variable with sudo",
         boolean: true
 
+      # runtime - prefixes to ssh command string
       option :use_sudo_password,
         long: "--use-sudo-password",
         description: "Execute the bootstrap via sudo with password",
         boolean: false
 
+      # runtime - client_builder - set runlist when creating node
       option :run_list,
         short: "-r RUN_LIST",
         long: "--run-list RUN_LIST",
@@ -142,22 +187,26 @@ class Chef
         proc: lambda { |o| o.split(/[\s,]+/) },
         default: []
 
+      # runtime - client_builder - set policy name when creating node
       option :policy_name,
         long: "--policy-name POLICY_NAME",
         description: "Policyfile name to use (--policy-group must also be given)",
         default: nil
 
+      # runtime - client_builder - set policy group when creating node
       option :policy_group,
         long: "--policy-group POLICY_GROUP",
         description: "Policy group name to use (--policy-name must also be given)",
         default: nil
 
+      # runtime - client_builder -  node tags
       option :tags,
         long: "--tags TAGS",
         description: "Comma separated list of tags to apply to the node",
         proc: lambda { |o| o.split(/[\s,]+/) },
         default: []
 
+      # runtime -  bootstrap template
       option :first_boot_attributes,
         short: "-j JSON_ATTRIBS",
         long: "--json-attributes",
@@ -165,18 +214,23 @@ class Chef
         proc: lambda { |o| Chef::JSONCompat.parse(o) },
         default: nil
 
+      # runtime -  bootstrap template
       option :first_boot_attributes_from_file,
         long: "--json-attribute-file FILE",
         description: "A JSON file to be used to the first run of chef-client",
         proc: lambda { |o| Chef::JSONCompat.parse(File.read(o)) },
         default: nil
 
+      # ssh options - train options[:verify_host_key]
       option :host_key_verify,
         long: "--[no-]host-key-verify",
         description: "Verify host key, enabled by default.",
         boolean: true,
         default: true
 
+
+      # bootstrap template
+      # Create ohai hints in /etc/hef/ohai/hints, fname=hintname, content=value
       option :hint,
         long: "--hint HINT_NAME[=HINT_FILE]",
         description: "Specify Ohai Hint to be set on the bootstrap target. Use multiple --hint options to specify multiple hints.",
@@ -186,6 +240,7 @@ class Chef
           Chef::Config[:knife][:hints][name] = path ? Chef::JSONCompat.parse(::File.read(path)) : Hash.new
         }
 
+      # bootstrap overrides  that change bootstrap behavior - runs on target
       option :bootstrap_url,
         long: "--bootstrap-url URL",
         description: "URL to a custom installation script",
@@ -201,32 +256,18 @@ class Chef
              description: "Custom commands to run before installing chef-client",
              proc: Proc.new { |preic| Chef::Config[:knife][:bootstrap_preinstall_command] = preic }
 
+      # runtime on target - can this go away with switch to train + actions - uses mixlib-install.
       option :bootstrap_wget_options,
         long: "--bootstrap-wget-options OPTIONS",
         description: "Add options to wget when installing chef-client",
         proc: Proc.new { |wo| Chef::Config[:knife][:bootstrap_wget_options] = wo }
 
+      # runtime - can this go away with switch to train + actions - uses mixlib-install.
       option :bootstrap_curl_options,
         long: "--bootstrap-curl-options OPTIONS",
         description: "Add options to curl when install chef-client",
         proc: Proc.new { |co| Chef::Config[:knife][:bootstrap_curl_options] = co }
 
-      option :node_ssl_verify_mode,
-        long: "--node-ssl-verify-mode [peer|none]",
-        description: "Whether or not to verify the SSL cert for all HTTPS requests.",
-        proc: Proc.new { |v|
-          valid_values = %w{none peer}
-          unless valid_values.include?(v)
-            raise "Invalid value '#{v}' for --node-ssl-verify-mode. Valid values are: #{valid_values.join(", ")}"
-          end
-          v
-        }
-
-      option :node_verify_api_cert,
-        long: "--[no-]node-verify-api-cert",
-        description: "Verify the SSL cert for HTTPS requests to the Chef server API.",
-        boolean: true
-
       option :bootstrap_vault_file,
         long: "--bootstrap-vault-file VAULT_FILE",
         description: "A JSON file with a list of vault(s) and item(s) to be updated"
@@ -248,6 +289,7 @@ class Chef
 
       def initialize(argv = [])
         super
+        # TODO - these map cleanly to action support classes
         @client_builder = Chef::Knife::Bootstrap::ClientBuilder.new(
           chef_config: Chef::Config,
           knife_config: config,
@@ -384,14 +426,36 @@ class Chef
         ui.info("Connecting to #{ui.color(server_name, :bold)}")
 
         begin
-          knife_ssh.run
-        rescue Net::SSH::AuthenticationFailed
-          if config[:ssh_password]
-            raise
-          else
-            ui.info("Failed to authenticate #{knife_ssh.config[:ssh_user]} - trying password auth")
-            knife_ssh_with_password_auth.run
+          # TODO live stream output may take some doing, and knife ssh does it already
+          @target_host = ChefCore::TargetHost.new(server_name, ssh_opts)
+          target_host.connect!
+          bootstrap_path = render_and_upload_bootstrap
+          r = target_host.run_command(ssh_command(bootstrap_path))
+          if r.exit_status != 0
+            ui.error("The following error occurred on on #{server_name}:")
+            ui.error(r.stderr)
+            exit 1
           end
+
+          # TODO - woudl be nice to pull in chef-cdore error printing, but that'll change expected output
+          # TODO mp 2019-02-22 this *should* be the same behavior under train without
+          # forcing the behavior here, but we need to verify that.
+          #
+          # rescue Net::SSH::AuthenticationFailed
+          #   if config[:ssh_password]
+          #     raise
+          #   else
+          #     ui.info("Failed to authenticate #{knife_ssh.config[:ssh_user]} - trying password auth")
+          #     knife_ssh_with_password_auth.run
+          # def knife_ssh_with_password_auth
+          #   # prompt for a password then return a knife ssh object with that password set
+          #   # and with ssh_identity_file set to nil
+          #   ssh = knife_ssh
+          #   ssh.config[:ssh_identity_file] = nil
+          #   ssh.config[:ssh_password] = ssh.get_password
+          #   ssh
+          # end
+          #   end
         end
       end
 
@@ -427,45 +491,65 @@ class Chef
       # setup a Chef::Knife::Ssh object using the passed config options
       #
       # @return Chef::Knife::Ssh
-      def knife_ssh
-        ssh = Chef::Knife::Ssh.new
-        ssh.ui = ui
-        ssh.name_args = [ server_name, ssh_command ]
-        ssh.config[:ssh_user] = user_name || config[:ssh_user]
-        ssh.config[:ssh_password] = config[:ssh_password]
-        ssh.config[:ssh_port] = config[:ssh_port]
-        ssh.config[:ssh_gateway] = config[:ssh_gateway]
-        ssh.config[:ssh_gateway_identity] = config[:ssh_gateway_identity]
-        ssh.config[:forward_agent] = config[:forward_agent]
-        ssh.config[:ssh_identity_file] = config[:ssh_identity_file]
-        ssh.config[:manual] = true
-        ssh.config[:host_key_verify] = config[:host_key_verify]
-        ssh.config[:on_error] = true
-        ssh
+      def ssh_opts
+        opts = {
+          # TODO based on khife ssh, we will set :keys_only to true if a key is present.
+          host: server_name,
+          port: config[:ssh_port],
+          user: user_name || config[:ssh_user],
+          key_files: config[:ssh_identity_file],
+          logger:  Chef::Log
+        }
+        if config[:ssh_password]
+          opts[:password] = config[:ssh_password]
+        end
+        if config[:use_sudo]
+          opts[:sudo] = true
+          if opts[:use_sudo_password]
+            opts[:sudo_password] = config[:ssh_password]
+          end
+          if opts[:preserve_home]
+             opts[:sudo_options] = "-H"
+          end
+        end
+        opts
+
+        # TODO - looks like we can password, or we can sudo password, but we can't
+        # do both currently in bootstrap.  train permits both if we want to add the option
+        # TODO - we can now allow a custom sudo_command
+          #
+        # ssh.config[:ssh_gateway_identity] = config[:ssh_gateway_identity]
+        # ssh.config[:forward_agent] = config[:forward_agent]
+        # ssh.config[:ssh_identity_file] = config[:ssh_identity_file]
+        # ssh.config[:manual] = true
+          # TODO train appears to false this to always false.  We'll need to make it an option.
+        # ssh.config[:host_key_verify] = config[:host_key_verify]
+        # ssh.config[:on_error] = true
+        # TODO: proxy command
+        # TODO - ssh_identity_file and ssh_gateway_identity appear to be implemented
+        #        as mutually exclsuive in knife ssh. Is there a valid case for two keys?
+        #        If so, train should accept more than one.
+        # key_files << config[:ssh_identity_file]
+        # TODO _ we're forcing knife ssh :on_error to true which will cause immediate exit on problem.
+        #        Need to see what that means, and if we have to implement anything in train to support it.
+        # option :on_error,
+        # short: "-e",
+        # long: "--exit-on-error",
+        # description: "Immediately exit if an error is encountered.",
       end
 
-      # prompt for a password then return a knife ssh object with that password set
-      # and with ssh_identity_file set to nil
-      #
-      # @return Chef::Knife::Ssh
-      def knife_ssh_with_password_auth
-        ssh = knife_ssh
-        ssh.config[:ssh_identity_file] = nil
-        ssh.config[:ssh_password] = ssh.get_password
-        ssh
+      def render_and_upload_bootstrap
+        content = render_template
+        remote_path = target_host.normalize_path(File.join(target_host.temp_dir, "bootstrap.sh"))
+        target_host.save_as_remote_file(content, remote_path)
+        remote_path
       end
 
-      # build the ssh dommand for bootrapping
-      # @return String
-      def ssh_command
-        command = render_template
-
-        if config[:use_sudo]
-          sudo_prefix = config[:use_sudo_password] ? "echo '#{config[:ssh_password]}' | sudo -S " : "sudo "
-          command = config[:preserve_home] ? "#{sudo_prefix} #{command}" : "#{sudo_prefix} -H #{command}"
-        end
 
-        command
+      # build the ssh command for bootrapping
+      # @return String
+      def ssh_command(remote_path)
+        "sh #{remote_path} "
       end
 
       private