(MODULES-3980) Fix ipv4 regex validator

This also updates all ipv4 tests to use the same test data for better
comparability. Closes #676, #679

Fix-Originally-By: Nate Potter <nathaniel.potter@intel.com>

6 files changed, 67 insertions, 52 deletions

diff --git a/spec/aliases/ipv4_spec.rb b/spec/aliases/ipv4_spec.rb
index e767b45..210b4b1 100644
--- a/spec/aliases/ipv4_spec.rb
+++ b/spec/aliases/ipv4_spec.rb
@@ -3,12 +3,7 @@ require 'spec_helper'
 if Puppet.version.to_f >= 4.5
   describe 'test::ipv4', type: :class do
     describe 'accepts ipv4 addresses' do
-      [
-        '224.0.0.0',
-        '255.255.255.255',
-        '0.0.0.0',
-        '192.88.99.0'
-      ].each do |value|
+      SharedData::IPV4_PATTERNS.each do |value|
         describe value.inspect do
           let(:params) {{ value: value }}
           it { is_expected.to compile }
@@ -16,12 +11,7 @@ if Puppet.version.to_f >= 4.5
       end
     end
     describe 'rejects other values' do
-      [
-        'nope',
-        '77',
-        '4.4.4',
-        '2001:0db8:85a3:0000:0000:8a2e:0370:73342001:0db8:85a3:0000:0000:8a2e:0370:7334'
-      ].each do |value|
+      SharedData::IPV4_NEGATIVE_PATTERNS.each do |value|
         describe value.inspect do
           let(:params) {{ value: value }}
           it { is_expected.to compile.and_raise_error(/parameter 'value' expects a match for Stdlib::Compat::Ipv4/) }
diff --git a/spec/functions/is_ipv4_address_spec.rb b/spec/functions/is_ipv4_address_spec.rb
index e39d93b..985260c 100644
--- a/spec/functions/is_ipv4_address_spec.rb
+++ b/spec/functions/is_ipv4_address_spec.rb
@@ -1,15 +1,17 @@
 require 'spec_helper'
 
 describe 'is_ipv4_address' do
-  
+
   it { is_expected.not_to eq(nil) }
   it { is_expected.to run.with_params().and_raise_error(Puppet::ParseError, /wrong number of arguments/i) }
-  it { is_expected.to run.with_params('1.2.3.4').and_return(true) }
-  it { is_expected.to run.with_params('1.2.3.255').and_return(true) }
-  it { is_expected.to run.with_params('1.2.3').and_return(false) }
-  it { is_expected.to run.with_params('1.2.3.4.5').and_return(false) }
-  it { is_expected.to run.with_params('').and_return(false) }
-  it { is_expected.to run.with_params('one').and_return(false) }
+
+  SharedData::IPV4_PATTERNS.each do |value|
+    it { is_expected.to run.with_params(value).and_return(true) }
+  end
+
+  SharedData::IPV4_NEGATIVE_PATTERNS.each do |value|
+    it { is_expected.to run.with_params(value).and_return(false) }
+  end
 
   context 'Checking for deprecation warning', if: Puppet.version.to_f < 4.0 do
     after(:all) do
@@ -19,12 +21,12 @@ describe 'is_ipv4_address' do
     it 'should display a single deprecation' do
       ENV['STDLIB_LOG_DEPRECATIONS'] = "true"
       scope.expects(:warning).with(includes('This method is deprecated'))
-      is_expected.to run.with_params('1.2.3.4').and_return(true)
+      is_expected.to run.with_params(SharedData::IPV4_PATTERNS.first).and_return(true)
     end
     it 'should display no warning for deprecation' do
       ENV['STDLIB_LOG_DEPRECATIONS'] = "false"
       scope.expects(:warning).with(includes('This method is deprecated')).never
-      is_expected.to run.with_params('1.2.3.4').and_return(true)
+      is_expected.to run.with_params(SharedData::IPV4_PATTERNS.first).and_return(true)
     end
   end
 end
