Merge pull request #154 from ckaenzig/fix-params-validation

Fix params validation + specs

manifests/conffile.pp

@@ -56,7 +56,6 @@ define postfix::conffile (
   validate_absolute_path($path)
   if !is_string($source) and !is_array($source) { fail("value for source should be either String type or Array type got ${source}") }
   if !is_string($content) { fail("value for content should be String type; got ${content}") }
-  validate_string($ensure)
   validate_re($ensure, ['present', 'absent', 'directory'],
     "\$ensure must be either 'present', 'absent' or 'directory', got '${ensure}'")
   validate_hash($options)

manifests/config.pp

@@ -29,11 +29,9 @@
 #
 define postfix::config ($value = undef, $ensure = 'present') {
 
-  validate_string($ensure)
   validate_re($ensure, ['present', 'absent', 'blank'],
     "\$ensure must be either 'present', 'absent' or 'blank', got '${ensure}'")
   if ($ensure == 'present') {
-    validate_string($value)
     validate_re($value, '^.+$',
       '$value can not be empty if ensure = present')
   }

manifests/files.pp

@@ -5,6 +5,8 @@ class postfix::files {
   $inet_interfaces     = $postfix::inet_interfaces
   $mail_user           = $postfix::mail_user
   $manage_conffiles    = $postfix::manage_conffiles
+  $maincf_source       = $postfix::maincf_source
+  $mastercf_source     = $postfix::mastercf_source
   $master_smtp         = $postfix::master_smtp
   $master_smtps        = $postfix::master_smtps
   $master_submission   = $postfix::master_submission
@@ -17,6 +19,10 @@ class postfix::files {
   $use_schleuder       = $postfix::use_schleuder
   $use_sympa           = $postfix::use_sympa
 
+  validate_string($mastercf_source)
+  validate_string($master_smtp)
+  validate_string($master_smtps)
+
   File {
     replace => $manage_conffiles,
   }
@@ -45,7 +51,7 @@ class postfix::files {
   }
 
   # Config files
-  if $postfix::mastercf_source {
+  if $mastercf_source {
     $mastercf_content = undef
   } else {
     $mastercf_content = template(
@@ -61,7 +67,7 @@ class postfix::files {
     mode    => '0644',
     owner   => 'root',
     seltype => $postfix::params::seltype,
-    source  => $postfix::mastercf_source,
+    source  => $mastercf_source,
   }
 
   # Config files
@@ -72,7 +78,7 @@ class postfix::files {
     owner   => 'root',
     replace => false,
     seltype => $postfix::params::seltype,
-    source  => $postfix::maincf_source,
+    source  => $maincf_source,
   }
 
   ::postfix::config {

manifests/hash.pp

@@ -31,11 +31,8 @@ define postfix::hash (
   include ::postfix::params
 
   validate_absolute_path($name)
-#  validate_string($source)
-#  validate_string($content)
   if !is_string($source) and !is_array($source) { fail("value for source should be either String type or Array type got ${source}") }
   if !is_string($content) and !is_array($content) { fail("value for source should be either String type or Array type got ${content}") }
-  validate_string($ensure)
   validate_re($ensure, ['present', 'absent'],
     "\$ensure must be either 'present' or 'absent', got '${ensure}'")
 

manifests/init.pp

@@ -118,14 +118,7 @@ class postfix (
 
   validate_string($alias_maps)
   validate_string($inet_interfaces)
-  validate_string($ldap_base)
-  validate_string($ldap_host)
-  validate_string($ldap_options)
   validate_string($mail_user)
-  validate_string($maincf_source)
-  validate_string($mastercf_source)
-  validate_string($master_smtp)
-  validate_string($master_smtps)
   validate_string($mydestination)
   validate_string($mynetworks)
   validate_string($myorigin)

manifests/ldap.pp

@@ -14,6 +14,11 @@
 # include postfix::ldap
 #
 class postfix::ldap {
+
+  validate_string($postfix::ldap_base)
+  validate_string($postfix::ldap_host)
+  validate_string($postfix::ldap_options)
+
   if $::osfamily == 'Debian' {
     package {'postfix-ldap':
       before  => File['/etc/postfix/ldap-aliases.cf'],

manifests/map.pp

@@ -36,11 +36,8 @@ define postfix::map (
   include ::postfix::params
 
   validate_absolute_path($path)
-#  validate_string($source)
-#  validate_string($content)
   if !is_string($source) and !is_array($source) { fail("value for source should be either String type or Array type got ${source}") }
   if !is_string($content) and !is_array($content) { fail("value for source should be either String type or Array type got ${content}") }
-  validate_string($ensure)
   validate_re($ensure, ['present', 'absent'],
     "\$ensure must be either 'present' or 'absent', got '${ensure}'")
 

spec/defines/postfix_conffile_spec.rb

@@ -20,7 +20,7 @@ describe 'postfix::conffile' do
         it 'should fail' do
           expect {
             is_expected.to contain_file('postfix conffile foo')
-          }.to raise_error(Puppet::Error, /\["present"\] is not a string/)
+          }.to raise_error
         end
       end
 

spec/defines/postfix_config_spec.rb

@@ -17,7 +17,7 @@ describe 'postfix::config' do
         it 'should fail' do
           expect {
             is_expected.to contain_augeas("set postfix 'foo'")
-          }.to raise_error(Puppet::Error, /value can not be empty/)
+          }.to raise_error
         end
       end
 
@@ -28,7 +28,7 @@ describe 'postfix::config' do
         it 'should fail' do
           expect {
             is_expected.to contain_augeas("set postfix 'foo'")
-          }.to raise_error(Puppet::Error, /\["bar"\] is not a string/)
+          }.to raise_error
         end
       end
 
@@ -40,7 +40,7 @@ describe 'postfix::config' do
         it 'should fail' do
           expect {
             is_expected.to contain_augeas("set postfix 'foo'")
-          }.to raise_error(Puppet::Error, /\["present"\] is not a string/)
+          }.to raise_error
         end
       end
 

spec/defines/postfix_hash_spec.rb

@@ -20,7 +20,7 @@ describe 'postfix::hash' do
         it 'should fail' do
           expect {
             is_expected.to contain_file('/tmp/foo')
-          }.to raise_error(Puppet::Error, /\["present"\] is not a string/)
+          }.to raise_error
         end
       end
 

spec/defines/postfix_map_spec.rb

@@ -20,7 +20,7 @@ describe 'postfix::map' do
         it 'should fail' do
           expect {
             is_expected.to contain_file('postfix map foo')
-          }.to raise_error(Puppet::Error, /\["present"\] is not a string/)
+          }.to raise_error
         end
       end
 

spec/defines/postfix_transport_spec.rb

@@ -24,7 +24,7 @@ describe 'postfix::transport' do
         it 'should fail' do
           expect {
             is_expected.to contain_augeas('Postfix transport - foo')
-          }.to raise_error(Puppet::Error, /\["bar"\] is not a string/)
+          }.to raise_error
         end
       end
 
@@ -37,7 +37,7 @@ describe 'postfix::transport' do
         it 'should fail' do
           expect {
             is_expected.to contain_augeas('Postfix transport - foo')
-          }.to raise_error(Puppet::Error, /\["baz"\] is not a string/)
+          }.to raise_error
         end
       end
 
@@ -50,7 +50,7 @@ describe 'postfix::transport' do
         it 'should fail' do
           expect {
             is_expected.to contain_augeas('Postfix transport - foo')
-          }.to raise_error(Puppet::Error, /\["baz"\] is not a string/)
+          }.to raise_error
         end
       end
 
@@ -76,7 +76,7 @@ describe 'postfix::transport' do
         it 'should fail' do
           expect {
             is_expected.to contain_augeas('Postfix transport - foo')
-          }.to raise_error(Puppet::Error, /\["baz"\] is not a string/)
+          }.to raise_error
         end
       end
 

spec/defines/postfix_virtual_spec.rb

@@ -32,7 +32,7 @@ describe 'postfix::virtual' do
         it 'should fail' do
           expect {
             is_expected.to contain_augeas('Postfix virtual - foo')
-          }.to raise_error(Puppet::Error, /\["bar"\] is not a string/)
+          }.to raise_error
         end
       end
 
@@ -45,7 +45,7 @@ describe 'postfix::virtual' do
         it 'should fail' do
           expect {
             is_expected.to contain_augeas('Postfix virtual - foo')
-          }.to raise_error(Puppet::Error, /\["baz"\] is not a string/)
+          }.to raise_error
         end
       end
 
@@ -71,7 +71,7 @@ describe 'postfix::virtual' do
         it 'should fail' do
           expect {
             is_expected.to contain_augeas('Postfix virtual - foo')
-          }.to raise_error(Puppet::Error, /\["baz"\] is not a string/)
+          }.to raise_error
         end
       end
 

spec/spec_helper.rb

@@ -24,6 +24,8 @@ require 'pathname'
 dir = Pathname.new(__FILE__).parent
 Puppet[:modulepath] = File.join(dir, 'fixtures', 'modules')
 
+RSpec::Expectations.configuration.on_potential_false_positives = :nothing
+
 # There's no real need to make this version dependent, but it helps find
 # regressions in Puppet
 #