Merge pull request #1683 from jestabro/config-script-dependency

T4845: add smoketest to detect cycles in config-mode script dependency calls

4 files changed, 87 insertions, 15 deletions

diff --git a/data/config-mode-dependencies.json b/data/config-mode-dependencies.json
new file mode 100644
index 000000000..dd0efda10
--- /dev/null
+++ b/data/config-mode-dependencies.json
@@ -0,0 +1 @@
+{"firewall": {"group_resync": ["nat", "policy-route"]}}
diff --git a/python/vyos/configdep.py b/python/vyos/configdep.py
index e6b82ca93..ca05cb092 100644
--- a/python/vyos/configdep.py
+++ b/python/vyos/configdep.py
@@ -14,11 +14,15 @@
 # along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
+import json
 from inspect import stack
 
 from vyos.util import load_as_module
+from vyos.defaults import directories
+from vyos.configsource import VyOSError
+from vyos import ConfigError
 
-dependents = {}
+dependent_func = {}
 
 def canon_name(name: str) -> str:
     return os.path.splitext(name)[0].replace('-', '_')
@@ -30,9 +34,22 @@ def canon_name_of_path(path: str) -> str:
 def caller_name() -> str:
     return stack()[-1].filename
 
-def run_config_mode_script(script: str, config):
-    from vyos.defaults import directories
+def read_dependency_dict():
+    path = os.path.join(directories['data'],
+                        'config-mode-dependencies.json')
+    with open(path) as f:
+        d = json.load(f)
+    return d
+
+def get_dependency_dict(config):
+    if hasattr(config, 'cached_dependency_dict'):
+        d = getattr(config, 'cached_dependency_dict')
+    else:
+        d = read_dependency_dict()
+        setattr(config, 'cached_dependency_dict', d)
+    return d
 
+def run_config_mode_script(script: str, config):
     path = os.path.join(directories['conf_mode'], script)
     name = canon_name(script)
     mod = load_as_module(name, path)
@@ -46,20 +63,23 @@ def run_config_mode_script(script: str, config):
     except (VyOSError, ConfigError) as e:
         raise ConfigError(repr(e))
 
-def def_closure(script: str, config):
+def def_closure(target: str, config):
+    script = target + '.py'
     def func_impl():
         run_config_mode_script(script, config)
     return func_impl
 
-def set_dependent(target: str, config):
+def set_dependents(case, config):
+    d = get_dependency_dict(config)
     k = canon_name_of_path(caller_name())
-    l = dependents.setdefault(k, [])
-    func = def_closure(target, config)
-    l.append(func)
+    l = dependent_func.setdefault(k, [])
+    for target in d[k][case]:
+        func = def_closure(target, config)
+        l.append(func)
 
 def call_dependents():
     k = canon_name_of_path(caller_name())
-    l = dependents.get(k, [])
+    l = dependent_func.get(k, [])
     while l:
         f = l.pop(0)
         f()
diff --git a/smoketest/scripts/cli/test_dependency_graph.py b/smoketest/scripts/cli/test_dependency_graph.py
new file mode 100755
index 000000000..45a40acc4
--- /dev/null
+++ b/smoketest/scripts/cli/test_dependency_graph.py
@@ -0,0 +1,54 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2022 VyOS maintainers and contributors
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# 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 json
+import unittest
+from graphlib import TopologicalSorter, CycleError
+
+DEP_FILE = '/usr/share/vyos/config-mode-dependencies.json'
+
+def graph_from_dict(d):
+    g = {}
+    for k in list(d):
+        g[k] = set()
+        # add the dependencies for every sub-case; should there be cases
+        # that are mutally exclusive in the future, the graphs will be
+        # distinguished
+        for el in list(d[k]):
+            g[k] |= set(d[k][el])
+    return g
+
+class TestDependencyGraph(unittest.TestCase):
+    def setUp(self):
+        with open(DEP_FILE) as f:
+            dd = json.load(f)
+            self.dependency_graph = graph_from_dict(dd)
+
+    def test_cycles(self):
+        ts = TopologicalSorter(self.dependency_graph)
+        out = None
+        try:
+            # get node iterator
+            order = ts.static_order()
+            # try iteration
+            _ = [*order]
+        except CycleError as e:
+            out = e.args
+
+        self.assertIsNone(out)
+
+if __name__ == '__main__':
+    unittest.main(verbosity=2)
diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py
index 9fee20358..38a332be3 100755
--- a/src/conf_mode/firewall.py
+++ b/src/conf_mode/firewall.py
@@ -26,7 +26,7 @@ from vyos.config import Config
 from vyos.configdict import dict_merge
 from vyos.configdict import node_changed
 from vyos.configdiff import get_config_diff, Diff
-from vyos.configdep import set_dependent, call_dependents
+from vyos.configdep import set_dependents, call_dependents
 # from vyos.configverify import verify_interface_exists
 from vyos.firewall import fqdn_config_parse
 from vyos.firewall import geoip_update
@@ -162,11 +162,8 @@ def get_config(config=None):
 
     firewall['group_resync'] = bool('group' in firewall or node_changed(conf, base + ['group']))
     if firewall['group_resync']:
-        # Update nat as firewall groups were updated
-        set_dependent(nat_conf_script, conf)
-        # Update policy route as firewall groups were updated
-        set_dependent(policy_route_conf_script, conf)
-
+        # Update nat and policy-route as firewall groups were updated
+        set_dependents('group_resync', conf)
 
     if 'config_trap' in firewall and firewall['config_trap'] == 'enable':
         diff = get_config_diff(conf)