]> review.fuel-infra Code Review - puppet-modules/puppetlabs-firewall.git/commitdiff
(MODULES-5645) Choose correct IP version for hostname resolution
authorKevin Peng <kpengboy@ocf.berkeley.edu>
Sun, 24 Sep 2017 07:43:42 +0000 (00:43 -0700)
committerKevin Peng <kpengboy@ocf.berkeley.edu>
Sun, 24 Sep 2017 07:43:42 +0000 (00:43 -0700)
Currently hostnames specified in a `source` or `destination` field
in a firewall rule are always resolved as IPv4, even when the
provider is `ip6tables`. Instead, intelligently determine whether
the hostname should be resolved as an IPv4 address or IPv6 address
based on the provider.

lib/puppet/type/firewall.rb
lib/puppet/util/firewall.rb
lib/puppet/util/ipcidr.rb
spec/unit/puppet/util/firewall_spec.rb

index 981fdb90aa6d6993f2d99c5178a55adb9774c260..55065f41f6d3066a8e247839c2d665a84a447e92 100644 (file)
@@ -134,8 +134,17 @@ Puppet::Type.newtype(:firewall) do
     EOS
 
     munge do |value|
+      case @resource[:provider]
+      when :iptables
+        protocol = :IPv4
+      when :ip6tables
+        protocol = :IPv6
+      else
+        self.fail("cannot work out protocol family")
+      end
+
       begin
-        @resource.host_to_mask(value)
+        @resource.host_to_mask(value, protocol)
       rescue Exception => e
         self.fail("host_to_ip failed for #{value}, exception #{e}")
       end
@@ -184,8 +193,17 @@ Puppet::Type.newtype(:firewall) do
     EOS
 
     munge do |value|
+      case @resource[:provider]
+      when :iptables
+        protocol = :IPv4
+      when :ip6tables
+        protocol = :IPv6
+      else
+        self.fail("cannot work out protocol family")
+      end
+
       begin
-        @resource.host_to_mask(value)
+        @resource.host_to_mask(value, protocol)
       rescue Exception => e
         self.fail("host_to_ip failed for #{value}, exception #{e}")
       end
index cdffaafec78c73265378435095a2b66f37d9beec..5f340595d6ec84614935f84a713af02325e5676d 100644 (file)
@@ -88,7 +88,9 @@ module Puppet::Util::Firewall
     end
   end
 
-  # Takes an address and returns it in CIDR notation.
+  # Takes an address and protocol and returns the address in CIDR notation.
+  #
+  # The protocol is only used when the address is a hostname.
   #
   # If the address is:
   #
@@ -105,27 +107,49 @@ module Puppet::Util::Firewall
   #   - Any address with a resulting prefix length of zero:
   #     It will return nil which is equivilent to not specifying an address
   #
-  def host_to_ip(value)
+  def host_to_ip(value, proto = nil)
     begin
       value = Puppet::Util::IPCidr.new(value)
     rescue
-      value = Puppet::Util::IPCidr.new(Resolv.getaddress(value))
+      family = case proto
+               when :IPv4
+                 Socket::AF_INET
+               when :IPv6
+                 Socket::AF_INET6
+               when nil
+                 raise ArgumentError, "Proto must be specified for a hostname"
+               else
+                 raise ArgumentError, "Unsupported address family: #{proto}"
+               end
+
+      new_value = nil
+      Resolv.each_address(value) do |addr|
+        begin
+          new_value = Puppet::Util::IPCidr.new(addr, family)
+          break
+        rescue
+        end
+      end
+
+      raise "Failed to resolve hostname #{value}" unless new_value != nil
+      value = new_value
     end
 
     return nil if value.prefixlen == 0
     value.cidr
   end
 
-  # Takes an address mask and converts the host portion to CIDR notation.
+  # Takes an address mask and protocol and converts the host portion to CIDR
+  # notation.
   #
   # This takes into account you can negate a mask but follows all rules
   # defined in host_to_ip for the host/address part.
   #
-  def host_to_mask(value)
+  def host_to_mask(value, proto)
     match = value.match /(!)\s?(.*)$/
-    return host_to_ip(value) unless match
+    return host_to_ip(value, proto) unless match
 
-    cidr = host_to_ip(match[2])
+    cidr = host_to_ip(match[2], proto)
     return nil if cidr == nil
     "#{match[1]} #{cidr}"
   end
index 87e8d5e37209a7f992e0688d3c4561e640c2cd18..890be12362b70813b2e991a1edf93e98aead4b9a 100644 (file)
@@ -4,9 +4,9 @@ require 'ipaddr'
 module Puppet
   module Util
     class IPCidr < IPAddr
-      def initialize(ipaddr)
+      def initialize(ipaddr, family = Socket::AF_UNSPEC)
         begin
-          super(ipaddr)
+          super(ipaddr, family)
         rescue ArgumentError => e
           if e.message =~ /invalid address/
             raise ArgumentError, "Invalid address from IPAddr.new: #{ipaddr}"
