Add --session-timeout bootstrap option for both ssh & winrm

 - Add bootstrap option --session-timeout.
 - Add wirnrm-session to deprecations.
 - Default --session-timeout value is 60 secs(same was added for --winrm-session-timeout).
 - Modify existing specs related to old winrm session timeout key and add specs for session timeout.

Signed-off-by: Vivek Singh <vivek.singh@msystechnologies.com>

2 files changed, 35 insertions, 10 deletions

diff --git a/lib/chef/knife/bootstrap.rb b/lib/chef/knife/bootstrap.rb
index 59d7e7007c..ec6f33e82e 100644
--- a/lib/chef/knife/bootstrap.rb
+++ b/lib/chef/knife/bootstrap.rb
@@ -66,6 +66,11 @@ class Chef
         long: "--max-wait SECONDS",
         description: "The maximum time to wait for the initial connection to be established."
 
+      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 }
+
       # WinRM Authentication
       option :winrm_ssl_peer_fingerprint,
         long: "--winrm-ssl-peer-fingerprint FINGERPRINT",
@@ -117,11 +122,6 @@ class Chef
         description: "The Kerberos service used for authentication.",
         proc: Proc.new { |protocol| Chef::Config[:knife][:kerberos_service] = protocol }
 
-      option :winrm_session_timeout,
-        long: "--winrm-session-timeout SECONDS",
-        description: "The number of seconds to wait for each WinRM operation to be acknowledged while running bootstrap.",
-        proc: Proc.new { |protocol| Chef::Config[:knife][:winrm_session_timeout] = protocol }
-
       ## SSH Authentication
       option :ssh_gateway,
         short: "-G GATEWAY",
@@ -381,6 +381,8 @@ class Chef
           [:connection_port, "--winrm-port"],
         winrm_authentication_protocol:
           [:winrm_auth_method, "--winrm-authentication-protocol PROTOCOL"],
+        winrm_session_timeout:
+          [:session_timeout, "--winrm-session-timeout SECONDS"],
       }.freeze
 
       DEPRECATED_FLAGS.each do |deprecated_key, deprecation_entry|
@@ -836,6 +838,7 @@ class Chef
         return opts if connection_protocol == "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 || 60
         opts
       end
 
@@ -934,7 +937,7 @@ class Chef
           opts[:ca_trust_file] = config_value(:ca_trust_file)
         end
 
-        opts[:operation_timeout] = config_value(:winrm_session_timeout) || 60
+        opts[:operation_timeout] = config_value(:session_timeout)&.to_i || 60
 
         opts
       end
diff --git a/spec/unit/knife/bootstrap_spec.rb b/spec/unit/knife/bootstrap_spec.rb
index 5280e3b64b..a3681bf962 100644
--- a/spec/unit/knife/bootstrap_spec.rb
+++ b/spec/unit/knife/bootstrap_spec.rb
@@ -831,7 +831,7 @@ describe Chef::Knife::Bootstrap do
             Chef::Config[:knife][:winrm_auth_method] = "kerberos" # default is negotiate
             Chef::Config[:knife][:winrm_basic_auth_only] = true
             Chef::Config[:knife][:winrm_no_verify_cert] = true
-            Chef::Config[:knife][:winrm_session_timeout] = 9999
+            Chef::Config[:knife][:session_timeout] = 9999
             Chef::Config[:knife][:winrm_ssl] = true
             Chef::Config[:knife][:winrm_ssl_peer_fingerprint] = "ABCDEF"
           end
@@ -926,7 +926,7 @@ describe Chef::Knife::Bootstrap do
               knife.config[:winrm_auth_method] = "kerberos" # default is negotiate
               knife.config[:winrm_basic_auth_only] = false
               knife.config[:winrm_no_verify_cert] = false
-              knife.config[:winrm_session_timeout] = 1000
+              knife.config[:session_timeout] = 1000
               knife.config[:winrm_ssl] = false
               knife.config[:winrm_ssl_peer_fingerprint] = "FEDCBA"
             end
