(#10723) Munge hostnames and IPs to IPs with CIDR

Previously when hostnames were used in the source and destination properties
they were being converted to IP address by iptables. This meant that later
comparisons were failing because the property in code (a hostname) and the
'real' property returned by introspection (an ip address) were not matching.

This code using the munge facility will automatically detect and convert
hostnames to IP addresses in the type so the comparison works as expected.

The side-effect is that puppet does the hostname to IP conversion, not
iptables.

diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb
index 4cecb78fe3a60c53ce72d916645d07cc3b515cc0..de3889819ee711ac761d571e89cb66f368f6ea53 100644 (file)
--- a/lib/puppet/type/firewall.rb
+++ b/lib/puppet/type/firewall.rb
@@ -87,6 +87,10 @@ Puppet::Type.newtype(:firewall) do
 
       The source can also be an IPv6 address if your provider supports it.
     EOS
+
+    munge do |value|
+      @resource.host_to_ip(value)
+    end
   end
 
   newproperty(:destination) do
@@ -97,6 +101,10 @@ Puppet::Type.newtype(:firewall) do
 
       The destination can also be an IPv6 address if your provider supports it.
     EOS
+
+    munge do |value|
+      @resource.host_to_ip(value)
+    end
   end
 
   newproperty(:sport, :array_matching => :all) do

diff --git a/lib/puppet/util/firewall.rb b/lib/puppet/util/firewall.rb
index cf7f659393e2902152db1316b4514a3a24383a13..f376fc22c8d288975509c88e96cd65d0197d6016 100644 (file)
--- a/lib/puppet/util/firewall.rb
+++ b/lib/puppet/util/firewall.rb
@@ -1,4 +1,5 @@
 require 'socket'
+require 'resolv'
 require 'puppet/util/ipcidr'
 
 # Util module for puppetlabs-firewall
@@ -67,4 +68,12 @@ module Puppet::Util::Firewall
       Socket.getservbyname(value)
     end
   end
+
+  def host_to_ip(value)
+    begin
+      Puppet::Util::IPCidr.new(value).cidr
+    rescue
+      Puppet::Util::IPCidr.new(Resolv.getaddress(value)).cidr
+    end
+  end
 end

diff --git a/spec/fixtures/iptables/conversion_hash.rb b/spec/fixtures/iptables/conversion_hash.rb
index 6f8ed6d096212d6f91a4d862f79dbd92e24c07f3..98ac27ae53da5c288f3e7d2406a98e82478af75f 100644 (file)
--- a/spec/fixtures/iptables/conversion_hash.rb
+++ b/spec/fixtures/iptables/conversion_hash.rb
@@ -7,20 +7,20 @@
 # which will be used to create a resource.
 ARGS_TO_HASH = {
   'long_rule_1' => {
-    :line => '-A INPUT -s 1.1.1.1 -d 1.1.1.1 -p tcp -m multiport --dports 7061,7062 -m multiport --sports 7061,7062 -m comment --comment "000 allow foo" -j ACCEPT',
+    :line => '-A INPUT -s 1.1.1.1/32 -d 1.1.1.1/32 -p tcp -m multiport --dports 7061,7062 -m multiport --sports 7061,7062 -m comment --comment "000 allow foo" -j ACCEPT',
     :table => 'filter',
     :compare_all => true,
     :params => {
       :action => "accept",
       :chain => "INPUT",
-      :destination => "1.1.1.1",
+      :destination => "1.1.1.1/32",
       :dport => ["7061","7062"],
       :ensure => :present,
-      :line => '-A INPUT -s 1.1.1.1 -d 1.1.1.1 -p tcp -m multiport --dports 7061,7062 -m multiport --sports 7061,7062 -m comment --comment "000 allow foo" -j ACCEPT',
+      :line => '-A INPUT -s 1.1.1.1/32 -d 1.1.1.1/32 -p tcp -m multiport --dports 7061,7062 -m multiport --sports 7061,7062 -m comment --comment "000 allow foo" -j ACCEPT',
       :name => "000 allow foo",
       :proto => "tcp",
       :provider => "iptables",
-      :source => "1.1.1.1",
+      :source => "1.1.1.1/32",
       :sport => ["7061","7062"],
       :table => "filter",
     },
@@ -186,7 +186,7 @@ HASH_TO_ARGS = {
       :sport => ["7061","7062"],
       :table => "filter",
     },
-    :args => ["-t", :filter, "-s", "1.1.1.1", "-d", "1.1.1.1", "-p", :tcp, "-m", "multiport", "--sports", "7061,7062", "-m", "multiport", "--dports", "7061,7062", "-m", "comment", "--comment", "000 allow foo", "-j", "ACCEPT"],
+    :args => ["-t", :filter, "-s", "1.1.1.1/32", "-d", "1.1.1.1/32", "-p", :tcp, "-m", "multiport", "--sports", "7061,7062", "-m", "multiport", "--dports", "7061,7062", "-m", "comment", "--comment", "000 allow foo", "-j", "ACCEPT"],
   },
   'long_rule_2' => {
     :params => {
@@ -201,7 +201,7 @@ HASH_TO_ARGS = {
       :sport => ["7061","7062"],
       :table => "filter",
     },
-    :args => ["-t", :filter, "-s", "1.1.1.1", "-d", "2.10.13.3/24", "-p", :udp, "-m", "multiport", "--sports", "7061,7062", "-m", "multiport", "--dports", "7061", "-m", "comment", "--comment", "700 allow bar", "-j", "my_custom_chain"],
+    :args => ["-t", :filter, "-s", "1.1.1.1/32", "-d", "2.10.13.0/24", "-p", :udp, "-m", "multiport", "--sports", "7061,7062", "-m", "multiport", "--dports", "7061", "-m", "comment", "--comment", "700 allow bar", "-j", "my_custom_chain"],
   },
   'no_action' => {
     :params => {
@@ -258,7 +258,7 @@ HASH_TO_ARGS = {
       :table => 'filter',
       :source => '192.168.0.1'
     },
-    :args => ['-t', :filter, '-s', '192.168.0.1', '-p', :tcp, '-m', 'comment', '--comment', '000 allow from 192.168.0.1, please'],
+    :args => ['-t', :filter, '-s', '192.168.0.1/32', '-p', :tcp, '-m', 'comment', '--comment', '000 allow from 192.168.0.1, please'],
   },
   'port_property' => {
     :params => {

diff --git a/spec/unit/puppet/type/firewall_spec.rb b/spec/unit/puppet/type/firewall_spec.rb
index 03c3a0366afa5d10cfefeff3beea455087a003e9..5c2d8d40db5c8b81f4525c3f4b5400155921c016 100755 (executable)
--- a/spec/unit/puppet/type/firewall_spec.rb
+++ b/spec/unit/puppet/type/firewall_spec.rb
@@ -114,7 +114,7 @@ describe firewall do
     describe addr do
       it "should accept a #{addr} as a string" do
         @resource[addr] = '127.0.0.1'
-        @resource[addr].should == '127.0.0.1'
+        @resource[addr].should == '127.0.0.1/32'
       end
     end
   end

diff --git a/spec/unit/puppet/util/firewall_spec.rb b/spec/unit/puppet/util/firewall_spec.rb
new file mode 100644 (file)
index 0000000..a79e733
--- /dev/null
+++ b/spec/unit/puppet/util/firewall_spec.rb
@@ -0,0 +1,45 @@
+require 'spec_helper'
+
+describe 'Puppet::Util::Firewall' do
+  let(:resource) {
+    type = Puppet::Type.type(:firewall)
+    provider = stub 'provider'
+    provider.stubs(:name).returns(:iptables)
+    Puppet::Type::Firewall.stubs(:defaultprovider).returns(provider)
+    type.new({:name => '000 test foo'})
+  }
+
+  before(:each) { resource }
+
+  describe '#host_to_ip' do
+    subject { resource }
+    specify { subject.host_to_ip('puppetlabs.com').should == '96.126.112.51/32' }
+    specify { subject.host_to_ip('96.126.112.51').should == '96.126.112.51/32' }
+    specify { subject.host_to_ip('96.126.112.51/32').should == '96.126.112.51/32' }
+    specify { subject.host_to_ip('2001:db8:85a3:0:0:8a2e:370:7334').should == '2001:db8:85a3::8a2e:370:7334/128' }
+    specify { subject.host_to_ip('2001:db8:1234::/48').should == '2001:db8:1234::/48' }
+  end
+
+  describe '#icmp_name_to_number' do
+    subject { resource }
+    specify { subject.icmp_name_to_number('echo-reply').should == '0' }
+    specify { subject.icmp_name_to_number('destination-unreachable').should == '3' }
+    specify { subject.icmp_name_to_number('source-quench').should == '4' }
+    specify { subject.icmp_name_to_number('redirect').should == '6' }
+    specify { subject.icmp_name_to_number('echo-request').should == '8' }
+    specify { subject.icmp_name_to_number('router-advertisement').should == '9' }
+    specify { subject.icmp_name_to_number('router-solicitation').should == '10' }
+    specify { subject.icmp_name_to_number('time-exceeded').should == '11' }
+    specify { subject.icmp_name_to_number('parameter-problem').should == '12' }
+    specify { subject.icmp_name_to_number('timestamp-request').should == '13' }
+    specify { subject.icmp_name_to_number('timestamp-reply').should == '14' }
+    specify { subject.icmp_name_to_number('address-mask-request').should == '17' }
+    specify { subject.icmp_name_to_number('address-mask-reply').should == '18' }
+  end
+
+  describe '#string_to_port' do
+    subject { resource }
+    specify { subject.string_to_port('80').should == '80' }
+    specify { subject.string_to_port('http').should == '80' }
+  end
+end

diff --git a/spec/unit/puppet/util/ipcidr_spec.rb b/spec/unit/puppet/util/ipcidr_spec.rb
new file mode 100644 (file)
index 0000000..4e19e86
--- /dev/null
+++ b/spec/unit/puppet/util/ipcidr_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+describe 'Puppet::Util::IPCidr' do
+  describe 'ipv4 address' do
+    before { @ipaddr = Puppet::Util::IPCidr.new('96.126.112.51') }
+    subject { @ipaddr }
+    specify { subject.cidr.should == '96.126.112.51/32' }
+    specify { subject.prefixlen.should == 32 }
+    specify { subject.netmask.should == '255.255.255.255' }
+  end
+
+  describe 'single ipv4 address with cidr' do
+    before { @ipcidr = Puppet::Util::IPCidr.new('96.126.112.51/32') }
+    subject { @ipcidr }
+    specify { subject.cidr.should == '96.126.112.51/32' }
+    specify { subject.prefixlen.should == 32 }
+    specify { subject.netmask.should == '255.255.255.255' }
+  end
+
+  describe 'ipv4 address range with cidr' do
+    before { @ipcidr = Puppet::Util::IPCidr.new('96.126.112.0/24') }
+    subject { @ipcidr }
+    specify { subject.cidr.should == '96.126.112.0/24' }
+    specify { subject.prefixlen.should == 24 }
+    specify { subject.netmask.should == '255.255.255.0' }
+  end
+
+  describe 'ipv6 address' do
+    before { @ipaddr = Puppet::Util::IPCidr.new('2001:db8:85a3:0:0:8a2e:370:7334') }
+    subject { @ipaddr }
+    specify { subject.cidr.should == '2001:db8:85a3::8a2e:370:7334/128' }
+    specify { subject.prefixlen.should == 128 }
+    specify { subject.netmask.should == 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff' }
+  end
+
+  describe 'single ipv6 addr with cidr' do
+    before { @ipaddr = Puppet::Util::IPCidr.new('2001:db8:85a3:0:0:8a2e:370:7334/128') }
+    subject { @ipaddr }
+    specify { subject.cidr.should == '2001:db8:85a3::8a2e:370:7334/128' }
+    specify { subject.prefixlen.should == 128 }
+    specify { subject.netmask.should == 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff' }
+  end
+
+  describe 'ipv6 addr range with cidr' do
+    before { @ipaddr = Puppet::Util::IPCidr.new('2001:db8:1234::/48') }
+    subject { @ipaddr }
+    specify { subject.cidr.should == '2001:db8:1234::/48' }
+    specify { subject.prefixlen.should == 48 }
+    specify { subject.netmask.should == 'ffff:ffff:ffff:0000:0000:0000:0000:0000' }
+  end
+end