diff --git a/spec/functions/validate_ipv4_address_spec.rb b/spec/functions/validate_ipv4_address_spec.rb
index d221793..6e4ca05 100755
--- a/spec/functions/validate_ipv4_address_spec.rb
+++ b/spec/functions/validate_ipv4_address_spec.rb
@@ -15,44 +15,27 @@ describe 'validate_ipv4_address' do
     it 'should display a single deprecation' do
       ENV['STDLIB_LOG_DEPRECATIONS'] = "true"
       scope.expects(:warning).with(includes('This method is deprecated'))
-      is_expected.to run.with_params('1.2.3.4')
+      is_expected.to run.with_params(SharedData::IPV4_PATTERNS.first)
     end
     it 'should display no warning for deprecation' do
       ENV['STDLIB_LOG_DEPRECATIONS'] = "false"
       scope.expects(:warning).with(includes('This method is deprecated')).never
-      is_expected.to run.with_params('1.2.3.4')
+      is_expected.to run.with_params(SharedData::IPV4_PATTERNS.first)
     end
-  end 
+  end
 
-  describe 'valid inputs' do
-    it { is_expected.to run.with_params('0.0.0.0') }
-    it { is_expected.to run.with_params('8.8.8.8') }
-    it { is_expected.to run.with_params('127.0.0.1') }
-    it { is_expected.to run.with_params('10.10.10.10') }
-    it { is_expected.to run.with_params('194.232.104.150') }
-    it { is_expected.to run.with_params('244.24.24.24') }
-    it { is_expected.to run.with_params('255.255.255.255') }
-    it { is_expected.to run.with_params('1.2.3.4', '5.6.7.8') }
-    context 'with netmasks' do
-      it { is_expected.to run.with_params('8.8.8.8/0') }
-      it { is_expected.to run.with_params('8.8.8.8/16') }
-      it { is_expected.to run.with_params('8.8.8.8/32') }
-      it { is_expected.to run.with_params('8.8.8.8/255.255.0.0') }
-    end
+  SharedData::IPV4_PATTERNS.each do |value|
+    it { is_expected.to run.with_params(value) }
+  end
+
+  SharedData::IPV4_NEGATIVE_PATTERNS.each do |value|
+    it { is_expected.to run.with_params(value).and_raise_error(Puppet::ParseError, /is not a valid IPv4/) }
   end
 
   describe 'invalid inputs' do
-    it { is_expected.to run.with_params({}).and_raise_error(Puppet::ParseError, /is not a string/) }
-    it { is_expected.to run.with_params(1).and_raise_error(Puppet::ParseError, /is not a string/) }
-    it { is_expected.to run.with_params(true).and_raise_error(Puppet::ParseError, /is not a string/) }
-    it { is_expected.to run.with_params('one').and_raise_error(Puppet::ParseError, /is not a valid IPv4/) }
-    it { is_expected.to run.with_params('0.0.0').and_raise_error(Puppet::ParseError, /is not a valid IPv4/) }
-    it { is_expected.to run.with_params('0.0.0.256').and_raise_error(Puppet::ParseError, /is not a valid IPv4/) }
-    it { is_expected.to run.with_params('0.0.0.0.0').and_raise_error(Puppet::ParseError, /is not a valid IPv4/) }
-    it { is_expected.to run.with_params('affe::beef').and_raise_error(Puppet::ParseError, /is not a valid IPv4/) }
-    it { is_expected.to run.with_params('1.2.3.4', {}).and_raise_error(Puppet::ParseError, /is not a string/) }
-    it { is_expected.to run.with_params('1.2.3.4', 1).and_raise_error(Puppet::ParseError, /is not a string/) }
-    it { is_expected.to run.with_params('1.2.3.4', true).and_raise_error(Puppet::ParseError, /is not a string/) }
-    it { is_expected.to run.with_params('1.2.3.4', 'one').and_raise_error(Puppet::ParseError, /is not a valid IPv4/) }
+    [ {}, [], 1, true ].each do |invalid|
+      it { is_expected.to run.with_params(invalid).and_raise_error(Puppet::ParseError, /is not a string/) }
+      it { is_expected.to run.with_params(SharedData::IPV4_PATTERNS.first, invalid).and_raise_error(Puppet::ParseError, /is not a string/) }
+    end
   end
 end
