From bb1eba7195fe9bbc775cf712486b086ed4a4c8cc Mon Sep 17 00:00:00 2001
From: Takashi Kajinami <kajinamit@oss.nttdata.com>
Date: Mon, 7 Oct 2024 12:56:32 +0900
Subject: [PATCH] Use Hash values for properties

Use Hash values instead of Array values to avoid unnecessary conversion
between actual value type and internal data type. This allows us to
avoid potential issues caused by tricky parsing or conversion.

Note that this could not be backword compatible and users have to
update their manifests to adopt to this change.

Change-Id: Id4a32752eb1073c6467d089bc97c8271741feba0
---
 lib/puppet/provider/cinder_qos/openstack.rb   | 29 +++++++++----------
 lib/puppet/provider/cinder_type/openstack.rb  | 23 +++++++--------
 lib/puppet/type/cinder_qos.rb                 | 15 +++++-----
 lib/puppet/type/cinder_type.rb                | 15 +++++-----
 manifests/backend/dellemc_powerflex.pp        |  2 +-
 manifests/backend/dellemc_powermax.pp         |  2 +-
 manifests/backend/dellemc_powerstore.pp       |  2 +-
 manifests/backend/dellemc_sc.pp               |  2 +-
 manifests/backend/dellemc_unity.pp            |  2 +-
 manifests/backend/dellemc_xtremio.pp          |  2 +-
 manifests/backend/emc_vnx.pp                  |  2 +-
 manifests/backend/gpfs.pp                     |  2 +-
 manifests/backend/hpe3par_iscsi.pp            |  2 +-
 manifests/backend/ibm_svf.pp                  |  2 +-
 manifests/backend/iscsi.pp                    |  2 +-
 manifests/backend/netapp.pp                   |  2 +-
 manifests/backend/nexenta.pp                  |  2 +-
 manifests/backend/nfs.pp                      |  2 +-
 manifests/backend/pure.pp                     |  2 +-
 manifests/backend/quobyte.pp                  |  2 +-
 manifests/backend/rbd.pp                      |  2 +-
 manifests/backend/san.pp                      |  2 +-
 manifests/backend/solidfire.pp                |  2 +-
 manifests/backend/vstorage.pp                 |  4 +--
 .../properties-hash-840c382dd23bcdbf.yaml     |  5 ++++
 spec/acceptance/10_basic_cinder_spec.rb       |  6 +++-
 .../cinder_backend_dellemc_powerflex_spec.rb  |  2 +-
 .../cinder_backend_dellemc_powermax_spec.rb   |  2 +-
 .../cinder_backend_dellemc_powerstore_spec.rb |  4 +--
 .../defines/cinder_backend_dellemc_sc_spec.rb |  5 +++-
 .../cinder_backend_dellemc_unity_spec.rb      |  2 +-
 .../cinder_backend_dellemc_xtremio_spec.rb    |  4 +--
 spec/defines/cinder_backend_emc_vnx_spec.rb   |  2 +-
 spec/defines/cinder_backend_gpfs_spec.rb      |  2 +-
 .../cinder_backend_hpe3par_iscsi_spec.rb      |  2 +-
 spec/defines/cinder_backend_ibm_svf_spec.rb   |  2 +-
 spec/defines/cinder_backend_iscsi_spec.rb     |  2 +-
 spec/defines/cinder_backend_netapp_spec.rb    |  2 +-
 spec/defines/cinder_backend_nexenta_spec.rb   |  2 +-
 spec/defines/cinder_backend_nfs_spec.rb       |  2 +-
 spec/defines/cinder_backend_pure_spec.rb      |  2 +-
 spec/defines/cinder_backend_quobyte_spec.rb   |  2 +-
 spec/defines/cinder_backend_rbd_spec.rb       |  2 +-
 spec/defines/cinder_backend_san_spec.rb       |  2 +-
 spec/defines/cinder_backend_solidfire_spec.rb |  2 +-
 spec/defines/cinder_backend_vstorage_spec.rb  |  6 ++--
 .../provider/cinder_qos/openstack_spec.rb     | 12 ++++----
 .../provider/cinder_type/openstack_spec.rb    | 26 ++++++++++-------
 spec/unit/type/cinder_qos_spec.rb             |  8 ++---
 spec/unit/type/cinder_type_spec.rb            |  8 ++---
 50 files changed, 126 insertions(+), 114 deletions(-)
 create mode 100644 releasenotes/notes/properties-hash-840c382dd23bcdbf.yaml

