diff options
author | Marc A. Paradise <marc.paradise@gmail.com> | 2019-04-26 00:30:40 -0400 |
---|---|---|
committer | Marc A. Paradise <marc.paradise@gmail.com> | 2019-04-29 18:10:13 -0400 |
commit | 6976b774e44405a4a566715c9c93e756bdd6c9a0 (patch) | |
tree | 0cefb4d42c6b3df2d56a82353043347ee119d279 | |
parent | a58345b43e8d9d9906625e594c1c4bb78229b4b6 (diff) | |
download | chef-6976b774e44405a4a566715c9c93e756bdd6c9a0.tar.gz |
Switch bootstrap to use TrainConnector.
This replaces TargetHost, and the associated renames/minor updates.
Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>
-rw-r--r-- | lib/chef/knife/bootstrap.rb | 95 | ||||
-rw-r--r-- | spec/unit/knife/bootstrap_spec.rb | 83 |
2 files changed, 81 insertions, 97 deletions
diff --git a/lib/chef/knife/bootstrap.rb b/lib/chef/knife/bootstrap.rb index daca0957d4..b2c4b72f5e 100644 --- a/lib/chef/knife/bootstrap.rb +++ b/lib/chef/knife/bootstrap.rb @@ -387,14 +387,10 @@ class Chef attr_accessor :client_builder attr_accessor :chef_vault_handler - attr_reader :target_host + attr_reader :connection deps do require "chef/json_compat" - require "tempfile" - require "chef_core/text" # i18n and standardized error structures - require "chef_core/target_host" - require "chef_core/target_resolver" end banner "knife bootstrap [PROTOCOL://][USER@]FQDN (options)" @@ -417,7 +413,7 @@ class Chef # # @return [String] Default bootstrap template def default_bootstrap_template - if target_host.base_os == :windows + if connection.windows? "windows-#{Chef::Dist::CLIENT}-msi" else "chef-full" @@ -482,11 +478,11 @@ class Chef end # Establish bootstrap context for template rendering. - # Requires target_host to be a live connection in order to determine + # Requires connection to be a live connection in order to determine # the correct platform. def bootstrap_context @bootstrap_context ||= - if target_host.base_os == :windows + if connection.windows? require "chef/knife/core/windows_bootstrap_context" Knife::Core::WindowsBootstrapContext.new(config, config[:run_list], Chef::Config, secret) else @@ -556,17 +552,12 @@ class Chef bootstrap_path = upload_bootstrap(content) perform_bootstrap(bootstrap_path) ensure - target_host.del_file(bootstrap_path) if target_host && bootstrap_path + connection.del_file!(bootstrap_path) if connection && bootstrap_path end def register_client # chef-vault integration must use the new client-side hawtness, otherwise to use the # new client-side hawtness, just delete your validation key. - # 2019-04-01 TODO - # TODO - should this raise if config says to use vault because json/file/item exists - # but we still have a validation key? That means we can't use the new client hawtness, - # but we also don't tell the operator that their requested vault operations - # won't be performed if chef_vault_handler.doing_chef_vault? || (Chef::Config[:validation_key] && !File.exist?(File.expand_path(Chef::Config[:validation_key]))) @@ -589,8 +580,8 @@ class Chef def perform_bootstrap(remote_bootstrap_script_path) ui.info("Bootstrapping #{ui.color(server_name, :bold)}") cmd = bootstrap_command(remote_bootstrap_script_path) - r = target_host.run_command(cmd) do |data| - ui.msg("#{ui.color(" [#{target_host.hostname}]", :cyan)} #{data}") + r = connection.run_command(cmd) do |data| + ui.msg("#{ui.color(" [#{connection.hostname}]", :cyan)} #{data}") end if r.exit_status != 0 ui.error("The following error occurred on #{server_name}:") @@ -603,24 +594,13 @@ class Chef ui.info("Connecting to #{ui.color(server_name, :bold)}") opts = connection_opts.dup do_connect(opts) - rescue => e - # Ugh. TODO: Train raises a Train::Transports::SSHFailed for a number of different errors. chef_core makes that - # a more general ConnectionFailed, with an error code based on the specific error text/reason provided from trainm. - # This means we have to look three layers into the exception to find out what actually happened instead of just - # looking at the exception type - # - # It doesn't help to provide our own error if it does't let the caller know what they need to identify the problem. - # Let's update chef_core to be a bit smarter about resolving the errors to an appropriate exception type - # (eg ChefCore::ConnectionFailed::AuthError or similar) that will work across protocols, instead of just a single - # ConnectionFailure type - # - - if e.cause && e.cause.cause && e.cause.cause.class == Net::SSH::AuthenticationFailed - if opts[:password] + rescue Train::Error => e + if e.cause && e.cause.class == Net::SSH::AuthenticationFailed + if connection.password_auth? raise else ui.warn("Failed to authenticate #{opts[:user]} to #{server_name} - trying password auth") - password = ui.ask("Enter password for #{opts[:user]}@#{server_name} - trying password auth") do |q| + password = ui.ask("Enter password for #{opts[:user]}@#{server_name}.") do |q| q.echo = false end end @@ -631,6 +611,9 @@ class Chef end end + # TODO - maybe remove the footgun detection this was built on. + # url values override CLI flags, if you provide both + # we'll use the one that you gave in the URL. def connection_protocol return @connection_protocol if @connection_protocol from_url = host_descriptor =~ /^(.*):\/\// ? $1 : nil @@ -640,14 +623,10 @@ class Chef end def do_connect(conn_options) - # Resolve the given host name to a TargetHost instance. We will limit - # the number of hosts to 1 (effectivly eliminating wildcard support) since - # we only support running bootstrap against one host at a time. - resolver = ChefCore::TargetResolver.new(host_descriptor, connection_protocol, - conn_options, max_expanded_targets: 1) - @target_host = resolver.targets.first - target_host.connect! - target_host + require "chef/knife/bootstrap/train_connector" + + @connection = TrainConnector.new(host_descriptor, connection_protocol, conn_options) + connection.connect! end # Fail if both first_boot_attributes and first_boot_attributes_from_file @@ -664,7 +643,7 @@ class Chef # TODO test for this method # TODO check that the protoocol is valid. def validate_winrm_transport_opts! - return true if connection_protocol != "winrm" + return true unless winrm? if Chef::Config[:validation_key] && !File.exist?(File.expand_path(Chef::Config[:validation_key])) if config_value(:winrm_auth_method) == "plaintext" && @@ -739,7 +718,7 @@ class Chef end def winrm_warn_no_ssl_verification - return if connection_protocol != "winrm" + return unless winrm? # REVIEWER NOTE # The original check from knife plugin did not include winrm_ssl_peer_fingerprint @@ -768,13 +747,8 @@ class Chef end end - # - - # Create a configuration hash for TargetHost to connect - # to the remote host via Train. - # # @return a configuration hash suitable for connecting to the remote - # host via TargetHost. + # host via train def connection_opts return @connection_opts unless @connection_opts.nil? @connection_opts = {} @@ -788,9 +762,16 @@ class Chef @connection_opts end + def winrm? + connection_protocol == "winrm" + end + + def ssh? + connection_protocol == "ssh" + end + # Common configuration for all protocols def base_opts - # port = config_value(:connection_port, knife_key_for_protocol(connection_protocol, :port)) user = config_value(:connection_user, @@ -807,10 +788,9 @@ class Chef end def host_verify_opts - case connection_protocol - when "winrm" + if winrm? { self_signed: config_value(:winrm_no_verify_cert) === true } - when "ssh" + elsif ssh? # Fall back to the old knife config key name for back compat. { verify_host_key: config_value(:ssh_verify_host_key, :host_key_verify, true) === true } @@ -959,16 +939,16 @@ class Chef end def upload_bootstrap(content) - script_name = target_host.base_os == :windows ? "bootstrap.bat" : "bootstrap.sh" - remote_path = target_host.normalize_path(File.join(target_host.temp_dir, script_name)) - target_host.save_as_remote_file(content, remote_path) + script_name = connection.windows? ? "bootstrap.bat" : "bootstrap.sh" + remote_path = connection.normalize_path(File.join(connection.temp_dir, script_name)) + connection.upload_file_content!(content, remote_path) remote_path end # build the command string for bootrapping # @return String def bootstrap_command(remote_path) - if target_host.base_os == :windows + if connection.windows? "cmd.exe /C #{remote_path}" else "sh #{remote_path}" @@ -977,8 +957,9 @@ class Chef # To avoid cluttering the CLI options, some flags (such as port and user) # are shared between protocols. However, there is still a need to allow the operator - # to specify defaults separately, since they may not be the same values for different protocols. - # + # to specify defaults separately, since they may not be the same values for different + # protocols. + # These keys are available in Chef::Config, and are prefixed with the protocol name. # For example, :user CLI option will map to :winrm_user and :ssh_user Chef::Config keys, # based on the connection protocol in use. diff --git a/spec/unit/knife/bootstrap_spec.rb b/spec/unit/knife/bootstrap_spec.rb index f54c8ac1d6..ce590fc9ee 100644 --- a/spec/unit/knife/bootstrap_spec.rb +++ b/spec/unit/knife/bootstrap_spec.rb @@ -25,8 +25,17 @@ describe Chef::Knife::Bootstrap do let(:bootstrap_template) { nil } let(:stderr) { StringIO.new } let(:bootstrap_cli_options) { [ ] } - let(:base_os) { :linux } - let(:target_host) { double("TargetHost") } + let(:linux_test) { true } + let(:windows_test) { false } + let(:linux_test) { false } + let(:unix_test) { false } + let(:ssh_test) { false } + + let(:connection) do + double("TrainConnector", + windows?: windows_test, + linux?: linux_test, + unix?: unix_test) end let(:knife) do Chef::Log.logger = Logger.new(StringIO.new) @@ -35,15 +44,11 @@ describe Chef::Knife::Bootstrap do k = Chef::Knife::Bootstrap.new(bootstrap_cli_options) allow(k.ui).to receive(:stderr).and_return(stderr) allow(k).to receive(:encryption_secret_provided_ignore_encrypt_flag?).and_return(false) - allow(k).to receive(:target_host).and_return target_host + allow(k).to receive(:connection).and_return connection k.merge_configs k end - before do - allow(target_host).to receive(:base_os).and_return base_os - end - context "#bootstrap_template" do it "should default to chef-full" do expect(knife.bootstrap_template).to be_a_kind_of(String) @@ -320,7 +325,7 @@ describe Chef::Knife::Bootstrap do subject(:knife) do k = described_class.new Chef::Config[:knife][:bootstrap_template] = template_file - allow(k).to receive(:target_host).and_return target_host + allow(k).to receive(:connection).and_return connection k.parse_options(options) k.merge_configs k @@ -1578,7 +1583,7 @@ describe Chef::Knife::Bootstrap do expect(knife).to receive(:render_template).and_return "content" expect(knife).to receive(:upload_bootstrap).with("content").and_return "/remote/path.sh" expect(knife).to receive(:perform_bootstrap).with("/remote/path.sh") - expect(target_host).to receive(:del_file) # Make sure cleanup happens + expect(connection).to receive(:del_file!) # Make sure cleanup happens knife.run @@ -1687,14 +1692,14 @@ describe Chef::Knife::Bootstrap do let(:result_mock) { double("result", exit_status: exit_status, stderr: "A message") } before do - allow(target_host).to receive(:hostname).and_return "testhost" + allow(connection).to receive(:hostname).and_return "testhost" end it "runs the remote script and logs the output" do expect(knife.ui).to receive(:info).with(/Bootstrapping.*/) expect(knife).to receive(:bootstrap_command) .with("/path.sh") .and_return("sh /path.sh") - expect(target_host) + expect(connection) .to receive(:run_command) .with("sh /path.sh") .and_yield("output here") @@ -1710,7 +1715,7 @@ describe Chef::Knife::Bootstrap do expect(knife).to receive(:bootstrap_command) .with("/path.sh") .and_return("sh /path.sh") - expect(target_host).to receive(:run_command).with("sh /path.sh").and_return result_mock + expect(connection).to receive(:run_command).with("sh /path.sh").and_return result_mock expect { knife.perform_bootstrap("/path.sh") }.to raise_error(SystemExit) end end @@ -1738,13 +1743,11 @@ describe Chef::Knife::Bootstrap do context "when an auth failure occurs" do let(:expected_error) do - # TODO This is awkward and ugly. Requires some refactor of chef_core/error - # to make it not so. See comment in rescue block of connect! for details. - e = RuntimeError.new - interim = RuntimeError.new + e = Train::Error.new actual = Net::SSH::AuthenticationFailed.new - allow(interim).to receive(:cause).and_return(actual) - allow(e).to receive(:cause).and_return(interim) + # Simulate train's nested error - they wrap + # ssh/network errors in a TrainError. + allow(e).to receive(:cause).and_return(actual) e end @@ -1754,7 +1757,7 @@ describe Chef::Knife::Bootstrap do context "and password auth was used" do before do - knife.config[:connection_password] = "tryme" + allow(connection).to receive(:password_auth?).and_return true end it "re-raises the error so as not to resubmit the same failing password" do @@ -1765,8 +1768,8 @@ describe Chef::Knife::Bootstrap do context "and password auth was not used" do before do - knife.config.delete :connection_password - allow(target_host).to receive(:user).and_return "testuser" + allow(connection).to receive(:password_auth?).and_return false + allow(connection).to receive(:user).and_return "testuser" end it "warns, prompts for password, then reconnects with a password-enabled configuration using the new password" do @@ -1793,7 +1796,7 @@ describe Chef::Knife::Bootstrap do describe "#bootstrap_context" do context "under Windows" do - let(:base_os) { :windows } + let(:windows_test) { true } it "creates a WindowsBootstrapContext" do require "chef/knife/core/windows_bootstrap_context" expect(knife.bootstrap_context.class).to eq Chef::Knife::Core::WindowsBootstrapContext @@ -1801,7 +1804,7 @@ describe Chef::Knife::Bootstrap do end context "under linux" do - let(:base_os) { :linux } + let(:linux_test) { true } it "creates a BootstrapContext" do require "chef/knife/core/bootstrap_context" expect(knife.bootstrap_context.class).to eq Chef::Knife::Core::BootstrapContext @@ -1841,25 +1844,25 @@ describe Chef::Knife::Bootstrap do describe "#upload_bootstrap" do before do - allow(target_host).to receive(:temp_dir).and_return(temp_dir) - allow(target_host).to receive(:normalize_path) { |a| a } + allow(connection).to receive(:temp_dir).and_return(temp_dir) + allow(connection).to receive(:normalize_path) { |a| a } end let(:content) { "bootstrap script content" } context "under Windows" do - let(:base_os) { :windows } + let(:windows_test) { true } let(:temp_dir) { "C:/Temp/bootstrap" } - it "creates a bat file in the temp dir provided by target_host, using given content" do - expect(target_host).to receive(:save_as_remote_file).with(content, "C:/Temp/bootstrap/bootstrap.bat") + it "creates a bat file in the temp dir provided by connection, using given content" do + expect(connection).to receive(:upload_file_content!).with(content, "C:/Temp/bootstrap/bootstrap.bat") expect(knife.upload_bootstrap(content)).to eq "C:/Temp/bootstrap/bootstrap.bat" end end context "under Linux" do - let(:base_os) { :linux } + let(:linux_test) { true } let(:temp_dir) { "/tmp/bootstrap" } - it "creates a 'sh file in the temp dir provided by target_host, using given content" do - expect(target_host).to receive(:save_as_remote_file).with(content, "/tmp/bootstrap/bootstrap.sh") + it "creates a 'sh file in the temp dir provided by connection, using given content" do + expect(connection).to receive(:upload_file_content!).with(content, "/tmp/bootstrap/bootstrap.sh") expect(knife.upload_bootstrap(content)).to eq "/tmp/bootstrap/bootstrap.sh" end end @@ -1867,14 +1870,14 @@ describe Chef::Knife::Bootstrap do describe "#bootstrap_command" do context "under Windows" do - let(:base_os) { :windows } + let(:windows_test) { true } it "prefixes the command to run under cmd.exe" do expect(knife.bootstrap_command("autoexec.bat")).to eq "cmd.exe /C autoexec.bat" end end context "under Linux" do - let(:base_os) { :linux } + let(:linux_test) { true } it "prefixes the command to run under sh" do expect(knife.bootstrap_command("bootstrap")).to eq "sh bootstrap" end @@ -1883,14 +1886,14 @@ describe Chef::Knife::Bootstrap do describe "#default_bootstrap_template" do context "under Windows" do - let(:base_os) { :windows } + let(:windows_test) { true } it "is windows-chef-client-msi" do expect(knife.default_bootstrap_template).to eq "windows-chef-client-msi" end end context "under Linux" do - let(:base_os) { :linux } + let(:linux_test) { true } it "is chef-full" do expect(knife.default_bootstrap_template).to eq "chef-full" end @@ -1899,15 +1902,15 @@ describe Chef::Knife::Bootstrap do describe "#do_connect" do let(:host_descriptor) { "example.com" } - let(:target_host) { double("TargetHost") } - let(:resolver_mock) { double("TargetResolver", targets: [ target_host ]) } + let(:connection) { double("TrainConnector") } + let(:connector_mock) { double("TargetResolver", targets: [ connection ]) } before do allow(knife).to receive(:host_descriptor).and_return host_descriptor end - it "resolves the target and connects it" do - expect(ChefCore::TargetResolver).to receive(:new).and_return resolver_mock - expect(target_host).to receive(:connect!) + it "creates a TrainConnector and connects it" do + expect(Chef::Knife::Bootstrap::TrainConnector).to receive(:new).and_return connection + expect(connection).to receive(:connect!) knife.do_connect({}) end end |