diff --git a/lib/puppet/parser/functions/check_array_of_hash.rb b/lib/puppet/parser/functions/check_array_of_hash.rb deleted file mode 100644 index be75db296..000000000 --- a/lib/puppet/parser/functions/check_array_of_hash.rb +++ /dev/null @@ -1,27 +0,0 @@ -require 'json' - -def array_of_hash?(list) - return false unless !list.empty? && list.class == Array - list.each do |e| - return false unless e.class == Hash - end - true -end - -module Puppet::Parser::Functions - newfunction(:check_array_of_hash, :arity =>1, :type => :rvalue, :doc => "Check - input String is a valid Array of Hash in JSON style") do |arg| - if arg[0].class == String - begin - list = JSON.load(arg[0].gsub("'","\"")) - rescue JSON::ParserError - raise Puppet::ParseError, "Syntax error: #{arg[0]} is invalid" - else - return arg[0] if array_of_hash?(list) - end - else - raise Puppet::ParseError, "Syntax error: #{arg[0]} is not a String" - end - return '' - end -end diff --git a/lib/puppet/parser/functions/to_array_of_json_strings.rb b/lib/puppet/parser/functions/to_array_of_json_strings.rb new file mode 100644 index 000000000..3a6cd26aa --- /dev/null +++ b/lib/puppet/parser/functions/to_array_of_json_strings.rb @@ -0,0 +1,38 @@ +require 'json' + +def array_of_hash?(list) + return false unless list.class == Array + list.each do |e| + return false unless e.class == Hash + end + true +end + +module Puppet::Parser::Functions + newfunction(:to_array_of_json_strings, :arity =>1, :type => :rvalue, :doc => "Convert + input array of hashes (optionally JSON encoded) to a puppet Array of JSON encoded Strings") do |arg| + list = arg[0] + if list.class == String + begin + begin + list = JSON.load(list) + rescue JSON::ParserError + # If parsing failed it could be a legacy format that uses single quotes. + # NB This will corrupt valid JSON data, e.g {"foo": "\'"} => {"foo": "\""} + list = JSON.load(list.gsub("'","\"")) + Puppet.warning("#{arg[0]} is not valid JSON. Support for this format is deprecated and may be removed in future.") + end + rescue JSON::ParserError + raise Puppet::ParseError, "Syntax error: #{arg[0]} is not valid" + end + end + unless array_of_hash?(list) + raise Puppet::ParseError, "Syntax error: #{arg[0]} is not an Array or JSON encoded String" + end + rv = [] + list.each do |e| + rv.push(e.to_json) + end + return rv + end +end diff --git a/manifests/api.pp b/manifests/api.pp index ef8238f9c..44356562c 100644 --- a/manifests/api.pp +++ b/manifests/api.pp @@ -75,10 +75,11 @@ # Defaults to undef # # [*pci_alias*] -# (optional) Pci passthrough for controller: -# Defaults to undef -# Example -# "[ {'vendor_id':'1234', 'product_id':'5678', 'name':'default'}, {...} ]" +# (optional) A list of pci alias hashes +# Defaults to $::os_service_default +# Example: +# [{"vendor_id" => "1234", "product_id" => "5678", "name" => "default"}, +# {"vendor_id" => "1234", "product_id" => "6789", "name" => "other"}] # # [*ratelimits*] # (optional) A string that is a semicolon-separated list of 5-tuples. @@ -285,7 +286,7 @@ class nova::api( $db_online_data_migrations = false, $neutron_metadata_proxy_shared_secret = undef, $default_floating_pool = 'nova', - $pci_alias = undef, + $pci_alias = $::os_service_default, $ratelimits = undef, $ratelimits_factory = 'nova.api.openstack.compute.limits:RateLimitingMiddleware.factory', @@ -523,10 +524,13 @@ as a standalone service, or httpd for being run by a httpd server") 'filter:authtoken/auth_admin_prefix': ensure => absent; } - if $pci_alias { - nova_config { - 'DEFAULT/pci_alias': value => join(any2array(check_array_of_hash($pci_alias)), ','); - } + if !is_service_default($pci_alias) and !empty($pci_alias) { + $pci_alias_real = to_array_of_json_strings($pci_alias) + } else { + $pci_alias_real = $::os_service_default + } + nova_config { + 'DEFAULT/pci_alias': value => $pci_alias_real; } if $validate { diff --git a/manifests/compute.pp b/manifests/compute.pp index baa58a8fd..b5f5eee3c 100644 --- a/manifests/compute.pp +++ b/manifests/compute.pp @@ -87,8 +87,8 @@ # (optional) Pci passthrough list of hash. # Defaults to $::os_service_default # Example of format: -# "[ { 'vendor_id':'1234','product_id':'5678' }, -# { 'vendor_id':'4321','product_id':'8765','physical_network':'default' } ] " +# [ { "vendor_id" => "1234","product_id" => "5678" }, +# { "vendor_id" => "4321","product_id" => "8765", "physical_network" => "default" } ] # # [*config_drive_format*] # (optional) Config drive format. One of iso9660 (default) or vfat @@ -234,7 +234,7 @@ is used. It will be removed once Nova removes it.") # the value is computed in a function and it makes things more complex. Let's just check if # a value is set or if it's empty. if !is_service_default($pci_passthrough) and !empty($pci_passthrough) { - $pci_passthrough_real = check_array_of_hash($pci_passthrough) + $pci_passthrough_real = to_array_of_json_strings($pci_passthrough) } else { $pci_passthrough_real = $::os_service_default } diff --git a/releasenotes/notes/pci_params_format-bf62bf47585ad1cc.yaml b/releasenotes/notes/pci_params_format-bf62bf47585ad1cc.yaml new file mode 100644 index 000000000..5ebafa2fe --- /dev/null +++ b/releasenotes/notes/pci_params_format-bf62bf47585ad1cc.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + The nova::api::pci_alias and nova::compute::pci_passthrough params now + accept an array of hashes. A valid JSON encoded array of objects is also + acceptable. +deprecations: + - | + Invalid JSON was previously accepted for the nova::api::pci_alias and + nova::compute::pci_passthrough parameters. This format is now deprecated + and may not be accepted in future. diff --git a/spec/classes/nova_api_spec.rb b/spec/classes/nova_api_spec.rb index ce83fa175..04fa984a0 100644 --- a/spec/classes/nova_api_spec.rb +++ b/spec/classes/nova_api_spec.rb @@ -81,6 +81,7 @@ describe 'nova::api' do is_expected.to contain_nova_config('DEFAULT/enable_instance_password').with('value' => '') is_expected.to contain_nova_config('DEFAULT/password_length').with('value' => '') is_expected.to contain_nova_config('DEFAULT/allow_resize_to_same_host').with('value' => false) + is_expected.to contain_nova_config('DEFAULT/pci_alias').with(:value => '') end it 'unconfigures neutron_metadata proxy' do @@ -125,7 +126,6 @@ describe 'nova::api' do :enable_network_quota => false, :enable_instance_password => true, :password_length => 12, - :pci_alias => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\",\"name\":\"graphic_card\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"name\":\"network_card\"}]", :allow_resize_to_same_host => true, }) end @@ -189,14 +189,56 @@ describe 'nova::api' do is_expected.to contain_nova_config('DEFAULT/password_length').with('value' => '12') is_expected.to contain_nova_config('DEFAULT/allow_resize_to_same_host').with('value' => true) end + end + context 'with pci_alias array' do + before do + params.merge!({ + :pci_alias => [{ + "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('DEFAULT/pci_alias').with( - 'value' => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\",\"name\":\"graphic_card\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"name\":\"network_card\"}]" + 'value' => ['{"vendor_id":"8086","product_id":"0126","name":"graphic_card"}','{"vendor_id":"9096","product_id":"1520","name":"network_card"}'] ) end end + context 'with pci_alias JSON encoded string (deprecated)' do + before do + params.merge!({ + :pci_alias => "[{\"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('DEFAULT/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 pci_alias is empty' do + before do + params.merge!({ + :pci_alias => "" + }) + end + + it 'clears pci_alias configuration' do + is_expected.to contain_nova_config('DEFAULT/pci_alias').with(:value => '') + end + end + context 'while validating the service with default command' do before do params.merge!({ diff --git a/spec/classes/nova_compute_spec.rb b/spec/classes/nova_compute_spec.rb index b2b510bf0..9068dfa29 100644 --- a/spec/classes/nova_compute_spec.rb +++ b/spec/classes/nova_compute_spec.rb @@ -33,6 +33,7 @@ describe 'nova::compute' do it { is_expected.to contain_nova_config('barbican/barbican_endpoint').with_value('') } it { is_expected.to contain_nova_config('barbican/barbican_api_version').with_value('') } it { is_expected.to contain_nova_config('barbican/auth_endpoint').with_value('') } + it { is_expected.to contain_nova_config('DEFAULT/pci_passthrough_whitelist').with(:value => '') } it { is_expected.to_not contain_package('cryptsetup').with( :ensure => 'present' )} @@ -72,7 +73,6 @@ describe 'nova::compute' do :default_schedule_zone => 'az2', :internal_service_availability_zone => 'az_int1', :heal_instance_info_cache_interval => '120', - :pci_passthrough => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"physical_network\":\"physnet1\"}]", :config_drive_format => 'vfat', :resize_confirm_window => '3', :vcpu_pin_set => ['4-12','^8','15'], @@ -137,12 +137,6 @@ describe 'nova::compute' do it { is_expected.to contain_nova_config('DEFAULT/resume_guests_state_on_host_boot').with_value(true) } - it 'configures nova pci_passthrough_whitelist entries' do - is_expected.to contain_nova_config('DEFAULT/pci_passthrough_whitelist').with( - 'value' => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"physical_network\":\"physnet1\"}]" - ) - end - it 'configures nova config_drive_format to vfat' do is_expected.to contain_nova_config('DEFAULT/config_drive_format').with_value('vfat') is_expected.to_not contain_package('genisoimage').with( @@ -151,6 +145,43 @@ describe 'nova::compute' do end end + context 'with pci_passthrough array' do + let :params do + { + :pci_passthrough => [ + { + "vendor_id" => "8086", + "product_id" => "0126" + }, + { + "vendor_id" => "9096", + "product_id" => "1520", + "physical_network" => "physnet1" + } + ] + } + end + + it 'configures nova pci_passthrough_whitelist entries' do + is_expected.to contain_nova_config('DEFAULT/pci_passthrough_whitelist').with( + 'value' => ['{"vendor_id":"8086","product_id":"0126"}','{"vendor_id":"9096","product_id":"1520","physical_network":"physnet1"}'] + ) + end + end + + context 'with pci_passthrough JSON encoded string (deprecated)' do + let :params do + { + :pci_passthrough => "[{\"vendor_id\":\"8086\",\"product_id\":\"0126\"},{\"vendor_id\":\"9096\",\"product_id\":\"1520\",\"physical_network\":\"physnet1\"}]" + } + end + + it 'configures nova pci_passthrough_whitelist entries' do + is_expected.to contain_nova_config('DEFAULT/pci_passthrough_whitelist').with( + 'value' => ['{"vendor_id":"8086","product_id":"0126"}','{"vendor_id":"9096","product_id":"1520","physical_network":"physnet1"}'] + ) + end + end context 'when vcpu_pin_set and pci_passthrough are empty' do let :params do