From b25070176dacf8b2f57fe9ae7e4fae4a6dbb2288 Mon Sep 17 00:00:00 2001 From: isabelmsft <67024108+isabelmsft@users.noreply.github.com> Date: Tue, 18 Oct 2022 12:13:42 -0700 Subject: [PATCH] YANG Validation for ConfigDB Updates: Fix Decorator Bug (#2405) --- config/validated_config_db_connector.py | 117 +++++++++++--------- tests/config_test.py | 4 +- tests/portchannel_test.py | 4 +- tests/validated_config_db_connector_test.py | 20 +++- 4 files changed, 84 insertions(+), 61 deletions(-) diff --git a/config/validated_config_db_connector.py b/config/validated_config_db_connector.py index b94a2df4a5..b230354ee3 100644 --- a/config/validated_config_db_connector.py +++ b/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) diff --git a/tests/config_test.py b/tests/config_test.py index d2e7b270c0..69febf2c78 100644 --- a/tests/config_test.py +++ b/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 diff --git a/tests/portchannel_test.py b/tests/portchannel_test.py index b35b93d552..4d6eb33ed0 100644 --- a/tests/portchannel_test.py +++ b/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 diff --git a/tests/validated_config_db_connector_test.py b/tests/validated_config_db_connector_test.py index 48d559cd9a..0e1b608366 100644 --- a/tests/validated_config_db_connector_test.py +++ b/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