[GCU] [MA] Apply-patch fails if 'path' value includes '/' which is encoded as '~1' #20377

okaravasi · 2024-09-30T08:54:21Z

Description

In case the path value in a json file that is used in gcu with 'sudo config apply-patch' includes '/', which is replaced with as '~1', the patch fails as the path is not recognized.

As it seems the code reads the code until the '~1' characters and tries to find the path as it is with as a result getting a KeyError.
This behavior seems to be broken only in multi-asics platform where namespace prefix is used in path.

Please see the console output from an apply-patch with a path that falls above category in a successful run (single-asic) vs a failed run (in multi-asic).

# Contents of 'add_portchannel_single_asic.json'

[
        {
       "op": "add",
       "path": "/PORTCHANNEL_INTERFACE/PortChannel101|10.0.0.56~131",
       "value": {}
   }
]

# Apply patch of 'add_portchannel_single_asic.json' (SUCCESS)

admin@vlab-01:$ sudo config apply-patch add_portchannel_single_asic.json
Patch Applier: localhost: Patch application starting.
Patch Applier: localhost: Patch: [{"op": "add", "path": "/PORTCHANNEL_INTERFACE/PortChannel101|10.0.0.56131", "value": {}}]
Patch Applier: localhost getting current config db.
Patch Applier: localhost: simulating the target full config after applying the patch.
Patch Applier: localhost: validating all JsonPatch operations are permitted on the specified fields
Patch Applier: localhost: validating target config does not have empty tables,
since they do not show up in ConfigDb.
Patch Applier: localhost: sorting patch updates.
Patch Applier: The localhost patch was converted into 0 changes.
Patch Applier: localhost: applying 0 changes in order.
Patch Applier: localhost: verifying patch updates are reflected on ConfigDB.
Patch Applier: localhost patch application completed.
Patch applied successfully.

# Contents of 'add_portchannel_multi_asic.json'

[
	{
       "op": "add",
       "path": "/asic1/PORTCHANNEL_INTERFACE/PortChannel105|10.0.0.18~131",
       "value": {}
   }
]

# Apply patch of 'add_portchannel_multi_asic.json' (FAILED)

admin@ixre-egl-board30:~$ sudo add_portchannel_multi_asic.json
Patch Applier: asic1: Patch application starting.
Patch Applier: asic1: Patch: [{"op": "add", "path": "/PORTCHANNEL_INTERFACE/PortChannel105|10.0.0.18/31", "value": {}}]
Patch Applier: asic1 getting current config db.
Patch Applier: asic1: simulating the target full config after applying the patch.
Failed to apply patch due to: Failed to apply patch on the following scopes:

asic1: member 'PortChannel105|10.0.0.18' not found in {'PortChannel105': {}, 'PortChannel105|10.0.0.18/31': {}, 'PortChannel105|FC00::25/126': {}}
Usage: config apply-patch [OPTIONS] PATCH_FILE_PATH
Try "config apply-patch -h" for help.

Error: Failed to apply patch on the following scopes:

asic1: member 'PortChannel105|10.0.0.18' not found in {'PortChannel105': {}, 'PortChannel105|10.0.0.18/31': {}, 'PortChannel105|FC00::25/126': {}}

Steps to reproduce the issue

Failed test cases:

generic_config_updater/test_lo_interface.py/test_lo_interface_tc1_suite
generic_config_updater/test_portchannel_interface.py/test_portchannel_interface_tc1_suite

(Automated test cases support for running in t2 topology is added via PR sonic-net/sonic-mgmt#14070 )

Manual steps:

Apply below steps in a duthost multi-asic.
Create a file "tmp.json" to add a path that includes '/'.
Example:

[
        {
       "op": "add",
       "path": "/asic1/PORTCHANNEL_INTERFACE/PortChannel105|10.0.0.18~131",
       "value": {}
   }
]

4.sudo config apply-patch tmp.json

Describe the results you received

apply-patch fails and path is not recognized as expected.

Failed to apply patch due to: Failed to apply patch on the following scopes:

asic1: member 'PortChannel105|10.0.0.18' not found in {'PortChannel105': {}, 'PortChannel105|10.0.0.18/31': {}, 'PortChannel105|FC00::25/126': {}}

Here, member 'PortChannel105|10.0.0.18' is recognized although it should read 'PortChannel105|10.0.0.18/31'.

Describe the results you expected

apply-patch shoudl succeed.

In above error, member 'PortChannel105|10.0.0.18' is recognized although it should read 'PortChannel105|10.0.0.18/31'.

Additional information you deem important (e.g. issue happens only occasionally)

This behavior is successful in single-asic platforms. Issue is only for multi-asic.

Output of `show version`

admin@ixre-egl-board9:~$ show version

SONiC Software Version: SONiC.HEAD.832829-nokia-master-bf8e2c9a7
SONiC OS Version: 12
Distribution: Debian 12.7
Kernel: 6.1.0-22-2-amd64
Build commit: bf8e2c9a7
Build date: Wed Sep 25 15:57:47 UTC 2024
Built by: gitlab-runner@wfrv-sonicbld06

Platform: x86_64-nokia_ixr7250e_36x400g-r0
HwSKU: Nokia-IXR7250E-36x400G
ASIC: broadcom
ASIC Count: 2
Serial Number: EAG2-02-045
Model Number: N/A
Hardware Revision: 56
Uptime: 13:50:23 up  5:23,  1 user,  load average: 1.65, 1.89, 1.89
Date: Fri 27 Sep 2024 13:50:23

Docker images:
REPOSITORY                    TAG                                  IMAGE ID       SIZE
docker-macsec                 latest                               f178be710750   406MB
docker-dhcp-relay             latest                               e5960cfc6019   384MB
docker-snmp                   HEAD.832829-nokia-master-bf8e2c9a7   f1902b7928cb   418MB
docker-snmp                   latest                               f1902b7928cb   418MB
docker-platform-monitor       HEAD.832829-nokia-master-bf8e2c9a7   6a906f0e6739   459MB
docker-platform-monitor       latest                               6a906f0e6739   459MB
docker-orchagent              HEAD.832829-nokia-master-bf8e2c9a7   76bc76f58932   416MB
docker-orchagent              latest                               76bc76f58932   416MB
docker-nat                    HEAD.832829-nokia-master-bf8e2c9a7   0bf0a967a985   406MB
docker-nat                    latest                               0bf0a967a985   406MB
docker-fpm-frr                HEAD.832829-nokia-master-bf8e2c9a7   624306e87567   435MB
docker-fpm-frr                latest                               624306e87567   435MB
docker-eventd                 HEAD.832829-nokia-master-bf8e2c9a7   d147b480e6e5   374MB
docker-eventd                 latest                               d147b480e6e5   374MB
docker-database               HEAD.832829-nokia-master-bf8e2c9a7   091078e9aebd   383MB
docker-database               latest                               091078e9aebd   383MB
docker-sonic-mgmt-framework   HEAD.832829-nokia-master-bf8e2c9a7   3d2388474edc   424MB
docker-sonic-mgmt-framework   latest                               3d2388474edc   424MB
docker-teamd                  HEAD.832829-nokia-master-bf8e2c9a7   42c01511be73   403MB
docker-teamd                  latest                               42c01511be73   403MB
docker-sflow                  HEAD.832829-nokia-master-bf8e2c9a7   7fa8d1cd42d2   404MB
docker-sflow                  latest                               7fa8d1cd42d2   404MB
docker-router-advertiser      HEAD.832829-nokia-master-bf8e2c9a7   c2b35825bf43   374MB
docker-router-advertiser      latest                               c2b35825bf43   374MB
docker-mux                    HEAD.832829-nokia-master-bf8e2c9a7   d8bfad4511df   386MB
docker-mux                    latest                               d8bfad4511df   386MB
docker-lldp                   HEAD.832829-nokia-master-bf8e2c9a7   51d3f507b05a   383MB
docker-lldp                   latest                               51d3f507b05a   383MB
docker-sonic-gnmi             HEAD.832829-nokia-master-bf8e2c9a7   a46ced6f5be5   459MB
docker-sonic-gnmi             latest                               a46ced6f5be5   459MB
docker-syncd-brcm-dnx         HEAD.832829-nokia-master-bf8e2c9a7   6ed78c7518fb   759MB
docker-syncd-brcm-dnx         latest                               6ed78c7518fb   759MB
docker-gbsyncd-broncos        HEAD.832829-nokia-master-bf8e2c9a7   7d357e969457   410MB
docker-gbsyncd-broncos        latest                               7d357e969457   410MB
docker-gbsyncd-credo          HEAD.832829-nokia-master-bf8e2c9a7   4a412757e74c   383MB
docker-gbsyncd-credo          latest                               4a412757e74c   383MB

The text was updated successfully, but these errors were encountered:

okaravasi · 2024-09-30T08:56:05Z

Moving issue from sonic-utilities repo, here. Closing the original ticket (sonic-net/sonic-utilities#3559) as duplicate.

xincunli-sonic · 2024-10-09T22:18:24Z

This is expected, please ref this issue: json-patch/json-patch-tests#42

okaravasi · 2024-10-10T14:41:32Z

This is expected, please ref this issue: json-patch/json-patch-tests#42

Indeed @xincunli-sonic , the replacement of '/' with '~1' is expected, as referenced in the ticket you mentioned. However, this is not the issue we are encountering. Could you please refer to the issue description?

We are using the JSON path in the expected way, and while it works correctly in single-ASIC, we are experiencing failures in multi-ASIC. It seems there is a change in how the path is being interpreted in multi-ASIC, which is causing the functionality to break. Could you please investigate this further?

xincunli-sonic · 2024-10-11T16:57:28Z

Thanks @okaravasi explanation! This is a good finding, here is the fix: sonic-net/sonic-utilities#3573, please help verify the change and help sign off, Thanks!

okaravasi · 2024-10-17T14:50:27Z

Thanks @okaravasi explanation! This is a good finding, here is the fix: sonic-net/sonic-utilities#3573, please help verify the change and help sign off, Thanks!

@xincunli-sonic I tested manually the fix from sonic-net/sonic-utilities#3573 in t2 multi-asic and vs-kvm-t0 and seems ok.

#### 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. ```

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""" _RE_ARRAY_INDEX = re.compile('0|[1-9][0-9]*$') _RE_INVALID_ESCAPE = re.compile('(~[^01]|~$)') def __init__(self, pointer): 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 ``` 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. ```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. ```

okaravasi mentioned this issue Sep 30, 2024

[GCU] [MA] Apply-patch fails if 'path' includes '/' which is provided as '~1' sonic-net/sonic-utilities#3559

Closed

mlok-nokia added this to SONiC Chassis Oct 2, 2024

judyjoseph assigned xincunli-sonic Oct 9, 2024

judyjoseph added Triaged this issue has been triaged MSFT Multi-ASIC labels Oct 9, 2024

xincunli-sonic assigned okaravasi and unassigned xincunli-sonic Oct 9, 2024

okaravasi removed their assignment Oct 17, 2024

xincunli-sonic mentioned this issue Oct 22, 2024

Fix slash in path. sonic-net/sonic-utilities#3573

Merged

arlakshm added Issue for 202405 Chassis 🤖 Modular chassis support labels Oct 25, 2024

xincunli-sonic closed this as completed Nov 12, 2024

github-project-automation bot moved this to Done in SONiC Chassis Nov 12, 2024

xincunli-sonic mentioned this issue Nov 26, 2024

Fix slash in path. (#3573) sonic-net/sonic-utilities#3628

Closed

xincunli-sonic mentioned this issue Dec 4, 2024

Fix slash in path. (#3573) sonic-net/sonic-utilities#3649

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GCU] [MA] Apply-patch fails if 'path' value includes '/' which is encoded as '~1' #20377

[GCU] [MA] Apply-patch fails if 'path' value includes '/' which is encoded as '~1' #20377

okaravasi commented Sep 30, 2024

okaravasi commented Sep 30, 2024 •

edited

Loading

xincunli-sonic commented Oct 9, 2024

okaravasi commented Oct 10, 2024

xincunli-sonic commented Oct 11, 2024 •

edited

Loading

okaravasi commented Oct 17, 2024

[GCU] [MA] Apply-patch fails if 'path' value includes '/' which is encoded as '~1' #20377

[GCU] [MA] Apply-patch fails if 'path' value includes '/' which is encoded as '~1' #20377

Comments

okaravasi commented Sep 30, 2024

Description

Steps to reproduce the issue

Describe the results you received

Describe the results you expected

Additional information you deem important (e.g. issue happens only occasionally)

Output of show version

okaravasi commented Sep 30, 2024 • edited Loading

xincunli-sonic commented Oct 9, 2024

okaravasi commented Oct 10, 2024

xincunli-sonic commented Oct 11, 2024 • edited Loading

okaravasi commented Oct 17, 2024

Output of `show version`

okaravasi commented Sep 30, 2024 •

edited

Loading

xincunli-sonic commented Oct 11, 2024 •

edited

Loading