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

generic_config_updater/field_operation_validators.py

@@ -67,31 +67,52 @@ def get_asic_name():
     return asic
 
 
-def rdma_config_update_validator(scope, patch_element):
+def fields_match_exact(cleaned_patch_field, gcu_field):
+    return cleaned_patch_field == gcu_field
+
+
+def fields_match_endswith(cleaned_patch_field, gcu_field):
+    """
+    Checks if cleaned_patch_field ends with gcu_field
+    """
+    field = cleaned_patch_field.split('/')[-1]
+    return field == gcu_field
+
+
+# If exact_field_match is True, then each field in GCU_TABLE_MOD_CONF_FILE must match exactly with
+# the corresponding cleaned field from the patch.
+# If exact_field_match is False, then each field in GCU_TABLE_MOD_CONF_FILE must appear at the end of
+# the corresponding cleaned fields from the patch.
+# remove_port controls the behavior of the _get_fields_in_patch function.
+def rdma_config_update_validator_common(scope, patch_element, exact_field_match=False, remove_port=False):
     asic = get_asic_name()
     if asic == "unknown":
         return False
     version_info = device_info.get_sonic_version_info()
     build_version = version_info.get('build_version')
     version_substrings = build_version.split('.')
     branch_version = None
-    
+
     for substring in version_substrings:
         if substring.isdigit() and re.match(r'^\d{8}$', substring):
             branch_version = substring
-    
+
     path = patch_element["path"]
     table = jsonpointer.JsonPointer(path).parts[0]
-    
+
     # Helper function to return relevant cleaned paths, considers case where the jsonpatch value is a dict
-    # For paths like /PFC_WD/Ethernet112/action, remove Ethernet112 from the path so that we can clearly determine the relevant field (i.e. action, not Ethernet112)
+    # If remove_port is True, then for paths like /PFC_WD/Ethernet112/action, remove Ethernet112 from
+    # the path so that we can clearly determine the relevant field (i.e. action, not Ethernet112)
     def _get_fields_in_patch():
         cleaned_fields = []
 
         field_elements = jsonpointer.JsonPointer(path).parts[1:]
-        cleaned_field_elements = [elem for elem in field_elements if not any(char.isdigit() for char in elem)]
+        if remove_port:
+            cleaned_field_elements = [elem for elem in field_elements if not any(char.isdigit() for char in elem)]
+        else:
+            cleaned_field_elements = field_elements
         cleaned_field = '/'.join(cleaned_field_elements).lower()
-        
+
 
         if 'value' in patch_element.keys() and isinstance(patch_element['value'], dict):
             for key in patch_element['value']:
@@ -103,7 +124,7 @@ def _get_fields_in_patch():
             cleaned_fields.append(cleaned_field)
 
         return cleaned_fields
-    
+
     if os.path.exists(GCU_TABLE_MOD_CONF_FILE):
         with open(GCU_TABLE_MOD_CONF_FILE, "r") as s:
             gcu_field_operation_conf = json.load(s)
@@ -112,24 +133,27 @@ def _get_fields_in_patch():
 
     tables = gcu_field_operation_conf["tables"]
     scenarios = tables[table]["validator_data"]["rdma_config_update_validator"]
-
-    cleaned_fields = _get_fields_in_patch()
-    for cleaned_field in cleaned_fields:
+    cleaned_patch_fields = _get_fields_in_patch()
+    fields_match = fields_match_exact if exact_field_match else fields_match_endswith
+    for cleaned_patch_field in cleaned_patch_fields:
         scenario = None
         for key in scenarios.keys():
-            if cleaned_field in scenarios[key]["fields"]:
-                scenario = scenarios[key]
+            for gcu_field in scenarios[key]["fields"]:
+                if fields_match(cleaned_patch_field, gcu_field):
+                    scenario = scenarios[key]
+                    break
+            if scenario:
                 break
-    
+
         if scenario is None:
             return False
-        
-        if scenario["platforms"][asic] == "":
+
+        if not scenario["platforms"].get(asic):  # None or empty string
             return False
 
         if patch_element['op'] not in scenario["operations"]:
             return False
-    
+
         if branch_version is not None:
             if asic in scenario["platforms"]:
                 if branch_version < scenario["platforms"][asic]:
@@ -140,6 +164,14 @@ def _get_fields_in_patch():
     return True
 
 