@@ -984,6 +984,7 @@ describe Chef::Knife::Bootstrap do
             # Set everything to easily identifiable and obviously fake values
             # to verify that Chef::Config is being sourced instead of knife.config
             Chef::Config[:knife][:max_wait] = 9999
+            Chef::Config[:knife][:session_timeout] = 9999
             Chef::Config[:knife][:ssh_user] = "sshbob"
             Chef::Config[:knife][:ssh_port] = 9999
             Chef::Config[:knife][:host_key_verify] = false
@@ -1001,6 +1002,7 @@ describe Chef::Knife::Bootstrap do
               {
                 logger: Chef::Log, # not configurable
                 max_wait_until_ready: 9999,
+                connection_timeout: 9999,
                 user: "sshbob",
                 bastion_host: "mygateway.local",
                 bastion_port: 1234,
@@ -1043,6 +1045,7 @@ describe Chef::Knife::Bootstrap do
               knife.config[:ssh_port] = "13" # canary to indirectly verify we're not looking for the wrong CLI flag
               knife.config[:connection_password] = "feta cheese"
               knife.config[:max_wait] = 150
+              knife.config[:session_timeout] = 120
               knife.config[:use_sudo] = true
               knife.config[:use_sudo_pasword] = true
               knife.config[:ssh_forward_agent] = true
@@ -1052,6 +1055,7 @@ describe Chef::Knife::Bootstrap do
               {
                 logger: Chef::Log, # not configurable
                 max_wait_until_ready: 150, # cli
+                connection_timeout: 120, #cli
                 user: "sshalice", # cli
                 password: "feta cheese", # cli
                 bastion_host: "mygateway.local", # Config
@@ -1075,6 +1079,7 @@ describe Chef::Knife::Bootstrap do
           context "and all CLI options have been given" do
             before do
               knife.config[:max_wait] = 150
+              knife.config[:session_timeout] = 120
               knife.config[:connection_user] = "sshroot"
               knife.config[:connection_port] = 1000
               knife.config[:connection_password] = "blah"
@@ -1099,6 +1104,7 @@ describe Chef::Knife::Bootstrap do
               {
                 logger: Chef::Log, # not configurable
                 max_wait_until_ready: 150,
+                connection_timeout: 120,
                 user: "sshroot",
                 password: "blah",
                 port: 1000,
@@ -1124,6 +1130,7 @@ describe Chef::Knife::Bootstrap do
           before do
             knife.config = {}
           end
+
           let(:expected_result) do
             {
               forward_agent: false,
@@ -1133,6 +1140,7 @@ describe Chef::Knife::Bootstrap do
               sudo: false,
               verify_host_key: true,
               non_interactive: true,
+              connection_timeout: 60,
             }
           end
           it "populates appropriate defaults" do
@@ -1485,12 +1493,26 @@ describe Chef::Knife::Bootstrap do
           knife.config[:ssh_forward_agent] = true
         end
         it "returns a configuration hash with forward_agent set to true. non-interactive is always true" do
-          expect(knife.ssh_opts).to eq({ forward_agent: true, non_interactive: true })
+          expect(knife.ssh_opts).to eq({ forward_agent: true, non_interactive: true, connection_timeout: 60 })
         end
       end
       context "when ssh_forward_agent is not set" do
         it "returns a configuration hash with forward_agent set to false. non-interactive is always true" do
-          expect(knife.ssh_opts).to eq({ forward_agent: false, non_interactive: true })
+          expect(knife.ssh_opts).to eq({ forward_agent: false, non_interactive: true, connection_timeout: 60 })
+        end
+      end
+      context "when session_timeout has a value" do
+        before do
+          knife.config[:session_timeout] = 120
+        end
+        it "returns a configuration hash with connection_timeout value." do
+          expect(knife.ssh_opts).to eq({ forward_agent: false, non_interactive: true, connection_timeout: 120 })
+        end
+      end
+
+      context "when session_timeout is not set" do
+        it "returns a configuration hash with connection_timeout default value." do
+          expect(knife.ssh_opts).to eq({ forward_agent: false, non_interactive: true, connection_timeout: 60 })
         end
       end
     end