Merge "Fix incomplete validation by validate_network_vlan_ranges"

lib/puppet/functions/validate_network_vlan_ranges.rb

@@ -26,19 +26,19 @@ Puppet::Functions.create_function(:validate_network_vlan_ranges) do
     end
 
     value.each do |range|
-      if m = /^(.+:)?(\d+):(\d+)$/.match(range)
+      if m = /^([^:]+):(\d+):(\d+)$/.match(range)
+        # <physical network>:<min>:<max>
         first_id = Integer(m[-2])
         second_id = Integer(m[-1])
-        if (first_id > 4094) || (second_id > 4094)
+        if (first_id < 1) || (second_id > 4094)
-          raise Puppet::Error, "vlan id are invalid."
+          raise Puppet::Error, "invalid vlan ids are used in vlan ranges."
         end
-        if ((second_id - first_id) < 0 )
+        if second_id < first_id
           raise Puppet::Error, "network vlan ranges are invalid."
         end
-      elsif m = /^([^:]+)?(:\d+)?$/.match(range)
+      elsif m = /^([^:]+)$/.match(range)
-        # Either only name of physical network or single vlan id has
+        # Only name of physical network. This is also correct.
-        # been passed. This is also correct.
+      else
-      elsif range
         raise Puppet::Error, "network vlan ranges are invalid."
       end
     end

spec/classes/neutron_plugins_ml2_spec.rb

@@ -33,7 +33,7 @@ describe 'neutron::plugins::ml2' do
       :tenant_network_types  => ['local', 'flat', 'vlan', 'gre', 'vxlan'],
       :mechanism_drivers     => ['openvswitch'],
       :flat_networks         => '*',
-      :network_vlan_ranges   => '10:50',
+      :network_vlan_ranges   => 'datacentre:10:50',
       :tunnel_id_ranges      => '20:100',
       :vxlan_group           => '224.0.0.1',
       :vni_ranges            => '10:100',
@@ -176,7 +176,7 @@ describe 'neutron::plugins::ml2' do
 
     context 'when using vlan driver with valid values' do
       before :each do
-        params.merge!(:network_vlan_ranges => ['1:20', '400:4094'])
+        params.merge!(:network_vlan_ranges => ['net1:1:20', 'net2:400:4094'])
       end
       it 'configures vlan_networks with 1:20 and 400:4094 VLAN ranges' do
         should contain_neutron_plugin_ml2('ml2_type_vlan/network_vlan_ranges').with_value(p[:network_vlan_ranges].join(','))
@@ -185,15 +185,15 @@ describe 'neutron::plugins::ml2' do
 
     context 'when using vlan driver with invalid vlan id' do
       before :each do
-       params.merge!(:network_vlan_ranges => ['1:20', '400:4099'])
+       params.merge!(:network_vlan_ranges => ['net1:1:20', 'net2:400:4099'])
       end
 
-      it { should raise_error(Puppet::Error, /vlan id are invalid./) }
+      it { should raise_error(Puppet::Error, /invalid vlan ids are used in vlan ranges./) }
     end
 
     context 'when using vlan driver with invalid vlan range' do
       before :each do
-        params.merge!(:network_vlan_ranges => ['2938:1'])
+        params.merge!(:network_vlan_ranges => ['net1:2938:1'])
       end
 
       it { should raise_error(Puppet::Error, /vlan ranges are invalid./) }

spec/functions/validate_network_vlan_ranges_spec.rb

@@ -5,39 +5,35 @@ describe 'validate_network_vlan_ranges' do
     is_expected.not_to eq(nil)
   end
 
-  it 'fails with invalid first id max' do
+  context 'with valid values' do
-    is_expected.to run.with_params('4095:4096').and_raise_error(Puppet::Error)
+    [
+      # only physnet
+      'datecentre',
+      # physnet:<min>:<max>
+      'datecentre:1:100',
+      'datecentre:100:100', # min = max should be accepted
+      # array
+      ['datacentre', 'datacentre2:1:100'],
+    ].each do |value|
+      it { is_expected.to run.with_params(value) }
+    end
   end
 
-  it 'fails with valid first id but invalid second id' do
+  context 'with invalid values' do
-    is_expected.to run.with_params('1024:4096').and_raise_error(Puppet::Error)
+    [
-  end
+      '', # empty string
-
+      '1:100', # missing physnet
-  it 'fails with first range valid and second invalid' do
+      'datecentre:1:', # missing max
-    is_expected.to run.with_params(['1024:1050', '4095:4096']).and_raise_error(Puppet::Error)
+      'datecentre::100', # missing min
-  end
+      'datecentre:a:100', # min is not integer
-
+      'datecentre:1:b', # max is not integer
-  it 'fails with invalid vlan range' do
+      'datecentre:1', # not enough fields
-    is_expected.to run.with_params('2048:2000').and_raise_error(Puppet::Error)
+      'datecentre:1:100:1000', # too many fields
-  end
+      'datecentre:1:4095', # max is too large
-
+      'datecentre:0:4094', # min is too small
-  it 'fails with invalid vlan range in array' do
+      'datecentre:101:100', # min > max
-    is_expected.to run.with_params(['2048:2000']).and_raise_error(Puppet::Error)
+    ].each do |value|
-  end
+      it { is_expected.to run.with_params(value).and_raise_error(Puppet::Error) }
-
+    end
-  it 'works with valid vlan range' do
-    is_expected.to run.with_params('1024:1048')
-  end
-
-  it 'works with valid vlan range in array' do
-    is_expected.to run.with_params(['1024:1048', '1050:1060'])
-  end
-
-  it 'works with a physical net name' do
-    is_expected.to run.with_params('physnet1')
-  end
-
-  it 'works with a single vlan' do
-    is_expected.to run.with_params('1024')
   end
 end