diff options
author | Marc A. Paradise <marc.paradise@gmail.com> | 2019-05-13 17:20:52 -0400 |
---|---|---|
committer | Marc A. Paradise <marc.paradise@gmail.com> | 2019-05-13 17:31:24 -0400 |
commit | 218bd7f031e8820d2e0a7a7bc6fcc3c16df99c60 (patch) | |
tree | c9e3dbeaef287d05421d8da9d5c81578c412e3df | |
parent | 4a28a2ffe6d830634017ccb5cb19749faa304dca (diff) | |
download | chef-218bd7f031e8820d2e0a7a7bc6fcc3c16df99c60.tar.gz |
Add a warning when session-timeout is too short
We converted --winrm-session-timeout MINUTES
to --session-timeout SECONDS
If someone is providing a short session timeout, it's possible
that they're doing so based on the flag accepting minutes. We'll warn
them and suggest an alternative so that if things go wrong,
they'll know where to start looking.
We can remove this once we remove the deprecation for
winrm-session-timeout
Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>
-rw-r--r-- | lib/chef/knife/bootstrap.rb | 19 | ||||
-rw-r--r-- | spec/unit/knife/bootstrap_spec.rb | 27 |
2 files changed, 44 insertions, 2 deletions
diff --git a/lib/chef/knife/bootstrap.rb b/lib/chef/knife/bootstrap.rb index 3f8f3642b5..b0f793e5c6 100644 --- a/lib/chef/knife/bootstrap.rb +++ b/lib/chef/knife/bootstrap.rb @@ -549,6 +549,7 @@ class Chef validate_policy_options! winrm_warn_no_ssl_verification + warn_on_short_session_timeout $stdout.sync = true register_client @@ -752,6 +753,24 @@ class Chef true end + # If session_timeout is too short, it is likely + # a holdover from "--winrm-session-timeout" which used + # 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 + ui.warn <<~EOM + --session-timeout is set to #{config[:session_timeout]} minutes. + Did you mean "--session-timeout #{config[:session_timeout] * 60}" seconds? + EOM + end + end + def winrm_warn_no_ssl_verification return unless winrm? diff --git a/spec/unit/knife/bootstrap_spec.rb b/spec/unit/knife/bootstrap_spec.rb index 99fa861a07..5b5d6bd1a9 100644 --- a/spec/unit/knife/bootstrap_spec.rb +++ b/spec/unit/knife/bootstrap_spec.rb @@ -958,7 +958,7 @@ describe Chef::Knife::Bootstrap do before do # We will use knife's actual config since these tests # have assumptions based on CLI default values - knife.merge_configs + knife.merge_configs end let(:expected_result) do { @@ -1132,7 +1132,7 @@ describe Chef::Knife::Bootstrap do before do # We will use knife's actual config since these tests # have assumptions based on CLI default values - knife.merge_configs + knife.merge_configs end let(:expected_result) do @@ -1659,6 +1659,7 @@ describe Chef::Knife::Bootstrap do expect(knife).to receive(:validate_winrm_transport_opts!).ordered expect(knife).to receive(:validate_policy_options!).ordered expect(knife).to receive(:winrm_warn_no_ssl_verification).ordered + expect(knife).to receive(:warn_on_short_session_timeout).ordered expect(knife).to receive(:register_client).ordered expect(knife).to receive(:connect!).ordered expect(knife).to receive(:render_template).and_return "content" @@ -2133,4 +2134,26 @@ describe Chef::Knife::Bootstrap do end end end + + describe "#warn_on_short_session_timeout" do + let(:session_timeout) { 0 } + before do + allow(knife).to receive(:config).and_return(session_timeout: session_timeout) + end + + context "timeout is more than 15" do + let(:session_timeout) { 16 } + 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 15 or less" do + let(:session_timeout) { 15 } + it "issues a warning" do + expect(knife.ui).to receive(:warn) + knife.warn_on_short_session_timeout + end + end + end end |