index a55ed876d81bb04ff38e5dcd24463c6a8f4802c4..0addb6703174052e92dd9fc78791cde340fcf344 100644 (file)
@@ -14,8 +14,9 @@ describe 'Puppet::Util::Firewall' do
   describe '#host_to_ip' do
     subject { resource }
     it {
-      expect(Resolv).to receive(:getaddress).with('puppetlabs.com').and_return('96.126.112.51')
-      expect(subject.host_to_ip('puppetlabs.com')).to eql '96.126.112.51/32'
+      expect(Resolv).to receive(:each_address).at_least(:once).with('puppetlabs.com').and_yield('96.126.112.51').and_yield('2001:DB8:4650::13:8A')
+      expect(subject.host_to_ip('puppetlabs.com', :IPv4)).to eql '96.126.112.51/32'
+      expect(subject.host_to_ip('puppetlabs.com', :IPv6)).to eql '2001:db8:4650::13:8a/128'
     }
     it { expect(subject.host_to_ip('96.126.112.51')).to eql '96.126.112.51/32' }
     it { expect(subject.host_to_ip('96.126.112.51/32')).to eql '96.126.112.51/32' }
@@ -28,22 +29,24 @@ describe 'Puppet::Util::Firewall' do
   describe '#host_to_mask' do
     subject { resource }
     it {
-      expect(Resolv).to receive(:getaddress).at_least(:once).with('puppetlabs.com').and_return('96.126.112.51')
-      expect(subject.host_to_mask('puppetlabs.com')).to eql '96.126.112.51/32'
-      expect(subject.host_to_mask('!puppetlabs.com')).to eql '! 96.126.112.51/32'
+      expect(Resolv).to receive(:each_address).at_least(:once).with('puppetlabs.com').and_yield('96.126.112.51').and_yield('2001:DB8:4650::13:8A')
+      expect(subject.host_to_mask('puppetlabs.com', :IPv4)).to eql '96.126.112.51/32'
+      expect(subject.host_to_mask('!puppetlabs.com', :IPv4)).to eql '! 96.126.112.51/32'
+      expect(subject.host_to_mask('puppetlabs.com', :IPv6)).to eql '2001:db8:4650::13:8a/128'
+      expect(subject.host_to_mask('!puppetlabs.com', :IPv6)).to eql '! 2001:db8:4650::13:8a/128'
     }
-    it { expect(subject.host_to_mask('96.126.112.51')).to eql '96.126.112.51/32' }
-    it { expect(subject.host_to_mask('!96.126.112.51')).to eql '! 96.126.112.51/32' }
-    it { expect(subject.host_to_mask('96.126.112.51/32')).to eql '96.126.112.51/32' }
-    it { expect(subject.host_to_mask('! 96.126.112.51/32')).to eql '! 96.126.112.51/32' }
-    it { expect(subject.host_to_mask('2001:db8:85a3:0:0:8a2e:370:7334')).to eql '2001:db8:85a3::8a2e:370:7334/128' }
-    it { expect(subject.host_to_mask('!2001:db8:85a3:0:0:8a2e:370:7334')).to eql '! 2001:db8:85a3::8a2e:370:7334/128' }
-    it { expect(subject.host_to_mask('2001:db8:1234::/48')).to eql '2001:db8:1234::/48' }
-    it { expect(subject.host_to_mask('! 2001:db8:1234::/48')).to eql '! 2001:db8:1234::/48' }
-    it { expect(subject.host_to_mask('0.0.0.0/0')).to eql nil }
-    it { expect(subject.host_to_mask('!0.0.0.0/0')).to eql nil }
-    it { expect(subject.host_to_mask('::/0')).to eql nil }
-    it { expect(subject.host_to_mask('! ::/0')).to eql nil }
+    it { expect(subject.host_to_mask('96.126.112.51', :IPv4)).to eql '96.126.112.51/32' }
+    it { expect(subject.host_to_mask('!96.126.112.51', :IPv4)).to eql '! 96.126.112.51/32' }
+    it { expect(subject.host_to_mask('96.126.112.51/32', :IPv4)).to eql '96.126.112.51/32' }
+    it { expect(subject.host_to_mask('! 96.126.112.51/32', :IPv4)).to eql '! 96.126.112.51/32' }
+    it { expect(subject.host_to_mask('2001:db8:85a3:0:0:8a2e:370:7334', :IPv6)).to eql '2001:db8:85a3::8a2e:370:7334/128' }
+    it { expect(subject.host_to_mask('!2001:db8:85a3:0:0:8a2e:370:7334', :IPv6)).to eql '! 2001:db8:85a3::8a2e:370:7334/128' }
+    it { expect(subject.host_to_mask('2001:db8:1234::/48', :IPv6)).to eql '2001:db8:1234::/48' }
+    it { expect(subject.host_to_mask('! 2001:db8:1234::/48', :IPv6)).to eql '! 2001:db8:1234::/48' }
+    it { expect(subject.host_to_mask('0.0.0.0/0', :IPv4)).to eql nil }
+    it { expect(subject.host_to_mask('!0.0.0.0/0', :IPv4)).to eql nil }
+    it { expect(subject.host_to_mask('::/0', :IPv6)).to eql nil }
+    it { expect(subject.host_to_mask('! ::/0', :IPv6)).to eql nil }
   end
 
   describe '#icmp_name_to_number' do