sonic-utilities icon indicating copy to clipboard operation
sonic-utilities copied to clipboard

Fix slash in path.

Open xincunli-sonic opened this issue 1 year ago • 3 comments

What I did

Addressing issue #20377. The issue caused by unescape in JsonPointer implementation which followed RFC 6901

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 will handle it correctly.

How to verify it

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.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

xincunli-sonic avatar Oct 11 '24 16:10 xincunli-sonic

        except Exception:

Can we use specific (not too general) exception type? #Closed


Refers to: tests/generic_config_updater/multiasic_change_applier_test.py:80 in 26ee2a4. [](commit_id = 26ee2a4e99bbfb1adff460621950355c5ea69646, deletion_comment = False)

qiluo-msft avatar Oct 28 '24 23:10 qiluo-msft

        except Exception:

still applicable.


In reply to: 2442871334


Refers to: tests/generic_config_updater/multiasic_change_applier_test.py:80 in 26ee2a4. [](commit_id = 26ee2a4e99bbfb1adff460621950355c5ea69646, deletion_comment = False)

qiluo-msft avatar Oct 29 '24 23:10 qiluo-msft

discussed offline, change to specific error handling to avoid unexpected errors.

xincunli-sonic avatar Oct 29 '24 23:10 xincunli-sonic

@xincunli-sonic This PR has cherry-pick conflict. Please help to address it so that can cherry-pick to 202405. Thanks

mlok-nokia avatar Nov 25 '24 14:11 mlok-nokia

This requires the previous PR be cherry picked into 202405, after it done, I will reuquest the current PR backport again.

xincunli-sonic avatar Nov 26 '24 22:11 xincunli-sonic

This requires the previous PR be cherry picked into 202405, after it done, I will reuquest the current PR backport again.

Thanks.

mlok-nokia avatar Nov 27 '24 14:11 mlok-nokia

Included in 202405 by PR https://github.com/sonic-net/sonic-utilities/pull/3649

bingwang-ms avatar Feb 04 '25 22:02 bingwang-ms