Only set ssh connection params if transport is ssh (#1224)

Skip setting various additional connection params if the transport for
the libvirt connection is not ssh based as these will be ignored and may
cause confusion as to why they do not apply.

lib/vagrant-libvirt/config.rb

@@ -711,11 +711,25 @@ module VagrantPlugins
           uri = 'qemu' # use QEMU uri for KVM domain type
         end
 
+        params = {}
+
         if @connect_via_ssh
           uri << '+ssh://'
           uri << @username + '@' if @username
 
           uri << ( @host ? @host : 'localhost' )
+
+          params['no_verify'] = '1'
+
+          if @id_ssh_key_file
+            # set default if using ssh while allowing a user using nil to disable this
+            @id_ssh_key_file = 'id_rsa' if @id_ssh_key_file == UNSET_VALUE
+
+            # set ssh key for access to Libvirt host
+            # if no slash, prepend $HOME/.ssh/
+            @id_ssh_key_file.prepend("#{ENV['HOME']}/.ssh/") if @id_ssh_key_file !~ /\A\//
+            params['keyfile'] = @id_ssh_key_file
+          end
         else
           uri << '://'
           uri << @host if @host
@@ -723,18 +737,10 @@ module VagrantPlugins
 
         uri << virt_path
 
-        params = {'no_verify' => '1'}
-
-        if @id_ssh_key_file
-          # set ssh key for access to Libvirt host
-          # if no slash, prepend $HOME/.ssh/
-          @id_ssh_key_file.prepend("#{ENV['HOME']}/.ssh/") if @id_ssh_key_file !~ /\A\//
-          params['keyfile'] = @id_ssh_key_file
-        end
         # set path to Libvirt socket
         params['socket'] = @socket if @socket
 
-        uri << "?" + params.map{|pair| pair.join('=')}.join('&')
+        uri << "?" + params.map{|pair| pair.join('=')}.join('&') if !params.empty?
         uri
       end
 
@@ -755,7 +761,6 @@ module VagrantPlugins
         @connect_via_ssh = false if @connect_via_ssh == UNSET_VALUE
         @username = nil if @username == UNSET_VALUE
         @password = nil if @password == UNSET_VALUE
-        @id_ssh_key_file = 'id_rsa' if @id_ssh_key_file == UNSET_VALUE
         @socket = nil if @socket == UNSET_VALUE
 
         # If uri isn't set then let's build one from various sources.

spec/unit/config_spec.rb

@@ -70,7 +70,7 @@ describe VagrantPlugins::ProviderLibvirt::Config do
         # settings
         [ # all default
           {},
-          {:uri => "qemu:///system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa"},
+          {:uri => "qemu:///system"},
         ],
 
         # with LIBVIRT_DEFAULT_URI
@@ -125,7 +125,7 @@ describe VagrantPlugins::ProviderLibvirt::Config do
         ],
         [ # when host explicitly set
           {:host => 'remote'},
-          {:uri => 'qemu://remote/system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa'},
+          {:uri => 'qemu://remote/system'},
           {'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
         ],
         [ # when connect_via_ssh explicitly set
@@ -133,48 +133,46 @@ describe VagrantPlugins::ProviderLibvirt::Config do
           {:uri => 'qemu+ssh://localhost/system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa'},
           {'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
         ],
-        [ # when username explicitly set without host
+        [ # when username explicitly set without ssh
           {:username => 'my_user' },
-          {:uri => 'qemu:///system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa'},
+          {:uri => 'qemu:///system'},
           {'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
         ],
-        [ # when username explicitly set with host
+        [ # when username explicitly set with host but without ssh
           {:username => 'my_user', :host => 'remote'},
-          {:uri => 'qemu://remote/system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa'},
+          {:uri => 'qemu://remote/system'},
           {'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
         ],
         [ # when password explicitly set
           {:password => 'some_password'},
-          {:uri => 'qemu:///system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa'},
+          {:uri => 'qemu:///system'},
           {'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
         ],
 
         # driver settings
         [ # set to kvm only
-          {:driver => 'kvm', :id_ssh_key_file => nil},
+          {:driver => 'kvm'},
-          {:uri => "qemu:///system?no_verify=1"},
+          {:uri => "qemu:///system"},
         ],
         [ # set to qemu only
-          {:driver => 'qemu', :id_ssh_key_file => nil},
+          {:driver => 'qemu'},
-          {:uri => "qemu:///system?no_verify=1"},
+          {:uri => "qemu:///system"},
         ],
         [ # set to qemu with session enabled
-          {:driver => 'qemu', :qemu_use_session => true, :id_ssh_key_file => nil},
+          {:driver => 'qemu', :qemu_use_session => true},
-          {:uri => "qemu:///session?no_verify=1"},
+          {:uri => "qemu:///session"},
         ],
         [ # set to openvz only
-          {:driver => 'openvz', :id_ssh_key_file => nil},
+          {:driver => 'openvz'},
-          {:uri => "openvz:///system?no_verify=1"},
+          {:uri => "openvz:///system"},
         ],
         [ # set to esx
           {:driver => 'esx'},
           {:uri => "esx:///"},
-          {},
-          "Should not be adding query options that don't work to esx connection uri",
         ],
         [ # set to vbox only
-          {:driver => 'vbox', :id_ssh_key_file => nil},
+          {:driver => 'vbox'},
-          {:uri => "vbox:///session?no_verify=1"},
+          {:uri => "vbox:///session"},
         ],
 
         # connect_via_ssh settings
@@ -192,13 +190,9 @@ describe VagrantPlugins::ProviderLibvirt::Config do
         ],
 
         # id_ssh_key_file behaviour
-        [ # set
+        [ # set should infer use of ssh
           {:id_ssh_key_file => '/path/to/keyfile'},
-          {:uri => "qemu:///system?no_verify=1&keyfile=/path/to/keyfile"},
+          {:uri => "qemu+ssh://localhost/system?no_verify=1&keyfile=/path/to/keyfile"},
-        ],
-        [ # set infer use of ssh
-          {:id_ssh_key_file => '/path/to/keyfile'},
-          {:uri => "qemu+ssh:///system?no_verify=1&keyfile=/path/to/keyfile"},
           {},
           "setting of ssh key file does not yet enable connect via ssh",
         ],
@@ -206,7 +200,7 @@ describe VagrantPlugins::ProviderLibvirt::Config do
         # socket behaviour
         [ # set
           {:socket => '/var/run/libvirt/libvirt-sock'},
-          {:uri => "qemu:///system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa&socket=/var/run/libvirt/libvirt-sock"},
+          {:uri => "qemu:///system?socket=/var/run/libvirt/libvirt-sock"},
         ],
       ].each do |inputs, outputs, env, allow_failure|
         it "should handle inputs #{inputs} with env #{env}" do