Skip to content

[action] [PR:3910] Improved GCU's field validation logic for the WRED_PROFILE table#3934

Merged
mssonicbld merged 1 commit intosonic-net:202505from
mssonicbld:cherry/202505/3910
Jun 25, 2025
Merged

[action] [PR:3910] Improved GCU's field validation logic for the WRED_PROFILE table#3934
mssonicbld merged 1 commit intosonic-net:202505from
mssonicbld:cherry/202505/3910

Conversation

@mssonicbld
Copy link
Collaborator

What I did

When using config apply-patch to update CONFIG_DB, GCU tries to validate the patch according to the rules in gcu_field_operation_validators.conf.json. As explained in Issue 22295, since fields for the WRED_PROFILE table in this file contain the profile name azure_lossless, patches that target other WRED profiles will be rejected with the following error:

Error: Failed to apply patch on the following scopes:
- localhost: Modification of WRED_PROFILE table is illegal- validating function generic_config_updater.field_operation_validators.rdma_config_update_validator returned False

How I did it

  1. Modified gcu_field_operation_validators.conf.json to remove the profile name azure_lossless from WRED_PROFILE fields.
  2. In order to minimize side effect for other tables, we created a new validation function specifically for the WRED_PROFILE table named wred_profile_config_update_validator. This function modifies the path in patch by removing the table name (i.e., WRED_PROFILE), and then converting the whole path to lowercase. Then it verifies that the last part of this path matches a field defined in gcu_field_operation_validators.conf.json for the WRED_PROFILE table. For example, a path like /WRED_PROFILE/AZURE_LOSSY/green_min_threshold will be converted to azure_lossy/green_min_threshold, which will match green_min_threshold.

How to verify it

Store a JSON patch that targets a WRED profile different from azure_lossless in a file, and then run sudo config apply-patch {your_patch_name.json}.

Test 1: Wrong WRED profile name
Patch: [{"op": "replace", "path": "/WRED_PROFILE/DUMMY/green_min_threshold", "value": "1234"}]
Result:

Failed to apply patch due to: Validate json patch: [{"op": "replace", "path": "/WRED_PROFILE/DUMMY/green_min_threshold", "value": "1234"}] failed due to:member 'DUMMY' not found