+def rdma_config_update_validator(scope, patch_element):
+    return rdma_config_update_validator_common(scope, patch_element, exact_field_match=True, remove_port=True)
+
+
+def wred_profile_config_update_validator(scope, patch_element):
+    return rdma_config_update_validator_common(scope, patch_element)
+
+
 def read_statedb_entry(scope, table, key, field):
     state_db = swsscommon.DBConnector(STATE_DB_NAME, REDIS_TIMEOUT_MSECS, True, scope)
     tbl = swsscommon.Table(state_db, table)

generic_config_updater/gcu_field_operation_validators.conf.json

@@ -165,19 +165,19 @@
             }
         },
         "WRED_PROFILE": {
-            "field_operation_validators": [ "generic_config_updater.field_operation_validators.rdma_config_update_validator" ],
+            "field_operation_validators": [ "generic_config_updater.field_operation_validators.wred_profile_config_update_validator" ],
             "validator_data": {
                 "rdma_config_update_validator": {
                     "ECN tuning": {
                         "fields": [
-                            "azure_lossless/green_min_threshold",
-                            "azure_lossless/green_max_threshold",
-                            "azure_lossless/green_drop_probability"
+                            "green_min_threshold",
+                            "green_max_threshold",
+                            "green_drop_probability"
                         ],
                         "operations": ["replace"],
                         "platforms": {
                             "spc1": "20181100",
-			    "spc2": "20191100",
+                            "spc2": "20191100",
                             "spc3": "20220500",
                             "spc4": "20221100",
                             "spc5": "20241200",

tests/generic_config_updater/field_operation_validator_test.py

@@ -293,6 +293,132 @@ def test_rdma_config_update_validator_spc_asic_other_field(self):
             assert generic_config_updater.field_operation_validators.\
                 rdma_config_update_validator(scope, patch_element) is False
 
+    @patch("sonic_py_common.device_info.get_sonic_version_info",
+           mock.Mock(return_value={"build_version": "20250530.12"}))
+    @patch("generic_config_updater.field_operation_validators.get_asic_name",
+           mock.Mock(return_value="th5"))
+    @patch("os.path.exists", mock.Mock(return_value=True))
+    @patch("builtins.open", mock_open(read_data='''{"tables": {"WRED_PROFILE": {"validator_data": {
+        "rdma_config_update_validator": {"ECN tuning": {"fields": [
+            "green_min_threshold", "green_max_threshold", "green_drop_probability"
+        ], "operations": ["replace"], "platforms": {"th5": "20240500"}}}}}}}'''))
+    def test_wred_profile_config_update_validator_lossless(self):
+        patch_element = {
+            "path": "/WRED_PROFILE/AZURE_LOSSLESS/green_min_threshold",
+            "op": "replace",
+            "value": "1234"
+        }
+        for scope in ["localhost", "asic0"]:
+            assert generic_config_updater.field_operation_validators.\
+                wred_profile_config_update_validator(scope, patch_element) is True
+
+    @patch("sonic_py_common.device_info.get_sonic_version_info",
+           mock.Mock(return_value={"build_version": "20250530.12"}))
+    @patch("generic_config_updater.field_operation_validators.get_asic_name",
+           mock.Mock(return_value="th5"))
+    @patch("os.path.exists", mock.Mock(return_value=True))
+    @patch("builtins.open", mock_open(read_data='''{"tables": {"WRED_PROFILE": {"validator_data": {
+        "rdma_config_update_validator": {"ECN tuning": {"fields": [
+            "green_min_threshold", "green_max_threshold", "green_drop_probability"
+        ], "operations": ["replace"], "platforms": {"th5": "20240500"}}}}}}}'''))
+    def test_wred_profile_config_update_validator_lossy(self):
+        patch_element = {
+            "path": "/WRED_PROFILE/AZURE_LOSSY/green_min_threshold",
+            "op": "replace",
+            "value": "1234"
+        }
+        for scope in ["localhost", "asic0"]:
+            assert generic_config_updater.field_operation_validators.\
+                wred_profile_config_update_validator(scope, patch_element) is True
+
+    @patch("sonic_py_common.device_info.get_sonic_version_info",
+           mock.Mock(return_value={"build_version": "20250530.12"}))
+    @patch("generic_config_updater.field_operation_validators.get_asic_name",
+           mock.Mock(return_value="th5"))
+    @patch("os.path.exists", mock.Mock(return_value=True))
+    @patch("builtins.open", mock_open(read_data='''{"tables": {"WRED_PROFILE": {"validator_data": {
+        "rdma_config_update_validator": {"ECN tuning": {"fields": [
+            "green_min_threshold", "green_max_threshold", "green_drop_probability"
+        ], "operations": ["replace"], "platforms": {"th5": "20240500"}}}}}}}'''))
+    def test_wred_profile_config_update_validator_invalid_field(self):
+        patch_element = {
+            "path": "/WRED_PROFILE/AZURE_LOSSY/invalid",
+            "op": "replace",
+            "value": "1234"
+        }
+        for scope in ["localhost", "asic0"]:
+            assert generic_config_updater.field_operation_validators.\
+                wred_profile_config_update_validator(scope, patch_element) is False
+
+    @patch("generic_config_updater.field_operation_validators.get_asic_name",
+           mock.Mock(return_value="unknown"))
+    def test_wred_profile_config_update_validator_unknown_asic(self):
+        patch_element = {
+            "path": "/WRED_PROFILE/AZURE_LOSSY/green_min_threshold",
+            "op": "replace",
+            "value": "1234"
+        }
+        for scope in ["localhost", "asic0"]:
+            assert generic_config_updater.field_operation_validators.\
+                wred_profile_config_update_validator(scope, patch_element) is False
+
+    @patch("sonic_py_common.device_info.get_sonic_version_info",
+           mock.Mock(return_value={"build_version": "SONiC.20220530"}))
+    @patch("generic_config_updater.field_operation_validators.get_asic_name",
+           mock.Mock(return_value="th5"))
+    @patch("os.path.exists", mock.Mock(return_value=True))
+    @patch("builtins.open", mock_open(read_data='''{"tables": {"WRED_PROFILE": {"validator_data": {
+        "rdma_config_update_validator": {"ECN tuning": {"fields": [
+            "green_min_threshold", "green_max_threshold", "green_drop_probability"
+        ], "operations": ["replace"], "platforms": {"th5": "20240500"}}}}}}}'''))
+    def test_wred_profile_config_update_validator_old_version(self):
+        patch_element = {
+            "path": "/WRED_PROFILE/AZURE_LOSSY/green_min_threshold",
+            "op": "replace",
+            "value": "1234"
+        }
+        for scope in ["localhost", "asic0"]:
+            assert generic_config_updater.field_operation_validators.\
+                wred_profile_config_update_validator(scope, patch_element) is False
+
+    @patch("sonic_py_common.device_info.get_sonic_version_info",
+           mock.Mock(return_value={"build_version": "20250530.12"}))
+    @patch("generic_config_updater.field_operation_validators.get_asic_name",
+           mock.Mock(return_value="th5"))
+    @patch("os.path.exists", mock.Mock(return_value=True))
+    @patch("builtins.open", mock_open(read_data='''{"tables": {"WRED_PROFILE": {"validator_data": {
+        "rdma_config_update_validator": {"ECN tuning": {"fields": [
+            "green_min_threshold", "green_max_threshold", "green_drop_probability"
+        ], "operations": ["replace"], "platforms": {"th5": "20240500"}}}}}}}'''))
+    def test_wred_profile_config_update_validator_invalid_op(self):
+        patch_element = {
+            "path": "/WRED_PROFILE/AZURE_LOSSY/green_min_threshold",
+            "op": "add",
+            "value": "1234"
+        }
+        for scope in ["localhost", "asic0"]:
+            assert generic_config_updater.field_operation_validators.\
+                wred_profile_config_update_validator(scope, patch_element) is False
+
+    @patch("sonic_py_common.device_info.get_sonic_version_info",
+           mock.Mock(return_value={"build_version": "20250530.12"}))
+    @patch("generic_config_updater.field_operation_validators.get_asic_name",
+           mock.Mock(return_value="spc1"))
+    @patch("os.path.exists", mock.Mock(return_value=True))
+    @patch("builtins.open", mock_open(read_data='''{"tables": {"WRED_PROFILE": {"validator_data": {
+        "rdma_config_update_validator": {"ECN tuning": {"fields": [
+            "green_min_threshold", "green_max_threshold", "green_drop_probability"
+        ], "operations": ["replace"], "platforms": {"th5": "20240500"}}}}}}}'''))
+    def test_wred_profile_config_update_validator_invalid_asic(self):
+        patch_element = {
+            "path": "/WRED_PROFILE/AZURE_LOSSY/green_min_threshold",
+            "op": "replace",
+            "value": "1234"
+        }
+        for scope in ["localhost", "asic0"]:
+            assert generic_config_updater.field_operation_validators.\
+                wred_profile_config_update_validator(scope, patch_element) is False
+
     def test_validate_field_operation_illegal__pfcwd(self):
         old_config = {"PFC_WD": {"GLOBAL": {"POLL_INTERVAL": "60"}}}
         target_config = {"PFC_WD": {"GLOBAL": {}}}