summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarc A. Paradise <marc.paradise@gmail.com>2019-05-14 16:20:29 -0400
committerMarc A. Paradise <marc.paradise@gmail.com>2019-05-14 16:35:02 -0400
commit210900d54cb5fea5a77ffb39c2b8ae3f540ca1fd (patch)
treec3e991f75bec0939a346f581be53e4b4b5391c0a
parentc6f3b113057e4848e80cb04c30f3298898faa030 (diff)
downloadchef-210900d54cb5fea5a77ffb39c2b8ae3f540ca1fd.tar.gz
Bootstrap updatesmp/bootstrap-updates
This includes a few fixes: 1. properly handle the default value for session_timeout in cases where merge_config is not called (plugins) to populate defaults. 2. verify that we're using the ssh protoocl before referencing ssh constants. 3. capture and compare the higher-level Train::Error for the check to see if we've gotten a missing fingerprint error, instead of using Train::Transports::SSHFailed which may not be loaded when exception is being evaluated. 4. Add tests for missing fingerprint behavior 5. do not save session_timeout to Chef::Config[:knife], it was not referenced as knife config in any location and we're trying to move away from pushing CLI values into knife config. Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>
-rw-r--r--lib/chef/knife/bootstrap.rb57
-rw-r--r--spec/unit/knife/bootstrap_spec.rb53
2 files changed, 73 insertions, 37 deletions
diff --git a/lib/chef/knife/bootstrap.rb b/lib/chef/knife/bootstrap.rb
index 477074fe49..1e5580a61d 100644
--- a/lib/chef/knife/bootstrap.rb
+++ b/lib/chef/knife/bootstrap.rb
@@ -61,7 +61,6 @@ class Chef
option :session_timeout,
long: "--session-timeout SECONDS",
description: "The number of seconds to wait for each connection operation to be acknowledged while running bootstrap.",
- proc: Proc.new { |protocol| Chef::Config[:knife][:session_timeout] = protocol },
default: 60
# WinRM Authentication
@@ -572,7 +571,7 @@ class Chef
chef_vault_handler.run(client_builder.client)
else
ui.info <<~EOM
- Performing legacy client registration with the validation key at #{Chef::Config[:validation_key]}..."
+ Performing legacy client registration with the validation key at #{Chef::Config[:validation_key]}...
Delete your validation key in order to use your user credentials for client registration instead.
EOM
@@ -596,27 +595,28 @@ class Chef
ui.info("Connecting to #{ui.color(server_name, :bold)}")
opts = connection_opts.dup
do_connect(opts)
- rescue Train::Transports::SSHFailed => e
- if e.message =~ /fingerprint (\S+) is unknown for "(.+)"/
+ rescue Train::Error => e
+ # We handle these by message text only because train only loads the
+ # transports and protocols that it needs - so the exceptions may not be defined,
+ # and we don't want to require files internal to train.
+ if e.message =~ /fingerprint (\S+) is unknown for "(.+)"/ # Train::Transports::SSHFailed
fingerprint = $1
hostname, ip = $2.split(",")
# TODO: convert the SHA256 base64 value to hex with colons
# 'ssh' example output:
# RSA key fingerprint is e5:cb:c0:e2:21:3b:12:52:f8:ce:cb:00:24:e2:0c:92.
# ECDSA key fingerprint is 5d:67:61:08:a9:d7:01:fd:5e:ae:7e:09:40:ef:c0:3c.
- puts "The authenticity of host '#{hostname} (#{ip})' can't be established."
- puts "fingerprint is #{fingerprint}."
- ui.confirm("Are you sure you want to continue connecting") # will exit 3 on N
+ # will exit 3 on N
+ ui.confirm <<~EOM
+ The authenticity of host '#{hostname} (#{ip})' can't be established.
+ fingerprint is #{fingerprint}.
+
+ Are you sure you want to continue connecting
+ EOM
# FIXME: this should save the key to known_hosts but doesn't appear to be
config[:ssh_verify_host_key] = :accept_new
- connection_opts(reset: true)
- retry
- end
-
- raise e
- rescue Train::Error => e
- require "net/ssh"
- if e.cause && e.cause.class == Net::SSH::AuthenticationFailed
+ do_connect(connection_opts(reset: true))
+ elsif ssh? && e.cause && e.cause.class == Net::SSH::AuthenticationFailed
if connection.password_auth?
raise
else
@@ -632,6 +632,9 @@ class Chef
end
end
+ def handle_ssh_error(e)
+ end
+
# url values override CLI flags, if you provide both
# we'll use the one that you gave in the URL.
def connection_protocol
@@ -769,15 +772,11 @@ class Chef
# minutes as its unit, instead of seconds.
# Warn the human so that they are not surprised.
#
- # This will also erroneously warn if a string value is given,
- # but argument type validation is something that needs addressing
- # more broadly.
def warn_on_short_session_timeout
- timeout = config_value(:session_timeout).to_i
- if timeout <= 15
+ if session_timeout && session_timeout <= 15
ui.warn <<~EOM
- --session-timeout is set to #{config[:session_timeout]} minutes.
- Did you mean "--session-timeout #{config[:session_timeout] * 60}" seconds?
+ You provided '--session-timeout #{session_timeout}' second(s).
+ Did you mean '--session-timeout #{session_timeout * 60}' seconds?
EOM
end
end
@@ -868,7 +867,7 @@ class Chef
return opts if winrm?
opts[:non_interactive] = true # Prevent password prompts from underlying net/ssh
opts[:forward_agent] = (config_value(:ssh_forward_agent) === true)
- opts[:connection_timeout] = config_value(:session_timeout).to_i
+ opts[:connection_timeout] = session_timeout
opts
end
@@ -964,10 +963,10 @@ class Chef
end
if config_value(:ca_trust_file)
- opts[:ca_trust_file] = config_value(:ca_trust_file)
+ opts[:ca_trust_path] = config_value(:ca_trust_file)
end
- opts[:operation_timeout] = config_value(:session_timeout).to_i
+ opts[:operation_timeout] = session_timeout
opts
end
@@ -1052,6 +1051,14 @@ class Chef
def incomplete_policyfile_options?
(!!config[:policy_name] ^ config[:policy_group])
end
+
+ # session_timeout option has a default that may not arrive, particularly if
+ # we're being invoked from a plugin that doesn't merge_config.
+ def session_timeout
+ timeout = config_value(:session_timeout)
+ return options[:session_timeout][:default] if timeout.nil?
+ timeout.to_i
+ end
end
end
end
diff --git a/spec/unit/knife/bootstrap_spec.rb b/spec/unit/knife/bootstrap_spec.rb
index 4bfd1011fa..5118442327 100644
--- a/spec/unit/knife/bootstrap_spec.rb
+++ b/spec/unit/knife/bootstrap_spec.rb
@@ -19,7 +19,6 @@
require "spec_helper"
Chef::Knife::Bootstrap.load_deps
-require "net/ssh"
describe Chef::Knife::Bootstrap do
let(:bootstrap_template) { nil }
@@ -853,7 +852,7 @@ describe Chef::Knife::Bootstrap do
let(:expected_result) do
{
logger: Chef::Log, # not configurable
- ca_trust_file: "trust.me",
+ ca_trust_path: "trust.me",
max_wait_until_ready: 9999,
operation_timeout: 9999,
ssl_peer_fingerprint: "ABCDEF",
@@ -878,7 +877,7 @@ describe Chef::Knife::Bootstrap do
let(:expected_result) do
{
logger: Chef::Log, # not configurable
- ca_trust_file: "no trust",
+ ca_trust_path: "no trust",
max_wait_until_ready: 9999,
operation_timeout: 60,
ssl_peer_fingerprint: "ABCDEF",
@@ -933,7 +932,7 @@ describe Chef::Knife::Bootstrap do
let(:expected_result) do
{
logger: Chef::Log, # not configurable
- ca_trust_file: "trust.the.internet",
+ ca_trust_path: "trust.the.internet",
max_wait_until_ready: 1000,
operation_timeout: 1000,
ssl_peer_fingerprint: "FEDCBA",
@@ -1594,7 +1593,7 @@ describe Chef::Knife::Bootstrap do
context "with ca_trust_file" do
let(:ca_trust_expected) do
- expected.merge({ ca_trust_file: "/trust.me" })
+ expected.merge({ ca_trust_path: "/trust.me" })
end
before do
knife.config[:ca_trust_file] = "/trust.me"
@@ -1806,6 +1805,14 @@ describe Chef::Knife::Bootstrap do
end
describe "#connect!" do
+ before do
+ # These are not required at run-time because train will handle its own
+ # protocol loading. In this case, we're simulating train failures and have to load
+ # them ourselves.
+ require "net/ssh"
+ require "train/transports/ssh"
+ end
+
context "in the normal case" do
it "connects using the connection_opts and notifies the operator of progress" do
expect(knife.ui).to receive(:info).with(/Connecting to.*/)
@@ -1815,7 +1822,7 @@ describe Chef::Knife::Bootstrap do
end
end
- context "when a non-auth-failure occurs" do
+ context "when a general non-auth-failure occurs" do
let(:expected_error) { RuntimeError.new }
before do
allow(knife).to receive(:do_connect).and_raise(expected_error)
@@ -1825,6 +1832,23 @@ describe Chef::Knife::Bootstrap do
end
end
+ context "when ssh fingerprint is invalid" do
+ let(:expected_error) { Train::Error.new("fingerprint AA:BB is unknown for \"blah,127.0.0.1\"") }
+ before do
+ allow(knife).to receive(:do_connect).and_raise(expected_error)
+ end
+ it "warns, prompts to accept, then connects with verify_host_key of accept_new" do
+ expect(knife).to receive(:do_connect).and_raise(expected_error)
+ expect(knife.ui).to receive(:confirm)
+ .with(/.*host 'blah \(127.0.0.1\)'.*AA:BB.*Are you sure you want to continue.*/m)
+ .and_return(true)
+ expect(knife).to receive(:do_connect) do |opts|
+ expect(opts[:verify_host_key]).to eq :accept_new
+ end
+ knife.connect!
+ end
+ end
+
context "when an auth failure occurs" do
let(:expected_error) do
e = Train::Error.new
@@ -1835,10 +1859,6 @@ describe Chef::Knife::Bootstrap do
e
end
- before do
- require "net/ssh"
- end
-
context "and password auth was used" do
before do
allow(connection).to receive(:password_auth?).and_return true
@@ -2136,9 +2156,18 @@ describe Chef::Knife::Bootstrap do
end
describe "#warn_on_short_session_timeout" do
- let(:session_timeout) { 0 }
+ let(:session_timeout) { 60 }
+
before do
- allow(knife).to receive(:config).and_return(session_timeout: session_timeout)
+ allow(knife).to receive(:session_timeout).and_return(session_timeout)
+ end
+
+ context "timeout is not set at all" do
+ let(:session_timeout) { nil }
+ it "does not issue a warning" do
+ expect(knife.ui).to_not receive(:warn)
+ knife.warn_on_short_session_timeout
+ end
end
context "timeout is more than 15" do