diff --git a/generic_config_updater/generic_updater.py b/generic_config_updater/generic_updater.py index be2ddb0091..aa41853204 100644 --- a/generic_config_updater/generic_updater.py +++ b/generic_config_updater/generic_updater.py @@ -47,6 +47,10 @@ def apply(self, patch): # Generate target config self.logger.log_notice("Simulating the target full config after applying the patch.") target_config = self.patch_wrapper.simulate_patch(patch, old_config) + + # Validate all JsonPatch operations on specified fields + self.logger.log_notice("Validating all JsonPatch operations are permitted on the specified fields") + self.config_wrapper.validate_field_operation(old_config, target_config) # Validate target config does not have empty tables since they do not show up in ConfigDb self.logger.log_notice("Validating target config does not have empty tables, " \ diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 22e8d820d8..d2561d0532 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -16,6 +16,9 @@ class GenericConfigUpdaterError(Exception): pass +class IllegalPatchOperationError(ValueError): + pass + class EmptyTableError(ValueError): pass @@ -136,6 +139,22 @@ def validate_config_db_config(self, config_db_as_json): return True, None + def validate_field_operation(self, old_config, target_config): + """ + Some fields in ConfigDB are restricted and may not allow third-party addition, replacement, or removal. + Because YANG only validates state and not transitions, this method helps to JsonPatch operations/transitions for the specified fields. + """ + patch = jsonpatch.JsonPatch.from_diff(old_config, target_config) + + # illegal_operations_to_fields_map['remove'] yields a list of fields for which `remove` is an illegal operation + illegal_operations_to_fields_map = {'add':[], + 'replace': [], + 'remove': ['/PFC_WD/GLOBAL/POLL_INTERVAL', '/PFC_WD/GLOBAL']} + for operation, field_list in illegal_operations_to_fields_map.items(): + for field in field_list: + if any(op['op'] == operation and field == op['path'] for op in patch): + raise IllegalPatchOperationError("Given patch operation is invalid. Operation: {} is illegal on field: {}".format(operation, field)) + def validate_lanes(self, config_db): if "PORT" not in config_db: return True, None diff --git a/tests/generic_config_updater/gu_common_test.py b/tests/generic_config_updater/gu_common_test.py index 6ba4923664..398558bd65 100644 --- a/tests/generic_config_updater/gu_common_test.py +++ b/tests/generic_config_updater/gu_common_test.py @@ -57,6 +57,18 @@ def setUp(self): self.config_wrapper_mock = gu_common.ConfigWrapper() self.config_wrapper_mock.get_config_db_as_json=MagicMock(return_value=Files.CONFIG_DB_AS_JSON) + def test_validate_field_operation_legal(self): + old_config = {"PFC_WD": {"GLOBAL": {"POLL_INTERVAL": "60"}}} + target_config = {"PFC_WD": {"GLOBAL": {"POLL_INTERVAL": "40"}}} + config_wrapper = gu_common.ConfigWrapper() + config_wrapper.validate_field_operation(old_config, target_config) + + def test_validate_field_operation_illegal(self): + old_config = {"PFC_WD": {"GLOBAL": {"POLL_INTERVAL": 60}}} + target_config = {"PFC_WD": {"GLOBAL": {}}} + config_wrapper = gu_common.ConfigWrapper() + self.assertRaises(gu_common.IllegalPatchOperationError, config_wrapper.validate_field_operation, old_config, target_config) + def test_ctor__default_values_set(self): config_wrapper = gu_common.ConfigWrapper()