Merge pull request voxpupuli#1031 from bastelfreak/puppet4

yastupin · web-flow · commit 81837ec88892 · 2017-03-09T23:57:02.000+01:00
replace validate_* with datatypes
diff --git a/manifests/config.pp b/manifests/config.pp
@@ -178,9 +178,6 @@
     owner  => $daemon_user,
   }
 
-  if ($daemon) {
-    validate_re($daemon, '^(on|off)$')
-  }
 
   file {$proxy_temp_path:
     ensure => directory,
diff --git a/manifests/init.pp b/manifests/init.pp
diff --git a/manifests/resource/geo.pp b/manifests/resource/geo.pp
@@ -53,26 +53,16 @@
 
 
 define nginx::resource::geo (
-  $networks,
-  $default         = undef,
-  $ensure          = 'present',
-  $ranges          = false,
-  $address         = undef,
-  $delete          = undef,
-  $proxies         = undef,
-  $proxy_recursive = undef
+  Hash $networks,
+  Optional[String] $default           = undef,
+  Enum['present', 'absent'] $ensure   = 'present',
+  Boolean $ranges                     = false,
+  Optional[String] $address           = undef,
+  Optional[String] $delete            = undef,
+  Optional[Array] $proxies            = undef,
+  Optional[Boolean] $proxy_recursive  = undef
 ) {
 
-  validate_hash($networks)
-  validate_bool($ranges)
-  validate_re($ensure, '^(present|absent)$',
-    "Invalid ensure value '${ensure}'. Expected 'present' or 'absent'")
-  if ($default != undef) { validate_string($default) }
-  if ($address != undef) { validate_string($address) }
-  if ($delete != undef) { validate_string($delete) }
-  if ($proxies != undef) { validate_array($proxies) }
-  if ($proxy_recursive != undef) { validate_bool($proxy_recursive) }
-
   $root_group = $::nginx::root_group
   $conf_dir   = "${::nginx::conf_dir}/conf.d"
 
diff --git a/manifests/resource/map.pp b/manifests/resource/map.pp
@@ -63,22 +63,14 @@
 
 
 define nginx::resource::map (
-  $string,
-  $mappings,
-  $default    = undef,
-  $ensure     = 'present',
-  $hostnames  = false
+  String $string,
+  Variant[Array, Hash] $mappings,
+  Optional[String] $default         = undef,
+  Enum['absent', 'present'] $ensure = 'present',
+  Boolean $hostnames                = false
 ) {
-  validate_string($string)
   validate_re($string, '^.{2,}$',
     "Invalid string value [${string}]. Expected a minimum of 2 characters.")
-  if ! ( is_array($mappings) or is_hash($mappings) ) {
-    fail("\$mappings must be a hash of the form { 'foo' => 'pool_b' } or array of hashes of form [{ 'key' => 'foo', 'value' => 'pool_b' }, ...]")
-  }
-  validate_bool($hostnames)
-  validate_re($ensure, '^(present|absent)$',
-    "Invalid ensure value '${ensure}'. Expected 'present' or 'absent'")
-  if ($default != undef) { validate_string($default) }
 
   $root_group = $::nginx::root_group
   $conf_dir   = "${::nginx::conf_dir}/conf.d"
diff --git a/manifests/resource/streamhost.pp b/manifests/resource/streamhost.pp
@@ -47,60 +47,25 @@
 #    ensure   => present,
 #  }
 define nginx::resource::streamhost (
-  $ensure                       = 'present',
-  $listen_ip                    = '*',
-  $listen_port                  = 80,
-  $listen_options               = undef,
-  $ipv6_enable                  = false,
-  $ipv6_listen_ip               = '::',
-  $ipv6_listen_port             = 80,
-  $ipv6_listen_options          = 'default ipv6only=on',
-  $proxy                        = undef,
-  $proxy_read_timeout           = $::nginx::proxy_read_timeout,
-  $proxy_connect_timeout        = $::nginx::proxy_connect_timeout,
-  $resolver                     = [],
-  $raw_prepend                  = undef,
-  $raw_append                   = undef,
-  $owner                        = $::nginx::global_owner,
-  $group                        = $::nginx::global_group,
-  $mode                         = $::nginx::global_mode,
+  Enum['absent', 'present'] $ensure       = 'present',
+  Variant[Array, String] $listen_ip       = '*',
+  Integer $listen_port                    = 80,
+  Optional[String] $listen_options        = undef,
+  Boolean $ipv6_enable                    = false,
+  Variant[Array, String] $ipv6_listen_ip  = '::',
+  Integer $ipv6_listen_port               = 80,
+  String $ipv6_listen_options             = 'default ipv6only=on',
+  $proxy                                  = undef,
+  String $proxy_read_timeout              = $::nginx::proxy_read_timeout,
+  $proxy_connect_timeout                  = $::nginx::proxy_connect_timeout,
+  Array $resolver                         = [],
+  $raw_prepend                            = undef,
+  $raw_append                             = undef,
+  String $owner                           = $::nginx::global_owner,
+  String $group                           = $::nginx::global_group,
+  String $mode                            = $::nginx::global_mode,
 ) {
 
-  validate_re($ensure, '^(present|absent)$',
-    "${ensure} is not supported for ensure. Allowed values are 'present' and 'absent'.")
-  if !(is_array($listen_ip) or is_string($listen_ip)) {
-    fail('$listen_ip must be a string or array.')
-  }
-  if is_string($listen_port) {
-    warning('DEPRECATION: String $listen_port must be converted to an integer. Integer string support will be removed in a future release.')
-  }
-  elsif !is_integer($listen_port) {
-    fail('$listen_port must be an integer.')
-  }
-  if ($listen_options != undef) {
-    validate_string($listen_options)
-  }
-  validate_bool($ipv6_enable)
-  if !(is_array($ipv6_listen_ip) or is_string($ipv6_listen_ip)) {
-    fail('$ipv6_listen_ip must be a string or array.')
-  }
-  if is_string($ipv6_listen_port) {
-    warning('DEPRECATION: String $ipv6_listen_port must be converted to an integer. Integer string support will be removed in a future release.')
-  }
-  elsif !is_integer($ipv6_listen_port) {
-    fail('$ipv6_listen_port must be an integer.')
-  }
-  validate_string($ipv6_listen_options)
-
-  validate_string($proxy_read_timeout)
-
-  validate_array($resolver)
-
-  validate_string($owner)
-  validate_string($group)
-  validate_re($mode, '^\d{4}$',
-    "${mode} is not valid. It should be 4 digits (0644 by default).")
-
   # Variables
   if $::nginx::confd_only {
     $streamhost_dir = "${::nginx::conf_dir}/conf.stream.d"
diff --git a/manifests/resource/upstream.pp b/manifests/resource/upstream.pp
@@ -41,30 +41,16 @@
 #    upstream_cfg_prepend => $my_config,
 #  }
 define nginx::resource::upstream (
-  $members               = undef,
-  $members_tag           = undef,
-  $ensure                = 'present',
-  $upstream_cfg_append   = undef,
-  $upstream_cfg_prepend  = undef,
-  $upstream_fail_timeout = '10s',
-  $upstream_max_fails    = undef,
-  $upstream_context      = 'http',
+  Optional[Array] $members                  = undef,
+  $members_tag                              = undef,
+  Enum['present', 'absent'] $ensure         = 'present',
+  Optional[Hash] $upstream_cfg_append       = undef,
+  Optional[Hash] $upstream_cfg_prepend      = undef,
+  $upstream_fail_timeout                    = '10s',
+  $upstream_max_fails                       = undef,
+  Enum['http', 'stream'] $upstream_context  = 'http',
 ) {
 
-  if $members != undef {
-    validate_array($members)
-  }
-  validate_re($ensure, '^(present|absent)$',
-    "${ensure} is not supported for ensure. Allowed values are 'present' and 'absent'.")
-  validate_re($upstream_context, '^(http|stream)$',
-      "${upstream_context} is not supported for upstream_context. Allowed values are 'http' and 'stream'.")
-  if ($upstream_cfg_append != undef) {
-    validate_hash($upstream_cfg_append)
-  }
-  if ($upstream_cfg_prepend != undef) {
-    validate_hash($upstream_cfg_prepend)
-  }
-
   $root_group = $::nginx::root_group
 
   $ensure_real = $ensure ? {
diff --git a/manifests/resource/upstream/member.pp b/manifests/resource/upstream/member.pp
@@ -37,26 +37,11 @@
 define nginx::resource::upstream::member (
   $upstream,
   $server,
-  $ensure                 = 'present',
-  $port                   = 80,
+  Enum['present', 'absent'] $ensure = 'present',
+  Integer $port                     = 80,
   $upstream_fail_timeout  = '10s',
 ) {
 
-  validate_re($ensure, '^(present|absent)$',
-    "${ensure} is not supported for ensure. Allowed values are 'present' and 'absent'.")
-
-  if is_string($port) {
-    warning('DEPRECATION: String $port must be converted to an integer. Integer string support will be removed in a future release.')
-  }
-  elsif !is_integer($port) {
-    fail('$port must be an integer.')
-  }
-
-  $ensure_real = $ensure ? {
-    'absent' => absent,
-    default  => present,
-  }
-
   # Uses: $server, $port, $upstream_fail_timeout
   concat::fragment { "${upstream}_upstream_member_${name}":
     target  => "${::nginx::conf_dir}/conf.d/${upstream}-upstream.conf",
diff --git a/spec/classes/nginx_spec.rb b/spec/classes/nginx_spec.rb
@@ -395,7 +395,7 @@
           {
             title: 'should set worker_processes',
             attr: 'worker_processes',
-            value: '4',
+            value: 4,
             match: 'worker_processes 4;'
           },
           {
@@ -407,7 +407,7 @@
           {
             title: 'should set worker_rlimit_nofile',
             attr: 'worker_rlimit_nofile',
-            value: '10000',
+            value: 10_000,
             match: 'worker_rlimit_nofile 10000;'
           },
           {
@@ -464,7 +464,7 @@
           {
             title: 'should set worker_connections',
             attr: 'worker_connections',
-            value: '100',
+            value: 100,
             match: '  worker_connections 100;'
           },
           {
@@ -503,12 +503,6 @@
             value: 'eventport',
             match: %r{\s*use\s+eventport;}
           },
-          {
-            title: 'should not set events_use',
-            attr: 'events_use',
-            value: false,
-            notmatch: %r{use }
-          },
           {
             title: 'should set access_log',
             attr: 'http_access_log',
@@ -686,36 +680,18 @@
             value: '/path/to/proxy.cache',
             match: %r{\s+proxy_cache_path\s+/path/to/proxy.cache levels=1 keys_zone=d2:100m max_size=500m inactive=20m;}
           },
-          {
-            title: 'should not set proxy_cache_path',
-            attr: 'proxy_cache_path',
-            value: false,
-            notmatch: %r{proxy_cache_path}
-          },
           {
             title: 'should set fastcgi_cache_path',
             attr: 'fastcgi_cache_path',
             value: '/path/to/proxy.cache',
             match: %r{\s*fastcgi_cache_path\s+/path/to/proxy.cache levels=1 keys_zone=d3:100m max_size=500m inactive=20m;}
           },
-          {
-            title: 'should not set fastcgi_cache_path',
-            attr: 'fastcgi_cache_path',
-            value: false,
-            notmatch: %r{fastcgi_cache_path}
-          },
           {
             title: 'should set fastcgi_cache_use_stale',
             attr: 'fastcgi_cache_use_stale',
             value: 'invalid_header',
             match: '  fastcgi_cache_use_stale invalid_header;'
           },
-          {
-            title: 'should not set fastcgi_cache_use_stale',
-            attr: 'fastcgi_cache_use_stale',
-            value: false,
-            notmatch: %r{fastcgi_cache_use_stale}
-          },
           {
             title: 'should contain ordered appended directives from hash',
             attr: 'http_cfg_prepend',
@@ -906,7 +882,13 @@
       end
 
       context 'when proxy_cache_path is /path/to/proxy.cache and loader_files is 1000' do
-        let(:params) { { conf_dir: '/path/to/nginx', proxy_cache_path: '/path/to/proxy.cache', proxy_cache_loader_files: '1000' } }
+        let(:params) do
+          {
+            conf_dir: '/path/to/nginx',
+            proxy_cache_path: '/path/to/proxy.cache',
+            proxy_cache_loader_files: 1000
+          }
+        end
         it { is_expected.to contain_file('/path/to/nginx/nginx.conf').with_content(%r{\s+proxy_cache_path\s+/path/to/proxy.cache levels=1 keys_zone=d2:100m max_size=500m inactive=20m loader_files=1000;}) }
       end
 
@@ -1085,12 +1067,6 @@
         it { is_expected.to contain_file('/etc/nginx/nginx.conf').with_content %r{^user www-data;} }
       end
 
-      context 'when nginx_error_log_severity = invalid' do
-        let(:params) { { nginx_error_log_severity: 'invalid' } }
-
-        it { expect { is_expected.to contain_class('nginx::config') }.to raise_error(Puppet::Error, %r{\$nginx_error_log_severity must be debug, info, notice, warn, error, crit, alert or emerg}) }
-      end
-
       context 'when log_dir is non-default' do
         let(:params) { { log_dir: '/foo/bar' } }
 
diff --git a/spec/defines/resource_map_spec.rb b/spec/defines/resource_map_spec.rb
@@ -104,16 +104,6 @@
 
         it { is_expected.to contain_file("/etc/nginx/conf.d/#{title}-map.conf").with_ensure('absent') }
       end
-
-      context 'when mappings is a string' do
-        let :params do
-          default_params.merge(
-            mappings: 'foo pool_b'
-          )
-        end
-
-        it { is_expected.to raise_error(Puppet::Error, %r[mappings must be a hash of the form { 'foo' => 'pool_b' }]) }
-      end
     end
   end
 end

Original file line number	Diff line number	Diff line change
`@@ -178,9 +178,6 @@`
`178`	`178`	`owner => $daemon_user,`
`179`	`179`	`}`
`180`	`180`
`181`		`- if ($daemon) {`
`182`		`- validate_re($daemon, '^(on\|off)$')`
`183`		`- }`
`184`	`181`
`185`	`182`	`file {$proxy_temp_path:`
`186`	`183`	`ensure => directory,`