From b36db04b3d9b2d46a0147e4883300e0ae6584599 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 18 May 2020 17:21:19 -0700 Subject: [PATCH] Validate and get default disk extensions for provider This commit updates the hyperv and virtualbox provider caps to validate and also return the default disk extension. --- plugins/kernel_v2/config/disk.rb | 15 +++++---------- plugins/providers/hyperv/cap/validate_disk_ext.rb | 13 ++++++++++--- plugins/providers/hyperv/plugin.rb | 7 ++++++- .../providers/virtualbox/cap/validate_disk_ext.rb | 13 ++++++++++--- plugins/providers/virtualbox/plugin.rb | 7 ++++++- test/unit/plugins/kernel_v2/config/disk_test.rb | 15 ++++++++++++--- 6 files changed, 49 insertions(+), 21 deletions(-) diff --git a/plugins/kernel_v2/config/disk.rb b/plugins/kernel_v2/config/disk.rb index 6d95c750a..b38858475 100644 --- a/plugins/kernel_v2/config/disk.rb +++ b/plugins/kernel_v2/config/disk.rb @@ -132,7 +132,6 @@ module VagrantPlugins # @return [Array] array of strings of error messages from config option validation def validate(machine) errors = _detected_errors - # validate type with list of known disk types if !DEFAULT_DISK_TYPES.include?(@type) @@ -141,14 +140,10 @@ module VagrantPlugins end if @disk_ext == UNSET_VALUE - # Work around to finalize disk_ext with a valid default per-provider - if machine.provider_name == :virtualbox - @disk_ext = "vdi" - elsif machine.provider_name == :vmware_desktop - @disk_ext = "vmdk" - elsif machine.provider_name == :hyperv - @disk_ext = "vhdx" + if machine.provider.capability?(:set_default_disk_ext) + @disk_ext = machine.provider.capability(:set_default_disk_ext) else + @logger.warn("No provider capability defined to set default 'disk_ext' type. Will use 'vdi' for disk extension.") @disk_ext = "vdi" end elsif @disk_ext @@ -156,8 +151,8 @@ module VagrantPlugins if machine.provider.capability?(:validate_disk_ext) if !machine.provider.capability(:validate_disk_ext, @disk_ext) - if machine.provider.capability?(:get_default_disk_ext) - disk_exts = machine.provider.capability(:get_default_disk_ext).join(', ') + if machine.provider.capability?(:default_disk_exts) + disk_exts = machine.provider.capability(:default_disk_exts).join(', ') else disk_exts = "not found" end diff --git a/plugins/providers/hyperv/cap/validate_disk_ext.rb b/plugins/providers/hyperv/cap/validate_disk_ext.rb index 92e467f51..74512094a 100644 --- a/plugins/providers/hyperv/cap/validate_disk_ext.rb +++ b/plugins/providers/hyperv/cap/validate_disk_ext.rb @@ -7,18 +7,25 @@ module VagrantPlugins LOGGER = Log4r::Logger.new("vagrant::plugins::hyperv::validate_disk_ext") # The default set of disk formats that Hyper-V supports - DEFAULT_DISK_EXT = ["vhd", "vhdx"].map(&:freeze).freeze + DEFAULT_DISK_EXT_LIST = ["vhd", "vhdx"].map(&:freeze).freeze + DEFAULT_DISK_EXT = "vhdx".freeze # @param [Vagrant::Machine] machine # @param [String] disk_ext # @return [Bool] def self.validate_disk_ext(machine, disk_ext) - DEFAULT_DISK_EXT.include?(disk_ext) + DEFAULT_DISK_EXT_LIST.include?(disk_ext) end # @param [Vagrant::Machine] machine # @return [Array] - def self.get_default_disk_ext(machine) + def self.default_disk_exts(machine) + DEFAULT_DISK_EXT_LIST + end + + # @param [Vagrant::Machine] machine + # @return [String] + def self.set_default_disk_ext(machine) DEFAULT_DISK_EXT end end diff --git a/plugins/providers/hyperv/plugin.rb b/plugins/providers/hyperv/plugin.rb index cdf88d7e3..c7d6eb2b9 100644 --- a/plugins/providers/hyperv/plugin.rb +++ b/plugins/providers/hyperv/plugin.rb @@ -47,7 +47,12 @@ module VagrantPlugins Cap::ValidateDiskExt end - provider_capability(:hyperv, :get_default_disk_ext) do + provider_capability(:hyperv, :default_disk_exts) do + require_relative "cap/validate_disk_ext" + Cap::ValidateDiskExt + end + + provider_capability(:hyperv, :set_default_disk_ext) do require_relative "cap/validate_disk_ext" Cap::ValidateDiskExt end diff --git a/plugins/providers/virtualbox/cap/validate_disk_ext.rb b/plugins/providers/virtualbox/cap/validate_disk_ext.rb index e820c39ab..f5da4e8b4 100644 --- a/plugins/providers/virtualbox/cap/validate_disk_ext.rb +++ b/plugins/providers/virtualbox/cap/validate_disk_ext.rb @@ -7,18 +7,25 @@ module VagrantPlugins LOGGER = Log4r::Logger.new("vagrant::plugins::virtualbox::validate_disk_ext") # The default set of disk formats that VirtualBox supports - DEFAULT_DISK_EXT = ["vdi", "vmdk", "vhd"].map(&:freeze).freeze + DEFAULT_DISK_EXT_LIST = ["vdi", "vmdk", "vhd"].map(&:freeze).freeze + DEFAULT_DISK_EXT = "vdi".freeze # @param [Vagrant::Machine] machine # @param [String] disk_ext # @return [Bool] def self.validate_disk_ext(machine, disk_ext) - DEFAULT_DISK_EXT.include?(disk_ext) + DEFAULT_DISK_EXT_LIST.include?(disk_ext) end # @param [Vagrant::Machine] machine # @return [Array] - def self.get_default_disk_ext(machine) + def self.default_disk_exts(machine) + DEFAULT_DISK_EXT_LIST + end + + # @param [Vagrant::Machine] machine + # @return [String] + def self.set_default_disk_ext(machine) DEFAULT_DISK_EXT end end diff --git a/plugins/providers/virtualbox/plugin.rb b/plugins/providers/virtualbox/plugin.rb index 9cb46b54e..63f6a0e21 100644 --- a/plugins/providers/virtualbox/plugin.rb +++ b/plugins/providers/virtualbox/plugin.rb @@ -54,7 +54,12 @@ module VagrantPlugins Cap::ValidateDiskExt end - provider_capability(:virtualbox, :get_default_disk_ext) do + provider_capability(:virtualbox, :default_disk_exts) do + require_relative "cap/validate_disk_ext" + Cap::ValidateDiskExt + end + + provider_capability(:virtualbox, :set_default_disk_ext) do require_relative "cap/validate_disk_ext" Cap::ValidateDiskExt end diff --git a/test/unit/plugins/kernel_v2/config/disk_test.rb b/test/unit/plugins/kernel_v2/config/disk_test.rb index cb37ead49..967a9046d 100644 --- a/test/unit/plugins/kernel_v2/config/disk_test.rb +++ b/test/unit/plugins/kernel_v2/config/disk_test.rb @@ -37,6 +37,8 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do subject.size = 100 allow(provider).to receive(:capability?).with(:validate_disk_ext).and_return(true) allow(provider).to receive(:capability).with(:validate_disk_ext, "vdi").and_return(true) + allow(provider).to receive(:capability?).with(:set_default_disk_ext).and_return(true) + allow(provider).to receive(:capability).with(:set_default_disk_ext).and_return("vdi") end describe "with defaults" do @@ -64,10 +66,17 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigDisk do subject.finalize! assert_invalid end - end - describe "defining a new config that needs to match internal restraints" do - before do + context "with an invalid disk extension" do + before do + allow(provider).to receive(:capability?).with(:validate_disk_ext).and_return(true) + allow(provider).to receive(:capability).with(:validate_disk_ext, "fake").and_return(false) + end + + it "raises an error" do + subject.finalize! + assert_invalid + end end end end