(MODULES-1141) Fail on sending array of ICMP types

diff --git a/README.markdown b/README.markdown
index b4ba88c99bfe24cfec3b8e3dc233cbf739d901b5..8eea1b021ded5b243e4e7bf0eb5204fae084c127 100644 (file)
--- a/README.markdown
+++ b/README.markdown
@@ -589,7 +589,7 @@ If Puppet is managing the iptables or iptables-persistent packages, and the prov
 
 * `hop_limit`: Hop limiting value for matched packets. Values must match '/^\d+$/'. Requires the `hop_limiting` feature.
 
-* `icmp`: When matching ICMP packets, this indicates the type of ICMP packet to match. A value of 'any' is not supported. To match any type of ICMP packet, the parameter should be omitted or undefined. Requires the `icmp_match` feature.
+* `icmp`: When matching ICMP packets, this indicates the type of ICMP packet to match. A value of 'any' is not supported. To match any type of ICMP packet, the parameter should be omitted or undefined. Passing in an array of values is not supported. You can either create separate rules for each ICMP type, or alternatively look at the firewall_multi module (https://forge.puppetlabs.com/alexharvey/firewall_multi). Requires the `icmp_match` feature.
 
 * `iniface`: Input interface to filter on. Values must match '/^!?\s?[a-zA-Z0-9\-\._\+\:]+$/'.  Requires the `interface_match` feature.  Supports interface alias (eth0:0) and negation.
 
@@ -692,7 +692,7 @@ firewall { '999 this runs last':
 
 * `provider`: The specific backend to use for this firewall resource. You will seldom need to specify this --- Puppet will usually discover the appropriate provider for your platform. Available providers are ip6tables and iptables. See the [Providers](#providers) section above for details about these providers.
 
-* `queue_bypass`: When using a `jump` value of 'NFQUEUE' this boolean will allow packets to bypass `queue_num`. This is useful when the process in userspace may not be listening on `queue_num` all the time. 
+* `queue_bypass`: When using a `jump` value of 'NFQUEUE' this boolean will allow packets to bypass `queue_num`. This is useful when the process in userspace may not be listening on `queue_num` all the time.
 
 * `queue_num`: When using a `jump` value of 'NFQUEUE' this parameter specifies the queue number to send packets to.
 

diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb
index c480beb4a411827d99cba073570e6a28d49c1c97..a1a5ff1b96f4390c0f7a8971f2658e132440cc31 100644 (file)
--- a/lib/puppet/type/firewall.rb
+++ b/lib/puppet/type/firewall.rb
@@ -691,6 +691,8 @@ Puppet::Type.newtype(:firewall) do
 
       A value of "any" is not supported. To achieve this behaviour the
       parameter should simply be omitted or undefined.
+      An array of values is also not supported. To match against multiple ICMP
+      types, please use separate rules for each ICMP type.
     EOS
 
     validate do |value|
@@ -699,6 +701,11 @@ Puppet::Type.newtype(:firewall) do
           "Value 'any' is not valid. This behaviour should be achieved " \
           "by omitting or undefining the ICMP parameter."
       end
+      if value.kind_of?(Array)
+        raise ArgumentError,
+          "Argument must not be an array of values. To match multiple " \
+          "ICMP types, please use separate rules for each ICMP type."
+      end
     end
 
     munge do |value|
@@ -722,6 +729,7 @@ Puppet::Type.newtype(:firewall) do
         self.fail("cannot work out icmp type")
       end
       value
+      
     end
   end
 

diff --git a/spec/unit/puppet/type/firewall_spec.rb b/spec/unit/puppet/type/firewall_spec.rb
index 793c0fa450aa5e97136d5d0666966f062a82d0b7..8ed8921957f8f4487c346e67456d41b486bc2f43 100755 (executable)
--- a/spec/unit/puppet/type/firewall_spec.rb
+++ b/spec/unit/puppet/type/firewall_spec.rb
@@ -345,6 +345,9 @@ describe firewall do
     it 'should fail if icmp type is "any"' do
       expect(lambda { @resource[:icmp] = 'any' }).to raise_error(Puppet::Error)
     end
+    it 'should fail if icmp type is an array' do
+      expect(lambda { @resource[:icmp] = ['0', '8'] }).to raise_error(Puppet::Error)
+    end
 
     it 'should fail if icmp type cannot be mapped to a numeric' do
       expect(lambda { @resource[:icmp] = 'foo' }).to raise_error(Puppet::Error)