From 7d013dff07d6b8a32f0f9d635822956d25d1a821 Mon Sep 17 00:00:00 2001 From: Xincun Li <147451452+xincunli-sonic@users.noreply.github.com> Date: Thu, 7 Nov 2024 16:07:47 -0800 Subject: [PATCH] Fix slash in path. (#3573) #### What I did Addressing issue [#20377](https://github.com/sonic-net/sonic-buildimage/issues/20377). The issue caused by unescape in JsonPointer implementation which followed [RFC 6901](https://www.rfc-editor.org/rfc/rfc6901) ```python pointer = jsonpointer.JsonPointer(path) ... class JsonPointer(object): """A JSON Pointer that can reference parts of a JSON document""" # Array indices must not contain: # leading zeros, signs, spaces, decimals, etc _RE_ARRAY_INDEX = re.compile('0|[1-9][0-9]*$') _RE_INVALID_ESCAPE = re.compile('(~[^01]|~$)') def __init__(self, pointer): # validate escapes invalid_escape = self._RE_INVALID_ESCAPE.search(pointer) if invalid_escape: raise JsonPointerException('Found invalid escape {}'.format( invalid_escape.group())) parts = pointer.split('/') if parts.pop(0) != '': raise JsonPointerException('Location must start with /') parts = [unescape(part) for part in parts] self.parts = parts ``` #### How I did it Re-escape `/` to `~1` to match the real key in json, and [JsonPatch](https://www.rfc-editor.org/rfc/rfc6902#appendix-A.14) will handle it correctly. #### How to verify it ```shell admin@str2-7250-lc1-2:~$ cat link.json [ { "op": "add", "path": "/asic1/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131", "value": {} } ] admin@str2-7250-lc1-2:~$ sudo config apply-patch link.json sonic_yang(6):Note: Below table(s) have no YANG models: DHCP_SERVER sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER sonic_yang(6):Note: Below table(s) have no YANG models: LOGGER Patch Applier: asic1: Patch application starting. Patch Applier: asic1: Patch: [{"op": "add", "path": "/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131", "value": {}}] Patch Applier: asic1 getting current config db. Patch Applier: asic1: simulating the target full config after applying the patch. Patch Applier: asic1: validating all JsonPatch operations are permitted on the specified fields Patch Applier: asic1: validating target config does not have empty tables, since they do not show up in ConfigDb. Patch Applier: asic1: sorting patch updates. Patch Applier: The asic1 patch was converted into 0 changes. Patch Applier: asic1: applying 0 changes in order. Patch Applier: asic1: verifying patch updates are reflected on ConfigDB. Patch Applier: asic1 patch application completed. Patch applied successfully. ``` --- generic_config_updater/generic_updater.py | 6 ++++-- .../multiasic_change_applier_test.py | 20 +++++++++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/generic_config_updater/generic_updater.py b/generic_config_updater/generic_updater.py index 8ce27455bb..e8bb021808 100644 --- a/generic_config_updater/generic_updater.py +++ b/generic_config_updater/generic_updater.py @@ -17,9 +17,11 @@ def extract_scope(path): if not path: - raise Exception("Wrong patch with empty path.") + raise GenericConfigUpdaterError("Wrong patch with empty path.") pointer = jsonpointer.JsonPointer(path) - parts = pointer.parts + + # Re-escapes + parts = [jsonpointer.escape(part) for part in pointer.parts] if not parts: raise GenericConfigUpdaterError("Wrong patch with empty path.") if parts[0].startswith("asic"): diff --git a/tests/generic_config_updater/multiasic_change_applier_test.py b/tests/generic_config_updater/multiasic_change_applier_test.py index afa1b449f3..743969737d 100644 --- a/tests/generic_config_updater/multiasic_change_applier_test.py +++ b/tests/generic_config_updater/multiasic_change_applier_test.py @@ -1,7 +1,9 @@ +import jsonpointer import unittest from importlib import reload from unittest.mock import patch, MagicMock from generic_config_updater.generic_updater import extract_scope +from generic_config_updater.generic_updater import GenericConfigUpdaterError import generic_config_updater.change_applier import generic_config_updater.services_validator import generic_config_updater.gu_common @@ -49,6 +51,12 @@ def test_extract_scope_multiasic(self, mock_is_multi_asic): "/asic0123456789/PORTCHANNEL/PortChannel102/admin_status": ( True, "asic0123456789", "/PORTCHANNEL/PortChannel102/admin_status" ), + "/asic1/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6/31": ( + True, "asic1", "/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6/31" + ), + "/asic1/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131": ( + True, "asic1", "/PORTCHANNEL_INTERFACE/PortChannel106|10.0.0.6~131" + ), "/localhost/BGP_DEVICE_GLOBAL/STATE/tsa_enabled": ( True, "localhost", "/BGP_DEVICE_GLOBAL/STATE/tsa_enabled" ), @@ -95,7 +103,11 @@ def test_extract_scope_multiasic(self, mock_is_multi_asic): scope, remainder = extract_scope(test_path) assert(scope == expectedscope) assert(remainder == expectedremainder) - except Exception: + except AssertionError: + assert(not result) + except GenericConfigUpdaterError: + assert(not result) + except jsonpointer.JsonPointerException: assert(not result) @patch('sonic_py_common.multi_asic.is_multi_asic') @@ -158,7 +170,11 @@ def test_extract_scope_singleasic(self, mock_is_multi_asic): scope, remainder = extract_scope(test_path) assert(scope == expectedscope) assert(remainder == expectedremainder) - except Exception: + except AssertionError: + assert(not result) + except GenericConfigUpdaterError: + assert(not result) + except jsonpointer.JsonPointerException: assert(not result) @patch('generic_config_updater.change_applier.get_config_db_as_json', autospec=True)