From 9a2d24cae9c1f68d686ef4964ba81dabeb6e3122 Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Mon, 24 Oct 2016 14:57:37 +0100 Subject: [PATCH] (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 --- spec/aliases/ipv4_spec.rb | 14 +------ spec/functions/is_ipv4_address_spec.rb | 20 +++++---- spec/functions/validate_ipv4_address_spec.rb | 43 ++++++-------------- spec/spec_helper_local.rb | 2 + spec/support/shared_data.rb | 33 +++++++++++++++ types/compat/ipv4.pp | 2 +- 6 files changed, 62 insertions(+), 52 deletions(-) create mode 100644 spec/support/shared_data.rb diff --git a/spec/aliases/ipv4_spec.rb b/spec/aliases/ipv4_spec.rb index e767b45b8..210b4b1aa 100644 --- a/spec/aliases/ipv4_spec.rb +++ b/spec/aliases/ipv4_spec.rb @@ -3,12 +3,7 @@ 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 @@ 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 e39d93bf8..985260cd1 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 @@ 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 d22179378..6e4ca0546 100755 --- a/spec/functions/validate_ipv4_address_spec.rb +++ b/spec/functions/validate_ipv4_address_spec.rb @@ -15,44 +15,27 @@ 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 023a862ee..616490c6e 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 000000000..362b240fc --- /dev/null +++ b/spec/support/shared_data.rb @@ -0,0 +1,33 @@ +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 = [ + '', + '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 1d72ebd09..6eaae356d 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))[.]?){4})(\/((([0-9](?!\d)|[1-9][0-9](?!\d)|1[0-9]{2}(?!\d)|2[0-4][0-9](?!\d)|25[0-5](?!\d))[.]?){4}|[0-9]+))?$/]