From 8f343ebb3303f234f39aeb46b4778b7ab6a07449 Mon Sep 17 00:00:00 2001 From: isabelmsft <67024108+isabelmsft@users.noreply.github.com> Date: Wed, 16 Aug 2023 15:27:23 -0700 Subject: [PATCH] [GCU] Add PORT table StateDB Validator (#2936) --- .../field_operation_validators.py | 77 +++++++++++++++++- .../gcu_field_operation_validators.conf.json | 3 + scripts/portconfig | 4 +- .../field_operation_validator_test.py | 79 +++++++++++++++++++ utilities_common/constants.py | 1 + 5 files changed, 161 insertions(+), 3 deletions(-) diff --git a/generic_config_updater/field_operation_validators.py b/generic_config_updater/field_operation_validators.py index a52a7f5a56..1aae399ade 100644 --- a/generic_config_updater/field_operation_validators.py +++ b/generic_config_updater/field_operation_validators.py @@ -5,7 +5,8 @@ import subprocess from sonic_py_common import device_info from .gu_common import GenericConfigUpdaterError - +from swsscommon import swsscommon +from utilities_common.constants import DEFAULT_SUPPORTED_FECS_LIST SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) GCU_TABLE_MOD_CONF_FILE = f"{SCRIPT_DIR}/gcu_field_operation_validators.conf.json" @@ -75,7 +76,7 @@ def rdma_config_update_validator(patch_element): path = patch_element["path"] table = jsonpointer.JsonPointer(path).parts[0] - # Helper function to return relevant cleaned paths, consdiers case where the jsonpatch value is a dict + # 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) def _get_fields_in_patch(): cleaned_fields = [] @@ -130,3 +131,75 @@ def _get_fields_in_patch(): return False return True + + +def read_statedb_entry(table, key, field): + state_db = swsscommon.DBConnector("STATE_DB", 0) + tbl = swsscommon.Table(state_db, table) + return tbl.hget(key, field)[1] + + +def port_config_update_validator(patch_element): + + def _validate_field(field, port, value): + if field == "fec": + supported_fecs_str = read_statedb_entry("PORT_TABLE", port, "supported_fecs") + if supported_fecs_str: + if supported_fecs_str != 'N/A': + supported_fecs_list = [element.strip() for element in supported_fecs_str.split(',')] + else: + supported_fecs_list = [] + else: + supported_fecs_list = DEFAULT_SUPPORTED_FECS_LIST + if value.strip() not in supported_fecs_list: + return False + return True + if field == "speed": + supported_speeds_str = read_statedb_entry("PORT_TABLE", port, "supported_speeds") or '' + try: + supported_speeds = [int(s) for s in supported_speeds_str.split(',') if s] + if supported_speeds and int(value) not in supported_speeds: + return False + except ValueError: + return False + return True + return False + + def _parse_port_from_path(path): + match = re.search(r"Ethernet\d+", path) + if match: + port = match.group(0) + return port + return None + + if patch_element["op"] == "remove": + return True + + # for PORT speed and fec configs, need to ensure value is allowed based on StateDB + patch_element_str = json.dumps(patch_element) + path = patch_element["path"] + value = patch_element.get("value") + fields = ['fec', 'speed'] + for field in fields: + if field in patch_element_str: + if path.endswith(field): + port = _parse_port_from_path(path) + if not _validate_field(field, port, value): + return False + elif isinstance(value, dict): + if field in value.keys(): + port = _parse_port_from_path(path) + value = value[field] + if not _validate_field(field, port, value): + return False + else: + for port_name, port_info in value.items(): + if isinstance(port_info, dict): + port = port_name + if field in port_info.keys(): + value = port_info[field] + if not _validate_field(field, port, value): + return False + else: + continue + return True diff --git a/generic_config_updater/gcu_field_operation_validators.conf.json b/generic_config_updater/gcu_field_operation_validators.conf.json index 7f7d1b9b02..f447b0d851 100644 --- a/generic_config_updater/gcu_field_operation_validators.conf.json +++ b/generic_config_updater/gcu_field_operation_validators.conf.json @@ -152,6 +152,9 @@ } } } + }, + "PORT": { + "field_operation_validators": [ "generic_config_updater.field_operation_validators.port_config_update_validator" ] } } } diff --git a/scripts/portconfig b/scripts/portconfig index becc203a90..acdebcc236 100755 --- a/scripts/portconfig +++ b/scripts/portconfig @@ -30,6 +30,8 @@ import sys import decimal import argparse +from utilities_common.constants import DEFAULT_SUPPORTED_FECS_LIST + # mock the redis for unit test purposes # try: if os.environ["UTILITIES_UNIT_TESTING"] == "1" or os.environ["UTILITIES_UNIT_TESTING"] == "2": @@ -276,7 +278,7 @@ class portconfig(object): else: supported_fecs_list = [] else: - supported_fecs_list = ["rs", "fc", "none"] + supported_fecs_list = DEFAULT_SUPPORTED_FECS_LIST return supported_fecs_list diff --git a/tests/generic_config_updater/field_operation_validator_test.py b/tests/generic_config_updater/field_operation_validator_test.py index 08a56dcf7c..1bc396740c 100644 --- a/tests/generic_config_updater/field_operation_validator_test.py +++ b/tests/generic_config_updater/field_operation_validator_test.py @@ -14,6 +14,85 @@ class TestValidateFieldOperation(unittest.TestCase): + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="")) + def test_port_config_update_validator_valid_speed_no_state_db(self): + patch_element = {"path": "/PORT/Ethernet3", "op": "add", "value": {"speed": "234"}} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == True + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="40000,30000")) + def test_port_config_update_validator_invalid_speed_existing_state_db(self): + patch_element = {"path": "/PORT/Ethernet3", "op": "add", "value": {"speed": "xyz"}} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == False + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) + def test_port_config_update_validator_valid_speed_existing_state_db(self): + patch_element = {"path": "/PORT/Ethernet3", "op": "add", "value": {"speed": "234"}} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == True + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) + def test_port_config_update_validator_valid_speed_existing_state_db(self): + patch_element = {"path": "/PORT/Ethernet3/speed", "op": "add", "value": "234"} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == True + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) + def test_port_config_update_validator_invalid_speed_existing_state_db(self): + patch_element = {"path": "/PORT/Ethernet3/speed", "op": "add", "value": "235"} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == False + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) + def test_port_config_update_validator_invalid_speed_existing_state_db_nested(self): + patch_element = {"path": "/PORT", "op": "add", "value": {"Ethernet3": {"alias": "Eth0", "speed": "235"}}} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == False + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) + def test_port_config_update_validator_valid_speed_existing_state_db_nested(self): + patch_element = {"path": "/PORT", "op": "add", "value": {"Ethernet3": {"alias": "Eth0", "speed": "234"}, "Ethernet4": {"alias": "Eth4", "speed": "234"}}} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == True + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) + def test_port_config_update_validator_invalid_speed_existing_state_db_nested_2(self): + patch_element = {"path": "/PORT", "op": "add", "value": {"Ethernet3": {"alias": "Eth0", "speed": "234"}, "Ethernet4": {"alias": "Eth4", "speed": "236"}}} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == False + + def test_port_config_update_validator_remove(self): + patch_element = {"path": "/PORT/Ethernet3", "op": "remove"} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == True + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="rs, fc")) + def test_port_config_update_validator_invalid_fec_existing_state_db(self): + patch_element = {"path": "/PORT/Ethernet3/fec", "op": "add", "value": "asf"} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == False + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="rs, fc")) + def test_port_config_update_validator_invalid_fec_existing_state_db_nested(self): + patch_element = {"path": "/PORT", "op": "add", "value": {"Ethernet3": {"alias": "Eth0", "fec": "none"}, "Ethernet4": {"alias": "Eth4", "fec": "fs"}}} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == False + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="rs, fc")) + def test_port_config_update_validator_valid_fec_existing_state_db_nested(self): + patch_element = {"path": "/PORT", "op": "add", "value": {"Ethernet3": {"alias": "Eth0", "fec": "fc"}}} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == True + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="rs, fc")) + def test_port_config_update_validator_valid_fec_existing_state_db_nested_2(self): + patch_element = {"path": "/PORT", "op": "add", "value": {"Ethernet3": {"alias": "Eth0", "fec": "rs"}, "Ethernet4": {"alias": "Eth4", "fec": "fc"}}} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == True + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="rs, fc")) + def test_port_config_update_validator_valid_fec_existing_state_db(self): + patch_element = {"path": "/PORT/Ethernet3/fec", "op": "add", "value": "rs"} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == True + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="")) + def test_port_config_update_validator_valid_fec_no_state_db(self): + patch_element = {"path": "/PORT/Ethernet3", "op": "add", "value": {"fec": "rs"}} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == True + + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="")) + def test_port_config_update_validator_invalid_fec_no_state_db(self): + patch_element = {"path": "/PORT/Ethernet3/fec", "op": "add", "value": "rsf"} + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) == False + @patch("generic_config_updater.field_operation_validators.get_asic_name", mock.Mock(return_value="unknown")) def test_rdma_config_update_validator_unknown_asic(self): patch_element = {"path": "/PFC_WD/Ethernet4/restoration_time", "op": "replace", "value": "234234"} diff --git a/utilities_common/constants.py b/utilities_common/constants.py index 536965d009..f5c157941c 100644 --- a/utilities_common/constants.py +++ b/utilities_common/constants.py @@ -1,6 +1,7 @@ #All the constant used in sonic-utilities DEFAULT_NAMESPACE = '' +DEFAULT_SUPPORTED_FECS_LIST = [ 'rs', 'fc', 'none'] DISPLAY_ALL = 'all' DISPLAY_EXTERNAL = 'frontend' BGP_NEIGH_OBJ = 'BGP_NEIGH'