diff --git a/lib/puppet/provider/cinder_qos/openstack.rb b/lib/puppet/provider/cinder_qos/openstack.rb
index 259b2e5e..d3b69eff 100644
--- a/lib/puppet/provider/cinder_qos/openstack.rb
+++ b/lib/puppet/provider/cinder_qos/openstack.rb
@@ -16,8 +16,10 @@ Puppet::Type.type(:cinder_qos).provide(
     unless resource[:consumer].empty?
       properties << '--consumer' << resource[:consumer]
     end
-    resource[:properties].each do |item|
-      properties << '--property' << item
+    if resource[:properties]
+      resource[:properties].each do |k, v|
+        properties << '--property' << "#{k}=#{v}"
+      end
     end
     properties << name
     self.class.request('volume qos', 'create', properties)
@@ -43,12 +45,14 @@ Puppet::Type.type(:cinder_qos).provide(
   end
 
   def properties=(value)
-    properties = []
-    (value - @property_hash[:properties]).each do |item|
-      properties << '--property' << item
+    added = []
+    @property_hash[:properties].each do |k, v|
+      if value[k] != v
+        added << '--property' << "#{k}=#{value[k]}"
+      end
     end
-    unless properties.empty?
-      self.class.request('volume qos', 'set', [properties, name])
+    unless added.empty?
+      self.class.request('volume type', 'set', [properties, added])
       @property_hash[:properties] = value
     end
   end
@@ -73,7 +77,7 @@ Puppet::Type.type(:cinder_qos).provide(
         :name         => qos[:name],
         :ensure       => :present,
         :id           => qos[:id],
-        :properties   => pythondict2array(properties),
+        :properties   => pythondict2hash(properties),
         :consumer     => qos[:consumer],
         :associations => string2array(qos[:associations])
       })
@@ -93,12 +97,7 @@ Puppet::Type.type(:cinder_qos).provide(
     return input.delete("'").split(/,\s/)
   end
 
-  def self.pythondict2array(input)
-    json_input = JSON.parse(input.gsub(/'/, '"'))
-    output = []
-    json_input.each do | k, v |
-      output = output + ["#{k}=#{v}"]
-    end
-    return output
+  def self.pythondict2hash(input)
+    return JSON.parse(input.gsub(/'/, '"'))
   end
 end
diff --git a/lib/puppet/provider/cinder_type/openstack.rb b/lib/puppet/provider/cinder_type/openstack.rb
index 7b739c7b..e4c42956 100644
--- a/lib/puppet/provider/cinder_type/openstack.rb
+++ b/lib/puppet/provider/cinder_type/openstack.rb
@@ -13,8 +13,10 @@ Puppet::Type.type(:cinder_type).provide(
 
   def create
     properties = []
-    resource[:properties].each do |item|
-      properties << '--property' << item
+    if resource[:properties]
+      resource[:properties].each do |k, v|
+        properties << '--property' << "#{k}=#{v}"
+      end
     end
     properties << (@resource[:is_public] == :true ? '--public' : '--private')
     properties << name
@@ -40,8 +42,10 @@ Puppet::Type.type(:cinder_type).provide(
 
   def properties=(value)
     properties = []
-    (value - @property_hash[:properties]).each do |item|
-      properties << '--property' << item
+    @property_hash[:properties].each do |k, v|
+      if value[k] != v
+        properties << '--property' << "#{k}=#{value[k]}"
+      end
     end
     unless properties.empty?
       self.class.request('volume type', 'set', [properties, name])
@@ -93,7 +97,7 @@ Puppet::Type.type(:cinder_type).provide(
         :name               => type[:name],
         :ensure             => :present,
         :id                 => type[:id],
-        :properties         => pythondict2array(type[:properties]),
+        :properties         => pythondict2hash(type[:properties]),
         :is_public          => type[:is_public],
         :access_project_ids => type[:access_project_ids]
       })
@@ -113,12 +117,7 @@ Puppet::Type.type(:cinder_type).provide(
     return input.delete("'").split(/,\s/)
   end
 
-  def self.pythondict2array(input)
-    json_input = JSON.parse(input.gsub(/'/, '"'))
-    output = []
-    json_input.each do | k, v |
-      output = output + ["#{k}=#{v}"]
-    end
-    return output
+  def self.pythondict2hash(input)
+    return JSON.parse(input.gsub(/'/, '"'))
   end
 end
diff --git a/lib/puppet/type/cinder_qos.rb b/lib/puppet/type/cinder_qos.rb
index b9e012ce..9876e37c 100644
--- a/lib/puppet/type/cinder_qos.rb
+++ b/lib/puppet/type/cinder_qos.rb
@@ -20,15 +20,14 @@ Puppet::Type.newtype(:cinder_qos) do
     end
   end
 
-  newproperty(:properties, :array_matching => :all) do
-    desc 'The Properties of the cinder qos. Should be an array'
-    defaultto []
-    def insync?(is)
-      return false unless is.is_a? Array
-      is.sort == should.sort
-    end
+  newproperty(:properties) do
+    desc "The set of volume qos properties"
     validate do |value|
-      raise ArgumentError, "Properties doesn't match" unless value.match(/^\s*[^=\s]+=[^=\s]+$/)
+      if value.is_a?(Hash)
+        return true
+      else
+        raise ArgumentError, "Invalid properties #{value}. Requires a Hash, not a #{value.class}"
+      end
     end
   end
 
diff --git a/lib/puppet/type/cinder_type.rb b/lib/puppet/type/cinder_type.rb
index 616509d1..57d1f58d 100644
--- a/lib/puppet/type/cinder_type.rb
+++ b/lib/puppet/type/cinder_type.rb
@@ -8,15 +8,14 @@ Puppet::Type.newtype(:cinder_type) do
     newvalues(/\S+/)
   end
 
-  newproperty(:properties, :array_matching => :all) do
-    desc 'The properties of the cinder type. Should be an array, all items should match pattern <key=value>'
-    defaultto []
-    def insync?(is)
-      return false unless is.is_a? Array
-      is.sort == should.sort
-    end
+  newproperty(:properties) do
+    desc "The set of volume type properties"
     validate do |value|
-      raise ArgumentError, "Properties doesn't match" unless value.match(/^[^=\s]+=[^=]+$/)
+      if value.is_a?(Hash)
+        return true
+      else
+        raise ArgumentError, "Invalid properties #{value}. Requires a Hash, not a #{value.class}"
+      end
     end
   end
 
diff --git a/manifests/backend/dellemc_powerflex.pp b/manifests/backend/dellemc_powerflex.pp
index 20f6d4da..05826966 100644
--- a/manifests/backend/dellemc_powerflex.pp
+++ b/manifests/backend/dellemc_powerflex.pp
@@ -172,7 +172,7 @@ define cinder::backend::dellemc_powerflex(
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/dellemc_powermax.pp b/manifests/backend/dellemc_powermax.pp
index 2c4aac94..65eeffe1 100644
--- a/manifests/backend/dellemc_powermax.pp
+++ b/manifests/backend/dellemc_powermax.pp
@@ -138,7 +138,7 @@ define cinder::backend::dellemc_powermax (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/dellemc_powerstore.pp b/manifests/backend/dellemc_powerstore.pp
index 62532513..9f15d82a 100644
--- a/manifests/backend/dellemc_powerstore.pp
+++ b/manifests/backend/dellemc_powerstore.pp
@@ -101,7 +101,7 @@ define cinder::backend::dellemc_powerstore (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/dellemc_sc.pp b/manifests/backend/dellemc_sc.pp
index 482f42ef..1a22364b 100644
--- a/manifests/backend/dellemc_sc.pp
+++ b/manifests/backend/dellemc_sc.pp
@@ -170,7 +170,7 @@ define cinder::backend::dellemc_sc (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/dellemc_unity.pp b/manifests/backend/dellemc_unity.pp
index acf43cca..b9e2ac8f 100644
--- a/manifests/backend/dellemc_unity.pp
+++ b/manifests/backend/dellemc_unity.pp
@@ -109,7 +109,7 @@ define cinder::backend::dellemc_unity (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/dellemc_xtremio.pp b/manifests/backend/dellemc_xtremio.pp
index 727aa24f..2ebb56a5 100644
--- a/manifests/backend/dellemc_xtremio.pp
+++ b/manifests/backend/dellemc_xtremio.pp
@@ -136,7 +136,7 @@ define cinder::backend::dellemc_xtremio (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/emc_vnx.pp b/manifests/backend/emc_vnx.pp
index ffe0cd92..e8152968 100644
--- a/manifests/backend/emc_vnx.pp
+++ b/manifests/backend/emc_vnx.pp
@@ -200,7 +200,7 @@ define cinder::backend::emc_vnx (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/gpfs.pp b/manifests/backend/gpfs.pp
index 2044640d..a7ac0baa 100644
--- a/manifests/backend/gpfs.pp
+++ b/manifests/backend/gpfs.pp
@@ -159,7 +159,7 @@ define cinder::backend::gpfs (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/hpe3par_iscsi.pp b/manifests/backend/hpe3par_iscsi.pp
index 807acfeb..a06b2dd1 100644
--- a/manifests/backend/hpe3par_iscsi.pp
+++ b/manifests/backend/hpe3par_iscsi.pp
@@ -147,7 +147,7 @@ define cinder::backend::hpe3par_iscsi(
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/ibm_svf.pp b/manifests/backend/ibm_svf.pp
index 554cd525..754cecef 100644
--- a/manifests/backend/ibm_svf.pp
+++ b/manifests/backend/ibm_svf.pp
@@ -138,7 +138,7 @@ define cinder::backend::ibm_svf (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/iscsi.pp b/manifests/backend/iscsi.pp
index e442b6a9..7c633982 100644
--- a/manifests/backend/iscsi.pp
+++ b/manifests/backend/iscsi.pp
@@ -130,7 +130,7 @@ define cinder::backend::iscsi (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/netapp.pp b/manifests/backend/netapp.pp
index 4e88150f..bee1b0a0 100644
--- a/manifests/backend/netapp.pp
+++ b/manifests/backend/netapp.pp
@@ -268,7 +268,7 @@ and will be removed in a future release.")
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/nexenta.pp b/manifests/backend/nexenta.pp
index 7a1b6f03..eef4fd19 100644
--- a/manifests/backend/nexenta.pp
+++ b/manifests/backend/nexenta.pp
@@ -124,7 +124,7 @@ define cinder::backend::nexenta (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/nfs.pp b/manifests/backend/nfs.pp
index 0e73b735..a5898804 100644
--- a/manifests/backend/nfs.pp
+++ b/manifests/backend/nfs.pp
@@ -160,7 +160,7 @@ define cinder::backend::nfs (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/pure.pp b/manifests/backend/pure.pp
index e10e542a..e2eaaec0 100644
--- a/manifests/backend/pure.pp
+++ b/manifests/backend/pure.pp
@@ -165,7 +165,7 @@ define cinder::backend::pure(
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/quobyte.pp b/manifests/backend/quobyte.pp
index 748f70d7..27b6a16a 100644
--- a/manifests/backend/quobyte.pp
+++ b/manifests/backend/quobyte.pp
@@ -97,7 +97,7 @@ define cinder::backend::quobyte (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/rbd.pp b/manifests/backend/rbd.pp
index 8cadb987..47af0b3a 100644
--- a/manifests/backend/rbd.pp
+++ b/manifests/backend/rbd.pp
@@ -178,7 +178,7 @@ define cinder::backend::rbd (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/san.pp b/manifests/backend/san.pp
index 61bc0091..0006675d 100644
--- a/manifests/backend/san.pp
+++ b/manifests/backend/san.pp
@@ -140,7 +140,7 @@ define cinder::backend::san (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/solidfire.pp b/manifests/backend/solidfire.pp
index 8f30dfcf..8744b5aa 100644
--- a/manifests/backend/solidfire.pp
+++ b/manifests/backend/solidfire.pp
@@ -181,7 +181,7 @@ define cinder::backend::solidfire(
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ["volume_backend_name=${volume_backend_name}"],
+      properties => {'volume_backend_name' => $volume_backend_name},
     }
   }
 
diff --git a/manifests/backend/vstorage.pp b/manifests/backend/vstorage.pp
index 8c412d31..4ef6b363 100644
--- a/manifests/backend/vstorage.pp
+++ b/manifests/backend/vstorage.pp
@@ -98,11 +98,11 @@ define cinder::backend::vstorage (
   if $manage_volume_type {
     cinder_type { $volume_backend_name:
       ensure     => present,
-      properties => ['vz:volume_format=qcow2'],
+      properties => {'vz:volume_format' => 'qcow2'},
     }
     cinder_type { "${volume_backend_name}-ploop":
       ensure     => present,
-      properties => ['vz:volume_format=ploop'],
+      properties => {'vz:volume_format' => 'ploop'},
     }
   }
 
diff --git a/releasenotes/notes/properties-hash-840c382dd23bcdbf.yaml b/releasenotes/notes/properties-hash-840c382dd23bcdbf.yaml
new file mode 100644
index 00000000..8098a83b
--- /dev/null
+++ b/releasenotes/notes/properties-hash-840c382dd23bcdbf.yaml
@@ -0,0 +1,5 @@
+---
+upgrade:
+  - |
+    The ``properties`` property of ``cinder_type`` resource and ``cinder_qos``
+    resource now accepts Hash values instead of Array values.
diff --git a/spec/acceptance/10_basic_cinder_spec.rb b/spec/acceptance/10_basic_cinder_spec.rb
index aacc4e5b..5a1721c4 100644
--- a/spec/acceptance/10_basic_cinder_spec.rb
+++ b/spec/acceptance/10_basic_cinder_spec.rb
@@ -16,7 +16,11 @@ describe 'basic cinder' do
       include openstack_integration::cinder
 
       cinder_type { 'testvolumetype' :
-        properties => ['k=v', 'key1=val1', 'key2=<is> True']
+        properties => {
+          'k'    => 'v',
+          'key1' => 'val1',
+          'key2' => '<is> True'
+        }
       }
       EOS
 
diff --git a/spec/defines/cinder_backend_dellemc_powerflex_spec.rb b/spec/defines/cinder_backend_dellemc_powerflex_spec.rb
index 14771e0a..ee542f97 100644
--- a/spec/defines/cinder_backend_dellemc_powerflex_spec.rb
+++ b/spec/defines/cinder_backend_dellemc_powerflex_spec.rb
@@ -75,7 +75,7 @@ describe 'cinder::backend::dellemc_powerflex' do
 
       it { is_expected.to contain_cinder_type("#{title}").with(
         :ensure     => 'present',
-        :properties => ["volume_backend_name=#{title}"]
+        :properties => {'volume_backend_name' => title}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_dellemc_powermax_spec.rb b/spec/defines/cinder_backend_dellemc_powermax_spec.rb
index 2b99f18e..54588f3d 100644
--- a/spec/defines/cinder_backend_dellemc_powermax_spec.rb
+++ b/spec/defines/cinder_backend_dellemc_powermax_spec.rb
@@ -121,7 +121,7 @@ describe 'cinder::backend::dellemc_powermax' do
 
       it { is_expected.to contain_cinder_type('dellemc_powermax').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=dellemc_powermax']
+        :properties => {'volume_backend_name' => 'dellemc_powermax'}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_dellemc_powerstore_spec.rb b/spec/defines/cinder_backend_dellemc_powerstore_spec.rb
index 7c7d5acc..01ef578f 100644
--- a/spec/defines/cinder_backend_dellemc_powerstore_spec.rb
+++ b/spec/defines/cinder_backend_dellemc_powerstore_spec.rb
@@ -107,8 +107,8 @@ describe 'cinder::backend::dellemc_powerstore' do
       end
 
       it { is_expected.to contain_cinder_type('dellemc_powerstore').with(
-        :ensure => 'present',
-        :properties => ['volume_backend_name=dellemc_powerstore']
+        :ensure     => 'present',
+        :properties => {'volume_backend_name' => 'dellemc_powerstore'}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_dellemc_sc_spec.rb b/spec/defines/cinder_backend_dellemc_sc_spec.rb
index b3264815..42bc25b3 100644
--- a/spec/defines/cinder_backend_dellemc_sc_spec.rb
+++ b/spec/defines/cinder_backend_dellemc_sc_spec.rb
@@ -120,7 +120,10 @@ describe 'cinder::backend::dellemc_sc' do
         params.merge!({:manage_volume_type => true})
       end
 
-      it { is_expected.to contain_cinder_type('dellemc_sc').with(:ensure => :present, :properties => ['volume_backend_name=dellemc_sc']) }
+      it { is_expected.to contain_cinder_type('dellemc_sc').with(
+        :ensure     => :present,
+        :properties => {'volume_backend_name' => 'dellemc_sc'}
+      ) }
     end
   end
 
diff --git a/spec/defines/cinder_backend_dellemc_unity_spec.rb b/spec/defines/cinder_backend_dellemc_unity_spec.rb
index 47e36a0c..67380243 100644
--- a/spec/defines/cinder_backend_dellemc_unity_spec.rb
+++ b/spec/defines/cinder_backend_dellemc_unity_spec.rb
@@ -99,7 +99,7 @@ describe 'cinder::backend::dellemc_unity' do
 
       it { is_expected.to contain_cinder_type('dellemc_unity').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=dellemc_unity']
+        :properties => {'volume_backend_name' => 'dellemc_unity'}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_dellemc_xtremio_spec.rb b/spec/defines/cinder_backend_dellemc_xtremio_spec.rb
index ac702244..32da7d30 100644
--- a/spec/defines/cinder_backend_dellemc_xtremio_spec.rb
+++ b/spec/defines/cinder_backend_dellemc_xtremio_spec.rb
@@ -110,8 +110,8 @@ describe 'cinder::backend::dellemc_xtremio' do
       end
 
       it { is_expected.to contain_cinder_type('dellemc_xtremio').with(
-        :ensure => 'present',
-        :properties => ['volume_backend_name=dellemc_xtremio']
+        :ensure     => 'present',
+        :properties => {'volume_backend_name' => 'dellemc_xtremio'}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_emc_vnx_spec.rb b/spec/defines/cinder_backend_emc_vnx_spec.rb
index 7c9a83b3..60b76b09 100644
--- a/spec/defines/cinder_backend_emc_vnx_spec.rb
+++ b/spec/defines/cinder_backend_emc_vnx_spec.rb
@@ -95,7 +95,7 @@ describe 'cinder::backend::emc_vnx' do
 
       it { is_expected.to contain_cinder_type('emc').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=emc']
+        :properties => {'volume_backend_name' => 'emc'}
       )}
     end
 
diff --git a/spec/defines/cinder_backend_gpfs_spec.rb b/spec/defines/cinder_backend_gpfs_spec.rb
index 4c5aff57..2d44a30a 100644
--- a/spec/defines/cinder_backend_gpfs_spec.rb
+++ b/spec/defines/cinder_backend_gpfs_spec.rb
@@ -113,7 +113,7 @@ describe 'cinder::backend::gpfs' do
 
       it { is_expected.to contain_cinder_type('gpfs').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=gpfs']
+        :properties => {'volume_backend_name' => 'gpfs'}
       )}
     end
 
diff --git a/spec/defines/cinder_backend_hpe3par_iscsi_spec.rb b/spec/defines/cinder_backend_hpe3par_iscsi_spec.rb
index 96d5536e..2057cedb 100644
--- a/spec/defines/cinder_backend_hpe3par_iscsi_spec.rb
+++ b/spec/defines/cinder_backend_hpe3par_iscsi_spec.rb
@@ -69,7 +69,7 @@ describe 'cinder::backend::hpe3par_iscsi' do
 
       it { is_expected.to contain_cinder_type('hpe3par_iscsi').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=hpe3par_iscsi']
+        :properties => {'volume_backend_name' => 'hpe3par_iscsi'}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_ibm_svf_spec.rb b/spec/defines/cinder_backend_ibm_svf_spec.rb
index 79caa704..01b56074 100644
--- a/spec/defines/cinder_backend_ibm_svf_spec.rb
+++ b/spec/defines/cinder_backend_ibm_svf_spec.rb
@@ -91,7 +91,7 @@ describe 'cinder::backend::ibm_svf' do
 
       it { is_expected.to contain_cinder_type('ibm_svf').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=ibm_svf']
+        :properties => {'volume_backend_name' => 'ibm_svf'}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_iscsi_spec.rb b/spec/defines/cinder_backend_iscsi_spec.rb
index e19956e6..83d59575 100644
--- a/spec/defines/cinder_backend_iscsi_spec.rb
+++ b/spec/defines/cinder_backend_iscsi_spec.rb
@@ -111,7 +111,7 @@ describe 'cinder::backend::iscsi' do
 
       it { is_expected.to contain_cinder_type('hippo').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=hippo']
+        :properties => {'volume_backend_name' => 'hippo'}
       )}
     end
 
diff --git a/spec/defines/cinder_backend_netapp_spec.rb b/spec/defines/cinder_backend_netapp_spec.rb
index 41749bfd..36beace5 100644
--- a/spec/defines/cinder_backend_netapp_spec.rb
+++ b/spec/defines/cinder_backend_netapp_spec.rb
@@ -77,7 +77,7 @@ describe 'cinder::backend::netapp' do
 
       it { is_expected.to contain_cinder_type('netapp-cdot-nfs').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=netapp-cdot-nfs']
+        :properties => {'volume_backend_name' => 'netapp-cdot-nfs'}
       )}
     end
 
diff --git a/spec/defines/cinder_backend_nexenta_spec.rb b/spec/defines/cinder_backend_nexenta_spec.rb
index df39ade2..cf3a5653 100644
--- a/spec/defines/cinder_backend_nexenta_spec.rb
+++ b/spec/defines/cinder_backend_nexenta_spec.rb
@@ -59,7 +59,7 @@ describe 'cinder::backend::nexenta' do
 
       it { is_expected.to contain_cinder_type('nexenta').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=nexenta']
+        :properties => {'volume_backend_name' => 'nexenta'}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_nfs_spec.rb b/spec/defines/cinder_backend_nfs_spec.rb
index f54741cf..316726dd 100644
--- a/spec/defines/cinder_backend_nfs_spec.rb
+++ b/spec/defines/cinder_backend_nfs_spec.rb
@@ -95,7 +95,7 @@ describe 'cinder::backend::nfs' do
 
       it { is_expected.to contain_cinder_type('hippo').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=hippo']
+        :properties => {'volume_backend_name' => 'hippo'}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_pure_spec.rb b/spec/defines/cinder_backend_pure_spec.rb
index 0131e59f..bbd96b61 100644
--- a/spec/defines/cinder_backend_pure_spec.rb
+++ b/spec/defines/cinder_backend_pure_spec.rb
@@ -126,7 +126,7 @@ describe 'cinder::backend::pure' do
 
       it { is_expected.to contain_cinder_type('pure').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=pure']
+        :properties => {'volume_backend_name' => 'pure'}
       )}
     end
 
diff --git a/spec/defines/cinder_backend_quobyte_spec.rb b/spec/defines/cinder_backend_quobyte_spec.rb
index 82b136ce..8f374bed 100644
--- a/spec/defines/cinder_backend_quobyte_spec.rb
+++ b/spec/defines/cinder_backend_quobyte_spec.rb
@@ -37,7 +37,7 @@ describe 'cinder::backend::quobyte' do
 
       it { is_expected.to contain_cinder_type('myquobyte').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=myquobyte']
+        :properties => {'volume_backend_name' => 'myquobyte'}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_rbd_spec.rb b/spec/defines/cinder_backend_rbd_spec.rb
index f900ecdf..cd00f837 100644
--- a/spec/defines/cinder_backend_rbd_spec.rb
+++ b/spec/defines/cinder_backend_rbd_spec.rb
@@ -133,7 +133,7 @@ describe 'cinder::backend::rbd' do
 
         it { is_expected.to contain_cinder_type('rbd-ssd').with(
           :ensure     => 'present',
-          :properties => ['volume_backend_name=rbd-ssd']
+          :properties => {'volume_backend_name' => 'rbd-ssd'}
         )}
       end
       context 'rbd backend without managing package' do
diff --git a/spec/defines/cinder_backend_san_spec.rb b/spec/defines/cinder_backend_san_spec.rb
index f3f5b939..5de25e25 100644
--- a/spec/defines/cinder_backend_san_spec.rb
+++ b/spec/defines/cinder_backend_san_spec.rb
@@ -85,7 +85,7 @@ describe 'cinder::backend::san' do
 
       it { is_expected.to contain_cinder_type('mysan').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=mysan']
+        :properties => {'volume_backend_name' => 'mysan'}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_solidfire_spec.rb b/spec/defines/cinder_backend_solidfire_spec.rb
index fa85e123..c5238b85 100644
--- a/spec/defines/cinder_backend_solidfire_spec.rb
+++ b/spec/defines/cinder_backend_solidfire_spec.rb
@@ -115,7 +115,7 @@ describe 'cinder::backend::solidfire' do
 
       it { is_expected.to contain_cinder_type('solidfire').with(
         :ensure     => 'present',
-        :properties => ['volume_backend_name=solidfire']
+        :properties => {'volume_backend_name' => 'solidfire'}
       )}
     end
   end
diff --git a/spec/defines/cinder_backend_vstorage_spec.rb b/spec/defines/cinder_backend_vstorage_spec.rb
index b70e9f9c..46024947 100644
--- a/spec/defines/cinder_backend_vstorage_spec.rb
+++ b/spec/defines/cinder_backend_vstorage_spec.rb
@@ -53,13 +53,13 @@ describe 'cinder::backend::vstorage' do
       end
 
       it { is_expected.to contain_cinder_type('vstorage').with(
-        :ensure => 'present',
-        :properties => ['vz:volume_format=qcow2']
+        :ensure     => 'present',
+        :properties => {'vz:volume_format' => 'qcow2'}
       )}
 
       it { is_expected.to contain_cinder_type('vstorage-ploop').with(
         :ensure => 'present',
-        :properties => ['vz:volume_format=ploop']
+        :properties => {'vz:volume_format' => 'ploop'}
       )}
     end
   end
diff --git a/spec/unit/provider/cinder_qos/openstack_spec.rb b/spec/unit/provider/cinder_qos/openstack_spec.rb
index facc860d..5dafba1d 100644
--- a/spec/unit/provider/cinder_qos/openstack_spec.rb
+++ b/spec/unit/provider/cinder_qos/openstack_spec.rb
@@ -16,7 +16,7 @@ describe provider_class do
       {
          :name       => 'QoS_1',
          :ensure     => :present,
-         :properties => ['key1=value1', 'key2=value2'],
+         :properties => {'key1' => 'value1', 'key2' => 'value2'},
       }
     end
 
@@ -66,11 +66,11 @@ properties="{\'key1\': \'value1\', \'key2\': \'value2\'}"
           expect(instances[0].name).to eq('qos-1')
           expect(instances[0].associations).to eq(['my_type1', 'my_type2'])
           expect(instances[0].consumer).to eq('front-end')
-          expect(instances[0].properties).to eq(['read_iops=value1', 'write_iops=value2'])
+          expect(instances[0].properties).to eq({'read_iops'=>'value1', 'write_iops'=>'value2'})
           expect(instances[1].name).to eq('qos-2')
           expect(instances[1].consumer).to eq('both')
           expect(instances[1].associations).to eq([])
-          expect(instances[1].properties).to eq([])
+          expect(instances[1].properties).to eq({})
         end
       end
 
@@ -81,10 +81,10 @@ properties="{\'key1\': \'value1\', \'key2\': \'value2\'}"
         end
       end
 
-      describe '#pythondict2array' do
-        it 'should return an array with key-value when provided with a python dict' do
+      describe '#pythondict2hash' do
+        it 'should return a hash when provided with a python dict' do
           s = "{'key': 'value', 'key2': 'value2'}"
-          expect(provider_class.pythondict2array(s)).to eq(['key=value', 'key2=value2'])
+          expect(provider_class.pythondict2hash(s)).to eq({'key'=>'value', 'key2'=>'value2'})
         end
       end
     end
diff --git a/spec/unit/provider/cinder_type/openstack_spec.rb b/spec/unit/provider/cinder_type/openstack_spec.rb
index ffb0e85e..42dfb436 100644
--- a/spec/unit/provider/cinder_type/openstack_spec.rb
+++ b/spec/unit/provider/cinder_type/openstack_spec.rb
@@ -14,11 +14,15 @@ describe provider_class do
 
     let(:type_attributes) do
       {
-         :name               => 'Backend_1',
-         :ensure             => :present,
-         :properties         => ['key=value', 'new_key=a-new_value', 'multiattach="<is> True"'],
-         :is_public          => true,
-         :access_project_ids => [],
+        :name               => 'Backend_1',
+        :ensure             => :present,
+        :properties         => {
+          'key'         => 'value',
+          'new_key'     => 'a-new_value',
+          'multiattach' => '<is> True'
+        },
+        :is_public          => true,
+        :access_project_ids => [],
       }
     end
 
@@ -36,7 +40,7 @@ describe provider_class do
       describe '#create' do
         it 'creates a type' do
           expect(provider_class).to receive(:openstack)
-            .with('volume type', 'create', '--format', 'shell', ['--property', 'key=value', '--property', 'new_key=a-new_value', '--property', 'multiattach="<is> True"', '--public', 'Backend_1'])
+            .with('volume type', 'create', '--format', 'shell', ['--property', 'key=value', '--property', 'new_key=a-new_value', '--property', 'multiattach=<is> True', '--public', 'Backend_1'])
             .and_return('id="90e19aff-1b35-4d60-9ee3-383c530275ab"
 name="Backend_1"
 properties="{\'key\': \'value\', \'new_key\': \'a-new_value\', \'multiattach\': \'<is> True\'}"
@@ -82,8 +86,8 @@ access_project_ids="54f4d231201b4944a5fa4587a09bda23, 54f4d231201b4944a5fa4587a0
           expect(instances[1].is_public).to be false
           expect(instances[0].access_project_ids).to match_array([])
           expect(instances[1].access_project_ids).to match_array(['54f4d231201b4944a5fa4587a09bda23', '54f4d231201b4944a5fa4587a09bda28'])
-          expect(instances[0].properties).to eq(["key1=value1"])
-          expect(instances[1].properties).to eq(["key2=value2"])
+          expect(instances[0].properties).to eq({'key1'=>'value1'})
+          expect(instances[1].properties).to eq({'key2'=>'value2'})
         end
       end
 
@@ -94,10 +98,10 @@ access_project_ids="54f4d231201b4944a5fa4587a09bda23, 54f4d231201b4944a5fa4587a0
         end
       end
 
-      describe '#pythondict2array' do
-        it 'should return an array with key-value when provided with a python dict' do
+      describe '#pythondict2hash' do
+        it 'should return a hash when provided with a python dict' do
           s = "{'key': 'value', 'key2': 'value2'}"
-          expect(provider_class.pythondict2array(s)).to eq(['key=value', 'key2=value2'])
+          expect(provider_class.pythondict2hash(s)).to eq({'key'=>'value', 'key2'=>'value2'})
         end
       end
     end
diff --git a/spec/unit/type/cinder_qos_spec.rb b/spec/unit/type/cinder_qos_spec.rb
index 5459bc33..08238462 100644
--- a/spec/unit/type/cinder_qos_spec.rb
+++ b/spec/unit/type/cinder_qos_spec.rb
@@ -12,17 +12,17 @@ describe Puppet::Type.type(:cinder_qos) do
       :name       => 'test_qos',
       :properties => ['some_key1 = some_value2']
     }
-    expect { Puppet::Type.type(:cinder_qos).new(incorrect_input) }.to raise_error(Puppet::ResourceError, /Parameter properties failed/)
+    expect { Puppet::Type.type(:cinder_type).new(incorrect_input) }.to raise_error(Puppet::ResourceError, /Invalid properties/)
   end
 
   it 'should default to no properties and no associations' do
     catalog = Puppet::Resource::Catalog.new
     anchor = Puppet::Type.type(:anchor).new(:name => 'cinder::service::end')
     correct_input = {
-      :name       => 'test_qos',
+      :name => 'test_qos',
     }
     cinder_qos = Puppet::Type.type(:cinder_qos).new(correct_input)
-    expect(cinder_qos[:properties]).to eq([])
+    expect(cinder_qos[:properties]).to eq(nil)
     expect(cinder_qos[:associations]).to eq([])
 
     catalog.add_resource anchor, cinder_qos
@@ -36,7 +36,7 @@ describe Puppet::Type.type(:cinder_qos) do
   it 'should autorequire cinder-api service' do
     catalog = Puppet::Resource::Catalog.new
     anchor = Puppet::Type.type(:anchor).new(:name => 'cinder::service::end')
-    properties = ['read_iops=value1', 'write_iops=value2']
+    properties = {'read_iops' => 'value1', 'write_iops' => 'value2'}
     correct_input = {
       :name       => 'test_qos',
       :properties => properties,
diff --git a/spec/unit/type/cinder_type_spec.rb b/spec/unit/type/cinder_type_spec.rb
index 5f2bd170..a486e308 100644
--- a/spec/unit/type/cinder_type_spec.rb
+++ b/spec/unit/type/cinder_type_spec.rb
@@ -12,17 +12,17 @@ describe Puppet::Type.type(:cinder_type) do
       :name       => 'test_type',
       :properties => ['some_key1 = some_value2']
     }
-    expect { Puppet::Type.type(:cinder_type).new(incorrect_input) }.to raise_error(Puppet::ResourceError, /Parameter properties failed/)
+    expect { Puppet::Type.type(:cinder_type).new(incorrect_input) }.to raise_error(Puppet::ResourceError, /Invalid properties/)
   end
 
   it 'should default to no properties' do
     catalog = Puppet::Resource::Catalog.new
     anchor = Puppet::Type.type(:anchor).new(:name => 'cinder::service::end')
     correct_input = {
-      :name       => 'test_type',
+      :name => 'test_type',
     }
     cinder_type = Puppet::Type.type(:cinder_type).new(correct_input)
-    expect(cinder_type[:properties]).to eq([])
+    expect(cinder_type[:properties]).to eq(nil)
 
     catalog.add_resource anchor, cinder_type
     dependency = cinder_type.autorequire
@@ -35,7 +35,7 @@ describe Puppet::Type.type(:cinder_type) do
   it 'should autorequire cinder-api service' do
     catalog = Puppet::Resource::Catalog.new
     anchor = Puppet::Type.type(:anchor).new(:name => 'cinder::service::end')
-    properties = ['some_key1=value', 'some_key2=value1,value2']
+    properties = {'some_key1' => 'value', 'some_key2' => 'value1,value2'}
     correct_input = {
       :name       => 'test_type',
       :properties => properties,