Merge pull request #1419 from sshock/plh/add-drivetype-to-filesystem-plugin-take2

Fix missing volume name and re-implement adding drivetype to filesystem plugin

2 files changed, 33 insertions, 11 deletions

diff --git a/lib/ohai/plugins/filesystem.rb b/lib/ohai/plugins/filesystem.rb
index f81d1ada..ca9df27c 100644
--- a/lib/ohai/plugins/filesystem.rb
+++ b/lib/ohai/plugins/filesystem.rb
@@ -162,6 +162,9 @@ Ohai.plugin(:Filesystem) do
   end
 
   ### Windows specific methods BEGINS
+  # Drive types
+  DRIVE_TYPE ||= %w{unknown no_root_dir removable local network cd ram}.freeze
+
   # Volume encryption or decryption status
   #
   # @see https://docs.microsoft.com/en-us/windows/desktop/SecProv/getconversionstatus-win32-encryptablevolume#parameters
@@ -173,7 +176,7 @@ Ohai.plugin(:Filesystem) do
 
   # Returns a Mash loaded with logical details
   #
-  # Uses Win32_LogicalDisk and logical_properties to return encryption details of volumes.
+  # Uses Win32_LogicalDisk and logical_properties to return general details of volumes.
   #
   # Returns an empty Mash in case of any WMI exception.
   #
@@ -184,7 +187,7 @@ Ohai.plugin(:Filesystem) do
   def logical_info
     wmi = WmiLite::Wmi.new("Root\\CIMV2")
 
-    # Note: we should really be parsing Win32_Volume and Win32_Mapped drive.
+    # TODO: We should really be parsing Win32_Volume and Win32_MountPoint.
     disks = wmi.instances_of("Win32_LogicalDisk")
     logical_properties(disks)
   rescue WmiLite::WmiException
@@ -234,9 +237,9 @@ Ohai.plugin(:Filesystem) do
     properties = Mash.new
     disks.each do |disk|
       property = Mash.new
-      # In windows the closet thing we have to a device is the volume_name
+      # In windows the closest thing we have to a device is the volume name
       # and the "mountpoint" is the drive letter...
-      device = disk["volume_name"].to_s.downcase
+      device = disk["volumename"].to_s.downcase
       mount = disk["deviceid"]
       property[:kb_size] = disk["size"] ? disk["size"].to_i / 1000 : 0
       property[:kb_available] = disk["freespace"].to_i / 1000
@@ -244,7 +247,10 @@ Ohai.plugin(:Filesystem) do
       property[:percent_used] = (property[:kb_size] == 0 ? 0 : (property[:kb_used] * 100 / property[:kb_size]))
       property[:mount] = mount
       property[:fs_type] = disk["filesystem"].to_s.downcase
-      property[:volume_name] = device
+      property[:drive_type] = disk["drivetype"].to_i
+      property[:drive_type_string] = DRIVE_TYPE[disk["drivetype"].to_i]
+      property[:drive_type_human] = disk["description"].to_s
+      property[:volume_name] = disk["volumename"].to_s
       property[:device] = device
 
       key = "#{device},#{mount}"
diff --git a/spec/unit/plugins/windows/filesystem_spec.rb b/spec/unit/plugins/windows/filesystem_spec.rb
index 62996897..7082259b 100644
--- a/spec/unit/plugins/windows/filesystem_spec.rb
+++ b/spec/unit/plugins/windows/filesystem_spec.rb
@@ -31,18 +31,22 @@ describe Ohai::System, "Windows Filesystem Plugin", :windows_only do
         "deviceid" => "C:",
         "size" => "10000000",
         "filesystem" => "NTFS",
+        "drivetype" => 3,
+        "description" => "Local Fixed Disk",
         "freespace" => "100000",
         "name" => "C:",
-        "volumename " => "",
+        # omit "volumename"; it will be added in (some) tests below
       },
       {
         "caption" => "D:",
         "deviceid" => "D:",
         "size" => "10000000",
         "filesystem" => "FAT32",
+        "drivetype" => 2,
+        "description" => "Removable Disk",
         "freespace" => "100000",
         "name" => "D:",
-        # Lets not pass "volumename" for this drive
+        # omit "volumename"; it will be added in (some) tests below
       },
     ]
   end
@@ -95,6 +99,9 @@ describe Ohai::System, "Windows Filesystem Plugin", :windows_only do
       it "returns disk information" do
         {
           "fs_type" => "ntfs",
+          "drive_type" => 3,
+          "drive_type_string" => "local",
+          "drive_type_human" => "Local Fixed Disk",
           "volume_name" => "",
           "encryption_status" => "FullyDecrypted",
         }.each do |k, v|
@@ -104,6 +111,9 @@ describe Ohai::System, "Windows Filesystem Plugin", :windows_only do
 
         {
           "fs_type" => "fat32",
+          "drive_type" => 2,
+          "drive_type_string" => "removable",
+          "drive_type_human" => "Removable Disk",
           "volume_name" => "",
           "encryption_status" => "EncryptionInProgress",
         }.each do |k, v|
@@ -116,7 +126,7 @@ describe Ohai::System, "Windows Filesystem Plugin", :windows_only do
     context "when there are volume names" do
       before do
         ldi = logical_disks_instances
-        ldi.each_with_index { |d, i| d["volume_name"] = "Volume #{i}" }
+        ldi.each_with_index { |d, i| d["volumename"] = "Volume #{i}" }
         allow(plugin).to receive(:logical_info).and_return(plugin.logical_properties(ldi))
         allow(plugin).to receive(:encryptable_info).and_return(plugin.encryption_properties(encryptable_volume_instances))
         plugin.run
@@ -139,7 +149,10 @@ describe Ohai::System, "Windows Filesystem Plugin", :windows_only do
       it "returns disk information" do
         {
           "fs_type" => "ntfs",
-          "volume_name" => "volume 0",
+          "drive_type" => 3,
+          "drive_type_string" => "local",
+          "drive_type_human" => "Local Fixed Disk",
+          "volume_name" => "Volume 0",
           "encryption_status" => "FullyDecrypted",
         }.each do |k, v|
           expect(plugin[:filesystem]["C:"][k]).to eq(v)
@@ -148,7 +161,10 @@ describe Ohai::System, "Windows Filesystem Plugin", :windows_only do
 
         {
           "fs_type" => "fat32",
-          "volume_name" => "volume 1",
+          "drive_type" => 2,
+          "drive_type_string" => "removable",
+          "drive_type_human" => "Removable Disk",
+          "volume_name" => "Volume 1",
           "encryption_status" => "EncryptionInProgress",
         }.each do |k, v|
           expect(plugin[:filesystem]["D:"][k]).to eq(v)
@@ -160,7 +176,7 @@ describe Ohai::System, "Windows Filesystem Plugin", :windows_only do
 
   describe "#logical_properties" do
     let(:disks) { logical_disks_instances }
-    let(:logical_props) { %i{kb_size kb_available kb_used percent_used mount fs_type volume_name device} }
+    let(:logical_props) { %i{kb_size kb_available kb_used percent_used mount fs_type drive_type drive_type_string drive_type_human volume_name device} }
 
     it "Returns a mash" do
       expect(plugin.logical_properties(disks)).to be_a(Mash)