Merge pull request #1308 from c-po/t4369-openvpn-equuleus

openvpn: T4369: enforce daemon-restart on openvpn-option CLI change (equuleus)

3 files changed, 47 insertions, 3 deletions

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index 1f245f3d2..be10cbdfc 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -104,6 +104,12 @@ def list_diff(first, second):
     second = set(second)
     return [item for item in first if item not in second]
 
+def is_node_changed(conf, path):
+   from vyos.configdiff import get_config_diff
+   D = get_config_diff(conf, key_mangling=('-', '_'))
+   D.set_level(conf.get_level())
+   return D.is_node_changed(path)
+
 def leaf_node_changed(conf, path):
     """
     Check if a leaf node was altered. If it has been altered - values has been
diff --git a/smoketest/scripts/cli/test_interfaces_openvpn.py b/smoketest/scripts/cli/test_interfaces_openvpn.py
index 24df0af4d..55218ecac 100755
--- a/smoketest/scripts/cli/test_interfaces_openvpn.py
+++ b/smoketest/scripts/cli/test_interfaces_openvpn.py
@@ -607,6 +607,44 @@ class TestInterfacesOpenVPN(VyOSUnitTestSHIM.TestCase):
             interface = f'vtun{ii}'
             self.assertNotIn(interface, interfaces())
 
+    def test_openvpn_options(self):
+        # Ensure OpenVPN process restart on openvpn-option CLI node change
+
+        interface = 'vtun5001'
+        path = base_path + [interface]
+
+        self.cli_set(path + ['mode', 'site-to-site'])
+        self.cli_set(path + ['local-address', '10.0.0.2'])
+        self.cli_set(path + ['remote-address', '192.168.0.3'])
+        self.cli_set(path + ['shared-secret-key-file', s2s_key])
+
+        self.cli_commit()
+
+        # Now verify the OpenVPN "raw" option passing. Once an openvpn-option is
+        # added, modified or deleted from the CLI, OpenVPN daemon must be restarted
+        cur_pid = process_named_running('openvpn')
+        self.cli_set(path + ['openvpn-option', '--persist-tun'])
+        self.cli_commit()
+
+        # PID must be different as OpenVPN Must be restarted
+        new_pid = process_named_running('openvpn')
+        self.assertNotEqual(cur_pid, new_pid)
+        cur_pid = new_pid
+
+        self.cli_set(path + ['openvpn-option', '--persist-key'])
+        self.cli_commit()
+
+        # PID must be different as OpenVPN Must be restarted
+        new_pid = process_named_running('openvpn')
+        self.assertNotEqual(cur_pid, new_pid)
+        cur_pid = new_pid
+
+        self.cli_delete(path + ['openvpn-option'])
+        self.cli_commit()
+
+        # PID must be different as OpenVPN Must be restarted
+        new_pid = process_named_running('openvpn')
+        self.assertNotEqual(cur_pid, new_pid)
 
 if __name__ == '__main__':
     # Our SSL certificates need a subject ...
diff --git a/src/conf_mode/interfaces-openvpn.py b/src/conf_mode/interfaces-openvpn.py
index 38ed127ff..f7edddcbf 100755
--- a/src/conf_mode/interfaces-openvpn.py
+++ b/src/conf_mode/interfaces-openvpn.py
@@ -29,7 +29,7 @@ from shutil import rmtree
 
 from vyos.config import Config
 from vyos.configdict import get_interface_dict
-from vyos.configdict import leaf_node_changed
+from vyos.configdict import is_node_changed
 from vyos.configverify import verify_vrf
 from vyos.configverify import verify_bridge_delete
 from vyos.configverify import verify_diffie_hellman_length
@@ -83,8 +83,8 @@ def get_config(config=None):
     openvpn = get_interface_dict(conf, base)
 
     if 'deleted' not in openvpn:
-        tmp = leaf_node_changed(conf, ['openvpn-option'])
-        if tmp: openvpn['restart_required'] = ''
+        if is_node_changed(conf, ['openvpn-option']):
+            openvpn.update({'restart_required': {}})
 
     openvpn['auth_user_pass_file'] = '/run/openvpn/{ifname}.pw'.format(**openvpn)
     return openvpn