From ba1ec6d40ca6cdfdac8f234af18aab7fe1e0dc8b Mon Sep 17 00:00:00 2001 From: Tobias Urdin Date: Sat, 23 Feb 2019 22:57:48 +0100 Subject: [PATCH] Use validate_legacy This changes all the puppet 3 validate_* functions to use the validate_legacy function. The validate_legacy function has been available since about three years but require Puppet >= 4.4.0 and since there is Puppet 4.10.12 as latest we should assume people are running a fairly new Puppet 4 version. This is the first step to then remove all validate function calls and use proper types for parameter as described in spec [1]. [1] https://review.openstack.org/#/c/568929/ Change-Id: I5661c2d685b4bf2422936326db1c3d543a49f92a --- manifests/config.pp | 7 ++++--- manifests/proxy.pp | 6 +++--- manifests/proxy/tempauth.pp | 14 +++++++------- manifests/ringbuilder/create.pp | 3 ++- manifests/ringbuilder/policy_ring.pp | 2 +- manifests/ringbuilder/rebalance.pp | 6 ++++-- manifests/storage/generic.pp | 3 ++- manifests/storage/node.pp | 3 ++- manifests/storage/server.pp | 11 ++++++----- spec/classes/swift_proxy_spec.rb | 2 +- spec/defines/swift_storage_generic_spec.rb | 2 +- spec/defines/swift_storage_node_spec.rb | 2 +- spec/defines/swift_storage_server_spec.rb | 4 ++-- 13 files changed, 36 insertions(+), 29 deletions(-) diff --git a/manifests/config.pp b/manifests/config.pp index b1384754..786031fd 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -35,9 +35,10 @@ class swift::config ( ) { include ::swift::deps - validate_hash($swift_config) - validate_hash($swift_container_sync_realms_config) - validate_hash($swift_proxy_config) + + validate_legacy(Hash, 'validate_hash', $swift_config) + validate_legacy(Hash, 'validate_hash', $swift_container_sync_realms_config) + validate_legacy(Hash, 'validate_hash', $swift_proxy_config) create_resources('swift_config', $swift_config) create_resources('swift_container_sync_realms_config', $swift_container_sync_realms_config) diff --git a/manifests/proxy.pp b/manifests/proxy.pp index 62fb69c9..c3bb0723 100644 --- a/manifests/proxy.pp +++ b/manifests/proxy.pp @@ -170,9 +170,9 @@ class swift::proxy( include ::swift::deps Swift_config<| |> ~> Service['swift-proxy-server'] - validate_bool($account_autocreate) - validate_bool($allow_account_management) - validate_array($pipeline) + validate_legacy(Boolean, 'validate_bool', $account_autocreate) + validate_legacy(Boolean, 'validate_bool', $allow_account_management) + validate_legacy(Array, 'validate_array', $pipeline) if($write_affinity_node_count and ! $write_affinity) { fail('Usage of write_affinity_node_count requires write_affinity to be set') diff --git a/manifests/proxy/tempauth.pp b/manifests/proxy/tempauth.pp index 11f72753..224de9bd 100644 --- a/manifests/proxy/tempauth.pp +++ b/manifests/proxy/tempauth.pp @@ -83,30 +83,30 @@ class swift::proxy::tempauth ( include ::swift::deps - validate_array($account_user_list) + validate_legacy(Array, 'validate_array', $account_user_list) if ($reseller_prefix) { - validate_string($reseller_prefix) + validate_legacy(String, 'validate_string', $reseller_prefix) $reseller_prefix_upcase = upcase($reseller_prefix) } if ($token_life) { - validate_integer($token_life) + validate_legacy(Integer, 'validate_integer', $token_life) } if ($auth_prefix) { - validate_re($auth_prefix,'\/(.*)+\/') + validate_legacy(Pattern[/\/(.*)+\//], 'validate_re', $auth_prefix, ['\/(.*)+\/']) } if ($allow_overrides) { - validate_bool($allow_overrides) + validate_legacy(Boolean, 'validate_bool', $allow_overrides) } if ($storage_url_scheme) { - validate_re($storage_url_scheme, ['http','https','default']) + validate_legacy(Enum['http', 'https', 'default'], 'validate_re', + $storage_url_scheme, [['http', 'https', 'default']]) } - swift_proxy_config { 'filter:tempauth/use': value => 'egg:swift#tempauth'; 'filter:tempauth/reseller_prefix': value => $reseller_prefix_upcase; diff --git a/manifests/ringbuilder/create.pp b/manifests/ringbuilder/create.pp index d959f1a2..6d083dc5 100644 --- a/manifests/ringbuilder/create.pp +++ b/manifests/ringbuilder/create.pp @@ -41,7 +41,8 @@ define swift::ringbuilder::create( include ::swift::deps - validate_re($name, '^object|container|account$') + validate_legacy(Enum['object', 'container', 'account'], 'validate_re', $name, + ['^object|container|account$']) exec { "create_${name}": command => "swift-ring-builder /etc/swift/${name}.builder create ${part_power} ${replicas} ${min_part_hours}", diff --git a/manifests/ringbuilder/policy_ring.pp b/manifests/ringbuilder/policy_ring.pp index c5499692..13223f16 100644 --- a/manifests/ringbuilder/policy_ring.pp +++ b/manifests/ringbuilder/policy_ring.pp @@ -35,8 +35,8 @@ define swift::ringbuilder::policy_ring( $min_part_hours = undef, ) { + validate_legacy(Pattern[/^\d+$/], 'validate_re', $title, ['^\d+$']) - validate_re($title, '^\d+$') include ::swift::deps Class['swift'] -> Swift::Ringbuilder::Policy_ring[$title] diff --git a/manifests/ringbuilder/rebalance.pp b/manifests/ringbuilder/rebalance.pp index 5d525fd8..ecc31f23 100644 --- a/manifests/ringbuilder/rebalance.pp +++ b/manifests/ringbuilder/rebalance.pp @@ -14,9 +14,11 @@ define swift::ringbuilder::rebalance( include ::swift::deps - validate_re($name, '^object|container|account$') + validate_legacy(Enum['object', 'container', 'account'], 'validate_re', $name, + ['^object|container|account$']) + if $seed { - validate_re($seed, '^\d+$') + validate_legacy(Integer, 'validate_re', $seed, ['^\d+$']) } exec { "rebalance_${name}": diff --git a/manifests/storage/generic.pp b/manifests/storage/generic.pp index e2070db1..16725710 100644 --- a/manifests/storage/generic.pp +++ b/manifests/storage/generic.pp @@ -46,7 +46,8 @@ define swift::storage::generic( Swift_config<| |> ~> Service["swift-${name}-auditor"] Swift_config<| |> ~> Service["swift-${name}-replicator"] - validate_re($name, '^object|container|account$') + validate_legacy(Enum['object', 'container', 'account'], 'validate_re', + $name, ['^object|container|account$']) package { "swift-${name}": ensure => $package_ensure, diff --git a/manifests/storage/node.pp b/manifests/storage/node.pp index 0ed25228..1a3f60e2 100644 --- a/manifests/storage/node.pp +++ b/manifests/storage/node.pp @@ -54,7 +54,8 @@ define swift::storage::node( include ::swift::deps - validate_re($zone, '^\d+$', 'The zone parameter must be an integer') + validate_legacy(Integer, 'validate_re', $zone, + ['^\d+$', 'The zone parameter must be an integer']) Swift::Storage::Server { storage_local_net_ip => $storage_local_net_ip, diff --git a/manifests/storage/server.pp b/manifests/storage/server.pp index 8b2aea25..68296727 100644 --- a/manifests/storage/server.pp +++ b/manifests/storage/server.pp @@ -209,11 +209,12 @@ define swift::storage::server( include "::swift::storage::${type}" - validate_re($name, '^\d+$') - validate_re($type, '^object|container|account$') - validate_array($pipeline) - validate_bool($allow_versions) - validate_bool($splice) + validate_legacy(Pattern[/^\d+$/], 'validate_re', $name, ['^\d+$']) + validate_legacy(Enum['object', 'container', 'account'], 'validate_re', + $type, ['^object|container|account$']) + validate_legacy(Array, 'validate_array', $pipeline) + validate_legacy(Boolean, 'validate_bool', $allow_versions) + validate_legacy(Boolean, 'validate_bool', $splice) # TODO - validate that name is an integer $bind_port = $name diff --git a/spec/classes/swift_proxy_spec.rb b/spec/classes/swift_proxy_spec.rb index b0c7d1fa..7c14250f 100644 --- a/spec/classes/swift_proxy_spec.rb +++ b/spec/classes/swift_proxy_spec.rb @@ -230,7 +230,7 @@ describe 'swift::proxy' do [:account_autocreate, :allow_account_management].each do |param| it "should fail when #{param} is not passed a boolean" do params[param] = 'false' - should raise_error(Puppet::Error, /is not a boolean/) + should raise_error(Puppet::Error) end end diff --git a/spec/defines/swift_storage_generic_spec.rb b/spec/defines/swift_storage_generic_spec.rb index 9b0d4765..0f7065d4 100644 --- a/spec/defines/swift_storage_generic_spec.rb +++ b/spec/defines/swift_storage_generic_spec.rb @@ -22,7 +22,7 @@ describe 'swift::storage::generic' do 'foo' end - it_raises 'a Puppet::Error', /does not match/ + it { should raise_error(Puppet::Error) } end %w(account object container).each do |t| diff --git a/spec/defines/swift_storage_node_spec.rb b/spec/defines/swift_storage_node_spec.rb index 2b99ebcb..13820493 100644 --- a/spec/defines/swift_storage_node_spec.rb +++ b/spec/defines/swift_storage_node_spec.rb @@ -32,7 +32,7 @@ describe 'swift::storage::node' do :mnt_base_dir => '/srv/node' } end - it_raises 'a Puppet::Error', /The zone parameter must be an integer/ + it { should raise_error(Puppet::Error) } end describe 'with valid preconditons and policy_index=1 should contain ring devices' do diff --git a/spec/defines/swift_storage_server_spec.rb b/spec/defines/swift_storage_server_spec.rb index 39bcdcb9..2018ef24 100644 --- a/spec/defines/swift_storage_server_spec.rb +++ b/spec/defines/swift_storage_server_spec.rb @@ -36,7 +36,7 @@ describe 'swift::storage::server' do 'foo' end - it_raises 'a Puppet::Error', /does not match/ + it { should raise_error(Puppet::Error) } end ['account', 'object', 'container'].each do |t| @@ -84,7 +84,7 @@ describe 'swift::storage::server' do describe "when pipeline is not passed an array" do let :params do req_params.merge({:pipeline => 'not an array'}) end - it_raises 'a Puppet::Error', /is not an Array/ + it { should raise_error(Puppet::Error) } end describe "when replicator_concurrency is set" do