Merge pull request #5169 from jestabro/exclusive-mask-config-sync

T8502: Add exclusion mask to config-sync

12 files changed, 229 insertions, 33 deletions

diff --git a/data/config-sync-exclude.json b/data/config-sync-exclude.json
new file mode 100644
index 000000000..dc4cb88e8
--- /dev/null
+++ b/data/config-sync-exclude.json
@@ -0,0 +1,17 @@
+[
+ [
+  "interfaces",
+  "ethernet",
+  "hw-id"
+ ],
+ [
+  "interfaces",
+  "ethernet",
+  "address"
+ ],
+ [
+  "interfaces",
+  "ethernet",
+  "offload"
+ ]
+]
diff --git a/libvyosconfig/Makefile b/libvyosconfig/Makefile
index 26563ef78..14b087fae 100644
--- a/libvyosconfig/Makefile
+++ b/libvyosconfig/Makefile
@@ -42,7 +42,7 @@ all: sharedlib
 PHONY: depends
 depends:
 	sudo sh -c 'eval $$(opam env --root=/opt/opam --set-root) ;\
-		opam pin add vyos1x-config https://github.com/vyos/vyos1x-config.git#e80771d973fc249ab04fd20a6bb2463698194880 -y ; \
+		opam pin add vyos1x-config https://github.com/vyos/vyos1x-config.git#60b546f26d9fd43dbc6b6bfd1a7edc309c835760 -y ; \
 		opam pin add vyconf https://github.com/vyos/vyconf.git#e25b13ae3040d02326f01bf9bedd097795fb3a62 -y'
 
 sharedlib: depends $(BUILDDIR)/libvyosconfig$(EXTDLL)
diff --git a/libvyosconfig/lib/bindings.ml b/libvyosconfig/lib/bindings.ml
index 0759edae2..d35df0e0b 100644
--- a/libvyosconfig/lib/bindings.ml
+++ b/libvyosconfig/lib/bindings.ml
@@ -497,6 +497,7 @@ let subtree_from_partial r_ptr c_ptr i_ptr path =
     let input = Root.get i_ptr in
     let path = split_on_whitespace path in
     try
+        error_message := "";
         let ct_ret = (CD.subtree_from_partial[@alert "-exn"]) rt ct input path in
         Ctypes.Root.create ct_ret
     with
diff --git a/python/vyos/configsession.py b/python/vyos/configsession.py
index 8fee8eca1..f2abd3a5b 100644
--- a/python/vyos/configsession.py
+++ b/python/vyos/configsession.py
@@ -16,6 +16,7 @@
 import os
 import re
 import sys
+import json
 import weakref
 import subprocess
 from tempfile import NamedTemporaryFile
@@ -31,6 +32,9 @@ from vyos.vyconf_session import VyconfSession
 from vyos.base import Warning as Warn
 from vyos.defaults import DEFAULT_COMMIT_CONFIRM_MINUTES
 from vyos.configtree import ConfigTree
+from vyos.configtree import ConfigTreeError
+from vyos.configtree import delete_dict_from_masks
+from vyos.derivedtree import subtree_from_list_of_partial_paths
 
 # type of config file path or configtree
 ConfigObj: TypeAlias = Union[str, ConfigTree]
@@ -302,15 +306,33 @@ class ConfigSession(object):
         except (ValueError, ConfigSessionError) as e:
             raise ConfigSessionError(e)
 
-    def load_section_tree(self, mask: dict, d: dict):
+    def load_section_tree(
+        self, config_tree: ConfigTree, mask_dict: dict, config_dict: dict
+    ):
+        if (
+            not mask_dict
+            or 'inclusive' not in mask_dict
+            or 'exclusive' not in mask_dict
+        ):
+            raise ConfigSessionError(
+                "Missing mask data can damage the config: expected keys 'inclusive' and 'exclusive'"
+            )
         try:
-            if mask:
-                for p in dict_to_paths(mask):
+            mask_in = ConfigTree(internal_string=mask_dict['inclusive'])
+
+            mask_ex_list = json.loads(mask_dict['exclusive'])
+            mask_ex = subtree_from_list_of_partial_paths(config_tree, mask_ex_list)
+
+            delete_dict = delete_dict_from_masks(config_tree, mask_in, mask_ex)
+
+            if delete_dict:
+                for p in dict_to_paths(delete_dict):
                     self.delete(p)