diff --git a/spec/spec_helper_local.rb b/spec/spec_helper_local.rb
index 023a862..616490c 100644
--- a/spec/spec_helper_local.rb
+++ b/spec/spec_helper_local.rb
@@ -1,3 +1,5 @@
+# automatically load any shared examples or contexts
+Dir["./spec/support/**/*.rb"].sort.each { |f| require f }
 
 # hack to enable all the expect syntax (like allow_any_instance_of) in rspec-puppet examples
 RSpec::Mocks::Syntax.enable_expect(RSpec::Puppet::ManifestMatchers)
diff --git a/spec/support/shared_data.rb b/spec/support/shared_data.rb
new file mode 100644
index 0000000..ea9b7a0
--- /dev/null
+++ b/spec/support/shared_data.rb
@@ -0,0 +1,38 @@
+module SharedData
+  IPV4_PATTERNS = [
+    '0.0.0.0',
+    '1.2.3.4',
+    '10.10.10.10',
+    '127.0.0.1',
+    '192.88.99.0',
+    '194.232.104.150',
+    '224.0.0.0',
+    '244.24.24.24',
+    '255.255.255.255',
+    '8.8.8.8',
+    '8.8.8.8/0',
+    '8.8.8.8/16',
+    '8.8.8.8/255.255.0.0',
+    '8.8.8.8/32',
+  ]
+  IPV4_NEGATIVE_PATTERNS = [
+    '',
+    '0000',
+    '0.0.0.0.',
+    '0.0.0.0./0.0.0.0.',
+    '0.0.0.0./1',
+    '0.0.0.0.0',
+    '0.0.0.0/0.0.0.0.',
+    '0.0.0.256',
+    '0.0.0',
+    '1.2.3.4.5',
+    '1.2.3',
+    '10.010.10.10',
+    '2001:0db8:85a3:0000:0000:8a2e:0370:73342001:0db8:85a3:0000:0000:8a2e:0370:7334',
+    '4.4.4',
+    '77',
+    '9999.9999.9999.9999',
+    'affe::beef',
+    'nope',
+  ]
+end
diff --git a/types/compat/ipv4.pp b/types/compat/ipv4.pp
index 1d72ebd..a0ba0d6 100644
--- a/types/compat/ipv4.pp
+++ b/types/compat/ipv4.pp
@@ -1,2 +1,2 @@
 # Emulate the validate_ipv4_address and is_ipv4_address functions
-type Stdlib::Compat::Ipv4 = Pattern[/^(\d+)\.(\d+)\.(\d+)\.(\d+)$/]
+type Stdlib::Compat::Ipv4 = Pattern[/^((([0-9](?!\d)|[1-9][0-9](?!\d)|1[0-9]{2}(?!\d)|2[0-4][0-9](?!\d)|25[0-5](?!\d))[.]){3}([0-9](?!\d)|[1-9][0-9](?!\d)|1[0-9]{2}(?!\d)|2[0-4][0-9](?!\d)|25[0-5](?!\d)))(\/((([0-9](?!\d)|[1-9][0-9](?!\d)|1[0-9]{2}(?!\d)|2[0-4][0-9](?!\d)|25[0-5](?!\d))[.]){3}([0-9](?!\d)|[1-9][0-9](?!\d)|1[0-9]{2}(?!\d)|2[0-4][0-9](?!\d)|25[0-5](?!\d))|[0-9]+))?$/]