YANG Validation for ConfigDB Updates: Fix Decorator Bug (#2405)

Author: isabelmsft

config/validated_config_db_connector.py

@@ -5,59 +5,66 @@
 from generic_config_updater.generic_updater import GenericUpdater, ConfigFormat
 from generic_config_updater.gu_common import EmptyTableError, genericUpdaterLogging
 
-def ValidatedConfigDBConnector(config_db_connector):
-    yang_enabled = device_info.is_yang_config_validation_enabled(config_db_connector) 
-    if yang_enabled:
-        config_db_connector.set_entry = validated_set_entry
-        config_db_connector.delete_table = validated_delete_table
-    return config_db_connector
-
-def make_path_value_jsonpatch_compatible(table, key, value):
-    if type(key) == tuple:
-        path = JsonPointer.from_parts([table, '|'.join(key)]).path
-    else:
-        path = JsonPointer.from_parts([table, key]).path
-    if value == {"NULL" : "NULL"}:
-        value = {}
-    return path, value
-
-def create_gcu_patch(op, table, key=None, value=None):
-    if key:
-        path, value = make_path_value_jsonpatch_compatible(table, key, value)
-    else: 
-        path = "/{}".format(table)
-
-    gcu_json_input = []
-    gcu_json = {"op": "{}".format(op),
-                "path": "{}".format(path)}
-    if op == "add":
-        gcu_json["value"] = value
-
-    gcu_json_input.append(gcu_json)
-    gcu_patch = jsonpatch.JsonPatch(gcu_json_input)
-    return gcu_patch
-
-def validated_delete_table(table):
-    gcu_patch = create_gcu_patch("remove", table)
-    format = ConfigFormat.CONFIGDB.name
-    config_format = ConfigFormat[format.upper()]
-    try:
-        GenericUpdater().apply_patch(patch=gcu_patch, config_format=config_format, verbose=False, dry_run=False, ignore_non_yang_tables=False, ignore_paths=None)
-    except ValueError as e:
-        logger = genericUpdaterLogging.get_logger(title="Patch Applier", print_all_to_console=True)
-        logger.log_notice("Unable to remove entry, as doing so will result in invalid config. Error: {}".format(e))
-
-def validated_set_entry(table, key, value):
-    if value is not None:
-        op = "add"
-    else:
-        op = "remove"
+class ValidatedConfigDBConnector(object):
 
-    gcu_patch = create_gcu_patch(op, table, key, value)
-    format = ConfigFormat.CONFIGDB.name
-    config_format = ConfigFormat[format.upper()]
-
-    try:
-        GenericUpdater().apply_patch(patch=gcu_patch, config_format=config_format, verbose=False, dry_run=False, ignore_non_yang_tables=False, ignore_paths=None)
-    except EmptyTableError:
-        validated_delete_table(table)
+    def __init__(self, config_db_connector):
+        self.connector = config_db_connector
+        self.yang_enabled = device_info.is_yang_config_validation_enabled(self.connector)
+
+    def __getattr__(self, name):
+        if self.yang_enabled:
+            if name == "set_entry":
+                return self.validated_set_entry
+            if name == "delete_table":
+                return self.validated_delete_table
+        return self.connector.__getattribute__(name)
+
+    def make_path_value_jsonpatch_compatible(self, table, key, value):
+        if type(key) == tuple:
+            path = JsonPointer.from_parts([table, '|'.join(key)]).path
+        else:
+            path = JsonPointer.from_parts([table, key]).path
+        if value == {"NULL" : "NULL"}:
+            value = {}
+        return path, value
+
+    def create_gcu_patch(self, op, table, key=None, value=None):
+        if key:
+            path, value = self.make_path_value_jsonpatch_compatible(table, key, value)
+        else: 
+            path = "/{}".format(table)
+
+        gcu_json_input = []
+        gcu_json = {"op": "{}".format(op),
+                    "path": "{}".format(path)}
+        if op == "add":
+            gcu_json["value"] = value
+
+        gcu_json_input.append(gcu_json)
+        gcu_patch = jsonpatch.JsonPatch(gcu_json_input)
+        return gcu_patch
+
+    def validated_delete_table(self, table):
+        gcu_patch = self.create_gcu_patch("remove", table)
+        format = ConfigFormat.CONFIGDB.name
+        config_format = ConfigFormat[format.upper()]
+        try:
+            GenericUpdater().apply_patch(patch=gcu_patch, config_format=config_format, verbose=False, dry_run=False, ignore_non_yang_tables=False, ignore_paths=None)
+        except ValueError as e:
+            logger = genericUpdaterLogging.get_logger(title="Patch Applier", print_all_to_console=True)
+            logger.log_notice("Unable to remove entry, as doing so will result in invalid config. Error: {}".format(e))
+
+    def validated_set_entry(self, table, key, value):
+        if value is not None:
+            op = "add"
+        else:
+            op = "remove"
+    
+        gcu_patch = self.create_gcu_patch(op, table, key, value)
+        format = ConfigFormat.CONFIGDB.name
+        config_format = ConfigFormat[format.upper()]
+
+        try:
+            GenericUpdater().apply_patch(patch=gcu_patch, config_format=config_format, verbose=False, dry_run=False, ignore_non_yang_tables=False, ignore_paths=None)
+        except EmptyTableError:
+            self.validated_delete_table(table)

tests/config_test.py

@@ -1638,7 +1638,7 @@ def setup_class(cls):
         importlib.reload(config.main)
 
     @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
-    @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(side_effect=ValueError))
+    @patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=ValueError))
     def test_add_loopback_with_invalid_name_yang_validation(self):
         config.ADHOC_VALIDATION = False
         runner = CliRunner()