-            if d:
-                for p in dict_to_paths(d):
+
+            if config_dict:
+                for p in dict_to_paths(config_dict):
                     self.set(p)
-        except (ValueError, ConfigSessionError) as e:
+        except (ValueError, ConfigSessionError, ConfigTreeError) as e:
             raise ConfigSessionError(e)
 
     def comment(self, path, value=None):
diff --git a/python/vyos/configtree.py b/python/vyos/configtree.py
index d92137c14..47e56bc46 100644
--- a/python/vyos/configtree.py
+++ b/python/vyos/configtree.py
@@ -667,6 +667,28 @@ def mask_exclusive(left, right, libpath=LIBPATH):
     return tree
 
 
+def delete_tree_from_masks(
+    config_tree: ConfigTree, include_mask: ConfigTree, exclude_mask: ConfigTree
+):
+    masked_inc = mask_inclusive(config_tree, include_mask)
+    # Here we want the reversed stand-alone exclusion/inclusion.
+    # This simplifies definition of delete paths as (delete)
+    # difference between the two trees of config data.
+    masked_upper_bound = mask_exclusive(config_tree, include_mask)
+    masked_lower_bound = mask_inclusive(config_tree, exclude_mask)
+    masked_exc = union(masked_upper_bound, masked_lower_bound)
+
+    ret = DiffTree(masked_inc, masked_exc)
+    return ret.delete
+
+
+def delete_dict_from_masks(
+    config_tree: ConfigTree, include_mask: ConfigTree, exclude_mask: ConfigTree
+):
+    ret = delete_tree_from_masks(config_tree, include_mask, exclude_mask)
+    return json.loads(ret.to_json())
+
+
 def subtree_from_partial(
     config_tree: ConfigTree,
     path: list[str],
diff --git a/python/vyos/defaults.py b/python/vyos/defaults.py
index 78721c0d2..22aa1f62a 100644
--- a/python/vyos/defaults.py
+++ b/python/vyos/defaults.py
@@ -106,3 +106,7 @@ reference_tree_cache = '/usr/share/vyos/reftree.cache'
 activation_list = os.path.join(directories['config'], 'activation-list')
 activation_init = os.path.join(directories['data'], 'activation-init')
 activation_hint = os.path.join(directories['data'], '.activation_hint')
+
+config_sync_exclusion_list = os.path.join(
+    directories['data'], 'config-sync-exclude.json'
+)
diff --git a/python/vyos/derivedtree.py b/python/vyos/derivedtree.py
index 0a57921b2..dc46b8eb3 100644
--- a/python/vyos/derivedtree.py
+++ b/python/vyos/derivedtree.py
@@ -25,7 +25,10 @@ class DerivedTreeError(Exception):
 
 
 def subtree_from_list_of_partial_paths(
-    ctree: ConfigTree, paths: list[list[str]], accumulator: ConfigTree = None
+    ctree: ConfigTree,
+    paths: list[list[str]],
+    accumulator: ConfigTree = None,
+    reference_tree: ReferenceTree = None,
 ):
     """Return the union of subtrees of the ConfigTree argument matching each
     of the 'partial' paths. A partial path is one that may or may not
@@ -33,19 +36,23 @@ def subtree_from_list_of_partial_paths(
     values that apply.
 
     An existing subtree may be passed as the initial value of accumulator.
+
+    For testing or use outside of the canonical environment, an instance of
+    the ReferenceTree may be passed from an alternative cache location.
     """
-    if accumulator:
+    if reference_tree is None:
+        reference_tree = ReferenceTree()
+
+    if accumulator is not None:
         if not isinstance(accumulator, ConfigTree):
             raise TypeError("Argument 'accumulator' must be an instance of ConfigTree")
     else:
         accumulator = ConfigTree('')
 
-    rtree = ReferenceTree()
-
     errors = []
     for path in paths:
         try:
-            accumulator = subtree_from_partial(ctree, path, rtree, accumulator)
+            accumulator = subtree_from_partial(ctree, path, reference_tree, accumulator)
         except ConfigTreeError as e:
             errors.append(str(e))
             continue
diff --git a/smoketest/scripts/cli/test_service_config-sync.py b/smoketest/scripts/cli/test_service_config-sync.py
index 8e22800c0..926c08435 100644
--- a/smoketest/scripts/cli/test_service_config-sync.py
+++ b/smoketest/scripts/cli/test_service_config-sync.py
@@ -125,6 +125,8 @@ class TestConfigSyncWithHTTPS(VyOSUnitTestSHIM.TestCase):
         self.cli_set(['system', 'time-zone', 'UTC'])
         self.cli_commit()
 
+        time.sleep(2)
+
         output = self.op_mode(
             [
                 'show',
diff --git a/src/helpers/vyos_config_sync.py b/src/helpers/vyos_config_sync.py
index f0650c5fc..9e425d7c4 100755
--- a/src/helpers/vyos_config_sync.py
+++ b/src/helpers/vyos_config_sync.py
@@ -17,6 +17,7 @@
 #
 
 import os
+import sys
 import json
 import requests
 import urllib3
@@ -26,6 +27,9 @@ from typing import Optional, List, Tuple, Dict, Any
 from vyos.config import Config
 from vyos.configtree import ConfigTree
 from vyos.configtree import mask_inclusive
+from vyos.configtree import mask_exclusive
+from vyos.defaults import config_sync_exclusion_list
+from vyos.derivedtree import subtree_from_list_of_partial_paths
 from vyos.template import bracketize_ipv6
 
 
@@ -68,8 +72,9 @@ def post_request(
     return response
 
 
-
-def retrieve_config(sections: List[list[str]]) -> Tuple[Dict[str, Any], Dict[str, Any]]:
+def retrieve_config(
+    sections: List[list[str]], exclusions: List[list[str]]
+) -> Tuple[Dict[str, Any], Dict[str, Any]]:
     """Retrieves the configuration from the local server.
 
     Args:
@@ -82,18 +87,32 @@ def retrieve_config(sections: List[list[str]]) -> Tuple[Dict[str, Any], Dict[str
             - config: The subtree of masked config data, as a dictionary.
     """
 
-    mask = ConfigTree('')
-    for section in sections:
-        mask.set(section)
-    mask_dict = json.loads(mask.to_json())
-
     config = Config()
     config_tree = config.get_config_tree()
-    masked = mask_inclusive(config_tree, mask)
+
+    # set inclusion mask
+    mask_in = ConfigTree('')
+    for section in sections:
+        mask_in.set(section)
+    mask_in_str = mask_in.write_internal_string()
+
+    ## set exclusion mask
+    # pass global settings, read at startup:
+    exclude_list = exclusions
+    # read local settings from Config
+    # ... exclude_list += ...
+    mask_ex = subtree_from_list_of_partial_paths(config_tree, exclude_list)
+    mask_ex_str = json.dumps(exclude_list)
+
+    masked = mask_inclusive(config_tree, mask_in)
+    masked = mask_exclusive(masked, mask_ex)
+
+    mask_dict = {'inclusive': mask_in_str, 'exclusive': mask_ex_str}
     config_dict = json.loads(masked.to_json())
 
     return mask_dict, config_dict
 
+
 def set_remote_config(
         address: str,
         key: str,
@@ -147,14 +166,19 @@ def is_section_revised(section: List[str]) -> bool:
     return is_node_revised(section)
 
 
-def config_sync(secondary_address: str,
-                secondary_key: str,
-                sections: List[list[str]],
-                mode: str,
-                secondary_port: int):
+def config_sync(
+    secondary_address: str,
+    secondary_key: str,
+    sections: List[list[str]],
+    mode: str,
+    secondary_port: int,
+    exclusions: List[list[str]],
+):
     """Retrieve a config section from primary router in JSON format and send it to
        secondary router
     """
+    # pylint: disable=too-many-arguments
+
     if not any(map(is_section_revised, sections)):
         return
 
@@ -163,7 +187,7 @@ def config_sync(secondary_address: str,
     )
 
     # Sync sections ("nat", "firewall", etc)
-    mask_dict, config_dict = retrieve_config(sections)
+    mask_dict, config_dict = retrieve_config(sections, exclusions)
     logger.debug(
         f"Retrieved config for sections '{sections}': {config_dict}")
 
@@ -181,7 +205,19 @@ if __name__ == '__main__':
     # Read configuration from file
     if not os.path.exists(CONFIG_FILE):
         logger.error(f"Post-commit: No config file '{CONFIG_FILE}' exists")
-        exit(0)
+        sys.exit()
+
+    try:
+        with open(config_sync_exclusion_list) as f:
+            exclude_list = json.load(f)
+    except FileNotFoundError:
+        logger.error(f"Exclusion list '{config_sync_exclusion_list}' not found")
+        sys.exit()
+    except (json.JSONDecodeError, OSError) as e:
+        logger.error(
+            f"Failed to load config-sync exclusion list '{config_sync_exclusion_list}': {e}"
+        )
+        sys.exit()
 
     with open(CONFIG_FILE, 'r') as f:
         config_data = f.read()
@@ -198,7 +234,7 @@ if __name__ == '__main__':
 
     if not all([mode, secondary_address, secondary_key, sections]):
         logger.error("Missing required configuration data for config synchronization.")
-        exit(0)
+        sys.exit()
 
     # Generate list_sections of sections/subsections
     # [
@@ -212,4 +248,11 @@ if __name__ == '__main__':
         else:
             list_sections.append([section])
 
-    config_sync(secondary_address, secondary_key, list_sections, mode, secondary_port)
+    config_sync(
+        secondary_address,
+        secondary_key,
+        list_sections,
+        mode,
+        secondary_port,
+        exclude_list,
+    )
diff --git a/src/services/api/rest/routers.py b/src/services/api/rest/routers.py
index 058bcc20d..fe67d4612 100644
--- a/src/services/api/rest/routers.py
+++ b/src/services/api/rest/routers.py
@@ -421,8 +421,8 @@ def _execute_configure_op(
                 section = c.section
 
             elif isinstance(c, BaseConfigSectionTreeModel):
-                mask = c.mask
-                config = c.config
+                mask_dict = c.mask
+                config_dict = c.config
 
             if isinstance(c, BaseConfigureModel):
                 if op == 'set':
@@ -448,9 +448,10 @@ def _execute_configure_op(
 
             elif isinstance(c, BaseConfigSectionTreeModel):
                 if op == 'set':
-                    session.set_section_tree(config)
+                    session.set_section_tree(config_dict)
                 elif op == 'load':
-                    session.load_section_tree(mask, config)
+                    config_tree = config.get_config_tree()
+                    session.load_section_tree(config_tree, mask_dict, config_dict)
                 else:
                     raise op_error
         # end for
diff --git a/src/tests/test_config_tree.py b/src/tests/test_config_tree.py
new file mode 100644
index 000000000..d6339c570
--- /dev/null
+++ b/src/tests/test_config_tree.py
@@ -0,0 +1,48 @@
+# Copyright (C) VyOS Inc.
+#
+# 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 unittest import TestCase
+
+from vyos.configtree import ConfigTree
+from vyos.referencetree import ReferenceTree
+from vyos.derivedtree import subtree_from_list_of_partial_paths
+
+
+class TestInitialSetup(TestCase):
+    def setUp(self):
+        with open('data/config.boot.default') as f:
+            config_str = f.read()
+            self.ct = ConfigTree(config_str)
+
+    def test_subtree_from_partial(self):
+        reftree = ReferenceTree(cache_file='data/reftree.cache')
+
+        # workaround since configtree.list_nodes does not take an empty path
+        d = json.loads(self.ct.to_json())
+        top_nodes = list(d)
+        paths = [s.split() for s in top_nodes]
+
+        reassemble = subtree_from_list_of_partial_paths(
+            self.ct, paths, reference_tree=reftree
+        )
+
+        self.assertEqual(self.ct, reassemble)
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/src/utils/add-config-sync-exclude-paths.py b/src/utils/add-config-sync-exclude-paths.py
new file mode 100755
index 000000000..54a209490
--- /dev/null
+++ b/src/utils/add-config-sync-exclude-paths.py
@@ -0,0 +1,29 @@
+#!/usr/bin/env python3
+
+import sys
+import json
+import argparse
+
+parser = argparse.ArgumentParser()
+parser.add_argument('--file', default='data/config-sync-exclude.json')
+parser.add_argument('--paths', nargs='+')
+
+args = parser.parse_args()
+paths = args.paths
+file = args.file
+
+try:
+    with open(file) as f:
+        exclude_str = json.load(f)
+except FileNotFoundError:
+    print(f'Adding new file: {file}')
+    exclude_str = []
+except json.JSONDecodeError as e:
+    sys.exit(e)
+
+for path in (paths or []):
+    exclude_str.append(path.split())
+
+with open(file, 'w') as f:
+    json.dump(exclude_str, f, indent=1)
+    f.write('\n')