volume filters always returns volume object when no match found

[electrofelix]

In trying to understand why the following piece of code https://github.com/hpe-hcss/authorization/blob/master/api/openapi-spec/authorization-broker-v1alpha2.yml#L120 always works, as in if there is no match found when using the filter argument to volumes.all() at https://github.com/fog/fog-libvirt/blob/master/lib/fog/libvirt/models/compute/volumes.rb#L10-L12, why would we be able to check the returned volume to see if it's id is not nil?

Doing some testing with the following where "someimage.img" does not exist

require 'fog/libvirt'

conn_attr = {:provider => 'libvirt', :libvirt_uri => 'qemu:///system'}
conn = Fog::Compute.new(conn_attr)
vols = conn.volumes.all(name: "someimage.img")
puts "vols => #{vols}"

and I'd get the outpu

vols => [  <Fog::Libvirt::Compute::Volume
    id=nil,
    pool_name="default",
    key=nil,
    name="fog-567594173461216",
    path=nil,
    capacity="10G",
    allocation="1G",
    owner="0",
    group="0",
    format_type="raw",
    backing_volume=nil
  >]

While digging into the code I spotted that that when a filter is used, https://github.com/fog/fog-libvirt/blob/master/lib/fog/libvirt/requests/compute/list_volumes.rb#L20 calls get_volume, which is defined at https://github.com/fog/fog-libvirt/blob/master/lib/fog/libvirt/requests/compute/list_volumes.rb#L72 and will return an empty hash in the case there is no volume.

Looking at https://github.com/fog/fog-libvirt/blob/master/lib/fog/libvirt/requests/compute/list_volumes.rb#L20 and https://github.com/fog/fog-libvirt/blob/master/lib/fog/libvirt/models/compute/volumes.rb#L11 it appears this results in [ {} ] being passed to https://github.com/fog/fog-core/blob/master/lib/fog/core/collection.rb#L75-L78 which will result in a collection containing a single volume created from an empty hash getting returned.

This seems like a bug, as if I do the same check with pools:

require 'fog/libvirt'

conn_attr = {:provider => 'libvirt', :libvirt_uri => 'qemu:///system'}
conn = Fog::Compute.new(conn_attr)

pools = conn.pools.all(name: "does not exist")
puts "pools => #{pools}"

the output is:

pools => []

A quick experiment suggests that the following is sufficient to resolve:

diff --git a/lib/fog/libvirt/requests/compute/list_volumes.rb b/lib/fog/libvirt/requests/compute/list_volumes.rb
index 9670f0a..3b837f7 100644
--- a/lib/fog/libvirt/requests/compute/list_volumes.rb
+++ b/lib/fog/libvirt/requests/compute/list_volumes.rb
@@ -17,7 +17,7 @@ module Fog
               end
             end
           else
-            return [get_volume(filter)]
+            data << get_volume(filter)
           end
           data.compact
         end
@@ -69,7 +69,8 @@ module Fog
               return raw ? vol : volume_to_attributes(vol)
             end
           end
-          { }
+
+          nil
         end
       end

Will send a PR up shortly assuming this seems reasonable

[github-actions[bot]]

This issue has been marked inactive and will be closed if no further activity occurs.

[electrofelix]

This is fixed, while now you can get a nil item added, it's consistent as the path to return a single volume above it also can return a nil item. So the same filter works for both.