diff --git a/lib/puppet/functions/to_array_of_json_strings.rb b/lib/puppet/functions/to_array_of_json_strings.rb index 58ea7bb5f..a93c87464 100644 --- a/lib/puppet/functions/to_array_of_json_strings.rb +++ b/lib/puppet/functions/to_array_of_json_strings.rb @@ -10,20 +10,10 @@ Puppet::Functions.create_function(:to_array_of_json_strings) do end def to_array_of_json_strings(*args) - require 'json' - if (args.size != 1) then raise Puppet::ParseError, 'to_array_of_json_strings(): Wrong number of arguments' end list = args[0] - if list.class == String - begin - list = JSON.load(list) - rescue JSON::ParserError - raise Puppet::ParseError, "Syntax error: #{args[0]} is not valid" - end - list = [list] unless list.class == Array - end unless _array_of_hash?(list) raise Puppet::ParseError, "Syntax error: #{args[0]} is not an Array or JSON encoded String" end diff --git a/manifests/compute/pci.pp b/manifests/compute/pci.pp index 21c62b7e4..5ecdc1bcb 100644 --- a/manifests/compute/pci.pp +++ b/manifests/compute/pci.pp @@ -6,7 +6,7 @@ # # [*device_specs*] # (optional) Specify the PCI devices available to VMs. -# Defaults to $facts['os_service_default'] +# Defaults to [] # Example of format: # [ { "vendor_id" => "1234","product_id" => "5678" }, # { "vendor_id" => "4321","product_id" => "8765", "physical_network" => "default" } ] @@ -22,22 +22,22 @@ # Defaults to undef # class nova::compute::pci( - $device_specs = $facts['os_service_default'], - $report_in_placement = $facts['os_service_default'], + Array[Hash] $device_specs = [], + $report_in_placement = $facts['os_service_default'], # DEPRECATED PARAMETERS - $passthrough = undef, + Optional[Array[Hash]] $passthrough = undef, ) { include nova::deps if $passthrough != undef { warning('The passthrough parameter is deprecated. Use the device_specs parameter.') - if empty($passthrough) or is_service_default($passthrough) { + if empty($passthrough) { $device_specs_real = $facts['os_service_default'] } else { $device_specs_real = to_array_of_json_strings($passthrough) } } else { - if empty($device_specs) or is_service_default($device_specs) { + if empty($device_specs) { $device_specs_real = $facts['os_service_default'] } else { $device_specs_real = to_array_of_json_strings($device_specs) diff --git a/manifests/pci.pp b/manifests/pci.pp index ea60290cf..2a7bf182f 100644 --- a/manifests/pci.pp +++ b/manifests/pci.pp @@ -6,20 +6,20 @@ # # [*aliases*] # (optional) A list of pci alias hashes -# Defaults to $facts['os_service_default'] +# Defaults to [] # Example: # [{"vendor_id" => "1234", "product_id" => "5678", "name" => "default"}, # {"vendor_id" => "1234", "product_id" => "6789", "name" => "other"}] class nova::pci( - $aliases = $facts['os_service_default'] + Array[Hash] $aliases = [] ) { include nova::deps - if !is_service_default($aliases) and !empty($aliases) { - $aliases_real = to_array_of_json_strings($aliases) - } else { + if empty($aliases) { $aliases_real = $facts['os_service_default'] + } else { + $aliases_real = to_array_of_json_strings($aliases) } nova_config { 'pci/alias': value => $aliases_real; diff --git a/releasenotes/notes/require-hash-instead-of-encoded-json-string-3c016bb1968c2aca.yaml b/releasenotes/notes/require-hash-instead-of-encoded-json-string-3c016bb1968c2aca.yaml new file mode 100644 index 000000000..d4d2f62d7 --- /dev/null +++ b/releasenotes/notes/require-hash-instead-of-encoded-json-string-3c016bb1968c2aca.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + The following parameters no longer accept string values. Use arrays of + hashes instead. + + - ``nova::compute::pci::passthrough`` + - ``nova::pci::aliases`` diff --git a/spec/classes/nova_compute_pci_spec.rb b/spec/classes/nova_compute_pci_spec.rb index f9b0d8930..05cfad3ef 100644 --- a/spec/classes/nova_compute_pci_spec.rb +++ b/spec/classes/nova_compute_pci_spec.rb @@ -45,20 +45,6 @@ describe 'nova::compute::pci' do end end - context 'with device_specs JSON encoded string' do - let :params do - { - :device_specs => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"physical_network\":\"physnet1\"}]", - } - end - - it 'configures nova pci device_spec entries' do - is_expected.to contain_nova_config('pci/device_spec').with( - 'value' => ['{"vendor_id":"8086","product_id":"0126"}','{"vendor_id":"9096","product_id":"1520","physical_network":"physnet1"}'] - ) - end - end - context 'when device_specs is empty' do let :params do { @@ -70,18 +56,6 @@ describe 'nova::compute::pci' do is_expected.to contain_nova_config('pci/device_spec').with(:value => '') end end - - context 'when device_specs is empty string' do - let :params do - { - :device_specs => "" - } - end - - it 'clears pci device_spec configuration' do - is_expected.to contain_nova_config('pci/device_spec').with(:value => '') - end - end end on_supported_os({ diff --git a/spec/classes/nova_pci_spec.rb b/spec/classes/nova_pci_spec.rb index 707adbffa..cb3336258 100644 --- a/spec/classes/nova_pci_spec.rb +++ b/spec/classes/nova_pci_spec.rb @@ -32,19 +32,6 @@ describe 'nova::pci' do end end - context 'with aliases JSON encoded string' do - let :params do - { - :aliases => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\",\"name\":\"graphic_card\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"name\":\"network_card\"}]", - } - end - it 'configures nova pci_alias entries' do - is_expected.to contain_nova_config('pci/alias').with( - 'value' => ['{"vendor_id":"8086","product_id":"0126","name":"graphic_card"}','{"vendor_id":"9096","product_id":"1520","name":"network_card"}'] - ) - end - end - context 'when aliases is empty' do let :params do { @@ -56,18 +43,6 @@ describe 'nova::pci' do is_expected.to contain_nova_config('pci/alias').with(:value => '') end end - - context 'when aliases is empty string' do - let :params do - { - :aliases => "" - } - end - - it 'clears pci_alias configuration' do - is_expected.to contain_nova_config('pci/alias').with(:value => '') - end - end end on_supported_os({ diff --git a/spec/functions/to_array_of_json_strings_spec.rb b/spec/functions/to_array_of_json_strings_spec.rb index 8cc5e258d..b974bfbbc 100644 --- a/spec/functions/to_array_of_json_strings_spec.rb +++ b/spec/functions/to_array_of_json_strings_spec.rb @@ -13,33 +13,23 @@ describe 'to_array_of_json_strings' do is_expected.to run.with_params('arg1', 'arg2').and_raise_error(Puppet::ParseError) end - it 'fails with invalid json string' do - data = 'invalid json' - is_expected.to run.with_params(data).and_raise_error(Puppet::ParseError) - end - - it 'fails with array of json string' do - data = ['{"valid": "json", "syntax": "here"}', '{"some": "data"}'] - is_expected.to run.with_params(data).and_raise_error(Puppet::ParseError) - end - - it 'works with valid json string' do + it 'fails with a formatted json string' do data = '{"valid": "json", "syntax": "here"}' - retval = ['{"valid":"json","syntax":"here"}'] - is_expected.to run.with_params(data).and_return(retval) + is_expected.to run.with_params(data).and_raise_error(Puppet::ParseError) end - it 'fails unless its an array or string' do - is_expected.to run.with_params(1234).and_raise_error(Puppet::ParseError) + it 'fails with a hash' do + data = {:some => "entry"} + is_expected.to run.with_params(data).and_raise_error(Puppet::ParseError) end - it 'fails unless array doesnt have hashes' do - data = [12, 23] + it 'fails unless array does not have hashes' do + data = ['{"valid": "json", "syntax": "here"}'] is_expected.to run.with_params(data).and_raise_error(Puppet::ParseError) end it 'fails if array but only some entries are valid' do - data = [{:some => "entry"}, 23] + data = [{:some => "entry"}, '{"valid": "json", "syntax": "here"}'] is_expected.to run.with_params(data).and_raise_error(Puppet::ParseError) end