Improve id_ssh_key_file finalizing (#1230)

Move finalizing the id_ssh_key_file based on how other settings are
currently defined to a separate private function and extend the tests to
accept defining additional expects/allows within the table.

This should apply a consistent set of rules where if the user explicitly
supplies the key, it will attempt to resolve it based on the expected
ssh directory, and will always retain the explicit setting even if it
doesn't exist. Where connect_via_ssh is enabled, it will attempt to
detect if the default key exists, otherwise it will disable the setting.
If the user does not want automatic guesses, they can explicitly disable
by setting it to `nil`.

Fixes: #1228
This commit is contained in:
Darragh Bailey 2021-03-20 15:38:02 +00:00 committed by GitHub
parent 9f8912d7aa
commit 876e906de0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 127 additions and 41 deletions

View File

@ -714,22 +714,15 @@ module VagrantPlugins
params = {}
if @connect_via_ssh
finalize_id_ssh_key_file
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
params['keyfile'] = @id_ssh_key_file if @id_ssh_key_file
else
uri << '://'
uri << @host if @host
@ -787,6 +780,8 @@ module VagrantPlugins
@username = uri.user if @username == nil
end
finalize_id_ssh_key_file
@storage_pool_name = 'default' if @storage_pool_name == UNSET_VALUE
@snapshot_pool_name = @storage_pool_name if @snapshot_pool_name == UNSET_VALUE
@storage_pool_path = nil if @storage_pool_path == UNSET_VALUE
@ -989,6 +984,36 @@ module VagrantPlugins
result.qemu_env = c
end
end
private
def resolve_ssh_key_file(key_file)
# set ssh key for access to Libvirt host
# if no slash, prepend $HOME/.ssh/
key_file.prepend("#{ENV['HOME']}/.ssh/") if key_file && key_file !~ /\A\//
key_file
end
def finalize_id_ssh_key_file
# resolve based on the following roles
# 1) if @connect_via_ssh is set to true, and id_ssh_key_file not current set,
# set default if the file exists
# 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
# 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)
elsif @id_ssh_key_file != UNSET_VALUE
id_ssh_key_file = resolve_ssh_key_file(@id_ssh_key_file)
else
id_ssh_key_file = nil
end
@id_ssh_key_file = id_ssh_key_file
end
end
end
end

View File

@ -1,3 +1,5 @@
require 'contextual_proc'
require 'spec_helper'
require 'support/sharedcontext'
@ -77,76 +79,104 @@ describe VagrantPlugins::ProviderLibvirt::Config do
[ # all other set to default
{},
{:uri => "custom:///custom_path", :qemu_use_session => false},
{'LIBVIRT_DEFAULT_URI' => "custom:///custom_path"},
{
:env => {'LIBVIRT_DEFAULT_URI' => "custom:///custom_path"},
}
],
[ # with session
{},
{:uri => "qemu:///session", :qemu_use_session => true},
{'LIBVIRT_DEFAULT_URI' => "qemu:///session"},
{
:env => {'LIBVIRT_DEFAULT_URI' => "qemu:///session"},
}
],
[ # with session and using ssh
[ # with session and using ssh infer connect by ssh and ignore host
{},
{:uri => "qemu+ssh:///session", :qemu_use_session => true},
{'LIBVIRT_DEFAULT_URI' => "qemu+ssh:///session"},
{
: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'},
{'LIBVIRT_DEFAULT_URI' => "qemu+ssh:///session"},
"not yet inferring connect_via_ssh", # once working remove the preceding test
{
: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},
{'LIBVIRT_DEFAULT_URI' => "qemu+ssh://remote/session?keyfile=my_id_rsa"},
{
: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'},
{'LIBVIRT_DEFAULT_URI' => "qemu+ssh://remote/session?keyfile=my_id_rsa"},
"not yet inferring host correctly", # once working remove the preceding test
{
: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
{},
{:uri => "qemu:///system", :qemu_use_session => false},
{'LIBVIRT_DEFAULT_URI' => "qemu:///system"},
{
:env => {'LIBVIRT_DEFAULT_URI' => "qemu:///system"},
}
],
[ # when session appearing elsewhere
{},
{:uri => "qemu://remote/system?keyfile=my_session_id", :qemu_use_session => false},
{'LIBVIRT_DEFAULT_URI' => "qemu://remote/system?keyfile=my_session_id"},
{
:env => {'LIBVIRT_DEFAULT_URI' => "qemu://remote/system?keyfile=my_session_id"},
}
],
# ignore LIBVIRT_DEFAULT_URI due to explicit settings
[ # when uri explicitly set
{:uri => 'qemu:///system'},
{:uri => 'qemu:///system'},
{'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
{
:env => {'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
}
],
[ # when host explicitly set
{:host => 'remote'},
{:uri => 'qemu://remote/system'},
{'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
{
:env => {'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
}
],
[ # when connect_via_ssh explicitly set
{:connect_via_ssh => true},
{:uri => 'qemu+ssh://localhost/system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa'},
{'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
{:uri => 'qemu+ssh://localhost/system?no_verify=1'},
{
:env => {'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
}
],
[ # when username explicitly set without ssh
{:username => 'my_user' },
{:uri => 'qemu:///system'},
{'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
{
:env => {'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
}
],
[ # when username explicitly set with host but without ssh
{:username => 'my_user', :host => 'remote'},
{:uri => 'qemu://remote/system'},
{'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
{
:env => {'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
}
],
[ # when password explicitly set
{:password => 'some_password'},
{:uri => 'qemu:///system'},
{'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
{:uri => 'qemu:///system', :password => 'some_password'},
{
:env => {'LIBVIRT_DEFAULT_URI' => 'qemu://session'},
}
],
# driver settings
@ -178,23 +208,46 @@ describe VagrantPlugins::ProviderLibvirt::Config do
# connect_via_ssh settings
[ # enabled
{:connect_via_ssh => true},
{:uri => "qemu+ssh://localhost/system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa"},
{:uri => "qemu+ssh://localhost/system?no_verify=1"},
],
[ # enabled with user
{:connect_via_ssh => true, :username => 'my_user'},
{:uri => "qemu+ssh://my_user@localhost/system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa"},
{:uri => "qemu+ssh://my_user@localhost/system?no_verify=1"},
],
[ # enabled with host
{:connect_via_ssh => true, :host => 'remote_server'},
{:uri => "qemu+ssh://remote_server/system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa"},
{:uri => "qemu+ssh://remote_server/system?no_verify=1"},
],
# id_ssh_key_file behaviour
[ # set should take given value
{:connect_via_ssh => true, :id_ssh_key_file => '/path/to/keyfile'},
{:uri => 'qemu+ssh://localhost/system?no_verify=1&keyfile=/path/to/keyfile', :connect_via_ssh => true},
],
[ # 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"},
{},
"setting of ssh key file does not yet enable connect via ssh",
{: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},
{:uri => 'qemu+ssh://localhost/system?no_verify=1', :id_ssh_key_file => nil},
{
:setup => ContextualProc.new {
expect(File).to receive(:file?).with("/home/tests/.ssh/id_rsa").and_return(false)
}
}
],
[ # connect_via_ssh should enable default and include due to existing
{:connect_via_ssh => true},
{:uri => 'qemu+ssh://localhost/system?no_verify=1&keyfile=/home/tests/.ssh/id_rsa', :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)
}
}
],
# socket behaviour
@ -202,19 +255,26 @@ describe VagrantPlugins::ProviderLibvirt::Config do
{: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
].each do |inputs, outputs, options|
opts = {}
opts.merge!(options) if options
it "should handle inputs #{inputs} with env (#{opts[:env]})" do
# allow some of these to fail for now if marked as such
if !allow_failure.nil?
pending(allow_failure)
if !opts[:allow_failure].nil?
pending(opts[:allow_failure])
end
if !opts[:setup].nil?
opts[:setup].apply(binding)
end
inputs.each do |k, v|
subject.instance_variable_set("@#{k}", v)
end
if !env.nil?
env.each do |k, v|
if !opts[:env].nil?
opts[:env].each do |k, v|
fake_env[k] = v
end
end

View File

@ -16,6 +16,7 @@ Gem::Specification.new do |s|
s.require_paths = ['lib']
s.version = VagrantPlugins::ProviderLibvirt.get_version
s.add_development_dependency "contextual_proc"
s.add_development_dependency "rspec-core", "~> 3.5.0"
s.add_development_dependency "rspec-expectations", "~> 3.5.0"
s.add_development_dependency "rspec-mocks", "~> 3.5.0"