Skip to content

Commit

Permalink
Fix slash in path. (sonic-net#3573)
Browse files Browse the repository at this point in the history
#### What I did

Addressing issue [#20377](sonic-net/sonic-buildimage#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.
```
  • Loading branch information
xincunli-sonic authored Nov 8, 2024
1 parent 0af4386 commit 7d013df
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
6 changes: 4 additions & 2 deletions generic_config_updater/generic_updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"):
Expand Down
20 changes: 18 additions & 2 deletions tests/generic_config_updater/multiasic_change_applier_test.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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"
),
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 7d013df

Please sign in to comment.