Enable ssh connection on keyfile being set (#1225)

Auto enable ssh connections if the ssh keyfile has been explicitly set
and switch a number of settings from being explicitly set until after
attempts to resolve the values have failed in order to allow decisions
to be main on whether to set them based on inferred values only if not
explicitly provided elsewhere.

Add some additional tests and transport modes to expand the coverage of
what is accepted to align as close to what libvirt will accept as
possible.

lib/vagrant-libvirt/config.rb

@@ -711,21 +711,26 @@ module VagrantPlugins
           uri = 'qemu' # use QEMU uri for KVM domain type
         end
 
+        # turn on ssh if an ssh key file is explicitly provided
+        if @connect_via_ssh == UNSET_VALUE && @id_ssh_key_file && @id_ssh_key_file != UNSET_VALUE
+          @connect_via_ssh = true
+        end
+
         params = {}
 
-        if @connect_via_ssh
+        if @connect_via_ssh == true
           finalize_id_ssh_key_file
 
           uri << '+ssh://'
-          uri << @username + '@' if @username
+          uri << @username + '@' if @username && @username != UNSET_VALUE
 
-          uri << ( @host ? @host : 'localhost' )
+          uri << ( @host && @host != UNSET_VALUE ? @host : 'localhost' )
 
           params['no_verify'] = '1'
           params['keyfile'] = @id_ssh_key_file if @id_ssh_key_file
         else
           uri << '://'
-          uri << @host if @host
+          uri << @host if @host && @host != UNSET_VALUE
         end
 
         uri << virt_path
@@ -750,9 +755,6 @@ module VagrantPlugins
 
         # settings which _generate_uri
         @driver = 'kvm' if @driver == UNSET_VALUE
-        @host = nil if @host == UNSET_VALUE
-        @connect_via_ssh = false if @connect_via_ssh == UNSET_VALUE
-        @username = nil if @username == UNSET_VALUE
         @password = nil if @password == UNSET_VALUE
         @socket = nil if @socket == UNSET_VALUE
 
@@ -765,6 +767,10 @@ module VagrantPlugins
         # Parse uri to extract individual components
         uri = _parse_uri(@uri)
 
+        # only set @connect_via_ssh if not explicitly to avoid overriding
+        # and allow an error to occur if the @uri and @connect_via_ssh disagree
+        @connect_via_ssh = uri.scheme.include? "ssh" if @connect_via_ssh == UNSET_VALUE
+
         # Set qemu_use_session based on the URI if it wasn't set by the user
         if @qemu_use_session == UNSET_VALUE
           if (uri.scheme.start_with? "qemu") && (uri.path.include? "session")
@@ -774,11 +780,9 @@ module VagrantPlugins
           end
         end
 
-        # Extract host and username values from uri if not set when connect_via_ssh option is used
-        if @connect_via_ssh
-          @host = uri.host if @host == nil
-          @username = uri.user if @username == nil
-        end
+        # Extract host and username values from uri if provided, otherwise nil
+        @host = uri.host
+        @username = uri.user
 
         finalize_id_ssh_key_file
 
@@ -1002,7 +1006,7 @@ module VagrantPlugins
         #  2) if supplied the key name, attempt to expand based on user home
         #  3) otherwise set to nil
 
-        if @connect_via_ssh && @id_ssh_key_file == UNSET_VALUE
+        if @connect_via_ssh == true && @id_ssh_key_file == UNSET_VALUE
           # set default if using ssh while allowing a user using nil to disable this
           id_ssh_key_file = resolve_ssh_key_file('id_rsa')
           id_ssh_key_file = nil if !File.file?(id_ssh_key_file)

spec/unit/config_spec.rb

@@ -75,6 +75,53 @@ describe VagrantPlugins::ProviderLibvirt::Config do
           {:uri => "qemu:///system"},
         ],
 
