(#10164) Reject and document icmp => "any"

iptables accepts the string "any" as an ICMP type and stores it behind the
scenes as the fake (IANA reserved) numeric 255. This is functionally
equivalent to not specifying an `--icmp-type` argument.

ip6tables didn't carry this "feature" over. Like many other providers, the
matching of any ICMP packet type is only achieved by omitting the
`--icmpv6-type` arugment.

For the purpose of simpler logic and future provider compatibility we
prevent people from using the value "any" and advise them to omit/undefine
the param instead.

Include a test that somewhat duplicates the prevention of invalid strings
but would preserve this behaviour should icmp_name_to_number() ever change.

diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb
index 4843895c3cfd8f02d4080757b1917b33d10f06fe..37b3762a918a50a6ec4b63344563da331e0c3730 100644 (file)
--- a/lib/puppet/type/firewall.rb
+++ b/lib/puppet/type/firewall.rb
@@ -362,8 +362,19 @@ Puppet::Type.newtype(:firewall) do
   newproperty(:icmp, :required_features => :icmp_match) do
     desc <<-EOS
       When matching ICMP packets, this is the type of ICMP packet to match.
+
+      A value of "any" is not supported. To achieve this behaviour the
+      parameter should simply be omitted or undefined.
     EOS
 
+    validate do |value|
+      if value == "any"
+        raise ArgumentError,
+          "Value 'any' is not valid. This behaviour should be achieved " \
+          "by omitting or undefining the ICMP parameter."
+      end
+    end
+
     munge do |value|
       if value.kind_of?(String)
         value = @resource.icmp_name_to_number(value)

diff --git a/spec/unit/puppet/type/firewall_spec.rb b/spec/unit/puppet/type/firewall_spec.rb
index 4d1eca8e4737326accb0cbb2fbbb58a5dc57b870..6e209ebf83cd7c326fb3a07d5cbf45930d916afe 100755 (executable)
--- a/spec/unit/puppet/type/firewall_spec.rb
+++ b/spec/unit/puppet/type/firewall_spec.rb
@@ -232,7 +232,11 @@ describe firewall do
       @resource[:icmp].should == 9
     end
 
-    it 'should fail if icmp type is not recognized' do
+    it 'should fail if icmp type is "any"' do
+      lambda { @resource[:icmp] = 'any' }.should raise_error(Puppet::Error)
+    end
+
+    it 'should fail if icmp type cannot be mapped to a numeric' do
       lambda { @resource[:icmp] = 'foo' }.should raise_error(Puppet::Error)
     end
   end