vrf: T31: add support for - and _ in VRF names

(cherry picked from commit 8953a14b01a4183ea890f1c3d569aae54e363614)

3 files changed, 51 insertions, 34 deletions

diff --git a/smoketest/scripts/cli/test_service_ssh.py b/smoketest/scripts/cli/test_service_ssh.py
index eede042de..1e099b0a5 100755
--- a/smoketest/scripts/cli/test_service_ssh.py
+++ b/smoketest/scripts/cli/test_service_ssh.py
@@ -27,7 +27,7 @@ from vyos.util import read_file
 PROCESS_NAME = 'sshd'
 SSHD_CONF = '/run/sshd/sshd_config'
 base_path = ['service', 'ssh']
-vrf = 'ssh-test'
+vrf = 'mgmt'
 
 def get_config_value(key):
     tmp = read_file(SSHD_CONF)
diff --git a/smoketest/scripts/cli/test_vrf.py b/smoketest/scripts/cli/test_vrf.py
index c0f610323..bd8c5ab02 100755
--- a/smoketest/scripts/cli/test_vrf.py
+++ b/smoketest/scripts/cli/test_vrf.py
@@ -14,6 +14,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import re
 import os
 import unittest
 from netifaces import interfaces
@@ -24,39 +25,44 @@ from vyos.util import read_file
 from vyos.validate import is_intf_addr_assigned
 from vyos.ifconfig import Interface
 
+base_path = ['vrf']
+vrfs = ['red', 'green', 'blue', 'foo-bar', 'baz_foo']
+
 class VRFTest(unittest.TestCase):
     def setUp(self):
         self.session = ConfigSession(os.getpid())
-        self._vrfs = ['red', 'green', 'blue']
 
     def tearDown(self):
         # delete all VRFs
-        self.session.delete(['vrf'])
+        self.session.delete(base_path)
         self.session.commit()
-        del self.session
+        for vrf in vrfs:
+            self.assertTrue(vrf not in interfaces())
 
     def test_vrf_table_id(self):
-        table = 1000
-        for vrf in self._vrfs:
-            base = ['vrf', 'name', vrf]
+        table = '1000'
+        for vrf in vrfs:
+            base = base_path + ['name', vrf]
             description = f'VyOS-VRF-{vrf}'
             self.session.set(base + ['description', description])
 
-            if vrf == 'green':
-                self.session.set(base + ['disable'])
-
             # check validate() - a table ID is mandatory
             with self.assertRaises(ConfigSessionError):
                 self.session.commit()
 
-            self.session.set(base + ['table', str(table)])
-            table += 1
+            self.session.set(base + ['table', table])
+            if vrf == 'green':
+                self.session.set(base + ['disable'])
+
+            table = str(int(table) + 1)
 
         # commit changes
         self.session.commit()
 
         # Verify VRF configuration
-        for vrf in self._vrfs:
+        table = '1000'
+        iproute2_config = read_file('/etc/iproute2/rt_tables.d/vyos-vrf.conf')
+        for vrf in vrfs:
             description = f'VyOS-VRF-{vrf}'
             self.assertTrue(vrf in interfaces())
             vrf_if = Interface(vrf)
@@ -68,18 +74,28 @@ class VRFTest(unittest.TestCase):
                 state = 'down'
             self.assertEqual(vrf_if.get_admin_state(), state)
 
+            # Test the iproute2 lookup file, syntax is as follows:
+            #
+            # # id       vrf name         comment
+            # 1000       red              # VyOS-VRF-red
+            # 1001       green            # VyOS-VRF-green
+            #  ...
+            regex = f'{table}\s+{vrf}\s+#\s+{description}'
+            self.assertTrue(re.findall(regex, iproute2_config))
+            table = str(int(table) + 1)
+
     def test_vrf_loopback_ips(self):
-        table = 1000
-        for vrf in self._vrfs:
-            base = ['vrf', 'name', vrf]
+        table = '2000'
+        for vrf in vrfs:
+            base = base_path + ['name', vrf]
             self.session.set(base + ['table', str(table)])
-            table += 1
+            table = str(int(table) + 1)
 
         # commit changes
         self.session.commit()
 
         # Verify VRF configuration
-        for vrf in self._vrfs:
+        for vrf in vrfs:
             self.assertTrue(vrf in interfaces())
             self.assertTrue(is_intf_addr_assigned(vrf, '127.0.0.1'))
             self.assertTrue(is_intf_addr_assigned(vrf, '::1'))
@@ -87,10 +103,10 @@ class VRFTest(unittest.TestCase):
     def test_vrf_table_id_is_unalterable(self):
         # Linux Kernel prohibits the change of a VRF table  on the fly.
         # VRF must be deleted and recreated!
-        table = 666
-        vrf = self._vrfs[0]
-        base = ['vrf', 'name', vrf]
-        self.session.set(base + ['table', str(table)])
+        table = '1000'
+        vrf = vrfs[0]
+        base = base_path + ['name', vrf]
+        self.session.set(base + ['table', table])
 
         # commit changes
         self.session.commit()
@@ -98,12 +114,11 @@ class VRFTest(unittest.TestCase):
         # Check if VRF has been created
         self.assertTrue(vrf in interfaces())
 
-        table += 1
-        self.session.set(base + ['table', str(table)])
+        table = str(int(table) + 1)
+        self.session.set(base + ['table', table])
         # check validate() - table ID can not be altered!
         with self.assertRaises(ConfigSessionError):
             self.session.commit()
 
-
 if __name__ == '__main__':
-    unittest.main(verbosity=2)
+    unittest.main(verbosity=2, failfast=True)
diff --git a/src/conf_mode/vrf.py b/src/conf_mode/vrf.py
index ebad7e246..6c6e219a5 100755
--- a/src/conf_mode/vrf.py
+++ b/src/conf_mode/vrf.py
@@ -74,18 +74,20 @@ def get_config(config=None):
         conf = Config()
 
     base = ['vrf']
-    vrf = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True)
+    vrf = conf.get_config_dict(base, get_first_key=True)
 
     # determine which VRF has been removed
-    tmp = node_changed(conf, base + ['name'])
-    for name in tmp:
-        vrf.update({'vrf_remove' : { name : ''}})
+    for name in node_changed(conf, base + ['name']):
+        if 'vrf_remove' not in vrf:
+            vrf.update({'vrf_remove' : {}})
+
+        vrf['vrf_remove'][name] = {}
         # get VRF bound interfaces
         interfaces = vrf_interfaces(conf, name)
-        if interfaces: vrf.update({'vrf_remove' : { name : {'interfaces' : ''}}})
+        if interfaces: vrf['vrf_remove'][name]['interface'] = interfaces
         # get VRF bound routing instances
         routes = vrf_routing(conf, name)
-        if routes: vrf.update({'vrf_remove' : { name : {'routes' : routes}}})
+        if routes: vrf['vrf_remove'][name]['route'] = routes
 
     return vrf
 
@@ -93,10 +95,10 @@ def verify(vrf):
     # ensure VRF is not assigned to any interface
     if 'vrf_remove' in vrf:
         for name, config in vrf['vrf_remove'].items():
-            if 'interfaces' in config:
+            if 'interface' in config:
                 raise ConfigError(f'Can not remove VRF "{name}", it still has '\
                                   f'member interfaces!')
-            if 'routes' in config:
+            if 'route' in config:
                 raise ConfigError(f'Can not remove VRF "{name}", it still has '\
                                   f'static routes installed!')