Use modern facts and IP address validation

theforeman · Dec 7, 2018 · 5eb0088 · 5eb0088
1 parent d42821f
commit 5eb0088
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 8 deletions.
diff --git a/manifests/proxydns.pp b/manifests/proxydns.pp
@@ -35,18 +35,18 @@
   # puppet fact names are converted from ethX.X and ethX:X to ethX_X
   # so for alias and vlan interfaces we have to modify the name accordingly
   $interface_fact_name = regsubst($interface, '[.:]', '_')
-  $ip = getvar("::ipaddress_${interface_fact_name}")
+  $ip = fact("ipaddress_${interface_fact_name}")
 
-  unless is_ip_address($ip) {
-    fail("Could not get the ip address from fact ipaddress_${interface_fact_name}")
+  assert_type(Stdlib::Compat::Ipv4, $ip) |$expected, $actual| {
+    fail("Could not get a valid IP address from fact ipaddress_${interface_fact_name}: '${ip}' (${actual})")
   }
 
   if $reverse_zone {
     $reverse = $reverse_zone
   } else {
-    $netmask = getvar("::netmask_${interface_fact_name}")
-    unless is_ip_address($netmask) {
-      fail("Could not get the netmask from fact netmask_${interface_fact_name}")
+    $netmask = fact("netmask_${interface_fact_name}")
+    assert_type(Stdlib::Compat::Ipv4, $netmask) |$expected, $actual| {
+      fail("Could not get a valid netmask from fact netmask_${interface_fact_name}: '${netmask}' (${actual})")
     }
     $reverse = foreman_proxy::get_network_in_addr($ip, $netmask)
     assert_type(String[1], $reverse) |$expected, $actual| {

diff --git a/spec/classes/foreman_proxy__proxydns__spec.rb b/spec/classes/foreman_proxy__proxydns__spec.rb
@@ -160,6 +160,18 @@
           end
         end
 
+        context 'with invalid interface' do
+          let :facts do
+            facts
+          end
+
+          let :params do
+            base_params.merge(:interface => 'invalid')
+          end
+
+          it { should raise_error(Puppet::Error, /Could not get a valid IP address from fact ipaddress_invalid: '' \(Undef\)/) }
+        end
+
         context "with dns_reverse value" do
           let :facts do
             facts.merge(
@@ -207,14 +219,14 @@
         context 'with an invalid reverse' do
           context 'missing netmask fact' do
             let :facts do
-              facts
+              facts.merge(ipaddress_invalid: '192.0.2.1')
             end
 
             let :params do
               base_params.merge(:interface => 'invalid')
             end
 
-            it { should raise_error(Puppet::Error, /Could not get the /) }
+            it { should raise_error(Puppet::Error, /Could not get a valid netmask from fact netmask_invalid: '' \(Undef\)/) }
           end
 
           context 'invalid netmask fact' do