@@ -1687,7 +1687,7 @@ def test_del_nonexistent_loopback_adhoc_validation(self):
         assert result.exit_code != 0
         assert "Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>'" in result.output
 
-    @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(return_value=True))
+    @patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(return_value=True))
     @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
     def test_add_loopback_yang_validation(self):
         config.ADHOC_VALIDATION = False

tests/portchannel_test.py

@@ -19,7 +19,7 @@ def setup_class(cls):
         print("SETUP")
 
     @patch("config.main.is_portchannel_present_in_db", mock.Mock(return_value=False))
-    @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(side_effect=ValueError))
+    @patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=ValueError))
     @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
     def test_add_portchannel_with_invalid_name_yang_validation(self):
         config.ADHOC_VALIDATION = False
@@ -46,7 +46,7 @@ def test_add_portchannel_with_invalid_name_adhoc_validation(self):
         assert result.exit_code != 0
         assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>'" in result.output
 
-    @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(side_effect=JsonPatchConflict))
+    @patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=JsonPatchConflict))
     @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
     def test_delete_nonexistent_portchannel_yang_validation(self):
         config.ADHOC_VALIDATION = False

tests/validated_config_db_connector_test.py

@@ -1,6 +1,7 @@
 import imp
 import os
 import mock
+import jsonpatch
 
 imp.load_source('validated_config_db_connector', \
     os.path.join(os.path.dirname(__file__), '..', 'config', 'validated_config_db_connector.py'))
@@ -9,6 +10,9 @@
 from unittest import TestCase
 from mock import patch
 from generic_config_updater.gu_common import EmptyTableError
+from validated_config_db_connector import ValidatedConfigDBConnector
+from swsscommon.swsscommon import ConfigDBConnector
+
 from utilities_common.db import Db
 
 SAMPLE_TABLE = 'VLAN'
@@ -22,9 +26,21 @@ class TestValidatedConfigDBConnector(TestCase):
         Test Class for validated_config_db_connector.py
 
     '''
-    def test_validated_config_db_connector_empty_table(self): 
+    def test_validated_set_entry_empty_table(self): 
         mock_generic_updater = mock.Mock()
         mock_generic_updater.apply_patch = mock.Mock(side_effect=EmptyTableError)
         with mock.patch('validated_config_db_connector.GenericUpdater', return_value=mock_generic_updater):
-            remove_entry_success = validated_config_db_connector.validated_set_entry(SAMPLE_TABLE, SAMPLE_KEY, SAMPLE_VALUE_EMPTY)
+            remove_entry_success = validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry(mock.Mock(), SAMPLE_TABLE, SAMPLE_KEY, SAMPLE_VALUE_EMPTY)
             assert not remove_entry_success
+
+    def test_validated_delete_table_invalid_delete(self):
+        mock_generic_updater = mock.Mock()
+        mock_generic_updater.apply_patch = mock.Mock(side_effect=ValueError)
+        with mock.patch('validated_config_db_connector.GenericUpdater', return_value=mock_generic_updater):
+            delete_table_success = validated_config_db_connector.ValidatedConfigDBConnector.validated_delete_table(mock.Mock(), SAMPLE_TABLE)
+            assert not delete_table_success
+
+    def test_create_gcu_patch(self):
+        expected_gcu_patch = jsonpatch.JsonPatch([{"op": "add", "path": "/PORTCHANNEL/PortChannel01", "value": "test"}])
+        created_gcu_patch = validated_config_db_connector.ValidatedConfigDBConnector.create_gcu_patch(ValidatedConfigDBConnector(ConfigDBConnector()), "add", "PORTCHANNEL", "PortChannel01", "test")
+        assert expected_gcu_patch == created_gcu_patch