+        # explicit uri settings
+        [ # transport and hostname
+          {:uri => "qemu+ssh://localhost/system"},
+          {:uri => "qemu+ssh://localhost/system", :connect_via_ssh => true, :host => "localhost", :username => nil},
+        ],
+        [ # tcp transport with port
+          {:uri => "qemu+tcp://localhost:5000/system"},
+          {:uri => "qemu+tcp://localhost:5000/system", :connect_via_ssh => false, :host => "localhost", :username => nil},
+        ],
+        [ # connect explicit to unix socket
+          {:uri => "qemu+unix:///system"},
+          {:uri => "qemu+unix:///system", :connect_via_ssh => false, :host => nil, :username => nil},
+        ],
+        [ # via libssh2 should enable ssh as well
+          {:uri => "qemu+libssh2://user@remote/system?known_hosts=/home/user/.ssh/known_hosts"},
+          {
+            :uri => "qemu+libssh2://user@remote/system?known_hosts=/home/user/.ssh/known_hosts",
+            :connect_via_ssh => true, :host => "remote", :username => "user",
+          },
+        ],
+        [ # xen
+          {:uri => "xen://remote/system?no_verify=1"},
+          {
+            :uri => "xen://remote/system?no_verify=1",
+            :connect_via_ssh => false, :host => "remote", :username => nil,
+            :id_ssh_key_file => nil,
+          },
+          {
+            :setup => ContextualProc.new {
+              expect(File).to_not receive(:file?)
+            }
+          }
+        ],
+        [ # xen
+          {:uri => "xen+ssh://remote/system?no_verify=1"},
+          {
+            :uri => "xen+ssh://remote/system?no_verify=1",
+            :connect_via_ssh => true, :host => "remote", :username => nil,
+            :id_ssh_key_file => "/home/tests/.ssh/id_rsa",
+          },
+          {
+            :setup => ContextualProc.new {
+              expect(File).to receive(:file?).with("/home/tests/.ssh/id_rsa").and_return(true)
+            }
+          }
+        ],
+
         # with LIBVIRT_DEFAULT_URI
         [ # all other set to default
           {},
@@ -90,34 +137,18 @@ describe VagrantPlugins::ProviderLibvirt::Config do
             :env => {'LIBVIRT_DEFAULT_URI' => "qemu:///session"},
           }
         ],
-        [ # with session and using ssh infer connect by ssh and ignore host
+        [ # with session and using ssh infer connect by ssh and ignore host as not provided
           {},
-          {:uri => "qemu+ssh:///session", :qemu_use_session => true},
+          {:uri => "qemu+ssh:///session", :qemu_use_session => true, :connect_via_ssh => true, :host => nil},
           {
             :env => {'LIBVIRT_DEFAULT_URI' => "qemu+ssh:///session"},
           }
         ],
-        [ # with session and using ssh infer host and connect by ssh
-          {},
-          {:uri => "qemu+ssh:///session", :qemu_use_session => true, :connect_via_ssh => true, :host => 'localhost'},
-          {
-            :env => {'LIBVIRT_DEFAULT_URI' => "qemu+ssh:///session"},
-            :allow_failure => "not yet inferring connect_via_ssh", # once working remove the preceding test
-          }
-        ],
-        [ # with session and using ssh to specific host with additional query options provided
-          {},
-          {:uri => "qemu+ssh://remote/session?keyfile=my_id_rsa", :qemu_use_session => true},
-          {
-            :env => {'LIBVIRT_DEFAULT_URI' => "qemu+ssh://remote/session?keyfile=my_id_rsa"},
-          }
-        ],
         [ # with session and using ssh to specific host with additional query options provided, infer host and ssh
           {},
           {:uri => "qemu+ssh://remote/session?keyfile=my_id_rsa", :qemu_use_session => true, :connect_via_ssh => true, :host => 'remote'},
           {
             :env => {'LIBVIRT_DEFAULT_URI' => "qemu+ssh://remote/session?keyfile=my_id_rsa"},
-            :allow_failure => "not yet inferring host correctly", # once working remove the preceding test
           }
         ],
         [ # when session not set
@@ -227,9 +258,6 @@ describe VagrantPlugins::ProviderLibvirt::Config do
         [ # set should infer use of ssh
           {:id_ssh_key_file => '/path/to/keyfile'},
           {:uri => 'qemu+ssh://localhost/system?no_verify=1&keyfile=/path/to/keyfile', :connect_via_ssh => true},
-          {
-            :allow_failure => 'setting id_ssh_key_file explicitly does not yet infer ssh connection', # once fixed replace above
-          }
         ],
         [ # connect_via_ssh should enable default but ignore due to not existing
           {:connect_via_ssh => true},