From bbbcdaa44b848495ff473cce80b157d016613a83 Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Wed, 17 Mar 2021 15:36:09 +0000 Subject: [PATCH] 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 | 25 +++++++++++-------- spec/unit/config_spec.rb | 46 +++++++++++++++-------------------- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/lib/vagrant-libvirt/config.rb b/lib/vagrant-libvirt/config.rb index 230f64a..479e8dd 100644 --- a/lib/vagrant-libvirt/config.rb +++ b/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. diff --git a/spec/unit/config_spec.rb b/spec/unit/config_spec.rb index 18d362f..3addf7a 100644 --- a/spec/unit/config_spec.rb +++ b/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}, - {:uri => "qemu:///system?no_verify=1"}, + {:driver => 'kvm'}, + {:uri => "qemu:///system"}, ], [ # set to qemu only - {:driver => 'qemu', :id_ssh_key_file => nil}, - {:uri => "qemu:///system?no_verify=1"}, + {:driver => 'qemu'}, + {:uri => "qemu:///system"}, ], [ # set to qemu with session enabled - {:driver => 'qemu', :qemu_use_session => true, :id_ssh_key_file => nil}, - {:uri => "qemu:///session?no_verify=1"}, + {:driver => 'qemu', :qemu_use_session => true}, + {:uri => "qemu:///session"}, ], [ # set to openvz only - {:driver => 'openvz', :id_ssh_key_file => nil}, - {:uri => "openvz:///system?no_verify=1"}, + {:driver => 'openvz'}, + {: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}, - {:uri => "vbox:///session?no_verify=1"}, + {:driver => 'vbox'}, + {: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"}, - ], - [ # set infer use of ssh - {:id_ssh_key_file => '/path/to/keyfile'}, - {:uri => "qemu+ssh:///system?no_verify=1&keyfile=/path/to/keyfile"}, + {:uri => "qemu+ssh://localhost/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