Test 2: Existing field name that is not in gcu_field_operation_validators.conf.json
Patch: [{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/red_min_threshold", "value": "1234"}]
Result:

Error: Failed to apply patch on the following scopes:
- localhost: Modification of WRED_PROFILE table is illegal- validating function generic_config_updater.field_operation_validators.wred_profile_config_update_validator returned False

Test 3: Correct patch for 1 field
Patch: [{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/green_min_threshold", "value": "1234"}]
Result:

Patch applied successfully.
$ ecnconfig -l
Profile: AZURE_LOSSY
-----------------------  -------
...
green_min_threshold      1234
...
-----------------------  -------

Test 4: Non-existent field name
Patch: [{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/min_threshold", "value": "2468"}]
Result:

Error: Validate json patch: [{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/min_threshold", "value": "2468"}] failed due to:can't replace a non-existent object 'min_threshold'

Test 5: Correct patch for multiple fields
Patch: [{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/green_min_threshold", "value": "2468"},{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/green_max_threshold", "value": "9876"},{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/green_drop_probability", "value": "10"}]
Result:

Patch applied successfully.
$ ecnconfig -l
Profile: AZURE_LOSSY
-----------------------  -------
...
green_drop_probability   10
green_max_threshold      9876
green_min_threshold      2468
...
-----------------------  -------

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

N/A

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

N/A

<!--
    Please make sure you've read and understood our contributing guidelines:
    https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

    ** Make sure all your commits include a signature generated with `git commit -s` **

    If this is a bug fix, make sure your description includes "closes #xxxx",
    "fixes #xxxx" or "resolves #xxxx" so that GitHub automatically closes the related
    issue when the PR is merged.

    If you are adding/modifying/removing any command or utility script, please also
    make sure to add/modify/remove any unit tests from the tests
    directory as appropriate.

    If you are modifying or removing an existing 'show', 'config' or 'sonic-clear'
    subcommand, or you are adding a new subcommand, please make sure you also
    update the Command Line Reference Guide (doc/Command-Reference.md) to reflect
    your changes.

    Please provide the following information:
-->

#### What I did
When using `config apply-patch` to update CONFIG_DB, GCU tries to validate the patch according to the rules in `gcu_field_operation_validators.conf.json`. As explained in [Issue 22295](sonic-net/sonic-buildimage#22295), since fields for the `WRED_PROFILE` table in this file contain the profile name `azure_lossless`, patches that target other WRED profiles will be rejected with the following error:
```
Error: Failed to apply patch on the following scopes:
- localhost: Modification of WRED_PROFILE table is illegal- validating function generic_config_updater.field_operation_validators.rdma_config_update_validator returned False
```

#### How I did it
1. Modified `gcu_field_operation_validators.conf.json` to remove the profile name `azure_lossless` from `WRED_PROFILE` fields.
2. In order to minimize side effect for other tables, we created a new validation function specifically for the `WRED_PROFILE` table named `wred_profile_config_update_validator`. This function modifies the path in patch by removing the table name (i.e., `WRED_PROFILE`), and then converting the whole path to lowercase. Then it verifies that the last part of this path matches a field defined in `gcu_field_operation_validators.conf.json` for the `WRED_PROFILE` table. For example, a path like `/WRED_PROFILE/AZURE_LOSSY/green_min_threshold` will be converted to `azure_lossy/green_min_threshold`, which will match `green_min_threshold`.

#### How to verify it
Store a JSON patch that targets a WRED profile different from `azure_lossless` in a file, and then run `sudo config apply-patch {your_patch_name.json}`.

*Test 1: Wrong WRED profile name*
Patch: `[{"op": "replace", "path": "/WRED_PROFILE/DUMMY/green_min_threshold", "value": "1234"}]`
Result:
```
Failed to apply patch due to: Validate json patch: [{"op": "replace", "path": "/WRED_PROFILE/DUMMY/green_min_threshold", "value": "1234"}] failed due to:member 'DUMMY' not found
```

*Test 2: Existing field name that is not in `gcu_field_operation_validators.conf.json`*
Patch: `[{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/red_min_threshold", "value": "1234"}]`
Result:
```
Error: Failed to apply patch on the following scopes:
- localhost: Modification of WRED_PROFILE table is illegal- validating function generic_config_updater.field_operation_validators.wred_profile_config_update_validator returned False
```

*Test 3: Correct patch for 1 field*
Patch: `[{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/green_min_threshold", "value": "1234"}]`
Result:
```
Patch applied successfully.
$ ecnconfig -l
Profile: AZURE_LOSSY
-----------------------  -------
...
green_min_threshold      1234
...
-----------------------  -------
```

*Test 4: Non-existent field name*
Patch: `[{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/min_threshold", "value": "2468"}]`
Result:
```
Error: Validate json patch: [{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/min_threshold", "value": "2468"}] failed due to:can't replace a non-existent object 'min_threshold'
```

*Test 5: Correct patch for multiple fields*
Patch: `[{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/green_min_threshold", "value": "2468"},{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/green_max_threshold", "value": "9876"},{"op": "replace", "path": "/WRED_PROFILE/AZURE_LOSSY/green_drop_probability", "value": "10"}]`
Result:
```
Patch applied successfully.
$ ecnconfig -l
Profile: AZURE_LOSSY
-----------------------  -------
...
green_drop_probability   10
green_max_threshold      9876
green_min_threshold      2468
...
-----------------------  -------
```

#### Previous command output (if the output of a command-line utility has changed)
N/A

#### New command output (if the output of a command-line utility has changed)
N/A
@mssonicbld
Copy link
Collaborator Author

Original PR: #3910

@mssonicbld
Copy link
Collaborator Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld mssonicbld merged commit a4212fe into sonic-net:202505 Jun 25, 2025
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant