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

[okaravasi]

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:

1. Apply below steps in a duthost multi-asic.
2. 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

[okaravasi]

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

[xincunli-sonic]

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

[okaravasi]

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]

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!